[Qemu-devel] [RFC 0/2] Break the debug session on sw errors

[Qemu-devel] [RFC 0/2] Break the debug session on sw errors

[edgar.iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Hi,

I've been using variations of this scheme to aid with debugging
guest errors that QEMU can detect. I'm posting this RFC to see
if there is interest to get something like this upstreamed.

Patch 1 adds the mechanism to break the debugger session from
within emulation models.

Patch 2 hardcodes the interruption of the GDB session on
log_guest_error calls to qemu_log_mask.
Some possible ways to conditionalize the behaviour could be
through cmdline options or through GDB remote commands.

Any thoughts/ideas on this?

Cheers,
Edgar

Edgar E. Iglesias (2):
  gdbstub: Add gdbserver_break()
  qemu-log: Interrupt the GDB session on guest-errors

 gdbstub.c              |   68 ++++++++++++++++++++++++++++++++++++++----------
 include/exec/gdbstub.h |    2 ++
 qemu-log.c             |   20 ++++++++++++++
 3 files changed, 76 insertions(+), 14 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [RFC 1/2] gdbstub: Add gdbserver_break()

[edgar.iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Makes it possible to request interruption of the GDB
debug session.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 gdbstub.c              |   68 ++++++++++++++++++++++++++++++++++++++----------
 include/exec/gdbstub.h |    2 ++
 2 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index e80e1d3..84232f6 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -297,6 +297,7 @@ typedef struct GDBState {
     uint8_t last_packet[MAX_PACKET_LENGTH + 4];
     int last_packet_len;
     int signal;
+    int client_connected;
 #ifdef CONFIG_USER_ONLY
     int fd;
     int running_state;
@@ -2505,12 +2506,56 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
     return RS_IDLE;
 }
 
+static void gdb_output(GDBState *s, const char *msg, int len)
+{
+    char buf[MAX_PACKET_LENGTH];
+
+    buf[0] = 'O';
+    if (len > (MAX_PACKET_LENGTH/2) - 1) {
+        len = (MAX_PACKET_LENGTH/2) - 1;
+    }
+    memtohex(buf + 1, (uint8_t *)msg, len);
+    put_packet(s, buf);
+}
+
 void gdb_set_stop_cpu(CPUArchState *env)
 {
     gdbserver_state->c_cpu = env;
     gdbserver_state->g_cpu = env;
 }
 
+static int gdbserver_has_client(void)
+{
+    return gdbserver_state && gdbserver_state->client_connected;
+}
+
+int gdbserver_break(const char *msg)
+{
+
+    if (!gdbserver_has_client()) {
+        return 1;
+    }
+
+    if (msg) {
+        gdb_output(gdbserver_state, msg, strlen(msg));
+    }
+
+    /* If there's a CPU running, break it's execution.  */
+    if (cpu_single_env) {
+        CPUState *cpu = ENV_GET_CPU(cpu_single_env);
+        cpu_single_env->exception_index = EXCP_DEBUG;
+        if (cpu->current_tb) {
+            /* Break out of current TB and request debug action.  */
+            cpu_loop_exit(cpu_single_env);
+        }
+    }
+#ifndef CONFIG_USER_ONLY
+    /* Request global debug action.  */
+    qemu_system_debug_request();
+#endif
+    return 0;
+}
+
 #ifndef CONFIG_USER_ONLY
 static void gdb_vm_state_change(void *opaque, int running, RunState state)
 {
@@ -2815,6 +2860,7 @@ gdb_handlesig (CPUArchState *env, int sig)
         {
           /* XXX: Connection closed.  Should probably wait for another
              connection before continuing.  */
+          s->client_connected = false;
           return sig;
         }
   }
@@ -2868,7 +2914,7 @@ static void gdb_accept(void)
     gdb_has_xml = 0;
 
     gdbserver_state = s;
-
+    s->client_connected = true;
     fcntl(fd, F_SETFL, O_NONBLOCK);
 }
 
@@ -2952,23 +2998,17 @@ static void gdb_chr_event(void *opaque, int event)
     case CHR_EVENT_OPENED:
         vm_stop(RUN_STATE_PAUSED);
         gdb_has_xml = 0;
+        gdbserver_state->client_connected = true;
         break;
+    case CHR_EVENT_CLOSED: {
+        gdbserver_state->client_connected = false;
+        break;
+    }
     default:
         break;
     }
 }
 
