[PATCH] coresight: etm-perf: Add support for PID tracing for kernel at EL2

[PATCH] coresight: etm-perf: Add support for PID tracing for kernel at EL2

[Suzuki K Poulose]

When the kernel is running at EL2, the PID is stored in CONTEXTIDR_EL2.
So, tracing CONTEXTIDR_EL1 doesn't give us the pid of the process.
Thus we should trace the VMID with VMIDOPT set to trace
CONTEXTIDR_EL2 instead of VMID. Given that we have an existing
config option "contextid" and this will be useful for tracing
virtual machines (when we get to support virtualization). So instead,
this patch adds a new option, contextid_in_vmid as a separate config.
Thus on an EL2 kernel, we will have two options available for
the perf tool. However, to make it easier for the user to
do pid tracing, we add a new format which will default to
"contextid" (on EL1 kernel) or "contextid_in_vmid" (on EL2
kernel). So that the user doesn't have to bother which EL the
kernel is running.

 i.e, perf record -e cs_etm/pid/u --

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

Also, the perf tool will be updated to automatically select
"pid" config instead of the "contextid" for system wide/CPU wide
mode.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Al Grant <al.grant@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: Leo Yan <leo.yan@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
Changes since previous version:
  - Fix build break on 32bit kernel (kernel test robot <lkp@intel.com>)
---
 .../hwtracing/coresight/coresight-etm-perf.c   | 18 ++++++++++++++++++
 .../hwtracing/coresight/coresight-etm4x-core.c |  9 +++++++++
 include/linux/coresight-pmu.h                  | 11 +++++++----
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index bdc34ca449f7..f13c3a7bbc84 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -30,14 +30,32 @@ static DEFINE_PER_CPU(struct coresight_device *, csdev_src);
 /* ETMv3.5/PTM's ETMCR is 'config' */
 PMU_FORMAT_ATTR(cycacc,		"config:" __stringify(ETM_OPT_CYCACC));
 PMU_FORMAT_ATTR(contextid,	"config:" __stringify(ETM_OPT_CTXTID));
+PMU_FORMAT_ATTR(contextid_in_vmid,	"config:" __stringify(ETM_OPT_CTXTID_IN_VMID));
 PMU_FORMAT_ATTR(timestamp,	"config:" __stringify(ETM_OPT_TS));
 PMU_FORMAT_ATTR(retstack,	"config:" __stringify(ETM_OPT_RETSTK));
 /* Sink ID - same for all ETMs */
 PMU_FORMAT_ATTR(sinkid,		"config2:0-31");
 
