[PATCH 00/19] coresight: trbe: Support trigger and circle buffer modes

[PATCH 00/19] coresight: trbe: Support trigger and circle buffer modes

[Leo Yan]

The current TRBE driver operates only in fill mode, where tracing stops
at the top of buffer and a maintenance interrupt is raised.  Due to
interrupt latency, tracing is halted while the program continues to run,
resulting in trace data lose.

This series enhances the driver for the trigger mode to mitigate trace
discontinuity.  The circle buffer mode is introduced to avoid any
maintenance interrupts during the snapshot session.

It can be divided into three parts for easier review:

  Patches 01~05: Minor refactoring for disabling operations and
                 clearing status.
  Patches 06~12: Refactor fault action and trace size calculation.
  Patches 13~19: Support trigger and circle modes. To better utilize the
                 new buffer modes, perf sets the watermark to
		 one-quarter of the buffer size.

This series is applied on coresight-next branch and has been validated
on Orion O6 board:

  1) The trigger count and wrap mode are verified using Ftrace logs.
  2) A new kunit test module verifies limit and count calculations.
  3) Basic perf record / script commands and module load/unload have
     been tested successfully.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
Leo Yan (19):
      coresight: trbe: Use helpers for checking errata
      coresight: trbe: Remove redundant disable operation
      coresight: trbe: Remove buffer disabling in trbe_handle_overflow()
      coresight: trbe: Remove set_trbe_disabled() from the enable flow
      coresight: trbe: Refactor status clearing
      coresight: trbe: Refactor syndrome decoding
      coresight: trbe: Refactor AUX flag setting
      coresight: trbe: Use PERF_AUX_FLAG_PARTIAL instead of PERF_AUX_FLAG_COLLISION
      coresight: trbe: Add fault action argument to trbe_handle_overflow()
      coresight: trbe: Always check fault action when updating buffer
      coresight: trbe: Apply overwrite erratum for only wrap event
      coresight: trbe: Calculate size for buffer wrapping
      coresight: trbe: Remove misleading comment
      coresight: trbe: Refactor compute_trbe_buffer_limit()
      coresight: trbe: Add static key for bypassing trigger mode
      coresight: trbe: Support trigger mode
      coresight: trbe: Enable circle mode for snapshot
      coresight: trbe: Add kunit tests
      perf: cs-etm: Set watermark for AUX trace

 drivers/hwtracing/coresight/Kconfig                |   9 +
 drivers/hwtracing/coresight/Makefile               |   1 +
 .../coresight/coresight-trbe-kunit-tests.c         | 536 +++++++++++++++++++++
 drivers/hwtracing/coresight/coresight-trbe.c       | 440 +++++++++--------
 drivers/hwtracing/coresight/coresight-trbe.h       | 111 ++++-
 tools/perf/arch/arm/util/cs-etm.c                  |   7 +
 6 files changed, 896 insertions(+), 208 deletions(-)
---
base-commit: 9e9182cab5ebc3ee7544e60ef08ba19fdf216920
change-id: 20251120-trbe_buffer_refactor_v1-1-8f8023105469

Best regards,
-- 
Leo Yan <leo.yan@arm.com>

[PATCH 01/19] coresight: trbe: Use helpers for checking errata

[Leo Yan]

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

-- 
2.34.1

Re: [PATCH 01/19] coresight: trbe: Use helpers for checking errata

[Anshuman Khandual]

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>

[PATCH 02/19] coresight: trbe: Remove redundant disable operation

[Leo Yan]

The trace unit is already disabled when trbe_stop_and_truncate_event()
is called, so draining and stopping the buffer in the function is
redundant.

Remove the unnecessary disable operation and rename the function to
trbe_truncate_event() to better reflect its purpose.

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

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 0ddb3db0213cf0014e29decfb79da68b0a351b31..2f44e4a65e0ee2b2c8fdd06a51ab01fc57f44a4e 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -284,18 +284,10 @@ static void trbe_report_wrap_event(struct perf_output_handle *handle)
 	perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
 }
 
-static void trbe_stop_and_truncate_event(struct perf_output_handle *handle)
+static void trbe_truncate_event(struct perf_output_handle *handle)
 {
 	struct trbe_buf *buf = etm_perf_sink_config(handle);
 
-	/*
-	 * We cannot proceed with the buffer collection and we
-	 * do not have any data for the current session. The
-	 * etm_perf driver expects to close out the aux_buffer
-	 * at event_stop(). So disable the TRBE here and leave
-	 * the update_buffer() to return a 0 size.
-	 */
-	trbe_drain_and_disable_local(buf->cpudata);
 	perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
 	perf_aux_output_end(handle, 0);
 	*this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL;
@@ -1008,7 +1000,7 @@ static int __arm_trbe_enable(struct trbe_buf *buf,
 	trbe_enable_hw(buf);
 	return 0;
 err:
-	trbe_stop_and_truncate_event(handle);
+	trbe_truncate_event(handle);
 	return ret;
 }
 
@@ -1169,7 +1161,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 		trbe_handle_spurious(handle);
 		break;
 	case TRBE_FAULT_ACT_FATAL:
-		trbe_stop_and_truncate_event(handle);
+		trbe_truncate_event(handle);
 		truncated = true;
 		break;
 	}

-- 
2.34.1

Re: [PATCH 02/19] coresight: trbe: Remove redundant disable operation

[Anshuman Khandual]

On 01/12/25 4:51 PM, Leo Yan wrote:
> The trace unit is already disabled when trbe_stop_and_truncate_event()
> is called, so draining and stopping the buffer in the function is
> redundant.
> 
> Remove the unnecessary disable operation and rename the function to
> trbe_truncate_event() to better reflect its purpose.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-trbe.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 0ddb3db0213cf0014e29decfb79da68b0a351b31..2f44e4a65e0ee2b2c8fdd06a51ab01fc57f44a4e 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -284,18 +284,10 @@ static void trbe_report_wrap_event(struct perf_output_handle *handle)
>  	perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
>  }
>  
> -static void trbe_stop_and_truncate_event(struct perf_output_handle *handle)
> +static void trbe_truncate_event(struct perf_output_handle *handle)
>  {
>  	struct trbe_buf *buf = etm_perf_sink_config(handle);
>  
> -	/*
> -	 * We cannot proceed with the buffer collection and we
> -	 * do not have any data for the current session. The
> -	 * etm_perf driver expects to close out the aux_buffer
> -	 * at event_stop(). So disable the TRBE here and leave
> -	 * the update_buffer() to return a 0 size.
> -	 */
> -	trbe_drain_and_disable_local(buf->cpudata);
>  	perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
>  	perf_aux_output_end(handle, 0);
>  	*this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL;
> @@ -1008,7 +1000,7 @@ static int __arm_trbe_enable(struct trbe_buf *buf,
>  	trbe_enable_hw(buf);
>  	return 0;
>  err:
> -	trbe_stop_and_truncate_event(handle);
> +	trbe_truncate_event(handle);
>  	return ret;
>  }
>  
> @@ -1169,7 +1161,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
>  		trbe_handle_spurious(handle);
>  		break;
>  	case TRBE_FAULT_ACT_FATAL:
> -		trbe_stop_and_truncate_event(handle);
> +		trbe_truncate_event(handle);
>  		truncated = true;
>  		break;
>  	}
> 

In arm_trbe_irq_handler() function trbe_drain_and_disable_local() is
called early on before TRBE_FAULT_ACT_FATAL gets handled but for the
other case __arm_trbe_enable() - do all the code paths leading upto
present trbe_stop_and_truncate_event() would have definitely called
trbe_drain_and_disable_local() as well ? If yes, please do call out
those code paths clearly in the commit message.

Besides it might be worth adding an WARN_ON() in trbe_truncate_event()
if the TRBE has not been disabled as now being expected there.

[PATCH 03/19] coresight: trbe: Remove buffer disabling in trbe_handle_overflow()

[Leo Yan]

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

-- 
2.34.1

Re: [PATCH 03/19] coresight: trbe: Remove buffer disabling in trbe_handle_overflow()

[Anshuman Khandual]

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>

[PATCH 04/19] coresight: trbe: Remove set_trbe_disabled() from the enable flow

[Leo Yan]

set_trbe_disabled() should never appear in the enable flow, otherwise,
it may potentially hide bugs in the disable flow.

Remove set_trbe_disabled() from the enable path.

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 f5597bd9b5fba9a8f5053d5823b03380fd468b5c..e426991e2c2c398a9d3982e9d0f7f542e404cbab 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);
-	set_trbe_disabled(buf->cpudata);
 	clr_trbe_status();
 	set_trbe_base_pointer(buf->trbe_hw_base);
 	set_trbe_write_pointer(buf->trbe_write);

-- 
2.34.1

Re: [PATCH 04/19] coresight: trbe: Remove set_trbe_disabled() from the enable flow

[Anshuman Khandual]

On 01/12/25 4:51 PM, Leo Yan wrote:
> set_trbe_disabled() should never appear in the enable flow, otherwise,
> it may potentially hide bugs in the disable flow.
> 
> Remove set_trbe_disabled() from the enable path.

IIRC without first disabling TRBLIMITR_EL1_E - TRBE registers or their
fields should not be fetched or interpreted. Without that none of the
subsequent HW operations should be performed inside trbe_enable_hw()
leading upto enabling 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 f5597bd9b5fba9a8f5053d5823b03380fd468b5c..e426991e2c2c398a9d3982e9d0f7f542e404cbab 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);
> -	set_trbe_disabled(buf->cpudata);
>  	clr_trbe_status();
>  	set_trbe_base_pointer(buf->trbe_hw_base);
>  	set_trbe_write_pointer(buf->trbe_write);
> 

Re: [PATCH 04/19] coresight: trbe: Remove set_trbe_disabled() from the enable flow

[Leo Yan]

On Thu, Dec 04, 2025 at 06:13:56PM +0530, Anshuman Khandual wrote:
> On 01/12/25 4:51 PM, Leo Yan wrote:
> > set_trbe_disabled() should never appear in the enable flow, otherwise,
> > it may potentially hide bugs in the disable flow.
> > 
> > Remove set_trbe_disabled() from the enable path.
> 
> IIRC without first disabling TRBLIMITR_EL1_E - TRBE registers or their
> fields should not be fetched or interpreted.

Yes. I think you are referring to the rule DJMDD in Arm ARM: "The PE
might ignore a direct or external write to any of certain Trace Buffer
Unit registers ... (when) TRBLIMITR_EL1.E is 1, and the Trace Buffer
Unit is using Self-hosted mode."

> Without that none of the
> subsequent HW operations should be performed inside trbe_enable_hw()
> leading upto enabling it.

Fair enough.

If we can conclude the trace unit has been disabled properly in below
cases, no reason to arbitrarily calling set_trbe_disabled() during each
enable.

  1) The SYS_TRBLIMITR_EL1 register is cleared in trbe_reset_local()
     during probe phase.
  2) The SYS_TRBLIMITR_EL1.E bit is cleared in arm_trbe_irq_handler()
     for interrupt handling.
  3) The SYS_TRBLIMITR_EL1.E bit is cleared in the disable flow.


Seems to me, this is not only for code cleanup, we need to promise a
sane logic in the flow.

Thanks,
Leo

[PATCH 05/19] coresight: trbe: Refactor status clearing

[Leo Yan]

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

-- 
2.34.1

Re: [PATCH 05/19] coresight: trbe: Refactor status clearing

[Anshuman Khandual]

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

Re: [PATCH 05/19] coresight: trbe: Refactor status clearing

[Leo Yan]

On Thu, Dec 04, 2025 at 06:27:27PM +0530, Anshuman Khandual wrote:
> 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 ?

It is about the driver's sanity.

The driver should clear the status immediately when the trace unit is
disabled, rather than waiting until the next enable.  This avoids
unexpected behaviour, such as a spurious TRBE interrupt.

Consider an edge case: if TRBE is being disabled at the same moment it
is about to raise an interrupt, arm_trbe_update_buffer() may miss the
IRQ bit due to latency.  If arm_trbe_disable() does not clear that bit,
arm_trbe_irq_handler() will still run after the perf event has been
stopped.  The interrupt handler then retrieves the trbe_buf pointer
from the perf output handle, which is dangerous because the perf
session has already ended.

Thanks,
Leo

[PATCH 06/19] coresight: trbe: Refactor syndrome decoding

[Leo Yan]

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.

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

-- 
2.34.1

Re: [PATCH 06/19] coresight: trbe: Refactor syndrome decoding

[Suzuki K Poulose]

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

Re: [PATCH 06/19] coresight: trbe: Refactor syndrome decoding

[Leo Yan]

On Tue, Dec 02, 2025 at 11:06:48AM +0000, 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.

Sure, I will note this later.

> 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"

Will do this and followed comment.  Thanks for suggestion!

Leo

Re: [PATCH 06/19] coresight: trbe: Refactor syndrome decoding

[James Clark]

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

Re: [PATCH 06/19] coresight: trbe: Refactor syndrome decoding

[Leo Yan]

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

Re: [PATCH 06/19] coresight: trbe: Refactor syndrome decoding

[Anshuman Khandual]

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

Re: [PATCH 06/19] coresight: trbe: Refactor syndrome decoding

[Leo Yan]

On Fri, Dec 05, 2025 at 09:40:33AM +0530, Anshuman Khandual wrote:
> 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.

Agreed.  I will refine the commit log to state that we need to diagnose
the error code based on the hierarchy information (EC and BSC).

> > 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 ?

Good point.  I will consider extracting the change; this would make
the review clearer.

[...]

> >  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 ?

No.  EC_OTHERS means "this is a normal maintenance interrupt".

> > +	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.

Now we have two level's (EC/BSC) error codes, it would be complex to
maintain a string array and map to two level's error codes.

> > +	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 ?

The TRBSR_EL1.WRAP bit reliably indicates that the pointer has
wrapped.  I don't think we need any special handling for the overwrite
erratum here; we can simply return TRBE_FAULT_ACT_WRAP.

Afterward, trbe_get_trace_size() consumes the wrap flag and will
compute the trace size appropriately for the overwrite erratum when
the wrap flag is set.

[...]

> > -#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.

As James suggested in another reply, I will consider to add sysreg
enum.

Thanks,
Leo

[PATCH 07/19] coresight: trbe: Refactor AUX flag setting

[Leo Yan]

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

-- 
2.34.1

Re: [PATCH 07/19] coresight: trbe: Refactor AUX flag setting

[Suzuki K Poulose]

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

Re: [PATCH 07/19] coresight: trbe: Refactor AUX flag setting

[Leo Yan]

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

Re: [PATCH 07/19] coresight: trbe: Refactor AUX flag setting

[James Clark]

On 01/12/2025 11:21 am, 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))
> +		perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
> +

Do we need the complexity of COLLISION (changed to PARTIAL in a later 
commit) and TRUNCATED at the same time?

 From my understanding, TRUNCATED would be when Perf is too slow and we 
want to disable the event for it to catch up, and PARTIAL is when there 
was some buffer error so technically we don't need the event to be 
disabled, but Perf still needs to be notified if we want to start only 
resetting the decoder when a flag is set rather than on all aux records?

There is no happy path where there is a buffer error, so I'm not sure 
why we need the complexity of an extra case? There's no harm in 
disabling the event on a buffer error. Also it's hard to see what the 
exact scenario for PARTIAL is, because we set truncated anyway for fatal 
statuses. The comment in the later commit for the PARTIAL change just 
says "trace was stopped", but not why.

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

Re: [PATCH 07/19] coresight: trbe: Refactor AUX flag setting

[Leo Yan]

On Tue, Dec 09, 2025 at 01:37:39PM +0000, James Clark wrote:

[...]

> > +	if (!is_trbe_running(trbsr))
> > +		perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
> > +
> 
> Do we need the complexity of COLLISION (changed to PARTIAL in a later
> commit) and TRUNCATED at the same time?

> From my understanding, TRUNCATED would be when Perf is too slow and we want
> to disable the event for it to catch up, and PARTIAL is when there was some
> buffer error so technically we don't need the event to be disabled, but Perf
> still needs to be notified if we want to start only resetting the decoder
> when a flag is set rather than on all aux records?

The main difference between the TRUNCATED flag and the COLLISION/PARTIAL
flags is that the TRUNCATED flag implicitly asks the event core layer to
disable the PMU event.  In contrast, the COLLISION and PARTIAL flags are
only stored in AUX record and no additional action in the event core
layer.

Setting the TRUNCATED flag would be a over-killer if the buffer has
free space, as it would run the expensive sequence of disabling both ETE
and TRBE and then requiring userspace to re-enable them.

> There is no happy path where there is a buffer error, so I'm not sure why we
> need the complexity of an extra case? There's no harm in disabling the event
> on a buffer error. Also it's hard to see what the exact scenario for PARTIAL
> is, because we set truncated anyway for fatal statuses. The comment in the
> later commit for the PARTIAL change just says "trace was stopped", but not
> why.

The trace can be stopped on Fill mode (stop on wrap) or Stop on trigger, so
it is not (only) about buffer error.  In these cases, even though the trace
unit is stopped, the buffer may still have space available, tracing can be
directly re-enabled in interrupt handler, thus the COLLISION/PARTIAL flags
are better choices.

Only when the buffer is actually used out (i.e., handle->size == 0), it is
appropriate to set TRUNCATED flag, since at that point we must wait for
userspace to read out the data.

Thanks,
Leo

Re: [PATCH 07/19] coresight: trbe: Refactor AUX flag setting

[James Clark]

On 10/12/2025 17:43, Leo Yan wrote:
> On Tue, Dec 09, 2025 at 01:37:39PM +0000, James Clark wrote:
> 
> [...]
> 
>>> +	if (!is_trbe_running(trbsr))
>>> +		perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
>>> +
>>
>> Do we need the complexity of COLLISION (changed to PARTIAL in a later
>> commit) and TRUNCATED at the same time?
> 
>>  From my understanding, TRUNCATED would be when Perf is too slow and we want
>> to disable the event for it to catch up, and PARTIAL is when there was some
>> buffer error so technically we don't need the event to be disabled, but Perf
>> still needs to be notified if we want to start only resetting the decoder
>> when a flag is set rather than on all aux records?
> 
> The main difference between the TRUNCATED flag and the COLLISION/PARTIAL
> flags is that the TRUNCATED flag implicitly asks the event core layer to
> disable the PMU event.  In contrast, the COLLISION and PARTIAL flags are
> only stored in AUX record and no additional action in the event core
> layer.
> 
> Setting the TRUNCATED flag would be a over-killer if the buffer has
> free space, as it would run the expensive sequence of disabling both ETE
> and TRBE and then requiring userspace to re-enable them.
> 
>> There is no happy path where there is a buffer error, so I'm not sure why we
>> need the complexity of an extra case? There's no harm in disabling the event
>> on a buffer error. Also it's hard to see what the exact scenario for PARTIAL
>> is, because we set truncated anyway for fatal statuses. The comment in the
>> later commit for the PARTIAL change just says "trace was stopped", but not
>> why.
> 
> The trace can be stopped on Fill mode (stop on wrap) or Stop on trigger, so
> it is not (only) about buffer error.  In these cases, even though the trace
> unit is stopped, the buffer may still have space available, tracing can be
> directly re-enabled in interrupt handler, thus the COLLISION/PARTIAL flags
> are better choices.

I suppose this is the bit that I don't understand. If there is space 
left then why is the stop hit at all?

For the other cases, I agree that it's overkill, but my point is that 
doesn't matter because we don't expect it to be hit in normal cases 
anyway. So it's worth the simplification of not doing anything different 
than TRUNCATED.

> 
> Only when the buffer is actually used out (i.e., handle->size == 0), it is
> appropriate to set TRUNCATED flag, since at that point we must wait for
> userspace to read out the data.
> 
> Thanks,
> Leo

Re: [PATCH 07/19] coresight: trbe: Refactor AUX flag setting

[Leo Yan]

On Fri, Dec 12, 2025 at 04:50:52PM +0200, James Clark wrote:

[...]

> > The trace can be stopped on Fill mode (stop on wrap) or Stop on trigger, so
> > it is not (only) about buffer error.  In these cases, even though the trace
> > unit is stopped, the buffer may still have space available, tracing can be
> > directly re-enabled in interrupt handler, thus the COLLISION/PARTIAL flags
> > are better choices.
> 
> I suppose this is the bit that I don't understand. If there is space left
> then why is the stop hit at all?

For instance, the buffer becomes empty in the middle:

   +----------------------------+
   |$$$$$$$|            |$$$$$$$|
   +----------------------------+
   ` base  `wptr        `limit

In this case, the trace unit stops when it reaches the limit.
Meanwhile, the tool might have already read out the old trace data so
we will get some free space, thus no need to set TRUNCATED flag.

Re: [PATCH 07/19] coresight: trbe: Refactor AUX flag setting

[James Clark]

On 12/12/2025 17:27, Leo Yan wrote:
> On Fri, Dec 12, 2025 at 04:50:52PM +0200, James Clark wrote:
> 
> [...]
> 
>>> The trace can be stopped on Fill mode (stop on wrap) or Stop on trigger, so
>>> it is not (only) about buffer error.  In these cases, even though the trace
>>> unit is stopped, the buffer may still have space available, tracing can be
>>> directly re-enabled in interrupt handler, thus the COLLISION/PARTIAL flags
>>> are better choices.
>>
>> I suppose this is the bit that I don't understand. If there is space left
>> then why is the stop hit at all?
> 
> For instance, the buffer becomes empty in the middle:
> 
>     +----------------------------+
>     |$$$$$$$|            |$$$$$$$|
>     +----------------------------+
>     ` base  `wptr        `limit
> 
> In this case, the trace unit stops when it reaches the limit.
> Meanwhile, the tool might have already read out the old trace data so
> we will get some free space, thus no need to set TRUNCATED flag.

Hmmm I see, I still think that means that Perf is being too slow and 
disabling the event wouldn't be that bad.

Also isn't there a very small chance to hit this? I'm wondering if you 
ever saw PARTIAL flags in practice?

I think the following all have to be true:

  * 3 out of 4 sections were full on the last interrupt
  * Perf hadn't managed to finish reading even 1 section in that time
  * The last section fills up
  * Perf had managed to finish reading 1 section between the time of
    the previous interrupt and the hard stop

There would have to be quite a high variability in read speed to make 
that possible. Maybe it happens and it's worthwhile optimizing for. The 
comments could explain it better if we do decide to keep it.

[PATCH 08/19] coresight: trbe: Use PERF_AUX_FLAG_PARTIAL instead of PERF_AUX_FLAG_COLLISION

[Leo Yan]

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);
 
 	if (is_trbe_wrap(trbsr))
 		return TRBE_FAULT_ACT_WRAP;

-- 
2.34.1

Re: [PATCH 08/19] coresight: trbe: Use PERF_AUX_FLAG_PARTIAL instead of PERF_AUX_FLAG_COLLISION

[Anshuman Khandual]

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 ?

>  
>  	if (is_trbe_wrap(trbsr))
>  		return TRBE_FAULT_ACT_WRAP;
> 

Re: [PATCH 08/19] coresight: trbe: Use PERF_AUX_FLAG_PARTIAL instead of PERF_AUX_FLAG_COLLISION

[James Clark]

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

Re: [PATCH 08/19] coresight: trbe: Use PERF_AUX_FLAG_PARTIAL instead of PERF_AUX_FLAG_COLLISION

[Leo Yan]

On Tue, Dec 09, 2025 at 01:40:35PM +0000, James Clark wrote:

[...]

> > >   	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 I extend a bit, I would say that the TRUNCATED flag is used to prevent
buffer overwrites, while the other flags indicate why a record is
incomplete.

Thanks,
Leo

[PATCH 09/19] coresight: trbe: Add fault action argument to trbe_handle_overflow()

[Leo Yan]

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

-- 
2.34.1

[PATCH 10/19] coresight: trbe: Always check fault action when updating buffer

[Leo Yan]

The current code checks the fault action only via the IRQ status bit,
which is unreliable due to possible hardware latency.

Move the fault action check out of the IRQ status condition.  This also
causes the buffer size to be calculated for non-WRAP and fault cases,
which is fine since the write pointer is trusted for the calculation.

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

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index f56ecdeaa6596afb440e4d53732e08a85f9bf89d..e579ea98523c24d23a0cd265dcdd0a46b52b52da 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -806,7 +806,6 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev,
 	enum trbe_fault_action act;
 	unsigned long size, status;
 	unsigned long flags;
-	bool wrap = false;
 
 	WARN_ON(buf->cpudata != cpudata);
 	WARN_ON(cpudata->cpu != smp_processor_id());
@@ -858,21 +857,11 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev,
 		 */
 		clr_trbe_irq();
 		isb();
-
-		act = trbe_get_fault_act(handle, status);
-		/*
-		 * If this was not due to a WRAP event, we have some
-		 * errors and as such buffer is empty.
-		 */
-		if (act != TRBE_FAULT_ACT_WRAP) {
-			size = 0;
-			goto done;
-		}
-
-		wrap = true;
 	}
 
-	size = trbe_get_trace_size(handle, buf, wrap);
+	act = trbe_get_fault_act(handle, status);
+
+	size = trbe_get_trace_size(handle, buf, act == TRBE_FAULT_ACT_WRAP);
 
 done:
 	local_irq_restore(flags);

-- 
2.34.1

Re: [PATCH 10/19] coresight: trbe: Always check fault action when updating buffer

[Suzuki K Poulose]

On 01/12/2025 11:22, Leo Yan wrote:
> The current code checks the fault action only via the IRQ status bit,
> which is unreliable due to possible hardware latency.
> 
> Move the fault action check out of the IRQ status condition.  This also
> causes the buffer size to be calculated for non-WRAP and fault cases,
> which is fine since the write pointer is trusted for the calculation.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-trbe.c | 17 +++--------------
>   1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index f56ecdeaa6596afb440e4d53732e08a85f9bf89d..e579ea98523c24d23a0cd265dcdd0a46b52b52da 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -806,7 +806,6 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev,
>   	enum trbe_fault_action act;
>   	unsigned long size, status;
>   	unsigned long flags;
> -	bool wrap = false;
>   
>   	WARN_ON(buf->cpudata != cpudata);
>   	WARN_ON(cpudata->cpu != smp_processor_id());
> @@ -858,21 +857,11 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev,
>   		 */
>   		clr_trbe_irq();
>   		isb();
> -
> -		act = trbe_get_fault_act(handle, status);
> -		/*
> -		 * If this was not due to a WRAP event, we have some
> -		 * errors and as such buffer is empty.
> -		 */
> -		if (act != TRBE_FAULT_ACT_WRAP) {
> -			size = 0;
> -			goto done;
> -		}
> -
> -		wrap = true;
>   	}
>   
> -	size = trbe_get_trace_size(handle, buf, wrap);
> +	act = trbe_get_fault_act(handle, status);

Also act is valid only when there is an IRQ ?

> +
> +	size = trbe_get_trace_size(handle, buf, act == TRBE_FAULT_ACT_WRAP);

We have certain assumptions in trbe_get_trace_size(), which may be
broken with this change.

e.g., if we get a fatal error, we don't detect that the buffer is empty
and may trigger a WARN_ON() on systems with CPU erratum as below ?

>         /*                                                                      
>          * If the TRBE is affected by the following erratum, we must fill           
>          * 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) &&
>             !WARN_ON(size < overwrite_skip))                                    
>                 __trbe_pad_buf(buf, start_off, overwrite_skip); 



Suzuki


>   
>   done:
>   	local_irq_restore(flags);
> 

[PATCH 11/19] coresight: trbe: Apply overwrite erratum for only wrap event

[Leo Yan]

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

-- 
2.34.1

Re: [PATCH 11/19] coresight: trbe: Apply overwrite erratum for only wrap event

[Suzuki K Poulose]

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

Re: [PATCH 11/19] coresight: trbe: Apply overwrite erratum for only wrap event

[Leo Yan]

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=

Re: [PATCH 11/19] coresight: trbe: Apply overwrite erratum for only wrap event

[Leo Yan]

On Tue, Dec 02, 2025 at 04:56:47PM +0000, Coresight ML wrote:
> 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.

Thought again, we should not worry about out-of-range case.

As the workaround always ensures sufficient space beyond the limit, the
write pointer remains valid within the allocated buffer even if it is
updated and crosses that limit.

[PATCH 12/19] coresight: trbe: Calculate size for buffer wrapping

[Leo Yan]

Calculate the wrapped size when the end position is less than the start.

If the start equals the end, the "wrap" flag is used to decide whether
the buffer is full or empty.

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

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 2600af12a8fb94bb8c74efda2a101aacd01b0b34..48bc03bd339908b5eac9466dc60325ff1b238976 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -696,7 +696,7 @@ static unsigned long trbe_get_trace_size(struct perf_output_handle *handle,
 {
 	u64 write;
 	u64 start_off, end_off;
-	u64 size;
+	u64 size, buf_size;
 	u64 overwrite_skip = TRBE_WORKAROUND_OVERWRITE_FILL_MODE_SKIP_BYTES;
 
 	/*
@@ -726,11 +726,18 @@ static unsigned long trbe_get_trace_size(struct perf_output_handle *handle,
 	 */
 	end_off = write - buf->trbe_base;
 	start_off = PERF_IDX2OFF(handle->head, buf);
+	buf_size = buf->trbe_limit - buf->trbe_base;
+
+	if (end_off > start_off)
+		size = end_off - start_off;
+	else if (end_off < start_off)
+		size = end_off + buf_size - start_off;
+	else if (wrap)
+		/* The start is the same as the end, just wrapped */
+		size = buf_size;
+	else
+		size = 0;
 
-	if (WARN_ON_ONCE(end_off < start_off))
-		return 0;
-
-	size = end_off - start_off;
 	/*
 	 * If the TRBE is affected by the following erratum, we must fill
 	 * the space we skipped with IGNORE packets. And we are always

-- 
2.34.1

[PATCH 13/19] coresight: trbe: Remove misleading comment

[Leo Yan]

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;

-- 
2.34.1

[PATCH 14/19] coresight: trbe: Refactor compute_trbe_buffer_limit()

[Leo Yan]

Refactor compute_trbe_buffer_limit() to perform the computation and
handle failures. The return type is changed from a limit offset to an
error number (0 is for success).

This refactoring is for future extensions, such as calculating
additional values (e.g., the trigger count). No functional changes are
introduced.

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

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 206eaf103cd94f36220cb6bddd1a78012f5de35a..941aa46e9b11f60c707eb40093964de454a3fd83 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -332,7 +332,7 @@ static void trbe_pad_buf(struct perf_output_handle *handle, int len)
 		perf_aux_output_skip(handle, len);
 }
 
-static unsigned long trbe_snapshot_offset(struct perf_output_handle *handle)
+static int trbe_snapshot_offset(struct perf_output_handle *handle)
 {
 	struct trbe_buf *buf = etm_perf_sink_config(handle);
 
@@ -341,7 +341,8 @@ static unsigned long trbe_snapshot_offset(struct perf_output_handle *handle)
 	 * the decoder to reset in case of an overflow or corruption.
 	 * So we can use the entire buffer for the snapshot mode.
 	 */
-	return buf->nr_pages * PAGE_SIZE;
+	buf->trbe_limit = buf->trbe_base + buf->nr_pages * PAGE_SIZE;
+	return 0;
 }
 
 static u64 trbe_min_trace_buf_size(struct perf_output_handle *handle)
@@ -510,7 +511,7 @@ static unsigned long __trbe_normal_offset(struct perf_output_handle *handle)
 	return 0;
 }
 
-static unsigned long trbe_normal_offset(struct perf_output_handle *handle)
+static int trbe_normal_offset(struct perf_output_handle *handle)
 {
 	struct trbe_buf *buf = etm_perf_sink_config(handle);
 	u64 limit = __trbe_normal_offset(handle);
@@ -529,19 +530,34 @@ static unsigned long trbe_normal_offset(struct perf_output_handle *handle)
 		limit = __trbe_normal_offset(handle);
 		head = PERF_IDX2OFF(handle->head, buf);
 	}
-	return limit;
+
+	if (!limit)
+		return -ENOSPC;
+
+	buf->trbe_limit = buf->trbe_base + limit;
+	return 0;
 }
 
-static unsigned long compute_trbe_buffer_limit(struct perf_output_handle *handle)
+static int trbe_compute_next(struct perf_output_handle *handle)
 {
 	struct trbe_buf *buf = etm_perf_sink_config(handle);
-	unsigned long offset;
+	int ret;
+
+	perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW);
 
 	if (buf->snapshot)
-		offset = trbe_snapshot_offset(handle);
+		ret = trbe_snapshot_offset(handle);
 	else
-		offset = trbe_normal_offset(handle);
-	return buf->trbe_base + offset;
+		ret = trbe_normal_offset(handle);
+
+	if (ret)
+		return ret;
+
+	buf->trbe_write = buf->trbe_base + PERF_IDX2OFF(handle->head, buf);
+
+	/* Set the base of the TRBE to the buffer base */
+	buf->trbe_hw_base = buf->trbe_base;
+	return 0;
 }
 
 static void clr_trbe_status(void)
