Re: [PATCH rfcv2 03/20] HostIOMMUDevice: Introduce realize_late callback

[Cédric Le Goater]

On 2/19/25 09:22, Zhenzhong Duan wrote:
Currently we have realize() callback which is called before attachment.
But there are still some elements e.g., hwpt_id is not ready before
attachment. So we need a realize_late() callback to further initialize
them.

The relation between objects HostIOMMUDevice and VFIOIOMMU is starting
to look too complex for me.

I think it makes sense to realize HostIOMMUDevice after the device
is attached. Can't we move :

        hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
        vbasedev->hiod = hiod;

under ->attach_device() and also the call :

    if (!vfio_device_hiod_realize(vbasedev, errp)) {

later in the ->attach_device() patch ?

hiod_legacy_vfio_realize() doesn't do much. We might need to rework
hiod_iommufd_vfio_realize() which queries the iommufd hw caps, later
used by intel-iommu.

Anyway, it is good time to cleanup our interfaces before adding more.

Thanks,

C.



    
Currently, this callback is only useful for iommufd backend. For legacy
backend nothing needs to be initialized after attachment.

Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
---
  include/system/host_iommu_device.h | 17 +++++++++++++++++
  hw/vfio/common.c                   | 17 ++++++++++++++---
  2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/include/system/host_iommu_device.h 
b/include/system/host_iommu_device.h
index 809cced4ba..df782598f2 100644
--- a/include/system/host_iommu_device.h
+++ b/include/system/host_iommu_device.h
@@ -66,6 +66,23 @@ struct HostIOMMUDeviceClass {
       * Returns: true on success, false on failure.
       */
      bool (*realize)(HostIOMMUDevice *hiod, void *opaque, Error **errp);
+    /**
+     * @realize_late: initialize host IOMMU device instance after attachment,
+     *                some elements e.g., ioas are ready only after attachment.
+     *                This callback initialize them.
+     *
+     * Optional callback.
+     *
+     * @hiod: pointer to a host IOMMU device instance.
+     *
+     * @opaque: pointer to agent device of this host IOMMU device,
+     *          e.g., VFIO base device or VDPA device.
+     *
+     * @errp: pass an Error out when realize fails.
+     *
+     * Returns: true on success, false on failure.
+     */
+    bool (*realize_late)(HostIOMMUDevice *hiod, void *opaque, Error **errp);
      /**
       * @get_cap: check if a host IOMMU device capability is supported.
       *
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index abbdc56b6d..e198b1e5a2 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1550,6 +1550,7 @@ bool vfio_attach_device(char *name, VFIODevice *vbasedev,
      const VFIOIOMMUClass *ops =
          VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY));
      HostIOMMUDevice *hiod = NULL;
+    HostIOMMUDeviceClass *hiod_ops = NULL;
  
      if (vbasedev->iommufd) {
          ops = VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
@@ -1560,16 +1561,26 @@ bool vfio_attach_device(char *name, VFIODevice 
*vbasedev,
  
      if (!vbasedev->mdev) {
          hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
+        hiod_ops = HOST_IOMMU_DEVICE_GET_CLASS(hiod);
          vbasedev->hiod = hiod;
      }
  
      if (!ops->attach_device(name, vbasedev, as, errp)) {
-        object_unref(hiod);
-        vbasedev->hiod = NULL;
-        return false;
+        goto err_attach;
+    }
+
+    if (hiod_ops && hiod_ops->realize_late &&
+        !hiod_ops->realize_late(hiod, vbasedev, errp)) {
+        ops->detach_device(vbasedev);
+        goto err_attach;
      }
  
      return true;
+
+err_attach:
+    object_unref(hiod);
+    vbasedev->hiod = NULL;
+    return false;
  }
  
  void vfio_detach_device(VFIODevice *vbasedev)