Re: [PATCH v5 4/9] iommufd: Allow binding to a noiommu device

[Yi Liu]

On 5/20/26 23:54, Jacob Pan wrote:
> Hi Yi,
>
> On Wed, 20 May 2026 15:20:06 +0800
> Yi Liu <yi.l.liu@xxxxxxxxx> wrote:
>
> > On 5/12/26 02:41, Jacob Pan wrote:
> > > From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxx>
> > > Signed-off-by: Jacob Pan <jacob.pan@xxxxxxxxxxxxxxxxxxx>
> > > ---
> > > v5:
> > >      - simplify logic and rename iommufd_device_is_noiommu (Kevin,
> > > Yi)
> > >      - use a helper iommufd_bind_noiommu instead of open coding
> > > (Kevin)
> > >      - move IOMMU cap check under iommufd_bind_iommu() (Yi)
> > >      - reword comments for partial init (Yi)
> > >      - misc minor clean up
> > > v4:
> > >      - Update the description of the module parameter (Alex)
> > > v3:
> > >      - Consolidate into fewer patches
> > > ---
> > >    drivers/iommu/iommufd/device.c | 148
> > > ++++++++++++++++++++++++--------- 1 file changed, 109
> > > insertions(+), 39 deletions(-)
> > >
> > > diff --git a/drivers/iommu/iommufd/device.c
> > > b/drivers/iommu/iommufd/device.c index d03076fcf3c2..4d75720432cc
> > > 100644 --- a/drivers/iommu/iommufd/device.c
> > > +++ b/drivers/iommu/iommufd/device.c
> > > @@ -23,6 +23,16 @@ struct iommufd_attach {
> > >    	struct xarray device_array;
> > >    };
> > >    
> > > +/*
> > > + * A noiommu device has no IOMMU driver attached regardless of
> > > whether it
> > > + * enters via the cdev path (no iommu_group) or the group path
> > > (fake
> > > + * noiommu iommu_group). In both cases dev->iommu is NULL.
> >
> > Are you trying to identify the case where both group and cdev
> > interfaces coexist? In such a scenario, the group path creates a fake
> > iommu group while the cdev path creates a dummy igroup. Both
> > operations need to be performed, but checking iommu_group might
> > interfere with the cdev noiommu path logic.
> >
> > Is that why you're checking idev->dev->iommu here instead? If so,
> > could you please make this rationale more explicit in the comment?
> >
> > At first glance, the comment is quite confusing. It's unclear why a
> > cdev-path-only helper function needs to reference the group path at
> > all. The last sentence in particular seems to suggest this helper
> > covers both paths, which adds to the confusion.
> Yes, Baolu also commented on this asking why not use
> device_iommu_mapped. I will update the comment as follows:
>   /*
> - * A noiommu device has no IOMMU driver attached regardless of whether
> it
> - * enters via the cdev path (no iommu_group) or the group path (fake
> - * noiommu iommu_group). In both cases dev->iommu is NULL.
> + * Detect a noiommu device for the cdev path. We check dev->iommu
> rather than
> + * using device_iommu_mapped() (which checks dev->iommu_group) because
> when
> + * both group and cdev interfaces coexist, the group path assigns a
> fake
> + * noiommu iommu_group to the device. That would cause
> device_iommu_mapped()
> + * to return true and hide the noiommu case from the cdev path.
> dev->iommu is
> + * reliably NULL when no IOMMU driver is managing the device.
>    */
>   static bool iommufd_device_is_noiommu(struct iommufd_device *idev)
>   {

this is clear to me now. I was just confused by the comment. And then
realized what you really mean.