@@ -986,15 +1002,9 @@ static int __arm_trbe_enable(struct trbe_buf *buf,
 {
 	int ret = 0;
 
-	perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW);
-	buf->trbe_limit = compute_trbe_buffer_limit(handle);
-	buf->trbe_write = buf->trbe_base + PERF_IDX2OFF(handle->head, buf);
-	if (buf->trbe_limit == buf->trbe_base) {
-		ret = -ENOSPC;
+	ret = trbe_compute_next(handle);
+	if (ret)
 		goto err;
-	}
-	/* Set the base of the TRBE to the buffer base */
-	buf->trbe_hw_base = buf->trbe_base;
 
 	ret = trbe_apply_work_around_before_enable(buf);
 	if (ret)

-- 
2.34.1

[PATCH 15/19] coresight: trbe: Add static key for bypassing trigger mode

[Leo Yan]

To avoid complexity, if any CPU in the system has the fill mode erratum,
the driver will not use trigger mode, it simply rolls back to fill mode
only and apply the workaround on it.

Add a static key to control trigger mode bypassing.  During each CPU
probe, the key is enabled when the relevant erratum is detected.

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

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 941aa46e9b11f60c707eb40093964de454a3fd83..8390d0a8fe23d35945610df15f21751279ee37ee 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -17,6 +17,7 @@
 
 #include <asm/barrier.h>
 #include <asm/cpufeature.h>
+#include <linux/jump_label.h>
 #include <linux/kvm_host.h>
 #include <linux/vmalloc.h>
 
@@ -147,6 +148,12 @@ struct trbe_drvdata {
 	struct platform_device *pdev;
 };
 
+DEFINE_STATIC_KEY_FALSE(trbe_trigger_mode_bypass);
+
+#define trbe_trigger_mode_need_bypass(cpudata)		\
+	(trbe_may_overwrite_in_fill_mode((cpudata)) ||	\
+	 trbe_may_write_out_of_range((cpudata)))
+
 static void trbe_check_errata(struct trbe_cpudata *cpudata)
 {
 	int i;
@@ -1306,6 +1313,14 @@ static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cp
 
 	dev_set_drvdata(&trbe_csdev->dev, cpudata);
 	coresight_set_percpu_sink(cpu, trbe_csdev);
+
+	/*
+	 * If any CPU cannot use trigger mode, bypass the mode globally for
+	 * consistent tracing behaviour.
+	 */
+	if (trbe_trigger_mode_need_bypass(cpudata))
+		static_branch_enable(&trbe_trigger_mode_bypass);
+
 	return;
 cpu_clear:
 	cpumask_clear_cpu(cpu, &drvdata->supported_cpus);

-- 
2.34.1

Re: [PATCH 15/19] coresight: trbe: Add static key for bypassing trigger mode

[Suzuki K Poulose]

On 01/12/2025 11:22, Leo Yan wrote:
> To avoid complexity, if any CPU in the system has the fill mode erratum,
> the driver will not use trigger mode, it simply rolls back to fill mode
> only and apply the workaround on it.
> 
> Add a static key to control trigger mode bypassing.  During each CPU
> probe, the key is enabled when the relevant erratum is detected.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-trbe.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 941aa46e9b11f60c707eb40093964de454a3fd83..8390d0a8fe23d35945610df15f21751279ee37ee 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -17,6 +17,7 @@
>   
>   #include <asm/barrier.h>
>   #include <asm/cpufeature.h>
> +#include <linux/jump_label.h>
>   #include <linux/kvm_host.h>
>   #include <linux/vmalloc.h>
>   
> @@ -147,6 +148,12 @@ struct trbe_drvdata {
>   	struct platform_device *pdev;
>   };
>   
> +DEFINE_STATIC_KEY_FALSE(trbe_trigger_mode_bypass);
> +
> +#define trbe_trigger_mode_need_bypass(cpudata)		\
> +	(trbe_may_overwrite_in_fill_mode((cpudata)) ||	\
> +	 trbe_may_write_out_of_range((cpudata)))
> +

Please make that a static inline function.

>   static void trbe_check_errata(struct trbe_cpudata *cpudata)
>   {
>   	int i;
> @@ -1306,6 +1313,14 @@ static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cp
>   
>   	dev_set_drvdata(&trbe_csdev->dev, cpudata);
>   	coresight_set_percpu_sink(cpu, trbe_csdev);
> +
> +	/*
> +	 * If any CPU cannot use trigger mode, bypass the mode globally for
> +	 * consistent tracing behaviour.
> +	 */
> +	if (trbe_trigger_mode_need_bypass(cpudata))
> +		static_branch_enable(&trbe_trigger_mode_bypass);

You only need to enable this once, not for every CPU.

Suzuki


> +
>   	return;
>   cpu_clear:
>   	cpumask_clear_cpu(cpu, &drvdata->supported_cpus);
> 

[PATCH 16/19] coresight: trbe: Support trigger mode

[Leo Yan]

The buffer currently operates in fill mode, where tracing stops when it
reaches the end of the buffer and a maintenance interrupt is raised.
However, due to IRQ latency, trace data may be lost during the window in
which tracing is halted but the program continues to run.

To mitigate the issue, this commit enables the trigger count to support
buffer maintenance without disabling tracing.  This is fulfilled with
two modes:

1) Set a trigger count as a watermark and use fill mode to prevent the
   buffer from being overwritten.  Once the count is decremented to
   zero, an interrupt is raised for buffer maintenance, but the hardware
   continues collecting trace data until limit.

          head            watermark tail
     +----+---------------+---------+-------+
     |$$$$|               |         |$$$$$$$|
     +----+---------------+---------+-------+
     base `---- count ----'         limit   base + nr_pages

     $$$ : Filled trace data

2) Use wrap mode so that tracing continues when reach the top of the
   buffer.  The trigger count is configured as "Stop on trigger" to
   guard the trace data not to be overwritten.

              watermark   tail      head
     +--------+-----------+---------+-------+
     |        |           |$$$$$$$$$|       |
     +--------+-----------+---------+-------+
     base                                   base + nr_pages
                                            limit

                                    `------->
     >-- counter ---------'

     $$$ : Filled trace data

The modes are selected by comparing the limit with the trigger position.

An extra TRBE_FAULT_ACT_TRIG state is introduced for fault action, it is
used to distinguish the trigger event from the WRAP event.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-trbe.c | 101 +++++++++++++++++++++------
 drivers/hwtracing/coresight/coresight-trbe.h |  14 ++++
 2 files changed, 94 insertions(+), 21 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 8390d0a8fe23d35945610df15f21751279ee37ee..0551ea9b4f8286c156e3c9c7ac94e2ecd3b9dc3f 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -48,6 +48,7 @@
 #define TRBE_TRACE_MIN_BUF_SIZE		64
 
 enum trbe_fault_action {
+	TRBE_FAULT_ACT_TRIG,
 	TRBE_FAULT_ACT_WRAP,
 	TRBE_FAULT_ACT_SPURIOUS,
 	TRBE_FAULT_ACT_FATAL,
@@ -67,6 +68,7 @@ struct trbe_buf {
 	unsigned long trbe_hw_base;
 	unsigned long trbe_limit;
 	unsigned long trbe_write;
+	unsigned long trbe_count;
 	int nr_pages;
 	void **pages;
 	bool snapshot;
@@ -478,6 +480,10 @@ static unsigned long __trbe_normal_offset(struct perf_output_handle *handle)
 	if (head < tail)
 		limit = round_down(tail, PAGE_SIZE);
 
+	/* If trigger mode is enabled, no need to use limit for watermark */
+	if (!static_branch_unlikely(&trbe_trigger_mode_bypass))
+		goto out;
+
 	/*
 	 * Wakeup may be arbitrarily far into the future. If it's not in the
 	 * current generation, either we'll wrap before hitting it, or it's
@@ -495,6 +501,7 @@ static unsigned long __trbe_normal_offset(struct perf_output_handle *handle)
 	if (handle->wakeup < (handle->head + handle->size) && head <= wakeup)
 		limit = min(limit, round_up(wakeup, PAGE_SIZE));
 
+out:
 	/*
 	 * There is a situation when this can happen i.e limit is before
 	 * the head and hence TRBE cannot be configured.
@@ -518,6 +525,39 @@ static unsigned long __trbe_normal_offset(struct perf_output_handle *handle)
 	return 0;
 }
 
+static u64 __trbe_normal_trigger_count(struct perf_output_handle *handle)
+{
+	struct trbe_buf *buf = etm_perf_sink_config(handle);
+	struct trbe_cpudata *cpudata = buf->cpudata;
+	u64 limit, head, wakeup;
+	u64 count = 0;
+
+	if (static_branch_unlikely(&trbe_trigger_mode_bypass))
+		return 0;
+
+	limit = buf->trbe_limit - buf->trbe_base;
+	head = PERF_IDX2OFF(handle->head, buf);
+	wakeup = PERF_IDX2OFF(handle->wakeup, buf);
+
+	/* Set the count to guard the end of free buffer after wrap around */
+	if (limit == buf->nr_pages * PAGE_SIZE && (head + handle->size) > limit)
+		count = handle->size;
+
+	/*
+	 * If the watermark is less than the limit, use the trigger count for
+	 * the watermark maintenance.
+	 */
+	if (handle->wakeup < (handle->head + handle->size) && head <= wakeup) {
+		u64 wakeup_count =
+			round_up(wakeup - head, cpudata->trbe_hw_align);
+
+		if (head + wakeup_count < limit)
+			count = wakeup_count;
+	}
+
+	return count;
+}
+
 static int trbe_normal_offset(struct perf_output_handle *handle)
 {
 	struct trbe_buf *buf = etm_perf_sink_config(handle);
@@ -542,6 +582,7 @@ static int trbe_normal_offset(struct perf_output_handle *handle)
 		return -ENOSPC;
 
 	buf->trbe_limit = buf->trbe_base + limit;
+	buf->trbe_count = __trbe_normal_trigger_count(handle);
 	return 0;
 }
 
@@ -594,24 +635,40 @@ static void set_trbe_limit_pointer_enabled(struct trbe_buf *buf)
 	trblimitr &= ~TRBLIMITR_EL1_TM_MASK;
 	trblimitr &= ~TRBLIMITR_EL1_LIMIT_MASK;
 
-	/*
-	 * Fill trace buffer mode is used here while configuring the
-	 * TRBE for trace capture. In this particular mode, the trace
-	 * collection is stopped and a maintenance interrupt is raised
-	 * when the current write pointer wraps. This pause in trace
-	 * collection gives the software an opportunity to capture the
-	 * trace data in the interrupt handler, before reconfiguring
-	 * the TRBE.
-	 */
-	trblimitr |= (TRBLIMITR_EL1_FM_FILL << TRBLIMITR_EL1_FM_SHIFT) &
-		     TRBLIMITR_EL1_FM_MASK;
+	if (!buf->trbe_count ||
+	    buf->trbe_write + buf->trbe_count == buf->trbe_limit) {
+		/*
+		 * Fill trace buffer mode is used here while configuring the
+		 * TRBE for trace capture. In this particular mode, the trace
+		 * collection is stopped and a maintenance interrupt is raised
+		 * when the current write pointer wraps. This pause in trace
+		 * collection gives the software an opportunity to capture the
+		 * trace data in the interrupt handler, before reconfiguring
+		 * the TRBE.
+		 */
+		trblimitr |= FIELD_PREP(TRBLIMITR_EL1_FM_MASK, TRBLIMITR_EL1_FM_FILL) |
+			     FIELD_PREP(TRBLIMITR_EL1_TM_MASK, TRBLIMITR_EL1_TM_IGNR);
+	} else if (buf->trbe_write + buf->trbe_count < buf->trbe_limit) {
+		/*
+		 * Fill mode is used here to stop trace collection and prevent
+		 * the buffer from being overwritten. Trigger mode continues
+		 * trace collection and raises a maintenance interrupt on a
+		 * trigger event, which acts as a watermark for notifying
+		 * userspace.
+		 */
+		trblimitr |= FIELD_PREP(TRBLIMITR_EL1_FM_MASK, TRBLIMITR_EL1_FM_FILL) |
+			     FIELD_PREP(TRBLIMITR_EL1_TM_MASK, TRBLIMITR_EL1_TM_IRQ);
+	} else if (buf->trbe_write + buf->trbe_count > buf->trbe_limit) {
+		/*
+		 * Wrap buffer mode continues trace collection and raises
+		 * maintenance interrupt on buffer wrap. Trigger mode stops
+		 * trace on trigger event to guard the buffer from being
+		 * overwritten.
+		 */
+		trblimitr |= FIELD_PREP(TRBLIMITR_EL1_FM_MASK, TRBLIMITR_EL1_FM_WRAP) |
+			     FIELD_PREP(TRBLIMITR_EL1_TM_MASK, TRBLIMITR_EL1_TM_STOP);
+	}
 
-	/*
-	 * Trigger mode is not used here while configuring the TRBE for
-	 * the trace capture. Hence just keep this in the ignore mode.
-	 */
-	trblimitr |= (TRBLIMITR_EL1_TM_IGNR << TRBLIMITR_EL1_TM_SHIFT) &
-		     TRBLIMITR_EL1_TM_MASK;
 	trblimitr |= (addr & PAGE_MASK);
 	set_trbe_enabled(buf->cpudata, trblimitr);
 }
@@ -623,6 +680,7 @@ static void trbe_enable_hw(struct trbe_buf *buf)
 	WARN_ON(buf->trbe_write >= buf->trbe_limit);
 	set_trbe_base_pointer(buf->trbe_hw_base);
 	set_trbe_write_pointer(buf->trbe_write);
+	set_trbe_trigger_count(buf->trbe_count);
 
 	/*
 	 * Synchronize all the register updates
@@ -639,8 +697,6 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand
 	int ec = get_trbe_ec(trbsr);
 	int bsc = get_trbe_bsc(trbsr);
 
-	WARN_ON(is_trbe_running(trbsr));
-
 	if (is_trbe_abort(trbsr)) {
 		err_str = "External abort";
 		goto out_fatal;
@@ -672,8 +728,7 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand
 	case TRBE_BSC_FILLED:
 		break;
 	case TRBE_BSC_TRIGGERED:
-		err_str = "Unexpected trigger status";
-		goto out_fatal;
+		break;
 	default:
 		err_str = "Unexpected buffer status code";
 		goto out_fatal;
@@ -692,6 +747,9 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand
 	if (is_trbe_wrap(trbsr))
 		return TRBE_FAULT_ACT_WRAP;
 
+	if (is_trbe_trg(trbsr))
+		return TRBE_FAULT_ACT_TRIG;
+
 	return TRBE_FAULT_ACT_SPURIOUS;
 
 out_fatal:
@@ -1180,6 +1238,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 		clr_trbe_status();
 
 	switch (act) {
+	case TRBE_FAULT_ACT_TRIG:
 	case TRBE_FAULT_ACT_WRAP:
 		truncated = !!trbe_handle_overflow(handle, act);
 		break;
diff --git a/drivers/hwtracing/coresight/coresight-trbe.h b/drivers/hwtracing/coresight/coresight-trbe.h
index d7f7cd763c0c7139cf322b7336ee563073e3bea0..4c65d164a946ec9860825e7564196745b60d730b 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.h
+++ b/drivers/hwtracing/coresight/coresight-trbe.h
@@ -114,6 +114,20 @@ static inline void set_trbe_write_pointer(unsigned long addr)
 	write_sysreg_s(addr, SYS_TRBPTR_EL1);
 }
 
+static inline void set_trbe_trigger_count(unsigned long count)
+{
+	u64 trbsr;
+
+	write_sysreg_s(count, SYS_TRBTRG_EL1);
+
+	/* TRBSR_EL1.TRG has been cleared in clr_trbe_status() */
+	if (!count)
+		return;
+
+	trbsr = read_sysreg_s(SYS_TRBSR_EL1);
+	write_sysreg_s(trbsr | TRBSR_EL1_TRG, SYS_TRBSR_EL1);
+}
+
 static inline unsigned long get_trbe_limit_pointer(void)
 {
 	u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);

-- 
2.34.1

[PATCH 17/19] coresight: trbe: Enable circle mode for snapshot

[Leo Yan]

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

-- 
2.34.1

[PATCH 18/19] coresight: trbe: Add kunit tests

[Leo Yan]

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

-- 
2.34.1

[PATCH 19/19] perf: cs-etm: Set watermark for AUX trace

[Leo Yan]

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.

-- 
2.34.1

Re: [PATCH 19/19] perf: cs-etm: Set watermark for AUX trace

[Anshuman Khandual]

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.
> 

Re: [PATCH 19/19] perf: cs-etm: Set watermark for AUX trace

[James Clark]

On 05/12/2025 4:48 am, Anshuman Khandual wrote:
> 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 ?
> 

It's not really about the size of the buffer but where the wakeup points 
are. With only two wakeup points (middle and end) once you pass the 
first one you are immediately into the last chance end section.

Now the driver is writing into the end section, but Perf is still 
reading from the first one. Once the end one fills up, you end up with 
no space left without Perf having a chance to say it's finished reading.

To guarantee free space, there needs to be at least one additional 
section as a buffer zone. Ideally we'd set the wakeup point at thirds so 
there were fewer wakeups and still have one free section, but it might 
not always divide by 3, so 4 is also fine.

The commit message could probably explain it a bit better though.

The reason this is different now is because we want to be able to enable 
wrap mode as much as possible. If you imagine the scenario where a half 
way watermark is hit, you can't enable wrap mode without wrapping into 
Perf's part of the buffer (the beginning) as it's all that's left after 
the second half.

>>> 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.
>>
> 

Re: [PATCH 19/19] perf: cs-etm: Set watermark for AUX trace

[Anshuman Khandual]

On 09/12/25 8:24 PM, James Clark wrote:
> 
> 
> On 05/12/2025 4:48 am, Anshuman Khandual wrote:
>> 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 ?
>>
> 
> It's not really about the size of the buffer but where the wakeup points are. With only two wakeup points (middle and end) once you pass the first one you are immediately into the last chance end section.
> 
> Now the driver is writing into the end section, but Perf is still reading from the first one. Once the end one fills up, you end up with no space left without Perf having a chance to say it's finished reading.

Understood.

> 
> To guarantee free space, there needs to be at least one additional section as a buffer zone. Ideally we'd set the wakeup point at thirds so there were fewer wakeups and still have one free section, but it might not always divide by 3, so 4 is also fine.
> 
> The commit message could probably explain it a bit better though.

Agreed, the commit message should explain this problem in detail. Otherwise it appeared
as if the buffer sizing alone is impacting the perf records collection.

> 
> The reason this is different now is because we want to be able to enable wrap mode as much as possible. If you imagine the scenario where a half way watermark is hit, you can't enable wrap mode without wrapping into Perf's part of the buffer (the beginning) as it's all that's left after the second half.

Got it.

> 
>>>> 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.
>>>
>>
> 

Re: [PATCH 00/19] coresight: trbe: Support trigger and circle buffer modes

[Anshuman Khandual]

On 01/12/25 4:51 PM, Leo Yan wrote:
> The current TRBE driver operates only in fill mode, where tracing stops
> at the top of buffer and a maintenance interrupt is raised.  Due to
> interrupt latency, tracing is halted while the program continues to run,
> resulting in trace data lose.
> 
> This series enhances the driver for the trigger mode to mitigate trace
> discontinuity.  The circle buffer mode is introduced to avoid any
> maintenance interrupts during the snapshot session.
> 
> It can be divided into three parts for easier review:
> 
>   Patches 01~05: Minor refactoring for disabling operations and
>                  clearing status.

Could these patches be posted first ?

>   Patches 06~12: Refactor fault action and trace size calculation.

Followed by this set which still keeps the existing functionality intact.

>   Patches 13~19: Support trigger and circle modes. To better utilize the
>                  new buffer modes, perf sets the watermark to
> 		 one-quarter of the buffer size.

Then finally all above changes which are newer functions and tests etc.

I guess that will be much easier to review.

> 
> This series is applied on coresight-next branch and has been validated
> on Orion O6 board:
> 
>   1) The trigger count and wrap mode are verified using Ftrace logs.
>   2) A new kunit test module verifies limit and count calculations.
>   3) Basic perf record / script commands and module load/unload have
>      been tested successfully.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
> Leo Yan (19):
>       coresight: trbe: Use helpers for checking errata
>       coresight: trbe: Remove redundant disable operation
>       coresight: trbe: Remove buffer disabling in trbe_handle_overflow()
>       coresight: trbe: Remove set_trbe_disabled() from the enable flow
>       coresight: trbe: Refactor status clearing
>       coresight: trbe: Refactor syndrome decoding
>       coresight: trbe: Refactor AUX flag setting
>       coresight: trbe: Use PERF_AUX_FLAG_PARTIAL instead of PERF_AUX_FLAG_COLLISION
>       coresight: trbe: Add fault action argument to trbe_handle_overflow()
>       coresight: trbe: Always check fault action when updating buffer
>       coresight: trbe: Apply overwrite erratum for only wrap event
>       coresight: trbe: Calculate size for buffer wrapping
>       coresight: trbe: Remove misleading comment
>       coresight: trbe: Refactor compute_trbe_buffer_limit()
>       coresight: trbe: Add static key for bypassing trigger mode
>       coresight: trbe: Support trigger mode
>       coresight: trbe: Enable circle mode for snapshot
>       coresight: trbe: Add kunit tests
>       perf: cs-etm: Set watermark for AUX trace
> 
>  drivers/hwtracing/coresight/Kconfig                |   9 +
>  drivers/hwtracing/coresight/Makefile               |   1 +
>  .../coresight/coresight-trbe-kunit-tests.c         | 536 +++++++++++++++++++++
>  drivers/hwtracing/coresight/coresight-trbe.c       | 440 +++++++++--------
>  drivers/hwtracing/coresight/coresight-trbe.h       | 111 ++++-
>  tools/perf/arch/arm/util/cs-etm.c                  |   7 +
>  6 files changed, 896 insertions(+), 208 deletions(-)
> ---
> base-commit: 9e9182cab5ebc3ee7544e60ef08ba19fdf216920
> change-id: 20251120-trbe_buffer_refactor_v1-1-8f8023105469
> 
> Best regards,