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

[Duan, Zhenzhong]

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH v3 13/19] vfio: Create host IOMMU device instance
>
>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_IOMMUF
>D));
>> @@ -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.

I see, also VFIOIOMMUClass:: setup and VFIOIOMMUClass::add_window.
I can add cleanup patches to fix them if you have no other plan.

Thanks
Zhenzhong

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