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

[Tian, Kevin]

> From: Liu, Yi L <yi.l.liu at 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.