[U-Boot] [PATCH] BUGFIX: arm: armv7: data abort in get_bad_stack_swi
When swi instruction is executed, it is expected to get message "software interrupt" in console and dump registers and reboot, as do_software_interrupt() in arch/arm/lib/interrupts.c. Actually it causes data abort in get_bad_stack_swi macro in arch/arm/cpu/v7/start.S. This patch fixes this problem.
Signed-off-by: Tetsuyuki Kobayashi koba@kmckk.co.jp --- Hello, I found this bug on KZM-A9-GT board (aka. kzm9g) on v2013.04-rc2. And tested this patch on the board. I found the same typo in arch/arm/cpu/{arm1136,arm1176,pxa}/start.S.
arch/arm/cpu/armv7/start.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 36a4c3c..9bf950b 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -465,7 +465,7 @@ ENDPROC(cpu_init_crit) @ spots for abort stack str lr, [r0] @ save caller lr in position 0 @ of saved stack - mrs r0, spsr @ get the spsr + mrs lr, spsr @ get the spsr str lr, [r0, #4] @ save spsr in position 1 of @ saved stack ldr r0, [r13] @ restore r0
Hi Tetsuyuki,
On Thu, 4 Apr 2013 13:52:47 +0900, Tetsuyuki Kobayashi koba@kmckk.co.jp wrote:
When swi instruction is executed, it is expected to get message "software interrupt" in console and dump registers and reboot, as do_software_interrupt() in arch/arm/lib/interrupts.c. Actually it causes data abort in get_bad_stack_swi macro in arch/arm/cpu/v7/start.S. This patch fixes this problem.
Signed-off-by: Tetsuyuki Kobayashi koba@kmckk.co.jp
Hello, I found this bug on KZM-A9-GT board (aka. kzm9g) on v2013.04-rc2. And tested this patch on the board. I found the same typo in arch/arm/cpu/{arm1136,arm1176,pxa}/start.S.
The can you please submit a V2 patch fixing all four of them?
Amicalement,
When swi instruction is executed, it is expected to get message "software interrupt" in console and dump registers and reboot, as do_software_interrupt() in arch/arm/lib/interrupts.c. But, actually it causes data abort accessing wrong address in get_bad_stack_swi macro in arch/arm/cpu/v7/start.S. This patch fixes this problem.
The same mistake in arch/arm/cpu/{arm1136,arm1176,pxa}/start.S.
Signed-off-by: Tetsuyuki Kobayashi koba@kmckk.co.jp --- Changes for v2: - added arch/arm/cpu/{arm1136,arm1176,pxa}/start.S (But not tested, because I don't have test boards of them) arm/arm/cpu/armv7/start.S is tested on KZM-A9-GT board.
arch/arm/cpu/arm1136/start.S | 2 +- arch/arm/cpu/arm1176/start.S | 2 +- arch/arm/cpu/armv7/start.S | 2 +- arch/arm/cpu/pxa/start.S | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S index eba2324..7b9db2b 100644 --- a/arch/arm/cpu/arm1136/start.S +++ b/arch/arm/cpu/arm1136/start.S @@ -392,7 +392,7 @@ cpu_init_crit: str r0, [r13] @ save R0's value. ldr r0, IRQ_STACK_START_IN @ get data regions start str lr, [r0] @ save caller lr in position 0 of saved stack - mrs r0, spsr @ get the spsr + mrs lr, spsr @ get the spsr str lr, [r0, #4] @ save spsr in position 1 of saved stack ldr r0, [r13] @ restore r0 add r13, r13, #4 @ pop stack entry diff --git a/arch/arm/cpu/arm1176/start.S b/arch/arm/cpu/arm1176/start.S index 3c291fb..8534fb8 100644 --- a/arch/arm/cpu/arm1176/start.S +++ b/arch/arm/cpu/arm1176/start.S @@ -480,7 +480,7 @@ phy_last_jump: /* save caller lr in position 0 of saved stack */ str lr, [r0] /* get the spsr */ - mrs r0, spsr + mrs lr, spsr /* save spsr in position 1 of saved stack */ str lr, [r0, #4] /* restore r0 */ diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 36a4c3c..9bf950b 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -465,7 +465,7 @@ ENDPROC(cpu_init_crit) @ spots for abort stack str lr, [r0] @ save caller lr in position 0 @ of saved stack - mrs r0, spsr @ get the spsr + mrs lr, spsr @ get the spsr str lr, [r0, #4] @ save spsr in position 1 of @ saved stack ldr r0, [r13] @ restore r0 diff --git a/arch/arm/cpu/pxa/start.S b/arch/arm/cpu/pxa/start.S index 456a783..18131bb 100644 --- a/arch/arm/cpu/pxa/start.S +++ b/arch/arm/cpu/pxa/start.S @@ -387,7 +387,7 @@ cpu_init_crit: str r0, [r13] @ save R0's value. ldr r0, IRQ_STACK_START_IN @ get data regions start str lr, [r0] @ save caller lr in position 0 of saved stack - mrs r0, spsr @ get the spsr + mrs lr, spsr @ get the spsr str lr, [r0, #4] @ save spsr in position 1 of saved stack ldr r0, [r13] @ restore r0 add r13, r13, #4 @ pop stack entry
Hi Tetsuyuki,
On Fri, 5 Apr 2013 10:45:14 +0900, Tetsuyuki Kobayashi koba@kmckk.co.jp wrote:
When swi instruction is executed, it is expected to get message "software interrupt" in console and dump registers and reboot, as do_software_interrupt() in arch/arm/lib/interrupts.c. But, actually it causes data abort accessing wrong address in get_bad_stack_swi macro in arch/arm/cpu/v7/start.S. This patch fixes this problem.
The same mistake in arch/arm/cpu/{arm1136,arm1176,pxa}/start.S.
Signed-off-by: Tetsuyuki Kobayashi koba@kmckk.co.jp
Changes for v2:
- added arch/arm/cpu/{arm1136,arm1176,pxa}/start.S (But not tested, because I don't have test boards of them) arm/arm/cpu/armv7/start.S is tested on KZM-A9-GT board.
arch/arm/cpu/arm1136/start.S | 2 +- arch/arm/cpu/arm1176/start.S | 2 +- arch/arm/cpu/armv7/start.S | 2 +- arch/arm/cpu/pxa/start.S | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S index eba2324..7b9db2b 100644 --- a/arch/arm/cpu/arm1136/start.S +++ b/arch/arm/cpu/arm1136/start.S @@ -392,7 +392,7 @@ cpu_init_crit: str r0, [r13] @ save R0's value. ldr r0, IRQ_STACK_START_IN @ get data regions start str lr, [r0] @ save caller lr in position 0 of saved stack
- mrs r0, spsr @ get the spsr
- mrs lr, spsr @ get the spsr str lr, [r0, #4] @ save spsr in position 1 of saved stack ldr r0, [r13] @ restore r0 add r13, r13, #4 @ pop stack entry
Sorry for not checking this in V1, but I see that get_bad_stack_swi does not preserve lr, so when bad_save_user_regs is expanded, it will save the wrong value for lr. You need to restore lr from [r0] before you restore r0 from [r13].
diff --git a/arch/arm/cpu/arm1176/start.S b/arch/arm/cpu/arm1176/start.S
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
diff --git a/arch/arm/cpu/pxa/start.S b/arch/arm/cpu/pxa/start.S
Ditto for all four files, of course.
Amicalement,
Hi Albert
(04/05/2013 04:04 PM), Albert ARIBAUD wrote:
Hi Tetsuyuki,
On Fri, 5 Apr 2013 10:45:14 +0900, Tetsuyuki Kobayashi koba@kmckk.co.jp wrote:
When swi instruction is executed, it is expected to get message "software interrupt" in console and dump registers and reboot, as do_software_interrupt() in arch/arm/lib/interrupts.c. But, actually it causes data abort accessing wrong address in get_bad_stack_swi macro in arch/arm/cpu/v7/start.S. This patch fixes this problem.
The same mistake in arch/arm/cpu/{arm1136,arm1176,pxa}/start.S.
Signed-off-by: Tetsuyuki Kobayashi koba@kmckk.co.jp
Changes for v2:
- added arch/arm/cpu/{arm1136,arm1176,pxa}/start.S (But not tested, because I don't have test boards of them) arm/arm/cpu/armv7/start.S is tested on KZM-A9-GT board.
arch/arm/cpu/arm1136/start.S | 2 +- arch/arm/cpu/arm1176/start.S | 2 +- arch/arm/cpu/armv7/start.S | 2 +- arch/arm/cpu/pxa/start.S | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S index eba2324..7b9db2b 100644 --- a/arch/arm/cpu/arm1136/start.S +++ b/arch/arm/cpu/arm1136/start.S @@ -392,7 +392,7 @@ cpu_init_crit: str r0, [r13] @ save R0's value. ldr r0, IRQ_STACK_START_IN @ get data regions start str lr, [r0] @ save caller lr in position 0 of saved stack
- mrs r0, spsr @ get the spsr
- mrs lr, spsr @ get the spsr str lr, [r0, #4] @ save spsr in position 1 of saved stack ldr r0, [r13] @ restore r0 add r13, r13, #4 @ pop stack entry
Sorry for not checking this in V1, but I see that get_bad_stack_swi does not preserve lr, so when bad_save_user_regs is expanded, it will save the wrong value for lr. You need to restore lr from [r0] before you restore r0 from [r13].
Thank you, I was not aware that. The dumped lr value was wrong. I am going to post V3 patch for all 4 files.
diff --git a/arch/arm/cpu/arm1176/start.S b/arch/arm/cpu/arm1176/start.S
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
diff --git a/arch/arm/cpu/pxa/start.S b/arch/arm/cpu/pxa/start.S
Ditto for all four files, of course.
Amicalement,
When swi instruction is executed, it is expected to get message "software interrupt" in console and dump registers and reboot, as do_software_interrupt() in arch/arm/lib/interrupts.c. But, actually it causes data abort accessing wrong address in get_bad_stack_swi macro in arch/arm/cpu/v7/start.S. This patch fixes this problem.
The same mistake in arch/arm/cpu/{arm1136,arm1176,pxa}/start.S.
Signed-off-by: Tetsuyuki Kobayashi koba@kmckk.co.jp --- Changes for v2: - added arch/arm/cpu/{arm1136,arm1176,pxa}/start.S Changes for v3: - restore lr register in all 4 files. Only arm/arm/cpu/armv7/start.S is tested on KZM-A9-GT board.
arch/arm/cpu/arm1136/start.S | 3 ++- arch/arm/cpu/arm1176/start.S | 4 +++- arch/arm/cpu/armv7/start.S | 3 ++- arch/arm/cpu/pxa/start.S | 3 ++- 4 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S index eba2324..bccfdbf 100644 --- a/arch/arm/cpu/arm1136/start.S +++ b/arch/arm/cpu/arm1136/start.S @@ -392,8 +392,9 @@ cpu_init_crit: str r0, [r13] @ save R0's value. ldr r0, IRQ_STACK_START_IN @ get data regions start str lr, [r0] @ save caller lr in position 0 of saved stack - mrs r0, spsr @ get the spsr + mrs lr, spsr @ get the spsr str lr, [r0, #4] @ save spsr in position 1 of saved stack + ldr lr, [r0] @ restore lr ldr r0, [r13] @ restore r0 add r13, r13, #4 @ pop stack entry .endm diff --git a/arch/arm/cpu/arm1176/start.S b/arch/arm/cpu/arm1176/start.S index 3c291fb..694f79d 100644 --- a/arch/arm/cpu/arm1176/start.S +++ b/arch/arm/cpu/arm1176/start.S @@ -480,9 +480,11 @@ phy_last_jump: /* save caller lr in position 0 of saved stack */ str lr, [r0] /* get the spsr */ - mrs r0, spsr + mrs lr, spsr /* save spsr in position 1 of saved stack */ str lr, [r0, #4] + /* restore lr */ + ldr lr, [r0] /* restore r0 */ ldr r0, [r13] /* pop stack entry */ diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 36a4c3c..e5e157b 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -465,9 +465,10 @@ ENDPROC(cpu_init_crit) @ spots for abort stack str lr, [r0] @ save caller lr in position 0 @ of saved stack - mrs r0, spsr @ get the spsr + mrs lr, spsr @ get the spsr str lr, [r0, #4] @ save spsr in position 1 of @ saved stack + ldr lr, [r0] @ restore lr ldr r0, [r13] @ restore r0 add r13, r13, #4 @ pop stack entry .endm diff --git a/arch/arm/cpu/pxa/start.S b/arch/arm/cpu/pxa/start.S index 456a783..9c525e8 100644 --- a/arch/arm/cpu/pxa/start.S +++ b/arch/arm/cpu/pxa/start.S @@ -387,8 +387,9 @@ cpu_init_crit: str r0, [r13] @ save R0's value. ldr r0, IRQ_STACK_START_IN @ get data regions start str lr, [r0] @ save caller lr in position 0 of saved stack - mrs r0, spsr @ get the spsr + mrs lr, spsr @ get the spsr str lr, [r0, #4] @ save spsr in position 1 of saved stack + ldr lr, [r0] @ restore lr ldr r0, [r13] @ restore r0 add r13, r13, #4 @ pop stack entry .endm
Hello Albert
How about this patch?
(04/05/2013 07:12 PM), Tetsuyuki Kobayashi wrote:
When swi instruction is executed, it is expected to get message "software interrupt" in console and dump registers and reboot, as do_software_interrupt() in arch/arm/lib/interrupts.c. But, actually it causes data abort accessing wrong address in get_bad_stack_swi macro in arch/arm/cpu/v7/start.S. This patch fixes this problem.
The same mistake in arch/arm/cpu/{arm1136,arm1176,pxa}/start.S.
Signed-off-by: Tetsuyuki Kobayashi koba@kmckk.co.jp
Changes for v2:
- added arch/arm/cpu/{arm1136,arm1176,pxa}/start.S
Changes for v3:
- restore lr register in all 4 files. Only arm/arm/cpu/armv7/start.S is tested on KZM-A9-GT board.
arch/arm/cpu/arm1136/start.S | 3 ++- arch/arm/cpu/arm1176/start.S | 4 +++- arch/arm/cpu/armv7/start.S | 3 ++- arch/arm/cpu/pxa/start.S | 3 ++- 4 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S index eba2324..bccfdbf 100644 --- a/arch/arm/cpu/arm1136/start.S +++ b/arch/arm/cpu/arm1136/start.S @@ -392,8 +392,9 @@ cpu_init_crit: str r0, [r13] @ save R0's value. ldr r0, IRQ_STACK_START_IN @ get data regions start str lr, [r0] @ save caller lr in position 0 of saved stack
- mrs r0, spsr @ get the spsr
- mrs lr, spsr @ get the spsr str lr, [r0, #4] @ save spsr in position 1 of saved stack
- ldr lr, [r0] @ restore lr ldr r0, [r13] @ restore r0 add r13, r13, #4 @ pop stack entry .endm
diff --git a/arch/arm/cpu/arm1176/start.S b/arch/arm/cpu/arm1176/start.S index 3c291fb..694f79d 100644 --- a/arch/arm/cpu/arm1176/start.S +++ b/arch/arm/cpu/arm1176/start.S @@ -480,9 +480,11 @@ phy_last_jump: /* save caller lr in position 0 of saved stack */ str lr, [r0] /* get the spsr */
- mrs r0, spsr
- mrs lr, spsr /* save spsr in position 1 of saved stack */ str lr, [r0, #4]
- /* restore lr */
- ldr lr, [r0] /* restore r0 */ ldr r0, [r13] /* pop stack entry */
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 36a4c3c..e5e157b 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -465,9 +465,10 @@ ENDPROC(cpu_init_crit) @ spots for abort stack str lr, [r0] @ save caller lr in position 0 @ of saved stack
- mrs r0, spsr @ get the spsr
- mrs lr, spsr @ get the spsr str lr, [r0, #4] @ save spsr in position 1 of @ saved stack
- ldr lr, [r0] @ restore lr ldr r0, [r13] @ restore r0 add r13, r13, #4 @ pop stack entry .endm
diff --git a/arch/arm/cpu/pxa/start.S b/arch/arm/cpu/pxa/start.S index 456a783..9c525e8 100644 --- a/arch/arm/cpu/pxa/start.S +++ b/arch/arm/cpu/pxa/start.S @@ -387,8 +387,9 @@ cpu_init_crit: str r0, [r13] @ save R0's value. ldr r0, IRQ_STACK_START_IN @ get data regions start str lr, [r0] @ save caller lr in position 0 of saved stack
- mrs r0, spsr @ get the spsr
- mrs lr, spsr @ get the spsr str lr, [r0, #4] @ save spsr in position 1 of saved stack
- ldr lr, [r0] @ restore lr ldr r0, [r13] @ restore r0 add r13, r13, #4 @ pop stack entry .endm
Hi Tetsuyuki,
On Fri, 5 Apr 2013 19:12:51 +0900, Tetsuyuki Kobayashi koba@kmckk.co.jp wrote:
When swi instruction is executed, it is expected to get message "software interrupt" in console and dump registers and reboot, as do_software_interrupt() in arch/arm/lib/interrupts.c. But, actually it causes data abort accessing wrong address in get_bad_stack_swi macro in arch/arm/cpu/v7/start.S. This patch fixes this problem.
The same mistake in arch/arm/cpu/{arm1136,arm1176,pxa}/start.S.
Signed-off-by: Tetsuyuki Kobayashi koba@kmckk.co.jp
Changes for v2:
- added arch/arm/cpu/{arm1136,arm1176,pxa}/start.S
Changes for v3:
- restore lr register in all 4 files. Only arm/arm/cpu/armv7/start.S is tested on KZM-A9-GT board.
arch/arm/cpu/arm1136/start.S | 3 ++- arch/arm/cpu/arm1176/start.S | 4 +++- arch/arm/cpu/armv7/start.S | 3 ++- arch/arm/cpu/pxa/start.S | 3 ++- 4 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S index eba2324..bccfdbf 100644 --- a/arch/arm/cpu/arm1136/start.S +++ b/arch/arm/cpu/arm1136/start.S @@ -392,8 +392,9 @@ cpu_init_crit: str r0, [r13] @ save R0's value. ldr r0, IRQ_STACK_START_IN @ get data regions start str lr, [r0] @ save caller lr in position 0 of saved stack
- mrs r0, spsr @ get the spsr
- mrs lr, spsr @ get the spsr str lr, [r0, #4] @ save spsr in position 1 of saved stack
- ldr lr, [r0] @ restore lr ldr r0, [r13] @ restore r0 add r13, r13, #4 @ pop stack entry .endm
diff --git a/arch/arm/cpu/arm1176/start.S b/arch/arm/cpu/arm1176/start.S index 3c291fb..694f79d 100644 --- a/arch/arm/cpu/arm1176/start.S +++ b/arch/arm/cpu/arm1176/start.S @@ -480,9 +480,11 @@ phy_last_jump: /* save caller lr in position 0 of saved stack */ str lr, [r0] /* get the spsr */
- mrs r0, spsr
- mrs lr, spsr /* save spsr in position 1 of saved stack */ str lr, [r0, #4]
- /* restore lr */
- ldr lr, [r0] /* restore r0 */ ldr r0, [r13] /* pop stack entry */
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 36a4c3c..e5e157b 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -465,9 +465,10 @@ ENDPROC(cpu_init_crit) @ spots for abort stack str lr, [r0] @ save caller lr in position 0 @ of saved stack
- mrs r0, spsr @ get the spsr
- mrs lr, spsr @ get the spsr str lr, [r0, #4] @ save spsr in position 1 of @ saved stack
- ldr lr, [r0] @ restore lr ldr r0, [r13] @ restore r0 add r13, r13, #4 @ pop stack entry .endm
diff --git a/arch/arm/cpu/pxa/start.S b/arch/arm/cpu/pxa/start.S index 456a783..9c525e8 100644 --- a/arch/arm/cpu/pxa/start.S +++ b/arch/arm/cpu/pxa/start.S @@ -387,8 +387,9 @@ cpu_init_crit: str r0, [r13] @ save R0's value. ldr r0, IRQ_STACK_START_IN @ get data regions start str lr, [r0] @ save caller lr in position 0 of saved stack
- mrs r0, spsr @ get the spsr
- mrs lr, spsr @ get the spsr str lr, [r0, #4] @ save spsr in position 1 of saved stack
- ldr lr, [r0] @ restore lr ldr r0, [r13] @ restore r0 add r13, r13, #4 @ pop stack entry .endm
Applied to u-boot-arm/master, thanks!
Amicalement,
participants (2)
-
Albert ARIBAUD -
Tetsuyuki Kobayashi