Hi Leo

On 2/2/21 4:38 PM, Leo Yan wrote:

In theory, the options should be arbitrary values and are neutral for any ETM version; so far perf tool uses ETMv3.5/PTM ETMCR config bits except for register's bit definitions, also uses as options.

This can introduce confusion, especially if we want to add a new option but the new option is not supported by ETMv3.5/PTM ETMCR. But on the other hand, we cannot change options since these options are generic CoreSight PMU ABI.

For easier maintenance and avoid confusion, this patch refines the comment to clarify perf options, and gives out the background info for these bits are coming from ETMv3.5/PTM. Afterwards, we should take these options as general knobs, and if there have any confliction with ETMv3.5/PTM, should consider to define saperate macros for ETMv3.5/PTM ETMCR config bits.

Suggested-by: Suzuki K Poulose suzuki.poulose@arm.com Signed-off-by: Leo Yan leo.yan@linaro.org

The patch looks good to me. The only concern I have is, whether we should split this patch to kernel vs tools ? As the kernel changes go via coresight tree and the tools patch may go via perf tree ?

Either way, for the patch:

Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com

---

.../hwtracing/coresight/coresight-etm-perf.c | 5 ++++- include/linux/coresight-pmu.h | 17 ++++++++++++----- tools/include/linux/coresight-pmu.h | 17 ++++++++++++----- 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index bdc34ca449f7..465ef1aa8c82 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -27,7 +27,10 @@ static bool etm_perf_up; static DEFINE_PER_CPU(struct perf_output_handle, ctx_handle); static DEFINE_PER_CPU(struct coresight_device *, csdev_src); -/* ETMv3.5/PTM's ETMCR is 'config' */ +/*

• • The PMU formats were orignally for ETMv3.5/PTM's ETMCR 'config';
• • now take them as general formats and apply on all ETMs.
• */ PMU_FORMAT_ATTR(cycacc, "config:" __stringify(ETM_OPT_CYCACC)); PMU_FORMAT_ATTR(contextid, "config:" __stringify(ETM_OPT_CTXTID)); PMU_FORMAT_ATTR(timestamp, "config:" __stringify(ETM_OPT_TS));

diff --git a/include/linux/coresight-pmu.h b/include/linux/coresight-pmu.h index b0e35eec6499..5dc47cfdcf07 100644 --- a/include/linux/coresight-pmu.h +++ b/include/linux/coresight-pmu.h @@ -10,11 +10,18 @@ #define CORESIGHT_ETM_PMU_NAME "cs_etm" #define CORESIGHT_ETM_PMU_SEED 0x10 -/* ETMv3.5/PTM's ETMCR config bit */ -#define ETM_OPT_CYCACC 12 -#define ETM_OPT_CTXTID 14 -#define ETM_OPT_TS 28 -#define ETM_OPT_RETSTK 29 +/*

• • Below are the definition of bit offsets for perf option, and works as
• • arbitrary values for all ETM versions.
• • Most of them are orignally from ETMv3.5/PTM's ETMCR config, therefore,
• • ETMv3.5/PTM doesn't define ETMCR config bits with prefix "ETM3_" and
• • directly use below macros as config bits.
• */

+#define ETM_OPT_CYCACC 12 +#define ETM_OPT_CTXTID 14 +#define ETM_OPT_TS 28 +#define ETM_OPT_RETSTK 29 /* ETMv4 CONFIGR programming bits for the ETM OPTs */ #define ETM4_CFG_BIT_CYCACC 4 diff --git a/tools/include/linux/coresight-pmu.h b/tools/include/linux/coresight-pmu.h index b0e35eec6499..5dc47cfdcf07 100644 --- a/tools/include/linux/coresight-pmu.h +++ b/tools/include/linux/coresight-pmu.h @@ -10,11 +10,18 @@ #define CORESIGHT_ETM_PMU_NAME "cs_etm" #define CORESIGHT_ETM_PMU_SEED 0x10 -/* ETMv3.5/PTM's ETMCR config bit */ -#define ETM_OPT_CYCACC 12 -#define ETM_OPT_CTXTID 14 -#define ETM_OPT_TS 28 -#define ETM_OPT_RETSTK 29 +/*

• • Below are the definition of bit offsets for perf option, and works as
• • arbitrary values for all ETM versions.
• • Most of them are orignally from ETMv3.5/PTM's ETMCR config, therefore,
• • ETMv3.5/PTM doesn't define ETMCR config bits with prefix "ETM3_" and
• • directly use below macros as config bits.
• */

+#define ETM_OPT_CYCACC 12 +#define ETM_OPT_CTXTID 14 +#define ETM_OPT_TS 28 +#define ETM_OPT_RETSTK 29 /* ETMv4 CONFIGR programming bits for the ETM OPTs */ #define ETM4_CFG_BIT_CYCACC 4

On Tue, 2 Feb 2021 at 16:39, Leo Yan leo.yan@linaro.org wrote:

From: Suzuki K Poulose suzuki.poulose@arm.com

When set option with macros ETM_OPT_CTXTID and ETM_OPT_TS, it wrongly takes these two values (14 and 28 prespectively) as bit masks, but actually both are the offset for bits. But this doesn't lead to further failure due to the AND logic operation will be always true for ETM_OPT_CTXTID / ETM_OPT_TS.

This patch defines new independent macros (rather than using the "config" bits) for requesting the "contextid" and "timestamp" for cs_etm_set_option().

[leoy: Extract the change as a separate patch for easier review] Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com Signed-off-by: Leo Yan leo.yan@linaro.org

---

tools/perf/arch/arm/util/cs-etm.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index bd446aba64f7..c25c878fd06c 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -156,6 +156,10 @@ static int cs_etm_set_timestamp(struct auxtrace_record *itr, return err; }

+#define ETM_SET_OPT_CTXTID (1 << 0) +#define ETM_SET_OPT_TS (1 << 1) +#define ETM_SET_OPT_MASK (ETM_SET_OPT_CTXTID | ETM_SET_OPT_TS)

static int cs_etm_set_option(struct auxtrace_record *itr, struct evsel *evsel, u32 option) { @@ -169,17 +173,17 @@ static int cs_etm_set_option(struct auxtrace_record *itr, !cpu_map__has(online_cpus, i)) continue;

•           if (option & ETM_OPT_CTXTID) {
  

•           if (option & ETM_SET_OPT_CTXTID) {
                    err = cs_etm_set_context_id(itr, evsel, i);
                    if (err)
                            goto out;
            }
  

•           if (option & ETM_OPT_TS) {
  

•           if (option & ETM_SET_OPT_TS) {
                    err = cs_etm_set_timestamp(itr, evsel, i);
                    if (err)
                            goto out;
            }
  

•           if (option & ~(ETM_OPT_CTXTID | ETM_OPT_TS))
  

•           if (option & ~(ETM_SET_OPT_MASK))
                    /* Nothing else is currently supported */
                    goto out;
    }
  

@@ -406,7 +410,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr, evsel__set_sample_bit(cs_etm_evsel, CPU);

            err = cs_etm_set_option(itr, cs_etm_evsel,

•                                   ETM_OPT_CTXTID | ETM_OPT_TS);
  

•                                   ETM_SET_OPT_CTXTID | ETM_SET_OPT_TS);
            if (err)
                    goto out;
    }
  

-- 2.25.1

Reivewed-by: Mike Leach mike.leach@linaro.org

On Tue, 2 Feb 2021 at 16:39, Leo Yan leo.yan@linaro.org wrote:

From: Suzuki K Poulose suzuki.poulose@arm.com

If the kernel is running at EL2, the pid of a task is exposed via VMID instead of the CONTEXTID. Add support for this in the perf tool.

This patch respects user setting if user has specified any configs from "contextid", "contextid1" or "contextid2"; otherwise, it dynamically sets config based on PMU format "contextid".

Cc: Mike Leach mike.leach@linaro.org Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Al Grant al.grant@arm.com Co-developed-by: Leo Yan leo.yan@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com Signed-off-by: Leo Yan leo.yan@linaro.org

---

tools/include/linux/coresight-pmu.h | 3 ++ tools/perf/arch/arm/util/cs-etm.c | 61 +++++++++++++++++++++++------ 2 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/tools/include/linux/coresight-pmu.h b/tools/include/linux/coresight-pmu.h index 5dc47cfdcf07..4ac5c081af93 100644 --- a/tools/include/linux/coresight-pmu.h +++ b/tools/include/linux/coresight-pmu.h @@ -20,14 +20,17 @@ */ #define ETM_OPT_CYCACC 12 #define ETM_OPT_CTXTID 14 +#define ETM_OPT_CTXTID2 15 #define ETM_OPT_TS 28 #define ETM_OPT_RETSTK 29

/* ETMv4 CONFIGR programming bits for the ETM OPTs */ #define ETM4_CFG_BIT_CYCACC 4 #define ETM4_CFG_BIT_CTXTID 6 +#define ETM4_CFG_BIT_VMID 7 #define ETM4_CFG_BIT_TS 11 #define ETM4_CFG_BIT_RETSTK 12 +#define ETM4_CFG_BIT_VMID_OPT 15

static inline int coresight_get_trace_id(int cpu) { diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index c25c878fd06c..fa6f91a7c8a1 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -67,6 +67,7 @@ static int cs_etm_set_context_id(struct auxtrace_record *itr, char path[PATH_MAX]; int err = -EINVAL; u32 val;

•   u64 contextid;
  
    ptr = container_of(itr, struct cs_etm_recording, itr);
    cs_etm_pmu = ptr->cs_etm_pmu;
  

@@ -86,25 +87,59 @@ static int cs_etm_set_context_id(struct auxtrace_record *itr, goto out; }

•   /* User has configured for PID tracing, respects it. */
  

•   contextid = evsel->core.attr.config &
  

•                   (BIT(ETM_OPT_CTXTID) | BIT(ETM_OPT_CTXTID2));
  

•   /*
  

•    * TRCIDR2.CIDSIZE, bit [9-5], indicates whether contextID tracing
  

•    * is supported:
  

•    *  0b00000 Context ID tracing is not supported.
  

•    *  0b00100 Maximum of 32-bit Context ID size.
  

•    *  All other values are reserved.
  

•    * If user doesn't configure the contextid format, parse PMU format and
  

•    * enable PID tracing according to the "contextid" format bits:
  

•    *
  

•    *   If bit ETM_OPT_CTXTID is set, trace CONTEXTIDR_EL1;
  

•    *   If bit ETM_OPT_CTXTID2 is set, trace CONTEXTIDR_EL2.
     */
  

•   val = BMVAL(val, 5, 9);
  

•   if (!val || val != 0x4) {
  

•           err = -EINVAL;
  

•           goto out;
  

•   if (!contextid)
  

•           contextid = perf_pmu__format_bits(&cs_etm_pmu->format,
  

•                                             "contextid");
  

•   if (contextid & BIT(ETM_OPT_CTXTID)) {
  

•           /*
  

•            * TRCIDR2.CIDSIZE, bit [9-5], indicates whether contextID
  

•            * tracing is supported:
  

•            *  0b00000 Context ID tracing is not supported.
  

•            *  0b00100 Maximum of 32-bit Context ID size.
  

•            *  All other values are reserved.
  

•            */
  

•           val = BMVAL(val, 5, 9);
  

•           if (!val || val != 0x4) {
  

•                   pr_err("%s: CONTEXTIDR_EL1 isn't supported\n",
  

•                          CORESIGHT_ETM_PMU_NAME);
  

•                   err = -EINVAL;
  

•                   goto out;
  

•           }
  

•   }
  

•   if (contextid & BIT(ETM_OPT_CTXTID2)) {
  

•           /*
  

•            * TRCIDR2.VMIDOPT[30:29] != 0 and
  

•            * TRCIDR2.VMIDSIZE[14:10] == 0b00100 (32bit virtual contextid)
  

•            * We can't support CONTEXTIDR in VMID if the size of the
  

•            * virtual context id is < 32bit.
  

•            * Any value of VMIDSIZE >= 4 (i.e, > 32bit) is fine for us.
  

•            */
  

•           if (!BMVAL(val, 29, 30) || BMVAL(val, 10, 14) < 4) {
  

•                   pr_err("%s: CONTEXTIDR_EL2 isn't supported\n",
  

•                          CORESIGHT_ETM_PMU_NAME);
  

•                   err = -EINVAL;
  

•                   goto out;
  

•           }
    }
  
    /* All good, let the kernel know */
  

•   evsel->core.attr.config |= (1 << ETM_OPT_CTXTID);
  

•   evsel->core.attr.config |= contextid;
    err = 0;
  
  

out:

•   return err;
  

}

@@ -489,7 +524,9 @@ static u64 cs_etmv4_get_config(struct auxtrace_record *itr) config |= BIT(ETM4_CFG_BIT_TS); if (config_opts & BIT(ETM_OPT_RETSTK)) config |= BIT(ETM4_CFG_BIT_RETSTK);

•   if (config_opts & BIT(ETM_OPT_CTXTID2))
  

•           config |= BIT(ETM4_CFG_BIT_VMID) |
  

•                     BIT(ETM4_CFG_BIT_VMID_OPT);
    return config;
  

}

-- 2.25.1

reviewed-by: Mike Leach mike.leach@linaro.org

-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

On 2/2/21 4:38 PM, Leo Yan wrote:

This patch adds helper function cs_etm__get_pid_fmt(), by passing parameter "traceID", it returns the PID format.

Signed-off-by: Leo Yan leo.yan@linaro.org

tools/perf/util/cs-etm.c | 43 ++++++++++++++++++++++++++++++++++++++++ tools/perf/util/cs-etm.h | 1 + 2 files changed, 44 insertions(+)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index a2a369e2fbb6..8194ddbd01e5 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -7,6 +7,7 @@ */ #include <linux/bitops.h> +#include <linux/coresight-pmu.h> #include <linux/err.h> #include <linux/kernel.h> #include <linux/log2.h> @@ -156,6 +157,48 @@ int cs_etm__get_cpu(u8 trace_chan_id, int *cpu) return 0; } +/*

• • The returned PID format is presented by two bits:
• • Bit ETM_OPT_CTXTID: CONTEXTIDR or CONTEXTIDR_EL1 is traced;
• • Bit ETM_OPT_CTXTID2: CONTEXTIDR_EL2 is traced.
• • It's possible that these two bits are set together, this means the tracing
• • contains PIDs for both CONTEXTIDR_EL1 and CONTEXTIDR_EL2.

This is a bit confusing. If both the bits are set, the session was run on an EL2 kernel. Thus, the PID is always in CONTEXTIDR_EL2.

• */

+int cs_etm__get_pid_fmt(u8 trace_chan_id, u64 *pid_fmt) +{

• struct int_node *inode;
• u64 *metadata, val;
• inode = intlist__find(traceid_list, trace_chan_id);
• if (!inode)

• return -EINVAL;
  

• metadata = inode->priv;
• if (metadata[CS_ETM_MAGIC] == __perf_cs_etmv3_magic) {

• val = metadata[CS_ETM_ETMCR];
  

• /* CONTEXTIDR is traced */
  

• if (val & BIT(ETM_OPT_CTXTID))
  

• 	*pid_fmt = BIT(ETM_OPT_CTXTID);
  

• } else {

• val = metadata[CS_ETMV4_TRCCONFIGR];
  

• *pid_fmt = 0;
  

• /* CONTEXTIDR_EL2 is traced */
  

• if (val & (BIT(ETM4_CFG_BIT_VMID) | BIT(ETM4_CFG_BIT_VMID_OPT)))
  

• 	*pid_fmt = BIT(ETM_OPT_CTXTID2);
  

• /* CONTEXTIDR_EL1 is traced */
  

• if (val & BIT(ETM4_CFG_BIT_CTXTID))
  

I haven't looked at how this gets used. But, Shouldn't this be :

else if (val & BIT(ETM4_CFG_BIT_CTXTID)) ?

Suzuki

On 2/4/21 3:47 AM, Leo Yan wrote:

On Tue, Feb 02, 2021 at 11:19:22PM +0000, Suzuki Kuruppassery Poulose wrote:

...

On 2/2/21 4:38 PM, Leo Yan wrote:

...

This patch adds helper function cs_etm__get_pid_fmt(), by passing parameter "traceID", it returns the PID format.

Signed-off-by: Leo Yan leo.yan@linaro.org

tools/perf/util/cs-etm.c | 43 ++++++++++++++++++++++++++++++++++++++++ tools/perf/util/cs-etm.h | 1 + 2 files changed, 44 insertions(+)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index a2a369e2fbb6..8194ddbd01e5 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -7,6 +7,7 @@ */ #include <linux/bitops.h> +#include <linux/coresight-pmu.h> #include <linux/err.h> #include <linux/kernel.h> #include <linux/log2.h> @@ -156,6 +157,48 @@ int cs_etm__get_cpu(u8 trace_chan_id, int *cpu) return 0; } +/*

• • The returned PID format is presented by two bits:
• • Bit ETM_OPT_CTXTID: CONTEXTIDR or CONTEXTIDR_EL1 is traced;
• • Bit ETM_OPT_CTXTID2: CONTEXTIDR_EL2 is traced.
• • It's possible that these two bits are set together, this means the tracing
• • contains PIDs for both CONTEXTIDR_EL1 and CONTEXTIDR_EL2.

This is a bit confusing. If both the bits are set, the session was run on an EL2 kernel. Thus, the PID is always in CONTEXTIDR_EL2.

Sorry for confusion. I'd like to rephrase as:

It's possible that the two bits ETM_OPT_CTXTID and ETM_OPT_CTXTID2 are enabled at the same time when the session runs on an EL2 kernel. This means the CONTEXTIDR_EL1 and CONTEXTIDR_EL2 both will be recorded in the trace data, the tool will selectively use CONTEXTIDR_EL2 as PID.

...

...

• */

+int cs_etm__get_pid_fmt(u8 trace_chan_id, u64 *pid_fmt) +{

• struct int_node *inode;
• u64 *metadata, val;
• inode = intlist__find(traceid_list, trace_chan_id);
• if (!inode)

• return -EINVAL;
  

• metadata = inode->priv;
• if (metadata[CS_ETM_MAGIC] == __perf_cs_etmv3_magic) {

• val = metadata[CS_ETM_ETMCR];
  

• /* CONTEXTIDR is traced */
  

• if (val & BIT(ETM_OPT_CTXTID))
  

• 	*pid_fmt = BIT(ETM_OPT_CTXTID);
  

• } else {

• val = metadata[CS_ETMV4_TRCCONFIGR];
  

• *pid_fmt = 0;
  

• /* CONTEXTIDR_EL2 is traced */
  

• if (val & (BIT(ETM4_CFG_BIT_VMID) | BIT(ETM4_CFG_BIT_VMID_OPT)))
  

• 	*pid_fmt = BIT(ETM_OPT_CTXTID2);
  

• /* CONTEXTIDR_EL1 is traced */
  

• if (val & BIT(ETM4_CFG_BIT_CTXTID))
  

I haven't looked at how this gets used. But, Shouldn't this be :

else if (val & BIT(ETM4_CFG_BIT_CTXTID)) ?

Actually it's deliberately to set both bits ETM_OPT_CTXTID2 and ETM_OPT_CTXTID if user has enable configs "contextid1" and "contextid2". So this is exactly the reversed flow in the function cs_etmv4_get_config().

The point is, we don't care if the user selected both options. What we care is, where can we find the PID. CONTEXTIDR_EL1 or CONTEXTIDR_EL2. As such, get_pid_fmt simply should make that decision and pass it on. So, if the CONTEXTIDR_EL2 is selected (which can only be done successfully on an EL2 kernel), thats our pid.

So we should return the format for the PID here. i.e ETM_OPT_CTXTID2 OR ETM_OPT_CTXTID. But not both.

Cheers Suzuki

On Tue, Feb 02, 2021 at 11:29:47PM +0000, Suzuki Kuruppassery Poulose wrote:

On 2/2/21 4:38 PM, Leo Yan wrote:

...

From: Suzuki K Poulose suzuki.poulose@arm.com

The PID of the task could be traced as VMID when the kernel is running at EL2. Teach the decoder to look for VMID when the CONTEXTIDR (Arm32) or CONTEXTIDR_EL1 (Arm64) is invalid but we have a valid VMID.

Cc: Mike Leach mike.leach@linaro.org Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Al Grant al.grant@arm.com Co-developed-by: Leo Yan leo.yan@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com Signed-off-by: Leo Yan leo.yan@linaro.org

---

.../perf/util/cs-etm-decoder/cs-etm-decoder.c | 32 ++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c index 3f4bc4050477..fb2a163ff74e 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -6,6 +6,7 @@

• Author: Mathieu Poirier mathieu.poirier@linaro.org

*/ +#include <linux/coresight-pmu.h> #include <linux/err.h> #include <linux/list.h> #include <linux/zalloc.h> @@ -491,13 +492,36 @@ cs_etm_decoder__set_tid(struct cs_etm_queue *etmq, const ocsd_generic_trace_elem *elem, const uint8_t trace_chan_id) {

• pid_t tid;

• pid_t tid = -1;
• u64 pid_fmt;
• int ret;

• /* Ignore PE_CONTEXT packets that don't have a valid contextID */
• if (!elem->context.ctxt_id_valid)

• ret = cs_etm__get_pid_fmt(trace_chan_id, &pid_fmt);
• if (ret)

Is this something we can cache in this function ? e.g, static u64 pid_fmt;

if (!pid_pfmt) ret = cs_etm__get_pid_fmt(trace_chan_id, &pid_fmt);

As all the ETMs will be running at the same exception level.

Sorry that I let you repeated your comments again.

To be honest, I considered this after read your comment in the previous series, but I thought it's possible that multiple CPUs have different PID format, especially for big.LITTLE arch. After read your suggestion again, I think my concern is not valid, even for big.LITTLE, all CPUs should run on the same kernel exception level.

So will follow up your suggestion to cache "pid_fmt".

...

• return OCSD_RESP_FATAL_SYS_ERR;
  

• /*

• * Process the PE_CONTEXT packets if we have a valid contextID or VMID.
  

• * If the kernel is running at EL2, the PID is traced in CONTEXTIDR_EL2
  

• * as VMID, Bit ETM_OPT_CTXTID2 is set in this case.
  

• */
  

• switch (pid_fmt) {
• case BIT(ETM_OPT_CTXTID):

• if (elem->context.ctxt_id_valid)
  

• 	tid = elem->context.context_id;
  

• break;
  

• case BIT(ETM_OPT_CTXTID2) | BIT(ETM_OPT_CTXTID):

I would rather fix the cs_etm__get_pid_fmt() to return either of these as commented. i.e, ETM_OPT_CTXTID or ETM_OPT_CTXTID2. Thus we don't need the this case.

I explained why I set both bits for ETM_OPT_CTXTID and ETM_OPT_CTXTID2 in the patch 05/07. Could you take a look for it?

With the above two addressed:

Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com

Thanks, Leo

On Tue, Feb 02, 2021 at 11:24:48PM +0000, Suzuki Kuruppassery Poulose wrote:

On 2/2/21 4:38 PM, Leo Yan wrote:

...

After support the PID tracing for the kernel in EL1 or EL2, the usage gets more complicated.

This patch gives description for the PMU formats of contextID configs, this can help users to understand how to control the knobs for PID tracing when the kernel is in different ELs.

Signed-off-by: Leo Yan leo.yan@linaro.org

Documentation/trace/coresight/coresight.rst | 37 +++++++++++++++++++++ 1 file changed, 37 insertions(+)

diff --git a/Documentation/trace/coresight/coresight.rst b/Documentation/trace/coresight/coresight.rst index 0b73acb44efa..771558f22938 100644 --- a/Documentation/trace/coresight/coresight.rst +++ b/Documentation/trace/coresight/coresight.rst @@ -512,6 +512,43 @@ The --itrace option controls the type and frequency of synthesized events Note that only 64-bit programs are currently supported - further work is required to support instruction decode of 32-bit Arm programs. +2.2) Tracing PID

+When the kernel is running at EL2 with Virtualization Host Extensions (VHE), +perf records CONTEXTIDR_EL2 in the trace data and can be used as PID when +decoding; and if the kernel is running at EL1 with nVHE, CONTEXTIDR_EL1 is +traced for PID.

+To support tracing PID for the kernel runs at different exception levels, +the PMU formats are defined as follow:

• "contextid1": Available on both EL1 kernel and EL2 kernel. When the

•            kernel is running at EL1, "contextid1" enables the PID
  

•            tracing; when the kernel is running at EL2, this enables
  

•            tracing the PID of guest applications.
  

• "contextid2": Only usable when the kernel is running at EL2. When

•            selected, enables PID tracing on EL2 kernel.
  

• "contextid": Will be an alias for the option that enables PID

•            tracing.  I.e,
  

•            contextid == contextid1, on EL1 kernel.
  

•            contextid == contextid2, on EL2 kernel.
  

+The perf tool automatically sets corresponding bit for the "contextid" config, +therefore, the user doesn't have to bother which EL the kernel is running.

• i.e, perf record -e cs_etm/contextid/u -- uname
• or perf record -e cs_etm//u -- uname

+will always do the "PID" tracing, independent of the kernel EL.

+When the kernel is running at EL2 with VHE, if user wants to trace both the +PIDs for both host and guest, the two configs "contextid1" and "contextid2" +can be set at the same time:

• perf record -e cs_etm/contextid1,contextid2/u -- uname

To make this case clear, we could change the command from uname to something like:

perf record -e cs_etm/contextid1,contextid2/u -- vm

Exactly, will refine.

Otherwise looks good to me.

With the above fixed,

Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com

Thanks, Leo