Re: [Qemu-devel] [PATCH 04/11] target-arm: Define correct mmu_idx values and pass them in TB flags

2015-01-29 Thread Greg Bellows
On Wed, Jan 28, 2015 at 4:34 PM, Peter Maydell peter.mayd...@linaro.org
wrote:

 On 28 January 2015 at 21:57, Greg Bellows greg.bell...@linaro.org wrote:
  After getting through patch 9, I wonder if the TB NS bit can also be
 removed
  as it is implied in the MMU index.

 No, because for a 32-bit EL3 we are always running under a Secure
 translation regime (S1E3) but the TBFLAG_NS bit may be either 0 or
 1 depending on the value of the SCR.NS bit.


​That is correct.​



 -- PMM



Re: [Qemu-devel] [PATCH 04/11] target-arm: Define correct mmu_idx values and pass them in TB flags

2015-01-28 Thread Greg Bellows
On Fri, Jan 23, 2015 at 12:20 PM, Peter Maydell peter.mayd...@linaro.org
wrote:

 We currently claim that for ARM the mmu_idx should simply be the current
 exception level. However this isn't actually correct -- secure EL0 and EL1
 should have separate indexes from non-secure EL0 and EL1 since their
 VA-PA mappings may differ. We also will want an index for stage 2
 translations when we properly support EL2.

 Define and document all seven mmu index values that we require, and
 pass the mmu index in the TB flags rather than exception level or
 priv/user bit.

 This change doesn't update the get_phys_addr() code, so our page
 table walking still assumes a simplistic user or priv? model for
 the moment.

 Signed-off-by: Peter Maydell peter.mayd...@linaro.org
 ---
 This leaves some odd gaps in the TB flags usage. I will circle
 back and clean this up later (including moving the other common
 flags like the singlestep ones to the top of the flags word),
 but I didn't want to bloat this patchseries further.
 ---
  target-arm/cpu.h   | 113
 -
  target-arm/helper.c|   3 +-
  target-arm/translate-a64.c |   5 +-
  target-arm/translate.c |   5 +-
  target-arm/translate.h |   3 +-
  5 files changed, 101 insertions(+), 28 deletions(-)

 diff --git a/target-arm/cpu.h b/target-arm/cpu.h
 index 3eb00f4..cf7b9ab 100644
 --- a/target-arm/cpu.h
 +++ b/target-arm/cpu.h
 @@ -98,7 +98,7 @@ typedef uint32_t ARMReadCPFunc(void *opaque, int cp_info,

  struct arm_boot_info;

 -#define NB_MMU_MODES 4
 +#define NB_MMU_MODES 7

  /* We currently assume float and double are IEEE single and double
 precision respectively.
 @@ -1572,13 +1572,92 @@ static inline CPUARMState *cpu_init(const char
 *cpu_model)
  #define cpu_signal_handler cpu_arm_signal_handler
  #define cpu_list arm_cpu_list

 -/* MMU modes definitions */
 +/* ARM has the following translation regimes (as the ARM ARM calls
 them):
 + *
 + * If EL3 is 64-bit:
 + *  + NonSecure EL1  0 stage 1
 + *  + NonSecure EL1  0 stage 2
 + *  + NonSecure EL2
 + *  + Secure EL1  EL0
 + *  + Secure EL3
 + * If EL3 is 32-bit:
 + *  + NonSecure PL1  0 stage 1
 + *  + NonSecure PL1  0 stage 2
 + *  + NonSecure PL2
 + *  + Secure PL0  PL1
 + * (reminder: for 32 bit EL3, Secure PL1 is *EL3*, not EL1.)
 + *
 + * For QEMU, an mmu_idx is not quite the same as a translation regime
 because:
 + *  1. we need to split the EL1  0 regimes into two mmu_idxes, because
 they
 + * may differ in access permissions even if the VA-PA map is the same
 + *  2. we want to cache in our TLB the full VA-IPA-PA lookup for a
 stage 1+2
 + * translation, which means that we have one mmu_idx that deals with
 two
 + * concatenated translation regimes [this sort of combined s1+2 TLB is
 + * architecturally permitted]
 + *  3. we don't need to allocate an mmu_idx to translations that we won't
 be
 + * handling via the TLB. The only way to do a stage 1 translation
 without
 + * the immediate stage 2 translation is via the ATS or AT system
 insns,
 + * which can be slow-pathed and always do a page table walk.
 + *  4. we can also safely fold together the 32 bit EL3 and 64 bit EL3
 + * translation regimes, because they map reasonably well to each other
 + * and they can't both be active at the same time.
 + * This gives us the following list of mmu_idx values:
 + *
 + * NS EL0 (aka NS PL0) stage 1+2
 + * NS EL1 (aka NS PL1) stage 1+2
 + * NS EL2 (aka NS PL2)
 + * S EL3 (aka S PL1)
 + * S EL0 (aka S PL0)
 + * S EL1 (not used if EL3 is 32 bit)
 + * NS EL0+1 stage 2
 + *
 + * (The last of these is an mmu_idx because we want to be able to use the
 TLB
 + * for the accesses done as part of a stage 1 page table walk, rather than
 + * having to walk the stage 2 page table over and over.)
 + *
 + * Our enumeration includes at the end some entries which are not true
 + * mmu_idx values in that they don't have corresponding TLBs and are only
 + * valid for doing slow path page table walks.
 + *
 + * The constant names here are patterned after the general style of the
 names
 + * of the AT/ATS operations.
 + * The values used are carefully arranged to make mmu_idx = EL lookup
 easy.
 + */
 +typedef enum ARMMMUIdx {
 +ARMMMUIdx_S12NSE0 = 0,
 +ARMMMUIdx_S12NSE1 = 1,
 +ARMMMUIdx_S1E2 = 2,
 +ARMMMUIdx_S1E3 = 3,
 +ARMMMUIdx_S1SE0 = 4,
 +ARMMMUIdx_S1SE1 = 5,
 +ARMMMUIdx_S2NS = 6,
 +/* 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 = 7,
 +ARMMMUIdx_S1NSE1 = 8,
 +} ARMMMUIdx;
 +
  #define MMU_MODE0_SUFFIX _user
  #define MMU_MODE1_SUFFIX _kernel
  #define MMU_USER_IDX 0
 +
 +/* Return the exception level we're running at if this is our mmu_idx */
 +static inline int arm_mmu_idx_to_el(ARMMMUIdx mmu_idx)
 +{
 +assert(mmu_idx  ARMMMUIdx_S2NS);
 +return mmu_idx  3;
 +}
 +
 

Re: [Qemu-devel] [PATCH 04/11] target-arm: Define correct mmu_idx values and pass them in TB flags

2015-01-28 Thread Peter Maydell
On 28 January 2015 at 21:57, Greg Bellows greg.bell...@linaro.org wrote:
 After getting through patch 9, I wonder if the TB NS bit can also be removed
 as it is implied in the MMU index.

No, because for a 32-bit EL3 we are always running under a Secure
translation regime (S1E3) but the TBFLAG_NS bit may be either 0 or
1 depending on the value of the SCR.NS bit.

-- PMM



Re: [Qemu-devel] [PATCH 04/11] target-arm: Define correct mmu_idx values and pass them in TB flags

2015-01-27 Thread Peter Maydell
On 23 January 2015 at 18:20, Peter Maydell peter.mayd...@linaro.org wrote:
 +/* Determine the current mmu_idx to use for normal loads/stores */
  static inline int cpu_mmu_index (CPUARMState *env)
  {
 -return arm_current_el(env);
 +int el = arm_current_el(env);
 +
 +if (el  3  arm_is_secure_below_el3(env)) {
 +return ARMMMUIdx_S1SE0 + el;
 +}
 +return el;
  }

Just noticed the stray extra space between function name and '('
here; will delete the space in v2, since we're rewriting the whole
function body anyhow...

-- PMM



Re: [Qemu-devel] [PATCH 04/11] target-arm: Define correct mmu_idx values and pass them in TB flags

2015-01-26 Thread Peter Maydell
On 24 January 2015 at 19:31, Peter Maydell peter.mayd...@linaro.org wrote:
 On 24 January 2015 at 16:36, Greg Bellows greg.bell...@linaro.org wrote:
 I understand what the code is doing, my point is that you rely on
 arm_is_secure_below_el3 to deal with EL2 when you could have just as easily
 checked for el2.  Not a big deal though.

 Well, I rely on the architecture to tell me that it isn't
 possible for arm_is_secure_below_el3 to be true if we're
 in EL2. Otherwise there'd be a bug in that function...

On reflection, I see your point, so have changed to 'el  2  ...'
for v2.

thanks
-- PMM



Re: [Qemu-devel] [PATCH 04/11] target-arm: Define correct mmu_idx values and pass them in TB flags

2015-01-24 Thread Peter Maydell
On 24 January 2015 at 16:36, Greg Bellows greg.bell...@linaro.org wrote:
 I understand what the code is doing, my point is that you rely on
 arm_is_secure_below_el3 to deal with EL2 when you could have just as easily
 checked for el2.  Not a big deal though.

Well, I rely on the architecture to tell me that it isn't
possible for arm_is_secure_below_el3 to be true if we're
in EL2. Otherwise there'd be a bug in that function...

-- PMM



Re: [Qemu-devel] [PATCH 04/11] target-arm: Define correct mmu_idx values and pass them in TB flags

2015-01-24 Thread Greg Bellows
On Jan 23, 2015 7:12 PM, Peter Maydell peter.mayd...@linaro.org wrote:

 On 23 January 2015 at 21:44, Greg Bellows greg.bell...@linaro.org wrote:
  On Fri, Jan 23, 2015 at 12:20 PM, Peter Maydell
  peter.mayd...@linaro.org wrote:
  +typedef enum ARMMMUIdx {
  +ARMMMUIdx_S12NSE0 = 0,
  +ARMMMUIdx_S12NSE1 = 1,
  +ARMMMUIdx_S1E2 = 2,
  +ARMMMUIdx_S1E3 = 3,
  +ARMMMUIdx_S1SE0 = 4,
  +ARMMMUIdx_S1SE1 = 5,
  +ARMMMUIdx_S2NS = 6,
  +/* 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 = 7,
  +ARMMMUIdx_S1NSE1 = 8,
  +} ARMMMUIdx;
  +
   #define MMU_MODE0_SUFFIX _user
   #define MMU_MODE1_SUFFIX _kernel
   #define MMU_USER_IDX 0
  +
  +/* Return the exception level we're running at if this is our mmu_idx
*/
  +static inline int arm_mmu_idx_to_el(ARMMMUIdx mmu_idx)
  +{
  +assert(mmu_idx  ARMMMUIdx_S2NS);
  +return mmu_idx  3;
  +}
  +
  +/* Determine the current mmu_idx to use for normal loads/stores */
   static inline int cpu_mmu_index (CPUARMState *env)
   {
  -return arm_current_el(env);
  +int el = arm_current_el(env);
  +
  +if (el  3  arm_is_secure_below_el3(env)) {
 
  We bypass the secure check for EL3 but not EL2.  We should either
  circumvent both EL2  3 or neither.

 Not sure what you mean here. The point of this code is to return
 a suitable MMU idx for the current CPU state. If we are in
 EL2 or EL3 then just el is correct (being ARMMMUIdx_S1E2 and
 ARMMMUIdx_S1E3). We can't be in this condition with el == 2

I understand what the code is doing, my point is that you rely on
arm_is_secure_below_el3 to deal with EL2 when you could have just as easily
checked for el2.  Not a big deal though.

 because if we're in EL2 then the NS bit must be set (there's
 no such thing as Secure EL2) and so arm_is_secure_below_el3()
 will have returned false. So we know here that el is 0 or 1,
 and this addition below:

  +return ARMMMUIdx_S1SE0 + el;

 means we end up with either _S1SE0 or _S1SE1, as required.

 (the condition could equally well be written
if (el  2  arm_is_secure_below_el3(env))
  as the two are true in exactly the same set of cases.  3 seemed
  marginally better to me, since it's expressing if we're secure
 and not in EL3 which is the set of cases where we need to
 change the mmu_idx from the 0/1/2/3 values.)

  +}
  +return el;

 Anything else (_S12NSE0, _S12NSE1, _S1E2, _S1E3) is this return.

   }

 thanks
 -- PMM


[Qemu-devel] [PATCH 04/11] target-arm: Define correct mmu_idx values and pass them in TB flags

2015-01-23 Thread Peter Maydell
We currently claim that for ARM the mmu_idx should simply be the current
exception level. However this isn't actually correct -- secure EL0 and EL1
should have separate indexes from non-secure EL0 and EL1 since their
VA-PA mappings may differ. We also will want an index for stage 2
translations when we properly support EL2.

Define and document all seven mmu index values that we require, and
pass the mmu index in the TB flags rather than exception level or
priv/user bit.

This change doesn't update the get_phys_addr() code, so our page
table walking still assumes a simplistic user or priv? model for
the moment.

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
This leaves some odd gaps in the TB flags usage. I will circle
back and clean this up later (including moving the other common
flags like the singlestep ones to the top of the flags word),
but I didn't want to bloat this patchseries further.
---
 target-arm/cpu.h   | 113 -
 target-arm/helper.c|   3 +-
 target-arm/translate-a64.c |   5 +-
 target-arm/translate.c |   5 +-
 target-arm/translate.h |   3 +-
 5 files changed, 101 insertions(+), 28 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 3eb00f4..cf7b9ab 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -98,7 +98,7 @@ typedef uint32_t ARMReadCPFunc(void *opaque, int cp_info,
 
 struct arm_boot_info;
 
-#define NB_MMU_MODES 4
+#define NB_MMU_MODES 7
 
 /* We currently assume float and double are IEEE single and double
precision respectively.
@@ -1572,13 +1572,92 @@ static inline CPUARMState *cpu_init(const char 
*cpu_model)
 #define cpu_signal_handler cpu_arm_signal_handler
 #define cpu_list arm_cpu_list
 
-/* MMU modes definitions */
+/* ARM has the following translation regimes (as the ARM ARM calls them):
+ *
+ * If EL3 is 64-bit:
+ *  + NonSecure EL1  0 stage 1
+ *  + NonSecure EL1  0 stage 2
+ *  + NonSecure EL2
+ *  + Secure EL1  EL0
+ *  + Secure EL3
+ * If EL3 is 32-bit:
+ *  + NonSecure PL1  0 stage 1
+ *  + NonSecure PL1  0 stage 2
+ *  + NonSecure PL2
+ *  + Secure PL0  PL1
+ * (reminder: for 32 bit EL3, Secure PL1 is *EL3*, not EL1.)
+ *
+ * For QEMU, an mmu_idx is not quite the same as a translation regime because:
+ *  1. we need to split the EL1  0 regimes into two mmu_idxes, because they
+ * may differ in access permissions even if the VA-PA map is the same
+ *  2. we want to cache in our TLB the full VA-IPA-PA lookup for a stage 1+2
+ * translation, which means that we have one mmu_idx that deals with two
+ * concatenated translation regimes [this sort of combined s1+2 TLB is
+ * architecturally permitted]
+ *  3. we don't need to allocate an mmu_idx to translations that we won't be
+ * handling via the TLB. The only way to do a stage 1 translation without
+ * the immediate stage 2 translation is via the ATS or AT system insns,
+ * which can be slow-pathed and always do a page table walk.
+ *  4. we can also safely fold together the 32 bit EL3 and 64 bit EL3
+ * translation regimes, because they map reasonably well to each other
+ * and they can't both be active at the same time.
+ * This gives us the following list of mmu_idx values:
+ *
+ * NS EL0 (aka NS PL0) stage 1+2
+ * NS EL1 (aka NS PL1) stage 1+2
+ * NS EL2 (aka NS PL2)
+ * S EL3 (aka S PL1)
+ * S EL0 (aka S PL0)
+ * S EL1 (not used if EL3 is 32 bit)
+ * NS EL0+1 stage 2
+ *
+ * (The last of these is an mmu_idx because we want to be able to use the TLB
+ * for the accesses done as part of a stage 1 page table walk, rather than
+ * having to walk the stage 2 page table over and over.)
+ *
+ * Our enumeration includes at the end some entries which are not true
+ * mmu_idx values in that they don't have corresponding TLBs and are only
+ * valid for doing slow path page table walks.
+ *
+ * The constant names here are patterned after the general style of the names
+ * of the AT/ATS operations.
+ * The values used are carefully arranged to make mmu_idx = EL lookup easy.
+ */
+typedef enum ARMMMUIdx {
+ARMMMUIdx_S12NSE0 = 0,
+ARMMMUIdx_S12NSE1 = 1,
+ARMMMUIdx_S1E2 = 2,
+ARMMMUIdx_S1E3 = 3,
+ARMMMUIdx_S1SE0 = 4,
+ARMMMUIdx_S1SE1 = 5,
+ARMMMUIdx_S2NS = 6,
+/* 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 = 7,
+ARMMMUIdx_S1NSE1 = 8,
+} ARMMMUIdx;
+
 #define MMU_MODE0_SUFFIX _user
 #define MMU_MODE1_SUFFIX _kernel
 #define MMU_USER_IDX 0
+
+/* Return the exception level we're running at if this is our mmu_idx */
+static inline int arm_mmu_idx_to_el(ARMMMUIdx mmu_idx)
+{
+assert(mmu_idx  ARMMMUIdx_S2NS);
+return mmu_idx  3;
+}
+
+/* Determine the current mmu_idx to use for normal loads/stores */
 static inline int cpu_mmu_index (CPUARMState *env)
 {
-return arm_current_el(env);
+int el = arm_current_el(env);
+
+if (el 

Re: [Qemu-devel] [PATCH 04/11] target-arm: Define correct mmu_idx values and pass them in TB flags

2015-01-23 Thread Greg Bellows
On Fri, Jan 23, 2015 at 12:20 PM, Peter Maydell
peter.mayd...@linaro.org wrote:
 We currently claim that for ARM the mmu_idx should simply be the current
 exception level. However this isn't actually correct -- secure EL0 and EL1
 should have separate indexes from non-secure EL0 and EL1 since their
 VA-PA mappings may differ. We also will want an index for stage 2
 translations when we properly support EL2.

 Define and document all seven mmu index values that we require, and
 pass the mmu index in the TB flags rather than exception level or
 priv/user bit.

 This change doesn't update the get_phys_addr() code, so our page
 table walking still assumes a simplistic user or priv? model for
 the moment.

 Signed-off-by: Peter Maydell peter.mayd...@linaro.org
 ---
 This leaves some odd gaps in the TB flags usage. I will circle
 back and clean this up later (including moving the other common
 flags like the singlestep ones to the top of the flags word),
 but I didn't want to bloat this patchseries further.
 ---
  target-arm/cpu.h   | 113 
 -
  target-arm/helper.c|   3 +-
  target-arm/translate-a64.c |   5 +-
  target-arm/translate.c |   5 +-
  target-arm/translate.h |   3 +-
  5 files changed, 101 insertions(+), 28 deletions(-)

 diff --git a/target-arm/cpu.h b/target-arm/cpu.h
 index 3eb00f4..cf7b9ab 100644
 --- a/target-arm/cpu.h
 +++ b/target-arm/cpu.h
 @@ -98,7 +98,7 @@ typedef uint32_t ARMReadCPFunc(void *opaque, int cp_info,

  struct arm_boot_info;

 -#define NB_MMU_MODES 4
 +#define NB_MMU_MODES 7

  /* We currently assume float and double are IEEE single and double
 precision respectively.
 @@ -1572,13 +1572,92 @@ static inline CPUARMState *cpu_init(const char 
 *cpu_model)
  #define cpu_signal_handler cpu_arm_signal_handler
  #define cpu_list arm_cpu_list

 -/* MMU modes definitions */
 +/* ARM has the following translation regimes (as the ARM ARM calls them):
 + *
 + * If EL3 is 64-bit:
 + *  + NonSecure EL1  0 stage 1
 + *  + NonSecure EL1  0 stage 2
 + *  + NonSecure EL2
 + *  + Secure EL1  EL0
 + *  + Secure EL3
 + * If EL3 is 32-bit:
 + *  + NonSecure PL1  0 stage 1
 + *  + NonSecure PL1  0 stage 2
 + *  + NonSecure PL2
 + *  + Secure PL0  PL1
 + * (reminder: for 32 bit EL3, Secure PL1 is *EL3*, not EL1.)
 + *
 + * For QEMU, an mmu_idx is not quite the same as a translation regime 
 because:
 + *  1. we need to split the EL1  0 regimes into two mmu_idxes, because 
 they
 + * may differ in access permissions even if the VA-PA map is the same
 + *  2. we want to cache in our TLB the full VA-IPA-PA lookup for a stage 
 1+2
 + * translation, which means that we have one mmu_idx that deals with two
 + * concatenated translation regimes [this sort of combined s1+2 TLB is
 + * architecturally permitted]
 + *  3. we don't need to allocate an mmu_idx to translations that we won't be
 + * handling via the TLB. The only way to do a stage 1 translation without
 + * the immediate stage 2 translation is via the ATS or AT system insns,
 + * which can be slow-pathed and always do a page table walk.
 + *  4. we can also safely fold together the 32 bit EL3 and 64 bit EL3
 + * translation regimes, because they map reasonably well to each other
 + * and they can't both be active at the same time.
 + * This gives us the following list of mmu_idx values:
 + *
 + * NS EL0 (aka NS PL0) stage 1+2
 + * NS EL1 (aka NS PL1) stage 1+2
 + * NS EL2 (aka NS PL2)
 + * S EL3 (aka S PL1)
 + * S EL0 (aka S PL0)
 + * S EL1 (not used if EL3 is 32 bit)
 + * NS EL0+1 stage 2
 + *
 + * (The last of these is an mmu_idx because we want to be able to use the TLB
 + * for the accesses done as part of a stage 1 page table walk, rather than
 + * having to walk the stage 2 page table over and over.)
 + *
 + * Our enumeration includes at the end some entries which are not true
 + * mmu_idx values in that they don't have corresponding TLBs and are only
 + * valid for doing slow path page table walks.
 + *
 + * The constant names here are patterned after the general style of the names
 + * of the AT/ATS operations.
 + * The values used are carefully arranged to make mmu_idx = EL lookup easy.
 + */
 +typedef enum ARMMMUIdx {
 +ARMMMUIdx_S12NSE0 = 0,
 +ARMMMUIdx_S12NSE1 = 1,
 +ARMMMUIdx_S1E2 = 2,
 +ARMMMUIdx_S1E3 = 3,
 +ARMMMUIdx_S1SE0 = 4,
 +ARMMMUIdx_S1SE1 = 5,
 +ARMMMUIdx_S2NS = 6,
 +/* 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 = 7,
 +ARMMMUIdx_S1NSE1 = 8,
 +} ARMMMUIdx;
 +
  #define MMU_MODE0_SUFFIX _user
  #define MMU_MODE1_SUFFIX _kernel
  #define MMU_USER_IDX 0
 +
 +/* Return the exception level we're running at if this is our mmu_idx */
 +static inline int arm_mmu_idx_to_el(ARMMMUIdx mmu_idx)
 +{
 +assert(mmu_idx  ARMMMUIdx_S2NS);
 +return mmu_idx  3;
 +}
 +
 +/* 

Re: [Qemu-devel] [PATCH 04/11] target-arm: Define correct mmu_idx values and pass them in TB flags

2015-01-23 Thread Peter Maydell
On 23 January 2015 at 21:44, Greg Bellows greg.bell...@linaro.org wrote:
 On Fri, Jan 23, 2015 at 12:20 PM, Peter Maydell
 peter.mayd...@linaro.org wrote:
 +typedef enum ARMMMUIdx {
 +ARMMMUIdx_S12NSE0 = 0,
 +ARMMMUIdx_S12NSE1 = 1,
 +ARMMMUIdx_S1E2 = 2,
 +ARMMMUIdx_S1E3 = 3,
 +ARMMMUIdx_S1SE0 = 4,
 +ARMMMUIdx_S1SE1 = 5,
 +ARMMMUIdx_S2NS = 6,
 +/* 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 = 7,
 +ARMMMUIdx_S1NSE1 = 8,
 +} ARMMMUIdx;
 +
  #define MMU_MODE0_SUFFIX _user
  #define MMU_MODE1_SUFFIX _kernel
  #define MMU_USER_IDX 0
 +
 +/* Return the exception level we're running at if this is our mmu_idx */
 +static inline int arm_mmu_idx_to_el(ARMMMUIdx mmu_idx)
 +{
 +assert(mmu_idx  ARMMMUIdx_S2NS);
 +return mmu_idx  3;
 +}
 +
 +/* Determine the current mmu_idx to use for normal loads/stores */
  static inline int cpu_mmu_index (CPUARMState *env)
  {
 -return arm_current_el(env);
 +int el = arm_current_el(env);
 +
 +if (el  3  arm_is_secure_below_el3(env)) {

 We bypass the secure check for EL3 but not EL2.  We should either
 circumvent both EL2  3 or neither.

Not sure what you mean here. The point of this code is to return
a suitable MMU idx for the current CPU state. If we are in
EL2 or EL3 then just el is correct (being ARMMMUIdx_S1E2 and
ARMMMUIdx_S1E3). We can't be in this condition with el == 2
because if we're in EL2 then the NS bit must be set (there's
no such thing as Secure EL2) and so arm_is_secure_below_el3()
will have returned false. So we know here that el is 0 or 1,
and this addition below:

 +return ARMMMUIdx_S1SE0 + el;

means we end up with either _S1SE0 or _S1SE1, as required.

(the condition could equally well be written
   if (el  2  arm_is_secure_below_el3(env))
 as the two are true in exactly the same set of cases.  3 seemed
 marginally better to me, since it's expressing if we're secure
and not in EL3 which is the set of cases where we need to
change the mmu_idx from the 0/1/2/3 values.)

 +}
 +return el;

Anything else (_S12NSE0, _S12NSE1, _S1E2, _S1E3) is this return.

  }

thanks
-- PMM