Re: [RFC PATCH] iommu: Handle domain allocation failure gracefully

[Baolu Lu <baolu.lu@linux.intel.com>]

On 9/14/22 10:44 PM, Vasant Hegde wrote:
> Current code does not validate domain allocation failures. This will
> result in random failure at later stage. Hence handle domain allocation
> failures.
> 
> Signed-off-by: Vasant Hegde<vasant.hegde@amd.com>
> ---
> Hi Joerg,
>    With current code whenever amd driver failes to allocate domain we hit
>    failure at late stage and it became difficult to debug. Hence I
>    thought of adding this fix.
> 
>    I see that in commit 6e1aa2049154 you have intentionally didn't check
>    the return value. Looking into drivers/iommu/* it looks like all
>    drivers has domain_alloc() callback handler. But some drivers (like msm
>    - msm_iommu_domain_alloc()) allocates default domain for certain type
>    only. Hence I'm not sure whether its ok to handle domain allocation
>    failures or not. Hence I have marked this patch as RFC.
> 
>    Let me know whether this patch is fine -OR- is there any other way we can
>    handle domain allocation failures?
> 
> -Vasant
> 
>   drivers/iommu/iommu.c | 24 +++++++++++++-----------
>   1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 780fb7071577..e8a1e7fbffb9 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -289,18 +289,21 @@ int iommu_probe_device(struct device *dev)
>   
>   	/*
>   	 * Try to allocate a default domain - needs support from the
> -	 * IOMMU driver. There are still some drivers which don't
> -	 * support default domains, so the return value is not yet
> -	 * checked.
> +	 * IOMMU driver.
>   	 */
>   	mutex_lock(&group->mutex);
> -	iommu_alloc_default_domain(group, dev);
> +	ret = iommu_alloc_default_domain(group, dev);
> +	if (ret) {
> +		mutex_unlock(&group->mutex);
> +		iommu_group_put(group);
> +		goto err_release;
> +	}

Some IOMMU drivers (s390, gart, tegra...) don't support default domains.
iommu_alloc_default_domain() returns a ENOMEM and the probe process
continues.

Above changes this behavior. The probe process will always fails with
those IOMMU drivers.

Best regards,
baolu