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

From: Mike Leach
Date: Wed Jul 17 2024 - 11:04:59 EST

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