[Intel-gfx] [PATCH v4 09/19] vfio/pci: Accept device fd for hot reset

[Liu, Yi L]

> From: Tian, Kevin <kevin.tian at intel.com>
> Sent: Wednesday, February 22, 2023 3:26 PM
> 
> > From: Liu, Yi L <yi.l.liu at intel.com>
> > Sent: Tuesday, February 21, 2023 11:48 AM
> >
> >  	/*
> >  	 * We can't let userspace give us an arbitrarily large buffer to copy,
> > -	 * so verify how many we think there could be.  Note groups can
> have
> > -	 * multiple devices so one group per device is the max.
> > +	 * so verify how many we think there could be.  Note user may
> > provide
> > +	 * a set of groups, group can have multiple devices so one group per
> > +	 * device is the max.
> 
> well this change doesn't include cdev

For cdev, it should be the number of devices. 😊

> 
> > @@ -1320,7 +1321,7 @@ static int vfio_pci_ioctl_pci_hot_reset(struct
> > vfio_pci_core_device *vdev,
> >  		}
> >
> >  		/* Ensure the FD is a vfio FD.*/
> > -		if (!vfio_file_is_valid(file)) {
> > +		if (!vfio_file_is_device_opened(file)) {
> >  			fput(file);
> >  			ret = -EINVAL;
> >  			break;
> 
> that function is not just for checking device.
> 
> Probably rename it to vfio_file_is_reset_valid().

How about vfio_file_is_resettable()?

> btw this patch is insufficient to handle device fd. The current logic
> requires every device in the dev_set covered by provided fd's:
> 
> static bool vfio_dev_in_groups(struct vfio_pci_core_device *vdev,
>                                struct vfio_pci_group_info *groups)
> {
>  	unsigned int i;
> 
> 	for (i = 0; i < groups->count; i++)
> 		if (vfio_file_has_dev(groups->files[i], &vdev->vdev))
> 			return true;
> 	return false;
> }
> 
> Presumably when cdev fd is provided above should compare iommu
> group of the fd and that of the vdev. Otherwise it expects the user
> to have full access to every device in the set which is impractical.

Yes. This can be done by checking the device->vfio_group->iommu_group.
But group code may be compiled out eventually. If CONFIG_VFIO_GROUP==n.
needs to use iommu_group_get() to get iommu_group and compare.

btw. I have a doubt about if it is possible that the iommu_group
can disappear during the reset. If yes, maybe better store iommu_group
in vfio_device in the vfio_register_group_dev() path if CONFIG_VFIO_GROUP
is not enabled.

Regards,
Yi Liu