[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3b2285e0-3ac2-448a-8a53-c1bb6ead78b8@linaro.org>
Date: Fri, 19 Jul 2024 10:29:37 +0100
From: James Clark <james.clark@...aro.org>
To: Mike Leach <mike.leach@...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
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
Powered by blists - more mailing lists