Skip to contentSkip to conversation

coresight: trbe: Refactor buffer and IRQ handling

PATCH In review Leo Yan 3 participants 52 messages Oct 16 – Nov 11

Subsystems (3)
lore
Share & tools
lore

Leo Yan

author reviewer
#0 lore raw

v2 cover coresight: trbe: Refactor buffer and IRQ handling

This series follows the earlier TRBE buffer refactoring posting [1]. The work is now split into two parts. This first part contains fixes and refactoring in preparation for wrap and trigger mode support. Support for those modes will be sent separately.

The patches are grouped as follows for easier review:

Patches 1-5: Minor fixes
Add register definitions, fix the watermark range check, and improve erratum checks and perf handle validation.

Patches 6-11: Enable/disable lifecycle fixes
Clean up session setup, status clearing and IRQ handle ownership.

Patches 12-17: Refine status classification
Decode stop reasons, report truncation on errors and improve trace-size accounting.

Patches 18-23: Improve IRQ and overflow handling
Keep the handle in per-CPU data, stabilize status reads, and improve AUX restarts and trace-gap reporting.

A race also remains between TRBE interrupt handling and AUX pause/resume from a sampling NMI. This should be resolved as part of the state machine refactoring in the CoreSight AUX sampling series [2].

[1] https://lore.kernel.org/linux-arm-kernel/[email protected]/
[2] https://lore.kernel.org/linux-arm-kernel/[email protected]/T/#t

---
Leo Yan (23):
arm64/sysreg: Define TRBSR_EL1.EC event classes
coresight: trbe: Make watermark range check overflow safe in __trbe_normal_offset()
coresight: trbe: Use helpers for checking errata
coresight: trbe: Check the buffer association before IRQ validation
coresight: trbe: Validate perf handle before using sink data
coresight: trbe: Drop redundant disable from enable path
coresight: trbe: Publish the session after successful enable
coresight: trbe: Clear stale external abort status
coresight: trbe: Clear TRBSR status at stop sites
coresight: trbe: Drop redundant disables
coresight: trbe: Assign the perf handle in enable and disable
coresight: trbe: Decode TRBSR status codes
coresight: trbe: Simplify wrap status handling
coresight: trbe: Treat access not allowed stops as fatal
coresight: trbe: Report truncated trace on buffer update errors
coresight: trbe: Always classify TRBSR when updating buffer
coresight: trbe: Calculate trace size across buffer wrap
coresight: trbe: Keep the perf handle in per-CPU data
coresight: trbe: Reload the AUX buffer on overflow restart
coresight: trbe: Add fault action argument to trbe_handle_overflow()
coresight: trbe: Read TRBSR after prohibiting and flushing trace
coresight: trbe: Restart trace source for stopped buffers
coresight: trbe: Report trace gaps with PARTIAL instead of COLLISION

 arch/arm64/include/asm/sysreg.h              |   6 +
 arch/arm64/tools/sysreg                      |   8 +-
 drivers/hwtracing/coresight/coresight-trbe.c | 481 ++++++++++++++++++---------
 drivers/hwtracing/coresight/coresight-trbe.h |   8 -
 4 files changed, 345 insertions(+), 158 deletions(-)

---
base-commit: 343cfcbb3ecfdb80aa576a428cfa3b65ad783842
change-id: 20261016-trbe_enable_disable_irq_handle_refactor-0a3450f03473

Best regards,

Signature · 1 line
Leo Yan <[email protected]>
author reviewer
#1 lore raw

PATCH v2 1/23 arm64/sysreg: Define TRBSR_EL1.EC event classes

TRBSR_EL1.EC identifies the cause of a trace buffer management event, but the sysreg description does not name its architected values. Define the five encodings as an unsigned enum so TRBE status handling can refer to each cause by name.

The values cover other events, Granule Protection Check faults, implementation-defined events, and stage 1 and stage 2 Data Aborts on writes to the trace buffer, as specified by Arm ARM (ARM DDI 0487 M.c).

---
 arch/arm64/tools/sysreg | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
arch/arm64/tools/sysreg +7 -1
index 94bf065c8ac71f20f482c91a5147cb6e001dfba2..f68c23dd4e83ee4d00610e097d9188d3a2c9ed15 100644
@@ -5834,7 +5834,13 @@ EndSysreg
 Sysreg	TRBSR_EL1	3	0	9	11	3
 Res0	63:56
 Field	55:32	MSS2
-Field	31:26	EC
+UnsignedEnum	31:26	EC
+	0b000000	OTHER
+	0b011110	FAULT_GPC
+	0b011111	IMP_DEF
+	0b100100	FAULT_WRITE_S1
+	0b100101	FAULT_WRITE_S2
+EndEnum
 Res0	25:24
 Field	23	DAT
 Field	22	IRQ
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 1/23
#2 lore raw

Re: [PATCH v2 01/23] arm64/sysreg: Define TRBSR_EL1.EC event classes

Quoted 24 lines · Leo Yan

TRBSR_EL1.EC identifies the cause of a trace buffer management event, but the sysreg description does not name its architected values. Define the five encodings as an unsigned enum so TRBE status handling can refer to each cause by name.

The values cover other events, Granule Protection Check faults, implementation-defined events, and stage 1 and stage 2 Data Aborts on writes to the trace buffer, as specified by Arm ARM (ARM DDI 0487 M.c).

---
  arch/arm64/tools/sysreg | 8 +++++++-
  1 file changed, 7 insertions(+), 1 deletion(-)
arch/arm64/tools/sysreg +0 -1
index 94bf065c8ac71f20f482c91a5147cb6e001dfba2..f68c23dd4e83ee4d00610e097d9188d3a2c9ed15 100644
@@ -5834,7 +5834,13 @@ EndSysreg
  Sysreg	TRBSR_EL1	3	0	9	11	3
  Res0	63:56
  Field	55:32	MSS2
-Field	31:26	EC

arch/arm64/tools/sysreg EndSysreg · line 5846

Quoted 10 lines
arch/arm64/tools/sysreg +7 -0
+UnsignedEnum	31:26	EC
+	0b000000	OTHER
+	0b011110	FAULT_GPC
+	0b011111	IMP_DEF
+	0b100100	FAULT_WRITE_S1
+	0b100101	FAULT_WRITE_S2
+EndEnum
  Res0	25:24
  Field	23	DAT
  Field	22	IRQ
author reviewer
#3 lore raw

PATCH v2 2/23 coresight: trbe: Make watermark range check overflow safe in __trbe_normal_offset()

In reply to Leo Yan

The current code checks whether the wakeup point lies before the current tail by comparing against handle->head + handle->size. That addition can overflow when the monotonically increasing perf AUX head is close to ULONG_MAX, causing a valid wakeup point in the current writable range to be missed.

Use unsigned subtraction to compare the distance from head to wakeup against the buffer size instead. This keeps the check correct across potential wrap.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/hwtracing/coresight/coresight-trbe.c +1 -1
index eca8e13a97447b46a4b383049aacb3fa7ad3c329..02d12445763a86d58c79bdf5143f761827456bc3 100644
@@ -526,7 +526,7 @@ static unsigned long __trbe_normal_offset(struct perf_output_handle *handle)
 	 * +----|---------------|-------|-------+
 	 * trbe_base		limit		trbe_base + nr_pages
 	 */
-	if (handle->wakeup < (handle->head + handle->size) && head <= wakeup)
+	if ((handle->wakeup - handle->head) < handle->size && head <= wakeup)
 		limit = min(limit, round_up(wakeup, PAGE_SIZE));
 
 	/*
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 2/23
#4 lore raw

Re: [PATCH v2 02/23] coresight: trbe: Make watermark range check overflow safe in __trbe_normal_offset()

Quoted 21 lines · Leo Yan

The current code checks whether the wakeup point lies before the current tail by comparing against handle->head + handle->size. That addition can overflow when the monotonically increasing perf AUX head is close to ULONG_MAX, causing a valid wakeup point in the current writable range to be missed.

Use unsigned subtraction to compare the distance from head to wakeup against the buffer size instead. This keeps the check correct across potential wrap.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/hwtracing/coresight/coresight-trbe.c +0 -0
index eca8e13a97447b46a4b383049aacb3fa7ad3c329..02d12445763a86d58c79bdf5143f761827456bc3 100644

drivers/hwtracing/coresight/coresight-trbe.c static unsigned long __trbe_normal_offset(struct perf_output_handle *handle) · line 532

Quoted 8 lines
drivers/hwtracing/coresight/coresight-trbe.c +1 -1
@@ -526,7 +526,7 @@ static unsigned long __trbe_normal_offset(struct perf_output_handle *handle)
  	 * +----|---------------|-------|-------+
  	 * trbe_base		limit		trbe_base + nr_pages
  	 */
-	if (handle->wakeup < (handle->head + handle->size) && head <= wakeup)
+	if ((handle->wakeup - handle->head) < handle->size && head <= wakeup)
  		limit = min(limit, round_up(wakeup, PAGE_SIZE));
 
  	/*
author reviewer
#5 lore raw

PATCH v2 3/23 coresight: trbe: Use helpers for checking errata

In reply to Leo Yan

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

---
 drivers/hwtracing/coresight/coresight-trbe.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +3 -3
index 02d12445763a86d58c79bdf5143f761827456bc3..8ec0b4c4987f25d10ee0b2f80c27e10f76357387 100644
@@ -738,7 +738,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);
 
@@ -961,7 +961,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;
@@ -985,7 +985,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
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 3/23
#6 lore raw

Re: [PATCH v2 03/23] coresight: trbe: Use helpers for checking errata

Quoted 33 lines · Leo Yan

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

---
  drivers/hwtracing/coresight/coresight-trbe.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +2 -2
index 02d12445763a86d58c79bdf5143f761827456bc3..8ec0b4c4987f25d10ee0b2f80c27e10f76357387 100644
@@ -738,7 +738,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);
 
@@ -961,7 +961,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;

drivers/hwtracing/coresight/coresight-trbe.c static int trbe_apply_work_around_before_enable(struct trbe_buf *buf) · line 991

Quoted 8 lines
drivers/hwtracing/coresight/coresight-trbe.c +1 -1
@@ -985,7 +985,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
author reviewer
#7 lore raw

PATCH v2 4/23 coresight: trbe: Check the buffer association before IRQ validation

In reply to Leo Yan

is_perf_trbe() obtains the sink buffer from the output handle and dereferences buf->cpudata before validating either pointer. A live AUX transaction does not guarantee that the selected buffer has a CPU association.

Reject a missing buffer or CPU association before reading CPU driver data or checking the hardware buffer addresses. Treat these configurations as an invalid perf TRBE session.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +8 -2
index 8ec0b4c4987f25d10ee0b2f80c27e10f76357387..91fbdbad3676ffae399df0b4d4ddcd603a40e86c 100644
@@ -1111,10 +1111,16 @@ static int trbe_handle_overflow(struct perf_output_handle *handle)
 static bool is_perf_trbe(struct perf_output_handle *handle)
 {
 	struct trbe_buf *buf = etm_perf_sink_config(handle);
-	struct trbe_cpudata *cpudata = buf->cpudata;
-	struct trbe_drvdata *drvdata = cpudata->drvdata;
+	struct trbe_cpudata *cpudata;
+	struct trbe_drvdata *drvdata;
 	int cpu = smp_processor_id();
 
+	if (!buf || !buf->cpudata)
+		return false;
+
+	cpudata = buf->cpudata;
+	drvdata = cpudata->drvdata;
+
 	WARN_ON(buf->trbe_hw_base != get_trbe_base_pointer());
 	WARN_ON(buf->trbe_limit != get_trbe_limit_pointer());
 
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 4/23
#8 lore raw

Re: [PATCH v2 04/23] coresight: trbe: Check the buffer association before IRQ validation

Quoted 28 lines · Leo Yan

is_perf_trbe() obtains the sink buffer from the output handle and dereferences buf->cpudata before validating either pointer. A live AUX transaction does not guarantee that the selected buffer has a CPU association.

Reject a missing buffer or CPU association before reading CPU driver data or checking the hardware buffer addresses. Treat these configurations as an invalid perf TRBE session.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 10 ++++++++--
  1 file changed, 8 insertions(+), 2 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +2 -2
index 8ec0b4c4987f25d10ee0b2f80c27e10f76357387..91fbdbad3676ffae399df0b4d4ddcd603a40e86c 100644
@@ -1111,10 +1111,16 @@ static int trbe_handle_overflow(struct perf_output_handle *handle)
  static bool is_perf_trbe(struct perf_output_handle *handle)
  {
  	struct trbe_buf *buf = etm_perf_sink_config(handle);
-	struct trbe_cpudata *cpudata = buf->cpudata;
-	struct trbe_drvdata *drvdata = cpudata->drvdata;
+	struct trbe_cpudata *cpudata;
+	struct trbe_drvdata *drvdata;
  	int cpu = smp_processor_id();

drivers/hwtracing/coresight/coresight-trbe.c static int trbe_handle_overflow(struct perf_output_handle *handle) · line 1126

Quoted 10 lines
drivers/hwtracing/coresight/coresight-trbe.c +6 -0
 
+	if (!buf || !buf->cpudata)
+		return false;
+
+	cpudata = buf->cpudata;
+	drvdata = cpudata->drvdata;
+
  	WARN_ON(buf->trbe_hw_base != get_trbe_base_pointer());
  	WARN_ON(buf->trbe_limit != get_trbe_limit_pointer());
 
author reviewer
#9 lore raw

PATCH v2 5/23 coresight: trbe: Validate perf handle before using sink data

In reply to Leo Yan

The TRBE IRQ handler calls etm_perf_sink_config(handle) before validating the per-CPU perf handle and its AUX transaction. This may dereference a NULL handle after disabling TRBE or access stale sink data after perf_aux_output_end().

Validate the handle and AUX transaction before accessing the sink configuration. Move the local buffer lookup closer to its use. Remove the warning for a NULL handle, which is expected after disabling TRBE.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +8 -7
index 91fbdbad3676ffae399df0b4d4ddcd603a40e86c..a7c1037a048f5236598f8f3a7cc4f47bff9d329c 100644
@@ -1150,12 +1150,18 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 {
 	struct perf_output_handle **handle_ptr = dev;
 	struct perf_output_handle *handle = *handle_ptr;
-	struct trbe_buf *buf = etm_perf_sink_config(handle);
+	struct trbe_buf *buf;
 	enum trbe_fault_action act;
 	u64 status;
 	bool truncated = false;
 	u64 trfcr;
 
+	if (!handle || !perf_get_aux(handle))
+		return IRQ_NONE;
+
+	if (!is_perf_trbe(handle))
+		return IRQ_NONE;
+
 	/* Reads to TRBSR_EL1 is fine when TRBE is active */
 	status = read_sysreg_s(SYS_TRBSR_EL1);
 	/*
@@ -1171,16 +1177,11 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 	 * Ensure the trace is visible to the CPUs and
 	 * any external aborts have been resolved.
 	 */
+	buf = etm_perf_sink_config(handle);
 	trbe_drain_and_disable_local(buf->cpudata);
 	clr_trbe_irq();
 	isb();
 
