[Qemu-devel] [PATCH 1/5] single step with no IRQs and timers

[Qemu-devel] [PATCH 1/5] single step with no IRQs and timers

[Jason Wessel]

[-- Attachment #1: Type: text/plain, Size: 445 bytes --]


This patch adds the functionality to the gdb-stub to single step with 
the IRQs and timers disabled.  It greatly improves gdb's ability to 
perform run control while running a linux kernel and stepping off of 
breakpoints or stepping into certain types of functions.  I have also 
included individual controls for IRQs and timers to restore the original 
behavior, since it is useful as well.

signed-off-by: jason.wessel@windriver.com

Jason.

[-- Attachment #2: single_step_noirq_notimer.patch --]
[-- Type: text/plain, Size: 6280 bytes --]

Index: qemu/cpu-exec.c
===================================================================
--- qemu.orig/cpu-exec.c
+++ qemu/cpu-exec.c
@@ -452,7 +452,8 @@ int cpu_exec(CPUState *env1)
                 tmp_T0 = T0;
 #endif	    
                 interrupt_request = env->interrupt_request;
-                if (__builtin_expect(interrupt_request, 0)) {
+                if (__builtin_expect(interrupt_request, 0) &&
+                    !(env->singlestep_enabled & SSTEP_NOIRQ)) {
 #if defined(TARGET_I386)
                     /* if hardware interrupt pending, we execute it */
                     if ((interrupt_request & CPU_INTERRUPT_HARD) &&
Index: qemu/vl.c
===================================================================
--- qemu.orig/vl.c
+++ qemu/vl.c
@@ -4407,6 +4407,8 @@ void qemu_system_powerdown_request(void)
         cpu_interrupt(cpu_single_env, CPU_INTERRUPT_EXIT);
 }
 
+static CPUState *cur_cpu;
+
 void main_loop_wait(int timeout)
 {
     IOHandlerRecord *ioh, *ioh_next;
@@ -4500,19 +4502,19 @@ void main_loop_wait(int timeout)
 #endif
 
     if (vm_running) {
-        qemu_run_timers(&active_timers[QEMU_TIMER_VIRTUAL], 
-                        qemu_get_clock(vm_clock));
+        if (!(cur_cpu->singlestep_enabled & SSTEP_NOTIMER))
+            qemu_run_timers(&active_timers[QEMU_TIMER_VIRTUAL],
+                            qemu_get_clock(vm_clock));
         /* run dma transfers, if any */
         DMA_run();
     }
     
     /* real time timers */
-    qemu_run_timers(&active_timers[QEMU_TIMER_REALTIME], 
-                    qemu_get_clock(rt_clock));
+    if (!(cur_cpu->singlestep_enabled & SSTEP_NOTIMER))
+        qemu_run_timers(&active_timers[QEMU_TIMER_REALTIME],
+                        qemu_get_clock(rt_clock));
 }
 
-static CPUState *cur_cpu;
-
 int main_loop(void)
 {
     int ret, timeout;
Index: qemu/gdbstub.c
===================================================================
--- qemu.orig/gdbstub.c
+++ qemu/gdbstub.c
@@ -46,6 +46,11 @@ enum RSState {
 /* XXX: This is not thread safe.  Do we care?  */
 static int gdbserver_fd = -1;
 
+/* By default use no IRQs and no timers while single stepping so as to
+ * make single stepping like an ICE HW step.
+ */
+static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
+
 typedef struct GDBState {
     CPUState *env; /* current CPU */
     enum RSState state; /* parsing state */
@@ -596,7 +601,7 @@ static int gdb_handle_packet(GDBState *s
 	    env->pc = addr;
 #endif
         }
-        cpu_single_step(env, 1);
+        cpu_single_step(env, sstep_flags);
 #ifdef CONFIG_USER_ONLY
         s->running_state = 1;
 #else
@@ -672,8 +677,35 @@ static int gdb_handle_packet(GDBState *s
             goto breakpoint_error;
         }
         break;
+    case 'q':
+        /* parse any 'q' packets here */
+        if (!strcmp(p,"sstepbits")) {
+            /* Query Breakpoint bit definitions */
+            sprintf(buf,"ENABLE=%x,NOIRQ=%x,NOTIMER=%x",
+                    SSTEP_ENABLE,
+                    SSTEP_NOIRQ,
+                    SSTEP_NOTIMER);
+            put_packet(s, buf);
+            break;
+        } else if (strncmp(p,"sstep",5) == 0) {
+            /* Display or change the sstep_flags */
+            p += 5;
+            if (*p != '=') {
+                /* Display current setting */
+                sprintf(buf,"0x%x", sstep_flags);
+                put_packet(s, buf);
+                break;
+            }
+            p++;
+            type = strtoul(p, (char **)&p, 16);
+            sstep_flags = type;
+            put_packet(s, "OK");
+            break;
+        }
+        goto unknown_command;
+        break;
     default:
-        //        unknown_command:
+    unknown_command:
         /* put empty packet */
         buf[0] = '\0';
         put_packet(s, buf);
Index: qemu/cpu-all.h
===================================================================
--- qemu.orig/cpu-all.h
+++ qemu/cpu-all.h
@@ -768,6 +768,11 @@ void cpu_reset_interrupt(CPUState *env, 
 
 int cpu_breakpoint_insert(CPUState *env, target_ulong pc);
 int cpu_breakpoint_remove(CPUState *env, target_ulong pc);
+
+#define SSTEP_ENABLE  0x1  /* Enable simulated HW single stepping */
+#define SSTEP_NOIRQ   0x2  /* Do not use IRQ while single stepping */
+#define SSTEP_NOTIMER 0x4  /* Do not Timers while single stepping */
+
 void cpu_single_step(CPUState *env, int enabled);
 void cpu_reset(CPUState *s);
 
Index: qemu/qemu-doc.texi
===================================================================
--- qemu.orig/qemu-doc.texi
+++ qemu/qemu-doc.texi
@@ -1219,6 +1219,36 @@ Use @code{set architecture i8086} to dum
 @code{x/10i $cs*16+$eip} to dump the code at the PC position.
 @end enumerate
 
+Advanced debugging options:
+
+The default single stepping behavior is step with the IRQs and timer service routines off.  It is set this way because when gdb executes a single step it expects to advance beyond the current instruction.  With the IRQs and and timer service routines on, a single step might jump into the one of the interrupt or exception vectors instead of executing the current instruction. This means you may hit the same breakpoint a number of times before executing the instruction gdb wants to have executed.  Because there are rare circumstances where you want to single step into an interrupt vector the behavior can be controlled from GDB.  There are three commands you can query and set the single step behavior:
+@enumerate @code
+@item maintenance packet qsstepbits
+
+This will display the MASK bits used to control the single stepping IE:
+@example
+(gdb) maintenance packet qsstepbits
+sending: "qsstepbits"
+received: "ENABLE=1,NOIRQ=2,NOTIMER=4"
+@end example
+@item maintenance packet qsstep
+
+This will display the current value of the mask used when single stepping IE:
+@example
+(gdb) maintenance packet qsstep
+sending: "qsstep"
+received: "0x7"
+@end example
+@item maintenance packet qsstep=HEX_VALUE
+
+This will change the single step mask, so if wanted to enable IRQs on the single step, but not timers, you would use:
+@example
+(gdb) maintenance packet qsstep=0x5
+sending: "qsstep=0x5"
+received: "OK"
+@end example
+@end enumerate
+
 @node pcsys_os_specific
 @section Target OS specific information
 

Re: [Qemu-devel] [PATCH 1/5] single step with no IRQs and timers

[Mulyadi Santosa]

Hi Jason...

> This patch adds the functionality to the gdb-stub to single step with
> the IRQs and timers disabled.  It greatly improves gdb's ability to
> perform run control while running a linux kernel and stepping off of
> breakpoints or stepping into certain types of functions.  I have also
> included individual controls for IRQs and timers to restore the
> original behavior, since it is useful as well.

Thank you very much for posting this patch. I was getting trouble when 
doing single stepping when running linux guest (2.6.14) using qemu 
0.8.0, so maybe this is the right time to test your patch.

BTW, this patch is non arch specific patch, right? I mean, since I am 
using i386-softmmu, does your patch also work on x86 full system 
emulation too?

NB: Putting this patch on qemu forum (qemu.dad-answers.com) is nice 
while awaiting Fabrice's approval.

regards,

Mulyadi

RE: [Qemu-devel] [PATCH 1/5] single step with no IRQs and timers

[Wessel, Jason]

The patch is completely arch independent and should work with any of the
targets that use the gdb-stub.

Cheers,
Jason. 

> -----Original Message-----
> From: Mulyadi Santosa [mailto:a_mulyadi@softhome.net] 
> Sent: Sunday, May 21, 2006 1:54 AM
> To: qemu-devel@nongnu.org; Wessel, Jason
> Subject: Re: [Qemu-devel] [PATCH 1/5] single step with no 
> IRQs and timers
> 
> Hi Jason...
> 
> > This patch adds the functionality to the gdb-stub to single 
> step with 
> > the IRQs and timers disabled.  It greatly improves gdb's ability to 
> > perform run control while running a linux kernel and 
> stepping off of 
> > breakpoints or stepping into certain types of functions.  I 
> have also 
> > included individual controls for IRQs and timers to restore the 
> > original behavior, since it is useful as well.
> 
> Thank you very much for posting this patch. I was getting 
> trouble when doing single stepping when running linux guest 
> (2.6.14) using qemu 0.8.0, so maybe this is the right time to 
> test your patch.
> 
> BTW, this patch is non arch specific patch, right? I mean, 
> since I am using i386-softmmu, does your patch also work on 
> x86 full system emulation too?
> 
> NB: Putting this patch on qemu forum (qemu.dad-answers.com) 
> is nice while awaiting Fabrice's approval.
> 
> regards,
> 
> Mulyadi
> 
> 

Re: [Qemu-devel] [PATCH 1/5] single step with no IRQs and timers

[Daniel Jacobowitz]

On Sat, May 20, 2006 at 04:01:10PM -0500, Jason Wessel wrote:
> +    case 'q':
> +        /* parse any 'q' packets here */
> +        if (!strcmp(p,"sstepbits")) {

Hi Jason,

Not that I'm always good about this myself, but could I ask you to
follow this paragraph from the GDB manual, since these are commands
unlikely to be supported by a general GDB (at least not unless someone
proposes them...):

   * The names of custom vendor packets should use a company prefix, in
     lower case, followed by a period.  For example, packets designed at
     the Acme Corporation might begin with `qacme.foo' (for querying
     foos) or `Qacme.bar' (for setting bars).

Company name could be whatever here - either wrs or qemu, I suppose,
probably qemu.  By that logic it would probably be Qqemu.sstep=5, too,
though that's less important.  The goal is of course not to conflict
with later versions of GDB.

And thanks for doing this!  What a great idea!

-- 
Daniel Jacobowitz
CodeSourcery

Re: [Qemu-devel] [PATCH 1/5] single step with no IRQs and timers

[Jason Wessel]

[-- Attachment #1: Type: text/plain, Size: 1003 bytes --]

Daniel,

Here is the revised patch (against CVS HEAD 5/21/06).  The docs are 
updated as well.

signed-off-by: jason.wessel@windriver.com

Cheers,
Jason.

Daniel Jacobowitz wrote:
> Hi Jason,
>
> Not that I'm always good about this myself, but could I ask you to
> follow this paragraph from the GDB manual, since these are commands
> unlikely to be supported by a general GDB (at least not unless someone
> proposes them...):
>
>    * The names of custom vendor packets should use a company prefix, in
>      lower case, followed by a period.  For example, packets designed at
>      the Acme Corporation might begin with `qacme.foo' (for querying
>      foos) or `Qacme.bar' (for setting bars).
>
> Company name could be whatever here - either wrs or qemu, I suppose,
> probably qemu.  By that logic it would probably be Qqemu.sstep=5, too,
> though that's less important.  The goal is of course not to conflict
> with later versions of GDB.
>
> And thanks for doing this!  What a great idea!
>
>   


[-- Attachment #2: single_step_noirq_notimer.patch --]
[-- Type: text/plain, Size: 6351 bytes --]

Index: qemu/cpu-exec.c
===================================================================
--- qemu.orig/cpu-exec.c
+++ qemu/cpu-exec.c
@@ -452,7 +452,8 @@ int cpu_exec(CPUState *env1)
                 tmp_T0 = T0;
 #endif	    
                 interrupt_request = env->interrupt_request;
-                if (__builtin_expect(interrupt_request, 0)) {
+                if (__builtin_expect(interrupt_request, 0) &&
+                    !(env->singlestep_enabled & SSTEP_NOIRQ)) {
 #if defined(TARGET_I386)
                     /* if hardware interrupt pending, we execute it */
                     if ((interrupt_request & CPU_INTERRUPT_HARD) &&
Index: qemu/vl.c
===================================================================
--- qemu.orig/vl.c
+++ qemu/vl.c
@@ -4438,6 +4438,8 @@ void qemu_system_powerdown_request(void)
         cpu_interrupt(cpu_single_env, CPU_INTERRUPT_EXIT);
 }
 
+static CPUState *cur_cpu;
+
 void main_loop_wait(int timeout)
 {
     IOHandlerRecord *ioh, *ioh_next;
@@ -4531,19 +4533,19 @@ void main_loop_wait(int timeout)
 #endif
 
     if (vm_running) {
-        qemu_run_timers(&active_timers[QEMU_TIMER_VIRTUAL], 
-                        qemu_get_clock(vm_clock));
+        if (!(cur_cpu->singlestep_enabled & SSTEP_NOTIMER))
+            qemu_run_timers(&active_timers[QEMU_TIMER_VIRTUAL],
+                            qemu_get_clock(vm_clock));
         /* run dma transfers, if any */
         DMA_run();
     }
     
     /* real time timers */
-    qemu_run_timers(&active_timers[QEMU_TIMER_REALTIME], 
-                    qemu_get_clock(rt_clock));
+    if (!(cur_cpu->singlestep_enabled & SSTEP_NOTIMER))
+        qemu_run_timers(&active_timers[QEMU_TIMER_REALTIME],
+                        qemu_get_clock(rt_clock));
 }
 
-static CPUState *cur_cpu;
-
 int main_loop(void)
 {
     int ret, timeout;
Index: qemu/gdbstub.c
===================================================================
--- qemu.orig/gdbstub.c
+++ qemu/gdbstub.c
@@ -46,6 +46,11 @@ enum RSState {
 /* XXX: This is not thread safe.  Do we care?  */
 static int gdbserver_fd = -1;
 
+/* By default use no IRQs and no timers while single stepping so as to
+ * make single stepping like an ICE HW step.
+ */
+static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
+
 typedef struct GDBState {
     CPUState *env; /* current CPU */
     enum RSState state; /* parsing state */
@@ -596,7 +601,7 @@ static int gdb_handle_packet(GDBState *s
 	    env->pc = addr;
 #endif
         }
-        cpu_single_step(env, 1);
+        cpu_single_step(env, sstep_flags);
 #ifdef CONFIG_USER_ONLY
         s->running_state = 1;
 #else
@@ -672,8 +677,36 @@ static int gdb_handle_packet(GDBState *s
             goto breakpoint_error;
         }
         break;
+    case 'q':
+    case 'Q':
+        /* parse any 'q' packets here */
+        if (!strcmp(p,"qemu.sstepbits")) {
+            /* Query Breakpoint bit definitions */
+            sprintf(buf,"ENABLE=%x,NOIRQ=%x,NOTIMER=%x",
+                    SSTEP_ENABLE,
+                    SSTEP_NOIRQ,
+                    SSTEP_NOTIMER);
+            put_packet(s, buf);
+            break;
+        } else if (strncmp(p,"qemu.sstep",10) == 0) {
+            /* Display or change the sstep_flags */
+            p += 10;
+            if (*p != '=') {
+                /* Display current setting */
+                sprintf(buf,"0x%x", sstep_flags);
+                put_packet(s, buf);
+                break;
+            }
+            p++;
+            type = strtoul(p, (char **)&p, 16);
+            sstep_flags = type;
+            put_packet(s, "OK");
+            break;
+        }
+        goto unknown_command;
+        break;
     default:
-        //        unknown_command:
+    unknown_command:
         /* put empty packet */
         buf[0] = '\0';
         put_packet(s, buf);
Index: qemu/cpu-all.h
===================================================================
--- qemu.orig/cpu-all.h
+++ qemu/cpu-all.h
@@ -768,6 +768,11 @@ void cpu_reset_interrupt(CPUState *env, 
 
 int cpu_breakpoint_insert(CPUState *env, target_ulong pc);
 int cpu_breakpoint_remove(CPUState *env, target_ulong pc);
+
+#define SSTEP_ENABLE  0x1  /* Enable simulated HW single stepping */
+#define SSTEP_NOIRQ   0x2  /* Do not use IRQ while single stepping */
+#define SSTEP_NOTIMER 0x4  /* Do not Timers while single stepping */
+
 void cpu_single_step(CPUState *env, int enabled);
 void cpu_reset(CPUState *s);
 
Index: qemu/qemu-doc.texi
===================================================================
--- qemu.orig/qemu-doc.texi
+++ qemu/qemu-doc.texi
@@ -1219,6 +1219,36 @@ Use @code{set architecture i8086} to dum
 @code{x/10i $cs*16+$eip} to dump the code at the PC position.
 @end enumerate
 
+Advanced debugging options:
+
+The default single stepping behavior is step with the IRQs and timer service routines off.  It is set this way because when gdb executes a single step it expects to advance beyond the current instruction.  With the IRQs and and timer service routines on, a single step might jump into the one of the interrupt or exception vectors instead of executing the current instruction. This means you may hit the same breakpoint a number of times before executing the instruction gdb wants to have executed.  Because there are rare circumstances where you want to single step into an interrupt vector the behavior can be controlled from GDB.  There are three commands you can query and set the single step behavior:
+@enumerate @code
+@item maintenance packet qqemu.sstepbits
+
+This will display the MASK bits used to control the single stepping IE:
+@example
+(gdb) maintenance packet qqemu.sstepbits
+sending: "qqemu.sstepbits"
+received: "ENABLE=1,NOIRQ=2,NOTIMER=4"
+@end example
+@item maintenance packet qqemu.sstep
+
+This will display the current value of the mask used when single stepping IE:
+@example
+(gdb) maintenance packet qqemu.sstep
+sending: "qqemu.sstep"
+received: "0x7"
+@end example
+@item maintenance packet Qqemu.sstep=HEX_VALUE
+
+This will change the single step mask, so if wanted to enable IRQs on the single step, but not timers, you would use:
+@example
+(gdb) maintenance packet Qqemu.sstep=0x5
+sending: "qemu.sstep=0x5"
+received: "OK"
+@end example
+@end enumerate
+
 @node pcsys_os_specific
 @section Target OS specific information
 

Re: [Qemu-devel] [PATCH 1/5] single step with no IRQs and timers

[Fabrice Bellard]

Hi,

I agree that the feature is useful, but I am not sure yet if I will 
apply as is. I am adding support in QEMU for deterministic execution and 
your patch won't be needed once this new feature is released.

BTW, why do you stop real time timers ? These timers are only used for 
things which do not interfere with the VM (such as VGA refresh).

Fabrice.

Jason Wessel wrote:
> 
> This patch adds the functionality to the gdb-stub to single step with 
> the IRQs and timers disabled.  It greatly improves gdb's ability to 
> perform run control while running a linux kernel and stepping off of 
> breakpoints or stepping into certain types of functions.  I have also 
> included individual controls for IRQs and timers to restore the original 
> behavior, since it is useful as well.
> 
> signed-off-by: jason.wessel@windriver.com
> 
> Jason.

RE: [Qemu-devel] [PATCH 1/5] single step with no IRQs and timers

[Wessel, Jason]

Fabrice,

I will defer to you on the real time timers.  I had not examined the
code thoroughly enough to understand if there was more to the real time
timers.  I had wanted to guard against any kind of interrupts or
triggers that might fire off.  Just call it a lack of total
understanding of the complete QEMU infrastructure.  I would say the real
time timers can run if you feel the dependencies for precise single
instruction execution are not violated.

I found gdb against a linux kernel to be next to useless for any real
run control debugging with out the patch.  Deterministic execution
sounds great.  I would be happy to add debugger hook controls to further
control it at runtime with GDB, if your deterministic execution controls
are not already there at time.  I have found QEMU to be very useful for
the analysis of strange run control issues which is where the patch came
from in the first place.

Just let me know if you want me to re-gen the patch or if it will
ultimately go to the bit bucket in favor of something better.  It is a
great stopgap in the in mean time.

Thanks,
Jason.

> -----Original Message-----
> From: 
> qemu-devel-bounces+jason.wessel=windriver.com@nongnu.org 
> [mailto:qemu-devel-bounces+jason.wessel=windriver.com@nongnu.o
> rg] On Behalf Of Fabrice Bellard
> Sent: Monday, May 22, 2006 4:37 PM
> To: qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH 1/5] single step with no 
> IRQs and timers
> 
> Hi,
> 
> I agree that the feature is useful, but I am not sure yet if 
> I will apply as is. I am adding support in QEMU for 
> deterministic execution and your patch won't be needed once 
> this new feature is released.
> 
> BTW, why do you stop real time timers ? These timers are only 
> used for things which do not interfere with the VM (such as 
> VGA refresh).
> 
> Fabrice.