diff mbox

[v2,10/10] target-arm/kvm: make reg sync code common between kvm32/64

Message ID 1405007407-23549-11-git-send-email-alex.bennee@linaro.org
State New
Headers show

Commit Message

Alex Bennée July 10, 2014, 3:50 p.m. UTC
Before we launch a guest we query KVM for the list of "co-processor"
registers it knows about which is used later for save/restore of machine
state. The logic is identical for both 32-bit and 64-bit so I've moved
it all into the common code and simplified the exit paths (as failure =>
exit).

This list may well have more registers than are known by the TCG
emulation which is not necessarily a problem but it does stop us from
migrating between KVM and TCG hosted guests. I've added some additional
checking to report those registers under -d unimp.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Comments

Peter Maydell Aug. 4, 2014, 1:04 p.m. UTC | #1
On 10 July 2014 16:50, Alex Bennée <alex.bennee@linaro.org> wrote:
> Before we launch a guest we query KVM for the list of "co-processor"
> registers it knows about which is used later for save/restore of machine
> state. The logic is identical for both 32-bit and 64-bit so I've moved
> it all into the common code and simplified the exit paths (as failure =>
> exit).
>
> This list may well have more registers than are known by the TCG
> emulation which is not necessarily a problem but it does stop us from
> migrating between KVM and TCG hosted guests. I've added some additional
> checking to report those registers under -d unimp.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

This definitely shouldn't be in this patchset...

thanks
-- PMM
diff mbox

Patch

diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index 72e242d..a2895dc 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[] = {
@@ -289,6 +290,130 @@  static void failed_cpreg_operation(ARMCPU *cpu, uint64_t regidx, int ret,
                   func, ret, regidx, cpreg ? cpreg->name : "unknown");
 }
 
+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;
+}
+
+static bool reg_syncs_via_tuple_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:
+#ifdef KVM_REG_ARM_VFP
+    case KVM_REG_ARM_VFP:
+#endif
+        return false;
+    default:
+        return true;
+    }
+}
+
+/*
+ * Fetch a list of registers from KVM that we will need to be able to
+ * migrate the state. These registers may or may not map onto real
+ * hardware registers but either way QEMU uses the KVM_GET/SET_ONE_REG
+ * api to copy their state back and forth when required.
+ *
+ * For migration between KVM and TCG both models need to understand
+ * the same set of registers.
+ *
+ * If we exit due to failure we would leak memory but we'll be exiting
+ * anyway so the return path is kept simple.
+ */
+bool kvm_arm_sync_register_list(CPUState *cs)
+{
+    struct kvm_reg_list rl;
+    struct kvm_reg_list *rlp;
+    int i, j, ret, arraylen;
+    ARMCPU *cpu = ARM_CPU(cs);
+
+    /* 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 FALSE;
+    }
+
+    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) {
+        fprintf(stderr, "%s: failed to get register list\n", __func__);
+        return FALSE;
+    }
+    /* 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);
+
+    /* Count how many of these registers we'll actually sync through
+     * the cpreg_indexes mechanism and overwrite the existing TCG
+     * built array of registers.
+     */
+    for (i = 0, arraylen = 0; i < rlp->n; i++) {
+        uint64_t regidx = rlp->reg[i];
+        if (reg_syncs_via_tuple_list(regidx)) {
+            gboolean found = FALSE;
+            arraylen++;
+            for (j = 0; j < cpu->cpreg_array_len; j++) {
+                if (regidx == cpu->cpreg_indexes[j]) {
+                    found = TRUE;
+                    break;
+                }
+            }
+            if (!found) {
+                qemu_log_mask(LOG_UNIMP,
+                              "%s: TCG missing definition of %"PRIx64"\n",
+                              __func__, regidx);
+            }
+        }
+    }
+
+    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;
+        }
+        switch (regidx & KVM_REG_SIZE_MASK) {
+        case KVM_REG_SIZE_U32:
+        case KVM_REG_SIZE_U64:
+            break;
+        default:
+            fprintf(stderr,
+                    "%s: un-handled register size (%"PRIx64") in kernel list\n",
+                    __func__, regidx);
+            return FALSE;
+        }
+        cpu->cpreg_indexes[arraylen] = regidx;
+        arraylen++;
+    }
+
+    g_assert(cpu->cpreg_array_len == arraylen);
+
+    return TRUE;
+}
+
 bool write_kvmstate_to_list(ARMCPU *cpu)
 {
     CPUState *cs = CPU(cpu);
diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
index 39117c7..adfc902 100644
--- a/target-arm/kvm32.c
+++ b/target-arm/kvm32.c
@@ -138,39 +138,11 @@  bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
     return true;
 }
 
-static bool reg_syncs_via_tuple_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:
-    case KVM_REG_ARM_VFP:
-        return false;
-    default:
-        return true;
-    }
-}
-
-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 i, 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 +178,17 @@  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++;
+    if (!kvm_arm_sync_register_list(cpu)) {
+        return -EINVAL;
     }
-    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;
+        return -EINVAL;
     }
-
-out:
-    g_free(rlp);
-    return ret;
 }
 
 typedef struct Reg {
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index 7a022a6..0e28901 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -102,7 +102,7 @@  int kvm_arch_init_vcpu(CPUState *cs)
         return ret;
     }
 
-    /* TODO : support for save/restore/reset of system regs via tuple list */
+    kvm_arm_sync_register_list(cs);
 
     return 0;
 }
diff --git a/target-arm/kvm_arm.h b/target-arm/kvm_arm.h
index af93105..2efd0b7 100644
--- a/target-arm/kvm_arm.h
+++ b/target-arm/kvm_arm.h
@@ -47,6 +47,18 @@  void kvm_arm_register_device(MemoryRegion *mr, uint64_t devid, uint64_t group,
                              uint64_t attr, int dev_fd);
 
 /**
+ * kvm_arm_sync_register_list:
+ * @cs: CPUState
+ *
+ * Before migration can occur we need to sync the list of additional
+ * registers that KVM knows about which we can then use when we start
+ * doing migration. It's OK for the TCG side not to know about
+ * registers exposed by the KVM side although this will break
+ * migration between the two VM types.
+ */
+bool kvm_arm_sync_register_list(CPUState *cs);
+
+/**
  * write_list_to_kvmstate:
  * @cpu: ARMCPU
  *