qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/15] Enhance debugging support
@ 2008-06-23 14:22 Jan Kiszka
  2008-06-23 14:23 ` [Qemu-devel] [PATCH 1/15] Convert remaining __builtin_expect to likely/unlikely Jan Kiszka
                   ` (14 more replies)
  0 siblings, 15 replies; 16+ messages in thread
From: Jan Kiszka @ 2008-06-23 14:22 UTC (permalink / raw)
  To: qemu-devel

Hi,

it is done, finally: I'm happy to announce the new debug patch series,
now with full x86 debug register support. It took several refactoring
rounds to make at least one person happy about the API design. Hopefully
others will like it, too.

Feedback is welcome. As some patches have broader scope, early feedback
would be very much appreciated.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH 1/15] Convert remaining __builtin_expect to likely/unlikely
  2008-06-23 14:22 [Qemu-devel] [PATCH 0/15] Enhance debugging support Jan Kiszka
@ 2008-06-23 14:23 ` Jan Kiszka
  2008-06-23 14:24 ` [Qemu-devel] [PATCH 2/15] Introduce SSTEP_INTERNAL Jan Kiszka
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2008-06-23 14:23 UTC (permalink / raw)
  To: qemu-devel

QEMU wraps __builtin_expect with the more handy likely/unlikely macros,
but doesn't use them consequently. This patch removes the remaining
__builtin_expect spots.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpu-exec.c              |    6 +++---
 exec-all.h              |    4 ++--
 softmmu_header.h        |   12 ++++++------
 target-arm/op_helper.c  |    2 +-
 target-arm/translate.c  |    4 ++--
 target-cris/op_helper.c |    2 +-
 target-cris/translate.c |    2 +-
 target-m68k/op_helper.c |    2 +-
 target-m68k/translate.c |    4 ++--
 9 files changed, 19 insertions(+), 19 deletions(-)

Index: b/cpu-exec.c
===================================================================
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -221,8 +221,8 @@ static inline TranslationBlock *tb_find_
 #error unsupported CPU
 #endif
     tb = env->tb_jmp_cache[tb_jmp_cache_hash_func(pc)];
-    if (__builtin_expect(!tb || tb->pc != pc || tb->cs_base != cs_base ||
-                         tb->flags != flags, 0)) {
+    if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base ||
+                 tb->flags != flags)) {
         tb = tb_find_slow(pc, cs_base, flags);
     }
     return tb;
