>-----Original Message----- >From: Eric Auger <eric.au...@redhat.com> >Subject: Re: [PATCH v1 05/11] vfio: Introduce host_iommu_device_create >callback > > > >On 2/28/24 04:58, Zhenzhong Duan wrote: >> Introduce host_iommu_device_create callback and a wrapper for it. >> >> This callback is used to allocate a host iommu device instance and >> initialize it based on type. >> >> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com> >> --- >> include/hw/vfio/vfio-common.h | 1 + >> include/hw/vfio/vfio-container-base.h | 1 + >> hw/vfio/common.c | 8 ++++++++ >> 3 files changed, 10 insertions(+) >> >> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio- >common.h >> index b6676c9f79..9fefea4b89 100644 >> --- a/include/hw/vfio/vfio-common.h >> +++ b/include/hw/vfio/vfio-common.h >> @@ -208,6 +208,7 @@ struct vfio_device_info *vfio_get_device_info(int >fd); >> int vfio_attach_device(char *name, VFIODevice *vbasedev, >> AddressSpace *as, Error **errp); >> void vfio_detach_device(VFIODevice *vbasedev); >> +void host_iommu_device_create(VFIODevice *vbasedev); >> >> int vfio_kvm_device_add_fd(int fd, Error **errp); >> int vfio_kvm_device_del_fd(int fd, Error **errp); >> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio- >container-base.h >> index b2813b0c11..dc003f6eb2 100644 >> --- a/include/hw/vfio/vfio-container-base.h >> +++ b/include/hw/vfio/vfio-container-base.h >> @@ -120,6 +120,7 @@ struct VFIOIOMMUClass { >> int (*attach_device)(const char *name, VFIODevice *vbasedev, >> AddressSpace *as, Error **errp); >> void (*detach_device)(VFIODevice *vbasedev); >> + void (*host_iommu_device_create)(VFIODevice *vbasedev); >Maybe return an int instead. It is common the allocation can fail and >the deallocation cannot. While at it I would also pass an errp in case >it fails
Currently host_iommu_device_create implementation only calls g_malloc0, so never fails, so I returned void. I'm fine to return an int, will be like below, take iommufd for example: --- a/hw/vfio/iommufd.c +++ b/hw/vfio/iommufd.c @@ -651,7 +651,7 @@ static IOMMUFDDeviceOps vfio_iommufd_device_ops = { .detach_hwpt = vfio_iommufd_device_detach_hwpt, }; -static void vfio_cdev_host_iommu_device_create(VFIODevice *vbasedev) +static int vfio_cdev_host_iommu_device_create(VFIODevice *vbasedev, Error **errp) { IOMMUFDDevice *idev = g_malloc0(sizeof(IOMMUFDDevice)); VFIOIOMMUFDContainer *container = container_of(vbasedev->bcontainer, @@ -661,6 +661,8 @@ static void vfio_cdev_host_iommu_device_create(VFIODevice *vbasedev) iommufd_device_init(idev, vbasedev->iommufd, vbasedev->devid, container->ioas_id, vbasedev, &vfio_iommufd_device_ops); + + return 0; } Thanks Zhenzhong > >Eric >> /* migration feature */ >> int (*set_dirty_page_tracking)(const VFIOContainerBase *bcontainer, >> bool start); >> diff --git a/hw/vfio/common.c b/hw/vfio/common.c >> index 059bfdc07a..41e9031c59 100644 >> --- a/hw/vfio/common.c >> +++ b/hw/vfio/common.c >> @@ -1521,3 +1521,11 @@ void vfio_detach_device(VFIODevice >*vbasedev) >> } >> vbasedev->bcontainer->ops->detach_device(vbasedev); >> } >> + >> +void host_iommu_device_create(VFIODevice *vbasedev) >> +{ >> + const VFIOIOMMUClass *ops = vbasedev->bcontainer->ops; >> + >> + assert(ops->host_iommu_device_create); >> + ops->host_iommu_device_create(vbasedev); >> +}