[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor
From: |
Jason Wessel |
Subject: |
Re: [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support |
Date: |
Thu, 22 May 2008 11:55:58 -0500 |
User-agent: |
Thunderbird 2.0.0.14 (X11/20080502) |
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
>>>>> <address@hidden> 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.
From: Jason Wessel <address@hidden>
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 <address@hidden>
---
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
address@hidden maintenance packet qqemu.sstepbits
address@hidden 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
address@hidden maintenance packet qqemu.sstep
address@hidden 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
address@hidden maintenance packet Qqemu.sstep=HEX_VALUE
address@hidden 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:
address@hidden
-(gdb) maintenance packet Qqemu.sstep=0x5
-sending: "qemu.sstep=0x5"
-received: "OK"
address@hidden example
+Turn off or on the the irq processing when single stepping, which is defaulted
to off.
address@hidden 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
From: Jason Wessel <address@hidden>
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 <address@hidden>
---
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.
+
address@hidden
+(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].
address@hidden example
+
@node pcsys_os_specific
@section Target OS specific information
- [Qemu-devel] [PATCH 5/5] ppc: fix crash in ppc system single step support, (continued)
- [Qemu-devel] [PATCH 5/5] ppc: fix crash in ppc system single step support, Jason Wessel, 2008/05/15
- Re: [Qemu-devel] [PATCH 4/5] gdbstub: support for gdb "detach/kill/quit", Edgar E. Iglesias, 2008/05/15
- Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Edgar E. Iglesias, 2008/05/15
- Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Jason Wessel, 2008/05/19
- Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Paul Brook, 2008/05/19
- Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Maxim Gorbachyov, 2008/05/21
- Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Jason Wessel, 2008/05/21
- [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Jan Kiszka, 2008/05/22
- Re: [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Jason Wessel, 2008/05/22
- [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Jan Kiszka, 2008/05/22
- Re: [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support,
Jason Wessel <=
- [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Jan Kiszka, 2008/05/23
Re: [Qemu-devel] [PATCH 1/5] gdbstub: replace singlestep q packets with qRcmd packets, Edgar E. Iglesias, 2008/05/15