> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> 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
