[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 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()?

> > @@ -96,6 +104,18 @@ int vfio_iommufd_physical_attach_ioas(struct
> vfio_device *vdev, u32 *pt_id)
> >  }
> >  EXPORT_SYMBOL_GPL(vfio_iommufd_physical_attach_ioas);
> >
> > +void vfio_iommufd_physical_detach_ioas(struct vfio_device *vdev)
> > +{
> > +	lockdep_assert_held(&vdev->dev_set->lock);
> > +
> > +	if (!vdev->iommufd_device || !vdev->iommufd_attached)
> > +		return;
> 
> Same

Sure. Will apply same warn when above comment is aligned.

Regards,
Yi Liu