On Thu, 9 Oct 2014 17:28:57 +0100
Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 9 October 2014 14:36, Cornelia Huck <cornelia.h...@de.ibm.com> wrote:
> > From: Thomas Huth <th...@linux.vnet.ibm.com>
> >
> > 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 <th...@linux.vnet.ibm.com>
> > Signed-off-by: David Hildenbrand <d...@linux.vnet.ibm.com>
> > [provide cpu_post_load()]
> > Signed-off-by: Jens Freimann <jf...@linux.vnet.ibm.com>
> > CC: Andreas Faerber <afaer...@suse.de>
> > CC: Christian Borntraeger <borntrae...@de.ibm.com>
> > CC: Jason J. Herne <jjhe...@us.ibm.com>
> > Tested-by: Christian Borntraeger <borntrae...@de.ibm.com>
> > Signed-off-by: Cornelia Huck <cornelia.h...@de.ibm.com>
> > ---
> > 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 :-))
Probably. Thomas, can you look into that (and the other comments)?
>
> > @@ -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.
We also need to propagate state to vcpus that have not been yet run, as
code targeting other vcpus may need to check it.
>
> > +
> > + 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
>
