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

[PATCH 0/2] 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, made below trick, during emergency
reset in guest, memory region [0xfef00000 - 0xffffffffffffffff] is
deleted, I fake it to be a unmap_all request.

--- a/hw/vfio/listener.c
+++ b/hw/vfio/listener.c
@@ -714,8 +714,10 @@ static void vfio_listener_region_del(MemoryListener *listener,
     if (try_unmap) {
         bool unmap_all = false;

-        if (int128_eq(llsize, int128_2_64())) {
+        if (int128_eq(llsize, int128_2_64()) ||
+            iova == 0xfef00000) {
             unmap_all = true;
+            iova = 0;
             llsize = int128_zero();
         }

The log shows ioctl() succeed on the whole iova space:

vfio_listener_region_del region_del 0xfef00000 - 0xffffffffffffffff
iommufd_backend_unmap_dma  iommufd=10 ioas=5 iova=0x0 size=0xffffffffffffffff (0)

Same result for legacy VFIO.

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.

Thanks
Zhenzhong

Zhenzhong Duan (2):
  vfio/container: Support unmap all in one ioctl()
  vfio/iommufd: Support unmap all in one ioctl()

 hw/vfio/container.c | 33 ++++++++++++++++++++-------------
 hw/vfio/iommufd.c   | 16 ++--------------
 2 files changed, 22 insertions(+), 27 deletions(-)

-- 
2.47.1

[PATCH 1/2] 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.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/container.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 030c6d3f89..2e13f04803 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -122,12 +122,12 @@ unmap_exit:
 
 static int vfio_legacy_dma_unmap_one(const VFIOContainerBase *bcontainer,
                                      hwaddr iova, ram_addr_t size,
-                                     IOMMUTLBEntry *iotlb)
+                                     uint32_t flags, IOMMUTLBEntry *iotlb)
 {
     const VFIOContainer *container = VFIO_IOMMU_LEGACY(bcontainer);
     struct vfio_iommu_type1_dma_unmap unmap = {
         .argsz = sizeof(unmap),
-        .flags = 0,
+        .flags = flags,
         .iova = iova,
         .size = size,
     };
@@ -187,25 +187,32 @@ static int vfio_legacy_dma_unmap(const VFIOContainerBase *bcontainer,
                                  hwaddr iova, ram_addr_t size,
                                  IOMMUTLBEntry *iotlb, bool unmap_all)
 {
+    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);
+        const VFIOContainer *container = VFIO_IOMMU_LEGACY(bcontainer);
 
-        ret = vfio_legacy_dma_unmap_one(bcontainer, 0, int128_get64(llsize),
-                                        iotlb);
+        assert(!iova && !size);
 
-        if (ret == 0) {
-            ret = vfio_legacy_dma_unmap_one(bcontainer, int128_get64(llsize),
-                                            int128_get64(llsize), iotlb);
-        }
+        ret = ioctl(container->fd, VFIO_CHECK_EXTENSION, VFIO_UNMAP_ALL);
+        if (ret) {
+            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(bcontainer, 0, size, flags, iotlb);
+            if (ret) {
+                return ret;
+            }
 
-    } else {
-        ret = vfio_legacy_dma_unmap_one(bcontainer, iova, size, iotlb);
+            iova = size;
+        }
     }
 
-    return ret;
+    return vfio_legacy_dma_unmap_one(bcontainer, iova, size, flags, iotlb);
 }
 
 static int vfio_legacy_dma_map(const VFIOContainerBase *bcontainer, hwaddr iova,
-- 
2.47.1

[PATCH 2/2] 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 | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 8c27222f75..02e4b8774a 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -65,21 +65,9 @@ static int iommufd_cdev_unmap(const VFIOContainerBase *bcontainer,
     const VFIOIOMMUFDContainer *container =
         container_of(bcontainer, VFIOIOMMUFDContainer, 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;
+        assert(!iova && !size);
+        size = UINT64_MAX;
     }
 
     /* TODO: Handle dma_unmap_bitmap with iotlb args (migration) */
-- 
2.47.1

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

[Cédric Le Goater]

On 9/24/25 09:02, Zhenzhong Duan wrote:
> 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.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   hw/vfio/container.c | 33 ++++++++++++++++++++-------------
>   1 file changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 030c6d3f89..2e13f04803 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -122,12 +122,12 @@ unmap_exit:
>   
>   static int vfio_legacy_dma_unmap_one(const VFIOContainerBase *bcontainer,

Side note for rebase :

I would prefer internal routines of file hw/vfio/container.c,
which was renamed recently to hw/vfio/container-legacy.c, to
take a 'VFIOLegacyContainer *container' parameter.

The 'VFIOContainer *bcontainer' parameter should be kept for
high-level routines that wrap the IOMMU backend implementation.

>                                        hwaddr iova, ram_addr_t size,
> -                                     IOMMUTLBEntry *iotlb)
> +                                     uint32_t flags, IOMMUTLBEntry *iotlb)
>   {
>       const VFIOContainer *container = VFIO_IOMMU_LEGACY(bcontainer);
>       struct vfio_iommu_type1_dma_unmap unmap = {
>           .argsz = sizeof(unmap),
> -        .flags = 0,
> +        .flags = flags,
>           .iova = iova,>           .size = size,
>       };
> @@ -187,25 +187,32 @@ static int vfio_legacy_dma_unmap(const VFIOContainerBase *bcontainer,
>                                    hwaddr iova, ram_addr_t size,
>                                    IOMMUTLBEntry *iotlb, bool unmap_all)
>   {
> +    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);
> +        const VFIOContainer *container = VFIO_IOMMU_LEGACY(bcontainer);
>   
> -        ret = vfio_legacy_dma_unmap_one(bcontainer, 0, int128_get64(llsize),
> -                                        iotlb);
> +        assert(!iova && !size);

The assert deserves an explanation and so probably a new patch.

    
> -        if (ret == 0) {
> -            ret = vfio_legacy_dma_unmap_one(bcontainer, int128_get64(llsize),
> -                                            int128_get64(llsize), iotlb);
> -        }
> +        ret = ioctl(container->fd, VFIO_CHECK_EXTENSION, VFIO_UNMAP_ALL);
> +        if (ret) {
> +            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(bcontainer, 0, size, flags, iotlb);
> +            if (ret) {
> +                return ret;
> +            }

Could we introduce an helper to test 'unmap_all' support in the host
kernel ? The result would be something like :

   if (unmap_all) {
         if (vfio_legacy_has_unmap_all(VFIO_IOMMU_LEGACY(bcontainer))) {
             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);
             ...
         }
   }


Thanks,

C.




> -    } else {
> -        ret = vfio_legacy_dma_unmap_one(bcontainer, iova, size, iotlb);
> +            iova = size;
> +        }
>       }
>   
> -    return ret;
> +    return vfio_legacy_dma_unmap_one(bcontainer, iova, size, flags, iotlb);
>   }
>   
>   static int vfio_legacy_dma_map(const VFIOContainerBase *bcontainer, hwaddr iova,

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

[John Levon]

On Tue, Sep 30, 2025 at 05:26:59PM +0200, Cédric Le Goater wrote:

> > -        }
> > +        ret = ioctl(container->fd, VFIO_CHECK_EXTENSION, VFIO_UNMAP_ALL);
> 
> Could we introduce an helper to test 'unmap_all' support in the host
> kernel ? The result would be something like :
> 
>   if (unmap_all) {
>         if (vfio_legacy_has_unmap_all(VFIO_IOMMU_LEGACY(bcontainer))) {
>             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);
>             ...
>         }
>   }

