[Qemu-devel] [PATCH 0/8] target-arm: support mixed 32/64 bit execution beyond EL0

[Qemu-devel] [PATCH 0/8] target-arm: support mixed 32/64 bit execution beyond EL0

[Peter Maydell]

This patch series fixes the code for ARM exception entry and exit
so that we can support mixed 32/64-bit interprocessing for cases
beyond the current "EL1 is 64-bit, EL0 might be 32-bit or 64-bit".
This involves:
 * making arm_el_is_aa64() handle EL2 and EL3 and their associated
   register-width bits for controlling lower exception levels
 * making the do_interrupt entrypoint determine whether to do a
   32- or 64-bit exception entry dynamically rather than as a
   static property of the CPU class
 * handling exception return from AArch64 to AArch32 for all cases, not
   just where we're returning to EL0
 * fixing the code that picks the AArch64 vector entry point: this
   depends on the register-width of the EL below the target EL, not
   on the width of the EL the exception is taken from

The last two patches fix minor bugs noticed along the way.

PS: I've tested this for various images I have, but I don't actually
happen to have a setup for a 32-bit EL1 under 64-bit EL3 just yet :-)

These patches are written on top of the multi-ases work, though
there shouldn't be any dependencies I think beyond the possible
merely textual.

thanks
-- PMM

Peter Maydell (8):
  target-arm: Properly support EL2 and EL3 in arm_el_is_aa64()
  target-arm: Move aarch64_cpu_do_interrupt() to helper.c
  target-arm: Use a single entry point for AArch64 and AArch32 exceptions
  target-arm: Pull semihosting handling out to arm_cpu_do_interrupt()
  target-arm: Fix wrong AArch64 entry offset for EL2/EL3 target
  target-arm: Handle exception return from AArch64 to non-EL0 AArch32
  target-arm: Implement remaining illegal return event checks
  target-arm: ignore ELR_ELx[1] for exception return to 32-bit ARM mode

 target-arm/cpu-qom.h    |   2 -
 target-arm/cpu.h        |  33 ++++--
 target-arm/cpu64.c      |   3 -
 target-arm/helper-a64.c | 104 -------------------
 target-arm/helper.c     | 268 +++++++++++++++++++++++++++++++++++++++---------
 target-arm/op_helper.c  |  95 +++++++++++++----
 6 files changed, 319 insertions(+), 186 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH 1/8] target-arm: Properly support EL2 and EL3 in arm_el_is_aa64()

[Peter Maydell]

Support EL2 and EL3 in arm_el_is_aa64() by implementing the
logic for checking the SCR_EL3 and HCR_EL2 register-width bits
as appropriate to determine the register width of lower exception
levels.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 5f81342..b8b3364 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -969,18 +969,33 @@ static inline bool arm_is_secure(CPUARMState *env)
 /* Return true if the specified exception level is running in AArch64 state. */
 static inline bool arm_el_is_aa64(CPUARMState *env, int el)
 {
-    /* We don't currently support EL2, and this isn't valid for EL0
-     * (if we're in EL0, is_a64() is what you want, and if we're not in EL0
-     * then the state of EL0 isn't well defined.)
+    /* This isn't valid for EL0 (if we're in EL0, is_a64() is what you want,
+     * and if we're not in EL0 then the state of EL0 isn't well defined.)
      */
-    assert(el == 1 || el == 3);
+    assert(el >= 1 && el <= 3);
+    bool aa64 = arm_feature(env, ARM_FEATURE_AARCH64);
 
-    /* AArch64-capable CPUs always run with EL1 in AArch64 mode. This
-     * is a QEMU-imposed simplification which we may wish to change later.
-     * If we in future support EL2 and/or EL3, then the state of lower
-     * exception levels is controlled by the HCR.RW and SCR.RW bits.
+    /* The highest exception level is always at the maximum supported
+     * register width, and then lower levels have a register width controlled
+     * by bits in the SCR or HCR registers.
      */
-    return arm_feature(env, ARM_FEATURE_AARCH64);
+    if (el == 3) {
+        return aa64;
+    }
+
+    if (arm_feature(env, ARM_FEATURE_EL3)) {
+        aa64 = aa64 && (env->cp15.scr_el3 & SCR_RW);
+    }
+
+    if (el == 2) {
+        return aa64;
+    }
+
+    if (arm_feature(env, ARM_FEATURE_EL2) && !arm_is_secure_below_el3(env)) {
+        aa64 = aa64 && (env->cp15.hcr_el2 & HCR_RW);
+    }
+
+    return aa64;
 }
 
 /* Function for determing whether guest cp register reads and writes should
-- 
1.9.1

[Qemu-devel] [PATCH 2/8] target-arm: Move aarch64_cpu_do_interrupt() to helper.c

[Peter Maydell]

Move the aarch64_cpu_do_interrupt() function to helper.c. We want
to be able to call this from code that isn't AArch64-only, and
the move allows us to avoid awkward #ifdeffery at the callsite.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu-qom.h    |   2 +-
 target-arm/helper-a64.c | 104 ------------------------------------------------
 target-arm/helper.c     | 100 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+), 105 deletions(-)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index e4d4270..bda2af8 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -247,8 +247,8 @@ void arm_gt_stimer_cb(void *opaque);
 #ifdef TARGET_AARCH64
 int aarch64_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
 int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
+#endif
 
 void aarch64_cpu_do_interrupt(CPUState *cs);
-#endif
 
 #endif
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index fc3ccdf..a322e7b 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -25,7 +25,6 @@
 #include "qemu/bitops.h"
 #include "internals.h"
 #include "qemu/crc32c.h"
-#include "sysemu/kvm.h"
 #include <zlib.h> /* For crc32 */
 
 /* C2.4.7 Multiply and divide */
@@ -443,106 +442,3 @@ uint64_t HELPER(crc32c_64)(uint64_t acc, uint64_t val, uint32_t bytes)
     /* Linux crc32c converts the output to one's complement.  */
     return crc32c(acc, buf, bytes) ^ 0xffffffff;
 }
-
-#if !defined(CONFIG_USER_ONLY)
-
-/* Handle a CPU exception.  */
-void aarch64_cpu_do_interrupt(CPUState *cs)
-{
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-    unsigned int new_el = env->exception.target_el;
-    target_ulong addr = env->cp15.vbar_el[new_el];
-    unsigned int new_mode = aarch64_pstate_mode(new_el, true);
-
-    if (arm_current_el(env) < new_el) {
-        if (env->aarch64) {
-            addr += 0x400;
-        } else {
-            addr += 0x600;
-        }
-    } else if (pstate_read(env) & PSTATE_SP) {
-        addr += 0x200;
-    }
-
-    arm_log_exception(cs->exception_index);
-    qemu_log_mask(CPU_LOG_INT, "...from EL%d to EL%d\n", arm_current_el(env),
-                  new_el);
-    if (qemu_loglevel_mask(CPU_LOG_INT)
-        && !excp_is_internal(cs->exception_index)) {
-        qemu_log_mask(CPU_LOG_INT, "...with ESR %x/0x%" PRIx32 "\n",
-                      env->exception.syndrome >> ARM_EL_EC_SHIFT,
-                      env->exception.syndrome);
-    }
-
-    if (arm_is_psci_call(cpu, cs->exception_index)) {
-        arm_handle_psci_call(cpu);
-        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
-        return;
-    }
-
-    switch (cs->exception_index) {
-    case EXCP_PREFETCH_ABORT:
-    case EXCP_DATA_ABORT:
-        env->cp15.far_el[new_el] = env->exception.vaddress;
-        qemu_log_mask(CPU_LOG_INT, "...with FAR 0x%" PRIx64 "\n",
-                      env->cp15.far_el[new_el]);
-        /* fall through */
-    case EXCP_BKPT:
-    case EXCP_UDEF:
-    case EXCP_SWI:
-    case EXCP_HVC:
-    case EXCP_HYP_TRAP:
-    case EXCP_SMC:
-        env->cp15.esr_el[new_el] = env->exception.syndrome;
-        break;
-    case EXCP_IRQ:
-    case EXCP_VIRQ:
-        addr += 0x80;
-        break;
-    case EXCP_FIQ:
-    case EXCP_VFIQ:
-        addr += 0x100;
-        break;
-    case EXCP_SEMIHOST:
-        qemu_log_mask(CPU_LOG_INT,
-                      "...handling as semihosting call 0x%" PRIx64 "\n",
-                      env->xregs[0]);
-        env->xregs[0] = do_arm_semihosting(env);
-        return;
-    default:
-        cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
-    }
-
-    if (is_a64(env)) {
-        env->banked_spsr[aarch64_banked_spsr_index(new_el)] = pstate_read(env);
-        aarch64_save_sp(env, arm_current_el(env));
-        env->elr_el[new_el] = env->pc;
-    } else {
-        env->banked_spsr[aarch64_banked_spsr_index(new_el)] = cpsr_read(env);
-        if (!env->thumb) {
-            env->cp15.esr_el[new_el] |= 1 << 25;
-        }
-        env->elr_el[new_el] = env->regs[15];
-
-        aarch64_sync_32_to_64(env);
-
-        env->condexec_bits = 0;
-    }
-    qemu_log_mask(CPU_LOG_INT, "...with ELR 0x%" PRIx64 "\n",
-                  env->elr_el[new_el]);
-
-    pstate_write(env, PSTATE_DAIF | new_mode);
-    env->aarch64 = 1;
-    aarch64_restore_sp(env, new_el);
-
-    env->pc = addr;
-
-    qemu_log_mask(CPU_LOG_INT, "...to EL%d PC 0x%" PRIx64 " PSTATE 0x%x\n",
-                  new_el, env->pc, pstate_read(env));
-
-    if (!kvm_enabled()) {
-        cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
-    }
-}
-#endif
diff --git a/target-arm/helper.c b/target-arm/helper.c
index a06bfaf..519f066 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -11,6 +11,7 @@
 #include "arm_ldst.h"
 #include <zlib.h> /* For crc32 */
 #include "exec/semihost.h"
+#include "sysemu/kvm.h"
 
 #define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */
 
@@ -5901,6 +5902,105 @@ void arm_cpu_do_interrupt(CPUState *cs)
     cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
 }
 
+/* Handle a CPU exception.  */
+void aarch64_cpu_do_interrupt(CPUState *cs)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+    unsigned int new_el = env->exception.target_el;
+    target_ulong addr = env->cp15.vbar_el[new_el];
+    unsigned int new_mode = aarch64_pstate_mode(new_el, true);
+
+    if (arm_current_el(env) < new_el) {
+        if (env->aarch64) {
+            addr += 0x400;
+        } else {
+            addr += 0x600;
+        }
+    } else if (pstate_read(env) & PSTATE_SP) {
+        addr += 0x200;
+    }
+
+    arm_log_exception(cs->exception_index);
+    qemu_log_mask(CPU_LOG_INT, "...from EL%d to EL%d\n", arm_current_el(env),
+                  new_el);
+    if (qemu_loglevel_mask(CPU_LOG_INT)
+        && !excp_is_internal(cs->exception_index)) {
+        qemu_log_mask(CPU_LOG_INT, "...with ESR %x/0x%" PRIx32 "\n",
+                      env->exception.syndrome >> ARM_EL_EC_SHIFT,
+                      env->exception.syndrome);
+    }
+
+    if (arm_is_psci_call(cpu, cs->exception_index)) {
+        arm_handle_psci_call(cpu);
+        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
+        return;
+    }
+
+    switch (cs->exception_index) {
+    case EXCP_PREFETCH_ABORT:
+    case EXCP_DATA_ABORT:
+        env->cp15.far_el[new_el] = env->exception.vaddress;
+        qemu_log_mask(CPU_LOG_INT, "...with FAR 0x%" PRIx64 "\n",
+                      env->cp15.far_el[new_el]);
+        /* fall through */
+    case EXCP_BKPT:
+    case EXCP_UDEF:
+    case EXCP_SWI:
+    case EXCP_HVC:
+    case EXCP_HYP_TRAP:
+    case EXCP_SMC:
+        env->cp15.esr_el[new_el] = env->exception.syndrome;
+        break;
+    case EXCP_IRQ:
+    case EXCP_VIRQ:
+        addr += 0x80;
+        break;
+    case EXCP_FIQ:
+    case EXCP_VFIQ:
+        addr += 0x100;
+        break;
+    case EXCP_SEMIHOST:
+        qemu_log_mask(CPU_LOG_INT,
+                      "...handling as semihosting call 0x%" PRIx64 "\n",
+                      env->xregs[0]);
+        env->xregs[0] = do_arm_semihosting(env);
+        return;
+    default:
+        cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
+    }
+
+    if (is_a64(env)) {
+        env->banked_spsr[aarch64_banked_spsr_index(new_el)] = pstate_read(env);
+        aarch64_save_sp(env, arm_current_el(env));
+        env->elr_el[new_el] = env->pc;
+    } else {
+        env->banked_spsr[aarch64_banked_spsr_index(new_el)] = cpsr_read(env);
+        if (!env->thumb) {
+            env->cp15.esr_el[new_el] |= 1 << 25;
+        }
+        env->elr_el[new_el] = env->regs[15];
+
+        aarch64_sync_32_to_64(env);
+
+        env->condexec_bits = 0;
+    }
+    qemu_log_mask(CPU_LOG_INT, "...with ELR 0x%" PRIx64 "\n",
+                  env->elr_el[new_el]);
+
+    pstate_write(env, PSTATE_DAIF | new_mode);
+    env->aarch64 = 1;
+    aarch64_restore_sp(env, new_el);
+
+    env->pc = addr;
+
+    qemu_log_mask(CPU_LOG_INT, "...to EL%d PC 0x%" PRIx64 " PSTATE 0x%x\n",
+                  new_el, env->pc, pstate_read(env));
+
+    if (!kvm_enabled()) {
+        cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
+    }
+}
 
 /* Return the exception level which controls this address translation regime */
 static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
-- 
1.9.1

[Qemu-devel] [PATCH 3/8] target-arm: Use a single entry point for AArch64 and AArch32 exceptions

[Peter Maydell]

If EL2 or EL3 is present on an AArch64 CPU, then exceptions can be
taken to an exception level which is running AArch32 (if only EL0
and EL1 are present then EL1 must be AArch64 and all exceptions are
taken to AArch64). To support this we need to have a single
implementation of the CPU do_interrupt() method which can handle both
32 and 64 bit exception entry.

Pull the common parts of aarch64_cpu_do_interrupt() and
arm_cpu_do_interrupt() out into a new function which calls
either the AArch32 or AArch64 specific entry code once it has
worked out which one is needed.

We temporarily special-case the handling of EXCP_SEMIHOST to
avoid an assertion in arm_el_is_aa64(); the next patch will
pull all the semihosting handling out to the arm_cpu_do_interrupt()
level (since semihosting semantics depend on the register width
of the calling code, not on that of any higher EL).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu-qom.h |  2 --
 target-arm/cpu64.c   |  3 ---
 target-arm/helper.c  | 75 ++++++++++++++++++++++++++++++----------------------
 3 files changed, 44 insertions(+), 36 deletions(-)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index bda2af8..eae6cd1 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -249,6 +249,4 @@ int aarch64_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
 int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 #endif
 
-void aarch64_cpu_do_interrupt(CPUState *cs);
-
 #endif
diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index 63c8b1c..edb41f7 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -290,9 +290,6 @@ static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
 {
     CPUClass *cc = CPU_CLASS(oc);
 
-#if !defined(CONFIG_USER_ONLY)
-    cc->do_interrupt = aarch64_cpu_do_interrupt;
-#endif
     cc->cpu_exec_interrupt = arm_cpu_exec_interrupt;
     cc->set_pc = aarch64_cpu_set_pc;
     cc->gdb_read_register = aarch64_cpu_gdb_read_register;
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 519f066..962bb3c 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -5707,8 +5707,7 @@ void aarch64_sync_64_to_32(CPUARMState *env)
     env->regs[15] = env->pc;
 }
 
-/* Handle a CPU exception.  */
-void arm_cpu_do_interrupt(CPUState *cs)
+static void arm_cpu_do_interrupt_aarch32(CPUState *cs)
 {
     ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;
@@ -5718,16 +5717,6 @@ void arm_cpu_do_interrupt(CPUState *cs)
     uint32_t offset;
     uint32_t moe;
 
-    assert(!IS_M(env));
-
-    arm_log_exception(cs->exception_index);
-
-    if (arm_is_psci_call(cpu, cs->exception_index)) {
-        arm_handle_psci_call(cpu);
-        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
-        return;
-    }
-
     /* If this is a debug exception we must update the DBGDSCR.MOE bits */
     switch (env->exception.syndrome >> ARM_EL_EC_SHIFT) {
     case EC_BREAKPOINT:
@@ -5899,11 +5888,10 @@ void arm_cpu_do_interrupt(CPUState *cs)
     }
     env->regs[14] = env->regs[15] + offset;
     env->regs[15] = addr;
-    cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
 }
 
-/* Handle a CPU exception.  */
-void aarch64_cpu_do_interrupt(CPUState *cs)
+/* Handle exception entry to a target EL which is using AArch64 */
+static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
 {
     ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;
@@ -5921,22 +5909,6 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
         addr += 0x200;
     }
 
-    arm_log_exception(cs->exception_index);
-    qemu_log_mask(CPU_LOG_INT, "...from EL%d to EL%d\n", arm_current_el(env),
-                  new_el);
-    if (qemu_loglevel_mask(CPU_LOG_INT)
-        && !excp_is_internal(cs->exception_index)) {
-        qemu_log_mask(CPU_LOG_INT, "...with ESR %x/0x%" PRIx32 "\n",
-                      env->exception.syndrome >> ARM_EL_EC_SHIFT,
-                      env->exception.syndrome);
-    }
-
-    if (arm_is_psci_call(cpu, cs->exception_index)) {
-        arm_handle_psci_call(cpu);
-        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
-        return;
-    }
-
     switch (cs->exception_index) {
     case EXCP_PREFETCH_ABORT:
     case EXCP_DATA_ABORT:
@@ -5996,6 +5968,47 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
 
     qemu_log_mask(CPU_LOG_INT, "...to EL%d PC 0x%" PRIx64 " PSTATE 0x%x\n",
                   new_el, env->pc, pstate_read(env));
+}
+
+/* Handle a CPU exception for A and R profile CPUs.
+ * Do any appropriate logging, handle PSCI calls, and then hand off
+ * to the AArch64-entry or AArch32-entry function depending on the
+ * target exception level's register width.
+ */
+void arm_cpu_do_interrupt(CPUState *cs)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+    unsigned int new_el = env->exception.target_el;
+
+    assert(!IS_M(env));
+
+    arm_log_exception(cs->exception_index);
+    qemu_log_mask(CPU_LOG_INT, "...from EL%d to EL%d\n", arm_current_el(env),
+                  new_el);
+    if (qemu_loglevel_mask(CPU_LOG_INT)
+        && !excp_is_internal(cs->exception_index)) {
+        qemu_log_mask(CPU_LOG_INT, "...with ESR %x/0x%" PRIx32 "\n",
+                      env->exception.syndrome >> ARM_EL_EC_SHIFT,
+                      env->exception.syndrome);
+    }
+
+    if (arm_is_psci_call(cpu, cs->exception_index)) {
+        arm_handle_psci_call(cpu);
+        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
+        return;
+    }
+
+    /* Temporary special case for EXCP_SEMIHOST, which is used only
+     * for 64-bit semihosting calls -- as this is an internal exception
+     * it has no specified target level and arm_el_is_aa64() would
+     * assert because new_el could be 0.
+     */
+    if (cs->exception_index == EXCP_SEMIHOST || arm_el_is_aa64(env, new_el)) {
+        arm_cpu_do_interrupt_aarch64(cs);
+    } else {
+        arm_cpu_do_interrupt_aarch32(cs);
+    }
 
     if (!kvm_enabled()) {
         cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
-- 
1.9.1

[Qemu-devel] [PATCH 4/8] target-arm: Pull semihosting handling out to arm_cpu_do_interrupt()

[Peter Maydell]

Handling of semihosting calls should depend on the register width
of the calling code, not on that of any higher exception level,
so we need to identify and handle semihosting calls before we
decide whether to deliver the exception as an entry to AArch32
or AArch64. (EXCP_SEMIHOST is also an "internal exception" so
it has no target exception level in the first place.)

This will allow AArch32 EL1 code to use semihosting calls when
running under an AArch64 EL3.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper.c | 120 +++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 81 insertions(+), 39 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 962bb3c..d37c82c 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -5754,27 +5754,6 @@ static void arm_cpu_do_interrupt_aarch32(CPUState *cs)
             offset = 4;
         break;
     case EXCP_SWI:
-        if (semihosting_enabled()) {
-            /* Check for semihosting interrupt.  */
-            if (env->thumb) {
-                mask = arm_lduw_code(env, env->regs[15] - 2, env->bswap_code)
-                    & 0xff;
-            } else {
-                mask = arm_ldl_code(env, env->regs[15] - 4, env->bswap_code)
-                    & 0xffffff;
-            }
-            /* Only intercept calls from privileged modes, to provide some
-               semblance of security.  */
-            if (((mask == 0x123456 && !env->thumb)
-                    || (mask == 0xab && env->thumb))
-                  && (env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR) {
-                qemu_log_mask(CPU_LOG_INT,
-                              "...handling as semihosting call 0x%x\n",
-                              env->regs[0]);
-                env->regs[0] = do_arm_semihosting(env);
-                return;
-            }
-        }
         new_mode = ARM_CPU_MODE_SVC;
         addr = 0x08;
         mask = CPSR_I;
@@ -5782,19 +5761,6 @@ static void arm_cpu_do_interrupt_aarch32(CPUState *cs)
         offset = 0;
         break;
     case EXCP_BKPT:
-        /* See if this is a semihosting syscall.  */
-        if (env->thumb && semihosting_enabled()) {
-            mask = arm_lduw_code(env, env->regs[15], env->bswap_code) & 0xff;
-            if (mask == 0xab
-                  && (env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR) {
-                env->regs[15] += 2;
-                qemu_log_mask(CPU_LOG_INT,
-                              "...handling as semihosting call 0x%x\n",
-                              env->regs[0]);
-                env->regs[0] = do_arm_semihosting(env);
-                return;
-            }
-        }
         env->exception.fsr = 2;
         /* Fall through to prefetch abort.  */
     case EXCP_PREFETCH_ABORT:
@@ -5970,6 +5936,78 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
                   new_el, env->pc, pstate_read(env));
 }
 
+static inline bool check_for_semihosting(CPUState *cs)
+{
+    /* Check whether this exception is a semihosting call; if so
+     * then handle it and return true; otherwise return false.
+     */
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+
+    if (is_a64(env)) {
+        if (cs->exception_index == EXCP_SEMIHOST) {
+            /* This is always the 64-bit semihosting exception.
+             * The "is this usermode" and "is semihosting enabled"
+             * checks have been done at translate time.
+             */
+            qemu_log_mask(CPU_LOG_INT,
+                          "...handling as semihosting call 0x%" PRIx64 "\n",
+                          env->xregs[0]);
+            env->xregs[0] = do_arm_semihosting(env);
+            return true;
+        }
+        return false;
+    } else {
+        uint32_t imm;
+
+        /* Only intercept calls from privileged modes, to provide some
+         * semblance of security.
+         */
+        if (!semihosting_enabled() ||
+            ((env->uncached_cpsr & CPSR_M) == ARM_CPU_MODE_USR)) {
+            return false;
+        }
+
+        switch (cs->exception_index) {
+        case EXCP_SWI:
+            /* Check for semihosting interrupt.  */
+            if (env->thumb) {
+                imm = arm_lduw_code(env, env->regs[15] - 2, env->bswap_code)
+                    & 0xff;
+                if (imm == 0xab) {
+                    break;
+                }
+            } else {
+                imm = arm_ldl_code(env, env->regs[15] - 4, env->bswap_code)
+                    & 0xffffff;
+                if (imm == 0x123456) {
+                    break;
+                }
+            }
+            return false;
+        case EXCP_BKPT:
+            /* See if this is a semihosting syscall.  */
+            if (env->thumb) {
+                imm = arm_lduw_code(env, env->regs[15], env->bswap_code)
+                    & 0xff;
+                if (imm == 0xab) {
+                    env->regs[15] += 2;
+                    break;
+                }
+            }
+            return false;
+        default:
+            return false;
+        }
+
+        qemu_log_mask(CPU_LOG_INT,
+                      "...handling as semihosting call 0x%x\n",
+                      env->regs[0]);
+        env->regs[0] = do_arm_semihosting(env);
+        return true;
+    }
+}
+
 /* Handle a CPU exception for A and R profile CPUs.
  * Do any appropriate logging, handle PSCI calls, and then hand off
  * to the AArch64-entry or AArch32-entry function depending on the
@@ -5999,12 +6037,16 @@ void arm_cpu_do_interrupt(CPUState *cs)
         return;
     }
 
-    /* Temporary special case for EXCP_SEMIHOST, which is used only
-     * for 64-bit semihosting calls -- as this is an internal exception
-     * it has no specified target level and arm_el_is_aa64() would
-     * assert because new_el could be 0.
+    /* Semihosting semantics depend on the register width of the
+     * code that caused the exception, not the target exception level,
+     * so must be handled here.
      */
-    if (cs->exception_index == EXCP_SEMIHOST || arm_el_is_aa64(env, new_el)) {
+    if (check_for_semihosting(cs)) {
+        return;
+    }
+
+    assert(!excp_is_internal(cs->exception_index));
+    if (arm_el_is_aa64(env, new_el)) {
         arm_cpu_do_interrupt_aarch64(cs);
     } else {
         arm_cpu_do_interrupt_aarch32(cs);
-- 
1.9.1

[Qemu-devel] [PATCH 5/8] target-arm: Fix wrong AArch64 entry offset for EL2/EL3 target

[Peter Maydell]

The entry offset when taking an exception to AArch64 from a lower
exception level may be 0x400 or 0x600. 0x400 is used if the
implemented exception level immediately lower than the target level
is using AArch64, and 0x600 if it is using AArch32. We were
incorrectly implementing this as checking the exception level
that the exception was taken from. (The two can be different if
for example we take an exception from EL0 to AArch64 EL3; we should
in this case be checking EL2 if EL2 is implemented, and EL1 if
EL2 is not implemented.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index d37c82c..196c111 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -5866,7 +5866,26 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
     unsigned int new_mode = aarch64_pstate_mode(new_el, true);
 
     if (arm_current_el(env) < new_el) {
-        if (env->aarch64) {
+        /* Entry vector offset depends on whether the implemented EL
+         * immediately lower than the target level is using AArch32 or AArch64
+         */
+        bool is_aa64;
+
+        switch (new_el) {
+        case 3:
+            is_aa64 = (env->cp15.scr_el3 & SCR_RW) != 0;
+            break;
+        case 2:
+            is_aa64 = (env->cp15.hcr_el2 & HCR_RW) != 0;
+            break;
+        case 1:
+            is_aa64 = is_a64(env);
+            break;
+        default:
+            g_assert_not_reached();
+        }
+
+        if (is_aa64) {
             addr += 0x400;
         } else {
             addr += 0x600;
-- 
1.9.1

[Qemu-devel] [PATCH 6/8] target-arm: Handle exception return from AArch64 to non-EL0 AArch32

[Peter Maydell]

Remove the assumptions that the AArch64 exception return code was
making about a return to AArch32 always being a return to EL0.
This includes pulling out the illegal-SPSR checks so we can apply
them for return to 32 bit as well as return to 64-bit.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/op_helper.c | 80 +++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 59 insertions(+), 21 deletions(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index e42d287..38d46d8 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -640,12 +640,51 @@ void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)
     }
 }
 
+static int el_from_spsr(uint32_t spsr)
+{
+    /* Return the exception level that this SPSR is requesting a return to,
+     * or -1 if it is invalid (an illegal return)
+     */
+    if (spsr & PSTATE_nRW) {
+        switch (spsr & CPSR_M) {
+        case ARM_CPU_MODE_USR:
+            return 0;
+        case ARM_CPU_MODE_HYP:
+            return 2;
+        case ARM_CPU_MODE_FIQ:
+        case ARM_CPU_MODE_IRQ:
+        case ARM_CPU_MODE_SVC:
+        case ARM_CPU_MODE_ABT:
+        case ARM_CPU_MODE_UND:
+        case ARM_CPU_MODE_SYS:
+            return 1;
+        case ARM_CPU_MODE_MON:
+            /* Returning to Mon from AArch64 is never possible,
+             * so this is an illegal return.
+             */
+        default:
+            return -1;
+        }
+    } else {
+        if (extract32(spsr, 1, 1)) {
+            /* Return with reserved M[1] bit set */
+            return -1;
+        }
+        if (extract32(spsr, 0, 4) == 1) {
+            /* return to EL0 with M[0] bit set */
+            return -1;
+        }
+        return extract32(spsr, 2, 2);
+    }
+}
+
 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;
+    bool return_to_aa64 = (spsr & PSTATE_nRW) == 0;
 
     aarch64_save_sp(env, cur_el);
 
@@ -662,35 +701,34 @@ void HELPER(exception_return)(CPUARMState *env)
         spsr &= ~PSTATE_SS;
     }
 
-    if (spsr & PSTATE_nRW) {
-        /* TODO: We currently assume EL1/2/3 are running in AArch64.  */
+    new_el = el_from_spsr(spsr);
+    if (new_el == -1) {
+        goto illegal_return;
+    }
+    if (new_el > cur_el
+        || (new_el == 2 && !arm_feature(env, ARM_FEATURE_EL2))) {
+        /* Disallow return to an EL which is unimplemented or higher
+         * than the current one.
+         */
+        goto illegal_return;
+    }
+
+    if (new_el != 0 && arm_el_is_aa64(env, new_el) != return_to_aa64) {
+        /* Return to an EL which is configured for a different register width */
+        goto illegal_return;
+    }
+
+    if (!return_to_aa64) {
         env->aarch64 = 0;
-        new_el = 0;
-        env->uncached_cpsr = 0x10;
+        env->uncached_cpsr = spsr & CPSR_M;
         cpsr_write(env, spsr, ~0);
         if (!arm_singlestep_active(env)) {
             env->uncached_cpsr &= ~PSTATE_SS;
         }
         aarch64_sync_64_to_32(env);
 
-        env->regs[15] = env->elr_el[1] & ~0x1;
+        env->regs[15] = env->elr_el[cur_el] & ~0x1;
     } else {
-        new_el = extract32(spsr, 2, 2);
-        if (new_el > cur_el
-            || (new_el == 2 && !arm_feature(env, ARM_FEATURE_EL2))) {
-            /* Disallow return to an EL which is unimplemented or higher
-             * than the current one.
-             */
-            goto illegal_return;
-        }
-        if (extract32(spsr, 1, 1)) {
-            /* Return with reserved M[1] bit set */
-            goto illegal_return;
-        }
-        if (new_el == 0 && (spsr & PSTATE_SP)) {
-            /* Return to EL0 with M[0] bit set */
-            goto illegal_return;
-        }
         env->aarch64 = 1;
         pstate_write(env, spsr);
         if (!arm_singlestep_active(env)) {
-- 
1.9.1

[Qemu-devel] [PATCH 7/8] target-arm: Implement remaining illegal return event checks

[Peter Maydell]

We already implement almost all the checks for the illegal
return events from AArch64 state described in the ARM ARM section
D1.11.2. Add the two missing ones:
 * return to EL2 when EL3 is implemented and SCR_EL3.NS is 0
 * return to Non-secure EL1 when EL2 is implemented and HCR_EL2.TGE is 1

(We don't implement external debug, so the case of "debug state exit
from EL0 using AArch64 state to EL0 using AArch32 state" doesn't apply
for QEMU.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/op_helper.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 38d46d8..5789ccb 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -718,6 +718,17 @@ void HELPER(exception_return)(CPUARMState *env)
         goto illegal_return;
     }
 
+    if (new_el == 2 && arm_is_secure_below_el3(env)) {
+        /* Return to the non-existent secure-EL2 */
+        goto illegal_return;
+    }
+
+    if (new_el == 1 &&
+        arm_feature(env, ARM_FEATURE_EL2) && (env->cp15.hcr_el2 & HCR_TGE)
+        && !arm_is_secure_below_el3(env)) {
+        goto illegal_return;
+    }
+
     if (!return_to_aa64) {
         env->aarch64 = 0;
         env->uncached_cpsr = spsr & CPSR_M;
-- 
1.9.1

[Qemu-devel] [PATCH 8/8] target-arm: ignore ELR_ELx[1] for exception return to 32-bit ARM mode

[Peter Maydell]

The architecture requires that for an exception return to AArch32 the
low bits of ELR_ELx are ignored when the PC is set from them:
 * if returning to Thumb mode, ignore ELR_ELx[0]
 * if returning to ARM mode, ignore ELR_ELx[1:0]

We were only squashing bit 0; also squash bit 1 if the SPSR T bit
indicates this is a return to ARM code.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/op_helper.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 5789ccb..171d6b8 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -738,7 +738,11 @@ void HELPER(exception_return)(CPUARMState *env)
         }
         aarch64_sync_64_to_32(env);
 
-        env->regs[15] = env->elr_el[cur_el] & ~0x1;
+        if (spsr & CPSR_T) {
+            env->regs[15] = env->elr_el[cur_el] & ~0x1;
+        } else {
+            env->regs[15] = env->elr_el[cur_el] & ~0x3;
+        }
     } else {
         env->aarch64 = 1;
         pstate_write(env, spsr);
-- 
1.9.1

Re: [Qemu-devel] [PATCH 1/8] target-arm: Properly support EL2 and EL3 in arm_el_is_aa64()

[Edgar E. Iglesias]

On Thu, Jan 14, 2016 at 06:34:04PM +0000, Peter Maydell wrote:
> Support EL2 and EL3 in arm_el_is_aa64() by implementing the
> logic for checking the SCR_EL3 and HCR_EL2 register-width bits
> as appropriate to determine the register width of lower exception
> levels.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Hi Peter,

On the ZynqMP we've got the Cortex-A53 EL3 RW configurable at reset
time. At some later point we'll likely have to implement that
runtime option...

Anyway:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> ---
>  target-arm/cpu.h | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 5f81342..b8b3364 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -969,18 +969,33 @@ static inline bool arm_is_secure(CPUARMState *env)
>  /* Return true if the specified exception level is running in AArch64 state. */
>  static inline bool arm_el_is_aa64(CPUARMState *env, int el)
>  {
> -    /* We don't currently support EL2, and this isn't valid for EL0
> -     * (if we're in EL0, is_a64() is what you want, and if we're not in EL0
> -     * then the state of EL0 isn't well defined.)
> +    /* This isn't valid for EL0 (if we're in EL0, is_a64() is what you want,
> +     * and if we're not in EL0 then the state of EL0 isn't well defined.)
>       */
> -    assert(el == 1 || el == 3);
> +    assert(el >= 1 && el <= 3);
> +    bool aa64 = arm_feature(env, ARM_FEATURE_AARCH64);
>  
> -    /* AArch64-capable CPUs always run with EL1 in AArch64 mode. This
> -     * is a QEMU-imposed simplification which we may wish to change later.
> -     * If we in future support EL2 and/or EL3, then the state of lower
> -     * exception levels is controlled by the HCR.RW and SCR.RW bits.
> +    /* The highest exception level is always at the maximum supported
> +     * register width, and then lower levels have a register width controlled
> +     * by bits in the SCR or HCR registers.
>       */
> -    return arm_feature(env, ARM_FEATURE_AARCH64);
> +    if (el == 3) {
> +        return aa64;
> +    }
> +
> +    if (arm_feature(env, ARM_FEATURE_EL3)) {
> +        aa64 = aa64 && (env->cp15.scr_el3 & SCR_RW);
> +    }
> +
> +    if (el == 2) {
> +        return aa64;
> +    }
> +
> +    if (arm_feature(env, ARM_FEATURE_EL2) && !arm_is_secure_below_el3(env)) {
> +        aa64 = aa64 && (env->cp15.hcr_el2 & HCR_RW);
> +    }
> +
> +    return aa64;
>  }
>  
>  /* Function for determing whether guest cp register reads and writes should
> -- 
> 1.9.1
> 

Re: [Qemu-devel] [PATCH 2/8] target-arm: Move aarch64_cpu_do_interrupt() to helper.c

[Edgar E. Iglesias]

On Thu, Jan 14, 2016 at 06:34:05PM +0000, Peter Maydell wrote:
> Move the aarch64_cpu_do_interrupt() function to helper.c. We want
> to be able to call this from code that isn't AArch64-only, and
> the move allows us to avoid awkward #ifdeffery at the callsite.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> ---
>  target-arm/cpu-qom.h    |   2 +-
>  target-arm/helper-a64.c | 104 ------------------------------------------------
>  target-arm/helper.c     | 100 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 101 insertions(+), 105 deletions(-)
> 
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index e4d4270..bda2af8 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -247,8 +247,8 @@ void arm_gt_stimer_cb(void *opaque);
>  #ifdef TARGET_AARCH64
>  int aarch64_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
>  int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
> +#endif
>  
>  void aarch64_cpu_do_interrupt(CPUState *cs);
> -#endif
>  
>  #endif
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index fc3ccdf..a322e7b 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -25,7 +25,6 @@
>  #include "qemu/bitops.h"
>  #include "internals.h"
>  #include "qemu/crc32c.h"
> -#include "sysemu/kvm.h"
>  #include <zlib.h> /* For crc32 */
>  
>  /* C2.4.7 Multiply and divide */
> @@ -443,106 +442,3 @@ uint64_t HELPER(crc32c_64)(uint64_t acc, uint64_t val, uint32_t bytes)
>      /* Linux crc32c converts the output to one's complement.  */
>      return crc32c(acc, buf, bytes) ^ 0xffffffff;
>  }
> -
> -#if !defined(CONFIG_USER_ONLY)
> -
> -/* Handle a CPU exception.  */
> -void aarch64_cpu_do_interrupt(CPUState *cs)
> -{
> -    ARMCPU *cpu = ARM_CPU(cs);
> -    CPUARMState *env = &cpu->env;
> -    unsigned int new_el = env->exception.target_el;
> -    target_ulong addr = env->cp15.vbar_el[new_el];
> -    unsigned int new_mode = aarch64_pstate_mode(new_el, true);
> -
> -    if (arm_current_el(env) < new_el) {
> -        if (env->aarch64) {
> -            addr += 0x400;
> -        } else {
> -            addr += 0x600;
> -        }
> -    } else if (pstate_read(env) & PSTATE_SP) {
> -        addr += 0x200;
> -    }
> -
> -    arm_log_exception(cs->exception_index);
> -    qemu_log_mask(CPU_LOG_INT, "...from EL%d to EL%d\n", arm_current_el(env),
> -                  new_el);
> -    if (qemu_loglevel_mask(CPU_LOG_INT)
> -        && !excp_is_internal(cs->exception_index)) {
> -        qemu_log_mask(CPU_LOG_INT, "...with ESR %x/0x%" PRIx32 "\n",
> -                      env->exception.syndrome >> ARM_EL_EC_SHIFT,
> -                      env->exception.syndrome);
> -    }
> -
> -    if (arm_is_psci_call(cpu, cs->exception_index)) {
> -        arm_handle_psci_call(cpu);
> -        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
> -        return;
> -    }
> -
> -    switch (cs->exception_index) {
> -    case EXCP_PREFETCH_ABORT:
> -    case EXCP_DATA_ABORT:
> -        env->cp15.far_el[new_el] = env->exception.vaddress;
> -        qemu_log_mask(CPU_LOG_INT, "...with FAR 0x%" PRIx64 "\n",
> -                      env->cp15.far_el[new_el]);
> -        /* fall through */
> -    case EXCP_BKPT:
> -    case EXCP_UDEF:
> -    case EXCP_SWI:
> -    case EXCP_HVC:
> -    case EXCP_HYP_TRAP:
> -    case EXCP_SMC:
> -        env->cp15.esr_el[new_el] = env->exception.syndrome;
> -        break;
> -    case EXCP_IRQ:
> -    case EXCP_VIRQ:
> -        addr += 0x80;
> -        break;
> -    case EXCP_FIQ:
> -    case EXCP_VFIQ:
> -        addr += 0x100;
> -        break;
> -    case EXCP_SEMIHOST:
> -        qemu_log_mask(CPU_LOG_INT,
> -                      "...handling as semihosting call 0x%" PRIx64 "\n",
> -                      env->xregs[0]);
> -        env->xregs[0] = do_arm_semihosting(env);
> -        return;
> -    default:
> -        cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
> -    }
> -
> -    if (is_a64(env)) {
> -        env->banked_spsr[aarch64_banked_spsr_index(new_el)] = pstate_read(env);
> -        aarch64_save_sp(env, arm_current_el(env));
> -        env->elr_el[new_el] = env->pc;
> -    } else {
> -        env->banked_spsr[aarch64_banked_spsr_index(new_el)] = cpsr_read(env);
> -        if (!env->thumb) {
> -            env->cp15.esr_el[new_el] |= 1 << 25;
> -        }
> -        env->elr_el[new_el] = env->regs[15];
> -
> -        aarch64_sync_32_to_64(env);
> -
> -        env->condexec_bits = 0;
> -    }
> -    qemu_log_mask(CPU_LOG_INT, "...with ELR 0x%" PRIx64 "\n",
> -                  env->elr_el[new_el]);
> -
> -    pstate_write(env, PSTATE_DAIF | new_mode);
> -    env->aarch64 = 1;
> -    aarch64_restore_sp(env, new_el);
> -
> -    env->pc = addr;
> -
> -    qemu_log_mask(CPU_LOG_INT, "...to EL%d PC 0x%" PRIx64 " PSTATE 0x%x\n",
> -                  new_el, env->pc, pstate_read(env));
> -
> -    if (!kvm_enabled()) {
> -        cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
> -    }
> -}
> -#endif
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index a06bfaf..519f066 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -11,6 +11,7 @@
>  #include "arm_ldst.h"
>  #include <zlib.h> /* For crc32 */
>  #include "exec/semihost.h"
> +#include "sysemu/kvm.h"
>  
>  #define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */
>  
> @@ -5901,6 +5902,105 @@ void arm_cpu_do_interrupt(CPUState *cs)
>      cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
>  }
>  
> +/* Handle a CPU exception.  */
> +void aarch64_cpu_do_interrupt(CPUState *cs)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +    unsigned int new_el = env->exception.target_el;
> +    target_ulong addr = env->cp15.vbar_el[new_el];
> +    unsigned int new_mode = aarch64_pstate_mode(new_el, true);
> +
> +    if (arm_current_el(env) < new_el) {
> +        if (env->aarch64) {
> +            addr += 0x400;
> +        } else {
> +            addr += 0x600;
> +        }
> +    } else if (pstate_read(env) & PSTATE_SP) {
> +        addr += 0x200;
> +    }
> +
> +    arm_log_exception(cs->exception_index);
> +    qemu_log_mask(CPU_LOG_INT, "...from EL%d to EL%d\n", arm_current_el(env),
> +                  new_el);
> +    if (qemu_loglevel_mask(CPU_LOG_INT)
> +        && !excp_is_internal(cs->exception_index)) {
> +        qemu_log_mask(CPU_LOG_INT, "...with ESR %x/0x%" PRIx32 "\n",
> +                      env->exception.syndrome >> ARM_EL_EC_SHIFT,
> +                      env->exception.syndrome);
> +    }
> +
> +    if (arm_is_psci_call(cpu, cs->exception_index)) {
> +        arm_handle_psci_call(cpu);
> +        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
> +        return;
> +    }
> +
> +    switch (cs->exception_index) {
> +    case EXCP_PREFETCH_ABORT:
> +    case EXCP_DATA_ABORT:
> +        env->cp15.far_el[new_el] = env->exception.vaddress;
> +        qemu_log_mask(CPU_LOG_INT, "...with FAR 0x%" PRIx64 "\n",
> +                      env->cp15.far_el[new_el]);
> +        /* fall through */
> +    case EXCP_BKPT:
> +    case EXCP_UDEF:
> +    case EXCP_SWI:
> +    case EXCP_HVC:
> +    case EXCP_HYP_TRAP:
> +    case EXCP_SMC:
> +        env->cp15.esr_el[new_el] = env->exception.syndrome;
> +        break;
> +    case EXCP_IRQ:
> +    case EXCP_VIRQ:
> +        addr += 0x80;
> +        break;
> +    case EXCP_FIQ:
> +    case EXCP_VFIQ:
> +        addr += 0x100;
> +        break;
> +    case EXCP_SEMIHOST:
> +        qemu_log_mask(CPU_LOG_INT,
> +                      "...handling as semihosting call 0x%" PRIx64 "\n",
> +                      env->xregs[0]);
> +        env->xregs[0] = do_arm_semihosting(env);
> +        return;
> +    default:
> +        cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
> +    }
> +
> +    if (is_a64(env)) {
> +        env->banked_spsr[aarch64_banked_spsr_index(new_el)] = pstate_read(env);
> +        aarch64_save_sp(env, arm_current_el(env));
> +        env->elr_el[new_el] = env->pc;
> +    } else {
> +        env->banked_spsr[aarch64_banked_spsr_index(new_el)] = cpsr_read(env);
> +        if (!env->thumb) {
> +            env->cp15.esr_el[new_el] |= 1 << 25;
> +        }
> +        env->elr_el[new_el] = env->regs[15];
> +
> +        aarch64_sync_32_to_64(env);
> +
> +        env->condexec_bits = 0;
> +    }
> +    qemu_log_mask(CPU_LOG_INT, "...with ELR 0x%" PRIx64 "\n",
> +                  env->elr_el[new_el]);
> +
> +    pstate_write(env, PSTATE_DAIF | new_mode);
> +    env->aarch64 = 1;
> +    aarch64_restore_sp(env, new_el);
> +
> +    env->pc = addr;
> +
> +    qemu_log_mask(CPU_LOG_INT, "...to EL%d PC 0x%" PRIx64 " PSTATE 0x%x\n",
> +                  new_el, env->pc, pstate_read(env));
> +
> +    if (!kvm_enabled()) {
> +        cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
> +    }
> +}
>  
>  /* Return the exception level which controls this address translation regime */
>  static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
> -- 
> 1.9.1
> 

Re: [Qemu-devel] [PATCH 1/8] target-arm: Properly support EL2 and EL3 in arm_el_is_aa64()

[Peter Maydell]

On 15 January 2016 at 14:38, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Thu, Jan 14, 2016 at 06:34:04PM +0000, Peter Maydell wrote:
>> Support EL2 and EL3 in arm_el_is_aa64() by implementing the
>> logic for checking the SCR_EL3 and HCR_EL2 register-width bits
>> as appropriate to determine the register width of lower exception
>> levels.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> Hi Peter,
>
> On the ZynqMP we've got the Cortex-A53 EL3 RW configurable at reset
> time. At some later point we'll likely have to implement that
> runtime option...

That might be tricky, we fairly well bake in "AARCH64 feature means
64-bit highest EL" at the moment. The KVM code takes the approach
of "if it's not going to reset in AArch64 then unset the feature bit".

Anyway, we'll cross that bridge when we get to it.

Do you have much locally extra that you needed for enabling
EL3 in the Cortex-A53? I have an ARM Trusted Firmware + OP-TEE
setup now that I'm going to use to work through the missing bits,
but if you've already gone through that effort there's no need
my duplicating work...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/8] target-arm: Use a single entry point for AArch64 and AArch32 exceptions

[Edgar E. Iglesias]

On Thu, Jan 14, 2016 at 06:34:06PM +0000, Peter Maydell wrote:
> If EL2 or EL3 is present on an AArch64 CPU, then exceptions can be
> taken to an exception level which is running AArch32 (if only EL0
> and EL1 are present then EL1 must be AArch64 and all exceptions are
> taken to AArch64). To support this we need to have a single
> implementation of the CPU do_interrupt() method which can handle both
> 32 and 64 bit exception entry.
> 
> Pull the common parts of aarch64_cpu_do_interrupt() and
> arm_cpu_do_interrupt() out into a new function which calls
> either the AArch32 or AArch64 specific entry code once it has
> worked out which one is needed.
> 
> We temporarily special-case the handling of EXCP_SEMIHOST to
> avoid an assertion in arm_el_is_aa64(); the next patch will
> pull all the semihosting handling out to the arm_cpu_do_interrupt()
> level (since semihosting semantics depend on the register width
> of the calling code, not on that of any higher EL).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> ---
>  target-arm/cpu-qom.h |  2 --
>  target-arm/cpu64.c   |  3 ---
>  target-arm/helper.c  | 75 ++++++++++++++++++++++++++++++----------------------
>  3 files changed, 44 insertions(+), 36 deletions(-)
> 
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index bda2af8..eae6cd1 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -249,6 +249,4 @@ int aarch64_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
>  int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>  #endif
>  
> -void aarch64_cpu_do_interrupt(CPUState *cs);
> -
>  #endif
> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
> index 63c8b1c..edb41f7 100644
> --- a/target-arm/cpu64.c
> +++ b/target-arm/cpu64.c
> @@ -290,9 +290,6 @@ static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
>  {
>      CPUClass *cc = CPU_CLASS(oc);
>  
> -#if !defined(CONFIG_USER_ONLY)
> -    cc->do_interrupt = aarch64_cpu_do_interrupt;
> -#endif
>      cc->cpu_exec_interrupt = arm_cpu_exec_interrupt;
>      cc->set_pc = aarch64_cpu_set_pc;
>      cc->gdb_read_register = aarch64_cpu_gdb_read_register;
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 519f066..962bb3c 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -5707,8 +5707,7 @@ void aarch64_sync_64_to_32(CPUARMState *env)
>      env->regs[15] = env->pc;
>  }
>  
> -/* Handle a CPU exception.  */
> -void arm_cpu_do_interrupt(CPUState *cs)
> +static void arm_cpu_do_interrupt_aarch32(CPUState *cs)
>  {
>      ARMCPU *cpu = ARM_CPU(cs);
>      CPUARMState *env = &cpu->env;
> @@ -5718,16 +5717,6 @@ void arm_cpu_do_interrupt(CPUState *cs)
>      uint32_t offset;
>      uint32_t moe;
>  
> -    assert(!IS_M(env));
> -
> -    arm_log_exception(cs->exception_index);
> -
> -    if (arm_is_psci_call(cpu, cs->exception_index)) {
> -        arm_handle_psci_call(cpu);
> -        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
> -        return;
> -    }
> -
>      /* If this is a debug exception we must update the DBGDSCR.MOE bits */
>      switch (env->exception.syndrome >> ARM_EL_EC_SHIFT) {
>      case EC_BREAKPOINT:
> @@ -5899,11 +5888,10 @@ void arm_cpu_do_interrupt(CPUState *cs)
>      }
>      env->regs[14] = env->regs[15] + offset;
>      env->regs[15] = addr;
> -    cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
>  }
>  
> -/* Handle a CPU exception.  */
> -void aarch64_cpu_do_interrupt(CPUState *cs)
> +/* Handle exception entry to a target EL which is using AArch64 */
> +static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
>  {
>      ARMCPU *cpu = ARM_CPU(cs);
>      CPUARMState *env = &cpu->env;
> @@ -5921,22 +5909,6 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>          addr += 0x200;
>      }
>  
> -    arm_log_exception(cs->exception_index);
> -    qemu_log_mask(CPU_LOG_INT, "...from EL%d to EL%d\n", arm_current_el(env),
> -                  new_el);
> -    if (qemu_loglevel_mask(CPU_LOG_INT)
> -        && !excp_is_internal(cs->exception_index)) {
> -        qemu_log_mask(CPU_LOG_INT, "...with ESR %x/0x%" PRIx32 "\n",
> -                      env->exception.syndrome >> ARM_EL_EC_SHIFT,
> -                      env->exception.syndrome);
> -    }
> -
> -    if (arm_is_psci_call(cpu, cs->exception_index)) {
> -        arm_handle_psci_call(cpu);
> -        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
> -        return;
> -    }
> -
>      switch (cs->exception_index) {
>      case EXCP_PREFETCH_ABORT:
>      case EXCP_DATA_ABORT:
> @@ -5996,6 +5968,47 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>  
>      qemu_log_mask(CPU_LOG_INT, "...to EL%d PC 0x%" PRIx64 " PSTATE 0x%x\n",
>                    new_el, env->pc, pstate_read(env));
> +}
> +
> +/* Handle a CPU exception for A and R profile CPUs.
> + * Do any appropriate logging, handle PSCI calls, and then hand off
> + * to the AArch64-entry or AArch32-entry function depending on the
> + * target exception level's register width.
> + */
> +void arm_cpu_do_interrupt(CPUState *cs)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +    unsigned int new_el = env->exception.target_el;
> +
> +    assert(!IS_M(env));
> +
> +    arm_log_exception(cs->exception_index);
> +    qemu_log_mask(CPU_LOG_INT, "...from EL%d to EL%d\n", arm_current_el(env),
> +                  new_el);
> +    if (qemu_loglevel_mask(CPU_LOG_INT)
> +        && !excp_is_internal(cs->exception_index)) {
> +        qemu_log_mask(CPU_LOG_INT, "...with ESR %x/0x%" PRIx32 "\n",
> +                      env->exception.syndrome >> ARM_EL_EC_SHIFT,
> +                      env->exception.syndrome);
> +    }
> +
> +    if (arm_is_psci_call(cpu, cs->exception_index)) {
> +        arm_handle_psci_call(cpu);
> +        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
> +        return;
> +    }
> +
> +    /* Temporary special case for EXCP_SEMIHOST, which is used only
> +     * for 64-bit semihosting calls -- as this is an internal exception
> +     * it has no specified target level and arm_el_is_aa64() would
> +     * assert because new_el could be 0.
> +     */
> +    if (cs->exception_index == EXCP_SEMIHOST || arm_el_is_aa64(env, new_el)) {
> +        arm_cpu_do_interrupt_aarch64(cs);
> +    } else {
> +        arm_cpu_do_interrupt_aarch32(cs);
> +    }
>  
>      if (!kvm_enabled()) {
>          cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
> -- 
> 1.9.1
> 

Re: [Qemu-devel] [PATCH 1/8] target-arm: Properly support EL2 and EL3 in arm_el_is_aa64()

[Edgar E. Iglesias]

On Fri, Jan 15, 2016 at 02:50:24PM +0000, Peter Maydell wrote:
> On 15 January 2016 at 14:38, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > On Thu, Jan 14, 2016 at 06:34:04PM +0000, Peter Maydell wrote:
> >> Support EL2 and EL3 in arm_el_is_aa64() by implementing the
> >> logic for checking the SCR_EL3 and HCR_EL2 register-width bits
> >> as appropriate to determine the register width of lower exception
> >> levels.
> >>
> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >
> > Hi Peter,
> >
> > On the ZynqMP we've got the Cortex-A53 EL3 RW configurable at reset
> > time. At some later point we'll likely have to implement that
> > runtime option...
> 
> That might be tricky, we fairly well bake in "AARCH64 feature means
> 64-bit highest EL" at the moment. The KVM code takes the approach
> of "if it's not going to reset in AArch64 then unset the feature bit".
> 
> Anyway, we'll cross that bridge when we get to it.
> 
> Do you have much locally extra that you needed for enabling
> EL3 in the Cortex-A53? I have an ARM Trusted Firmware + OP-TEE
> setup now that I'm going to use to work through the missing bits,
> but if you've already gone through that effort there's no need
> my duplicating work...

I don't have anything immediate for EL3 beyond enabling it and some
boot thing for a15/aarch32 to allow me to run my tests. I haven't
really looked at the boot in detail for aa32 so I haven't bothered
submitting it. This is it:

commit b30c7102624241a67ebb2d3df70e88a4148f68a4
Author: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Date:   Sun Sep 13 09:52:01 2015 +0200

    target-arm: Start EL3 capable ARMv7 cores in MON mode
    
    Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index f6f5539..485965f 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -164,6 +164,9 @@ static void arm_cpu_reset(CPUState *s)
 #else
     /* SVC mode with interrupts disabled.  */
     env->uncached_cpsr = ARM_CPU_MODE_SVC;
+    if (arm_feature(env, ARM_FEATURE_EL3)) {
+        env->uncached_cpsr = ARM_CPU_MODE_MON;
+    }
     env->daif = PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F;
     /* On ARMv7-M the CPSR_I is the value of the PRIMASK register, and is
      * clear at reset. Initial SP and PC are loaded from ROM.

Re: [Qemu-devel] [PATCH 1/8] target-arm: Properly support EL2 and EL3 in arm_el_is_aa64()

[Peter Maydell]

On 15 January 2016 at 15:37, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Fri, Jan 15, 2016 at 02:50:24PM +0000, Peter Maydell wrote:
>> Do you have much locally extra that you needed for enabling
>> EL3 in the Cortex-A53? I have an ARM Trusted Firmware + OP-TEE
>> setup now that I'm going to use to work through the missing bits,
>> but if you've already gone through that effort there's no need
>> my duplicating work...
>
> I don't have anything immediate for EL3 beyond enabling it and some
> boot thing for a15/aarch32 to allow me to run my tests.

Cool. I'm not sure at what point to add the patch that turns
on the EL3 feature bit, but I guess in the not too distant future :-)

> I haven't
> really looked at the boot in detail for aa32 so I haven't bothered
> submitting it. This is it:
>
> commit b30c7102624241a67ebb2d3df70e88a4148f68a4
> Author: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> Date:   Sun Sep 13 09:52:01 2015 +0200
>
>     target-arm: Start EL3 capable ARMv7 cores in MON mode
>
>     Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index f6f5539..485965f 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -164,6 +164,9 @@ static void arm_cpu_reset(CPUState *s)
>  #else
>      /* SVC mode with interrupts disabled.  */
>      env->uncached_cpsr = ARM_CPU_MODE_SVC;
> +    if (arm_feature(env, ARM_FEATURE_EL3)) {
> +        env->uncached_cpsr = ARM_CPU_MODE_MON;
> +    }
>      env->daif = PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F;
>      /* On ARMv7-M the CPSR_I is the value of the PRIMASK register, and is
>       * clear at reset. Initial SP and PC are loaded from ROM.

This doesn't look right. A 32-bit CPU with TrustZone boots into
Secure-SVC, not Mon. This works because in the v7 security model
Secure-SVC and Mon are at the same privilege level, unlike AArch64
where EL3 is higher privilege than EL1. If guest code needs to
get into Mon mode it can do so from S-SVC (eg set MVBAR, make
sure there's sensible code at that vector entrypoint, execute
an SMC).

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/8] target-arm: Properly support EL2 and EL3 in arm_el_is_aa64()

[Edgar E. Iglesias]

On Fri, Jan 15, 2016 at 03:47:17PM +0000, Peter Maydell wrote:
> On 15 January 2016 at 15:37, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > On Fri, Jan 15, 2016 at 02:50:24PM +0000, Peter Maydell wrote:
> >> Do you have much locally extra that you needed for enabling
> >> EL3 in the Cortex-A53? I have an ARM Trusted Firmware + OP-TEE
> >> setup now that I'm going to use to work through the missing bits,
> >> but if you've already gone through that effort there's no need
> >> my duplicating work...
> >
> > I don't have anything immediate for EL3 beyond enabling it and some
> > boot thing for a15/aarch32 to allow me to run my tests.
> 
> Cool. I'm not sure at what point to add the patch that turns
> on the EL3 feature bit, but I guess in the not too distant future :-)

IMO, we should have enabled it by now :-)
I guess the EL2 needs the 2 stage MMU error handling and maybe the GIC virt features
to be reasonably useful, I don't know.

> 
> > I haven't
> > really looked at the boot in detail for aa32 so I haven't bothered
> > submitting it. This is it:
> >
> > commit b30c7102624241a67ebb2d3df70e88a4148f68a4
> > Author: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > Date:   Sun Sep 13 09:52:01 2015 +0200
> >
> >     target-arm: Start EL3 capable ARMv7 cores in MON mode
> >
> >     Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> >
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index f6f5539..485965f 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -164,6 +164,9 @@ static void arm_cpu_reset(CPUState *s)
> >  #else
> >      /* SVC mode with interrupts disabled.  */
> >      env->uncached_cpsr = ARM_CPU_MODE_SVC;
> > +    if (arm_feature(env, ARM_FEATURE_EL3)) {
> > +        env->uncached_cpsr = ARM_CPU_MODE_MON;
> > +    }
> >      env->daif = PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F;
> >      /* On ARMv7-M the CPSR_I is the value of the PRIMASK register, and is
> >       * clear at reset. Initial SP and PC are loaded from ROM.
> 
> This doesn't look right. A 32-bit CPU with TrustZone boots into
> Secure-SVC, not Mon. This works because in the v7 security model
> Secure-SVC and Mon are at the same privilege level, unlike AArch64
> where EL3 is higher privilege than EL1. If guest code needs to
> get into Mon mode it can do so from S-SVC (eg set MVBAR, make
> sure there's sensible code at that vector entrypoint, execute
> an SMC).

Thanks. I figured my zero research would prove me wrong :-)
Thinking about it, I have never ran my aa32 TZ testsuite on real
HW so there are tons of stuff that could be wrong!

Best regards,
Edgar

Re: [Qemu-devel] [PATCH 5/8] target-arm: Fix wrong AArch64 entry offset for EL2/EL3 target

[Edgar E. Iglesias]

On Thu, Jan 14, 2016 at 06:34:08PM +0000, Peter Maydell wrote:
> The entry offset when taking an exception to AArch64 from a lower
> exception level may be 0x400 or 0x600. 0x400 is used if the
> implemented exception level immediately lower than the target level
> is using AArch64, and 0x600 if it is using AArch32. We were
> incorrectly implementing this as checking the exception level
> that the exception was taken from. (The two can be different if
> for example we take an exception from EL0 to AArch64 EL3; we should
> in this case be checking EL2 if EL2 is implemented, and EL1 if
> EL2 is not implemented.)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>



> ---
>  target-arm/helper.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index d37c82c..196c111 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -5866,7 +5866,26 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
>      unsigned int new_mode = aarch64_pstate_mode(new_el, true);
>  
>      if (arm_current_el(env) < new_el) {
> -        if (env->aarch64) {
> +        /* Entry vector offset depends on whether the implemented EL
> +         * immediately lower than the target level is using AArch32 or AArch64
> +         */
> +        bool is_aa64;
> +
> +        switch (new_el) {
> +        case 3:
> +            is_aa64 = (env->cp15.scr_el3 & SCR_RW) != 0;
> +            break;
> +        case 2:
> +            is_aa64 = (env->cp15.hcr_el2 & HCR_RW) != 0;
> +            break;
> +        case 1:
> +            is_aa64 = is_a64(env);
> +            break;
> +        default:
> +            g_assert_not_reached();
> +        }
> +
> +        if (is_aa64) {
>              addr += 0x400;
>          } else {
>              addr += 0x600;
> -- 
> 1.9.1
> 

Re: [Qemu-devel] [PATCH 6/8] target-arm: Handle exception return from AArch64 to non-EL0 AArch32

[Edgar E. Iglesias]

On Thu, Jan 14, 2016 at 06:34:09PM +0000, Peter Maydell wrote:
> Remove the assumptions that the AArch64 exception return code was
> making about a return to AArch32 always being a return to EL0.
> This includes pulling out the illegal-SPSR checks so we can apply
> them for return to 32 bit as well as return to 64-bit.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> ---
>  target-arm/op_helper.c | 80 +++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 59 insertions(+), 21 deletions(-)
> 
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index e42d287..38d46d8 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -640,12 +640,51 @@ void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)
>      }
>  }
>  
> +static int el_from_spsr(uint32_t spsr)
> +{
> +    /* Return the exception level that this SPSR is requesting a return to,
> +     * or -1 if it is invalid (an illegal return)
> +     */
> +    if (spsr & PSTATE_nRW) {
> +        switch (spsr & CPSR_M) {
> +        case ARM_CPU_MODE_USR:
> +            return 0;
> +        case ARM_CPU_MODE_HYP:
> +            return 2;
> +        case ARM_CPU_MODE_FIQ:
> +        case ARM_CPU_MODE_IRQ:
> +        case ARM_CPU_MODE_SVC:
> +        case ARM_CPU_MODE_ABT:
> +        case ARM_CPU_MODE_UND:
> +        case ARM_CPU_MODE_SYS:
> +            return 1;
> +        case ARM_CPU_MODE_MON:
> +            /* Returning to Mon from AArch64 is never possible,
> +             * so this is an illegal return.
> +             */
> +        default:
> +            return -1;
> +        }
> +    } else {
> +        if (extract32(spsr, 1, 1)) {
> +            /* Return with reserved M[1] bit set */
> +            return -1;
> +        }
> +        if (extract32(spsr, 0, 4) == 1) {
> +            /* return to EL0 with M[0] bit set */
> +            return -1;
> +        }
> +        return extract32(spsr, 2, 2);
> +    }
> +}
> +
>  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;
> +    bool return_to_aa64 = (spsr & PSTATE_nRW) == 0;
>  
>      aarch64_save_sp(env, cur_el);
>  
> @@ -662,35 +701,34 @@ void HELPER(exception_return)(CPUARMState *env)
>          spsr &= ~PSTATE_SS;
>      }
>  
> -    if (spsr & PSTATE_nRW) {
> -        /* TODO: We currently assume EL1/2/3 are running in AArch64.  */
> +    new_el = el_from_spsr(spsr);
> +    if (new_el == -1) {
> +        goto illegal_return;
> +    }
> +    if (new_el > cur_el
> +        || (new_el == 2 && !arm_feature(env, ARM_FEATURE_EL2))) {
> +        /* Disallow return to an EL which is unimplemented or higher
> +         * than the current one.
> +         */
> +        goto illegal_return;
> +    }
> +
> +    if (new_el != 0 && arm_el_is_aa64(env, new_el) != return_to_aa64) {
> +        /* Return to an EL which is configured for a different register width */
> +        goto illegal_return;
> +    }
> +
> +    if (!return_to_aa64) {
>          env->aarch64 = 0;
> -        new_el = 0;
> -        env->uncached_cpsr = 0x10;
> +        env->uncached_cpsr = spsr & CPSR_M;
>          cpsr_write(env, spsr, ~0);
>          if (!arm_singlestep_active(env)) {
>              env->uncached_cpsr &= ~PSTATE_SS;
>          }
>          aarch64_sync_64_to_32(env);
>  
> -        env->regs[15] = env->elr_el[1] & ~0x1;
> +        env->regs[15] = env->elr_el[cur_el] & ~0x1;
>      } else {
> -        new_el = extract32(spsr, 2, 2);
> -        if (new_el > cur_el
> -            || (new_el == 2 && !arm_feature(env, ARM_FEATURE_EL2))) {
> -            /* Disallow return to an EL which is unimplemented or higher
> -             * than the current one.
> -             */
> -            goto illegal_return;
> -        }
> -        if (extract32(spsr, 1, 1)) {
> -            /* Return with reserved M[1] bit set */
> -            goto illegal_return;
> -        }
> -        if (new_el == 0 && (spsr & PSTATE_SP)) {
> -            /* Return to EL0 with M[0] bit set */
> -            goto illegal_return;
> -        }
>          env->aarch64 = 1;
>          pstate_write(env, spsr);
>          if (!arm_singlestep_active(env)) {
> -- 
> 1.9.1
> 

Re: [Qemu-devel] [PATCH 7/8] target-arm: Implement remaining illegal return event checks

[Edgar E. Iglesias]

On Thu, Jan 14, 2016 at 06:34:10PM +0000, Peter Maydell wrote:
> We already implement almost all the checks for the illegal
> return events from AArch64 state described in the ARM ARM section
> D1.11.2. Add the two missing ones:
>  * return to EL2 when EL3 is implemented and SCR_EL3.NS is 0
>  * return to Non-secure EL1 when EL2 is implemented and HCR_EL2.TGE is 1
> 
> (We don't implement external debug, so the case of "debug state exit
> from EL0 using AArch64 state to EL0 using AArch32 state" doesn't apply
> for QEMU.)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/op_helper.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 38d46d8..5789ccb 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -718,6 +718,17 @@ void HELPER(exception_return)(CPUARMState *env)
>          goto illegal_return;
>      }
>  
> +    if (new_el == 2 && arm_is_secure_below_el3(env)) {
> +        /* Return to the non-existent secure-EL2 */
> +        goto illegal_return;
> +    }
> +
> +    if (new_el == 1 &&
> +        arm_feature(env, ARM_FEATURE_EL2) && (env->cp15.hcr_el2 & HCR_TGE)

I'm not sure you need to check for EL2 as hcr_el2.tge cannot be reached/set otherwise.

Anyway:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>



> +        && !arm_is_secure_below_el3(env)) {
> +        goto illegal_return;
> +    }
> +
>      if (!return_to_aa64) {
>          env->aarch64 = 0;
>          env->uncached_cpsr = spsr & CPSR_M;
> -- 
> 1.9.1
> 

Re: [Qemu-devel] [PATCH 8/8] target-arm: ignore ELR_ELx[1] for exception return to 32-bit ARM mode

[Edgar E. Iglesias]

On Thu, Jan 14, 2016 at 06:34:11PM +0000, Peter Maydell wrote:
> The architecture requires that for an exception return to AArch32 the
> low bits of ELR_ELx are ignored when the PC is set from them:
>  * if returning to Thumb mode, ignore ELR_ELx[0]
>  * if returning to ARM mode, ignore ELR_ELx[1:0]
> 
> We were only squashing bit 0; also squash bit 1 if the SPSR T bit
> indicates this is a return to ARM code.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>



> ---
>  target-arm/op_helper.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 5789ccb..171d6b8 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -738,7 +738,11 @@ void HELPER(exception_return)(CPUARMState *env)
>          }
>          aarch64_sync_64_to_32(env);
>  
> -        env->regs[15] = env->elr_el[cur_el] & ~0x1;
> +        if (spsr & CPSR_T) {
> +            env->regs[15] = env->elr_el[cur_el] & ~0x1;
> +        } else {
> +            env->regs[15] = env->elr_el[cur_el] & ~0x3;
> +        }
>      } else {
>          env->aarch64 = 1;
>          pstate_write(env, spsr);
> -- 
> 1.9.1
> 

Re: [Qemu-devel] [PATCH 7/8] target-arm: Implement remaining illegal return event checks

[Peter Maydell]

On 19 January 2016 at 16:53, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Thu, Jan 14, 2016 at 06:34:10PM +0000, Peter Maydell wrote:
>> We already implement almost all the checks for the illegal
>> return events from AArch64 state described in the ARM ARM section
>> D1.11.2. Add the two missing ones:
>>  * return to EL2 when EL3 is implemented and SCR_EL3.NS is 0
>>  * return to Non-secure EL1 when EL2 is implemented and HCR_EL2.TGE is 1
>>
>> (We don't implement external debug, so the case of "debug state exit
>> from EL0 using AArch64 state to EL0 using AArch32 state" doesn't apply
>> for QEMU.)
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  target-arm/op_helper.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
>> index 38d46d8..5789ccb 100644
>> --- a/target-arm/op_helper.c
>> +++ b/target-arm/op_helper.c
>> @@ -718,6 +718,17 @@ void HELPER(exception_return)(CPUARMState *env)
>>          goto illegal_return;
>>      }
>>
>> +    if (new_el == 2 && arm_is_secure_below_el3(env)) {
>> +        /* Return to the non-existent secure-EL2 */
>> +        goto illegal_return;
>> +    }
>> +
>> +    if (new_el == 1 &&
>> +        arm_feature(env, ARM_FEATURE_EL2) && (env->cp15.hcr_el2 & HCR_TGE)
>
> I'm not sure you need to check for EL2 as hcr_el2.tge cannot be reached/set otherwise.

Mmm. I think we have existing code that takes both approaches here
but mostly we're going for the "no explicit EL2 feature check".
I'll take that bit of the condition out (but won't bother resending
the patchset unless there's something else that needs fixing).

> Anyway:
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/8] target-arm: Properly support EL2 and EL3 in arm_el_is_aa64()

[Sergey Fedorov]

On 14.01.2016 21:34, Peter Maydell wrote:
> Support EL2 and EL3 in arm_el_is_aa64() by implementing the
> logic for checking the SCR_EL3 and HCR_EL2 register-width bits
> as appropriate to determine the register width of lower exception
> levels.

Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>

>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.h | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 5f81342..b8b3364 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -969,18 +969,33 @@ static inline bool arm_is_secure(CPUARMState *env)
>  /* Return true if the specified exception level is running in AArch64 state. */
>  static inline bool arm_el_is_aa64(CPUARMState *env, int el)
>  {
> -    /* We don't currently support EL2, and this isn't valid for EL0
> -     * (if we're in EL0, is_a64() is what you want, and if we're not in EL0
> -     * then the state of EL0 isn't well defined.)
> +    /* This isn't valid for EL0 (if we're in EL0, is_a64() is what you want,
> +     * and if we're not in EL0 then the state of EL0 isn't well defined.)
>       */
> -    assert(el == 1 || el == 3);
> +    assert(el >= 1 && el <= 3);
> +    bool aa64 = arm_feature(env, ARM_FEATURE_AARCH64);
>  
> -    /* AArch64-capable CPUs always run with EL1 in AArch64 mode. This
> -     * is a QEMU-imposed simplification which we may wish to change later.
> -     * If we in future support EL2 and/or EL3, then the state of lower
> -     * exception levels is controlled by the HCR.RW and SCR.RW bits.
> +    /* The highest exception level is always at the maximum supported
> +     * register width, and then lower levels have a register width controlled
> +     * by bits in the SCR or HCR registers.
>       */
> -    return arm_feature(env, ARM_FEATURE_AARCH64);
> +    if (el == 3) {
> +        return aa64;
> +    }
> +
> +    if (arm_feature(env, ARM_FEATURE_EL3)) {
> +        aa64 = aa64 && (env->cp15.scr_el3 & SCR_RW);
> +    }
> +
> +    if (el == 2) {
> +        return aa64;
> +    }
> +
> +    if (arm_feature(env, ARM_FEATURE_EL2) && !arm_is_secure_below_el3(env)) {
> +        aa64 = aa64 && (env->cp15.hcr_el2 & HCR_RW);
> +    }
> +
> +    return aa64;
>  }
>  
>  /* Function for determing whether guest cp register reads and writes should

Re: [Qemu-devel] [PATCH 2/8] target-arm: Move aarch64_cpu_do_interrupt() to helper.c

[Sergey Fedorov]

On 14.01.2016 21:34, Peter Maydell wrote:
> Move the aarch64_cpu_do_interrupt() function to helper.c. We want
> to be able to call this from code that isn't AArch64-only, and
> the move allows us to avoid awkward #ifdeffery at the callsite.

Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu-qom.h    |   2 +-
>  target-arm/helper-a64.c | 104 ------------------------------------------------
>  target-arm/helper.c     | 100 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 101 insertions(+), 105 deletions(-)
>
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index e4d4270..bda2af8 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -247,8 +247,8 @@ void arm_gt_stimer_cb(void *opaque);
>  #ifdef TARGET_AARCH64
>  int aarch64_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
>  int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
> +#endif
>  
>  void aarch64_cpu_do_interrupt(CPUState *cs);
> -#endif
>  
>  #endif
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index fc3ccdf..a322e7b 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -25,7 +25,6 @@
>  #include "qemu/bitops.h"
>  #include "internals.h"
>  #include "qemu/crc32c.h"
> -#include "sysemu/kvm.h"
>  #include <zlib.h> /* For crc32 */
>  
>  /* C2.4.7 Multiply and divide */
> @@ -443,106 +442,3 @@ uint64_t HELPER(crc32c_64)(uint64_t acc, uint64_t val, uint32_t bytes)
>      /* Linux crc32c converts the output to one's complement.  */
>      return crc32c(acc, buf, bytes) ^ 0xffffffff;
>  }
> -
> -#if !defined(CONFIG_USER_ONLY)
> -
> -/* Handle a CPU exception.  */
> -void aarch64_cpu_do_interrupt(CPUState *cs)
> -{
> -    ARMCPU *cpu = ARM_CPU(cs);
> -    CPUARMState *env = &cpu->env;
> -    unsigned int new_el = env->exception.target_el;
> -    target_ulong addr = env->cp15.vbar_el[new_el];
> -    unsigned int new_mode = aarch64_pstate_mode(new_el, true);
> -
> -    if (arm_current_el(env) < new_el) {
> -        if (env->aarch64) {
> -            addr += 0x400;
> -        } else {
> -            addr += 0x600;
> -        }
> -    } else if (pstate_read(env) & PSTATE_SP) {
> -        addr += 0x200;
> -    }
> -
> -    arm_log_exception(cs->exception_index);
> -    qemu_log_mask(CPU_LOG_INT, "...from EL%d to EL%d\n", arm_current_el(env),
> -                  new_el);
> -    if (qemu_loglevel_mask(CPU_LOG_INT)
> -        && !excp_is_internal(cs->exception_index)) {
> -        qemu_log_mask(CPU_LOG_INT, "...with ESR %x/0x%" PRIx32 "\n",
> -                      env->exception.syndrome >> ARM_EL_EC_SHIFT,
> -                      env->exception.syndrome);
> -    }
> -
> -    if (arm_is_psci_call(cpu, cs->exception_index)) {
> -        arm_handle_psci_call(cpu);
> -        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
> -        return;
> -    }
> -
> -    switch (cs->exception_index) {
> -    case EXCP_PREFETCH_ABORT:
> -    case EXCP_DATA_ABORT:
> -        env->cp15.far_el[new_el] = env->exception.vaddress;
> -        qemu_log_mask(CPU_LOG_INT, "...with FAR 0x%" PRIx64 "\n",
> -                      env->cp15.far_el[new_el]);
> -        /* fall through */
> -    case EXCP_BKPT:
> -    case EXCP_UDEF:
> -    case EXCP_SWI:
> -    case EXCP_HVC:
> -    case EXCP_HYP_TRAP:
> -    case EXCP_SMC:
> -        env->cp15.esr_el[new_el] = env->exception.syndrome;
> -        break;
> -    case EXCP_IRQ:
> -    case EXCP_VIRQ:
> -        addr += 0x80;
> -        break;
> -    case EXCP_FIQ:
> -    case EXCP_VFIQ:
> -        addr += 0x100;
> -        break;
> -    case EXCP_SEMIHOST:
> -        qemu_log_mask(CPU_LOG_INT,
> -                      "...handling as semihosting call 0x%" PRIx64 "\n",
> -                      env->xregs[0]);
> -        env->xregs[0] = do_arm_semihosting(env);
> -        return;
> -    default:
> -        cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
> -    }
> -
> -    if (is_a64(env)) {
> -        env->banked_spsr[aarch64_banked_spsr_index(new_el)] = pstate_read(env);
> -        aarch64_save_sp(env, arm_current_el(env));
> -        env->elr_el[new_el] = env->pc;
> -    } else {
> -        env->banked_spsr[aarch64_banked_spsr_index(new_el)] = cpsr_read(env);
> -        if (!env->thumb) {
> -            env->cp15.esr_el[new_el] |= 1 << 25;
> -        }
> -        env->elr_el[new_el] = env->regs[15];
> -
> -        aarch64_sync_32_to_64(env);
> -
> -        env->condexec_bits = 0;
> -    }
> -    qemu_log_mask(CPU_LOG_INT, "...with ELR 0x%" PRIx64 "\n",
> -                  env->elr_el[new_el]);
> -
> -    pstate_write(env, PSTATE_DAIF | new_mode);
> -    env->aarch64 = 1;
> -    aarch64_restore_sp(env, new_el);
> -
> -    env->pc = addr;
> -
> -    qemu_log_mask(CPU_LOG_INT, "...to EL%d PC 0x%" PRIx64 " PSTATE 0x%x\n",
> -                  new_el, env->pc, pstate_read(env));
> -
> -    if (!kvm_enabled()) {
> -        cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
> -    }
> -}
> -#endif
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index a06bfaf..519f066 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -11,6 +11,7 @@
>  #include "arm_ldst.h"
>  #include <zlib.h> /* For crc32 */
>  #include "exec/semihost.h"
> +#include "sysemu/kvm.h"
>  
>  #define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */
>  
> @@ -5901,6 +5902,105 @@ void arm_cpu_do_interrupt(CPUState *cs)
>      cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
>  }
>  
> +/* Handle a CPU exception.  */
> +void aarch64_cpu_do_interrupt(CPUState *cs)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +    unsigned int new_el = env->exception.target_el;
> +    target_ulong addr = env->cp15.vbar_el[new_el];
> +    unsigned int new_mode = aarch64_pstate_mode(new_el, true);
> +
> +    if (arm_current_el(env) < new_el) {
> +        if (env->aarch64) {
> +            addr += 0x400;
> +        } else {
> +            addr += 0x600;
> +        }
> +    } else if (pstate_read(env) & PSTATE_SP) {
> +        addr += 0x200;
> +    }
> +
> +    arm_log_exception(cs->exception_index);
> +    qemu_log_mask(CPU_LOG_INT, "...from EL%d to EL%d\n", arm_current_el(env),
> +                  new_el);
> +    if (qemu_loglevel_mask(CPU_LOG_INT)
> +        && !excp_is_internal(cs->exception_index)) {
> +        qemu_log_mask(CPU_LOG_INT, "...with ESR %x/0x%" PRIx32 "\n",
> +                      env->exception.syndrome >> ARM_EL_EC_SHIFT,
> +                      env->exception.syndrome);
> +    }
> +
> +    if (arm_is_psci_call(cpu, cs->exception_index)) {
> +        arm_handle_psci_call(cpu);
> +        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
> +        return;
> +    }
> +
> +    switch (cs->exception_index) {
> +    case EXCP_PREFETCH_ABORT:
> +    case EXCP_DATA_ABORT:
> +        env->cp15.far_el[new_el] = env->exception.vaddress;
> +        qemu_log_mask(CPU_LOG_INT, "...with FAR 0x%" PRIx64 "\n",
> +                      env->cp15.far_el[new_el]);
> +        /* fall through */
> +    case EXCP_BKPT:
> +    case EXCP_UDEF:
> +    case EXCP_SWI:
> +    case EXCP_HVC:
> +    case EXCP_HYP_TRAP:
> +    case EXCP_SMC:
> +        env->cp15.esr_el[new_el] = env->exception.syndrome;
> +        break;
> +    case EXCP_IRQ:
> +    case EXCP_VIRQ:
> +        addr += 0x80;
> +        break;
> +    case EXCP_FIQ:
> +    case EXCP_VFIQ:
> +        addr += 0x100;
> +        break;
> +    case EXCP_SEMIHOST:
> +        qemu_log_mask(CPU_LOG_INT,
> +                      "...handling as semihosting call 0x%" PRIx64 "\n",
> +                      env->xregs[0]);
> +        env->xregs[0] = do_arm_semihosting(env);
> +        return;
> +    default:
> +        cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
> +    }
> +
> +    if (is_a64(env)) {
> +        env->banked_spsr[aarch64_banked_spsr_index(new_el)] = pstate_read(env);
> +        aarch64_save_sp(env, arm_current_el(env));
> +        env->elr_el[new_el] = env->pc;
> +    } else {
> +        env->banked_spsr[aarch64_banked_spsr_index(new_el)] = cpsr_read(env);
> +        if (!env->thumb) {
> +            env->cp15.esr_el[new_el] |= 1 << 25;
> +        }
> +        env->elr_el[new_el] = env->regs[15];
> +
> +        aarch64_sync_32_to_64(env);
> +
> +        env->condexec_bits = 0;
> +    }
> +    qemu_log_mask(CPU_LOG_INT, "...with ELR 0x%" PRIx64 "\n",
> +                  env->elr_el[new_el]);
> +
> +    pstate_write(env, PSTATE_DAIF | new_mode);
> +    env->aarch64 = 1;
> +    aarch64_restore_sp(env, new_el);
> +
> +    env->pc = addr;
> +
> +    qemu_log_mask(CPU_LOG_INT, "...to EL%d PC 0x%" PRIx64 " PSTATE 0x%x\n",
> +                  new_el, env->pc, pstate_read(env));
> +
> +    if (!kvm_enabled()) {
> +        cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
> +    }
> +}
>  
>  /* Return the exception level which controls this address translation regime */
>  static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)

