Re: [PATCH v4 11/13] perf: cs-etm: Handle PERF_RECORD_AUX_OUTPUT_HW_ID packet

On 23/08/2022 10:10, Mike Leach wrote:

When using dynamically assigned CoreSight trace IDs the drivers can output the ID / CPU association as a PERF_RECORD_AUX_OUTPUT_HW_ID packet.

Update cs-etm decoder to handle this packet by setting the CPU/Trace ID mapping.

Signed-off-by: Mike Leach mike.leach@linaro.org

[...]

• /* before aux records are queued, need to map metadata to trace IDs */
• err = cs_etm__map_trace_ids_metadata(num_cpu, metadata);

• /*

• * Map Trace ID values to CPU metadata.
  

• *
  

• * Trace metadata will always contain Trace ID values from the legacy algorithm. If the
  

• * files has been recorded by a "new" perf updated to handle AUX_HW_ID then the metadata
  

• * ID value will also have the CORESIGHT_TRACE_ID_UNUSED_FLAG set.
  

• *
  

• * The updated kernel drivers that use AUX_HW_ID to sent Trace IDs will attempt to use
  

• * the same IDs as the old algorithm as far as is possible, unless there are clashes
  

• * in which case a different value will be used. This means an older perf may still
  

• * be able to record and read files generate on a newer system.
  

• *
  

• * For a perf able to interpret AUX_HW_ID packets we first check for the presence of
  

• * those packets. If they are there then the values will be mapped and plugged into
  

• * the metadata. We then set any remaining metadata values with the used flag to a
  

• * value CORESIGHT_TRACE_ID_UNUSED_VAL - which indicates no decoder is required.
  

• *
  

• * If no AUX_HW_ID packets are present - which means a file recorded on an old kernel
  

• * then we map Trace ID values to CPU directly from the metadata - clearing any unused
  

• * flags if present.
  

• */
  

• /* first 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,
  

• 			cs_etm__process_aux_hw_id_cb, &aux_hw_id_found);
  

• if (err)

• goto err_delete_thread;
  

• /* if HW ID found then clear any unused metadata ID values */
• if (aux_hw_id_found)

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

• if (err) goto err_delete_thread;

@@ -3124,13 +3342,14 @@ int cs_etm__process_auxtrace_info(union perf_event *event, etm->data_queued = etm->queues.populated; /*

• * Print warning in pipe mode, see cs_etm__process_auxtrace_event() and
  

• * Print error in pipe mode, see cs_etm__process_auxtrace_event() and
  

  • cs_etm__queue_aux_fragment() for details relating to limitations.
  */

• if (!etm->data_queued)

• pr_warning("CS ETM warning: Coresight decode and TRBE support requires random file access.\n"
  

• 	   "Continuing with best effort decoding in piped mode.\n\n");
  

• if (!etm->data_queued) {

• pr_err("CS ETM: Coresight decode and TRBE support need random file access.\n");
  

• err = -EINVAL;
  

• goto err_delete_thread;
  

• }

This error message is never hit because the peek that was added is followed by:

if (err) goto err_delete_thread;

Peek will return -1 in pipe mode so then you get this output instead:

./perf record -e cs_etm//u -o - -- ls > stdio.data cat stdio.data | ./perf report -i -

0x1464 [0x168]: failed to process type: 70 Error: failed to process sample

It would be simpler to add this new check to the very beginning of cs_etm__process_auxtrace_info() and print the message/quit there instead:

if (perf_data__is_pipe(session->data)) return -1;

Then etm->data_queued can also be removed because it's always true.

Apart from that issue:

Reviewed-by: James Clark james.clark@arm.com

Thanks James

return 0; err_delete_thread: