[Intel-gfx] [PATCH 3/7] vfio/pci: Allow passing zero-length fd array in VFIO_DEVICE_PCI_HOT_RESET

[Liu, Yi L]

> From: Jason Gunthorpe <jgg at nvidia.com>
> Sent: Tuesday, March 21, 2023 3:03 AM
> On Thu, Mar 16, 2023 at 05:41:52AM -0700, Yi Liu wrote:
> > as an alternative method for ownership check when iommufd is used. In
> > this case all opened devices in the affected dev_set are verified to
> > be bound to a same valid iommufd value to allow reset. It's 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.
> >
> > a device in noiommu mode doesn't have a valid iommufd, so this method
> > should not be used in a dev_set which contains multiple devices and one
> > of them is in noiommu. The only allowed noiommu scenario is that the
> > calling device is noiommu and it's in a singleton dev_set.
> >
> > Suggested-by: Jason Gunthorpe <jgg at nvidia.com>
> > Signed-off-by: Jason Gunthorpe <jgg at nvidia.com>
> > Signed-off-by: Yi Liu <yi.l.liu at intel.com>
> > ---
> >  drivers/iommu/iommufd/device.c   |  6 ++
> >  drivers/vfio/iommufd.c           |  8 +++
> >  drivers/vfio/pci/vfio_pci_core.c | 94 +++++++++++++++++++++++---------
> >  include/linux/iommufd.h          |  1 +
> >  include/linux/vfio.h             |  3 +
> >  include/uapi/linux/vfio.h        |  9 ++-
> >  6 files changed, 93 insertions(+), 28 deletions(-)
> 
> This could probably be split to two or three patches
> 
> > -static int vfio_pci_ioctl_pci_hot_reset(struct vfio_pci_core_device *vdev,
> > -					struct vfio_pci_hot_reset __user
> *arg)
> > +static int
> > +vfio_pci_ioctl_pci_hot_reset_groups(struct vfio_pci_core_device *vdev,
> > +				    struct vfio_pci_hot_reset *hdr,
> > +				    bool slot,
> > +				    struct vfio_pci_hot_reset __user *arg)
> >  {
> 
> At least this mechanical re-organization should be in its own patch

Sure. 

> > diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> > index 3188d8a374bd..f0a5ff317b20 100644
> > --- a/include/linux/vfio.h
> > +++ b/include/linux/vfio.h
> > @@ -116,6 +116,7 @@ struct vfio_device_ops {
> >  int vfio_iommufd_physical_bind(struct vfio_device *vdev,
> >  			       struct iommufd_ctx *ictx, u32 *out_device_id);
> >  void vfio_iommufd_physical_unbind(struct vfio_device *vdev);
> > +struct iommufd_ctx *vfio_iommufd_physical_ictx(struct vfio_device
> *vdev);
> >  int vfio_iommufd_physical_attach_ioas(struct vfio_device *vdev, u32
> *pt_id);
> >  int vfio_iommufd_emulated_bind(struct vfio_device *vdev,
> >  			       struct iommufd_ctx *ictx, u32 *out_device_id);
> > @@ -127,6 +128,8 @@ int vfio_iommufd_emulated_attach_ioas(struct
> vfio_device *vdev, u32 *pt_id);
> >  		  u32 *out_device_id)) NULL)
> >  #define vfio_iommufd_physical_unbind \
> >  	((void (*)(struct vfio_device *vdev)) NULL)
> > +#define vfio_iommufd_physical_ictx \
> > +	((struct iommufd_ctx * (*)(struct vfio_device *vdev)) NULL)
> 
> ??
> 
> This should just be a normal static inline?? It won't compile like
> this.

Yes. in the case of !CONFIG_IOMMUFD, just return NULL.

> 
> It would also be a nice touch to include a new vfio_pci_hot_reset_info
> that returns the dev_id's of the other devices in the reset group
> instead of a BDF. It would be alot easier for userspace to work with.

Yeah, just as we are chatting in another thread. Btw. Do we expect the
new _INFO ioctl that return dev_ids work for the legacy group path under
compat mode? If no, then I may need to organize this series after cdev
series since dev_id is returned to user in cdev series.

Regards,
Yi Liu