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 james.clark@linaro.org --- 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-7a194aa...
Changes in v2: - Drop driver changes and move to https://lore.kernel.org/linux-perf-users/20261001-james-cs-retry-hw_id-v1-0-... - 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-9d532dd...
--- 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, -- James Clark james.clark@linaro.org
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 james.clark@linaro.org --- tools/perf/util/cs-etm.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 2d1ab34f7b6b..fda6b6c02e26 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -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,
On Fri, Oct 02, 2026 at 12:15:11PM +0100, James Clark wrote:
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 james.clark@linaro.org
Reviewed-by: Leo Yan leo.yan@arm.com
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 leo.yan@arm.com Signed-off-by: James Clark james.clark@linaro.org --- 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(-)
diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c index 26940f1f1b0b..b56cf57a68c8 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -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) diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h index 12c782fa6db2..2a8214f17958 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h @@ -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, diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index fda6b6c02e26..78ae8443ac96 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -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);
From: Leo Yan leo.yan@arm.com
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 leo.yan@arm.com Signed-off-by: James Clark james.clark@linaro.org --- tools/perf/util/cs-etm.c | 85 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 27 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 78ae8443ac96..c0152549e3e2 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -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;
On Fri, Oct 02, 2026 at 12:15:13PM +0100, James Clark wrote:
[...]
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.
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 leo.yan@arm.com
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.
On 02/10/2026 18:14, Leo Yan wrote:
On Fri, Oct 02, 2026 at 12:15:13PM +0100, James Clark wrote:
[...]
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.
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:
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.
Reviewed-by: Leo Yan leo.yan@arm.com
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.
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 adrian.hunter@intel.com Signed-off-by: James Clark james.clark@linaro.org --- 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(-)
diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c index ed2113190785..2557cbec37b3 100644 --- a/tools/perf/util/auxtrace.c +++ b/tools/perf/util/auxtrace.c @@ -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; diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h index 5012bead1b92..c29a8444f08e 100644 --- a/tools/perf/util/auxtrace.h +++ b/tools/perf/util/auxtrace.h @@ -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" \ diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index c0152549e3e2..bdcf94813129 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -965,7 +965,7 @@ static int cs_etm__flush_events(struct perf_session *session, struct cs_etm_auxtrace *etm = container_of(session->auxtrace, struct cs_etm_auxtrace, auxtrace); - if (dump_trace) + if (dump_trace || auxtrace__dont_decode(session)) return 0;
if (!tool->ordered_events) @@ -3002,7 +3002,7 @@ static int cs_etm__process_event(struct perf_session *session, struct cs_etm_auxtrace, auxtrace);
- if (dump_trace) + if (dump_trace || auxtrace__dont_decode(session)) return 0;
if (!tool->ordered_events) { @@ -3552,27 +3552,49 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, int err = 0; int aux_hw_id_found; int i; - u64 *ptr = NULL; + u64 *ptr = (u64 *) auxtrace_info->priv; u64 **metadata = NULL;
- /* First the global part */ - ptr = (u64 *) auxtrace_info->priv; - num_cpu = ptr[CS_PMU_TYPE_CPUS] & 0xffffffff; + etm = zalloc(sizeof(*etm)); + if (!etm) + return -ENOMEM; + + session->auxtrace = &etm->auxtrace; + etm->auxtrace.free = cs_etm__free; + etm->auxtrace.evsel_is_auxtrace = cs_etm__evsel_is_auxtrace; + etm->auxtrace.process_event = cs_etm__process_event; + etm->auxtrace.process_auxtrace_event = cs_etm__process_auxtrace_event; + etm->auxtrace.flush_events = cs_etm__flush_events; + etm->auxtrace.free_events = cs_etm__free_events; + etm->pmu_type = (unsigned int) ((ptr[CS_PMU_TYPE_CPUS] >> 32) & 0xffffffff); + + /* + * Don't go further than the minimum required to identify this event as + * auxtrace with cs_etm__evsel_is_auxtrace() so unleader_auxtrace() + * works. + */ + if (auxtrace__dont_decode(session)) + return 0;
/* * Bound num_cpu by the event size: the global header consumes * CS_ETM_HEADER_SIZE bytes, and each CPU needs at least one u64 * metadata entry after that. */ + num_cpu = ptr[CS_PMU_TYPE_CPUS] & 0xffffffff; priv_size = total_size - event_header_size - INFO_HEADER_SIZE - CS_ETM_HEADER_SIZE; if (num_cpu <= 0 || priv_size <= 0 || - num_cpu > priv_size / (int)sizeof(u64)) - return -EINVAL; + num_cpu > priv_size / (int)sizeof(u64)) { + err = -EINVAL; + goto err_free_etm; + }
metadata = zalloc(sizeof(*metadata) * num_cpu); - if (!metadata) - return -ENOMEM; + if (!metadata) { + err = -ENOMEM; + goto err_free_etm; + }
/* Start parsing after the common part of the header */ i = CS_HEADER_VERSION_MAX; @@ -3631,13 +3653,6 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, goto err_free_metadata; }
- etm = zalloc(sizeof(*etm)); - - if (!etm) { - err = -ENOMEM; - goto err_free_metadata; - } - /* * As all the ETMs run at the same exception level, the system should * have the same PID format crossing CPUs. So cache the PID format @@ -3647,7 +3662,7 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
err = auxtrace_queues__init_nr(&etm->queues, max_cpu + 1); if (err) - goto err_free_etm; + goto err_free_metadata;
for (unsigned int j = 0; j < etm->queues.nr_queues; ++j) { err = cs_etm__setup_queue(etm, &etm->queues.queue_array[j], j); @@ -3685,7 +3700,6 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, etm->session = session;
etm->num_cpu = num_cpu; - etm->pmu_type = (unsigned int) ((ptr[CS_PMU_TYPE_CPUS] >> 32) & 0xffffffff); etm->snapshot_mode = (ptr[CS_ETM_SNAPSHOT] != 0); etm->metadata = metadata; etm->auxtrace_type = auxtrace_info->type; @@ -3712,14 +3726,6 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, "you can specify the itrace option 'T' for timestamp decoding\n" "if the Coresight timestamp on the platform is same with the kernel time.\n\n");
- etm->auxtrace.process_event = cs_etm__process_event; - etm->auxtrace.process_auxtrace_event = cs_etm__process_auxtrace_event; - etm->auxtrace.flush_events = cs_etm__flush_events; - etm->auxtrace.free_events = cs_etm__free_events; - etm->auxtrace.free = cs_etm__free; - etm->auxtrace.evsel_is_auxtrace = cs_etm__evsel_is_auxtrace; - session->auxtrace = &etm->auxtrace; - cs_etm__setup_timeless_decoding(etm);
etm->tc.time_shift = tc->time_shift; @@ -3797,13 +3803,13 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
err_free_queues: cs_etm__free_queues(etm); - session->auxtrace = NULL; -err_free_etm: - zfree(&etm); err_free_metadata: /* No need to check @metadata[j], free(NULL) is supported */ for (int j = 0; j < num_cpu; j++) zfree(&metadata[j]); zfree(&metadata); +err_free_etm: + session->auxtrace = NULL; + zfree(&etm); return err; }
On Fri, Oct 02, 2026 at 12:15:14PM +0100, James Clark wrote:
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 adrian.hunter@intel.com Signed-off-by: James Clark james.clark@linaro.org
Reviewed-by: Leo Yan leo.yan@arm.com
On 02/10/2026 12:15, James Clark wrote:
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 james.clark@linaro.org
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-7a194aa...
Changes in v2:
- Drop driver changes and move to https://lore.kernel.org/linux-perf-users/20261001-james-cs-retry-hw_id-v1-0-...
- 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-9d532dd...
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,
James Clark james.clark@linaro.org
Hi Arnaldo,
Is it ok to take this one please, Leo has left his review tags.
Thanks James
On Wed, Oct 21, 2026 at 09:42:24AM +0100, James Clark wrote:
On 02/10/2026 12:15, James Clark wrote:
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
-
Arnaldo Carvalho de Melo -
James Clark -
Leo Yan