> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> 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@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
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
