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").
At the same time, give the driver another chance to send them in commit ("coresight: perf: Retry failed HW_ID writes"). The other commits are semi-related improvements and fixes.
("perf: cs-etm: Respect --no-itrace option") makes debugging broken Coresight perf.data files easier.
Applies on top of "[PATCH v2 00/14] perf cs-etm: Per-thread mode fixes and snapshot wrap support"
Signed-off-by: James Clark james.clark@linaro.org --- James Clark (5): 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 perf/core: Return errors from perf_report_aux_output_id() coresight: perf: Retry failed HW_ID writes
Leo Yan (1): perf cs-etm: Synthesize missing HW_ID mappings for raw trace
drivers/hwtracing/coresight/coresight-etm-perf.c | 52 +++++---- include/linux/perf_event.h | 2 +- kernel/events/core.c | 6 +- tools/perf/util/auxtrace.c | 2 +- tools/perf/util/auxtrace.h | 1 + tools/perf/util/cs-etm.c | 143 ++++++++++++++++------- 6 files changed, 139 insertions(+), 67 deletions(-) --- base-commit: 78148c85297024ffe7a709acb7cc4fc907271176 change-id: 20260706-james-cs-hw_id-output-failure-1d06d042ef96 prerequisite-change-id: 20260605-james-cs-unformatted-per-thread-fix-50e723aa7f0e:v2 prerequisite-patch-id: 1aa32269a3a7dc76840dd8a24cb5a8715507e898 prerequisite-patch-id: ef471f468351462f67efa58a09a3461306ac5a0a prerequisite-patch-id: c78946cf4ec1c7722570865403a3562625bdaa33 prerequisite-patch-id: 0b953eee0252db3c7ba3ef1e3397048a94482ca2 prerequisite-patch-id: 27da72c2f01bcc68205bbcc14d5019369c02cb83 prerequisite-patch-id: 365c6d5f71c754e4c690b5ebed3453565bb5b809 prerequisite-patch-id: 64e9419a41082a2db3daf255baf0fb40efbae5dd prerequisite-patch-id: 1dc2e6ef8e76b369736b4c88560302861b2e4faf prerequisite-patch-id: b7de38ec4d90f5b45d56390a4e5919a1d6439951 prerequisite-patch-id: 3548d5b161cfb8c11a166ca2b3228ca4821f1ca0 prerequisite-patch-id: 97e66600218a2b32e5b21d0abcb6321590f98de9 prerequisite-patch-id: 292282b20bd8ad7d096ee7ffbc55dcfb75455f53 prerequisite-patch-id: c80259b47850ba65c985c85a29c8cf4948463d59
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 6e04602ec204..8dfafd389066 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -306,27 +306,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, Aug 21, 2026 at 10:50:29AM +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.
I understand a session can support multiple sinks, but I don't expect to mix up formatted and unformatted trace (see sinks_compatible()).
For the mentioned fallback, I am also struggling to establish a case. Even AUX_OUTPUT_HW_ID records are absent randomly, I don't think this impacts the queue's format initalization, no?
On 04/09/2026 17:43, Leo Yan wrote:
On Fri, Aug 21, 2026 at 10:50:29AM +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.
I understand a session can support multiple sinks, but I don't expect to mix up formatted and unformatted trace (see sinks_compatible()).
But sinks_compatible() is only called multiple times for per-thread sessions. For per-CPU, userspace opens separate events per-CPU. If the user doesn't specify a sink then the default one is chosen which can be a different type for each CPU/path.
For the mentioned fallback, I am also struggling to establish a case. Even AUX_OUTPUT_HW_ID records are absent randomly, I don't think this impacts the queue's format initalization, no?
The fallback uses the metadata only, without sink information, which is equivalent to v0 records. So it's just another way to trigger the same problem.
If you're using global v0 mappings, then we assume that all trace IDs are possible in all queues, which results in multiple decoders being created and triggers an error in OpenCSD once it sees that being combined with unformatted.
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.
Signed-off-by: James Clark james.clark@linaro.org --- tools/perf/util/cs-etm.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 8dfafd389066..44e20067da1f 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -1014,13 +1014,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); @@ -1030,13 +1027,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++) @@ -3520,6 +3527,7 @@ static int cs_etm__create_queue_decoders(struct cs_etm_queue *etmq)
out_free_decoder: cs_etm_decoder__free(etmq->decoder); + etmq->decoder = NULL; out_free: zfree(&t_params); return -EINVAL; @@ -3801,7 +3809,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);
On Fri, Aug 21, 2026 at 10:50:30AM +0100, James Clark wrote:
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.
[...]
@@ -3520,6 +3527,7 @@ static int cs_etm__create_queue_decoders(struct cs_etm_queue *etmq) out_free_decoder: cs_etm_decoder__free(etmq->decoder);
- etmq->decoder = NULL;
cs_etm__free_queue() also calls cs_etm_decoder__free() but does not clear etmq->decoder. Should we do the same there, or drop this unrelated change?
Otherwise, for the queues free fix:
Reviewed-by: Leo Yan leo.yan@arm.com
On 04/09/2026 18:12, Leo Yan wrote:
On Fri, Aug 21, 2026 at 10:50:30AM +0100, James Clark wrote:
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.
[...]
@@ -3520,6 +3527,7 @@ static int cs_etm__create_queue_decoders(struct cs_etm_queue *etmq) out_free_decoder: cs_etm_decoder__free(etmq->decoder);
- etmq->decoder = NULL;
cs_etm__free_queue() also calls cs_etm_decoder__free() but does not clear etmq->decoder. Should we do the same there, or drop this unrelated change?
Yep we should do the same, I'll make cs_etm_decoder__free() call zfree() and drop the explicit NULL.
Otherwise, for the queues free fix:
Reviewed-by: Leo Yan leo.yan@arm.com
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.
Backfill missing mappings for non-empty UNFORMATTED queues whose trace ID map is still empty after the HW_ID record scan. Real HW_ID mappings remain preferred and are not overwritten in case someone is trying to link what the kernel outputs to the raw trace dump.
Signed-off-by: Leo Yan leo.yan@arm.com Signed-off-by: James Clark james.clark@linaro.org --- tools/perf/util/cs-etm.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 44e20067da1f..dd15e91b7b38 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -427,6 +427,39 @@ static int cs_etm__process_trace_id_v0_1(struct cs_etm_auxtrace *etm, int cpu, return 0; }
+static int cs_etm__synth_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; + u8 trace_id; + int ret; + + if (list_empty(&queue->head) || !etmq || etmq->format != UNFORMATTED) + continue; + + if (!intlist__empty(etmq->traceid_list)) + continue; + + cpu_data = get_cpu_data(etm, queue->cpu); + if (!cpu_data) + return -EINVAL; + + ret = cs_etm__metadata_get_trace_id(&trace_id, cpu_data); + if (ret) + return ret; + + ret = cs_etm__insert_trace_id_node(etmq, trace_id, cpu_data); + if (ret) + return ret; + } + + return 0; +} + static int cs_etm__metadata_get_trace_id(u8 *trace_chan_id, u64 *cpu_metadata) { u64 cs_etm_magic = cpu_metadata[CS_ETM_MAGIC]; @@ -3801,6 +3834,16 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, goto err_free_queues; }
+ /* + * Add fake IDs for any unformatted queues missing them. The IDs + * themselves are only cosmetic in unformatted mode (raw dump output), + * but a decoder won't be created if one was dropped and Perf can be + * resilient to this. + */ + err = cs_etm__synth_unformatted_trace_ids(etm); + if (err) + goto err_free_queues; + err = cs_etm__create_decoders(etm); if (err) goto err_free_queues;
On Fri, Aug 21, 2026 at 10:50:31AM +0100, James Clark wrote:
[...]
+static int cs_etm__synth_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;u8 trace_id;int ret;if (list_empty(&queue->head) || !etmq || etmq->format != UNFORMATTED)continue;if (!intlist__empty(etmq->traceid_list))continue;cpu_data = get_cpu_data(etm, queue->cpu);if (!cpu_data)return -EINVAL;ret = cs_etm__metadata_get_trace_id(&trace_id, cpu_data);if (ret)return ret;
Since this is an unformatted queue, can we simply ignore the trace ID? Instead of fetching it from the metadata, we could insert 0 to create the trace ID list.
This would also align with future refactoring to remove trace ID handling for unformatted queues.
On 04/09/2026 18:29, Leo Yan wrote:
On Fri, Aug 21, 2026 at 10:50:31AM +0100, James Clark wrote:
[...]
+static int cs_etm__synth_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;u8 trace_id;int ret;if (list_empty(&queue->head) || !etmq || etmq->format != UNFORMATTED)continue;if (!intlist__empty(etmq->traceid_list))continue;cpu_data = get_cpu_data(etm, queue->cpu);if (!cpu_data)return -EINVAL;ret = cs_etm__metadata_get_trace_id(&trace_id, cpu_data);if (ret)return ret;Since this is an unformatted queue, can we simply ignore the trace ID? Instead of fetching it from the metadata, we could insert 0 to create the trace ID list.
It's printed in the raw output though and it's useful for eyeballing which CPU any bit of trace ran on because the AUXTRACE header might be off the page, and it makes output consistent with ETM:
. ... CoreSight ETE Trace data: size 0x10000 bytes Idx:0; ID:22; I_ASYNC : Alignment Synchronisation.
I don't think it costs anything or is complex to do this, but if you really want to set them all to 0 we could do it.
This would also align with future refactoring to remove trace ID handling for unformatted queues.
At that point if they're all going to be 0 then we can ignore all processing of HW_ID events for UNFORMATTED. Instead of saying cs_etm__synth_unformatted_trace_ids() is for 'fixing up missing IDs' it just creates them outright.
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.
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 e3c770b46e94..caa53056d349 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 971b817d3396..47fb97136836 100644 --- a/tools/perf/util/auxtrace.h +++ b/tools/perf/util/auxtrace.h @@ -651,6 +651,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 dd15e91b7b38..22884c9fdff8 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -969,7 +969,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) @@ -3040,7 +3040,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) { @@ -3603,27 +3603,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; @@ -3682,13 +3704,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 @@ -3698,7 +3713,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); @@ -3736,7 +3751,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; @@ -3763,14 +3777,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; @@ -3853,13 +3859,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, Aug 21, 2026 at 10:50:32AM +0100, James Clark wrote:
[...]
@@ -3603,27 +3603,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;
TBH, I don't look in details, currently auxtrace__dont_decode() only exits in the common util/auxtrace.c, why other AUX trace can work well the option but cs-etm.c needs particular initialization?
Seems to me, we even don't need to do any initialization in cs-etm.c if the no-itrace flag is set.
On 04/09/2026 18:45, Leo Yan wrote:
On Fri, Aug 21, 2026 at 10:50:32AM +0100, James Clark wrote:
[...]
@@ -3603,27 +3603,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;TBH, I don't look in details, currently auxtrace__dont_decode() only exits in the common util/auxtrace.c, why other AUX trace can work well the option but cs-etm.c needs particular initialization?
Seems to me, we even don't need to do any initialization in cs-etm.c if the no-itrace flag is set.
Because we have 2 peek() passes over the file which can hit processing errors at this point. I assume other users have a fairly trivial process_auxtrace_info() callback which can never fail.
It's useful to know if this failed or not so a retry can be attempted later. Return the error code or success.
Signed-off-by: James Clark james.clark@linaro.org --- include/linux/perf_event.h | 2 +- kernel/events/core.c | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 48d851fbd8ea..01d318a4de50 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -1896,7 +1896,7 @@ static inline struct fasync_struct **perf_event_fasync(struct perf_event *event) }
extern void perf_event_addr_filters_sync(struct perf_event *event); -extern void perf_report_aux_output_id(struct perf_event *event, u64 hw_id); +extern int perf_report_aux_output_id(struct perf_event *event, u64 hw_id);
extern int perf_output_begin(struct perf_output_handle *handle, struct perf_sample_data *data, diff --git a/kernel/events/core.c b/kernel/events/core.c index ba5bd6a78fe7..bcdc9e7d2637 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -10565,7 +10565,7 @@ static void perf_log_itrace_start(struct perf_event *event) perf_output_end(&handle); }
-void perf_report_aux_output_id(struct perf_event *event, u64 hw_id) +int perf_report_aux_output_id(struct perf_event *event, u64 hw_id) { struct perf_output_handle handle; struct perf_sample_data sample; @@ -10587,12 +10587,14 @@ void perf_report_aux_output_id(struct perf_event *event, u64 hw_id) ret = perf_output_begin(&handle, &sample, event, rec.header.size);
if (ret) - return; + return ret;
perf_output_put(&handle, rec); perf_event__output_id_sample(event, &handle, &sample);
perf_output_end(&handle); + + return 0; } EXPORT_SYMBOL_GPL(perf_report_aux_output_id);
Don't set the aux_hwid_done bit when writes fail so that it can be retried later if the ring buffer was full.
Add a second attempt on stop in case this session only has one call to start. Otherwise there is no chance to retry and it isn't fixed by not setting aux_hwid_done.
Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etm-perf.c | 52 +++++++++++++++--------- 1 file changed, 32 insertions(+), 20 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 09b21a711a87..0ffc5cd9c88f 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -536,6 +536,31 @@ static int etm_event_resume(struct coresight_path *path) return ret; }
+static void etm_output_hw_id(struct perf_event *event, + struct etm_event_data *event_data, + struct coresight_path *path, + struct coresight_device *sink) +{ + int cpu = smp_processor_id(); + u64 hw_id; + + /* + * output cpu / trace ID in perf record, once for the lifetime + * of the event. + */ + if (!cpumask_test_cpu(cpu, &event_data->aux_hwid_done)) { + hw_id = FIELD_PREP(CS_AUX_HW_ID_MAJOR_VERSION_MASK, + CS_AUX_HW_ID_MAJOR_VERSION); + hw_id |= FIELD_PREP(CS_AUX_HW_ID_MINOR_VERSION_MASK, + CS_AUX_HW_ID_MINOR_VERSION); + hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK, path->trace_id); + hw_id |= FIELD_PREP(CS_AUX_HW_ID_SINK_ID_MASK, coresight_get_sink_id(sink)); + + if (!perf_report_aux_output_id(event, hw_id)) + cpumask_set_cpu(cpu, &event_data->aux_hwid_done); + } +} + static void etm_event_start(struct perf_event *event, int flags) { int cpu = smp_processor_id(); @@ -544,7 +569,6 @@ static void etm_event_start(struct perf_event *event, int flags) struct perf_output_handle *handle = &ctxt->handle; struct coresight_device *source, *sink; struct coresight_path *path; - u64 hw_id;
if (flags & PERF_EF_RESUME) { path = etm_event_get_ctxt_path(ctxt); @@ -596,22 +620,7 @@ static void etm_event_start(struct perf_event *event, int flags) if (coresight_enable_source(source, event, CS_MODE_PERF, path)) goto fail_disable_path;
- /* - * output cpu / trace ID in perf record, once for the lifetime - * of the event. - */ - if (!cpumask_test_cpu(cpu, &event_data->aux_hwid_done)) { - cpumask_set_cpu(cpu, &event_data->aux_hwid_done); - - hw_id = FIELD_PREP(CS_AUX_HW_ID_MAJOR_VERSION_MASK, - CS_AUX_HW_ID_MAJOR_VERSION); - hw_id |= FIELD_PREP(CS_AUX_HW_ID_MINOR_VERSION_MASK, - CS_AUX_HW_ID_MINOR_VERSION); - hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK, path->trace_id); - hw_id |= FIELD_PREP(CS_AUX_HW_ID_SINK_ID_MASK, coresight_get_sink_id(sink)); - - perf_report_aux_output_id(event, hw_id); - } + etm_output_hw_id(event, event_data, path, sink);
out: /* Tell the perf core the event is alive */ @@ -630,7 +639,7 @@ static void etm_event_start(struct perf_event *event, int flags) */ if (READ_ONCE(handle->event)) { perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); - perf_aux_output_end(handle, 0); + perf_aux_output_end(path->handle, 0); } fail: event->hw.state = PERF_HES_STOPPED; @@ -679,6 +688,7 @@ static void etm_event_pause(struct coresight_path *path, if (!size) return;
+ etm_output_hw_id(event, event_data, path, sink); perf_aux_output_end(handle, size); perf_aux_output_begin(handle, event); } else { @@ -769,10 +779,12 @@ static void etm_event_stop(struct perf_event *event, int mode) * But we should never get a non-zero size with * an invalid handle. */ - if (READ_ONCE(handle->event)) + if (READ_ONCE(handle->event)) { + etm_output_hw_id(event, event_data, path, sink); perf_aux_output_end(handle, size); - else + } else { WARN_ON(size); + } }
/* Disabling the path make its elements available to other sessions */
On 21/08/2026 10:50, James Clark wrote:
Don't set the aux_hwid_done bit when writes fail so that it can be retried later if the ring buffer was full.
Add a second attempt on stop in case this session only has one call to start. Otherwise there is no chance to retry and it isn't fixed by not setting aux_hwid_done.
Signed-off-by: James Clark james.clark@linaro.org
drivers/hwtracing/coresight/coresight-etm-perf.c | 52 +++++++++++++++--------- 1 file changed, 32 insertions(+), 20 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 09b21a711a87..0ffc5cd9c88f 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -536,6 +536,31 @@ static int etm_event_resume(struct coresight_path *path) return ret; } +static void etm_output_hw_id(struct perf_event *event,
struct etm_event_data *event_data,struct coresight_path *path,struct coresight_device *sink)+{
- int cpu = smp_processor_id();
- u64 hw_id;
- /*
* output cpu / trace ID in perf record, once for the lifetime* of the event.*/- if (!cpumask_test_cpu(cpu, &event_data->aux_hwid_done)) {
hw_id = FIELD_PREP(CS_AUX_HW_ID_MAJOR_VERSION_MASK,CS_AUX_HW_ID_MAJOR_VERSION);hw_id |= FIELD_PREP(CS_AUX_HW_ID_MINOR_VERSION_MASK,CS_AUX_HW_ID_MINOR_VERSION);hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK, path->trace_id);hw_id |= FIELD_PREP(CS_AUX_HW_ID_SINK_ID_MASK, coresight_get_sink_id(sink));if (!perf_report_aux_output_id(event, hw_id))cpumask_set_cpu(cpu, &event_data->aux_hwid_done);- }
+}
- static void etm_event_start(struct perf_event *event, int flags) { int cpu = smp_processor_id();
@@ -544,7 +569,6 @@ static void etm_event_start(struct perf_event *event, int flags) struct perf_output_handle *handle = &ctxt->handle; struct coresight_device *source, *sink; struct coresight_path *path;
- u64 hw_id;
if (flags & PERF_EF_RESUME) { path = etm_event_get_ctxt_path(ctxt); @@ -596,22 +620,7 @@ static void etm_event_start(struct perf_event *event, int flags) if (coresight_enable_source(source, event, CS_MODE_PERF, path)) goto fail_disable_path;
- /*
* output cpu / trace ID in perf record, once for the lifetime* of the event.*/- if (!cpumask_test_cpu(cpu, &event_data->aux_hwid_done)) {
cpumask_set_cpu(cpu, &event_data->aux_hwid_done);hw_id = FIELD_PREP(CS_AUX_HW_ID_MAJOR_VERSION_MASK,CS_AUX_HW_ID_MAJOR_VERSION);hw_id |= FIELD_PREP(CS_AUX_HW_ID_MINOR_VERSION_MASK,CS_AUX_HW_ID_MINOR_VERSION);hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK, path->trace_id);hw_id |= FIELD_PREP(CS_AUX_HW_ID_SINK_ID_MASK, coresight_get_sink_id(sink));perf_report_aux_output_id(event, hw_id);- }
- etm_output_hw_id(event, event_data, path, sink);
out: /* Tell the perf core the event is alive */ @@ -630,7 +639,7 @@ static void etm_event_start(struct perf_event *event, int flags) */ if (READ_ONCE(handle->event)) { perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
perf_aux_output_end(handle, 0);
perf_aux_output_end(path->handle, 0);
Accidentally left this in from hacking. It doesn't do anything so I'll remove it.
-
James Clark -
Leo Yan