Use the existed helpers for checking errata instead of open coded equivalent.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-trbe.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 293715b4ff0eb0abe30f9b477700ca94f81cd4a2..0ddb3db0213cf0014e29decfb79da68b0a351b31 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -723,7 +723,7 @@ static unsigned long trbe_get_trace_size(struct perf_output_handle *handle, * the space we skipped with IGNORE packets. And we are always * guaranteed to have at least a PAGE_SIZE space in the buffer. */ - if (trbe_has_erratum(buf->cpudata, TRBE_WORKAROUND_OVERWRITE_FILL_MODE) && + if (trbe_may_overwrite_in_fill_mode(buf->cpudata) && !WARN_ON(size < overwrite_skip)) __trbe_pad_buf(buf, start_off, overwrite_skip);

@@ -946,7 +946,7 @@ static int trbe_apply_work_around_before_enable(struct trbe_buf *buf) * - At trace collection: * - Pad the 256bytes skipped above again with IGNORE packets. */ - if (trbe_has_erratum(buf->cpudata, TRBE_WORKAROUND_OVERWRITE_FILL_MODE)) { + if (trbe_may_overwrite_in_fill_mode(buf->cpudata)) { if (WARN_ON(!IS_ALIGNED(buf->trbe_write, PAGE_SIZE))) return -EINVAL; buf->trbe_hw_base = buf->trbe_write; @@ -970,7 +970,7 @@ static int trbe_apply_work_around_before_enable(struct trbe_buf *buf) * - Adjust the TRBLIMITR.LIMIT to leave the extra PAGE outside * the TRBE's range (i.e [TRBBASER, TRBLIMITR.LIMI] ). */ - if (trbe_has_erratum(buf->cpudata, TRBE_WORKAROUND_WRITE_OUT_OF_RANGE)) { + if (trbe_may_write_out_of_range(buf->cpudata)) { s64 space = buf->trbe_limit - buf->trbe_write; /* * We must have more than a PAGE_SIZE worth space in the proposed

When trbe_handle_overflow() runs, the buffer has already been disabled and drained, so the duplicate operation is unnecessary. Remove it.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-trbe.c | 1 - 1 file changed, 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 2f44e4a65e0ee2b2c8fdd06a51ab01fc57f44a4e..f5597bd9b5fba9a8f5053d5823b03380fd468b5c 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1077,7 +1077,6 @@ static int trbe_handle_overflow(struct perf_output_handle *handle) * is able to detect this with a disconnected handle * (handle->event = NULL). */ - trbe_drain_and_disable_local(buf->cpudata); *this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL; return -EINVAL; }

If the driver does not clear the status when disabling the trace buffer unit, stale state will carry over to the next enable, though the driver clears it again on enable.

Explicitly clear status after the trace is disabled in the interrupt handling and when a perf session ends. Keep the status for spurious interrupts for continuous tracing.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-trbe.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index e426991e2c2c398a9d3982e9d0f7f542e404cbab..9e565122816949b37b8ff5e4ba04cfbc317c6f25 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -629,7 +629,6 @@ static void trbe_enable_hw(struct trbe_buf *buf) WARN_ON(buf->trbe_hw_base < buf->trbe_base); WARN_ON(buf->trbe_write < buf->trbe_hw_base); WARN_ON(buf->trbe_write >= buf->trbe_limit); - clr_trbe_status(); set_trbe_base_pointer(buf->trbe_hw_base); set_trbe_write_pointer(buf->trbe_write);

@@ -1036,6 +1035,8 @@ static int arm_trbe_disable(struct coresight_device *csdev) return -EINVAL;

trbe_drain_and_disable_local(cpudata); + clr_trbe_status(); + buf->cpudata = NULL; cpudata->buf = NULL; cpudata->mode = CS_MODE_DISABLED; @@ -1151,6 +1152,10 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) return IRQ_NONE;

act = trbe_get_fault_act(handle, status); + + if (act != TRBE_FAULT_ACT_SPURIOUS) + clr_trbe_status(); + switch (act) { case TRBE_FAULT_ACT_WRAP: truncated = !!trbe_handle_overflow(handle);

Rather than spreading AUX flag setting in different functions, use trbe_get_fault_act() as a central place for setting the flag.

Later we will support WRAP mode with continuous trace, so the WRAP event does not necessarily cause the trace discontinuity, change to check the stop status instead.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-trbe.c | 38 +++++++++++++--------------- 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 28e2bfa68074f19ccaa4a737d00af577aea818fe..b06885a08e082fd34f68d9588518807b5c47c86e 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -265,25 +265,6 @@ static void trbe_reset_local(struct trbe_cpudata *cpudata) write_sysreg_s(0, SYS_TRBSR_EL1); }

-static void trbe_report_wrap_event(struct perf_output_handle *handle) -{ - /* - * Mark the buffer to indicate that there was a WRAP event by - * setting the COLLISION flag. This indicates to the user that - * the TRBE trace collection was stopped without stopping the - * ETE and thus there might be some amount of trace that was - * lost between the time the WRAP was detected and the IRQ - * was consumed by the CPU. - * - * Setting the TRUNCATED flag would move the event to STOPPED - * state unnecessarily, even when there is space left in the - * ring buffer. Using the COLLISION flag doesn't have this side - * effect. We only set TRUNCATED flag when there is no space - * left in the ring buffer. - */ - perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION); -} - static void trbe_truncate_event(struct perf_output_handle *handle) { struct trbe_buf *buf = etm_perf_sink_config(handle); @@ -687,6 +668,23 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand goto out_fatal; }

+ /* + * Mark the buffer to indicate that there was a WRAP event by + * setting the COLLISION flag. This indicates to the user that + * the TRBE trace collection was stopped without stopping the + * ETE and thus there might be some amount of trace that was + * lost between the time the WRAP was detected and the IRQ + * was consumed by the CPU. + * + * Setting the TRUNCATED flag would move the event to STOPPED + * state unnecessarily, even when there is space left in the + * ring buffer. Using the COLLISION flag doesn't have this side + * effect. We only set TRUNCATED flag when there is no space + * left in the ring buffer. + */ + if (!is_trbe_running(trbsr)) + perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION); + if (is_trbe_wrap(trbsr)) return TRBE_FAULT_ACT_WRAP;

@@ -878,7 +876,6 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev, goto done; }

- trbe_report_wrap_event(handle); wrap = true; }

@@ -1099,7 +1096,6 @@ static int trbe_handle_overflow(struct perf_output_handle *handle) if (buf->snapshot) handle->head += size;

- trbe_report_wrap_event(handle); perf_aux_output_end(handle, size); event_data = perf_aux_output_begin(handle, event); if (!event_data) {

Add a new argument to trbe_handle_overflow() for the fault action, which is used to compare the wrap event for trace size calculation.

No functional change intended; this is preparation for a later update.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-trbe.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 0caa4a6b437a3aa39fc6bcc72a23711b54f7c598..f56ecdeaa6596afb440e4d53732e08a85f9bf89d 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1078,14 +1078,15 @@ static void trbe_handle_spurious(struct perf_output_handle *handle) set_trbe_enabled(buf->cpudata, trblimitr); }

-static int trbe_handle_overflow(struct perf_output_handle *handle) +static int trbe_handle_overflow(struct perf_output_handle *handle, + enum trbe_fault_action act) { struct perf_event *event = handle->event; struct trbe_buf *buf = etm_perf_sink_config(handle); unsigned long size; struct etm_event_data *event_data;

- size = trbe_get_trace_size(handle, buf, true); + size = trbe_get_trace_size(handle, buf, act == TRBE_FAULT_ACT_WRAP); if (buf->snapshot) handle->head += size;

@@ -1179,7 +1180,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)

switch (act) { case TRBE_FAULT_ACT_WRAP: - truncated = !!trbe_handle_overflow(handle); + truncated = !!trbe_handle_overflow(handle, act); break; case TRBE_FAULT_ACT_SPURIOUS: trbe_handle_spurious(handle);

The overwrite erratum occurs only on wrap events, so apply the extra wrap condition check in the workaround.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-trbe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index e579ea98523c24d23a0cd265dcdd0a46b52b52da..2600af12a8fb94bb8c74efda2a101aacd01b0b34 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -714,7 +714,7 @@ static unsigned long trbe_get_trace_size(struct perf_output_handle *handle, * 64bytes. Thus we ignore the potential triggering of the erratum * on WRAP and limit the data to LIMIT. */ - if (wrap) + if (wrap && trbe_may_overwrite_in_fill_mode(buf->cpudata)) write = get_trbe_limit_pointer(); else write = get_trbe_write_pointer(); @@ -736,7 +736,7 @@ static unsigned long trbe_get_trace_size(struct perf_output_handle *handle, * the space we skipped with IGNORE packets. And we are always * guaranteed to have at least a PAGE_SIZE space in the buffer. */ - if (trbe_may_overwrite_in_fill_mode(buf->cpudata) && + if (wrap && trbe_may_overwrite_in_fill_mode(buf->cpudata) && !WARN_ON(size < overwrite_skip)) __trbe_pad_buf(buf, start_off, overwrite_skip);

Since the rounded-up wakeup address is always higher than the head, the limit cannot be less than the head caused by wakeup capping.

The described scenario is never valid caused by wakeup capping, remove the comment to avoid confusion.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-trbe.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 48bc03bd339908b5eac9466dc60325ff1b238976..206eaf103cd94f36220cb6bddd1a78012f5de35a 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -488,10 +488,10 @@ static unsigned long __trbe_normal_offset(struct perf_output_handle *handle) limit = min(limit, round_up(wakeup, PAGE_SIZE));

/* - * There are two situation when this can happen i.e limit is before + * There is a situation when this can happen i.e limit is before * the head and hence TRBE cannot be configured. * - * 1) head < tail (aligned down with PAGE_SIZE) and also they are both + * head < tail (aligned down with PAGE_SIZE) and also they are both * within the same PAGE size range. * * PAGE_SIZE @@ -502,18 +502,6 @@ static unsigned long __trbe_normal_offset(struct perf_output_handle *handle) * |$$$$$$$$$$$$$$$$$$$|========|$$$$$$$| * +------------|------|--------|-------+ * trbe_base trbe_base + nr_pages - * - * 2) head < wakeup (aligned up with PAGE_SIZE) < tail and also both - * head and wakeup are within same PAGE size range. - * - * PAGE_SIZE - * |----------------------| - * - * limit head wakeup tail - * +----|------|-------|--------|-------+ - * |$$$$$$$$$$$|=======|========|$$$$$$$| - * +----|------|-------|--------|-------+ - * trbe_base trbe_base + nr_pages */ if (limit > head) return limit;

A snapshot session captures trace data only when userspace receives a signal and disables tracing. Before disabling, enable circle mode instead of fill mode to avoid unnecessary interrupt handling.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-trbe.c | 7 +++++++ 1 file changed, 7 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 0551ea9b4f8286c156e3c9c7ac94e2ecd3b9dc3f..ee9993d518d2a41f0d709b7d0690b2dfe0bef2d9 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -635,6 +635,12 @@ static void set_trbe_limit_pointer_enabled(struct trbe_buf *buf) trblimitr &= ~TRBLIMITR_EL1_TM_MASK; trblimitr &= ~TRBLIMITR_EL1_LIMIT_MASK;

+ if (buf->snapshot) { + trblimitr |= FIELD_PREP(TRBLIMITR_EL1_FM_MASK, TRBLIMITR_EL1_FM_CBUF) | + FIELD_PREP(TRBLIMITR_EL1_TM_MASK, TRBLIMITR_EL1_TM_IGNR); + goto enable_trace_buf; + } + if (!buf->trbe_count || buf->trbe_write + buf->trbe_count == buf->trbe_limit) { /* @@ -669,6 +675,7 @@ static void set_trbe_limit_pointer_enabled(struct trbe_buf *buf) FIELD_PREP(TRBLIMITR_EL1_TM_MASK, TRBLIMITR_EL1_TM_STOP); }

+enable_trace_buf: trblimitr |= (addr & PAGE_MASK); set_trbe_enabled(buf->cpudata, trblimitr); }

Add tests to verify the calculation of the limit and trigger count.

Because trigger mode can be disabled, provide two test suites: one with trigger mode enabled and one with it disabled.

The cpudata structure is initialized by the test stub, so move its definition into the header for including.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/Kconfig | 9 + drivers/hwtracing/coresight/Makefile | 1 + .../coresight/coresight-trbe-kunit-tests.c | 536 +++++++++++++++++++++ drivers/hwtracing/coresight/coresight-trbe.c | 112 +---- drivers/hwtracing/coresight/coresight-trbe.h | 89 ++++ 5 files changed, 660 insertions(+), 87 deletions(-)

diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index 6a4239ebb582e95f0ebe8e9c8738a726f27f60a1..f5758563c0090141cdca67f16e7b7b32e7c75bb8 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -214,6 +214,15 @@ config CORESIGHT_TRBE To compile this driver as a module, choose M here: the module will be called coresight-trbe.

+config CORESIGHT_TRBE_KUNIT_TESTS + tristate "Enable Coresight TRBE unit tests" + depends on KUNIT + depends on CORESIGHT_TRBE + default KUNIT_ALL_TESTS + help + Enable Coresight TRBE unit tests. Only useful for development and not + intended for production. + config ULTRASOC_SMB tristate "Ultrasoc system memory buffer drivers" depends on ACPI || COMPILE_TEST diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index ab16d06783a572ea1308dfb3a30c96df9e5ffdb7..f8961f6883d167bc2c4bca8008eceb08c3c3a0e9 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -57,3 +57,4 @@ obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o obj-$(CONFIG_CORESIGHT_CTCU) += coresight-ctcu.o coresight-ctcu-y := coresight-ctcu-core.o obj-$(CONFIG_CORESIGHT_KUNIT_TESTS) += coresight-kunit-tests.o +obj-$(CONFIG_CORESIGHT_TRBE_KUNIT_TESTS) += coresight-trbe-kunit-tests.o diff --git a/drivers/hwtracing/coresight/coresight-trbe-kunit-tests.c b/drivers/hwtracing/coresight/coresight-trbe-kunit-tests.c new file mode 100644 index 0000000000000000000000000000000000000000..836f76dce155d533f9076e85dc97ba25221b7bbf --- /dev/null +++ b/drivers/hwtracing/coresight/coresight-trbe-kunit-tests.c @@ -0,0 +1,536 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <kunit/test.h> +#include <kunit/device.h> +#include <linux/coresight.h> + +#include "coresight-priv.h" +#include "coresight-trbe.h" + +MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING"); + +static void test_compute_offset(struct kunit *test) +{ + struct perf_output_handle handle = { 0 }; + struct trbe_buf buf = { 0 }; + struct trbe_cpudata cpudata = { .trbe_align = PAGE_SIZE }; + unsigned long limit; + + if (!static_branch_unlikely(&trbe_trigger_mode_bypass)) + return; + + cpudata.trbe_hw_align = 1; + + buf.nr_pages = SZ_1M / SZ_4K; + buf.cpudata = &cpudata; + + handle.rb = (void *)&buf; + + /* + * ### : Free space, $$$ : Filled space + * + * |################|################| + * `head `wakeup + * `tail `limit + */ + handle.head = 0; + handle.size = SZ_1M; + handle.wakeup = SZ_1M / 2; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, SZ_1M / 2); + + /* + * |################|################| + * `head `wakeup `tail + * `limit + */ + handle.head = 0; + handle.size = SZ_1M - 1; + handle.wakeup = SZ_1M / 2; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, SZ_1M / 2); + + /* + * |#################################| + * `head `tail + * `wakeup `limit + */ + handle.head = 0; + handle.size = SZ_1M - 1; + handle.wakeup = 0; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, 0); + + /* + * |#################################| + * `head `tail + * `wakeup + * `limit + */ + handle.head = 0; + handle.size = SZ_1M - 1; + handle.wakeup = SZ_1M; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, SZ_1M - SZ_4K); + + /* + * |$$$$$$$$$$$$$$$$|########|#######| + * `head `tail + * `wakeup + * `limit + */ + handle.head = SZ_1M / 2; + handle.size = SZ_1M / 2 - 1; + handle.wakeup = SZ_1M * 3 / 4; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, SZ_1M * 3 / 4); + + /* + * |$$$$$$$$|$$$$$$$|################| + * `head `tail + * `wakeup + * `limit + */ + handle.head = SZ_1M / 2; + handle.size = SZ_1M / 2 - 1; + handle.wakeup = SZ_1M * 1 / 4; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, SZ_1M - SZ_4K); + + /* + * |$$$$$$$$$$$$$$$$|################| + * `head `tail + * `wakeup + * `limit + */ + handle.head = SZ_1M / 2; + handle.size = SZ_1M / 2 - 1; + handle.wakeup = SZ_1M - 1; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, SZ_1M - SZ_4K); + + /* + * |#########|$$$$$$$$$$|########|###| + * `tail `head `wakeup + * `limit + */ + handle.head = SZ_1M * 3 / 4; + handle.size = SZ_1M / 2; + handle.wakeup = handle.head + SZ_1M / 8; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, SZ_1M * 7 / 8); + + /* + * |####|####|$$$$$$$$$$|############| + * `tail `head + * `wakeup + * `limit + */ + handle.head = SZ_1M * 3 / 4; + handle.size = SZ_1M / 2; + handle.wakeup = SZ_1M + SZ_1M / 8; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, SZ_1M); + + /* + * |#######|########|$$$$$$$$$$$$$$$$| + * `head `wakeup `>tail + * `limit + */ + handle.head = SZ_1M; + handle.wakeup = SZ_1M + SZ_1M / 8; + handle.size = SZ_1M / 2; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, SZ_1M / 8); + + /* + * |#######|$$$$$$$$$$$$$$$$$|#######| + * `tail `head + * `wakeup + * `limit + */ + handle.head = SZ_1M * 3 / 4; + handle.size = SZ_1M / 2; + handle.wakeup = SZ_1M + SZ_1M / 4; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, SZ_1M); + + /* + * |#######|$$$$$$$$|$$$$$$$$|#######| + * `tail `wakeup `head + * `limit + */ + handle.head = SZ_1M * 3 / 4; + handle.size = SZ_1M / 2; + handle.wakeup = SZ_1M + SZ_1M / 2; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, SZ_1M); + + /* + * |$$$$$$$|########|########|$$$$$$$| + * `head `wakeup `tail + * `limit + */ + handle.head = SZ_1M / 4; + handle.size = SZ_1M / 2; + handle.wakeup = SZ_1M / 2; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, SZ_1M / 2); + + /* + * |$$$$$$$|#################|$$$$$$$| + * `head `tail + * `wakeup + * `limit + */ + handle.head = SZ_1M / 4; + handle.size = SZ_1M / 2; + handle.wakeup = SZ_1M * 3 / 4; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, SZ_1M * 3 / 4); + + /* + * |$$$$$$$|#################|$$$$$$$| + * `wakeup `head `tail + * `limit + */ + handle.head = SZ_1M / 4; + handle.size = SZ_1M / 2; + handle.wakeup = 0; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, SZ_1M * 3 / 4); + + /* + * |$$$$$$|$$$$$$$$$$$$$$$$$$$$$$$$$$| + * `head + * `tail + */ + handle.head = SZ_1M / 4; + handle.size = 0; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, 0); + + /* + * |$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$|#$| + * `head + * `tail + */ + handle.head = SZ_1M - SZ_1K * 2; + handle.size = SZ_1K; + handle.wakeup = 0; + + limit = __trbe_normal_offset(&handle); + KUNIT_ASSERT_EQ(test, limit, 0); +} + +static void test_compute_offset_and_counter(struct kunit *test) +{ + struct perf_output_handle handle = { 0 }; + struct trbe_buf buf = { 0 }; + struct trbe_cpudata cpudata = { .trbe_align = PAGE_SIZE }; + unsigned long limit; + u64 count; + + if (static_branch_unlikely(&trbe_trigger_mode_bypass)) + return; + + cpudata.trbe_hw_align = 1; + + buf.nr_pages = SZ_1M / SZ_4K; + buf.cpudata = &cpudata; + + handle.rb = (void *)&buf; + + /* + * ### : Free space, $$$ : Filled space + * + * |################|################| + * `head `wakeup `limit + * `tail + * `----- count ----' + */ + handle.head = 0; + handle.size = SZ_1M; + handle.wakeup = SZ_1M / 2; + + limit = __trbe_normal_offset(&handle); + buf.trbe_limit = limit; + count = __trbe_normal_trigger_count(&handle); + + KUNIT_ASSERT_EQ(test, limit, SZ_1M); + KUNIT_ASSERT_EQ(test, count, SZ_1M / 2); + + /* + * |################|################| + * `head `wakeup `tail + * `limit + * `----- count ----' + */ + handle.head = 0; + handle.size = SZ_1M - 1; + handle.wakeup = SZ_1M / 2; + + limit = __trbe_normal_offset(&handle); + buf.trbe_limit = limit; + count = __trbe_normal_trigger_count(&handle); + + KUNIT_ASSERT_EQ(test, limit, SZ_1M - SZ_4K); + KUNIT_ASSERT_EQ(test, count, SZ_1M / 2); + + /* + * |#################################| + * `head `tail + * `wakeup `limit + */ + handle.head = 0; + handle.size = SZ_1M - 1; + handle.wakeup = 0; + + limit = __trbe_normal_offset(&handle); + buf.trbe_limit = limit; + count = __trbe_normal_trigger_count(&handle); + + KUNIT_ASSERT_EQ(test, limit, SZ_1M - SZ_4K); + KUNIT_ASSERT_EQ(test, count, 0); + + /* + * |#################################| + * `head `tail + * `wakeup + * `limit + */ + handle.head = 0; + handle.size = SZ_1M - 1; + handle.wakeup = SZ_1M; + + limit = __trbe_normal_offset(&handle); + buf.trbe_limit = limit; + count = __trbe_normal_trigger_count(&handle); + + KUNIT_ASSERT_EQ(test, limit, SZ_1M - SZ_4K); + KUNIT_ASSERT_EQ(test, count, 0); + + /* + * |$$$$$$$$$$$$$$$$|########|#######| + * `head `tail + * `wakeup + * `limit + * [ count ] + */ + handle.head = SZ_1M / 2; + handle.size = SZ_1M / 2 - 1; + handle.wakeup = SZ_1M * 3 / 4; + + limit = __trbe_normal_offset(&handle); + buf.trbe_limit = limit; + count = __trbe_normal_trigger_count(&handle); + + KUNIT_ASSERT_EQ(test, limit, SZ_1M - SZ_4K); + KUNIT_ASSERT_EQ(test, count, SZ_1M / 4); + + /* + * |$$$$$$$$|$$$$$$$|################| + * `head `tail + * `wakeup + * `limit + */ + handle.head = SZ_1M / 2; + handle.size = SZ_1M / 2 - 1; + handle.wakeup = SZ_1M * 1 / 4; + + limit = __trbe_normal_offset(&handle); + buf.trbe_limit = limit; + count = __trbe_normal_trigger_count(&handle); + + KUNIT_ASSERT_EQ(test, limit, SZ_1M - SZ_4K); + KUNIT_ASSERT_EQ(test, count, 0); + + /* + * |$$$$$$$$$$$$$$$$|################| + * `head `tail + * `wakeup + * `limit + */ + handle.head = SZ_1M / 2; + handle.size = SZ_1M / 2 - 1; + handle.wakeup = SZ_1M - 1; + + limit = __trbe_normal_offset(&handle); + buf.trbe_limit = limit; + count = __trbe_normal_trigger_count(&handle); + + KUNIT_ASSERT_EQ(test, limit, SZ_1M - SZ_4K); + KUNIT_ASSERT_EQ(test, count, 0); + + /* + * |#########|$$$$$$$$$$|########|###| + * `tail `head `wakeup + * `limit + * [ count ] + */ + handle.head = SZ_1M * 3 / 4; + handle.size = SZ_1M / 2; + handle.wakeup = handle.head + SZ_1M / 8; + + limit = __trbe_normal_offset(&handle); + buf.trbe_limit = limit; + count = __trbe_normal_trigger_count(&handle); + + KUNIT_ASSERT_EQ(test, limit, SZ_1M); + KUNIT_ASSERT_EQ(test, count, SZ_1M / 8); + + /* + * |####|####|$$$$$$$$$$|############| + * `tail `head + * `wakeup + * `limit + * [ count >>> + * >>> ] + */ + handle.head = SZ_1M * 3 / 4; + handle.size = SZ_1M / 2; + handle.wakeup = SZ_1M + SZ_1M / 8; + + limit = __trbe_normal_offset(&handle); + buf.trbe_limit = limit; + count = __trbe_normal_trigger_count(&handle); + + KUNIT_ASSERT_EQ(test, limit, SZ_1M); + KUNIT_ASSERT_EQ(test, count, SZ_1M / 2); + + /* + * |#######|########|$$$$$$$$$$$$$$$$| + * `head `wakeup `>tail + * `limit + * [ count ] + */ + handle.head = SZ_1M; + handle.wakeup = SZ_1M + SZ_1M / 8; + handle.size = SZ_1M / 2; + + limit = __trbe_normal_offset(&handle); + buf.trbe_limit = limit; + count = __trbe_normal_trigger_count(&handle); + + KUNIT_ASSERT_EQ(test, limit, SZ_1M / 2); + KUNIT_ASSERT_EQ(test, count, SZ_1M / 8); + + /* + * |#######|$$$$$$$$$$$$$$$$$|#######| + * `tail `head + * `wakeup + * `limit + * [ count > + * >>> ] + */ + handle.head = SZ_1M * 3 / 4; + handle.size = SZ_1M / 2; + handle.wakeup = SZ_1M + SZ_1M / 4; + + limit = __trbe_normal_offset(&handle); + buf.trbe_limit = limit; + count = __trbe_normal_trigger_count(&handle); + + KUNIT_ASSERT_EQ(test, limit, SZ_1M); + KUNIT_ASSERT_EQ(test, count, SZ_1M / 2); + + /* + * |#######|$$$$$$$$|$$$$$$$$|#######| + * `tail `wakeup `head + * `limit + * [ count > + * >>> ] + */ + handle.head = SZ_1M * 3 / 4; + handle.size = SZ_1M / 2; + handle.wakeup = SZ_1M + SZ_1M / 2; + + limit = __trbe_normal_offset(&handle); + buf.trbe_limit = limit; + count = __trbe_normal_trigger_count(&handle); + + KUNIT_ASSERT_EQ(test, limit, SZ_1M); + KUNIT_ASSERT_EQ(test, count, SZ_1M / 2); + + /* + * |$$$$$$$|########|########|$$$$$$$| + * `head `wakeup `tail + * `limit + * [ count ] + */ + handle.head = SZ_1M / 4; + handle.size = SZ_1M / 2; + handle.wakeup = SZ_1M / 2; + + limit = __trbe_normal_offset(&handle); + buf.trbe_limit = limit; + count = __trbe_normal_trigger_count(&handle); + + KUNIT_ASSERT_EQ(test, limit, SZ_1M * 3 / 4); + KUNIT_ASSERT_EQ(test, count, SZ_1M / 4); + + /* + * |$$$$$$$|#################|$$$$$$$| + * `head `tail + * `wakeup + * `limit + */ + handle.head = SZ_1M / 4; + handle.size = SZ_1M / 2; + handle.wakeup = SZ_1M * 3 / 4; + + limit = __trbe_normal_offset(&handle); + buf.trbe_limit = limit; + count = __trbe_normal_trigger_count(&handle); + + KUNIT_ASSERT_EQ(test, limit, SZ_1M * 3 / 4); + KUNIT_ASSERT_EQ(test, count, 0); + + /* + * |$$$$$$$|#################|$$$$$$$| + * `wakeup `head `tail + * `limit + */ + handle.head = SZ_1M / 4; + handle.size = SZ_1M / 2; + handle.wakeup = 0; + + limit = __trbe_normal_offset(&handle); + buf.trbe_limit = limit; + count = __trbe_normal_trigger_count(&handle); + + KUNIT_ASSERT_EQ(test, limit, SZ_1M * 3 / 4); + KUNIT_ASSERT_EQ(test, count, 0); +} + +static struct kunit_case coresight_trbe_testcases[] = { + KUNIT_CASE(test_compute_offset), + KUNIT_CASE(test_compute_offset_and_counter), + {} +}; + +static struct kunit_suite coresight_trbe_test_suite = { + .name = "coresight_trbe_test_suite", + .test_cases = coresight_trbe_testcases, +}; + +kunit_test_suites(&coresight_trbe_test_suite); +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Leo Yan leo.yan@arm.com"); +MODULE_DESCRIPTION("Arm CoreSight TRBE KUnit tests"); diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index ee9993d518d2a41f0d709b7d0690b2dfe0bef2d9..25d42683ab74b55efa2e19a2d77ab8ae2d68d228 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -54,92 +54,12 @@ enum trbe_fault_action { TRBE_FAULT_ACT_FATAL, };

