Re: [PATCH 06/19] coresight: trbe: Refactor syndrome decoding
From: Suzuki K Poulose <suzuki.poulose@arm.com>
Date: 2025-12-02 11:06:52
Also in:
linux-perf-users, lkml
Hi Leo A couple of minor nits below. Otherwise looks like a nice cleanup On 01/12/2025 11:21, Leo Yan wrote:
It gives priority to TRBSR_EL1.EA (external abort); an external abort will immediately bail out and return an error. Next, the syndrome decoding is refactored based on two levels of information: the EC (Event Class) bits and the BSC (Trace Buffer Status Code) bits. If TRBSR_EL1.EC==0b000000, the driver continues parsing TRBSR_EL1.BSC to identify the specific trace buffer event. Otherwise, any non-zero TRBSR_EL1.EC is treated as an error. For error cases, the driver prints an error string and dumps registers for debugging.
minor nit: Please avoid describing the code, which cod does well. Briefly mention what you are doing, giving any additional contexts that may not be evident from the code. e.g: "Add support for decoding the syndrome for TRBE, providing a verbose description of the code for Fatal/unhandled events. While at it add the new definitions from the latest Arm ARM for EC and BSC"
No additional checks are required for wrap mode beyond verifying the TRBSR_EL1.WRAP bit, even on units with overwrite errata, as this bit
quoted hunk ↗ jump to hunk
reliably indicates a buffer wrap. Signed-off-by: Leo Yan <leo.yan@arm.com> --- drivers/hwtracing/coresight/coresight-trbe.c | 60 +++++++++++++++++++++------- drivers/hwtracing/coresight/coresight-trbe.h | 8 ++-- 2 files changed, 51 insertions(+), 17 deletions(-)diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 9e565122816949b37b8ff5e4ba04cfbc317c6f25..28e2bfa68074f19ccaa4a737d00af577aea818fe 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c@@ -643,29 +643,61 @@ static void trbe_enable_hw(struct trbe_buf *buf) static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *handle, u64 trbsr) { + const char *err_str; int ec = get_trbe_ec(trbsr); int bsc = get_trbe_bsc(trbsr); - struct trbe_buf *buf = etm_perf_sink_config(handle); - struct trbe_cpudata *cpudata = buf->cpudata; WARN_ON(is_trbe_running(trbsr)); - if (is_trbe_trg(trbsr) || is_trbe_abort(trbsr)) - return TRBE_FAULT_ACT_FATAL; - if ((ec == TRBE_EC_STAGE1_ABORT) || (ec == TRBE_EC_STAGE2_ABORT)) - return TRBE_FAULT_ACT_FATAL; + if (is_trbe_abort(trbsr)) { + err_str = "External abort"; + goto out_fatal; + } - /* - * If the trbe is affected by TRBE_WORKAROUND_OVERWRITE_FILL_MODE, - * it might write data after a WRAP event in the fill mode. - * Thus the check TRBPTR == TRBBASER will not be honored. - */ - if ((is_trbe_wrap(trbsr) && (ec == TRBE_EC_OTHERS) && (bsc == TRBE_BSC_FILLED)) && - (trbe_may_overwrite_in_fill_mode(cpudata) || - get_trbe_write_pointer() == get_trbe_base_pointer())) + switch (ec) { + case TRBE_EC_OTHERS: + break; + case TRBE_EC_BUF_MGMT_IMPL: + err_str = "Unexpected implemented management";
minor nit: "Unexpected IMPDEF buffer management event" Suzuki
quoted hunk ↗ jump to hunk
+ goto out_fatal; + case TRBE_EC_GP_CHECK_FAULT: + err_str = "Granule Protection Check fault"; + goto out_fatal; + case TRBE_EC_STAGE1_ABORT: + err_str = "Stage 1 data abort"; + goto out_fatal; + case TRBE_EC_STAGE2_ABORT: + err_str = "Stage 2 data abort"; + goto out_fatal; + default: + err_str = "Unknown error code"; + goto out_fatal; + } + + switch (bsc) { + case TRBE_BSC_NOT_STOPPED: + break; + case TRBE_BSC_FILLED: + break; + case TRBE_BSC_TRIGGERED: + err_str = "Unexpected trigger status"; + goto out_fatal; + default: + err_str = "Unexpected buffer status code"; + goto out_fatal; + } + + if (is_trbe_wrap(trbsr)) return TRBE_FAULT_ACT_WRAP; return TRBE_FAULT_ACT_SPURIOUS; + +out_fatal: + pr_err_ratelimited("%s on CPU %d [TRBSR=0x%016llx, TRBPTR=0x%016llx, TRBLIMITR=0x%016llx]\n", + err_str, smp_processor_id(), trbsr, + read_sysreg_s(SYS_TRBPTR_EL1), + read_sysreg_s(SYS_TRBLIMITR_EL1)); + return TRBE_FAULT_ACT_FATAL; } static unsigned long trbe_get_trace_size(struct perf_output_handle *handle,diff --git a/drivers/hwtracing/coresight/coresight-trbe.h b/drivers/hwtracing/coresight/coresight-trbe.h index 45202c48accec7c86ba56130e2737bc2d1830fae..d7f7cd763c0c7139cf322b7336ee563073e3bea0 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.h +++ b/drivers/hwtracing/coresight/coresight-trbe.h@@ -35,9 +35,11 @@ static inline bool is_trbe_enabled(void) return trblimitr & TRBLIMITR_EL1_E; } -#define TRBE_EC_OTHERS 0 -#define TRBE_EC_STAGE1_ABORT 36 -#define TRBE_EC_STAGE2_ABORT 37 +#define TRBE_EC_OTHERS 0x0 +#define TRBE_EC_GP_CHECK_FAULT 0X1e +#define TRBE_EC_BUF_MGMT_IMPL 0x1f +#define TRBE_EC_STAGE1_ABORT 0x24 +#define TRBE_EC_STAGE2_ABORT 0x25 static inline int get_trbe_ec(u64 trbsr) {