diff mbox series

ARM: v7m: remove register save to stack before svc

Message ID 20191013095237.4413-1-afzal.mohd.ma@gmail.com (mailing list archive)
State Mainlined
Commit 2ecb287998a47cc0a766f6071f63bc185f338540
Headers show
Series ARM: v7m: remove register save to stack before svc | expand

Commit Message

afzal mohammed Oct. 13, 2019, 9:52 a.m. UTC
r0-r3 & r12 registers are saved & restored, before & after svc
respectively. Intention was to preserve those registers across thread to
handler mode switch.

On v7-M, hardware saves the register context upon exception in AAPCS
complaint way. Restoring r0-r3 & r12 is done from stack location where
hardware saves it, not from the location on stack where these registers
were saved.

To clarify, on stm32f429 discovery board:

1. before svc, sp - 0x90009ff8
2. r0-r3,r12 saved to 0x90009ff8 - 0x9000a00b
3. upon svc, h/w decrements sp by 32 & pushes registers onto stack
4. after svc,  sp - 0x90009fd8
5. r0-r3,r12 restored from 0x90009fd8 - 0x90009feb

Above means r0-r3,r12 is not restored from the location where they are
saved, but since hardware pushes the registers onto stack, the registers
are restored correctly.

Note that during register saving to stack (step 2), it goes past
0x9000a000. And it seems, based on objdump, there are global symbols
residing there, and it perhaps can cause issues on a non-XIP Kernel
(on XIP, data section is setup later).

Based on the analysis above, manually saving registers onto stack is at
best no-op and at worst can cause data section corruption. Hence remove
storing of registers onto stack before svc.

Fixes: b70cd406d7fe ("ARM: 8671/1: V7M: Preserve registers across switch from Thread to Handler mode")

Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
---
 arch/arm/mm/proc-v7m.S | 1 -
 1 file changed, 1 deletion(-)

Comments

Vladimir Murzin Oct. 14, 2019, 9:03 a.m. UTC | #1
On 10/13/19 10:52 AM, afzal mohammed wrote:
> r0-r3 & r12 registers are saved & restored, before & after svc
> respectively. Intention was to preserve those registers across thread to
> handler mode switch.
> 
> On v7-M, hardware saves the register context upon exception in AAPCS
> complaint way. Restoring r0-r3 & r12 is done from stack location where
> hardware saves it, not from the location on stack where these registers
> were saved.
> 
> To clarify, on stm32f429 discovery board:
> 
> 1. before svc, sp - 0x90009ff8
> 2. r0-r3,r12 saved to 0x90009ff8 - 0x9000a00b
> 3. upon svc, h/w decrements sp by 32 & pushes registers onto stack
> 4. after svc,  sp - 0x90009fd8
> 5. r0-r3,r12 restored from 0x90009fd8 - 0x90009feb
> 
> Above means r0-r3,r12 is not restored from the location where they are
> saved, but since hardware pushes the registers onto stack, the registers
> are restored correctly.
> 
> Note that during register saving to stack (step 2), it goes past
> 0x9000a000. And it seems, based on objdump, there are global symbols
> residing there, and it perhaps can cause issues on a non-XIP Kernel
> (on XIP, data section is setup later).
> 
> Based on the analysis above, manually saving registers onto stack is at
> best no-op and at worst can cause data section corruption. Hence remove
> storing of registers onto stack before svc.
> 
> Fixes: b70cd406d7fe ("ARM: 8671/1: V7M: Preserve registers across switch from Thread to Handler mode")
> 
> Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
> ---
>  arch/arm/mm/proc-v7m.S | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm/mm/proc-v7m.S b/arch/arm/mm/proc-v7m.S
> index efebf4120a0c..1a49d503eafc 100644
> --- a/arch/arm/mm/proc-v7m.S
> +++ b/arch/arm/mm/proc-v7m.S
> @@ -132,7 +132,6 @@ ENDPROC(cpu_cm7_proc_fin)
>  	dsb
>  	mov	r6, lr			@ save LR
>  	ldr	sp, =init_thread_union + THREAD_START_SP
> -	stmia	sp, {r0-r3, r12}
>  	cpsie	i
>  	svc	#0
>  1:	cpsid	i
> 

Acked-by: Vladimir Murzin <vladimir.murzin@arm.com>

Thanks
Vladimir
diff mbox series

Patch

diff --git a/arch/arm/mm/proc-v7m.S b/arch/arm/mm/proc-v7m.S
index efebf4120a0c..1a49d503eafc 100644
--- a/arch/arm/mm/proc-v7m.S
+++ b/arch/arm/mm/proc-v7m.S
@@ -132,7 +132,6 @@  ENDPROC(cpu_cm7_proc_fin)
 	dsb
 	mov	r6, lr			@ save LR
 	ldr	sp, =init_thread_union + THREAD_START_SP
-	stmia	sp, {r0-r3, r12}
 	cpsie	i
 	svc	#0
 1:	cpsid	i