Hi Alex, Jason,
> From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> Sent: Tuesday, February 14, 2023 3:19 PM
>
> > From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> > Sent: Tuesday, February 14, 2023 10:03 AM
> >
> > > 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.
> >
> > " vfio_file_is_valid - True if the file is vfio files (group or device)"
>
> I guess your point is this is also called in the pci hot reset path. And
> in the reset path, the device referred by the device fd should be fully
> opened. vfio_file_is_valid() only checks f_ops, which is not enough to
> show the device is fully-opened for cdev fd. However, view the high-level
> flow, for cdev fd, the device access (neither VFIO_DEVICE_PCI_HOT_RESET
> nor VFIO_DEVICE_GET_PCI_HOT_RESET_INFO) is not allowed until the
> device is fully-opened (done in the bind_iommufd). So if the
> VFIO_DEVICE_PCI_HOT_RESET path goes such far to call vfio_file_is_valid(),
> the device should have been fully-opened.
One more thinking on this. For a single device, my above reply is true.
The device should have been fully-opened when its GET_PCI_HOT_RESET_INFO
and HOT_RESET path have been unblocked. However, when there are
multiple devices that have been affected by the hotreset. User may only
have one device that is fully opened while others are not yet. In such case,
existing vfio_file_is_valid() is not enough. Shall we have another API for
this purpose? E.g. if it's cdev fd, then the new API return true only when
the device is fully opened. Any suggestion here?
Regards,
Yi Liu
