On Tue, Feb 28, 2023 at 12:42:31PM +0000, Liu, Yi L wrote: > > From: Jason Gunthorpe <jgg@xxxxxxxxxx> > > 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