diff mbox series

[v4,10/40] target/arm: Rename ARMMMUIdx_S1NSE* to ARMMMUIdx_Stage1_E*

Message ID 20191203022937.1474-11-richard.henderson@linaro.org
State New
Headers show
Series target/arm: Implement ARMv8.1-VHE | expand

Commit Message

Richard Henderson Dec. 3, 2019, 2:29 a.m. UTC 
This is part of a reorganization to the set of mmu_idx.
The EL1&0 regime is the only one that uses 2-stage translation.
Spelling out Stage avoids confusion with Secure.

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

---
 target/arm/cpu.h       |  4 ++--
 target/arm/internals.h |  6 +++---
 target/arm/helper.c    | 27 ++++++++++++++-------------
 3 files changed, 19 insertions(+), 18 deletions(-)

-- 
2.17.1

Comments

Alex Bennée Dec. 4, 2019, 11 a.m. UTC | #1 
Richard Henderson <richard.henderson@linaro.org> writes:

> This is part of a reorganization to the set of mmu_idx.

> The EL1&0 regime is the only one that uses 2-stage translation.

> Spelling out Stage avoids confusion with Secure.

>

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

<snip>
> diff --git a/target/arm/helper.c b/target/arm/helper.c

> index 97677f8482..a34accec20 100644

> --- a/target/arm/helper.c

> +++ b/target/arm/helper.c

> @@ -2992,7 +2992,8 @@ 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)

> -            && (mmu_idx == ARMMMUIdx_S1NSE1 || mmu_idx == ARMMMUIdx_S1NSE0)) {

> +            && (mmu_idx == ARMMMUIdx_Stage1_E1

> +                || mmu_idx == ARMMMUIdx_Stage1_E0)) {


Personal nit: I think ||\nfoo == scans more nicely as it lines up but
maybe that's just me.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


-- 
Alex Bennée
Peter Maydell Dec. 6, 2019, 3:47 p.m. UTC | #2 
On Tue, 3 Dec 2019 at 02:29, Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> This is part of a reorganization to the set of mmu_idx.

> The EL1&0 regime is the only one that uses 2-stage translation.

> Spelling out Stage avoids confusion with Secure.


If you didn't delete the 'NS' from the name then it
wouldn't be confusing...

-- PMM
Richard Henderson Dec. 6, 2019, 6:20 p.m. UTC | #3 
On 12/6/19 7:47 AM, Peter Maydell wrote:
> On Tue, 3 Dec 2019 at 02:29, Richard Henderson

> <richard.henderson@linaro.org> wrote:

>>

>> This is part of a reorganization to the set of mmu_idx.

>> The EL1&0 regime is the only one that uses 2-stage translation.

>> Spelling out Stage avoids confusion with Secure.

> 

> If you didn't delete the 'NS' from the name then it

> wouldn't be confusing...


It is not obvious to my eye how to break up S1NSE0 -- 'NS' is buried in the
middle.  It might be different if these names were architectural, as if the AT
names, but they're not.

I think it is clearer to reserve 'S' for 'Secure', and use it consistently.
E.g. not the current "S1E3" (stage 1 e3), but "SE3" (secure e3).

I think it is clearer to simply document that, unless "Stage" is present in the
name, the tlb contains the complete translation.  This makes "S12" redundant
off of "NSE0", and "S1" off of "E3".

We'll still need a renaming of with SecEL2, since "SE0" is no longer complete;
we will need to specify "Secure EL1&0" vs "Secure EL2&0".  Thus perhaps
"SEL10_0" to match the "EL10_0" name that I'm using in this series.


r~
diff mbox series

Patch

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index fdb868f2e9..0714c52176 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2882,8 +2882,8 @@  typedef enum ARMMMUIdx {
     /* Indexes below here don't have TLBs and are used only for AT system
      * instructions or for the first stage of an S12 page table walk.
      */
-    ARMMMUIdx_S1NSE0 = 0 | ARM_MMU_IDX_NOTLB,
-    ARMMMUIdx_S1NSE1 = 1 | ARM_MMU_IDX_NOTLB,
+    ARMMMUIdx_Stage1_E0 = 0 | ARM_MMU_IDX_NOTLB,
+    ARMMMUIdx_Stage1_E1 = 1 | ARM_MMU_IDX_NOTLB,
 } ARMMMUIdx;
 
 /* Bit macros for the core-mmu-index values for each index,
diff --git a/target/arm/internals.h b/target/arm/internals.h
index ca8be78bbf..3fd1518f3b 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -810,8 +810,8 @@  static inline bool regime_is_secure(CPUARMState *env, ARMMMUIdx mmu_idx)
     switch (mmu_idx) {
     case ARMMMUIdx_EL10_0:
     case ARMMMUIdx_EL10_1:
-    case ARMMMUIdx_S1NSE0:
-    case ARMMMUIdx_S1NSE1:
+    case ARMMMUIdx_Stage1_E0:
+    case ARMMMUIdx_Stage1_E1:
     case ARMMMUIdx_S1E2:
     case ARMMMUIdx_Stage2:
     case ARMMMUIdx_MPrivNegPri:
@@ -975,7 +975,7 @@  ARMMMUIdx arm_mmu_idx(CPUARMState *env);
 #ifdef CONFIG_USER_ONLY
 static inline ARMMMUIdx arm_stage1_mmu_idx(CPUARMState *env)
 {
-    return ARMMMUIdx_S1NSE0;
+    return ARMMMUIdx_Stage1_E0;
 }
 #else
 ARMMMUIdx arm_stage1_mmu_idx(CPUARMState *env);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 97677f8482..a34accec20 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2992,7 +2992,8 @@  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)
-            && (mmu_idx == ARMMMUIdx_S1NSE1 || mmu_idx == ARMMMUIdx_S1NSE0)) {
+            && (mmu_idx == ARMMMUIdx_Stage1_E1
+                || mmu_idx == ARMMMUIdx_Stage1_E0)) {
             /*
              * Synchronous stage 2 fault on an access made as part of the
              * translation table walk for AT S1E0* or AT S1E1* insn
@@ -3140,10 +3141,10 @@  static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
             mmu_idx = ARMMMUIdx_S1E3;
             break;
         case 2:
-            mmu_idx = ARMMMUIdx_S1NSE1;
+            mmu_idx = ARMMMUIdx_Stage1_E1;
             break;
         case 1:
-            mmu_idx = secure ? ARMMMUIdx_S1SE1 : ARMMMUIdx_S1NSE1;
+            mmu_idx = secure ? ARMMMUIdx_S1SE1 : ARMMMUIdx_Stage1_E1;
             break;
         default:
             g_assert_not_reached();
@@ -3156,10 +3157,10 @@  static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
             mmu_idx = ARMMMUIdx_S1SE0;
             break;
         case 2:
-            mmu_idx = ARMMMUIdx_S1NSE0;
+            mmu_idx = ARMMMUIdx_Stage1_E0;
             break;
         case 1:
-            mmu_idx = secure ? ARMMMUIdx_S1SE0 : ARMMMUIdx_S1NSE0;
+            mmu_idx = secure ? ARMMMUIdx_S1SE0 : ARMMMUIdx_Stage1_E0;
             break;
         default:
             g_assert_not_reached();
@@ -3213,7 +3214,7 @@  static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri,
     case 0:
         switch (ri->opc1) {
         case 0: /* AT S1E1R, AT S1E1W */
-            mmu_idx = secure ? ARMMMUIdx_S1SE1 : ARMMMUIdx_S1NSE1;
+            mmu_idx = secure ? ARMMMUIdx_S1SE1 : ARMMMUIdx_Stage1_E1;
             break;
         case 4: /* AT S1E2R, AT S1E2W */
             mmu_idx = ARMMMUIdx_S1E2;
@@ -3226,7 +3227,7 @@  static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri,
         }
         break;
     case 2: /* AT S1E0R, AT S1E0W */
