[PATCH] vfio/listener: Fix translated_addr for non-identity-mapped RAM sections

Cédric Le Goater posted 1 patch 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260625134352.3122572-1-clg@redhat.com
Maintainers: Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>
hw/vfio/listener.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[PATCH] vfio/listener: Fix translated_addr for non-identity-mapped RAM sections
Posted by Cédric Le Goater 1 month ago
In vfio_listener_region_del(), when dirty tracking is active and a
writable RAM section is deleted, a synthetic IOMMUTLBEntry is built to
flush dirty pages. Setting translated_addr to the IOVA (GPA) is only
correct for identity-mapped regions where GPA == ram_addr_t.

For RAM sections with GPA far above main RAM (e.g., nested VT-d interrupt
remapping table at 58 TB), translated_addr is too large, causing a crash
in physical_memory_set_dirty_lebitmap() when indexing beyond the allocated
dirty memory blocks array :

  bitmap_set_atomic(map=NULL, start=1, nr=1)
  physical_memory_set_dirty_range(start=0x380004040000, length=4096)
  physical_memory_set_dirty_lebitmap(start=0x380004040000, pages=3)
  vfio_container_query_dirty_bitmap(translated_addr=0x380004040000)
  vfio_legacy_dma_unmap_one(iova=0x380004040000, size=12288)
  vfio_listener_region_del()

Fix this by setting translated_addr to the ram_addr_t of the section, which
is consistent with other vfio dirty tracking code:

  translated_addr = memory_region_get_ram_addr(section->mr) +
                    section->offset_within_region;

