Re: [PATCH 2/2] arm64: Cope with CPUs stuck in VHE mode

Will Deacon
2021-03-29T18:04:37Z

On Fri, Mar 26, 2021 at 11:20:18AM +0000, Marc Zyngier wrote:
> On Thu, 25 Mar 2021 19:33:19 +0000,
> Will Deacon <will@kernel.org> wrote:
> >
> > On Thu, Mar 25, 2021 at 12:47:21PM +0000, Marc Zyngier wrote:
> > > It seems that the CPUs part of the SoC known as Apple M1 have the
> > > terrible habit of being stuck with HCR_EL2.E2H==1, in violation
> > > of the architecture.
> > >
> > > Try and work around this deplorable state of affairs by detecting
> > > the stuck bit early and short-circuit the nVHE dance. Additional
> > > filtering code ensures that attempts at switching to nVHE from
> > > the command-line are also ignored.
> > >
> > > It is still unknown whether there are many more such nuggets
> > > to be found...
> > >
> > > Reported-by: Hector Martin <marcan@marcan.st>
> > > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > > ---
> > > arch/arm64/kernel/head.S | 33 +++++++++++++++++++++++++++---
> > > arch/arm64/kernel/hyp-stub.S | 15 ++++++++++----
> > > arch/arm64/kernel/idreg-override.c | 13 +++++++++++-
> > > 3 files changed, 53 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > > index 840bda1869e9..db2de5b8f3d9 100644
> > > --- a/arch/arm64/kernel/head.S
> > > +++ b/arch/arm64/kernel/head.S
> > > @@ -477,14 +477,13 @@ EXPORT_SYMBOL(kimage_vaddr)
> > > * booted in EL1 or EL2 respectively.
> > > */
> > > SYM_FUNC_START(init_kernel_el)
> > > - mov_q x0, INIT_SCTLR_EL1_MMU_OFF
> > > - msr sctlr_el1, x0
> > > -
> > > mrs x0, CurrentEL
> > > cmp x0, #CurrentEL_EL2
> > > b.eq init_el2
> > >
> > > SYM_INNER_LABEL(init_el1, SYM_L_LOCAL)
> > > + mov_q x0, INIT_SCTLR_EL1_MMU_OFF
> > > + msr sctlr_el1, x0
> > > isb
> > > mov_q x0, INIT_PSTATE_EL1
> > > msr spsr_el1, x0
> > > @@ -504,6 +503,34 @@ SYM_INNER_LABEL(init_el2, SYM_L_LOCAL)
> > > msr vbar_el2, x0
> > > isb
> > >
> > > + /*
> > > + * Fruity CPUs seem to have HCR_EL2.E2H set to RES1,
> > > + * making it impossible to start in nVHE mode. Is that
> > > + * compliant with the architecture? Absolutely not!
> > > + */
> > > + mrs x0, hcr_el2
> > > + and x0, x0, #HCR_E2H
> > > + cbz x0, 1f
> > > +
> > > + /* Switching to VHE requires a sane SCTLR_EL1 as a start */
> > > + mov_q x0, INIT_SCTLR_EL1_MMU_OFF
> > > + msr_s SYS_SCTLR_EL12, x0
> > > +
> > > + /*
> > > + * Force an eret into a helper "function", and let it return
> > > + * to our original caller... This makes sure that we have
> > > + * initialised the basic PSTATE state.
> > > + */
> > > + mov x0, #INIT_PSTATE_EL2
> > > + msr spsr_el1, x0
> > > + adr_l x0, stick_to_vhe
> > > + msr elr_el1, x0
> > > + eret
> >
> > What does this do if CONFIG_VHE=n on one of these CPUs?
>
> Interesting question. With this patch, it will actually boot, and
> behave just fine as long as you don't run a guest (the percpu offset
> being stored in TPIDR_EL1 will then be corrupted, though you may not
> even get there because of the sysreg renaming being unexpectedly
> active).
>
> I guess I could either make this code conditional on CONFIG_ARM64_VHE
> and let the machine crash early without a word, or have some later
> checks once the machine started booting. In the later case, displaying
> anything useful is going to be a challenge though (the odds of someone
> having a serial console on this box are close to nil). Pick your poison.

I think the best thing to do would be to fail to initialise KVM if the
kernel is stuck at EL2 but we don't have VHE support compiled in. Is that
do-able?

Will