[Qemu-devel] [PATCH v2 2/2] target-arm: Fix arm_el_is_aa64() function to support EL2 and EL3

[Sergey Sorokin]

Function arm_el_is_aa64() was fixed to support EL2 and EL3.
It is needed for a future support of EL2 and/or EL3,
and 32 bit EL1/EL2 support for ARMv8 cpu.
ARM_FEATURE_AARCH64 flag means that the highest exception level is
in AArch64 state. The state of lower exception levels is controlled
by the HCR_EL2.RW, SCR_EL3.RW and SCR_EL3.NS bits.
If EL2 or EL3 is not permitted by the appropriate ARM_FEATURE flag,
then the function arm_el_is_aa64() aborts on an attempt to get
the bitness of this EL. So we need to be sure that EL passed to
the function is enabled. Therefore some conditional statements was
slightly changed to check it.

Signed-off-by: Sergey Sorokin <address@hidden>
---
Changes since previous version:
* Some typos was fixed.
* Extended comments was added.
* The initial patch was divided in two parts.
* The erroneous changes in arm_excp_unmasked() function was fixed.

 hw/arm/boot.c        |  4 ++++
 target-arm/cpu.c     | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/cpu.h     | 51 ++++++++++++++++++++++++++++++++-------------
 target-arm/helper.c  | 23 +++++++++++++++++++-
 target-arm/machine.c |  5 +++++
 5 files changed, 126 insertions(+), 16 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 5b969cd..e5d00af 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -486,6 +486,10 @@ static void do_cpu_reset(void *opaque)
                 if (!info->secure_boot) {
                     /* Linux expects non-secure state */
                     env->cp15.scr_el3 |= SCR_NS;
+                    /* EL bitness can depend on SCR.NS bit, so we need
+                     * recalc it.
+                     */
+                    calc_el_bitness(env);
                 }
             }
 
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index cc6c6f3..af4beab 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -100,6 +100,64 @@ static void cp_reg_check_reset(gpointer key, gpointer 
value,  gpointer opaque)
     assert(oldvalue == newvalue);
 }
 
+/* This function must be called if SCR_EL3.RW, SCR_EL3.NS
+ * or HCR_EL2.RW has been changed.
+ */
+void calc_el_bitness(CPUARMState *env)
+{
+    /* -1 is invalid value */
+    env->el_is_aa64[0] = -1;
+
+    /* If the highest EL is not AArch64, then all ELs are AArch32. */
+    if (!arm_feature(env, ARM_FEATURE_AARCH64)) {
+        if (arm_feature(env, ARM_FEATURE_EL3)) {
+            env->el_is_aa64[3] = 0;
+        } else {
+            env->el_is_aa64[3] = -1;
+        }
+
+        if (arm_feature(env, ARM_FEATURE_EL2)) {
+            env->el_is_aa64[2] = 0;
+        } else {
+            env->el_is_aa64[2] = -1;
+        }
+
+        env->el_is_aa64[1] = 0;
+        return;
+    }
+
+    if (arm_feature(env, ARM_FEATURE_EL3)) {
+        env->el_is_aa64[3] = 1;
+        int8_t scr_rw = env->cp15.scr_el3 & SCR_RW ? 1 : 0;
+
+        if (arm_feature(env, ARM_FEATURE_EL2) && (env->cp15.scr_el3 & SCR_NS)) 
{
+            if (scr_rw) {
+                env->el_is_aa64[2] = 1;
+                env->el_is_aa64[1] = env->cp15.hcr_el2 & HCR_RW ? 1 : 0;
+            } else {
+                env->el_is_aa64[2] = 0;
+                env->el_is_aa64[1] = 0;
+            }
+        } else {
+            /* If there is no EL2 or if secure. */
+            env->el_is_aa64[2] = -1;
+            env->el_is_aa64[1] = scr_rw;
+        }
+    } else {
+        env->el_is_aa64[3] = -1;
+
+        if (arm_feature(env, ARM_FEATURE_EL2)) {
+            /* The highest EL is EL2, and it is AArch64. */
+            env->el_is_aa64[2] = 1;
+            env->el_is_aa64[1] = env->cp15.hcr_el2 & HCR_RW ? 1 : 0;
+        } else {
+            /* The highest EL is EL1, and it is AArch64. */
+            env->el_is_aa64[2] = -1;
+            env->el_is_aa64[1] = 1;
+        }
+    }
+}
+
 /* CPUClass::reset() */
 static void arm_cpu_reset(CPUState *s)
 {
@@ -151,6 +209,7 @@ static void arm_cpu_reset(CPUState *s)
         env->cp15.cpacr_el1 = deposit64(env->cp15.cpacr_el1, 20, 4, 0xf);
 #endif
     }
+    calc_el_bitness(env);
 
 #if defined(CONFIG_USER_ONLY)
     env->uncached_cpsr = ARM_CPU_MODE_USR;
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 31825d3..222ad3d 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -177,6 +177,8 @@ typedef struct CPUARMState {
     uint64_t elr_el[4]; /* AArch64 exception link regs  */
     uint64_t sp_el[4]; /* AArch64 banked stack pointers */
 
+    int8_t el_is_aa64[4]; /* Cached flags describing the EL bitness */
+
     /* System control coprocessor (cp15) */
     struct {
         uint32_t c0_cpuid;
@@ -924,7 +926,7 @@ static inline bool arm_is_secure_below_el3(CPUARMState *env)
     if (arm_feature(env, ARM_FEATURE_EL3)) {
         return !(env->cp15.scr_el3 & SCR_NS);
     } else {
-        /* If EL2 is not supported then the secure state is implementation
+        /* If EL3 is not supported then the secure state is implementation
          * defined, in which case QEMU defaults to non-secure.
          */
         return false;
@@ -959,21 +961,38 @@ static inline bool arm_is_secure(CPUARMState *env)
 }
 #endif
 
-/* Return true if the specified exception level is running in AArch64 state. */
+/**
+ * calc_el_bitness:
+ * @env: The CPU where we need to recalculate the cached EL bitness.
+ *
+ * Calculates the bitness for each enabled EL and caches the values.
+ * The value -1 means the EL is not enabled in current configuration.
+ *
+ * This function must be called if SCR_EL3.RW, SCR_EL3.NS
+ * or HCR_EL2.RW has been changed.
+ */
+void calc_el_bitness(CPUARMState *env);
+
+/**
+ * arm_el_is_aa64:
+ * @env: The CPU where we want to know EL bitness.
+ * @el: The EL whose bitness we want to know, except EL0.
+ *
+ * Note: This function aborts if @el is not enabled in current configuration,
+ * or if @el == 0.
+ *
+ * Returns: %true if the specified exception level is running in AArch64 state;
+ * %false otherwise.
+ */
 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);
+    assert(env->el_is_aa64[el] >= 0);
 
-    /* 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.
-     */
-    return arm_feature(env, ARM_FEATURE_AARCH64);
+    return env->el_is_aa64[el];
 }
 
 /* Function for determing whether guest cp register reads and writes should
@@ -1012,11 +1031,11 @@ static inline bool access_secure_reg(CPUARMState *env)
  */
 #define A32_BANKED_CURRENT_REG_GET(_env, _regname)        \
     A32_BANKED_REG_GET((_env), _regname,                \
-                       ((!arm_el_is_aa64((_env), 3) && arm_is_secure(_env))))
+                       (arm_is_secure(_env) && !arm_el_is_aa64((_env), 3)))
 
 #define A32_BANKED_CURRENT_REG_SET(_env, _regname, _val)                       
\
     A32_BANKED_REG_SET((_env), _regname,                                    \
-                       ((!arm_el_is_aa64((_env), 3) && arm_is_secure(_env))),  
\
+                       (arm_is_secure(_env) && !arm_el_is_aa64((_env), 3)), \
                        (_val))
 
 void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf);
