On Tue, Sep 21, 2021 at 02:41:13PM +0100, Suzuki K Poulose wrote:
ARM Neoverse-N2 (#2139208) and Cortex-A710(##2119858) suffers from
an erratum, which when triggered, might cause the TRBE to overwrite
the trace data already collected in FILL mode, in the event of a WRAP.
i.e, the TRBE doesn't stop writing the data, instead wraps to the base
and could write upto 3 cache line size worth trace. Thus, this could
corrupt the trace at the "BASE" pointer.
The workaround is to program the write pointer 256bytes from the
base, such that if the erratum is triggered, it doesn't overwrite
the trace data that was captured. This skipped region could be
padded with ignore packets at the end of the session, so that
the decoder sees a continuous buffer with some padding at the
beginning. The trace data written at the base is considered
lost as the limit could have been in the middle of the perf
ring buffer, and jumping to the "base" is not acceptable.
We set the flags already to indicate that some amount of trace
was lost during the FILL event IRQ. So this is fine.
One important change with the work around is, we program the
TRBBASER_EL1 to current page where we are allowed to write.
Otherwise, it could overwrite a region that may be consumed
by the perf. Towards this, we always make sure that the
"handle->head" and thus the trbe_write is PAGE_SIZE aligned,
so that we can set the BASE to the PAGE base and move the
TRBPTR to the 256bytes offset.
Cc: Mike Leach <mike.leach@xxxxxxxxxx>
Cc: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
Cc: Anshuman Khandual <anshuman.khandual@xxxxxxx>
Cc: Leo Yan <leo.yan@xxxxxxxxxx>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
---
Change since v1:
- Updated comment with ASCII art
- Add _BYTES suffix for the space to skip for the work around.
---
drivers/hwtracing/coresight/coresight-trbe.c | 144 +++++++++++++++++--
1 file changed, 132 insertions(+), 12 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index f569010c672b..983dd5039e52 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -16,6 +16,7 @@
#define pr_fmt(fmt) DRVNAME ": " fmt
#include <asm/barrier.h>
+#include <asm/cpufeature.h>
#include <asm/cputype.h>
#include "coresight-self-hosted-trace.h"
@@ -84,9 +85,17 @@ struct trbe_buf {
* per TRBE instance, we keep track of the list of errata that
* affects the given instance of the TRBE.
*/
-#define TRBE_ERRATA_MAX 0
+#define TRBE_WORKAROUND_OVERWRITE_FILL_MODE 0
+#define TRBE_ERRATA_MAX 1
+
+/*
+ * Safe limit for the number of bytes that may be overwritten
+ * when the erratum is triggered.
+ */
+#define TRBE_WORKAROUND_OVERWRITE_FILL_MODE_SKIP_BYTES 256
static unsigned long trbe_errata_cpucaps[TRBE_ERRATA_MAX] = {
+ [TRBE_WORKAROUND_OVERWRITE_FILL_MODE] = ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE,
};
/*
@@ -519,10 +528,13 @@ static void trbe_enable_hw(struct trbe_buf *buf)
set_trbe_limit_pointer_enabled(buf->trbe_limit);
}
-static enum trbe_fault_action trbe_get_fault_act(u64 trbsr)
+static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *handle,
+ u64 trbsr)
{
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))
@@ -531,10 +543,16 @@ static enum trbe_fault_action trbe_get_fault_act(u64 trbsr)
if ((ec == TRBE_EC_STAGE1_ABORT) || (ec == TRBE_EC_STAGE2_ABORT))
return TRBE_FAULT_ACT_FATAL;
- if (is_trbe_wrap(trbsr) && (ec == TRBE_EC_OTHERS) && (bsc == TRBE_BSC_FILLED)) {
- if (get_trbe_write_pointer() == get_trbe_base_pointer())
- return TRBE_FAULT_ACT_WRAP;
- }
+ /*
+ * 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_has_erratum(cpudata, TRBE_WORKAROUND_OVERWRITE_FILL_MODE) ||
+ get_trbe_write_pointer() == get_trbe_base_pointer()))
+ return TRBE_FAULT_ACT_WRAP;
+
I'm very perplexed by the trbe_has_erratum() infrastructure... Since this is a
TRBE the code will always run on the CPU it is associated with, and if
I'm correct here we could call this_cpu_has_cap() directly with the same
outcome. I doubt that all divers using the cpucaps subsystem carry a shadow
structure to keep the same information.
Thanks,
Mathieu