[Qemu-devel] [FOR 0.12][PATCH] target-i386: Fix evaluation of DR7 register

[Qemu-devel] [FOR 0.12][PATCH] target-i386: Fix evaluation of DR7 register

[Jan Kiszka]

hw_breakpoint_type and hw_breakpoint_len used the wrong index multiplier
to extract type and len.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 target-i386/cpu.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 9ef1be4..e835f23 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -834,12 +834,12 @@ static inline int hw_breakpoint_enabled(unsigned long dr7, int index)
 
 static inline int hw_breakpoint_type(unsigned long dr7, int index)
 {
-    return (dr7 >> (DR7_TYPE_SHIFT + (index * 2))) & 3;
+    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 * 2))) & 3);
+    int len = ((dr7 >> (DR7_LEN_SHIFT + (index * 4))) & 3);
     return (len == 2) ? 8 : len + 1;
 }
 

Re: [Qemu-devel] [FOR 0.12][PATCH] target-i386: Fix evaluation of DR7 register

[TeLeMan]

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

raise_exception() in op_helper.c should not be used outside
op_helper.c because of tcg. This patch uses raise_exception2() instead
of raise_exception().

[-- Attachment #2: target-i386-fix-raise_exception-for-hw-breakpoints.patch --]
[-- Type: application/octet-stream, Size: 1896 bytes --]

From: TeLeMan <geleman@gmail.com>
Subject: [PATCH] target-i386: fix raise_exception for hw breakpoints

---
 target-i386/helper.c    |    6 +++---
 target-i386/op_helper.c |    6 ++++++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index da36a01..4ee7c4a 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1590,7 +1590,7 @@ int check_hw_breakpoints(CPUState *env, int force_dr6_update)
 
 static CPUDebugExcpHandler *prev_debug_excp_handler;
 
-void raise_exception(int exception_index);
+void raise_exception2(CPUState *env1, int exception_index);
 
 static void breakpoint_handler(CPUState *env)
 {
@@ -1600,7 +1600,7 @@ static void breakpoint_handler(CPUState *env)
         if (env->watchpoint_hit->flags & BP_CPU) {
             env->watchpoint_hit = NULL;
             if (check_hw_breakpoints(env, 0))
-                raise_exception(EXCP01_DB);
+                raise_exception2(env, EXCP01_DB);
             else
                 cpu_resume_from_signal(env, NULL);
         }
@@ -1609,7 +1609,7 @@ static void breakpoint_handler(CPUState *env)
             if (bp->pc == env->eip) {
                 if (bp->flags & BP_CPU) {
                     check_hw_breakpoints(env, 1);
-                    raise_exception(EXCP01_DB);
+                    raise_exception2(env, EXCP01_DB);
                 }
                 break;
             }
diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
index 4f3ed57..f2e2f6d 100644
--- a/target-i386/op_helper.c
+++ b/target-i386/op_helper.c
@@ -1351,6 +1351,12 @@ void raise_exception(int exception_index)
     raise_interrupt(exception_index, 0, 0, 0);
 }
 
+void raise_exception2(CPUState *env1, int exception_index)
+{
+    env = env1;
+    raise_exception(exception_index);
+}
+
 /* SMM support */
 
 #if defined(CONFIG_USER_ONLY)
-- 
1.6.5.1.1367.gcd48

[Qemu-devel] Re: [FOR 0.12][PATCH] target-i386: Fix evaluation of DR7 register

[Jan Kiszka]

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

TeLeMan wrote:
> raise_exception() in op_helper.c should not be used outside
> op_helper.c because of tcg. This patch uses raise_exception2() instead
> of raise_exception().

Please do not post new patches as reply to others if they are not
directly related. Please do not attach patches and do not use base64
encoding.

Now to the content: env already equals single_cpu_env, so there is no
need in writing it back. That's because break/watchpoints are triggered
synchronously over the context in which they may raise an exception as
result.

Jan


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

[Qemu-devel] Re: [FOR 0.12][PATCH] target-i386: Fix evaluation of DR7 register

[TeLeMan]

> Now to the content: env already equals single_cpu_env, so there is no
> need in writing it back. That's because break/watchpoints are triggered
> synchronously over the context in which they may raise an exception as
> result.
env in op_help.c is asm(AREG0) not single_cpu_env.
Did you test your codes?

[Qemu-devel] Re: [FOR 0.12][PATCH] target-i386: Fix evaluation of DR7 register

[Jan Kiszka]

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

TeLeMan wrote:
>> Now to the content: env already equals single_cpu_env, so there is no
>> need in writing it back. That's because break/watchpoints are triggered
>> synchronously over the context in which they may raise an exception as
>> result.
> env in op_help.c is asm(AREG0) not single_cpu_env.

Which makes no difference due to the synchronous characteristics. The
point where env and single_cpu_env diverges while a vcpu is running is
the beginning of a bug.

> Did you test your codes?

I'm using it, including SMP.

There is still a bug, but it's unrelated to the context. It's related to
watchpoints triggering in helper function instead of generated code.
Will fix that later.

Jan


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