[Qemu-devel] [PATCH v2 2/2] target-arm: Fix CPU breakpoint handling

[Sergey Fedorov <serge.fdrv@gmail.com>]

A QEMU breakpoint match is not definitely an architectural breakpoint
match. If an exception is generated unconditionally during translation,
it is hardly possible to ignore it in the debug exception handler.

Generate a call to a helper to check CPU breakpoints and raise an
exception only if any breakpoint matches architecturally.

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
---
 target-arm/helper.h        |  2 ++
 target-arm/op_helper.c     | 29 ++++++++++++++++++-----------
 target-arm/translate-a64.c | 14 ++++++++------
 target-arm/translate.c     | 13 ++++++++-----
 4 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/target-arm/helper.h b/target-arm/helper.h
index 827b33d..c2a85c7 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -54,6 +54,8 @@ DEF_HELPER_1(yield, void, env)
 DEF_HELPER_1(pre_hvc, void, env)
 DEF_HELPER_2(pre_smc, void, env, i32)
 
+DEF_HELPER_1(check_breakpoints, void, env)
+
 DEF_HELPER_3(cpsr_write, void, env, i32, i32)
 DEF_HELPER_1(cpsr_read, i32, env)
 
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 1d4d8cb..8ec8590 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -867,6 +867,15 @@ static bool check_breakpoints(ARMCPU *cpu)
     return false;
 }
 
+void HELPER(check_breakpoints)(CPUARMState *env)
+{
+    ARMCPU *cpu = arm_env_get_cpu(env);
+
+    if (check_breakpoints(cpu)) {
+        HELPER(exception_internal(env, EXCP_DEBUG));
+    }
+}
+
 void arm_debug_excp_handler(CPUState *cs)
 {
     /* Called by core code when a watchpoint or breakpoint fires;
@@ -899,6 +908,7 @@ void arm_debug_excp_handler(CPUState *cs)
     } else {
         CPUBreakpoint *bp;
         uint64_t pc = is_a64(env) ? env->pc : env->regs[15];
+        bool same_el = (arm_debug_target_el(env) == arm_current_el(env));
 
         QTAILQ_FOREACH(bp, &cs->breakpoints, entry) {
             if (bp->pc == pc && !(bp->flags & BP_CPU)) {
@@ -906,18 +916,15 @@ void arm_debug_excp_handler(CPUState *cs)
             }
         }
 
-        if (check_breakpoints(cpu)) {
-            bool same_el = (arm_debug_target_el(env) == arm_current_el(env));
-            if (extended_addresses_enabled(env)) {
-                env->exception.fsr = (1 << 9) | 0x22;
-            } else {
-                env->exception.fsr = 0x2;
-            }
-            /* FAR is UNKNOWN, so doesn't need setting */
-            raise_exception(env, EXCP_PREFETCH_ABORT,
-                            syn_breakpoint(same_el),
-                            arm_debug_target_el(env));
+        if (extended_addresses_enabled(env)) {
+            env->exception.fsr = (1 << 9) | 0x22;
+        } else {
+            env->exception.fsr = 0x2;
         }
+        /* FAR is UNKNOWN, so doesn't need setting */
+        raise_exception(env, EXCP_PREFETCH_ABORT,
+                        syn_breakpoint(same_el),
+                        arm_debug_target_el(env));
     }
 }
 
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index ec0936c..426229f 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -11082,11 +11082,14 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
         if (unlikely(!QTAILQ_EMPTY(&cs->breakpoints))) {
             QTAILQ_FOREACH(bp, &cs->breakpoints, entry) {
                 if (bp->pc == dc->pc) {
-                    gen_exception_internal_insn(dc, 0, EXCP_DEBUG);
-                    /* Advance PC so that clearing the breakpoint will
-                       invalidate this TB.  */
-                    dc->pc += 2;
-                    goto done_generating;
+                    if (bp->flags & BP_CPU) {
+                        gen_helper_check_breakpoints(cpu_env);
+                    } else {
+                        gen_exception_internal_insn(dc, 0, EXCP_DEBUG);
+                    }
+                    /* End the TB early; it's likely not going to be executed */
+                    dc->is_jmp = DISAS_UPDATE;
+                    break;
                 }
             }
         }
@@ -11209,7 +11212,6 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
         }
     }
 
-done_generating:
     gen_tb_end(tb, num_insns);
 
 #ifdef DEBUG_DISAS
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 84a21ac..405d6d0 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -11328,11 +11328,14 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
         if (unlikely(!QTAILQ_EMPTY(&cs->breakpoints))) {
             QTAILQ_FOREACH(bp, &cs->breakpoints, entry) {
                 if (bp->pc == dc->pc) {
-                    gen_exception_internal_insn(dc, 0, EXCP_DEBUG);
-                    /* Advance PC so that clearing the breakpoint will
-                       invalidate this TB.  */
-                    dc->pc += 2;
-                    goto done_generating;
+                    if (bp->flags & BP_CPU) {
+                        gen_helper_check_breakpoints(cpu_env);
+                    } else {
+                        gen_exception_internal_insn(dc, 0, EXCP_DEBUG);
+                    }
+                    /* End the TB early; it's likely not going to be executed */
+                    dc->is_jmp = DISAS_UPDATE;
+                    break;
                 }
             }
         }
-- 
1.9.1