iommu.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings
@ 2020-09-04 15:55 Bjorn Andersson
  2020-09-04 15:55 ` [PATCH v3 1/8] iommu/arm-smmu: Refactor context bank allocation Bjorn Andersson
                   ` (13 more replies)
  0 siblings, 14 replies; 33+ messages in thread
From: Bjorn Andersson @ 2020-09-04 15:55 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Joerg Roedel, Bjorn Andersson,
	Sai Prakash Ranjan, Jordan Crouse, Rob Clark
  Cc: linux-arm-msm, iommu, Sibi Sankar, linux-arm-kernel, linux-kernel

Based on previous attempts and discussions this is the latest attempt at
inheriting stream mappings set up by the bootloader, for e.g. boot splash or
efifb.

Per Will's request this builds on the work by Jordan and Rob for the Adreno
SMMU support. It applies cleanly ontop of v16 of their series, which can be
found at
https://lore.kernel.org/linux-arm-msm/20200901164707.2645413-1-robdclark@gmail.com/

Bjorn Andersson (8):
  iommu/arm-smmu: Refactor context bank allocation
  iommu/arm-smmu: Delay modifying domain during init
  iommu/arm-smmu: Consult context bank allocator for identify domains
  iommu/arm-smmu-qcom: Emulate bypass by using context banks
  iommu/arm-smmu-qcom: Consistently initialize stream mappings
  iommu/arm-smmu: Add impl hook for inherit boot mappings
  iommu/arm-smmu: Provide helper for allocating identity domain
  iommu/arm-smmu-qcom: Setup identity domain for boot mappings

 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 111 ++++++++++++++++++-
 drivers/iommu/arm/arm-smmu/arm-smmu.c      | 122 ++++++++++++++-------
 drivers/iommu/arm/arm-smmu/arm-smmu.h      |  14 ++-
 3 files changed, 205 insertions(+), 42 deletions(-)

-- 
2.28.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v3 1/8] iommu/arm-smmu: Refactor context bank allocation
  2020-09-04 15:55 [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Bjorn Andersson
@ 2020-09-04 15:55 ` Bjorn Andersson
  2020-09-08 18:42   ` Jordan Crouse
  2020-09-11  8:18   ` Sai Prakash Ranjan
  2020-09-04 15:55 ` [PATCH v3 2/8] iommu/arm-smmu: Delay modifying domain during init Bjorn Andersson
                   ` (12 subsequent siblings)
  13 siblings, 2 replies; 33+ messages in thread
From: Bjorn Andersson @ 2020-09-04 15:55 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Joerg Roedel, Bjorn Andersson,
	Sai Prakash Ranjan, Jordan Crouse, Rob Clark
  Cc: linux-arm-msm, iommu, Sibi Sankar, linux-arm-kernel, linux-kernel

Extract the conditional invocation of the platform defined
alloc_context_bank() to a separate function to keep
arm_smmu_init_domain_context() cleaner.

Instead pass a reference to the arm_smmu_device as parameter to the
call. Also remove the count parameter, as this can be read from the
newly passed object.

This allows us to not assign smmu_domain->smmu before attempting to
allocate the context bank and as such we don't need to roll back this
assignment on failure.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Note that this series applies ontop of:
https://lore.kernel.org/linux-arm-msm/20200901164707.2645413-1-robdclark@gmail.com/

This could either go on its own, or be squashed with "[PATCH v16 14/20]
iommu/arm-smmu: Prepare for the adreno-smmu implementation" from Rob's series.

Changes since v2:
- New patch

 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c |  6 ++++--
 drivers/iommu/arm/arm-smmu/arm-smmu.c      | 23 ++++++++++++----------
 drivers/iommu/arm/arm-smmu/arm-smmu.h      |  3 ++-
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 2aa6249050ff..0663d7d26908 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -91,9 +91,10 @@ static int qcom_adreno_smmu_set_ttbr0_cfg(const void *cookie,
 }
 
 static int qcom_adreno_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_domain,
-		struct device *dev, int start, int count)
+					       struct arm_smmu_device *smmu,
+					       struct device *dev, int start)
 {
-	struct arm_smmu_device *smmu = smmu_domain->smmu;
+	int count;
 
 	/*
 	 * Assign context bank 0 to the GPU device so the GPU hardware can
@@ -104,6 +105,7 @@ static int qcom_adreno_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_doma
 		count = 1;
 	} else {
 		start = 1;
+		count = smmu->num_context_banks;
 	}
 
 	return __arm_smmu_alloc_bitmap(smmu->context_map, start, count);
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index bbec5793faf8..e19d7bdc7674 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -623,6 +623,16 @@ void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx)
 	arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_SCTLR, reg);
 }
 
+static int arm_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_domain,
+				       struct arm_smmu_device *smmu,
+				       struct device *dev, unsigned int start)
+{
+	if (smmu->impl && smmu->impl->alloc_context_bank)
+		return smmu->impl->alloc_context_bank(smmu_domain, smmu, dev, start);
+
+	return __arm_smmu_alloc_bitmap(smmu->context_map, start, smmu->num_context_banks);
+}
+
 static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 					struct arm_smmu_device *smmu,
 					struct device *dev)
@@ -741,20 +751,13 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 		goto out_unlock;
 	}
 
-	smmu_domain->smmu = smmu;
-
-	if (smmu->impl && smmu->impl->alloc_context_bank)
-		ret = smmu->impl->alloc_context_bank(smmu_domain, dev,
-				start, smmu->num_context_banks);
-	else
-		ret = __arm_smmu_alloc_bitmap(smmu->context_map, start,
-				      smmu->num_context_banks);
-
+	ret = arm_smmu_alloc_context_bank(smmu_domain, smmu, dev, start);
 	if (ret < 0) {
-		smmu_domain->smmu = NULL;
 		goto out_unlock;
 	}
 
+	smmu_domain->smmu = smmu;
+
 	cfg->cbndx = ret;
 	if (smmu->version < ARM_SMMU_V2) {
 		cfg->irptndx = atomic_inc_return(&smmu->irptndx);
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index 2df3a70a8a41..ddf2ca4c923d 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -437,7 +437,8 @@ struct arm_smmu_impl {
 	irqreturn_t (*global_fault)(int irq, void *dev);
 	irqreturn_t (*context_fault)(int irq, void *dev);
 	int (*alloc_context_bank)(struct arm_smmu_domain *smmu_domain,
-			struct device *dev, int start, int max);
+				  struct arm_smmu_device *smmu,
+				  struct device *dev, int start);
 };
 
 #define INVALID_SMENDX			-1
-- 
2.28.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 2/8] iommu/arm-smmu: Delay modifying domain during init
  2020-09-04 15:55 [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Bjorn Andersson
  2020-09-04 15:55 ` [PATCH v3 1/8] iommu/arm-smmu: Refactor context bank allocation Bjorn Andersson
@ 2020-09-04 15:55 ` Bjorn Andersson
  2020-09-11  8:20   ` Sai Prakash Ranjan
  2020-09-04 15:55 ` [PATCH v3 3/8] iommu/arm-smmu: Consult context bank allocator for identify domains Bjorn Andersson
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 33+ messages in thread
From: Bjorn Andersson @ 2020-09-04 15:55 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Joerg Roedel, Bjorn Andersson,
	Sai Prakash Ranjan, Jordan Crouse, Rob Clark
  Cc: linux-arm-msm, iommu, Sibi Sankar, linux-arm-kernel, linux-kernel

Delay modifications to the domain during arm_smmu_init_domain_context()
until we've allocated a context bank. This will allow us to postpone the
special handling of identity domains until the platform specific context
bank allocator has been executed, in a later patch.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- New patch to allow us to rely on the impl specific alloc_context_bank().

 drivers/iommu/arm/arm-smmu/arm-smmu.c | 40 +++++++++++++++------------
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index e19d7bdc7674..add2e1807e21 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -645,6 +645,9 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
 	irqreturn_t (*context_fault)(int irq, void *dev);
+	struct arm_smmu_cfg new_cfg = *cfg;
+	enum arm_smmu_domain_stage new_stage = smmu_domain->stage;
+	const struct iommu_flush_ops *flush_ops;
 
 	mutex_lock(&smmu_domain->init_mutex);
 	if (smmu_domain->smmu)
@@ -675,9 +678,9 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 	 * Note that you can't actually request stage-2 mappings.
 	 */
 	if (!(smmu->features & ARM_SMMU_FEAT_TRANS_S1))
-		smmu_domain->stage = ARM_SMMU_DOMAIN_S2;
+		new_stage = ARM_SMMU_DOMAIN_S2;
 	if (!(smmu->features & ARM_SMMU_FEAT_TRANS_S2))
-		smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
+		new_stage = ARM_SMMU_DOMAIN_S1;
 
 	/*
 	 * Choosing a suitable context format is even more fiddly. Until we
@@ -688,32 +691,32 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 	 * support to be a superset of AArch32 support...
 	 */
 	if (smmu->features & ARM_SMMU_FEAT_FMT_AARCH32_L)
-		cfg->fmt = ARM_SMMU_CTX_FMT_AARCH32_L;
+		new_cfg.fmt = ARM_SMMU_CTX_FMT_AARCH32_L;
 	if (IS_ENABLED(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) &&
 	    !IS_ENABLED(CONFIG_64BIT) && !IS_ENABLED(CONFIG_ARM_LPAE) &&
 	    (smmu->features & ARM_SMMU_FEAT_FMT_AARCH32_S) &&
-	    (smmu_domain->stage == ARM_SMMU_DOMAIN_S1))
-		cfg->fmt = ARM_SMMU_CTX_FMT_AARCH32_S;
-	if ((IS_ENABLED(CONFIG_64BIT) || cfg->fmt == ARM_SMMU_CTX_FMT_NONE) &&
+	    (new_stage == ARM_SMMU_DOMAIN_S1))
+		new_cfg.fmt = ARM_SMMU_CTX_FMT_AARCH32_S;
+	if ((IS_ENABLED(CONFIG_64BIT) || new_cfg.fmt == ARM_SMMU_CTX_FMT_NONE) &&
 	    (smmu->features & (ARM_SMMU_FEAT_FMT_AARCH64_64K |
 			       ARM_SMMU_FEAT_FMT_AARCH64_16K |
 			       ARM_SMMU_FEAT_FMT_AARCH64_4K)))
-		cfg->fmt = ARM_SMMU_CTX_FMT_AARCH64;
+		new_cfg.fmt = ARM_SMMU_CTX_FMT_AARCH64;
 
-	if (cfg->fmt == ARM_SMMU_CTX_FMT_NONE) {
+	if (new_cfg.fmt == ARM_SMMU_CTX_FMT_NONE) {
 		ret = -EINVAL;
 		goto out_unlock;
 	}
 
-	switch (smmu_domain->stage) {
+	switch (new_stage) {
 	case ARM_SMMU_DOMAIN_S1:
-		cfg->cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS;
+		new_cfg.cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS;
 		start = smmu->num_s2_context_banks;
 		ias = smmu->va_size;
 		oas = smmu->ipa_size;
-		if (cfg->fmt == ARM_SMMU_CTX_FMT_AARCH64) {
+		if (new_cfg.fmt == ARM_SMMU_CTX_FMT_AARCH64) {
 			fmt = ARM_64_LPAE_S1;
-		} else if (cfg->fmt == ARM_SMMU_CTX_FMT_AARCH32_L) {
+		} else if (new_cfg.fmt == ARM_SMMU_CTX_FMT_AARCH32_L) {
 			fmt = ARM_32_LPAE_S1;
 			ias = min(ias, 32UL);
 			oas = min(oas, 40UL);
@@ -722,7 +725,7 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 			ias = min(ias, 32UL);
 			oas = min(oas, 32UL);
 		}
-		smmu_domain->flush_ops = &arm_smmu_s1_tlb_ops;
+		flush_ops = &arm_smmu_s1_tlb_ops;
 		break;
 	case ARM_SMMU_DOMAIN_NESTED:
 		/*
@@ -730,11 +733,11 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 		 * involved.
 		 */
 	case ARM_SMMU_DOMAIN_S2:
-		cfg->cbar = CBAR_TYPE_S2_TRANS;
+		new_cfg.cbar = CBAR_TYPE_S2_TRANS;
 		start = 0;
 		ias = smmu->ipa_size;
 		oas = smmu->pa_size;
-		if (cfg->fmt == ARM_SMMU_CTX_FMT_AARCH64) {
+		if (new_cfg.fmt == ARM_SMMU_CTX_FMT_AARCH64) {
 			fmt = ARM_64_LPAE_S2;
 		} else {
 			fmt = ARM_32_LPAE_S2;
@@ -742,9 +745,9 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 			oas = min(oas, 40UL);
 		}
 		if (smmu->version == ARM_SMMU_V2)
-			smmu_domain->flush_ops = &arm_smmu_s2_tlb_ops_v2;
+			flush_ops = &arm_smmu_s2_tlb_ops_v2;
 		else
-			smmu_domain->flush_ops = &arm_smmu_s2_tlb_ops_v1;
+			flush_ops = &arm_smmu_s2_tlb_ops_v1;
 		break;
 	default:
 		ret = -EINVAL;
@@ -757,6 +760,9 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 	}
 
 	smmu_domain->smmu = smmu;
+	smmu_domain->cfg = new_cfg;
+	smmu_domain->stage = new_stage;
+	smmu_domain->flush_ops = flush_ops;
 
 	cfg->cbndx = ret;
 	if (smmu->version < ARM_SMMU_V2) {
-- 
2.28.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 3/8] iommu/arm-smmu: Consult context bank allocator for identify domains
  2020-09-04 15:55 [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Bjorn Andersson
  2020-09-04 15:55 ` [PATCH v3 1/8] iommu/arm-smmu: Refactor context bank allocation Bjorn Andersson
  2020-09-04 15:55 ` [PATCH v3 2/8] iommu/arm-smmu: Delay modifying domain during init Bjorn Andersson
@ 2020-09-04 15:55 ` Bjorn Andersson
  2020-09-11  8:21   ` Sai Prakash Ranjan
  2020-09-11  8:24   ` Sai Prakash Ranjan
  2020-09-04 15:55 ` [PATCH v3 4/8] iommu/arm-smmu-qcom: Emulate bypass by using context banks Bjorn Andersson
                   ` (10 subsequent siblings)
  13 siblings, 2 replies; 33+ messages in thread
From: Bjorn Andersson @ 2020-09-04 15:55 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Joerg Roedel, Bjorn Andersson,
	Sai Prakash Ranjan, Jordan Crouse, Rob Clark
  Cc: linux-arm-msm, iommu, Sibi Sankar, linux-arm-kernel, linux-kernel

For implementations of the ARM SMMU where stream mappings of bypass type
are prohibited identity domains can be implemented by using context
banks with translation disabled.

Postpone the decision to skip allocating a context bank until the
implementation specific context bank allocator has been consulted and if
it decides to use a context bank for the identity map, don't enable
translation (i.e. omit ARM_SMMU_SCTLR_M).

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- Tie this to alloc_context_bank rather than carrying a Qualcomm specific quirk
  in the generic code.

 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c |  4 ++++
 drivers/iommu/arm/arm-smmu/arm-smmu.c      | 23 +++++++++++++++-------
 drivers/iommu/arm/arm-smmu/arm-smmu.h      |  3 +++
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 0663d7d26908..229fc8ff8cea 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -94,8 +94,12 @@ static int qcom_adreno_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_doma
 					       struct arm_smmu_device *smmu,
 					       struct device *dev, int start)
 {
+	struct iommu_domain *domain = &smmu_domain->domain;
 	int count;
 
+	if (domain->type == IOMMU_DOMAIN_IDENTITY)
+		return ARM_SMMU_CBNDX_BYPASS;
+
 	/*
 	 * Assign context bank 0 to the GPU device so the GPU hardware can
 	 * switch pagetables
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index add2e1807e21..eb5c6ca5c138 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -611,7 +611,9 @@ void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx)
 
 	/* SCTLR */
 	reg = ARM_SMMU_SCTLR_CFIE | ARM_SMMU_SCTLR_CFRE | ARM_SMMU_SCTLR_AFE |
-	      ARM_SMMU_SCTLR_TRE | ARM_SMMU_SCTLR_M;
+	      ARM_SMMU_SCTLR_TRE;
+	if (cfg->m)
+		reg |= ARM_SMMU_SCTLR_M;
 	if (stage1)
 		reg |= ARM_SMMU_SCTLR_S1_ASIDPNE;
 	if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
@@ -627,9 +629,14 @@ static int arm_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_domain,
 				       struct arm_smmu_device *smmu,
 				       struct device *dev, unsigned int start)
 {
+	struct iommu_domain *domain = &smmu_domain->domain;
+
 	if (smmu->impl && smmu->impl->alloc_context_bank)
 		return smmu->impl->alloc_context_bank(smmu_domain, smmu, dev, start);
 
+	if (domain->type == IOMMU_DOMAIN_IDENTITY)
+		return ARM_SMMU_CBNDX_BYPASS;
+
 	return __arm_smmu_alloc_bitmap(smmu->context_map, start, smmu->num_context_banks);
 }
 
