> From: Jason Gunthorpe <jgg@nvidia.com>
> 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 <yi.l.liu@intel.com>
> > ---
> > 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