Hi Leo,
On Fri, Dec 18, 2020 at 2:46 AM Daniel Kiss Daniel.Kiss@arm.com wrote:
Adding Denis.
On 16 Nov 2020, at 10:46, Leo Yan leo.yan@linaro.org wrote:
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".
I have tested the patches on Chrome OS EL2 kernel and they worked fine for me. Note that "perf cs-etm: Add PID format into metadata" patch breaks perf backward compatibility. It may cause a problem in off-target decoding if there is a version skew in perf. I saw a discussion about perf compatibility in https://lists.linaro.org/pipermail/coresight/2020-November/005326.html. I understand that perf doesn't guarantee backward compatibility but in fact incompatibility issues occur rarely. I think if there is an (easy) way to do it the compatibility breakage should be avoided.
This is a critical fix for Chrome OS. Please let me know what you think.
Thanks, Denis
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/302600d5676c45ebfa8a221b2d5fa4172ff4...
CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight