Re: [Qemu-devel] [PATCH RESEND] mips: Ensure PC update with MTC0 single-stepping

[Leon Alrae <leon.alrae@imgtec.com>]

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