Re: [PATCH v5 16/17] coresight: Emit sink ID in the HW_ID packets

From: Mike Leach
Date: Fri Jul 19 2024 - 05:48:28 EST

Indeed! missed out the:

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

On Fri, 19 Jul 2024 at 10:29, James Clark <james.clark@xxxxxxxxxx> wrote:
>
>
>
> On 17/07/2024 4:03 pm, Mike Leach wrote:
> > On Fri, 12 Jul 2024 at 11:23, James Clark <james.clark@xxxxxxxxxx> wrote:
> >>
> >> From: James Clark <james.clark@xxxxxxx>
> >>
> >> 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@xxxxxxx>
> >> Signed-off-by: James Clark <james.clark@xxxxxxxxxx>
> >> ---
> >> 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