[PATCH v3] Coresight: etm4x: Add support for Self-hosted trace

[Suzuki K Poulose]

Jonathan

On 9/18/20 3:30 AM, Jonathan Zhou wrote:
> ARMv8.4 architecture extension introduces ARMv8.4-Trace, Armv8.4
> Self-hosted Trace Extensions. It provides control of exception
> levels and security states. Let's add this feature detection and
> enable E1TRE and E0TRE in TRFCR_EL1 if Self-hosted Trace is
> supported.
> 
> Cc: Mathieu Poirier <mathieu.poirier at linaro.org>
> Cc: Catalin Marinas <catalin.marinas at arm.com>
> Cc: Will Deacon <will at kernel.org>
> Cc: Suzuki K Poulose <suzuki.poulose at arm.com>
> Cc: Shaokun Zhang <zhangshaokun at hisilicon.com>
> Signed-off-by: Jonathan Zhou <jonathan.zhouwen at huawei.com>
> ---
> ChangeLog in v3:
>      * address Suzuki's comments
> 
>   arch/arm64/include/asm/sysreg.h               | 17 +++++++++++++
>   drivers/hwtracing/coresight/coresight-etm4x.c | 35 +++++++++++++++++++++++++++
>   2 files changed, 52 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 554a7e8ecb07..31b84a5fa35d 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -184,6 +184,22 @@
>   
>   #define SYS_ZCR_EL1			sys_reg(3, 0, 1, 2, 0)
>   
> +/* Trace Filter control EL1*/
> +#define SYS_TRFCR_EL1			sys_reg(3, 0, 1, 2, 1)
> +/* Trace Filter control EL2 */
> +#define SYS_TRFCR_EL2			sys_reg(3, 4, 1, 2, 1)

Please keep the ordering of the definitions.

> +
> +/* Trace is allowed at EL0 */
> +#define TRFCR_EL1_E0TRE			BIT(0)

TRFCR_ELx_E0TRE ?

> +/* Trace is allowed at EL1 */
> +#define TRFCR_EL1_E1TRE			BIT(1)

May be TRFCR_ELx_ELxTRE  ?

> +#define TRFCR_TS_SHIFT			5
> +#define TRFCR_TS_MASK			GENMASK(6, 5)
> +/* Virtual timestamp */
> +#define TRFCR_TS_VT			(0x1UL << TRFCR_TS_SHIFT)

> +/* Context idx is allowed at EL2 */
> +#define TRFCR_EL2_CX			BIT(3)
> +
>   #define SYS_TTBR0_EL1			sys_reg(3, 0, 2, 0, 0)
>   #define SYS_TTBR1_EL1			sys_reg(3, 0, 2, 0, 1)
>   #define SYS_TCR_EL1			sys_reg(3, 0, 2, 0, 2)
> @@ -772,6 +788,7 @@
>   #define ID_AA64MMFR2_CNP_SHIFT		0
>   
>   /* id_aa64dfr0 */
> +#define ID_AA64DFR0_SELF_HOSTED_SHIFT	40

Please stick to the architcted name for the field, TRACE_FILT

>   #define ID_AA64DFR0_DOUBLELOCK_SHIFT	36
>   #define ID_AA64DFR0_PMSVER_SHIFT	32
>   #define ID_AA64DFR0_CTX_CMPS_SHIFT	28
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
> index 96425e818fc2..82648a3c7a8e 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
> @@ -28,6 +28,7 @@
>   #include <linux/perf_event.h>
>   #include <linux/pm_runtime.h>
>   #include <linux/property.h>
> +#include <asm/sysreg.h>
>   #include <asm/sections.h>
>   #include <asm/local.h>
>   #include <asm/virt.h>
> @@ -785,6 +786,36 @@ static void etm4_init_arch_data(void *info)
>   	CS_LOCK(drvdata->base);
>   }
>   
> +static void etm4_init_sysctrl(void *info)
> +{
> +	u64 sys_trfcr_el1, dfr0;
> +	int trace_filt;
> +
> +	dfr0 = read_sysreg(id_aa64dfr0_el1);
> +
> +	trace_filt = cpuid_feature_extract_unsigned_field(dfr0,
> +					ID_AA64DFR0_SELF_HOSTED_SHIFT);
> +	if (trace_filt == 0x1) {
> +		sys_trfcr_el1 = read_sysreg_s(SYS_TRFCR_EL1);
> +		sys_trfcr_el1 &= (~TRFCR_TS_MASK);
> +		/* Enable EL1 & EL0 tracing. */
> +		sys_trfcr_el1 |= TRFCR_EL1_E0TRE;
> +		sys_trfcr_el1 |= TRFCR_EL1_E1TRE;
> +		/* Use virtual timestamp */
> +		sys_trfcr_el1 |= TRFCR_TS_VT;


Could we simply do :

		u64 trcfr = (TRFCR_ELx_E0TRE |
			     TRFCR_ELx_ExTRE |
			     TRFCR_ELx_TS_VT)
	
> +		/* enable contextid_el2 and vmid tracing. */
> +		if (is_kernel_in_hyp_mode())
> +			sys_trfcr_el1 |= TRFCR_EL2_CX;
> +		/*
> +		 * If pe is in EL2, we assume HCR_EL2.E2H == 1. Thus, accessing
> +		 * TRFCR_EL1 is routed to TRFCR_EL2 and leave TRFCR_EL1 to be
> +		 * configured by the kvm.
> +		 */
> +		write_sysreg_s(sys_trfcr_el1, SYS_TRFCR_EL1);
> +		isb();



> +	}
> +}


> +
>   static void etm4_set_default_config(struct etmv4_config *config)
>   {
>   	/* disable all events tracing */
> @@ -1504,6 +1535,10 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
>   				etm4_init_arch_data,  drvdata, 1))
>   		dev_err(dev, "ETM arch init failed\n");
>   
> +	if (smp_call_function_single(drvdata->cpu,
> +				etm4_init_sysctrl, drvdata, 1))
> +		dev_err(dev, "ETM sysctrl init failed\n");

We could as well move this to etm4_init_arch_data(), which is called
already on the CPU.

Suzuki