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

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

On 2/19/25 9:22 AM, 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.
from the description it is not obvious why the realize() could not have
been called after the attach. Could you remind the reader what is the
reason?

Thanks

Eric
>
> 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.duan@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)