Re: [RFC v2 05/11] vfio: Make vfio_device_open() group agnostic

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Dec 01, 2022 at 03:08:12PM +0800, Yi Liu wrote:
> On 2022/11/28 17:19, Yi Liu wrote:
> > On 2022/11/28 16:17, Tian, Kevin wrote:
> > > > From: Liu, Yi L <yi.l.liu@intel.com>
> > > > Sent: Thursday, November 24, 2022 8:27 PM
> > > > 
> > > > This prepares for moving group specific code to separate file.
> > > > 
> > > > Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> > > > ---
> > > >   drivers/vfio/vfio_main.c | 7 ++++---
> > > >   1 file changed, 4 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> > > > index edcfa8a61096..fcb9f778fc9b 100644
> > > > --- a/drivers/vfio/vfio_main.c
> > > > +++ b/drivers/vfio/vfio_main.c
> > > > @@ -878,9 +878,6 @@ static struct file *vfio_device_open(struct vfio_device
> > > > *device)
> > > >        */
> > > >       filep->f_mode |= (FMODE_PREAD | FMODE_PWRITE);
> > > > 
> > > > -    if (device->group->type == VFIO_NO_IOMMU)
> > > > -        dev_warn(device->dev, "vfio-noiommu device opened by
> > > > user "
> > > > -             "(%s:%d)\n", current->comm, task_pid_nr(current));
> > > >       /*
> > > >        * On success the ref of device is moved to the file and
> > > >        * put in vfio_device_fops_release()
> > > > @@ -927,6 +924,10 @@ static int vfio_group_ioctl_get_device_fd(struct
> > > > vfio_group *group,
> > > >           goto err_put_fdno;
> > > >       }
> > > > 
> > > > +    if (group->type == VFIO_NO_IOMMU)
> > > > +        dev_warn(device->dev, "vfio-noiommu device opened by
> > > > user "
> > > > +             "(%s:%d)\n", current->comm, task_pid_nr(current));
> > > > +
> > > >       fd_install(fdno, filep);
> > > >       return fdno;
> > > > 
> > > 
> > > Do we want to support no-iommu mode in future cdev path?
> > > 
> > > If yes keeping the check in vfio_device_open() makes more sense. Just
> > > replace direct device->group reference with a helper e.g.:
> > > 
> > >     vfio_device_group_noiommu()
> > 
> > I didn't see a reason cdev cannot support no-iommu mode. so a helper to
> > check noiommu is reasonable.
> 
> This check should be done after opening device and the file. Current
> vfio_device_open() opens device first and then open file. Open file is
> group path specific, not needed in future device cdev path. So if want to
> have this check in the common function, the open device and open file
> order should be swapped. However, it is not necessary here. So may just
> drop this patch and consider it in future device cdev series.

the point here was to remove the device->group touches from
vfio_main.c, which this does and seems appropraite.

cdev no-iommu mode is going to be quite different since it will work
without groups

Jason