Message ID | 1412861761-22047-8-git-send-email-cornelia.huck@de.ibm.com |
---|---|
State | New |
Headers | show |
On 9 October 2014 14:36, Cornelia Huck <cornelia.huck@de.ibm.com> wrote: > From: Thomas Huth <thuth@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 <thuth@linux.vnet.ibm.com> > Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com> > [provide cpu_post_load()] > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com> > CC: Andreas Faerber <afaerber@suse.de> > CC: Christian Borntraeger <borntraeger@de.ibm.com> > CC: Jason J. Herne <jjherne@us.ibm.com> > Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> > Signed-off-by: Cornelia Huck <cornelia.huck@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 :-)) > @@ -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
On Thu, 9 Oct 2014 17:28:57 +0100 Peter Maydell <peter.maydell@linaro.org> wrote: > On 9 October 2014 14:36, Cornelia Huck <cornelia.huck@de.ibm.com> wrote: > > From: Thomas Huth <thuth@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 <thuth@linux.vnet.ibm.com> > > Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com> > > [provide cpu_post_load()] > > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com> > > CC: Andreas Faerber <afaerber@suse.de> > > CC: Christian Borntraeger <borntraeger@de.ibm.com> > > CC: Jason J. Herne <jjherne@us.ibm.com> > > Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> > > Signed-off-by: Cornelia Huck <cornelia.huck@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 >
On Thu, 9 Oct 2014 17:28:57 +0100 Peter Maydell <peter.maydell@linaro.org> wrote: > On 9 October 2014 14:36, Cornelia Huck <cornelia.huck@de.ibm.com> wrote: > > From: Thomas Huth <thuth@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 <thuth@linux.vnet.ibm.com> > > Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com> > > [provide cpu_post_load()] > > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com> > > CC: Andreas Faerber <afaerber@suse.de> > > CC: Christian Borntraeger <borntraeger@de.ibm.com> > > CC: Jason J. Herne <jjherne@us.ibm.com> > > Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> > > Signed-off-by: Cornelia Huck <cornelia.huck@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 :-)) We originally had that file for s390x, too, but it had been removed by Andreas with this commit: c7396bbb2597577b1463fc997a73e67b8a067880 I guess we could re-introduce it again now that we have some meaningful contents for the file. Andreas, is that ok for you? > > 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. Ok, I'll remove it. > > + .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? Likely an old copy-n-paste error ... I'll remove that, too. Thomas
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 @@ -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); + } + + 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, + .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 */ + } + } }; static void s390_cpu_class_init(ObjectClass *oc, void *data) @@ -323,11 +378,11 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data) cc->handle_mmu_fault = s390_cpu_handle_mmu_fault; #else cc->get_phys_page_debug = s390_cpu_get_phys_page_debug; + cc->vmsd = &vmstate_s390_cpu; cc->write_elf64_note = s390_cpu_write_elf64_note; cc->write_elf64_qemunote = s390_cpu_write_elf64_qemunote; cc->cpu_exec_interrupt = s390_cpu_exec_interrupt; #endif - dc->vmsd = &vmstate_s390_cpu; cc->gdb_num_core_regs = S390_NUM_CORE_REGS; cc->gdb_core_xml_file = "s390x-core64.xml"; }