Messages in this thread | | | Date | Mon, 10 Jan 2022 11:03:05 +0000 | Subject | Re: [PATCH V2 5/7] coresight: trbe: Work around the ignored system register writes | From | Suzuki K Poulose <> |
| |
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>
| |