-struct trbe_buf { - /* - * Even though trbe_base represents vmap() - * mapped allocated buffer's start address, - * it's being as unsigned long for various - * arithmetic and comparision operations & - * also to be consistent with trbe_write & - * trbe_limit sibling pointers. - */ - unsigned long trbe_base; - /* The base programmed into the TRBE */ - unsigned long trbe_hw_base; - unsigned long trbe_limit; - unsigned long trbe_write; - unsigned long trbe_count; - int nr_pages; - void **pages; - bool snapshot; - struct trbe_cpudata *cpudata; -}; - -/* - * TRBE erratum list - * - * The errata are defined in arm64 generic cpu_errata framework. - * Since the errata work arounds could be applied individually - * to the affected CPUs inside the TRBE driver, we need to know if - * a given CPU is affected by the erratum. Unlike the other erratum - * work arounds, TRBE driver needs to check multiple times during - * a trace session. Thus we need a quicker access to per-CPU - * errata and not issue costly this_cpu_has_cap() everytime. - * We keep a set of the affected errata in trbe_cpudata, per TRBE. - * - * We rely on the corresponding cpucaps to be defined for a given - * TRBE erratum. We map the given cpucap into a TRBE internal number - * to make the tracking of the errata lean. - * - * This helps in : - * - Not duplicating the detection logic - * - Streamlined detection of erratum across the system - */ -#define TRBE_WORKAROUND_OVERWRITE_FILL_MODE 0 -#define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE 1 -#define TRBE_NEEDS_DRAIN_AFTER_DISABLE 2 -#define TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE 3 -#define TRBE_IS_BROKEN 4 - -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, - [TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE] = ARM64_WORKAROUND_2038923, - [TRBE_IS_BROKEN] = ARM64_WORKAROUND_1902691, - -1, /* Sentinel, must be the last entry */ -}; - -/* The total number of listed errata in trbe_errata_cpucaps */ -#define TRBE_ERRATA_MAX (ARRAY_SIZE(trbe_errata_cpucaps) - 1) - /* * Safe limit for the number of bytes that may be overwritten * when ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE is triggered. */ #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE_SKIP_BYTES 256

