On Tue, Dec 19, 2017 at 03:59:59PM +0100, Paolo Bonzini wrote:
> On 19/12/2017 15:09, Alex Williamson wrote:
> > On Tue, 19 Dec 2017 12:12:35 +0100
> > Paolo Bonzini <pbonz...@redhat.com> wrote:
> >
> >> On 12/12/2017 06:46, Alex Williamson wrote:
> >>>> +enum IOMMUMemoryRegionAttr {
> >>>> + IOMMU_ATTR_KVM_FD
> >>>
> >>> You're generalizing the wrong thing here, this is specifically a
> >>> SPAPR_TCE_FD, call it that.
> >>
> >> ... and you're not even implementing set_attr, so let's drop it.
> >>
> >> My suggestion is to add a function in hw/vfio:
> >>
> >> int vfio_container_attach_kvm_spapr_tce(VFIOContainer *cont,
> >> int tablefd);
> >>
> >> and an IOMMUMemoryRegionClass member:
> >>
> >> int (*set_vfio_container_attrs)(IOMMUMemoryRegion *iommu,
> >> VFIOContainer *cont)
> >>
> >> Then your implementation for the latter is as simple as this:
> >>
> >> if (!kvm_enabled() || !kvmppc_has_cap_spapr_vfio()) {
> >> sPAPRTCETable *tcet = container_of(iommu, sPAPRTCETable, iommu);
> >> return vfio_container_attach_kvm_spapr_tce(cont, tcet->fd);
> >> }
> >
> > Ugh, exactly the sort of interface I've been trying to avoid, vfio
> > specific callbacks on common data structures handing out vfio private
> > data pointers,
>
> True, VFIOContainer* is private, but in those declarations it's also opaque.
>
> The VFIO container is the representation of the IOMMU, so it makes sense
> to me to have a function to set it up according to QEMU's IOMMU object.
> I don't think we will be introducing another object soon that is similar
> to the VFIO container.
>
> > requiring additional exported functions from vfio for
> > each new user of it. Why is this better?
>
> I understand that you don't like having many exported functions, but you
> are just pushing the problem on the memory.h side where you'd get many
> attribute enums.
It's more than just enums, doing it the other way around is putting
fairly intimate knowledge of a specific guest IOMMU workings into the
VFIO code.
Fundamentally this *requires* linking vfio knowledge to guest iommu
(kvm) knowledge, so some cross linkage we'd usually want to avoid is
inevitable. I don't see that there's a strong argument for whether we
put the bit of vfio knowledge into the spapr viommu or the bit of
spapr viommu knowledge into vfio.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
