Re: [PATCH V1 18/26] vfio/iommufd: define iommufd_cdev_make_hwpt

2025-02-04 Thread Steven Sistare 

On 2/4/2025 11:22 AM, Cédric Le Goater wrote:

On 1/29/25 15:43, Steve Sistare wrote:

Refactor and define iommufd_cdev_make_hwpt, to be called by CPR in a
a later patch.  No functional change.

Signed-off-by: Steve Sistare 
---
  hw/vfio/iommufd.c | 69 +--
  1 file changed, 41 insertions(+), 28 deletions(-)

diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 3490a8f..42ba63f 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -275,6 +275,41 @@ static bool iommufd_cdev_detach_ioas_hwpt(VFIODevice 
*vbasedev, Error **errp)
  return true;
  }
+static void iommufd_cdev_set_hwpt(VFIODevice *vbasedev, VFIOIOASHwpt *hwpt)
+{
+    vbasedev->hwpt = hwpt;
+    vbasedev->iommu_dirty_tracking = iommufd_hwpt_dirty_tracking(hwpt);
+    QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, hwpt_next);
+}
+
+static VFIOIOASHwpt *iommufd_cdev_make_hwpt(VFIODevice *vbasedev,
+    VFIOIOMMUFDContainer *container,
+    uint32_t hwpt_id)
+{
+    VFIOIOASHwpt *hwpt = g_malloc0(sizeof(*hwpt));
+    uint32_t flags = 0;
+
+    /*
+ * 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;
+    }
+
+    hwpt->hwpt_id = hwpt_id;
+    hwpt->hwpt_flags = flags;
+    QLIST_INIT(&hwpt->device_list);
+
+    QLIST_INSERT_HEAD(&container->hwpt_list, hwpt, next);
+    container->bcontainer.dirty_pages_supported |=
+    vbasedev->iommu_dirty_tracking;
+    return hwpt;
+}
+
  static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
   VFIOIOMMUFDContainer *container,
   Error **errp)
@@ -304,24 +339,11 @@ static bool iommufd_cdev_autodomains_get(VFIODevice 
*vbasedev,
  return false;
  } else {
-    vbasedev->hwpt = hwpt;
-    QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, hwpt_next);
-    vbasedev->iommu_dirty_tracking = iommufd_hwpt_dirty_tracking(hwpt);
+    iommufd_cdev_set_hwpt(vbasedev, hwpt);
  return true;
  }
  }
-    /*
- * 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;
-    }



AFAICT, iommufd_backend_alloc_hwpt() below needs the flag value.


Good catch, that's a bug, will fix.

- Steve


  if (!iommufd_backend_alloc_hwpt(iommufd, vbasedev->devid,
  container->ioas_id, flags,
  IOMMU_HWPT_DATA_NONE, 0, NULL,
@@ -329,24 +351,15 @@ static bool iommufd_cdev_autodomains_get(VFIODevice 
*vbasedev,
  return false;
  }
-    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);
+    ret = iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt_id, errp);
  if (ret) {
-    iommufd_backend_free_id(container->be, hwpt->hwpt_id);
-    g_free(hwpt);
+    iommufd_backend_free_id(container->be, hwpt_id);
  return false;
  }
-    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;
+    hwpt = iommufd_cdev_make_hwpt(vbasedev, container, hwpt_id);
+    iommufd_cdev_set_hwpt(vbasedev, hwpt);
+
  if (container->bcontainer.dirty_pages_supported &&
  !vbasedev->iommu_dirty_tracking) {
  warn_report("IOMMU instance for device %s doesn't support dirty 
tracking",

Re: [PATCH V1 18/26] vfio/iommufd: define iommufd_cdev_make_hwpt

2025-02-04 Thread Cédric Le Goater 

On 1/29/25 15:43, Steve Sistare wrote:

Refactor and define iommufd_cdev_make_hwpt, to be called by CPR in a
a later patch.  No functional change.

Signed-off-by: Steve Sistare 
---
  hw/vfio/iommufd.c | 69 +--
  1 file changed, 41 insertions(+), 28 deletions(-)

diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 3490a8f..42ba63f 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -275,6 +275,41 @@ static bool iommufd_cdev_detach_ioas_hwpt(VFIODevice 
*vbasedev, Error **errp)
  return true;
  }
  
+static void iommufd_cdev_set_hwpt(VFIODevice *vbasedev, VFIOIOASHwpt *hwpt)

+{
+vbasedev->hwpt = hwpt;
+vbasedev->iommu_dirty_tracking = iommufd_hwpt_dirty_tracking(hwpt);
+QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, hwpt_next);
+}
+
+static VFIOIOASHwpt *iommufd_cdev_make_hwpt(VFIODevice *vbasedev,
+VFIOIOMMUFDContainer *container,
+uint32_t hwpt_id)
+{
+VFIOIOASHwpt *hwpt = g_malloc0(sizeof(*hwpt));
+uint32_t flags = 0;
+
+/*
+ * 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;
+}
+
+hwpt->hwpt_id = hwpt_id;
+hwpt->hwpt_flags = flags;
+QLIST_INIT(&hwpt->device_list);
+
+QLIST_INSERT_HEAD(&container->hwpt_list, hwpt, next);
+container->bcontainer.dirty_pages_supported |=
+vbasedev->iommu_dirty_tracking;
+return hwpt;
+}
+
  static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
   VFIOIOMMUFDContainer *container,
   Error **errp)
@@ -304,24 +339,11 @@ static bool iommufd_cdev_autodomains_get(VFIODevice 
*vbasedev,
  
  return false;

  } else {
-vbasedev->hwpt = hwpt;
-QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, hwpt_next);
-vbasedev->iommu_dirty_tracking = iommufd_hwpt_dirty_tracking(hwpt);
+iommufd_cdev_set_hwpt(vbasedev, hwpt);
  return true;
  }
  }
  
-/*

- * 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;
-}



AFAICT, iommufd_backend_alloc_hwpt() below needs the flag value.

Thanks,

C.




  if (!iommufd_backend_alloc_hwpt(iommufd, vbasedev->devid,
  container->ioas_id, flags,
  IOMMU_HWPT_DATA_NONE, 0, NULL,
@@ -329,24 +351,15 @@ static bool iommufd_cdev_autodomains_get(VFIODevice 
*vbasedev,
  return false;
  }
  
-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);
+ret = iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt_id, errp);
  if (ret) {
-iommufd_backend_free_id(container->be, hwpt->hwpt_id);
-g_free(hwpt);
+iommufd_backend_free_id(container->be, hwpt_id);
  return false;
  }
  
-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;
+hwpt = iommufd_cdev_make_hwpt(vbasedev, container, hwpt_id);
+iommufd_cdev_set_hwpt(vbasedev, hwpt);
+
  if (container->bcontainer.dirty_pages_supported &&
  !vbasedev->iommu_dirty_tracking) {
  warn_report("IOMMU instance for device %s doesn't support dirty 
tracking",

[PATCH V1 18/26] vfio/iommufd: define iommufd_cdev_make_hwpt

2025-01-29 Thread Steve Sistare 
Refactor and define iommufd_cdev_make_hwpt, to be called by CPR in a
a later patch.  No functional change.

Signed-off-by: Steve Sistare 
---
 hw/vfio/iommufd.c | 69 +--
 1 file changed, 41 insertions(+), 28 deletions(-)

diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 3490a8f..42ba63f 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -275,6 +275,41 @@ static bool iommufd_cdev_detach_ioas_hwpt(VFIODevice 
*vbasedev, Error **errp)
 return true;
 }
 
+static void iommufd_cdev_set_hwpt(VFIODevice *vbasedev, VFIOIOASHwpt *hwpt)
+{
+vbasedev->hwpt = hwpt;
+vbasedev->iommu_dirty_tracking = iommufd_hwpt_dirty_tracking(hwpt);
+QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, hwpt_next);
+}
+
+static VFIOIOASHwpt *iommufd_cdev_make_hwpt(VFIODevice *vbasedev,
+VFIOIOMMUFDContainer *container,
+uint32_t hwpt_id)
+{
+VFIOIOASHwpt *hwpt = g_malloc0(sizeof(*hwpt));
+uint32_t flags = 0;
+
+/*
+ * 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;
+}
+
+hwpt->hwpt_id = hwpt_id;
+hwpt->hwpt_flags = flags;
+QLIST_INIT(&hwpt->device_list);
+
+QLIST_INSERT_HEAD(&container->hwpt_list, hwpt, next);
+container->bcontainer.dirty_pages_supported |=
+vbasedev->iommu_dirty_tracking;
+return hwpt;
+}
+
 static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
  VFIOIOMMUFDContainer *container,
  Error **errp)
@@ -304,24 +339,11 @@ static bool iommufd_cdev_autodomains_get(VFIODevice 
*vbasedev,
 
 return false;
 } else {
-vbasedev->hwpt = hwpt;
-QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, hwpt_next);
-vbasedev->iommu_dirty_tracking = iommufd_hwpt_dirty_tracking(hwpt);
+iommufd_cdev_set_hwpt(vbasedev, hwpt);
 return true;
 }
 }
 
-/*
- * 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,
@@ -329,24 +351,15 @@ static bool iommufd_cdev_autodomains_get(VFIODevice 
*vbasedev,
 return false;
 }
 
-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);
+ret = iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt_id, errp);
 if (ret) {
-iommufd_backend_free_id(container->be, hwpt->hwpt_id);
-g_free(hwpt);
+iommufd_backend_free_id(container->be, hwpt_id);
 return false;
 }
 
-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;
+hwpt = iommufd_cdev_make_hwpt(vbasedev, container, hwpt_id);
+iommufd_cdev_set_hwpt(vbasedev, hwpt);
+
 if (container->bcontainer.dirty_pages_supported &&
 !vbasedev->iommu_dirty_tracking) {
 warn_report("IOMMU instance for device %s doesn't support dirty 
tracking",
-- 
1.8.3.1