diff mbox

[v2,3/3] m68k: fix usp processing on interrupt entry and exception exit

Message ID 1434721406-25288-4-git-send-email-gerg@uclinux.org
State New
Headers show

Commit Message

Greg Ungerer June 19, 2015, 1:43 p.m. UTC
From: Greg Ungerer <gerg@uclinux.org>

The action to potentially switch sp register is not occurring at the correct
point in the interrupt entry or exception exit sequences.

For the interrupt entry case the sp on entry is used to create the stack
exception frame - but this may well be the user stack pointer, since we
haven't done the switch yet. Re-order the flow to switch the sp regs then
use the current sp to create the exception frame.

For the return from exception case the code is unwinding the sp after
switching sp registers. But it should always unwind the supervisor sp
first, then carry out any required sp switch.

Note that these problems don't effect operation unless the user sp bit is
set in the CACR register. Only a single sp is used in the default power up
state. Previously Linux only used this single sp mode. But modern versions
of Linux use the user sp mode now, so we need correct behavior for Linux
to work.

Signed-off-by: Greg Ungerer <gerg@uclinux.org>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 target-m68k/op_helper.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Laurent Vivier June 19, 2015, 7:35 p.m. UTC | #1
Le 19/06/2015 15:43, gerg@uclinux.org a écrit :
> From: Greg Ungerer <gerg@uclinux.org>
> 
> The action to potentially switch sp register is not occurring at the correct
> point in the interrupt entry or exception exit sequences.
> 
> For the interrupt entry case the sp on entry is used to create the stack
> exception frame - but this may well be the user stack pointer, since we
> haven't done the switch yet. Re-order the flow to switch the sp regs then
> use the current sp to create the exception frame.
> 
> For the return from exception case the code is unwinding the sp after
> switching sp registers. But it should always unwind the supervisor sp
> first, then carry out any required sp switch.
> 
> Note that these problems don't effect operation unless the user sp bit is
> set in the CACR register. Only a single sp is used in the default power up
> state. Previously Linux only used this single sp mode. But modern versions
> of Linux use the user sp mode now, so we need correct behavior for Linux
> to work.
> 
> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
>  target-m68k/op_helper.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Greg Ungerer June 21, 2015, 11:11 p.m. UTC | #2
Hi Laurent,

On 20/06/15 05:35, Laurent Vivier wrote:
> Le 19/06/2015 15:43, gerg@uclinux.org a écrit :
>> From: Greg Ungerer <gerg@uclinux.org>
>>
>> The action to potentially switch sp register is not occurring at the correct
>> point in the interrupt entry or exception exit sequences.
>>
>> For the interrupt entry case the sp on entry is used to create the stack
>> exception frame - but this may well be the user stack pointer, since we
>> haven't done the switch yet. Re-order the flow to switch the sp regs then
>> use the current sp to create the exception frame.
>>
>> For the return from exception case the code is unwinding the sp after
>> switching sp registers. But it should always unwind the supervisor sp
>> first, then carry out any required sp switch.
>>
>> Note that these problems don't effect operation unless the user sp bit is
>> set in the CACR register. Only a single sp is used in the default power up
>> state. Previously Linux only used this single sp mode. But modern versions
>> of Linux use the user sp mode now, so we need correct behavior for Linux
>> to work.
>>
>> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
>> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>>  target-m68k/op_helper.c | 7 +++----
>>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Thanks for the reviews.

Regards
Greg
diff mbox

Patch

diff --git a/target-m68k/op_helper.c b/target-m68k/op_helper.c
index 06661f5..3a0d16f 100644
--- a/target-m68k/op_helper.c
+++ b/target-m68k/op_helper.c
@@ -63,8 +63,8 @@  static void do_rte(CPUM68KState *env)
     env->pc = cpu_ldl_kernel(env, sp + 4);
     sp |= (fmt >> 28) & 3;
     env->sr = fmt & 0xffff;
-    m68k_switch_sp(env);
     env->aregs[7] = sp + 8;
+    m68k_switch_sp(env);
 }
 
 static void do_interrupt_all(CPUM68KState *env, int is_hw)
@@ -108,10 +108,7 @@  static void do_interrupt_all(CPUM68KState *env, int is_hw)
 
     vector = cs->exception_index << 2;
 
-    sp = env->aregs[7];
-
     fmt |= 0x40000000;
-    fmt |= (sp & 3) << 28;
     fmt |= vector << 16;
     fmt |= env->sr;
 
@@ -121,6 +118,8 @@  static void do_interrupt_all(CPUM68KState *env, int is_hw)
         env->sr &= ~SR_M;
     }
     m68k_switch_sp(env);
+    sp = env->aregs[7];
+    fmt |= (sp & 3) << 28;
 
     /* ??? This could cause MMU faults.  */
     sp &= ~3;