From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KAnC6-0006Cg-3K for qemu-devel@nongnu.org; Mon, 23 Jun 2008 10:38:58 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KAnC5-0006CI-4p for qemu-devel@nongnu.org; Mon, 23 Jun 2008 10:38:57 -0400 Received: from [199.232.76.173] (port=33450 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KAnC4-0006C5-T2 for qemu-devel@nongnu.org; Mon, 23 Jun 2008 10:38:56 -0400 Received: from gecko.sbs.de ([194.138.37.40]:19917) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1KAnC4-0004sJ-8o for qemu-devel@nongnu.org; Mon, 23 Jun 2008 10:38:56 -0400 Received: from mail1.sbs.de (localhost [127.0.0.1]) by gecko.sbs.de (8.12.11.20060308/8.12.11) with ESMTP id m5NEZnf3002107 for ; Mon, 23 Jun 2008 16:35:49 +0200 Received: from [139.25.109.167] (mchn012c.mchp.siemens.de [139.25.109.167] (may be forged)) by mail1.sbs.de (8.12.11.20060308/8.12.11) with ESMTP id m5NEZm8o017899 for ; Mon, 23 Jun 2008 16:35:48 +0200 Resent-To: qemu-devel@nongnu.org Resent-Message-Id: <485FB4C4.1010800@siemens.com> Message-ID: <485FB3BB.4060103@siemens.com> Date: Mon, 23 Jun 2008 16:31:23 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <485FB18E.1090801@siemens.com> In-Reply-To: <485FB18E.1090801@siemens.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] [PATCH 11/15] Improve debugging of SMP guests - v2 Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org 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. Without this property, SMP guest debugging is practically unfeasible. Signed-off-by: Jan Kiszka --- gdbstub.c | 85 ++++++++++++++++++++++++++++++++++++++++---------------------- monitor.c | 19 ++++++++----- monitor.h | 15 ++++++++++ vl.c | 2 + 4 files changed, 85 insertions(+), 36 deletions(-) Index: b/gdbstub.c =================================================================== --- a/gdbstub.c +++ b/gdbstub.c @@ -34,6 +34,7 @@ #include "sysemu.h" #include "gdbstub.h" #endif +#include "monitor.h" #include "qemu_socket.h" #ifdef _WIN32 @@ -58,7 +59,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; @@ -977,43 +977,71 @@ const int xlat_gdb_type[] = { }; #endif -static int gdb_breakpoint_insert(CPUState *env, target_ulong addr, - target_ulong len, int type) +static int gdb_breakpoint_insert(target_ulong addr, target_ulong len, int type) { + CPUState *env; + int err = 0; + switch (type) { case GDB_BREAKPOINT_SW ... GDB_BREAKPOINT_HW: - return cpu_breakpoint_insert(env, addr, BP_GDB, NULL); + for (env = first_cpu; env != NULL; env = env->next_cpu) { + err = cpu_breakpoint_insert(env, addr, BP_GDB, NULL); + if (err) + break; + } + return err; #ifndef CONFIG_USER_ONLY case GDB_WATCHPOINT_WRITE ... GDB_WATCHPOINT_ACCESS: - return cpu_watchpoint_insert(env, addr, len, xlat_gdb_type[type], - NULL); + for (env = first_cpu; env != NULL; env = env->next_cpu) { + err = cpu_watchpoint_insert(env, addr, len, xlat_gdb_type[type], + NULL); + if (err) + break; + } + return err; #endif default: return -ENOSYS; } } -static int gdb_breakpoint_remove(CPUState *env, target_ulong addr, - target_ulong len, int type) +static int gdb_breakpoint_remove(target_ulong addr, target_ulong len, int type) { + CPUState *env; + int err = 0; + switch (type) { case GDB_BREAKPOINT_SW ... GDB_BREAKPOINT_HW: - return cpu_breakpoint_remove(env, addr, BP_GDB); + for (env = first_cpu; env != NULL; env = env->next_cpu) { + err = cpu_breakpoint_remove(env, addr, BP_GDB); + if (err) + break; + } + return err; #ifndef CONFIG_USER_ONLY case GDB_WATCHPOINT_WRITE ... GDB_WATCHPOINT_ACCESS: - return cpu_watchpoint_remove(env, addr, len, xlat_gdb_type[type]); + for (env = first_cpu; env != NULL; env = env->next_cpu) { + err = cpu_watchpoint_remove(env, addr, len, xlat_gdb_type[type]); + if (err) + break; + } + return err; #endif default: return -ENOSYS; } } -static void gdb_breakpoint_remove_all(CPUState *env) +static void gdb_breakpoint_remove_all(void) { - cpu_breakpoint_remove_all(env, BP_GDB); + CPUState *env; + + for (env = first_cpu; env != NULL; env = env->next_cpu) { + cpu_breakpoint_remove_all(env, BP_GDB); #ifndef CONFIG_USER_ONLY - cpu_watchpoint_remove_all(env, BP_GDB); + cpu_watchpoint_remove_all(env, BP_GDB); #endif + } } static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf) @@ -1039,7 +1067,7 @@ static int gdb_handle_packet(GDBState *s * because gdb is doing and initial connect and the state * should be cleaned up. */ - gdb_breakpoint_remove_all(env); + gdb_breakpoint_remove_all(); break; case 'c': if (*p != '\0') { @@ -1073,7 +1101,7 @@ static int gdb_handle_packet(GDBState *s exit(0); case 'D': /* Detach packet */ - gdb_breakpoint_remove_all(env); + gdb_breakpoint_remove_all(); gdb_continue(s); put_packet(s, "OK"); break; @@ -1116,7 +1144,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 { @@ -1171,9 +1199,9 @@ static int gdb_handle_packet(GDBState *s p++; len = strtoull(p, (char **)&p, 16); if (ch == 'Z') - res = gdb_breakpoint_insert(env, addr, len, type); + res = gdb_breakpoint_insert(addr, len, type); else - res = gdb_breakpoint_remove(env, addr, len, type); + res = gdb_breakpoint_remove(addr, len, type); if (res >= 0) put_packet(s, "OK"); else if (res == -ENOSYS) @@ -1237,6 +1265,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]; const char *type; int ret; @@ -1245,11 +1274,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_hit->flags & BP_MEM_ACCESS) { + if (env->watchpoint_hit) { + switch (env->watchpoint_hit->flags & BP_MEM_ACCESS) { case BP_MEM_READ: type = "r"; break; @@ -1261,12 +1290,12 @@ static void gdb_vm_stopped(void *opaque, break; } snprintf(buf, sizeof(buf), "T%02x%swatch:" TARGET_FMT_lx ";", - SIGTRAP, type, s->env->watchpoint_hit->vaddr); + SIGTRAP, type, env->watchpoint_hit->vaddr); put_packet(s, buf); - s->env->watchpoint_hit = NULL; + env->watchpoint_hit = NULL; return; } - tb_flush(s->env); + tb_flush(env); ret = SIGTRAP; } else if (reason == EXCP_INTERRUPT) { ret = SIGINT; @@ -1336,15 +1365,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; @@ -1508,7 +1537,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; @@ -1611,7 +1639,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/monitor.c =================================================================== --- a/monitor.c +++ b/monitor.c @@ -261,8 +261,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; @@ -275,10 +274,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); } return mon_cpu; } @@ -302,8 +307,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:", @@ -326,7 +331,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/monitor.h =================================================================== --- /dev/null +++ b/monitor.h @@ -0,0 +1,15 @@ +#ifndef QEMU_MONITOR_H +#define QEMU_MONITOR_H + +void mon_set_cpu(CPUState *env); + +#ifdef CONFIG_USER_ONLY +static inline CPUState *mon_get_cpu(void) +{ + return first_cpu; +} +#else +CPUState *mon_get_cpu(void); +#endif + +#endif /* QEMU_MONITOR_H */ Index: b/vl.c =================================================================== --- a/vl.c +++ b/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" @@ -7112,6 +7113,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 */