diff mbox

[13/25] vmstate: port ppc cpu

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

Commit Message

Juan Quintela Oct. 25, 2011, 2 p.m. UTC 
Added sdr1_vmstate because storing the value requires calling ppc_store_sdr1().
The position when the function is called also changes (I think it is save).

Signed-off-by: Juan Quintela <quintela@redhat.com>
CC: Alexander Graf <agraf@suse.de>
---
 target-ppc/cpu.h     |    4 +-
 target-ppc/machine.c |  245 ++++++++++++++++++--------------------------------
 2 files changed, 89 insertions(+), 160 deletions(-)

Comments

Alexander Graf Oct. 30, 2011, 4:36 p.m. UTC | #1 
On 25.10.2011, at 16:00, Juan Quintela wrote:

> Added sdr1_vmstate because storing the value requires calling ppc_store_sdr1().
> The position when the function is called also changes (I think it is save).

Thanks for converting this. I'm fairly sure that vmsave is broken atm anyways and that someone will have to go through all the fields and make sure they're still working properly. However you're definitely not making the situation worse, so here's only a small comment:

> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> CC: Alexander Graf <agraf@suse.de>
> ---
> target-ppc/cpu.h     |    4 +-
> target-ppc/machine.c |  245 ++++++++++++++++++--------------------------------
> 2 files changed, 89 insertions(+), 160 deletions(-)
> 
> +        VMSTATE_UINTTL_ARRAY(spr, CPUState, 1024),

Shouldn't sizeof(((CPUState*)NULL)->spr / sizeof(target_ulong)) work as size field?

Alex
Juan Quintela Oct. 31, 2011, 7:43 p.m. UTC | #2 
Alexander Graf <agraf@suse.de> wrote:
> On 25.10.2011, at 16:00, Juan Quintela wrote:
>
>> Added sdr1_vmstate because storing the value requires calling ppc_store_sdr1().
>> The position when the function is called also changes (I think it is save).
>
> Thanks for converting this. I'm fairly sure that vmsave is broken atm anyways and that someone will have to go through all the fields and make sure they're still working properly. However you're definitely not making the situation worse, so here's only a small comment:
>
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> CC: Alexander Graf <agraf@suse.de>
>> ---
>> target-ppc/cpu.h     |    4 +-
>> target-ppc/machine.c |  245 ++++++++++++++++++--------------------------------
>> 2 files changed, 89 insertions(+), 160 deletions(-)
>> 
>> +        VMSTATE_UINTTL_ARRAY(spr, CPUState, 1024),
>
> Shouldn't sizeof(((CPUState*)NULL)->spr / sizeof(target_ulong)) work as size field?

vmstate is inconsistent about this.

It "calculates" sizes for "strings" (ar uint8_t * if your preffer), but
not for arrays.

#define VMSTATE_BUFFER_V(_f, _s, _v)                                  \
    VMSTATE_STATIC_BUFFER(_f, _s, _v, NULL, 0, sizeof(typeof_field(_s, _f)))

#define VMSTATE_BUFFER(_f, _s)                                        \
    VMSTATE_BUFFER_V(_f, _s, 0)

And I don't know what the "right" solution is:
- Putting explicit size helps detect changes on that size (good)
- But, on lots of places, we are putting there a constant:
        VMSTATE_INT32_ARRAY(tx_fifo, smc91c111_state, NUM_PACKETS),
  So, we are not going to detect changes either.

Next on my plate is to get someway of unittesting and being able to
detect this kind of changes (then we can actually drop the sizes
always).

Later, Juan.
diff mbox

Patch

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 3f77e30..e7f6625 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1029,6 +1029,8 @@  struct CPUPPCState {
      */
     uint8_t fit_period[4];
     uint8_t wdt_period[4];
+
+    target_ulong sdr1_vmstate;
 };

 #define SET_FIT_PERIOD(a_, b_, c_, d_)          \
@@ -1183,8 +1185,6 @@  int ppc_dcr_write (ppc_dcr_t *dcr_env, int dcrn, uint32_t val);
 #define cpu_signal_handler cpu_ppc_signal_handler
 #define cpu_list ppc_cpu_list

-#define CPU_SAVE_VERSION 4
-
 /* MMU modes definitions */
 #define MMU_MODE0_SUFFIX _user
 #define MMU_MODE1_SUFFIX _kernel
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index 226b4a5..8d3cd86 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -2,172 +2,101 @@ 
 #include "hw/boards.h"
 #include "kvm.h"

