Re: [PATCH 14/17] qemu: x86: Debug register emulation

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 11738 bytes --]

Avi Kivity wrote:
> Jan Kiszka wrote:
>> Built on top of previously enhanced breakpoint/watchpoint support, this
>> patch adds full debug register emulation for the x86 architecture.
>>
>> Many corner cases were considered, and the result was successfully
>> tested inside a Linux guest with gdb, but I won't be surprised if one
>> or two scenarios still behave differently in reality.
>>
>> +
>> +static inline int hw_breakpoint_len(unsigned long dr7, int index)
>> +{
>> +    return ((dr7 >> (DR7_LEN_SHIFT + (index * 2))) & 3) + 1;
>> +}
>> +
>>   
> 
> A len encoding of 2 means an 8-byte breakpoint.
> 

True, fixed version follows:

------------

Built on top of previously enhanced breakpoint/watchpoint support, this
patch adds full debug register emulation for the x86 architecture.

Many corner cases were considered, and the result was successfully
tested inside a Linux guest with gdb, but I won't be surprised if one
or two scenarios still behave differently in reality.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 qemu/linux-user/main.c       |    4 -
 qemu/target-i386/cpu.h       |   34 +++++++++++
 qemu/target-i386/helper.c    |  126 +++++++++++++++++++++++++++++++------------
 qemu/target-i386/op_helper.c |   79 +++++++++++++++++++++++++-
 4 files changed, 203 insertions(+), 40 deletions(-)

Index: b/qemu/linux-user/main.c
===================================================================
--- a/qemu/linux-user/main.c
+++ b/qemu/linux-user/main.c
@@ -406,7 +406,7 @@ void cpu_loop(CPUX86State *env)
                 queue_signal(env, info.si_signo, &info);
             }
             break;
-        case EXCP01_SSTP:
+        case EXCP01_DB:
         case EXCP03_INT3:
 #ifndef TARGET_X86_64
             if (env->eflags & VM_MASK) {
@@ -416,7 +416,7 @@ void cpu_loop(CPUX86State *env)
             {
                 info.si_signo = SIGTRAP;
                 info.si_errno = 0;
-                if (trapnr == EXCP01_SSTP) {
+                if (trapnr == EXCP01_DB) {
                     info.si_code = TARGET_TRAP_BRKPT;
                     info._sifields._sigfault._addr = env->eip;
                 } else {
Index: b/qemu/target-i386/cpu.h
===================================================================
--- a/qemu/target-i386/cpu.h
+++ b/qemu/target-i386/cpu.h
@@ -205,6 +205,16 @@
 #define CR4_OSFXSR_MASK (1 << CR4_OSFXSR_SHIFT)
 #define CR4_OSXMMEXCPT_MASK  (1 << 10)
 
+#define DR6_BD          (1 << 13)
+#define DR6_BS          (1 << 14)
+#define DR6_BT          (1 << 15)
+#define DR6_FIXED_1     0xffff0ff0
+
+#define DR7_GD          (1 << 13)
+#define DR7_TYPE_SHIFT  16
+#define DR7_LEN_SHIFT   18
+#define DR7_FIXED_1     0x00000400
+
 #define PG_PRESENT_BIT	0
 #define PG_RW_BIT	1
 #define PG_USER_BIT	2
@@ -361,7 +371,7 @@
 #define CPUID_MWAIT_EMX     (1 << 0) /* enumeration supported */
 
 #define EXCP00_DIVZ	0
-#define EXCP01_SSTP	1
+#define EXCP01_DB	1
 #define EXCP02_NMI	2
 #define EXCP03_INT3	3
 #define EXCP04_INTO	4
@@ -594,6 +604,10 @@ typedef struct CPUX86State {
     int exception_is_int;
     target_ulong exception_next_eip;
     target_ulong dr[8]; /* debug registers */
+    union {
+        CPUBreakpoint *cpu_breakpoint[4];
+        CPUWatchpoint *cpu_watchpoint[4];
+    }; /* break/watchpoints for dr[0..3] */
     uint32_t smbase;
     int old_exception;  /* exception in flight */
 
@@ -789,6 +803,24 @@ static inline void cpu_clone_regs(CPUSta
 
 #define CPU_PC_FROM_TB(env, tb) env->eip = tb->pc - tb->cs_base
 
+static inline int hw_breakpoint_enabled(unsigned long dr7, int index)
+{
+    return (dr7 >> (index * 2)) & 3;
+}
+
+static inline int hw_breakpoint_type(unsigned long dr7, int index)
+{
+    return (dr7 >> (DR7_TYPE_SHIFT + (index * 2))) & 3;
+}
+
+static inline int hw_breakpoint_len(unsigned long dr7, int index)
+{
+    int len = ((dr7 >> (DR7_LEN_SHIFT + (index * 2))) & 3);
+    return (len == 2) ? 8 : len + 1;
+}
+
+int check_hw_breakpoints(CPUState *env, int force_dr6_update);
+
 #include "cpu-all.h"
 
 #include "svm.h"
Index: b/qemu/target-i386/helper.c
===================================================================
--- a/qemu/target-i386/helper.c
+++ b/qemu/target-i386/helper.c
@@ -34,8 +34,6 @@
 
 //#define DEBUG_MMU
 
-static int cpu_x86_register (CPUX86State *env, const char *cpu_model);
-
 static void add_flagname_to_bitmaps(char *flagname, uint32_t *features, 
                                     uint32_t *ext_features, 
                                     uint32_t *ext2_features, 
@@ -95,37 +93,6 @@ static void add_flagname_to_bitmaps(char
 
 extern const char *cpu_vendor_string;
 
-CPUX86State *cpu_x86_init(const char *cpu_model)
-{
-    CPUX86State *env;
-    static int inited;
-
-    env = qemu_mallocz(sizeof(CPUX86State));
-    if (!env)
-        return NULL;
-    cpu_exec_init(env);
-    env->cpu_model_str = cpu_model;
-
-    /* init various static tables */
-    if (!inited) {
-        inited = 1;
-        optimize_flags_init();
-    }
-    if (cpu_x86_register(env, cpu_model) < 0) {
-        cpu_x86_close(env);
-        return NULL;
-    }
-    cpu_reset(env);
-#ifdef USE_KQEMU
-    kqemu_init(env);
-#endif
-#ifdef USE_KVM
-    if (kvm_enabled())
-       kvm_init_new_ap(env->cpu_index, env);
-#endif
-    return env;
-}
-
 typedef struct x86_def_t {
     const char *name;
     uint32_t level;
@@ -482,6 +449,12 @@ void cpu_reset(CPUX86State *env)
     env->fpuc = 0x37f;
 
     env->mxcsr = 0x1f80;
+
+    memset(env->dr, 0, sizeof(env->dr));
+    env->dr[6] = DR6_FIXED_1;
+    env->dr[7] = DR7_FIXED_1;
+    cpu_breakpoint_remove_all(env, BP_CPU);
+    cpu_watchpoint_remove_all(env, BP_CPU);
 }
 
 void cpu_x86_close(CPUX86State *env)
@@ -1278,4 +1251,91 @@ target_phys_addr_t cpu_get_phys_page_deb
     paddr = (pte & TARGET_PAGE_MASK) + page_offset;
     return paddr;
 }
+
+int check_hw_breakpoints(CPUState *env, int force_dr6_update)
+{
+    target_ulong dr6;
+    int reg, type;
+    int hit_enabled = 0;
+
+    dr6 = env->dr[6] & ~0xf;
+    for (reg = 0; reg < 4; reg++) {
+        type = hw_breakpoint_type(env->dr[7], reg);
+        if ((type == 0 && env->dr[reg] == env->eip) ||
+            ((type & 1) && env->cpu_watchpoint[reg] &&
+             (env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT))) {
+            dr6 |= 1 << reg;
+            if (hw_breakpoint_enabled(env->dr[7], reg))
+                hit_enabled = 1;
+        }
+    }
+    if (hit_enabled || force_dr6_update)
+        env->dr[6] = dr6;
+    return hit_enabled;
+}
+
+static CPUDebugExcpHandler *prev_debug_excp_handler;
+
+void raise_exception(int exception_index);
+
+static void breakpoint_handler(CPUState *env)
+{
+    CPUBreakpoint *bp;
+
+    if (env->watchpoint_hit) {
+        if (env->watchpoint_hit->flags & BP_CPU) {
+            env->watchpoint_hit = NULL;
+            if (check_hw_breakpoints(env, 0))
+                raise_exception(EXCP01_DB);
+            else
+                cpu_resume_from_signal(env, NULL);
+        }
+    } else {
+        for (bp = env->breakpoints; bp != NULL; bp = bp->next)
+            if (bp->pc == env->eip) {
+                if (bp->flags & BP_CPU) {
+                    check_hw_breakpoints(env, 1);
+                    raise_exception(EXCP01_DB);
+                }
+                break;
+            }
+    }
+    if (prev_debug_excp_handler)
+        prev_debug_excp_handler(env);
+}
 #endif /* !CONFIG_USER_ONLY */
+
+CPUX86State *cpu_x86_init(const char *cpu_model)
+{
+    CPUX86State *env;
+    static int inited;
+
+    env = qemu_mallocz(sizeof(CPUX86State));
+    if (!env)
+        return NULL;
+    cpu_exec_init(env);
+    env->cpu_model_str = cpu_model;
+
+    /* init various static stuff */
+    if (!inited) {
+        inited = 1;
+        optimize_flags_init();
+#ifndef CONFIG_USER_ONLY
+        prev_debug_excp_handler =
+            cpu_set_debug_excp_handler(breakpoint_handler);
+#endif
+    }
+    if (cpu_x86_register(env, cpu_model) < 0) {
+        cpu_x86_close(env);
+        return NULL;
+    }
+    cpu_reset(env);
+#ifdef USE_KQEMU
+    kqemu_init(env);
+#endif
+#ifdef USE_KVM
+    if (kvm_enabled())
+       kvm_init_new_ap(env->cpu_index, env);
+#endif
+    return env;
+}
Index: b/qemu/target-i386/op_helper.c
===================================================================
--- a/qemu/target-i386/op_helper.c
+++ b/qemu/target-i386/op_helper.c
@@ -94,6 +94,53 @@ const CPU86_LDouble f15rk[7] =
     3.32192809488736234781L,  /*l2t*/
 };
 
+static void hw_breakpoint_insert(int index)
+{
+    int type, err = 0;
+
+    switch (hw_breakpoint_type(env->dr[7], index)) {
+    case 0:
+        if (hw_breakpoint_enabled(env->dr[7], index))
+            err = cpu_breakpoint_insert(env, env->dr[index], BP_CPU,
+                                        &env->cpu_breakpoint[index]);
+        break;
+    case 1:
+        type = BP_CPU | BP_MEM_WRITE;
+        goto insert_wp;
+    case 2:
+         /* No support for I/O watchpoints yet */
+        break;
+    case 3:
+        type = BP_CPU | BP_MEM_ACCESS;
+    insert_wp:
+        err = cpu_watchpoint_insert(env, env->dr[index],
+                                    hw_breakpoint_len(env->dr[7], index),
+                                    type, &env->cpu_watchpoint[index]);
+        break;
+    }
+    if (err)
+        env->cpu_breakpoint[index] = NULL;
+}
+
+static void hw_breakpoint_remove(int index)
+{
+    if (!env->cpu_breakpoint[index])
+        return;
+    switch (hw_breakpoint_type(env->dr[7], index)) {
+    case 0:
+        if (hw_breakpoint_enabled(env->dr[7], index))
+            cpu_breakpoint_remove_by_ref(env, env->cpu_breakpoint[index]);
+        break;
+    case 1:
+    case 3:
+        cpu_watchpoint_remove_by_ref(env, env->cpu_watchpoint[index]);
+        break;
+    case 2:
+        /* No support for I/O watchpoints yet */
+        break;
+    }
+}
+
 /* broken thread support */
 
 spinlock_t global_cpu_lock = SPIN_LOCK_UNLOCKED;
@@ -496,6 +543,15 @@ static void switch_tss(int tss_selector,
         /* XXX: different exception if CALL ? */
         raise_exception_err(EXCP0D_GPF, 0);
     }
+
+    /* reset local breakpoints */
+    if (env->dr[7] & 0x55) {
+        for (i = 0; i < 4; i++) {
+            if (hw_breakpoint_enabled(env->dr[7], i) == 0x1)
+                hw_breakpoint_remove(i);
+        }
+        env->dr[7] &= ~0x55;
+    }
 }
 
 /* check if Port I/O is allowed in TSS */
@@ -1879,8 +1935,11 @@ void helper_cmpxchg16b(target_ulong a0)
 
 void helper_single_step(void)
 {
-    env->dr[6] |= 0x4000;
-    raise_exception(EXCP01_SSTP);
+#ifndef CONFIG_USER_ONLY
+    check_hw_breakpoints(env, 1);
+#endif
+    env->dr[6] |= DR6_BS;
+    raise_exception(EXCP01_DB);
 }
 
 void helper_cpuid(void)
@@ -3082,10 +3141,22 @@ void helper_clts(void)
     env->hflags &= ~HF_TS_MASK;
 }
 
-/* XXX: do more */
 void helper_movl_drN_T0(int reg, target_ulong t0)
 {
-    env->dr[reg] = t0;
+    int i;
+
+    if (reg < 4) {
+        hw_breakpoint_remove(reg);
+        env->dr[reg] = t0;
+        hw_breakpoint_insert(reg);
+    } else if (reg == 7) {
+        for (i = 0; i < 4; i++)
+            hw_breakpoint_remove(i);
+        env->dr[7] = t0;
+        for (i = 0; i < 4; i++)
+            hw_breakpoint_insert(i);
+    } else
+        env->dr[reg] = t0;
 }
 
 void helper_invlpg(target_ulong addr)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]