On 10/09/2025 03:36, Zhenzhong Duan wrote:
> Pass IOMMU_HWPT_GET_DIRTY_BITMAP_NO_CLEAR when doing the last dirty
> bitmap query right before unmap, no PTEs flushes. This accelerates the
> query without issue because unmap will tear down the mapping anyway.
>
> Add a new element dirty_tracking_flags in VFIOIOMMUFDContainer to
> be used for the flags of iommufd dirty tracking. Currently it is
> set to either IOMMU_HWPT_GET_DIRTY_BITMAP_NO_CLEAR or 0 based on
> the scenario.
>
> Co-developed-by: Joao Martins <[email protected]>
> Signed-off-by: Joao Martins <[email protected]>
> Signed-off-by: Zhenzhong Duan <[email protected]>
> Tested-by: Xudong Hao <[email protected]>
> Tested-by: Giovannio Cabiddu <[email protected]>
> ---
> hw/vfio/vfio-iommufd.h | 1 +
> include/system/iommufd.h | 2 +-
> backends/iommufd.c | 5 +++--
> hw/vfio/iommufd.c | 6 +++++-
> backends/trace-events | 2 +-
> 5 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/hw/vfio/vfio-iommufd.h b/hw/vfio/vfio-iommufd.h
> index 07ea0f4304..e0af241c75 100644
> --- a/hw/vfio/vfio-iommufd.h
> +++ b/hw/vfio/vfio-iommufd.h
> @@ -26,6 +26,7 @@ typedef struct VFIOIOMMUFDContainer {
> VFIOContainerBase bcontainer;
> IOMMUFDBackend *be;
> uint32_t ioas_id;
> + uint64_t dirty_tracking_flags;
> QLIST_HEAD(, VFIOIOASHwpt) hwpt_list;
> } VFIOIOMMUFDContainer;
>
> diff --git a/include/system/iommufd.h b/include/system/iommufd.h
> index c9c72ffc45..63898e7b0d 100644
> --- a/include/system/iommufd.h
> +++ b/include/system/iommufd.h
> @@ -64,7 +64,7 @@ bool iommufd_backend_set_dirty_tracking(IOMMUFDBackend *be,
> uint32_t hwpt_id,
> bool iommufd_backend_get_dirty_bitmap(IOMMUFDBackend *be, uint32_t hwpt_id,
> uint64_t iova, ram_addr_t size,
> uint64_t page_size, uint64_t *data,
> - Error **errp);
> + uint64_t flags, Error **errp);
> bool iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t id,
> uint32_t data_type, uint32_t entry_len,
> uint32_t *entry_num, void *data,
> diff --git a/backends/iommufd.c b/backends/iommufd.c
> index 2a33c7ab0b..3c4f6157e2 100644
> --- a/backends/iommufd.c
> +++ b/backends/iommufd.c
> @@ -361,7 +361,7 @@ bool iommufd_backend_get_dirty_bitmap(IOMMUFDBackend *be,
> uint32_t hwpt_id,
> uint64_t iova, ram_addr_t size,
> uint64_t page_size, uint64_t *data,
> - Error **errp)
> + uint64_t flags, Error **errp)
> {
> int ret;
> struct iommu_hwpt_get_dirty_bitmap get_dirty_bitmap = {
> @@ -371,11 +371,12 @@ bool iommufd_backend_get_dirty_bitmap(IOMMUFDBackend
> *be,
> .length = size,
> .page_size = page_size,
> .data = (uintptr_t)data,
> + .flags = flags,
> };
>
> ret = ioctl(be->fd, IOMMU_HWPT_GET_DIRTY_BITMAP, &get_dirty_bitmap);
> trace_iommufd_backend_get_dirty_bitmap(be->fd, hwpt_id, iova, size,
> - page_size, ret ? errno : 0);
> + flags, page_size, ret ? errno :
> 0);
> if (ret) {
> error_setg_errno(errp, errno,
> "IOMMU_HWPT_GET_DIRTY_BITMAP (iova: 0x%"HWADDR_PRIx
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 0057488ce9..c897aa6b17 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -62,7 +62,7 @@ static int iommufd_cdev_unmap_one(const VFIOContainerBase
> *bcontainer,
> hwaddr iova, ram_addr_t size,
> IOMMUTLBEntry *iotlb)
> {
> - const VFIOIOMMUFDContainer *container =
> + VFIOIOMMUFDContainer *container =
> container_of(bcontainer, VFIOIOMMUFDContainer, bcontainer);
> bool need_dirty_sync = false;
> Error *local_err = NULL;
> @@ -73,9 +73,12 @@ static int iommufd_cdev_unmap_one(const VFIOContainerBase
> *bcontainer,
> if (iotlb && vfio_container_dirty_tracking_is_started(bcontainer)) {
> if (!vfio_container_devices_dirty_tracking_is_supported(bcontainer)
> &&
> bcontainer->dirty_pages_supported) {
> + container->dirty_tracking_flags =
> + IOMMU_HWPT_GET_DIRTY_BITMAP_NO_CLEAR;
> ret = vfio_container_query_dirty_bitmap(bcontainer, iova, size,
> iotlb->translated_addr,
> &local_err);
> + container->dirty_tracking_flags = 0;
Why not changing vfio_container_query_dirty_bitmap to pass a flags too, like the
original patches? This is a little unnecssary odd style to pass a flag via
container structure rather and then clearing.
Part of the reason the original series had a VFIO_GET_DIRTY_NO_FLUSH for generic
container abstraction was to not mix IOMMUFD UAPI specifics into base container
API. Then in getting a VFIO_GET_DIRTY_NO_FLUSH, then type1 backend could just
ignore the flag, while IOMMUFD translates it to
IOMMU_HWPT_GET_DIRTY_BITMAP_NO_CLEAR
> if (ret) {
> error_report_err(local_err);
> }
> @@ -248,6 +251,7 @@ static int iommufd_query_dirty_bitmap(const
> VFIOContainerBase *bcontainer,
> if (!iommufd_backend_get_dirty_bitmap(container->be, hwpt->hwpt_id,
> iova, size, page_size,
> (uint64_t *)vbmap->bitmap,
> +
> container->dirty_tracking_flags,
> errp)) {
> return -EINVAL;
> }
> diff --git a/backends/trace-events b/backends/trace-events
> index 56132d3fd2..e1992ba12f 100644
> --- a/backends/trace-events
> +++ b/backends/trace-events
> @@ -19,5 +19,5 @@ iommufd_backend_alloc_ioas(int iommufd, uint32_t ioas) "
> iommufd=%d ioas=%d"
> iommufd_backend_alloc_hwpt(int iommufd, uint32_t dev_id, uint32_t pt_id,
> uint32_t flags, uint32_t hwpt_type, uint32_t len, uint64_t data_ptr, uint32_t
> out_hwpt_id, int ret) " iommufd=%d dev_id=%u pt_id=%u flags=0x%x hwpt_type=%u
> len=%u data_ptr=0x%"PRIx64" out_hwpt=%u (%d)"
> iommufd_backend_free_id(int iommufd, uint32_t id, int ret) " iommufd=%d
> id=%d (%d)"
> iommufd_backend_set_dirty(int iommufd, uint32_t hwpt_id, bool start, int
> ret) " iommufd=%d hwpt=%u enable=%d (%d)"
> -iommufd_backend_get_dirty_bitmap(int iommufd, uint32_t hwpt_id, uint64_t
> iova, uint64_t size, uint64_t page_size, int ret) " iommufd=%d hwpt=%u
> iova=0x%"PRIx64" size=0x%"PRIx64" page_size=0x%"PRIx64" (%d)"
> +iommufd_backend_get_dirty_bitmap(int iommufd, uint32_t hwpt_id, uint64_t
> iova, uint64_t size, uint64_t flags, uint64_t page_size, int ret) "
> iommufd=%d hwpt=%u iova=0x%"PRIx64" size=0x%"PRIx64" flags=0x%"PRIx64"
> page_size=0x%"PRIx64" (%d)"
> iommufd_backend_invalidate_cache(int iommufd, uint32_t id, uint32_t
> data_type, uint32_t entry_len, uint32_t entry_num, uint32_t done_num,
> uint64_t data_ptr, int ret) " iommufd=%d id=%u data_type=%u entry_len=%u
> entry_num=%u done_num=%u data_ptr=0x%"PRIx64" (%d)"
