[PATCH 0/6] perf: cs-etm: HW_ID improvements

[PATCH 0/6] perf: cs-etm: HW_ID improvements

[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").

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>

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

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

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,

-- 
2.43.0

[PATCH 2/6] perf cs-etm: Free partially created queues

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

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

-- 
2.43.0

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

[James Clark]

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;

-- 
2.43.0

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

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

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

-- 
2.43.0

[PATCH 5/6] perf/core: Return errors from perf_report_aux_output_id()

[James Clark]

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

-- 
2.43.0

[PATCH 6/6] coresight: perf: Retry failed HW_ID writes

[James Clark]

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 */

-- 
2.43.0

Re: [PATCH 6/6] coresight: perf: Retry failed HW_ID writes

[James Clark]

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.

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

[sashiko-bot]

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

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-james-cs-hw_id-output-failure-v1-0-9d532ddabcc3@linaro.org?part=1

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

[sashiko-bot]

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

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-james-cs-hw_id-output-failure-v1-0-9d532ddabcc3@linaro.org?part=2

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

[sashiko-bot]

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

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-james-cs-hw_id-output-failure-v1-0-9d532ddabcc3@linaro.org?part=3

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

[sashiko-bot]

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

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-james-cs-hw_id-output-failure-v1-0-9d532ddabcc3@linaro.org?part=4

Re: [PATCH 5/6] perf/core: Return errors from perf_report_aux_output_id()

[sashiko-bot]

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

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-james-cs-hw_id-output-failure-v1-0-9d532ddabcc3@linaro.org?part=5

Re: [PATCH 6/6] coresight: perf: Retry failed HW_ID writes

[sashiko-bot]

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

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-james-cs-hw_id-output-failure-v1-0-9d532ddabcc3@linaro.org?part=6