Re: [PATCH] Revert "iommu/arm-smmu: Convert to domain_alloc_paging()"

From: Robin Murphy <robin.murphy@arm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Will Deacon <will@kernel.org>, Joerg Roedel <joro@8bytes.org>,
	Jason Gunthorpe <jgg@ziepe.ca>, Rob Clark <robdclark@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev,
	linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH] Revert "iommu/arm-smmu: Convert to domain_alloc_paging()"
Date: Tue, 13 Feb 2024 11:38:08 +0000	[thread overview]
Message-ID: <02a590bd-6379-455a-8184-d631ae88e63a@arm.com> (raw)
In-Reply-To: <20240213-iommu-revert-domain-alloc-v1-1-325ff55dece4@linaro.org>

On 2024-02-13 11:31 am, Dmitry Baryshkov wrote:
> This reverts commit 9b3febc3a3da ("iommu/arm-smmu: Convert to
> domain_alloc_paging()"). It breaks Qualcomm MSM8996 platform. Calling
> arm_smmu_write_context_bank() from new codepath results in the platform
> being reset because of the unclocked hardware access.

Acked-by: Robin Murphy <robin.murphy@arm.com>

> Fixes: 9b3febc3a3da ("iommu/arm-smmu: Convert to domain_alloc_paging()")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/iommu/arm/arm-smmu/arm-smmu.c | 17 ++++++-----------
>   1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 68b6bc5e7c71..6317aaf7b3ab 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -859,10 +859,14 @@ static void arm_smmu_destroy_domain_context(struct arm_smmu_domain *smmu_domain)
>   	arm_smmu_rpm_put(smmu);
>   }
>   
> -static struct iommu_domain *arm_smmu_domain_alloc_paging(struct device *dev)
> +static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
>   {
>   	struct arm_smmu_domain *smmu_domain;
>   
> +	if (type != IOMMU_DOMAIN_UNMANAGED) {
> +		if (using_legacy_binding || type != IOMMU_DOMAIN_DMA)
> +			return NULL;
> +	}
>   	/*
>   	 * Allocate the domain and initialise some of its data structures.
>   	 * We can't really do anything meaningful until we've added a
> @@ -875,15 +879,6 @@ static struct iommu_domain *arm_smmu_domain_alloc_paging(struct device *dev)
>   	mutex_init(&smmu_domain->init_mutex);
>   	spin_lock_init(&smmu_domain->cb_lock);
>   
> -	if (dev) {
> -		struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
> -
> -		if (arm_smmu_init_domain_context(smmu_domain, cfg->smmu, dev)) {
> -			kfree(smmu_domain);
> -			return NULL;
> -		}
> -	}
> -
>   	return &smmu_domain->domain;
>   }
>   
> @@ -1600,7 +1595,7 @@ static struct iommu_ops arm_smmu_ops = {
>   	.identity_domain	= &arm_smmu_identity_domain,
>   	.blocked_domain		= &arm_smmu_blocked_domain,
>   	.capable		= arm_smmu_capable,
> -	.domain_alloc_paging	= arm_smmu_domain_alloc_paging,
> +	.domain_alloc		= arm_smmu_domain_alloc,
>   	.probe_device		= arm_smmu_probe_device,
>   	.release_device		= arm_smmu_release_device,
>   	.probe_finalize		= arm_smmu_probe_finalize,
> 
> ---
> base-commit: 46d4e2eb58e14c8935fa0e27d16d4c62ef82849a
> change-id: 20240213-iommu-revert-domain-alloc-fa729e537df5
> 
> Best regards,