[Qemu-devel] [PATCH] target-arm: Fix and improve AA32 singlestep translation completion code

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

The AArch32 translation completion code for singlestep enabled/active
case was a way more confusing and too repetitive then it needs to be.
Probably that was the cause for a bug to be introduced into it at some
point. The bug was that SWI/HVC/SMC exception would be generated in
condition-failed instruction code path whereas it shouldn't.

This patch rewrites the code in a way similar to the non-singlestep
case.

In the condition-passed/unconditional instruction code path we need to:
 - Write the condexec bits back to the CPU state
 - Advance the singlestep state machine and generate a corresponding
   exception in case of SWI/HVC/SMC
 - Write the PC back to the CPU state if it hasn't already been written
   and generate an appropriate singlestep exception otherwise

In the condition-failed instruction code path we need to:
 - Set a TCG label to jump to it if the condition is failed
 - Write the condexec bits back to the CPU state
 - Write the PC back to the CPU state since it hasn't been written in
   this case
 - Generate an appropriate singlestep exception

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
---
 target-arm/translate.c | 65 ++++++++++++++++++++++++--------------------------
 1 file changed, 31 insertions(+), 34 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 5d22879..c9eeb09 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -11480,48 +11480,45 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb)
        instruction was a conditional branch or trap, and the PC has
        already been written.  */
     if (unlikely(cs->singlestep_enabled || dc->ss_active)) {
-        /* Make sure the pc is updated, and raise a debug exception.  */
-        if (dc->condjmp) {
-            gen_set_condexec(dc);
-            if (dc->is_jmp == DISAS_SWI) {
-                gen_ss_advance(dc);
-                gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb),
-                              default_exception_el(dc));
-            } else if (dc->is_jmp == DISAS_HVC) {
-                gen_ss_advance(dc);
-                gen_exception(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2);
-            } else if (dc->is_jmp == DISAS_SMC) {
-                gen_ss_advance(dc);
-                gen_exception(EXCP_SMC, syn_aa32_smc(), 3);
-            } else if (dc->ss_active) {
-                gen_step_complete_exception(dc);
-            } else {
-                gen_exception_internal(EXCP_DEBUG);
-            }
-            gen_set_label(dc->condlabel);
-        }
-        if (dc->condjmp || dc->is_jmp == DISAS_NEXT ||
-            dc->is_jmp == DISAS_UPDATE) {
-            gen_set_pc_im(dc, dc->pc);
-            dc->condjmp = 0;
-        }
+        /* Unconditional and "condition passed" instruction codepath. */
         gen_set_condexec(dc);
-        if (dc->is_jmp == DISAS_SWI && !dc->condjmp) {
+        switch (dc->is_jmp) {
+        case DISAS_SWI:
             gen_ss_advance(dc);
             gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb),
                           default_exception_el(dc));
-        } else if (dc->is_jmp == DISAS_HVC && !dc->condjmp) {
+            break;
+        case DISAS_HVC:
             gen_ss_advance(dc);
             gen_exception(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2);
-        } else if (dc->is_jmp == DISAS_SMC && !dc->condjmp) {
+            break;
+        case DISAS_SMC:
             gen_ss_advance(dc);
             gen_exception(EXCP_SMC, syn_aa32_smc(), 3);
-        } else if (dc->ss_active) {
-            gen_step_complete_exception(dc);
-        } else {
-            /* FIXME: Single stepping a WFI insn will not halt
-               the CPU.  */
-            gen_exception_internal(EXCP_DEBUG);
+            break;
+        case DISAS_NEXT:
+        case DISAS_UPDATE:
+            gen_set_pc_im(dc, dc->pc);
+            /* fall through */
+        default:
+            if (dc->ss_active) {
+                gen_step_complete_exception(dc);
+            } else {
+                /* FIXME: Single stepping a WFI insn will not halt
+                   the CPU.  */
+                gen_exception_internal(EXCP_DEBUG);
+            }
+        }
+        if (dc->condjmp) {
+            /* "Condition failed" instruction codepath. */
+            gen_set_label(dc->condlabel);
+            gen_set_condexec(dc);
+            gen_set_pc_im(dc, dc->pc);
+            if (dc->ss_active) {
+                gen_step_complete_exception(dc);
+            } else {
+                gen_exception_internal(EXCP_DEBUG);
+            }
         }
     } else {
         /* While branches must always occur at the end of an IT block,
-- 
1.9.1