[Qemu-devel] [PATCH v3 00/22] target-arm: Preparations for A64 EL2 and 3

[Qemu-devel] [PATCH v3 00/22] target-arm: Preparations for A64 EL2 and 3

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Hi,

I've been doing some work on modeling parts of EL2 and 3 + some of
the system-wide virtualization features for ARMv8. A lot is missing
but I've got a series with enough to for example run KVM A64 guests
on top of EL3 firmware inside emulated QEMU A64 VMs.
I'm working on cleaning things up and plan to send patches and publish
things as I go.

This series does a first round of preparations. Most of it has little
or no user visible impact as the EL2 and 3 features remain disabled.

For the arrayification of the various EL regs that do not exist for
EL0, I've now switched to using padded arrays with the index beeing
the EL nr.

Regarding the discussion on 1:1 mapping between EL and MMU-idx:
http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03563.html
I've appended two RFC patches and the end of the series showing
what a switch to 1:1 mapping between EL and MMU-idx could look like.
If the 1:1 mapping is prefered, I'll fold the patches for the
next spin.

Happy to address any comments people may have.

Cheers,
Edgar

v2 -> v3:
* Rephrase comment in update_spsel
* Switch arrayification pattern to padded with el as index.
* Rename arm64_spsr_banked_index to aarch64_spsr_banked_index.
* Avoid using the result from aarch64_spsr_banked_index in a32 mode.
* aarch64 - > AArch64 in comments.
* Make VBAR_EL2 RES0 if EL2 is not enabled but higher ELs are.
* Drop friendly DAIF logging
* Squash Add storage for VBAR_EL2 and 3 into reg of VBAR

v1 -> v2:
* Name hyp and mon in cpu_mode_names[]
* Friendly log of full DAIF, delimited by space
* Use raw_write in vbar_write
* Blank line after MMU IDX
* Use 1:1 EL array mapping in arm64_banked_spsr_index
* Fix typo in commit message adding arm64_banked_spsr_index.
* Register EL2 and EL3 regs outside if ARM_FEATURE_V8
* Avoid split declaration in exception_return
* Merge comments in exception_return
* Use VBAR_EL_IDX when registering VBAR_EL2 and 3.

Edgar E. Iglesias (22):
  target-arm: Make elr_el1 an array
  target-arm: Make esr_el1 an array
  target-arm: c12_vbar -> vbar_el[]
  target-arm: Add arm_el_to_mmu_idx()
  target-arm: Move get_mem_index to translate.h
  target-arm: A64: Add SP entries for EL2 and 3
  target-arm: A64: Add ELR entries for EL2 and 3
  target-arm: Add SPSR entries for EL2/HYP and EL3/MON
  target-arm: A64: Introduce aarch64_banked_spsr_index()
  target-arm: Add a feature flag for EL2
  target-arm: Add a feature flag for EL3
  target-arm: Register EL2 versions of ELR and SPSR
  target-arm: Register EL3 versions of ELR and SPSR
  target-arm: A64: Forbid ERET to increase the EL
  target-arm: A64: Forbid ERET to unimplemented ELs
  target-arm: A64: Generalize ERET to various ELs
  target-arm: A64: Generalize update_spsel for the various ELs
  target-arm: Make vbar_write writeback to any CPREG
  target-arm: A64: Register VBAR_EL2
  target-arm: A64: Register VBAR_EL3
  RFC: target-arm: A32: Use get_mem_index for load/stores
  RFC: target-arm: Use a 1:1 mapping between EL and MMU index

 target-arm/cpu.h           |  24 +++--
 target-arm/helper-a64.c    |  12 +--
 target-arm/helper.c        |  80 ++++++++++++++--
 target-arm/internals.h     |  25 ++++-
 target-arm/kvm64.c         |   4 +-
 target-arm/machine.c       |  10 +-
 target-arm/op_helper.c     |  19 ++--
 target-arm/translate-a64.c |   9 --
 target-arm/translate.c     | 224 +++++++++++++++++++++++----------------------
 target-arm/translate.h     |   5 +
 10 files changed, 251 insertions(+), 161 deletions(-)

-- 
1.8.3.2

[Qemu-devel] [PATCH v3 01/22] target-arm: Make elr_el1 an array

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

No functional change.
Prepares for future additions of the EL2 and 3 versions of this reg.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h        | 2 +-
 target-arm/helper-a64.c | 4 ++--
 target-arm/helper.c     | 3 ++-
 target-arm/kvm64.c      | 4 ++--
 target-arm/machine.c    | 2 +-
 target-arm/op_helper.c  | 6 +++---
 6 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index c83f249..19013ad 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -162,7 +162,7 @@ typedef struct CPUARMState {
     uint32_t condexec_bits; /* IT bits.  cpsr[15:10,26:25].  */
     uint64_t daif; /* exception masks, in the bits they are in in PSTATE */
 
-    uint64_t elr_el1; /* AArch64 ELR_EL1 */
+    uint64_t elr_el[2]; /* AArch64 exception link regs  */
     uint64_t sp_el[2]; /* AArch64 banked stack pointers */
 
     /* System control coprocessor (cp15) */
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index bf921cc..7e5073b 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -491,13 +491,13 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
         env->banked_spsr[0] = pstate_read(env);
         env->sp_el[arm_current_pl(env)] = env->xregs[31];
         env->xregs[31] = env->sp_el[1];
-        env->elr_el1 = env->pc;
+        env->elr_el[1] = env->pc;
     } else {
         env->banked_spsr[0] = cpsr_read(env);
         if (!env->thumb) {
             env->cp15.esr_el1 |= 1 << 25;
         }
-        env->elr_el1 = env->regs[15];
+        env->elr_el[1] = env->regs[15];
 
         for (i = 0; i < 15; i++) {
             env->xregs[i] = env->regs[i];
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 417161e..6d6601e 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2055,7 +2055,8 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
     { .name = "ELR_EL1", .state = ARM_CP_STATE_AA64,
       .type = ARM_CP_NO_MIGRATE,
       .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 0, .opc2 = 1,
-      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, elr_el1) },
+      .access = PL1_RW,
+      .fieldoffset = offsetof(CPUARMState, elr_el[1]) },
     { .name = "SPSR_EL1", .state = ARM_CP_STATE_AA64,
       .type = ARM_CP_NO_MIGRATE,
       .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 0, .opc2 = 0,
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index c729b9e..70f311b 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -161,7 +161,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
     }
 
     reg.id = AARCH64_CORE_REG(elr_el1);
-    reg.addr = (uintptr_t) &env->elr_el1;
+    reg.addr = (uintptr_t) &env->elr_el[1];
     ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
     if (ret) {
         return ret;
@@ -241,7 +241,7 @@ int kvm_arch_get_registers(CPUState *cs)
     }
 
     reg.id = AARCH64_CORE_REG(elr_el1);
-    reg.addr = (uintptr_t) &env->elr_el1;
+    reg.addr = (uintptr_t) &env->elr_el[1];
     ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
     if (ret) {
         return ret;
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 5092dcd..b0fa46d 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -238,7 +238,7 @@ const VMStateDescription vmstate_arm_cpu = {
         VMSTATE_UINT32_ARRAY(env.banked_r14, ARMCPU, 6),
         VMSTATE_UINT32_ARRAY(env.usr_regs, ARMCPU, 5),
         VMSTATE_UINT32_ARRAY(env.fiq_regs, ARMCPU, 5),
-        VMSTATE_UINT64(env.elr_el1, ARMCPU),
+        VMSTATE_UINT64(env.elr_el[1], ARMCPU),
         VMSTATE_UINT64_ARRAY(env.sp_el, ARMCPU, 2),
         /* The length-check must come before the arrays to avoid
          * incoming data possibly overflowing the array.
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index fb90676..f120b02 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -406,7 +406,7 @@ void HELPER(exception_return)(CPUARMState *env)
             env->regs[i] = env->xregs[i];
         }
 
-        env->regs[15] = env->elr_el1 & ~0x1;
+        env->regs[15] = env->elr_el[1] & ~0x1;
     } else {
         new_el = extract32(spsr, 2, 2);
         if (new_el > 1) {
@@ -424,7 +424,7 @@ void HELPER(exception_return)(CPUARMState *env)
         env->aarch64 = 1;
         pstate_write(env, spsr);
         env->xregs[31] = env->sp_el[new_el];
-        env->pc = env->elr_el1;
+        env->pc = env->elr_el[1];
     }
 
     return;
@@ -438,7 +438,7 @@ illegal_return:
      * no change to exception level, execution state or stack pointer
      */
     env->pstate |= PSTATE_IL;
-    env->pc = env->elr_el1;
+    env->pc = env->elr_el[1];
     spsr &= PSTATE_NZCV | PSTATE_DAIF;
     spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
     pstate_write(env, spsr);
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 02/22] target-arm: Make esr_el1 an array

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

No functional change.
Prepares for future addtion of EL2 and 3 versions of this reg.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h        |  2 +-
 target-arm/helper-a64.c |  4 ++--
 target-arm/helper.c     | 10 +++++-----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 19013ad..0b30ab8 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -185,7 +185,7 @@ typedef struct CPUARMState {
         uint32_t pmsav5_data_ap; /* PMSAv5 MPU data access permissions */
         uint32_t pmsav5_insn_ap; /* PMSAv5 MPU insn access permissions */
         uint32_t ifsr_el2; /* Fault status registers.  */
-        uint64_t esr_el1;
+        uint64_t esr_el[2];
         uint32_t c6_region[8]; /* MPU base/size registers.  */
         uint64_t far_el1; /* Fault address registers.  */
         uint64_t par_el1;  /* Translation result. */
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 7e5073b..13c5865 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -464,7 +464,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
                       env->exception.syndrome);
     }
 
-    env->cp15.esr_el1 = env->exception.syndrome;
+    env->cp15.esr_el[1] = env->exception.syndrome;
     env->cp15.far_el1 = env->exception.vaddress;
 
     switch (cs->exception_index) {
@@ -495,7 +495,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
     } else {
         env->banked_spsr[0] = cpsr_read(env);
         if (!env->thumb) {
-            env->cp15.esr_el1 |= 1 << 25;
+            env->cp15.esr_el[1] |= 1 << 25;
         }
         env->elr_el[1] = env->regs[15];
 
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 6d6601e..107cd5f 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1452,7 +1452,7 @@ static void vmsa_ttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
 static const ARMCPRegInfo vmsa_cp_reginfo[] = {
     { .name = "DFSR", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW, .type = ARM_CP_NO_MIGRATE,
-      .fieldoffset = offsetoflow32(CPUARMState, cp15.esr_el1),
+      .fieldoffset = offsetoflow32(CPUARMState, cp15.esr_el[1]),
       .resetfn = arm_cp_reset_ignore, },
     { .name = "IFSR", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 1,
       .access = PL1_RW,
@@ -1460,7 +1460,7 @@ static const ARMCPRegInfo vmsa_cp_reginfo[] = {
     { .name = "ESR_EL1", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .crn = 5, .crm = 2, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW,
-      .fieldoffset = offsetof(CPUARMState, cp15.esr_el1), .resetvalue = 0, },
+      .fieldoffset = offsetof(CPUARMState, cp15.esr_el[1]), .resetvalue = 0, },
     { .name = "TTBR0_EL1", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .crn = 2, .crm = 0, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.ttbr0_el1),
@@ -1521,7 +1521,7 @@ static void omap_cachemaint_write(CPUARMState *env, const ARMCPRegInfo *ri,
 static const ARMCPRegInfo omap_cp_reginfo[] = {
     { .name = "DFSR", .cp = 15, .crn = 5, .crm = CP_ANY,
       .opc1 = CP_ANY, .opc2 = CP_ANY, .access = PL1_RW, .type = ARM_CP_OVERRIDE,
-      .fieldoffset = offsetoflow32(CPUARMState, cp15.esr_el1),
+      .fieldoffset = offsetoflow32(CPUARMState, cp15.esr_el[1]),
       .resetvalue = 0, },
     { .name = "", .cp = 15, .crn = 15, .crm = 0, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW, .type = ARM_CP_NOP },
@@ -3338,11 +3338,11 @@ void arm_cpu_do_interrupt(CPUState *cs)
         offset = 4;
         break;
     case EXCP_DATA_ABORT:
-        env->cp15.esr_el1 = env->exception.fsr;
+        env->cp15.esr_el[1] = env->exception.fsr;
         env->cp15.far_el1 = deposit64(env->cp15.far_el1, 0, 32,
                                       env->exception.vaddress);
         qemu_log_mask(CPU_LOG_INT, "...with DFSR 0x%x DFAR 0x%x\n",
-                      (uint32_t)env->cp15.esr_el1,
+                      (uint32_t)env->cp15.esr_el[1],
                       (uint32_t)env->exception.vaddress);
         new_mode = ARM_CPU_MODE_ABT;
         addr = 0x10;
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 03/22] target-arm: c12_vbar -> vbar_el[]

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

No functional change.
Preparation for adding EL2 and 3 versions of this reg.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h        | 2 +-
 target-arm/helper-a64.c | 2 +-
 target-arm/helper.c     | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 0b30ab8..11b7a0b 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -198,7 +198,7 @@ typedef struct CPUARMState {
         uint32_t c9_pmuserenr; /* perf monitor user enable */
         uint32_t c9_pminten; /* perf monitor interrupt enables */
         uint64_t mair_el1;
-        uint64_t c12_vbar; /* vector base address register */
+        uint64_t vbar_el[2]; /* vector base address register */
         uint32_t c13_fcse; /* FCSE PID.  */
         uint64_t contextidr_el1; /* Context ID.  */
         uint64_t tpidr_el0; /* User RW Thread register.  */
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 13c5865..b8e6d56 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -443,7 +443,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
 {
     ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;
-    target_ulong addr = env->cp15.c12_vbar;
+    target_ulong addr = env->cp15.vbar_el[1];
     int i;
 
     if (arm_current_pl(env) == 0) {
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 107cd5f..bba7297 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -657,7 +657,7 @@ static void vbar_write(CPUARMState *env, const ARMCPRegInfo *ri,
      * contexts. (ARMv8 would permit us to do no masking at all, but ARMv7
      * requires the bottom five bits to be RAZ/WI because they're UNK/SBZP.)
      */
-    env->cp15.c12_vbar = value & ~0x1FULL;
+    env->cp15.vbar_el[1] = value & ~0x1FULL;
 }
 
 static uint64_t ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
@@ -766,7 +766,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
     { .name = "VBAR", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .crn = 12, .crm = 0, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW, .writefn = vbar_write,
-      .fieldoffset = offsetof(CPUARMState, cp15.c12_vbar),
+      .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[1]),
       .resetvalue = 0 },
     { .name = "SCR", .cp = 15, .crn = 1, .crm = 1, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.c1_scr),
@@ -3379,7 +3379,7 @@ void arm_cpu_do_interrupt(CPUState *cs)
          * and is never in monitor mode this feature is always active.
          * Note: only bits 31:5 are valid.
          */
-        addr += env->cp15.c12_vbar;
+        addr += env->cp15.vbar_el[1];
     }
     switch_mode (env, new_mode);
     env->spsr = cpsr_read(env);
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 04/22] target-arm: Add arm_el_to_mmu_idx()

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Maps a given EL to the corresponding MMU index.

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h           | 22 +++++++++++++++++++++-
 target-arm/translate-a64.c |  8 ++------
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 11b7a0b..b6ad913 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1083,9 +1083,29 @@ static inline CPUARMState *cpu_init(const char *cpu_model)
 #define MMU_MODE0_SUFFIX _kernel
 #define MMU_MODE1_SUFFIX _user
 #define MMU_USER_IDX 1
+
+static inline int arm_el_to_mmu_idx(int current_el)
+{
+#ifdef CONFIG_USER_ONLY
+    return MMU_USER_IDX;
+#else
+    switch (current_el) {
+    case 0:
+        return MMU_USER_IDX;
+    case 1:
+        return 0;
+    default:
+        /* Unsupported EL.  */
+        assert(0);
+        return 0;
+    }
+#endif
+}
+
 static inline int cpu_mmu_index (CPUARMState *env)
 {
-    return arm_current_pl(env) ? 0 : 1;
+    int cur_el = arm_current_pl(env);
+    return arm_el_to_mmu_idx(cur_el);
 }
 
 #include "exec/cpu-all.h"
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index b62db4d..7fce05f 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -162,13 +162,9 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
     }
 }
 
-static int get_mem_index(DisasContext *s)
+static inline int get_mem_index(DisasContext *s)
 {
-#ifdef CONFIG_USER_ONLY
-    return 1;
-#else
-    return s->user;
-#endif
+    return arm_el_to_mmu_idx(s->current_pl);
 }
 
 void gen_a64_set_pc_im(uint64_t val)
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 05/22] target-arm: Move get_mem_index to translate.h

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

So that it can be shared with the A32 code in the future.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/translate-a64.c | 5 -----
 target-arm/translate.h     | 5 +++++
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 7fce05f..bfd139a 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -162,11 +162,6 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
     }
 }
 
-static inline int get_mem_index(DisasContext *s)
-{
-    return arm_el_to_mmu_idx(s->current_pl);
-}
-
 void gen_a64_set_pc_im(uint64_t val)
 {
     tcg_gen_movi_i64(cpu_pc, val);
diff --git a/target-arm/translate.h b/target-arm/translate.h
index 34328f4..db6f0af 100644
--- a/target-arm/translate.h
+++ b/target-arm/translate.h
@@ -52,6 +52,11 @@ static inline int arm_dc_feature(DisasContext *dc, int feature)
     return (dc->features & (1ULL << feature)) != 0;
 }
 
+static inline int get_mem_index(DisasContext *s)
+{
+    return arm_el_to_mmu_idx(s->current_pl);
+}
+
 /* target-specific extra values for is_jmp */
 /* These instructions trap after executing, so the A32/T32 decoder must
  * defer them until after the conditional execution state has been updated.
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 06/22] target-arm: A64: Add SP entries for EL2 and 3

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h     | 2 +-
 target-arm/machine.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index b6ad913..95665b0 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -163,7 +163,7 @@ typedef struct CPUARMState {
     uint64_t daif; /* exception masks, in the bits they are in in PSTATE */
 
     uint64_t elr_el[2]; /* AArch64 exception link regs  */
-    uint64_t sp_el[2]; /* AArch64 banked stack pointers */
+    uint64_t sp_el[4]; /* AArch64 banked stack pointers */
 
     /* System control coprocessor (cp15) */
     struct {
diff --git a/target-arm/machine.c b/target-arm/machine.c
index b0fa46d..7b18a90 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -218,8 +218,8 @@ static int cpu_post_load(void *opaque, int version_id)
 
 const VMStateDescription vmstate_arm_cpu = {
     .name = "cpu",
-    .version_id = 17,
-    .minimum_version_id = 17,
+    .version_id = 18,
+    .minimum_version_id = 18,
     .pre_save = cpu_pre_save,
     .post_load = cpu_post_load,
     .fields = (VMStateField[]) {
@@ -239,7 +239,7 @@ const VMStateDescription vmstate_arm_cpu = {
         VMSTATE_UINT32_ARRAY(env.usr_regs, ARMCPU, 5),
         VMSTATE_UINT32_ARRAY(env.fiq_regs, ARMCPU, 5),
         VMSTATE_UINT64(env.elr_el[1], ARMCPU),
-        VMSTATE_UINT64_ARRAY(env.sp_el, ARMCPU, 2),
+        VMSTATE_UINT64_ARRAY(env.sp_el, ARMCPU, 4),
         /* The length-check must come before the arrays to avoid
          * incoming data possibly overflowing the array.
          */
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 07/22] target-arm: A64: Add ELR entries for EL2 and 3

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h     | 2 +-
 target-arm/machine.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 95665b0..817d057 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -162,7 +162,7 @@ typedef struct CPUARMState {
     uint32_t condexec_bits; /* IT bits.  cpsr[15:10,26:25].  */
     uint64_t daif; /* exception masks, in the bits they are in in PSTATE */
 
-    uint64_t elr_el[2]; /* AArch64 exception link regs  */
+    uint64_t elr_el[4]; /* AArch64 exception link regs  */
     uint64_t sp_el[4]; /* AArch64 banked stack pointers */
 
     /* System control coprocessor (cp15) */
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 7b18a90..233e70d 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -218,8 +218,8 @@ static int cpu_post_load(void *opaque, int version_id)
 
 const VMStateDescription vmstate_arm_cpu = {
     .name = "cpu",
-    .version_id = 18,
-    .minimum_version_id = 18,
+    .version_id = 19,
+    .minimum_version_id = 19,
     .pre_save = cpu_pre_save,
     .post_load = cpu_post_load,
     .fields = (VMStateField[]) {
@@ -238,7 +238,7 @@ const VMStateDescription vmstate_arm_cpu = {
         VMSTATE_UINT32_ARRAY(env.banked_r14, ARMCPU, 6),
         VMSTATE_UINT32_ARRAY(env.usr_regs, ARMCPU, 5),
         VMSTATE_UINT32_ARRAY(env.fiq_regs, ARMCPU, 5),
-        VMSTATE_UINT64(env.elr_el[1], ARMCPU),
+        VMSTATE_UINT64_ARRAY(env.elr_el, ARMCPU, 4),
         VMSTATE_UINT64_ARRAY(env.sp_el, ARMCPU, 4),
         /* The length-check must come before the arrays to avoid
          * incoming data possibly overflowing the array.
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 08/22] target-arm: Add SPSR entries for EL2/HYP and EL3/MON

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h       | 4 +++-
 target-arm/helper.c    | 4 ++++
 target-arm/machine.c   | 6 +++---
 target-arm/translate.c | 4 ++--
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 817d057..41ee3c2 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -143,7 +143,7 @@ typedef struct CPUARMState {
     uint32_t spsr;
 
     /* Banked registers.  */
-    uint64_t banked_spsr[6];
+    uint64_t banked_spsr[8];
     uint32_t banked_r13[6];
     uint32_t banked_r14[6];
 
@@ -563,7 +563,9 @@ enum arm_cpu_mode {
   ARM_CPU_MODE_FIQ = 0x11,
   ARM_CPU_MODE_IRQ = 0x12,
   ARM_CPU_MODE_SVC = 0x13,
+  ARM_CPU_MODE_MON = 0x16,
   ARM_CPU_MODE_ABT = 0x17,
+  ARM_CPU_MODE_HYP = 0x1a,
   ARM_CPU_MODE_UND = 0x1b,
   ARM_CPU_MODE_SYS = 0x1f
 };
diff --git a/target-arm/helper.c b/target-arm/helper.c
index bba7297..5e2eac3 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3084,6 +3084,10 @@ int bank_number(int mode)
         return 4;
     case ARM_CPU_MODE_FIQ:
         return 5;
+    case ARM_CPU_MODE_HYP:
+        return 6;
+    case ARM_CPU_MODE_MON:
+        return 7;
     }
     hw_error("bank number requested for bad CPSR mode value 0x%x\n", mode);
 }
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 233e70d..3bcc7cc 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -218,8 +218,8 @@ static int cpu_post_load(void *opaque, int version_id)
 
 const VMStateDescription vmstate_arm_cpu = {
     .name = "cpu",
-    .version_id = 19,
-    .minimum_version_id = 19,
+    .version_id = 20,
+    .minimum_version_id = 20,
     .pre_save = cpu_pre_save,
     .post_load = cpu_post_load,
     .fields = (VMStateField[]) {
@@ -233,7 +233,7 @@ const VMStateDescription vmstate_arm_cpu = {
             .offset = 0,
         },
         VMSTATE_UINT32(env.spsr, ARMCPU),
-        VMSTATE_UINT64_ARRAY(env.banked_spsr, ARMCPU, 6),
+        VMSTATE_UINT64_ARRAY(env.banked_spsr, ARMCPU, 8),
         VMSTATE_UINT32_ARRAY(env.banked_r13, ARMCPU, 6),
         VMSTATE_UINT32_ARRAY(env.banked_r14, ARMCPU, 6),
         VMSTATE_UINT32_ARRAY(env.usr_regs, ARMCPU, 5),
diff --git a/target-arm/translate.c b/target-arm/translate.c
index a4d920b..c9fa5d9 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -11047,8 +11047,8 @@ void gen_intermediate_code_pc(CPUARMState *env, TranslationBlock *tb)
 }
 
 static const char *cpu_mode_names[16] = {
-  "usr", "fiq", "irq", "svc", "???", "???", "???", "abt",
-  "???", "???", "???", "und", "???", "???", "???", "sys"
+  "usr", "fiq", "irq", "svc", "???", "???", "mon", "abt",
+  "???", "???", "hyp", "und", "???", "???", "???", "sys"
 };
 
 void arm_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 09/22] target-arm: A64: Introduce aarch64_banked_spsr_index()

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Add aarch64_banked_spsr_index(), used to map an Exception Level
to an index in the banked_spsr array.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper-a64.c |  2 +-
 target-arm/internals.h  | 14 ++++++++++++++
 target-arm/op_helper.c  |  3 ++-
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index b8e6d56..b970fd1 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -488,7 +488,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
     }
 
     if (is_a64(env)) {
-        env->banked_spsr[0] = pstate_read(env);
+        env->banked_spsr[aarch64_banked_spsr_index(1)] = pstate_read(env);
         env->sp_el[arm_current_pl(env)] = env->xregs[31];
         env->xregs[31] = env->sp_el[1];
         env->elr_el[1] = env->pc;
diff --git a/target-arm/internals.h b/target-arm/internals.h
index d63a975..c9897c2 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -75,6 +75,20 @@ static inline void arm_log_exception(int idx)
  */
 #define GTIMER_SCALE 16
 
+/*
+ * For AArch64, map a given EL to an index in the banked_spsr array.
+ */
+static inline unsigned int aarch64_banked_spsr_index(unsigned int el)
+{
+    static const unsigned int map[4] = {
+        [1] = 0, /* EL1.  */
+        [2] = 6, /* EL2.  */
+        [3] = 7, /* EL3.  */
+    };
+    assert(el >= 1 && el <= 3);
+    return map[el];
+}
+
 int bank_number(int mode);
 void switch_mode(CPUARMState *, int);
 void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu);
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index f120b02..c05a839 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -386,7 +386,8 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
 
 void HELPER(exception_return)(CPUARMState *env)
 {
-    uint32_t spsr = env->banked_spsr[0];
+    unsigned int spsr_idx = is_a64(env) ? aarch64_banked_spsr_index(1) : 0;
+    uint32_t spsr = env->banked_spsr[spsr_idx];
     int new_el, i;
 
     if (env->pstate & PSTATE_SP) {
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v3 09/22] target-arm: A64: Introduce aarch64_banked_spsr_index()

[Peter Maydell]

On 19 May 2014 10:22, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Add aarch64_banked_spsr_index(), used to map an Exception Level
> to an index in the banked_spsr array.
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index f120b02..c05a839 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -386,7 +386,8 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
>
>  void HELPER(exception_return)(CPUARMState *env)
>  {
> -    uint32_t spsr = env->banked_spsr[0];
> +    unsigned int spsr_idx = is_a64(env) ? aarch64_banked_spsr_index(1) : 0;

This is unnecessary -- if we get here we must have is_a64(env) true,
because this is the helper for an A64 instruction.

> +    uint32_t spsr = env->banked_spsr[spsr_idx];
>      int new_el, i;
>
>      if (env->pstate & PSTATE_SP) {

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 09/22] target-arm: A64: Introduce aarch64_banked_spsr_index()

[Edgar E. Iglesias]

On Wed, May 21, 2014 at 08:01:54PM +0100, Peter Maydell wrote:
> On 19 May 2014 10:22, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Add aarch64_banked_spsr_index(), used to map an Exception Level
> > to an index in the banked_spsr array.
> >
> > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> > index f120b02..c05a839 100644
> > --- a/target-arm/op_helper.c
> > +++ b/target-arm/op_helper.c
> > @@ -386,7 +386,8 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
> >
> >  void HELPER(exception_return)(CPUARMState *env)
> >  {
> > -    uint32_t spsr = env->banked_spsr[0];
> > +    unsigned int spsr_idx = is_a64(env) ? aarch64_banked_spsr_index(1) : 0;
> 
> This is unnecessary -- if we get here we must have is_a64(env) true,
> because this is the helper for an A64 instruction.

Yep, got confused while your previous comments. Fixed


> 
> > +    uint32_t spsr = env->banked_spsr[spsr_idx];
> >      int new_el, i;
> >
> >      if (env->pstate & PSTATE_SP) {
> 
> thanks
> -- PMM

[Qemu-devel] [PATCH v3 10/22] target-arm: Add a feature flag for EL2

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 41ee3c2..0108c62 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -633,6 +633,7 @@ enum arm_features {
     ARM_FEATURE_CBAR, /* has cp15 CBAR */
     ARM_FEATURE_CRC, /* ARMv8 CRC instructions */
     ARM_FEATURE_CBAR_RO, /* has cp15 CBAR and it is read-only */
+    ARM_FEATURE_EL2, /* has EL2 Virtualization support */
 };
 
 static inline int arm_feature(CPUARMState *env, int feature)
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 11/22] target-arm: Add a feature flag for EL3

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 0108c62..7322e03 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -634,6 +634,7 @@ enum arm_features {
     ARM_FEATURE_CRC, /* ARMv8 CRC instructions */
     ARM_FEATURE_CBAR_RO, /* has cp15 CBAR and it is read-only */
     ARM_FEATURE_EL2, /* has EL2 Virtualization support */
+    ARM_FEATURE_EL3, /* has EL3 Secure monitor support */
 };
 
 static inline int arm_feature(CPUARMState *env, int feature)
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 12/22] target-arm: Register EL2 versions of ELR and SPSR

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 5e2eac3..81de010 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2077,6 +2077,19 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
     REGINFO_SENTINEL
 };
 
+static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
+    { .name = "ELR_EL2", .state = ARM_CP_STATE_AA64,
+      .type = ARM_CP_NO_MIGRATE,
+      .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 1,
+      .access = PL2_RW,
+      .fieldoffset = offsetof(CPUARMState, elr_el[2]) },
+    { .name = "SPSR_EL2", .state = ARM_CP_STATE_AA64,
+      .type = ARM_CP_NO_MIGRATE,
+      .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 0,
+      .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[6]) },
+    REGINFO_SENTINEL
+};
+
 static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                         uint64_t value)
 {
@@ -2328,6 +2341,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         define_arm_cp_regs(cpu, v8_cp_reginfo);
         define_aarch64_debug_regs(cpu);
     }
+    if (arm_feature(env, ARM_FEATURE_EL2)) {
+        define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
+    }
     if (arm_feature(env, ARM_FEATURE_MPU)) {
         /* These are the MPU registers prior to PMSAv6. Any new
          * PMSA core later than the ARM946 will require that we
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 13/22] target-arm: Register EL3 versions of ELR and SPSR

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 81de010..cb7c964a 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2090,6 +2090,19 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
     REGINFO_SENTINEL
 };
 
+static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
+    { .name = "ELR_EL3", .state = ARM_CP_STATE_AA64,
+      .type = ARM_CP_NO_MIGRATE,
+      .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 0, .opc2 = 1,
+      .access = PL3_RW,
+      .fieldoffset = offsetof(CPUARMState, elr_el[3]) },
+    { .name = "SPSR_EL3", .state = ARM_CP_STATE_AA64,
+      .type = ARM_CP_NO_MIGRATE,
+      .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 0, .opc2 = 0,
+      .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[7]) },
+    REGINFO_SENTINEL
+};
+
 static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                         uint64_t value)
 {
@@ -2344,6 +2357,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
     if (arm_feature(env, ARM_FEATURE_EL2)) {
         define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
     }
+    if (arm_feature(env, ARM_FEATURE_EL3)) {
+        define_arm_cp_regs(cpu, v8_el3_cp_reginfo);
+    }
     if (arm_feature(env, ARM_FEATURE_MPU)) {
         /* These are the MPU registers prior to PMSAv6. Any new
          * PMSA core later than the ARM946 will require that we
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 14/22] target-arm: A64: Forbid ERET to increase the EL

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/op_helper.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index c05a839..a6eedb2 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -389,6 +389,7 @@ void HELPER(exception_return)(CPUARMState *env)
     unsigned int spsr_idx = is_a64(env) ? aarch64_banked_spsr_index(1) : 0;
     uint32_t spsr = env->banked_spsr[spsr_idx];
     int new_el, i;
+    int cur_el = arm_current_pl(env);
 
     if (env->pstate & PSTATE_SP) {
         env->sp_el[1] = env->xregs[31];
@@ -410,6 +411,10 @@ void HELPER(exception_return)(CPUARMState *env)
         env->regs[15] = env->elr_el[1] & ~0x1;
     } else {
         new_el = extract32(spsr, 2, 2);
+        if (new_el > cur_el) {
+            /* Disallow returns to higher ELs than the current one.  */
+            goto illegal_return;
+        }
         if (new_el > 1) {
             /* Return to unimplemented EL */
             goto illegal_return;
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 15/22] target-arm: A64: Forbid ERET to unimplemented ELs

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Check for EL2 support before returning to it.

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/op_helper.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index a6eedb2..d89755a 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -411,12 +411,11 @@ void HELPER(exception_return)(CPUARMState *env)
         env->regs[15] = env->elr_el[1] & ~0x1;
     } else {
         new_el = extract32(spsr, 2, 2);
-        if (new_el > cur_el) {
-            /* Disallow returns to higher ELs than the current one.  */
-            goto illegal_return;
-        }
-        if (new_el > 1) {
-            /* Return to unimplemented EL */
+        if (new_el > cur_el
+            || (new_el == 2 && !arm_feature(env, ARM_FEATURE_EL2))) {
+            /* Disallow return to an EL which is unimplemented or higher
+             * than the current one.
+             */
             goto illegal_return;
         }
         if (extract32(spsr, 1, 1)) {
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v3 15/22] target-arm: A64: Forbid ERET to unimplemented ELs

[Peter Maydell]

On 19 May 2014 10:22, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Check for EL2 support before returning to it.
>
> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/op_helper.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index a6eedb2..d89755a 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -411,12 +411,11 @@ void HELPER(exception_return)(CPUARMState *env)
>          env->regs[15] = env->elr_el[1] & ~0x1;
>      } else {
>          new_el = extract32(spsr, 2, 2);
> -        if (new_el > cur_el) {
> -            /* Disallow returns to higher ELs than the current one.  */
> -            goto illegal_return;
> -        }

...you just added this if() in the previous patch. Suggest
squashing the two together.

> -        if (new_el > 1) {
> -            /* Return to unimplemented EL */
> +        if (new_el > cur_el
> +            || (new_el == 2 && !arm_feature(env, ARM_FEATURE_EL2))) {
> +            /* Disallow return to an EL which is unimplemented or higher
> +             * than the current one.
> +             */
>              goto illegal_return;
>          }
>          if (extract32(spsr, 1, 1)) {
> --
> 1.8.3.2
>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 15/22] target-arm: A64: Forbid ERET to unimplemented ELs

[Edgar E. Iglesias]

On Wed, May 21, 2014 at 08:06:32PM +0100, Peter Maydell wrote:
> On 19 May 2014 10:22, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Check for EL2 support before returning to it.
> >
> > Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/op_helper.c | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> > index a6eedb2..d89755a 100644
> > --- a/target-arm/op_helper.c
> > +++ b/target-arm/op_helper.c
> > @@ -411,12 +411,11 @@ void HELPER(exception_return)(CPUARMState *env)
> >          env->regs[15] = env->elr_el[1] & ~0x1;
> >      } else {
> >          new_el = extract32(spsr, 2, 2);
> > -        if (new_el > cur_el) {
> > -            /* Disallow returns to higher ELs than the current one.  */
> > -            goto illegal_return;
> > -        }
> 
> ...you just added this if() in the previous patch. Suggest
> squashing the two together.

Squashed

> 
> > -        if (new_el > 1) {
> > -            /* Return to unimplemented EL */
> > +        if (new_el > cur_el
> > +            || (new_el == 2 && !arm_feature(env, ARM_FEATURE_EL2))) {
> > +            /* Disallow return to an EL which is unimplemented or higher
> > +             * than the current one.
> > +             */
> >              goto illegal_return;
> >          }
> >          if (extract32(spsr, 1, 1)) {
> > --
> > 1.8.3.2
> >
> 
> thanks
> -- PMM

[Qemu-devel] [PATCH v3 16/22] target-arm: A64: Generalize ERET to various ELs

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Adds support for ERET to Aarch64 EL2 and 3.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/op_helper.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index d89755a..c632dd6 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -386,13 +386,13 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
 
 void HELPER(exception_return)(CPUARMState *env)
 {
-    unsigned int spsr_idx = is_a64(env) ? aarch64_banked_spsr_index(1) : 0;
+    int cur_el = arm_current_pl(env);
+    unsigned int spsr_idx = is_a64(env) ? aarch64_banked_spsr_index(cur_el) : 0;
     uint32_t spsr = env->banked_spsr[spsr_idx];
     int new_el, i;
-    int cur_el = arm_current_pl(env);
 
     if (env->pstate & PSTATE_SP) {
-        env->sp_el[1] = env->xregs[31];
+        env->sp_el[cur_el] = env->xregs[31];
     } else {
         env->sp_el[0] = env->xregs[31];
     }
@@ -429,7 +429,7 @@ void HELPER(exception_return)(CPUARMState *env)
         env->aarch64 = 1;
         pstate_write(env, spsr);
         env->xregs[31] = env->sp_el[new_el];
-        env->pc = env->elr_el[1];
+        env->pc = env->elr_el[cur_el];
     }
 
     return;
@@ -443,7 +443,7 @@ illegal_return:
      * no change to exception level, execution state or stack pointer
      */
     env->pstate |= PSTATE_IL;
-    env->pc = env->elr_el[1];
+    env->pc = env->elr_el[cur_el];
     spsr &= PSTATE_NZCV | PSTATE_DAIF;
     spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
     pstate_write(env, spsr);
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v3 16/22] target-arm: A64: Generalize ERET to various ELs

[Peter Maydell]

On 19 May 2014 10:22, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Adds support for ERET to Aarch64 EL2 and 3.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/op_helper.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index d89755a..c632dd6 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -386,13 +386,13 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
>
>  void HELPER(exception_return)(CPUARMState *env)
>  {
> -    unsigned int spsr_idx = is_a64(env) ? aarch64_banked_spsr_index(1) : 0;
> +    int cur_el = arm_current_pl(env);
> +    unsigned int spsr_idx = is_a64(env) ? aarch64_banked_spsr_index(cur_el) : 0;

This will now allow the guest to trigger an assert() by doing an
ERET in EL0... The fix for that is to put in the check in translate-a64.c,
I think, since ERET in EL0 should be an UnallocatedEncoding.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 16/22] target-arm: A64: Generalize ERET to various ELs

[Edgar E. Iglesias]

On Wed, May 21, 2014 at 08:10:53PM +0100, Peter Maydell wrote:
> On 19 May 2014 10:22, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Adds support for ERET to Aarch64 EL2 and 3.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/op_helper.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> > index d89755a..c632dd6 100644
> > --- a/target-arm/op_helper.c
> > +++ b/target-arm/op_helper.c
> > @@ -386,13 +386,13 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
> >
> >  void HELPER(exception_return)(CPUARMState *env)
> >  {
> > -    unsigned int spsr_idx = is_a64(env) ? aarch64_banked_spsr_index(1) : 0;
> > +    int cur_el = arm_current_pl(env);
> > +    unsigned int spsr_idx = is_a64(env) ? aarch64_banked_spsr_index(cur_el) : 0;
> 
> This will now allow the guest to trigger an assert() by doing an
> ERET in EL0... The fix for that is to put in the check in translate-a64.c,
> I think, since ERET in EL0 should be an UnallocatedEncoding.

Nice catch. I've prepended this patch with a patch traping eret in el0 at
translation time.

Thanks,
Edgar

Re: [Qemu-devel] [PATCH v3 16/22] target-arm: A64: Generalize ERET to various ELs

[Peter Maydell]

On 19 May 2014 10:22, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Adds support for ERET to Aarch64 EL2 and 3.

"AArch64". Also "to" here is ambiguous. The ARM ARM tries to
keep the terminology straight to avoid confusion: exceptions
are "taken from ELx" and "taken to ELx"; we "return from ELx"
and "return to ELx". This looks like it's talking about "return to
EL2/EL3" but probably doesn't really mean that.

If this patch is adding support for ERET from EL2/EL3
it needs to also fix the bit in the "returning to an exception
level which is 32 bit" which says "new_el = 0" since that's
not guaranteed to be true any more. (Also I think the register
mapping for AArch32 EL2/EL1 needs handling correctly.)

> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/op_helper.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index d89755a..c632dd6 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -386,13 +386,13 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
>
>  void HELPER(exception_return)(CPUARMState *env)
>  {
> -    unsigned int spsr_idx = is_a64(env) ? aarch64_banked_spsr_index(1) : 0;
> +    int cur_el = arm_current_pl(env);
> +    unsigned int spsr_idx = is_a64(env) ? aarch64_banked_spsr_index(cur_el) : 0;
>      uint32_t spsr = env->banked_spsr[spsr_idx];
>      int new_el, i;
> -    int cur_el = arm_current_pl(env);
>
>      if (env->pstate & PSTATE_SP) {
> -        env->sp_el[1] = env->xregs[31];
> +        env->sp_el[cur_el] = env->xregs[31];
>      } else {
>          env->sp_el[0] = env->xregs[31];
>      }
> @@ -429,7 +429,7 @@ void HELPER(exception_return)(CPUARMState *env)
>          env->aarch64 = 1;
>          pstate_write(env, spsr);
>          env->xregs[31] = env->sp_el[new_el];
> -        env->pc = env->elr_el[1];
> +        env->pc = env->elr_el[cur_el];
>      }
>
>      return;
> @@ -443,7 +443,7 @@ illegal_return:
>       * no change to exception level, execution state or stack pointer
>       */
>      env->pstate |= PSTATE_IL;
> -    env->pc = env->elr_el[1];
> +    env->pc = env->elr_el[cur_el];
>      spsr &= PSTATE_NZCV | PSTATE_DAIF;
>      spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
>      pstate_write(env, spsr);
> --
> 1.8.3.2
>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 16/22] target-arm: A64: Generalize ERET to various ELs

[Edgar E. Iglesias]

On Wed, May 21, 2014 at 08:20:20PM +0100, Peter Maydell wrote:
> On 19 May 2014 10:22, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Adds support for ERET to Aarch64 EL2 and 3.
> 
> "AArch64". Also "to" here is ambiguous. The ARM ARM tries to

Changed to AArch64.

> keep the terminology straight to avoid confusion: exceptions
> are "taken from ELx" and "taken to ELx"; we "return from ELx"
> and "return to ELx". This looks like it's talking about "return to
> EL2/EL3" but probably doesn't really mean that.
> 
> If this patch is adding support for ERET from EL2/EL3

My first tests when starting booting in EL3 where to take exceptions
and return back to EL3 and EL2, the commit log msg didn't
evolve since but I guess I mean from and to EL2/3. Changed it to:

Adds support for ERET to and from AArch64 EL2 and 3.

If you have better suggestions I'm happy to change it to whatever.

> it needs to also fix the bit in the "returning to an exception
> level which is 32 bit" which says "new_el = 0" since that's
> not guaranteed to be true any more. (Also I think the register
> mapping for AArch32 EL2/EL1 needs handling correctly.)

I've tried to stay away from touching too much of the AArch32
code as I haven't had a setup to test 64/32 transitions
beyond a64/el1 and a32/el0.

I do have a follow-up patch (not very tested) for ERET to aarch32.
If you feel strongly about it I can squash the little I've got
for aarch32 allthough I'd rather do it incrementally and 
leave it for follow-ups.

Cheers,
Edgar



> 
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/op_helper.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> > index d89755a..c632dd6 100644
> > --- a/target-arm/op_helper.c
> > +++ b/target-arm/op_helper.c
> > @@ -386,13 +386,13 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
> >
> >  void HELPER(exception_return)(CPUARMState *env)
> >  {
> > -    unsigned int spsr_idx = is_a64(env) ? aarch64_banked_spsr_index(1) : 0;
> > +    int cur_el = arm_current_pl(env);
> > +    unsigned int spsr_idx = is_a64(env) ? aarch64_banked_spsr_index(cur_el) : 0;
> >      uint32_t spsr = env->banked_spsr[spsr_idx];
> >      int new_el, i;
> > -    int cur_el = arm_current_pl(env);
> >
> >      if (env->pstate & PSTATE_SP) {
> > -        env->sp_el[1] = env->xregs[31];
> > +        env->sp_el[cur_el] = env->xregs[31];
> >      } else {
> >          env->sp_el[0] = env->xregs[31];
> >      }
> > @@ -429,7 +429,7 @@ void HELPER(exception_return)(CPUARMState *env)
> >          env->aarch64 = 1;
> >          pstate_write(env, spsr);
> >          env->xregs[31] = env->sp_el[new_el];
> > -        env->pc = env->elr_el[1];
> > +        env->pc = env->elr_el[cur_el];
> >      }
> >
> >      return;
> > @@ -443,7 +443,7 @@ illegal_return:
> >       * no change to exception level, execution state or stack pointer
> >       */
> >      env->pstate |= PSTATE_IL;
> > -    env->pc = env->elr_el[1];
> > +    env->pc = env->elr_el[cur_el];
> >      spsr &= PSTATE_NZCV | PSTATE_DAIF;
> >      spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
> >      pstate_write(env, spsr);
> > --
> > 1.8.3.2
> >
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v3 16/22] target-arm: A64: Generalize ERET to various ELs

[Peter Maydell]

On 22 May 2014 01:48, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Wed, May 21, 2014 at 08:20:20PM +0100, Peter Maydell wrote:
>> it needs to also fix the bit in the "returning to an exception
>> level which is 32 bit" which says "new_el = 0" since that's
>> not guaranteed to be true any more. (Also I think the register
>> mapping for AArch32 EL2/EL1 needs handling correctly.)
>
> I've tried to stay away from touching too much of the AArch32
> code as I haven't had a setup to test 64/32 transitions
> beyond a64/el1 and a32/el0.

OK; if we put in a TODO comment that we assume EL1..EL3
are 64 bit currently, we'll have a marker to come back and fix
later.

thanks
-- PMM

[Qemu-devel] [PATCH v3 17/22] target-arm: A64: Generalize update_spsel for the various ELs

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/internals.h | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/target-arm/internals.h b/target-arm/internals.h
index c9897c2..564b5fa 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -107,6 +107,7 @@ int arm_rmode_to_sf(int rmode);
 
 static inline void update_spsel(CPUARMState *env, uint32_t imm)
 {
+    unsigned int cur_el = arm_current_pl(env);
     /* Update PSTATE SPSel bit; this requires us to update the
      * working stack pointer in xregs[31].
      */
@@ -115,17 +116,17 @@ static inline void update_spsel(CPUARMState *env, uint32_t imm)
     }
     env->pstate = deposit32(env->pstate, 0, 1, imm);
 
-    /* EL0 has no access rights to update SPSel, and this code
-     * assumes we are updating SP for EL1 while running as EL1.
+    /* We rely on illegal updates to SPsel from EL0 to get trapped
+     * at translation time.
      */
-    assert(arm_current_pl(env) == 1);
+    assert(cur_el >= 1 && cur_el <= 3);
     if (env->pstate & PSTATE_SP) {
         /* Switch from using SP_EL0 to using SP_ELx */
         env->sp_el[0] = env->xregs[31];
-        env->xregs[31] = env->sp_el[1];
+        env->xregs[31] = env->sp_el[cur_el];
     } else {
         /* Switch from SP_EL0 to SP_ELx */
-        env->sp_el[1] = env->xregs[31];
+        env->sp_el[cur_el] = env->xregs[31];
         env->xregs[31] = env->sp_el[0];
     }
 }
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 18/22] target-arm: Make vbar_write writeback to any CPREG

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index cb7c964a..5a2073e 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -657,7 +657,7 @@ static void vbar_write(CPUARMState *env, const ARMCPRegInfo *ri,
      * contexts. (ARMv8 would permit us to do no masking at all, but ARMv7
      * requires the bottom five bits to be RAZ/WI because they're UNK/SBZP.)
      */
-    env->cp15.vbar_el[1] = value & ~0x1FULL;
+    raw_write(env, ri, value & ~0x1FULL);
 }
 
 static uint64_t ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 19/22] target-arm: A64: Register VBAR_EL2

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h    |  2 +-
 target-arm/helper.c | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 7322e03..693ad0f 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -198,7 +198,7 @@ typedef struct CPUARMState {
         uint32_t c9_pmuserenr; /* perf monitor user enable */
         uint32_t c9_pminten; /* perf monitor interrupt enables */
         uint64_t mair_el1;
-        uint64_t vbar_el[2]; /* vector base address register */
+        uint64_t vbar_el[3]; /* vector base address register */
         uint32_t c13_fcse; /* FCSE PID.  */
         uint64_t contextidr_el1; /* Context ID.  */
         uint64_t tpidr_el0; /* User RW Thread register.  */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 5a2073e..7ca63a8 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2077,6 +2077,15 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
     REGINFO_SENTINEL
 };
 
+/* Used to describe the behaviour of EL2 regs when EL2 does not exist.  */
+static const ARMCPRegInfo v8_no_el2_cp_reginfo[] = {
+    { .name = "VBAR_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
+      .access = PL2_RW,
+      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
+    REGINFO_SENTINEL
+};
+
 static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
     { .name = "ELR_EL2", .state = ARM_CP_STATE_AA64,
       .type = ARM_CP_NO_MIGRATE,
@@ -2087,6 +2096,11 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
       .type = ARM_CP_NO_MIGRATE,
       .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 0,
       .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[6]) },
+    { .name = "VBAR_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
+      .access = PL2_RW, .writefn = vbar_write,
+      .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[2]),
+      .resetvalue = 0 },
     REGINFO_SENTINEL
 };
 
@@ -2356,6 +2370,12 @@ void register_cp_regs_for_features(ARMCPU *cpu)
     }
     if (arm_feature(env, ARM_FEATURE_EL2)) {
         define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
+    } else {
+        /* If EL2 is missing but higher ELs are enabled, we need to
+         * register the no_el2 reginfos.  */
+        if (arm_feature(env, ARM_FEATURE_EL3)) {
+            define_arm_cp_regs(cpu, v8_no_el2_cp_reginfo);
+        }
     }
     if (arm_feature(env, ARM_FEATURE_EL3)) {
         define_arm_cp_regs(cpu, v8_el3_cp_reginfo);
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v3 19/22] target-arm: A64: Register VBAR_EL2

[Peter Crosthwaite]

On Mon, May 19, 2014 at 7:23 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/cpu.h    |  2 +-
>  target-arm/helper.c | 20 ++++++++++++++++++++
>  2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 7322e03..693ad0f 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -198,7 +198,7 @@ typedef struct CPUARMState {
>          uint32_t c9_pmuserenr; /* perf monitor user enable */
>          uint32_t c9_pminten; /* perf monitor interrupt enables */
>          uint64_t mair_el1;
> -        uint64_t vbar_el[2]; /* vector base address register */
> +        uint64_t vbar_el[3]; /* vector base address register */
>          uint32_t c13_fcse; /* FCSE PID.  */
>          uint64_t contextidr_el1; /* Context ID.  */
>          uint64_t tpidr_el0; /* User RW Thread register.  */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 5a2073e..7ca63a8 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2077,6 +2077,15 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
>      REGINFO_SENTINEL
>  };
>
> +/* Used to describe the behaviour of EL2 regs when EL2 does not exist.  */
> +static const ARMCPRegInfo v8_no_el2_cp_reginfo[] = {

Should it be v8_el3_no_el2_cp_reginfo?

Otherwise,

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

> +    { .name = "VBAR_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
> +      .access = PL2_RW,
> +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> +    REGINFO_SENTINEL
> +};
> +
>  static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
>      { .name = "ELR_EL2", .state = ARM_CP_STATE_AA64,
>        .type = ARM_CP_NO_MIGRATE,
> @@ -2087,6 +2096,11 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
>        .type = ARM_CP_NO_MIGRATE,
>        .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 0,
>        .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[6]) },
> +    { .name = "VBAR_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
> +      .access = PL2_RW, .writefn = vbar_write,
> +      .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[2]),
> +      .resetvalue = 0 },
>      REGINFO_SENTINEL
>  };
>
> @@ -2356,6 +2370,12 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>      }
>      if (arm_feature(env, ARM_FEATURE_EL2)) {
>          define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
> +    } else {
> +        /* If EL2 is missing but higher ELs are enabled, we need to
> +         * register the no_el2 reginfos.  */
> +        if (arm_feature(env, ARM_FEATURE_EL3)) {
> +            define_arm_cp_regs(cpu, v8_no_el2_cp_reginfo);
> +        }
>      }
>      if (arm_feature(env, ARM_FEATURE_EL3)) {
>          define_arm_cp_regs(cpu, v8_el3_cp_reginfo);
> --
> 1.8.3.2
>
>

Re: [Qemu-devel] [PATCH v3 19/22] target-arm: A64: Register VBAR_EL2

[Edgar E. Iglesias]

On Tue, May 20, 2014 at 07:02:37PM +1000, Peter Crosthwaite wrote:
> On Mon, May 19, 2014 at 7:23 PM, Edgar E. Iglesias
> <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/cpu.h    |  2 +-
> >  target-arm/helper.c | 20 ++++++++++++++++++++
> >  2 files changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 7322e03..693ad0f 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -198,7 +198,7 @@ typedef struct CPUARMState {
> >          uint32_t c9_pmuserenr; /* perf monitor user enable */
> >          uint32_t c9_pminten; /* perf monitor interrupt enables */
> >          uint64_t mair_el1;
> > -        uint64_t vbar_el[2]; /* vector base address register */
> > +        uint64_t vbar_el[3]; /* vector base address register */
> >          uint32_t c13_fcse; /* FCSE PID.  */
> >          uint64_t contextidr_el1; /* Context ID.  */
> >          uint64_t tpidr_el0; /* User RW Thread register.  */
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 5a2073e..7ca63a8 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -2077,6 +2077,15 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
> >      REGINFO_SENTINEL
> >  };
> >
> > +/* Used to describe the behaviour of EL2 regs when EL2 does not exist.  */
> > +static const ARMCPRegInfo v8_no_el2_cp_reginfo[] = {
> 
> Should it be v8_el3_no_el2_cp_reginfo?

I've changed it to v8_el3_no_el2_cp_reginfo.

Thanks,
Edgar

> 
> Otherwise,
> 
> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Re: [Qemu-devel] [PATCH v3 19/22] target-arm: A64: Register VBAR_EL2

[Peter Maydell]

On 19 May 2014 10:23, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

> @@ -2356,6 +2370,12 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>      }
>      if (arm_feature(env, ARM_FEATURE_EL2)) {
>          define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
> +    } else {
> +        /* If EL2 is missing but higher ELs are enabled, we need to
> +         * register the no_el2 reginfos.  */

Very nitpicky, but my preferred multiline comment style puts the
final "*/" on its own line rather than at the end of the preceding.

> +        if (arm_feature(env, ARM_FEATURE_EL3)) {
> +            define_arm_cp_regs(cpu, v8_no_el2_cp_reginfo);
> +        }
>      }
>      if (arm_feature(env, ARM_FEATURE_EL3)) {
>          define_arm_cp_regs(cpu, v8_el3_cp_reginfo);
> --
> 1.8.3.2

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 19/22] target-arm: A64: Register VBAR_EL2

[Edgar E. Iglesias]

On Wed, May 21, 2014 at 08:22:27PM +0100, Peter Maydell wrote:
> On 19 May 2014 10:23, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> 
> > @@ -2356,6 +2370,12 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> >      }
> >      if (arm_feature(env, ARM_FEATURE_EL2)) {
> >          define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
> > +    } else {
> > +        /* If EL2 is missing but higher ELs are enabled, we need to
> > +         * register the no_el2 reginfos.  */
> 
> Very nitpicky, but my preferred multiline comment style puts the
> final "*/" on its own line rather than at the end of the preceding.

I changed it to your style.

Personally, I try to avoid these kind of requests because I feel it
adds unnecessary friction to upstreaming but I have no problems
with fixing it up, so feel free to keep on "nitpicking" :-)

Thanks,
Edgar


> 
> > +        if (arm_feature(env, ARM_FEATURE_EL3)) {
> > +            define_arm_cp_regs(cpu, v8_no_el2_cp_reginfo);
> > +        }
> >      }
> >      if (arm_feature(env, ARM_FEATURE_EL3)) {
> >          define_arm_cp_regs(cpu, v8_el3_cp_reginfo);
> > --
> > 1.8.3.2
> 
> thanks
> -- PMM

[Qemu-devel] [PATCH v3 20/22] target-arm: A64: Register VBAR_EL3

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h    | 2 +-
 target-arm/helper.c | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 693ad0f..566f9ed 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -198,7 +198,7 @@ typedef struct CPUARMState {
         uint32_t c9_pmuserenr; /* perf monitor user enable */
         uint32_t c9_pminten; /* perf monitor interrupt enables */
         uint64_t mair_el1;
-        uint64_t vbar_el[3]; /* vector base address register */
+        uint64_t vbar_el[4]; /* vector base address register */
         uint32_t c13_fcse; /* FCSE PID.  */
         uint64_t contextidr_el1; /* Context ID.  */
         uint64_t tpidr_el0; /* User RW Thread register.  */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 7ca63a8..9bc5b3d 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2114,6 +2114,11 @@ static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
       .type = ARM_CP_NO_MIGRATE,
       .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 0, .opc2 = 0,
       .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[7]) },
+    { .name = "VBAR_EL3", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 6, .crn = 12, .crm = 0, .opc2 = 0,
+      .access = PL3_RW, .writefn = vbar_write,
+      .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[3]),
+      .resetvalue = 0 },
     REGINFO_SENTINEL
 };
 
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 21/22] RFC: target-arm: A32: Use get_mem_index for load/stores

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Avoid using IS_USER directly as the MMU-idx to simplify future
changes to the MMU layout.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/translate.c | 220 +++++++++++++++++++++++++------------------------
 1 file changed, 113 insertions(+), 107 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index c9fa5d9..15aa00b 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -1165,18 +1165,18 @@ VFP_GEN_FIX(ulto, )
 static inline void gen_vfp_ld(DisasContext *s, int dp, TCGv_i32 addr)
 {
     if (dp) {
-        gen_aa32_ld64(cpu_F0d, addr, IS_USER(s));
+        gen_aa32_ld64(cpu_F0d, addr, get_mem_index(s));
     } else {
-        gen_aa32_ld32u(cpu_F0s, addr, IS_USER(s));
+        gen_aa32_ld32u(cpu_F0s, addr, get_mem_index(s));
     }
 }
 
 static inline void gen_vfp_st(DisasContext *s, int dp, TCGv_i32 addr)
 {
     if (dp) {
-        gen_aa32_st64(cpu_F0d, addr, IS_USER(s));
+        gen_aa32_st64(cpu_F0d, addr, get_mem_index(s));
     } else {
-        gen_aa32_st32(cpu_F0s, addr, IS_USER(s));
+        gen_aa32_st32(cpu_F0s, addr, get_mem_index(s));
     }
 }
 
@@ -1514,24 +1514,24 @@ static int disas_iwmmxt_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
         if (insn & ARM_CP_RW_BIT) {
             if ((insn >> 28) == 0xf) {			/* WLDRW wCx */
                 tmp = tcg_temp_new_i32();
-                gen_aa32_ld32u(tmp, addr, IS_USER(s));
+                gen_aa32_ld32u(tmp, addr, get_mem_index(s));
                 iwmmxt_store_creg(wrd, tmp);
             } else {
                 i = 1;
                 if (insn & (1 << 8)) {
                     if (insn & (1 << 22)) {		/* WLDRD */
-                        gen_aa32_ld64(cpu_M0, addr, IS_USER(s));
+                        gen_aa32_ld64(cpu_M0, addr, get_mem_index(s));
                         i = 0;
                     } else {				/* WLDRW wRd */
                         tmp = tcg_temp_new_i32();
-                        gen_aa32_ld32u(tmp, addr, IS_USER(s));
+                        gen_aa32_ld32u(tmp, addr, get_mem_index(s));
                     }
                 } else {
                     tmp = tcg_temp_new_i32();
                     if (insn & (1 << 22)) {		/* WLDRH */
-                        gen_aa32_ld16u(tmp, addr, IS_USER(s));
+                        gen_aa32_ld16u(tmp, addr, get_mem_index(s));
                     } else {				/* WLDRB */
-                        gen_aa32_ld8u(tmp, addr, IS_USER(s));
+                        gen_aa32_ld8u(tmp, addr, get_mem_index(s));
                     }
                 }
                 if (i) {
@@ -1543,24 +1543,24 @@ static int disas_iwmmxt_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
         } else {
             if ((insn >> 28) == 0xf) {			/* WSTRW wCx */
                 tmp = iwmmxt_load_creg(wrd);
-                gen_aa32_st32(tmp, addr, IS_USER(s));
+                gen_aa32_st32(tmp, addr, get_mem_index(s));
             } else {
                 gen_op_iwmmxt_movq_M0_wRn(wrd);
                 tmp = tcg_temp_new_i32();
                 if (insn & (1 << 8)) {
                     if (insn & (1 << 22)) {		/* WSTRD */
-                        gen_aa32_st64(cpu_M0, addr, IS_USER(s));
+                        gen_aa32_st64(cpu_M0, addr, get_mem_index(s));
                     } else {				/* WSTRW wRd */
                         tcg_gen_trunc_i64_i32(tmp, cpu_M0);
-                        gen_aa32_st32(tmp, addr, IS_USER(s));
+                        gen_aa32_st32(tmp, addr, get_mem_index(s));
                     }
                 } else {
                     if (insn & (1 << 22)) {		/* WSTRH */
                         tcg_gen_trunc_i64_i32(tmp, cpu_M0);
-                        gen_aa32_st16(tmp, addr, IS_USER(s));
+                        gen_aa32_st16(tmp, addr, get_mem_index(s));
                     } else {				/* WSTRB */
                         tcg_gen_trunc_i64_i32(tmp, cpu_M0);
-                        gen_aa32_st8(tmp, addr, IS_USER(s));
+                        gen_aa32_st8(tmp, addr, get_mem_index(s));
                     }
                 }
             }
@@ -2625,15 +2625,15 @@ static TCGv_i32 gen_load_and_replicate(DisasContext *s, TCGv_i32 addr, int size)
     TCGv_i32 tmp = tcg_temp_new_i32();
     switch (size) {
     case 0:
-        gen_aa32_ld8u(tmp, addr, IS_USER(s));
+        gen_aa32_ld8u(tmp, addr, get_mem_index(s));
         gen_neon_dup_u8(tmp, 0);
         break;
     case 1:
-        gen_aa32_ld16u(tmp, addr, IS_USER(s));
+        gen_aa32_ld16u(tmp, addr, get_mem_index(s));
         gen_neon_dup_low16(tmp);
         break;
     case 2:
-        gen_aa32_ld32u(tmp, addr, IS_USER(s));
+        gen_aa32_ld32u(tmp, addr, get_mem_index(s));
         break;
     default: /* Avoid compiler warnings.  */
         abort();
@@ -4304,11 +4304,11 @@ static int disas_neon_ls_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
             if (size == 3) {
                 tmp64 = tcg_temp_new_i64();
                 if (load) {
-                    gen_aa32_ld64(tmp64, addr, IS_USER(s));
+                    gen_aa32_ld64(tmp64, addr, get_mem_index(s));
                     neon_store_reg64(tmp64, rd);
                 } else {
                     neon_load_reg64(tmp64, rd);
-                    gen_aa32_st64(tmp64, addr, IS_USER(s));
+                    gen_aa32_st64(tmp64, addr, get_mem_index(s));
                 }
                 tcg_temp_free_i64(tmp64);
                 tcg_gen_addi_i32(addr, addr, stride);
@@ -4317,21 +4317,21 @@ static int disas_neon_ls_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
                     if (size == 2) {
                         if (load) {
                             tmp = tcg_temp_new_i32();
-                            gen_aa32_ld32u(tmp, addr, IS_USER(s));
+                            gen_aa32_ld32u(tmp, addr, get_mem_index(s));
                             neon_store_reg(rd, pass, tmp);
                         } else {
                             tmp = neon_load_reg(rd, pass);
-                            gen_aa32_st32(tmp, addr, IS_USER(s));
+                            gen_aa32_st32(tmp, addr, get_mem_index(s));
                             tcg_temp_free_i32(tmp);
                         }
                         tcg_gen_addi_i32(addr, addr, stride);
                     } else if (size == 1) {
                         if (load) {
                             tmp = tcg_temp_new_i32();
-                            gen_aa32_ld16u(tmp, addr, IS_USER(s));
+                            gen_aa32_ld16u(tmp, addr, get_mem_index(s));
                             tcg_gen_addi_i32(addr, addr, stride);
                             tmp2 = tcg_temp_new_i32();
-                            gen_aa32_ld16u(tmp2, addr, IS_USER(s));
+                            gen_aa32_ld16u(tmp2, addr, get_mem_index(s));
                             tcg_gen_addi_i32(addr, addr, stride);
                             tcg_gen_shli_i32(tmp2, tmp2, 16);
                             tcg_gen_or_i32(tmp, tmp, tmp2);
@@ -4341,10 +4341,10 @@ static int disas_neon_ls_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
                             tmp = neon_load_reg(rd, pass);
                             tmp2 = tcg_temp_new_i32();
                             tcg_gen_shri_i32(tmp2, tmp, 16);
-                            gen_aa32_st16(tmp, addr, IS_USER(s));
+                            gen_aa32_st16(tmp, addr, get_mem_index(s));
                             tcg_temp_free_i32(tmp);
                             tcg_gen_addi_i32(addr, addr, stride);
-                            gen_aa32_st16(tmp2, addr, IS_USER(s));
+                            gen_aa32_st16(tmp2, addr, get_mem_index(s));
                             tcg_temp_free_i32(tmp2);
                             tcg_gen_addi_i32(addr, addr, stride);
                         }
@@ -4353,7 +4353,7 @@ static int disas_neon_ls_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
                             TCGV_UNUSED_I32(tmp2);
                             for (n = 0; n < 4; n++) {
                                 tmp = tcg_temp_new_i32();
-                                gen_aa32_ld8u(tmp, addr, IS_USER(s));
+                                gen_aa32_ld8u(tmp, addr, get_mem_index(s));
                                 tcg_gen_addi_i32(addr, addr, stride);
                                 if (n == 0) {
                                     tmp2 = tmp;
@@ -4373,7 +4373,7 @@ static int disas_neon_ls_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
                                 } else {
                                     tcg_gen_shri_i32(tmp, tmp2, n * 8);
                                 }
-                                gen_aa32_st8(tmp, addr, IS_USER(s));
+                                gen_aa32_st8(tmp, addr, get_mem_index(s));
                                 tcg_temp_free_i32(tmp);
                                 tcg_gen_addi_i32(addr, addr, stride);
                             }
@@ -4497,13 +4497,13 @@ static int disas_neon_ls_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
                     tmp = tcg_temp_new_i32();
                     switch (size) {
                     case 0:
-                        gen_aa32_ld8u(tmp, addr, IS_USER(s));
+                        gen_aa32_ld8u(tmp, addr, get_mem_index(s));
                         break;
                     case 1:
-                        gen_aa32_ld16u(tmp, addr, IS_USER(s));
+                        gen_aa32_ld16u(tmp, addr, get_mem_index(s));
                         break;
                     case 2:
-                        gen_aa32_ld32u(tmp, addr, IS_USER(s));
+                        gen_aa32_ld32u(tmp, addr, get_mem_index(s));
                         break;
                     default: /* Avoid compiler warnings.  */
                         abort();
@@ -4521,13 +4521,13 @@ static int disas_neon_ls_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
                         tcg_gen_shri_i32(tmp, tmp, shift);
                     switch (size) {
                     case 0:
-                        gen_aa32_st8(tmp, addr, IS_USER(s));
+                        gen_aa32_st8(tmp, addr, get_mem_index(s));
                         break;
                     case 1:
-                        gen_aa32_st16(tmp, addr, IS_USER(s));
+                        gen_aa32_st16(tmp, addr, get_mem_index(s));
                         break;
                     case 2:
-                        gen_aa32_st32(tmp, addr, IS_USER(s));
+                        gen_aa32_st32(tmp, addr, get_mem_index(s));
                         break;
                     }
                     tcg_temp_free_i32(tmp);
@@ -7173,14 +7173,14 @@ static void gen_load_exclusive(DisasContext *s, int rt, int rt2,
 
     switch (size) {
     case 0:
-        gen_aa32_ld8u(tmp, addr, IS_USER(s));
+        gen_aa32_ld8u(tmp, addr, get_mem_index(s));
         break;
     case 1:
-        gen_aa32_ld16u(tmp, addr, IS_USER(s));
+        gen_aa32_ld16u(tmp, addr, get_mem_index(s));
         break;
     case 2:
     case 3:
-        gen_aa32_ld32u(tmp, addr, IS_USER(s));
+        gen_aa32_ld32u(tmp, addr, get_mem_index(s));
         break;
     default:
         abort();
@@ -7191,7 +7191,7 @@ static void gen_load_exclusive(DisasContext *s, int rt, int rt2,
         TCGv_i32 tmp3 = tcg_temp_new_i32();
 
         tcg_gen_addi_i32(tmp2, addr, 4);
-        gen_aa32_ld32u(tmp3, tmp2, IS_USER(s));
+        gen_aa32_ld32u(tmp3, tmp2, get_mem_index(s));
         tcg_temp_free_i32(tmp2);
         tcg_gen_concat_i32_i64(cpu_exclusive_val, tmp, tmp3);
         store_reg(s, rt2, tmp3);
@@ -7242,14 +7242,14 @@ static void gen_store_exclusive(DisasContext *s, int rd, int rt, int rt2,
     tmp = tcg_temp_new_i32();
     switch (size) {
     case 0:
-        gen_aa32_ld8u(tmp, addr, IS_USER(s));
+        gen_aa32_ld8u(tmp, addr, get_mem_index(s));
         break;
     case 1:
-        gen_aa32_ld16u(tmp, addr, IS_USER(s));
+        gen_aa32_ld16u(tmp, addr, get_mem_index(s));
         break;
     case 2:
     case 3:
-        gen_aa32_ld32u(tmp, addr, IS_USER(s));
+        gen_aa32_ld32u(tmp, addr, get_mem_index(s));
         break;
     default:
         abort();
@@ -7260,7 +7260,7 @@ static void gen_store_exclusive(DisasContext *s, int rd, int rt, int rt2,
         TCGv_i32 tmp2 = tcg_temp_new_i32();
         TCGv_i32 tmp3 = tcg_temp_new_i32();
         tcg_gen_addi_i32(tmp2, addr, 4);
-        gen_aa32_ld32u(tmp3, tmp2, IS_USER(s));
+        gen_aa32_ld32u(tmp3, tmp2, get_mem_index(s));
         tcg_temp_free_i32(tmp2);
         tcg_gen_concat_i32_i64(val64, tmp, tmp3);
         tcg_temp_free_i32(tmp3);
@@ -7275,14 +7275,14 @@ static void gen_store_exclusive(DisasContext *s, int rd, int rt, int rt2,
     tmp = load_reg(s, rt);
     switch (size) {
     case 0:
-        gen_aa32_st8(tmp, addr, IS_USER(s));
+        gen_aa32_st8(tmp, addr, get_mem_index(s));
         break;
     case 1:
-        gen_aa32_st16(tmp, addr, IS_USER(s));
+        gen_aa32_st16(tmp, addr, get_mem_index(s));
         break;
     case 2:
     case 3:
-        gen_aa32_st32(tmp, addr, IS_USER(s));
+        gen_aa32_st32(tmp, addr, get_mem_index(s));
         break;
     default:
         abort();
@@ -7291,7 +7291,7 @@ static void gen_store_exclusive(DisasContext *s, int rd, int rt, int rt2,
     if (size == 3) {
         tcg_gen_addi_i32(addr, addr, 4);
         tmp = load_reg(s, rt2);
-        gen_aa32_st32(tmp, addr, IS_USER(s));
+        gen_aa32_st32(tmp, addr, get_mem_index(s));
         tcg_temp_free_i32(tmp);
     }
     tcg_gen_movi_i32(cpu_R[rd], 0);
@@ -7495,10 +7495,10 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
                 tcg_gen_addi_i32(addr, addr, offset);
             /* Load PC into tmp and CPSR into tmp2.  */
             tmp = tcg_temp_new_i32();
-            gen_aa32_ld32u(tmp, addr, 0);
+            gen_aa32_ld32u(tmp, addr, get_mem_index(s));
             tcg_gen_addi_i32(addr, addr, 4);
             tmp2 = tcg_temp_new_i32();
-            gen_aa32_ld32u(tmp2, addr, 0);
+            gen_aa32_ld32u(tmp2, addr, get_mem_index(s));
             if (insn & (1 << 21)) {
                 /* Base writeback.  */
                 switch (i) {
@@ -8087,13 +8087,13 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
                                 tmp = tcg_temp_new_i32();
                                 switch (op1) {
                                 case 0: /* lda */
-                                    gen_aa32_ld32u(tmp, addr, IS_USER(s));
+                                    gen_aa32_ld32u(tmp, addr, get_mem_index(s));
                                     break;
                                 case 2: /* ldab */
-                                    gen_aa32_ld8u(tmp, addr, IS_USER(s));
+                                    gen_aa32_ld8u(tmp, addr, get_mem_index(s));
                                     break;
                                 case 3: /* ldah */
-                                    gen_aa32_ld16u(tmp, addr, IS_USER(s));
+                                    gen_aa32_ld16u(tmp, addr, get_mem_index(s));
                                     break;
                                 default:
                                     abort();
@@ -8104,13 +8104,13 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
                                 tmp = load_reg(s, rm);
                                 switch (op1) {
                                 case 0: /* stl */
-                                    gen_aa32_st32(tmp, addr, IS_USER(s));
+                                    gen_aa32_st32(tmp, addr, get_mem_index(s));
                                     break;
                                 case 2: /* stlb */
-                                    gen_aa32_st8(tmp, addr, IS_USER(s));
+                                    gen_aa32_st8(tmp, addr, get_mem_index(s));
                                     break;
                                 case 3: /* stlh */
-                                    gen_aa32_st16(tmp, addr, IS_USER(s));
+                                    gen_aa32_st16(tmp, addr, get_mem_index(s));
                                     break;
                                 default:
                                     abort();
@@ -8165,11 +8165,11 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
                         tmp = load_reg(s, rm);
                         tmp2 = tcg_temp_new_i32();
                         if (insn & (1 << 22)) {
-                            gen_aa32_ld8u(tmp2, addr, IS_USER(s));
-                            gen_aa32_st8(tmp, addr, IS_USER(s));
+                            gen_aa32_ld8u(tmp2, addr, get_mem_index(s));
+                            gen_aa32_st8(tmp, addr, get_mem_index(s));
                         } else {
-                            gen_aa32_ld32u(tmp2, addr, IS_USER(s));
-                            gen_aa32_st32(tmp, addr, IS_USER(s));
+                            gen_aa32_ld32u(tmp2, addr, get_mem_index(s));
+                            gen_aa32_st32(tmp, addr, get_mem_index(s));
                         }
                         tcg_temp_free_i32(tmp);
                         tcg_temp_free_i32(addr);
@@ -8191,14 +8191,14 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
                     tmp = tcg_temp_new_i32();
                     switch(sh) {
                     case 1:
-                        gen_aa32_ld16u(tmp, addr, IS_USER(s));
+                        gen_aa32_ld16u(tmp, addr, get_mem_index(s));
                         break;
                     case 2:
-                        gen_aa32_ld8s(tmp, addr, IS_USER(s));
+                        gen_aa32_ld8s(tmp, addr, get_mem_index(s));
                         break;
                     default:
                     case 3:
-                        gen_aa32_ld16s(tmp, addr, IS_USER(s));
+                        gen_aa32_ld16s(tmp, addr, get_mem_index(s));
                         break;
                     }
                     load = 1;
@@ -8208,21 +8208,21 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
                     if (sh & 1) {
                         /* store */
                         tmp = load_reg(s, rd);
-                        gen_aa32_st32(tmp, addr, IS_USER(s));
+                        gen_aa32_st32(tmp, addr, get_mem_index(s));
                         tcg_temp_free_i32(tmp);
                         tcg_gen_addi_i32(addr, addr, 4);
                         tmp = load_reg(s, rd + 1);
-                        gen_aa32_st32(tmp, addr, IS_USER(s));
+                        gen_aa32_st32(tmp, addr, get_mem_index(s));
                         tcg_temp_free_i32(tmp);
                         load = 0;
                     } else {
                         /* load */
                         tmp = tcg_temp_new_i32();
-                        gen_aa32_ld32u(tmp, addr, IS_USER(s));
+                        gen_aa32_ld32u(tmp, addr, get_mem_index(s));
                         store_reg(s, rd, tmp);
                         tcg_gen_addi_i32(addr, addr, 4);
                         tmp = tcg_temp_new_i32();
-                        gen_aa32_ld32u(tmp, addr, IS_USER(s));
+                        gen_aa32_ld32u(tmp, addr, get_mem_index(s));
                         rd++;
                         load = 1;
                     }
@@ -8230,7 +8230,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
                 } else {
                     /* store */
                     tmp = load_reg(s, rd);
-                    gen_aa32_st16(tmp, addr, IS_USER(s));
+                    gen_aa32_st16(tmp, addr, get_mem_index(s));
                     tcg_temp_free_i32(tmp);
                     load = 0;
                 }
@@ -8568,7 +8568,11 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
             rn = (insn >> 16) & 0xf;
             rd = (insn >> 12) & 0xf;
             tmp2 = load_reg(s, rn);
-            i = (IS_USER(s) || (insn & 0x01200000) == 0x00200000);
+            if (IS_USER(s) || (insn & 0x01200000) == 0x00200000) {
+                i = 0;
+            } else {
+                i = get_mem_index(s);
+            }
             if (insn & (1 << 24))
                 gen_add_data_offset(s, insn, tmp2);
             if (insn & (1 << 20)) {
@@ -8652,7 +8656,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
                         if (insn & (1 << 20)) {
                             /* load */
                             tmp = tcg_temp_new_i32();
-                            gen_aa32_ld32u(tmp, addr, IS_USER(s));
+                            gen_aa32_ld32u(tmp, addr, get_mem_index(s));
                             if (user) {
                                 tmp2 = tcg_const_i32(i);
                                 gen_helper_set_user_reg(cpu_env, tmp2, tmp);
@@ -8679,7 +8683,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
                             } else {
                                 tmp = load_reg(s, i);
                             }
-                            gen_aa32_st32(tmp, addr, IS_USER(s));
+                            gen_aa32_st32(tmp, addr, get_mem_index(s));
                             tcg_temp_free_i32(tmp);
                         }
                         j++;
@@ -8945,20 +8949,20 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
                 if (insn & (1 << 20)) {
                     /* ldrd */
                     tmp = tcg_temp_new_i32();
-                    gen_aa32_ld32u(tmp, addr, IS_USER(s));
+                    gen_aa32_ld32u(tmp, addr, get_mem_index(s));
                     store_reg(s, rs, tmp);
                     tcg_gen_addi_i32(addr, addr, 4);
                     tmp = tcg_temp_new_i32();
-                    gen_aa32_ld32u(tmp, addr, IS_USER(s));
+                    gen_aa32_ld32u(tmp, addr, get_mem_index(s));
                     store_reg(s, rd, tmp);
                 } else {
                     /* strd */
                     tmp = load_reg(s, rs);
-                    gen_aa32_st32(tmp, addr, IS_USER(s));
+                    gen_aa32_st32(tmp, addr, get_mem_index(s));
                     tcg_temp_free_i32(tmp);
                     tcg_gen_addi_i32(addr, addr, 4);
                     tmp = load_reg(s, rd);
-                    gen_aa32_st32(tmp, addr, IS_USER(s));
+                    gen_aa32_st32(tmp, addr, get_mem_index(s));
                     tcg_temp_free_i32(tmp);
                 }
                 if (insn & (1 << 21)) {
@@ -8996,11 +9000,11 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
                     tcg_gen_add_i32(addr, addr, tmp);
                     tcg_temp_free_i32(tmp);
                     tmp = tcg_temp_new_i32();
-                    gen_aa32_ld16u(tmp, addr, IS_USER(s));
+                    gen_aa32_ld16u(tmp, addr, get_mem_index(s));
                 } else { /* tbb */
                     tcg_temp_free_i32(tmp);
                     tmp = tcg_temp_new_i32();
-                    gen_aa32_ld8u(tmp, addr, IS_USER(s));
+                    gen_aa32_ld8u(tmp, addr, get_mem_index(s));
                 }
                 tcg_temp_free_i32(addr);
                 tcg_gen_shli_i32(tmp, tmp, 1);
@@ -9037,13 +9041,13 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
                         tmp = tcg_temp_new_i32();
                         switch (op) {
                         case 0: /* ldab */
-                            gen_aa32_ld8u(tmp, addr, IS_USER(s));
+                            gen_aa32_ld8u(tmp, addr, get_mem_index(s));
                             break;
                         case 1: /* ldah */
-                            gen_aa32_ld16u(tmp, addr, IS_USER(s));
+                            gen_aa32_ld16u(tmp, addr, get_mem_index(s));
                             break;
                         case 2: /* lda */
-                            gen_aa32_ld32u(tmp, addr, IS_USER(s));
+                            gen_aa32_ld32u(tmp, addr, get_mem_index(s));
                             break;
                         default:
                             abort();
@@ -9053,13 +9057,13 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
                         tmp = load_reg(s, rs);
                         switch (op) {
                         case 0: /* stlb */
-                            gen_aa32_st8(tmp, addr, IS_USER(s));
+                            gen_aa32_st8(tmp, addr, get_mem_index(s));
                             break;
                         case 1: /* stlh */
-                            gen_aa32_st16(tmp, addr, IS_USER(s));
+                            gen_aa32_st16(tmp, addr, get_mem_index(s));
                             break;
                         case 2: /* stl */
-                            gen_aa32_st32(tmp, addr, IS_USER(s));
+                            gen_aa32_st32(tmp, addr, get_mem_index(s));
                             break;
                         default:
                             abort();
@@ -9087,10 +9091,10 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
                         tcg_gen_addi_i32(addr, addr, -8);
                     /* Load PC into tmp and CPSR into tmp2.  */
                     tmp = tcg_temp_new_i32();
-                    gen_aa32_ld32u(tmp, addr, 0);
+                    gen_aa32_ld32u(tmp, addr, get_mem_index(s));
                     tcg_gen_addi_i32(addr, addr, 4);
                     tmp2 = tcg_temp_new_i32();
-                    gen_aa32_ld32u(tmp2, addr, 0);
+                    gen_aa32_ld32u(tmp2, addr, get_mem_index(s));
                     if (insn & (1 << 21)) {
                         /* Base writeback.  */
                         if (insn & (1 << 24)) {
@@ -9129,7 +9133,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
                     if (insn & (1 << 20)) {
                         /* Load.  */
                         tmp = tcg_temp_new_i32();
-                        gen_aa32_ld32u(tmp, addr, IS_USER(s));
+                        gen_aa32_ld32u(tmp, addr, get_mem_index(s));
                         if (i == 15) {
                             gen_bx(s, tmp);
                         } else if (i == rn) {
@@ -9141,7 +9145,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
                     } else {
                         /* Store.  */
                         tmp = load_reg(s, i);
-                        gen_aa32_st32(tmp, addr, IS_USER(s));
+                        gen_aa32_st32(tmp, addr, get_mem_index(s));
                         tcg_temp_free_i32(tmp);
                     }
                     tcg_gen_addi_i32(addr, addr, 4);
@@ -9946,6 +9950,8 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
         }
         if (insn & (1 << 20)) {
             /* Load.  */
+            user = user ? 0 : get_mem_index(s);
+
             tmp = tcg_temp_new_i32();
             switch (op) {
             case 0:
@@ -10121,7 +10127,7 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
             addr = tcg_temp_new_i32();
             tcg_gen_movi_i32(addr, val);
             tmp = tcg_temp_new_i32();
-            gen_aa32_ld32u(tmp, addr, IS_USER(s));
+            gen_aa32_ld32u(tmp, addr, get_mem_index(s));
             tcg_temp_free_i32(addr);
             store_reg(s, rd, tmp);
             break;
@@ -10324,28 +10330,28 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
 
         switch (op) {
         case 0: /* str */
-            gen_aa32_st32(tmp, addr, IS_USER(s));
+            gen_aa32_st32(tmp, addr, get_mem_index(s));
             break;
         case 1: /* strh */
-            gen_aa32_st16(tmp, addr, IS_USER(s));
+            gen_aa32_st16(tmp, addr, get_mem_index(s));
             break;
         case 2: /* strb */
-            gen_aa32_st8(tmp, addr, IS_USER(s));
+            gen_aa32_st8(tmp, addr, get_mem_index(s));
             break;
         case 3: /* ldrsb */
-            gen_aa32_ld8s(tmp, addr, IS_USER(s));
+            gen_aa32_ld8s(tmp, addr, get_mem_index(s));
             break;
         case 4: /* ldr */
-            gen_aa32_ld32u(tmp, addr, IS_USER(s));
+            gen_aa32_ld32u(tmp, addr, get_mem_index(s));
             break;
         case 5: /* ldrh */
-            gen_aa32_ld16u(tmp, addr, IS_USER(s));
+            gen_aa32_ld16u(tmp, addr, get_mem_index(s));
             break;
         case 6: /* ldrb */
-            gen_aa32_ld8u(tmp, addr, IS_USER(s));
+            gen_aa32_ld8u(tmp, addr, get_mem_index(s));
             break;
         case 7: /* ldrsh */
-            gen_aa32_ld16s(tmp, addr, IS_USER(s));
+            gen_aa32_ld16s(tmp, addr, get_mem_index(s));
             break;
         }
         if (op >= 3) { /* load */
@@ -10367,12 +10373,12 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
         if (insn & (1 << 11)) {
             /* load */
             tmp = tcg_temp_new_i32();
-            gen_aa32_ld32u(tmp, addr, IS_USER(s));
+            gen_aa32_ld32u(tmp, addr, get_mem_index(s));
             store_reg(s, rd, tmp);
         } else {
             /* store */
             tmp = load_reg(s, rd);
-            gen_aa32_st32(tmp, addr, IS_USER(s));
+            gen_aa32_st32(tmp, addr, get_mem_index(s));
             tcg_temp_free_i32(tmp);
         }
         tcg_temp_free_i32(addr);
@@ -10389,12 +10395,12 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
         if (insn & (1 << 11)) {
             /* load */
             tmp = tcg_temp_new_i32();
-            gen_aa32_ld8u(tmp, addr, IS_USER(s));
+            gen_aa32_ld8u(tmp, addr, get_mem_index(s));
             store_reg(s, rd, tmp);
         } else {
             /* store */
             tmp = load_reg(s, rd);
-            gen_aa32_st8(tmp, addr, IS_USER(s));
+            gen_aa32_st8(tmp, addr, get_mem_index(s));
             tcg_temp_free_i32(tmp);
         }
         tcg_temp_free_i32(addr);
@@ -10411,12 +10417,12 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
         if (insn & (1 << 11)) {
             /* load */
             tmp = tcg_temp_new_i32();
-            gen_aa32_ld16u(tmp, addr, IS_USER(s));
+            gen_aa32_ld16u(tmp, addr, get_mem_index(s));
             store_reg(s, rd, tmp);
         } else {
             /* store */
             tmp = load_reg(s, rd);
-            gen_aa32_st16(tmp, addr, IS_USER(s));
+            gen_aa32_st16(tmp, addr, get_mem_index(s));
             tcg_temp_free_i32(tmp);
         }
         tcg_temp_free_i32(addr);
@@ -10432,12 +10438,12 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
         if (insn & (1 << 11)) {
             /* load */
             tmp = tcg_temp_new_i32();
-            gen_aa32_ld32u(tmp, addr, IS_USER(s));
+            gen_aa32_ld32u(tmp, addr, get_mem_index(s));
             store_reg(s, rd, tmp);
         } else {
             /* store */
             tmp = load_reg(s, rd);
-            gen_aa32_st32(tmp, addr, IS_USER(s));
+            gen_aa32_st32(tmp, addr, get_mem_index(s));
             tcg_temp_free_i32(tmp);
         }
         tcg_temp_free_i32(addr);
@@ -10505,12 +10511,12 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
                     if (insn & (1 << 11)) {
                         /* pop */
                         tmp = tcg_temp_new_i32();
-                        gen_aa32_ld32u(tmp, addr, IS_USER(s));
+                        gen_aa32_ld32u(tmp, addr, get_mem_index(s));
                         store_reg(s, i, tmp);
                     } else {
                         /* push */
                         tmp = load_reg(s, i);
-                        gen_aa32_st32(tmp, addr, IS_USER(s));
+                        gen_aa32_st32(tmp, addr, get_mem_index(s));
                         tcg_temp_free_i32(tmp);
                     }
                     /* advance to the next address.  */
@@ -10522,13 +10528,13 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
                 if (insn & (1 << 11)) {
                     /* pop pc */
                     tmp = tcg_temp_new_i32();
-                    gen_aa32_ld32u(tmp, addr, IS_USER(s));
+                    gen_aa32_ld32u(tmp, addr, get_mem_index(s));
                     /* don't set the pc until the rest of the instruction
                        has completed */
                 } else {
                     /* push lr */
                     tmp = load_reg(s, 14);
-                    gen_aa32_st32(tmp, addr, IS_USER(s));
+                    gen_aa32_st32(tmp, addr, get_mem_index(s));
                     tcg_temp_free_i32(tmp);
                 }
                 tcg_gen_addi_i32(addr, addr, 4);
@@ -10657,7 +10663,7 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
                 if (insn & (1 << 11)) {
                     /* load */
                     tmp = tcg_temp_new_i32();
-                    gen_aa32_ld32u(tmp, addr, IS_USER(s));
+                    gen_aa32_ld32u(tmp, addr, get_mem_index(s));
                     if (i == rn) {
                         loaded_var = tmp;
                     } else {
@@ -10666,7 +10672,7 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
                 } else {
                     /* store */
                     tmp = load_reg(s, i);
-                    gen_aa32_st32(tmp, addr, IS_USER(s));
+                    gen_aa32_st32(tmp, addr, get_mem_index(s));
                     tcg_temp_free_i32(tmp);
                 }
                 /* advance to the next address */
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v3 21/22] RFC: target-arm: A32: Use get_mem_index for load/stores

[Peter Maydell]

On 19 May 2014 10:23, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Avoid using IS_USER directly as the MMU-idx to simplify future
> changes to the MMU layout.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

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

I think this makes sense; the bits involving the ldrt/strt handling
are a little ugly after this change but we could probably clean
that up later.

-- PMM

Re: [Qemu-devel] [PATCH v3 21/22] RFC: target-arm: A32: Use get_mem_index for load/stores

[Edgar E. Iglesias]

On Wed, May 21, 2014 at 08:27:52PM +0100, Peter Maydell wrote:
> On 19 May 2014 10:23, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Avoid using IS_USER directly as the MMU-idx to simplify future
> > changes to the MMU layout.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> I think this makes sense; the bits involving the ldrt/strt handling
> are a little ugly after this change but we could probably clean
> that up later.

Thanks, I'll squash this into v4 then!

Cheers,
Edgar

[Qemu-devel] [PATCH v3 22/22] RFC: target-arm: Use a 1:1 mapping between EL and MMU index

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
---
 target-arm/cpu.h       | 26 ++++----------------------
 target-arm/translate.h |  2 +-
 2 files changed, 5 insertions(+), 23 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 566f9ed..3b7ef32 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1084,32 +1084,14 @@ static inline CPUARMState *cpu_init(const char *cpu_model)
 #define cpu_list arm_cpu_list
 
 /* MMU modes definitions */
-#define MMU_MODE0_SUFFIX _kernel
-#define MMU_MODE1_SUFFIX _user
-#define MMU_USER_IDX 1
-
-static inline int arm_el_to_mmu_idx(int current_el)
-{
-#ifdef CONFIG_USER_ONLY
-    return MMU_USER_IDX;
-#else
-    switch (current_el) {
-    case 0:
-        return MMU_USER_IDX;
-    case 1:
-        return 0;
-    default:
-        /* Unsupported EL.  */
-        assert(0);
-        return 0;
-    }
-#endif
-}
+#define MMU_MODE0_SUFFIX _user
+#define MMU_MODE1_SUFFIX _kernel
+#define MMU_USER_IDX 0
 
 static inline int cpu_mmu_index (CPUARMState *env)
 {
     int cur_el = arm_current_pl(env);
-    return arm_el_to_mmu_idx(cur_el);
+    return cur_el;
 }
 
 #include "exec/cpu-all.h"
diff --git a/target-arm/translate.h b/target-arm/translate.h
index db6f0af..31a0104 100644
--- a/target-arm/translate.h
+++ b/target-arm/translate.h
@@ -54,7 +54,7 @@ static inline int arm_dc_feature(DisasContext *dc, int feature)
 
 static inline int get_mem_index(DisasContext *s)
 {
-    return arm_el_to_mmu_idx(s->current_pl);
+    return s->current_pl;
 }
 
 /* target-specific extra values for is_jmp */
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v3 22/22] RFC: target-arm: Use a 1:1 mapping between EL and MMU index

[Peter Crosthwaite]

On Mon, May 19, 2014 at 7:23 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> ---
>  target-arm/cpu.h       | 26 ++++----------------------
>  target-arm/translate.h |  2 +-
>  2 files changed, 5 insertions(+), 23 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 566f9ed..3b7ef32 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1084,32 +1084,14 @@ static inline CPUARMState *cpu_init(const char *cpu_model)
>  #define cpu_list arm_cpu_list
>
>  /* MMU modes definitions */
> -#define MMU_MODE0_SUFFIX _kernel
> -#define MMU_MODE1_SUFFIX _user
> -#define MMU_USER_IDX 1
> -
> -static inline int arm_el_to_mmu_idx(int current_el)
> -{
> -#ifdef CONFIG_USER_ONLY
> -    return MMU_USER_IDX;
> -#else
> -    switch (current_el) {
> -    case 0:
> -        return MMU_USER_IDX;
> -    case 1:
> -        return 0;
> -    default:
> -        /* Unsupported EL.  */
> -        assert(0);
> -        return 0;
> -    }
> -#endif
> -}
> +#define MMU_MODE0_SUFFIX _user
> +#define MMU_MODE1_SUFFIX _kernel
> +#define MMU_USER_IDX 0
>
>  static inline int cpu_mmu_index (CPUARMState *env)
>  {
>      int cur_el = arm_current_pl(env);
> -    return arm_el_to_mmu_idx(cur_el);
> +    return cur_el;
>  }
>
>  #include "exec/cpu-all.h"
> diff --git a/target-arm/translate.h b/target-arm/translate.h
> index db6f0af..31a0104 100644
> --- a/target-arm/translate.h
> +++ b/target-arm/translate.h
> @@ -54,7 +54,7 @@ static inline int arm_dc_feature(DisasContext *dc, int feature)
>
>  static inline int get_mem_index(DisasContext *s)
>  {
> -    return arm_el_to_mmu_idx(s->current_pl);
> +    return s->current_pl;
>  }
>
>  /* target-specific extra values for is_jmp */
> --
> 1.8.3.2
>
>

I'm a fan of this change. Should it squash into earlier in the series?

Regards,
Peter

Re: [Qemu-devel] [PATCH v3 22/22] RFC: target-arm: Use a 1:1 mapping between EL and MMU index

[Aggeler Fabian]

I guess this makes sense. Shouldn’t we implement two more MMUs to separate S-EL0/EL0 and S-EL1/EL1
at least for ARMv8 with EL3 running in Aarch64 state? For ARMv7 and ARMv8 with EL3 in Aarch32 S-PL1 
is mapped to PL3, so we only need one additional MMU for S-PL0. If you agree I could add this change in 
the Security Extension patches after this patch makes it into the tree.

Best,
Fabian

On 19 May 2014, at 11:23, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:

> From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> ---
> target-arm/cpu.h       | 26 ++++----------------------
> target-arm/translate.h |  2 +-
> 2 files changed, 5 insertions(+), 23 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 566f9ed..3b7ef32 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1084,32 +1084,14 @@ static inline CPUARMState *cpu_init(const char *cpu_model)
> #define cpu_list arm_cpu_list
> 
> /* MMU modes definitions */
> -#define MMU_MODE0_SUFFIX _kernel
> -#define MMU_MODE1_SUFFIX _user
> -#define MMU_USER_IDX 1
> -
> -static inline int arm_el_to_mmu_idx(int current_el)
> -{
> -#ifdef CONFIG_USER_ONLY
> -    return MMU_USER_IDX;
> -#else
> -    switch (current_el) {
> -    case 0:
> -        return MMU_USER_IDX;
> -    case 1:
> -        return 0;
> -    default:
> -        /* Unsupported EL.  */
> -        assert(0);
> -        return 0;
> -    }
> -#endif
> -}
> +#define MMU_MODE0_SUFFIX _user
> +#define MMU_MODE1_SUFFIX _kernel
> +#define MMU_USER_IDX 0
> 
> static inline int cpu_mmu_index (CPUARMState *env)
> {
>     int cur_el = arm_current_pl(env);
> -    return arm_el_to_mmu_idx(cur_el);
> +    return cur_el;
> }
> 
> #include "exec/cpu-all.h"
> diff --git a/target-arm/translate.h b/target-arm/translate.h
> index db6f0af..31a0104 100644
> --- a/target-arm/translate.h
> +++ b/target-arm/translate.h
> @@ -54,7 +54,7 @@ static inline int arm_dc_feature(DisasContext *dc, int feature)
> 
> static inline int get_mem_index(DisasContext *s)
> {
> -    return arm_el_to_mmu_idx(s->current_pl);
> +    return s->current_pl;
> }
> 
> /* target-specific extra values for is_jmp */
> -- 
> 1.8.3.2
> 

Re: [Qemu-devel] [PATCH v3 22/22] RFC: target-arm: Use a 1:1 mapping between EL and MMU index

[Edgar E. Iglesias]

On Tue, May 20, 2014 at 09:47:47AM +0000, Aggeler  Fabian wrote:
> I guess this makes sense. Shouldn’t we implement two more MMUs to separate S-EL0/EL0 and S-EL1/EL1
> at least for ARMv8 with EL3 running in Aarch64 state?

Maybe with future patches. My understanding is that on aarch64 the world
switch between S/NS requires EL3 firmware to reprogram the TTBR regs.
Currently in QEMU, the re-programming of TTBR will flush the TLBs. We would
need to do something about that before adding MMU tables for aarch64 Secure
EL0/1 does any good. I think it's better to keep it simple for now and leave
this as a possible future optimization.

Another possible future optimization is to add some kind of dynamic allocation
of a limited set of MMU tables for different ASIDs and VMIDs. For emulated
virtualization, it might help quite a bit.


> For ARMv7 and ARMv8 with EL3 in Aarch32 S-PL1 
> is mapped to PL3, so we only need one additional MMU for S-PL0. If you agree I could add this change in 
> the Security Extension patches after this patch makes it into the tree.

Yes, I avoided this patch in my v1 because I thought we would need a non
linear mapping for aarch32 S/NS anyway. But I agree that a combination is
good. Keeping a 1:1 mapping between EL -> MMU idx and have additional MMU
tables for specific features like S/NS.

Thanks,
Edgar

> 
> Best,
> Fabian
> 
> On 19 May 2014, at 11:23, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> 
> > From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
> > 
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> > ---
> > target-arm/cpu.h       | 26 ++++----------------------
> > target-arm/translate.h |  2 +-
> > 2 files changed, 5 insertions(+), 23 deletions(-)
> > 
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 566f9ed..3b7ef32 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -1084,32 +1084,14 @@ static inline CPUARMState *cpu_init(const char *cpu_model)
> > #define cpu_list arm_cpu_list
> > 
> > /* MMU modes definitions */
> > -#define MMU_MODE0_SUFFIX _kernel
> > -#define MMU_MODE1_SUFFIX _user
> > -#define MMU_USER_IDX 1
> > -
> > -static inline int arm_el_to_mmu_idx(int current_el)
> > -{
> > -#ifdef CONFIG_USER_ONLY
> > -    return MMU_USER_IDX;
> > -#else
> > -    switch (current_el) {
> > -    case 0:
> > -        return MMU_USER_IDX;
> > -    case 1:
> > -        return 0;
> > -    default:
> > -        /* Unsupported EL.  */
> > -        assert(0);
> > -        return 0;
> > -    }
> > -#endif
> > -}
> > +#define MMU_MODE0_SUFFIX _user
> > +#define MMU_MODE1_SUFFIX _kernel
> > +#define MMU_USER_IDX 0
> > 
> > static inline int cpu_mmu_index (CPUARMState *env)
> > {
> >     int cur_el = arm_current_pl(env);
> > -    return arm_el_to_mmu_idx(cur_el);
> > +    return cur_el;
> > }
> > 
> > #include "exec/cpu-all.h"
> > diff --git a/target-arm/translate.h b/target-arm/translate.h
> > index db6f0af..31a0104 100644
> > --- a/target-arm/translate.h
> > +++ b/target-arm/translate.h
> > @@ -54,7 +54,7 @@ static inline int arm_dc_feature(DisasContext *dc, int feature)
> > 
> > static inline int get_mem_index(DisasContext *s)
> > {
> > -    return arm_el_to_mmu_idx(s->current_pl);
> > +    return s->current_pl;
> > }
> > 
> > /* target-specific extra values for is_jmp */
> > -- 
> > 1.8.3.2
> > 
> 

Re: [Qemu-devel] [PATCH v3 22/22] RFC: target-arm: Use a 1:1 mapping between EL and MMU index

[Peter Maydell]

On 20 May 2014 14:28, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Tue, May 20, 2014 at 09:47:47AM +0000, Aggeler  Fabian wrote:
>> I guess this makes sense. Shouldn’t we implement two more MMUs to separate S-EL0/EL0 and S-EL1/EL1
>> at least for ARMv8 with EL3 running in Aarch64 state?
>
> Maybe with future patches. My understanding is that on aarch64 the world
> switch between S/NS requires EL3 firmware to reprogram the TTBR regs.
> Currently in QEMU, the re-programming of TTBR will flush the TLBs. We would
> need to do something about that before adding MMU tables for aarch64 Secure
> EL0/1 does any good. I think it's better to keep it simple for now and leave
> this as a possible future optimization.
>
> Another possible future optimization is to add some kind of dynamic allocation
> of a limited set of MMU tables for different ASIDs and VMIDs. For emulated
> virtualization, it might help quite a bit.

I think the right way to do that is to have QEMU's TLB
structure include some sort of general equivalent to the
ASID/VMID mechanism (presumably other target CPUs have some
equivalent). Then we can honour 'flush by ASID' as well.
(We make a forlorn gesture in this direction with the
completely ignored 'flush_global' parameter to tlb_flush().)

This is all definitely 'maybe future' stuff though.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 22/22] RFC: target-arm: Use a 1:1 mapping between EL and MMU index

[Alexander Graf]

On 20.05.14 15:47, Peter Maydell wrote:
> On 20 May 2014 14:28, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
>> On Tue, May 20, 2014 at 09:47:47AM +0000, Aggeler  Fabian wrote:
>>> I guess this makes sense. Shouldn’t we implement two more MMUs to separate S-EL0/EL0 and S-EL1/EL1
>>> at least for ARMv8 with EL3 running in Aarch64 state?
>> Maybe with future patches. My understanding is that on aarch64 the world
>> switch between S/NS requires EL3 firmware to reprogram the TTBR regs.
>> Currently in QEMU, the re-programming of TTBR will flush the TLBs. We would
>> need to do something about that before adding MMU tables for aarch64 Secure
>> EL0/1 does any good. I think it's better to keep it simple for now and leave
>> this as a possible future optimization.
>>
>> Another possible future optimization is to add some kind of dynamic allocation
>> of a limited set of MMU tables for different ASIDs and VMIDs. For emulated
>> virtualization, it might help quite a bit.
> I think the right way to do that is to have QEMU's TLB
> structure include some sort of general equivalent to the
> ASID/VMID mechanism (presumably other target CPUs have some
> equivalent). Then we can honour 'flush by ASID' as well.
> (We make a forlorn gesture in this direction with the
> completely ignored 'flush_global' parameter to tlb_flush().)

I tried to implement this a few years ago, but utterly failed. If 
anyone's interested I can probably dig out the patches I had :).


Alex

Re: [Qemu-devel] [PATCH v3 22/22] RFC: target-arm: Use a 1:1 mapping between EL and MMU index

[Edgar E. Iglesias]

On Tue, May 20, 2014 at 02:47:49PM +0100, Peter Maydell wrote:
> On 20 May 2014 14:28, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > On Tue, May 20, 2014 at 09:47:47AM +0000, Aggeler  Fabian wrote:
> >> I guess this makes sense. Shouldn’t we implement two more MMUs to separate S-EL0/EL0 and S-EL1/EL1
> >> at least for ARMv8 with EL3 running in Aarch64 state?
> >
> > Maybe with future patches. My understanding is that on aarch64 the world
> > switch between S/NS requires EL3 firmware to reprogram the TTBR regs.
> > Currently in QEMU, the re-programming of TTBR will flush the TLBs. We would
> > need to do something about that before adding MMU tables for aarch64 Secure
> > EL0/1 does any good. I think it's better to keep it simple for now and leave
> > this as a possible future optimization.
> >
> > Another possible future optimization is to add some kind of dynamic allocation
> > of a limited set of MMU tables for different ASIDs and VMIDs. For emulated
> > virtualization, it might help quite a bit.
> 
> I think the right way to do that is to have QEMU's TLB
> structure include some sort of general equivalent to the
> ASID/VMID mechanism (presumably other target CPUs have some
> equivalent). Then we can honour 'flush by ASID' as well.
> (We make a forlorn gesture in this direction with the
> completely ignored 'flush_global' parameter to tlb_flush().)

Makes sense.

Cheers,
Edgar

> 
> This is all definitely 'maybe future' stuff though.)
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v3 00/22] target-arm: Preparations for A64 EL2 and 3

[Peter Crosthwaite]

On Mon, May 19, 2014 at 7:22 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Hi,
>
> I've been doing some work on modeling parts of EL2 and 3 + some of
> the system-wide virtualization features for ARMv8. A lot is missing
> but I've got a series with enough to for example run KVM A64 guests
> on top of EL3 firmware inside emulated QEMU A64 VMs.
> I'm working on cleaning things up and plan to send patches and publish
> things as I go.
>
> This series does a first round of preparations. Most of it has little
> or no user visible impact as the EL2 and 3 features remain disabled.
>
> For the arrayification of the various EL regs that do not exist for
> EL0, I've now switched to using padded arrays with the index beeing
> the EL nr.
>
> Regarding the discussion on 1:1 mapping between EL and MMU-idx:
> http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03563.html
> I've appended two RFC patches and the end of the series showing
> what a switch to 1:1 mapping between EL and MMU-idx could look like.
> If the 1:1 mapping is prefered, I'll fold the patches for the
> next spin.
>
> Happy to address any comments people may have.
>
> Cheers,
> Edgar
>
> v2 -> v3:
> * Rephrase comment in update_spsel
> * Switch arrayification pattern to padded with el as index.
> * Rename arm64_spsr_banked_index to aarch64_spsr_banked_index.
> * Avoid using the result from aarch64_spsr_banked_index in a32 mode.
> * aarch64 - > AArch64 in comments.
> * Make VBAR_EL2 RES0 if EL2 is not enabled but higher ELs are.
> * Drop friendly DAIF logging
> * Squash Add storage for VBAR_EL2 and 3 into reg of VBAR
>
> v1 -> v2:
> * Name hyp and mon in cpu_mode_names[]
> * Friendly log of full DAIF, delimited by space
> * Use raw_write in vbar_write
> * Blank line after MMU IDX
> * Use 1:1 EL array mapping in arm64_banked_spsr_index
> * Fix typo in commit message adding arm64_banked_spsr_index.
> * Register EL2 and EL3 regs outside if ARM_FEATURE_V8
> * Avoid split declaration in exception_return
> * Merge comments in exception_return
> * Use VBAR_EL_IDX when registering VBAR_EL2 and 3.
>
> Edgar E. Iglesias (22):
>   target-arm: Make elr_el1 an array
>   target-arm: Make esr_el1 an array
>   target-arm: c12_vbar -> vbar_el[]
>   target-arm: Add arm_el_to_mmu_idx()
>   target-arm: Move get_mem_index to translate.h
>   target-arm: A64: Add SP entries for EL2 and 3
>   target-arm: A64: Add ELR entries for EL2 and 3
>   target-arm: Add SPSR entries for EL2/HYP and EL3/MON
>   target-arm: A64: Introduce aarch64_banked_spsr_index()
>   target-arm: Add a feature flag for EL2
>   target-arm: Add a feature flag for EL3
>   target-arm: Register EL2 versions of ELR and SPSR
>   target-arm: Register EL3 versions of ELR and SPSR
>   target-arm: A64: Forbid ERET to increase the EL
>   target-arm: A64: Forbid ERET to unimplemented ELs
>   target-arm: A64: Generalize ERET to various ELs
>   target-arm: A64: Generalize update_spsel for the various ELs
>   target-arm: Make vbar_write writeback to any CPREG
>   target-arm: A64: Register VBAR_EL2
>   target-arm: A64: Register VBAR_EL3

Looks good to me. For 1-20:

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

>   RFC: target-arm: A32: Use get_mem_index for load/stores
>   RFC: target-arm: Use a 1:1 mapping between EL and MMU index
>
>  target-arm/cpu.h           |  24 +++--
>  target-arm/helper-a64.c    |  12 +--
>  target-arm/helper.c        |  80 ++++++++++++++--
>  target-arm/internals.h     |  25 ++++-
>  target-arm/kvm64.c         |   4 +-
>  target-arm/machine.c       |  10 +-
>  target-arm/op_helper.c     |  19 ++--
>  target-arm/translate-a64.c |   9 --
>  target-arm/translate.c     | 224 +++++++++++++++++++++++----------------------
>  target-arm/translate.h     |   5 +
>  10 files changed, 251 insertions(+), 161 deletions(-)
>

> --
> 1.8.3.2
>
>

Re: [Qemu-devel] [PATCH v3 00/22] target-arm: Preparations for A64 EL2 and 3

[Peter Maydell]

On 19 May 2014 10:22, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Hi,
>
> I've been doing some work on modeling parts of EL2 and 3 + some of
> the system-wide virtualization features for ARMv8. A lot is missing
> but I've got a series with enough to for example run KVM A64 guests
> on top of EL3 firmware inside emulated QEMU A64 VMs.
> I'm working on cleaning things up and plan to send patches and publish
> things as I go.

Can you comment on the on-list set of patches from Fabian, please?
In particular we need to get the handling of banked registers correct
to support both AArch32 EL2/EL3 and AArch64 EL2/EL3, so if you've
already worked through the AArch64 issues that will save me having
to think about it all from scratch...

I've made some comments on a few patches but otherwise I think
this looks good and we should try to get it committed reasonably
soon so that Fabian can rebase his patchset on top of this...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 00/22] target-arm: Preparations for A64 EL2 and 3

[Edgar E. Iglesias]

On Wed, May 21, 2014 at 08:30:30PM +0100, Peter Maydell wrote:
> On 19 May 2014 10:22, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Hi,
> >
> > I've been doing some work on modeling parts of EL2 and 3 + some of
> > the system-wide virtualization features for ARMv8. A lot is missing
> > but I've got a series with enough to for example run KVM A64 guests
> > on top of EL3 firmware inside emulated QEMU A64 VMs.
> > I'm working on cleaning things up and plan to send patches and publish
> > things as I go.
> 
> Can you comment on the on-list set of patches from Fabian, please?
> In particular we need to get the handling of banked registers correct
> to support both AArch32 EL2/EL3 and AArch64 EL2/EL3, so if you've
> already worked through the AArch64 issues that will save me having
> to think about it all from scratch...
> 
> I've made some comments on a few patches but otherwise I think
> this looks good and we should try to get it committed reasonably
> soon so that Fabian can rebase his patchset on top of this...
>

Thanks for excellent reviewing. I've looked at the TZ series a bit,
but will take a closer look and try to give some feedback.

Cheers,
Edgar