[Qemu-devel] [RFC/PATCH 0/3] Initial migration patches for s390

[Qemu-devel] [RFC/PATCH 0/3] Initial migration patches for s390

[Christian Borntraeger]

Alex,

here is a small subset of migration patches for s390. Several things
are still missing/under work (storage key/dirty pages tracking, guest
time..)

Anyway, with those 3 patches, it is possible to stop + save a guest 
that contains just a kernel+busybox initrd into a file and reload it
afterwards.

Patch 2 should be applied together with patch 1 to avoid a bump in the
version number.

Please have a look

Christian Borntraeger (1):
  s390/migration: Provide a cpu save for initial life migration work

Heinz Graalfs (1):
  s390/migration: Add code to support SCLP live migration

Jason J. herne (1):
  s390/migration: Qemu S390 special register migration

 hw/s390x/event-facility.c |   46 +++++++++++++
 hw/s390x/event-facility.h |    3 +
 hw/s390x/sclpconsole.c    |   46 +++++++++++++
 hw/s390x/sclpquiesce.c    |   18 ++++++
 target-s390x/cpu.h        |    5 ++
 target-s390x/machine.c    |  156 +++++++++++++++++++++++++++++++++++++++++----
 6 files changed, 261 insertions(+), 13 deletions(-)

-- 
1.7.10.1

[Qemu-devel] [PATCH 1/3] s390/migration: Provide a cpu save for initial life migration work

[Christian Borntraeger]

This provides a simple cpu load and save function. With the recent
addition of sync regs we have the crs,acrs, the prefix and the
PSW already up to date. Lets also save the fpu via pre/post hooks.

This patch also changes the license of machine.c to GPLv2 or later.
(The old code was just empty glue code, so there is no need
to go the "contributions after" way).

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target-s390x/cpu.h     |    1 +
 target-s390x/machine.c |  115 ++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 103 insertions(+), 13 deletions(-)

diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 0f9a1f7..ba695dd 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -27,6 +27,7 @@
 #define ELF_MACHINE	EM_S390
 
 #define CPUArchState struct CPUS390XState
+#define CPU_SAVE_VERSION 1
 
 #include "cpu-defs.h"
 #define TARGET_PAGE_BITS 12
diff --git a/target-s390x/machine.c b/target-s390x/machine.c
index 3e79be6..02706fd 100644
--- a/target-s390x/machine.c
+++ b/target-s390x/machine.c
@@ -2,29 +2,118 @@
  * QEMU S390x machine definitions
  *
  * Copyright (c) 2009 Alexander Graf <agraf@suse.de>
+ * Copyright IBM Corp. 2012
  *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at your
+ * option) any later version.  See the COPYING file in the top-level directory.
  */
 
 #include "hw/hw.h"
 #include "hw/boards.h"
+#include "cpu.h"
+#include "kvm.h"
+
+#if defined CONFIG_KVM
+static void cpu_pre_save(void *opaque)
+{
+    CPUS390XState *env = opaque;
+    struct kvm_fpu fpu;
+    int i, r;
+
+    if (!kvm_enabled()) {
+        return;
+    }
+
+    r = kvm_vcpu_ioctl(env, KVM_GET_FPU, &fpu);
+    assert(r == 0);
+    for (i = 0; i < 16; i++) {
+        env->fregs[i].ll = fpu.fprs[i];
+    }
+    env->fpc = fpu.fpc;
+}
+
+static int cpu_post_load(void *opaque, int version_id)
+{
+    CPUS390XState *env = opaque;
+    struct kvm_fpu fpu;
+    int i, r;
+
+    if (!kvm_enabled()) {
+        return 0;
+    }
+
+    for (i = 0; i < 16; i++) {
+        fpu.fprs[i] = env->fregs[i].ll;
+    }
+    fpu.fpc = env->fpc;
+
+    r = kvm_vcpu_ioctl(env, KVM_SET_FPU, &fpu);
+    assert(r == 0);
+
+    return 0;
+}
+#else
+static int cpu_post_load(void *opaque, int version_id)
+{
+    return 0;
+}
+
+static void cpu_pre_save(void *opaque)
+{
+}
+#endif
+
+
+static const VMStateDescription vmstate_cpu = {
+    .name = "cpu",
+    .version_id = CPU_SAVE_VERSION,
+    .pre_save = cpu_pre_save,
+    .post_load = cpu_post_load,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT64(fregs[0].ll, CPUS390XState),
+        VMSTATE_UINT64(fregs[1].ll, CPUS390XState),
+        VMSTATE_UINT64(fregs[2].ll, CPUS390XState),
+        VMSTATE_UINT64(fregs[3].ll, CPUS390XState),
+        VMSTATE_UINT64(fregs[4].ll, CPUS390XState),
+        VMSTATE_UINT64(fregs[5].ll, CPUS390XState),
+        VMSTATE_UINT64(fregs[6].ll, CPUS390XState),
+        VMSTATE_UINT64(fregs[7].ll, CPUS390XState),
+        VMSTATE_UINT64(fregs[8].ll, CPUS390XState),
+        VMSTATE_UINT64(fregs[9].ll, CPUS390XState),
+        VMSTATE_UINT64(fregs[10].ll, CPUS390XState),
+        VMSTATE_UINT64(fregs[11].ll, CPUS390XState),
+        VMSTATE_UINT64(fregs[12].ll, CPUS390XState),
+        VMSTATE_UINT64(fregs[13].ll, CPUS390XState),
+        VMSTATE_UINT64(fregs[14].ll, CPUS390XState),
+        VMSTATE_UINT64(fregs[15].ll, CPUS390XState),
+        VMSTATE_UINT64_ARRAY(regs, CPUS390XState, 16),
+        VMSTATE_UINT64(psw.mask, CPUS390XState),
+        VMSTATE_UINT64(psw.addr, CPUS390XState),
+        VMSTATE_UINT64(psa, CPUS390XState),
+        VMSTATE_UINT32(fpc, CPUS390XState),
+        VMSTATE_UINT32_ARRAY(aregs, CPUS390XState, 16),
+        VMSTATE_UINT64_ARRAY(cregs, CPUS390XState, 16),
+        VMSTATE_END_OF_LIST()
+     },
+    .subsections = (VMStateSubsection[]) {
+        {
+            /* empty */
+        }
+    }
+
+};
+
+
+
 
 void cpu_save(QEMUFile *f, void *opaque)
 {
+    vmstate_save_state(f, &vmstate_cpu, opaque);
 }
 
 int cpu_load(QEMUFile *f, void *opaque, int version_id)
 {
-    return 0;
+    return vmstate_load_state(f, &vmstate_cpu, opaque, version_id);
 }
-- 
1.7.10.1

[Qemu-devel] [PATCH 2/3] s390/migration: Qemu S390 special register migration

[Christian Borntraeger]

From: "Jason J. herne" <jjherne@us.ibm.com>

Use the KVM ONE_REG capability to save and restore the following special S390
cpu registers: clock comparator, tod clock programmable register and the cpu
timer.  Save/loading of these registers is required to enable guest migration on
the S390 platform.