-        mmu_idx = secure ? ARMMMUIdx_S1SE0 : ARMMMUIdx_S1NSE0;
+        mmu_idx = secure ? ARMMMUIdx_S1SE0 : ARMMMUIdx_Stage1_E0;
         break;
     case 4: /* AT S12E1R, AT S12E1W */
         mmu_idx = secure ? ARMMMUIdx_S1SE1 : ARMMMUIdx_EL10_1;
@@ -8571,8 +8572,8 @@  static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
     case ARMMMUIdx_S1SE0:
         return arm_el_is_aa64(env, 3) ? 1 : 3;
     case ARMMMUIdx_S1SE1:
-    case ARMMMUIdx_S1NSE0:
-    case ARMMMUIdx_S1NSE1:
+    case ARMMMUIdx_Stage1_E0:
+    case ARMMMUIdx_Stage1_E1:
     case ARMMMUIdx_MPrivNegPri:
     case ARMMMUIdx_MUserNegPri:
     case ARMMMUIdx_MPriv:
@@ -8630,7 +8631,7 @@  static inline bool regime_translation_disabled(CPUARMState *env,
     }
 
     if ((env->cp15.hcr_el2 & HCR_DC) &&
-        (mmu_idx == ARMMMUIdx_S1NSE0 || mmu_idx == ARMMMUIdx_S1NSE1)) {
+        (mmu_idx == ARMMMUIdx_Stage1_E0 || mmu_idx == ARMMMUIdx_Stage1_E1)) {
         /* HCR.DC means SCTLR_EL1.M behaves as 0 */
         return true;
     }
@@ -8675,7 +8676,7 @@  static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
 static inline ARMMMUIdx stage_1_mmu_idx(ARMMMUIdx mmu_idx)
 {
     if (mmu_idx == ARMMMUIdx_EL10_0 || mmu_idx == ARMMMUIdx_EL10_1) {
-        mmu_idx += (ARMMMUIdx_S1NSE0 - ARMMMUIdx_EL10_0);
+        mmu_idx += (ARMMMUIdx_Stage1_E0 - ARMMMUIdx_EL10_0);
     }
     return mmu_idx;
 }
@@ -8710,7 +8711,7 @@  static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx)
 {
     switch (mmu_idx) {
     case ARMMMUIdx_S1SE0:
-    case ARMMMUIdx_S1NSE0:
+    case ARMMMUIdx_Stage1_E0:
     case ARMMMUIdx_MUser:
     case ARMMMUIdx_MSUser:
     case ARMMMUIdx_MUserNegPri:
@@ -8941,7 +8942,7 @@  static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
                                hwaddr addr, MemTxAttrs txattrs,
                                ARMMMUFaultInfo *fi)
 {
-    if ((mmu_idx == ARMMMUIdx_S1NSE0 || mmu_idx == ARMMMUIdx_S1NSE1) &&
+    if ((mmu_idx == ARMMMUIdx_Stage1_E0 || mmu_idx == ARMMMUIdx_Stage1_E1) &&
         !regime_translation_disabled(env, ARMMMUIdx_Stage2)) {
         target_ulong s2size;
         hwaddr s2pa;