Re: [PATCH V3 03/10] iommufd: Allow binding to a noiommu device

[Jacob Pan <jacob.pan@linux.microsoft.com>]

Hi Jason,

On Thu, 9 Apr 2026 14:06:57 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Thu, Apr 02, 2026 at 10:11:39PM -0700, Jacob Pan wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > 
> > Allow iommufd to bind devices without an IOMMU (noiommu mode) by
> > creating a dummy IOMMU group for such devices and skipping hwpt
> > operations.
> > 
> > This enables noiommu devices to operate through the same iommufd
> > API as IOMMU- capable devices.
> > 
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > Signed-off-by: Jacob Pan <jacob.pan@linux.microsoft.com>
> > 
> > ---
> > v3:
> >    - handle new error cases iommufd_device_bind. (Mostafa)
> > ---
> >  drivers/iommu/iommufd/device.c | 126
> > +++++++++++++++++++++++---------- 1 file changed, 87 insertions(+),
> > 39 deletions(-)  
> 
> 
> > @@ -294,11 +334,10 @@ struct iommufd_device
> > *iommufd_device_bind(struct iommufd_ctx *ictx, *id = idev->obj.id;
> >  	return idev;
> >  
> > -out_release_owner:
> > -	iommu_device_release_dma_owner(dev);
> > -out_group_put:
> > -	iommufd_put_group(igroup);
> > +err_out:
> > +	iommufd_object_abort_and_destroy(ictx, &idev->obj);
> >  	return ERR_PTR(rc);  
> 
> I fed this to Sashiko and it pointed out the abort flow here will call
> iommufd_device_destroy() bit it isn't prepared to handle a partially
> initialized idev. It looks like igroup is only set once it is
> sufficiently initialized so:
> 
> @@ -211,6 +211,8 @@ void iommufd_device_destroy(struct iommufd_object
> *obj) struct iommufd_device *idev =
>                 container_of(obj, struct iommufd_device, obj);
>  
> +       if (!idev->igroup)
> +               return;
>         if (!is_vfio_noiommu(idev))
>                 iommu_device_release_dma_owner(idev->dev);
>         iommufd_put_group(idev->igroup);
> 
> Should fix it, but lets add a comment because it is pretty fragile:
> 
> @@ -335,6 +337,10 @@ struct iommufd_device
> *iommufd_device_bind(struct iommufd_ctx *ictx, return idev;
>  
>  err_out:
> +       /*
> +        * Be careful that iommufd_device_destroy() can handle partial
> +        * initialization.
> +        */
>         iommufd_object_abort_and_destroy(ictx, &idev->obj);
>         return ERR_PTR(rc);
Yeah, will add to the next version.

Tested the error path by faking a failed group allocation, works as
expected.
#  RUN           vfio_noiommu.device_bind_iommufd ...
vfio_iommufd_noiommu_test.c:265:device_bind_iommufd:Expected 0 )
# device_bind_iommufd: Test terminated by assertion
#          FAIL  vfio_noiommu.device_bind_iommufd
not ok 17 vfio_noiommu.device_bind_iommufd