> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Wednesday, February 15, 2023 8:39 PM
>
> On Tue, Feb 14, 2023 at 02:02:37AM +0000, Liu, Yi L wrote:
> > > From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > > Sent: Tuesday, February 14, 2023 7:44 AM
> > >
> > > On Mon, Feb 13, 2023 at 07:13:36AM -0800, Yi Liu wrote:
> > > > +static struct vfio_device *vfio_device_from_file(struct file *file)
> > > > +{
> > > > + struct vfio_device_file *df = file->private_data;
> > > > +
> > > > + if (file->f_op != &vfio_device_fops)
> > > > + return NULL;
> > > > + return df->device;
> > > > +}
> > > > +
> > > > /**
> > > > * vfio_file_is_valid - True if the file is usable with VFIO APIS
> > > > * @file: VFIO group file or VFIO device file
> > > > */
> > > > bool vfio_file_is_valid(struct file *file)
> > > > {
> > > > - return vfio_group_from_file(file);
> > > > + return vfio_group_from_file(file) ||
> > > > + vfio_device_from_file(file);
> > > > }
> > > > EXPORT_SYMBOL_GPL(vfio_file_is_valid);
> > >
> > > This can only succeed on a device cdev that has been fully opened.
> >
> > Actually, we cannot. This is used in the kvm-vfio code to see if the
> > user-provided fd is vfio fds in the SET_KVM path. And we don't
> > have the device cdev fully opened until BIND_IOMMUFD. But we do
> > need to invoke SET_KVM before issuing BIND_IOMMUFD as the device
> > open needs kvm pointer. So if we cannot apply fully opened limit to this
> > interface. Maybe an updated function comment is needed.
>
> This also seems sketchy, KVM is using the VFIO fd as a "proof" to
> enable the wbinvd stuff. A half opened cdev should not be used as that
> proof.
>From this angle, the group path seems has the same concern. Device is not
opened until VFIO_GROUP_GET_DEVICE_FD. GROUP_ADD happens before
VFIO_GROUP_GET_DEVICE_FD.
But group path has one advantage, which make it ok. Group can only be
opened by one application. So once it is opened, the devices within the
group are somehow obtained by the application until group fd close.
Cdev path may also do similar thing. E.g. one cdev can be opened by
one application. Then it should be ok to use the cdev fd as proof to
enable the wbinvd stuff even the device is not opened yet..
> Regardless it needs to be fixed for the pci usage.
For the pci usage, does my below reply make any sense?
https://lore.kernel.org/kvm/DS0PR11MB7529CFCE99E8A77AAC76DC7CC3A39@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/T/#m7c00ae5dcae15f42b6dc0b3767c7037b99f53a56
Thanks,
Yi Liu
