Add single stepping option for all targets

This patch replaces the compile time options SH4_SINGLE_STEP, DO_SINGLE_STEP and MIPS_SINGLE_STEP
by a command line option -singlestep.

It also adds single step mode for targets which did not have a compile time option,
so all system emulations can be used with -singlestep. Please note that I did only
run a short test for i386 and mips targets.

A new monitor command is provided to enable or disable single step mode. The monitor command "info status" was modified to display single step mode when activated.

Single stepping in Qemu's system emulation mode is useful to see the cpu state
for each cpu instruction when used with -d in_asm,cpu. It is also a simple way to
slow down the emulation.

The patch does not add single step mode for Qemu's user mode emulation. Would this be useful, too?

Kind regards
Stefan Weil

The patch does not add single step mode for Qemu's user mode emulation. Would this be useful, too?

This patch replaces the compile time options SH4_SINGLE_STEP, DO_SINGLE_STEP and MIPS_SINGLE_STEP
by a command line option -singlestep.

It also adds single step mode for targets which did not have a compile time option,
so all system emulations can be used with -singlestep. Please note that I did only run a short test for i386 and mips targets.

A new monitor command is provided to enable or disable single step mode. The monitor command "info status" was modified to display single step mode when activated.

Single stepping in Qemu's system emulation mode is useful to see the cpu state
for each cpu instruction when used with -d in_asm,cpu. It is also a simple way to slow down the emulation.

The patch does not add single step mode for Qemu's user mode emulation. Would this be useful, too?

Kind regards
Stefan Weil

Quoted 21 lines · Stefan Weil

This patch replaces the compile time options SH4_SINGLE_STEP, DO_SINGLE_STEP and MIPS_SINGLE_STEP
by a command line option -singlestep.

It also adds single step mode for targets which did not have a compile time option,
so all system emulations can be used with -singlestep. Please note that I did only run a short test for i386 and mips targets.

A new monitor command is provided to enable or disable single step mode. The monitor command "info status" was modified to display single step mode when activated.

Single stepping in Qemu's system emulation mode is useful to see the cpu state
for each cpu instruction when used with -d in_asm,cpu. It is also a simple way to slow down the emulation.

The patch does not add single step mode for Qemu's user mode emulation. Would this be useful, too?

Quoted 4 lines

Kind regards
Stefan Weil

Hello,

here is an update of the patch. Please apply it to Qemu trunk.

I have two comments:
- as Stuart previously wrote, user mode would be useful to have

- shouldn't you also prevent block chaining from being done for this feature to be really useful?

Laurent

Quoted 18 lines

This patch replaces the compile time options SH4_SINGLE_STEP, DO_SINGLE_STEP and MIPS_SINGLE_STEP
by a command line option -singlestep.

It also adds single step mode for targets which did not have a compile time option,
so all system emulations can be used with -singlestep. Please note that I did only run a short test for i386 and mips targets.

A new monitor command is provided to enable or disable single step mode. The monitor command "info status" was modified to display single step mode when activated.

Single stepping in Qemu's system emulation mode is useful to see the cpu state
for each cpu instruction when used with -d in_asm,cpu. It is also a simple way to slow down the emulation.

Quoted 8 lines

The patch does not add single step mode for Qemu's user mode emulation. Would this be useful, too?

Kind regards
Stefan Weil

Quoted 18 lines · Stefan Weil

Quoted 18 lines

This patch replaces the compile time options SH4_SINGLE_STEP, DO_SINGLE_STEP and MIPS_SINGLE_STEP
by a command line option -singlestep.

It also adds single step mode for targets which did not have a compile time option,
so all system emulations can be used with -singlestep. Please note that I did only run a short test for i386 and mips targets.

A new monitor command is provided to enable or disable single step mode. The monitor command "info status" was modified to display single step mode when activated.

Single stepping in Qemu's system emulation mode is useful to see the cpu state
for each cpu instruction when used with -d in_asm,cpu. It is also a simple way to slow down the emulation.

Quoted 8 lines

Quoted 8 lines

The patch does not add single step mode for Qemu's user mode emulation. Would this be useful, too?

Kind regards
Stefan Weil

The patch is still missing in Qemu trunk. There was some feedback, but not from a maintainer, so I don't know why the patch is not integrated. Do you need an update of the patch? Do you need more information? Do you want something changed or added?

Quoted 18 lines · Stefan Weil

This patch replaces the compile time options SH4_SINGLE_STEP, DO_SINGLE_STEP and MIPS_SINGLE_STEP
by a command line option -singlestep.

It also adds single step mode for targets which did not have a compile time option,
so all system emulations can be used with -singlestep. Please note that I did only run a short test for i386 and mips targets.

A new monitor command is provided to enable or disable single step mode. The monitor command "info status" was modified to display single step mode when activated.

Single stepping in Qemu's system emulation mode is useful to see the cpu state
for each cpu instruction when used with -d in_asm,cpu. It is also a simple way to slow down the emulation.

Quoted 7 lines

The patch does not add single step mode for Qemu's user mode emulation. Would this be useful, too?

Kind regards
Stefan Weil

Hello,

here is an update of the patch. Please apply it to Qemu trunk.

Regards
Stefan Weil

Add new command line option for tcg single stepping. This replaces a compile time option for some targets and adds this feature to targets which did not have a compile time option.

Add monitor command to enable or disable single step mode.

Modify monitor command "info status" to display single step mode.

Signed-off-by Stefan Weil <[email protected]>

Index: trunk/target-sh4/translate.c
===================================================================

target-sh4/translate.c +2 -3

@@ -1896,9 +1896,8 @@
 	    break;
         if (num_insns >= max_insns)
             break;
-#ifdef SH4_SINGLE_STEP
-	break;
-#endif
+        if (vm_singlestep)
+            break;
     }
     if (tb->cflags & CF_LAST_IO)
         gen_io_end();

Index: trunk/target-cris/translate.c
===================================================================

target-cris/translate.c +1 -0

@@ -3271,6 +3271,7 @@
 			break;
 	} while (!dc->is_jmp && !dc->cpustate_changed
 		 && gen_opc_ptr < gen_opc_end
+		 && !vm_singlestep
 		 && (dc->pc < next_page_start)
                  && num_insns < max_insns);
 

Index: trunk/target-alpha/translate.c
===================================================================

target-alpha/translate.c +3 -4

@@ -2413,11 +2413,10 @@
         if (env->singlestep_enabled) {
             gen_excp(&ctx, EXCP_DEBUG, 0);
             break;
-	}
+        }
 
-#if defined (DO_SINGLE_STEP)
-        break;
-#endif
+        if (vm_singlestep)
+            break;
     }
     if (ret != 1 && ret != 3) {
         tcg_gen_movi_i64(cpu_pc, ctx.pc);

Index: trunk/vl.c
===================================================================

vl.c +3 -0

@@ -193,6 +193,7 @@
 int nb_nics;
 NICInfo nd_table[MAX_NICS];
 int vm_running;
+int vm_singlestep;
 static int rtc_utc = 1;
 static int rtc_date_offset = -1; /* -1 means no change */
 int cirrus_vga_enabled = 1;

@@ -3984,6 +3985,7 @@
            "-parallel dev   redirect the parallel port to char device 'dev'\n"
            "-monitor dev    redirect the monitor to char device 'dev'\n"
            "-pidfile file   write PID to 'file'\n"
+           "-singlestep     always run in singlestep mode\n"
            "-S              freeze CPU at startup (use 'c' to start execution)\n"
            "-s              wait gdb connection to port\n"
            "-p port         set gdb connection port [default=%s]\n"

@@ -4119,6 +4121,7 @@
     QEMU_OPTION_parallel,
     QEMU_OPTION_monitor,
     QEMU_OPTION_pidfile,
+    QEMU_OPTION_singlestep,
     QEMU_OPTION_S,
     QEMU_OPTION_s,
     QEMU_OPTION_p,

Index: trunk/target-ppc/translate.c
===================================================================

target-ppc/translate.c +3 -4

@@ -38,7 +38,6 @@
 #define GDBSTUB_SINGLE_STEP 0x4
 
 /* Include definitions for instructions classes and implementations flags */
-//#define DO_SINGLE_STEP
 //#define PPC_DEBUG_DISAS
 //#define DO_PPC_STATISTICS
 

@@ -8305,9 +8304,9 @@
              */
             break;
         }
-#if defined (DO_SINGLE_STEP)
-        break;
-#endif
+
+        if (vm_singlestep)
+            break;
     }
     if (tb->cflags & CF_LAST_IO)
         gen_io_end();

Index: trunk/target-mips/translate.c
===================================================================

target-mips/translate.c +3 -4

@@ -38,7 +38,6 @@
 
 //#define MIPS_DEBUG_DISAS
 //#define MIPS_DEBUG_SIGN_EXTENSIONS
-//#define MIPS_SINGLE_STEP
 
 /* MIPS major opcodes */
 #define MASK_OP_MAJOR(op)  (op & (0x3F << 26))

@@ -8247,9 +8246,9 @@
 
         if (num_insns >= max_insns)
             break;
-#if defined (MIPS_SINGLE_STEP)
-        break;
-#endif
+
+        if (vm_singlestep)
+            break;
     }
     if (tb->cflags & CF_LAST_IO)
         gen_io_end();

Index: trunk/monitor.c
===================================================================

monitor.c +21 -3

@@ -489,6 +489,18 @@
     cpu_set_log(mask);
 }
 
+static void do_singlestep(const char *option)
+{
+    qemu_printf("setting vm_singlestep to %s\n", option);
+    if (!option) {
+        vm_singlestep = 1;
+    } else if (!strcmp(option, "off")) {
+        vm_singlestep = 0;
+    } else {
+        term_printf("unexpected option %s\n", option);
+    }
+}
+
 static void do_stop(void)
 {
     vm_stop(EXCP_INTERRUPT);

@@ -1403,9 +1415,13 @@
 
 static void do_info_status(void)
 {
-    if (vm_running)
-       term_printf("VM status: running\n");
-    else
+    if (vm_running) {
+        if (vm_singlestep) {
+            term_printf("VM status: running (single step mode)\n");
+        } else {
+            term_printf("VM status: running\n");
+        }
+    } else
        term_printf("VM status: paused\n");
 }
 

@@ -1455,6 +1471,8 @@
       "tag|id", "restore a VM snapshot from its tag or id" },
     { "delvm", "s", do_delvm,
       "tag|id", "delete a VM snapshot from its tag or id" },
+    { "singlestep", "s?", do_singlestep,
+      "[off]", "run emulation in singlestep mode or switch to normal mode", },
     { "stop", "", do_stop,
       "", "stop emulation", },
     { "c|cont", "", do_cont,

Index: trunk/target-i386/translate.c
===================================================================

target-i386/translate.c +5 -0

@@ -7661,6 +7661,11 @@
             gen_eob(dc);
             break;
         }
+        if (vm_singlestep) {
+            gen_jmp_im(pc_ptr - dc->cs_base);
+            gen_eob(dc);
+            break;
+        }
     }
     if (tb->cflags & CF_LAST_IO)
         gen_io_end();

Index: trunk/target-arm/translate.c
===================================================================

target-arm/translate.c +1 -1

@@ -8788,7 +8788,7 @@
          * ensures prefetch aborts occur at the right place.  */
         num_insns ++;
     } while (!dc->is_jmp && gen_opc_ptr < gen_opc_end &&
-             !env->singlestep_enabled &&
+             !env->singlestep_enabled && !vm_singlestep &&
              dc->pc < next_page_start &&
              num_insns < max_insns);
 

Index: trunk/target-m68k/translate.c
===================================================================

target-m68k/translate.c +1 -1

@@ -3028,7 +3028,7 @@
 	disas_m68k_insn(env, dc);
         num_insns++;
     } while (!dc->is_jmp && gen_opc_ptr < gen_opc_end &&
-             !env->singlestep_enabled &&
+             !env->singlestep_enabled && !vm_singlestep &&
              (pc_offset) < (TARGET_PAGE_SIZE - 32) &&
              num_insns < max_insns);
 

Index: trunk/target-sparc/translate.c
===================================================================

target-sparc/translate.c +1 -1

@@ -4858,7 +4858,7 @@
             break;
         /* if single step mode, we generate only one instruction and
            generate an exception */
