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

["Cédric Le Goater" <clg@redhat.com>]

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,