diff mbox

[RESEND] mips: Ensure PC update with MTC0 single-stepping

Message ID alpine.DEB.1.10.1411060637400.2881@tp.orcam.me.uk
State New
Headers show

Commit Message

Maciej W. Rozycki Nov. 6, 2014, 8:38 p.m. UTC
Correct the way PC is updated when single-stepping instructions, by 
keeping the old PC only for the BS_EXCP (exception condition) state.

Some MTC0 (and possibly other) instructions switch to the BS_STOP state 
to terminate the current translation block, so that the state transition 
of the simulated CPU resulting from the CP0 operation takes effect with 
the following instruction.  This happens with `mtc0 <reg>,c0_config' for 
example, typically used to set KSEG0 cacheability.

While single-stepping this has a side-effect of not advancing the PC 
past the instruction just executed; subsequent single-step traps will 
stop at the same instruction repeatedly.  Example:

(gdb) stepi
0x80004d24 in _start ()
5: x/i $pc
=> 0x80004d24 <_start+364>:     mfc0    t1,c0_config
(gdb)
0x80004d28 in _start ()
5: x/i $pc
=> 0x80004d28 <_start+368>:     li      at,-8
(gdb)
0x80004d2c in _start ()
5: x/i $pc
=> 0x80004d2c <_start+372>:     and     t1,t1,at
(gdb)
0x80004d30 in _start ()
5: x/i $pc
=> 0x80004d30 <_start+376>:     ori     t1,t1,0x3
(gdb)
0x80004d34 in _start ()
5: x/i $pc
=> 0x80004d34 <_start+380>:     mtc0    t1,c0_config
(gdb)
0x80004d34 in _start ()
5: x/i $pc
=> 0x80004d34 <_start+380>:     mtc0    t1,c0_config
(gdb)
0x80004d34 in _start ()
5: x/i $pc
=> 0x80004d34 <_start+380>:     mtc0    t1,c0_config
(gdb)
0x80004d34 in _start ()
5: x/i $pc
=> 0x80004d34 <_start+380>:     mtc0    t1,c0_config
(gdb)

-- oops!

Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
---
 It's been lost and waited for too long now, the original submission has 
been archived here:

http://lists.gnu.org/archive/html/qemu-devel/2012-06/msg01227.html

I have verified with a manual check that the issue is still there and 
that the fix still works.  Please apply.

  Maciej

qemu-mips-mtc0-step.diff

Comments

Leon Alrae Nov. 7, 2014, 10:42 a.m. UTC | #1
On 06/11/2014 20:38, Maciej W. Rozycki wrote:
> Correct the way PC is updated when single-stepping instructions, by 
> keeping the old PC only for the BS_EXCP (exception condition) state.
> 
> Some MTC0 (and possibly other) instructions switch to the BS_STOP state 
> to terminate the current translation block, so that the state transition 
> of the simulated CPU resulting from the CP0 operation takes effect with 
> the following instruction.  This happens with `mtc0 <reg>,c0_config' for 
> example, typically used to set KSEG0 cacheability.
> 
> While single-stepping this has a side-effect of not advancing the PC 
> past the instruction just executed; subsequent single-step traps will 
> stop at the same instruction repeatedly.  Example:
> 
> (gdb) stepi
> 0x80004d24 in _start ()
> 5: x/i $pc
> => 0x80004d24 <_start+364>:     mfc0    t1,c0_config
> (gdb)
> 0x80004d28 in _start ()
> 5: x/i $pc
> => 0x80004d28 <_start+368>:     li      at,-8
> (gdb)
> 0x80004d2c in _start ()
> 5: x/i $pc
> => 0x80004d2c <_start+372>:     and     t1,t1,at
> (gdb)
> 0x80004d30 in _start ()
> 5: x/i $pc
> => 0x80004d30 <_start+376>:     ori     t1,t1,0x3
> (gdb)
> 0x80004d34 in _start ()
> 5: x/i $pc
> => 0x80004d34 <_start+380>:     mtc0    t1,c0_config
> (gdb)
> 0x80004d34 in _start ()
> 5: x/i $pc
> => 0x80004d34 <_start+380>:     mtc0    t1,c0_config
> (gdb)
> 0x80004d34 in _start ()
> 5: x/i $pc
> => 0x80004d34 <_start+380>:     mtc0    t1,c0_config
> (gdb)
> 0x80004d34 in _start ()
> 5: x/i $pc
> => 0x80004d34 <_start+380>:     mtc0    t1,c0_config
> (gdb)
> 
> -- oops!
> 
> Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
> ---
>  It's been lost and waited for too long now, the original submission has 
> been archived here:
> 
> http://lists.gnu.org/archive/html/qemu-devel/2012-06/msg01227.html
> 
> I have verified with a manual check that the issue is still there and 
> that the fix still works.  Please apply.
> 
>   Maciej
> 
> qemu-mips-mtc0-step.diff
> Index: qemu-git-trunk/target-mips/translate.c
> ===================================================================
> --- qemu-git-trunk.orig/target-mips/translate.c	2014-11-02 18:51:10.838947420 +0000
> +++ qemu-git-trunk/target-mips/translate.c	2014-11-02 18:51:14.838939198 +0000
> @@ -17522,7 +17522,7 @@ gen_intermediate_code_internal(MIPSCPU *
>          gen_io_end();
>      }
>      if (cs->singlestep_enabled && ctx.bstate != BS_BRANCH) {
> -        save_cpu_state(&ctx, ctx.bstate == BS_NONE);
> +        save_cpu_state(&ctx, ctx.bstate != BS_EXCP);
>          gen_helper_0e0i(raise_exception, EXCP_DEBUG);
>      } else {
>          switch (ctx.bstate) {
> 

Good fix for 2.2, thanks.

Reviewed-by: Leon Alrae <leon.alrae@imgtec.com>

Regards,
Leon
diff mbox

Patch

Index: qemu-git-trunk/target-mips/translate.c
===================================================================
--- qemu-git-trunk.orig/target-mips/translate.c	2014-11-02 18:51:10.838947420 +0000
+++ qemu-git-trunk/target-mips/translate.c	2014-11-02 18:51:14.838939198 +0000
@@ -17522,7 +17522,7 @@  gen_intermediate_code_internal(MIPSCPU *
         gen_io_end();
     }
     if (cs->singlestep_enabled && ctx.bstate != BS_BRANCH) {
-        save_cpu_state(&ctx, ctx.bstate == BS_NONE);
+        save_cpu_state(&ctx, ctx.bstate != BS_EXCP);
         gen_helper_0e0i(raise_exception, EXCP_DEBUG);
     } else {
         switch (ctx.bstate) {