Signed-off-by: Jason J. herne <jjherne@us.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target-s390x/cpu.h     |    4 ++++
 target-s390x/machine.c |   41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index ba695dd..de77335 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -92,6 +92,10 @@ typedef struct CPUS390XState {
 
     int ext_index;
 
+    uint64_t ckc;
+    uint64_t cputm;
+    uint32_t todpr;
+
     CPU_COMMON
 
     /* reset does memset(0) up to here */
diff --git a/target-s390x/machine.c b/target-s390x/machine.c
index 02706fd..925afb5 100644
--- a/target-s390x/machine.c
+++ b/target-s390x/machine.c
@@ -18,6 +18,7 @@ static void cpu_pre_save(void *opaque)
 {
     CPUS390XState *env = opaque;
     struct kvm_fpu fpu;
+    struct kvm_one_reg reg;
     int i, r;
 
     if (!kvm_enabled()) {
@@ -30,12 +31,31 @@ static void cpu_pre_save(void *opaque)
         env->fregs[i].ll = fpu.fprs[i];
     }
     env->fpc = fpu.fpc;
+
+    /* Retreive cpu timer value from kvm */
+    reg.id = KVM_REG_S390_CPU_TIMER;
+    reg.addr = (__u64)&(env->cputm);
+    r = kvm_vcpu_ioctl(env, KVM_GET_ONE_REG, &reg);
+    assert(r == 0);
+
+    /* Retreive clock comparator value from kvm */
+    reg.id = KVM_REG_S390_CLOCK_COMP;
+    reg.addr = (__u64)&(env->ckc);
+    r = kvm_vcpu_ioctl(env, KVM_GET_ONE_REG, &reg);
+    assert(r == 0);
+
+    /* Retreive clock comparator value from kvm */
+    reg.id = KVM_REG_S390_TODPR;
+    reg.addr = (__u64)&(env->todpr);
+    r = kvm_vcpu_ioctl(env, KVM_GET_ONE_REG, &reg);
+    assert(r == 0);
 }
 
 static int cpu_post_load(void *opaque, int version_id)
 {
     CPUS390XState *env = opaque;
     struct kvm_fpu fpu;
+    struct kvm_one_reg reg;
     int i, r;
 
     if (!kvm_enabled()) {
@@ -50,6 +70,24 @@ static int cpu_post_load(void *opaque, int version_id)
     r = kvm_vcpu_ioctl(env, KVM_SET_FPU, &fpu);
     assert(r == 0);
 
+    /* Tell KVM what the new cpu timer value is */
+    reg.id = KVM_REG_S390_CPU_TIMER;
+    reg.addr = (__u64)&(env->cputm);
+    r = kvm_vcpu_ioctl(env, KVM_SET_ONE_REG, &reg);
+    assert(r == 0);
+
+    /* Tell KVM what the new clock comparator value is */
+    reg.id = KVM_REG_S390_CLOCK_COMP;
+    reg.addr = (__u64)&(env->ckc);
+    r = kvm_vcpu_ioctl(env, KVM_SET_ONE_REG, &reg);
+    assert(r == 0);
+
+    /* Tell KVM what the new todpr value is */
+    reg.id = KVM_REG_S390_TODPR;
+    reg.addr = (__u64)&(env->todpr);
+    r = kvm_vcpu_ioctl(env, KVM_SET_ONE_REG, &reg);
+    assert(r == 0);
+
     return 0;
 }
 #else
@@ -93,6 +131,9 @@ static const VMStateDescription vmstate_cpu = {
         VMSTATE_UINT64(psw.addr, CPUS390XState),
         VMSTATE_UINT64(psa, CPUS390XState),
         VMSTATE_UINT32(fpc, CPUS390XState),
+        VMSTATE_UINT32(todpr, CPUS390XState),
+        VMSTATE_UINT64(cputm, CPUS390XState),
+        VMSTATE_UINT64(ckc, CPUS390XState),
         VMSTATE_UINT32_ARRAY(aregs, CPUS390XState, 16),
         VMSTATE_UINT64_ARRAY(cregs, CPUS390XState, 16),
         VMSTATE_END_OF_LIST()
-- 
1.7.10.1

[Qemu-devel] [PATCH 3/3] s390/migration: Add code to support SCLP live migration

[Christian Borntraeger]

From: Heinz Graalfs <graalfs@linux.vnet.ibm.com>

This patch adds the necessary life migration pieces to the sclp code.

Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/s390x/event-facility.c |   46 +++++++++++++++++++++++++++++++++++++++++++++
 hw/s390x/event-facility.h |    3 +++
 hw/s390x/sclpconsole.c    |   46 +++++++++++++++++++++++++++++++++++++++++++++
 hw/s390x/sclpquiesce.c    |   18 ++++++++++++++++++
 4 files changed, 113 insertions(+)

diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 9367660..47a1bab 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -312,6 +312,49 @@ static void command_handler(SCLPEventFacility *ef, SCCB *sccb, uint64_t code)
     }
 }
 
+static void event_facility_save(QEMUFile *f, void *opaque)
+{
+    S390SCLPDevice *sdev = opaque;
+    BusChild *kid;
+    SCLPEvent *event;
+    SCLPEventClass *event_class;
+
+    qemu_put_be32(f, sdev->ef->receive_mask);
+
+    QTAILQ_FOREACH(kid, &sdev->ef->sbus.qbus.children, sibling) {
+        DeviceState *qdev = kid->child;
+        event = DO_UPCAST(SCLPEvent, qdev, qdev);
+        event_class = SCLP_EVENT_GET_CLASS(event);
+        if (event_class->save) {
+            event_class->save(f, event);
+        }
+    }
+}
+
+static int event_facility_load(QEMUFile *f, void *opaque, int version_id)
+{
+    S390SCLPDevice *sdev = opaque;
+    int rc = 0;
+    BusChild *kid;
+    SCLPEvent *event;
+    SCLPEventClass *event_class;
+
+    sdev->ef->receive_mask = qemu_get_be32(f);
+
+    QTAILQ_FOREACH(kid, &sdev->ef->sbus.qbus.children, sibling) {
+        DeviceState *qdev = kid->child;
+        event = DO_UPCAST(SCLPEvent, qdev, qdev);
+        event_class = SCLP_EVENT_GET_CLASS(event);
+        if (event_class->load) {
+            rc = event_class->load(f, event, version_id);
+            if (rc) {
+                break;
+            }
+        }
+    }
+    return rc;
+}
+
 static int init_event_facility(S390SCLPDevice *sdev)
 {
     SCLPEventFacility *event_facility;
@@ -334,6 +377,9 @@ static int init_event_facility(S390SCLPDevice *sdev)
     }
     qdev_init_nofail(quiesce);
 
+    register_savevm(&sdev->busdev.qdev, "event-facility", -1, 0,
+                    event_facility_save, event_facility_load, sdev);
+
     return 0;
 }
 
diff --git a/hw/s390x/event-facility.h b/hw/s390x/event-facility.h
index 30af0a7..4405022 100644
--- a/hw/s390x/event-facility.h
+++ b/hw/s390x/event-facility.h
@@ -91,6 +91,9 @@ typedef struct SCLPEventClass {
     /* returns the supported event type */
     int (*event_type)(void);
 
+    /* live migration */
+    int (*load)(QEMUFile *f, void *opaque, int version_id);
+    void (*save)(QEMUFile *f, void *opaque);
 } SCLPEventClass;
 
 #endif
diff --git a/hw/s390x/sclpconsole.c b/hw/s390x/sclpconsole.c
index 0ec5623..96168a1 100644
--- a/hw/s390x/sclpconsole.c
+++ b/hw/s390x/sclpconsole.c
@@ -243,6 +243,50 @@ static void trigger_ascii_console_data(void *env, int n, int level)
 /* qemu object creation and initialization functions */
 
 /* tell character layer our call-back functions */
+
+static void console_save(QEMUFile *f, void *opaque)
+{
+    SCLPConsole *scon = opaque;
+
+    if (!scon->iov) {
+        return;
+    }
+
+    qemu_put_be16(f, scon->event.event_pending ? 1 : 0);
+    qemu_put_be32(f, scon->iov_data_len);
+    qemu_put_be32(f, scon->iov_sclp_rest);
+    qemu_put_be32(f, scon->iov_sclp - scon->iov);
+    qemu_put_be32(f, scon->iov_bs - scon->iov);
+    if (scon->event.event_pending) {
+        qemu_put_buffer(f, scon->iov, SIZE_BUFFER_VT220);
+    }
+}
+
+static int console_load(QEMUFile *f, void *opaque, int version_id)
+{
+    SCLPConsole *scon = opaque;
+    int l;
+
+    if (!scon->iov) {
+        scon->iov = g_malloc0(SIZE_BUFFER_VT220);
+    }
+
+    scon->event.event_pending = qemu_get_be16(f) ? true : false;
+    scon->iov_data_len = qemu_get_be32(f);
+    scon->iov_sclp_rest = qemu_get_be32(f);
+    scon->iov_bs = scon->iov + qemu_get_be32(f);
+    scon->iov_sclp = scon->iov + qemu_get_be32(f);
+    if (scon->event.event_pending) {
+        l = qemu_get_buffer(f, scon->iov, SIZE_BUFFER_VT220);
+        if (l != SIZE_BUFFER_VT220) {
+            error_report("Failed to restore SCLP console buffer.");
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
 static int console_init(SCLPEvent *event)
 {
     static bool console_available;
@@ -288,6 +332,8 @@ static void console_class_init(ObjectClass *klass, void *data)
     ec->event_type = event_type;
     ec->read_event_data = read_event_data;
     ec->write_event_data = write_event_data;
+    ec->load = console_load;
+    ec->save = console_save;
 }
 
 static TypeInfo sclp_console_info = {
diff --git a/hw/s390x/sclpquiesce.c b/hw/s390x/sclpquiesce.c
index 9a773b8..e12fb7c 100644
--- a/hw/s390x/sclpquiesce.c
+++ b/hw/s390x/sclpquiesce.c
@@ -65,6 +65,22 @@ static int read_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr,
     return 1;
 }
 
+static void quiesce_save(QEMUFile *f, void *opaque)
+{
+    SCLPEvent *event = opaque;
+
+    qemu_put_be16(f, event->event_pending ? 1 : 0);
+}
+
+static int quiesce_load(QEMUFile *f, void *opaque, int version_id)
+{
+    SCLPEvent *event = opaque;
+
+    event->event_pending = qemu_get_be16(f) ? true : false;
+
+    return 0;
+}
+
 typedef struct QuiesceNotifier QuiesceNotifier;
 
 static struct QuiesceNotifier {
@@ -105,6 +121,8 @@ static void quiesce_class_init(ObjectClass *klass, void *data)
     k->event_type = event_type;
     k->read_event_data = read_event_data;
     k->write_event_data = NULL;
+    k->load = quiesce_load;
+    k->save = quiesce_save;
 }
 
 static TypeInfo sclp_quiesce_info = {
-- 
1.7.10.1

Re: [Qemu-devel] [PATCH 1/3] s390/migration: Provide a cpu save for initial life migration work

[Alexander Graf]

On 11/21/2012 03:46 PM, Christian Borntraeger wrote:
> This provides a simple cpu load and save function. With the recent
> addition of sync regs we have the crs,acrs, the prefix and the
> PSW already up to date. Lets also save the fpu via pre/post hooks.
>
> This patch also changes the license of machine.c to GPLv2 or later.
> (The old code was just empty glue code, so there is no need
> to go the "contributions after" way).
>
> Signed-off-by: Christian Borntraeger<borntraeger@de.ibm.com>
> ---
>   target-s390x/cpu.h     |    1 +
>   target-s390x/machine.c |  115 ++++++++++++++++++++++++++++++++++++++++++------
>   2 files changed, 103 insertions(+), 13 deletions(-)
>
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index 0f9a1f7..ba695dd 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -27,6 +27,7 @@
>   #define ELF_MACHINE	EM_S390
>
>   #define CPUArchState struct CPUS390XState
> +#define CPU_SAVE_VERSION 1
>
>   #include "cpu-defs.h"
>   #define TARGET_PAGE_BITS 12
> diff --git a/target-s390x/machine.c b/target-s390x/machine.c
> index 3e79be6..02706fd 100644
> --- a/target-s390x/machine.c
> +++ b/target-s390x/machine.c
> @@ -2,29 +2,118 @@
>    * QEMU S390x machine definitions
>    *
>    * Copyright (c) 2009 Alexander Graf<agraf@suse.de>
> + * Copyright IBM Corp. 2012
>    *
> - * This library is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU Lesser General Public
> - * License as published by the Free Software Foundation; either
> - * version 2 of the License, or (at your option) any later version.
> - *
> - * This library is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - * Lesser General Public License for more details.
> - *
> - * You should have received a copy of the GNU Lesser General Public
> - * License along with this library; if not, see<http://www.gnu.org/licenses/>.
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at your
> + * option) any later version.  See the COPYING file in the top-level directory.
>    */
>
>   #include "hw/hw.h"
>   #include "hw/boards.h"
> +#include "cpu.h"
> +#include "kvm.h"
> +
> +#if defined CONFIG_KVM
> +static void cpu_pre_save(void *opaque)
> +{
> +    CPUS390XState *env = opaque;
> +    struct kvm_fpu fpu;
> +    int i, r;
> +
> +    if (!kvm_enabled()) {
> +        return;
> +    }
> +
> +    r = kvm_vcpu_ioctl(env, KVM_GET_FPU,&fpu);
> +    assert(r == 0);
> +    for (i = 0; i<  16; i++) {
> +        env->fregs[i].ll = fpu.fprs[i];
> +    }
> +    env->fpc = fpu.fpc;
> +}
> +
> +static int cpu_post_load(void *opaque, int version_id)
> +{
> +    CPUS390XState *env = opaque;
> +    struct kvm_fpu fpu;
> +    int i, r;
> +
> +    if (!kvm_enabled()) {
> +        return 0;
> +    }
> +
> +    for (i = 0; i<  16; i++) {
> +        fpu.fprs[i] = env->fregs[i].ll;
> +    }
> +    fpu.fpc = env->fpc;
> +
> +    r = kvm_vcpu_ioctl(env, KVM_SET_FPU,&fpu);
> +    assert(r == 0);
> +
> +    return 0;
> +}

The kvm register sync needs to happen in the kvm register sync function :)


Alex

> +#else
> +static int cpu_post_load(void *opaque, int version_id)
> +{
> +    return 0;
> +}
> +
> +static void cpu_pre_save(void *opaque)
> +{
> +}
> +#endif
> +
> +
> +static const VMStateDescription vmstate_cpu = {
> +    .name = "cpu",
> +    .version_id = CPU_SAVE_VERSION,
> +    .pre_save = cpu_pre_save,
> +    .post_load = cpu_post_load,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_UINT64(fregs[0].ll, CPUS390XState),
> +        VMSTATE_UINT64(fregs[1].ll, CPUS390XState),
> +        VMSTATE_UINT64(fregs[2].ll, CPUS390XState),
> +        VMSTATE_UINT64(fregs[3].ll, CPUS390XState),
> +        VMSTATE_UINT64(fregs[4].ll, CPUS390XState),
> +        VMSTATE_UINT64(fregs[5].ll, CPUS390XState),
> +        VMSTATE_UINT64(fregs[6].ll, CPUS390XState),
> +        VMSTATE_UINT64(fregs[7].ll, CPUS390XState),
> +        VMSTATE_UINT64(fregs[8].ll, CPUS390XState),
> +        VMSTATE_UINT64(fregs[9].ll, CPUS390XState),
> +        VMSTATE_UINT64(fregs[10].ll, CPUS390XState),
> +        VMSTATE_UINT64(fregs[11].ll, CPUS390XState),
> +        VMSTATE_UINT64(fregs[12].ll, CPUS390XState),
> +        VMSTATE_UINT64(fregs[13].ll, CPUS390XState),
> +        VMSTATE_UINT64(fregs[14].ll, CPUS390XState),
> +        VMSTATE_UINT64(fregs[15].ll, CPUS390XState),
> +        VMSTATE_UINT64_ARRAY(regs, CPUS390XState, 16),
> +        VMSTATE_UINT64(psw.mask, CPUS390XState),
> +        VMSTATE_UINT64(psw.addr, CPUS390XState),
> +        VMSTATE_UINT64(psa, CPUS390XState),
> +        VMSTATE_UINT32(fpc, CPUS390XState),
> +        VMSTATE_UINT32_ARRAY(aregs, CPUS390XState, 16),
> +        VMSTATE_UINT64_ARRAY(cregs, CPUS390XState, 16),
> +        VMSTATE_END_OF_LIST()
> +     },
> +    .subsections = (VMStateSubsection[]) {
> +        {
> +            /* empty */
> +        }
> +    }
> +
> +};
> +
> +
> +
>
>   void cpu_save(QEMUFile *f, void *opaque)
>   {
> +    vmstate_save_state(f,&vmstate_cpu, opaque);
>   }
>
>   int cpu_load(QEMUFile *f, void *opaque, int version_id)
>   {
> -    return 0;
> +    return vmstate_load_state(f,&vmstate_cpu, opaque, version_id);
>   }

Re: [Qemu-devel] [PATCH 2/3] s390/migration: Qemu S390 special register migration

[Alexander Graf]

On 11/21/2012 03:46 PM, Christian Borntraeger wrote:
> From: "Jason J. herne"<jjherne@us.ibm.com>
>
> Use the KVM ONE_REG capability to save and restore the following special S390
> cpu registers: clock comparator, tod clock programmable register and the cpu
> timer.  Save/loading of these registers is required to enable guest migration on
> the S390 platform.
>
> Signed-off-by: Jason J. herne<jjherne@us.ibm.com>
> Signed-off-by: Christian Borntraeger<borntraeger@de.ibm.com>
> ---
>   target-s390x/cpu.h     |    4 ++++
>   target-s390x/machine.c |   41 +++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 45 insertions(+)
>
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index ba695dd..de77335 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -92,6 +92,10 @@ typedef struct CPUS390XState {
>
>       int ext_index;
>
> +    uint64_t ckc;
> +    uint64_t cputm;
> +    uint32_t todpr;
> +
>       CPU_COMMON
>
>       /* reset does memset(0) up to here */
> diff --git a/target-s390x/machine.c b/target-s390x/machine.c
> index 02706fd..925afb5 100644
> --- a/target-s390x/machine.c
> +++ b/target-s390x/machine.c
> @@ -18,6 +18,7 @@ static void cpu_pre_save(void *opaque)
>   {
>       CPUS390XState *env = opaque;
>       struct kvm_fpu fpu;
> +    struct kvm_one_reg reg;
>       int i, r;
>
>       if (!kvm_enabled()) {
> @@ -30,12 +31,31 @@ static void cpu_pre_save(void *opaque)
>           env->fregs[i].ll = fpu.fprs[i];
>       }
>       env->fpc = fpu.fpc;
> +
> +    /* Retreive cpu timer value from kvm */
> +    reg.id = KVM_REG_S390_CPU_TIMER;
> +    reg.addr = (__u64)&(env->cputm);
> +    r = kvm_vcpu_ioctl(env, KVM_GET_ONE_REG,&reg);
> +    assert(r == 0);
> +
> +    /* Retreive clock comparator value from kvm */
> +    reg.id = KVM_REG_S390_CLOCK_COMP;
> +    reg.addr = (__u64)&(env->ckc);
> +    r = kvm_vcpu_ioctl(env, KVM_GET_ONE_REG,&reg);
> +    assert(r == 0);
> +
> +    /* Retreive clock comparator value from kvm */
> +    reg.id = KVM_REG_S390_TODPR;
> +    reg.addr = (__u64)&(env->todpr);
> +    r = kvm_vcpu_ioctl(env, KVM_GET_ONE_REG,&reg);

Same here. There must not be any KVM specific code in the save/restore path.


Alex

> +    assert(r == 0);
>   }
>
>   static int cpu_post_load(void *opaque, int version_id)
>   {
>       CPUS390XState *env = opaque;
>       struct kvm_fpu fpu;
> +    struct kvm_one_reg reg;
>       int i, r;
>
>       if (!kvm_enabled()) {
> @@ -50,6 +70,24 @@ static int cpu_post_load(void *opaque, int version_id)
>       r = kvm_vcpu_ioctl(env, KVM_SET_FPU,&fpu);
>       assert(r == 0);
>
> +    /* Tell KVM what the new cpu timer value is */
> +    reg.id = KVM_REG_S390_CPU_TIMER;
> +    reg.addr = (__u64)&(env->cputm);
> +    r = kvm_vcpu_ioctl(env, KVM_SET_ONE_REG,&reg);
> +    assert(r == 0);
> +
> +    /* Tell KVM what the new clock comparator value is */
> +    reg.id = KVM_REG_S390_CLOCK_COMP;
> +    reg.addr = (__u64)&(env->ckc);
> +    r = kvm_vcpu_ioctl(env, KVM_SET_ONE_REG,&reg);
> +    assert(r == 0);
> +
> +    /* Tell KVM what the new todpr value is */
> +    reg.id = KVM_REG_S390_TODPR;
> +    reg.addr = (__u64)&(env->todpr);
> +    r = kvm_vcpu_ioctl(env, KVM_SET_ONE_REG,&reg);
> +    assert(r == 0);
> +
>       return 0;
>   }
>   #else
> @@ -93,6 +131,9 @@ static const VMStateDescription vmstate_cpu = {
>           VMSTATE_UINT64(psw.addr, CPUS390XState),
>           VMSTATE_UINT64(psa, CPUS390XState),
>           VMSTATE_UINT32(fpc, CPUS390XState),
> +        VMSTATE_UINT32(todpr, CPUS390XState),
> +        VMSTATE_UINT64(cputm, CPUS390XState),
> +        VMSTATE_UINT64(ckc, CPUS390XState),
>           VMSTATE_UINT32_ARRAY(aregs, CPUS390XState, 16),
>           VMSTATE_UINT64_ARRAY(cregs, CPUS390XState, 16),
>           VMSTATE_END_OF_LIST()

Re: [Qemu-devel] [PATCH 3/3] s390/migration: Add code to support SCLP live migration

[Alexander Graf]

On 11/21/2012 03:46 PM, Christian Borntraeger wrote:
> From: Heinz Graalfs<graalfs@linux.vnet.ibm.com>
>
> This patch adds the necessary life migration pieces to the sclp code.

Juan, could you please review and note whether it's possible to use 
VMSTATE for this? :)


Alex

>
> Signed-off-by: Heinz Graalfs<graalfs@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger<borntraeger@de.ibm.com>
> ---
>   hw/s390x/event-facility.c |   46 +++++++++++++++++++++++++++++++++++++++++++++
>   hw/s390x/event-facility.h |    3 +++
>   hw/s390x/sclpconsole.c    |   46 +++++++++++++++++++++++++++++++++++++++++++++
>   hw/s390x/sclpquiesce.c    |   18 ++++++++++++++++++
>   4 files changed, 113 insertions(+)
>
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index 9367660..47a1bab 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -312,6 +312,49 @@ static void command_handler(SCLPEventFacility *ef, SCCB *sccb, uint64_t code)
>       }
>   }
>
> +static void event_facility_save(QEMUFile *f, void *opaque)
> +{
> +    S390SCLPDevice *sdev = opaque;
> +    BusChild *kid;
> +    SCLPEvent *event;
> +    SCLPEventClass *event_class;
> +
> +    qemu_put_be32(f, sdev->ef->receive_mask);
> +
> +    QTAILQ_FOREACH(kid,&sdev->ef->sbus.qbus.children, sibling) {
> +        DeviceState *qdev = kid->child;
> +        event = DO_UPCAST(SCLPEvent, qdev, qdev);
> +        event_class = SCLP_EVENT_GET_CLASS(event);
> +        if (event_class->save) {
> +            event_class->save(f, event);
> +        }
> +    }
> +}
> +
> +static int event_facility_load(QEMUFile *f, void *opaque, int version_id)
> +{
> +    S390SCLPDevice *sdev = opaque;
> +    int rc = 0;
> +    BusChild *kid;
> +    SCLPEvent *event;
> +    SCLPEventClass *event_class;
> +
> +    sdev->ef->receive_mask = qemu_get_be32(f);
> +
> +    QTAILQ_FOREACH(kid,&sdev->ef->sbus.qbus.children, sibling) {
> +        DeviceState *qdev = kid->child;
> +        event = DO_UPCAST(SCLPEvent, qdev, qdev);
> +        event_class = SCLP_EVENT_GET_CLASS(event);
> +        if (event_class->load) {
> +            rc = event_class->load(f, event, version_id);
> +            if (rc) {
> +                break;
> +            }
> +        }
> +    }
> +    return rc;
> +}
> +
>   static int init_event_facility(S390SCLPDevice *sdev)
>   {
>       SCLPEventFacility *event_facility;
> @@ -334,6 +377,9 @@ static int init_event_facility(S390SCLPDevice *sdev)
>       }
>       qdev_init_nofail(quiesce);
>
> +    register_savevm(&sdev->busdev.qdev, "event-facility", -1, 0,
> +                    event_facility_save, event_facility_load, sdev);
> +
>       return 0;
>   }
>
> diff --git a/hw/s390x/event-facility.h b/hw/s390x/event-facility.h
> index 30af0a7..4405022 100644
> --- a/hw/s390x/event-facility.h
> +++ b/hw/s390x/event-facility.h
> @@ -91,6 +91,9 @@ typedef struct SCLPEventClass {
>       /* returns the supported event type */
>       int (*event_type)(void);
>
> +    /* live migration */
> +    int (*load)(QEMUFile *f, void *opaque, int version_id);
> +    void (*save)(QEMUFile *f, void *opaque);
>   } SCLPEventClass;
>
>   #endif
> diff --git a/hw/s390x/sclpconsole.c b/hw/s390x/sclpconsole.c
> index 0ec5623..96168a1 100644
> --- a/hw/s390x/sclpconsole.c
> +++ b/hw/s390x/sclpconsole.c
> @@ -243,6 +243,50 @@ static void trigger_ascii_console_data(void *env, int n, int level)
>   /* qemu object creation and initialization functions */
>
>   /* tell character layer our call-back functions */
> +
> +static void console_save(QEMUFile *f, void *opaque)
> +{
> +    SCLPConsole *scon = opaque;
> +
> +    if (!scon->iov) {
> +        return;
> +    }
> +
> +    qemu_put_be16(f, scon->event.event_pending ? 1 : 0);
> +    qemu_put_be32(f, scon->iov_data_len);
> +    qemu_put_be32(f, scon->iov_sclp_rest);
> +    qemu_put_be32(f, scon->iov_sclp - scon->iov);
> +    qemu_put_be32(f, scon->iov_bs - scon->iov);
> +    if (scon->event.event_pending) {
> +        qemu_put_buffer(f, scon->iov, SIZE_BUFFER_VT220);
> +    }
> +}
> +
> +static int console_load(QEMUFile *f, void *opaque, int version_id)
> +{
> +    SCLPConsole *scon = opaque;
> +    int l;
> +
> +    if (!scon->iov) {
> +        scon->iov = g_malloc0(SIZE_BUFFER_VT220);
> +    }
> +
> +    scon->event.event_pending = qemu_get_be16(f) ? true : false;
> +    scon->iov_data_len = qemu_get_be32(f);
> +    scon->iov_sclp_rest = qemu_get_be32(f);
> +    scon->iov_bs = scon->iov + qemu_get_be32(f);
> +    scon->iov_sclp = scon->iov + qemu_get_be32(f);
> +    if (scon->event.event_pending) {
> +        l = qemu_get_buffer(f, scon->iov, SIZE_BUFFER_VT220);
> +        if (l != SIZE_BUFFER_VT220) {
> +            error_report("Failed to restore SCLP console buffer.");
> +            return -1;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>   static int console_init(SCLPEvent *event)
>   {
>       static bool console_available;
> @@ -288,6 +332,8 @@ static void console_class_init(ObjectClass *klass, void *data)
>       ec->event_type = event_type;
>       ec->read_event_data = read_event_data;
>       ec->write_event_data = write_event_data;
> +    ec->load = console_load;
> +    ec->save = console_save;
>   }
>
>   static TypeInfo sclp_console_info = {
> diff --git a/hw/s390x/sclpquiesce.c b/hw/s390x/sclpquiesce.c
> index 9a773b8..e12fb7c 100644
> --- a/hw/s390x/sclpquiesce.c
> +++ b/hw/s390x/sclpquiesce.c
> @@ -65,6 +65,22 @@ static int read_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr,
>       return 1;
>   }
>
> +static void quiesce_save(QEMUFile *f, void *opaque)
> +{
> +    SCLPEvent *event = opaque;
> +
> +    qemu_put_be16(f, event->event_pending ? 1 : 0);
> +}
> +
> +static int quiesce_load(QEMUFile *f, void *opaque, int version_id)
> +{
> +    SCLPEvent *event = opaque;
> +
> +    event->event_pending = qemu_get_be16(f) ? true : false;
> +
> +    return 0;
> +}
> +
>   typedef struct QuiesceNotifier QuiesceNotifier;
>
>   static struct QuiesceNotifier {
> @@ -105,6 +121,8 @@ static void quiesce_class_init(ObjectClass *klass, void *data)
>       k->event_type = event_type;
>       k->read_event_data = read_event_data;
>       k->write_event_data = NULL;
> +    k->load = quiesce_load;
> +    k->save = quiesce_save;
>   }
>
>   static TypeInfo sclp_quiesce_info = {

Re: [Qemu-devel] [PATCH 1/3] s390/migration: Provide a cpu save for initial life migration work

[Christian Borntraeger]

On 21/11/12 15:56, Alexander Graf wrote:
> On 11/21/2012 03:46 PM, Christian Borntraeger wrote:
>> This provides a simple cpu load and save function. With the recent
>> addition of sync regs we have the crs,acrs, the prefix and the
>> PSW already up to date. Lets also save the fpu via pre/post hooks.
>>
>> This patch also changes the license of machine.c to GPLv2 or later.
>> (The old code was just empty glue code, so there is no need
>> to go the "contributions after" way).
>>
>> Signed-off-by: Christian Borntraeger<borntraeger@de.ibm.com>
>> ---
>>   target-s390x/cpu.h     |    1 +
>>   target-s390x/machine.c |  115 ++++++++++++++++++++++++++++++++++++++++++------
>>   2 files changed, 103 insertions(+), 13 deletions(-)
>>
>> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
>> index 0f9a1f7..ba695dd 100644
>> --- a/target-s390x/cpu.h
>> +++ b/target-s390x/cpu.h
>> @@ -27,6 +27,7 @@
>>   #define ELF_MACHINE    EM_S390
>>
>>   #define CPUArchState struct CPUS390XState
>> +#define CPU_SAVE_VERSION 1
>>
>>   #include "cpu-defs.h"
>>   #define TARGET_PAGE_BITS 12
>> diff --git a/target-s390x/machine.c b/target-s390x/machine.c
>> index 3e79be6..02706fd 100644
>> --- a/target-s390x/machine.c
>> +++ b/target-s390x/machine.c
>> @@ -2,29 +2,118 @@
>>    * QEMU S390x machine definitions
>>    *
>>    * Copyright (c) 2009 Alexander Graf<agraf@suse.de>
>> + * Copyright IBM Corp. 2012
>>    *
>> - * This library is free software; you can redistribute it and/or
>> - * modify it under the terms of the GNU Lesser General Public
>> - * License as published by the Free Software Foundation; either
>> - * version 2 of the License, or (at your option) any later version.
>> - *
>> - * This library is distributed in the hope that it will be useful,
>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> - * Lesser General Public License for more details.
>> - *
>> - * You should have received a copy of the GNU Lesser General Public
>> - * License along with this library; if not, see<http://www.gnu.org/licenses/>.
>> + * This work is licensed under the terms of the GNU GPL, version 2 or (at your
>> + * option) any later version.  See the COPYING file in the top-level directory.
>>    */
>>
>>   #include "hw/hw.h"
>>   #include "hw/boards.h"
>> +#include "cpu.h"
>> +#include "kvm.h"
>> +
>> +#if defined CONFIG_KVM
>> +static void cpu_pre_save(void *opaque)
>> +{
>> +    CPUS390XState *env = opaque;
>> +    struct kvm_fpu fpu;
>> +    int i, r;
>> +
>> +    if (!kvm_enabled()) {
>> +        return;
>> +    }
>> +
>> +    r = kvm_vcpu_ioctl(env, KVM_GET_FPU,&fpu);
>> +    assert(r == 0);
>> +    for (i = 0; i<  16; i++) {
>> +        env->fregs[i].ll = fpu.fprs[i];
>> +    }
>> +    env->fpc = fpu.fpc;
>> +}
>> +
>> +static int cpu_post_load(void *opaque, int version_id)
>> +{
>> +    CPUS390XState *env = opaque;
>> +    struct kvm_fpu fpu;
>> +    int i, r;
>> +
>> +    if (!kvm_enabled()) {
>> +        return 0;
>> +    }
>> +
>> +    for (i = 0; i<  16; i++) {
>> +        fpu.fprs[i] = env->fregs[i].ll;
>> +    }
>> +    fpu.fpc = env->fpc;
>> +
>> +    r = kvm_vcpu_ioctl(env, KVM_SET_FPU,&fpu);
>> +    assert(r == 0);
>> +
>> +    return 0;
>> +}
> 
> The kvm register sync needs to happen in the kvm register sync function :)

That would eliminate the whole purpose of sync regs and forces us to have an
expensive ioctl on lots of exits (again). I would prefer to sync the registers 
that we never need in qemu just here.

Re: [Qemu-devel] [PATCH 1/3] s390/migration: Provide a cpu save for initial life migration work

[Alexander Graf]

On 11/21/2012 03:59 PM, Christian Borntraeger wrote:
> On 21/11/12 15:56, Alexander Graf wrote:
>> On 11/21/2012 03:46 PM, Christian Borntraeger wrote:
>>> This provides a simple cpu load and save function. With the recent
>>> addition of sync regs we have the crs,acrs, the prefix and the
>>> PSW already up to date. Lets also save the fpu via pre/post hooks.
>>>
>>> This patch also changes the license of machine.c to GPLv2 or later.
>>> (The old code was just empty glue code, so there is no need
>>> to go the "contributions after" way).
>>>
>>> Signed-off-by: Christian Borntraeger<borntraeger@de.ibm.com>
>>> ---
>>>    target-s390x/cpu.h     |    1 +
>>>    target-s390x/machine.c |  115 ++++++++++++++++++++++++++++++++++++++++++------
>>>    2 files changed, 103 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
>>> index 0f9a1f7..ba695dd 100644
>>> --- a/target-s390x/cpu.h
>>> +++ b/target-s390x/cpu.h
>>> @@ -27,6 +27,7 @@
>>>    #define ELF_MACHINE    EM_S390
>>>
>>>    #define CPUArchState struct CPUS390XState
>>> +#define CPU_SAVE_VERSION 1
>>>
>>>    #include "cpu-defs.h"
>>>    #define TARGET_PAGE_BITS 12
>>> diff --git a/target-s390x/machine.c b/target-s390x/machine.c
>>> index 3e79be6..02706fd 100644
>>> --- a/target-s390x/machine.c
>>> +++ b/target-s390x/machine.c
>>> @@ -2,29 +2,118 @@
>>>     * QEMU S390x machine definitions
>>>     *
>>>     * Copyright (c) 2009 Alexander Graf<agraf@suse.de>
>>> + * Copyright IBM Corp. 2012
>>>     *
>>> - * This library is free software; you can redistribute it and/or
>>> - * modify it under the terms of the GNU Lesser General Public
>>> - * License as published by the Free Software Foundation; either
>>> - * version 2 of the License, or (at your option) any later version.
>>> - *
>>> - * This library is distributed in the hope that it will be useful,
>>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> - * Lesser General Public License for more details.
>>> - *
>>> - * You should have received a copy of the GNU Lesser General Public
>>> - * License along with this library; if not, see<http://www.gnu.org/licenses/>.
>>> + * This work is licensed under the terms of the GNU GPL, version 2 or (at your
>>> + * option) any later version.  See the COPYING file in the top-level directory.
>>>     */
>>>
>>>    #include "hw/hw.h"
>>>    #include "hw/boards.h"
>>> +#include "cpu.h"
>>> +#include "kvm.h"
>>> +
>>> +#if defined CONFIG_KVM
>>> +static void cpu_pre_save(void *opaque)
>>> +{
>>> +    CPUS390XState *env = opaque;
>>> +    struct kvm_fpu fpu;
>>> +    int i, r;
>>> +
>>> +    if (!kvm_enabled()) {
>>> +        return;
>>> +    }
>>> +
>>> +    r = kvm_vcpu_ioctl(env, KVM_GET_FPU,&fpu);
>>> +    assert(r == 0);
>>> +    for (i = 0; i<   16; i++) {
>>> +        env->fregs[i].ll = fpu.fprs[i];
>>> +    }
>>> +    env->fpc = fpu.fpc;
>>> +}
>>> +
>>> +static int cpu_post_load(void *opaque, int version_id)
>>> +{
>>> +    CPUS390XState *env = opaque;
>>> +    struct kvm_fpu fpu;
>>> +    int i, r;
>>> +
>>> +    if (!kvm_enabled()) {
>>> +        return 0;
>>> +    }
>>> +
>>> +    for (i = 0; i<   16; i++) {
>>> +        fpu.fprs[i] = env->fregs[i].ll;
>>> +    }
>>> +    fpu.fpc = env->fpc;
>>> +
>>> +    r = kvm_vcpu_ioctl(env, KVM_SET_FPU,&fpu);
>>> +    assert(r == 0);
>>> +
>>> +    return 0;
>>> +}
>> The kvm register sync needs to happen in the kvm register sync function :)
> That would eliminate the whole purpose of sync regs and forces us to have an
> expensive ioctl on lots of exits (again). I would prefer to sync the registers
> that we never need in qemu just here.

That's why the register sync has different stages.


Alex

Re: [Qemu-devel] [PATCH 1/3] s390/migration: Provide a cpu save for initial life migration work

[Christian Borntraeger]

On 21/11/12 16:02, Alexander Graf wrote:
> On 11/21/2012 03:59 PM, Christian Borntraeger wrote:
>> On 21/11/12 15:56, Alexander Graf wrote:
>>> On 11/21/2012 03:46 PM, Christian Borntraeger wrote:
>>>> This provides a simple cpu load and save function. With the recent
>>>> addition of sync regs we have the crs,acrs, the prefix and the
>>>> PSW already up to date. Lets also save the fpu via pre/post hooks.
>>>>
>>>> This patch also changes the license of machine.c to GPLv2 or later.
>>>> (The old code was just empty glue code, so there is no need
>>>> to go the "contributions after" way).
>>>>
>>>> Signed-off-by: Christian Borntraeger<borntraeger@de.ibm.com>
>>>> ---
>>>>    target-s390x/cpu.h     |    1 +
>>>>    target-s390x/machine.c |  115 ++++++++++++++++++++++++++++++++++++++++++------
>>>>    2 files changed, 103 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
>>>> index 0f9a1f7..ba695dd 100644
>>>> --- a/target-s390x/cpu.h
>>>> +++ b/target-s390x/cpu.h
>>>> @@ -27,6 +27,7 @@
>>>>    #define ELF_MACHINE    EM_S390
>>>>
>>>>    #define CPUArchState struct CPUS390XState
>>>> +#define CPU_SAVE_VERSION 1
>>>>
>>>>    #include "cpu-defs.h"
>>>>    #define TARGET_PAGE_BITS 12
>>>> diff --git a/target-s390x/machine.c b/target-s390x/machine.c
>>>> index 3e79be6..02706fd 100644
>>>> --- a/target-s390x/machine.c
>>>> +++ b/target-s390x/machine.c
>>>> @@ -2,29 +2,118 @@
>>>>     * QEMU S390x machine definitions
>>>>     *
>>>>     * Copyright (c) 2009 Alexander Graf<agraf@suse.de>
>>>> + * Copyright IBM Corp. 2012
>>>>     *
>>>> - * This library is free software; you can redistribute it and/or
>>>> - * modify it under the terms of the GNU Lesser General Public
>>>> - * License as published by the Free Software Foundation; either
>>>> - * version 2 of the License, or (at your option) any later version.
>>>> - *
>>>> - * This library is distributed in the hope that it will be useful,
>>>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>>> - * Lesser General Public License for more details.
>>>> - *
>>>> - * You should have received a copy of the GNU Lesser General Public
>>>> - * License along with this library; if not, see<http://www.gnu.org/licenses/>.
>>>> + * This work is licensed under the terms of the GNU GPL, version 2 or (at your
>>>> + * option) any later version.  See the COPYING file in the top-level directory.
>>>>     */
>>>>
>>>>    #include "hw/hw.h"
>>>>    #include "hw/boards.h"
>>>> +#include "cpu.h"
>>>> +#include "kvm.h"
>>>> +
>>>> +#if defined CONFIG_KVM
>>>> +static void cpu_pre_save(void *opaque)
>>>> +{
>>>> +    CPUS390XState *env = opaque;
>>>> +    struct kvm_fpu fpu;
>>>> +    int i, r;
>>>> +
>>>> +    if (!kvm_enabled()) {
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    r = kvm_vcpu_ioctl(env, KVM_GET_FPU,&fpu);
>>>> +    assert(r == 0);
>>>> +    for (i = 0; i<   16; i++) {
>>>> +        env->fregs[i].ll = fpu.fprs[i];
>>>> +    }
>>>> +    env->fpc = fpu.fpc;
>>>> +}
>>>> +
>>>> +static int cpu_post_load(void *opaque, int version_id)
>>>> +{
>>>> +    CPUS390XState *env = opaque;
>>>> +    struct kvm_fpu fpu;
>>>> +    int i, r;
>>>> +
>>>> +    if (!kvm_enabled()) {
>>>> +        return 0;
>>>> +    }
>>>> +
>>>> +    for (i = 0; i<   16; i++) {
>>>> +        fpu.fprs[i] = env->fregs[i].ll;
>>>> +    }
>>>> +    fpu.fpc = env->fpc;
>>>> +
>>>> +    r = kvm_vcpu_ioctl(env, KVM_SET_FPU,&fpu);
>>>> +    assert(r == 0);
>>>> +
>>>> +    return 0;
>>>> +}
>>> The kvm register sync needs to happen in the kvm register sync function :)
>> That would eliminate the whole purpose of sync regs and forces us to have an
>> expensive ioctl on lots of exits (again). I would prefer to sync the registers
>> that we never need in qemu just here.
> 
> That's why the register sync has different stages.

Not the get_register. Which is called on every synchronize_state. Which happen quite often
on s390.

Re: [Qemu-devel] [PATCH 1/3] s390/migration: Provide a cpu save for initial life migration work

[Alexander Graf]

On 11/21/2012 04:03 PM, Christian Borntraeger wrote:
> On 21/11/12 16:02, Alexander Graf wrote:
>> On 11/21/2012 03:59 PM, Christian Borntraeger wrote:
>>> On 21/11/12 15:56, Alexander Graf wrote:
>>>> On 11/21/2012 03:46 PM, Christian Borntraeger wrote:
>>>>> This provides a simple cpu load and save function. With the recent
>>>>> addition of sync regs we have the crs,acrs, the prefix and the
>>>>> PSW already up to date. Lets also save the fpu via pre/post hooks.
>>>>>
>>>>> This patch also changes the license of machine.c to GPLv2 or later.
>>>>> (The old code was just empty glue code, so there is no need
>>>>> to go the "contributions after" way).
>>>>>
>>>>> Signed-off-by: Christian Borntraeger<borntraeger@de.ibm.com>
>>>>> ---
>>>>>     target-s390x/cpu.h     |    1 +
>>>>>     target-s390x/machine.c |  115 ++++++++++++++++++++++++++++++++++++++++++------
>>>>>     2 files changed, 103 insertions(+), 13 deletions(-)
>>>>>
>>>>> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
>>>>> index 0f9a1f7..ba695dd 100644
>>>>> --- a/target-s390x/cpu.h
>>>>> +++ b/target-s390x/cpu.h
>>>>> @@ -27,6 +27,7 @@
>>>>>     #define ELF_MACHINE    EM_S390
>>>>>
>>>>>     #define CPUArchState struct CPUS390XState
>>>>> +#define CPU_SAVE_VERSION 1
>>>>>
>>>>>     #include "cpu-defs.h"
>>>>>     #define TARGET_PAGE_BITS 12
>>>>> diff --git a/target-s390x/machine.c b/target-s390x/machine.c
>>>>> index 3e79be6..02706fd 100644
>>>>> --- a/target-s390x/machine.c
>>>>> +++ b/target-s390x/machine.c
>>>>> @@ -2,29 +2,118 @@
>>>>>      * QEMU S390x machine definitions
>>>>>      *
>>>>>      * Copyright (c) 2009 Alexander Graf<agraf@suse.de>
>>>>> + * Copyright IBM Corp. 2012
>>>>>      *
>>>>> - * This library is free software; you can redistribute it and/or
>>>>> - * modify it under the terms of the GNU Lesser General Public
>>>>> - * License as published by the Free Software Foundation; either
>>>>> - * version 2 of the License, or (at your option) any later version.
>>>>> - *
>>>>> - * This library is distributed in the hope that it will be useful,
>>>>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>>>> - * Lesser General Public License for more details.
>>>>> - *
>>>>> - * You should have received a copy of the GNU Lesser General Public
>>>>> - * License along with this library; if not, see<http://www.gnu.org/licenses/>.
>>>>> + * This work is licensed under the terms of the GNU GPL, version 2 or (at your
>>>>> + * option) any later version.  See the COPYING file in the top-level directory.
>>>>>      */
>>>>>
>>>>>     #include "hw/hw.h"
>>>>>     #include "hw/boards.h"
>>>>> +#include "cpu.h"
>>>>> +#include "kvm.h"
>>>>> +
>>>>> +#if defined CONFIG_KVM
>>>>> +static void cpu_pre_save(void *opaque)
>>>>> +{
>>>>> +    CPUS390XState *env = opaque;
>>>>> +    struct kvm_fpu fpu;
>>>>> +    int i, r;
>>>>> +
>>>>> +    if (!kvm_enabled()) {
>>>>> +        return;
>>>>> +    }
>>>>> +
>>>>> +    r = kvm_vcpu_ioctl(env, KVM_GET_FPU,&fpu);
>>>>> +    assert(r == 0);
>>>>> +    for (i = 0; i<    16; i++) {
>>>>> +        env->fregs[i].ll = fpu.fprs[i];
>>>>> +    }
>>>>> +    env->fpc = fpu.fpc;
>>>>> +}
>>>>> +
>>>>> +static int cpu_post_load(void *opaque, int version_id)
>>>>> +{
>>>>> +    CPUS390XState *env = opaque;
>>>>> +    struct kvm_fpu fpu;
>>>>> +    int i, r;
>>>>> +
>>>>> +    if (!kvm_enabled()) {
>>>>> +        return 0;
>>>>> +    }
>>>>> +
>>>>> +    for (i = 0; i<    16; i++) {
>>>>> +        fpu.fprs[i] = env->fregs[i].ll;
>>>>> +    }
>>>>> +    fpu.fpc = env->fpc;
>>>>> +
>>>>> +    r = kvm_vcpu_ioctl(env, KVM_SET_FPU,&fpu);
>>>>> +    assert(r == 0);
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>> The kvm register sync needs to happen in the kvm register sync function :)
>>> That would eliminate the whole purpose of sync regs and forces us to have an
>>> expensive ioctl on lots of exits (again). I would prefer to sync the registers
>>> that we never need in qemu just here.
>> That's why the register sync has different stages.
> Not the get_register. Which is called on every synchronize_state. Which happen quite often
> on s390.

Sounds like bad design then :).

Maybe we should explicitly tell the register synchronization which 
register sets to sync, so that we don't waste time getting _all_ the 
state every time we sync registers?

Jan, ideas?


Alex

Re: [Qemu-devel] [PATCH 1/3] s390/migration: Provide a cpu save for initial life migration work

[Christian Borntraeger]

On 21/11/12 16:06, Alexander Graf wrote:
[...]
>>>>>> +static int cpu_post_load(void *opaque, int version_id)
>>>>>> +{
>>>>>> +    CPUS390XState *env = opaque;
>>>>>> +    struct kvm_fpu fpu;
>>>>>> +    int i, r;
>>>>>> +
>>>>>> +    if (!kvm_enabled()) {
>>>>>> +        return 0;
>>>>>> +    }
>>>>>> +
>>>>>> +    for (i = 0; i<    16; i++) {
>>>>>> +        fpu.fprs[i] = env->fregs[i].ll;
>>>>>> +    }
>>>>>> +    fpu.fpc = env->fpc;
>>>>>> +
>>>>>> +    r = kvm_vcpu_ioctl(env, KVM_SET_FPU,&fpu);
>>>>>> +    assert(r == 0);
>>>>>> +
>>>>>> +    return 0;
>>>>>> +}
>>>>> The kvm register sync needs to happen in the kvm register sync function :)
>>>> That would eliminate the whole purpose of sync regs and forces us to have an
>>>> expensive ioctl on lots of exits (again). I would prefer to sync the registers
>>>> that we never need in qemu just here.
>>> That's why the register sync has different stages.
>> Not the get_register. Which is called on every synchronize_state. Which happen quite often
>> on s390.
> 
> Sounds like bad design then :).
> 
> Maybe we should explicitly tell the register synchronization which register sets to sync, so that we don't waste time getting _all_ the state every time we sync registers?

Yes, a level statement for kvm_arch_get_registers would be good.

Re: [Qemu-devel] [PATCH 1/3] s390/migration: Provide a cpu save for initial life migration work

[Jan Kiszka]

On 2012-11-21 16:08, Christian Borntraeger wrote:
> On 21/11/12 16:06, Alexander Graf wrote:
> [...]
>>>>>>> +static int cpu_post_load(void *opaque, int version_id)
>>>>>>> +{
>>>>>>> +    CPUS390XState *env = opaque;
>>>>>>> +    struct kvm_fpu fpu;
>>>>>>> +    int i, r;
>>>>>>> +
>>>>>>> +    if (!kvm_enabled()) {
>>>>>>> +        return 0;
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    for (i = 0; i<    16; i++) {
>>>>>>> +        fpu.fprs[i] = env->fregs[i].ll;
>>>>>>> +    }
>>>>>>> +    fpu.fpc = env->fpc;
>>>>>>> +
>>>>>>> +    r = kvm_vcpu_ioctl(env, KVM_SET_FPU,&fpu);
>>>>>>> +    assert(r == 0);
>>>>>>> +
>>>>>>> +    return 0;
>>>>>>> +}
>>>>>> The kvm register sync needs to happen in the kvm register sync function :)
>>>>> That would eliminate the whole purpose of sync regs and forces us to have an
>>>>> expensive ioctl on lots of exits (again). I would prefer to sync the registers
>>>>> that we never need in qemu just here.
>>>> That's why the register sync has different stages.
>>> Not the get_register. Which is called on every synchronize_state. Which happen quite often
>>> on s390.
>>
>> Sounds like bad design then :).
>>
>> Maybe we should explicitly tell the register synchronization which register sets to sync, so that we don't waste time getting _all_ the state every time we sync registers?
> 
> Yes, a level statement for kvm_arch_get_registers would be good.
> 

