Re: [PATCH 3/3] rfc: perf: cs_etm: Detect pid in VMID for kernel running at EL2

[Leo Yan <leo.yan@linaro.org>]

Hi Suzuki,

On Fri, Nov 13, 2020 at 09:47:42AM +0000, Suzuki Kuruppassery Poulose wrote:

[...]

> > Rather than heuristic method, I think we can use metadata to store
> > "pid" entry in the function cs_etm_get_metadata(), so that in the
> > record phase, the "pid" is stored into perf data file.
> 
> True, that is a good idea. That makes it future proof.
> 
> > 
> > When decode the trace data, we can retrieve "pid" value from the
> > metadata, and can base on the "pid" value to make decision for using
> > context_id or VMID.
> 
> > 
> > The metadata can be accessed by referring
> > cs_etm_queue::cs_etm_auxtrace::metadata; but the file
> > util/cs-etm-decoder/cs-etm-decoder.c cannot directly access the
> > metadata structure due to "cs_etm_queue" and "cs_etm_auxtrace" both
> > are defined in util/cs-etm.c.  It's better to introduce a helper
> > function in util/cs-etm.c to retrieve "pid" value, like:
> > 
> >    u64 cs_etm__etmq_get_pid_type(struct cs_etm_queue *etmq);
> 
> I am not an expert in that side, how it interacts with the OpenCSD.
> thus left this patch as an RFC.
> 
> I appreciate your feedback and thoughts. If you feel like you could
> cleanup and implement this, please feel free to do so. Otherwise, I
> might try it out whenever I have some spare cycles to understand
> this side of the code (and this might be delayed).

Below is the drafted patch for support PID format in the metadata and
I tested for "perf record" and "perf script" command and can work as
expected.

P.s. I uploaded the patches into the github [1] and gave some minor
refactoring for your last patch "perf cs-etm: Detect pid in VMID for
kernel running at EL2".

Please review and consider to consolidate the changes if look good for
you.

Thanks,

---8<---

From ba70531217c51f2b4115965bd7e4b7b51770a626 Mon Sep 17 00:00:00 2001
From: Leo Yan <leo.yan@linaro.org>
Date: Sat, 14 Nov 2020 09:47:12 +0800
Subject: [PATCH] perf cs-etm: Add PID format into metadata

It's possible for CoreSight to trace PID in either CONTEXTIDR_EL1 or
CONTEXTIDR_EL2, the PID format info can be used to distinguish the PID
is traced in which register.

This patch saves PID format value into the metadata when record the
trace data; during the decoding phase, it provides a helper function
cs_etm__get_pid_fmt() for easier retrieving PID format from metadata.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/arch/arm/util/cs-etm.c | 21 +++++++++++++++++++++
 tools/perf/util/cs-etm.c          | 21 +++++++++++++++++++++
 tools/perf/util/cs-etm.h          |  3 +++
 3 files changed, 45 insertions(+)

diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index e6207ce7cc85..837eebe30a90 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -606,6 +606,7 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,
 	struct cs_etm_recording *ptr =
 			container_of(itr, struct cs_etm_recording, itr);
 	struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
+	u64 pid_fmt;
 
 	/* first see what kind of tracer this cpu is affined to */
 	if (cs_etm_is_etmv4(itr, cpu)) {
@@ -634,6 +635,16 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,
 				      metadata_etmv4_ro
 				      [CS_ETMV4_TRCAUTHSTATUS]);
 
+		/*
+		 * The PID format will be used when decode the trace data;
+		 * based on it the decoder will make decision for setting
+		 * sample's PID as context_id or VMID.
+		 */
+		pid_fmt = perf_pmu__format_bits(&cs_etm_pmu->format, "pid");
+		if (!pid_fmt)
+			pid_fmt = 1ULL << ETM_OPT_CTXTID;
+		info->priv[*offset + CS_ETMV4_PID_FMT] = pid_fmt;
+
 		/* How much space was used */
 		increment = CS_ETMV4_PRIV_MAX;
 	} else {
@@ -651,6 +662,16 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,
 			cs_etm_get_ro(cs_etm_pmu, cpu,
 				      metadata_etmv3_ro[CS_ETM_ETMIDR]);
 
+		/*
+		 * The PID format will be used when decode the trace data;
+		 * based on it the decoder will make decision for setting
+		 * sample's PID as context_id or VMID.
+		 */
+		pid_fmt = perf_pmu__format_bits(&cs_etm_pmu->format, "pid");
+		if (!pid_fmt)
+			pid_fmt = 1ULL << ETM_OPT_CTXTID;
+		info->priv[*offset + CS_ETM_PID_FMT] = pid_fmt;
+
 		/* How much space was used */
 		increment = CS_ETM_PRIV_MAX;
 	}
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index a2a369e2fbb6..ffd91c8dadf0 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -156,6 +156,25 @@ int cs_etm__get_cpu(u8 trace_chan_id, int *cpu)
 	return 0;
 }
 
