KVM, just like plain QEMU, does not allow debugging of SMP guests
reliably because the debugged CPU is fixed and, even worse, breakpoints
only have effect on that single CPU.

Patch below overcomes this by exploiting the attached carrier patch for
QEMU (which couples the monitor with the debugger focus). Furthermore,
the patch ensures that KVM_SET_GUEST_DEBUG is properly updated for all
vcpus if the first soft breakpoint is added or the last one is removed.

This patch is RFC as it depends on that "carrier" patch which needs more
work (SMP debugging with plain QEMU is still borken) and only applies to
KVM right now (QEMU trunk contains a few interesting debugger
extensions, and even more are queued). However, if you want to play with
SMP debugging under KVM, you may like these two.

---
 qemu/monitor.c  |    1 +
 qemu/qemu-kvm.c |   29 ++++++++++++++++++++++-------
 2 files changed, 23 insertions(+), 7 deletions(-)

Index: b/qemu/monitor.c
===================================================================
--- a/qemu/monitor.c
+++ b/qemu/monitor.c
@@ -319,6 +319,7 @@ static void do_info_cpus(void)
         mon_set_cpu(first_cpu);
 
     for(env = first_cpu; env != NULL; env = env->next_cpu) {
+        kvm_save_registers(env);
         term_printf("%c CPU #%d:",
                     (env == mon_cpu) ? '*' : ' ',
                     env->cpu_index);
Index: b/qemu/qemu-kvm.c
===================================================================
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -19,6 +19,7 @@ int kvm_pit = 1;
 #include "qemu-common.h"
 #include "console.h"
 #include "block.h"
+#include "monitor.h"
 
 #include "qemu-kvm.h"
 #include <libkvm.h>
@@ -58,7 +59,7 @@ pthread_t io_thread;
 static int io_thread_fd = -1;
 static int io_thread_sigfd = -1;
 
-static int kvm_debug_stop_requested;
+static CPUState *kvm_debug_cpu_requested;
 
 struct kvm_sw_breakpoint *first_sw_breakpoint;
 
@@ -548,9 +549,10 @@ int kvm_main_loop(void)
             qemu_system_powerdown();
         else if (qemu_reset_requested())
            qemu_kvm_system_reset();
-       else if (kvm_debug_stop_requested) {
-           kvm_debug_stop_requested = 0;
+       else if (kvm_debug_cpu_requested) {
+           mon_set_cpu(kvm_debug_cpu_requested);
            vm_stop(EXCP_DEBUG);
+           kvm_debug_cpu_requested = NULL;
        }
     }
 
@@ -576,7 +578,7 @@ static int kvm_debug(void *opaque, int v
        break;
     }
     if (handle) {
-       kvm_debug_stop_requested = 1;
+       kvm_debug_cpu_requested = cpu_single_env;
        vcpu_info[vcpu].stopped = 1;
     } else
        kvm_update_guest_debug(cpu_single_env, break_type);
@@ -855,8 +857,15 @@ int kvm_insert_breakpoint(CPUState *env,
     first_sw_breakpoint = bp;
     if (bp->next)
        bp->next->prev = bp;
-
-    return kvm_update_guest_debug(env, KVM_GDB_BREAKPOINT_NONE);
+    else {
+       /* Just added the first breakpoint, so update the kernel state. */
+       foreach_cpu(env) {
+           err = kvm_update_guest_debug(env, KVM_GDB_BREAKPOINT_NONE);
+           if (err)
+               break;
+       }
+    }
+    return err;
 #else
     return -EINVAL;
 #endif
@@ -894,7 +903,13 @@ int kvm_remove_breakpoint(CPUState *env,
 
     free(bp);
 
-    return kvm_update_guest_debug(env, KVM_GDB_BREAKPOINT_NONE);
+    if (!first_sw_breakpoint)
+       foreach_cpu(env) {
+           err = kvm_update_guest_debug(env, KVM_GDB_BREAKPOINT_NONE);
+           if (err)
+               break;
+       }
+    return err;
 #else
     return -EINVAL;
 #endif
---
 qemu/cpu-defs.h |    3 +++
 qemu/gdbstub.c  |   23 +++++++++++------------
 qemu/monitor.c  |   19 ++++++++++++-------
 qemu/monitor.h  |    7 +++++++
 qemu/vl.c       |    2 ++
 5 files changed, 35 insertions(+), 19 deletions(-)

Index: b/qemu/cpu-defs.h
===================================================================
--- a/qemu/cpu-defs.h
+++ b/qemu/cpu-defs.h
@@ -168,3 +168,6 @@ typedef struct CPUTLBEntry {
     const char *cpu_model_str;
 
 #endif
+
+#define foreach_cpu(env) \
+    for(env = first_cpu; env != NULL; env = env->next_cpu)
Index: b/qemu/gdbstub.c
===================================================================
--- a/qemu/gdbstub.c
+++ b/qemu/gdbstub.c
@@ -35,6 +35,7 @@
 #include "gdbstub.h"
 #include "qemu-kvm.h"
 #endif
+#include "monitor.h"
 
 #include "qemu_socket.h"
 #ifdef _WIN32
@@ -59,7 +60,6 @@ enum RSState {
     RS_SYSCALL,
 };
 typedef struct GDBState {
-    CPUState *env; /* current CPU */
     enum RSState state; /* parsing state */
     char line_buf[4096];
     int line_buf_index;
@@ -962,7 +962,7 @@ static int gdb_handle_packet(GDBState *s
                 p++;
             type = *p;
             if (gdb_current_syscall_cb)
-                gdb_current_syscall_cb(s->env, ret, err);
+                gdb_current_syscall_cb(mon_get_cpu(), ret, err);
             if (type == 'C') {
                 put_packet(s, "T02");
             } else {
@@ -1092,6 +1092,7 @@ extern void tb_flush(CPUState *env);
 static void gdb_vm_stopped(void *opaque, int reason)
 {
     GDBState *s = opaque;
+    CPUState *env = mon_get_cpu();
     char buf[256];
     int ret;
 
@@ -1099,18 +1100,18 @@ static void gdb_vm_stopped(void *opaque,
         return;
 
     /* disable single step if it was enable */
-    cpu_single_step(s->env, 0);
+    cpu_single_step(env, 0);
 
     if (reason == EXCP_DEBUG) {
-        if (s->env->watchpoint_hit) {
+        if (env->watchpoint_hit) {
             snprintf(buf, sizeof(buf), "T%02xwatch:" TARGET_FMT_lx ";",
                      SIGTRAP,
-                     s->env->watchpoint[s->env->watchpoint_hit - 1].vaddr);
+                     env->watchpoint[env->watchpoint_hit - 1].vaddr);
             put_packet(s, buf);
-            s->env->watchpoint_hit = 0;
+            env->watchpoint_hit = 0;
             return;
         }
-	tb_flush(s->env);
+	tb_flush(env);
         ret = SIGTRAP;
     } else if (reason == EXCP_INTERRUPT) {
         ret = SIGINT;
@@ -1180,15 +1181,15 @@ void gdb_do_syscall(gdb_syscall_complete
     va_end(va);
     put_packet(s, buf);
 #ifdef CONFIG_USER_ONLY
-    gdb_handlesig(s->env, 0);
+    gdb_handlesig(mon_get_cpu(), 0);
 #else
-    cpu_interrupt(s->env, CPU_INTERRUPT_EXIT);
+    cpu_interrupt(mon_get_cpu(), CPU_INTERRUPT_EXIT);
 #endif
 }
 
 static void gdb_read_byte(GDBState *s, int ch)
 {
-    CPUState *env = s->env;
+    CPUState *env = mon_get_cpu();
     int i, csum;
     uint8_t reply;
 
@@ -1348,7 +1349,6 @@ static void gdb_accept(void *opaque)
 
     s = &gdbserver_state;
     memset (s, 0, sizeof (GDBState));
-    s->env = first_cpu; /* XXX: allow to change CPU */
     s->fd = fd;
 
     gdb_syscall_state = s;
@@ -1451,7 +1451,6 @@ int gdbserver_start(const char *port)
     if (!s) {
         return -1;
     }
-    s->env = first_cpu; /* XXX: allow to change CPU */
     s->chr = chr;
     qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive,
                           gdb_chr_event, s);
Index: b/qemu/monitor.c
===================================================================
--- a/qemu/monitor.c
+++ b/qemu/monitor.c
@@ -266,8 +266,7 @@ static void do_info_blockstats(void)
     bdrv_info_stats();
 }
 
-/* get the current CPU defined by the user */
-static int mon_set_cpu(int cpu_index)
+static int mon_set_cpu_index(int cpu_index)
 {
     CPUState *env;
 
@@ -280,10 +279,16 @@ static int mon_set_cpu(int cpu_index)
     return -1;
 }
 
-static CPUState *mon_get_cpu(void)
+void mon_set_cpu(CPUState *env)
+{
+    mon_cpu = env;
+}
+
+/* get the current CPU defined by the user or by a breakpoint hit */
+CPUState *mon_get_cpu(void)
 {
     if (!mon_cpu) {
-        mon_set_cpu(0);
+        mon_set_cpu(first_cpu);
     }
 
     kvm_save_registers(mon_cpu);
@@ -310,8 +315,8 @@ static void do_info_cpus(void)
 {
     CPUState *env;
 
-    /* just to set the default cpu if not already done */
-    mon_get_cpu();
+    if (!mon_cpu)
+        mon_set_cpu(first_cpu);
 
     for(env = first_cpu; env != NULL; env = env->next_cpu) {
         term_printf("%c CPU #%d:",
@@ -341,7 +346,7 @@ static void do_info_cpus(void)
 
 static void do_cpu_set(int index)
 {
-    if (mon_set_cpu(index) < 0)
+    if (mon_set_cpu_index(index) < 0)
         term_printf("Invalid CPU index\n");
 }
 
Index: b/qemu/monitor.h
===================================================================
--- /dev/null
+++ b/qemu/monitor.h
@@ -0,0 +1,7 @@
+#ifndef QEMU_MONITOR_H
+#define QEMU_MONITOR_H
+
+void mon_set_cpu(CPUState *env);
+CPUState *mon_get_cpu(void);
+
+#endif /* QEMU_MONITOR_H */
Index: b/qemu/vl.c
===================================================================
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -33,6 +33,7 @@
 #include "console.h"
 #include "sysemu.h"
 #include "gdbstub.h"
+#include "monitor.h"
 #include "qemu-timer.h"
 #include "qemu-char.h"
 #include "block.h"
@@ -7644,6 +7645,7 @@ static int main_loop(void)
                 ret = EXCP_INTERRUPT;
             }
             if (unlikely(ret == EXCP_DEBUG)) {
+                mon_set_cpu(cur_cpu);
                 vm_stop(EXCP_DEBUG);
             }
             /* If all cpus are halted then wait until the next IRQ */

Reply via email to