The challenge is defining those levels generically - as it is also
generic code that calls cpu_synchronize_state. What levels do you have
in mind? And where would they be applied?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 1/3] s390/migration: Provide a cpu save for initial life migration work

[Christian Borntraeger]

On 21/11/12 16:22, Jan Kiszka wrote:
> On 2012-11-21 16:08, Christian Borntraeger wrote:
>> On 21/11/12 16:06, Alexander Graf wrote:
>> [...]
>>>>>>>> +static int cpu_post_load(void *opaque, int version_id)
>>>>>>>> +{
>>>>>>>> +    CPUS390XState *env = opaque;
>>>>>>>> +    struct kvm_fpu fpu;
>>>>>>>> +    int i, r;
>>>>>>>> +
>>>>>>>> +    if (!kvm_enabled()) {
>>>>>>>> +        return 0;
>>>>>>>> +    }
>>>>>>>> +
>>>>>>>> +    for (i = 0; i<    16; i++) {
>>>>>>>> +        fpu.fprs[i] = env->fregs[i].ll;
>>>>>>>> +    }
>>>>>>>> +    fpu.fpc = env->fpc;
>>>>>>>> +
>>>>>>>> +    r = kvm_vcpu_ioctl(env, KVM_SET_FPU,&fpu);
>>>>>>>> +    assert(r == 0);
>>>>>>>> +
>>>>>>>> +    return 0;
>>>>>>>> +}
>>>>>>> The kvm register sync needs to happen in the kvm register sync function :)
>>>>>> That would eliminate the whole purpose of sync regs and forces us to have an
>>>>>> expensive ioctl on lots of exits (again). I would prefer to sync the registers
>>>>>> that we never need in qemu just here.
>>>>> That's why the register sync has different stages.
>>>> Not the get_register. Which is called on every synchronize_state. Which happen quite often
>>>> on s390.
>>>
>>> Sounds like bad design then :).
>>>
>>> Maybe we should explicitly tell the register synchronization which register sets to sync, so that we don't waste time getting _all_ the state every time we sync registers?
>>
>> Yes, a level statement for kvm_arch_get_registers would be good.
>>
> 
> The challenge is defining those levels generically - as it is also
> generic code that calls cpu_synchronize_state. What levels do you have
> in mind? And where would they be applied?

I think that RUNTIME_STATE and FULL_STATE would be sufficient for the needs
that I have. The registers that I need during runtime can be accessed quite
fast, but for life migration I also need those registers that are accessed
via ONE_REG or other ioctls.

Christian

Re: [Qemu-devel] [PATCH 1/3] s390/migration: Provide a cpu save for initial life migration work

[Jan Kiszka]

On 2012-11-21 16:27, Christian Borntraeger wrote:
> On 21/11/12 16:22, Jan Kiszka wrote:
>> On 2012-11-21 16:08, Christian Borntraeger wrote:
>>> On 21/11/12 16:06, Alexander Graf wrote:
>>> [...]
>>>>>>>>> +static int cpu_post_load(void *opaque, int version_id)
>>>>>>>>> +{
>>>>>>>>> +    CPUS390XState *env = opaque;
>>>>>>>>> +    struct kvm_fpu fpu;
>>>>>>>>> +    int i, r;
>>>>>>>>> +
>>>>>>>>> +    if (!kvm_enabled()) {
>>>>>>>>> +        return 0;
>>>>>>>>> +    }
>>>>>>>>> +
>>>>>>>>> +    for (i = 0; i<    16; i++) {
>>>>>>>>> +        fpu.fprs[i] = env->fregs[i].ll;
>>>>>>>>> +    }
>>>>>>>>> +    fpu.fpc = env->fpc;
>>>>>>>>> +
>>>>>>>>> +    r = kvm_vcpu_ioctl(env, KVM_SET_FPU,&fpu);
>>>>>>>>> +    assert(r == 0);
>>>>>>>>> +
>>>>>>>>> +    return 0;
>>>>>>>>> +}
>>>>>>>> The kvm register sync needs to happen in the kvm register sync function :)
>>>>>>> That would eliminate the whole purpose of sync regs and forces us to have an
>>>>>>> expensive ioctl on lots of exits (again). I would prefer to sync the registers
>>>>>>> that we never need in qemu just here.
>>>>>> That's why the register sync has different stages.
>>>>> Not the get_register. Which is called on every synchronize_state. Which happen quite often
>>>>> on s390.
>>>>
>>>> Sounds like bad design then :).
>>>>
>>>> Maybe we should explicitly tell the register synchronization which register sets to sync, so that we don't waste time getting _all_ the state every time we sync registers?
>>>
>>> Yes, a level statement for kvm_arch_get_registers would be good.
>>>
>>
>> The challenge is defining those levels generically - as it is also
>> generic code that calls cpu_synchronize_state. What levels do you have
>> in mind? And where would they be applied?
> 
> I think that RUNTIME_STATE and FULL_STATE would be sufficient for the needs
> that I have. The registers that I need during runtime can be accessed quite
> fast, but for life migration I also need those registers that are accessed
> via ONE_REG or other ioctls.

OK, if all existing synchronization points remain FULL_STATE and only
s390-specific points become RUNTIME_STATE, I'm fine with it. Other archs
could then do their optimizations as the like (and actually need) to.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux