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@xxxxxxxxxx> > Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx> > Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx> > --- > 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 > 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. 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. Otherwise this looks basically OK. Jason