Re: [PATCH 1/5] vfio/iommufd: Add framework code to support getting dirty bitmap before unmap

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

Hello Zhenzhong

On 9/10/25 04:36, Zhenzhong Duan wrote:
> Currently we support device and iommu dirty tracking, device dirty
> tracking is preferred.
> 
> Add the framework code in iommufd_cdev_unmap_one() to choose either
> device or iommu dirty tracking, just like vfio_legacy_dma_unmap_one().

I wonder if commit 567d7d3e6be5 ("vfio/common: Work around kernel
overflow bug in DMA unmap") could be removed now to make the code
common to both VFIO IOMMU Type1 and IOMMUFD backends.

I asked Alex and Peter in another thread.

Thanks,

C.




> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Tested-by: Xudong Hao <xudong.hao@intel.com>
> Tested-by: Giovannio Cabiddu <giovanni.cabiddu@intel.com>
> ---
>   hw/vfio/iommufd.c | 56 +++++++++++++++++++++++++++++++++++++----------
>   1 file changed, 45 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 48c590b6a9..b5d6e54c45 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -58,33 +58,67 @@ static int iommufd_cdev_map_file(const VFIOContainerBase *bcontainer,
>                                           iova, size, fd, start, readonly);
>   }
>   
> -static int iommufd_cdev_unmap(const VFIOContainerBase *bcontainer,
> -                              hwaddr iova, ram_addr_t size,
> -                              IOMMUTLBEntry *iotlb, bool unmap_all)
> +static int iommufd_cdev_unmap_one(const VFIOContainerBase *bcontainer,
> +                                  hwaddr iova, ram_addr_t size,
> +                                  IOMMUTLBEntry *iotlb)
>   {
>       const VFIOIOMMUFDContainer *container =
>           container_of(bcontainer, VFIOIOMMUFDContainer, bcontainer);
> +    bool need_dirty_sync = false;
> +    Error *local_err = NULL;
> +    IOMMUFDBackend *be = container->be;
> +    uint32_t ioas_id = container->ioas_id;
> +    int ret;
> +
> +    if (iotlb && vfio_container_dirty_tracking_is_started(bcontainer)) {
> +        if (!vfio_container_devices_dirty_tracking_is_supported(bcontainer) &&
> +            bcontainer->dirty_pages_supported) {
> +            /* TODO: query dirty bitmap before DMA unmap */
> +            return iommufd_backend_unmap_dma(be, ioas_id, iova, size);
> +        }
> +
> +        need_dirty_sync = true;
> +    }
> +
> +    ret = iommufd_backend_unmap_dma(be, ioas_id, iova, size);
> +    if (ret) {
> +        return ret;
> +    }
>   
> +    if (need_dirty_sync) {
> +        ret = vfio_container_query_dirty_bitmap(bcontainer, iova, size,
> +                                                iotlb->translated_addr,
> +                                                &local_err);
> +        if (ret) {
> +            error_report_err(local_err);
> +            return ret;
> +        }
> +    }
> +
> +    return 0;
> +}
> +static int iommufd_cdev_unmap(const VFIOContainerBase *bcontainer,
> +                              hwaddr iova, ram_addr_t size,
> +                              IOMMUTLBEntry *iotlb, bool unmap_all)
> +{
>       /* 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));
> +        ret = iommufd_cdev_unmap_one(bcontainer, 0, int128_get64(llsize),
> +                                     iotlb);
>   
>           if (ret == 0) {
> -            ret = iommufd_backend_unmap_dma(container->be, container->ioas_id,
> -                                            int128_get64(llsize),
> -                                            int128_get64(llsize));
> +            ret = iommufd_cdev_unmap_one(bcontainer, int128_get64(llsize),
> +                                         int128_get64(llsize), iotlb);
>           }
>   
>           return ret;
>       }
>   
> -    /* TODO: Handle dma_unmap_bitmap with iotlb args (migration) */
> -    return iommufd_backend_unmap_dma(container->be,
> -                                     container->ioas_id, iova, size);
> +    return iommufd_cdev_unmap_one(bcontainer, iova, size, iotlb);
>   }
>   
>   static bool iommufd_cdev_kvm_device_add(VFIODevice *vbasedev, Error **errp)