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

[Eric Auger <eric.auger@redhat.com>]

Hi Joao,

On 7/22/24 23:13, Joao Martins wrote:
> 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);
>      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;
Is it possible to have several devices with different

iommu_dirty_tracking value in the same container? In other words would they be attached to different container/ioas?

Eric

>      return true;
>  }
>