> From: Jason Gunthorpe > > Sent: Tuesday, September 21, 2021 11:57 PM > > On Sun, Sep 19, 2021 at 02:38:30PM +0800, Liu Yi L wrote: > > This patch introduces a new interface (/dev/vfio/devices/$DEVICE) for > > userspace to directly open a vfio device w/o relying on container/group > > (/dev/vfio/$GROUP). Anything related to group is now hidden behind > > iommufd (more specifically in iommu core by this RFC) in a device-centric > > manner. > > > > In case a device is exposed in both legacy and new interfaces (see next > > patch for how to decide it), this patch also ensures that when the device > > is already opened via one interface then the other one must be blocked. > > > > Signed-off-by: Liu Yi L > > > --- > > drivers/vfio/vfio.c | 228 +++++++++++++++++++++++++++++++++++++++--- > - > > include/linux/vfio.h | 2 + > > 2 files changed, 213 insertions(+), 17 deletions(-) > > > +static int vfio_init_device_class(void) > > +{ > > + int ret; > > + > > + mutex_init(&vfio.device_lock); > > + idr_init(&vfio.device_idr); > > + > > + /* /dev/vfio/devices/$DEVICE */ > > + vfio.device_class = class_create(THIS_MODULE, "vfio-device"); > > + if (IS_ERR(vfio.device_class)) > > + return PTR_ERR(vfio.device_class); > > + > > + vfio.device_class->devnode = vfio_device_devnode; > > + > > + ret = alloc_chrdev_region(&vfio.device_devt, 0, MINORMASK + 1, > "vfio-device"); > > + if (ret) > > + goto err_alloc_chrdev; > > + > > + cdev_init(&vfio.device_cdev, &vfio_device_fops); > > + ret = cdev_add(&vfio.device_cdev, vfio.device_devt, MINORMASK + > 1); > > + if (ret) > > + goto err_cdev_add; > > Huh? This is not how cdevs are used. This patch needs rewriting. > > The struct vfio_device should gain a 'struct device' and 'struct cdev' > as non-pointer members > > vfio register path should end up doing cdev_device_add() for each > vfio_device > > vfio_unregister path should do cdev_device_del() > > No idr should be needed, an ida is used to allocate minor numbers > > The struct device release function should trigger a kfree which > requires some reworking of the callers thanks for the guiding. will also refer to your vfio_group_cdev series. Need to double confirm here. Not quite following on the kfree. Is this kfree to free the vfio_device structure? But now the vfio_device pointer is provided by callers (e.g. vfio-pci). Do you want to let vfio core allocate the vfio_device struct and return the pointer to callers? Thanks, Yi Liu > vfio_init_group_dev() should do a device_initialize() > vfio_uninit_group_dev() should do a device_put() > > The opened atomic is aweful. A newly created fd should start in a > state where it has a disabled fops > > The only thing the disabled fops can do is register the device to the > iommu fd. When successfully registered the device gets the normal fops. > > The registration steps should be done under a normal lock inside the > vfio_device. If a vfio_device is already registered then further > registration should fail. > > Getting the device fd via the group fd triggers the same sequence as > above. > > Jason