All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 01/14] target/arm: add arm_is_el2_enabled() helper
       [not found] <2172054.ElGaqSPkdT@basile.remlab.net>
@ 2020-11-02 10:57 ` remi.denis.courmont
  2020-11-02 11:06   ` Peter Maydell
  2020-11-03 16:42   ` Richard Henderson
  2020-11-02 10:57 ` [PATCH 02/14] target/arm: use arm_is_el2_enabled() where applicable remi.denis.courmont
                   ` (12 subsequent siblings)
  13 siblings, 2 replies; 35+ messages in thread
From: remi.denis.courmont @ 2020-11-02 10:57 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

This checks if EL2 is enabled (meaning EL2 registers take effects) in
the current security context.

Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/cpu.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index c18a916766..aaf3671806 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2040,6 +2040,15 @@ static inline bool arm_is_secure(CPUARMState *env)
     return arm_is_secure_below_el3(env);
 }
 
+/* Return true if the current security state has AArch64 EL2 or AArch32 Hyp */
+static inline bool arm_is_el2_enabled(CPUARMState *env)
+{
+    if (arm_feature(env, ARM_FEATURE_EL2)) {
+        return !arm_is_secure_below_el3(env);
+    }
+    return false;
+}
+
 #else
 static inline bool arm_is_secure_below_el3(CPUARMState *env)
 {
@@ -2050,6 +2059,11 @@ static inline bool arm_is_secure(CPUARMState *env)
 {
     return false;
 }
+
+static inline bool arm_is_el2_enabled(CPUARMState *env)
+{
+    return false;
+}
 #endif
 
 /**
-- 
2.29.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 02/14] target/arm: use arm_is_el2_enabled() where applicable
       [not found] <2172054.ElGaqSPkdT@basile.remlab.net>
  2020-11-02 10:57 ` [PATCH 01/14] target/arm: add arm_is_el2_enabled() helper remi.denis.courmont
@ 2020-11-02 10:57 ` remi.denis.courmont
  2020-11-03 16:53   ` Richard Henderson
  2020-11-02 10:57 ` [PATCH 03/14] target/arm: use arm_hcr_el2_eff() " remi.denis.courmont
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 35+ messages in thread
From: remi.denis.courmont @ 2020-11-02 10:57 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

Do not assume that EL2 is available in non-secure context.
That equivalence is broken by ARMv8.4-SEL2.

Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/cpu.h        |  4 ++--
 target/arm/helper-a64.c |  8 +-------
 target/arm/helper.c     | 33 +++++++++++++--------------------
 3 files changed, 16 insertions(+), 29 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index aaf3671806..263e650560 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2099,7 +2099,7 @@ static inline bool arm_el_is_aa64(CPUARMState *env, int el)
         return aa64;
     }
 
-    if (arm_feature(env, ARM_FEATURE_EL2) && !arm_is_secure_below_el3(env)) {
+    if (arm_is_el2_enabled(env)) {
         aa64 = aa64 && (env->cp15.hcr_el2 & HCR_RW);
     }
 
@@ -3043,7 +3043,7 @@ static inline int arm_debug_target_el(CPUARMState *env)
     bool secure = arm_is_secure(env);
     bool route_to_el2 = false;
 
-    if (arm_feature(env, ARM_FEATURE_EL2) && !secure) {
+    if (arm_is_el2_enabled(env)) {
         route_to_el2 = env->cp15.hcr_el2 & HCR_TGE ||
                        env->cp15.mdcr_el2 & MDCR_TDE;
     }
diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c
index 030821489b..c385fe82e9 100644
--- a/target/arm/helper-a64.c
+++ b/target/arm/helper-a64.c
@@ -972,8 +972,7 @@ void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc)
     if (new_el == -1) {
         goto illegal_return;
     }
-    if (new_el > cur_el
-        || (new_el == 2 && !arm_feature(env, ARM_FEATURE_EL2))) {
+    if (new_el > cur_el || (new_el == 2 && !arm_is_el2_enabled(env))) {
         /* Disallow return to an EL which is unimplemented or higher
          * than the current one.
          */
@@ -985,11 +984,6 @@ void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc)
         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_hcr_el2_eff(env) & HCR_TGE)) {
         goto illegal_return;
     }
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 97bb6b8c01..70050134e0 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1049,8 +1049,8 @@ static CPAccessResult cpacr_access(CPUARMState *env, const ARMCPRegInfo *ri,
 {
     if (arm_feature(env, ARM_FEATURE_V8)) {
         /* Check if CPACR accesses are to be trapped to EL2 */
-        if (arm_current_el(env) == 1 &&
-            (env->cp15.cptr_el[2] & CPTR_TCPAC) && !arm_is_secure(env)) {
+        if (arm_current_el(env) == 1 && arm_is_el2_enabled(env) &&
+            (env->cp15.cptr_el[2] & CPTR_TCPAC)) {
             return CP_ACCESS_TRAP_EL2;
         /* Check if CPACR accesses are to be trapped to EL3 */
         } else if (arm_current_el(env) < 3 &&
@@ -2522,7 +2522,7 @@ static CPAccessResult gt_counter_access(CPUARMState *env, int timeridx,
                                         bool isread)
 {
     unsigned int cur_el = arm_current_el(env);
-    bool secure = arm_is_secure(env);
+    bool has_el2 = arm_is_el2_enabled(env);
     uint64_t hcr = arm_hcr_el2_eff(env);
 
     switch (cur_el) {
@@ -2546,8 +2546,7 @@ static CPAccessResult gt_counter_access(CPUARMState *env, int timeridx,
             }
         } else {
             /* If HCR_EL2.<E2H> == 0: check CNTHCTL_EL2.EL1PCEN. */
-            if (arm_feature(env, ARM_FEATURE_EL2) &&
-                timeridx == GTIMER_PHYS && !secure &&
+            if (has_el2 && timeridx == GTIMER_PHYS &&
                 !extract32(env->cp15.cnthctl_el2, 1, 1)) {
                 return CP_ACCESS_TRAP_EL2;
             }
@@ -2556,8 +2555,7 @@ static CPAccessResult gt_counter_access(CPUARMState *env, int timeridx,
 
     case 1:
         /* Check CNTHCTL_EL2.EL1PCTEN, which changes location based on E2H. */
-        if (arm_feature(env, ARM_FEATURE_EL2) &&
-            timeridx == GTIMER_PHYS && !secure &&
+        if (has_el2 && timeridx == GTIMER_PHYS &&
             (hcr & HCR_E2H
              ? !extract32(env->cp15.cnthctl_el2, 10, 1)
              : !extract32(env->cp15.cnthctl_el2, 0, 1))) {
@@ -2572,7 +2570,7 @@ static CPAccessResult gt_timer_access(CPUARMState *env, int timeridx,
                                       bool isread)
 {
     unsigned int cur_el = arm_current_el(env);
-    bool secure = arm_is_secure(env);
+    bool has_el2 = arm_is_el2_enabled(env);
     uint64_t hcr = arm_hcr_el2_eff(env);
 
     switch (cur_el) {
@@ -2593,8 +2591,7 @@ static CPAccessResult gt_timer_access(CPUARMState *env, int timeridx,
         /* fall through */
 
     case 1:
-        if (arm_feature(env, ARM_FEATURE_EL2) &&
-            timeridx == GTIMER_PHYS && !secure) {
+        if (has_el2 && timeridx == GTIMER_PHYS) {
             if (hcr & HCR_E2H) {
                 /* If HCR_EL2.<E2H,TGE> == '10': check CNTHCTL_EL2.EL1PTEN. */
                 if (!extract32(env->cp15.cnthctl_el2, 11, 1)) {
@@ -4250,11 +4247,9 @@ static const ARMCPRegInfo strongarm_cp_reginfo[] = {
 
 static uint64_t midr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
-    ARMCPU *cpu = env_archcpu(env);
     unsigned int cur_el = arm_current_el(env);
-    bool secure = arm_is_secure(env);
 
-    if (arm_feature(&cpu->env, ARM_FEATURE_EL2) && !secure && cur_el == 1) {
+    if (arm_is_el2_enabled(env) && cur_el == 1) {
         return env->cp15.vpidr_el2;
     }
     return raw_read(env, ri);
@@ -4281,9 +4276,8 @@ static uint64_t mpidr_read_val(CPUARMState *env)
 static uint64_t mpidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     unsigned int cur_el = arm_current_el(env);
-    bool secure = arm_is_secure(env);
 
-    if (arm_feature(env, ARM_FEATURE_EL2) && !secure && cur_el == 1) {
+    if (arm_is_el2_enabled(env) && cur_el == 1) {
         return env->cp15.vmpidr_el2;
     }
     return mpidr_read_val(env);
@@ -5350,7 +5344,7 @@ uint64_t arm_hcr_el2_eff(CPUARMState *env)
 {
     uint64_t ret = env->cp15.hcr_el2;
 
-    if (arm_is_secure_below_el3(env)) {
+    if (!arm_is_el2_enabled(env)) {
         /*
          * "This register has no effect if EL2 is not enabled in the
          * current Security state".  This is ARMv8.4-SecEL2 speak for
@@ -6147,7 +6141,7 @@ int sve_exception_el(CPUARMState *env, int el)
     /* CPTR_EL2.  Since TZ and TFP are positive,
      * they will be zero when EL2 is not present.
      */
-    if (el <= 2 && !arm_is_secure_below_el3(env)) {
+    if (el <= 2 && arm_is_el2_enabled(env)) {
         if (env->cp15.cptr_el[2] & CPTR_TZ) {
             return 2;
         }
@@ -8735,8 +8729,7 @@ static int bad_mode_switch(CPUARMState *env, int mode, CPSRWriteType write_type)
         }
         return 0;
     case ARM_CPU_MODE_HYP:
-        return !arm_feature(env, ARM_FEATURE_EL2)
-            || arm_current_el(env) < 2 || arm_is_secure_below_el3(env);
+        return !arm_is_el2_enabled(env) || arm_current_el(env) < 2;
     case ARM_CPU_MODE_MON:
         return arm_current_el(env) < 3;
     default:
@@ -12646,7 +12639,7 @@ int fp_exception_el(CPUARMState *env, int cur_el)
 
     /* CPTR_EL2 : present in v7VE or v8 */
     if (cur_el <= 2 && extract32(env->cp15.cptr_el[2], 10, 1)
-        && !arm_is_secure_below_el3(env)) {
+        && arm_is_el2_enabled(env)) {
         /* Trap FP ops at EL2, NS-EL1 or NS-EL0 to EL2 */
         return 2;
     }
-- 
2.29.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 03/14] target/arm: use arm_hcr_el2_eff() where applicable
       [not found] <2172054.ElGaqSPkdT@basile.remlab.net>
  2020-11-02 10:57 ` [PATCH 01/14] target/arm: add arm_is_el2_enabled() helper remi.denis.courmont
  2020-11-02 10:57 ` [PATCH 02/14] target/arm: use arm_is_el2_enabled() where applicable remi.denis.courmont
@ 2020-11-02 10:57 ` remi.denis.courmont
  2020-11-03 16:56   ` Richard Henderson
  2020-11-02 10:57 ` [PATCH 04/14] target/arm: factor MDCR_EL2 common handling remi.denis.courmont
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 35+ messages in thread
From: remi.denis.courmont @ 2020-11-02 10:57 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

