Hi Cédric, >-----Original Message----- >From: Cédric Le Goater <c...@redhat.com> >Sent: Friday, June 14, 2024 6:05 PM >To: eric.au...@redhat.com; eric.auger....@gmail.com; qemu- >de...@nongnu.org; qemu-...@nongnu.org; m...@redhat.com; jean- >phili...@linaro.org; peter.mayd...@linaro.org; yangh...@redhat.com; Duan, >Zhenzhong <zhenzhong.d...@intel.com> >Cc: alex.william...@redhat.com; jasow...@redhat.com; >pbonz...@redhat.com; berra...@redhat.com >Subject: Re: [PATCH v3 1/7] HostIOMMUDevice: Store the VFIO/VDPA agent > > >>> Talking of which, why are we passing a 'VFIODevice *' parameter to >>> HostIOMMUDeviceClass::realize ? I don't see a good reason >>> >>> I think a 'VFIOContainerBase *' would be more appropriate since >>> 'HostIOMMUDevice' represents a device on the host which is common >>> to all VFIO devices. >>> >>> In that case, HostIOMMUDevice::agent wouldn't need to be opaque >>> anymore. It could simply be a 'VFIOContainerBase *' and >>> hiod_legacy_vfio_get_iova_ranges() in patch 3 would grab the >>> 'iova_ranges' from the 'VFIOContainerBase *' directly. >>> >>> This means some rework : >>> >>> * vfio_device_get_aw_bits() would use a 'VFIOContainerBase *' instead. >>> * HostIOMMUDevice::name would be removed. This is just for error >>> messages. >>> * hiod_iommufd_vfio_realize() would use VFIOIOMMUFDContainer::be. >>> >>> That said, I think we need the QOMification changes first. >> >> OK I need to review your series first. At the moment I have just >> addressed Zhenzhong's comment in v4, just sent. > >Yep. Just take a look at mine. If both of you agree with above >proposal, I can care of it and resend all 3. It's a small change.
I would suggest using opaque pointer and VFIODevice for two reasons, 1. in nesting series vIOMMU needs to attach/detaching hwpt which is VFIODevice operations. See https://github.com/yiliu1765/qemu/commit/3ca559d35adc9840555e361a56708af4c6338b3d 2. If we plan to support VDPA Device in future, the opaque pointer can also point to an VDPADevice structure. Thanks Zhenzhong
