Hi Joao,
On 12/02/2024 15:56, Joao Martins wrote:
External email: Use caution opening links or attachments
Probe hardware dirty tracking support by querying device hw capabilities
via IOMMUFD_GET_HW_INFO.
In preparation to using the dirty tracking UAPI, request dirty tracking in
the HWPT flags when the device doesn't support dirty page tracking or has
it disabled; or when support when the VF backing IOMMU supports dirty
tracking. The latter is in the possibility of a device being attached
that doesn't have a dirty tracker.
Signed-off-by: Joao Martins <joao.m.mart...@oracle.com>
---
hw/vfio/common.c | 18 ++++++++++++++++++
hw/vfio/iommufd.c | 25 ++++++++++++++++++++++++-
include/hw/vfio/vfio-common.h | 2 ++
3 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index f7f85160be88..d8fc7077f839 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -216,6 +216,24 @@ bool vfio_devices_all_device_dirty_tracking(const
VFIOContainerBase *bcontainer)
return true;
}
+bool vfio_device_migration_supported(VFIODevice *vbasedev)
+{
+ if (!vbasedev->migration) {
+ return false;
+ }
+
+ return vbasedev->migration->mig_flags & VFIO_MIGRATION_STOP_COPY;
I think this is redundant, as (vbasedev->migration != NULL) implies
(vbasedev->migration->mig_flags & VFIO_MIGRATION_STOP_COPY) == true.
+}
+
+bool vfio_device_dirty_pages_supported(VFIODevice *vbasedev)
+{
+ if (vbasedev->pre_copy_dirty_page_tracking == ON_OFF_AUTO_OFF) {
+ return false;
+ }
+
+ return !vbasedev->dirty_pages_supported;
+}
+
/*
* Check if all VFIO devices are running and migration is active, which is
* essentially equivalent to the migration being in pre-copy phase.
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index ca7ec45e725c..edacb6d72748 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -219,11 +219,26 @@ static int iommufd_cdev_detach_ioas_hwpt(VFIODevice
*vbasedev, Error **errp)
return ret;
}
+static bool iommufd_dirty_pages_supported(IOMMUFDDevice *iommufd_dev,
+ Error **errp)
+{
+ uint64_t caps;
+ int r;
+
+ r = iommufd_device_get_hw_capabilities(iommufd_dev, &caps, errp);
+ if (r) {
+ return false;
+ }
+
+ return caps & IOMMU_HW_CAP_DIRTY_TRACKING;
The false return value of this function is overloaded, it can indicate
both error and lack of DPT support.
Should we fail iommufd_cdev_autodomains_get() if
iommufd_dirty_pages_supported() fails?
Otherwise, errp argument of iommufd_dirty_pages_supported() is redundant
and we can handle iommufd_device_get_hw_capabilities() error locally.
+}
+
static int iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
VFIOIOMMUFDContainer *container,
Error **errp)
{
int iommufd = vbasedev->iommufd_dev.iommufd->fd;
+ uint32_t flags = 0;
VFIOIOASHwpt *hwpt;
Error *err = NULL;
int ret = -EINVAL;
@@ -245,9 +260,15 @@ static int iommufd_cdev_autodomains_get(VFIODevice
*vbasedev,
}
}
+ if ((vfio_device_migration_supported(vbasedev) &&
+ !vfio_device_dirty_pages_supported(vbasedev)) ||
+ iommufd_dirty_pages_supported(&vbasedev->iommufd_dev, &err)) {
I think it's too early to check vfio_device_migration_supported() and
vfio_device_dirty_pages_supported() here, as vfio_migration_init()
hasn't been called yet so vbasedev->migration and
vbasedev->dirty_pages_supported are not initialized.
Why do we need to check this? Can't we simply request IOMMUFD DPT if
it's supported?
Thanks.
+ flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
+ }
+
ret = iommufd_backend_alloc_hwpt(iommufd,
vbasedev->iommufd_dev.devid,
- container->ioas_id, 0, 0, 0,
+ container->ioas_id, flags, 0, 0,
NULL, &hwpt_id);
if (ret) {
error_append_hint(&err,
@@ -271,6 +292,8 @@ static int iommufd_cdev_autodomains_get(VFIODevice
*vbasedev,
vbasedev->hwpt = hwpt;
QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, hwpt_next);
QLIST_INSERT_HEAD(&container->hwpt_list, hwpt, next);
+ container->bcontainer.dirty_pages_supported =
+ (flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING);
return 0;
}
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 7f7d823221e2..a3e691c126c6 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -271,6 +271,8 @@ bool
vfio_devices_all_running_and_mig_active(const VFIOContainerBase *bcontainer);
bool
vfio_devices_all_device_dirty_tracking(const VFIOContainerBase *bcontainer);
+bool vfio_device_migration_supported(VFIODevice *vbasedev);
+bool vfio_device_dirty_pages_supported(VFIODevice *vbasedev);
int vfio_devices_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
VFIOBitmap *vbmap, hwaddr iova,
hwaddr size);
--
2.39.3
