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, requiring additional exported functions from vfio for
each new user of it. Why is this better? Thanks,
Alex
