Re: [PATCH v2 02/27] vfio: Introduce base object for VFIOContainer and targetted interface

[Cédric Le Goater]

Hello Zhenzhong,

> > > you would need to do that for all types for vfio devices, ap, ccw,
> > > platform. Looks heavy to me. Why would we need to use a different
> > > vfio-pci-* device while we could switch the iommu backend according to
> > > the "iommufd" prop presence. The initial discussion was about QOMyfying
> > > the container instead.
> >
> > yes.
> >
> > I took a closer look at the first part which adds the backend ops,
> > including patch 19 adding the iommufd backend, not saying that I have
> > identified all the dark corners.
> >
> > A QOM-like design would have introduced a VFIOLegacyContainer,
> > inheriting from VFIOContainer (same for VFIOIOMMUFDContainer) with a
> > VFIOContainerClass to implement the specific backend ops.
> > VFIOspaprContainer would have made sense also.
> >
> > But QOM doesn't seem well adapted for the current needs. So let's try
> > a simpler approach. It seems that VFIOIOMMUBackendOpsClass is
> > useless. IMO, it could be a callbacks structure like we have for
> > memory regions initialized with vfio_container_init(). This would
> > remove some noise around the QOM typeinfo definitions.
>
> Yes, good suggestion, will do.
Thanks,

> > 'struct vfio_iommu_ops' reads/sounds like a good name.

This should be read as VFIOIOMMUOps :)

> > Can we try that in a v3 ? It should not be such an earthquake.
>
> Sure.
>
> > spapr has some singularities which would be good to isolate in a
> > vfio_iommu_spapr_ops to remove all the VFIO_SPAPR_TCE_*_IOMMU code in
> > container.c. vfio_legacy_{add,del}_section_window are SPAPR specific.
>
> Yes, let me try it.

There is a large amount of spapr code in container.c. If it could
be moved under its own backend (and in hw/ppc/spapr_pci_vfio.c),
we would have two backends : spapr and legacy. We could possibly
merge this first part in 8.2.

I have access to a PPC P9 system. I can make sure we are not breaking
support.

C.



> Thanks
> Zhenzhong
> > FYI, I did some adjustements bc of the recent introduction of iova_ranges
> > in my branch :
> >
> >   https://github.com/legoater/qemu/commits/vfio-8.2
> >
> > Thanks,
> >
> > C.