-        if (env->singlestep_enabled) {
+        if (env->singlestep_enabled || vm_singlestep) {
             tcg_gen_movi_tl(cpu_pc, dc->pc);
             tcg_gen_exit_tb(0);
             break;

Index: trunk/qemu-doc.texi
===================================================================

qemu-doc.texi +7 -0

@@ -1064,6 +1064,9 @@
 @item -s
 Wait gdb connection to port 1234 (@pxref{gdb_usage}).
 
+@item -singlestep
+Run the emulation in single step mode.
+
 @item -p @var{port}
 Change gdb connection port.  @var{port} can be either a decimal number
 to specify a TCP port, or a host device (same devices as the serial port).

@@ -1383,6 +1386,10 @@
 @item delvm @var{tag}|@var{id}
 Delete the snapshot identified by @var{tag} or @var{id}.
 
+@item singlestep [off]
+Run the emulation in single step mode.
+If called with option off, the emulation returns to normal mode.
+
 @item stop
 Stop emulation.
 

Index: trunk/exec-all.h
===================================================================

exec-all.h +0 -0

@@ -386,6 +386,12 @@
 
 #endif

Quoted 14 lines · Stefan Weil

Quoted 13 lines · Stefan Weil

This patch replaces the compile time options SH4_SINGLE_STEP, DO_SINGLE_STEP and MIPS_SINGLE_STEP
by a command line option -singlestep.

It also adds single step mode for targets which did not have a compile time option,
so all system emulations can be used with -singlestep. Please note that I did only run a short test for i386 and mips targets.

A new monitor command is provided to enable or disable single step mode. The monitor command "info status" was modified to display single step mode when activated.

Quoted 6 lines

Quoted 6 lines

Single stepping in Qemu's system emulation mode is useful to see the cpu state
for each cpu instruction when used with -d in_asm,cpu. It is also a simple way to slow down the emulation.

Please find my comments below.

Quoted 33 lines

Signed-off-by Stefan Weil <[email protected]>

...
Index: trunk/vl.c
===================================================================

vl.c 2009-02-01 19:18:33.000000000 +0100 renamed from vl.c 2009-02-01 19:12:41.000000000 +0100 +1 -0

@@ -193,6 +193,7 @@
 int nb_nics;
 NICInfo nd_table[MAX_NICS];
 int vm_running;
+int vm_singlestep;
 static int rtc_utc = 1;

static int rtc_date_offset = -1; /* -1 means no change */ int cirrus_vga_enabled = 1;

vl.c 2009-02-01 19:18:33.000000000 +0100 renamed from vl.c 2009-02-01 19:12:41.000000000 +0100 +0 -0

@@ -3984,6 +3985,7 @@

"-parallel dev redirect the parallel port to char device 'dev'\n" "-monitor dev redirect the monitor to char device 'dev'\n" "-pidfile file write PID to 'file'\n"
+ "-singlestep always run in singlestep mode\n"
"-S freeze CPU at startup (use 'c' to start execution)\n" "-s wait gdb connection to port\n"
"-p port set gdb connection port [default=%s]\n"

vl.c 2009-02-01 19:18:33.000000000 +0100 renamed from vl.c 2009-02-01 19:12:41.000000000 +0100 +1 -0

@@ -4119,6 +4121,7 @@
 QEMU_OPTION_parallel,
 QEMU_OPTION_monitor,
 QEMU_OPTION_pidfile,
+ QEMU_OPTION_singlestep,
 QEMU_OPTION_S,
 QEMU_OPTION_s,
 QEMU_OPTION_p,

@@ -4238,6 +4241,7 @@
 { "parallel", HAS_ARG, QEMU_OPTION_parallel },
 { "monitor", HAS_ARG, QEMU_OPTION_monitor },

vl.c 2009-02-01 19:18:33.000000000 +0100 line 4247

Quoted 5 lines

vl.c 2009-02-01 19:18:33.000000000 +0100 renamed from vl.c 2009-02-01 19:12:41.000000000 +0100 +1 -0

 { "pidfile", HAS_ARG, QEMU_OPTION_pidfile },
+ { "singlestep", 0, QEMU_OPTION_singlestep },
 { "S", 0, QEMU_OPTION_S },
 { "s", 0, QEMU_OPTION_s },
 { "p", HAS_ARG, QEMU_OPTION_p },

It might be a good idea to provide that as a subset of the -d option, as it is clearly something only useful for debugging.

Quoted 3 lines

It might be a good idea to provide that as a subset of the -d option, as it is clearly something only useful for debugging.

Today -d does not take suboptions.
Could you please explain your proposal with more details?

Quoted 5 lines · Stefan Weil

Quoted 2 lines

It might be a good idea to provide that as a subset of the -d option, as it is clearly something only useful for debugging.

Today -d does not take suboptions.
Could you please explain your proposal with more details?

It kind of does by using cpu_str_to_log_mask. On the other hand I'm not sure your option belongs to -d as all of -d suboptions are there to enable some kind of output log.

This is an update of my patch which adds a new command line option -singlestep, now for both system and user mode.

So it now respects the feedback from Stuart, Laurent and Aurelien who all wanted user mode support, too.

There remains one open feedback point which I did not understand - see below.

I hope the patch will be added to trunk finally...

Regards
Stefan Weil

This is an update of my patch which adds a new command line option -singlestep, now for both system and user mode.

So it now respects the feedback from Stuart, Laurent and Aurelien who all wanted user mode support, too.

There remains one open feedback point which I did not understand - see below.

I hope the patch will be added to trunk finally...

Regards
Stefan Weil

Quoted 57 lines · Aurelien Jarno

Quoted 14 lines · Stefan Weil

Quoted 13 lines · Stefan Weil

This patch replaces the compile time options SH4_SINGLE_STEP, DO_SINGLE_STEP and MIPS_SINGLE_STEP
by a command line option -singlestep.

It also adds single step mode for targets which did not have a compile time option,
so all system emulations can be used with -singlestep. Please note that I did only run a short test for i386 and mips targets.

A new monitor command is provided to enable or disable single step mode. The monitor command "info status" was modified to display single step mode when activated.

Quoted 6 lines

Quoted 6 lines

Single stepping in Qemu's system emulation mode is useful to see the cpu state
for each cpu instruction when used with -d in_asm,cpu. It is also a simple way to slow down the emulation.

Please find my comments below.

Quoted 33 lines

Signed-off-by Stefan Weil <[email protected]>

...
Index: trunk/vl.c
===================================================================

vl.c 2009-02-01 19:18:33.000000000 +0100 renamed from vl.c 2009-02-01 19:12:41.000000000 +0100 +1 -0

@@ -193,6 +193,7 @@
 int nb_nics;
 NICInfo nd_table[MAX_NICS];
 int vm_running;
+int vm_singlestep;
 static int rtc_utc = 1;

static int rtc_date_offset = -1; /* -1 means no change */ int cirrus_vga_enabled = 1;

vl.c 2009-02-01 19:18:33.000000000 +0100 renamed from vl.c 2009-02-01 19:12:41.000000000 +0100 +0 -0

@@ -3984,6 +3985,7 @@

"-parallel dev redirect the parallel port to char device 'dev'\n" "-monitor dev redirect the monitor to char device 'dev'\n" "-pidfile file write PID to 'file'\n"
+ "-singlestep always run in singlestep mode\n"
"-S freeze CPU at startup (use 'c' to start execution)\n" "-s wait gdb connection to port\n"
"-p port set gdb connection port [default=%s]\n"

vl.c 2009-02-01 19:18:33.000000000 +0100 renamed from vl.c 2009-02-01 19:12:41.000000000 +0100 +1 -0

@@ -4119,6 +4121,7 @@
 QEMU_OPTION_parallel,
 QEMU_OPTION_monitor,
 QEMU_OPTION_pidfile,
+ QEMU_OPTION_singlestep,
 QEMU_OPTION_S,
 QEMU_OPTION_s,
 QEMU_OPTION_p,

@@ -4238,6 +4241,7 @@
 { "parallel", HAS_ARG, QEMU_OPTION_parallel },
 { "monitor", HAS_ARG, QEMU_OPTION_monitor },

Quoted 8 lines

vl.c 2009-02-01 19:18:33.000000000 +0100 line 4247

Quoted 5 lines

vl.c 2009-02-01 19:18:33.000000000 +0100 renamed from vl.c 2009-02-01 19:12:41.000000000 +0100 +1 -0

 { "pidfile", HAS_ARG, QEMU_OPTION_pidfile },
+ { "singlestep", 0, QEMU_OPTION_singlestep },
 { "S", 0, QEMU_OPTION_S },
 { "s", 0, QEMU_OPTION_s },
 { "p", HAS_ARG, QEMU_OPTION_p },

It might be a good idea to provide that as a subset of the -d option, as it is clearly something only useful for debugging.

Today -d does not take suboptions.
Could you please explain your proposal with more details?

Stefan

Add new command line option -singlestep for tcg single stepping.

This replaces a compile time option for some targets and adds this feature to targets which did not have a compile time option.

Add monitor command to enable or disable single step mode.

Modify monitor command "info status" to display single step mode.

Signed-off-by Stefan Weil <[email protected]>

Index: trunk/target-sh4/translate.c
===================================================================

target-sh4/translate.c +2 -3

@@ -1929,9 +1929,8 @@
 	    break;
         if (num_insns >= max_insns)
             break;
-#ifdef SH4_SINGLE_STEP
-	break;
-#endif
+        if (vm_singlestep)
+            break;
     }
     if (tb->cflags & CF_LAST_IO)
         gen_io_end();

Index: trunk/target-cris/translate.c
===================================================================

target-cris/translate.c +1 -0

@@ -3272,6 +3272,7 @@
 			break;
 	} while (!dc->is_jmp && !dc->cpustate_changed
 		 && gen_opc_ptr < gen_opc_end
+		 && !vm_singlestep
 		 && (dc->pc < next_page_start)
                  && num_insns < max_insns);
 

