[PATCH 0/4] coresight: trbe: Workaround Cortex-A510 erratas

[PATCH 0/4] coresight: trbe: Workaround Cortex-A510 erratas

[Anshuman Khandual]

	This series adds three different workarounds in the TRBE driver for
Cortex-A510 specific erratas. But first, this adds Cortex-A510 specific cpu
part number definition in the platform. This series applies on 5.16-rc8.

Relevant errata documents can be found here.

https://developer.arm.com/documentation/SDEN2397239/900
https://developer.arm.com/documentation/SDEN2397589/900

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Suzuki Poulose <suzuki.poulose@arm.com>
Cc: coresight@lists.linaro.org
Cc: linux-doc@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org

Anshuman Khandual (4):
  arm64: Add Cortex-A510 CPU part definition
  coresight: trbe: Work around the ignored system register writes
  coresight: trbe: Work around the invalid prohibited states
  coresight: trbe: Workaround TRBE trace data corruption

 Documentation/arm64/silicon-errata.rst       |   6 +
 arch/arm64/Kconfig                           |  57 ++++++++++
 arch/arm64/include/asm/cputype.h             |   2 +
 arch/arm64/kernel/cpu_errata.c               |  27 +++++
 arch/arm64/tools/cpucaps                     |   3 +
 drivers/hwtracing/coresight/coresight-trbe.c | 111 ++++++++++++++-----
 drivers/hwtracing/coresight/coresight-trbe.h |   8 --
 7 files changed, 181 insertions(+), 33 deletions(-)

-- 
2.25.1

[PATCH 0/4] coresight: trbe: Workaround Cortex-A510 erratas

[Anshuman Khandual]

	This series adds three different workarounds in the TRBE driver for
Cortex-A510 specific erratas. But first, this adds Cortex-A510 specific cpu
part number definition in the platform. This series applies on 5.16-rc8.

Relevant errata documents can be found here.

https://developer.arm.com/documentation/SDEN2397239/900
https://developer.arm.com/documentation/SDEN2397589/900

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Suzuki Poulose <suzuki.poulose@arm.com>
Cc: coresight@lists.linaro.org
Cc: linux-doc@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org

Anshuman Khandual (4):
  arm64: Add Cortex-A510 CPU part definition
  coresight: trbe: Work around the ignored system register writes
  coresight: trbe: Work around the invalid prohibited states
  coresight: trbe: Workaround TRBE trace data corruption

 Documentation/arm64/silicon-errata.rst       |   6 +
 arch/arm64/Kconfig                           |  57 ++++++++++
 arch/arm64/include/asm/cputype.h             |   2 +
 arch/arm64/kernel/cpu_errata.c               |  27 +++++
 arch/arm64/tools/cpucaps                     |   3 +
 drivers/hwtracing/coresight/coresight-trbe.c | 111 ++++++++++++++-----
 drivers/hwtracing/coresight/coresight-trbe.h |   8 --
 7 files changed, 181 insertions(+), 33 deletions(-)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/4] arm64: Add Cortex-A510 CPU part definition

[Anshuman Khandual]

Add the CPU Partnumbers for the new Arm designs.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Suzuki Poulose <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm64/include/asm/cputype.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index 19b8441aa8f2..e8fdc10395b6 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -73,6 +73,7 @@
 #define ARM_CPU_PART_CORTEX_A76		0xD0B
 #define ARM_CPU_PART_NEOVERSE_N1	0xD0C
 #define ARM_CPU_PART_CORTEX_A77		0xD0D
+#define ARM_CPU_PART_CORTEX_A510	0xD46
 #define ARM_CPU_PART_CORTEX_A710	0xD47
 #define ARM_CPU_PART_NEOVERSE_N2	0xD49
 
@@ -115,6 +116,7 @@
 #define MIDR_CORTEX_A76	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A76)
 #define MIDR_NEOVERSE_N1 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N1)
 #define MIDR_CORTEX_A77	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A77)
+#define MIDR_CORTEX_A510 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A510)
 #define MIDR_CORTEX_A710 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A710)
 #define MIDR_NEOVERSE_N2 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N2)
 #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
-- 
2.25.1

[PATCH 1/4] arm64: Add Cortex-A510 CPU part definition

[Anshuman Khandual]

Add the CPU Partnumbers for the new Arm designs.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Suzuki Poulose <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm64/include/asm/cputype.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index 19b8441aa8f2..e8fdc10395b6 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -73,6 +73,7 @@
 #define ARM_CPU_PART_CORTEX_A76		0xD0B
 #define ARM_CPU_PART_NEOVERSE_N1	0xD0C
 #define ARM_CPU_PART_CORTEX_A77		0xD0D
+#define ARM_CPU_PART_CORTEX_A510	0xD46
 #define ARM_CPU_PART_CORTEX_A710	0xD47
 #define ARM_CPU_PART_NEOVERSE_N2	0xD49
 
@@ -115,6 +116,7 @@
 #define MIDR_CORTEX_A76	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A76)
 #define MIDR_NEOVERSE_N1 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N1)
 #define MIDR_CORTEX_A77	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A77)
+#define MIDR_CORTEX_A510 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A510)
 #define MIDR_CORTEX_A710 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A710)
 #define MIDR_NEOVERSE_N2 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N2)
 #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] arm64: Add Cortex-A510 CPU part definition

[Suzuki K Poulose]

On 05/01/2022 05:05, Anshuman Khandual wrote:
> Add the CPU Partnumbers for the new Arm designs.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>   arch/arm64/include/asm/cputype.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index 19b8441aa8f2..e8fdc10395b6 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -73,6 +73,7 @@
>   #define ARM_CPU_PART_CORTEX_A76		0xD0B
>   #define ARM_CPU_PART_NEOVERSE_N1	0xD0C
>   #define ARM_CPU_PART_CORTEX_A77		0xD0D
> +#define ARM_CPU_PART_CORTEX_A510	0xD46
>   #define ARM_CPU_PART_CORTEX_A710	0xD47
>   #define ARM_CPU_PART_NEOVERSE_N2	0xD49

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

Re: [PATCH 1/4] arm64: Add Cortex-A510 CPU part definition

[Suzuki K Poulose]

On 05/01/2022 05:05, Anshuman Khandual wrote:
> Add the CPU Partnumbers for the new Arm designs.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>   arch/arm64/include/asm/cputype.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index 19b8441aa8f2..e8fdc10395b6 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -73,6 +73,7 @@
>   #define ARM_CPU_PART_CORTEX_A76		0xD0B
>   #define ARM_CPU_PART_NEOVERSE_N1	0xD0C
>   #define ARM_CPU_PART_CORTEX_A77		0xD0D
> +#define ARM_CPU_PART_CORTEX_A510	0xD46
>   #define ARM_CPU_PART_CORTEX_A710	0xD47
>   #define ARM_CPU_PART_NEOVERSE_N2	0xD49

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/4] coresight: trbe: Work around the ignored system register writes

[Anshuman Khandual]

TRBE implementations affected by Arm erratum #2064142 might fail to write
into certain system registers after the TRBE has been disabled. Under some
conditions after TRBE has been disabled, writes into certain TRBE registers
TRBLIMITR_EL1, TRBPTR_EL1, TRBBASER_EL1, TRBSR_EL1 and TRBTRG_EL1 will be
ignored and not be effected.

Work around this problem in the TRBE driver by executing TSB CSYNC and DSB
just after the trace collection has stopped and before performing a system
register write to one of the affected registers. This adds a new cpu errata
in arm64 errata framework and also updates TRBE driver as required.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Suzuki Poulose <suzuki.poulose@arm.com>
Cc: coresight@lists.linaro.org
Cc: linux-doc@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 Documentation/arm64/silicon-errata.rst       |  2 +
 arch/arm64/Kconfig                           | 18 +++++++
 arch/arm64/kernel/cpu_errata.c               |  9 ++++
 arch/arm64/tools/cpucaps                     |  1 +
 drivers/hwtracing/coresight/coresight-trbe.c | 54 ++++++++++++++------
 drivers/hwtracing/coresight/coresight-trbe.h |  8 ---
 6 files changed, 68 insertions(+), 24 deletions(-)

diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
index 5342e895fb60..c9b30e6c2b6c 100644
--- a/Documentation/arm64/silicon-errata.rst
+++ b/Documentation/arm64/silicon-errata.rst
@@ -52,6 +52,8 @@ stable kernels.
 | Allwinner      | A64/R18         | UNKNOWN1        | SUN50I_ERRATUM_UNKNOWN1     |
 +----------------+-----------------+-----------------+-----------------------------+
 +----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
++----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index c4207cf9bb17..2105b68d88db 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -778,6 +778,24 @@ config ARM64_ERRATUM_2224489
 
 	  If unsure, say Y.
 
+config ARM64_ERRATUM_2064142
+	bool "Cortex-A510: 2064142: workaround TRBE register writes while disabled"
+	depends on CORESIGHT_TRBE
+	default y
+	help
+	  This option adds the workaround for ARM Cortex-A510 erratum 2064142.
+
+	  Affected Cortex-A510 core might fail to write into system registers after the
+	  TRBE has been disabled. Under some conditions after the TRBE has been disabled
+	  writes into TRBE registers TRBLIMITR_EL1, TRBPTR_EL1, TRBBASER_EL1, TRBSR_EL1,
+	  and TRBTRG_EL1 will be ignored and will not be effected.
+
+	  Work around this in the driver by executing TSB CSYNC and DSB after collection
+	  is stopped and before performing a system register write to one of the affected
+	  registers.
+
+	  If unsure, say Y.
+
 config CAVIUM_ERRATUM_22375
 	bool "Cavium erratum 22375, 24313"
 	default y
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 9e1c1aef9ebd..cbb7d5a9aee7 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -597,6 +597,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		.type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,
 		CAP_MIDR_RANGE_LIST(trbe_write_out_of_range_cpus),
 	},
+#endif
+#ifdef CONFIG_ARM64_ERRATUM_2064142
+	{
+		.desc = "ARM erratum 2064142",
+		.capability = ARM64_WORKAROUND_2064142,
+
+		/* Cortex-A510 r0p0 - r0p2 */
+		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
+	},
 #endif
 	{
 	}
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 870c39537dd0..fca3cb329e1d 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -55,6 +55,7 @@ WORKAROUND_1418040
 WORKAROUND_1463225
 WORKAROUND_1508412
 WORKAROUND_1542419
+WORKAROUND_2064142
 WORKAROUND_TRBE_OVERWRITE_FILL_MODE
 WORKAROUND_TSB_FLUSH_FAILURE
 WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 276862c07e32..ec24b62b2cec 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -91,10 +91,12 @@ struct trbe_buf {
  */
 #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE	0
 #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE	1
+#define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE	2
 
 static int trbe_errata_cpucaps[] = {
 	[TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
 	[TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
+	[TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
 	-1,		/* Sentinel, must be the last entry */
 };
 
@@ -167,6 +169,11 @@ static inline bool trbe_may_write_out_of_range(struct trbe_cpudata *cpudata)
 	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_WRITE_OUT_OF_RANGE);
 }
 
+static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
+{
+	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_SYSREG_WRITE_FAILURE);
+}
+
 static int trbe_alloc_node(struct perf_event *event)
 {
 	if (event->cpu == -1)
@@ -174,30 +181,42 @@ static int trbe_alloc_node(struct perf_event *event)
 	return cpu_to_node(event->cpu);
 }
 
-static void trbe_drain_buffer(void)
+static inline void trbe_drain_buffer(void)
 {
 	tsb_csync();
 	dsb(nsh);
 }
 
-static void trbe_drain_and_disable_local(void)
+static inline void set_trbe_disabled(struct trbe_cpudata *cpudata)
 {
 	u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
 
-	trbe_drain_buffer();
-
 	/*
 	 * Disable the TRBE without clearing LIMITPTR which
 	 * might be required for fetching the buffer limits.
 	 */
 	trblimitr &= ~TRBLIMITR_ENABLE;
 	write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
+
+	/*
+	 * Errata affected TRBE implementation will need TSB CSYNC and
+	 * DSB in order to prevent subsequent writes into certain TRBE
+	 * system registers from being ignored and not effected.
+	 */
+	if (trbe_may_fail_sysreg_write(cpudata))
+		trbe_drain_buffer();
 	isb();
 }
 
-static void trbe_reset_local(void)
+static void trbe_drain_and_disable_local(struct trbe_cpudata *cpudata)
 {
-	trbe_drain_and_disable_local();
+	trbe_drain_buffer();
+	set_trbe_disabled(cpudata);
+}
+
+static void trbe_reset_local(struct trbe_cpudata *cpudata)
+{
+	trbe_drain_and_disable_local(cpudata);
 	write_sysreg_s(0, SYS_TRBLIMITR_EL1);
 	write_sysreg_s(0, SYS_TRBPTR_EL1);
 	write_sysreg_s(0, SYS_TRBBASER_EL1);
@@ -234,7 +253,7 @@ static void trbe_stop_and_truncate_event(struct perf_output_handle *handle)
 	 * at event_stop(). So disable the TRBE here and leave
 	 * the update_buffer() to return a 0 size.
 	 */
-	trbe_drain_and_disable_local();
+	trbe_drain_and_disable_local(buf->cpudata);
 	perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
 	perf_aux_output_end(handle, 0);
 	*this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL;
@@ -579,8 +598,7 @@ static void trbe_enable_hw(struct trbe_buf *buf)
 	WARN_ON(buf->trbe_hw_base < buf->trbe_base);
 	WARN_ON(buf->trbe_write < buf->trbe_hw_base);
 	WARN_ON(buf->trbe_write >= buf->trbe_limit);
-	set_trbe_disabled();
-	isb();
+	set_trbe_disabled(buf->cpudata);
 	clr_trbe_status();
 	set_trbe_base_pointer(buf->trbe_hw_base);
 	set_trbe_write_pointer(buf->trbe_write);
@@ -775,7 +793,7 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev,
 	 * the TRBE here will ensure that no IRQ could be generated when the perf
 	 * handle gets freed in etm_event_stop().
 	 */
-	trbe_drain_and_disable_local();
+	trbe_drain_and_disable_local(cpudata);
 
 	/* Check if there is a pending interrupt and handle it here */
 	status = read_sysreg_s(SYS_TRBSR_EL1);
@@ -986,7 +1004,7 @@ static int arm_trbe_disable(struct coresight_device *csdev)
 	if (cpudata->mode != CS_MODE_PERF)
 		return -EINVAL;
 
-	trbe_drain_and_disable_local();
+	trbe_drain_and_disable_local(cpudata);
 	buf->cpudata = NULL;
 	cpudata->buf = NULL;
 	cpudata->mode = CS_MODE_DISABLED;
@@ -1028,7 +1046,7 @@ static int trbe_handle_overflow(struct perf_output_handle *handle)
 		 * is able to detect this with a disconnected handle
 		 * (handle->event = NULL).
 		 */
-		trbe_drain_and_disable_local();
+		trbe_drain_and_disable_local(buf->cpudata);
 		*this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL;
 		return -EINVAL;
 	}
@@ -1062,6 +1080,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 {
 	struct perf_output_handle **handle_ptr = dev;
 	struct perf_output_handle *handle = *handle_ptr;
+	struct trbe_buf *buf = etm_perf_sink_config(handle);
 	enum trbe_fault_action act;
 	u64 status;
 	bool truncated = false;
@@ -1082,7 +1101,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 	 * Ensure the trace is visible to the CPUs and
 	 * any external aborts have been resolved.
 	 */
-	trbe_drain_and_disable_local();
+	trbe_drain_and_disable_local(buf->cpudata);
 	clr_trbe_irq();
 	isb();
 
@@ -1167,8 +1186,9 @@ static const struct attribute_group *arm_trbe_groups[] = {
 static void arm_trbe_enable_cpu(void *info)
 {
 	struct trbe_drvdata *drvdata = info;
+	struct trbe_cpudata *cpudata = this_cpu_ptr(drvdata->cpudata);
 
-	trbe_reset_local();
+	trbe_reset_local(cpudata);
 	enable_percpu_irq(drvdata->irq, IRQ_TYPE_NONE);
 }
 
@@ -1276,7 +1296,7 @@ static void arm_trbe_remove_coresight_cpu(void *info)
 	struct coresight_device *trbe_csdev = coresight_get_percpu_sink(cpu);
 
 	disable_percpu_irq(drvdata->irq);
-	trbe_reset_local();
+	trbe_reset_local(cpudata);
 	if (trbe_csdev) {
 		coresight_unregister(trbe_csdev);
 		cpudata->drvdata = NULL;
@@ -1349,8 +1369,10 @@ static int arm_trbe_cpu_teardown(unsigned int cpu, struct hlist_node *node)
 	struct trbe_drvdata *drvdata = hlist_entry_safe(node, struct trbe_drvdata, hotplug_node);
 
 	if (cpumask_test_cpu(cpu, &drvdata->supported_cpus)) {
+		struct trbe_cpudata *cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
+
 		disable_percpu_irq(drvdata->irq);
-		trbe_reset_local();
+		trbe_reset_local(cpudata);
 	}
 	return 0;
 }
diff --git a/drivers/hwtracing/coresight/coresight-trbe.h b/drivers/hwtracing/coresight/coresight-trbe.h
index abf3e36082f0..30e4d7db4f8e 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.h
+++ b/drivers/hwtracing/coresight/coresight-trbe.h
@@ -91,14 +91,6 @@ static inline bool is_trbe_running(u64 trbsr)
 #define TRBE_FILL_MODE_WRAP		1
 #define TRBE_FILL_MODE_CIRCULAR_BUFFER	3
 
-static inline void set_trbe_disabled(void)
-{
-	u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
-
-	trblimitr &= ~TRBLIMITR_ENABLE;
-	write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
-}
-
 static inline bool get_trbe_flag_update(u64 trbidr)
 {
 	return trbidr & TRBIDR_FLAG;
-- 
2.25.1

[PATCH 2/4] coresight: trbe: Work around the ignored system register writes

[Anshuman Khandual]

TRBE implementations affected by Arm erratum #2064142 might fail to write
into certain system registers after the TRBE has been disabled. Under some
conditions after TRBE has been disabled, writes into certain TRBE registers
TRBLIMITR_EL1, TRBPTR_EL1, TRBBASER_EL1, TRBSR_EL1 and TRBTRG_EL1 will be
ignored and not be effected.

Work around this problem in the TRBE driver by executing TSB CSYNC and DSB
just after the trace collection has stopped and before performing a system
register write to one of the affected registers. This adds a new cpu errata
in arm64 errata framework and also updates TRBE driver as required.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Suzuki Poulose <suzuki.poulose@arm.com>
Cc: coresight@lists.linaro.org
Cc: linux-doc@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 Documentation/arm64/silicon-errata.rst       |  2 +
 arch/arm64/Kconfig                           | 18 +++++++
 arch/arm64/kernel/cpu_errata.c               |  9 ++++
 arch/arm64/tools/cpucaps                     |  1 +
 drivers/hwtracing/coresight/coresight-trbe.c | 54 ++++++++++++++------
 drivers/hwtracing/coresight/coresight-trbe.h |  8 ---
 6 files changed, 68 insertions(+), 24 deletions(-)

diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
index 5342e895fb60..c9b30e6c2b6c 100644
--- a/Documentation/arm64/silicon-errata.rst
+++ b/Documentation/arm64/silicon-errata.rst
@@ -52,6 +52,8 @@ stable kernels.
 | Allwinner      | A64/R18         | UNKNOWN1        | SUN50I_ERRATUM_UNKNOWN1     |
 +----------------+-----------------+-----------------+-----------------------------+
 +----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
++----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index c4207cf9bb17..2105b68d88db 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -778,6 +778,24 @@ config ARM64_ERRATUM_2224489
 
 	  If unsure, say Y.
 
+config ARM64_ERRATUM_2064142
+	bool "Cortex-A510: 2064142: workaround TRBE register writes while disabled"
+	depends on CORESIGHT_TRBE
+	default y
+	help
+	  This option adds the workaround for ARM Cortex-A510 erratum 2064142.
+
+	  Affected Cortex-A510 core might fail to write into system registers after the
+	  TRBE has been disabled. Under some conditions after the TRBE has been disabled
+	  writes into TRBE registers TRBLIMITR_EL1, TRBPTR_EL1, TRBBASER_EL1, TRBSR_EL1,
+	  and TRBTRG_EL1 will be ignored and will not be effected.
+
+	  Work around this in the driver by executing TSB CSYNC and DSB after collection
+	  is stopped and before performing a system register write to one of the affected
+	  registers.
+
+	  If unsure, say Y.
+
 config CAVIUM_ERRATUM_22375
 	bool "Cavium erratum 22375, 24313"
 	default y
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 9e1c1aef9ebd..cbb7d5a9aee7 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -597,6 +597,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		.type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,
 		CAP_MIDR_RANGE_LIST(trbe_write_out_of_range_cpus),
 	},
+#endif
+#ifdef CONFIG_ARM64_ERRATUM_2064142
+	{
+		.desc = "ARM erratum 2064142",
+		.capability = ARM64_WORKAROUND_2064142,
+
+		/* Cortex-A510 r0p0 - r0p2 */
+		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
+	},
 #endif
 	{
 	}
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 870c39537dd0..fca3cb329e1d 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -55,6 +55,7 @@ WORKAROUND_1418040
 WORKAROUND_1463225
 WORKAROUND_1508412
 WORKAROUND_1542419
+WORKAROUND_2064142
 WORKAROUND_TRBE_OVERWRITE_FILL_MODE
 WORKAROUND_TSB_FLUSH_FAILURE
 WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 276862c07e32..ec24b62b2cec 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -91,10 +91,12 @@ struct trbe_buf {
  */
 #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE	0
 #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE	1
+#define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE	2
 
 static int trbe_errata_cpucaps[] = {
 	[TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
 	[TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
+	[TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
 	-1,		/* Sentinel, must be the last entry */
 };
 
@@ -167,6 +169,11 @@ static inline bool trbe_may_write_out_of_range(struct trbe_cpudata *cpudata)
 	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_WRITE_OUT_OF_RANGE);
 }
 
+static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
+{
+	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_SYSREG_WRITE_FAILURE);
+}
+
 static int trbe_alloc_node(struct perf_event *event)
 {
 	if (event->cpu == -1)
@@ -174,30 +181,42 @@ static int trbe_alloc_node(struct perf_event *event)
 	return cpu_to_node(event->cpu);
 }
 
-static void trbe_drain_buffer(void)
+static inline void trbe_drain_buffer(void)
 {
 	tsb_csync();
 	dsb(nsh);
 }
 
-static void trbe_drain_and_disable_local(void)
+static inline void set_trbe_disabled(struct trbe_cpudata *cpudata)
 {
 	u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
 
-	trbe_drain_buffer();
-
 	/*
 	 * Disable the TRBE without clearing LIMITPTR which
 	 * might be required for fetching the buffer limits.
 	 */
 	trblimitr &= ~TRBLIMITR_ENABLE;
 	write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
+
+	/*
+	 * Errata affected TRBE implementation will need TSB CSYNC and
+	 * DSB in order to prevent subsequent writes into certain TRBE
+	 * system registers from being ignored and not effected.
+	 */
+	if (trbe_may_fail_sysreg_write(cpudata))
+		trbe_drain_buffer();
 	isb();
 }
 
-static void trbe_reset_local(void)
+static void trbe_drain_and_disable_local(struct trbe_cpudata *cpudata)
 {
-	trbe_drain_and_disable_local();
+	trbe_drain_buffer();
+	set_trbe_disabled(cpudata);
+}
+
+static void trbe_reset_local(struct trbe_cpudata *cpudata)
+{
+	trbe_drain_and_disable_local(cpudata);
 	write_sysreg_s(0, SYS_TRBLIMITR_EL1);
 	write_sysreg_s(0, SYS_TRBPTR_EL1);
 	write_sysreg_s(0, SYS_TRBBASER_EL1);
@@ -234,7 +253,7 @@ static void trbe_stop_and_truncate_event(struct perf_output_handle *handle)
 	 * at event_stop(). So disable the TRBE here and leave
 	 * the update_buffer() to return a 0 size.
 	 */
-	trbe_drain_and_disable_local();
+	trbe_drain_and_disable_local(buf->cpudata);
 	perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
 	perf_aux_output_end(handle, 0);
 	*this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL;
@@ -579,8 +598,7 @@ static void trbe_enable_hw(struct trbe_buf *buf)
 	WARN_ON(buf->trbe_hw_base < buf->trbe_base);
 	WARN_ON(buf->trbe_write < buf->trbe_hw_base);
 	WARN_ON(buf->trbe_write >= buf->trbe_limit);
-	set_trbe_disabled();
-	isb();
+	set_trbe_disabled(buf->cpudata);
 	clr_trbe_status();
 	set_trbe_base_pointer(buf->trbe_hw_base);
 	set_trbe_write_pointer(buf->trbe_write);
@@ -775,7 +793,7 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev,
 	 * the TRBE here will ensure that no IRQ could be generated when the perf
 	 * handle gets freed in etm_event_stop().
 	 */
-	trbe_drain_and_disable_local();
+	trbe_drain_and_disable_local(cpudata);
 
 	/* Check if there is a pending interrupt and handle it here */
 	status = read_sysreg_s(SYS_TRBSR_EL1);
@@ -986,7 +1004,7 @@ static int arm_trbe_disable(struct coresight_device *csdev)
 	if (cpudata->mode != CS_MODE_PERF)
 		return -EINVAL;
 
-	trbe_drain_and_disable_local();
+	trbe_drain_and_disable_local(cpudata);
 	buf->cpudata = NULL;
 	cpudata->buf = NULL;
 	cpudata->mode = CS_MODE_DISABLED;
@@ -1028,7 +1046,7 @@ static int trbe_handle_overflow(struct perf_output_handle *handle)
 		 * is able to detect this with a disconnected handle
 		 * (handle->event = NULL).
 		 */
-		trbe_drain_and_disable_local();
+		trbe_drain_and_disable_local(buf->cpudata);
 		*this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL;
 		return -EINVAL;
 	}
@@ -1062,6 +1080,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 {
 	struct perf_output_handle **handle_ptr = dev;
 	struct perf_output_handle *handle = *handle_ptr;
+	struct trbe_buf *buf = etm_perf_sink_config(handle);
 	enum trbe_fault_action act;
 	u64 status;
 	bool truncated = false;
@@ -1082,7 +1101,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 	 * Ensure the trace is visible to the CPUs and
 	 * any external aborts have been resolved.
 	 */
-	trbe_drain_and_disable_local();
+	trbe_drain_and_disable_local(buf->cpudata);
 	clr_trbe_irq();
 	isb();
 
@@ -1167,8 +1186,9 @@ static const struct attribute_group *arm_trbe_groups[] = {
 static void arm_trbe_enable_cpu(void *info)
 {
 	struct trbe_drvdata *drvdata = info;
+	struct trbe_cpudata *cpudata = this_cpu_ptr(drvdata->cpudata);
 
-	trbe_reset_local();
+	trbe_reset_local(cpudata);
 	enable_percpu_irq(drvdata->irq, IRQ_TYPE_NONE);
 }
 
@@ -1276,7 +1296,7 @@ static void arm_trbe_remove_coresight_cpu(void *info)
 	struct coresight_device *trbe_csdev = coresight_get_percpu_sink(cpu);
 
 	disable_percpu_irq(drvdata->irq);
-	trbe_reset_local();
+	trbe_reset_local(cpudata);
 	if (trbe_csdev) {
 		coresight_unregister(trbe_csdev);
 		cpudata->drvdata = NULL;
@@ -1349,8 +1369,10 @@ static int arm_trbe_cpu_teardown(unsigned int cpu, struct hlist_node *node)
 	struct trbe_drvdata *drvdata = hlist_entry_safe(node, struct trbe_drvdata, hotplug_node);
 
 	if (cpumask_test_cpu(cpu, &drvdata->supported_cpus)) {
+		struct trbe_cpudata *cpudata = per_cpu_ptr(drvdata->cpudata, cpu);
+
 		disable_percpu_irq(drvdata->irq);
-		trbe_reset_local();
+		trbe_reset_local(cpudata);
 	}
 	return 0;
 }
diff --git a/drivers/hwtracing/coresight/coresight-trbe.h b/drivers/hwtracing/coresight/coresight-trbe.h
index abf3e36082f0..30e4d7db4f8e 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.h
+++ b/drivers/hwtracing/coresight/coresight-trbe.h
@@ -91,14 +91,6 @@ static inline bool is_trbe_running(u64 trbsr)
 #define TRBE_FILL_MODE_WRAP		1
 #define TRBE_FILL_MODE_CIRCULAR_BUFFER	3
 
-static inline void set_trbe_disabled(void)
-{
-	u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
-
-	trblimitr &= ~TRBLIMITR_ENABLE;
-	write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
-}
-
 static inline bool get_trbe_flag_update(u64 trbidr)
 {
 	return trbidr & TRBIDR_FLAG;
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] coresight: trbe: Work around the ignored system register writes

[Suzuki K Poulose]

Hi Anshuman

On 05/01/2022 05:05, Anshuman Khandual wrote:
> TRBE implementations affected by Arm erratum #2064142 might fail to write
> into certain system registers after the TRBE has been disabled. Under some
> conditions after TRBE has been disabled, writes into certain TRBE registers
> TRBLIMITR_EL1, TRBPTR_EL1, TRBBASER_EL1, TRBSR_EL1 and TRBTRG_EL1 will be
> ignored and not be effected.
> 
> Work around this problem in the TRBE driver by executing TSB CSYNC and DSB
> just after the trace collection has stopped and before performing a system
> register write to one of the affected registers. This adds a new cpu errata
> in arm64 errata framework and also updates TRBE driver as required.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
> Cc: coresight@lists.linaro.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>   Documentation/arm64/silicon-errata.rst       |  2 +
>   arch/arm64/Kconfig                           | 18 +++++++
>   arch/arm64/kernel/cpu_errata.c               |  9 ++++
>   arch/arm64/tools/cpucaps                     |  1 +
>   drivers/hwtracing/coresight/coresight-trbe.c | 54 ++++++++++++++------
>   drivers/hwtracing/coresight/coresight-trbe.h |  8 ---
>   6 files changed, 68 insertions(+), 24 deletions(-)

I think it may be a good idea to split the patch into
two parts :

- arm64 kernel part. (cpucap defintions and detection)
- trbe driver part

So that it is easier to merge these series upstream to
avoid conflicts.

> 
> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
> index 5342e895fb60..c9b30e6c2b6c 100644
> --- a/Documentation/arm64/silicon-errata.rst
> +++ b/Documentation/arm64/silicon-errata.rst
> @@ -52,6 +52,8 @@ stable kernels.
>   | Allwinner      | A64/R18         | UNKNOWN1        | SUN50I_ERRATUM_UNKNOWN1     |
>   +----------------+-----------------+-----------------+-----------------------------+
>   +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
> ++----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>   +----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index c4207cf9bb17..2105b68d88db 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -778,6 +778,24 @@ config ARM64_ERRATUM_2224489
>   
>   	  If unsure, say Y.
>   
> +config ARM64_ERRATUM_2064142
> +	bool "Cortex-A510: 2064142: workaround TRBE register writes while disabled"
> +	depends on CORESIGHT_TRBE
> +	default y
> +	help
> +	  This option adds the workaround for ARM Cortex-A510 erratum 2064142.
> +
> +	  Affected Cortex-A510 core might fail to write into system registers after the
> +	  TRBE has been disabled. Under some conditions after the TRBE has been disabled
> +	  writes into TRBE registers TRBLIMITR_EL1, TRBPTR_EL1, TRBBASER_EL1, TRBSR_EL1,
> +	  and TRBTRG_EL1 will be ignored and will not be effected.
> +
> +	  Work around this in the driver by executing TSB CSYNC and DSB after collection
> +	  is stopped and before performing a system register write to one of the affected
> +	  registers.
> +
> +	  If unsure, say Y.
> +
>   config CAVIUM_ERRATUM_22375
>   	bool "Cavium erratum 22375, 24313"
>   	default y
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 9e1c1aef9ebd..cbb7d5a9aee7 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -597,6 +597,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>   		.type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,
>   		CAP_MIDR_RANGE_LIST(trbe_write_out_of_range_cpus),
>   	},
> +#endif
> +#ifdef CONFIG_ARM64_ERRATUM_2064142
> +	{
> +		.desc = "ARM erratum 2064142",
> +		.capability = ARM64_WORKAROUND_2064142,
> +
> +		/* Cortex-A510 r0p0 - r0p2 */
> +		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
> +	},
>   #endif
>   	{
>   	}
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> index 870c39537dd0..fca3cb329e1d 100644
> --- a/arch/arm64/tools/cpucaps
> +++ b/arch/arm64/tools/cpucaps
> @@ -55,6 +55,7 @@ WORKAROUND_1418040
>   WORKAROUND_1463225
>   WORKAROUND_1508412
>   WORKAROUND_1542419
> +WORKAROUND_2064142
>   WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>   WORKAROUND_TSB_FLUSH_FAILURE
>   WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 276862c07e32..ec24b62b2cec 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -91,10 +91,12 @@ struct trbe_buf {
>    */
>   #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE	0
>   #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE	1
> +#define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE	2
>   
>   static int trbe_errata_cpucaps[] = {
>   	[TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>   	[TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
> +	[TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,

super minor nit: TRBE_NEEDS_DRAIN_AFTER_DISABLE ?

>   	-1,		/* Sentinel, must be the last entry */
>   };
>   
> @@ -167,6 +169,11 @@ static inline bool trbe_may_write_out_of_range(struct trbe_cpudata *cpudata)
>   	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_WRITE_OUT_OF_RANGE);
>   }
>   
> +static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
> +{

minor nit: trbe_needs_drain_after_disable() ?

Either ways, the patch looks good to me.

Suzuki

Re: [PATCH 2/4] coresight: trbe: Work around the ignored system register writes

[Suzuki K Poulose]

Hi Anshuman

On 05/01/2022 05:05, Anshuman Khandual wrote:
> TRBE implementations affected by Arm erratum #2064142 might fail to write
> into certain system registers after the TRBE has been disabled. Under some
> conditions after TRBE has been disabled, writes into certain TRBE registers
> TRBLIMITR_EL1, TRBPTR_EL1, TRBBASER_EL1, TRBSR_EL1 and TRBTRG_EL1 will be
> ignored and not be effected.
> 
> Work around this problem in the TRBE driver by executing TSB CSYNC and DSB
> just after the trace collection has stopped and before performing a system
> register write to one of the affected registers. This adds a new cpu errata
> in arm64 errata framework and also updates TRBE driver as required.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
> Cc: coresight@lists.linaro.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>   Documentation/arm64/silicon-errata.rst       |  2 +
>   arch/arm64/Kconfig                           | 18 +++++++
>   arch/arm64/kernel/cpu_errata.c               |  9 ++++
>   arch/arm64/tools/cpucaps                     |  1 +
>   drivers/hwtracing/coresight/coresight-trbe.c | 54 ++++++++++++++------
>   drivers/hwtracing/coresight/coresight-trbe.h |  8 ---
>   6 files changed, 68 insertions(+), 24 deletions(-)

I think it may be a good idea to split the patch into
two parts :

- arm64 kernel part. (cpucap defintions and detection)
- trbe driver part

So that it is easier to merge these series upstream to
avoid conflicts.

> 
> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
> index 5342e895fb60..c9b30e6c2b6c 100644
> --- a/Documentation/arm64/silicon-errata.rst
> +++ b/Documentation/arm64/silicon-errata.rst
> @@ -52,6 +52,8 @@ stable kernels.
>   | Allwinner      | A64/R18         | UNKNOWN1        | SUN50I_ERRATUM_UNKNOWN1     |
>   +----------------+-----------------+-----------------+-----------------------------+
>   +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
> ++----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>   +----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index c4207cf9bb17..2105b68d88db 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -778,6 +778,24 @@ config ARM64_ERRATUM_2224489
>   
>   	  If unsure, say Y.
>   
> +config ARM64_ERRATUM_2064142
> +	bool "Cortex-A510: 2064142: workaround TRBE register writes while disabled"
> +	depends on CORESIGHT_TRBE
> +	default y
> +	help
> +	  This option adds the workaround for ARM Cortex-A510 erratum 2064142.
> +
> +	  Affected Cortex-A510 core might fail to write into system registers after the
> +	  TRBE has been disabled. Under some conditions after the TRBE has been disabled
> +	  writes into TRBE registers TRBLIMITR_EL1, TRBPTR_EL1, TRBBASER_EL1, TRBSR_EL1,
> +	  and TRBTRG_EL1 will be ignored and will not be effected.
> +
> +	  Work around this in the driver by executing TSB CSYNC and DSB after collection
> +	  is stopped and before performing a system register write to one of the affected
> +	  registers.
> +
> +	  If unsure, say Y.
> +
>   config CAVIUM_ERRATUM_22375
>   	bool "Cavium erratum 22375, 24313"
>   	default y
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 9e1c1aef9ebd..cbb7d5a9aee7 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -597,6 +597,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>   		.type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,
>   		CAP_MIDR_RANGE_LIST(trbe_write_out_of_range_cpus),
>   	},
> +#endif
> +#ifdef CONFIG_ARM64_ERRATUM_2064142
> +	{
> +		.desc = "ARM erratum 2064142",
> +		.capability = ARM64_WORKAROUND_2064142,
> +
> +		/* Cortex-A510 r0p0 - r0p2 */
> +		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
> +	},
>   #endif
>   	{
>   	}
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> index 870c39537dd0..fca3cb329e1d 100644
> --- a/arch/arm64/tools/cpucaps
> +++ b/arch/arm64/tools/cpucaps
> @@ -55,6 +55,7 @@ WORKAROUND_1418040
>   WORKAROUND_1463225
>   WORKAROUND_1508412
>   WORKAROUND_1542419
> +WORKAROUND_2064142
>   WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>   WORKAROUND_TSB_FLUSH_FAILURE
>   WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 276862c07e32..ec24b62b2cec 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -91,10 +91,12 @@ struct trbe_buf {
>    */
>   #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE	0
>   #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE	1
> +#define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE	2
>   
>   static int trbe_errata_cpucaps[] = {
>   	[TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>   	[TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
> +	[TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,

super minor nit: TRBE_NEEDS_DRAIN_AFTER_DISABLE ?

>   	-1,		/* Sentinel, must be the last entry */
>   };
>   
> @@ -167,6 +169,11 @@ static inline bool trbe_may_write_out_of_range(struct trbe_cpudata *cpudata)
>   	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_WRITE_OUT_OF_RANGE);
>   }
>   
> +static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
> +{

minor nit: trbe_needs_drain_after_disable() ?

Either ways, the patch looks good to me.

Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] coresight: trbe: Work around the ignored system register writes

[Anshuman Khandual]

On 1/5/22 3:31 PM, Suzuki K Poulose wrote:
> Hi Anshuman
> 
> On 05/01/2022 05:05, Anshuman Khandual wrote:
>> TRBE implementations affected by Arm erratum #2064142 might fail to write
>> into certain system registers after the TRBE has been disabled. Under some
>> conditions after TRBE has been disabled, writes into certain TRBE registers
>> TRBLIMITR_EL1, TRBPTR_EL1, TRBBASER_EL1, TRBSR_EL1 and TRBTRG_EL1 will be
>> ignored and not be effected.
>>
>> Work around this problem in the TRBE driver by executing TSB CSYNC and DSB
>> just after the trace collection has stopped and before performing a system
>> register write to one of the affected registers. This adds a new cpu errata
>> in arm64 errata framework and also updates TRBE driver as required.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
>> Cc: coresight@lists.linaro.org
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>   Documentation/arm64/silicon-errata.rst       |  2 +
>>   arch/arm64/Kconfig                           | 18 +++++++
>>   arch/arm64/kernel/cpu_errata.c               |  9 ++++
>>   arch/arm64/tools/cpucaps                     |  1 +
>>   drivers/hwtracing/coresight/coresight-trbe.c | 54 ++++++++++++++------
>>   drivers/hwtracing/coresight/coresight-trbe.h |  8 ---
>>   6 files changed, 68 insertions(+), 24 deletions(-)
> 
> I think it may be a good idea to split the patch into
> two parts :
> 
> - arm64 kernel part. (cpucap defintions and detection)
> - trbe driver part
> 
> So that it is easier to merge these series upstream to
> avoid conflicts.

Sure, will do.

> 
>>
>> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
>> index 5342e895fb60..c9b30e6c2b6c 100644
>> --- a/Documentation/arm64/silicon-errata.rst
>> +++ b/Documentation/arm64/silicon-errata.rst
>> @@ -52,6 +52,8 @@ stable kernels.
>>   | Allwinner      | A64/R18         | UNKNOWN1        | SUN50I_ERRATUM_UNKNOWN1     |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index c4207cf9bb17..2105b68d88db 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -778,6 +778,24 @@ config ARM64_ERRATUM_2224489
>>           If unsure, say Y.
>>   +config ARM64_ERRATUM_2064142
>> +    bool "Cortex-A510: 2064142: workaround TRBE register writes while disabled"
>> +    depends on CORESIGHT_TRBE
>> +    default y
>> +    help
>> +      This option adds the workaround for ARM Cortex-A510 erratum 2064142.
>> +
>> +      Affected Cortex-A510 core might fail to write into system registers after the
>> +      TRBE has been disabled. Under some conditions after the TRBE has been disabled
>> +      writes into TRBE registers TRBLIMITR_EL1, TRBPTR_EL1, TRBBASER_EL1, TRBSR_EL1,
>> +      and TRBTRG_EL1 will be ignored and will not be effected.
>> +
>> +      Work around this in the driver by executing TSB CSYNC and DSB after collection
>> +      is stopped and before performing a system register write to one of the affected
>> +      registers.
>> +
>> +      If unsure, say Y.
>> +
>>   config CAVIUM_ERRATUM_22375
>>       bool "Cavium erratum 22375, 24313"
>>       default y
>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>> index 9e1c1aef9ebd..cbb7d5a9aee7 100644
>> --- a/arch/arm64/kernel/cpu_errata.c
>> +++ b/arch/arm64/kernel/cpu_errata.c
>> @@ -597,6 +597,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>>           .type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,
>>           CAP_MIDR_RANGE_LIST(trbe_write_out_of_range_cpus),
>>       },
>> +#endif
>> +#ifdef CONFIG_ARM64_ERRATUM_2064142
>> +    {
>> +        .desc = "ARM erratum 2064142",
>> +        .capability = ARM64_WORKAROUND_2064142,
>> +
>> +        /* Cortex-A510 r0p0 - r0p2 */
>> +        ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>> +    },
>>   #endif
>>       {
>>       }
>> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>> index 870c39537dd0..fca3cb329e1d 100644
>> --- a/arch/arm64/tools/cpucaps
>> +++ b/arch/arm64/tools/cpucaps
>> @@ -55,6 +55,7 @@ WORKAROUND_1418040
>>   WORKAROUND_1463225
>>   WORKAROUND_1508412
>>   WORKAROUND_1542419
>> +WORKAROUND_2064142
>>   WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>>   WORKAROUND_TSB_FLUSH_FAILURE
>>   WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>> index 276862c07e32..ec24b62b2cec 100644
>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>> @@ -91,10 +91,12 @@ struct trbe_buf {
>>    */
>>   #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE    0
>>   #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE    1
>> +#define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE    2
>>     static int trbe_errata_cpucaps[] = {
>>       [TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>>       [TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>> +    [TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
> 
> super minor nit: TRBE_NEEDS_DRAIN_AFTER_DISABLE
>>>       -1,        /* Sentinel, must be the last entry */
>>   };
>>   @@ -167,6 +169,11 @@ static inline bool trbe_may_write_out_of_range(struct trbe_cpudata *cpudata)
>>       return trbe_has_erratum(cpudata, TRBE_WORKAROUND_WRITE_OUT_OF_RANGE);
>>   }
>>   +static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
>> +{
> 
> minor nit: trbe_needs_drain_after_disable() ?

Changing this helper name and its index above would reflect actual
workaround implemented, instead of the errata which it is trying
to solve. Although the series has already broken that convention
in the last patch with trbe_is_broken() and TRBE_IS_BROKEN. If we
are going with TRBE_<work around required> format, I will have to
change the next workaround patch as well.

> 
> Either ways, the patch looks good to me.> 
> Suzuki

Re: [PATCH 2/4] coresight: trbe: Work around the ignored system register writes

[Anshuman Khandual]

On 1/5/22 3:31 PM, Suzuki K Poulose wrote:
> Hi Anshuman
> 
> On 05/01/2022 05:05, Anshuman Khandual wrote:
>> TRBE implementations affected by Arm erratum #2064142 might fail to write
>> into certain system registers after the TRBE has been disabled. Under some
>> conditions after TRBE has been disabled, writes into certain TRBE registers
>> TRBLIMITR_EL1, TRBPTR_EL1, TRBBASER_EL1, TRBSR_EL1 and TRBTRG_EL1 will be
>> ignored and not be effected.
>>
>> Work around this problem in the TRBE driver by executing TSB CSYNC and DSB
>> just after the trace collection has stopped and before performing a system
>> register write to one of the affected registers. This adds a new cpu errata
>> in arm64 errata framework and also updates TRBE driver as required.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
>> Cc: coresight@lists.linaro.org
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>   Documentation/arm64/silicon-errata.rst       |  2 +
>>   arch/arm64/Kconfig                           | 18 +++++++
>>   arch/arm64/kernel/cpu_errata.c               |  9 ++++
>>   arch/arm64/tools/cpucaps                     |  1 +
>>   drivers/hwtracing/coresight/coresight-trbe.c | 54 ++++++++++++++------
>>   drivers/hwtracing/coresight/coresight-trbe.h |  8 ---
>>   6 files changed, 68 insertions(+), 24 deletions(-)
> 
> I think it may be a good idea to split the patch into
> two parts :
> 
> - arm64 kernel part. (cpucap defintions and detection)
> - trbe driver part
> 
> So that it is easier to merge these series upstream to
> avoid conflicts.

Sure, will do.

> 
>>
>> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
>> index 5342e895fb60..c9b30e6c2b6c 100644
>> --- a/Documentation/arm64/silicon-errata.rst
>> +++ b/Documentation/arm64/silicon-errata.rst
>> @@ -52,6 +52,8 @@ stable kernels.
>>   | Allwinner      | A64/R18         | UNKNOWN1        | SUN50I_ERRATUM_UNKNOWN1     |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index c4207cf9bb17..2105b68d88db 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -778,6 +778,24 @@ config ARM64_ERRATUM_2224489
>>           If unsure, say Y.
>>   +config ARM64_ERRATUM_2064142
>> +    bool "Cortex-A510: 2064142: workaround TRBE register writes while disabled"
>> +    depends on CORESIGHT_TRBE
>> +    default y
>> +    help
>> +      This option adds the workaround for ARM Cortex-A510 erratum 2064142.
>> +
>> +      Affected Cortex-A510 core might fail to write into system registers after the
>> +      TRBE has been disabled. Under some conditions after the TRBE has been disabled
>> +      writes into TRBE registers TRBLIMITR_EL1, TRBPTR_EL1, TRBBASER_EL1, TRBSR_EL1,
>> +      and TRBTRG_EL1 will be ignored and will not be effected.
>> +
>> +      Work around this in the driver by executing TSB CSYNC and DSB after collection
>> +      is stopped and before performing a system register write to one of the affected
>> +      registers.
>> +
>> +      If unsure, say Y.
>> +
>>   config CAVIUM_ERRATUM_22375
>>       bool "Cavium erratum 22375, 24313"
>>       default y
>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>> index 9e1c1aef9ebd..cbb7d5a9aee7 100644
>> --- a/arch/arm64/kernel/cpu_errata.c
>> +++ b/arch/arm64/kernel/cpu_errata.c
>> @@ -597,6 +597,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>>           .type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,
>>           CAP_MIDR_RANGE_LIST(trbe_write_out_of_range_cpus),
>>       },
>> +#endif
>> +#ifdef CONFIG_ARM64_ERRATUM_2064142
>> +    {
>> +        .desc = "ARM erratum 2064142",
>> +        .capability = ARM64_WORKAROUND_2064142,
>> +
>> +        /* Cortex-A510 r0p0 - r0p2 */
>> +        ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>> +    },
>>   #endif
>>       {
>>       }
>> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>> index 870c39537dd0..fca3cb329e1d 100644
>> --- a/arch/arm64/tools/cpucaps
>> +++ b/arch/arm64/tools/cpucaps
>> @@ -55,6 +55,7 @@ WORKAROUND_1418040
>>   WORKAROUND_1463225
>>   WORKAROUND_1508412
>>   WORKAROUND_1542419
>> +WORKAROUND_2064142
>>   WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>>   WORKAROUND_TSB_FLUSH_FAILURE
>>   WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>> index 276862c07e32..ec24b62b2cec 100644
>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>> @@ -91,10 +91,12 @@ struct trbe_buf {
>>    */
>>   #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE    0
>>   #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE    1
>> +#define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE    2
>>     static int trbe_errata_cpucaps[] = {
>>       [TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>>       [TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>> +    [TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
> 
> super minor nit: TRBE_NEEDS_DRAIN_AFTER_DISABLE
>>>       -1,        /* Sentinel, must be the last entry */
>>   };
>>   @@ -167,6 +169,11 @@ static inline bool trbe_may_write_out_of_range(struct trbe_cpudata *cpudata)
>>       return trbe_has_erratum(cpudata, TRBE_WORKAROUND_WRITE_OUT_OF_RANGE);
>>   }
>>   +static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
>> +{
> 
> minor nit: trbe_needs_drain_after_disable() ?

Changing this helper name and its index above would reflect actual
workaround implemented, instead of the errata which it is trying
to solve. Although the series has already broken that convention
in the last patch with trbe_is_broken() and TRBE_IS_BROKEN. If we
are going with TRBE_<work around required> format, I will have to
change the next workaround patch as well.

> 
> Either ways, the patch looks good to me.> 
> Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/4] coresight: trbe: Work around the invalid prohibited states

[Anshuman Khandual]

TRBE implementations affected by Arm erratum #2038923 might get TRBE into
an inconsistent view on whether trace is prohibited within the CPU. As a
result, the trace buffer or trace buffer state might be corrupted. This
happens after TRBE buffer has been enabled by setting TRBLIMITR_EL1.E,
followed by just a single context synchronization event before execution
changes from a context, in which trace is prohibited to one where it isn't,
or vice versa. In these mentioned conditions, the view of whether trace is
prohibited is inconsistent between parts of the CPU, and the trace buffer
or the trace buffer state might be corrupted.

Work around this problem in the TRBE driver by preventing an inconsistent
view of whether the trace is prohibited or not based on TRBLIMITR_EL1.E by
immediately following a change to TRBLIMITR_EL1.E with at least one ISB
instruction before an ERET, or two ISB instructions if no ERET is to take
place. This adds a new cpu errata in arm64 errata framework and also
updates TRBE driver as required.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Suzuki Poulose <suzuki.poulose@arm.com>
Cc: coresight@lists.linaro.org
Cc: linux-doc@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 Documentation/arm64/silicon-errata.rst       |  2 +
 arch/arm64/Kconfig                           | 23 ++++++++++
 arch/arm64/kernel/cpu_errata.c               |  9 ++++
 arch/arm64/tools/cpucaps                     |  1 +
 drivers/hwtracing/coresight/coresight-trbe.c | 47 +++++++++++++++-----
 5 files changed, 72 insertions(+), 10 deletions(-)

diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
index c9b30e6c2b6c..e0ef3e9a4b8b 100644
--- a/Documentation/arm64/silicon-errata.rst
+++ b/Documentation/arm64/silicon-errata.rst
@@ -54,6 +54,8 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
 +----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
++----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 2105b68d88db..026e34fb6fad 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -796,6 +796,29 @@ config ARM64_ERRATUM_2064142
 
 	  If unsure, say Y.
 
+config ARM64_ERRATUM_2038923
+	bool "Cortex-A510: 2038923: workaround TRBE corruption with enable"
+	depends on CORESIGHT_TRBE
+	default y
+	help
+	  This option adds the workaround for ARM Cortex-A510 erratum 2038923.
+
+	  Affected Cortex-A510 core might cause an inconsistent view on whether trace is
+	  prohibited within the CPU. As a result, the trace buffer or trace buffer state
+	  might be corrupted. This happens after TRBE buffer has been enabled by setting
+	  TRBLIMITR_EL1.E, followed by just a single context synchronization event before
+	  execution changes from a context, in which trace is prohibited to one where it
+	  isn't, or vice versa. In these mentioned conditions, the view of whether trace
+	  is prohibited is inconsistent between parts of the CPU, and the trace buffer or
+	  the trace buffer state might be corrupted.
+
+	  Work around this in the driver by preventing an inconsistent view of whether the
+	  trace is prohibited or not based on TRBLIMITR_EL1.E by immediately following a
+	  change to TRBLIMITR_EL1.E with at least one ISB instruction before an ERET, or
+	  two ISB instructions if no ERET is to take place.
+
+	  If unsure, say Y.
+
 config CAVIUM_ERRATUM_22375
 	bool "Cavium erratum 22375, 24313"
 	default y
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index cbb7d5a9aee7..60b0c1f1d912 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -607,6 +607,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
 	},
 #endif
+#ifdef CONFIG_ARM64_ERRATUM_2038923
+	{
+		.desc = "ARM erratum 2038923",
+		.capability = ARM64_WORKAROUND_2038923,
+
+		/* Cortex-A510 r0p0 - r0p2 */
+		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
+	},
+#endif
 	{
 	}
 };
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index fca3cb329e1d..45a06d36d080 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -56,6 +56,7 @@ WORKAROUND_1463225
 WORKAROUND_1508412
 WORKAROUND_1542419
 WORKAROUND_2064142
+WORKAROUND_2038923
 WORKAROUND_TRBE_OVERWRITE_FILL_MODE
 WORKAROUND_TSB_FLUSH_FAILURE
 WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index ec24b62b2cec..0689c6dab96d 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -92,11 +92,13 @@ struct trbe_buf {
 #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE	0
 #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE	1
 #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE	2
+#define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE	3
 
 static int trbe_errata_cpucaps[] = {
 	[TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
 	[TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
 	[TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
+	[TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
 	-1,		/* Sentinel, must be the last entry */
 };
 
@@ -174,6 +176,11 @@ static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
 	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_SYSREG_WRITE_FAILURE);
 }
 
+static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
+{
+	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
+}
+
 static int trbe_alloc_node(struct perf_event *event)
 {
 	if (event->cpu == -1)
@@ -187,6 +194,30 @@ static inline void trbe_drain_buffer(void)
 	dsb(nsh);
 }
 
+static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
+{
+	u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+
+	/*
+	 * Enable the TRBE without clearing LIMITPTR which
+	 * might be required for fetching the buffer limits.
+	 */
+	trblimitr |= TRBLIMITR_ENABLE;
+	write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
+
+	/* Synchronize the TRBE enable event */
+	isb();
+
+	/*
+	 * Errata affected TRBE implementation will need an additional
+	 * context synchronization in order to prevent an inconsistent
+	 * TRBE prohibited region view on the CPU which could possibly
+	 * corrupt the TRBE buffer or the TRBE state.
+	 */
+	if (trbe_may_corrupt_with_enable(cpudata))
+		isb();
+}
+
 static inline void set_trbe_disabled(struct trbe_cpudata *cpudata)
 {
 	u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
@@ -555,9 +586,10 @@ static void clr_trbe_status(void)
 	write_sysreg_s(trbsr, SYS_TRBSR_EL1);
 }
 
-static void set_trbe_limit_pointer_enabled(unsigned long addr)
+static void set_trbe_limit_pointer_enabled(struct trbe_buf *buf)
 {
 	u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	unsigned long addr = buf->trbe_limit;
 
 	WARN_ON(!IS_ALIGNED(addr, (1UL << TRBLIMITR_LIMIT_SHIFT)));
 	WARN_ON(!IS_ALIGNED(addr, PAGE_SIZE));
@@ -586,11 +618,8 @@ static void set_trbe_limit_pointer_enabled(unsigned long addr)
 		      TRBLIMITR_TRIG_MODE_SHIFT;
 	trblimitr |= (addr & PAGE_MASK);
 
-	trblimitr |= TRBLIMITR_ENABLE;
 	write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
-
-	/* Synchronize the TRBE enable event */
-	isb();
+	set_trbe_enabled(buf->cpudata);
 }
 
 static void trbe_enable_hw(struct trbe_buf *buf)
@@ -608,7 +637,7 @@ static void trbe_enable_hw(struct trbe_buf *buf)
 	 * till now before enabling the TRBE.
 	 */
 	isb();
-	set_trbe_limit_pointer_enabled(buf->trbe_limit);
+	set_trbe_limit_pointer_enabled(buf);
 }
 
 static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *handle,
@@ -1013,16 +1042,14 @@ static int arm_trbe_disable(struct coresight_device *csdev)
 
 static void trbe_handle_spurious(struct perf_output_handle *handle)
 {
-	u64 limitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	struct trbe_buf *buf = etm_perf_sink_config(handle);
 
 	/*
 	 * If the IRQ was spurious, simply re-enable the TRBE
 	 * back without modifying the buffer parameters to
 	 * retain the trace collected so far.
 	 */
-	limitr |= TRBLIMITR_ENABLE;
-	write_sysreg_s(limitr, SYS_TRBLIMITR_EL1);
-	isb();
+	set_trbe_enabled(buf->cpudata);
 }
 
 static int trbe_handle_overflow(struct perf_output_handle *handle)
-- 
2.25.1

[PATCH 3/4] coresight: trbe: Work around the invalid prohibited states

[Anshuman Khandual]

TRBE implementations affected by Arm erratum #2038923 might get TRBE into
an inconsistent view on whether trace is prohibited within the CPU. As a
result, the trace buffer or trace buffer state might be corrupted. This
happens after TRBE buffer has been enabled by setting TRBLIMITR_EL1.E,
followed by just a single context synchronization event before execution
changes from a context, in which trace is prohibited to one where it isn't,
or vice versa. In these mentioned conditions, the view of whether trace is
prohibited is inconsistent between parts of the CPU, and the trace buffer
or the trace buffer state might be corrupted.

Work around this problem in the TRBE driver by preventing an inconsistent
view of whether the trace is prohibited or not based on TRBLIMITR_EL1.E by
immediately following a change to TRBLIMITR_EL1.E with at least one ISB
instruction before an ERET, or two ISB instructions if no ERET is to take
place. This adds a new cpu errata in arm64 errata framework and also
updates TRBE driver as required.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Suzuki Poulose <suzuki.poulose@arm.com>
Cc: coresight@lists.linaro.org
Cc: linux-doc@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 Documentation/arm64/silicon-errata.rst       |  2 +
 arch/arm64/Kconfig                           | 23 ++++++++++
 arch/arm64/kernel/cpu_errata.c               |  9 ++++
 arch/arm64/tools/cpucaps                     |  1 +
 drivers/hwtracing/coresight/coresight-trbe.c | 47 +++++++++++++++-----
 5 files changed, 72 insertions(+), 10 deletions(-)

diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
index c9b30e6c2b6c..e0ef3e9a4b8b 100644
--- a/Documentation/arm64/silicon-errata.rst
+++ b/Documentation/arm64/silicon-errata.rst
@@ -54,6 +54,8 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
 +----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
++----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 2105b68d88db..026e34fb6fad 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -796,6 +796,29 @@ config ARM64_ERRATUM_2064142
 
 	  If unsure, say Y.
 
+config ARM64_ERRATUM_2038923
+	bool "Cortex-A510: 2038923: workaround TRBE corruption with enable"
+	depends on CORESIGHT_TRBE
+	default y
+	help
+	  This option adds the workaround for ARM Cortex-A510 erratum 2038923.
+
+	  Affected Cortex-A510 core might cause an inconsistent view on whether trace is
+	  prohibited within the CPU. As a result, the trace buffer or trace buffer state
+	  might be corrupted. This happens after TRBE buffer has been enabled by setting
+	  TRBLIMITR_EL1.E, followed by just a single context synchronization event before
+	  execution changes from a context, in which trace is prohibited to one where it
+	  isn't, or vice versa. In these mentioned conditions, the view of whether trace
+	  is prohibited is inconsistent between parts of the CPU, and the trace buffer or
+	  the trace buffer state might be corrupted.
+
+	  Work around this in the driver by preventing an inconsistent view of whether the
+	  trace is prohibited or not based on TRBLIMITR_EL1.E by immediately following a
+	  change to TRBLIMITR_EL1.E with at least one ISB instruction before an ERET, or
+	  two ISB instructions if no ERET is to take place.
+
+	  If unsure, say Y.
+
 config CAVIUM_ERRATUM_22375
 	bool "Cavium erratum 22375, 24313"
 	default y
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index cbb7d5a9aee7..60b0c1f1d912 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -607,6 +607,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
 	},
 #endif
+#ifdef CONFIG_ARM64_ERRATUM_2038923
+	{
+		.desc = "ARM erratum 2038923",
+		.capability = ARM64_WORKAROUND_2038923,
+
+		/* Cortex-A510 r0p0 - r0p2 */
+		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
+	},
+#endif
 	{
 	}
 };
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index fca3cb329e1d..45a06d36d080 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -56,6 +56,7 @@ WORKAROUND_1463225
 WORKAROUND_1508412
 WORKAROUND_1542419
 WORKAROUND_2064142
+WORKAROUND_2038923
 WORKAROUND_TRBE_OVERWRITE_FILL_MODE
 WORKAROUND_TSB_FLUSH_FAILURE
 WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index ec24b62b2cec..0689c6dab96d 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -92,11 +92,13 @@ struct trbe_buf {
 #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE	0
 #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE	1
 #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE	2
+#define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE	3
 
 static int trbe_errata_cpucaps[] = {
 	[TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
 	[TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
 	[TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
+	[TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
 	-1,		/* Sentinel, must be the last entry */
 };
 
@@ -174,6 +176,11 @@ static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
 	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_SYSREG_WRITE_FAILURE);
 }
 
+static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
+{
+	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
+}
+
 static int trbe_alloc_node(struct perf_event *event)
 {
 	if (event->cpu == -1)
@@ -187,6 +194,30 @@ static inline void trbe_drain_buffer(void)
 	dsb(nsh);
 }
 
+static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
+{
+	u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+
+	/*
+	 * Enable the TRBE without clearing LIMITPTR which
+	 * might be required for fetching the buffer limits.
+	 */
+	trblimitr |= TRBLIMITR_ENABLE;
+	write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
+
+	/* Synchronize the TRBE enable event */
+	isb();
+
+	/*
+	 * Errata affected TRBE implementation will need an additional
+	 * context synchronization in order to prevent an inconsistent
+	 * TRBE prohibited region view on the CPU which could possibly
+	 * corrupt the TRBE buffer or the TRBE state.
+	 */
+	if (trbe_may_corrupt_with_enable(cpudata))
+		isb();
+}
+
 static inline void set_trbe_disabled(struct trbe_cpudata *cpudata)
 {
 	u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
@@ -555,9 +586,10 @@ static void clr_trbe_status(void)
 	write_sysreg_s(trbsr, SYS_TRBSR_EL1);
 }
 
-static void set_trbe_limit_pointer_enabled(unsigned long addr)
+static void set_trbe_limit_pointer_enabled(struct trbe_buf *buf)
 {
 	u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	unsigned long addr = buf->trbe_limit;
 
 	WARN_ON(!IS_ALIGNED(addr, (1UL << TRBLIMITR_LIMIT_SHIFT)));
 	WARN_ON(!IS_ALIGNED(addr, PAGE_SIZE));
@@ -586,11 +618,8 @@ static void set_trbe_limit_pointer_enabled(unsigned long addr)
 		      TRBLIMITR_TRIG_MODE_SHIFT;
 	trblimitr |= (addr & PAGE_MASK);
 
-	trblimitr |= TRBLIMITR_ENABLE;
 	write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
-
-	/* Synchronize the TRBE enable event */
-	isb();
+	set_trbe_enabled(buf->cpudata);
 }
 
 static void trbe_enable_hw(struct trbe_buf *buf)
@@ -608,7 +637,7 @@ static void trbe_enable_hw(struct trbe_buf *buf)
 	 * till now before enabling the TRBE.
 	 */
 	isb();
-	set_trbe_limit_pointer_enabled(buf->trbe_limit);
+	set_trbe_limit_pointer_enabled(buf);
 }
 
 static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *handle,
@@ -1013,16 +1042,14 @@ static int arm_trbe_disable(struct coresight_device *csdev)
 
 static void trbe_handle_spurious(struct perf_output_handle *handle)
 {
-	u64 limitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	struct trbe_buf *buf = etm_perf_sink_config(handle);
 
 	/*
 	 * If the IRQ was spurious, simply re-enable the TRBE
 	 * back without modifying the buffer parameters to
 	 * retain the trace collected so far.
 	 */
-	limitr |= TRBLIMITR_ENABLE;
-	write_sysreg_s(limitr, SYS_TRBLIMITR_EL1);
-	isb();
+	set_trbe_enabled(buf->cpudata);
 }
 
 static int trbe_handle_overflow(struct perf_output_handle *handle)
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/4] coresight: trbe: Work around the invalid prohibited states

[Suzuki K Poulose]

Hi Anshuman

On 05/01/2022 05:05, Anshuman Khandual wrote:
> TRBE implementations affected by Arm erratum #2038923 might get TRBE into
> an inconsistent view on whether trace is prohibited within the CPU. As a
> result, the trace buffer or trace buffer state might be corrupted. This
> happens after TRBE buffer has been enabled by setting TRBLIMITR_EL1.E,
> followed by just a single context synchronization event before execution
> changes from a context, in which trace is prohibited to one where it isn't,
> or vice versa. In these mentioned conditions, the view of whether trace is
> prohibited is inconsistent between parts of the CPU, and the trace buffer
> or the trace buffer state might be corrupted.
> 
> Work around this problem in the TRBE driver by preventing an inconsistent
> view of whether the trace is prohibited or not based on TRBLIMITR_EL1.E by
> immediately following a change to TRBLIMITR_EL1.E with at least one ISB
> instruction before an ERET, or two ISB instructions if no ERET is to take
> place. This adds a new cpu errata in arm64 errata framework and also
> updates TRBE driver as required.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
> Cc: coresight@lists.linaro.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>   Documentation/arm64/silicon-errata.rst       |  2 +
>   arch/arm64/Kconfig                           | 23 ++++++++++
>   arch/arm64/kernel/cpu_errata.c               |  9 ++++
>   arch/arm64/tools/cpucaps                     |  1 +
>   drivers/hwtracing/coresight/coresight-trbe.c | 47 +++++++++++++++-----
>   5 files changed, 72 insertions(+), 10 deletions(-)

As with the previous patch, it may be a good idea to split the
patch to arm64 and trbe parts.

> 
> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
> index c9b30e6c2b6c..e0ef3e9a4b8b 100644
> --- a/Documentation/arm64/silicon-errata.rst
> +++ b/Documentation/arm64/silicon-errata.rst
> @@ -54,6 +54,8 @@ stable kernels.
>   +----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
>   +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
> ++----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>   +----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 2105b68d88db..026e34fb6fad 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -796,6 +796,29 @@ config ARM64_ERRATUM_2064142
>   
>   	  If unsure, say Y.
>   
> +config ARM64_ERRATUM_2038923
> +	bool "Cortex-A510: 2038923: workaround TRBE corruption with enable"
> +	depends on CORESIGHT_TRBE
> +	default y
> +	help
> +	  This option adds the workaround for ARM Cortex-A510 erratum 2038923.
> +
> +	  Affected Cortex-A510 core might cause an inconsistent view on whether trace is
> +	  prohibited within the CPU. As a result, the trace buffer or trace buffer state
> +	  might be corrupted. This happens after TRBE buffer has been enabled by setting
> +	  TRBLIMITR_EL1.E, followed by just a single context synchronization event before
> +	  execution changes from a context, in which trace is prohibited to one where it
> +	  isn't, or vice versa. In these mentioned conditions, the view of whether trace
> +	  is prohibited is inconsistent between parts of the CPU, and the trace buffer or
> +	  the trace buffer state might be corrupted.
> +
> +	  Work around this in the driver by preventing an inconsistent view of whether the
> +	  trace is prohibited or not based on TRBLIMITR_EL1.E by immediately following a
> +	  change to TRBLIMITR_EL1.E with at least one ISB instruction before an ERET, or
> +	  two ISB instructions if no ERET is to take place.
> +
> +	  If unsure, say Y.
> +
>   config CAVIUM_ERRATUM_22375
>   	bool "Cavium erratum 22375, 24313"
>   	default y
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index cbb7d5a9aee7..60b0c1f1d912 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -607,6 +607,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>   		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>   	},
>   #endif
> +#ifdef CONFIG_ARM64_ERRATUM_2038923
> +	{
> +		.desc = "ARM erratum 2038923",
> +		.capability = ARM64_WORKAROUND_2038923,
> +
> +		/* Cortex-A510 r0p0 - r0p2 */
> +		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
> +	},
> +#endif
>   	{
>   	}
>   };
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> index fca3cb329e1d..45a06d36d080 100644
> --- a/arch/arm64/tools/cpucaps
> +++ b/arch/arm64/tools/cpucaps
> @@ -56,6 +56,7 @@ WORKAROUND_1463225
>   WORKAROUND_1508412
>   WORKAROUND_1542419
>   WORKAROUND_2064142
> +WORKAROUND_2038923
>   WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>   WORKAROUND_TSB_FLUSH_FAILURE
>   WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index ec24b62b2cec..0689c6dab96d 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -92,11 +92,13 @@ struct trbe_buf {
>   #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE	0
>   #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE	1
>   #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE	2
> +#define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE	3
>   
>   static int trbe_errata_cpucaps[] = {
>   	[TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>   	[TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>   	[TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
> +	[TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
>   	-1,		/* Sentinel, must be the last entry */
>   };
>   
> @@ -174,6 +176,11 @@ static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
>   	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_SYSREG_WRITE_FAILURE);
>   }
>   
> +static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
> +{

minor nit: trbe_needs_{ctxt_sync, isb}_after_enable() ?

> +	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
> +}
> +
>   static int trbe_alloc_node(struct perf_event *event)
>   {
>   	if (event->cpu == -1)
> @@ -187,6 +194,30 @@ static inline void trbe_drain_buffer(void)
>   	dsb(nsh);
>   }
>   
> +static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
> +{
> +	u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);

minor nit: This implies we do the TRBE programming in the following
manner in the common case (i.e, TRBE enabled in the beginning of a
session).
   -> set TRBE LIMIT
   -> read TRBE LIMIT
   -> set TRBE ENABLED

Could we please optimize this ? I believe the buf->trbe_limit
must hold the LIMITR value at any point in time. And thus this
function could simply be :

set_trbe_enabled(trbe_buf)
{
	limitr = trbe_buf->limit | LIMITR_ENABLE
	write(limitr, TRBLIMITR_EL1);
	...
}

Otherwise looks good to me

Suzuki

Re: [PATCH 3/4] coresight: trbe: Work around the invalid prohibited states

[Suzuki K Poulose]

Hi Anshuman

On 05/01/2022 05:05, Anshuman Khandual wrote:
> TRBE implementations affected by Arm erratum #2038923 might get TRBE into
> an inconsistent view on whether trace is prohibited within the CPU. As a
> result, the trace buffer or trace buffer state might be corrupted. This
> happens after TRBE buffer has been enabled by setting TRBLIMITR_EL1.E,
> followed by just a single context synchronization event before execution
> changes from a context, in which trace is prohibited to one where it isn't,
> or vice versa. In these mentioned conditions, the view of whether trace is
> prohibited is inconsistent between parts of the CPU, and the trace buffer
> or the trace buffer state might be corrupted.
> 
> Work around this problem in the TRBE driver by preventing an inconsistent
> view of whether the trace is prohibited or not based on TRBLIMITR_EL1.E by
> immediately following a change to TRBLIMITR_EL1.E with at least one ISB
> instruction before an ERET, or two ISB instructions if no ERET is to take
> place. This adds a new cpu errata in arm64 errata framework and also
> updates TRBE driver as required.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
> Cc: coresight@lists.linaro.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>   Documentation/arm64/silicon-errata.rst       |  2 +
>   arch/arm64/Kconfig                           | 23 ++++++++++
>   arch/arm64/kernel/cpu_errata.c               |  9 ++++
>   arch/arm64/tools/cpucaps                     |  1 +
>   drivers/hwtracing/coresight/coresight-trbe.c | 47 +++++++++++++++-----
>   5 files changed, 72 insertions(+), 10 deletions(-)

As with the previous patch, it may be a good idea to split the
patch to arm64 and trbe parts.

> 
> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
> index c9b30e6c2b6c..e0ef3e9a4b8b 100644
> --- a/Documentation/arm64/silicon-errata.rst
> +++ b/Documentation/arm64/silicon-errata.rst
> @@ -54,6 +54,8 @@ stable kernels.
>   +----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
>   +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
> ++----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>   +----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 2105b68d88db..026e34fb6fad 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -796,6 +796,29 @@ config ARM64_ERRATUM_2064142
>   
>   	  If unsure, say Y.
>   
> +config ARM64_ERRATUM_2038923
> +	bool "Cortex-A510: 2038923: workaround TRBE corruption with enable"
> +	depends on CORESIGHT_TRBE
> +	default y
> +	help
> +	  This option adds the workaround for ARM Cortex-A510 erratum 2038923.
> +
> +	  Affected Cortex-A510 core might cause an inconsistent view on whether trace is
> +	  prohibited within the CPU. As a result, the trace buffer or trace buffer state
> +	  might be corrupted. This happens after TRBE buffer has been enabled by setting
> +	  TRBLIMITR_EL1.E, followed by just a single context synchronization event before
> +	  execution changes from a context, in which trace is prohibited to one where it
> +	  isn't, or vice versa. In these mentioned conditions, the view of whether trace
> +	  is prohibited is inconsistent between parts of the CPU, and the trace buffer or
> +	  the trace buffer state might be corrupted.
> +
> +	  Work around this in the driver by preventing an inconsistent view of whether the
> +	  trace is prohibited or not based on TRBLIMITR_EL1.E by immediately following a
> +	  change to TRBLIMITR_EL1.E with at least one ISB instruction before an ERET, or
> +	  two ISB instructions if no ERET is to take place.
> +
> +	  If unsure, say Y.
> +
>   config CAVIUM_ERRATUM_22375
>   	bool "Cavium erratum 22375, 24313"
>   	default y
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index cbb7d5a9aee7..60b0c1f1d912 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -607,6 +607,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>   		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>   	},
>   #endif
> +#ifdef CONFIG_ARM64_ERRATUM_2038923
> +	{
> +		.desc = "ARM erratum 2038923",
> +		.capability = ARM64_WORKAROUND_2038923,
> +
> +		/* Cortex-A510 r0p0 - r0p2 */
> +		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
> +	},
> +#endif
>   	{
>   	}
>   };
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> index fca3cb329e1d..45a06d36d080 100644
> --- a/arch/arm64/tools/cpucaps
> +++ b/arch/arm64/tools/cpucaps
> @@ -56,6 +56,7 @@ WORKAROUND_1463225
>   WORKAROUND_1508412
>   WORKAROUND_1542419
>   WORKAROUND_2064142
> +WORKAROUND_2038923
>   WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>   WORKAROUND_TSB_FLUSH_FAILURE
>   WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index ec24b62b2cec..0689c6dab96d 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -92,11 +92,13 @@ struct trbe_buf {
>   #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE	0
>   #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE	1
>   #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE	2
> +#define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE	3
>   
>   static int trbe_errata_cpucaps[] = {
>   	[TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>   	[TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>   	[TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
> +	[TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
>   	-1,		/* Sentinel, must be the last entry */
>   };
>   
> @@ -174,6 +176,11 @@ static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
>   	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_SYSREG_WRITE_FAILURE);
>   }
>   
> +static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
> +{

minor nit: trbe_needs_{ctxt_sync, isb}_after_enable() ?

> +	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
> +}
> +
>   static int trbe_alloc_node(struct perf_event *event)
>   {
>   	if (event->cpu == -1)
> @@ -187,6 +194,30 @@ static inline void trbe_drain_buffer(void)
>   	dsb(nsh);
>   }
>   
> +static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
> +{
> +	u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);

minor nit: This implies we do the TRBE programming in the following
manner in the common case (i.e, TRBE enabled in the beginning of a
session).
   -> set TRBE LIMIT
   -> read TRBE LIMIT
   -> set TRBE ENABLED

Could we please optimize this ? I believe the buf->trbe_limit
must hold the LIMITR value at any point in time. And thus this
function could simply be :

set_trbe_enabled(trbe_buf)
{
	limitr = trbe_buf->limit | LIMITR_ENABLE
	write(limitr, TRBLIMITR_EL1);
	...
}

Otherwise looks good to me

Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/4] coresight: trbe: Work around the invalid prohibited states

[Anshuman Khandual]

On 1/5/22 3:43 PM, Suzuki K Poulose wrote:
> Hi Anshuman
> 
> On 05/01/2022 05:05, Anshuman Khandual wrote:
>> TRBE implementations affected by Arm erratum #2038923 might get TRBE into
>> an inconsistent view on whether trace is prohibited within the CPU. As a
>> result, the trace buffer or trace buffer state might be corrupted. This
>> happens after TRBE buffer has been enabled by setting TRBLIMITR_EL1.E,
>> followed by just a single context synchronization event before execution
>> changes from a context, in which trace is prohibited to one where it isn't,
>> or vice versa. In these mentioned conditions, the view of whether trace is
>> prohibited is inconsistent between parts of the CPU, and the trace buffer
>> or the trace buffer state might be corrupted.
>>
>> Work around this problem in the TRBE driver by preventing an inconsistent
>> view of whether the trace is prohibited or not based on TRBLIMITR_EL1.E by
>> immediately following a change to TRBLIMITR_EL1.E with at least one ISB
>> instruction before an ERET, or two ISB instructions if no ERET is to take
>> place. This adds a new cpu errata in arm64 errata framework and also
>> updates TRBE driver as required.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
>> Cc: coresight@lists.linaro.org
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>   Documentation/arm64/silicon-errata.rst       |  2 +
>>   arch/arm64/Kconfig                           | 23 ++++++++++
>>   arch/arm64/kernel/cpu_errata.c               |  9 ++++
>>   arch/arm64/tools/cpucaps                     |  1 +
>>   drivers/hwtracing/coresight/coresight-trbe.c | 47 +++++++++++++++-----
>>   5 files changed, 72 insertions(+), 10 deletions(-)
> 
> As with the previous patch, it may be a good idea to split the
> patch to arm64 and trbe parts.

Sure, will do.

> 
>>
>> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
>> index c9b30e6c2b6c..e0ef3e9a4b8b 100644
>> --- a/Documentation/arm64/silicon-errata.rst
>> +++ b/Documentation/arm64/silicon-errata.rst
>> @@ -54,6 +54,8 @@ stable kernels.
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 2105b68d88db..026e34fb6fad 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -796,6 +796,29 @@ config ARM64_ERRATUM_2064142
>>           If unsure, say Y.
>>   +config ARM64_ERRATUM_2038923
>> +    bool "Cortex-A510: 2038923: workaround TRBE corruption with enable"
>> +    depends on CORESIGHT_TRBE
>> +    default y
>> +    help
>> +      This option adds the workaround for ARM Cortex-A510 erratum 2038923.
>> +
>> +      Affected Cortex-A510 core might cause an inconsistent view on whether trace is
>> +      prohibited within the CPU. As a result, the trace buffer or trace buffer state
>> +      might be corrupted. This happens after TRBE buffer has been enabled by setting
>> +      TRBLIMITR_EL1.E, followed by just a single context synchronization event before
>> +      execution changes from a context, in which trace is prohibited to one where it
>> +      isn't, or vice versa. In these mentioned conditions, the view of whether trace
>> +      is prohibited is inconsistent between parts of the CPU, and the trace buffer or
>> +      the trace buffer state might be corrupted.
>> +
>> +      Work around this in the driver by preventing an inconsistent view of whether the
>> +      trace is prohibited or not based on TRBLIMITR_EL1.E by immediately following a
>> +      change to TRBLIMITR_EL1.E with at least one ISB instruction before an ERET, or
>> +      two ISB instructions if no ERET is to take place.
>> +
>> +      If unsure, say Y.
>> +
>>   config CAVIUM_ERRATUM_22375
>>       bool "Cavium erratum 22375, 24313"
>>       default y
>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>> index cbb7d5a9aee7..60b0c1f1d912 100644
>> --- a/arch/arm64/kernel/cpu_errata.c
>> +++ b/arch/arm64/kernel/cpu_errata.c
>> @@ -607,6 +607,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>>           ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>>       },
>>   #endif
>> +#ifdef CONFIG_ARM64_ERRATUM_2038923
>> +    {
>> +        .desc = "ARM erratum 2038923",
>> +        .capability = ARM64_WORKAROUND_2038923,
>> +
>> +        /* Cortex-A510 r0p0 - r0p2 */
>> +        ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>> +    },
>> +#endif
>>       {
>>       }
>>   };
>> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>> index fca3cb329e1d..45a06d36d080 100644
>> --- a/arch/arm64/tools/cpucaps
>> +++ b/arch/arm64/tools/cpucaps
>> @@ -56,6 +56,7 @@ WORKAROUND_1463225
>>   WORKAROUND_1508412
>>   WORKAROUND_1542419
>>   WORKAROUND_2064142
>> +WORKAROUND_2038923
>>   WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>>   WORKAROUND_TSB_FLUSH_FAILURE
>>   WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>> index ec24b62b2cec..0689c6dab96d 100644
>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>> @@ -92,11 +92,13 @@ struct trbe_buf {
>>   #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE    0
>>   #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE    1
>>   #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE    2
>> +#define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE    3
>>     static int trbe_errata_cpucaps[] = {
>>       [TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>>       [TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>>       [TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
>> +    [TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
>>       -1,        /* Sentinel, must be the last entry */
>>   };
>>   @@ -174,6 +176,11 @@ static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
>>       return trbe_has_erratum(cpudata, TRBE_WORKAROUND_SYSREG_WRITE_FAILURE);
>>   }
>>   +static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
>> +{
> 
> minor nit: trbe_needs_{ctxt_sync, isb}_after_enable() ?

trbe_needs_ctxt_sync_after_enable() sounds better. Also will have to change
the index above as well .. TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE.

> 
>> +    return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
>> +}
>> +
>>   static int trbe_alloc_node(struct perf_event *event)
>>   {
>>       if (event->cpu == -1)
>> @@ -187,6 +194,30 @@ static inline void trbe_drain_buffer(void)
>>       dsb(nsh);
>>   }
>>   +static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
>> +{
>> +    u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
> 
> minor nit: This implies we do the TRBE programming in the following
> manner in the common case (i.e, TRBE enabled in the beginning of a
> session).
>   -> set TRBE LIMIT
>   -> read TRBE LIMIT
>   -> set TRBE ENABLED
> 
> Could we please optimize this ? I believe the buf->trbe_limit
> must hold the LIMITR value at any point in time. And thus this

But is not bit risky though ! We have got the following places where
given trbe_limit instance changes its value.

drivers/../coresight-trbe.c:   buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE;
drivers/../coresight-trbe.c:   buf->trbe_limit = compute_trbe_buffer_limit(handle);
drivers/../coresight-trbe.c:   buf->trbe_limit -= PAGE_SIZE;

> function could simply be :
> 
> set_trbe_enabled(trbe_buf)
> {
>     limitr = trbe_buf->limit | LIMITR_ENABLE
>     write(limitr, TRBLIMITR_EL1);
>     ...
> }

Is the potential for performance improvement here, out weigh possible
risks of using buf->trbe_limit directly while enabling the TRBE ?

> 
> Otherwise looks good to me
> 
> Suzuki

Re: [PATCH 3/4] coresight: trbe: Work around the invalid prohibited states

[Anshuman Khandual]

On 1/5/22 3:43 PM, Suzuki K Poulose wrote:
> Hi Anshuman
> 
> On 05/01/2022 05:05, Anshuman Khandual wrote:
>> TRBE implementations affected by Arm erratum #2038923 might get TRBE into
>> an inconsistent view on whether trace is prohibited within the CPU. As a
>> result, the trace buffer or trace buffer state might be corrupted. This
>> happens after TRBE buffer has been enabled by setting TRBLIMITR_EL1.E,
>> followed by just a single context synchronization event before execution
>> changes from a context, in which trace is prohibited to one where it isn't,
>> or vice versa. In these mentioned conditions, the view of whether trace is
>> prohibited is inconsistent between parts of the CPU, and the trace buffer
>> or the trace buffer state might be corrupted.
>>
>> Work around this problem in the TRBE driver by preventing an inconsistent
>> view of whether the trace is prohibited or not based on TRBLIMITR_EL1.E by
>> immediately following a change to TRBLIMITR_EL1.E with at least one ISB
>> instruction before an ERET, or two ISB instructions if no ERET is to take
>> place. This adds a new cpu errata in arm64 errata framework and also
>> updates TRBE driver as required.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
>> Cc: coresight@lists.linaro.org
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>   Documentation/arm64/silicon-errata.rst       |  2 +
>>   arch/arm64/Kconfig                           | 23 ++++++++++
>>   arch/arm64/kernel/cpu_errata.c               |  9 ++++
>>   arch/arm64/tools/cpucaps                     |  1 +
>>   drivers/hwtracing/coresight/coresight-trbe.c | 47 +++++++++++++++-----
>>   5 files changed, 72 insertions(+), 10 deletions(-)
> 
> As with the previous patch, it may be a good idea to split the
> patch to arm64 and trbe parts.

Sure, will do.

> 
>>
>> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
>> index c9b30e6c2b6c..e0ef3e9a4b8b 100644
>> --- a/Documentation/arm64/silicon-errata.rst
>> +++ b/Documentation/arm64/silicon-errata.rst
>> @@ -54,6 +54,8 @@ stable kernels.
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 2105b68d88db..026e34fb6fad 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -796,6 +796,29 @@ config ARM64_ERRATUM_2064142
>>           If unsure, say Y.
>>   +config ARM64_ERRATUM_2038923
>> +    bool "Cortex-A510: 2038923: workaround TRBE corruption with enable"
>> +    depends on CORESIGHT_TRBE
>> +    default y
>> +    help
>> +      This option adds the workaround for ARM Cortex-A510 erratum 2038923.
>> +
>> +      Affected Cortex-A510 core might cause an inconsistent view on whether trace is
>> +      prohibited within the CPU. As a result, the trace buffer or trace buffer state
>> +      might be corrupted. This happens after TRBE buffer has been enabled by setting
>> +      TRBLIMITR_EL1.E, followed by just a single context synchronization event before
>> +      execution changes from a context, in which trace is prohibited to one where it
>> +      isn't, or vice versa. In these mentioned conditions, the view of whether trace
>> +      is prohibited is inconsistent between parts of the CPU, and the trace buffer or
>> +      the trace buffer state might be corrupted.
>> +
>> +      Work around this in the driver by preventing an inconsistent view of whether the
>> +      trace is prohibited or not based on TRBLIMITR_EL1.E by immediately following a
>> +      change to TRBLIMITR_EL1.E with at least one ISB instruction before an ERET, or
>> +      two ISB instructions if no ERET is to take place.
>> +
>> +      If unsure, say Y.
>> +
>>   config CAVIUM_ERRATUM_22375
>>       bool "Cavium erratum 22375, 24313"
>>       default y
>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>> index cbb7d5a9aee7..60b0c1f1d912 100644
>> --- a/arch/arm64/kernel/cpu_errata.c
>> +++ b/arch/arm64/kernel/cpu_errata.c
>> @@ -607,6 +607,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>>           ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>>       },
>>   #endif
>> +#ifdef CONFIG_ARM64_ERRATUM_2038923
>> +    {
>> +        .desc = "ARM erratum 2038923",
>> +        .capability = ARM64_WORKAROUND_2038923,
>> +
>> +        /* Cortex-A510 r0p0 - r0p2 */
>> +        ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>> +    },
>> +#endif
>>       {
>>       }
>>   };
>> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>> index fca3cb329e1d..45a06d36d080 100644
>> --- a/arch/arm64/tools/cpucaps
>> +++ b/arch/arm64/tools/cpucaps
>> @@ -56,6 +56,7 @@ WORKAROUND_1463225
>>   WORKAROUND_1508412
>>   WORKAROUND_1542419
>>   WORKAROUND_2064142
>> +WORKAROUND_2038923
>>   WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>>   WORKAROUND_TSB_FLUSH_FAILURE
>>   WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>> index ec24b62b2cec..0689c6dab96d 100644
>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>> @@ -92,11 +92,13 @@ struct trbe_buf {
>>   #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE    0
>>   #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE    1
>>   #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE    2
>> +#define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE    3
>>     static int trbe_errata_cpucaps[] = {
>>       [TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>>       [TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>>       [TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
>> +    [TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
>>       -1,        /* Sentinel, must be the last entry */
>>   };
>>   @@ -174,6 +176,11 @@ static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
>>       return trbe_has_erratum(cpudata, TRBE_WORKAROUND_SYSREG_WRITE_FAILURE);
>>   }
>>   +static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
>> +{
> 
> minor nit: trbe_needs_{ctxt_sync, isb}_after_enable() ?

trbe_needs_ctxt_sync_after_enable() sounds better. Also will have to change
the index above as well .. TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE.

> 
>> +    return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
>> +}
>> +
>>   static int trbe_alloc_node(struct perf_event *event)
>>   {
>>       if (event->cpu == -1)
>> @@ -187,6 +194,30 @@ static inline void trbe_drain_buffer(void)
>>       dsb(nsh);
>>   }
>>   +static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
>> +{
>> +    u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
> 
> minor nit: This implies we do the TRBE programming in the following
> manner in the common case (i.e, TRBE enabled in the beginning of a
> session).
>   -> set TRBE LIMIT
>   -> read TRBE LIMIT
>   -> set TRBE ENABLED
> 
> Could we please optimize this ? I believe the buf->trbe_limit
> must hold the LIMITR value at any point in time. And thus this

But is not bit risky though ! We have got the following places where
given trbe_limit instance changes its value.

drivers/../coresight-trbe.c:   buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE;
drivers/../coresight-trbe.c:   buf->trbe_limit = compute_trbe_buffer_limit(handle);
drivers/../coresight-trbe.c:   buf->trbe_limit -= PAGE_SIZE;

> function could simply be :
> 
> set_trbe_enabled(trbe_buf)
> {
>     limitr = trbe_buf->limit | LIMITR_ENABLE
>     write(limitr, TRBLIMITR_EL1);
>     ...
> }

Is the potential for performance improvement here, out weigh possible
risks of using buf->trbe_limit directly while enabling the TRBE ?

> 
> Otherwise looks good to me
> 
> Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/4] coresight: trbe: Work around the invalid prohibited states

[Suzuki K Poulose]

On 05/01/2022 11:16, Anshuman Khandual wrote:
> 
> 
> On 1/5/22 3:43 PM, Suzuki K Poulose wrote:
>> Hi Anshuman
>>
>> On 05/01/2022 05:05, Anshuman Khandual wrote:
>>> TRBE implementations affected by Arm erratum #2038923 might get TRBE into
>>> an inconsistent view on whether trace is prohibited within the CPU. As a
>>> result, the trace buffer or trace buffer state might be corrupted. This
>>> happens after TRBE buffer has been enabled by setting TRBLIMITR_EL1.E,
>>> followed by just a single context synchronization event before execution
>>> changes from a context, in which trace is prohibited to one where it isn't,
>>> or vice versa. In these mentioned conditions, the view of whether trace is
>>> prohibited is inconsistent between parts of the CPU, and the trace buffer
>>> or the trace buffer state might be corrupted.
>>>
>>> Work around this problem in the TRBE driver by preventing an inconsistent
>>> view of whether the trace is prohibited or not based on TRBLIMITR_EL1.E by
>>> immediately following a change to TRBLIMITR_EL1.E with at least one ISB
>>> instruction before an ERET, or two ISB instructions if no ERET is to take
>>> place. This adds a new cpu errata in arm64 errata framework and also
>>> updates TRBE driver as required.
>>>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
>>> Cc: coresight@lists.linaro.org
>>> Cc: linux-doc@vger.kernel.org
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>>    Documentation/arm64/silicon-errata.rst       |  2 +
>>>    arch/arm64/Kconfig                           | 23 ++++++++++
>>>    arch/arm64/kernel/cpu_errata.c               |  9 ++++
>>>    arch/arm64/tools/cpucaps                     |  1 +
>>>    drivers/hwtracing/coresight/coresight-trbe.c | 47 +++++++++++++++-----
>>>    5 files changed, 72 insertions(+), 10 deletions(-)
>>
>> As with the previous patch, it may be a good idea to split the
>> patch to arm64 and trbe parts.
> 
> Sure, will do.
> 
>>
>>>
>>> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
>>> index c9b30e6c2b6c..e0ef3e9a4b8b 100644
>>> --- a/Documentation/arm64/silicon-errata.rst
>>> +++ b/Documentation/arm64/silicon-errata.rst
>>> @@ -54,6 +54,8 @@ stable kernels.
>>>    +----------------+-----------------+-----------------+-----------------------------+
>>>    | ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
>>>    +----------------+-----------------+-----------------+-----------------------------+
>>> +| ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>    | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>>>    +----------------+-----------------+-----------------+-----------------------------+
>>>    | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index 2105b68d88db..026e34fb6fad 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -796,6 +796,29 @@ config ARM64_ERRATUM_2064142
>>>            If unsure, say Y.
>>>    +config ARM64_ERRATUM_2038923
>>> +    bool "Cortex-A510: 2038923: workaround TRBE corruption with enable"
>>> +    depends on CORESIGHT_TRBE
>>> +    default y
>>> +    help
>>> +      This option adds the workaround for ARM Cortex-A510 erratum 2038923.
>>> +
>>> +      Affected Cortex-A510 core might cause an inconsistent view on whether trace is
>>> +      prohibited within the CPU. As a result, the trace buffer or trace buffer state
>>> +      might be corrupted. This happens after TRBE buffer has been enabled by setting
>>> +      TRBLIMITR_EL1.E, followed by just a single context synchronization event before
>>> +      execution changes from a context, in which trace is prohibited to one where it
>>> +      isn't, or vice versa. In these mentioned conditions, the view of whether trace
>>> +      is prohibited is inconsistent between parts of the CPU, and the trace buffer or
>>> +      the trace buffer state might be corrupted.
>>> +
>>> +      Work around this in the driver by preventing an inconsistent view of whether the
>>> +      trace is prohibited or not based on TRBLIMITR_EL1.E by immediately following a
>>> +      change to TRBLIMITR_EL1.E with at least one ISB instruction before an ERET, or
>>> +      two ISB instructions if no ERET is to take place.
>>> +
>>> +      If unsure, say Y.
>>> +
>>>    config CAVIUM_ERRATUM_22375
>>>        bool "Cavium erratum 22375, 24313"
>>>        default y
>>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>>> index cbb7d5a9aee7..60b0c1f1d912 100644
>>> --- a/arch/arm64/kernel/cpu_errata.c
>>> +++ b/arch/arm64/kernel/cpu_errata.c
>>> @@ -607,6 +607,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>>>            ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>>>        },
>>>    #endif
>>> +#ifdef CONFIG_ARM64_ERRATUM_2038923
>>> +    {
>>> +        .desc = "ARM erratum 2038923",
>>> +        .capability = ARM64_WORKAROUND_2038923,
>>> +
>>> +        /* Cortex-A510 r0p0 - r0p2 */
>>> +        ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>>> +    },
>>> +#endif
>>>        {
>>>        }
>>>    };
>>> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>>> index fca3cb329e1d..45a06d36d080 100644
>>> --- a/arch/arm64/tools/cpucaps
>>> +++ b/arch/arm64/tools/cpucaps
>>> @@ -56,6 +56,7 @@ WORKAROUND_1463225
>>>    WORKAROUND_1508412
>>>    WORKAROUND_1542419
>>>    WORKAROUND_2064142
>>> +WORKAROUND_2038923
>>>    WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>>>    WORKAROUND_TSB_FLUSH_FAILURE
>>>    WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
>>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>>> index ec24b62b2cec..0689c6dab96d 100644
>>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>>> @@ -92,11 +92,13 @@ struct trbe_buf {
>>>    #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE    0
>>>    #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE    1
>>>    #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE    2
>>> +#define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE    3
>>>      static int trbe_errata_cpucaps[] = {
>>>        [TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>>>        [TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>>>        [TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
>>> +    [TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
>>>        -1,        /* Sentinel, must be the last entry */
>>>    };
>>>    @@ -174,6 +176,11 @@ static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
>>>        return trbe_has_erratum(cpudata, TRBE_WORKAROUND_SYSREG_WRITE_FAILURE);
>>>    }
>>>    +static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
>>> +{
>>
>> minor nit: trbe_needs_{ctxt_sync, isb}_after_enable() ?
> 
> trbe_needs_ctxt_sync_after_enable() sounds better. Also will have to change
> the index above as well .. TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE.
> 
>>
>>> +    return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
>>> +}
>>> +
>>>    static int trbe_alloc_node(struct perf_event *event)
>>>    {
>>>        if (event->cpu == -1)
>>> @@ -187,6 +194,30 @@ static inline void trbe_drain_buffer(void)
>>>        dsb(nsh);
>>>    }
>>>    +static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
>>> +{
>>> +    u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
>>
>> minor nit: This implies we do the TRBE programming in the following
>> manner in the common case (i.e, TRBE enabled in the beginning of a
>> session).
>>    -> set TRBE LIMIT
>>    -> read TRBE LIMIT
>>    -> set TRBE ENABLED
>>
>> Could we please optimize this ? I believe the buf->trbe_limit
>> must hold the LIMITR value at any point in time. And thus this
> 
> But is not bit risky though ! We have got the following places where
> given trbe_limit instance changes its value.
> 
> drivers/../coresight-trbe.c:   buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE;
> drivers/../coresight-trbe.c:   buf->trbe_limit = compute_trbe_buffer_limit(handle);
> drivers/../coresight-trbe.c:   buf->trbe_limit -= PAGE_SIZE;

Those are the places where we compute the trbe_limit, *before*
we enable the TRBE. And we don't change recompute the limit
*without disabling* the TRBE. To make it more clear, the
only place where we set TRBE enabled without "computing"
the trbe_limit is when we hit a spurious interrupt.
But the value in the TRBLIMITR should already match the
buf->trbe_limit and we are only going to re-enable the
TRBE with the same limit. The other option is to
pass down the "limit" to the set_trbe_enabled().

> 
>> function could simply be :
>>
>> set_trbe_enabled(trbe_buf)
>> {
>>      limitr = trbe_buf->limit | LIMITR_ENABLE
>>      write(limitr, TRBLIMITR_EL1);
>>      ...
>> }
> 
> Is the potential for performance improvement here, out weigh possible
> risks of using buf->trbe_limit directly while enabling the TRBE ?

I somehow don't like the fact that we have additional write and read
for the most common case of the TRBE usage (i.e, for arm_trbe_enable()).
If we could avoid that, that may be better.

Cheers
Suzuki

Re: [PATCH 3/4] coresight: trbe: Work around the invalid prohibited states

[Suzuki K Poulose]

On 05/01/2022 11:16, Anshuman Khandual wrote:
> 
> 
> On 1/5/22 3:43 PM, Suzuki K Poulose wrote:
>> Hi Anshuman
>>
>> On 05/01/2022 05:05, Anshuman Khandual wrote:
>>> TRBE implementations affected by Arm erratum #2038923 might get TRBE into
>>> an inconsistent view on whether trace is prohibited within the CPU. As a
>>> result, the trace buffer or trace buffer state might be corrupted. This
>>> happens after TRBE buffer has been enabled by setting TRBLIMITR_EL1.E,
>>> followed by just a single context synchronization event before execution
>>> changes from a context, in which trace is prohibited to one where it isn't,
>>> or vice versa. In these mentioned conditions, the view of whether trace is
>>> prohibited is inconsistent between parts of the CPU, and the trace buffer
>>> or the trace buffer state might be corrupted.
>>>
>>> Work around this problem in the TRBE driver by preventing an inconsistent
>>> view of whether the trace is prohibited or not based on TRBLIMITR_EL1.E by
>>> immediately following a change to TRBLIMITR_EL1.E with at least one ISB
>>> instruction before an ERET, or two ISB instructions if no ERET is to take
>>> place. This adds a new cpu errata in arm64 errata framework and also
>>> updates TRBE driver as required.
>>>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
>>> Cc: coresight@lists.linaro.org
>>> Cc: linux-doc@vger.kernel.org
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>>    Documentation/arm64/silicon-errata.rst       |  2 +
>>>    arch/arm64/Kconfig                           | 23 ++++++++++
>>>    arch/arm64/kernel/cpu_errata.c               |  9 ++++
>>>    arch/arm64/tools/cpucaps                     |  1 +
>>>    drivers/hwtracing/coresight/coresight-trbe.c | 47 +++++++++++++++-----
>>>    5 files changed, 72 insertions(+), 10 deletions(-)
>>
>> As with the previous patch, it may be a good idea to split the
>> patch to arm64 and trbe parts.
> 
> Sure, will do.
> 
>>
>>>
>>> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
>>> index c9b30e6c2b6c..e0ef3e9a4b8b 100644
>>> --- a/Documentation/arm64/silicon-errata.rst
>>> +++ b/Documentation/arm64/silicon-errata.rst
>>> @@ -54,6 +54,8 @@ stable kernels.
>>>    +----------------+-----------------+-----------------+-----------------------------+
>>>    | ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
>>>    +----------------+-----------------+-----------------+-----------------------------+
>>> +| ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>    | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>>>    +----------------+-----------------+-----------------+-----------------------------+
>>>    | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index 2105b68d88db..026e34fb6fad 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -796,6 +796,29 @@ config ARM64_ERRATUM_2064142
>>>            If unsure, say Y.
>>>    +config ARM64_ERRATUM_2038923
>>> +    bool "Cortex-A510: 2038923: workaround TRBE corruption with enable"
>>> +    depends on CORESIGHT_TRBE
>>> +    default y
>>> +    help
>>> +      This option adds the workaround for ARM Cortex-A510 erratum 2038923.
>>> +
>>> +      Affected Cortex-A510 core might cause an inconsistent view on whether trace is
>>> +      prohibited within the CPU. As a result, the trace buffer or trace buffer state
>>> +      might be corrupted. This happens after TRBE buffer has been enabled by setting
>>> +      TRBLIMITR_EL1.E, followed by just a single context synchronization event before
>>> +      execution changes from a context, in which trace is prohibited to one where it
>>> +      isn't, or vice versa. In these mentioned conditions, the view of whether trace
>>> +      is prohibited is inconsistent between parts of the CPU, and the trace buffer or
>>> +      the trace buffer state might be corrupted.
>>> +
>>> +      Work around this in the driver by preventing an inconsistent view of whether the
>>> +      trace is prohibited or not based on TRBLIMITR_EL1.E by immediately following a
>>> +      change to TRBLIMITR_EL1.E with at least one ISB instruction before an ERET, or
>>> +      two ISB instructions if no ERET is to take place.
>>> +
>>> +      If unsure, say Y.
>>> +
>>>    config CAVIUM_ERRATUM_22375
>>>        bool "Cavium erratum 22375, 24313"
>>>        default y
>>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>>> index cbb7d5a9aee7..60b0c1f1d912 100644
>>> --- a/arch/arm64/kernel/cpu_errata.c
>>> +++ b/arch/arm64/kernel/cpu_errata.c
>>> @@ -607,6 +607,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>>>            ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>>>        },
>>>    #endif
>>> +#ifdef CONFIG_ARM64_ERRATUM_2038923
>>> +    {
>>> +        .desc = "ARM erratum 2038923",
>>> +        .capability = ARM64_WORKAROUND_2038923,
>>> +
>>> +        /* Cortex-A510 r0p0 - r0p2 */
>>> +        ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>>> +    },
>>> +#endif
>>>        {
>>>        }
>>>    };
>>> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>>> index fca3cb329e1d..45a06d36d080 100644
>>> --- a/arch/arm64/tools/cpucaps
>>> +++ b/arch/arm64/tools/cpucaps
>>> @@ -56,6 +56,7 @@ WORKAROUND_1463225
>>>    WORKAROUND_1508412
>>>    WORKAROUND_1542419
>>>    WORKAROUND_2064142
>>> +WORKAROUND_2038923
>>>    WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>>>    WORKAROUND_TSB_FLUSH_FAILURE
>>>    WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
>>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>>> index ec24b62b2cec..0689c6dab96d 100644
>>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>>> @@ -92,11 +92,13 @@ struct trbe_buf {
>>>    #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE    0
>>>    #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE    1
>>>    #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE    2
>>> +#define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE    3
>>>      static int trbe_errata_cpucaps[] = {
>>>        [TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>>>        [TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>>>        [TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
>>> +    [TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
>>>        -1,        /* Sentinel, must be the last entry */
>>>    };
>>>    @@ -174,6 +176,11 @@ static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
>>>        return trbe_has_erratum(cpudata, TRBE_WORKAROUND_SYSREG_WRITE_FAILURE);
>>>    }
>>>    +static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
>>> +{
>>
>> minor nit: trbe_needs_{ctxt_sync, isb}_after_enable() ?
> 
> trbe_needs_ctxt_sync_after_enable() sounds better. Also will have to change
> the index above as well .. TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE.
> 
>>
>>> +    return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
>>> +}
>>> +
>>>    static int trbe_alloc_node(struct perf_event *event)
>>>    {
>>>        if (event->cpu == -1)
>>> @@ -187,6 +194,30 @@ static inline void trbe_drain_buffer(void)
>>>        dsb(nsh);
>>>    }
>>>    +static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
>>> +{
>>> +    u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
>>
>> minor nit: This implies we do the TRBE programming in the following
>> manner in the common case (i.e, TRBE enabled in the beginning of a
>> session).
>>    -> set TRBE LIMIT
>>    -> read TRBE LIMIT
>>    -> set TRBE ENABLED
>>
>> Could we please optimize this ? I believe the buf->trbe_limit
>> must hold the LIMITR value at any point in time. And thus this
> 
> But is not bit risky though ! We have got the following places where
> given trbe_limit instance changes its value.
> 
> drivers/../coresight-trbe.c:   buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE;
> drivers/../coresight-trbe.c:   buf->trbe_limit = compute_trbe_buffer_limit(handle);
> drivers/../coresight-trbe.c:   buf->trbe_limit -= PAGE_SIZE;

Those are the places where we compute the trbe_limit, *before*
we enable the TRBE. And we don't change recompute the limit
*without disabling* the TRBE. To make it more clear, the
only place where we set TRBE enabled without "computing"
the trbe_limit is when we hit a spurious interrupt.
But the value in the TRBLIMITR should already match the
buf->trbe_limit and we are only going to re-enable the
TRBE with the same limit. The other option is to
pass down the "limit" to the set_trbe_enabled().

> 
>> function could simply be :
>>
>> set_trbe_enabled(trbe_buf)
>> {
>>      limitr = trbe_buf->limit | LIMITR_ENABLE
>>      write(limitr, TRBLIMITR_EL1);
>>      ...
>> }
> 
> Is the potential for performance improvement here, out weigh possible
> risks of using buf->trbe_limit directly while enabling the TRBE ?

I somehow don't like the fact that we have additional write and read
for the most common case of the TRBE usage (i.e, for arm_trbe_enable()).
If we could avoid that, that may be better.

Cheers
Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/4] coresight: trbe: Work around the invalid prohibited states

[Anshuman Khandual]

On 1/5/22 7:24 PM, Suzuki K Poulose wrote:
> On 05/01/2022 11:16, Anshuman Khandual wrote:
>>
>>
>> On 1/5/22 3:43 PM, Suzuki K Poulose wrote:
>>> Hi Anshuman
>>>
>>> On 05/01/2022 05:05, Anshuman Khandual wrote:
>>>> TRBE implementations affected by Arm erratum #2038923 might get TRBE into
>>>> an inconsistent view on whether trace is prohibited within the CPU. As a
>>>> result, the trace buffer or trace buffer state might be corrupted. This
>>>> happens after TRBE buffer has been enabled by setting TRBLIMITR_EL1.E,
>>>> followed by just a single context synchronization event before execution
>>>> changes from a context, in which trace is prohibited to one where it isn't,
>>>> or vice versa. In these mentioned conditions, the view of whether trace is
>>>> prohibited is inconsistent between parts of the CPU, and the trace buffer
>>>> or the trace buffer state might be corrupted.
>>>>
>>>> Work around this problem in the TRBE driver by preventing an inconsistent
>>>> view of whether the trace is prohibited or not based on TRBLIMITR_EL1.E by
>>>> immediately following a change to TRBLIMITR_EL1.E with at least one ISB
>>>> instruction before an ERET, or two ISB instructions if no ERET is to take
>>>> place. This adds a new cpu errata in arm64 errata framework and also
>>>> updates TRBE driver as required.
>>>>
>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>> Cc: Will Deacon <will@kernel.org>
>>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>>> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
>>>> Cc: coresight@lists.linaro.org
>>>> Cc: linux-doc@vger.kernel.org
>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>> ---
>>>>    Documentation/arm64/silicon-errata.rst       |  2 +
>>>>    arch/arm64/Kconfig                           | 23 ++++++++++
>>>>    arch/arm64/kernel/cpu_errata.c               |  9 ++++
>>>>    arch/arm64/tools/cpucaps                     |  1 +
>>>>    drivers/hwtracing/coresight/coresight-trbe.c | 47 +++++++++++++++-----
>>>>    5 files changed, 72 insertions(+), 10 deletions(-)
>>>
>>> As with the previous patch, it may be a good idea to split the
>>> patch to arm64 and trbe parts.
>>
>> Sure, will do.
>>
>>>
>>>>
>>>> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
>>>> index c9b30e6c2b6c..e0ef3e9a4b8b 100644
>>>> --- a/Documentation/arm64/silicon-errata.rst
>>>> +++ b/Documentation/arm64/silicon-errata.rst
>>>> @@ -54,6 +54,8 @@ stable kernels.
>>>>    +----------------+-----------------+-----------------+-----------------------------+
>>>>    | ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
>>>>    +----------------+-----------------+-----------------+-----------------------------+
>>>> +| ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
>>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>>    | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>>>>    +----------------+-----------------+-----------------+-----------------------------+
>>>>    | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>> index 2105b68d88db..026e34fb6fad 100644
>>>> --- a/arch/arm64/Kconfig
>>>> +++ b/arch/arm64/Kconfig
>>>> @@ -796,6 +796,29 @@ config ARM64_ERRATUM_2064142
>>>>            If unsure, say Y.
>>>>    +config ARM64_ERRATUM_2038923
>>>> +    bool "Cortex-A510: 2038923: workaround TRBE corruption with enable"
>>>> +    depends on CORESIGHT_TRBE
>>>> +    default y
>>>> +    help
>>>> +      This option adds the workaround for ARM Cortex-A510 erratum 2038923.
>>>> +
>>>> +      Affected Cortex-A510 core might cause an inconsistent view on whether trace is
>>>> +      prohibited within the CPU. As a result, the trace buffer or trace buffer state
>>>> +      might be corrupted. This happens after TRBE buffer has been enabled by setting
>>>> +      TRBLIMITR_EL1.E, followed by just a single context synchronization event before
>>>> +      execution changes from a context, in which trace is prohibited to one where it
>>>> +      isn't, or vice versa. In these mentioned conditions, the view of whether trace
>>>> +      is prohibited is inconsistent between parts of the CPU, and the trace buffer or
>>>> +      the trace buffer state might be corrupted.
>>>> +
>>>> +      Work around this in the driver by preventing an inconsistent view of whether the
>>>> +      trace is prohibited or not based on TRBLIMITR_EL1.E by immediately following a
>>>> +      change to TRBLIMITR_EL1.E with at least one ISB instruction before an ERET, or
>>>> +      two ISB instructions if no ERET is to take place.
>>>> +
>>>> +      If unsure, say Y.
>>>> +
>>>>    config CAVIUM_ERRATUM_22375
>>>>        bool "Cavium erratum 22375, 24313"
>>>>        default y
>>>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>>>> index cbb7d5a9aee7..60b0c1f1d912 100644
>>>> --- a/arch/arm64/kernel/cpu_errata.c
>>>> +++ b/arch/arm64/kernel/cpu_errata.c
>>>> @@ -607,6 +607,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>>>>            ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>>>>        },
>>>>    #endif
>>>> +#ifdef CONFIG_ARM64_ERRATUM_2038923
>>>> +    {
>>>> +        .desc = "ARM erratum 2038923",
>>>> +        .capability = ARM64_WORKAROUND_2038923,
>>>> +
>>>> +        /* Cortex-A510 r0p0 - r0p2 */
>>>> +        ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>>>> +    },
>>>> +#endif
>>>>        {
>>>>        }
>>>>    };
>>>> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>>>> index fca3cb329e1d..45a06d36d080 100644
>>>> --- a/arch/arm64/tools/cpucaps
>>>> +++ b/arch/arm64/tools/cpucaps
>>>> @@ -56,6 +56,7 @@ WORKAROUND_1463225
>>>>    WORKAROUND_1508412
>>>>    WORKAROUND_1542419
>>>>    WORKAROUND_2064142
>>>> +WORKAROUND_2038923
>>>>    WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>>>>    WORKAROUND_TSB_FLUSH_FAILURE
>>>>    WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
>>>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>>>> index ec24b62b2cec..0689c6dab96d 100644
>>>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>>>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>>>> @@ -92,11 +92,13 @@ struct trbe_buf {
>>>>    #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE    0
>>>>    #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE    1
>>>>    #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE    2
>>>> +#define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE    3
>>>>      static int trbe_errata_cpucaps[] = {
>>>>        [TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>>>>        [TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>>>>        [TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
>>>> +    [TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
>>>>        -1,        /* Sentinel, must be the last entry */
>>>>    };
>>>>    @@ -174,6 +176,11 @@ static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
>>>>        return trbe_has_erratum(cpudata, TRBE_WORKAROUND_SYSREG_WRITE_FAILURE);
>>>>    }
>>>>    +static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
>>>> +{
>>>
>>> minor nit: trbe_needs_{ctxt_sync, isb}_after_enable() ?
>>
>> trbe_needs_ctxt_sync_after_enable() sounds better. Also will have to change
>> the index above as well .. TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE.
>>
>>>
>>>> +    return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
>>>> +}
>>>> +
>>>>    static int trbe_alloc_node(struct perf_event *event)
>>>>    {
>>>>        if (event->cpu == -1)
>>>> @@ -187,6 +194,30 @@ static inline void trbe_drain_buffer(void)
>>>>        dsb(nsh);
>>>>    }
>>>>    +static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
>>>> +{
>>>> +    u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
>>>
>>> minor nit: This implies we do the TRBE programming in the following
>>> manner in the common case (i.e, TRBE enabled in the beginning of a
>>> session).
>>>    -> set TRBE LIMIT
>>>    -> read TRBE LIMIT
>>>    -> set TRBE ENABLED
>>>
>>> Could we please optimize this ? I believe the buf->trbe_limit
>>> must hold the LIMITR value at any point in time. And thus this
>>
>> But is not bit risky though ! We have got the following places where
>> given trbe_limit instance changes its value.
>>
>> drivers/../coresight-trbe.c:   buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE;
>> drivers/../coresight-trbe.c:   buf->trbe_limit = compute_trbe_buffer_limit(handle);
>> drivers/../coresight-trbe.c:   buf->trbe_limit -= PAGE_SIZE;
> 
> Those are the places where we compute the trbe_limit, *before*
> we enable the TRBE. And we don't change recompute the limit
> *without disabling* the TRBE. To make it more clear, the
> only place where we set TRBE enabled without "computing"
> the trbe_limit is when we hit a spurious interrupt.
> But the value in the TRBLIMITR should already match the
> buf->trbe_limit and we are only going to re-enable the
> TRBE with the same limit. The other option is to
> pass down the "limit" to the set_trbe_enabled().

Since there are just two instances where set_trbe_enabled() gets
called, passing down an additional parameter 'trblimitr' should
still be okay. Some additional code change (like the following)
will achieve this. Does this look okay ?

--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -201,10 +201,8 @@ static inline void trbe_drain_buffer(void)
        dsb(nsh);
 }
 
-static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
+static inline void set_trbe_enabled(struct trbe_cpudata *cpudata, u64 trblimitr)
 {
-       u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
-
        /*
         * Enable the TRBE without clearing LIMITPTR which
         * might be required for fetching the buffer limits.
@@ -626,7 +624,7 @@ static void set_trbe_limit_pointer_enabled(struct trbe_buf *buf)
        trblimitr |= (addr & PAGE_MASK);
 
        write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
-       set_trbe_enabled(buf->cpudata);
+       set_trbe_enabled(buf->cpudata, trblimitr);
 }
 
 static void trbe_enable_hw(struct trbe_buf *buf)
@@ -1050,13 +1048,14 @@ static int arm_trbe_disable(struct coresight_device *csdev)
 static void trbe_handle_spurious(struct perf_output_handle *handle)
 {
        struct trbe_buf *buf = etm_perf_sink_config(handle);
+       u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
 
        /*
         * If the IRQ was spurious, simply re-enable the TRBE
         * back without modifying the buffer parameters to
         * retain the trace collected so far.
         */
-       set_trbe_enabled(buf->cpudata);
+       set_trbe_enabled(buf->cpudata, trblimitr);
 }
 
 static int trbe_handle_overflow(struct perf_output_handle *handle)

 
> 
>>
>>> function could simply be :
>>>
>>> set_trbe_enabled(trbe_buf)
>>> {
>>>      limitr = trbe_buf->limit | LIMITR_ENABLE
>>>      write(limitr, TRBLIMITR_EL1);
>>>      ...
>>> }
>>
>> Is the potential for performance improvement here, out weigh possible
>> risks of using buf->trbe_limit directly while enabling the TRBE ?
> 
> I somehow don't like the fact that we have additional write and read
> for the most common case of the TRBE usage (i.e, for arm_trbe_enable()).
> If we could avoid that, that may be better.
> 
> Cheers
> Suzuki

Re: [PATCH 3/4] coresight: trbe: Work around the invalid prohibited states

[Anshuman Khandual]

On 1/5/22 7:24 PM, Suzuki K Poulose wrote:
> On 05/01/2022 11:16, Anshuman Khandual wrote:
>>
>>
>> On 1/5/22 3:43 PM, Suzuki K Poulose wrote:
>>> Hi Anshuman
>>>
>>> On 05/01/2022 05:05, Anshuman Khandual wrote:
>>>> TRBE implementations affected by Arm erratum #2038923 might get TRBE into
>>>> an inconsistent view on whether trace is prohibited within the CPU. As a
>>>> result, the trace buffer or trace buffer state might be corrupted. This
>>>> happens after TRBE buffer has been enabled by setting TRBLIMITR_EL1.E,
>>>> followed by just a single context synchronization event before execution
>>>> changes from a context, in which trace is prohibited to one where it isn't,
>>>> or vice versa. In these mentioned conditions, the view of whether trace is
>>>> prohibited is inconsistent between parts of the CPU, and the trace buffer
>>>> or the trace buffer state might be corrupted.
>>>>
>>>> Work around this problem in the TRBE driver by preventing an inconsistent
>>>> view of whether the trace is prohibited or not based on TRBLIMITR_EL1.E by
>>>> immediately following a change to TRBLIMITR_EL1.E with at least one ISB
>>>> instruction before an ERET, or two ISB instructions if no ERET is to take
>>>> place. This adds a new cpu errata in arm64 errata framework and also
>>>> updates TRBE driver as required.
>>>>
>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>> Cc: Will Deacon <will@kernel.org>
>>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>>> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
>>>> Cc: coresight@lists.linaro.org
>>>> Cc: linux-doc@vger.kernel.org
>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>> ---
>>>>    Documentation/arm64/silicon-errata.rst       |  2 +
>>>>    arch/arm64/Kconfig                           | 23 ++++++++++
>>>>    arch/arm64/kernel/cpu_errata.c               |  9 ++++
>>>>    arch/arm64/tools/cpucaps                     |  1 +
>>>>    drivers/hwtracing/coresight/coresight-trbe.c | 47 +++++++++++++++-----
>>>>    5 files changed, 72 insertions(+), 10 deletions(-)
>>>
>>> As with the previous patch, it may be a good idea to split the
>>> patch to arm64 and trbe parts.
>>
>> Sure, will do.
>>
>>>
>>>>
>>>> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
>>>> index c9b30e6c2b6c..e0ef3e9a4b8b 100644
>>>> --- a/Documentation/arm64/silicon-errata.rst
>>>> +++ b/Documentation/arm64/silicon-errata.rst
>>>> @@ -54,6 +54,8 @@ stable kernels.
>>>>    +----------------+-----------------+-----------------+-----------------------------+
>>>>    | ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
>>>>    +----------------+-----------------+-----------------+-----------------------------+
>>>> +| ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
>>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>>    | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>>>>    +----------------+-----------------+-----------------+-----------------------------+
>>>>    | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>> index 2105b68d88db..026e34fb6fad 100644
>>>> --- a/arch/arm64/Kconfig
>>>> +++ b/arch/arm64/Kconfig
>>>> @@ -796,6 +796,29 @@ config ARM64_ERRATUM_2064142
>>>>            If unsure, say Y.
>>>>    +config ARM64_ERRATUM_2038923
>>>> +    bool "Cortex-A510: 2038923: workaround TRBE corruption with enable"
>>>> +    depends on CORESIGHT_TRBE
>>>> +    default y
>>>> +    help
>>>> +      This option adds the workaround for ARM Cortex-A510 erratum 2038923.
>>>> +
>>>> +      Affected Cortex-A510 core might cause an inconsistent view on whether trace is
>>>> +      prohibited within the CPU. As a result, the trace buffer or trace buffer state
>>>> +      might be corrupted. This happens after TRBE buffer has been enabled by setting
>>>> +      TRBLIMITR_EL1.E, followed by just a single context synchronization event before
>>>> +      execution changes from a context, in which trace is prohibited to one where it
>>>> +      isn't, or vice versa. In these mentioned conditions, the view of whether trace
>>>> +      is prohibited is inconsistent between parts of the CPU, and the trace buffer or
>>>> +      the trace buffer state might be corrupted.
>>>> +
>>>> +      Work around this in the driver by preventing an inconsistent view of whether the
>>>> +      trace is prohibited or not based on TRBLIMITR_EL1.E by immediately following a
>>>> +      change to TRBLIMITR_EL1.E with at least one ISB instruction before an ERET, or
>>>> +      two ISB instructions if no ERET is to take place.
>>>> +
>>>> +      If unsure, say Y.
>>>> +
>>>>    config CAVIUM_ERRATUM_22375
>>>>        bool "Cavium erratum 22375, 24313"
>>>>        default y
>>>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>>>> index cbb7d5a9aee7..60b0c1f1d912 100644
>>>> --- a/arch/arm64/kernel/cpu_errata.c
>>>> +++ b/arch/arm64/kernel/cpu_errata.c
>>>> @@ -607,6 +607,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>>>>            ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>>>>        },
>>>>    #endif
>>>> +#ifdef CONFIG_ARM64_ERRATUM_2038923
>>>> +    {
>>>> +        .desc = "ARM erratum 2038923",
>>>> +        .capability = ARM64_WORKAROUND_2038923,
>>>> +
>>>> +        /* Cortex-A510 r0p0 - r0p2 */
>>>> +        ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>>>> +    },
>>>> +#endif
>>>>        {
>>>>        }
>>>>    };
>>>> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>>>> index fca3cb329e1d..45a06d36d080 100644
>>>> --- a/arch/arm64/tools/cpucaps
>>>> +++ b/arch/arm64/tools/cpucaps
>>>> @@ -56,6 +56,7 @@ WORKAROUND_1463225
>>>>    WORKAROUND_1508412
>>>>    WORKAROUND_1542419
>>>>    WORKAROUND_2064142
>>>> +WORKAROUND_2038923
>>>>    WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>>>>    WORKAROUND_TSB_FLUSH_FAILURE
>>>>    WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
>>>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>>>> index ec24b62b2cec..0689c6dab96d 100644
>>>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>>>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>>>> @@ -92,11 +92,13 @@ struct trbe_buf {
>>>>    #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE    0
>>>>    #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE    1
>>>>    #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE    2
>>>> +#define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE    3
>>>>      static int trbe_errata_cpucaps[] = {
>>>>        [TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>>>>        [TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>>>>        [TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
>>>> +    [TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
>>>>        -1,        /* Sentinel, must be the last entry */
>>>>    };
>>>>    @@ -174,6 +176,11 @@ static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
>>>>        return trbe_has_erratum(cpudata, TRBE_WORKAROUND_SYSREG_WRITE_FAILURE);
>>>>    }
>>>>    +static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
>>>> +{
>>>
>>> minor nit: trbe_needs_{ctxt_sync, isb}_after_enable() ?
>>
>> trbe_needs_ctxt_sync_after_enable() sounds better. Also will have to change
>> the index above as well .. TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE.
>>
>>>
>>>> +    return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
>>>> +}
>>>> +
>>>>    static int trbe_alloc_node(struct perf_event *event)
>>>>    {
>>>>        if (event->cpu == -1)
>>>> @@ -187,6 +194,30 @@ static inline void trbe_drain_buffer(void)
>>>>        dsb(nsh);
>>>>    }
>>>>    +static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
>>>> +{
>>>> +    u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
>>>
>>> minor nit: This implies we do the TRBE programming in the following
>>> manner in the common case (i.e, TRBE enabled in the beginning of a
>>> session).
>>>    -> set TRBE LIMIT
>>>    -> read TRBE LIMIT
>>>    -> set TRBE ENABLED
>>>
>>> Could we please optimize this ? I believe the buf->trbe_limit
>>> must hold the LIMITR value at any point in time. And thus this
>>
>> But is not bit risky though ! We have got the following places where
>> given trbe_limit instance changes its value.
>>
>> drivers/../coresight-trbe.c:   buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE;
>> drivers/../coresight-trbe.c:   buf->trbe_limit = compute_trbe_buffer_limit(handle);
>> drivers/../coresight-trbe.c:   buf->trbe_limit -= PAGE_SIZE;
> 
> Those are the places where we compute the trbe_limit, *before*
> we enable the TRBE. And we don't change recompute the limit
> *without disabling* the TRBE. To make it more clear, the
> only place where we set TRBE enabled without "computing"
> the trbe_limit is when we hit a spurious interrupt.
> But the value in the TRBLIMITR should already match the
> buf->trbe_limit and we are only going to re-enable the
> TRBE with the same limit. The other option is to
> pass down the "limit" to the set_trbe_enabled().

Since there are just two instances where set_trbe_enabled() gets
called, passing down an additional parameter 'trblimitr' should
still be okay. Some additional code change (like the following)
will achieve this. Does this look okay ?

--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -201,10 +201,8 @@ static inline void trbe_drain_buffer(void)
        dsb(nsh);
 }
 
-static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
+static inline void set_trbe_enabled(struct trbe_cpudata *cpudata, u64 trblimitr)
 {
-       u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
-
        /*
         * Enable the TRBE without clearing LIMITPTR which
         * might be required for fetching the buffer limits.
@@ -626,7 +624,7 @@ static void set_trbe_limit_pointer_enabled(struct trbe_buf *buf)
        trblimitr |= (addr & PAGE_MASK);
 
        write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);
-       set_trbe_enabled(buf->cpudata);
+       set_trbe_enabled(buf->cpudata, trblimitr);
 }
 
 static void trbe_enable_hw(struct trbe_buf *buf)
@@ -1050,13 +1048,14 @@ static int arm_trbe_disable(struct coresight_device *csdev)
 static void trbe_handle_spurious(struct perf_output_handle *handle)
 {
        struct trbe_buf *buf = etm_perf_sink_config(handle);
+       u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
 
        /*
         * If the IRQ was spurious, simply re-enable the TRBE
         * back without modifying the buffer parameters to
         * retain the trace collected so far.
         */
-       set_trbe_enabled(buf->cpudata);
+       set_trbe_enabled(buf->cpudata, trblimitr);
 }
 
 static int trbe_handle_overflow(struct perf_output_handle *handle)

 
> 
>>
>>> function could simply be :
>>>
>>> set_trbe_enabled(trbe_buf)
>>> {
>>>      limitr = trbe_buf->limit | LIMITR_ENABLE
>>>      write(limitr, TRBLIMITR_EL1);
>>>      ...
>>> }
>>
>> Is the potential for performance improvement here, out weigh possible
>> risks of using buf->trbe_limit directly while enabling the TRBE ?
> 
> I somehow don't like the fact that we have additional write and read
> for the most common case of the TRBE usage (i.e, for arm_trbe_enable()).
> If we could avoid that, that may be better.
> 
> Cheers
> Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/4] coresight: trbe: Work around the invalid prohibited states

[Suzuki K Poulose]

On 06/01/2022 09:10, Anshuman Khandual wrote:
> 
> 
> On 1/5/22 7:24 PM, Suzuki K Poulose wrote:
>> On 05/01/2022 11:16, Anshuman Khandual wrote:
>>>
>>>
>>> On 1/5/22 3:43 PM, Suzuki K Poulose wrote:
>>>> Hi Anshuman
>>>>
>>>> On 05/01/2022 05:05, Anshuman Khandual wrote:
>>>>> TRBE implementations affected by Arm erratum #2038923 might get TRBE into
>>>>> an inconsistent view on whether trace is prohibited within the CPU. As a
>>>>> result, the trace buffer or trace buffer state might be corrupted. This
>>>>> happens after TRBE buffer has been enabled by setting TRBLIMITR_EL1.E,
>>>>> followed by just a single context synchronization event before execution
>>>>> changes from a context, in which trace is prohibited to one where it isn't,
>>>>> or vice versa. In these mentioned conditions, the view of whether trace is
>>>>> prohibited is inconsistent between parts of the CPU, and the trace buffer
>>>>> or the trace buffer state might be corrupted.
>>>>>
>>>>> Work around this problem in the TRBE driver by preventing an inconsistent
>>>>> view of whether the trace is prohibited or not based on TRBLIMITR_EL1.E by
>>>>> immediately following a change to TRBLIMITR_EL1.E with at least one ISB
>>>>> instruction before an ERET, or two ISB instructions if no ERET is to take
>>>>> place. This adds a new cpu errata in arm64 errata framework and also
>>>>> updates TRBE driver as required.
>>>>>
>>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>>> Cc: Will Deacon <will@kernel.org>
>>>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>>>> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
>>>>> Cc: coresight@lists.linaro.org
>>>>> Cc: linux-doc@vger.kernel.org
>>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>>> Cc: linux-kernel@vger.kernel.org
>>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>>> ---
>>>>>     Documentation/arm64/silicon-errata.rst       |  2 +
>>>>>     arch/arm64/Kconfig                           | 23 ++++++++++
>>>>>     arch/arm64/kernel/cpu_errata.c               |  9 ++++
>>>>>     arch/arm64/tools/cpucaps                     |  1 +
>>>>>     drivers/hwtracing/coresight/coresight-trbe.c | 47 +++++++++++++++-----
>>>>>     5 files changed, 72 insertions(+), 10 deletions(-)
>>>>
>>>> As with the previous patch, it may be a good idea to split the
>>>> patch to arm64 and trbe parts.
>>>
>>> Sure, will do.
>>>
>>>>
>>>>>
>>>>> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
>>>>> index c9b30e6c2b6c..e0ef3e9a4b8b 100644
>>>>> --- a/Documentation/arm64/silicon-errata.rst
>>>>> +++ b/Documentation/arm64/silicon-errata.rst
>>>>> @@ -54,6 +54,8 @@ stable kernels.
>>>>>     +----------------+-----------------+-----------------+-----------------------------+
>>>>>     | ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
>>>>>     +----------------+-----------------+-----------------+-----------------------------+
>>>>> +| ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
>>>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>>>     | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>>>>>     +----------------+-----------------+-----------------+-----------------------------+
>>>>>     | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
>>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>>> index 2105b68d88db..026e34fb6fad 100644
>>>>> --- a/arch/arm64/Kconfig
>>>>> +++ b/arch/arm64/Kconfig
>>>>> @@ -796,6 +796,29 @@ config ARM64_ERRATUM_2064142
>>>>>             If unsure, say Y.
>>>>>     +config ARM64_ERRATUM_2038923
>>>>> +    bool "Cortex-A510: 2038923: workaround TRBE corruption with enable"
>>>>> +    depends on CORESIGHT_TRBE
>>>>> +    default y
>>>>> +    help
>>>>> +      This option adds the workaround for ARM Cortex-A510 erratum 2038923.
>>>>> +
>>>>> +      Affected Cortex-A510 core might cause an inconsistent view on whether trace is
>>>>> +      prohibited within the CPU. As a result, the trace buffer or trace buffer state
>>>>> +      might be corrupted. This happens after TRBE buffer has been enabled by setting
>>>>> +      TRBLIMITR_EL1.E, followed by just a single context synchronization event before
>>>>> +      execution changes from a context, in which trace is prohibited to one where it
>>>>> +      isn't, or vice versa. In these mentioned conditions, the view of whether trace
>>>>> +      is prohibited is inconsistent between parts of the CPU, and the trace buffer or
>>>>> +      the trace buffer state might be corrupted.
>>>>> +
>>>>> +      Work around this in the driver by preventing an inconsistent view of whether the
>>>>> +      trace is prohibited or not based on TRBLIMITR_EL1.E by immediately following a
>>>>> +      change to TRBLIMITR_EL1.E with at least one ISB instruction before an ERET, or
>>>>> +      two ISB instructions if no ERET is to take place.
>>>>> +
>>>>> +      If unsure, say Y.
>>>>> +
>>>>>     config CAVIUM_ERRATUM_22375
>>>>>         bool "Cavium erratum 22375, 24313"
>>>>>         default y
>>>>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>>>>> index cbb7d5a9aee7..60b0c1f1d912 100644
>>>>> --- a/arch/arm64/kernel/cpu_errata.c
>>>>> +++ b/arch/arm64/kernel/cpu_errata.c
>>>>> @@ -607,6 +607,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>>>>>             ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>>>>>         },
>>>>>     #endif
>>>>> +#ifdef CONFIG_ARM64_ERRATUM_2038923
>>>>> +    {
>>>>> +        .desc = "ARM erratum 2038923",
>>>>> +        .capability = ARM64_WORKAROUND_2038923,
>>>>> +
>>>>> +        /* Cortex-A510 r0p0 - r0p2 */
>>>>> +        ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>>>>> +    },
>>>>> +#endif
>>>>>         {
>>>>>         }
>>>>>     };
>>>>> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>>>>> index fca3cb329e1d..45a06d36d080 100644
>>>>> --- a/arch/arm64/tools/cpucaps
>>>>> +++ b/arch/arm64/tools/cpucaps
>>>>> @@ -56,6 +56,7 @@ WORKAROUND_1463225
>>>>>     WORKAROUND_1508412
>>>>>     WORKAROUND_1542419
>>>>>     WORKAROUND_2064142
>>>>> +WORKAROUND_2038923
>>>>>     WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>>>>>     WORKAROUND_TSB_FLUSH_FAILURE
>>>>>     WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
>>>>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>>>>> index ec24b62b2cec..0689c6dab96d 100644
>>>>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>>>>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>>>>> @@ -92,11 +92,13 @@ struct trbe_buf {
>>>>>     #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE    0
>>>>>     #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE    1
>>>>>     #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE    2
>>>>> +#define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE    3
>>>>>       static int trbe_errata_cpucaps[] = {
>>>>>         [TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>>>>>         [TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>>>>>         [TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
>>>>> +    [TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
>>>>>         -1,        /* Sentinel, must be the last entry */
>>>>>     };
>>>>>     @@ -174,6 +176,11 @@ static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
>>>>>         return trbe_has_erratum(cpudata, TRBE_WORKAROUND_SYSREG_WRITE_FAILURE);
>>>>>     }
>>>>>     +static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
>>>>> +{
>>>>
>>>> minor nit: trbe_needs_{ctxt_sync, isb}_after_enable() ?
>>>
>>> trbe_needs_ctxt_sync_after_enable() sounds better. Also will have to change
>>> the index above as well .. TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE.
>>>
>>>>
>>>>> +    return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
>>>>> +}
>>>>> +
>>>>>     static int trbe_alloc_node(struct perf_event *event)
>>>>>     {
>>>>>         if (event->cpu == -1)
>>>>> @@ -187,6 +194,30 @@ static inline void trbe_drain_buffer(void)
>>>>>         dsb(nsh);
>>>>>     }
>>>>>     +static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
>>>>> +{
>>>>> +    u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
>>>>
>>>> minor nit: This implies we do the TRBE programming in the following
>>>> manner in the common case (i.e, TRBE enabled in the beginning of a
>>>> session).
>>>>     -> set TRBE LIMIT
>>>>     -> read TRBE LIMIT
>>>>     -> set TRBE ENABLED
>>>>
>>>> Could we please optimize this ? I believe the buf->trbe_limit
>>>> must hold the LIMITR value at any point in time. And thus this
>>>
>>> But is not bit risky though ! We have got the following places where
>>> given trbe_limit instance changes its value.
>>>
>>> drivers/../coresight-trbe.c:   buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE;
>>> drivers/../coresight-trbe.c:   buf->trbe_limit = compute_trbe_buffer_limit(handle);
>>> drivers/../coresight-trbe.c:   buf->trbe_limit -= PAGE_SIZE;
>>
>> Those are the places where we compute the trbe_limit, *before*
>> we enable the TRBE. And we don't change recompute the limit
>> *without disabling* the TRBE. To make it more clear, the
>> only place where we set TRBE enabled without "computing"
>> the trbe_limit is when we hit a spurious interrupt.
>> But the value in the TRBLIMITR should already match the
>> buf->trbe_limit and we are only going to re-enable the
>> TRBE with the same limit. The other option is to
>> pass down the "limit" to the set_trbe_enabled().
> 
> Since there are just two instances where set_trbe_enabled() gets
> called, passing down an additional parameter 'trblimitr' should
> still be okay. Some additional code change (like the following)
> will achieve this. Does this look okay ?
> 
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -201,10 +201,8 @@ static inline void trbe_drain_buffer(void)
>          dsb(nsh);
>   }
>   
> -static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
> +static inline void set_trbe_enabled(struct trbe_cpudata *cpudata, u64 trblimitr)
>   {
> -       u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
> -
>          /*
>           * Enable the TRBE without clearing LIMITPTR which
>           * might be required for fetching the buffer limits.
> @@ -626,7 +624,7 @@ static void set_trbe_limit_pointer_enabled(struct trbe_buf *buf)
>          trblimitr |= (addr & PAGE_MASK);
>   
>          write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);

You could skip this ^ write. Otherwise looks good to me.

Suzuki

> -       set_trbe_enabled(buf->cpudata);
> +       set_trbe_enabled(buf->cpudata, trblimitr);
>   }
>   
>   static void trbe_enable_hw(struct trbe_buf *buf)
> @@ -1050,13 +1048,14 @@ static int arm_trbe_disable(struct coresight_device *csdev)
>   static void trbe_handle_spurious(struct perf_output_handle *handle)
>   {
>          struct trbe_buf *buf = etm_perf_sink_config(handle);
> +       u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
>   
>          /*
>           * If the IRQ was spurious, simply re-enable the TRBE
>           * back without modifying the buffer parameters to
>           * retain the trace collected so far.
>           */
> -       set_trbe_enabled(buf->cpudata);
> +       set_trbe_enabled(buf->cpudata, trblimitr);
>   }
>   
>   static int trbe_handle_overflow(struct perf_output_handle *handle)
> 
>   
>>
>>>
>>>> function could simply be :
>>>>
>>>> set_trbe_enabled(trbe_buf)
>>>> {
>>>>       limitr = trbe_buf->limit | LIMITR_ENABLE
>>>>       write(limitr, TRBLIMITR_EL1);
>>>>       ...
>>>> }
>>>
>>> Is the potential for performance improvement here, out weigh possible
>>> risks of using buf->trbe_limit directly while enabling the TRBE ?
>>
>> I somehow don't like the fact that we have additional write and read
>> for the most common case of the TRBE usage (i.e, for arm_trbe_enable()).
>> If we could avoid that, that may be better.
>>
>> Cheers
>> Suzuki

Re: [PATCH 3/4] coresight: trbe: Work around the invalid prohibited states

[Suzuki K Poulose]

On 06/01/2022 09:10, Anshuman Khandual wrote:
> 
> 
> On 1/5/22 7:24 PM, Suzuki K Poulose wrote:
>> On 05/01/2022 11:16, Anshuman Khandual wrote:
>>>
>>>
>>> On 1/5/22 3:43 PM, Suzuki K Poulose wrote:
>>>> Hi Anshuman
>>>>
>>>> On 05/01/2022 05:05, Anshuman Khandual wrote:
>>>>> TRBE implementations affected by Arm erratum #2038923 might get TRBE into
>>>>> an inconsistent view on whether trace is prohibited within the CPU. As a
>>>>> result, the trace buffer or trace buffer state might be corrupted. This
>>>>> happens after TRBE buffer has been enabled by setting TRBLIMITR_EL1.E,
>>>>> followed by just a single context synchronization event before execution
>>>>> changes from a context, in which trace is prohibited to one where it isn't,
>>>>> or vice versa. In these mentioned conditions, the view of whether trace is
>>>>> prohibited is inconsistent between parts of the CPU, and the trace buffer
>>>>> or the trace buffer state might be corrupted.
>>>>>
>>>>> Work around this problem in the TRBE driver by preventing an inconsistent
>>>>> view of whether the trace is prohibited or not based on TRBLIMITR_EL1.E by
>>>>> immediately following a change to TRBLIMITR_EL1.E with at least one ISB
>>>>> instruction before an ERET, or two ISB instructions if no ERET is to take
>>>>> place. This adds a new cpu errata in arm64 errata framework and also
>>>>> updates TRBE driver as required.
>>>>>
>>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>>> Cc: Will Deacon <will@kernel.org>
>>>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>>>> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
>>>>> Cc: coresight@lists.linaro.org
>>>>> Cc: linux-doc@vger.kernel.org
>>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>>> Cc: linux-kernel@vger.kernel.org
>>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>>> ---
>>>>>     Documentation/arm64/silicon-errata.rst       |  2 +
>>>>>     arch/arm64/Kconfig                           | 23 ++++++++++
>>>>>     arch/arm64/kernel/cpu_errata.c               |  9 ++++
>>>>>     arch/arm64/tools/cpucaps                     |  1 +
>>>>>     drivers/hwtracing/coresight/coresight-trbe.c | 47 +++++++++++++++-----
>>>>>     5 files changed, 72 insertions(+), 10 deletions(-)
>>>>
>>>> As with the previous patch, it may be a good idea to split the
>>>> patch to arm64 and trbe parts.
>>>
>>> Sure, will do.
>>>
>>>>
>>>>>
>>>>> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
>>>>> index c9b30e6c2b6c..e0ef3e9a4b8b 100644
>>>>> --- a/Documentation/arm64/silicon-errata.rst
>>>>> +++ b/Documentation/arm64/silicon-errata.rst
>>>>> @@ -54,6 +54,8 @@ stable kernels.
>>>>>     +----------------+-----------------+-----------------+-----------------------------+
>>>>>     | ARM            | Cortex-A510     | #2064142        | ARM64_ERRATUM_2064142       |
>>>>>     +----------------+-----------------+-----------------+-----------------------------+
>>>>> +| ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
>>>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>>>     | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>>>>>     +----------------+-----------------+-----------------+-----------------------------+
>>>>>     | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
>>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>>> index 2105b68d88db..026e34fb6fad 100644
>>>>> --- a/arch/arm64/Kconfig
>>>>> +++ b/arch/arm64/Kconfig
>>>>> @@ -796,6 +796,29 @@ config ARM64_ERRATUM_2064142
>>>>>             If unsure, say Y.
>>>>>     +config ARM64_ERRATUM_2038923
>>>>> +    bool "Cortex-A510: 2038923: workaround TRBE corruption with enable"
>>>>> +    depends on CORESIGHT_TRBE
>>>>> +    default y
>>>>> +    help
>>>>> +      This option adds the workaround for ARM Cortex-A510 erratum 2038923.
>>>>> +
>>>>> +      Affected Cortex-A510 core might cause an inconsistent view on whether trace is
>>>>> +      prohibited within the CPU. As a result, the trace buffer or trace buffer state
>>>>> +      might be corrupted. This happens after TRBE buffer has been enabled by setting
>>>>> +      TRBLIMITR_EL1.E, followed by just a single context synchronization event before
>>>>> +      execution changes from a context, in which trace is prohibited to one where it
>>>>> +      isn't, or vice versa. In these mentioned conditions, the view of whether trace
>>>>> +      is prohibited is inconsistent between parts of the CPU, and the trace buffer or
>>>>> +      the trace buffer state might be corrupted.
>>>>> +
>>>>> +      Work around this in the driver by preventing an inconsistent view of whether the
>>>>> +      trace is prohibited or not based on TRBLIMITR_EL1.E by immediately following a
>>>>> +      change to TRBLIMITR_EL1.E with at least one ISB instruction before an ERET, or
>>>>> +      two ISB instructions if no ERET is to take place.
>>>>> +
>>>>> +      If unsure, say Y.
>>>>> +
>>>>>     config CAVIUM_ERRATUM_22375
>>>>>         bool "Cavium erratum 22375, 24313"
>>>>>         default y
>>>>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>>>>> index cbb7d5a9aee7..60b0c1f1d912 100644
>>>>> --- a/arch/arm64/kernel/cpu_errata.c
>>>>> +++ b/arch/arm64/kernel/cpu_errata.c
>>>>> @@ -607,6 +607,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>>>>>             ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>>>>>         },
>>>>>     #endif
>>>>> +#ifdef CONFIG_ARM64_ERRATUM_2038923
>>>>> +    {
>>>>> +        .desc = "ARM erratum 2038923",
>>>>> +        .capability = ARM64_WORKAROUND_2038923,
>>>>> +
>>>>> +        /* Cortex-A510 r0p0 - r0p2 */
>>>>> +        ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>>>>> +    },
>>>>> +#endif
>>>>>         {
>>>>>         }
>>>>>     };
>>>>> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>>>>> index fca3cb329e1d..45a06d36d080 100644
>>>>> --- a/arch/arm64/tools/cpucaps
>>>>> +++ b/arch/arm64/tools/cpucaps
>>>>> @@ -56,6 +56,7 @@ WORKAROUND_1463225
>>>>>     WORKAROUND_1508412
>>>>>     WORKAROUND_1542419
>>>>>     WORKAROUND_2064142
>>>>> +WORKAROUND_2038923
>>>>>     WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>>>>>     WORKAROUND_TSB_FLUSH_FAILURE
>>>>>     WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
>>>>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>>>>> index ec24b62b2cec..0689c6dab96d 100644
>>>>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>>>>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>>>>> @@ -92,11 +92,13 @@ struct trbe_buf {
>>>>>     #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE    0
>>>>>     #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE    1
>>>>>     #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE    2
>>>>> +#define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE    3
>>>>>       static int trbe_errata_cpucaps[] = {
>>>>>         [TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>>>>>         [TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>>>>>         [TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
>>>>> +    [TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
>>>>>         -1,        /* Sentinel, must be the last entry */
>>>>>     };
>>>>>     @@ -174,6 +176,11 @@ static inline bool trbe_may_fail_sysreg_write(struct trbe_cpudata *cpudata)
>>>>>         return trbe_has_erratum(cpudata, TRBE_WORKAROUND_SYSREG_WRITE_FAILURE);
>>>>>     }
>>>>>     +static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
>>>>> +{
>>>>
>>>> minor nit: trbe_needs_{ctxt_sync, isb}_after_enable() ?
>>>
>>> trbe_needs_ctxt_sync_after_enable() sounds better. Also will have to change
>>> the index above as well .. TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE.
>>>
>>>>
>>>>> +    return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
>>>>> +}
>>>>> +
>>>>>     static int trbe_alloc_node(struct perf_event *event)
>>>>>     {
>>>>>         if (event->cpu == -1)
>>>>> @@ -187,6 +194,30 @@ static inline void trbe_drain_buffer(void)
>>>>>         dsb(nsh);
>>>>>     }
>>>>>     +static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
>>>>> +{
>>>>> +    u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
>>>>
>>>> minor nit: This implies we do the TRBE programming in the following
>>>> manner in the common case (i.e, TRBE enabled in the beginning of a
>>>> session).
>>>>     -> set TRBE LIMIT
>>>>     -> read TRBE LIMIT
>>>>     -> set TRBE ENABLED
>>>>
>>>> Could we please optimize this ? I believe the buf->trbe_limit
>>>> must hold the LIMITR value at any point in time. And thus this
>>>
>>> But is not bit risky though ! We have got the following places where
>>> given trbe_limit instance changes its value.
>>>
>>> drivers/../coresight-trbe.c:   buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE;
>>> drivers/../coresight-trbe.c:   buf->trbe_limit = compute_trbe_buffer_limit(handle);
>>> drivers/../coresight-trbe.c:   buf->trbe_limit -= PAGE_SIZE;
>>
>> Those are the places where we compute the trbe_limit, *before*
>> we enable the TRBE. And we don't change recompute the limit
>> *without disabling* the TRBE. To make it more clear, the
>> only place where we set TRBE enabled without "computing"
>> the trbe_limit is when we hit a spurious interrupt.
>> But the value in the TRBLIMITR should already match the
>> buf->trbe_limit and we are only going to re-enable the
>> TRBE with the same limit. The other option is to
>> pass down the "limit" to the set_trbe_enabled().
> 
> Since there are just two instances where set_trbe_enabled() gets
> called, passing down an additional parameter 'trblimitr' should
> still be okay. Some additional code change (like the following)
> will achieve this. Does this look okay ?
> 
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -201,10 +201,8 @@ static inline void trbe_drain_buffer(void)
>          dsb(nsh);
>   }
>   
> -static inline void set_trbe_enabled(struct trbe_cpudata *cpudata)
> +static inline void set_trbe_enabled(struct trbe_cpudata *cpudata, u64 trblimitr)
>   {
> -       u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
> -
>          /*
>           * Enable the TRBE without clearing LIMITPTR which
>           * might be required for fetching the buffer limits.
> @@ -626,7 +624,7 @@ static void set_trbe_limit_pointer_enabled(struct trbe_buf *buf)
>          trblimitr |= (addr & PAGE_MASK);
>   
>          write_sysreg_s(trblimitr, SYS_TRBLIMITR_EL1);

You could skip this ^ write. Otherwise looks good to me.

Suzuki

> -       set_trbe_enabled(buf->cpudata);
> +       set_trbe_enabled(buf->cpudata, trblimitr);
>   }
>   
>   static void trbe_enable_hw(struct trbe_buf *buf)
> @@ -1050,13 +1048,14 @@ static int arm_trbe_disable(struct coresight_device *csdev)
>   static void trbe_handle_spurious(struct perf_output_handle *handle)
>   {
>          struct trbe_buf *buf = etm_perf_sink_config(handle);
> +       u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
>   
>          /*
>           * If the IRQ was spurious, simply re-enable the TRBE
>           * back without modifying the buffer parameters to
>           * retain the trace collected so far.
>           */
> -       set_trbe_enabled(buf->cpudata);
> +       set_trbe_enabled(buf->cpudata, trblimitr);
>   }
>   
>   static int trbe_handle_overflow(struct perf_output_handle *handle)
> 
>   
>>
>>>
>>>> function could simply be :
>>>>
>>>> set_trbe_enabled(trbe_buf)
>>>> {
>>>>       limitr = trbe_buf->limit | LIMITR_ENABLE
>>>>       write(limitr, TRBLIMITR_EL1);
>>>>       ...
>>>> }
>>>
>>> Is the potential for performance improvement here, out weigh possible
>>> risks of using buf->trbe_limit directly while enabling the TRBE ?
>>
>> I somehow don't like the fact that we have additional write and read
>> for the most common case of the TRBE usage (i.e, for arm_trbe_enable()).
>> If we could avoid that, that may be better.
>>
>> Cheers
>> Suzuki


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/4] coresight: trbe: Workaround TRBE trace data corruption

[Anshuman Khandual]

TRBE implementations affected by Arm erratum #1902691 might corrupt trace
data or deadlock, when it's being written into the memory. Workaround this
problem in the driver, by preventing TRBE initialization on affected cpus.
This adds a new cpu errata in arm64 errata framework and also updates TRBE
driver as required.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Suzuki Poulose <suzuki.poulose@arm.com>
Cc: coresight@lists.linaro.org
Cc: linux-doc@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 Documentation/arm64/silicon-errata.rst       |  2 ++
 arch/arm64/Kconfig                           | 16 ++++++++++++++++
 arch/arm64/kernel/cpu_errata.c               |  9 +++++++++
 arch/arm64/tools/cpucaps                     |  1 +
 drivers/hwtracing/coresight/coresight-trbe.c | 12 ++++++++++++
 5 files changed, 40 insertions(+)

diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
index e0ef3e9a4b8b..50018f60c4d4 100644
--- a/Documentation/arm64/silicon-errata.rst
+++ b/Documentation/arm64/silicon-errata.rst
@@ -56,6 +56,8 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
 +----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Cortex-A510     | #1902691        | ARM64_ERRATUM_1902691       |
++----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 026e34fb6fad..1ea5c3b4aac0 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -819,6 +819,22 @@ config ARM64_ERRATUM_2038923
 
 	  If unsure, say Y.
 
+config ARM64_ERRATUM_1902691
+	bool "Cortex-A510: 1902691: workaround TRBE trace corruption"
+	depends on CORESIGHT_TRBE
+	default y
+	help
+	  This option adds the workaround for ARM Cortex-A510 erratum 1902691.
+
+	  Affected Cortex-A510 core might cause trace data corruption, when being written
+	  into the memory. Effectively TRBE is broken and hence cannot be used to capture
+	  trace data.
+
+	  Work around this problem in the driver by just preventing TRBE initialization on
+	  affected cpus.
+
+	  If unsure, say Y.
+
 config CAVIUM_ERRATUM_22375
 	bool "Cavium erratum 22375, 24313"
 	default y
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 60b0c1f1d912..a3336dfb5a8a 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -615,6 +615,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		/* Cortex-A510 r0p0 - r0p2 */
 		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
 	},
+#endif
+#ifdef CONFIG_ARM64_ERRATUM_1902691
+	{
+		.desc = "ARM erratum 1902691",
+		.capability = ARM64_WORKAROUND_1902691,
+
+		/* Cortex-A510 r0p0 - r0p1 */
+		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 1)
+	},
 #endif
 	{
 	}
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 45a06d36d080..e7719e8f18de 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -57,6 +57,7 @@ WORKAROUND_1508412
 WORKAROUND_1542419
 WORKAROUND_2064142
 WORKAROUND_2038923
+WORKAROUND_1902691
 WORKAROUND_TRBE_OVERWRITE_FILL_MODE
 WORKAROUND_TSB_FLUSH_FAILURE
 WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 0689c6dab96d..b9b4e34fac15 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -93,12 +93,14 @@ struct trbe_buf {
 #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE	1
 #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE	2
 #define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE	3
+#define TRBE_IS_BROKEN	4
 
 static int trbe_errata_cpucaps[] = {
 	[TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
 	[TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
 	[TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
 	[TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
+	[TRBE_IS_BROKEN] = ARM64_WORKAROUND_1902691,
 	-1,		/* Sentinel, must be the last entry */
 };
 
@@ -181,6 +183,11 @@ static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
 	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
 }
 
+static inline bool trbe_is_broken(struct trbe_cpudata *cpudata)
+{
+	return trbe_has_erratum(cpudata, TRBE_IS_BROKEN);
+}
+
 static int trbe_alloc_node(struct perf_event *event)
 {
 	if (event->cpu == -1)
@@ -1291,6 +1298,11 @@ static void arm_trbe_probe_cpu(void *info)
 	 */
 	trbe_check_errata(cpudata);
 
+	if (trbe_is_broken(cpudata)) {
+		pr_err("TRBE might corrupt the trace on cpu %d\n", cpu);
+		goto cpu_clear;
+	}
+
 	/*
 	 * If the TRBE is affected by erratum TRBE_WORKAROUND_OVERWRITE_FILL_MODE,
 	 * we must always program the TBRPTR_EL1, 256bytes from a page
-- 
2.25.1

[PATCH 4/4] coresight: trbe: Workaround TRBE trace data corruption

[Anshuman Khandual]

TRBE implementations affected by Arm erratum #1902691 might corrupt trace
data or deadlock, when it's being written into the memory. Workaround this
problem in the driver, by preventing TRBE initialization on affected cpus.
This adds a new cpu errata in arm64 errata framework and also updates TRBE
driver as required.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Suzuki Poulose <suzuki.poulose@arm.com>
Cc: coresight@lists.linaro.org
Cc: linux-doc@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 Documentation/arm64/silicon-errata.rst       |  2 ++
 arch/arm64/Kconfig                           | 16 ++++++++++++++++
 arch/arm64/kernel/cpu_errata.c               |  9 +++++++++
 arch/arm64/tools/cpucaps                     |  1 +
 drivers/hwtracing/coresight/coresight-trbe.c | 12 ++++++++++++
 5 files changed, 40 insertions(+)

diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
index e0ef3e9a4b8b..50018f60c4d4 100644
--- a/Documentation/arm64/silicon-errata.rst
+++ b/Documentation/arm64/silicon-errata.rst
@@ -56,6 +56,8 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
 +----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Cortex-A510     | #1902691        | ARM64_ERRATUM_1902691       |
++----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 026e34fb6fad..1ea5c3b4aac0 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -819,6 +819,22 @@ config ARM64_ERRATUM_2038923
 
 	  If unsure, say Y.
 
+config ARM64_ERRATUM_1902691
+	bool "Cortex-A510: 1902691: workaround TRBE trace corruption"
+	depends on CORESIGHT_TRBE
+	default y
+	help
+	  This option adds the workaround for ARM Cortex-A510 erratum 1902691.
+
+	  Affected Cortex-A510 core might cause trace data corruption, when being written
+	  into the memory. Effectively TRBE is broken and hence cannot be used to capture
+	  trace data.
+
+	  Work around this problem in the driver by just preventing TRBE initialization on
+	  affected cpus.
+
+	  If unsure, say Y.
+
 config CAVIUM_ERRATUM_22375
 	bool "Cavium erratum 22375, 24313"
 	default y
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 60b0c1f1d912..a3336dfb5a8a 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -615,6 +615,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		/* Cortex-A510 r0p0 - r0p2 */
 		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
 	},
+#endif
+#ifdef CONFIG_ARM64_ERRATUM_1902691
+	{
+		.desc = "ARM erratum 1902691",
+		.capability = ARM64_WORKAROUND_1902691,
+
+		/* Cortex-A510 r0p0 - r0p1 */
+		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 1)
+	},
 #endif
 	{
 	}
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 45a06d36d080..e7719e8f18de 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -57,6 +57,7 @@ WORKAROUND_1508412
 WORKAROUND_1542419
 WORKAROUND_2064142
 WORKAROUND_2038923
+WORKAROUND_1902691
 WORKAROUND_TRBE_OVERWRITE_FILL_MODE
 WORKAROUND_TSB_FLUSH_FAILURE
 WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 0689c6dab96d..b9b4e34fac15 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -93,12 +93,14 @@ struct trbe_buf {
 #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE	1
 #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE	2
 #define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE	3
+#define TRBE_IS_BROKEN	4
 
 static int trbe_errata_cpucaps[] = {
 	[TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
 	[TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
 	[TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
 	[TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
+	[TRBE_IS_BROKEN] = ARM64_WORKAROUND_1902691,
 	-1,		/* Sentinel, must be the last entry */
 };
 
@@ -181,6 +183,11 @@ static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
 	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
 }
 
+static inline bool trbe_is_broken(struct trbe_cpudata *cpudata)
+{
+	return trbe_has_erratum(cpudata, TRBE_IS_BROKEN);
+}
+
 static int trbe_alloc_node(struct perf_event *event)
 {
 	if (event->cpu == -1)
@@ -1291,6 +1298,11 @@ static void arm_trbe_probe_cpu(void *info)
 	 */
 	trbe_check_errata(cpudata);
 
+	if (trbe_is_broken(cpudata)) {
+		pr_err("TRBE might corrupt the trace on cpu %d\n", cpu);
+		goto cpu_clear;
+	}
+
 	/*
 	 * If the TRBE is affected by erratum TRBE_WORKAROUND_OVERWRITE_FILL_MODE,
 	 * we must always program the TBRPTR_EL1, 256bytes from a page
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] coresight: trbe: Workaround TRBE trace data corruption

[Suzuki K Poulose]

On 05/01/2022 05:05, Anshuman Khandual wrote:
> TRBE implementations affected by Arm erratum #1902691 might corrupt trace
> data or deadlock, when it's being written into the memory. Workaround this
> problem in the driver, by preventing TRBE initialization on affected cpus.
> This adds a new cpu errata in arm64 errata framework and also updates TRBE
> driver as required.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
> Cc: coresight@lists.linaro.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>   Documentation/arm64/silicon-errata.rst       |  2 ++
>   arch/arm64/Kconfig                           | 16 ++++++++++++++++
>   arch/arm64/kernel/cpu_errata.c               |  9 +++++++++
>   arch/arm64/tools/cpucaps                     |  1 +
>   drivers/hwtracing/coresight/coresight-trbe.c | 12 ++++++++++++
>   5 files changed, 40 insertions(+)
> 
> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
> index e0ef3e9a4b8b..50018f60c4d4 100644
> --- a/Documentation/arm64/silicon-errata.rst
> +++ b/Documentation/arm64/silicon-errata.rst
> @@ -56,6 +56,8 @@ stable kernels.
>   +----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
>   +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Cortex-A510     | #1902691        | ARM64_ERRATUM_1902691       |
> ++----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>   +----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 026e34fb6fad..1ea5c3b4aac0 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -819,6 +819,22 @@ config ARM64_ERRATUM_2038923
>   
>   	  If unsure, say Y.
>   
> +config ARM64_ERRATUM_1902691
> +	bool "Cortex-A510: 1902691: workaround TRBE trace corruption"
> +	depends on CORESIGHT_TRBE
> +	default y
> +	help
> +	  This option adds the workaround for ARM Cortex-A510 erratum 1902691.
> +
> +	  Affected Cortex-A510 core might cause trace data corruption, when being written
> +	  into the memory. Effectively TRBE is broken and hence cannot be used to capture
> +	  trace data.
> +
> +	  Work around this problem in the driver by just preventing TRBE initialization on
> +	  affected cpus.
> +
> +	  If unsure, say Y.
> +

It might be better to add :

"The firmware must have disabled the access to TRBE for the kernel on 
such implementations. This will cover the kernel for any firmware that
doesn't do this already"


>   config CAVIUM_ERRATUM_22375
>   	bool "Cavium erratum 22375, 24313"
>   	default y
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 60b0c1f1d912..a3336dfb5a8a 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -615,6 +615,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {	
>   		/* Cortex-A510 r0p0 - r0p2 */
>   		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>   	},
> +#endif
> +#ifdef CONFIG_ARM64_ERRATUM_1902691
> +	{
> +		.desc = "ARM erratum 1902691",
> +		.capability = ARM64_WORKAROUND_1902691,
> +
> +		/* Cortex-A510 r0p0 - r0p1 */
> +		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 1)
> +	},
>   #endif
>   	{
>   	}
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> index 45a06d36d080..e7719e8f18de 100644
> --- a/arch/arm64/tools/cpucaps
> +++ b/arch/arm64/tools/cpucaps
> @@ -57,6 +57,7 @@ WORKAROUND_1508412
>   WORKAROUND_1542419
>   WORKAROUND_2064142
>   WORKAROUND_2038923
> +WORKAROUND_1902691
>   WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>   WORKAROUND_TSB_FLUSH_FAILURE
>   WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 0689c6dab96d..b9b4e34fac15 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -93,12 +93,14 @@ struct trbe_buf {
>   #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE	1
>   #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE	2
>   #define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE	3
> +#define TRBE_IS_BROKEN	4
>   
>   static int trbe_errata_cpucaps[] = {
>   	[TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>   	[TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>   	[TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
>   	[TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
> +	[TRBE_IS_BROKEN] = ARM64_WORKAROUND_1902691,
>   	-1,		/* Sentinel, must be the last entry */
>   };
>   
> @@ -181,6 +183,11 @@ static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
>   	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
>   }
>   
> +static inline bool trbe_is_broken(struct trbe_cpudata *cpudata)
> +{
> +	return trbe_has_erratum(cpudata, TRBE_IS_BROKEN);
> +}
> +
>   static int trbe_alloc_node(struct perf_event *event)
>   {
>   	if (event->cpu == -1)
> @@ -1291,6 +1298,11 @@ static void arm_trbe_probe_cpu(void *info)
>   	 */
>   	trbe_check_errata(cpudata);
>   
> +	if (trbe_is_broken(cpudata)) {
> +		pr_err("TRBE might corrupt the trace on cpu %d\n", cpu);

It may be better to say:

		"Disabling TRBE on CPU%d due to erratum"

Otherwise looks good to me.

Suzuki

Re: [PATCH 4/4] coresight: trbe: Workaround TRBE trace data corruption

[Suzuki K Poulose]

On 05/01/2022 05:05, Anshuman Khandual wrote:
> TRBE implementations affected by Arm erratum #1902691 might corrupt trace
> data or deadlock, when it's being written into the memory. Workaround this
> problem in the driver, by preventing TRBE initialization on affected cpus.
> This adds a new cpu errata in arm64 errata framework and also updates TRBE
> driver as required.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
> Cc: coresight@lists.linaro.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>   Documentation/arm64/silicon-errata.rst       |  2 ++
>   arch/arm64/Kconfig                           | 16 ++++++++++++++++
>   arch/arm64/kernel/cpu_errata.c               |  9 +++++++++
>   arch/arm64/tools/cpucaps                     |  1 +
>   drivers/hwtracing/coresight/coresight-trbe.c | 12 ++++++++++++
>   5 files changed, 40 insertions(+)
> 
> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
> index e0ef3e9a4b8b..50018f60c4d4 100644
> --- a/Documentation/arm64/silicon-errata.rst
> +++ b/Documentation/arm64/silicon-errata.rst
> @@ -56,6 +56,8 @@ stable kernels.
>   +----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
>   +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Cortex-A510     | #1902691        | ARM64_ERRATUM_1902691       |
> ++----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>   +----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 026e34fb6fad..1ea5c3b4aac0 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -819,6 +819,22 @@ config ARM64_ERRATUM_2038923
>   
>   	  If unsure, say Y.
>   
> +config ARM64_ERRATUM_1902691
> +	bool "Cortex-A510: 1902691: workaround TRBE trace corruption"
> +	depends on CORESIGHT_TRBE
> +	default y
> +	help
> +	  This option adds the workaround for ARM Cortex-A510 erratum 1902691.
> +
> +	  Affected Cortex-A510 core might cause trace data corruption, when being written
> +	  into the memory. Effectively TRBE is broken and hence cannot be used to capture
> +	  trace data.
> +
> +	  Work around this problem in the driver by just preventing TRBE initialization on
> +	  affected cpus.
> +
> +	  If unsure, say Y.
> +

It might be better to add :

"The firmware must have disabled the access to TRBE for the kernel on 
such implementations. This will cover the kernel for any firmware that
doesn't do this already"


>   config CAVIUM_ERRATUM_22375
>   	bool "Cavium erratum 22375, 24313"
>   	default y
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 60b0c1f1d912..a3336dfb5a8a 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -615,6 +615,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {	
>   		/* Cortex-A510 r0p0 - r0p2 */
>   		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>   	},
> +#endif
> +#ifdef CONFIG_ARM64_ERRATUM_1902691
> +	{
> +		.desc = "ARM erratum 1902691",
> +		.capability = ARM64_WORKAROUND_1902691,
> +
> +		/* Cortex-A510 r0p0 - r0p1 */
> +		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 1)
> +	},
>   #endif
>   	{
>   	}
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> index 45a06d36d080..e7719e8f18de 100644
> --- a/arch/arm64/tools/cpucaps
> +++ b/arch/arm64/tools/cpucaps
> @@ -57,6 +57,7 @@ WORKAROUND_1508412
>   WORKAROUND_1542419
>   WORKAROUND_2064142
>   WORKAROUND_2038923
> +WORKAROUND_1902691
>   WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>   WORKAROUND_TSB_FLUSH_FAILURE
>   WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 0689c6dab96d..b9b4e34fac15 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -93,12 +93,14 @@ struct trbe_buf {
>   #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE	1
>   #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE	2
>   #define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE	3
> +#define TRBE_IS_BROKEN	4
>   
>   static int trbe_errata_cpucaps[] = {
>   	[TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>   	[TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>   	[TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
>   	[TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
> +	[TRBE_IS_BROKEN] = ARM64_WORKAROUND_1902691,
>   	-1,		/* Sentinel, must be the last entry */
>   };
>   
> @@ -181,6 +183,11 @@ static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
>   	return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
>   }
>   
> +static inline bool trbe_is_broken(struct trbe_cpudata *cpudata)
> +{
> +	return trbe_has_erratum(cpudata, TRBE_IS_BROKEN);
> +}
> +
>   static int trbe_alloc_node(struct perf_event *event)
>   {
>   	if (event->cpu == -1)
> @@ -1291,6 +1298,11 @@ static void arm_trbe_probe_cpu(void *info)
>   	 */
>   	trbe_check_errata(cpudata);
>   
> +	if (trbe_is_broken(cpudata)) {
> +		pr_err("TRBE might corrupt the trace on cpu %d\n", cpu);

It may be better to say:

		"Disabling TRBE on CPU%d due to erratum"

Otherwise looks good to me.

Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] coresight: trbe: Workaround TRBE trace data corruption

[Anshuman Khandual]

On 1/5/22 5:29 PM, Suzuki K Poulose wrote:
> On 05/01/2022 05:05, Anshuman Khandual wrote:
>> TRBE implementations affected by Arm erratum #1902691 might corrupt trace
>> data or deadlock, when it's being written into the memory. Workaround this
>> problem in the driver, by preventing TRBE initialization on affected cpus.
>> This adds a new cpu errata in arm64 errata framework and also updates TRBE
>> driver as required.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
>> Cc: coresight@lists.linaro.org
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>   Documentation/arm64/silicon-errata.rst       |  2 ++
>>   arch/arm64/Kconfig                           | 16 ++++++++++++++++
>>   arch/arm64/kernel/cpu_errata.c               |  9 +++++++++
>>   arch/arm64/tools/cpucaps                     |  1 +
>>   drivers/hwtracing/coresight/coresight-trbe.c | 12 ++++++++++++
>>   5 files changed, 40 insertions(+)
>>
>> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
>> index e0ef3e9a4b8b..50018f60c4d4 100644
>> --- a/Documentation/arm64/silicon-errata.rst
>> +++ b/Documentation/arm64/silicon-errata.rst
>> @@ -56,6 +56,8 @@ stable kernels.
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-A510     | #1902691        | ARM64_ERRATUM_1902691       |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 026e34fb6fad..1ea5c3b4aac0 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -819,6 +819,22 @@ config ARM64_ERRATUM_2038923
>>           If unsure, say Y.
>>   +config ARM64_ERRATUM_1902691
>> +    bool "Cortex-A510: 1902691: workaround TRBE trace corruption"
>> +    depends on CORESIGHT_TRBE
>> +    default y
>> +    help
>> +      This option adds the workaround for ARM Cortex-A510 erratum 1902691.
>> +
>> +      Affected Cortex-A510 core might cause trace data corruption, when being written
>> +      into the memory. Effectively TRBE is broken and hence cannot be used to capture
>> +      trace data.
>> +
>> +      Work around this problem in the driver by just preventing TRBE initialization on
>> +      affected cpus.
>> +
>> +      If unsure, say Y.
>> +
> 
> It might be better to add :
> 
> "The firmware must have disabled the access to TRBE for the kernel on such implementations. This will cover the kernel for any firmware that
> doesn't do this already"

Right, will add it.

> 
> 
>>   config CAVIUM_ERRATUM_22375
>>       bool "Cavium erratum 22375, 24313"
>>       default y
>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>> index 60b0c1f1d912..a3336dfb5a8a 100644
>> --- a/arch/arm64/kernel/cpu_errata.c
>> +++ b/arch/arm64/kernel/cpu_errata.c
>> @@ -615,6 +615,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {   
>>           /* Cortex-A510 r0p0 - r0p2 */
>>           ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>>       },
>> +#endif
>> +#ifdef CONFIG_ARM64_ERRATUM_1902691
>> +    {
>> +        .desc = "ARM erratum 1902691",
>> +        .capability = ARM64_WORKAROUND_1902691,
>> +
>> +        /* Cortex-A510 r0p0 - r0p1 */
>> +        ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 1)
>> +    },
>>   #endif
>>       {
>>       }
>> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>> index 45a06d36d080..e7719e8f18de 100644
>> --- a/arch/arm64/tools/cpucaps
>> +++ b/arch/arm64/tools/cpucaps
>> @@ -57,6 +57,7 @@ WORKAROUND_1508412
>>   WORKAROUND_1542419
>>   WORKAROUND_2064142
>>   WORKAROUND_2038923
>> +WORKAROUND_1902691
>>   WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>>   WORKAROUND_TSB_FLUSH_FAILURE
>>   WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>> index 0689c6dab96d..b9b4e34fac15 100644
>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>> @@ -93,12 +93,14 @@ struct trbe_buf {
>>   #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE    1
>>   #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE    2
>>   #define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE    3
>> +#define TRBE_IS_BROKEN    4
>>     static int trbe_errata_cpucaps[] = {
>>       [TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>>       [TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>>       [TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
>>       [TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
>> +    [TRBE_IS_BROKEN] = ARM64_WORKAROUND_1902691,
>>       -1,        /* Sentinel, must be the last entry */
>>   };
>>   @@ -181,6 +183,11 @@ static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
>>       return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
>>   }
>>   +static inline bool trbe_is_broken(struct trbe_cpudata *cpudata)
>> +{
>> +    return trbe_has_erratum(cpudata, TRBE_IS_BROKEN);
>> +}
>> +
>>   static int trbe_alloc_node(struct perf_event *event)
>>   {
>>       if (event->cpu == -1)
>> @@ -1291,6 +1298,11 @@ static void arm_trbe_probe_cpu(void *info)
>>        */
>>       trbe_check_errata(cpudata);
>>   +    if (trbe_is_broken(cpudata)) {
>> +        pr_err("TRBE might corrupt the trace on cpu %d\n", cpu);
> 
> It may be better to say:
> 
>         "Disabling TRBE on CPU%d due to erratum"

Mention the erratum #1902691 as well or not required ?

> 
> Otherwise looks good to me.
> 
> Suzuki

Re: [PATCH 4/4] coresight: trbe: Workaround TRBE trace data corruption

[Anshuman Khandual]

On 1/5/22 5:29 PM, Suzuki K Poulose wrote:
> On 05/01/2022 05:05, Anshuman Khandual wrote:
>> TRBE implementations affected by Arm erratum #1902691 might corrupt trace
>> data or deadlock, when it's being written into the memory. Workaround this
>> problem in the driver, by preventing TRBE initialization on affected cpus.
>> This adds a new cpu errata in arm64 errata framework and also updates TRBE
>> driver as required.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
>> Cc: coresight@lists.linaro.org
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>   Documentation/arm64/silicon-errata.rst       |  2 ++
>>   arch/arm64/Kconfig                           | 16 ++++++++++++++++
>>   arch/arm64/kernel/cpu_errata.c               |  9 +++++++++
>>   arch/arm64/tools/cpucaps                     |  1 +
>>   drivers/hwtracing/coresight/coresight-trbe.c | 12 ++++++++++++
>>   5 files changed, 40 insertions(+)
>>
>> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
>> index e0ef3e9a4b8b..50018f60c4d4 100644
>> --- a/Documentation/arm64/silicon-errata.rst
>> +++ b/Documentation/arm64/silicon-errata.rst
>> @@ -56,6 +56,8 @@ stable kernels.
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A510     | #2038923        | ARM64_ERRATUM_2038923       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-A510     | #1902691        | ARM64_ERRATUM_1902691       |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319        |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319        |
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 026e34fb6fad..1ea5c3b4aac0 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -819,6 +819,22 @@ config ARM64_ERRATUM_2038923
>>           If unsure, say Y.
>>   +config ARM64_ERRATUM_1902691
>> +    bool "Cortex-A510: 1902691: workaround TRBE trace corruption"
>> +    depends on CORESIGHT_TRBE
>> +    default y
>> +    help
>> +      This option adds the workaround for ARM Cortex-A510 erratum 1902691.
>> +
>> +      Affected Cortex-A510 core might cause trace data corruption, when being written
>> +      into the memory. Effectively TRBE is broken and hence cannot be used to capture
>> +      trace data.
>> +
>> +      Work around this problem in the driver by just preventing TRBE initialization on
>> +      affected cpus.
>> +
>> +      If unsure, say Y.
>> +
> 
> It might be better to add :
> 
> "The firmware must have disabled the access to TRBE for the kernel on such implementations. This will cover the kernel for any firmware that
> doesn't do this already"

Right, will add it.

> 
> 
>>   config CAVIUM_ERRATUM_22375
>>       bool "Cavium erratum 22375, 24313"
>>       default y
>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>> index 60b0c1f1d912..a3336dfb5a8a 100644
>> --- a/arch/arm64/kernel/cpu_errata.c
>> +++ b/arch/arm64/kernel/cpu_errata.c
>> @@ -615,6 +615,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = {   
>>           /* Cortex-A510 r0p0 - r0p2 */
>>           ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2)
>>       },
>> +#endif
>> +#ifdef CONFIG_ARM64_ERRATUM_1902691
>> +    {
>> +        .desc = "ARM erratum 1902691",
>> +        .capability = ARM64_WORKAROUND_1902691,
>> +
>> +        /* Cortex-A510 r0p0 - r0p1 */
>> +        ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 1)
>> +    },
>>   #endif
>>       {
>>       }
>> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>> index 45a06d36d080..e7719e8f18de 100644
>> --- a/arch/arm64/tools/cpucaps
>> +++ b/arch/arm64/tools/cpucaps
>> @@ -57,6 +57,7 @@ WORKAROUND_1508412
>>   WORKAROUND_1542419
>>   WORKAROUND_2064142
>>   WORKAROUND_2038923
>> +WORKAROUND_1902691
>>   WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>>   WORKAROUND_TSB_FLUSH_FAILURE
>>   WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>> index 0689c6dab96d..b9b4e34fac15 100644
>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>> @@ -93,12 +93,14 @@ struct trbe_buf {
>>   #define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE    1
>>   #define TRBE_WORKAROUND_SYSREG_WRITE_FAILURE    2
>>   #define TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE    3
>> +#define TRBE_IS_BROKEN    4
>>     static int trbe_errata_cpucaps[] = {
>>       [TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
>>       [TRBE_WORKAROUND_WRITE_OUT_OF_RANGE] = ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE,
>>       [TRBE_WORKAROUND_SYSREG_WRITE_FAILURE] = ARM64_WORKAROUND_2064142,
>>       [TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE] = ARM64_WORKAROUND_2038923,
>> +    [TRBE_IS_BROKEN] = ARM64_WORKAROUND_1902691,
>>       -1,        /* Sentinel, must be the last entry */
>>   };
>>   @@ -181,6 +183,11 @@ static inline bool trbe_may_corrupt_with_enable(struct trbe_cpudata *cpudata)
>>       return trbe_has_erratum(cpudata, TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE);
>>   }
>>   +static inline bool trbe_is_broken(struct trbe_cpudata *cpudata)
>> +{
>> +    return trbe_has_erratum(cpudata, TRBE_IS_BROKEN);
>> +}
>> +
>>   static int trbe_alloc_node(struct perf_event *event)
>>   {
>>       if (event->cpu == -1)
>> @@ -1291,6 +1298,11 @@ static void arm_trbe_probe_cpu(void *info)
>>        */
>>       trbe_check_errata(cpudata);
>>   +    if (trbe_is_broken(cpudata)) {
>> +        pr_err("TRBE might corrupt the trace on cpu %d\n", cpu);
> 
> It may be better to say:
> 
>         "Disabling TRBE on CPU%d due to erratum"

Mention the erratum #1902691 as well or not required ?

> 
> Otherwise looks good to me.
> 
> Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel