From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.0 required=3.0 tests=BAYES_00,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9FDF5C433E9 for ; Wed, 24 Mar 2021 20:01:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8EE0B61A2E for ; Wed, 24 Mar 2021 20:01:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238116AbhCXUAe (ORCPT ); Wed, 24 Mar 2021 16:00:34 -0400 Received: from mail.kernel.org ([198.145.29.99]:47384 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237991AbhCXUAQ (ORCPT ); Wed, 24 Mar 2021 16:00:16 -0400 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B732E61A29; Wed, 24 Mar 2021 20:00:15 +0000 (UTC) Received: from 78.163-31-62.static.virginmediabusiness.co.uk ([62.31.163.78] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94) (envelope-from ) id 1lP9fp-003axh-Qv; Wed, 24 Mar 2021 20:00:13 +0000 Date: Wed, 24 Mar 2021 20:00:12 +0000 Message-ID: <875z1gs4oj.wl-maz@kernel.org> From: Marc Zyngier To: Will Deacon Cc: Hector Martin , linux-arm-kernel@lists.infradead.org, Rob Herring , Arnd Bergmann , Olof Johansson , Krzysztof Kozlowski , Mark Kettenis , Tony Lindgren , Mohamed Mediouni , Stan Skowronek , Alexander Graf , Linus Walleij , Mark Rutland , Andy Shevchenko , Greg Kroah-Hartman , Jonathan Corbet , Catalin Marinas , Christoph Hellwig , "David S. Miller" , devicetree@vger.kernel.org, linux-serial@vger.kernel.org, linux-doc@vger.kernel.org, linux-samsung-soc@vger.kernel.org, linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFT PATCH v3 01/27] arm64: Cope with CPUs stuck in VHE mode In-Reply-To: <20210324180546.GA13181@willie-the-truck> References: <20210304213902.83903-1-marcan@marcan.st> <20210304213902.83903-2-marcan@marcan.st> <20210324180546.GA13181@willie-the-truck> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 62.31.163.78 X-SA-Exim-Rcpt-To: will@kernel.org, marcan@marcan.st, linux-arm-kernel@lists.infradead.org, robh@kernel.org, arnd@kernel.org, olof@lixom.net, krzk@kernel.org, mark.kettenis@xs4all.nl, tony@atomide.com, mohamed.mediouni@caramail.com, stan@corellium.com, graf@amazon.com, linus.walleij@linaro.org, mark.rutland@arm.com, andy.shevchenko@gmail.com, gregkh@linuxfoundation.org, corbet@lwn.net, catalin.marinas@arm.com, hch@infradead.org, davem@davemloft.net, devicetree@vger.kernel.org, linux-serial@vger.kernel.org, linux-doc@vger.kernel.org, linux-samsung-soc@vger.kernel.org, linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 24 Mar 2021 18:05:46 +0000, Will Deacon wrote: > > On Fri, Mar 05, 2021 at 06:38:36AM +0900, Hector Martin wrote: > > From: Marc Zyngier > > > > It seems that the CPU known as Apple M1 has 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. It is still > > unknown whether there are many more such nuggets to be found... > > > > Reported-by: Hector Martin > > Signed-off-by: Marc Zyngier > > --- > > arch/arm64/kernel/head.S | 33 ++++++++++++++++++++++++++++++--- > > arch/arm64/kernel/hyp-stub.S | 28 ++++++++++++++++++++++++---- > > 2 files changed, 54 insertions(+), 7 deletions(-) > > > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S > > index 66b0e0b66e31..673002b11865 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..c7601030ee82 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,23 @@ SYM_FUNC_START(switch_to_vhe) > > #endif > > ret > > SYM_FUNC_END(switch_to_vhe) > > + > > +SYM_FUNC_START(stick_to_vhe) > > + /* > > + * Make sure the switch to VHE cannot fail, by overriding the > > + * override. This is hilarious. > > + */ > > + adr_l x1, id_aa64mmfr1_override > > + add x1, x1, #FTR_OVR_MASK_OFFSET > > + dc civac, x1 > > + dsb sy > > + isb > > Why do we need an ISB here? Hmmm. Probably me being paranoid and trying to come up with something for Hector to test before I had access to the HW. The DSB is more than enough to order CMO and load/store. > > + ldr x0, [x1] > > + bic x0, x0, #(0xf << ID_AA64MMFR1_VHE_SHIFT) > > + str x0, [x1] > > I find it a bit bizarre doing this here, as for the primary CPU we're still > a way away from parsing the early paramaters and for secondary CPUs this > doesn't need to be done for each one. Furthermore, this same code is run > on the resume path, which can probably then race with itself. Agreed, this isn't great. > Is it possible to do it later on the boot CPU only, e.g. in > init_feature_override()? We should probably also log a warning that we're > ignoring the option because nVHE is not available. I've come up with this on top of the original patch, spitting a warning when the right conditions are met. It's pretty ugly, but hey, so is the HW this runs on. M. diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S index c7601030ee82..e6fa5cdd790a 100644 --- a/arch/arm64/kernel/hyp-stub.S +++ b/arch/arm64/kernel/hyp-stub.S @@ -245,19 +245,6 @@ SYM_FUNC_START(switch_to_vhe) SYM_FUNC_END(switch_to_vhe) SYM_FUNC_START(stick_to_vhe) - /* - * Make sure the switch to VHE cannot fail, by overriding the - * override. This is hilarious. - */ - adr_l x1, id_aa64mmfr1_override - add x1, x1, #FTR_OVR_MASK_OFFSET - dc civac, x1 - dsb sy - isb - ldr x0, [x1] - bic x0, x0, #(0xf << ID_AA64MMFR1_VHE_SHIFT) - str x0, [x1] - mov x0, #HVC_VHE_RESTART hvc #0 mov x0, #BOOT_CPU_MODE_EL2 diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c index 83f1c4b92095..ec07e150cf5c 100644 --- a/arch/arm64/kernel/idreg-override.c +++ b/arch/arm64/kernel/idreg-override.c @@ -195,6 +195,8 @@ static __init void parse_cmdline(void) __parse_cmdline(prop, true); } +static bool nvhe_impaired __initdata; + /* Keep checkers quiet */ void init_feature_override(void); @@ -211,9 +213,32 @@ asmlinkage void __init init_feature_override(void) parse_cmdline(); + /* + * 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. + */ + if (is_kernel_in_hyp_mode()) { + u64 mask = 0xfUL << ID_AA64MMFR1_VHE_SHIFT; + + if ((id_aa64mmfr1_override.mask & mask) && + !(id_aa64mmfr1_override.val & mask)) { + nvhe_impaired = true; + id_aa64mmfr1_override.mask &= ~mask; + } + } + for (i = 0; i < ARRAY_SIZE(regs); i++) { if (regs[i]->override) __flush_dcache_area(regs[i]->override, sizeof(*regs[i]->override)); } } + +static int __init check_feature_override(void) +{ + WARN_ON(nvhe_impaired); + return 0; +} +core_initcall(check_feature_override); -- Without deviation from the norm, progress is not possible.