Relax sanity checking for mismatched AArch32 EL1
Will Deacon
maintainerv2 cover Relax sanity checking for mismatched AArch32 EL1
Hi folks,
This is version two of the patches I previously posted here:
https://lore.kernel.org/lkml/[email protected]/
Changes since v1 include:
* Typo fixes
* Added comment to update_32bit_cpu_features() callsite regarding sanitisation
* Extended comment in final patch to mention KVM
* Add acks/tested-bys
Cheers,
Will
--->8
Sai Prakash Ranjan (1):
arm64: cpufeature: Relax check for IESB support
Will Deacon (7):
arm64: cpufeature: Spell out register fields for ID_ISAR4 and ID_PFR1
arm64: cpufeature: Add CPU capability for AArch32 EL1 support
arm64: cpufeature: Remove redundant call to id_aa64pfr0_32bit_el0()
arm64: cpufeature: Factor out checking of AArch32 features
arm64: cpufeature: Relax AArch32 system checks if EL1 is 64-bit only
arm64: cpufeature: Relax checks for AArch32 support at EL[0-2]
arm64: cpufeature: Add an overview comment for the cpufeature
framework
arch/arm64/include/asm/cpucaps.h | 3 +- arch/arm64/include/asm/cpufeature.h | 7 + arch/arm64/include/asm/sysreg.h | 18 ++ arch/arm64/kernel/cpufeature.c | 247 +++++++++++++++++++++------- arch/arm64/kvm/reset.c | 12 +- 5 files changed, 217 insertions(+), 70 deletions(-)
Signature · 1 line
2.26.1.301.g55bc3eb7cb9-goog
Will Deacon
maintainerPATCH v2 1/8 arm64: cpufeature: Relax check for IESB support
From: Sai Prakash Ranjan <[email protected]>
We don't care if IESB is supported or not as we always set SCTLR_ELx.IESB and, if it works, that's really great.
Relax the ID_AA64MMFR2.IESB cpufeature check so that we don't warn and taint if it's mismatched.
--- arch/arm64/kernel/cpufeature.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/cpufeature.c +1 -1
@@ -247,7 +247,7 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr2[] = {ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_AA64MMFR2_FWB_SHIFT ,4 ,0 ),ARM64_FTR_BITS (FTR_VISIBLE ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_AA64MMFR2_AT_SHIFT ,4 ,0 ),ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_AA64MMFR2_LVA_SHIFT ,4 ,0 ), -ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_AA64MMFR2_IESB_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_NONSTRICT ,FTR_LOWER_SAFE ,ID_AA64MMFR2_IESB_SHIFT ,4 ,0 ),ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_AA64MMFR2_LSM_SHIFT ,4 ,0 ),ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_AA64MMFR2_UAO_SHIFT ,4 ,0 ), ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_CNP_SHIFT, 4, 0),
Signature · 1 line
2.26.1.301.g55bc3eb7cb9-goog
Will Deacon
maintainerPATCH v2 2/8 arm64: cpufeature: Spell out register fields for ID_ISAR4 and ID_PFR1
In reply to Will DeaconIn preparation for runtime updates to the strictness of some AArch32 features, spell out the register fields for ID_ISAR4 and ID_PFR1 to make things clearer to read. Note that this isn't functionally necessary, as the feature arrays themselves are not modified dynamically and remain 'const'.
--- arch/arm64/include/asm/sysreg.h | 17 +++++++++++++++++ arch/arm64/kernel/cpufeature.c | 28 ++++++++++++++++++++++++++-- 2 files changed, 43 insertions(+), 2 deletions(-)
arch/arm64/include/asm/sysreg.h +17 -0
@@ -752,6 +752,15 @@#define ID_DFR0_PERFMON_8_1 0x4 +#define ID_ISAR4_SWP_FRAC_SHIFT 28 +#define ID_ISAR4_PSR_M_SHIFT 24 +#define ID_ISAR4_SYNCH_PRIM_FRAC_SHIFT 20 +#define ID_ISAR4_BARRIER_SHIFT 16 +#define ID_ISAR4_SMC_SHIFT 12 +#define ID_ISAR4_WRITEBACK_SHIFT 8 +#define ID_ISAR4_WITHSHIFTS_SHIFT 4 +#define ID_ISAR4_UNPRIV_SHIFT 0 +#define ID_ISAR5_RDM_SHIFT 24#define ID_ISAR5_CRC32_SHIFT 16#define ID_ISAR5_SHA2_SHIFT 12
@@ -785,6 +794,14 @@#define MVFR1_FPDNAN_SHIFT 4#define MVFR1_FPFTZ_SHIFT 0 +#define ID_PFR1_GIC_SHIFT 28 +#define ID_PFR1_VIRT_FRAC_SHIFT 24 +#define ID_PFR1_SEC_FRAC_SHIFT 20 +#define ID_PFR1_GENTIMER_SHIFT 16 +#define ID_PFR1_VIRTUALIZATION_SHIFT 12 +#define ID_PFR1_MPROGMOD_SHIFT 8 +#define ID_PFR1_SECURITY_SHIFT 4 +#define ID_PFR1_PROGMOD_SHIFT 0#define ID_AA64MMFR0_TGRAN4_SHIFT 28#define ID_AA64MMFR0_TGRAN64_SHIFT 24
arch/arm64/kernel/cpufeature.c +26 -2
@@ -332,6 +332,18 @@ static const struct arm64_ftr_bits ftr_id_mmfr4[] = { ARM64_FTR_END, }; +static const struct arm64_ftr_bits ftr_id_isar4[] = { +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_ISAR4_SWP_FRAC_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_ISAR4_PSR_M_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_ISAR4_SYNCH_PRIM_FRAC_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_ISAR4_BARRIER_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_ISAR4_SMC_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_ISAR4_WRITEBACK_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_ISAR4_WITHSHIFTS_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_ISAR4_UNPRIV_SHIFT ,4 ,0 ), +ARM64_FTR_END , +}; +static const struct arm64_ftr_bits ftr_id_isar6[] = {ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_ISAR6_I8MM_SHIFT ,4 ,0 ),ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_ISAR6_BF16_SHIFT ,4 ,0 ),
@@ -351,6 +363,18 @@ static const struct arm64_ftr_bits ftr_id_pfr0[] = { ARM64_FTR_END, }; +static const struct arm64_ftr_bits ftr_id_pfr1[] = { +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_PFR1_GIC_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_PFR1_VIRT_FRAC_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_PFR1_SEC_FRAC_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_PFR1_GENTIMER_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_PFR1_VIRTUALIZATION_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_PFR1_MPROGMOD_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_PFR1_SECURITY_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_PFR1_PROGMOD_SHIFT ,4 ,0 ), +ARM64_FTR_END , +}; +static const struct arm64_ftr_bits ftr_id_dfr0[] = {ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,28 ,4 ,0 ),S_ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,24 ,4 ,0xf ),/* PerfMon */
@@ -411,7 +435,7 @@ static const struct __ftr_reg_entry {/* Op1 = 0, CRn = 0, CRm = 1 */ ARM64_FTR_REG (SYS_ID_PFR0_EL1 , ftr_id_pfr0), -ARM64_FTR_REG (SYS_ID_PFR1_EL1 , ftr_generic_32bits), +ARM64_FTR_REG (SYS_ID_PFR1_EL1 , ftr_id_pfr1),ARM64_FTR_REG (SYS_ID_DFR0_EL1 , ftr_id_dfr0),ARM64_FTR_REG (SYS_ID_MMFR0_EL1 , ftr_id_mmfr0), ARM64_FTR_REG(SYS_ID_MMFR1_EL1, ftr_generic_32bits),
@@ -423,7 +447,7 @@ static const struct __ftr_reg_entry {ARM64_FTR_REG (SYS_ID_ISAR1_EL1 , ftr_generic_32bits),ARM64_FTR_REG (SYS_ID_ISAR2_EL1 , ftr_generic_32bits),ARM64_FTR_REG (SYS_ID_ISAR3_EL1 , ftr_generic_32bits), -ARM64_FTR_REG (SYS_ID_ISAR4_EL1 , ftr_generic_32bits), +ARM64_FTR_REG (SYS_ID_ISAR4_EL1 , ftr_id_isar4),ARM64_FTR_REG (SYS_ID_ISAR5_EL1 , ftr_id_isar5),ARM64_FTR_REG (SYS_ID_MMFR4_EL1 , ftr_id_mmfr4), ARM64_FTR_REG(SYS_ID_ISAR6_EL1, ftr_id_isar6),
Signature · 6 lines
2.26.1.301.g55bc3eb7cb9-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Will Deacon
maintainerPATCH v2 3/8 arm64: cpufeature: Add CPU capability for AArch32 EL1 support
In reply to Will DeaconAlthough we emit a "SANITY CHECK" warning and taint the kernel if we detect a CPU mismatch for AArch32 support at EL1, we still online the CPU with disastrous consequences for any running 32-bit VMs.
Introduce a capability for AArch32 support at EL1 so that late onlining of incompatible CPUs is forbidden.
--- arch/arm64/include/asm/cpucaps.h | 3 ++- arch/arm64/include/asm/sysreg.h | 1 + arch/arm64/kernel/cpufeature.c | 12 ++++++++++++ arch/arm64/kvm/reset.c | 12 ++---------- 4 files changed, 17 insertions(+), 11 deletions(-)
arch/arm64/include/asm/cpucaps.h +2 -1
@@ -61,7 +61,8 @@#define ARM64_HAS_AMU_EXTN 51#define ARM64_HAS_ADDRESS_AUTH 52#define ARM64_HAS_GENERIC_AUTH 53 +#define ARM64_HAS_32BIT_EL1 54 -#define ARM64_NCAPS 54 +#define ARM64_NCAPS 55#endif /* __ASM_CPUCAPS_H */
arch/arm64/include/asm/sysreg.h +1 -0
@@ -655,6 +655,7 @@#define ID_AA64PFR0_ASIMD_NI 0xf#define ID_AA64PFR0_ASIMD_SUPPORTED 0x0#define ID_AA64PFR0_EL1_64BIT_ONLY 0x1 +#define ID_AA64PFR0_EL1_32BIT_64BIT 0x2#define ID_AA64PFR0_EL0_64BIT_ONLY 0x1#define ID_AA64PFR0_EL0_32BIT_64BIT 0x2
arch/arm64/kernel/cpufeature.c +12 -0
@@ -1535,6 +1535,18 @@ static const struct arm64_cpu_capabilities arm64_features[] = { .field_pos = ID_AA64PFR0_EL0_SHIFT, .min_field_value = ID_AA64PFR0_EL0_32BIT_64BIT, }, +#ifdef CONFIG_KVM + { + .desc = "32-bit EL1 Support", + .capability = ARM64_HAS_32BIT_EL1, + .type = ARM64_CPUCAP_SYSTEM_FEATURE, + .matches = has_cpuid_feature, + .sys_reg = SYS_ID_AA64PFR0_EL1, + .sign = FTR_UNSIGNED, + .field_pos = ID_AA64PFR0_EL1_SHIFT, + .min_field_value = ID_AA64PFR0_EL1_32BIT_64BIT, + }, +#endif { .desc = "Kernel page table isolation (KPTI)", .capability = ARM64_UNMAP_KERNEL_AT_EL0,
arch/arm64/kvm/reset.c +2 -10
@@ -46,14 +46,6 @@ static const struct kvm_regs default_regs_reset32 = { PSR_AA32_I_BIT | PSR_AA32_F_BIT), }; -static bool cpu_has_32bit_el1 (void ) -{ -u64 pfr0; - - pfr0 =read_sanitised_ftr_reg (SYS_ID_AA64PFR0_EL1 ); -return !!(pfr0 &0x20 ); -} -/** * kvm_arch_vm_ioctl_check_extension *
@@ -66,7 +58,7 @@ int kvm_arch_vm_ioctl_check_extension(struct kvm *kvm, long ext)switch (ext) {case KVM_CAP_ARM_EL1_32BIT : - r =cpu_has_32bit_el1 (); + r =cpus_have_const_cap (ARM64_HAS_32BIT_EL1 );break ;case KVM_CAP_GUEST_DEBUG_HW_BPS : r =get_num_brps ();
@@ -288,7 +280,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)switch (vcpu->arch.target) {default :if (test_bit (KVM_ARM_VCPU_EL1_32BIT , vcpu->arch.features)) { -if (!cpu_has_32bit_el1 ()) +if (!cpus_have_const_cap (ARM64_HAS_32BIT_EL1 )) goto out; cpu_reset = &default_regs_reset32; }else {
Signature · 1 line
2.26.1.301.g55bc3eb7cb9-goog
Will Deacon
maintainerPATCH v2 4/8 arm64: cpufeature: Remove redundant call to id_aa64pfr0_32bit_el0()
In reply to Will DeaconThere's no need to call id_aa64pfr0_32bit_el0() twice because the sanitised value of ID_AA64PFR0_EL1 has already been updated for the CPU being onlined.
Remove the redundant function call.
--- arch/arm64/kernel/cpufeature.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
arch/arm64/kernel/cpufeature.c +1 -3
@@ -792,9 +792,7 @@ void update_cpu_features(int cpu,* If we have AArch32, we care about 32-bit features for compat. * If the system doesn't support AArch32, don't update them. */ -if (id_aa64pfr0_32bit_el0 (read_sanitised_ftr_reg (SYS_ID_AA64PFR0_EL1 )) && -id_aa64pfr0_32bit_el0 (info->reg_id_aa64pfr0)) { - +if (id_aa64pfr0_32bit_el0 (read_sanitised_ftr_reg (SYS_ID_AA64PFR0_EL1 ))) { taint |=check_update_ftr_reg (SYS_ID_DFR0_EL1 , cpu, info->reg_id_dfr0, boot->reg_id_dfr0); taint |= check_update_ftr_reg(SYS_ID_ISAR0_EL1, cpu,
Signature · 6 lines
2.26.1.301.g55bc3eb7cb9-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Will Deacon
maintainerPATCH v2 5/8 arm64: cpufeature: Factor out checking of AArch32 features
In reply to Will Deaconupdate_cpu_features() is pretty large, so split out the checking of the AArch32 features into a separate function and call it after checking the AArch64 features.
--- arch/arm64/kernel/cpufeature.c | 112 +++++++++++++++++++-------------- 1 file changed, 65 insertions(+), 47 deletions(-)
arch/arm64/kernel/cpufeature.c +65 -47
@@ -715,6 +715,65 @@ static int check_update_ftr_reg(u32 sys_id, int cpu, u64 val, u64 boot)return 1 ; } +static int update_32bit_cpu_features (int cpu,struct cpuinfo_arm64 *info, +struct cpuinfo_arm64 *boot) +{ +int taint =0 ; +u64 pfr0 =read_sanitised_ftr_reg (SYS_ID_AA64PFR0_EL1 ); + +/* +* If we don't have AArch32 at all then skip the checks entirely +* as the register values may be UNKNOWN and we're not going to be +* using them for anything. +*/ +if (!id_aa64pfr0_32bit_el0 (pfr0)) +return taint; + + taint |=check_update_ftr_reg (SYS_ID_DFR0_EL1 , cpu, + info->reg_id_dfr0, boot->reg_id_dfr0); + taint |=check_update_ftr_reg (SYS_ID_ISAR0_EL1 , cpu, + info->reg_id_isar0, boot->reg_id_isar0); + taint |=check_update_ftr_reg (SYS_ID_ISAR1_EL1 , cpu, + info->reg_id_isar1, boot->reg_id_isar1); + taint |=check_update_ftr_reg (SYS_ID_ISAR2_EL1 , cpu, + info->reg_id_isar2, boot->reg_id_isar2); + taint |=check_update_ftr_reg (SYS_ID_ISAR3_EL1 , cpu, + info->reg_id_isar3, boot->reg_id_isar3); + taint |=check_update_ftr_reg (SYS_ID_ISAR4_EL1 , cpu, + info->reg_id_isar4, boot->reg_id_isar4); + taint |=check_update_ftr_reg (SYS_ID_ISAR5_EL1 , cpu, + info->reg_id_isar5, boot->reg_id_isar5); + taint |=check_update_ftr_reg (SYS_ID_ISAR6_EL1 , cpu, + info->reg_id_isar6, boot->reg_id_isar6); + +/* +* Regardless of the value of the AuxReg field, the AIFSR, ADFSR, and +* ACTLR formats could differ across CPUs and therefore would have to +* be trapped for virtualization anyway. +*/ + taint |=check_update_ftr_reg (SYS_ID_MMFR0_EL1 , cpu, + info->reg_id_mmfr0, boot->reg_id_mmfr0); + taint |=check_update_ftr_reg (SYS_ID_MMFR1_EL1 , cpu, + info->reg_id_mmfr1, boot->reg_id_mmfr1); + taint |=check_update_ftr_reg (SYS_ID_MMFR2_EL1 , cpu, + info->reg_id_mmfr2, boot->reg_id_mmfr2); + taint |=check_update_ftr_reg (SYS_ID_MMFR3_EL1 , cpu, + info->reg_id_mmfr3, boot->reg_id_mmfr3); + taint |=check_update_ftr_reg (SYS_ID_PFR0_EL1 , cpu, + info->reg_id_pfr0, boot->reg_id_pfr0); + taint |=check_update_ftr_reg (SYS_ID_PFR1_EL1 , cpu, + info->reg_id_pfr1, boot->reg_id_pfr1); + taint |=check_update_ftr_reg (SYS_MVFR0_EL1 , cpu, + info->reg_mvfr0, boot->reg_mvfr0); + taint |=check_update_ftr_reg (SYS_MVFR1_EL1 , cpu, + info->reg_mvfr1, boot->reg_mvfr1); + taint |=check_update_ftr_reg (SYS_MVFR2_EL1 , cpu, + info->reg_mvfr2, boot->reg_mvfr2); + +return taint; +} + +/* * Update system wide CPU feature registers with the values from a * non-boot CPU. Also performs SANITY checks to make sure that there
@@ -788,53 +847,6 @@ void update_cpu_features(int cpu, taint |=check_update_ftr_reg (SYS_ID_AA64ZFR0_EL1 , cpu, info->reg_id_aa64zfr0, boot->reg_id_aa64zfr0); -/* -* If we have AArch32, we care about 32-bit features for compat. -* If the system doesn't support AArch32, don't update them. -*/ -if (id_aa64pfr0_32bit_el0 (read_sanitised_ftr_reg (SYS_ID_AA64PFR0_EL1 ))) { - taint |=check_update_ftr_reg (SYS_ID_DFR0_EL1 , cpu, - info->reg_id_dfr0, boot->reg_id_dfr0); - taint |=check_update_ftr_reg (SYS_ID_ISAR0_EL1 , cpu, - info->reg_id_isar0, boot->reg_id_isar0); - taint |=check_update_ftr_reg (SYS_ID_ISAR1_EL1 , cpu, - info->reg_id_isar1, boot->reg_id_isar1); - taint |=check_update_ftr_reg (SYS_ID_ISAR2_EL1 , cpu, - info->reg_id_isar2, boot->reg_id_isar2); - taint |=check_update_ftr_reg (SYS_ID_ISAR3_EL1 , cpu, - info->reg_id_isar3, boot->reg_id_isar3); - taint |=check_update_ftr_reg (SYS_ID_ISAR4_EL1 , cpu, - info->reg_id_isar4, boot->reg_id_isar4); - taint |=check_update_ftr_reg (SYS_ID_ISAR5_EL1 , cpu, - info->reg_id_isar5, boot->reg_id_isar5); - taint |=check_update_ftr_reg (SYS_ID_ISAR6_EL1 , cpu, - info->reg_id_isar6, boot->reg_id_isar6); - -/* -* Regardless of the value of the AuxReg field, the AIFSR, ADFSR, and -* ACTLR formats could differ across CPUs and therefore would have to -* be trapped for virtualization anyway. -*/ - taint |=check_update_ftr_reg (SYS_ID_MMFR0_EL1 , cpu, - info->reg_id_mmfr0, boot->reg_id_mmfr0); - taint |=check_update_ftr_reg (SYS_ID_MMFR1_EL1 , cpu, - info->reg_id_mmfr1, boot->reg_id_mmfr1); - taint |=check_update_ftr_reg (SYS_ID_MMFR2_EL1 , cpu, - info->reg_id_mmfr2, boot->reg_id_mmfr2); - taint |=check_update_ftr_reg (SYS_ID_MMFR3_EL1 , cpu, - info->reg_id_mmfr3, boot->reg_id_mmfr3); - taint |=check_update_ftr_reg (SYS_ID_PFR0_EL1 , cpu, - info->reg_id_pfr0, boot->reg_id_pfr0); - taint |=check_update_ftr_reg (SYS_ID_PFR1_EL1 , cpu, - info->reg_id_pfr1, boot->reg_id_pfr1); - taint |=check_update_ftr_reg (SYS_MVFR0_EL1 , cpu, - info->reg_mvfr0, boot->reg_mvfr0); - taint |=check_update_ftr_reg (SYS_MVFR1_EL1 , cpu, - info->reg_mvfr1, boot->reg_mvfr1); - taint |=check_update_ftr_reg (SYS_MVFR2_EL1 , cpu, - info->reg_mvfr2, boot->reg_mvfr2); - } -if (id_aa64pfr0_sve (info->reg_id_aa64pfr0)) { taint |=check_update_ftr_reg (SYS_ZCR_EL1 , cpu, info->reg_zcr, boot->reg_zcr);
@@ -845,6 +857,12 @@ void update_cpu_features(int cpu,sve_update_vq_map (); } +/* +* This relies on a sanitised view of the AArch64 ID registers +* (e.g. SYS_ID_AA64PFR0_EL1), so we call it last. +*/ + taint |=update_32bit_cpu_features (cpu, info, boot); +/* * Mismatched CPU features are a recipe for disaster. Don't even * pretend to support them.
Signature · 1 line
2.26.1.301.g55bc3eb7cb9-goog
Will Deacon
maintainerPATCH v2 6/8 arm64: cpufeature: Relax AArch32 system checks if EL1 is 64-bit only
In reply to Will DeaconIf AArch32 is not supported at EL1, the AArch32 feature register fields no longer advertise support for some system features:
* ISAR4.SMC
* PFR1.{Virt_frac, Sec_frac, Virtualization, Security, ProgMod}
In which case, we don't need to emit "SANITY CHECK" failures for all of them.
Add logic to relax the strictness of individual feature register fields at runtime and use this for the fields above if 32-bit EL1 is not supported.
--- arch/arm64/include/asm/cpufeature.h | 7 ++++++ arch/arm64/kernel/cpufeature.c | 33 ++++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-)
arch/arm64/include/asm/cpufeature.h +7 -0
@@ -551,6 +551,13 @@ static inline bool id_aa64mmfr0_mixed_endian_el0(u64 mmfr0)cpuid_feature_extract_unsigned_field (mmfr0,ID_AA64MMFR0_BIGENDEL0_SHIFT ) ==0x1 ; } +static inline bool id_aa64pfr0_32bit_el1 (u64 pfr0) +{ +u32 val =cpuid_feature_extract_unsigned_field (pfr0,ID_AA64PFR0_EL1_SHIFT ); + +return val ==ID_AA64PFR0_EL1_32BIT_64BIT ; +} +static inline bool id_aa64pfr0_32bit_el0 (u64 pfr0) {u32 val =cpuid_feature_extract_unsigned_field (pfr0,ID_AA64PFR0_EL0_SHIFT );
arch/arm64/kernel/cpufeature.c +32 -1
@@ -715,6 +715,25 @@ static int check_update_ftr_reg(u32 sys_id, int cpu, u64 val, u64 boot)return 1 ; } +static void relax_cpu_ftr_reg (u32 sys_id,int field) +{ +const struct arm64_ftr_bits *ftrp; +struct arm64_ftr_reg *regp =get_arm64_ftr_reg (sys_id); + +if (WARN_ON (!regp)) +return ; + +for (ftrp = regp->ftr_bits; ftrp->width; ftrp++) { +if (ftrp->shift == field) { + regp->strict_mask &= ~arm64_ftr_mask (ftrp); +break ; + } + } + +/* Bogus field? */ +WARN_ON (!ftrp->width); +} +static int update_32bit_cpu_features (int cpu,struct cpuinfo_arm64 *info,struct cpuinfo_arm64 *boot) {
@@ -729,6 +748,19 @@ static int update_32bit_cpu_features(int cpu, struct cpuinfo_arm64 *info,if (!id_aa64pfr0_32bit_el0 (pfr0))return taint; +/* +* If we don't have AArch32 at EL1, then relax the strictness of +* EL1-dependent register fields to avoid spurious sanity check fails. +*/ +if (!id_aa64pfr0_32bit_el1 (pfr0)) { +relax_cpu_ftr_reg (SYS_ID_ISAR4_EL1 ,ID_ISAR4_SMC_SHIFT ); +relax_cpu_ftr_reg (SYS_ID_PFR1_EL1 ,ID_PFR1_VIRT_FRAC_SHIFT ); +relax_cpu_ftr_reg (SYS_ID_PFR1_EL1 ,ID_PFR1_SEC_FRAC_SHIFT ); +relax_cpu_ftr_reg (SYS_ID_PFR1_EL1 ,ID_PFR1_VIRTUALIZATION_SHIFT ); +relax_cpu_ftr_reg (SYS_ID_PFR1_EL1 ,ID_PFR1_SECURITY_SHIFT ); +relax_cpu_ftr_reg (SYS_ID_PFR1_EL1 ,ID_PFR1_PROGMOD_SHIFT ); + } + taint |=check_update_ftr_reg (SYS_ID_DFR0_EL1 , cpu, info->reg_id_dfr0, boot->reg_id_dfr0); taint |= check_update_ftr_reg(SYS_ID_ISAR0_EL1, cpu,
@@ -773,7 +805,6 @@ static int update_32bit_cpu_features(int cpu, struct cpuinfo_arm64 *info,return taint; } -/* * Update system wide CPU feature registers with the values from a * non-boot CPU. Also performs SANITY checks to make sure that there
Signature · 1 line
2.26.1.301.g55bc3eb7cb9-goog
Will Deacon
maintainerPATCH v2 7/8 arm64: cpufeature: Relax checks for AArch32 support at EL[0-2]
In reply to Will DeaconWe don't need to be quite as strict about mismatched AArch32 support, which is good because the friendly hardware folks have been busy mismatching this to their hearts' content.
* We don't care about EL2 or EL3 (there are silly comments concerning the latter, so remove those)
* EL1 support is gated by the ARM64_HAS_32BIT_EL1 capability and handled gracefully when a mismatch occurs
* EL0 support is gated by the ARM64_HAS_32BIT_EL0 capability and handled gracefully when a mismatch occurs
Relax the AArch32 checks to FTR_NONSTRICT.
--- arch/arm64/kernel/cpufeature.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
arch/arm64/kernel/cpufeature.c +3 -7
@@ -172,11 +172,10 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = {ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_AA64PFR0_GIC_SHIFT ,4 ,0 ),S_ARM64_FTR_BITS (FTR_VISIBLE ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_AA64PFR0_ASIMD_SHIFT ,4 ,ID_AA64PFR0_ASIMD_NI ),S_ARM64_FTR_BITS (FTR_VISIBLE ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_AA64PFR0_FP_SHIFT ,4 ,ID_AA64PFR0_FP_NI ), -/* Linux doesn't care about the EL3 */ ARM64_FTR_BITS (FTR_HIDDEN ,FTR_NONSTRICT ,FTR_LOWER_SAFE ,ID_AA64PFR0_EL3_SHIFT ,4 ,0 ), -ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_AA64PFR0_EL2_SHIFT ,4 ,0 ), -ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_AA64PFR0_EL1_SHIFT ,4 ,ID_AA64PFR0_EL1_64BIT_ONLY ), -ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_AA64PFR0_EL0_SHIFT ,4 ,ID_AA64PFR0_EL0_64BIT_ONLY ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_NONSTRICT ,FTR_LOWER_SAFE ,ID_AA64PFR0_EL2_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_NONSTRICT ,FTR_LOWER_SAFE ,ID_AA64PFR0_EL1_SHIFT ,4 ,ID_AA64PFR0_EL1_64BIT_ONLY ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_NONSTRICT ,FTR_LOWER_SAFE ,ID_AA64PFR0_EL0_SHIFT ,4 ,ID_AA64PFR0_EL0_64BIT_ONLY ), ARM64_FTR_END, };
@@ -867,9 +866,6 @@ void update_cpu_features(int cpu, taint |=check_update_ftr_reg (SYS_ID_AA64MMFR2_EL1 , cpu, info->reg_id_aa64mmfr2, boot->reg_id_aa64mmfr2); -/* -* EL3 is not our concern. -*/ taint |=check_update_ftr_reg (SYS_ID_AA64PFR0_EL1 , cpu, info->reg_id_aa64pfr0, boot->reg_id_aa64pfr0); taint |= check_update_ftr_reg(SYS_ID_AA64PFR1_EL1, cpu,
Signature · 1 line
2.26.1.301.g55bc3eb7cb9-goog
Will Deacon
maintainerPATCH v2 8/8 arm64: cpufeature: Add an overview comment for the cpufeature framework
In reply to Will DeaconNow that Suzuki isn't within throwing distance, I thought I'd better add a rough overview comment to cpufeature.c so that it doesn't take me days to remember how it works next time.
--- arch/arm64/kernel/cpufeature.c | 50 ++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)
arch/arm64/kernel/cpufeature.c +50 -0
@@ -3,6 +3,56 @@* Contains CPU feature definitions * * Copyright (C) 2015 ARM Ltd. +* +* A note for the weary kernel hacker: the code here is confusing and hard to +* follow! That's partly because it's solving a nasty problem, but also because +* there's a little bit of over-abstraction that tends to obscure what's going +* on behind a maze of helper functions and macros. +* +* The basic problem is that hardware folks have started gluing together CPUs +* with distinct architectural features; in some cases even creating SoCs where +* user-visible instructions are available only on a subset of the available +* cores. We try to address this by snapshotting the feature registers of the +* boot CPU and comparing these with the feature registers of each secondary +* CPU when bringing them up. If there is a mismatch, then we update the +* snapshot state to indicate the lowest-common denominator of the feature, +* known as the "safe" value. This snapshot state can be queried to view the +* "sanitised" value of a feature register. +* +* The sanitised register values are used to decide which capabilities we +* have in the system. These may be in the form of traditional "hwcaps" +* advertised to userspace or internal "cpucaps" which are used to configure +* things like alternative patching and static keys. While a feature mismatch +* may result in a TAINT_CPU_OUT_OF_SPEC kernel taint, a capability mismatch +* may prevent a CPU from being onlined at all. +* +* Some implementation details worth remembering: +* +* - Mismatched features are *always* sanitised to a "safe" value, which +* usually indicates that the feature is not supported. +* +* - A mismatched feature marked with FTR_STRICT will cause a "SANITY CHECK" +* warning when onlining an offending CPU and the kernel will be tainted +* with TAINT_CPU_OUT_OF_SPEC. +* +* - Features marked as FTR_VISIBLE have their sanitised value visible to +* userspace. FTR_VISIBLE features in registers that are only visible +* to EL0 by trapping *must* have a corresponding HWCAP so that late +* onlining of CPUs cannot lead to features disappearing at runtime. +* +* - A "feature" is typically a 4-bit register field. A "capability" is the +* high-level description derived from the sanitised field value. +* +* - Read the Arm ARM (DDI 0487F.a) section D13.1.3 ("Principles of the ID +* scheme for fields in ID registers") to understand when feature fields +* may be signed or unsigned (FTR_SIGNED and FTR_UNSIGNED accordingly). +* +* - KVM exposes its own view of the feature registers to guest operating +* systems regardless of FTR_VISIBLE. This is typically driven from the +* sanitised register values to allow virtual CPUs to be migrated between +* arbitrary physical CPUs, but some features not present on the host are +* also advertised and emulated. Look at sys_reg_descs[] for the gory +* details. */ #define pr_fmt (fmt) "CPU features: " fmt
Signature · 6 lines
2.26.1.301.g55bc3eb7cb9-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 8/8] arm64: cpufeature: Add an overview comment for the cpufeature framework
Quoted 6 lines · Will Deacon
Now that Suzuki isn't within throwing distance, I thought I'd better add a rough overview comment to
cpufeature.cso that it doesn't take me days to remember how it works next time.Signed-off-by Will Deacon <[email protected]>---