[Qemu-devel] [PATCH 0/2] support migration/save/load on AArch64 CPUs

[Qemu-devel] [PATCH 0/2] support migration/save/load on AArch64 CPUs

[Peter Maydell]

These patches implement support for migration/save/load on AArch64
CPUs. The first one from Alex (with some mangling from me) just
moves the sysreg sync code we have for 32-bit across to 64-bit.
The second fills in the gaps in the main CPU vmstate struct.

Notes:
 * currently doing 'from cold' incoming migration with -loadvm
   or -incoming command line arguments is broken if using the VGIC;
   this is a kernel bug, fixed by this patch:
   http://www.spinics.net/lists/arm-kernel/msg383736.html
 * Pranav: patch 1 here is what you need to rebase the bigendian
   virtio patch on so it can use a single common hook for 32 and
   64 bit without hacks to work around missing register sync
 * Christoffer: patch 1 is also the one which moves reset around
 
Alex Bennée (1):
  target-arm/kvm: make reg sync code common between kvm32/64

Peter Maydell (1):
  target-arm: Support save/load for 64 bit CPUs

 target-arm/kvm.c     | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/kvm32.c   | 94 ++-----------------------------------------------
 target-arm/kvm64.c   | 24 +++++++------
 target-arm/kvm_arm.h | 22 ++++++++++++
 target-arm/machine.c | 22 ++++++++++--
 5 files changed, 156 insertions(+), 104 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH 1/2] target-arm/kvm: make reg sync code common between kvm32/64

[Peter Maydell]

From: Alex Bennée <alex.bennee@linaro.org>

Before we launch a guest we query KVM for the list of "co-processor"
registers it knows about. This is used to synchronize system
register state for the bulk of coprocessor/system registers.
Move this code from the 32-bit specific vcpu init function into
a common routine and call it also from the 64-bit vcpu init.

This allows system registers to migrate correctly when using
KVM, and also permits QEMU code to see the current KVM register
state (which will be needed to support big-endian guests, since
the virtio endianness callback must check for some system register
settings).

Since vcpu reset also has to sync registers, we move the
32 bit kvm_arm_reset_vcpu() into common code as well and
share it with the 64 bit version.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
[PMM: just copy the 32-bit code rather than improving it along the way;
 don't share reg_syncs_via_tuple_list() between 32 and 64 bit;
 tweak function names; move reset]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/kvm.c     | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/kvm32.c   | 94 ++-----------------------------------------------
 target-arm/kvm64.c   | 24 +++++++------
 target-arm/kvm_arm.h | 22 ++++++++++++
 4 files changed, 137 insertions(+), 101 deletions(-)

diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index 319784d..191e759 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -21,6 +21,7 @@
 #include "sysemu/kvm.h"
 #include "kvm_arm.h"
 #include "cpu.h"
+#include "internals.h"
 #include "hw/arm/arm.h"
 
 const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
@@ -279,6 +280,94 @@ void kvm_arm_register_device(MemoryRegion *mr, uint64_t devid, uint64_t group,
     memory_region_ref(kd->mr);
 }
 
+static int compare_u64(const void *a, const void *b)
+{
+    if (*(uint64_t *)a > *(uint64_t *)b) {
+        return 1;
+    }
+    if (*(uint64_t *)a < *(uint64_t *)b) {
+        return -1;
+    }
+    return 0;
+}
+
+/* Initialize the CPUState's cpreg list according to the kernel's
+ * definition of what CPU registers it knows about (and throw away
+ * the previous TCG-created cpreg list).
+ */
+int kvm_arm_init_cpreg_list(ARMCPU *cpu)
+{
+    struct kvm_reg_list rl;
+    struct kvm_reg_list *rlp;
+    int i, ret, arraylen;
+    CPUState *cs = CPU(cpu);
+
+    rl.n = 0;
+    ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, &rl);
+    if (ret != -E2BIG) {
+        return ret;
+    }
+    rlp = g_malloc(sizeof(struct kvm_reg_list) + rl.n * sizeof(uint64_t));
+    rlp->n = rl.n;
+    ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, rlp);
+    if (ret) {
+        goto out;
+    }
+    /* Sort the list we get back from the kernel, since cpreg_tuples
+     * must be in strictly ascending order.
+     */
+    qsort(&rlp->reg, rlp->n, sizeof(rlp->reg[0]), compare_u64);
+
+    for (i = 0, arraylen = 0; i < rlp->n; i++) {
+        if (!kvm_arm_reg_syncs_via_cpreg_list(rlp->reg[i])) {
+            continue;
+        }
+        switch (rlp->reg[i] & KVM_REG_SIZE_MASK) {
+        case KVM_REG_SIZE_U32:
+        case KVM_REG_SIZE_U64:
+            break;
+        default:
+            fprintf(stderr, "Can't handle size of register in kernel list\n");
+            ret = -EINVAL;
+            goto out;
+        }
+
+        arraylen++;
+    }
+
+    cpu->cpreg_indexes = g_renew(uint64_t, cpu->cpreg_indexes, arraylen);
+    cpu->cpreg_values = g_renew(uint64_t, cpu->cpreg_values, arraylen);
+    cpu->cpreg_vmstate_indexes = g_renew(uint64_t, cpu->cpreg_vmstate_indexes,
+                                         arraylen);
+    cpu->cpreg_vmstate_values = g_renew(uint64_t, cpu->cpreg_vmstate_values,
+                                        arraylen);
+    cpu->cpreg_array_len = arraylen;
+    cpu->cpreg_vmstate_array_len = arraylen;
+
+    for (i = 0, arraylen = 0; i < rlp->n; i++) {
+        uint64_t regidx = rlp->reg[i];
+        if (!kvm_arm_reg_syncs_via_cpreg_list(regidx)) {
+            continue;
+        }
+        cpu->cpreg_indexes[arraylen] = regidx;
+        arraylen++;
+    }
+    assert(cpu->cpreg_array_len == arraylen);
+
+    if (!write_kvmstate_to_list(cpu)) {
+        /* Shouldn't happen unless kernel is inconsistent about
+         * what registers exist.
+         */
+        fprintf(stderr, "Initial read of kernel register state failed\n");
+        ret = -EINVAL;
+        goto out;
+    }
+
+out:
+    g_free(rlp);
+    return ret;
+}
+
 bool write_kvmstate_to_list(ARMCPU *cpu)
 {
     CPUState *cs = CPU(cpu);
@@ -351,6 +440,15 @@ bool write_list_to_kvmstate(ARMCPU *cpu)
     return ok;
 }
 
+void kvm_arm_reset_vcpu(ARMCPU *cpu)
+{
+    /* Re-init VCPU so that all registers are set to
+     * their respective reset values.
+     */
+    kvm_arm_vcpu_init(CPU(cpu));
+    write_kvmstate_to_list(cpu);
+}
+
 void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
 {
 }
diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
index 5ec4eb1..73fe96f 100644
--- a/target-arm/kvm32.c
+++ b/target-arm/kvm32.c
@@ -138,7 +138,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
     return true;
 }
 
-static bool reg_syncs_via_tuple_list(uint64_t regidx)
+bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx)
 {
     /* Return true if the regidx is a register we should synchronize
      * via the cpreg_tuples array (ie is not a core reg we sync by
@@ -153,24 +153,11 @@ static bool reg_syncs_via_tuple_list(uint64_t regidx)
     }
 }
 
-static int compare_u64(const void *a, const void *b)
-{
-    if (*(uint64_t *)a > *(uint64_t *)b) {
-        return 1;
-    }
-    if (*(uint64_t *)a < *(uint64_t *)b) {
-        return -1;
-    }
-    return 0;
-}
-
 int kvm_arch_init_vcpu(CPUState *cs)
 {
-    int i, ret, arraylen;
+    int ret;
     uint64_t v;
     struct kvm_one_reg r;
-    struct kvm_reg_list rl;
-    struct kvm_reg_list *rlp;
     ARMCPU *cpu = ARM_CPU(cs);
 
     if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE) {
@@ -206,73 +193,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
         return -EINVAL;
     }
 
-    /* Populate the cpreg list based on the kernel's idea
-     * of what registers exist (and throw away the TCG-created list).
-     */
-    rl.n = 0;
-    ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, &rl);
-    if (ret != -E2BIG) {
-        return ret;
-    }
-    rlp = g_malloc(sizeof(struct kvm_reg_list) + rl.n * sizeof(uint64_t));
-    rlp->n = rl.n;
-    ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, rlp);
-    if (ret) {
-        goto out;
-    }
-    /* Sort the list we get back from the kernel, since cpreg_tuples
-     * must be in strictly ascending order.
-     */
-    qsort(&rlp->reg, rlp->n, sizeof(rlp->reg[0]), compare_u64);
-
-    for (i = 0, arraylen = 0; i < rlp->n; i++) {
-        if (!reg_syncs_via_tuple_list(rlp->reg[i])) {
-            continue;
-        }
-        switch (rlp->reg[i] & KVM_REG_SIZE_MASK) {
-        case KVM_REG_SIZE_U32:
-        case KVM_REG_SIZE_U64:
-            break;
-        default:
-            fprintf(stderr, "Can't handle size of register in kernel list\n");
-            ret = -EINVAL;
-            goto out;
-        }
-
-        arraylen++;
-    }
-
-    cpu->cpreg_indexes = g_renew(uint64_t, cpu->cpreg_indexes, arraylen);
-    cpu->cpreg_values = g_renew(uint64_t, cpu->cpreg_values, arraylen);
-    cpu->cpreg_vmstate_indexes = g_renew(uint64_t, cpu->cpreg_vmstate_indexes,
-                                         arraylen);
-    cpu->cpreg_vmstate_values = g_renew(uint64_t, cpu->cpreg_vmstate_values,
-                                        arraylen);
-    cpu->cpreg_array_len = arraylen;
-    cpu->cpreg_vmstate_array_len = arraylen;
-
-    for (i = 0, arraylen = 0; i < rlp->n; i++) {
-        uint64_t regidx = rlp->reg[i];
-        if (!reg_syncs_via_tuple_list(regidx)) {
-            continue;
-        }
-        cpu->cpreg_indexes[arraylen] = regidx;
-        arraylen++;
-    }
-    assert(cpu->cpreg_array_len == arraylen);
-
-    if (!write_kvmstate_to_list(cpu)) {
-        /* Shouldn't happen unless kernel is inconsistent about
-         * what registers exist.
-         */
-        fprintf(stderr, "Initial read of kernel register state failed\n");
-        ret = -EINVAL;
-        goto out;
-    }
-
-out:
-    g_free(rlp);
-    return ret;
+    return kvm_arm_init_cpreg_list(cpu);
 }
 
 typedef struct Reg {
@@ -508,12 +429,3 @@ int kvm_arch_get_registers(CPUState *cs)
 
     return 0;
 }