@@ -653,12 +660,6 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 	if (smmu_domain->smmu)
 		goto out_unlock;
 
-	if (domain->type == IOMMU_DOMAIN_IDENTITY) {
-		smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
-		smmu_domain->smmu = smmu;
-		goto out_unlock;
-	}
-
 	/*
 	 * Mapping the requested stage onto what we support is surprisingly
 	 * complicated, mainly because the spec allows S1+S2 SMMUs without
@@ -757,6 +758,10 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 	ret = arm_smmu_alloc_context_bank(smmu_domain, smmu, dev, start);
 	if (ret < 0) {
 		goto out_unlock;
+	} else if (ret == ARM_SMMU_CBNDX_BYPASS) {
+		smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
+		smmu_domain->smmu = smmu;
+		goto out_unlock;
 	}
 
 	smmu_domain->smmu = smmu;
@@ -813,6 +818,10 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 
 	domain->geometry.force_aperture = true;
 
+	/* Enable translation for non-identity context banks */
+	if (domain->type != IOMMU_DOMAIN_IDENTITY)
+		cfg->m = true;
+
 	/* Initialise the context bank with our page table cfg */
 	arm_smmu_init_context_bank(smmu_domain, &pgtbl_cfg);
 	arm_smmu_write_context_bank(smmu, cfg->cbndx);
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index ddf2ca4c923d..235d9a3a6ab6 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -243,6 +243,8 @@ enum arm_smmu_cbar_type {
 #define TLB_LOOP_TIMEOUT		1000000	/* 1s! */
 #define TLB_SPIN_COUNT			10
 
+#define ARM_SMMU_CBNDX_BYPASS		0xffff
+
 /* Shared driver definitions */
 enum arm_smmu_arch_version {
 	ARM_SMMU_V1,
@@ -346,6 +348,7 @@ struct arm_smmu_cfg {
 	u32				sctlr_clr;    /* bits to mask in SCTLR */
 	enum arm_smmu_cbar_type		cbar;
 	enum arm_smmu_context_fmt	fmt;
+	bool				m;
 };
 #define ARM_SMMU_INVALID_IRPTNDX	0xff
 
-- 
2.28.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 4/8] iommu/arm-smmu-qcom: Emulate bypass by using context banks
  2020-09-04 15:55 [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Bjorn Andersson
                   ` (2 preceding siblings ...)
  2020-09-04 15:55 ` [PATCH v3 3/8] iommu/arm-smmu: Consult context bank allocator for identify domains Bjorn Andersson
@ 2020-09-04 15:55 ` Bjorn Andersson
  2020-09-11  8:25   ` Sai Prakash Ranjan
  2020-09-04 15:55 ` [PATCH v3 5/8] iommu/arm-smmu-qcom: Consistently initialize stream mappings Bjorn Andersson
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 33+ messages in thread
From: Bjorn Andersson @ 2020-09-04 15:55 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Joerg Roedel, Bjorn Andersson,
	Sai Prakash Ranjan, Jordan Crouse, Rob Clark
  Cc: linux-arm-msm, iommu, Sibi Sankar, linux-arm-kernel, linux-kernel

Some firmware found on various Qualcomm platforms traps writes to S2CR
of type BYPASS and writes FAULT into the register. In particular, this
prevents us from marking the streams for the display controller as
BYPASS to allow continued scanout of the screen through the
initialization of the ARM SMMU.

This adds a Qualcomm specific cfg_probe function, which probes for the
broken behavior of the S2CR registers and implements a custom
alloc_context_bank() that when necessary allocates a context bank
(without translation) for these domains as well.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- Move quirk from arm_smmudevice to qcom_smmu, as we localize the quirk
  handling to the Qualcomm specific implemntation.

 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 52 ++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 229fc8ff8cea..284761a1cd8e 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -11,8 +11,14 @@
 
 struct qcom_smmu {
 	struct arm_smmu_device smmu;
+	bool bypass_broken;
 };
 
+static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
+{
+	return container_of(smmu, struct qcom_smmu, smmu);
+}
+
 #define QCOM_ADRENO_SMMU_GPU_SID 0
 
 static bool qcom_adreno_smmu_is_gpu_device(struct device *dev)
@@ -162,6 +168,50 @@ static const struct of_device_id qcom_smmu_client_of_match[] __maybe_unused = {
 	{ }
 };
 
+static int qcom_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_domain,
+					struct arm_smmu_device *smmu,
+					struct device *dev, int start)
+{
+	struct iommu_domain *domain = &smmu_domain->domain;
+	struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
+
+	/* Keep identity domains as bypass, unless bypass is broken */
+	if (domain->type == IOMMU_DOMAIN_IDENTITY && !qsmmu->bypass_broken)
+		return ARM_SMMU_CBNDX_BYPASS;
+
+	/*
+	 * The identity domain to emulate bypass is the only domain without a
+	 * dev, use the last context bank for this to avoid collisions with
+	 * active contexts during initialization.
+	 */
+	if (!dev)
+		start = smmu->num_context_banks - 1;
+
+	return __arm_smmu_alloc_bitmap(smmu->context_map, start, smmu->num_context_banks);
+}
+
+static int qcom_smmu_cfg_probe(struct arm_smmu_device *smmu)
+{
+	unsigned int last_s2cr = ARM_SMMU_GR0_S2CR(smmu->num_mapping_groups - 1);
+	struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
+	u32 reg;
+
+	/*
+	 * With some firmware writes to S2CR of type FAULT are ignored, and
+	 * writing BYPASS will end up as FAULT in the register. Perform a write
+	 * to S2CR to detect if this is the case with the current firmware.
+	 */
+	reg = FIELD_PREP(ARM_SMMU_S2CR_TYPE, S2CR_TYPE_BYPASS) |
+	      FIELD_PREP(ARM_SMMU_S2CR_CBNDX, 0xff) |
+	      FIELD_PREP(ARM_SMMU_S2CR_PRIVCFG, S2CR_PRIVCFG_DEFAULT);
+	arm_smmu_gr0_write(smmu, last_s2cr, reg);
+	reg = arm_smmu_gr0_read(smmu, last_s2cr);
+	if (FIELD_GET(ARM_SMMU_S2CR_TYPE, reg) != S2CR_TYPE_BYPASS)
+		qsmmu->bypass_broken = true;
+
+	return 0;
+}
+
 static int qcom_smmu_def_domain_type(struct device *dev)
 {
 	const struct of_device_id *match =
@@ -200,6 +250,8 @@ static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
 }
 
 static const struct arm_smmu_impl qcom_smmu_impl = {
+	.alloc_context_bank = qcom_smmu_alloc_context_bank,
+	.cfg_probe = qcom_smmu_cfg_probe,
 	.def_domain_type = qcom_smmu_def_domain_type,
 	.reset = qcom_smmu500_reset,
 };
-- 
2.28.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 5/8] iommu/arm-smmu-qcom: Consistently initialize stream mappings
  2020-09-04 15:55 [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Bjorn Andersson
                   ` (3 preceding siblings ...)
  2020-09-04 15:55 ` [PATCH v3 4/8] iommu/arm-smmu-qcom: Emulate bypass by using context banks Bjorn Andersson
@ 2020-09-04 15:55 ` Bjorn Andersson
  2020-09-11  8:26   ` Sai Prakash Ranjan
  2020-09-04 15:55 ` [PATCH v3 6/8] iommu/arm-smmu: Add impl hook for inherit boot mappings Bjorn Andersson
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 33+ messages in thread
From: Bjorn Andersson @ 2020-09-04 15:55 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Joerg Roedel, Bjorn Andersson,
	Sai Prakash Ranjan, Jordan Crouse, Rob Clark
  Cc: linux-arm-msm, iommu, Sibi Sankar, linux-arm-kernel, linux-kernel

Firmware that traps writes to S2CR to translate BYPASS into FAULT also
ignores writes of type FAULT. As such booting with "disable_bypass" set
will result in all S2CR registers left as configured by the bootloader.

This has been seen to result in indeterministic results, as these
mappings might linger and reference context banks that Linux is
reconfiguring.

Use the fact that BYPASS writes result in FAULT type to force all stream
mappings to FAULT.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- None
 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 284761a1cd8e..70a1eaa52e14 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -195,6 +195,7 @@ static int qcom_smmu_cfg_probe(struct arm_smmu_device *smmu)
 	unsigned int last_s2cr = ARM_SMMU_GR0_S2CR(smmu->num_mapping_groups - 1);
 	struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
 	u32 reg;
+	int i;
 
 	/*
 	 * With some firmware writes to S2CR of type FAULT are ignored, and
@@ -206,9 +207,24 @@ static int qcom_smmu_cfg_probe(struct arm_smmu_device *smmu)
 	      FIELD_PREP(ARM_SMMU_S2CR_PRIVCFG, S2CR_PRIVCFG_DEFAULT);
 	arm_smmu_gr0_write(smmu, last_s2cr, reg);
 	reg = arm_smmu_gr0_read(smmu, last_s2cr);
-	if (FIELD_GET(ARM_SMMU_S2CR_TYPE, reg) != S2CR_TYPE_BYPASS)
+	if (FIELD_GET(ARM_SMMU_S2CR_TYPE, reg) != S2CR_TYPE_BYPASS) {
 		qsmmu->bypass_broken = true;
 
+		/*
+		 * With firmware ignoring writes of type FAULT, booting the
+		 * Linux kernel with disable_bypass disabled (i.e. "enable
+		 * bypass") the initialization during probe will leave mappings
+		 * in an inconsistent state. Avoid this by configuring all
+		 * S2CRs to BYPASS.
+		 */
+		for (i = 0; i < smmu->num_mapping_groups; i++) {
+			smmu->s2crs[i].type = S2CR_TYPE_BYPASS;
+			smmu->s2crs[i].privcfg = S2CR_PRIVCFG_DEFAULT;
+			smmu->s2crs[i].cbndx = 0xff;
+			smmu->s2crs[i].count = 0;
+		}
+	}
+
 	return 0;
 }
 
