Re: [PATCH 1/5] iommu: Make @handle mandatory in iommu_{attach|replace}_group_handle()

[Nicolin Chen <nicolinc@nvidia.com>]

On Tue, Feb 11, 2025 at 10:05:36PM -0800, Yi Liu wrote:
> Caller of the two APIs always provide a valid handle, make @handle as
> mandatory parameter.
> 
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

A side note here:

> @@ -3511,8 +3511,10 @@ int iommu_attach_group_handle(struct iommu_domain *domain,
>  {
>  	int ret;
>  
> -	if (handle)
> -		handle->domain = domain;
> +	if (!handle)
> +		return -EINVAL;
> +
> +	handle->domain = domain;
>  
>  	mutex_lock(&group->mutex);
>  	ret = xa_insert(&group->pasid_array, IOMMU_NO_PASID, handle, GFP_KERNEL);
> @@ -3568,16 +3570,15 @@ int iommu_replace_group_handle(struct iommu_group *group,
>  	void *curr;
>  	int ret;
>  
> -	if (!new_domain)
> +	if (!new_domain || !handle)
>  		return -EINVAL;
>  
> +	handle->domain = new_domain;
> +
>  	mutex_lock(&group->mutex);

So, we don't have the mutex fencing the handle->domain, since an
attach handle is always a newly allocated one in the caller, and
there won't be any potential race occurring to it.

Yet, this feels like an implication, which we should have stated
explicitly in the kdocs of these two functions?

Thanks
Nicolin