-
-void kvm_arm_reset_vcpu(ARMCPU *cpu)
-{
-    /* Re-init VCPU so that all registers are set to
-     * their respective reset values.
-     */
-    kvm_arm_vcpu_init(CPU(cpu));
-    write_kvmstate_to_list(cpu);
-}
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index c615286..ba16821 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -103,9 +103,21 @@ int kvm_arch_init_vcpu(CPUState *cs)
         return ret;
     }
 
-    /* TODO : support for save/restore/reset of system regs via tuple list */
+    return kvm_arm_init_cpreg_list(cpu);
+}
 
-    return 0;
+bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx)
+{
+    /* Return true if the regidx is a register we should synchronize
+     * via the cpreg_tuples array (ie is not a core reg we sync by
+     * hand in kvm_arch_get/put_registers())
+     */
+    switch (regidx & KVM_REG_ARM_COPROC_MASK) {
+    case KVM_REG_ARM_CORE:
+        return false;
+    default:
+        return true;
+    }
 }
 
 #define AARCH64_CORE_REG(x)   (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \
@@ -260,11 +272,3 @@ int kvm_arch_get_registers(CPUState *cs)
     /* TODO: other registers */
     return ret;
 }
-
-void kvm_arm_reset_vcpu(ARMCPU *cpu)
-{
-    /* Re-init VCPU so that all registers are set to
-     * their respective reset values.
-     */
-    kvm_arm_vcpu_init(CPU(cpu));
-}
diff --git a/target-arm/kvm_arm.h b/target-arm/kvm_arm.h
index af93105..455dea3 100644
--- a/target-arm/kvm_arm.h
+++ b/target-arm/kvm_arm.h
@@ -47,6 +47,28 @@ void kvm_arm_register_device(MemoryRegion *mr, uint64_t devid, uint64_t group,
                              uint64_t attr, int dev_fd);
 
 /**
+ * kvm_arm_init_cpreg_list:
+ * @cs: CPUState
+ *
+ * Initialize the CPUState's cpreg list according to the kernel's
+ * definition of what CPU registers it knows about (and throw away
+ * the previous TCG-created cpreg list).
+ *
+ * Returns: 0 if success, else < 0 error code
+ */
+int kvm_arm_init_cpreg_list(ARMCPU *cpu);
+
+/**
+ * kvm_arm_reg_syncs_via_cpreg_list
+ * regidx: KVM register index
+ *
+ * Return true if this KVM register should be synchronized via the
+ * cpreg list of arbitrary system registers, false if it is synchronized
+ * by hand using code in kvm_arch_get/put_registers().
+ */
+bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx);
+
+/**
  * write_list_to_kvmstate:
  * @cpu: ARMCPU
  *
-- 
1.9.1

[Qemu-devel] [PATCH 2/2] target-arm: Support save/load for 64 bit CPUs

[Peter Maydell]

For migration to work on 64 bit CPUs, we need to include both
the 64-bit integer register file and the PSTATE. Everything
else is either stored in the same place as existing 32-bit CPU
state or handled by the generic sysreg mechanism.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
The pstate save/load is a little ugly and suggests that we
could perhaps benefit from unifying the state storage/access
for 64-bit pstate and 32-bit PSR; however Alex had a go at
that a while back and it turns out to be trickier than it
looks. This is a pragmatic fix which makes save/load work
(and the on-the-wire state is right, at least).
---
 target-arm/machine.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/target-arm/machine.c b/target-arm/machine.c
index 6437690..c29e7a2 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -127,6 +127,13 @@ static int get_cpsr(QEMUFile *f, void *opaque, size_t size)
     CPUARMState *env = &cpu->env;
     uint32_t val = qemu_get_be32(f);
 
+    env->aarch64 = ((val & PSTATE_nRW) == 0);
+
+    if (is_a64(env)) {
+        pstate_write(env, val);
+        return 0;
+    }
+
     /* Avoid mode switch when restoring CPSR */
     env->uncached_cpsr = val & CPSR_M;
     cpsr_write(env, val, 0xffffffff);
