On 1/10/22 4:33 PM, Suzuki K Poulose wrote:
> 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@xxxxxxx>
>> Cc: Will Deacon <will@xxxxxxxxxx>
>> Cc: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
>> Cc: Suzuki Poulose <suzuki.poulose@xxxxxxx>
>> Cc: coresight@xxxxxxxxxxxxxxxx
>> Cc: linux-doc@xxxxxxxxxxxxxxx
>> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
>> ---
>> 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.
Sure, will move the comment just before the function definition.
>
> Either ways:
>
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
>
