Re: [PATCH v2 07/11] vfio-iommufd: Support iommufd for physical VFIO devices

[Yi Liu <yi.l.liu@xxxxxxxxx>]

On 2022/11/8 14:10, Nicolin Chen wrote:
> On Mon, Nov 07, 2022 at 08:52:51PM -0400, Jason Gunthorpe wrote:
>
> > @@ -795,6 +800,10 @@ static int vfio_device_first_open(struct vfio_device *device)
> >   		ret = vfio_group_use_container(device->group);
> >   		if (ret)
> >   			goto err_module_put;
> > +	} else if (device->group->iommufd) {
> > +		ret = vfio_iommufd_bind(device, device->group->iommufd);
>
> Here we check device->group->iommufd...
>
> > +		if (ret)
> > +			goto err_module_put;
> >   	}
> >   
> >   	device->kvm = device->group->kvm;
> > @@ -812,6 +821,7 @@ static int vfio_device_first_open(struct vfio_device *device)
> >   	device->kvm = NULL;
> >   	if (device->group->container)
> >   		vfio_group_unuse_container(device->group);
> > +	vfio_iommufd_unbind(device);
>
> ...yet, missing here, which could result in kernel oops.
>
> Should probably add something similar:
> +	if (device->group->iommufd)
> +		vfio_iommufd_unbind(device);
>
> Or should check !vdev->iommufd_device inside the ->unbind.

this check was in prior version, but removed in this version. any
special reason? Jason?

> >   err_module_put:
> >   	mutex_unlock(&device->group->group_lock);
> >   	module_put(device->dev->driver->owner);
> > @@ -830,6 +840,7 @@ static void vfio_device_last_close(struct vfio_device *device)
> >   	device->kvm = NULL;
> >   	if (device->group->container)
> >   		vfio_group_unuse_container(device->group);
> > +	vfio_iommufd_unbind(device);
>
> Ditto

--
Regards,
Yi Liu