Re: [Intel-gfx] [PATCH v5 09/10] vfio/pci: Extend VFIO_DEVICE_GET_PCI_HOT_RESET_INFO for vfio device cdev

Alex Williamson
2023-05-18T19:53:56Z

On Thu, 18 May 2023 13:31:47 +0000
"Liu, Yi L" <yi.l.liu@intel.com> wrote:

> > From: Liu, Yi L <yi.l.liu@intel.com>
> > Sent: Thursday, May 18, 2023 9:22 PM
> >
> > > From: Alex Williamson <alex.williamson@redhat.com>
> > > Sent: Thursday, May 18, 2023 6:02 AM
> > >
> > > On Sat, 13 May 2023 06:21:35 -0700
> > > Yi Liu <yi.l.liu@intel.com> wrote:
>
> > >
> > > static int vfio_hot_reset_devid(struct vfio_device *vdev,
> > > struct iommufd_ctx *iommufd_ctx)
> > > {
> > > struct iommu_group *group;
> > > int devid;
> > >
> > > if (!vdev)
> > > return VFIO_PCI_DEVID_NOT_OWNED;
> > >
> > > if (vfio_iommufd_physical_ictx(vdev) == iommufd_ctx)
> > > return vfio_iommufd_physical_devid(vdev);
>
> Do we need to check the return of this helper? It returns -EINVAL
> when iommufd_access and iommufd_device are both null. Though
> not possible in this path. Is a WARN_ON needed or not?

I also came to the conclusion that it wasn't possible while reviewing
the code. I wouldn't got to extreme measures to introduce paranoia
checks for impossible conditions. Thanks,

Alex

> > >
> > > group = iommu_group_get(vdev->dev);
> > > if (!group)
> > > return VFIO_PCI_DEVID_NOT_OWNED;
> > >
> > > if (iommufd_ctx_has_group(iommufd_ctx, group))
> > > devid = VFIO_PCI_DEVID_OWNED;
> > >
> > > iommu_group_put(group);
> > >
> > > return devid;
> > > }