On Tue, 13 Jun 2023 12:50:43 +0000 "Liu, Yi L" <yi.l....@intel.com> wrote:
> > From: Jason Gunthorpe <j...@nvidia.com> > > Sent: Tuesday, June 13, 2023 7:47 PM > > > > On Fri, Jun 02, 2023 at 05:15:14AM -0700, Yi Liu wrote: > > > +/* > > > + * Return devid for a device which is affected by hot-reset. > > > + * - valid devid > 0 for the device that is bound to the input > > > + * iommufd_ctx. > > > + * - devid == VFIO_PCI_DEVID_OWNED for the device that has not > > > + * been bound to any iommufd_ctx but other device within its > > > + * group has been bound to the input iommufd_ctx. > > > + * - devid == VFIO_PCI_DEVID_NOT_OWNED for others. e.g. device > > > + * is bound to other iommufd_ctx etc. > > > + */ > > > +int vfio_iommufd_device_hot_reset_devid(struct vfio_device *vdev, > > > + struct iommufd_ctx *ictx) > > > +{ > > > + struct iommu_group *group; > > > + int devid; > > > + > > > + if (vfio_iommufd_device_ictx(vdev) == ictx) > > > + return vfio_iommufd_device_id(vdev); > > > + > > > + group = iommu_group_get(vdev->dev); > > > + if (!group) > > > + return VFIO_PCI_DEVID_NOT_OWNED; > > > + > > > + if (iommufd_ctx_has_group(ictx, group)) > > > + devid = VFIO_PCI_DEVID_OWNED; > > > + else > > > + devid = VFIO_PCI_DEVID_NOT_OWNED; > > > + > > > + iommu_group_put(group); > > > + > > > + return devid; > > > +} > > > +EXPORT_SYMBOL_GPL(vfio_iommufd_device_hot_reset_devid); > > > > This function really should not be in the core iommufd.c file - it is > > a purely vfio-pci function - why did you have to place it here? > > Put it here can avoid calling iommufd_ctx_has_group() in vfio-pci, > which requires to import IOMMUFD_NS. If this reason is not so > strong I can move it back to vfio-pci code. The PCI-isms here are the name of the function and the return value, otherwise this is simply a "give me the devid for this device in this context". The function name is trivial to change and we can define the internal errno usage such that the vfio-pci-core code can interpret the correct uAPI value. For example, -EXDEV ("Cross-device link") could maybe be interpreted as owned and any other errno is not-owned, -ENODEV maybe being the default. Errno values are often contentious, are there other suggestions? Thanks, Alex