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 5100CC433F5 for ; Mon, 17 Jan 2022 14:40:30 +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=SYnbzDZ5DW/app63poGsMpZNQV2nUEOIDiXnt167YRA=; b=12nEuZisx3Pifn AhPf+uJb5B0o+CQvI7OPkOQS6HMESR/EvjzqawyAzagW5oyB9eX0FxfqXVf1Ub35+DkmYL0178HxF /eTsTwLaeinhLZzNxX7URSdM7AjJcyOkSmLinvN9h39wnoxgOwdW8ICH2XJc7Ocx9xtTYRdyxQquA u4kKyOoxP0udlOeeICYjkR+YwAPx47sPMd3rmm6CSpHFbp0MYvUdpyiRCuKFJu9KwVU+CrrBBD7yO As495uynGvk9RtmiisU/q095+b/u+bASNthB0H1t98iFNnZVUgl1ejK4xWpzXDBqcklX+EGav5HEz U1K8BHGqj9yHBQ7ig3wQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n9TAH-00FDMn-PO; Mon, 17 Jan 2022 14:39:21 +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 1n9TAD-00FDKq-6U for linux-arm-kernel@lists.infradead.org; Mon, 17 Jan 2022 14:39:19 +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 79B271FB; Mon, 17 Jan 2022 06:39:16 -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 927E43F766; Mon, 17 Jan 2022 06:39:15 -0800 (PST) Date: Mon, 17 Jan 2022 14:39:13 +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 08/13] Announce boot-wrapper mode / exception level Message-ID: <20220117143913.520a553c@donnerap.cambridge.arm.com> In-Reply-To: <20220114105653.3003399-9-mark.rutland@arm.com> References: <20220114105653.3003399-1-mark.rutland@arm.com> <20220114105653.3003399-9-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_063917_395796_351984EE X-CRM114-Status: GOOD ( 29.14 ) 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:48 +0000 Mark Rutland wrote: Hi Mark, > When something goes wrong within the boot-wrapper, it can be very > helpful to know where we started from. Add an arch_announce() function > to log this early in the boot process. More information can be added > here in future. > > This is logged ot the serial console as: > > | Boot-wrapper v0.2 > | Entered at EL3 I like that one, and apart from the (already existing) UART issue below, this looks fine: > Signed-off-by: Mark Rutland Reviewed-by: Andre Przywara Cheers, Andre > --- > Makefile.am | 2 +- > arch/aarch32/include/asm/cpu.h | 9 +++++++++ > arch/aarch32/init.c | 30 ++++++++++++++++++++++++++++++ > arch/aarch64/init.c | 19 +++++++++++++++++++ > common/init.c | 6 +++++- > common/platform.c | 24 ++++++++++++++---------- > include/platform.h | 1 + > 7 files changed, 79 insertions(+), 12 deletions(-) > create mode 100644 arch/aarch32/init.c > create mode 100644 arch/aarch64/init.c > > diff --git a/Makefile.am b/Makefile.am > index 0651c38..885a77c 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -36,7 +36,7 @@ PSCI_CPU_OFF := 0x84000002 > COMMON_SRC := common/ > COMMON_OBJ := boot.o bakery_lock.o platform.o lib.o init.o > > -ARCH_OBJ := boot.o stack.o utils.o > +ARCH_OBJ := boot.o stack.o utils.o init.o > > if BOOTWRAPPER_32 > CPPFLAGS += -DBOOTWRAPPER_32 > diff --git a/arch/aarch32/include/asm/cpu.h b/arch/aarch32/include/asm/cpu.h > index d691c7b..aa72204 100644 > --- a/arch/aarch32/include/asm/cpu.h > +++ b/arch/aarch32/include/asm/cpu.h > @@ -44,6 +44,15 @@ > #define sevl() asm volatile ("sev" : : : "memory") > #endif > > +static inline unsigned long read_cpsr(void) > +{ > + unsigned long cpsr; > + asm volatile ("mrs %0, cpsr\n" : "=r" (cpsr)); > + return cpsr; > +} > + > +#define read_cpsr_mode() (read_cpsr() & PSR_MODE_MASK) > + > #define MPIDR "p15, 0, %0, c0, c0, 5" > #define ID_PFR1 "p15, 0, %0, c0, c1, 1" > #define ICIALLU "p15, 0, %0, c7, c5, 0" > diff --git a/arch/aarch32/init.c b/arch/aarch32/init.c > new file mode 100644 > index 0000000..b29ebb4 > --- /dev/null > +++ b/arch/aarch32/init.c > @@ -0,0 +1,30 @@ > +/* > + * init.c - common boot-wrapper initialization > + * > + * 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. > + */ > +#include > + > +#include > + > +static const char *mode_string(void) > +{ > + switch (read_cpsr_mode()) { > + case PSR_MON: > + return "PL1"; > + case PSR_HYP: > + return "PL2 (Non-secure)"; > + default: > + return ""; > + } > +} > + > +void announce_arch(void) > +{ > + print_string("Entered at "); > + print_string(mode_string()); > + print_string("\r\n"); > +} > diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c > new file mode 100644 > index 0000000..82816e7 > --- /dev/null > +++ b/arch/aarch64/init.c > @@ -0,0 +1,19 @@ > +/* > + * init.c - common boot-wrapper initialization > + * > + * 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. > + */ > +#include > +#include > + > +void announce_arch(void) > +{ > + unsigned char el = mrs(CurrentEl) >> 2; > + > + print_string("Entered at EL"); > + print_char('0' + el); > + print_string("\r\n"); > +} > diff --git a/common/init.c b/common/init.c > index 9c471c9..2600f73 100644 > --- a/common/init.c > +++ b/common/init.c > @@ -11,14 +11,18 @@ > > static void announce_bootwrapper(void) > { > - print_string("Boot-wrapper v0.2\r\n\r\n"); > + print_string("Boot-wrapper v0.2\r\n"); > } > > +void announce_arch(void); > + > void cpu_init_bootwrapper(void) > { > if (this_cpu_logical_id() == 0) { > init_uart(); > announce_bootwrapper(); > + announce_arch(); > + print_string("\r\n"); > init_platform(); > } > } > diff --git a/common/platform.c b/common/platform.c > index 47bf547..80d0562 100644 > --- a/common/platform.c > +++ b/common/platform.c > @@ -31,21 +31,25 @@ > #define V2M_SYS(reg) ((void *)SYSREGS_BASE + V2M_SYS_##reg) > #endif > > -void print_string(const char *str) > +void print_char(char c) > { > uint32_t flags; > > - while (*str) { > - do > - flags = raw_readl(PL011(UARTFR)); > - while (flags & PL011_UARTFR_FIFO_FULL); > + do { > + flags = raw_readl(PL011(UARTFR)); > + } while (flags & PL011_UARTFR_FIFO_FULL); > + > + raw_writel(c, PL011(UARTDR)); > > - raw_writel(*str++, PL011(UARTDR)); > + do { > + flags = raw_readl(PL011(UARTFR)); > + } while (flags & PL011_UARTFR_BUSY); Apologies if that appears to be nitpicking over a totally pointless issue (given the nature of the *emulated* PL011 in the model), but: I understand that this code has not changed, but this loop basically renders the FIFOs ineffective. Is this intended? I mean we explicitly enable the FIFOs in init_uart(), but then poll here until the transmit FIFO becomes empty, after *every* character pushed. I see that we probably want the output to be synchronous, for debug reasons, but I wonder if this should be achieved via an extra uart_flush() routine, once per line output? So call uart_flush() just in print_string(), but not in print_char(), for instance? Cheers, Andre > +} > > - do > - flags = raw_readl(PL011(UARTFR)); > - while (flags & PL011_UARTFR_BUSY); > - } > +void print_string(const char *str) > +{ > + while (*str) > + print_char(*str++); > } > > void init_uart(void) > diff --git a/include/platform.h b/include/platform.h > index e5248e1..237b481 100644 > --- a/include/platform.h > +++ b/include/platform.h > @@ -9,6 +9,7 @@ > #ifndef __PLATFORM_H > #define __PLATFORM_H > > +void print_char(char c); > void print_string(const char *str); > void init_uart(void); > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel