>-----Original Message-----
>From: Cédric Le Goater <c...@redhat.com>
>Subject: Re: [PATCH v2 03/10] backends/iommufd: Introduce abstract
>HIODIOMMUFD device
>
>On 4/8/24 10:12, Zhenzhong Duan wrote:
>> HIODIOMMUFD represents a host IOMMU device under iommufd backend.
>>
>> Currently it includes only public iommufd handle and device id.
>> which could be used to get hw IOMMU information.
>>
>> When nested translation is supported in future, vIOMMU is going
>> to have iommufd related operations like attaching/detaching hwpt,
>> So IOMMUFDDevice interface will be further extended at that time.
>>
>> VFIO and VDPA device have different way of attaching/detaching hwpt.
>> So HIODIOMMUFD is still an abstract class which will be inherited by
>> VFIO and VDPA device.
>>
>> Introduce a helper hiod_iommufd_init() to initialize HIODIOMMUFD
>> device.
>>
>> Suggested-by: Cédric Le Goater <c...@redhat.com>
>> Originally-by: Yi Liu <yi.l....@intel.com>
>> Signed-off-by: Yi Sun <yi.y....@linux.intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>> ---
>> include/sysemu/iommufd.h | 22 +++++++++++++++++++
>> backends/iommufd.c | 47 ++++++++++++++++++++++++++--------------
>> 2 files changed, 53 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/sysemu/iommufd.h b/include/sysemu/iommufd.h
>> index 9af27ebd6c..71c53cbb45 100644
>> --- a/include/sysemu/iommufd.h
>> +++ b/include/sysemu/iommufd.h
>> @@ -4,6 +4,7 @@
>> #include "qom/object.h"
>> #include "exec/hwaddr.h"
>> #include "exec/cpu-common.h"
>> +#include "sysemu/host_iommu_device.h"
>>
>> #define TYPE_IOMMUFD_BACKEND "iommufd"
>> OBJECT_DECLARE_TYPE(IOMMUFDBackend, IOMMUFDBackendClass,
>IOMMUFD_BACKEND)
>> @@ -33,4 +34,25 @@ int
>iommufd_backend_map_dma(IOMMUFDBackend *be, uint32_t ioas_id,
>hwaddr iova,
>> ram_addr_t size, void *vaddr, bool readonly);
>> int iommufd_backend_unmap_dma(IOMMUFDBackend *be, uint32_t
>ioas_id,
>> hwaddr iova, ram_addr_t size);
>> +
>> +#define TYPE_HIOD_IOMMUFD TYPE_HOST_IOMMU_DEVICE "-iommufd"
>
>Please keep TYPE_HOST_IOMMU_DEVICE
Sure.
>
>> +OBJECT_DECLARE_TYPE(HIODIOMMUFD, HIODIOMMUFDClass,
>HIOD_IOMMUFD)
>> +
>> +struct HIODIOMMUFD {
>> + /*< private >*/
>> + HostIOMMUDevice parent;
>> + void *opaque;
>> +
>> + /*< public >*/
>> + IOMMUFDBackend *iommufd;
>> + uint32_t devid;
>> +};
>> +
>> +struct HIODIOMMUFDClass {
>> + /*< private >*/
>> + HostIOMMUDeviceClass parent_class;
>> +};
>
>This new class doesn't seem useful. Do you have plans for handlers ?
Yes, In nesting series
https://github.com/yiliu1765/qemu/commits/zhenzhong/iommufd_nesting_rfcv2/
This commit
https://github.com/yiliu1765/qemu/commit/581fc900aa296988eaa48abee6d68d3670faf8c9
implement [at|de]tach_hwpt handlers.
So I add an extra layer of abstract HIODIOMMUFDClass.
>
>> +
>> +void hiod_iommufd_init(HIODIOMMUFD *idev, IOMMUFDBackend
>*iommufd,
>> + uint32_t devid);
>> #endif
>> diff --git a/backends/iommufd.c b/backends/iommufd.c
>> index 62a79fa6b0..ef8b3a808b 100644
>> --- a/backends/iommufd.c
>> +++ b/backends/iommufd.c
>> @@ -212,23 +212,38 @@ int
>iommufd_backend_unmap_dma(IOMMUFDBackend *be, uint32_t ioas_id,
>> return ret;
>> }
>>
>> -static const TypeInfo iommufd_backend_info = {
>> - .name = TYPE_IOMMUFD_BACKEND,
>> - .parent = TYPE_OBJECT,
>> - .instance_size = sizeof(IOMMUFDBackend),
>> - .instance_init = iommufd_backend_init,
>> - .instance_finalize = iommufd_backend_finalize,
>> - .class_size = sizeof(IOMMUFDBackendClass),
>> - .class_init = iommufd_backend_class_init,
>> - .interfaces = (InterfaceInfo[]) {
>> - { TYPE_USER_CREATABLE },
>> - { }
>> - }
>> -};
>> +void hiod_iommufd_init(HIODIOMMUFD *idev, IOMMUFDBackend
>*iommufd,
>> + uint32_t devid)
>> +{
>> + idev->iommufd = iommufd;
>> + idev->devid = devid;
>> +}
>
>This routine doesn't seem useful. I wonder if we shouldn't introduce
>properties. I'm not sure this is useful either.
This routine is called in patch8 to initialize iommu, devid and ioas(in future
nesting series).
I didn't choose properties as HIODIOMMUFD is not user creatable, property is a
bit heavy
here. But I'm fine to use it if you prefer.
Thanks
Zhenzhong
>
>
>> -static void register_types(void)
>> +static void hiod_iommufd_class_init(ObjectClass *oc, void *data)
>> {
>> - type_register_static(&iommufd_backend_info);
>> }
>>
>> -type_init(register_types);
>> +static const TypeInfo types[] = {
>> + {
>> + .name = TYPE_IOMMUFD_BACKEND,
>> + .parent = TYPE_OBJECT,
>> + .instance_size = sizeof(IOMMUFDBackend),
>> + .instance_init = iommufd_backend_init,
>> + .instance_finalize = iommufd_backend_finalize,
>> + .class_size = sizeof(IOMMUFDBackendClass),
>> + .class_init = iommufd_backend_class_init,
>> + .interfaces = (InterfaceInfo[]) {
>> + { TYPE_USER_CREATABLE },
>> + { }
>> + }
>> + }, {
>> + .name = TYPE_HIOD_IOMMUFD,
>> + .parent = TYPE_HOST_IOMMU_DEVICE,
>> + .instance_size = sizeof(HIODIOMMUFD),
>> + .class_size = sizeof(HIODIOMMUFDClass),
>> + .class_init = hiod_iommufd_class_init,
>> + .abstract = true,
>> + }
>> +};
>> +
>> +DEFINE_TYPES(types)
