Re: [Intel-gfx] [PATCH v7 8/9] vfio/pci: Extend VFIO_DEVICE_GET_PCI_HOT_RESET_INFO for vfio device cdev

Liu, Yi L
2023-06-09T00:04:24Z

> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: Friday, June 9, 2023 6:27 AM
>
> On Fri, 2 Jun 2023 05:15:14 -0700
> Yi Liu <yi.l.liu@intel.com> wrote:
>
> > This allows VFIO_DEVICE_GET_PCI_HOT_RESET_INFO ioctl use the iommufd_ctx
> > of the cdev device to check the ownership of the other affected devices.
> >
> > When VFIO_DEVICE_GET_PCI_HOT_RESET_INFO is called on an IOMMUFD managed
> > device, the new flag VFIO_PCI_HOT_RESET_FLAG_DEV_ID is reported to indicate
> > the values returned are IOMMUFD devids rather than group IDs as used when
> > accessing vfio devices through the conventional vfio group interface.
> > Additionally the flag VFIO_PCI_HOT_RESET_FLAG_DEV_ID_OWNED will be reported
> > in this mode if all of the devices affected by the hot-reset are owned by
> > either virtue of being directly bound to the same iommufd context as the
> > calling device, or implicitly owned via a shared IOMMU group.
> >
> > Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> > Suggested-by: Alex Williamson <alex.williamson@redhat.com>
> > Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> > ---
> > drivers/vfio/iommufd.c | 49 +++++++++++++++++++++++++++++++
> > drivers/vfio/pci/vfio_pci_core.c | 47 +++++++++++++++++++++++++-----
> > include/linux/vfio.h | 16 ++++++++++
> > include/uapi/linux/vfio.h | 50 +++++++++++++++++++++++++++++++-
> > 4 files changed, 154 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c
> > index 88b00c501015..a04f3a493437 100644
> > --- a/drivers/vfio/iommufd.c
> > +++ b/drivers/vfio/iommufd.c
> > @@ -66,6 +66,55 @@ void vfio_iommufd_unbind(struct vfio_device *vdev)
> > vdev->ops->unbind_iommufd(vdev);
> > }
> >
> > +struct iommufd_ctx *vfio_iommufd_device_ictx(struct vfio_device *vdev)
> > +{
> > + if (vdev->iommufd_device)
> > + return iommufd_device_to_ictx(vdev->iommufd_device);
> > + return NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(vfio_iommufd_device_ictx);
> > +
> > +static int vfio_iommufd_device_id(struct vfio_device *vdev)
> > +{
> > + if (vdev->iommufd_device)
> > + return iommufd_device_to_id(vdev->iommufd_device);
> > + return -EINVAL;
>
> If this is actually reachable, it allows us to return -EINVAL as a
> devid in the reset-info ioctl, which is not a defined value. Should
> this return VFIO_PCI_DEVID_NOT_OWNED or do you want to catch the errno
> value in the caller? Thanks,

This error can be reached if user invokes _INFO or HOT_RESET on an emulated
device or a physical device that has not been bound to iommufd. Both should
be considered as not-owned. So return VFIO_PCI_DEVID_NOT_OWNED makes
more sense.

Regards,
Yi Liu