-	if (WARN_ON_ONCE(!handle) || !perf_get_aux(handle))
-		return IRQ_NONE;
-
-	if (!is_perf_trbe(handle))
-		return IRQ_NONE;
-
 	act = trbe_get_fault_act(handle, status);
 	switch (act) {
 	case TRBE_FAULT_ACT_WRAP:
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 5/23
#10 lore raw

Re: [PATCH v2 05/23] coresight: trbe: Validate perf handle before using sink data

Quoted 48 lines · Leo Yan

The TRBE IRQ handler calls etm_perf_sink_config(handle) before validating the per-CPU perf handle and its AUX transaction. This may dereference a NULL handle after disabling TRBE or access stale sink data after perf_aux_output_end().

Validate the handle and AUX transaction before accessing the sink configuration. Move the local buffer lookup closer to its use. Remove the warning for a NULL handle, which is expected after disabling TRBE.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 15 ++++++++-------
  1 file changed, 8 insertions(+), 7 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +8 -1
index 91fbdbad3676ffae399df0b4d4ddcd603a40e86c..a7c1037a048f5236598f8f3a7cc4f47bff9d329c 100644
@@ -1150,12 +1150,18 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
  {
  	struct perf_output_handle **handle_ptr = dev;
  	struct perf_output_handle *handle = *handle_ptr;
-	struct trbe_buf *buf = etm_perf_sink_config(handle);
+	struct trbe_buf *buf;
  	enum trbe_fault_action act;
  	u64 status;
  	bool truncated = false;
  	u64 trfcr;
 
+	if (!handle || !perf_get_aux(handle))
+		return IRQ_NONE;
+
+	if (!is_perf_trbe(handle))
+		return IRQ_NONE;
+
  	/* Reads to TRBSR_EL1 is fine when TRBE is active */
  	status = read_sysreg_s(SYS_TRBSR_EL1);
  	/*
@@ -1171,16 +1177,11 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
  	 * Ensure the trace is visible to the CPUs and
  	 * any external aborts have been resolved.
  	 */
+	buf = etm_perf_sink_config(handle);
  	trbe_drain_and_disable_local(buf->cpudata);
  	clr_trbe_irq();
  	isb();

drivers/hwtracing/coresight/coresight-trbe.c static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) · line 1187

Quoted 10 lines
drivers/hwtracing/coresight/coresight-trbe.c +0 -6
 
-	if (WARN_ON_ONCE(!handle) || !perf_get_aux(handle))
-		return IRQ_NONE;
-
-	if (!is_perf_trbe(handle))
-		return IRQ_NONE;
-
  	act = trbe_get_fault_act(handle, status);
  	switch (act) {
  	case TRBE_FAULT_ACT_WRAP:
author reviewer
#11 lore raw

PATCH v2 6/23 coresight: trbe: Drop redundant disable from enable path

In reply to Leo Yan

The Arm ARM (ARM DDI 0487 M.c) rule RDJMDD says:

 | When the following conditions apply, the PE might ignore a direct or
 | external write to any of certain Trace Buffer Unit registers, other
 | than a direct write or external write to TRBLIMITR_EL1 that clears
 | the conditions:
 | • TRBLIMITR_EL1.E is 1, and either FEAT_TRBE_EXT is not implemented
 |   or the Trace Buffer Unit is using Self-hosted mode.

trbe_enable_hw() calls set_trbe_disabled() first to ensure TRBE has already been disabled before programming any other TRBE registers.

However, TRBE is already expected to be disabled before trbe_enable_hw() runs:

1. CPU probe resets TRBLIMITR_EL1 via trbe_reset_local();
2. Interrupt handling disables the buffer before re-enabling it;
3. The normal disable path clears TRBLIMITR_EL1.E.

Calling set_trbe_disabled() from the enable path is therefore redundant. Drop the call from trbe_enable_hw(), which only needs to program a known-disabled TRBE.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 1 -
 1 file changed, 1 deletion(-)
drivers/hwtracing/coresight/coresight-trbe.c +0 -1
index a7c1037a048f5236598f8f3a7cc4f47bff9d329c..979d0fa84c1c7b448c1ce5cd7a37dff8845795ef 100644
@@ -652,7 +652,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);
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 6/23
#12 lore raw

Re: [PATCH v2 06/23] coresight: trbe: Drop redundant disable from enable path

Quoted 9 lines · Leo Yan

The Arm ARM (ARM DDI 0487 M.c) rule RDJMDD says:

  | When the following conditions apply, the PE might ignore a direct or
  | external write to any of certain Trace Buffer Unit registers, other
  | than a direct write or external write to TRBLIMITR_EL1 that clears
  | the conditions:
  | • TRBLIMITR_EL1.E is 1, and either FEAT_TRBE_EXT is not implemented
  |   or the Trace Buffer Unit is using Self-hosted mode.

Doesn't the rule just say that the buffer must be disabled before doing writes to other registers? But the explanation for removing the extra disable is that it's already disabled at this point, so nothing changes.

The commit message would be clearer without this bit IMO. There's no architectural requirement to make this change, it's only a software one.

Otherwise:

Quoted 32 lines

trbe_enable_hw() calls set_trbe_disabled() first to ensure TRBE has already been disabled before programming any other TRBE registers.

However, TRBE is already expected to be disabled before trbe_enable_hw() runs:

1. CPU probe resets TRBLIMITR_EL1 via trbe_reset_local();
2. Interrupt handling disables the buffer before re-enabling it;
3. The normal disable path clears TRBLIMITR_EL1.E.

Calling set_trbe_disabled() from the enable path is therefore redundant. Drop the call from trbe_enable_hw(), which only needs to program a known-disabled TRBE.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 1 -
  1 file changed, 1 deletion(-)
drivers/hwtracing/coresight/coresight-trbe.c +0 -1
index a7c1037a048f5236598f8f3a7cc4f47bff9d329c..979d0fa84c1c7b448c1ce5cd7a37dff8845795ef 100644
@@ -652,7 +652,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);
author reviewer
#13 lore raw

PATCH v2 7/23 coresight: trbe: Publish the session after successful enable

In reply to Leo Yan

arm_trbe_enable() records the session buffer and perf mode before __arm_trbe_enable() can fail. CoreSight skips the sink disable callback when sink enable fails, leaving that session state published even though tracing did not start.

Install buf->cpudata temporarily for the buffer setup helpers, then call __arm_trbe_enable() before assigning cpudata->buf and cpudata->mode. Clear the temporary buffer association if setup fails; the inner helper still truncates and ends the AUX transaction.

After successful enable, retain the buffer association across AUX transactions and clear it in arm_trbe_disable().

---
 drivers/hwtracing/coresight/coresight-trbe.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +12 -3
index 979d0fa84c1c7b448c1ce5cd7a37dff8845795ef..9dfc6bb7afb4c42dfdec643ee7e4f2f193e2b70c 100644
@@ -69,6 +69,7 @@ struct trbe_buf {
 	int nr_pages;
 	void **pages;
 	bool snapshot;
+	/* CPU used to configure this buffer. */
 	struct trbe_cpudata *cpudata;
 };
 
@@ -137,6 +138,7 @@ struct trbe_save_state {
  * @trbe_align		- Software alignment used for the TRBPTR_EL1.
  * @cpu			- CPU this TRBE belongs to.
  * @mode		- Mode of current operation. (perf/disabled)
+ * @buf			- Buffer configuration retained for the enabled session.
  * @drvdata		- TRBE specific drvdata
  * @errata		- Bit map for the errata on this TRBE.
  */
@@ -1033,17 +1035,24 @@ static int arm_trbe_enable(struct coresight_device *csdev, enum cs_mode mode,
 	struct trbe_cpudata *cpudata = dev_get_drvdata(&csdev->dev);
 	struct perf_output_handle *handle = path->handle;
 	struct trbe_buf *buf = etm_perf_sink_config(handle);
+	int ret;
 
 	WARN_ON(cpudata->cpu != smp_processor_id());
 	WARN_ON(cpudata->drvdata != drvdata);
 	if (mode != CS_MODE_PERF)
 		return -EINVAL;
 
-	cpudata->buf = buf;
-	cpudata->mode = mode;
+	/* Buffer helpers need the CPU association while setting up tracing. */
 	buf->cpudata = cpudata;
+	ret = __arm_trbe_enable(buf, handle);
+	if (ret) {
+		buf->cpudata = NULL;
+		return ret;
+	}
 
-	return __arm_trbe_enable(buf, handle);
+	cpudata->buf = buf;
+	cpudata->mode = mode;
+	return 0;
 }
 
 static int arm_trbe_disable(struct coresight_device *csdev)
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 7/23
#14 lore raw

Re: [PATCH v2 07/23] coresight: trbe: Publish the session after successful enable

Quoted 57 lines · Leo Yan

arm_trbe_enable() records the session buffer and perf mode before __arm_trbe_enable() can fail. CoreSight skips the sink disable callback when sink enable fails, leaving that session state published even though tracing did not start.

Install buf->cpudata temporarily for the buffer setup helpers, then call __arm_trbe_enable() before assigning cpudata->buf and cpudata->mode. Clear the temporary buffer association if setup fails; the inner helper still truncates and ends the AUX transaction.

After successful enable, retain the buffer association across AUX transactions and clear it in arm_trbe_disable().

---
  drivers/hwtracing/coresight/coresight-trbe.c | 15 ++++++++++++---
  1 file changed, 12 insertions(+), 3 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +7 -2
index 979d0fa84c1c7b448c1ce5cd7a37dff8845795ef..9dfc6bb7afb4c42dfdec643ee7e4f2f193e2b70c 100644
@@ -69,6 +69,7 @@ struct trbe_buf {
  	int nr_pages;
  	void **pages;
  	bool snapshot;
+	/* CPU used to configure this buffer. */
  	struct trbe_cpudata *cpudata;
  };
 
@@ -137,6 +138,7 @@ struct trbe_save_state {
   * @trbe_align		- Software alignment used for the TRBPTR_EL1.
   * @cpu			- CPU this TRBE belongs to.
   * @mode		- Mode of current operation. (perf/disabled)
+ * @buf			- Buffer configuration retained for the enabled session.
   * @drvdata		- TRBE specific drvdata
   * @errata		- Bit map for the errata on this TRBE.
   */
@@ -1033,17 +1035,24 @@ static int arm_trbe_enable(struct coresight_device *csdev, enum cs_mode mode,
  	struct trbe_cpudata *cpudata = dev_get_drvdata(&csdev->dev);
  	struct perf_output_handle *handle = path->handle;
  	struct trbe_buf *buf = etm_perf_sink_config(handle);
+	int ret;
 
  	WARN_ON(cpudata->cpu != smp_processor_id());
  	WARN_ON(cpudata->drvdata != drvdata);
  	if (mode != CS_MODE_PERF)
  		return -EINVAL;
 
-	cpudata->buf = buf;
-	cpudata->mode = mode;
+	/* Buffer helpers need the CPU association while setting up tracing. */
  	buf->cpudata = cpudata;
+	ret = __arm_trbe_enable(buf, handle);
+	if (ret) {
+		buf->cpudata = NULL;

drivers/hwtracing/coresight/coresight-trbe.c static int arm_trbe_enable(struct coresight_device *csdev, enum cs_mode mode, · line 1058

Quoted 10 lines
drivers/hwtracing/coresight/coresight-trbe.c +5 -1
+		return ret;
+	}
 
-	return __arm_trbe_enable(buf, handle);
+	cpudata->buf = buf;
+	cpudata->mode = mode;
+	return 0;
  }
 
  static int arm_trbe_disable(struct coresight_device *csdev)
author reviewer
#15 lore raw

PATCH v2 8/23 coresight: trbe: Clear stale external abort status

In reply to Leo Yan

clr_trbe_status() clears the buffer status and syndrome fields, but leaves TRBSR_EL1.EA unchanged. After an external abort, this indication can survive into a later perf session on the same CPU.

Clear EA along with the other status fields while TRBE is disabled. The status captured for the current fault remains available for classification, so external aborts are still handled as fatal.

Fixes 3fbf7f011f24 ("coresight: sink: Add TRBE driver")
---
 drivers/hwtracing/coresight/coresight-trbe.c | 1 +
 1 file changed, 1 insertion(+)
drivers/hwtracing/coresight/coresight-trbe.c +1 -0
index 9dfc6bb7afb4c42dfdec643ee7e4f2f193e2b70c..26e7554d6bdd31f8a55a8bc3a7b6d4c49b984b9b 100644
@@ -608,6 +608,7 @@ static void clr_trbe_status(void)
 	trbsr &= ~TRBSR_EL1_IRQ;
 	trbsr &= ~TRBSR_EL1_TRG;
 	trbsr &= ~TRBSR_EL1_WRAP;
+	trbsr &= ~TRBSR_EL1_EA;
 	trbsr &= ~TRBSR_EL1_EC_MASK;
 	trbsr &= ~TRBSR_EL1_BSC_MASK;
 	trbsr &= ~TRBSR_EL1_S;
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 8/23
#16 lore raw

Re: [PATCH v2 08/23] coresight: trbe: Clear stale external abort status

Quoted 20 lines · Leo Yan

clr_trbe_status() clears the buffer status and syndrome fields, but leaves TRBSR_EL1.EA unchanged. After an external abort, this indication can survive into a later perf session on the same CPU.

Clear EA along with the other status fields while TRBE is disabled. The status captured for the current fault remains available for classification, so external aborts are still handled as fatal.

Fixes 3fbf7f011f24 ("coresight: sink: Add TRBE driver")
---
  drivers/hwtracing/coresight/coresight-trbe.c | 1 +
  1 file changed, 1 insertion(+)
drivers/hwtracing/coresight/coresight-trbe.c +0 -0
index 9dfc6bb7afb4c42dfdec643ee7e4f2f193e2b70c..26e7554d6bdd31f8a55a8bc3a7b6d4c49b984b9b 100644

drivers/hwtracing/coresight/coresight-trbe.c static void clr_trbe_status(void) · line 614

Quoted 7 lines
drivers/hwtracing/coresight/coresight-trbe.c +1 -0
@@ -608,6 +608,7 @@ static void clr_trbe_status(void)
  	trbsr &= ~TRBSR_EL1_IRQ;
  	trbsr &= ~TRBSR_EL1_TRG;
  	trbsr &= ~TRBSR_EL1_WRAP;
+	trbsr &= ~TRBSR_EL1_EA;
  	trbsr &= ~TRBSR_EL1_EC_MASK;
  	trbsr &= ~TRBSR_EL1_BSC_MASK;
  	trbsr &= ~TRBSR_EL1_S;
author reviewer
#17 lore raw

PATCH v2 9/23 coresight: trbe: Clear TRBSR status at stop sites

In reply to Leo Yan

TRBSR_EL1 records the status of the buffer that just stopped. Clearing it in trbe_enable_hw() postpones cleanup until another run starts. If AUX output cannot restart, for example when AUX space is exhausted, that clear never executes. Clear the status in arm_trbe_disable() after disabling TRBE instead.

trbe_handle_overflow() can start the next run without calling arm_trbe_disable(). Clear the stopped buffer's syndrome and status bits while TRBE is disabled, before attempting the AUX restart. This also leaves the status clean when the restart fails.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/hwtracing/coresight/coresight-trbe.c +9 -1
index 26e7554d6bdd31f8a55a8bc3a7b6d4c49b984b9b..dd9c9330bb0c739795f300dafc6ce34e72fbf280 100644
@@ -655,7 +655,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);
 
@@ -1069,6 +1068,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;
@@ -1095,6 +1096,13 @@ static int trbe_handle_overflow(struct perf_output_handle *handle)
 	unsigned long size;
 	struct etm_event_data *event_data;
 
+	/*
+	 * Clear the status. No context synchronization is required here,
+	 * since context synchronization will be performed before TRBE is
+	 * re-enabled (see trbe_enable_hw()).
+	 */
+	clr_trbe_status();
+
 	size = trbe_get_trace_size(handle, buf, true);
 	if (buf->snapshot)
 		handle->head += size;
Signature · 1 line
2.34.1
reviewer
#18 lore raw

Re: [PATCH v2 09/23] coresight: trbe: Clear TRBSR status at stop sites

Quoted 5 lines · Leo Yan

TRBSR_EL1 records the status of the buffer that just stopped. Clearing it in trbe_enable_hw() postpones cleanup until another run starts. If AUX output cannot restart, for example when AUX space is exhausted, that clear never executes. Clear the status in arm_trbe_disable() after disabling TRBE instead.

Why is leaving it set a problem though? That's not mentioned. Maybe there is some later patch that requires it?

It seems like clearing on enable is cleaner because it only requires one call to clear, but this replaces it with two separate calls without really saying what the benefit of the extra complexity is.

Quoted 47 lines

trbe_handle_overflow() can start the next run without calling arm_trbe_disable(). Clear the stopped buffer's syndrome and status bits while TRBE is disabled, before attempting the AUX restart. This also leaves the status clean when the restart fails.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 10 +++++++++-
  1 file changed, 9 insertions(+), 1 deletion(-)
drivers/hwtracing/coresight/coresight-trbe.c +9 -1
index 26e7554d6bdd31f8a55a8bc3a7b6d4c49b984b9b..dd9c9330bb0c739795f300dafc6ce34e72fbf280 100644
@@ -655,7 +655,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);
 
@@ -1069,6 +1068,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;
@@ -1095,6 +1096,13 @@ static int trbe_handle_overflow(struct perf_output_handle *handle)
  	unsigned long size;
  	struct etm_event_data *event_data;
 
+	/*
+	 * Clear the status. No context synchronization is required here,
+	 * since context synchronization will be performed before TRBE is
+	 * re-enabled (see trbe_enable_hw()).
+	 */
+	clr_trbe_status();
+
  	size = trbe_get_trace_size(handle, buf, true);
  	if (buf->snapshot)
  		handle->head += size;
reviewer
#19 lore raw

Re: [PATCH v2 09/23] coresight: trbe: Clear TRBSR status at stop sites

Quoted 9 lines · James Clark
Quoted 5 lines · Leo Yan

TRBSR_EL1 records the status of the buffer that just stopped. Clearing it in trbe_enable_hw() postpones cleanup until another run starts. If AUX output cannot restart, for example when AUX space is exhausted, that clear never executes. Clear the status in arm_trbe_disable() after disabling TRBE instead.

Quoted 4 lines

Why is leaving it set a problem though? That's not mentioned. Maybe there is some later patch that requires it?

It seems like clearing on enable is cleaner because it only requires one call to clear, but this replaces it with two separate calls without really saying what the benefit of the extra complexity is.

I see Anshuman left a similar comment on V1 and you said there was a race condition. But that explanation wasn't added to the commit message, and I'm not sure if it exists anyway. A late IRQ after already disabling the buffer would just hit the check for a NULL aux handle and not run the handler.

Quoted 47 lines
Quoted 46 lines

trbe_handle_overflow() can start the next run without calling arm_trbe_disable(). Clear the stopped buffer's syndrome and status bits while TRBE is disabled, before attempting the AUX restart. This also leaves the status clean when the restart fails.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 10 +++++++++-

  1 file changed, 9 insertions(+), 1 deletion(-)
drivers/hwtracing/coresight/coresight-trbe.c +9 -1
index 26e7554d6bdd31f8a55a8bc3a7b6d4c49b984b9b..dd9c9330bb0c739795f300dafc6ce34e72fbf280 100644
@@ -655,7 +655,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);
@@ -1069,6 +1068,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;
@@ -1095,6 +1096,13 @@ static int trbe_handle_overflow(struct 
 perf_output_handle *handle)
       unsigned long size;
       struct etm_event_data *event_data;
+    /*
+     * Clear the status. No context synchronization is required here,
+     * since context synchronization will be performed before TRBE is
+     * re-enabled (see trbe_enable_hw()).
+     */
+    clr_trbe_status();
+
       size = trbe_get_trace_size(handle, buf, true);
       if (buf->snapshot)
           handle->head += size;
maintainer
#20 lore raw

Re: [PATCH v2 09/23] coresight: trbe: Clear TRBSR status at stop sites

In reply to Leo Yan
Quoted 21 lines · Leo Yan

TRBSR_EL1 records the status of the buffer that just stopped. Clearing it in trbe_enable_hw() postpones cleanup until another run starts. If AUX output cannot restart, for example when AUX space is exhausted, that clear never executes. Clear the status in arm_trbe_disable() after disabling TRBE instead.

trbe_handle_overflow() can start the next run without calling arm_trbe_disable(). Clear the stopped buffer's syndrome and status bits while TRBE is disabled, before attempting the AUX restart. This also leaves the status clean when the restart fails.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 10 +++++++++-
  1 file changed, 9 insertions(+), 1 deletion(-)
drivers/hwtracing/coresight/coresight-trbe.c +0 -0
index 26e7554d6bdd31f8a55a8bc3a7b6d4c49b984b9b..dd9c9330bb0c739795f300dafc6ce34e72fbf280 100644

drivers/hwtracing/coresight/coresight-trbe.c static void trbe_enable_hw(struct trbe_buf *buf) · line 658

Quoted 4 lines
drivers/hwtracing/coresight/coresight-trbe.c +0 -1
@@ -655,7 +655,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();

TRBSR_ELx is UNKNOWN at reset and if we don't clear it at probe time, we endup running the first session with unknown values. It is safer to clear this at enable_hw.

Suzuki

Quoted 27 lines
drivers/hwtracing/coresight/coresight-trbe.c +9 -0
  	set_trbe_base_pointer(buf->trbe_hw_base);
  	set_trbe_write_pointer(buf->trbe_write);
 
@@ -1069,6 +1068,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;
@@ -1095,6 +1096,13 @@ static int trbe_handle_overflow(struct perf_output_handle *handle)
  	unsigned long size;
  	struct etm_event_data *event_data;
 
+	/*
+	 * Clear the status. No context synchronization is required here,
+	 * since context synchronization will be performed before TRBE is
+	 * re-enabled (see trbe_enable_hw()).
+	 */
+	clr_trbe_status();
+
  	size = trbe_get_trace_size(handle, buf, true);
  	if (buf->snapshot)
  		handle->head += size;
author reviewer
#21 lore raw

Re: [PATCH v2 09/23] coresight: trbe: Clear TRBSR status at stop sites

Quoted 17 lines · Suzuki K Poulose
Quoted 16 lines · Leo Yan

TRBSR_EL1 records the status of the buffer that just stopped. Clearing it in trbe_enable_hw() postpones cleanup until another run starts. If AUX output cannot restart, for example when AUX space is exhausted, that clear never executes. Clear the status in arm_trbe_disable() after disabling TRBE instead.

trbe_handle_overflow() can start the next run without calling arm_trbe_disable(). Clear the stopped buffer's syndrome and status bits while TRBE is disabled, before attempting the AUX restart. This also leaves the status clean when the restart fails.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 10 +++++++++-
  1 file changed, 9 insertions(+), 1 deletion(-)
Quoted 13 lines

drivers/hwtracing/coresight/coresight-trbe.c static void trbe_enable_hw(struct trbe_buf *buf) · line 658

Quoted 9 lines
drivers/hwtracing/coresight/coresight-trbe.c +0 -1
index 26e7554d6bdd31f8a55a8bc3a7b6d4c49b984b9b..dd9c9330bb0c739795f300dafc6ce34e72fbf280 100644
@@ -655,7 +655,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();

TRBSR_ELx is UNKNOWN at reset and if we don't clear it at probe time, we endup running the first session with unknown values. It is safer to clear this at enable_hw.

This is incorrect. trbe_reset_local() is called locally on each CPU to clear TRBSR_ELx, it has been guaranteed when the first session starts.

Thanks,
Leo

maintainer
#22 lore raw

Re: [PATCH v2 09/23] coresight: trbe: Clear TRBSR status at stop sites

Quoted 32 lines · Leo Yan
Quoted 17 lines · Suzuki K Poulose
Quoted 16 lines · Leo Yan

TRBSR_EL1 records the status of the buffer that just stopped. Clearing it in trbe_enable_hw() postpones cleanup until another run starts. If AUX output cannot restart, for example when AUX space is exhausted, that clear never executes. Clear the status in arm_trbe_disable() after disabling TRBE instead.

trbe_handle_overflow() can start the next run without calling arm_trbe_disable(). Clear the stopped buffer's syndrome and status bits while TRBE is disabled, before attempting the AUX restart. This also leaves the status clean when the restart fails.

---
   drivers/hwtracing/coresight/coresight-trbe.c | 10 +++++++++-
   1 file changed, 9 insertions(+), 1 deletion(-)
Quoted 13 lines

drivers/hwtracing/coresight/coresight-trbe.c static void trbe_enable_hw(struct trbe_buf *buf) · line 658

Quoted 9 lines
drivers/hwtracing/coresight/coresight-trbe.c +0 -1
index 26e7554d6bdd31f8a55a8bc3a7b6d4c49b984b9b..dd9c9330bb0c739795f300dafc6ce34e72fbf280 100644
@@ -655,7 +655,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();

TRBSR_ELx is UNKNOWN at reset and if we don't clear it at probe time, we endup running the first session with unknown values. It is safer to clear this at enable_hw.

Quoted 3 lines

This is incorrect. trbe_reset_local() is called locally on each CPU to clear TRBSR_ELx, it has been guaranteed when the first session starts.

Ah, you're right. Ignore the noise

Cheers
Suzuki

Quoted 2 lines

Thanks,
Leo

author reviewer
#23 lore raw

PATCH v2 10/23 coresight: trbe: Drop redundant disables

In reply to Leo Yan

The IRQ handler has a common point that disables TRBE before dispatching the fault action. Once execution reaches the overflow handling or truncate path, the trace buffer has already been stopped.

The __arm_trbe_enable() error path does not need a disable there either, as TRBE is already in disabled state.

Remove TRBE disable from the truncate helper and from the overflow failure path. The truncate helper only needs to mark and end the AUX output, so rename it to trbe_truncate_event() to reflect its remaining purpose.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +3 -12
index dd9c9330bb0c739795f300dafc6ce34e72fbf280..4a041bbc6e970d72a7d735e15f7221037cb091d6 100644
@@ -301,18 +301,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;
@@ -1024,7 +1016,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;
 }
 
@@ -1117,7 +1109,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;
 	}
@@ -1208,7 +1199,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;
 	}
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 10/23
#24 lore raw

Re: [PATCH v2 10/23] coresight: trbe: Drop redundant disables

Quoted 60 lines · Leo Yan

The IRQ handler has a common point that disables TRBE before dispatching the fault action. Once execution reaches the overflow handling or truncate path, the trace buffer has already been stopped.

The __arm_trbe_enable() error path does not need a disable there either, as TRBE is already in disabled state.

Remove TRBE disable from the truncate helper and from the overflow failure path. The truncate helper only needs to mark and end the AUX output, so rename it to trbe_truncate_event() to reflect its remaining purpose.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 15 +++------------
  1 file changed, 3 insertions(+), 12 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +2 -11
index dd9c9330bb0c739795f300dafc6ce34e72fbf280..4a041bbc6e970d72a7d735e15f7221037cb091d6 100644
@@ -301,18 +301,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;
@@ -1024,7 +1016,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;
  }
 
@@ -1117,7 +1109,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;
  	}

drivers/hwtracing/coresight/coresight-trbe.c static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) · line 1205

Quoted 8 lines
drivers/hwtracing/coresight/coresight-trbe.c +1 -1
@@ -1208,7 +1199,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;
  	}
author reviewer
#25 lore raw

PATCH v2 11/23 coresight: trbe: Assign the perf handle in enable and disable

In reply to Leo Yan

etm-perf owns the perf output handle embedded in its per-CPU etm_ctxt and passes it to TRBE through arm_trbe_enable(). TRBE binds that handle after successful sink enable and unbinds it in arm_trbe_disable(). The handle object remains valid throughout this interval, even if the current AUX transaction ends or a new transaction begins.

The inner enable helper currently publishes the handle, while truncation and failed rollover clear it. Sink disable leaves the handle installed after dropping the buffer association. These assignments track the AUX transaction rather than the enabled sink session.

Publish the handle only in arm_trbe_enable() after successful setup and clear it in arm_trbe_disable() before dropping the buffer association. Remove the assignments from the inner enable, overflow and truncation paths so the handle association follows the enabled sink session.

A valid handle object may have no active AUX transaction, so the IRQ handler continues to check perf_get_aux() before accessing the transaction's configuration.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +7 -13
index 4a041bbc6e970d72a7d735e15f7221037cb091d6..1f51c33de83638479436683fd96e31d69c3b860f 100644
@@ -303,11 +303,8 @@ static void trbe_report_wrap_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);
-
 	perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
 	perf_aux_output_end(handle, 0);
-	*this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL;
 }
 
 /*
@@ -1012,7 +1009,6 @@ static int __arm_trbe_enable(struct trbe_buf *buf,
 	if (ret)
 		goto err;
 
-	*this_cpu_ptr(buf->cpudata->drvdata->handle) = handle;
 	trbe_enable_hw(buf);
 	return 0;
 err:
@@ -1042,6 +1038,7 @@ static int arm_trbe_enable(struct coresight_device *csdev, enum cs_mode mode,
 		return ret;
 	}
 
+	*this_cpu_ptr(drvdata->handle) = handle;
 	cpudata->buf = buf;
 	cpudata->mode = mode;
 	return 0;
@@ -1062,6 +1059,7 @@ static int arm_trbe_disable(struct coresight_device *csdev)
 	trbe_drain_and_disable_local(cpudata);
 	clr_trbe_status();
 
+	*this_cpu_ptr(drvdata->handle) = NULL;
 	buf->cpudata = NULL;
 	cpudata->buf = NULL;
 	cpudata->mode = CS_MODE_DISABLED;
@@ -1102,16 +1100,12 @@ static int trbe_handle_overflow(struct perf_output_handle *handle)
 	trbe_report_wrap_event(handle);
 	perf_aux_output_end(handle, size);
 	event_data = perf_aux_output_begin(handle, event);
-	if (!event_data) {
-		/*
-		 * We are unable to restart the trace collection,
-		 * thus leave the TRBE disabled. The etm-perf driver
-		 * is able to detect this with a disconnected handle
-		 * (handle->event = NULL).
-		 */
-		*this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL;
+	/*
+	 * If trace collection cannot be restarted, leave TRBE disabled.
+	 * The etm-perf driver detects this through handle->event == NULL.
+	 */
+	if (!event_data)
 		return -EINVAL;
-	}
 
 	return __arm_trbe_enable(buf, handle);
 }
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 11/23
#26 lore raw

Re: [PATCH v2 11/23] coresight: trbe: Assign the perf handle in enable and disable

Quoted 78 lines · Leo Yan

etm-perf owns the perf output handle embedded in its per-CPU etm_ctxt and passes it to TRBE through arm_trbe_enable(). TRBE binds that handle after successful sink enable and unbinds it in arm_trbe_disable(). The handle object remains valid throughout this interval, even if the current AUX transaction ends or a new transaction begins.

The inner enable helper currently publishes the handle, while truncation and failed rollover clear it. Sink disable leaves the handle installed after dropping the buffer association. These assignments track the AUX transaction rather than the enabled sink session.

Publish the handle only in arm_trbe_enable() after successful setup and clear it in arm_trbe_disable() before dropping the buffer association. Remove the assignments from the inner enable, overflow and truncation paths so the handle association follows the enabled sink session.

A valid handle object may have no active AUX transaction, so the IRQ handler continues to check perf_get_aux() before accessing the transaction's configuration.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 20 +++++++-------------
  1 file changed, 7 insertions(+), 13 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +2 -12
index 4a041bbc6e970d72a7d735e15f7221037cb091d6..1f51c33de83638479436683fd96e31d69c3b860f 100644
@@ -303,11 +303,8 @@ static void trbe_report_wrap_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);
-
  	perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
  	perf_aux_output_end(handle, 0);
-	*this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL;
  }
 
  /*
@@ -1012,7 +1009,6 @@ static int __arm_trbe_enable(struct trbe_buf *buf,
  	if (ret)
  		goto err;
 
-	*this_cpu_ptr(buf->cpudata->drvdata->handle) = handle;
  	trbe_enable_hw(buf);
  	return 0;
  err:
@@ -1042,6 +1038,7 @@ static int arm_trbe_enable(struct coresight_device *csdev, enum cs_mode mode,
  		return ret;
  	}
 
+	*this_cpu_ptr(drvdata->handle) = handle;
  	cpudata->buf = buf;
  	cpudata->mode = mode;
  	return 0;
@@ -1062,6 +1059,7 @@ static int arm_trbe_disable(struct coresight_device *csdev)
  	trbe_drain_and_disable_local(cpudata);
  	clr_trbe_status();
 
+	*this_cpu_ptr(drvdata->handle) = NULL;
  	buf->cpudata = NULL;
  	cpudata->buf = NULL;
  	cpudata->mode = CS_MODE_DISABLED;
@@ -1102,16 +1100,12 @@ static int trbe_handle_overflow(struct perf_output_handle *handle)
  	trbe_report_wrap_event(handle);
  	perf_aux_output_end(handle, size);
  	event_data = perf_aux_output_begin(handle, event);
-	if (!event_data) {
-		/*
-		 * We are unable to restart the trace collection,
-		 * thus leave the TRBE disabled. The etm-perf driver
-		 * is able to detect this with a disconnected handle
-		 * (handle->event = NULL).
-		 */
-		*this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL;

drivers/hwtracing/coresight/coresight-trbe.c static int trbe_handle_overflow(struct perf_output_handle *handle) · line 1111

Quoted 10 lines
drivers/hwtracing/coresight/coresight-trbe.c +5 -1
+	/*
+	 * If trace collection cannot be restarted, leave TRBE disabled.
+	 * The etm-perf driver detects this through handle->event == NULL.
+	 */
+	if (!event_data)
  		return -EINVAL;
-	}
 
  	return __arm_trbe_enable(buf, handle);
  }
author reviewer
#27 lore raw

PATCH v2 12/23 coresight: trbe: Decode TRBSR status codes

In reply to Leo Yan

trbe_get_fault_act() currently handles only a small set of TRBE stop
reasons: external aborts, trigger events and stage 1 and 2 aborts. Other TRBSR_EL1 EC/BSC values are reported as spurious events, which hides useful failure information and makes the status handling hard to extend.

Decode the EC and BSC fields explicitly. Keep the normal fill-mode handling unchanged, but treat unsupported or unexpected status values as fatal and report them with a ratelimited diagnostic containing TRBSR_EL1, TRBPTR_EL1 and TRBLIMITR_EL1. Use %p for the buffer pointers so the printk diagnostic hashes their address values by default.

Stop using is_trbe_trg(), instead, use TRBSR_EL1_BSC_TRIGGER_EVENT as the unsupported trigger case. This keeps the stop-reason handling in the EC/BSC decode path, the driver can validate the status fields safely.