+int cs_etm__get_pid_fmt(u8 trace_chan_id, u64 *pid_fmt)
+{
+	struct int_node *inode;
+	u64 *metadata;
+
+	inode = intlist__find(traceid_list, trace_chan_id);
+	if (!inode)
+		return -EINVAL;
+
+	metadata = inode->priv;
+
+	if (metadata[CS_ETM_MAGIC] == __perf_cs_etmv3_magic)
+		*pid_fmt = (int)metadata[CS_ETM_PID_FMT];
+	else
+		*pid_fmt = (int)metadata[CS_ETMV4_PID_FMT];
+
+	return 0;
+}
+
 void cs_etm__etmq_set_traceid_queue_timestamp(struct cs_etm_queue *etmq,
 					      u8 trace_chan_id)
 {
@@ -2447,6 +2466,7 @@ static const char * const cs_etm_priv_fmts[] = {
 	[CS_ETM_ETMTRACEIDR]	= "	ETMTRACEIDR		       %llx\n",
 	[CS_ETM_ETMCCER]	= "	ETMCCER			       %llx\n",
 	[CS_ETM_ETMIDR]		= "	ETMIDR			       %llx\n",
+	[CS_ETM_PID_FMT]	= "	PID Format		       %llx\n",
 };
 
 static const char * const cs_etmv4_priv_fmts[] = {
@@ -2459,6 +2479,7 @@ static const char * const cs_etmv4_priv_fmts[] = {
 	[CS_ETMV4_TRCIDR2]	= "	TRCIDR2			       %llx\n",
 	[CS_ETMV4_TRCIDR8]	= "	TRCIDR8			       %llx\n",
 	[CS_ETMV4_TRCAUTHSTATUS] = "	TRCAUTHSTATUS		       %llx\n",
+	[CS_ETMV4_PID_FMT]	= "	PID Format		       %llx\n",
 };
 
 static void cs_etm__print_auxtrace_info(__u64 *val, int num)
diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h
index 4ad925d6d799..98801040175f 100644
--- a/tools/perf/util/cs-etm.h
+++ b/tools/perf/util/cs-etm.h
@@ -38,6 +38,7 @@ enum {
 	/* RO, taken from sysFS */
 	CS_ETM_ETMCCER,
 	CS_ETM_ETMIDR,
+	CS_ETM_PID_FMT,
 	CS_ETM_PRIV_MAX,
 };
 
@@ -52,6 +53,7 @@ enum {
 	CS_ETMV4_TRCIDR2,
 	CS_ETMV4_TRCIDR8,
 	CS_ETMV4_TRCAUTHSTATUS,
+	CS_ETMV4_PID_FMT,
 	CS_ETMV4_PRIV_MAX,
 };
 
@@ -173,6 +175,7 @@ struct cs_etm_packet_queue {
 int cs_etm__process_auxtrace_info(union perf_event *event,
 				  struct perf_session *session);
 int cs_etm__get_cpu(u8 trace_chan_id, int *cpu);
+int cs_etm__get_pid_fmt(u8 trace_chan_id, u64 *pid_fmt);
 int cs_etm__etmq_set_tid(struct cs_etm_queue *etmq,
 			 pid_t tid, u8 trace_chan_id);
 bool cs_etm__etmq_is_timeless(struct cs_etm_queue *etmq);
-- 
2.17.1


[1] https://github.com/Leo-Yan/linux/tree/perf_cs_etm_pid_tracing_vhe_for_suzuki
[2] https://github.com/Leo-Yan/linux/commit/302600d5676c45ebfa8a221b2d5fa4172ff42a91

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel