| Message ID | 20240201072818.327930-5-zhenzhong.duan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Check and sync host IOMMU cap/ecap with vIOMMU | expand |
Hi Zhenzhong, On 2/1/24 08:28, Zhenzhong Duan wrote: > Either IOMMULegacyDevice or IOMMUFDDevice into VFIODevice, neither > both. > > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> > --- > include/hw/vfio/vfio-common.h | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h > index 8bfb9cbe94..1bbad003ee 100644 > --- a/include/hw/vfio/vfio-common.h > +++ b/include/hw/vfio/vfio-common.h > @@ -32,6 +32,7 @@ > #include "sysemu/sysemu.h" > #include "hw/vfio/vfio-container-base.h" > #include "sysemu/host_iommu_device.h" > +#include "sysemu/iommufd.h" > > #define VFIO_MSG_PREFIX "vfio %s: " > > @@ -132,8 +133,18 @@ typedef struct VFIODevice { > bool dirty_tracking; > int devid; > IOMMUFDBackend *iommufd; > + union { > + HostIOMMUDevice base_hdev; I don't think we want the base object above to be usable here Thanks Eric > + IOMMULegacyDevice legacy_dev; > + IOMMUFDDevice iommufd_dev; > + }; > } VFIODevice; > > +QEMU_BUILD_BUG_ON(offsetof(VFIODevice, legacy_dev.base) != > + offsetof(VFIODevice, base_hdev)); > +QEMU_BUILD_BUG_ON(offsetof(VFIODevice, iommufd_dev.base) != > + offsetof(VFIODevice, base_hdev)); > + > struct VFIODeviceOps { > void (*vfio_compute_needs_reset)(VFIODevice *vdev); > int (*vfio_hot_reset_multi)(VFIODevice *vdev);
On 2/1/24 08:28, Zhenzhong Duan wrote: > Either IOMMULegacyDevice or IOMMUFDDevice into VFIODevice, neither > both. > > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> > --- > include/hw/vfio/vfio-common.h | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h > index 8bfb9cbe94..1bbad003ee 100644 > --- a/include/hw/vfio/vfio-common.h > +++ b/include/hw/vfio/vfio-common.h > @@ -32,6 +32,7 @@ > #include "sysemu/sysemu.h" > #include "hw/vfio/vfio-container-base.h" > #include "sysemu/host_iommu_device.h" > +#include "sysemu/iommufd.h" > > #define VFIO_MSG_PREFIX "vfio %s: " > > @@ -132,8 +133,18 @@ typedef struct VFIODevice { > bool dirty_tracking; > int devid; > IOMMUFDBackend *iommufd; > + union { > + HostIOMMUDevice base_hdev; > + IOMMULegacyDevice legacy_dev; > + IOMMUFDDevice iommufd_dev; I think you should rather have a HostIOMMUDevice handle. host_iommu_device_init cb would allocate the right type of the derived object and you would store the base object pointer here. Eric > + }; > } VFIODevice; > > +QEMU_BUILD_BUG_ON(offsetof(VFIODevice, legacy_dev.base) != > + offsetof(VFIODevice, base_hdev)); > +QEMU_BUILD_BUG_ON(offsetof(VFIODevice, iommufd_dev.base) != > + offsetof(VFIODevice, base_hdev)); > + > struct VFIODeviceOps { > void (*vfio_compute_needs_reset)(VFIODevice *vdev); > int (*vfio_hot_reset_multi)(VFIODevice *vdev);
Hi Eric, >-----Original Message----- >From: Eric Auger <eric.auger@redhat.com> >Subject: Re: [PATCH rfcv2 04/18] vfio: Add host iommu device instance into >VFIODevice > > > >On 2/1/24 08:28, Zhenzhong Duan wrote: >> Either IOMMULegacyDevice or IOMMUFDDevice into VFIODevice, neither >> both. >> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >> --- >> include/hw/vfio/vfio-common.h | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio- >common.h >> index 8bfb9cbe94..1bbad003ee 100644 >> --- a/include/hw/vfio/vfio-common.h >> +++ b/include/hw/vfio/vfio-common.h >> @@ -32,6 +32,7 @@ >> #include "sysemu/sysemu.h" >> #include "hw/vfio/vfio-container-base.h" >> #include "sysemu/host_iommu_device.h" >> +#include "sysemu/iommufd.h" >> >> #define VFIO_MSG_PREFIX "vfio %s: " >> >> @@ -132,8 +133,18 @@ typedef struct VFIODevice { >> bool dirty_tracking; >> int devid; >> IOMMUFDBackend *iommufd; >> + union { >> + HostIOMMUDevice base_hdev; >> + IOMMULegacyDevice legacy_dev; >> + IOMMUFDDevice iommufd_dev; >I think you should rather have a HostIOMMUDevice handle. > >host_iommu_device_init cb would allocate the right type of the derived >object and you would store the base object pointer here. Sorry for late response, just back from vacation. I didn't use a HostIOMMUDevice handle but a statically allocated one Because in following patch: '[PATCH rfcv2 05/18] vfio: Remove redundant iommufd and devid elements in VFIODevice' iommufd and devid are removed from VFIODevice. I need a place to store them early in attachment. The handle is dynamically allocated after attachment and can't be used for that purpose. I'll change to use HostIOMMUDevice handle and drop patch5 in rfcv3, Let me know if you have other comments. Thanks Zhenzhong
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index 8bfb9cbe94..1bbad003ee 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -32,6 +32,7 @@ #include "sysemu/sysemu.h" #include "hw/vfio/vfio-container-base.h" #include "sysemu/host_iommu_device.h" +#include "sysemu/iommufd.h" #define VFIO_MSG_PREFIX "vfio %s: " @@ -132,8 +133,18 @@ typedef struct VFIODevice { bool dirty_tracking; int devid; IOMMUFDBackend *iommufd; + union { + HostIOMMUDevice base_hdev; + IOMMULegacyDevice legacy_dev; + IOMMUFDDevice iommufd_dev; + }; } VFIODevice; +QEMU_BUILD_BUG_ON(offsetof(VFIODevice, legacy_dev.base) != + offsetof(VFIODevice, base_hdev)); +QEMU_BUILD_BUG_ON(offsetof(VFIODevice, iommufd_dev.base) != + offsetof(VFIODevice, base_hdev)); + struct VFIODeviceOps { void (*vfio_compute_needs_reset)(VFIODevice *vdev); int (*vfio_hot_reset_multi)(VFIODevice *vdev);
Either IOMMULegacyDevice or IOMMUFDDevice into VFIODevice, neither both. Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- include/hw/vfio/vfio-common.h | 11 +++++++++++ 1 file changed, 11 insertions(+)