Re: [PATCH v8 01/12] iommu: Add iommu_attach_device_pasid_handle()

[Nicolin Chen <nicolinc@nvidia.com>]

On Thu, Feb 27, 2025 at 09:27:12AM +0800, Yi Liu wrote:
> On 2025/2/27 06:16, Nicolin Chen wrote:
> > On Wed, Feb 26, 2025 at 03:40:21AM -0800, Yi Liu wrote:
> > > The existing iommu_attach_device_pasid() function allows both a valid
> > > handle and a NULL handle, which is not consistent with the RID path where
> > > iommu_attach_group() and iommu_attach_group_handle() coexist. To refine
> > > it, this adds iommu_attach_device_pasid_handle() to cover the case with
> > > valid handle, while let the iommu_attach_device_pasid() only deals with
> > > the case with NULL handle.
> > 
> > Hmm, I am not very sure about the necessity of this change. The
> > underlying function being called from those two helpers (with/
> > without handle) is still taking a NULL handle, which looks very
> > straightforward to me already, so this extra layer feels a bit
> > redundant..
> 
> I think I should have added a if (!handle) check in the beginning of
> iommu_attach_device_pasid_handle() just like the other _handle() APIs.
> If so, this should be clearer. is it?

My point is that the extra layer of these two helpers really do
nothing but be two wrappers around the exact same function with
just different "handle" inputs which only introduces some extra
complicity to the core code, not to mention that the underlying
function __iommu_attach_device_pasid is even in the header :-/

So, overall it doesn't feel very necessary to me...

The iommu_attach_group/iommu_attach_group_handle() case, on the
other hand, does take care of some underlying difference. Thus,
having two separate functions could make things clear.

Thanks
Nic