| Message ID | 20210330173947.999859-4-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Dealing with VHE-only CPUs | expand |
On Tue, Mar 30, 2021 at 06:39:47PM +0100, Marc Zyngier wrote: > CPUs stuck in VHE mode need some additional care if the kernel > is compiled without CONFIG_ARM64_VHE. > > Treat this case as another version of a mismatched boot, and > prevent KVM from being initialised. The machine will boot in > some bizarre state, using TPIDR_EL1 instead of TPIDR_EL2, but > otherwise be functional. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > arch/arm64/include/asm/virt.h | 18 +++++++++++++----- > arch/arm64/kvm/va_layout.c | 9 +++++++++ > 2 files changed, 22 insertions(+), 5 deletions(-) Hmm, I think we definitely need _something_ here, but it's a bit annoying to put ourselves into this weird state just for the sake of one stupid machine. What if we dropped CONFIG_ARM64_VHE and made the VHE code unconditional instead? Is there a good reason to allow it to be disabled nowadays? Will
On 2021-04-07 22:18, Will Deacon wrote: > On Tue, Mar 30, 2021 at 06:39:47PM +0100, Marc Zyngier wrote: >> CPUs stuck in VHE mode need some additional care if the kernel >> is compiled without CONFIG_ARM64_VHE. >> >> Treat this case as another version of a mismatched boot, and >> prevent KVM from being initialised. The machine will boot in >> some bizarre state, using TPIDR_EL1 instead of TPIDR_EL2, but >> otherwise be functional. >> >> Signed-off-by: Marc Zyngier <maz@kernel.org> >> --- >> arch/arm64/include/asm/virt.h | 18 +++++++++++++----- >> arch/arm64/kvm/va_layout.c | 9 +++++++++ >> 2 files changed, 22 insertions(+), 5 deletions(-) > > Hmm, I think we definitely need _something_ here, but it's a bit > annoying > to put ourselves into this weird state just for the sake of one stupid > machine. Which is why I'm not keen at all on this patch, and I'm happy to see the machine die a painful death. We really can't be blamed for terminally buggy HW, which the M1 obviously is. > What if we dropped CONFIG_ARM64_VHE and made the VHE code unconditional > instead? Is there a good reason to allow it to be disabled nowadays? What do we do for the other camp, aka people really wanting to run nVHE without any command line parameter? I can't see why you'd want to do that, but hey, that's only me. I'd be quite happy to see CONFIG_ARM64_VHE go though. Let me know if you want a patch doing that instead. Thanks, M.
On Thu, Apr 08, 2021 at 11:31:42AM +0100, Marc Zyngier wrote: > On 2021-04-07 22:18, Will Deacon wrote: > > On Tue, Mar 30, 2021 at 06:39:47PM +0100, Marc Zyngier wrote: > > > CPUs stuck in VHE mode need some additional care if the kernel > > > is compiled without CONFIG_ARM64_VHE. > > > > > > Treat this case as another version of a mismatched boot, and > > > prevent KVM from being initialised. The machine will boot in > > > some bizarre state, using TPIDR_EL1 instead of TPIDR_EL2, but > > > otherwise be functional. > > > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > > --- > > > arch/arm64/include/asm/virt.h | 18 +++++++++++++----- > > > arch/arm64/kvm/va_layout.c | 9 +++++++++ > > > 2 files changed, 22 insertions(+), 5 deletions(-) > > > > Hmm, I think we definitely need _something_ here, but it's a bit > > annoying > > to put ourselves into this weird state just for the sake of one stupid > > machine. > > Which is why I'm not keen at all on this patch, and I'm happy to see > the machine die a painful death. We really can't be blamed for terminally > buggy HW, which the M1 obviously is. Perhaps, but it's not clear at all how the problem manifests and so we _will_ be blamed when things go wonky, especially as the real culprits all seem to be hiding... > > What if we dropped CONFIG_ARM64_VHE and made the VHE code unconditional > > instead? Is there a good reason to allow it to be disabled nowadays? > > What do we do for the other camp, aka people really wanting to run nVHE > without any command line parameter? I can't see why you'd want to do > that, but hey, that's only me. They can pass the command-line parameter, no? If we ever get CMDLINE_EXTEND support back, they could even add it in there! > I'd be quite happy to see CONFIG_ARM64_VHE go though. Let me know if you > want a patch doing that instead. Yes please. Will
On Thu, 08 Apr 2021 11:58:23 +0100, Will Deacon <will@kernel.org> wrote: > > On Thu, Apr 08, 2021 at 11:31:42AM +0100, Marc Zyngier wrote: > > On 2021-04-07 22:18, Will Deacon wrote: > > > On Tue, Mar 30, 2021 at 06:39:47PM +0100, Marc Zyngier wrote: > > > > CPUs stuck in VHE mode need some additional care if the kernel > > > > is compiled without CONFIG_ARM64_VHE. > > > > > > > > Treat this case as another version of a mismatched boot, and > > > > prevent KVM from being initialised. The machine will boot in > > > > some bizarre state, using TPIDR_EL1 instead of TPIDR_EL2, but > > > > otherwise be functional. > > > > > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > > > --- > > > > arch/arm64/include/asm/virt.h | 18 +++++++++++++----- > > > > arch/arm64/kvm/va_layout.c | 9 +++++++++ > > > > 2 files changed, 22 insertions(+), 5 deletions(-) > > > > > > Hmm, I think we definitely need _something_ here, but it's a bit > > > annoying > > > to put ourselves into this weird state just for the sake of one stupid > > > machine. > > > > Which is why I'm not keen at all on this patch, and I'm happy to see > > the machine die a painful death. We really can't be blamed for terminally > > buggy HW, which the M1 obviously is. > > Perhaps, but it's not clear at all how the problem manifests and so we > _will_ be blamed when things go wonky, especially as the real culprits > all seem to be hiding... > > > > What if we dropped CONFIG_ARM64_VHE and made the VHE code unconditional > > > instead? Is there a good reason to allow it to be disabled nowadays? > > > > What do we do for the other camp, aka people really wanting to run nVHE > > without any command line parameter? I can't see why you'd want to do > > that, but hey, that's only me. > > They can pass the command-line parameter, no? If we ever get CMDLINE_EXTEND > support back, they could even add it in there! My point was that they could do it *without* the parameter. No skin off my nose anyway. > > > I'd be quite happy to see CONFIG_ARM64_VHE go though. Let me know if you > > want a patch doing that instead. > > Yes please. Right, let me repost the series then. M.
diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h index 7379f35ae2c6..69bc4e26aa26 100644 --- a/arch/arm64/include/asm/virt.h +++ b/arch/arm64/include/asm/virt.h @@ -72,6 +72,11 @@ void __hyp_reset_vectors(void); DECLARE_STATIC_KEY_FALSE(kvm_protected_mode_initialized); +static inline bool is_kernel_in_hyp_mode(void) +{ + return read_sysreg(CurrentEL) == CurrentEL_EL2; +} + /* Reports the availability of HYP mode */ static inline bool is_hyp_mode_available(void) { @@ -83,6 +88,10 @@ static inline bool is_hyp_mode_available(void) static_branch_likely(&kvm_protected_mode_initialized)) return true; + /* Catch braindead CPUs */ + if (!IS_ENABLED(CONFIG_ARM64_VHE) && is_kernel_in_hyp_mode()) + return false; + return (__boot_cpu_mode[0] == BOOT_CPU_MODE_EL2 && __boot_cpu_mode[1] == BOOT_CPU_MODE_EL2); } @@ -98,12 +107,11 @@ static inline bool is_hyp_mode_mismatched(void) static_branch_likely(&kvm_protected_mode_initialized)) return false; - return __boot_cpu_mode[0] != __boot_cpu_mode[1]; -} + /* Catch braindead CPUs */ + if (!IS_ENABLED(CONFIG_ARM64_VHE) && is_kernel_in_hyp_mode()) + return true; -static inline bool is_kernel_in_hyp_mode(void) -{ - return read_sysreg(CurrentEL) == CurrentEL_EL2; + return __boot_cpu_mode[0] != __boot_cpu_mode[1]; } static __always_inline bool has_vhe(void) diff --git a/arch/arm64/kvm/va_layout.c b/arch/arm64/kvm/va_layout.c index 978301392d67..edb048654e00 100644 --- a/arch/arm64/kvm/va_layout.c +++ b/arch/arm64/kvm/va_layout.c @@ -156,6 +156,9 @@ void __init kvm_update_va_mask(struct alt_instr *alt, { int i; + if (!is_hyp_mode_available()) + return; + BUG_ON(nr_inst != 5); for (i = 0; i < nr_inst; i++) { @@ -191,6 +194,9 @@ void kvm_patch_vector_branch(struct alt_instr *alt, u64 addr; u32 insn; + if (!is_hyp_mode_available()) + return; + BUG_ON(nr_inst != 4); if (!cpus_have_const_cap(ARM64_SPECTRE_V3A) || WARN_ON_ONCE(has_vhe())) @@ -244,6 +250,9 @@ static void generate_mov_q(u64 val, __le32 *origptr, __le32 *updptr, int nr_inst { u32 insn, oinsn, rd; + if (!is_hyp_mode_available()) + return; + BUG_ON(nr_inst != 4); /* Compute target register */
CPUs stuck in VHE mode need some additional care if the kernel is compiled without CONFIG_ARM64_VHE. Treat this case as another version of a mismatched boot, and prevent KVM from being initialised. The machine will boot in some bizarre state, using TPIDR_EL1 instead of TPIDR_EL2, but otherwise be functional. Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/arm64/include/asm/virt.h | 18 +++++++++++++----- arch/arm64/kvm/va_layout.c | 9 +++++++++ 2 files changed, 22 insertions(+), 5 deletions(-)