RE: [PATCH v6 5/9] vfio/iommufd: Probe and request hwpt dirty tracking capability

[Duan, Zhenzhong]

>-----Original Message-----
>From: Joao Martins <joao.m.martins@oracle.com>
>Subject: [PATCH v6 5/9] vfio/iommufd: Probe and request hwpt dirty
>tracking capability
>
>In preparation to using the dirty tracking UAPI, probe whether the IOMMU
>supports dirty tracking. This is done via the data stored in
>hiod::caps::hw_caps initialized from GET_HW_INFO.
>
>Qemu doesn't know if VF dirty tracking is supported when allocating
>hardware pagetable in iommufd_cdev_autodomains_get(). This is because
>VFIODevice migration state hasn't been initialized *yet* hence it can't pick
>between VF dirty tracking vs IOMMU dirty tracking. So, if IOMMU supports
>dirty tracking it always creates HWPTs with
>IOMMU_HWPT_ALLOC_DIRTY_TRACKING
>even if later on VFIOMigration decides to use VF dirty tracking instead.
>
>Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>---
> include/hw/vfio/vfio-common.h |  2 ++
> hw/vfio/iommufd.c             | 20 ++++++++++++++++++++
> 2 files changed, 22 insertions(+)
>
>diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-
>common.h
>index 4e44b26d3c45..1e02c98b09ba 100644
>--- a/include/hw/vfio/vfio-common.h
>+++ b/include/hw/vfio/vfio-common.h
>@@ -97,6 +97,7 @@ typedef struct IOMMUFDBackend IOMMUFDBackend;
>
> typedef struct VFIOIOASHwpt {
>     uint32_t hwpt_id;
>+    uint32_t hwpt_flags;
>     QLIST_HEAD(, VFIODevice) device_list;
>     QLIST_ENTRY(VFIOIOASHwpt) next;
> } VFIOIOASHwpt;
>@@ -139,6 +140,7 @@ typedef struct VFIODevice {
>     OnOffAuto pre_copy_dirty_page_tracking;
>     bool dirty_pages_supported;
>     bool dirty_tracking;
>+    bool iommu_dirty_tracking;
>     HostIOMMUDevice *hiod;
>     int devid;
>     IOMMUFDBackend *iommufd;
>diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>index 2324bf892c56..7afea0b041ed 100644
>--- a/hw/vfio/iommufd.c
>+++ b/hw/vfio/iommufd.c
>@@ -110,6 +110,11 @@ static void
>iommufd_cdev_unbind_and_disconnect(VFIODevice *vbasedev)
>     iommufd_backend_disconnect(vbasedev->iommufd);
> }
>
>+static bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt)
>+{
>+    return hwpt && hwpt->hwpt_flags &
>IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
>+}
>+
> static int iommufd_cdev_getfd(const char *sysfs_path, Error **errp)
> {
>     ERRP_GUARD();
>@@ -246,6 +251,17 @@ static bool
>iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>         }
>     }
>
>+    /*
>+     * This is quite early and VFIO Migration state isn't yet fully
>+     * initialized, thus rely only on IOMMU hardware capabilities as to
>+     * whether IOMMU dirty tracking is going to be requested. Later
>+     * vfio_migration_realize() may decide to use VF dirty tracking
>+     * instead.
>+     */
>+    if (vbasedev->hiod->caps.hw_caps &
>IOMMU_HW_CAP_DIRTY_TRACKING) {
>+        flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
>+    }
>+
>     if (!iommufd_backend_alloc_hwpt(iommufd, vbasedev->devid,
>                                     container->ioas_id, flags,
>                                     IOMMU_HWPT_DATA_NONE, 0, NULL,
>@@ -255,6 +271,7 @@ static bool
>iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>
>     hwpt = g_malloc0(sizeof(*hwpt));
>     hwpt->hwpt_id = hwpt_id;
>+    hwpt->hwpt_flags = flags;
>     QLIST_INIT(&hwpt->device_list);
>
>     ret = iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt->hwpt_id, errp);
>@@ -265,8 +282,11 @@ static bool
>iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>     }
>
>     vbasedev->hwpt = hwpt;
>+    vbasedev->iommu_dirty_tracking =
>iommufd_hwpt_dirty_tracking(hwpt);

Don't we need to do same if attach to existing hwpt?

>     QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, hwpt_next);
>     QLIST_INSERT_HEAD(&container->hwpt_list, hwpt, next);
>+    container->bcontainer.dirty_pages_supported |=
>+                                vbasedev->iommu_dirty_tracking;
>     return true;
> }
>
>--
>2.17.2