On Fri, Dec 02, 2022 at 02:37:18PM +0800, Yi Liu wrote:
> On 2022/12/2 14:15, Tian, Kevin wrote:
> > > From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> > > Sent: Thursday, December 1, 2022 10:56 PM
> > >
> > > This refactor makes the vfio_device_open() to accept device, iommufd_ctx
> > > pointer and kvm pointer. These parameters are generic items in today's
> > > group path and furute device cdev path. Caller of vfio_device_open() should
> > > take care the necessary protections. e.g. the current group path need to
> > > hold the group_lock to ensure the iommufd_ctx and kvm pointer are valid.
> > >
> > > This refactor also wraps the group spefcific codes in the device open and
> > > close paths to be paired helpers like:
> > >
> > > - vfio_device_group_open/close(): call vfio_device_open/close()
> > > - vfio_device_group_use/unuse_iommu(): call iommufd or container
> > > use/unuse
> >
> > this pair is container specific. iommufd vs. container is selected
> > in vfio_device_first_open().
>
> got it.
>
> > > @@ -765,77 +796,56 @@ static int vfio_device_first_open(struct vfio_device
> > > *device)
> > > if (!try_module_get(device->dev->driver->owner))
> > > return -ENODEV;
> > >
> > > - /*
> > > - * Here we pass the KVM pointer with the group under the lock. If
> > > the
> > > - * device driver will use it, it must obtain a reference and release it
> > > - * during close_device.
> > > - */
> > > - mutex_lock(&device->group->group_lock);
> > > - if (!vfio_group_has_iommu(device->group)) {
> > > - ret = -EINVAL;
> > > + if (iommufd)
> > > + ret = vfio_iommufd_bind(device, iommufd);
> >
> > This probably should be renamed to vfio_device_iommufd_bind().
>
> I'd like to see if Jason wants to modify it or not as it is
> introduced in vfio compat series.
>
> https://lore.kernel.org/kvm/6-v4-42cd2eb0e3eb+335a-vfio_iommufd_jgg@xxxxxxxxxx/
Ah, it is a bit late, if we want to change it please make a patch
> >
> > > + else
> > > + ret = vfio_device_group_use_iommu(device);
> >
> > what about vfio_device_container_bind()?
>
> maybe use_iommu seems suit more. bind is more likely a kind of
> associating something with something. but this helper is more kind
> of using the container. so I chose use_iommu. But I see the value
> of using bind, it would make the two branches aligned.
I prefer use iommu, "bind" is so vauge
Jason