-/* - * struct trbe_cpudata: TRBE instance specific data - * @trbe_flag - TRBE dirty/access flag support - * @trbe_hw_align - Actual TRBE alignment required for TRBPTR_EL1. - * @trbe_align - Software alignment used for the TRBPTR_EL1. - * @cpu - CPU this TRBE belongs to. - * @mode - Mode of current operation. (perf/disabled) - * @drvdata - TRBE specific drvdata - * @errata - Bit map for the errata on this TRBE. - */ -struct trbe_cpudata { - bool trbe_flag; - u64 trbe_hw_align; - u64 trbe_align; - int cpu; - enum cs_mode mode; - struct trbe_buf *buf; - struct trbe_drvdata *drvdata; - DECLARE_BITMAP(errata, TRBE_ERRATA_MAX); -}; - struct trbe_drvdata { struct trbe_cpudata __percpu *cpudata; struct perf_output_handle * __percpu *handle; @@ -150,7 +70,8 @@ struct trbe_drvdata { struct platform_device *pdev; };

-DEFINE_STATIC_KEY_FALSE(trbe_trigger_mode_bypass); +VISIBLE_IF_KUNIT DEFINE_STATIC_KEY_FALSE(trbe_trigger_mode_bypass); +EXPORT_SYMBOL_IF_KUNIT(trbe_trigger_mode_bypass);

#define trbe_trigger_mode_need_bypass(cpudata) \ (trbe_may_overwrite_in_fill_mode((cpudata)) || \ @@ -333,8 +254,17 @@ static void __trbe_pad_buf(struct trbe_buf *buf, u64 offset, int len)

static void trbe_pad_buf(struct perf_output_handle *handle, int len) { - struct trbe_buf *buf = etm_perf_sink_config(handle); - u64 head = PERF_IDX2OFF(handle->head, buf); + struct trbe_buf *buf; + u64 head; + + if (kunit_get_current_test()) { + handle->head += len; + handle->size -= len; + return; + } + + buf = etm_perf_sink_config(handle); + head = PERF_IDX2OFF(handle->head, buf);

__trbe_pad_buf(buf, head, len); if (!buf->snapshot) @@ -383,9 +313,11 @@ static u64 trbe_min_trace_buf_size(struct perf_output_handle *handle) * %%%% - Free area, disabled, trace will not be written * ==== - Free area, padded with ETE_IGNORE_PACKET, trace will be skipped */ -static unsigned long __trbe_normal_offset(struct perf_output_handle *handle) +VISIBLE_IF_KUNIT +unsigned long __trbe_normal_offset(struct perf_output_handle *handle) { - struct trbe_buf *buf = etm_perf_sink_config(handle); + struct trbe_buf *buf = + kunit_get_current_test() ? handle->rb : etm_perf_sink_config(handle); struct trbe_cpudata *cpudata = buf->cpudata; const u64 bufsize = buf->nr_pages * PAGE_SIZE; u64 limit = bufsize; @@ -525,9 +457,13 @@ static unsigned long __trbe_normal_offset(struct perf_output_handle *handle) return 0; }

-static u64 __trbe_normal_trigger_count(struct perf_output_handle *handle) +EXPORT_SYMBOL_IF_KUNIT(__trbe_normal_offset); + +VISIBLE_IF_KUNIT +u64 __trbe_normal_trigger_count(struct perf_output_handle *handle) { - struct trbe_buf *buf = etm_perf_sink_config(handle); + struct trbe_buf *buf = + kunit_get_current_test() ? handle->rb : etm_perf_sink_config(handle); struct trbe_cpudata *cpudata = buf->cpudata; u64 limit, head, wakeup; u64 count = 0; @@ -558,6 +494,8 @@ static u64 __trbe_normal_trigger_count(struct perf_output_handle *handle) return count; }

+EXPORT_SYMBOL_IF_KUNIT(__trbe_normal_trigger_count); + static int trbe_normal_offset(struct perf_output_handle *handle) { struct trbe_buf *buf = etm_perf_sink_config(handle); diff --git a/drivers/hwtracing/coresight/coresight-trbe.h b/drivers/hwtracing/coresight/coresight-trbe.h index 4c65d164a946ec9860825e7564196745b60d730b..8f90836b5f71d44213699ec1915d59864863a4db 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.h +++ b/drivers/hwtracing/coresight/coresight-trbe.h @@ -17,8 +17,91 @@ #include <linux/platform_device.h> #include <linux/smp.h>

+#include <kunit/test-bug.h> +#include <kunit/visibility.h> + #include "coresight-etm-perf.h"

+struct trbe_buf { + /* + * Even though trbe_base represents vmap() + * mapped allocated buffer's start address, + * it's being as unsigned long for various + * arithmetic and comparision operations & + * also to be consistent with trbe_write & + * trbe_limit sibling pointers. + */ + unsigned long trbe_base; + /* The base programmed into the TRBE */ + unsigned long trbe_hw_base; + unsigned long trbe_limit; + unsigned long trbe_write; + unsigned long trbe_count; + int nr_pages; + void **pages; + bool snapshot; + struct trbe_cpudata *cpudata; +}; + +/* + * TRBE erratum list + * + * The errata are defined in arm64 generic cpu_errata framework. + * Since the errata work arounds could be applied individually + * to the affected CPUs inside the TRBE driver, we need to know if + * a given CPU is affected by the erratum. Unlike the other erratum + * work arounds, TRBE driver needs to check multiple times during + * a trace session. Thus we need a quicker access to per-CPU + * errata and not issue costly this_cpu_has_cap() everytime. + * We keep a set of the affected errata in trbe_cpudata, per TRBE. + * + * We rely on the corresponding cpucaps to be defined for a given + * TRBE erratum. We map the given cpucap into a TRBE internal number + * to make the tracking of the errata lean. + * + * This helps in : + * - Not duplicating the detection logic + * - Streamlined detection of erratum across the system + */ +#define TRBE_WORKAROUND_OVERWRITE_FILL_MODE 0 +#define TRBE_WORKAROUND_WRITE_OUT_OF_RANGE 1 +#define TRBE_NEEDS_DRAIN_AFTER_DISABLE 2 +#define TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE 3 +#define TRBE_IS_BROKEN 4 + +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, + [TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE] = ARM64_WORKAROUND_2038923, + [TRBE_IS_BROKEN] = ARM64_WORKAROUND_1902691, + -1, /* Sentinel, must be the last entry */ +}; + +/* The total number of listed errata in trbe_errata_cpucaps */ +#define TRBE_ERRATA_MAX (ARRAY_SIZE(trbe_errata_cpucaps) - 1) + +/* + * struct trbe_cpudata: TRBE instance specific data + * @trbe_flag - TRBE dirty/access flag support + * @trbe_hw_align - Actual TRBE alignment required for TRBPTR_EL1. + * @trbe_align - Software alignment used for the TRBPTR_EL1. + * @cpu - CPU this TRBE belongs to. + * @mode - Mode of current operation. (perf/disabled) + * @drvdata - TRBE specific drvdata + * @errata - Bit map for the errata on this TRBE. + */ +struct trbe_cpudata { + bool trbe_flag; + u64 trbe_hw_align; + u64 trbe_align; + int cpu; + enum cs_mode mode; + struct trbe_buf *buf; + struct trbe_drvdata *drvdata; + DECLARE_BITMAP(errata, TRBE_ERRATA_MAX); +}; + static inline bool is_trbe_available(void) { u64 aa64dfr0 = read_sysreg_s(SYS_ID_AA64DFR0_EL1); @@ -153,3 +236,9 @@ static inline void set_trbe_base_pointer(unsigned long addr) WARN_ON(!IS_ALIGNED(addr, PAGE_SIZE)); write_sysreg_s(addr, SYS_TRBBASER_EL1); } + +#if IS_ENABLED(CONFIG_KUNIT) +DECLARE_STATIC_KEY_FALSE(trbe_trigger_mode_bypass); +unsigned long __trbe_normal_offset(struct perf_output_handle *handle); +u64 __trbe_normal_trigger_count(struct perf_output_handle *handle); +#endif

The default watermark is half of the total buffer size. In many cases, the tool can not be notified with sufficient free space, especially when profiling with small AUX buffer (e.g., 64KiB).

Setting watermark to quarter of the buffer to notifies the tool to read data earlier and prevents the data loss.

Signed-off-by: Leo Yan leo.yan@arm.com --- tools/perf/arch/arm/util/cs-etm.c | 7 +++++++ 1 file changed, 7 insertions(+)

diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index ea891d12f8f40beebf8dee1d3db71cad701f5666..649b8b0d0f92b4af45fb97db9da3c5ccf24a978b 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -424,6 +424,13 @@ static int cs_etm_recording_options(struct auxtrace_record *itr, pr_debug2("%s snapshot size: %zu\n", CORESIGHT_ETM_PMU_NAME, opts->auxtrace_snapshot_size);

+ if (!opts->auxtrace_snapshot_mode && !opts->auxtrace_sample_mode) { + size_t aw = opts->auxtrace_mmap_pages * (size_t)page_size / 4; + u32 aux_watermark = aw > UINT_MAX ? UINT_MAX : aw; + + cs_etm_evsel->core.attr.aux_watermark = aux_watermark; + } + /* * To obtain the auxtrace buffer file descriptor, the auxtrace * event must come first.

On 01/12/2025 11:21, Leo Yan wrote:

Rather than spreading AUX flag setting in different functions, use trbe_get_fault_act() as a central place for setting the flag.

Later we will support WRAP mode with continuous trace, so the WRAP event does not necessarily cause the trace discontinuity, change to check the stop status instead.

Signed-off-by: Leo Yan leo.yan@arm.com

drivers/hwtracing/coresight/coresight-trbe.c | 38 +++++++++++++--------------- 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 28e2bfa68074f19ccaa4a737d00af577aea818fe..b06885a08e082fd34f68d9588518807b5c47c86e 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -265,25 +265,6 @@ static void trbe_reset_local(struct trbe_cpudata *cpudata) write_sysreg_s(0, SYS_TRBSR_EL1); } -static void trbe_report_wrap_event(struct perf_output_handle *handle) -{

• /*

• * Mark the buffer to indicate that there was a WRAP event by
  

• * setting the COLLISION flag. This indicates to the user that
  

• * the TRBE trace collection was stopped without stopping the
  

• * ETE and thus there might be some amount of trace that was
  

• * lost between the time the WRAP was detected and the IRQ
  

• * was consumed by the CPU.
  

• *
  

• * Setting the TRUNCATED flag would move the event to STOPPED
  

• * state unnecessarily, even when there is space left in the
  

• * ring buffer. Using the COLLISION flag doesn't have this side
  

• * effect. We only set TRUNCATED flag when there is no space
  

• * left in the ring buffer.
  

• */
  

• perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);

-}

• static void trbe_truncate_event(struct perf_output_handle *handle) { struct trbe_buf *buf = etm_perf_sink_config(handle);

@@ -687,6 +668,23 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand goto out_fatal; }

• /*

• * Mark the buffer to indicate that there was a WRAP event by
  

• * setting the COLLISION flag. This indicates to the user that
  

• * the TRBE trace collection was stopped without stopping the
  

• * ETE and thus there might be some amount of trace that was
  

• * lost between the time the WRAP was detected and the IRQ
  

• * was consumed by the CPU.
  

• *
  

• * Setting the TRUNCATED flag would move the event to STOPPED
  

• * state unnecessarily, even when there is space left in the
  

• * ring buffer. Using the COLLISION flag doesn't have this side
  

• * effect. We only set TRUNCATED flag when there is no space
  

• * left in the ring buffer.
  

• */
  

• if (!is_trbe_running(trbsr))

Is this really required ? There is a WARN_ON(is_trbe_running(trbsr)) above ?

• perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
  

Also, setting this would unnecessarily mark COLLISION while stopping the TRBE buffer normally (when called from arm_trbe_update_buffer), when there is no WRAP event ?

• if (is_trbe_wrap(trbsr)) return TRBE_FAULT_ACT_WRAP;

Couldn't this be :

/* Move the big fat comment here */ if (is_trbe_wrap(trbsr)) { perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION); return TRBE_FAULT_ACT_WRAP; }

Suzuki

On 01/12/2025 11:22, Leo Yan wrote:

The overwrite erratum occurs only on wrap events, so apply the extra wrap condition check in the workaround.

Signed-off-by: Leo Yan leo.yan@arm.com

drivers/hwtracing/coresight/coresight-trbe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index e579ea98523c24d23a0cd265dcdd0a46b52b52da..2600af12a8fb94bb8c74efda2a101aacd01b0b34 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -714,7 +714,7 @@ static unsigned long trbe_get_trace_size(struct perf_output_handle *handle, * 64bytes. Thus we ignore the potential triggering of the erratum * on WRAP and limit the data to LIMIT. */

• if (wrap)

• if (wrap && trbe_may_overwrite_in_fill_mode(buf->cpudata)) write = get_trbe_limit_pointer();

This must be trbe_may_write_out_of_range() ?

Suzuki

else write = get_trbe_write_pointer(); @@ -736,7 +736,7 @@ static unsigned long trbe_get_trace_size(struct perf_output_handle *handle, * the space we skipped with IGNORE packets. And we are always * guaranteed to have at least a PAGE_SIZE space in the buffer. */

• if (trbe_may_overwrite_in_fill_mode(buf->cpudata) &&

• if (wrap && trbe_may_overwrite_in_fill_mode(buf->cpudata) && !WARN_ON(size < overwrite_skip)) __trbe_pad_buf(buf, start_off, overwrite_skip);

On Tue, Dec 02, 2025 at 11:15:47AM +0000, Suzuki K Poulose wrote:

[...]

...

@@ -687,6 +668,23 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand goto out_fatal; }

• /*

• * Mark the buffer to indicate that there was a WRAP event by
  

• * setting the COLLISION flag. This indicates to the user that
  

• * the TRBE trace collection was stopped without stopping the
  

• * ETE and thus there might be some amount of trace that was
  

• * lost between the time the WRAP was detected and the IRQ
  

• * was consumed by the CPU.
  

• *
  

• * Setting the TRUNCATED flag would move the event to STOPPED
  

• * state unnecessarily, even when there is space left in the
  

• * ring buffer. Using the COLLISION flag doesn't have this side
  

• * effect. We only set TRUNCATED flag when there is no space
  

• * left in the ring buffer.
  

• */
  

• if (!is_trbe_running(trbsr))

Is this really required ? There is a WARN_ON(is_trbe_running(trbsr)) above ?

The WARN_ON() will reports warning for running case, here it sets COLLISION flag for not running case, which is not conflict.

This works for the Fill mode (stop collection), we always set the COLLISION flag for it. Once we enable trigger and wrap modes, due to the trace continues to run, we don't set the flag for these modes.

...

• perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
  

Also, setting this would unnecessarily mark COLLISION while stopping the TRBE buffer normally (when called from arm_trbe_update_buffer), when there is no WRAP event ?

Good point. I should refactor arm_trbe_update_buffer(), it firstly reads out TRBSR, stop trace afterwards, and then call trbe_get_fault_act().

In this case, it is in running mode so COLLISION will not be set for normal disable flow.

I will add a new patch for this and place it before this one.

...

• if (is_trbe_wrap(trbsr)) return TRBE_FAULT_ACT_WRAP;

Couldn't this be :

/* Move the big fat comment here */ if (is_trbe_wrap(trbsr)) { perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION); return TRBE_FAULT_ACT_WRAP; }

It deliberately de-couples the COLLISION flag from the WRAP event.

In later change, a WRAP event only means buffer wrap around, it does mean the trace has been stopped. The AUX flag is only set for stopping case.

Thanks, Leo

On Tue, Dec 02, 2025 at 12:05:52PM +0000, Suzuki K Poulose wrote:

On 01/12/2025 11:22, Leo Yan wrote:

...

The overwrite erratum occurs only on wrap events, so apply the extra wrap condition check in the workaround.

Signed-off-by: Leo Yan leo.yan@arm.com

drivers/hwtracing/coresight/coresight-trbe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index e579ea98523c24d23a0cd265dcdd0a46b52b52da..2600af12a8fb94bb8c74efda2a101aacd01b0b34 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -714,7 +714,7 @@ static unsigned long trbe_get_trace_size(struct perf_output_handle *handle, * 64bytes. Thus we ignore the potential triggering of the erratum * on WRAP and limit the data to LIMIT. */

• if (wrap)

• if (wrap && trbe_may_overwrite_in_fill_mode(buf->cpudata)) write = get_trbe_limit_pointer();

This must be trbe_may_write_out_of_range() ?

SDEN (e.g., [1]) is ambiguous about how the write pointer acts for the out-of-range erratum.

However, SDEN explicitly states for overwrite erratum: "The current write pointer also increments by the same number of cache line locations.

If this is still concerned, I can add check trbe_may_write_out_of_range() in next spin.

Thanks, Leo

[1] https://documentation-service.arm.com/static/636a660b4e6cf12278ad89c4?token=

On 01/12/25 4:51 PM, Leo Yan wrote:

Use the existed helpers for checking errata instead of open coded

Small nit - s/the existed helpers/the existing helpers/

equivalent.> Signed-off-by: Leo Yan leo.yan@arm.com

---

drivers/hwtracing/coresight/coresight-trbe.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 293715b4ff0eb0abe30f9b477700ca94f81cd4a2..0ddb3db0213cf0014e29decfb79da68b0a351b31 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -723,7 +723,7 @@ static unsigned long trbe_get_trace_size(struct perf_output_handle *handle, * the space we skipped with IGNORE packets. And we are always * guaranteed to have at least a PAGE_SIZE space in the buffer. */

• if (trbe_has_erratum(buf->cpudata, TRBE_WORKAROUND_OVERWRITE_FILL_MODE) &&

• if (trbe_may_overwrite_in_fill_mode(buf->cpudata) && !WARN_ON(size < overwrite_skip)) __trbe_pad_buf(buf, start_off, overwrite_skip);

@@ -946,7 +946,7 @@ static int trbe_apply_work_around_before_enable(struct trbe_buf *buf) * - At trace collection: * - Pad the 256bytes skipped above again with IGNORE packets. */

• if (trbe_has_erratum(buf->cpudata, TRBE_WORKAROUND_OVERWRITE_FILL_MODE)) {

• if (trbe_may_overwrite_in_fill_mode(buf->cpudata)) { if (WARN_ON(!IS_ALIGNED(buf->trbe_write, PAGE_SIZE))) return -EINVAL; buf->trbe_hw_base = buf->trbe_write;

@@ -970,7 +970,7 @@ static int trbe_apply_work_around_before_enable(struct trbe_buf *buf) * - Adjust the TRBLIMITR.LIMIT to leave the extra PAGE outside * the TRBE's range (i.e [TRBBASER, TRBLIMITR.LIMI] ). */

• if (trbe_has_erratum(buf->cpudata, TRBE_WORKAROUND_WRITE_OUT_OF_RANGE)) {

• if (trbe_may_write_out_of_range(buf->cpudata)) { s64 space = buf->trbe_limit - buf->trbe_write; /*
  • We must have more than a PAGE_SIZE worth space in the proposed

LGTM

Reviewed-by: Anshuman Khandual anshuman.khandual@arm.com

On 01/12/25 4:51 PM, Leo Yan wrote:

When trbe_handle_overflow() runs, the buffer has already been disabled and drained, so the duplicate operation is unnecessary. Remove it.

Signed-off-by: Leo Yan leo.yan@arm.com

drivers/hwtracing/coresight/coresight-trbe.c | 1 - 1 file changed, 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 2f44e4a65e0ee2b2c8fdd06a51ab01fc57f44a4e..f5597bd9b5fba9a8f5053d5823b03380fd468b5c 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1077,7 +1077,6 @@ static int trbe_handle_overflow(struct perf_output_handle *handle) * is able to detect this with a disconnected handle * (handle->event = NULL). */

• trbe_drain_and_disable_local(buf->cpudata);
  

  *this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL; return -EINVAL; }

Like the previous patch - trbe_drain_and_disable_local() would have run if TRBE_FAULT_ACT_WRAP case is being handled via trbe_handle_overflow(). But please do mention these details in the commit message for things to be clear and explicit.

Reviewed-by: Anshuman Khandual anshuman.khandual@arm.com

On 01/12/25 4:51 PM, Leo Yan wrote:

If the driver does not clear the status when disabling the trace buffer unit, stale state will carry over to the next enable, though the driver clears it again on enable.

There is no problem now ! Because trbe_enable_hw() calls clr_trbe_status().

Explicitly clear status after the trace is disabled in the interrupt handling and when a perf session ends. Keep the status for spurious interrupts for continuous tracing.

But is not that the behaviour already without this change ?

clr_trbe_status() in trbe_enable_hw() ensures that no TRBE session can be started without first clearing the existing status. Still wondering what is the purpose of this change ?

Signed-off-by: Leo Yan leo.yan@arm.com

drivers/hwtracing/coresight/coresight-trbe.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index e426991e2c2c398a9d3982e9d0f7f542e404cbab..9e565122816949b37b8ff5e4ba04cfbc317c6f25 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -629,7 +629,6 @@ static void trbe_enable_hw(struct trbe_buf *buf) WARN_ON(buf->trbe_hw_base < buf->trbe_base); WARN_ON(buf->trbe_write < buf->trbe_hw_base); WARN_ON(buf->trbe_write >= buf->trbe_limit);

• clr_trbe_status(); set_trbe_base_pointer(buf->trbe_hw_base); set_trbe_write_pointer(buf->trbe_write);

@@ -1036,6 +1035,8 @@ static int arm_trbe_disable(struct coresight_device *csdev) return -EINVAL; trbe_drain_and_disable_local(cpudata);

• clr_trbe_status();
• buf->cpudata = NULL; cpudata->buf = NULL; cpudata->mode = CS_MODE_DISABLED;

@@ -1151,6 +1152,10 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) return IRQ_NONE; act = trbe_get_fault_act(handle, status);

• if (act != TRBE_FAULT_ACT_SPURIOUS)

• clr_trbe_status();
  

• switch (act) { case TRBE_FAULT_ACT_WRAP: truncated = !!trbe_handle_overflow(handle);

On 01/12/25 4:51 PM, Leo Yan wrote:

It gives priority to TRBSR_EL1.EA (external abort); an external abort will immediately bail out and return an error.

This is an abrupt starting for a commit message without giving context. The rationale for the change needs to be explained rather than details of implementation.

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.

No additional checks are required for wrap mode beyond verifying the TRBSR_EL1.WRAP bit, even on units with overwrite errata, as this bit reliably indicates a buffer wrap.

Should the errata related changes be done in a separate patch instead ?

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;
  

No message for this ?

• case TRBE_EC_BUF_MGMT_IMPL:

• err_str = "Unexpected implemented management";
  

• 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;
  

• }

Just wondering if it would be cleaner to add a const char * based static array mapping these EC/BSC codes with above error messages.

• if (is_trbe_wrap(trbsr))

But what about TRBE affected with trbe_may_overwrite_in_fill_mode() is that still being taken care of some how ?

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));
  

Right - comprehensive TRBE configuration register dump when a fatal error happens.

• 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

Please do mention the document source for these new EC codes in the commit message.

static inline int get_trbe_ec(u64 trbsr) {

On 01/12/25 4:52 PM, Leo Yan wrote:

The default watermark is half of the total buffer size. In many cases, the tool can not be notified with sufficient free space, especially when profiling with small AUX buffer (e.g., 64KiB).

Could not smaller AUX buffer be prevented for TRBE instead ?

...

Setting watermark to quarter of the buffer to notifies the tool to read

data earlier and prevents the data loss.

Signed-off-by: Leo Yan leo.yan@arm.com

tools/perf/arch/arm/util/cs-etm.c | 7 +++++++ 1 file changed, 7 insertions(+)

diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index ea891d12f8f40beebf8dee1d3db71cad701f5666..649b8b0d0f92b4af45fb97db9da3c5ccf24a978b 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -424,6 +424,13 @@ static int cs_etm_recording_options(struct auxtrace_record *itr, pr_debug2("%s snapshot size: %zu\n", CORESIGHT_ETM_PMU_NAME, opts->auxtrace_snapshot_size);

• if (!opts->auxtrace_snapshot_mode && !opts->auxtrace_sample_mode) {

• size_t aw = opts->auxtrace_mmap_pages * (size_t)page_size / 4;
  

• u32 aux_watermark = aw > UINT_MAX ? UINT_MAX : aw;
  

• cs_etm_evsel->core.attr.aux_watermark = aux_watermark;
  

• }
• /*
  • To obtain the auxtrace buffer file descriptor, the auxtrace
  • event must come first.

On 02/12/2025 11:06 am, Suzuki K Poulose wrote:

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

...

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

...

+        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));

Don't you need to use %p to not leak pointers?

...

+    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

I know some were already defined here, but this should probably be an enum in sysreg.

...

static inline int get_trbe_ec(u64 trbsr)   {

On 05/12/2025 4:28 am, Anshuman Khandual wrote:

On 01/12/25 4:51 PM, Leo Yan wrote:

...

When tracing stops, there is no collision with other samples, so using PERF_AUX_FLAG_COLLISION does not accurately reflect the trace state and may mislead userspace.

Use PERF_AUX_FLAG_PARTIAL instead to indicate that tracing stopped and the record may contain gaps.

Signed-off-by: Leo Yan leo.yan@arm.com

drivers/hwtracing/coresight/coresight-trbe.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index b06885a08e082fd34f68d9588518807b5c47c86e..0caa4a6b437a3aa39fc6bcc72a23711b54f7c598 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -669,21 +669,14 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand } /*

• * Mark the buffer to indicate that there was a WRAP event by
  

• * setting the COLLISION flag. This indicates to the user that
  

• * the TRBE trace collection was stopped without stopping the
  

• * ETE and thus there might be some amount of trace that was
  

• * lost between the time the WRAP was detected and the IRQ
  

• * was consumed by the CPU.
  

• *
  

• * Setting the TRUNCATED flag would move the event to STOPPED
  

• * state unnecessarily, even when there is space left in the
  

• * ring buffer. Using the COLLISION flag doesn't have this side
  

• * effect. We only set TRUNCATED flag when there is no space
  

• * left in the ring buffer.
  

• * Mark the buffer to indicate that the trace is stopped by setting
  

• * the PARTIAL flag. This indicates to the user that the TRBE trace
  

• * collection was stopped without stopping the ETE and thus there
  

• * might be some amount of trace that was lost between the time the
  

• * TRBE event was detected and the IRQ was consumed by the CPU.
  

  */ if (!is_trbe_running(trbsr))

• perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
  

• perf_aux_output_flag(handle, PERF_AUX_FLAG_PARTIAL);
  

This is interesting. So there is a no possibility for the records to be overridden in the perf ring buffer and incomplete record is all that can happen when TRBE is stopped, without stopping the ETE first ? Could both of these scenarios might be possible as well ?

There's no chance of overwriting because that would be the TRUNCATED case and the event would get disabled. PARTIAL seems to be when there was an error that's recoverable in the driver only, but as in my other comment I don't think the extra complexity is necessary and always using TRUNCATED would be enough.

...

if (is_trbe_wrap(trbsr)) return TRBE_FAULT_ACT_WRAP;

On Tue, Dec 09, 2025 at 01:17:50PM +0000, James Clark wrote:

[...]

...

...

+    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));

Don't you need to use %p to not leak pointers?

I will use "%pK" instead for debugging purpose when set kptr_restrict=0.

...

...

+    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

I know some were already defined here, but this should probably be an enum in sysreg.

Good point. Will do.

Thanks, Leo