| Message ID | 20240729161501.1806271-4-mark.rutland@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cleanup initialization | expand |
On Mon, 29 Jul 2024 17:14:53 +0100, Mark Rutland <mark.rutland@arm.com> wrote: > > When the boot-wrapper is entered at EL2 it does not initialise > CNTFRQ_EL0, and in future it may need to initialize other CPU state > regardless of the exeption level it was entered at. > > Use a common cpu_init_arch() function to initialize CPU state regardless > of the exception level the boot-wrapper was entered at. > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > Cc: Akos Denke <akos.denke@arm.com> > Cc: Andre Przywara <andre.przywara@arm.com> > Cc: Luca Fancellu <luca.fancellu@arm.com> > Cc: Marc Zyngier <maz@kernel.org> > --- > arch/aarch64/boot.S | 4 +++- > arch/aarch64/init.c | 12 +++++++++--- > 2 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S > index 51ef41b..d8d38dd 100644 > --- a/arch/aarch64/boot.S > +++ b/arch/aarch64/boot.S > @@ -51,7 +51,7 @@ reset_at_el3: > > bl cpu_init_bootwrapper > > - bl cpu_init_el3 > + bl cpu_init_arch > > bl gic_secure_init > > @@ -82,6 +82,8 @@ reset_at_el2: > > bl cpu_init_bootwrapper > > + bl cpu_init_arch > + > b start_bootmethod > > err_invalid_id: > diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c > index c9fc7f1..49abdf7 100644 > --- a/arch/aarch64/init.c > +++ b/arch/aarch64/init.c > @@ -52,7 +52,7 @@ static inline bool cpu_has_permission_indirection(void) > return mrs(ID_AA64MMFR3_EL1) & mask; > } > > -void cpu_init_el3(void) > +static void cpu_init_el3(void) > { > unsigned long scr = SCR_EL3_RES1 | SCR_EL3_NS | SCR_EL3_HCE; > unsigned long mdcr = 0; > @@ -153,8 +153,6 @@ void cpu_init_el3(void) > > msr(SMCR_EL3, smcr); > } > - > - msr(CNTFRQ_EL0, COUNTER_FREQ); > } > > #ifdef PSCI > @@ -171,3 +169,11 @@ bool cpu_init_psci_arch(void) > return true; > } > #endif > + > +void cpu_init_arch(void) > +{ > + if (mrs(CurrentEL) == CURRENTEL_EL3) > + cpu_init_el3(); > + > + msr(CNTFRQ_EL0, COUNTER_FREQ); > +} Hmmm. This means that you cannot use the BW on a system where EL3 is implemented, but where you decide to enter at EL2 anyway (the write to CNTFRQ_EL0 will UNDEF). I don't care much (I always want the BW to be the first piece of SW to run), but this is a rather subtle change in behaviour, and we'd better capture it in the commit message. With that, Acked-by: Marc Zyngier <maz@kernel.org> M.
On Fri, Aug 02, 2024 at 10:29:36AM +0100, Marc Zyngier wrote: > On Mon, 29 Jul 2024 17:14:53 +0100, > Mark Rutland <mark.rutland@arm.com> wrote: > > > > When the boot-wrapper is entered at EL2 it does not initialise > > CNTFRQ_EL0, and in future it may need to initialize other CPU state > > regardless of the exeption level it was entered at. > > > > Use a common cpu_init_arch() function to initialize CPU state regardless > > of the exception level the boot-wrapper was entered at. > > > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > > Cc: Akos Denke <akos.denke@arm.com> > > Cc: Andre Przywara <andre.przywara@arm.com> > > Cc: Luca Fancellu <luca.fancellu@arm.com> > > Cc: Marc Zyngier <maz@kernel.org> > > --- > > arch/aarch64/boot.S | 4 +++- > > arch/aarch64/init.c | 12 +++++++++--- > > 2 files changed, 12 insertions(+), 4 deletions(-) > > > > diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S > > index 51ef41b..d8d38dd 100644 > > --- a/arch/aarch64/boot.S > > +++ b/arch/aarch64/boot.S > > @@ -51,7 +51,7 @@ reset_at_el3: > > > > bl cpu_init_bootwrapper > > > > - bl cpu_init_el3 > > + bl cpu_init_arch > > > > bl gic_secure_init > > > > @@ -82,6 +82,8 @@ reset_at_el2: > > > > bl cpu_init_bootwrapper > > > > + bl cpu_init_arch > > + > > b start_bootmethod > > > > err_invalid_id: > > diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c > > index c9fc7f1..49abdf7 100644 > > --- a/arch/aarch64/init.c > > +++ b/arch/aarch64/init.c > > @@ -52,7 +52,7 @@ static inline bool cpu_has_permission_indirection(void) > > return mrs(ID_AA64MMFR3_EL1) & mask; > > } > > > > -void cpu_init_el3(void) > > +static void cpu_init_el3(void) > > { > > unsigned long scr = SCR_EL3_RES1 | SCR_EL3_NS | SCR_EL3_HCE; > > unsigned long mdcr = 0; > > @@ -153,8 +153,6 @@ void cpu_init_el3(void) > > > > msr(SMCR_EL3, smcr); > > } > > - > > - msr(CNTFRQ_EL0, COUNTER_FREQ); > > } > > > > #ifdef PSCI > > @@ -171,3 +169,11 @@ bool cpu_init_psci_arch(void) > > return true; > > } > > #endif > > + > > +void cpu_init_arch(void) > > +{ > > + if (mrs(CurrentEL) == CURRENTEL_EL3) > > + cpu_init_el3(); > > + > > + msr(CNTFRQ_EL0, COUNTER_FREQ); > > +} > > Hmmm. This means that you cannot use the BW on a system where EL3 is > implemented, but where you decide to enter at EL2 anyway (the write to > CNTFRQ_EL0 will UNDEF). > > I don't care much (I always want the BW to be the first piece of SW to > run), but this is a rather subtle change in behaviour, and we'd better > capture it in the commit message. Booting in that way has been explciitly documented as not supported since January 2022 in commit: 286b8ecc86393c26 ("Document entry requirements") ... which added documentation to the start of boot.S which says: | The boot-wrapper must be entered from the reset vector at the | highest implemented exception level. .. but regardless I can add some text to the commit message to say that violating this requirement will now cause a silent failure. > With that, > > Acked-by: Marc Zyngier <maz@kernel.org> Thanks! Mark.
diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S index 51ef41b..d8d38dd 100644 --- a/arch/aarch64/boot.S +++ b/arch/aarch64/boot.S @@ -51,7 +51,7 @@ reset_at_el3: bl cpu_init_bootwrapper - bl cpu_init_el3 + bl cpu_init_arch bl gic_secure_init @@ -82,6 +82,8 @@ reset_at_el2: bl cpu_init_bootwrapper + bl cpu_init_arch + b start_bootmethod err_invalid_id: diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c index c9fc7f1..49abdf7 100644 --- a/arch/aarch64/init.c +++ b/arch/aarch64/init.c @@ -52,7 +52,7 @@ static inline bool cpu_has_permission_indirection(void) return mrs(ID_AA64MMFR3_EL1) & mask; } -void cpu_init_el3(void) +static void cpu_init_el3(void) { unsigned long scr = SCR_EL3_RES1 | SCR_EL3_NS | SCR_EL3_HCE; unsigned long mdcr = 0; @@ -153,8 +153,6 @@ void cpu_init_el3(void) msr(SMCR_EL3, smcr); } - - msr(CNTFRQ_EL0, COUNTER_FREQ); } #ifdef PSCI @@ -171,3 +169,11 @@ bool cpu_init_psci_arch(void) return true; } #endif + +void cpu_init_arch(void) +{ + if (mrs(CurrentEL) == CURRENTEL_EL3) + cpu_init_el3(); + + msr(CNTFRQ_EL0, COUNTER_FREQ); +}
When the boot-wrapper is entered at EL2 it does not initialise CNTFRQ_EL0, and in future it may need to initialize other CPU state regardless of the exeption level it was entered at. Use a common cpu_init_arch() function to initialize CPU state regardless of the exception level the boot-wrapper was entered at. Signed-off-by: Mark Rutland <mark.rutland@arm.com> Cc: Akos Denke <akos.denke@arm.com> Cc: Andre Przywara <andre.przywara@arm.com> Cc: Luca Fancellu <luca.fancellu@arm.com> Cc: Marc Zyngier <maz@kernel.org> --- arch/aarch64/boot.S | 4 +++- arch/aarch64/init.c | 12 +++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-)