[Qemu-devel] [PATCH v2 0/8] target-arm: Translate based on TB flags, not CPUState

[Qemu-devel] [PATCH v2 0/8] target-arm: Translate based on TB flags, not CPUState

[Peter Maydell]

This patchset corrects a number of places in the ARM translation code
which were generating code which was dependent on values in the CPUState
structure which might change at runtime. This is a bad idea for two
reasons. Firstly, we might try to reuse the generated code later when
the assumptions baked into the generated code were no longer valid.
Secondly, we might try to retranslate the same TB (eg when an exception
results in our calling cpu_restore_state()) but get different generated
code, which could result in qemu crashing.

Bug https://bugs.launchpad.net/bugs/604872 is a particular example
of the latter case involving the IT bits; this patchset fixes that bug.

I believe that this patchset deals with all the problems. Remaining
CPUState fields referred to in translate.c are either constant after
system init or trigger flushing of affected TBs when they are changed.

Differences from v1: I've added some macros for the TB flags bitfields,
as suggested by Aurelien.

Peter Maydell (8):
  target-arm: Don't generate code specific to current CPU mode for SRS
  target-arm: Add symbolic constants for bitfields in TB flags
  target-arm: Translate with VFP-enabled from TB flags, not CPUState
  target-arm: Translate with VFP len/stride from TB flags, not CPUState
  target-arm: Translate with Thumb state from TB flags, not CPUState
  target-arm: Translate with condexec bits from TB flags, not CPUState
  target-arm: Set privileged bit in TB flags correctly for M profile
  target-arm: Translate with user-state from TB flags, not CPUState

 target-arm/cpu.h       |   51 ++++++++++++++++++++++++---
 target-arm/helper.c    |   12 +++++-
 target-arm/translate.c |   88 ++++++++++++++++++-----------------------------
 3 files changed, 89 insertions(+), 62 deletions(-)

[Qemu-devel] [PATCH 1/8] target-arm: Don't generate code specific to current CPU mode for SRS

[Peter Maydell]

When translating the SRS instruction, handle the "store registers
to stack of current mode" case in the helper function rather than
inline. This means the generated code does not make assumptions
about the current CPU mode which might not be valid when the TB
is executed later.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper.c    |   12 ++++++++++--
 target-arm/translate.c |   46 +++++++++++++++-------------------------------
 2 files changed, 25 insertions(+), 33 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 705b99f..f08e09e 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1846,12 +1846,20 @@ bad_reg:
 
 void HELPER(set_r13_banked)(CPUState *env, uint32_t mode, uint32_t val)
 {
-    env->banked_r13[bank_number(mode)] = val;
+    if ((env->uncached_cpsr & CPSR_M) == mode) {
+        env->regs[13] = val;
+    } else {
+        env->banked_r13[bank_number(mode)] = val;
+    }
 }
 
 uint32_t HELPER(get_r13_banked)(CPUState *env, uint32_t mode)
 {
-    return env->banked_r13[bank_number(mode)];
+    if ((env->uncached_cpsr & CPSR_M) == mode) {
+        return env->regs[13];
+    } else {
+        return env->banked_r13[bank_number(mode)];
+    }
 }
 
 uint32_t HELPER(v7m_mrs)(CPUState *env, uint32_t reg)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 2ce82f3..c391398 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -6101,14 +6101,10 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
                 goto illegal_op;
             ARCH(6);
             op1 = (insn & 0x1f);
-            if (op1 == (env->uncached_cpsr & CPSR_M)) {
-                addr = load_reg(s, 13);
-            } else {
-                addr = new_tmp();
-                tmp = tcg_const_i32(op1);
-                gen_helper_get_r13_banked(addr, cpu_env, tmp);
-                tcg_temp_free_i32(tmp);
-            }
+            addr = new_tmp();
+            tmp = tcg_const_i32(op1);
+            gen_helper_get_r13_banked(addr, cpu_env, tmp);
+            tcg_temp_free_i32(tmp);
             i = (insn >> 23) & 3;
             switch (i) {
             case 0: offset = -4; break; /* DA */
@@ -6135,14 +6131,10 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
                 }
                 if (offset)
                     tcg_gen_addi_i32(addr, addr, offset);
-                if (op1 == (env->uncached_cpsr & CPSR_M)) {
-                    store_reg(s, 13, addr);
-                } else {
-                    tmp = tcg_const_i32(op1);
-                    gen_helper_set_r13_banked(cpu_env, tmp, addr);
-                    tcg_temp_free_i32(tmp);
-                    dead_tmp(addr);
-                }
+                tmp = tcg_const_i32(op1);
+                gen_helper_set_r13_banked(cpu_env, tmp, addr);
+                tcg_temp_free_i32(tmp);
+                dead_tmp(addr);
             } else {
                 dead_tmp(addr);
             }
@@ -7554,14 +7546,10 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                 } else {
                     /* srs */
                     op = (insn & 0x1f);
-                    if (op == (env->uncached_cpsr & CPSR_M)) {
-                        addr = load_reg(s, 13);
-                    } else {
-                        addr = new_tmp();
-                        tmp = tcg_const_i32(op);
-                        gen_helper_get_r13_banked(addr, cpu_env, tmp);
-                        tcg_temp_free_i32(tmp);
-                    }
+                    addr = new_tmp();
+                    tmp = tcg_const_i32(op);
+                    gen_helper_get_r13_banked(addr, cpu_env, tmp);
+                    tcg_temp_free_i32(tmp);
                     if ((insn & (1 << 24)) == 0) {
                         tcg_gen_addi_i32(addr, addr, -8);
                     }
@@ -7577,13 +7565,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                         } else {
                             tcg_gen_addi_i32(addr, addr, 4);
                         }