@@ -357,7 +357,7 @@ int cpu_exec(CPUState *env1)
             next_tb = 0; /* force lookup of first TB */
             for(;;) {
                 interrupt_request = env->interrupt_request;
-                if (__builtin_expect(interrupt_request, 0) &&
+                if (unlikely(interrupt_request) &&
                     likely(!(env->singlestep_enabled & SSTEP_NOIRQ))) {
                     if (interrupt_request & CPU_INTERRUPT_DEBUG) {
                         env->interrupt_request &= ~CPU_INTERRUPT_DEBUG;
Index: b/exec-all.h
===================================================================
--- a/exec-all.h
+++ b/exec-all.h
@@ -350,8 +350,8 @@ static inline target_ulong get_phys_addr
 
     page_index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
     mmu_idx = cpu_mmu_index(env1);
-    if (__builtin_expect(env1->tlb_table[mmu_idx][page_index].addr_code !=
-                         (addr & TARGET_PAGE_MASK), 0)) {
+    if (unlikely(env1->tlb_table[mmu_idx][page_index].addr_code !=
+                 (addr & TARGET_PAGE_MASK))) {
         ldub_code(addr);
     }
     pd = env1->tlb_table[mmu_idx][page_index].addr_code & ~TARGET_PAGE_MASK;
Index: b/softmmu_header.h
===================================================================
--- a/softmmu_header.h
+++ b/softmmu_header.h
@@ -231,8 +231,8 @@ static inline RES_TYPE glue(glue(ld, USU
     addr = ptr;
     page_index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
     mmu_idx = CPU_MMU_INDEX;
-    if (__builtin_expect(env->tlb_table[mmu_idx][page_index].ADDR_READ !=
-                         (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))), 0)) {
+    if (unlikely(env->tlb_table[mmu_idx][page_index].ADDR_READ !=
+                 (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) {
         res = glue(glue(__ld, SUFFIX), MMUSUFFIX)(addr, mmu_idx);
     } else {
         physaddr = addr + env->tlb_table[mmu_idx][page_index].addend;
@@ -252,8 +252,8 @@ static inline int glue(glue(lds, SUFFIX)
     addr = ptr;
     page_index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
     mmu_idx = CPU_MMU_INDEX;
-    if (__builtin_expect(env->tlb_table[mmu_idx][page_index].ADDR_READ !=
-                         (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))), 0)) {
+    if (unlikely(env->tlb_table[mmu_idx][page_index].ADDR_READ !=
+                 (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) {
         res = (DATA_STYPE)glue(glue(__ld, SUFFIX), MMUSUFFIX)(addr, mmu_idx);
     } else {
         physaddr = addr + env->tlb_table[mmu_idx][page_index].addend;
@@ -277,8 +277,8 @@ static inline void glue(glue(st, SUFFIX)
     addr = ptr;
     page_index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
     mmu_idx = CPU_MMU_INDEX;
-    if (__builtin_expect(env->tlb_table[mmu_idx][page_index].addr_write !=
-                         (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))), 0)) {
+    if (unlikely(env->tlb_table[mmu_idx][page_index].addr_write !=
+                 (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) {
         glue(glue(__st, SUFFIX), MMUSUFFIX)(addr, v, mmu_idx);
     } else {
         physaddr = addr + env->tlb_table[mmu_idx][page_index].addend;
Index: b/target-arm/op_helper.c
===================================================================
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -97,7 +97,7 @@ void tlb_fill (target_ulong addr, int is
     saved_env = env;
     env = cpu_single_env;
     ret = cpu_arm_handle_mmu_fault(env, addr, is_write, mmu_idx, 1);
-    if (__builtin_expect(ret, 0)) {
+    if (unlikely(ret)) {
         if (retaddr) {
             /* now we have a real cpu fault */
             pc = (unsigned long)retaddr;
Index: b/target-arm/translate.c
===================================================================
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -3390,7 +3390,7 @@ static inline void gen_goto_tb(DisasCont
 
 static inline void gen_jmp (DisasContext *s, uint32_t dest)
 {
-    if (__builtin_expect(s->singlestep_enabled, 0)) {
+    if (unlikely(s->singlestep_enabled)) {
         /* An indirect jump so that we still trigger the debug exception.  */
         if (s->thumb)
             dest |= 1;
@@ -8666,7 +8666,7 @@ static inline int gen_intermediate_code_
     /* At this stage dc->condjmp will only be set when the skipped
        instruction was a conditional branch or trap, and the PC has
        already been written.  */
-    if (__builtin_expect(env->singlestep_enabled, 0)) {
+    if (unlikely(env->singlestep_enabled)) {
         /* Make sure the pc is updated, and raise a debug exception.  */
         if (dc->condjmp) {
             gen_set_condexec(dc);
Index: b/target-cris/op_helper.c
===================================================================
--- a/target-cris/op_helper.c
+++ b/target-cris/op_helper.c
@@ -61,7 +61,7 @@ void tlb_fill (target_ulong addr, int is
     D(fprintf(logfile, "%s pc=%x tpc=%x ra=%x\n", __func__, 
 	     env->pc, env->debug1, retaddr));
     ret = cpu_cris_handle_mmu_fault(env, addr, is_write, mmu_idx, 1);
-    if (__builtin_expect(ret, 0)) {
+    if (unlikely(ret)) {
         if (retaddr) {
             /* now we have a real cpu fault */
             pc = (unsigned long)retaddr;
Index: b/target-cris/translate.c
===================================================================
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3173,7 +3173,7 @@ gen_intermediate_code_internal(CPUState 
 
 	cris_evaluate_flags (dc);
 
-	if (__builtin_expect(env->singlestep_enabled, 0)) {
+	if (unlikely(env->singlestep_enabled)) {
 		tcg_gen_movi_tl(env_pc, npc);
 		t_gen_raise_exception(EXCP_DEBUG);
 	} else {
Index: b/target-m68k/op_helper.c
===================================================================
--- a/target-m68k/op_helper.c
+++ b/target-m68k/op_helper.c
@@ -61,7 +61,7 @@ void tlb_fill (target_ulong addr, int is
     saved_env = env;
     env = cpu_single_env;
     ret = cpu_m68k_handle_mmu_fault(env, addr, is_write, mmu_idx, 1);
-    if (__builtin_expect(ret, 0)) {
+    if (unlikely(ret)) {
         if (retaddr) {
             /* now we have a real cpu fault */
             pc = (unsigned long)retaddr;
Index: b/target-m68k/translate.c
===================================================================
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -871,7 +871,7 @@ static void gen_jmp_tb(DisasContext *s, 
     TranslationBlock *tb;
 
     tb = s->tb;
-    if (__builtin_expect (s->singlestep_enabled, 0)) {
+    if (unlikely(s->singlestep_enabled)) {
         gen_exception(s, dest, EXCP_DEBUG);
     } else if ((tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK) ||
                (s->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK)) {
@@ -2974,7 +2974,7 @@ gen_intermediate_code_internal(CPUState 
              !env->singlestep_enabled &&
              (pc_offset) < (TARGET_PAGE_SIZE - 32));
 
-    if (__builtin_expect(env->singlestep_enabled, 0)) {
+    if (unlikely(env->singlestep_enabled)) {
         /* Make sure the pc is updated, and raise a debug exception.  */
         if (!dc->is_jmp) {
             gen_flush_cc_op(dc);

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH 2/15] Introduce SSTEP_INTERNAL
  2008-06-23 14:22 [Qemu-devel] [PATCH 0/15] Enhance debugging support Jan Kiszka
  2008-06-23 14:23 ` [Qemu-devel] [PATCH 1/15] Convert remaining __builtin_expect to likely/unlikely Jan Kiszka
@ 2008-06-23 14:24 ` Jan Kiszka
  2008-06-23 14:24 ` [Qemu-devel] [PATCH 3/15] Replace CF_SINGLE_INSN with SSTEP_INTERNAL - v2 Jan Kiszka
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2008-06-23 14:24 UTC (permalink / raw)
  To: qemu-devel

Introducing SSTEP_INTERNAL, this patch allows to reuse the (host-
injected) single-step infrastructure to let the emulator generate and
execute TBs that only include a single instruction.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpu-all.h               |    7 ++++---
 cpu-exec.c              |    4 +++-
 exec.c                  |    2 ++
 gdbstub.c               |    4 ++--
 target-arm/translate.c  |    2 +-
 target-cris/translate.c |    2 +-
 target-i386/translate.c |    2 +-
 target-m68k/translate.c |    4 ++--
 target-mips/translate.c |    2 +-
 target-ppc/translate.c  |    2 +-
 target-sh4/translate.c  |    6 +++---
 vl.c                    |    7 ++++---
 12 files changed, 25 insertions(+), 19 deletions(-)

Index: b/cpu-all.h
===================================================================
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -804,9 +804,10 @@ int cpu_breakpoint_insert(CPUState *env,
 int cpu_breakpoint_remove(CPUState *env, target_ulong pc);
 void cpu_breakpoint_remove_all(CPUState *env);
 
-#define SSTEP_ENABLE  0x1  /* Enable simulated HW single stepping */
-#define SSTEP_NOIRQ   0x2  /* Do not use IRQ while single stepping */
-#define SSTEP_NOTIMER 0x4  /* Do not Timers while single stepping */
+#define SSTEP_DEBUG    0x1  /* Enable simulated HW single stepping */
+#define SSTEP_NOIRQ    0x2  /* Do not use IRQ while single stepping */
+#define SSTEP_NOTIMER  0x4  /* Do not Timers while single stepping */
+#define SSTEP_INTERNAL 0x8  /* QEMU internal, do not generate breakpoint */
 
 void cpu_single_step(CPUState *env, int enabled);
 void cpu_reset(CPUState *s);
Index: b/cpu-exec.c
===================================================================
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -358,7 +358,8 @@ int cpu_exec(CPUState *env1)
             for(;;) {
                 interrupt_request = env->interrupt_request;
                 if (unlikely(interrupt_request) &&
-                    likely(!(env->singlestep_enabled & SSTEP_NOIRQ))) {
+                    likely(!(env->singlestep_enabled &
+                             (SSTEP_NOIRQ | SSTEP_INTERNAL)))) {
                     if (interrupt_request & CPU_INTERRUPT_DEBUG) {
                         env->interrupt_request &= ~CPU_INTERRUPT_DEBUG;
                         env->exception_index = EXCP_DEBUG;
@@ -614,6 +615,7 @@ int cpu_exec(CPUState *env1)
 #endif
                 next_tb = tcg_qemu_tb_exec(tc_ptr);
                 env->current_tb = NULL;
+                env->singlestep_enabled &= ~SSTEP_INTERNAL;
                 /* reset soft MMU for next block (it can currently
                    only be set by a memory fault) */
 #if defined(USE_KQEMU)
Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -1316,6 +1316,8 @@ int cpu_breakpoint_remove(CPUState *env,
 void cpu_single_step(CPUState *env, int enabled)
 {
 #if defined(TARGET_HAS_ICE)
+    enabled &= SSTEP_DEBUG | SSTEP_NOIRQ | SSTEP_NOTIMER;
+    enabled |= env->singlestep_enabled & SSTEP_INTERNAL;
     if (env->singlestep_enabled != enabled) {
         env->singlestep_enabled = enabled;
         /* must flush all the translated code to avoid inconsistancies */
Index: b/target-arm/translate.c
===================================================================
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -8666,7 +8666,7 @@ static inline int gen_intermediate_code_
     /* At this stage dc->condjmp will only be set when the skipped
        instruction was a conditional branch or trap, and the PC has
        already been written.  */
-    if (unlikely(env->singlestep_enabled)) {
+    if (unlikely(env->singlestep_enabled & SSTEP_DEBUG)) {
         /* Make sure the pc is updated, and raise a debug exception.  */
         if (dc->condjmp) {
             gen_set_condexec(dc);
Index: b/target-cris/translate.c
===================================================================
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3173,7 +3173,7 @@ gen_intermediate_code_internal(CPUState 
 
 	cris_evaluate_flags (dc);
 
-	if (unlikely(env->singlestep_enabled)) {
+	if (unlikely(env->singlestep_enabled & SSTEP_DEBUG)) {
 		tcg_gen_movi_tl(env_pc, npc);
 		t_gen_raise_exception(EXCP_DEBUG);
 	} else {
Index: b/target-i386/translate.c
===================================================================
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -2618,7 +2618,7 @@ static void gen_eob(DisasContext *s)
     if (s->tb->flags & HF_INHIBIT_IRQ_MASK) {
         tcg_gen_helper_0_0(helper_reset_inhibit_irq);
     }
-    if (s->singlestep_enabled) {
+    if (s->singlestep_enabled & SSTEP_DEBUG) {
         tcg_gen_helper_0_0(helper_debug);
     } else if (s->tf) {
 	tcg_gen_helper_0_0(helper_single_step);
Index: b/target-m68k/translate.c
===================================================================
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -871,7 +871,7 @@ static void gen_jmp_tb(DisasContext *s, 
     TranslationBlock *tb;
 
     tb = s->tb;
-    if (unlikely(s->singlestep_enabled)) {
+    if (unlikely(s->singlestep_enabled & SSTEP_DEBUG)) {
         gen_exception(s, dest, EXCP_DEBUG);
     } else if ((tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK) ||
                (s->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK)) {
@@ -2974,7 +2974,7 @@ gen_intermediate_code_internal(CPUState 
              !env->singlestep_enabled &&
              (pc_offset) < (TARGET_PAGE_SIZE - 32));
 
-    if (unlikely(env->singlestep_enabled)) {
+    if (unlikely(env->singlestep_enabled & SSTEP_DEBUG)) {
         /* Make sure the pc is updated, and raise a debug exception.  */
         if (!dc->is_jmp) {
             gen_flush_cc_op(dc);
Index: b/target-mips/translate.c
===================================================================
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -7744,7 +7744,7 @@ gen_intermediate_code_internal (CPUState
         break;
 #endif
     }
-    if (env->singlestep_enabled) {
+    if (env->singlestep_enabled & SSTEP_DEBUG) {
         save_cpu_state(&ctx, ctx.bstate == BS_NONE);
         tcg_gen_helper_0_i(do_raise_exception, EXCP_DEBUG);
     } else {
Index: b/target-ppc/translate.c
===================================================================
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -6319,7 +6319,7 @@ static always_inline int gen_intermediat
     if (ctx.exception == POWERPC_EXCP_NONE) {
         gen_goto_tb(&ctx, 0, ctx.nip);
     } else if (ctx.exception != POWERPC_EXCP_BRANCH) {
-        if (unlikely(env->singlestep_enabled)) {
+        if (unlikely(env->singlestep_enabled & SSTEP_DEBUG)) {
             gen_update_nip(&ctx, ctx.nip);
             gen_op_debug();
         }
Index: b/target-sh4/translate.c
===================================================================
--- a/target-sh4/translate.c
+++ b/target-sh4/translate.c
@@ -161,7 +161,7 @@ static void gen_goto_tb(DisasContext * c
         tcg_gen_exit_tb((long) tb + n);
     } else {
         gen_op_movl_imm_PC(dest);
-        if (ctx->singlestep_enabled)
+        if (ctx->singlestep_enabled & SSTEP_DEBUG)
             gen_op_debug();
         tcg_gen_exit_tb(0);
     }
@@ -173,7 +173,7 @@ static void gen_jump(DisasContext * ctx)
 	/* Target is not statically known, it comes necessarily from a
 	   delayed jump as immediate jump are conditinal jumps */
 	gen_op_movl_delayed_pc_PC();
-	if (ctx->singlestep_enabled)
+	if (ctx->singlestep_enabled & SSTEP_DEBUG)
 	    gen_op_debug();
 	tcg_gen_exit_tb(0);
     } else {
@@ -1251,7 +1251,7 @@ gen_intermediate_code_internal(CPUState 
 	break;
 #endif
     }
-    if (env->singlestep_enabled) {
+    if (env->singlestep_enabled & SSTEP_DEBUG) {
         gen_op_debug();
     } else {
 	switch (ctx.bstate) {
Index: b/vl.c
===================================================================
--- a/vl.c
+++ b/vl.c
@@ -7027,9 +7027,10 @@ void main_loop_wait(int timeout)
     qemu_aio_poll();
 
     if (vm_running) {
-        if (likely(!(cur_cpu->singlestep_enabled & SSTEP_NOTIMER)))
-        qemu_run_timers(&active_timers[QEMU_TIMER_VIRTUAL],
-                        qemu_get_clock(vm_clock));
+        if (likely(!(cur_cpu->singlestep_enabled &
+                     (SSTEP_NOTIMER | SSTEP_INTERNAL))))
+            qemu_run_timers(&active_timers[QEMU_TIMER_VIRTUAL],
+                            qemu_get_clock(vm_clock));
         /* run dma transfers, if any */
         DMA_run();
     }
Index: b/gdbstub.c
===================================================================
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -77,7 +77,7 @@ typedef struct GDBState {
 /* By default use no IRQs and no timers while single stepping so as to
  * make single stepping like an ICE HW step.
  */
-static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
+static int sstep_flags = SSTEP_DEBUG | SSTEP_NOIRQ | SSTEP_NOTIMER;
 
 #ifdef CONFIG_USER_ONLY
 /* XXX: This is not thread safe.  Do we care?  */
@@ -1174,7 +1174,7 @@ static int gdb_handle_packet(GDBState *s
         if (!strcmp(p,"qemu.sstepbits")) {
             /* Query Breakpoint bit definitions */
             sprintf(buf,"ENABLE=%x,NOIRQ=%x,NOTIMER=%x",
-                    SSTEP_ENABLE,
+                    SSTEP_DEBUG,
                     SSTEP_NOIRQ,
                     SSTEP_NOTIMER);
             put_packet(s, buf);

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH 3/15] Replace CF_SINGLE_INSN with SSTEP_INTERNAL - v2
  2008-06-23 14:22 [Qemu-devel] [PATCH 0/15] Enhance debugging support Jan Kiszka
  2008-06-23 14:23 ` [Qemu-devel] [PATCH 1/15] Convert remaining __builtin_expect to likely/unlikely Jan Kiszka
  2008-06-23 14:24 ` [Qemu-devel] [PATCH 2/15] Introduce SSTEP_INTERNAL Jan Kiszka
@ 2008-06-23 14:24 ` Jan Kiszka
  2008-06-23 14:25 ` [Qemu-devel] [PATCH 4/15] Remove unused TB cflags Jan Kiszka
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2008-06-23 14:24 UTC (permalink / raw)
  To: qemu-devel

With the help of SSTEP_INTERNAL, we can overcome CF_SINGLE_INSN and,
thus, tb_gen_code with its setup code.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 exec.c |   75 +++++------------------------------------------------------------
 1 file changed, 6 insertions(+), 69 deletions(-)

Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -746,43 +746,6 @@ static void build_page_bitmap(PageDesc *
     }
 }
 
-#ifdef TARGET_HAS_PRECISE_SMC
-
-static void tb_gen_code(CPUState *env,
-                        target_ulong pc, target_ulong cs_base, int flags,
-                        int cflags)
-{
-    TranslationBlock *tb;
-    uint8_t *tc_ptr;
-    target_ulong phys_pc, phys_page2, virt_page2;
-    int code_gen_size;
-
-    phys_pc = get_phys_addr_code(env, pc);
-    tb = tb_alloc(pc);
-    if (!tb) {
-        /* flush must be done */
-        tb_flush(env);
-        /* cannot fail at this point */
-        tb = tb_alloc(pc);
-    }
-    tc_ptr = code_gen_ptr;
-    tb->tc_ptr = tc_ptr;
-    tb->cs_base = cs_base;
-    tb->flags = flags;
-    tb->cflags = cflags;
-    cpu_gen_code(env, tb, &code_gen_size);
-    code_gen_ptr = (void *)(((unsigned long)code_gen_ptr + code_gen_size + CODE_GEN_ALIGN - 1) & ~(CODE_GEN_ALIGN - 1));
-
-    /* check next page if needed */
-    virt_page2 = (pc + tb->size - 1) & TARGET_PAGE_MASK;
-    phys_page2 = -1;
-    if ((pc & TARGET_PAGE_MASK) != virt_page2) {
-        phys_page2 = get_phys_addr_code(env, virt_page2);
-    }
-    tb_link_phys(tb, phys_pc, phys_page2);
-}
-#endif
-
 /* invalidate all TBs which intersect with the target physical page
    starting in range [start;end[. NOTE: start and end must refer to
    the same physical page. 'is_cpu_write_access' should be true if called
@@ -791,12 +754,11 @@ static void tb_gen_code(CPUState *env,
 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)
@@ -813,9 +775,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;
@@ -842,7 +801,7 @@ void tb_invalidate_phys_page_range(targe
                 }
             }
             if (current_tb == tb &&
-                !(current_tb->cflags & CF_SINGLE_INSN)) {
+                !(env->singlestep_enabled & SSTEP_INTERNAL)) {
                 /* If we are modifying the current TB, we must stop
                 its execution. We could be more precise by checking
                 that the modification is after the current PC, but it
@@ -852,14 +811,6 @@ void tb_invalidate_phys_page_range(targe
                 current_tb_modified = 1;
                 cpu_restore_state(current_tb, env,
                                   env->mem_write_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
@@ -893,8 +844,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,
-                    CF_SINGLE_INSN);
+        env->singlestep_enabled |= SSTEP_INTERNAL;
         cpu_resume_from_signal(env, NULL);
     }
 #endif
@@ -933,8 +883,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
@@ -948,9 +897,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);
@@ -961,7 +907,7 @@ static void tb_invalidate_phys_page(targ
         tb = (TranslationBlock *)((long)tb & ~3);
 #ifdef TARGET_HAS_PRECISE_SMC
         if (current_tb == tb &&
-            !(current_tb->cflags & CF_SINGLE_INSN)) {
+            !(env->singlestep_enabled & SSTEP_INTERNAL)) {
                 /* If we are modifying the current TB, we must stop
                    its execution. We could be more precise by checking
                    that the modification is after the current PC, but it
@@ -970,14 +916,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);
@@ -990,8 +928,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,
-                    CF_SINGLE_INSN);
+        env->singlestep_enabled |= SSTEP_INTERNAL;
         cpu_resume_from_signal(env, puc);
     }
 #endif

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH 4/15] Remove unused TB cflags
  2008-06-23 14:22 [Qemu-devel] [PATCH 0/15] Enhance debugging support Jan Kiszka
                   ` (2 preceding siblings ...)
  2008-06-23 14:24 ` [Qemu-devel] [PATCH 3/15] Replace CF_SINGLE_INSN with SSTEP_INTERNAL - v2 Jan Kiszka
@ 2008-06-23 14:25 ` Jan Kiszka
  2008-06-23 14:26 ` [Qemu-devel] [PATCH 5/15] Return appropriate watch message to gdb Jan Kiszka
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2008-06-23 14:25 UTC (permalink / raw)
  To: qemu-devel

TranslationBlock.cflags is unused, now that no one is interested in
CF_SINGLE_INSN anymore. Also the other CF_* flags have no users. So
let's clean this up.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 exec-all.h              |    4 ----
 exec.c                  |    1 -
 target-i386/translate.c |    6 ++----
 3 files changed, 2 insertions(+), 9 deletions(-)

Index: b/exec-all.h
===================================================================
--- a/exec-all.h
+++ b/exec-all.h
@@ -123,10 +123,6 @@ typedef struct TranslationBlock {
     uint64_t flags; /* flags defining in which context the code was generated */
     uint16_t size;      /* size of target code for this block (1 <=
                            size <= TARGET_PAGE_SIZE) */
-    uint16_t cflags;    /* compile flags */
-#define CF_TB_FP_USED  0x0002 /* fp ops are used in the TB */
-#define CF_FP_USED     0x0004 /* fp ops are used in the TB or in a chained TB */
-#define CF_SINGLE_INSN 0x0008 /* compile only a single instruction */
 
     uint8_t *tc_ptr;    /* pointer to the translated code */
     /* next matching tb for physical address. */
Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -1001,7 +1001,6 @@ TranslationBlock *tb_alloc(target_ulong 
         return NULL;
     tb = &tbs[nb_tbs++];
     tb->pc = pc;
-    tb->cflags = 0;
     return tb;
 }
 
Index: b/target-i386/translate.c
===================================================================
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7105,7 +7105,7 @@ static inline int gen_intermediate_code_
     DisasContext dc1, *dc = &dc1;
     target_ulong pc_ptr;
     uint16_t *gen_opc_end;
-    int j, lj, cflags;
+    int j, lj;
     uint64_t flags;
     target_ulong pc_start;
     target_ulong cs_base;
@@ -7114,7 +7114,6 @@ static inline int gen_intermediate_code_
     pc_start = tb->pc;
     cs_base = tb->cs_base;
     flags = tb->flags;
-    cflags = tb->cflags;
 
     dc->pe = (flags >> HF_PE_SHIFT) & 1;
     dc->code32 = (flags >> HF_CS32_SHIFT) & 1;
@@ -7210,8 +7209,7 @@ static inline int gen_intermediate_code_
            the flag and abort the translation to give the irqs a
            change to be happen */
         if (dc->tf || dc->singlestep_enabled ||
-            (flags & HF_INHIBIT_IRQ_MASK) ||
-            (cflags & CF_SINGLE_INSN)) {
+            (flags & HF_INHIBIT_IRQ_MASK)) {
             gen_jmp_im(pc_ptr - dc->cs_base);
             gen_eob(dc);
             break;

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH 5/15] Return appropriate watch message to gdb
  2008-06-23 14:22 [Qemu-devel] [PATCH 0/15] Enhance debugging support Jan Kiszka
                   ` (3 preceding siblings ...)
  2008-06-23 14:25 ` [Qemu-devel] [PATCH 4/15] Remove unused TB cflags Jan Kiszka
@ 2008-06-23 14:26 ` Jan Kiszka
  2008-06-23 14:26 ` [Qemu-devel] [PATCH 6/15] Refactor and enhance break/watchpoint API - v5 Jan Kiszka
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2008-06-23 14:26 UTC (permalink / raw)
  To: qemu-devel

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>
---
 gdbstub.c |   17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Index: b/gdbstub.c
===================================================================
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1225,6 +1225,7 @@ static void gdb_vm_stopped(void *opaque,
 {
     GDBState *s = opaque;
     char buf[256];
+    const char *type;
     int ret;
 
     if (s->state == RS_SYSCALL)
@@ -1235,8 +1236,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] 16+ messages in thread

* [Qemu-devel] [PATCH 6/15] Refactor and enhance break/watchpoint API - v5
  2008-06-23 14:22 [Qemu-devel] [PATCH 0/15] Enhance debugging support Jan Kiszka
                   ` (4 preceding siblings ...)
  2008-06-23 14:26 ` [Qemu-devel] [PATCH 5/15] Return appropriate watch message to gdb Jan Kiszka
@ 2008-06-23 14:26 ` Jan Kiszka
  2008-06-23 14:27 ` [Qemu-devel] [PATCH 7/15] Extend mem_write_* to mem_access_* Jan Kiszka
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2008-06-23 14:26 UTC (permalink / raw)
  To: qemu-devel

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>
---
 cpu-all.h                |   23 +++--
 cpu-defs.h               |   26 +++---
 exec.c                   |  201 ++++++++++++++++++++++++++++-------------------
 gdbstub.c                |  131 ++++++++++++++++--------------
 target-alpha/translate.c |    7 -
 target-arm/translate.c   |    9 +-
 target-cris/translate.c  |    9 +-
 target-i386/translate.c  |    7 -
 target-m68k/translate.c  |    9 +-
 target-mips/translate.c  |    7 -
 target-ppc/translate.c   |    7 -
 target-sh4/translate.c   |   23 ++---
 target-sparc/translate.c |    7 -
 13 files changed, 272 insertions(+), 194 deletions(-)

Index: b/cpu-all.h
===================================================================
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -797,12 +797,23 @@ extern CPUState *cpu_single_env;
 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_DEBUG    0x1  /* Enable simulated HW single stepping */
 #define SSTEP_NOIRQ    0x2  /* Do not use IRQ while single stepping */
Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -448,7 +448,6 @@ void cpu_exec_init(CPUState *env)
         cpu_index++;
     }
     env->cpu_index = cpu_index;
-    env->nb_watchpoints = 0;
     *penv = env;
 }
 
@@ -1143,107 +1142,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
 }
 
@@ -1702,7 +1744,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);
@@ -1743,8 +1785,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.  */
@@ -2235,13 +2277,12 @@ static void check_watchpoint(int offset,
 {
     CPUState *env = cpu_single_env;
     target_ulong vaddr;
-    int i;
+    CPUWatchpoint *wp;
 
     vaddr = (env->mem_write_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;
         }
@@ -2253,40 +2294,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/gdbstub.c
===================================================================
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -962,10 +962,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
+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;
@@ -985,8 +1039,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') {
@@ -1020,8 +1073,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;
@@ -1110,44 +1162,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 == ',')
@@ -1156,17 +1170,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':
@@ -1236,12 +1249,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:
@@ -1249,10 +1261,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/cpu-defs.h
===================================================================
--- a/cpu-defs.h
+++ b/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)
@@ -130,6 +128,19 @@ typedef struct CPUTLBEntry {
                    sizeof(target_phys_addr_t))];
 } CPUTLBEntry;
 
+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  */  \
@@ -151,16 +162,11 @@ typedef struct CPUTLBEntry {
                                                                         \
     /* 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/target-i386/translate.c
===================================================================
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7105,6 +7105,7 @@ static inline int gen_intermediate_code_
     DisasContext dc1, *dc = &dc1;
     target_ulong pc_ptr;
     uint16_t *gen_opc_end;
+    CPUBreakpoint *bp;
     int j, lj;
     uint64_t flags;
     target_ulong pc_start;
@@ -7180,9 +7181,9 @@ static inline int gen_intermediate_code_
     lj = -1;
 
     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/target-alpha/translate.c
===================================================================
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -1968,6 +1968,7 @@ static always_inline int gen_intermediat
     target_ulong pc_start;
     uint32_t insn;
     uint16_t *gen_opc_end;
+    CPUBreakpoint *bp;
     int j, lj = -1;
     int ret;
 
@@ -1982,9 +1983,9 @@ static always_inline int gen_intermediat
     ctx.pal_mode = env->ipr[IPR_EXC_ADDR] & 1;
 #endif
     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/target-arm/translate.c
===================================================================
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -8535,6 +8535,7 @@ static inline int gen_intermediate_code_
                                                  int search_pc)
 {
     DisasContext dc1, *dc = &dc1;
+    CPUBreakpoint *bp;
     uint16_t *gen_opc_end;
     int j, lj;
     target_ulong pc_start;
@@ -8601,9 +8602,9 @@ static inline int 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);
@@ -8652,7 +8653,7 @@ static inline int 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/target-cris/translate.c
===================================================================
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -2971,10 +2971,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/target-m68k/translate.c
===================================================================
--- a/target-m68k/translate.c
+++ b/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;
@@ -2940,9 +2941,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;
@@ -2968,7 +2969,7 @@ gen_intermediate_code_internal(CPUState 
         /* 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;
     } while (!dc->is_jmp && gen_opc_ptr < gen_opc_end &&
              !env->singlestep_enabled &&
Index: b/target-mips/translate.c
===================================================================
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -7666,6 +7666,7 @@ gen_intermediate_code_internal (CPUState
     DisasContext ctx;
     target_ulong pc_start;
     uint16_t *gen_opc_end;
+    CPUBreakpoint *bp;
     int j, lj = -1;
 
     if (search_pc && loglevel)
@@ -7699,9 +7700,9 @@ gen_intermediate_code_internal (CPUState
                 tb, ctx.mem_idx, ctx.hflags);
 #endif
     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/target-ppc/translate.c
===================================================================
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -6167,6 +6167,7 @@ static always_inline int gen_intermediat
     target_ulong pc_start;
     uint16_t *gen_opc_end;
     int supervisor, little_endian;
+    CPUBreakpoint *bp;
     int j, lj = -1;
 
     pc_start = tb->pc;
@@ -6213,9 +6214,9 @@ static always_inline int gen_intermediat
 #endif
     /* 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/target-sh4/translate.c
===================================================================
--- a/target-sh4/translate.c
+++ b/target-sh4/translate.c
@@ -1188,6 +1188,7 @@ gen_intermediate_code_internal(CPUState 
     DisasContext ctx;
     target_ulong pc_start;
     static uint16_t *gen_opc_end;
+    CPUBreakpoint *bp;
     int i, ii;
 
     pc_start = tb->pc;
@@ -1214,17 +1215,17 @@ gen_intermediate_code_internal(CPUState 
 
     ii = -1;
     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]) {
-		    /* We have hit a breakpoint - make sure PC is up-to-date */
-		    gen_op_movl_imm_PC(ctx.pc);
-		    gen_op_debug();
-		    ctx.bstate = BS_EXCP;
-		    break;
-		}
-	    }
-	}
+        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 */
+                    gen_op_movl_imm_PC(ctx.pc);
+                    gen_op_debug();
+                    ctx.bstate = BS_EXCP;
+                    break;
+                }
+            }
+        }
         if (search_pc) {
             i = gen_opc_ptr - gen_opc_buf;
             if (ii < i) {
Index: b/target-sparc/translate.c
===================================================================
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -4718,6 +4718,7 @@ static inline int gen_intermediate_code_
     target_ulong pc_start, last_pc;
     uint16_t *gen_opc_end;
     DisasContext dc1, *dc = &dc1;
+    CPUBreakpoint *bp;
     int j, lj = -1;
 
     memset(dc, 0, sizeof(DisasContext));
@@ -4748,9 +4749,9 @@ static inline int gen_intermediate_code_
     cpu_addr = tcg_temp_local_new(TCG_TYPE_TL);
 
     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] 16+ messages in thread

* [Qemu-devel] [PATCH 7/15] Extend mem_write_* to mem_access_*
  2008-06-23 14:22 [Qemu-devel] [PATCH 0/15] Enhance debugging support Jan Kiszka
                   ` (5 preceding siblings ...)
  2008-06-23 14:26 ` [Qemu-devel] [PATCH 6/15] Refactor and enhance break/watchpoint API - v5 Jan Kiszka
@ 2008-06-23 14:27 ` Jan Kiszka
  2008-06-23 14:28 ` [Qemu-devel] [PATCH 8/15] Respect length of watchpoints Jan Kiszka
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2008-06-23 14:27 UTC (permalink / raw)
  To: qemu-devel

For full read-watchpoint support, it is required to keep track of the
accessed vaddr as well as the accessing pc also for read operations.
This patch extends the use of mem_write_pc/vaddr to mem_access_pc/vaddr
therefore.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpu-defs.h         |    8 ++++----
 exec.c             |   18 +++++++++---------
 softmmu_template.h |   15 ++++++++++-----
 3 files changed, 23 insertions(+), 18 deletions(-)

Index: b/cpu-defs.h
===================================================================
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -148,10 +148,10 @@ typedef struct CPUWatchpoint {
     /* in order to avoid passing too many arguments to the memory       \
        write helpers, we store some rarely used information in the CPU  \
        context) */                                                      \
-    unsigned long mem_write_pc; /* host pc at which the memory was      \
-                                   written */                           \
-    target_ulong mem_write_vaddr; /* target virtual addr at which the   \
-                                     memory was written */              \
+    unsigned long mem_access_pc; /* host pc at which the memory was     \
+                                    accessed */                         \
+    target_ulong mem_access_vaddr; /* target virtual addr at which the  \
+                                      memory was accessed */            \
     int halted; /* TRUE if the CPU is in suspend state */               \
     /* The meaning of the MMU modes is defined in the target code. */   \
     CPUTLBEntry tlb_table[NB_MMU_MODES][CPU_TLB_SIZE];                  \
Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -794,9 +794,9 @@ void tb_invalidate_phys_page_range(targe
             if (current_tb_not_found) {
                 current_tb_not_found = 0;
                 current_tb = NULL;
-                if (env->mem_write_pc) {
+                if (env->mem_access_pc) {
                     /* now we have a real cpu fault */
-                    current_tb = tb_find_pc(env->mem_write_pc);
+                    current_tb = tb_find_pc(env->mem_access_pc);
                 }
             }
             if (current_tb == tb &&
@@ -809,7 +809,7 @@ void tb_invalidate_phys_page_range(targe
 
                 current_tb_modified = 1;
                 cpu_restore_state(current_tb, env,
-                                  env->mem_write_pc, NULL);
+                                  env->mem_access_pc, NULL);
             }
 #endif /* TARGET_HAS_PRECISE_SMC */
             /* we need to do that to handle the case where a signal
@@ -833,7 +833,7 @@ void tb_invalidate_phys_page_range(targe
     if (!p->first_tb) {
         invalidate_page_bitmap(p);
         if (is_cpu_write_access) {
-            tlb_unprotect_code_phys(env, start, env->mem_write_vaddr);
+            tlb_unprotect_code_phys(env, start, env->mem_access_vaddr);
         }
     }
 #endif
@@ -858,7 +858,7 @@ static inline void tb_invalidate_phys_pa
     if (1) {
         if (loglevel) {
             fprintf(logfile, "modifying code at 0x%x size=%d EIP=%x PC=%08x\n",
-                   cpu_single_env->mem_write_vaddr, len,
+                   cpu_single_env->mem_access_vaddr, len,
                    cpu_single_env->eip,
                    cpu_single_env->eip + (long)cpu_single_env->segs[R_CS].base);
         }
@@ -2207,7 +2207,7 @@ static void notdirty_mem_writeb(void *op
     /* we remove the notdirty callback only if the code has been
        flushed */
     if (dirty_flags == 0xff)
-        tlb_set_dirty(cpu_single_env, cpu_single_env->mem_write_vaddr);
+        tlb_set_dirty(cpu_single_env, cpu_single_env->mem_access_vaddr);
 }
 
 static void notdirty_mem_writew(void *opaque, target_phys_addr_t ram_addr,
@@ -2232,7 +2232,7 @@ static void notdirty_mem_writew(void *op
     /* we remove the notdirty callback only if the code has been
        flushed */
     if (dirty_flags == 0xff)
-        tlb_set_dirty(cpu_single_env, cpu_single_env->mem_write_vaddr);
+        tlb_set_dirty(cpu_single_env, cpu_single_env->mem_access_vaddr);
 }
 
 static void notdirty_mem_writel(void *opaque, target_phys_addr_t ram_addr,
@@ -2257,7 +2257,7 @@ static void notdirty_mem_writel(void *op
     /* we remove the notdirty callback only if the code has been
        flushed */
     if (dirty_flags == 0xff)
-        tlb_set_dirty(cpu_single_env, cpu_single_env->mem_write_vaddr);
+        tlb_set_dirty(cpu_single_env, cpu_single_env->mem_access_vaddr);
 }
 
 static CPUReadMemoryFunc *error_mem_read[3] = {
@@ -2279,7 +2279,7 @@ static void check_watchpoint(int offset,
     target_ulong vaddr;
     CPUWatchpoint *wp;
 
-    vaddr = (env->mem_write_vaddr & TARGET_PAGE_MASK) + offset;
+    vaddr = (env->mem_access_vaddr & TARGET_PAGE_MASK) + offset;
     for (wp = env->watchpoints; wp != NULL; wp = wp->next) {
         if (vaddr == wp->vaddr && (wp->flags & flags)) {
             env->watchpoint_hit = wp;
Index: b/softmmu_template.h
===================================================================
--- a/softmmu_template.h
+++ b/softmmu_template.h
@@ -51,13 +51,16 @@ static DATA_TYPE glue(glue(slow_ld, SUFF
                                                         int mmu_idx,
                                                         void *retaddr);
 static inline DATA_TYPE glue(io_read, SUFFIX)(target_phys_addr_t physaddr,
-                                              target_ulong addr)
+                                              target_ulong addr,
+                                              void *retaddr)
 {
     DATA_TYPE res;
     int index;
     index = (physaddr >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
     physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
 
+    env->mem_access_vaddr = addr;
+    env->mem_access_pc = (unsigned long)retaddr;
 #if SHIFT <= 2
     res = io_mem_read[index][SHIFT](io_mem_opaque[index], physaddr);
 #else
@@ -96,7 +99,8 @@ DATA_TYPE REGPARM glue(glue(__ld, SUFFIX
             if ((addr & (DATA_SIZE - 1)) != 0)
                 goto do_unaligned_access;
             addend = env->iotlb[mmu_idx][index];
-            res = glue(io_read, SUFFIX)(addend, addr);
+            retaddr = GETPC();
+            res = glue(io_read, SUFFIX)(addend, addr, retaddr);
         } else if (((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1) >= TARGET_PAGE_SIZE) {
             /* slow unaligned access (it spans two pages or IO) */
         do_unaligned_access:
@@ -149,7 +153,8 @@ static DATA_TYPE glue(glue(slow_ld, SUFF
             if ((addr & (DATA_SIZE - 1)) != 0)
                 goto do_unaligned_access;
             addend = env->iotlb[mmu_idx][index];
-            res = glue(io_read, SUFFIX)(addend, addr);
+            retaddr = GETPC();
+            res = glue(io_read, SUFFIX)(addend, addr, retaddr);
         } else if (((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1) >= TARGET_PAGE_SIZE) {
         do_unaligned_access:
             /* slow unaligned access (it spans two pages) */
@@ -195,8 +200,8 @@ static inline void glue(io_write, SUFFIX
     index = (physaddr >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
     physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
 
-    env->mem_write_vaddr = addr;
-    env->mem_write_pc = (unsigned long)retaddr;
+    env->mem_access_vaddr = addr;
+    env->mem_access_pc = (unsigned long)retaddr;
 #if SHIFT <= 2
     io_mem_write[index][SHIFT](io_mem_opaque[index], physaddr, val);
 #else

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH 8/15] Respect length of watchpoints
  2008-06-23 14:22 [Qemu-devel] [PATCH 0/15] Enhance debugging support Jan Kiszka
                   ` (6 preceding siblings ...)
  2008-06-23 14:27 ` [Qemu-devel] [PATCH 7/15] Extend mem_write_* to mem_access_* Jan Kiszka
@ 2008-06-23 14:28 ` Jan Kiszka
  2008-06-23 14:29 ` [Qemu-devel] [PATCH 9/15] Restore pc on watchpoint hits Jan Kiszka
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2008-06-23 14:28 UTC (permalink / raw)
  To: qemu-devel

This adds length support for watchpoints. To keep things simple, only
aligned watchpoints are accepted.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpu-defs.h |    2 +-
 exec.c     |   28 ++++++++++++++++++----------
 2 files changed, 19 insertions(+), 11 deletions(-)

Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -1145,14 +1145,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;
@@ -1176,10 +1181,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;
         }
@@ -2273,7 +2280,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;
@@ -2281,7 +2288,8 @@ static void check_watchpoint(int offset,
 
     vaddr = (env->mem_access_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;
@@ -2294,40 +2302,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/cpu-defs.h
===================================================================
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -136,7 +136,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] 16+ messages in thread

* [Qemu-devel] [PATCH 9/15] Restore pc on watchpoint hits
  2008-06-23 14:22 [Qemu-devel] [PATCH 0/15] Enhance debugging support Jan Kiszka
                   ` (7 preceding siblings ...)
  2008-06-23 14:28 ` [Qemu-devel] [PATCH 8/15] Respect length of watchpoints Jan Kiszka
@ 2008-06-23 14:29 ` Jan Kiszka
  2008-06-23 14:30 ` [Qemu-devel] [PATCH 10/15] Remove premature memop TB terminations Jan Kiszka
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2008-06-23 14:29 UTC (permalink / raw)
  To: qemu-devel

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.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpu-all.h |    1 +
 exec.c    |   19 ++++++++++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -2283,15 +2283,32 @@ 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_access_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);
+            tb = tb_find_pc(env->mem_access_pc);
+            if (tb) {
+                cpu_restore_state(tb, env, env->mem_access_pc, NULL);
+                tb_phys_invalidate(tb, -1);
+            }
+            if (wp->flags & BP_STOP_BEFORE_ACCESS)
+                env->exception_index = EXCP_DEBUG;
+            else
+                env->singlestep_enabled |= SSTEP_INTERNAL;
+            cpu_resume_from_signal(env, NULL);
             break;
         }
     }
Index: b/cpu-all.h
===================================================================
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -801,6 +801,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] 16+ messages in thread

* [Qemu-devel] [PATCH 10/15] Remove premature memop TB terminations
  2008-06-23 14:22 [Qemu-devel] [PATCH 0/15] Enhance debugging support Jan Kiszka
                   ` (8 preceding siblings ...)
  2008-06-23 14:29 ` [Qemu-devel] [PATCH 9/15] Restore pc on watchpoint hits Jan Kiszka
@ 2008-06-23 14:30 ` Jan Kiszka
  2008-06-23 14:31 ` [Qemu-devel] [PATCH 11/15] Improve debugging of SMP guests - v2 Jan Kiszka
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2008-06-23 14:30 UTC (permalink / raw)
  To: qemu-devel

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>
---
 exec.c                  |    4 ----
 target-arm/translate.c  |    6 ------
 target-m68k/translate.c |    6 ------
 3 files changed, 16 deletions(-)

Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -1167,10 +1167,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/target-arm/translate.c
===================================================================
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -8650,12 +8650,6 @@ static inline int 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/target-m68k/translate.c
===================================================================
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -2965,12 +2965,6 @@ gen_intermediate_code_internal(CPUState 
         last_cc_op = dc->cc_op;
         dc->insn_pc = dc->pc;
 	disas_m68k_insn(env, dc);
-
-        /* 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;
     } while (!dc->is_jmp && gen_opc_ptr < gen_opc_end &&
              !env->singlestep_enabled &&
              (pc_offset) < (TARGET_PAGE_SIZE - 32));

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH 11/15] Improve debugging of SMP guests - v2
  2008-06-23 14:22 [Qemu-devel] [PATCH 0/15] Enhance debugging support Jan Kiszka
                   ` (9 preceding siblings ...)
  2008-06-23 14:30 ` [Qemu-devel] [PATCH 10/15] Remove premature memop TB terminations Jan Kiszka
@ 2008-06-23 14:31 ` Jan Kiszka
  2008-06-23 14:32 ` [Qemu-devel] [PATCH 12/15] Introduce BP_WATCHPOINT_HIT flag Jan Kiszka
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2008-06-23 14:31 UTC (permalink / raw)
  To: qemu-devel

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>
---
 gdbstub.c |   85 ++++++++++++++++++++++++++++++++++++++++----------------------
 monitor.c |   19 ++++++++-----
 monitor.h |   15 ++++++++++
 vl.c      |    2 +
 4 files changed, 85 insertions(+), 36 deletions(-)

Index: b/gdbstub.c
===================================================================
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -34,6 +34,7 @@
 #include "sysemu.h"
 #include "gdbstub.h"
 #endif
+#include "monitor.h"
 
 #include "qemu_socket.h"
 #ifdef _WIN32
@@ -58,7 +59,6 @@ enum RSState {
     RS_SYSCALL,
 };
 typedef struct GDBState {
-    CPUState *env; /* current CPU */
     enum RSState state; /* parsing state */
     char line_buf[4096];
     int line_buf_index;
@@ -977,43 +977,71 @@ const int xlat_gdb_type[] = {
 };
 #endif
 
-static int gdb_breakpoint_insert(CPUState *env, target_ulong addr,
-                                 target_ulong len, int type)
+static int gdb_breakpoint_insert(target_ulong addr, target_ulong len, int type)
 {
+    CPUState *env;
+    int err = 0;
+
     switch (type) {
     case GDB_BREAKPOINT_SW ... GDB_BREAKPOINT_HW:
-        return cpu_breakpoint_insert(env, addr, BP_GDB, NULL);
+        for (env = first_cpu; env != NULL; env = env->next_cpu) {
+            err = cpu_breakpoint_insert(env, addr, BP_GDB, NULL);
+            if (err)
+                break;
+        }
+        return err;
 #ifndef CONFIG_USER_ONLY
     case GDB_WATCHPOINT_WRITE ... GDB_WATCHPOINT_ACCESS:
-        return cpu_watchpoint_insert(env, addr, len, xlat_gdb_type[type],
-                                     NULL);
+        for (env = first_cpu; env != NULL; env = env->next_cpu) {
+            err = cpu_watchpoint_insert(env, addr, len, xlat_gdb_type[type],
+                                        NULL);
+            if (err)
+                break;
+        }
+        return err;
 #endif
     default:
         return -ENOSYS;
     }
 }
 
-static int gdb_breakpoint_remove(CPUState *env, target_ulong addr,
-                                 target_ulong len, int type)
+static int gdb_breakpoint_remove(target_ulong addr, target_ulong len, int type)
 {
+    CPUState *env;
+    int err = 0;
+
     switch (type) {
     case GDB_BREAKPOINT_SW ... GDB_BREAKPOINT_HW:
-        return cpu_breakpoint_remove(env, addr, BP_GDB);
+        for (env = first_cpu; env != NULL; env = env->next_cpu) {
+            err = cpu_breakpoint_remove(env, addr, BP_GDB);
+            if (err)
+                break;
+        }
+        return err;
 #ifndef CONFIG_USER_ONLY
     case GDB_WATCHPOINT_WRITE ... GDB_WATCHPOINT_ACCESS:
-        return cpu_watchpoint_remove(env, addr, len, xlat_gdb_type[type]);
+        for (env = first_cpu; env != NULL; env = env->next_cpu) {
+            err = cpu_watchpoint_remove(env, addr, len, xlat_gdb_type[type]);
+            if (err)
+                break;
+        }
+        return err;
 #endif
     default:
         return -ENOSYS;
     }
 }
 
-static void gdb_breakpoint_remove_all(CPUState *env)
+static void gdb_breakpoint_remove_all(void)
 {
-    cpu_breakpoint_remove_all(env, BP_GDB);
+    CPUState *env;
+
+    for (env = first_cpu; env != NULL; env = env->next_cpu) {
+        cpu_breakpoint_remove_all(env, BP_GDB);
 #ifndef CONFIG_USER_ONLY
-    cpu_watchpoint_remove_all(env, BP_GDB);
+        cpu_watchpoint_remove_all(env, BP_GDB);
 #endif
+    }
 }
 
 static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf)
@@ -1039,7 +1067,7 @@ static int gdb_handle_packet(GDBState *s
          * because gdb is doing and initial connect and the state
          * should be cleaned up.
          */
-        gdb_breakpoint_remove_all(env);
+        gdb_breakpoint_remove_all();
         break;
     case 'c':
         if (*p != '\0') {
@@ -1073,7 +1101,7 @@ static int gdb_handle_packet(GDBState *s
         exit(0);
     case 'D':
         /* Detach packet */
-        gdb_breakpoint_remove_all(env);
+        gdb_breakpoint_remove_all();
         gdb_continue(s);
         put_packet(s, "OK");
         break;
@@ -1116,7 +1144,7 @@ static int gdb_handle_packet(GDBState *s
                 p++;
             type = *p;
             if (gdb_current_syscall_cb)
-                gdb_current_syscall_cb(s->env, ret, err);
+                gdb_current_syscall_cb(mon_get_cpu(), ret, err);
             if (type == 'C') {
                 put_packet(s, "T02");
             } else {
@@ -1171,9 +1199,9 @@ static int gdb_handle_packet(GDBState *s
             p++;
         len = strtoull(p, (char **)&p, 16);
         if (ch == 'Z')
-            res = gdb_breakpoint_insert(env, addr, len, type);
+            res = gdb_breakpoint_insert(addr, len, type);
         else
-            res = gdb_breakpoint_remove(env, addr, len, type);
+            res = gdb_breakpoint_remove(addr, len, type);
         if (res >= 0)
              put_packet(s, "OK");
         else if (res == -ENOSYS)
@@ -1237,6 +1265,7 @@ extern void tb_flush(CPUState *env);
 static void gdb_vm_stopped(void *opaque, int reason)
 {
     GDBState *s = opaque;
+    CPUState *env = mon_get_cpu();
     char buf[256];
     const char *type;
     int ret;
@@ -1245,11 +1274,11 @@ static void gdb_vm_stopped(void *opaque,
         return;
 
     /* disable single step if it was enable */
-    cpu_single_step(s->env, 0);
+    cpu_single_step(env, 0);
 
     if (reason == EXCP_DEBUG) {
-        if (s->env->watchpoint_hit) {
-            switch (s->env->watchpoint_hit->flags & BP_MEM_ACCESS) {
+        if (env->watchpoint_hit) {
+            switch (env->watchpoint_hit->flags & BP_MEM_ACCESS) {
             case BP_MEM_READ:
                 type = "r";
                 break;
@@ -1261,12 +1290,12 @@ static void gdb_vm_stopped(void *opaque,
                 break;
             }
             snprintf(buf, sizeof(buf), "T%02x%swatch:" TARGET_FMT_lx ";",
-                     SIGTRAP, type, s->env->watchpoint_hit->vaddr);
+                     SIGTRAP, type, env->watchpoint_hit->vaddr);
             put_packet(s, buf);
-            s->env->watchpoint_hit = NULL;
+            env->watchpoint_hit = NULL;
             return;
         }
-	tb_flush(s->env);
+	tb_flush(env);
         ret = SIGTRAP;
     } else if (reason == EXCP_INTERRUPT) {
         ret = SIGINT;
@@ -1336,15 +1365,15 @@ void gdb_do_syscall(gdb_syscall_complete
     va_end(va);
     put_packet(s, buf);
 #ifdef CONFIG_USER_ONLY
-    gdb_handlesig(s->env, 0);
+    gdb_handlesig(mon_get_cpu(), 0);
 #else
-    cpu_interrupt(s->env, CPU_INTERRUPT_EXIT);
+    cpu_interrupt(mon_get_cpu(), CPU_INTERRUPT_EXIT);
 #endif
 }
 
 static void gdb_read_byte(GDBState *s, int ch)
 {
-    CPUState *env = s->env;
+    CPUState *env = mon_get_cpu();
     int i, csum;
     uint8_t reply;
 
@@ -1508,7 +1537,6 @@ static void gdb_accept(void *opaque)
 
     s = &gdbserver_state;
     memset (s, 0, sizeof (GDBState));
-    s->env = first_cpu; /* XXX: allow to change CPU */
     s->fd = fd;
 
     gdb_syscall_state = s;
@@ -1611,7 +1639,6 @@ int gdbserver_start(const char *port)
     if (!s) {
         return -1;
     }
-    s->env = first_cpu; /* XXX: allow to change CPU */
     s->chr = chr;
     qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive,
                           gdb_chr_event, s);
Index: b/monitor.c
===================================================================
--- a/monitor.c
+++ b/monitor.c
@@ -261,8 +261,7 @@ static void do_info_blockstats(void)
     bdrv_info_stats();
 }
 
-/* get the current CPU defined by the user */
-static int mon_set_cpu(int cpu_index)
+static int mon_set_cpu_index(int cpu_index)
 {
     CPUState *env;
 
@@ -275,10 +274,16 @@ static int mon_set_cpu(int cpu_index)
     return -1;
 }
 
-static CPUState *mon_get_cpu(void)
+void mon_set_cpu(CPUState *env)
+{
+    mon_cpu = env;
+}
+
+/* get the current CPU defined by the user or by a breakpoint hit */
+CPUState *mon_get_cpu(void)
 {
     if (!mon_cpu) {
-        mon_set_cpu(0);
+        mon_set_cpu(first_cpu);
     }
     return mon_cpu;
 }
@@ -302,8 +307,8 @@ static void do_info_cpus(void)
 {
     CPUState *env;
 
-    /* just to set the default cpu if not already done */
-    mon_get_cpu();
+    if (!mon_cpu)
+        mon_set_cpu(first_cpu);
 
     for(env = first_cpu; env != NULL; env = env->next_cpu) {
         term_printf("%c CPU #%d:",
@@ -326,7 +331,7 @@ static void do_info_cpus(void)
 
 static void do_cpu_set(int index)
 {
-    if (mon_set_cpu(index) < 0)
+    if (mon_set_cpu_index(index) < 0)
         term_printf("Invalid CPU index\n");
 }
 
Index: b/monitor.h
===================================================================
--- /dev/null
+++ b/monitor.h
@@ -0,0 +1,15 @@
+#ifndef QEMU_MONITOR_H
+#define QEMU_MONITOR_H
+
+void mon_set_cpu(CPUState *env);
+
+#ifdef CONFIG_USER_ONLY
+static inline CPUState *mon_get_cpu(void)
+{
+    return first_cpu;
+}
+#else
+CPUState *mon_get_cpu(void);
+#endif
+
+#endif /* QEMU_MONITOR_H */
Index: b/vl.c
===================================================================
--- a/vl.c
+++ b/vl.c
@@ -33,6 +33,7 @@
 #include "console.h"
 #include "sysemu.h"
 #include "gdbstub.h"
+#include "monitor.h"
 #include "qemu-timer.h"
 #include "qemu-char.h"
 #include "block.h"
@@ -7112,6 +7113,7 @@ static int main_loop(void)
                 ret = EXCP_INTERRUPT;
             }
             if (unlikely(ret == EXCP_DEBUG)) {
+                mon_set_cpu(cur_cpu);
                 vm_stop(EXCP_DEBUG);
             }
             /* If all cpus are halted then wait until the next IRQ */

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH 12/15] Introduce BP_WATCHPOINT_HIT flag
  2008-06-23 14:22 [Qemu-devel] [PATCH 0/15] Enhance debugging support Jan Kiszka
                   ` (10 preceding siblings ...)
  2008-06-23 14:31 ` [Qemu-devel] [PATCH 11/15] Improve debugging of SMP guests - v2 Jan Kiszka
@ 2008-06-23 14:32 ` Jan Kiszka
  2008-06-23 14:32 ` [Qemu-devel] [PATCH 13/15] Add debug exception hook Jan Kiszka
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2008-06-23 14:32 UTC (permalink / raw)
  To: qemu-devel

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>
---
 cpu-all.h  |    1 +
 cpu-exec.c |   11 +++++++++++
 exec.c     |   29 +++++++++++++++++------------
 3 files changed, 29 insertions(+), 12 deletions(-)

Index: b/cpu-all.h
===================================================================
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -802,6 +802,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/cpu-exec.c
===================================================================
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -228,6 +228,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)
@@ -282,6 +291,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/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -1182,7 +1182,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;
         }
@@ -2294,20 +2294,25 @@ 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_access_pc);
-            if (tb) {
-                cpu_restore_state(tb, env, env->mem_access_pc, NULL);
-                tb_phys_invalidate(tb, -1);
+            wp->flags |= BP_WATCHPOINT_HIT;
+            if (!env->watchpoint_hit) {
+                env->watchpoint_hit = wp;
+                tb = tb_find_pc(env->mem_access_pc);
+                if (tb) {
+                    cpu_restore_state(tb, env, env->mem_access_pc, NULL);
+                    tb_phys_invalidate(tb, -1);
+                }
+                if (wp->flags & BP_STOP_BEFORE_ACCESS)
+                    env->exception_index = EXCP_DEBUG;
+                else
+                    env->singlestep_enabled |= SSTEP_INTERNAL;
             }
-            if (wp->flags & BP_STOP_BEFORE_ACCESS)
-                env->exception_index = EXCP_DEBUG;
-            else
-                env->singlestep_enabled |= SSTEP_INTERNAL;
-            cpu_resume_from_signal(env, NULL);
-            break;
+        } else {
+            wp->flags &= ~BP_WATCHPOINT_HIT;
         }
     }
+    if (env->watchpoint_hit)
+        cpu_resume_from_signal(env, NULL);
 }
 
 /* Watchpoint access routines.  Watchpoints are inserted using TLB tricks,

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH 13/15] Add debug exception hook
  2008-06-23 14:22 [Qemu-devel] [PATCH 0/15] Enhance debugging support Jan Kiszka
                   ` (11 preceding siblings ...)
  2008-06-23 14:32 ` [Qemu-devel] [PATCH 12/15] Introduce BP_WATCHPOINT_HIT flag Jan Kiszka
@ 2008-06-23 14:32 ` Jan Kiszka
  2008-06-23 14:33 ` [Qemu-devel] [PATCH 14/15] Introduce BP_CPU as a breakpoint type Jan Kiszka
  2008-06-23 14:33 ` [Qemu-devel] [PATCH 15/15] x86: Debug register emulation Jan Kiszka
  14 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2008-06-23 14:32 UTC (permalink / raw)
  To: qemu-devel

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>
---
 cpu-exec.c |   13 +++++++++++++
 exec-all.h |    4 ++++
 2 files changed, 17 insertions(+)

Index: b/cpu-exec.c
===================================================================
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -228,6 +228,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;
@@ -235,6 +245,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/exec-all.h
===================================================================
--- a/exec-all.h
+++ b/exec-all.h
@@ -393,3 +393,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] 16+ messages in thread

* [Qemu-devel] [PATCH 14/15] Introduce BP_CPU as a breakpoint type
  2008-06-23 14:22 [Qemu-devel] [PATCH 0/15] Enhance debugging support Jan Kiszka
                   ` (12 preceding siblings ...)
  2008-06-23 14:32 ` [Qemu-devel] [PATCH 13/15] Add debug exception hook Jan Kiszka
@ 2008-06-23 14:33 ` Jan Kiszka
  2008-06-23 14:33 ` [Qemu-devel] [PATCH 15/15] x86: Debug register emulation Jan Kiszka
  14 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2008-06-23 14:33 UTC (permalink / raw)
  To: qemu-devel

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>
---
 cpu-all.h |    1 +
 exec.c    |   46 ++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 39 insertions(+), 8 deletions(-)

Index: b/cpu-all.h
===================================================================
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -804,6 +804,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/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -1146,7 +1146,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))
@@ -1160,11 +1160,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);
 
@@ -1220,7 +1235,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)
@@ -1229,11 +1244,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] 16+ messages in thread

* [Qemu-devel] [PATCH 15/15] x86: Debug register emulation
  2008-06-23 14:22 [Qemu-devel] [PATCH 0/15] Enhance debugging support Jan Kiszka
                   ` (13 preceding siblings ...)
  2008-06-23 14:33 ` [Qemu-devel] [PATCH 14/15] Introduce BP_CPU as a breakpoint type Jan Kiszka
@ 2008-06-23 14:33 ` Jan Kiszka
  14 siblings, 0 replies; 16+ messages in thread
From: Jan Kiszka @ 2008-06-23 14:33 UTC (permalink / raw)
  To: qemu-devel

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>
---
 linux-user/main.c       |    4 -
 target-i386/cpu.h       |   33 +++++++++++++
 target-i386/helper.c    |  118 ++++++++++++++++++++++++++++++++++++------------
 target-i386/op_helper.c |   79 ++++++++++++++++++++++++++++++--
 4 files changed, 198 insertions(+), 36 deletions(-)

Index: b/linux-user/main.c
===================================================================
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -403,7 +403,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) {
@@ -413,7 +413,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/target-i386/cpu.h
===================================================================
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -199,6 +199,16 @@
 #define CR4_OSFXSR_MASK (1 << 9)
 #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
@@ -334,7 +344,7 @@
 #define CPUID_EXT3_SKINIT  (1 << 12)
 
 #define EXCP00_DIVZ	0
-#define EXCP01_SSTP	1
+#define EXCP01_DB	1
 #define EXCP02_NMI	2
 #define EXCP03_INT3	3
 #define EXCP04_INTO	4
@@ -566,6 +576,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 interrupt_request;
     int user_mode_only; /* user mode only simulation */
@@ -753,6 +767,23 @@ static inline void cpu_clone_regs(CPUSta
 }
 #endif
 
+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/target-i386/helper.c
===================================================================
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -32,8 +32,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, 
@@ -91,33 +89,6 @@ static void add_flagname_to_bitmaps(char
     fprintf(stderr, "CPU feature %s not found\n", flagname);
 }
 
-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
-    return env;
-}
-
 typedef struct x86_def_t {
     const char *name;
     uint32_t level;
@@ -429,6 +400,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)
@@ -1223,4 +1200,87 @@ 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
+    return env;
+}
Index: b/target-i386/op_helper.c
===================================================================
--- a/target-i386/op_helper.c
+++ b/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 */
@@ -1875,8 +1931,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)
@@ -2991,10 +3050,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] 16+ messages in thread

end of thread, other threads:[~2008-06-23 14:39 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-23 14:22 [Qemu-devel] [PATCH 0/15] Enhance debugging support Jan Kiszka
2008-06-23 14:23 ` [Qemu-devel] [PATCH 1/15] Convert remaining __builtin_expect to likely/unlikely Jan Kiszka
2008-06-23 14:24 ` [Qemu-devel] [PATCH 2/15] Introduce SSTEP_INTERNAL Jan Kiszka
2008-06-23 14:24 ` [Qemu-devel] [PATCH 3/15] Replace CF_SINGLE_INSN with SSTEP_INTERNAL - v2 Jan Kiszka
2008-06-23 14:25 ` [Qemu-devel] [PATCH 4/15] Remove unused TB cflags Jan Kiszka
2008-06-23 14:26 ` [Qemu-devel] [PATCH 5/15] Return appropriate watch message to gdb Jan Kiszka
2008-06-23 14:26 ` [Qemu-devel] [PATCH 6/15] Refactor and enhance break/watchpoint API - v5 Jan Kiszka
2008-06-23 14:27 ` [Qemu-devel] [PATCH 7/15] Extend mem_write_* to mem_access_* Jan Kiszka
2008-06-23 14:28 ` [Qemu-devel] [PATCH 8/15] Respect length of watchpoints Jan Kiszka
2008-06-23 14:29 ` [Qemu-devel] [PATCH 9/15] Restore pc on watchpoint hits Jan Kiszka
2008-06-23 14:30 ` [Qemu-devel] [PATCH 10/15] Remove premature memop TB terminations Jan Kiszka
2008-06-23 14:31 ` [Qemu-devel] [PATCH 11/15] Improve debugging of SMP guests - v2 Jan Kiszka
2008-06-23 14:32 ` [Qemu-devel] [PATCH 12/15] Introduce BP_WATCHPOINT_HIT flag Jan Kiszka
2008-06-23 14:32 ` [Qemu-devel] [PATCH 13/15] Add debug exception hook Jan Kiszka
2008-06-23 14:33 ` [Qemu-devel] [PATCH 14/15] Introduce BP_CPU as a breakpoint type Jan Kiszka
2008-06-23 14:33 ` [Qemu-devel] [PATCH 15/15] x86: Debug register emulation Jan Kiszka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).