lkml.org 
[lkml]   [2022]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v4 11/13] perf: cs-etm: Handle PERF_RECORD_AUX_OUTPUT_HW_ID packet
From


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:

\
 
 \ /
  Last update: 2022-09-05 13:37    [W:0.181 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site