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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 07924C433EF for ; Mon, 17 Jan 2022 17:44:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=91iAuRzUauf4ItTcDg1VKdVDb8reXegQNeE+uKXXeMU=; b=raceHycCfqLndn nPMx6DBI1PivvJFaXju8V01hXOPKPSVNMVI/DQhbsBX6oskqut9kPbE8aY69tiQkeEuBHkpgByX57 f5ZyJML3I3P9Va8pGss3V2VwYyGKxr1EduWh4Rsz9wdvxT2qAbJTQesqmxiiibw5YGJ33hY0PujRs ThbgLEYyZNLjSvccMO8UG59dZs+x7KqEpIiCW8NT17pYfSb5xcT//yohypmQU0u46kTPAloI+urLV XtGkn3r6/xn3KamsVsESCKKDdTV8gXh3gqhUpofr1Saqz1VJB+z6GEUfQT/O1kUSlGqfISoLGLIKu QFBozalEjC3M2G/Hs+lw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n9W29-00Fyin-IX; Mon, 17 Jan 2022 17:43:09 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n9W24-00Fyhu-KU for linux-arm-kernel@lists.infradead.org; Mon, 17 Jan 2022 17:43:06 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 8FF976D; Mon, 17 Jan 2022 09:43:03 -0800 (PST) Received: from donnerap.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A55563F766; Mon, 17 Jan 2022 09:43:02 -0800 (PST) Date: Mon, 17 Jan 2022 17:43:00 +0000 From: Andre Przywara To: Mark Rutland Cc: linux-arm-kernel@lists.infradead.org, Jaxson.Han@arm.com, robin.murphy@arm.com, vladimir.murzin@arm.com, Wei.Chen@arm.com Subject: Re: [bootwrapper PATCH v2 12/13] Rework bootmethod initialization Message-ID: <20220117174300.0a91fcf6@donnerap.cambridge.arm.com> In-Reply-To: <20220114105653.3003399-13-mark.rutland@arm.com> References: <20220114105653.3003399-1-mark.rutland@arm.com> <20220114105653.3003399-13-mark.rutland@arm.com> Organization: ARM X-Mailer: Claws Mail 3.17.5 (GTK+ 2.24.32; aarch64-unknown-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220117_094304_834254_F8A67395 X-CRM114-Status: GOOD ( 32.90 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 14 Jan 2022 10:56:52 +0000 Mark Rutland wrote: Hi Mark, > We currently initialize the bootmethod late, in assembly code. This > requires us to maintain the el3/no_el3 distintion late into the boot > process, and means we cannot produce any helpful diagnostic when booted > at an unexpected exception level. > > Rework things so that we initialize the bootmethod early, with a warning > when things are wrong. The el3/no_el3 distinction is now irrelevant to > the bootmethod code, and can be removed in subsequent patches. > > When a boot-wrapper configured for PSCI is entered at EL2, a warning is > looged to the serial console as: > > | Boot-wrapper v0.2 > | Entered at EL2 > | Memory layout: > | [0000000080000000..0000000080001f90] => boot-wrapper > | [000000008000fff8..0000000080010000] => mbox > | [0000000080200000..00000000822af200] => kernel > | [0000000088000000..0000000088002857] => dtb > | > | WARNING: PSCI could not be initialized. Boot may fail > > Signed-off-by: Mark Rutland > --- > arch/aarch32/include/asm/cpu.h | 1 + > arch/aarch32/include/asm/psci.h | 28 ++++++++++++++++++++++++++++ > arch/aarch32/psci.S | 8 +------- > arch/aarch32/utils.S | 9 --------- > arch/aarch64/include/asm/psci.h | 28 ++++++++++++++++++++++++++++ > arch/aarch64/psci.S | 21 ++------------------- > arch/aarch64/spin.S | 3 +++ > arch/aarch64/utils.S | 9 --------- > common/init.c | 7 ++++++- > common/psci.c | 22 +++++++++++++++++++--- > include/boot.h | 2 ++ > 11 files changed, 90 insertions(+), 48 deletions(-) > create mode 100644 arch/aarch32/include/asm/psci.h > create mode 100644 arch/aarch64/include/asm/psci.h > > diff --git a/arch/aarch32/include/asm/cpu.h b/arch/aarch32/include/asm/cpu.h > index c1bce9a..3426075 100644 > --- a/arch/aarch32/include/asm/cpu.h > +++ b/arch/aarch32/include/asm/cpu.h > @@ -63,6 +63,7 @@ static inline unsigned long read_cpsr(void) > #define SCR "p15, 0, %0, c1, c1, 0" > #define NSACR "p15, 0, %0, c1, c1, 2" > #define ICIALLU "p15, 0, %0, c7, c5, 0" > +#define MVBAR "p15, 0, %0, c12, c0, 1" > > #define ICC_SRE "p15, 6, %0, c12, c12, 5" > #define ICC_CTLR "p15, 6, %0, c12, c12, 4" > diff --git a/arch/aarch32/include/asm/psci.h b/arch/aarch32/include/asm/psci.h > new file mode 100644 > index 0000000..50c7c70 > --- /dev/null > +++ b/arch/aarch32/include/asm/psci.h > @@ -0,0 +1,28 @@ > +/* > + * arch/aarch64/include/asm/psci.h > + * > + * Copyright (C) 2021 ARM Limited. All rights reserved. > + * > + * Use of this source code is governed by a BSD-style license that can be > + * found in the LICENSE.txt file. > + */ > +#ifndef __ASM_AARCH64_PSCI_H > +#define __ASM_AARCH64_PSCI_H > + > +#include > +#include > + > +extern char psci_vectors[]; > + > +static inline bool cpu_init_psci_arch(void) > +{ > + if (read_cpsr_mode() != PSR_MON) > + return false; > + > + mcr(MVBAR, (unsigned long)psci_vectors); > + isb(); > + > + return true; > +} > + > +#endif > diff --git a/arch/aarch32/psci.S b/arch/aarch32/psci.S > index e0d2972..cdc36b0 100644 > --- a/arch/aarch32/psci.S > +++ b/arch/aarch32/psci.S > @@ -15,7 +15,7 @@ > > .section .vectors > .align 6 > -smc_vectors: > +ASM_DATA(psci_vectors) > b err_exception @ Reset > b err_exception @ Undef > b handle_smc @ SMC > @@ -39,11 +39,5 @@ handle_smc: > movs pc, lr > > ASM_FUNC(start_el3) > - ldr r0, =smc_vectors > - blx setup_vector > - /* pass through */ > - > ASM_FUNC(start_no_el3) > - cpuid r0, r1 > - blx find_logical_id > b psci_first_spin > diff --git a/arch/aarch32/utils.S b/arch/aarch32/utils.S > index 5809f48..58279aa 100644 > --- a/arch/aarch32/utils.S > +++ b/arch/aarch32/utils.S > @@ -35,12 +35,3 @@ ASM_FUNC(find_logical_id) > bx lr > 3: mov r0, #MPIDR_INVALID > bx lr > - > -/* > - * Setup EL3 vectors. > - * r0: vector address > - */ > -ASM_FUNC(setup_vector) > - mcr p15, 0, r0, c12, c0, 1 @ MVBAR > - isb > - bx lr > diff --git a/arch/aarch64/include/asm/psci.h b/arch/aarch64/include/asm/psci.h > new file mode 100644 > index 0000000..491e685 > --- /dev/null > +++ b/arch/aarch64/include/asm/psci.h > @@ -0,0 +1,28 @@ > +/* > + * arch/aarch64/include/asm/psci.h > + * > + * Copyright (C) 2021 ARM Limited. All rights reserved. > + * > + * Use of this source code is governed by a BSD-style license that can be > + * found in the LICENSE.txt file. > + */ > +#ifndef __ASM_AARCH64_PSCI_H > +#define __ASM_AARCH64_PSCI_H > + > +#include > +#include > + > +extern char psci_vectors[]; > + > +static inline bool cpu_init_psci_arch(void) > +{ > + if (mrs(CurrentEL) != CURRENTEL_EL3) > + return false; > + > + msr(VBAR_EL3, (unsigned long)psci_vectors); > + isb(); > + > + return true; > +} > + > +#endif Is there any particular reason that needs to live as a static inline in a header file? Can't we have the prototype in, say include/boot.h, and then have this in a proper C file, for instance arch/aarch/init.c? The rest looks alright, though the change is somewhat hard to digest. Cheers, Andre > diff --git a/arch/aarch64/psci.S b/arch/aarch64/psci.S > index 8bd224b..d6ca2eb 100644 > --- a/arch/aarch64/psci.S > +++ b/arch/aarch64/psci.S > @@ -19,7 +19,7 @@ > .section .vectors, "w" > > .align 11 > -vector: > +ASM_DATA(psci_vectors) > // current EL, SP_EL0 > ventry err_exception // synchronous > ventry err_exception // IRQ > @@ -80,22 +80,5 @@ smc_exit: > eret > > ASM_FUNC(start_el3) > - ldr x0, =vector > - bl setup_vector > - > - /* only boot the primary cpu (entry 0 in the table) */ > - cpuid x0, x1 > - bl find_logical_id > - b psci_first_spin > - > -/* > - * This PSCI implementation requires EL3. Without EL3 we'll only boot the > - * primary cpu, all others will be trapped in an infinite loop. > - */ > ASM_FUNC(start_no_el3) > - cpuid x0, x1 > - bl find_logical_id > - cbz x0, psci_first_spin > -spin_dead: > - wfe > - b spin_dead > + b psci_first_spin > diff --git a/arch/aarch64/spin.S b/arch/aarch64/spin.S > index 1ea1c0b..764c532 100644 > --- a/arch/aarch64/spin.S > +++ b/arch/aarch64/spin.S > @@ -12,6 +12,9 @@ > > .text > > +ASM_FUNC(cpu_init_bootmethod) > + ret > + > ASM_FUNC(start_el3) > ASM_FUNC(start_no_el3) > cpuid x0, x1 > diff --git a/arch/aarch64/utils.S b/arch/aarch64/utils.S > index 85c7f8a..32393cc 100644 > --- a/arch/aarch64/utils.S > +++ b/arch/aarch64/utils.S > @@ -32,12 +32,3 @@ ASM_FUNC(find_logical_id) > ret > 3: mov x0, #MPIDR_INVALID > ret > - > -/* > - * Setup EL3 vectors > - * x0: vector address > - */ > -ASM_FUNC(setup_vector) > - msr VBAR_EL3, x0 > - isb > - ret > diff --git a/common/init.c b/common/init.c > index fc74b9e..3c05ac3 100644 > --- a/common/init.c > +++ b/common/init.c > @@ -6,6 +6,7 @@ > * Use of this source code is governed by a BSD-style license that can be > * found in the LICENSE.txt file. > */ > +#include > #include > #include > > @@ -44,7 +45,9 @@ void announce_arch(void); > > void cpu_init_bootwrapper(void) > { > - if (this_cpu_logical_id() == 0) { > + unsigned int cpu = this_cpu_logical_id(); > + > + if (cpu == 0) { > init_uart(); > announce_bootwrapper(); > announce_arch(); > @@ -52,4 +55,6 @@ void cpu_init_bootwrapper(void) > print_string("\r\n"); > init_platform(); > } > + > + cpu_init_bootmethod(cpu); > } > diff --git a/common/psci.c b/common/psci.c > index a0e8700..8af6870 100644 > --- a/common/psci.c > +++ b/common/psci.c > @@ -12,8 +12,11 @@ > #include > #include > #include > +#include > #include > > +#include > + > #ifndef CPU_IDS > #error "No MPIDRs provided" > #endif > @@ -78,12 +81,25 @@ long psci_call(unsigned long fid, unsigned long arg1, unsigned long arg2) > } > } > > -void __noreturn psci_first_spin(unsigned int cpu) > +void __noreturn psci_first_spin(void) > { > - if (cpu == MPIDR_INVALID) > - while (1); > + unsigned int cpu = this_cpu_logical_id(); > > first_spin(cpu, branch_table + cpu, PSCI_ADDR_INVALID); > > unreachable(); > } > + > +void cpu_init_bootmethod(unsigned int cpu) > +{ > + if (cpu_init_psci_arch()) > + return; > + > + if (cpu == 0) { > + print_string("WARNING: PSCI could not be initialized. Boot may fail\r\n\r\n"); > + return; > + } > + > + while (1) > + wfe(); > +} > diff --git a/include/boot.h b/include/boot.h > index d75e013..10968f8 100644 > --- a/include/boot.h > +++ b/include/boot.h > @@ -16,4 +16,6 @@ void __noreturn spin(unsigned long *mbox, unsigned long invalid, int is_entry); > void __noreturn first_spin(unsigned int cpu, unsigned long *mbox, > unsigned long invalid_addr); > > +void cpu_init_bootmethod(unsigned int cpu); > + > #endif _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel