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

From: Leo Yan <leo.yan@linaro.org>
To: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: mark.rutland@arm.com, Al Grant <al.grant@arm.com>,
	lkp@intel.com, mathieu.poirier@linaro.org,
	alexander.shishkin@linux.intel.com, coresight@lists.linaro.org,
	john.garry@huawei.com, acme@kernel.org, peterz@infradead.org,
	mingo@redhat.com, will@kernel.org,
	linux-arm-kernel@lists.infradead.org, mike.leach@linaro.org
Subject: Re: [PATCH] coresight: etm-perf: Add support for PID tracing for kernel at EL2
Date: Sun, 10 Jan 2021 09:41:33 +0800	[thread overview]
Message-ID: <20210110014133.GA3776@leoy-ThinkPad-X240s> (raw)
In-Reply-To: <20210110012430.1874799-1-suzuki.poulose@arm.com>

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