2021-01-09 07:46:45

by Leo Yan

[permalink] [raw]
Subject: [PATCH v1 2/7] perf cs_etm: Use pid tracing explicitly instead of contextid

From: Suzuki K Poulose <[email protected]>

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

By default the perf tool requests contextid and timestamp for
task bound events. Instead of hard coding contextid, switch
to "pid" config exposed by the kernel. While at it, define new
independent macros (rather than using the "config" bits) for
requesting the "pid" and "timestamp" for cs_etm_set_option(),
since the PID config is now dynamic depending on the kernel
exception level.

Cc: Mike Leach <[email protected]>
Cc: Mathieu Poirier <[email protected]>
Cc: Al Grant <[email protected]>
Signed-off-by: Suzuki K Poulose <[email protected]>
Signed-off-by: Leo Yan <[email protected]>
---
tools/include/linux/coresight-pmu.h | 11 +++--
tools/perf/arch/arm/util/cs-etm.c | 68 ++++++++++++++++++++++-------
2 files changed, 59 insertions(+), 20 deletions(-)

diff --git a/tools/include/linux/coresight-pmu.h b/tools/include/linux/coresight-pmu.h
index b0e35eec6499..927c6285ce5d 100644
--- a/tools/include/linux/coresight-pmu.h
+++ b/tools/include/linux/coresight-pmu.h
@@ -11,16 +11,19 @@
#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
+#define ETM_OPT_CYCACC 12
+#define ETM_OPT_CTXTID 14
+#define ETM_OPT_CTXTID_IN_VMID 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 cad7bf783413..fad7b6e13ccc 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -59,14 +59,15 @@ static const char *metadata_etmv4_ro[CS_ETMV4_PRIV_MAX] = {

static bool cs_etm_is_etmv4(struct auxtrace_record *itr, int cpu);