Move the TRBSR_EL1 BSC definitions to the arm64 sysreg header, and use the common TRBSR_EL1 EC/BSC names in the TRBE driver.

---
 arch/arm64/include/asm/sysreg.h              |  6 +++
 drivers/hwtracing/coresight/coresight-trbe.c | 57 +++++++++++++++++++++++++---
 drivers/hwtracing/coresight/coresight-trbe.h |  8 ----
 3 files changed, 58 insertions(+), 13 deletions(-)
arch/arm64/include/asm/sysreg.h +6 -0
index 7aa08d59d49448cfa3bb61f5cb42e0a6f1aa8a89..0e602f07625b3c4fe6cd3fd71a658ebae768d8fd 100644
@@ -357,6 +357,12 @@
 #define TRBSR_EL1_BSC_MASK		GENMASK(5, 0)
 #define TRBSR_EL1_BSC_SHIFT		0
 
+#define TRBSR_EL1_BSC_NOT_STOPPED	UL(0)
+#define TRBSR_EL1_BSC_FILLED		UL(1)
+#define TRBSR_EL1_BSC_TRIGGER_EVENT	UL(2)
+#define TRBSR_EL1_BSC_MANUAL_STOP	UL(3)
+#define TRBSR_EL1_BSC_BUF_SIZE		UL(4)
+
 #define SYS_PMINTENSET_EL1		sys_reg(3, 0, 9, 14, 1)
 #define SYS_PMINTENCLR_EL1		sys_reg(3, 0, 9, 14, 2)
 
drivers/hwtracing/coresight/coresight-trbe.c +52 -5

81 lines not on this page · raw message

drivers/hwtracing/coresight/coresight-trbe.h +0 -8
index 45202c48accec7c86ba56130e2737bc2d1830fae..8ba9ef4056e4dc3968816c6a7cbf2859f2bbed36 100644
@@ -35,19 +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
-
 static inline int get_trbe_ec(u64 trbsr)
 {
 	return (trbsr & TRBSR_EL1_EC_MASK) >> TRBSR_EL1_EC_SHIFT;
 }
 
-#define TRBE_BSC_NOT_STOPPED 0
-#define TRBE_BSC_FILLED      1
-#define TRBE_BSC_TRIGGERED   2
-
 static inline int get_trbe_bsc(u64 trbsr)
 {
 	return (trbsr & TRBSR_EL1_BSC_MASK) >> TRBSR_EL1_BSC_SHIFT;
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 12/23
#28 lore raw

Re: [PATCH v2 12/23] coresight: trbe: Decode TRBSR status codes

Quoted 143 lines · Leo Yan

trbe_get_fault_act() currently handles only a small set of TRBE stop
reasons: external aborts, trigger events and stage 1 and 2 aborts. Other TRBSR_EL1 EC/BSC values are reported as spurious events, which hides useful failure information and makes the status handling hard to extend.

Decode the EC and BSC fields explicitly. Keep the normal fill-mode handling unchanged, but treat unsupported or unexpected status values as fatal and report them with a ratelimited diagnostic containing TRBSR_EL1, TRBPTR_EL1 and TRBLIMITR_EL1. Use %p for the buffer pointers so the printk diagnostic hashes their address values by default.

Stop using is_trbe_trg(), instead, use TRBSR_EL1_BSC_TRIGGER_EVENT as the unsupported trigger case. This keeps the stop-reason handling in the EC/BSC decode path, the driver can validate the status fields safely.

Move the TRBSR_EL1 BSC definitions to the arm64 sysreg header, and use the common TRBSR_EL1 EC/BSC names in the TRBE driver.

---
  arch/arm64/include/asm/sysreg.h              |  6 +++
  drivers/hwtracing/coresight/coresight-trbe.c | 57 +++++++++++++++++++++++++---
  drivers/hwtracing/coresight/coresight-trbe.h |  8 ----
  3 files changed, 58 insertions(+), 13 deletions(-)
arch/arm64/include/asm/sysreg.h +6 -0
index 7aa08d59d49448cfa3bb61f5cb42e0a6f1aa8a89..0e602f07625b3c4fe6cd3fd71a658ebae768d8fd 100644
@@ -357,6 +357,12 @@
  #define TRBSR_EL1_BSC_MASK		GENMASK(5, 0)
  #define TRBSR_EL1_BSC_SHIFT		0
 
+#define TRBSR_EL1_BSC_NOT_STOPPED	UL(0)
+#define TRBSR_EL1_BSC_FILLED		UL(1)
+#define TRBSR_EL1_BSC_TRIGGER_EVENT	UL(2)
+#define TRBSR_EL1_BSC_MANUAL_STOP	UL(3)
+#define TRBSR_EL1_BSC_BUF_SIZE		UL(4)
+
  #define SYS_PMINTENSET_EL1		sys_reg(3, 0, 9, 14, 1)
  #define SYS_PMINTENCLR_EL1		sys_reg(3, 0, 9, 14, 2)
 
drivers/hwtracing/coresight/coresight-trbe.c +52 -5
index 1f51c33de83638479436683fd96e31d69c3b860f..a851f09d806a6b6a6386d6f7547b01a17f61b88e 100644
@@ -658,29 +658,76 @@ 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;
+	}
+
+	switch (ec) {
+	case TRBSR_EL1_EC_OTHER:
+		break;
+	case TRBSR_EL1_EC_IMP_DEF:
+		err_str = "Unexpected IMPDEF buffer management event";
+		goto out_fatal;
+	case TRBSR_EL1_EC_FAULT_GPC:
+		err_str = "Granule Protection Check fault";
+		goto out_fatal;
+	case TRBSR_EL1_EC_FAULT_WRITE_S1:
+		err_str = "Stage 1 data abort";
+		goto out_fatal;
+	case TRBSR_EL1_EC_FAULT_WRITE_S2:
+		err_str = "Stage 2 data abort";
+		goto out_fatal;
+	default:
+		err_str = "Unknown error code";
+		goto out_fatal;
+	}
+
+	switch (bsc) {
+	case TRBSR_EL1_BSC_NOT_STOPPED:
+		break;
+	case TRBSR_EL1_BSC_FILLED:
+		break;
+	case TRBSR_EL1_BSC_TRIGGER_EVENT:
+		err_str = "Unsupported trigger event";
+		goto out_fatal;
+	case TRBSR_EL1_BSC_MANUAL_STOP:
+		err_str = "Unsupported manual stop";
+		goto out_fatal;
+	case TRBSR_EL1_BSC_BUF_SIZE:
+		err_str = "Buffer size was too large";
+		goto out_fatal;
+	default:
+		err_str = "Unexpected buffer status code";
+		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)) &&
+	if ((is_trbe_wrap(trbsr) && (ec == TRBSR_EL1_EC_OTHER) && (bsc == TRBSR_EL1_BSC_FILLED)) &&
  	    (trbe_may_overwrite_in_fill_mode(cpudata) ||
  	     get_trbe_write_pointer() == get_trbe_base_pointer()))
  		return TRBE_FAULT_ACT_WRAP;
 
  	return TRBE_FAULT_ACT_SPURIOUS;
+
+out_fatal:
+	pr_err_ratelimited("%s on CPU %d [TRBSR=0x%016llx, TRBPTR=%p, TRBLIMITR=%p]\n",
+			   err_str, smp_processor_id(), trbsr,
+			   (void *)read_sysreg_s(SYS_TRBPTR_EL1),
+			   (void *)read_sysreg_s(SYS_TRBLIMITR_EL1));
+	return TRBE_FAULT_ACT_FATAL;
  }
 
  static unsigned long trbe_get_trace_size(struct perf_output_handle *handle,
drivers/hwtracing/coresight/coresight-trbe.h +0 -4
index 45202c48accec7c86ba56130e2737bc2d1830fae..8ba9ef4056e4dc3968816c6a7cbf2859f2bbed36 100644
@@ -35,19 +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
-
  static inline int get_trbe_ec(u64 trbsr)
  {

drivers/hwtracing/coresight/coresight-trbe.h static inline bool is_trbe_enabled(void) · line 45

Quoted 10 lines
drivers/hwtracing/coresight/coresight-trbe.h +0 -4
  	return (trbsr & TRBSR_EL1_EC_MASK) >> TRBSR_EL1_EC_SHIFT;
  }
 
-#define TRBE_BSC_NOT_STOPPED 0
-#define TRBE_BSC_FILLED      1
-#define TRBE_BSC_TRIGGERED   2
-
  static inline int get_trbe_bsc(u64 trbsr)
  {
  	return (trbsr & TRBSR_EL1_BSC_MASK) >> TRBSR_EL1_BSC_SHIFT;
author reviewer
#29 lore raw

PATCH v2 13/23 coresight: trbe: Simplify wrap status handling

In reply to Leo Yan

After explicitly decoding the TRBSR_EL1 EC/BSC values, unsupported or unexpected stop reasons are handled before the WRAP check. And the erratum and pointer checks is no longer relevant when deciding the buffer wrap.

Remove the redundant checks and treat any remaining TRBSR_EL1.WRAP status as a wrap event. This also means WRAP with BSC_NOT_STOPPED is reported as an overflow instead of a spurious event, which prepares for for TRBE wrap mode where collection continues and a maintenance interrupt is raised on current write pointer wrap.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +1 -10
index a851f09d806a6b6a6386d6f7547b01a17f61b88e..4aeed1a17c4bf1653fb41731ac15f31feec7ed2e 100644
@@ -661,8 +661,6 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand
 	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));
 
@@ -710,14 +708,7 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand
 		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 == TRBSR_EL1_EC_OTHER) && (bsc == TRBSR_EL1_BSC_FILLED)) &&
-	    (trbe_may_overwrite_in_fill_mode(cpudata) ||
-	     get_trbe_write_pointer() == get_trbe_base_pointer()))
+	if (is_trbe_wrap(trbsr))
 		return TRBE_FAULT_ACT_WRAP;
 
 	return TRBE_FAULT_ACT_SPURIOUS;
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 13/23
#30 lore raw

Re: [PATCH v2 13/23] coresight: trbe: Simplify wrap status handling

Quoted 36 lines · Leo Yan

After explicitly decoding the TRBSR_EL1 EC/BSC values, unsupported or unexpected stop reasons are handled before the WRAP check. And the erratum and pointer checks is no longer relevant when deciding the buffer wrap.

Remove the redundant checks and treat any remaining TRBSR_EL1.WRAP status as a wrap event. This also means WRAP with BSC_NOT_STOPPED is reported as an overflow instead of a spurious event, which prepares for for TRBE wrap mode where collection continues and a maintenance interrupt is raised on current write pointer wrap.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 11 +----------
  1 file changed, 1 insertion(+), 10 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +0 -4
index a851f09d806a6b6a6386d6f7547b01a17f61b88e..4aeed1a17c4bf1653fb41731ac15f31feec7ed2e 100644
@@ -661,8 +661,6 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand
  	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));
 
@@ -710,14 +708,7 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand
  		goto out_fatal;
  	}
 
-	/*
-	 * If the trbe is affected by TRBE_WORKAROUND_OVERWRITE_FILL_MODE,

drivers/hwtracing/coresight/coresight-trbe.c static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand · line 714

Quoted 10 lines
drivers/hwtracing/coresight/coresight-trbe.c +1 -6
-	 * 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 == TRBSR_EL1_EC_OTHER) && (bsc == TRBSR_EL1_BSC_FILLED)) &&
-	    (trbe_may_overwrite_in_fill_mode(cpudata) ||
-	     get_trbe_write_pointer() == get_trbe_base_pointer()))
+	if (is_trbe_wrap(trbsr))
  		return TRBE_FAULT_ACT_WRAP;
 
  	return TRBE_FAULT_ACT_SPURIOUS;
author reviewer
#31 lore raw

PATCH v2 14/23 coresight: trbe: Treat access not allowed stops as fatal

In reply to Leo Yan

Arm ARM (ARM DDI 0487 M.b), the rule R_RRCNN in "D6.11 UNPREDICTABLE behavior" permits a buffer management event when the effective SCR_EL3 security state does not match the owning Security state. This can stop collection with IRQ=1, S=1, EC=0 and BSC=0 while leaving the other status fields unchanged.

With EA and WRAP clear, trbe_get_fault_act() currently returns SPURIOUS for this status. The spurious handler re-enables TRBE without clearing S, so collection can remain stopped. A retained WRAP indication can instead cause the status to be treated as a normal buffer wrap.

Recognize this architecturally permitted stopped status as an access not allowed error and return FATAL so the fatal handling reports truncated trace. Check the syndrome before WRAP and do not exclude TRG, since both fields are left unchanged by this stop. Check the saved IRQ bit to preserve updates without a pending interrupt.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/hwtracing/coresight/coresight-trbe.c +8 -0
index 4aeed1a17c4bf1653fb41731ac15f31feec7ed2e..1c585b6123df2f20d08747678d836742ce9564cc 100644
@@ -708,6 +708,14 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand
 		goto out_fatal;
 	}
 
+	/* An access not allowed stop can leave WRAP and TRG unchanged. */
+	if (is_trbe_irq(trbsr) && !is_trbe_running(trbsr) &&
+	    ec == TRBSR_EL1_EC_OTHER &&
+	    bsc == TRBSR_EL1_BSC_NOT_STOPPED) {
+		err_str = "Trace buffer access not allowed";
+		goto out_fatal;
+	}
+
 	if (is_trbe_wrap(trbsr))
 		return TRBE_FAULT_ACT_WRAP;
 
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 14/23
#32 lore raw

Re: [PATCH v2 14/23] coresight: trbe: Treat access not allowed stops as fatal

Quoted 32 lines · Leo Yan

Arm ARM (ARM DDI 0487 M.b), the rule R_RRCNN in "D6.11 UNPREDICTABLE behavior" permits a buffer management event when the effective SCR_EL3 security state does not match the owning Security state. This can stop collection with IRQ=1, S=1, EC=0 and BSC=0 while leaving the other status fields unchanged.

With EA and WRAP clear, trbe_get_fault_act() currently returns SPURIOUS for this status. The spurious handler re-enables TRBE without clearing S, so collection can remain stopped. A retained WRAP indication can instead cause the status to be treated as a normal buffer wrap.

Recognize this architecturally permitted stopped status as an access not allowed error and return FATAL so the fatal handling reports truncated trace. Check the syndrome before WRAP and do not exclude TRG, since both fields are left unchanged by this stop. Check the saved IRQ bit to preserve updates without a pending interrupt.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 8 ++++++++
  1 file changed, 8 insertions(+)
drivers/hwtracing/coresight/coresight-trbe.c +1 -0
index 4aeed1a17c4bf1653fb41731ac15f31feec7ed2e..1c585b6123df2f20d08747678d836742ce9564cc 100644
@@ -708,6 +708,14 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand
  		goto out_fatal;
  	}
 
+	/* An access not allowed stop can leave WRAP and TRG unchanged. */

drivers/hwtracing/coresight/coresight-trbe.c static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand · line 721

Quoted 10 lines
drivers/hwtracing/coresight/coresight-trbe.c +7 -0
+	if (is_trbe_irq(trbsr) && !is_trbe_running(trbsr) &&
+	    ec == TRBSR_EL1_EC_OTHER &&
+	    bsc == TRBSR_EL1_BSC_NOT_STOPPED) {
+		err_str = "Trace buffer access not allowed";
+		goto out_fatal;
+	}
+
  	if (is_trbe_wrap(trbsr))
  		return TRBE_FAULT_ACT_WRAP;
 
author reviewer
#33 lore raw

PATCH v2 15/23 coresight: trbe: Report truncated trace on buffer update errors

In reply to Leo Yan

When arm_trbe_update_buffer() handles a pending interrupt that is not a wrap event, it discards the buffer by returning zero. The AUX output is not marked as truncated, so userspace is not informed of the lost trace.

Set PERF_AUX_FLAG_TRUNCATED before returning zero so perf reports the trace loss when closing the AUX output.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 1 +
 1 file changed, 1 insertion(+)
drivers/hwtracing/coresight/coresight-trbe.c +1 -0
index 1c585b6123df2f20d08747678d836742ce9564cc..ac49098aa08412a23a20d32f84c2e1977ee4a223 100644
@@ -903,6 +903,7 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev,
 		 * errors and as such buffer is empty.
 		 */
 		if (act != TRBE_FAULT_ACT_WRAP) {
+			perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
 			size = 0;
 			goto done;
 		}
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 15/23
#34 lore raw

Re: [PATCH v2 15/23] coresight: trbe: Report truncated trace on buffer update errors

Quoted 19 lines · Leo Yan

When arm_trbe_update_buffer() handles a pending interrupt that is not a wrap event, it discards the buffer by returning zero. The AUX output is not marked as truncated, so userspace is not informed of the lost trace.

Set PERF_AUX_FLAG_TRUNCATED before returning zero so perf reports the trace loss when closing the AUX output.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 1 +
  1 file changed, 1 insertion(+)
drivers/hwtracing/coresight/coresight-trbe.c +0 -0
index 1c585b6123df2f20d08747678d836742ce9564cc..ac49098aa08412a23a20d32f84c2e1977ee4a223 100644

drivers/hwtracing/coresight/coresight-trbe.c static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev, · line 909

Quoted 7 lines
drivers/hwtracing/coresight/coresight-trbe.c +1 -0
@@ -903,6 +903,7 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev,
  		 * errors and as such buffer is empty.
  		 */
  		if (act != TRBE_FAULT_ACT_WRAP) {
+			perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
  			size = 0;
  			goto done;
  		}
maintainer
#35 lore raw

Re: [PATCH v2 15/23] coresight: trbe: Report truncated trace on buffer update errors

In reply to Leo Yan
Quoted 18 lines · Leo Yan

When arm_trbe_update_buffer() handles a pending interrupt that is not a wrap event, it discards the buffer by returning zero. The AUX output is not marked as truncated, so userspace is not informed of the lost trace.

Set PERF_AUX_FLAG_TRUNCATED before returning zero so perf reports the trace loss when closing the AUX output.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 1 +
  1 file changed, 1 insertion(+)
drivers/hwtracing/coresight/coresight-trbe.c +0 -0
index 1c585b6123df2f20d08747678d836742ce9564cc..ac49098aa08412a23a20d32f84c2e1977ee4a223 100644

drivers/hwtracing/coresight/coresight-trbe.c static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev, · line 906

Quoted 4 lines
drivers/hwtracing/coresight/coresight-trbe.c +1 -0
@@ -903,6 +903,7 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev,
  		 * errors and as such buffer is empty.
  		 */
  		if (act != TRBE_FAULT_ACT_WRAP) {
+			perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);

Do we need this though ? If the buffer has space left, we shouldn't use TRUNCATED. TRUNCATED causes the event to be stopped and the userspace needs to re-enable the event.

Cheers
Suzuki

Quoted 4 lines
drivers/hwtracing/coresight/coresight-trbe.c +0 -0
  			size = 0;
  			goto done;
  		}
author reviewer
#36 lore raw

PATCH v2 16/23 coresight: trbe: Always classify TRBSR when updating buffer

In reply to Leo Yan

arm_trbe_update_buffer() must account trace when perf stops a session, even if no maintenance interrupt has been handled. Prepare it for trigger and wrap modes by making the trace-size decision from the drained TRBSR status rather than gating classification on IRQ. Acknowledge IRQ only when it is set.

In FILL mode, reaching LIMIT sets WRAP and IRQ and returns TRBPTR to BASE. Interrupt delivery can be delayed, so update_buffer() may run before the IRQ handler. The existing IRQ-gated path uses WRAP to size through LIMIT in that case; preserve this while also classifying updates without a pending IRQ.

With classification on the no-IRQ path, a normal software stop can have TRBSR.S clear and an ignored detected trigger can set TRBSR.TRG. Neither should warn or discard valid trace. Mark fatal status as truncated and return zero. Use LIMIT for a filled WRAP and TRBPTR otherwise.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +9 -15
index ac49098aa08412a23a20d32f84c2e1977ee4a223..661f4a78e67c30652de8c834e4773819fa67b5ac 100644
@@ -662,8 +662,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;
@@ -844,7 +842,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());
@@ -896,23 +893,20 @@ 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) {
-			perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
-			size = 0;
-			goto done;
-		}
+	act = trbe_get_fault_act(handle, status);
 
+	/* If an error occurred, the buffer may be empty */
+	if (act == TRBE_FAULT_ACT_FATAL) {
+		perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
+		size = 0;
+		goto done;
+	} else if (act == TRBE_FAULT_ACT_WRAP) {
 		trbe_report_wrap_event(handle);
-		wrap = true;
 	}
 
-	size = trbe_get_trace_size(handle, buf, wrap);
+	size = trbe_get_trace_size(handle, buf, act == TRBE_FAULT_ACT_WRAP);
 
 done:
 	local_irq_restore(flags);
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 16/23
#37 lore raw

Re: [PATCH v2 16/23] coresight: trbe: Always classify TRBSR when updating buffer

Quoted 68 lines · Leo Yan

arm_trbe_update_buffer() must account trace when perf stops a session, even if no maintenance interrupt has been handled. Prepare it for trigger and wrap modes by making the trace-size decision from the drained TRBSR status rather than gating classification on IRQ. Acknowledge IRQ only when it is set.

In FILL mode, reaching LIMIT sets WRAP and IRQ and returns TRBPTR to BASE. Interrupt delivery can be delayed, so update_buffer() may run before the IRQ handler. The existing IRQ-gated path uses WRAP to size through LIMIT in that case; preserve this while also classifying updates without a pending IRQ.

With classification on the no-IRQ path, a normal software stop can have TRBSR.S clear and an ignored detected trigger can set TRBSR.TRG. Neither should warn or discard valid trace. Mark fatal status as truncated and return zero. Use LIMIT for a filled WRAP and TRBPTR otherwise.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 24 +++++++++---------------
  1 file changed, 9 insertions(+), 15 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +7 -13
index ac49098aa08412a23a20d32f84c2e1977ee4a223..661f4a78e67c30652de8c834e4773819fa67b5ac 100644
@@ -662,8 +662,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;
@@ -844,7 +842,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());
@@ -896,23 +893,20 @@ 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) {
-			perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
-			size = 0;
-			goto done;
-		}
+	act = trbe_get_fault_act(handle, status);
 
+	/* If an error occurred, the buffer may be empty */
+	if (act == TRBE_FAULT_ACT_FATAL) {
+		perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
+		size = 0;
+		goto done;

drivers/hwtracing/coresight/coresight-trbe.c static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev, · line 912

Quoted 10 lines
drivers/hwtracing/coresight/coresight-trbe.c +2 -2
+	} else if (act == TRBE_FAULT_ACT_WRAP) {
  		trbe_report_wrap_event(handle);
-		wrap = true;
  	}
 
-	size = trbe_get_trace_size(handle, buf, wrap);
+	size = trbe_get_trace_size(handle, buf, act == TRBE_FAULT_ACT_WRAP);
 
  done:
  	local_irq_restore(flags);
author reviewer
#38 lore raw

PATCH v2 17/23 coresight: trbe: Calculate trace size across buffer wrap

In reply to Leo Yan

trbe_get_trace_size() assumes that the TRBE write pointer is always ahead of the perf AUX head. That is not true after a WRAP event, where the write pointer can move back to the start of the programmed TRBE span.

Calculate the size in two parts for WRAP: the bytes from the current AUX head to TRBLIMITR_EL1, plus any bytes from TRBBASER_EL1 to the current TRBPTR_EL1. Keep the non-WRAP path strict and reject an unexpected write pointer below the AUX head.

For CPUs affected by the overwrite-fill-mode or write-out-of-range errata, continue to ignore TRBPTR_EL1 on WRAP and account data only up to TRBLIMITR_EL1, since bytes beyond the programmed limit are uncertain.

Apply this fallback before validating the write pointer so a WRAP report with a pointer outside the programmed range still publishes the trace from the AUX head to the programmed limit.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 62 +++++++++++++++++-----------
 1 file changed, 37 insertions(+), 25 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +37 -25
