diff mbox

[24/25] vmstate: port arm cpu

Message ID b8700d241c008dbd3c5dcd8bc0ceb35b10b01878.1319550280.git.quintela@redhat.com
State New
Headers show

Commit Message

Juan Quintela Oct. 25, 2011, 2 p.m. UTC 
Use one subsection for each feature.  This means that we don't need to
bump the version field each time that a new feature gets introduced.

Introduce cpsr_vmstate field, as I am not sure if I can "use"
uncached_cpsr for saving state.

Signed-off-by: Juan Quintela <quintela@redhat.com>
CC: Paul Brook <paul@codesourcery.com>
CC: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h     |    4 +-
 target-arm/machine.c |  332 ++++++++++++++++++++++---------------------------
 2 files changed, 151 insertions(+), 185 deletions(-)

Comments

Paul Brook Oct. 25, 2011, 6:27 p.m. UTC | #1 
> -    /* Avoid mode switch when restoring CPSR.  */
> -    env->uncached_cpsr = val & CPSR_M;
> -    cpsr_write(env, val, 0xffffffff);
> +
> +    env->uncached_cpsr = env->cpsr_vmstate & CPSR_M;
> +    cpsr_write(env, env->cpsr_vmstate, 0xffffffff);

You've removed the helpful comment explaining why this wierdness exists. 
Please put it back.

A comment in cpu.h explaining what cpsr_vmstate is for would also be nice.
Specifically it's a hack[1] to transfer data between the pre_save/post_load 
hooks and the vmstate machinery.  IMO "vmstate" is a sufficiently generic term 
that it could be confused with some actual cpu register, especially once we 
get round to implementing the hypervisor extensions.

Paul

[1] Maybe a necessary hack, but still a wart from requiring static table 
driven vmstate descriptions.
Richard Henderson Oct. 25, 2011, 7:08 p.m. UTC | #2 
On 10/25/2011 11:27 AM, Paul Brook wrote:
>> -    /* Avoid mode switch when restoring CPSR.  */
>> -    env->uncached_cpsr = val & CPSR_M;
>> -    cpsr_write(env, val, 0xffffffff);
>> +
>> +    env->uncached_cpsr = env->cpsr_vmstate & CPSR_M;
>> +    cpsr_write(env, env->cpsr_vmstate, 0xffffffff);
> 
> You've removed the helpful comment explaining why this wierdness exists. 
> Please put it back.
> 
> A comment in cpu.h explaining what cpsr_vmstate is for would also be nice.
> Specifically it's a hack[1] to transfer data between the pre_save/post_load 
> hooks and the vmstate machinery.  IMO "vmstate" is a sufficiently generic term 
> that it could be confused with some actual cpu register, especially once we 
> get round to implementing the hypervisor extensions.
> 
> Paul
> 
> [1] Maybe a necessary hack, but still a wart from requiring static table 
> driven vmstate descriptions.

I wonder if we shouldn't make it easier to do what I do for Alpha
with the rather different hack for the fpcr.

That might avoid some of the other pre/post_load hooks, and the
extra data fields that requires in the cpu structure.


r~
Paul Brook Oct. 25, 2011, 8:39 p.m. UTC | #3 
> >> +    env->uncached_cpsr = env->cpsr_vmstate & CPSR_M;
> >> +    cpsr_write(env, env->cpsr_vmstate, 0xffffffff);
> > 
> > [1] Maybe a necessary hack, but still a wart from requiring static table
> > driven vmstate descriptions.
> 
> I wonder if we shouldn't make it easier to do what I do for Alpha
> with the rather different hack for the fpcr.
>
> That might avoid some of the other pre/post_load hooks, and the
> extra data fields that requires in the cpu structure.

Sounds reasonable.

Having either woraround is a bit lame.  Having ARM and Alpha use different 
workarounds for the same problem really sucks.

Paul
diff mbox

Patch

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index f6d9436..ec02f85 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -228,6 +228,8 @@  typedef struct CPUARMState {
     } cp[15];
     void *nvic;
     const struct arm_boot_info *boot_info;
+
+    uint32_t cpsr_vmstate;
 } CPUARMState;

 CPUARMState *cpu_arm_init(const char *cpu_model);
@@ -446,8 +448,6 @@  void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
 #define cpu_signal_handler cpu_arm_signal_handler
 #define cpu_list arm_cpu_list

-#define CPU_SAVE_VERSION 5
-
 /* MMU modes definitions */
 #define MMU_MODE0_SUFFIX _kernel
 #define MMU_MODE1_SUFFIX _user
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 3a3b325..b97b06b 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -1,207 +1,173 @@ 
 #include "hw/hw.h"
 #include "hw/boards.h"

-void cpu_save(QEMUFile *f, void *opaque)
+static bool feature_vfp_needed(void *opaque)
 {
-    int i;
-    CPUARMState *env = (CPUARMState *)opaque;
+    CPUState *env = opaque;

-    for (i = 0; i < 16; i++) {
-        qemu_put_be32(f, env->regs[i]);
-    }
-    qemu_put_be32(f, cpsr_read(env));
-    qemu_put_be32(f, env->spsr);
-    for (i = 0; i < 6; i++) {
-        qemu_put_be32(f, env->banked_spsr[i]);
-        qemu_put_be32(f, env->banked_r13[i]);
-        qemu_put_be32(f, env->banked_r14[i]);
-    }
-    for (i = 0; i < 5; i++) {
-        qemu_put_be32(f, env->usr_regs[i]);
-        qemu_put_be32(f, env->fiq_regs[i]);
-    }
-    qemu_put_be32(f, env->cp15.c0_cpuid);
-    qemu_put_be32(f, env->cp15.c0_cachetype);
-    qemu_put_be32(f, env->cp15.c0_cssel);
-    qemu_put_be32(f, env->cp15.c1_sys);
-    qemu_put_be32(f, env->cp15.c1_coproc);
-    qemu_put_be32(f, env->cp15.c1_xscaleauxcr);
-    qemu_put_be32(f, env->cp15.c2_base0);
-    qemu_put_be32(f, env->cp15.c2_base1);
-    qemu_put_be32(f, env->cp15.c2_control);
-    qemu_put_be32(f, env->cp15.c2_mask);
-    qemu_put_be32(f, env->cp15.c2_base_mask);
-    qemu_put_be32(f, env->cp15.c2_data);
-    qemu_put_be32(f, env->cp15.c2_insn);
-    qemu_put_be32(f, env->cp15.c3);
-    qemu_put_be32(f, env->cp15.c5_insn);
-    qemu_put_be32(f, env->cp15.c5_data);
-    for (i = 0; i < 8; i++) {
-        qemu_put_be32(f, env->cp15.c6_region[i]);
-    }
-    qemu_put_be32(f, env->cp15.c6_insn);
-    qemu_put_be32(f, env->cp15.c6_data);
-    qemu_put_be32(f, env->cp15.c7_par);
-    qemu_put_be32(f, env->cp15.c9_insn);
-    qemu_put_be32(f, env->cp15.c9_data);
-    qemu_put_be32(f, env->cp15.c9_pmcr);
-    qemu_put_be32(f, env->cp15.c9_pmcnten);
-    qemu_put_be32(f, env->cp15.c9_pmovsr);
-    qemu_put_be32(f, env->cp15.c9_pmxevtyper);
-    qemu_put_be32(f, env->cp15.c9_pmuserenr);
-    qemu_put_be32(f, env->cp15.c9_pminten);
-    qemu_put_be32(f, env->cp15.c13_fcse);
-    qemu_put_be32(f, env->cp15.c13_context);
-    qemu_put_be32(f, env->cp15.c13_tls1);
-    qemu_put_be32(f, env->cp15.c13_tls2);
-    qemu_put_be32(f, env->cp15.c13_tls3);
-    qemu_put_be32(f, env->cp15.c15_cpar);
-
-    qemu_put_be32(f, env->features);
-
-    if (arm_feature(env, ARM_FEATURE_VFP)) {
-        for (i = 0;  i < 32; i++) {
-            CPU_DoubleU u;
-            u.d = env->vfp.regs[i];
-            qemu_put_be32(f, u.l.upper);
-            qemu_put_be32(f, u.l.lower);
-        }
-        for (i = 0; i < 16; i++) {
-            qemu_put_be32(f, env->vfp.xregs[i]);
-        }
+    return arm_feature(env, ARM_FEATURE_VFP);
+}

+static const VMStateDescription vmstate_feature_vfp = {
+    .name = "feature_vfp",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_FLOAT64_ARRAY(vfp.regs, CPUState, 32),
+        VMSTATE_UINT32_ARRAY(vfp.xregs, CPUState, 16),
         /* TODO: Should use proper FPSCR access functions.  */
-        qemu_put_be32(f, env->vfp.vec_len);
-        qemu_put_be32(f, env->vfp.vec_stride);
+        VMSTATE_INT32(vfp.vec_len, CPUState),
+        VMSTATE_INT32(vfp.vec_stride, CPUState),
+        VMSTATE_END_OF_LIST()
     }
+};

-    if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
-        for (i = 0; i < 16; i++) {
-            qemu_put_be64(f, env->iwmmxt.regs[i]);
-        }
-        for (i = 0; i < 16; i++) {
-            qemu_put_be32(f, env->iwmmxt.cregs[i]);
-        }
-    }
+static bool feature_iwmmxt_needed(void *opaque)
+{
+    CPUState *env = opaque;

-    if (arm_feature(env, ARM_FEATURE_M)) {
-        qemu_put_be32(f, env->v7m.other_sp);
-        qemu_put_be32(f, env->v7m.vecbase);
-        qemu_put_be32(f, env->v7m.basepri);
-        qemu_put_be32(f, env->v7m.control);
-        qemu_put_be32(f, env->v7m.current_sp);
-        qemu_put_be32(f, env->v7m.exception);
-    }
+    return arm_feature(env, ARM_FEATURE_IWMMXT);
+}

-    if (arm_feature(env, ARM_FEATURE_THUMB2EE)) {
-        qemu_put_be32(f, env->teecr);
-        qemu_put_be32(f, env->teehbr);
+static const VMStateDescription vmstate_feature_iwmmxt = {
+    .name = "feature_iwmmxt",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT64_ARRAY(iwmmxt.regs, CPUState, 16),
+        VMSTATE_UINT32_ARRAY(iwmmxt.cregs, CPUState, 16),
+        VMSTATE_END_OF_LIST()
     }
-}
+};

-int cpu_load(QEMUFile *f, void *opaque, int version_id)
+static bool feature_m_needed(void *opaque)
 {
-    CPUARMState *env = (CPUARMState *)opaque;
-    int i;
-    uint32_t val;
+    CPUState *env = opaque;

-    if (version_id != CPU_SAVE_VERSION)
-        return -EINVAL;
+    return arm_feature(env, ARM_FEATURE_M);
+}

-    for (i = 0; i < 16; i++) {
-        env->regs[i] = qemu_get_be32(f);
-    }
-    val = qemu_get_be32(f);
-    /* Avoid mode switch when restoring CPSR.  */
-    env->uncached_cpsr = val & CPSR_M;
-    cpsr_write(env, val, 0xffffffff);
-    env->spsr = qemu_get_be32(f);
-    for (i = 0; i < 6; i++) {
-        env->banked_spsr[i] = qemu_get_be32(f);
-        env->banked_r13[i] = qemu_get_be32(f);
-        env->banked_r14[i] = qemu_get_be32(f);
-    }
-    for (i = 0; i < 5; i++) {
-        env->usr_regs[i] = qemu_get_be32(f);
-        env->fiq_regs[i] = qemu_get_be32(f);
-    }
-    env->cp15.c0_cpuid = qemu_get_be32(f);
-    env->cp15.c0_cachetype = qemu_get_be32(f);
-    env->cp15.c0_cssel = qemu_get_be32(f);
-    env->cp15.c1_sys = qemu_get_be32(f);
-    env->cp15.c1_coproc = qemu_get_be32(f);
-    env->cp15.c1_xscaleauxcr = qemu_get_be32(f);
-    env->cp15.c2_base0 = qemu_get_be32(f);
-    env->cp15.c2_base1 = qemu_get_be32(f);
-    env->cp15.c2_control = qemu_get_be32(f);
-    env->cp15.c2_mask = qemu_get_be32(f);
-    env->cp15.c2_base_mask = qemu_get_be32(f);
-    env->cp15.c2_data = qemu_get_be32(f);
-    env->cp15.c2_insn = qemu_get_be32(f);
-    env->cp15.c3 = qemu_get_be32(f);
-    env->cp15.c5_insn = qemu_get_be32(f);
-    env->cp15.c5_data = qemu_get_be32(f);
-    for (i = 0; i < 8; i++) {
-        env->cp15.c6_region[i] = qemu_get_be32(f);
-    }
-    env->cp15.c6_insn = qemu_get_be32(f);
-    env->cp15.c6_data = qemu_get_be32(f);
-    env->cp15.c7_par = qemu_get_be32(f);
-    env->cp15.c9_insn = qemu_get_be32(f);
-    env->cp15.c9_data = qemu_get_be32(f);
-    env->cp15.c9_pmcr = qemu_get_be32(f);
-    env->cp15.c9_pmcnten = qemu_get_be32(f);
-    env->cp15.c9_pmovsr = qemu_get_be32(f);
-    env->cp15.c9_pmxevtyper = qemu_get_be32(f);
-    env->cp15.c9_pmuserenr = qemu_get_be32(f);
-    env->cp15.c9_pminten = qemu_get_be32(f);
-    env->cp15.c13_fcse = qemu_get_be32(f);
-    env->cp15.c13_context = qemu_get_be32(f);
-    env->cp15.c13_tls1 = qemu_get_be32(f);
-    env->cp15.c13_tls2 = qemu_get_be32(f);
-    env->cp15.c13_tls3 = qemu_get_be32(f);
-    env->cp15.c15_cpar = qemu_get_be32(f);
-
-    env->features = qemu_get_be32(f);
-
-    if (arm_feature(env, ARM_FEATURE_VFP)) {
-        for (i = 0;  i < 32; i++) {
-            CPU_DoubleU u;
-            u.l.upper = qemu_get_be32(f);
-            u.l.lower = qemu_get_be32(f);
-            env->vfp.regs[i] = u.d;
-        }
-        for (i = 0; i < 16; i++) {
-            env->vfp.xregs[i] = qemu_get_be32(f);
-        }
+static const VMStateDescription vmstate_feature_m = {
+    .name = "feature_m",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT32(v7m.other_sp, CPUState),
+        VMSTATE_UINT32(v7m.vecbase, CPUState),
+        VMSTATE_UINT32(v7m.basepri, CPUState),
+        VMSTATE_UINT32(v7m.control, CPUState),
+        VMSTATE_INT32(v7m.current_sp, CPUState),
+        VMSTATE_INT32(v7m.exception, CPUState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static bool feature_thumb2ee_needed(void *opaque)
+{
+    CPUState *env = opaque;

-        /* TODO: Should use proper FPSCR access functions.  */
-        env->vfp.vec_len = qemu_get_be32(f);
-        env->vfp.vec_stride = qemu_get_be32(f);
-    }
+    return arm_feature(env, ARM_FEATURE_THUMB2EE);
+}

-    if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
-        for (i = 0; i < 16; i++) {
-            env->iwmmxt.regs[i] = qemu_get_be64(f);
-        }
-        for (i = 0; i < 16; i++) {
-            env->iwmmxt.cregs[i] = qemu_get_be32(f);
-        }
+static const VMStateDescription vmstate_feature_thumb2ee = {
+    .name = "feature_thumb2ee",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT32(teecr, CPUState),
+        VMSTATE_UINT32(teehbr, CPUState),
+        VMSTATE_END_OF_LIST()
     }
+};

-    if (arm_feature(env, ARM_FEATURE_M)) {
-        env->v7m.other_sp = qemu_get_be32(f);
-        env->v7m.vecbase = qemu_get_be32(f);
-        env->v7m.basepri = qemu_get_be32(f);
-        env->v7m.control = qemu_get_be32(f);
-        env->v7m.current_sp = qemu_get_be32(f);
-        env->v7m.exception = qemu_get_be32(f);
-    }
+static void cpu_pre_save(void *opaque)
+{
+    CPUState *env = opaque;

-    if (arm_feature(env, ARM_FEATURE_THUMB2EE)) {
-        env->teecr = qemu_get_be32(f);
-        env->teehbr = qemu_get_be32(f);
-    }
+    env->cpsr_vmstate = cpsr_read(env);
+}

+static int cpu_post_load(void *opaque, int version_id)
+{
+    CPUState *env = opaque;
+
+    env->uncached_cpsr = env->cpsr_vmstate & CPSR_M;
+    cpsr_write(env, env->cpsr_vmstate, 0xffffffff);
     return 0;
 }
+
+const VMStateDescription vmstate_cpu = {
+    .name = "cpu",
+    .version_id = 6,
+    .minimum_version_id = 6,
+    .minimum_version_id_old = 6,
+    .pre_save = cpu_pre_save,
+    .post_load = cpu_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(regs, CPUState, 16),
+        VMSTATE_UINT32(cpsr_vmstate, CPUState),
+        VMSTATE_UINT32(spsr, CPUState),
+        VMSTATE_UINT32_ARRAY(banked_spsr, CPUState, 6),
+        VMSTATE_UINT32_ARRAY(banked_r13, CPUState, 6),
+        VMSTATE_UINT32_ARRAY(banked_r14, CPUState, 6),
+        VMSTATE_UINT32_ARRAY(usr_regs, CPUState, 5),
+        VMSTATE_UINT32_ARRAY(fiq_regs, CPUState, 5),
+        VMSTATE_UINT32(cp15.c0_cpuid, CPUState),
+        VMSTATE_UINT32(cp15.c0_cachetype, CPUState),
+        VMSTATE_UINT32(cp15.c0_cssel, CPUState),
+        VMSTATE_UINT32(cp15.c1_sys, CPUState),
+        VMSTATE_UINT32(cp15.c1_coproc, CPUState),
+        VMSTATE_UINT32(cp15.c1_xscaleauxcr, CPUState),
+        VMSTATE_UINT32(cp15.c2_base0, CPUState),
+        VMSTATE_UINT32(cp15.c2_base1, CPUState),
+        VMSTATE_UINT32(cp15.c2_control, CPUState),
+        VMSTATE_UINT32(cp15.c2_mask, CPUState),
+        VMSTATE_UINT32(cp15.c2_base_mask, CPUState),
+        VMSTATE_UINT32(cp15.c2_data, CPUState),
+        VMSTATE_UINT32(cp15.c2_insn, CPUState),
+        VMSTATE_UINT32(cp15.c3, CPUState),
+        VMSTATE_UINT32(cp15.c5_insn, CPUState),
+        VMSTATE_UINT32(cp15.c5_data, CPUState),
+        VMSTATE_UINT32_ARRAY(cp15.c6_region, CPUState, 8),
+        VMSTATE_UINT32(cp15.c6_insn, CPUState),
+        VMSTATE_UINT32(cp15.c6_data, CPUState),
+        VMSTATE_UINT32(cp15.c7_par, CPUState),
+        VMSTATE_UINT32(cp15.c9_insn, CPUState),
+        VMSTATE_UINT32(cp15.c9_data, CPUState),
+        VMSTATE_UINT32(cp15.c9_pmcr, CPUState),
+        VMSTATE_UINT32(cp15.c9_pmcnten, CPUState),
+        VMSTATE_UINT32(cp15.c9_pmovsr, CPUState),
+        VMSTATE_UINT32(cp15.c9_pmxevtyper, CPUState),
+        VMSTATE_UINT32(cp15.c9_pmuserenr, CPUState),
+        VMSTATE_UINT32(cp15.c9_pminten, CPUState),
+        VMSTATE_UINT32(cp15.c13_fcse, CPUState),
+        VMSTATE_UINT32(cp15.c13_context, CPUState),
+        VMSTATE_UINT32(cp15.c13_tls1, CPUState),
+        VMSTATE_UINT32(cp15.c13_tls2, CPUState),
+        VMSTATE_UINT32(cp15.c13_tls3, CPUState),
+        VMSTATE_UINT32(cp15.c15_cpar, CPUState),
+        VMSTATE_UINT32(features, CPUState),
+        VMSTATE_END_OF_LIST()
+    },
+    .subsections = (VMStateSubsection[]) {
+        {
+            .vmsd = &vmstate_feature_vfp,
+            .needed = feature_vfp_needed,
+        } , {
+            .vmsd = &vmstate_feature_iwmmxt,
+            .needed = feature_iwmmxt_needed,
+        } , {
+            .vmsd = &vmstate_feature_m,
+            .needed = feature_m_needed,
+        } , {
+            .vmsd = &vmstate_feature_thumb2ee,
+            .needed = feature_thumb2ee_needed,
+        } , {
+            /* empty */
+        }
+    }
+};