+static ssize_t format_attr_pid_show(struct device *dev,
+				    struct device_attribute *attr,
+				    char *page)
+{
+	int pid_fmt = ETM_OPT_CTXTID;
+
+#ifdef CONFIG_CORESIGHT_SOURCE_ETM4X
+	if (is_kernel_in_hyp_mode())
+		pid_fmt  = ETM_OPT_CTXTID_IN_VMID;
+#endif
+	return sprintf(page, "config:%d\n", pid_fmt);
+}
+
+struct device_attribute format_attr_pid = __ATTR(pid, 0444, format_attr_pid_show, NULL);
+
 static struct attribute *etm_config_formats_attr[] = {
 	&format_attr_cycacc.attr,
 	&format_attr_contextid.attr,
+	&format_attr_contextid_in_vmid.attr,
+	&format_attr_pid.attr,
 	&format_attr_timestamp.attr,
 	&format_attr_retstack.attr,
 	&format_attr_sinkid.attr,
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index b20b6ff17cf6..8b7c7a8b2874 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -477,6 +477,15 @@ static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,
 		/* bit[6], Context ID tracing bit */
 		config->cfg |= BIT(ETM4_CFG_BIT_CTXTID);
 
+	/* Do not enable VMID tracing if we are not running in EL2 */
+	if (attr->config & BIT(ETM_OPT_CTXTID_IN_VMID)) {
+		if (!is_kernel_in_hyp_mode()) {
+			ret = -EINVAL;
+			goto out;
+		}
+		config->cfg |= BIT(ETM4_CFG_BIT_VMID) | BIT(ETM4_CFG_BIT_VMID_OPT);
+	}
+
 	/* return stack - enable if selected and supported */
 	if ((attr->config & BIT(ETM_OPT_RETSTK)) && drvdata->retstack)
 		/* bit[12], Return stack enable bit */
diff --git a/include/linux/coresight-pmu.h b/include/linux/coresight-pmu.h
index b0e35eec6499..927c6285ce5d 100644
--- a/include/linux/coresight-pmu.h
+++ b/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)
 {
-- 
2.24.1


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

Re: [PATCH] coresight: etm-perf: Add support for PID tracing for kernel at EL2

[Leo Yan]

Hi Suzuki,

On Sun, Jan 10, 2021 at 01:24:30AM +0000, Suzuki Kuruppassery Poulose wrote:

[...]

> +static ssize_t format_attr_pid_show(struct device *dev,
> +				    struct device_attribute *attr,
> +				    char *page)
> +{
> +	int pid_fmt = ETM_OPT_CTXTID;
> +
> +#ifdef CONFIG_CORESIGHT_SOURCE_ETM4X
> +	if (is_kernel_in_hyp_mode())
> +		pid_fmt  = ETM_OPT_CTXTID_IN_VMID;
> +#endif

I have a different idea for this.  As Armv8 Arm describes: "The
Virtualization Host Extensions only apply to an implementation that
includes EL2 using AArch64.", seems to me it's more reasable to use
the condition "#ifdef CONFIG_64BIT", like below:

#ifdef CONFIG_64BIT
        if (is_kernel_in_hyp_mode())
                pid_fmt  = ETM_OPT_CTXTID_IN_VMID;
#endif

Please let me know if this is better or not.  I will resend the patch
series after agreement.

Thanks,
Leo

> +	return sprintf(page, "config:%d\n", pid_fmt);
> +}
> +
> +struct device_attribute format_attr_pid = __ATTR(pid, 0444, format_attr_pid_show, NULL);
> +
>  static struct attribute *etm_config_formats_attr[] = {
>  	&format_attr_cycacc.attr,
>  	&format_attr_contextid.attr,
> +	&format_attr_contextid_in_vmid.attr,
> +	&format_attr_pid.attr,
>  	&format_attr_timestamp.attr,
>  	&format_attr_retstack.attr,
>  	&format_attr_sinkid.attr,
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index b20b6ff17cf6..8b7c7a8b2874 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -477,6 +477,15 @@ static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,
>  		/* bit[6], Context ID tracing bit */
>  		config->cfg |= BIT(ETM4_CFG_BIT_CTXTID);
>  
> +	/* Do not enable VMID tracing if we are not running in EL2 */
> +	if (attr->config & BIT(ETM_OPT_CTXTID_IN_VMID)) {
> +		if (!is_kernel_in_hyp_mode()) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +		config->cfg |= BIT(ETM4_CFG_BIT_VMID) | BIT(ETM4_CFG_BIT_VMID_OPT);
> +	}
> +
>  	/* return stack - enable if selected and supported */
>  	if ((attr->config & BIT(ETM_OPT_RETSTK)) && drvdata->retstack)
>  		/* bit[12], Return stack enable bit */
> diff --git a/include/linux/coresight-pmu.h b/include/linux/coresight-pmu.h
> index b0e35eec6499..927c6285ce5d 100644
> --- a/include/linux/coresight-pmu.h
> +++ b/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)
>  {
> -- 
> 2.24.1
> 

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

Re: [PATCH] coresight: etm-perf: Add support for PID tracing for kernel at EL2

[Suzuki K Poulose]

On 1/10/21 1:41 AM, Leo Yan wrote:
> Hi Suzuki,
> 
> On Sun, Jan 10, 2021 at 01:24:30AM +0000, Suzuki Kuruppassery Poulose wrote:
> 
> [...]
> 
>> +static ssize_t format_attr_pid_show(struct device *dev,
>> +				    struct device_attribute *attr,
>> +				    char *page)
>> +{
>> +	int pid_fmt = ETM_OPT_CTXTID;
>> +
>> +#ifdef CONFIG_CORESIGHT_SOURCE_ETM4X
>> +	if (is_kernel_in_hyp_mode())
>> +		pid_fmt  = ETM_OPT_CTXTID_IN_VMID;
>> +#endif
> 
> I have a different idea for this.  As Armv8 Arm describes: "The
> Virtualization Host Extensions only apply to an implementation that
> includes EL2 using AArch64.", seems to me it's more reasable to use
> the condition "#ifdef CONFIG_64BIT", like below:
> 
> #ifdef CONFIG_64BIT
>          if (is_kernel_in_hyp_mode())
>                  pid_fmt  = ETM_OPT_CTXTID_IN_VMID;
> #endif
> 
> Please let me know if this is better or not.  I will resend the patch
> series after agreement.

I did think of that. But CORESIGHT_SOURCE_ETM4X already depends on CONFIG_ARM64.
So, that is more restrictive in terms of usage. And this wouldn't be the only
build failure if someone tried enable ETM4X for arm32. So I wouldn't worry about
someone trying to use ETM4X on arm32 accidentally.

Cheers
Suzuki

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

Re: [PATCH] coresight: etm-perf: Add support for PID tracing for kernel at EL2

[Leo Yan]

On Sun, Jan 10, 2021 at 10:34:57PM +0000, Suzuki Kuruppassery Poulose wrote:
> On 1/10/21 1:41 AM, Leo Yan wrote:
> > Hi Suzuki,
> > 
> > On Sun, Jan 10, 2021 at 01:24:30AM +0000, Suzuki Kuruppassery Poulose wrote:
> > 
> > [...]
> > 
> > > +static ssize_t format_attr_pid_show(struct device *dev,
> > > +				    struct device_attribute *attr,
> > > +				    char *page)
> > > +{
> > > +	int pid_fmt = ETM_OPT_CTXTID;
> > > +
> > > +#ifdef CONFIG_CORESIGHT_SOURCE_ETM4X
> > > +	if (is_kernel_in_hyp_mode())
> > > +		pid_fmt  = ETM_OPT_CTXTID_IN_VMID;
> > > +#endif
> > 
> > I have a different idea for this.  As Armv8 Arm describes: "The
> > Virtualization Host Extensions only apply to an implementation that
> > includes EL2 using AArch64.", seems to me it's more reasable to use
> > the condition "#ifdef CONFIG_64BIT", like below:
> > 
> > #ifdef CONFIG_64BIT
> >          if (is_kernel_in_hyp_mode())
> >                  pid_fmt  = ETM_OPT_CTXTID_IN_VMID;
> > #endif
> > 
> > Please let me know if this is better or not.  I will resend the patch
> > series after agreement.
> 
> I did think of that. But CORESIGHT_SOURCE_ETM4X already depends on CONFIG_ARM64.
> So, that is more restrictive in terms of usage. And this wouldn't be the only
> build failure if someone tried enable ETM4X for arm32. So I wouldn't worry about
> someone trying to use ETM4X on arm32 accidentally.

Makes sense, will send v2 with this change.

Thanks for confirmation,
Leo

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