The likely fix for this is to update Perf so print a helpful message.

Signed-off-by: James Clark james.clark@arm.com --- tools/perf/util/cs-etm.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index d65d7485886c..32818bd7cd17 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -335,8 +335,11 @@ static int cs_etm__process_aux_output_hw_id(struct perf_session *session, trace_chan_id = FIELD_GET(CS_AUX_HW_ID_TRACE_ID_MASK, hw_id);

/* check that we can handle this version */ - if (version > CS_AUX_HW_ID_CURR_VERSION) + if (version > CS_AUX_HW_ID_CURR_VERSION) { + pr_err("CS ETM Trace: PERF_RECORD_AUX_OUTPUT_HW_ID version %d not supported. Please update Perf.\n", + version); return -EINVAL; + }

/* get access to the etm metadata */ etm = container_of(session->auxtrace, struct cs_etm_auxtrace, auxtrace);

Both of these passes gather information about how to create the decoders. AUX records determine formatted/unformatted, and the HW_IDs determine the traceID/metadata mappings. Therefore it makes sense to cache the information and wait until both passes are over until creating the decoders, rather than creating them at the first HW_ID found. This will allow a simplification of the creation process where cs_etm_queue->traceid_list will exclusively used to create the decoders, rather than the current two methods depending on whether the trace is formatted or not.

Previously the sample CPU from the AUX record was used to initialize the decoder CPU, but actually sample CPU == AUX queue index in per-CPU mode, so saving the sample CPU isn't required. Similarly formatted/unformatted was used upfront to create the decoders, but now it's cached until later.

Signed-off-by: James Clark james.clark@arm.com --- tools/perf/util/cs-etm.c | 167 ++++++++++++++++++++++++--------------- 1 file changed, 102 insertions(+), 65 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 32818bd7cd17..f09004c4ba44 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -103,6 +103,7 @@ struct cs_etm_queue { struct auxtrace_buffer *buffer; unsigned int queue_nr; u8 pending_timestamp_chan_id; + bool formatted; u64 offset; const unsigned char *buf; size_t buf_len, buf_used; @@ -738,8 +739,7 @@ static int cs_etm__init_trace_params(struct cs_etm_trace_params *t_params,

static int cs_etm__init_decoder_params(struct cs_etm_decoder_params *d_params, struct cs_etm_queue *etmq, - enum cs_etm_decoder_operation mode, - bool formatted) + enum cs_etm_decoder_operation mode) { int ret = -EINVAL;

@@ -749,7 +749,7 @@ static int cs_etm__init_decoder_params(struct cs_etm_decoder_params *d_params, d_params->packet_printer = cs_etm__packet_dump; d_params->operation = mode; d_params->data = etmq; - d_params->formatted = formatted; + d_params->formatted = etmq->formatted; d_params->fsyncs = false; d_params->hsyncs = false; d_params->frame_aligned = true; @@ -1041,81 +1041,34 @@ static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u8 trace_chan_id, return ret; }

-static struct cs_etm_queue *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm, - bool formatted, int sample_cpu) +static struct cs_etm_queue *cs_etm__alloc_queue(void) { - struct cs_etm_decoder_params d_params; - struct cs_etm_trace_params *t_params = NULL; - struct cs_etm_queue *etmq; - /* - * Each queue can only contain data from one CPU when unformatted, so only one decoder is - * needed. - */ - int decoders = formatted ? etm->num_cpu : 1; - - etmq = zalloc(sizeof(*etmq)); + struct cs_etm_queue *etmq = zalloc(sizeof(*etmq)); if (!etmq) return NULL;

etmq->traceid_queues_list = intlist__new(NULL); if (!etmq->traceid_queues_list) - goto out_free; - - /* Use metadata to fill in trace parameters for trace decoder */ - t_params = zalloc(sizeof(*t_params) * decoders); + free(etmq);

- if (!t_params) - goto out_free; - - if (cs_etm__init_trace_params(t_params, etm, formatted, sample_cpu, decoders)) - goto out_free; - - /* Set decoder parameters to decode trace packets */ - if (cs_etm__init_decoder_params(&d_params, etmq, - dump_trace ? CS_ETM_OPERATION_PRINT : - CS_ETM_OPERATION_DECODE, - formatted)) - goto out_free; - - etmq->decoder = cs_etm_decoder__new(decoders, &d_params, - t_params); - - if (!etmq->decoder) - goto out_free; - - /* - * Register a function to handle all memory accesses required by - * the trace decoder library. - */ - if (cs_etm_decoder__add_mem_access_cb(etmq->decoder, - 0x0L, ((u64) -1L), - cs_etm__mem_access)) - goto out_free_decoder; - - zfree(&t_params); return etmq; - -out_free_decoder: - cs_etm_decoder__free(etmq->decoder); -out_free: - intlist__delete(etmq->traceid_queues_list); - free(etmq); - - return NULL; }

static int cs_etm__setup_queue(struct cs_etm_auxtrace *etm, struct auxtrace_queue *queue, - unsigned int queue_nr, - bool formatted, - int sample_cpu) + unsigned int queue_nr, bool formatted) { struct cs_etm_queue *etmq = queue->priv;

+ if (etmq && formatted != etmq->formatted) { + pr_err("CS_ETM: mixed formatted and unformatted trace not supported\n"); + return -EINVAL; + } + if (list_empty(&queue->head) || etmq) return 0;

- etmq = cs_etm__alloc_queue(etm, formatted, sample_cpu); + etmq = cs_etm__alloc_queue();

if (!etmq) return -ENOMEM; @@ -1123,7 +1076,9 @@ static int cs_etm__setup_queue(struct cs_etm_auxtrace *etm, queue->priv = etmq; etmq->etm = etm; etmq->queue_nr = queue_nr; + queue->cpu = queue_nr; /* Placeholder, may be reset to -1 in per-thread mode */ etmq->offset = 0; + etmq->formatted = formatted;

return 0; } @@ -2843,7 +2798,7 @@ static int cs_etm__process_auxtrace_event(struct perf_session *session, * formatted in piped mode (true). */ err = cs_etm__setup_queue(etm, &etm->queues.queue_array[idx], - idx, true, -1); + idx, true); if (err) return err;

@@ -3048,8 +3003,8 @@ static int cs_etm__queue_aux_fragment(struct perf_session *session, off_t file_o

idx = auxtrace_event->idx; formatted = !(aux_event->flags & PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW); - return cs_etm__setup_queue(etm, &etm->queues.queue_array[idx], - idx, formatted, sample->cpu); + + return cs_etm__setup_queue(etm, &etm->queues.queue_array[idx], idx, formatted); }

/* Wasn't inside this buffer, but there were no parse errors. 1 == 'not found' */ @@ -3233,6 +3188,84 @@ static int cs_etm__clear_unused_trace_ids_metadata(int num_cpu, u64 **metadata) return 0; }

+/* + * Use the data gathered by the peeks for HW_ID (trace ID mappings) and AUX + * (formatted or not) packets to create the decoders. + */ +static int cs_etm__create_queue_decoders(struct cs_etm_queue *etmq) +{ + struct cs_etm_decoder_params d_params; + + /* + * Each queue can only contain data from one CPU when unformatted, so only one decoder is + * needed. + */ + int decoders = etmq->formatted ? etmq->etm->num_cpu : 1; + + /* Use metadata to fill in trace parameters for trace decoder */ + struct cs_etm_trace_params *t_params = zalloc(sizeof(*t_params) * decoders); + + if (!t_params) + goto out_free; + + if (cs_etm__init_trace_params(t_params, etmq->etm, etmq->formatted, + etmq->queue_nr, decoders)) + goto out_free; + + /* Set decoder parameters to decode trace packets */ + if (cs_etm__init_decoder_params(&d_params, etmq, + dump_trace ? CS_ETM_OPERATION_PRINT : + CS_ETM_OPERATION_DECODE)) + goto out_free; + + etmq->decoder = cs_etm_decoder__new(decoders, &d_params, + t_params); + + if (!etmq->decoder) + goto out_free; + + /* + * Register a function to handle all memory accesses required by + * the trace decoder library. + */ + if (cs_etm_decoder__add_mem_access_cb(etmq->decoder, + 0x0L, ((u64) -1L), + cs_etm__mem_access)) + goto out_free_decoder; + + zfree(&t_params); + return 0; + +out_free_decoder: + cs_etm_decoder__free(etmq->decoder); +out_free: + zfree(&t_params); + return -EINVAL; +} + +static int cs_etm__create_decoders(struct cs_etm_auxtrace *etm) +{ + struct auxtrace_queues *queues = &etm->queues; + + for (unsigned int i = 0; i < queues->nr_queues; i++) { + bool empty = list_empty(&queues->queue_array[i].head); + struct cs_etm_queue *etmq = queues->queue_array[i].priv; + int ret; + + /* + * Don't create decoders for empty queues, mainly because + * etmq->formatted is unknown for empty queues. + */ + if (empty) + continue; + + ret = cs_etm__create_queue_decoders(etmq); + if (ret) + return ret; + } + return 0; +} + int cs_etm__process_auxtrace_info_full(union perf_event *event, struct perf_session *session) { @@ -3396,6 +3429,10 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, if (err) goto err_free_queues;

+ err = cs_etm__queue_aux_records(session); + if (err) + goto err_free_queues; + /* * Map Trace ID values to CPU metadata. * @@ -3418,7 +3455,7 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, * flags if present. */

- /* first scan for AUX_OUTPUT_HW_ID records to map trace ID values to CPU metadata */ + /* Scan for AUX_OUTPUT_HW_ID records to map trace ID values to CPU metadata */ aux_hw_id_found = 0; err = perf_session__peek_events(session, session->header.data_offset, session->header.data_size, @@ -3436,7 +3473,7 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, if (err) goto err_free_queues;

- err = cs_etm__queue_aux_records(session); + err = cs_etm__create_decoders(etm); if (err) goto err_free_queues;

The global list won't work for per-sink trace ID allocations, so put a list in each queue where the IDs will be unique to that queue.

To keep the same behavior as before, for version 0 of the HW_ID packets, copy all the HW_ID mappings into all queues.

This change doesn't effect the decoders, only trace ID lookups on the Perf side. The decoders are still created with global mappings which will be fixed in a later commit.

Signed-off-by: James Clark james.clark@arm.com --- .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 28 +-- tools/perf/util/cs-etm.c | 195 ++++++++++-------- tools/perf/util/cs-etm.h | 2 +- 3 files changed, 126 insertions(+), 99 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 e917985bbbe6..0c9c48cedbf1 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -388,7 +388,8 @@ cs_etm_decoder__reset_timestamp(struct cs_etm_packet_queue *packet_queue) }

static ocsd_datapath_resp_t -cs_etm_decoder__buffer_packet(struct cs_etm_packet_queue *packet_queue, +cs_etm_decoder__buffer_packet(struct cs_etm_queue *etmq, + struct cs_etm_packet_queue *packet_queue, const u8 trace_chan_id, enum cs_etm_sample_type sample_type) { @@ -398,7 +399,7 @@ cs_etm_decoder__buffer_packet(struct cs_etm_packet_queue *packet_queue, if (packet_queue->packet_count >= CS_ETM_PACKET_MAX_BUFFER - 1) return OCSD_RESP_FATAL_SYS_ERR;

- if (cs_etm__get_cpu(trace_chan_id, &cpu) < 0) + if (cs_etm__get_cpu(etmq, trace_chan_id, &cpu) < 0) return OCSD_RESP_FATAL_SYS_ERR;

et = packet_queue->tail; @@ -436,7 +437,7 @@ cs_etm_decoder__buffer_range(struct cs_etm_queue *etmq, int ret = 0; struct cs_etm_packet *packet;

- ret = cs_etm_decoder__buffer_packet(packet_queue, trace_chan_id, + ret = cs_etm_decoder__buffer_packet(etmq, packet_queue, trace_chan_id, CS_ETM_RANGE); if (ret != OCSD_RESP_CONT && ret != OCSD_RESP_WAIT) return ret; @@ -496,7 +497,8 @@ cs_etm_decoder__buffer_range(struct cs_etm_queue *etmq, }

static ocsd_datapath_resp_t -cs_etm_decoder__buffer_discontinuity(struct cs_etm_packet_queue *queue, +cs_etm_decoder__buffer_discontinuity(struct cs_etm_queue *etmq, + struct cs_etm_packet_queue *queue, const uint8_t trace_chan_id) { /* @@ -504,18 +506,19 @@ cs_etm_decoder__buffer_discontinuity(struct cs_etm_packet_queue *queue, * reset time statistics. */ cs_etm_decoder__reset_timestamp(queue); - return cs_etm_decoder__buffer_packet(queue, trace_chan_id, + return cs_etm_decoder__buffer_packet(etmq, queue, trace_chan_id, CS_ETM_DISCONTINUITY); }

static ocsd_datapath_resp_t -cs_etm_decoder__buffer_exception(struct cs_etm_packet_queue *queue, +cs_etm_decoder__buffer_exception(struct cs_etm_queue *etmq, + struct cs_etm_packet_queue *queue, const ocsd_generic_trace_elem *elem, const uint8_t trace_chan_id) { int ret = 0; struct cs_etm_packet *packet;

- ret = cs_etm_decoder__buffer_packet(queue, trace_chan_id, + ret = cs_etm_decoder__buffer_packet(etmq, queue, trace_chan_id, CS_ETM_EXCEPTION); if (ret != OCSD_RESP_CONT && ret != OCSD_RESP_WAIT) return ret; @@ -527,10 +530,11 @@ cs_etm_decoder__buffer_exception(struct cs_etm_packet_queue *queue, }

static ocsd_datapath_resp_t -cs_etm_decoder__buffer_exception_ret(struct cs_etm_packet_queue *queue, +cs_etm_decoder__buffer_exception_ret(struct cs_etm_queue *etmq, + struct cs_etm_packet_queue *queue, const uint8_t trace_chan_id) { - return cs_etm_decoder__buffer_packet(queue, trace_chan_id, + return cs_etm_decoder__buffer_packet(etmq, queue, trace_chan_id, CS_ETM_EXCEPTION_RET); }

@@ -599,7 +603,7 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer( case OCSD_GEN_TRC_ELEM_EO_TRACE: case OCSD_GEN_TRC_ELEM_NO_SYNC: case OCSD_GEN_TRC_ELEM_TRACE_ON: - resp = cs_etm_decoder__buffer_discontinuity(packet_queue, + resp = cs_etm_decoder__buffer_discontinuity(etmq, packet_queue, trace_chan_id); break; case OCSD_GEN_TRC_ELEM_INSTR_RANGE: @@ -607,11 +611,11 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer( trace_chan_id); break; case OCSD_GEN_TRC_ELEM_EXCEPTION: - resp = cs_etm_decoder__buffer_exception(packet_queue, elem, + resp = cs_etm_decoder__buffer_exception(etmq, packet_queue, elem, trace_chan_id); break; case OCSD_GEN_TRC_ELEM_EXCEPTION_RET: - resp = cs_etm_decoder__buffer_exception_ret(packet_queue, + resp = cs_etm_decoder__buffer_exception_ret(etmq, packet_queue, trace_chan_id); break; case OCSD_GEN_TRC_ELEM_TIMESTAMP: diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index e40dfd09d3ed..be858aed26c4 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -111,16 +111,18 @@ struct cs_etm_queue { /* Conversion between traceID and index in traceid_queues array */ struct intlist *traceid_queues_list; struct cs_etm_traceid_queue **traceid_queues; + /* Conversion between traceID and metadata pointers */ + struct intlist *traceid_list; };

-/* RB tree for quick conversion between traceID and metadata pointers */ -static struct intlist *traceid_list; - static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm); static int cs_etm__process_timeless_queues(struct cs_etm_auxtrace *etm, pid_t tid); static int cs_etm__get_data_block(struct cs_etm_queue *etmq); static int cs_etm__decode_data_block(struct cs_etm_queue *etmq); +static int cs_etm__metadata_get_trace_id(u8 *trace_chan_id, u64 *cpu_metadata); +static u64 *get_cpu_data(struct cs_etm_auxtrace *etm, int cpu); +static int cs_etm__metadata_set_trace_id(u8 trace_chan_id, u64 *cpu_metadata);

/* PTMs ETMIDR [11:8] set to b0011 */ #define ETMIDR_PTM_VERSION 0x00000300 @@ -146,12 +148,12 @@ static u32 cs_etm__get_v7_protocol_version(u32 etmidr) return CS_ETM_PROTO_ETMV3; }

-static int cs_etm__get_magic(u8 trace_chan_id, u64 *magic) +static int cs_etm__get_magic(struct cs_etm_queue *etmq, u8 trace_chan_id, u64 *magic) { struct int_node *inode; u64 *metadata;

- inode = intlist__find(traceid_list, trace_chan_id); + inode = intlist__find(etmq->traceid_list, trace_chan_id); if (!inode) return -EINVAL;

@@ -160,12 +162,12 @@ static int cs_etm__get_magic(u8 trace_chan_id, u64 *magic) return 0; }

-int cs_etm__get_cpu(u8 trace_chan_id, int *cpu) +int cs_etm__get_cpu(struct cs_etm_queue *etmq, u8 trace_chan_id, int *cpu) { struct int_node *inode; u64 *metadata;

- inode = intlist__find(traceid_list, trace_chan_id); + inode = intlist__find(etmq->traceid_list, trace_chan_id); if (!inode) return -EINVAL;

@@ -217,30 +219,86 @@ enum cs_etm_pid_fmt cs_etm__get_pid_fmt(struct cs_etm_queue *etmq) return etmq->etm->pid_fmt; }

-static int cs_etm__map_trace_id(u8 trace_chan_id, u64 *cpu_metadata) +static int cs_etm__insert_trace_id_node(struct cs_etm_queue *etmq, + u8 trace_chan_id, u64 *cpu_metadata) { - struct int_node *inode; - /* Get an RB node for this CPU */ - inode = intlist__findnew(traceid_list, trace_chan_id); + struct int_node *inode = intlist__findnew(etmq->traceid_list, trace_chan_id);

/* Something went wrong, no need to continue */ if (!inode) return -ENOMEM;

- /* - * The node for that CPU should not be taken. - * Back out if that's the case. - */ - if (inode->priv) - return -EINVAL; + /* Disallow re-mapping a different traceID to metadata pair. */ + if (inode->priv) { + u64 *curr_cpu_data = inode->priv; + u8 curr_chan_id; + int err; + + if (curr_cpu_data[CS_ETM_CPU] != cpu_metadata[CS_ETM_CPU]) { + pr_err("CS_ETM: map mismatch between HW_ID packet CPU and Trace ID\n"); + return -EINVAL; + } + + /* check that the mapped ID matches */ + err = cs_etm__metadata_get_trace_id(&curr_chan_id, curr_cpu_data); + if (err) + return err;

- /* All good, associate the traceID with the metadata pointer */ + if (curr_chan_id != trace_chan_id) { + pr_err("CS_ETM: mismatch between CPU trace ID and HW_ID packet ID\n"); + return -EINVAL; + } + + /* Skip re-adding the same mappings if everything matched */ + return 0; + } + + /* Not one we've seen before, associate the traceID with the metadata pointer */ inode->priv = cpu_metadata;

return 0; }

+static int cs_etm__map_trace_id_v0(struct cs_etm_auxtrace *etm, u8 trace_chan_id, + u64 *cpu_metadata) +{ + /* Version 0 trace IDs are global so save them into every queue */ + for (unsigned int i = 0; i < etm->queues.nr_queues; ++i) { + int ret; + struct cs_etm_queue *etmq = etm->queues.queue_array[i].priv; + + ret = cs_etm__insert_trace_id_node(etmq, trace_chan_id, + cpu_metadata); + if (ret) + return ret; + } + + return 0; +} + +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); + + cpu_data = get_cpu_data(etm, 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); +} + static int cs_etm__metadata_get_trace_id(u8 *trace_chan_id, u64 *cpu_metadata) { u64 cs_etm_magic = cpu_metadata[CS_ETM_MAGIC]; @@ -324,17 +382,13 @@ static int cs_etm__process_aux_output_hw_id(struct perf_session *session, { struct cs_etm_auxtrace *etm; struct perf_sample sample; - struct int_node *inode; struct evsel *evsel; - u64 *cpu_data; u64 hw_id; int cpu, version, err; - u8 trace_chan_id, curr_chan_id;

/* extract and parse the HW ID */ hw_id = event->aux_output_hw_id.hw_id; version = FIELD_GET(CS_AUX_HW_ID_VERSION_MASK, hw_id); - trace_chan_id = FIELD_GET(CS_AUX_HW_ID_TRACE_ID_MASK, hw_id);

/* check that we can handle this version */ if (version > CS_AUX_HW_ID_CURR_VERSION) { @@ -362,43 +416,7 @@ static int cs_etm__process_aux_output_hw_id(struct perf_session *session, return -EINVAL; }

- /* See if the ID is mapped to a CPU, and it matches the current CPU */ - inode = intlist__find(traceid_list, trace_chan_id); - if (inode) { - cpu_data = inode->priv; - if ((int)cpu_data[CS_ETM_CPU] != cpu) { - pr_err("CS_ETM: map mismatch between HW_ID packet CPU and Trace ID\n"); - return -EINVAL; - } - - /* check that the mapped ID matches */ - err = cs_etm__metadata_get_trace_id(&curr_chan_id, cpu_data); - if (err) - return err; - if (curr_chan_id != trace_chan_id) { - pr_err("CS_ETM: mismatch between CPU trace ID and HW_ID packet ID\n"); - return -EINVAL; - } - - /* mapped and matched - return OK */ - return 0; - } - - cpu_data = get_cpu_data(etm, cpu); - if (cpu_data == NULL) - return err; - - /* not one we've seen before - lets map it */ - err = cs_etm__map_trace_id(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. - */ - err = cs_etm__metadata_set_trace_id(trace_chan_id, cpu_data); - return err; + return cs_etm__process_trace_id_v0(etm, cpu, hw_id); }

void cs_etm__etmq_set_traceid_queue_timestamp(struct cs_etm_queue *etmq, @@ -851,6 +869,7 @@ static void cs_etm__free_traceid_queues(struct cs_etm_queue *etmq)

static void cs_etm__free_queue(void *priv) { + struct int_node *inode, *tmp; struct cs_etm_queue *etmq = priv;

if (!etmq) @@ -858,6 +877,14 @@ static void cs_etm__free_queue(void *priv)

cs_etm_decoder__free(etmq->decoder); cs_etm__free_traceid_queues(etmq); + + /* First remove all traceID/metadata nodes for the RB tree */ + intlist__for_each_entry_safe(inode, tmp, etmq->traceid_list) + intlist__remove(etmq->traceid_list, inode); + + /* Then the RB tree itself */ + intlist__delete(etmq->traceid_list); + free(etmq); }

@@ -880,19 +907,12 @@ static void cs_etm__free_events(struct perf_session *session) static void cs_etm__free(struct perf_session *session) { int i; - struct int_node *inode, *tmp; struct cs_etm_auxtrace *aux = container_of(session->auxtrace, struct cs_etm_auxtrace, auxtrace); cs_etm__free_events(session); session->auxtrace = NULL;

- /* First remove all traceID/metadata nodes for the RB tree */ - intlist__for_each_entry_safe(inode, tmp, traceid_list) - intlist__remove(traceid_list, inode); - /* Then the RB tree itself */ - intlist__delete(traceid_list); - for (i = 0; i < aux->num_cpu; i++) zfree(&aux->metadata[i]);

@@ -1050,9 +1070,24 @@ static struct cs_etm_queue *cs_etm__alloc_queue(void)

etmq->traceid_queues_list = intlist__new(NULL); if (!etmq->traceid_queues_list) - free(etmq); + goto out_free; + + /* + * Create an RB tree for traceID-metadata tuple. Since the conversion + * has to be made for each packet that gets decoded, optimizing access + * in anything other than a sequential array is worth doing. + */ + etmq->traceid_list = intlist__new(NULL); + if (!etmq->traceid_list) + goto out_free;

return etmq; + +out_free: + intlist__delete(etmq->traceid_queues_list); + free(etmq); + + return NULL; }

static int cs_etm__setup_queue(struct cs_etm_auxtrace *etm, @@ -2202,7 +2237,7 @@ static int cs_etm__set_sample_flags(struct cs_etm_queue *etmq, PERF_IP_FLAG_TRACE_END; break; case CS_ETM_EXCEPTION: - ret = cs_etm__get_magic(packet->trace_chan_id, &magic); + ret = cs_etm__get_magic(etmq, packet->trace_chan_id, &magic); if (ret) return ret;

@@ -3119,7 +3154,8 @@ static bool cs_etm__has_virtual_ts(u64 **metadata, int num_cpu) }

/* map trace ids to correct metadata block, from information in metadata */ -static int cs_etm__map_trace_ids_metadata(int num_cpu, u64 **metadata) +static int cs_etm__map_trace_ids_metadata(struct cs_etm_auxtrace *etm, int num_cpu, + u64 **metadata) { u64 cs_etm_magic; u8 trace_chan_id; @@ -3141,7 +3177,7 @@ static int cs_etm__map_trace_ids_metadata(int num_cpu, u64 **metadata) /* unknown magic number */ return -EINVAL; } - err = cs_etm__map_trace_id(trace_chan_id, metadata[i]); + err = cs_etm__map_trace_id_v0(etm, trace_chan_id, metadata[i]); if (err) return err; } @@ -3272,23 +3308,12 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, u64 *ptr = NULL; u64 **metadata = NULL;

- /* - * Create an RB tree for traceID-metadata tuple. Since the conversion - * has to be made for each packet that gets decoded, optimizing access - * in anything other than a sequential array is worth doing. - */ - traceid_list = intlist__new(NULL); - if (!traceid_list) - return -ENOMEM; - /* First the global part */ ptr = (u64 *) auxtrace_info->priv; num_cpu = ptr[CS_PMU_TYPE_CPUS] & 0xffffffff; metadata = zalloc(sizeof(*metadata) * num_cpu); - if (!metadata) { - err = -ENOMEM; - goto err_free_traceid_list; - } + if (!metadata) + return -ENOMEM;

/* Start parsing after the common part of the header */ i = CS_HEADER_VERSION_MAX; @@ -3467,7 +3492,7 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, err = cs_etm__clear_unused_trace_ids_metadata(num_cpu, metadata); /* otherwise, this is a file with metadata values only, map from metadata */ else - err = cs_etm__map_trace_ids_metadata(num_cpu, metadata); + err = cs_etm__map_trace_ids_metadata(etm, num_cpu, metadata);

if (err) goto err_free_queues; @@ -3489,7 +3514,5 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, for (int j = 0; j < num_cpu; j++) zfree(&metadata[j]); zfree(&metadata); -err_free_traceid_list: - intlist__delete(traceid_list); return err; } diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h index 4696267a32f0..f4f69f7cc0f3 100644 --- a/tools/perf/util/cs-etm.h +++ b/tools/perf/util/cs-etm.h @@ -252,7 +252,7 @@ enum cs_etm_pid_fmt {

#ifdef HAVE_CSTRACE_SUPPORT #include <opencsd/ocsd_if_types.h> -int cs_etm__get_cpu(u8 trace_chan_id, int *cpu); +int cs_etm__get_cpu(struct cs_etm_queue *etmq, u8 trace_chan_id, int *cpu); enum cs_etm_pid_fmt cs_etm__get_pid_fmt(struct cs_etm_queue *etmq); int cs_etm__etmq_set_tid_el(struct cs_etm_queue *etmq, pid_t tid, u8 trace_chan_id, ocsd_ex_level el);

v0.1 HW_ID packets have a new field that describes which sink each CPU writes to. Use the sink ID to link trace ID maps to each other so that mappings are shared wherever the sink is shared.

Also update the error message to show that overlapping IDs aren't an error in per-thread mode, just not supported. In the future we can use the CPU ID from the AUX records, or watch for changing sink IDs on HW_ID packets to use the correct decoders.

Signed-off-by: James Clark james.clark@arm.com --- tools/include/linux/coresight-pmu.h | 17 ++++- tools/perf/util/cs-etm.c | 110 +++++++++++++++++++++++++--- 2 files changed, 113 insertions(+), 14 deletions(-)

diff --git a/tools/include/linux/coresight-pmu.h b/tools/include/linux/coresight-pmu.h index 51ac441a37c3..89b0ac0014b0 100644 --- a/tools/include/linux/coresight-pmu.h +++ b/tools/include/linux/coresight-pmu.h @@ -49,12 +49,21 @@ * Interpretation of the PERF_RECORD_AUX_OUTPUT_HW_ID payload. * Used to associate a CPU with the CoreSight Trace ID. * [07:00] - Trace ID - uses 8 bits to make value easy to read in file. - * [59:08] - Unused (SBZ) - * [63:60] - Version + * [39:08] - Sink ID - as reported in /sys/bus/event_source/devices/cs_etm/sinks/ + * Added in minor version 1. + * [55:40] - Unused (SBZ) + * [59:56] - Minor Version - previously existing fields are compatible with + * all minor versions. + * [63:60] - Major Version - previously existing fields mean different things + * in new major versions. */ #define CS_AUX_HW_ID_TRACE_ID_MASK GENMASK_ULL(7, 0) -#define CS_AUX_HW_ID_VERSION_MASK GENMASK_ULL(63, 60) +#define CS_AUX_HW_ID_SINK_ID_MASK GENMASK_ULL(39, 8)

-#define CS_AUX_HW_ID_CURR_VERSION 0 +#define CS_AUX_HW_ID_MINOR_VERSION_MASK GENMASK_ULL(59, 56) +#define CS_AUX_HW_ID_MAJOR_VERSION_MASK GENMASK_ULL(63, 60) + +#define CS_AUX_HW_ID_MAJOR_VERSION 0 +#define CS_AUX_HW_ID_MINOR_VERSION 1

#endif diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 73fc0ab2fb09..b0f9a3933b03 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -113,6 +113,12 @@ struct cs_etm_queue { struct cs_etm_traceid_queue **traceid_queues; /* Conversion between traceID and metadata pointers */ struct intlist *traceid_list; + /* + * Same as traceid_list, but traceid_list may be a reference to another + * queue's which has a matching sink ID. + */ + struct intlist *own_traceid_list; + u32 sink_id; };

static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm); @@ -137,6 +143,7 @@ static int cs_etm__metadata_set_trace_id(u8 trace_chan_id, u64 *cpu_metadata); (queue_nr << 16 | trace_chan_id) #define TO_QUEUE_NR(cs_queue_nr) (cs_queue_nr >> 16) #define TO_TRACE_CHAN_ID(cs_queue_nr) (cs_queue_nr & 0x0000ffff) +#define SINK_UNSET ((u32) -1)

static u32 cs_etm__get_v7_protocol_version(u32 etmidr) { @@ -236,7 +243,16 @@ static int cs_etm__insert_trace_id_node(struct cs_etm_queue *etmq, int err;

if (curr_cpu_data[CS_ETM_CPU] != cpu_metadata[CS_ETM_CPU]) { - pr_err("CS_ETM: map mismatch between HW_ID packet CPU and Trace ID\n"); + /* + * With > CORESIGHT_TRACE_IDS_MAX ETMs, overlapping IDs + * are expected (but not supported) in per-thread mode, + * rather than signifying an error. + */ + if (etmq->etm->per_thread_decoding) + pr_err("CS_ETM: overlapping Trace IDs aren't currently supported in per-thread mode\n"); + else + pr_err("CS_ETM: map mismatch between HW_ID packet CPU and Trace ID\n"); + return -EINVAL; }

@@ -303,6 +319,74 @@ static int cs_etm__process_trace_id_v0(struct cs_etm_auxtrace *etm, int cpu, return cs_etm__metadata_set_trace_id(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; + 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); + + /* In per thread mode there is only one queue and it's index 0 */ + if (etm->per_thread_decoding) + etmq = etm->queues.queue_array[0].priv; + else + etmq = etm->queues.queue_array[cpu].priv; + + /* Ignore HW_IDs on unformatted queues */ + if (etmq->formatted_set && !etmq->formatted) + 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 + * most cases IDs won't overlap even if the sink changes. + */ + if (!etmq->etm->per_thread_decoding && etmq->sink_id != SINK_UNSET && + etmq->sink_id != sink_id) { + pr_err("CS_ETM: mismatch between sink IDs\n"); + return -EINVAL; + } + + etmq->sink_id = sink_id; + + /* Find which other queues use this sink and link their ID maps */ + for (unsigned int i = 0; i < etm->queues.nr_queues; ++i) { + struct cs_etm_queue *other_etmq = etm->queues.queue_array[i].priv; + + /* Different sinks, skip */ + if (other_etmq->sink_id != etmq->sink_id) + continue; + + /* Already linked, skip */ + if (other_etmq->traceid_list == etmq->traceid_list) + continue; + + /* At the point of first linking, this one should be empty */ + if (!intlist__empty(etmq->traceid_list)) { + pr_err("CS_ETM: Can't link populated trace ID lists\n"); + return -EINVAL; + } + + etmq->own_traceid_list = NULL; + intlist__delete(etmq->traceid_list); + etmq->traceid_list = other_etmq->traceid_list; + break; + } + + cpu_data = get_cpu_data(etm, cpu); + ret = cs_etm__insert_trace_id_node(etmq, trace_id, cpu_data); + if (ret) + return ret; + + ret = cs_etm__metadata_set_trace_id(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]; @@ -392,10 +476,10 @@ static int cs_etm__process_aux_output_hw_id(struct perf_session *session,

/* extract and parse the HW ID */ hw_id = event->aux_output_hw_id.hw_id; - version = FIELD_GET(CS_AUX_HW_ID_VERSION_MASK, hw_id); + version = FIELD_GET(CS_AUX_HW_ID_MAJOR_VERSION_MASK, hw_id);

/* check that we can handle this version */ - if (version > CS_AUX_HW_ID_CURR_VERSION) { + if (version > CS_AUX_HW_ID_MAJOR_VERSION) { pr_err("CS ETM Trace: PERF_RECORD_AUX_OUTPUT_HW_ID version %d not supported. Please update Perf.\n", version); return -EINVAL; @@ -420,7 +504,10 @@ static int cs_etm__process_aux_output_hw_id(struct perf_session *session, return -EINVAL; }

- return cs_etm__process_trace_id_v0(etm, cpu, hw_id); + if (FIELD_GET(CS_AUX_HW_ID_MINOR_VERSION_MASK, hw_id) == 0) + return cs_etm__process_trace_id_v0(etm, cpu, hw_id); + else + return cs_etm__process_trace_id_v0_1(etm, cpu, hw_id); }

void cs_etm__etmq_set_traceid_queue_timestamp(struct cs_etm_queue *etmq, @@ -860,12 +947,14 @@ static void cs_etm__free_queue(void *priv) cs_etm_decoder__free(etmq->decoder); cs_etm__free_traceid_queues(etmq);

- /* First remove all traceID/metadata nodes for the RB tree */ - intlist__for_each_entry_safe(inode, tmp, etmq->traceid_list) - intlist__remove(etmq->traceid_list, inode); + if (etmq->own_traceid_list) { + /* First remove all traceID/metadata nodes for the RB tree */ + intlist__for_each_entry_safe(inode, tmp, etmq->own_traceid_list) + intlist__remove(etmq->own_traceid_list, inode);

- /* Then the RB tree itself */ - intlist__delete(etmq->traceid_list); + /* Then the RB tree itself */ + intlist__delete(etmq->own_traceid_list); + }

free(etmq); } @@ -1059,7 +1148,7 @@ static struct cs_etm_queue *cs_etm__alloc_queue(void) * has to be made for each packet that gets decoded, optimizing access * in anything other than a sequential array is worth doing. */ - etmq->traceid_list = intlist__new(NULL); + etmq->traceid_list = etmq->own_traceid_list = intlist__new(NULL); if (!etmq->traceid_list) goto out_free;

@@ -1091,6 +1180,7 @@ static int cs_etm__setup_queue(struct cs_etm_auxtrace *etm, etmq->queue_nr = queue_nr; queue->cpu = queue_nr; /* Placeholder, may be reset to -1 in per-thread mode */ etmq->offset = 0; + etmq->sink_id = SINK_UNSET;

return 0; }

"Process being monitored" and "pid of the process to monitor" imply that this would be the same PID if there were two sessions targeting the same process. But this is actually the PID of the process that did the Perf event open call, rather than the target of the session. So update the comments to make this clearer.

Reviewed-by: Anshuman Khandual anshuman.khandual@arm.com Reviewed-by: Mike Leach mike.leach@linaro.org Signed-off-by: James Clark james.clark@arm.com --- drivers/hwtracing/coresight/coresight-tmc-etr.c | 5 +++-- drivers/hwtracing/coresight/coresight-tmc.h | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index e75428fa1592..8962fc27d04f 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -36,7 +36,8 @@ struct etr_buf_hw { * etr_perf_buffer - Perf buffer used for ETR * @drvdata - The ETR drvdaga this buffer has been allocated for. * @etr_buf - Actual buffer used by the ETR - * @pid - The PID this etr_perf_buffer belongs to. + * @pid - The PID of the session owner that etr_perf_buffer + * belongs to. * @snaphost - Perf session mode * @nr_pages - Number of pages in the ring buffer. * @pages - Array of Pages in the ring buffer. @@ -1662,7 +1663,7 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data) goto unlock_out; }

- /* Get a handle on the pid of the process to monitor */ + /* Get a handle on the pid of the session owner */ pid = etr_perf->pid;

/* Do not proceed if this device is associated with another session */ diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index c77763b49de0..2671926be62a 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -171,8 +171,9 @@ struct etr_buf { * @csdev: component vitals needed by the framework. * @miscdev: specifics to handle "/dev/xyz.tmc" entry. * @spinlock: only one at a time pls. - * @pid: Process ID of the process being monitored by the session - * that is using this component. + * @pid: Process ID of the process that owns the session that is using + * this component. For example this would be the pid of the Perf + * process. * @buf: Snapshot of the trace data for ETF/ETB. * @etr_buf: details of buffer used in TMC-ETR * @len: size of the available trace for ETF/ETB.

The trace ID API is currently hard coded to always use the global map. Add public versions that allow the map to be passed in so that Perf mode can use per-sink maps. Keep the non-map versions so that sysfs mode can continue to use the default global map.

System ID functions are unchanged because they will always use the default map.

Signed-off-by: James Clark james.clark@arm.com --- .../hwtracing/coresight/coresight-trace-id.c | 36 ++++++++++++++----- .../hwtracing/coresight/coresight-trace-id.h | 20 +++++++++-- 2 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trace-id.c b/drivers/hwtracing/coresight/coresight-trace-id.c index 19005b5b4dc4..5561989a03fa 100644 --- a/drivers/hwtracing/coresight/coresight-trace-id.c +++ b/drivers/hwtracing/coresight/coresight-trace-id.c @@ -12,7 +12,7 @@

#include "coresight-trace-id.h"

-/* Default trace ID map. Used on systems that don't require per sink mappings */ +/* Default trace ID map. Used in sysfs mode and for system sources */ static struct coresight_trace_id_map id_map_default;

/* maintain a record of the mapping of IDs and pending releases per cpu */ @@ -47,7 +47,7 @@ static void coresight_trace_id_dump_table(struct coresight_trace_id_map *id_map, #endif

/* unlocked read of current trace ID value for given CPU */ -static int _coresight_trace_id_read_cpu_id(int cpu) +static int _coresight_trace_id_read_cpu_id(int cpu, struct coresight_trace_id_map *id_map) { return atomic_read(&per_cpu(cpu_id, cpu)); } @@ -152,7 +152,7 @@ static void coresight_trace_id_release_all_pending(void) DUMP_ID_MAP(id_map); }

-static int coresight_trace_id_map_get_cpu_id(int cpu, struct coresight_trace_id_map *id_map) +static int _coresight_trace_id_get_cpu_id(int cpu, struct coresight_trace_id_map *id_map) { unsigned long flags; int id; @@ -160,7 +160,7 @@ static int coresight_trace_id_map_get_cpu_id(int cpu, struct coresight_trace_id_ spin_lock_irqsave(&id_map_lock, flags);

/* check for existing allocation for this CPU */ - id = _coresight_trace_id_read_cpu_id(cpu); + id = _coresight_trace_id_read_cpu_id(cpu, id_map); if (id) goto get_cpu_id_clr_pend;

@@ -196,13 +196,13 @@ static int coresight_trace_id_map_get_cpu_id(int cpu, struct coresight_trace_id_ return id; }

-static void coresight_trace_id_map_put_cpu_id(int cpu, struct coresight_trace_id_map *id_map) +static void _coresight_trace_id_put_cpu_id(int cpu, struct coresight_trace_id_map *id_map) { unsigned long flags; int id;

/* check for existing allocation for this CPU */ - id = _coresight_trace_id_read_cpu_id(cpu); + id = _coresight_trace_id_read_cpu_id(cpu, id_map); if (!id) return;

@@ -254,22 +254,40 @@ static void coresight_trace_id_map_put_system_id(struct coresight_trace_id_map *

int coresight_trace_id_get_cpu_id(int cpu) { - return coresight_trace_id_map_get_cpu_id(cpu, &id_map_default); + return _coresight_trace_id_get_cpu_id(cpu, &id_map_default); } EXPORT_SYMBOL_GPL(coresight_trace_id_get_cpu_id);

+int coresight_trace_id_get_cpu_id_map(int cpu, struct coresight_trace_id_map *id_map) +{ + return _coresight_trace_id_get_cpu_id(cpu, id_map); +} +EXPORT_SYMBOL_GPL(coresight_trace_id_get_cpu_id_map); + void coresight_trace_id_put_cpu_id(int cpu) { - coresight_trace_id_map_put_cpu_id(cpu, &id_map_default); + _coresight_trace_id_put_cpu_id(cpu, &id_map_default); } EXPORT_SYMBOL_GPL(coresight_trace_id_put_cpu_id);

+void coresight_trace_id_put_cpu_id_map(int cpu, struct coresight_trace_id_map *id_map) +{ + _coresight_trace_id_put_cpu_id(cpu, id_map); +} +EXPORT_SYMBOL_GPL(coresight_trace_id_put_cpu_id_map); + int coresight_trace_id_read_cpu_id(int cpu) { - return _coresight_trace_id_read_cpu_id(cpu); + return _coresight_trace_id_read_cpu_id(cpu, &id_map_default); } EXPORT_SYMBOL_GPL(coresight_trace_id_read_cpu_id);

+int coresight_trace_id_read_cpu_id_map(int cpu, struct coresight_trace_id_map *id_map) +{ + return _coresight_trace_id_read_cpu_id(cpu, id_map); +} +EXPORT_SYMBOL_GPL(coresight_trace_id_read_cpu_id_map); + int coresight_trace_id_get_system_id(void) { return coresight_trace_id_map_get_system_id(&id_map_default); diff --git a/drivers/hwtracing/coresight/coresight-trace-id.h b/drivers/hwtracing/coresight/coresight-trace-id.h index 49438a96fcc6..840babdd0794 100644 --- a/drivers/hwtracing/coresight/coresight-trace-id.h +++ b/drivers/hwtracing/coresight/coresight-trace-id.h @@ -42,8 +42,6 @@ #define IS_VALID_CS_TRACE_ID(id) \ ((id > CORESIGHT_TRACE_ID_RES_0) && (id < CORESIGHT_TRACE_ID_RES_TOP))

-/* Allocate and release IDs for a single default trace ID map */ - /** * Read and optionally allocate a CoreSight trace ID and associate with a CPU. * @@ -59,6 +57,12 @@ */ int coresight_trace_id_get_cpu_id(int cpu);

+/** + * Version of coresight_trace_id_get_cpu_id() that allows the ID map to operate + * on to be provided. + */ +int coresight_trace_id_get_cpu_id_map(int cpu, struct coresight_trace_id_map *id_map); + /** * Release an allocated trace ID associated with the CPU. * @@ -72,6 +76,12 @@ int coresight_trace_id_get_cpu_id(int cpu); */ void coresight_trace_id_put_cpu_id(int cpu);

+/** + * Version of coresight_trace_id_put_cpu_id() that allows the ID map to operate + * on to be provided. + */ +void coresight_trace_id_put_cpu_id_map(int cpu, struct coresight_trace_id_map *id_map); + /** * Read the current allocated CoreSight Trace ID value for the CPU. * @@ -92,6 +102,12 @@ void coresight_trace_id_put_cpu_id(int cpu); */ int coresight_trace_id_read_cpu_id(int cpu);

+/** + * Version of coresight_trace_id_read_cpu_id() that allows the ID map to operate + * on to be provided. + */ +int coresight_trace_id_read_cpu_id_map(int cpu, struct coresight_trace_id_map *id_map); + /** * Allocate a CoreSight trace ID for a system component. *

This will allow sessions with more than CORESIGHT_TRACE_IDS_MAX ETMs as long as there are fewer than that many ETMs connected to each sink.

Each sink owns its own trace ID map, and any Perf session connecting to that sink will allocate from it, even if the sink is currently in use by other users. This is similar to the existing behavior where the dynamic trace IDs are constant as long as there is any concurrent Perf session active. It's not completely optimal because slightly more IDs will be used than necessary, but the optimal solution involves tracking the PIDs of each session and allocating ID maps based on the session owner. This is difficult to do with the combination of per-thread and per-cpu modes and some scheduling issues. The complexity of this isn't likely to worth it because even with multiple users they'd just see a difference in the ordering of ID allocations rather than hitting any limits (unless the hardware does have too many ETMs connected to one sink).

Signed-off-by: James Clark james.clark@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 10 ++++++++++ drivers/hwtracing/coresight/coresight-dummy.c | 3 ++- drivers/hwtracing/coresight/coresight-etm-perf.c | 15 ++++++++++----- .../hwtracing/coresight/coresight-etm3x-core.c | 9 +++++---- .../hwtracing/coresight/coresight-etm4x-core.c | 9 +++++---- drivers/hwtracing/coresight/coresight-stm.c | 3 ++- drivers/hwtracing/coresight/coresight-sysfs.c | 3 ++- drivers/hwtracing/coresight/coresight-tpdm.c | 3 ++- include/linux/coresight.h | 3 ++- 9 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 9fc6f6b863e0..d5aaeafe5c7d 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -902,6 +902,7 @@ static void coresight_device_release(struct device *dev) struct coresight_device *csdev = to_coresight_device(dev);

fwnode_handle_put(csdev->dev.fwnode); + free_percpu(csdev->perf_sink_id_map.cpu_map); kfree(csdev); }

@@ -1159,6 +1160,14 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) csdev->dev.fwnode = fwnode_handle_get(dev_fwnode(desc->dev)); dev_set_name(&csdev->dev, "%s", desc->name);

+ if (csdev->type == CORESIGHT_DEV_TYPE_SINK || + csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) { + csdev->perf_sink_id_map.cpu_map = alloc_percpu(atomic_t); + if (!csdev->perf_sink_id_map.cpu_map) { + ret = -ENOMEM; + goto err_out; + } + } /* * Make sure the device registration and the connection fixup * are synchronised, so that we don't see uninitialised devices @@ -1216,6 +1225,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) err_out: /* Cleanup the connection information */ coresight_release_platform_data(NULL, desc->dev, desc->pdata); + kfree(csdev); return ERR_PTR(ret); } EXPORT_SYMBOL_GPL(coresight_register); diff --git a/drivers/hwtracing/coresight/coresight-dummy.c b/drivers/hwtracing/coresight/coresight-dummy.c index ac70c0b491be..1f1b9ad160f6 100644 --- a/drivers/hwtracing/coresight/coresight-dummy.c +++ b/drivers/hwtracing/coresight/coresight-dummy.c @@ -21,7 +21,8 @@ DEFINE_CORESIGHT_DEVLIST(source_devs, "dummy_source"); DEFINE_CORESIGHT_DEVLIST(sink_devs, "dummy_sink");

static int dummy_source_enable(struct coresight_device *csdev, - struct perf_event *event, enum cs_mode mode) + struct perf_event *event, enum cs_mode mode, + __maybe_unused struct coresight_trace_id_map *id_map) { dev_dbg(csdev->dev.parent, "Dummy source enabled\n");

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index c0c60e6a1703..7fb55dafb639 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -229,10 +229,13 @@ static void free_event_data(struct work_struct *work) struct list_head **ppath;

ppath = etm_event_cpu_path_ptr(event_data, cpu); - if (!(IS_ERR_OR_NULL(*ppath))) + if (!(IS_ERR_OR_NULL(*ppath))) { + struct coresight_device *sink = coresight_get_sink(*ppath); + + coresight_trace_id_put_cpu_id_map(cpu, &sink->perf_sink_id_map); coresight_release_path(*ppath); + } *ppath = NULL; - coresight_trace_id_put_cpu_id(cpu); }

/* mark perf event as done for trace id allocator */ @@ -401,7 +404,7 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, }

/* ensure we can allocate a trace ID for this CPU */ - trace_id = coresight_trace_id_get_cpu_id(cpu); + trace_id = coresight_trace_id_get_cpu_id_map(cpu, &sink->perf_sink_id_map); if (!IS_VALID_CS_TRACE_ID(trace_id)) { cpumask_clear_cpu(cpu, mask); coresight_release_path(path); @@ -495,7 +498,8 @@ static void etm_event_start(struct perf_event *event, int flags) goto fail_end_stop;

/* Finally enable the tracer */ - if (source_ops(csdev)->enable(csdev, event, CS_MODE_PERF)) + if (source_ops(csdev)->enable(csdev, event, CS_MODE_PERF, + &sink->perf_sink_id_map)) goto fail_disable_path;

/* @@ -507,7 +511,8 @@ static void etm_event_start(struct perf_event *event, int flags) hw_id = FIELD_PREP(CS_AUX_HW_ID_VERSION_MASK, CS_AUX_HW_ID_CURR_VERSION); hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK, - coresight_trace_id_read_cpu_id(cpu)); + coresight_trace_id_read_cpu_id_map(cpu, + &sink->perf_sink_id_map)); perf_report_aux_output_id(event, hw_id); }

diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c index 9d5c1391ffb1..65cf7456426a 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -481,7 +481,8 @@ void etm_release_trace_id(struct etm_drvdata *drvdata) }

static int etm_enable_perf(struct coresight_device *csdev, - struct perf_event *event) + struct perf_event *event, + struct coresight_trace_id_map *id_map) { struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); int trace_id; @@ -500,7 +501,7 @@ static int etm_enable_perf(struct coresight_device *csdev, * with perf locks - we know the ID cannot change until perf shuts down * the session */ - trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu); + trace_id = coresight_trace_id_read_cpu_id_map(drvdata->cpu, id_map); if (!IS_VALID_CS_TRACE_ID(trace_id)) { dev_err(&drvdata->csdev->dev, "Failed to set trace ID for %s on CPU%d\n", dev_name(&drvdata->csdev->dev), drvdata->cpu); @@ -553,7 +554,7 @@ static int etm_enable_sysfs(struct coresight_device *csdev) }

static int etm_enable(struct coresight_device *csdev, struct perf_event *event, - enum cs_mode mode) + enum cs_mode mode, struct coresight_trace_id_map *id_map) { int ret; struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); @@ -568,7 +569,7 @@ static int etm_enable(struct coresight_device *csdev, struct perf_event *event, ret = etm_enable_sysfs(csdev); break; case CS_MODE_PERF: - ret = etm_enable_perf(csdev, event); + ret = etm_enable_perf(csdev, event, id_map); break; default: ret = -EINVAL; diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index a0bdfabddbc6..fcd0f9ba562d 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -752,7 +752,8 @@ static int etm4_parse_event_config(struct coresight_device *csdev, }

static int etm4_enable_perf(struct coresight_device *csdev, - struct perf_event *event) + struct perf_event *event, + struct coresight_trace_id_map *id_map) { int ret = 0, trace_id; struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); @@ -775,7 +776,7 @@ static int etm4_enable_perf(struct coresight_device *csdev, * with perf locks - we know the ID cannot change until perf shuts down * the session */ - trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu); + trace_id = coresight_trace_id_read_cpu_id_map(drvdata->cpu, id_map); if (!IS_VALID_CS_TRACE_ID(trace_id)) { dev_err(&drvdata->csdev->dev, "Failed to set trace ID for %s on CPU%d\n", dev_name(&drvdata->csdev->dev), drvdata->cpu); @@ -837,7 +838,7 @@ static int etm4_enable_sysfs(struct coresight_device *csdev) }

static int etm4_enable(struct coresight_device *csdev, struct perf_event *event, - enum cs_mode mode) + enum cs_mode mode, struct coresight_trace_id_map *id_map) { int ret;

@@ -851,7 +852,7 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event, ret = etm4_enable_sysfs(csdev); break; case CS_MODE_PERF: - ret = etm4_enable_perf(csdev, event); + ret = etm4_enable_perf(csdev, event, id_map); break; default: ret = -EINVAL; diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c index e1c62820dfda..a80ad1de4c23 100644 --- a/drivers/hwtracing/coresight/coresight-stm.c +++ b/drivers/hwtracing/coresight/coresight-stm.c @@ -194,7 +194,8 @@ static void stm_enable_hw(struct stm_drvdata *drvdata) }

static int stm_enable(struct coresight_device *csdev, struct perf_event *event, - enum cs_mode mode) + enum cs_mode mode, + __maybe_unused struct coresight_trace_id_map *trace_id) { struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);

diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c index 1e67cc7758d7..a01c9e54e2ed 100644 --- a/drivers/hwtracing/coresight/coresight-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-sysfs.c @@ -9,6 +9,7 @@ #include <linux/kernel.h>

#include "coresight-priv.h" +#include "coresight-trace-id.h"

/* * Use IDR to map the hash of the source's device name @@ -63,7 +64,7 @@ static int coresight_enable_source_sysfs(struct coresight_device *csdev, */ lockdep_assert_held(&coresight_mutex); if (coresight_get_mode(csdev) != CS_MODE_SYSFS) { - ret = source_ops(csdev)->enable(csdev, data, mode); + ret = source_ops(csdev)->enable(csdev, data, mode, NULL); if (ret) return ret; } diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c index a9708ab0d488..0376ad326a2f 100644 --- a/drivers/hwtracing/coresight/coresight-tpdm.c +++ b/drivers/hwtracing/coresight/coresight-tpdm.c @@ -439,7 +439,8 @@ static void __tpdm_enable(struct tpdm_drvdata *drvdata) }

static int tpdm_enable(struct coresight_device *csdev, struct perf_event *event, - enum cs_mode mode) + enum cs_mode mode, + __maybe_unused struct coresight_trace_id_map *id_map) { struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);

diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 7d62b88bfb5c..9c3067e2e38b 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -290,6 +290,7 @@ struct coresight_device { bool sysfs_sink_activated; struct dev_ext_attribute *ea; struct coresight_device *def_sink; + struct coresight_trace_id_map perf_sink_id_map; /* sysfs links between components */ int nr_links; bool has_conns_grp; @@ -384,7 +385,7 @@ struct coresight_ops_link { struct coresight_ops_source { int (*cpu_id)(struct coresight_device *csdev); int (*enable)(struct coresight_device *csdev, struct perf_event *event, - enum cs_mode mode); + enum cs_mode mode, struct coresight_trace_id_map *id_map); void (*disable)(struct coresight_device *csdev, struct perf_event *event); };

In per-cpu mode there are multiple aux buffers and each one has a fixed sink, so the hw ID mappings which only need to be emitted once for each buffer, even with the new per-sink trace ID pools.

But in per-thread mode there is only a single buffer which can be written to from any sink with now potentially overlapping trace IDs, so hw ID mappings need to be re-emitted every time the sink changes.

This will require a change in Perf to track this so it knows which decode tree to use for each segment of the buffer. In theory it's also possible to look at the CPU ID on the AUX records, but this is more consistent with the existing system, and allows for correct decode using either mechanism.

Signed-off-by: James Clark james.clark@arm.com --- drivers/hwtracing/coresight/coresight-etm-perf.c | 14 ++++++++++++++ drivers/hwtracing/coresight/coresight-etm-perf.h | 2 ++ 2 files changed, 16 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 17cafa1a7f18..b6f505b50e67 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -499,6 +499,20 @@ static void etm_event_start(struct perf_event *event, int flags) &sink->perf_sink_id_map)) goto fail_disable_path;

+ /* + * In per-cpu mode there are multiple aux buffers and each one has a + * fixed sink, so the hw ID mappings which only need to be emitted once + * for each buffer. + * + * But in per-thread mode there is only a single buffer which can be + * written to from any sink with potentially overlapping trace IDs, so + * hw ID mappings need to be re-emitted every time the sink changes. + */ + if (event->cpu == -1 && event_data->last_sink_hwid != sink) { + cpumask_clear(&event_data->aux_hwid_done); + event_data->last_sink_hwid = sink; + } + /* * output cpu / trace ID in perf record, once for the lifetime * of the event. diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h index 744531158d6b..bd4553b2a1ec 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.h +++ b/drivers/hwtracing/coresight/coresight-etm-perf.h @@ -52,6 +52,7 @@ struct etm_filters { * @snk_config: The sink configuration. * @cfg_hash: The hash id of any coresight config selected. * @path: An array of path, each slot for one CPU. + * @last_sink_hwid: Last sink that a hwid was emitted for. */ struct etm_event_data { struct work_struct work; @@ -60,6 +61,7 @@ struct etm_event_data { void *snk_config; u32 cfg_hash; struct list_head * __percpu *path; + struct coresight_device *last_sink_hwid; };

int etm_perf_symlink(struct coresight_device *csdev, bool link);