From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: [PATCH 5/11] QEMU: Improve SMP debugging support Date: Tue, 27 May 2008 00:10:02 +0200 Message-ID: <483B353A.1010909@web.de> References: <4839B14A.3010406@web.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Cc: Avi Kivity , Hollis Blanchard , Jerone Young , Joerg Roedel To: kvm-devel Return-path: Received: from fmmailgate02.web.de ([217.72.192.227]:33646 "EHLO fmmailgate02.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755253AbYEZWKE (ORCPT ); Mon, 26 May 2008 18:10:04 -0400 In-Reply-To: <4839B14A.3010406@web.de> Sender: kvm-owner@vger.kernel.org List-ID: This patch enhances QEMU's built-in debugger for SMP guest debugging. It allows to set the debugger focus explicitly via the monitor command "cpu", and it automatically switches the focus on breakpoint hit to the reporting CPU. Furthermore, the patch propagates breakpoint and watchpoint insertions or removals to all CPUs, not just the current one as it was the case so far. Signed-off-by: Jan Kiszka --- qemu/cpu-defs.h | 3 +++ qemu/exec.c | 30 +++++++++++++++++++++--------- qemu/gdbstub.c | 25 ++++++++++++------------- qemu/monitor.c | 19 ++++++++++++------- qemu/monitor.h | 7 +++++++ qemu/vl.c | 2 ++ 6 files changed, 57 insertions(+), 29 deletions(-) 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]; char *type; int ret; @@ -1100,11 +1101,11 @@ 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) { - switch (s->env->watchpoint[s->env->watchpoint_hit - 1].type) { + if (env->watchpoint_hit) { + switch (env->watchpoint[env->watchpoint_hit - 1].type) { case GDB_WATCHPOINT_READ: type = "r"; break; @@ -1117,12 +1118,12 @@ static void gdb_vm_stopped(void *opaque, } snprintf(buf, sizeof(buf), "T%02x%swatch:" TARGET_FMT_lx ";", SIGTRAP, type, - 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; @@ -1192,15 +1193,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; @@ -1360,7 +1361,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; @@ -1463,7 +1463,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) { kvm_save_registers(env); @@ -342,7 +347,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 */ Index: b/qemu/cpu-defs.h =================================================================== --- a/qemu/cpu-defs.h +++ b/qemu/cpu-defs.h @@ -170,3 +170,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/exec.c =================================================================== --- a/qemu/exec.c +++ b/qemu/exec.c @@ -1107,6 +1107,7 @@ static void breakpoint_invalidate(CPUSta int cpu_watchpoint_insert(CPUState *env, target_ulong addr, target_ulong len, int type) { + CPUState *env_iter; int i; if (type != GDB_WATCHPOINT_WRITE) @@ -1119,8 +1120,10 @@ int cpu_watchpoint_insert(CPUState *env, if (env->nb_watchpoints >= MAX_WATCHPOINTS) return -ENOBUFS; - i = env->nb_watchpoints++; - env->watchpoint[i].vaddr = addr; + foreach_cpu(env_iter) { + i = env_iter->nb_watchpoints++; + env_iter->watchpoint[i].vaddr = addr; + } tlb_flush_page(env, addr); /* FIXME: This flush is needed because of the hack to make memory ops terminate the TB. It can be removed once the proper IO trap and @@ -1133,6 +1136,7 @@ int cpu_watchpoint_insert(CPUState *env, int cpu_watchpoint_remove(CPUState *env, target_ulong addr, target_ulong len, int type) { + CPUState *env_iter; int i; if (type != GDB_WATCHPOINT_WRITE) @@ -1140,8 +1144,11 @@ int cpu_watchpoint_remove(CPUState *env, for (i = 0; i < env->nb_watchpoints; i++) { if (addr == env->watchpoint[i].vaddr) { - env->nb_watchpoints--; - env->watchpoint[i] = env->watchpoint[env->nb_watchpoints]; + foreach_cpu(env_iter) { + env_iter->nb_watchpoints--; + env_iter->watchpoint[i] = + env_iter->watchpoint[env_iter->nb_watchpoints]; + } tlb_flush_page(env, addr); return 0; } @@ -1155,6 +1162,7 @@ int cpu_breakpoint_insert(CPUState *env, int type) { #if defined(TARGET_HAS_ICE) + CPUState *env_iter; int i; for(i = 0; i < env->nb_breakpoints; i++) { @@ -1164,7 +1172,8 @@ int cpu_breakpoint_insert(CPUState *env, if (env->nb_breakpoints >= MAX_BREAKPOINTS) return -ENOBUFS; - env->breakpoints[env->nb_breakpoints++] = pc; + foreach_cpu(env_iter) + env_iter->breakpoints[env_iter->nb_breakpoints++] = pc; if (kvm_enabled()) kvm_update_debugger(env); @@ -1181,6 +1190,7 @@ int cpu_breakpoint_remove(CPUState *env, int type) { #if defined(TARGET_HAS_ICE) + CPUState *env_iter; int i; for(i = 0; i < env->nb_breakpoints; i++) { if (env->breakpoints[i] == pc) @@ -1188,13 +1198,15 @@ int cpu_breakpoint_remove(CPUState *env, } return -ENOENT; found: - env->nb_breakpoints--; - if (i < env->nb_breakpoints) - env->breakpoints[i] = env->breakpoints[env->nb_breakpoints]; + foreach_cpu(env_iter) { + env_iter->nb_breakpoints--; + env_iter->breakpoints[i] = + env_iter->breakpoints[env_iter->nb_breakpoints]; + } if (kvm_enabled()) kvm_update_debugger(env); - + breakpoint_invalidate(env, pc); return 0; #else