-static void gdb_monitor_output(GDBState *s, const char *msg, int len)
-{
-    char buf[MAX_PACKET_LENGTH];
-
-    buf[0] = 'O';
-    if (len > (MAX_PACKET_LENGTH/2) - 1)
-        len = (MAX_PACKET_LENGTH/2) - 1;
-    memtohex(buf + 1, (uint8_t *)msg, len);
-    put_packet(s, buf);
-}
-
 static int gdb_monitor_write(CharDriverState *chr, const uint8_t *buf, int len)
 {
     const char *p = (const char *)buf;
@@ -2977,10 +3017,10 @@ static int gdb_monitor_write(CharDriverState *chr, const uint8_t *buf, int len)
     max_sz = (sizeof(gdbserver_state->last_packet) - 2) / 2;
     for (;;) {
         if (len <= max_sz) {
-            gdb_monitor_output(gdbserver_state, p, len);
+            gdb_output(gdbserver_state, p, len);
             break;
         }
-        gdb_monitor_output(gdbserver_state, p, max_sz);
+        gdb_output(gdbserver_state, p, max_sz);
         p += max_sz;
         len -= max_sz;
     }
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index ba20afa..6f29d2a 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -47,6 +47,8 @@ int gdbserver_start(int);
 int gdbserver_start(const char *port);
 #endif
 
+int gdbserver_break(const char *msg);
+
 /* in gdbstub-xml.c, generated by scripts/feature_to_c.sh */
 extern const char *const xml_builtin[][2];
 
-- 
1.7.10.4

[Qemu-devel] [RFC 2/2] qemu-log: Interrupt the GDB session on guest-errors

[edgar.iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 qemu-log.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/qemu-log.c b/qemu-log.c
index 797f2af..693bc94 100644
--- a/qemu-log.c
+++ b/qemu-log.c
@@ -19,6 +19,7 @@
 
 #include "qemu-common.h"
 #include "qemu/log.h"
+#include "exec/gdbstub.h"
 
 static char *logfilename;
 FILE *qemu_logfile;
@@ -45,6 +46,25 @@ void qemu_log_mask(int mask, const char *fmt, ...)
         vfprintf(qemu_logfile, fmt, ap);
     }
     va_end(ap);
+
+    /*
+     * Break the GDB session (if connected) so that the user can inspect the
+     * guest state.
+     *
+     * TODO: Consider conditionalizing this on a cmdline option.
+     */
+    if (mask & LOG_GUEST_ERROR) {
+        char *msg;
+
+        va_start(ap, fmt);
+        if (vasprintf(&msg, fmt, ap) < 0) {
+            msg = NULL;
+        }
+        va_end(ap);
+
+        gdbserver_break(msg);
+        g_free(msg);
+    }
 }
 
 /* enable or disable low levels log */
-- 
1.7.10.4

Re: [Qemu-devel] [RFC 2/2] qemu-log: Interrupt the GDB session on guest-errors

[Peter Maydell]

On 22 May 2013 11:38,  <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> @@ -45,6 +46,25 @@ void qemu_log_mask(int mask, const char *fmt, ...)
>          vfprintf(qemu_logfile, fmt, ap);
>      }
>      va_end(ap);
> +
> +    /*
> +     * Break the GDB session (if connected) so that the user can inspect the
> +     * guest state.
> +     *
> +     * TODO: Consider conditionalizing this on a cmdline option.
> +     */

This is definitely way too intrusive to be unconditional -- it can
happen really frequently (for instance Linux on OMAP3 will access
a nonexistent register every time it takes an interrupt).

thanks
-- PMM

Re: [Qemu-devel] [RFC 2/2] qemu-log: Interrupt the GDB session on guest-errors

[Edgar E. Iglesias]

On Wed, May 22, 2013 at 11:45:46AM +0100, Peter Maydell wrote:
> On 22 May 2013 11:38,  <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> > @@ -45,6 +46,25 @@ void qemu_log_mask(int mask, const char *fmt, ...)
> >          vfprintf(qemu_logfile, fmt, ap);
> >      }
> >      va_end(ap);
> > +
> > +    /*
> > +     * Break the GDB session (if connected) so that the user can inspect the
> > +     * guest state.
> > +     *
> > +     * TODO: Consider conditionalizing this on a cmdline option.
> > +     */
> 
> This is definitely way too intrusive to be unconditional -- it can
> happen really frequently (for instance Linux on OMAP3 will access
> a nonexistent register every time it takes an interrupt).

Ye, I figured this would be the case.

Maybe a qemu monitor flag controllable from the gdb client itself might be
the most useful, default off. Then one can turn the breaks on/off on
the fly while debugging.

monitor gdb_break_on_guest_errors or something like that.

Cheers,
Edgar