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@xxxxxxxxx> > > > > 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@xxxxxxxxx> > > > > --- > > > > 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