Re: [PATCH v10 2/2] vhost-vdpa: add support for vIOMMU
On Mon, Oct 31, 2022 at 08:57:02PM +0800, Cindy Lu wrote:
> Add support for vIOMMU. add the new function to deal with iommu MR.
> - during iommu_region_add register a specific IOMMU notifier,
> and store all notifiers in a list.
> - during iommu_region_del, compare and delete the IOMMU notifier from the list
>
> Verified in vp_vdpa and vdpa_sim_net driver
>
> Signed-off-by: Cindy Lu
> ---
> hw/virtio/vhost-vdpa.c | 122 ++---
> include/hw/virtio/vhost-vdpa.h | 10 +++
> 2 files changed, 121 insertions(+), 11 deletions(-)
>
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 3ff9ce3501..e483d6e60b 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -26,6 +26,7 @@
> #include "cpu.h"
> #include "trace.h"
> #include "qapi/error.h"
> +#include "hw/virtio/virtio-access.h"
>
> /*
> * Return one past the end of the end of section. Be careful with uint64_t
> @@ -44,7 +45,6 @@ static bool
> vhost_vdpa_listener_skipped_section(MemoryRegionSection *section,
> uint64_t iova_min,
> uint64_t iova_max)
> {
> -Int128 llend;
>
> if ((!memory_region_is_ram(section->mr) &&
> !memory_region_is_iommu(section->mr)) ||
> @@ -61,14 +61,6 @@ static bool
> vhost_vdpa_listener_skipped_section(MemoryRegionSection *section,
> return true;
> }
>
> -llend = vhost_vdpa_section_end(section);
> -if (int128_gt(llend, int128_make64(iova_max))) {
> -error_report("RAM section out of device range (max=0x%" PRIx64
> - ", end addr=0x%" PRIx64 ")",
> - iova_max, int128_get64(llend));
> -return true;
> -}
> -
> return false;
> }
>
OK so you said this check is not really needed, and I would like
a separate patch to just drop this check with proper
explanation why it's safe.
Sorry if I was not clear this is what I asked for!
Thanks!
> @@ -173,6 +165,106 @@ static void vhost_vdpa_listener_commit(MemoryListener
> *listener)
> v->iotlb_batch_begin_sent = false;
> }
>
> +static void vhost_vdpa_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry
> *iotlb)
> +{
> +struct vdpa_iommu *iommu = container_of(n, struct vdpa_iommu, n);
> +
> +hwaddr iova = iotlb->iova + iommu->iommu_offset;
> +struct vhost_vdpa *v = iommu->dev;
> +void *vaddr;
> +int ret;
> +
> +if (iotlb->target_as != _space_memory) {
> +error_report("Wrong target AS \"%s\", only system memory is allowed",
> + iotlb->target_as->name ? iotlb->target_as->name :
> "none");
> +return;
> +}
> +RCU_READ_LOCK_GUARD();
> +vhost_vdpa_iotlb_batch_begin_once(v);
> +
> +if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
> +bool read_only;
> +
> +if (!memory_get_xlat_addr(iotlb, , NULL, _only, NULL)) {
> +return;
> +}
> +ret =
> +vhost_vdpa_dma_map(v, iova, iotlb->addr_mask + 1, vaddr,
> read_only);
> +if (ret) {
> +error_report("vhost_vdpa_dma_map(%p, 0x%" HWADDR_PRIx ", "
> + "0x%" HWADDR_PRIx ", %p) = %d (%m)",
> + v, iova, iotlb->addr_mask + 1, vaddr, ret);
> +}
> +} else {
> +ret = vhost_vdpa_dma_unmap(v, iova, iotlb->addr_mask + 1);
> +if (ret) {
> +error_report("vhost_vdpa_dma_unmap(%p, 0x%" HWADDR_PRIx ", "
> + "0x%" HWADDR_PRIx ") = %d (%m)",
> + v, iova, iotlb->addr_mask + 1, ret);
> +}
> +}
> +}
> +
> +static void vhost_vdpa_iommu_region_add(MemoryListener *listener,
> +MemoryRegionSection *section)
> +{
> +struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa,
> listener);
> +
> +struct vdpa_iommu *iommu;
> +Int128 end;
> +int iommu_idx;
> +IOMMUMemoryRegion *iommu_mr;
> +int ret;
> +
> +iommu_mr = IOMMU_MEMORY_REGION(section->mr);
> +
> +iommu = g_malloc0(sizeof(*iommu));
> +end = int128_add(int128_make64(section->offset_within_region),
> +section->size);
> +end = int128_sub(end, int128_one());
> +iommu_idx = memory_region_iommu_attrs_to_index(iommu_mr,
> +MEMTXATTRS_UNSPECIFIED);
> +
> +iommu->iommu_mr = iommu_mr;
> +
> +iommu_notifier_init(
> +>n, vhost_vdpa_iommu_map_notify, IOMMU_NOTIFIER_IOTLB_EVENTS,
> +section->offset_within_region, int128_get64(end), iommu_idx);
> +iommu->iommu_offset =
> +section->offset_within_address_space - section->offset_within_region;
> +iommu->dev = v;
> +
> +ret = memory_region_register_iommu_notifier(section->mr, >n,
> NULL);
> +if (ret) {
> +g_free(iommu);
> +return;
> +}
> +
> +QLIST_INSERT_HEAD(>iommu_list, iommu, iommu_next);
> +
[PATCH v10 2/2] vhost-vdpa: add support for vIOMMU
Add support for vIOMMU. add the new function to deal with iommu MR.
- during iommu_region_add register a specific IOMMU notifier,
and store all notifiers in a list.
- during iommu_region_del, compare and delete the IOMMU notifier from the list
Verified in vp_vdpa and vdpa_sim_net driver
Signed-off-by: Cindy Lu
---
hw/virtio/vhost-vdpa.c | 122 ++---
include/hw/virtio/vhost-vdpa.h | 10 +++
2 files changed, 121 insertions(+), 11 deletions(-)
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 3ff9ce3501..e483d6e60b 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -26,6 +26,7 @@
#include "cpu.h"
#include "trace.h"
#include "qapi/error.h"
+#include "hw/virtio/virtio-access.h"
/*
* Return one past the end of the end of section. Be careful with uint64_t
@@ -44,7 +45,6 @@ static bool
vhost_vdpa_listener_skipped_section(MemoryRegionSection *section,
uint64_t iova_min,
uint64_t iova_max)
{
-Int128 llend;
if ((!memory_region_is_ram(section->mr) &&
!memory_region_is_iommu(section->mr)) ||
@@ -61,14 +61,6 @@ static bool
vhost_vdpa_listener_skipped_section(MemoryRegionSection *section,
return true;
}
-llend = vhost_vdpa_section_end(section);
-if (int128_gt(llend, int128_make64(iova_max))) {
-error_report("RAM section out of device range (max=0x%" PRIx64
- ", end addr=0x%" PRIx64 ")",
- iova_max, int128_get64(llend));
-return true;
-}
-
return false;
}
@@ -173,6 +165,106 @@ static void vhost_vdpa_listener_commit(MemoryListener
*listener)
v->iotlb_batch_begin_sent = false;
}
+static void vhost_vdpa_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
+{
+struct vdpa_iommu *iommu = container_of(n, struct vdpa_iommu, n);
+
+hwaddr iova = iotlb->iova + iommu->iommu_offset;
+struct vhost_vdpa *v = iommu->dev;
+void *vaddr;
+int ret;
+
+if (iotlb->target_as != _space_memory) {
+error_report("Wrong target AS \"%s\", only system memory is allowed",
+ iotlb->target_as->name ? iotlb->target_as->name : "none");
+return;
+}
+RCU_READ_LOCK_GUARD();
+vhost_vdpa_iotlb_batch_begin_once(v);
+
+if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
+bool read_only;
+
+if (!memory_get_xlat_addr(iotlb, , NULL, _only, NULL)) {
+return;
+}
+ret =
+vhost_vdpa_dma_map(v, iova, iotlb->addr_mask + 1, vaddr,
read_only);
+if (ret) {
+error_report("vhost_vdpa_dma_map(%p, 0x%" HWADDR_PRIx ", "
+ "0x%" HWADDR_PRIx ", %p) = %d (%m)",
+ v, iova, iotlb->addr_mask + 1, vaddr, ret);
+}
+} else {
+ret = vhost_vdpa_dma_unmap(v, iova, iotlb->addr_mask + 1);
+if (ret) {
+error_report("vhost_vdpa_dma_unmap(%p, 0x%" HWADDR_PRIx ", "
+ "0x%" HWADDR_PRIx ") = %d (%m)",
+ v, iova, iotlb->addr_mask + 1, ret);
+}
+}
+}
+
+static void vhost_vdpa_iommu_region_add(MemoryListener *listener,
+MemoryRegionSection *section)
+{
+struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, listener);
+
+struct vdpa_iommu *iommu;
+Int128 end;
+int iommu_idx;
+IOMMUMemoryRegion *iommu_mr;
+int ret;
+
+iommu_mr = IOMMU_MEMORY_REGION(section->mr);
+
+iommu = g_malloc0(sizeof(*iommu));
+end = int128_add(int128_make64(section->offset_within_region),
+section->size);
+end = int128_sub(end, int128_one());
+iommu_idx = memory_region_iommu_attrs_to_index(iommu_mr,
+MEMTXATTRS_UNSPECIFIED);
+
+iommu->iommu_mr = iommu_mr;
+
+iommu_notifier_init(
+>n, vhost_vdpa_iommu_map_notify, IOMMU_NOTIFIER_IOTLB_EVENTS,
+section->offset_within_region, int128_get64(end), iommu_idx);
+iommu->iommu_offset =
+section->offset_within_address_space - section->offset_within_region;
+iommu->dev = v;
+
+ret = memory_region_register_iommu_notifier(section->mr, >n, NULL);
+if (ret) {
+g_free(iommu);
+return;
+}
+
+QLIST_INSERT_HEAD(>iommu_list, iommu, iommu_next);
+memory_region_iommu_replay(iommu->iommu_mr, >n);
+
+return;
+}
+
+static void vhost_vdpa_iommu_region_del(MemoryListener *listener,
+MemoryRegionSection *section)
+{
+struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, listener);
+
+struct vdpa_iommu *iommu;
+
+QLIST_FOREACH(iommu, >iommu_list, iommu_next)
+{
+if (MEMORY_REGION(iommu->iommu_mr) == section->mr &&
+iommu->n.start == section->offset_within_region) {
+memory_region_unregister_iommu_notifier(section->mr,
