* [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions
@ 2015-09-15 18:45 Richard Henderson
2015-09-15 18:45 ` [Qemu-devel] [PATCH 1/8] target-i386: Move breakpoint related functions to new file Richard Henderson
` (8 more replies)
0 siblings, 9 replies; 20+ messages in thread
From: Richard Henderson @ 2015-09-15 18:45 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, ehabkost
Best guess, since I can't find any code that actually uses them.
Linux actively turns them off at boot...
r~
Richard Henderson (8):
target-i386: Move breakpoint related functions to new file
target-i386: Make check_hw_breakpoints static
target-i386: Introduce cpu_x86_update_dr7
target-i386: Re-introduce optimal breakpoint removal
target-i386: Move hw_*breakpoint_* functions
target-i386: Optimize setting dr[0-3]
target-i386: Handle I/O breakpoints
target-i386: Check CR4[DE] for processing DR4/DR5
target-i386/Makefile.objs | 2 +-
target-i386/bpt_helper.c | 324 ++++++++++++++++++++++++++++++++++++++++++++++
target-i386/cpu.h | 36 +-----
target-i386/helper.c | 128 ------------------
target-i386/helper.h | 4 +-
target-i386/machine.c | 8 +-
target-i386/misc_helper.c | 34 -----
target-i386/seg_helper.c | 8 +-
target-i386/translate.c | 30 ++++-
9 files changed, 365 insertions(+), 209 deletions(-)
create mode 100644 target-i386/bpt_helper.c
--
2.1.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 1/8] target-i386: Move breakpoint related functions to new file
2015-09-15 18:45 [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions Richard Henderson
@ 2015-09-15 18:45 ` Richard Henderson
2015-09-18 18:27 ` Eduardo Habkost
2015-09-15 18:45 ` [Qemu-devel] [PATCH 2/8] target-i386: Make check_hw_breakpoints static Richard Henderson
` (7 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2015-09-15 18:45 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, ehabkost
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target-i386/Makefile.objs | 2 +-
target-i386/bpt_helper.c | 182 ++++++++++++++++++++++++++++++++++++++++++++++
target-i386/helper.c | 128 --------------------------------
target-i386/misc_helper.c | 34 ---------
4 files changed, 183 insertions(+), 163 deletions(-)
create mode 100644 target-i386/bpt_helper.c
diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs
index 7a1df2c..df51db5 100644
--- a/target-i386/Makefile.objs
+++ b/target-i386/Makefile.objs
@@ -1,4 +1,4 @@
-obj-y += translate.o helper.o cpu.o
+obj-y += translate.o helper.o cpu.o bpt_helper.o
obj-y += excp_helper.o fpu_helper.o cc_helper.o int_helper.o svm_helper.o
obj-y += smm_helper.o misc_helper.o mem_helper.o seg_helper.o
obj-y += gdbstub.o
diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
new file mode 100644
index 0000000..6f6537d
--- /dev/null
+++ b/target-i386/bpt_helper.c
@@ -0,0 +1,182 @@
+/*
+ * i386 breakpoint helpers
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "cpu.h"
+#include "exec/helper-proto.h"
+
+
+void hw_breakpoint_insert(CPUX86State *env, int index)
+{
+ CPUState *cs = CPU(x86_env_get_cpu(env));
+ int type = 0, err = 0;
+
+ switch (hw_breakpoint_type(env->dr[7], index)) {
+ case DR7_TYPE_BP_INST:
+ if (hw_breakpoint_enabled(env->dr[7], index)) {
+ err = cpu_breakpoint_insert(cs, env->dr[index], BP_CPU,
+ &env->cpu_breakpoint[index]);
+ }
+ break;
+ case DR7_TYPE_DATA_WR:
+ type = BP_CPU | BP_MEM_WRITE;
+ break;
+ case DR7_TYPE_IO_RW:
+ /* No support for I/O watchpoints yet */
+ break;
+ case DR7_TYPE_DATA_RW:
+ type = BP_CPU | BP_MEM_ACCESS;
+ break;
+ }
+
+ if (type != 0) {
+ err = cpu_watchpoint_insert(cs, env->dr[index],
+ hw_breakpoint_len(env->dr[7], index),
+ type, &env->cpu_watchpoint[index]);
+ }
+
+ if (err) {
+ env->cpu_breakpoint[index] = NULL;
+ }
+}
+
+void hw_breakpoint_remove(CPUX86State *env, int index)
+{
+ CPUState *cs;
+
+ if (!env->cpu_breakpoint[index]) {
+ return;
+ }
+ cs = CPU(x86_env_get_cpu(env));
+ switch (hw_breakpoint_type(env->dr[7], index)) {
+ case DR7_TYPE_BP_INST:
+ if (hw_breakpoint_enabled(env->dr[7], index)) {
+ cpu_breakpoint_remove_by_ref(cs, env->cpu_breakpoint[index]);
+ }
+ break;
+ case DR7_TYPE_DATA_WR:
+ case DR7_TYPE_DATA_RW:
+ cpu_watchpoint_remove_by_ref(cs, env->cpu_watchpoint[index]);
+ break;
+ case DR7_TYPE_IO_RW:
+ /* No support for I/O watchpoints yet */
+ break;
+ }
+}
+
+bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
+{
+ target_ulong dr6;
+ int reg;
+ bool hit_enabled = false;
+
+ dr6 = env->dr[6] & ~0xf;
+ for (reg = 0; reg < DR7_MAX_BP; reg++) {
+ bool bp_match = false;
+ bool wp_match = false;
+
+ switch (hw_breakpoint_type(env->dr[7], reg)) {
+ case DR7_TYPE_BP_INST:
+ if (env->dr[reg] == env->eip) {
+ bp_match = true;
+ }
+ break;
+ case DR7_TYPE_DATA_WR:
+ case DR7_TYPE_DATA_RW:
+ if (env->cpu_watchpoint[reg] &&
+ env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT) {
+ wp_match = true;
+ }
+ break;
+ case DR7_TYPE_IO_RW:
+ break;
+ }
+ if (bp_match || wp_match) {
+ dr6 |= 1 << reg;
+ if (hw_breakpoint_enabled(env->dr[7], reg)) {
+ hit_enabled = true;
+ }
+ }
+ }
+
+ if (hit_enabled || force_dr6_update) {
+ env->dr[6] = dr6;
+ }
+
+ return hit_enabled;
+}
+
+void breakpoint_handler(CPUState *cs)
+{
+ X86CPU *cpu = X86_CPU(cs);
+ CPUX86State *env = &cpu->env;
+ CPUBreakpoint *bp;
+
+ if (cs->watchpoint_hit) {
+ if (cs->watchpoint_hit->flags & BP_CPU) {
+ cs->watchpoint_hit = NULL;
+ if (check_hw_breakpoints(env, false)) {
+ raise_exception(env, EXCP01_DB);
+ } else {
+ cpu_resume_from_signal(cs, NULL);
+ }
+ }
+ } else {
+ QTAILQ_FOREACH(bp, &cs->breakpoints, entry) {
+ if (bp->pc == env->eip) {
+ if (bp->flags & BP_CPU) {
+ check_hw_breakpoints(env, true);
+ raise_exception(env, EXCP01_DB);
+ }
+ break;
+ }
+ }
+ }
+}
+
+void helper_single_step(CPUX86State *env)
+{
+#ifndef CONFIG_USER_ONLY
+ check_hw_breakpoints(env, true);
+ env->dr[6] |= DR6_BS;
+#endif
+ raise_exception(env, EXCP01_DB);
+}
+
+void helper_movl_drN_T0(CPUX86State *env, int reg, target_ulong t0)
+{
+#ifndef CONFIG_USER_ONLY
+ int i;
+
+ if (reg < 4) {
+ hw_breakpoint_remove(env, reg);
+ env->dr[reg] = t0;
+ hw_breakpoint_insert(env, reg);
+ } else if (reg == 7) {
+ for (i = 0; i < DR7_MAX_BP; i++) {
+ hw_breakpoint_remove(env, i);
+ }
+ env->dr[7] = t0;
+ for (i = 0; i < DR7_MAX_BP; i++) {
+ hw_breakpoint_insert(env, i);
+ }
+ } else {
+ env->dr[reg] = t0;
+ }
+#endif
+}
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 5480a96..0c073e5 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -905,134 +905,6 @@ out:
return pte | page_offset;
}
-void hw_breakpoint_insert(CPUX86State *env, int index)
-{
- CPUState *cs = CPU(x86_env_get_cpu(env));
- int type = 0, err = 0;
-
- switch (hw_breakpoint_type(env->dr[7], index)) {
- case DR7_TYPE_BP_INST:
- if (hw_breakpoint_enabled(env->dr[7], index)) {
- err = cpu_breakpoint_insert(cs, env->dr[index], BP_CPU,
- &env->cpu_breakpoint[index]);
- }
- break;
- case DR7_TYPE_DATA_WR:
- type = BP_CPU | BP_MEM_WRITE;
- break;
- case DR7_TYPE_IO_RW:
- /* No support for I/O watchpoints yet */
- break;
- case DR7_TYPE_DATA_RW:
- type = BP_CPU | BP_MEM_ACCESS;
- break;
- }
-
- if (type != 0) {
- err = cpu_watchpoint_insert(cs, env->dr[index],
- hw_breakpoint_len(env->dr[7], index),
- type, &env->cpu_watchpoint[index]);
- }
-
- if (err) {
- env->cpu_breakpoint[index] = NULL;
- }
-}
-
-void hw_breakpoint_remove(CPUX86State *env, int index)
-{
- CPUState *cs;
-
- if (!env->cpu_breakpoint[index]) {
- return;
- }
- cs = CPU(x86_env_get_cpu(env));
- switch (hw_breakpoint_type(env->dr[7], index)) {
- case DR7_TYPE_BP_INST:
- if (hw_breakpoint_enabled(env->dr[7], index)) {
- cpu_breakpoint_remove_by_ref(cs, env->cpu_breakpoint[index]);
- }
- break;
- case DR7_TYPE_DATA_WR:
- case DR7_TYPE_DATA_RW:
- cpu_watchpoint_remove_by_ref(cs, env->cpu_watchpoint[index]);
- break;
- case DR7_TYPE_IO_RW:
- /* No support for I/O watchpoints yet */
- break;
- }
-}
-
-bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
-{
- target_ulong dr6;
- int reg;
- bool hit_enabled = false;
-
- dr6 = env->dr[6] & ~0xf;
- for (reg = 0; reg < DR7_MAX_BP; reg++) {
- bool bp_match = false;
- bool wp_match = false;
-
- switch (hw_breakpoint_type(env->dr[7], reg)) {
- case DR7_TYPE_BP_INST:
- if (env->dr[reg] == env->eip) {
- bp_match = true;
- }
- break;
- case DR7_TYPE_DATA_WR:
- case DR7_TYPE_DATA_RW:
- if (env->cpu_watchpoint[reg] &&
- env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT) {
- wp_match = true;
- }
- break;
- case DR7_TYPE_IO_RW:
- break;
- }
- if (bp_match || wp_match) {
- dr6 |= 1 << reg;
- if (hw_breakpoint_enabled(env->dr[7], reg)) {
- hit_enabled = true;
- }
- }
- }
-
- if (hit_enabled || force_dr6_update) {
- env->dr[6] = dr6;
- }
-
- return hit_enabled;
-}
-
-void breakpoint_handler(CPUState *cs)
-{
- X86CPU *cpu = X86_CPU(cs);
- CPUX86State *env = &cpu->env;
- CPUBreakpoint *bp;
-
- if (cs->watchpoint_hit) {
- if (cs->watchpoint_hit->flags & BP_CPU) {
- cs->watchpoint_hit = NULL;
- if (check_hw_breakpoints(env, false)) {
- raise_exception(env, EXCP01_DB);
- } else {
- cpu_resume_from_signal(cs, NULL);
- }
- }
- } else {
- QTAILQ_FOREACH(bp, &cs->breakpoints, entry) {
- if (bp->pc == env->eip) {
- if (bp->flags & BP_CPU) {
- check_hw_breakpoints(env, true);
- raise_exception(env, EXCP01_DB);
- }
- break;
- }
- }
- }
-}
-
typedef struct MCEInjectionParams {
Monitor *mon;
X86CPU *cpu;
diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
index 6bfc7dd..13bd4f5 100644
--- a/target-i386/misc_helper.c
+++ b/target-i386/misc_helper.c
@@ -95,15 +95,6 @@ void helper_into(CPUX86State *env, int next_eip_addend)
}
}
-void helper_single_step(CPUX86State *env)
-{
-#ifndef CONFIG_USER_ONLY
- check_hw_breakpoints(env, true);
- env->dr[6] |= DR6_BS;
-#endif
- raise_exception(env, EXCP01_DB);
-}
-
void helper_cpuid(CPUX86State *env)
{
uint32_t eax, ebx, ecx, edx;
@@ -127,10 +118,6 @@ target_ulong helper_read_crN(CPUX86State *env, int reg)
void helper_write_crN(CPUX86State *env, int reg, target_ulong t0)
{
}
-
-void helper_movl_drN_T0(CPUX86State *env, int reg, target_ulong t0)
-{
-}
#else
target_ulong helper_read_crN(CPUX86State *env, int reg)
{
@@ -176,27 +163,6 @@ void helper_write_crN(CPUX86State *env, int reg, target_ulong t0)
break;
}
}
-
-void helper_movl_drN_T0(CPUX86State *env, int reg, target_ulong t0)
-{
- int i;
-
- if (reg < 4) {
- hw_breakpoint_remove(env, reg);
- env->dr[reg] = t0;
- hw_breakpoint_insert(env, reg);
- } else if (reg == 7) {
- for (i = 0; i < DR7_MAX_BP; i++) {
- hw_breakpoint_remove(env, i);
- }
- env->dr[7] = t0;
- for (i = 0; i < DR7_MAX_BP; i++) {
- hw_breakpoint_insert(env, i);
- }
- } else {
- env->dr[reg] = t0;
- }
-}
#endif
void helper_lmsw(CPUX86State *env, target_ulong t0)
--
2.1.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 2/8] target-i386: Make check_hw_breakpoints static
2015-09-15 18:45 [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions Richard Henderson
2015-09-15 18:45 ` [Qemu-devel] [PATCH 1/8] target-i386: Move breakpoint related functions to new file Richard Henderson
@ 2015-09-15 18:45 ` Richard Henderson
2015-09-18 18:29 ` Eduardo Habkost
2015-09-15 18:45 ` [Qemu-devel] [PATCH 3/8] target-i386: Introduce cpu_x86_update_dr7 Richard Henderson
` (6 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2015-09-15 18:45 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, ehabkost
The function is now only used from within a single file.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target-i386/bpt_helper.c | 2 +-
target-i386/cpu.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index 6f6537d..c071c24 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -79,7 +79,7 @@ void hw_breakpoint_remove(CPUX86State *env, int index)
}
}
-bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
+static bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
{
target_ulong dr6;
int reg;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 3bcf2f6..9189a63 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1152,7 +1152,6 @@ static inline int hw_breakpoint_len(unsigned long dr7, int index)
void hw_breakpoint_insert(CPUX86State *env, int index);
void hw_breakpoint_remove(CPUX86State *env, int index);
-bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update);
void breakpoint_handler(CPUState *cs);
/* will be suppressed */
--
2.1.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 3/8] target-i386: Introduce cpu_x86_update_dr7
2015-09-15 18:45 [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions Richard Henderson
2015-09-15 18:45 ` [Qemu-devel] [PATCH 1/8] target-i386: Move breakpoint related functions to new file Richard Henderson
2015-09-15 18:45 ` [Qemu-devel] [PATCH 2/8] target-i386: Make check_hw_breakpoints static Richard Henderson
@ 2015-09-15 18:45 ` Richard Henderson
2015-09-15 18:45 ` [Qemu-devel] [PATCH 4/8] target-i386: Re-introduce optimal breakpoint removal Richard Henderson
` (5 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2015-09-15 18:45 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, ehabkost
This moves the last of the iteration over breakpoints into
the bpt_helper.c file. This also allows us to make several
breakpoint functions static.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target-i386/bpt_helper.c | 29 ++++++++++++++++++-----------
target-i386/cpu.h | 4 ++--
target-i386/machine.c | 8 ++++++--
target-i386/seg_helper.c | 8 +-------
4 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index c071c24..f14788a 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -21,7 +21,8 @@
#include "exec/helper-proto.h"
-void hw_breakpoint_insert(CPUX86State *env, int index)
+#ifndef CONFIG_USER_ONLY
+static void hw_breakpoint_insert(CPUX86State *env, int index)
{
CPUState *cs = CPU(x86_env_get_cpu(env));
int type = 0, err = 0;
@@ -55,7 +56,7 @@ void hw_breakpoint_insert(CPUX86State *env, int index)
}
}
-void hw_breakpoint_remove(CPUX86State *env, int index)
+static void hw_breakpoint_remove(CPUX86State *env, int index)
{
CPUState *cs;
@@ -79,6 +80,20 @@ void hw_breakpoint_remove(CPUX86State *env, int index)
}
}
+void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7)
+{
+ int i;
+
+ for (i = 0; i < DR7_MAX_BP; i++) {
+ hw_breakpoint_remove(env, i);
+ }
+ env->dr[7] = new_dr7;
+ for (i = 0; i < DR7_MAX_BP; i++) {
+ hw_breakpoint_insert(env, i);
+ }
+}
+#endif
+
static bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
{
target_ulong dr6;
@@ -161,20 +176,12 @@ void helper_single_step(CPUX86State *env)
void helper_movl_drN_T0(CPUX86State *env, int reg, target_ulong t0)
{
#ifndef CONFIG_USER_ONLY
- int i;
-
if (reg < 4) {
hw_breakpoint_remove(env, reg);
env->dr[reg] = t0;
hw_breakpoint_insert(env, reg);
} else if (reg == 7) {
- for (i = 0; i < DR7_MAX_BP; i++) {
- hw_breakpoint_remove(env, i);
- }
- env->dr[7] = t0;
- for (i = 0; i < DR7_MAX_BP; i++) {
- hw_breakpoint_insert(env, i);
- }
+ cpu_x86_update_dr7(env, t0);
} else {
env->dr[reg] = t0;
}
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 9189a63..6d3e4f9 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -234,6 +234,7 @@
#define DR7_TYPE_SHIFT 16
#define DR7_LEN_SHIFT 18
#define DR7_FIXED_1 0x00000400
+#define DR7_GLOBAL_BP_MASK 0xaa
#define DR7_LOCAL_BP_MASK 0x55
#define DR7_MAX_BP 4
#define DR7_TYPE_BP_INST 0x0
@@ -1150,14 +1151,13 @@ static inline int hw_breakpoint_len(unsigned long dr7, int index)
return (len == 2) ? 8 : len + 1;
}
-void hw_breakpoint_insert(CPUX86State *env, int index);
-void hw_breakpoint_remove(CPUX86State *env, int index);
void breakpoint_handler(CPUState *cs);
/* will be suppressed */
void cpu_x86_update_cr0(CPUX86State *env, uint32_t new_cr0);
void cpu_x86_update_cr3(CPUX86State *env, target_ulong new_cr3);
void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4);
+void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7);
/* hw/pc.c */
uint64_t cpu_get_tsc(CPUX86State *env);
diff --git a/target-i386/machine.c b/target-i386/machine.c
index a0df64b..782d057 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -367,8 +367,12 @@ static int cpu_post_load(void *opaque, int version_id)
cpu_breakpoint_remove_all(cs, BP_CPU);
cpu_watchpoint_remove_all(cs, BP_CPU);
- for (i = 0; i < DR7_MAX_BP; i++) {
- hw_breakpoint_insert(env, i);
+ {
+ /* Indicate all breakpoints disabled, as they are, then
+ let the helper re-enable them. */
+ target_ulong dr7 = env->dr[7];
+ env->dr[7] = dr7 & ~(DR7_GLOBAL_BP_MASK | DR7_LOCAL_BP_MASK);
+ cpu_x86_update_dr7(env, dr7);
}
tlb_flush(cs, 1);
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index 1a3a2e7..0257706 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -501,13 +501,7 @@ static void switch_tss_ra(CPUX86State *env, int tss_selector,
#ifndef CONFIG_USER_ONLY
/* reset local breakpoints */
if (env->dr[7] & DR7_LOCAL_BP_MASK) {
- for (i = 0; i < DR7_MAX_BP; i++) {
- if (hw_local_breakpoint_enabled(env->dr[7], i) &&
- !hw_global_breakpoint_enabled(env->dr[7], i)) {
- hw_breakpoint_remove(env, i);
- }
- }
- env->dr[7] &= ~DR7_LOCAL_BP_MASK;
+ cpu_x86_update_dr7(env, env->dr[7] & ~DR7_LOCAL_BP_MASK);
}
#endif
}
--
2.1.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 4/8] target-i386: Re-introduce optimal breakpoint removal
2015-09-15 18:45 [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions Richard Henderson
` (2 preceding siblings ...)
2015-09-15 18:45 ` [Qemu-devel] [PATCH 3/8] target-i386: Introduce cpu_x86_update_dr7 Richard Henderson
@ 2015-09-15 18:45 ` Richard Henderson
2015-09-16 8:57 ` Paolo Bonzini
2015-09-15 18:45 ` [Qemu-devel] [PATCH 5/8] target-i386: Move hw_*breakpoint_* functions Richard Henderson
` (4 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2015-09-15 18:45 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, ehabkost
Before the last patch, we had an efficient loop that disabled
local breakpoints on task switch. Re-add that, but in a more
general way that handles changes to the global enable bits too.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target-i386/bpt_helper.c | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)
diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index f14788a..911d9f8 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -82,14 +82,36 @@ static void hw_breakpoint_remove(CPUX86State *env, int index)
void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7)
{
+ target_ulong old_dr7 = env->dr[7];
int i;
- for (i = 0; i < DR7_MAX_BP; i++) {
- hw_breakpoint_remove(env, i);
- }
- env->dr[7] = new_dr7;
- for (i = 0; i < DR7_MAX_BP; i++) {
- hw_breakpoint_insert(env, i);
+ /* If nothing is changing except the global/local enable bits,
+ then we can make the change more efficient. */
+ if (((old_dr7 ^ new_dr7) & ~0xff) == 0) {
+ /* Fold the global and local enable bits together into the
+ global fields, then xor to show which registers have
+ changed collective enable state. */
+ int mod = ((old_dr7 | old_dr7 * 2) ^ (new_dr7 | new_dr7 * 2)) & 0xff;
+
+ for (i = 0; i < DR7_MAX_BP; i++) {
+ if (mod & (2 << i * 2)) {
+ /* We know that register i has changed enable state;
+ recheck what that state should be and apply. */
+ if (hw_breakpoint_enabled(new_dr7, i)) {
+ hw_breakpoint_insert(env, i);
+ } else {
+ hw_breakpoint_remove(env, i);
+ }
+ }
+ }
+ } else {
+ for (i = 0; i < DR7_MAX_BP; i++) {
+ hw_breakpoint_remove(env, i);
+ }
+ env->dr[7] = new_dr7;
+ for (i = 0; i < DR7_MAX_BP; i++) {
+ hw_breakpoint_insert(env, i);
+ }
}
}
#endif
--
2.1.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 5/8] target-i386: Move hw_*breakpoint_* functions
2015-09-15 18:45 [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions Richard Henderson
` (3 preceding siblings ...)
2015-09-15 18:45 ` [Qemu-devel] [PATCH 4/8] target-i386: Re-introduce optimal breakpoint removal Richard Henderson
@ 2015-09-15 18:45 ` Richard Henderson
2015-09-15 18:45 ` [Qemu-devel] [PATCH 6/8] target-i386: Optimize setting dr[0-3] Richard Henderson
` (3 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2015-09-15 18:45 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, ehabkost
They're only used from bpt_helper.c now.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target-i386/bpt_helper.c | 29 ++++++++++++++++++++++++++++-
target-i386/cpu.h | 27 ---------------------------
2 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index 911d9f8..10bbf9f 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -22,6 +22,33 @@
#ifndef CONFIG_USER_ONLY
+static inline bool hw_local_breakpoint_enabled(unsigned long dr7, int index)
+{
+ return (dr7 >> (index * 2)) & 1;
+}
+
+static inline bool hw_global_breakpoint_enabled(unsigned long dr7, int index)
+{
+ return (dr7 >> (index * 2)) & 2;
+
+}
+static inline bool hw_breakpoint_enabled(unsigned long dr7, int index)
+{
+ return hw_global_breakpoint_enabled(dr7, index) ||
+ hw_local_breakpoint_enabled(dr7, index);
+}
+
+static inline int hw_breakpoint_type(unsigned long dr7, int index)
+{
+ return (dr7 >> (DR7_TYPE_SHIFT + (index * 4))) & 3;
+}
+
+static inline int hw_breakpoint_len(unsigned long dr7, int index)
+{
+ int len = ((dr7 >> (DR7_LEN_SHIFT + (index * 4))) & 3);
+ return (len == 2) ? 8 : len + 1;
+}
+
static void hw_breakpoint_insert(CPUX86State *env, int index)
{
CPUState *cs = CPU(x86_env_get_cpu(env));
@@ -114,7 +141,6 @@ void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7)
}
}
}
-#endif
static bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
{
@@ -185,6 +211,7 @@ void breakpoint_handler(CPUState *cs)
}
}
}
+#endif
void helper_single_step(CPUX86State *env)
{
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 6d3e4f9..8f07db1 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1124,33 +1124,6 @@ void x86_stl_phys(CPUState *cs, hwaddr addr, uint32_t val);
void x86_stq_phys(CPUState *cs, hwaddr addr, uint64_t val);
#endif
-static inline bool hw_local_breakpoint_enabled(unsigned long dr7, int index)
-{
- return (dr7 >> (index * 2)) & 1;
-}
-
-static inline bool hw_global_breakpoint_enabled(unsigned long dr7, int index)
-{
- return (dr7 >> (index * 2)) & 2;
-
-}
-static inline bool hw_breakpoint_enabled(unsigned long dr7, int index)
-{
- return hw_global_breakpoint_enabled(dr7, index) ||
- hw_local_breakpoint_enabled(dr7, index);
-}
-
-static inline int hw_breakpoint_type(unsigned long dr7, int index)
-{
- return (dr7 >> (DR7_TYPE_SHIFT + (index * 4))) & 3;
-}
-
-static inline int hw_breakpoint_len(unsigned long dr7, int index)
-{
- int len = ((dr7 >> (DR7_LEN_SHIFT + (index * 4))) & 3);
- return (len == 2) ? 8 : len + 1;
-}
-
void breakpoint_handler(CPUState *cs);
/* will be suppressed */
--
2.1.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 6/8] target-i386: Optimize setting dr[0-3]
2015-09-15 18:45 [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions Richard Henderson
` (4 preceding siblings ...)
2015-09-15 18:45 ` [Qemu-devel] [PATCH 5/8] target-i386: Move hw_*breakpoint_* functions Richard Henderson
@ 2015-09-15 18:45 ` Richard Henderson
2015-09-15 18:45 ` [Qemu-devel] [PATCH 7/8] target-i386: Handle I/O breakpoints Richard Henderson
` (2 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2015-09-15 18:45 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, ehabkost
If the debug register is not enabled, we need
do nothing besides update the register.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target-i386/bpt_helper.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index 10bbf9f..f872387 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -226,9 +226,14 @@ void helper_movl_drN_T0(CPUX86State *env, int reg, target_ulong t0)
{
#ifndef CONFIG_USER_ONLY
if (reg < 4) {
- hw_breakpoint_remove(env, reg);
- env->dr[reg] = t0;
- hw_breakpoint_insert(env, reg);
+ if (hw_breakpoint_enabled(env->dr[7], reg)
+ && hw_breakpoint_type(env->dr[7], reg) != DR7_TYPE_IO_RW) {
+ hw_breakpoint_remove(env, reg);
+ env->dr[reg] = t0;
+ hw_breakpoint_insert(env, reg);
+ } else {
+ env->dr[reg] = t0;
+ }
} else if (reg == 7) {
cpu_x86_update_dr7(env, t0);
} else {
--
2.1.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 7/8] target-i386: Handle I/O breakpoints
2015-09-15 18:45 [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions Richard Henderson
` (5 preceding siblings ...)
2015-09-15 18:45 ` [Qemu-devel] [PATCH 6/8] target-i386: Optimize setting dr[0-3] Richard Henderson
@ 2015-09-15 18:45 ` Richard Henderson
2015-09-15 18:45 ` [Qemu-devel] [PATCH 8/8] target-i386: Check CR4[DE] for processing DR4/DR5 Richard Henderson
2015-09-21 12:05 ` [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions Paolo Bonzini
8 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2015-09-15 18:45 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, ehabkost
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target-i386/bpt_helper.c | 99 +++++++++++++++++++++++++++++++++++-------------
target-i386/cpu.h | 2 +
target-i386/helper.h | 1 +
target-i386/translate.c | 20 +++++++++-
4 files changed, 94 insertions(+), 28 deletions(-)
diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index f872387..9050006 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -49,60 +49,72 @@ static inline int hw_breakpoint_len(unsigned long dr7, int index)
return (len == 2) ? 8 : len + 1;
}
-static void hw_breakpoint_insert(CPUX86State *env, int index)
+static int hw_breakpoint_insert(CPUX86State *env, int index)
{
CPUState *cs = CPU(x86_env_get_cpu(env));
- int type = 0, err = 0;
+ target_ulong dr7 = env->dr[7];
+ target_ulong drN = env->dr[index];
+ int err = 0;
- switch (hw_breakpoint_type(env->dr[7], index)) {
+ switch (hw_breakpoint_type(dr7, index)) {
case DR7_TYPE_BP_INST:
- if (hw_breakpoint_enabled(env->dr[7], index)) {
- err = cpu_breakpoint_insert(cs, env->dr[index], BP_CPU,
+ if (hw_breakpoint_enabled(dr7, index)) {
+ err = cpu_breakpoint_insert(cs, drN, BP_CPU,
&env->cpu_breakpoint[index]);
}
break;
- case DR7_TYPE_DATA_WR:
- type = BP_CPU | BP_MEM_WRITE;
- break;
+
case DR7_TYPE_IO_RW:
- /* No support for I/O watchpoints yet */
+ /* Notice when we should enable calls to bpt_io. */
+ return (hw_breakpoint_enabled(env->dr[7], index)
+ ? HF_IOBPT_MASK : 0);
+
+ case DR7_TYPE_DATA_WR:
+ if (hw_breakpoint_enabled(dr7, index)) {
+ err = cpu_watchpoint_insert(cs, drN,
+ hw_breakpoint_len(dr7, index),
+ BP_CPU | BP_MEM_WRITE,
+ &env->cpu_watchpoint[index]);
+ }
break;
+
case DR7_TYPE_DATA_RW:
- type = BP_CPU | BP_MEM_ACCESS;
+ if (hw_breakpoint_enabled(dr7, index)) {
+ err = cpu_watchpoint_insert(cs, drN,
+ hw_breakpoint_len(dr7, index),
+ BP_CPU | BP_MEM_ACCESS,
+ &env->cpu_watchpoint[index]);
+ }
break;
}
-
- if (type != 0) {
- err = cpu_watchpoint_insert(cs, env->dr[index],
- hw_breakpoint_len(env->dr[7], index),
- type, &env->cpu_watchpoint[index]);
- }
-
if (err) {
env->cpu_breakpoint[index] = NULL;
}
+ return 0;
}
static void hw_breakpoint_remove(CPUX86State *env, int index)
{
- CPUState *cs;
+ CPUState *cs = CPU(x86_env_get_cpu(env));
- if (!env->cpu_breakpoint[index]) {
- return;
- }
- cs = CPU(x86_env_get_cpu(env));
switch (hw_breakpoint_type(env->dr[7], index)) {
case DR7_TYPE_BP_INST:
- if (hw_breakpoint_enabled(env->dr[7], index)) {
+ if (env->cpu_breakpoint[index]) {
cpu_breakpoint_remove_by_ref(cs, env->cpu_breakpoint[index]);
+ env->cpu_breakpoint[index] = NULL;
}
break;
+
case DR7_TYPE_DATA_WR:
case DR7_TYPE_DATA_RW:
- cpu_watchpoint_remove_by_ref(cs, env->cpu_watchpoint[index]);
+ if (env->cpu_breakpoint[index]) {
+ cpu_watchpoint_remove_by_ref(cs, env->cpu_watchpoint[index]);
+ env->cpu_breakpoint[index] = NULL;
+ }
break;
+
case DR7_TYPE_IO_RW:
- /* No support for I/O watchpoints yet */
+ /* HF_IOBPT_MASK cleared elsewhere. */
break;
}
}
@@ -110,6 +122,7 @@ static void hw_breakpoint_remove(CPUX86State *env, int index)
void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7)
{
target_ulong old_dr7 = env->dr[7];
+ int iobpt = 0;
int i;
/* If nothing is changing except the global/local enable bits,
@@ -125,10 +138,13 @@ void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7)
/* We know that register i has changed enable state;
recheck what that state should be and apply. */
if (hw_breakpoint_enabled(new_dr7, i)) {
- hw_breakpoint_insert(env, i);
+ iobpt |= hw_breakpoint_insert(env, i);
} else {
hw_breakpoint_remove(env, i);
}
+ } else if (hw_breakpoint_type(new_dr7, i) == DR7_TYPE_IO_RW
+ && hw_breakpoint_enabled(new_dr7, i)) {
+ iobpt |= HF_IOBPT_MASK;
}
}
} else {
@@ -137,9 +153,11 @@ void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7)
}
env->dr[7] = new_dr7;
for (i = 0; i < DR7_MAX_BP; i++) {
- hw_breakpoint_insert(env, i);
+ iobpt |= hw_breakpoint_insert(env, i);
}
}
+
+ env->hflags = (env->hflags & ~HF_IOBPT_MASK) | iobpt;
}
static bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
@@ -241,3 +259,30 @@ void helper_movl_drN_T0(CPUX86State *env, int reg, target_ulong t0)
}
#endif
}
+
+/* Check if Port I/O is trapped by a breakpoint. */
+void helper_bpt_io(CPUX86State *env, uint32_t port,
+ uint32_t size, target_ulong next_eip)
+{
+#ifndef CONFIG_USER_ONLY
+ target_ulong dr7 = env->dr[7];
+ int i, hit = 0;
+
+ for (i = 0; i < DR7_MAX_BP; ++i) {
+ if (hw_breakpoint_type(dr7, i) == DR7_TYPE_IO_RW
+ && hw_breakpoint_enabled(dr7, i)) {
+ int bpt_len = hw_breakpoint_len(dr7, i);
+ if (port + size - 1 >= env->dr[i]
+ && port <= env->dr[i] + bpt_len - 1) {
+ hit |= 1 << i;
+ }
+ }
+ }
+
+ if (hit) {
+ env->dr[6] = (env->dr[6] & ~0xf) | hit;
+ env->eip = next_eip;
+ raise_exception(env, EXCP01_DB);
+ }
+#endif
+}
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 8f07db1..74914f0 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -154,6 +154,7 @@
#define HF_SVMI_SHIFT 21 /* SVM intercepts are active */
#define HF_OSFXSR_SHIFT 22 /* CR4.OSFXSR */
#define HF_SMAP_SHIFT 23 /* CR4.SMAP */
+#define HF_IOBPT_SHIFT 24 /* an io breakpoint enabled */
#define HF_CPL_MASK (3 << HF_CPL_SHIFT)
#define HF_SOFTMMU_MASK (1 << HF_SOFTMMU_SHIFT)
@@ -177,6 +178,7 @@
#define HF_SVMI_MASK (1 << HF_SVMI_SHIFT)
#define HF_OSFXSR_MASK (1 << HF_OSFXSR_SHIFT)
#define HF_SMAP_MASK (1 << HF_SMAP_SHIFT)
+#define HF_IOBPT_MASK (1 << HF_IOBPT_SHIFT)
/* hflags2 */
diff --git a/target-i386/helper.h b/target-i386/helper.h
index 8454a04..e9858c0 100644
--- a/target-i386/helper.h
+++ b/target-i386/helper.h
@@ -92,6 +92,7 @@ DEF_HELPER_3(outw, void, env, i32, i32)
DEF_HELPER_2(inw, tl, env, i32)
DEF_HELPER_3(outl, void, env, i32, i32)
DEF_HELPER_2(inl, tl, env, i32)
+DEF_HELPER_FLAGS_4(bpt_io, TCG_CALL_NO_WG, void, env, i32, i32, tl)
DEF_HELPER_3(svm_check_intercept_param, void, env, i32, i64)
DEF_HELPER_3(vmexit, void, env, i32, i64)
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 4c89488..2c06f01 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -1156,6 +1156,19 @@ static inline void gen_cmps(DisasContext *s, TCGMemOp ot)
gen_op_add_reg_T0(s->aflag, R_EDI);
}
+static void gen_bpt_io(DisasContext *s, TCGv_i32 t_port, int ot)
+{
+ if (s->flags & HF_IOBPT_MASK) {
+ TCGv_i32 t_size = tcg_const_i32(1 << ot);
+ TCGv t_next = tcg_const_tl(s->pc - s->cs_base);
+
+ gen_helper_bpt_io(cpu_env, t_port, t_size, t_next);
+ tcg_temp_free_i32(t_size);
+ tcg_temp_free(t_next);
+ }
+}
+
+
static inline void gen_ins(DisasContext *s, TCGMemOp ot)
{
if (s->tb->cflags & CF_USE_ICOUNT) {
@@ -1172,6 +1185,7 @@ static inline void gen_ins(DisasContext *s, TCGMemOp ot)
gen_op_st_v(s, ot, cpu_T[0], cpu_A0);
gen_op_movl_T0_Dshift(ot);
gen_op_add_reg_T0(s->aflag, R_EDI);
+ gen_bpt_io(s, cpu_tmp2_i32, ot);
if (s->tb->cflags & CF_USE_ICOUNT) {
gen_io_end();
}
@@ -1189,9 +1203,9 @@ static inline void gen_outs(DisasContext *s, TCGMemOp ot)
tcg_gen_andi_i32(cpu_tmp2_i32, cpu_tmp2_i32, 0xffff);
tcg_gen_trunc_tl_i32(cpu_tmp3_i32, cpu_T[0]);
gen_helper_out_func(ot, cpu_tmp2_i32, cpu_tmp3_i32);
-
gen_op_movl_T0_Dshift(ot);
gen_op_add_reg_T0(s->aflag, R_ESI);
+ gen_bpt_io(s, cpu_tmp2_i32, ot);
if (s->tb->cflags & CF_USE_ICOUNT) {
gen_io_end();
}
@@ -6276,6 +6290,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
tcg_gen_movi_i32(cpu_tmp2_i32, val);
gen_helper_in_func(ot, cpu_T[1], cpu_tmp2_i32);
gen_op_mov_reg_v(ot, R_EAX, cpu_T[1]);
+ gen_bpt_io(s, cpu_tmp2_i32, ot);
if (s->tb->cflags & CF_USE_ICOUNT) {
gen_io_end();
gen_jmp(s, s->pc - s->cs_base);
@@ -6296,6 +6311,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
tcg_gen_movi_i32(cpu_tmp2_i32, val);
tcg_gen_trunc_tl_i32(cpu_tmp3_i32, cpu_T[1]);
gen_helper_out_func(ot, cpu_tmp2_i32, cpu_tmp3_i32);
+ gen_bpt_io(s, cpu_tmp2_i32, ot);
if (s->tb->cflags & CF_USE_ICOUNT) {
gen_io_end();
gen_jmp(s, s->pc - s->cs_base);
@@ -6313,6 +6329,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
tcg_gen_trunc_tl_i32(cpu_tmp2_i32, cpu_T[0]);
gen_helper_in_func(ot, cpu_T[1], cpu_tmp2_i32);
gen_op_mov_reg_v(ot, R_EAX, cpu_T[1]);
+ gen_bpt_io(s, cpu_tmp2_i32, ot);
if (s->tb->cflags & CF_USE_ICOUNT) {
gen_io_end();
gen_jmp(s, s->pc - s->cs_base);
@@ -6332,6 +6349,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
tcg_gen_trunc_tl_i32(cpu_tmp2_i32, cpu_T[0]);
tcg_gen_trunc_tl_i32(cpu_tmp3_i32, cpu_T[1]);
gen_helper_out_func(ot, cpu_tmp2_i32, cpu_tmp3_i32);
+ gen_bpt_io(s, cpu_tmp2_i32, ot);
if (s->tb->cflags & CF_USE_ICOUNT) {
gen_io_end();
gen_jmp(s, s->pc - s->cs_base);
--
2.1.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 8/8] target-i386: Check CR4[DE] for processing DR4/DR5
2015-09-15 18:45 [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions Richard Henderson
` (6 preceding siblings ...)
2015-09-15 18:45 ` [Qemu-devel] [PATCH 7/8] target-i386: Handle I/O breakpoints Richard Henderson
@ 2015-09-15 18:45 ` Richard Henderson
2015-09-21 12:05 ` [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions Paolo Bonzini
8 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2015-09-15 18:45 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, ehabkost
Introduce helper_get_dr so that we don't have to put CR4[DE]
into the scarce HFLAGS resource. At the same time, rename
helper_movl_drN_T0 to helper_set_dr and set the helper flags.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target-i386/bpt_helper.c | 46 +++++++++++++++++++++++++++++++++++++++++-----
target-i386/cpu.h | 2 +-
target-i386/helper.h | 3 ++-
target-i386/translate.c | 10 ++++++----
4 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index 9050006..c258598 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -240,10 +240,11 @@ void helper_single_step(CPUX86State *env)
raise_exception(env, EXCP01_DB);
}
-void helper_movl_drN_T0(CPUX86State *env, int reg, target_ulong t0)
+void helper_set_dr(CPUX86State *env, int reg, target_ulong t0)
{
#ifndef CONFIG_USER_ONLY
- if (reg < 4) {
+ switch (reg) {
+ case 0: case 1: case 2: case 3:
if (hw_breakpoint_enabled(env->dr[7], reg)
&& hw_breakpoint_type(env->dr[7], reg) != DR7_TYPE_IO_RW) {
hw_breakpoint_remove(env, reg);
@@ -252,14 +253,49 @@ void helper_movl_drN_T0(CPUX86State *env, int reg, target_ulong t0)
} else {
env->dr[reg] = t0;
}
- } else if (reg == 7) {
+ return;
+ case 4:
+ if (env->cr[4] & CR4_DE_MASK) {
+ break;
+ }
+ /* fallthru */
+ case 6:
+ env->dr[6] = t0;
+ return;
+ case 5:
+ if (env->cr[4] & CR4_DE_MASK) {
+ break;
+ }
+ /* fallthru */
+ case 7:
cpu_x86_update_dr7(env, t0);
- } else {
- env->dr[reg] = t0;
+ return;
}
+ raise_exception_err_ra(env, EXCP06_ILLOP, 0, GETPC());
#endif
}
+target_ulong helper_get_dr(CPUX86State *env, int reg)
+{
+ switch (reg) {
+ case 0: case 1: case 2: case 3: case 6: case 7:
+ return env->dr[reg];
+ case 4:
+ if (env->cr[4] & CR4_DE_MASK) {
+ break;
+ } else {
+ return env->dr[6];
+ }
+ case 5:
+ if (env->cr[4] & CR4_DE_MASK) {
+ break;
+ } else {
+ return env->dr[7];
+ }
+ }
+ raise_exception_err_ra(env, EXCP06_ILLOP, 0, GETPC());
+}
+
/* Check if Port I/O is trapped by a breakpoint. */
void helper_bpt_io(CPUX86State *env, uint32_t port,
uint32_t size, target_ulong next_eip)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 74914f0..e71fb1b 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -916,7 +916,7 @@ typedef struct CPUX86State {
int error_code;
int exception_is_int;
target_ulong exception_next_eip;
- target_ulong dr[8]; /* debug registers */
+ target_ulong dr[8]; /* debug registers; note dr4 and dr5 are unused */
union {
struct CPUBreakpoint *cpu_breakpoint[4];
struct CPUWatchpoint *cpu_watchpoint[4];
diff --git a/target-i386/helper.h b/target-i386/helper.h
index e9858c0..ecfcfd1 100644
--- a/target-i386/helper.h
+++ b/target-i386/helper.h
@@ -40,7 +40,8 @@ DEF_HELPER_2(read_crN, tl, env, int)
DEF_HELPER_3(write_crN, void, env, int, tl)
DEF_HELPER_2(lmsw, void, env, tl)
DEF_HELPER_1(clts, void, env)
-DEF_HELPER_3(movl_drN_T0, void, env, int, tl)
+DEF_HELPER_FLAGS_3(set_dr, TCG_CALL_NO_WG, void, env, int, tl)
+DEF_HELPER_FLAGS_2(get_dr, TCG_CALL_NO_WG, tl, env, int)
DEF_HELPER_2(invlpg, void, env, tl)
DEF_HELPER_4(enter_level, void, env, int, int, tl)
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 2c06f01..fbef312 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7634,18 +7634,20 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
ot = MO_64;
else
ot = MO_32;
- /* XXX: do it dynamically with CR4.DE bit */
- if (reg == 4 || reg == 5 || reg >= 8)
+ if (reg >= 8) {
goto illegal_op;
+ }
if (b & 2) {
gen_svm_check_intercept(s, pc_start, SVM_EXIT_WRITE_DR0 + reg);
gen_op_mov_v_reg(ot, cpu_T[0], rm);
- gen_helper_movl_drN_T0(cpu_env, tcg_const_i32(reg), cpu_T[0]);
+ tcg_gen_movi_i32(cpu_tmp2_i32, reg);
+ gen_helper_set_dr(cpu_env, cpu_tmp2_i32, cpu_T[0]);
gen_jmp_im(s->pc - s->cs_base);
gen_eob(s);
} else {
gen_svm_check_intercept(s, pc_start, SVM_EXIT_READ_DR0 + reg);
- tcg_gen_ld_tl(cpu_T[0], cpu_env, offsetof(CPUX86State,dr[reg]));
+ tcg_gen_movi_i32(cpu_tmp2_i32, reg);
+ gen_helper_get_dr(cpu_T[0], cpu_env, cpu_tmp2_i32);
gen_op_mov_reg_v(ot, rm, cpu_T[0]);
}
}
--
2.1.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 4/8] target-i386: Re-introduce optimal breakpoint removal
2015-09-15 18:45 ` [Qemu-devel] [PATCH 4/8] target-i386: Re-introduce optimal breakpoint removal Richard Henderson
@ 2015-09-16 8:57 ` Paolo Bonzini
2015-09-16 14:57 ` Richard Henderson
2015-09-18 18:38 ` Eduardo Habkost
0 siblings, 2 replies; 20+ messages in thread
From: Paolo Bonzini @ 2015-09-16 8:57 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: ehabkost
On 15/09/2015 20:45, Richard Henderson wrote:
> + /* Fold the global and local enable bits together into the
> + global fields, then xor to show which registers have
> + changed collective enable state. */
> + int mod = ((old_dr7 | old_dr7 * 2) ^ (new_dr7 | new_dr7 * 2)) & 0xff;
The AND is not needed at all but, if you add it, you might as well use
"& 0xaa" which is clearer. But even better, just do:
target_ulong old_dr7 = env->dr[7];
int mod = old_dr7 ^ new_dr7;
...
if ((mod & ~0xff) == 0) {
and test with
if (mod & (3 << i * 2))
inside the loop.
Otherwise looks good, I'll write a kvm-unit-tests patch for this.
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 4/8] target-i386: Re-introduce optimal breakpoint removal
2015-09-16 8:57 ` Paolo Bonzini
@ 2015-09-16 14:57 ` Richard Henderson
2015-09-16 14:59 ` Paolo Bonzini
2015-09-18 18:38 ` Eduardo Habkost
1 sibling, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2015-09-16 14:57 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: ehabkost
On 09/16/2015 01:57 AM, Paolo Bonzini wrote:
>
>
> On 15/09/2015 20:45, Richard Henderson wrote:
>> + /* Fold the global and local enable bits together into the
>> + global fields, then xor to show which registers have
>> + changed collective enable state. */
>> + int mod = ((old_dr7 | old_dr7 * 2) ^ (new_dr7 | new_dr7 * 2)) & 0xff;
>
> The AND is not needed at all but, if you add it, you might as well use
> "& 0xaa" which is clearer. But even better, just do:
>
> target_ulong old_dr7 = env->dr[7];
> int mod = old_dr7 ^ new_dr7;
> ...
> if ((mod & ~0xff) == 0) {
>
>
> and test with
>
> if (mod & (3 << i * 2))
>
> inside the loop.
Nope. I wrote that the first time myself. We're interested in two different
things: (1) whether or not something changed outside enable bits, and (2)
whether the enable state changed.
Since (2) is a combination of both global and local enable bits, we must
combine them *and then xor* to see if the enable state actually changes. Just
using (mod & (3 << n)) will report "change" when local enable turns off, but
global enable remains on. Which is not what we want.
Perhaps that comment could stand to be expanded...
r~
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 4/8] target-i386: Re-introduce optimal breakpoint removal
2015-09-16 14:57 ` Richard Henderson
@ 2015-09-16 14:59 ` Paolo Bonzini
0 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2015-09-16 14:59 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: ehabkost
On 16/09/2015 16:57, Richard Henderson wrote:
>>> >> + /* Fold the global and local enable bits together into the
>>> >> + global fields, then xor to show which registers have
>>> >> + changed collective enable state. */
>>> >> + int mod = ((old_dr7 | old_dr7 * 2) ^ (new_dr7 | new_dr7 * 2)) & 0xff;
>> >
>> > The AND is not needed at all but, if you add it, you might as well use
>> > "& 0xaa" which is clearer. But even better, just do:
>> >
>> > target_ulong old_dr7 = env->dr[7];
>> > int mod = old_dr7 ^ new_dr7;
>> > ...
>> > if ((mod & ~0xff) == 0) {
>> >
>> >
>> > and test with
>> >
>> > if (mod & (3 << i * 2))
>> >
>> > inside the loop.
> Nope. I wrote that the first time myself. We're interested in two different
> things: (1) whether or not something changed outside enable bits, and (2)
> whether the enable state changed.
>
> Since (2) is a combination of both global and local enable bits, we must
> combine them *and then xor* to see if the enable state actually changes. Just
> using (mod & (3 << n)) will report "change" when local enable turns off, but
> global enable remains on. Which is not what we want.
Ah, I see now. Perhaps
int old_enable = (old_dr7 | (old_dr7 << 1)) & 0xaa;
int new_enable = (new_dr7 | (new_dr7 << 1)) & 0xaa;
?
> Perhaps that comment could stand to be expanded...
I think at least for me it's just the expression that is too complex.
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] target-i386: Move breakpoint related functions to new file
2015-09-15 18:45 ` [Qemu-devel] [PATCH 1/8] target-i386: Move breakpoint related functions to new file Richard Henderson
@ 2015-09-18 18:27 ` Eduardo Habkost
0 siblings, 0 replies; 20+ messages in thread
From: Eduardo Habkost @ 2015-09-18 18:27 UTC (permalink / raw)
To: Richard Henderson; +Cc: pbonzini, qemu-devel
On Tue, Sep 15, 2015 at 11:45:06AM -0700, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Applied to x86 tree. Thanks!
--
Eduardo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/8] target-i386: Make check_hw_breakpoints static
2015-09-15 18:45 ` [Qemu-devel] [PATCH 2/8] target-i386: Make check_hw_breakpoints static Richard Henderson
@ 2015-09-18 18:29 ` Eduardo Habkost
0 siblings, 0 replies; 20+ messages in thread
From: Eduardo Habkost @ 2015-09-18 18:29 UTC (permalink / raw)
To: Richard Henderson; +Cc: pbonzini, qemu-devel
On Tue, Sep 15, 2015 at 11:45:07AM -0700, Richard Henderson wrote:
> The function is now only used from within a single file.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Applied to x86 tree. Thanks!
--
Eduardo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 4/8] target-i386: Re-introduce optimal breakpoint removal
2015-09-16 8:57 ` Paolo Bonzini
2015-09-16 14:57 ` Richard Henderson
@ 2015-09-18 18:38 ` Eduardo Habkost
1 sibling, 0 replies; 20+ messages in thread
From: Eduardo Habkost @ 2015-09-18 18:38 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Richard Henderson
On Wed, Sep 16, 2015 at 10:57:57AM +0200, Paolo Bonzini wrote:
[...]
> Otherwise looks good, I'll write a kvm-unit-tests patch for this.
I am planning to apply the series as soon as we get kvm-unit-tests
results.
If you think this is ready to get included as-is, even before we have it
tested by kvm-unit-tests, just give me an Acked-by line. :)
--
Eduardo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions
2015-09-15 18:45 [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions Richard Henderson
` (7 preceding siblings ...)
2015-09-15 18:45 ` [Qemu-devel] [PATCH 8/8] target-i386: Check CR4[DE] for processing DR4/DR5 Richard Henderson
@ 2015-09-21 12:05 ` Paolo Bonzini
2015-09-21 14:05 ` Eduardo Habkost
` (2 more replies)
8 siblings, 3 replies; 20+ messages in thread
From: Paolo Bonzini @ 2015-09-21 12:05 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: ehabkost
On 15/09/2015 20:45, Richard Henderson wrote:
> Best guess, since I can't find any code that actually uses them.
> Linux actively turns them off at boot...
I've sent a kvm-unit-tests patch to test debug extensions. It shows
that debug extensions work, but the following needs to be squashed in
patch 4:
diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index c258598..b24e446 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -134,14 +134,14 @@ void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7)
int mod = ((old_dr7 | old_dr7 * 2) ^ (new_dr7 | new_dr7 * 2)) & 0xff;
for (i = 0; i < DR7_MAX_BP; i++) {
- if (mod & (2 << i * 2)) {
- /* We know that register i has changed enable state;
- recheck what that state should be and apply. */
- if (hw_breakpoint_enabled(new_dr7, i)) {
- iobpt |= hw_breakpoint_insert(env, i);
- } else {
- hw_breakpoint_remove(env, i);
- }
+ if ((mod & (2 << i * 2)) && !hw_breakpoint_enabled(new_dr7, i)) {
+ hw_breakpoint_remove(env, i);
+ }
+ }
+ env->dr[7] = new_dr7 | DR7_FIXED_1;
+ for (i = 0; i < DR7_MAX_BP; i++) {
+ if (mod & (2 << i * 2) && hw_breakpoint_enabled(new_dr7, i)) {
+ iobpt |= hw_breakpoint_insert(env, i);
} else if (hw_breakpoint_type(new_dr7, i) == DR7_TYPE_IO_RW
&& hw_breakpoint_enabled(new_dr7, i)) {
iobpt |= HF_IOBPT_MASK;
Otherwise, hw_breakpoint_insert doesn't work because it expects to
see an updated env->dr[7].
There are a couple other issues that the tests expose, but they are
not regressions so I will send patches later.
Paolo
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions
2015-09-21 12:05 ` [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions Paolo Bonzini
@ 2015-09-21 14:05 ` Eduardo Habkost
2015-09-21 14:11 ` Paolo Bonzini
2015-09-28 18:26 ` Eduardo Habkost
2015-09-28 18:48 ` Eduardo Habkost
2 siblings, 1 reply; 20+ messages in thread
From: Eduardo Habkost @ 2015-09-21 14:05 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Richard Henderson
On Mon, Sep 21, 2015 at 02:05:52PM +0200, Paolo Bonzini wrote:
> On 15/09/2015 20:45, Richard Henderson wrote:
> > Best guess, since I can't find any code that actually uses them.
> > Linux actively turns them off at boot...
>
> I've sent a kvm-unit-tests patch to test debug extensions. It shows
> that debug extensions work, but the following needs to be squashed in
> patch 4:
>
> diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
> index c258598..b24e446 100644
> --- a/target-i386/bpt_helper.c
> +++ b/target-i386/bpt_helper.c
> @@ -134,14 +134,14 @@ void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7)
> int mod = ((old_dr7 | old_dr7 * 2) ^ (new_dr7 | new_dr7 * 2)) & 0xff;
>
> for (i = 0; i < DR7_MAX_BP; i++) {
> - if (mod & (2 << i * 2)) {
> - /* We know that register i has changed enable state;
> - recheck what that state should be and apply. */
> - if (hw_breakpoint_enabled(new_dr7, i)) {
> - iobpt |= hw_breakpoint_insert(env, i);
> - } else {
> - hw_breakpoint_remove(env, i);
> - }
> + if ((mod & (2 << i * 2)) && !hw_breakpoint_enabled(new_dr7, i)) {
> + hw_breakpoint_remove(env, i);
> + }
> + }
> + env->dr[7] = new_dr7 | DR7_FIXED_1;
> + for (i = 0; i < DR7_MAX_BP; i++) {
> + if (mod & (2 << i * 2) && hw_breakpoint_enabled(new_dr7, i)) {
> + iobpt |= hw_breakpoint_insert(env, i);
> } else if (hw_breakpoint_type(new_dr7, i) == DR7_TYPE_IO_RW
> && hw_breakpoint_enabled(new_dr7, i)) {
> iobpt |= HF_IOBPT_MASK;
Thanks! If that's the only change needed in the series, I can squash it
when applying. I just need your Signed-off-by line.
--
Eduardo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions
2015-09-21 14:05 ` Eduardo Habkost
@ 2015-09-21 14:11 ` Paolo Bonzini
0 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2015-09-21 14:11 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: qemu-devel, Richard Henderson
On 21/09/2015 16:05, Eduardo Habkost wrote:
> On Mon, Sep 21, 2015 at 02:05:52PM +0200, Paolo Bonzini wrote:
>> On 15/09/2015 20:45, Richard Henderson wrote:
>>> Best guess, since I can't find any code that actually uses them.
>>> Linux actively turns them off at boot...
>>
>> I've sent a kvm-unit-tests patch to test debug extensions. It shows
>> that debug extensions work, but the following needs to be squashed in
>> patch 4:
>>
>> diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
>> index c258598..b24e446 100644
>> --- a/target-i386/bpt_helper.c
>> +++ b/target-i386/bpt_helper.c
>> @@ -134,14 +134,14 @@ void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7)
>> int mod = ((old_dr7 | old_dr7 * 2) ^ (new_dr7 | new_dr7 * 2)) & 0xff;
>>
>> for (i = 0; i < DR7_MAX_BP; i++) {
>> - if (mod & (2 << i * 2)) {
>> - /* We know that register i has changed enable state;
>> - recheck what that state should be and apply. */
>> - if (hw_breakpoint_enabled(new_dr7, i)) {
>> - iobpt |= hw_breakpoint_insert(env, i);
>> - } else {
>> - hw_breakpoint_remove(env, i);
>> - }
>> + if ((mod & (2 << i * 2)) && !hw_breakpoint_enabled(new_dr7, i)) {
>> + hw_breakpoint_remove(env, i);
>> + }
>> + }
>> + env->dr[7] = new_dr7 | DR7_FIXED_1;
>> + for (i = 0; i < DR7_MAX_BP; i++) {
>> + if (mod & (2 << i * 2) && hw_breakpoint_enabled(new_dr7, i)) {
>> + iobpt |= hw_breakpoint_insert(env, i);
>> } else if (hw_breakpoint_type(new_dr7, i) == DR7_TYPE_IO_RW
>> && hw_breakpoint_enabled(new_dr7, i)) {
>> iobpt |= HF_IOBPT_MASK;
>
> Thanks! If that's the only change needed in the series, I can squash it
> when applying. I just need your Signed-off-by line.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
:)
but perhaps wait a little for Richard to chime in.
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions
2015-09-21 12:05 ` [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions Paolo Bonzini
2015-09-21 14:05 ` Eduardo Habkost
@ 2015-09-28 18:26 ` Eduardo Habkost
2015-09-28 18:48 ` Eduardo Habkost
2 siblings, 0 replies; 20+ messages in thread
From: Eduardo Habkost @ 2015-09-28 18:26 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Richard Henderson
On Mon, Sep 21, 2015 at 02:05:52PM +0200, Paolo Bonzini wrote:
>
>
> On 15/09/2015 20:45, Richard Henderson wrote:
> > Best guess, since I can't find any code that actually uses them.
> > Linux actively turns them off at boot...
>
> I've sent a kvm-unit-tests patch to test debug extensions. It shows
> that debug extensions work, but the following needs to be squashed in
> patch 4:
This can't be squashed directly into patch 4/8 because patch 7/8 touches
the same code, but I have solved the conflicts manually (the resulting
tree is exactly the same). I will send the conflict resolution diffs as
replies to patches 4/8 and 7/8.
>
> diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
> index c258598..b24e446 100644
> --- a/target-i386/bpt_helper.c
> +++ b/target-i386/bpt_helper.c
> @@ -134,14 +134,14 @@ void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7)
> int mod = ((old_dr7 | old_dr7 * 2) ^ (new_dr7 | new_dr7 * 2)) & 0xff;
>
> for (i = 0; i < DR7_MAX_BP; i++) {
> - if (mod & (2 << i * 2)) {
> - /* We know that register i has changed enable state;
> - recheck what that state should be and apply. */
> - if (hw_breakpoint_enabled(new_dr7, i)) {
> - iobpt |= hw_breakpoint_insert(env, i);
> - } else {
> - hw_breakpoint_remove(env, i);
> - }
> + if ((mod & (2 << i * 2)) && !hw_breakpoint_enabled(new_dr7, i)) {
> + hw_breakpoint_remove(env, i);
> + }
> + }
> + env->dr[7] = new_dr7 | DR7_FIXED_1;
> + for (i = 0; i < DR7_MAX_BP; i++) {
> + if (mod & (2 << i * 2) && hw_breakpoint_enabled(new_dr7, i)) {
> + iobpt |= hw_breakpoint_insert(env, i);
> } else if (hw_breakpoint_type(new_dr7, i) == DR7_TYPE_IO_RW
> && hw_breakpoint_enabled(new_dr7, i)) {
> iobpt |= HF_IOBPT_MASK;
>
> Otherwise, hw_breakpoint_insert doesn't work because it expects to
> see an updated env->dr[7].
>
> There are a couple other issues that the tests expose, but they are
> not regressions so I will send patches later.
>
> Paolo
>
--
Eduardo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions
2015-09-21 12:05 ` [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions Paolo Bonzini
2015-09-21 14:05 ` Eduardo Habkost
2015-09-28 18:26 ` Eduardo Habkost
@ 2015-09-28 18:48 ` Eduardo Habkost
2 siblings, 0 replies; 20+ messages in thread
From: Eduardo Habkost @ 2015-09-28 18:48 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Richard Henderson
On Mon, Sep 21, 2015 at 02:05:52PM +0200, Paolo Bonzini wrote:
> On 15/09/2015 20:45, Richard Henderson wrote:
> > Best guess, since I can't find any code that actually uses them.
> > Linux actively turns them off at boot...
>
> I've sent a kvm-unit-tests patch to test debug extensions. It shows
> that debug extensions work, but the following needs to be squashed in
> patch 4:
>
> diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
> index c258598..b24e446 100644
> --- a/target-i386/bpt_helper.c
> +++ b/target-i386/bpt_helper.c
> @@ -134,14 +134,14 @@ void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7)
(I've expanded the diff context below, to make review easier)
> /* If nothing is changing except the global/local enable bits,
> then we can make the change more efficient. */
> if (((old_dr7 ^ new_dr7) & ~0xff) == 0) {
> /* Fold the global and local enable bits together into the
> global fields, then xor to show which registers have
> changed collective enable state. */
> int mod = ((old_dr7 | old_dr7 * 2) ^ (new_dr7 | new_dr7 * 2)) & 0xff;
>
> for (i = 0; i < DR7_MAX_BP; i++) {
> - if (mod & (2 << i * 2)) {
> - /* We know that register i has changed enable state;
> - recheck what that state should be and apply. */
> - if (hw_breakpoint_enabled(new_dr7, i)) {
> - iobpt |= hw_breakpoint_insert(env, i);
> - } else {
> - hw_breakpoint_remove(env, i);
> - }
> + if ((mod & (2 << i * 2)) && !hw_breakpoint_enabled(new_dr7, i)) {
> + hw_breakpoint_remove(env, i);
> + }
> + }
> + env->dr[7] = new_dr7 | DR7_FIXED_1;
Why is DR7_FIXED_1 forced here, but not at the other branch below?
Also, the previous code didn't force DR7_FIXED_1, so shouldn't we implement
this fix in a separate patch instead of squashing it into patch 4/8?
> + for (i = 0; i < DR7_MAX_BP; i++) {
> + if (mod & (2 << i * 2) && hw_breakpoint_enabled(new_dr7, i)) {
> + iobpt |= hw_breakpoint_insert(env, i);
> } else if (hw_breakpoint_type(new_dr7, i) == DR7_TYPE_IO_RW
> && hw_breakpoint_enabled(new_dr7, i)) {
> iobpt |= HF_IOBPT_MASK;
> }
> }
> } else {
> for (i = 0; i < DR7_MAX_BP; i++) {
> hw_breakpoint_remove(env, i);
> }
> env->dr[7] = new_dr7;
> for (i = 0; i < DR7_MAX_BP; i++) {
> iobpt |= hw_breakpoint_insert(env, i);
> }
> }
--
Eduardo
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2015-09-28 18:48 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-15 18:45 [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions Richard Henderson
2015-09-15 18:45 ` [Qemu-devel] [PATCH 1/8] target-i386: Move breakpoint related functions to new file Richard Henderson
2015-09-18 18:27 ` Eduardo Habkost
2015-09-15 18:45 ` [Qemu-devel] [PATCH 2/8] target-i386: Make check_hw_breakpoints static Richard Henderson
2015-09-18 18:29 ` Eduardo Habkost
2015-09-15 18:45 ` [Qemu-devel] [PATCH 3/8] target-i386: Introduce cpu_x86_update_dr7 Richard Henderson
2015-09-15 18:45 ` [Qemu-devel] [PATCH 4/8] target-i386: Re-introduce optimal breakpoint removal Richard Henderson
2015-09-16 8:57 ` Paolo Bonzini
2015-09-16 14:57 ` Richard Henderson
2015-09-16 14:59 ` Paolo Bonzini
2015-09-18 18:38 ` Eduardo Habkost
2015-09-15 18:45 ` [Qemu-devel] [PATCH 5/8] target-i386: Move hw_*breakpoint_* functions Richard Henderson
2015-09-15 18:45 ` [Qemu-devel] [PATCH 6/8] target-i386: Optimize setting dr[0-3] Richard Henderson
2015-09-15 18:45 ` [Qemu-devel] [PATCH 7/8] target-i386: Handle I/O breakpoints Richard Henderson
2015-09-15 18:45 ` [Qemu-devel] [PATCH 8/8] target-i386: Check CR4[DE] for processing DR4/DR5 Richard Henderson
2015-09-21 12:05 ` [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions Paolo Bonzini
2015-09-21 14:05 ` Eduardo Habkost
2015-09-21 14:11 ` Paolo Bonzini
2015-09-28 18:26 ` Eduardo Habkost
2015-09-28 18:48 ` Eduardo Habkost
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).