[BOOT-WRAPPER v2 07/10] Unify assembly setup paths
Mark Rutland
mark.rutland at arm.com
Thu Aug 22 02:50:44 PDT 2024
On Wed, Aug 21, 2024 at 05:54:45PM +0100, Andre Przywara wrote:
> On Mon, 12 Aug 2024 11:15:52 +0100
> Mark Rutland <mark.rutland at arm.com> wrote:
>
> > The early assembly paths for EL3/Secure-PL1 and EL2/Hyp are almost
> > identical aside from the EL3/Secure-PL1 paths calling gic_secure_init().
> >
> > Simplify the easlery assembly paths by conditionally calling
Whoops; I'll s/easlery/early/ here.
> > gic_secure_init() from cpu_init_arch(), allowing the EL3/Secure-PL1 and
> > EL2/Hyp paths to be unified.
> >
> > In order to call gic_secure_init() from C code we need to expose a
> > prototype for gic_secure_init(), requiring a new <gic.h> header. For
> > clarity the existing <asm/gic-v3.h> headers are renamed to <asm/gic.h>
> > and are included through the common header.
> >
> > Signed-off-by: Mark Rutland <mark.rutland at arm.com>
> > Acked-by: Marc Zyngier <maz at kernel.org>
> > Cc: Akos Denke <akos.denke at arm.com>
> > Cc: Andre Przywara <andre.przywara at arm.com>
> > Cc: Luca Fancellu <luca.fancellu at arm.com>
>
> Thanks, that seems like a nice cleanup and refactoring, and looks good to
> me:
>
> Reviewed-by: Andre Przywara <andre.przywara at arm.com>
Cheers!
> Just not sure what to do about the TODO comments in there?
They're a note that the AArch32 code doesn't currently initialize
registers which are UNKNOWN at reset, and without the notes it looks a
little odd to have those labels at all ratehr than branching directly to
reset_common() above.
I'd prefer to keep those for now to indicate that there are known issues
here.
Mark.
>
> Cheers,
> Andre
>
> > ---
> > arch/aarch32/boot.S | 20 ++++++--------------
> > arch/aarch32/include/asm/{gic-v3.h => gic.h} | 2 +-
> > arch/aarch32/init.c | 5 ++++-
> > arch/aarch64/boot.S | 17 ++++-------------
> > arch/aarch64/include/asm/{gic-v3.h => gic.h} | 2 +-
> > arch/aarch64/init.c | 5 ++++-
> > common/gic-v3.c | 2 +-
> > common/gic.c | 2 +-
> > include/gic.h | 16 ++++++++++++++++
> > 9 files changed, 38 insertions(+), 33 deletions(-)
> > rename arch/aarch32/include/asm/{gic-v3.h => gic.h} (91%)
> > rename arch/aarch64/include/asm/{gic-v3.h => gic.h} (92%)
> > create mode 100644 include/gic.h
> >
> > diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
> > index 78d19a0..8b6ffac 100644
> > --- a/arch/aarch32/boot.S
> > +++ b/arch/aarch32/boot.S
> > @@ -53,22 +53,14 @@ reset_at_svc:
> > movs pc, lr
> >
> > reset_at_mon:
> > - cpuid r0, r1
> > - bl find_logical_id
> > - cmp r0, #MPIDR_INVALID
> > - beq err_invalid_id
> > -
> > - bl setup_stack
> > -
> > - bl cpu_init_bootwrapper
> > -
> > - bl cpu_init_arch
> > -
> > - bl gic_secure_init
> > -
> > - b start_bootmethod
> > + // TODO initialise monitor state
> > + b reset_common
> >
> > reset_at_hyp:
> > + // TODO initialise hyp state
> > + b reset_common
> > +
> > +reset_common:
> > cpuid r0, r1
> > bl find_logical_id
> > cmp r0, #MPIDR_INVALID
> > diff --git a/arch/aarch32/include/asm/gic-v3.h b/arch/aarch32/include/asm/gic.h
> > similarity index 91%
> > rename from arch/aarch32/include/asm/gic-v3.h
> > rename to arch/aarch32/include/asm/gic.h
> > index b28136a..0b9425d 100644
> > --- a/arch/aarch32/include/asm/gic-v3.h
> > +++ b/arch/aarch32/include/asm/gic.h
> > @@ -1,5 +1,5 @@
> > /*
> > - * arch/aarch32/include/asm/gic-v3.h
> > + * arch/aarch32/include/asm/gic.h
> > *
> > * Copyright (C) 2015 ARM Limited. All rights reserved.
> > *
> > diff --git a/arch/aarch32/init.c b/arch/aarch32/init.c
> > index 35da37c..cb67bf6 100644
> > --- a/arch/aarch32/init.c
> > +++ b/arch/aarch32/init.c
> > @@ -7,6 +7,7 @@
> > * found in the LICENSE.txt file.
> > */
> > #include <cpu.h>
> > +#include <gic.h>
> > #include <platform.h>
> > #include <stdbool.h>
> >
> > @@ -56,8 +57,10 @@ bool cpu_init_psci_arch(void)
> >
> > void cpu_init_arch(void)
> > {
> > - if (read_cpsr_mode() == PSR_MON)
> > + if (read_cpsr_mode() == PSR_MON) {
> > cpu_init_monitor();
> > + gic_secure_init();
> > + }
> >
> > mcr(CNTFRQ, COUNTER_FREQ);
> > }
> > diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
> > index 0ac0c98..98ae77d 100644
> > --- a/arch/aarch64/boot.S
> > +++ b/arch/aarch64/boot.S
> > @@ -43,19 +43,7 @@ reset_at_el3:
> > msr sctlr_el3, x0
> > isb
> >
> > - cpuid x0, x1
> > - bl find_logical_id
> > - cmp x0, #MPIDR_INVALID
> > - b.eq err_invalid_id
> > - bl setup_stack
> > -
> > - bl cpu_init_bootwrapper
> > -
> > - bl cpu_init_arch
> > -
> > - bl gic_secure_init
> > -
> > - b start_bootmethod
> > + b reset_common
> >
> > /*
> > * EL2 initialization
> > @@ -70,6 +58,9 @@ reset_at_el2:
> > msr sctlr_el2, x0
> > isb
> >
> > + b reset_common
> > +
> > +reset_common:
> > cpuid x0, x1
> > bl find_logical_id
> > cmp x0, #MPIDR_INVALID
> > diff --git a/arch/aarch64/include/asm/gic-v3.h b/arch/aarch64/include/asm/gic.h
> > similarity index 92%
> > rename from arch/aarch64/include/asm/gic-v3.h
> > rename to arch/aarch64/include/asm/gic.h
> > index 2447480..9d716f6 100644
> > --- a/arch/aarch64/include/asm/gic-v3.h
> > +++ b/arch/aarch64/include/asm/gic.h
> > @@ -1,5 +1,5 @@
> > /*
> > - * arch/aarch64/include/asm/gic-v3.h
> > + * arch/aarch64/include/asm/gic.h
> > *
> > * Copyright (C) 2015 ARM Limited. All rights reserved.
> > *
> > diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
> > index 49abdf7..68c220b 100644
> > --- a/arch/aarch64/init.c
> > +++ b/arch/aarch64/init.c
> > @@ -7,6 +7,7 @@
> > * found in the LICENSE.txt file.
> > */
> > #include <cpu.h>
> > +#include <gic.h>
> > #include <platform.h>
> > #include <stdbool.h>
> >
> > @@ -172,8 +173,10 @@ bool cpu_init_psci_arch(void)
> >
> > void cpu_init_arch(void)
> > {
> > - if (mrs(CurrentEL) == CURRENTEL_EL3)
> > + if (mrs(CurrentEL) == CURRENTEL_EL3) {
> > cpu_init_el3();
> > + gic_secure_init();
> > + }
> >
> > msr(CNTFRQ_EL0, COUNTER_FREQ);
> > }
> > diff --git a/common/gic-v3.c b/common/gic-v3.c
> > index 6207007..4d8e620 100644
> > --- a/common/gic-v3.c
> > +++ b/common/gic-v3.c
> > @@ -10,7 +10,7 @@
> > #include <stdint.h>
> >
> > #include <cpu.h>
> > -#include <asm/gic-v3.h>
> > +#include <gic.h>
> > #include <asm/io.h>
> >
> > #define GICD_CTLR 0x0
> > diff --git a/common/gic.c b/common/gic.c
> > index 04d4289..15a3410 100644
> > --- a/common/gic.c
> > +++ b/common/gic.c
> > @@ -10,7 +10,7 @@
> > #include <stdint.h>
> >
> > #include <cpu.h>
> > -#include <asm/gic-v3.h>
> > +#include <gic.h>
> > #include <asm/io.h>
> >
> > #define GICD_CTLR 0x0
> > diff --git a/include/gic.h b/include/gic.h
> > new file mode 100644
> > index 0000000..127f82b
> > --- /dev/null
> > +++ b/include/gic.h
> > @@ -0,0 +1,16 @@
> > +/*
> > + * include/gic.h
> > + *
> > + * Copyright (C) 2024 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 __GIC_H
> > +#define __GIC_H
> > +
> > +#include <asm/gic.h>
> > +
> > +void gic_secure_init(void);
> > +
> > +#endif
>
More information about the linux-arm-kernel
mailing list