On 10/29/2024 6:17 PM, Will Deacon wrote:
On Fri, Oct 25, 2024 at 07:51:22PM +0530, Bibek Kumar Patro wrote:
On 10/24/2024 6:22 PM, Will Deacon wrote:
On Tue, Oct 08, 2024 at 06:24:06PM +0530, Bibek Kumar Patro wrote:
Default MMU-500 reset operation disables context caching in
prefetch buffer. It is however expected for context banks using
the ACTLR register to retain their prefetch value during reset
and runtime suspend.
Replace default MMU-500 reset operation with Qualcomm specific reset
operation which envelope the default reset operation and re-enables
context caching in prefetch buffer for Qualcomm SoCs.
Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
Signed-off-by: Bibek Kumar Patro <quic_bibekkum@xxxxxxxxxxx>
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 45 ++++++++++++++++++++--
1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 087fb4f6f4d3..0cb10b354802 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -16,6 +16,16 @@
#define QCOM_DUMMY_VAL -1
+/*
+ * SMMU-500 TRM defines BIT(0) as CMTLB (Enable context caching in the
+ * macro TLB) and BIT(1) as CPRE (Enable context caching in the prefetch
+ * buffer). The remaining bits are implementation defined and vary across
+ * SoCs.
+ */
+
+#define CPRE (1 << 1)
+#define CMTLB (1 << 0)
+
static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
{
return container_of(smmu, struct qcom_smmu, smmu);
@@ -396,11 +406,40 @@ static int qcom_smmu_def_domain_type(struct device *dev)
return match ? IOMMU_DOMAIN_IDENTITY : 0;
}
+static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
+{
+ int ret;
+ u32 val;
+ int i;
+
+ ret = arm_mmu500_reset(smmu);
+ if (ret)
+ return ret;
+
+ /*
+ * arm_mmu500_reset() disables CPRE which is re-enabled here.
+ * The errata for MMU-500 before the r2p2 revision requires CPRE to be
+ * disabled. The arm_mmu500_reset function disables CPRE to accommodate all
+ * RTL revisions. Since all Qualcomm SoCs are on the r2p4 revision, where
+ * the CPRE bit can be enabled, the qcom_smmu500_reset function re-enables
+ * the CPRE bit for the next-page prefetcher to retain the prefetch value
+ * during reset and runtime suspend operations.
+ */
+
+ for (i = 0; i < smmu->num_context_banks; ++i) {
+ val = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR);
+ val |= CPRE;
+ arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, val);
+ }
If CPRE only needs to be disabled prior to r2p2, then please teach the
MMU-500 code about that instead of adding qualcomm-specific logic here.
Doing this on MMU-500 code would make it generic and reflect for SoC of all
the vendors on this platform.
We can make sure that it won't cause any problems in Qualcomm SoCs as we
have been enabling this since for some years now and could not
observe/reproduce any issues around these errata.
Unless you can explain definitively hy that's the case, I still don't
think we should be second-guessing the core SMMU driver code in the
Qualcomm backend.
But we won't be able to guarantee the same behavior in SoC for other vendors
where these errata might still be applicable as per [1] and [2].
So as per my understanding it's safe to include in Qualcomm specific
implementation and not changing the default behavior in all other vendors'
SoC even if they are not prior to r2p2 revision [3].
If you want to gate the errata workarounds on policy, then please follow
what we do for the CPU: add a Kconfig option (e.g.
ARM_SMMU_WORKAROUND_BROKEN_CPRE) which defaults to "on" (assuming that
the relevant errata aren't all "rare") and update silicon-errata.rst
accordingly.
Then you can choose to disable them in your .config if you're happy to
pick up the pieces.
This seems to be a good idea to me . I am thinking of this approach
based on your suggestion,
i.e. we can bind the original workaround in
arm_mmu500_reset implementation within ARM_SMMU_WORKAROUND_BROKEN_CPRE
config (defualts to on, CPRE would be disabled) and in QCOM SoCs default
it to off
(when ARM_SMMU_QCOM=Y -> switch ARM_SMMU_WORKAROUND_BROKEN_CPRE=N).
In silicon-errata.rst would updating ARM_SMMU_WORKAROUND_BROKEN_CPRE be
okay , as the config names are based on erratum number.
Thanks & regards,
Bibek
As an aside, I'm happy with the rest of the series now.
Will
