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,

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

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;

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

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