* Re: [PATCH V5] vfio: return mr from vfio_get_xlat_addr
2025-05-19 13:26 [PATCH V5] vfio: return mr from vfio_get_xlat_addr Steve Sistare
@ 2025-05-19 13:46 ` John Levon
2025-05-20 6:46 ` Cédric Le Goater
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: John Levon @ 2025-05-19 13:46 UTC (permalink / raw)
To: Steve Sistare
Cc: qemu-devel, Marc-Andre Lureau, Thanos Makatos, Daniel P. Berrange,
Paolo Bonzini, Peter Xu, David Hildenbrand, Cedric Le Goater,
Stefano Garzarella, Michael S. Tsirkin, Alex Williamson,
Philippe Mathieu-Daude
On Mon, May 19, 2025 at 06:26:43AM -0700, Steve Sistare wrote:
> !-------------------------------------------------------------------|
> CAUTION: External Email
>
> |-------------------------------------------------------------------!
>
> 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.sistare@oracle.com>
> Acked-by: David Hildenbrand <david@redhat.com>
Reviewed-by: John Levon <john.levon@nutanix.com>
regards
john
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH V5] vfio: return mr from vfio_get_xlat_addr
2025-05-19 13:26 [PATCH V5] vfio: return mr from vfio_get_xlat_addr Steve Sistare
2025-05-19 13:46 ` John Levon
@ 2025-05-20 6:46 ` Cédric Le Goater
2025-05-20 7:32 ` Cédric Le Goater
2025-05-26 16:57 ` Cédric Le Goater
3 siblings, 0 replies; 9+ messages in thread
From: Cédric Le Goater @ 2025-05-20 6:46 UTC (permalink / raw)
To: Steve Sistare, qemu-devel
Cc: Marc-Andre Lureau, Thanos Makatos, Daniel P. Berrange,
Paolo Bonzini, Peter Xu, David Hildenbrand, Stefano Garzarella,
Michael S. Tsirkin, Alex Williamson, Philippe Mathieu-Daude,
John Levon
On 5/19/25 15:26, Steve Sistare wrote:
> 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.sistare@oracle.com>
> Acked-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
> ---
> hw/vfio/listener.c | 33 ++++++++++++++++++++++-----------
> hw/virtio/vhost-vdpa.c | 9 +++++++--
> include/system/memory.h | 19 +++++++++----------
> system/memory.c | 32 +++++++-------------------------
> 4 files changed, 45 insertions(+), 48 deletions(-)
>
> 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/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/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)
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH V5] vfio: return mr from vfio_get_xlat_addr
2025-05-19 13:26 [PATCH V5] vfio: return mr from vfio_get_xlat_addr Steve Sistare
2025-05-19 13:46 ` John Levon
2025-05-20 6:46 ` Cédric Le Goater
@ 2025-05-20 7:32 ` Cédric Le Goater
2025-05-20 13:42 ` Steven Sistare
2025-05-26 16:57 ` Cédric Le Goater
3 siblings, 1 reply; 9+ messages in thread
From: Cédric Le Goater @ 2025-05-20 7:32 UTC (permalink / raw)
To: Steve Sistare, qemu-devel
Cc: Marc-Andre Lureau, Thanos Makatos, Daniel P. Berrange,
Paolo Bonzini, Peter Xu, David Hildenbrand, Stefano Garzarella,
Michael S. Tsirkin, Alex Williamson, Philippe Mathieu-Daude,
John Levon
On 5/19/25 15:26, Steve Sistare wrote:
> 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.sistare@oracle.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> ---
> hw/vfio/listener.c | 33 ++++++++++++++++++++++-----------
> hw/virtio/vhost-vdpa.c | 9 +++++++--
> include/system/memory.h | 19 +++++++++----------
> system/memory.c | 32 +++++++-------------------------
> 4 files changed, 45 insertions(+), 48 deletions(-)
>
> 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;
xlat should be :
hwaddr xlat;
Thanks,
C.
>
> 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/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/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)
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH V5] vfio: return mr from vfio_get_xlat_addr
2025-05-20 7:32 ` Cédric Le Goater
@ 2025-05-20 13:42 ` Steven Sistare
2025-05-20 13:46 ` Cédric Le Goater
0 siblings, 1 reply; 9+ messages in thread
From: Steven Sistare @ 2025-05-20 13:42 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel
Cc: Marc-Andre Lureau, Thanos Makatos, Daniel P. Berrange,
Paolo Bonzini, Peter Xu, David Hildenbrand, Stefano Garzarella,
Michael S. Tsirkin, Alex Williamson, Philippe Mathieu-Daude,
John Levon
On 5/20/2025 3:32 AM, Cédric Le Goater wrote:
> On 5/19/25 15:26, Steve Sistare wrote:
>> 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.sistare@oracle.com>
>> Acked-by: David Hildenbrand <david@redhat.com>
>> ---
>> hw/vfio/listener.c | 33 ++++++++++++++++++++++-----------
>> hw/virtio/vhost-vdpa.c | 9 +++++++--
>> include/system/memory.h | 19 +++++++++----------
>> system/memory.c | 32 +++++++-------------------------
>> 4 files changed, 45 insertions(+), 48 deletions(-)
>>
>> 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;
>
> xlat should be :
>
> hwaddr xlat;
Will you make that change when you pull, or do you want me to submit a V6
with this and the RB's?
- Steve
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/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/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)
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH V5] vfio: return mr from vfio_get_xlat_addr
2025-05-20 13:42 ` Steven Sistare
@ 2025-05-20 13:46 ` Cédric Le Goater
2025-05-25 21:23 ` Cédric Le Goater
0 siblings, 1 reply; 9+ messages in thread
From: Cédric Le Goater @ 2025-05-20 13:46 UTC (permalink / raw)
To: Steven Sistare, qemu-devel
Cc: Marc-Andre Lureau, Thanos Makatos, Daniel P. Berrange,
Paolo Bonzini, Peter Xu, David Hildenbrand, Stefano Garzarella,
Michael S. Tsirkin, Alex Williamson, Philippe Mathieu-Daude,
John Levon
>>> @@ -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;
>>
>> xlat should be :
>>
>> hwaddr xlat;
>
> Will you make that change when you pull, or do you want me to submit a V6
> with this and the RB's?
I can handle it.
An ack from Michael (vhost) is needed for the PR.
Thanks,
C.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V5] vfio: return mr from vfio_get_xlat_addr
2025-05-20 13:46 ` Cédric Le Goater
@ 2025-05-25 21:23 ` Cédric Le Goater
2025-05-26 14:37 ` Michael S. Tsirkin
0 siblings, 1 reply; 9+ messages in thread
From: Cédric Le Goater @ 2025-05-25 21:23 UTC (permalink / raw)
To: Steven Sistare, qemu-devel
Cc: Marc-Andre Lureau, Thanos Makatos, Daniel P. Berrange,
Paolo Bonzini, Peter Xu, David Hildenbrand, Stefano Garzarella,
Michael S. Tsirkin, Alex Williamson, Philippe Mathieu-Daude,
John Levon
Michael,
On 5/20/25 15:46, Cédric Le Goater wrote:
>
>>>> @@ -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;
>>>
>>> xlat should be :
>>>
>>> hwaddr xlat;
>>
>> Will you make that change when you pull, or do you want me to submit a V6
>> with this and the RB's?
>
> I can handle it.
>
> An ack from Michael (vhost) is needed for the PR.
Are you okay with us merging this patch via the vfio queue ?
This would facilitate the progress of 2 larges series : live update
and vfio-user
Thanks,
C.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V5] vfio: return mr from vfio_get_xlat_addr
2025-05-25 21:23 ` Cédric Le Goater
@ 2025-05-26 14:37 ` Michael S. Tsirkin
0 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2025-05-26 14:37 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Steven Sistare, qemu-devel, Marc-Andre Lureau, Thanos Makatos,
Daniel P. Berrange, Paolo Bonzini, Peter Xu, David Hildenbrand,
Stefano Garzarella, Alex Williamson, Philippe Mathieu-Daude,
John Levon
On Sun, May 25, 2025 at 11:23:57PM +0200, Cédric Le Goater wrote:
> Michael,
>
> On 5/20/25 15:46, Cédric Le Goater wrote:
> >
> > > > > @@ -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;
> > > >
> > > > xlat should be :
> > > >
> > > > hwaddr xlat;
> > >
> > > Will you make that change when you pull, or do you want me to submit a V6
> > > with this and the RB's?
> >
> > I can handle it.
> >
> > An ack from Michael (vhost) is needed for the PR.
>
> Are you okay with us merging this patch via the vfio queue ?
>
> This would facilitate the progress of 2 larges series : live update
> and vfio-user
>
> Thanks,
>
> C.
sure, thanks!
Acked-by: Michael S. Tsirkin <mst@redhat.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V5] vfio: return mr from vfio_get_xlat_addr
2025-05-19 13:26 [PATCH V5] vfio: return mr from vfio_get_xlat_addr Steve Sistare
` (2 preceding siblings ...)
2025-05-20 7:32 ` Cédric Le Goater
@ 2025-05-26 16:57 ` Cédric Le Goater
3 siblings, 0 replies; 9+ messages in thread
From: Cédric Le Goater @ 2025-05-26 16:57 UTC (permalink / raw)
To: Steve Sistare, qemu-devel
Cc: Marc-Andre Lureau, Thanos Makatos, Daniel P. Berrange,
Paolo Bonzini, Peter Xu, David Hildenbrand, Stefano Garzarella,
Michael S. Tsirkin, Alex Williamson, Philippe Mathieu-Daude,
John Levon
On 5/19/25 15:26, Steve Sistare wrote:
> 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.sistare@oracle.com>
> Acked-by: David Hildenbrand <david@redhat.com>
Applied to vfio-next.
Thanks,
C.
> ---
> hw/vfio/listener.c | 33 ++++++++++++++++++++++-----------
> hw/virtio/vhost-vdpa.c | 9 +++++++--
> include/system/memory.h | 19 +++++++++----------
> system/memory.c | 32 +++++++-------------------------
> 4 files changed, 45 insertions(+), 48 deletions(-)
>
> 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/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/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)
^ permalink raw reply [flat|nested] 9+ messages in thread