Re: [Qemu-devel] [PULL 7/8] s390x/migration: migrate CPU state

[Peter Maydell]

On 9 October 2014 14:36, Cornelia Huck <address@hidden> wrote:
> From: Thomas Huth <address@hidden>
>
> This patch provides the cpu save information for dumps and later life
> migration and enables migration of the CPU state. The code is based on
> earlier work from Christian Borntraeger and Jason Herne.
>
> Signed-off-by: Thomas Huth <address@hidden>
> Signed-off-by: David Hildenbrand <address@hidden>
> [provide cpu_post_load()]
> Signed-off-by: Jens Freimann <address@hidden>
> CC: Andreas Faerber <address@hidden>
> CC: Christian Borntraeger <address@hidden>
> CC: Jason J. Herne <address@hidden>
> Tested-by: Christian Borntraeger <address@hidden>
> Signed-off-by: Cornelia Huck <address@hidden>
> ---
>  target-s390x/cpu.c |   59 
> ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 57 insertions(+), 2 deletions(-)
>
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index ec7df90..c9c237f 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c

I think the migration code should live in machine.c like
it does for our other targets. (Among other useful things,
this means you can have the makefile say
   obj-$(CONFIG_SOFTMMU) += machine.o
so it doesn't try to build it for the linux-user target :-))

> @@ -292,9 +292,64 @@ unsigned int s390_cpu_set_state(uint8_t cpu_state, 
> S390CPU *cpu)
>  }
>  #endif
>
> +static int cpu_post_load(void *opaque, int version_id)
> +{
> +    S390CPU *cpu = opaque;
> +
> +    /* the cpu state is fine for QEMU - we just need to push it to kvm */
> +    if (kvm_enabled()) {
> +        kvm_s390_set_cpu_state(cpu, cpu->env.cpu_state);
> +    }

Haven't looked at the detail but I'm vaguely surprised
this has to be done manually rather than it just
being automatically resynced when we next try to
run the vCPU.

> +
> +    return 0;
> +}
> +
>  static const VMStateDescription vmstate_s390_cpu = {
>      .name = "cpu",
> -    .unmigratable = 1,
> +    .post_load = cpu_post_load,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,

You don't need minimum_version_id_old any more.

> +    .fields      = (VMStateField[]) {
> +        VMSTATE_UINT64(env.fregs[0].ll, S390CPU),
> +        VMSTATE_UINT64(env.fregs[1].ll, S390CPU),
> +        VMSTATE_UINT64(env.fregs[2].ll, S390CPU),
> +        VMSTATE_UINT64(env.fregs[3].ll, S390CPU),
> +        VMSTATE_UINT64(env.fregs[4].ll, S390CPU),
> +        VMSTATE_UINT64(env.fregs[5].ll, S390CPU),
> +        VMSTATE_UINT64(env.fregs[6].ll, S390CPU),
> +        VMSTATE_UINT64(env.fregs[7].ll, S390CPU),
> +        VMSTATE_UINT64(env.fregs[8].ll, S390CPU),
> +        VMSTATE_UINT64(env.fregs[9].ll, S390CPU),
> +        VMSTATE_UINT64(env.fregs[10].ll, S390CPU),
> +        VMSTATE_UINT64(env.fregs[11].ll, S390CPU),
> +        VMSTATE_UINT64(env.fregs[12].ll, S390CPU),
> +        VMSTATE_UINT64(env.fregs[13].ll, S390CPU),
> +        VMSTATE_UINT64(env.fregs[14].ll, S390CPU),
> +        VMSTATE_UINT64(env.fregs[15].ll, S390CPU),
> +        VMSTATE_UINT64_ARRAY(env.regs, S390CPU, 16),
> +        VMSTATE_UINT64(env.psw.mask, S390CPU),
> +        VMSTATE_UINT64(env.psw.addr, S390CPU),
> +        VMSTATE_UINT64(env.psa, S390CPU),
> +        VMSTATE_UINT32(env.fpc, S390CPU),
> +        VMSTATE_UINT32(env.todpr, S390CPU),
> +        VMSTATE_UINT64(env.pfault_token, S390CPU),
> +        VMSTATE_UINT64(env.pfault_compare, S390CPU),
> +        VMSTATE_UINT64(env.pfault_select, S390CPU),
> +        VMSTATE_UINT64(env.cputm, S390CPU),
> +        VMSTATE_UINT64(env.ckc, S390CPU),
> +        VMSTATE_UINT64(env.gbea, S390CPU),
> +        VMSTATE_UINT64(env.pp, S390CPU),
> +        VMSTATE_UINT32_ARRAY(env.aregs, S390CPU, 16),
> +        VMSTATE_UINT64_ARRAY(env.cregs, S390CPU, 16),
> +        VMSTATE_UINT8(env.cpu_state, S390CPU),
> +        VMSTATE_END_OF_LIST()
> +     },
> +    .subsections = (VMStateSubsection[]) {
> +        {
> +            /* empty */
> +        }

Why the empty subsections list?

> +    }
>  };

thanks
-- PMM