lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