Re: [U-Boot] [PATCH v2] BUGFIX: arm: data abort in get_bad_stack_swi

2013-04-05 Thread Albert ARIBAUD 
Hi Tetsuyuki,

On Fri,  5 Apr 2013 10:45:14 +0900, Tetsuyuki Kobayashi
k...@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 k...@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,
-- 
Albert.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2] BUGFIX: arm: data abort in get_bad_stack_swi

2013-04-05 Thread Tetsuyuki Kobayashi 
Hi Albert

(04/05/2013 04:04 PM), Albert ARIBAUD wrote:
 Hi Tetsuyuki,
 
 On Fri,  5 Apr 2013 10:45:14 +0900, Tetsuyuki Kobayashi
 k...@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 k...@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,
 

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2] BUGFIX: arm: data abort in get_bad_stack_swi

2013-04-04 Thread Tetsuyuki Kobayashi 
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 k...@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
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot