diff mbox

[RFC,v4,06/20] target-arm: Move CPU feature flags out of CPUState

Message ID 1331398436-20761-7-git-send-email-afaerber@suse.de
State New
Headers show

Commit Message

Andreas Färber March 10, 2012, 4:53 p.m. UTC 
The internal CPU feature flags were only ever set in
cpu_reset_model_id(). Therefore move their initialization into
ARMCPUClass. We might want to tweak them in the future though (e.g.,
-cpu cortex-r4,+fpu), so keep a copy in ARMCPU. This in turn means we
need to infer features for both ARMCPUClass and ARMCPU, so move feature
inference to arm_infer_features() and use macros to simplify it.

Since cpu.h defines ARMCPUState, which has been incorporated into
ARMCPU, and tries to use arm_feature() in cpu_get_tb_cpu_state(),
move arm_feature() to cpu-core.h and add a forward declaration.

Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu-qom.h |   12 ++++
 target-arm/cpu.c     |  159 ++++++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/cpu.h     |    9 +--
 target-arm/helper.c  |   95 ------------------------------
 target-arm/machine.c |    6 +-
 5 files changed, 177 insertions(+), 104 deletions(-)

Comments

Paul Brook March 15, 2012, 6:56 p.m. UTC | #1 
> The internal CPU feature flags were only ever set in
> cpu_reset_model_id(). Therefore move their initialization into
> ARMCPUClass. We might want to tweak them in the future though (e.g.,
> -cpu cortex-r4,+fpu), so keep a copy in ARMCPU. This in turn means we
> need to infer features for both ARMCPUClass and ARMCPU, so move feature
> inference to arm_infer_features() and use macros to simplify it.
> 
> Since cpu.h defines ARMCPUState, which has been incorporated into
> ARMCPU, and tries to use arm_feature() in cpu_get_tb_cpu_state(),
> move arm_feature() to cpu-core.h and add a forward declaration.

I don't like how this has ended up.  You've got a confusing mix of 
decalarative and imperative code to set CPU features.

i.e. some cores set features in ARMCPU.class_init, others use ARMCPU.features.  
I'd much prefer doing everything in the former, and removing the latter.

Also, you must not assume that the feature bits will fit in a single word.
The features code is deliberately structured so that everything outside 
arm_feature and get_feature use the ARM_FEATURE_* enumeration.  This means 
underlying bitmap implementation can be easily changed/extended.  Your patch 
breaks that, making it inconvenient to expand beyond 32 features, and very 
hard to go beyond 64.  Even just considering currently available ARM variants, 
we're likely to hit these limits very soon.

Infering of additional features should probably be done in set_feature.

Paul
diff mbox

Patch

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 9e33b6a..6c97337 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -35,6 +35,7 @@ 
 /**
  * ARMCPUClass:
  * @parent_reset: The parent class' reset handler.
+ * @features: Internal CPU feature flags.
  *
  * An ARM CPU model.
  */
@@ -48,10 +49,13 @@  typedef struct ARMCPUClass {
     struct {
         uint32_t c0_cpuid;
     } cp15;
+
+    uint32_t features;
 } ARMCPUClass;
 
 /**
  * ARMCPU:
+ * @features: Internal CPU feature flags.
  * @env: Legacy CPU state.
  *
  * An ARM CPU core.
@@ -61,6 +65,8 @@  typedef struct ARMCPU {
     CPUState parent_obj;
     /*< public >*/
 
+    uint32_t features;
+
     /* TODO Inline this and split off common state */
     CPUARMState env;
 } ARMCPU;
@@ -72,5 +78,11 @@  static inline ARMCPU *arm_env_get_cpu(CPUARMState *env)
 
 #define ENV_GET_CPU(e) CPU(arm_env_get_cpu(e))
 
+static inline int arm_feature(CPUARMState *env, int feature)
+{
+    ARMCPU *cpu = arm_env_get_cpu(env);
+    return (cpu->features & (1u << feature)) != 0;
+}
+
 
 #endif
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 097701f..f4c05d8 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -97,14 +97,88 @@  static void arm_cpu_reset(CPUState *c)
     tb_flush(env);
 }
 
