RE: [PATCH v6 12/24] vfio/pci: Allow passing zero-length fd array in VFIO_DEVICE_PCI_HOT_RESET

Liu, Yi L
2023-03-10T06:04:13Z

> From: Tian, Kevin <kevin.tian@intel.com>
> Sent: Friday, March 10, 2023 1:31 PM
>
> > From: Yi Liu
> > Sent: Wednesday, March 8, 2023 9:29 PM
> >
> > This is another method to issue PCI hot reset for the users that bounds
> > device to a positive iommufd value. In such case, iommufd is a proof of
> > device ownership. By passing a zero-length fd array, user indicates kernel
> > to do ownership check with the bound iommufd. All the opened devices
> > within
> > the affected dev_set should have been bound to the same iommufd. This
> is
> > simpler and faster as user does not need to pass a set of fds and kernel
> > no need to search the device within the given fds.
> >
> > Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> > Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>
> I think you also need a s-o-b from Jason since he wrote most of the
> code here.

Yes, it is. I'll add it if no objection from Jason.

> > +struct iommufd_ctx *vfio_iommufd_physical_ctx(struct vfio_device
> *vdev)
> > +{
> > + /* Only serve for physical device */
> > + if (!vdev->iommufd_device)
> > + return NULL;
>
> pointless comment

Will remove it.

> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -682,6 +682,11 @@ struct vfio_pci_hot_reset_info {
> > * The ownership can be proved by:
> > * - An array of group fds
> > * - An array of device fds
> > + * - A zero-length array
> > + *
> > + * In the last case all affected devices which are opened by this user
> > + * must have been bound to a same iommufd_ctx. This approach is only
> > + * available for devices bound to positive iommufd.
>
> As we chatted before I still think the last sentence is pointless. If a device
> is bound to negative iommufd value (i.e. noiommu) it doesn't have a
> valid iommufd_ctx so won't meet "bound to a same iommufd_ctx".

Yes, it is. But iommufd_ctx is more a kernel thing, userspace may just
know whether it has bound a positive iommufd or a negative iommufd
to the device. So positive iommufd may be more straightforward to
userspace programmers. 😊 If it's really redundant, I can remove it
as well.

Regards,
Yi Liu