* [PATCH 0/2] arm64: Dealing with VHE-only CPUs @ 2021-03-25 12:47 Marc Zyngier 2021-03-25 12:47 ` [PATCH 1/2] arm64: cpufeature: Allow early filtering of feature override Marc Zyngier 2021-03-25 12:47 ` [PATCH 2/2] arm64: Cope with CPUs stuck in VHE mode Marc Zyngier 0 siblings, 2 replies; 10+ messages in thread From: Marc Zyngier @ 2021-03-25 12:47 UTC (permalink / raw) To: linux-arm-kernel Cc: Hector Martin, Arnd Bergmann, Mark Rutland, Will Deacon, Catalin Marinas, kernel-team This short series is a rewrite of [1] after some reviewing from Will. It simplifies the esoteric "stay at EL2" path, and the feature override code where it actually belongs, allowing us to tell the user that no, nVHE isn't a thing on these system. This allows the infamous M1 to boot (tested on a M1 Mini). Hector, feel free to pull these two patches as a preamble to the next version of your series, though I'd expect this to go via the arm64 tree for obvious reasons. [1] https://lore.kernel.org/r/20210304213902.83903-2-marcan@marcan.st Marc Zyngier (2): arm64: cpufeature: Allow early filtering of feature override arm64: Cope with CPUs stuck in VHE mode arch/arm64/kernel/cpufeature.c | 6 ++++++ arch/arm64/kernel/head.S | 33 +++++++++++++++++++++++++++--- arch/arm64/kernel/hyp-stub.S | 15 ++++++++++---- arch/arm64/kernel/idreg-override.c | 26 ++++++++++++++++++++++- 4 files changed, 72 insertions(+), 8 deletions(-) -- 2.29.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] arm64: cpufeature: Allow early filtering of feature override 2021-03-25 12:47 [PATCH 0/2] arm64: Dealing with VHE-only CPUs Marc Zyngier @ 2021-03-25 12:47 ` Marc Zyngier 2021-03-25 19:27 ` Will Deacon 2021-03-25 12:47 ` [PATCH 2/2] arm64: Cope with CPUs stuck in VHE mode Marc Zyngier 1 sibling, 1 reply; 10+ messages in thread From: Marc Zyngier @ 2021-03-25 12:47 UTC (permalink / raw) To: linux-arm-kernel Cc: Hector Martin, Arnd Bergmann, Mark Rutland, Will Deacon, Catalin Marinas, kernel-team Some CPUs are broken enough that some overrides need to be rejected at the earliest opportunity. In some cases, that's right at cpu feature override time. Provide the necessary infrastructure to filter out overrides, and to report such filtered out overrides to the core cpufeature code. Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/arm64/kernel/cpufeature.c | 6 ++++++ arch/arm64/kernel/idreg-override.c | 13 +++++++++++++ 2 files changed, 19 insertions(+) diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 066030717a4c..6de15deaa912 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -809,6 +809,12 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new) reg->name, ftrp->shift + ftrp->width - 1, ftrp->shift, str, tmp); + } else if ((ftr_mask & reg->override->val) == ftr_mask) { + reg->override->val &= ~ftr_mask; + pr_warn("%s[%d:%d]: impossible override, ignored\n", + reg->name, + ftrp->shift + ftrp->width - 1, + ftrp->shift); } val = arm64_ftr_set_value(ftrp, val, ftr_new); diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c index 83f1c4b92095..be92fcd319a1 100644 --- a/arch/arm64/kernel/idreg-override.c +++ b/arch/arm64/kernel/idreg-override.c @@ -25,6 +25,7 @@ struct ftr_set_desc { struct { char name[FTR_DESC_FIELD_LEN]; u8 shift; + bool (*filter)(u64 val); } fields[]; }; @@ -124,6 +125,18 @@ static void __init match_options(const char *cmdline) if (find_field(cmdline, regs[i], f, &v)) continue; + /* + * If an override gets filtered out, advertise + * it by setting the value to 0xf, but + * clearing the mask... Yes, this is fragile. + */ + if (regs[i]->fields[f].filter && + !regs[i]->fields[f].filter(v)) { + regs[i]->override->val |= mask; + regs[i]->override->mask &= ~mask; + continue; + } + regs[i]->override->val &= ~mask; regs[i]->override->val |= (v << shift) & mask; regs[i]->override->mask |= mask; -- 2.29.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] arm64: cpufeature: Allow early filtering of feature override 2021-03-25 12:47 ` [PATCH 1/2] arm64: cpufeature: Allow early filtering of feature override Marc Zyngier @ 2021-03-25 19:27 ` Will Deacon 2021-03-26 10:56 ` Marc Zyngier 0 siblings, 1 reply; 10+ messages in thread From: Will Deacon @ 2021-03-25 19:27 UTC (permalink / raw) To: Marc Zyngier Cc: linux-arm-kernel, Hector Martin, Arnd Bergmann, Mark Rutland, Catalin Marinas, kernel-team On Thu, Mar 25, 2021 at 12:47:20PM +0000, Marc Zyngier wrote: > Some CPUs are broken enough that some overrides need to be rejected > at the earliest opportunity. In some cases, that's right at cpu > feature override time. > > Provide the necessary infrastructure to filter out overrides, > and to report such filtered out overrides to the core cpufeature code. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > arch/arm64/kernel/cpufeature.c | 6 ++++++ > arch/arm64/kernel/idreg-override.c | 13 +++++++++++++ > 2 files changed, 19 insertions(+) > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > index 066030717a4c..6de15deaa912 100644 > --- a/arch/arm64/kernel/cpufeature.c > +++ b/arch/arm64/kernel/cpufeature.c > @@ -809,6 +809,12 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new) > reg->name, > ftrp->shift + ftrp->width - 1, > ftrp->shift, str, tmp); > + } else if ((ftr_mask & reg->override->val) == ftr_mask) { This seems to rely on 'val == mask' being invalid, but I'm not sure why that's generally true. Can we just invoke the filter function again here to figure out if the field has been ignored? Then in match_options, we can just clear the override val/mask to zero. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] arm64: cpufeature: Allow early filtering of feature override 2021-03-25 19:27 ` Will Deacon @ 2021-03-26 10:56 ` Marc Zyngier 2021-03-29 10:21 ` Will Deacon 0 siblings, 1 reply; 10+ messages in thread From: Marc Zyngier @ 2021-03-26 10:56 UTC (permalink / raw) To: Will Deacon Cc: linux-arm-kernel, Hector Martin, Arnd Bergmann, Mark Rutland, Catalin Marinas, kernel-team On Thu, 25 Mar 2021 19:27:59 +0000, Will Deacon <will@kernel.org> wrote: > > On Thu, Mar 25, 2021 at 12:47:20PM +0000, Marc Zyngier wrote: > > Some CPUs are broken enough that some overrides need to be rejected > > at the earliest opportunity. In some cases, that's right at cpu > > feature override time. > > > > Provide the necessary infrastructure to filter out overrides, > > and to report such filtered out overrides to the core cpufeature code. > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > --- > > arch/arm64/kernel/cpufeature.c | 6 ++++++ > > arch/arm64/kernel/idreg-override.c | 13 +++++++++++++ > > 2 files changed, 19 insertions(+) > > > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > > index 066030717a4c..6de15deaa912 100644 > > --- a/arch/arm64/kernel/cpufeature.c > > +++ b/arch/arm64/kernel/cpufeature.c > > @@ -809,6 +809,12 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new) > > reg->name, > > ftrp->shift + ftrp->width - 1, > > ftrp->shift, str, tmp); > > + } else if ((ftr_mask & reg->override->val) == ftr_mask) { > > This seems to rely on 'val == mask' being invalid, but I'm not sure why > that's generally true. This is really 'ovr->val == mask && ovr->mask != mask', thanks to being on the 'else' branch. The encoding rules of val/mask are, for a given field: - no override set: mask = 0, val = 0 - valid override set: mask = 0xf, val = (override value) - invalid override set: mask = 0, val = 0xf I don't see where the ambiguity could be (though the above could figure in a comment to make things clearer). > Can we just invoke the filter function again here to figure out if > the field has been ignored? Then in match_options, we can just clear > the override val/mask to zero. The filter function isn't available outside of idreg-override.c: that's where the per-field override structures are defined, and I'd rather not expose that to the rest of the kernel. Also, calling the filter implies that you parse the whole command-line again, and you get into a real mess because the invalid override can come from the expansion of an alias (e.g. 'kvm-arm.mode=nvhe'). Seems totally overkill to me. If, for some reason that I can't see at the moment, we need an extra u64 to communicate that there is an invalid option, we can add that to the override structure. Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] arm64: cpufeature: Allow early filtering of feature override 2021-03-26 10:56 ` Marc Zyngier @ 2021-03-29 10:21 ` Will Deacon 0 siblings, 0 replies; 10+ messages in thread From: Will Deacon @ 2021-03-29 10:21 UTC (permalink / raw) To: Marc Zyngier Cc: linux-arm-kernel, Hector Martin, Arnd Bergmann, Mark Rutland, Catalin Marinas, kernel-team On Fri, Mar 26, 2021 at 10:56:23AM +0000, Marc Zyngier wrote: > On Thu, 25 Mar 2021 19:27:59 +0000, > Will Deacon <will@kernel.org> wrote: > > > > On Thu, Mar 25, 2021 at 12:47:20PM +0000, Marc Zyngier wrote: > > > Some CPUs are broken enough that some overrides need to be rejected > > > at the earliest opportunity. In some cases, that's right at cpu > > > feature override time. > > > > > > Provide the necessary infrastructure to filter out overrides, > > > and to report such filtered out overrides to the core cpufeature code. > > > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > > --- > > > arch/arm64/kernel/cpufeature.c | 6 ++++++ > > > arch/arm64/kernel/idreg-override.c | 13 +++++++++++++ > > > 2 files changed, 19 insertions(+) > > > > > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > > > index 066030717a4c..6de15deaa912 100644 > > > --- a/arch/arm64/kernel/cpufeature.c > > > +++ b/arch/arm64/kernel/cpufeature.c > > > @@ -809,6 +809,12 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new) > > > reg->name, > > > ftrp->shift + ftrp->width - 1, > > > ftrp->shift, str, tmp); > > > + } else if ((ftr_mask & reg->override->val) == ftr_mask) { > > > > This seems to rely on 'val == mask' being invalid, but I'm not sure why > > that's generally true. > > This is really 'ovr->val == mask && ovr->mask != mask', thanks to > being on the 'else' branch. The encoding rules of val/mask are, for a > given field: > > - no override set: mask = 0, val = 0 > - valid override set: mask = 0xf, val = (override value) > - invalid override set: mask = 0, val = 0xf > > I don't see where the ambiguity could be (though the above could > figure in a comment to make things clearer). Thanks, this makes sense to me now; I'd missed the other part of the conditional. So yeah, with that comment added: Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] arm64: Cope with CPUs stuck in VHE mode 2021-03-25 12:47 [PATCH 0/2] arm64: Dealing with VHE-only CPUs Marc Zyngier 2021-03-25 12:47 ` [PATCH 1/2] arm64: cpufeature: Allow early filtering of feature override Marc Zyngier @ 2021-03-25 12:47 ` Marc Zyngier 2021-03-25 19:33 ` Will Deacon 1 sibling, 1 reply; 10+ messages in thread From: Marc Zyngier @ 2021-03-25 12:47 UTC (permalink / raw) To: linux-arm-kernel Cc: Hector Martin, Arnd Bergmann, Mark Rutland, Will Deacon, Catalin Marinas, kernel-team 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 + +1: + mov_q x0, INIT_SCTLR_EL1_MMU_OFF + msr sctlr_el1, x0 + msr elr_el2, lr mov w0, #BOOT_CPU_MODE_EL2 eret diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S index 5eccbd62fec8..e6fa5cdd790a 100644 --- a/arch/arm64/kernel/hyp-stub.S +++ b/arch/arm64/kernel/hyp-stub.S @@ -27,12 +27,12 @@ SYM_CODE_START(__hyp_stub_vectors) ventry el2_fiq_invalid // FIQ EL2t ventry el2_error_invalid // Error EL2t - ventry el2_sync_invalid // Synchronous EL2h + ventry elx_sync // Synchronous EL2h ventry el2_irq_invalid // IRQ EL2h ventry el2_fiq_invalid // FIQ EL2h ventry el2_error_invalid // Error EL2h - ventry el1_sync // Synchronous 64-bit EL1 + ventry elx_sync // Synchronous 64-bit EL1 ventry el1_irq_invalid // IRQ 64-bit EL1 ventry el1_fiq_invalid // FIQ 64-bit EL1 ventry el1_error_invalid // Error 64-bit EL1 @@ -45,7 +45,7 @@ SYM_CODE_END(__hyp_stub_vectors) .align 11 -SYM_CODE_START_LOCAL(el1_sync) +SYM_CODE_START_LOCAL(elx_sync) cmp x0, #HVC_SET_VECTORS b.ne 1f msr vbar_el2, x1 @@ -71,7 +71,7 @@ SYM_CODE_START_LOCAL(el1_sync) 9: mov x0, xzr eret -SYM_CODE_END(el1_sync) +SYM_CODE_END(elx_sync) // nVHE? No way! Give me the real thing! SYM_CODE_START_LOCAL(mutate_to_vhe) @@ -243,3 +243,10 @@ SYM_FUNC_START(switch_to_vhe) #endif ret SYM_FUNC_END(switch_to_vhe) + +SYM_FUNC_START(stick_to_vhe) + mov x0, #HVC_VHE_RESTART + hvc #0 + mov x0, #BOOT_CPU_MODE_EL2 + ret +SYM_FUNC_END(stick_to_vhe) diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c index be92fcd319a1..6a8a14955fba 100644 --- a/arch/arm64/kernel/idreg-override.c +++ b/arch/arm64/kernel/idreg-override.c @@ -29,11 +29,22 @@ struct ftr_set_desc { } fields[]; }; +static bool __init mmfr1_vh_filter(u64 val) +{ + /* + * If we ever reach this point while running VHE, we're + * guaranteed to be on one of these funky, VHE-stuck CPUs. If + * the user was trying to force nVHE on us, proceed with + * attitude adjustment. + */ + return !(is_kernel_in_hyp_mode() && val == 0); +} + static const struct ftr_set_desc mmfr1 __initconst = { .name = "id_aa64mmfr1", .override = &id_aa64mmfr1_override, .fields = { - { "vh", ID_AA64MMFR1_VHE_SHIFT }, + { "vh", ID_AA64MMFR1_VHE_SHIFT, mmfr1_vh_filter }, {} }, }; -- 2.29.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] arm64: Cope with CPUs stuck in VHE mode 2021-03-25 12:47 ` [PATCH 2/2] arm64: Cope with CPUs stuck in VHE mode Marc Zyngier @ 2021-03-25 19:33 ` Will Deacon 2021-03-26 11:20 ` Marc Zyngier 0 siblings, 1 reply; 10+ messages in thread From: Will Deacon @ 2021-03-25 19:33 UTC (permalink / raw) To: Marc Zyngier Cc: linux-arm-kernel, Hector Martin, Arnd Bergmann, Mark Rutland, Catalin Marinas, kernel-team 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? Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] arm64: Cope with CPUs stuck in VHE mode 2021-03-25 19:33 ` Will Deacon @ 2021-03-26 11:20 ` Marc Zyngier 2021-03-29 10:22 ` Will Deacon 0 siblings, 1 reply; 10+ messages in thread From: Marc Zyngier @ 2021-03-26 11:20 UTC (permalink / raw) To: Will Deacon Cc: linux-arm-kernel, Hector Martin, Arnd Bergmann, Mark Rutland, Catalin Marinas, kernel-team 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. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] arm64: Cope with CPUs stuck in VHE mode 2021-03-26 11:20 ` Marc Zyngier @ 2021-03-29 10:22 ` Will Deacon 2021-03-30 17:00 ` Marc Zyngier 0 siblings, 1 reply; 10+ messages in thread From: Will Deacon @ 2021-03-29 10:22 UTC (permalink / raw) To: Marc Zyngier Cc: linux-arm-kernel, Hector Martin, Arnd Bergmann, Mark Rutland, Catalin Marinas, kernel-team 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] arm64: Cope with CPUs stuck in VHE mode 2021-03-29 10:22 ` Will Deacon @ 2021-03-30 17:00 ` Marc Zyngier 0 siblings, 0 replies; 10+ messages in thread From: Marc Zyngier @ 2021-03-30 17:00 UTC (permalink / raw) To: Will Deacon Cc: linux-arm-kernel, Hector Martin, Arnd Bergmann, Mark Rutland, Catalin Marinas, kernel-team On Mon, 29 Mar 2021 11:22:00 +0100, Will Deacon <will@kernel.org> wrote: > > On Fri, Mar 26, 2021 at 11:20:18AM +0000, Marc Zyngier wrote: > > 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? To quote someone, it is "a little ugly on the side". I came up with the following hack. Can't say I'm in love with it, specially the sprinkling of checks in the alternative callbacks, but hey, I can boot the machine without CONFIG_ARM64_VHE, and get the expected splat at boot time: [ 0.033604] ------------[ cut here ]------------ [ 0.033850] CPU: CPUs started in inconsistent modes [ 0.033854] WARNING: CPU: 0 PID: 1 at arch/arm64/kernel/smp.c:434 hyp_mode_check+0x90/0xc4 [ 0.034863] Modules linked in: [ 0.035100] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0-rc3-00103-geee3f110c447-dirty #3231 [ 0.035776] Hardware name: Apple Mac mini (M1, 2020) (DT) [ 0.036192] pstate: 60400009 (nZCv daif +PAN -UAO -TCO BTYPE=--) [ 0.036654] pc : hyp_mode_check+0x90/0xc4 [ 0.036963] lr : hyp_mode_check+0x90/0xc4 [ 0.037271] sp : ffff800010053e30 [ 0.037526] x29: ffff800010053e30 x28: 0000000000000000 [ 0.037935] x27: 0000000000000000 x26: 0000000000000000 [ 0.038344] x25: 0000000000000000 x24: 0000000000000000 [ 0.038754] x23: 0000000000000000 x22: 0000000000000000 [ 0.039163] x21: 0000000000000000 x20: ffffca3b2f53fc04 [ 0.039572] x19: ffffca3b2fac1000 x18: 0000000000000001 [ 0.039981] x17: 00000000cc4379d6 x16: 000000005c7b6156 [ 0.040391] x15: 0000000000000030 x14: ffffffffffffffff [ 0.040800] x13: ffff800090053ab7 x12: ffff800010053ac0 [ 0.041209] x11: 0000000bbe2c6238 x10: ffffca3b2faa0ad8 [ 0.041618] x9 : ffffca3b2e310df0 x8 : fffffffffffe18b8 [ 0.042027] x7 : ffffca3b2fa481d8 x6 : 0000000000002ffd [ 0.042437] x5 : 0000000000000000 x4 : 0000000000000000 [ 0.042846] x3 : 00000000ffffffff x2 : 0000000000000000 [ 0.043255] x1 : 0000000000000000 x0 : ffff4af181631280 [ 0.043665] Call trace: [ 0.043852] hyp_mode_check+0x90/0xc4 [ 0.044134] smp_cpus_done+0x34/0x48 [ 0.044409] smp_init+0x80/0x90 [ 0.044651] kernel_init_freeable+0x108/0x160 [ 0.044986] kernel_init+0x20/0x12c [ 0.045254] ret_from_fork+0x10/0x3c [ 0.045530] ---[ end trace 0736417247c9e9a3 ]--- [...] [ 0.616800] kvm [1]: HYP mode not available I'll wrap that up in a separate patch, and we can then discuss whether we really want it... Thanks, 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 */ -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-03-30 17:02 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-03-25 12:47 [PATCH 0/2] arm64: Dealing with VHE-only CPUs Marc Zyngier 2021-03-25 12:47 ` [PATCH 1/2] arm64: cpufeature: Allow early filtering of feature override Marc Zyngier 2021-03-25 19:27 ` Will Deacon 2021-03-26 10:56 ` Marc Zyngier 2021-03-29 10:21 ` Will Deacon 2021-03-25 12:47 ` [PATCH 2/2] arm64: Cope with CPUs stuck in VHE mode Marc Zyngier 2021-03-25 19:33 ` Will Deacon 2021-03-26 11:20 ` Marc Zyngier 2021-03-29 10:22 ` Will Deacon 2021-03-30 17:00 ` Marc Zyngier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).