* [Qemu-devel] [PATCH 0/5] gdbstub and single step improvments
@ 2008-05-15 14:11 Jason Wessel
2008-05-15 14:11 ` [Qemu-devel] [PATCH 1/5] gdbstub: replace singlestep q packets with qRcmd packets Jason Wessel
0 siblings, 1 reply; 20+ messages in thread
From: Jason Wessel @ 2008-05-15 14:11 UTC (permalink / raw)
To: qemu-devel
The following series of patches improves the gdbstub inside qemu to
add some functionality as well as to fix some minor problems.
The last patch in the series fixes a ppc specific single stepping
problem found with testing the new code.
---
Jason Wessel (5):
gdbstub: replace singlestep q packets with qRcmd packets
gdbstub: gdb pass-through qemu monitor support
vl.c: always run the real time timers when single stepping
gdbstub: support for gdb "detach/kill/quit"
ppc: fix crash in ppc system single step support
console.h | 1 +
cpu-all.h | 2 +
cpu-exec.c | 2 +-
exec.c | 21 ++++++++
gdbstub.c | 135 +++++++++++++++++++++++++++++++++++++++--------
monitor.c | 8 ++-
qemu-doc.texi | 43 +++++++++------
target-ppc/translate.c | 4 --
vl.c | 3 +-
9 files changed, 170 insertions(+), 49 deletions(-)
^ permalink raw reply [flat|nested] 20+ messages in thread* [Qemu-devel] [PATCH 1/5] gdbstub: replace singlestep q packets with qRcmd packets 2008-05-15 14:11 [Qemu-devel] [PATCH 0/5] gdbstub and single step improvments Jason Wessel @ 2008-05-15 14:11 ` Jason Wessel 2008-05-15 14:11 ` [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support Jason Wessel 2008-05-15 21:33 ` [Qemu-devel] [PATCH 1/5] gdbstub: replace singlestep q packets with qRcmd packets Edgar E. Iglesias 0 siblings, 2 replies; 20+ messages in thread From: Jason Wessel @ 2008-05-15 14:11 UTC (permalink / raw) To: qemu-devel At the gdbserial protocol level, using the qRcmd packets allows gdb to use the "monitor" command to access the controls for single stepping behavior. Now you can use a gdb "monitor" command instead of a gdb "maintenance" command. The qemu docs were updated to reflect this change. Signed-off-by: Jason Wessel <jason.wessel@windriver.com> --- gdbstub.c | 82 +++++++++++++++++++++++++++++++++++++++++---------------- qemu-doc.texi | 34 +++++++++++------------ 2 files changed, 75 insertions(+), 41 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 9a361e3..4bc7999 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -239,6 +239,27 @@ static int put_packet(GDBState *s, char *buf) return 0; } +static void monitor_output(GDBState *s, const char *msg) +{ + char *buf; + int len = strlen(msg); + + buf = malloc(len * 2 + 2); + buf[0] = 'O'; + memtohex(buf + 1, msg, len); + put_packet(s, buf); + free(buf); +} + +static void monitor_help(GDBState *s) +{ + monitor_output(s, "gdbstub specific monitor commands:\n"); + monitor_output(s, "s_show -- Show gdbstub Single Stepping variables\n"); + monitor_output(s, "set s_step <0|1> -- Single Stepping enabled\n"); + monitor_output(s, "set s_irq <0|1> -- Single Stepping with qemu irq handlers enabled\n"); + monitor_output(s, "set s_timer <0|1> -- Single Stepping with qemu timers enabled\n"); +} + #if defined(TARGET_I386) #ifdef TARGET_X86_64 @@ -943,6 +964,42 @@ static void cpu_gdb_write_registers(CPUState *env, uint8_t *mem_buf, int size) #endif +static void gdb_rcmd(GDBState *s, const char *p, char *buf, char *mem_buf) +{ + int len = strlen(p); + + if ((len % 2) != 0) { + put_packet(s, "E01"); + return; + } + hextomem(mem_buf, p, len); + mem_buf[(len >> 1)] = 0; + + if (!strcmp(mem_buf, "s_show")) { + sprintf(buf, "s_step == %i\n", (sstep_flags & SSTEP_ENABLE) != 0); + monitor_output(s, buf); + sprintf(buf, "s_irq == %i\n", (sstep_flags & SSTEP_NOIRQ) == 0); + monitor_output(s, buf); + sprintf(buf, "s_timer == %i\n", (sstep_flags & SSTEP_NOTIMER) == 0); + monitor_output(s, buf); + } else if (!strcmp(mem_buf, "s_step 1")) { + sstep_flags |= SSTEP_ENABLE; + } else if (!strcmp(mem_buf, "s_step 0")) { + sstep_flags &= ~SSTEP_ENABLE; + } else if (!strcmp(mem_buf, "s_irq 1")) { + sstep_flags &= ~SSTEP_NOIRQ; + } else if (!strcmp(mem_buf, "s_irq 0")) { + sstep_flags |= SSTEP_NOIRQ; + } else if (!strcmp(mem_buf, "s_timer 1")) { + sstep_flags &= ~SSTEP_NOTIMER; + } else if (!strcmp(mem_buf, "s_timer 0")) { + sstep_flags |= SSTEP_NOTIMER; + } else if (!strcmp(mem_buf, "help") || !strcmp(mem_buf, "?")) { + monitor_help(s); + } + put_packet(s, "OK"); +} + static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf) { const char *p; @@ -1113,29 +1170,8 @@ static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf) } 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"); + if (!strncmp(p, "Rcmd,", 5)) { + gdb_rcmd(s, p + 5, buf, mem_buf); break; } #ifdef CONFIG_LINUX_USER diff --git a/qemu-doc.texi b/qemu-doc.texi index cca483c..11c7058 100644 --- a/qemu-doc.texi +++ b/qemu-doc.texi @@ -1950,32 +1950,30 @@ Use @code{set architecture i8086} to dump 16 bit code. Then use 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: +The default single stepping behavior is to 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 execute. Because there are rare circumstances where you want to single step into an interrupt vector the behavior can be controlled from GDB. There are several commands you use to query and set the single step behavior while inside gdb: @table @code -@item maintenance packet qqemu.sstepbits +@item monitor s_show -This will display the MASK bits used to control the single stepping IE: +This will display the values of the single stepping controls IE: @example -(gdb) maintenance packet qqemu.sstepbits -sending: "qqemu.sstepbits" -received: "ENABLE=1,NOIRQ=2,NOTIMER=4" +(gdb) monitor s_show +s_step == 1 +s_irq == 0 +s_timer == 0 @end example -@item maintenance packet qqemu.sstep +@item monitor s_step <0|1> -This will display the current value of the mask used when single stepping IE: +Turn off or on the single stepping feature all together, which is defaulted to on. @example -(gdb) maintenance packet qqemu.sstep -sending: "qqemu.sstep" -received: "0x7" +(gdb) monitor s_step 1 +(gdb) monitor s_step 0 @end example -@item maintenance packet Qqemu.sstep=HEX_VALUE +@item monitor s_irq <0|1> -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 +Turn off or on the the irq processing when single stepping, which is defaulted to off. +@item monitor s_timer <0|1> + +Turn off or on the the timer processing when single stepping, which is defaulted to off. @end table @node pcsys_os_specific -- 1.5.5.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support 2008-05-15 14:11 ` [Qemu-devel] [PATCH 1/5] gdbstub: replace singlestep q packets with qRcmd packets Jason Wessel @ 2008-05-15 14:11 ` Jason Wessel 2008-05-15 14:11 ` [Qemu-devel] [PATCH 3/5] vl.c: always run the real time timers when single stepping Jason Wessel 2008-05-15 22:17 ` [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support Edgar E. Iglesias 2008-05-15 21:33 ` [Qemu-devel] [PATCH 1/5] gdbstub: replace singlestep q packets with qRcmd packets Edgar E. Iglesias 1 sibling, 2 replies; 20+ messages in thread From: Jason Wessel @ 2008-05-15 14:11 UTC (permalink / raw) To: qemu-devel This patch adds a feature to the gdbstub to allow gdb to issue monitor commands that can pass-through to the qemu monitor. In order to make this work, the MAX_MON (the maximum number of monitor connections) had to get incremented by 1 to support the case when the gdbstub is setup along with all the other connections. A small check was added avoid strange crashes when you allocate too many qemu monitor connections. The monitor_handle_command had to be exported in order to allow gdbstub to pass the strings directly to the monitor. The gdbstub registers as an output device of the qemu monitor such that no further changes were needed to the monitor other than to have a global variable in the gdbstub that controls when to transmit data from a pass through monitor command to an attached debugger. The qemu docs were updated to reflect this change. Signed-off-by: Jason Wessel <jason.wessel@windriver.com> --- console.h | 1 + gdbstub.c | 47 +++++++++++++++++++++++++++++++++++++++++++---- monitor.c | 8 ++++++-- qemu-doc.texi | 11 +++++++++++ 4 files changed, 61 insertions(+), 6 deletions(-) diff --git a/console.h b/console.h index c7f29f5..041b4ef 100644 --- a/console.h +++ b/console.h @@ -159,6 +159,7 @@ extern uint8_t _translate_keycode(const int key); does not need to include console.h */ /* monitor.c */ void monitor_init(CharDriverState *hd, int show_banner); +void monitor_handle_command(const char *cmdline); void term_puts(const char *str); void term_vprintf(const char *fmt, va_list ap); void term_printf(const char *fmt, ...) __attribute__ ((__format__ (__printf__, 1, 2))); diff --git a/gdbstub.c b/gdbstub.c index 4bc7999..833cdd9 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -33,6 +33,7 @@ #include "qemu-char.h" #include "sysemu.h" #include "gdbstub.h" +#include "console.h" #endif #include "qemu_socket.h" @@ -70,6 +71,8 @@ typedef struct GDBState { int running_state; #else CharDriverState *chr; + CharDriverState *mon; + int allow_monitor; #endif } GDBState; @@ -239,10 +242,11 @@ static int put_packet(GDBState *s, char *buf) return 0; } -static void monitor_output(GDBState *s, const char *msg) +static void monitor_output_len(GDBState *s, const char *msg, int len) { char *buf; - int len = strlen(msg); + if (!len) + len = strlen(msg); buf = malloc(len * 2 + 2); buf[0] = 'O'; @@ -251,6 +255,11 @@ static void monitor_output(GDBState *s, const char *msg) free(buf); } +static void monitor_output(GDBState *s, const char *msg) +{ + monitor_output_len(s, msg, 0); +} + static void monitor_help(GDBState *s) { monitor_output(s, "gdbstub specific monitor commands:\n"); @@ -258,6 +267,7 @@ static void monitor_help(GDBState *s) monitor_output(s, "set s_step <0|1> -- Single Stepping enabled\n"); monitor_output(s, "set s_irq <0|1> -- Single Stepping with qemu irq handlers enabled\n"); monitor_output(s, "set s_timer <0|1> -- Single Stepping with qemu timers enabled\n"); + monitor_output(s, "qemu monitor pass-through commands:\n"); } #if defined(TARGET_I386) @@ -994,8 +1004,14 @@ static void gdb_rcmd(GDBState *s, const char *p, char *buf, char *mem_buf) sstep_flags &= ~SSTEP_NOTIMER; } else if (!strcmp(mem_buf, "s_timer 0")) { sstep_flags |= SSTEP_NOTIMER; - } else if (!strcmp(mem_buf, "help") || !strcmp(mem_buf, "?")) { - monitor_help(s); + } else { + if (!strcmp(mem_buf, "help") || !strcmp(mem_buf, "?")) + monitor_help(s); +#ifndef CONFIG_USER_ONLY + s->allow_monitor = 1; + monitor_handle_command(mem_buf); + s->allow_monitor = 0; +#endif } put_packet(s, "OK"); } @@ -1536,6 +1552,26 @@ static void gdb_chr_event(void *opaque, int event) } } +static int gdb_mon_chr_write(CharDriverState *chr, const uint8_t *buf, int len) +{ + GDBState *s = chr->opaque; + if (s->allow_monitor) + monitor_output_len(s, buf, len); + return len; +} + +static CharDriverState *qemu_chr_open_gdb_mon(GDBState *s) +{ + CharDriverState *chr; + + chr = qemu_mallocz(sizeof(CharDriverState)); + if (!chr) + return NULL; + chr->chr_write = gdb_mon_chr_write; + chr->opaque = s; + return chr; +} + int gdbserver_start(const char *port) { GDBState *s; @@ -1568,6 +1604,9 @@ int gdbserver_start(const char *port) qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive, gdb_chr_event, s); qemu_add_vm_stop_handler(gdb_vm_stopped, s); + /* Initialize a monitor port for gdb */ + s->mon = qemu_chr_open_gdb_mon(s); + monitor_init(s->mon, 0); return 0; } #endif diff --git a/monitor.c b/monitor.c index 236b827..2d2a110 100644 --- a/monitor.c +++ b/monitor.c @@ -69,7 +69,7 @@ typedef struct term_cmd_t { const char *help; } term_cmd_t; -#define MAX_MON 4 +#define MAX_MON 5 static CharDriverState *monitor_hd[MAX_MON]; static int hide_banner; @@ -2096,7 +2096,7 @@ static int default_fmt_size = 4; #define MAX_ARGS 16 -static void monitor_handle_command(const char *cmdline) +void monitor_handle_command(const char *cmdline) { const char *p, *pstart, *typestr; char *q; @@ -2649,6 +2649,10 @@ void monitor_init(CharDriverState *hd, int show_banner) break; } } + if (i >= MAX_MON) { + fprintf(stderr, "ERROR too many monitor connections requested\n"); + exit(0); + } hide_banner = !show_banner; diff --git a/qemu-doc.texi b/qemu-doc.texi index 11c7058..f981b08 100644 --- a/qemu-doc.texi +++ b/qemu-doc.texi @@ -1976,6 +1976,17 @@ Turn off or on the the irq processing when single stepping, which is defaulted t Turn off or on the the timer processing when single stepping, which is defaulted to off. @end table +You may also use gdb to send pass-through monitor commands to the qemu monitor. It is easiest to start off with using just using @code{monitor help} to display the list of commands in available from the monitor. Then you can prefix the @code{monitor} key word prior to any command. The example below show the use of the qemu monitor's @code{info pci} command. + +@example +(gdb) monitor info pci + Bus 0, device 0, function 0: + Host bridge: PCI device 1057:4801 + Bus 0, device 1, function 0: + VGA controller: PCI device 1234:1111 + BAR0: 32 bit memory at 0xf0000000 [0xf07fffff]. +@end example + @node pcsys_os_specific @section Target OS specific information -- 1.5.5.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 3/5] vl.c: always run the real time timers when single stepping 2008-05-15 14:11 ` [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support Jason Wessel @ 2008-05-15 14:11 ` Jason Wessel 2008-05-15 14:11 ` [Qemu-devel] [PATCH 4/5] gdbstub: support for gdb "detach/kill/quit" Jason Wessel 2008-05-15 22:17 ` [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support Edgar E. Iglesias 1 sibling, 1 reply; 20+ messages in thread From: Jason Wessel @ 2008-05-15 14:11 UTC (permalink / raw) To: qemu-devel The real time timers should always be processed regardless if you are single stepping or not. 99.9% of the time we are not single stepping so optimize for it. Signed-off-by: Jason Wessel <jason.wessel@windriver.com> --- cpu-exec.c | 2 +- vl.c | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cpu-exec.c b/cpu-exec.c index 435fdf8..5fd9cad 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -420,7 +420,7 @@ int cpu_exec(CPUState *env1) #if defined(TARGET_I386) && env->hflags & HF_GIF_MASK #endif - && !(env->singlestep_enabled & SSTEP_NOIRQ)) { + && likely(!(env->singlestep_enabled & SSTEP_NOIRQ))) { if (interrupt_request & CPU_INTERRUPT_DEBUG) { env->interrupt_request &= ~CPU_INTERRUPT_DEBUG; env->exception_index = EXCP_DEBUG; diff --git a/vl.c b/vl.c index 67712f0..5430ae6 100644 --- a/vl.c +++ b/vl.c @@ -7032,7 +7032,7 @@ void main_loop_wait(int timeout) qemu_aio_poll(); if (vm_running) { - if (!(cur_cpu->singlestep_enabled & SSTEP_NOTIMER)) + if (likely(!(cur_cpu->singlestep_enabled & SSTEP_NOTIMER))) qemu_run_timers(&active_timers[QEMU_TIMER_VIRTUAL], qemu_get_clock(vm_clock)); /* run dma transfers, if any */ @@ -7040,7 +7040,6 @@ void main_loop_wait(int timeout) } /* real time timers */ - if (!(cur_cpu->singlestep_enabled & SSTEP_NOTIMER)) qemu_run_timers(&active_timers[QEMU_TIMER_REALTIME], qemu_get_clock(rt_clock)); -- 1.5.5.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 4/5] gdbstub: support for gdb "detach/kill/quit" 2008-05-15 14:11 ` [Qemu-devel] [PATCH 3/5] vl.c: always run the real time timers when single stepping Jason Wessel @ 2008-05-15 14:11 ` Jason Wessel 2008-05-15 14:11 ` [Qemu-devel] [PATCH 5/5] ppc: fix crash in ppc system single step support Jason Wessel 2008-05-15 21:13 ` [Qemu-devel] [PATCH 4/5] gdbstub: support for gdb "detach/kill/quit" Edgar E. Iglesias 0 siblings, 2 replies; 20+ messages in thread From: Jason Wessel @ 2008-05-15 14:11 UTC (permalink / raw) To: qemu-devel Implement the 'k' gdbserial packet which kills the qemu instance via the debugger stub. Implement the 'D' detach packet for the gdb stub such that you can disconnect gdb with the "detach" command. This required implementing a cpu_breakpoint_remove_all() and a cpu_watchpoint_remove_all() function to cleanup all the breakpoints and watchpoints prior to leaving the gdb stub else simulation can stop with no debugger attached. On a '?' packet remove all the breakpoints and watchpoints. This is considered more of a safety net in case you force killed gdb or it crashed and you are reconnecting. The identical behavior exists for kgdb in the linux kernel. Signed-off-by: Jason Wessel <jason.wessel@windriver.com> --- cpu-all.h | 2 ++ exec.c | 21 +++++++++++++++++++++ gdbstub.c | 17 +++++++++++++++++ 3 files changed, 40 insertions(+), 0 deletions(-) diff --git a/cpu-all.h b/cpu-all.h index 7e77f76..d71166c 100644 --- a/cpu-all.h +++ b/cpu-all.h @@ -760,8 +760,10 @@ void cpu_reset_interrupt(CPUState *env, int mask); int cpu_watchpoint_insert(CPUState *env, target_ulong addr); int cpu_watchpoint_remove(CPUState *env, target_ulong addr); +void cpu_watchpoint_remove_all(CPUState *env); int cpu_breakpoint_insert(CPUState *env, target_ulong pc); int cpu_breakpoint_remove(CPUState *env, target_ulong pc); +void cpu_breakpoint_remove_all(CPUState *env); #define SSTEP_ENABLE 0x1 /* Enable simulated HW single stepping */ #define SSTEP_NOIRQ 0x2 /* Do not use IRQ while single stepping */ diff --git a/exec.c b/exec.c index 2fd0078..8f5c868 100644 --- a/exec.c +++ b/exec.c @@ -1139,6 +1139,16 @@ int cpu_watchpoint_remove(CPUState *env, target_ulong addr) return -1; } +/* Remove all watchpoints. */ +void cpu_watchpoint_remove_all(CPUState *env) { + int i; + + for (i = 0; i < env->nb_watchpoints; i++) { + tlb_flush_page(env, env->watchpoint[i].vaddr); + } + env->nb_watchpoints = 0; +} + /* add a breakpoint. EXCP_DEBUG is returned by the CPU loop if a breakpoint is reached */ int cpu_breakpoint_insert(CPUState *env, target_ulong pc) @@ -1162,6 +1172,17 @@ int cpu_breakpoint_insert(CPUState *env, target_ulong pc) #endif } +/* remove all breakpoints */ +void cpu_breakpoint_remove_all(CPUState *env) { +#if defined(TARGET_HAS_ICE) + int i; + for(i = 0; i < env->nb_breakpoints; i++) { + breakpoint_invalidate(env, env->breakpoints[i]); + } + env->nb_breakpoints = 0; +#endif +} + /* remove a breakpoint */ int cpu_breakpoint_remove(CPUState *env, target_ulong pc) { diff --git a/gdbstub.c b/gdbstub.c index 833cdd9..fb2d948 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1035,6 +1035,12 @@ static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf) /* TODO: Make this return the correct value for user-mode. */ snprintf(buf, sizeof(buf), "S%02x", SIGTRAP); put_packet(s, buf); + /* Remove all the breakpoints when this query is issued, + * because gdb is doing and initial connect and the state + * should be cleaned up. + */ + cpu_breakpoint_remove_all(env); + cpu_watchpoint_remove_all(env); break; case 'c': if (*p != '\0') { @@ -1058,6 +1064,17 @@ static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf) } gdb_continue(s); return RS_IDLE; + case 'k': + /* Kill the target */ + fprintf(stderr, "\nQEMU: Terminated via GDBstub\n"); + exit(0); + case 'D': + /* Detach packet */ + cpu_breakpoint_remove_all(env); + cpu_watchpoint_remove_all(env); + gdb_continue(s); + put_packet(s, "OK"); + break; case 's': if (*p != '\0') { addr = strtoull(p, (char **)&p, 16); -- 1.5.5.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 5/5] ppc: fix crash in ppc system single step support 2008-05-15 14:11 ` [Qemu-devel] [PATCH 4/5] gdbstub: support for gdb "detach/kill/quit" Jason Wessel @ 2008-05-15 14:11 ` Jason Wessel 2008-05-15 21:13 ` [Qemu-devel] [PATCH 4/5] gdbstub: support for gdb "detach/kill/quit" Edgar E. Iglesias 1 sibling, 0 replies; 20+ messages in thread From: Jason Wessel @ 2008-05-15 14:11 UTC (permalink / raw) To: qemu-devel There was a bogus case where two system debug ops get generated. This patch removes the broken system debug op. This was a left over after making some changes to correctly generate debug ops on branch operations inside gen_goto_tb(); The test case against this patch is to turn on single stepping with timers, boot a linux kernel, set a breakpoint a do_fork and in gdb execute "si 3000". Then qemu-system-ppc will fault executing a debug op, which should not have been executed. Signed-off-by: Jason Wessel <jason.wessel@windriver.com> --- target-ppc/translate.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-) diff --git a/target-ppc/translate.c b/target-ppc/translate.c index 4e71614..45da869 100644 --- a/target-ppc/translate.c +++ b/target-ppc/translate.c @@ -3002,10 +3002,6 @@ static always_inline void gen_bcond (DisasContext *ctx, int type) #endif gen_op_btest_T1(ctx->nip); no_test: - if (ctx->singlestep_enabled & GDBSTUB_SINGLE_STEP) { - gen_update_nip(ctx, ctx->nip); - gen_op_debug(); - } tcg_gen_exit_tb(0); } } -- 1.5.5.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 4/5] gdbstub: support for gdb "detach/kill/quit" 2008-05-15 14:11 ` [Qemu-devel] [PATCH 4/5] gdbstub: support for gdb "detach/kill/quit" Jason Wessel 2008-05-15 14:11 ` [Qemu-devel] [PATCH 5/5] ppc: fix crash in ppc system single step support Jason Wessel @ 2008-05-15 21:13 ` Edgar E. Iglesias 1 sibling, 0 replies; 20+ messages in thread From: Edgar E. Iglesias @ 2008-05-15 21:13 UTC (permalink / raw) To: Jason Wessel; +Cc: qemu-devel On Thu, May 15, 2008 at 09:11:32AM -0500, Jason Wessel wrote: > Implement the 'k' gdbserial packet which kills the qemu instance via > the debugger stub. > > Implement the 'D' detach packet for the gdb stub such that you can > disconnect gdb with the "detach" command. This required implementing > a cpu_breakpoint_remove_all() and a cpu_watchpoint_remove_all() > function to cleanup all the breakpoints and watchpoints prior to > leaving the gdb stub else simulation can stop with no debugger > attached. > > On a '?' packet remove all the breakpoints and watchpoints. This is > considered more of a safety net in case you force killed gdb or it > crashed and you are reconnecting. The identical behavior exists for > kgdb in the linux kernel. > > Signed-off-by: Jason Wessel <jason.wessel@windriver.com> Thanks Jason, this one looks good to me. > --- > cpu-all.h | 2 ++ > exec.c | 21 +++++++++++++++++++++ > gdbstub.c | 17 +++++++++++++++++ > 3 files changed, 40 insertions(+), 0 deletions(-) > > diff --git a/cpu-all.h b/cpu-all.h > index 7e77f76..d71166c 100644 > --- a/cpu-all.h > +++ b/cpu-all.h > @@ -760,8 +760,10 @@ void cpu_reset_interrupt(CPUState *env, int mask); > > int cpu_watchpoint_insert(CPUState *env, target_ulong addr); > int cpu_watchpoint_remove(CPUState *env, target_ulong addr); > +void cpu_watchpoint_remove_all(CPUState *env); > int cpu_breakpoint_insert(CPUState *env, target_ulong pc); > int cpu_breakpoint_remove(CPUState *env, target_ulong pc); > +void cpu_breakpoint_remove_all(CPUState *env); > > #define SSTEP_ENABLE 0x1 /* Enable simulated HW single stepping */ > #define SSTEP_NOIRQ 0x2 /* Do not use IRQ while single stepping */ > diff --git a/exec.c b/exec.c > index 2fd0078..8f5c868 100644 > --- a/exec.c > +++ b/exec.c > @@ -1139,6 +1139,16 @@ int cpu_watchpoint_remove(CPUState *env, target_ulong addr) > return -1; > } > > +/* Remove all watchpoints. */ > +void cpu_watchpoint_remove_all(CPUState *env) { > + int i; > + > + for (i = 0; i < env->nb_watchpoints; i++) { > + tlb_flush_page(env, env->watchpoint[i].vaddr); > + } > + env->nb_watchpoints = 0; > +} > + > /* add a breakpoint. EXCP_DEBUG is returned by the CPU loop if a > breakpoint is reached */ > int cpu_breakpoint_insert(CPUState *env, target_ulong pc) > @@ -1162,6 +1172,17 @@ int cpu_breakpoint_insert(CPUState *env, target_ulong pc) > #endif > } > > +/* remove all breakpoints */ > +void cpu_breakpoint_remove_all(CPUState *env) { > +#if defined(TARGET_HAS_ICE) > + int i; > + for(i = 0; i < env->nb_breakpoints; i++) { > + breakpoint_invalidate(env, env->breakpoints[i]); > + } > + env->nb_breakpoints = 0; > +#endif > +} > + > /* remove a breakpoint */ > int cpu_breakpoint_remove(CPUState *env, target_ulong pc) > { > diff --git a/gdbstub.c b/gdbstub.c > index 833cdd9..fb2d948 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -1035,6 +1035,12 @@ static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf) > /* TODO: Make this return the correct value for user-mode. */ > snprintf(buf, sizeof(buf), "S%02x", SIGTRAP); > put_packet(s, buf); > + /* Remove all the breakpoints when this query is issued, > + * because gdb is doing and initial connect and the state > + * should be cleaned up. > + */ > + cpu_breakpoint_remove_all(env); > + cpu_watchpoint_remove_all(env); > break; > case 'c': > if (*p != '\0') { > @@ -1058,6 +1064,17 @@ static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf) > } > gdb_continue(s); > return RS_IDLE; > + case 'k': > + /* Kill the target */ > + fprintf(stderr, "\nQEMU: Terminated via GDBstub\n"); > + exit(0); > + case 'D': > + /* Detach packet */ > + cpu_breakpoint_remove_all(env); > + cpu_watchpoint_remove_all(env); > + gdb_continue(s); > + put_packet(s, "OK"); > + break; > case 's': > if (*p != '\0') { > addr = strtoull(p, (char **)&p, 16); > -- > 1.5.5.1 > > -- Edgar E. Iglesias Axis Communications AB ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support 2008-05-15 14:11 ` [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support Jason Wessel 2008-05-15 14:11 ` [Qemu-devel] [PATCH 3/5] vl.c: always run the real time timers when single stepping Jason Wessel @ 2008-05-15 22:17 ` Edgar E. Iglesias 2008-05-19 13:29 ` Jason Wessel 1 sibling, 1 reply; 20+ messages in thread From: Edgar E. Iglesias @ 2008-05-15 22:17 UTC (permalink / raw) To: Jason Wessel; +Cc: qemu-devel On Thu, May 15, 2008 at 09:11:30AM -0500, Jason Wessel wrote: > This patch adds a feature to the gdbstub to allow gdb to issue monitor > commands that can pass-through to the qemu monitor. > > In order to make this work, the MAX_MON (the maximum number of monitor > connections) had to get incremented by 1 to support the case when the > gdbstub is setup along with all the other connections. A small check > was added avoid strange crashes when you allocate too many qemu > monitor connections. > > The monitor_handle_command had to be exported in order to allow > gdbstub to pass the strings directly to the monitor. The gdbstub > registers as an output device of the qemu monitor such that no further > changes were needed to the monitor other than to have a global > variable in the gdbstub that controls when to transmit data from a > pass through monitor command to an attached debugger. > > The qemu docs were updated to reflect this change. > > Signed-off-by: Jason Wessel <jason.wessel@windriver.com> This sounds like a neat feature to me. AFAICS the patch looks mostly ok, just a few nitpicks. > --- > console.h | 1 + > gdbstub.c | 47 +++++++++++++++++++++++++++++++++++++++++++---- > monitor.c | 8 ++++++-- > qemu-doc.texi | 11 +++++++++++ > 4 files changed, 61 insertions(+), 6 deletions(-) > > diff --git a/console.h b/console.h > index c7f29f5..041b4ef 100644 > --- a/console.h > +++ b/console.h > @@ -159,6 +159,7 @@ extern uint8_t _translate_keycode(const int key); > does not need to include console.h */ > /* monitor.c */ > void monitor_init(CharDriverState *hd, int show_banner); > +void monitor_handle_command(const char *cmdline); > void term_puts(const char *str); > void term_vprintf(const char *fmt, va_list ap); > void term_printf(const char *fmt, ...) __attribute__ ((__format__ (__printf__, 1, 2))); > diff --git a/gdbstub.c b/gdbstub.c > index 4bc7999..833cdd9 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -33,6 +33,7 @@ > #include "qemu-char.h" > #include "sysemu.h" > #include "gdbstub.h" > +#include "console.h" > #endif > > #include "qemu_socket.h" > @@ -70,6 +71,8 @@ typedef struct GDBState { > int running_state; > #else > CharDriverState *chr; > + CharDriverState *mon; > + int allow_monitor; > #endif > } GDBState; > > @@ -239,10 +242,11 @@ static int put_packet(GDBState *s, char *buf) > return 0; > } > > -static void monitor_output(GDBState *s, const char *msg) > +static void monitor_output_len(GDBState *s, const char *msg, int len) > { > char *buf; > - int len = strlen(msg); > + if (!len) > + len = strlen(msg); > > buf = malloc(len * 2 + 2); > buf[0] = 'O'; > @@ -251,6 +255,11 @@ static void monitor_output(GDBState *s, const char *msg) > free(buf); > } > > +static void monitor_output(GDBState *s, const char *msg) > +{ > + monitor_output_len(s, msg, 0); > +} > + If you change the zero argument into a strlen(msg) you can drop the check in monitor_output_len(). > static void monitor_help(GDBState *s) > { > monitor_output(s, "gdbstub specific monitor commands:\n"); > @@ -258,6 +267,7 @@ static void monitor_help(GDBState *s) > monitor_output(s, "set s_step <0|1> -- Single Stepping enabled\n"); > monitor_output(s, "set s_irq <0|1> -- Single Stepping with qemu irq handlers enabled\n"); > monitor_output(s, "set s_timer <0|1> -- Single Stepping with qemu timers enabled\n"); > + monitor_output(s, "qemu monitor pass-through commands:\n"); > } You might want to consider not showing that last line for CONFIG_USER_ONLY. Best regards -- Edgar E. Iglesias Axis Communications AB ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support 2008-05-15 22:17 ` [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support Edgar E. Iglesias @ 2008-05-19 13:29 ` Jason Wessel 2008-05-19 16:30 ` Paul Brook 2008-05-21 12:58 ` Maxim Gorbachyov 0 siblings, 2 replies; 20+ messages in thread From: Jason Wessel @ 2008-05-19 13:29 UTC (permalink / raw) To: Edgar E. Iglesias; +Cc: qemu-devel [-- Attachment #1: Type: text/plain, Size: 1227 bytes --] Edgar E. Iglesias wrote: > On Thu, May 15, 2008 at 09:11:30AM -0500, Jason Wessel wrote: > >> This patch adds a feature to the gdbstub to allow gdb to issue monitor >> commands that can pass-through to the qemu monitor. >> >> >> >> +static void monitor_output(GDBState *s, const char *msg) >> +{ >> + monitor_output_len(s, msg, 0); >> +} >> + >> > > > If you change the zero argument into a strlen(msg) you can drop the check in monitor_output_len(). > > > Sure. That seems reasonable. >> static void monitor_help(GDBState *s) >> { >> monitor_output(s, "gdbstub specific monitor commands:\n"); >> @@ -258,6 +267,7 @@ static void monitor_help(GDBState *s) >> monitor_output(s, "set s_step <0|1> -- Single Stepping enabled\n"); >> monitor_output(s, "set s_irq <0|1> -- Single Stepping with qemu irq handlers enabled\n"); >> monitor_output(s, "set s_timer <0|1> -- Single Stepping with qemu timers enabled\n"); >> + monitor_output(s, "qemu monitor pass-through commands:\n"); >> } >> > > Seems reasonable too, see newly attached patch. Obviously you need the new monitor patch applied prior to this one. This patch was updated against the prior patch. Thanks, Jason. [-- Attachment #2: gdb_monitor_plus_qemu_monitor.patch --] [-- Type: text/x-diff, Size: 6329 bytes --] From: Jason Wessel <jason.wessel@windriver.com> Subject: [PATCH] gdbstub: gdb pass-through qemu monitor support This patch adds a feature to the gdbstub to allow gdb to issue monitor commands that can pass-through to the qemu monitor. In order to make this work, the MAX_MON (the maximum number of monitor connections) had to get incremented by 1 to support the case when the gdbstub is setup along with all the other connections. A small check was added avoid strange crashes when you allocate too many qemu monitor connections. The monitor_handle_command had to be exported in order to allow gdbstub to pass the strings directly to the monitor. The gdbstub registers as an output device of the qemu monitor such that no further changes were needed to the monitor other than to have a global variable in the gdbstub that controls when to transmit data from a pass through monitor command to an attached debugger. The qemu docs were updated to reflect this change. Signed-off-by: Jason Wessel <jason.wessel@windriver.com> --- console.h | 1 + gdbstub.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- monitor.c | 8 ++++++-- qemu-doc.texi | 11 +++++++++++ 4 files changed, 74 insertions(+), 4 deletions(-) --- a/gdbstub.c +++ b/gdbstub.c @@ -33,6 +33,7 @@ #include "qemu-char.h" #include "sysemu.h" #include "gdbstub.h" +#include "console.h" #endif #include "qemu_socket.h" @@ -71,6 +72,8 @@ typedef struct GDBState { int running_state; #else CharDriverState *chr; + CharDriverState *mon; + int allow_monitor; #endif } GDBState; @@ -260,6 +263,11 @@ static void monitor_output(GDBState *s, put_packet_hex(s, msg, strlen(msg), 1); } +static void monitor_output_len(GDBState *s, const char *msg, int len) +{ + put_packet_hex(s, msg, len, 1); +} + static void monitor_help(GDBState *s) { monitor_output(s, "gdbstub specific monitor commands:\n"); @@ -267,6 +275,10 @@ static void monitor_help(GDBState *s) monitor_output(s, "set sstep <0|1> -- Single Stepping enabled\n"); monitor_output(s, "set sirq <0|1> -- Single Stepping with qemu irq handlers enabled\n"); monitor_output(s, "set stimers <0|1> -- Single Stepping with qemu timers enabled\n"); +#ifndef CONFIG_USER_ONLY + monitor_output(s, "qemu monitor pass-through commands:\n"); +#endif + } #if defined(TARGET_I386) @@ -1005,8 +1017,14 @@ static void gdb_rcmd(GDBState *s, const sstep_flags &= ~SSTEP_NOTIMER; } else if (!strcmp(mem_buf, "set stimers 0")) { sstep_flags |= SSTEP_NOTIMER; - } else if (!strcmp(mem_buf, "help") || !strcmp(mem_buf, "?")) { - monitor_help(s); + } else { + if (!strcmp(mem_buf, "help") || !strcmp(mem_buf, "?")) + monitor_help(s); +#ifndef CONFIG_USER_ONLY + s->allow_monitor = 1; + monitor_handle_command(mem_buf); + s->allow_monitor = 0; +#endif } put_packet(s, "OK"); } @@ -1572,6 +1590,39 @@ static void gdb_chr_event(void *opaque, } } +static int gdb_mon_chr_write(CharDriverState *chr, const uint8_t *buf, int len) +{ + int max_sz; + const uint8_t *p = buf; + GDBState *s = chr->opaque; + + if (s->allow_monitor) { + max_sz = (sizeof(s->last_packet) - 2) / 2; + for (;;) { + if (len <= max_sz) { + monitor_output_len(s, p, len); + break; + } + monitor_output_len(s, p, max_sz); + p += max_sz; + len -= max_sz; + } + } + return len; +} + +static CharDriverState *qemu_chr_open_gdb_mon(GDBState *s) +{ + CharDriverState *chr; + + chr = qemu_mallocz(sizeof(CharDriverState)); + if (!chr) + return NULL; + chr->chr_write = gdb_mon_chr_write; + chr->opaque = s; + return chr; +} + int gdbserver_start(const char *port) { GDBState *s; @@ -1604,6 +1655,9 @@ int gdbserver_start(const char *port) qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive, gdb_chr_event, s); qemu_add_vm_stop_handler(gdb_vm_stopped, s); + /* Initialize a monitor port for gdb */ + s->mon = qemu_chr_open_gdb_mon(s); + monitor_init(s->mon, 0); return 0; } #endif --- a/monitor.c +++ b/monitor.c @@ -69,7 +69,7 @@ typedef struct term_cmd_t { const char *help; } term_cmd_t; -#define MAX_MON 4 +#define MAX_MON 5 static CharDriverState *monitor_hd[MAX_MON]; static int hide_banner; @@ -2096,7 +2096,7 @@ static int default_fmt_size = 4; #define MAX_ARGS 16 -static void monitor_handle_command(const char *cmdline) +void monitor_handle_command(const char *cmdline) { const char *p, *pstart, *typestr; char *q; @@ -2649,6 +2649,10 @@ void monitor_init(CharDriverState *hd, i break; } } + if (i >= MAX_MON) { + fprintf(stderr, "ERROR too many monitor connections requested\n"); + exit(0); + } hide_banner = !show_banner; --- a/console.h +++ b/console.h @@ -159,6 +159,7 @@ extern uint8_t _translate_keycode(const does not need to include console.h */ /* monitor.c */ void monitor_init(CharDriverState *hd, int show_banner); +void monitor_handle_command(const char *cmdline); void term_puts(const char *str); void term_vprintf(const char *fmt, va_list ap); void term_printf(const char *fmt, ...) __attribute__ ((__format__ (__printf__, 1, 2))); --- a/qemu-doc.texi +++ b/qemu-doc.texi @@ -1979,6 +1979,17 @@ Turn off or on the the irq processing wh Turn off or on the the timer processing when single stepping, which is defaulted to off. @end table +You may also use gdb to send pass-through monitor commands to the qemu monitor. It is easiest to start off with using just using @code{monitor help} to display the list of commands in available from the monitor. Then you can prefix the @code{monitor} key word prior to any command. The example below shows the use of the qemu monitor's @code{info pci} command. + +@example +(gdb) monitor info pci + Bus 0, device 0, function 0: + Host bridge: PCI device 1057:4801 + Bus 0, device 1, function 0: + VGA controller: PCI device 1234:1111 + BAR0: 32 bit memory at 0xf0000000 [0xf07fffff]. +@end example + @node pcsys_os_specific @section Target OS specific information ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support 2008-05-19 13:29 ` Jason Wessel @ 2008-05-19 16:30 ` Paul Brook 2008-05-21 12:58 ` Maxim Gorbachyov 1 sibling, 0 replies; 20+ messages in thread From: Paul Brook @ 2008-05-19 16:30 UTC (permalink / raw) To: qemu-devel; +Cc: Edgar E. Iglesias On Monday 19 May 2008, Jason Wessel wrote: > Edgar E. Iglesias wrote: > > On Thu, May 15, 2008 at 09:11:30AM -0500, Jason Wessel wrote: > >> This patch adds a feature to the gdbstub to allow gdb to issue monitor > >> commands that can pass-through to the qemu monitor. It seems rather silly to have both gdbstub qRcmd and the monitor interface support a different set of commands. I see no reason why the singlestep control should be gdbstub specific, rather than just annother monitor command. Paul ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support 2008-05-19 13:29 ` Jason Wessel 2008-05-19 16:30 ` Paul Brook @ 2008-05-21 12:58 ` Maxim Gorbachyov 2008-05-21 17:03 ` Jason Wessel 1 sibling, 1 reply; 20+ messages in thread From: Maxim Gorbachyov @ 2008-05-21 12:58 UTC (permalink / raw) To: qemu-devel On Mon, May 19, 2008 at 5:29 PM, Jason Wessel <jason.wessel@windriver.com> wrote: [cut] > > Seems reasonable too, see newly attached patch. > > Obviously you need the new monitor patch applied prior to this one. > This patch was updated against the prior patch. It was noticed [by Jan Kiszka and me] that the output on the monitor console goes both to the local as well as the remote one - but only if you issue a command via gdb. When you type in the command locally (ie. on the qemu side), that output is not forwarded to the gdb frontend. Is this behavior desired? ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support 2008-05-21 12:58 ` Maxim Gorbachyov @ 2008-05-21 17:03 ` Jason Wessel 2008-05-22 13:24 ` [Qemu-devel] " Jan Kiszka 0 siblings, 1 reply; 20+ messages in thread From: Jason Wessel @ 2008-05-21 17:03 UTC (permalink / raw) To: qemu-devel Maxim Gorbachyov wrote: > On Mon, May 19, 2008 at 5:29 PM, Jason Wessel > <jason.wessel@windriver.com> wrote: > > [cut] > >> Seems reasonable too, see newly attached patch. >> >> Obviously you need the new monitor patch applied prior to this one. >> This patch was updated against the prior patch. >> > > It was noticed [by Jan Kiszka and me] that the output on the monitor > console goes both to the local as well as the remote one - but only if > you issue a command via gdb. When you type in the command locally (ie. > on the qemu side), that output is not forwarded to the gdb frontend. > Is this behavior desired? > > > That was certainly the intended behavior when I created the patch. There is no reason to feed the monitor output up to the debugger that may or may not be attached. The behavior could get altered to check if the debugger is in the "continue" state and then send the information from the monitor, but it seemed of little value to do so. Jason. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support 2008-05-21 17:03 ` Jason Wessel @ 2008-05-22 13:24 ` Jan Kiszka 2008-05-22 13:45 ` Jason Wessel 0 siblings, 1 reply; 20+ messages in thread From: Jan Kiszka @ 2008-05-22 13:24 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: text/plain, Size: 1217 bytes --] Jason Wessel wrote: > Maxim Gorbachyov wrote: >> On Mon, May 19, 2008 at 5:29 PM, Jason Wessel >> <jason.wessel@windriver.com> wrote: >> >> [cut] >> >>> Seems reasonable too, see newly attached patch. >>> >>> Obviously you need the new monitor patch applied prior to this one. >>> This patch was updated against the prior patch. >>> >> It was noticed [by Jan Kiszka and me] that the output on the monitor >> console goes both to the local as well as the remote one - but only if >> you issue a command via gdb. When you type in the command locally (ie. >> on the qemu side), that output is not forwarded to the gdb frontend. >> Is this behavior desired? >> >> >> > > That was certainly the intended behavior when I created the patch. > There is no reason to feed the monitor output up to the debugger that > may or may not be attached. That's not what we were confused by. > The behavior could get altered to check if > the debugger is in the "continue" state and then send the information > from the monitor, but it seemed of little value to do so. Rather the contrary: What is the point of mirroring the output you see in gdb to the local monitor console? Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 254 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support 2008-05-22 13:24 ` [Qemu-devel] " Jan Kiszka @ 2008-05-22 13:45 ` Jason Wessel 2008-05-22 13:53 ` Jan Kiszka 0 siblings, 1 reply; 20+ messages in thread From: Jason Wessel @ 2008-05-22 13:45 UTC (permalink / raw) To: qemu-devel Jan Kiszka wrote: > Jason Wessel wrote: > >> Maxim Gorbachyov wrote: >> >>> On Mon, May 19, 2008 at 5:29 PM, Jason Wessel >>> <jason.wessel@windriver.com> wrote: >>> >>> [cut] >>> >>> >>>> Seems reasonable too, see newly attached patch. >>>> >>>> Obviously you need the new monitor patch applied prior to this one. >>>> This patch was updated against the prior patch. >>>> >>>> >>> It was noticed [by Jan Kiszka and me] that the output on the monitor >>> console goes both to the local as well as the remote one - but only if >>> you issue a command via gdb. When you type in the command locally (ie. >>> on the qemu side), that output is not forwarded to the gdb frontend. >>> Is this behavior desired? >>> >>> >>> >>> >> That was certainly the intended behavior when I created the patch. >> There is no reason to feed the monitor output up to the debugger that >> may or may not be attached. >> > > That's not what we were confused by. > > >> The behavior could get altered to check if >> the debugger is in the "continue" state and then send the information >> from the monitor, but it seemed of little value to do so. >> > > Rather the contrary: What is the point of mirroring the output you see > in gdb to the local monitor console? > > That happens to have little to do with the gdb implementation. It is a function of the monitor mux. The monitor mux always writes out to all the output channels automatically. In the case of gdb sending input for which you only want to receive the response it would be plausible to add a "focus" parameter to the monitor mux code to indicate which "channel" to send the output to. The monitor mux could also be changed to simply shift the output focus to the most recent place it received an input, and if the focus was -1 to start it would broadcast every where, the way it does today. It is a trivial enough change, if this is what you are looking for. Jason. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support 2008-05-22 13:45 ` Jason Wessel @ 2008-05-22 13:53 ` Jan Kiszka 2008-05-22 16:55 ` Jason Wessel 0 siblings, 1 reply; 20+ messages in thread From: Jan Kiszka @ 2008-05-22 13:53 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: text/plain, Size: 2133 bytes --] Jason Wessel wrote: > Jan Kiszka wrote: >> Jason Wessel wrote: >> >>> Maxim Gorbachyov wrote: >>> >>>> On Mon, May 19, 2008 at 5:29 PM, Jason Wessel >>>> <jason.wessel@windriver.com> wrote: >>>> >>>> [cut] >>>> >>>> >>>>> Seems reasonable too, see newly attached patch. >>>>> >>>>> Obviously you need the new monitor patch applied prior to this one. >>>>> This patch was updated against the prior patch. >>>>> >>>>> >>>> It was noticed [by Jan Kiszka and me] that the output on the monitor >>>> console goes both to the local as well as the remote one - but only if >>>> you issue a command via gdb. When you type in the command locally (ie. >>>> on the qemu side), that output is not forwarded to the gdb frontend. >>>> Is this behavior desired? >>>> >>>> >>>> >>>> >>> That was certainly the intended behavior when I created the patch. >>> There is no reason to feed the monitor output up to the debugger that >>> may or may not be attached. >>> >> That's not what we were confused by. >> >> >>> The behavior could get altered to check if >>> the debugger is in the "continue" state and then send the information >>> from the monitor, but it seemed of little value to do so. >>> >> Rather the contrary: What is the point of mirroring the output you see >> in gdb to the local monitor console? >> >> > > That happens to have little to do with the gdb implementation. It is a > function of the monitor mux. The monitor mux always writes out to all > the output channels automatically. In the case of gdb sending input > for which you only want to receive the response it would be plausible to > add a "focus" parameter to the monitor mux code to indicate which > "channel" to send the output to. > > The monitor mux could also be changed to simply shift the output focus > to the most recent place it received an input, and if the focus was -1 > to start it would broadcast every where, the way it does today. It is a > trivial enough change, if this is what you are looking for. Yep, would be nice. Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 254 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support 2008-05-22 13:53 ` Jan Kiszka @ 2008-05-22 16:55 ` Jason Wessel 2008-05-23 15:33 ` Jan Kiszka 0 siblings, 1 reply; 20+ messages in thread From: Jason Wessel @ 2008-05-22 16:55 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: text/plain, Size: 2723 bytes --] Jan Kiszka wrote: > Jason Wessel wrote: > >> Jan Kiszka wrote: >> >>> Jason Wessel wrote: >>> >>> >>>> Maxim Gorbachyov wrote: >>>> >>>> >>>>> On Mon, May 19, 2008 at 5:29 PM, Jason Wessel >>>>> <jason.wessel@windriver.com> wrote: >>>>> >>>>> [cut] >>>>> >>>>> >>>>> >>>>>> Seems reasonable too, see newly attached patch. >>>>>> >>>>>> Obviously you need the new monitor patch applied prior to this one. >>>>>> This patch was updated against the prior patch. >>>>>> >>>>>> >>>>>> >>>>> It was noticed [by Jan Kiszka and me] that the output on the monitor >>>>> console goes both to the local as well as the remote one - but only if >>>>> you issue a command via gdb. When you type in the command locally (ie. >>>>> on the qemu side), that output is not forwarded to the gdb frontend. >>>>> Is this behavior desired? >>>>> >>>>> >>>>> >>>>> >>>>> >>>> That was certainly the intended behavior when I created the patch. >>>> There is no reason to feed the monitor output up to the debugger that >>>> may or may not be attached. >>>> >>>> >>> That's not what we were confused by. >>> >>> >>> >>>> The behavior could get altered to check if >>>> the debugger is in the "continue" state and then send the information >>>> from the monitor, but it seemed of little value to do so. >>>> >>>> >>> Rather the contrary: What is the point of mirroring the output you see >>> in gdb to the local monitor console? >>> >>> >>> >> That happens to have little to do with the gdb implementation. It is a >> function of the monitor mux. The monitor mux always writes out to all >> the output channels automatically. In the case of gdb sending input >> for which you only want to receive the response it would be plausible to >> add a "focus" parameter to the monitor mux code to indicate which >> "channel" to send the output to. >> >> The monitor mux could also be changed to simply shift the output focus >> to the most recent place it received an input, and if the focus was -1 >> to start it would broadcast every where, the way it does today. It is a >> trivial enough change, if this is what you are looking for. >> > > Yep, would be nice. > > Jan > > Jan, See the newly attached patches which address the problem you raised. I removed the monitor broadcast code entirely and replaced it with the "mon_active" because it is not likely that there will ever be two exactly concurrent users of the qemu monitor. You either use it from one place or another at a time. Apply in order: gdb_single_step_monitor_cmd.patch gdb_monitor_plus_qemu_monitor.patch Cheers, Jason. [-- Attachment #2: gdb_single_step_monitor_cmd.patch --] [-- Type: text/x-diff, Size: 8148 bytes --] From: Jason Wessel <jason.wessel@windriver.com> Subject: [PATCH] gdbstub: replace singlestep q packets with qRcmd packets At the gdbserial protocol level, using the qRcmd packets allows gdb to use the "monitor" command to access the controls for single stepping behavior. Now you can use a gdb "monitor" command instead of a gdb "maintenance" command. The qemu docs were updated to reflect this change. Signed-off-by: Jason Wessel <jason.wessel@windriver.com> --- gdbstub.c | 100 +++++++++++++++++++++++++++++++++++++++++----------------- qemu-doc.texi | 36 ++++++++++---------- 2 files changed, 89 insertions(+), 47 deletions(-) --- a/gdbstub.c +++ b/gdbstub.c @@ -205,9 +205,9 @@ static void hextomem(uint8_t *mem, const } /* return -1 if error, 0 if OK */ -static int put_packet(GDBState *s, char *buf) +static int put_packet_hex(GDBState *s, const char *buf, int len, int isHex) { - int len, csum, i; + int csum, i; uint8_t *p; #ifdef DEBUG_GDB @@ -217,13 +217,19 @@ static int put_packet(GDBState *s, char for(;;) { p = s->last_packet; *(p++) = '$'; - len = strlen(buf); - memcpy(p, buf, len); - p += len; + if (isHex) { + p[0] = 'O'; + memtohex(p + 1, buf, len); + len = strlen(p); + } else { + memcpy(p, buf, len); + } + csum = 0; for(i = 0; i < len; i++) { - csum += buf[i]; + csum += p[i]; } + p += len; *(p++) = '#'; *(p++) = tohex((csum >> 4) & 0xf); *(p++) = tohex((csum) & 0xf); @@ -244,6 +250,25 @@ static int put_packet(GDBState *s, char return 0; } +/* return -1 if error, 0 if OK */ +static int put_packet(GDBState *s, const char *buf) { + return put_packet_hex(s, buf, strlen(buf), 0); +} + +static void monitor_output(GDBState *s, const char *msg) +{ + put_packet_hex(s, msg, strlen(msg), 1); +} + +static void monitor_help(GDBState *s) +{ + monitor_output(s, "gdbstub specific monitor commands:\n"); + monitor_output(s, "show <sstep|sirq|stimer> -- Show gdbstub Single Stepping variables\n"); + monitor_output(s, "set sstep <0|1> -- Single Stepping enabled\n"); + monitor_output(s, "set sirq <0|1> -- Single Stepping with qemu irq handlers enabled\n"); + monitor_output(s, "set stimers <0|1> -- Single Stepping with qemu timers enabled\n"); +} + #if defined(TARGET_I386) #ifdef TARGET_X86_64 @@ -948,6 +973,44 @@ static void cpu_gdb_write_registers(CPUS #endif +static void gdb_rcmd(GDBState *s, const char *p, char *buf, char *mem_buf) +{ + int len = strlen(p); + + if ((len % 2) != 0) { + put_packet(s, "E01"); + return; + } + hextomem(mem_buf, p, len); + mem_buf[(len >> 1)] = 0; + + if (!strcmp(mem_buf, "show sstep")) { + sprintf(buf, "sstep == %i\n", (sstep_flags & SSTEP_ENABLE) != 0); + monitor_output(s, buf); + } else if (!strcmp(mem_buf, "show sirq")) { + sprintf(buf, "sirq == %i\n", (sstep_flags & SSTEP_NOIRQ) == 0); + monitor_output(s, buf); + } else if (!strcmp(mem_buf, "show stimers")) { + sprintf(buf, "stimers == %i\n", (sstep_flags & SSTEP_NOTIMER) == 0); + monitor_output(s, buf); + } else if (!strcmp(mem_buf, "set sstep 1")) { + sstep_flags |= SSTEP_ENABLE; + } else if (!strcmp(mem_buf, "set step 0")) { + sstep_flags &= ~SSTEP_ENABLE; + } else if (!strcmp(mem_buf, "set sirq 1")) { + sstep_flags &= ~SSTEP_NOIRQ; + } else if (!strcmp(mem_buf, "set sirq 0")) { + sstep_flags |= SSTEP_NOIRQ; + } else if (!strcmp(mem_buf, "set stimers 1")) { + sstep_flags &= ~SSTEP_NOTIMER; + } else if (!strcmp(mem_buf, "set stimers 0")) { + sstep_flags |= SSTEP_NOTIMER; + } else if (!strcmp(mem_buf, "help") || !strcmp(mem_buf, "?")) { + monitor_help(s); + } + put_packet(s, "OK"); +} + static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf) { const char *p; @@ -1139,29 +1202,8 @@ static int gdb_handle_packet(GDBState *s } 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"); + if (!strncmp(p, "Rcmd,", 5)) { + gdb_rcmd(s, p + 5, buf, mem_buf); break; } #ifdef CONFIG_LINUX_USER --- a/qemu-doc.texi +++ b/qemu-doc.texi @@ -1951,32 +1951,32 @@ Use @code{set architecture i8086} to dum 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: +The default single stepping behavior is to 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 execute. Because there are rare circumstances where you want to single step into an interrupt vector the behavior can be controlled from GDB. There are several commands you use to query and set the single step behavior while inside gdb: @table @code -@item maintenance packet qqemu.sstepbits +@item monitor show <sstep|sirq|stimers> -This will display the MASK bits used to control the single stepping IE: +This will display the values of the single stepping controls IE: @example -(gdb) maintenance packet qqemu.sstepbits -sending: "qqemu.sstepbits" -received: "ENABLE=1,NOIRQ=2,NOTIMER=4" +(gdb) monitor show sstep +sstep == 1 +(gdb) monitor show sirq +sirq == 0 +(gdb) monitor show stimers +stimers == 0 @end example -@item maintenance packet qqemu.sstep +@item monitor set sstep <0|1> -This will display the current value of the mask used when single stepping IE: +Turn off(0) or on(1) the single stepping feature all together, which is defaulted to on. @example -(gdb) maintenance packet qqemu.sstep -sending: "qqemu.sstep" -received: "0x7" +(gdb) monitor set sstep 0 +(gdb) monitor set sstep 1 @end example -@item maintenance packet Qqemu.sstep=HEX_VALUE +@item monitor set sirq <0|1> -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 +Turn off or on the the irq processing when single stepping, which is defaulted to off. +@item monitor set stimers <0|1> + +Turn off or on the the timer processing when single stepping, which is defaulted to off. @end table @node pcsys_os_specific [-- Attachment #3: gdb_monitor_plus_qemu_monitor.patch --] [-- Type: text/x-diff, Size: 8778 bytes --] From: Jason Wessel <jason.wessel@windriver.com> Subject: [PATCH] gdbstub: gdb pass-through qemu monitor support This patch adds a feature to the gdbstub to allow gdb to issue monitor commands that can pass-through to the qemu monitor. This patch also changes the qemu monitor such that it only sends output to a monitor instance that sends an input. In order to make this work, the MAX_MON (the maximum number of monitor connections) had to get incremented by 1 to support the case when the gdbstub is setup along with all the other connections. A small check was added avoid strange crashes when you allocate too many qemu monitor connections. The mon_active variable represents the current output channel for any output that comes from the qemu monitor. A new exported function called monitor_handle_ext_command had to be exported in order to allow gdbstub to pass the strings directly to the monitor. The gdbstub registers as an output device of the qemu monitor such that no further changes were needed to the monitor other than to have a global variable in the gdbstub that controls when to transmit data from a pass through monitor command to an attached debugger. The qemu docs were updated to reflect this change. Signed-off-by: Jason Wessel <jason.wessel@windriver.com> --- console.h | 1 + gdbstub.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- monitor.c | 38 +++++++++++++++++++++++--------------- qemu-doc.texi | 11 +++++++++++ 4 files changed, 91 insertions(+), 17 deletions(-) --- a/gdbstub.c +++ b/gdbstub.c @@ -33,6 +33,7 @@ #include "qemu-char.h" #include "sysemu.h" #include "gdbstub.h" +#include "console.h" #endif #include "qemu_socket.h" @@ -71,6 +72,8 @@ typedef struct GDBState { int running_state; #else CharDriverState *chr; + CharDriverState *mon; + int allow_monitor; #endif } GDBState; @@ -260,6 +263,11 @@ static void monitor_output(GDBState *s, put_packet_hex(s, msg, strlen(msg), 1); } +static void monitor_output_len(GDBState *s, const char *msg, int len) +{ + put_packet_hex(s, msg, len, 1); +} + static void monitor_help(GDBState *s) { monitor_output(s, "gdbstub specific monitor commands:\n"); @@ -267,6 +275,10 @@ static void monitor_help(GDBState *s) monitor_output(s, "set sstep <0|1> -- Single Stepping enabled\n"); monitor_output(s, "set sirq <0|1> -- Single Stepping with qemu irq handlers enabled\n"); monitor_output(s, "set stimers <0|1> -- Single Stepping with qemu timers enabled\n"); +#ifndef CONFIG_USER_ONLY + monitor_output(s, "qemu monitor pass-through commands:\n"); +#endif + } #if defined(TARGET_I386) @@ -1005,8 +1017,14 @@ static void gdb_rcmd(GDBState *s, const sstep_flags &= ~SSTEP_NOTIMER; } else if (!strcmp(mem_buf, "set stimers 0")) { sstep_flags |= SSTEP_NOTIMER; - } else if (!strcmp(mem_buf, "help") || !strcmp(mem_buf, "?")) { - monitor_help(s); + } else { + if (!strcmp(mem_buf, "help") || !strcmp(mem_buf, "?")) + monitor_help(s); +#ifndef CONFIG_USER_ONLY + s->allow_monitor = 1; + monitor_handle_ext_command(s->mon, mem_buf); + s->allow_monitor = 0; +#endif } put_packet(s, "OK"); } @@ -1572,6 +1590,39 @@ static void gdb_chr_event(void *opaque, } } +static int gdb_mon_chr_write(CharDriverState *chr, const uint8_t *buf, int len) +{ + int max_sz; + const uint8_t *p = buf; + GDBState *s = chr->opaque; + + if (s->allow_monitor) { + max_sz = (sizeof(s->last_packet) - 2) / 2; + for (;;) { + if (len <= max_sz) { + monitor_output_len(s, p, len); + break; + } + monitor_output_len(s, p, max_sz); + p += max_sz; + len -= max_sz; + } + } + return len; +} + +static CharDriverState *qemu_chr_open_gdb_mon(GDBState *s) +{ + CharDriverState *chr; + + chr = qemu_mallocz(sizeof(CharDriverState)); + if (!chr) + return NULL; + chr->chr_write = gdb_mon_chr_write; + chr->opaque = s; + return chr; +} + int gdbserver_start(const char *port) { GDBState *s; @@ -1604,6 +1655,9 @@ int gdbserver_start(const char *port) qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive, gdb_chr_event, s); qemu_add_vm_stop_handler(gdb_vm_stopped, s); + /* Initialize a monitor port for gdb */ + s->mon = qemu_chr_open_gdb_mon(s); + monitor_init(s->mon, 0); return 0; } #endif --- a/monitor.c +++ b/monitor.c @@ -69,8 +69,9 @@ typedef struct term_cmd_t { const char *help; } term_cmd_t; -#define MAX_MON 4 +#define MAX_MON 5 static CharDriverState *monitor_hd[MAX_MON]; +static CharDriverState *mon_active; static int hide_banner; static term_cmd_t term_cmds[]; @@ -85,13 +86,11 @@ CPUState *mon_cpu = NULL; void term_flush(void) { - int i; - if (term_outbuf_index > 0) { - for (i = 0; i < MAX_MON; i++) - if (monitor_hd[i] && monitor_hd[i]->focus == 0) - qemu_chr_write(monitor_hd[i], term_outbuf, term_outbuf_index); - term_outbuf_index = 0; - } + if (term_outbuf_index <= 0) + return; + if (mon_active) + qemu_chr_write(mon_active, term_outbuf, term_outbuf_index); + term_outbuf_index = 0; } /* flush at every end of line or if the buffer is full */ @@ -2603,12 +2602,19 @@ static int term_can_read(void *opaque) static void term_read(void *opaque, const uint8_t *buf, int size) { int i; + mon_active = opaque; for(i = 0; i < size; i++) readline_handle_byte(buf[i]); } static void monitor_start_input(void); +void monitor_handle_ext_command(void *opaque, const char *cmdline) +{ + mon_active = opaque; + monitor_handle_command(cmdline); +} + static void monitor_handle_command1(void *opaque, const char *cmdline) { monitor_handle_command(cmdline); @@ -2625,6 +2631,7 @@ static void term_event(void *opaque, int if (event != CHR_EVENT_RESET) return; + mon_active = opaque; if (!hide_banner) term_printf("QEMU %s monitor - type 'help' for more information\n", QEMU_VERSION); @@ -2646,13 +2653,18 @@ void monitor_init(CharDriverState *hd, i for (i = 0; i < MAX_MON; i++) { if (monitor_hd[i] == NULL) { monitor_hd[i] = hd; + mon_active = hd; break; } } + if (i >= MAX_MON) { + fprintf(stderr, "ERROR too many monitor connections requested\n"); + exit(0); + } hide_banner = !show_banner; - qemu_chr_add_handlers(hd, term_can_read, term_read, term_event, NULL); + qemu_chr_add_handlers(hd, term_can_read, term_read, term_event, hd); readline_start("", 0, monitor_handle_command1, NULL); } @@ -2672,12 +2684,8 @@ static void monitor_readline_cb(void *op void monitor_readline(const char *prompt, int is_password, char *buf, int buf_size) { - int i; - - if (is_password) { - for (i = 0; i < MAX_MON; i++) - if (monitor_hd[i] && monitor_hd[i]->focus == 0) - qemu_chr_send_event(monitor_hd[i], CHR_EVENT_FOCUS); + if (is_password && mon_active) { + qemu_chr_send_event(mon_active, CHR_EVENT_FOCUS); } readline_start(prompt, is_password, monitor_readline_cb, NULL); monitor_readline_buf = buf; --- a/console.h +++ b/console.h @@ -159,6 +159,7 @@ extern uint8_t _translate_keycode(const does not need to include console.h */ /* monitor.c */ void monitor_init(CharDriverState *hd, int show_banner); +void monitor_handle_ext_command(void *opaque, const char *cmdline); void term_puts(const char *str); void term_vprintf(const char *fmt, va_list ap); void term_printf(const char *fmt, ...) __attribute__ ((__format__ (__printf__, 1, 2))); --- a/qemu-doc.texi +++ b/qemu-doc.texi @@ -1979,6 +1979,17 @@ Turn off or on the the irq processing wh Turn off or on the the timer processing when single stepping, which is defaulted to off. @end table +You may also use gdb to send pass-through monitor commands to the qemu monitor. It is easiest to start off with using just using @code{monitor help} to display the list of commands in available from the monitor. Then you can prefix the @code{monitor} key word prior to any command. The example below shows the use of the qemu monitor's @code{info pci} command. + +@example +(gdb) monitor info pci + Bus 0, device 0, function 0: + Host bridge: PCI device 1057:4801 + Bus 0, device 1, function 0: + VGA controller: PCI device 1234:1111 + BAR0: 32 bit memory at 0xf0000000 [0xf07fffff]. +@end example + @node pcsys_os_specific @section Target OS specific information ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support 2008-05-22 16:55 ` Jason Wessel @ 2008-05-23 15:33 ` Jan Kiszka 0 siblings, 0 replies; 20+ messages in thread From: Jan Kiszka @ 2008-05-23 15:33 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: text/plain, Size: 2943 bytes --] Jason Wessel wrote: > Jan Kiszka wrote: >> Jason Wessel wrote: >> >>> Jan Kiszka wrote: >>> >>>> Jason Wessel wrote: >>>> >>>> >>>>> Maxim Gorbachyov wrote: >>>>> >>>>> >>>>>> On Mon, May 19, 2008 at 5:29 PM, Jason Wessel >>>>>> <jason.wessel@windriver.com> wrote: >>>>>> >>>>>> [cut] >>>>>> >>>>>> >>>>>> >>>>>>> Seems reasonable too, see newly attached patch. >>>>>>> >>>>>>> Obviously you need the new monitor patch applied prior to this one. >>>>>>> This patch was updated against the prior patch. >>>>>>> >>>>>>> >>>>>>> >>>>>> It was noticed [by Jan Kiszka and me] that the output on the monitor >>>>>> console goes both to the local as well as the remote one - but only if >>>>>> you issue a command via gdb. When you type in the command locally (ie. >>>>>> on the qemu side), that output is not forwarded to the gdb frontend. >>>>>> Is this behavior desired? >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>> That was certainly the intended behavior when I created the patch. >>>>> There is no reason to feed the monitor output up to the debugger that >>>>> may or may not be attached. >>>>> >>>>> >>>> That's not what we were confused by. >>>> >>>> >>>> >>>>> The behavior could get altered to check if >>>>> the debugger is in the "continue" state and then send the information >>>>> from the monitor, but it seemed of little value to do so. >>>>> >>>>> >>>> Rather the contrary: What is the point of mirroring the output you see >>>> in gdb to the local monitor console? >>>> >>>> >>>> >>> That happens to have little to do with the gdb implementation. It is a >>> function of the monitor mux. The monitor mux always writes out to all >>> the output channels automatically. In the case of gdb sending input >>> for which you only want to receive the response it would be plausible to >>> add a "focus" parameter to the monitor mux code to indicate which >>> "channel" to send the output to. >>> >>> The monitor mux could also be changed to simply shift the output focus >>> to the most recent place it received an input, and if the focus was -1 >>> to start it would broadcast every where, the way it does today. It is a >>> trivial enough change, if this is what you are looking for. >>> >> Yep, would be nice. >> >> Jan >> >> > > Jan, > > See the newly attached patches which address the problem you raised. I > removed the monitor broadcast code entirely and replaced it with the > "mon_active" because it is not likely that there will ever be two > exactly concurrent users of the qemu monitor. You either use it from > one place or another at a time. > > Apply in order: > gdb_single_step_monitor_cmd.patch > gdb_monitor_plus_qemu_monitor.patch Thanks, work nicely here! Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 254 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] gdbstub: replace singlestep q packets with qRcmd packets 2008-05-15 14:11 ` [Qemu-devel] [PATCH 1/5] gdbstub: replace singlestep q packets with qRcmd packets Jason Wessel 2008-05-15 14:11 ` [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support Jason Wessel @ 2008-05-15 21:33 ` Edgar E. Iglesias 2008-05-19 13:27 ` Jason Wessel 1 sibling, 1 reply; 20+ messages in thread From: Edgar E. Iglesias @ 2008-05-15 21:33 UTC (permalink / raw) To: Jason Wessel; +Cc: qemu-devel On Thu, May 15, 2008 at 09:11:29AM -0500, Jason Wessel wrote: > At the gdbserial protocol level, using the qRcmd packets allows gdb to > use the "monitor" command to access the controls for single stepping > behavior. Now you can use a gdb "monitor" command instead of a gdb > "maintenance" command. > > The qemu docs were updated to reflect this change. > > Signed-off-by: Jason Wessel <jason.wessel@windriver.com> Thanks Jason, a few comments inlined. > --- > gdbstub.c | 82 +++++++++++++++++++++++++++++++++++++++++---------------- > qemu-doc.texi | 34 +++++++++++------------ > 2 files changed, 75 insertions(+), 41 deletions(-) > > diff --git a/gdbstub.c b/gdbstub.c > index 9a361e3..4bc7999 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -239,6 +239,27 @@ static int put_packet(GDBState *s, char *buf) > return 0; > } > > +static void monitor_output(GDBState *s, const char *msg) > +{ > + char *buf; > + int len = strlen(msg); > + > + buf = malloc(len * 2 + 2); > + buf[0] = 'O'; > + memtohex(buf + 1, msg, len); > + put_packet(s, buf); > + free(buf); > +} It feels odd that the code path that ends up here has line_buf, buf and mem_buf available for parsing the query and generating the response and still we need to malloc for more. Isn't there a way to reuse some of that memory? > + > +static void monitor_help(GDBState *s) > +{ > + monitor_output(s, "gdbstub specific monitor commands:\n"); > + monitor_output(s, "s_show -- Show gdbstub Single Stepping variables\n"); > + monitor_output(s, "set s_step <0|1> -- Single Stepping enabled\n"); > + monitor_output(s, "set s_irq <0|1> -- Single Stepping with qemu irq handlers enabled\n"); > + monitor_output(s, "set s_timer <0|1> -- Single Stepping with qemu timers enabled\n"); > +} I'd prefer if either have a show/set interface or we don't, this is kind of a mix. Some suggestions: monitor sstep enable/disable monitor sstep irq/noirq monitor sstep timers/notimers monitor sstep show or: monitor set/show sstep monitor set/show sirq monitor set/show stimers What do you think? Best regards -- Edgar E. Iglesias Axis Communications AB ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] gdbstub: replace singlestep q packets with qRcmd packets 2008-05-15 21:33 ` [Qemu-devel] [PATCH 1/5] gdbstub: replace singlestep q packets with qRcmd packets Edgar E. Iglesias @ 2008-05-19 13:27 ` Jason Wessel 2008-05-19 15:14 ` Edgar E. Iglesias 0 siblings, 1 reply; 20+ messages in thread From: Jason Wessel @ 2008-05-19 13:27 UTC (permalink / raw) To: Edgar E. Iglesias; +Cc: qemu-devel [-- Attachment #1: Type: text/plain, Size: 1859 bytes --] Edgar E. Iglesias wrote: > On Thu, May 15, 2008 at 09:11:29AM -0500, Jason Wessel wrote: > >> + buf = malloc(len * 2 + 2); >> + buf[0] = 'O'; >> + memtohex(buf + 1, msg, len); >> + put_packet(s, buf); >> + free(buf); >> +} >> > > > It feels odd that the code path that ends up here has line_buf, buf and mem_buf available for parsing the query and generating the response and still we need to malloc for more. Isn't there a way to reuse some of that memory? > Given that put_packet is what really needed the memory and already had its own private global, I modified put_packet to accept a length and contain the memtohex invocation. > > >> + >> +static void monitor_help(GDBState *s) >> +{ >> + monitor_output(s, "gdbstub specific monitor commands:\n"); >> + monitor_output(s, "s_show -- Show gdbstub Single Stepping variables\n"); >> + monitor_output(s, "set s_step <0|1> -- Single Stepping enabled\n"); >> + monitor_output(s, "set s_irq <0|1> -- Single Stepping with qemu irq handlers enabled\n"); >> + monitor_output(s, "set s_timer <0|1> -- Single Stepping with qemu timers enabled\n"); >> +} >> > > I'd prefer if either have a show/set interface or we don't, this is kind of a mix. > > Some suggestions: > monitor sstep enable/disable > monitor sstep irq/noirq > monitor sstep timers/notimers > monitor sstep show > > or: > monitor set/show sstep > monitor set/show sirq > monitor set/show stimers > > What do you think? > I explicitly did not use "monitor set/show" because I figured these commands would have been used by the qemu monitor, even though they are presently not used today to my surprise. After you asked the question it occurred to me that the "else if" block we have now will take care of the problem, so long as unique variables are used. Attached is the new version. Jason. [-- Attachment #2: gdb_single_step_monitor_cmd.patch --] [-- Type: text/x-diff, Size: 8148 bytes --] From: Jason Wessel <jason.wessel@windriver.com> Subject: [PATCH] gdbstub: replace singlestep q packets with qRcmd packets At the gdbserial protocol level, using the qRcmd packets allows gdb to use the "monitor" command to access the controls for single stepping behavior. Now you can use a gdb "monitor" command instead of a gdb "maintenance" command. The qemu docs were updated to reflect this change. Signed-off-by: Jason Wessel <jason.wessel@windriver.com> --- gdbstub.c | 100 +++++++++++++++++++++++++++++++++++++++++----------------- qemu-doc.texi | 36 ++++++++++---------- 2 files changed, 89 insertions(+), 47 deletions(-) --- a/gdbstub.c +++ b/gdbstub.c @@ -205,9 +205,9 @@ static void hextomem(uint8_t *mem, const } /* return -1 if error, 0 if OK */ -static int put_packet(GDBState *s, char *buf) +static int put_packet_hex(GDBState *s, const char *buf, int len, int isHex) { - int len, csum, i; + int csum, i; uint8_t *p; #ifdef DEBUG_GDB @@ -217,13 +217,19 @@ static int put_packet(GDBState *s, char for(;;) { p = s->last_packet; *(p++) = '$'; - len = strlen(buf); - memcpy(p, buf, len); - p += len; + if (isHex) { + p[0] = 'O'; + memtohex(p + 1, buf, len); + len = strlen(p); + } else { + memcpy(p, buf, len); + } + csum = 0; for(i = 0; i < len; i++) { - csum += buf[i]; + csum += p[i]; } + p += len; *(p++) = '#'; *(p++) = tohex((csum >> 4) & 0xf); *(p++) = tohex((csum) & 0xf); @@ -244,6 +250,25 @@ static int put_packet(GDBState *s, char return 0; } +/* return -1 if error, 0 if OK */ +static int put_packet(GDBState *s, const char *buf) { + return put_packet_hex(s, buf, strlen(buf), 0); +} + +static void monitor_output(GDBState *s, const char *msg) +{ + put_packet_hex(s, msg, strlen(msg), 1); +} + +static void monitor_help(GDBState *s) +{ + monitor_output(s, "gdbstub specific monitor commands:\n"); + monitor_output(s, "show <sstep|sirq|stimer> -- Show gdbstub Single Stepping variables\n"); + monitor_output(s, "set sstep <0|1> -- Single Stepping enabled\n"); + monitor_output(s, "set sirq <0|1> -- Single Stepping with qemu irq handlers enabled\n"); + monitor_output(s, "set stimers <0|1> -- Single Stepping with qemu timers enabled\n"); +} + #if defined(TARGET_I386) #ifdef TARGET_X86_64 @@ -948,6 +973,44 @@ static void cpu_gdb_write_registers(CPUS #endif +static void gdb_rcmd(GDBState *s, const char *p, char *buf, char *mem_buf) +{ + int len = strlen(p); + + if ((len % 2) != 0) { + put_packet(s, "E01"); + return; + } + hextomem(mem_buf, p, len); + mem_buf[(len >> 1)] = 0; + + if (!strcmp(mem_buf, "show sstep")) { + sprintf(buf, "sstep == %i\n", (sstep_flags & SSTEP_ENABLE) != 0); + monitor_output(s, buf); + } else if (!strcmp(mem_buf, "show sirq")) { + sprintf(buf, "sirq == %i\n", (sstep_flags & SSTEP_NOIRQ) == 0); + monitor_output(s, buf); + } else if (!strcmp(mem_buf, "show stimers")) { + sprintf(buf, "stimers == %i\n", (sstep_flags & SSTEP_NOTIMER) == 0); + monitor_output(s, buf); + } else if (!strcmp(mem_buf, "set sstep 1")) { + sstep_flags |= SSTEP_ENABLE; + } else if (!strcmp(mem_buf, "set step 0")) { + sstep_flags &= ~SSTEP_ENABLE; + } else if (!strcmp(mem_buf, "set sirq 1")) { + sstep_flags &= ~SSTEP_NOIRQ; + } else if (!strcmp(mem_buf, "set sirq 0")) { + sstep_flags |= SSTEP_NOIRQ; + } else if (!strcmp(mem_buf, "set stimers 1")) { + sstep_flags &= ~SSTEP_NOTIMER; + } else if (!strcmp(mem_buf, "set stimers 0")) { + sstep_flags |= SSTEP_NOTIMER; + } else if (!strcmp(mem_buf, "help") || !strcmp(mem_buf, "?")) { + monitor_help(s); + } + put_packet(s, "OK"); +} + static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf) { const char *p; @@ -1139,29 +1202,8 @@ static int gdb_handle_packet(GDBState *s } 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"); + if (!strncmp(p, "Rcmd,", 5)) { + gdb_rcmd(s, p + 5, buf, mem_buf); break; } #ifdef CONFIG_LINUX_USER --- a/qemu-doc.texi +++ b/qemu-doc.texi @@ -1951,32 +1951,32 @@ Use @code{set architecture i8086} to dum 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: +The default single stepping behavior is to 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 execute. Because there are rare circumstances where you want to single step into an interrupt vector the behavior can be controlled from GDB. There are several commands you use to query and set the single step behavior while inside gdb: @table @code -@item maintenance packet qqemu.sstepbits +@item monitor show <sstep|sirq|stimers> -This will display the MASK bits used to control the single stepping IE: +This will display the values of the single stepping controls IE: @example -(gdb) maintenance packet qqemu.sstepbits -sending: "qqemu.sstepbits" -received: "ENABLE=1,NOIRQ=2,NOTIMER=4" +(gdb) monitor show sstep +sstep == 1 +(gdb) monitor show sirq +sirq == 0 +(gdb) monitor show stimers +stimers == 0 @end example -@item maintenance packet qqemu.sstep +@item monitor set sstep <0|1> -This will display the current value of the mask used when single stepping IE: +Turn off(0) or on(1) the single stepping feature all together, which is defaulted to on. @example -(gdb) maintenance packet qqemu.sstep -sending: "qqemu.sstep" -received: "0x7" +(gdb) monitor set sstep 0 +(gdb) monitor set sstep 1 @end example -@item maintenance packet Qqemu.sstep=HEX_VALUE +@item monitor set sirq <0|1> -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 +Turn off or on the the irq processing when single stepping, which is defaulted to off. +@item monitor set stimers <0|1> + +Turn off or on the the timer processing when single stepping, which is defaulted to off. @end table @node pcsys_os_specific ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] gdbstub: replace singlestep q packets with qRcmd packets 2008-05-19 13:27 ` Jason Wessel @ 2008-05-19 15:14 ` Edgar E. Iglesias 0 siblings, 0 replies; 20+ messages in thread From: Edgar E. Iglesias @ 2008-05-19 15:14 UTC (permalink / raw) To: Jason Wessel; +Cc: qemu-devel On Mon, May 19, 2008 at 08:27:42AM -0500, Jason Wessel wrote: > Edgar E. Iglesias wrote: > > On Thu, May 15, 2008 at 09:11:29AM -0500, Jason Wessel wrote: > >> + buf = malloc(len * 2 + 2); > >> + buf[0] = 'O'; > >> + memtohex(buf + 1, msg, len); > >> + put_packet(s, buf); > >> + free(buf); > >> +} > >> > > It feels odd that the code path that ends up here has line_buf, buf > > and mem_buf available for parsing the query and generating the response > > and still we need to malloc for more. Isn't there a way to reuse some of > > that memory? > > Given that put_packet is what really needed the memory and already had > its own private global, I modified put_packet to accept a length and > contain the memtohex invocation. > > > or: > > monitor set/show sstep > > monitor set/show sirq > > monitor set/show stimers > > > > What do you think? > > > I explicitly did not use "monitor set/show" because I figured these > commands would have been used by the qemu monitor, even though they > are presently not used today to my surprise. After you asked the > question it occurred to me that the "else if" block we have now will > take care of the problem, so long as unique variables are used. > > Attached is the new version. Thanks for fixing it up, I think this version looks good. Best regards > From: Jason Wessel <jason.wessel@windriver.com> > Subject: [PATCH] gdbstub: replace singlestep q packets with qRcmd packets > > At the gdbserial protocol level, using the qRcmd packets allows gdb to > use the "monitor" command to access the controls for single stepping > behavior. Now you can use a gdb "monitor" command instead of a gdb > "maintenance" command. > > The qemu docs were updated to reflect this change. > > Signed-off-by: Jason Wessel <jason.wessel@windriver.com> > > --- > gdbstub.c | 100 +++++++++++++++++++++++++++++++++++++++++----------------- > qemu-doc.texi | 36 ++++++++++---------- > 2 files changed, 89 insertions(+), 47 deletions(-) > > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -205,9 +205,9 @@ static void hextomem(uint8_t *mem, const > } > > /* return -1 if error, 0 if OK */ > -static int put_packet(GDBState *s, char *buf) > +static int put_packet_hex(GDBState *s, const char *buf, int len, int isHex) > { > - int len, csum, i; > + int csum, i; > uint8_t *p; > > #ifdef DEBUG_GDB > @@ -217,13 +217,19 @@ static int put_packet(GDBState *s, char > for(;;) { > p = s->last_packet; > *(p++) = '$'; > - len = strlen(buf); > - memcpy(p, buf, len); > - p += len; > + if (isHex) { > + p[0] = 'O'; > + memtohex(p + 1, buf, len); > + len = strlen(p); > + } else { > + memcpy(p, buf, len); > + } > + > csum = 0; > for(i = 0; i < len; i++) { > - csum += buf[i]; > + csum += p[i]; > } > + p += len; > *(p++) = '#'; > *(p++) = tohex((csum >> 4) & 0xf); > *(p++) = tohex((csum) & 0xf); > @@ -244,6 +250,25 @@ static int put_packet(GDBState *s, char > return 0; > } > > +/* return -1 if error, 0 if OK */ > +static int put_packet(GDBState *s, const char *buf) { > + return put_packet_hex(s, buf, strlen(buf), 0); > +} > + > +static void monitor_output(GDBState *s, const char *msg) > +{ > + put_packet_hex(s, msg, strlen(msg), 1); > +} > + > +static void monitor_help(GDBState *s) > +{ > + monitor_output(s, "gdbstub specific monitor commands:\n"); > + monitor_output(s, "show <sstep|sirq|stimer> -- Show gdbstub Single Stepping variables\n"); > + monitor_output(s, "set sstep <0|1> -- Single Stepping enabled\n"); > + monitor_output(s, "set sirq <0|1> -- Single Stepping with qemu irq handlers enabled\n"); > + monitor_output(s, "set stimers <0|1> -- Single Stepping with qemu timers enabled\n"); > +} > + > #if defined(TARGET_I386) > > #ifdef TARGET_X86_64 > @@ -948,6 +973,44 @@ static void cpu_gdb_write_registers(CPUS > > #endif > > +static void gdb_rcmd(GDBState *s, const char *p, char *buf, char *mem_buf) > +{ > + int len = strlen(p); > + > + if ((len % 2) != 0) { > + put_packet(s, "E01"); > + return; > + } > + hextomem(mem_buf, p, len); > + mem_buf[(len >> 1)] = 0; > + > + if (!strcmp(mem_buf, "show sstep")) { > + sprintf(buf, "sstep == %i\n", (sstep_flags & SSTEP_ENABLE) != 0); > + monitor_output(s, buf); > + } else if (!strcmp(mem_buf, "show sirq")) { > + sprintf(buf, "sirq == %i\n", (sstep_flags & SSTEP_NOIRQ) == 0); > + monitor_output(s, buf); > + } else if (!strcmp(mem_buf, "show stimers")) { > + sprintf(buf, "stimers == %i\n", (sstep_flags & SSTEP_NOTIMER) == 0); > + monitor_output(s, buf); > + } else if (!strcmp(mem_buf, "set sstep 1")) { > + sstep_flags |= SSTEP_ENABLE; > + } else if (!strcmp(mem_buf, "set step 0")) { > + sstep_flags &= ~SSTEP_ENABLE; > + } else if (!strcmp(mem_buf, "set sirq 1")) { > + sstep_flags &= ~SSTEP_NOIRQ; > + } else if (!strcmp(mem_buf, "set sirq 0")) { > + sstep_flags |= SSTEP_NOIRQ; > + } else if (!strcmp(mem_buf, "set stimers 1")) { > + sstep_flags &= ~SSTEP_NOTIMER; > + } else if (!strcmp(mem_buf, "set stimers 0")) { > + sstep_flags |= SSTEP_NOTIMER; > + } else if (!strcmp(mem_buf, "help") || !strcmp(mem_buf, "?")) { > + monitor_help(s); > + } > + put_packet(s, "OK"); > +} > + > static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf) > { > const char *p; > @@ -1139,29 +1202,8 @@ static int gdb_handle_packet(GDBState *s > } > 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"); > + if (!strncmp(p, "Rcmd,", 5)) { > + gdb_rcmd(s, p + 5, buf, mem_buf); > break; > } > #ifdef CONFIG_LINUX_USER > --- a/qemu-doc.texi > +++ b/qemu-doc.texi > @@ -1951,32 +1951,32 @@ Use @code{set architecture i8086} to dum > > 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: > +The default single stepping behavior is to 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 execute. Because there are rare circumstances where you want to single step into an interrupt vector the behavior can be controlled from GDB. There are several commands you use to query and set the single step behavior while inside gdb: > @table @code > -@item maintenance packet qqemu.sstepbits > +@item monitor show <sstep|sirq|stimers> > > -This will display the MASK bits used to control the single stepping IE: > +This will display the values of the single stepping controls IE: > @example > -(gdb) maintenance packet qqemu.sstepbits > -sending: "qqemu.sstepbits" > -received: "ENABLE=1,NOIRQ=2,NOTIMER=4" > +(gdb) monitor show sstep > +sstep == 1 > +(gdb) monitor show sirq > +sirq == 0 > +(gdb) monitor show stimers > +stimers == 0 > @end example > -@item maintenance packet qqemu.sstep > +@item monitor set sstep <0|1> > > -This will display the current value of the mask used when single stepping IE: > +Turn off(0) or on(1) the single stepping feature all together, which is defaulted to on. > @example > -(gdb) maintenance packet qqemu.sstep > -sending: "qqemu.sstep" > -received: "0x7" > +(gdb) monitor set sstep 0 > +(gdb) monitor set sstep 1 > @end example > -@item maintenance packet Qqemu.sstep=HEX_VALUE > +@item monitor set sirq <0|1> > > -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 > +Turn off or on the the irq processing when single stepping, which is defaulted to off. > +@item monitor set stimers <0|1> > + > +Turn off or on the the timer processing when single stepping, which is defaulted to off. > @end table > > @node pcsys_os_specific -- Edgar E. Iglesias Axis Communications AB ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2008-05-23 15:33 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-05-15 14:11 [Qemu-devel] [PATCH 0/5] gdbstub and single step improvments Jason Wessel 2008-05-15 14:11 ` [Qemu-devel] [PATCH 1/5] gdbstub: replace singlestep q packets with qRcmd packets Jason Wessel 2008-05-15 14:11 ` [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support Jason Wessel 2008-05-15 14:11 ` [Qemu-devel] [PATCH 3/5] vl.c: always run the real time timers when single stepping Jason Wessel 2008-05-15 14:11 ` [Qemu-devel] [PATCH 4/5] gdbstub: support for gdb "detach/kill/quit" Jason Wessel 2008-05-15 14:11 ` [Qemu-devel] [PATCH 5/5] ppc: fix crash in ppc system single step support Jason Wessel 2008-05-15 21:13 ` [Qemu-devel] [PATCH 4/5] gdbstub: support for gdb "detach/kill/quit" Edgar E. Iglesias 2008-05-15 22:17 ` [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support Edgar E. Iglesias 2008-05-19 13:29 ` Jason Wessel 2008-05-19 16:30 ` Paul Brook 2008-05-21 12:58 ` Maxim Gorbachyov 2008-05-21 17:03 ` Jason Wessel 2008-05-22 13:24 ` [Qemu-devel] " Jan Kiszka 2008-05-22 13:45 ` Jason Wessel 2008-05-22 13:53 ` Jan Kiszka 2008-05-22 16:55 ` Jason Wessel 2008-05-23 15:33 ` Jan Kiszka 2008-05-15 21:33 ` [Qemu-devel] [PATCH 1/5] gdbstub: replace singlestep q packets with qRcmd packets Edgar E. Iglesias 2008-05-19 13:27 ` Jason Wessel 2008-05-19 15:14 ` Edgar E. Iglesias
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).