Re: [Intel-gfx] [PATCH v12 17/24] vfio: Add cdev for vfio_device

Jason Gunthorpe
2023-06-23T15:59:02Z

On Fri, Jun 02, 2023 at 05:16:46AM -0700, Yi Liu wrote:
> This allows user to directly open a vfio device w/o using the legacy
> container/group interface, as a prerequisite for supporting new iommu
> features like nested translation.
>
> The device fd opened in this manner doesn't have the capability to access
> the device as the fops open() doesn't open the device until the successful
> BIND_IOMMUFD which be added in next patch.
>
> With this patch, devices registered to vfio core have both group and device
> interface created.
>
> - group interface : /dev/vfio/$groupID
> - device interface: /dev/vfio/devices/vfioX - normal device
> ("X" is the minor number and is unique across devices)
>
> Given a vfio device the user can identify the matching vfioX by checking
> the sysfs path of the device. Take PCI device (0000:6a:01.0) for example,
> /sys/bus/pci/devices/0000\:6a\:01.0/vfio-dev/vfio0/dev contains the
> major:minor of the matching vfioX.
>
> Userspace then opens the /dev/vfio/devices/vfioX and checks with fstat
> that the major:minor matches.
>
> The vfio_device cdev logic in this patch:
> *) __vfio_register_dev() path ends up doing cdev_device_add() for each
> vfio_device if VFIO_DEVICE_CDEV configured.
> *) vfio_unregister_group_dev() path does cdev_device_del();
>
> device interface does not support noiommu devices, noiommu users should
> use the legacy group interface.
>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Tested-by: Terrence Xu <terrence.xu@intel.com>
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Tested-by: Matthew Rosato <mjrosato@linux.ibm.com>
> Tested-by: Yanting Jiang <yanting.jiang@intel.com>
> Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
> drivers/vfio/Kconfig | 12 ++++++++
> drivers/vfio/Makefile | 1 +
> drivers/vfio/device_cdev.c | 62 ++++++++++++++++++++++++++++++++++++++
> drivers/vfio/vfio.h | 54 +++++++++++++++++++++++++++++++++
> drivers/vfio/vfio_main.c | 23 +++++++++++---
> include/linux/vfio.h | 4 +++
> 6 files changed, 151 insertions(+), 5 deletions(-)
> create mode 100644 drivers/vfio/device_cdev.c

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

> +/*
> + * device access via the fd opened by this function is blocked until
> + * .open_device() is called successfully during BIND_IOMMUFD.
> + */
> +int vfio_device_fops_cdev_open(struct inode *inode, struct file *filep)
> +{
> + struct vfio_device *device = container_of(inode->i_cdev,
> + struct vfio_device, cdev);
> + struct vfio_device_file *df;
> + int ret;
> +

Add the comment

/* Paired with the put in vfio_device_fops_release() */
> + if (!vfio_device_try_get_registration(device))
> + return -ENODEV;

> @@ -338,6 +338,12 @@ void vfio_unregister_group_dev(struct vfio_device *device)
> */
> vfio_device_group_unregister(device);
>
> + /*
> + * Balances vfio_device_add() in register path, also prevents
> + * new device opened by userspace in the cdev path.
> + */
> + vfio_device_del(device);
> +
> vfio_device_put_registration(device);
> rc = try_wait_for_completion(&device->comp);
> while (rc <= 0) {
> @@ -361,9 +367,6 @@ void vfio_unregister_group_dev(struct vfio_device *device)
> }
> }
>
> - /* Balances device_add in register path */
> - device_del(&device->device);
> -

This looks OK from what I can tell, but it might deserve its own patch
like was done for other movement.

Jason