This will simplify accessing HCR conditionally in secure state.

Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/helper.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 70050134e0..f4822fab0c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4435,16 +4435,16 @@ static CPAccessResult aa64_cacheop_pou_access(CPUARMState *env,
 
 static int vae1_tlbmask(CPUARMState *env)
 {
-    /* Since we exclude secure first, we may read HCR_EL2 directly. */
-    if (arm_is_secure_below_el3(env)) {
-        return ARMMMUIdxBit_SE10_1 |
-               ARMMMUIdxBit_SE10_1_PAN |
-               ARMMMUIdxBit_SE10_0;
-    } else if ((env->cp15.hcr_el2 & (HCR_E2H | HCR_TGE))
-               == (HCR_E2H | HCR_TGE)) {
+    uint64_t hcr = arm_hcr_el2_eff(env);
+
+    if ((hcr & (HCR_E2H | HCR_TGE)) == (HCR_E2H | HCR_TGE)) {
         return ARMMMUIdxBit_E20_2 |
                ARMMMUIdxBit_E20_2_PAN |
                ARMMMUIdxBit_E20_0;
+    } else if (arm_is_secure_below_el3(env)) {
+        return ARMMMUIdxBit_SE10_1 |
+               ARMMMUIdxBit_SE10_1_PAN |
+               ARMMMUIdxBit_SE10_0;
     } else {
         return ARMMMUIdxBit_E10_1 |
                ARMMMUIdxBit_E10_1_PAN |
@@ -9980,6 +9980,8 @@ static inline uint64_t regime_sctlr(CPUARMState *env, ARMMMUIdx mmu_idx)
 static inline bool regime_translation_disabled(CPUARMState *env,
                                                ARMMMUIdx mmu_idx)
 {
+    uint64_t hcr_el2;
+
     if (arm_feature(env, ARM_FEATURE_M)) {
         switch (env->v7m.mpu_ctrl[regime_is_secure(env, mmu_idx)] &
                 (R_V7M_MPU_CTRL_ENABLE_MASK | R_V7M_MPU_CTRL_HFNMIENA_MASK)) {
@@ -9998,19 +10000,21 @@ static inline bool regime_translation_disabled(CPUARMState *env,
         }
     }
 
+    hcr_el2 = arm_hcr_el2_eff(env);
+
     if (mmu_idx == ARMMMUIdx_Stage2) {
         /* HCR.DC means HCR.VM behaves as 1 */
-        return (env->cp15.hcr_el2 & (HCR_DC | HCR_VM)) == 0;
+        return (hcr_el2 & (HCR_DC | HCR_VM)) == 0;
     }
 
-    if (env->cp15.hcr_el2 & HCR_TGE) {
+    if (hcr_el2 & HCR_TGE) {
         /* TGE means that NS EL0/1 act as if SCTLR_EL1.M is zero */
         if (!regime_is_secure(env, mmu_idx) && regime_el(env, mmu_idx) == 1) {
             return true;
         }
     }
 
-    if ((env->cp15.hcr_el2 & HCR_DC) && arm_mmu_idx_is_stage1_of_2(mmu_idx)) {
+    if ((hcr_el2 & HCR_DC) && arm_mmu_idx_is_stage1_of_2(mmu_idx)) {
         /* HCR.DC means SCTLR_EL1.M behaves as 0 */
         return true;
     }
@@ -10361,7 +10365,7 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
             fi->s1ptw = true;
             return ~0;
         }
-        if ((env->cp15.hcr_el2 & HCR_PTW) && (cacheattrs.attrs & 0xf0) == 0) {
+        if ((arm_hcr_el2_eff(env) & HCR_PTW) && (cacheattrs.attrs & 0xf0) == 0) {
             /*
              * PTW set and S1 walk touched S2 Device memory:
              * generate Permission fault.
@@ -10794,7 +10798,7 @@ static uint8_t convert_stage2_attrs(CPUARMState *env, uint8_t s2attrs)
     uint8_t hihint = 0, lohint = 0;
 
     if (hiattr != 0) { /* normal memory */
-        if ((env->cp15.hcr_el2 & HCR_CD) != 0) { /* cache disabled */
+        if (arm_hcr_el2_eff(env) & HCR_CD) { /* cache disabled */
             hiattr = loattr = 1; /* non-cacheable */
         } else {
             if (hiattr != 1) { /* Write-through or write-back */
@@ -12111,7 +12115,7 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
             }
 
             /* Combine the S1 and S2 cache attributes. */
-            if (env->cp15.hcr_el2 & HCR_DC) {
+            if (arm_hcr_el2_eff(env) & HCR_DC) {
                 /*
                  * HCR.DC forces the first stage attributes to
                  *  Normal Non-Shareable,
-- 
2.29.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 04/14] target/arm: factor MDCR_EL2 common handling
       [not found] <2172054.ElGaqSPkdT@basile.remlab.net>
                   ` (2 preceding siblings ...)
  2020-11-02 10:57 ` [PATCH 03/14] target/arm: use arm_hcr_el2_eff() " remi.denis.courmont
@ 2020-11-02 10:57 ` remi.denis.courmont
  2020-11-03 17:00   ` Richard Henderson
  2020-11-02 10:57 ` [PATCH 05/14] target/arm: declare new AA64PFR0 bit-fields remi.denis.courmont
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 35+ messages in thread
From: remi.denis.courmont @ 2020-11-02 10:57 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

This adds a common helper to compute the effective value of MDCR_EL2.
That is the actual value if EL2 is enabled in the current security
context, or 0 elsewise.

Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/helper.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index f4822fab0c..d0ee9ff3fc 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -538,6 +538,11 @@ static CPAccessResult access_trap_aa32s_el1(CPUARMState *env,
     return CP_ACCESS_TRAP_UNCATEGORIZED;
 }
 
+static uint64_t arm_mdcr_el2_eff(CPUARMState *env)
+{
+    return arm_is_el2_enabled(env) ? env->cp15.mdcr_el2 : 0;
+}
+
 /* Check for traps to "powerdown debug" registers, which are controlled
  * by MDCR.TDOSA
  */
@@ -545,11 +550,11 @@ static CPAccessResult access_tdosa(CPUARMState *env, const ARMCPRegInfo *ri,
                                    bool isread)
 {
     int el = arm_current_el(env);
-    bool mdcr_el2_tdosa = (env->cp15.mdcr_el2 & MDCR_TDOSA) ||
-        (env->cp15.mdcr_el2 & MDCR_TDE) ||
+    uint64_t mdcr_el2 = arm_mdcr_el2_eff(env);
+    bool mdcr_el2_tdosa = (mdcr_el2 & MDCR_TDOSA) || (mdcr_el2 & MDCR_TDE) ||
         (arm_hcr_el2_eff(env) & HCR_TGE);
 
-    if (el < 2 && mdcr_el2_tdosa && !arm_is_secure_below_el3(env)) {
+    if (el < 2 && mdcr_el2_tdosa) {
         return CP_ACCESS_TRAP_EL2;
     }
     if (el < 3 && (env->cp15.mdcr_el3 & MDCR_TDOSA)) {
@@ -565,11 +570,11 @@ static CPAccessResult access_tdra(CPUARMState *env, const ARMCPRegInfo *ri,
                                   bool isread)
 {
     int el = arm_current_el(env);
-    bool mdcr_el2_tdra = (env->cp15.mdcr_el2 & MDCR_TDRA) ||
-        (env->cp15.mdcr_el2 & MDCR_TDE) ||
+    uint64_t mdcr_el2 = arm_mdcr_el2_eff(env);
+    bool mdcr_el2_tdra = (mdcr_el2 & MDCR_TDRA) || (mdcr_el2 & MDCR_TDE) ||
         (arm_hcr_el2_eff(env) & HCR_TGE);
 
-    if (el < 2 && mdcr_el2_tdra && !arm_is_secure_below_el3(env)) {
+    if (el < 2 && mdcr_el2_tdra) {
         return CP_ACCESS_TRAP_EL2;
     }
     if (el < 3 && (env->cp15.mdcr_el3 & MDCR_TDA)) {
@@ -585,11 +590,11 @@ static CPAccessResult access_tda(CPUARMState *env, const ARMCPRegInfo *ri,
                                   bool isread)
 {
     int el = arm_current_el(env);
-    bool mdcr_el2_tda = (env->cp15.mdcr_el2 & MDCR_TDA) ||
-        (env->cp15.mdcr_el2 & MDCR_TDE) ||
+    uint64_t mdcr_el2 = arm_mdcr_el2_eff(env);
+    bool mdcr_el2_tda = (mdcr_el2 & MDCR_TDA) || (mdcr_el2 & MDCR_TDE) ||
         (arm_hcr_el2_eff(env) & HCR_TGE);
 
-    if (el < 2 && mdcr_el2_tda && !arm_is_secure_below_el3(env)) {
+    if (el < 2 && mdcr_el2_tda) {
         return CP_ACCESS_TRAP_EL2;
     }
     if (el < 3 && (env->cp15.mdcr_el3 & MDCR_TDA)) {
@@ -605,9 +610,9 @@ static CPAccessResult access_tpm(CPUARMState *env, const ARMCPRegInfo *ri,
                                  bool isread)
 {
     int el = arm_current_el(env);
+    uint64_t mdcr_el2 = arm_mdcr_el2_eff(env);
 
-    if (el < 2 && (env->cp15.mdcr_el2 & MDCR_TPM)
-        && !arm_is_secure_below_el3(env)) {
+    if (el < 2 && (mdcr_el2 & MDCR_TPM)) {
         return CP_ACCESS_TRAP_EL2;
     }
     if (el < 3 && (env->cp15.mdcr_el3 & MDCR_TPM)) {
@@ -1348,12 +1353,12 @@ static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri,
      * trapping to EL2 or EL3 for other accesses.
      */
     int el = arm_current_el(env);
+    uint64_t mdcr_el2 = arm_mdcr_el2_eff(env);
 
     if (el == 0 && !(env->cp15.c9_pmuserenr & 1)) {
         return CP_ACCESS_TRAP;
     }
-    if (el < 2 && (env->cp15.mdcr_el2 & MDCR_TPM)
-        && !arm_is_secure_below_el3(env)) {
+    if (el < 2 && (mdcr_el2 & MDCR_TPM)) {
         return CP_ACCESS_TRAP_EL2;
     }
     if (el < 3 && (env->cp15.mdcr_el3 & MDCR_TPM)) {
@@ -1432,7 +1437,8 @@ static bool pmu_counter_enabled(CPUARMState *env, uint8_t counter)
     bool enabled, prohibited, filtered;
     bool secure = arm_is_secure(env);
     int el = arm_current_el(env);
-    uint8_t hpmn = env->cp15.mdcr_el2 & MDCR_HPMN;
+    uint64_t mdcr_el2 = arm_mdcr_el2_eff(env);
+    uint8_t hpmn = mdcr_el2 & MDCR_HPMN;
 
     if (!arm_feature(env, ARM_FEATURE_PMU)) {
         return false;
@@ -1442,13 +1448,13 @@ static bool pmu_counter_enabled(CPUARMState *env, uint8_t counter)
             (counter < hpmn || counter == 31)) {
         e = env->cp15.c9_pmcr & PMCRE;
     } else {
-        e = env->cp15.mdcr_el2 & MDCR_HPME;
+        e = mdcr_el2 & MDCR_HPME;
     }
     enabled = e && (env->cp15.c9_pmcnten & (1 << counter));
 
     if (!secure) {
         if (el == 2 && (counter < hpmn || counter == 31)) {
-            prohibited = env->cp15.mdcr_el2 & MDCR_HPMD;
+            prohibited = mdcr_el2 & MDCR_HPMD;
         } else {
             prohibited = false;
         }
-- 
2.29.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 05/14] target/arm: declare new AA64PFR0 bit-fields
       [not found] <2172054.ElGaqSPkdT@basile.remlab.net>
                   ` (3 preceding siblings ...)
  2020-11-02 10:57 ` [PATCH 04/14] target/arm: factor MDCR_EL2 common handling remi.denis.courmont
@ 2020-11-02 10:57 ` remi.denis.courmont
  2020-11-03 17:02   ` Richard Henderson
  2020-11-02 10:57 ` [PATCH 06/14] target/arm: add 64-bit S-EL2 to EL exception table remi.denis.courmont
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 35+ messages in thread
From: remi.denis.courmont @ 2020-11-02 10:57 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/cpu.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 263e650560..724b11ee57 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1849,6 +1849,12 @@ FIELD(ID_AA64PFR0, ADVSIMD, 20, 4)
 FIELD(ID_AA64PFR0, GIC, 24, 4)
 FIELD(ID_AA64PFR0, RAS, 28, 4)
 FIELD(ID_AA64PFR0, SVE, 32, 4)
+FIELD(ID_AA64PFR0, SEL2, 36, 4)
+FIELD(ID_AA64PFR0, MPAM, 40, 4)
+FIELD(ID_AA64PFR0, AMU, 44, 4)
+FIELD(ID_AA64PFR0, DIT, 48, 4)
+FIELD(ID_AA64PFR0, CSV2, 56, 4)
+FIELD(ID_AA64PFR0, CSV3, 60, 4)
 
 FIELD(ID_AA64PFR1, BT, 0, 4)
 FIELD(ID_AA64PFR1, SBSS, 4, 4)
@@ -3879,6 +3885,11 @@ static inline bool isar_feature_aa64_sve(const ARMISARegisters *id)
     return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, SVE) != 0;
 }
 
+static inline bool isar_feature_aa64_sel2(const ARMISARegisters *id)
+{
+    return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, SEL2) != 0;
+}
+
 static inline bool isar_feature_aa64_vh(const ARMISARegisters *id)
 {
     return FIELD_EX64(id->id_aa64mmfr1, ID_AA64MMFR1, VH) != 0;
-- 
2.29.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 06/14] target/arm: add 64-bit S-EL2 to EL exception table
       [not found] <2172054.ElGaqSPkdT@basile.remlab.net>
                   ` (4 preceding siblings ...)
  2020-11-02 10:57 ` [PATCH 05/14] target/arm: declare new AA64PFR0 bit-fields remi.denis.courmont
@ 2020-11-02 10:57 ` remi.denis.courmont
  2020-11-02 10:57 ` [PATCH 07/14] target/arm: return the stage 2 index for stage 1 remi.denis.courmont
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 35+ messages in thread
From: remi.denis.courmont @ 2020-11-02 10:57 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

With the ARMv8.4-SEL2 extension, EL2 is a legal exception level in
secure mode, though it can only be AArch64.

This patch adds the target EL for exceptions from 64-bit S-EL2.

It also fixes the target EL to EL2 when HCR.{A,F,I}MO are set in secure
mode. Those values were never used in practice as the effective value of
HCR was always 0 in secure mode.

Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/helper.c    | 10 +++++-----
 target/arm/op_helper.c |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index d0ee9ff3fc..a86ea7a28a 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9017,13 +9017,13 @@ static const int8_t target_el_table[2][2][2][2][2][4] = {
       {{/* 0   1   1   0 */{ 3,  3,  3, -1 },{ 3, -1, -1,  3 },},
        {/* 0   1   1   1 */{ 3,  3,  3, -1 },{ 3, -1, -1,  3 },},},},},
     {{{{/* 1   0   0   0 */{ 1,  1,  2, -1 },{ 1,  1, -1,  1 },},
-       {/* 1   0   0   1 */{ 2,  2,  2, -1 },{ 1,  1, -1,  1 },},},
-      {{/* 1   0   1   0 */{ 1,  1,  1, -1 },{ 1,  1, -1,  1 },},
-       {/* 1   0   1   1 */{ 2,  2,  2, -1 },{ 1,  1, -1,  1 },},},},
+       {/* 1   0   0   1 */{ 2,  2,  2, -1 },{ 2,  2, -1,  1 },},},
+      {{/* 1   0   1   0 */{ 1,  1,  1, -1 },{ 1,  1,  1,  1 },},
+       {/* 1   0   1   1 */{ 2,  2,  2, -1 },{ 2,  2,  2,  1 },},},},
      {{{/* 1   1   0   0 */{ 3,  3,  3, -1 },{ 3,  3, -1,  3 },},
        {/* 1   1   0   1 */{ 3,  3,  3, -1 },{ 3,  3, -1,  3 },},},
-      {{/* 1   1   1   0 */{ 3,  3,  3, -1 },{ 3,  3, -1,  3 },},
-       {/* 1   1   1   1 */{ 3,  3,  3, -1 },{ 3,  3, -1,  3 },},},},},
+      {{/* 1   1   1   0 */{ 3,  3,  3, -1 },{ 3,  3,  3,  3 },},
+       {/* 1   1   1   1 */{ 3,  3,  3, -1 },{ 3,  3,  3,  3 },},},},},
 };
 
 /*
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index b1065216b2..c3c3b30920 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -649,10 +649,10 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome,
         target_el = exception_target_el(env);
         break;
     case CP_ACCESS_TRAP_EL2:
-        /* Requesting a trap to EL2 when we're in EL3 or S-EL0/1 is
+        /* Requesting a trap to EL2 when we're in EL3 is
          * a bug in the access function.
          */
-        assert(!arm_is_secure(env) && arm_current_el(env) != 3);
+        assert(arm_current_el(env) != 3);
         target_el = 2;
         break;
     case CP_ACCESS_TRAP_EL3:
-- 
2.29.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 07/14] target/arm: return the stage 2 index for stage 1
       [not found] <2172054.ElGaqSPkdT@basile.remlab.net>
                   ` (5 preceding siblings ...)
  2020-11-02 10:57 ` [PATCH 06/14] target/arm: add 64-bit S-EL2 to EL exception table remi.denis.courmont
@ 2020-11-02 10:57 ` remi.denis.courmont
  2020-11-03 17:04   ` Richard Henderson
  2020-11-02 10:57 ` [PATCH 08/14] target/arm: add MMU stage 1 for Secure EL2 remi.denis.courmont
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 35+ messages in thread
From: remi.denis.courmont @ 2020-11-02 10:57 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

This makes arm_mmu_idx_is_stage1_of_2() optionally return the stage 2
MMU index. With Secure EL2, there are more than one stage 2 regimes, so
we can no longer hard-code a constant index for it.

Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/helper.c    | 13 +++++++------
 target/arm/internals.h | 15 ++++++++++-----
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index a86ea7a28a..30c7f09b64 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3427,7 +3427,7 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value,
         bool take_exc = false;
 
         if (fi.s1ptw && current_el == 1 && !arm_is_secure(env)
-            && arm_mmu_idx_is_stage1_of_2(mmu_idx)) {
+            && arm_mmu_idx_is_stage1_of_2(mmu_idx, NULL)) {
             /*
              * Synchronous stage 2 fault on an access made as part of the
              * translation table walk for AT S1E0* or AT S1E1* insn
@@ -10020,7 +10020,7 @@ static inline bool regime_translation_disabled(CPUARMState *env,
         }
     }
 
-    if ((hcr_el2 & HCR_DC) && arm_mmu_idx_is_stage1_of_2(mmu_idx)) {
+    if ((hcr_el2 & HCR_DC) && arm_mmu_idx_is_stage1_of_2(mmu_idx, NULL)) {
         /* HCR.DC means SCTLR_EL1.M behaves as 0 */
         return true;
     }
@@ -10352,16 +10352,17 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
                                hwaddr addr, MemTxAttrs txattrs,
                                ARMMMUFaultInfo *fi)
 {
-    if (arm_mmu_idx_is_stage1_of_2(mmu_idx) &&
-        !regime_translation_disabled(env, ARMMMUIdx_Stage2)) {
+    ARMMMUIdx s2_mmu_idx;
+
+    if (arm_mmu_idx_is_stage1_of_2(mmu_idx, &s2_mmu_idx) &&
+        !regime_translation_disabled(env, s2_mmu_idx)) {
         target_ulong s2size;
         hwaddr s2pa;
         int s2prot;
         int ret;
         ARMCacheAttrs cacheattrs = {};
 
-        ret = get_phys_addr_lpae(env, addr, MMU_DATA_LOAD, ARMMMUIdx_Stage2,
-                                 false,
+        ret = get_phys_addr_lpae(env, addr, MMU_DATA_LOAD, s2_mmu_idx, false,
                                  &s2pa, &txattrs, &s2prot, &s2size, fi,
                                  &cacheattrs);
         if (ret) {
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 5460678756..55ffc08747 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1146,17 +1146,22 @@ ARMMMUIdx arm_stage1_mmu_idx(CPUARMState *env);
 
 /**
  * arm_mmu_idx_is_stage1_of_2:
- * @mmu_idx: The ARMMMUIdx to test
+ * @s1_mmu_idx: The ARMMMUIdx to test
+ * @s2_mmu_idx: Storage space for the stage 2 ARMMMUIdx
  *
- * Return true if @mmu_idx is a NOTLB mmu_idx that is the
- * first stage of a two stage regime.
+ * Return true if @mmu_idx is a NOTLB mmu_idx that is the first stage
+ * of a two stage regime. The corresponding second stage will be
+ * stored in @s2_mmu_idx.
  */
-static inline bool arm_mmu_idx_is_stage1_of_2(ARMMMUIdx mmu_idx)
+static inline bool arm_mmu_idx_is_stage1_of_2(ARMMMUIdx s1_mmu_idx,
+                                              ARMMMUIdx *s2_mmu_idx)
 {
-    switch (mmu_idx) {
+    switch (s1_mmu_idx) {
     case ARMMMUIdx_Stage1_E0:
     case ARMMMUIdx_Stage1_E1:
     case ARMMMUIdx_Stage1_E1_PAN:
+        if (s2_mmu_idx != NULL)
+            *s2_mmu_idx = ARMMMUIdx_Stage2;
         return true;
     default:
         return false;
-- 
2.29.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 08/14] target/arm: add MMU stage 1 for Secure EL2
       [not found] <2172054.ElGaqSPkdT@basile.remlab.net>
                   ` (6 preceding siblings ...)
  2020-11-02 10:57 ` [PATCH 07/14] target/arm: return the stage 2 index for stage 1 remi.denis.courmont
@ 2020-11-02 10:57 ` remi.denis.courmont
  2020-11-03 18:32   ` Richard Henderson
  2020-11-02 10:57 ` [PATCH 09/14] target/arm: add ARMv8.4-SEL2 system registers remi.denis.courmont
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 35+ messages in thread
From: remi.denis.courmont @ 2020-11-02 10:57 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

This adds the MMU indices for EL2 stage 1 in secure mode.

To keep code contained, which is largelly identical between secure and
non-secure modes, this patch introduces a secure bit for all new and
existing stage 1 translation regimes.

Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/cpu-param.h     |   2 +-
 target/arm/cpu.h           |  22 ++++--
 target/arm/helper.c        | 143 ++++++++++++++++++++++++-------------
 target/arm/internals.h     |  12 ++++
 target/arm/translate-a64.c |   4 ++
 5 files changed, 127 insertions(+), 56 deletions(-)

diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
index 6321385b46..0db5e37c17 100644
--- a/target/arm/cpu-param.h
+++ b/target/arm/cpu-param.h
@@ -29,6 +29,6 @@
 # define TARGET_PAGE_BITS_MIN  10
 #endif
 
-#define NB_MMU_MODES 11
+#define NB_MMU_MODES 16
 
 #endif
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 724b11ee57..3fbb70e273 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2944,6 +2944,9 @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync);
 #define ARM_MMU_IDX_NOTLB 0x20  /* does not have a TLB */
 #define ARM_MMU_IDX_M     0x40  /* M profile */
 
+/* Meanings of the bits for A profile mmu idx values */
+#define ARM_MMU_IDX_A_S      0x8
+
 /* Meanings of the bits for M profile mmu idx values */
 #define ARM_MMU_IDX_M_PRIV   0x1
 #define ARM_MMU_IDX_M_NEGPRI 0x2
@@ -2967,10 +2970,17 @@ typedef enum ARMMMUIdx {
     ARMMMUIdx_E20_2      =  5 | ARM_MMU_IDX_A,
     ARMMMUIdx_E20_2_PAN  =  6 | ARM_MMU_IDX_A,
 
-    ARMMMUIdx_SE10_0     = 7 | ARM_MMU_IDX_A,
-    ARMMMUIdx_SE10_1     = 8 | ARM_MMU_IDX_A,
-    ARMMMUIdx_SE10_1_PAN = 9 | ARM_MMU_IDX_A,
-    ARMMMUIdx_SE3        = 10 | ARM_MMU_IDX_A,
+    ARMMMUIdx_SE10_0     = ARMMMUIdx_E10_0 | ARM_MMU_IDX_A_S,
+    ARMMMUIdx_SE20_0     = ARMMMUIdx_E20_0 | ARM_MMU_IDX_A_S,
+
+    ARMMMUIdx_SE10_1     = ARMMMUIdx_E10_1 | ARM_MMU_IDX_A_S,
+    ARMMMUIdx_SE10_1_PAN = ARMMMUIdx_E10_1_PAN | ARM_MMU_IDX_A_S,
+
+    ARMMMUIdx_SE2        = ARMMMUIdx_E2 | ARM_MMU_IDX_A_S,
+    ARMMMUIdx_SE20_2     = ARMMMUIdx_E20_2 | ARM_MMU_IDX_A_S,
+    ARMMMUIdx_SE20_2_PAN = ARMMMUIdx_E20_2_PAN | ARM_MMU_IDX_A_S,
+
+    ARMMMUIdx_SE3        = 15 | ARM_MMU_IDX_A,
 
     /*
      * These are not allocated TLBs and are used only for AT system
@@ -3017,8 +3027,12 @@ typedef enum ARMMMUIdxBit {
     TO_CORE_BIT(E20_2),
     TO_CORE_BIT(E20_2_PAN),
     TO_CORE_BIT(SE10_0),
+    TO_CORE_BIT(SE20_0),
     TO_CORE_BIT(SE10_1),
     TO_CORE_BIT(SE10_1_PAN),
+    TO_CORE_BIT(SE2),
+    TO_CORE_BIT(SE20_2),
+    TO_CORE_BIT(SE20_2_PAN),
     TO_CORE_BIT(SE3),
 
     TO_CORE_BIT(MUser),
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 30c7f09b64..eeae3ecff2 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2864,6 +2864,9 @@ static int gt_phys_redir_timeridx(CPUARMState *env)
     case ARMMMUIdx_E20_0:
     case ARMMMUIdx_E20_2:
     case ARMMMUIdx_E20_2_PAN:
+    case ARMMMUIdx_SE20_0:
+    case ARMMMUIdx_SE20_2:
+    case ARMMMUIdx_SE20_2_PAN:
         return GTIMER_HYP;
     default:
         return GTIMER_PHYS;
@@ -2876,6 +2879,9 @@ static int gt_virt_redir_timeridx(CPUARMState *env)
     case ARMMMUIdx_E20_0:
     case ARMMMUIdx_E20_2:
     case ARMMMUIdx_E20_2_PAN:
+    case ARMMMUIdx_SE20_0:
+    case ARMMMUIdx_SE20_2:
+    case ARMMMUIdx_SE20_2_PAN:
         return GTIMER_HYPVIRT;
     default:
         return GTIMER_VIRT;
@@ -3579,7 +3585,7 @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
             mmu_idx = ARMMMUIdx_SE3;
             break;
         case 2:
-            g_assert(!secure);  /* TODO: ARMv8.4-SecEL2 */
+            g_assert(!secure);  /* ARMv8.4-SecEL2 is 64-bit only */
             /* fall through */
         case 1:
             if (ri->crm == 9 && (env->uncached_cpsr & CPSR_PAN)) {
@@ -3649,7 +3655,7 @@ static void ats1h_write(CPUARMState *env, const ARMCPRegInfo *ri,
 static CPAccessResult at_s1e2_access(CPUARMState *env, const ARMCPRegInfo *ri,
                                      bool isread)
 {
-    if (arm_current_el(env) == 3 && !(env->cp15.scr_el3 & SCR_NS)) {
+    if (arm_current_el(env) == 3 && !(env->cp15.scr_el3 & (SCR_NS|SCR_EEL2))) {
         return CP_ACCESS_TRAP;
     }
     return CP_ACCESS_OK;
@@ -3675,7 +3681,7 @@ static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri,
             }
             break;
         case 4: /* AT S1E2R, AT S1E2W */
-            mmu_idx = ARMMMUIdx_E2;
+            mmu_idx = secure ? ARMMMUIdx_SE2 : ARMMMUIdx_E2;
             break;
         case 6: /* AT S1E3R, AT S1E3W */
             mmu_idx = ARMMMUIdx_SE3;
@@ -3990,10 +3996,14 @@ static void vmsa_tcr_ttbr_el2_write(CPUARMState *env, const ARMCPRegInfo *ri,
      */
     if (extract64(raw_read(env, ri) ^ value, 48, 16) &&
         (arm_hcr_el2_eff(env) & HCR_E2H)) {
-        tlb_flush_by_mmuidx(env_cpu(env),
-                            ARMMMUIdxBit_E20_2 |
-                            ARMMMUIdxBit_E20_2_PAN |
-                            ARMMMUIdxBit_E20_0);
+        uint16_t mask = ARMMMUIdxBit_E20_2 |
+                        ARMMMUIdxBit_E20_2_PAN |
+                        ARMMMUIdxBit_E20_0;
+
+        if (arm_is_secure_below_el3(env))
+            mask <<= ARM_MMU_IDX_A_S;
+
+        tlb_flush_by_mmuidx(env_cpu(env), mask);
     }
     raw_write(env, ri, value);
 }
@@ -4009,10 +4019,14 @@ static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
      * the combined stage 1&2 tlbs (EL10_1 and EL10_0).
      */
     if (raw_read(env, ri) != value) {
-        tlb_flush_by_mmuidx(cs,
-                            ARMMMUIdxBit_E10_1 |
-                            ARMMMUIdxBit_E10_1_PAN |
-                            ARMMMUIdxBit_E10_0);
+        uint16_t mask = ARMMMUIdxBit_E10_1 |
+                        ARMMMUIdxBit_E10_1_PAN |
+                        ARMMMUIdxBit_E10_0;
+
+        if (arm_is_secure_below_el3(env))
+            mask <<= ARM_MMU_IDX_A_S;
+
+        tlb_flush_by_mmuidx(cs, mask);
         raw_write(env, ri, value);
     }
 }
@@ -4442,20 +4456,23 @@ static CPAccessResult aa64_cacheop_pou_access(CPUARMState *env,
 static int vae1_tlbmask(CPUARMState *env)
 {
     uint64_t hcr = arm_hcr_el2_eff(env);
+    uint16_t mask;
 
     if ((hcr & (HCR_E2H | HCR_TGE)) == (HCR_E2H | HCR_TGE)) {
-        return ARMMMUIdxBit_E20_2 |
+        mask = ARMMMUIdxBit_E20_2 |
                ARMMMUIdxBit_E20_2_PAN |
                ARMMMUIdxBit_E20_0;
-    } else if (arm_is_secure_below_el3(env)) {
-        return ARMMMUIdxBit_SE10_1 |
-               ARMMMUIdxBit_SE10_1_PAN |
-               ARMMMUIdxBit_SE10_0;
     } else {
-        return ARMMMUIdxBit_E10_1 |
+        mask = ARMMMUIdxBit_E10_1 |
                ARMMMUIdxBit_E10_1_PAN |
                ARMMMUIdxBit_E10_0;
     }
+
+    if (arm_is_secure_below_el3(env)) {
+        mask <<= ARM_MMU_IDX_A_S;
+    }
+
+    return mask;
 }
 
 /* Return 56 if TBI is enabled, 64 otherwise. */
@@ -4471,17 +4488,20 @@ static int tlbbits_for_regime(CPUARMState *env, ARMMMUIdx mmu_idx,
 
 static int vae1_tlbbits(CPUARMState *env, uint64_t addr)
 {
+    uint64_t hcr = arm_hcr_el2_eff(env);
     ARMMMUIdx mmu_idx;
 
     /* Only the regime of the mmu_idx below is significant. */
-    if (arm_is_secure_below_el3(env)) {
-        mmu_idx = ARMMMUIdx_SE10_0;
-    } else if ((env->cp15.hcr_el2 & (HCR_E2H | HCR_TGE))
-               == (HCR_E2H | HCR_TGE)) {
+    if ((hcr & (HCR_E2H | HCR_TGE)) == (HCR_E2H | HCR_TGE)) {
         mmu_idx = ARMMMUIdx_E20_0;
     } else {
         mmu_idx = ARMMMUIdx_E10_0;
     }
+
+    if (arm_is_secure_below_el3(env)) {
+        mmu_idx |= ARM_MMU_IDX_A_S;
+    }
+
     return tlbbits_for_regime(env, mmu_idx, addr);
 }
 
@@ -4527,11 +4547,17 @@ static int alle1_tlbmask(CPUARMState *env)
 
 static int e2_tlbmask(CPUARMState *env)
 {
-    /* TODO: ARMv8.4-SecEL2 */
-    return ARMMMUIdxBit_E20_0 |
-           ARMMMUIdxBit_E20_2 |
-           ARMMMUIdxBit_E20_2_PAN |
-           ARMMMUIdxBit_E2;
+    if (arm_is_secure_below_el3(env)) {
+        return ARMMMUIdxBit_SE20_0 |
+               ARMMMUIdxBit_SE20_2 |
+               ARMMMUIdxBit_SE20_2_PAN |
+               ARMMMUIdxBit_SE2;
+    } else {
+        return ARMMMUIdxBit_E20_0 |
+               ARMMMUIdxBit_E20_2 |
+               ARMMMUIdxBit_E20_2_PAN |
+               ARMMMUIdxBit_E2;
+    }
 }
 
 static void tlbi_aa64_alle1_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -4651,10 +4677,12 @@ static void tlbi_aa64_vae2is_write(CPUARMState *env, const ARMCPRegInfo *ri,
 {
     CPUState *cs = env_cpu(env);
     uint64_t pageaddr = sextract64(value << 12, 0, 56);
-    int bits = tlbbits_for_regime(env, ARMMMUIdx_E2, pageaddr);
+    bool secure = arm_is_secure_below_el3(env);
+    int mask = secure ? ARMMMUIdxBit_SE2 : ARMMMUIdxBit_E2;
+    int bits = tlbbits_for_regime(env, secure ? ARMMMUIdx_E2 : ARMMMUIdx_SE2,
+                                  pageaddr);
 
-    tlb_flush_page_bits_by_mmuidx_all_cpus_synced(cs, pageaddr,
-                                                  ARMMMUIdxBit_E2, bits);
+    tlb_flush_page_bits_by_mmuidx_all_cpus_synced(cs, pageaddr, mask, bits);
 }
 
 static void tlbi_aa64_vae3is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -9969,7 +9997,8 @@ uint64_t arm_sctlr(CPUARMState *env, int el)
     /* Only EL0 needs to be adjusted for EL1&0 or EL2&0. */
     if (el == 0) {
         ARMMMUIdx mmu_idx = arm_mmu_idx_el(env, 0);
-        el = (mmu_idx == ARMMMUIdx_E20_0 ? 2 : 1);
+        el = (mmu_idx == ARMMMUIdx_E20_0 || mmu_idx == ARMMMUIdx_SE20_0)
+             ? 2 : 1;
     }
     return env->cp15.sctlr_el[el];
 }
@@ -10098,6 +10127,7 @@ static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx)
     switch (mmu_idx) {
     case ARMMMUIdx_SE10_0:
     case ARMMMUIdx_E20_0:
+    case ARMMMUIdx_SE20_0:
     case ARMMMUIdx_Stage1_E0:
     case ARMMMUIdx_MUser:
     case ARMMMUIdx_MSUser:
@@ -12675,6 +12705,7 @@ int arm_mmu_idx_to_el(ARMMMUIdx mmu_idx)
     case ARMMMUIdx_E10_0:
     case ARMMMUIdx_E20_0:
     case ARMMMUIdx_SE10_0:
+    case ARMMMUIdx_SE20_0:
         return 0;
     case ARMMMUIdx_E10_1:
     case ARMMMUIdx_E10_1_PAN:
@@ -12684,6 +12715,9 @@ int arm_mmu_idx_to_el(ARMMMUIdx mmu_idx)
     case ARMMMUIdx_E2:
     case ARMMMUIdx_E20_2:
     case ARMMMUIdx_E20_2_PAN:
+    case ARMMMUIdx_SE2:
+    case ARMMMUIdx_SE20_2:
+    case ARMMMUIdx_SE20_2_PAN:
         return 2;
     case ARMMMUIdx_SE3:
         return 3;
@@ -12701,6 +12735,9 @@ ARMMMUIdx arm_v7m_mmu_idx_for_secstate(CPUARMState *env, bool secstate)
 
 ARMMMUIdx arm_mmu_idx_el(CPUARMState *env, int el)
 {
+    ARMMMUIdx idx;
+    uint64_t hcr;
+
     if (arm_feature(env, ARM_FEATURE_M)) {
         return arm_v7m_mmu_idx_for_secstate(env, env->v7m.secure);
     }
@@ -12708,40 +12745,43 @@ ARMMMUIdx arm_mmu_idx_el(CPUARMState *env, int el)
     /* See ARM pseudo-function ELIsInHost.  */
     switch (el) {
     case 0:
-        if (arm_is_secure_below_el3(env)) {
-            return ARMMMUIdx_SE10_0;
-        }
-        if ((env->cp15.hcr_el2 & (HCR_E2H | HCR_TGE)) == (HCR_E2H | HCR_TGE)
-            && arm_el_is_aa64(env, 2)) {
-            return ARMMMUIdx_E20_0;
+        hcr = arm_hcr_el2_eff(env);
+        if ((hcr & (HCR_E2H | HCR_TGE)) == (HCR_E2H | HCR_TGE)) {
+            idx = ARMMMUIdx_E20_0;
+        } else {
+            idx = ARMMMUIdx_E10_0;
         }
-        return ARMMMUIdx_E10_0;
+        break;
     case 1:
-        if (arm_is_secure_below_el3(env)) {
-            if (env->pstate & PSTATE_PAN) {
-                return ARMMMUIdx_SE10_1_PAN;
-            }
-            return ARMMMUIdx_SE10_1;
-        }
         if (env->pstate & PSTATE_PAN) {
-            return ARMMMUIdx_E10_1_PAN;
+            idx = ARMMMUIdx_E10_1_PAN;
+        } else {
+            idx = ARMMMUIdx_E10_1;
         }
-        return ARMMMUIdx_E10_1;
+        break;
     case 2:
-        /* TODO: ARMv8.4-SecEL2 */
         /* Note that TGE does not apply at EL2.  */
-        if ((env->cp15.hcr_el2 & HCR_E2H) && arm_el_is_aa64(env, 2)) {
+        if (arm_hcr_el2_eff(env) & HCR_E2H) {
             if (env->pstate & PSTATE_PAN) {
-                return ARMMMUIdx_E20_2_PAN;
+                idx = ARMMMUIdx_E20_2_PAN;
+            } else {
+                idx = ARMMMUIdx_E20_2;
             }
-            return ARMMMUIdx_E20_2;
+        } else {
+            idx = ARMMMUIdx_E2;
         }
-        return ARMMMUIdx_E2;
+        break;
     case 3:
         return ARMMMUIdx_SE3;
     default:
         g_assert_not_reached();
     }
+
+    if (arm_is_secure_below_el3(env)) {
+        idx |= ARM_MMU_IDX_A_S;
+    }
+
+    return idx;
 }
 
 ARMMMUIdx arm_mmu_idx(CPUARMState *env)
@@ -12906,7 +12946,8 @@ static uint32_t rebuild_hflags_a64(CPUARMState *env, int el, int fp_el,
             break;
         case ARMMMUIdx_E20_2:
         case ARMMMUIdx_E20_2_PAN:
-            /* TODO: ARMv8.4-SecEL2 */
+        case ARMMMUIdx_SE20_2:
+        case ARMMMUIdx_SE20_2_PAN:
             /*
              * Note that EL20_2 is gated by HCR_EL2.E2H == 1, but EL20_0 is
              * gated by HCR_EL2.<E2H,TGE> == '11', and so is LDTR.
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 55ffc08747..469c94962a 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -860,6 +860,9 @@ static inline bool regime_has_2_ranges(ARMMMUIdx mmu_idx)
     case ARMMMUIdx_SE10_0:
     case ARMMMUIdx_SE10_1:
     case ARMMMUIdx_SE10_1_PAN:
+    case ARMMMUIdx_SE20_0:
+    case ARMMMUIdx_SE20_2:
+    case ARMMMUIdx_SE20_2_PAN:
         return true;
     default:
         return false;
@@ -890,6 +893,10 @@ static inline bool regime_is_secure(CPUARMState *env, ARMMMUIdx mmu_idx)
     case ARMMMUIdx_SE10_0:
     case ARMMMUIdx_SE10_1:
     case ARMMMUIdx_SE10_1_PAN:
+    case ARMMMUIdx_SE20_0:
+    case ARMMMUIdx_SE20_2:
+    case ARMMMUIdx_SE20_2_PAN:
+    case ARMMMUIdx_SE2:
     case ARMMMUIdx_MSPrivNegPri:
     case ARMMMUIdx_MSUserNegPri:
     case ARMMMUIdx_MSPriv:
@@ -907,6 +914,7 @@ static inline bool regime_is_pan(CPUARMState *env, ARMMMUIdx mmu_idx)
     case ARMMMUIdx_E10_1_PAN:
     case ARMMMUIdx_E20_2_PAN:
     case ARMMMUIdx_SE10_1_PAN:
+    case ARMMMUIdx_SE20_2_PAN:
         return true;
     default:
         return false;
@@ -917,10 +925,14 @@ static inline bool regime_is_pan(CPUARMState *env, ARMMMUIdx mmu_idx)
 static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
 {
     switch (mmu_idx) {
+    case ARMMMUIdx_SE20_0:
+    case ARMMMUIdx_SE20_2:
+    case ARMMMUIdx_SE20_2_PAN:
     case ARMMMUIdx_E20_0:
     case ARMMMUIdx_E20_2:
     case ARMMMUIdx_E20_2_PAN:
     case ARMMMUIdx_Stage2:
+    case ARMMMUIdx_SE2:
     case ARMMMUIdx_E2:
         return 2;
     case ARMMMUIdx_SE3:
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 072754fa24..ca820e8ecd 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -118,6 +118,10 @@ static int get_a64_user_mem_index(DisasContext *s)
         case ARMMMUIdx_SE10_1_PAN:
             useridx = ARMMMUIdx_SE10_0;
             break;
+        case ARMMMUIdx_SE20_2:
+        case ARMMMUIdx_SE20_2_PAN:
+            useridx = ARMMMUIdx_SE20_0;
+            break;
         default:
             g_assert_not_reached();
         }
-- 
2.29.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 09/14] target/arm: add ARMv8.4-SEL2 system registers
       [not found] <2172054.ElGaqSPkdT@basile.remlab.net>
                   ` (7 preceding siblings ...)
  2020-11-02 10:57 ` [PATCH 08/14] target/arm: add MMU stage 1 for Secure EL2 remi.denis.courmont
@ 2020-11-02 10:57 ` remi.denis.courmont
  2020-11-03 19:49   ` Richard Henderson
  2020-11-02 10:57 ` [PATCH 10/14] target/arm: do S1_ptw_translate() before address space lookup remi.denis.courmont
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 35+ messages in thread
From: remi.denis.courmont @ 2020-11-02 10:57 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/cpu.h    |  2 ++
 target/arm/helper.c | 24 ++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 3fbb70e273..c047b17a81 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -323,9 +323,11 @@ typedef struct CPUARMState {
             uint64_t ttbr1_el[4];
         };
         uint64_t vttbr_el2; /* Virtualization Translation Table Base.  */
+        uint64_t vsttbr_el2; /* Secure Virtualization Translation Table. */
         /* MMU translation table base control. */
         TCR tcr_el[4];
         TCR vtcr_el2; /* Virtualization Translation Control.  */
+        TCR vstcr_el2; /* Secure Virtualization Translation Control. */
         uint32_t c2_data; /* MPU data cacheable bits.  */
         uint32_t c2_insn; /* MPU instruction cacheable bits.  */
         union { /* MMU domain access control register
diff --git a/target/arm/helper.c b/target/arm/helper.c
index eeae3ecff2..4c86e4f57c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5724,6 +5724,27 @@ static const ARMCPRegInfo el2_v8_cp_reginfo[] = {
     REGINFO_SENTINEL
 };
 
+static CPAccessResult sel2_access(CPUARMState *env, const ARMCPRegInfo *ri,
+                                  bool isread)
+{
+    if (arm_current_el(env) == 3 || arm_is_secure_below_el3(env)) {
+        return CP_ACCESS_OK;
+    }
+    return CP_ACCESS_TRAP_UNCATEGORIZED;
+}
+
+static const ARMCPRegInfo el2_sec_cp_reginfo[] = {
+    { .name = "VSTTBR_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 6, .opc2 = 0,
+      .access = PL2_RW, .accessfn = sel2_access,
+      .fieldoffset = offsetof(CPUARMState, cp15.vsttbr_el2) },
+    { .name = "VSTCR_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 6, .opc2 = 2,
+      .access = PL2_RW, .accessfn = sel2_access,
+      .fieldoffset = offsetof(CPUARMState, cp15.vstcr_el2) },
+    REGINFO_SENTINEL
+};
+
 static CPAccessResult nsacr_access(CPUARMState *env, const ARMCPRegInfo *ri,
                                    bool isread)
 {
@@ -7745,6 +7766,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         if (arm_feature(env, ARM_FEATURE_V8)) {
             define_arm_cp_regs(cpu, el2_v8_cp_reginfo);
         }
+        if (cpu_isar_feature(aa64_sel2, cpu)) {
+            define_arm_cp_regs(cpu, el2_sec_cp_reginfo);
+        }
         /* RVBAR_EL2 is only implemented if EL2 is the highest EL */
         if (!arm_feature(env, ARM_FEATURE_EL3)) {
             ARMCPRegInfo rvbar = {
-- 
2.29.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 10/14] target/arm: do S1_ptw_translate() before address space lookup
       [not found] <2172054.ElGaqSPkdT@basile.remlab.net>
                   ` (8 preceding siblings ...)
  2020-11-02 10:57 ` [PATCH 09/14] target/arm: add ARMv8.4-SEL2 system registers remi.denis.courmont
@ 2020-11-02 10:57 ` remi.denis.courmont
  2020-11-03 19:54   ` Richard Henderson
  2020-11-02 10:57 ` [PATCH 11/14] target/arm: secure stage 2 translation regime remi.denis.courmont
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 35+ messages in thread
From: remi.denis.courmont @ 2020-11-02 10:57 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

In the secure stage 2 translation regime, the VSTCR.SW and VTCR.NSW
bits can invert the secure flag for pagetable walks. This patchset
allows S1_ptw_translate() to change the non-secure bit.

Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/helper.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 4c86e4f57c..7c70460e65 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10403,7 +10403,7 @@ static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx,
 
 /* Translate a S1 pagetable walk through S2 if needed.  */
 static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
-                               hwaddr addr, MemTxAttrs txattrs,
+                               hwaddr addr, bool *is_secure,
                                ARMMMUFaultInfo *fi)
 {
     ARMMMUIdx s2_mmu_idx;
@@ -10415,6 +10415,9 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
         int s2prot;
         int ret;
         ARMCacheAttrs cacheattrs = {};
+        MemTxAttrs txattrs = {};
+
+        assert(!*is_secure); /* TODO: S-EL2 */
 
         ret = get_phys_addr_lpae(env, addr, MMU_DATA_LOAD, s2_mmu_idx, false,
                                  &s2pa, &txattrs, &s2prot, &s2size, fi,
@@ -10453,9 +10456,9 @@ static uint32_t arm_ldl_ptw(CPUState *cs, hwaddr addr, bool is_secure,
     AddressSpace *as;
     uint32_t data;
 
+    addr = S1_ptw_translate(env, mmu_idx, addr, &is_secure, fi);
     attrs.secure = is_secure;
     as = arm_addressspace(cs, attrs);
-    addr = S1_ptw_translate(env, mmu_idx, addr, attrs, fi);
     if (fi->s1ptw) {
         return 0;
     }
@@ -10482,9 +10485,9 @@ static uint64_t arm_ldq_ptw(CPUState *cs, hwaddr addr, bool is_secure,
     AddressSpace *as;
     uint64_t data;
 
+    addr = S1_ptw_translate(env, mmu_idx, addr, &is_secure, fi);
     attrs.secure = is_secure;
     as = arm_addressspace(cs, attrs);
-    addr = S1_ptw_translate(env, mmu_idx, addr, attrs, fi);
     if (fi->s1ptw) {
         return 0;
     }
-- 
2.29.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 11/14] target/arm: secure stage 2 translation regime
       [not found] <2172054.ElGaqSPkdT@basile.remlab.net>
                   ` (9 preceding siblings ...)
  2020-11-02 10:57 ` [PATCH 10/14] target/arm: do S1_ptw_translate() before address space lookup remi.denis.courmont
@ 2020-11-02 10:57 ` remi.denis.courmont
  2020-11-02 10:58 ` [PATCH 12/14] target/arm: set HPFAR_EL2.NS on secure stage 2 faults remi.denis.courmont
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 35+ messages in thread
From: remi.denis.courmont @ 2020-11-02 10:57 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/cpu.h       | 11 ++++-
 target/arm/helper.c    | 95 ++++++++++++++++++++++++++++++++----------
 target/arm/internals.h | 24 +++++++++++
 3 files changed, 106 insertions(+), 24 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index c047b17a81..a5ee7452d9 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -168,6 +168,11 @@ typedef struct {
     uint32_t base_mask;
 } TCR;
 
+#define VTCR_NSW	(1u << 29)
+#define VTCR_NSA	(1u << 30)
+#define VSTCR_SW	VTCR_NSW
+#define VSTCR_SA	VTCR_NSA
+
 /* Define a maximum sized vector register.
  * For 32-bit, this is a 128-bit NEON/AdvSIMD register.
  * For 64-bit, this is a 2048-bit SVE register.
@@ -2991,6 +2996,9 @@ typedef enum ARMMMUIdx {
     ARMMMUIdx_Stage1_E0 = 0 | ARM_MMU_IDX_NOTLB,
     ARMMMUIdx_Stage1_E1 = 1 | ARM_MMU_IDX_NOTLB,
     ARMMMUIdx_Stage1_E1_PAN = 2 | ARM_MMU_IDX_NOTLB,
+    ARMMMUIdx_Stage1_SE0 = 3 | ARM_MMU_IDX_NOTLB,
+    ARMMMUIdx_Stage1_SE1 = 4 | ARM_MMU_IDX_NOTLB,
+    ARMMMUIdx_Stage1_SE1_PAN = 5 | ARM_MMU_IDX_NOTLB,
     /*
      * Not allocated a TLB: used only for second stage of an S12 page
      * table walk, or for descriptor loads during first stage of an S1
@@ -2998,7 +3006,8 @@ typedef enum ARMMMUIdx {
      * then various TLB flush insns which currently are no-ops or flush
      * only stage 1 MMU indexes will need to change to flush stage 2.
      */
-    ARMMMUIdx_Stage2     = 3 | ARM_MMU_IDX_NOTLB,
+    ARMMMUIdx_Stage2     = 6 | ARM_MMU_IDX_NOTLB,
+    ARMMMUIdx_Stage2_S   = 7 | ARM_MMU_IDX_NOTLB,
 
     /*
      * M-profile.
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 7c70460e65..a2bc6801e0 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3432,7 +3432,7 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value,
         uint32_t syn, fsr, fsc;
         bool take_exc = false;
 
-        if (fi.s1ptw && current_el == 1 && !arm_is_secure(env)
+        if (fi.s1ptw && current_el == 1
             && arm_mmu_idx_is_stage1_of_2(mmu_idx, NULL)) {
             /*
              * Synchronous stage 2 fault on an access made as part of the
@@ -3589,10 +3589,10 @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
             /* fall through */
         case 1:
             if (ri->crm == 9 && (env->uncached_cpsr & CPSR_PAN)) {
-                mmu_idx = (secure ? ARMMMUIdx_SE10_1_PAN
+                mmu_idx = (secure ? ARMMMUIdx_Stage1_SE1_PAN
                            : ARMMMUIdx_Stage1_E1_PAN);
             } else {
-                mmu_idx = secure ? ARMMMUIdx_SE10_1 : ARMMMUIdx_Stage1_E1;
+                mmu_idx = secure ? ARMMMUIdx_Stage1_SE1 : ARMMMUIdx_Stage1_E1;
             }
             break;
         default:
@@ -3606,10 +3606,11 @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
             mmu_idx = ARMMMUIdx_SE10_0;
             break;
         case 2:
+            g_assert(!secure);  /* ARMv8.4-SecEL2 is 64-bit only */
             mmu_idx = ARMMMUIdx_Stage1_E0;
             break;
         case 1:
-            mmu_idx = secure ? ARMMMUIdx_SE10_0 : ARMMMUIdx_Stage1_E0;
+            mmu_idx = secure ? ARMMMUIdx_Stage1_SE0 : ARMMMUIdx_Stage1_E0;
             break;
         default:
             g_assert_not_reached();
@@ -3674,10 +3675,10 @@ static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri,
         switch (ri->opc1) {
         case 0: /* AT S1E1R, AT S1E1W, AT S1E1RP, AT S1E1WP */
             if (ri->crm == 9 && (env->pstate & PSTATE_PAN)) {
-                mmu_idx = (secure ? ARMMMUIdx_SE10_1_PAN
+                mmu_idx = (secure ? ARMMMUIdx_Stage1_SE1_PAN
                            : ARMMMUIdx_Stage1_E1_PAN);
             } else {
-                mmu_idx = secure ? ARMMMUIdx_SE10_1 : ARMMMUIdx_Stage1_E1;
+                mmu_idx = secure ? ARMMMUIdx_Stage1_SE1 : ARMMMUIdx_Stage1_E1;
             }
             break;
         case 4: /* AT S1E2R, AT S1E2W */
@@ -3691,7 +3692,7 @@ static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri,
         }
         break;
     case 2: /* AT S1E0R, AT S1E0W */
-        mmu_idx = secure ? ARMMMUIdx_SE10_0 : ARMMMUIdx_Stage1_E0;
+        mmu_idx = secure ? ARMMMUIdx_Stage1_SE0 : ARMMMUIdx_Stage1_E0;
         break;
     case 4: /* AT S12E1R, AT S12E1W */
         mmu_idx = secure ? ARMMMUIdx_SE10_1 : ARMMMUIdx_E10_1;
@@ -10061,7 +10062,7 @@ static inline bool regime_translation_disabled(CPUARMState *env,
 
     hcr_el2 = arm_hcr_el2_eff(env);
 
-    if (mmu_idx == ARMMMUIdx_Stage2) {
+    if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
         /* HCR.DC means HCR.VM behaves as 1 */
         return (hcr_el2 & (HCR_DC | HCR_VM)) == 0;
     }
@@ -10094,6 +10095,9 @@ static inline uint64_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx,
     if (mmu_idx == ARMMMUIdx_Stage2) {
         return env->cp15.vttbr_el2;
     }
+    if (mmu_idx == ARMMMUIdx_Stage2_S) {
+        return env->cp15.vsttbr_el2;
+    }
     if (ttbrn == 0) {
         return env->cp15.ttbr0_el[regime_el(env, mmu_idx)];
     } else {
@@ -10109,6 +10113,12 @@ static inline uint64_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx,
 static inline ARMMMUIdx stage_1_mmu_idx(ARMMMUIdx mmu_idx)
 {
     switch (mmu_idx) {
+    case ARMMMUIdx_SE10_0:
+        return ARMMMUIdx_Stage1_SE0;
+    case ARMMMUIdx_SE10_1:
+        return ARMMMUIdx_Stage1_SE1;
+    case ARMMMUIdx_SE10_1_PAN:
+        return ARMMMUIdx_Stage1_SE1_PAN;
     case ARMMMUIdx_E10_0:
         return ARMMMUIdx_Stage1_E0;
     case ARMMMUIdx_E10_1:
@@ -10153,6 +10163,7 @@ static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx)
     case ARMMMUIdx_E20_0:
     case ARMMMUIdx_SE20_0:
     case ARMMMUIdx_Stage1_E0:
+    case ARMMMUIdx_Stage1_SE0:
     case ARMMMUIdx_MUser:
     case ARMMMUIdx_MSUser:
     case ARMMMUIdx_MUserNegPri:
@@ -10318,6 +10329,7 @@ static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
     int wxn = 0;
 
     assert(mmu_idx != ARMMMUIdx_Stage2);
+    assert(mmu_idx != ARMMMUIdx_Stage2_S);
 
     user_rw = simple_ap_to_rw_prot_is_user(ap, true);
     if (is_user) {
@@ -10417,7 +10429,21 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
         ARMCacheAttrs cacheattrs = {};
         MemTxAttrs txattrs = {};
 
-        assert(!*is_secure); /* TODO: S-EL2 */
+        if (s2_mmu_idx == ARMMMUIdx_Stage2_S) {
+            /* Check if page table walk is to secure or non-secure PA space. */
+            if (*is_secure) {
+                *is_secure = !(env->cp15.vstcr_el2.raw_tcr & VSTCR_SW);
+            } else {
+                *is_secure = !(env->cp15.vtcr_el2.raw_tcr & VTCR_NSW);
+            }
+            if (*is_secure) {
+                txattrs.secure = true;
+            } else {
+                s2_mmu_idx = ARMMMUIdx_Stage2;
+            }
+        } else {
+            assert(!*is_secure);
+        }
 
         ret = get_phys_addr_lpae(env, addr, MMU_DATA_LOAD, s2_mmu_idx, false,
                                  &s2pa, &txattrs, &s2prot, &s2size, fi,
@@ -10882,7 +10908,7 @@ static int aa64_va_parameter_tbi(uint64_t tcr, ARMMMUIdx mmu_idx)
 {
     if (regime_has_2_ranges(mmu_idx)) {
         return extract64(tcr, 37, 2);
-    } else if (mmu_idx == ARMMMUIdx_Stage2) {
+    } else if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
         return 0; /* VTCR_EL2 */
     } else {
         /* Replicate the single TBI bit so we always have 2 bits.  */
@@ -10894,7 +10920,7 @@ static int aa64_va_parameter_tbid(uint64_t tcr, ARMMMUIdx mmu_idx)
 {
     if (regime_has_2_ranges(mmu_idx)) {
         return extract64(tcr, 51, 2);
-    } else if (mmu_idx == ARMMMUIdx_Stage2) {
+    } else if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
         return 0; /* VTCR_EL2 */
     } else {
         /* Replicate the single TBID bit so we always have 2 bits.  */
@@ -10924,7 +10950,7 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va,
         tsz = extract32(tcr, 0, 6);
         using64k = extract32(tcr, 14, 1);
         using16k = extract32(tcr, 15, 1);
-        if (mmu_idx == ARMMMUIdx_Stage2) {
+        if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
             /* VTCR_EL2 */
             hpd = false;
         } else {
@@ -10982,6 +11008,8 @@ static ARMVAParameters aa32_va_parameters(CPUARMState *env, uint32_t va,
     int select, tsz;
     bool epd, hpd;
 
+    assert(mmu_idx != ARMMMUIdx_Stage2_S);
+
     if (mmu_idx == ARMMMUIdx_Stage2) {
         /* VTCR */
         bool sext = extract32(tcr, 4, 1);
@@ -11147,7 +11175,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
         goto do_fault;
     }
 
-    if (mmu_idx != ARMMMUIdx_Stage2) {
+    if (mmu_idx != ARMMMUIdx_Stage2 && mmu_idx != ARMMMUIdx_Stage2_S) {
         /* The starting level depends on the virtual address size (which can
          * be up to 48 bits) and the translation granule size. It indicates
          * the number of strides (stride bits at a time) needed to
@@ -11251,7 +11279,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
         attrs = extract64(descriptor, 2, 10)
             | (extract64(descriptor, 52, 12) << 10);
 
-        if (mmu_idx == ARMMMUIdx_Stage2) {
+        if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
             /* Stage 2 table descriptors do not include any attribute fields */
             break;
         }
@@ -11281,8 +11309,10 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
 
     ap = extract32(attrs, 4, 2);
 
-    if (mmu_idx == ARMMMUIdx_Stage2) {
-        ns = true;
+    if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
+        if (mmu_idx == ARMMMUIdx_Stage2) {
+            ns = true;
+        }
         xn = extract32(attrs, 11, 2);
         *prot = get_S2prot(env, ap, xn, s1_is_el0);
     } else {
@@ -11309,7 +11339,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
         arm_tlb_bti_gp(txattrs) = true;
     }
 
-    if (mmu_idx == ARMMMUIdx_Stage2) {
+    if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
         cacheattrs->attrs = convert_stage2_attrs(env, extract32(attrs, 0, 4));
     } else {
         /* Index into MAIR registers for cache attributes */
@@ -11328,7 +11358,8 @@ do_fault:
     fi->type = fault_type;
     fi->level = level;
     /* Tag the error as S2 for failed S1 PTW at S2 or ordinary S2.  */
-    fi->stage2 = fi->s1ptw || (mmu_idx == ARMMMUIdx_Stage2);
+    fi->stage2 = fi->s1ptw || (mmu_idx == ARMMMUIdx_Stage2 ||
+                               mmu_idx == ARMMMUIdx_Stage2_S);
     return true;
 }
 
@@ -12144,7 +12175,10 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
 {
     if (mmu_idx == ARMMMUIdx_E10_0 ||
         mmu_idx == ARMMMUIdx_E10_1 ||
-        mmu_idx == ARMMMUIdx_E10_1_PAN) {
+        mmu_idx == ARMMMUIdx_E10_1_PAN ||
+        mmu_idx == ARMMMUIdx_SE10_0 ||
+        mmu_idx == ARMMMUIdx_SE10_1 ||
+        mmu_idx == ARMMMUIdx_SE10_1_PAN) {
         /* Call ourselves recursively to do the stage 1 and then stage 2
          * translations.
          */
@@ -12152,21 +12186,25 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
             hwaddr ipa;
             int s2_prot;
             int ret;
+            ARMMMUIdx s2_mmu_idx;
+            bool is_el0;
             ARMCacheAttrs cacheattrs2 = {};
 
             ret = get_phys_addr(env, address, access_type,
                                 stage_1_mmu_idx(mmu_idx), &ipa, attrs,
                                 prot, page_size, fi, cacheattrs);
 
+            s2_mmu_idx = attrs->secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
+            is_el0 = mmu_idx == ARMMMUIdx_SE10_0 || mmu_idx == ARMMMUIdx_E10_0;
+
             /* If S1 fails or S2 is disabled, return early.  */
-            if (ret || regime_translation_disabled(env, ARMMMUIdx_Stage2)) {
+            if (ret || regime_translation_disabled(env, s2_mmu_idx)) {
                 *phys_ptr = ipa;
                 return ret;
             }
 
             /* S1 is done. Now do S2 translation.  */
-            ret = get_phys_addr_lpae(env, ipa, access_type, ARMMMUIdx_Stage2,
-                                     mmu_idx == ARMMMUIdx_E10_0,
+            ret = get_phys_addr_lpae(env, ipa, access_type, s2_mmu_idx, is_el0,
                                      phys_ptr, attrs, &s2_prot,
                                      page_size, fi, &cacheattrs2);
             fi->s2addr = ipa;
@@ -12178,6 +12216,17 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
                 return ret;
             }
 
+            /* Check if IPA translates to secure or non-secure PA space. */
+            if (arm_is_secure_below_el3(env)) {
+                if (attrs->secure) {
+                    attrs->secure =
+                        !(env->cp15.vstcr_el2.raw_tcr & (VSTCR_SA | VSTCR_SW));
+                } else {
+                    attrs->secure =
+                        !(env->cp15.vtcr_el2.raw_tcr & (VTCR_NSA | VTCR_NSW));
+                }
+            }
+
             /* Combine the S1 and S2 cache attributes. */
             if (arm_hcr_el2_eff(env) & HCR_DC) {
                 /*
@@ -12261,7 +12310,7 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
          * MMU disabled.  S1 addresses within aa64 translation regimes are
          * still checked for bounds -- see AArch64.TranslateAddressS1Off.
          */
-        if (mmu_idx != ARMMMUIdx_Stage2) {
+        if (mmu_idx != ARMMMUIdx_Stage2 && mmu_idx != ARMMMUIdx_Stage2_S) {
             int r_el = regime_el(env, mmu_idx);
             if (arm_el_is_aa64(env, r_el)) {
                 int pamax = arm_pamax(env_archcpu(env));
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 469c94962a..d53b0c22fe 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -851,6 +851,9 @@ static inline bool regime_has_2_ranges(ARMMMUIdx mmu_idx)
     case ARMMMUIdx_Stage1_E0:
     case ARMMMUIdx_Stage1_E1:
     case ARMMMUIdx_Stage1_E1_PAN:
+    case ARMMMUIdx_Stage1_SE0:
+    case ARMMMUIdx_Stage1_SE1:
+    case ARMMMUIdx_Stage1_SE1_PAN:
     case ARMMMUIdx_E10_0:
     case ARMMMUIdx_E10_1:
     case ARMMMUIdx_E10_1_PAN:
@@ -896,7 +899,11 @@ static inline bool regime_is_secure(CPUARMState *env, ARMMMUIdx mmu_idx)
     case ARMMMUIdx_SE20_0:
     case ARMMMUIdx_SE20_2:
     case ARMMMUIdx_SE20_2_PAN:
+    case ARMMMUIdx_Stage1_SE0:
+    case ARMMMUIdx_Stage1_SE1:
+    case ARMMMUIdx_Stage1_SE1_PAN:
     case ARMMMUIdx_SE2:
+    case ARMMMUIdx_Stage2_S:
     case ARMMMUIdx_MSPrivNegPri:
     case ARMMMUIdx_MSUserNegPri:
     case ARMMMUIdx_MSPriv:
@@ -911,6 +918,7 @@ static inline bool regime_is_pan(CPUARMState *env, ARMMMUIdx mmu_idx)
 {
     switch (mmu_idx) {
     case ARMMMUIdx_Stage1_E1_PAN:
+    case ARMMMUIdx_Stage1_SE1_PAN:
     case ARMMMUIdx_E10_1_PAN:
     case ARMMMUIdx_E20_2_PAN:
     case ARMMMUIdx_SE10_1_PAN:
@@ -932,6 +940,7 @@ static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
     case ARMMMUIdx_E20_2:
     case ARMMMUIdx_E20_2_PAN:
     case ARMMMUIdx_Stage2:
+    case ARMMMUIdx_Stage2_S:
     case ARMMMUIdx_SE2:
     case ARMMMUIdx_E2:
         return 2;
@@ -944,6 +953,9 @@ static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
     case ARMMMUIdx_Stage1_E0:
     case ARMMMUIdx_Stage1_E1:
     case ARMMMUIdx_Stage1_E1_PAN:
+    case ARMMMUIdx_Stage1_SE0:
+    case ARMMMUIdx_Stage1_SE1:
+    case ARMMMUIdx_Stage1_SE1_PAN:
     case ARMMMUIdx_E10_0:
     case ARMMMUIdx_E10_1:
     case ARMMMUIdx_E10_1_PAN:
@@ -967,6 +979,12 @@ static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
     if (mmu_idx == ARMMMUIdx_Stage2) {
         return &env->cp15.vtcr_el2;
     }
+    if (mmu_idx == ARMMMUIdx_Stage2_S) {
+        /* Note: Secure stage 2 nominally shares fields from VTCR_EL2, but
+         * those are not currently used by QEMU, so just return VSTCR_EL2.
+         */
+        return &env->cp15.vstcr_el2;
+    }
     return &env->cp15.tcr_el[regime_el(env, mmu_idx)];
 }
 
@@ -1175,6 +1193,12 @@ static inline bool arm_mmu_idx_is_stage1_of_2(ARMMMUIdx s1_mmu_idx,
         if (s2_mmu_idx != NULL)
             *s2_mmu_idx = ARMMMUIdx_Stage2;
         return true;
+    case ARMMMUIdx_Stage1_SE0:
+    case ARMMMUIdx_Stage1_SE1:
+    case ARMMMUIdx_Stage1_SE1_PAN:
+        if (s2_mmu_idx != NULL)
+            *s2_mmu_idx = ARMMMUIdx_Stage2_S;
+        return true;
     default:
         return false;
     }
-- 
2.29.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 12/14] target/arm: set HPFAR_EL2.NS on secure stage 2 faults
       [not found] <2172054.ElGaqSPkdT@basile.remlab.net>
                   ` (10 preceding siblings ...)
  2020-11-02 10:57 ` [PATCH 11/14] target/arm: secure stage 2 translation regime remi.denis.courmont
@ 2020-11-02 10:58 ` remi.denis.courmont
  2020-11-02 10:58 ` [PATCH 13/14] target/arm: add ARMv8.4-SEL2 extension remi.denis.courmont
  2020-11-02 10:58 ` [PATCH 14/14] target/arm: enable Secure EL2 in max CPU remi.denis.courmont
  13 siblings, 0 replies; 35+ messages in thread
From: remi.denis.courmont @ 2020-11-02 10:58 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/cpu.h        | 2 ++
 target/arm/helper.c     | 6 ++++++
 target/arm/internals.h  | 2 ++
 target/arm/tlb_helper.c | 3 +++
 4 files changed, 13 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index a5ee7452d9..495acf2d0a 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1482,6 +1482,8 @@ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
 #define HCR_TWEDEN    (1ULL << 59)
 #define HCR_TWEDEL    MAKE_64BIT_MASK(60, 4)
 
+#define HPFAR_NS      (1ULL << 63)
+
 #define SCR_NS                (1U << 0)
 #define SCR_IRQ               (1U << 1)
 #define SCR_FIQ               (1U << 2)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index a2bc6801e0..74fd759d48 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3447,6 +3447,9 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value,
                 target_el = 3;
             } else {
                 env->cp15.hpfar_el2 = extract64(fi.s2addr, 12, 47) << 4;
+                if (fi.s1ns) {
+                    env->cp15.hpfar_el2 |= HPFAR_NS;
+                }
                 target_el = 2;
             }
             take_exc = true;
@@ -10453,6 +10456,7 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
             fi->s2addr = addr;
             fi->stage2 = true;
             fi->s1ptw = true;
+            fi->s1ns = !*is_secure;
             return ~0;
         }
         if ((arm_hcr_el2_eff(env) & HCR_PTW) && (cacheattrs.attrs & 0xf0) == 0) {
@@ -10464,6 +10468,7 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
             fi->s2addr = addr;
             fi->stage2 = true;
             fi->s1ptw = true;
+            fi->s1ns = !*is_secure;
             return ~0;
         }
         addr = s2pa;
@@ -11360,6 +11365,7 @@ do_fault:
     /* Tag the error as S2 for failed S1 PTW at S2 or ordinary S2.  */
     fi->stage2 = fi->s1ptw || (mmu_idx == ARMMMUIdx_Stage2 ||
                                mmu_idx == ARMMMUIdx_Stage2_S);
+    fi->s1ns = mmu_idx == ARMMMUIdx_Stage2_S;
     return true;
 }
 
diff --git a/target/arm/internals.h b/target/arm/internals.h
index d53b0c22fe..188997c701 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -593,6 +593,7 @@ typedef enum ARMFaultType {
  * @s2addr: Address that caused a fault at stage 2
  * @stage2: True if we faulted at stage 2
  * @s1ptw: True if we faulted at stage 2 while doing a stage 1 page-table walk
+ * @s1ns: True if we faulted on a non-secure IPA while in secure state
  * @ea: True if we should set the EA (external abort type) bit in syndrome
  */
 typedef struct ARMMMUFaultInfo ARMMMUFaultInfo;
@@ -603,6 +604,7 @@ struct ARMMMUFaultInfo {
     int domain;
     bool stage2;
     bool s1ptw;
+    bool s1ns;
     bool ea;
 };
 
diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
index b35dc8a011..450969a742 100644
--- a/target/arm/tlb_helper.c
+++ b/target/arm/tlb_helper.c
@@ -63,6 +63,9 @@ static void QEMU_NORETURN arm_deliver_fault(ARMCPU *cpu, vaddr addr,
     if (fi->stage2) {
         target_el = 2;
         env->cp15.hpfar_el2 = extract64(fi->s2addr, 12, 47) << 4;
+        if (fi->s1ns) {
+            env->cp15.hpfar_el2 |= HPFAR_NS;
+        }
     }
     same_el = (arm_current_el(env) == target_el);
 
-- 
2.29.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 13/14] target/arm: add ARMv8.4-SEL2 extension
       [not found] <2172054.ElGaqSPkdT@basile.remlab.net>
                   ` (11 preceding siblings ...)
  2020-11-02 10:58 ` [PATCH 12/14] target/arm: set HPFAR_EL2.NS on secure stage 2 faults remi.denis.courmont
@ 2020-11-02 10:58 ` remi.denis.courmont
  2020-11-03 20:14   ` Richard Henderson
  2020-11-02 10:58 ` [PATCH 14/14] target/arm: enable Secure EL2 in max CPU remi.denis.courmont
  13 siblings, 1 reply; 35+ messages in thread
From: remi.denis.courmont @ 2020-11-02 10:58 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

This adds handling for the SCR_EL3.EEL2 bit.

A translation block flag is added in A32 mode to route exceptions
correctly from AArch32 S-EL1 to (AArch64) S-EL2.

Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/cpu.c       |  2 +-
 target/arm/cpu.h       | 11 ++++++++---
 target/arm/helper.c    | 19 +++++++++++++++++--
 target/arm/translate.c |  6 ++++--
 target/arm/translate.h |  1 +
 5 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 07492e9f9a..22d2177a9f 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -476,7 +476,7 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
              * masked from Secure state. The HCR and SCR settings
              * don't affect the masking logic, only the interrupt routing.
              */
-            if (target_el == 3 || !secure) {
+            if (target_el == 3 || !secure || (env->cp15.scr_el3 & SCR_EEL2)) {
                 unmasked = true;
             }
         } else {
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 495acf2d0a..3f2b0be1ba 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2059,7 +2059,10 @@ static inline bool arm_is_secure(CPUARMState *env)
 static inline bool arm_is_el2_enabled(CPUARMState *env)
 {
     if (arm_feature(env, ARM_FEATURE_EL2)) {
-        return !arm_is_secure_below_el3(env);
+        if (arm_is_secure_below_el3(env)) {
+            return (env->cp15.scr_el3 & SCR_EEL2) != 0;
+        }
+        return true;
     }
     return false;
 }
@@ -2106,7 +2109,8 @@ static inline bool arm_el_is_aa64(CPUARMState *env, int el)
         return aa64;
     }
 
-    if (arm_feature(env, ARM_FEATURE_EL3)) {
+    if (arm_feature(env, ARM_FEATURE_EL3) &&
+        ((env->cp15.scr_el3 & SCR_NS) || !(env->cp15.scr_el3 & SCR_EEL2))) {
         aa64 = aa64 && (env->cp15.scr_el3 & SCR_RW);
     }
 
@@ -3267,7 +3271,7 @@ typedef ARMCPU ArchCPU;
  * We put flags which are shared between 32 and 64 bit mode at the top
  * of the word, and flags which apply to only one mode at the bottom.
  *
- *  31          20    18    14          9              0
+ *  31          20    19    14          9              0
  * +--------------+-----+-----+----------+--------------+
  * |              |     |   TBFLAG_A32   |              |
  * |              |     +-----+----------+  TBFLAG_AM32 |
@@ -3316,6 +3320,7 @@ FIELD(TBFLAG_A32, HSTR_ACTIVE, 16, 1)
  * the same thing as the current security state of the processor!
  */
 FIELD(TBFLAG_A32, NS, 17, 1)
+FIELD(TBFLAG_A32, EEL2, 18, 1)
 
 /*
  * Bit usage when in AArch32 state, for M-profile only.
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 74fd759d48..dd3c6b52c1 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -532,6 +532,9 @@ static CPAccessResult access_trap_aa32s_el1(CPUARMState *env,
         return CP_ACCESS_OK;
     }
     if (arm_is_secure_below_el3(env)) {
+        if (env->cp15.scr_el3 & SCR_EEL2) {
+            return CP_ACCESS_TRAP_EL2;
+        }
         return CP_ACCESS_TRAP_EL3;
     }
     /* This will be EL1 NS and EL2 NS, which just UNDEF */
@@ -2030,6 +2033,9 @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
         if (cpu_isar_feature(aa64_pauth, cpu)) {
             valid_mask |= SCR_API | SCR_APK;
         }
+        if (cpu_isar_feature(aa64_sel2, cpu)) {
+            valid_mask |= SCR_EEL2;
+        }
         if (cpu_isar_feature(aa64_mte, cpu)) {
             valid_mask |= SCR_ATA;
         }
@@ -3390,13 +3396,16 @@ static CPAccessResult ats_access(CPUARMState *env, const ARMCPRegInfo *ri,
                                  bool isread)
 {
     if (ri->opc2 & 4) {
-        /* The ATS12NSO* operations must trap to EL3 if executed in
+        /* The ATS12NSO* operations must trap to EL3 or EL2 if executed in
          * Secure EL1 (which can only happen if EL3 is AArch64).
          * They are simply UNDEF if executed from NS EL1.
          * They function normally from EL2 or EL3.
          */
         if (arm_current_el(env) == 1) {
             if (arm_is_secure_below_el3(env)) {
+                if (env->cp15.scr_el3 & SCR_EEL2) {
+                    return CP_ACCESS_TRAP_UNCATEGORIZED_EL2;
+                }
                 return CP_ACCESS_TRAP_UNCATEGORIZED_EL3;
             }
             return CP_ACCESS_TRAP_UNCATEGORIZED;
@@ -5753,12 +5762,15 @@ static CPAccessResult nsacr_access(CPUARMState *env, const ARMCPRegInfo *ri,
                                    bool isread)
 {
     /* The NSACR is RW at EL3, and RO for NS EL1 and NS EL2.
-     * At Secure EL1 it traps to EL3.
+     * At Secure EL1 it traps to EL3 or EL2.
      */
     if (arm_current_el(env) == 3) {
         return CP_ACCESS_OK;
     }
     if (arm_is_secure_below_el3(env)) {
+        if (env->cp15.scr_el3 & SCR_EEL2) {
+            return CP_ACCESS_TRAP_EL2;
+        }
         return CP_ACCESS_TRAP_EL3;
     }
     /* Accesses from EL1 NS and EL2 NS are UNDEF for write but allow reads. */
@@ -12903,6 +12915,9 @@ static uint32_t rebuild_hflags_common_32(CPUARMState *env, int fp_el,
         flags = FIELD_DP32(flags, TBFLAG_ANY, BE_DATA, 1);
     }
     flags = FIELD_DP32(flags, TBFLAG_A32, NS, !access_secure_reg(env));
+    if (arm_is_secure_below_el3(env) && (env->cp15.scr_el3 & SCR_EEL2)) {
+        flags = FIELD_DP32(flags, TBFLAG_A32, EEL2, 1);
+    }
 
     return rebuild_hflags_common(env, fp_el, mmu_idx, flags);
 }
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 38371db540..3ec7ec7d1e 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -2703,9 +2703,10 @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn,
         }
         if (s->current_el == 1) {
             /* If we're in Secure EL1 (which implies that EL3 is AArch64)
-             * then accesses to Mon registers trap to EL3
+             * then accesses to Mon registers trap to Secure EL2 if it exists
+             * otherwise EL3.
              */
-            exc_target = 3;
+            exc_target = s->sel2 ? 2 : 3;
             goto undef;
         }
         break;
@@ -8723,6 +8724,7 @@ static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
         dc->sctlr_b = FIELD_EX32(tb_flags, TBFLAG_A32, SCTLR_B);
         dc->hstr_active = FIELD_EX32(tb_flags, TBFLAG_A32, HSTR_ACTIVE);
         dc->ns = FIELD_EX32(tb_flags, TBFLAG_A32, NS);
+        dc->sel2 = FIELD_EX32(tb_flags, TBFLAG_A32, EEL2);
         dc->vfp_enabled = FIELD_EX32(tb_flags, TBFLAG_A32, VFPEN);
         if (arm_feature(env, ARM_FEATURE_XSCALE)) {
             dc->c15_cpar = FIELD_EX32(tb_flags, TBFLAG_A32, XSCALE_CPAR);
diff --git a/target/arm/translate.h b/target/arm/translate.h
index 423b0e08df..bf3624791b 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -32,6 +32,7 @@ typedef struct DisasContext {
     uint8_t tbid;      /* TBI1|TBI0 for data */
     uint8_t tcma;      /* TCMA1|TCMA0 for MTE */
     bool ns;        /* Use non-secure CPREG bank on access */
+    bool sel2;      /* Secure EL2 enabled (only used in AArch32) */
     int fp_excp_el; /* FP exception EL or 0 if enabled */
     int sve_excp_el; /* SVE exception EL or 0 if enabled */
     int sve_len;     /* SVE vector length in bytes */
-- 
2.29.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 14/14] target/arm: enable Secure EL2 in max CPU
       [not found] <2172054.ElGaqSPkdT@basile.remlab.net>
                   ` (12 preceding siblings ...)
  2020-11-02 10:58 ` [PATCH 13/14] target/arm: add ARMv8.4-SEL2 extension remi.denis.courmont
@ 2020-11-02 10:58 ` remi.denis.courmont
  2020-11-03  7:38   ` Rémi Denis-Courmont
  2020-11-03 20:15   ` Richard Henderson
  13 siblings, 2 replies; 35+ messages in thread
From: remi.denis.courmont @ 2020-11-02 10:58 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/cpu64.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 649213082f..8c3749268e 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -641,6 +641,7 @@ static void aarch64_max_initfn(Object *obj)
         t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
         t = FIELD_DP64(t, ID_AA64PFR0, FP, 1);
         t = FIELD_DP64(t, ID_AA64PFR0, ADVSIMD, 1);
+        t = FIELD_DP64(t, ID_AA64PFR0, SEL2, 1);
         cpu->isar.id_aa64pfr0 = t;
 
         t = cpu->isar.id_aa64pfr1;
-- 
2.29.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH 01/14] target/arm: add arm_is_el2_enabled() helper
  2020-11-02 10:57 ` [PATCH 01/14] target/arm: add arm_is_el2_enabled() helper remi.denis.courmont
@ 2020-11-02 11:06   ` Peter Maydell
  2020-11-02 11:27     ` Peter Maydell
  2020-11-03 16:42   ` Richard Henderson
  1 sibling, 1 reply; 35+ messages in thread
From: Peter Maydell @ 2020-11-02 11:06 UTC (permalink / raw)
  To: remi.denis.courmont; +Cc: qemu-arm, QEMU Developers

On Mon, 2 Nov 2020 at 10:58, <remi.denis.courmont@huawei.com> wrote:
>
> From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
>
> This checks if EL2 is enabled (meaning EL2 registers take effects) in
> the current security context.
>
> Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/cpu.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index c18a916766..aaf3671806 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -2040,6 +2040,15 @@ static inline bool arm_is_secure(CPUARMState *env)
>      return arm_is_secure_below_el3(env);
>  }
>
> +/* Return true if the current security state has AArch64 EL2 or AArch32 Hyp */
> +static inline bool arm_is_el2_enabled(CPUARMState *env)
> +{
> +    if (arm_feature(env, ARM_FEATURE_EL2)) {
> +        return !arm_is_secure_below_el3(env);
> +    }
> +    return false;

You could usefully add a comment mentioning the EL2Enabled()
pseudocode and that we'll need to adjust this if we implement
Secure EL2 in future.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 01/14] target/arm: add arm_is_el2_enabled() helper
  2020-11-02 11:06   ` Peter Maydell
@ 2020-11-02 11:27     ` Peter Maydell
  2020-11-02 13:35       ` Remi Denis Courmont
  0 siblings, 1 reply; 35+ messages in thread
From: Peter Maydell @ 2020-11-02 11:27 UTC (permalink / raw)
  To: remi.denis.courmont; +Cc: qemu-arm, QEMU Developers

On Mon, 2 Nov 2020 at 11:06, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 2 Nov 2020 at 10:58, <remi.denis.courmont@huawei.com> wrote:
> >
> > From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> >
> > This checks if EL2 is enabled (meaning EL2 registers take effects) in
> > the current security context.
> >
> > Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> > ---
> >  target/arm/cpu.h | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> > index c18a916766..aaf3671806 100644
> > --- a/target/arm/cpu.h
> > +++ b/target/arm/cpu.h
> > @@ -2040,6 +2040,15 @@ static inline bool arm_is_secure(CPUARMState *env)
> >      return arm_is_secure_below_el3(env);
> >  }
> >
> > +/* Return true if the current security state has AArch64 EL2 or AArch32 Hyp */
> > +static inline bool arm_is_el2_enabled(CPUARMState *env)
> > +{
> > +    if (arm_feature(env, ARM_FEATURE_EL2)) {
> > +        return !arm_is_secure_below_el3(env);
> > +    }
> > +    return false;
>
> You could usefully add a comment mentioning the EL2Enabled()
> pseudocode and that we'll need to adjust this if we implement
> Secure EL2 in future.

I replied to this before the cover letter explaining what the
series is doing arrived in my inbox, so the second part of
my suggestion would be a bit pointless. The first part
(mentioning the name of the pseudocode function) still
seems worth doing.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 35+ messages in thread

* RE: [PATCH 01/14] target/arm: add arm_is_el2_enabled() helper
  2020-11-02 11:27     ` Peter Maydell
@ 2020-11-02 13:35       ` Remi Denis Courmont
  0 siblings, 0 replies; 35+ messages in thread
From: Remi Denis Courmont @ 2020-11-02 13:35 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-arm, QEMU Developers

Hi,

> I replied to this before the cover letter explaining what the series
> is doing arrived in my inbox, so the second part of my suggestion
> would be a bit pointless. The first part (mentioning the name of
> the pseudocode function) still seems worth doing.

Yeah well, some spam filter seems to helped itself to the original cover letter, had to resend it, so I figured.

And then it seems I made a booboo and accidentally dropped the first patch in the series, handling HCR.{F,I}MO bits.



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 14/14] target/arm: enable Secure EL2 in max CPU
  2020-11-02 10:58 ` [PATCH 14/14] target/arm: enable Secure EL2 in max CPU remi.denis.courmont
@ 2020-11-03  7:38   ` Rémi Denis-Courmont
  2020-11-03 16:38     ` Richard Henderson
  2020-11-03 20:15   ` Richard Henderson
  1 sibling, 1 reply; 35+ messages in thread
From: Rémi Denis-Courmont @ 2020-11-03  7:38 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

Le maanantaina 2. marraskuuta 2020, 12.58.02 EET 
remi.denis.courmont@huawei.com a écrit :
> From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/cpu64.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 649213082f..8c3749268e 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -641,6 +641,7 @@ static void aarch64_max_initfn(Object *obj)
>          t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
>          t = FIELD_DP64(t, ID_AA64PFR0, FP, 1);
>          t = FIELD_DP64(t, ID_AA64PFR0, ADVSIMD, 1);
> +        t = FIELD_DP64(t, ID_AA64PFR0, SEL2, 1);
>          cpu->isar.id_aa64pfr0 = t;
> 
>          t = cpu->isar.id_aa64pfr1;

Answering my own patch as I have a policy question here...

This exposes SEL2 without TTST (small translation tables). On a logical level, 
the two extensions are orthogonal. But per DDI0487, SEL2 implies TTST, so I am 
not sure if this is considered an acceptable deviation in QEMU, or if 
implementing TTST is deemed necessary.

Note that there's what seems like an editorial error in the spec: VSTCR 
documentation covers the scenario that TTST is not supported by the CPU, even 
though then VSTCR should not exist.

-- 
Реми Дёни-Курмон
http://www.remlab.net/





^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 14/14] target/arm: enable Secure EL2 in max CPU
  2020-11-03  7:38   ` Rémi Denis-Courmont
@ 2020-11-03 16:38     ` Richard Henderson
  0 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2020-11-03 16:38 UTC (permalink / raw)
  To: Rémi Denis-Courmont, qemu-arm; +Cc: qemu-devel

On 11/2/20 11:38 PM, Rémi Denis-Courmont wrote:
> Answering my own patch as I have a policy question here...
> 
> This exposes SEL2 without TTST (small translation tables). On a logical level, 
> the two extensions are orthogonal. But per DDI0487, SEL2 implies TTST, so I am 
> not sure if this is considered an acceptable deviation in QEMU, or if 
> implementing TTST is deemed necessary.

We should implement TTST, yes.  I don't think we need to be 100% strict on the
ordering, so long as they are both done for the next qemu release in the
spring.  I don't think it should be difficult.

FWIW, we left aarch32 fp16 unimplemented for quite some time, even though that
was required for full compliance with aarch64 fp16.

> Note that there's what seems like an editorial error in the spec: VSTCR 
> documentation covers the scenario that TTST is not supported by the CPU, even 
> though then VSTCR should not exist.

Indeed.  ;-)


r~



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 01/14] target/arm: add arm_is_el2_enabled() helper
  2020-11-02 10:57 ` [PATCH 01/14] target/arm: add arm_is_el2_enabled() helper remi.denis.courmont
  2020-11-02 11:06   ` Peter Maydell
@ 2020-11-03 16:42   ` Richard Henderson
  1 sibling, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2020-11-03 16:42 UTC (permalink / raw)
  To: remi.denis.courmont, qemu-arm; +Cc: qemu-devel

On 11/2/20 2:57 AM, remi.denis.courmont@huawei.com wrote:
> From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> This checks if EL2 is enabled (meaning EL2 registers take effects) in
> the current security context.
> 
> Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/cpu.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 02/14] target/arm: use arm_is_el2_enabled() where applicable
  2020-11-02 10:57 ` [PATCH 02/14] target/arm: use arm_is_el2_enabled() where applicable remi.denis.courmont
@ 2020-11-03 16:53   ` Richard Henderson
  0 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2020-11-03 16:53 UTC (permalink / raw)
  To: remi.denis.courmont, qemu-arm; +Cc: qemu-devel

On 11/2/20 2:57 AM, remi.denis.courmont@huawei.com wrote:
> From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> Do not assume that EL2 is available in non-secure context.
> That equivalence is broken by ARMv8.4-SEL2.
> 
> Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/cpu.h        |  4 ++--
>  target/arm/helper-a64.c |  8 +-------
>  target/arm/helper.c     | 33 +++++++++++++--------------------
>  3 files changed, 16 insertions(+), 29 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 03/14] target/arm: use arm_hcr_el2_eff() where applicable
  2020-11-02 10:57 ` [PATCH 03/14] target/arm: use arm_hcr_el2_eff() " remi.denis.courmont
@ 2020-11-03 16:56   ` Richard Henderson
  0 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2020-11-03 16:56 UTC (permalink / raw)
  To: remi.denis.courmont, qemu-arm; +Cc: qemu-devel

On 11/2/20 2:57 AM, remi.denis.courmont@huawei.com wrote:
> From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> This will simplify accessing HCR conditionally in secure state.
> 
> Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/helper.c | 30 +++++++++++++++++-------------
>  1 file changed, 17 insertions(+), 13 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 04/14] target/arm: factor MDCR_EL2 common handling
  2020-11-02 10:57 ` [PATCH 04/14] target/arm: factor MDCR_EL2 common handling remi.denis.courmont
@ 2020-11-03 17:00   ` Richard Henderson
  0 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2020-11-03 17:00 UTC (permalink / raw)
  To: remi.denis.courmont, qemu-arm; +Cc: qemu-devel

On 11/2/20 2:57 AM, remi.denis.courmont@huawei.com wrote:
> From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> This adds a common helper to compute the effective value of MDCR_EL2.
> That is the actual value if EL2 is enabled in the current security
> context, or 0 elsewise.
> 
> Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/helper.c | 38 ++++++++++++++++++++++----------------
>  1 file changed, 22 insertions(+), 16 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 05/14] target/arm: declare new AA64PFR0 bit-fields
  2020-11-02 10:57 ` [PATCH 05/14] target/arm: declare new AA64PFR0 bit-fields remi.denis.courmont
@ 2020-11-03 17:02   ` Richard Henderson
  0 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2020-11-03 17:02 UTC (permalink / raw)
  To: remi.denis.courmont, qemu-arm; +Cc: qemu-devel

On 11/2/20 2:57 AM, remi.denis.courmont@huawei.com wrote:
> From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/cpu.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 07/14] target/arm: return the stage 2 index for stage 1
  2020-11-02 10:57 ` [PATCH 07/14] target/arm: return the stage 2 index for stage 1 remi.denis.courmont
@ 2020-11-03 17:04   ` Richard Henderson
  0 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2020-11-03 17:04 UTC (permalink / raw)
  To: remi.denis.courmont, qemu-arm; +Cc: qemu-devel

On 11/2/20 2:57 AM, remi.denis.courmont@huawei.com wrote:
> +        if (s2_mmu_idx != NULL)
> +            *s2_mmu_idx = ARMMMUIdx_Stage2;

Missing braces.  Otherwise,

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 08/14] target/arm: add MMU stage 1 for Secure EL2
  2020-11-02 10:57 ` [PATCH 08/14] target/arm: add MMU stage 1 for Secure EL2 remi.denis.courmont
@ 2020-11-03 18:32   ` Richard Henderson
  2020-11-03 18:49     ` Rémi Denis-Courmont
  0 siblings, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2020-11-03 18:32 UTC (permalink / raw)
  To: remi.denis.courmont, qemu-arm; +Cc: qemu-devel

On 11/2/20 2:57 AM, remi.denis.courmont@huawei.com wrote:
> From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> This adds the MMU indices for EL2 stage 1 in secure mode.
> 
> To keep code contained, which is largelly identical between secure and
> non-secure modes, this patch introduces a secure bit for all new and
> existing stage 1 translation regimes.
> 
> Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/cpu-param.h     |   2 +-
>  target/arm/cpu.h           |  22 ++++--
>  target/arm/helper.c        | 143 ++++++++++++++++++++++++-------------
>  target/arm/internals.h     |  12 ++++
>  target/arm/translate-a64.c |   4 ++
>  5 files changed, 127 insertions(+), 56 deletions(-)
> 
> diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
> index 6321385b46..0db5e37c17 100644
> --- a/target/arm/cpu-param.h
> +++ b/target/arm/cpu-param.h
> @@ -29,6 +29,6 @@
>  # define TARGET_PAGE_BITS_MIN  10
>  #endif
>  
> -#define NB_MMU_MODES 11
> +#define NB_MMU_MODES 16
>  
>  #endif
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 724b11ee57..3fbb70e273 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -2944,6 +2944,9 @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync);
>  #define ARM_MMU_IDX_NOTLB 0x20  /* does not have a TLB */
>  #define ARM_MMU_IDX_M     0x40  /* M profile */
>  
> +/* Meanings of the bits for A profile mmu idx values */
> +#define ARM_MMU_IDX_A_S      0x8
> +
>  /* Meanings of the bits for M profile mmu idx values */
>  #define ARM_MMU_IDX_M_PRIV   0x1
>  #define ARM_MMU_IDX_M_NEGPRI 0x2
> @@ -2967,10 +2970,17 @@ typedef enum ARMMMUIdx {
>      ARMMMUIdx_E20_2      =  5 | ARM_MMU_IDX_A,
>      ARMMMUIdx_E20_2_PAN  =  6 | ARM_MMU_IDX_A,
>  
> -    ARMMMUIdx_SE10_0     = 7 | ARM_MMU_IDX_A,
> -    ARMMMUIdx_SE10_1     = 8 | ARM_MMU_IDX_A,
> -    ARMMMUIdx_SE10_1_PAN = 9 | ARM_MMU_IDX_A,
> -    ARMMMUIdx_SE3        = 10 | ARM_MMU_IDX_A,
> +    ARMMMUIdx_SE10_0     = ARMMMUIdx_E10_0 | ARM_MMU_IDX_A_S,
> +    ARMMMUIdx_SE20_0     = ARMMMUIdx_E20_0 | ARM_MMU_IDX_A_S,
> +
> +    ARMMMUIdx_SE10_1     = ARMMMUIdx_E10_1 | ARM_MMU_IDX_A_S,
> +    ARMMMUIdx_SE10_1_PAN = ARMMMUIdx_E10_1_PAN | ARM_MMU_IDX_A_S,
> +
> +    ARMMMUIdx_SE2        = ARMMMUIdx_E2 | ARM_MMU_IDX_A_S,
> +    ARMMMUIdx_SE20_2     = ARMMMUIdx_E20_2 | ARM_MMU_IDX_A_S,
> +    ARMMMUIdx_SE20_2_PAN = ARMMMUIdx_E20_2_PAN | ARM_MMU_IDX_A_S,
> +
> +    ARMMMUIdx_SE3        = 15 | ARM_MMU_IDX_A,

Hum.  So, we're adding 4 new mmu_idx, and increasing the mmu_idx count by 5.
The unused index would be 7 -- no non-secure el3.

Is it worth reversing the S bit to NS, so that index 15 becomes the one that is
unused, and so we don't actually have to add it to NB_MMU_MODES?

> @@ -3649,7 +3655,7 @@ static void ats1h_write(CPUARMState *env, const ARMCPRegInfo *ri,
>  static CPAccessResult at_s1e2_access(CPUARMState *env, const ARMCPRegInfo *ri,
>                                       bool isread)
>  {
> -    if (arm_current_el(env) == 3 && !(env->cp15.scr_el3 & SCR_NS)) {
> +    if (arm_current_el(env) == 3 && !(env->cp15.scr_el3 & (SCR_NS|SCR_EEL2))) {

This seems to belong to a different patch?

> +        if (arm_is_secure_below_el3(env))
> +            mask <<= ARM_MMU_IDX_A_S;

Braces.

>      if (raw_read(env, ri) != value) {
> -        tlb_flush_by_mmuidx(cs,
> -                            ARMMMUIdxBit_E10_1 |
> -                            ARMMMUIdxBit_E10_1_PAN |
> -                            ARMMMUIdxBit_E10_0);
> +        uint16_t mask = ARMMMUIdxBit_E10_1 |
> +                        ARMMMUIdxBit_E10_1_PAN |
> +                        ARMMMUIdxBit_E10_0;
> +
> +        if (arm_is_secure_below_el3(env))
> +            mask <<= ARM_MMU_IDX_A_S;

Again.  This appears to be an existing bug with SEL1?  Perhaps it should be
split to a separate patch.

>      if ((hcr & (HCR_E2H | HCR_TGE)) == (HCR_E2H | HCR_TGE)) {
> -        return ARMMMUIdxBit_E20_2 |
> +        mask = ARMMMUIdxBit_E20_2 |
>                 ARMMMUIdxBit_E20_2_PAN |
>                 ARMMMUIdxBit_E20_0;
> -    } else if (arm_is_secure_below_el3(env)) {
> -        return ARMMMUIdxBit_SE10_1 |
> -               ARMMMUIdxBit_SE10_1_PAN |
> -               ARMMMUIdxBit_SE10_0;
>      } else {
> -        return ARMMMUIdxBit_E10_1 |
> +        mask = ARMMMUIdxBit_E10_1 |
>                 ARMMMUIdxBit_E10_1_PAN |
>                 ARMMMUIdxBit_E10_0;
>      }
> +
> +    if (arm_is_secure_below_el3(env)) {
> +        mask <<= ARM_MMU_IDX_A_S;
> +    }

Likewise.

The rest of the patch looks mechanically correct.


r~


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 08/14] target/arm: add MMU stage 1 for Secure EL2
  2020-11-03 18:32   ` Richard Henderson
@ 2020-11-03 18:49     ` Rémi Denis-Courmont
  2020-11-03 19:41       ` Richard Henderson
  0 siblings, 1 reply; 35+ messages in thread
From: Rémi Denis-Courmont @ 2020-11-03 18:49 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

Le tiistaina 3. marraskuuta 2020, 20.32.21 EET Richard Henderson a écrit :
> On 11/2/20 2:57 AM, remi.denis.courmont@huawei.com wrote:
> > From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> > 
> > This adds the MMU indices for EL2 stage 1 in secure mode.
> > 
> > To keep code contained, which is largelly identical between secure and
> > non-secure modes, this patch introduces a secure bit for all new and
> > existing stage 1 translation regimes.
> > 
> > Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> > ---
> > 
> >  target/arm/cpu-param.h     |   2 +-
> >  target/arm/cpu.h           |  22 ++++--
> >  target/arm/helper.c        | 143 ++++++++++++++++++++++++-------------
> >  target/arm/internals.h     |  12 ++++
> >  target/arm/translate-a64.c |   4 ++
> >  5 files changed, 127 insertions(+), 56 deletions(-)
> > 
> > diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
> > index 6321385b46..0db5e37c17 100644
> > --- a/target/arm/cpu-param.h
> > +++ b/target/arm/cpu-param.h
> > @@ -29,6 +29,6 @@
> > 
> >  # define TARGET_PAGE_BITS_MIN  10
> >  #endif
> > 
> > -#define NB_MMU_MODES 11
> > +#define NB_MMU_MODES 16
> > 
> >  #endif
> > 
> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> > index 724b11ee57..3fbb70e273 100644
> > --- a/target/arm/cpu.h
> > +++ b/target/arm/cpu.h
> > @@ -2944,6 +2944,9 @@ bool write_cpustate_to_list(ARMCPU *cpu, bool
> > kvm_sync);> 
> >  #define ARM_MMU_IDX_NOTLB 0x20  /* does not have a TLB */
> >  #define ARM_MMU_IDX_M     0x40  /* M profile */
> > 
> > +/* Meanings of the bits for A profile mmu idx values */
> > +#define ARM_MMU_IDX_A_S      0x8
> > +
> > 
> >  /* Meanings of the bits for M profile mmu idx values */
> >  #define ARM_MMU_IDX_M_PRIV   0x1
> >  #define ARM_MMU_IDX_M_NEGPRI 0x2
> > 
> > @@ -2967,10 +2970,17 @@ typedef enum ARMMMUIdx {
> > 
> >      ARMMMUIdx_E20_2      =  5 | ARM_MMU_IDX_A,
> >      ARMMMUIdx_E20_2_PAN  =  6 | ARM_MMU_IDX_A,
> > 
> > -    ARMMMUIdx_SE10_0     = 7 | ARM_MMU_IDX_A,
> > -    ARMMMUIdx_SE10_1     = 8 | ARM_MMU_IDX_A,
> > -    ARMMMUIdx_SE10_1_PAN = 9 | ARM_MMU_IDX_A,
> > -    ARMMMUIdx_SE3        = 10 | ARM_MMU_IDX_A,
> > +    ARMMMUIdx_SE10_0     = ARMMMUIdx_E10_0 | ARM_MMU_IDX_A_S,
> > +    ARMMMUIdx_SE20_0     = ARMMMUIdx_E20_0 | ARM_MMU_IDX_A_S,
> > +
> > +    ARMMMUIdx_SE10_1     = ARMMMUIdx_E10_1 | ARM_MMU_IDX_A_S,
> > +    ARMMMUIdx_SE10_1_PAN = ARMMMUIdx_E10_1_PAN | ARM_MMU_IDX_A_S,
> > +
> > +    ARMMMUIdx_SE2        = ARMMMUIdx_E2 | ARM_MMU_IDX_A_S,
> > +    ARMMMUIdx_SE20_2     = ARMMMUIdx_E20_2 | ARM_MMU_IDX_A_S,
> > +    ARMMMUIdx_SE20_2_PAN = ARMMMUIdx_E20_2_PAN | ARM_MMU_IDX_A_S,
> > +
> > +    ARMMMUIdx_SE3        = 15 | ARM_MMU_IDX_A,
> 
> Hum.  So, we're adding 4 new mmu_idx, and increasing the mmu_idx count by 5.
> The unused index would be 7 -- no non-secure el3.
> 
> Is it worth reversing the S bit to NS, so that index 15 becomes the one that
> is unused, and so we don't actually have to add it to NB_MMU_MODES?

Possible. It would save a few hundred bytes from a quick glance.

It could also be argued that E2 and E20_2 should be one and the same. The 
regimes are distinct but they cannot coexist. The mode's TLB mode could be 
flushed when HCR.E2H is flipped, I guess.


-- 
Rémi Denis-Courmont




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 08/14] target/arm: add MMU stage 1 for Secure EL2
  2020-11-03 18:49     ` Rémi Denis-Courmont
@ 2020-11-03 19:41       ` Richard Henderson
  0 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2020-11-03 19:41 UTC (permalink / raw)
  To: Rémi Denis-Courmont, qemu-arm; +Cc: qemu-devel

On 11/3/20 10:49 AM, Rémi Denis-Courmont wrote:
> It could also be argued that E2 and E20_2 should be one and the same. The 
> regimes are distinct but they cannot coexist. The mode's TLB mode could be 
> flushed when HCR.E2H is flipped, I guess.

I don't think so, as there are other implications.  E.g. regime_has_2_ranges.
If we merged these two, then we would have to differentiate them in some other way.


r~


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 09/14] target/arm: add ARMv8.4-SEL2 system registers
  2020-11-02 10:57 ` [PATCH 09/14] target/arm: add ARMv8.4-SEL2 system registers remi.denis.courmont
@ 2020-11-03 19:49   ` Richard Henderson
  2020-11-03 21:09     ` Peter Maydell
  0 siblings, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2020-11-03 19:49 UTC (permalink / raw)
  To: remi.denis.courmont, qemu-arm; +Cc: qemu-devel

On 11/2/20 2:57 AM, remi.denis.courmont@huawei.com wrote:
> From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/cpu.h    |  2 ++
>  target/arm/helper.c | 24 ++++++++++++++++++++++++
>  2 files changed, 26 insertions(+)

These need a new VMStateDescription section in machine.c.


r~


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 10/14] target/arm: do S1_ptw_translate() before address space lookup
  2020-11-02 10:57 ` [PATCH 10/14] target/arm: do S1_ptw_translate() before address space lookup remi.denis.courmont
@ 2020-11-03 19:54   ` Richard Henderson
  2020-11-03 21:21     ` Rémi Denis-Courmont
  0 siblings, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2020-11-03 19:54 UTC (permalink / raw)
  To: remi.denis.courmont, qemu-arm; +Cc: qemu-devel

On 11/2/20 2:57 AM, remi.denis.courmont@huawei.com wrote:
> From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> In the secure stage 2 translation regime, the VSTCR.SW and VTCR.NSW
> bits can invert the secure flag for pagetable walks. This patchset
> allows S1_ptw_translate() to change the non-secure bit.
> 
> Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/helper.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 4c86e4f57c..7c70460e65 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -10403,7 +10403,7 @@ static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx,
>  
>  /* Translate a S1 pagetable walk through S2 if needed.  */
>  static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
> -                               hwaddr addr, MemTxAttrs txattrs,
> +                               hwaddr addr, bool *is_secure,
>                                 ARMMMUFaultInfo *fi)
>  {
>      ARMMMUIdx s2_mmu_idx;
> @@ -10415,6 +10415,9 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
>          int s2prot;
>          int ret;
>          ARMCacheAttrs cacheattrs = {};
> +        MemTxAttrs txattrs = {};
> +
> +        assert(!*is_secure); /* TODO: S-EL2 */


Are you sure that you don't want to pass in txattrs via pointer instead?  This
change by itself looks questionable.  I guess I'll have to look forward to the
other patch...


r~


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 13/14] target/arm: add ARMv8.4-SEL2 extension
  2020-11-02 10:58 ` [PATCH 13/14] target/arm: add ARMv8.4-SEL2 extension remi.denis.courmont
@ 2020-11-03 20:14   ` Richard Henderson
  0 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2020-11-03 20:14 UTC (permalink / raw)
  To: remi.denis.courmont, qemu-arm; +Cc: qemu-devel

On 11/2/20 2:58 AM, remi.denis.courmont@huawei.com wrote:
> From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> This adds handling for the SCR_EL3.EEL2 bit.
> 
> A translation block flag is added in A32 mode to route exceptions
> correctly from AArch32 S-EL1 to (AArch64) S-EL2.
> 
> Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/cpu.c       |  2 +-
>  target/arm/cpu.h       | 11 ++++++++---
>  target/arm/helper.c    | 19 +++++++++++++++++--
>  target/arm/translate.c |  6 ++++--
>  target/arm/translate.h |  1 +
>  5 files changed, 31 insertions(+), 8 deletions(-)

Annoying that a new hflags bit is required, but I don't see a good way around that.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 14/14] target/arm: enable Secure EL2 in max CPU
  2020-11-02 10:58 ` [PATCH 14/14] target/arm: enable Secure EL2 in max CPU remi.denis.courmont
  2020-11-03  7:38   ` Rémi Denis-Courmont
@ 2020-11-03 20:15   ` Richard Henderson
  1 sibling, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2020-11-03 20:15 UTC (permalink / raw)
  To: remi.denis.courmont, qemu-arm; +Cc: qemu-devel

On 11/2/20 2:58 AM, remi.denis.courmont@huawei.com wrote:
> From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/cpu64.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 09/14] target/arm: add ARMv8.4-SEL2 system registers
  2020-11-03 19:49   ` Richard Henderson
@ 2020-11-03 21:09     ` Peter Maydell
  2020-11-03 21:40       ` Richard Henderson
  0 siblings, 1 reply; 35+ messages in thread
From: Peter Maydell @ 2020-11-03 21:09 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-arm, QEMU Developers, remi.denis.courmont

On Tue, 3 Nov 2020 at 19:50, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 11/2/20 2:57 AM, remi.denis.courmont@huawei.com wrote:
> > From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> >
> > Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> > ---
> >  target/arm/cpu.h    |  2 ++
> >  target/arm/helper.c | 24 ++++++++++++++++++++++++
> >  2 files changed, 26 insertions(+)
>
> These need a new VMStateDescription section in machine.c.

Do they? I think they're handled by the existing generic
migration-of-sysregs code...

thanks
-- PMM


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 10/14] target/arm: do S1_ptw_translate() before address space lookup
  2020-11-03 19:54   ` Richard Henderson
@ 2020-11-03 21:21     ` Rémi Denis-Courmont
  0 siblings, 0 replies; 35+ messages in thread
From: Rémi Denis-Courmont @ 2020-11-03 21:21 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

Le tiistaina 3. marraskuuta 2020, 21.54.48 EET Richard Henderson a écrit :
> On 11/2/20 2:57 AM, remi.denis.courmont@huawei.com wrote:
> > From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> > 
> > In the secure stage 2 translation regime, the VSTCR.SW and VTCR.NSW
> > bits can invert the secure flag for pagetable walks. This patchset
> > allows S1_ptw_translate() to change the non-secure bit.
> > 
> > Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> > ---
> > 
> >  target/arm/helper.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target/arm/helper.c b/target/arm/helper.c
> > index 4c86e4f57c..7c70460e65 100644
> > --- a/target/arm/helper.c
> > +++ b/target/arm/helper.c
> > @@ -10403,7 +10403,7 @@ static bool get_level1_table_address(CPUARMState
> > *env, ARMMMUIdx mmu_idx,> 
> >  /* Translate a S1 pagetable walk through S2 if needed.  */
> >  static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
> > 
> > -                               hwaddr addr, MemTxAttrs txattrs,
> > +                               hwaddr addr, bool *is_secure,
> > 
> >                                 ARMMMUFaultInfo *fi)
> >  
> >  {
> >  
> >      ARMMMUIdx s2_mmu_idx;
> > 
> > @@ -10415,6 +10415,9 @@ static hwaddr S1_ptw_translate(CPUARMState *env,
> > ARMMMUIdx mmu_idx,> 
> >          int s2prot;
> >          int ret;
> >          ARMCacheAttrs cacheattrs = {};
> > 
> > +        MemTxAttrs txattrs = {};
> > +
> > +        assert(!*is_secure); /* TODO: S-EL2 */
> 
> Are you sure that you don't want to pass in txattrs via pointer instead?

That's possible too, and more like the existing code. Though I thought it 
clearer to pass only a pointer to the secure bit in/out, seen as that's the 
only in/out parameter.

> This change by itself looks questionable.  I guess I'll have to look
> forward to the other patch...


-- 
Rémi Denis-Courmont




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 09/14] target/arm: add ARMv8.4-SEL2 system registers
  2020-11-03 21:09     ` Peter Maydell
@ 2020-11-03 21:40       ` Richard Henderson
  0 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2020-11-03 21:40 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-arm, QEMU Developers, remi.denis.courmont

On 11/3/20 1:09 PM, Peter Maydell wrote:
> On Tue, 3 Nov 2020 at 19:50, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> On 11/2/20 2:57 AM, remi.denis.courmont@huawei.com wrote:
>>> From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
>>>
>>> Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
>>> ---
>>>  target/arm/cpu.h    |  2 ++
>>>  target/arm/helper.c | 24 ++++++++++++++++++++++++
>>>  2 files changed, 26 insertions(+)
>>
>> These need a new VMStateDescription section in machine.c.
> 
> Do they? I think they're handled by the existing generic
> migration-of-sysregs code...

Doh, yes indeed.  ARM magic...


r~


^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2020-11-03 21:41 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <2172054.ElGaqSPkdT@basile.remlab.net>
2020-11-02 10:57 ` [PATCH 01/14] target/arm: add arm_is_el2_enabled() helper remi.denis.courmont
2020-11-02 11:06   ` Peter Maydell
2020-11-02 11:27     ` Peter Maydell
2020-11-02 13:35       ` Remi Denis Courmont
2020-11-03 16:42   ` Richard Henderson
2020-11-02 10:57 ` [PATCH 02/14] target/arm: use arm_is_el2_enabled() where applicable remi.denis.courmont
2020-11-03 16:53   ` Richard Henderson
2020-11-02 10:57 ` [PATCH 03/14] target/arm: use arm_hcr_el2_eff() " remi.denis.courmont
2020-11-03 16:56   ` Richard Henderson
2020-11-02 10:57 ` [PATCH 04/14] target/arm: factor MDCR_EL2 common handling remi.denis.courmont
2020-11-03 17:00   ` Richard Henderson
2020-11-02 10:57 ` [PATCH 05/14] target/arm: declare new AA64PFR0 bit-fields remi.denis.courmont
2020-11-03 17:02   ` Richard Henderson
2020-11-02 10:57 ` [PATCH 06/14] target/arm: add 64-bit S-EL2 to EL exception table remi.denis.courmont
2020-11-02 10:57 ` [PATCH 07/14] target/arm: return the stage 2 index for stage 1 remi.denis.courmont
2020-11-03 17:04   ` Richard Henderson
2020-11-02 10:57 ` [PATCH 08/14] target/arm: add MMU stage 1 for Secure EL2 remi.denis.courmont
2020-11-03 18:32   ` Richard Henderson
2020-11-03 18:49     ` Rémi Denis-Courmont
2020-11-03 19:41       ` Richard Henderson
2020-11-02 10:57 ` [PATCH 09/14] target/arm: add ARMv8.4-SEL2 system registers remi.denis.courmont
2020-11-03 19:49   ` Richard Henderson
2020-11-03 21:09     ` Peter Maydell
2020-11-03 21:40       ` Richard Henderson
2020-11-02 10:57 ` [PATCH 10/14] target/arm: do S1_ptw_translate() before address space lookup remi.denis.courmont
2020-11-03 19:54   ` Richard Henderson
2020-11-03 21:21     ` Rémi Denis-Courmont
2020-11-02 10:57 ` [PATCH 11/14] target/arm: secure stage 2 translation regime remi.denis.courmont
2020-11-02 10:58 ` [PATCH 12/14] target/arm: set HPFAR_EL2.NS on secure stage 2 faults remi.denis.courmont
2020-11-02 10:58 ` [PATCH 13/14] target/arm: add ARMv8.4-SEL2 extension remi.denis.courmont
2020-11-03 20:14   ` Richard Henderson
2020-11-02 10:58 ` [PATCH 14/14] target/arm: enable Secure EL2 in max CPU remi.denis.courmont
2020-11-03  7:38   ` Rémi Denis-Courmont
2020-11-03 16:38     ` Richard Henderson
2020-11-03 20:15   ` Richard Henderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.