@@ -137,8 +144,15 @@ static void put_cpsr(QEMUFile *f, void *opaque, size_t size)
 {
     ARMCPU *cpu = opaque;
     CPUARMState *env = &cpu->env;
+    uint32_t val;
+
+    if (is_a64(env)) {
+        val = pstate_read(env);
+    } else {
+        val = cpsr_read(env);
+    }
 
-    qemu_put_be32(f, cpsr_read(env));
+    qemu_put_be32(f, val);
 }
 
 static const VMStateInfo vmstate_cpsr = {
@@ -222,12 +236,14 @@ static int cpu_post_load(void *opaque, int version_id)
 
 const VMStateDescription vmstate_arm_cpu = {
     .name = "cpu",
-    .version_id = 21,
-    .minimum_version_id = 21,
+    .version_id = 22,
+    .minimum_version_id = 22,
     .pre_save = cpu_pre_save,
     .post_load = cpu_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32_ARRAY(env.regs, ARMCPU, 16),
+        VMSTATE_UINT64_ARRAY(env.xregs, ARMCPU, 32),
+        VMSTATE_UINT64(env.pc, ARMCPU),
         {
             .name = "cpsr",
             .version_id = 0,
-- 
1.9.1

Re: [Qemu-devel] [PATCH 0/2] support migration/save/load on AArch64 CPUs

[Pranavkumar Sawargaonkar]

Hi PMM,

On 5 December 2014 at 19:41, Peter Maydell <peter.maydell@linaro.org> wrote:
> These patches implement support for migration/save/load on AArch64
> CPUs. The first one from Alex (with some mangling from me) just
> moves the sysreg sync code we have for 32-bit across to 64-bit.
> The second fills in the gaps in the main CPU vmstate struct.
>
> Notes:
>  * currently doing 'from cold' incoming migration with -loadvm
>    or -incoming command line arguments is broken if using the VGIC;
>    this is a kernel bug, fixed by this patch:
>    http://www.spinics.net/lists/arm-kernel/msg383736.html
>  * Pranav: patch 1 here is what you need to rebase the bigendian
>    virtio patch on so it can use a single common hook for 32 and
>    64 bit without hacks to work around missing register sync

Thanks for the patch.
I have applied this patch and called cpu_synchronize_state() to get
sctlr_el1 value in env->cp15.c1_sys  during virtio endianness
determination while ARM64 guest was booting , but value getting read
is 0.
Do I need to do anything else ?

Thanks,
Pranav

>  * Christoffer: patch 1 is also the one which moves reset around
>
> Alex Bennée (1):
>   target-arm/kvm: make reg sync code common between kvm32/64
>
> Peter Maydell (1):
>   target-arm: Support save/load for 64 bit CPUs
>
>  target-arm/kvm.c     | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  target-arm/kvm32.c   | 94 ++-----------------------------------------------
>  target-arm/kvm64.c   | 24 +++++++------
>  target-arm/kvm_arm.h | 22 ++++++++++++
>  target-arm/machine.c | 22 ++++++++++--
>  5 files changed, 156 insertions(+), 104 deletions(-)
>
> --
> 1.9.1
>

Re: [Qemu-devel] [PATCH 0/2] support migration/save/load on AArch64 CPUs

[Peter Maydell]

On 9 December 2014 at 11:07, Pranavkumar Sawargaonkar
<pranavkumar@linaro.org> wrote:
> Thanks for the patch.
> I have applied this patch and called cpu_synchronize_state() to get
> sctlr_el1 value in env->cp15.c1_sys  during virtio endianness
> determination while ARM64 guest was booting , but value getting read
> is 0.
> Do I need to do anything else ?

Hmm, on a 64 bit host that should work. (There's an issue with
32-bit hosts that means we won't sync registers properly which
I'm working on, but it shouldn't affect 64-bit.) I suggest you
debug further to see why the sync doesn't get the right SCTLR
value across.

-- PMM