[bootwrapper PATCH v2 08/13] Announce boot-wrapper mode / exception level
Andre Przywara
andre.przywara at arm.com
Mon Jan 17 06:39:13 PST 2022
On Fri, 14 Jan 2022 10:56:48 +0000
Mark Rutland <mark.rutland at arm.com> 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 <mark.rutland at arm.com>
Reviewed-by: Andre Przywara <andre.przywara at arm.com>
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 <platform.h>
> +
> +#include <asm/cpu.h>
> +
> +static const char *mode_string(void)
> +{
> + switch (read_cpsr_mode()) {
> + case PSR_MON:
> + return "PL1";
> + case PSR_HYP:
> + return "PL2 (Non-secure)";
> + default:
> + return "<UNKNOWN MODE>";
> + }
> +}
> +
> +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 <cpu.h>
> +#include <platform.h>
> +
> +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);
>
More information about the linux-arm-kernel
mailing list