For reference/consideration, the previous approach taken in the vfio-user
series:

https://lore.kernel.org/qemu-devel/20250219144858.266455-4-john.levon@nutanix.com/

@@ -533,6 +562,11 @@ static bool vfio_legacy_setup(VFIOContainerBase *bcontainer, Error **errp)
     vfio_get_info_iova_range(info, bcontainer);
 
     vfio_get_iommu_info_migration(container, info);
+
+    ret = ioctl(container->fd, VFIO_CHECK_EXTENSION, VFIO_UNMAP_ALL);
+
+    container->unmap_all_supported = (ret != 0);

(I dropped this particular change as part of getting merged.)

regards
john

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

[Cédric Le Goater]

On 10/1/25 00:04, John Levon wrote:
> On Tue, Sep 30, 2025 at 05:26:59PM +0200, Cédric Le Goater wrote:
> 
>>> -        }
>>> +        ret = ioctl(container->fd, VFIO_CHECK_EXTENSION, VFIO_UNMAP_ALL);
>>
>> Could we introduce an helper to test 'unmap_all' support in the host
>> kernel ? The result would be something like :
>>
>>    if (unmap_all) {
>>          if (vfio_legacy_has_unmap_all(VFIO_IOMMU_LEGACY(bcontainer))) {
>>              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);
>>              ...
>>          }
>>    }
> 
> For reference/consideration, the previous approach taken in the vfio-user
> series:
> 
> https://lore.kernel.org/qemu-devel/20250219144858.266455-4-john.levon@nutanix.com/
> 
> @@ -533,6 +562,11 @@ static bool vfio_legacy_setup(VFIOContainerBase *bcontainer, Error **errp)
>       vfio_get_info_iova_range(info, bcontainer);
>   
>       vfio_get_iommu_info_migration(container, info);
> +
> +    ret = ioctl(container->fd, VFIO_CHECK_EXTENSION, VFIO_UNMAP_ALL);
> +
> +    container->unmap_all_supported = (ret != 0);
> 
> (I dropped this particular change as part of getting merged.)
Yes. I’m reconsidering now.

Should we introduce a VFIOContainerBase attribute/flag 'unmap_all_supported',
set in the vioc->setup handler ?

Thanks,

C.

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

[Duan, Zhenzhong]

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH 1/2] vfio/container: Support unmap all in one ioctl()
>
>On 9/24/25 09:02, Zhenzhong Duan wrote:
>> 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.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   hw/vfio/container.c | 33 ++++++++++++++++++++-------------
>>   1 file changed, 20 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>> index 030c6d3f89..2e13f04803 100644
>> --- a/hw/vfio/container.c
>> +++ b/hw/vfio/container.c
>> @@ -122,12 +122,12 @@ unmap_exit:
>>
>>   static int vfio_legacy_dma_unmap_one(const VFIOContainerBase
>*bcontainer,
>
>Side note for rebase :
>
>I would prefer internal routines of file hw/vfio/container.c,
>which was renamed recently to hw/vfio/container-legacy.c, to
>take a 'VFIOLegacyContainer *container' parameter.
>
>The 'VFIOContainer *bcontainer' parameter should be kept for
>high-level routines that wrap the IOMMU backend implementation.

Sure, will do

>
>>                                        hwaddr iova, ram_addr_t
>size,
>> -                                     IOMMUTLBEntry *iotlb)
>> +                                     uint32_t flags,
>IOMMUTLBEntry *iotlb)
>>   {
>>       const VFIOContainer *container =
>VFIO_IOMMU_LEGACY(bcontainer);
>>       struct vfio_iommu_type1_dma_unmap unmap = {
>>           .argsz = sizeof(unmap),
>> -        .flags = 0,
>> +        .flags = flags,
>>           .iova = iova,>           .size = size,
>>       };
>> @@ -187,25 +187,32 @@ static int vfio_legacy_dma_unmap(const
>VFIOContainerBase *bcontainer,
>>                                    hwaddr iova, ram_addr_t size,
>>                                    IOMMUTLBEntry *iotlb, bool
>unmap_all)
>>   {
>> +    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);
>> +        const VFIOContainer *container =
>VFIO_IOMMU_LEGACY(bcontainer);
>>
>> -        ret = vfio_legacy_dma_unmap_one(bcontainer, 0,
>int128_get64(llsize),
>> -                                        iotlb);
>> +        assert(!iova && !size);
>
>The assert deserves an explanation and so probably a new patch.

Sure, will do.
I'd like to move it into core code vfio_listener_region_del().

Thanks
Zhenzhong

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

[Duan, Zhenzhong]

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH 1/2] vfio/container: Support unmap all in one ioctl()
>
>On 10/1/25 00:04, John Levon wrote:
>> On Tue, Sep 30, 2025 at 05:26:59PM +0200, Cédric Le Goater wrote:
>>
>>>> -        }
>>>> +        ret = ioctl(container->fd, VFIO_CHECK_EXTENSION,
>VFIO_UNMAP_ALL);
>>>
>>> Could we introduce an helper to test 'unmap_all' support in the host
>>> kernel ? The result would be something like :
>>>
>>>    if (unmap_all) {
>>>          if
>(vfio_legacy_has_unmap_all(VFIO_IOMMU_LEGACY(bcontainer))) {
>>>              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);
>>>              ...
>>>          }
>>>    }
>>
>> For reference/consideration, the previous approach taken in the vfio-user
>> series:
>>
>>
>https://lore.kernel.org/qemu-devel/20250219144858.266455-4-john.levon@
>nutanix.com/
>>
>> @@ -533,6 +562,11 @@ static bool vfio_legacy_setup(VFIOContainerBase
>*bcontainer, Error **errp)
>>       vfio_get_info_iova_range(info, bcontainer);
>>
>>       vfio_get_iommu_info_migration(container, info);
>> +
>> +    ret = ioctl(container->fd, VFIO_CHECK_EXTENSION,
>VFIO_UNMAP_ALL);
>> +
>> +    container->unmap_all_supported = (ret != 0);

