From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: [RFC][PATCH] qemu-kvm: SMP guest debugging Date: Fri, 23 May 2008 18:06:30 +0200 Message-ID: <4836EB86.5090800@web.de> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------090500040704030505070104" To: kvm-devel Return-path: Received: from fmmailgate02.web.de ([217.72.192.227]:36769 "EHLO fmmailgate02.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752418AbYEWQGc (ORCPT ); Fri, 23 May 2008 12:06:32 -0400 Received: from smtp05.web.de (fmsmtp05.dlan.cinetic.de [172.20.4.166]) by fmmailgate02.web.de (Postfix) with ESMTP id 50692DE0EFFB for ; Fri, 23 May 2008 18:06:31 +0200 (CEST) Received: from [88.64.30.47] (helo=[192.168.1.198]) by smtp05.web.de with asmtp (TLSv1:AES256-SHA:256) (WEB.DE 4.109 #226) id 1JzZmp-0004h9-00 for kvm@vger.kernel.org; Fri, 23 May 2008 18:06:31 +0200 Sender: kvm-owner@vger.kernel.org List-ID: This is a multi-part message in MIME format. --------------090500040704030505070104 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit 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 @@ -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 --------------090500040704030505070104 Content-Type: text/x-patch; name="qemu-improve-smp-debugging.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="qemu-improve-smp-debugging.patch" --- 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 */ --------------090500040704030505070104--