Re: [Qemu-devel] [PATCH 13/36] vmstate: port ppc cpu

[Andreas Färber]

Am 19.03.2012 23:57, schrieb Juan Quintela:
> 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 <address@hidden>
> ---
>  target-ppc/cpu.h     |    5 +-
>  target-ppc/machine.c |  245 
> ++++++++++++++++++--------------------------------
>  2 files changed, 90 insertions(+), 160 deletions(-)

I do not feel comfortable ack'ing this change, and there's CCs missing.

David or Scott, are you familiar with this part of code? Otherwise we'll
need to defer it to next week when Alex is back. Series seems fairly
self-contained, so the Soft Freeze shouldn't be a problem.

Note that this slightly conflicts with CPU QOM'ification, where the goal
is to move fields out of CPUPPCState and into PowerPCCPU. But that could
be adjusted in a follow-up. Probably applies to other patches in the
series, too.

Andreas

> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index ca6f1cb..595ffb1 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -1064,6 +1064,9 @@ struct CPUPPCState {
>       */
>      uint8_t fit_period[4];
>      uint8_t wdt_period[4];
> +
> +    /* Fields needed as intermediate for vmstate */
> +    target_ulong sdr1_vmstate;
>  };
> 
>  #define SET_FIT_PERIOD(a_, b_, c_, d_)          \
> @@ -1219,8 +1222,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 d6c2ee4..ca60bc0 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)
>  {
> -    CPUPPCState *env = (CPUPPCState *)opaque;
> -    unsigned int i, j;
> +    CPUPPCState *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)
>  {
> -    CPUPPCState *env = (CPUPPCState *)opaque;
> -    unsigned int i, j;
> -    target_ulong sdr1;
> +    CPUPPCState *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)
> +{
> +    CPUPPCState *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, CPUPPCState, 32),
>  #if !defined(TARGET_PPC64)
> -    for (i = 0; i < 32; i++)
> -        qemu_get_betls(f, &env->gprh[i]);
> +        VMSTATE_UINTTL_ARRAY(gprh, CPUPPCState, 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, CPUPPCState),
> +        VMSTATE_UINTTL(ctr, CPUPPCState),
> +        VMSTATE_UINT32_ARRAY(crf, CPUPPCState, 8),
> +        VMSTATE_UINTTL(xer, CPUPPCState),
> +        VMSTATE_UINTTL(reserve_addr, CPUPPCState),
> +        VMSTATE_UINTTL(msr, CPUPPCState),
> +        VMSTATE_UINTTL_ARRAY(tgpr, CPUPPCState, 4),
> +        VMSTATE_FLOAT64_ARRAY(fpr, CPUPPCState, 32),
> +        VMSTATE_UINT32(fpscr, CPUPPCState),
> +        VMSTATE_INT32(access_type, CPUPPCState),
>  #if defined(TARGET_PPC64)
> -    qemu_get_betls(f, &env->asr);
> -    qemu_get_sbe32s(f, &env->slb_nr);
> +        VMSTATE_UINTTL(asr, CPUPPCState),
> +        VMSTATE_INT32(slb_nr, CPUPPCState),
>  #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], CPUPPCState),
> +        VMSTATE_UINTTL_ARRAY(sr, CPUPPCState, 32),
> +        VMSTATE_UINTTL_ARRAY(DBAT[0], CPUPPCState, 8),
> +        VMSTATE_UINTTL_ARRAY(DBAT[1], CPUPPCState, 8),
> +        VMSTATE_UINTTL_ARRAY(IBAT[0], CPUPPCState, 8),
> +        VMSTATE_UINTTL_ARRAY(IBAT[1], CPUPPCState, 8),
> +        VMSTATE_INT32(nb_tlb, CPUPPCState),
> +        VMSTATE_INT32(tlb_per_way, CPUPPCState),
> +        VMSTATE_INT32(nb_ways, CPUPPCState),
> +        VMSTATE_INT32(last_way, CPUPPCState),
> +        VMSTATE_INT32(id_tlbs, CPUPPCState),
> +        VMSTATE_INT32(nb_pids, CPUPPCState),
> +        VMSTATE_STRUCT_VARRAY_INT32_TEST(tlb.tlb6, CPUPPCState, nb_tlb,
> +                                         is_tlb6, vmstate_tlb, ppc6xx_tlb_t),
> +        VMSTATE_UINTTL_ARRAY(pb, CPUPPCState, 4),
> +        VMSTATE_UINTTL_ARRAY(spr, CPUPPCState, 1024),
> +        VMSTATE_UINT32(vscr, CPUPPCState),
> +        VMSTATE_UINT64(spe_acc, CPUPPCState),
> +        VMSTATE_UINT32(spe_fscr, CPUPPCState),
> +        VMSTATE_UINTTL(msr_mask, CPUPPCState),
> +        VMSTATE_UINT32(flags, CPUPPCState),
> +        VMSTATE_INT32(error_code, CPUPPCState),
> +        VMSTATE_UINT32(pending_interrupts, CPUPPCState),
> +        VMSTATE_UINT32(irq_input_state, CPUPPCState),
> +        VMSTATE_UINTTL_ARRAY(excp_vectors, CPUPPCState, POWERPC_EXCP_NB),
> +        VMSTATE_UINTTL(excp_prefix, CPUPPCState),
> +        VMSTATE_UINTTL(hreset_excp_prefix, CPUPPCState),
> +        VMSTATE_UINTTL(ivor_mask, CPUPPCState),
> +        VMSTATE_UINTTL(ivpr_mask, CPUPPCState),
> +        VMSTATE_UINTTL(hreset_vector, CPUPPCState),
> +        VMSTATE_UINTTL(nip, CPUPPCState),
> +        VMSTATE_UINTTL(hflags, CPUPPCState),
> +        VMSTATE_UINTTL(hflags_nmsr, CPUPPCState),
> +        VMSTATE_INT32(mmu_idx, CPUPPCState),
> +        VMSTATE_INT32(power_mode, CPUPPCState),
> +        VMSTATE_END_OF_LIST()
> +    },
> +};
> 
> -    return 0;
> -}

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg