On Tue, Jan 17, 2023 at 05:49:40AM -0800, 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 (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;
> *) vfio_unregister_group_dev() path does cdev_device_del();
>
> Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx>
> Signed-off-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
> ---
> drivers/vfio/vfio_main.c | 103 ++++++++++++++++++++++++++++++++++++---
> include/linux/vfio.h | 7 ++-
> 2 files changed, 102 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index 78725c28b933..6068ffb7c6b7 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@ -43,6 +43,9 @@
> static struct vfio {
> struct class *device_class;
> struct ida device_ida;
> +#if IS_ENABLED(CONFIG_IOMMUFD)
> + dev_t device_devt;
> +#endif
It is a bit strange to see this called CONFIG_IOMMUFD, maybe we should
have a CONFIG_VFIO_DEVICE_FILE that depends on IOMMUFD?
Please try to use a plain 'if (IS_ENABLED())' for more of these
It probably isn't worth saving a few bytes in memory to complicate all
the code, so maybe just always include things like this.
> @@ -156,7 +159,11 @@ static void vfio_device_release(struct device *dev)
> container_of(dev, struct vfio_device, device);
>
> vfio_release_device_set(device);
> +#if IS_ENABLED(CONFIG_IOMMUFD)
> + ida_free(&vfio.device_ida, MINOR(device->device.devt));
> +#else
> ida_free(&vfio.device_ida, device->index);
> +#endif
A vfio_device_get_index() would help this
Jason
