perf: cs-etm: HW_ID improvements

PATCH Applied James Clark 4 participants 16 messages Oct 2 – Oct 22

Subsystems (3)

perf events Arm/CoreSight framework and drivers Performance events tooling arm64

lore

Share & tools

lore

James Clark

author reviewer

2026-10-02 11:15 UTC #0 lore raw

v3 cover perf: cs-etm: HW_ID improvements

The driver might not always be able to write HW_IDs, but we don't need them for unformatted mode anyway, so fix that in commit ("perf cs-etm:
Synthesize missing HW_ID mappings for raw trace").

Include some other minor improvements encountered on the way to appease Sashiko, like properly freeing queues and ("perf: cs-etm: Respect --no-itrace option") makes debugging broken Coresight perf.data files easier.

Signed-off-by James Clark <[email protected]>

---
Changes in v3:
- Remove old b4 prerequisite metadata that's no longer relevant.
- Link to v2: https://patch.msgid.link/20261002-james-cs-hw_id-output-failure-v2-0-7a194aa61173@linaro.org

Changes in v2:
- Drop driver changes and move to https://lore.kernel.org/linux-perf-users/[email protected]/T/#t
- Don't process any HW_IDs for unformatted queues, just create decoders
unconditionally with trace ID 1.
- Use zfree() instead of etmq->decoder = NULL
- Remove dependency on "[PATCH v2 00/14] perf cs-etm: Per-thread mode
fixes and snapshot wrap support", it wasn't functionally dependant,
just touching some of the same lines.
- Link to v1: https://patch.msgid.link/20260821-james-cs-hw_id-output-failure-v1-0-9d532ddabcc3@linaro.org

---
James Clark (3):
perf: cs-etm: Don't add global v0 HW_IDs to unformatted queues perf cs-etm: Free partially created queues
perf: cs-etm: Respect --no-itrace option

Leo Yan (1):
perf cs-etm: Synthesize missing HW_ID mappings for raw trace

 tools/perf/util/auxtrace.c                      |   2 +-
 tools/perf/util/auxtrace.h                      |   1 +
 tools/perf/util/cs-etm-decoder/cs-etm-decoder.c |  12 +-
 tools/perf/util/cs-etm-decoder/cs-etm-decoder.h |   2 +-
 tools/perf/util/cs-etm.c                        | 184 +++++++++++++++---------
 5 files changed, 123 insertions(+), 78 deletions(-)

---
base-commit: 705da5b15ab89ba97b11eedbe507c2fd83d31cb9
change-id: 20260706-james-cs-hw_id-output-failure-1d06d042ef96

Best regards,

Signature · 1 line

James Clark <[email protected]>

James Clark

author reviewer

2026-10-02 11:15 UTC #1 lore raw

PATCH v3 1/4 perf: cs-etm: Don't add global v0 HW_IDs to unformatted queues

In the case of mixed formatted and unformatted trace with v0 HW_IDs, or no HW_IDs at all resulting in the cs_etm__map_trace_ids_metadata() fallback, formatted queues will write their HW_IDs to unformatted queues. This will result in an assert because of multiple decoders on an unformatted queue.

Fix it by not inserting trace ID nodes on other unformatted queues. This isn't an issue for v0.1 HW_IDs because they aren't global so will only add to their own queue.

Fixes 19c3e4db38c5 ("perf: cs-etm: Create decoders based on the trace ID mappings")

Signed-off-by James Clark <[email protected]>

---
 tools/perf/util/cs-etm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

tools/perf/util/cs-etm.c +6 -6

index 2d1ab34f7b6b..fda6b6c02e26 100644
@@ -309,27 +309,27 @@ static int cs_etm__map_trace_id_v0(struct cs_etm_auxtrace *etm, u8 trace_chan_id
 {
 	struct cs_etm_queue *etmq;
 
-	/*
-	 * If the queue is unformatted then only save one mapping in the
-	 * queue associated with that CPU so only one decoder is made.
-	 */
 	etmq = cs_etm__get_queue(etm, cpu_metadata[CS_ETM_CPU]);
 	if (!etmq)
 		return -EINVAL;
 
+	/*
+	 * If the queue is unformatted then only save one mapping in the
+	 * queue associated with that CPU so only one decoder is made.
+	 */
 	if (etmq->format == UNFORMATTED)
 		return cs_etm__insert_trace_id_node(etmq, trace_chan_id,
 						    cpu_metadata);
 
 	/*
 	 * Otherwise, version 0 trace IDs are global so save them into every
-	 * queue.
+	 * formatted queue.
 	 */
 	for (unsigned int i = 0; i < etm->queues.nr_queues; ++i) {
 		int ret;
 
 		etmq = etm->queues.queue_array[i].priv;
-		if (!etmq)
+		if (!etmq || etmq->format == UNFORMATTED)
 			continue;
 
 		ret = cs_etm__insert_trace_id_node(etmq, trace_chan_id,

Signature · 1 line

2.43.0

[email protected]

AI review

2026-10-02 11:28 UTC #2 lore raw

Re: [PATCH v3 1/4] perf: cs-etm: Don't add global v0 HW_IDs to unformatted queues

Sashiko has reviewed this patch and found no issues. It looks great!

Quoted 6 lines

In the case of mixed formatted and unformatted trace with v0 HW_IDs, or no HW_IDs at all resulting in the cs_etm__map_trace_ids_metadata() fallback, formatted queues will write their HW_IDs to unformatted queues. This will result in an assert because of multiple decoders on an unformatted queue.

Quoted 6 lines

Fix it by not inserting trace ID nodes on other unformatted queues. This isn't an issue for v0.1 HW_IDs because they aren't global so will only add to their own queue.
Fixes 19c3e4db38c5 ("perf: cs-etm: Create decoders based on the trace ID mappings")
Signed-off-by James Clark <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/20261002-james-cs-hw_id-output-failure-v3-0-1d8b6ef339c8@linaro.org?part=1

Leo Yan

reviewer Reviewed-by on v3 1/4

2026-10-02 16:02 UTC #3 lore raw

Re: [PATCH v3 1/4] perf: cs-etm: Don't add global v0 HW_IDs to unformatted queues

In reply to James Clark

Quoted 6 lines · James Clark

In the case of mixed formatted and unformatted trace with v0 HW_IDs, or no HW_IDs at all resulting in the cs_etm__map_trace_ids_metadata() fallback, formatted queues will write their HW_IDs to unformatted queues. This will result in an assert because of multiple decoders on an unformatted queue.

Quoted 6 lines

Fix it by not inserting trace ID nodes on other unformatted queues. This isn't an issue for v0.1 HW_IDs because they aren't global so will only add to their own queue.
Fixes 19c3e4db38c5 ("perf: cs-etm: Create decoders based on the trace ID mappings")
Signed-off-by James Clark <[email protected]>

Reviewed-by Leo Yan <[email protected]>

James Clark

author reviewer

2026-10-02 11:15 UTC #4 lore raw

PATCH v3 2/4 perf cs-etm: Free partially created queues

In reply to James Clark

cs_etm__create_decoders() can fail half way through and the err_free_queues label won't free them. Call the full queue free function here instead. If no decoders were created it's a nop.

Reviewed-by Leo Yan <[email protected]>

Signed-off-by James Clark <[email protected]>

---
 tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 12 ++++++------
 tools/perf/util/cs-etm-decoder/cs-etm-decoder.h |  2 +-
 tools/perf/util/cs-etm.c                        | 25 ++++++++++++++++---------
 3 files changed, 23 insertions(+), 16 deletions(-)

tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +6 -6

index 26940f1f1b0b..b56cf57a68c8 100644
@@ -741,7 +741,7 @@ cs_etm_decoder__new(int decoders, struct cs_etm_decoder_params *d_params,
 	return decoder;
 
 err_free_decoder:
-	cs_etm_decoder__free(decoder);
+	cs_etm_decoder__zfree(&decoder);
 	return NULL;
 }

@@ -793,14 +793,14 @@ int cs_etm_decoder__process_data_block(struct cs_etm_decoder *decoder,
 	return ret;
 }
 
-void cs_etm_decoder__free(struct cs_etm_decoder *decoder)
+void cs_etm_decoder__zfree(struct cs_etm_decoder **decoder)
 {
-	if (!decoder)
+	if (!(*decoder))
 		return;
 
-	ocsd_destroy_dcd_tree(decoder->dcd_tree);
-	decoder->dcd_tree = NULL;
-	free(decoder);
+	ocsd_destroy_dcd_tree((*decoder)->dcd_tree);
+	(*decoder)->dcd_tree = NULL;
+	zfree(decoder);
 }
 
 const char *cs_etm_decoder__get_name(struct cs_etm_decoder *decoder)

tools/perf/util/cs-etm-decoder/cs-etm-decoder.h +1 -1

index 12c782fa6db2..2a8214f17958 100644
@@ -96,7 +96,7 @@ cs_etm_decoder__new(int num_cpu,
 		    struct cs_etm_decoder_params *d_params,
 		    struct cs_etm_trace_params t_params[]);
 
-void cs_etm_decoder__free(struct cs_etm_decoder *decoder);
+void cs_etm_decoder__zfree(struct cs_etm_decoder **decoder);
 
 int cs_etm_decoder__add_mem_access_cb(struct cs_etm_decoder *decoder,
 				      u64 start, u64 end,

tools/perf/util/cs-etm.c +16 -9

index fda6b6c02e26..78ae8443ac96 100644
@@ -1002,7 +1002,7 @@ static void cs_etm__free_queue(void *priv)
 	if (!etmq)
 		return;
 
-	cs_etm_decoder__free(etmq->decoder);
+	cs_etm_decoder__zfree(&etmq->decoder);
 	cs_etm__free_traceid_queues(etmq);
 
 	if (etmq->own_traceid_list) {

@@ -1017,13 +1017,10 @@ static void cs_etm__free_queue(void *priv)
 	free(etmq);
 }
 
-static void cs_etm__free_events(struct perf_session *session)
+static void cs_etm__free_queues(struct cs_etm_auxtrace *etm)
 {
 	unsigned int i;
-	struct cs_etm_auxtrace *aux = container_of(session->auxtrace,
-						   struct cs_etm_auxtrace,
-						   auxtrace);
-	struct auxtrace_queues *queues = &aux->queues;
+	struct auxtrace_queues *queues = &etm->queues;
 
 	for (i = 0; i < queues->nr_queues; i++) {
 		cs_etm__free_queue(queues->queue_array[i].priv);

@@ -1033,13 +1030,23 @@ static void cs_etm__free_events(struct perf_session *session)
 	auxtrace_queues__free(queues);
 }
 
+static void cs_etm__free_events(struct perf_session *session)
+{
+	struct cs_etm_auxtrace *aux = container_of(session->auxtrace,
+						   struct cs_etm_auxtrace,
+						   auxtrace);
+
+	cs_etm__free_queues(aux);
+}
+
 static void cs_etm__free(struct perf_session *session)
 {
 	int i;
 	struct cs_etm_auxtrace *aux = container_of(session->auxtrace,
 						   struct cs_etm_auxtrace,
 						   auxtrace);
-	cs_etm__free_events(session);
+
+	cs_etm__free_queues(aux);
 	session->auxtrace = NULL;
 
 	for (i = 0; i < aux->num_cpu; i++)

@@ -3476,7 +3483,7 @@ static int cs_etm__create_queue_decoders(struct cs_etm_queue *etmq)
 	return 0;
 
 out_free_decoder:
-	cs_etm_decoder__free(etmq->decoder);
+	cs_etm_decoder__zfree(&etmq->decoder);
 out_free:
 	zfree(&t_params);
 	return -EINVAL;

@@ -3758,7 +3765,7 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
 	return 0;
 
 err_free_queues:
-	auxtrace_queues__free(&etm->queues);
+	cs_etm__free_queues(etm);
 	session->auxtrace = NULL;
 err_free_etm:
 	zfree(&etm);

Signature · 1 line

2.43.0

[email protected]

AI review

2026-10-02 11:27 UTC #5 lore raw

Re: [PATCH v3 2/4] perf cs-etm: Free partially created queues

Sashiko has reviewed this patch and found no issues. It looks great!

Quoted 6 lines

cs_etm__create_decoders() can fail half way through and the err_free_queues label won't free them. Call the full queue free function here instead. If no decoders were created it's a nop.
Reviewed-by Leo Yan <[email protected]>
Signed-off-by James Clark <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/20261002-james-cs-hw_id-output-failure-v3-0-1d8b6ef339c8@linaro.org?part=2

James Clark

author reviewer

2026-10-02 11:15 UTC #6 lore raw

PATCH v3 3/4 perf cs-etm: Synthesize missing HW_ID mappings for raw trace

In reply to James Clark

From: Leo Yan <[email protected]>

The CoreSight driver emits PERF_RECORD_AUX_OUTPUT_HW_ID via perf_report_aux_output_id(). If the perf ring buffer is full, perf_report_aux_output_id() can fail to emit the record and return silently.

When at least one HW_ID record is present in the file, perf skips the legacy metadata-only trace ID mapping path. Therefore any raw AUX queue whose HW_ID record was dropped can be left with an empty traceid_list, and cs_etm__create_queue_decoders() will not create a decoder for that queue.

Fix it by ignoring all HW_IDs for unformatted queues and always create one decoder per queue unconditionally. Trace ID is unused so we can hard code it to 1 (0 is a special value used in 'pending_timestamp_chan_id' to indicate not pending). We also need put the same trace ID in the CPU metadata because the relationship between CPU metadata and traceid_list is bidirectional. Moving cs_etm__metadata_set_trace_id() into the mapping function solves this and simplifies existing code. cs_etm__insert_trace_id_node() now does two things, so rename it to a more generic cs_etm__map_trace_id().

Signed-off-by Leo Yan <[email protected]>

Signed-off-by James Clark <[email protected]>

---
 tools/perf/util/cs-etm.c | 85 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 58 insertions(+), 27 deletions(-)

tools/perf/util/cs-etm.c +58 ~~-27~~

index 78ae8443ac96..c0152549e3e2 100644
@@ -243,8 +243,12 @@ enum cs_etm_pid_fmt cs_etm__get_pid_fmt(struct cs_etm_queue *etmq)
 	return etmq->etm->pid_fmt;
 }
 
-static int cs_etm__insert_trace_id_node(struct cs_etm_queue *etmq,
-					u8 trace_chan_id, u64 *cpu_metadata)
+/*
+ * Add a traceid_list entry that maps a trace ID to a CPU metadata for this etmq
+ * and update the trace ID in the cpu_metadata.
+ */
+static int cs_etm__map_trace_id(struct cs_etm_queue *etmq, u8 trace_chan_id,
+				u64 *cpu_metadata)
 {
 	/* Get an RB node for this CPU */
 	struct int_node *inode = intlist__findnew(etmq->traceid_list, trace_chan_id);

@@ -289,8 +293,7 @@ static int cs_etm__insert_trace_id_node(struct cs_etm_queue *etmq,
 
 	/* Not one we've seen before, associate the traceID with the metadata pointer */
 	inode->priv = cpu_metadata;
-
-	return 0;
+	return cs_etm__metadata_set_trace_id(trace_chan_id, cpu_metadata);
 }
 
 static struct cs_etm_queue *cs_etm__get_queue(struct cs_etm_auxtrace *etm, int cpu)

@@ -314,12 +317,11 @@ static int cs_etm__map_trace_id_v0(struct cs_etm_auxtrace *etm, u8 trace_chan_id
 		return -EINVAL;
 
 	/*
-	 * If the queue is unformatted then only save one mapping in the
-	 * queue associated with that CPU so only one decoder is made.
+	 * Ignore HW_IDs for unformatted, a decoder will be made
+	 * unconditionally later.
 	 */
 	if (etmq->format == UNFORMATTED)
-		return cs_etm__insert_trace_id_node(etmq, trace_chan_id,
-						    cpu_metadata);
+		return 0;
 
 	/*
 	 * Otherwise, version 0 trace IDs are global so save them into every

@@ -332,8 +334,7 @@ static int cs_etm__map_trace_id_v0(struct cs_etm_auxtrace *etm, u8 trace_chan_id
 		if (!etmq || etmq->format == UNFORMATTED)
 			continue;
 
-		ret = cs_etm__insert_trace_id_node(etmq, trace_chan_id,
-						   cpu_metadata);
+		ret = cs_etm__map_trace_id(etmq, trace_chan_id, cpu_metadata);
 		if (ret)
 			return ret;
 	}

@@ -344,7 +345,6 @@ static int cs_etm__map_trace_id_v0(struct cs_etm_auxtrace *etm, u8 trace_chan_id
 static int cs_etm__process_trace_id_v0(struct cs_etm_auxtrace *etm, int cpu,
 				       u64 hw_id)
 {
-	int err;
 	u64 *cpu_data;
 	u8 trace_chan_id = FIELD_GET(CS_AUX_HW_ID_TRACE_ID_MASK, hw_id);

@@ -352,22 +352,13 @@ static int cs_etm__process_trace_id_v0(struct cs_etm_auxtrace *etm, int cpu,
 	if (cpu_data == NULL)
 		return -EINVAL;
 
-	err = cs_etm__map_trace_id_v0(etm, trace_chan_id, cpu_data);
-	if (err)
-		return err;
-
-	/*
-	 * if we are picking up the association from the packet, need to plug
-	 * the correct trace ID into the metadata for setting up decoders later.
-	 */
-	return cs_etm__metadata_set_trace_id(trace_chan_id, cpu_data);
+	return cs_etm__map_trace_id_v0(etm, trace_chan_id, cpu_data);
 }
 
 static int cs_etm__process_trace_id_v0_1(struct cs_etm_auxtrace *etm, int cpu,
 					 u64 hw_id)
 {
 	struct cs_etm_queue *etmq = cs_etm__get_queue(etm, cpu);
-	int ret;
 	u64 *cpu_data;
 	u32 sink_id = FIELD_GET(CS_AUX_HW_ID_SINK_ID_MASK, hw_id);
 	u8 trace_id = FIELD_GET(CS_AUX_HW_ID_TRACE_ID_MASK, hw_id);

@@ -375,6 +366,13 @@ static int cs_etm__process_trace_id_v0_1(struct cs_etm_auxtrace *etm, int cpu,
 	if (!etmq)
 		return -EINVAL;
 
+	/*
+	 * Ignore HW_IDs for unformatted, a decoder will be made
+	 * unconditionally later.
+	 */
+	if (etmq->format == UNFORMATTED)
+		return 0;
+
 	/*
 	 * Check sink id hasn't changed in per-cpu mode. In per-thread mode,
 	 * let it pass for now until an actual overlapping trace ID is hit. In

@@ -419,13 +417,41 @@ static int cs_etm__process_trace_id_v0_1(struct cs_etm_auxtrace *etm, int cpu,
 	if (!cpu_data)
 		return -EINVAL;
 
-	ret = cs_etm__insert_trace_id_node(etmq, trace_id, cpu_data);
-	if (ret)
-		return ret;
+	return cs_etm__map_trace_id(etmq, trace_id, cpu_data);
+}
 
-	ret = cs_etm__metadata_set_trace_id(trace_id, cpu_data);
-	if (ret)
-		return ret;
+static int cs_etm__map_unformatted_trace_ids(struct cs_etm_auxtrace *etm)
+{
+	struct auxtrace_queues *queues = &etm->queues;
+
+	for (unsigned int i = 0; i < queues->nr_queues; i++) {
+		struct auxtrace_queue *queue = &queues->queue_array[i];
+		struct cs_etm_queue *etmq = queue->priv;
+		u64 *cpu_data;
+		int ret;
+
+		if (list_empty(&queue->head) || !etmq || etmq->format != UNFORMATTED)
+			continue;
+
+		if (!intlist__empty(etmq->traceid_list)) {
+			pr_err("CS_ETM: Unformatted queue should't have trace ID\n");
+			return -EINVAL;
+		}
+
+		if (etm->per_thread_decoding)
+			cpu_data = etm->metadata[0];
+		else
+			cpu_data = get_cpu_data(etm, queue->cpu);
+
+		if (!cpu_data) {
+			pr_err("CS_ETM: Unformatted queue missing CPU metadata\n");
+			return -EINVAL;
+		}
+
+		ret = cs_etm__map_trace_id(etmq, 1, cpu_data);
+		if (ret)
+			return ret;
+	}
 
 	return 0;
 }

@@ -3757,6 +3783,11 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
 			goto err_free_queues;
 	}
 
+	/* Create IDs for TRBE sinks so that a decoder is made for each one. */
+	err = cs_etm__map_unformatted_trace_ids(etm);
+	if (err)
+		goto err_free_queues;
+
 	err = cs_etm__create_decoders(etm);
 	if (err)
 		goto err_free_queues;

Signature · 1 line

2.43.0

[email protected]

AI review

2026-10-02 11:23 UTC #7 lore raw

Re: [PATCH v3 3/4] perf cs-etm: Synthesize missing HW_ID mappings for raw trace

Sashiko has reviewed this patch and found no issues. It looks great!

Quoted 23 lines

From: Leo Yan <[email protected]>
The CoreSight driver emits PERF_RECORD_AUX_OUTPUT_HW_ID via perf_report_aux_output_id(). If the perf ring buffer is full, perf_report_aux_output_id() can fail to emit the record and return silently.
When at least one HW_ID record is present in the file, perf skips the legacy metadata-only trace ID mapping path. Therefore any raw AUX queue whose HW_ID record was dropped can be left with an empty traceid_list, and cs_etm__create_queue_decoders() will not create a decoder for that queue.
Fix it by ignoring all HW_IDs for unformatted queues and always create one decoder per queue unconditionally. Trace ID is unused so we can hard code it to 1 (0 is a special value used in 'pending_timestamp_chan_id' to indicate not pending). We also need put the same trace ID in the CPU metadata because the relationship between CPU metadata and traceid_list is bidirectional. Moving cs_etm__metadata_set_trace_id() into the mapping function solves this and simplifies existing code. cs_etm__insert_trace_id_node() now does two things, so rename it to a more generic cs_etm__map_trace_id().

Quoted 2 lines

Signed-off-by Leo Yan <[email protected]>
Signed-off-by James Clark <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/20261002-james-cs-hw_id-output-failure-v3-0-1d8b6ef339c8@linaro.org?part=3

Leo Yan

reviewer Reviewed-by on v3 3/4

2026-10-02 17:14 UTC #8 lore raw

Re: [PATCH v3 3/4] perf cs-etm: Synthesize missing HW_ID mappings for raw trace

In reply to James Clark

On Fri, Oct 02, 2026 at 12:15:13PM +0100, James Clark wrote:

[...]

Quoted 4 lines

static struct cs_etm_queue *cs_etm__get_queue(struct cs_etm_auxtrace *etm, int cpu)
tools/perf/util/cs-etm.c +0 -0
@@ -314,12 +317,11 @@ static int cs_etm__map_trace_id_v0(struct cs_etm_auxtrace *etm, u8 trace_chan_id
 		return -EINVAL;
 

tools/perf/util/cs-etm.c static int cs_etm__map_trace_id_v0(struct cs_etm_auxtrace *etm, u8 trace_chan_id · line 324

Quoted 10 lines

tools/perf/util/cs-etm.c +3 -4

 	/*
-	 * If the queue is unformatted then only save one mapping in the
-	 * queue associated with that CPU so only one decoder is made.
+	 * Ignore HW_IDs for unformatted, a decoder will be made
+	 * unconditionally later.
 	 */
 	if (etmq->format == UNFORMATTED)
-		return cs_etm__insert_trace_id_node(etmq, trace_chan_id,
-						    cpu_metadata);
+		return 0;

Nitpick: It seems to me that we can move the
etmq->format == UNFORMATTED check into cs_etm__process_aux_output_hw_id().

This avoids duplicating the check in both cs_etm__map_trace_id_v0() and cs_etm__process_trace_id_v0_1(), and makes it more apparent that the unformatted case does not use HW_ID at all.

It's up to you whether to keep it as is or update it. Either way:

Reviewed-by Leo Yan [email protected]

P.S. Although I contributed the initial idea, the change is now quite different from the original patch I shared. Please feel free to drop my name if you prefer.

James Clark

author reviewer

2026-10-05 08:33 UTC #9 lore raw

Re: [PATCH v3 3/4] perf cs-etm: Synthesize missing HW_ID mappings for raw trace

Quoted 22 lines · Leo Yan

On Fri, Oct 02, 2026 at 12:15:13PM +0100, James Clark wrote:
[...]
Quoted 4 lines
static struct cs_etm_queue *cs_etm__get_queue(struct cs_etm_auxtrace *etm, int cpu)
tools/perf/util/cs-etm.c +0 -0
@@ -314,12 +317,11 @@ static int cs_etm__map_trace_id_v0(struct cs_etm_auxtrace *etm, u8 trace_chan_id
  		return -EINVAL;
 
tools/perf/util/cs-etm.c static int cs_etm__map_trace_id_v0(struct cs_etm_auxtrace *etm, u8 trace_chan_id · line 324
Quoted 10 lines
tools/perf/util/cs-etm.c +3 -4
  	/*
-	 * If the queue is unformatted then only save one mapping in the
-	 * queue associated with that CPU so only one decoder is made.
+	 * Ignore HW_IDs for unformatted, a decoder will be made
+	 * unconditionally later.
  	 */
  	if (etmq->format == UNFORMATTED)
-		return cs_etm__insert_trace_id_node(etmq, trace_chan_id,
-						    cpu_metadata);
+		return 0;
Nitpick: It seems to me that we can move the
etmq->format == UNFORMATTED check into cs_etm__process_aux_output_hw_id().

Quoted 6 lines

This avoids duplicating the check in both cs_etm__map_trace_id_v0() and cs_etm__process_trace_id_v0_1(), and makes it more apparent that the unformatted case does not use HW_ID at all.
It's up to you whether to keep it as is or update it. Either way:

I might leave it for now. It's also called by cs_etm__map_trace_ids_metadata() which doesn't have a reference to the queue, and neither does cs_etm__process_aux_output_hw_id(). Probably not worth another refactor at this point.

Quoted 5 lines

Reviewed-by Leo Yan [email protected]
P.S. Although I contributed the initial idea, the change is now quite different from the original patch I shared. Please feel free to drop my name if you prefer.

Will do if I send another one, but it looks like it can be applied now so it might have to stay.

James Clark

author reviewer

2026-10-02 11:15 UTC #10 lore raw

PATCH v3 4/4 perf: cs-etm: Respect --no-itrace option

In reply to James Clark

If there is a bug in decoding then Perf will exit early and you can't investigate by using the raw dump mode. Make the --no-itrace option stop Coresight decoding after printing the aux header so decode errors don't stop the rest of the samples being printed.

pmu_type and evsel_is_auxtrace need to be set so that the unleader_auxtrace() behavior is the same as without --no-itrace.

Reviewed-by Adrian Hunter <[email protected]>

Signed-off-by James Clark <[email protected]>

---
 tools/perf/util/auxtrace.c |  2 +-
 tools/perf/util/auxtrace.h |  1 +
 tools/perf/util/cs-etm.c   | 66 +++++++++++++++++++++++++---------------------
 3 files changed, 38 insertions(+), 31 deletions(-)

tools/perf/util/auxtrace.c +1 -1

index ed2113190785..2557cbec37b3 100644
@@ -123,7 +123,7 @@ static int evlist__regroup(struct evlist *evlist, struct evsel *leader, struct e
 	return 0;
 }
 
-static bool auxtrace__dont_decode(struct perf_session *session)
+bool auxtrace__dont_decode(struct perf_session *session)
 {
 	return !session->itrace_synth_opts ||
 	       session->itrace_synth_opts->dont_decode;

tools/perf/util/auxtrace.h +1 -0

index 5012bead1b92..c29a8444f08e 100644
@@ -650,6 +650,7 @@ void auxtrace__free(struct perf_session *session);
 bool auxtrace__evsel_is_auxtrace(struct perf_session *session,
 				 struct evsel *evsel);
 u64 auxtrace_synth_id_range_start(struct evsel *evsel);
+bool auxtrace__dont_decode(struct perf_session *session);
 
 #define ITRACE_HELP \
 "				i[period]:    		synthesize instructions events\n" \

tools/perf/util/cs-etm.c +36 ~~-30~~

131 lines not on this page · raw message

Signature · 1 line

2.43.0

[email protected]

AI review

2026-10-02 11:26 UTC #11 lore raw

Re: [PATCH v3 4/4] perf: cs-etm: Respect --no-itrace option

Sashiko has reviewed this patch and found no issues. It looks great!

Quoted 10 lines

If there is a bug in decoding then Perf will exit early and you can't investigate by using the raw dump mode. Make the --no-itrace option stop Coresight decoding after printing the aux header so decode errors don't stop the rest of the samples being printed.
pmu_type and evsel_is_auxtrace need to be set so that the unleader_auxtrace() behavior is the same as without --no-itrace.
Reviewed-by Adrian Hunter <[email protected]>
Signed-off-by James Clark <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/20261002-james-cs-hw_id-output-failure-v3-0-1d8b6ef339c8@linaro.org?part=4

Leo Yan

reviewer Reviewed-by on v3 4/4

2026-10-02 17:22 UTC #12 lore raw

Re: [PATCH v3 4/4] perf: cs-etm: Respect --no-itrace option

In reply to James Clark

Quoted 10 lines · James Clark

If there is a bug in decoding then Perf will exit early and you can't investigate by using the raw dump mode. Make the --no-itrace option stop Coresight decoding after printing the aux header so decode errors don't stop the rest of the samples being printed.
pmu_type and evsel_is_auxtrace need to be set so that the unleader_auxtrace() behavior is the same as without --no-itrace.
Reviewed-by Adrian Hunter <[email protected]>
Signed-off-by James Clark <[email protected]>

Reviewed-by Leo Yan <[email protected]>

James Clark

author reviewer

2026-10-21 08:42 UTC #13 lore raw

In reply to James Clark

Quoted 40 lines · James Clark

The driver might not always be able to write HW_IDs, but we don't need them for unformatted mode anyway, so fix that in commit ("perf cs-etm:
Synthesize missing HW_ID mappings for raw trace").
Include some other minor improvements encountered on the way to appease Sashiko, like properly freeing queues and ("perf: cs-etm: Respect --no-itrace option") makes debugging broken Coresight perf.data files easier.
Signed-off-by James Clark <[email protected]>
---
Changes in v3:
- Remove old b4 prerequisite metadata that's no longer relevant.
- Link to v2: https://patch.msgid.link/20261002-james-cs-hw_id-output-failure-v2-0-7a194aa61173@linaro.org
Changes in v2:
- Drop driver changes and move to https://lore.kernel.org/linux-perf-users/[email protected]/T/#t
- Don't process any HW_IDs for unformatted queues, just create decoders
unconditionally with trace ID 1.
- Use zfree() instead of etmq->decoder = NULL
- Remove dependency on "[PATCH v2 00/14] perf cs-etm: Per-thread mode
fixes and snapshot wrap support", it wasn't functionally dependant,
just touching some of the same lines.
- Link to v1: https://patch.msgid.link/20260821-james-cs-hw_id-output-failure-v1-0-9d532ddabcc3@linaro.org
---
James Clark (3):
perf: cs-etm: Don't add global v0 HW_IDs to unformatted queues perf cs-etm: Free partially created queues
perf: cs-etm: Respect --no-itrace option
Leo Yan (1):
perf cs-etm: Synthesize missing HW_ID mappings for raw trace
  tools/perf/util/auxtrace.c                      |   2 +-
  tools/perf/util/auxtrace.h                      |   1 +
  tools/perf/util/cs-etm-decoder/cs-etm-decoder.c |  12 +-
  tools/perf/util/cs-etm-decoder/cs-etm-decoder.h |   2 +-
  tools/perf/util/cs-etm.c                        | 184 +++++++++++++++---------
  5 files changed, 123 insertions(+), 78 deletions(-)

Quoted 8 lines

---
base-commit: 705da5b15ab89ba97b11eedbe507c2fd83d31cb9
change-id: 20260706-james-cs-hw_id-output-failure-1d06d042ef96
Best regards,
--
James Clark <[email protected]>

Hi Arnaldo,

Is it ok to take this one please, Leo has left his review tags.

Thanks
James

Arnaldo Carvalho de Melo

maintainer Applied

2026-10-21 13:50 UTC #14 lore raw

Quoted 9 lines · James Clark

Quoted 4 lines · James Clark
James Clark (3):
perf: cs-etm: Don't add global v0 HW_IDs to unformatted queues perf cs-etm: Free partially created queues
perf: cs-etm: Respect --no-itrace option
Hi Arnaldo,
Is it ok to take this one please, Leo has left his review tags.

Thanks, applied to perf-tools-next, for v7.4.

- Arnaldo

James Clark

author reviewer

2026-10-22 08:28 UTC #15 lore raw

Quoted 7 lines · Arnaldo Carvalho de Melo

Quoted 5 lines · James Clark
Quoted 4 lines · James Clark
James Clark (3):
perf: cs-etm: Don't add global v0 HW_IDs to unformatted queues perf cs-etm: Free partially created queues
perf: cs-etm: Respect --no-itrace option

Quoted 7 lines

Quoted 3 lines
Hi Arnaldo,
Is it ok to take this one please, Leo has left his review tags.
Thanks, applied to perf-tools-next, for v7.4.
- Arnaldo

Thanks!