Re: [PATCH v3 13/19] vfio: Create host IOMMU device instance

[Cédric Le Goater]

On 4/29/24 08:50, Zhenzhong Duan wrote:
> Create host IOMMU device instance in vfio_attach_device() and call
> .realize() to initialize it further.
>
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   include/hw/vfio/vfio-common.h |  1 +
>   hw/vfio/common.c              | 18 +++++++++++++++++-
>   2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 0943add3bc..b204b93a55 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -126,6 +126,7 @@ typedef struct VFIODevice {
>       OnOffAuto pre_copy_dirty_page_tracking;
>       bool dirty_pages_supported;
>       bool dirty_tracking;
> +    HostIOMMUDevice *hiod;
>       int devid;
>       IOMMUFDBackend *iommufd;
>   } VFIODevice;
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 8f9cbdc026..0be8b70ebd 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1497,6 +1497,8 @@ int vfio_attach_device(char *name, VFIODevice *vbasedev,
>   {
>       const VFIOIOMMUClass *ops =
>           VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY));
> +    HostIOMMUDevice *hiod;
> +    int ret;
>   
>       if (vbasedev->iommufd) {
>           ops = VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
> @@ -1504,7 +1506,20 @@ int vfio_attach_device(char *name, VFIODevice *vbasedev,
>   
>       assert(ops);
>   
> -    return ops->attach_device(name, vbasedev, as, errp);
> +    ret = ops->attach_device(name, vbasedev, as, errp);
> +    if (ret < 0) {
> +        return ret;


hmm, I wonder if we should change the return value of vfio_attach_device()
to be a bool.


Thanks,

C.

        

> +    }
> +
> +    hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
> +    if (!HOST_IOMMU_DEVICE_GET_CLASS(hiod)->realize(hiod, vbasedev, errp)) {
> +        object_unref(hiod);
> +        ops->detach_device(vbasedev);
> +        return -EINVAL;
> +    }
> +    vbasedev->hiod = hiod;
> +
> +    return 0;
>   }
>   
>   void vfio_detach_device(VFIODevice *vbasedev)
> @@ -1512,5 +1527,6 @@ void vfio_detach_device(VFIODevice *vbasedev)
>       if (!vbasedev->bcontainer) {
>           return;
>       }
> +    object_unref(vbasedev->hiod);
>       vbasedev->bcontainer->ops->detach_device(vbasedev);
>   }