-- 
2.28.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 6/8] iommu/arm-smmu: Add impl hook for inherit boot mappings
  2020-09-04 15:55 [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Bjorn Andersson
                   ` (4 preceding siblings ...)
  2020-09-04 15:55 ` [PATCH v3 5/8] iommu/arm-smmu-qcom: Consistently initialize stream mappings Bjorn Andersson
@ 2020-09-04 15:55 ` Bjorn Andersson
  2020-09-11  8:27   ` Sai Prakash Ranjan
  2020-09-11 17:13   ` Robin Murphy
  2020-09-04 15:55 ` [PATCH v3 7/8] iommu/arm-smmu: Provide helper for allocating identity domain Bjorn Andersson
                   ` (7 subsequent siblings)
  13 siblings, 2 replies; 33+ messages in thread
From: Bjorn Andersson @ 2020-09-04 15:55 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Joerg Roedel, Bjorn Andersson,
	Sai Prakash Ranjan, Jordan Crouse, Rob Clark
  Cc: linux-arm-msm, iommu, Sibi Sankar, linux-arm-kernel, linux-kernel

Add a new operation to allow platform implementations to inherit any
stream mappings from the boot loader.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- New patch/interface

 drivers/iommu/arm/arm-smmu/arm-smmu.c | 11 ++++++-----
 drivers/iommu/arm/arm-smmu/arm-smmu.h |  6 ++++++
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index eb5c6ca5c138..4c4d302cd747 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -85,11 +85,6 @@ static inline void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
 		pm_runtime_put_autosuspend(smmu->dev);
 }
 
-static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
-{
-	return container_of(dom, struct arm_smmu_domain, domain);
-}
-
 static struct platform_driver arm_smmu_driver;
 static struct iommu_ops arm_smmu_ops;
 
@@ -2188,6 +2183,12 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	if (smmu->impl->inherit_mappings) {
+		err = smmu->impl->inherit_mappings(smmu);
+		if (err)
+			return err;
+	}
+
 	if (smmu->version == ARM_SMMU_V2) {
 		if (smmu->num_context_banks > smmu->num_context_irqs) {
 			dev_err(dev,
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index 235d9a3a6ab6..f58164976e74 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -378,6 +378,11 @@ struct arm_smmu_domain {
 	struct iommu_domain		domain;
 };
 
+static inline struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
+{
+	return container_of(dom, struct arm_smmu_domain, domain);
+}
+
 struct arm_smmu_master_cfg {
 	struct arm_smmu_device		*smmu;
 	s16				smendx[];
@@ -442,6 +447,7 @@ struct arm_smmu_impl {
 	int (*alloc_context_bank)(struct arm_smmu_domain *smmu_domain,
 				  struct arm_smmu_device *smmu,
 				  struct device *dev, int start);
+	int (*inherit_mappings)(struct arm_smmu_device *smmu);
 };
 
 #define INVALID_SMENDX			-1
-- 
2.28.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 7/8] iommu/arm-smmu: Provide helper for allocating identity domain
  2020-09-04 15:55 [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Bjorn Andersson
                   ` (5 preceding siblings ...)
  2020-09-04 15:55 ` [PATCH v3 6/8] iommu/arm-smmu: Add impl hook for inherit boot mappings Bjorn Andersson
@ 2020-09-04 15:55 ` Bjorn Andersson
  2020-09-11  8:28   ` Sai Prakash Ranjan
  2020-09-04 15:55 ` [PATCH v3 8/8] iommu/arm-smmu-qcom: Setup identity domain for boot mappings Bjorn Andersson
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 33+ messages in thread
From: Bjorn Andersson @ 2020-09-04 15:55 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Joerg Roedel, Bjorn Andersson,
	Sai Prakash Ranjan, Jordan Crouse, Rob Clark
  Cc: linux-arm-msm, iommu, Sibi Sankar, linux-arm-kernel, linux-kernel

Some platform implementations needs to be able to allocate a domain for
emulating identity mappings using a context bank without translation.
Provide a helper function to allocate such a domain.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- Extracted from previous arm_smmu_setup_identity() implementation

 drivers/iommu/arm/arm-smmu/arm-smmu.c | 25 +++++++++++++++++++++++++
 drivers/iommu/arm/arm-smmu/arm-smmu.h |  2 ++
 2 files changed, 27 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 4c4d302cd747..3c06146dfdb9 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1924,6 +1924,31 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
 	return 0;
 }
 
+struct iommu_domain *arm_smmu_alloc_identity_domain(struct arm_smmu_device *smmu)
+{
+	struct iommu_domain *identity;
+	int ret;
+
+	/* Create a IDENTITY domain to use for all inherited streams */
+	identity = arm_smmu_domain_alloc(IOMMU_DOMAIN_IDENTITY);
+	if (!identity) {
+		dev_err(smmu->dev, "failed to create identity domain\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	identity->pgsize_bitmap = smmu->pgsize_bitmap;
+	identity->type = IOMMU_DOMAIN_IDENTITY;
+	identity->ops = &arm_smmu_ops;
+
+	ret = arm_smmu_init_domain_context(identity, smmu, NULL);
+	if (ret < 0) {
+		dev_err(smmu->dev, "failed to initialize identity domain: %d\n", ret);
+		return ERR_PTR(ret);
+	}
+
+	return identity;
+}
+
 struct arm_smmu_match_data {
 	enum arm_smmu_arch_version version;
 	enum arm_smmu_implementation model;
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index f58164976e74..fbdf3d7ca70d 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -537,4 +537,6 @@ struct arm_smmu_device *qcom_adreno_smmu_impl_init(struct arm_smmu_device *smmu)
 void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx);
 int arm_mmu500_reset(struct arm_smmu_device *smmu);
 
+struct iommu_domain *arm_smmu_alloc_identity_domain(struct arm_smmu_device *smmu);
+
 #endif /* _ARM_SMMU_H */
-- 
2.28.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 8/8] iommu/arm-smmu-qcom: Setup identity domain for boot mappings
  2020-09-04 15:55 [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Bjorn Andersson
                   ` (6 preceding siblings ...)
  2020-09-04 15:55 ` [PATCH v3 7/8] iommu/arm-smmu: Provide helper for allocating identity domain Bjorn Andersson
@ 2020-09-04 15:55 ` Bjorn Andersson
  2020-09-11  8:29   ` Sai Prakash Ranjan
  2020-09-11 17:29   ` Robin Murphy
  2020-09-05 22:27 ` [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Rob Clark
                   ` (5 subsequent siblings)
  13 siblings, 2 replies; 33+ messages in thread
From: Bjorn Andersson @ 2020-09-04 15:55 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Joerg Roedel, Bjorn Andersson,
	Sai Prakash Ranjan, Jordan Crouse, Rob Clark
  Cc: linux-arm-msm, iommu, Sibi Sankar, linux-arm-kernel, linux-kernel

With many Qualcomm platforms not having functional S2CR BYPASS a
temporary IOMMU domain, without translation, needs to be allocated in
order to allow these memory transactions.

Unfortunately the boot loader uses the first few context banks, so
rather than overwriting a active bank the last context bank is used and
streams are diverted here during initialization.

This also performs the readback of SMR registers for the Qualcomm
platform, to trigger the mechanism.

This is based on prior work by Thierry Reding and Laurentiu Tudor.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- Combined from pieces spread between the Qualcomm impl and generic code in v2.
- Moved to use the newly introduced inherit_mapping op.

 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 70a1eaa52e14..a54302190932 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -12,6 +12,7 @@
 struct qcom_smmu {
 	struct arm_smmu_device smmu;
 	bool bypass_broken;
+	struct iommu_domain *identity;
 };
 
 static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
@@ -228,6 +229,37 @@ static int qcom_smmu_cfg_probe(struct arm_smmu_device *smmu)
 	return 0;
 }
 
+static int qcom_smmu_inherit_mappings(struct arm_smmu_device *smmu)
+{
+	struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
+	int cbndx;
+	u32 smr;
+	int i;
+
+	qsmmu->identity = arm_smmu_alloc_identity_domain(smmu);
+	if (IS_ERR(qsmmu->identity))
+		return PTR_ERR(qsmmu->identity);
+
+	cbndx = to_smmu_domain(qsmmu->identity)->cfg.cbndx;
+
+	for (i = 0; i < smmu->num_mapping_groups; i++) {
+		smr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_SMR(i));
+
+		if (FIELD_GET(ARM_SMMU_SMR_VALID, smr)) {
+			smmu->smrs[i].id = FIELD_GET(ARM_SMMU_SMR_ID, smr);
+			smmu->smrs[i].mask = FIELD_GET(ARM_SMMU_SMR_MASK, smr);
+			smmu->smrs[i].valid = true;
+
+			smmu->s2crs[i].type = S2CR_TYPE_TRANS;
+			smmu->s2crs[i].privcfg = S2CR_PRIVCFG_DEFAULT;
+			smmu->s2crs[i].cbndx = cbndx;
+			smmu->s2crs[i].count++;
+		}
+	}
+
+	return 0;
+}
+
 static int qcom_smmu_def_domain_type(struct device *dev)
 {
 	const struct of_device_id *match =
@@ -270,6 +302,7 @@ static const struct arm_smmu_impl qcom_smmu_impl = {
 	.cfg_probe = qcom_smmu_cfg_probe,
 	.def_domain_type = qcom_smmu_def_domain_type,
 	.reset = qcom_smmu500_reset,
+	.inherit_mappings = qcom_smmu_inherit_mappings,
 };
 
 static const struct arm_smmu_impl qcom_adreno_smmu_impl = {
-- 
2.28.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings
  2020-09-04 15:55 [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Bjorn Andersson
                   ` (7 preceding siblings ...)
  2020-09-04 15:55 ` [PATCH v3 8/8] iommu/arm-smmu-qcom: Setup identity domain for boot mappings Bjorn Andersson
@ 2020-09-05 22:27 ` Rob Clark
  2020-09-09 14:46 ` Laurentiu Tudor
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 33+ messages in thread
From: Rob Clark @ 2020-09-05 22:27 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Clark, Will Deacon,
	list@263.net:IOMMU DRIVERS
	<iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>, ,
	Linux Kernel Mailing List, Sibi Sankar, linux-arm-msm,
	Robin Murphy,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Fri, Sep 4, 2020 at 8:55 AM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> Based on previous attempts and discussions this is the latest attempt at
> inheriting stream mappings set up by the bootloader, for e.g. boot splash or
> efifb.
>
> Per Will's request this builds on the work by Jordan and Rob for the Adreno
> SMMU support. It applies cleanly ontop of v16 of their series, which can be
> found at
> https://lore.kernel.org/linux-arm-msm/20200901164707.2645413-1-robdclark@gmail.com/
>
> Bjorn Andersson (8):
>   iommu/arm-smmu: Refactor context bank allocation
>   iommu/arm-smmu: Delay modifying domain during init
>   iommu/arm-smmu: Consult context bank allocator for identify domains
>   iommu/arm-smmu-qcom: Emulate bypass by using context banks
>   iommu/arm-smmu-qcom: Consistently initialize stream mappings
>   iommu/arm-smmu: Add impl hook for inherit boot mappings
>   iommu/arm-smmu: Provide helper for allocating identity domain
>   iommu/arm-smmu-qcom: Setup identity domain for boot mappings

I have squashed 1/8 into v17 of the adreno-smmu series as suggested by
Bjorn, the remainder are:

Reviewed-by: Rob Clark <robdclark@gmail.com>

and on the lenovo c630,

Tested-by: Rob Clark <robdclark@gmail.com>

>  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 111 ++++++++++++++++++-
>  drivers/iommu/arm/arm-smmu/arm-smmu.c      | 122 ++++++++++++++-------
>  drivers/iommu/arm/arm-smmu/arm-smmu.h      |  14 ++-
>  3 files changed, 205 insertions(+), 42 deletions(-)
>
> --
> 2.28.0
>
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 1/8] iommu/arm-smmu: Refactor context bank allocation
  2020-09-04 15:55 ` [PATCH v3 1/8] iommu/arm-smmu: Refactor context bank allocation Bjorn Andersson
@ 2020-09-08 18:42   ` Jordan Crouse
  2020-09-08 18:46     ` Jordan Crouse
  2020-09-11  8:18   ` Sai Prakash Ranjan
  1 sibling, 1 reply; 33+ messages in thread
From: Jordan Crouse @ 2020-09-08 18:42 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Clark, Will Deacon, linux-kernel, iommu, Sibi Sankar,
	linux-arm-msm, Robin Murphy, linux-arm-kernel

On Fri, Sep 04, 2020 at 03:55:06PM +0000, Bjorn Andersson wrote:
> Extract the conditional invocation of the platform defined
> alloc_context_bank() to a separate function to keep
> arm_smmu_init_domain_context() cleaner.
> 
> Instead pass a reference to the arm_smmu_device as parameter to the
> call. Also remove the count parameter, as this can be read from the
> newly passed object.
> 
> This allows us to not assign smmu_domain->smmu before attempting to
> allocate the context bank and as such we don't need to roll back this
> assignment on failure.

Much nicer.

Reviewed-by: Jordan Crouse <jcrouse@codeaurora.org>

> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> Note that this series applies ontop of:
> https://lore.kernel.org/linux-arm-msm/20200901164707.2645413-1-robdclark@gmail.com/
> 
> This could either go on its own, or be squashed with "[PATCH v16 14/20]
> iommu/arm-smmu: Prepare for the adreno-smmu implementation" from Rob's series.
> 
> Changes since v2:
> - New patch
> 
>  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c |  6 ++++--
>  drivers/iommu/arm/arm-smmu/arm-smmu.c      | 23 ++++++++++++----------
>  drivers/iommu/arm/arm-smmu/arm-smmu.h      |  3 ++-
>  3 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 2aa6249050ff..0663d7d26908 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -91,9 +91,10 @@ static int qcom_adreno_smmu_set_ttbr0_cfg(const void *cookie,
>  }
>  
>  static int qcom_adreno_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_domain,
> -		struct device *dev, int start, int count)
> +					       struct arm_smmu_device *smmu,
> +					       struct device *dev, int start)
>  {
> -	struct arm_smmu_device *smmu = smmu_domain->smmu;
> +	int count;
>  
>  	/*
>  	 * Assign context bank 0 to the GPU device so the GPU hardware can
> @@ -104,6 +105,7 @@ static int qcom_adreno_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_doma
>  		count = 1;
>  	} else {
>  		start = 1;
> +		count = smmu->num_context_banks;
>  	}
>  
>  	return __arm_smmu_alloc_bitmap(smmu->context_map, start, count);
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index bbec5793faf8..e19d7bdc7674 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -623,6 +623,16 @@ void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx)
>  	arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_SCTLR, reg);
>  }
>  
> +static int arm_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_domain,
> +				       struct arm_smmu_device *smmu,
> +				       struct device *dev, unsigned int start)
> +{
> +	if (smmu->impl && smmu->impl->alloc_context_bank)
> +		return smmu->impl->alloc_context_bank(smmu_domain, smmu, dev, start);
> +
> +	return __arm_smmu_alloc_bitmap(smmu->context_map, start, smmu->num_context_banks);
> +}
> +
>  static int arm_smmu_init_domain_context(struct iommu_domain *domain,
>  					struct arm_smmu_device *smmu,
>  					struct device *dev)
> @@ -741,20 +751,13 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
>  		goto out_unlock;
>  	}
>  
> -	smmu_domain->smmu = smmu;
> -
> -	if (smmu->impl && smmu->impl->alloc_context_bank)
> -		ret = smmu->impl->alloc_context_bank(smmu_domain, dev,
> -				start, smmu->num_context_banks);
> -	else
> -		ret = __arm_smmu_alloc_bitmap(smmu->context_map, start,
> -				      smmu->num_context_banks);
> -
> +	ret = arm_smmu_alloc_context_bank(smmu_domain, smmu, dev, start);
>  	if (ret < 0) {
> -		smmu_domain->smmu = NULL;
>  		goto out_unlock;
>  	}
>  
> +	smmu_domain->smmu = smmu;
> +
>  	cfg->cbndx = ret;
>  	if (smmu->version < ARM_SMMU_V2) {
>  		cfg->irptndx = atomic_inc_return(&smmu->irptndx);
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> index 2df3a70a8a41..ddf2ca4c923d 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> @@ -437,7 +437,8 @@ struct arm_smmu_impl {
>  	irqreturn_t (*global_fault)(int irq, void *dev);
>  	irqreturn_t (*context_fault)(int irq, void *dev);
>  	int (*alloc_context_bank)(struct arm_smmu_domain *smmu_domain,
> -			struct device *dev, int start, int max);
> +				  struct arm_smmu_device *smmu,
> +				  struct device *dev, int start);
>  };
>  
>  #define INVALID_SMENDX			-1
> -- 
> 2.28.0
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 1/8] iommu/arm-smmu: Refactor context bank allocation
  2020-09-08 18:42   ` Jordan Crouse
@ 2020-09-08 18:46     ` Jordan Crouse
  0 siblings, 0 replies; 33+ messages in thread
From: Jordan Crouse @ 2020-09-08 18:46 UTC (permalink / raw)
  To: Bjorn Andersson, Will Deacon, Robin Murphy, Joerg Roedel,
	Sai Prakash Ranjan, Rob Clark, Sibi Sankar, linux-arm-kernel,
	iommu, linux-kernel, linux-arm-msm

On Tue, Sep 08, 2020 at 06:42:06PM +0000, Jordan Crouse wrote:
> On Fri, Sep 04, 2020 at 03:55:06PM +0000, Bjorn Andersson wrote:
> > Extract the conditional invocation of the platform defined
> > alloc_context_bank() to a separate function to keep
> > arm_smmu_init_domain_context() cleaner.
> > 
> > Instead pass a reference to the arm_smmu_device as parameter to the
> > call. Also remove the count parameter, as this can be read from the
> > newly passed object.
> > 
> > This allows us to not assign smmu_domain->smmu before attempting to
> > allocate the context bank and as such we don't need to roll back this
> > assignment on failure.
> 
> Much nicer.
> 
> Reviewed-by: Jordan Crouse <jcrouse@codeaurora.org>

I didn't notice that Rob had grabbed this one for his stack. That's fine too.

> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> > 
> > Note that this series applies ontop of:
> > https://lore.kernel.org/linux-arm-msm/20200901164707.2645413-1-robdclark@gmail.com/
> > 
> > This could either go on its own, or be squashed with "[PATCH v16 14/20]
> > iommu/arm-smmu: Prepare for the adreno-smmu implementation" from Rob's series.
> > 
> > Changes since v2:
> > - New patch
> > 
> >  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c |  6 ++++--
> >  drivers/iommu/arm/arm-smmu/arm-smmu.c      | 23 ++++++++++++----------
> >  drivers/iommu/arm/arm-smmu/arm-smmu.h      |  3 ++-
> >  3 files changed, 19 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> > index 2aa6249050ff..0663d7d26908 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> > @@ -91,9 +91,10 @@ static int qcom_adreno_smmu_set_ttbr0_cfg(const void *cookie,
> >  }
> >  
> >  static int qcom_adreno_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_domain,
> > -		struct device *dev, int start, int count)
> > +					       struct arm_smmu_device *smmu,
> > +					       struct device *dev, int start)
> >  {
> > -	struct arm_smmu_device *smmu = smmu_domain->smmu;
> > +	int count;
> >  
> >  	/*
> >  	 * Assign context bank 0 to the GPU device so the GPU hardware can
> > @@ -104,6 +105,7 @@ static int qcom_adreno_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_doma
> >  		count = 1;
> >  	} else {
> >  		start = 1;
> > +		count = smmu->num_context_banks;
> >  	}
> >  
> >  	return __arm_smmu_alloc_bitmap(smmu->context_map, start, count);
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > index bbec5793faf8..e19d7bdc7674 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > @@ -623,6 +623,16 @@ void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx)
> >  	arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_SCTLR, reg);
> >  }
> >  
> > +static int arm_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_domain,
> > +				       struct arm_smmu_device *smmu,
> > +				       struct device *dev, unsigned int start)
> > +{
> > +	if (smmu->impl && smmu->impl->alloc_context_bank)
> > +		return smmu->impl->alloc_context_bank(smmu_domain, smmu, dev, start);
> > +
> > +	return __arm_smmu_alloc_bitmap(smmu->context_map, start, smmu->num_context_banks);
> > +}
> > +
> >  static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> >  					struct arm_smmu_device *smmu,
> >  					struct device *dev)
> > @@ -741,20 +751,13 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> >  		goto out_unlock;
> >  	}
> >  
> > -	smmu_domain->smmu = smmu;
> > -
> > -	if (smmu->impl && smmu->impl->alloc_context_bank)
> > -		ret = smmu->impl->alloc_context_bank(smmu_domain, dev,
> > -				start, smmu->num_context_banks);
> > -	else
> > -		ret = __arm_smmu_alloc_bitmap(smmu->context_map, start,
> > -				      smmu->num_context_banks);
> > -
> > +	ret = arm_smmu_alloc_context_bank(smmu_domain, smmu, dev, start);
> >  	if (ret < 0) {
> > -		smmu_domain->smmu = NULL;
> >  		goto out_unlock;
> >  	}
> >  
> > +	smmu_domain->smmu = smmu;
> > +
> >  	cfg->cbndx = ret;
> >  	if (smmu->version < ARM_SMMU_V2) {
> >  		cfg->irptndx = atomic_inc_return(&smmu->irptndx);
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > index 2df3a70a8a41..ddf2ca4c923d 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > @@ -437,7 +437,8 @@ struct arm_smmu_impl {
> >  	irqreturn_t (*global_fault)(int irq, void *dev);
> >  	irqreturn_t (*context_fault)(int irq, void *dev);
> >  	int (*alloc_context_bank)(struct arm_smmu_domain *smmu_domain,
> > -			struct device *dev, int start, int max);
> > +				  struct arm_smmu_device *smmu,
> > +				  struct device *dev, int start);
> >  };
> >  
> >  #define INVALID_SMENDX			-1
> > -- 
> > 2.28.0
> > 
> 
> -- 
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings
  2020-09-04 15:55 [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Bjorn Andersson
                   ` (8 preceding siblings ...)
  2020-09-05 22:27 ` [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Rob Clark
@ 2020-09-09 14:46 ` Laurentiu Tudor
  2020-09-10 22:56 ` John Stultz
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 33+ messages in thread
From: Laurentiu Tudor @ 2020-09-09 14:46 UTC (permalink / raw)
  To: Bjorn Andersson, Will Deacon, Robin Murphy, Joerg Roedel,
	Sai Prakash Ranjan, Jordan Crouse, Rob Clark
  Cc: linux-arm-msm, iommu, Sibi Sankar, linux-arm-kernel, linux-kernel

Hi Bjorn,

On 9/4/2020 6:55 PM, Bjorn Andersson wrote:
> Based on previous attempts and discussions this is the latest attempt at
> inheriting stream mappings set up by the bootloader, for e.g. boot splash or
> efifb.
> 
> Per Will's request this builds on the work by Jordan and Rob for the Adreno
> SMMU support. It applies cleanly ontop of v16 of their series, which can be
> found at
> https://lore.kernel.org/linux-arm-msm/20200901164707.2645413-1-robdclark@gmail.com/

Is there a git repo available with all the patches put together?

---
Thanks & Best Regards, Laurentiu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings
  2020-09-04 15:55 [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Bjorn Andersson
                   ` (9 preceding siblings ...)
  2020-09-09 14:46 ` Laurentiu Tudor
@ 2020-09-10 22:56 ` John Stultz
  2020-09-11  8:16 ` Sai Prakash Ranjan
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 33+ messages in thread
From: John Stultz @ 2020-09-10 22:56 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Clark, Will Deacon, iommu, lkml, Sibi Sankar, linux-arm-msm,
	Robin Murphy, linux-arm-kernel

On Fri, Sep 4, 2020 at 8:56 AM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> Based on previous attempts and discussions this is the latest attempt at
> inheriting stream mappings set up by the bootloader, for e.g. boot splash or
> efifb.
>
> Per Will's request this builds on the work by Jordan and Rob for the Adreno
> SMMU support. It applies cleanly ontop of v16 of their series, which can be
> found at
> https://lore.kernel.org/linux-arm-msm/20200901164707.2645413-1-robdclark@gmail.com/
>

Apologies, I just found this today. I've pulled your patches and Rob's
into my own tree here:
  https://git.linaro.org/people/john.stultz/android-dev.git/log/?h=dev/db845c-mainline-WIP

And they all work fine on the db845c.

So for your whole series:
Tested-by: John Stultz <john.stultz@linaro.org>

thanks
-john
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings
  2020-09-04 15:55 [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Bjorn Andersson
                   ` (10 preceding siblings ...)
  2020-09-10 22:56 ` John Stultz
@ 2020-09-11  8:16 ` Sai Prakash Ranjan
  2020-09-11 16:10 ` Amit Pundir
  2020-09-16 10:09 ` Laurentiu Tudor
  13 siblings, 0 replies; 33+ messages in thread
From: Sai Prakash Ranjan @ 2020-09-11  8:16 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Clark, Will Deacon, iommu, linux-kernel, Sibi Sankar,
	linux-arm-msm, Robin Murphy, linux-arm-kernel

Hi Bjorn,

On 2020-09-04 21:25, Bjorn Andersson wrote:
> Based on previous attempts and discussions this is the latest attempt 
> at
> inheriting stream mappings set up by the bootloader, for e.g. boot 
> splash or
> efifb.
> 
> Per Will's request this builds on the work by Jordan and Rob for the 
> Adreno
> SMMU support. It applies cleanly ontop of v16 of their series, which 
> can be
> found at
> https://lore.kernel.org/linux-arm-msm/20200901164707.2645413-1-robdclark@gmail.com/
> 

Thanks for working on this, I have tested this on qcom platforms
where firmware does these shenanigans(most android) and this series
works well and where firmware doesn't do all this (chrome) and no
regressions there. Review and test tags given on individual patches.

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 1/8] iommu/arm-smmu: Refactor context bank allocation
  2020-09-04 15:55 ` [PATCH v3 1/8] iommu/arm-smmu: Refactor context bank allocation Bjorn Andersson
  2020-09-08 18:42   ` Jordan Crouse
@ 2020-09-11  8:18   ` Sai Prakash Ranjan
  1 sibling, 0 replies; 33+ messages in thread
From: Sai Prakash Ranjan @ 2020-09-11  8:18 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Clark, Will Deacon, iommu, linux-kernel, Sibi Sankar,
	linux-arm-msm, Robin Murphy, linux-arm-kernel

On 2020-09-04 21:25, Bjorn Andersson wrote:
> Extract the conditional invocation of the platform defined
> alloc_context_bank() to a separate function to keep
> arm_smmu_init_domain_context() cleaner.
> 
> Instead pass a reference to the arm_smmu_device as parameter to the
> call. Also remove the count parameter, as this can be read from the
> newly passed object.
> 
> This allows us to not assign smmu_domain->smmu before attempting to
> allocate the context bank and as such we don't need to roll back this
> assignment on failure.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---

Reviewed-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 2/8] iommu/arm-smmu: Delay modifying domain during init
  2020-09-04 15:55 ` [PATCH v3 2/8] iommu/arm-smmu: Delay modifying domain during init Bjorn Andersson
@ 2020-09-11  8:20   ` Sai Prakash Ranjan
  0 siblings, 0 replies; 33+ messages in thread
From: Sai Prakash Ranjan @ 2020-09-11  8:20 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Clark, Will Deacon, iommu, linux-kernel, Sibi Sankar,
	linux-arm-msm, Robin Murphy, linux-arm-kernel

On 2020-09-04 21:25, Bjorn Andersson wrote:
> Delay modifications to the domain during arm_smmu_init_domain_context()
> until we've allocated a context bank. This will allow us to postpone 
> the
> special handling of identity domains until the platform specific 
> context
> bank allocator has been executed, in a later patch.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---

Reviewed-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 3/8] iommu/arm-smmu: Consult context bank allocator for identify domains
  2020-09-04 15:55 ` [PATCH v3 3/8] iommu/arm-smmu: Consult context bank allocator for identify domains Bjorn Andersson
@ 2020-09-11  8:21   ` Sai Prakash Ranjan
  2020-09-11  8:24   ` Sai Prakash Ranjan
  1 sibling, 0 replies; 33+ messages in thread
From: Sai Prakash Ranjan @ 2020-09-11  8:21 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Clark, Will Deacon, iommu, linux-kernel, Sibi Sankar,
	linux-arm-msm, Robin Murphy, linux-arm-kernel

On 2020-09-04 21:25, Bjorn Andersson wrote:
> For implementations of the ARM SMMU where stream mappings of bypass 
> type
> are prohibited identity domains can be implemented by using context
> banks with translation disabled.
> 
> Postpone the decision to skip allocating a context bank until the
> implementation specific context bank allocator has been consulted and 
> if
> it decides to use a context bank for the identity map, don't enable
> translation (i.e. omit ARM_SMMU_SCTLR_M).
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 

Minor nit in the subject: identify -> identity

Reviewed-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 3/8] iommu/arm-smmu: Consult context bank allocator for identify domains
  2020-09-04 15:55 ` [PATCH v3 3/8] iommu/arm-smmu: Consult context bank allocator for identify domains Bjorn Andersson
  2020-09-11  8:21   ` Sai Prakash Ranjan
@ 2020-09-11  8:24   ` Sai Prakash Ranjan
  1 sibling, 0 replies; 33+ messages in thread
From: Sai Prakash Ranjan @ 2020-09-11  8:24 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Clark, Will Deacon, iommu, linux-kernel, Sibi Sankar,
	linux-arm-msm, Robin Murphy, linux-arm-kernel

On 2020-09-04 21:25, Bjorn Andersson wrote:
> For implementations of the ARM SMMU where stream mappings of bypass 
> type
> are prohibited identity domains can be implemented by using context
> banks with translation disabled.
> 
> Postpone the decision to skip allocating a context bank until the
> implementation specific context bank allocator has been consulted and 
> if
> it decides to use a context bank for the identity map, don't enable
> translation (i.e. omit ARM_SMMU_SCTLR_M).
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 

<snip>...

> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> index ddf2ca4c923d..235d9a3a6ab6 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> @@ -243,6 +243,8 @@ enum arm_smmu_cbar_type {
>  #define TLB_LOOP_TIMEOUT		1000000	/* 1s! */
>  #define TLB_SPIN_COUNT			10
> 
> +#define ARM_SMMU_CBNDX_BYPASS		0xffff
> +
>  /* Shared driver definitions */
>  enum arm_smmu_arch_version {
>  	ARM_SMMU_V1,
> @@ -346,6 +348,7 @@ struct arm_smmu_cfg {
>  	u32				sctlr_clr;    /* bits to mask in SCTLR */
>  	enum arm_smmu_cbar_type		cbar;
>  	enum arm_smmu_context_fmt	fmt;
> +	bool				m;

Can we use mmu_enable instead of m here to be more descriptive?

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 4/8] iommu/arm-smmu-qcom: Emulate bypass by using context banks
  2020-09-04 15:55 ` [PATCH v3 4/8] iommu/arm-smmu-qcom: Emulate bypass by using context banks Bjorn Andersson
@ 2020-09-11  8:25   ` Sai Prakash Ranjan
  0 siblings, 0 replies; 33+ messages in thread
From: Sai Prakash Ranjan @ 2020-09-11  8:25 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Clark, Will Deacon, iommu, linux-kernel, Sibi Sankar,
	linux-arm-msm, Robin Murphy, linux-arm-kernel

On 2020-09-04 21:25, Bjorn Andersson wrote:
> Some firmware found on various Qualcomm platforms traps writes to S2CR
> of type BYPASS and writes FAULT into the register. In particular, this
> prevents us from marking the streams for the display controller as
> BYPASS to allow continued scanout of the screen through the
> initialization of the ARM SMMU.
> 
> This adds a Qualcomm specific cfg_probe function, which probes for the
> broken behavior of the S2CR registers and implements a custom
> alloc_context_bank() that when necessary allocates a context bank
> (without translation) for these domains as well.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> Changes since v2:
> - Move quirk from arm_smmudevice to qcom_smmu, as we localize the quirk
>   handling to the Qualcomm specific implemntation.
> 
>  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 52 ++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 229fc8ff8cea..284761a1cd8e 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -11,8 +11,14 @@
> 
>  struct qcom_smmu {
>  	struct arm_smmu_device smmu;
> +	bool bypass_broken;
>  };
> 
> +static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
> +{
> +	return container_of(smmu, struct qcom_smmu, smmu);
> +}
> +
>  #define QCOM_ADRENO_SMMU_GPU_SID 0
> 
>  static bool qcom_adreno_smmu_is_gpu_device(struct device *dev)
> @@ -162,6 +168,50 @@ static const struct of_device_id
> qcom_smmu_client_of_match[] __maybe_unused = {
>  	{ }
>  };
> 
> +static int qcom_smmu_alloc_context_bank(struct arm_smmu_domain 
> *smmu_domain,
> +					struct arm_smmu_device *smmu,
> +					struct device *dev, int start)
> +{
> +	struct iommu_domain *domain = &smmu_domain->domain;
> +	struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
> +
> +	/* Keep identity domains as bypass, unless bypass is broken */
> +	if (domain->type == IOMMU_DOMAIN_IDENTITY && !qsmmu->bypass_broken)
> +		return ARM_SMMU_CBNDX_BYPASS;
> +
> +	/*
> +	 * The identity domain to emulate bypass is the only domain without a
> +	 * dev, use the last context bank for this to avoid collisions with
> +	 * active contexts during initialization.
> +	 */
> +	if (!dev)
> +		start = smmu->num_context_banks - 1;
> +
> +	return __arm_smmu_alloc_bitmap(smmu->context_map, start,
> smmu->num_context_banks);
> +}
> +
> +static int qcom_smmu_cfg_probe(struct arm_smmu_device *smmu)
> +{
> +	unsigned int last_s2cr = ARM_SMMU_GR0_S2CR(smmu->num_mapping_groups - 
> 1);
> +	struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
> +	u32 reg;
> +
> +	/*
> +	 * With some firmware writes to S2CR of type FAULT are ignored, and
> +	 * writing BYPASS will end up as FAULT in the register. Perform a 
> write
> +	 * to S2CR to detect if this is the case with the current firmware.
> +	 */
> +	reg = FIELD_PREP(ARM_SMMU_S2CR_TYPE, S2CR_TYPE_BYPASS) |
> +	      FIELD_PREP(ARM_SMMU_S2CR_CBNDX, 0xff) |
> +	      FIELD_PREP(ARM_SMMU_S2CR_PRIVCFG, S2CR_PRIVCFG_DEFAULT);
> +	arm_smmu_gr0_write(smmu, last_s2cr, reg);
> +	reg = arm_smmu_gr0_read(smmu, last_s2cr);
> +	if (FIELD_GET(ARM_SMMU_S2CR_TYPE, reg) != S2CR_TYPE_BYPASS)
> +		qsmmu->bypass_broken = true;
> +

Clever :)

Reviewed-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 5/8] iommu/arm-smmu-qcom: Consistently initialize stream mappings
  2020-09-04 15:55 ` [PATCH v3 5/8] iommu/arm-smmu-qcom: Consistently initialize stream mappings Bjorn Andersson
@ 2020-09-11  8:26   ` Sai Prakash Ranjan
  0 siblings, 0 replies; 33+ messages in thread
From: Sai Prakash Ranjan @ 2020-09-11  8:26 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Clark, Will Deacon, iommu, linux-kernel, Sibi Sankar,
	linux-arm-msm, Robin Murphy, linux-arm-kernel

On 2020-09-04 21:25, Bjorn Andersson wrote:
> Firmware that traps writes to S2CR to translate BYPASS into FAULT also
> ignores writes of type FAULT. As such booting with "disable_bypass" set
> will result in all S2CR registers left as configured by the bootloader.
> 
> This has been seen to result in indeterministic results, as these
> mappings might linger and reference context banks that Linux is
> reconfiguring.
> 
> Use the fact that BYPASS writes result in FAULT type to force all 
> stream
> mappings to FAULT.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---

Reviewed-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>


-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 6/8] iommu/arm-smmu: Add impl hook for inherit boot mappings
  2020-09-04 15:55 ` [PATCH v3 6/8] iommu/arm-smmu: Add impl hook for inherit boot mappings Bjorn Andersson
@ 2020-09-11  8:27   ` Sai Prakash Ranjan
  2020-09-11 17:13   ` Robin Murphy
  1 sibling, 0 replies; 33+ messages in thread
From: Sai Prakash Ranjan @ 2020-09-11  8:27 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Clark, Will Deacon, iommu, linux-kernel, Sibi Sankar,
	linux-arm-msm, Robin Murphy, linux-arm-kernel

On 2020-09-04 21:25, Bjorn Andersson wrote:
> Add a new operation to allow platform implementations to inherit any
> stream mappings from the boot loader.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 

Reviewed-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 7/8] iommu/arm-smmu: Provide helper for allocating identity domain
  2020-09-04 15:55 ` [PATCH v3 7/8] iommu/arm-smmu: Provide helper for allocating identity domain Bjorn Andersson
@ 2020-09-11  8:28   ` Sai Prakash Ranjan
  0 siblings, 0 replies; 33+ messages in thread
From: Sai Prakash Ranjan @ 2020-09-11  8:28 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Clark, Will Deacon, iommu, linux-kernel, Sibi Sankar,
	linux-arm-msm, Robin Murphy, linux-arm-kernel

On 2020-09-04 21:25, Bjorn Andersson wrote:
> Some platform implementations needs to be able to allocate a domain for
> emulating identity mappings using a context bank without translation.
> Provide a helper function to allocate such a domain.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 

Reviewed-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 8/8] iommu/arm-smmu-qcom: Setup identity domain for boot mappings
  2020-09-04 15:55 ` [PATCH v3 8/8] iommu/arm-smmu-qcom: Setup identity domain for boot mappings Bjorn Andersson
@ 2020-09-11  8:29   ` Sai Prakash Ranjan
  2020-09-11 17:29   ` Robin Murphy
  1 sibling, 0 replies; 33+ messages in thread
From: Sai Prakash Ranjan @ 2020-09-11  8:29 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Clark, Will Deacon, iommu, linux-kernel, Sibi Sankar,
	linux-arm-msm, Robin Murphy, linux-arm-kernel

On 2020-09-04 21:25, Bjorn Andersson wrote:
> With many Qualcomm platforms not having functional S2CR BYPASS a
> temporary IOMMU domain, without translation, needs to be allocated in
> order to allow these memory transactions.
> 
> Unfortunately the boot loader uses the first few context banks, so
> rather than overwriting a active bank the last context bank is used and
> streams are diverted here during initialization.
> 
> This also performs the readback of SMR registers for the Qualcomm
> platform, to trigger the mechanism.
> 
> This is based on prior work by Thierry Reding and Laurentiu Tudor.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 

Reviewed-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings
  2020-09-04 15:55 [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Bjorn Andersson
                   ` (11 preceding siblings ...)
  2020-09-11  8:16 ` Sai Prakash Ranjan
@ 2020-09-11 16:10 ` Amit Pundir
  2020-09-16 10:09 ` Laurentiu Tudor
  13 siblings, 0 replies; 33+ messages in thread
From: Amit Pundir @ 2020-09-11 16:10 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Clark, Will Deacon, iommu, lkml, Sibi Sankar, linux-arm-msm,
	Robin Murphy, linux-arm-kernel

On Fri, 4 Sep 2020 at 21:25, Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
>
> Based on previous attempts and discussions this is the latest attempt at
> inheriting stream mappings set up by the bootloader, for e.g. boot splash or
> efifb.
>
> Per Will's request this builds on the work by Jordan and Rob for the Adreno
> SMMU support. It applies cleanly ontop of v16 of their series, which can be
> found at
> https://lore.kernel.org/linux-arm-msm/20200901164707.2645413-1-robdclark@gmail.com/
>

Boot tested the series on Xiaomi Poco F1 phone (sdm845)

Tested-by: Amit Pundir <amit.pundir@linaro.org>

> Bjorn Andersson (8):
>   iommu/arm-smmu: Refactor context bank allocation
>   iommu/arm-smmu: Delay modifying domain during init
>   iommu/arm-smmu: Consult context bank allocator for identify domains
>   iommu/arm-smmu-qcom: Emulate bypass by using context banks
>   iommu/arm-smmu-qcom: Consistently initialize stream mappings
>   iommu/arm-smmu: Add impl hook for inherit boot mappings
>   iommu/arm-smmu: Provide helper for allocating identity domain
>   iommu/arm-smmu-qcom: Setup identity domain for boot mappings
>
>  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 111 ++++++++++++++++++-
>  drivers/iommu/arm/arm-smmu/arm-smmu.c      | 122 ++++++++++++++-------
>  drivers/iommu/arm/arm-smmu/arm-smmu.h      |  14 ++-
>  3 files changed, 205 insertions(+), 42 deletions(-)
>
> --
> 2.28.0
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 6/8] iommu/arm-smmu: Add impl hook for inherit boot mappings
  2020-09-04 15:55 ` [PATCH v3 6/8] iommu/arm-smmu: Add impl hook for inherit boot mappings Bjorn Andersson
  2020-09-11  8:27   ` Sai Prakash Ranjan
@ 2020-09-11 17:13   ` Robin Murphy
  2020-09-13  3:25     ` Bjorn Andersson
  1 sibling, 1 reply; 33+ messages in thread
From: Robin Murphy @ 2020-09-11 17:13 UTC (permalink / raw)
  To: Bjorn Andersson, Will Deacon, Joerg Roedel, Sai Prakash Ranjan,
	Jordan Crouse, Rob Clark
  Cc: linux-arm-msm, iommu, Sibi Sankar, linux-arm-kernel, linux-kernel

On 2020-09-04 16:55, Bjorn Andersson wrote:
> Add a new operation to allow platform implementations to inherit any
> stream mappings from the boot loader.

Is there a reason we need an explicit step for this? The aim of the 
cfg_probe hook is that the SMMU software state should all be set up by 
then, and you can mess about with it however you like before 
arm_smmu_reset() actually commits anything to hardware. I would have 
thought you could permanently steal a context bank, configure it as your 
bypass hole, read out the previous SME configuration and tweak 
smmu->smrs and smmu->s2crs appropriately all together "invisibly" at 
that point. If that can't work, I'm very curious as to what I've overlooked.

Robin.

> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> Changes since v2:
> - New patch/interface
> 
>   drivers/iommu/arm/arm-smmu/arm-smmu.c | 11 ++++++-----
>   drivers/iommu/arm/arm-smmu/arm-smmu.h |  6 ++++++
>   2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index eb5c6ca5c138..4c4d302cd747 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -85,11 +85,6 @@ static inline void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
>   		pm_runtime_put_autosuspend(smmu->dev);
>   }
>   
> -static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
> -{
> -	return container_of(dom, struct arm_smmu_domain, domain);
> -}
> -
>   static struct platform_driver arm_smmu_driver;
>   static struct iommu_ops arm_smmu_ops;
>   
> @@ -2188,6 +2183,12 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>   	if (err)
>   		return err;
>   
> +	if (smmu->impl->inherit_mappings) {
> +		err = smmu->impl->inherit_mappings(smmu);
> +		if (err)
> +			return err;
> +	}
> +
>   	if (smmu->version == ARM_SMMU_V2) {
>   		if (smmu->num_context_banks > smmu->num_context_irqs) {
>   			dev_err(dev,
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> index 235d9a3a6ab6..f58164976e74 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> @@ -378,6 +378,11 @@ struct arm_smmu_domain {
>   	struct iommu_domain		domain;
>   };
>   
> +static inline struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
> +{
> +	return container_of(dom, struct arm_smmu_domain, domain);
> +}
> +
>   struct arm_smmu_master_cfg {
>   	struct arm_smmu_device		*smmu;
>   	s16				smendx[];
> @@ -442,6 +447,7 @@ struct arm_smmu_impl {
>   	int (*alloc_context_bank)(struct arm_smmu_domain *smmu_domain,
>   				  struct arm_smmu_device *smmu,
>   				  struct device *dev, int start);
> +	int (*inherit_mappings)(struct arm_smmu_device *smmu);
>   };
>   
>   #define INVALID_SMENDX			-1
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 8/8] iommu/arm-smmu-qcom: Setup identity domain for boot mappings
  2020-09-04 15:55 ` [PATCH v3 8/8] iommu/arm-smmu-qcom: Setup identity domain for boot mappings Bjorn Andersson
  2020-09-11  8:29   ` Sai Prakash Ranjan
@ 2020-09-11 17:29   ` Robin Murphy
  1 sibling, 0 replies; 33+ messages in thread
From: Robin Murphy @ 2020-09-11 17:29 UTC (permalink / raw)
  To: Bjorn Andersson, Will Deacon, Joerg Roedel, Sai Prakash Ranjan,
	Jordan Crouse, Rob Clark
  Cc: linux-arm-msm, iommu, Sibi Sankar, linux-arm-kernel, linux-kernel

On 2020-09-04 16:55, Bjorn Andersson wrote:
> With many Qualcomm platforms not having functional S2CR BYPASS a
> temporary IOMMU domain, without translation, needs to be allocated in
> order to allow these memory transactions.
> 
> Unfortunately the boot loader uses the first few context banks, so
> rather than overwriting a active bank the last context bank is used and
> streams are diverted here during initialization.
> 
> This also performs the readback of SMR registers for the Qualcomm
> platform, to trigger the mechanism.
> 
> This is based on prior work by Thierry Reding and Laurentiu Tudor.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> Changes since v2:
> - Combined from pieces spread between the Qualcomm impl and generic code in v2.
> - Moved to use the newly introduced inherit_mapping op.
> 
>   drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 33 ++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 70a1eaa52e14..a54302190932 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -12,6 +12,7 @@
>   struct qcom_smmu {
>   	struct arm_smmu_device smmu;
>   	bool bypass_broken;
> +	struct iommu_domain *identity;
>   };
>   
>   static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
> @@ -228,6 +229,37 @@ static int qcom_smmu_cfg_probe(struct arm_smmu_device *smmu)
>   	return 0;
>   }
>   
> +static int qcom_smmu_inherit_mappings(struct arm_smmu_device *smmu)
> +{
> +	struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
> +	int cbndx;
> +	u32 smr;
> +	int i;
> +
> +	qsmmu->identity = arm_smmu_alloc_identity_domain(smmu);
> +	if (IS_ERR(qsmmu->identity))
> +		return PTR_ERR(qsmmu->identity);
> +
> +	cbndx = to_smmu_domain(qsmmu->identity)->cfg.cbndx;

I don't really get the point of going through the dance of allocating a 
whole iommu_domain() just to get a context. If you don't want to simply 
statically reserve a context at probe time, then just allocate from 
smmu->context_map here (where AFAICS "here" should be in cfg_probe 
anyway). This is entirely driver-internal, so there shouldn't be any 
need for IOMMU-API-level stuff to be involved.

Robin.

> +
> +	for (i = 0; i < smmu->num_mapping_groups; i++) {
> +		smr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_SMR(i));
> +
> +		if (FIELD_GET(ARM_SMMU_SMR_VALID, smr)) {
> +			smmu->smrs[i].id = FIELD_GET(ARM_SMMU_SMR_ID, smr);
> +			smmu->smrs[i].mask = FIELD_GET(ARM_SMMU_SMR_MASK, smr);
> +			smmu->smrs[i].valid = true;
> +
> +			smmu->s2crs[i].type = S2CR_TYPE_TRANS;
> +			smmu->s2crs[i].privcfg = S2CR_PRIVCFG_DEFAULT;
> +			smmu->s2crs[i].cbndx = cbndx;
> +			smmu->s2crs[i].count++;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>   static int qcom_smmu_def_domain_type(struct device *dev)
>   {
>   	const struct of_device_id *match =
> @@ -270,6 +302,7 @@ static const struct arm_smmu_impl qcom_smmu_impl = {
>   	.cfg_probe = qcom_smmu_cfg_probe,
>   	.def_domain_type = qcom_smmu_def_domain_type,
>   	.reset = qcom_smmu500_reset,
> +	.inherit_mappings = qcom_smmu_inherit_mappings,
>   };
>   
>   static const struct arm_smmu_impl qcom_adreno_smmu_impl = {
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 6/8] iommu/arm-smmu: Add impl hook for inherit boot mappings
  2020-09-11 17:13   ` Robin Murphy
@ 2020-09-13  3:25     ` Bjorn Andersson
  2020-09-21 21:08       ` Will Deacon
  0 siblings, 1 reply; 33+ messages in thread
From: Bjorn Andersson @ 2020-09-13  3:25 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Rob Clark, linux-arm-msm, iommu, linux-kernel, Sibi Sankar,
	Will Deacon, linux-arm-kernel

On Fri 11 Sep 12:13 CDT 2020, Robin Murphy wrote:

> On 2020-09-04 16:55, Bjorn Andersson wrote:
> > Add a new operation to allow platform implementations to inherit any
> > stream mappings from the boot loader.
> 
> Is there a reason we need an explicit step for this? The aim of the
> cfg_probe hook is that the SMMU software state should all be set up by then,
> and you can mess about with it however you like before arm_smmu_reset()
> actually commits anything to hardware. I would have thought you could
> permanently steal a context bank, configure it as your bypass hole, read out
> the previous SME configuration and tweak smmu->smrs and smmu->s2crs
> appropriately all together "invisibly" at that point.

I did this because as of 6a79a5a3842b ("iommu/arm-smmu: Call
configuration impl hook before consuming features") we no longer have
setup pgsize_bitmap as we hit cfg_probe, which means that I need to
replicate this logic to set up the iommu_domain.

If I avoid setting up an iommu_domain for the identity context, as you
request in patch 8, this shouldn't be needed anymore.

> If that can't work, I'm very curious as to what I've overlooked.
> 

I believe that will work, I will rework the patches and try it out.

Thanks,
Bjorn

> Robin.
> 
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> > 
> > Changes since v2:
> > - New patch/interface
> > 
> >   drivers/iommu/arm/arm-smmu/arm-smmu.c | 11 ++++++-----
> >   drivers/iommu/arm/arm-smmu/arm-smmu.h |  6 ++++++
> >   2 files changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > index eb5c6ca5c138..4c4d302cd747 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > @@ -85,11 +85,6 @@ static inline void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
> >   		pm_runtime_put_autosuspend(smmu->dev);
> >   }
> > -static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
> > -{
> > -	return container_of(dom, struct arm_smmu_domain, domain);
> > -}
> > -
> >   static struct platform_driver arm_smmu_driver;
> >   static struct iommu_ops arm_smmu_ops;
> > @@ -2188,6 +2183,12 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
> >   	if (err)
> >   		return err;
> > +	if (smmu->impl->inherit_mappings) {
> > +		err = smmu->impl->inherit_mappings(smmu);
> > +		if (err)
> > +			return err;
> > +	}
> > +
> >   	if (smmu->version == ARM_SMMU_V2) {
> >   		if (smmu->num_context_banks > smmu->num_context_irqs) {
> >   			dev_err(dev,
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > index 235d9a3a6ab6..f58164976e74 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > @@ -378,6 +378,11 @@ struct arm_smmu_domain {
> >   	struct iommu_domain		domain;
> >   };
> > +static inline struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
> > +{
> > +	return container_of(dom, struct arm_smmu_domain, domain);
> > +}
> > +
> >   struct arm_smmu_master_cfg {
> >   	struct arm_smmu_device		*smmu;
> >   	s16				smendx[];
> > @@ -442,6 +447,7 @@ struct arm_smmu_impl {
> >   	int (*alloc_context_bank)(struct arm_smmu_domain *smmu_domain,
> >   				  struct arm_smmu_device *smmu,
> >   				  struct device *dev, int start);
> > +	int (*inherit_mappings)(struct arm_smmu_device *smmu);
> >   };
> >   #define INVALID_SMENDX			-1
> > 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings
  2020-09-04 15:55 [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Bjorn Andersson
                   ` (12 preceding siblings ...)
  2020-09-11 16:10 ` Amit Pundir
@ 2020-09-16 10:09 ` Laurentiu Tudor
  13 siblings, 0 replies; 33+ messages in thread
From: Laurentiu Tudor @ 2020-09-16 10:09 UTC (permalink / raw)
  To: Bjorn Andersson, Will Deacon, Robin Murphy, Joerg Roedel,
	Sai Prakash Ranjan, Jordan Crouse, Rob Clark
  Cc: linux-arm-msm, iommu, Sibi Sankar, linux-arm-kernel, linux-kernel


On 9/4/2020 6:55 PM, Bjorn Andersson wrote:
> Based on previous attempts and discussions this is the latest attempt at
> inheriting stream mappings set up by the bootloader, for e.g. boot splash or
> efifb.
> 
> Per Will's request this builds on the work by Jordan and Rob for the Adreno
> SMMU support. It applies cleanly ontop of v16 of their series, which can be
> found at
> https://lore.kernel.org/linux-arm-msm/20200901164707.2645413-1-robdclark@gmail.com/
> 
> Bjorn Andersson (8):
>   iommu/arm-smmu: Refactor context bank allocation
>   iommu/arm-smmu: Delay modifying domain during init
>   iommu/arm-smmu: Consult context bank allocator for identify domains
>   iommu/arm-smmu-qcom: Emulate bypass by using context banks
>   iommu/arm-smmu-qcom: Consistently initialize stream mappings
>   iommu/arm-smmu: Add impl hook for inherit boot mappings
>   iommu/arm-smmu: Provide helper for allocating identity domain
>   iommu/arm-smmu-qcom: Setup identity domain for boot mappings
> 
>  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 111 ++++++++++++++++++-
>  drivers/iommu/arm/arm-smmu/arm-smmu.c      | 122 ++++++++++++++-------
>  drivers/iommu/arm/arm-smmu/arm-smmu.h      |  14 ++-
>  3 files changed, 205 insertions(+), 42 deletions(-)
> 

Tested on a NXP LX2160A with John's tree [1] and below diff [2], so for
the whole series:

Tested-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>

[1]
https://git.linaro.org/people/john.stultz/android-dev.git/log/?h=dev/db845c-mainline-WIP
[2]
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
@@ -190,11 +190,43 @@ static const struct arm_smmu_impl mrvl_mmu500_impl = {
        .reset = arm_mmu500_reset,
 };

+static int nxp_smmu_inherit_mappings(struct arm_smmu_device *smmu)
+{
+       u32 smr;
+       int i, cnt = 0;
+
+       for (i = 0; i < smmu->num_mapping_groups; i++) {
+               smr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_SMR(i));
+
+               if (FIELD_GET(ARM_SMMU_SMR_VALID, smr)) {
+                       smmu->smrs[i].id = FIELD_GET(ARM_SMMU_SMR_ID, smr);
+                       smmu->smrs[i].mask =
FIELD_GET(ARM_SMMU_SMR_MASK, smr);
+                       smmu->smrs[i].valid = true;
+
+                       smmu->s2crs[i].type = S2CR_TYPE_BYPASS;
+                       smmu->s2crs[i].privcfg = S2CR_PRIVCFG_DEFAULT;
+                       smmu->s2crs[i].count++;
+
+                       cnt++;
+               }
+       }
+
+       dev_notice(smmu->dev, "\tpreserved %d boot mapping%s\n", cnt,
+                  cnt == 1 ? "" : "s");
+
+       return 0;
+}
+
+static const struct arm_smmu_impl nxp_impl = {
+       .inherit_mappings = nxp_smmu_inherit_mappings,
+};

 struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
 {
        const struct device_node *np = smmu->dev->of_node;

        /*
         * Set the impl for model-specific implementation quirks first,
         * such that platform integration quirks can pick it up and
@@ -229,5 +261,12 @@ struct arm_smmu_device *arm_smmu_impl_init(struct
arm_smmu_device *smmu)
        if (of_device_is_compatible(np, "marvell,ap806-smmu-500"))
                smmu->impl = &mrvl_mmu500_impl;

+       if (of_property_read_bool(np, "nxp,keep-bypass-mappings"))
+               smmu->impl = &nxp_impl;

---
Best Regards, Laurentiu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 6/8] iommu/arm-smmu: Add impl hook for inherit boot mappings
  2020-09-13  3:25     ` Bjorn Andersson
@ 2020-09-21 21:08       ` Will Deacon
  2020-09-24 15:55         ` Bjorn Andersson
  2020-10-12  7:31         ` Bjorn Andersson
  0 siblings, 2 replies; 33+ messages in thread
From: Will Deacon @ 2020-09-21 21:08 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Clark, linux-arm-msm, iommu, linux-kernel, Sibi Sankar,
	Robin Murphy, linux-arm-kernel

On Sat, Sep 12, 2020 at 10:25:59PM -0500, Bjorn Andersson wrote:
> On Fri 11 Sep 12:13 CDT 2020, Robin Murphy wrote:
> > On 2020-09-04 16:55, Bjorn Andersson wrote:
> > > Add a new operation to allow platform implementations to inherit any
> > > stream mappings from the boot loader.
> > 
> > Is there a reason we need an explicit step for this? The aim of the
> > cfg_probe hook is that the SMMU software state should all be set up by then,
> > and you can mess about with it however you like before arm_smmu_reset()
> > actually commits anything to hardware. I would have thought you could
> > permanently steal a context bank, configure it as your bypass hole, read out
> > the previous SME configuration and tweak smmu->smrs and smmu->s2crs
> > appropriately all together "invisibly" at that point.
> 
> I did this because as of 6a79a5a3842b ("iommu/arm-smmu: Call
> configuration impl hook before consuming features") we no longer have
> setup pgsize_bitmap as we hit cfg_probe, which means that I need to
> replicate this logic to set up the iommu_domain.
> 
> If I avoid setting up an iommu_domain for the identity context, as you
> request in patch 8, this shouldn't be needed anymore.
> 
> > If that can't work, I'm very curious as to what I've overlooked.
> > 
> 
> I believe that will work, I will rework the patches and try it out.

Did you get a chance to rework this?

Will
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 6/8] iommu/arm-smmu: Add impl hook for inherit boot mappings
  2020-09-21 21:08       ` Will Deacon
@ 2020-09-24 15:55         ` Bjorn Andersson
  2020-10-12  7:31         ` Bjorn Andersson
  1 sibling, 0 replies; 33+ messages in thread
From: Bjorn Andersson @ 2020-09-24 15:55 UTC (permalink / raw)
  To: Will Deacon
  Cc: Rob Clark, linux-arm-msm, iommu, linux-kernel, Sibi Sankar,
	Robin Murphy, linux-arm-kernel

On Mon 21 Sep 16:08 CDT 2020, Will Deacon wrote:

> On Sat, Sep 12, 2020 at 10:25:59PM -0500, Bjorn Andersson wrote:
> > On Fri 11 Sep 12:13 CDT 2020, Robin Murphy wrote:
> > > On 2020-09-04 16:55, Bjorn Andersson wrote:
> > > > Add a new operation to allow platform implementations to inherit any
> > > > stream mappings from the boot loader.
> > > 
> > > Is there a reason we need an explicit step for this? The aim of the
> > > cfg_probe hook is that the SMMU software state should all be set up by then,
> > > and you can mess about with it however you like before arm_smmu_reset()
> > > actually commits anything to hardware. I would have thought you could
> > > permanently steal a context bank, configure it as your bypass hole, read out
> > > the previous SME configuration and tweak smmu->smrs and smmu->s2crs
> > > appropriately all together "invisibly" at that point.
> > 
> > I did this because as of 6a79a5a3842b ("iommu/arm-smmu: Call
> > configuration impl hook before consuming features") we no longer have
> > setup pgsize_bitmap as we hit cfg_probe, which means that I need to
> > replicate this logic to set up the iommu_domain.
> > 
> > If I avoid setting up an iommu_domain for the identity context, as you
> > request in patch 8, this shouldn't be needed anymore.
> > 
> > > If that can't work, I'm very curious as to what I've overlooked.
> > > 
> > 
> > I believe that will work, I will rework the patches and try it out.
> 
> Did you get a chance to rework this?
> 

Unfortunately not, I hope to get to this shortly.

Thanks,
Bjorn
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 6/8] iommu/arm-smmu: Add impl hook for inherit boot mappings
  2020-09-21 21:08       ` Will Deacon
  2020-09-24 15:55         ` Bjorn Andersson
@ 2020-10-12  7:31         ` Bjorn Andersson
  2020-10-13 16:47           ` Robin Murphy
  1 sibling, 1 reply; 33+ messages in thread
From: Bjorn Andersson @ 2020-10-12  7:31 UTC (permalink / raw)
  To: Will Deacon
  Cc: Rob Clark, linux-arm-msm, iommu, linux-kernel, Sibi Sankar,
	Robin Murphy, linux-arm-kernel

On Mon 21 Sep 23:08 CEST 2020, Will Deacon wrote:

> On Sat, Sep 12, 2020 at 10:25:59PM -0500, Bjorn Andersson wrote:
> > On Fri 11 Sep 12:13 CDT 2020, Robin Murphy wrote:
> > > On 2020-09-04 16:55, Bjorn Andersson wrote:
> > > > Add a new operation to allow platform implementations to inherit any
> > > > stream mappings from the boot loader.
> > > 
> > > Is there a reason we need an explicit step for this? The aim of the
> > > cfg_probe hook is that the SMMU software state should all be set up by then,
> > > and you can mess about with it however you like before arm_smmu_reset()
> > > actually commits anything to hardware. I would have thought you could
> > > permanently steal a context bank, configure it as your bypass hole, read out
> > > the previous SME configuration and tweak smmu->smrs and smmu->s2crs
> > > appropriately all together "invisibly" at that point.
> > 
> > I did this because as of 6a79a5a3842b ("iommu/arm-smmu: Call
> > configuration impl hook before consuming features") we no longer have
> > setup pgsize_bitmap as we hit cfg_probe, which means that I need to
> > replicate this logic to set up the iommu_domain.
> > 
> > If I avoid setting up an iommu_domain for the identity context, as you
> > request in patch 8, this shouldn't be needed anymore.
> > 
> > > If that can't work, I'm very curious as to what I've overlooked.
> > > 
> > 
> > I believe that will work, I will rework the patches and try it out.
> 
> Did you get a chance to rework this?
> 

Finally got a chance to dig through this properly.

Initial results where positive and with an implementation of cfg_probe
in qcom_smmu_impl I'm able to probe the arm-smmu driver just fine - and
display (e.g. efifb) stays alive.

Unfortunately as the display driver (drivers/gpu/drm/msm) is about to
probe a new iommu domain is created, which due to its match against
qcom_smmu_client_of_match[] becomes of type IOMMU_DOMAIN_IDENTITY.
This results in a S2CR of BYPASS type, which the firmware intercepts and
turns the stream into a type FAULT.

So while the cfg_probe looks very reasonable we're still in need of a
mechanism to use the fake identity context for the iommu domain
associated with the display controller.


The workings of the display driver is that it gets the iommu domain
setup for byass and then after that creates a translation context for
this same stream where it maps the framebuffer.

For testing purposes I made def_domain_type always return 0 in the qcom
impl and the result is that we get a few page faults while probing the
display driver, but these are handled somewhat gracefully and the
initialization did proceed and the system comes up nicely (but in the
case that the display driver would probe defer this leads to an storm of
faults as the screen continues to be refreshed).

TL;DR I think we still need to have a way to get the arm-smmu driver to
allow the qcom implementation to configure identity domains to use
translation - but we can make the setup of the identity context a detail
of the qcom driver.

Regards,
Bjorn
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 6/8] iommu/arm-smmu: Add impl hook for inherit boot mappings
  2020-10-12  7:31         ` Bjorn Andersson
@ 2020-10-13 16:47           ` Robin Murphy
  0 siblings, 0 replies; 33+ messages in thread
From: Robin Murphy @ 2020-10-13 16:47 UTC (permalink / raw)
  To: Bjorn Andersson, Will Deacon
  Cc: Rob Clark, linux-arm-msm, iommu, linux-kernel, Sibi Sankar,
	linux-arm-kernel

On 2020-10-12 08:31, Bjorn Andersson wrote:
> On Mon 21 Sep 23:08 CEST 2020, Will Deacon wrote:
> 
>> On Sat, Sep 12, 2020 at 10:25:59PM -0500, Bjorn Andersson wrote:
>>> On Fri 11 Sep 12:13 CDT 2020, Robin Murphy wrote:
>>>> On 2020-09-04 16:55, Bjorn Andersson wrote:
>>>>> Add a new operation to allow platform implementations to inherit any
>>>>> stream mappings from the boot loader.
>>>>
>>>> Is there a reason we need an explicit step for this? The aim of the
>>>> cfg_probe hook is that the SMMU software state should all be set up by then,
>>>> and you can mess about with it however you like before arm_smmu_reset()
>>>> actually commits anything to hardware. I would have thought you could
>>>> permanently steal a context bank, configure it as your bypass hole, read out
>>>> the previous SME configuration and tweak smmu->smrs and smmu->s2crs
>>>> appropriately all together "invisibly" at that point.
>>>
>>> I did this because as of 6a79a5a3842b ("iommu/arm-smmu: Call
>>> configuration impl hook before consuming features") we no longer have
>>> setup pgsize_bitmap as we hit cfg_probe, which means that I need to
>>> replicate this logic to set up the iommu_domain.
>>>
>>> If I avoid setting up an iommu_domain for the identity context, as you
>>> request in patch 8, this shouldn't be needed anymore.
>>>
>>>> If that can't work, I'm very curious as to what I've overlooked.
>>>>
>>>
>>> I believe that will work, I will rework the patches and try it out.
>>
>> Did you get a chance to rework this?
>>
> 
> Finally got a chance to dig through this properly.
> 
> Initial results where positive and with an implementation of cfg_probe
> in qcom_smmu_impl I'm able to probe the arm-smmu driver just fine - and
> display (e.g. efifb) stays alive.
> 
> Unfortunately as the display driver (drivers/gpu/drm/msm) is about to
> probe a new iommu domain is created, which due to its match against
> qcom_smmu_client_of_match[] becomes of type IOMMU_DOMAIN_IDENTITY.
> This results in a S2CR of BYPASS type, which the firmware intercepts and
> turns the stream into a type FAULT.
> 
> So while the cfg_probe looks very reasonable we're still in need of a
> mechanism to use the fake identity context for the iommu domain
> associated with the display controller.

Yes, we'll still need some kind of hook somewhere to make identity 
domains work at all - my point about cfg_probe was to keep the 
reservation and configuration of the special identity context, plus the 
handling of the initial SME state, simple and entirely internal to the 
impl. In terms of where said hook should be, TBH it might actually work 
out pretty clean to simply hook GR0 register accesses so you can rewrite 
between S2CR bypass entries and translation entries targeting your 
reserved context on-the-fly. Failing that, something to massage "type" 
and "cbndx" in arm_smmu_domain_add_master() would be the next best 
option, I think.

Robin.

> The workings of the display driver is that it gets the iommu domain
> setup for byass and then after that creates a translation context for
> this same stream where it maps the framebuffer.
> 
> For testing purposes I made def_domain_type always return 0 in the qcom
> impl and the result is that we get a few page faults while probing the
> display driver, but these are handled somewhat gracefully and the
> initialization did proceed and the system comes up nicely (but in the
> case that the display driver would probe defer this leads to an storm of
> faults as the screen continues to be refreshed).
> 
> TL;DR I think we still need to have a way to get the arm-smmu driver to
> allow the qcom implementation to configure identity domains to use
> translation - but we can make the setup of the identity context a detail
> of the qcom driver.
> 
> Regards,
> Bjorn
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2020-10-13 16:48 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-04 15:55 [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Bjorn Andersson
2020-09-04 15:55 ` [PATCH v3 1/8] iommu/arm-smmu: Refactor context bank allocation Bjorn Andersson
2020-09-08 18:42   ` Jordan Crouse
2020-09-08 18:46     ` Jordan Crouse
2020-09-11  8:18   ` Sai Prakash Ranjan
2020-09-04 15:55 ` [PATCH v3 2/8] iommu/arm-smmu: Delay modifying domain during init Bjorn Andersson
2020-09-11  8:20   ` Sai Prakash Ranjan
2020-09-04 15:55 ` [PATCH v3 3/8] iommu/arm-smmu: Consult context bank allocator for identify domains Bjorn Andersson
2020-09-11  8:21   ` Sai Prakash Ranjan
2020-09-11  8:24   ` Sai Prakash Ranjan
2020-09-04 15:55 ` [PATCH v3 4/8] iommu/arm-smmu-qcom: Emulate bypass by using context banks Bjorn Andersson
2020-09-11  8:25   ` Sai Prakash Ranjan
2020-09-04 15:55 ` [PATCH v3 5/8] iommu/arm-smmu-qcom: Consistently initialize stream mappings Bjorn Andersson
2020-09-11  8:26   ` Sai Prakash Ranjan
2020-09-04 15:55 ` [PATCH v3 6/8] iommu/arm-smmu: Add impl hook for inherit boot mappings Bjorn Andersson
2020-09-11  8:27   ` Sai Prakash Ranjan
2020-09-11 17:13   ` Robin Murphy
2020-09-13  3:25     ` Bjorn Andersson
2020-09-21 21:08       ` Will Deacon
2020-09-24 15:55         ` Bjorn Andersson
2020-10-12  7:31         ` Bjorn Andersson
2020-10-13 16:47           ` Robin Murphy
2020-09-04 15:55 ` [PATCH v3 7/8] iommu/arm-smmu: Provide helper for allocating identity domain Bjorn Andersson
2020-09-11  8:28   ` Sai Prakash Ranjan
2020-09-04 15:55 ` [PATCH v3 8/8] iommu/arm-smmu-qcom: Setup identity domain for boot mappings Bjorn Andersson
2020-09-11  8:29   ` Sai Prakash Ranjan
2020-09-11 17:29   ` Robin Murphy
2020-09-05 22:27 ` [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings Rob Clark
2020-09-09 14:46 ` Laurentiu Tudor
2020-09-10 22:56 ` John Stultz
2020-09-11  8:16 ` Sai Prakash Ranjan
2020-09-11 16:10 ` Amit Pundir
2020-09-16 10:09 ` Laurentiu Tudor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).