From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B2313C433F5 for ; Wed, 25 May 2022 08:47:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ychOCjppf7r318Bc/L9SZTbScTo9+3nV5x8QEXYKDq0=; b=X9sAkT+b+LxljS J3QI3cDaz02PuloeyxI60XZgXViCtESCqdq/YwQ2wM2ZjmaPLLipBfxzFFGIktPhmTIvC7fYWu20Z vvRbMt7cxCUfLfVw5xkvWQjwaQLrcD6gNUC0Z0QsdtekwXvF2Chp2KwxDjzOci9FcxOvBS7FfQR9X qVyzBAL2CoiZMROEh1gr91j3TTCuY+JWXI6hPGf97R4sYmTaT+obA2ngj3m7xcv6XBBTSWLUrOxeR beyCfXaDCrA5yY5qlxF6XAip3eEV2ZQAHa0EA6JK0qxCq3RtOsX9Mj9HDSXaIl4tNSOKkqfPkSfDD AD3njDxcxRBQ6nglQepg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ntmev-00ATPW-La; Wed, 25 May 2022 08:46:25 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ntmeq-00ATOU-Tg for linux-arm-kernel@lists.infradead.org; Wed, 25 May 2022 08:46:23 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 39E721FB; Wed, 25 May 2022 01:46:19 -0700 (PDT) Received: from FVFF77S0Q05N (unknown [10.57.0.228]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1F6C53F73D; Wed, 25 May 2022 01:46:17 -0700 (PDT) Date: Wed, 25 May 2022 09:46:13 +0100 From: Mark Rutland To: Mark Brown Cc: Catalin Marinas , Will Deacon , Marc Zyngier , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH RFC] arm64/sysregs: Align field names in ID_AA64DFR0_EL1 with architecture Message-ID: References: <20220518145750.251387-1-broonie@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220518145750.251387-1-broonie@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220525_014621_099033_14CA8508 X-CRM114-Status: GOOD ( 34.60 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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 > --- > > 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 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel