| Message ID | 20240408081230.1030078-4-zhenzhong.duan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add a host IOMMU device abstraction | expand |
Hi All, >-----Original Message----- >From: Duan, Zhenzhong <zhenzhong.duan@intel.com> >Subject: [PATCH v2 03/10] backends/iommufd: Introduce abstract >HIODIOMMUFD device > >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 <clg@redhat.com> >Originally-by: Yi Liu <yi.l.liu@intel.com> >Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> >Signed-off-by: Zhenzhong Duan <zhenzhong.duan@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" >+OBJECT_DECLARE_TYPE(HIODIOMMUFD, HIODIOMMUFDClass, >HIOD_IOMMUFD) >+ >+struct HIODIOMMUFD { >+ /*< private >*/ >+ HostIOMMUDevice parent; >+ void *opaque; Please ignore above line "void *opaque;", it's totally useless, I forgot to remove it. Sorry for noise. Thanks Zhenzhong
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 <clg@redhat.com> > Originally-by: Yi Liu <yi.l.liu@intel.com> > Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@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 > +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 ? > + > +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. > -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)
>-----Original Message----- >From: Cédric Le Goater <clg@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 <clg@redhat.com> >> Originally-by: Yi Liu <yi.l.liu@intel.com> >> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@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)
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" +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; +}; + +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; +} -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)