Good suggestion, thanks John.

>>
>> (I dropped this particular change as part of getting merged.)
>Yes. I'm reconsidering now.
>
>Should we introduce a VFIOContainerBase attribute/flag
>'unmap_all_supported',
>set in the vioc->setup handler ?

Do you mean to check bcontainer->unmap_all_supported and do the split in vfio_listener_region_del()?

If only checking it in legacy container, putting it in VFIOLegacyContainer sounds better?

Thanks
Zhenzhong

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

[Cédric Le Goater]

On 10/8/25 12:18, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Cédric Le Goater <clg@redhat.com>
>> Subject: Re: [PATCH 1/2] vfio/container: Support unmap all in one ioctl()
>>
>> On 10/1/25 00:04, John Levon wrote:
>>> On Tue, Sep 30, 2025 at 05:26:59PM +0200, Cédric Le Goater wrote:
>>>
>>>>> -        }
>>>>> +        ret = ioctl(container->fd, VFIO_CHECK_EXTENSION,
>> VFIO_UNMAP_ALL);
>>>>
>>>> Could we introduce an helper to test 'unmap_all' support in the host
>>>> kernel ? The result would be something like :
>>>>
>>>>     if (unmap_all) {
>>>>           if
>> (vfio_legacy_has_unmap_all(VFIO_IOMMU_LEGACY(bcontainer))) {
>>>>               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);
>>>>               ...
>>>>           }
>>>>     }
>>>
>>> For reference/consideration, the previous approach taken in the vfio-user
>>> series:
>>>
>>>
>> https://lore.kernel.org/qemu-devel/20250219144858.266455-4-john.levon@
>> nutanix.com/
>>>
>>> @@ -533,6 +562,11 @@ static bool vfio_legacy_setup(VFIOContainerBase
>> *bcontainer, Error **errp)
>>>        vfio_get_info_iova_range(info, bcontainer);
>>>
>>>        vfio_get_iommu_info_migration(container, info);
>>> +
>>> +    ret = ioctl(container->fd, VFIO_CHECK_EXTENSION,
>> VFIO_UNMAP_ALL);
>>> +
>>> +    container->unmap_all_supported = (ret != 0);
> 
> Good suggestion, thanks John.
> 
>>>
>>> (I dropped this particular change as part of getting merged.)
>> Yes. I'm reconsidering now.
>>
>> Should we introduce a VFIOContainerBase attribute/flag
>> 'unmap_all_supported',
>> set in the vioc->setup handler ?
> 
> Do you mean to check bcontainer->unmap_all_supported and do the split in vfio_listener_region_del()?
> 
> If only checking it in legacy container, putting it in VFIOLegacyContainer sounds better?

It depends if vfio-user needs it too.

C.

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

[John Levon]

On Wed, Oct 08, 2025 at 03:01:34PM +0200, Cédric Le Goater wrote:

> > > Should we introduce a VFIOContainerBase attribute/flag
> > > 'unmap_all_supported',
> > > set in the vioc->setup handler ?
> > 
> > Do you mean to check bcontainer->unmap_all_supported and do the split in vfio_listener_region_del()?
> > 
> > If only checking it in legacy container, putting it in VFIOLegacyContainer sounds better?
> 
> It depends if vfio-user needs it too.

vfio-user always supports this flag.

regards
john

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

[Cédric Le Goater]

On 10/8/25 15:11, John Levon wrote:
> On Wed, Oct 08, 2025 at 03:01:34PM +0200, Cédric Le Goater wrote:
> 
>>>> Should we introduce a VFIOContainerBase attribute/flag
>>>> 'unmap_all_supported',
>>>> set in the vioc->setup handler ?
>>>
>>> Do you mean to check bcontainer->unmap_all_supported and do the split in vfio_listener_region_del()?
>>>
>>> If only checking it in legacy container, putting it in VFIOLegacyContainer sounds better?
>>
>> It depends if vfio-user needs it too.
> 
> vfio-user always supports this flag.

In that case, putting it in VFIOLegacyContainer sounds better.

Thanks,

C.