-                        if (op == (env->uncached_cpsr & CPSR_M)) {
-                            store_reg(s, 13, addr);
-                        } else {
-                            tmp = tcg_const_i32(op);
-                            gen_helper_set_r13_banked(cpu_env, tmp, addr);
-                            tcg_temp_free_i32(tmp);
-                        }
+                        tmp = tcg_const_i32(op);
+                        gen_helper_set_r13_banked(cpu_env, tmp, addr);
+                        tcg_temp_free_i32(tmp);
                     } else {
                         dead_tmp(addr);
                     }
-- 
1.6.3.3

[Qemu-devel] [PATCH 2/8] target-arm: Add symbolic constants for bitfields in TB flags

[Peter Maydell]

Add symbolic constants for the bitfields we use in the TB flags.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h |   45 +++++++++++++++++++++++++++++++++++++++------
 1 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 340933e..3adb118 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -440,17 +440,50 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
 
 #include "cpu-all.h"
 
+/* Bit usage in the TB flags field: */
+#define ARM_TBFLAG_THUMB_SHIFT      0
+#define ARM_TBFLAG_THUMB_MASK       (1 << ARM_TBFLAG_THUMB_SHIFT)
+#define ARM_TBFLAG_VECLEN_SHIFT     1
+#define ARM_TBFLAG_VECLEN_MASK      (0x7 << ARM_TBFLAG_VECLEN_SHIFT)
+#define ARM_TBFLAG_VECSTRIDE_SHIFT  4
+#define ARM_TBFLAG_VECSTRIDE_MASK   (0x3 << ARM_TBFLAG_VECSTRIDE_SHIFT)
+#define ARM_TBFLAG_PRIV_SHIFT       6
+#define ARM_TBFLAG_PRIV_MASK        (1 << ARM_TBFLAG_PRIV_SHIFT)
+#define ARM_TBFLAG_VFPEN_SHIFT      7
+#define ARM_TBFLAG_VFPEN_MASK       (1 << ARM_TBFLAG_VFPEN_SHIFT)
+#define ARM_TBFLAG_CONDEXEC_SHIFT   8
+#define ARM_TBFLAG_CONDEXEC_MASK    (0xff << ARM_TBFLAG_CONDEXEC_SHIFT)
+/* Bits 31..16 are currently unused. */
+
+/* some convenience accessor macros */
+#define ARM_TBFLAG_THUMB(F) \
+    (((F) & ARM_TBFLAG_THUMB_MASK) >> ARM_TBFLAG_THUMB_SHIFT)
+#define ARM_TBFLAG_VECLEN(F) \
+    (((F) & ARM_TBFLAG_VECLEN_MASK) >> ARM_TBFLAG_VECLEN_SHIFT)
+#define ARM_TBFLAG_VECSTRIDE(F) \
+    (((F) & ARM_TBFLAG_VECSTRIDE_MASK) >> ARM_TBFLAG_VECSTRIDE_SHIFT)
+#define ARM_TBFLAG_PRIV(F) \
+    (((F) & ARM_TBFLAG_PRIV_MASK) >> ARM_TBFLAG_PRIV_SHIFT)
+#define ARM_TBFLAG_VFPEN(F) \
+    (((F) & ARM_TBFLAG_VFPEN_MASK) >> ARM_TBFLAG_VFPEN_SHIFT)
+#define ARM_TBFLAG_CONDEXEC(F) \
+    (((F) & ARM_TBFLAG_CONDEXEC_MASK) >> ARM_TBFLAG_CONDEXEC_SHIFT)
+
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
                                         target_ulong *cs_base, int *flags)
 {
     *pc = env->regs[15];
     *cs_base = 0;
-    *flags = env->thumb | (env->vfp.vec_len << 1)
-            | (env->vfp.vec_stride << 4) | (env->condexec_bits << 8);
-    if ((env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR)
-        *flags |= (1 << 6);
-    if (env->vfp.xregs[ARM_VFP_FPEXC] & (1 << 30))
-        *flags |= (1 << 7);
+    *flags = (env->thumb << ARM_TBFLAG_THUMB_SHIFT)
+        | (env->vfp.vec_len << ARM_TBFLAG_VECLEN_SHIFT)
+        | (env->vfp.vec_stride << ARM_TBFLAG_VECSTRIDE_SHIFT)
+        | (env->condexec_bits << ARM_TBFLAG_CONDEXEC_SHIFT);
+    if ((env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR) {
+        *flags |= ARM_TBFLAG_PRIV_MASK;
+    }
+    if (env->vfp.xregs[ARM_VFP_FPEXC] & (1 << 30)) {
+        *flags |= ARM_TBFLAG_VFPEN_MASK;
+    }
 }
 
 #endif
-- 
1.6.3.3

[Qemu-devel] [PATCH 3/8] target-arm: Translate with VFP-enabled from TB flags, not CPUState

[Peter Maydell]

When translating code, whether the VFP unit is enabled for this TB
is stored in a bit in the TB flags. Use this rather than incorrectly
reading the FPEXC from the CPUState passed to translation.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate.c |   14 +++++---------
 1 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index c391398..9e0b0b1 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -59,6 +59,7 @@ typedef struct DisasContext {
 #if !defined(CONFIG_USER_ONLY)
     int user;
 #endif
+    int vfp_enabled;
 } DisasContext;
 
 #if defined(CONFIG_USER_ONLY)
@@ -2603,12 +2604,6 @@ static void gen_vfp_msr(TCGv tmp)
     dead_tmp(tmp);
 }
 
-static inline int
-vfp_enabled(CPUState * env)
-{
-    return ((env->vfp.xregs[ARM_VFP_FPEXC] & (1 << 30)) != 0);
-}
-
 static void gen_neon_dup_u8(TCGv var, int shift)
 {
     TCGv tmp = new_tmp();
@@ -2653,7 +2648,7 @@ static int disas_vfp_insn(CPUState * env, DisasContext *s, uint32_t insn)
     if (!arm_feature(env, ARM_FEATURE_VFP))
         return 1;
 
-    if (!vfp_enabled(env)) {
+    if (!s->vfp_enabled) {
         /* VFP disabled.  Only allow fmxr/fmrx to/from some control regs.  */
         if ((insn & 0x0fe00fff) != 0x0ee00a10)
             return 1;
@@ -3804,7 +3799,7 @@ static int disas_neon_ls_insn(CPUState * env, DisasContext *s, uint32_t insn)
     TCGv tmp2;
     TCGv_i64 tmp64;
 
-    if (!vfp_enabled(env))
+    if (!s->vfp_enabled)
       return 1;
     VFP_DREG_D(rd, insn);
     rn = (insn >> 16) & 0xf;
@@ -4199,7 +4194,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
     TCGv tmp, tmp2, tmp3, tmp4, tmp5;
     TCGv_i64 tmp64;
 
-    if (!vfp_enabled(env))
+    if (!s->vfp_enabled)
       return 1;
     q = (insn & (1 << 6)) != 0;
     u = (insn >> 24) & 1;
@@ -9087,6 +9082,7 @@ static inline void gen_intermediate_code_internal(CPUState *env,
         dc->user = (env->uncached_cpsr & 0x1f) == ARM_CPU_MODE_USR;
     }
 #endif
+    dc->vfp_enabled = ARM_TBFLAG_VFPEN(tb->flags);
     cpu_F0s = tcg_temp_new_i32();
     cpu_F1s = tcg_temp_new_i32();
     cpu_F0d = tcg_temp_new_i64();
-- 
1.6.3.3

[Qemu-devel] [PATCH 4/8] target-arm: Translate with VFP len/stride from TB flags, not CPUState

[Peter Maydell]

When translating, the VFP vector length and stride for this TB are encoded
in the TB flags; the CPUState copies may be different and must not be used.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 9e0b0b1..624a443 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -60,6 +60,8 @@ typedef struct DisasContext {
     int user;
 #endif
     int vfp_enabled;
+    int vec_len;
+    int vec_stride;
 } DisasContext;
 
 #if defined(CONFIG_USER_ONLY)
@@ -2895,7 +2897,7 @@ static int disas_vfp_insn(CPUState * env, DisasContext *s, uint32_t insn)
                 rm = VFP_SREG_M(insn);
             }
 
-            veclen = env->vfp.vec_len;
+            veclen = s->vec_len;
             if (op == 15 && rn > 3)
                 veclen = 0;
 
@@ -2916,9 +2918,9 @@ static int disas_vfp_insn(CPUState * env, DisasContext *s, uint32_t insn)
                     veclen = 0;
                 } else {
                     if (dp)
-                        delta_d = (env->vfp.vec_stride >> 1) + 1;
+                        delta_d = (s->vec_stride >> 1) + 1;
                     else
-                        delta_d = env->vfp.vec_stride + 1;
+                        delta_d = s->vec_stride + 1;
 
                     if ((rm & bank_mask) == 0) {
                         /* mixed scalar/vector */
@@ -9083,6 +9085,8 @@ static inline void gen_intermediate_code_internal(CPUState *env,
     }
 #endif
     dc->vfp_enabled = ARM_TBFLAG_VFPEN(tb->flags);
+    dc->vec_len = ARM_TBFLAG_VECLEN(tb->flags);
+    dc->vec_stride = ARM_TBFLAG_VECSTRIDE(tb->flags);
     cpu_F0s = tcg_temp_new_i32();
     cpu_F1s = tcg_temp_new_i32();
     cpu_F0d = tcg_temp_new_i64();
-- 
1.6.3.3

[Qemu-devel] [PATCH 5/8] target-arm: Translate with Thumb state from TB flags, not CPUState

[Peter Maydell]

The Thumb/ARM state for the TB being translated should come from
the TB flags, not the CPUState.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 624a443..bda5d47 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -9074,7 +9074,7 @@ static inline void gen_intermediate_code_internal(CPUState *env,
     dc->pc = pc_start;
     dc->singlestep_enabled = env->singlestep_enabled;
     dc->condjmp = 0;
-    dc->thumb = env->thumb;
+    dc->thumb = ARM_TBFLAG_THUMB(tb->flags);
     dc->condexec_mask = (env->condexec_bits & 0xf) << 1;
     dc->condexec_cond = env->condexec_bits >> 4;
 #if !defined(CONFIG_USER_ONLY)
@@ -9161,7 +9161,7 @@ static inline void gen_intermediate_code_internal(CPUState *env,
         if (num_insns + 1 == max_insns && (tb->cflags & CF_LAST_IO))
             gen_io_start();
 
-        if (env->thumb) {
+        if (dc->thumb) {
             disas_thumb_insn(env, dc);
             if (dc->condexec_mask) {
                 dc->condexec_cond = (dc->condexec_cond & 0xe)
@@ -9275,7 +9275,7 @@ done_generating:
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("----------------\n");
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(pc_start, dc->pc - pc_start, env->thumb);
+        log_target_disas(pc_start, dc->pc - pc_start, dc->thumb);
         qemu_log("\n");
     }
 #endif
-- 
1.6.3.3

[Qemu-devel] [PATCH 6/8] target-arm: Translate with condexec bits from TB flags, not CPUState

[Peter Maydell]

When translating, the condexec bits for the TB are in the TB flags;
the CPUState condexec bits may be different.

This patch fixes https://bugs.launchpad.net/bugs/604872 where we might
segfault if we took an exception in the middle of a TB with an IT
block, because when we came to retranslate in cpu_restore_state()
the CPUState condexec bits would have advanced compared to the start
of the TB and we would generate different (wrong) code.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index bda5d47..4fe202d 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -9075,8 +9075,8 @@ static inline void gen_intermediate_code_internal(CPUState *env,
     dc->singlestep_enabled = env->singlestep_enabled;
     dc->condjmp = 0;
     dc->thumb = ARM_TBFLAG_THUMB(tb->flags);
-    dc->condexec_mask = (env->condexec_bits & 0xf) << 1;
-    dc->condexec_cond = env->condexec_bits >> 4;
+    dc->condexec_mask = (ARM_TBFLAG_CONDEXEC(tb->flags) & 0xf) << 1;
+    dc->condexec_cond = ARM_TBFLAG_CONDEXEC(tb->flags) >> 4;
 #if !defined(CONFIG_USER_ONLY)
     if (IS_M(env)) {
         dc->user = ((env->v7m.exception == 0) && (env->v7m.control & 1));
@@ -9105,7 +9105,7 @@ static inline void gen_intermediate_code_internal(CPUState *env,
     gen_icount_start();
     /* Reset the conditional execution bits immediately. This avoids
        complications trying to do it at the end of the block.  */
-    if (env->condexec_bits)
+    if (dc->condexec_mask || dc->condexec_cond)
       {
         TCGv tmp = new_tmp();
         tcg_gen_movi_i32(tmp, 0);
-- 
1.6.3.3

[Qemu-devel] [PATCH 7/8] target-arm: Set privileged bit in TB flags correctly for M profile

[Peter Maydell]

M profile ARM cores don't have a CPSR mode field. Set the bit in the
TB flags that indicates non-user mode correctly for these cores.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 3adb118..3cd69c4 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -472,13 +472,19 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
                                         target_ulong *cs_base, int *flags)
 {
+    int privmode;
     *pc = env->regs[15];
     *cs_base = 0;
     *flags = (env->thumb << ARM_TBFLAG_THUMB_SHIFT)
         | (env->vfp.vec_len << ARM_TBFLAG_VECLEN_SHIFT)
         | (env->vfp.vec_stride << ARM_TBFLAG_VECSTRIDE_SHIFT)
         | (env->condexec_bits << ARM_TBFLAG_CONDEXEC_SHIFT);
-    if ((env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR) {
+    if (arm_feature(env, ARM_FEATURE_M)) {
+        privmode = !((env->v7m.exception == 0) && (env->v7m.control & 1));
+    } else {
+        privmode = (env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR;
+    }
+    if (privmode) {
         *flags |= ARM_TBFLAG_PRIV_MASK;
     }
     if (env->vfp.xregs[ARM_VFP_FPEXC] & (1 << 30)) {
-- 
1.6.3.3

[Qemu-devel] [PATCH 8/8] target-arm: Translate with user-state from TB flags, not CPUState

[Peter Maydell]

When translating, get the user/priv state from the TB flags, not
the CPUState.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 4fe202d..aa3c60a 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -9078,11 +9078,7 @@ static inline void gen_intermediate_code_internal(CPUState *env,
     dc->condexec_mask = (ARM_TBFLAG_CONDEXEC(tb->flags) & 0xf) << 1;
     dc->condexec_cond = ARM_TBFLAG_CONDEXEC(tb->flags) >> 4;
 #if !defined(CONFIG_USER_ONLY)
-    if (IS_M(env)) {
-        dc->user = ((env->v7m.exception == 0) && (env->v7m.control & 1));
-    } else {
-        dc->user = (env->uncached_cpsr & 0x1f) == ARM_CPU_MODE_USR;
-    }
+    dc->user = (ARM_TBFLAG_PRIV(tb->flags) == 0);
 #endif
     dc->vfp_enabled = ARM_TBFLAG_VFPEN(tb->flags);
     dc->vec_len = ARM_TBFLAG_VECLEN(tb->flags);
-- 
1.6.3.3

Re: [Qemu-devel] [PATCH 1/8] target-arm: Don't generate code specific to current CPU mode for SRS

[Aurelien Jarno]

On Tue, Jan 11, 2011 at 10:12:11PM +0000, Peter Maydell wrote:
> When translating the SRS instruction, handle the "store registers
> to stack of current mode" case in the helper function rather than
> inline. This means the generated code does not make assumptions
> about the current CPU mode which might not be valid when the TB
> is executed later.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/helper.c    |   12 ++++++++++--
>  target-arm/translate.c |   46 +++++++++++++++-------------------------------
>  2 files changed, 25 insertions(+), 33 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 705b99f..f08e09e 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1846,12 +1846,20 @@ bad_reg:
>  
>  void HELPER(set_r13_banked)(CPUState *env, uint32_t mode, uint32_t val)
>  {
> -    env->banked_r13[bank_number(mode)] = val;
> +    if ((env->uncached_cpsr & CPSR_M) == mode) {
> +        env->regs[13] = val;
> +    } else {
> +        env->banked_r13[bank_number(mode)] = val;
> +    }
>  }
>  
>  uint32_t HELPER(get_r13_banked)(CPUState *env, uint32_t mode)
>  {
> -    return env->banked_r13[bank_number(mode)];
> +    if ((env->uncached_cpsr & CPSR_M) == mode) {
> +        return env->regs[13];
> +    } else {
> +        return env->banked_r13[bank_number(mode)];
> +    }
>  }
>  
>  uint32_t HELPER(v7m_mrs)(CPUState *env, uint32_t reg)
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 2ce82f3..c391398 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -6101,14 +6101,10 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                  goto illegal_op;
>              ARCH(6);
>              op1 = (insn & 0x1f);
> -            if (op1 == (env->uncached_cpsr & CPSR_M)) {
> -                addr = load_reg(s, 13);
> -            } else {
> -                addr = new_tmp();
> -                tmp = tcg_const_i32(op1);
> -                gen_helper_get_r13_banked(addr, cpu_env, tmp);
> -                tcg_temp_free_i32(tmp);
> -            }
> +            addr = new_tmp();
> +            tmp = tcg_const_i32(op1);
> +            gen_helper_get_r13_banked(addr, cpu_env, tmp);
> +            tcg_temp_free_i32(tmp);
>              i = (insn >> 23) & 3;
>              switch (i) {
>              case 0: offset = -4; break; /* DA */
> @@ -6135,14 +6131,10 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                  }
>                  if (offset)
>                      tcg_gen_addi_i32(addr, addr, offset);
> -                if (op1 == (env->uncached_cpsr & CPSR_M)) {
> -                    store_reg(s, 13, addr);
> -                } else {
> -                    tmp = tcg_const_i32(op1);
> -                    gen_helper_set_r13_banked(cpu_env, tmp, addr);
> -                    tcg_temp_free_i32(tmp);
> -                    dead_tmp(addr);
> -                }
> +                tmp = tcg_const_i32(op1);
> +                gen_helper_set_r13_banked(cpu_env, tmp, addr);
> +                tcg_temp_free_i32(tmp);
> +                dead_tmp(addr);
>              } else {
>                  dead_tmp(addr);
>              }
> @@ -7554,14 +7546,10 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                  } else {
>                      /* srs */
>                      op = (insn & 0x1f);
> -                    if (op == (env->uncached_cpsr & CPSR_M)) {
> -                        addr = load_reg(s, 13);
> -                    } else {
> -                        addr = new_tmp();
> -                        tmp = tcg_const_i32(op);
> -                        gen_helper_get_r13_banked(addr, cpu_env, tmp);
> -                        tcg_temp_free_i32(tmp);
> -                    }
> +                    addr = new_tmp();
> +                    tmp = tcg_const_i32(op);
> +                    gen_helper_get_r13_banked(addr, cpu_env, tmp);
> +                    tcg_temp_free_i32(tmp);
>                      if ((insn & (1 << 24)) == 0) {
>                          tcg_gen_addi_i32(addr, addr, -8);
>                      }
> @@ -7577,13 +7565,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                          } else {
>                              tcg_gen_addi_i32(addr, addr, 4);
>                          }
> -                        if (op == (env->uncached_cpsr & CPSR_M)) {
> -                            store_reg(s, 13, addr);
> -                        } else {
> -                            tmp = tcg_const_i32(op);
> -                            gen_helper_set_r13_banked(cpu_env, tmp, addr);
> -                            tcg_temp_free_i32(tmp);
> -                        }
> +                        tmp = tcg_const_i32(op);
> +                        gen_helper_set_r13_banked(cpu_env, tmp, addr);
> +                        tcg_temp_free_i32(tmp);
>                      } else {
>                          dead_tmp(addr);
>                      }
> -- 
> 1.6.3.3
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 2/8] target-arm: Add symbolic constants for bitfields in TB flags

[Aurelien Jarno]

On Tue, Jan 11, 2011 at 10:12:12PM +0000, Peter Maydell wrote:
> Add symbolic constants for the bitfields we use in the TB flags.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.h |   45 +++++++++++++++++++++++++++++++++++++++------
>  1 files changed, 39 insertions(+), 6 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 340933e..3adb118 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -440,17 +440,50 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
>  
>  #include "cpu-all.h"
>  
> +/* Bit usage in the TB flags field: */
> +#define ARM_TBFLAG_THUMB_SHIFT      0
> +#define ARM_TBFLAG_THUMB_MASK       (1 << ARM_TBFLAG_THUMB_SHIFT)
> +#define ARM_TBFLAG_VECLEN_SHIFT     1
> +#define ARM_TBFLAG_VECLEN_MASK      (0x7 << ARM_TBFLAG_VECLEN_SHIFT)
> +#define ARM_TBFLAG_VECSTRIDE_SHIFT  4
> +#define ARM_TBFLAG_VECSTRIDE_MASK   (0x3 << ARM_TBFLAG_VECSTRIDE_SHIFT)
> +#define ARM_TBFLAG_PRIV_SHIFT       6
> +#define ARM_TBFLAG_PRIV_MASK        (1 << ARM_TBFLAG_PRIV_SHIFT)
> +#define ARM_TBFLAG_VFPEN_SHIFT      7
> +#define ARM_TBFLAG_VFPEN_MASK       (1 << ARM_TBFLAG_VFPEN_SHIFT)
> +#define ARM_TBFLAG_CONDEXEC_SHIFT   8
> +#define ARM_TBFLAG_CONDEXEC_MASK    (0xff << ARM_TBFLAG_CONDEXEC_SHIFT)
> +/* Bits 31..16 are currently unused. */
> +
> +/* some convenience accessor macros */
> +#define ARM_TBFLAG_THUMB(F) \
> +    (((F) & ARM_TBFLAG_THUMB_MASK) >> ARM_TBFLAG_THUMB_SHIFT)
> +#define ARM_TBFLAG_VECLEN(F) \
> +    (((F) & ARM_TBFLAG_VECLEN_MASK) >> ARM_TBFLAG_VECLEN_SHIFT)
> +#define ARM_TBFLAG_VECSTRIDE(F) \
> +    (((F) & ARM_TBFLAG_VECSTRIDE_MASK) >> ARM_TBFLAG_VECSTRIDE_SHIFT)
> +#define ARM_TBFLAG_PRIV(F) \
> +    (((F) & ARM_TBFLAG_PRIV_MASK) >> ARM_TBFLAG_PRIV_SHIFT)
> +#define ARM_TBFLAG_VFPEN(F) \
> +    (((F) & ARM_TBFLAG_VFPEN_MASK) >> ARM_TBFLAG_VFPEN_SHIFT)
> +#define ARM_TBFLAG_CONDEXEC(F) \
> +    (((F) & ARM_TBFLAG_CONDEXEC_MASK) >> ARM_TBFLAG_CONDEXEC_SHIFT)
> +
>  static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
>                                          target_ulong *cs_base, int *flags)
>  {
>      *pc = env->regs[15];
>      *cs_base = 0;
> -    *flags = env->thumb | (env->vfp.vec_len << 1)
> -            | (env->vfp.vec_stride << 4) | (env->condexec_bits << 8);
> -    if ((env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR)
> -        *flags |= (1 << 6);
> -    if (env->vfp.xregs[ARM_VFP_FPEXC] & (1 << 30))
> -        *flags |= (1 << 7);
> +    *flags = (env->thumb << ARM_TBFLAG_THUMB_SHIFT)
> +        | (env->vfp.vec_len << ARM_TBFLAG_VECLEN_SHIFT)
> +        | (env->vfp.vec_stride << ARM_TBFLAG_VECSTRIDE_SHIFT)
> +        | (env->condexec_bits << ARM_TBFLAG_CONDEXEC_SHIFT);
> +    if ((env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR) {
> +        *flags |= ARM_TBFLAG_PRIV_MASK;
> +    }
> +    if (env->vfp.xregs[ARM_VFP_FPEXC] & (1 << 30)) {
> +        *flags |= ARM_TBFLAG_VFPEN_MASK;
> +    }
>  }
>  
>  #endif
> -- 
> 1.6.3.3
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 3/8] target-arm: Translate with VFP-enabled from TB flags, not CPUState

[Aurelien Jarno]

On Tue, Jan 11, 2011 at 10:12:13PM +0000, Peter Maydell wrote:
> When translating code, whether the VFP unit is enabled for this TB
> is stored in a bit in the TB flags. Use this rather than incorrectly
> reading the FPEXC from the CPUState passed to translation.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate.c |   14 +++++---------
>  1 files changed, 5 insertions(+), 9 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
 
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index c391398..9e0b0b1 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -59,6 +59,7 @@ typedef struct DisasContext {
>  #if !defined(CONFIG_USER_ONLY)
>      int user;
>  #endif
> +    int vfp_enabled;
>  } DisasContext;
>  
>  #if defined(CONFIG_USER_ONLY)
> @@ -2603,12 +2604,6 @@ static void gen_vfp_msr(TCGv tmp)
>      dead_tmp(tmp);
>  }
>  
> -static inline int
> -vfp_enabled(CPUState * env)
> -{
> -    return ((env->vfp.xregs[ARM_VFP_FPEXC] & (1 << 30)) != 0);
> -}
> -
>  static void gen_neon_dup_u8(TCGv var, int shift)
>  {
>      TCGv tmp = new_tmp();
> @@ -2653,7 +2648,7 @@ static int disas_vfp_insn(CPUState * env, DisasContext *s, uint32_t insn)
>      if (!arm_feature(env, ARM_FEATURE_VFP))
>          return 1;
>  
> -    if (!vfp_enabled(env)) {
> +    if (!s->vfp_enabled) {
>          /* VFP disabled.  Only allow fmxr/fmrx to/from some control regs.  */
>          if ((insn & 0x0fe00fff) != 0x0ee00a10)
>              return 1;
> @@ -3804,7 +3799,7 @@ static int disas_neon_ls_insn(CPUState * env, DisasContext *s, uint32_t insn)
>      TCGv tmp2;
>      TCGv_i64 tmp64;
>  
> -    if (!vfp_enabled(env))
> +    if (!s->vfp_enabled)
>        return 1;
>      VFP_DREG_D(rd, insn);
>      rn = (insn >> 16) & 0xf;
> @@ -4199,7 +4194,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
>      TCGv tmp, tmp2, tmp3, tmp4, tmp5;
>      TCGv_i64 tmp64;
>  
> -    if (!vfp_enabled(env))
> +    if (!s->vfp_enabled)
>        return 1;
>      q = (insn & (1 << 6)) != 0;
>      u = (insn >> 24) & 1;
> @@ -9087,6 +9082,7 @@ static inline void gen_intermediate_code_internal(CPUState *env,
>          dc->user = (env->uncached_cpsr & 0x1f) == ARM_CPU_MODE_USR;
>      }
>  #endif
> +    dc->vfp_enabled = ARM_TBFLAG_VFPEN(tb->flags);
>      cpu_F0s = tcg_temp_new_i32();
>      cpu_F1s = tcg_temp_new_i32();
>      cpu_F0d = tcg_temp_new_i64();
> -- 
> 1.6.3.3
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 4/8] target-arm: Translate with VFP len/stride from TB flags, not CPUState

[Aurelien Jarno]

On Tue, Jan 11, 2011 at 10:12:14PM +0000, Peter Maydell wrote:
> When translating, the VFP vector length and stride for this TB are encoded
> in the TB flags; the CPUState copies may be different and must not be used.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate.c |   10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
 
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 9e0b0b1..624a443 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -60,6 +60,8 @@ typedef struct DisasContext {
>      int user;
>  #endif
>      int vfp_enabled;
> +    int vec_len;
> +    int vec_stride;
>  } DisasContext;
>  
>  #if defined(CONFIG_USER_ONLY)
> @@ -2895,7 +2897,7 @@ static int disas_vfp_insn(CPUState * env, DisasContext *s, uint32_t insn)
>                  rm = VFP_SREG_M(insn);
>              }
>  
> -            veclen = env->vfp.vec_len;
> +            veclen = s->vec_len;
>              if (op == 15 && rn > 3)
>                  veclen = 0;
>  
> @@ -2916,9 +2918,9 @@ static int disas_vfp_insn(CPUState * env, DisasContext *s, uint32_t insn)
>                      veclen = 0;
>                  } else {
>                      if (dp)
> -                        delta_d = (env->vfp.vec_stride >> 1) + 1;
> +                        delta_d = (s->vec_stride >> 1) + 1;
>                      else
> -                        delta_d = env->vfp.vec_stride + 1;
> +                        delta_d = s->vec_stride + 1;
>  
>                      if ((rm & bank_mask) == 0) {
>                          /* mixed scalar/vector */
> @@ -9083,6 +9085,8 @@ static inline void gen_intermediate_code_internal(CPUState *env,
>      }
>  #endif
>      dc->vfp_enabled = ARM_TBFLAG_VFPEN(tb->flags);
> +    dc->vec_len = ARM_TBFLAG_VECLEN(tb->flags);
> +    dc->vec_stride = ARM_TBFLAG_VECSTRIDE(tb->flags);
>      cpu_F0s = tcg_temp_new_i32();
>      cpu_F1s = tcg_temp_new_i32();
>      cpu_F0d = tcg_temp_new_i64();
> -- 
> 1.6.3.3
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 5/8] target-arm: Translate with Thumb state from TB flags, not CPUState

[Aurelien Jarno]

On Tue, Jan 11, 2011 at 10:12:15PM +0000, Peter Maydell wrote:
> The Thumb/ARM state for the TB being translated should come from
> the TB flags, not the CPUState.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
 
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 624a443..bda5d47 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -9074,7 +9074,7 @@ static inline void gen_intermediate_code_internal(CPUState *env,
>      dc->pc = pc_start;
>      dc->singlestep_enabled = env->singlestep_enabled;
>      dc->condjmp = 0;
> -    dc->thumb = env->thumb;
> +    dc->thumb = ARM_TBFLAG_THUMB(tb->flags);
>      dc->condexec_mask = (env->condexec_bits & 0xf) << 1;
>      dc->condexec_cond = env->condexec_bits >> 4;
>  #if !defined(CONFIG_USER_ONLY)
> @@ -9161,7 +9161,7 @@ static inline void gen_intermediate_code_internal(CPUState *env,
>          if (num_insns + 1 == max_insns && (tb->cflags & CF_LAST_IO))
>              gen_io_start();
>  
> -        if (env->thumb) {
> +        if (dc->thumb) {
>              disas_thumb_insn(env, dc);
>              if (dc->condexec_mask) {
>                  dc->condexec_cond = (dc->condexec_cond & 0xe)
> @@ -9275,7 +9275,7 @@ done_generating:
>      if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
>          qemu_log("----------------\n");
>          qemu_log("IN: %s\n", lookup_symbol(pc_start));
> -        log_target_disas(pc_start, dc->pc - pc_start, env->thumb);
> +        log_target_disas(pc_start, dc->pc - pc_start, dc->thumb);
>          qemu_log("\n");
>      }
>  #endif
> -- 
> 1.6.3.3
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 6/8] target-arm: Translate with condexec bits from TB flags, not CPUState

[Aurelien Jarno]

On Tue, Jan 11, 2011 at 10:12:16PM +0000, Peter Maydell wrote:
> When translating, the condexec bits for the TB are in the TB flags;
> the CPUState condexec bits may be different.
> 
> This patch fixes https://bugs.launchpad.net/bugs/604872 where we might
> segfault if we took an exception in the middle of a TB with an IT
> block, because when we came to retranslate in cpu_restore_state()
> the CPUState condexec bits would have advanced compared to the start
> of the TB and we would generate different (wrong) code.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
 
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index bda5d47..4fe202d 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -9075,8 +9075,8 @@ static inline void gen_intermediate_code_internal(CPUState *env,
>      dc->singlestep_enabled = env->singlestep_enabled;
>      dc->condjmp = 0;
>      dc->thumb = ARM_TBFLAG_THUMB(tb->flags);
> -    dc->condexec_mask = (env->condexec_bits & 0xf) << 1;
> -    dc->condexec_cond = env->condexec_bits >> 4;
> +    dc->condexec_mask = (ARM_TBFLAG_CONDEXEC(tb->flags) & 0xf) << 1;
> +    dc->condexec_cond = ARM_TBFLAG_CONDEXEC(tb->flags) >> 4;
>  #if !defined(CONFIG_USER_ONLY)
>      if (IS_M(env)) {
>          dc->user = ((env->v7m.exception == 0) && (env->v7m.control & 1));
> @@ -9105,7 +9105,7 @@ static inline void gen_intermediate_code_internal(CPUState *env,
>      gen_icount_start();
>      /* Reset the conditional execution bits immediately. This avoids
>         complications trying to do it at the end of the block.  */
> -    if (env->condexec_bits)
> +    if (dc->condexec_mask || dc->condexec_cond)
>        {
>          TCGv tmp = new_tmp();
>          tcg_gen_movi_i32(tmp, 0);
> -- 
> 1.6.3.3
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 7/8] target-arm: Set privileged bit in TB flags correctly for M profile

[Aurelien Jarno]

On Tue, Jan 11, 2011 at 10:12:17PM +0000, Peter Maydell wrote:
> M profile ARM cores don't have a CPSR mode field. Set the bit in the
> TB flags that indicates non-user mode correctly for these cores.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.h |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 3adb118..3cd69c4 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -472,13 +472,19 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
>  static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
>                                          target_ulong *cs_base, int *flags)
>  {
> +    int privmode;
>      *pc = env->regs[15];
>      *cs_base = 0;
>      *flags = (env->thumb << ARM_TBFLAG_THUMB_SHIFT)
>          | (env->vfp.vec_len << ARM_TBFLAG_VECLEN_SHIFT)
>          | (env->vfp.vec_stride << ARM_TBFLAG_VECSTRIDE_SHIFT)
>          | (env->condexec_bits << ARM_TBFLAG_CONDEXEC_SHIFT);
> -    if ((env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR) {
> +    if (arm_feature(env, ARM_FEATURE_M)) {
> +        privmode = !((env->v7m.exception == 0) && (env->v7m.control & 1));
> +    } else {
> +        privmode = (env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR;
> +    }
> +    if (privmode) {
>          *flags |= ARM_TBFLAG_PRIV_MASK;
>      }
>      if (env->vfp.xregs[ARM_VFP_FPEXC] & (1 << 30)) {
> -- 
> 1.6.3.3
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 8/8] target-arm: Translate with user-state from TB flags, not CPUState

[Aurelien Jarno]

On Tue, Jan 11, 2011 at 10:12:18PM +0000, Peter Maydell wrote:
> When translating, get the user/priv state from the TB flags, not
> the CPUState.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate.c |    6 +-----
>  1 files changed, 1 insertions(+), 5 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
 
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 4fe202d..aa3c60a 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -9078,11 +9078,7 @@ static inline void gen_intermediate_code_internal(CPUState *env,
>      dc->condexec_mask = (ARM_TBFLAG_CONDEXEC(tb->flags) & 0xf) << 1;
>      dc->condexec_cond = ARM_TBFLAG_CONDEXEC(tb->flags) >> 4;
>  #if !defined(CONFIG_USER_ONLY)
> -    if (IS_M(env)) {
> -        dc->user = ((env->v7m.exception == 0) && (env->v7m.control & 1));
> -    } else {
> -        dc->user = (env->uncached_cpsr & 0x1f) == ARM_CPU_MODE_USR;
> -    }
> +    dc->user = (ARM_TBFLAG_PRIV(tb->flags) == 0);
>  #endif
>      dc->vfp_enabled = ARM_TBFLAG_VFPEN(tb->flags);
>      dc->vec_len = ARM_TBFLAG_VECLEN(tb->flags);
> -- 
> 1.6.3.3
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH v2 0/8] target-arm: Translate based on TB flags, not CPUState

[Aurelien Jarno]

On Tue, Jan 11, 2011 at 10:12:10PM +0000, Peter Maydell wrote:
> This patchset corrects a number of places in the ARM translation code
> which were generating code which was dependent on values in the CPUState
> structure which might change at runtime. This is a bad idea for two
> reasons. Firstly, we might try to reuse the generated code later when
> the assumptions baked into the generated code were no longer valid.
> Secondly, we might try to retranslate the same TB (eg when an exception
> results in our calling cpu_restore_state()) but get different generated
> code, which could result in qemu crashing.
> 
> Bug https://bugs.launchpad.net/bugs/604872 is a particular example
> of the latter case involving the IT bits; this patchset fixes that bug.
> 
> I believe that this patchset deals with all the problems. Remaining
> CPUState fields referred to in translate.c are either constant after
> system init or trigger flushing of affected TBs when they are changed.
> 
> Differences from v1: I've added some macros for the TB flags bitfields,
> as suggested by Aurelien.
> 
> Peter Maydell (8):
>   target-arm: Don't generate code specific to current CPU mode for SRS
>   target-arm: Add symbolic constants for bitfields in TB flags
>   target-arm: Translate with VFP-enabled from TB flags, not CPUState
>   target-arm: Translate with VFP len/stride from TB flags, not CPUState
>   target-arm: Translate with Thumb state from TB flags, not CPUState
>   target-arm: Translate with condexec bits from TB flags, not CPUState
>   target-arm: Set privileged bit in TB flags correctly for M profile
>   target-arm: Translate with user-state from TB flags, not CPUState
> 
>  target-arm/cpu.h       |   51 ++++++++++++++++++++++++---
>  target-arm/helper.c    |   12 +++++-
>  target-arm/translate.c |   88 ++++++++++++++++++-----------------------------
>  3 files changed, 89 insertions(+), 62 deletions(-)
> 
> 
> 

Thanks, all applied.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net