index 661f4a78e67c30652de8c834e4773819fa67b5ac..9a469c8009ce8a8a8b804c2b63a68e741f1dceaa 100644
@@ -730,43 +730,55 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand
 static unsigned long trbe_get_trace_size(struct perf_output_handle *handle,
 					 struct trbe_buf *buf, bool wrap)
 {
-	u64 write;
-	u64 start_off, end_off;
-	u64 size;
 	u64 overwrite_skip = TRBE_WORKAROUND_OVERWRITE_FILL_MODE_SKIP_BYTES;
+	u64 base = buf->trbe_base;
+	u64 start_off = PERF_IDX2OFF(handle->head, buf);
+	u64 write = get_trbe_write_pointer();
+	u64 limit = get_trbe_limit_pointer();
+	u64 limit_off, write_off, size;
 
 	/*
-	 * If the TRBE has wrapped around the write pointer has
-	 * wrapped and should be treated as limit.
-	 *
 	 * When the TRBE is affected by TRBE_WORKAROUND_WRITE_OUT_OF_RANGE,
-	 * it may write upto 64bytes beyond the "LIMIT". The driver already
+	 * it may write up to 64bytes beyond the "LIMIT". The driver already
 	 * keeps a valid page next to the LIMIT and we could potentially
 	 * consume the trace data that may have been collected there. But we
 	 * cannot be really sure it is available, and the TRBPTR may not
-	 * indicate the same. Also, affected cores are also affected by another
-	 * erratum which forces the PAGE_SIZE alignment on the TRBPTR, and thus
-	 * could potentially pad an entire PAGE_SIZE - 64bytes, to get those
-	 * 64bytes. Thus we ignore the potential triggering of the erratum
-	 * on WRAP and limit the data to LIMIT.
+	 * indicate the same.
+	 *
+	 * Also, affected cores are also affected by another erratum
+	 * TRBE_WORKAROUND_OVERWRITE_FILL_MODE, which forces the PAGE_SIZE
+	 * alignment on the TRBPTR, and thus could potentially pad an entire
+	 * PAGE_SIZE - 64bytes, to get those 64bytes. Thus we ignore the
+	 * potential effects of the erratum on WRAP and limit the data to
+	 * LIMIT.
 	 */
-	if (wrap)
-		write = get_trbe_limit_pointer();
-	else
-		write = get_trbe_write_pointer();
+	if (wrap &&
+	    (trbe_may_overwrite_in_fill_mode(buf->cpudata) ||
+	     trbe_may_write_out_of_range(buf->cpudata)))
+		write = limit;
 
-	/*
-	 * TRBE may use a different base address than the base
-	 * of the ring buffer. Thus use the beginning of the ring
-	 * buffer to compute the offsets.
-	 */
-	end_off = write - buf->trbe_base;
-	start_off = PERF_IDX2OFF(handle->head, buf);
+	/* Should never happen ... */
+	if (WARN_ON_ONCE(write < base || write > limit))
+		return 0;
 
-	if (WARN_ON_ONCE(end_off < start_off))
+	limit_off = limit - base;
+	write_off = write - base;
+
+	if (WARN_ON_ONCE(start_off >= limit_off))
 		return 0;
 
-	size = end_off - start_off;
+	if (!wrap) {
+		if (WARN_ON_ONCE(write_off < start_off))
+			return 0;
+
+		size = write_off - start_off;
+	} else {
+		size = limit_off - start_off;
+
+		if (write != limit)
+			size += write_off;
+	}
+
 	/*
 	 * If the TRBE is affected by the following erratum, we must fill
 	 * the space we skipped with IGNORE packets. And we are always
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 17/23
#39 lore raw

Re: [PATCH v2 17/23] coresight: trbe: Calculate trace size across buffer wrap

Quoted 98 lines · Leo Yan

trbe_get_trace_size() assumes that the TRBE write pointer is always ahead of the perf AUX head. That is not true after a WRAP event, where the write pointer can move back to the start of the programmed TRBE span.

Calculate the size in two parts for WRAP: the bytes from the current AUX head to TRBLIMITR_EL1, plus any bytes from TRBBASER_EL1 to the current TRBPTR_EL1. Keep the non-WRAP path strict and reject an unexpected write pointer below the AUX head.

For CPUs affected by the overwrite-fill-mode or write-out-of-range errata, continue to ignore TRBPTR_EL1 on WRAP and account data only up to TRBLIMITR_EL1, since bytes beyond the programmed limit are uncertain.

Apply this fallback before validating the write pointer so a WRAP report with a pointer outside the programmed range still publishes the trace from the AUX head to the programmed limit.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 62 +++++++++++++++++-----------
  1 file changed, 37 insertions(+), 25 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +30 -25
index 661f4a78e67c30652de8c834e4773819fa67b5ac..9a469c8009ce8a8a8b804c2b63a68e741f1dceaa 100644
@@ -730,43 +730,55 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand
  static unsigned long trbe_get_trace_size(struct perf_output_handle *handle,
  					 struct trbe_buf *buf, bool wrap)
  {
-	u64 write;
-	u64 start_off, end_off;
-	u64 size;
  	u64 overwrite_skip = TRBE_WORKAROUND_OVERWRITE_FILL_MODE_SKIP_BYTES;
+	u64 base = buf->trbe_base;
+	u64 start_off = PERF_IDX2OFF(handle->head, buf);
+	u64 write = get_trbe_write_pointer();
+	u64 limit = get_trbe_limit_pointer();
+	u64 limit_off, write_off, size;
 
  	/*
-	 * If the TRBE has wrapped around the write pointer has
-	 * wrapped and should be treated as limit.
-	 *
  	 * When the TRBE is affected by TRBE_WORKAROUND_WRITE_OUT_OF_RANGE,
-	 * it may write upto 64bytes beyond the "LIMIT". The driver already
+	 * it may write up to 64bytes beyond the "LIMIT". The driver already
  	 * keeps a valid page next to the LIMIT and we could potentially
  	 * consume the trace data that may have been collected there. But we
  	 * cannot be really sure it is available, and the TRBPTR may not
-	 * indicate the same. Also, affected cores are also affected by another
-	 * erratum which forces the PAGE_SIZE alignment on the TRBPTR, and thus
-	 * could potentially pad an entire PAGE_SIZE - 64bytes, to get those
-	 * 64bytes. Thus we ignore the potential triggering of the erratum
-	 * on WRAP and limit the data to LIMIT.
+	 * indicate the same.
+	 *
+	 * Also, affected cores are also affected by another erratum
+	 * TRBE_WORKAROUND_OVERWRITE_FILL_MODE, which forces the PAGE_SIZE
+	 * alignment on the TRBPTR, and thus could potentially pad an entire
+	 * PAGE_SIZE - 64bytes, to get those 64bytes. Thus we ignore the
+	 * potential effects of the erratum on WRAP and limit the data to
+	 * LIMIT.
  	 */
-	if (wrap)
-		write = get_trbe_limit_pointer();
-	else
-		write = get_trbe_write_pointer();
+	if (wrap &&
+	    (trbe_may_overwrite_in_fill_mode(buf->cpudata) ||
+	     trbe_may_write_out_of_range(buf->cpudata)))
+		write = limit;
 
-	/*
-	 * TRBE may use a different base address than the base
-	 * of the ring buffer. Thus use the beginning of the ring
-	 * buffer to compute the offsets.
-	 */
-	end_off = write - buf->trbe_base;
-	start_off = PERF_IDX2OFF(handle->head, buf);
+	/* Should never happen ... */
+	if (WARN_ON_ONCE(write < base || write > limit))
+		return 0;
 
-	if (WARN_ON_ONCE(end_off < start_off))
+	limit_off = limit - base;
+	write_off = write - base;
+
+	if (WARN_ON_ONCE(start_off >= limit_off))
  		return 0;
 
-	size = end_off - start_off;
+	if (!wrap) {
+		if (WARN_ON_ONCE(write_off < start_off))
+			return 0;
+
+		size = write_off - start_off;

drivers/hwtracing/coresight/coresight-trbe.c static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand · line 784

Quoted 10 lines
drivers/hwtracing/coresight/coresight-trbe.c +7 -0
+	} else {
+		size = limit_off - start_off;
+
+		if (write != limit)
+			size += write_off;
+	}
+
  	/*
  	 * If the TRBE is affected by the following erratum, we must fill
  	 * the space we skipped with IGNORE packets. And we are always
author reviewer
#40 lore raw

PATCH v2 18/23 coresight: trbe: Keep the perf handle in per-CPU data

In reply to Leo Yan

Arm SPE owns a per-CPU perf output handle for the PMU lifetime. TRBE instead receives a handle from CoreSight when enabling its sink.

Store that session handle in trbe_cpudata instead of a separate per-CPU pointer array, and pass the CPU data as the per-CPU IRQ context. Use this context directly for overflow and spurious interrupt handling so local hardware control does not depend on the current AUX buffer configuration.

Allocate the CPU data before registering the IRQ and free it after releasing the IRQ, keeping the IRQ context valid throughout registration.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 44 ++++++++++++----------------
 1 file changed, 19 insertions(+), 25 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +19 -25
index 9a469c8009ce8a8a8b804c2b63a68e741f1dceaa..2328aab0a084a0ac123e950f846f404d120ec2c0 100644
@@ -139,6 +139,7 @@ struct trbe_save_state {
  * @cpu			- CPU this TRBE belongs to.
  * @mode		- Mode of current operation. (perf/disabled)
  * @buf			- Buffer configuration retained for the enabled session.
+ * @handle		- Perf output handle from successful enable until disable.
  * @drvdata		- TRBE specific drvdata
  * @errata		- Bit map for the errata on this TRBE.
  */
@@ -149,6 +150,7 @@ struct trbe_cpudata {
 	int cpu;
 	enum cs_mode mode;
 	struct trbe_buf *buf;
+	struct perf_output_handle *handle;
 	struct trbe_drvdata *drvdata;
 	struct trbe_save_state save_state;
 	DECLARE_BITMAP(errata, TRBE_ERRATA_MAX);
@@ -156,7 +158,6 @@ struct trbe_cpudata {
 
 struct trbe_drvdata {
 	struct trbe_cpudata __percpu *cpudata;
-	struct perf_output_handle * __percpu *handle;
 	struct hlist_node hotplug_node;
 	int irq;
 	cpumask_t supported_cpus;
@@ -1091,7 +1092,7 @@ static int arm_trbe_enable(struct coresight_device *csdev, enum cs_mode mode,
 		return ret;
 	}
 
-	*this_cpu_ptr(drvdata->handle) = handle;
+	cpudata->handle = handle;
 	cpudata->buf = buf;
 	cpudata->mode = mode;
 	return 0;
@@ -1112,16 +1113,15 @@ static int arm_trbe_disable(struct coresight_device *csdev)
 	trbe_drain_and_disable_local(cpudata);
 	clr_trbe_status();
 
-	*this_cpu_ptr(drvdata->handle) = NULL;
 	buf->cpudata = NULL;
+	cpudata->handle = NULL;
 	cpudata->buf = NULL;
 	cpudata->mode = CS_MODE_DISABLED;
 	return 0;
 }
 
-static void trbe_handle_spurious(struct perf_output_handle *handle)
+static void trbe_handle_spurious(struct trbe_cpudata *cpudata)
 {
-	struct trbe_buf *buf = etm_perf_sink_config(handle);
 	u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
 
 	/*
@@ -1129,11 +1129,12 @@ static void trbe_handle_spurious(struct perf_output_handle *handle)
 	 * back without modifying the buffer parameters to
 	 * retain the trace collected so far.
 	 */
-	set_trbe_enabled(buf->cpudata, trblimitr);
+	set_trbe_enabled(cpudata, trblimitr);
 }
 
-static int trbe_handle_overflow(struct perf_output_handle *handle)
+static int trbe_handle_overflow(struct trbe_cpudata *cpudata)
 {
+	struct perf_output_handle *handle = cpudata->handle;
 	struct perf_event *event = handle->event;
 	struct trbe_buf *buf = etm_perf_sink_config(handle);
 	unsigned long size;
@@ -1203,9 +1204,8 @@ static u64 cpu_prohibit_trace(void)
 
 static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 {
-	struct perf_output_handle **handle_ptr = dev;
-	struct perf_output_handle *handle = *handle_ptr;
-	struct trbe_buf *buf;
+	struct trbe_cpudata *cpudata = dev;
+	struct perf_output_handle *handle = cpudata->handle;
 	enum trbe_fault_action act;
 	u64 status;
 	bool truncated = false;
@@ -1232,18 +1232,17 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 	 * Ensure the trace is visible to the CPUs and
 	 * any external aborts have been resolved.
 	 */
-	buf = etm_perf_sink_config(handle);
-	trbe_drain_and_disable_local(buf->cpudata);
+	trbe_drain_and_disable_local(cpudata);
 	clr_trbe_irq();
 	isb();
 
 	act = trbe_get_fault_act(handle, status);
 	switch (act) {
 	case TRBE_FAULT_ACT_WRAP:
-		truncated = !!trbe_handle_overflow(handle);
+		truncated = !!trbe_handle_overflow(cpudata);
 		break;
 	case TRBE_FAULT_ACT_SPURIOUS:
-		trbe_handle_spurious(handle);
+		trbe_handle_spurious(cpudata);
 		break;
 	case TRBE_FAULT_ACT_FATAL:
 		trbe_truncate_event(handle);
@@ -1507,10 +1506,6 @@ static int arm_trbe_probe_coresight(struct trbe_drvdata *drvdata)
 {
 	int cpu;
 
-	drvdata->cpudata = alloc_percpu(typeof(*drvdata->cpudata));
-	if (!drvdata->cpudata)
-		return -ENOMEM;
-
 	for_each_cpu(cpu, &drvdata->supported_cpus) {
 		/* If we fail to probe the CPU, let us defer it to hotplug callbacks */
 		if (smp_call_function_single(cpu, arm_trbe_probe_cpu, drvdata, 1))
@@ -1531,7 +1526,6 @@ static int arm_trbe_remove_coresight(struct trbe_drvdata *drvdata)
 		smp_call_function_single(cpu, arm_trbe_disable_cpu, drvdata, 1);
 		arm_trbe_remove_coresight_cpu(drvdata, cpu);
 	}
-	free_percpu(drvdata->cpudata);
 	return 0;
 }
 
@@ -1618,14 +1612,14 @@ static int arm_trbe_probe_irq(struct platform_device *pdev,
 
 	cpumask_copy(&drvdata->supported_cpus, affinity);
 
-	drvdata->handle = alloc_percpu(struct perf_output_handle *);
-	if (!drvdata->handle)
+	drvdata->cpudata = alloc_percpu(typeof(*drvdata->cpudata));
+	if (!drvdata->cpudata)
 		return -ENOMEM;
 
 	ret = request_percpu_irq_affinity(drvdata->irq, arm_trbe_irq_handler, DRVNAME,
-					  affinity, drvdata->handle);
+					  affinity, drvdata->cpudata);
 	if (ret) {
-		free_percpu(drvdata->handle);
+		free_percpu(drvdata->cpudata);
 		return ret;
 	}
 	return 0;
@@ -1633,8 +1627,8 @@ static int arm_trbe_probe_irq(struct platform_device *pdev,
 
 static void arm_trbe_remove_irq(struct trbe_drvdata *drvdata)
 {
-	free_percpu_irq(drvdata->irq, drvdata->handle);
-	free_percpu(drvdata->handle);
+	free_percpu_irq(drvdata->irq, drvdata->cpudata);
+	free_percpu(drvdata->cpudata);
 }
 
 static int arm_trbe_device_probe(struct platform_device *pdev)
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 18/23
#41 lore raw

Re: [PATCH v2 18/23] coresight: trbe: Keep the perf handle in per-CPU data

Quoted 161 lines · Leo Yan

Arm SPE owns a per-CPU perf output handle for the PMU lifetime. TRBE instead receives a handle from CoreSight when enabling its sink.

Store that session handle in trbe_cpudata instead of a separate per-CPU pointer array, and pass the CPU data as the per-CPU IRQ context. Use this context directly for overflow and spurious interrupt handling so local hardware control does not depend on the current AUX buffer configuration.

Allocate the CPU data before registering the IRQ and free it after releasing the IRQ, keeping the IRQ context valid throughout registration.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 44 ++++++++++++----------------
  1 file changed, 19 insertions(+), 25 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +17 -23
index 9a469c8009ce8a8a8b804c2b63a68e741f1dceaa..2328aab0a084a0ac123e950f846f404d120ec2c0 100644
@@ -139,6 +139,7 @@ struct trbe_save_state {
   * @cpu			- CPU this TRBE belongs to.
   * @mode		- Mode of current operation. (perf/disabled)
   * @buf			- Buffer configuration retained for the enabled session.
+ * @handle		- Perf output handle from successful enable until disable.
   * @drvdata		- TRBE specific drvdata
   * @errata		- Bit map for the errata on this TRBE.
   */
@@ -149,6 +150,7 @@ struct trbe_cpudata {
  	int cpu;
  	enum cs_mode mode;
  	struct trbe_buf *buf;
+	struct perf_output_handle *handle;
  	struct trbe_drvdata *drvdata;
  	struct trbe_save_state save_state;
  	DECLARE_BITMAP(errata, TRBE_ERRATA_MAX);
@@ -156,7 +158,6 @@ struct trbe_cpudata {
 
  struct trbe_drvdata {
  	struct trbe_cpudata __percpu *cpudata;
-	struct perf_output_handle * __percpu *handle;
  	struct hlist_node hotplug_node;
  	int irq;
  	cpumask_t supported_cpus;
@@ -1091,7 +1092,7 @@ static int arm_trbe_enable(struct coresight_device *csdev, enum cs_mode mode,
  		return ret;
  	}
 
-	*this_cpu_ptr(drvdata->handle) = handle;
+	cpudata->handle = handle;
  	cpudata->buf = buf;
  	cpudata->mode = mode;
  	return 0;
@@ -1112,16 +1113,15 @@ static int arm_trbe_disable(struct coresight_device *csdev)
  	trbe_drain_and_disable_local(cpudata);
  	clr_trbe_status();
 
-	*this_cpu_ptr(drvdata->handle) = NULL;
  	buf->cpudata = NULL;
+	cpudata->handle = NULL;
  	cpudata->buf = NULL;
  	cpudata->mode = CS_MODE_DISABLED;
  	return 0;
  }
 
-static void trbe_handle_spurious(struct perf_output_handle *handle)
+static void trbe_handle_spurious(struct trbe_cpudata *cpudata)
  {
-	struct trbe_buf *buf = etm_perf_sink_config(handle);
  	u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
 
  	/*
@@ -1129,11 +1129,12 @@ static void trbe_handle_spurious(struct perf_output_handle *handle)
  	 * back without modifying the buffer parameters to
  	 * retain the trace collected so far.
  	 */
-	set_trbe_enabled(buf->cpudata, trblimitr);
+	set_trbe_enabled(cpudata, trblimitr);
  }
 
-static int trbe_handle_overflow(struct perf_output_handle *handle)
+static int trbe_handle_overflow(struct trbe_cpudata *cpudata)
  {
+	struct perf_output_handle *handle = cpudata->handle;
  	struct perf_event *event = handle->event;
  	struct trbe_buf *buf = etm_perf_sink_config(handle);
  	unsigned long size;
@@ -1203,9 +1204,8 @@ static u64 cpu_prohibit_trace(void)
 
  static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
  {
-	struct perf_output_handle **handle_ptr = dev;
-	struct perf_output_handle *handle = *handle_ptr;
-	struct trbe_buf *buf;
+	struct trbe_cpudata *cpudata = dev;
+	struct perf_output_handle *handle = cpudata->handle;
  	enum trbe_fault_action act;
  	u64 status;
  	bool truncated = false;
@@ -1232,18 +1232,17 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
  	 * Ensure the trace is visible to the CPUs and
  	 * any external aborts have been resolved.
  	 */
-	buf = etm_perf_sink_config(handle);
-	trbe_drain_and_disable_local(buf->cpudata);
+	trbe_drain_and_disable_local(cpudata);
  	clr_trbe_irq();
  	isb();
 
  	act = trbe_get_fault_act(handle, status);
  	switch (act) {
  	case TRBE_FAULT_ACT_WRAP:
-		truncated = !!trbe_handle_overflow(handle);
+		truncated = !!trbe_handle_overflow(cpudata);
  		break;
  	case TRBE_FAULT_ACT_SPURIOUS:
-		trbe_handle_spurious(handle);
+		trbe_handle_spurious(cpudata);
  		break;
  	case TRBE_FAULT_ACT_FATAL:
  		trbe_truncate_event(handle);
@@ -1507,10 +1506,6 @@ static int arm_trbe_probe_coresight(struct trbe_drvdata *drvdata)
  {
  	int cpu;
 
-	drvdata->cpudata = alloc_percpu(typeof(*drvdata->cpudata));
-	if (!drvdata->cpudata)
-		return -ENOMEM;
-
  	for_each_cpu(cpu, &drvdata->supported_cpus) {
  		/* If we fail to probe the CPU, let us defer it to hotplug callbacks */
  		if (smp_call_function_single(cpu, arm_trbe_probe_cpu, drvdata, 1))
@@ -1531,7 +1526,6 @@ static int arm_trbe_remove_coresight(struct trbe_drvdata *drvdata)
  		smp_call_function_single(cpu, arm_trbe_disable_cpu, drvdata, 1);
  		arm_trbe_remove_coresight_cpu(drvdata, cpu);
  	}
-	free_percpu(drvdata->cpudata);
  	return 0;
  }
 
@@ -1618,14 +1612,14 @@ static int arm_trbe_probe_irq(struct platform_device *pdev,
 
  	cpumask_copy(&drvdata->supported_cpus, affinity);
 
-	drvdata->handle = alloc_percpu(struct perf_output_handle *);
-	if (!drvdata->handle)
+	drvdata->cpudata = alloc_percpu(typeof(*drvdata->cpudata));
+	if (!drvdata->cpudata)
  		return -ENOMEM;
 
  	ret = request_percpu_irq_affinity(drvdata->irq, arm_trbe_irq_handler, DRVNAME,
-					  affinity, drvdata->handle);
+					  affinity, drvdata->cpudata);
  	if (ret) {
-		free_percpu(drvdata->handle);
+		free_percpu(drvdata->cpudata);
  		return ret;
  	}
  	return 0;

drivers/hwtracing/coresight/coresight-trbe.c static int arm_trbe_probe_irq(struct platform_device *pdev, · line 1634

Quoted 10 lines
drivers/hwtracing/coresight/coresight-trbe.c +2 -2
@@ -1633,8 +1627,8 @@ static int arm_trbe_probe_irq(struct platform_device *pdev,
 
  static void arm_trbe_remove_irq(struct trbe_drvdata *drvdata)
  {
-	free_percpu_irq(drvdata->irq, drvdata->handle);
-	free_percpu(drvdata->handle);
+	free_percpu_irq(drvdata->irq, drvdata->cpudata);
+	free_percpu(drvdata->cpudata);
  }
 
  static int arm_trbe_device_probe(struct platform_device *pdev)
author reviewer
#42 lore raw

PATCH v2 19/23 coresight: trbe: Reload the AUX buffer on overflow restart

In reply to Leo Yan

PERF_EVENT_IOC_SET_OUTPUT lets events share another event's mapped output buffer. For AUX events, this also selects the associated AUX area and sink configuration. Perf normally establishes this sharing during setup.

Consider the following hypothetical live-redirection sequence, permitted by the inspected perf-core paths. P is a parent event and C is its inherited event, with C->parent = P. A and B are mapped AUX destination buffers owned by separate, disabled cs_etm events in different task contexts. All three are per-task events restricted to the same CPU, with compatible output settings. P has no mmap of its own, allowing its output to be redirected while A and B remain mapped through their owner events.

1. P's output is redirected to A. C inherits from P and begins tracing on the selected CPU, so its open AUX transaction and TRBE hardware both use A. P can be inactive while C is running because their events are scheduled with their respective tasks.
2. A userspace controller calls PERF_EVENT_IOC_SET_OUTPUT on P to select B. ring_buffer_attach() calls perf_event_stop(P, 0), which returns for inactive P without stopping C. P->rb now selects B, but C's existing handle and hardware still use A.
3. C's next buffer overflow invokes trbe_handle_overflow() from the IRQ handler. It saves buf for A, ends that AUX transaction, then calls perf_aux_output_begin(handle, C). AUX begin follows C->parent and reads P's current rb, rebinding the same handle to B.

This is a hypothetical API sequence, not a reproduced TRBE failure or an observed buffer switch in normal perf record operation.

Without reloading buf, __arm_trbe_enable() calculates the next buffer using B through the handle, but its workaround and hardware setup still receive A. If B has never been enabled, its missing CPU association can cause the normal-offset calculation to dereference NULL. With a snapshot buffer, hardware may instead keep using A while perf accounts output to B.

Reload the sink configuration after perf_aux_output_begin() so the buffer and handle refer to the same AUX transaction. If the buffer is missing or its CPU association does not match the interrupt's CPU data, truncate the new transaction and leave TRBE disabled. Truncation requests perf to stop the event; a subsequent normal restart installs the buffer and its CPU association through the sink enable path.

Stopping inherited writers during output redirection remains a separate perf-core fix.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
drivers/hwtracing/coresight/coresight-trbe.c +14 -0
index 2328aab0a084a0ac123e950f846f404d120ec2c0..f1f102df6c387f53feaa12b89d3df89f734afcdb 100644
@@ -1161,6 +1161,20 @@ static int trbe_handle_overflow(struct trbe_cpudata *cpudata)
 	if (!event_data)
 		return -EINVAL;
 
+	/*
+	 * Reload the buffer after perf_aux_output_begin() so it matches the
+	 * handle. A race with PERF_EVENT_IOC_SET_OUTPUT may select a buffer
+	 * that has not been installed on this CPU and has no valid cpudata.
+	 * If the buffer is missing or its CPU association does not match,
+	 * truncate the trace to stop the event. A subsequent normal restart
+	 * installs the buffer and its CPU association through sink enable.
+	 */
+	buf = etm_perf_sink_config(handle);
+	if (!buf || buf->cpudata != cpudata) {
+		trbe_truncate_event(handle);
+		return -EINVAL;
+	}
+
 	return __arm_trbe_enable(buf, handle);
 }
 
Signature · 1 line
2.34.1
reviewer
#43 lore raw

Re: [PATCH v2 19/23] coresight: trbe: Reload the AUX buffer on overflow restart

Quoted 41 lines · Leo Yan

PERF_EVENT_IOC_SET_OUTPUT lets events share another event's mapped output buffer. For AUX events, this also selects the associated AUX area and sink configuration. Perf normally establishes this sharing during setup.

Consider the following hypothetical live-redirection sequence, permitted by the inspected perf-core paths. P is a parent event and C is its inherited event, with C->parent = P. A and B are mapped AUX destination buffers owned by separate, disabled cs_etm events in different task contexts. All three are per-task events restricted to the same CPU, with compatible output settings. P has no mmap of its own, allowing its output to be redirected while A and B remain mapped through their owner events.

1. P's output is redirected to A. C inherits from P and begins tracing on the selected CPU, so its open AUX transaction and TRBE hardware both use A. P can be inactive while C is running because their events are scheduled with their respective tasks.
2. A userspace controller calls PERF_EVENT_IOC_SET_OUTPUT on P to select B. ring_buffer_attach() calls perf_event_stop(P, 0), which returns for inactive P without stopping C. P->rb now selects B, but C's existing handle and hardware still use A.
3. C's next buffer overflow invokes trbe_handle_overflow() from the IRQ handler. It saves buf for A, ends that AUX transaction, then calls perf_aux_output_begin(handle, C). AUX begin follows C->parent and reads P's current rb, rebinding the same handle to B.

This is a hypothetical API sequence, not a reproduced TRBE failure or an observed buffer switch in normal perf record operation.

Without reloading buf, __arm_trbe_enable() calculates the next buffer using B through the handle, but its workaround and hardware setup still receive A. If B has never been enabled, its missing CPU association can cause the normal-offset calculation to dereference NULL. With a snapshot buffer, hardware may instead keep using A while perf accounts output to B.

Reload the sink configuration after perf_aux_output_begin() so the buffer and handle refer to the same AUX transaction. If the buffer is missing or its CPU association does not match the interrupt's CPU data, truncate the new transaction and leave TRBE disabled. Truncation requests perf to stop the event; a subsequent normal restart installs the buffer and its CPU association through the sink enable path.

Quoted 3 lines

Stopping inherited writers during output redirection remains a separate perf-core fix.

Shouldn't we do that instead of this? It seems much better to fix this in perf core rather than here which is hard to understand and appears to be a bit hacky, and doesn't apply to all users of aux buffers. I would expect that even when using PERF_EVENT_IOC_SET_OUTPUT, the buffer should be consistent between start and stops from the point of view of individual drivers. It makes sense to add a workaround in the driver if we can't control that behavior, but the code is in the same repo. Maybe we could even just leave this patch out and do the core fix later?

Unrelated to this, the sashiko report that says there is an out of bounds read if the output is switched to an ETF buffer seems to be pre-existing. But it's something worth thinking about.

Quoted 31 lines
---
  drivers/hwtracing/coresight/coresight-trbe.c | 14 ++++++++++++++
  1 file changed, 14 insertions(+)
drivers/hwtracing/coresight/coresight-trbe.c +14 -0
index 2328aab0a084a0ac123e950f846f404d120ec2c0..f1f102df6c387f53feaa12b89d3df89f734afcdb 100644
@@ -1161,6 +1161,20 @@ static int trbe_handle_overflow(struct trbe_cpudata *cpudata)
  	if (!event_data)
  		return -EINVAL;
 
+	/*
+	 * Reload the buffer after perf_aux_output_begin() so it matches the
+	 * handle. A race with PERF_EVENT_IOC_SET_OUTPUT may select a buffer
+	 * that has not been installed on this CPU and has no valid cpudata.
+	 * If the buffer is missing or its CPU association does not match,
+	 * truncate the trace to stop the event. A subsequent normal restart
+	 * installs the buffer and its CPU association through sink enable.
+	 */
+	buf = etm_perf_sink_config(handle);
+	if (!buf || buf->cpudata != cpudata) {
+		trbe_truncate_event(handle);
+		return -EINVAL;
+	}
+
  	return __arm_trbe_enable(buf, handle);
  }
 
author reviewer
#44 lore raw

PATCH v2 20/23 coresight: trbe: Add fault action argument to trbe_handle_overflow()

In reply to 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.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +4 -3
index f1f102df6c387f53feaa12b89d3df89f734afcdb..c83c440d4ac13c346bc2a7f9093951c186c90157 100644
@@ -1132,7 +1132,8 @@ static void trbe_handle_spurious(struct trbe_cpudata *cpudata)
 	set_trbe_enabled(cpudata, trblimitr);
 }
 
-static int trbe_handle_overflow(struct trbe_cpudata *cpudata)
+static int trbe_handle_overflow(struct trbe_cpudata *cpudata,
+				enum trbe_fault_action act)
 {
 	struct perf_output_handle *handle = cpudata->handle;
 	struct perf_event *event = handle->event;
@@ -1147,7 +1148,7 @@ static int trbe_handle_overflow(struct trbe_cpudata *cpudata)
 	 */
 	clr_trbe_status();
 
-	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;
 
@@ -1253,7 +1254,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 	act = trbe_get_fault_act(handle, status);
 	switch (act) {
 	case TRBE_FAULT_ACT_WRAP:
-		truncated = !!trbe_handle_overflow(cpudata);
+		truncated = !!trbe_handle_overflow(cpudata, act);
 		break;
 	case TRBE_FAULT_ACT_SPURIOUS:
 		trbe_handle_spurious(cpudata);
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 20/23
#45 lore raw

Re: [PATCH v2 20/23] coresight: trbe: Add fault action argument to trbe_handle_overflow()

Quoted 35 lines · 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.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 7 ++++---
  1 file changed, 4 insertions(+), 3 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +3 -2
index f1f102df6c387f53feaa12b89d3df89f734afcdb..c83c440d4ac13c346bc2a7f9093951c186c90157 100644
@@ -1132,7 +1132,8 @@ static void trbe_handle_spurious(struct trbe_cpudata *cpudata)
  	set_trbe_enabled(cpudata, trblimitr);
  }
 
-static int trbe_handle_overflow(struct trbe_cpudata *cpudata)
+static int trbe_handle_overflow(struct trbe_cpudata *cpudata,
+				enum trbe_fault_action act)
  {
  	struct perf_output_handle *handle = cpudata->handle;
  	struct perf_event *event = handle->event;
@@ -1147,7 +1148,7 @@ static int trbe_handle_overflow(struct trbe_cpudata *cpudata)
  	 */
  	clr_trbe_status();
 
-	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;
 

drivers/hwtracing/coresight/coresight-trbe.c static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) · line 1260

Quoted 8 lines
drivers/hwtracing/coresight/coresight-trbe.c +1 -1
@@ -1253,7 +1254,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
  	act = trbe_get_fault_act(handle, status);
  	switch (act) {
  	case TRBE_FAULT_ACT_WRAP:
-		truncated = !!trbe_handle_overflow(cpudata);
+		truncated = !!trbe_handle_overflow(cpudata, act);
  		break;
  	case TRBE_FAULT_ACT_SPURIOUS:
  		trbe_handle_spurious(cpudata);
author reviewer
#46 lore raw

PATCH v2 21/23 coresight: trbe: Read TRBSR after prohibiting and flushing trace

In reply to Leo Yan

The IRQ handler needs a stable TRBSR_EL1 value before classifying the interrupt. If the PE can still generate trace, or if pending trace has not been flushed, TRBSR can still change while the handler is sampling it. This is especially important for TRBSR.S bit, which is used to decide whether the TRBE has stopped.

Move the drain buffer into arm_trbe_prohibit_trace(), so the handler first prohibits trace and flushes trace before reading TRBSR_EL1. Once this has been done, the IRQ path no longer needs the unconditional trbe_drain_and_disable_local(); it only needs to disable TRBE before handling the stopped-buffer status.

Refactor the IRQ exit path accordingly: restore TRFCR on non-truncated exits, while the truncated path still runs the perf IRQ work to shut down the event.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 40 +++++++++++++++++++---------
 1 file changed, 28 insertions(+), 12 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +28 -12
index c83c440d4ac13c346bc2a7f9093951c186c90157..36f143a15e680b28aa6ca6a43ad7d3ee134693be 100644
@@ -1207,24 +1207,34 @@ static bool is_perf_trbe(struct perf_output_handle *handle)
 	return true;
 }
 
-static u64 cpu_prohibit_trace(void)
+static u64 arm_trbe_prohibit_trace(void)
 {
 	u64 trfcr = read_trfcr();
 
 	/* Prohibit tracing at EL0 & the kernel EL */
 	write_trfcr(trfcr & ~(TRFCR_EL1_ExTRE | TRFCR_EL1_E0TRE));
+
+	/* Ensure the program-flow trace is flushed */
+	trbe_drain_buffer();
+
 	/* Return the original value of the TRFCR */
 	return trfcr;
 }
 
+static void arm_trbe_allow_trace(u64 trfcr)
+{
+	/* Allow tracing at EL0 & the kernel EL */
+	write_trfcr(trfcr);
+}
+
 static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 {
 	struct trbe_cpudata *cpudata = dev;
 	struct perf_output_handle *handle = cpudata->handle;
 	enum trbe_fault_action act;
-	u64 status;
 	bool truncated = false;
-	u64 trfcr;
+	u64 status, trfcr;
+	irqreturn_t ret;
 
 	if (!handle || !perf_get_aux(handle))
 		return IRQ_NONE;
@@ -1232,22 +1242,25 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 	if (!is_perf_trbe(handle))
 		return IRQ_NONE;
 
+	/* Prohibit the CPU from tracing before we disable the TRBE */
+	trfcr = arm_trbe_prohibit_trace();
+
 	/* Reads to TRBSR_EL1 is fine when TRBE is active */
 	status = read_sysreg_s(SYS_TRBSR_EL1);
 	/*
 	 * If the pending IRQ was handled by update_buffer callback
 	 * we have nothing to do here.
 	 */
-	if (!is_trbe_irq(status))
-		return IRQ_NONE;
+	if (!is_trbe_irq(status)) {
+		ret = IRQ_NONE;
+		goto out;
+	}
 
-	/* Prohibit the CPU from tracing before we disable the TRBE */
-	trfcr = cpu_prohibit_trace();
 	/*
 	 * Ensure the trace is visible to the CPUs and
 	 * any external aborts have been resolved.
 	 */
-	trbe_drain_and_disable_local(cpudata);
+	set_trbe_disabled(cpudata);
 	clr_trbe_irq();
 	isb();
 
@@ -1272,12 +1285,15 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 	 * Otherwise, restore the trace filter controls to
 	 * allow the tracing.
 	 */
-	if (truncated)
+	if (truncated) {
 		irq_work_run();
-	else
-		write_trfcr(trfcr);
+		return IRQ_HANDLED;
+	}
 
-	return IRQ_HANDLED;
+	ret = IRQ_HANDLED;
+out:
+	arm_trbe_allow_trace(trfcr);
+	return ret;
 }
 
 static int arm_trbe_save(struct coresight_device *csdev)
Signature · 1 line
2.34.1
reviewer
#47 lore raw

Re: [PATCH v2 21/23] coresight: trbe: Read TRBSR after prohibiting and flushing trace

Quoted 64 lines · Leo Yan

The IRQ handler needs a stable TRBSR_EL1 value before classifying the interrupt. If the PE can still generate trace, or if pending trace has not been flushed, TRBSR can still change while the handler is sampling it. This is especially important for TRBSR.S bit, which is used to decide whether the TRBE has stopped.

Move the drain buffer into arm_trbe_prohibit_trace(), so the handler first prohibits trace and flushes trace before reading TRBSR_EL1. Once this has been done, the IRQ path no longer needs the unconditional trbe_drain_and_disable_local(); it only needs to disable TRBE before handling the stopped-buffer status.

Refactor the IRQ exit path accordingly: restore TRFCR on non-truncated exits, while the truncated path still runs the perf IRQ work to shut down the event.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 40 +++++++++++++++++++---------
  1 file changed, 28 insertions(+), 12 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +13 -3
index c83c440d4ac13c346bc2a7f9093951c186c90157..36f143a15e680b28aa6ca6a43ad7d3ee134693be 100644
@@ -1207,24 +1207,34 @@ static bool is_perf_trbe(struct perf_output_handle *handle)
  	return true;
  }
 
-static u64 cpu_prohibit_trace(void)
+static u64 arm_trbe_prohibit_trace(void)
  {
  	u64 trfcr = read_trfcr();
 
  	/* Prohibit tracing at EL0 & the kernel EL */
  	write_trfcr(trfcr & ~(TRFCR_EL1_ExTRE | TRFCR_EL1_E0TRE));
+
+	/* Ensure the program-flow trace is flushed */
+	trbe_drain_buffer();
+
  	/* Return the original value of the TRFCR */
  	return trfcr;
  }
 
+static void arm_trbe_allow_trace(u64 trfcr)
+{
+	/* Allow tracing at EL0 & the kernel EL */
+	write_trfcr(trfcr);
+}
+
  static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
  {
  	struct trbe_cpudata *cpudata = dev;
  	struct perf_output_handle *handle = cpudata->handle;
  	enum trbe_fault_action act;
-	u64 status;
  	bool truncated = false;
-	u64 trfcr;
+	u64 status, trfcr;
+	irqreturn_t ret;
 
  	if (!handle || !perf_get_aux(handle))
  		return IRQ_NONE;

drivers/hwtracing/coresight/coresight-trbe.c static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) · line 1249

Quoted 8 lines
drivers/hwtracing/coresight/coresight-trbe.c +3 -0
@@ -1232,22 +1242,25 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
  	if (!is_perf_trbe(handle))
  		return IRQ_NONE;
 
+	/* Prohibit the CPU from tracing before we disable the TRBE */
+	trfcr = arm_trbe_prohibit_trace();
+
  	/* Reads to TRBSR_EL1 is fine when TRBE is active */
  	status = read_sysreg_s(SYS_TRBSR_EL1);

I think sashiko is right, you need an isb() between arm_trbe_prohibit_trace() and the status read, otherwise it can still be stale.

Quoted 43 lines
drivers/hwtracing/coresight/coresight-trbe.c +12 -9
  	/*
  	 * If the pending IRQ was handled by update_buffer callback
  	 * we have nothing to do here.
  	 */
-	if (!is_trbe_irq(status))
-		return IRQ_NONE;
+	if (!is_trbe_irq(status)) {
+		ret = IRQ_NONE;
+		goto out;
+	}
 
-	/* Prohibit the CPU from tracing before we disable the TRBE */
-	trfcr = cpu_prohibit_trace();
  	/*
  	 * Ensure the trace is visible to the CPUs and
  	 * any external aborts have been resolved.
  	 */
-	trbe_drain_and_disable_local(cpudata);
+	set_trbe_disabled(cpudata);
  	clr_trbe_irq();
  	isb();
 
@@ -1272,12 +1285,15 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
  	 * Otherwise, restore the trace filter controls to
  	 * allow the tracing.
  	 */
-	if (truncated)
+	if (truncated) {
  		irq_work_run();
-	else
-		write_trfcr(trfcr);
+		return IRQ_HANDLED;
+	}
 
-	return IRQ_HANDLED;
+	ret = IRQ_HANDLED;
+out:
+	arm_trbe_allow_trace(trfcr);
+	return ret;
  }
 
  static int arm_trbe_save(struct coresight_device *csdev)
author reviewer
#48 lore raw

PATCH v2 22/23 coresight: trbe: Restart trace source for stopped buffers

In reply to Leo Yan

When TRBSR reports a stopped buffer, the IRQ handler closes the current AUX buffer and programs TRBE for the next run. This creates a trace discontinuity. User tool has to reset the decoder when moving to the next AUX data block, which can reduce decoded trace coverage.

Stop and restart the trace source around TRBE reprogramming for stopped buffers. This emits ASYNC packets, allowing the decoder to recognize discontinuity without a full reset.

This duplicates part of the trace source enable/disable code in the ETMv4 driver. Reusing that code would require splitting the ETMv4 callbacks into finer-grained helpers that TRBE can call, which would make the change much larger. Keep the TRBE change self-contained for now.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 121 +++++++++++++++++++++++++--
 1 file changed, 116 insertions(+), 5 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +116 -5
index 36f143a15e680b28aa6ca6a43ad7d3ee134693be..6bc82d5b55539795059da37039d2929b4ea504e3 100644
@@ -17,9 +17,11 @@
 
 #include <asm/barrier.h>
 #include <asm/cpufeature.h>
+#include <linux/iopoll.h>
 #include <linux/kvm_host.h>
 #include <linux/vmalloc.h>
 
+#include "coresight-etm4x.h"
 #include "coresight-self-hosted-trace.h"
 #include "coresight-trbe.h"
 
@@ -1132,14 +1134,106 @@ static void trbe_handle_spurious(struct trbe_cpudata *cpudata)
 	set_trbe_enabled(cpudata, trblimitr);
 }
 
+static u64 read_sys_trcstatr(void)
+{
+	/* Context synchronization for each polling */
+	isb();
+	return read_sysreg_s(SYS_TRCSTATR);
+}
+
+static int arm_trbe_start_tracer(u64 trcprgctlr)
+{
+	u64 val;
+
+	/* A pending IRQ after AUX pause? */
+	if (!(trcprgctlr & BIT(0)))
+		return 0;
+
+	/* Restore the saved tracer state */
+	write_sysreg_s(trcprgctlr, SYS_TRCPRGCTLR);
+
+	/*
+	 * Context synchronization between writing to the TRCPRGCTLR and
+	 * reading the TRCSTATR.
+	 */
+	isb();
+
+	/* Wait for TRCSTATR.IDLE to go down to 0 */
+	if (read_poll_timeout_atomic(read_sys_trcstatr, val,
+				     !(val & BIT(TRCSTATR_IDLE_BIT)),
+				     1,		/* delay_us */
+				     TIMEOUT_US, /* timeout_us */
+				     false)) {	/* delay_before_read */
+		pr_err("TRBE: timeout for TRCSTATR IDLE\n");
+		return -ETIME;
+	}
+
+	/* Context synchronization after programming the tracer */
+	isb();
+
+	return 0;
+}
+
+static int arm_trbe_stop_tracer(u64 *trcprgctlr)
+{
+	u64 val;
+
+	/* Disable tracer and save trcprgctlr */
+	*trcprgctlr = read_sysreg_s(SYS_TRCPRGCTLR);
+
+	/* No need to stop tracer again */
+	if (!(*trcprgctlr & BIT(0)))
+		return 0;
+
+	write_sysreg_s(*trcprgctlr & ~BIT(0), SYS_TRCPRGCTLR);
+
+	/*
+	 * Context synchronization between writing to the TRCPRGCTLR and
+	 * reading the TRCSTATR.
+	 */
+	isb();
+
+	/*
+	 * PMSTABLE alone does not permit re-enabling the tracer.
+	 * Wait for TRCSTATR.IDLE to go to 1.
+	 */
+	if (read_poll_timeout_atomic(read_sys_trcstatr, val,
+				     val & BIT(TRCSTATR_IDLE_BIT),
+				     1,
+				     TIMEOUT_US,
+				     false)) {
+		pr_err("TRBE: timeout for TRCSTATR IDLE=1\n");
+		return -ETIME;
+	}
+
+	/* Context synchronization after programming the tracer */
+	isb();
+
+	return 0;
+}
+
 static int trbe_handle_overflow(struct trbe_cpudata *cpudata,
-				enum trbe_fault_action act)
+				enum trbe_fault_action act,
+				bool is_buffer_stopped)
 {
 	struct perf_output_handle *handle = cpudata->handle;
 	struct perf_event *event = handle->event;
 	struct trbe_buf *buf = etm_perf_sink_config(handle);
 	unsigned long size;
 	struct etm_event_data *event_data;
+	u64 trcprgctlr = 0;
+	int ret = 0;
+
+	/*
+	 * Since the trace buffer has stopped, disable and re-enable the
+	 * tracer to generate ASYNC packets. This allows the decoder to
+	 * recognize the discontinuity in the trace stream.
+	 */
+	if (is_buffer_stopped) {
+		ret = arm_trbe_stop_tracer(&trcprgctlr);
+		if (ret)
+			goto truncate_event;
+	}
 
 	/*
 	 * Clear the status. No context synchronization is required here,
@@ -1172,11 +1266,27 @@ static int trbe_handle_overflow(struct trbe_cpudata *cpudata,
 	 */
 	buf = etm_perf_sink_config(handle);
 	if (!buf || buf->cpudata != cpudata) {
-		trbe_truncate_event(handle);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto truncate_event;
+	}
+
+	ret = __arm_trbe_enable(buf, handle);
+	if (ret)
+		return ret;
+
+	if (is_buffer_stopped) {
+		ret = arm_trbe_start_tracer(trcprgctlr);
+		if (ret) {
+			trbe_drain_and_disable_local(cpudata);
+			goto truncate_event;
+		}
 	}
 
-	return __arm_trbe_enable(buf, handle);
+	return 0;
+
+truncate_event:
+	trbe_truncate_event(handle);
+	return ret;
 }
 
 static bool is_perf_trbe(struct perf_output_handle *handle)
@@ -1267,7 +1377,8 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 	act = trbe_get_fault_act(handle, status);
 	switch (act) {
 	case TRBE_FAULT_ACT_WRAP:
-		truncated = !!trbe_handle_overflow(cpudata, act);
+		truncated = !!trbe_handle_overflow(cpudata, act,
+						   !is_trbe_running(status));
 		break;
 	case TRBE_FAULT_ACT_SPURIOUS:
 		trbe_handle_spurious(cpudata);
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 22/23
#49 lore raw

Re: [PATCH v2 22/23] coresight: trbe: Restart trace source for stopped buffers

Quoted 177 lines · Leo Yan

When TRBSR reports a stopped buffer, the IRQ handler closes the current AUX buffer and programs TRBE for the next run. This creates a trace discontinuity. User tool has to reset the decoder when moving to the next AUX data block, which can reduce decoded trace coverage.

Stop and restart the trace source around TRBE reprogramming for stopped buffers. This emits ASYNC packets, allowing the decoder to recognize discontinuity without a full reset.

This duplicates part of the trace source enable/disable code in the ETMv4 driver. Reusing that code would require splitting the ETMv4 callbacks into finer-grained helpers that TRBE can call, which would make the change much larger. Keep the TRBE change self-contained for now.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 121 +++++++++++++++++++++++++--
  1 file changed, 116 insertions(+), 5 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +114 -4
index 36f143a15e680b28aa6ca6a43ad7d3ee134693be..6bc82d5b55539795059da37039d2929b4ea504e3 100644
@@ -17,9 +17,11 @@
 
  #include <asm/barrier.h>
  #include <asm/cpufeature.h>
+#include <linux/iopoll.h>
  #include <linux/kvm_host.h>
  #include <linux/vmalloc.h>
 
+#include "coresight-etm4x.h"
  #include "coresight-self-hosted-trace.h"
  #include "coresight-trbe.h"
 
@@ -1132,14 +1134,106 @@ static void trbe_handle_spurious(struct trbe_cpudata *cpudata)
  	set_trbe_enabled(cpudata, trblimitr);
  }
 
+static u64 read_sys_trcstatr(void)
+{
+	/* Context synchronization for each polling */
+	isb();
+	return read_sysreg_s(SYS_TRCSTATR);
+}
+
+static int arm_trbe_start_tracer(u64 trcprgctlr)
+{
+	u64 val;
+
+	/* A pending IRQ after AUX pause? */
+	if (!(trcprgctlr & BIT(0)))
+		return 0;
+
+	/* Restore the saved tracer state */
+	write_sysreg_s(trcprgctlr, SYS_TRCPRGCTLR);
+
+	/*
+	 * Context synchronization between writing to the TRCPRGCTLR and
+	 * reading the TRCSTATR.
+	 */
+	isb();
+
+	/* Wait for TRCSTATR.IDLE to go down to 0 */
+	if (read_poll_timeout_atomic(read_sys_trcstatr, val,
+				     !(val & BIT(TRCSTATR_IDLE_BIT)),
+				     1,		/* delay_us */
+				     TIMEOUT_US, /* timeout_us */
+				     false)) {	/* delay_before_read */
+		pr_err("TRBE: timeout for TRCSTATR IDLE\n");
+		return -ETIME;
+	}
+
+	/* Context synchronization after programming the tracer */
+	isb();
+
+	return 0;
+}
+
+static int arm_trbe_stop_tracer(u64 *trcprgctlr)
+{
+	u64 val;
+
+	/* Disable tracer and save trcprgctlr */
+	*trcprgctlr = read_sysreg_s(SYS_TRCPRGCTLR);
+
+	/* No need to stop tracer again */
+	if (!(*trcprgctlr & BIT(0)))
+		return 0;
+
+	write_sysreg_s(*trcprgctlr & ~BIT(0), SYS_TRCPRGCTLR);
+
+	/*
+	 * Context synchronization between writing to the TRCPRGCTLR and
+	 * reading the TRCSTATR.
+	 */
+	isb();
+
+	/*
+	 * PMSTABLE alone does not permit re-enabling the tracer.
+	 * Wait for TRCSTATR.IDLE to go to 1.
+	 */
+	if (read_poll_timeout_atomic(read_sys_trcstatr, val,
+				     val & BIT(TRCSTATR_IDLE_BIT),
+				     1,
+				     TIMEOUT_US,
+				     false)) {
+		pr_err("TRBE: timeout for TRCSTATR IDLE=1\n");
+		return -ETIME;
+	}
+
+	/* Context synchronization after programming the tracer */
+	isb();
+
+	return 0;
+}
+
  static int trbe_handle_overflow(struct trbe_cpudata *cpudata,
-				enum trbe_fault_action act)
+				enum trbe_fault_action act,
+				bool is_buffer_stopped)
  {
  	struct perf_output_handle *handle = cpudata->handle;
  	struct perf_event *event = handle->event;
  	struct trbe_buf *buf = etm_perf_sink_config(handle);
  	unsigned long size;
  	struct etm_event_data *event_data;
+	u64 trcprgctlr = 0;
+	int ret = 0;
+
+	/*
+	 * Since the trace buffer has stopped, disable and re-enable the
+	 * tracer to generate ASYNC packets. This allows the decoder to
+	 * recognize the discontinuity in the trace stream.
+	 */
+	if (is_buffer_stopped) {
+		ret = arm_trbe_stop_tracer(&trcprgctlr);
+		if (ret)
+			goto truncate_event;
+	}
 
  	/*
  	 * Clear the status. No context synchronization is required here,
@@ -1172,11 +1266,27 @@ static int trbe_handle_overflow(struct trbe_cpudata *cpudata,
  	 */
  	buf = etm_perf_sink_config(handle);
  	if (!buf || buf->cpudata != cpudata) {
-		trbe_truncate_event(handle);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto truncate_event;
+	}
+
+	ret = __arm_trbe_enable(buf, handle);
+	if (ret)
+		return ret;
+
+	if (is_buffer_stopped) {
+		ret = arm_trbe_start_tracer(trcprgctlr);
+		if (ret) {
+			trbe_drain_and_disable_local(cpudata);
+			goto truncate_event;
+		}
  	}
 
-	return __arm_trbe_enable(buf, handle);
+	return 0;
+
+truncate_event:
+	trbe_truncate_event(handle);
+	return ret;
  }
 
  static bool is_perf_trbe(struct perf_output_handle *handle)

drivers/hwtracing/coresight/coresight-trbe.c static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) · line 1384

Quoted 9 lines
drivers/hwtracing/coresight/coresight-trbe.c +2 -1
@@ -1267,7 +1377,8 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
  	act = trbe_get_fault_act(handle, status);
  	switch (act) {
  	case TRBE_FAULT_ACT_WRAP:
-		truncated = !!trbe_handle_overflow(cpudata, act);
+		truncated = !!trbe_handle_overflow(cpudata, act,
+						   !is_trbe_running(status));
  		break;
  	case TRBE_FAULT_ACT_SPURIOUS:
  		trbe_handle_spurious(cpudata);
author reviewer
#50 lore raw

PATCH v2 23/23 coresight: trbe: Report trace gaps with PARTIAL instead of COLLISION

In reply to Leo Yan

When a fill-mode wrap stops TRBE, the trace source can continue running until software handles the stop. Trace generated during this interval may be lost.

The overflow path now stops and restarts the trace source around buffer reprogramming. This emits ASYNC packets so the decoder can synchronize with the next trace segment. Use PERF_AUX_FLAG_PARTIAL to report the possible gap instead of PERF_AUX_FLAG_COLLISION.

Set PARTIAL in both the overflow handler and the buffer update callback. This also preserves the gap indication when event shutdown handles a pending wrap, even if no further trace segment follows. Limit the flag to stopped buffers.

---
 drivers/hwtracing/coresight/coresight-trbe.c | 29 +++++++---------------------
 1 file changed, 7 insertions(+), 22 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +7 -22
index 6bc82d5b55539795059da37039d2929b4ea504e3..29a1882b1f3c2eb7166f71eeef37badece67039c 100644
@@ -285,25 +285,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)
 {
 	perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
@@ -917,10 +898,12 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev,
 		perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
 		size = 0;
 		goto done;
-	} else if (act == TRBE_FAULT_ACT_WRAP) {
-		trbe_report_wrap_event(handle);
 	}
 
+	/* Trace may have been lost before the source was stopped. */
+	if (act == TRBE_FAULT_ACT_WRAP && !is_trbe_running(status))
+		perf_aux_output_flag(handle, PERF_AUX_FLAG_PARTIAL);
+
 	size = trbe_get_trace_size(handle, buf, act == TRBE_FAULT_ACT_WRAP);
 
 done:
@@ -1246,7 +1229,9 @@ static int trbe_handle_overflow(struct trbe_cpudata *cpudata,
 	if (buf->snapshot)
 		handle->head += size;
 
-	trbe_report_wrap_event(handle);
+	if (is_buffer_stopped)
+		perf_aux_output_flag(handle, PERF_AUX_FLAG_PARTIAL);
+
 	perf_aux_output_end(handle, size);
 	event_data = perf_aux_output_begin(handle, event);
 	/*
Signature · 1 line
2.34.1
reviewer Reviewed-by on v2 23/23
#51 lore raw

Re: [PATCH v2 23/23] coresight: trbe: Report trace gaps with PARTIAL instead of COLLISION

Quoted 66 lines · Leo Yan

When a fill-mode wrap stops TRBE, the trace source can continue running until software handles the stop. Trace generated during this interval may be lost.

The overflow path now stops and restarts the trace source around buffer reprogramming. This emits ASYNC packets so the decoder can synchronize with the next trace segment. Use PERF_AUX_FLAG_PARTIAL to report the possible gap instead of PERF_AUX_FLAG_COLLISION.

Set PARTIAL in both the overflow handler and the buffer update callback. This also preserves the gap indication when event shutdown handles a pending wrap, even if no further trace segment follows. Limit the flag to stopped buffers.

---
  drivers/hwtracing/coresight/coresight-trbe.c | 29 +++++++---------------------
  1 file changed, 7 insertions(+), 22 deletions(-)
drivers/hwtracing/coresight/coresight-trbe.c +4 -21
index 6bc82d5b55539795059da37039d2929b4ea504e3..29a1882b1f3c2eb7166f71eeef37badece67039c 100644
@@ -285,25 +285,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)
  {
  	perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
@@ -917,10 +898,12 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev,
  		perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
  		size = 0;
  		goto done;
-	} else if (act == TRBE_FAULT_ACT_WRAP) {
-		trbe_report_wrap_event(handle);
  	}
 
+	/* Trace may have been lost before the source was stopped. */
+	if (act == TRBE_FAULT_ACT_WRAP && !is_trbe_running(status))
+		perf_aux_output_flag(handle, PERF_AUX_FLAG_PARTIAL);
+
  	size = trbe_get_trace_size(handle, buf, act == TRBE_FAULT_ACT_WRAP);
 
  done:

drivers/hwtracing/coresight/coresight-trbe.c static int trbe_handle_overflow(struct trbe_cpudata *cpudata, · line 1237

Quoted 10 lines
drivers/hwtracing/coresight/coresight-trbe.c +3 -1
@@ -1246,7 +1229,9 @@ static int trbe_handle_overflow(struct trbe_cpudata *cpudata,
  	if (buf->snapshot)
  		handle->head += size;
 
-	trbe_report_wrap_event(handle);
+	if (is_buffer_stopped)
+		perf_aux_output_flag(handle, PERF_AUX_FLAG_PARTIAL);
+
  	perf_aux_output_end(handle, size);
  	event_data = perf_aux_output_begin(handle, event);
  	/*