mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH master v2] ARM: cpu: don't clobber sp when booted in HYP mode
@ 2022-05-31  8:29 Ahmad Fatoum
  2022-06-01  9:51 ` Sascha Hauer
  0 siblings, 1 reply; 2+ messages in thread
From: Ahmad Fatoum @ 2022-05-31  8:29 UTC (permalink / raw)
  To: barebox; +Cc: sha, lst, Ahmad Fatoum

arm_cpu_lowlevel_init() is usually called first thing and will ensure
barebox runs in SVC mode. If barebox is started in HYP mode instead,
like is the case on Raspberry Pi 2-3, it will do an exception return
into SVC mode, which will bank the previously used SP_Hyp and
restore SP_Svc that may not have been properly initialized by barebox.

This wasn't too bad so far, because arm_setup_stack was usually called
after arm_cpu_lowlevel_init, but with ENTRY_FUNCTION_WITHSTACK, SP is
initialized early on in the naked entry point with
arm_cpu_lowlevel_init() being called after that.

This can lead to spurious boot hangs in the Raspberry Pi 2 and 3 entry
points. Fix this by always saving sp to r3 and restoring it, like we do
with lr. This is safe to do, because r3 isn't clobbered by any
instruction in arm_cpu_lowlevel_init() and because it's an argument
register, callers have to expect it being overwritten by the callee.

Fixes: b267578d0567 ("ARM: rpi: use ENTRY_FUNCTION_WITHSTACK to prepare for ARM64 support")
Fixes: 41292192c01b ("ARM: safely switch from HYP to SVC mode if required")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - add source code comment on why sp is saved (Sascha)
---
 arch/arm/cpu/lowlevel.S | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/cpu/lowlevel.S b/arch/arm/cpu/lowlevel.S
index 5a7dd3c2093f..960a92b78c0a 100644
--- a/arch/arm/cpu/lowlevel.S
+++ b/arch/arm/cpu/lowlevel.S
@@ -9,6 +9,8 @@
 ENTRY(arm_cpu_lowlevel_init)
 	/* save lr, since it may be banked away with a processor mode change */
 	mov	r2, lr
+	/* save sp, because possible HYP -> SVC transition below clobbers it */
+	mov	r3, sp
 
 #ifdef CONFIG_CPU_32v7
 	/* careful: the hyp install corrupts r0 and r1 */
@@ -77,6 +79,7 @@ THUMB(	orr	r12, r12, #PSR_T_BIT	)
 
 	mcr	p15, 0, r12, c1, c0, 0		/* SCTLR */
 
+	mov	sp, r3
 	mov	pc, r2
 ENDPROC(arm_cpu_lowlevel_init)
 
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH master v2] ARM: cpu: don't clobber sp when booted in HYP mode
  2022-05-31  8:29 [PATCH master v2] ARM: cpu: don't clobber sp when booted in HYP mode Ahmad Fatoum
@ 2022-06-01  9:51 ` Sascha Hauer
  0 siblings, 0 replies; 2+ messages in thread
From: Sascha Hauer @ 2022-06-01  9:51 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox, lst

On Tue, May 31, 2022 at 10:29:14AM +0200, Ahmad Fatoum wrote:
> arm_cpu_lowlevel_init() is usually called first thing and will ensure
> barebox runs in SVC mode. If barebox is started in HYP mode instead,
> like is the case on Raspberry Pi 2-3, it will do an exception return
> into SVC mode, which will bank the previously used SP_Hyp and
> restore SP_Svc that may not have been properly initialized by barebox.
> 
> This wasn't too bad so far, because arm_setup_stack was usually called
> after arm_cpu_lowlevel_init, but with ENTRY_FUNCTION_WITHSTACK, SP is
> initialized early on in the naked entry point with
> arm_cpu_lowlevel_init() being called after that.
> 
> This can lead to spurious boot hangs in the Raspberry Pi 2 and 3 entry
> points. Fix this by always saving sp to r3 and restoring it, like we do
> with lr. This is safe to do, because r3 isn't clobbered by any
> instruction in arm_cpu_lowlevel_init() and because it's an argument
> register, callers have to expect it being overwritten by the callee.
> 
> Fixes: b267578d0567 ("ARM: rpi: use ENTRY_FUNCTION_WITHSTACK to prepare for ARM64 support")
> Fixes: 41292192c01b ("ARM: safely switch from HYP to SVC mode if required")
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> v1 -> v2:
>   - add source code comment on why sp is saved (Sascha)
> ---
>  arch/arm/cpu/lowlevel.S | 3 +++
>  1 file changed, 3 insertions(+)

Applied, thanks

Sascha

> 
> diff --git a/arch/arm/cpu/lowlevel.S b/arch/arm/cpu/lowlevel.S
> index 5a7dd3c2093f..960a92b78c0a 100644
> --- a/arch/arm/cpu/lowlevel.S
> +++ b/arch/arm/cpu/lowlevel.S
> @@ -9,6 +9,8 @@
>  ENTRY(arm_cpu_lowlevel_init)
>  	/* save lr, since it may be banked away with a processor mode change */
>  	mov	r2, lr
> +	/* save sp, because possible HYP -> SVC transition below clobbers it */
> +	mov	r3, sp
>  
>  #ifdef CONFIG_CPU_32v7
>  	/* careful: the hyp install corrupts r0 and r1 */
> @@ -77,6 +79,7 @@ THUMB(	orr	r12, r12, #PSR_T_BIT	)
>  
>  	mcr	p15, 0, r12, c1, c0, 0		/* SCTLR */
>  
> +	mov	sp, r3
>  	mov	pc, r2
>  ENDPROC(arm_cpu_lowlevel_init)
>  
> -- 
> 2.30.2
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-06-01 10:14 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-31  8:29 [PATCH master v2] ARM: cpu: don't clobber sp when booted in HYP mode Ahmad Fatoum
2022-06-01  9:51 ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox