From: Leo Yan <leo.yan@linaro.org>
To: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: al.grant@arm.com, mathieu.poirier@linaro.org,
anshuman.khandual@arm.com, coresight@lists.linaro.org,
linux-arm-kernel@lists.infradead.org, mike.leach@linaro.org
Subject: Re: [PATCH 2/3] perf: cs_etm: Use pid tracing explicitly instead of contextid
Date: Thu, 12 Nov 2020 18:00:10 +0800 [thread overview]
Message-ID: <20201112100010.GB17274@leoy-ThinkPad-X240s> (raw)
In-Reply-To: <20201110183313.1823760-3-suzuki.poulose@arm.com>
On Tue, Nov 10, 2020 at 06:33:12PM +0000, Suzuki Kuruppassery Poulose wrote:
> 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.
>
> Cc: Leo Yan <leo.yan@linaro.org>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Al Grant <al.grant@arm.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> tools/include/linux/coresight-pmu.h | 11 +++--
> tools/perf/arch/arm/util/cs-etm.c | 65 +++++++++++++++++++++--------
> 2 files changed, 54 insertions(+), 22 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..e6207ce7cc85 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,43 @@ static int cs_etm_set_context_id(struct auxtrace_record *itr,
> goto out;
> }
>
> - /*
> - * 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) {
> + pid_fmt = perf_pmu__format_bits(&cs_etm_pmu->format, "pid");
> + if (!pid_fmt)
> + pid_fmt = 1ULL << ETM_OPT_CTXTID;
If doesn't find "pid" format bits, should it mean the kernel is
running an old version so doesn't support "pid" entry? It's good to
add comment for this.
> +
> + 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 context id size)
> + */
> + if (!BMVAL(val, 29, 30) || BMVAL(val, 10, 14) < 4) {
The comment is not alignment with the code. Based on the comment, the
code should be:
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 +179,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 +196,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);
I don't understand what's the reason for introducing the new macros
"ETM_SET_OPT_XXX", seems to me the old macros still can be used at
here. Could you help explian for this?
Thanks,
Leo
> 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 +433,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 +512,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.24.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-11-12 10:01 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-10 18:33 [PATCH 0/3] coresight: etm-perf: Fix pid tracing with VHE Suzuki K Poulose
2020-11-10 18:33 ` [PATCH 1/3] coresight: etm-perf: Add support for PID tracing for kernel at EL2 Suzuki K Poulose
2020-11-12 10:27 ` Leo Yan
2020-11-12 12:41 ` Suzuki K Poulose
2020-11-10 18:33 ` [PATCH 2/3] perf: cs_etm: Use pid tracing explicitly instead of contextid Suzuki K Poulose
2020-11-12 10:00 ` Leo Yan [this message]
2020-11-12 10:54 ` Suzuki K Poulose
2020-11-12 12:24 ` Leo Yan
2020-11-10 18:33 ` [PATCH 3/3] rfc: perf: cs_etm: Detect pid in VMID for kernel running at EL2 Suzuki K Poulose
2020-11-10 22:57 ` Suzuki K Poulose
2020-11-11 11:03 ` Al Grant
2020-11-11 11:40 ` Suzuki K Poulose
2020-11-13 0:11 ` Leo Yan
2020-11-13 9:47 ` Suzuki K Poulose
2020-11-13 10:42 ` Leo Yan
2020-11-16 9:46 ` Leo Yan
2020-12-18 10:46 ` Daniel Kiss
[not found] ` <CADDJ8CVqz8Gdkx42H+TjdBOS-Vjk4MAVV7PhAaeBNq9Ejh=usA@mail.gmail.com>
2020-12-23 8:05 ` Leo Yan
2021-01-04 17:33 ` Suzuki K Poulose
2021-01-04 18:06 ` Mathieu Poirier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201112100010.GB17274@leoy-ThinkPad-X240s \
--to=leo.yan@linaro.org \
--cc=al.grant@arm.com \
--cc=anshuman.khandual@arm.com \
--cc=coresight@lists.linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=mathieu.poirier@linaro.org \
--cc=mike.leach@linaro.org \
--cc=suzuki.poulose@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox