[PATCH v2 0/3] Optimize unmap_all with one ioctl()

[PATCH v2 0/3] Optimize unmap_all with one ioctl()

[Zhenzhong Duan]

Currently unmap_all is split into two ioctl() with each unmap half of
the whole iova space.

IOMMUFD supports unmap_all ioctl() from beginning, after kernel commit
c19650995374 ("vfio/type1: implement unmap all") added same support
for VFIO type1, the split becomes unnecessary.

So optimize the code to only do one ioctl() to unmap_all for both
backends.

Test:
In order to trigger unmap_all request, I have to fake a unmap_all request
on x86. Maybe it's easy to trigger unmap_all with other arch, e.g., arm smmu,
but for x86, iommu memory region is split by iommu_ir, unmap_all isn't
triggered by default. See
https://github.com/yiliu1765/qemu/commit/7afc7adac8fa601abd978b821c86e90e073d31ba
for details.

Thanks
Zhenzhong

Changelog:
v2:
- check unmap_all_supported instead of ioctl() for every unmap_all (John Levon)
- make assertion a separate patch (Cedric)
- pass VFIOLegacyContainer pointer in vfio_legacy_dma_unmap_one() (Cedric)

Zhenzhong Duan (3):
  vfio/container: Support unmap all in one ioctl()
  vfio/iommufd: Support unmap all in one ioctl()
  vfio/listener: Add an assertion for unmap_all

 include/hw/vfio/vfio-container-legacy.h |  1 +
 hw/vfio/container-legacy.c              | 38 ++++++++++++++-----------
 hw/vfio/iommufd.c                       | 15 +---------
 hw/vfio/listener.c                      |  1 +
 4 files changed, 25 insertions(+), 30 deletions(-)

-- 
2.47.1

[PATCH v2 1/3] vfio/container: Support unmap all in one ioctl()

[Zhenzhong Duan]

VFIO type1 kernel uAPI supports unmapping whole address space in one call
since commit c19650995374 ("vfio/type1: implement unmap all"). Use the
unmap_all variant whenever it's supported in kernel.

Opportunistically pass VFIOLegacyContainer pointer in low level function
vfio_legacy_dma_unmap_one().

Co-developed-by: John Levon <levon@movementarian.org>
Signed-off-by: John Levon <levon@movementarian.org>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/hw/vfio/vfio-container-legacy.h |  1 +
 hw/vfio/container-legacy.c              | 38 ++++++++++++++-----------
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/include/hw/vfio/vfio-container-legacy.h b/include/hw/vfio/vfio-container-legacy.h
index 74a72df018..ffd594e80d 100644
--- a/include/hw/vfio/vfio-container-legacy.h
+++ b/include/hw/vfio/vfio-container-legacy.h
@@ -30,6 +30,7 @@ struct VFIOLegacyContainer {
 
     int fd; /* /dev/vfio/vfio, empowered by the attached groups */
     unsigned iommu_type;
+    bool unmap_all_supported;
     QLIST_HEAD(, VFIOGroup) group_list;
     VFIOContainerCPR cpr;
 };
diff --git a/hw/vfio/container-legacy.c b/hw/vfio/container-legacy.c
index a3615d7b5d..8e9639603e 100644
--- a/hw/vfio/container-legacy.c
+++ b/hw/vfio/container-legacy.c
@@ -121,14 +121,14 @@ unmap_exit:
     return ret;
 }
 
-static int vfio_legacy_dma_unmap_one(const VFIOContainer *bcontainer,
+static int vfio_legacy_dma_unmap_one(const VFIOLegacyContainer *container,
                                      hwaddr iova, uint64_t size,
-                                     IOMMUTLBEntry *iotlb)
+                                     uint32_t flags, IOMMUTLBEntry *iotlb)
 {
-    const VFIOLegacyContainer *container = VFIO_IOMMU_LEGACY(bcontainer);
+    const VFIOContainer *bcontainer = VFIO_IOMMU(container);
     struct vfio_iommu_type1_dma_unmap unmap = {
         .argsz = sizeof(unmap),
-        .flags = 0,
+        .flags = flags,
         .iova = iova,
         .size = size,
     };
@@ -170,25 +170,28 @@ static int vfio_legacy_dma_unmap(const VFIOContainer *bcontainer,
                                  hwaddr iova, uint64_t size,
                                  IOMMUTLBEntry *iotlb, bool unmap_all)
 {
+    const VFIOLegacyContainer *container = VFIO_IOMMU_LEGACY(bcontainer);
+    uint32_t flags = 0;
     int ret;
 
     if (unmap_all) {
-        /* The unmap ioctl doesn't accept a full 64-bit span. */
-        Int128 llsize = int128_rshift(int128_2_64(), 1);
-
-        ret = vfio_legacy_dma_unmap_one(bcontainer, 0, int128_get64(llsize),
-                                        iotlb);
+        if (container->unmap_all_supported) {
+            flags = VFIO_DMA_UNMAP_FLAG_ALL;
+        } else {
+            /* The unmap ioctl doesn't accept a full 64-bit span. */
+            Int128 llsize = int128_rshift(int128_2_64(), 1);
+            size = int128_get64(llsize);
+
+            ret = vfio_legacy_dma_unmap_one(container, 0, size, flags, iotlb);
+            if (ret) {
+                return ret;
+            }
 
-        if (ret == 0) {
-            ret = vfio_legacy_dma_unmap_one(bcontainer, int128_get64(llsize),
-                                            int128_get64(llsize), iotlb);
+            iova = size;
         }
-
-    } else {
-        ret = vfio_legacy_dma_unmap_one(bcontainer, iova, size, iotlb);
     }
 
-    return ret;
+    return vfio_legacy_dma_unmap_one(container, iova, size, flags, iotlb);
 }
 
 static int vfio_legacy_dma_map(const VFIOContainer *bcontainer, hwaddr iova,
@@ -519,6 +522,9 @@ static bool vfio_legacy_setup(VFIOContainer *bcontainer, Error **errp)
 
     vfio_get_info_iova_range(info, bcontainer);
 
+    ret = ioctl(container->fd, VFIO_CHECK_EXTENSION, VFIO_UNMAP_ALL);
+    container->unmap_all_supported = !!ret;
+
     vfio_get_iommu_info_migration(container, info);
     return true;
 }
-- 
2.47.1

[PATCH v2 2/3] vfio/iommufd: Support unmap all in one ioctl()

[Zhenzhong Duan]

IOMMUFD kernel uAPI supports unmapping whole address space in one call with
[iova, size] set to [0, UINT64_MAX], this can simplify iommufd_cdev_unmap()
a bit. See iommufd_ioas_unmap() in kernel for details.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/iommufd.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 68470d552e..336f8fdb04 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -62,21 +62,8 @@ static int iommufd_cdev_unmap(const VFIOContainer *bcontainer,
 {
     const VFIOIOMMUFDContainer *container = VFIO_IOMMU_IOMMUFD(bcontainer);
 
-    /* unmap in halves */
     if (unmap_all) {
-        Int128 llsize = int128_rshift(int128_2_64(), 1);
-        int ret;
-
-        ret = iommufd_backend_unmap_dma(container->be, container->ioas_id,
-                                        0, int128_get64(llsize));
-
-        if (ret == 0) {
-            ret = iommufd_backend_unmap_dma(container->be, container->ioas_id,
-                                            int128_get64(llsize),
-                                            int128_get64(llsize));
-        }
-
-        return ret;
+        size = UINT64_MAX;
     }
 
     /* TODO: Handle dma_unmap_bitmap with iotlb args (migration) */
-- 
2.47.1

[PATCH v2 3/3] vfio/listener: Add an assertion for unmap_all

[Zhenzhong Duan]

Currently the maximum of iommu address space is 64bit. So when a maximum
iommu memory section is deleted, it's in scope [0, 2^64). Add a
assertion for that.

Suggested-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/listener.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/vfio/listener.c b/hw/vfio/listener.c
index c6bb58f520..62766d8c46 100644
--- a/hw/vfio/listener.c
+++ b/hw/vfio/listener.c
@@ -715,6 +715,7 @@ static void vfio_listener_region_del(MemoryListener *listener,
         bool unmap_all = false;
 
         if (int128_eq(llsize, int128_2_64())) {
+            assert(!iova);
             unmap_all = true;
             llsize = int128_zero();
         }
-- 
2.47.1

Re: [PATCH v2 0/3] Optimize unmap_all with one ioctl()

[Cédric Le Goater]

On 10/9/25 06:01, Zhenzhong Duan wrote:
> Currently unmap_all is split into two ioctl() with each unmap half of
> the whole iova space.
> 
> IOMMUFD supports unmap_all ioctl() from beginning, after kernel commit
> c19650995374 ("vfio/type1: implement unmap all") added same support
> for VFIO type1, the split becomes unnecessary.
> 
> So optimize the code to only do one ioctl() to unmap_all for both
> backends.
> 
> Test:
> In order to trigger unmap_all request, I have to fake a unmap_all request
> on x86. Maybe it's easy to trigger unmap_all with other arch, e.g., arm smmu,
> but for x86, iommu memory region is split by iommu_ir, unmap_all isn't
> triggered by default. See
> https://github.com/yiliu1765/qemu/commit/7afc7adac8fa601abd978b821c86e90e073d31ba
> for details.
> 
> Thanks
> Zhenzhong
> 
> Changelog:
> v2:
> - check unmap_all_supported instead of ioctl() for every unmap_all (John Levon)
> - make assertion a separate patch (Cedric)
> - pass VFIOLegacyContainer pointer in vfio_legacy_dma_unmap_one() (Cedric)
> 
> Zhenzhong Duan (3):
>    vfio/container: Support unmap all in one ioctl()
>    vfio/iommufd: Support unmap all in one ioctl()
>    vfio/listener: Add an assertion for unmap_all
> 
>   include/hw/vfio/vfio-container-legacy.h |  1 +
>   hw/vfio/container-legacy.c              | 38 ++++++++++++++-----------
>   hw/vfio/iommufd.c                       | 15 +---------
>   hw/vfio/listener.c                      |  1 +
>   4 files changed, 25 insertions(+), 30 deletions(-)
> 


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.

Re: [PATCH v2 0/3] Optimize unmap_all with one ioctl()

[Cédric Le Goater]

On 10/9/25 06:01, Zhenzhong Duan wrote:
> Currently unmap_all is split into two ioctl() with each unmap half of
> the whole iova space.
> 
> IOMMUFD supports unmap_all ioctl() from beginning, after kernel commit
> c19650995374 ("vfio/type1: implement unmap all") added same support
> for VFIO type1, the split becomes unnecessary.
> 
> So optimize the code to only do one ioctl() to unmap_all for both
> backends.
> 
> Test:
> In order to trigger unmap_all request, I have to fake a unmap_all request
> on x86. Maybe it's easy to trigger unmap_all with other arch, e.g., arm smmu,
> but for x86, iommu memory region is split by iommu_ir, unmap_all isn't
> triggered by default. See
> https://github.com/yiliu1765/qemu/commit/7afc7adac8fa601abd978b821c86e90e073d31ba
> for details.
> 
> Thanks
> Zhenzhong
> 
> Changelog:
> v2:
> - check unmap_all_supported instead of ioctl() for every unmap_all (John Levon)
> - make assertion a separate patch (Cedric)
> - pass VFIOLegacyContainer pointer in vfio_legacy_dma_unmap_one() (Cedric)
> 
> Zhenzhong Duan (3):
>    vfio/container: Support unmap all in one ioctl()
>    vfio/iommufd: Support unmap all in one ioctl()
>    vfio/listener: Add an assertion for unmap_all
> 
>   include/hw/vfio/vfio-container-legacy.h |  1 +
>   hw/vfio/container-legacy.c              | 38 ++++++++++++++-----------
>   hw/vfio/iommufd.c                       | 15 +---------
>   hw/vfio/listener.c                      |  1 +
>   4 files changed, 25 insertions(+), 30 deletions(-)
> 


Applied to vfio-next.

Thanks,

C.