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

Marc Zyngier
2021-03-26T11:21:54Z

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.

M.

--
Without deviation from the norm, progress is not possible.