Re: [PATCH][Qemu-devel] Single stepping for PPC broken!
Marius Groeger wrote:
On Wed, 9 Jan 2008, Marius Groeger wrote:
On Wed, 9 Jan 2008, Marius Groeger wrote:
I'm having problems with qemu's (-M prep, -cpu 604) handling of the
MSR_SE bit. My gdbstub can successfully step along regular code, but
qemu chokes when stepping over a branch instruction like blr.
(Needless to say, that same gdbstub works fine on real hardware). I
tried older versions of qemu and found that the code base 8 months ago
worked fine.
I have now verified with booting a Linux image into qemu-system-ppc - same
problem. When stepi'ing over the following sequence, the system chokes on a
bl instruction:
The attached patch fixes the problem, but I have to admit I can't tell
for sure if this doesn't break other things (such as qemu's built-in
GDB server). Could some QEMU ppc expert please comment on this?
Thanks
Marius
The patch you originally attached definitely breaks the back end
debugger connection for qemu. It does point to the heart of the problem
though. The back end debugger uses the same variable to control the
single stepping state as the MSR_SE uses.
Attached is a patch that fixes the issue, as well as a generic problem
in cvs latest where the backend debugger is occasionally missing debug
exceptions on all archs.
Jason.
- Fix generic single step problem in vl.c
* Overwriting the ret code when there was
and interrupt pending causes the debugger
to miss exceptions
- For ppc, split run-time single stepping from the
debugger stub single stepping
* This fixes the hang problems when using single
stepping via the msr_se
Signed-off-by: Jason Wessel [EMAIL PROTECTED]
---
target-ppc/translate.c | 14 --
vl.c |4 ++--
2 files changed, 14 insertions(+), 4 deletions(-)
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -150,6 +150,7 @@ typedef struct DisasContext {
int spe_enabled;
ppc_spr_t *spr_cb; /* Needed to check rights for mfspr/mtspr */
int singlestep_enabled;
+int sys_sstep_enabled;
int dcache_line_size;
} DisasContext;
@@ -2802,8 +2803,10 @@ static always_inline void gen_goto_tb (D
else
#endif
gen_op_b_T1();
-if (ctx-singlestep_enabled)
+ if (unlikely(ctx-sys_sstep_enabled)) {
+gen_update_nip(ctx, ctx-nip);
gen_op_debug();
+}
tcg_gen_exit_tb(0);
}
}
@@ -2984,8 +2987,10 @@ static always_inline void gen_bcond (Dis
#endif
gen_op_btest_T1(ctx-nip);
no_test:
-if (ctx-singlestep_enabled)
+if (ctx-sys_sstep_enabled) {
+gen_update_nip(ctx, ctx-nip);
gen_op_debug();
+}
tcg_gen_exit_tb(0);
}
out:
@@ -6190,6 +6195,7 @@ static always_inline int gen_intermediat
branch_step = 1;
else
branch_step = 0;
+ctx.sys_sstep_enabled = env-singlestep_enabled;
ctx.singlestep_enabled = env-singlestep_enabled || single_step == 1;
#if defined (DO_SINGLE_STEP) 0
/* Single step trace mode */
@@ -6306,6 +6312,10 @@ static always_inline int gen_intermediat
if (ctx.exception == POWERPC_EXCP_NONE) {
gen_goto_tb(ctx, 0, ctx.nip);
} else if (ctx.exception != POWERPC_EXCP_BRANCH) {
+if (unlikely(ctx.sys_sstep_enabled)) {
+gen_update_nip(ctx, ctx.nip);
+gen_op_debug();
+}
/* Generate the return instruction */
tcg_gen_exit_tb(0);
}
--- a/vl.c
+++ b/vl.c
@@ -7523,7 +7523,7 @@ static int main_loop(void)
qemu_time += profile_getclock() - ti;
#endif
next_cpu = env-next_cpu ?: first_cpu;
-if (event_pending) {
+if (event_pending likely(ret != EXCP_DEBUG)) {
ret = EXCP_INTERRUPT;
event_pending = 0;
break;
@@ -7555,7 +7555,7 @@ static int main_loop(void)
qemu_system_powerdown();
ret = EXCP_INTERRUPT;
}
-if (ret == EXCP_DEBUG) {
+if (unlikely(ret == EXCP_DEBUG)) {
vm_stop(EXCP_DEBUG);
}
/* If all cpus are halted then wait until the next IRQ */
Re: [PATCH][Qemu-devel] Single stepping for PPC broken!
On Mon, 11 Feb 2008, Rob Landley wrote:
On Thursday 10 January 2008 07:57:50 Marius Groeger wrote:
The attached patch fixes the problem, but I have to admit I can't tell
for sure if this doesn't break other things (such as qemu's built-in
GDB server). Could some QEMU ppc expert please comment on this?
Looks fine to me, but I don't see it in the git mirror I follow...
Did anybody notice this patch?
Apparently not :-)
I just checked if it still applies, and it doesn't. Checking why I ran
into the following strangeness in target-ppc/translate.c:gen_goto_tb()
which appeared during the TCG migration:
..
if ((tb-pc TARGET_PAGE_MASK) == (dest TARGET_PAGE_MASK)
!ctx-singlestep_enabled) {
..
} else {
gen_set_T1(dest);
#if defined(TARGET_PPC64)
if (ctx-sf_mode)
gen_op_b_T1_64();
else
#endif
gen_op_b_T1();
if (ctx-singlestep_enabled)
gen_op_debug()
}
It seems to me that the second if (ctx-singlestep_enabled) is
rendundant.
I'll see if I can find some time to see if the patch is still needed
and if so, update it to the current HEAD.
Thanks
Marius
--
Marius Groeger
SYSGO AG Embedded and Real-Time Software
Voice: +49 6136 9948 0 FAX: +49 6136 9948 10
www.sysgo.com | www.elinos.com | www.osek.de | www.pikeos.com
Re: [PATCH][Qemu-devel] Single stepping for PPC broken!
On Wed, Feb 13, 2008 at 09:46:44AM +0100, Marius Groeger wrote:
if ((tb-pc TARGET_PAGE_MASK) == (dest TARGET_PAGE_MASK)
!ctx-singlestep_enabled) {
..
} else {
gen_set_T1(dest);
#if defined(TARGET_PPC64)
if (ctx-sf_mode)
gen_op_b_T1_64();
else
#endif
gen_op_b_T1();
if (ctx-singlestep_enabled)
gen_op_debug()
}
It seems to me that the second if (ctx-singlestep_enabled) is
rendundant.
No, if you've gone to a different page without single step then you
don't need the debug trap.
--
Daniel Jacobowitz
CodeSourcery
Re: [PATCH][Qemu-devel] Single stepping for PPC broken!
On Wed, Feb 13, 2008 at 04:52:22PM +0100, Marius Groeger wrote:
On Wed, 13 Feb 2008, Daniel Jacobowitz wrote:
On Wed, Feb 13, 2008 at 09:46:44AM +0100, Marius Groeger wrote:
if ((tb-pc TARGET_PAGE_MASK) == (dest TARGET_PAGE_MASK)
!ctx-singlestep_enabled) {
No, if you've gone to a different page without single step then you
don't need the debug trap.
Hm, so you mean betweeen the first if .. !ctx-singlestep_enabled
and the second one in the evaluation of ctx-singlestep_enabled
changes? What I meant is simply that the else clause already implies
that ctx-singlestep_enabled is true.
No it doesn't.
if (A !B) {
...
} else {
...
}
The else block will be entered if !A, or if A B.
--
Daniel Jacobowitz
CodeSourcery
Re: [PATCH][Qemu-devel] Single stepping for PPC broken!
On Wed, 13 Feb 2008, Daniel Jacobowitz wrote: The else block will be entered if !A, or if A B. Yeah - oops - sorry :-) Regards Marius -- Marius Groeger SYSGO AG Embedded and Real-Time Software Voice: +49 6136 9948 0 FAX: +49 6136 9948 10 www.sysgo.com | www.elinos.com | www.osek.de | www.pikeos.com
Re: [PATCH][Qemu-devel] Single stepping for PPC broken!
On Thursday 10 January 2008 07:57:50 Marius Groeger wrote: The attached patch fixes the problem, but I have to admit I can't tell for sure if this doesn't break other things (such as qemu's built-in GDB server). Could some QEMU ppc expert please comment on this? Looks fine to me, but I don't see it in the git mirror I follow... Did anybody notice this patch? Rob -- One of my most productive days was throwing away 1000 lines of code. - Ken Thompson.
Re: [PATCH][Qemu-devel] Single stepping for PPC broken!
On Wed, 9 Jan 2008, Marius Groeger wrote:
On Wed, 9 Jan 2008, Marius Groeger wrote:
I'm having problems with qemu's (-M prep, -cpu 604) handling of the
MSR_SE bit. My gdbstub can successfully step along regular code, but
qemu chokes when stepping over a branch instruction like blr.
(Needless to say, that same gdbstub works fine on real hardware). I
tried older versions of qemu and found that the code base 8 months ago
worked fine.
I have now verified with booting a Linux image into qemu-system-ppc - same
problem. When stepi'ing over the following sequence, the system chokes on a
bl instruction:
The attached patch fixes the problem, but I have to admit I can't tell
for sure if this doesn't break other things (such as qemu's built-in
GDB server). Could some QEMU ppc expert please comment on this?
Thanks
Marius
--
Marius Groeger [EMAIL PROTECTED]
SYSGO AG Embedded and Real-Time Software
Voice: +49 6136 9948 0 FAX: +49 6136 9948 10
www.sysgo.com | www.elinos.com | www.osek.de | www.pikeos.comIndex: target-ppc/translate.c
===
RCS file: /sources/qemu/qemu/target-ppc/translate.c,v
retrieving revision 1.115
diff -u -r1.115 translate.c
--- target-ppc/translate.c 24 Nov 2007 02:03:55 - 1.115
+++ target-ppc/translate.c 10 Jan 2008 13:54:36 -
@@ -2811,8 +2811,6 @@
#endif
gen_op_b_T1();
gen_op_set_T0((long)tb + n);
-if (ctx-singlestep_enabled)
-gen_op_debug();
gen_op_exit_tb();
} else {
gen_set_T1(dest);
@@ -2823,8 +2821,6 @@
#endif
gen_op_b_T1();
gen_op_reset_T0();
-if (ctx-singlestep_enabled)
-gen_op_debug();
gen_op_exit_tb();
}
}
@@ -3007,8 +3003,6 @@
gen_op_btest_T1(ctx-nip);
gen_op_reset_T0();
no_test:
-if (ctx-singlestep_enabled)
-gen_op_debug();
gen_op_exit_tb();
}
out:
