[Intel-gfx] [PATCH v5 11/19] vfio-iommufd: Add detach_ioas support for physical VFIO devices

[Liu, Yi L]

> From: Jason Gunthorpe <jgg at nvidia.com>
> Sent: Tuesday, February 28, 2023 8:33 PM
>
> On Tue, Feb 28, 2023 at 02:57:42AM +0000, Liu, Yi L wrote:
> > > From: Jason Gunthorpe <jgg at nvidia.com>
> > > Sent: Tuesday, February 28, 2023 2:45 AM
> > >
> > > On Mon, Feb 27, 2023 at 03:11:27AM -0800, Yi Liu wrote:
> > > > diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-
> > > mc/vfio_fsl_mc.c
> > > > index c89a047a4cd8..d540cf683d93 100644
> > > > --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> > > > +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> > > > @@ -594,6 +594,7 @@ static const struct vfio_device_ops
> > > vfio_fsl_mc_ops = {
> > > >  	.bind_iommufd	= vfio_iommufd_physical_bind,
> > > >  	.unbind_iommufd	= vfio_iommufd_physical_unbind,
> > > >  	.attach_ioas	= vfio_iommufd_physical_attach_ioas,
> > > > +	.detach_ioas	= vfio_iommufd_physical_detach_ioas,
> > > >  };
> > > >
> > > >  static struct fsl_mc_driver vfio_fsl_mc_driver = {
> > > > diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c
> > > > index beef6ca21107..bfaa9876499b 100644
> > > > --- a/drivers/vfio/iommufd.c
> > > > +++ b/drivers/vfio/iommufd.c
> > > > @@ -88,6 +88,14 @@ int vfio_iommufd_physical_attach_ioas(struct
> > > vfio_device *vdev, u32 *pt_id)
> > > >  {
> > > >  	int rc;
> > > >
> > > > +	lockdep_assert_held(&vdev->dev_set->lock);
> > > > +
> > > > +	if (!vdev->iommufd_device)
> > > > +		return -EINVAL;
> > >
> > > This should be a WARN_ON. The vdev->iommufd_ctx should be NULL if it
> > > hasn't been bound, and it can't be bound unless the
> > > iommufd_device/attach was created.
> >
> > sure. But it is a user-triggerable warn. If userspace triggers it on
> > purpose, will it be a bad thing for kernel? Maybe use
> > dev_warn_ratelimited()?
> 
> How can it be user triggerable? You shouldn't be able to reach this
> function until the device is bound because the ioctl should be after
> the is it bound check

Oh, yes. it is! ioctls are blocked until bound to iommufd.

Regards,
Yi Liu