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: <CAJ9a7VgQ5JfxObZpxbQdiEciOjkuijH98HFfdFVK4qkn7qbNUQ@mail.gmail.com>
Date: Fri, 19 Jul 2024 10:48:02 +0100
From: Mike Leach <mike.leach@...aro.org>
To: James Clark <james.clark@...aro.org>
Cc: coresight@...ts.linaro.org, suzuki.poulose@....com, 
	gankulkarni@...amperecomputing.com, leo.yan@...ux.dev, 
	anshuman.khandual@....com, James Clark <james.clark@....com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, 
	Maxime Coquelin <mcoquelin.stm32@...il.com>, Alexandre Torgue <alexandre.torgue@...s.st.com>, 
	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>, Namhyung Kim <namhyung@...nel.org>, 
	Mark Rutland <mark.rutland@....com>, Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>, 
	Adrian Hunter <adrian.hunter@...el.com>, "Liang, Kan" <kan.liang@...ux.intel.com>, 
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-stm32@...md-mailman.stormreply.com, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v5 16/17] coresight: Emit sink ID in the HW_ID packets

Indeed! missed out the:

Reviewed-by: Mike Leach <mike.leach@...aro.org>

On Fri, 19 Jul 2024 at 10:29, James Clark <james.clark@...aro.org> wrote:
>
>
>
> On 17/07/2024 4:03 pm, Mike Leach wrote:
> > On Fri, 12 Jul 2024 at 11:23, James Clark <james.clark@...aro.org> wrote:
> >>
> >> From: James Clark <james.clark@....com>
> >>
> >> For Perf to be able to decode when per-sink trace IDs are used, emit the
> >> sink that's being written to for each ETM.
> >>
> >> Perf currently errors out if it sees a newer packet version so instead
> >> of bumping it, add a new minor version field. This can be used to
> >> signify new versions that have backwards compatible fields. Considering
> >> this change is only for high core count machines, it doesn't make sense
> >> to make a breaking change for everyone.
> >>
> >> Signed-off-by: James Clark <james.clark@....com>
> >> Signed-off-by: James Clark <james.clark@...aro.org>
> >> ---
> >>   drivers/hwtracing/coresight/coresight-core.c  | 26 ++++++++++---------
> >>   .../hwtracing/coresight/coresight-etm-perf.c  | 16 ++++++++----
> >>   drivers/hwtracing/coresight/coresight-priv.h  |  1 +
> >>   include/linux/coresight-pmu.h                 | 17 +++++++++---
> >>   4 files changed, 39 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> >> index faf560ba8d64..c427e9344a84 100644
> >> --- a/drivers/hwtracing/coresight/coresight-core.c
> >> +++ b/drivers/hwtracing/coresight/coresight-core.c
> >> @@ -487,23 +487,25 @@ struct coresight_device *coresight_get_sink(struct list_head *path)
> >>          return csdev;
> >>   }
> >>
> >> +u32 coresight_get_sink_id(struct coresight_device *csdev)
> >> +{
> >> +       if (!csdev->ea)
> >> +               return 0;
> >> +
> >> +       /*
> >> +        * See function etm_perf_add_symlink_sink() to know where
> >> +        * this comes from.
> >> +        */
> >> +       return (u32) (unsigned long) csdev->ea->var;
> >> +}
> >> +
> >>   static int coresight_sink_by_id(struct device *dev, const void *data)
> >>   {
> >>          struct coresight_device *csdev = to_coresight_device(dev);
> >> -       unsigned long hash;
> >>
> >>          if (csdev->type == CORESIGHT_DEV_TYPE_SINK ||
> >> -            csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) {
> >> -
> >> -               if (!csdev->ea)
> >> -                       return 0;
> >> -               /*
> >> -                * See function etm_perf_add_symlink_sink() to know where
> >> -                * this comes from.
> >> -                */
> >> -               hash = (unsigned long)csdev->ea->var;
> >> -
> >> -               if ((u32)hash == *(u32 *)data)
> >> +           csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) {
> >> +               if (coresight_get_sink_id(csdev) == *(u32 *)data)
> >>                          return 1;
> >>          }
> >>
> >> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> >> index 70c99f0409b2..ad6a8f4b70b6 100644
> >> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> >> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> >> @@ -460,6 +460,7 @@ static void etm_event_start(struct perf_event *event, int flags)
> >>          struct coresight_device *sink, *csdev = per_cpu(csdev_src, cpu);
> >>          struct list_head *path;
> >>          u64 hw_id;
> >> +       u8 trace_id;
> >>
> >>          if (!csdev)
> >>                  goto fail;
> >> @@ -512,11 +513,16 @@ static void etm_event_start(struct perf_event *event, int flags)
> >>           */
> >>          if (!cpumask_test_cpu(cpu, &event_data->aux_hwid_done)) {
> >>                  cpumask_set_cpu(cpu, &event_data->aux_hwid_done);
> >> -               hw_id = FIELD_PREP(CS_AUX_HW_ID_VERSION_MASK,
> >> -                                  CS_AUX_HW_ID_CURR_VERSION);
> >> -               hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK,
> >> -                                   coresight_trace_id_read_cpu_id_map(cpu,
> >> -                                                                      &sink->perf_sink_id_map));
> >> +
> >> +               trace_id = coresight_trace_id_read_cpu_id_map(cpu, &sink->perf_sink_id_map);
> >> +
> >> +               hw_id = FIELD_PREP(CS_AUX_HW_ID_MAJOR_VERSION_MASK,
> >> +                               CS_AUX_HW_ID_MAJOR_VERSION);
> >> +               hw_id |= FIELD_PREP(CS_AUX_HW_ID_MINOR_VERSION_MASK,
> >> +                               CS_AUX_HW_ID_MINOR_VERSION);
> >> +               hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK, trace_id);
> >> +               hw_id |= FIELD_PREP(CS_AUX_HW_ID_SINK_ID_MASK, coresight_get_sink_id(sink));
> >> +
> >>                  perf_report_aux_output_id(event, hw_id);
> >>          }
> >>
> >> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> >> index 61a46d3bdcc8..05f891ca6b5c 100644
> >> --- a/drivers/hwtracing/coresight/coresight-priv.h
> >> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> >> @@ -148,6 +148,7 @@ int coresight_make_links(struct coresight_device *orig,
> >>                           struct coresight_device *target);
> >>   void coresight_remove_links(struct coresight_device *orig,
> >>                              struct coresight_connection *conn);
> >> +u32 coresight_get_sink_id(struct coresight_device *csdev);
> >>
> >>   #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X)
> >>   extern int etm_readl_cp14(u32 off, unsigned int *val);
> >> diff --git a/include/linux/coresight-pmu.h b/include/linux/coresight-pmu.h
> >> index 51ac441a37c3..89b0ac0014b0 100644
> >> --- a/include/linux/coresight-pmu.h
> >> +++ b/include/linux/coresight-pmu.h
> >> @@ -49,12 +49,21 @@
> >>    * Interpretation of the PERF_RECORD_AUX_OUTPUT_HW_ID payload.
> >>    * Used to associate a CPU with the CoreSight Trace ID.
> >>    * [07:00] - Trace ID - uses 8 bits to make value easy to read in file.
> >> - * [59:08] - Unused (SBZ)
> >> - * [63:60] - Version
> >> + * [39:08] - Sink ID - as reported in /sys/bus/event_source/devices/cs_etm/sinks/
> >> + *           Added in minor version 1.
> >> + * [55:40] - Unused (SBZ)
> >> + * [59:56] - Minor Version - previously existing fields are compatible with
> >> + *           all minor versions.
> >> + * [63:60] - Major Version - previously existing fields mean different things
> >> + *           in new major versions.
> >>    */
> >>   #define CS_AUX_HW_ID_TRACE_ID_MASK     GENMASK_ULL(7, 0)
> >> -#define CS_AUX_HW_ID_VERSION_MASK      GENMASK_ULL(63, 60)
> >> +#define CS_AUX_HW_ID_SINK_ID_MASK      GENMASK_ULL(39, 8)
> >>
> >> -#define CS_AUX_HW_ID_CURR_VERSION 0
> >> +#define CS_AUX_HW_ID_MINOR_VERSION_MASK        GENMASK_ULL(59, 56)
> >> +#define CS_AUX_HW_ID_MAJOR_VERSION_MASK        GENMASK_ULL(63, 60)
> >> +
> >> +#define CS_AUX_HW_ID_MAJOR_VERSION 0
> >> +#define CS_AUX_HW_ID_MINOR_VERSION 1
> >>
> >>   #endif
> >> --
> >> 2.34.1
> >>
> >
> >
> > --
> > Mike Leach
> > Principal Engineer, ARM Ltd.
> > Manchester Design Centre. UK
>
> Hi Mike,
>
> I think you miss-sent this one
>


-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