diff mbox series

arm64: add support for Self-hosted trace

Message ID 1593520333-42241-1-git-send-email-zhangshaokun@hisilicon.com (mailing list archive)
State New, archived
Headers show
Series arm64: add support for Self-hosted trace | expand

Commit Message

Shaokun Zhang June 30, 2020, 12:32 p.m. UTC
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: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
Signed-off-by: Jonathan Zhou <jonathan.zhouwen@huawei.com>
---
 arch/arm64/Kconfig               | 11 +++++++++++
 arch/arm64/include/asm/cpucaps.h |  3 ++-
 arch/arm64/include/asm/sysreg.h  |  6 ++++++
 arch/arm64/kernel/cpufeature.c   | 25 ++++++++++++++++++++++++-
 4 files changed, 43 insertions(+), 2 deletions(-)

Comments

Suzuki K Poulose June 30, 2020, 1:28 p.m. UTC | #1
Hi

On 06/30/2020 01:32 PM, Shaokun Zhang 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: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> Signed-off-by: Jonathan Zhou <jonathan.zhouwen@huawei.com>
> ---
>   arch/arm64/Kconfig               | 11 +++++++++++
>   arch/arm64/include/asm/cpucaps.h |  3 ++-
>   arch/arm64/include/asm/sysreg.h  |  6 ++++++
>   arch/arm64/kernel/cpufeature.c   | 25 ++++++++++++++++++++++++-
>   4 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index a4a094bedcb2..328188f7962c 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1596,6 +1596,17 @@ config ARM64_AMU_EXTN
>   	  correctly reflect reality. Most commonly, the value read will be 0,
>   	  indicating that the counter is not enabled.
>   
> +config ARM64_SELF_HOSTED_TRACE
> +	bool "Enable support for the Self-hosted Trace Extension"
> +	default y
> +	help
> +	  The Self-hosted Trace is introduced by ARMv8.4 CPU architecture.
> +          It adds controls of trace in a self-hosted system through system
> +	  registers.
> +
> +	  Selecting this option will control trace filter and allow trace
> +	  at EL1 and EL0.
> +
>   endmenu
>   
>   menu "ARMv8.5 architectural features"
> diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
> index d7b3bb0cb180..99ffd7b5117a 100644
> --- a/arch/arm64/include/asm/cpucaps.h
> +++ b/arch/arm64/include/asm/cpucaps.h
> @@ -62,7 +62,8 @@
>   #define ARM64_HAS_GENERIC_AUTH			52
>   #define ARM64_HAS_32BIT_EL1			53
>   #define ARM64_BTI				54
> +#define ARM64_HAS_SELF_HOSTED_TRACE		55
>   
> -#define ARM64_NCAPS				55
> +#define ARM64_NCAPS				56
>   
>   #endif /* __ASM_CPUCAPS_H */
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 463175f80341..137e26bd3cc4 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -183,6 +183,7 @@
>   #define SYS_CPACR_EL1			sys_reg(3, 0, 1, 0, 2)
>   
>   #define SYS_ZCR_EL1			sys_reg(3, 0, 1, 2, 0)
> +#define SYS_TRFCR_EL1			sys_reg(3, 0, 1, 2, 1)
>   
>   #define SYS_TTBR0_EL1			sys_reg(3, 0, 2, 0, 0)
>   #define SYS_TTBR1_EL1			sys_reg(3, 0, 2, 0, 1)
> @@ -755,6 +756,8 @@
>   #define ID_AA64MMFR2_CNP_SHIFT		0
>   
>   /* id_aa64dfr0 */
> +#define ID_AA64DFR0_SELF_HOSTED_SHIFT	40
> +#define ID_AA64DFR0_DOUBLELOCK_SHIFT	36
>   #define ID_AA64DFR0_PMSVER_SHIFT	32
>   #define ID_AA64DFR0_CTX_CMPS_SHIFT	28
>   #define ID_AA64DFR0_WRPS_SHIFT		20
> @@ -875,6 +878,9 @@
>   #define CPACR_EL1_ZEN_EL0EN	(BIT(17)) /* enable EL0 access, if EL1EN set */
>   #define CPACR_EL1_ZEN		(CPACR_EL1_ZEN_EL1EN | CPACR_EL1_ZEN_EL0EN)
>   
> +#define TRFCR_EL1_E0TRE		(BIT(0)) /* Trace is allowed at EL0 */
> +#define TRFCR_EL1_E1TRE		(BIT(1)) /* Trace is allowed at EL1 */
> +#define TRFCR_EL1_TRE		(TRFCR_EL1_E0TRE | TRFCR_EL1_E1TRE)
>   
>   /* Safe value for MPIDR_EL1: Bit31:RES1, Bit30:U:0, Bit24:MT:0 */
>   #define SYS_MPIDR_SAFE_VAL	(BIT(31))
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 4ae41670c2e6..6008f06ce2c4 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -368,7 +368,8 @@ static const struct arm64_ftr_bits ftr_id_mmfr0[] = {
>   };
>   
>   static const struct arm64_ftr_bits ftr_id_aa64dfr0[] = {
> -	S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 36, 4, 0),
> +	S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 44, 4, 0),
> +	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_SELF_HOSTED_SHIFT, 4, 0),
>   	ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR0_PMSVER_SHIFT, 4, 0),
>   	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_CTX_CMPS_SHIFT, 4, 0),
>   	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_WRPS_SHIFT, 4, 0),
> @@ -1655,6 +1656,15 @@ static void bti_enable(const struct arm64_cpu_capabilities *__unused)
>   }
>   #endif /* CONFIG_ARM64_BTI */
>   
> +#ifdef CONFIG_ARM64_SELF_HOSTED_TRACE
> +static void self_hosted_trace_enable(const struct arm64_cpu_capabilities *__unused)
> +{
> +	/* Enable E0TRE and E1TRE in TRFCR_EL1 */
> +	write_sysreg_s(read_sysreg_s(SYS_TRFCR_EL1) | TRFCR_EL1_TRE, SYS_TRFCR_EL1);
> +	isb();
> +}


