Re: [bootwrapper PATCH v2 12/13] Rework bootmethod initialization

[Mark Rutland <mark.rutland@arm.com>]

On Mon, Jan 17, 2022 at 05:43:00PM +0000, Andre Przywara wrote:
> On Fri, 14 Jan 2022 10:56:52 +0000
> Mark Rutland <mark.rutland@arm.com> 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 <mark.rutland@arm.com>

> > 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 <cpu.h>
> > +#include <stdbool.h>
> > +
> > +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<xx>/init.c?

At the time I originally wrote it, I had thought that this was the simplest
option, but you're right that it's cleaner to place this in the relevant init.c
file -- done. :)

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel