diff mbox

[PULL,7/8] s390x/migration: migrate CPU state

Message ID 1412861761-22047-8-git-send-email-cornelia.huck@de.ibm.com
State New
Headers show

Commit Message

Cornelia Huck Oct. 9, 2014, 1:36 p.m. UTC 
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(-)

Comments

Peter Maydell Oct. 9, 2014, 4:28 p.m. UTC | #1 
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
Cornelia Huck Oct. 9, 2014, 5:32 p.m. UTC | #2 
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
>
Thomas Huth Oct. 10, 2014, 7:08 a.m. UTC | #3 
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 mbox

Patch

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";
 }