> From: Liu, Yi L <yi.l....@intel.com>
> Sent: Wednesday, April 26, 2023 10:54 PM
> +
> +/*
> + * Check if a given iommu_group has been bound to an iommufd within a
> + * devset. Returns true if there is device in the devset which is in
> + * the input iommu_group and meanwhile bound to the input iommufd.
> + * Otherwise, returns false.
> + */
> +static bool
> +vfio_devset_iommufd_has_group(struct vfio_device_set *dev_set,
> + struct iommufd_ctx *iommufd,
> + struct iommu_group *iommu_group)
this function name reads confusing. Probably just use
vfio_dev_in_iommufd_ctx() from next patch which sounds clearer
and open-code this into that helper.
>
> - fill->devices[fill->cur].group_id = iommu_group_id(iommu_group);
> + if (fill->devid) {
> + struct vfio_device *vdev;
> +
> + /*
> + * Report devid for the affected devices:
> + * - valid devid > 0 for the devices that are bound with
> + * the iommufd of the calling device.
">0" is inaccurate. All values except 0 and -1 are valid.
> + * - devid == 0 for the devices that have not been opened
> + * but have same group with one of the devices bound to
> + * the iommufd of the calling device.
> + * - devid == -1 for others, and clear resettable flag.
> + */
> + vdev = vfio_pci_find_device_in_devset(dev_set, pdev);
> + if (vdev && iommufd == vfio_iommufd_physical_ictx(vdev)) {
> + fill->devices[fill->cur].dev_id =
> +
> vfio_iommufd_physical_devid(vdev);
> + if (unlikely(!fill->devices[fill->cur].dev_id))
> + return -EINVAL;
> + } else if (vfio_devset_iommufd_has_group(dev_set, iommufd,
> + iommu_group)) {
> + fill->devices[fill->cur].dev_id =
> VFIO_PCI_DEVID_NONBLOCKING;
> + } else {
> + fill->devices[fill->cur].dev_id =
> VFIO_PCI_DEVID_BLOCKING;
> + fill->resettable = false;
NONBLOCKING/BLOCKING is unclear in the context of devid.
since 0 and -1 are talked in all other places and above comment probably
it's clear enough to use plain values?
> + }
> + } else {
> + fill->devices[fill->cur].group_id =
> iommu_group_id(iommu_group);
> + }
move iommu_group_get/put() into 'else' branch.
when calling vfio_dev_in_iommufd_ctx() in the devid branch the group
will be only used in 'else' in this function.
