>-----Original Message-----
>From: Steve Sistare <steven.sist...@oracle.com>
>Subject: [PATCH V4 02/43] vfio: return mr from vfio_get_xlat_addr
>
>Modify memory_get_xlat_addr and vfio_get_xlat_addr to return the memory
>region that the translated address is found in. This will be needed by
>CPR in a subsequent patch to map blocks using IOMMU_IOAS_MAP_FILE.
>
>Also return the xlat offset, so we can simplify the interface by removing
>the out parameters that can be trivially derived from mr and xlat.
>
>Lastly, rename the functions to to memory_translate_iotlb() and
>vfio_translate_iotlb().
>
>Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
>Acked-by: David Hildenbrand <da...@redhat.com>
>Reviewed-by: John Levon <john.le...@nutanix.com>
>Reviewed-by: Cédric Le Goater <c...@redhat.com>
>Acked-by: Michael S. Tsirkin <m...@redhat.com>
Reviewed-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>---
> include/system/memory.h | 19 +++++++++----------
> hw/vfio/listener.c | 33 ++++++++++++++++++++++-----------
> hw/virtio/vhost-vdpa.c | 9 +++++++--
> system/memory.c | 32 +++++++-------------------------
> 4 files changed, 45 insertions(+), 48 deletions(-)
>
>diff --git a/include/system/memory.h b/include/system/memory.h
>index fbbf4cf..13416d7 100644
>--- a/include/system/memory.h
>+++ b/include/system/memory.h
>@@ -738,21 +738,20 @@ void
>ram_discard_manager_unregister_listener(RamDiscardManager *rdm,
> RamDiscardListener *rdl);
>
> /**
>- * memory_get_xlat_addr: Extract addresses from a TLB entry
>+ * memory_translate_iotlb: Extract addresses from a TLB entry.
>+ * Called with rcu_read_lock held.
> *
> * @iotlb: pointer to an #IOMMUTLBEntry
>- * @vaddr: virtual address
>- * @ram_addr: RAM address
>- * @read_only: indicates if writes are allowed
>- * @mr_has_discard_manager: indicates memory is controlled by a
>- * RamDiscardManager
>+ * @xlat_p: return the offset of the entry from the start of the returned
>+ * MemoryRegion.
> * @errp: pointer to Error*, to store an error if it happens.
> *
>- * Return: true on success, else false setting @errp with error.
>+ * Return: On success, return the MemoryRegion containing the @iotlb
>translated
>+ * addr. The MemoryRegion must not be accessed after rcu_read_unlock.
>+ * On failure, return NULL, setting @errp with error.
> */
>-bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
>- ram_addr_t *ram_addr, bool *read_only,
>- bool *mr_has_discard_manager, Error **errp);
>+MemoryRegion *memory_translate_iotlb(IOMMUTLBEntry *iotlb, hwaddr
>*xlat_p,
>+ Error **errp);
>
> typedef struct CoalescedMemoryRange CoalescedMemoryRange;
> typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd;
>diff --git a/hw/vfio/listener.c b/hw/vfio/listener.c
>index bfacb3d..0afafe3 100644
>--- a/hw/vfio/listener.c
>+++ b/hw/vfio/listener.c
>@@ -90,16 +90,17 @@ static bool
>vfio_listener_skipped_section(MemoryRegionSection *section)
> section->offset_within_address_space & (1ULL << 63);
> }
>
>-/* Called with rcu_read_lock held. */
>-static bool vfio_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
>- ram_addr_t *ram_addr, bool *read_only,
>- Error **errp)
>+/*
>+ * Called with rcu_read_lock held.
>+ * The returned MemoryRegion must not be accessed after calling
>rcu_read_unlock.
>+ */
>+static MemoryRegion *vfio_translate_iotlb(IOMMUTLBEntry *iotlb, hwaddr
>*xlat_p,
>+ Error **errp)
> {
>- bool ret, mr_has_discard_manager;
>+ MemoryRegion *mr;
>
>- ret = memory_get_xlat_addr(iotlb, vaddr, ram_addr, read_only,
>- &mr_has_discard_manager, errp);
>- if (ret && mr_has_discard_manager) {
>+ mr = memory_translate_iotlb(iotlb, xlat_p, errp);
>+ if (mr && memory_region_has_ram_discard_manager(mr)) {
> /*
> * Malicious VMs might trigger discarding of IOMMU-mapped memory. The
> * pages will remain pinned inside vfio until unmapped, resulting in a
>@@ -118,7 +119,7 @@ static bool vfio_get_xlat_addr(IOMMUTLBEntry *iotlb,
>void **vaddr,
> " intended via an IOMMU. It's possible to mitigate "
> " by setting/adjusting RLIMIT_MEMLOCK.");
> }
>- return ret;
>+ return mr;
> }
>
> static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
>@@ -126,6 +127,8 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n,
>IOMMUTLBEntry *iotlb)
> VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
> VFIOContainerBase *bcontainer = giommu->bcontainer;
> hwaddr iova = iotlb->iova + giommu->iommu_offset;
>+ MemoryRegion *mr;
>+ hwaddr xlat;
> void *vaddr;
> int ret;
> Error *local_err = NULL;
>@@ -150,10 +153,14 @@ static void vfio_iommu_map_notify(IOMMUNotifier
>*n, IOMMUTLBEntry *iotlb)
> if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
> bool read_only;
>
>- if (!vfio_get_xlat_addr(iotlb, &vaddr, NULL, &read_only, &local_err))
>{
>+ mr = vfio_translate_iotlb(iotlb, &xlat, &local_err);
>+ if (!mr) {
> error_report_err(local_err);
> goto out;
> }
>+ vaddr = memory_region_get_ram_ptr(mr) + xlat;
>+ read_only = !(iotlb->perm & IOMMU_WO) || mr->readonly;
>+
> /*
> * vaddr is only valid until rcu_read_unlock(). But after
> * vfio_dma_map has set up the mapping the pages will be
>@@ -1010,6 +1017,8 @@ static void
>vfio_iommu_map_dirty_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
> ram_addr_t translated_addr;
> Error *local_err = NULL;
> int ret = -EINVAL;
>+ MemoryRegion *mr;
>+ ram_addr_t xlat;
>
> trace_vfio_iommu_map_dirty_notify(iova, iova + iotlb->addr_mask);
>
>@@ -1021,9 +1030,11 @@ static void
>vfio_iommu_map_dirty_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
> }
>
> rcu_read_lock();
>- if (!vfio_get_xlat_addr(iotlb, NULL, &translated_addr, NULL, &local_err))
>{
>+ mr = vfio_translate_iotlb(iotlb, &xlat, &local_err);
>+ if (!mr) {
> goto out_unlock;
> }
>+ translated_addr = memory_region_get_ram_addr(mr) + xlat;
>
> ret = vfio_container_query_dirty_bitmap(bcontainer, iova,
> iotlb->addr_mask +
>1,
> translated_addr, &local_err);
>diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
>index 1ab2c11..a1dd9e1 100644
>--- a/hw/virtio/vhost-vdpa.c
>+++ b/hw/virtio/vhost-vdpa.c
>@@ -209,6 +209,8 @@ static void
>vhost_vdpa_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
> int ret;
> Int128 llend;
> Error *local_err = NULL;
>+ MemoryRegion *mr;
>+ hwaddr xlat;
>
> if (iotlb->target_as != &address_space_memory) {
> error_report("Wrong target AS \"%s\", only system memory is allowed",
>@@ -228,11 +230,14 @@ static void
>vhost_vdpa_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
> if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
> bool read_only;
>
>- if (!memory_get_xlat_addr(iotlb, &vaddr, NULL, &read_only, NULL,
>- &local_err)) {
>+ mr = memory_translate_iotlb(iotlb, &xlat, &local_err);
>+ if (!mr) {
> error_report_err(local_err);
> return;
> }
>+ vaddr = memory_region_get_ram_ptr(mr) + xlat;
>+ read_only = !(iotlb->perm & IOMMU_WO) || mr->readonly;
>+
> ret = vhost_vdpa_dma_map(s, VHOST_VDPA_GUEST_PA_ASID, iova,
> iotlb->addr_mask + 1, vaddr, read_only);
> if (ret) {
>diff --git a/system/memory.c b/system/memory.c
>index 63b983e..306e9ff 100644
>--- a/system/memory.c
>+++ b/system/memory.c
>@@ -2174,18 +2174,14 @@ void
>ram_discard_manager_unregister_listener(RamDiscardManager *rdm,
> }
>
> /* Called with rcu_read_lock held. */
>-bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
>- ram_addr_t *ram_addr, bool *read_only,
>- bool *mr_has_discard_manager, Error **errp)
>+MemoryRegion *memory_translate_iotlb(IOMMUTLBEntry *iotlb, hwaddr
>*xlat_p,
>+ Error **errp)
> {
> MemoryRegion *mr;
> hwaddr xlat;
> hwaddr len = iotlb->addr_mask + 1;
> bool writable = iotlb->perm & IOMMU_WO;
>
>- if (mr_has_discard_manager) {
>- *mr_has_discard_manager = false;
>- }
> /*
> * The IOMMU TLB entry we have just covers translation through
> * this IOMMU to its immediate target. We need to translate
>@@ -2195,7 +2191,7 @@ bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb,
>void **vaddr,
> &xlat, &len, writable,
> MEMTXATTRS_UNSPECIFIED);
> if (!memory_region_is_ram(mr)) {
> error_setg(errp, "iommu map to non memory area %" HWADDR_PRIx "",
>xlat);
>- return false;
>+ return NULL;
> } else if (memory_region_has_ram_discard_manager(mr)) {
> RamDiscardManager *rdm =
>memory_region_get_ram_discard_manager(mr);
> MemoryRegionSection tmp = {
>@@ -2203,9 +2199,6 @@ bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb,
>void **vaddr,
> .offset_within_region = xlat,
> .size = int128_make64(len),
> };
>- if (mr_has_discard_manager) {
>- *mr_has_discard_manager = true;
>- }
> /*
> * Malicious VMs can map memory into the IOMMU, which is expected
> * to remain discarded. vfio will pin all pages, populating memory.
>@@ -2216,7 +2209,7 @@ bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb,
>void **vaddr,
> error_setg(errp, "iommu map to discarded memory (e.g., unplugged"
> " via virtio-mem): %" HWADDR_PRIx "",
> iotlb->translated_addr);
>- return false;
>+ return NULL;
> }
> }
>
>@@ -2226,22 +2219,11 @@ bool memory_get_xlat_addr(IOMMUTLBEntry
>*iotlb, void **vaddr,
> */
> if (len & iotlb->addr_mask) {
> error_setg(errp, "iommu has granularity incompatible with target AS");
>- return false;
>+ return NULL;
> }
>
>- if (vaddr) {
>- *vaddr = memory_region_get_ram_ptr(mr) + xlat;
>- }
>-
>- if (ram_addr) {
>- *ram_addr = memory_region_get_ram_addr(mr) + xlat;
>- }
>-
>- if (read_only) {
>- *read_only = !writable || mr->readonly;
>- }
>-
>- return true;
>+ *xlat_p = xlat;
>+ return mr;
> }
>
> void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client)
>--
>1.8.3.1