@@ -1583,7 +1602,9 @@ static inline bool arm_excp_unmasked(CPUState *cs, 
unsigned int excp_idx,
      * interrupt.
      */
     if ((target_el > cur_el) && (target_el != 1)) {
-        if (arm_el_is_aa64(env, 3) || ((scr || hcr) && (!secure))) {
+        /* ARM_FEATURE_AARCH64 enabled means the higher EL is AArch64. */
+        if (arm_feature(env, ARM_FEATURE_AARCH64) ||
+            ((scr || hcr) && (!secure))) {
             unmasked = 1;
         }
     }
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 7df1f06..4a1c715 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -838,6 +838,7 @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo 
*ri, uint64_t value)
      * current execution mode before directly using the feature bit.
      */
     uint32_t valid_mask = SCR_AARCH64_MASK | SCR_AARCH32_MASK;
+    uint64_t old_value;
 
     if (!arm_feature(env, ARM_FEATURE_EL2)) {
         valid_mask &= ~SCR_HCE;
@@ -856,7 +857,14 @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo 
*ri, uint64_t value)
 
     /* Clear all-context RES0 bits.  */
     value &= valid_mask;
+    old_value = raw_read(env, ri);
     raw_write(env, ri, value);
+
+    /* We need to recalculate EL bitness if SCR_EL3.RW, SCR_EL3.NS
+     * or HCR_EL2.RW has been changed. */
+    if ((old_value ^ value) & (SCR_RW | SCR_NS)) {
+        calc_el_bitness(env);
+    }
 }
 
 static uint64_t ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
@@ -3146,6 +3154,7 @@ static void hcr_write(CPUARMState *env, const 
ARMCPRegInfo *ri, uint64_t value)
 {
     ARMCPU *cpu = arm_env_get_cpu(env);
     uint64_t valid_mask = HCR_MASK;
+    uint64_t old_value;
 
     if (arm_feature(env, ARM_FEATURE_EL3)) {
         valid_mask &= ~HCR_HCD;
@@ -3164,7 +3173,14 @@ static void hcr_write(CPUARMState *env, const 
ARMCPRegInfo *ri, uint64_t value)
     if ((raw_read(env, ri) ^ value) & (HCR_VM | HCR_PTW | HCR_DC)) {
         tlb_flush(CPU(cpu), 1);
     }
+    old_value = raw_read(env, ri);
     raw_write(env, ri, value);
+
+    /* We need to recalculate EL bitness if SCR_EL3.RW, SCR_EL3.NS
+     * or HCR_EL2.RW has been changed. */
+    if ((old_value ^ value) & HCR_RW) {
+        calc_el_bitness(env);
+    }
 }
 
 static const ARMCPRegInfo el2_cp_reginfo[] = {
@@ -5088,7 +5104,8 @@ uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t 
excp_idx,
     int scr;
     int hcr;
     int target_el;
-    int is64 = arm_el_is_aa64(env, 3);
+    /* Is the higher EL AArch64? */
+    int is64 = arm_feature(env, ARM_FEATURE_AARCH64);
 
     switch (excp_idx) {
     case EXCP_IRQ:
@@ -5645,6 +5662,10 @@ void arm_cpu_do_interrupt(CPUState *cs)
 
     if ((env->uncached_cpsr & CPSR_M) == ARM_CPU_MODE_MON) {
         env->cp15.scr_el3 &= ~SCR_NS;
+        /* SCR.NS bit is changed, but we don't need to call calc_el_bitness()
+         * here, because the transition to AArch32 EL3 supposes that
+         * all lower ELs are always 32-bit.
+         */
     }
 
     switch_mode (env, new_mode);
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 32adfe7..e5bfb59 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -265,6 +265,11 @@ static int cpu_post_load(void *opaque, int version_id)
         }
     }
 
+    /* It is not nesessary, because this call was made on
+     * HCR and SCR writes above, but just to make sure.
+     */
+    calc_el_bitness(&cpu->env);
+
     hw_breakpoint_update_all(cpu);
     hw_watchpoint_update_all(cpu);
 
-- 
1.9.3