-void cpu_save(QEMUFile *f, void *opaque)
+static const VMStateDescription vmstate_tlb = {
+    .name = "tlb",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINTTL(pte0, ppc6xx_tlb_t),
+        VMSTATE_UINTTL(pte1, ppc6xx_tlb_t),
+        VMSTATE_UINTTL(EPN, ppc6xx_tlb_t),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static bool is_tlb6(void *opaque, int version_id)
 {
-    CPUState *env = (CPUState *)opaque;
-    unsigned int i, j;
+    CPUState *env = opaque;

-    for (i = 0; i < 32; i++)
-        qemu_put_betls(f, &env->gpr[i]);
-#if !defined(TARGET_PPC64)
-    for (i = 0; i < 32; i++)
-        qemu_put_betls(f, &env->gprh[i]);
-#endif
-    qemu_put_betls(f, &env->lr);
-    qemu_put_betls(f, &env->ctr);
-    for (i = 0; i < 8; i++)
-        qemu_put_be32s(f, &env->crf[i]);
-    qemu_put_betls(f, &env->xer);
-    qemu_put_betls(f, &env->reserve_addr);
-    qemu_put_betls(f, &env->msr);
-    for (i = 0; i < 4; i++)
-        qemu_put_betls(f, &env->tgpr[i]);
-    for (i = 0; i < 32; i++) {
-        union {
-            float64 d;
-            uint64_t l;
-        } u;
-        u.d = env->fpr[i];
-        qemu_put_be64(f, u.l);
-    }
-    qemu_put_be32s(f, &env->fpscr);
-    qemu_put_sbe32s(f, &env->access_type);
-#if defined(TARGET_PPC64)
-    qemu_put_betls(f, &env->asr);
-    qemu_put_sbe32s(f, &env->slb_nr);
-#endif
-    qemu_put_betls(f, &env->spr[SPR_SDR1]);
-    for (i = 0; i < 32; i++)
-        qemu_put_betls(f, &env->sr[i]);
-    for (i = 0; i < 2; i++)
-        for (j = 0; j < 8; j++)
-            qemu_put_betls(f, &env->DBAT[i][j]);
-    for (i = 0; i < 2; i++)
-        for (j = 0; j < 8; j++)
-            qemu_put_betls(f, &env->IBAT[i][j]);
-    qemu_put_sbe32s(f, &env->nb_tlb);
-    qemu_put_sbe32s(f, &env->tlb_per_way);
-    qemu_put_sbe32s(f, &env->nb_ways);
-    qemu_put_sbe32s(f, &env->last_way);
-    qemu_put_sbe32s(f, &env->id_tlbs);
-    qemu_put_sbe32s(f, &env->nb_pids);
-    if (env->tlb.tlb6) {
-        // XXX assumes 6xx
-        for (i = 0; i < env->nb_tlb; i++) {
-            qemu_put_betls(f, &env->tlb.tlb6[i].pte0);
-            qemu_put_betls(f, &env->tlb.tlb6[i].pte1);
-            qemu_put_betls(f, &env->tlb.tlb6[i].EPN);
-        }
-    }
-    for (i = 0; i < 4; i++)
-        qemu_put_betls(f, &env->pb[i]);
-    for (i = 0; i < 1024; i++)
-        qemu_put_betls(f, &env->spr[i]);
-    qemu_put_be32s(f, &env->vscr);
-    qemu_put_be64s(f, &env->spe_acc);
-    qemu_put_be32s(f, &env->spe_fscr);
-    qemu_put_betls(f, &env->msr_mask);
-    qemu_put_be32s(f, &env->flags);
-    qemu_put_sbe32s(f, &env->error_code);
-    qemu_put_be32s(f, &env->pending_interrupts);
-    qemu_put_be32s(f, &env->irq_input_state);
-    for (i = 0; i < POWERPC_EXCP_NB; i++)
-        qemu_put_betls(f, &env->excp_vectors[i]);
-    qemu_put_betls(f, &env->excp_prefix);
-    qemu_put_betls(f, &env->hreset_excp_prefix);
-    qemu_put_betls(f, &env->ivor_mask);
-    qemu_put_betls(f, &env->ivpr_mask);
-    qemu_put_betls(f, &env->hreset_vector);
-    qemu_put_betls(f, &env->nip);
-    qemu_put_betls(f, &env->hflags);
-    qemu_put_betls(f, &env->hflags_nmsr);
-    qemu_put_sbe32s(f, &env->mmu_idx);
-    qemu_put_sbe32s(f, &env->power_mode);
+    return (env->tlb.tlb6 != NULL);
 }

-int cpu_load(QEMUFile *f, void *opaque, int version_id)
+static void cpu_pre_save(void *opaque)
 {
-    CPUState *env = (CPUState *)opaque;
-    unsigned int i, j;
-    target_ulong sdr1;
+    CPUState *env = opaque;
+    env->sdr1_vmstate = env->spr[SPR_SDR1];
+}

-    for (i = 0; i < 32; i++)
-        qemu_get_betls(f, &env->gpr[i]);
+static int cpu_post_load(void *opaque, int version_id)
+{
+    CPUState *env = opaque;
+    ppc_store_sdr1(env, env->sdr1_vmstate);
+    return 0;
+}
+
+const VMStateDescription vmstate_cpu = {
+    .name = "cpu",
+    .version_id = 4,
+    .minimum_version_id = 4,
+    .minimum_version_id_old = 4,
+    .pre_save = cpu_pre_save,
+    .post_load = cpu_post_load,
+   .fields      = (VMStateField[]) {
+        VMSTATE_UINTTL_ARRAY(gpr, CPUState, 32),
 #if !defined(TARGET_PPC64)
-    for (i = 0; i < 32; i++)
-        qemu_get_betls(f, &env->gprh[i]);
+        VMSTATE_UINTTL_ARRAY(gprh, CPUState, 32),
 #endif
-    qemu_get_betls(f, &env->lr);
-    qemu_get_betls(f, &env->ctr);
-    for (i = 0; i < 8; i++)
-        qemu_get_be32s(f, &env->crf[i]);
-    qemu_get_betls(f, &env->xer);
-    qemu_get_betls(f, &env->reserve_addr);
-    qemu_get_betls(f, &env->msr);
-    for (i = 0; i < 4; i++)
-        qemu_get_betls(f, &env->tgpr[i]);
-    for (i = 0; i < 32; i++) {
-        union {
-            float64 d;
-            uint64_t l;
-        } u;
-        u.l = qemu_get_be64(f);
-        env->fpr[i] = u.d;
-    }
-    qemu_get_be32s(f, &env->fpscr);
-    qemu_get_sbe32s(f, &env->access_type);
+        VMSTATE_UINTTL(lr, CPUState),
+        VMSTATE_UINTTL(ctr, CPUState),
+        VMSTATE_UINT32_ARRAY(crf, CPUState, 8),
+        VMSTATE_UINTTL(xer, CPUState),
+        VMSTATE_UINTTL(reserve_addr, CPUState),
+        VMSTATE_UINTTL(msr, CPUState),
+        VMSTATE_UINTTL_ARRAY(tgpr, CPUState, 4),
+        VMSTATE_FLOAT64_ARRAY(fpr, CPUState, 32),
+        VMSTATE_UINT32(fpscr, CPUState),
+        VMSTATE_INT32(access_type, CPUState),
 #if defined(TARGET_PPC64)
-    qemu_get_betls(f, &env->asr);
-    qemu_get_sbe32s(f, &env->slb_nr);
+        VMSTATE_UINTTL(asr, CPUState),
+        VMSTATE_INT32(slb_nr, CPUState),
 #endif
-    qemu_get_betls(f, &sdr1);
-    for (i = 0; i < 32; i++)
-        qemu_get_betls(f, &env->sr[i]);
-    for (i = 0; i < 2; i++)
-        for (j = 0; j < 8; j++)
-            qemu_get_betls(f, &env->DBAT[i][j]);
-    for (i = 0; i < 2; i++)
-        for (j = 0; j < 8; j++)
-            qemu_get_betls(f, &env->IBAT[i][j]);
-    qemu_get_sbe32s(f, &env->nb_tlb);
-    qemu_get_sbe32s(f, &env->tlb_per_way);
-    qemu_get_sbe32s(f, &env->nb_ways);
-    qemu_get_sbe32s(f, &env->last_way);
-    qemu_get_sbe32s(f, &env->id_tlbs);
-    qemu_get_sbe32s(f, &env->nb_pids);
-    if (env->tlb.tlb6) {
-        // XXX assumes 6xx
-        for (i = 0; i < env->nb_tlb; i++) {
-            qemu_get_betls(f, &env->tlb.tlb6[i].pte0);
-            qemu_get_betls(f, &env->tlb.tlb6[i].pte1);
-            qemu_get_betls(f, &env->tlb.tlb6[i].EPN);
-        }
-    }
-    for (i = 0; i < 4; i++)
-        qemu_get_betls(f, &env->pb[i]);
-    for (i = 0; i < 1024; i++)
-        qemu_get_betls(f, &env->spr[i]);
-    ppc_store_sdr1(env, sdr1);
-    qemu_get_be32s(f, &env->vscr);
-    qemu_get_be64s(f, &env->spe_acc);
-    qemu_get_be32s(f, &env->spe_fscr);
-    qemu_get_betls(f, &env->msr_mask);
-    qemu_get_be32s(f, &env->flags);
-    qemu_get_sbe32s(f, &env->error_code);
-    qemu_get_be32s(f, &env->pending_interrupts);
-    qemu_get_be32s(f, &env->irq_input_state);
-    for (i = 0; i < POWERPC_EXCP_NB; i++)
-        qemu_get_betls(f, &env->excp_vectors[i]);
-    qemu_get_betls(f, &env->excp_prefix);
-    qemu_get_betls(f, &env->hreset_excp_prefix);
-    qemu_get_betls(f, &env->ivor_mask);
-    qemu_get_betls(f, &env->ivpr_mask);
-    qemu_get_betls(f, &env->hreset_vector);
-    qemu_get_betls(f, &env->nip);
-    qemu_get_betls(f, &env->hflags);
-    qemu_get_betls(f, &env->hflags_nmsr);
-    qemu_get_sbe32s(f, &env->mmu_idx);
-    qemu_get_sbe32s(f, &env->power_mode);
+        VMSTATE_UINTTL(spr[SPR_SDR1], CPUState),
+        VMSTATE_UINTTL_ARRAY(sr, CPUState, 32),
+        VMSTATE_UINTTL_ARRAY(DBAT[0], CPUState, 8),
+        VMSTATE_UINTTL_ARRAY(DBAT[1], CPUState, 8),
+        VMSTATE_UINTTL_ARRAY(IBAT[0], CPUState, 8),
+        VMSTATE_UINTTL_ARRAY(IBAT[1], CPUState, 8),
+        VMSTATE_INT32(nb_tlb, CPUState),
+        VMSTATE_INT32(tlb_per_way, CPUState),
+        VMSTATE_INT32(nb_ways, CPUState),
+        VMSTATE_INT32(last_way, CPUState),
+        VMSTATE_INT32(id_tlbs, CPUState),
+        VMSTATE_INT32(nb_pids, CPUState),
+        VMSTATE_STRUCT_VARRAY_INT32_TEST(tlb.tlb6, CPUState, nb_tlb,
+                                         is_tlb6, vmstate_tlb, ppc6xx_tlb_t),
+        VMSTATE_UINTTL_ARRAY(pb, CPUState, 4),
+        VMSTATE_UINTTL_ARRAY(spr, CPUState, 1024),
+        VMSTATE_UINT32(vscr, CPUState),
+        VMSTATE_UINT64(spe_acc, CPUState),
+        VMSTATE_UINT32(spe_fscr, CPUState),
+        VMSTATE_UINTTL(msr_mask, CPUState),
+        VMSTATE_UINT32(flags, CPUState),
+        VMSTATE_INT32(error_code, CPUState),
+        VMSTATE_UINT32(pending_interrupts, CPUState),
+        VMSTATE_UINT32(irq_input_state, CPUState),
+        VMSTATE_UINTTL_ARRAY(excp_vectors, CPUState, POWERPC_EXCP_NB),
+        VMSTATE_UINTTL(excp_prefix, CPUState),
+        VMSTATE_UINTTL(hreset_excp_prefix, CPUState),
+        VMSTATE_UINTTL(ivor_mask, CPUState),
+        VMSTATE_UINTTL(ivpr_mask, CPUState),
+        VMSTATE_UINTTL(hreset_vector, CPUState),
+        VMSTATE_UINTTL(nip, CPUState),
+        VMSTATE_UINTTL(hflags, CPUState),
+        VMSTATE_UINTTL(hflags_nmsr, CPUState),
+        VMSTATE_INT32(mmu_idx, CPUState),
+        VMSTATE_INT32(power_mode, CPUState),
+        VMSTATE_END_OF_LIST()
+    },
+};

-    return 0;
-}