[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJ9a7VjbF2DFRHOtdCt=3o+iNrA++swpJBx9v5-R9Ky8tc4Mvw@mail.gmail.com>
Date: Fri, 23 Dec 2022 09:28:56 +0000
From: Mike Leach <mike.leach@...aro.org>
To: James Clark <james.clark@....com>
Cc: linux-perf-users@...r.kernel.org, tanmay@...vell.com,
sgoutham@...vell.com, gcherian@...vell.com, lcherian@...vell.com,
bbhushan2@...vell.com, German Gomez <german.gomez@....com>,
Mathieu Poirier <mathieu.poirier@...aro.org>,
Suzuki K Poulose <suzuki.poulose@....com>,
Leo Yan <leo.yan@...aro.org>,
John Garry <john.g.garry@...cle.com>,
Will Deacon <will@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>, coresight@...ts.linaro.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 6/7] perf cs_etm: Record ts_source in AUXTRACE_INFO for
ETMv4 and ETE
Hi,
There was a discussion some time ago in one of our coresight regular
dev meetings about this.
Can we just use only the necessary bits that TS source needs and leave
the remaining bits from the 64 as unused for future expansion - i..e
use this as a bitfield rather than have 64 bits occupied for what is
effectively a 2 bit value.
Perhaps call the full value something other than TS_SOURCE and have a
TS_SOURCE field with a known safe unset value.
Thanks
Mike
On Thu, 22 Dec 2022 at 16:07, James Clark <james.clark@....com> wrote:
>
> From: German Gomez <german.gomez@....com>
>
> Read the value of ts_source exposed by the driver and store it in the
> ETMv4 and ETE header. If the interface doesn't exist (such as in older
> Kernels), defaults to a safe value of -1.
>
> Signed-off-by: German Gomez <german.gomez@....com>
> Signed-off-by: James Clark <james.clark@....com>
> ---
> tools/perf/arch/arm/util/cs-etm.c | 48 +++++++++++++++++++++++++++++++
> tools/perf/util/cs-etm-base.c | 2 ++
> tools/perf/util/cs-etm.h | 2 ++
> 3 files changed, 52 insertions(+)
>
> diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
> index b526ffe550a5..481e170cd3f1 100644
> --- a/tools/perf/arch/arm/util/cs-etm.c
> +++ b/tools/perf/arch/arm/util/cs-etm.c
> @@ -53,6 +53,7 @@ static const char * const metadata_etmv4_ro[] = {
> [CS_ETMV4_TRCIDR2] = "trcidr/trcidr2",
> [CS_ETMV4_TRCIDR8] = "trcidr/trcidr8",
> [CS_ETMV4_TRCAUTHSTATUS] = "mgmt/trcauthstatus",
> + [CS_ETMV4_TS_SOURCE] = "ts_source",
> };
>
> static const char * const metadata_ete_ro[] = {
> @@ -62,6 +63,7 @@ static const char * const metadata_ete_ro[] = {
> [CS_ETE_TRCIDR8] = "trcidr/trcidr8",
> [CS_ETE_TRCAUTHSTATUS] = "mgmt/trcauthstatus",
> [CS_ETE_TRCDEVARCH] = "mgmt/trcdevarch",
> + [CS_ETE_TS_SOURCE] = "ts_source",
> };
>
> static bool cs_etm_is_etmv4(struct auxtrace_record *itr, int cpu);
> @@ -613,6 +615,32 @@ static int cs_etm_get_ro(struct perf_pmu *pmu, int cpu, const char *path)
> return val;
> }
>
> +static int cs_etm_get_ro_signed(struct perf_pmu *pmu, int cpu, const char *path)
> +{
> + char pmu_path[PATH_MAX];
> + int scan;
> + int val = 0;
> +
> + /* Get RO metadata from sysfs */
> + snprintf(pmu_path, PATH_MAX, "cpu%d/%s", cpu, path);
> +
> + scan = perf_pmu__scan_file(pmu, pmu_path, "%d", &val);
> + if (scan != 1)
> + pr_err("%s: error reading: %s\n", __func__, pmu_path);
> +
> + return val;
> +}
> +
> +static bool cs_etm_pmu_path_exists(struct perf_pmu *pmu, int cpu, const char *path)
> +{
> + char pmu_path[PATH_MAX];
> +
> + /* Get RO metadata from sysfs */
> + snprintf(pmu_path, PATH_MAX, "cpu%d/%s", cpu, path);
> +
> + return perf_pmu__file_exists(pmu, pmu_path);
> +}
> +
> #define TRCDEVARCH_ARCHPART_SHIFT 0
> #define TRCDEVARCH_ARCHPART_MASK GENMASK(11, 0)
> #define TRCDEVARCH_ARCHPART(x) (((x) & TRCDEVARCH_ARCHPART_MASK) >> TRCDEVARCH_ARCHPART_SHIFT)
> @@ -654,6 +682,16 @@ static void cs_etm_save_etmv4_header(__u64 data[], struct auxtrace_record *itr,
> metadata_etmv4_ro[CS_ETMV4_TRCIDR8]);
> data[CS_ETMV4_TRCAUTHSTATUS] = cs_etm_get_ro(cs_etm_pmu, cpu,
> metadata_etmv4_ro[CS_ETMV4_TRCAUTHSTATUS]);
> +
> + /* Kernels older than 5.19 may not expose ts_source */
> + if (cs_etm_pmu_path_exists(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TS_SOURCE]))
> + data[CS_ETMV4_TS_SOURCE] = (__u64) cs_etm_get_ro_signed(cs_etm_pmu, cpu,
> + metadata_etmv4_ro[CS_ETMV4_TS_SOURCE]);
> + else {
> + pr_warning("[%03d] pmu file 'ts_source' not found. Fallback to safe value (-1)\n",
> + cpu);
> + data[CS_ETMV4_TS_SOURCE] = (__u64) -1;
> + }
> }
>
> static void cs_etm_save_ete_header(__u64 data[], struct auxtrace_record *itr, int cpu)
> @@ -679,6 +717,16 @@ static void cs_etm_save_ete_header(__u64 data[], struct auxtrace_record *itr, in
> /* ETE uses the same registers as ETMv4 plus TRCDEVARCH */
> data[CS_ETE_TRCDEVARCH] = cs_etm_get_ro(cs_etm_pmu, cpu,
> metadata_ete_ro[CS_ETE_TRCDEVARCH]);
> +
> + /* Kernels older than 5.19 may not expose ts_source */
> + if (cs_etm_pmu_path_exists(cs_etm_pmu, cpu, metadata_ete_ro[CS_ETE_TS_SOURCE]))
> + data[CS_ETE_TS_SOURCE] = (__u64) cs_etm_get_ro_signed(cs_etm_pmu, cpu,
> + metadata_ete_ro[CS_ETE_TS_SOURCE]);
> + else {
> + pr_warning("[%03d] pmu file 'ts_source' not found. Fallback to safe value (-1)\n",
> + cpu);
> + data[CS_ETE_TS_SOURCE] = (__u64) -1;
> + }
> }
>
> static void cs_etm_get_metadata(int cpu, u32 *offset,
> diff --git a/tools/perf/util/cs-etm-base.c b/tools/perf/util/cs-etm-base.c
> index 282042c6e944..5f48b756c4cf 100644
> --- a/tools/perf/util/cs-etm-base.c
> +++ b/tools/perf/util/cs-etm-base.c
> @@ -36,6 +36,7 @@ static const char * const cs_etmv4_priv_fmts[] = {
> [CS_ETMV4_TRCIDR2] = " TRCIDR2 %llx\n",
> [CS_ETMV4_TRCIDR8] = " TRCIDR8 %llx\n",
> [CS_ETMV4_TRCAUTHSTATUS] = " TRCAUTHSTATUS %llx\n",
> + [CS_ETMV4_TS_SOURCE] = " TS_SOURCE %lld\n",
> };
>
> static const char * const cs_ete_priv_fmts[] = {
> @@ -50,6 +51,7 @@ static const char * const cs_ete_priv_fmts[] = {
> [CS_ETE_TRCIDR8] = " TRCIDR8 %llx\n",
> [CS_ETE_TRCAUTHSTATUS] = " TRCAUTHSTATUS %llx\n",
> [CS_ETE_TRCDEVARCH] = " TRCDEVARCH %llx\n",
> + [CS_ETE_TS_SOURCE] = " TS_SOURCE %lld\n",
> };
>
> static const char * const param_unk_fmt =
> diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h
> index c5925428afa9..ad790930bcbc 100644
> --- a/tools/perf/util/cs-etm.h
> +++ b/tools/perf/util/cs-etm.h
> @@ -71,6 +71,7 @@ enum {
> CS_ETMV4_TRCIDR2,
> CS_ETMV4_TRCIDR8,
> CS_ETMV4_TRCAUTHSTATUS,
> + CS_ETMV4_TS_SOURCE,
> CS_ETMV4_PRIV_MAX,
> };
>
> @@ -92,6 +93,7 @@ enum {
> CS_ETE_TRCIDR8,
> CS_ETE_TRCAUTHSTATUS,
> CS_ETE_TRCDEVARCH,
> + CS_ETE_TS_SOURCE,
> CS_ETE_PRIV_MAX
> };
>
> --
> 2.25.1
>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Powered by blists - more mailing lists