On Thu, 18 May 2023 13:31:47 +0000 "Liu, Yi L" <yi.l.liu@xxxxxxxxx> wrote: > > From: Liu, Yi L <yi.l.liu@xxxxxxxxx> > > Sent: Thursday, May 18, 2023 9:22 PM > > > > > From: Alex Williamson <alex.williamson@xxxxxxxxxx> > > > Sent: Thursday, May 18, 2023 6:02 AM > > > > > > On Sat, 13 May 2023 06:21:35 -0700 > > > Yi Liu <yi.l.liu@xxxxxxxxx> 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; > > > }