Hi Suzuki,
On Wed, 20 Jul 2022 at 10:30, Suzuki K Poulose suzuki.poulose@arm.com wrote:
On 04/07/2022 09:11, Mike Leach wrote:
Use the perf_report_aux_output_id() call to output the CoreSight trace ID and associated CPU as a PERF_RECORD_AUX_OUTPUT_HW_ID record in the perf.data file.
Signed-off-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/coresight-etm-perf.c | 10 ++++++++++ include/linux/coresight-pmu.h | 14 ++++++++++++++ 2 files changed, 24 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index ad3fdc07c60b..531f5d42272b 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -4,6 +4,7 @@
- Author: Mathieu Poirier mathieu.poirier@linaro.org
*/
+#include <linux/bitfield.h> #include <linux/coresight.h> #include <linux/coresight-pmu.h> #include <linux/cpumask.h> @@ -437,6 +438,7 @@ static void etm_event_start(struct perf_event *event, int flags) struct perf_output_handle *handle = &ctxt->handle; struct coresight_device *sink, *csdev = per_cpu(csdev_src, cpu); struct list_head *path;
u64 hw_id; if (!csdev) goto fail;
@@ -482,6 +484,11 @@ static void etm_event_start(struct perf_event *event, int flags) if (source_ops(csdev)->enable(csdev, event, CS_MODE_PERF)) goto fail_disable_path;
/* output cpu / trace ID in perf record */
hw_id = FIELD_PREP(CS_AUX_HW_ID_VERSION_MASK, CS_AUX_HW_ID_CURR_VERSION) |
FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK, coresight_trace_id_get_cpu_id(cpu));
perf_report_aux_output_id(event, hw_id);
- out: /* Tell the perf core the event is alive */ event->hw.state = 0;
@@ -600,6 +607,9 @@ static void etm_event_stop(struct perf_event *event, int mode)
/* Disabling the path make its elements available to other sessions */ coresight_disable_path(path);
/* release the trace ID we read on event start */
coresight_trace_id_put_cpu_id(cpu);
}
static int etm_event_add(struct perf_event *event, int mode)
diff --git a/include/linux/coresight-pmu.h b/include/linux/coresight-pmu.h index 9f7ee380266b..5572d0e10822 100644 --- a/include/linux/coresight-pmu.h +++ b/include/linux/coresight-pmu.h @@ -7,6 +7,8 @@ #ifndef _LINUX_CORESIGHT_PMU_H #define _LINUX_CORESIGHT_PMU_H
+#include <linux/bits.h>
#define CORESIGHT_ETM_PMU_NAME "cs_etm"
/*
@@ -38,4 +40,16 @@ #define ETM4_CFG_BIT_RETSTK 12 #define ETM4_CFG_BIT_VMID_OPT 15
+/*
- Interpretation of the PERF_RECORD_AUX_OUTPUT_HW_ID payload.
- Used to associate a CPU with the CoreSight Trace ID.
- [63:16] - unused SBZ
- [15:08] - Trace ID
- [07:00] - Version
Could we please re-arrange the fields, such that it is easier to comprehend the TraceID looking at the raw trace dump ? Also to accommodate the future changes.
e.g, [15:00] - Trace ID /* For future expansion, if at all */ [59:16] - RES0 [63:60] - Trace_ID_Version
I think we *might* (not sure yet) end up adding "sinkid" when we have sink specific allocation, so that we can associate the HW_ID of an event to the "AUXTRACE" record (i.e., trace buffer).
If we go to per sink trace ID maps, then I can't see how we could avoid needing some sort of ID in here, unless we can determine some other method of specifying which CPUs traced into which trace buffer.
So if we need to do that we could:
[15:00] - Trace ID /* For future expansion, if at all */ [47:16] - Trace Pool ID( == 0 if global, == sink_id if sink based) [59:48] - RES0 [63:60] - Trace_ID_Version == 1
Or we could adopt the above straight away.
I wouldn't want to commit to a size for the sink ID yet. And I would leave trace ID at what it is for now (8 bits). Make the fields represent what is and up-version and update when changes are actually required. I think this packet may be a candidate for delivering other trace related info we may need in future - such as the timestamp source that is being worked on?
Mike
Thoughts ?
Suzuki
- */
+#define CS_AUX_HW_ID_VERSION_MASK GENMASK_ULL(7, 0) +#define CS_AUX_HW_ID_TRACE_ID_MASK GENMASK_ULL(15, 8)
+#define CS_AUX_HW_ID_CURR_VERSION 0
- #endif