Index: trunk/target-alpha/translate.c
===================================================================

target-alpha/translate.c +3 -4

@@ -2413,11 +2413,10 @@
         if (env->singlestep_enabled) {
             gen_excp(&ctx, EXCP_DEBUG, 0);
             break;
-	}
+        }
 
-#if defined (DO_SINGLE_STEP)
-        break;
-#endif
+        if (vm_singlestep)
+            break;
     }
     if (ret != 1 && ret != 3) {
         tcg_gen_movi_i64(cpu_pc, ctx.pc);

Index: trunk/vl.c
===================================================================

vl.c +2 -0

 static int autostart;
 static int rtc_utc = 1;
 static int rtc_date_offset = -1; /* -1 means no change */

@@ -4081,6 +4082,7 @@
            "-parallel dev   redirect the parallel port to char device 'dev'\n"
            "-monitor dev    redirect the monitor to char device 'dev'\n"
            "-pidfile file   write PID to 'file'\n"
+           "-singlestep     always run in singlestep mode\n"
            "-S              freeze CPU at startup (use 'c' to start execution)\n"
            "-s              wait gdb connection to port\n"
            "-p port         set gdb connection port [default=%s]\n"

@@ -4221,6 +4223,7 @@
     QEMU_OPTION_parallel,
     QEMU_OPTION_monitor,
     QEMU_OPTION_pidfile,
+    QEMU_OPTION_singlestep,
     QEMU_OPTION_S,
     QEMU_OPTION_s,
     QEMU_OPTION_p,

Index: trunk/target-ppc/translate.c
===================================================================

target-ppc/translate.c +3 -4

@@ -39,7 +39,6 @@
 #define GDBSTUB_SINGLE_STEP 0x4
 
 /* Include definitions for instructions classes and implementations flags */
-//#define DO_SINGLE_STEP
 //#define PPC_DEBUG_DISAS
 //#define DO_PPC_STATISTICS
 

@@ -8294,9 +8293,9 @@
              */
             break;
         }
-#if defined (DO_SINGLE_STEP)
-        break;
-#endif
+
+        if (vm_singlestep)
+            break;
     }
     if (tb->cflags & CF_LAST_IO)
         gen_io_end();

Index: trunk/target-mips/translate.c
===================================================================

target-mips/translate.c +3 -4

@@ -38,7 +38,6 @@
 
 //#define MIPS_DEBUG_DISAS
 //#define MIPS_DEBUG_SIGN_EXTENSIONS
-//#define MIPS_SINGLE_STEP
 
 /* MIPS major opcodes */
 #define MASK_OP_MAJOR(op)  (op & (0x3F << 26))