+/* CPU feature flags */
+
+#define ARM_FEATURE(x) (1u << ARM_FEATURE_ ## x)
+
+#define has_feature(x) ((*features & ARM_FEATURE(x)) != 0)
+#define set_feature(x) (*features |= ARM_FEATURE(x))
+
+/**
+ * arm_infer_features:
+ * @features: Pointer to the feature flags of #ARMCPUClass or #ARMCPU.
+ *
+ * Some features automatically imply others.
+ */
+static void arm_infer_features(uint32_t *features)
+{
+    if (has_feature(V7)) {
+        set_feature(VAPA);
+        set_feature(THUMB2);
+        if (!has_feature(M)) {
+            set_feature(V6K);
+        } else {
+            set_feature(V6);
+        }
+    }
+    if (has_feature(V6K)) {
+        set_feature(V6);
+    }
+    if (has_feature(V6)) {
+        set_feature(V5);
+        if (!has_feature(M)) {
+            set_feature(AUXCR);
+        }
+    }
+    if (has_feature(V5)) {
+        set_feature(V4T);
+    }
+    if (has_feature(M)) {
+        set_feature(THUMB_DIV);
+    }
+    if (has_feature(ARM_DIV)) {
+        set_feature(THUMB_DIV);
+    }
+    if (has_feature(VFP4)) {
+        set_feature(VFP3);
+    }
+    if (has_feature(VFP3)) {
+        set_feature(VFP);
+    }
+}
+
+#undef has_feature
+#undef set_feature
+
+static inline void set_class_feature(ARMCPUClass *klass, int feature)
+{
+    klass->features |= 1u << feature;
+}
+
+static inline void unset_class_feature(ARMCPUClass *klass, int feature)
+{
+    klass->features &= ~(1u << feature);
+}
+
 /* CPU models */
 
 typedef struct ARMCPUInfo {
     const char *name;
     uint32_t id;
+    uint32_t features;
     void (*class_init)(ARMCPUClass *klass, const struct ARMCPUInfo *info);
 } ARMCPUInfo;
 
+static void arm1136_r0_class_init(ARMCPUClass *k, const ARMCPUInfo *info)
+{
+    ARMCPUClass *r1_class;
+
+    r1_class = ARM_CPU_CLASS(object_class_by_name("arm1136"));
+
+    k->features = r1_class->features;
+    unset_class_feature(k, ARM_FEATURE_V6K);
+}
+
 static void ti925t_reset(CPUState *c)
 {
     ARMCPU *cpu = ARM_CPU(c);
@@ -122,18 +196,43 @@  static void ti925t_class_init(ARMCPUClass *klass, const ARMCPUInfo *info)
     cpu_class->reset = ti925t_reset;
 }
 
+static void sa11xx_class_init(ARMCPUClass *k, const ARMCPUInfo *info)
+{
+    set_class_feature(k, ARM_FEATURE_STRONGARM);
+}
+
+static void pxa25x_class_init(ARMCPUClass *k, const ARMCPUInfo *info)
+{
+    set_class_feature(k, ARM_FEATURE_V5);
+    set_class_feature(k, ARM_FEATURE_XSCALE);
+}
+
+static void pxa270_class_init(ARMCPUClass *k, const ARMCPUInfo *info)
+{
+    set_class_feature(k, ARM_FEATURE_V5);
+    set_class_feature(k, ARM_FEATURE_XSCALE);
+    set_class_feature(k, ARM_FEATURE_IWMMXT);
+}
+
 static const ARMCPUInfo arm_cpus[] = {
     {
         .name = "arm926",
         .id = 0x41069265,
+        .features = ARM_FEATURE(V5) |
+                    ARM_FEATURE(VFP),
     },
     {
         .name = "arm946",
         .id = 0x41059461,
+        .features = ARM_FEATURE(V5) |
+                    ARM_FEATURE(MPU),
     },
     {
         .name = "arm1026",
         .id = 0x4106a262,
+        .features = ARM_FEATURE(V5) |
+                    ARM_FEATURE(VFP) |
+                    ARM_FEATURE(AUXCR),
     },
     /* What QEMU calls "arm1136-r2" is actually the 1136 r0p2, i.e. an
      * older core than plain "arm1136". In particular this does not
@@ -142,95 +241,150 @@  static const ARMCPUInfo arm_cpus[] = {
     {
         .name = "arm1136-r2",
         .id = 0x4107b362,
+        .class_init = arm1136_r0_class_init,
     },
     {
         .name = "arm1136",
         .id = 0x4117b363,
+        .features = ARM_FEATURE(V6) |
+                    ARM_FEATURE(VFP),
     },
     {
         .name = "arm1176",
         .id = 0x410fb767,
+        .features = ARM_FEATURE(V6K) |
+                    ARM_FEATURE(VFP) |
+                    ARM_FEATURE(VAPA),
     },
     {
         .name = "arm11mpcore",
         .id = 0x410fb022,
+        .features = ARM_FEATURE(V6K) |
+                    ARM_FEATURE(VFP) |
+                    ARM_FEATURE(VAPA),
     },
     {
         .name = "cortex-m3",
         .id = 0x410fc231,
+        .features = ARM_FEATURE(V7) |
+                    ARM_FEATURE(M),
     },
     {
         .name = "cortex-a8",
         .id = 0x410fc080,
+        .features = ARM_FEATURE(V7) |
+                    ARM_FEATURE(VFP3) |
+                    ARM_FEATURE(NEON) |
+                    ARM_FEATURE(THUMB2EE),
     },
     {
         .name = "cortex-a9",
         .id = 0x410fc090,
+        .features = ARM_FEATURE(V7) |
+                    ARM_FEATURE(VFP3) |
+                    ARM_FEATURE(VFP_FP16) |
+                    ARM_FEATURE(NEON) |
+                    ARM_FEATURE(THUMB2EE) |
+                    /* Note that A9 supports the MP extensions even for
+                     * A9UP and single-core A9MP (which are both different
+                     * and valid configurations; we don't model A9UP).
+                     */
+                    ARM_FEATURE(V7MP),
     },
     {
         .name = "cortex-a15",
         .id = 0x412fc0f1,
+        .features = ARM_FEATURE(V7) |
+                    ARM_FEATURE(VFP4) |
+                    ARM_FEATURE(VFP_FP16) |
+                    ARM_FEATURE(NEON) |
+                    ARM_FEATURE(THUMB2EE) |
+                    ARM_FEATURE(ARM_DIV) |
+                    ARM_FEATURE(V7MP) |
+                    ARM_FEATURE(GENERIC_TIMER),
     },
     {
         .name = "ti925t",
         .id = 0x54029252,
+        .features = ARM_FEATURE(V4T) |
+                    ARM_FEATURE(OMAPCP),
         .class_init = ti925t_class_init,
     },
     {
         .name = "sa1100",
         .id = 0x4401A11B,
+        .class_init = sa11xx_class_init,
     },
     {
         .name = "sa1110",
         .id = 0x6901B119,
+        .class_init = sa11xx_class_init,
     },
     {
         .name = "pxa250",
         .id = 0x69052100,
+        .class_init = pxa25x_class_init,
     },
     {
         .name = "pxa255",
         .id = 0x69052d00,
+        .class_init = pxa25x_class_init,
     },
     {
         .name = "pxa260",
         .id = 0x69052903,
+        .class_init = pxa25x_class_init,
     },
     {
         .name = "pxa261",
         .id = 0x69052d05,
+        .class_init = pxa25x_class_init,
     },
     {
         .name = "pxa262",
         .id = 0x69052d06,
+        .class_init = pxa25x_class_init,
     },
     {
         .name = "pxa270-a0",
         .id = 0x69054110,
+        .class_init = pxa270_class_init,
     },
     {
         .name = "pxa270-a1",
         .id = 0x69054111,
+        .class_init = pxa270_class_init,
     },
     {
         .name = "pxa270-b0",
         .id = 0x69054112,
+        .class_init = pxa270_class_init,
     },
     {
         .name = "pxa270-b1",
         .id = 0x69054113,
+        .class_init = pxa270_class_init,
     },
     {
         .name = "pxa270-c0",
         .id = 0x69054114,
+        .class_init = pxa270_class_init,
     },
     {
         .name = "pxa270-c5",
         .id = 0x69054117,
+        .class_init = pxa270_class_init,
     },
     {
         .name = "any",
         .id = 0xffffffff,
+        .features = ARM_FEATURE(V7) |
+                    ARM_FEATURE(VFP4) |
+                    ARM_FEATURE(VFP_FP16) |
+                    ARM_FEATURE(NEON) |
+                    ARM_FEATURE(THUMB2EE) |
+                    ARM_FEATURE(ARM_DIV) |
+                    ARM_FEATURE(V7MP),
     },
 };
 
@@ -239,6 +393,8 @@  static void arm_cpu_initfn(Object *obj)
     ARMCPU *cpu = ARM_CPU(obj);
     ARMCPUClass *cpu_class = ARM_CPU_GET_CLASS(obj);
 
+    cpu->features = cpu_class->features;
+
     memset(&cpu->env, 0, sizeof(CPUARMState));
     cpu_exec_init(&cpu->env);
 
@@ -256,10 +412,13 @@  static void arm_cpu_class_init(ObjectClass *klass, void *data)
     cpu_class->reset = arm_cpu_reset;
 
     k->cp15.c0_cpuid = info->id;
+    k->features = info->features;
 
     if (info->class_init != NULL) {
         (*info->class_init)(k, info);
     }
+
+    arm_infer_features(&k->features);
 }
 
 static void cpu_register(const ARMCPUInfo *info)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 4cfa09c..d14fb01 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -170,9 +170,6 @@  typedef struct CPUARMState {
     uint32_t teecr;
     uint32_t teehbr;
 
-    /* Internal CPU feature flags.  */
-    uint32_t features;
-
     /* VFP coprocessor state.  */
     struct {
         float64 regs[32];
@@ -385,10 +382,7 @@  enum arm_features {
     ARM_FEATURE_GENERIC_TIMER,
 };
 
-static inline int arm_feature(CPUARMState *env, int feature)
-{
-    return (env->features & (1u << feature)) != 0;
-}
+static inline int arm_feature(CPUARMState *env, int feature);
 
 void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 
@@ -475,6 +469,7 @@  static inline void cpu_clone_regs(CPUARMState *env, target_ulong newsp)
 #endif
 
 #include "cpu-all.h"
+#include "cpu-qom.h"
 
 /* Bit usage in the TB flags field: */
 #define ARM_TBFLAG_THUMB_SHIFT      0
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 5ebe308..0dd6065 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -44,46 +44,31 @@  static uint32_t arm1176_cp15_c0_c1[8] =
 static uint32_t arm1176_cp15_c0_c2[8] =
 { 0x0140011, 0x12002111, 0x11231121, 0x01102131, 0x01141, 0, 0, 0 };
 
-static inline void set_feature(CPUARMState *env, int feature)
-{
-    env->features |= 1u << feature;
-}
-
 static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
 {
     switch (id) {
     case ARM_CPUID_ARM926:
-        set_feature(env, ARM_FEATURE_V5);
-        set_feature(env, ARM_FEATURE_VFP);
         env->vfp.xregs[ARM_VFP_FPSID] = 0x41011090;
         env->cp15.c0_cachetype = 0x1dd20d2;
         env->cp15.c1_sys = 0x00090078;
         break;
     case ARM_CPUID_ARM946:
-        set_feature(env, ARM_FEATURE_V5);
-        set_feature(env, ARM_FEATURE_MPU);
         env->cp15.c0_cachetype = 0x0f004006;
         env->cp15.c1_sys = 0x00000078;
         break;
     case ARM_CPUID_ARM1026:
-        set_feature(env, ARM_FEATURE_V5);
-        set_feature(env, ARM_FEATURE_VFP);
-        set_feature(env, ARM_FEATURE_AUXCR);
         env->vfp.xregs[ARM_VFP_FPSID] = 0x410110a0;
         env->cp15.c0_cachetype = 0x1dd20d2;
         env->cp15.c1_sys = 0x00090078;
         break;
     case ARM_CPUID_ARM1136:
         /* This is the 1136 r1, which is a v6K core */
-        set_feature(env, ARM_FEATURE_V6K);
         /* Fall through */
     case ARM_CPUID_ARM1136_R2:
         /* What qemu calls "arm1136_r2" is actually the 1136 r0p2, ie an
          * older core than plain "arm1136". In particular this does not
          * have the v6K features.
          */
-        set_feature(env, ARM_FEATURE_V6);
-        set_feature(env, ARM_FEATURE_VFP);
         /* These ID register values are correct for 1136 but may be wrong
          * for 1136_r2 (in particular r0p2 does not actually implement most
          * of the ID registers).
@@ -97,9 +82,6 @@  static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c1_sys = 0x00050078;
         break;
     case ARM_CPUID_ARM1176:
-        set_feature(env, ARM_FEATURE_V6K);
-        set_feature(env, ARM_FEATURE_VFP);
-        set_feature(env, ARM_FEATURE_VAPA);
         env->vfp.xregs[ARM_VFP_FPSID] = 0x410120b5;
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x11111111;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x00000000;
@@ -109,9 +91,6 @@  static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c1_sys = 0x00050078;
         break;
     case ARM_CPUID_ARM11MPCORE:
-        set_feature(env, ARM_FEATURE_V6K);
-        set_feature(env, ARM_FEATURE_VFP);
-        set_feature(env, ARM_FEATURE_VAPA);
         env->vfp.xregs[ARM_VFP_FPSID] = 0x410120b4;
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x11111111;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x00000000;
@@ -120,10 +99,6 @@  static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c0_cachetype = 0x1dd20d2;
         break;
     case ARM_CPUID_CORTEXA8:
-        set_feature(env, ARM_FEATURE_V7);
-        set_feature(env, ARM_FEATURE_VFP3);
-        set_feature(env, ARM_FEATURE_NEON);
-        set_feature(env, ARM_FEATURE_THUMB2EE);
         env->vfp.xregs[ARM_VFP_FPSID] = 0x410330c0;
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x11110222;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x00011100;
@@ -137,16 +112,6 @@  static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c1_sys = 0x00c50078;
         break;
     case ARM_CPUID_CORTEXA9:
-        set_feature(env, ARM_FEATURE_V7);
-        set_feature(env, ARM_FEATURE_VFP3);
-        set_feature(env, ARM_FEATURE_VFP_FP16);
-        set_feature(env, ARM_FEATURE_NEON);
-        set_feature(env, ARM_FEATURE_THUMB2EE);
-        /* Note that A9 supports the MP extensions even for
-         * A9UP and single-core A9MP (which are both different
-         * and valid configurations; we don't model A9UP).
-         */
-        set_feature(env, ARM_FEATURE_V7MP);
         env->vfp.xregs[ARM_VFP_FPSID] = 0x41033090;
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x11110222;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x01111111;
@@ -159,14 +124,6 @@  static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c1_sys = 0x00c50078;
         break;
     case ARM_CPUID_CORTEXA15:
-        set_feature(env, ARM_FEATURE_V7);
-        set_feature(env, ARM_FEATURE_VFP4);
-        set_feature(env, ARM_FEATURE_VFP_FP16);
-        set_feature(env, ARM_FEATURE_NEON);
-        set_feature(env, ARM_FEATURE_THUMB2EE);
-        set_feature(env, ARM_FEATURE_ARM_DIV);
-        set_feature(env, ARM_FEATURE_V7MP);
-        set_feature(env, ARM_FEATURE_GENERIC_TIMER);
         env->vfp.xregs[ARM_VFP_FPSID] = 0x410430f0;
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x10110222;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x11111111;
@@ -180,22 +137,11 @@  static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c1_sys = 0x00c50078;
         break;
     case ARM_CPUID_CORTEXM3:
-        set_feature(env, ARM_FEATURE_V7);
-        set_feature(env, ARM_FEATURE_M);
         break;
     case ARM_CPUID_ANY: /* For userspace emulation.  */
-        set_feature(env, ARM_FEATURE_V7);
-        set_feature(env, ARM_FEATURE_VFP4);
-        set_feature(env, ARM_FEATURE_VFP_FP16);
-        set_feature(env, ARM_FEATURE_NEON);
-        set_feature(env, ARM_FEATURE_THUMB2EE);
-        set_feature(env, ARM_FEATURE_ARM_DIV);
-        set_feature(env, ARM_FEATURE_V7MP);
         break;
     case ARM_CPUID_TI915T:
     case ARM_CPUID_TI925T:
-        set_feature(env, ARM_FEATURE_V4T);
-        set_feature(env, ARM_FEATURE_OMAPCP);
         env->cp15.c0_cachetype = 0x5109149;
         env->cp15.c1_sys = 0x00000070;
         env->cp15.c15_i_max = 0x000;
@@ -206,8 +152,6 @@  static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
     case ARM_CPUID_PXA260:
     case ARM_CPUID_PXA261:
     case ARM_CPUID_PXA262:
-        set_feature(env, ARM_FEATURE_V5);
-        set_feature(env, ARM_FEATURE_XSCALE);
         /* JTAG_ID is ((id << 28) | 0x09265013) */
         env->cp15.c0_cachetype = 0xd172172;
         env->cp15.c1_sys = 0x00000078;
@@ -218,58 +162,19 @@  static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
     case ARM_CPUID_PXA270_B1:
     case ARM_CPUID_PXA270_C0:
     case ARM_CPUID_PXA270_C5:
-        set_feature(env, ARM_FEATURE_V5);
-        set_feature(env, ARM_FEATURE_XSCALE);
         /* JTAG_ID is ((id << 28) | 0x09265013) */
-        set_feature(env, ARM_FEATURE_IWMMXT);
         env->iwmmxt.cregs[ARM_IWMMXT_wCID] = 0x69051000 | 'Q';
         env->cp15.c0_cachetype = 0xd172172;
         env->cp15.c1_sys = 0x00000078;
         break;
     case ARM_CPUID_SA1100:
     case ARM_CPUID_SA1110:
-        set_feature(env, ARM_FEATURE_STRONGARM);
         env->cp15.c1_sys = 0x00000070;
         break;
     default:
         cpu_abort(env, "Bad CPU ID: %x\n", id);
         break;
     }
-
-    /* Some features automatically imply others: */
-    if (arm_feature(env, ARM_FEATURE_V7)) {
-        set_feature(env, ARM_FEATURE_VAPA);
-        set_feature(env, ARM_FEATURE_THUMB2);
-        if (!arm_feature(env, ARM_FEATURE_M)) {
-            set_feature(env, ARM_FEATURE_V6K);
-        } else {
-            set_feature(env, ARM_FEATURE_V6);
-        }
-    }
-    if (arm_feature(env, ARM_FEATURE_V6K)) {
-        set_feature(env, ARM_FEATURE_V6);
-    }
-    if (arm_feature(env, ARM_FEATURE_V6)) {
-        set_feature(env, ARM_FEATURE_V5);
-        if (!arm_feature(env, ARM_FEATURE_M)) {
-            set_feature(env, ARM_FEATURE_AUXCR);
-        }
-    }
-    if (arm_feature(env, ARM_FEATURE_V5)) {
-        set_feature(env, ARM_FEATURE_V4T);
-    }
-    if (arm_feature(env, ARM_FEATURE_M)) {
-        set_feature(env, ARM_FEATURE_THUMB_DIV);
-    }
-    if (arm_feature(env, ARM_FEATURE_ARM_DIV)) {
-        set_feature(env, ARM_FEATURE_THUMB_DIV);
-    }
-    if (arm_feature(env, ARM_FEATURE_VFP4)) {
-        set_feature(env, ARM_FEATURE_VFP3);
-    }
-    if (arm_feature(env, ARM_FEATURE_VFP3)) {
-        set_feature(env, ARM_FEATURE_VFP);
-    }
 }
 
 void cpu_state_reset(CPUARMState *env)
diff --git a/target-arm/machine.c b/target-arm/machine.c
index f66b8df..19c0c65 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -5,6 +5,7 @@  void cpu_save(QEMUFile *f, void *opaque)
 {
     int i;
     CPUARMState *env = (CPUARMState *)opaque;
+    ARMCPU *cpu = arm_env_get_cpu(env);
 
     for (i = 0; i < 16; i++) {
         qemu_put_be32(f, env->regs[i]);
@@ -61,7 +62,7 @@  void cpu_save(QEMUFile *f, void *opaque)
     qemu_put_be32(f, env->cp15.c15_diagnostic);
     qemu_put_be32(f, env->cp15.c15_power_diagnostic);
 
-    qemu_put_be32(f, env->features);
+    qemu_put_be32(f, cpu->features);
 
     if (arm_feature(env, ARM_FEATURE_VFP)) {
         for (i = 0;  i < 16; i++) {
@@ -115,6 +116,7 @@  void cpu_save(QEMUFile *f, void *opaque)
 int cpu_load(QEMUFile *f, void *opaque, int version_id)
 {
     CPUARMState *env = (CPUARMState *)opaque;
+    ARMCPU *cpu = arm_env_get_cpu(env);
     int i;
     uint32_t val;
 
@@ -179,7 +181,7 @@  int cpu_load(QEMUFile *f, void *opaque, int version_id)
     env->cp15.c15_diagnostic = qemu_get_be32(f);
     env->cp15.c15_power_diagnostic = qemu_get_be32(f);
 
-    env->features = qemu_get_be32(f);
+    cpu->features = qemu_get_be32(f);
 
     if (arm_feature(env, ARM_FEATURE_VFP)) {
         for (i = 0;  i < 16; i++) {