* [Qemu-devel] [PATCH] MIPS/system: MTC0 single-stepping PC update fix
@ 2012-06-08 1:05 Maciej W. Rozycki
2012-06-12 14:32 ` Richard Henderson
2014-11-06 20:38 ` [Qemu-devel] [PATCH RESEND] mips: Ensure PC update with MTC0 single-stepping Maciej W. Rozycki
0 siblings, 2 replies; 4+ messages in thread
From: Maciej W. Rozycki @ 2012-06-08 1:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Maciej W. Rozycki, Aurelien Jarno
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 for "mtc0 <reg>,c0_config" for
example.
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. This is obviously incorrect and (with my
limited understanding of QEMU internals) is fixed easily as below, making
the old PC be kept only for the BS_EXCP (exception condition) state.
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>
---
Please apply,
Maciej
qemu-mips-mtc0-step.diff
Index: qemu-git-trunk/target-mips/translate.c
===================================================================
--- qemu-git-trunk.orig/target-mips/translate.c 2012-06-04 04:16:57.755560324 +0100
+++ qemu-git-trunk/target-mips/translate.c 2012-06-04 05:01:42.435594656 +0100
@@ -12494,7 +12494,7 @@ gen_intermediate_code_internal (CPUMIPSS
if (tb->cflags & CF_LAST_IO)
gen_io_end();
if (env->singlestep_enabled && ctx.bstate != BS_BRANCH) {
- save_cpu_state(&ctx, ctx.bstate == BS_NONE);
+ save_cpu_state(&ctx, ctx.bstate != BS_EXCP);
gen_helper_0i(raise_exception, EXCP_DEBUG);
} else {
switch (ctx.bstate) {
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] MIPS/system: MTC0 single-stepping PC update fix
2012-06-08 1:05 [Qemu-devel] [PATCH] MIPS/system: MTC0 single-stepping PC update fix Maciej W. Rozycki
@ 2012-06-12 14:32 ` Richard Henderson
2014-11-06 20:38 ` [Qemu-devel] [PATCH RESEND] mips: Ensure PC update with MTC0 single-stepping Maciej W. Rozycki
1 sibling, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2012-06-12 14:32 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: qemu-devel, Aurelien Jarno
On 2012-06-07 18:05, Maciej W. Rozycki wrote:
> if (env->singlestep_enabled && ctx.bstate != BS_BRANCH) {
> - save_cpu_state(&ctx, ctx.bstate == BS_NONE);
> + save_cpu_state(&ctx, ctx.bstate != BS_EXCP);
> gen_helper_0i(raise_exception, EXCP_DEBUG);
Reviewed-by: Richard Henderson <rth@twiddle.net>
r~
^ permalink raw reply [flat|nested] 4+ messages in thread* [Qemu-devel] [PATCH RESEND] mips: Ensure PC update with MTC0 single-stepping
2012-06-08 1:05 [Qemu-devel] [PATCH] MIPS/system: MTC0 single-stepping PC update fix Maciej W. Rozycki
2012-06-12 14:32 ` Richard Henderson
@ 2014-11-06 20:38 ` Maciej W. Rozycki
2014-11-07 10:42 ` Leon Alrae
1 sibling, 1 reply; 4+ messages in thread
From: Maciej W. Rozycki @ 2014-11-06 20:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Leon Alrae, Aurelien Jarno
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) {
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH RESEND] mips: Ensure PC update with MTC0 single-stepping
2014-11-06 20:38 ` [Qemu-devel] [PATCH RESEND] mips: Ensure PC update with MTC0 single-stepping Maciej W. Rozycki
@ 2014-11-07 10:42 ` Leon Alrae
0 siblings, 0 replies; 4+ messages in thread
From: Leon Alrae @ 2014-11-07 10:42 UTC (permalink / raw)
To: Maciej W. Rozycki, qemu-devel; +Cc: Aurelien Jarno
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-11-07 10:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-08 1:05 [Qemu-devel] [PATCH] MIPS/system: MTC0 single-stepping PC update fix Maciej W. Rozycki
2012-06-12 14:32 ` Richard Henderson
2014-11-06 20:38 ` [Qemu-devel] [PATCH RESEND] mips: Ensure PC update with MTC0 single-stepping Maciej W. Rozycki
2014-11-07 10:42 ` Leon Alrae
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).