Re: [PATCH v3 01/12] iommu: Refactor __iommu_domain_alloc()

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

On 2024/10/17 22:01, Vasant Hegde wrote:
> From: Jason Gunthorpe <jgg@ziepe.ca>
> 
> Following patch will introduce iommu_paging_domain_alloc_flags() API.
> Hence move domain init code to separate function so that it can be
> resued.

A nit: s/resued/reused/

> 
> Also move iommu_get_dma_cookie() setup iommu_setup_default_domain() as
> its required in DMA API mode only.

a nit: s/its/it's/

Otherwise, LGTM.

Reviewed-by: Yi Liu <yi.l.liu@intel.com>

> 
> Signed-off-by: Jason Gunthorpe <jgg@ziepe.ca>
> [Split the patch and added description - Vasant]
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> ---
>   drivers/iommu/iommu.c | 46 +++++++++++++++++++++++--------------------
>   1 file changed, 25 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 83c8e617a2c5..811748bec4b0 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1934,6 +1934,22 @@ void iommu_set_fault_handler(struct iommu_domain *domain,
>   }
>   EXPORT_SYMBOL_GPL(iommu_set_fault_handler);
>   
> +static void iommu_domain_init(struct iommu_domain *domain, unsigned int type,
> +			      const struct iommu_ops *ops)
> +{
> +	domain->type = type;
> +	domain->owner = ops;
> +	if (!domain->ops)
> +		domain->ops = ops->default_domain_ops;
> +
> +	/*
> +	 * If not already set, assume all sizes by default; the driver
> +	 * may override this later
> +	 */
> +	if (!domain->pgsize_bitmap)
> +		domain->pgsize_bitmap = ops->pgsize_bitmap;
> +}
> +
>   static struct iommu_domain *__iommu_domain_alloc(const struct iommu_ops *ops,
>   						 struct device *dev,
>   						 unsigned int type)
> @@ -1962,27 +1978,7 @@ static struct iommu_domain *__iommu_domain_alloc(const struct iommu_ops *ops,
>   	if (!domain)
>   		return ERR_PTR(-ENOMEM);
>   
> -	domain->type = type;
> -	domain->owner = ops;
> -	/*
> -	 * If not already set, assume all sizes by default; the driver
> -	 * may override this later
> -	 */
> -	if (!domain->pgsize_bitmap)
> -		domain->pgsize_bitmap = ops->pgsize_bitmap;
> -
> -	if (!domain->ops)
> -		domain->ops = ops->default_domain_ops;
> -
> -	if (iommu_is_dma_domain(domain)) {
> -		int rc;
> -
> -		rc = iommu_get_dma_cookie(domain);
> -		if (rc) {
> -			iommu_domain_free(domain);
> -			return ERR_PTR(rc);
> -		}
> -	}
> +	iommu_domain_init(domain, type, ops);
>   	return domain;
>   }
>   
> @@ -2965,6 +2961,14 @@ static int iommu_setup_default_domain(struct iommu_group *group,
>   	if (group->default_domain == dom)
>   		return 0;
>   
> +	if (iommu_is_dma_domain(dom)) {
> +		ret = iommu_get_dma_cookie(dom);
> +		if (ret) {
> +			iommu_domain_free(dom);
> +			return ret;
> +		}
> +	}
> +
>   	/*
>   	 * IOMMU_RESV_DIRECT and IOMMU_RESV_DIRECT_RELAXABLE regions must be
>   	 * mapped before their device is attached, in order to guarantee

-- 
Regards,
Yi Liu