Is this really sufficient ? If we are running with VHE, don't we need
to set the TRFCR_EL2.E0HTRE ? Similarly we need to set E2TRE for
enabling kernel tracing.

> +#endif /* CONFIG_ARM64_SELF_HOSTED_TRACE */
> +
>   /* Internal helper functions to match cpu capability type */
>   static bool
>   cpucap_late_cpu_optional(const struct arm64_cpu_capabilities *cap)
> @@ -2054,6 +2064,19 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>   		.sign = FTR_UNSIGNED,
>   	},
>   #endif
> +#ifdef CONFIG_ARM64_SELF_HOSTED_TRACE
> +	{
> +		.desc = "Self-hosted Trace",
> +		.capability = ARM64_HAS_SELF_HOSTED_TRACE,
> +		.type = ARM64_CPUCAP_SYSTEM_FEATURE,

Also, if we are running on a system with a mix of CPUs, we would never
set the TRFCRx bits, which implies the ETMs won't be able to provide
useful trace information.

And finally, do we need a special config for this ? This can be a
default setting performed on any capable CPU.

Suzuki
Shaokun Zhang July 1, 2020, 9:18 a.m. UTC | #2
Hi Suzuki,

在 2020/6/30 21:28, Suzuki K Poulose 写道:
> Hi
> 
> On 06/30/2020 01:32 PM, Shaokun Zhang 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: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
>> Signed-off-by: Jonathan Zhou <jonathan.zhouwen@huawei.com>
>> ---
>>   arch/arm64/Kconfig               | 11 +++++++++++
>>   arch/arm64/include/asm/cpucaps.h |  3 ++-
>>   arch/arm64/include/asm/sysreg.h  |  6 ++++++
>>   arch/arm64/kernel/cpufeature.c   | 25 ++++++++++++++++++++++++-
>>   4 files changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index a4a094bedcb2..328188f7962c 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -1596,6 +1596,17 @@ config ARM64_AMU_EXTN
>>         correctly reflect reality. Most commonly, the value read will
>> be 0,
>>         indicating that the counter is not enabled.
>>   +config ARM64_SELF_HOSTED_TRACE
>> +    bool "Enable support for the Self-hosted Trace Extension"
>> +    default y
>> +    help
>> +      The Self-hosted Trace is introduced by ARMv8.4 CPU architecture.
>> +          It adds controls of trace in a self-hosted system through
>> system
>> +      registers.
>> +
>> +      Selecting this option will control trace filter and allow trace
>> +      at EL1 and EL0.
>> +
>>   endmenu
>>     menu "ARMv8.5 architectural features"
>> diff --git a/arch/arm64/include/asm/cpucaps.h
>> b/arch/arm64/include/asm/cpucaps.h
>> index d7b3bb0cb180..99ffd7b5117a 100644
>> --- a/arch/arm64/include/asm/cpucaps.h
>> +++ b/arch/arm64/include/asm/cpucaps.h
>> @@ -62,7 +62,8 @@
>>   #define ARM64_HAS_GENERIC_AUTH            52
>>   #define ARM64_HAS_32BIT_EL1            53
>>   #define ARM64_BTI                54
>> +#define ARM64_HAS_SELF_HOSTED_TRACE        55
>>   -#define ARM64_NCAPS                55
>> +#define ARM64_NCAPS                56
>>     #endif /* __ASM_CPUCAPS_H */
>> diff --git a/arch/arm64/include/asm/sysreg.h
>> b/arch/arm64/include/asm/sysreg.h
>> index 463175f80341..137e26bd3cc4 100644
>> --- a/arch/arm64/include/asm/sysreg.h
>> +++ b/arch/arm64/include/asm/sysreg.h
>> @@ -183,6 +183,7 @@
>>   #define SYS_CPACR_EL1            sys_reg(3, 0, 1, 0, 2)
>>     #define SYS_ZCR_EL1            sys_reg(3, 0, 1, 2, 0)
>> +#define SYS_TRFCR_EL1            sys_reg(3, 0, 1, 2, 1)
>>     #define SYS_TTBR0_EL1            sys_reg(3, 0, 2, 0, 0)
>>   #define SYS_TTBR1_EL1            sys_reg(3, 0, 2, 0, 1)
>> @@ -755,6 +756,8 @@
>>   #define ID_AA64MMFR2_CNP_SHIFT        0
>>     /* id_aa64dfr0 */
>> +#define ID_AA64DFR0_SELF_HOSTED_SHIFT    40
>> +#define ID_AA64DFR0_DOUBLELOCK_SHIFT    36
>>   #define ID_AA64DFR0_PMSVER_SHIFT    32
>>   #define ID_AA64DFR0_CTX_CMPS_SHIFT    28
>>   #define ID_AA64DFR0_WRPS_SHIFT        20
>> @@ -875,6 +878,9 @@
>>   #define CPACR_EL1_ZEN_EL0EN    (BIT(17)) /* enable EL0 access, if
>> EL1EN set */
>>   #define CPACR_EL1_ZEN        (CPACR_EL1_ZEN_EL1EN |
>> CPACR_EL1_ZEN_EL0EN)
>>   +#define TRFCR_EL1_E0TRE        (BIT(0)) /* Trace is allowed at EL0 */
>> +#define TRFCR_EL1_E1TRE        (BIT(1)) /* Trace is allowed at EL1 */
>> +#define TRFCR_EL1_TRE        (TRFCR_EL1_E0TRE | TRFCR_EL1_E1TRE)
>>     /* Safe value for MPIDR_EL1: Bit31:RES1, Bit30:U:0, Bit24:MT:0 */
>>   #define SYS_MPIDR_SAFE_VAL    (BIT(31))
>> diff --git a/arch/arm64/kernel/cpufeature.c
>> b/arch/arm64/kernel/cpufeature.c
>> index 4ae41670c2e6..6008f06ce2c4 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -368,7 +368,8 @@ static const struct arm64_ftr_bits ftr_id_mmfr0[] = {
>>   };
>>     static const struct arm64_ftr_bits ftr_id_aa64dfr0[] = {
>> -    S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 36, 4, 0),
>> +    S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 44, 4, 0),
>> +    ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE,
>> ID_AA64DFR0_SELF_HOSTED_SHIFT, 4, 0),
>>       ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE,
>> ID_AA64DFR0_PMSVER_SHIFT, 4, 0),
>>       ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE,
>> ID_AA64DFR0_CTX_CMPS_SHIFT, 4, 0),
>>       ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE,
>> ID_AA64DFR0_WRPS_SHIFT, 4, 0),
>> @@ -1655,6 +1656,15 @@ static void bti_enable(const struct
>> arm64_cpu_capabilities *__unused)
>>   }
>>   #endif /* CONFIG_ARM64_BTI */
>>   +#ifdef CONFIG_ARM64_SELF_HOSTED_TRACE
>> +static void self_hosted_trace_enable(const struct
>> arm64_cpu_capabilities *__unused)
>> +{
>> +    /* Enable E0TRE and E1TRE in TRFCR_EL1 */
>> +    write_sysreg_s(read_sysreg_s(SYS_TRFCR_EL1) | TRFCR_EL1_TRE,
>> SYS_TRFCR_EL1);
>> +    isb();
>> +}
> 
> 
> Is this really sufficient ? If we are running with VHE, don't we need
> to set the TRFCR_EL2.E0HTRE ? Similarly we need to set E2TRE for

Correct, I missed it.

> enabling kernel tracing.
> 
>> +#endif /* CONFIG_ARM64_SELF_HOSTED_TRACE */
>> +
>>   /* Internal helper functions to match cpu capability type */
>>   static bool
>>   cpucap_late_cpu_optional(const struct arm64_cpu_capabilities *cap)
>> @@ -2054,6 +2064,19 @@ static const struct arm64_cpu_capabilities
>> arm64_features[] = {
>>           .sign = FTR_UNSIGNED,
>>       },
>>   #endif
>> +#ifdef CONFIG_ARM64_SELF_HOSTED_TRACE
>> +    {
>> +        .desc = "Self-hosted Trace",
>> +        .capability = ARM64_HAS_SELF_HOSTED_TRACE,
>> +        .type = ARM64_CPUCAP_SYSTEM_FEATURE,
> 
> Also, if we are running on a system with a mix of CPUs, we would never
> set the TRFCRx bits, which implies the ETMs won't be able to provide

Sorry, Maybe I don't follow it correctly, what do you mean about "mix of
CPUs", do you mean some CPU support ETM and other may don't support ETM?

> useful trace information.
> 
> And finally, do we need a special config for this ? This can be a

Do you mean that firmware can do it if the user wants to enable trace?

Thanks,
Shaokun

> default setting performed on any capable CPU.
> 
> Suzuki
> 
> .
Suzuki K Poulose July 2, 2020, 12:06 a.m. UTC | #3
On 07/01/2020 10:18 AM, Shaokun Zhang wrote:
> Hi Suzuki,
> 
> 在 2020/6/30 21:28, Suzuki K Poulose 写道:
>> Hi
>>
>> On 06/30/2020 01:32 PM, Shaokun Zhang 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: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
>>> Signed-off-by: Jonathan Zhou <jonathan.zhouwen@huawei.com>
>>> ---
>>>    arch/arm64/Kconfig               | 11 +++++++++++
>>>    arch/arm64/include/asm/cpucaps.h |  3 ++-
>>>    arch/arm64/include/asm/sysreg.h  |  6 ++++++
>>>    arch/arm64/kernel/cpufeature.c   | 25 ++++++++++++++++++++++++-
>>>    4 files changed, 43 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index a4a094bedcb2..328188f7962c 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -1596,6 +1596,17 @@ config ARM64_AMU_EXTN
>>>          correctly reflect reality. Most commonly, the value read will
>>> be 0,
>>>          indicating that the counter is not enabled.
>>>    +config ARM64_SELF_HOSTED_TRACE
>>> +    bool "Enable support for the Self-hosted Trace Extension"
>>> +    default y
>>> +    help
>>> +      The Self-hosted Trace is introduced by ARMv8.4 CPU architecture.
>>> +          It adds controls of trace in a self-hosted system through
>>> system
>>> +      registers.
>>> +
>>> +      Selecting this option will control trace filter and allow trace
>>> +      at EL1 and EL0.
>>> +
>>>    endmenu
>>>      menu "ARMv8.5 architectural features"
>>> diff --git a/arch/arm64/include/asm/cpucaps.h
>>> b/arch/arm64/include/asm/cpucaps.h
>>> index d7b3bb0cb180..99ffd7b5117a 100644
>>> --- a/arch/arm64/include/asm/cpucaps.h
>>> +++ b/arch/arm64/include/asm/cpucaps.h
>>> @@ -62,7 +62,8 @@
>>>    #define ARM64_HAS_GENERIC_AUTH            52
>>>    #define ARM64_HAS_32BIT_EL1            53
>>>    #define ARM64_BTI                54
>>> +#define ARM64_HAS_SELF_HOSTED_TRACE        55
>>>    -#define ARM64_NCAPS                55
>>> +#define ARM64_NCAPS                56
>>>      #endif /* __ASM_CPUCAPS_H */
>>> diff --git a/arch/arm64/include/asm/sysreg.h
>>> b/arch/arm64/include/asm/sysreg.h
>>> index 463175f80341..137e26bd3cc4 100644
>>> --- a/arch/arm64/include/asm/sysreg.h
>>> +++ b/arch/arm64/include/asm/sysreg.h
>>> @@ -183,6 +183,7 @@
>>>    #define SYS_CPACR_EL1            sys_reg(3, 0, 1, 0, 2)
>>>      #define SYS_ZCR_EL1            sys_reg(3, 0, 1, 2, 0)
>>> +#define SYS_TRFCR_EL1            sys_reg(3, 0, 1, 2, 1)
>>>      #define SYS_TTBR0_EL1            sys_reg(3, 0, 2, 0, 0)
>>>    #define SYS_TTBR1_EL1            sys_reg(3, 0, 2, 0, 1)
>>> @@ -755,6 +756,8 @@
>>>    #define ID_AA64MMFR2_CNP_SHIFT        0
>>>      /* id_aa64dfr0 */
>>> +#define ID_AA64DFR0_SELF_HOSTED_SHIFT    40
>>> +#define ID_AA64DFR0_DOUBLELOCK_SHIFT    36
>>>    #define ID_AA64DFR0_PMSVER_SHIFT    32
>>>    #define ID_AA64DFR0_CTX_CMPS_SHIFT    28
>>>    #define ID_AA64DFR0_WRPS_SHIFT        20
>>> @@ -875,6 +878,9 @@
>>>    #define CPACR_EL1_ZEN_EL0EN    (BIT(17)) /* enable EL0 access, if
>>> EL1EN set */
>>>    #define CPACR_EL1_ZEN        (CPACR_EL1_ZEN_EL1EN |
>>> CPACR_EL1_ZEN_EL0EN)
>>>    +#define TRFCR_EL1_E0TRE        (BIT(0)) /* Trace is allowed at EL0 */
>>> +#define TRFCR_EL1_E1TRE        (BIT(1)) /* Trace is allowed at EL1 */
>>> +#define TRFCR_EL1_TRE        (TRFCR_EL1_E0TRE | TRFCR_EL1_E1TRE)
>>>      /* Safe value for MPIDR_EL1: Bit31:RES1, Bit30:U:0, Bit24:MT:0 */
>>>    #define SYS_MPIDR_SAFE_VAL    (BIT(31))
>>> diff --git a/arch/arm64/kernel/cpufeature.c
>>> b/arch/arm64/kernel/cpufeature.c
>>> index 4ae41670c2e6..6008f06ce2c4 100644
>>> --- a/arch/arm64/kernel/cpufeature.c
>>> +++ b/arch/arm64/kernel/cpufeature.c
>>> @@ -368,7 +368,8 @@ static const struct arm64_ftr_bits ftr_id_mmfr0[] = {
>>>    };
>>>      static const struct arm64_ftr_bits ftr_id_aa64dfr0[] = {
>>> -    S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 36, 4, 0),
>>> +    S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 44, 4, 0),
>>> +    ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE,
>>> ID_AA64DFR0_SELF_HOSTED_SHIFT, 4, 0),
>>>        ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE,
>>> ID_AA64DFR0_PMSVER_SHIFT, 4, 0),
>>>        ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE,
>>> ID_AA64DFR0_CTX_CMPS_SHIFT, 4, 0),
>>>        ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE,
>>> ID_AA64DFR0_WRPS_SHIFT, 4, 0),
>>> @@ -1655,6 +1656,15 @@ static void bti_enable(const struct
>>> arm64_cpu_capabilities *__unused)
>>>    }
>>>    #endif /* CONFIG_ARM64_BTI */
>>>    +#ifdef CONFIG_ARM64_SELF_HOSTED_TRACE
>>> +static void self_hosted_trace_enable(const struct
>>> arm64_cpu_capabilities *__unused)
>>> +{
>>> +    /* Enable E0TRE and E1TRE in TRFCR_EL1 */
>>> +    write_sysreg_s(read_sysreg_s(SYS_TRFCR_EL1) | TRFCR_EL1_TRE,
>>> SYS_TRFCR_EL1);
>>> +    isb();
>>> +}
>>
>>
>> Is this really sufficient ? If we are running with VHE, don't we need
>> to set the TRFCR_EL2.E0HTRE ? Similarly we need to set E2TRE for
> 
> Correct, I missed it.

Please ignore the comment above. Looking at the psuedo code for
TRFCR_EL1 access, with VHE the access is routed to TRFCR_EL2. So
what you have is fine. But, please see below.

> 
>> enabling kernel tracing.
>>
>>> +#endif /* CONFIG_ARM64_SELF_HOSTED_TRACE */
>>> +
>>>    /* Internal helper functions to match cpu capability type */
>>>    static bool
>>>    cpucap_late_cpu_optional(const struct arm64_cpu_capabilities *cap)
>>> @@ -2054,6 +2064,19 @@ static const struct arm64_cpu_capabilities
>>> arm64_features[] = {
>>>            .sign = FTR_UNSIGNED,
>>>        },
>>>    #endif
>>> +#ifdef CONFIG_ARM64_SELF_HOSTED_TRACE
>>> +    {
>>> +        .desc = "Self-hosted Trace",
>>> +        .capability = ARM64_HAS_SELF_HOSTED_TRACE,
>>> +        .type = ARM64_CPUCAP_SYSTEM_FEATURE,
>>
>> Also, if we are running on a system with a mix of CPUs, we would never
>> set the TRFCRx bits, which implies the ETMs won't be able to provide
> 
> Sorry, Maybe I don't follow it correctly, what do you mean about "mix of
> CPUs", do you mean some CPU support ETM and other may don't support ETM?
> 

I meant, if you have a mix of CPUs with and without the TraceFiltering 
support, the CPUs with TraceFiltering support will not be able to trace
unless they enable the tracing explicitly in the TRFCR_ELx.

>> useful trace information.
>>
>> And finally, do we need a special config for this ? This can be a
> 
> Do you mean that firmware can do it if the user wants to enable trace?

No. We should do this always, for a CPU capable of Trace filtering, by
checking the ID register. The ETM tracing is an existing functionality
used by the cores. So having a new config and hiding the control behind
it is not going to help the existing users. Also, given that the actual
trace functionality is controlled CORESIGHT configs, we should either
use that or leave the CoreSight drivers to do this setup.

My preference is to leave this to the CoreSight drivers to select the
appropriate filtering based on the "individual" trace sessions.

Suzuki
Shaokun Zhang July 7, 2020, 1:26 p.m. UTC | #4
Hi Suzuki,

在 2020/7/2 8:06, Suzuki K Poulose 写道:
> On 07/01/2020 10:18 AM, Shaokun Zhang wrote:
>> Hi Suzuki,
>>
>> 在 2020/6/30 21:28, Suzuki K Poulose 写道:
>>> Hi
>>>
>>> On 06/30/2020 01:32 PM, Shaokun Zhang 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: Catalin Marinas <catalin.marinas@arm.com>
>>>> Cc: Will Deacon <will@kernel.org>
>>>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
>>>> Signed-off-by: Jonathan Zhou <jonathan.zhouwen@huawei.com>
>>>> ---
>>>>    arch/arm64/Kconfig               | 11 +++++++++++
>>>>    arch/arm64/include/asm/cpucaps.h |  3 ++-
>>>>    arch/arm64/include/asm/sysreg.h  |  6 ++++++
>>>>    arch/arm64/kernel/cpufeature.c   | 25 ++++++++++++++++++++++++-
>>>>    4 files changed, 43 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>> index a4a094bedcb2..328188f7962c 100644
>>>> --- a/arch/arm64/Kconfig
>>>> +++ b/arch/arm64/Kconfig
>>>> @@ -1596,6 +1596,17 @@ config ARM64_AMU_EXTN
>>>>          correctly reflect reality. Most commonly, the value read will
>>>> be 0,
>>>>          indicating that the counter is not enabled.
>>>>    +config ARM64_SELF_HOSTED_TRACE
>>>> +    bool "Enable support for the Self-hosted Trace Extension"
>>>> +    default y
>>>> +    help
>>>> +      The Self-hosted Trace is introduced by ARMv8.4 CPU architecture.
>>>> +          It adds controls of trace in a self-hosted system through
>>>> system
>>>> +      registers.
>>>> +
>>>> +      Selecting this option will control trace filter and allow trace
>>>> +      at EL1 and EL0.
>>>> +
>>>>    endmenu
>>>>      menu "ARMv8.5 architectural features"
>>>> diff --git a/arch/arm64/include/asm/cpucaps.h
>>>> b/arch/arm64/include/asm/cpucaps.h
>>>> index d7b3bb0cb180..99ffd7b5117a 100644
>>>> --- a/arch/arm64/include/asm/cpucaps.h
>>>> +++ b/arch/arm64/include/asm/cpucaps.h
>>>> @@ -62,7 +62,8 @@
>>>>    #define ARM64_HAS_GENERIC_AUTH            52
>>>>    #define ARM64_HAS_32BIT_EL1            53
>>>>    #define ARM64_BTI                54
>>>> +#define ARM64_HAS_SELF_HOSTED_TRACE        55
>>>>    -#define ARM64_NCAPS                55
>>>> +#define ARM64_NCAPS                56
>>>>      #endif /* __ASM_CPUCAPS_H */
>>>> diff --git a/arch/arm64/include/asm/sysreg.h
>>>> b/arch/arm64/include/asm/sysreg.h
>>>> index 463175f80341..137e26bd3cc4 100644
>>>> --- a/arch/arm64/include/asm/sysreg.h
>>>> +++ b/arch/arm64/include/asm/sysreg.h
>>>> @@ -183,6 +183,7 @@
>>>>    #define SYS_CPACR_EL1            sys_reg(3, 0, 1, 0, 2)
>>>>      #define SYS_ZCR_EL1            sys_reg(3, 0, 1, 2, 0)
>>>> +#define SYS_TRFCR_EL1            sys_reg(3, 0, 1, 2, 1)
>>>>      #define SYS_TTBR0_EL1            sys_reg(3, 0, 2, 0, 0)
>>>>    #define SYS_TTBR1_EL1            sys_reg(3, 0, 2, 0, 1)
>>>> @@ -755,6 +756,8 @@
>>>>    #define ID_AA64MMFR2_CNP_SHIFT        0
>>>>      /* id_aa64dfr0 */
>>>> +#define ID_AA64DFR0_SELF_HOSTED_SHIFT    40
>>>> +#define ID_AA64DFR0_DOUBLELOCK_SHIFT    36
>>>>    #define ID_AA64DFR0_PMSVER_SHIFT    32
>>>>    #define ID_AA64DFR0_CTX_CMPS_SHIFT    28
>>>>    #define ID_AA64DFR0_WRPS_SHIFT        20
>>>> @@ -875,6 +878,9 @@
>>>>    #define CPACR_EL1_ZEN_EL0EN    (BIT(17)) /* enable EL0 access, if
>>>> EL1EN set */
>>>>    #define CPACR_EL1_ZEN        (CPACR_EL1_ZEN_EL1EN |
>>>> CPACR_EL1_ZEN_EL0EN)
>>>>    +#define TRFCR_EL1_E0TRE        (BIT(0)) /* Trace is allowed at
>>>> EL0 */
>>>> +#define TRFCR_EL1_E1TRE        (BIT(1)) /* Trace is allowed at EL1 */
>>>> +#define TRFCR_EL1_TRE        (TRFCR_EL1_E0TRE | TRFCR_EL1_E1TRE)
>>>>      /* Safe value for MPIDR_EL1: Bit31:RES1, Bit30:U:0, Bit24:MT:0 */
>>>>    #define SYS_MPIDR_SAFE_VAL    (BIT(31))
>>>> diff --git a/arch/arm64/kernel/cpufeature.c
>>>> b/arch/arm64/kernel/cpufeature.c
>>>> index 4ae41670c2e6..6008f06ce2c4 100644
>>>> --- a/arch/arm64/kernel/cpufeature.c
>>>> +++ b/arch/arm64/kernel/cpufeature.c
>>>> @@ -368,7 +368,8 @@ static const struct arm64_ftr_bits
>>>> ftr_id_mmfr0[] = {
>>>>    };
>>>>      static const struct arm64_ftr_bits ftr_id_aa64dfr0[] = {
>>>> -    S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 36, 4,
>>>> 0),
>>>> +    S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 44, 4,
>>>> 0),
>>>> +    ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE,
>>>> ID_AA64DFR0_SELF_HOSTED_SHIFT, 4, 0),
>>>>        ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE,
>>>> ID_AA64DFR0_PMSVER_SHIFT, 4, 0),
>>>>        ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE,
>>>> ID_AA64DFR0_CTX_CMPS_SHIFT, 4, 0),
>>>>        ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE,
>>>> ID_AA64DFR0_WRPS_SHIFT, 4, 0),
>>>> @@ -1655,6 +1656,15 @@ static void bti_enable(const struct
>>>> arm64_cpu_capabilities *__unused)
>>>>    }
>>>>    #endif /* CONFIG_ARM64_BTI */
>>>>    +#ifdef CONFIG_ARM64_SELF_HOSTED_TRACE
>>>> +static void self_hosted_trace_enable(const struct
>>>> arm64_cpu_capabilities *__unused)
>>>> +{
>>>> +    /* Enable E0TRE and E1TRE in TRFCR_EL1 */
>>>> +    write_sysreg_s(read_sysreg_s(SYS_TRFCR_EL1) | TRFCR_EL1_TRE,
>>>> SYS_TRFCR_EL1);
>>>> +    isb();
>>>> +}
>>>
>>>
>>> Is this really sufficient ? If we are running with VHE, don't we need
>>> to set the TRFCR_EL2.E0HTRE ? Similarly we need to set E2TRE for
>>
>> Correct, I missed it.
> 
> Please ignore the comment above. Looking at the psuedo code for
> TRFCR_EL1 access, with VHE the access is routed to TRFCR_EL2. So
> what you have is fine. But, please see below.
> 
>>
>>> enabling kernel tracing.
>>>
>>>> +#endif /* CONFIG_ARM64_SELF_HOSTED_TRACE */
>>>> +
>>>>    /* Internal helper functions to match cpu capability type */
>>>>    static bool
>>>>    cpucap_late_cpu_optional(const struct arm64_cpu_capabilities *cap)
>>>> @@ -2054,6 +2064,19 @@ static const struct arm64_cpu_capabilities
>>>> arm64_features[] = {
>>>>            .sign = FTR_UNSIGNED,
>>>>        },
>>>>    #endif
>>>> +#ifdef CONFIG_ARM64_SELF_HOSTED_TRACE
>>>> +    {
>>>> +        .desc = "Self-hosted Trace",
>>>> +        .capability = ARM64_HAS_SELF_HOSTED_TRACE,
>>>> +        .type = ARM64_CPUCAP_SYSTEM_FEATURE,
>>>
>>> Also, if we are running on a system with a mix of CPUs, we would never
>>> set the TRFCRx bits, which implies the ETMs won't be able to provide
>>
>> Sorry, Maybe I don't follow it correctly, what do you mean about "mix of
>> CPUs", do you mean some CPU support ETM and other may don't support ETM?
>>
> 
> I meant, if you have a mix of CPUs with and without the TraceFiltering
> support, the CPUs with TraceFiltering support will not be able to trace
> unless they enable the tracing explicitly in the TRFCR_ELx.
> 
>>> useful trace information.
>>>
>>> And finally, do we need a special config for this ? This can be a
>>
>> Do you mean that firmware can do it if the user wants to enable trace?
> 
> No. We should do this always, for a CPU capable of Trace filtering, by
> checking the ID register. The ETM tracing is an existing functionality
> used by the cores. So having a new config and hiding the control behind
> it is not going to help the existing users. Also, given that the actual
> trace functionality is controlled CORESIGHT configs, we should either
> use that or leave the CoreSight drivers to do this setup.
> 
> My preference is to leave this to the CoreSight drivers to select the

I appreciate your comments and I check the coresight driver quickly.
Hopefully I shall not miss some important things. I'm not sure whether
it is appropriate to check CPU ID register in coresight driver?

Thanks,
Shaokun

> appropriate filtering based on the "individual" trace sessions.
> 
> Suzuki
> 
> .
diff mbox series

Patch

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index a4a094bedcb2..328188f7962c 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1596,6 +1596,17 @@  config ARM64_AMU_EXTN
 	  correctly reflect reality. Most commonly, the value read will be 0,
 	  indicating that the counter is not enabled.
 
+config ARM64_SELF_HOSTED_TRACE
+	bool "Enable support for the Self-hosted Trace Extension"
+	default y
+	help
+	  The Self-hosted Trace is introduced by ARMv8.4 CPU architecture.
+          It adds controls of trace in a self-hosted system through system
+	  registers.
+
+	  Selecting this option will control trace filter and allow trace
+	  at EL1 and EL0.
+
 endmenu
 
 menu "ARMv8.5 architectural features"
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index d7b3bb0cb180..99ffd7b5117a 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -62,7 +62,8 @@ 
 #define ARM64_HAS_GENERIC_AUTH			52
 #define ARM64_HAS_32BIT_EL1			53
 #define ARM64_BTI				54
+#define ARM64_HAS_SELF_HOSTED_TRACE		55
 
-#define ARM64_NCAPS				55
+#define ARM64_NCAPS				56
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 463175f80341..137e26bd3cc4 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -183,6 +183,7 @@ 
 #define SYS_CPACR_EL1			sys_reg(3, 0, 1, 0, 2)
 
 #define SYS_ZCR_EL1			sys_reg(3, 0, 1, 2, 0)
+#define SYS_TRFCR_EL1			sys_reg(3, 0, 1, 2, 1)
 
 #define SYS_TTBR0_EL1			sys_reg(3, 0, 2, 0, 0)
 #define SYS_TTBR1_EL1			sys_reg(3, 0, 2, 0, 1)
@@ -755,6 +756,8 @@ 
 #define ID_AA64MMFR2_CNP_SHIFT		0
 
 /* id_aa64dfr0 */
+#define ID_AA64DFR0_SELF_HOSTED_SHIFT	40
+#define ID_AA64DFR0_DOUBLELOCK_SHIFT	36
 #define ID_AA64DFR0_PMSVER_SHIFT	32
 #define ID_AA64DFR0_CTX_CMPS_SHIFT	28
 #define ID_AA64DFR0_WRPS_SHIFT		20
@@ -875,6 +878,9 @@ 
 #define CPACR_EL1_ZEN_EL0EN	(BIT(17)) /* enable EL0 access, if EL1EN set */
 #define CPACR_EL1_ZEN		(CPACR_EL1_ZEN_EL1EN | CPACR_EL1_ZEN_EL0EN)
 
+#define TRFCR_EL1_E0TRE		(BIT(0)) /* Trace is allowed at EL0 */
+#define TRFCR_EL1_E1TRE		(BIT(1)) /* Trace is allowed at EL1 */
+#define TRFCR_EL1_TRE		(TRFCR_EL1_E0TRE | TRFCR_EL1_E1TRE)
 
 /* Safe value for MPIDR_EL1: Bit31:RES1, Bit30:U:0, Bit24:MT:0 */
 #define SYS_MPIDR_SAFE_VAL	(BIT(31))
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 4ae41670c2e6..6008f06ce2c4 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -368,7 +368,8 @@  static const struct arm64_ftr_bits ftr_id_mmfr0[] = {
 };
 
 static const struct arm64_ftr_bits ftr_id_aa64dfr0[] = {
-	S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 36, 4, 0),
+	S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 44, 4, 0),
+	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_SELF_HOSTED_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR0_PMSVER_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_CTX_CMPS_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_WRPS_SHIFT, 4, 0),
@@ -1655,6 +1656,15 @@  static void bti_enable(const struct arm64_cpu_capabilities *__unused)
 }
 #endif /* CONFIG_ARM64_BTI */
 
+#ifdef CONFIG_ARM64_SELF_HOSTED_TRACE
+static void self_hosted_trace_enable(const struct arm64_cpu_capabilities *__unused)
+{
+	/* Enable E0TRE and E1TRE in TRFCR_EL1 */
+	write_sysreg_s(read_sysreg_s(SYS_TRFCR_EL1) | TRFCR_EL1_TRE, SYS_TRFCR_EL1);
+	isb();
+}
+#endif /* CONFIG_ARM64_SELF_HOSTED_TRACE */
+
 /* Internal helper functions to match cpu capability type */
 static bool
 cpucap_late_cpu_optional(const struct arm64_cpu_capabilities *cap)
@@ -2054,6 +2064,19 @@  static const struct arm64_cpu_capabilities arm64_features[] = {
 		.sign = FTR_UNSIGNED,
 	},
 #endif
+#ifdef CONFIG_ARM64_SELF_HOSTED_TRACE
+	{
+		.desc = "Self-hosted Trace",
+		.capability = ARM64_HAS_SELF_HOSTED_TRACE,
+		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
+		.matches = has_cpuid_feature,
+		.sys_reg = SYS_ID_AA64DFR0_EL1,
+		.field_pos = ID_AA64DFR0_SELF_HOSTED_SHIFT,
+		.sign = FTR_UNSIGNED,
+		.min_field_value = 1,
+		.cpu_enable = self_hosted_trace_enable,
+	},
+#endif
 	{},
 };