target-arm: Use tuple list to sync cp regs with KVM

Use the tuple list of cp registers for syncing KVM state to QEMU
and for migration of register state when using KVM.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

1 files changed, 196 insertions, 71 deletions

diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index 4aea7c3390..b9c9127d9c 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -50,12 +50,35 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu)
     return cpu->cpu_index;
 }
 
+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)
+{
+    return *(uint64_t *)a - *(uint64_t *)b;
+}
+
 int kvm_arch_init_vcpu(CPUState *cs)
 {
     struct kvm_vcpu_init init;
-    int ret;
+    int i, ret, arraylen;
     uint64_t v;
     struct kvm_one_reg r;
+    struct kvm_reg_list rl;
+    struct kvm_reg_list *rlp;
+    ARMCPU *cpu = ARM_CPU(cs);
 
     init.target = KVM_ARM_TARGET_CORTEX_A15;
     memset(init.features, 0, sizeof(init.features));
@@ -74,6 +97,73 @@ int kvm_arch_init_vcpu(CPUState *cs)
     if (ret == -ENOENT) {
         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;
 }
 
@@ -163,6 +253,78 @@ void kvm_arm_register_device(MemoryRegion *mr, uint64_t devid)
     QSLIST_INSERT_HEAD(&kvm_devices_head, kd, entries);
 }
 
+bool write_kvmstate_to_list(ARMCPU *cpu)
+{
+    CPUState *cs = CPU(cpu);
+    int i;
+    bool ok = true;
+
+    for (i = 0; i < cpu->cpreg_array_len; i++) {
+        struct kvm_one_reg r;
+        uint64_t regidx = cpu->cpreg_indexes[i];
+        uint32_t v32;
+        int ret;
+
+        r.id = regidx;
+
+        switch (regidx & KVM_REG_SIZE_MASK) {
+        case KVM_REG_SIZE_U32:
+            r.addr = (uintptr_t)&v32;
+            ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
+            if (!ret) {
+                cpu->cpreg_values[i] = v32;
+            }
+            break;
+        case KVM_REG_SIZE_U64:
+            r.addr = (uintptr_t)(cpu->cpreg_values + i);
+            ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
+            break;
+        default:
+            abort();
+        }
+        if (ret) {
+            ok = false;
+        }
+    }
+    return ok;
+}
+
+bool write_list_to_kvmstate(ARMCPU *cpu)
+{
+    CPUState *cs = CPU(cpu);
+    int i;
+    bool ok = true;
+
+    for (i = 0; i < cpu->cpreg_array_len; i++) {
+        struct kvm_one_reg r;
+        uint64_t regidx = cpu->cpreg_indexes[i];
+        uint32_t v32;
+        int ret;
+
+        r.id = regidx;
+        switch (regidx & KVM_REG_SIZE_MASK) {
+        case KVM_REG_SIZE_U32:
+            v32 = cpu->cpreg_values[i];
+            r.addr = (uintptr_t)&v32;
+            break;
+        case KVM_REG_SIZE_U64:
+            r.addr = (uintptr_t)(cpu->cpreg_values + i);
+            break;
+        default:
+            abort();
+        }
+        ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
+        if (ret) {
+            /* We might fail for "unknown register" and also for
+             * "you tried to set a register which is constant with
+             * a different value from what it actually contains".
+             */
+            ok = false;
+        }
+    }
+    return ok;
+}
+
 typedef struct Reg {
     uint64_t id;
     int offset;
@@ -175,17 +337,6 @@ typedef struct Reg {
         offsetof(CPUARMState, QEMUFIELD)                     \
     }
 
-#define CP15REG(CRN, CRM, OPC1, OPC2, QEMUFIELD) \
-    {                                            \
-        KVM_REG_ARM | KVM_REG_SIZE_U32 |         \
-        (15 << KVM_REG_ARM_COPROC_SHIFT) |       \
-        ((CRN) << KVM_REG_ARM_32_CRN_SHIFT) |    \
-        ((CRM) << KVM_REG_ARM_CRM_SHIFT) |       \
-        ((OPC1) << KVM_REG_ARM_OPC1_SHIFT) |     \
-        ((OPC2) << KVM_REG_ARM_32_OPC2_SHIFT),   \
-        offsetof(CPUARMState, QEMUFIELD)         \
-    }
-
 #define VFPSYSREG(R)                                       \
     {                                                      \
         KVM_REG_ARM | KVM_REG_SIZE_U32 | KVM_REG_ARM_VFP | \
@@ -234,12 +385,6 @@ static const Reg regs[] = {
     COREREG(fiq_regs[7], banked_spsr[5]),
     /* R15 */
     COREREG(usr_regs.uregs[15], regs[15]),
-    /* A non-comprehensive set of cp15 registers.
-     * TODO: drive this from the cp_regs hashtable instead.
-     */
-    CP15REG(1, 0, 0, 0, cp15.c1_sys), /* SCTLR */
-    CP15REG(2, 0, 0, 2, cp15.c2_control), /* TTBCR */
-    CP15REG(3, 0, 0, 0, cp15.c3), /* DACR */
     /* VFP system registers */
     VFPSYSREG(FPSID),
     VFPSYSREG(MVFR1),
@@ -257,7 +402,6 @@ int kvm_arch_put_registers(CPUState *cs, int level)
     int mode, bn;
     int ret, i;
     uint32_t cpsr, fpscr;
-    uint64_t ttbr;
 
     /* Make sure the banked regs are properly set */
     mode = env->uncached_cpsr & CPSR_M;
@@ -291,26 +435,6 @@ int kvm_arch_put_registers(CPUState *cs, int level)
         return ret;
     }
 
-    /* TTBR0: cp15 crm=2 opc1=0 */
-    ttbr = ((uint64_t)env->cp15.c2_base0_hi << 32) | env->cp15.c2_base0;
-    r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | (15 << KVM_REG_ARM_COPROC_SHIFT) |
-        (2 << KVM_REG_ARM_CRM_SHIFT) | (0 << KVM_REG_ARM_OPC1_SHIFT);
-    r.addr = (uintptr_t)(&ttbr);
-    ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
-    if (ret) {
-        return ret;
-    }
-
-    /* TTBR1: cp15 crm=2 opc1=1 */
-    ttbr = ((uint64_t)env->cp15.c2_base1_hi << 32) | env->cp15.c2_base1;
-    r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | (15 << KVM_REG_ARM_COPROC_SHIFT) |
-        (2 << KVM_REG_ARM_CRM_SHIFT) | (1 << KVM_REG_ARM_OPC1_SHIFT);
-    r.addr = (uintptr_t)(&ttbr);
-    ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
-    if (ret) {
-        return ret;
-    }
-
     /* VFP registers */
     r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | KVM_REG_ARM_VFP;
     for (i = 0; i < 32; i++) {
@@ -327,6 +451,31 @@ int kvm_arch_put_registers(CPUState *cs, int level)
     fpscr = vfp_get_fpscr(env);
     r.addr = (uintptr_t)&fpscr;
     ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
+    if (ret) {
+        return ret;
+    }
+
+    /* Note that we do not call write_cpustate_to_list()
+     * here, so we are only writing the tuple list back to
+     * KVM. This is safe because nothing can change the
+     * CPUARMState cp15 fields (in particular gdb accesses cannot)
+     * and so there are no changes to sync. In fact syncing would
+     * be wrong at this point: for a constant register where TCG and
+     * KVM disagree about its value, the preceding write_list_to_cpustate()
+     * would not have had any effect on the CPUARMState value (since the
+     * register is read-only), and a write_cpustate_to_list() here would
+     * then try to write the TCG value back into KVM -- this would either
+     * fail or incorrectly change the value the guest sees.
+     *
+     * If we ever want to allow the user to modify cp15 registers via
+     * the gdb stub, we would need to be more clever here (for instance
+     * tracking the set of registers kvm_arch_get_registers() successfully
+     * managed to update the CPUARMState with, and only allowing those
+     * to be written back up into the kernel).
+     */
+    if (!write_list_to_kvmstate(cpu)) {
+        return EINVAL;
+    }
 
     return ret;
 }
@@ -339,7 +488,6 @@ int kvm_arch_get_registers(CPUState *cs)
     int mode, bn;
     int ret, i;
     uint32_t cpsr, fpscr;
-    uint64_t ttbr;
 
     for (i = 0; i < ARRAY_SIZE(regs); i++) {
         r.id = regs[i].id;
@@ -360,28 +508,6 @@ int kvm_arch_get_registers(CPUState *cs)
     }
     cpsr_write(env, cpsr, 0xffffffff);
 
-    /* TTBR0: cp15 crm=2 opc1=0 */
-    r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | (15 << KVM_REG_ARM_COPROC_SHIFT) |
-        (2 << KVM_REG_ARM_CRM_SHIFT) | (0 << KVM_REG_ARM_OPC1_SHIFT);
-    r.addr = (uintptr_t)(&ttbr);
-    ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
-    if (ret) {
-        return ret;
-    }
-    env->cp15.c2_base0_hi = ttbr >> 32;
-    env->cp15.c2_base0 = ttbr;
-
-    /* TTBR1: cp15 crm=2 opc1=1 */
-    r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | (15 << KVM_REG_ARM_COPROC_SHIFT) |
-        (2 << KVM_REG_ARM_CRM_SHIFT) | (1 << KVM_REG_ARM_OPC1_SHIFT);
-    r.addr = (uintptr_t)(&ttbr);
-    ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
-    if (ret) {
-        return ret;
-    }
-    env->cp15.c2_base1_hi = ttbr >> 32;
-    env->cp15.c2_base1 = ttbr;
-
     /* Make sure the current mode regs are properly set */
     mode = env->uncached_cpsr & CPSR_M;
     bn = bank_number(mode);
@@ -394,15 +520,6 @@ int kvm_arch_get_registers(CPUState *cs)
     env->regs[14] = env->banked_r14[bn];
     env->spsr = env->banked_spsr[bn];
 
-    /* The main GET_ONE_REG loop above set c2_control, but we need to
-     * update some extra cached precomputed values too.
-     * When this is driven from the cp_regs hashtable then this ugliness
-     * can disappear because we'll use the access function which sets
-     * these values automatically.
-     */
-    env->cp15.c2_mask = ~(0xffffffffu >> env->cp15.c2_control);
-    env->cp15.c2_base_mask = ~(0x3fffu >> env->cp15.c2_control);
-
     /* VFP registers */
     r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | KVM_REG_ARM_VFP;
     for (i = 0; i < 32; i++) {
@@ -423,6 +540,14 @@ int kvm_arch_get_registers(CPUState *cs)
     }
     vfp_set_fpscr(env, fpscr);
 
+    if (!write_kvmstate_to_list(cpu)) {
+        return EINVAL;
+    }
+    /* Note that it's OK to have registers which aren't in CPUState,
+     * so we can ignore a failure return here.
+     */
+    write_list_to_cpustate(cpu);
+
     return 0;
 }