* [PATCH 01/17] kvm-userspace: Remove old guest debugging hooks
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-06 9:14 ` [PATCH 02/17] qemu: Return appropriate watch message to gdb Jan Kiszka
` (17 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: kvm-qemu-remove-breakpoint-hooks.patch --]
[-- Type: text/plain, Size: 1912 bytes --]
Prepare to apply the QEMU debugging series by removing the old guest
debugging hooks for KVM.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/exec.c | 8 --------
qemu/qemu-kvm.c | 7 +------
2 files changed, 1 insertion(+), 14 deletions(-)
Index: b/qemu/exec.c
===================================================================
--- a/qemu/exec.c
+++ b/qemu/exec.c
@@ -1390,9 +1390,6 @@ int cpu_breakpoint_insert(CPUState *env,
return -1;
env->breakpoints[env->nb_breakpoints++] = pc;
- if (kvm_enabled())
- kvm_update_debugger(env);
-
breakpoint_invalidate(env, pc);
return 0;
#else
@@ -1426,9 +1423,6 @@ int cpu_breakpoint_remove(CPUState *env,
if (i < env->nb_breakpoints)
env->breakpoints[i] = env->breakpoints[env->nb_breakpoints];
- if (kvm_enabled())
- kvm_update_debugger(env);
-
breakpoint_invalidate(env, pc);
return 0;
#else
@@ -1447,8 +1441,6 @@ void cpu_single_step(CPUState *env, int
/* XXX: only flush what is necessary */
tb_flush(env);
}
- if (kvm_enabled())
- kvm_update_debugger(env);
#endif
}
Index: b/qemu/qemu-kvm.c
===================================================================
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -851,17 +851,12 @@ void kvm_invoke_guest_debug(void *data)
int kvm_update_debugger(CPUState *env)
{
struct kvm_guest_debug_data data;
- int i;
memset(data.dbg.breakpoints, 0, sizeof(data.dbg.breakpoints));
data.dbg.enabled = 0;
- if (env->nb_breakpoints || env->singlestep_enabled) {
+ if (env->singlestep_enabled) {
data.dbg.enabled = 1;
- for (i = 0; i < 4 && i < env->nb_breakpoints; ++i) {
- data.dbg.breakpoints[i].enabled = 1;
- data.dbg.breakpoints[i].address = env->breakpoints[i];
- }
data.dbg.singlestep = env->singlestep_enabled;
}
on_vcpu(env, kvm_invoke_guest_debug, &data);
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 02/17] qemu: Return appropriate watch message to gdb
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
2008-10-06 9:14 ` [PATCH 01/17] kvm-userspace: Remove old guest debugging hooks Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-06 9:14 ` [PATCH 03/17] qemu: Refactor and enhance break/watchpoint API Jan Kiszka
` (16 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: qemu-fix-gdbstub-stop-reply.patch --]
[-- Type: text/plain, Size: 1465 bytes --]
Return the appropriate type prefix (r, a, none) when reporting
watchpoint hits to the gdb front-end.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/gdbstub.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
Index: b/qemu/gdbstub.c
===================================================================
--- a/qemu/gdbstub.c
+++ b/qemu/gdbstub.c
@@ -1232,6 +1232,7 @@ static void gdb_vm_stopped(void *opaque,
{
GDBState *s = opaque;
char buf[256];
+ const char *type;
int ret;
if (s->state == RS_SYSCALL)
@@ -1242,8 +1243,20 @@ static void gdb_vm_stopped(void *opaque,
if (reason == EXCP_DEBUG) {
if (s->env->watchpoint_hit) {
- snprintf(buf, sizeof(buf), "T%02xwatch:" TARGET_FMT_lx ";",
- SIGTRAP,
+ switch (s->env->watchpoint[s->env->watchpoint_hit - 1].flags &
+ (PAGE_READ | PAGE_WRITE)) {
+ case PAGE_READ:
+ type = "r";
+ break;
+ case PAGE_READ | PAGE_WRITE:
+ type = "a";
+ break;
+ default:
+ type = "";
+ break;
+ }
+ snprintf(buf, sizeof(buf), "T%02x%swatch:" TARGET_FMT_lx ";",
+ SIGTRAP, type,
s->env->watchpoint[s->env->watchpoint_hit - 1].vaddr);
put_packet(s, buf);
s->env->watchpoint_hit = 0;
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 03/17] qemu: Refactor and enhance break/watchpoint API
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
2008-10-06 9:14 ` [PATCH 01/17] kvm-userspace: Remove old guest debugging hooks Jan Kiszka
2008-10-06 9:14 ` [PATCH 02/17] qemu: Return appropriate watch message to gdb Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-06 9:14 ` [PATCH 04/17] qemu: Set mem_io_vaddr on io_read Jan Kiszka
` (15 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: qemu-improve-breakpoint-watchpoint-interface.patch --]
[-- Type: text/plain, Size: 30833 bytes --]
This patch prepares the QEMU cpu_watchpoint/breakpoint API to allow the
succeeding enhancements this series comes with.
First of all, it overcomes MAX_BREAKPOINTS/MAX_WATCHPOINTS by switching
to dynamically allocated data structures that are kept in linked lists.
This also allows to return a stable reference to the related objects,
required for later introduced x86 debug register support.
Breakpoints and watchpoints are stored with their full information set
and an additional flag field that makes them easily extensible for use
beyond pure guest debugging.
Finally, this restructuring lays the foundation for KVM to hook into
the debugging infrastructure, providing its own services where hardware
virtualization demands it. Once QEMUAccel is considered for merge,
those entry point should be included into its abstraction layer so that
accellerators can hook in even more cleanly.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/cpu-all.h | 23 +++-
qemu/cpu-defs.h | 26 +++--
qemu/exec.c | 201 +++++++++++++++++++++++++-----------------
qemu/gdbstub.c | 131 ++++++++++++++-------------
qemu/target-alpha/translate.c | 7 -
qemu/target-arm/translate.c | 9 +
qemu/target-cris/translate.c | 9 +
qemu/target-i386/translate.c | 7 -
qemu/target-m68k/translate.c | 9 +
qemu/target-mips/translate.c | 7 -
qemu/target-ppc/translate.c | 7 -
qemu/target-sh4/translate.c | 7 -
qemu/target-sparc/translate.c | 7 -
13 files changed, 264 insertions(+), 186 deletions(-)
Index: b/qemu/cpu-all.h
===================================================================
--- a/qemu/cpu-all.h
+++ b/qemu/cpu-all.h
@@ -761,12 +761,23 @@ extern int use_icount;
void cpu_interrupt(CPUState *s, int mask);
void cpu_reset_interrupt(CPUState *env, int mask);
-int cpu_watchpoint_insert(CPUState *env, target_ulong addr, int type);
-int cpu_watchpoint_remove(CPUState *env, target_ulong addr);
-void cpu_watchpoint_remove_all(CPUState *env);
-int cpu_breakpoint_insert(CPUState *env, target_ulong pc);
-int cpu_breakpoint_remove(CPUState *env, target_ulong pc);
-void cpu_breakpoint_remove_all(CPUState *env);
+/* Breakpoint/watchpoint flags */
+#define BP_MEM_READ 0x01
+#define BP_MEM_WRITE 0x02
+#define BP_MEM_ACCESS (BP_MEM_READ | BP_MEM_WRITE)
+#define BP_GDB 0x10
+
+int cpu_breakpoint_insert(CPUState *env, target_ulong pc, int flags,
+ CPUBreakpoint **breakpoint);
+int cpu_breakpoint_remove(CPUState *env, target_ulong pc, int flags);
+void cpu_breakpoint_remove_by_ref(CPUState *env, CPUBreakpoint *breakpoint);
+void cpu_breakpoint_remove_all(CPUState *env, int mask);
+int cpu_watchpoint_insert(CPUState *env, target_ulong addr, target_ulong len,
+ int flags, CPUWatchpoint **watchpoint);
+int cpu_watchpoint_remove(CPUState *env, target_ulong addr,
+ target_ulong len, int flags);
+void cpu_watchpoint_remove_by_ref(CPUState *env, CPUWatchpoint *watchpoint);
+void cpu_watchpoint_remove_all(CPUState *env, int mask);
#define SSTEP_ENABLE 0x1 /* Enable simulated HW single stepping */
#define SSTEP_NOIRQ 0x2 /* Do not use IRQ while single stepping */
Index: b/qemu/exec.c
===================================================================
--- a/qemu/exec.c
+++ b/qemu/exec.c
@@ -547,7 +547,6 @@ void cpu_exec_init(CPUState *env)
cpu_index++;
}
env->cpu_index = cpu_index;
- env->nb_watchpoints = 0;
#ifdef __WIN32
env->thread_id = GetCurrentProcessId();
#else
@@ -1326,107 +1325,150 @@ static void breakpoint_invalidate(CPUSta
#endif
/* Add a watchpoint. */
-int cpu_watchpoint_insert(CPUState *env, target_ulong addr, int type)
+int cpu_watchpoint_insert(CPUState *env, target_ulong addr, target_ulong len,
+ int flags, CPUWatchpoint **watchpoint)
{
- int i;
+ CPUWatchpoint *wp;
- for (i = 0; i < env->nb_watchpoints; i++) {
- if (addr == env->watchpoint[i].vaddr)
- return 0;
- }
- if (env->nb_watchpoints >= MAX_WATCHPOINTS)
- return -1;
+ wp = qemu_malloc(sizeof(*wp));
+ if (!wp)
+ return -ENOBUFS;
+
+ wp->vaddr = addr;
+ wp->len = len;
+ wp->flags = flags;
+
+ wp->next = env->watchpoints;
+ wp->prev = NULL;
+ if (wp->next)
+ wp->next->prev = wp;
+ env->watchpoints = wp;
- i = env->nb_watchpoints++;
- env->watchpoint[i].vaddr = addr;
- env->watchpoint[i].type = type;
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
re-execute bits are in. */
tb_flush(env);
- return i;
-}
-/* Remove a watchpoint. */
-int cpu_watchpoint_remove(CPUState *env, target_ulong addr)
-{
- int i;
+ if (watchpoint)
+ *watchpoint = wp;
+ return 0;
+}
- 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];
- tlb_flush_page(env, addr);
+/* Remove a specific watchpoint. */
+int cpu_watchpoint_remove(CPUState *env, target_ulong addr, target_ulong len,
+ int flags)
+{
+ CPUWatchpoint *wp;
+
+ for (wp = env->watchpoints; wp != NULL; wp = wp->next) {
+ if (addr == wp->vaddr && len == wp->len && flags == wp->flags) {
+ cpu_watchpoint_remove_by_ref(env, wp);
return 0;
}
}
- return -1;
+ return -ENOENT;
}
-/* Remove all watchpoints. */
-void cpu_watchpoint_remove_all(CPUState *env) {
- int i;
+/* Remove a specific watchpoint by reference. */
+void cpu_watchpoint_remove_by_ref(CPUState *env, CPUWatchpoint *watchpoint)
+{
+ if (watchpoint->next)
+ watchpoint->next->prev = watchpoint->prev;
+ if (watchpoint->prev)
+ watchpoint->prev->next = watchpoint->next;
+ else
+ env->watchpoints = watchpoint->next;
- for (i = 0; i < env->nb_watchpoints; i++) {
- tlb_flush_page(env, env->watchpoint[i].vaddr);
- }
- env->nb_watchpoints = 0;
+ tlb_flush_page(env, watchpoint->vaddr);
+
+ qemu_free(watchpoint);
}
-/* add a breakpoint. EXCP_DEBUG is returned by the CPU loop if a
- breakpoint is reached */
-int cpu_breakpoint_insert(CPUState *env, target_ulong pc)
+/* Remove all matching watchpoints. */
+void cpu_watchpoint_remove_all(CPUState *env, int mask)
{
-#if defined(TARGET_HAS_ICE)
- int i;
+ CPUWatchpoint *wp;
- for(i = 0; i < env->nb_breakpoints; i++) {
- if (env->breakpoints[i] == pc)
- return 0;
- }
+ for (wp = env->watchpoints; wp != NULL; wp = wp->next)
+ if (wp->flags & mask)
+ cpu_watchpoint_remove_by_ref(env, wp);
+}
- if (env->nb_breakpoints >= MAX_BREAKPOINTS)
- return -1;
- env->breakpoints[env->nb_breakpoints++] = pc;
+/* Add a breakpoint. */
+int cpu_breakpoint_insert(CPUState *env, target_ulong pc, int flags,
+ CPUBreakpoint **breakpoint)
+{
+#if defined(TARGET_HAS_ICE)
+ CPUBreakpoint *bp;
+
+ bp = qemu_malloc(sizeof(*bp));
+ if (!bp)
+ return -ENOBUFS;
+
+ bp->pc = pc;
+ bp->flags = flags;
+
+ bp->next = env->breakpoints;
+ bp->prev = NULL;
+ if (bp->next)
+ bp->next->prev = bp;
+ env->breakpoints = bp;
breakpoint_invalidate(env, pc);
+
+ if (breakpoint)
+ *breakpoint = bp;
return 0;
#else
- return -1;
+ return -ENOSYS;
#endif
}
-/* remove all breakpoints */
-void cpu_breakpoint_remove_all(CPUState *env) {
+/* Remove a specific breakpoint. */
+int cpu_breakpoint_remove(CPUState *env, target_ulong pc, int flags)
+{
#if defined(TARGET_HAS_ICE)
- int i;
- for(i = 0; i < env->nb_breakpoints; i++) {
- breakpoint_invalidate(env, env->breakpoints[i]);
+ CPUBreakpoint *bp;
+
+ for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
+ if (bp->pc == pc && bp->flags == flags) {
+ cpu_breakpoint_remove_by_ref(env, bp);
+ return 0;
+ }
}
- env->nb_breakpoints = 0;
+ return -ENOENT;
+#else
+ return -ENOSYS;
#endif
}
-/* remove a breakpoint */
-int cpu_breakpoint_remove(CPUState *env, target_ulong pc)
+/* Remove a specific breakpoint by reference. */
+void cpu_breakpoint_remove_by_ref(CPUState *env, CPUBreakpoint *breakpoint)
{
#if defined(TARGET_HAS_ICE)
- int i;
- for(i = 0; i < env->nb_breakpoints; i++) {
- if (env->breakpoints[i] == pc)
- goto found;
- }
- return -1;
- found:
- env->nb_breakpoints--;
- if (i < env->nb_breakpoints)
- env->breakpoints[i] = env->breakpoints[env->nb_breakpoints];
+ if (breakpoint->next)
+ breakpoint->next->prev = breakpoint->prev;
+ if (breakpoint->prev)
+ breakpoint->prev->next = breakpoint->next;
+ else
+ env->breakpoints = breakpoint->next;
- breakpoint_invalidate(env, pc);
- return 0;
-#else
- return -1;
+ breakpoint_invalidate(env, breakpoint->pc);
+
+ qemu_free(breakpoint);
+#endif
+}
+
+/* Remove all matching breakpoints. */
+void cpu_breakpoint_remove_all(CPUState *env, int mask)
+{
+#if defined(TARGET_HAS_ICE)
+ CPUBreakpoint *bp;
+
+ for (bp = env->breakpoints; bp != NULL; bp = bp->next)
+ if (bp->flags & mask)
+ cpu_breakpoint_remove_by_ref(env, bp);
#endif
}
@@ -1914,7 +1956,7 @@ int tlb_set_page_exec(CPUState *env, tar
target_phys_addr_t addend;
int ret;
CPUTLBEntry *te;
- int i;
+ CPUWatchpoint *wp;
target_phys_addr_t iotlb;
p = phys_page_find(paddr >> TARGET_PAGE_BITS);
@@ -1955,8 +1997,8 @@ int tlb_set_page_exec(CPUState *env, tar
code_address = address;
/* Make accesses to pages with watchpoints go via the
watchpoint trap routines. */
- for (i = 0; i < env->nb_watchpoints; i++) {
- if (vaddr == (env->watchpoint[i].vaddr & TARGET_PAGE_MASK)) {
+ for (wp = env->watchpoints; wp != NULL; wp = wp->next) {
+ if (vaddr == (wp->vaddr & TARGET_PAGE_MASK)) {
iotlb = io_mem_watch + paddr;
/* TODO: The memory case can be optimized by not trapping
reads of pages with a write breakpoint. */
@@ -2452,13 +2494,12 @@ static void check_watchpoint(int offset,
{
CPUState *env = cpu_single_env;
target_ulong vaddr;
- int i;
+ CPUWatchpoint *wp;
vaddr = (env->mem_io_vaddr & TARGET_PAGE_MASK) + offset;
- for (i = 0; i < env->nb_watchpoints; i++) {
- if (vaddr == env->watchpoint[i].vaddr
- && (env->watchpoint[i].type & flags)) {
- env->watchpoint_hit = i + 1;
+ for (wp = env->watchpoints; wp != NULL; wp = wp->next) {
+ if (vaddr == wp->vaddr && (wp->flags & flags)) {
+ env->watchpoint_hit = wp;
cpu_interrupt(env, CPU_INTERRUPT_DEBUG);
break;
}
@@ -2470,40 +2511,40 @@ static void check_watchpoint(int offset,
phys routines. */
static uint32_t watch_mem_readb(void *opaque, target_phys_addr_t addr)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, PAGE_READ);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_READ);
return ldub_phys(addr);
}
static uint32_t watch_mem_readw(void *opaque, target_phys_addr_t addr)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, PAGE_READ);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_READ);
return lduw_phys(addr);
}
static uint32_t watch_mem_readl(void *opaque, target_phys_addr_t addr)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, PAGE_READ);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_READ);
return ldl_phys(addr);
}
static void watch_mem_writeb(void *opaque, target_phys_addr_t addr,
uint32_t val)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, PAGE_WRITE);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_WRITE);
stb_phys(addr, val);
}
static void watch_mem_writew(void *opaque, target_phys_addr_t addr,
uint32_t val)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, PAGE_WRITE);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_WRITE);
stw_phys(addr, val);
}
static void watch_mem_writel(void *opaque, target_phys_addr_t addr,
uint32_t val)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, PAGE_WRITE);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_WRITE);
stl_phys(addr, val);
}
Index: b/qemu/gdbstub.c
===================================================================
--- a/qemu/gdbstub.c
+++ b/qemu/gdbstub.c
@@ -963,10 +963,64 @@ static void cpu_gdb_write_registers(CPUS
#endif
+/* GDB breakpoint/watchpoint types */
+#define GDB_BREAKPOINT_SW 0
+#define GDB_BREAKPOINT_HW 1
+#define GDB_WATCHPOINT_WRITE 2
+#define GDB_WATCHPOINT_READ 3
+#define GDB_WATCHPOINT_ACCESS 4
+
+#ifndef CONFIG_USER_ONLY
+static const int xlat_gdb_type[] = {
+ [GDB_WATCHPOINT_WRITE] = BP_GDB | BP_MEM_WRITE,
+ [GDB_WATCHPOINT_READ] = BP_GDB | BP_MEM_READ,
+ [GDB_WATCHPOINT_ACCESS] = BP_GDB | BP_MEM_ACCESS,
+};
+#endif
+
+static int gdb_breakpoint_insert(CPUState *env, target_ulong addr,
+ target_ulong len, int type)
+{
+ switch (type) {
+ case GDB_BREAKPOINT_SW ... GDB_BREAKPOINT_HW:
+ return cpu_breakpoint_insert(env, addr, BP_GDB, NULL);
+#ifndef CONFIG_USER_ONLY
+ case GDB_WATCHPOINT_WRITE ... GDB_WATCHPOINT_ACCESS:
+ return cpu_watchpoint_insert(env, addr, len, xlat_gdb_type[type],
+ NULL);
+#endif
+ default:
+ return -ENOSYS;
+ }
+}
+
+static int gdb_breakpoint_remove(CPUState *env, target_ulong addr,
+ target_ulong len, int type)
+{
+ switch (type) {
+ case GDB_BREAKPOINT_SW ... GDB_BREAKPOINT_HW:
+ return cpu_breakpoint_remove(env, addr, BP_GDB);
+#ifndef CONFIG_USER_ONLY
+ case GDB_WATCHPOINT_WRITE ... GDB_WATCHPOINT_ACCESS:
+ return cpu_watchpoint_remove(env, addr, len, xlat_gdb_type[type]);
+#endif
+ default:
+ return -ENOSYS;
+ }
+}
+
+static void gdb_breakpoint_remove_all(CPUState *env)
+{
+ cpu_breakpoint_remove_all(env, BP_GDB);
+#ifndef CONFIG_USER_ONLY
+ cpu_watchpoint_remove_all(env, BP_GDB);
+#endif
+}
+
static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf)
{
const char *p;
- int ch, reg_size, type;
+ int ch, reg_size, type, res;
char buf[4096];
uint8_t mem_buf[4096];
uint32_t *registers;
@@ -986,8 +1040,7 @@ static int gdb_handle_packet(GDBState *s
* because gdb is doing and initial connect and the state
* should be cleaned up.
*/
- cpu_breakpoint_remove_all(env);
- cpu_watchpoint_remove_all(env);
+ gdb_breakpoint_remove_all(env);
break;
case 'c':
if (*p != '\0') {
@@ -1023,8 +1076,7 @@ static int gdb_handle_packet(GDBState *s
exit(0);
case 'D':
/* Detach packet */
- cpu_breakpoint_remove_all(env);
- cpu_watchpoint_remove_all(env);
+ gdb_breakpoint_remove_all(env);
gdb_continue(s);
put_packet(s, "OK");
break;
@@ -1117,44 +1169,6 @@ static int gdb_handle_packet(GDBState *s
put_packet(s, "OK");
break;
case 'Z':
- type = strtoul(p, (char **)&p, 16);
- if (*p == ',')
- p++;
- addr = strtoull(p, (char **)&p, 16);
- if (*p == ',')
- p++;
- len = strtoull(p, (char **)&p, 16);
- switch (type) {
- case 0:
- case 1:
- if (cpu_breakpoint_insert(env, addr) < 0)
- goto breakpoint_error;
- put_packet(s, "OK");
- break;
-#ifndef CONFIG_USER_ONLY
- case 2:
- type = PAGE_WRITE;
- goto insert_watchpoint;
- case 3:
- type = PAGE_READ;
- goto insert_watchpoint;
- case 4:
- type = PAGE_READ | PAGE_WRITE;
- insert_watchpoint:
- if (cpu_watchpoint_insert(env, addr, type) < 0)
- goto breakpoint_error;
- put_packet(s, "OK");
- break;
-#endif
- default:
- put_packet(s, "");
- break;
- }
- break;
- breakpoint_error:
- put_packet(s, "E22");
- break;
-
case 'z':
type = strtoul(p, (char **)&p, 16);
if (*p == ',')
@@ -1163,17 +1177,16 @@ static int gdb_handle_packet(GDBState *s
if (*p == ',')
p++;
len = strtoull(p, (char **)&p, 16);
- if (type == 0 || type == 1) {
- cpu_breakpoint_remove(env, addr);
- put_packet(s, "OK");
-#ifndef CONFIG_USER_ONLY
- } else if (type >= 2 || type <= 4) {
- cpu_watchpoint_remove(env, addr);
- put_packet(s, "OK");
-#endif
- } else {
+ if (ch == 'Z')
+ res = gdb_breakpoint_insert(env, addr, len, type);
+ else
+ res = gdb_breakpoint_remove(env, addr, len, type);
+ if (res >= 0)
+ put_packet(s, "OK");
+ else if (res == -ENOSYS)
put_packet(s, "");
- }
+ else
+ put_packet(s, "E22");
break;
case 'q':
case 'Q':
@@ -1243,12 +1256,11 @@ static void gdb_vm_stopped(void *opaque,
if (reason == EXCP_DEBUG) {
if (s->env->watchpoint_hit) {
- switch (s->env->watchpoint[s->env->watchpoint_hit - 1].flags &
- (PAGE_READ | PAGE_WRITE)) {
- case PAGE_READ:
+ switch (s->env->watchpoint_hit->flags & BP_MEM_ACCESS) {
+ case BP_MEM_READ:
type = "r";
break;
- case PAGE_READ | PAGE_WRITE:
+ case BP_MEM_ACCESS:
type = "a";
break;
default:
@@ -1256,10 +1268,9 @@ static void gdb_vm_stopped(void *opaque,
break;
}
snprintf(buf, sizeof(buf), "T%02x%swatch:" TARGET_FMT_lx ";",
- SIGTRAP, type,
- s->env->watchpoint[s->env->watchpoint_hit - 1].vaddr);
+ SIGTRAP, type, s->env->watchpoint_hit->vaddr);
put_packet(s, buf);
- s->env->watchpoint_hit = 0;
+ s->env->watchpoint_hit = NULL;
return;
}
tb_flush(s->env);
Index: b/qemu/cpu-defs.h
===================================================================
--- a/qemu/cpu-defs.h
+++ b/qemu/cpu-defs.h
@@ -82,8 +82,6 @@ typedef uint64_t target_phys_addr_t;
#define EXCP_HLT 0x10001 /* hlt instruction reached */
#define EXCP_DEBUG 0x10002 /* cpu stopped after a breakpoint or singlestep */
#define EXCP_HALTED 0x10003 /* cpu is halted (waiting for external event) */
-#define MAX_BREAKPOINTS 32
-#define MAX_WATCHPOINTS 32
#define TB_JMP_CACHE_BITS 12
#define TB_JMP_CACHE_SIZE (1 << TB_JMP_CACHE_BITS)
@@ -142,6 +140,19 @@ typedef struct icount_decr_u16 {
} icount_decr_u16;
#endif
+typedef struct CPUBreakpoint {
+ target_ulong pc;
+ int flags; /* BP_* */
+ struct CPUBreakpoint *prev, *next;
+} CPUBreakpoint;
+
+typedef struct CPUWatchpoint {
+ target_ulong vaddr;
+ target_ulong len;
+ int flags; /* BP_* */
+ struct CPUWatchpoint *prev, *next;
+} CPUWatchpoint;
+
#define CPU_TEMP_BUF_NLONGS 128
#define CPU_COMMON \
struct TranslationBlock *current_tb; /* currently executing TB */ \
@@ -174,16 +185,11 @@ typedef struct icount_decr_u16 {
\
/* from this point: preserved by CPU reset */ \
/* ice debug support */ \
- target_ulong breakpoints[MAX_BREAKPOINTS]; \
- int nb_breakpoints; \
+ CPUBreakpoint *breakpoints; \
int singlestep_enabled; \
\
- struct { \
- target_ulong vaddr; \
- int type; /* PAGE_READ/PAGE_WRITE */ \
- } watchpoint[MAX_WATCHPOINTS]; \
- int nb_watchpoints; \
- int watchpoint_hit; \
+ CPUWatchpoint *watchpoints; \
+ CPUWatchpoint *watchpoint_hit; \
\
/* Core interrupt code */ \
jmp_buf jmp_env; \
Index: b/qemu/target-i386/translate.c
===================================================================
--- a/qemu/target-i386/translate.c
+++ b/qemu/target-i386/translate.c
@@ -7533,6 +7533,7 @@ static inline void gen_intermediate_code
DisasContext dc1, *dc = &dc1;
target_ulong pc_ptr;
uint16_t *gen_opc_end;
+ CPUBreakpoint *bp;
int j, lj, cflags;
uint64_t flags;
target_ulong pc_start;
@@ -7616,9 +7617,9 @@ static inline void gen_intermediate_code
gen_icount_start();
for(;;) {
- if (env->nb_breakpoints > 0) {
- for(j = 0; j < env->nb_breakpoints; j++) {
- if (env->breakpoints[j] == pc_ptr) {
+ if (unlikely(env->breakpoints)) {
+ for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
+ if (bp->pc == pc_ptr) {
gen_debug(dc, pc_ptr - dc->cs_base);
break;
}
Index: b/qemu/target-alpha/translate.c
===================================================================
--- a/qemu/target-alpha/translate.c
+++ b/qemu/target-alpha/translate.c
@@ -2252,6 +2252,7 @@ static always_inline void gen_intermedia
target_ulong pc_start;
uint32_t insn;
uint16_t *gen_opc_end;
+ CPUBreakpoint *bp;
int j, lj = -1;
int ret;
int num_insns;
@@ -2274,9 +2275,9 @@ static always_inline void gen_intermedia
gen_icount_start();
for (ret = 0; ret == 0;) {
- if (env->nb_breakpoints > 0) {
- for(j = 0; j < env->nb_breakpoints; j++) {
- if (env->breakpoints[j] == ctx.pc) {
+ if (unlikely(env->breakpoints)) {
+ for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
+ if (bp->pc == ctx.pc) {
gen_excp(&ctx, EXCP_DEBUG, 0);
break;
}
Index: b/qemu/target-arm/translate.c
===================================================================
--- a/qemu/target-arm/translate.c
+++ b/qemu/target-arm/translate.c
@@ -8544,6 +8544,7 @@ static inline void gen_intermediate_code
int search_pc)
{
DisasContext dc1, *dc = &dc1;
+ CPUBreakpoint *bp;
uint16_t *gen_opc_end;
int j, lj;
target_ulong pc_start;
@@ -8620,9 +8621,9 @@ static inline void gen_intermediate_code
}
#endif
- if (env->nb_breakpoints > 0) {
- for(j = 0; j < env->nb_breakpoints; j++) {
- if (env->breakpoints[j] == dc->pc) {
+ if (unlikely(env->breakpoints)) {
+ for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
+ if (bp->pc == dc->pc) {
gen_set_condexec(dc);
gen_set_pc_im(dc->pc);
gen_exception(EXCP_DEBUG);
@@ -8675,7 +8676,7 @@ static inline void gen_intermediate_code
/* Terminate the TB on memory ops if watchpoints are present. */
/* FIXME: This should be replacd by the deterministic execution
* IRQ raising bits. */
- if (dc->is_mem && env->nb_watchpoints)
+ if (dc->is_mem && env->watchpoints)
break;
/* Translation stops when a conditional branch is enoutered.
Index: b/qemu/target-cris/translate.c
===================================================================
--- a/qemu/target-cris/translate.c
+++ b/qemu/target-cris/translate.c
@@ -2989,10 +2989,11 @@ cris_decoder(DisasContext *dc)
static void check_breakpoint(CPUState *env, DisasContext *dc)
{
- int j;
- if (env->nb_breakpoints > 0) {
- for(j = 0; j < env->nb_breakpoints; j++) {
- if (env->breakpoints[j] == dc->pc) {
+ CPUBreakpoint *bp;
+
+ if (unlikely(env->breakpoints)) {
+ for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
+ if (bp->pc == dc->pc) {
cris_evaluate_flags (dc);
tcg_gen_movi_tl(env_pc, dc->pc);
t_gen_raise_exception(EXCP_DEBUG);
Index: b/qemu/target-m68k/translate.c
===================================================================
--- a/qemu/target-m68k/translate.c
+++ b/qemu/target-m68k/translate.c
@@ -2915,6 +2915,7 @@ gen_intermediate_code_internal(CPUState
{
DisasContext dc1, *dc = &dc1;
uint16_t *gen_opc_end;
+ CPUBreakpoint *bp;
int j, lj;
target_ulong pc_start;
int pc_offset;
@@ -2948,9 +2949,9 @@ gen_intermediate_code_internal(CPUState
do {
pc_offset = dc->pc - pc_start;
gen_throws_exception = NULL;
- if (env->nb_breakpoints > 0) {
- for(j = 0; j < env->nb_breakpoints; j++) {
- if (env->breakpoints[j] == dc->pc) {
+ if (unlikely(env->breakpoints)) {
+ for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
+ if (bp->pc == dc->pc) {
gen_exception(dc, dc->pc, EXCP_DEBUG);
dc->is_jmp = DISAS_JUMP;
break;
@@ -2980,7 +2981,7 @@ gen_intermediate_code_internal(CPUState
/* Terminate the TB on memory ops if watchpoints are present. */
/* FIXME: This should be replaced by the deterministic execution
* IRQ raising bits. */
- if (dc->is_mem && env->nb_watchpoints)
+ if (dc->is_mem && env->watchpoints)
break;
} while (!dc->is_jmp && gen_opc_ptr < gen_opc_end &&
!env->singlestep_enabled &&
Index: b/qemu/target-mips/translate.c
===================================================================
--- a/qemu/target-mips/translate.c
+++ b/qemu/target-mips/translate.c
@@ -8442,6 +8442,7 @@ gen_intermediate_code_internal (CPUState
DisasContext ctx;
target_ulong pc_start;
uint16_t *gen_opc_end;
+ CPUBreakpoint *bp;
int j, lj = -1;
int num_insns;
int max_insns;
@@ -8481,9 +8482,9 @@ gen_intermediate_code_internal (CPUState
#endif
gen_icount_start();
while (ctx.bstate == BS_NONE) {
- if (env->nb_breakpoints > 0) {
- for(j = 0; j < env->nb_breakpoints; j++) {
- if (env->breakpoints[j] == ctx.pc) {
+ if (unlikely(env->breakpoints)) {
+ for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
+ if (bp->pc == ctx.pc) {
save_cpu_state(&ctx, 1);
ctx.bstate = BS_BRANCH;
tcg_gen_helper_0_i(do_raise_exception, EXCP_DEBUG);
Index: b/qemu/target-ppc/translate.c
===================================================================
--- a/qemu/target-ppc/translate.c
+++ b/qemu/target-ppc/translate.c
@@ -6198,6 +6198,7 @@ static always_inline void gen_intermedia
target_ulong pc_start;
uint16_t *gen_opc_end;
int supervisor, little_endian;
+ CPUBreakpoint *bp;
int j, lj = -1;
int num_insns;
int max_insns;
@@ -6252,9 +6253,9 @@ static always_inline void gen_intermedia
gen_icount_start();
/* Set env in case of segfault during code fetch */
while (ctx.exception == POWERPC_EXCP_NONE && gen_opc_ptr < gen_opc_end) {
- if (unlikely(env->nb_breakpoints > 0)) {
- for (j = 0; j < env->nb_breakpoints; j++) {
- if (env->breakpoints[j] == ctx.nip) {
+ if (unlikely(env->breakpoints)) {
+ for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
+ if (bp->pc == ctx.nip) {
gen_update_nip(&ctx, ctx.nip);
gen_op_debug();
break;
Index: b/qemu/target-sh4/translate.c
===================================================================
--- a/qemu/target-sh4/translate.c
+++ b/qemu/target-sh4/translate.c
@@ -1785,6 +1785,7 @@ gen_intermediate_code_internal(CPUState
DisasContext ctx;
target_ulong pc_start;
static uint16_t *gen_opc_end;
+ CPUBreakpoint *bp;
int i, ii;
int num_insns;
int max_insns;
@@ -1818,9 +1819,9 @@ gen_intermediate_code_internal(CPUState
max_insns = CF_COUNT_MASK;
gen_icount_start();
while (ctx.bstate == BS_NONE && gen_opc_ptr < gen_opc_end) {
- if (env->nb_breakpoints > 0) {
- for (i = 0; i < env->nb_breakpoints; i++) {
- if (ctx.pc == env->breakpoints[i]) {
+ if (unlikely(env->breakpoints)) {
+ for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
+ if (ctx.pc == bp->pc) {
/* We have hit a breakpoint - make sure PC is up-to-date */
tcg_gen_movi_i32(cpu_pc, ctx.pc);
tcg_gen_helper_0_0(helper_debug);
Index: b/qemu/target-sparc/translate.c
===================================================================
--- a/qemu/target-sparc/translate.c
+++ b/qemu/target-sparc/translate.c
@@ -4775,6 +4775,7 @@ static inline void gen_intermediate_code
target_ulong pc_start, last_pc;
uint16_t *gen_opc_end;
DisasContext dc1, *dc = &dc1;
+ CPUBreakpoint *bp;
int j, lj = -1;
int num_insns;
int max_insns;
@@ -4812,9 +4813,9 @@ static inline void gen_intermediate_code
max_insns = CF_COUNT_MASK;
gen_icount_start();
do {
- if (env->nb_breakpoints > 0) {
- for(j = 0; j < env->nb_breakpoints; j++) {
- if (env->breakpoints[j] == dc->pc) {
+ if (unlikely(env->breakpoints)) {
+ for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
+ if (bp->pc == dc->pc) {
if (dc->pc != pc_start)
save_state(dc, cpu_cond);
tcg_gen_helper_0_0(helper_debug);
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 04/17] qemu: Set mem_io_vaddr on io_read
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
` (2 preceding siblings ...)
2008-10-06 9:14 ` [PATCH 03/17] qemu: Refactor and enhance break/watchpoint API Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-06 9:14 ` [PATCH 05/17] qemu: Respect length of watchpoints Jan Kiszka
` (14 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: qemu-set-mem_io_vaddr-on-read.patch --]
[-- Type: text/plain, Size: 549 bytes --]
Required for read watchpoints.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/softmmu_template.h | 1 +
1 file changed, 1 insertion(+)
Index: b/qemu/softmmu_template.h
===================================================================
--- a/qemu/softmmu_template.h
+++ b/qemu/softmmu_template.h
@@ -64,6 +64,7 @@ static inline DATA_TYPE glue(io_read, SU
cpu_io_recompile(env, retaddr);
}
+ env->mem_io_vaddr = addr;
#if SHIFT <= 2
res = io_mem_read[index][SHIFT](io_mem_opaque[index], physaddr);
#else
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 05/17] qemu: Respect length of watchpoints
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
` (3 preceding siblings ...)
2008-10-06 9:14 ` [PATCH 04/17] qemu: Set mem_io_vaddr on io_read Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-07 12:04 ` Avi Kivity
2008-10-06 9:14 ` [PATCH 06/17] qemu: Introduce next_cflags Jan Kiszka
` (13 subsequent siblings)
18 siblings, 1 reply; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: qemu-watchpoint-len-support.patch --]
[-- Type: text/plain, Size: 4487 bytes --]
This adds length support for watchpoints. To keep things simple, only
aligned watchpoints are accepted.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/cpu-defs.h | 2 +-
qemu/exec.c | 28 ++++++++++++++++++----------
2 files changed, 19 insertions(+), 11 deletions(-)
Index: b/qemu/exec.c
===================================================================
--- a/qemu/exec.c
+++ b/qemu/exec.c
@@ -1328,14 +1328,19 @@ static void breakpoint_invalidate(CPUSta
int cpu_watchpoint_insert(CPUState *env, target_ulong addr, target_ulong len,
int flags, CPUWatchpoint **watchpoint)
{
+ target_ulong len_mask = ~(len - 1);
CPUWatchpoint *wp;
+ /* sanity checks: allow power-of-2 lengths, deny unaligned watchpoints */
+ if ((len != 1 && len != 2 && len != 4) || (addr & ~len_mask))
+ return -EINVAL;
+
wp = qemu_malloc(sizeof(*wp));
if (!wp)
return -ENOBUFS;
wp->vaddr = addr;
- wp->len = len;
+ wp->len_mask = len_mask;
wp->flags = flags;
wp->next = env->watchpoints;
@@ -1359,10 +1364,12 @@ int cpu_watchpoint_insert(CPUState *env,
int cpu_watchpoint_remove(CPUState *env, target_ulong addr, target_ulong len,
int flags)
{
+ target_ulong len_mask = ~(len - 1);
CPUWatchpoint *wp;
for (wp = env->watchpoints; wp != NULL; wp = wp->next) {
- if (addr == wp->vaddr && len == wp->len && flags == wp->flags) {
+ if (addr == wp->vaddr && len_mask == wp->len_mask
+ && flags == wp->flags) {
cpu_watchpoint_remove_by_ref(env, wp);
return 0;
}
@@ -2490,7 +2497,7 @@ static CPUWriteMemoryFunc *notdirty_mem_
};
/* Generate a debug exception if a watchpoint has been hit. */
-static void check_watchpoint(int offset, int flags)
+static void check_watchpoint(int offset, int len_mask, int flags)
{
CPUState *env = cpu_single_env;
target_ulong vaddr;
@@ -2498,7 +2505,8 @@ static void check_watchpoint(int offset,
vaddr = (env->mem_io_vaddr & TARGET_PAGE_MASK) + offset;
for (wp = env->watchpoints; wp != NULL; wp = wp->next) {
- if (vaddr == wp->vaddr && (wp->flags & flags)) {
+ if ((vaddr == (wp->vaddr & len_mask) ||
+ (vaddr & wp->len_mask) == wp->vaddr) && (wp->flags & flags)) {
env->watchpoint_hit = wp;
cpu_interrupt(env, CPU_INTERRUPT_DEBUG);
break;
@@ -2511,40 +2519,40 @@ static void check_watchpoint(int offset,
phys routines. */
static uint32_t watch_mem_readb(void *opaque, target_phys_addr_t addr)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_READ);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x0, BP_MEM_READ);
return ldub_phys(addr);
}
static uint32_t watch_mem_readw(void *opaque, target_phys_addr_t addr)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_READ);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x1, BP_MEM_READ);
return lduw_phys(addr);
}
static uint32_t watch_mem_readl(void *opaque, target_phys_addr_t addr)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_READ);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x3, BP_MEM_READ);
return ldl_phys(addr);
}
static void watch_mem_writeb(void *opaque, target_phys_addr_t addr,
uint32_t val)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_WRITE);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x0, BP_MEM_WRITE);
stb_phys(addr, val);
}
static void watch_mem_writew(void *opaque, target_phys_addr_t addr,
uint32_t val)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_WRITE);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x1, BP_MEM_WRITE);
stw_phys(addr, val);
}
static void watch_mem_writel(void *opaque, target_phys_addr_t addr,
uint32_t val)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_WRITE);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x3, BP_MEM_WRITE);
stl_phys(addr, val);
}
Index: b/qemu/cpu-defs.h
===================================================================
--- a/qemu/cpu-defs.h
+++ b/qemu/cpu-defs.h
@@ -148,7 +148,7 @@ typedef struct CPUBreakpoint {
typedef struct CPUWatchpoint {
target_ulong vaddr;
- target_ulong len;
+ target_ulong len_mask;
int flags; /* BP_* */
struct CPUWatchpoint *prev, *next;
} CPUWatchpoint;
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/17] qemu: Respect length of watchpoints
2008-10-06 9:14 ` [PATCH 05/17] qemu: Respect length of watchpoints Jan Kiszka
@ 2008-10-07 12:04 ` Avi Kivity
2008-10-08 20:22 ` Jan Kiszka
0 siblings, 1 reply; 31+ messages in thread
From: Avi Kivity @ 2008-10-07 12:04 UTC (permalink / raw)
To: Jan Kiszka; +Cc: kvm
Jan Kiszka wrote:
> This adds length support for watchpoints. To keep things simple, only
> aligned watchpoints are accepted.
>
> --- a/qemu/exec.c
> +++ b/qemu/exec.c
> @@ -1328,14 +1328,19 @@ static void breakpoint_invalidate(CPUSta
> int cpu_watchpoint_insert(CPUState *env, target_ulong addr, target_ulong len,
> int flags, CPUWatchpoint **watchpoint)
> {
> + target_ulong len_mask = ~(len - 1);
> CPUWatchpoint *wp;
>
> + /* sanity checks: allow power-of-2 lengths, deny unaligned watchpoints */
> + if ((len != 1 && len != 2 && len != 4) || (addr & ~len_mask))
> + return -EINVAL;
> +
>
It would be good to support 8-byte watchpoints (as x86-64 does); also,
print a message if we deny a breakpoint due to bad alignment, so people
know where to fix this.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 05/17] qemu: Respect length of watchpoints
2008-10-07 12:04 ` Avi Kivity
@ 2008-10-08 20:22 ` Jan Kiszka
0 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-08 20:22 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
[-- Attachment #1: Type: text/plain, Size: 5851 bytes --]
Avi Kivity wrote:
> Jan Kiszka wrote:
>> This adds length support for watchpoints. To keep things simple, only
>> aligned watchpoints are accepted.
>>
>> --- a/qemu/exec.c
>> +++ b/qemu/exec.c
>> @@ -1328,14 +1328,19 @@ static void breakpoint_invalidate(CPUSta
>> int cpu_watchpoint_insert(CPUState *env, target_ulong addr,
>> target_ulong len,
>> int flags, CPUWatchpoint **watchpoint)
>> {
>> + target_ulong len_mask = ~(len - 1);
>> CPUWatchpoint *wp;
>>
>> + /* sanity checks: allow power-of-2 lengths, deny unaligned
>> watchpoints */
>> + if ((len != 1 && len != 2 && len != 4) || (addr & ~len_mask))
>> + return -EINVAL;
>> +
>>
>
> It would be good to support 8-byte watchpoints (as x86-64 does); also,
> print a message if we deny a breakpoint due to bad alignment, so people
> know where to fix this.
>
Yep, here the updated patch.
[ This, as well as the rest, for the convenience of testers under KVM.
Will repost the updated QEMU series next week. ]
------------
This adds length support for watchpoints. To keep things simple, only
aligned watchpoints are accepted.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/cpu-defs.h | 2 +-
qemu/exec.c | 30 ++++++++++++++++++++----------
2 files changed, 21 insertions(+), 11 deletions(-)
Index: b/qemu/exec.c
===================================================================
--- a/qemu/exec.c
+++ b/qemu/exec.c
@@ -1328,14 +1328,21 @@ static void breakpoint_invalidate(CPUSta
int cpu_watchpoint_insert(CPUState *env, target_ulong addr, target_ulong len,
int flags, CPUWatchpoint **watchpoint)
{
+ target_ulong len_mask = ~(len - 1);
CPUWatchpoint *wp;
+ /* sanity checks: allow power-of-2 lengths, deny unaligned watchpoints */
+ if ((len != 1 && len != 2 && len != 4 && len != 8) || (addr & ~len_mask)) {
+ fprintf(stderr, "qemu: tried to set invalid watchpoint at "
+ TARGET_FMT_lx ", len=" TARGET_FMT_lu "\n", addr, len);
+ return -EINVAL;
+ }
wp = qemu_malloc(sizeof(*wp));
if (!wp)
return -ENOBUFS;
wp->vaddr = addr;
- wp->len = len;
+ wp->len_mask = len_mask;
wp->flags = flags;
wp->next = env->watchpoints;
@@ -1359,10 +1366,12 @@ int cpu_watchpoint_insert(CPUState *env,
int cpu_watchpoint_remove(CPUState *env, target_ulong addr, target_ulong len,
int flags)
{
+ target_ulong len_mask = ~(len - 1);
CPUWatchpoint *wp;
for (wp = env->watchpoints; wp != NULL; wp = wp->next) {
- if (addr == wp->vaddr && len == wp->len && flags == wp->flags) {
+ if (addr == wp->vaddr && len_mask == wp->len_mask
+ && flags == wp->flags) {
cpu_watchpoint_remove_by_ref(env, wp);
return 0;
}
@@ -2490,7 +2499,7 @@ static CPUWriteMemoryFunc *notdirty_mem_
};
/* Generate a debug exception if a watchpoint has been hit. */
-static void check_watchpoint(int offset, int flags)
+static void check_watchpoint(int offset, int len_mask, int flags)
{
CPUState *env = cpu_single_env;
target_ulong vaddr;
@@ -2498,7 +2507,8 @@ static void check_watchpoint(int offset,
vaddr = (env->mem_io_vaddr & TARGET_PAGE_MASK) + offset;
for (wp = env->watchpoints; wp != NULL; wp = wp->next) {
- if (vaddr == wp->vaddr && (wp->flags & flags)) {
+ if ((vaddr == (wp->vaddr & len_mask) ||
+ (vaddr & wp->len_mask) == wp->vaddr) && (wp->flags & flags)) {
env->watchpoint_hit = wp;
cpu_interrupt(env, CPU_INTERRUPT_DEBUG);
break;
@@ -2511,40 +2521,40 @@ static void check_watchpoint(int offset,
phys routines. */
static uint32_t watch_mem_readb(void *opaque, target_phys_addr_t addr)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_READ);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x0, BP_MEM_READ);
return ldub_phys(addr);
}
static uint32_t watch_mem_readw(void *opaque, target_phys_addr_t addr)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_READ);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x1, BP_MEM_READ);
return lduw_phys(addr);
}
static uint32_t watch_mem_readl(void *opaque, target_phys_addr_t addr)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_READ);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x3, BP_MEM_READ);
return ldl_phys(addr);
}
static void watch_mem_writeb(void *opaque, target_phys_addr_t addr,
uint32_t val)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_WRITE);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x0, BP_MEM_WRITE);
stb_phys(addr, val);
}
static void watch_mem_writew(void *opaque, target_phys_addr_t addr,
uint32_t val)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_WRITE);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x1, BP_MEM_WRITE);
stw_phys(addr, val);
}
static void watch_mem_writel(void *opaque, target_phys_addr_t addr,
uint32_t val)
{
- check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_WRITE);
+ check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x3, BP_MEM_WRITE);
stl_phys(addr, val);
}
Index: b/qemu/cpu-defs.h
===================================================================
--- a/qemu/cpu-defs.h
+++ b/qemu/cpu-defs.h
@@ -148,7 +148,7 @@ typedef struct CPUBreakpoint {
typedef struct CPUWatchpoint {
target_ulong vaddr;
- target_ulong len;
+ target_ulong len_mask;
int flags; /* BP_* */
struct CPUWatchpoint *prev, *next;
} CPUWatchpoint;
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 06/17] qemu: Introduce next_cflags
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
` (4 preceding siblings ...)
2008-10-06 9:14 ` [PATCH 05/17] qemu: Respect length of watchpoints Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-07 12:07 ` Avi Kivity
2008-10-06 9:14 ` [PATCH 07/17] qemu: Switch self-modified code recompilation to next_cflags Jan Kiszka
` (12 subsequent siblings)
18 siblings, 1 reply; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: qemu-add-next_cflags.patch --]
[-- Type: text/plain, Size: 2864 bytes --]
Introduce next_cflags as part of CPUState. It controls the compile flags
of the next newly generated TB. After use, it will automatically be reset
to zero. This allows the caller to simply set and then forget about it,
e.g. to ensure that the next, and only the next TB will contain just a
single instruction. To avoid that next_cflags hits the wrong TB,
interrupt delivery is suppressed when this field is non-zero.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/cpu-defs.h | 4 ++++
qemu/cpu-exec.c | 9 +++++++--
2 files changed, 11 insertions(+), 2 deletions(-)
Index: b/qemu/cpu-defs.h
===================================================================
--- a/qemu/cpu-defs.h
+++ b/qemu/cpu-defs.h
@@ -183,6 +183,10 @@ typedef struct CPUWatchpoint {
} icount_decr; \
uint32_t can_do_io; /* nonzero if memory mapped IO is safe. */ \
\
+ /* Compile flags for generating next regular TB. \
+ Will be automatically zeroed after use. */ \
+ uint16_t next_cflags; \
+ \
/* from this point: preserved by CPU reset */ \
/* ice debug support */ \
CPUBreakpoint *breakpoints; \
Index: b/qemu/cpu-exec.c
===================================================================
--- a/qemu/cpu-exec.c
+++ b/qemu/cpu-exec.c
@@ -154,7 +154,8 @@ static TranslationBlock *tb_find_slow(ta
}
not_found:
/* if no translated code available, then translate it now */
- tb = tb_gen_code(env, pc, cs_base, flags, 0);
+ tb = tb_gen_code(env, pc, cs_base, flags, env->next_cflags);
+ env->next_cflags = 0;
found:
/* we add the TB in the virtual pc hash table */
@@ -379,8 +380,12 @@ int cpu_exec(CPUState *env1)
next_tb = 0; /* force lookup of first TB */
for(;;) {
interrupt_request = env->interrupt_request;
+ /* Deliver interrupt, but only if we are not recompiling some
+ TB (non-zero next_cflags) and the current single-step mode
+ doesn't block IRQs. */
if (unlikely(interrupt_request) &&
- likely(!(env->singlestep_enabled & SSTEP_NOIRQ))) {
+ likely(env->next_cflags == 0 &&
+ !(env->singlestep_enabled & SSTEP_NOIRQ))) {
if (interrupt_request & CPU_INTERRUPT_DEBUG) {
env->interrupt_request &= ~CPU_INTERRUPT_DEBUG;
env->exception_index = EXCP_DEBUG;
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 06/17] qemu: Introduce next_cflags
2008-10-06 9:14 ` [PATCH 06/17] qemu: Introduce next_cflags Jan Kiszka
@ 2008-10-07 12:07 ` Avi Kivity
0 siblings, 0 replies; 31+ messages in thread
From: Avi Kivity @ 2008-10-07 12:07 UTC (permalink / raw)
To: Jan Kiszka; +Cc: kvm
Jan Kiszka wrote:
> Introduce next_cflags as part of CPUState. It controls the compile flags
> of the next newly generated TB. After use, it will automatically be reset
> to zero. This allows the caller to simply set and then forget about it,
> e.g. to ensure that the next, and only the next TB will contain just a
> single instruction. To avoid that next_cflags hits the wrong TB,
> interrupt delivery is suppressed when this field is non-zero.
>
That's a little unfortunate, in that it affects how the guest runs. I
don't see an alternative though.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 07/17] qemu: Switch self-modified code recompilation to next_cflags
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
` (5 preceding siblings ...)
2008-10-06 9:14 ` [PATCH 06/17] qemu: Introduce next_cflags Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-06 9:14 ` [PATCH 08/17] qemu: Restore pc on watchpoint hits - v3 Jan Kiszka
` (11 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: qemu-switch-selfmod-recompile-to-next_cflags.patch --]
[-- Type: text/plain, Size: 4169 bytes --]
Switching tb_invalidate_phys_page_range and tb_invalidate_phys_page over
to the new next_cflags scheme when self-modifying code was detected can
save a few lines of code and remove arch dependency.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/exec.c | 32 ++++----------------------------
1 file changed, 4 insertions(+), 28 deletions(-)
Index: b/qemu/exec.c
===================================================================
--- a/qemu/exec.c
+++ b/qemu/exec.c
@@ -900,12 +900,11 @@ TranslationBlock *tb_gen_code(CPUState *
void tb_invalidate_phys_page_range(target_phys_addr_t start, target_phys_addr_t end,
int is_cpu_write_access)
{
- int n, current_tb_modified, current_tb_not_found, current_flags;
+ int n, current_tb_modified, current_tb_not_found;
CPUState *env = cpu_single_env;
PageDesc *p;
TranslationBlock *tb, *tb_next, *current_tb, *saved_tb;
target_ulong tb_start, tb_end;
- target_ulong current_pc, current_cs_base;
p = page_find(start >> TARGET_PAGE_BITS);
if (!p)
@@ -922,9 +921,6 @@ void tb_invalidate_phys_page_range(targe
current_tb_not_found = is_cpu_write_access;
current_tb_modified = 0;
current_tb = NULL; /* avoid warning */
- current_pc = 0; /* avoid warning */
- current_cs_base = 0; /* avoid warning */
- current_flags = 0; /* avoid warning */
tb = p->first_tb;
while (tb != NULL) {
n = (long)tb & 3;
@@ -961,14 +957,6 @@ void tb_invalidate_phys_page_range(targe
current_tb_modified = 1;
cpu_restore_state(current_tb, env,
env->mem_io_pc, NULL);
-#if defined(TARGET_I386)
- current_flags = env->hflags;
- current_flags |= (env->eflags & (IOPL_MASK | TF_MASK | VM_MASK));
- current_cs_base = (target_ulong)env->segs[R_CS].base;
- current_pc = current_cs_base + env->eip;
-#else
-#error unsupported CPU
-#endif
}
#endif /* TARGET_HAS_PRECISE_SMC */
/* we need to do that to handle the case where a signal
@@ -1002,7 +990,7 @@ void tb_invalidate_phys_page_range(targe
modifying the memory. It will ensure that it cannot modify
itself */
env->current_tb = NULL;
- tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
+ env->next_cflags = 1;
cpu_resume_from_signal(env, NULL);
}
#endif
@@ -1041,8 +1029,7 @@ static inline void tb_invalidate_phys_pa
static void tb_invalidate_phys_page(target_phys_addr_t addr,
unsigned long pc, void *puc)
{
- int n, current_flags, current_tb_modified;
- target_ulong current_pc, current_cs_base;
+ int n, current_tb_modified;
PageDesc *p;
TranslationBlock *tb, *current_tb;
#ifdef TARGET_HAS_PRECISE_SMC
@@ -1056,9 +1043,6 @@ static void tb_invalidate_phys_page(targ
tb = p->first_tb;
current_tb_modified = 0;
current_tb = NULL;
- current_pc = 0; /* avoid warning */
- current_cs_base = 0; /* avoid warning */
- current_flags = 0; /* avoid warning */
#ifdef TARGET_HAS_PRECISE_SMC
if (tb && pc != 0) {
current_tb = tb_find_pc(pc);
@@ -1078,14 +1062,6 @@ static void tb_invalidate_phys_page(targ
current_tb_modified = 1;
cpu_restore_state(current_tb, env, pc, puc);
-#if defined(TARGET_I386)
- current_flags = env->hflags;
- current_flags |= (env->eflags & (IOPL_MASK | TF_MASK | VM_MASK));
- current_cs_base = (target_ulong)env->segs[R_CS].base;
- current_pc = current_cs_base + env->eip;
-#else
-#error unsupported CPU
-#endif
}
#endif /* TARGET_HAS_PRECISE_SMC */
tb_phys_invalidate(tb, addr);
@@ -1098,7 +1074,7 @@ static void tb_invalidate_phys_page(targ
modifying the memory. It will ensure that it cannot modify
itself */
env->current_tb = NULL;
- tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
+ env->next_cflags = 1;
cpu_resume_from_signal(env, puc);
}
#endif
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 08/17] qemu: Restore pc on watchpoint hits - v3
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
` (6 preceding siblings ...)
2008-10-06 9:14 ` [PATCH 07/17] qemu: Switch self-modified code recompilation to next_cflags Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-06 9:14 ` [PATCH 09/17] qemu: Remove premature memop TB terminations Jan Kiszka
` (10 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: qemu-restore-pc-on-watchpoint-hit.patch --]
[-- Type: text/plain, Size: 2664 bytes --]
In order to provide accurate information about the triggering
instruction, this patch adds the required bits to restore the pc if the
access happened inside a TB. With the BP_STOP_BEFORE_ACCESS flag, the
watchpoint user can control if the debug trap should be issued on or
after the accessing instruction.
In contrast to the earlier posted version, this one makes use of
next_cflags to ensure that the next TB contains just a single
instruction.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/cpu-all.h | 1 +
qemu/exec.c | 22 ++++++++++++++++++++--
2 files changed, 21 insertions(+), 2 deletions(-)
Index: b/qemu/exec.c
===================================================================
--- a/qemu/exec.c
+++ b/qemu/exec.c
@@ -2476,16 +2476,34 @@ static CPUWriteMemoryFunc *notdirty_mem_
static void check_watchpoint(int offset, int len_mask, int flags)
{
CPUState *env = cpu_single_env;
+ TranslationBlock *tb;
target_ulong vaddr;
CPUWatchpoint *wp;
+ if (env->watchpoint_hit) {
+ /* We re-entered the check after replacing the TB. Now raise
+ * the debug interrupt so that is will trigger after the
+ * current instruction. */
+ cpu_interrupt(env, CPU_INTERRUPT_DEBUG);
+ return;
+ }
vaddr = (env->mem_io_vaddr & TARGET_PAGE_MASK) + offset;
for (wp = env->watchpoints; wp != NULL; wp = wp->next) {
if ((vaddr == (wp->vaddr & len_mask) ||
(vaddr & wp->len_mask) == wp->vaddr) && (wp->flags & flags)) {
env->watchpoint_hit = wp;
- cpu_interrupt(env, CPU_INTERRUPT_DEBUG);
- break;
+ tb = tb_find_pc(env->mem_io_pc);
+ if (!tb) {
+ cpu_abort(env, "check_watchpoint: could not find TB for pc=%p",
+ (void *)env->mem_io_pc);
+ }
+ cpu_restore_state(tb, env, env->mem_io_pc, NULL);
+ tb_phys_invalidate(tb, -1);
+ if (wp->flags & BP_STOP_BEFORE_ACCESS)
+ env->exception_index = EXCP_DEBUG;
+ else
+ env->next_cflags = 1;
+ cpu_resume_from_signal(env, NULL);
}
}
}
Index: b/qemu/cpu-all.h
===================================================================
--- a/qemu/cpu-all.h
+++ b/qemu/cpu-all.h
@@ -765,6 +765,7 @@ void cpu_reset_interrupt(CPUState *env,
#define BP_MEM_READ 0x01
#define BP_MEM_WRITE 0x02
#define BP_MEM_ACCESS (BP_MEM_READ | BP_MEM_WRITE)
+#define BP_STOP_BEFORE_ACCESS 0x04
#define BP_GDB 0x10
int cpu_breakpoint_insert(CPUState *env, target_ulong pc, int flags,
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 09/17] qemu: Remove premature memop TB terminations
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
` (7 preceding siblings ...)
2008-10-06 9:14 ` [PATCH 08/17] qemu: Restore pc on watchpoint hits - v3 Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-06 9:14 ` [PATCH 10/17] qemu: Improve debugging of SMP guests Jan Kiszka
` (9 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: qemu-remove-memop-tb-termination.patch --]
[-- Type: text/plain, Size: 2410 bytes --]
Now that we can properly restore the pc on watchpoint hits, there is no
more need for prematurely terminating TBs if watchpoints are present.
Remove all related bits.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/exec.c | 4 ----
qemu/target-arm/translate.c | 6 ------
qemu/target-m68k/translate.c | 6 ------
3 files changed, 16 deletions(-)
Index: b/qemu/exec.c
===================================================================
--- a/qemu/exec.c
+++ b/qemu/exec.c
@@ -1326,10 +1326,6 @@ int cpu_watchpoint_insert(CPUState *env,
env->watchpoints = wp;
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
- re-execute bits are in. */
- tb_flush(env);
if (watchpoint)
*watchpoint = wp;
Index: b/qemu/target-arm/translate.c
===================================================================
--- a/qemu/target-arm/translate.c
+++ b/qemu/target-arm/translate.c
@@ -8673,12 +8673,6 @@ static inline void gen_intermediate_code
gen_set_label(dc->condlabel);
dc->condjmp = 0;
}
- /* Terminate the TB on memory ops if watchpoints are present. */
- /* FIXME: This should be replacd by the deterministic execution
- * IRQ raising bits. */
- if (dc->is_mem && env->watchpoints)
- break;
-
/* Translation stops when a conditional branch is enoutered.
* Otherwise the subsequent code could get translated several times.
* Also stop translation when a page boundary is reached. This
Index: b/qemu/target-m68k/translate.c
===================================================================
--- a/qemu/target-m68k/translate.c
+++ b/qemu/target-m68k/translate.c
@@ -2977,12 +2977,6 @@ gen_intermediate_code_internal(CPUState
dc->insn_pc = dc->pc;
disas_m68k_insn(env, dc);
num_insns++;
-
- /* Terminate the TB on memory ops if watchpoints are present. */
- /* FIXME: This should be replaced by the deterministic execution
- * IRQ raising bits. */
- if (dc->is_mem && env->watchpoints)
- break;
} while (!dc->is_jmp && gen_opc_ptr < gen_opc_end &&
!env->singlestep_enabled &&
(pc_offset) < (TARGET_PAGE_SIZE - 32) &&
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 10/17] qemu: Improve debugging of SMP guests
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
` (8 preceding siblings ...)
2008-10-06 9:14 ` [PATCH 09/17] qemu: Remove premature memop TB terminations Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-07 12:12 ` Avi Kivity
2008-10-06 9:14 ` [PATCH 11/17] qemu: Introduce BP_WATCHPOINT_HIT flag Jan Kiszka
` (8 subsequent siblings)
18 siblings, 1 reply; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: qemu-improve-smp-debugging.patch --]
[-- Type: text/plain, Size: 10084 bytes --]
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 <jan.kiszka@siemens.com>
---
qemu/gdbstub.c | 85 +++++++++++++++++++++++++++++++++++++--------------------
qemu/monitor.c | 19 ++++++++----
qemu/monitor.h | 15 ++++++++++
qemu/vl.c | 2 +
4 files changed, 85 insertions(+), 36 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;
@@ -978,43 +978,71 @@ static 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)
@@ -1040,7 +1068,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') {
@@ -1076,7 +1104,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;
@@ -1121,7 +1149,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 {
@@ -1178,9 +1206,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)
@@ -1244,6 +1272,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;
@@ -1252,11 +1281,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;
@@ -1268,12 +1297,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;
@@ -1344,15 +1373,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;
@@ -1516,7 +1545,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;
@@ -1619,7 +1647,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
@@ -272,8 +272,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;
@@ -286,10 +285,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);
@@ -316,8 +321,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,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/qemu/vl.c
===================================================================
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -34,6 +34,7 @@
#include "console.h"
#include "sysemu.h"
#include "gdbstub.h"
+#include "monitor.h"
#include "qemu-timer.h"
#include "qemu-char.h"
#include "block.h"
@@ -8408,6 +8409,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 */
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 10/17] qemu: Improve debugging of SMP guests
2008-10-06 9:14 ` [PATCH 10/17] qemu: Improve debugging of SMP guests Jan Kiszka
@ 2008-10-07 12:12 ` Avi Kivity
2008-10-08 20:25 ` Jan Kiszka
0 siblings, 1 reply; 31+ messages in thread
From: Avi Kivity @ 2008-10-07 12:12 UTC (permalink / raw)
To: Jan Kiszka; +Cc: kvm
Jan Kiszka wrote:
> 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.
>
>
For gdbstub, this should be done through the debugger, using the gdb
"thread" command, instead of through the monitor. The monitor may
belong to a management application which can issue it independently of
the person using the debugger.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 10/17] qemu: Improve debugging of SMP guests
2008-10-07 12:12 ` Avi Kivity
@ 2008-10-08 20:25 ` Jan Kiszka
0 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-08 20:25 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
[-- Attachment #1: Type: text/plain, Size: 20526 bytes --]
Avi Kivity wrote:
> Jan Kiszka wrote:
>> 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.
>>
>>
>
> For gdbstub, this should be done through the debugger, using the gdb
> "thread" command, instead of through the monitor. The monitor may
> belong to a management application which can issue it independently of
> the person using the debugger.
Yeah, I already thought about this a while ago as well, specifically to
allow switching the context directly via the frontend. But I was too
laz^Wbusy to hack it up. However, your remark and my long train ride
yesterday finally enabled this overdue rework.
The following has one cosmetic (=display) issue under KVM:
CPUState.halted is not properly tracked for in-kernel irqchip mode. /me
still has to understand what happens in that case, while I cannot simply
translate mp_state into halted, but all this looks messy on first sight.
------------
This patch enhances QEMU's built-in debugger for SMP guest debugging.
Using the thread support of the gdb remote protocol, each VCPU is mapped
on a pseudo thread and exposed to the gdb frontend. This way you can
easy switch the focus of gdb between the VCPUs and observe their states.
On breakpoint hit, the focus is automatically adjusted just as for
normal multi-threaded application under gdb control.
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, SMP guest debugging was practically unfeasible.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/gdbstub.c | 262 +++++++++++++++++++++++++++++++++++++++++----------------
qemu/gdbstub.h | 1
qemu/vl.c | 1
3 files changed, 191 insertions(+), 73 deletions(-)
Index: b/qemu/gdbstub.c
===================================================================
--- a/qemu/gdbstub.c
+++ b/qemu/gdbstub.c
@@ -18,6 +18,7 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
#include "config.h"
+#include "qemu-common.h"
#ifdef CONFIG_USER_ONLY
#include <stdlib.h>
#include <stdio.h>
@@ -29,7 +30,6 @@
#include "qemu.h"
#else
-#include "qemu-common.h"
#include "qemu-char.h"
#include "sysemu.h"
#include "gdbstub.h"
@@ -59,7 +59,9 @@ enum RSState {
RS_SYSCALL,
};
typedef struct GDBState {
- CPUState *env; /* current CPU */
+ CPUState *c_cpu; /* current CPU for step/continue ops */
+ CPUState *g_cpu; /* current CPU for other ops */
+ CPUState *query_cpu; /* for q{f|s}ThreadInfo */
enum RSState state; /* parsing state */
char line_buf[4096];
int line_buf_index;
@@ -80,13 +82,12 @@ typedef struct GDBState {
*/
static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
+static GDBState *gdbserver_state;
+
#ifdef CONFIG_USER_ONLY
/* XXX: This is not thread safe. Do we care? */
static int gdbserver_fd = -1;
-/* XXX: remove this hack. */
-static GDBState gdbserver_state;
-
static int get_char(GDBState *s)
{
uint8_t ch;
@@ -978,49 +979,78 @@ static 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)
+static int gdb_handle_packet(GDBState *s, const char *line_buf)
{
+ CPUState *env;
const char *p;
- int ch, reg_size, type, res;
+ int ch, reg_size, type, res, thread;
char buf[4096];
uint8_t mem_buf[4096];
uint32_t *registers;
@@ -1040,28 +1070,28 @@ 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') {
addr = strtoull(p, (char **)&p, 16);
#if defined(TARGET_I386)
- env->eip = addr;
- kvm_load_registers(env);
+ s->c_cpu->eip = addr;
+ kvm_load_registers(s->c_cpu);
#elif defined (TARGET_PPC)
- env->nip = addr;
- kvm_load_registers(env);
+ s->c_cpu->nip = addr;
+ kvm_load_registers(s->c_cpu);
#elif defined (TARGET_SPARC)
- env->pc = addr;
- env->npc = addr + 4;
+ s->c_cpu->pc = addr;
+ s->c_cpu->npc = addr + 4;
#elif defined (TARGET_ARM)
- env->regs[15] = addr;
+ s->c_cpu->regs[15] = addr;
#elif defined (TARGET_SH4)
- env->pc = addr;
+ s->c_cpu->pc = addr;
#elif defined (TARGET_MIPS)
- env->active_tc.PC = addr;
+ s->c_cpu->active_tc.PC = addr;
#elif defined (TARGET_CRIS)
- env->pc = addr;
+ s->c_cpu->pc = addr;
#endif
}
gdb_continue(s);
@@ -1076,7 +1106,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;
@@ -1084,25 +1114,25 @@ static int gdb_handle_packet(GDBState *s
if (*p != '\0') {
addr = strtoull(p, (char **)&p, 16);
#if defined(TARGET_I386)
- env->eip = addr;
- kvm_load_registers(env);
+ s->c_cpu->eip = addr;
+ kvm_load_registers(s->c_cpu);
#elif defined (TARGET_PPC)
- env->nip = addr;
- kvm_load_registers(env);
+ s->c_cpu->nip = addr;
+ kvm_load_registers(s->c_cpu);
#elif defined (TARGET_SPARC)
- env->pc = addr;
- env->npc = addr + 4;
+ s->c_cpu->pc = addr;
+ s->c_cpu->npc = addr + 4;
#elif defined (TARGET_ARM)
- env->regs[15] = addr;
+ s->c_cpu->regs[15] = addr;
#elif defined (TARGET_SH4)
- env->pc = addr;
+ s->c_cpu->pc = addr;
#elif defined (TARGET_MIPS)
- env->active_tc.PC = addr;
+ s->c_cpu->active_tc.PC = addr;
#elif defined (TARGET_CRIS)
- env->pc = addr;
+ s->c_cpu->pc = addr;
#endif
}
- cpu_single_step(env, sstep_flags);
+ cpu_single_step(s->c_cpu, sstep_flags);
gdb_continue(s);
return RS_IDLE;
case 'F':
@@ -1121,7 +1151,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(s->c_cpu, ret, err);
if (type == 'C') {
put_packet(s, "T02");
} else {
@@ -1130,8 +1160,8 @@ static int gdb_handle_packet(GDBState *s
}
break;
case 'g':
- kvm_save_registers(env);
- reg_size = cpu_gdb_read_registers(env, mem_buf);
+ kvm_save_registers(s->g_cpu);
+ reg_size = cpu_gdb_read_registers(s->g_cpu, mem_buf);
memtohex(buf, mem_buf, reg_size);
put_packet(s, buf);
break;
@@ -1139,8 +1169,8 @@ static int gdb_handle_packet(GDBState *s
registers = (void *)mem_buf;
len = strlen(p) / 2;
hextomem((uint8_t *)registers, p, len);
- cpu_gdb_write_registers(env, mem_buf, len);
- kvm_load_registers(env);
+ cpu_gdb_write_registers(s->g_cpu, mem_buf, len);
+ kvm_load_registers(s->g_cpu);
put_packet(s, "OK");
break;
case 'm':
@@ -1148,7 +1178,7 @@ static int gdb_handle_packet(GDBState *s
if (*p == ',')
p++;
len = strtoull(p, NULL, 16);
- if (cpu_memory_rw_debug(env, addr, mem_buf, len, 0) != 0) {
+ if (cpu_memory_rw_debug(s->g_cpu, addr, mem_buf, len, 0) != 0) {
put_packet (s, "E14");
} else {
memtohex(buf, mem_buf, len);
@@ -1163,7 +1193,7 @@ static int gdb_handle_packet(GDBState *s
if (*p == ':')
p++;
hextomem(mem_buf, p, len);
- if (cpu_memory_rw_debug(env, addr, mem_buf, len, 1) != 0)
+ if (cpu_memory_rw_debug(s->g_cpu, addr, mem_buf, len, 1) != 0)
put_packet(s, "E14");
else
put_packet(s, "OK");
@@ -1178,9 +1208,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)
@@ -1188,6 +1218,45 @@ static int gdb_handle_packet(GDBState *s
else
put_packet(s, "E22");
break;
+ case 'H':
+ type = *p++;
+ thread = strtoull(p, (char **)&p, 16);
+ if (thread == -1 || thread == 0) {
+ put_packet(s, "OK");
+ break;
+ }
+ for (env = first_cpu; env != NULL; env = env->next_cpu)
+ if (env->cpu_index + 1 == thread)
+ break;
+ if (env == NULL) {
+ put_packet(s, "E22");
+ break;
+ }
+ switch (type) {
+ case 'c':
+ s->c_cpu = env;
+ put_packet(s, "OK");
+ break;
+ case 'g':
+ s->g_cpu = env;
+ put_packet(s, "OK");
+ break;
+ default:
+ put_packet(s, "E22");
+ break;
+ }
+ break;
+ case 'T':
+ thread = strtoull(p, (char **)&p, 16);
+#ifndef CONFIG_USER_ONLY
+ if (thread > 0 && thread < smp_cpus + 1)
+#else
+ if (thread == 1)
+#endif
+ put_packet(s, "OK");
+ else
+ put_packet(s, "E22");
+ break;
case 'q':
case 'Q':
/* parse any 'q' packets here */
@@ -1213,10 +1282,39 @@ static int gdb_handle_packet(GDBState *s
sstep_flags = type;
put_packet(s, "OK");
break;
+ } else if (strcmp(p,"C") == 0) {
+ /* "Current thread" remains vague in the spec, so always return
+ * the first CPU (gdb returns the first thread). */
+ put_packet(s, "QC1");
+ break;
+ } else if (strcmp(p,"fThreadInfo") == 0) {
+ s->query_cpu = first_cpu;
+ goto report_cpuinfo;
+ } else if (strcmp(p,"sThreadInfo") == 0) {
+ report_cpuinfo:
+ if (s->query_cpu) {
+ snprintf(buf, sizeof(buf), "m%x", s->query_cpu->cpu_index+1);
+ put_packet(s, buf);
+ s->query_cpu = s->query_cpu->next_cpu;
+ } else
+ put_packet(s, "l");
+ break;
+ } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) {
+ thread = strtoull(p+16, (char **)&p, 16);
+ for (env = first_cpu; env != NULL; env = env->next_cpu)
+ if (env->cpu_index + 1 == thread) {
+ len = snprintf((char *)mem_buf, sizeof(mem_buf),
+ "CPU#%d [%s]", env->cpu_index,
+ env->halted ? "halted " : "running");
+ memtohex(buf, mem_buf, len);
+ put_packet(s, buf);
+ break;
+ }
+ break;
}
#ifdef CONFIG_LINUX_USER
else if (strncmp(p, "Offsets", 7) == 0) {
- TaskState *ts = env->opaque;
+ TaskState *ts = s->c_cpu->opaque;
snprintf(buf, sizeof(buf),
"Text=" TARGET_ABI_FMT_lx ";Data=" TARGET_ABI_FMT_lx
@@ -1240,10 +1338,17 @@ static int gdb_handle_packet(GDBState *s
extern void tb_flush(CPUState *env);
+void gdb_set_stop_cpu(CPUState *env)
+{
+ gdbserver_state->c_cpu = env;
+ gdbserver_state->g_cpu = env;
+}
+
#ifndef CONFIG_USER_ONLY
static void gdb_vm_stopped(void *opaque, int reason)
{
- GDBState *s = opaque;
+ GDBState *s = gdbserver_state;
+ CPUState *env = s->c_cpu;
char buf[256];
const char *type;
int ret;
@@ -1252,11 +1357,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;
@@ -1267,20 +1372,22 @@ static void gdb_vm_stopped(void *opaque,
type = "";
break;
}
- snprintf(buf, sizeof(buf), "T%02x%swatch:" TARGET_FMT_lx ";",
- SIGTRAP, type, s->env->watchpoint_hit->vaddr);
+ snprintf(buf, sizeof(buf),
+ "T%02xthread:%02x;%swatch:" TARGET_FMT_lx ";",
+ SIGTRAP, env->cpu_index+1, 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;
} else {
ret = 0;
}
- snprintf(buf, sizeof(buf), "S%02x", ret);
+ snprintf(buf, sizeof(buf), "T%02xthread:%02x;", ret, env->cpu_index+1);
put_packet(s, buf);
}
#endif
@@ -1344,15 +1451,14 @@ 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(s->c_cpu, 0);
#else
- cpu_interrupt(s->env, CPU_INTERRUPT_EXIT);
+ cpu_interrupt(s->c_cpu, CPU_INTERRUPT_EXIT);
#endif
}
static void gdb_read_byte(GDBState *s, int ch)
{
- CPUState *env = s->env;
int i, csum;
uint8_t reply;
@@ -1418,7 +1524,7 @@ static void gdb_read_byte(GDBState *s, i
} else {
reply = '+';
put_buffer(s, &reply, 1);
- s->state = gdb_handle_packet(s, env, s->line_buf);
+ s->state = gdb_handle_packet(s, s->line_buf);
}
break;
default:
@@ -1435,7 +1541,7 @@ gdb_handlesig (CPUState *env, int sig)
char buf[256];
int n;
- s = &gdbserver_state;
+ s = gdbserver_state;
if (gdbserver_fd < 0 || s->fd < 0)
return sig;
@@ -1483,7 +1589,7 @@ void gdb_exit(CPUState *env, int code)
GDBState *s;
char buf[4];
- s = &gdbserver_state;
+ s = gdbserver_state;
if (gdbserver_fd < 0 || s->fd < 0)
return;
@@ -1492,7 +1598,7 @@ void gdb_exit(CPUState *env, int code)
}
-static void gdb_accept(void *opaque)
+static void gdb_accept(void)
{
GDBState *s;
struct sockaddr_in sockaddr;
@@ -1514,11 +1620,19 @@ static void gdb_accept(void *opaque)
val = 1;
setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, (char *)&val, sizeof(val));
- s = &gdbserver_state;
+ s = qemu_mallocz(sizeof(GDBState));
+ if (!s) {
+ errno = ENOMEM;
+ perror("accept");
+ return;
+ }
+
memset (s, 0, sizeof (GDBState));
- s->env = first_cpu; /* XXX: allow to change CPU */
+ s->c_cpu = first_cpu;
+ s->g_cpu = first_cpu;
s->fd = fd;
+ gdbserver_state = s;
gdb_syscall_state = s;
fcntl(fd, F_SETFL, O_NONBLOCK);
@@ -1561,7 +1675,7 @@ int gdbserver_start(int port)
if (gdbserver_fd < 0)
return -1;
/* accept connections */
- gdb_accept (NULL);
+ gdb_accept();
return 0;
}
#else
@@ -1619,11 +1733,13 @@ int gdbserver_start(const char *port)
if (!s) {
return -1;
}
- s->env = first_cpu; /* XXX: allow to change CPU */
+ s->c_cpu = first_cpu;
+ s->g_cpu = first_cpu;
s->chr = chr;
+ gdbserver_state = s;
qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive,
gdb_chr_event, s);
- qemu_add_vm_stop_handler(gdb_vm_stopped, s);
+ qemu_add_vm_stop_handler(gdb_vm_stopped, NULL);
return 0;
}
#endif
Index: b/qemu/gdbstub.h
===================================================================
--- a/qemu/gdbstub.h
+++ b/qemu/gdbstub.h
@@ -8,6 +8,7 @@ typedef void (*gdb_syscall_complete_cb)(
void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...);
int use_gdb_syscalls(void);
+void gdb_set_stop_cpu(CPUState *env);
#ifdef CONFIG_USER_ONLY
int gdb_handlesig (CPUState *, int);
void gdb_exit(CPUState *, int);
Index: b/qemu/vl.c
===================================================================
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -8408,6 +8408,7 @@ static int main_loop(void)
ret = EXCP_INTERRUPT;
}
if (unlikely(ret == EXCP_DEBUG)) {
+ gdb_set_stop_cpu(cur_cpu);
vm_stop(EXCP_DEBUG);
}
/* If all cpus are halted then wait until the next IRQ */
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 11/17] qemu: Introduce BP_WATCHPOINT_HIT flag
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
` (9 preceding siblings ...)
2008-10-06 9:14 ` [PATCH 10/17] qemu: Improve debugging of SMP guests Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-06 9:14 ` [PATCH 12/17] qemu: Add debug exception hook Jan Kiszka
` (7 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: qemu-introduce-BP_WATCHPOINT_HIT-flag.patch --]
[-- Type: text/plain, Size: 4005 bytes --]
When one watchpoint is hit, others might have triggered as well. To
support users of the watchpoint API which need to detect such cases,
the BP_WATCHPOINT_HIT flag is introduced and maintained.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/cpu-all.h | 1 +
qemu/cpu-exec.c | 11 +++++++++++
qemu/exec.c | 31 ++++++++++++++++++-------------
3 files changed, 30 insertions(+), 13 deletions(-)
Index: b/qemu/cpu-all.h
===================================================================
--- a/qemu/cpu-all.h
+++ b/qemu/cpu-all.h
@@ -766,6 +766,7 @@ void cpu_reset_interrupt(CPUState *env,
#define BP_MEM_WRITE 0x02
#define BP_MEM_ACCESS (BP_MEM_READ | BP_MEM_WRITE)
#define BP_STOP_BEFORE_ACCESS 0x04
+#define BP_WATCHPOINT_HIT 0x08
#define BP_GDB 0x10
int cpu_breakpoint_insert(CPUState *env, target_ulong pc, int flags,
Index: b/qemu/cpu-exec.c
===================================================================
--- a/qemu/cpu-exec.c
+++ b/qemu/cpu-exec.c
@@ -244,6 +244,15 @@ static inline TranslationBlock *tb_find_
return tb;
}
+static void cpu_handle_debug_exception(CPUState *env)
+{
+ CPUWatchpoint *wp;
+
+ if (!env->watchpoint_hit)
+ for (wp = env->watchpoints; wp != NULL; wp = wp->next)
+ wp->flags &= ~BP_WATCHPOINT_HIT;
+}
+
/* main execution loop */
int cpu_exec(CPUState *env1)
@@ -299,6 +308,8 @@ int cpu_exec(CPUState *env1)
if (env->exception_index >= EXCP_INTERRUPT) {
/* exit request from the cpu execution loop */
ret = env->exception_index;
+ if (ret == EXCP_DEBUG)
+ cpu_handle_debug_exception(env);
break;
} else if (env->user_mode_only) {
/* if user mode only, we simulate a fake exception
Index: b/qemu/exec.c
===================================================================
--- a/qemu/exec.c
+++ b/qemu/exec.c
@@ -1341,7 +1341,7 @@ int cpu_watchpoint_remove(CPUState *env,
for (wp = env->watchpoints; wp != NULL; wp = wp->next) {
if (addr == wp->vaddr && len_mask == wp->len_mask
- && flags == wp->flags) {
+ && flags == (wp->flags & ~BP_WATCHPOINT_HIT)) {
cpu_watchpoint_remove_by_ref(env, wp);
return 0;
}
@@ -2487,19 +2487,24 @@ static void check_watchpoint(int offset,
for (wp = env->watchpoints; wp != NULL; wp = wp->next) {
if ((vaddr == (wp->vaddr & len_mask) ||
(vaddr & wp->len_mask) == wp->vaddr) && (wp->flags & flags)) {
- env->watchpoint_hit = wp;
- tb = tb_find_pc(env->mem_io_pc);
- if (!tb) {
- cpu_abort(env, "check_watchpoint: could not find TB for pc=%p",
- (void *)env->mem_io_pc);
+ wp->flags |= BP_WATCHPOINT_HIT;
+ if (!env->watchpoint_hit) {
+ env->watchpoint_hit = wp;
+ tb = tb_find_pc(env->mem_io_pc);
+ if (!tb) {
+ cpu_abort(env, "check_watchpoint: could not find TB for "
+ "pc=%p", (void *)env->mem_io_pc);
+ }
+ cpu_restore_state(tb, env, env->mem_io_pc, NULL);
+ tb_phys_invalidate(tb, -1);
+ if (wp->flags & BP_STOP_BEFORE_ACCESS)
+ env->exception_index = EXCP_DEBUG;
+ else
+ env->next_cflags = 1;
+ cpu_resume_from_signal(env, NULL);
}
- cpu_restore_state(tb, env, env->mem_io_pc, NULL);
- tb_phys_invalidate(tb, -1);
- if (wp->flags & BP_STOP_BEFORE_ACCESS)
- env->exception_index = EXCP_DEBUG;
- else
- env->next_cflags = 1;
- cpu_resume_from_signal(env, NULL);
+ } else {
+ wp->flags &= ~BP_WATCHPOINT_HIT;
}
}
}
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 12/17] qemu: Add debug exception hook
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
` (10 preceding siblings ...)
2008-10-06 9:14 ` [PATCH 11/17] qemu: Introduce BP_WATCHPOINT_HIT flag Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-06 9:14 ` [PATCH 13/17] qemu: Introduce BP_CPU as a breakpoint type Jan Kiszka
` (6 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: qemu-add-debug-exception-hook.patch --]
[-- Type: text/plain, Size: 1485 bytes --]
This patch allows to hook into the delivery of EXCP_DEBUG so that other
use beyond guest debugging becomes possible.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/cpu-exec.c | 13 +++++++++++++
qemu/exec-all.h | 4 ++++
2 files changed, 17 insertions(+)
Index: b/qemu/cpu-exec.c
===================================================================
--- a/qemu/cpu-exec.c
+++ b/qemu/cpu-exec.c
@@ -244,6 +244,16 @@ static inline TranslationBlock *tb_find_
return tb;
}
+static CPUDebugExcpHandler *debug_excp_handler;
+
+CPUDebugExcpHandler *cpu_set_debug_excp_handler(CPUDebugExcpHandler *handler)
+{
+ CPUDebugExcpHandler *old_handler = debug_excp_handler;
+
+ debug_excp_handler = handler;
+ return old_handler;
+}
+
static void cpu_handle_debug_exception(CPUState *env)
{
CPUWatchpoint *wp;
@@ -251,6 +261,9 @@ static void cpu_handle_debug_exception(C
if (!env->watchpoint_hit)
for (wp = env->watchpoints; wp != NULL; wp = wp->next)
wp->flags &= ~BP_WATCHPOINT_HIT;
+
+ if (debug_excp_handler)
+ debug_excp_handler(env);
}
/* main execution loop */
Index: b/qemu/exec-all.h
===================================================================
--- a/qemu/exec-all.h
+++ b/qemu/exec-all.h
@@ -385,3 +385,7 @@ static inline int kqemu_is_ok(CPUState *
}
#endif
+
+typedef void (CPUDebugExcpHandler)(CPUState *env);
+
+CPUDebugExcpHandler *cpu_set_debug_excp_handler(CPUDebugExcpHandler *handler);
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 13/17] qemu: Introduce BP_CPU as a breakpoint type
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
` (11 preceding siblings ...)
2008-10-06 9:14 ` [PATCH 12/17] qemu: Add debug exception hook Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-06 9:14 ` [PATCH 14/17] qemu: x86: Debug register emulation Jan Kiszka
` (5 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: qemu-introduce-BP_CPU.patch --]
[-- Type: text/plain, Size: 3435 bytes --]
Add another breakpoint/watchpoint type to BP_GDB: BP_CPU. This type is
intended for hardware-assisted break/watchpoint emulations like the x86
architecture requires.
To keep the highest priority for BP_GDB breakpoints, this type is
always inserted at the head of break/watchpoint lists, thus is found
first when looking up the origin of a debug interruption.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/cpu-all.h | 1 +
qemu/exec.c | 46 ++++++++++++++++++++++++++++++++++++++--------
2 files changed, 39 insertions(+), 8 deletions(-)
Index: b/qemu/cpu-all.h
===================================================================
--- a/qemu/cpu-all.h
+++ b/qemu/cpu-all.h
@@ -768,6 +768,7 @@ void cpu_reset_interrupt(CPUState *env,
#define BP_STOP_BEFORE_ACCESS 0x04
#define BP_WATCHPOINT_HIT 0x08
#define BP_GDB 0x10
+#define BP_CPU 0x20
int cpu_breakpoint_insert(CPUState *env, target_ulong pc, int flags,
CPUBreakpoint **breakpoint);
Index: b/qemu/exec.c
===================================================================
--- a/qemu/exec.c
+++ b/qemu/exec.c
@@ -1305,7 +1305,7 @@ int cpu_watchpoint_insert(CPUState *env,
int flags, CPUWatchpoint **watchpoint)
{
target_ulong len_mask = ~(len - 1);
- CPUWatchpoint *wp;
+ CPUWatchpoint *wp, *prev_wp;
/* sanity checks: allow power-of-2 lengths, deny unaligned watchpoints */
if ((len != 1 && len != 2 && len != 4) || (addr & ~len_mask))
@@ -1319,11 +1319,26 @@ int cpu_watchpoint_insert(CPUState *env,
wp->len_mask = len_mask;
wp->flags = flags;
- wp->next = env->watchpoints;
- wp->prev = NULL;
+ /* keep all GDB-injected watchpoints in front */
+ if (!(flags & BP_GDB) && env->watchpoints) {
+ prev_wp = env->watchpoints;
+ while (prev_wp->next != NULL && (prev_wp->next->flags & BP_GDB))
+ prev_wp = prev_wp->next;
+ } else {
+ prev_wp = NULL;
+ }
+
+ /* Insert new watchpoint */
+ if (prev_wp) {
+ wp->next = prev_wp->next;
+ prev_wp->next = wp;
+ } else {
+ wp->next = env->watchpoints;
+ env->watchpoints = wp;
+ }
if (wp->next)
wp->next->prev = wp;
- env->watchpoints = wp;
+ wp->prev = prev_wp;
tlb_flush_page(env, addr);
@@ -1379,7 +1394,7 @@ int cpu_breakpoint_insert(CPUState *env,
CPUBreakpoint **breakpoint)
{
#if defined(TARGET_HAS_ICE)
- CPUBreakpoint *bp;
+ CPUBreakpoint *bp, *prev_bp;
bp = qemu_malloc(sizeof(*bp));
if (!bp)
@@ -1388,11 +1403,26 @@ int cpu_breakpoint_insert(CPUState *env,
bp->pc = pc;
bp->flags = flags;
- bp->next = env->breakpoints;
- bp->prev = NULL;
+ /* keep all GDB-injected breakpoints in front */
+ if (!(flags & BP_GDB) && env->breakpoints) {
+ prev_bp = env->breakpoints;
+ while (prev_bp->next != NULL && (prev_bp->next->flags & BP_GDB))
+ prev_bp = prev_bp->next;
+ } else {
+ prev_bp = NULL;
+ }
+
+ /* Insert new breakpoint */
+ if (prev_bp) {
+ bp->next = prev_bp->next;
+ prev_bp->next = bp;
+ } else {
+ bp->next = env->breakpoints;
+ env->breakpoints = bp;
+ }
if (bp->next)
bp->next->prev = bp;
- env->breakpoints = bp;
+ bp->prev = prev_bp;
breakpoint_invalidate(env, pc);
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 14/17] qemu: x86: Debug register emulation
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
` (12 preceding siblings ...)
2008-10-06 9:14 ` [PATCH 13/17] qemu: Introduce BP_CPU as a breakpoint type Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-07 12:15 ` Avi Kivity
2008-10-06 9:14 ` [PATCH 15/17] kvm-userspace: Switch to new guest debug interface Jan Kiszka
` (4 subsequent siblings)
18 siblings, 1 reply; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: qemu-x86-debug-register-support.patch --]
[-- Type: text/plain, Size: 10651 bytes --]
Built on top of previously enhanced breakpoint/watchpoint support, this
patch adds full debug register emulation for the x86 architecture.
Many corner cases were considered, and the result was successfully
tested inside a Linux guest with gdb, but I won't be surprised if one
or two scenarios still behave differently in reality.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/linux-user/main.c | 4 -
qemu/target-i386/cpu.h | 33 ++++++++++-
qemu/target-i386/helper.c | 126 +++++++++++++++++++++++++++++++------------
qemu/target-i386/op_helper.c | 79 +++++++++++++++++++++++++-
4 files changed, 202 insertions(+), 40 deletions(-)
Index: b/qemu/linux-user/main.c
===================================================================
--- a/qemu/linux-user/main.c
+++ b/qemu/linux-user/main.c
@@ -406,7 +406,7 @@ void cpu_loop(CPUX86State *env)
queue_signal(env, info.si_signo, &info);
}
break;
- case EXCP01_SSTP:
+ case EXCP01_DB:
case EXCP03_INT3:
#ifndef TARGET_X86_64
if (env->eflags & VM_MASK) {
@@ -416,7 +416,7 @@ void cpu_loop(CPUX86State *env)
{
info.si_signo = SIGTRAP;
info.si_errno = 0;
- if (trapnr == EXCP01_SSTP) {
+ if (trapnr == EXCP01_DB) {
info.si_code = TARGET_TRAP_BRKPT;
info._sifields._sigfault._addr = env->eip;
} else {
Index: b/qemu/target-i386/cpu.h
===================================================================
--- a/qemu/target-i386/cpu.h
+++ b/qemu/target-i386/cpu.h
@@ -205,6 +205,16 @@
#define CR4_OSFXSR_MASK (1 << CR4_OSFXSR_SHIFT)
#define CR4_OSXMMEXCPT_MASK (1 << 10)
+#define DR6_BD (1 << 13)
+#define DR6_BS (1 << 14)
+#define DR6_BT (1 << 15)
+#define DR6_FIXED_1 0xffff0ff0
+
+#define DR7_GD (1 << 13)
+#define DR7_TYPE_SHIFT 16
+#define DR7_LEN_SHIFT 18
+#define DR7_FIXED_1 0x00000400
+
#define PG_PRESENT_BIT 0
#define PG_RW_BIT 1
#define PG_USER_BIT 2
@@ -361,7 +371,7 @@
#define CPUID_MWAIT_EMX (1 << 0) /* enumeration supported */
#define EXCP00_DIVZ 0
-#define EXCP01_SSTP 1
+#define EXCP01_DB 1
#define EXCP02_NMI 2
#define EXCP03_INT3 3
#define EXCP04_INTO 4
@@ -594,6 +604,10 @@ typedef struct CPUX86State {
int exception_is_int;
target_ulong exception_next_eip;
target_ulong dr[8]; /* debug registers */
+ union {
+ CPUBreakpoint *cpu_breakpoint[4];
+ CPUWatchpoint *cpu_watchpoint[4];
+ }; /* break/watchpoints for dr[0..3] */
uint32_t smbase;
int old_exception; /* exception in flight */
@@ -789,6 +803,23 @@ static inline void cpu_clone_regs(CPUSta
#define CPU_PC_FROM_TB(env, tb) env->eip = tb->pc - tb->cs_base
+static inline int hw_breakpoint_enabled(unsigned long dr7, int index)
+{
+ return (dr7 >> (index * 2)) & 3;
+}
+
+static inline int hw_breakpoint_type(unsigned long dr7, int index)
+{
+ return (dr7 >> (DR7_TYPE_SHIFT + (index * 2))) & 3;
+}
+
+static inline int hw_breakpoint_len(unsigned long dr7, int index)
+{
+ return ((dr7 >> (DR7_LEN_SHIFT + (index * 2))) & 3) + 1;
+}
+
+int check_hw_breakpoints(CPUState *env, int force_dr6_update);
+
#include "cpu-all.h"
#include "svm.h"
Index: b/qemu/target-i386/helper.c
===================================================================
--- a/qemu/target-i386/helper.c
+++ b/qemu/target-i386/helper.c
@@ -34,8 +34,6 @@
//#define DEBUG_MMU
-static int cpu_x86_register (CPUX86State *env, const char *cpu_model);
-
static void add_flagname_to_bitmaps(char *flagname, uint32_t *features,
uint32_t *ext_features,
uint32_t *ext2_features,
@@ -95,37 +93,6 @@ static void add_flagname_to_bitmaps(char
extern const char *cpu_vendor_string;
-CPUX86State *cpu_x86_init(const char *cpu_model)
-{
- CPUX86State *env;
- static int inited;
-
- env = qemu_mallocz(sizeof(CPUX86State));
- if (!env)
- return NULL;
- cpu_exec_init(env);
- env->cpu_model_str = cpu_model;
-
- /* init various static tables */
- if (!inited) {
- inited = 1;
- optimize_flags_init();
- }
- if (cpu_x86_register(env, cpu_model) < 0) {
- cpu_x86_close(env);
- return NULL;
- }
- cpu_reset(env);
-#ifdef USE_KQEMU
- kqemu_init(env);
-#endif
-#ifdef USE_KVM
- if (kvm_enabled())
- kvm_init_new_ap(env->cpu_index, env);
-#endif
- return env;
-}
-
typedef struct x86_def_t {
const char *name;
uint32_t level;
@@ -482,6 +449,12 @@ void cpu_reset(CPUX86State *env)
env->fpuc = 0x37f;
env->mxcsr = 0x1f80;
+
+ memset(env->dr, 0, sizeof(env->dr));
+ env->dr[6] = DR6_FIXED_1;
+ env->dr[7] = DR7_FIXED_1;
+ cpu_breakpoint_remove_all(env, BP_CPU);
+ cpu_watchpoint_remove_all(env, BP_CPU);
}
void cpu_x86_close(CPUX86State *env)
@@ -1278,4 +1251,91 @@ target_phys_addr_t cpu_get_phys_page_deb
paddr = (pte & TARGET_PAGE_MASK) + page_offset;
return paddr;
}
+
+int check_hw_breakpoints(CPUState *env, int force_dr6_update)
+{
+ target_ulong dr6;
+ int reg, type;
+ int hit_enabled = 0;
+
+ dr6 = env->dr[6] & ~0xf;
+ for (reg = 0; reg < 4; reg++) {
+ type = hw_breakpoint_type(env->dr[7], reg);
+ if ((type == 0 && env->dr[reg] == env->eip) ||
+ ((type & 1) && env->cpu_watchpoint[reg] &&
+ (env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT))) {
+ dr6 |= 1 << reg;
+ if (hw_breakpoint_enabled(env->dr[7], reg))
+ hit_enabled = 1;
+ }
+ }
+ if (hit_enabled || force_dr6_update)
+ env->dr[6] = dr6;
+ return hit_enabled;
+}
+
+static CPUDebugExcpHandler *prev_debug_excp_handler;
+
+void raise_exception(int exception_index);
+
+static void breakpoint_handler(CPUState *env)
+{
+ CPUBreakpoint *bp;
+
+ if (env->watchpoint_hit) {
+ if (env->watchpoint_hit->flags & BP_CPU) {
+ env->watchpoint_hit = NULL;
+ if (check_hw_breakpoints(env, 0))
+ raise_exception(EXCP01_DB);
+ else
+ cpu_resume_from_signal(env, NULL);
+ }
+ } else {
+ for (bp = env->breakpoints; bp != NULL; bp = bp->next)
+ if (bp->pc == env->eip) {
+ if (bp->flags & BP_CPU) {
+ check_hw_breakpoints(env, 1);
+ raise_exception(EXCP01_DB);
+ }
+ break;
+ }
+ }
+ if (prev_debug_excp_handler)
+ prev_debug_excp_handler(env);
+}
#endif /* !CONFIG_USER_ONLY */
+
+CPUX86State *cpu_x86_init(const char *cpu_model)
+{
+ CPUX86State *env;
+ static int inited;
+
+ env = qemu_mallocz(sizeof(CPUX86State));
+ if (!env)
+ return NULL;
+ cpu_exec_init(env);
+ env->cpu_model_str = cpu_model;
+
+ /* init various static stuff */
+ if (!inited) {
+ inited = 1;
+ optimize_flags_init();
+#ifndef CONFIG_USER_ONLY
+ prev_debug_excp_handler =
+ cpu_set_debug_excp_handler(breakpoint_handler);
+#endif
+ }
+ if (cpu_x86_register(env, cpu_model) < 0) {
+ cpu_x86_close(env);
+ return NULL;
+ }
+ cpu_reset(env);
+#ifdef USE_KQEMU
+ kqemu_init(env);
+#endif
+#ifdef USE_KVM
+ if (kvm_enabled())
+ kvm_init_new_ap(env->cpu_index, env);
+#endif
+ return env;
+}
Index: b/qemu/target-i386/op_helper.c
===================================================================
--- a/qemu/target-i386/op_helper.c
+++ b/qemu/target-i386/op_helper.c
@@ -94,6 +94,53 @@ const CPU86_LDouble f15rk[7] =
3.32192809488736234781L, /*l2t*/
};
+static void hw_breakpoint_insert(int index)
+{
+ int type, err = 0;
+
+ switch (hw_breakpoint_type(env->dr[7], index)) {
+ case 0:
+ if (hw_breakpoint_enabled(env->dr[7], index))
+ err = cpu_breakpoint_insert(env, env->dr[index], BP_CPU,
+ &env->cpu_breakpoint[index]);
+ break;
+ case 1:
+ type = BP_CPU | BP_MEM_WRITE;
+ goto insert_wp;
+ case 2:
+ /* No support for I/O watchpoints yet */
+ break;
+ case 3:
+ type = BP_CPU | BP_MEM_ACCESS;
+ insert_wp:
+ err = cpu_watchpoint_insert(env, env->dr[index],
+ hw_breakpoint_len(env->dr[7], index),
+ type, &env->cpu_watchpoint[index]);
+ break;
+ }
+ if (err)
+ env->cpu_breakpoint[index] = NULL;
+}
+
+static void hw_breakpoint_remove(int index)
+{
+ if (!env->cpu_breakpoint[index])
+ return;
+ switch (hw_breakpoint_type(env->dr[7], index)) {
+ case 0:
+ if (hw_breakpoint_enabled(env->dr[7], index))
+ cpu_breakpoint_remove_by_ref(env, env->cpu_breakpoint[index]);
+ break;
+ case 1:
+ case 3:
+ cpu_watchpoint_remove_by_ref(env, env->cpu_watchpoint[index]);
+ break;
+ case 2:
+ /* No support for I/O watchpoints yet */
+ break;
+ }
+}
+
/* broken thread support */
spinlock_t global_cpu_lock = SPIN_LOCK_UNLOCKED;
@@ -496,6 +543,15 @@ static void switch_tss(int tss_selector,
/* XXX: different exception if CALL ? */
raise_exception_err(EXCP0D_GPF, 0);
}
+
+ /* reset local breakpoints */
+ if (env->dr[7] & 0x55) {
+ for (i = 0; i < 4; i++) {
+ if (hw_breakpoint_enabled(env->dr[7], i) == 0x1)
+ hw_breakpoint_remove(i);
+ }
+ env->dr[7] &= ~0x55;
+ }
}
/* check if Port I/O is allowed in TSS */
@@ -1879,8 +1935,11 @@ void helper_cmpxchg16b(target_ulong a0)
void helper_single_step(void)
{
- env->dr[6] |= 0x4000;
- raise_exception(EXCP01_SSTP);
+#ifndef CONFIG_USER_ONLY
+ check_hw_breakpoints(env, 1);
+#endif
+ env->dr[6] |= DR6_BS;
+ raise_exception(EXCP01_DB);
}
void helper_cpuid(void)
@@ -3082,10 +3141,22 @@ void helper_clts(void)
env->hflags &= ~HF_TS_MASK;
}
-/* XXX: do more */
void helper_movl_drN_T0(int reg, target_ulong t0)
{
- env->dr[reg] = t0;
+ int i;
+
+ if (reg < 4) {
+ hw_breakpoint_remove(reg);
+ env->dr[reg] = t0;
+ hw_breakpoint_insert(reg);
+ } else if (reg == 7) {
+ for (i = 0; i < 4; i++)
+ hw_breakpoint_remove(i);
+ env->dr[7] = t0;
+ for (i = 0; i < 4; i++)
+ hw_breakpoint_insert(i);
+ } else
+ env->dr[reg] = t0;
}
void helper_invlpg(target_ulong addr)
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 14/17] qemu: x86: Debug register emulation
2008-10-06 9:14 ` [PATCH 14/17] qemu: x86: Debug register emulation Jan Kiszka
@ 2008-10-07 12:15 ` Avi Kivity
2008-10-08 20:25 ` Jan Kiszka
0 siblings, 1 reply; 31+ messages in thread
From: Avi Kivity @ 2008-10-07 12:15 UTC (permalink / raw)
To: Jan Kiszka; +Cc: kvm
Jan Kiszka wrote:
> Built on top of previously enhanced breakpoint/watchpoint support, this
> patch adds full debug register emulation for the x86 architecture.
>
> Many corner cases were considered, and the result was successfully
> tested inside a Linux guest with gdb, but I won't be surprised if one
> or two scenarios still behave differently in reality.
>
> +
> +static inline int hw_breakpoint_len(unsigned long dr7, int index)
> +{
> + return ((dr7 >> (DR7_LEN_SHIFT + (index * 2))) & 3) + 1;
> +}
> +
>
A len encoding of 2 means an 8-byte breakpoint.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 14/17] qemu: x86: Debug register emulation
2008-10-07 12:15 ` Avi Kivity
@ 2008-10-08 20:25 ` Jan Kiszka
0 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-08 20:25 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
[-- Attachment #1: Type: text/plain, Size: 11738 bytes --]
Avi Kivity wrote:
> Jan Kiszka wrote:
>> Built on top of previously enhanced breakpoint/watchpoint support, this
>> patch adds full debug register emulation for the x86 architecture.
>>
>> Many corner cases were considered, and the result was successfully
>> tested inside a Linux guest with gdb, but I won't be surprised if one
>> or two scenarios still behave differently in reality.
>>
>> +
>> +static inline int hw_breakpoint_len(unsigned long dr7, int index)
>> +{
>> + return ((dr7 >> (DR7_LEN_SHIFT + (index * 2))) & 3) + 1;
>> +}
>> +
>>
>
> A len encoding of 2 means an 8-byte breakpoint.
>
True, fixed version follows:
------------
Built on top of previously enhanced breakpoint/watchpoint support, this
patch adds full debug register emulation for the x86 architecture.
Many corner cases were considered, and the result was successfully
tested inside a Linux guest with gdb, but I won't be surprised if one
or two scenarios still behave differently in reality.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/linux-user/main.c | 4 -
qemu/target-i386/cpu.h | 34 +++++++++++
qemu/target-i386/helper.c | 126 +++++++++++++++++++++++++++++++------------
qemu/target-i386/op_helper.c | 79 +++++++++++++++++++++++++-
4 files changed, 203 insertions(+), 40 deletions(-)
Index: b/qemu/linux-user/main.c
===================================================================
--- a/qemu/linux-user/main.c
+++ b/qemu/linux-user/main.c
@@ -406,7 +406,7 @@ void cpu_loop(CPUX86State *env)
queue_signal(env, info.si_signo, &info);
}
break;
- case EXCP01_SSTP:
+ case EXCP01_DB:
case EXCP03_INT3:
#ifndef TARGET_X86_64
if (env->eflags & VM_MASK) {
@@ -416,7 +416,7 @@ void cpu_loop(CPUX86State *env)
{
info.si_signo = SIGTRAP;
info.si_errno = 0;
- if (trapnr == EXCP01_SSTP) {
+ if (trapnr == EXCP01_DB) {
info.si_code = TARGET_TRAP_BRKPT;
info._sifields._sigfault._addr = env->eip;
} else {
Index: b/qemu/target-i386/cpu.h
===================================================================
--- a/qemu/target-i386/cpu.h
+++ b/qemu/target-i386/cpu.h
@@ -205,6 +205,16 @@
#define CR4_OSFXSR_MASK (1 << CR4_OSFXSR_SHIFT)
#define CR4_OSXMMEXCPT_MASK (1 << 10)
+#define DR6_BD (1 << 13)
+#define DR6_BS (1 << 14)
+#define DR6_BT (1 << 15)
+#define DR6_FIXED_1 0xffff0ff0
+
+#define DR7_GD (1 << 13)
+#define DR7_TYPE_SHIFT 16
+#define DR7_LEN_SHIFT 18
+#define DR7_FIXED_1 0x00000400
+
#define PG_PRESENT_BIT 0
#define PG_RW_BIT 1
#define PG_USER_BIT 2
@@ -361,7 +371,7 @@
#define CPUID_MWAIT_EMX (1 << 0) /* enumeration supported */
#define EXCP00_DIVZ 0
-#define EXCP01_SSTP 1
+#define EXCP01_DB 1
#define EXCP02_NMI 2
#define EXCP03_INT3 3
#define EXCP04_INTO 4
@@ -594,6 +604,10 @@ typedef struct CPUX86State {
int exception_is_int;
target_ulong exception_next_eip;
target_ulong dr[8]; /* debug registers */
+ union {
+ CPUBreakpoint *cpu_breakpoint[4];
+ CPUWatchpoint *cpu_watchpoint[4];
+ }; /* break/watchpoints for dr[0..3] */
uint32_t smbase;
int old_exception; /* exception in flight */
@@ -789,6 +803,24 @@ static inline void cpu_clone_regs(CPUSta
#define CPU_PC_FROM_TB(env, tb) env->eip = tb->pc - tb->cs_base
+static inline int hw_breakpoint_enabled(unsigned long dr7, int index)
+{
+ return (dr7 >> (index * 2)) & 3;
+}
+
+static inline int hw_breakpoint_type(unsigned long dr7, int index)
+{
+ return (dr7 >> (DR7_TYPE_SHIFT + (index * 2))) & 3;
+}
+
+static inline int hw_breakpoint_len(unsigned long dr7, int index)
+{
+ int len = ((dr7 >> (DR7_LEN_SHIFT + (index * 2))) & 3);
+ return (len == 2) ? 8 : len + 1;
+}
+
+int check_hw_breakpoints(CPUState *env, int force_dr6_update);
+
#include "cpu-all.h"
#include "svm.h"
Index: b/qemu/target-i386/helper.c
===================================================================
--- a/qemu/target-i386/helper.c
+++ b/qemu/target-i386/helper.c
@@ -34,8 +34,6 @@
//#define DEBUG_MMU
-static int cpu_x86_register (CPUX86State *env, const char *cpu_model);
-
static void add_flagname_to_bitmaps(char *flagname, uint32_t *features,
uint32_t *ext_features,
uint32_t *ext2_features,
@@ -95,37 +93,6 @@ static void add_flagname_to_bitmaps(char
extern const char *cpu_vendor_string;
-CPUX86State *cpu_x86_init(const char *cpu_model)
-{
- CPUX86State *env;
- static int inited;
-
- env = qemu_mallocz(sizeof(CPUX86State));
- if (!env)
- return NULL;
- cpu_exec_init(env);
- env->cpu_model_str = cpu_model;
-
- /* init various static tables */
- if (!inited) {
- inited = 1;
- optimize_flags_init();
- }
- if (cpu_x86_register(env, cpu_model) < 0) {
- cpu_x86_close(env);
- return NULL;
- }
- cpu_reset(env);
-#ifdef USE_KQEMU
- kqemu_init(env);
-#endif
-#ifdef USE_KVM
- if (kvm_enabled())
- kvm_init_new_ap(env->cpu_index, env);
-#endif
- return env;
-}
-
typedef struct x86_def_t {
const char *name;
uint32_t level;
@@ -482,6 +449,12 @@ void cpu_reset(CPUX86State *env)
env->fpuc = 0x37f;
env->mxcsr = 0x1f80;
+
+ memset(env->dr, 0, sizeof(env->dr));
+ env->dr[6] = DR6_FIXED_1;
+ env->dr[7] = DR7_FIXED_1;
+ cpu_breakpoint_remove_all(env, BP_CPU);
+ cpu_watchpoint_remove_all(env, BP_CPU);
}
void cpu_x86_close(CPUX86State *env)
@@ -1278,4 +1251,91 @@ target_phys_addr_t cpu_get_phys_page_deb
paddr = (pte & TARGET_PAGE_MASK) + page_offset;
return paddr;
}
+
+int check_hw_breakpoints(CPUState *env, int force_dr6_update)
+{
+ target_ulong dr6;
+ int reg, type;
+ int hit_enabled = 0;
+
+ dr6 = env->dr[6] & ~0xf;
+ for (reg = 0; reg < 4; reg++) {
+ type = hw_breakpoint_type(env->dr[7], reg);
+ if ((type == 0 && env->dr[reg] == env->eip) ||
+ ((type & 1) && env->cpu_watchpoint[reg] &&
+ (env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT))) {
+ dr6 |= 1 << reg;
+ if (hw_breakpoint_enabled(env->dr[7], reg))
+ hit_enabled = 1;
+ }
+ }
+ if (hit_enabled || force_dr6_update)
+ env->dr[6] = dr6;
+ return hit_enabled;
+}
+
+static CPUDebugExcpHandler *prev_debug_excp_handler;
+
+void raise_exception(int exception_index);
+
+static void breakpoint_handler(CPUState *env)
+{
+ CPUBreakpoint *bp;
+
+ if (env->watchpoint_hit) {
+ if (env->watchpoint_hit->flags & BP_CPU) {
+ env->watchpoint_hit = NULL;
+ if (check_hw_breakpoints(env, 0))
+ raise_exception(EXCP01_DB);
+ else
+ cpu_resume_from_signal(env, NULL);
+ }
+ } else {
+ for (bp = env->breakpoints; bp != NULL; bp = bp->next)
+ if (bp->pc == env->eip) {
+ if (bp->flags & BP_CPU) {
+ check_hw_breakpoints(env, 1);
+ raise_exception(EXCP01_DB);
+ }
+ break;
+ }
+ }
+ if (prev_debug_excp_handler)
+ prev_debug_excp_handler(env);
+}
#endif /* !CONFIG_USER_ONLY */
+
+CPUX86State *cpu_x86_init(const char *cpu_model)
+{
+ CPUX86State *env;
+ static int inited;
+
+ env = qemu_mallocz(sizeof(CPUX86State));
+ if (!env)
+ return NULL;
+ cpu_exec_init(env);
+ env->cpu_model_str = cpu_model;
+
+ /* init various static stuff */
+ if (!inited) {
+ inited = 1;
+ optimize_flags_init();
+#ifndef CONFIG_USER_ONLY
+ prev_debug_excp_handler =
+ cpu_set_debug_excp_handler(breakpoint_handler);
+#endif
+ }
+ if (cpu_x86_register(env, cpu_model) < 0) {
+ cpu_x86_close(env);
+ return NULL;
+ }
+ cpu_reset(env);
+#ifdef USE_KQEMU
+ kqemu_init(env);
+#endif
+#ifdef USE_KVM
+ if (kvm_enabled())
+ kvm_init_new_ap(env->cpu_index, env);
+#endif
+ return env;
+}
Index: b/qemu/target-i386/op_helper.c
===================================================================
--- a/qemu/target-i386/op_helper.c
+++ b/qemu/target-i386/op_helper.c
@@ -94,6 +94,53 @@ const CPU86_LDouble f15rk[7] =
3.32192809488736234781L, /*l2t*/
};
+static void hw_breakpoint_insert(int index)
+{
+ int type, err = 0;
+
+ switch (hw_breakpoint_type(env->dr[7], index)) {
+ case 0:
+ if (hw_breakpoint_enabled(env->dr[7], index))
+ err = cpu_breakpoint_insert(env, env->dr[index], BP_CPU,
+ &env->cpu_breakpoint[index]);
+ break;
+ case 1:
+ type = BP_CPU | BP_MEM_WRITE;
+ goto insert_wp;
+ case 2:
+ /* No support for I/O watchpoints yet */
+ break;
+ case 3:
+ type = BP_CPU | BP_MEM_ACCESS;
+ insert_wp:
+ err = cpu_watchpoint_insert(env, env->dr[index],
+ hw_breakpoint_len(env->dr[7], index),
+ type, &env->cpu_watchpoint[index]);
+ break;
+ }
+ if (err)
+ env->cpu_breakpoint[index] = NULL;
+}
+
+static void hw_breakpoint_remove(int index)
+{
+ if (!env->cpu_breakpoint[index])
+ return;
+ switch (hw_breakpoint_type(env->dr[7], index)) {
+ case 0:
+ if (hw_breakpoint_enabled(env->dr[7], index))
+ cpu_breakpoint_remove_by_ref(env, env->cpu_breakpoint[index]);
+ break;
+ case 1:
+ case 3:
+ cpu_watchpoint_remove_by_ref(env, env->cpu_watchpoint[index]);
+ break;
+ case 2:
+ /* No support for I/O watchpoints yet */
+ break;
+ }
+}
+
/* broken thread support */
spinlock_t global_cpu_lock = SPIN_LOCK_UNLOCKED;
@@ -496,6 +543,15 @@ static void switch_tss(int tss_selector,
/* XXX: different exception if CALL ? */
raise_exception_err(EXCP0D_GPF, 0);
}
+
+ /* reset local breakpoints */
+ if (env->dr[7] & 0x55) {
+ for (i = 0; i < 4; i++) {
+ if (hw_breakpoint_enabled(env->dr[7], i) == 0x1)
+ hw_breakpoint_remove(i);
+ }
+ env->dr[7] &= ~0x55;
+ }
}
/* check if Port I/O is allowed in TSS */
@@ -1879,8 +1935,11 @@ void helper_cmpxchg16b(target_ulong a0)
void helper_single_step(void)
{
- env->dr[6] |= 0x4000;
- raise_exception(EXCP01_SSTP);
+#ifndef CONFIG_USER_ONLY
+ check_hw_breakpoints(env, 1);
+#endif
+ env->dr[6] |= DR6_BS;
+ raise_exception(EXCP01_DB);
}
void helper_cpuid(void)
@@ -3082,10 +3141,22 @@ void helper_clts(void)
env->hflags &= ~HF_TS_MASK;
}
-/* XXX: do more */
void helper_movl_drN_T0(int reg, target_ulong t0)
{
- env->dr[reg] = t0;
+ int i;
+
+ if (reg < 4) {
+ hw_breakpoint_remove(reg);
+ env->dr[reg] = t0;
+ hw_breakpoint_insert(reg);
+ } else if (reg == 7) {
+ for (i = 0; i < 4; i++)
+ hw_breakpoint_remove(i);
+ env->dr[7] = t0;
+ for (i = 0; i < 4; i++)
+ hw_breakpoint_insert(i);
+ } else
+ env->dr[reg] = t0;
}
void helper_invlpg(target_ulong addr)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 15/17] kvm-userspace: Switch to new guest debug interface
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
` (13 preceding siblings ...)
2008-10-06 9:14 ` [PATCH 14/17] qemu: x86: Debug register emulation Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-06 9:14 ` [PATCH 16/17] kvm-userspace: Provide compat wrapper for set_debugreg Jan Kiszka
` (3 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: kvm-userspace-new-guest-debug-interface.patch --]
[-- Type: text/plain, Size: 21184 bytes --]
This patch switches both libkvm as well as the qemu pieces over to the
new guest debug interface. It comes with full support for software-based
breakpoints (via guest code modification), hardware-assisted breakpoints
and watchpoints (x86-only so far).
Breakpoint management is done inside qemu-kvm, transparent to gdbstub
and also avoiding that the gdb frontend takes over. This allows for
running debuggers inside the guest while guest debugging it active,
because the host can cleanly tell apart host- and guest-originated
breakpoint events.
Yet improvable are x86 corner cases when using single-step ("forgotten"
debug flags on the guest's stack). And, of course, the yet empty non-x86
helper functions have to be populated.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
libkvm/libkvm.c | 14 ++-
libkvm/libkvm.h | 9 +-
qemu/exec.c | 10 +-
qemu/gdbstub.c | 18 ++--
qemu/gdbstub.h | 7 +
qemu/qemu-kvm-ia64.c | 35 ++++++++
qemu/qemu-kvm-powerpc.c | 35 ++++++++
qemu/qemu-kvm-x86.c | 172 +++++++++++++++++++++++++++++++++++++++++++
qemu/qemu-kvm.c | 192 ++++++++++++++++++++++++++++++++++++++++++------
qemu/qemu-kvm.h | 29 +++++++
user/main.c | 7 +
11 files changed, 489 insertions(+), 39 deletions(-)
Index: b/libkvm/libkvm.c
===================================================================
--- a/libkvm/libkvm.c
+++ b/libkvm/libkvm.c
@@ -674,7 +674,13 @@ static int handle_io(kvm_context_t kvm,
int handle_debug(kvm_context_t kvm, int vcpu)
{
- return kvm->callbacks->debug(kvm->opaque, vcpu);
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+ struct kvm_run *run = kvm->run[vcpu];
+
+ return kvm->callbacks->debug(kvm->opaque, vcpu, &run->debug.arch);
+#else
+ return 0;
+#endif
}
int kvm_get_regs(kvm_context_t kvm, int vcpu, struct kvm_regs *regs)
@@ -930,10 +936,12 @@ int kvm_inject_irq(kvm_context_t kvm, in
return ioctl(kvm->vcpu_fd[vcpu], KVM_INTERRUPT, &intr);
}
-int kvm_guest_debug(kvm_context_t kvm, int vcpu, struct kvm_debug_guest *dbg)
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+int kvm_set_guest_debug(kvm_context_t kvm, int vcpu, struct kvm_guest_debug *dbg)
{
- return ioctl(kvm->vcpu_fd[vcpu], KVM_DEBUG_GUEST, dbg);
+ return ioctl(kvm->vcpu_fd[vcpu], KVM_SET_GUEST_DEBUG, dbg);
}
+#endif
int kvm_set_signal_mask(kvm_context_t kvm, int vcpu, const sigset_t *sigset)
{
Index: b/qemu/exec.c
===================================================================
--- a/qemu/exec.c
+++ b/qemu/exec.c
@@ -1488,9 +1488,13 @@ void cpu_single_step(CPUState *env, int
#if defined(TARGET_HAS_ICE)
if (env->singlestep_enabled != enabled) {
env->singlestep_enabled = enabled;
- /* must flush all the translated code to avoid inconsistancies */
- /* XXX: only flush what is necessary */
- tb_flush(env);
+ if (kvm_enabled())
+ kvm_update_guest_debug(env, 0);
+ else {
+ /* must flush all the translated code to avoid inconsistancies */
+ /* XXX: only flush what is necessary */
+ tb_flush(env);
+ }
}
#endif
}
Index: b/qemu/qemu-kvm.c
===================================================================
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -20,6 +20,8 @@ int kvm_pit = 1;
#include "console.h"
#include "block.h"
#include "compatfd.h"
+#include "gdbstub.h"
+#include "monitor.h"
#include "qemu-kvm.h"
#include <libkvm.h>
@@ -72,7 +74,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;
static inline unsigned long kvm_get_thread_id(void)
{
@@ -599,9 +601,10 @@ int kvm_main_loop(void)
qemu_system_powerdown();
else if (qemu_reset_requested())
qemu_kvm_system_reset();
- else if (kvm_debug_stop_requested) {
+ else if (kvm_debug_cpu_requested) {
+ mon_set_cpu(kvm_debug_cpu_requested);
vm_stop(EXCP_DEBUG);
- kvm_debug_stop_requested = 0;
+ kvm_debug_cpu_requested = NULL;
}
}
@@ -611,12 +614,18 @@ int kvm_main_loop(void)
return 0;
}
-static int kvm_debug(void *opaque, int vcpu)
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+int kvm_debug(void *opaque, int vcpu, struct kvm_debug_exit_arch *arch_info)
{
- kvm_debug_stop_requested = 1;
- vcpu_info[vcpu].stopped = 1;
- return 1;
+ int handle = kvm_arch_debug(arch_info);
+
+ if (handle) {
+ kvm_debug_cpu_requested = cpu_single_env;
+ vcpu_info[vcpu].stopped = 1;
+ }
+ return handle;
}
+#endif
static int kvm_inb(void *opaque, uint16_t addr, uint8_t *data)
{
@@ -718,7 +727,9 @@ static int kvm_shutdown(void *opaque, in
}
static struct kvm_callbacks qemu_kvm_ops = {
+#ifdef KVM_CAP_SET_GUEST_DEBUG
.debug = kvm_debug,
+#endif
.inb = kvm_inb,
.inw = kvm_inw,
.inl = kvm_inl,
@@ -835,34 +846,171 @@ int kvm_qemu_init_env(CPUState *cenv)
return kvm_arch_qemu_init_env(cenv);
}
-struct kvm_guest_debug_data {
- struct kvm_debug_guest dbg;
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+struct kvm_sw_breakpoint *first_sw_breakpoint;
+
+struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(target_ulong pc)
+{
+ struct kvm_sw_breakpoint *bp = first_sw_breakpoint;
+
+ while (bp) {
+ if (bp->pc == pc)
+ break;
+ bp = bp->next;
+ }
+ return bp;
+}
+
+struct kvm_set_guest_debug_data {
+ struct kvm_guest_debug dbg;
int err;
};
-void kvm_invoke_guest_debug(void *data)
+void kvm_invoke_set_guest_debug(void *data)
{
- struct kvm_guest_debug_data *dbg_data = data;
+ struct kvm_set_guest_debug_data *dbg_data = data;
- dbg_data->err = kvm_guest_debug(kvm_context, cpu_single_env->cpu_index,
- &dbg_data->dbg);
+ dbg_data->err = kvm_set_guest_debug(kvm_context, cpu_single_env->cpu_index,
+ &dbg_data->dbg);
}
-int kvm_update_debugger(CPUState *env)
+int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap)
{
- struct kvm_guest_debug_data data;
+ struct kvm_set_guest_debug_data data;
- memset(data.dbg.breakpoints, 0, sizeof(data.dbg.breakpoints));
+ data.dbg.control = 0;
+ if (env->singlestep_enabled)
+ data.dbg.control = KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
- data.dbg.enabled = 0;
- if (env->singlestep_enabled) {
- data.dbg.enabled = 1;
- data.dbg.singlestep = env->singlestep_enabled;
- }
- on_vcpu(env, kvm_invoke_guest_debug, &data);
+ kvm_arch_update_guest_debug(env, &data.dbg);
+ data.dbg.control |= reinject_trap;
+
+ on_vcpu(env, kvm_invoke_set_guest_debug, &data);
return data.err;
}
+int kvm_insert_breakpoint(target_ulong addr, target_ulong len, int type)
+{
+ struct kvm_sw_breakpoint *bp;
+ CPUState *env;
+ int err;
+
+ if (type == GDB_BREAKPOINT_SW) {
+ bp = kvm_find_sw_breakpoint(addr);
+ if (bp) {
+ bp->usecount++;
+ return 0;
+ }
+
+ bp = qemu_malloc(sizeof(struct kvm_sw_breakpoint));
+ if (!bp)
+ return -ENOMEM;
+
+ bp->pc = addr;
+ bp->usecount = 1;
+ bp->prev = NULL;
+ bp->next = first_sw_breakpoint;
+ err = kvm_arch_insert_sw_breakpoint(bp);
+ if (err) {
+ free(bp);
+ return err;
+ }
+
+ first_sw_breakpoint = bp;
+ if (bp->next)
+ bp->next->prev = bp;
+ } else {
+ err = kvm_arch_insert_hw_breakpoint(addr, len, type);
+ if (err)
+ return err;
+ }
+
+ for (env = first_cpu; env != NULL; env = env->next_cpu) {
+ err = kvm_update_guest_debug(env, 0);
+ if (err)
+ return err;
+ }
+ return 0;
+}
+
+int kvm_remove_breakpoint(target_ulong addr, target_ulong len, int type)
+{
+ struct kvm_sw_breakpoint *bp;
+ CPUState *env;
+ int err;
+
+ if (type == GDB_BREAKPOINT_SW) {
+ bp = kvm_find_sw_breakpoint(addr);
+ if (!bp)
+ return -ENOENT;
+
+ if (bp->usecount > 1) {
+ bp->usecount--;
+ return 0;
+ }
+
+ err = kvm_arch_remove_sw_breakpoint(bp);
+ if (err)
+ return err;
+
+ if (bp->prev)
+ bp->prev->next = bp->next;
+ else
+ first_sw_breakpoint = bp->next;
+ if (bp->next)
+ bp->next->prev = bp->prev;
+
+ qemu_free(bp);
+ } else {
+ err = kvm_arch_remove_hw_breakpoint(addr, len, type);
+ if (err)
+ return err;
+ }
+
+ for (env = first_cpu; env != NULL; env = env->next_cpu) {
+ err = kvm_update_guest_debug(env, 0);
+ if (err)
+ return err;
+ }
+ return 0;
+}
+
+void kvm_remove_all_breakpoints(void)
+{
+ struct kvm_sw_breakpoint *bp = first_sw_breakpoint;
+ CPUState *env;
+
+ while (bp) {
+ kvm_arch_remove_sw_breakpoint(bp);
+ bp = bp->next;
+ }
+ kvm_arch_remove_all_hw_breakpoints();
+
+ for (env = first_cpu; env != NULL; env = env->next_cpu)
+ kvm_update_guest_debug(env, 0);
+}
+
+#else /* !KVM_CAP_SET_GUEST_DEBUG */
+
+int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap)
+{
+ return -EINVAL;
+}
+
+int kvm_insert_breakpoint(target_ulong addr, target_ulong len, int type)
+{
+ return -EINVAL;
+}
+
+int kvm_remove_breakpoint(target_ulong addr, target_ulong len, int type)
+{
+ return -EINVAL;
+}
+
+void kvm_remove_all_breakpoints(void)
+{
+}
+#endif /* !KVM_CAP_SET_GUEST_DEBUG */
/*
* dirty pages logging
Index: b/libkvm/libkvm.h
===================================================================
--- a/libkvm/libkvm.h
+++ b/libkvm/libkvm.h
@@ -55,7 +55,10 @@ struct kvm_callbacks {
/// generic memory writes to unmapped memory (For MMIO devices)
int (*mmio_write)(void *opaque, uint64_t addr, uint8_t *data,
int len);
- int (*debug)(void *opaque, int vcpu);
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+ int (*debug)(void *opaque, int vcpu,
+ struct kvm_debug_exit_arch *arch_info);
+#endif
/*!
* \brief Called when the VCPU issues an 'hlt' instruction.
*
@@ -359,7 +362,9 @@ static inline int kvm_reset_mpstate(kvm_
*/
int kvm_inject_irq(kvm_context_t kvm, int vcpu, unsigned irq);
-int kvm_guest_debug(kvm_context_t, int vcpu, struct kvm_debug_guest *dbg);
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+int kvm_set_guest_debug(kvm_context_t, int vcpu, struct kvm_guest_debug *dbg);
+#endif
#if defined(__i386__) || defined(__x86_64__)
/*!
Index: b/qemu/qemu-kvm.h
===================================================================
--- a/qemu/qemu-kvm.h
+++ b/qemu/qemu-kvm.h
@@ -23,7 +23,10 @@ void kvm_save_registers(CPUState *env);
void kvm_load_mpstate(CPUState *env);
void kvm_save_mpstate(CPUState *env);
int kvm_cpu_exec(CPUState *env);
-int kvm_update_debugger(CPUState *env);
+int kvm_insert_breakpoint(target_ulong addr, target_ulong len, int type);
+int kvm_remove_breakpoint(target_ulong addr, target_ulong len, int type);
+void kvm_remove_all_breakpoints(void);
+int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap);
int kvm_qemu_init_env(CPUState *env);
int kvm_qemu_check_extension(int ext);
void kvm_apic_init(CPUState *env);
@@ -66,6 +69,30 @@ int kvm_arch_try_push_nmi(void *opaque);
void kvm_arch_update_regs_for_sipi(CPUState *env);
void kvm_arch_cpu_reset(CPUState *env);
+struct kvm_guest_debug;
+struct kvm_debug_exit_arch;
+
+struct kvm_sw_breakpoint {
+ target_ulong pc;
+ target_ulong saved_insn;
+ int usecount;
+ struct kvm_sw_breakpoint *prev;
+ struct kvm_sw_breakpoint *next;
+};
+
+extern struct kvm_sw_breakpoint *first_sw_breakpoint;
+
+int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info);
+struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(target_ulong pc);
+int kvm_arch_insert_sw_breakpoint(struct kvm_sw_breakpoint *bp);
+int kvm_arch_remove_sw_breakpoint(struct kvm_sw_breakpoint *bp);
+int kvm_arch_insert_hw_breakpoint(target_ulong addr,
+ target_ulong len, int type);
+int kvm_arch_remove_hw_breakpoint(target_ulong addr,
+ target_ulong len, int type);
+void kvm_arch_remove_all_hw_breakpoints(void);
+void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg);
+
CPUState *qemu_kvm_cpu_env(int index);
void qemu_kvm_aio_wait_start(void);
Index: b/qemu/qemu-kvm-ia64.c
===================================================================
--- a/qemu/qemu-kvm-ia64.c
+++ b/qemu/qemu-kvm-ia64.c
@@ -66,6 +66,41 @@ void kvm_arch_update_regs_for_sipi(CPUSt
{
}
+int kvm_arch_insert_sw_breakpoint(struct kvm_sw_breakpoint *bp)
+{
+ return -EINVAL;
+}
+
+int kvm_arch_remove_sw_breakpoint(struct kvm_sw_breakpoint *bp)
+{
+ return -EINVAL;
+}
+
+int kvm_arch_insert_hw_breakpoint(target_ulong addr,
+ target_ulong len, int type)
+{
+ return -ENOSYS;
+}
+
+int kvm_arch_remove_hw_breakpoint(target_ulong addr,
+ target_ulong len, int type)
+{
+ return -ENOSYS;
+}
+
+void kvm_arch_remove_all_hw_breakpoints(void)
+{
+}
+
+int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info)
+{
+ return 0;
+}
+
+void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg)
+{
+}
+
void kvm_save_mpstate(CPUState *env)
{
#ifdef KVM_CAP_MP_STATE
Index: b/qemu/qemu-kvm-powerpc.c
===================================================================
--- a/qemu/qemu-kvm-powerpc.c
+++ b/qemu/qemu-kvm-powerpc.c
@@ -216,3 +216,38 @@ int handle_powerpc_dcr_write(int vcpu, u
void kvm_arch_cpu_reset(CPUState *env)
{
}
+
+int kvm_arch_insert_sw_breakpoint(struct kvm_sw_breakpoint *bp)
+{
+ return -EINVAL;
+}
+
+int kvm_arch_remove_sw_breakpoint(struct kvm_sw_breakpoint *bp)
+{
+ return -EINVAL;
+}
+
+int kvm_arch_insert_hw_breakpoint(target_ulong addr,
+ target_ulong len, int type)
+{
+ return -ENOSYS;
+}
+
+int kvm_arch_remove_hw_breakpoint(target_ulong addr,
+ target_ulong len, int type)
+{
+ return -ENOSYS;
+}
+
+void kvm_arch_remove_all_hw_breakpoints(void)
+{
+}
+
+int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info)
+{
+ return 0;
+}
+
+void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg)
+{
+}
Index: b/qemu/qemu-kvm-x86.c
===================================================================
--- a/qemu/qemu-kvm-x86.c
+++ b/qemu/qemu-kvm-x86.c
@@ -11,6 +11,8 @@
#include <string.h>
#include "hw/hw.h"
+#include "monitor.h"
+#include "gdbstub.h"
#include "qemu-kvm.h"
#include <libkvm.h>
@@ -717,3 +719,173 @@ void kvm_arch_cpu_reset(CPUState *env)
}
}
}
+
+int kvm_arch_insert_sw_breakpoint(struct kvm_sw_breakpoint *bp)
+{
+ uint8_t int3 = 0xcc;
+
+ if (cpu_memory_rw_debug(mon_get_cpu(), bp->pc, (uint8_t *)&bp->saved_insn,
+ 1, 0) ||
+ cpu_memory_rw_debug(mon_get_cpu(), bp->pc, &int3, 1, 1))
+ return -EINVAL;
+ return 0;
+}
+
+int kvm_arch_remove_sw_breakpoint(struct kvm_sw_breakpoint *bp)
+{
+ if (cpu_memory_rw_debug(mon_get_cpu(), bp->pc, (uint8_t *)&bp->saved_insn,
+ 1, 1))
+ return -EINVAL;
+ return 0;
+}
+
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+static struct {
+ target_ulong addr;
+ int len;
+ int type;
+} hw_breakpoint[4];
+
+static int nb_hw_breakpoint;
+
+static int find_hw_breakpoint(target_ulong addr, int len, int type)
+{
+ int n;
+
+ for (n = 0; n < nb_hw_breakpoint; n++)
+ if (hw_breakpoint[n].addr == addr && hw_breakpoint[n].type == type &&
+ (hw_breakpoint[n].len == len || len == -1))
+ return n;
+ return -1;
+}
+
+int kvm_arch_insert_hw_breakpoint(target_ulong addr,
+ target_ulong len, int type)
+{
+ switch (type) {
+ case GDB_BREAKPOINT_HW:
+ len = 1;
+ break;
+ case GDB_WATCHPOINT_WRITE:
+ case GDB_WATCHPOINT_ACCESS:
+ switch (len) {
+ case 1:
+ break;
+ case 2:
+ case 4:
+ case 8:
+ if (addr & (len - 1))
+ return -EINVAL;
+ break;
+ default:
+ return -EINVAL;
+ }
+ break;
+ default:
+ return -ENOSYS;
+ }
+
+ if (nb_hw_breakpoint == 4)
+ return -ENOBUFS;
+
+ if (find_hw_breakpoint(addr, len, type) >= 0)
+ return -EEXIST;
+
+ hw_breakpoint[nb_hw_breakpoint].addr = addr;
+ hw_breakpoint[nb_hw_breakpoint].len = len;
+ hw_breakpoint[nb_hw_breakpoint].type = type;
+ nb_hw_breakpoint++;
+
+ return 0;
+}
+
+int kvm_arch_remove_hw_breakpoint(target_ulong addr,
+ target_ulong len, int type)
+{
+ int n;
+
+ n = find_hw_breakpoint(addr, (type == GDB_BREAKPOINT_HW) ? 1 : len, type);
+ if (n < 0)
+ return -ENOENT;
+
+ nb_hw_breakpoint--;
+ hw_breakpoint[n] = hw_breakpoint[nb_hw_breakpoint];
+
+ return 0;
+}
+
+void kvm_arch_remove_all_hw_breakpoints(void)
+{
+ nb_hw_breakpoint = 0;
+}
+
+static CPUWatchpoint hw_watchpoint;
+
+int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info)
+{
+ int handle = 0;
+ int n;
+
+ if (arch_info->exception == 1) {
+ if (arch_info->dr6 & (1 << 14)) {
+ if (cpu_single_env->singlestep_enabled)
+ handle = 1;
+ } else {
+ for (n = 0; n < 4; n++)
+ if (arch_info->dr6 & (1 << n))
+ switch ((arch_info->dr7 >> (16 + n*4)) & 0x3) {
+ case 0x0:
+ handle = 1;
+ break;
+ case 0x1:
+ handle = 1;
+ cpu_single_env->watchpoint_hit = &hw_watchpoint;
+ hw_watchpoint.vaddr = hw_breakpoint[n].addr;
+ hw_watchpoint.flags = BP_MEM_WRITE;
+ break;
+ case 0x3:
+ handle = 1;
+ cpu_single_env->watchpoint_hit = &hw_watchpoint;
+ hw_watchpoint.vaddr = hw_breakpoint[n].addr;
+ hw_watchpoint.flags = BP_MEM_ACCESS;
+ break;
+ }
+ }
+ } else if (kvm_find_sw_breakpoint(arch_info->pc))
+ handle = 1;
+
+ if (!handle)
+ kvm_update_guest_debug(cpu_single_env,
+ (arch_info->exception == 1) ?
+ KVM_GUESTDBG_INJECT_DB : KVM_GUESTDBG_INJECT_BP);
+
+ return handle;
+}
+
+void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg)
+{
+ const uint8_t type_code[] = {
+ [GDB_BREAKPOINT_HW] = 0x0,
+ [GDB_WATCHPOINT_WRITE] = 0x1,
+ [GDB_WATCHPOINT_ACCESS] = 0x3
+ };
+ const uint8_t len_code[] = {
+ [1] = 0x0, [2] = 0x1, [4] = 0x3, [8] = 0x2
+ };
+ int n;
+
+ if (first_sw_breakpoint)
+ dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
+
+ if (nb_hw_breakpoint > 0) {
+ dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_HW_BP;
+ dbg->arch.debugreg[7] = 0x0600;
+ for (n = 0; n < nb_hw_breakpoint; n++) {
+ dbg->arch.debugreg[n] = hw_breakpoint[n].addr;
+ dbg->arch.debugreg[7] |= (2 << (n * 2)) |
+ (type_code[hw_breakpoint[n].type] << (16 + n*4)) |
+ (len_code[hw_breakpoint[n].len] << (18 + n*4));
+ }
+ }
+}
+#endif
Index: b/user/main.c
===================================================================
--- a/user/main.c
+++ b/user/main.c
@@ -284,11 +284,14 @@ static int test_outl(void *opaque, uint1
return 0;
}
-static int test_debug(void *opaque, int vcpu)
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+static int test_debug(void *opaque, int vcpu,
+ struct kvm_debug_exit_arch *arch_info)
{
printf("test_debug\n");
return 0;
}
+#endif
static int test_halt(void *opaque, int vcpu)
{
@@ -343,7 +346,9 @@ static struct kvm_callbacks test_callbac
.outl = test_outl,
.mmio_read = test_mem_read,
.mmio_write = test_mem_write,
+#ifdef KVM_CAP_SET_GUEST_DEBUG
.debug = test_debug,
+#endif
.halt = test_halt,
.io_window = test_io_window,
.try_push_interrupts = test_try_push_interrupts,
Index: b/qemu/gdbstub.c
===================================================================
--- a/qemu/gdbstub.c
+++ b/qemu/gdbstub.c
@@ -963,13 +963,6 @@ static void cpu_gdb_write_registers(CPUS
#endif
-/* GDB breakpoint/watchpoint types */
-#define GDB_BREAKPOINT_SW 0
-#define GDB_BREAKPOINT_HW 1
-#define GDB_WATCHPOINT_WRITE 2
-#define GDB_WATCHPOINT_READ 3
-#define GDB_WATCHPOINT_ACCESS 4
-
#ifndef CONFIG_USER_ONLY
static const int xlat_gdb_type[] = {
[GDB_WATCHPOINT_WRITE] = BP_GDB | BP_MEM_WRITE,
@@ -983,6 +976,9 @@ static int gdb_breakpoint_insert(target_
CPUState *env;
int err = 0;
+ if (kvm_enabled())
+ return kvm_insert_breakpoint(addr, len, type);
+
switch (type) {
case GDB_BREAKPOINT_SW ... GDB_BREAKPOINT_HW:
for (env = first_cpu; env != NULL; env = env->next_cpu) {
@@ -1011,6 +1007,9 @@ static int gdb_breakpoint_remove(target_
CPUState *env;
int err = 0;
+ if (kvm_enabled())
+ return kvm_remove_breakpoint(addr, len, type);
+
switch (type) {
case GDB_BREAKPOINT_SW ... GDB_BREAKPOINT_HW:
for (env = first_cpu; env != NULL; env = env->next_cpu) {
@@ -1037,6 +1036,11 @@ static void gdb_breakpoint_remove_all(vo
{
CPUState *env;
+ if (kvm_enabled()) {
+ kvm_remove_all_breakpoints();
+ return;
+ }
+
for (env = first_cpu; env != NULL; env = env->next_cpu) {
cpu_breakpoint_remove_all(env, BP_GDB);
#ifndef CONFIG_USER_ONLY
Index: b/qemu/gdbstub.h
===================================================================
--- a/qemu/gdbstub.h
+++ b/qemu/gdbstub.h
@@ -3,6 +3,13 @@
#define DEFAULT_GDBSTUB_PORT "1234"
+/* GDB breakpoint/watchpoint types */
+#define GDB_BREAKPOINT_SW 0
+#define GDB_BREAKPOINT_HW 1
+#define GDB_WATCHPOINT_WRITE 2
+#define GDB_WATCHPOINT_READ 3
+#define GDB_WATCHPOINT_ACCESS 4
+
typedef void (*gdb_syscall_complete_cb)(CPUState *env,
target_ulong ret, target_ulong err);
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 16/17] kvm-userspace: Provide compat wrapper for set_debugreg
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
` (14 preceding siblings ...)
2008-10-06 9:14 ` [PATCH 15/17] kvm-userspace: Switch to new guest debug interface Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-07 12:17 ` Avi Kivity
2008-10-06 9:14 ` [PATCH 17/17] kvm-userspace: remove obsolete special_reload_dr7 hack Jan Kiszka
` (2 subsequent siblings)
18 siblings, 1 reply; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: compat-support-for-set_debugreg.patch --]
[-- Type: text/plain, Size: 922 bytes --]
Older set_debugreg macros did not allow to pass the register number as
constant (without additional typcasting). Catch this as the latest kvm
debug changes make use of this property.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
kernel/external-module-compat-comm.h | 10 ++++++++++
1 file changed, 10 insertions(+)
Index: b/kernel/external-module-compat-comm.h
===================================================================
--- a/kernel/external-module-compat-comm.h
+++ b/kernel/external-module-compat-comm.h
@@ -539,6 +539,16 @@ struct pci_dev *pci_get_bus_and_slot(uns
#endif
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,25) && defined(__x86_64__)
+
+#undef set_debugreg
+#define set_debugreg(value, register) \
+ __asm__("movq %0,%%db" #register \
+ : /* no output */ \
+ :"r" ((unsigned long)value))
+
+#endif
+
#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,21)
#include <linux/relay.h>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 16/17] kvm-userspace: Provide compat wrapper for set_debugreg
2008-10-06 9:14 ` [PATCH 16/17] kvm-userspace: Provide compat wrapper for set_debugreg Jan Kiszka
@ 2008-10-07 12:17 ` Avi Kivity
2008-10-08 20:25 ` Jan Kiszka
0 siblings, 1 reply; 31+ messages in thread
From: Avi Kivity @ 2008-10-07 12:17 UTC (permalink / raw)
To: Jan Kiszka; +Cc: kvm
Jan Kiszka wrote:
> Older set_debugreg macros did not allow to pass the register number as
> constant (without additional typcasting). Catch this as the latest kvm
> debug changes make use of this property.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> kernel/external-module-compat-comm.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> Index: b/kernel/external-module-compat-comm.h
> ===================================================================
> --- a/kernel/external-module-compat-comm.h
> +++ b/kernel/external-module-compat-comm.h
> @@ -539,6 +539,16 @@ struct pci_dev *pci_get_bus_and_slot(uns
>
> #endif
>
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,25) && defined(__x86_64__)
> +
> +#undef set_debugreg
> +#define set_debugreg(value, register) \
> + __asm__("movq %0,%%db" #register \
> + : /* no output */ \
> + :"r" ((unsigned long)value))
> +
> +#endif
> +
> #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,21)
>
>
Should be in the x86 specific file (kernel/x86/external-module.h)
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 16/17] kvm-userspace: Provide compat wrapper for set_debugreg
2008-10-07 12:17 ` Avi Kivity
@ 2008-10-08 20:25 ` Jan Kiszka
0 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-08 20:25 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
[-- Attachment #1: Type: text/plain, Size: 2108 bytes --]
Avi Kivity wrote:
> Jan Kiszka wrote:
>> Older set_debugreg macros did not allow to pass the register number as
>> constant (without additional typcasting). Catch this as the latest kvm
>> debug changes make use of this property.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>> kernel/external-module-compat-comm.h | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> Index: b/kernel/external-module-compat-comm.h
>> ===================================================================
>> --- a/kernel/external-module-compat-comm.h
>> +++ b/kernel/external-module-compat-comm.h
>> @@ -539,6 +539,16 @@ struct pci_dev *pci_get_bus_and_slot(uns
>>
>> #endif
>>
>> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,25) && defined(__x86_64__)
>> +
>> +#undef set_debugreg
>> +#define set_debugreg(value, register) \
>> + __asm__("movq %0,%%db" #register \
>> + : /* no output */ \
>> + :"r" ((unsigned long)value))
>> +
>> +#endif
>> +
>> #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,21)
>>
>>
>
> Should be in the x86 specific file (kernel/x86/external-module.h)
>
Indeed.
-----------
Older set_debugreg macros did not allow to pass the register number as
constant (without additional typcasting). Catch this as the latest kvm
debug changes make use of this property.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
kernel/x86/external-module-compat.h | 10 ++++++++++
1 file changed, 10 insertions(+)
Index: b/kernel/x86/external-module-compat.h
===================================================================
--- a/kernel/x86/external-module-compat.h
+++ b/kernel/x86/external-module-compat.h
@@ -334,3 +334,13 @@ struct kvm_desc_ptr {
#define FEATURE_CONTROL_LOCKED (1<<0)
#define FEATURE_CONTROL_VMXON_ENABLED (1<<2)
#endif
+
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,25) && defined(__x86_64__)
+
+#undef set_debugreg
+#define set_debugreg(value, register) \
+ __asm__("movq %0,%%db" #register \
+ : /* no output */ \
+ :"r" ((unsigned long)value))
+
+#endif
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 17/17] kvm-userspace: remove obsolete special_reload_dr7 hack
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
` (15 preceding siblings ...)
2008-10-06 9:14 ` [PATCH 16/17] kvm-userspace: Provide compat wrapper for set_debugreg Jan Kiszka
@ 2008-10-06 9:14 ` Jan Kiszka
2008-10-07 12:18 ` [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Avi Kivity
2008-11-17 22:44 ` Markus Armbruster
18 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-06 9:14 UTC (permalink / raw)
To: kvm; +Cc: Jan Kiszka
[-- Attachment #1: remove-special_reload_dr7-hack.patch --]
[-- Type: text/plain, Size: 2004 bytes --]
Host debug registers are now properly saved and restored before/after
entering the guest.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
kernel/x86/external-module-compat.h | 2 --
kernel/x86/hack-module.awk | 4 ----
kernel/x86/preempt.c | 6 ------
3 files changed, 12 deletions(-)
Index: b/kernel/x86/external-module-compat.h
===================================================================
--- a/kernel/x86/external-module-compat.h
+++ b/kernel/x86/external-module-compat.h
@@ -171,7 +171,6 @@ static inline void preempt_notifier_init
void start_special_insn(void);
void end_special_insn(void);
void in_special_section(void);
-void special_reload_dr7(void);
void preempt_notifier_sys_init(void);
void preempt_notifier_sys_exit(void);
@@ -181,7 +180,6 @@ void preempt_notifier_sys_exit(void);
static inline void start_special_insn(void) {}
static inline void end_special_insn(void) {}
static inline void in_special_section(void) {}
-static inline void special_reload_dr7(void) {}
static inline void preempt_notifier_sys_init(void) {}
static inline void preempt_notifier_sys_exit(void) {}
Index: b/kernel/x86/hack-module.awk
===================================================================
--- a/kernel/x86/hack-module.awk
+++ b/kernel/x86/hack-module.awk
@@ -75,10 +75,6 @@ BEGIN { split("INIT_WORK tsc_khz desc_st
{ print }
-/kvm_x86_ops->run/ {
- print "\tspecial_reload_dr7();"
-}
-
/unsigned long flags;/ && vmx_load_host_state {
print "\tunsigned long gsbase;"
}
Index: b/kernel/x86/preempt.c
===================================================================
--- a/kernel/x86/preempt.c
+++ b/kernel/x86/preempt.c
@@ -40,12 +40,6 @@ static void preempt_enable_sched_in_noti
#endif
}
-void special_reload_dr7(void)
-{
- asm volatile ("mov %0, %%db7" : : "r"(0x701ul));
-}
-EXPORT_SYMBOL_GPL(special_reload_dr7);
-
static void __preempt_disable_notifiers(void)
{
asm volatile ("mov %0, %%db7" : : "r"(0ul));
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
` (16 preceding siblings ...)
2008-10-06 9:14 ` [PATCH 17/17] kvm-userspace: remove obsolete special_reload_dr7 hack Jan Kiszka
@ 2008-10-07 12:18 ` Avi Kivity
2008-10-07 12:20 ` Jan Kiszka
2008-11-17 22:44 ` Markus Armbruster
18 siblings, 1 reply; 31+ messages in thread
From: Avi Kivity @ 2008-10-07 12:18 UTC (permalink / raw)
To: Jan Kiszka; +Cc: kvm
Jan Kiszka wrote:
> While still waiting on the required merge of the QEMU bits in this
> series (sigh...), I want to provide an update of my guest debugging and
> x86 debug register improvement patches. First comes the kvm-userspace
> part, kernel bits follow in a separate series.
>
> No new features since my last posting. But thanks to heavy internal use,
> I was able to identify and fix several tricky corner case (/wrt VMX).
> The changes are:
> - rebased on top of the QEMU base series
> - fixed single-stepping over STI and MOV SS/POP SS (VMX)
> - proper compat wrapping for set_debugreg
> - cleanup special_reload_dr7
> - proper injection of soft exceptions like #BP (VMX)
>
> To summarize the contributions of this series (+ its related kernel
> bits):
> - fully functional guest debugging via gdbstub,
> including hardware breakpoints and watchpoints
> (pick up current gdb cvs to have hbreak via remote gdb)
> - (Almost) unlimited number of standard breakpoints
> - SMP guest debugging support
> - x86 debug registers support (makes guest's gdb and kgdb happy)
>
> The patches are in daily use for several moons here and have proven to
> be very helpful for tricky kernel debugging task. Specifically,
> reproducing and then tracking down certain races/deadlocks on SMP boxes
> is far more comfortable with KVM than on "real metal".
>
I'm no expert on the qemu debugger, but apart for a few minor comments
(sent as replies to the relevant patches) this looks good to me.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers
2008-10-07 12:18 ` [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Avi Kivity
@ 2008-10-07 12:20 ` Jan Kiszka
0 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-10-07 12:20 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
Avi Kivity wrote:
> Jan Kiszka wrote:
>> While still waiting on the required merge of the QEMU bits in this
>> series (sigh...), I want to provide an update of my guest debugging and
>> x86 debug register improvement patches. First comes the kvm-userspace
>> part, kernel bits follow in a separate series.
>>
>> No new features since my last posting. But thanks to heavy internal use,
>> I was able to identify and fix several tricky corner case (/wrt VMX).
>> The changes are:
>> - rebased on top of the QEMU base series
>> - fixed single-stepping over STI and MOV SS/POP SS (VMX)
>> - proper compat wrapping for set_debugreg
>> - cleanup special_reload_dr7
>> - proper injection of soft exceptions like #BP (VMX)
>>
>> To summarize the contributions of this series (+ its related kernel
>> bits):
>> - fully functional guest debugging via gdbstub,
>> including hardware breakpoints and watchpoints
>> (pick up current gdb cvs to have hbreak via remote gdb)
>> - (Almost) unlimited number of standard breakpoints
>> - SMP guest debugging support
>> - x86 debug registers support (makes guest's gdb and kgdb happy)
>>
>> The patches are in daily use for several moons here and have proven to
>> be very helpful for tricky kernel debugging task. Specifically,
>> reproducing and then tracking down certain races/deadlocks on SMP boxes
>> is far more comfortable with KVM than on "real metal".
>>
>
> I'm no expert on the qemu debugger, but apart for a few minor comments
> (sent as replies to the relevant patches) this looks good to me.
Thanks a lot for reviewing, specifically also the QEMU part! Will look
into the details later, but they make sense on first glance.
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers
2008-10-06 9:14 [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Jan Kiszka
` (17 preceding siblings ...)
2008-10-07 12:18 ` [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers Avi Kivity
@ 2008-11-17 22:44 ` Markus Armbruster
2008-11-18 9:08 ` Jan Kiszka
18 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2008-11-17 22:44 UTC (permalink / raw)
To: Jan Kiszka; +Cc: kvm
Jan Kiszka <jan.kiszka@siemens.com> writes:
[...]
> To summarize the contributions of this series (+ its related kernel
> bits):
> - fully functional guest debugging via gdbstub,
> including hardware breakpoints and watchpoints
> (pick up current gdb cvs to have hbreak via remote gdb)
> - (Almost) unlimited number of standard breakpoints
> - SMP guest debugging support
> - x86 debug registers support (makes guest's gdb and kgdb happy)
>
> The patches are in daily use for several moons here and have proven to
> be very helpful for tricky kernel debugging task. Specifically,
> reproducing and then tracking down certain races/deadlocks on SMP boxes
> is far more comfortable with KVM than on "real metal".
Sounds intriguing. Could you explain briefly what exactly you do to
wire a debuffer to a guest, so dummies like me can give it a whirl?
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 00/17] kvm-userspace: Fix and improve guest debugging and x86 debug registers
2008-11-17 22:44 ` Markus Armbruster
@ 2008-11-18 9:08 ` Jan Kiszka
0 siblings, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2008-11-18 9:08 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kvm
Markus Armbruster wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
>
> [...]
>> To summarize the contributions of this series (+ its related kernel
>> bits):
>> - fully functional guest debugging via gdbstub,
>> including hardware breakpoints and watchpoints
>> (pick up current gdb cvs to have hbreak via remote gdb)
>> - (Almost) unlimited number of standard breakpoints
>> - SMP guest debugging support
>> - x86 debug registers support (makes guest's gdb and kgdb happy)
>>
>> The patches are in daily use for several moons here and have proven to
>> be very helpful for tricky kernel debugging task. Specifically,
>> reproducing and then tracking down certain races/deadlocks on SMP boxes
>> is far more comfortable with KVM than on "real metal".
>
> Sounds intriguing. Could you explain briefly what exactly you do to
> wire a debuffer to a guest, so dummies like me can give it a whirl?
It's fairly simple, at least for Linux guests: Compile and install a
guest kernel with CONFIG_DEBUG_INFO, then fire up qemu with '-s'. It
will tell you that it's now listening on TCP port 1234 for incoming
remote gdb connections. Next you can start gdb (or some frontend like
ddd) with the recently compiled 'vmlinux' and connect to qemu via
'tar[et] re[mote] :1234'. You are now able to do source-level debugging
with you guest kernel.
Some things you may want to play with:
o Switching between the guest VCPUs (use 'info threads' and
'thread <n>')
o Hardware breakpoints ('hbreak <symbol>'), useful if you don't want
kvm to insert INT3 in the guest code or if the target address is
currently not addressable
o Hardware watchpoints ('watch <expression>')
BTW, the recommended gdb version for full functionality is not yet
released AFAIK: it's post 6.8. Release 6.8 introduced hbreak/watch via
remote links and fixed thread selection for single-stepping, but had
problems here with reading vmlinux symbols.
Yesterday I posted a new version of the qemu bits in this series and I'm
now hoping for their inclusion. A rebase of the kvm part will soon be
set out as well, but if you stumble over problems with what is currently
available for kvm earlier, let me know, will then try to accelerate things.
Enjoy,
Jan
--
Siemens AG, Corporate Technology, CT SE 2 ES-OS
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 31+ messages in thread