[PATCH RFC] arm64/sysregs: Align field names in ID_AA64DFR0_EL1 with architecture
Mark Rutland
mark.rutland at arm.com
Wed May 25 01:46:13 PDT 2022
Hi Mark,
On Wed, May 18, 2022 at 03:57:50PM +0100, Mark Brown wrote:
> The naming scheme the architecture uses for the fields in ID_AA64DFR0_EL1
> does not align well with kernel conventions, using as it does a lot of
> MixedCase in various arrangements. In preparation for automatically
> generating the defines for this register rename the defines used to match
> what is in the architecture.
>
> Signed-off-by: Mark Brown <broonie at kernel.org>
> ---
>
> I am not entirely convinced if the best approach here is to deviate from
> the kernel naming convention as this does or to follow the architecture
> as we've decided in other cases, I don't really mind but would like some
> feedback before going ahead and sorting out the remaining issues with
> this register.
It's unfortunate the architecture itself doesn't follow a consistent pattern.
:/
I don't personally have strong feelings here. I'm happy with either:
(a) Matching the case to the architectural names, even if that means some
fields are MixedCase, as with this patch
(b) Always using ALLCAPS for ID reg field definitions.
Catalin/Marc/Will, any preference?
Mark.
> arch/arm64/include/asm/assembler.h | 2 +-
> arch/arm64/include/asm/el2_setup.h | 8 ++++----
> arch/arm64/include/asm/hw_breakpoint.h | 4 ++--
> arch/arm64/include/asm/sysreg.h | 20 ++++++++++----------
> arch/arm64/kernel/cpufeature.c | 14 +++++++-------
> arch/arm64/kernel/debug-monitors.c | 2 +-
> arch/arm64/kernel/perf_event.c | 2 +-
> arch/arm64/kvm/debug.c | 4 ++--
> arch/arm64/kvm/hyp/nvhe/pkvm.c | 12 ++++++------
> arch/arm64/kvm/sys_regs.c | 14 +++++++-------
> 10 files changed, 41 insertions(+), 41 deletions(-)
>
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index 8c5a61aeaf8e..a97190938ff7 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -489,7 +489,7 @@ alternative_endif
> */
> .macro reset_pmuserenr_el0, tmpreg
> mrs \tmpreg, id_aa64dfr0_el1
> - sbfx \tmpreg, \tmpreg, #ID_AA64DFR0_PMUVER_SHIFT, #4
> + sbfx \tmpreg, \tmpreg, #ID_AA64DFR0_PMUVer_SHIFT, #4
> cmp \tmpreg, #1 // Skip if no PMU present
> b.lt 9000f
> msr pmuserenr_el0, xzr // Disable PMU access from EL0
> diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
> index 34ceff08cac4..5584c810bdc7 100644
> --- a/arch/arm64/include/asm/el2_setup.h
> +++ b/arch/arm64/include/asm/el2_setup.h
> @@ -40,7 +40,7 @@
>
> .macro __init_el2_debug
> mrs x1, id_aa64dfr0_el1
> - sbfx x0, x1, #ID_AA64DFR0_PMUVER_SHIFT, #4
> + sbfx x0, x1, #ID_AA64DFR0_PMUVer_SHIFT, #4
> cmp x0, #1
> b.lt .Lskip_pmu_\@ // Skip if no PMU present
> mrs x0, pmcr_el0 // Disable debug access traps
> @@ -49,7 +49,7 @@
> csel x2, xzr, x0, lt // all PMU counters from EL1
>
> /* Statistical profiling */
> - ubfx x0, x1, #ID_AA64DFR0_PMSVER_SHIFT, #4
> + ubfx x0, x1, #ID_AA64DFR0_PMSVer_SHIFT, #4
> cbz x0, .Lskip_spe_\@ // Skip if SPE not present
>
> mrs_s x0, SYS_PMBIDR_EL1 // If SPE available at EL2,
> @@ -65,7 +65,7 @@
>
> .Lskip_spe_\@:
> /* Trace buffer */
> - ubfx x0, x1, #ID_AA64DFR0_TRBE_SHIFT, #4
> + ubfx x0, x1, #ID_AA64DFR0_TraceBuffer_SHIFT, #4
> cbz x0, .Lskip_trace_\@ // Skip if TraceBuffer is not present
>
> mrs_s x0, SYS_TRBIDR_EL1
> @@ -195,7 +195,7 @@
>
> mov x0, xzr
> mrs x1, id_aa64dfr0_el1
> - ubfx x1, x1, #ID_AA64DFR0_PMSVER_SHIFT, #4
> + ubfx x1, x1, #ID_AA64DFR0_PMSVer_SHIFT, #4
> cmp x1, #3
> b.lt .Lset_debug_fgt_\@
> /* Disable PMSNEVFR_EL1 read and write traps */
> diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
> index bc7aaed4b34e..d667c03d5f5e 100644
> --- a/arch/arm64/include/asm/hw_breakpoint.h
> +++ b/arch/arm64/include/asm/hw_breakpoint.h
> @@ -142,7 +142,7 @@ static inline int get_num_brps(void)
> u64 dfr0 = read_sanitised_ftr_reg(SYS_ID_AA64DFR0_EL1);
> return 1 +
> cpuid_feature_extract_unsigned_field(dfr0,
> - ID_AA64DFR0_BRPS_SHIFT);
> + ID_AA64DFR0_BRPs_SHIFT);
> }
>
> /* Determine number of WRP registers available. */
> @@ -151,7 +151,7 @@ static inline int get_num_wrps(void)
> u64 dfr0 = read_sanitised_ftr_reg(SYS_ID_AA64DFR0_EL1);
> return 1 +
> cpuid_feature_extract_unsigned_field(dfr0,
> - ID_AA64DFR0_WRPS_SHIFT);
> + ID_AA64DFR0_WRPs_SHIFT);
> }
>
> #endif /* __ASM_BREAKPOINT_H */
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 9a44e04701d3..67691adce459 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -932,16 +932,16 @@
>
> /* id_aa64dfr0 */
> #define ID_AA64DFR0_MTPMU_SHIFT 48
> -#define ID_AA64DFR0_TRBE_SHIFT 44
> -#define ID_AA64DFR0_TRACE_FILT_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
> -#define ID_AA64DFR0_BRPS_SHIFT 12
> -#define ID_AA64DFR0_PMUVER_SHIFT 8
> -#define ID_AA64DFR0_TRACEVER_SHIFT 4
> -#define ID_AA64DFR0_DEBUGVER_SHIFT 0
> +#define ID_AA64DFR0_TraceBuffer_SHIFT 44
> +#define ID_AA64DFR0_TraceFilt_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
> +#define ID_AA64DFR0_BRPs_SHIFT 12
> +#define ID_AA64DFR0_PMUVer_SHIFT 8
> +#define ID_AA64DFR0_TraceVer_SHIFT 4
> +#define ID_AA64DFR0_DebugVer_SHIFT 0
>
> #define ID_AA64DFR0_PMUVER_8_0 0x1
> #define ID_AA64DFR0_PMUVER_8_1 0x4
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 089221eb5ae8..f60ba05214ab 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -432,17 +432,17 @@ 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, ID_AA64DFR0_DOUBLELOCK_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),
> - ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_BRPS_SHIFT, 4, 0),
> + S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_DoubleLock_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),
> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_BRPs_SHIFT, 4, 0),
> /*
> * We can instantiate multiple PMU instances with different levels
> * of support.
> */
> - S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_EXACT, ID_AA64DFR0_PMUVER_SHIFT, 4, 0),
> - ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64DFR0_DEBUGVER_SHIFT, 4, 0x6),
> + S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_EXACT, ID_AA64DFR0_PMUVer_SHIFT, 4, 0),
> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64DFR0_DebugVer_SHIFT, 4, 0x6),
> ARM64_FTR_END,
> };
>
> diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
> index bf9fe71589bc..572c8ac2873c 100644
> --- a/arch/arm64/kernel/debug-monitors.c
> +++ b/arch/arm64/kernel/debug-monitors.c
> @@ -28,7 +28,7 @@
> u8 debug_monitors_arch(void)
> {
> return cpuid_feature_extract_unsigned_field(read_sanitised_ftr_reg(SYS_ID_AA64DFR0_EL1),
> - ID_AA64DFR0_DEBUGVER_SHIFT);
> + ID_AA64DFR0_DebugVer_SHIFT);
> }
>
> /*
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index cb69ff1e6138..0f363546cc55 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -1145,7 +1145,7 @@ static void __armv8pmu_probe_pmu(void *info)
>
> dfr0 = read_sysreg(id_aa64dfr0_el1);
> pmuver = cpuid_feature_extract_unsigned_field(dfr0,
> - ID_AA64DFR0_PMUVER_SHIFT);
> + ID_AA64DFR0_PMUVer_SHIFT);
> if (pmuver == ID_AA64DFR0_PMUVER_IMP_DEF || pmuver == 0)
> return;
>
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index 4fd5c216c4bb..19a5bab1ea06 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -296,12 +296,12 @@ void kvm_arch_vcpu_load_debug_state_flags(struct kvm_vcpu *vcpu)
> * If SPE is present on this CPU and is available at current EL,
> * we may need to check if the host state needs to be saved.
> */
> - if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_PMSVER_SHIFT) &&
> + if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_PMSVer_SHIFT) &&
> !(read_sysreg_s(SYS_PMBIDR_EL1) & BIT(SYS_PMBIDR_EL1_P_SHIFT)))
> vcpu->arch.flags |= KVM_ARM64_DEBUG_STATE_SAVE_SPE;
>
> /* Check if we have TRBE implemented and available at the host */
> - if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_TRBE_SHIFT) &&
> + if (cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_TraceBuffer_SHIFT) &&
> !(read_sysreg_s(SYS_TRBIDR_EL1) & TRBIDR_PROG))
> vcpu->arch.flags |= KVM_ARM64_DEBUG_STATE_SAVE_TRBE;
> }
> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> index 99c8d8b73e70..6e4e5cb4fb55 100644
> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> @@ -86,32 +86,32 @@ static void pvm_init_traps_aa64dfr0(struct kvm_vcpu *vcpu)
> u64 cptr_set = 0;
>
> /* Trap/constrain PMU */
> - if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_PMUVER), feature_ids)) {
> + if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_PMUVer), feature_ids)) {
> mdcr_set |= MDCR_EL2_TPM | MDCR_EL2_TPMCR;
> mdcr_clear |= MDCR_EL2_HPME | MDCR_EL2_MTPME |
> MDCR_EL2_HPMN_MASK;
> }
>
> /* Trap Debug */
> - if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER), feature_ids))
> + if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_DebugVer), feature_ids))
> mdcr_set |= MDCR_EL2_TDRA | MDCR_EL2_TDA | MDCR_EL2_TDE;
>
> /* Trap OS Double Lock */
> - if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_DOUBLELOCK), feature_ids))
> + if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_DoubleLock), feature_ids))
> mdcr_set |= MDCR_EL2_TDOSA;
>
> /* Trap SPE */
> - if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_PMSVER), feature_ids)) {
> + if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_PMSVer), feature_ids)) {
> mdcr_set |= MDCR_EL2_TPMS;
> mdcr_clear |= MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT;
> }
>
> /* Trap Trace Filter */
> - if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_TRACE_FILT), feature_ids))
> + if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_TraceFilt), feature_ids))
> mdcr_set |= MDCR_EL2_TTRF;
>
> /* Trap Trace */
> - if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_TRACEVER), feature_ids))
> + if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_TraceVer), feature_ids))
> cptr_set |= CPTR_EL2_TTA;
>
> vcpu->arch.mdcr_el2 |= mdcr_set;
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index c06c0477fab5..dc3e42df6cde 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1150,14 +1150,14 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
> break;
> case SYS_ID_AA64DFR0_EL1:
> /* Limit debug to ARMv8.0 */
> - val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER);
> - val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER), 6);
> + val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_DebugVer);
> + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DebugVer), 6);
> /* Limit guests to PMUv3 for ARMv8.4 */
> val = cpuid_feature_cap_perfmon_field(val,
> - ID_AA64DFR0_PMUVER_SHIFT,
> + ID_AA64DFR0_PMUVer_SHIFT,
> kvm_vcpu_has_pmu(vcpu) ? ID_AA64DFR0_PMUVER_8_4 : 0);
> /* Hide SPE from guests */
> - val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_PMSVER);
> + val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_PMSVer);
> break;
> case SYS_ID_DFR0_EL1:
> /* Limit guests to PMUv3 for ARMv8.4 */
> @@ -1894,9 +1894,9 @@ static bool trap_dbgdidr(struct kvm_vcpu *vcpu,
> u64 pfr = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);
> u32 el3 = !!cpuid_feature_extract_unsigned_field(pfr, ID_AA64PFR0_EL3_SHIFT);
>
> - p->regval = ((((dfr >> ID_AA64DFR0_WRPS_SHIFT) & 0xf) << 28) |
> - (((dfr >> ID_AA64DFR0_BRPS_SHIFT) & 0xf) << 24) |
> - (((dfr >> ID_AA64DFR0_CTX_CMPS_SHIFT) & 0xf) << 20)
> + p->regval = ((((dfr >> ID_AA64DFR0_WRPs_SHIFT) & 0xf) << 28) |
> + (((dfr >> ID_AA64DFR0_BRPs_SHIFT) & 0xf) << 24) |
> + (((dfr >> ID_AA64DFR0_CTX_CMPs_SHIFT) & 0xf) << 20)
> | (6 << 16) | (1 << 15) | (el3 << 14) | (el3 << 12));
> return true;
> }
> --
> 2.30.2
>
More information about the linux-arm-kernel
mailing list