diff mbox

[23/35] vmstate: port arm cpu

Message ID 5a3abb34f8a5c286a12d6d067e11b14930757825.1336128412.git.quintela@redhat.com
State New
Headers show

Commit Message

Juan Quintela May 4, 2012, 10:54 a.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>
---
 target-arm/cpu.h     |    5 +-
 target-arm/machine.c |  344 ++++++++++++++++++++++----------------------------
 2 files changed, 156 insertions(+), 193 deletions(-)

Comments

Peter Maydell May 4, 2012, 1:04 p.m. UTC | #1 
On 4 May 2012 11:54, Juan Quintela <quintela@redhat.com> wrote:
> 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>
> ---
>  target-arm/cpu.h     |    5 +-
>  target-arm/machine.c |  344 ++++++++++++++++++++++----------------------------
>  2 files changed, 156 insertions(+), 193 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 9434902..37744c6 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -236,6 +236,9 @@ typedef struct CPUARMState {
>     } cp[15];
>     void *nvic;
>     const struct arm_boot_info *boot_info;
> +
> +    /* Fields needed as intermediate for vmstate */
> +    uint32_t cpsr_vmstate;
>  } CPUARMState;

I still think this is the wrong approach. We need to support
"this is how you read/write this field" functions. See also
target-alpha handling of the fpcr.

-- PMM
Juan Quintela May 4, 2012, 3:28 p.m. UTC | #2 
Peter Maydell <peter.maydell@linaro.org> wrote:
> On 4 May 2012 11:54, Juan Quintela <quintela@redhat.com> wrote:
>> 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>
>> ---
>>  target-arm/cpu.h     |    5 +-
>>  target-arm/machine.c |  344 ++++++++++++++++++++++----------------------------
>>  2 files changed, 156 insertions(+), 193 deletions(-)
>>
>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>> index 9434902..37744c6 100644
>> --- a/target-arm/cpu.h
>> +++ b/target-arm/cpu.h
>> @@ -236,6 +236,9 @@ typedef struct CPUARMState {
>>     } cp[15];
>>     void *nvic;
>>     const struct arm_boot_info *boot_info;
>> +
>> +    /* Fields needed as intermediate for vmstate */
>> +    uint32_t cpsr_vmstate;
>>  } CPUARMState;
>
> I still think this is the wrong approach. We need to support
> "this is how you read/write this field" functions. See also
> target-alpha handling of the fpcr.

It is easy to change to support that (see alpha example), but then, each
time that we change protocol format (and we have a pending change for
1.2), every user that is "like" uint64_t, but with accesor and setter,
needs to get a new change.  This other way (with pre_save/post_load)
instead makes trivial to change protocols.

So, we can have "cleaniness" or "flexibility", take one.
I still think that alpha should be changed to this other approach, for
the same reason.

Notice that we _know_ that what we sent is an uint32_t, it is how we get
that uint32_t what changes, that is why I preffer a new field and
pre_load/post_load functions.

The only other sane approach that I can think is changing:

#define VMSTATE_INT32_V(_f, _s, _v)                                   \

into something like:

VMSTATE_INT32_GENERAL(_f, _s, _v, _getter, _setter)

and then _getter/_setter functions for normal ints the obvious copy this
uint32_t?

Would that work for you?



Later, Juan.
Peter Maydell May 4, 2012, 3:37 p.m. UTC | #3 
On 4 May 2012 16:28, Juan Quintela <quintela@redhat.com> wrote:
> The only other sane approach that I can think is changing:
>
> #define VMSTATE_INT32_V(_f, _s, _v)                                   \
>
> into something like:
>
> VMSTATE_INT32_GENERAL(_f, _s, _v, _getter, _setter)
>
> and then _getter/_setter functions for normal ints the obvious copy this
> uint32_t?
>
> Would that work for you?

Yes, I'd be happy with that approach: it's the general idea of
"here's a function to get/set the value" that I like in the alpha
code, not the specific implementation (which is a bit of an ugly
hack).

-- PMM
diff mbox

Patch

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 9434902..37744c6 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -236,6 +236,9 @@  typedef struct CPUARMState {
     } cp[15];
     void *nvic;
     const struct arm_boot_info *boot_info;
+
+    /* Fields needed as intermediate for vmstate */
+    uint32_t cpsr_vmstate;
 } CPUARMState;

 #include "cpu-qom.h"
@@ -464,8 +467,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 7
-
 /* 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 9c0f773..31e49ac 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -1,215 +1,177 @@ 
 #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;
+    CPUARMState *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.c1_scr);
-    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->cp15.c15_power_control);
-    qemu_put_be32(f, env->cp15.c15_diagnostic);
-    qemu_put_be32(f, env->cp15.c15_power_diagnostic);
-
-    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, CPUARMState, 32),
+        VMSTATE_UINT32_ARRAY(vfp.xregs, CPUARMState, 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, CPUARMState),
+        VMSTATE_INT32(vfp.vec_stride, CPUARMState),
+        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)
+{
+    CPUARMState *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, CPUARMState, 16),
+        VMSTATE_UINT32_ARRAY(iwmmxt.cregs, CPUARMState, 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;
+    CPUARMState *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.c1_scr = 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->cp15.c15_power_control = qemu_get_be32(f);
-    env->cp15.c15_diagnostic = qemu_get_be32(f);
-    env->cp15.c15_power_diagnostic = 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, CPUARMState),
+        VMSTATE_UINT32(v7m.vecbase, CPUARMState),
+        VMSTATE_UINT32(v7m.basepri, CPUARMState),
+        VMSTATE_UINT32(v7m.control, CPUARMState),
+        VMSTATE_INT32(v7m.current_sp, CPUARMState),
+        VMSTATE_INT32(v7m.exception, CPUARMState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static bool feature_thumb2ee_needed(void *opaque)
+{
+    CPUARMState *env = opaque;
+    return arm_feature(env, ARM_FEATURE_THUMB2EE);
+}

-        /* TODO: Should use proper FPSCR access functions.  */
-        env->vfp.vec_len = qemu_get_be32(f);
-        env->vfp.vec_stride = 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, CPUARMState),
+        VMSTATE_UINT32(teehbr, CPUARMState),
+        VMSTATE_END_OF_LIST()
     }
+};

-    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 void cpu_pre_save(void *opaque)
+{
+    CPUARMState *env = opaque;

-    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);
-    }
+    env->cpsr_vmstate = cpsr_read(env);
+}

-    if (arm_feature(env, ARM_FEATURE_THUMB2EE)) {
-        env->teecr = qemu_get_be32(f);
-        env->teehbr = qemu_get_be32(f);
-    }
+static int cpu_post_load(void *opaque, int version_id)
+{
+    CPUARMState *env = opaque;

+    /* Avoid mode switch when restoring CPSR.  */
+    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 = 7,
+    .minimum_version_id = 7,
+    .minimum_version_id_old = 7,
+    .pre_save = cpu_pre_save,
+    .post_load = cpu_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(regs, CPUARMState, 16),
+        VMSTATE_UINT32(cpsr_vmstate, CPUARMState),
+        VMSTATE_UINT32(spsr, CPUARMState),
+        VMSTATE_UINT32_ARRAY(banked_spsr, CPUARMState, 6),
+        VMSTATE_UINT32_ARRAY(banked_r13, CPUARMState, 6),
+        VMSTATE_UINT32_ARRAY(banked_r14, CPUARMState, 6),
+        VMSTATE_UINT32_ARRAY(usr_regs, CPUARMState, 5),
+        VMSTATE_UINT32_ARRAY(fiq_regs, CPUARMState, 5),
+        VMSTATE_UINT32(cp15.c0_cpuid, CPUARMState),
+        VMSTATE_UINT32(cp15.c0_cachetype, CPUARMState),
+        VMSTATE_UINT32(cp15.c0_cssel, CPUARMState),
+        VMSTATE_UINT32(cp15.c1_sys, CPUARMState),
+        VMSTATE_UINT32(cp15.c1_coproc, CPUARMState),
+        VMSTATE_UINT32(cp15.c1_xscaleauxcr, CPUARMState),
+        VMSTATE_UINT32(cp15.c1_scr, CPUARMState),
+        VMSTATE_UINT32(cp15.c2_base0, CPUARMState),
+        VMSTATE_UINT32(cp15.c2_base1, CPUARMState),
+        VMSTATE_UINT32(cp15.c2_control, CPUARMState),
+        VMSTATE_UINT32(cp15.c2_mask, CPUARMState),
+        VMSTATE_UINT32(cp15.c2_base_mask, CPUARMState),
+        VMSTATE_UINT32(cp15.c2_data, CPUARMState),
+        VMSTATE_UINT32(cp15.c2_insn, CPUARMState),
+        VMSTATE_UINT32(cp15.c3, CPUARMState),
+        VMSTATE_UINT32(cp15.c5_insn, CPUARMState),
+        VMSTATE_UINT32(cp15.c5_data, CPUARMState),
+        VMSTATE_UINT32_ARRAY(cp15.c6_region, CPUARMState, 8),
+        VMSTATE_UINT32(cp15.c6_insn, CPUARMState),
+        VMSTATE_UINT32(cp15.c6_data, CPUARMState),
+        VMSTATE_UINT32(cp15.c7_par, CPUARMState),
+        VMSTATE_UINT32(cp15.c9_insn, CPUARMState),
+        VMSTATE_UINT32(cp15.c9_data, CPUARMState),
+        VMSTATE_UINT32(cp15.c9_pmcr, CPUARMState),
+        VMSTATE_UINT32(cp15.c9_pmcnten, CPUARMState),
+        VMSTATE_UINT32(cp15.c9_pmovsr, CPUARMState),
+        VMSTATE_UINT32(cp15.c9_pmxevtyper, CPUARMState),
+        VMSTATE_UINT32(cp15.c9_pmuserenr, CPUARMState),
+        VMSTATE_UINT32(cp15.c9_pminten, CPUARMState),
+        VMSTATE_UINT32(cp15.c13_fcse, CPUARMState),
+        VMSTATE_UINT32(cp15.c13_context, CPUARMState),
+        VMSTATE_UINT32(cp15.c13_tls1, CPUARMState),
+        VMSTATE_UINT32(cp15.c13_tls2, CPUARMState),
+        VMSTATE_UINT32(cp15.c13_tls3, CPUARMState),
+        VMSTATE_UINT32(cp15.c15_cpar, CPUARMState),
+        VMSTATE_UINT32(cp15.c15_power_control, CPUARMState),
+        VMSTATE_UINT32(cp15.c15_diagnostic, CPUARMState),
+        VMSTATE_UINT32(cp15.c15_power_diagnostic, CPUARMState),
+        VMSTATE_UINT32(features, CPUARMState),
+        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 */
+        }
+    }
+};