| Message ID | 20200414213114.2378-4-will@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Relax sanity checking for mismatched AArch32 EL1 | expand |
Hi Will, On 2020-04-14 22:31, Will Deacon wrote: > Although 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. > > Signed-off-by: Will Deacon <will@kernel.org> Definitely an improvement over the current situation, as the direct read of ID_AA64PFR0 was always a bit dodgy. Given that I'm pretty sure these new braindead SoCs are going to run an older version of the kernel, should we Cc stable for this? Otherwise: Acked-by: Marc Zyngier <maz@kernel.org> M.
On 04/14/2020 10:31 PM, Will Deacon wrote: > Although 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. > > Signed-off-by: Will Deacon <will@kernel.org> One of the other important missing sanity check for KVM is the VMID width check. I will code something up. Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
On Wed, Apr 15, 2020 at 11:13:54AM +0100, Suzuki K Poulose wrote: > On 04/14/2020 10:31 PM, Will Deacon wrote: > > Although 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. > > > > Signed-off-by: Will Deacon <will@kernel.org> > > One of the other important missing sanity check for KVM is the VMID width > check. I will code something up. Cheers! Do we handle things like the IPA size already? Will
On 04/15/2020 11:14 AM, Will Deacon wrote: > On Wed, Apr 15, 2020 at 11:13:54AM +0100, Suzuki K Poulose wrote: >> On 04/14/2020 10:31 PM, Will Deacon wrote: >>> Although 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. >>> >>> Signed-off-by: Will Deacon <will@kernel.org> >> >> One of the other important missing sanity check for KVM is the VMID width >> check. I will code something up. > > Cheers! Do we handle things like the IPA size already? Good point. No, we don't. I will include this too. Cheers Suzuki
On Wed, 15 Apr 2020 14:15:51 +0100 Suzuki K Poulose <suzuki.poulose@arm.com> wrote: > On 04/15/2020 11:14 AM, Will Deacon wrote: > > On Wed, Apr 15, 2020 at 11:13:54AM +0100, Suzuki K Poulose wrote: > >> On 04/14/2020 10:31 PM, Will Deacon wrote: > >>> Although 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. > >>> > >>> Signed-off-by: Will Deacon <will@kernel.org> > >> > >> One of the other important missing sanity check for KVM is the VMID width > >> check. I will code something up. > > > > Cheers! Do we handle things like the IPA size already? > > Good point. No, we don't. I will include this too. There is also the question of the ARMv8.5-GTG extension. I have a patch that treats it as non-strict, but that approach would fail with KVM if we online a late CPU without support for the right page size at S2. M.
Hi Marc, On Wed, Apr 15, 2020 at 09:55:57AM +0100, Marc Zyngier wrote: > On 2020-04-14 22:31, Will Deacon wrote: > > Although 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. > > > > Signed-off-by: Will Deacon <will@kernel.org> > > Definitely an improvement over the current situation, as the direct read > of ID_AA64PFR0 was always a bit dodgy. Given that I'm pretty sure these new > braindead SoCs are going to run an older version of the kernel, should we > Cc stable for this? I don't think there's a real need for -stable given that we do at least taint the kernel. That's likely to annoy vendors enough to backport this themselves ;) Will
On 04/15/2020 02:22 PM, Marc Zyngier wrote: > On Wed, 15 Apr 2020 14:15:51 +0100 > Suzuki K Poulose <suzuki.poulose@arm.com> wrote: > >> On 04/15/2020 11:14 AM, Will Deacon wrote: >>> On Wed, Apr 15, 2020 at 11:13:54AM +0100, Suzuki K Poulose wrote: >>>> On 04/14/2020 10:31 PM, Will Deacon wrote: >>>>> Although 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. >>>>> >>>>> Signed-off-by: Will Deacon <will@kernel.org> >>>> >>>> One of the other important missing sanity check for KVM is the VMID width >>>> check. I will code something up. >>> >>> Cheers! Do we handle things like the IPA size already? >> >> Good point. No, we don't. I will include this too. > > There is also the question of the ARMv8.5-GTG extension. I have a patch > that treats it as non-strict, but that approach would fail with KVM if > we online a late CPU without support for the right page size at S2. Good point. Again this can be added to the list of checks performed on the hot-plugged CPUs along with IPA, VMID width. Cheers Suzuki
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h index 8eb5a088ae65..c54c674e6c21 100644 --- a/arch/arm64/include/asm/cpucaps.h +++ b/arch/arm64/include/asm/cpucaps.h @@ -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 */ diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index 139cd24c181b..3d8e2f0347c4 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -653,6 +653,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 diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index b143f8bc6c52..838fe5cc8d7e 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -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, diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c index 30b7ea680f66..102e5c4e01a0 100644 --- a/arch/arm64/kvm/reset.c +++ b/arch/arm64/kvm/reset.c @@ -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 {
Although 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. Signed-off-by: Will Deacon <will@kernel.org> --- 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(-)