-static int cs_etm_set_context_id(struct auxtrace_record *itr,
- struct evsel *evsel, int cpu)
+static int cs_etm_set_pid(struct auxtrace_record *itr,
+ struct evsel *evsel, int cpu)
{
struct cs_etm_recording *ptr;
struct perf_pmu *cs_etm_pmu;
char path[PATH_MAX];
int err = -EINVAL;
u32 val;
+ u64 pid_fmt;

ptr = container_of(itr, struct cs_etm_recording, itr);
cs_etm_pmu = ptr->cs_etm_pmu;
@@ -86,21 +87,50 @@ static int cs_etm_set_context_id(struct auxtrace_record *itr,
goto out;
}

+ pid_fmt = perf_pmu__format_bits(&cs_etm_pmu->format, "pid");
/*
- * 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 the kernel doesn't support the "pid" format (older kernel),
+ * fall back to using the CTXTID.
*/
- val = BMVAL(val, 5, 9);
- if (!val || val != 0x4) {
+ if (!pid_fmt)
+ pid_fmt = 1ULL << ETM_OPT_CTXTID;
+
+ switch (pid_fmt) {
+ case (1ULL << 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) {
+ err = -EINVAL;
+ goto out;
+ }
+ break;
+ case (1ULL << ETM_OPT_CTXTID_IN_VMID):
+ /*
+ * 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) {
+ err = -EINVAL;
+ goto out;
+ }
+ break;
+ default:
err = -EINVAL;
goto out;
}

+
/* All good, let the kernel know */
- evsel->core.attr.config |= (1 << ETM_OPT_CTXTID);
+ evsel->core.attr.config |= pid_fmt;
err = 0;

out:
@@ -156,6 +186,10 @@ static int cs_etm_set_timestamp(struct auxtrace_record *itr,
return err;
}

+#define ETM_SET_OPT_PID (1 << 0)
+#define ETM_SET_OPT_TS (1 << 1)
+#define ETM_SET_OPT_MASK (ETM_SET_OPT_PID | ETM_SET_OPT_TS)
+
static int cs_etm_set_option(struct auxtrace_record *itr,
struct evsel *evsel, u32 option)
{
@@ -169,17 +203,17 @@ static int cs_etm_set_option(struct auxtrace_record *itr,
!cpu_map__has(online_cpus, i))
continue;

- if (option & ETM_OPT_CTXTID) {
- err = cs_etm_set_context_id(itr, evsel, i);
+ if (option & ETM_SET_OPT_PID) {
+ err = cs_etm_set_pid(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 +440,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_PID | ETM_SET_OPT_TS);
if (err)
goto out;
}
@@ -485,7 +519,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_CTXTID_IN_VMID))
+ config |= BIT(ETM4_CFG_BIT_VMID) |
+ BIT(ETM4_CFG_BIT_VMID_OPT);
return config;
}

--
2.25.1


2021-01-15 22:47:24

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH v1 2/7] perf cs_etm: Use pid tracing explicitly instead of contextid

On Sat, Jan 09, 2021 at 03:44:30PM +0800, Leo Yan wrote:
> From: Suzuki K Poulose <[email protected]>
>
> If the kernel is running at EL2, the pid of the task is exposed
> via VMID instead of the CONTEXTID. Add support for this in the
> perf tool.
>
> By default the perf tool requests contextid and timestamp for
> task bound events. Instead of hard coding contextid, switch
> to "pid" config exposed by the kernel. While at it, define new
> independent macros (rather than using the "config" bits) for
> requesting the "pid" and "timestamp" for cs_etm_set_option(),
> since the PID config is now dynamic depending on the kernel
> exception level.
>
> Cc: Mike Leach <[email protected]>
> Cc: Mathieu Poirier <[email protected]>
> Cc: Al Grant <[email protected]>
> Signed-off-by: Suzuki K Poulose <[email protected]>
> Signed-off-by: Leo Yan <[email protected]>
> ---
> tools/include/linux/coresight-pmu.h | 11 +++--
> tools/perf/arch/arm/util/cs-etm.c | 68 ++++++++++++++++++++++-------
> 2 files changed, 59 insertions(+), 20 deletions(-)
>
> diff --git a/tools/include/linux/coresight-pmu.h b/tools/include/linux/coresight-pmu.h
> index b0e35eec6499..927c6285ce5d 100644
> --- a/tools/include/linux/coresight-pmu.h
> +++ b/tools/include/linux/coresight-pmu.h
> @@ -11,16 +11,19 @@
> #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
> +#define ETM_OPT_CYCACC 12
> +#define ETM_OPT_CTXTID 14
> +#define ETM_OPT_CTXTID_IN_VMID 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

Wasn't this done in the previous patch?

>
> 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 cad7bf783413..fad7b6e13ccc 100644
> --- a/tools/perf/arch/arm/util/cs-etm.c
> +++ b/tools/perf/arch/arm/util/cs-etm.c
> @@ -59,14 +59,15 @@ static const char *metadata_etmv4_ro[CS_ETMV4_PRIV_MAX] = {
>
> static bool cs_etm_is_etmv4(struct auxtrace_record *itr, int cpu);
>
> -static int cs_etm_set_context_id(struct auxtrace_record *itr,
> - struct evsel *evsel, int cpu)
> +static int cs_etm_set_pid(struct auxtrace_record *itr,
> + struct evsel *evsel, int cpu)
> {
> struct cs_etm_recording *ptr;
> struct perf_pmu *cs_etm_pmu;
> char path[PATH_MAX];
> int err = -EINVAL;
> u32 val;
> + u64 pid_fmt;
>
> ptr = container_of(itr, struct cs_etm_recording, itr);
> cs_etm_pmu = ptr->cs_etm_pmu;
> @@ -86,21 +87,50 @@ static int cs_etm_set_context_id(struct auxtrace_record *itr,
> goto out;
> }
>
> + pid_fmt = perf_pmu__format_bits(&cs_etm_pmu->format, "pid");

If we go with my suggestion from patch 1 this would become "contextid".

I think what follows below is all good.

> /*
> - * 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 the kernel doesn't support the "pid" format (older kernel),
> + * fall back to using the CTXTID.
> */
> - val = BMVAL(val, 5, 9);
> - if (!val || val != 0x4) {
> + if (!pid_fmt)
> + pid_fmt = 1ULL << ETM_OPT_CTXTID;
> +
> + switch (pid_fmt) {
> + case (1ULL << 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) {
> + err = -EINVAL;
> + goto out;
> + }
> + break;
> + case (1ULL << ETM_OPT_CTXTID_IN_VMID):
> + /*
> + * 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) {
> + err = -EINVAL;
> + goto out;
> + }
> + break;
> + default:
> err = -EINVAL;
> goto out;
> }
>
> +
> /* All good, let the kernel know */
> - evsel->core.attr.config |= (1 << ETM_OPT_CTXTID);
> + evsel->core.attr.config |= pid_fmt;
> err = 0;
>
> out:
> @@ -156,6 +186,10 @@ static int cs_etm_set_timestamp(struct auxtrace_record *itr,
> return err;
> }
>
> +#define ETM_SET_OPT_PID (1 << 0)
> +#define ETM_SET_OPT_TS (1 << 1)
> +#define ETM_SET_OPT_MASK (ETM_SET_OPT_PID | ETM_SET_OPT_TS)
> +

I don't think we need this as part of the current patchset. Back in the days I
thought it would be a good idea to use the ETMv3.5/PTM configuration bits but in
hindsight it wasn't. That's what happens when your crystal ball isn't working.

That being said we can start treating the perf_event_attr::config bits
arbitrarily. Someone pointed out that it might get confusing but I don't think
it is anything that can't be overcomed with good in-line documentation.

> static int cs_etm_set_option(struct auxtrace_record *itr,
> struct evsel *evsel, u32 option)
> {
> @@ -169,17 +203,17 @@ static int cs_etm_set_option(struct auxtrace_record *itr,
> !cpu_map__has(online_cpus, i))
> continue;
>
> - if (option & ETM_OPT_CTXTID) {
> - err = cs_etm_set_context_id(itr, evsel, i);
> + if (option & ETM_SET_OPT_PID) {
> + err = cs_etm_set_pid(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 +440,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_PID | ETM_SET_OPT_TS);
> if (err)
> goto out;
> }
> @@ -485,7 +519,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_CTXTID_IN_VMID))
> + config |= BIT(ETM4_CFG_BIT_VMID) |
> + BIT(ETM4_CFG_BIT_VMID_OPT);
> return config;
> }
>
> --
> 2.25.1
>

2021-01-19 05:44:28

by Leo Yan

[permalink] [raw]
Subject: Re: [PATCH v1 2/7] perf cs_etm: Use pid tracing explicitly instead of contextid

Hi Mathieu,

On Fri, Jan 15, 2021 at 03:44:16PM -0700, Mathieu Poirier wrote:
> On Sat, Jan 09, 2021 at 03:44:30PM +0800, Leo Yan wrote:
> > From: Suzuki K Poulose <[email protected]>
> >
> > If the kernel is running at EL2, the pid of the task is exposed
> > via VMID instead of the CONTEXTID. Add support for this in the
> > perf tool.
> >
> > By default the perf tool requests contextid and timestamp for
> > task bound events. Instead of hard coding contextid, switch
> > to "pid" config exposed by the kernel. While at it, define new
> > independent macros (rather than using the "config" bits) for
> > requesting the "pid" and "timestamp" for cs_etm_set_option(),
> > since the PID config is now dynamic depending on the kernel
> > exception level.
> >
> > Cc: Mike Leach <[email protected]>
> > Cc: Mathieu Poirier <[email protected]>
> > Cc: Al Grant <[email protected]>
> > Signed-off-by: Suzuki K Poulose <[email protected]>
> > Signed-off-by: Leo Yan <[email protected]>
> > ---
> > tools/include/linux/coresight-pmu.h | 11 +++--
> > tools/perf/arch/arm/util/cs-etm.c | 68 ++++++++++++++++++++++-------
> > 2 files changed, 59 insertions(+), 20 deletions(-)
> >
> > diff --git a/tools/include/linux/coresight-pmu.h b/tools/include/linux/coresight-pmu.h
> > index b0e35eec6499..927c6285ce5d 100644
> > --- a/tools/include/linux/coresight-pmu.h
> > +++ b/tools/include/linux/coresight-pmu.h
> > @@ -11,16 +11,19 @@
> > #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
> > +#define ETM_OPT_CYCACC 12
> > +#define ETM_OPT_CTXTID 14
> > +#define ETM_OPT_CTXTID_IN_VMID 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
>
> Wasn't this done in the previous patch?

In the previous patch, these bits are defined in the kernel's header
include/linux/coresight-pmu.h; at here it defines the same bits in
tool's header.

To be honest, I struggled to understand your suggestions, finally I
think it's feasbile and we can simplify the implementation. I will
try to refine the patch series for the review.

Appreciate for good suggestions!

Leo