lkml.org 
[lkml]   [2022]   [Jan]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH V2 5/7] coresight: trbe: Work around the ignored system register writes
From
On 07/01/2022 01:10, 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 just updates the 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>
> ---
> arch/arm64/Kconfig | 2 +-
> drivers/hwtracing/coresight/coresight-trbe.c | 54 ++++++++++++++------
> drivers/hwtracing/coresight/coresight-trbe.h | 8 ---
> 3 files changed, 39 insertions(+), 25 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index f1651cb71ef3..b6d62672bf7d 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -780,7 +780,7 @@ config ARM64_ERRATUM_2224489
>
> config ARM64_ERRATUM_2064142
> bool "Cortex-A510: 2064142: workaround TRBE register writes while disabled"
> - depends on COMPILE_TEST # Until the CoreSight TRBE driver changes are in
> + depends on CORESIGHT_TRBE
> default y
> help
> This option adds the workaround for ARM Cortex-A510 erratum 2064142.
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 276862c07e32..850e9fca6847 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_NEEDS_DRAIN_AFTER_DISABLE 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_NEEDS_DRAIN_AFTER_DISABLE] = 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_needs_drain_after_disable(struct trbe_cpudata *cpudata)
> +{
> + return trbe_has_erratum(cpudata, TRBE_NEEDS_DRAIN_AFTER_DISABLE);
> +}
> +
> 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.
> + */

minor nit: This comment could be moved to the definition of the
function "trbe_needs_drain_after_disable()" to make more sense.
The name is implicit here indicating, why we are doing a drain.

Either ways:

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

\
 
 \ /
  Last update: 2022-01-10 12:04    [W:0.109 / U:0.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site