@@ -8245,9 +8244,9 @@
 
         if (num_insns >= max_insns)
             break;
-#if defined (MIPS_SINGLE_STEP)
-        break;
-#endif
+
+        if (vm_singlestep)
+            break;
     }
     if (tb->cflags & CF_LAST_IO)
         gen_io_end();

Index: trunk/monitor.c
===================================================================

monitor.c +21 -3

@@ -527,6 +527,18 @@
     cpu_set_log(mask);
 }
 
+static void do_singlestep(Monitor *mon, const char *option)
+{
+    qemu_printf("setting vm_singlestep to %s\n", option);
+    if (!option) {
+        vm_singlestep = 1;
+    } else if (!strcmp(option, "off")) {
+        vm_singlestep = 0;
+    } else {
+        monitor_printf(mon, "unexpected option %s\n", option);
+    }
+}
+
 static void do_stop(Monitor *mon)
 {
     vm_stop(EXCP_INTERRUPT);

@@ -1508,9 +1520,13 @@
 
 static void do_info_status(Monitor *mon)
 {
-    if (vm_running)
-       monitor_printf(mon, "VM status: running\n");
-    else
+    if (vm_running) {
+        if (vm_singlestep) {
+            monitor_printf(mon, "VM status: running (single step mode)\n");
+        } else {
+            monitor_printf(mon, "VM status: running\n");
+        }
+    } else
        monitor_printf(mon, "VM status: paused\n");
 }
 

@@ -1641,6 +1657,8 @@
       "tag|id", "restore a VM snapshot from its tag or id" },
     { "delvm", "s", do_delvm,
       "tag|id", "delete a VM snapshot from its tag or id" },
+    { "singlestep", "s?", do_singlestep,
+      "[off]", "run emulation in singlestep mode or switch to normal mode", },
     { "stop", "", do_stop,
       "", "stop emulation", },
     { "c|cont", "", do_cont,

Index: trunk/target-i386/translate.c
===================================================================

target-i386/translate.c +5 -0

@@ -7651,6 +7651,11 @@
             gen_eob(dc);
             break;
         }
+        if (vm_singlestep) {
+            gen_jmp_im(pc_ptr - dc->cs_base);
+            gen_eob(dc);
+            break;
+        }
     }
     if (tb->cflags & CF_LAST_IO)
         gen_io_end();

Index: trunk/target-arm/translate.c
===================================================================

target-arm/translate.c +1 -1

@@ -8788,7 +8788,7 @@
          * ensures prefetch aborts occur at the right place.  */
         num_insns ++;
     } while (!dc->is_jmp && gen_opc_ptr < gen_opc_end &&
-             !env->singlestep_enabled &&
+             !env->singlestep_enabled && !vm_singlestep &&
              dc->pc < next_page_start &&
              num_insns < max_insns);
 

Index: trunk/target-m68k/translate.c
===================================================================

target-m68k/translate.c +1 -1

@@ -3030,7 +3030,7 @@
 	disas_m68k_insn(env, dc);
         num_insns++;
     } while (!dc->is_jmp && gen_opc_ptr < gen_opc_end &&
-             !env->singlestep_enabled &&
+             !env->singlestep_enabled && !vm_singlestep &&
              (pc_offset) < (TARGET_PAGE_SIZE - 32) &&
              num_insns < max_insns);
 

Index: trunk/target-sparc/translate.c
===================================================================

target-sparc/translate.c +1 -1

@@ -4858,7 +4858,7 @@
             break;
         /* if single step mode, we generate only one instruction and
            generate an exception */
-        if (env->singlestep_enabled) {
+        if (env->singlestep_enabled || vm_singlestep) {
             tcg_gen_movi_tl(cpu_pc, dc->pc);
             tcg_gen_exit_tb(0);
             break;

Index: trunk/qemu-doc.texi
===================================================================

qemu-doc.texi +7 -0

@@ -1100,6 +1100,9 @@
 @item -s
 Wait gdb connection to port 1234 (@pxref{gdb_usage}).
 
+@item -singlestep
+Run the emulation in single step mode.
+
 @item -p @var{port}
 Change gdb connection port.  @var{port} can be either a decimal number
 to specify a TCP port, or a host device (same devices as the serial port).

@@ -1463,6 +1466,10 @@
 @item delvm @var{tag}|@var{id}
 Delete the snapshot identified by @var{tag} or @var{id}.
 
+@item singlestep [off]
+Run the emulation in single step mode.
+If called with option off, the emulation returns to normal mode.
+
 @item stop
 Stop emulation.
 

Index: trunk/exec-all.h
===================================================================

exec-all.h +2 -0

@@ -381,6 +381,8 @@
 
 #endif
 
+extern int vm_singlestep;
+
 typedef void (CPUDebugExcpHandler)(CPUState *env);
 
 CPUDebugExcpHandler *cpu_set_debug_excp_handler(CPUDebugExcpHandler *handler);

Index: trunk/bsd-user/main.c
===================================================================

bsd-user/main.c +5 -0

@@ -33,6 +33,8 @@
 
 #define DEBUG_LOGFILE "/tmp/qemu.log"
 
+int vm_singlestep;
+
 static const char *interp_prefix = CONFIG_QEMU_PREFIX;
 const char *qemu_uname_release = CONFIG_UNAME_RELEASE;
 extern char **environ;

@@ -378,6 +380,7 @@
            "Debug options:\n"
            "-d options   activate log (logfile=%s)\n"
            "-p pagesize  set the host page size to 'pagesize'\n"
+           "-singlestep  always run in singlestep mode\n"
            "-strace      log system calls\n"
            "\n"
            "Environment variables:\n"

@@ -500,6 +503,8 @@
                 usage();
             }
             optind++;
+        } else if (!strcmp(r, "singlestep")) {
+            vm_singlestep = 1;
         } else if (!strcmp(r, "strace")) {
             do_strace = 1;
         } else

Index: trunk/darwin-user/main.c
===================================================================

darwin-user/main.c +5 -0

@@ -41,6 +41,8 @@
 #include <mach/mach_init.h>
 #include <mach/vm_map.h>
 
+int vm_singlestep;
+
 const char *interp_prefix = "";
 
 asm(".zerofill __STD_PROG_ZONE, __STD_PROG_ZONE, __std_prog_zone, 0x0dfff000");

@@ -751,6 +753,7 @@
            "-d options   activate log (logfile='%s')\n"
            "-g wait for gdb on port 1234\n"
            "-p pagesize  set the host page size to 'pagesize'\n",
+           "-singlestep  always run in singlestep mode\n"
            TARGET_ARCH,
            TARGET_ARCH,
            interp_prefix,

@@ -842,6 +845,8 @@
 #endif
                 exit(1);
             }
+        } else if (!strcmp(r, "singlestep")) {
+            vm_singlestep = 1;
         } else
         {
             usage();

Index: trunk/linux-user/main.c
===================================================================

linux-user/main.c +5 -0

@@ -39,6 +39,8 @@
 
 char *exec_path;
 
+int vm_singlestep;
+
 static const char *interp_prefix = CONFIG_QEMU_PREFIX;
 const char *qemu_uname_release = CONFIG_UNAME_RELEASE;
 

@@ -2217,6 +2219,7 @@
            "Debug options:\n"
            "-d options   activate log (logfile=%s)\n"
            "-p pagesize  set the host page size to 'pagesize'\n"
+           "-singlestep  always run in singlestep mode\n"
            "-strace      log system calls\n"
            "\n"
            "Environment variables:\n"

@@ -2359,6 +2362,8 @@
             }
         } else if (!strcmp(r, "drop-ld-preload")) {
             (void) envlist_unsetenv(envlist, "LD_PRELOAD");
+        } else if (!strcmp(r, "singlestep")) {
+            vm_singlestep = 1;
         } else if (!strcmp(r, "strace")) {
             do_strace = 1;
         } else

Given those explanations and the comments from other people, I am fine with this option. I still have some comments though (see below).

vl.c 2009-03-13 17:08:01.000000000 +0100 line 214

Quoted 12 lines

...
Index: trunk/vl.c
===================================================================

vl.c 2009-03-13 17:08:01.000000000 +0100 renamed from vl.c 2009-03-13 17:07:47.000000000 +0100 +1 -0

@@ -211,6 +211,7 @@
 int nb_nics;
 NICInfo nd_table[MAX_NICS];
 int vm_running;
+int vm_singlestep;

You create a new variable. By the way, I think that calling it singlestep is better, and matches the naming of other options variable (like daemonize, graphic_rotate). You should define it to a default value of 0.

Quoted 10 lines

vl.c 2009-03-13 17:08:01.000000000 +0100 renamed from vl.c 2009-03-13 17:07:47.000000000 +0100 +1 -0

@@ -4221,6 +4223,7 @@
 QEMU_OPTION_parallel,
 QEMU_OPTION_monitor,
 QEMU_OPTION_pidfile,
+ QEMU_OPTION_singlestep,
 QEMU_OPTION_S,
 QEMU_OPTION_s,
 QEMU_OPTION_p,

@@ -4345,6 +4348,7 @@
 { "parallel", HAS_ARG, QEMU_OPTION_parallel },

vl.c 2009-03-13 17:08:01.000000000 +0100 line 4354

Quoted 6 lines

vl.c 2009-03-13 17:08:01.000000000 +0100 renamed from vl.c 2009-03-13 17:07:47.000000000 +0100 +1 -0

 { "monitor", HAS_ARG, QEMU_OPTION_monitor },
 { "pidfile", HAS_ARG, QEMU_OPTION_pidfile },
+ { "singlestep", 0, QEMU_OPTION_singlestep },
 { "S", 0, QEMU_OPTION_S },
 { "s", 0, QEMU_OPTION_s },
 { "p", HAS_ARG, QEMU_OPTION_p },

This option is never parsed, so the -singlestep option doesn't work.

Quoted 3 lines · Aurelien Jarno

Given those explanations and the comments from other people, I am fine with this option. I still have some comments though (see below).

Please see my comments and an updated patch below.

Quoted 18 lines

vl.c 2009-03-13 17:08:01.000000000 +0100 line 214

Quoted 12 lines

...
Index: trunk/vl.c
===================================================================

vl.c 2009-03-13 17:08:01.000000000 +0100 renamed from vl.c 2009-03-13 17:07:47.000000000 +0100 +1 -0

@@ -211,6 +211,7 @@
 int nb_nics;
 NICInfo nd_table[MAX_NICS];
 int vm_running;
+int vm_singlestep;

You create a new variable. By the way, I think that calling it singlestep is better, and matches the naming of other options variable (like daemonize, graphic_rotate). You should define it to a default value of 0.

Calling those option variables option_singlestep, option_daemonize might even be a better solution...

I called it vm_singlestep because it is somehow similar to vm_running. It is a variable which indicates a certain state of QEMU's VM. Nevertheless I renamed it to singlestep in my new patch.

The default value is already 0 because all globals in C/C++ have this default value (BSS segment). An explicit value just increases the size of the executable (only by 4 (or 8) bytes in this case, I admit).

Today, there is no consistent usage of global default values. Some globals are explicitly set to zero, others not. I personally use explicit default values only when they are needed (!= 0).

Please feel free to add a "= 0" if you think this should be QEMU's standard.

Quoted 10 lines

Quoted 10 lines

vl.c 2009-03-13 17:08:01.000000000 +0100 renamed from vl.c 2009-03-13 17:07:47.000000000 +0100 +1 -0

@@ -4221,6 +4223,7 @@
 QEMU_OPTION_parallel,
 QEMU_OPTION_monitor,
 QEMU_OPTION_pidfile,
+ QEMU_OPTION_singlestep,
 QEMU_OPTION_S,
 QEMU_OPTION_s,
 QEMU_OPTION_p,

@@ -4345,6 +4348,7 @@
 { "parallel", HAS_ARG, QEMU_OPTION_parallel },

Quoted 8 lines

vl.c 2009-03-13 17:08:01.000000000 +0100 line 4354

Quoted 6 lines

vl.c 2009-03-13 17:08:01.000000000 +0100 renamed from vl.c 2009-03-13 17:07:47.000000000 +0100 +1 -0

 { "monitor", HAS_ARG, QEMU_OPTION_monitor },
 { "pidfile", HAS_ARG, QEMU_OPTION_pidfile },
+ { "singlestep", 0, QEMU_OPTION_singlestep },
 { "S", 0, QEMU_OPTION_S },
 { "s", 0, QEMU_OPTION_s },
 { "p", HAS_ARG, QEMU_OPTION_p },

This option is never parsed, so the -singlestep option doesn't work.

True. The parser code got lost somewhere from my first to this incomplete patch
because of many, many merges in the meantime.

The new patch fixes this.

So I hope this will be the last iteration and the patch will finally find its
way to QEMU trunk.

Regards

Stefan

Add new command line option -singlestep for tcg single stepping.

This replaces a compile time option for some targets and adds this feature to targets which did not have a compile time option.

Add monitor command to enable or disable single step mode.

Modify monitor command "info status" to display single step mode.

Signed-off-by Stefan Weil <[email protected]>

Index: trunk/bsd-user/main.c
===================================================================

bsd-user/main.c +5 -0

@@ -33,6 +33,8 @@
 
 #define DEBUG_LOGFILE "/tmp/qemu.log"
 
+int singlestep;
+
 static const char *interp_prefix = CONFIG_QEMU_PREFIX;
 const char *qemu_uname_release = CONFIG_UNAME_RELEASE;
 extern char **environ;

@@ -378,6 +380,7 @@
            "Debug options:\n"
            "-d options   activate log (logfile=%s)\n"
            "-p pagesize  set the host page size to 'pagesize'\n"
+           "-singlestep  always run in singlestep mode\n"
            "-strace      log system calls\n"
            "\n"
            "Environment variables:\n"

@@ -500,6 +503,8 @@
                 usage();
             }
             optind++;
+        } else if (!strcmp(r, "singlestep")) {
+            singlestep = 1;
         } else if (!strcmp(r, "strace")) {
             do_strace = 1;
         } else

Index: trunk/darwin-user/main.c
===================================================================

darwin-user/main.c +5 -0

@@ -41,6 +41,8 @@
 #include <mach/mach_init.h>
 #include <mach/vm_map.h>
 
+int singlestep;
+
 const char *interp_prefix = "";
 
 asm(".zerofill __STD_PROG_ZONE, __STD_PROG_ZONE, __std_prog_zone, 0x0dfff000");

@@ -751,6 +753,7 @@
            "-d options   activate log (logfile='%s')\n"
            "-g wait for gdb on port 1234\n"
            "-p pagesize  set the host page size to 'pagesize'\n",
+           "-singlestep  always run in singlestep mode\n"
            TARGET_ARCH,
            TARGET_ARCH,
            interp_prefix,

@@ -842,6 +845,8 @@
 #endif
                 exit(1);
             }
+        } else if (!strcmp(r, "singlestep")) {
+            singlestep = 1;
         } else
         {
             usage();

Index: trunk/exec-all.h
===================================================================

exec-all.h +2 -0

@@ -381,6 +381,8 @@
 
 #endif
 
+extern int singlestep;
+
 typedef void (CPUDebugExcpHandler)(CPUState *env);
 
 CPUDebugExcpHandler *cpu_set_debug_excp_handler(CPUDebugExcpHandler *handler);

Index: trunk/linux-user/main.c
===================================================================

linux-user/main.c +5 -0

@@ -39,6 +39,8 @@
 
 char *exec_path;
 
+int singlestep;
+
 static const char *interp_prefix = CONFIG_QEMU_PREFIX;
 const char *qemu_uname_release = CONFIG_UNAME_RELEASE;
 

@@ -2217,6 +2219,7 @@
            "Debug options:\n"
            "-d options   activate log (logfile=%s)\n"
            "-p pagesize  set the host page size to 'pagesize'\n"
+           "-singlestep  always run in singlestep mode\n"
            "-strace      log system calls\n"
            "\n"
            "Environment variables:\n"

@@ -2359,6 +2362,8 @@
             }
         } else if (!strcmp(r, "drop-ld-preload")) {
             (void) envlist_unsetenv(envlist, "LD_PRELOAD");
+        } else if (!strcmp(r, "singlestep")) {
+            singlestep = 1;
         } else if (!strcmp(r, "strace")) {
             do_strace = 1;
         } else

Index: trunk/monitor.c
===================================================================

monitor.c +20 -3

@@ -527,6 +527,17 @@
     cpu_set_log(mask);
 }
 
+static void do_singlestep(Monitor *mon, const char *option)
+{
+    if (!option || !strcmp(option, "on")) {
+        singlestep = 1;
+    } else if (!strcmp(option, "off")) {
+        singlestep = 0;
+    } else {
+        monitor_printf(mon, "unexpected option %s\n", option);
+    }
+}
+
 static void do_stop(Monitor *mon)
 {
     vm_stop(EXCP_INTERRUPT);

@@ -1510,9 +1521,13 @@
 
 static void do_info_status(Monitor *mon)
 {
-    if (vm_running)
-       monitor_printf(mon, "VM status: running\n");
-    else
+    if (vm_running) {
+        if (singlestep) {
+            monitor_printf(mon, "VM status: running (single step mode)\n");
+        } else {
+            monitor_printf(mon, "VM status: running\n");
+        }
+    } else
        monitor_printf(mon, "VM status: paused\n");
 }
 

@@ -1643,6 +1658,8 @@
       "tag|id", "restore a VM snapshot from its tag or id" },
     { "delvm", "s", do_delvm,
       "tag|id", "delete a VM snapshot from its tag or id" },
+    { "singlestep", "s?", do_singlestep,
+      "[on|off]", "run emulation in singlestep mode or switch to normal mode", },
     { "stop", "", do_stop,
       "", "stop emulation", },
     { "c|cont", "", do_cont,

Index: trunk/qemu-doc.texi
===================================================================

qemu-doc.texi +10 -0

@@ -490,6 +490,10 @@
 @item delvm @var{tag}|@var{id}
 Delete the snapshot identified by @var{tag} or @var{id}.
 
+@item singlestep [off]
+Run the emulation in single step mode.
+If called with option off, the emulation returns to normal mode.
+
 @item stop
 Stop emulation.
 

@@ -2370,6 +2374,8 @@
 Act as if the host page size was 'pagesize' bytes
 @item -g port
 Wait gdb connection to port
+@item -singlestep
+Run the emulation in single step mode.
 @end table
 
 Environment variables:

@@ -2488,6 +2494,8 @@
 Activate log (logfile=/tmp/qemu.log)
 @item -p pagesize
 Act as if the host page size was 'pagesize' bytes
+@item -singlestep
+Run the emulation in single step mode.
 @end table
 
 @node BSD User space emulator

@@ -2550,6 +2558,8 @@
 Activate log (logfile=/tmp/qemu.log)
 @item -p pagesize
 Act as if the host page size was 'pagesize' bytes
+@item -singlestep
+Run the emulation in single step mode.
 @end table
 
 @node compilation

Index: trunk/qemu-options.hx
===================================================================

qemu-options.hx +7 -0

@@ -1209,6 +1209,13 @@
 from a script.
 ETEXI
 
+DEF("singlestep", 0, QEMU_OPTION_singlestep, \
+    "-singlestep   always run in singlestep mode\n")
+STEXI
+@item -singlestep
+Run the emulation in single step mode.
+ETEXI
+
 DEF("S", 0, QEMU_OPTION_S, \
     "-S              freeze CPU at startup (use 'c' to start execution)\n")
 STEXI

Index: trunk/target-alpha/translate.c
===================================================================

target-alpha/translate.c +4 -4

@@ -2412,11 +2412,11 @@
         if (env->singlestep_enabled) {
             gen_excp(&ctx, EXCP_DEBUG, 0);
             break;
-	}
+        }
 
-#if defined (DO_SINGLE_STEP)
-        break;
-#endif
+        if (singlestep) {
+            break;
+        }
     }
     if (ret != 1 && ret != 3) {
         tcg_gen_movi_i64(cpu_pc, ctx.pc);

Index: trunk/target-arm/translate.c
===================================================================

target-arm/translate.c +1 -0

@@ -8791,6 +8791,7 @@
         num_insns ++;
     } while (!dc->is_jmp && gen_opc_ptr < gen_opc_end &&
              !env->singlestep_enabled &&
+             !singlestep &&
              dc->pc < next_page_start &&
              num_insns < max_insns);
 

Index: trunk/target-cris/translate.c
===================================================================

target-cris/translate.c +1 -0

@@ -3272,6 +3272,7 @@
 			break;
 	} while (!dc->is_jmp && !dc->cpustate_changed
 		 && gen_opc_ptr < gen_opc_end
+                 && !singlestep
 		 && (dc->pc < next_page_start)
                  && num_insns < max_insns);
 

Index: trunk/target-i386/translate.c
===================================================================

target-i386/translate.c +5 -0

@@ -7651,6 +7651,11 @@
             gen_eob(dc);
             break;
         }
+        if (singlestep) {
+            gen_jmp_im(pc_ptr - dc->cs_base);
+            gen_eob(dc);
+            break;
+        }
     }
     if (tb->cflags & CF_LAST_IO)
         gen_io_end();

Index: trunk/target-m68k/translate.c
===================================================================

target-m68k/translate.c +1 -0

@@ -3031,6 +3031,7 @@
         num_insns++;
     } while (!dc->is_jmp && gen_opc_ptr < gen_opc_end &&
              !env->singlestep_enabled &&
+             !singlestep &&
              (pc_offset) < (TARGET_PAGE_SIZE - 32) &&
              num_insns < max_insns);
 

Index: trunk/target-mips/translate.c
===================================================================

target-mips/translate.c +3 -4

@@ -38,7 +38,6 @@
 
 //#define MIPS_DEBUG_DISAS
 //#define MIPS_DEBUG_SIGN_EXTENSIONS
-//#define MIPS_SINGLE_STEP
 
 /* MIPS major opcodes */
 #define MASK_OP_MAJOR(op)  (op & (0x3F << 26))

@@ -8140,9 +8139,9 @@
 
         if (num_insns >= max_insns)
             break;
-#if defined (MIPS_SINGLE_STEP)
-        break;
-#endif
+
+        if (singlestep)
+            break;
     }
     if (tb->cflags & CF_LAST_IO)
         gen_io_end();

Index: trunk/target-ppc/translate.c
===================================================================

target-ppc/translate.c +1 -4

@@ -39,7 +39,6 @@
 #define GDBSTUB_SINGLE_STEP 0x4
 
 /* Include definitions for instructions classes and implementations flags */
-//#define DO_SINGLE_STEP
 //#define PPC_DEBUG_DISAS
 //#define DO_PPC_STATISTICS
 

@@ -8288,15 +8287,13 @@
             gen_exception(ctxp, POWERPC_EXCP_TRACE);
         } else if (unlikely(((ctx.nip & (TARGET_PAGE_SIZE - 1)) == 0) ||
                             (env->singlestep_enabled) ||
+                            singlestep ||
                             num_insns >= max_insns)) {
             /* if we reach a page boundary or are single stepping, stop
              * generation
              */
             break;
         }
-#if defined (DO_SINGLE_STEP)
-        break;
-#endif
     }
     if (tb->cflags & CF_LAST_IO)
         gen_io_end();

Index: trunk/target-sh4/translate.c
===================================================================

target-sh4/translate.c +2 -3

@@ -1929,9 +1929,8 @@
 	    break;
         if (num_insns >= max_insns)
             break;
-#ifdef SH4_SINGLE_STEP
-	break;
-#endif
+        if (singlestep)
+            break;
     }
     if (tb->cflags & CF_LAST_IO)
         gen_io_end();

Index: trunk/target-sparc/translate.c
===================================================================

target-sparc/translate.c +1 -1

@@ -4838,7 +4838,7 @@
             break;
         /* if single step mode, we generate only one instruction and
            generate an exception */
-        if (env->singlestep_enabled) {
+        if (env->singlestep_enabled || singlestep) {
             tcg_gen_movi_tl(cpu_pc, dc->pc);
             tcg_gen_exit_tb(0);
             break;

Index: trunk/vl.c
===================================================================

vl.c +4 -0

@@ -212,6 +212,7 @@
 int nb_nics;
 NICInfo nd_table[MAX_NICS];
 int vm_running;
+int singlestep;
 static int autostart;
 static int rtc_utc = 1;
 static int rtc_date_offset = -1; /* -1 means no change */

@@ -4669,6 +4670,9 @@
             case QEMU_OPTION_bios:
                 bios_name = optarg;
                 break;
+            case QEMU_OPTION_singlestep:
+                singlestep = 1;
+                break;
             case QEMU_OPTION_S:
                 autostart = 0;
                 break;