[Qemu-devel] [RFC 14/14] bbvec: Properly detect conditional thumb2 branching instructions

[Christopher Covington <cov@codeaurora.org>]

Add bbvec detection of thumb2 branch instructions via a stripped-down
version of the thumb2 instruction decoding logic. Unfortunately, this
code still called into disas_neon_ls_insn(), which is apparently not
free from side effects. Therefore, calling into this function twice
(once in the added bbvec thumb2 branch detection code and once in the
original decoding code) causes QEMU to report segfaults and illegal
instruction exceptions to the guest kernel (possibly among other
undesirable, still-undiscovered behavior). Because neon instructions
are not allowed to write to the PC, eliminating the call to
disas_neon_ls_insn() altogether is safe.

Written by Aaron Lindsay.

Signed-off-by: Christopher Covington <cov@codeaurora.org>
---
 target-arm/translate.c | 264 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 261 insertions(+), 3 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index f9d69ef..8e9b97b 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -9411,6 +9411,260 @@ gen_thumb2_data_op(DisasContext *s, int op, int conds, uint32_t shifter_out,
     return 0;
 }
 
+#ifdef CONFIG_BBVEC
+/* Call gen_store_is_jmp(1) if the instruction is a thumb2 branch instruction.
+ * This is called by disas_thumb_insn. The switch/case/if structures are a
+ * simplified copy of the logic in disas_thumb2_insn */
+static void thumb2_branch_detection(CPUARMState *env, DisasContext *s, uint16_t insn_hw1)
+{
+    uint32_t insn, shift, rd, rn, rs;
+    int op;
+
+    if (!(arm_dc_feature(s, ARM_FEATURE_THUMB2)
+          || arm_dc_feature(s, ARM_FEATURE_M))) {
+        /* Thumb-1 cores may need to treat bl and blx as a pair of
+           16-bit instructions to get correct prefetch abort behavior.  */
+        insn = insn_hw1;
+        if ((insn & (1 << 12)) == 0) {
+            ARCH(5);
+            gen_store_is_jmp(1);
+            return;
+        }
+        if (insn & (1 << 11)) {
+            gen_store_is_jmp(1);
+            return;
+        }
+        if ((s->pc & ~TARGET_PAGE_MASK) == 0) {
+            return;
+        }
+        /* Fall through to 32-bit decode.  */
+    }
+
+    insn = arm_lduw_code(env, s->pc, s->bswap_code);
+    insn |= (uint32_t)insn_hw1 << 16;
+
+    if ((insn & 0xf800e800) != 0xf000e800) {
+        ARCH(6T2);
+    }
+
+    rn = (insn >> 16) & 0xf;
+    rs = (insn >> 12) & 0xf;
+    rd = (insn >> 8) & 0xf;
+    switch ((insn >> 25) & 0xf) {
+    case 0: case 1: case 2: case 3:
+        /* 16-bit instructions.  Should never happen.  */
+        return;
+    case 4:
+        if (insn & (1 << 22)) {
+            /* Other load/store, table branch.  */
+            if (insn & 0x01200000) {
+                /* Load/store doubleword.  */
+                if (insn & (1 << 20)) {
+                    /* ldrd */
+                    if (rs == 15 || rd == 15)
+                        gen_store_is_jmp(1);
+                }
+            } else if ((insn & (1 << 23)) == 0) {
+                /* Load/store exclusive word.  */
+            } else if ((insn & (7 << 5)) == 0) {
+                /* Table Branch.  */
+                gen_store_is_jmp(1);
+            } else {
+                int op2 = (insn >> 6) & 0x3;
+                op = (insn >> 4) & 0x3;
+                switch (op2) {
+                case 0:
+                    return;
+                case 1:
+                    /* Load/store exclusive byte/halfword/doubleword */
+                    if (op == 2) {
+                        return;
+                    }
+                    ARCH(7);
+                    break;
+                case 2:
+                    /* Load-acquire/store-release */
+                    if (op == 3) {
+                        return;
+                    }
+                    /* Fall through */
+                case 3:
+                    /* Load-acquire/store-release exclusive */
+                    ARCH(8);
+                    break;
+                }
+                if (!(op2 & 1)) {
+                    if (insn & (1 << 20)) {
+                        if (op >= 0 && op <= 2 && rs == 15)
+                            gen_store_is_jmp(1);
+                    }
+                } else if (insn & (1 << 20)) {
+                    if (rs == 15 || rd == 15)
+                        gen_store_is_jmp(1);
+                }
+            }
+        } else {
+            /* Load/store multiple, RFE, SRS.  */
+            if (((insn >> 23) & 1) == ((insn >> 24) & 1)) {
+                /* RFE, SRS: not available in user mode or on M profile */
+                if (IS_USER(s) || arm_dc_feature(s, ARM_FEATURE_M)) {
+                    goto illegal_op;
+                }
+                if (insn & (1 << 20)) {
+                    /* rfe */
+                    if (insn & (1 << 21)) {
+                        /* Base writeback.  */
+                        if (rn == 15)
+                            gen_store_is_jmp(1);
+                    }
+                }
+            } else {
+                int i, loaded_base = 0;
+                /* Load/store multiple.  */
+                for (i = 0; i < 16; i++) {
+                    if ((insn & (1 << i)) == 0)
+                        continue;
+                    if (insn & (1 << 20)) {
+                        /* Load.  */
+                        if (i == 15) {
+                            gen_store_is_jmp(1);
+                        } else if (i == rn) {
+                            loaded_base = 1;
+                        } else {
+                            if (i == 15)
+                                gen_store_is_jmp(1);
+                        }
+                    }
+                }
+                if (loaded_base) {
+                    if (rn == 15)
+                        gen_store_is_jmp(1);
+                }
+                if (insn & (1 << 21)) {
+                    /* Base register writeback.  */
+                    /* Fault if writeback register is in register list.  */
+                    if (insn & (1 << rn))
+                        goto illegal_op;
+                    if (rn == 15)
+                        gen_store_is_jmp(1);
+                }
+            }
+        }
+        break;
+    case 5:
+        op = (insn >> 21) & 0xf;
+        if (op == 6) {
+            if (rd == 15)
+                gen_store_is_jmp(1);
+        }
+        break;
+    case 13: /* Misc data processing.  */
+        op = ((insn >> 22) & 6) | ((insn >> 7) & 1);
+        if (op < 4 && (insn & 0xf000) != 0xf000)
+            goto illegal_op;
+        switch (op) {
+        case 0: /* Register controlled shift.  */
+            if ((insn & 0x70) != 0)
+                goto illegal_op;
+            if (rd == 15)
+                gen_store_is_jmp(1);
+            break;
+        case 1: /* Sign/zero extend.  */
+            op = (insn >> 20) & 7;
+            if (op < 0 || op > 5)
+                return;
+            if (rd == 15)
+                gen_store_is_jmp(1);
+            break;
+        case 2: /* SIMD add/subtract.  */
+            op = (insn >> 20) & 7;
+            shift = (insn >> 4) & 7;
+            if ((op & 3) == 3 || (shift & 3) == 3)
+                goto illegal_op;
+        case 3: /* Other data processing.  */
+        case 4: case 5: /* 32-bit multiply.  Sum of absolute differences.  */
+            if (rd == 15)
+                gen_store_is_jmp(1);
+            break;
+        case 6: case 7: /* 64-bit multiply, Divide.  */
+            op = ((insn >> 4) & 0xf) | ((insn >> 16) & 0x70);
+            if ((op & 0x50) == 0x10) {
+                /* sdiv, udiv */
+                if (!arm_dc_feature(s, ARM_FEATURE_THUMB_DIV)) {
+                    goto illegal_op;
+                }
+
+                if (rd == 15)
+                    gen_store_is_jmp(1);
+            }
+            break;
+        }
+        break;
+    case 8: case 9: case 10: case 11:
+        if (insn & (1 << 15)) {
+            /* Branches, misc control.  */
+            if (insn & 0x5000) {
+                /* Unconditional branch.  */
+                gen_store_is_jmp(1);
+            } else if (((insn >> 23) & 7) == 7) {
+                /* Misc control */
+                if (insn & (1 << 13))
+                    goto illegal_op;
+
+                if ((insn & (1 << 26)) == 0) {
+                    op = (insn >> 20) & 7;
+                    if (op == 4) /* bxj */
+                        gen_store_is_jmp(1);
+                }
+            } else {
+                /* Conditional branch.  */
+                gen_store_is_jmp(1);
+            }
+        } else {
+            /* Data processing immediate.  */
+            if (insn & (1 << 25)) {
+                if (insn & (1 << 24)) {
+                    if (insn & (1 << 20))
+                        goto illegal_op;
+                    /* Bitfield/Saturate.  */
+                    if (rd == 15)
+                        gen_store_is_jmp(1);
+                } else {
+                    if (rd == 15)
+                        gen_store_is_jmp(1);
+                }
+            }
+        }
+        break;
+    case 12: /* Load/store single data item.  */
+        {
+        if ((insn & 0x01100000) == 0x01000000)
+            break; // Ignore neon instructions, they should not update the PC
+        op = ((insn >> 21) & 3) | ((insn >> 22) & 4);
+        if (insn & (1 << 20)) {
+            /* Load.  */
+            switch (op) {
+            case 0:
+            case 4:
+            case 1:
+            case 5:
+            case 2:
+                break;
+            default:
+                goto illegal_op;
+            }
+            if (rs == 15) {
+                gen_store_is_jmp(1);
+            }
+        }
+        }
+        break;
+    }
+illegal_op:
+    return;
+}
+#endif // CONFIG_BBVEC
+
 /* Translate a 32-bit thumb instruction.  Returns nonzero if the instruction
    is not legal.  */
 static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw1)
@@ -10763,11 +11017,15 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
                         gen_store_is_jmp(1);
                     break;
                 case 14:
-                    /* branch instruction */
-                    if (!(insn & (1 << 11)))
+                    if (insn & (1 << 11)) {
+                        thumb2_branch_detection(env, s, insn);
+                    } else {
+                        /* branch instruction */
                         gen_store_is_jmp(1);
+                    }
                     break;
-                default:
+                case 15:
+                    thumb2_branch_detection(env, s, insn);
                     break;
                 }
             }
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project