2022-12-12 16:36:20

by James Clark

[permalink] [raw]
Subject: [PATCH 0/5] perf: cs-etm: Print auxtrace info even if OpenCSD isn't linked

The auxtrace info header can be useful for debugging, and at the
moment it's possible to record a file without OpenCSD linked but
not view the header even though it should be possible to do.

This patchset tidies up some of the related functions and
improves some of the error messages before making the above
possible in the last commit.

Testing done:

* Compiled on x86 and Arm both with and without CORESIGHT=1
* Ran the Coresight tests

Applies to perf/core (0c3852adae8)

James Clark (5):
perf: cs-etm: Print unknown header version as an error
perf: cs-etm: Remove unused stub methods
perf: cs-etm: Tidy up auxtrace info header printing
perf: cs-etm: Cleanup cs_etm__process_auxtrace_info()
perf: cs-etm: Print auxtrace info even if OpenCSD isn't linked

tools/perf/util/Build | 1 +
tools/perf/util/cs-etm-base.c | 174 ++++++++++++++++++++++++++++
tools/perf/util/cs-etm.c | 208 +++-------------------------------
tools/perf/util/cs-etm.h | 46 ++------
4 files changed, 200 insertions(+), 229 deletions(-)
create mode 100644 tools/perf/util/cs-etm-base.c

--
2.25.1


2022-12-12 16:41:51

by James Clark

[permalink] [raw]
Subject: [PATCH 4/5] perf: cs-etm: Cleanup cs_etm__process_auxtrace_info()

hdr is a copy of 3 values of ptr and doesn't need to be long lived. So
just use ptr instead which means the malloc and the extra error path can
be removed to simplify things.

Signed-off-by: James Clark <[email protected]>
---
tools/perf/util/cs-etm.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index bf8fbcec2d69..ab30591a6c6a 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -2888,7 +2888,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
int num_cpu, trcidr_idx;
int err = 0;
int i, j;
- u64 *ptr, *hdr = NULL;
+ u64 *ptr = NULL;
u64 **metadata = NULL;
u64 hdr_version;

@@ -2914,15 +2914,8 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
return -EINVAL;
}

- hdr = zalloc(sizeof(*hdr) * CS_HEADER_VERSION_MAX);
- if (!hdr)
- return -ENOMEM;
-
- /* Extract header information - see cs-etm.h for format */
- for (i = 0; i < CS_HEADER_VERSION_MAX; i++)
- hdr[i] = ptr[i];
- num_cpu = hdr[CS_PMU_TYPE_CPUS] & 0xffffffff;
- pmu_type = (unsigned int) ((hdr[CS_PMU_TYPE_CPUS] >> 32) &
+ num_cpu = ptr[CS_PMU_TYPE_CPUS] & 0xffffffff;
+ pmu_type = (unsigned int) ((ptr[CS_PMU_TYPE_CPUS] >> 32) &
0xffffffff);

if (dump_trace)
@@ -2934,10 +2927,8 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
* in anything other than a sequential array is worth doing.
*/
traceid_list = intlist__new(NULL);
- if (!traceid_list) {
- err = -ENOMEM;
- goto err_free_hdr;
- }
+ if (!traceid_list)
+ return -ENOMEM;

metadata = zalloc(sizeof(*metadata) * num_cpu);
if (!metadata) {
@@ -2945,6 +2936,9 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
goto err_free_traceid_list;
}

+ /* Start parsing after the common part of the header */
+ i = CS_HEADER_VERSION_MAX;
+
/*
* The metadata is stored in the auxtrace_info section and encodes
* the configuration of the ARM embedded trace macrocell which is
@@ -3043,7 +3037,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,

etm->num_cpu = num_cpu;
etm->pmu_type = pmu_type;
- etm->snapshot_mode = (hdr[CS_ETM_SNAPSHOT] != 0);
+ etm->snapshot_mode = (ptr[CS_ETM_SNAPSHOT] != 0);
etm->metadata = metadata;
etm->auxtrace_type = auxtrace_info->type;
etm->timeless_decoding = cs_etm__is_timeless_decoding(etm);
@@ -3110,7 +3104,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
zfree(&metadata);
err_free_traceid_list:
intlist__delete(traceid_list);
-err_free_hdr:
- zfree(&hdr);
return err;
}
--
2.25.1

2022-12-12 17:05:50

by James Clark

[permalink] [raw]
Subject: [PATCH 1/5] perf: cs-etm: Print unknown header version as an error

This is an error rather than just for the raw trace dump so always print
it as an error. Also remove the duplicate header version check.

Signed-off-by: James Clark <[email protected]>
---
tools/perf/util/cs-etm.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 16db965ac995..aeb1e30888db 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -2623,14 +2623,7 @@ static void cs_etm__print_auxtrace_info(__u64 *val, int num)
{
int i, cpu = 0, version, err;

- /* bail out early on bad header version */
version = val[0];
- if (version > CS_HEADER_CURRENT_VERSION) {
- /* failure.. return */
- fprintf(stdout, " Unknown Header Version = %x, ", version);
- fprintf(stdout, "Version supported <= %x\n", CS_HEADER_CURRENT_VERSION);
- return;
- }

for (i = 0; i < CS_HEADER_VERSION_MAX; i++)
fprintf(stdout, cs_etm_global_header_fmts[i], val[i]);
@@ -2916,9 +2909,8 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
/* Look for version of the header */
hdr_version = ptr[0];
if (hdr_version > CS_HEADER_CURRENT_VERSION) {
- /* print routine will print an error on bad version */
- if (dump_trace)
- cs_etm__print_auxtrace_info(auxtrace_info->priv, 0);
+ pr_err("\nCS ETM Trace: Unknown Header Version = %#" PRIx64, hdr_version);
+ pr_err(", version supported <= %x\n", CS_HEADER_CURRENT_VERSION);
return -EINVAL;
}

--
2.25.1

2022-12-12 20:03:06

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 0/5] perf: cs-etm: Print auxtrace info even if OpenCSD isn't linked

Em Mon, Dec 12, 2022 at 03:55:08PM +0000, James Clark escreveu:
> The auxtrace info header can be useful for debugging, and at the
> moment it's possible to record a file without OpenCSD linked but
> not view the header even though it should be possible to do.
>
> This patchset tidies up some of the related functions and
> improves some of the error messages before making the above
> possible in the last commit.
>
> Testing done:
>
> * Compiled on x86 and Arm both with and without CORESIGHT=1
> * Ran the Coresight tests
>
> Applies to perf/core (0c3852adae8)

Thanks, applied.

- Arnaldo


> James Clark (5):
> perf: cs-etm: Print unknown header version as an error
> perf: cs-etm: Remove unused stub methods
> perf: cs-etm: Tidy up auxtrace info header printing
> perf: cs-etm: Cleanup cs_etm__process_auxtrace_info()
> perf: cs-etm: Print auxtrace info even if OpenCSD isn't linked
>
> tools/perf/util/Build | 1 +
> tools/perf/util/cs-etm-base.c | 174 ++++++++++++++++++++++++++++
> tools/perf/util/cs-etm.c | 208 +++-------------------------------
> tools/perf/util/cs-etm.h | 46 ++------
> 4 files changed, 200 insertions(+), 229 deletions(-)
> create mode 100644 tools/perf/util/cs-etm-base.c
>
> --
> 2.25.1

--

- Arnaldo