From: Suzuki K Poulose <suzuki.poulose@arm.com>
To: James Clark <james.clark@arm.com>,
mathieu.poirier@linaro.org, leo.yan@linaro.org,
coresight@lists.linaro.org, linux-perf-users@vger.kernel.org,
mike.leach@linaro.org
Cc: acme@kernel.org, John Garry <john.garry@huawei.com>,
Will Deacon <will@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 4/9] perf cs-etm: Save TRCDEVARCH register
Date: Fri, 3 Sep 2021 10:08:22 +0100 [thread overview]
Message-ID: <44a386f9-7c02-2004-9e81-7c19cd9ce623@arm.com> (raw)
In-Reply-To: <20210806134109.1182235-5-james.clark@arm.com>
Hi James
On 06/08/2021 14:41, James Clark wrote:
> When ETE is present save the TRCDEVARCH register and set a new magic
> number. It will be used to configure the decoder in a later commit.
>
> Old versions of perf will not be able to open files with this new magic
> number, but old files will still work with newer versions of perf.
>
The patch looks good to me. There are some minor style related comments
below.
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
> tools/perf/arch/arm/util/cs-etm.c | 49 +++++++++++++++++++++++++++----
> tools/perf/util/cs-etm.c | 16 ++++++++--
> tools/perf/util/cs-etm.h | 10 +++++++
> 3 files changed, 68 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
> index ecb6fa55a210..e3500b79d972 100644
> --- a/tools/perf/arch/arm/util/cs-etm.c
> +++ b/tools/perf/arch/arm/util/cs-etm.c
> @@ -47,15 +47,17 @@ static const char *metadata_etmv3_ro[CS_ETM_PRIV_MAX] = {
> [CS_ETM_ETMIDR] = "mgmt/etmidr",
> };
>
> -static const char *metadata_etmv4_ro[CS_ETMV4_PRIV_MAX] = {
> +static const char * const metadata_etmv4_ro[] = {
> [CS_ETMV4_TRCIDR0] = "trcidr/trcidr0",
> [CS_ETMV4_TRCIDR1] = "trcidr/trcidr1",
> [CS_ETMV4_TRCIDR2] = "trcidr/trcidr2",
> [CS_ETMV4_TRCIDR8] = "trcidr/trcidr8",
> [CS_ETMV4_TRCAUTHSTATUS] = "mgmt/trcauthstatus",
> + [CS_ETE_TRCDEVARCH] = "mgmt/trcdevarch"
> };
>
> static bool cs_etm_is_etmv4(struct auxtrace_record *itr, int cpu);
> +static bool cs_etm_is_ete(struct auxtrace_record *itr, int cpu);
>
> static int cs_etm_set_context_id(struct auxtrace_record *itr,
> struct evsel *evsel, int cpu)
> @@ -533,7 +535,7 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused,
> struct evlist *evlist __maybe_unused)
> {
> int i;
> - int etmv3 = 0, etmv4 = 0;
> + int etmv3 = 0, etmv4 = 0, ete = 0;
> struct perf_cpu_map *event_cpus = evlist->core.cpus;
> struct perf_cpu_map *online_cpus = perf_cpu_map__new(NULL);
>
> @@ -544,7 +546,9 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused,
> !cpu_map__has(online_cpus, i))
> continue;
>
> - if (cs_etm_is_etmv4(itr, i))
> + if (cs_etm_is_ete(itr, i))
> + ete++;
> + else if (cs_etm_is_etmv4(itr, i))
> etmv4++;
> else
> etmv3++;
> @@ -555,7 +559,9 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused,
> if (!cpu_map__has(online_cpus, i))
> continue;
>
> - if (cs_etm_is_etmv4(itr, i))
> + if (cs_etm_is_ete(itr, i))
> + ete++;
> + else if (cs_etm_is_etmv4(itr, i))
> etmv4++;
> else
> etmv3++;
> @@ -565,6 +571,7 @@ cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused,
> perf_cpu_map__put(online_cpus);
>
> return (CS_ETM_HEADER_SIZE +
> + (ete * CS_ETE_PRIV_SIZE) +
> (etmv4 * CS_ETMV4_PRIV_SIZE) +
> (etmv3 * CS_ETMV3_PRIV_SIZE));
> }
> @@ -607,6 +614,27 @@ static int cs_etm_get_ro(struct perf_pmu *pmu, int cpu, const char *path)
> return val;
> }
>
> +#define TRCDEVARCH_ARCHPART_SHIFT 0
> +#define TRCDEVARCH_ARCHPART_MASK GENMASK(11, 0)
> +#define TRCDEVARCH_ARCHPART(x) (((x) & TRCDEVARCH_ARCHPART_MASK) >> TRCDEVARCH_ARCHPART_SHIFT)
> +
> +#define TRCDEVARCH_ARCHVER_SHIFT 12
> +#define TRCDEVARCH_ARCHVER_MASK GENMASK(15, 12)
> +#define TRCDEVARCH_ARCHVER(x) (((x) & TRCDEVARCH_ARCHVER_MASK) >> TRCDEVARCH_ARCHVER_SHIFT)
> +
> +static bool cs_etm_is_ete(struct auxtrace_record *itr, int cpu)
> +{
> + struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr);
> + struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
> + int trcdevarch = cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETE_TRCDEVARCH]);
> +
> + /*
> + * ETE if ARCHVER is 5 (ARCHVER is 4 for ETM) and ARCHPART is 0xA13.
> + * See ETM_DEVARCH_ETE_ARCH in coresight-etm4x.h
> + */
> + return TRCDEVARCH_ARCHVER(trcdevarch) == 5 && TRCDEVARCH_ARCHPART(trcdevarch) == 0xA13;
> +}
> +
> static void cs_etm_save_etmv4_header(__u64 data[], struct auxtrace_record *itr, int cpu)
> {
> struct cs_etm_recording *ptr = container_of(itr, struct cs_etm_recording, itr);
> @@ -640,7 +668,18 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,
> struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
>
> /* first see what kind of tracer this cpu is affined to */
> - if (cs_etm_is_etmv4(itr, cpu)) {
> + if (cs_etm_is_ete(itr, cpu)) {
> + magic = __perf_cs_ete_magic;
> + /* ETE uses the same registers as ETMv4 plus TRCDEVARCH */
> + cs_etm_save_etmv4_header(&info->priv[*offset], itr, cpu);
> + info->priv[*offset + CS_ETE_TRCDEVARCH] =
> + cs_etm_get_ro(cs_etm_pmu, cpu,
> + metadata_etmv4_ro[CS_ETE_TRCDEVARCH]);
> +
minor nit: It may be nicer to have a helper function like we do for
etmv4. i.e,
cs_etm_save_ete_header();
to keep the code consistent.
> + /* How much space was used */
> + increment = CS_ETE_PRIV_MAX;
> + nr_trc_params = CS_ETE_PRIV_MAX - CS_ETM_COMMON_BLK_MAX_V1;
> + } else if (cs_etm_is_etmv4(itr, cpu)) {
> magic = __perf_cs_etmv4_magic;
> cs_etm_save_etmv4_header(&info->priv[*offset], itr, cpu);
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index f4b2bff533f3..d540512a3c96 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> for (j = 0; j < total_params; j++, i++) {
> /* if newer record - could be excess params */
> - if (j >= CS_ETMV4_PRIV_MAX)
> + if (j >= CS_ETE_PRIV_MAX)
> fprintf(stdout, param_unk_fmt, j, val[i]);
> else
> fprintf(stdout, cs_etmv4_priv_fmts[j], val[i]);
> @@ -2943,6 +2949,12 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>
> /* The traceID is our handle */
> trcidr_idx = CS_ETMV4_TRCTRACEIDR;
> + } else if (ptr[i] == __perf_cs_ete_magic) {
> + metadata[j] =
> + cs_etm__create_meta_blk(ptr, &i, CS_ETE_PRIV_MAX, -1);
minor nit: I would prefer the arguments to be split into the next line,
rather than moving the function call to the next line. i.e,
metatdata[j] = cs_etm_create_meta_blk(ptr, &i,
....
> +
> + /* ETE shares first part of metadata with ETMv4 */
> + trcidr_idx = CS_ETMV4_TRCTRACEIDR;
> }
>
> if (!metadata[j]) {
> diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h
> index d65c7b19407d..08b84c21f065 100644
> --- a/tools/perf/util/cs-etm.h
> +++ b/tools/perf/util/cs-etm.h
> @@ -76,6 +76,14 @@ enum {
> /* define fixed version 0 length - allow new format reader to read old files. */
> #define CS_ETMV4_NR_TRC_PARAMS_V0 (CS_ETMV4_TRCAUTHSTATUS - CS_ETMV4_TRCCONFIGR + 1)
>
> +/* ETE metadata is ETMv4 plus TRCDEVARCH register and doesn't support header V0 since it was
minor nit: comment style.
/*
* ETE metadata...
*
*/
Suzuki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-09-03 9:11 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-06 13:41 [PATCH v2 0/9] Support ETE decoding James Clark
2021-08-06 13:41 ` [PATCH v2 1/9] perf cs-etm: Refactor initialisation of decoder params James Clark
2021-09-03 8:52 ` Suzuki K Poulose
2021-09-03 11:05 ` Arnaldo Carvalho de Melo
2021-09-05 21:24 ` Mike Leach
2021-08-06 13:41 ` [PATCH v2 2/9] perf cs-etm: Initialise architecture based on TRCIDR1 James Clark
2021-08-24 6:53 ` Leo Yan
2021-09-03 8:55 ` Suzuki K Poulose
2021-09-03 11:07 ` Arnaldo Carvalho de Melo
2021-09-05 21:24 ` Mike Leach
2021-08-06 13:41 ` [PATCH v2 3/9] perf cs-etm: Refactor out ETMv4 header saving James Clark
2021-08-24 6:58 ` Leo Yan
2021-09-03 8:56 ` Suzuki K Poulose
2021-09-05 21:24 ` Mike Leach
2021-08-06 13:41 ` [PATCH v2 4/9] perf cs-etm: Save TRCDEVARCH register James Clark
2021-08-24 7:14 ` Leo Yan
2021-09-03 9:08 ` Suzuki K Poulose [this message]
2021-09-03 11:15 ` Arnaldo Carvalho de Melo
2021-09-05 21:24 ` Mike Leach
2021-08-06 13:41 ` [PATCH v2 5/9] perf cs-etm: Fix typo James Clark
2021-08-24 7:15 ` Leo Yan
2021-09-03 9:09 ` Suzuki K Poulose
2021-09-05 21:24 ` Mike Leach
2021-08-06 13:41 ` [PATCH v2 6/9] perf cs-etm: Update OpenCSD decoder for ETE James Clark
2021-09-03 9:10 ` Suzuki K Poulose
2021-09-03 11:16 ` Arnaldo Carvalho de Melo
2021-09-05 21:24 ` Mike Leach
2021-08-06 13:41 ` [PATCH v2 7/9] perf cs-etm: Create ETE decoder James Clark
2021-08-24 8:33 ` Leo Yan
2021-08-25 10:13 ` James Clark
2021-09-03 9:15 ` Suzuki K Poulose
2021-09-03 11:16 ` Arnaldo Carvalho de Melo
2021-09-05 21:24 ` Mike Leach
2021-08-06 13:41 ` [PATCH v2 8/9] perf cs-etm: Print the decoder name James Clark
2021-09-03 9:17 ` Suzuki K Poulose
2021-09-03 11:18 ` Arnaldo Carvalho de Melo
2021-09-03 11:52 ` Arnaldo Carvalho de Melo
2021-09-05 21:24 ` Mike Leach
2021-08-06 13:41 ` [PATCH v2 9/9] perf cs-etm: Show a warning for an unknown magic number James Clark
2021-08-24 8:36 ` Leo Yan
2021-09-01 15:54 ` Arnaldo Carvalho de Melo
2021-09-01 16:07 ` Arnaldo Carvalho de Melo
2021-09-01 16:16 ` Arnaldo Carvalho de Melo
2021-09-01 16:25 ` Arnaldo Carvalho de Melo
2021-09-01 16:49 ` Arnaldo Carvalho de Melo
2021-09-02 7:53 ` Leo Yan
2021-09-02 17:48 ` Mathieu Poirier
2021-09-02 18:19 ` Arnaldo Carvalho de Melo
2021-09-02 19:22 ` Mathieu Poirier
2021-09-05 21:24 ` Mike Leach
2021-08-24 8:47 ` [PATCH v2 0/9] Support ETE decoding Leo Yan
2021-08-25 10:14 ` James Clark
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=44a386f9-7c02-2004-9e81-7c19cd9ce623@arm.com \
--to=suzuki.poulose@arm.com \
--cc=acme@kernel.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=coresight@lists.linaro.org \
--cc=james.clark@arm.com \
--cc=john.garry@huawei.com \
--cc=jolsa@redhat.com \
--cc=leo.yan@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mathieu.poirier@linaro.org \
--cc=mike.leach@linaro.org \
--cc=namhyung@kernel.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).