Re: [PATCH v5 17/19] vfio: Add VFIO_DEVICE_AT[DE]TACH_IOMMUFD_PT

Jason Gunthorpe
2023-02-28T12:54:04Z

On Tue, Feb 28, 2023 at 12:42:31PM +0000, Liu, Yi L wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > Sent: Tuesday, February 28, 2023 8:32 PM
> >
> > On Tue, Feb 28, 2023 at 02:51:28AM +0000, Liu, Yi L wrote:
> > > > This seems strange. no iommu mode should have a NULL dev-
> > >iommufctx.
> > > > Why do we have a df->noiommu at all?
> > >
> > > This is due to the vfio_device_first_open(). Detail as below comment (part
> > of
> > > patch 0016).
> > >
> > > + /*
> > > + * For group/container path, iommufd pointer is NULL when comes
> > > + * into this helper. Its noiommu support is handled by
> > > + * vfio_device_group_use_iommu()
> > > + *
> > > + * For iommufd compat mode, iommufd pointer here is a valid value.
> > > + * Its noiommu support is in vfio_iommufd_bind().
> > > + *
> > > + * For device cdev path, iommufd pointer here is a valid value for
> > > + * normal cases, but it is NULL if it's noiommu. Check df->noiommu
> > > + * to differentiate cdev noiommu from the group/container path
> > which
> > > + * also passes NULL iommufd pointer in. If set then do nothing.
> > > + */
> >
> > If the group is in iommufd mode then it should set this pointer too.
>
> Yes, but the key point is that both the group in legacy mode and the
> cdev path sets iommufd==NULL. And the handling for the two should
> be different. So needs this extra info to differentiate them in
> vfio_device_first_open().

Don't encode that in the iommufd pointer, it is confusing.

A null iommufd pointer and a bound df flag is sufficient to see that
it is compat mode.

Jason