diff mbox

[v4] target-arm: Break the TB after ISB to execute self-modified code correctly

Message ID 1443789482-13729-1-git-send-email-afarallax@yandex.ru
State New
Headers show

Commit Message

Sergey Sorokin Oct. 2, 2015, 12:38 p.m. UTC 
If any store instruction writes the code inside the same TB
after this store insn, the execution of the TB must be stopped
to execute new code correctly.
As described in ARMv8 manual D3.4.6 a self-modified code need to do
IC invalidation to be valid, and ISB after it. So it's enough to end the TB
after ISB instruction on the code translation.
Also this TB break is necessary to take any pending interrupts immediately
according to ARMv8 ARM D1.14.4.

Signed-off-by: Sergey Sorokin <afarallax@yandex.ru>
---
Changes since previous version:
* ARMv6 ISB was also fixed.
* Second reason for TB breaking was mentioned in comments
and the commit message.
* A compilation error was fixed.

 target-arm/helper.c        |  6 +++++-
 target-arm/translate-a64.c |  8 +++++++-
 target-arm/translate.c     | 17 +++++++++++++++--
 3 files changed, 27 insertions(+), 4 deletions(-)

Comments

Peter Maydell Oct. 4, 2015, 7:20 p.m. UTC | #1 
On 2 October 2015 at 13:38, Sergey Sorokin <afarallax@yandex.ru> wrote:
> If any store instruction writes the code inside the same TB
> after this store insn, the execution of the TB must be stopped
> to execute new code correctly.
> As described in ARMv8 manual D3.4.6 a self-modified code need to do
> IC invalidation to be valid, and ISB after it. So it's enough to end the TB
> after ISB instruction on the code translation.
> Also this TB break is necessary to take any pending interrupts immediately
> according to ARMv8 ARM D1.14.4.
>
> Signed-off-by: Sergey Sorokin <afarallax@yandex.ru>
> ---
> Changes since previous version:
> * ARMv6 ISB was also fixed.
> * Second reason for TB breaking was mentioned in comments
> and the commit message.
> * A compilation error was fixed.
>
>  target-arm/helper.c        |  6 +++++-
>  target-arm/translate-a64.c |  8 +++++++-
>  target-arm/translate.c     | 17 +++++++++++++++--
>  3 files changed, 27 insertions(+), 4 deletions(-)



Applied to target-arm.next, thanks.

-- PMM
diff mbox

Patch

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 8367997..8259d1e 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -657,8 +657,12 @@  static const ARMCPRegInfo v6_cp_reginfo[] = {
     { .name = "MVA_prefetch",
       .cp = 15, .crn = 7, .crm = 13, .opc1 = 0, .opc2 = 1,
       .access = PL1_W, .type = ARM_CP_NOP },
+    /* We need to break the TB after ISB to execute a self-modified code
+     * correctly and also to take any pending interrupts immediately.
+     * So use arm_cp_write_ignore() function instead of ARM_CP_NOP flag.
+     */
     { .name = "ISB", .cp = 15, .crn = 7, .crm = 5, .opc1 = 0, .opc2 = 4,
-      .access = PL0_W, .type = ARM_CP_NOP },
+      .access = PL0_W, .type = ARM_CP_NO_RAW, .writefn = arm_cp_write_ignore },
     { .name = "DSB", .cp = 15, .crn = 7, .crm = 10, .opc1 = 0, .opc2 = 4,
       .access = PL0_W, .type = ARM_CP_NOP },
     { .name = "DMB", .cp = 15, .crn = 7, .crm = 10, .opc1 = 0, .opc2 = 5,
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index ec0936c..a407b45 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1230,9 +1230,15 @@  static void handle_sync(DisasContext *s, uint32_t insn,
         return;
     case 4: /* DSB */
     case 5: /* DMB */
-    case 6: /* ISB */
         /* We don't emulate caches so barriers are no-ops */
         return;
+    case 6: /* ISB */
+        /* We need to break the TB after this insn to execute
+         * a self-modified code correctly and also to take
+         * any pending interrupts immediately.
+         */
+        s->is_jmp = DISAS_UPDATE;
+        return;
     default:
         unallocated_encoding(s);
         return;
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 84a21ac..28b9729 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7721,10 +7721,16 @@  static void disas_arm_insn(DisasContext *s, unsigned int insn)
                 return;
             case 4: /* dsb */
             case 5: /* dmb */
-            case 6: /* isb */
                 ARCH(7);
                 /* We don't emulate caches so these are a no-op.  */
                 return;
+            case 6: /* isb */
+                /* We need to break the TB after this insn to execute
+                 * a self-modified code correctly and also to take
+                 * any pending interrupts immediately.
+                 */
+                gen_lookup_tb(s);
+                return;
             default:
                 goto illegal_op;
             }
@@ -10031,9 +10037,16 @@  static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
                             break;
                         case 4: /* dsb */
                         case 5: /* dmb */
-                        case 6: /* isb */
                             /* These execute as NOPs.  */
                             break;
+                        case 6: /* isb */
+                            /* We need to break the TB after this insn
+                             * to execute a self-modified code correctly
+                             * and also to take any pending interrupts
+                             * immediately.
+                             */
+                            gen_lookup_tb(s);
+                            break;
                         default:
                             goto illegal_op;
                         }