Cc: Zhenzhong Duan <zhenzhong.duan@intel.com>
Fixes: 6e360c06176c ("vfio/listener: Add missing dirty tracking in region_del")
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/vfio/listener.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/listener.c b/hw/vfio/listener.c
index 14cca678aea788c23ea5c6ed4a3aceec5fe21484..785a6eef8d812123fcecf82e3716f5577d7c61e1 100644
--- a/hw/vfio/listener.c
+++ b/hw/vfio/listener.c
@@ -731,7 +731,7 @@ static void vfio_listener_region_del(MemoryListener *listener,
         }
 
         /*
-         * Fake an IOTLB entry for writable identity mapping which is needed
+         * Fake an IOTLB entry for writable RAM sections which is needed
          * by dirty tracking when switch out of PT domain. In fact, in
          * unmap_bitmap, only translated_addr field is used to set dirty
          * bitmap.
@@ -746,7 +746,8 @@ static void vfio_listener_region_del(MemoryListener *listener,
         if (global_dirty_tracking && memory_region_is_ram(section->mr) &&
             !section->readonly) {
             entry.iova = iova;
-            entry.translated_addr = iova;
+            entry.translated_addr = memory_region_get_ram_addr(section->mr) +
+                                    section->offset_within_region;
             iotlb = &entry;
         }
 
-- 
2.54.0


Re: [PATCH] vfio/listener: Fix translated_addr for non-identity-mapped RAM sections
Posted by Cédric Le Goater 4 weeks, 1 day ago
On 6/25/26 15:43, Cédric Le Goater wrote:
> In vfio_listener_region_del(), when dirty tracking is active and a
> writable RAM section is deleted, a synthetic IOMMUTLBEntry is built to
> flush dirty pages. Setting translated_addr to the IOVA (GPA) is only
> correct for identity-mapped regions where GPA == ram_addr_t.
> 
> For RAM sections with GPA far above main RAM (e.g., nested VT-d interrupt
> remapping table at 58 TB), translated_addr is too large, causing a crash
> in physical_memory_set_dirty_lebitmap() when indexing beyond the allocated
> dirty memory blocks array :
> 
>    bitmap_set_atomic(map=NULL, start=1, nr=1)
>    physical_memory_set_dirty_range(start=0x380004040000, length=4096)
>    physical_memory_set_dirty_lebitmap(start=0x380004040000, pages=3)
>    vfio_container_query_dirty_bitmap(translated_addr=0x380004040000)
>    vfio_legacy_dma_unmap_one(iova=0x380004040000, size=12288)
>    vfio_listener_region_del()
> 
> Fix this by setting translated_addr to the ram_addr_t of the section, which
> is consistent with other vfio dirty tracking code:
> 
>    translated_addr = memory_region_get_ram_addr(section->mr) +
>                      section->offset_within_region;
> 
> Cc: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Fixes: 6e360c06176c ("vfio/listener: Add missing dirty tracking in region_del")
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>   hw/vfio/listener.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/vfio/listener.c b/hw/vfio/listener.c
> index 14cca678aea788c23ea5c6ed4a3aceec5fe21484..785a6eef8d812123fcecf82e3716f5577d7c61e1 100644
> --- a/hw/vfio/listener.c
> +++ b/hw/vfio/listener.c
> @@ -731,7 +731,7 @@ static void vfio_listener_region_del(MemoryListener *listener,
>           }
>   
>           /*
> -         * Fake an IOTLB entry for writable identity mapping which is needed
> +         * Fake an IOTLB entry for writable RAM sections which is needed
>            * by dirty tracking when switch out of PT domain. In fact, in
>            * unmap_bitmap, only translated_addr field is used to set dirty
>            * bitmap.
> @@ -746,7 +746,8 @@ static void vfio_listener_region_del(MemoryListener *listener,
>           if (global_dirty_tracking && memory_region_is_ram(section->mr) &&
>               !section->readonly) {
>               entry.iova = iova;
> -            entry.translated_addr = iova;
> +            entry.translated_addr = memory_region_get_ram_addr(section->mr) +
> +                                    section->offset_within_region;
>               iotlb = &entry;
>           }
>   


Applied to

     https://github.com/legoater/qemu vfio-next

Thanks,

C.


RE: [PATCH] vfio/listener: Fix translated_addr for non-identity-mapped RAM sections
Posted by Duan, Zhenzhong 1 month ago

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: [PATCH] vfio/listener: Fix translated_addr for non-identity-mapped RAM
>sections
>
>In vfio_listener_region_del(), when dirty tracking is active and a
>writable RAM section is deleted, a synthetic IOMMUTLBEntry is built to
>flush dirty pages. Setting translated_addr to the IOVA (GPA) is only
>correct for identity-mapped regions where GPA == ram_addr_t.
>
>For RAM sections with GPA far above main RAM (e.g., nested VT-d interrupt
>remapping table at 58 TB), translated_addr is too large, causing a crash
>in physical_memory_set_dirty_lebitmap() when indexing beyond the allocated
>dirty memory blocks array :
>
>  bitmap_set_atomic(map=NULL, start=1, nr=1)
>  physical_memory_set_dirty_range(start=0x380004040000, length=4096)
>  physical_memory_set_dirty_lebitmap(start=0x380004040000, pages=3)
>  vfio_container_query_dirty_bitmap(translated_addr=0x380004040000)
>  vfio_legacy_dma_unmap_one(iova=0x380004040000, size=12288)
>  vfio_listener_region_del()
>
>Fix this by setting translated_addr to the ram_addr_t of the section, which
>is consistent with other vfio dirty tracking code:
>
>  translated_addr = memory_region_get_ram_addr(section->mr) +
>                    section->offset_within_region;
>
>Cc: Zhenzhong Duan <zhenzhong.duan@intel.com>
>Fixes: 6e360c06176c ("vfio/listener: Add missing dirty tracking in region_del")
>Signed-off-by: Cédric Le Goater <clg@redhat.com>

Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Tested-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Verified with below reproduce steps:

1. Start guest with VFIO device in legacy iommu mode, then start dirty rate tracking.

(qemu) calc_dirty_rate -b 60

2. disable VFIO device bar region by clear bit1 in COMMAND register in guest.
# setpci -s 01:00.0 COMMAND=0005

Thanks
Zhenzhong
Re: [PATCH] vfio/listener: Fix translated_addr for non-identity-mapped RAM sections
Posted by Philippe Mathieu-Daudé 1 month ago
On 25/6/26 15:43, Cédric Le Goater wrote:
> In vfio_listener_region_del(), when dirty tracking is active and a
> writable RAM section is deleted, a synthetic IOMMUTLBEntry is built to
> flush dirty pages. Setting translated_addr to the IOVA (GPA) is only
> correct for identity-mapped regions where GPA == ram_addr_t.
> 
> For RAM sections with GPA far above main RAM (e.g., nested VT-d interrupt
> remapping table at 58 TB), translated_addr is too large, causing a crash
> in physical_memory_set_dirty_lebitmap() when indexing beyond the allocated
> dirty memory blocks array :
> 
>    bitmap_set_atomic(map=NULL, start=1, nr=1)
>    physical_memory_set_dirty_range(start=0x380004040000, length=4096)
>    physical_memory_set_dirty_lebitmap(start=0x380004040000, pages=3)
>    vfio_container_query_dirty_bitmap(translated_addr=0x380004040000)
>    vfio_legacy_dma_unmap_one(iova=0x380004040000, size=12288)
>    vfio_listener_region_del()
> 
> Fix this by setting translated_addr to the ram_addr_t of the section, which
> is consistent with other vfio dirty tracking code:
> 
>    translated_addr = memory_region_get_ram_addr(section->mr) +
>                      section->offset_within_region;
> 
> Cc: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Fixes: 6e360c06176c ("vfio/listener: Add missing dirty tracking in region_del")
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>   hw/vfio/listener.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>