Re: [Qemu-devel] [Qemu-arm] [PATCH 3/8] target-arm: Use a single entry point for AArch64 and AArch32 exceptions

[Sergey Fedorov]

On 14.01.2016 21:34, Peter Maydell wrote:
> If EL2 or EL3 is present on an AArch64 CPU, then exceptions can be
> taken to an exception level which is running AArch32 (if only EL0
> and EL1 are present then EL1 must be AArch64 and all exceptions are
> taken to AArch64). To support this we need to have a single
> implementation of the CPU do_interrupt() method which can handle both
> 32 and 64 bit exception entry.
>
> Pull the common parts of aarch64_cpu_do_interrupt() and
> arm_cpu_do_interrupt() out into a new function which calls
> either the AArch32 or AArch64 specific entry code once it has
> worked out which one is needed.
>
> We temporarily special-case the handling of EXCP_SEMIHOST to
> avoid an assertion in arm_el_is_aa64(); the next patch will
> pull all the semihosting handling out to the arm_cpu_do_interrupt()
> level (since semihosting semantics depend on the register width
> of the calling code, not on that of any higher EL).

Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu-qom.h |  2 --
>  target-arm/cpu64.c   |  3 ---
>  target-arm/helper.c  | 75 ++++++++++++++++++++++++++++++----------------------
>  3 files changed, 44 insertions(+), 36 deletions(-)
>
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index bda2af8..eae6cd1 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -249,6 +249,4 @@ int aarch64_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
>  int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>  #endif
>  
> -void aarch64_cpu_do_interrupt(CPUState *cs);
> -
>  #endif
> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
> index 63c8b1c..edb41f7 100644
> --- a/target-arm/cpu64.c
> +++ b/target-arm/cpu64.c
> @@ -290,9 +290,6 @@ static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
>  {
>      CPUClass *cc = CPU_CLASS(oc);
>  
> -#if !defined(CONFIG_USER_ONLY)
> -    cc->do_interrupt = aarch64_cpu_do_interrupt;
> -#endif
>      cc->cpu_exec_interrupt = arm_cpu_exec_interrupt;
>      cc->set_pc = aarch64_cpu_set_pc;
>      cc->gdb_read_register = aarch64_cpu_gdb_read_register;
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 519f066..962bb3c 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -5707,8 +5707,7 @@ void aarch64_sync_64_to_32(CPUARMState *env)
>      env->regs[15] = env->pc;
>  }
>  
> -/* Handle a CPU exception.  */
> -void arm_cpu_do_interrupt(CPUState *cs)
> +static void arm_cpu_do_interrupt_aarch32(CPUState *cs)
>  {
>      ARMCPU *cpu = ARM_CPU(cs);
>      CPUARMState *env = &cpu->env;
> @@ -5718,16 +5717,6 @@ void arm_cpu_do_interrupt(CPUState *cs)
>      uint32_t offset;
>      uint32_t moe;
>  
> -    assert(!IS_M(env));
> -
> -    arm_log_exception(cs->exception_index);
> -
> -    if (arm_is_psci_call(cpu, cs->exception_index)) {
> -        arm_handle_psci_call(cpu);
> -        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
> -        return;
> -    }
> -
>      /* If this is a debug exception we must update the DBGDSCR.MOE bits */
>      switch (env->exception.syndrome >> ARM_EL_EC_SHIFT) {
>      case EC_BREAKPOINT:
> @@ -5899,11 +5888,10 @@ void arm_cpu_do_interrupt(CPUState *cs)
>      }
>      env->regs[14] = env->regs[15] + offset;
>      env->regs[15] = addr;
> -    cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
>  }
>  
> -/* Handle a CPU exception.  */
> -void aarch64_cpu_do_interrupt(CPUState *cs)
> +/* Handle exception entry to a target EL which is using AArch64 */
> +static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
>  {
>      ARMCPU *cpu = ARM_CPU(cs);
>      CPUARMState *env = &cpu->env;
> @@ -5921,22 +5909,6 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>          addr += 0x200;
>      }
>  
> -    arm_log_exception(cs->exception_index);
> -    qemu_log_mask(CPU_LOG_INT, "...from EL%d to EL%d\n", arm_current_el(env),
> -                  new_el);
> -    if (qemu_loglevel_mask(CPU_LOG_INT)
> -        && !excp_is_internal(cs->exception_index)) {
> -        qemu_log_mask(CPU_LOG_INT, "...with ESR %x/0x%" PRIx32 "\n",
> -                      env->exception.syndrome >> ARM_EL_EC_SHIFT,
> -                      env->exception.syndrome);
> -    }
> -
> -    if (arm_is_psci_call(cpu, cs->exception_index)) {
> -        arm_handle_psci_call(cpu);
> -        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
> -        return;
> -    }
> -
>      switch (cs->exception_index) {
>      case EXCP_PREFETCH_ABORT:
>      case EXCP_DATA_ABORT:
> @@ -5996,6 +5968,47 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>  
>      qemu_log_mask(CPU_LOG_INT, "...to EL%d PC 0x%" PRIx64 " PSTATE 0x%x\n",
>                    new_el, env->pc, pstate_read(env));
> +}
> +
> +/* Handle a CPU exception for A and R profile CPUs.
> + * Do any appropriate logging, handle PSCI calls, and then hand off
> + * to the AArch64-entry or AArch32-entry function depending on the
> + * target exception level's register width.
> + */
> +void arm_cpu_do_interrupt(CPUState *cs)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +    unsigned int new_el = env->exception.target_el;
> +
> +    assert(!IS_M(env));
> +
> +    arm_log_exception(cs->exception_index);
> +    qemu_log_mask(CPU_LOG_INT, "...from EL%d to EL%d\n", arm_current_el(env),
> +                  new_el);
> +    if (qemu_loglevel_mask(CPU_LOG_INT)
> +        && !excp_is_internal(cs->exception_index)) {
> +        qemu_log_mask(CPU_LOG_INT, "...with ESR %x/0x%" PRIx32 "\n",
> +                      env->exception.syndrome >> ARM_EL_EC_SHIFT,
> +                      env->exception.syndrome);
> +    }
> +
> +    if (arm_is_psci_call(cpu, cs->exception_index)) {
> +        arm_handle_psci_call(cpu);
> +        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
> +        return;
> +    }
> +
> +    /* Temporary special case for EXCP_SEMIHOST, which is used only
> +     * for 64-bit semihosting calls -- as this is an internal exception
> +     * it has no specified target level and arm_el_is_aa64() would
> +     * assert because new_el could be 0.
> +     */
> +    if (cs->exception_index == EXCP_SEMIHOST || arm_el_is_aa64(env, new_el)) {
> +        arm_cpu_do_interrupt_aarch64(cs);
> +    } else {
> +        arm_cpu_do_interrupt_aarch32(cs);
> +    }
>  
>      if (!kvm_enabled()) {
>          cs->interrupt_request |= CPU_INTERRUPT_EXITTB;

Re: [Qemu-devel] [PATCH 4/8] target-arm: Pull semihosting handling out to arm_cpu_do_interrupt()

[Sergey Fedorov]

On 14.01.2016 21:34, Peter Maydell wrote:
> Handling of semihosting calls should depend on the register width
> of the calling code, not on that of any higher exception level,
> so we need to identify and handle semihosting calls before we
> decide whether to deliver the exception as an entry to AArch32
> or AArch64. (EXCP_SEMIHOST is also an "internal exception" so
> it has no target exception level in the first place.)
>
> This will allow AArch32 EL1 code to use semihosting calls when
> running under an AArch64 EL3.

Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/helper.c | 120 +++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 81 insertions(+), 39 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 962bb3c..d37c82c 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -5754,27 +5754,6 @@ static void arm_cpu_do_interrupt_aarch32(CPUState *cs)
>              offset = 4;
>          break;
>      case EXCP_SWI:
> -        if (semihosting_enabled()) {
> -            /* Check for semihosting interrupt.  */
> -            if (env->thumb) {
> -                mask = arm_lduw_code(env, env->regs[15] - 2, env->bswap_code)
> -                    & 0xff;
> -            } else {
> -                mask = arm_ldl_code(env, env->regs[15] - 4, env->bswap_code)
> -                    & 0xffffff;
> -            }
> -            /* Only intercept calls from privileged modes, to provide some
> -               semblance of security.  */
> -            if (((mask == 0x123456 && !env->thumb)
> -                    || (mask == 0xab && env->thumb))
> -                  && (env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR) {
> -                qemu_log_mask(CPU_LOG_INT,
> -                              "...handling as semihosting call 0x%x\n",
> -                              env->regs[0]);
> -                env->regs[0] = do_arm_semihosting(env);
> -                return;
> -            }
> -        }
>          new_mode = ARM_CPU_MODE_SVC;
>          addr = 0x08;
>          mask = CPSR_I;
> @@ -5782,19 +5761,6 @@ static void arm_cpu_do_interrupt_aarch32(CPUState *cs)
>          offset = 0;
>          break;
>      case EXCP_BKPT:
> -        /* See if this is a semihosting syscall.  */
> -        if (env->thumb && semihosting_enabled()) {
> -            mask = arm_lduw_code(env, env->regs[15], env->bswap_code) & 0xff;
> -            if (mask == 0xab
> -                  && (env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR) {
> -                env->regs[15] += 2;
> -                qemu_log_mask(CPU_LOG_INT,
> -                              "...handling as semihosting call 0x%x\n",
> -                              env->regs[0]);
> -                env->regs[0] = do_arm_semihosting(env);
> -                return;
> -            }
> -        }
>          env->exception.fsr = 2;
>          /* Fall through to prefetch abort.  */
>      case EXCP_PREFETCH_ABORT:
> @@ -5970,6 +5936,78 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
>                    new_el, env->pc, pstate_read(env));
>  }
>  
> +static inline bool check_for_semihosting(CPUState *cs)
> +{
> +    /* Check whether this exception is a semihosting call; if so
> +     * then handle it and return true; otherwise return false.
> +     */
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +
> +    if (is_a64(env)) {
> +        if (cs->exception_index == EXCP_SEMIHOST) {
> +            /* This is always the 64-bit semihosting exception.
> +             * The "is this usermode" and "is semihosting enabled"
> +             * checks have been done at translate time.
> +             */
> +            qemu_log_mask(CPU_LOG_INT,
> +                          "...handling as semihosting call 0x%" PRIx64 "\n",
> +                          env->xregs[0]);
> +            env->xregs[0] = do_arm_semihosting(env);
> +            return true;
> +        }
> +        return false;
> +    } else {
> +        uint32_t imm;
> +
> +        /* Only intercept calls from privileged modes, to provide some
> +         * semblance of security.
> +         */
> +        if (!semihosting_enabled() ||
> +            ((env->uncached_cpsr & CPSR_M) == ARM_CPU_MODE_USR)) {
> +            return false;
> +        }
> +
> +        switch (cs->exception_index) {
> +        case EXCP_SWI:
> +            /* Check for semihosting interrupt.  */
> +            if (env->thumb) {
> +                imm = arm_lduw_code(env, env->regs[15] - 2, env->bswap_code)
> +                    & 0xff;
> +                if (imm == 0xab) {
> +                    break;
> +                }
> +            } else {
> +                imm = arm_ldl_code(env, env->regs[15] - 4, env->bswap_code)
> +                    & 0xffffff;
> +                if (imm == 0x123456) {
> +                    break;
> +                }
> +            }
> +            return false;
> +        case EXCP_BKPT:
> +            /* See if this is a semihosting syscall.  */
> +            if (env->thumb) {
> +                imm = arm_lduw_code(env, env->regs[15], env->bswap_code)
> +                    & 0xff;
> +                if (imm == 0xab) {
> +                    env->regs[15] += 2;
> +                    break;
> +                }
> +            }
> +            return false;
> +        default:
> +            return false;
> +        }
> +
> +        qemu_log_mask(CPU_LOG_INT,
> +                      "...handling as semihosting call 0x%x\n",
> +                      env->regs[0]);
> +        env->regs[0] = do_arm_semihosting(env);
> +        return true;
> +    }
> +}
> +
>  /* Handle a CPU exception for A and R profile CPUs.
>   * Do any appropriate logging, handle PSCI calls, and then hand off
>   * to the AArch64-entry or AArch32-entry function depending on the
> @@ -5999,12 +6037,16 @@ void arm_cpu_do_interrupt(CPUState *cs)
>          return;
>      }
>  
> -    /* Temporary special case for EXCP_SEMIHOST, which is used only
> -     * for 64-bit semihosting calls -- as this is an internal exception
> -     * it has no specified target level and arm_el_is_aa64() would
> -     * assert because new_el could be 0.
> +    /* Semihosting semantics depend on the register width of the
> +     * code that caused the exception, not the target exception level,
> +     * so must be handled here.
>       */
> -    if (cs->exception_index == EXCP_SEMIHOST || arm_el_is_aa64(env, new_el)) {
> +    if (check_for_semihosting(cs)) {
> +        return;
> +    }
> +
> +    assert(!excp_is_internal(cs->exception_index));
> +    if (arm_el_is_aa64(env, new_el)) {
>          arm_cpu_do_interrupt_aarch64(cs);
>      } else {
>          arm_cpu_do_interrupt_aarch32(cs);

Re: [Qemu-devel] [PATCH 5/8] target-arm: Fix wrong AArch64 entry offset for EL2/EL3 target

[Sergey Fedorov]

On 14.01.2016 21:34, Peter Maydell wrote:
> The entry offset when taking an exception to AArch64 from a lower
> exception level may be 0x400 or 0x600. 0x400 is used if the
> implemented exception level immediately lower than the target level
> is using AArch64, and 0x600 if it is using AArch32. We were
> incorrectly implementing this as checking the exception level
> that the exception was taken from. (The two can be different if
> for example we take an exception from EL0 to AArch64 EL3; we should
> in this case be checking EL2 if EL2 is implemented, and EL1 if
> EL2 is not implemented.)

Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/helper.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index d37c82c..196c111 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -5866,7 +5866,26 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
>      unsigned int new_mode = aarch64_pstate_mode(new_el, true);
>  
>      if (arm_current_el(env) < new_el) {
> -        if (env->aarch64) {
> +        /* Entry vector offset depends on whether the implemented EL
> +         * immediately lower than the target level is using AArch32 or AArch64
> +         */
> +        bool is_aa64;
> +
> +        switch (new_el) {
> +        case 3:
> +            is_aa64 = (env->cp15.scr_el3 & SCR_RW) != 0;
> +            break;
> +        case 2:
> +            is_aa64 = (env->cp15.hcr_el2 & HCR_RW) != 0;
> +            break;
> +        case 1:
> +            is_aa64 = is_a64(env);
> +            break;
> +        default:
> +            g_assert_not_reached();
> +        }
> +
> +        if (is_aa64) {
>              addr += 0x400;
>          } else {
>              addr += 0x600;

Re: [Qemu-devel] [Qemu-arm] [PATCH 6/8] target-arm: Handle exception return from AArch64 to non-EL0 AArch32

[Sergey Fedorov]

On 14.01.2016 21:34, Peter Maydell wrote:
> Remove the assumptions that the AArch64 exception return code was
> making about a return to AArch32 always being a return to EL0.
> This includes pulling out the illegal-SPSR checks so we can apply
> them for return to 32 bit as well as return to 64-bit.

Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/op_helper.c | 80 +++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 59 insertions(+), 21 deletions(-)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index e42d287..38d46d8 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -640,12 +640,51 @@ void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)
>      }
>  }
>  
> +static int el_from_spsr(uint32_t spsr)
> +{
> +    /* Return the exception level that this SPSR is requesting a return to,
> +     * or -1 if it is invalid (an illegal return)
> +     */
> +    if (spsr & PSTATE_nRW) {
> +        switch (spsr & CPSR_M) {
> +        case ARM_CPU_MODE_USR:
> +            return 0;
> +        case ARM_CPU_MODE_HYP:
> +            return 2;
> +        case ARM_CPU_MODE_FIQ:
> +        case ARM_CPU_MODE_IRQ:
> +        case ARM_CPU_MODE_SVC:
> +        case ARM_CPU_MODE_ABT:
> +        case ARM_CPU_MODE_UND:
> +        case ARM_CPU_MODE_SYS:
> +            return 1;
> +        case ARM_CPU_MODE_MON:
> +            /* Returning to Mon from AArch64 is never possible,
> +             * so this is an illegal return.
> +             */
> +        default:
> +            return -1;
> +        }
> +    } else {
> +        if (extract32(spsr, 1, 1)) {
> +            /* Return with reserved M[1] bit set */
> +            return -1;
> +        }
> +        if (extract32(spsr, 0, 4) == 1) {
> +            /* return to EL0 with M[0] bit set */
> +            return -1;
> +        }
> +        return extract32(spsr, 2, 2);
> +    }
> +}
> +
>  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;
> +    bool return_to_aa64 = (spsr & PSTATE_nRW) == 0;
>  
>      aarch64_save_sp(env, cur_el);
>  
> @@ -662,35 +701,34 @@ void HELPER(exception_return)(CPUARMState *env)
>          spsr &= ~PSTATE_SS;
>      }
>  
> -    if (spsr & PSTATE_nRW) {
> -        /* TODO: We currently assume EL1/2/3 are running in AArch64.  */
> +    new_el = el_from_spsr(spsr);
> +    if (new_el == -1) {
> +        goto illegal_return;
> +    }
> +    if (new_el > cur_el
> +        || (new_el == 2 && !arm_feature(env, ARM_FEATURE_EL2))) {
> +        /* Disallow return to an EL which is unimplemented or higher
> +         * than the current one.
> +         */
> +        goto illegal_return;
> +    }
> +
> +    if (new_el != 0 && arm_el_is_aa64(env, new_el) != return_to_aa64) {
> +        /* Return to an EL which is configured for a different register width */
> +        goto illegal_return;
> +    }
> +
> +    if (!return_to_aa64) {
>          env->aarch64 = 0;
> -        new_el = 0;
> -        env->uncached_cpsr = 0x10;
> +        env->uncached_cpsr = spsr & CPSR_M;
>          cpsr_write(env, spsr, ~0);
>          if (!arm_singlestep_active(env)) {
>              env->uncached_cpsr &= ~PSTATE_SS;
>          }
>          aarch64_sync_64_to_32(env);
>  
> -        env->regs[15] = env->elr_el[1] & ~0x1;
> +        env->regs[15] = env->elr_el[cur_el] & ~0x1;
>      } else {
> -        new_el = extract32(spsr, 2, 2);
> -        if (new_el > cur_el
> -            || (new_el == 2 && !arm_feature(env, ARM_FEATURE_EL2))) {
> -            /* Disallow return to an EL which is unimplemented or higher
> -             * than the current one.
> -             */
> -            goto illegal_return;
> -        }
> -        if (extract32(spsr, 1, 1)) {
> -            /* Return with reserved M[1] bit set */
> -            goto illegal_return;
> -        }
> -        if (new_el == 0 && (spsr & PSTATE_SP)) {
> -            /* Return to EL0 with M[0] bit set */
> -            goto illegal_return;
> -        }
>          env->aarch64 = 1;
>          pstate_write(env, spsr);
>          if (!arm_singlestep_active(env)) {

Re: [Qemu-devel] [PATCH 7/8] target-arm: Implement remaining illegal return event checks

[Sergey Fedorov]

On 14.01.2016 21:34, Peter Maydell wrote:
> We already implement almost all the checks for the illegal
> return events from AArch64 state described in the ARM ARM section
> D1.11.2. Add the two missing ones:
>  * return to EL2 when EL3 is implemented and SCR_EL3.NS is 0
>  * return to Non-secure EL1 when EL2 is implemented and HCR_EL2.TGE is 1
>
> (We don't implement external debug, so the case of "debug state exit
> from EL0 using AArch64 state to EL0 using AArch32 state" doesn't apply
> for QEMU.)

Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/op_helper.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 38d46d8..5789ccb 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -718,6 +718,17 @@ void HELPER(exception_return)(CPUARMState *env)
>          goto illegal_return;
>      }
>  
> +    if (new_el == 2 && arm_is_secure_below_el3(env)) {
> +        /* Return to the non-existent secure-EL2 */
> +        goto illegal_return;
> +    }
> +
> +    if (new_el == 1 &&
> +        arm_feature(env, ARM_FEATURE_EL2) && (env->cp15.hcr_el2 & HCR_TGE)
> +        && !arm_is_secure_below_el3(env)) {
> +        goto illegal_return;
> +    }
> +
>      if (!return_to_aa64) {
>          env->aarch64 = 0;
>          env->uncached_cpsr = spsr & CPSR_M;

Re: [Qemu-devel] [Qemu-arm] [PATCH 8/8] target-arm: ignore ELR_ELx[1] for exception return to 32-bit ARM mode

[Sergey Fedorov]

On 14.01.2016 21:34, Peter Maydell wrote:
> The architecture requires that for an exception return to AArch32 the
> low bits of ELR_ELx are ignored when the PC is set from them:
>  * if returning to Thumb mode, ignore ELR_ELx[0]
>  * if returning to ARM mode, ignore ELR_ELx[1:0]
>
> We were only squashing bit 0; also squash bit 1 if the SPSR T bit
> indicates this is a return to ARM code.

Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/op_helper.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 5789ccb..171d6b8 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -738,7 +738,11 @@ void HELPER(exception_return)(CPUARMState *env)
>          }
>          aarch64_sync_64_to_32(env);
>  
> -        env->regs[15] = env->elr_el[cur_el] & ~0x1;
> +        if (spsr & CPSR_T) {
> +            env->regs[15] = env->elr_el[cur_el] & ~0x1;
> +        } else {
> +            env->regs[15] = env->elr_el[cur_el] & ~0x3;
> +        }
>      } else {
>          env->aarch64 = 1;
>          pstate_write(env, spsr);

Re: [Qemu-devel] [PATCH 1/8] target-arm: Properly support EL2 and EL3 in arm_el_is_aa64()

[Sergey Fedorov]

On 29.01.2016 19:45, Sergey Fedorov wrote:
> On 14.01.2016 21:34, Peter Maydell wrote:
>> > Support EL2 and EL3 in arm_el_is_aa64() by implementing the
>> > logic for checking the SCR_EL3 and HCR_EL2 register-width bits
>> > as appropriate to determine the register width of lower exception
>> > levels.
> Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>

Oops... I should put this below the following "Signed-off-by" statement :)

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

Re: [Qemu-devel] [PATCH 1/8] target-arm: Properly support EL2 and EL3 in arm_el_is_aa64()

[Peter Maydell]

On 29 January 2016 at 16:45, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
> On 14.01.2016 21:34, Peter Maydell wrote:
>> Support EL2 and EL3 in arm_el_is_aa64() by implementing the
>> logic for checking the SCR_EL3 and HCR_EL2 register-width bits
>> as appropriate to determine the register width of lower exception
>> levels.
>
> Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>

Thanks for the review, but this series went into master last week :-)

-- PMM

Re: [Qemu-devel] [PATCH 1/8] target-arm: Properly support EL2 and EL3 in arm_el_is_aa64()

[Sergey Fedorov]

On 29.01.2016 20:05, Peter Maydell wrote:
> On 29 January 2016 at 16:45, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
>> > On 14.01.2016 21:34, Peter Maydell wrote:
>>> >> Support EL2 and EL3 in arm_el_is_aa64() by implementing the
>>> >> logic for checking the SCR_EL3 and HCR_EL2 register-width bits
>>> >> as appropriate to determine the register width of lower exception
>>> >> levels.
>> >
>> > Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>
> Thanks for the review, but this series went into master last week :-)

Heh, I missed that somehow :) Anyway, great patches!