Re: [PATCH 05/10] vfio: Move vfio_external_user_iommu_id() to vfio_file_ops

Jason Gunthorpe
2022-04-15T12:25:38Z

On Fri, Apr 15, 2022 at 09:31:25AM +0200, Christoph Hellwig wrote:
> On Thu, Apr 14, 2022 at 03:46:04PM -0300, Jason Gunthorpe wrote:
> > The only user wants to get a pointer to the struct iommu_group associated
> > with the VFIO file being used. Instead of returning the group ID then
> > searching sysfs for that string just directly return the iommu_group
> > pointer already held by the vfio_group struct.
> >
> > It already has a safe lifetime due to the struct file kref.
>
> Except for the strange function pointer indirection this does looks
> sensible to me, but I wonder if we want to do this change even more
> high level: The only thing that needs the IOMMU group is the PPC
> kvm_spapr_tce_attach_iommu_group / kvm_spapr_tce_release_vfio_group
> helpers. And these use it only to find the iommu_table_group
> structure. Why can't we just have the vfio_group point to that
> table group diretly in vfio or the vfio SPAPR backend and remove
> all the convoluted lookups?

The PPC specific iommu_table_group is PPC's "drvdata" for the common
struct iommu_group - it is obtained trivially by group->iommu_data

I think using iommu_group as the handle for it in common code is the
right thing to do.

What I don't entirely understand is what is 'tablefd' doing in all
this, or why the lookup of the kvmppc_spapr_tce_table is so
weird. PPC's unique iommu uapi is still a mystery to me.

Jason