[Qemu-devel] [PATCH v5 0/4] target-arm: ARM64: Adding EL1 AARCH32 guest support

[Qemu-devel] [PATCH v5 0/4] target-arm: ARM64: Adding EL1 AARCH32 guest support

[Greg Bellows]

Added support for running an AArch32 guest on a AArch64 KVM host.  Support has
only been added to the QEMU machvirt machine.  The addition of CPU properties
specifiable from the command line were added to allow disablement of AArch64
execution state hereby forcing EL1 to be AArch32.  The new CPU command line
property is "aarch64=on/off" that is specified as follows:

    aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-a57,aarch64=off ...

---
v3 -> v4
- Move and fix sync functions to properly handle current EL/PL
- Replace use of strtok
- Add disablement of aarch64 option if KVM disabled
- Revert and update AArch64 check on vcpu init to not check for AARch64 feature
  but rather AArch64 family type.
- Relocate register sync on get
- add missing env->aarch64 refresh after pstate fetch

v2 -> v3
- Fix KVM64/AArch64 hang by conditionalizing register sync
- Conditionalize 64-bit interrupt handler setting of aarch64

v1 -> v2
- Replaced custom property parsing with use of generic CPU property parser
- Added CPU property registration
- Fixed mulitple property handling in virt.c
- Removed unnecessary kernel load changes

Greg Bellows (4):
  target-arm: Add CPU property to disable AArch64
  target-arm: Add feature parsing to virt
  target-arm: Add 32/64-bit register sync
  target-arm: Add AArch32 guest support to KVM64

 hw/arm/virt.c           |  20 ++++-
 target-arm/cpu.c        |   5 +-
 target-arm/cpu.h        |   2 +
 target-arm/cpu64.c      |  39 ++++++++++
 target-arm/helper-a64.c |   5 +-
 target-arm/helper.c     | 198 ++++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/kvm64.c      |  36 ++++++++-
 target-arm/op_helper.c  |   6 +-
 8 files changed, 296 insertions(+), 15 deletions(-)

--
1.8.3.2

[Qemu-devel] [PATCH v5 1/4] target-arm: Add CPU property to disable AArch64

[Greg Bellows]

Adds registration and get/set functions for enabling/disabling the AArch64
execution state on AArch64 CPUs.  By default AArch64 execution state is enabled
on AArch64 CPUs, setting the property to off, will disable the execution state.
The below QEMU invocation would have AArch64 execution state disabled.

    $ ./qemu-system-aarch64 -machine virt -cpu cortex-a57,aarch64=off

Also adds stripping of features from CPU model string in acquiring the ARM CPU
by name.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>

---

v4 -> v5
- Fix error message.

v3 -> v4
- Switch from using strtok to g_strsplit
- Add disablement of aarch64 option if KVM is not enabled.

v1 -> v2
- Scrap the custom CPU feature parsing in favor of using the default CPU
  parsing.
- Add registration of CPU AArch64 property to disable/enable the AArch64
  feature.
---
 target-arm/cpu.c   |  5 ++++-
 target-arm/cpu64.c | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index d38af74..986f04c 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -544,13 +544,16 @@ static ObjectClass *arm_cpu_class_by_name(const char *cpu_model)
 {
     ObjectClass *oc;
     char *typename;
+    char **cpuname;
 
     if (!cpu_model) {
         return NULL;
     }
 
-    typename = g_strdup_printf("%s-" TYPE_ARM_CPU, cpu_model);
+    cpuname = g_strsplit(cpu_model, ",", 1);
+    typename = g_strdup_printf("%s-" TYPE_ARM_CPU, cpuname[0]);
     oc = object_class_by_name(typename);
+    g_strfreev(cpuname);
     g_free(typename);
     if (!oc || !object_class_dynamic_cast(oc, TYPE_ARM_CPU) ||
         object_class_is_abstract(oc)) {
diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index bb778b3..823c739 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -32,6 +32,11 @@ static inline void set_feature(CPUARMState *env, int feature)
     env->features |= 1ULL << feature;
 }
 
+static inline void unset_feature(CPUARMState *env, int feature)
+{
+    env->features &= ~(1ULL << feature);
+}
+
 #ifndef CONFIG_USER_ONLY
 static uint64_t a57_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
@@ -170,8 +175,42 @@ static const ARMCPUInfo aarch64_cpus[] = {
     { .name = NULL }
 };
 
+static bool aarch64_cpu_get_aarch64(Object *obj, Error **errp)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    return arm_feature(&cpu->env, ARM_FEATURE_AARCH64);
+}
+
+static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    /* At this time, this property is only allowed if KVM is enabled.  This
+     * restriction allows us to avoid fixing up functionality that assumes a
+     * uniform execution state like do_interrupt.
+     */
+    if (!kvm_enabled()) {
+        error_setg(errp, "'aarch64' feature cannot be disabled "
+                         "unless KVM is enabled");
+        return;
+    }
+
+    if (value == false) {
+        unset_feature(&cpu->env, ARM_FEATURE_AARCH64);
+    } else {
+        set_feature(&cpu->env, ARM_FEATURE_AARCH64);
+    }
+}
+
 static void aarch64_cpu_initfn(Object *obj)
 {
+    object_property_add_bool(obj, "aarch64", aarch64_cpu_get_aarch64,
+                             aarch64_cpu_set_aarch64, NULL);
+    object_property_set_description(obj, "aarch64",
+                                    "Set on/off to enable/disable aarch64 "
+                                    "execution state ",
+                                    NULL);
 }
 
 static void aarch64_cpu_finalizefn(Object *obj)
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v5 1/4] target-arm: Add CPU property to disable AArch64

[Peter Maydell]

On 12 February 2015 at 02:59, Greg Bellows <greg.bellows@linaro.org> wrote:
> Adds registration and get/set functions for enabling/disabling the AArch64
> execution state on AArch64 CPUs.  By default AArch64 execution state is enabled
> on AArch64 CPUs, setting the property to off, will disable the execution state.
> The below QEMU invocation would have AArch64 execution state disabled.
>
>     $ ./qemu-system-aarch64 -machine virt -cpu cortex-a57,aarch64=off
>
> Also adds stripping of features from CPU model string in acquiring the ARM CPU
> by name.
>
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>

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

thanks
-- PMM

[Qemu-devel] [PATCH v5 2/4] target-arm: Add feature parsing to virt

[Greg Bellows]

Added machvirt parsing of feature keywords added to the -cpu command line
option.  Parsing occurs during machine initialization.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

---

v3 -> v4
- Fix misspelling

v1 -> v2
- Fix multiple property handling
---
 hw/arm/virt.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 34d9379..3c37a5e 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -602,15 +602,19 @@ static void machvirt_init(MachineState *machine)
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     const char *cpu_model = machine->cpu_model;
     VirtBoardInfo *vbi;
+    char **cpustr;
 
     if (!cpu_model) {
         cpu_model = "cortex-a15";
     }
 
-    vbi = find_machine_info(cpu_model);
+    /* Separate the actual CPU model name from any appended features */
+    cpustr = g_strsplit(cpu_model, ",", 2);
+
+    vbi = find_machine_info(cpustr[0]);
 
     if (!vbi) {
-        error_report("mach-virt: CPU %s not supported", cpu_model);
+        error_report("mach-virt: CPU %s not supported", cpustr[0]);
         exit(1);
     }
 
@@ -624,8 +628,10 @@ static void machvirt_init(MachineState *machine)
     create_fdt(vbi);
 
     for (n = 0; n < smp_cpus; n++) {
-        ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
+        ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
+        CPUClass *cc = CPU_CLASS(oc);
         Object *cpuobj;
+        Error *err = NULL;
 
         if (!oc) {
             fprintf(stderr, "Unable to find CPU definition\n");
@@ -633,6 +639,13 @@ static void machvirt_init(MachineState *machine)
         }
         cpuobj = object_new(object_class_get_name(oc));
 
+        /* Handle any CPU options specified by the user */
+        cc->parse_features(CPU(cpuobj), cpustr[1], &err);
+        if (err) {
+            error_report("%s", error_get_pretty(err));
+            exit(1);
+        }
+
         if (!vms->secure) {
             object_property_set_bool(cpuobj, false, "has_el3", NULL);
         }
@@ -652,6 +665,7 @@ static void machvirt_init(MachineState *machine)
 
         object_property_set_bool(cpuobj, true, "realized", NULL);
     }
+    g_strfreev(cpustr);
     fdt_add_timer_nodes(vbi);
     fdt_add_cpu_nodes(vbi);
     fdt_add_psci_node(vbi);
-- 
1.8.3.2

[Qemu-devel] [PATCH v5 3/4] target-arm: Add 32/64-bit register sync

[Greg Bellows]

Add AArch32 to AArch64 register sychronization functions.
Replace manual register synchronization with new functions in
aarch64_cpu_do_interrupt() and HELPER(exception_return)().

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>

---

v4 -> v5
- Rework sync routines a bit more.

v3 -> v4
- Rework sync routines to cover various exception levels
- Move sync routines to helper.c

v2 -> v3
- Conditionalize interrupt handler update of aarch64.
---
 target-arm/cpu.h        |   2 +
 target-arm/helper-a64.c |   5 +-
 target-arm/helper.c     | 198 ++++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/op_helper.c  |   6 +-
 4 files changed, 203 insertions(+), 8 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 1830a12..11845a6 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -495,6 +495,8 @@ typedef struct CPUARMState {
 ARMCPU *cpu_arm_init(const char *cpu_model);
 int cpu_arm_exec(CPUARMState *s);
 uint32_t do_arm_semihosting(CPUARMState *env);
+void aarch64_sync_32_to_64(CPUARMState *env);
+void aarch64_sync_64_to_32(CPUARMState *env);
 
 static inline bool is_a64(CPUARMState *env)
 {
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 8aa40e9..7e0d038 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -466,7 +466,6 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
     unsigned int new_el = arm_excp_target_el(cs, cs->exception_index);
     target_ulong addr = env->cp15.vbar_el[new_el];
     unsigned int new_mode = aarch64_pstate_mode(new_el, true);
-    int i;
 
     if (arm_current_el(env) < new_el) {
         if (env->aarch64) {
@@ -530,9 +529,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
         }
         env->elr_el[new_el] = env->regs[15];
 
-        for (i = 0; i < 15; i++) {
-            env->xregs[i] = env->regs[i];
-        }
+        aarch64_sync_32_to_64(env);
 
         env->condexec_bits = 0;
     }
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 1a1a005..2b7c28e 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -4096,6 +4096,11 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
     return 1;
 }
 
+void aarch64_sync_64_to_32(CPUARMState *env)
+{
+    g_assert_not_reached();
+}
+
 #else
 
 /* Map CPU modes onto saved register banks.  */
@@ -4425,6 +4430,199 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
     env->thumb = addr & 1;
 }
 
+/* Function used to synchronize QEMU's AArch64 register set with AArch32
+ * register set.  This is necessary when switching between AArch32 and AArch64
+ * execution state.
+ */
+void aarch64_sync_32_to_64(CPUARMState *env)
+{
+    int i;
+    uint32_t mode = env->uncached_cpsr & CPSR_M;
+
+    /* We can blanket copy R[0:7] to X[0:7] */
+    for (i = 0; i < 8; i++) {
+        env->xregs[i] = env->regs[i];
+    }
+
+    /* Unless we are in FIQ mode, x8-x12 come from the active user registers
+     * r8-r12.  Otherwise, they come from the banked user regs.
+     */
+    if (mode == ARM_CPU_MODE_FIQ) {
+        for (i = 8; i < 13; i++) {
+            env->xregs[i] = env->usr_regs[i - 8];
+        }
+    } else {
+        for (i = 8; i < 13; i++) {
+            env->xregs[i] = env->regs[i];
+        }
+    }
+
+    /* Registers x13-x23 are the various mode SP and FP registers.  The active
+     * r13 and r14 are only copied if we are in that mode, otherwise we copy
+     * from the mode banked copy.
+     */
+    if (mode == ARM_CPU_MODE_USR || mode == ARM_CPU_MODE_SYS) {
+        env->xregs[13] = env->regs[13];
+        env->xregs[14] = env->regs[14];
+    } else {
+        env->xregs[13] = env->banked_r13[bank_number(ARM_CPU_MODE_USR)];
+        env->xregs[14] = env->banked_r14[bank_number(ARM_CPU_MODE_USR)];
+    }
+
+    if (mode == ARM_CPU_MODE_HYP) {
+        env->xregs[15] = env->regs[13];
+    } else {
+        env->xregs[15] = env->banked_r13[bank_number(ARM_CPU_MODE_HYP)];
+    }
+
+    if (mode == ARM_CPU_MODE_IRQ) {
+        env->xregs[16] = env->regs[13];
+        env->xregs[17] = env->regs[14];
+    } else {
+        env->xregs[16] = env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)];
+        env->xregs[17] = env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)];
+    }
+
+    if (mode == ARM_CPU_MODE_SVC) {
+        env->xregs[18] = env->regs[13];
+        env->xregs[19] = env->regs[14];
+    } else {
+        env->xregs[18] = env->banked_r13[bank_number(ARM_CPU_MODE_SVC)];
+        env->xregs[19] = env->banked_r14[bank_number(ARM_CPU_MODE_SVC)];
+    }
+
+    if (mode == ARM_CPU_MODE_ABT) {
+        env->xregs[20] = env->regs[13];
+        env->xregs[21] = env->regs[14];
+    } else {
+        env->xregs[20] = env->banked_r13[bank_number(ARM_CPU_MODE_ABT)];
+        env->xregs[21] = env->banked_r14[bank_number(ARM_CPU_MODE_ABT)];
+    }
+
+    if (mode == ARM_CPU_MODE_UND) {
+        env->xregs[22] = env->regs[13];
+        env->xregs[23] = env->regs[14];
+    } else {
+        env->xregs[22] = env->banked_r13[bank_number(ARM_CPU_MODE_UND)];
+        env->xregs[23] = env->banked_r14[bank_number(ARM_CPU_MODE_UND)];
+    }
+
+    /* Registers x24-x30 are mapped to r8-r14 in FIQ mode.  If we are in FIQ
+     * mode, then we can copy the active r8-r14.  Otherwise, we copy from the
+     * FIQ banks for r8-r14.
+     */
+    if (mode == ARM_CPU_MODE_FIQ) {
+        for (i = 24; i < 31; i++) {
+            env->xregs[i] = env->regs[i - 16];   /* X[24:30] <- R[8:14] */
+        }
+    } else {
+        for (i = 24; i < 29; i++) {
+            env->xregs[i] = env->fiq_regs[i - 24];
+        }
+        env->xregs[29] = env->banked_r13[bank_number(ARM_CPU_MODE_FIQ)];
+        env->xregs[30] = env->banked_r14[bank_number(ARM_CPU_MODE_FIQ)];
+    }
+
+    env->pc = env->regs[15];
+}
+
+/* Function used to synchronize QEMU's AArch32 register set with AArch64
+ * register set.  This is necessary when switching between AArch32 and AArch64
+ * execution state.
+ */
+void aarch64_sync_64_to_32(CPUARMState *env)
+{
+    int i;
+    uint32_t mode = env->uncached_cpsr & CPSR_M;
+
+    /* We can blanket copy X[0:7] to R[0:7] */
+    for (i = 0; i < 8; i++) {
+        env->regs[i] = env->xregs[i];
+    }
+
+    /* Unless we are in FIQ mode, r8-r12 come from the active user registers
+     * x8-x12.  Otherwise, they come from the FIQ mapped x24-x30..
+     */
+    if (mode == ARM_CPU_MODE_FIQ) {
+        for (i = 8; i < 13; i++) {
+            env->regs[i] = env->xregs[i + 16];   /* X[24:30] -> R[8:12] */
+        }
+    } else {
+        for (i = 8; i < 13; i++) {
+            env->regs[i] = env->xregs[i];
+        }
+    }
+
+    /* Registers r13 & r14 are dependent on the mode we are in.  For the
+     * current mode we actually copy from the respective x13 & x14 registers.
+     * Otherwise, we copy x13 and x14 into banked storage corresponding to the
+     * mode.
+     */
+    if (mode == ARM_CPU_MODE_USR || mode == ARM_CPU_MODE_SYS) {
+        env->regs[13] = env->xregs[13];
+        env->regs[14] = env->xregs[14];
+    } else {
+        for (i = 0; i < 5; i++) {
+            env->usr_regs[i] = env->xregs[i + 8];
+        }
+        env->banked_r13[bank_number(ARM_CPU_MODE_USR)] = env->xregs[13];
+        env->banked_r14[bank_number(ARM_CPU_MODE_USR)] = env->xregs[14];
+    }
+
+    if (mode == ARM_CPU_MODE_HYP) {
+        env->regs[13] = env->xregs[15];
+        env->regs[14] = env->xregs[14];
+    } else {
+        /* Only x15 maps to something in HYP mode, specifically r13 */
+        env->banked_r13[bank_number(ARM_CPU_MODE_HYP)] = env->xregs[15];
+    }
+
+    if (mode == ARM_CPU_MODE_IRQ) {
+        env->regs[13] = env->xregs[16];
+        env->regs[14] = env->xregs[17];
+    } else {
+        env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)] = env->xregs[16];
+        env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)] = env->xregs[17];
+    }
+
+    if (mode == ARM_CPU_MODE_SVC) {
+        env->regs[13] = env->xregs[18];
+        env->regs[14] = env->xregs[19];
+    } else {
+        env->banked_r13[bank_number(ARM_CPU_MODE_SVC)] = env->xregs[18];
+        env->banked_r14[bank_number(ARM_CPU_MODE_SVC)] = env->xregs[19];
+    }
+
+    if (mode == ARM_CPU_MODE_ABT) {
+        env->regs[13] = env->xregs[20];
+        env->regs[14] = env->xregs[21];
+    } else {
+        env->banked_r13[bank_number(ARM_CPU_MODE_ABT)] = env->xregs[20];
+        env->banked_r14[bank_number(ARM_CPU_MODE_ABT)] = env->xregs[21];
+    }
+
+    if (mode == ARM_CPU_MODE_UND) {
+        env->regs[13] = env->xregs[22];
+        env->regs[14] = env->xregs[23];
+    } else {
+        env->banked_r13[bank_number(ARM_CPU_MODE_UND)] = env->xregs[22];
+        env->banked_r14[bank_number(ARM_CPU_MODE_UND)] = env->xregs[23];
+    }
+
+    if (mode == ARM_CPU_MODE_FIQ) {
+        env->regs[13] = env->xregs[29];
+        env->regs[14] = env->xregs[30];
+    } else {
+        for (i = 0; i < 5; i++) {
+            env->fiq_regs[i] = env->xregs[i + 24];
+        }
+        env->banked_r13[bank_number(ARM_CPU_MODE_FIQ)] = env->xregs[29];
+        env->banked_r14[bank_number(ARM_CPU_MODE_FIQ)] = env->xregs[30];
+    }
+
+    env->regs[15] = env->pc;
+}
+
 /* Handle a CPU exception.  */
 void arm_cpu_do_interrupt(CPUState *cs)
 {
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 2bed914..7713022 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -465,7 +465,7 @@ void HELPER(exception_return)(CPUARMState *env)
     int cur_el = arm_current_el(env);
     unsigned int spsr_idx = aarch64_banked_spsr_index(cur_el);
     uint32_t spsr = env->banked_spsr[spsr_idx];
-    int new_el, i;
+    int new_el;
 
     aarch64_save_sp(env, cur_el);
 
@@ -491,9 +491,7 @@ void HELPER(exception_return)(CPUARMState *env)
         if (!arm_singlestep_active(env)) {
             env->uncached_cpsr &= ~PSTATE_SS;
         }
-        for (i = 0; i < 15; i++) {
-            env->regs[i] = env->xregs[i];
-        }
+        aarch64_sync_64_to_32(env);
 
         env->regs[15] = env->elr_el[1] & ~0x1;
     } else {
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v5 3/4] target-arm: Add 32/64-bit register sync

[Peter Maydell]

On 12 February 2015 at 02:59, Greg Bellows <greg.bellows@linaro.org> wrote:
> Add AArch32 to AArch64 register sychronization functions.
> Replace manual register synchronization with new functions in
> aarch64_cpu_do_interrupt() and HELPER(exception_return)().
>
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
>
> ---
>
> v4 -> v5
> - Rework sync routines a bit more.
>
> v3 -> v4
> - Rework sync routines to cover various exception levels
> - Move sync routines to helper.c
>
> v2 -> v3
> - Conditionalize interrupt handler update of aarch64.
> ---
>  target-arm/cpu.h        |   2 +
>  target-arm/helper-a64.c |   5 +-
>  target-arm/helper.c     | 198 ++++++++++++++++++++++++++++++++++++++++++++++++
>  target-arm/op_helper.c  |   6 +-
>  4 files changed, 203 insertions(+), 8 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 1830a12..11845a6 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -495,6 +495,8 @@ typedef struct CPUARMState {
>  ARMCPU *cpu_arm_init(const char *cpu_model);
>  int cpu_arm_exec(CPUARMState *s);
>  uint32_t do_arm_semihosting(CPUARMState *env);
> +void aarch64_sync_32_to_64(CPUARMState *env);
> +void aarch64_sync_64_to_32(CPUARMState *env);
>
>  static inline bool is_a64(CPUARMState *env)
>  {
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 8aa40e9..7e0d038 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -466,7 +466,6 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>      unsigned int new_el = arm_excp_target_el(cs, cs->exception_index);
>      target_ulong addr = env->cp15.vbar_el[new_el];
>      unsigned int new_mode = aarch64_pstate_mode(new_el, true);
> -    int i;
>
>      if (arm_current_el(env) < new_el) {
>          if (env->aarch64) {
> @@ -530,9 +529,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>          }
>          env->elr_el[new_el] = env->regs[15];
>
> -        for (i = 0; i < 15; i++) {
> -            env->xregs[i] = env->regs[i];
> -        }
> +        aarch64_sync_32_to_64(env);
>
>          env->condexec_bits = 0;
>      }
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 1a1a005..2b7c28e 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -4096,6 +4096,11 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
>      return 1;
>  }
>
> +void aarch64_sync_64_to_32(CPUARMState *env)
> +{
> +    g_assert_not_reached();
> +}
> +
>  #else
>
>  /* Map CPU modes onto saved register banks.  */
> @@ -4425,6 +4430,199 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
>      env->thumb = addr & 1;
>  }
>
> +/* Function used to synchronize QEMU's AArch64 register set with AArch32
> + * register set.  This is necessary when switching between AArch32 and AArch64
> + * execution state.
> + */
> +void aarch64_sync_32_to_64(CPUARMState *env)
> +{
> +    int i;
> +    uint32_t mode = env->uncached_cpsr & CPSR_M;
> +
> +    /* We can blanket copy R[0:7] to X[0:7] */
> +    for (i = 0; i < 8; i++) {
> +        env->xregs[i] = env->regs[i];
> +    }
> +
> +    /* Unless we are in FIQ mode, x8-x12 come from the active user registers
> +     * r8-r12.  Otherwise, they come from the banked user regs.
> +     */
> +    if (mode == ARM_CPU_MODE_FIQ) {
> +        for (i = 8; i < 13; i++) {
> +            env->xregs[i] = env->usr_regs[i - 8];
> +        }
> +    } else {
> +        for (i = 8; i < 13; i++) {
> +            env->xregs[i] = env->regs[i];
> +        }
> +    }
> +
> +    /* Registers x13-x23 are the various mode SP and FP registers.  The active
> +     * r13 and r14 are only copied if we are in that mode, otherwise we copy
> +     * from the mode banked copy.
> +     */
> +    if (mode == ARM_CPU_MODE_USR || mode == ARM_CPU_MODE_SYS) {
> +        env->xregs[13] = env->regs[13];
> +        env->xregs[14] = env->regs[14];
> +    } else {
> +        env->xregs[13] = env->banked_r13[bank_number(ARM_CPU_MODE_USR)];
> +        env->xregs[14] = env->banked_r14[bank_number(ARM_CPU_MODE_USR)];
> +    }

Hyp mode uses the USR mode r14, so if we're in Hyp you need to copy
regs[14] into xregs[14]. (But not r13, which has a separate banked
Hyp version, so xregs[13] and [14] need their own if () { .. } else { .. },
and can't share the same one.)

Otherwise this function is correct now.

> +
> +    if (mode == ARM_CPU_MODE_HYP) {
> +        env->xregs[15] = env->regs[13];
> +    } else {
> +        env->xregs[15] = env->banked_r13[bank_number(ARM_CPU_MODE_HYP)];
> +    }
> +
> +    if (mode == ARM_CPU_MODE_IRQ) {
> +        env->xregs[16] = env->regs[13];
> +        env->xregs[17] = env->regs[14];
> +    } else {
> +        env->xregs[16] = env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)];
> +        env->xregs[17] = env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)];
> +    }
> +
> +    if (mode == ARM_CPU_MODE_SVC) {
> +        env->xregs[18] = env->regs[13];
> +        env->xregs[19] = env->regs[14];
> +    } else {
> +        env->xregs[18] = env->banked_r13[bank_number(ARM_CPU_MODE_SVC)];
> +        env->xregs[19] = env->banked_r14[bank_number(ARM_CPU_MODE_SVC)];
> +    }
> +
> +    if (mode == ARM_CPU_MODE_ABT) {
> +        env->xregs[20] = env->regs[13];
> +        env->xregs[21] = env->regs[14];
> +    } else {
> +        env->xregs[20] = env->banked_r13[bank_number(ARM_CPU_MODE_ABT)];
> +        env->xregs[21] = env->banked_r14[bank_number(ARM_CPU_MODE_ABT)];
> +    }
> +
> +    if (mode == ARM_CPU_MODE_UND) {
> +        env->xregs[22] = env->regs[13];
> +        env->xregs[23] = env->regs[14];
> +    } else {
> +        env->xregs[22] = env->banked_r13[bank_number(ARM_CPU_MODE_UND)];
> +        env->xregs[23] = env->banked_r14[bank_number(ARM_CPU_MODE_UND)];
> +    }
> +
> +    /* Registers x24-x30 are mapped to r8-r14 in FIQ mode.  If we are in FIQ
> +     * mode, then we can copy the active r8-r14.  Otherwise, we copy from the
> +     * FIQ banks for r8-r14.
> +     */
> +    if (mode == ARM_CPU_MODE_FIQ) {
> +        for (i = 24; i < 31; i++) {
> +            env->xregs[i] = env->regs[i - 16];   /* X[24:30] <- R[8:14] */
> +        }
> +    } else {
> +        for (i = 24; i < 29; i++) {
> +            env->xregs[i] = env->fiq_regs[i - 24];
> +        }
> +        env->xregs[29] = env->banked_r13[bank_number(ARM_CPU_MODE_FIQ)];
> +        env->xregs[30] = env->banked_r14[bank_number(ARM_CPU_MODE_FIQ)];
> +    }
> +
> +    env->pc = env->regs[15];
> +}
> +
> +/* Function used to synchronize QEMU's AArch32 register set with AArch64
> + * register set.  This is necessary when switching between AArch32 and AArch64
> + * execution state.
> + */
> +void aarch64_sync_64_to_32(CPUARMState *env)
> +{
> +    int i;
> +    uint32_t mode = env->uncached_cpsr & CPSR_M;
> +
> +    /* We can blanket copy X[0:7] to R[0:7] */
> +    for (i = 0; i < 8; i++) {
> +        env->regs[i] = env->xregs[i];
> +    }
> +
> +    /* Unless we are in FIQ mode, r8-r12 come from the active user registers

"active" here should be deleted...

> +     * x8-x12.  Otherwise, they come from the FIQ mapped x24-x30..
> +     */
> +    if (mode == ARM_CPU_MODE_FIQ) {
> +        for (i = 8; i < 13; i++) {
> +            env->regs[i] = env->xregs[i + 16];   /* X[24:30] -> R[8:12] */
> +        }
> +    } else {
> +        for (i = 8; i < 13; i++) {
> +            env->regs[i] = env->xregs[i];
> +        }
> +    }
> +
> +    /* Registers r13 & r14 are dependent on the mode we are in.  For the
> +     * current mode we actually copy from the respective x13 & x14 registers.

This comment doesn't seem to make sense.

> +     * Otherwise, we copy x13 and x14 into banked storage corresponding to the
> +     * mode.
> +     */
> +    if (mode == ARM_CPU_MODE_USR || mode == ARM_CPU_MODE_SYS) {
> +        env->regs[13] = env->xregs[13];
> +        env->regs[14] = env->xregs[14];
> +    } else {
> +        for (i = 0; i < 5; i++) {
> +            env->usr_regs[i] = env->xregs[i + 8];
> +        }
> +        env->banked_r13[bank_number(ARM_CPU_MODE_USR)] = env->xregs[13];
> +        env->banked_r14[bank_number(ARM_CPU_MODE_USR)] = env->xregs[14];
> +    }

I think a clearer way to structure this function is going to be
to have it basically proceed through the xregs[] in order.
Every xreg[] from 0 to 30 should get copied to exactly one
place (in regs, or a banked reg, or whatever). Then it's
clear that we've handled every register we need to sync and
haven't left out any cases.


> +
> +    if (mode == ARM_CPU_MODE_HYP) {
> +        env->regs[13] = env->xregs[15];
> +        env->regs[14] = env->xregs[14];
> +    } else {
> +        /* Only x15 maps to something in HYP mode, specifically r13 */
> +        env->banked_r13[bank_number(ARM_CPU_MODE_HYP)] = env->xregs[15];
> +    }
> +
> +    if (mode == ARM_CPU_MODE_IRQ) {
> +        env->regs[13] = env->xregs[16];
> +        env->regs[14] = env->xregs[17];
> +    } else {
> +        env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)] = env->xregs[16];
> +        env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)] = env->xregs[17];
> +    }
> +
> +    if (mode == ARM_CPU_MODE_SVC) {
> +        env->regs[13] = env->xregs[18];
> +        env->regs[14] = env->xregs[19];
> +    } else {
> +        env->banked_r13[bank_number(ARM_CPU_MODE_SVC)] = env->xregs[18];
> +        env->banked_r14[bank_number(ARM_CPU_MODE_SVC)] = env->xregs[19];
> +    }
> +
> +    if (mode == ARM_CPU_MODE_ABT) {
> +        env->regs[13] = env->xregs[20];
> +        env->regs[14] = env->xregs[21];
> +    } else {
> +        env->banked_r13[bank_number(ARM_CPU_MODE_ABT)] = env->xregs[20];
> +        env->banked_r14[bank_number(ARM_CPU_MODE_ABT)] = env->xregs[21];
> +    }
> +
> +    if (mode == ARM_CPU_MODE_UND) {
> +        env->regs[13] = env->xregs[22];
> +        env->regs[14] = env->xregs[23];
> +    } else {
> +        env->banked_r13[bank_number(ARM_CPU_MODE_UND)] = env->xregs[22];
> +        env->banked_r14[bank_number(ARM_CPU_MODE_UND)] = env->xregs[23];
> +    }
> +
> +    if (mode == ARM_CPU_MODE_FIQ) {
> +        env->regs[13] = env->xregs[29];
> +        env->regs[14] = env->xregs[30];
> +    } else {
> +        for (i = 0; i < 5; i++) {
> +            env->fiq_regs[i] = env->xregs[i + 24];
> +        }
> +        env->banked_r13[bank_number(ARM_CPU_MODE_FIQ)] = env->xregs[29];
> +        env->banked_r14[bank_number(ARM_CPU_MODE_FIQ)] = env->xregs[30];
> +    }
> +
> +    env->regs[15] = env->pc;
> +}
> +
>  /* Handle a CPU exception.  */
>  void arm_cpu_do_interrupt(CPUState *cs)
>  {
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 2bed914..7713022 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -465,7 +465,7 @@ void HELPER(exception_return)(CPUARMState *env)
>      int cur_el = arm_current_el(env);
>      unsigned int spsr_idx = aarch64_banked_spsr_index(cur_el);
>      uint32_t spsr = env->banked_spsr[spsr_idx];
> -    int new_el, i;
> +    int new_el;
>
>      aarch64_save_sp(env, cur_el);
>
> @@ -491,9 +491,7 @@ void HELPER(exception_return)(CPUARMState *env)
>          if (!arm_singlestep_active(env)) {
>              env->uncached_cpsr &= ~PSTATE_SS;
>          }
> -        for (i = 0; i < 15; i++) {
> -            env->regs[i] = env->xregs[i];
> -        }
> +        aarch64_sync_64_to_32(env);
>
>          env->regs[15] = env->elr_el[1] & ~0x1;
>      } else {
> --
> 1.8.3.2
>

thanks
-- PMM

[Qemu-devel] [PATCH v5 4/4] target-arm: Add AArch32 guest support to KVM64

[Greg Bellows]

Add 32-bit to/from 64-bit register synchronization on register gets and puts.
Set EL1_32BIT feature flag passed to KVM

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>

---

v4 -> v5
- Fix target check

v3 -> v4
- Add check that to make sure KVM64 is only being used on AArch64 family of
  machines.
- Relocate register sync to follow register fetches.
- Refresh env->aarch64 prior to use.

v2 -> v3
- Conditionalize sync of 32-bit and 64-bit registers
---
 target-arm/kvm64.c | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index 033babf..48fdb87 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -82,7 +82,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
     ARMCPU *cpu = ARM_CPU(cs);
 
     if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE ||
-        !arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
+        !object_dynamic_cast(cpu, TYPE_AARCH64_CPU)) {
         fprintf(stderr, "KVM is not supported for this guest CPU type\n");
         return -EINVAL;
     }
@@ -96,6 +96,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
         cpu->psci_version = 2;
         cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PSCI_0_2;
     }
+    if (!arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
+        cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_EL1_32BIT;
+    }
 
     /* Do KVM_ARM_VCPU_INIT ioctl */
     ret = kvm_arm_vcpu_init(cs);
@@ -133,6 +136,13 @@ int kvm_arch_put_registers(CPUState *cs, int level)
     ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;
 
+    /* If we are in AArch32 mode then we need to copy the AArch32 regs to the
+     * AArch64 registers before pushing them out to 64-bit KVM.
+     */
+    if (!is_a64(env)) {
+        aarch64_sync_32_to_64(env);
+    }
+
     for (i = 0; i < 31; i++) {
         reg.id = AARCH64_CORE_REG(regs.regs[i]);
         reg.addr = (uintptr_t) &env->xregs[i];
@@ -162,7 +172,11 @@ int kvm_arch_put_registers(CPUState *cs, int level)
     }
 
     /* Note that KVM thinks pstate is 64 bit but we use a uint32_t */
-    val = pstate_read(env);
+    if (is_a64(env)) {
+        val = pstate_read(env);
+    } else {
+        val = cpsr_read(env);
+    }
     reg.id = AARCH64_CORE_REG(regs.pstate);
     reg.addr = (uintptr_t) &val;
     ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
@@ -242,7 +256,14 @@ int kvm_arch_get_registers(CPUState *cs)
     if (ret) {
         return ret;
     }
-    pstate_write(env, val);
+
+    env->aarch64 = ((val & PSTATE_nRW) == 0);
+    if (is_a64(env)) {
+        pstate_write(env, val);
+    } else {
+        env->uncached_cpsr = val & CPSR_M;
+        cpsr_write(env, val, 0xffffffff);
+    }
 
     /* KVM puts SP_EL0 in regs.sp and SP_EL1 in regs.sp_el1. On the
      * QEMU side we keep the current SP in xregs[31] as well.
@@ -256,6 +277,15 @@ int kvm_arch_get_registers(CPUState *cs)
         return ret;
     }
 
+    /* If we are in AArch32 mode then we need to sync the AArch32 regs with the
+     * incoming AArch64 regs received from 64-bit KVM.
+     * We must perform this after all of the registers have been acquired from
+     * the kernel.
+     */
+    if (!is_a64(env)) {
+        aarch64_sync_64_to_32(env);
+    }
+
     reg.id = AARCH64_CORE_REG(elr_el1);
     reg.addr = (uintptr_t) &env->elr_el[1];
     ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v5 4/4] target-arm: Add AArch32 guest support to KVM64

[Peter Maydell]

On 12 February 2015 at 02:59, Greg Bellows <greg.bellows@linaro.org> wrote:
> Add 32-bit to/from 64-bit register synchronization on register gets and puts.
> Set EL1_32BIT feature flag passed to KVM
>
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>

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

thanks
-- PMM