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

[Yi Liu <yi.l.liu@intel.com>]

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@intel.com> wrote:
> 
>> On 5/12/26 02:41, 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>
>>> ---
>>> 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.