[Qemu-devel] [PATCH v3 00/16] target-arm: Parts of the AArch64 EL2/3 exception model

[Qemu-devel] [PATCH v3 00/16] target-arm: Parts of the AArch64 EL2/3 exception model

[Edgar E. Iglesias]

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

Hi,

This is a second round of AArch64 EL2/3 patches working on the exception
model. Among other things adding HVC/SMC, interrupt routing to EL2/3 and
Virtual IRQs/FIQs. The VIRQ/VFIQ support only adds the external signal
delivery method.

Patch 3 is a bug fix.
Patch 14 fails checkpatch, seems like a bug in checkpatch, CC:d Blue.

This conflicts slightly with the PSCI emulation patches that Rob posted.
A rebase should be trivial, hooking in the PSCI emulation calls in the
HVC/SMC helpers.

Cheers,
Edgar

v2 -> v3:
* Add more HCR bitfield macros
* Flush TLB on hcr_write change of HCR RW, DC and PTW.
* Fix hvc helper, HVC is undefined in secure mode.
* Remove uint16_t imm16 syndrome gen change.
* Replace c1_scr with scr_el3

v1 -> v2:
* Avoid imm16 mask in syndrome generation
* Use g_assert_not_reached() in arm_excp_unmasked()
* Avoid some logic duplication in arm_excp_target_el and arm_excp_unmasked.
* Put arm_excp_target_el in helper.c to start with.
* Fix SMC disable (SMD or SCD) for ARMv7 only applies if EL2 exists
* SCR_RES0_MASK -> SCR_MASK
* HCR_RES0_MASK -> HCR_MASK
* Fix SMC routing to EL2, only applies for NS EL1.
* Fix CPreg defs for ESR_EL2/3
* Fix SMC helper, SMC routing to EL2 and SCR.SMD for AArch32.

Edgar E. Iglesias (16):
  target-arm: A64: Break out aarch64_save/restore_sp
  target-arm: A64: Respect SPSEL in ERET SP restore
  target-arm: A64: Respect SPSEL when taking exceptions
  target-arm: Make far_el1 an array
  target-arm: Add ESR_EL2 and 3
  target-arm: Add FAR_EL2 and 3
  target-arm: Add HCR_EL2
  target-arm: Add SCR_EL3
  target-arm: A64: Refactor aarch64_cpu_do_interrupt
  target-arm: Break out exception masking to a separate func
  target-arm: Don't take interrupts targeting lower ELs
  target-arm: A64: Correct updates to FAR and ESR on exceptions
  target-arm: A64: Emulate the HVC insn
  target-arm: A64: Emulate the SMC insn
  target-arm: Add IRQ and FIQ routing to EL2 and 3
  target-arm: Add support for VIRQ and VFIQ

 cpu-exec.c                 |  17 +++++-
 target-arm/cpu.c           |  22 ++++++-
 target-arm/cpu.h           | 120 ++++++++++++++++++++++++++++++++++++-
 target-arm/helper-a64.c    |  32 +++++-----
 target-arm/helper.c        | 145 ++++++++++++++++++++++++++++++++++++++++++---
 target-arm/helper.h        |   2 +
 target-arm/internals.h     |  43 +++++++++++---
 target-arm/kvm64.c         |  13 +---
 target-arm/op_helper.c     |  68 +++++++++++++++++++--
 target-arm/translate-a64.c |  31 ++++++++--
 10 files changed, 433 insertions(+), 60 deletions(-)

-- 
1.8.3.2

[Qemu-devel] [PATCH v3 01/16] target-arm: A64: Break out aarch64_save/restore_sp

[Edgar E. Iglesias]

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

Break out code to save/restore AArch64 SP into functions.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/internals.h | 29 ++++++++++++++++++++---------
 target-arm/kvm64.c     | 13 +++----------
 target-arm/op_helper.c |  6 +-----
 3 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/target-arm/internals.h b/target-arm/internals.h
index 564b5fa..08fa697 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -105,6 +105,24 @@ enum arm_fprounding {
 
 int arm_rmode_to_sf(int rmode);
 
+static inline void aarch64_save_sp(CPUARMState *env, int el)
+{
+    if (env->pstate & PSTATE_SP) {
+        env->sp_el[el] = env->xregs[31];
+    } else {
+        env->sp_el[0] = env->xregs[31];
+    }
+}
+
+static inline void aarch64_restore_sp(CPUARMState *env, int el)
+{
+    if (env->pstate & PSTATE_SP) {
+        env->xregs[31] = env->sp_el[el];
+    } else {
+        env->xregs[31] = env->sp_el[0];
+    }
+}
+
 static inline void update_spsel(CPUARMState *env, uint32_t imm)
 {
     unsigned int cur_el = arm_current_pl(env);
@@ -114,21 +132,14 @@ static inline void update_spsel(CPUARMState *env, uint32_t imm)
     if (!((imm ^ env->pstate) & PSTATE_SP)) {
         return;
     }
+    aarch64_save_sp(env, cur_el);
     env->pstate = deposit32(env->pstate, 0, 1, imm);
 
     /* We rely on illegal updates to SPsel from EL0 to get trapped
      * at translation time.
      */
     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[cur_el];
-    } else {
-        /* Switch from SP_EL0 to SP_ELx */
-        env->sp_el[cur_el] = env->xregs[31];
-        env->xregs[31] = env->sp_el[0];
-    }
+    aarch64_restore_sp(env, cur_el);
 }
 
 /* Valid Syndrome Register EC field values */
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index 70f311b..0542cd1 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -21,6 +21,7 @@
 #include "sysemu/kvm.h"
 #include "kvm_arm.h"
 #include "cpu.h"
+#include "internals.h"
 #include "hw/arm/arm.h"
 
 static inline void set_feature(uint64_t *features, int feature)
@@ -124,11 +125,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
     /* KVM puts SP_EL0 in regs.sp and SP_EL1 in regs.sp_el1. On the
      * QEMU side we keep the current SP in xregs[31] as well.
      */
-    if (env->pstate & PSTATE_SP) {
-        env->sp_el[1] = env->xregs[31];
-    } else {
-        env->sp_el[0] = env->xregs[31];
-    }
+    aarch64_save_sp(env, 1);
 
     reg.id = AARCH64_CORE_REG(regs.sp);
     reg.addr = (uintptr_t) &env->sp_el[0];
@@ -227,11 +224,7 @@ int kvm_arch_get_registers(CPUState *cs)
     /* KVM puts SP_EL0 in regs.sp and SP_EL1 in regs.sp_el1. On the
      * QEMU side we keep the current SP in xregs[31] as well.
      */
-    if (env->pstate & PSTATE_SP) {
-        env->xregs[31] = env->sp_el[1];
-    } else {
-        env->xregs[31] = env->sp_el[0];
-    }
+    aarch64_restore_sp(env, 1);
 
     reg.id = AARCH64_CORE_REG(regs.pc);
     reg.addr = (uintptr_t) &env->pc;
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 9c1ef52..90a946a 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -376,11 +376,7 @@ void HELPER(exception_return)(CPUARMState *env)
     uint32_t spsr = env->banked_spsr[spsr_idx];
     int new_el, i;
 
-    if (env->pstate & PSTATE_SP) {
-        env->sp_el[cur_el] = env->xregs[31];
-    } else {
-        env->sp_el[0] = env->xregs[31];
-    }
+    aarch64_save_sp(env, cur_el);
 
     env->exclusive_addr = -1;
 
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 02/16] target-arm: A64: Respect SPSEL in ERET SP restore

[Edgar E. Iglesias]

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

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/op_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 90a946a..25ad902 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -410,7 +410,7 @@ void HELPER(exception_return)(CPUARMState *env)
         }
         env->aarch64 = 1;
         pstate_write(env, spsr);
-        env->xregs[31] = env->sp_el[new_el];
+        aarch64_restore_sp(env, new_el);
         env->pc = env->elr_el[cur_el];
     }
 
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 03/16] target-arm: A64: Respect SPSEL when taking exceptions

[Edgar E. Iglesias]

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

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper-a64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 2b4ce6a..027434a 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -489,8 +489,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
 
     if (is_a64(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];
+        aarch64_save_sp(env, arm_current_pl(env));
         env->elr_el[1] = env->pc;
     } else {
         env->banked_spsr[0] = cpsr_read(env);
@@ -508,6 +507,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
 
     pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
     env->aarch64 = 1;
+    aarch64_restore_sp(env, 1);
 
     env->pc = addr;
     cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 04/16] target-arm: Make far_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.

Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.c        |  2 +-
 target-arm/cpu.h        |  2 +-
 target-arm/helper-a64.c |  4 ++--
 target-arm/helper.c     | 12 ++++++------
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index b877835..8e64c5a 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -446,7 +446,7 @@ static void arm1026_initfn(Object *obj)
         ARMCPRegInfo ifar = {
             .name = "IFAR", .cp = 15, .crn = 6, .crm = 0, .opc1 = 0, .opc2 = 1,
             .access = PL1_RW,
-            .fieldoffset = offsetofhigh32(CPUARMState, cp15.far_el1),
+            .fieldoffset = offsetofhigh32(CPUARMState, cp15.far_el[1]),
             .resetvalue = 0
         };
         define_one_arm_cp_reg(cpu, &ifar);
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 79e7f82..6417507 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -187,7 +187,7 @@ typedef struct CPUARMState {
         uint32_t ifsr_el2; /* Fault status registers.  */
         uint64_t esr_el[2];
         uint32_t c6_region[8]; /* MPU base/size registers.  */
-        uint64_t far_el1; /* Fault address registers.  */
+        uint64_t far_el[2]; /* Fault address registers.  */
         uint64_t par_el1;  /* Translation result. */
         uint32_t c9_insn; /* Cache lockdown registers.  */
         uint32_t c9_data;
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 027434a..2e9ef64 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -465,13 +465,13 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
     }
 
     env->cp15.esr_el[1] = env->exception.syndrome;
-    env->cp15.far_el1 = env->exception.vaddress;
+    env->cp15.far_el[1] = env->exception.vaddress;
 
     switch (cs->exception_index) {
     case EXCP_PREFETCH_ABORT:
     case EXCP_DATA_ABORT:
         qemu_log_mask(CPU_LOG_INT, "...with FAR 0x%" PRIx64 "\n",
-                      env->cp15.far_el1);
+                      env->cp15.far_el[1]);
         break;
     case EXCP_BKPT:
     case EXCP_UDEF:
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 050c409..0b1f2c9 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -521,7 +521,7 @@ static const ARMCPRegInfo v6_cp_reginfo[] = {
       .access = PL0_W, .type = ARM_CP_NOP },
     { .name = "IFAR", .cp = 15, .crn = 6, .crm = 0, .opc1 = 0, .opc2 = 2,
       .access = PL1_RW,
-      .fieldoffset = offsetofhigh32(CPUARMState, cp15.far_el1),
+      .fieldoffset = offsetofhigh32(CPUARMState, cp15.far_el[1]),
       .resetvalue = 0, },
     /* Watchpoint Fault Address Register : should actually only be present
      * for 1136, 1176, 11MPCore.
@@ -1505,7 +1505,7 @@ static const ARMCPRegInfo vmsa_cp_reginfo[] = {
     /* 64-bit FAR; this entry also gives us the AArch32 DFAR */
     { .name = "FAR_EL1", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .crn = 6, .crm = 0, .opc1 = 0, .opc2 = 0,
-      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.far_el1),
+      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.far_el[1]),
       .resetvalue = 0, },
     REGINFO_SENTINEL
 };
@@ -3414,8 +3414,8 @@ void arm_cpu_do_interrupt(CPUState *cs)
         /* Fall through to prefetch abort.  */
     case EXCP_PREFETCH_ABORT:
         env->cp15.ifsr_el2 = env->exception.fsr;
-        env->cp15.far_el1 = deposit64(env->cp15.far_el1, 32, 32,
-                                      env->exception.vaddress);
+        env->cp15.far_el[1] = deposit64(env->cp15.far_el[1], 32, 32,
+                                        env->exception.vaddress);
         qemu_log_mask(CPU_LOG_INT, "...with IFSR 0x%x IFAR 0x%x\n",
                       env->cp15.ifsr_el2, (uint32_t)env->exception.vaddress);
         new_mode = ARM_CPU_MODE_ABT;
@@ -3425,8 +3425,8 @@ void arm_cpu_do_interrupt(CPUState *cs)
         break;
     case EXCP_DATA_ABORT:
         env->cp15.esr_el[1] = env->exception.fsr;
-        env->cp15.far_el1 = deposit64(env->cp15.far_el1, 0, 32,
-                                      env->exception.vaddress);
+        env->cp15.far_el[1] = deposit64(env->cp15.far_el[1], 0, 32,
+                                        env->exception.vaddress);
         qemu_log_mask(CPU_LOG_INT, "...with DFSR 0x%x DFAR 0x%x\n",
                       (uint32_t)env->cp15.esr_el[1],
                       (uint32_t)env->exception.vaddress);
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 05/16] target-arm: Add ESR_EL2 and 3

[Edgar E. Iglesias]

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

Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h    | 2 +-
 target-arm/helper.c | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 6417507..d1d8c85 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_el[2];
+        uint64_t esr_el[4];
         uint32_t c6_region[8]; /* MPU base/size registers.  */
         uint64_t far_el[2]; /* Fault address registers.  */
         uint64_t par_el1;  /* Translation result. */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 0b1f2c9..94a3b41 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2116,6 +2116,10 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
       .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 1,
       .access = PL2_RW,
       .fieldoffset = offsetof(CPUARMState, elr_el[2]) },
+    { .name = "ESR_EL2", .state = ARM_CP_STATE_AA64,
+      .type = ARM_CP_NO_MIGRATE,
+      .opc0 = 3, .opc1 = 4, .crn = 5, .crm = 2, .opc2 = 0,
+      .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.esr_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,
@@ -2134,6 +2138,10 @@ static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
       .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 0, .opc2 = 1,
       .access = PL3_RW,
       .fieldoffset = offsetof(CPUARMState, elr_el[3]) },
+    { .name = "ESR_EL3", .state = ARM_CP_STATE_AA64,
+      .type = ARM_CP_NO_MIGRATE,
+      .opc0 = 3, .opc1 = 6, .crn = 5, .crm = 2, .opc2 = 0,
+      .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.esr_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,
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 06/16] target-arm: Add FAR_EL2 and 3

[Edgar E. Iglesias]

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

Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h    | 2 +-
 target-arm/helper.c | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index d1d8c85..1dbed92 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -187,7 +187,7 @@ typedef struct CPUARMState {
         uint32_t ifsr_el2; /* Fault status registers.  */
         uint64_t esr_el[4];
         uint32_t c6_region[8]; /* MPU base/size registers.  */
-        uint64_t far_el[2]; /* Fault address registers.  */
+        uint64_t far_el[4]; /* Fault address registers.  */
         uint64_t par_el1;  /* Translation result. */
         uint32_t c9_insn; /* Cache lockdown registers.  */
         uint32_t c9_data;
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 94a3b41..7170086 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2120,6 +2120,9 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
       .type = ARM_CP_NO_MIGRATE,
       .opc0 = 3, .opc1 = 4, .crn = 5, .crm = 2, .opc2 = 0,
       .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.esr_el[2]) },
+    { .name = "FAR_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 0,
+      .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.far_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,
@@ -2142,6 +2145,9 @@ static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
       .type = ARM_CP_NO_MIGRATE,
       .opc0 = 3, .opc1 = 6, .crn = 5, .crm = 2, .opc2 = 0,
       .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.esr_el[3]) },
+    { .name = "FAR_EL3", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 6, .crn = 6, .crm = 0, .opc2 = 0,
+      .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.far_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,
-- 
1.8.3.2

[Qemu-devel] [PATCH v3 07/16] target-arm: Add HCR_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    | 36 ++++++++++++++++++++++++++++++++++++
 target-arm/helper.c | 26 ++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 1dbed92..fd57fb5 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -184,6 +184,7 @@ typedef struct CPUARMState {
                         MPU write buffer control.  */
         uint32_t pmsav5_data_ap; /* PMSAv5 MPU data access permissions */
         uint32_t pmsav5_insn_ap; /* PMSAv5 MPU insn access permissions */
+        uint64_t hcr_el2; /* Hypervisor configuration register */
         uint32_t ifsr_el2; /* Fault status registers.  */
         uint64_t esr_el[4];
         uint32_t c6_region[8]; /* MPU base/size registers.  */
@@ -526,6 +527,41 @@ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
     }
 }
 
+#define HCR_VM        (1ULL << 0)
+#define HCR_SWIO      (1ULL << 1)
+#define HCR_PTW       (1ULL << 2)
+#define HCR_FMO       (1ULL << 3)
+#define HCR_IMO       (1ULL << 4)
+#define HCR_AMO       (1ULL << 5)
+#define HCR_VF        (1ULL << 6)
+#define HCR_VI        (1ULL << 7)
+#define HCR_VSE       (1ULL << 8)
+#define HCR_FB        (1ULL << 9)
+#define HCR_BSU_MASK  (3ULL << 10)
+#define HCR_DC        (1ULL << 12)
+#define HCR_TWI       (1ULL << 13)
+#define HCR_TWE       (1ULL << 14)
+#define HCR_TID0      (1ULL << 15)
+#define HCR_TID1      (1ULL << 16)
+#define HCR_TID2      (1ULL << 17)
+#define HCR_TID3      (1ULL << 18)
+#define HCR_TSC       (1ULL << 19)
+#define HCR_TIDCP     (1ULL << 20)
+#define HCR_TACR      (1ULL << 21)
+#define HCR_TSW       (1ULL << 22)
+#define HCR_TPC       (1ULL << 23)
+#define HCR_TPU       (1ULL << 24)
+#define HCR_TTLB      (1ULL << 25)
+#define HCR_TVM       (1ULL << 26)
+#define HCR_TGE       (1ULL << 27)
+#define HCR_TDZ       (1ULL << 28)
+#define HCR_HCD       (1ULL << 29)
+#define HCR_TRVM      (1ULL << 30)
+#define HCR_RW        (1ULL << 31)
+#define HCR_CD        (1ULL << 32)
+#define HCR_ID        (1ULL << 33)
+#define HCR_MASK      ((1ULL << 34) - 1)
+
 /* Return the current FPSCR value.  */
 uint32_t vfp_get_fpscr(CPUARMState *env);
 void vfp_set_fpscr(CPUARMState *env, uint32_t val);
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 7170086..b04fb5d 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2107,10 +2107,36 @@ static const ARMCPRegInfo v8_el3_no_el2_cp_reginfo[] = {
       .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
       .access = PL2_RW,
       .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
+    { .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,
+      .type = ARM_CP_NO_MIGRATE,
+      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
+      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
     REGINFO_SENTINEL
 };
 
+static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
+{
+    ARMCPU *cpu = arm_env_get_cpu(env);
+    uint64_t valid_mask = HCR_MASK;
+
+    if (!arm_feature(env, ARM_FEATURE_EL3)) {
+        valid_mask &= ~HCR_HCD;
+    }
+
+    /* Clear RES0 bits.  */
+    value &= valid_mask;
+
+    if ((raw_read(env, ri) ^ value) & (HCR_VM | HCR_RW | HCR_PTW | HCR_DC)) {
+        tlb_flush(CPU(cpu), 1);
+    }
+    raw_write(env, ri, value);
+}
+
 static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
+    { .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
+      .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.hcr_el2),
+      .writefn = hcr_write },
     { .name = "ELR_EL2", .state = ARM_CP_STATE_AA64,
       .type = ARM_CP_NO_MIGRATE,
       .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 1,
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v3 07/16] target-arm: Add HCR_EL2

[Greg Bellows]

[-- Attachment #1: Type: text/plain, Size: 4202 bytes --]

Reviewed-by: Greg Bellows <greg.bellows@linaro.org>


On 17 June 2014 03:45, 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    | 36 ++++++++++++++++++++++++++++++++++++
>  target-arm/helper.c | 26 ++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 1dbed92..fd57fb5 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -184,6 +184,7 @@ typedef struct CPUARMState {
>                          MPU write buffer control.  */
>          uint32_t pmsav5_data_ap; /* PMSAv5 MPU data access permissions */
>          uint32_t pmsav5_insn_ap; /* PMSAv5 MPU insn access permissions */
> +        uint64_t hcr_el2; /* Hypervisor configuration register */
>          uint32_t ifsr_el2; /* Fault status registers.  */
>          uint64_t esr_el[4];
>          uint32_t c6_region[8]; /* MPU base/size registers.  */
> @@ -526,6 +527,41 @@ static inline void xpsr_write(CPUARMState *env,
> uint32_t val, uint32_t mask)
>      }
>  }
>
> +#define HCR_VM        (1ULL << 0)
> +#define HCR_SWIO      (1ULL << 1)
> +#define HCR_PTW       (1ULL << 2)
> +#define HCR_FMO       (1ULL << 3)
> +#define HCR_IMO       (1ULL << 4)
> +#define HCR_AMO       (1ULL << 5)
> +#define HCR_VF        (1ULL << 6)
> +#define HCR_VI        (1ULL << 7)
> +#define HCR_VSE       (1ULL << 8)
> +#define HCR_FB        (1ULL << 9)
> +#define HCR_BSU_MASK  (3ULL << 10)
> +#define HCR_DC        (1ULL << 12)
> +#define HCR_TWI       (1ULL << 13)
> +#define HCR_TWE       (1ULL << 14)
> +#define HCR_TID0      (1ULL << 15)
> +#define HCR_TID1      (1ULL << 16)
> +#define HCR_TID2      (1ULL << 17)
> +#define HCR_TID3      (1ULL << 18)
> +#define HCR_TSC       (1ULL << 19)
> +#define HCR_TIDCP     (1ULL << 20)
> +#define HCR_TACR      (1ULL << 21)
> +#define HCR_TSW       (1ULL << 22)
> +#define HCR_TPC       (1ULL << 23)
> +#define HCR_TPU       (1ULL << 24)
> +#define HCR_TTLB      (1ULL << 25)
> +#define HCR_TVM       (1ULL << 26)
> +#define HCR_TGE       (1ULL << 27)
> +#define HCR_TDZ       (1ULL << 28)
> +#define HCR_HCD       (1ULL << 29)
> +#define HCR_TRVM      (1ULL << 30)
> +#define HCR_RW        (1ULL << 31)
> +#define HCR_CD        (1ULL << 32)
> +#define HCR_ID        (1ULL << 33)
> +#define HCR_MASK      ((1ULL << 34) - 1)
> +
>  /* Return the current FPSCR value.  */
>  uint32_t vfp_get_fpscr(CPUARMState *env);
>  void vfp_set_fpscr(CPUARMState *env, uint32_t val);
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 7170086..b04fb5d 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2107,10 +2107,36 @@ static const ARMCPRegInfo
> v8_el3_no_el2_cp_reginfo[] = {
>        .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
>        .access = PL2_RW,
>        .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> +    { .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,
> +      .type = ARM_CP_NO_MIGRATE,
> +      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
> +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
>      REGINFO_SENTINEL
>  };
>
> +static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t
> value)
> +{
> +    ARMCPU *cpu = arm_env_get_cpu(env);
> +    uint64_t valid_mask = HCR_MASK;
> +
> +    if (!arm_feature(env, ARM_FEATURE_EL3)) {
> +        valid_mask &= ~HCR_HCD;
> +    }
> +
> +    /* Clear RES0 bits.  */
> +    value &= valid_mask;
> +
> +    if ((raw_read(env, ri) ^ value) & (HCR_VM | HCR_RW | HCR_PTW |
> HCR_DC)) {
> +        tlb_flush(CPU(cpu), 1);
> +    }
> +    raw_write(env, ri, value);
> +}
> +
>  static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
> +    { .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
> +      .access = PL2_RW, .fieldoffset = offsetof(CPUARMState,
> cp15.hcr_el2),
> +      .writefn = hcr_write },
>      { .name = "ELR_EL2", .state = ARM_CP_STATE_AA64,
>        .type = ARM_CP_NO_MIGRATE,
>        .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 1,
> --
> 1.8.3.2
>
>

[-- Attachment #2: Type: text/html, Size: 5531 bytes --]

Re: [Qemu-devel] [PATCH v3 07/16] target-arm: Add HCR_EL2

[Peter Maydell]

On 17 June 2014 09:45, 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>

> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 7170086..b04fb5d 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2107,10 +2107,36 @@ static const ARMCPRegInfo v8_el3_no_el2_cp_reginfo[] = {
>        .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
>        .access = PL2_RW,
>        .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> +    { .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,
> +      .type = ARM_CP_NO_MIGRATE,
> +      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
> +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },

Isn't this missing the .access specifier ?

>      REGINFO_SENTINEL
>  };
>
> +static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
> +{
> +    ARMCPU *cpu = arm_env_get_cpu(env);
> +    uint64_t valid_mask = HCR_MASK;
> +
> +    if (!arm_feature(env, ARM_FEATURE_EL3)) {
> +        valid_mask &= ~HCR_HCD;
> +    }

This is inconsistent. HCD isn't the only bit that's "RES0 if
EL3 unimplemented"; TSC is as well, for instance.
(In fact the RES0 definition means you don't actually have
to mask this out unless it's more convenient to do so.)

> +
> +    /* Clear RES0 bits.  */
> +    value &= valid_mask;
> +
> +    if ((raw_read(env, ri) ^ value) & (HCR_VM | HCR_RW | HCR_PTW | HCR_DC)) {
> +        tlb_flush(CPU(cpu), 1);

Could maybe use a comment about why we need a TLB flush.

> +    }
> +    raw_write(env, ri, value);
> +}
> +
>  static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
> +    { .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
> +      .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.hcr_el2),
> +      .writefn = hcr_write },
>      { .name = "ELR_EL2", .state = ARM_CP_STATE_AA64,
>        .type = ARM_CP_NO_MIGRATE,
>        .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 1,
> --

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 07/16] target-arm: Add HCR_EL2

[Edgar E. Iglesias]

On Fri, Aug 01, 2014 at 02:29:28PM +0100, Peter Maydell wrote:
> On 17 June 2014 09:45, 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>
> 
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 7170086..b04fb5d 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -2107,10 +2107,36 @@ static const ARMCPRegInfo v8_el3_no_el2_cp_reginfo[] = {
> >        .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
> >        .access = PL2_RW,
> >        .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> > +    { .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,
> > +      .type = ARM_CP_NO_MIGRATE,
> > +      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
> > +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> 
> Isn't this missing the .access specifier ?

Good catch, thanks.

> 
> >      REGINFO_SENTINEL
> >  };
> >
> > +static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
> > +{
> > +    ARMCPU *cpu = arm_env_get_cpu(env);
> > +    uint64_t valid_mask = HCR_MASK;
> > +
> > +    if (!arm_feature(env, ARM_FEATURE_EL3)) {
> > +        valid_mask &= ~HCR_HCD;
> > +    }
> 
> This is inconsistent. HCD isn't the only bit that's "RES0 if
> EL3 unimplemented"; TSC is as well, for instance.
> (In fact the RES0 definition means you don't actually have
> to mask this out unless it's more convenient to do so.)

I've added TSC. Couldn't see any others..

> 
> > +
> > +    /* Clear RES0 bits.  */
> > +    value &= valid_mask;
> > +
> > +    if ((raw_read(env, ri) ^ value) & (HCR_VM | HCR_RW | HCR_PTW | HCR_DC)) {
> > +        tlb_flush(CPU(cpu), 1);
> 
> Could maybe use a comment about why we need a TLB flush.

I'll add one. Actually, when thinking more about it, I'm not convinced that
we will need to flush when HCR_RW changes because the 32/64bit state is part
of the tbflags.

Cheers,
Edgar

Re: [Qemu-devel] [PATCH v3 07/16] target-arm: Add HCR_EL2

[Edgar E. Iglesias]

On Mon, Aug 04, 2014 at 01:48:10PM +1000, Edgar E. Iglesias wrote:
> On Fri, Aug 01, 2014 at 02:29:28PM +0100, Peter Maydell wrote:
> > On 17 June 2014 09:45, 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>
> > 
> > > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > > index 7170086..b04fb5d 100644
> > > --- a/target-arm/helper.c
> > > +++ b/target-arm/helper.c
> > > @@ -2107,10 +2107,36 @@ static const ARMCPRegInfo v8_el3_no_el2_cp_reginfo[] = {
> > >        .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
> > >        .access = PL2_RW,
> > >        .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> > > +    { .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,
> > > +      .type = ARM_CP_NO_MIGRATE,
> > > +      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
> > > +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> > 
> > Isn't this missing the .access specifier ?
> 
> Good catch, thanks.
> 
> > 
> > >      REGINFO_SENTINEL
> > >  };
> > >
> > > +static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
> > > +{
> > > +    ARMCPU *cpu = arm_env_get_cpu(env);
> > > +    uint64_t valid_mask = HCR_MASK;
> > > +
> > > +    if (!arm_feature(env, ARM_FEATURE_EL3)) {
> > > +        valid_mask &= ~HCR_HCD;
> > > +    }
> > 
> > This is inconsistent. HCD isn't the only bit that's "RES0 if
> > EL3 unimplemented"; TSC is as well, for instance.
> > (In fact the RES0 definition means you don't actually have
> > to mask this out unless it's more convenient to do so.)
> 
> I've added TSC. Couldn't see any others..

Hmm, I was a bit confused here. HCD is RES0 if EL3 _is_ implemented.
TSC is RES0 if EL3 is not implemented.

I will fix this up but if you prefer I can drop the zeroing aswell.

Cheers,
Edgar

[Qemu-devel] [PATCH v3 08/16] target-arm: Add SCR_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    | 16 +++++++++++++++-
 target-arm/helper.c | 31 ++++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index fd57fb5..fa8dee0 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -172,7 +172,6 @@ typedef struct CPUARMState {
         uint64_t c1_sys; /* System control register.  */
         uint64_t c1_coproc; /* Coprocessor access register.  */
         uint32_t c1_xscaleauxcr; /* XScale auxiliary control register.  */
-        uint32_t c1_scr; /* secure config register.  */
         uint64_t ttbr0_el1; /* MMU translation table base 0. */
         uint64_t ttbr1_el1; /* MMU translation table base 1. */
         uint64_t c2_control; /* MMU translation table base control.  */
@@ -185,6 +184,7 @@ typedef struct CPUARMState {
         uint32_t pmsav5_data_ap; /* PMSAv5 MPU data access permissions */
         uint32_t pmsav5_insn_ap; /* PMSAv5 MPU insn access permissions */
         uint64_t hcr_el2; /* Hypervisor configuration register */
+        uint32_t scr_el3; /* Secure configuration register.  */
         uint32_t ifsr_el2; /* Fault status registers.  */
         uint64_t esr_el[4];
         uint32_t c6_region[8]; /* MPU base/size registers.  */
@@ -562,6 +562,20 @@ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
 #define HCR_ID        (1ULL << 33)
 #define HCR_MASK      ((1ULL << 34) - 1)
 
+#define SCR_NS        (1U << 0)
+#define SCR_IRQ       (1U << 1)
+#define SCR_FIQ       (1U << 2)
+#define SCR_EA        (1U << 3)
+#define SCR_SMD       (1U << 7)
+#define SCR_HCE       (1U << 8)
+#define SCR_SIF       (1U << 9)
+#define SCR_RW        (1U << 10)
+#define SCR_ST        (1U << 11)
+#define SCR_TWI       (1U << 12)
+#define SCR_TWE       (1U << 13)
+#define SCR_RES1_MASK (3U << 4)
+#define SCR_MASK      (0x3fff & ~SCR_RES1_MASK)
+
 /* Return the current FPSCR value.  */
 uint32_t vfp_get_fpscr(CPUARMState *env);
 void vfp_set_fpscr(CPUARMState *env, uint32_t val);
diff --git a/target-arm/helper.c b/target-arm/helper.c
index b04fb5d..6bacc24 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -793,7 +793,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
       .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),
+      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3),
       .resetvalue = 0, },
     { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0,
@@ -2161,6 +2161,31 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
     REGINFO_SENTINEL
 };
 
+static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
+{
+    uint32_t valid_mask = SCR_MASK;
+
+    if (!arm_feature(env, ARM_FEATURE_EL2)) {
+        valid_mask &= ~SCR_HCE;
+
+        /* On ARMv7, SMD (or SCD as it is called in v7) is only
+         * supported if EL2 exists. The bit is UNK/SBZP when
+         * EL2 is unavailable. In QEMU ARMv7, we force it to always zero
+         * when EL2 is unavailable.
+         */
+        if (arm_feature(env, ARM_FEATURE_V7)) {
+            valid_mask &= ~SCR_SMD;
+        }
+    }
+
+    /* Set RES1 bits.  */
+    value |= SCR_RES1_MASK;
+
+    /* Clear RES0 bits.  */
+    value &= valid_mask;
+    raw_write(env, ri, value);
+}
+
 static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
     { .name = "ELR_EL3", .state = ARM_CP_STATE_AA64,
       .type = ARM_CP_NO_MIGRATE,
@@ -2183,6 +2208,10 @@ static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
       .access = PL3_RW, .writefn = vbar_write,
       .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[3]),
       .resetvalue = 0 },
+    { .name = "SCR_EL3", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 0,
+      .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3),
+      .writefn = scr_write },
     REGINFO_SENTINEL
 };
 
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v3 08/16] target-arm: Add SCR_EL3

[Greg Bellows]

[-- Attachment #1: Type: text/plain, Size: 4582 bytes --]

Reviewed-by: Greg Bellows <greg.bellows@linaro.org>


On 17 June 2014 03:45, 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    | 16 +++++++++++++++-
>  target-arm/helper.c | 31 ++++++++++++++++++++++++++++++-
>  2 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index fd57fb5..fa8dee0 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -172,7 +172,6 @@ typedef struct CPUARMState {
>          uint64_t c1_sys; /* System control register.  */
>          uint64_t c1_coproc; /* Coprocessor access register.  */
>          uint32_t c1_xscaleauxcr; /* XScale auxiliary control register.  */
> -        uint32_t c1_scr; /* secure config register.  */
>          uint64_t ttbr0_el1; /* MMU translation table base 0. */
>          uint64_t ttbr1_el1; /* MMU translation table base 1. */
>          uint64_t c2_control; /* MMU translation table base control.  */
> @@ -185,6 +184,7 @@ typedef struct CPUARMState {
>          uint32_t pmsav5_data_ap; /* PMSAv5 MPU data access permissions */
>          uint32_t pmsav5_insn_ap; /* PMSAv5 MPU insn access permissions */
>          uint64_t hcr_el2; /* Hypervisor configuration register */
> +        uint32_t scr_el3; /* Secure configuration register.  */
>          uint32_t ifsr_el2; /* Fault status registers.  */
>          uint64_t esr_el[4];
>          uint32_t c6_region[8]; /* MPU base/size registers.  */
> @@ -562,6 +562,20 @@ static inline void xpsr_write(CPUARMState *env,
> uint32_t val, uint32_t mask)
>  #define HCR_ID        (1ULL << 33)
>  #define HCR_MASK      ((1ULL << 34) - 1)
>
> +#define SCR_NS        (1U << 0)
> +#define SCR_IRQ       (1U << 1)
> +#define SCR_FIQ       (1U << 2)
> +#define SCR_EA        (1U << 3)
> +#define SCR_SMD       (1U << 7)
> +#define SCR_HCE       (1U << 8)
> +#define SCR_SIF       (1U << 9)
> +#define SCR_RW        (1U << 10)
> +#define SCR_ST        (1U << 11)
> +#define SCR_TWI       (1U << 12)
> +#define SCR_TWE       (1U << 13)
> +#define SCR_RES1_MASK (3U << 4)
> +#define SCR_MASK      (0x3fff & ~SCR_RES1_MASK)
> +
>  /* Return the current FPSCR value.  */
>  uint32_t vfp_get_fpscr(CPUARMState *env);
>  void vfp_set_fpscr(CPUARMState *env, uint32_t val);
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index b04fb5d..6bacc24 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -793,7 +793,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
>        .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),
> +      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState,
> cp15.scr_el3),
>        .resetvalue = 0, },
>      { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH,
>        .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0,
> @@ -2161,6 +2161,31 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
>      REGINFO_SENTINEL
>  };
>
> +static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t
> value)
> +{
> +    uint32_t valid_mask = SCR_MASK;
> +
> +    if (!arm_feature(env, ARM_FEATURE_EL2)) {
> +        valid_mask &= ~SCR_HCE;
> +
> +        /* On ARMv7, SMD (or SCD as it is called in v7) is only
> +         * supported if EL2 exists. The bit is UNK/SBZP when
> +         * EL2 is unavailable. In QEMU ARMv7, we force it to always zero
> +         * when EL2 is unavailable.
> +         */
> +        if (arm_feature(env, ARM_FEATURE_V7)) {
> +            valid_mask &= ~SCR_SMD;
> +        }
> +    }
> +
> +    /* Set RES1 bits.  */
> +    value |= SCR_RES1_MASK;
> +
> +    /* Clear RES0 bits.  */
> +    value &= valid_mask;
> +    raw_write(env, ri, value);
> +}
> +
>  static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
>      { .name = "ELR_EL3", .state = ARM_CP_STATE_AA64,
>        .type = ARM_CP_NO_MIGRATE,
> @@ -2183,6 +2208,10 @@ static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
>        .access = PL3_RW, .writefn = vbar_write,
>        .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[3]),
>        .resetvalue = 0 },
> +    { .name = "SCR_EL3", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 0,
> +      .access = PL3_RW, .fieldoffset = offsetof(CPUARMState,
> cp15.scr_el3),
> +      .writefn = scr_write },
>      REGINFO_SENTINEL
>  };
>
> --
> 1.8.3.2
>
>

[-- Attachment #2: Type: text/html, Size: 5829 bytes --]

Re: [Qemu-devel] [PATCH v3 08/16] target-arm: Add SCR_EL3

[Peter Maydell]

On 17 June 2014 09:45, 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    | 16 +++++++++++++++-
>  target-arm/helper.c | 31 ++++++++++++++++++++++++++++++-
>  2 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index fd57fb5..fa8dee0 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -172,7 +172,6 @@ typedef struct CPUARMState {
>          uint64_t c1_sys; /* System control register.  */
>          uint64_t c1_coproc; /* Coprocessor access register.  */
>          uint32_t c1_xscaleauxcr; /* XScale auxiliary control register.  */
> -        uint32_t c1_scr; /* secure config register.  */
>          uint64_t ttbr0_el1; /* MMU translation table base 0. */
>          uint64_t ttbr1_el1; /* MMU translation table base 1. */
>          uint64_t c2_control; /* MMU translation table base control.  */
> @@ -185,6 +184,7 @@ typedef struct CPUARMState {
>          uint32_t pmsav5_data_ap; /* PMSAv5 MPU data access permissions */
>          uint32_t pmsav5_insn_ap; /* PMSAv5 MPU insn access permissions */
>          uint64_t hcr_el2; /* Hypervisor configuration register */
> +        uint32_t scr_el3; /* Secure configuration register.  */
>          uint32_t ifsr_el2; /* Fault status registers.  */
>          uint64_t esr_el[4];
>          uint32_t c6_region[8]; /* MPU base/size registers.  */
> @@ -562,6 +562,20 @@ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
>  #define HCR_ID        (1ULL << 33)
>  #define HCR_MASK      ((1ULL << 34) - 1)
>
> +#define SCR_NS        (1U << 0)
> +#define SCR_IRQ       (1U << 1)
> +#define SCR_FIQ       (1U << 2)
> +#define SCR_EA        (1U << 3)
> +#define SCR_SMD       (1U << 7)
> +#define SCR_HCE       (1U << 8)
> +#define SCR_SIF       (1U << 9)
> +#define SCR_RW        (1U << 10)
> +#define SCR_ST        (1U << 11)
> +#define SCR_TWI       (1U << 12)
> +#define SCR_TWE       (1U << 13)
> +#define SCR_RES1_MASK (3U << 4)
> +#define SCR_MASK      (0x3fff & ~SCR_RES1_MASK)
> +
>  /* Return the current FPSCR value.  */
>  uint32_t vfp_get_fpscr(CPUARMState *env);
>  void vfp_set_fpscr(CPUARMState *env, uint32_t val);
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index b04fb5d..6bacc24 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -793,7 +793,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
>        .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),
> +      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3),
>        .resetvalue = 0, },

It's awkward that this is now separate from the AArch64 reginfo
below, because it makes it non-obvious that they're both the
same underlying state. In particular that probably means this
one now needs a NO_MIGRATE marker?

>      { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH,
>        .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0,
> @@ -2161,6 +2161,31 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
>      REGINFO_SENTINEL
>  };
>
> +static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
> +{
> +    uint32_t valid_mask = SCR_MASK;
> +
> +    if (!arm_feature(env, ARM_FEATURE_EL2)) {
> +        valid_mask &= ~SCR_HCE;
> +
> +        /* On ARMv7, SMD (or SCD as it is called in v7) is only
> +         * supported if EL2 exists. The bit is UNK/SBZP when
> +         * EL2 is unavailable. In QEMU ARMv7, we force it to always zero
> +         * when EL2 is unavailable.
> +         */
> +        if (arm_feature(env, ARM_FEATURE_V7)) {
> +            valid_mask &= ~SCR_SMD;
> +        }
> +    }
> +
> +    /* Set RES1 bits.  */
> +    value |= SCR_RES1_MASK;
> +
> +    /* Clear RES0 bits.  */
> +    value &= valid_mask;
> +    raw_write(env, ri, value);
> +}
> +
>  static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
>      { .name = "ELR_EL3", .state = ARM_CP_STATE_AA64,
>        .type = ARM_CP_NO_MIGRATE,
> @@ -2183,6 +2208,10 @@ static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
>        .access = PL3_RW, .writefn = vbar_write,
>        .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[3]),
>        .resetvalue = 0 },
> +    { .name = "SCR_EL3", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 0,
> +      .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3),
> +      .writefn = scr_write },
>      REGINFO_SENTINEL
>  };

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 08/16] target-arm: Add SCR_EL3

[Edgar E. Iglesias]

On Fri, Aug 01, 2014 at 02:34:14PM +0100, Peter Maydell wrote:
> On 17 June 2014 09:45, 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    | 16 +++++++++++++++-
> >  target-arm/helper.c | 31 ++++++++++++++++++++++++++++++-
> >  2 files changed, 45 insertions(+), 2 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index fd57fb5..fa8dee0 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -172,7 +172,6 @@ typedef struct CPUARMState {
> >          uint64_t c1_sys; /* System control register.  */
> >          uint64_t c1_coproc; /* Coprocessor access register.  */
> >          uint32_t c1_xscaleauxcr; /* XScale auxiliary control register.  */
> > -        uint32_t c1_scr; /* secure config register.  */
> >          uint64_t ttbr0_el1; /* MMU translation table base 0. */
> >          uint64_t ttbr1_el1; /* MMU translation table base 1. */
> >          uint64_t c2_control; /* MMU translation table base control.  */
> > @@ -185,6 +184,7 @@ typedef struct CPUARMState {
> >          uint32_t pmsav5_data_ap; /* PMSAv5 MPU data access permissions */
> >          uint32_t pmsav5_insn_ap; /* PMSAv5 MPU insn access permissions */
> >          uint64_t hcr_el2; /* Hypervisor configuration register */
> > +        uint32_t scr_el3; /* Secure configuration register.  */
> >          uint32_t ifsr_el2; /* Fault status registers.  */
> >          uint64_t esr_el[4];
> >          uint32_t c6_region[8]; /* MPU base/size registers.  */
> > @@ -562,6 +562,20 @@ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
> >  #define HCR_ID        (1ULL << 33)
> >  #define HCR_MASK      ((1ULL << 34) - 1)
> >
> > +#define SCR_NS        (1U << 0)
> > +#define SCR_IRQ       (1U << 1)
> > +#define SCR_FIQ       (1U << 2)
> > +#define SCR_EA        (1U << 3)
> > +#define SCR_SMD       (1U << 7)
> > +#define SCR_HCE       (1U << 8)
> > +#define SCR_SIF       (1U << 9)
> > +#define SCR_RW        (1U << 10)
> > +#define SCR_ST        (1U << 11)
> > +#define SCR_TWI       (1U << 12)
> > +#define SCR_TWE       (1U << 13)
> > +#define SCR_RES1_MASK (3U << 4)
> > +#define SCR_MASK      (0x3fff & ~SCR_RES1_MASK)
> > +
> >  /* Return the current FPSCR value.  */
> >  uint32_t vfp_get_fpscr(CPUARMState *env);
> >  void vfp_set_fpscr(CPUARMState *env, uint32_t val);
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index b04fb5d..6bacc24 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -793,7 +793,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
> >        .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),
> > +      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3),
> >        .resetvalue = 0, },
> 
> It's awkward that this is now separate from the AArch64 reginfo
> below, because it makes it non-obvious that they're both the
> same underlying state. In particular that probably means this
> one now needs a NO_MIGRATE marker?

Yes, I've moved this into the el3 structure and added NO_MIGRATE.

Thanks,
Edgar


> 
> >      { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH,
> >        .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0,
> > @@ -2161,6 +2161,31 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
> >      REGINFO_SENTINEL
> >  };
> >
> > +static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
> > +{
> > +    uint32_t valid_mask = SCR_MASK;
> > +
> > +    if (!arm_feature(env, ARM_FEATURE_EL2)) {
> > +        valid_mask &= ~SCR_HCE;
> > +
> > +        /* On ARMv7, SMD (or SCD as it is called in v7) is only
> > +         * supported if EL2 exists. The bit is UNK/SBZP when
> > +         * EL2 is unavailable. In QEMU ARMv7, we force it to always zero
> > +         * when EL2 is unavailable.
> > +         */
> > +        if (arm_feature(env, ARM_FEATURE_V7)) {
> > +            valid_mask &= ~SCR_SMD;
> > +        }
> > +    }
> > +
> > +    /* Set RES1 bits.  */
> > +    value |= SCR_RES1_MASK;
> > +
> > +    /* Clear RES0 bits.  */
> > +    value &= valid_mask;
> > +    raw_write(env, ri, value);
> > +}
> > +
> >  static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
> >      { .name = "ELR_EL3", .state = ARM_CP_STATE_AA64,
> >        .type = ARM_CP_NO_MIGRATE,
> > @@ -2183,6 +2208,10 @@ static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
> >        .access = PL3_RW, .writefn = vbar_write,
> >        .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[3]),
> >        .resetvalue = 0 },
> > +    { .name = "SCR_EL3", .state = ARM_CP_STATE_AA64,
> > +      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 0,
> > +      .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3),
> > +      .writefn = scr_write },
> >      REGINFO_SENTINEL
> >  };
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v3 08/16] target-arm: Add SCR_EL3

[Greg Bellows]

[-- Attachment #1: Type: text/plain, Size: 6193 bytes --]

Hi Edgar,

I was just writing a test to verify the correct behavior of the SCR AW/FW
bits and I think there is an issue.

During an SCR write an initial valid mask is set from SCR_MASK which is
defined to not include these bits.  Then these bits are hard-coded into the
write value using RES1.  Last, the new value is masked against the valid
bits for which these bits are masked off.

I have a number of questions:
- Why are we marking these bits off as reserved?  Shouldn't they be RW?
- Are you intending to always enable them or always disable them?
- Why are we attempting to hard-code them 'on' in the value?  Is it because
they have no value when VIRT is enabled?  If so, we should check for EL2.

Thanks for any insight.

Greg


On 4 August 2014 10:19, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:

> On Fri, Aug 01, 2014 at 02:34:14PM +0100, Peter Maydell wrote:
> > On 17 June 2014 09:45, 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    | 16 +++++++++++++++-
> > >  target-arm/helper.c | 31 ++++++++++++++++++++++++++++++-
> > >  2 files changed, 45 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > > index fd57fb5..fa8dee0 100644
> > > --- a/target-arm/cpu.h
> > > +++ b/target-arm/cpu.h
> > > @@ -172,7 +172,6 @@ typedef struct CPUARMState {
> > >          uint64_t c1_sys; /* System control register.  */
> > >          uint64_t c1_coproc; /* Coprocessor access register.  */
> > >          uint32_t c1_xscaleauxcr; /* XScale auxiliary control
> register.  */
> > > -        uint32_t c1_scr; /* secure config register.  */
> > >          uint64_t ttbr0_el1; /* MMU translation table base 0. */
> > >          uint64_t ttbr1_el1; /* MMU translation table base 1. */
> > >          uint64_t c2_control; /* MMU translation table base control.
>  */
> > > @@ -185,6 +184,7 @@ typedef struct CPUARMState {
> > >          uint32_t pmsav5_data_ap; /* PMSAv5 MPU data access
> permissions */
> > >          uint32_t pmsav5_insn_ap; /* PMSAv5 MPU insn access
> permissions */
> > >          uint64_t hcr_el2; /* Hypervisor configuration register */
> > > +        uint32_t scr_el3; /* Secure configuration register.  */
> > >          uint32_t ifsr_el2; /* Fault status registers.  */
> > >          uint64_t esr_el[4];
> > >          uint32_t c6_region[8]; /* MPU base/size registers.  */
> > > @@ -562,6 +562,20 @@ static inline void xpsr_write(CPUARMState *env,
> uint32_t val, uint32_t mask)
> > >  #define HCR_ID        (1ULL << 33)
> > >  #define HCR_MASK      ((1ULL << 34) - 1)
> > >
> > > +#define SCR_NS        (1U << 0)
> > > +#define SCR_IRQ       (1U << 1)
> > > +#define SCR_FIQ       (1U << 2)
> > > +#define SCR_EA        (1U << 3)
> > > +#define SCR_SMD       (1U << 7)
> > > +#define SCR_HCE       (1U << 8)
> > > +#define SCR_SIF       (1U << 9)
> > > +#define SCR_RW        (1U << 10)
> > > +#define SCR_ST        (1U << 11)
> > > +#define SCR_TWI       (1U << 12)
> > > +#define SCR_TWE       (1U << 13)
> > > +#define SCR_RES1_MASK (3U << 4)
> > > +#define SCR_MASK      (0x3fff & ~SCR_RES1_MASK)
> > > +
> > >  /* Return the current FPSCR value.  */
> > >  uint32_t vfp_get_fpscr(CPUARMState *env);
> > >  void vfp_set_fpscr(CPUARMState *env, uint32_t val);
> > > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > > index b04fb5d..6bacc24 100644
> > > --- a/target-arm/helper.c
> > > +++ b/target-arm/helper.c
> > > @@ -793,7 +793,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
> > >        .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),
> > > +      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState,
> cp15.scr_el3),
> > >        .resetvalue = 0, },
> >
> > It's awkward that this is now separate from the AArch64 reginfo
> > below, because it makes it non-obvious that they're both the
> > same underlying state. In particular that probably means this
> > one now needs a NO_MIGRATE marker?
>
> Yes, I've moved this into the el3 structure and added NO_MIGRATE.
>
> Thanks,
> Edgar
>
>
> >
> > >      { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH,
> > >        .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0,
> > > @@ -2161,6 +2161,31 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] =
> {
> > >      REGINFO_SENTINEL
> > >  };
> > >
> > > +static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> uint64_t value)
> > > +{
> > > +    uint32_t valid_mask = SCR_MASK;
> > > +
> > > +    if (!arm_feature(env, ARM_FEATURE_EL2)) {
> > > +        valid_mask &= ~SCR_HCE;
> > > +
> > > +        /* On ARMv7, SMD (or SCD as it is called in v7) is only
> > > +         * supported if EL2 exists. The bit is UNK/SBZP when
> > > +         * EL2 is unavailable. In QEMU ARMv7, we force it to always
> zero
> > > +         * when EL2 is unavailable.
> > > +         */
> > > +        if (arm_feature(env, ARM_FEATURE_V7)) {
> > > +            valid_mask &= ~SCR_SMD;
> > > +        }
> > > +    }
> > > +
> > > +    /* Set RES1 bits.  */
> > > +    value |= SCR_RES1_MASK;
> > > +
> > > +    /* Clear RES0 bits.  */
> > > +    value &= valid_mask;
> > > +    raw_write(env, ri, value);
> > > +}
> > > +
> > >  static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
> > >      { .name = "ELR_EL3", .state = ARM_CP_STATE_AA64,
> > >        .type = ARM_CP_NO_MIGRATE,
> > > @@ -2183,6 +2208,10 @@ static const ARMCPRegInfo v8_el3_cp_reginfo[] =
> {
> > >        .access = PL3_RW, .writefn = vbar_write,
> > >        .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[3]),
> > >        .resetvalue = 0 },
> > > +    { .name = "SCR_EL3", .state = ARM_CP_STATE_AA64,
> > > +      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 0,
> > > +      .access = PL3_RW, .fieldoffset = offsetof(CPUARMState,
> cp15.scr_el3),
> > > +      .writefn = scr_write },
> > >      REGINFO_SENTINEL
> > >  };
> >
> > thanks
> > -- PMM
>

[-- Attachment #2: Type: text/html, Size: 8337 bytes --]

Re: [Qemu-devel] [PATCH v3 08/16] target-arm: Add SCR_EL3

[Edgar E. Iglesias]

On Wed, Aug 13, 2014 at 09:48:35AM -0500, Greg Bellows wrote:
> Hi Edgar,
> 
> I was just writing a test to verify the correct behavior of the SCR AW/FW
> bits and I think there is an issue.
> 
> During an SCR write an initial valid mask is set from SCR_MASK which is
> defined to not include these bits.  Then these bits are hard-coded into the
> write value using RES1.  Last, the new value is masked against the valid
> bits for which these bits are masked off.
> 
> I have a number of questions:
> - Why are we marking these bits off as reserved?  Shouldn't they be RW?
> - Are you intending to always enable them or always disable them?
> - Why are we attempting to hard-code them 'on' in the value?  Is it because
> they have no value when VIRT is enabled?  If so, we should check for EL2.

Hi,

For aarch32 AW/FW are RW but for aarch64 they are reserved.
For a64, I did get them RES0 instead of RES1 though. Will
fix that in v5.

I also plan to move revert the moving of the a32 SCR regdef to
the el3 struct becasue it will cause regression until we have
enough 32bit EL3/Monitor support to start enabling it on v7 CPUs.
I noticed that the 32bit TZ series has followup patches moving
SCR.

Best regards,
Edgar


> 
> Thanks for any insight.
> 
> Greg
> 
> 
> On 4 August 2014 10:19, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> 
> > On Fri, Aug 01, 2014 at 02:34:14PM +0100, Peter Maydell wrote:
> > > On 17 June 2014 09:45, 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    | 16 +++++++++++++++-
> > > >  target-arm/helper.c | 31 ++++++++++++++++++++++++++++++-
> > > >  2 files changed, 45 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > > > index fd57fb5..fa8dee0 100644
> > > > --- a/target-arm/cpu.h
> > > > +++ b/target-arm/cpu.h
> > > > @@ -172,7 +172,6 @@ typedef struct CPUARMState {
> > > >          uint64_t c1_sys; /* System control register.  */
> > > >          uint64_t c1_coproc; /* Coprocessor access register.  */
> > > >          uint32_t c1_xscaleauxcr; /* XScale auxiliary control
> > register.  */
> > > > -        uint32_t c1_scr; /* secure config register.  */
> > > >          uint64_t ttbr0_el1; /* MMU translation table base 0. */
> > > >          uint64_t ttbr1_el1; /* MMU translation table base 1. */
> > > >          uint64_t c2_control; /* MMU translation table base control.
> >  */
> > > > @@ -185,6 +184,7 @@ typedef struct CPUARMState {
> > > >          uint32_t pmsav5_data_ap; /* PMSAv5 MPU data access
> > permissions */
> > > >          uint32_t pmsav5_insn_ap; /* PMSAv5 MPU insn access
> > permissions */
> > > >          uint64_t hcr_el2; /* Hypervisor configuration register */
> > > > +        uint32_t scr_el3; /* Secure configuration register.  */
> > > >          uint32_t ifsr_el2; /* Fault status registers.  */
> > > >          uint64_t esr_el[4];
> > > >          uint32_t c6_region[8]; /* MPU base/size registers.  */
> > > > @@ -562,6 +562,20 @@ static inline void xpsr_write(CPUARMState *env,
> > uint32_t val, uint32_t mask)
> > > >  #define HCR_ID        (1ULL << 33)
> > > >  #define HCR_MASK      ((1ULL << 34) - 1)
> > > >
> > > > +#define SCR_NS        (1U << 0)
> > > > +#define SCR_IRQ       (1U << 1)
> > > > +#define SCR_FIQ       (1U << 2)
> > > > +#define SCR_EA        (1U << 3)
> > > > +#define SCR_SMD       (1U << 7)
> > > > +#define SCR_HCE       (1U << 8)
> > > > +#define SCR_SIF       (1U << 9)
> > > > +#define SCR_RW        (1U << 10)
> > > > +#define SCR_ST        (1U << 11)
> > > > +#define SCR_TWI       (1U << 12)
> > > > +#define SCR_TWE       (1U << 13)
> > > > +#define SCR_RES1_MASK (3U << 4)
> > > > +#define SCR_MASK      (0x3fff & ~SCR_RES1_MASK)
> > > > +
> > > >  /* Return the current FPSCR value.  */
> > > >  uint32_t vfp_get_fpscr(CPUARMState *env);
> > > >  void vfp_set_fpscr(CPUARMState *env, uint32_t val);
> > > > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > > > index b04fb5d..6bacc24 100644
> > > > --- a/target-arm/helper.c
> > > > +++ b/target-arm/helper.c
> > > > @@ -793,7 +793,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
> > > >        .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),
> > > > +      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState,
> > cp15.scr_el3),
> > > >        .resetvalue = 0, },
> > >
> > > It's awkward that this is now separate from the AArch64 reginfo
> > > below, because it makes it non-obvious that they're both the
> > > same underlying state. In particular that probably means this
> > > one now needs a NO_MIGRATE marker?
> >
> > Yes, I've moved this into the el3 structure and added NO_MIGRATE.
> >
> > Thanks,
> > Edgar
> >
> >
> > >
> > > >      { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH,
> > > >        .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0,
> > > > @@ -2161,6 +2161,31 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] =
> > {
> > > >      REGINFO_SENTINEL
> > > >  };
> > > >
> > > > +static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> > uint64_t value)
> > > > +{
> > > > +    uint32_t valid_mask = SCR_MASK;
> > > > +
> > > > +    if (!arm_feature(env, ARM_FEATURE_EL2)) {
> > > > +        valid_mask &= ~SCR_HCE;
> > > > +
> > > > +        /* On ARMv7, SMD (or SCD as it is called in v7) is only
> > > > +         * supported if EL2 exists. The bit is UNK/SBZP when
> > > > +         * EL2 is unavailable. In QEMU ARMv7, we force it to always
> > zero
> > > > +         * when EL2 is unavailable.
> > > > +         */
> > > > +        if (arm_feature(env, ARM_FEATURE_V7)) {
> > > > +            valid_mask &= ~SCR_SMD;
> > > > +        }
> > > > +    }
> > > > +
> > > > +    /* Set RES1 bits.  */
> > > > +    value |= SCR_RES1_MASK;
> > > > +
> > > > +    /* Clear RES0 bits.  */
> > > > +    value &= valid_mask;
> > > > +    raw_write(env, ri, value);
> > > > +}
> > > > +
> > > >  static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
> > > >      { .name = "ELR_EL3", .state = ARM_CP_STATE_AA64,
> > > >        .type = ARM_CP_NO_MIGRATE,
> > > > @@ -2183,6 +2208,10 @@ static const ARMCPRegInfo v8_el3_cp_reginfo[] =
> > {
> > > >        .access = PL3_RW, .writefn = vbar_write,
> > > >        .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[3]),
> > > >        .resetvalue = 0 },
> > > > +    { .name = "SCR_EL3", .state = ARM_CP_STATE_AA64,
> > > > +      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 0,
> > > > +      .access = PL3_RW, .fieldoffset = offsetof(CPUARMState,
> > cp15.scr_el3),
> > > > +      .writefn = scr_write },
> > > >      REGINFO_SENTINEL
> > > >  };
> > >
> > > thanks
> > > -- PMM
> >

[Qemu-devel] [PATCH v3 09/16] target-arm: A64: Refactor aarch64_cpu_do_interrupt

[Edgar E. Iglesias]

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

Introduce new_el and new_mode in preparation for future patches
that add support for taking exceptions to and from EL2 and 3.
No functional change.

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

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index fa8dee0..cd2f74a 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -460,6 +460,12 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
 #define PSTATE_MODE_EL1t 4
 #define PSTATE_MODE_EL0t 0
 
+/* Map EL and handler into a PSTATE_MODE.  */
+static inline unsigned int aarch64_pstate_mode(unsigned int el, bool handler)
+{
+    return (el << 2) | handler;
+}
+
 /* Return the current PSTATE value. For the moment we don't support 32<->64 bit
  * interprocessing, so we don't attempt to sync with the cpsr state used by
  * the 32 bit decoder.
@@ -712,6 +718,7 @@ static inline bool arm_el_is_aa64(CPUARMState *env, int el)
 }
 
 void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf);
+unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx);
 
 /* Interface between CPU and Interrupt controller.  */
 void armv7m_nvic_set_pending(void *opaque, int irq);
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 2e9ef64..4be0784 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -443,10 +443,12 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
 {
     ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;
-    target_ulong addr = env->cp15.vbar_el[1];
+    unsigned int new_el = arm_excp_target_el(cs, cs->exception_index);
+    target_ulong addr = env->cp15.vbar_el[new_el];
+    unsigned int new_mode = aarch64_pstate_mode(new_el, true);
     int i;
 
-    if (arm_current_pl(env) == 0) {
+    if (arm_current_pl(env) < new_el) {
         if (env->aarch64) {
             addr += 0x400;
         } else {
@@ -464,14 +466,14 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
                       env->exception.syndrome);
     }
 
-    env->cp15.esr_el[1] = env->exception.syndrome;
-    env->cp15.far_el[1] = env->exception.vaddress;
+    env->cp15.esr_el[new_el] = env->exception.syndrome;
+    env->cp15.far_el[new_el] = env->exception.vaddress;
 
     switch (cs->exception_index) {
     case EXCP_PREFETCH_ABORT:
     case EXCP_DATA_ABORT:
         qemu_log_mask(CPU_LOG_INT, "...with FAR 0x%" PRIx64 "\n",
-                      env->cp15.far_el[1]);
+                      env->cp15.far_el[new_el]);
         break;
     case EXCP_BKPT:
     case EXCP_UDEF:
@@ -488,15 +490,15 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
     }
 
     if (is_a64(env)) {
-        env->banked_spsr[aarch64_banked_spsr_index(1)] = pstate_read(env);
+        env->banked_spsr[aarch64_banked_spsr_index(new_el)] = pstate_read(env);
         aarch64_save_sp(env, arm_current_pl(env));
-        env->elr_el[1] = env->pc;
+        env->elr_el[new_el] = env->pc;
     } else {
         env->banked_spsr[0] = cpsr_read(env);
         if (!env->thumb) {
-            env->cp15.esr_el[1] |= 1 << 25;
+            env->cp15.esr_el[new_el] |= 1 << 25;
         }
-        env->elr_el[1] = env->regs[15];
+        env->elr_el[new_el] = env->regs[15];
 
         for (i = 0; i < 15; i++) {
             env->xregs[i] = env->regs[i];
@@ -505,9 +507,9 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
         env->condexec_bits = 0;
     }
 
-    pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
+    pstate_write(env, PSTATE_DAIF | new_mode);
     env->aarch64 = 1;
-    aarch64_restore_sp(env, 1);
+    aarch64_restore_sp(env, new_el);
 
     env->pc = addr;
     cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 6bacc24..972e91c 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3216,6 +3216,11 @@ uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
     return 0;
 }
 
+unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
+{
+    return 1;
+}
+
 #else
 
 /* Map CPU modes onto saved register banks.  */
@@ -3271,6 +3276,14 @@ void switch_mode(CPUARMState *env, int mode)
     env->spsr = env->banked_spsr[i];
 }
 
+/*
+ * Determine the target EL for a given exception type.
+ */
+unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
+{
+    return 1;
+}
+
 static void v7m_push(CPUARMState *env, uint32_t val)
 {
     CPUState *cs = CPU(arm_env_get_cpu(env));
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v3 09/16] target-arm: A64: Refactor aarch64_cpu_do_interrupt

[Peter Maydell]

On 17 June 2014 09:45, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Introduce new_el and new_mode in preparation for future patches
> that add support for taking exceptions to and from EL2 and 3.
> No functional change.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/cpu.h        |  7 +++++++
>  target-arm/helper-a64.c | 24 +++++++++++++-----------
>  target-arm/helper.c     | 13 +++++++++++++
>  3 files changed, 33 insertions(+), 11 deletions(-)

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

thanks
-- PMM

[Qemu-devel] [PATCH v3 10/16] target-arm: Break out exception masking to a separate func

[Edgar E. Iglesias]

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

Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 cpu-exec.c       |  5 ++---
 target-arm/cpu.h | 16 ++++++++++++++++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 38e5f02..a579ffc 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -478,7 +478,7 @@ int cpu_exec(CPUArchState *env)
                     }
 #elif defined(TARGET_ARM)
                     if (interrupt_request & CPU_INTERRUPT_FIQ
-                        && !(env->daif & PSTATE_F)) {
+                        && arm_excp_unmasked(cpu, EXCP_FIQ)) {
                         cpu->exception_index = EXCP_FIQ;
                         cc->do_interrupt(cpu);
                         next_tb = 0;
@@ -493,8 +493,7 @@ int cpu_exec(CPUArchState *env)
                        We avoid this by disabling interrupts when
                        pc contains a magic address.  */
                     if (interrupt_request & CPU_INTERRUPT_HARD
-                        && ((IS_M(env) && env->regs[15] < 0xfffffff0)
-                            || !(env->daif & PSTATE_I))) {
+                        && arm_excp_unmasked(cpu, EXCP_IRQ)) {
                         cpu->exception_index = EXCP_IRQ;
                         cc->do_interrupt(cpu);
                         next_tb = 0;
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index cd2f74a..9d21361 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1129,6 +1129,22 @@ bool write_cpustate_to_list(ARMCPU *cpu);
 #  define TARGET_VIRT_ADDR_SPACE_BITS 32
 #endif
 
+static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
+{
+    CPUARMState *env = cs->env_ptr;
+
+    switch (excp_idx) {
+    case EXCP_FIQ:
+        return !(env->daif & PSTATE_F);
+    case EXCP_IRQ:
+        return ((IS_M(env) && env->regs[15] < 0xfffffff0)
+                            || !(env->daif & PSTATE_I));
+    default:
+        g_assert_not_reached();
+        break;
+    }
+}
+
 static inline CPUARMState *cpu_init(const char *cpu_model)
 {
     ARMCPU *cpu = cpu_arm_init(cpu_model);
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v3 10/16] target-arm: Break out exception masking to a separate func

[Peter Maydell]

On 17 June 2014 09:45, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
>
> +static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
> +{
> +    CPUARMState *env = cs->env_ptr;
> +
> +    switch (excp_idx) {
> +    case EXCP_FIQ:
> +        return !(env->daif & PSTATE_F);
> +    case EXCP_IRQ:
> +        return ((IS_M(env) && env->regs[15] < 0xfffffff0)
> +                            || !(env->daif & PSTATE_I));
> +    default:
> +        g_assert_not_reached();
> +        break;

You don't need a break, we've just asserted that this isn't
reachable. (Conversely if it was possible to get past the
assert we'd be falling out of the function without returning
a value, so break is wrong either way. But "just don't
put in the break" is what we do elsewhere in target-arm.)

> +    }
> +}

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 10/16] target-arm: Break out exception masking to a separate func

[Edgar E. Iglesias]

On Fri, Aug 01, 2014 at 02:51:44PM +0100, Peter Maydell wrote:
> On 17 June 2014 09:45, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> >
> > +static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
> > +{
> > +    CPUARMState *env = cs->env_ptr;
> > +
> > +    switch (excp_idx) {
> > +    case EXCP_FIQ:
> > +        return !(env->daif & PSTATE_F);
> > +    case EXCP_IRQ:
> > +        return ((IS_M(env) && env->regs[15] < 0xfffffff0)
> > +                            || !(env->daif & PSTATE_I));
> > +    default:
> > +        g_assert_not_reached();
> > +        break;
> 
> You don't need a break, we've just asserted that this isn't
> reachable. (Conversely if it was possible to get past the
> assert we'd be falling out of the function without returning
> a value, so break is wrong either way. But "just don't
> put in the break" is what we do elsewhere in target-arm.)

I've removed the break, thanks.


> 
> > +    }
> > +}
> 
> thanks
> -- PMM

[Qemu-devel] [PATCH v3 11/16] target-arm: Don't take interrupts targeting lower ELs

[Edgar E. Iglesias]

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

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 9d21361..4f273ac 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1132,6 +1132,13 @@ bool write_cpustate_to_list(ARMCPU *cpu);
 static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
 {
     CPUARMState *env = cs->env_ptr;
+    unsigned int cur_el = arm_current_pl(env);
+    unsigned int target_el = arm_excp_target_el(cs, excp_idx);
+
+    /* Don't take exceptions if they target a lower EL.  */
+    if (cur_el > target_el) {
+        return false;
+    }
 
     switch (excp_idx) {
     case EXCP_FIQ:
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v3 11/16] target-arm: Don't take interrupts targeting lower ELs

[Peter Maydell]

On 17 June 2014 09:45, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/cpu.h | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 9d21361..4f273ac 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1132,6 +1132,13 @@ bool write_cpustate_to_list(ARMCPU *cpu);
>  static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
>  {
>      CPUARMState *env = cs->env_ptr;
> +    unsigned int cur_el = arm_current_pl(env);
> +    unsigned int target_el = arm_excp_target_el(cs, excp_idx);
> +
> +    /* Don't take exceptions if they target a lower EL.  */
> +    if (cur_el > target_el) {
> +        return false;
> +    }
>
>      switch (excp_idx) {
>      case EXCP_FIQ:

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

thanks
-- PMM

[Qemu-devel] [PATCH v3 12/16] target-arm: A64: Correct updates to FAR and ESR on exceptions

[Edgar E. Iglesias]

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

Not all exception types update both FAR and ESR.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper-a64.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 4be0784..cf8ce1e 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -466,18 +466,16 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
                       env->exception.syndrome);
     }
 
-    env->cp15.esr_el[new_el] = env->exception.syndrome;
-    env->cp15.far_el[new_el] = env->exception.vaddress;
-
     switch (cs->exception_index) {
     case EXCP_PREFETCH_ABORT:
     case EXCP_DATA_ABORT:
+        env->cp15.far_el[new_el] = env->exception.vaddress;
         qemu_log_mask(CPU_LOG_INT, "...with FAR 0x%" PRIx64 "\n",
                       env->cp15.far_el[new_el]);
-        break;
     case EXCP_BKPT:
     case EXCP_UDEF:
     case EXCP_SWI:
+        env->cp15.esr_el[new_el] = env->exception.syndrome;
         break;
     case EXCP_IRQ:
         addr += 0x80;
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v3 12/16] target-arm: A64: Correct updates to FAR and ESR on exceptions

[Peter Maydell]

On 17 June 2014 09:45, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Not all exception types update both FAR and ESR.
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/helper-a64.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 4be0784..cf8ce1e 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -466,18 +466,16 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>                        env->exception.syndrome);
>      }
>
> -    env->cp15.esr_el[new_el] = env->exception.syndrome;
> -    env->cp15.far_el[new_el] = env->exception.vaddress;
> -
>      switch (cs->exception_index) {
>      case EXCP_PREFETCH_ABORT:
>      case EXCP_DATA_ABORT:
> +        env->cp15.far_el[new_el] = env->exception.vaddress;
>          qemu_log_mask(CPU_LOG_INT, "...with FAR 0x%" PRIx64 "\n",
>                        env->cp15.far_el[new_el]);
> -        break;

If you want this to fall through, you need a /* fall through */ comment.

>      case EXCP_BKPT:
>      case EXCP_UDEF:
>      case EXCP_SWI:
> +        env->cp15.esr_el[new_el] = env->exception.syndrome;
>          break;
>      case EXCP_IRQ:
>          addr += 0x80;
> --

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 12/16] target-arm: A64: Correct updates to FAR and ESR on exceptions

[Edgar E. Iglesias]

On Fri, Aug 01, 2014 at 02:56:29PM +0100, Peter Maydell wrote:
> On 17 June 2014 09:45, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Not all exception types update both FAR and ESR.
> >
> > Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> > Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/helper-a64.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> > index 4be0784..cf8ce1e 100644
> > --- a/target-arm/helper-a64.c
> > +++ b/target-arm/helper-a64.c
> > @@ -466,18 +466,16 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> >                        env->exception.syndrome);
> >      }
> >
> > -    env->cp15.esr_el[new_el] = env->exception.syndrome;
> > -    env->cp15.far_el[new_el] = env->exception.vaddress;
> > -
> >      switch (cs->exception_index) {
> >      case EXCP_PREFETCH_ABORT:
> >      case EXCP_DATA_ABORT:
> > +        env->cp15.far_el[new_el] = env->exception.vaddress;
> >          qemu_log_mask(CPU_LOG_INT, "...with FAR 0x%" PRIx64 "\n",
> >                        env->cp15.far_el[new_el]);
> > -        break;
> 
> If you want this to fall through, you need a /* fall through */ comment.

Added, thanks.

> 
> >      case EXCP_BKPT:
> >      case EXCP_UDEF:
> >      case EXCP_SWI:
> > +        env->cp15.esr_el[new_el] = env->exception.syndrome;
> >          break;
> >      case EXCP_IRQ:
> >          addr += 0x80;
> > --
> 
> thanks
> -- PMM

[Qemu-devel] [PATCH v3 13/16] target-arm: A64: Emulate the HVC insn

[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           |  1 +
 target-arm/helper-a64.c    |  1 +
 target-arm/helper.c        | 28 +++++++++++++++++++++++++++-
 target-arm/helper.h        |  1 +
 target-arm/internals.h     |  6 ++++++
 target-arm/op_helper.c     | 33 +++++++++++++++++++++++++++++++++
 target-arm/translate-a64.c | 21 ++++++++++++++++-----
 7 files changed, 85 insertions(+), 6 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 4f273ac..258bc09 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -51,6 +51,7 @@
 #define EXCP_EXCEPTION_EXIT  8   /* Return from v7M exception.  */
 #define EXCP_KERNEL_TRAP     9   /* Jumped to kernel code page.  */
 #define EXCP_STREX          10
+#define EXCP_HVC            11   /* HyperVisor Call */
 
 #define ARMV7M_EXCP_RESET   1
 #define ARMV7M_EXCP_NMI     2
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index cf8ce1e..7382d0a 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -475,6 +475,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
     case EXCP_BKPT:
     case EXCP_UDEF:
     case EXCP_SWI:
+    case EXCP_HVC:
         env->cp15.esr_el[new_el] = env->exception.syndrome;
         break;
     case EXCP_IRQ:
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 972e91c..44e8d47 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3281,7 +3281,33 @@ void switch_mode(CPUARMState *env, int mode)
  */
 unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
 {
-    return 1;
+    CPUARMState *env = cs->env_ptr;
+    unsigned int cur_el = arm_current_pl(env);
+    unsigned int target_el = 1;
+    bool route_to_el2 = false;
+    /* FIXME: Use actual secure state.  */
+    bool secure = false;
+
+    if (!env->aarch64) {
+        /* TODO: Add EL2 and 3 exception handling for AArch32.  */
+        return 1;
+    }
+
+    if (!secure
+        && arm_feature(env, ARM_FEATURE_EL2)
+        && cur_el < 2
+        && (env->cp15.hcr_el2 & HCR_TGE)) {
+        route_to_el2 = true;
+    }
+
+    target_el = MAX(cur_el, route_to_el2 ? 2 : 1);
+
+    switch (excp_idx) {
+    case EXCP_HVC:
+        target_el = MAX(target_el, 2);
+        break;
+    }
+    return target_el;
 }
 
 static void v7m_push(CPUARMState *env, uint32_t val)
diff --git a/target-arm/helper.h b/target-arm/helper.h
index facfcd2..70cfd28 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -50,6 +50,7 @@ DEF_HELPER_2(exception_internal, void, env, i32)
 DEF_HELPER_3(exception_with_syndrome, void, env, i32, i32)
 DEF_HELPER_1(wfi, void, env)
 DEF_HELPER_1(wfe, void, env)
+DEF_HELPER_2(hvc, void, env, i32)
 
 DEF_HELPER_3(cpsr_write, void, env, i32, i32)
 DEF_HELPER_1(cpsr_read, i32, env)
diff --git a/target-arm/internals.h b/target-arm/internals.h
index 08fa697..b68e6f9 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -53,6 +53,7 @@ static const char * const excnames[] = {
     [EXCP_EXCEPTION_EXIT] = "QEMU v7M exception exit",
     [EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage",
     [EXCP_STREX] = "QEMU intercept of STREX",
+    [EXCP_HVC] = "Hypervisor Call",
 };
 
 static inline void arm_log_exception(int idx)
@@ -204,6 +205,11 @@ static inline uint32_t syn_aa64_svc(uint32_t imm16)
     return (EC_AA64_SVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff);
 }
 
+static inline uint32_t syn_aa64_hvc(uint16_t imm16)
+{
+    return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | imm16;
+}
+
 static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb)
 {
     return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0xffff)
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 25ad902..c1fa797 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -369,6 +369,39 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
     }
 }
 
+void HELPER(hvc)(CPUARMState *env, uint32_t syndrome)
+{
+    int cur_el = arm_current_pl(env);
+    /* FIXME: Use actual secure state.  */
+    bool secure = false;
+    bool udef;
+
+    /* We've already checked that EL2 exists at translation time.
+     * EL3.HCE has priority over EL2.HCD.
+     */
+    if (arm_feature(env, ARM_FEATURE_EL3)) {
+        udef = !(env->cp15.scr_el3 & SCR_HCE);
+    } else {
+        udef = env->cp15.hcr_el2 & HCR_HCD;
+    }
+
+    /* In ARMv7 and ARMv8/AArch32, HVC is udef in secure state.
+     * For ARMv8/AArch64, HVC is allowed in EL3.
+     * Note that we've already trapped HVC from EL0 at translation
+     * time.
+     */
+    if (secure && (!is_a64(env) || cur_el == 1)) {
+        udef = true;
+    }
+
+    if (udef) {
+        env->exception.syndrome = syn_uncategorized();
+        raise_exception(env, EXCP_UDEF);
+    }
+    env->exception.syndrome = syndrome;
+    raise_exception(env, EXCP_HVC);
+}
+
 void HELPER(exception_return)(CPUARMState *env)
 {
     int cur_el = arm_current_pl(env);
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 63ad787..5692dff 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1436,17 +1436,28 @@ static void disas_exc(DisasContext *s, uint32_t insn)
     int opc = extract32(insn, 21, 3);
     int op2_ll = extract32(insn, 0, 5);
     int imm16 = extract32(insn, 5, 16);
+    TCGv_i32 tmp;
 
     switch (opc) {
     case 0:
-        /* SVC, HVC, SMC; since we don't support the Virtualization
-         * or TrustZone extensions these all UNDEF except SVC.
-         */
-        if (op2_ll != 1) {
+        switch (op2_ll) {
+        case 1:
+            gen_exception_insn(s, 0, EXCP_SWI, syn_aa64_svc(imm16));
+            break;
+        case 2:
+            if (!arm_dc_feature(s, ARM_FEATURE_EL2) || s->current_pl == 0) {
+                unallocated_encoding(s);
+                break;
+            }
+            tmp = tcg_const_i32(syn_aa64_hvc(imm16));
+            gen_a64_set_pc_im(s->pc);
+            gen_helper_hvc(cpu_env, tmp);
+            tcg_temp_free_i32(tmp);
+            break;
+        default:
             unallocated_encoding(s);
             break;
         }
-        gen_exception_insn(s, 0, EXCP_SWI, syn_aa64_svc(imm16));
         break;
     case 1:
         if (op2_ll != 0) {
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v3 13/16] target-arm: A64: Emulate the HVC insn

[Peter Maydell]

On 17 June 2014 09:45, 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           |  1 +
>  target-arm/helper-a64.c    |  1 +
>  target-arm/helper.c        | 28 +++++++++++++++++++++++++++-
>  target-arm/helper.h        |  1 +
>  target-arm/internals.h     |  6 ++++++
>  target-arm/op_helper.c     | 33 +++++++++++++++++++++++++++++++++
>  target-arm/translate-a64.c | 21 ++++++++++++++++-----
>  7 files changed, 85 insertions(+), 6 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 4f273ac..258bc09 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -51,6 +51,7 @@
>  #define EXCP_EXCEPTION_EXIT  8   /* Return from v7M exception.  */
>  #define EXCP_KERNEL_TRAP     9   /* Jumped to kernel code page.  */
>  #define EXCP_STREX          10
> +#define EXCP_HVC            11   /* HyperVisor Call */
>
>  #define ARMV7M_EXCP_RESET   1
>  #define ARMV7M_EXCP_NMI     2
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index cf8ce1e..7382d0a 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -475,6 +475,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>      case EXCP_BKPT:
>      case EXCP_UDEF:
>      case EXCP_SWI:
> +    case EXCP_HVC:
>          env->cp15.esr_el[new_el] = env->exception.syndrome;
>          break;
>      case EXCP_IRQ:
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 972e91c..44e8d47 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3281,7 +3281,33 @@ void switch_mode(CPUARMState *env, int mode)
>   */
>  unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
>  {
> -    return 1;
> +    CPUARMState *env = cs->env_ptr;
> +    unsigned int cur_el = arm_current_pl(env);
> +    unsigned int target_el = 1;
> +    bool route_to_el2 = false;
> +    /* FIXME: Use actual secure state.  */
> +    bool secure = false;
> +
> +    if (!env->aarch64) {
> +        /* TODO: Add EL2 and 3 exception handling for AArch32.  */
> +        return 1;
> +    }
> +
> +    if (!secure
> +        && arm_feature(env, ARM_FEATURE_EL2)
> +        && cur_el < 2
> +        && (env->cp15.hcr_el2 & HCR_TGE)) {
> +        route_to_el2 = true;
> +    }
> +
> +    target_el = MAX(cur_el, route_to_el2 ? 2 : 1);
> +
> +    switch (excp_idx) {
> +    case EXCP_HVC:
> +        target_el = MAX(target_el, 2);
> +        break;
> +    }
> +    return target_el;

I was wondering if this was going to get a bit heavyweight to
call twice on each trip round the cpu-exec.c main loop, but
we'll only do that in the case that an interrupt is pending
but currently masked I guess so it should be OK.

>  }
>
>  static void v7m_push(CPUARMState *env, uint32_t val)
> diff --git a/target-arm/helper.h b/target-arm/helper.h
> index facfcd2..70cfd28 100644
> --- a/target-arm/helper.h
> +++ b/target-arm/helper.h
> @@ -50,6 +50,7 @@ DEF_HELPER_2(exception_internal, void, env, i32)
>  DEF_HELPER_3(exception_with_syndrome, void, env, i32, i32)
>  DEF_HELPER_1(wfi, void, env)
>  DEF_HELPER_1(wfe, void, env)
> +DEF_HELPER_2(hvc, void, env, i32)
>
>  DEF_HELPER_3(cpsr_write, void, env, i32, i32)
>  DEF_HELPER_1(cpsr_read, i32, env)
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index 08fa697..b68e6f9 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -53,6 +53,7 @@ static const char * const excnames[] = {
>      [EXCP_EXCEPTION_EXIT] = "QEMU v7M exception exit",
>      [EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage",
>      [EXCP_STREX] = "QEMU intercept of STREX",
> +    [EXCP_HVC] = "Hypervisor Call",
>  };
>
>  static inline void arm_log_exception(int idx)
> @@ -204,6 +205,11 @@ static inline uint32_t syn_aa64_svc(uint32_t imm16)
>      return (EC_AA64_SVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff);
>  }
>
> +static inline uint32_t syn_aa64_hvc(uint16_t imm16)

This isn't consistent with the other syn_ functions which take a
uint32_t imm16. (Didn't we do this before?)

> +{
> +    return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | imm16;
> +}
> +
>  static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb)
>  {
>      return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0xffff)
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 25ad902..c1fa797 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -369,6 +369,39 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
>      }
>  }
>
> +void HELPER(hvc)(CPUARMState *env, uint32_t syndrome)
> +{
> +    int cur_el = arm_current_pl(env);
> +    /* FIXME: Use actual secure state.  */
> +    bool secure = false;
> +    bool udef;
> +
> +    /* We've already checked that EL2 exists at translation time.
> +     * EL3.HCE has priority over EL2.HCD.
> +     */
> +    if (arm_feature(env, ARM_FEATURE_EL3)) {
> +        udef = !(env->cp15.scr_el3 & SCR_HCE);
> +    } else {
> +        udef = env->cp15.hcr_el2 & HCR_HCD;
> +    }
> +
> +    /* In ARMv7 and ARMv8/AArch32, HVC is udef in secure state.
> +     * For ARMv8/AArch64, HVC is allowed in EL3.
> +     * Note that we've already trapped HVC from EL0 at translation
> +     * time.
> +     */
> +    if (secure && (!is_a64(env) || cur_el == 1)) {
> +        udef = true;
> +    }
> +
> +    if (udef) {
> +        env->exception.syndrome = syn_uncategorized();
> +        raise_exception(env, EXCP_UDEF);
> +    }
> +    env->exception.syndrome = syndrome;
> +    raise_exception(env, EXCP_HVC);
> +}
> +
>  void HELPER(exception_return)(CPUARMState *env)
>  {
>      int cur_el = arm_current_pl(env);
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 63ad787..5692dff 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -1436,17 +1436,28 @@ static void disas_exc(DisasContext *s, uint32_t insn)
>      int opc = extract32(insn, 21, 3);
>      int op2_ll = extract32(insn, 0, 5);
>      int imm16 = extract32(insn, 5, 16);
> +    TCGv_i32 tmp;
>
>      switch (opc) {
>      case 0:
> -        /* SVC, HVC, SMC; since we don't support the Virtualization
> -         * or TrustZone extensions these all UNDEF except SVC.
> -         */
> -        if (op2_ll != 1) {
> +        switch (op2_ll) {
> +        case 1:
> +            gen_exception_insn(s, 0, EXCP_SWI, syn_aa64_svc(imm16));
> +            break;
> +        case 2:
> +            if (!arm_dc_feature(s, ARM_FEATURE_EL2) || s->current_pl == 0) {
> +                unallocated_encoding(s);
> +                break;
> +            }
> +            tmp = tcg_const_i32(syn_aa64_hvc(imm16));
> +            gen_a64_set_pc_im(s->pc);

(This set_pc_im is unnecessary.)

> +            gen_helper_hvc(cpu_env, tmp);

This means that exceptions due to HVC are going to be
runtime-detected and cause us to go through and retranslate
the TB to determine the guest PC. Maybe we should do:

    /* This helper will raise EXCP_UDEF if HVC is not permitted */
    gen_helper_hvc_access_check(cpu_env);
    /* Common case: HVC causes EXCP_HVC */
    gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16));

Then you only get the overhead of retranslating the TB in the
unexpected case where the guest does something dumb and
executes an HVC that UNDEFs.

> +            tcg_temp_free_i32(tmp);
> +            break;
> +        default:
>              unallocated_encoding(s);
>              break;
>          }
> -        gen_exception_insn(s, 0, EXCP_SWI, syn_aa64_svc(imm16));
>          break;
>      case 1:
>          if (op2_ll != 0) {

General comment, this is likely to clash with the PSCI support; I
guess we'll see which gets in first.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 13/16] target-arm: A64: Emulate the HVC insn

[Edgar E. Iglesias]

On Fri, Aug 01, 2014 at 03:21:08PM +0100, Peter Maydell wrote:
> On 17 June 2014 09:45, 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           |  1 +
> >  target-arm/helper-a64.c    |  1 +
> >  target-arm/helper.c        | 28 +++++++++++++++++++++++++++-
> >  target-arm/helper.h        |  1 +
> >  target-arm/internals.h     |  6 ++++++
> >  target-arm/op_helper.c     | 33 +++++++++++++++++++++++++++++++++
> >  target-arm/translate-a64.c | 21 ++++++++++++++++-----
> >  7 files changed, 85 insertions(+), 6 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 4f273ac..258bc09 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -51,6 +51,7 @@
> >  #define EXCP_EXCEPTION_EXIT  8   /* Return from v7M exception.  */
> >  #define EXCP_KERNEL_TRAP     9   /* Jumped to kernel code page.  */
> >  #define EXCP_STREX          10
> > +#define EXCP_HVC            11   /* HyperVisor Call */
> >
> >  #define ARMV7M_EXCP_RESET   1
> >  #define ARMV7M_EXCP_NMI     2
> > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> > index cf8ce1e..7382d0a 100644
> > --- a/target-arm/helper-a64.c
> > +++ b/target-arm/helper-a64.c
> > @@ -475,6 +475,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> >      case EXCP_BKPT:
> >      case EXCP_UDEF:
> >      case EXCP_SWI:
> > +    case EXCP_HVC:
> >          env->cp15.esr_el[new_el] = env->exception.syndrome;
> >          break;
> >      case EXCP_IRQ:
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 972e91c..44e8d47 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -3281,7 +3281,33 @@ void switch_mode(CPUARMState *env, int mode)
> >   */
> >  unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
> >  {
> > -    return 1;
> > +    CPUARMState *env = cs->env_ptr;
> > +    unsigned int cur_el = arm_current_pl(env);
> > +    unsigned int target_el = 1;
> > +    bool route_to_el2 = false;
> > +    /* FIXME: Use actual secure state.  */
> > +    bool secure = false;
> > +
> > +    if (!env->aarch64) {
> > +        /* TODO: Add EL2 and 3 exception handling for AArch32.  */
> > +        return 1;
> > +    }
> > +
> > +    if (!secure
> > +        && arm_feature(env, ARM_FEATURE_EL2)
> > +        && cur_el < 2
> > +        && (env->cp15.hcr_el2 & HCR_TGE)) {
> > +        route_to_el2 = true;
> > +    }
> > +
> > +    target_el = MAX(cur_el, route_to_el2 ? 2 : 1);
> > +
> > +    switch (excp_idx) {
> > +    case EXCP_HVC:
> > +        target_el = MAX(target_el, 2);
> > +        break;
> > +    }
> > +    return target_el;
> 
> I was wondering if this was going to get a bit heavyweight to
> call twice on each trip round the cpu-exec.c main loop, but
> we'll only do that in the case that an interrupt is pending
> but currently masked I guess so it should be OK.
> 
> >  }
> >
> >  static void v7m_push(CPUARMState *env, uint32_t val)
> > diff --git a/target-arm/helper.h b/target-arm/helper.h
> > index facfcd2..70cfd28 100644
> > --- a/target-arm/helper.h
> > +++ b/target-arm/helper.h
> > @@ -50,6 +50,7 @@ DEF_HELPER_2(exception_internal, void, env, i32)
> >  DEF_HELPER_3(exception_with_syndrome, void, env, i32, i32)
> >  DEF_HELPER_1(wfi, void, env)
> >  DEF_HELPER_1(wfe, void, env)
> > +DEF_HELPER_2(hvc, void, env, i32)
> >
> >  DEF_HELPER_3(cpsr_write, void, env, i32, i32)
> >  DEF_HELPER_1(cpsr_read, i32, env)
> > diff --git a/target-arm/internals.h b/target-arm/internals.h
> > index 08fa697..b68e6f9 100644
> > --- a/target-arm/internals.h
> > +++ b/target-arm/internals.h
> > @@ -53,6 +53,7 @@ static const char * const excnames[] = {
> >      [EXCP_EXCEPTION_EXIT] = "QEMU v7M exception exit",
> >      [EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage",
> >      [EXCP_STREX] = "QEMU intercept of STREX",
> > +    [EXCP_HVC] = "Hypervisor Call",
> >  };
> >
> >  static inline void arm_log_exception(int idx)
> > @@ -204,6 +205,11 @@ static inline uint32_t syn_aa64_svc(uint32_t imm16)
> >      return (EC_AA64_SVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff);
> >  }
> >
> > +static inline uint32_t syn_aa64_hvc(uint16_t imm16)
> 
> This isn't consistent with the other syn_ functions which take a
> uint32_t imm16. (Didn't we do this before?)

OK, I thought we were not going to change the existing calls in this
series but the ones I add would not use the explicit masking. Some
future series would eventually change the others.

It's got no importance at all for this series so I'm happy to change it
allthough I personally prefer the uint16_t code.


> 
> > +{
> > +    return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | imm16;
> > +}
> > +
> >  static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb)
> >  {
> >      return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0xffff)
> > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> > index 25ad902..c1fa797 100644
> > --- a/target-arm/op_helper.c
> > +++ b/target-arm/op_helper.c
> > @@ -369,6 +369,39 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
> >      }
> >  }
> >
> > +void HELPER(hvc)(CPUARMState *env, uint32_t syndrome)
> > +{
> > +    int cur_el = arm_current_pl(env);
> > +    /* FIXME: Use actual secure state.  */
> > +    bool secure = false;
> > +    bool udef;
> > +
> > +    /* We've already checked that EL2 exists at translation time.
> > +     * EL3.HCE has priority over EL2.HCD.
> > +     */
> > +    if (arm_feature(env, ARM_FEATURE_EL3)) {
> > +        udef = !(env->cp15.scr_el3 & SCR_HCE);
> > +    } else {
> > +        udef = env->cp15.hcr_el2 & HCR_HCD;
> > +    }
> > +
> > +    /* In ARMv7 and ARMv8/AArch32, HVC is udef in secure state.
> > +     * For ARMv8/AArch64, HVC is allowed in EL3.
> > +     * Note that we've already trapped HVC from EL0 at translation
> > +     * time.
> > +     */
> > +    if (secure && (!is_a64(env) || cur_el == 1)) {
> > +        udef = true;
> > +    }
> > +
> > +    if (udef) {
> > +        env->exception.syndrome = syn_uncategorized();
> > +        raise_exception(env, EXCP_UDEF);
> > +    }
> > +    env->exception.syndrome = syndrome;
> > +    raise_exception(env, EXCP_HVC);
> > +}
> > +
> >  void HELPER(exception_return)(CPUARMState *env)
> >  {
> >      int cur_el = arm_current_pl(env);
> > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> > index 63ad787..5692dff 100644
> > --- a/target-arm/translate-a64.c
> > +++ b/target-arm/translate-a64.c
> > @@ -1436,17 +1436,28 @@ static void disas_exc(DisasContext *s, uint32_t insn)
> >      int opc = extract32(insn, 21, 3);
> >      int op2_ll = extract32(insn, 0, 5);
> >      int imm16 = extract32(insn, 5, 16);
> > +    TCGv_i32 tmp;
> >
> >      switch (opc) {
> >      case 0:
> > -        /* SVC, HVC, SMC; since we don't support the Virtualization
> > -         * or TrustZone extensions these all UNDEF except SVC.
> > -         */
> > -        if (op2_ll != 1) {
> > +        switch (op2_ll) {
> > +        case 1:
> > +            gen_exception_insn(s, 0, EXCP_SWI, syn_aa64_svc(imm16));
> > +            break;
> > +        case 2:
> > +            if (!arm_dc_feature(s, ARM_FEATURE_EL2) || s->current_pl == 0) {
> > +                unallocated_encoding(s);
> > +                break;
> > +            }
> > +            tmp = tcg_const_i32(syn_aa64_hvc(imm16));
> > +            gen_a64_set_pc_im(s->pc);
> 
> (This set_pc_im is unnecessary.)
> 
> > +            gen_helper_hvc(cpu_env, tmp);
> 
> This means that exceptions due to HVC are going to be
> runtime-detected and cause us to go through and retranslate
> the TB to determine the guest PC. Maybe we should do:
> 
>     /* This helper will raise EXCP_UDEF if HVC is not permitted */
>     gen_helper_hvc_access_check(cpu_env);
>     /* Common case: HVC causes EXCP_HVC */
>     gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16));
> 
> Then you only get the overhead of retranslating the TB in the
> unexpected case where the guest does something dumb and
> executes an HVC that UNDEFs.

That doesn't match my understanding of what will happen with this kind
of exception raising. I think the set_pc_im matters and there won't
be any retranslation of TBs to figure out the guest PC.

Can you double check your thinking here or elaborate a little bit more
so we are on the same page?

Thanks,
Edgar



> 
> > +            tcg_temp_free_i32(tmp);
> > +            break;
> > +        default:
> >              unallocated_encoding(s);
> >              break;
> >          }
> > -        gen_exception_insn(s, 0, EXCP_SWI, syn_aa64_svc(imm16));
> >          break;
> >      case 1:
> >          if (op2_ll != 0) {
> 
> General comment, this is likely to clash with the PSCI support; I
> guess we'll see which gets in first.
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v3 13/16] target-arm: A64: Emulate the HVC insn

[Peter Maydell]

On 4 August 2014 05:12, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Fri, Aug 01, 2014 at 03:21:08PM +0100, Peter Maydell wrote:
>> On 17 June 2014 09:45, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
>> > +        case 2:
>> > +            if (!arm_dc_feature(s, ARM_FEATURE_EL2) || s->current_pl == 0) {
>> > +                unallocated_encoding(s);
>> > +                break;
>> > +            }
>> > +            tmp = tcg_const_i32(syn_aa64_hvc(imm16));
>> > +            gen_a64_set_pc_im(s->pc);
>>
>> (This set_pc_im is unnecessary.)
>>
>> > +            gen_helper_hvc(cpu_env, tmp);
>>
>> This means that exceptions due to HVC are going to be
>> runtime-detected and cause us to go through and retranslate
>> the TB to determine the guest PC. Maybe we should do:
>>
>>     /* This helper will raise EXCP_UDEF if HVC is not permitted */
>>     gen_helper_hvc_access_check(cpu_env);
>>     /* Common case: HVC causes EXCP_HVC */
>>     gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16));
>>
>> Then you only get the overhead of retranslating the TB in the
>> unexpected case where the guest does something dumb and
>> executes an HVC that UNDEFs.
>
> That doesn't match my understanding of what will happen with this kind
> of exception raising. I think the set_pc_im matters and there won't
> be any retranslation of TBs to figure out the guest PC.

Sorry, yes; you're right and I was wrong -- we only retranslate
where we call cpu_restore_state(), which is done only where
tlb_fill() is going to raise an exception.

(I think I need to think a bit about how I'm currently implementing
architectural debug singlestep, since at the moment I assume
that you can tell at translate time whether something is going
to be a valid SMC/HVC/SVC or not, and so whether or not to
advance the singlestep state machine. Maybe I can defer that
to exception entry...)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 13/16] target-arm: A64: Emulate the HVC insn

[Edgar E. Iglesias]

On Mon, Aug 04, 2014 at 03:24:42PM +0100, Peter Maydell wrote:
> On 4 August 2014 05:12, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > On Fri, Aug 01, 2014 at 03:21:08PM +0100, Peter Maydell wrote:
> >> On 17 June 2014 09:45, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> >> > +        case 2:
> >> > +            if (!arm_dc_feature(s, ARM_FEATURE_EL2) || s->current_pl == 0) {
> >> > +                unallocated_encoding(s);
> >> > +                break;
> >> > +            }
> >> > +            tmp = tcg_const_i32(syn_aa64_hvc(imm16));
> >> > +            gen_a64_set_pc_im(s->pc);
> >>
> >> (This set_pc_im is unnecessary.)
> >>
> >> > +            gen_helper_hvc(cpu_env, tmp);
> >>
> >> This means that exceptions due to HVC are going to be
> >> runtime-detected and cause us to go through and retranslate
> >> the TB to determine the guest PC. Maybe we should do:
> >>
> >>     /* This helper will raise EXCP_UDEF if HVC is not permitted */
> >>     gen_helper_hvc_access_check(cpu_env);
> >>     /* Common case: HVC causes EXCP_HVC */
> >>     gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16));
> >>
> >> Then you only get the overhead of retranslating the TB in the
> >> unexpected case where the guest does something dumb and
> >> executes an HVC that UNDEFs.
> >
> > That doesn't match my understanding of what will happen with this kind
> > of exception raising. I think the set_pc_im matters and there won't
> > be any retranslation of TBs to figure out the guest PC.
> 
> Sorry, yes; you're right and I was wrong -- we only retranslate
> where we call cpu_restore_state(), which is done only where
> tlb_fill() is going to raise an exception.
> 
> (I think I need to think a bit about how I'm currently implementing
> architectural debug singlestep, since at the moment I assume
> that you can tell at translate time whether something is going
> to be a valid SMC/HVC/SVC or not, and so whether or not to
> advance the singlestep state machine. Maybe I can defer that
> to exception entry...)

Aha, I see. THere is actually another twist to this code that I
found while testing more. The UDEFs and SMC route to EL2 case should
raise the exception on the SMC/HVC itself, while the success case should
raise it with ELR pointing ahead of the SMC/HVC insn.

My first thought was to fixup the PC in the helper but a split helper
approach might be OK aswell if it helps your debug implementation.

I'll look more at it tomorrow.

Thanks,
Edgar

> 
> thanks
> -- PMM

[Qemu-devel] [PATCH v3 14/16] target-arm: A64: Emulate the SMC insn

[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           |  1 +
 target-arm/helper-a64.c    |  1 +
 target-arm/helper.c        |  6 ++++++
 target-arm/helper.h        |  1 +
 target-arm/internals.h     |  6 ++++++
 target-arm/op_helper.c     | 27 +++++++++++++++++++++++++++
 target-arm/translate-a64.c | 10 ++++++++++
 7 files changed, 52 insertions(+)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 258bc09..42e0ed3 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -52,6 +52,7 @@
 #define EXCP_KERNEL_TRAP     9   /* Jumped to kernel code page.  */
 #define EXCP_STREX          10
 #define EXCP_HVC            11   /* HyperVisor Call */
+#define EXCP_SMC            12   /* Secure Monitor Call */
 
 #define ARMV7M_EXCP_RESET   1
 #define ARMV7M_EXCP_NMI     2
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 7382d0a..f0f33af 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -476,6 +476,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
     case EXCP_UDEF:
     case EXCP_SWI:
     case EXCP_HVC:
+    case EXCP_SMC:
         env->cp15.esr_el[new_el] = env->exception.syndrome;
         break;
     case EXCP_IRQ:
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 44e8d47..4945f67 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3306,6 +3306,12 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
     case EXCP_HVC:
         target_el = MAX(target_el, 2);
         break;
+    case EXCP_SMC:
+        target_el = 3;
+        if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
+            target_el = 2;
+        }
+        break;
     }
     return target_el;
 }
diff --git a/target-arm/helper.h b/target-arm/helper.h
index 70cfd28..4293453 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -51,6 +51,7 @@ DEF_HELPER_3(exception_with_syndrome, void, env, i32, i32)
 DEF_HELPER_1(wfi, void, env)
 DEF_HELPER_1(wfe, void, env)
 DEF_HELPER_2(hvc, void, env, i32)
+DEF_HELPER_2(smc, void, env, i32)
 
 DEF_HELPER_3(cpsr_write, void, env, i32, i32)
 DEF_HELPER_1(cpsr_read, i32, env)
diff --git a/target-arm/internals.h b/target-arm/internals.h
index b68e6f9..f6b7156 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -54,6 +54,7 @@ static const char * const excnames[] = {
     [EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage",
     [EXCP_STREX] = "QEMU intercept of STREX",
     [EXCP_HVC] = "Hypervisor Call",
+    [EXCP_SMC] = "Secure Monitor Call",
 };
 
 static inline void arm_log_exception(int idx)
@@ -210,6 +211,11 @@ static inline uint32_t syn_aa64_hvc(uint16_t imm16)
     return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | imm16;
 }
 
+static inline uint32_t syn_aa64_smc(uint16_t imm16)
+{
+    return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | imm16;
+}
+
 static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb)
 {
     return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0xffff)
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index c1fa797..11636e3 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -402,6 +402,33 @@ void HELPER(hvc)(CPUARMState *env, uint32_t syndrome)
     raise_exception(env, EXCP_HVC);
 }
 
+void HELPER(smc)(CPUARMState *env, uint32_t syndrome)
+{
+    int cur_el = arm_current_pl(env);
+    /* FIXME: Use real secure state.  */
+    bool secure = false;
+    bool smd = env->cp15.scr_el3 & SCR_SMD;
+    /* On ARMv8 AArch32, SMD only applies to NS mode.
+     * On ARMv7 SMD only applies to NS mode and only if EL2 is available.
+     * For ARMv7 non EL2, we force SMD to zero so we don't need to re-check
+     * the EL2 condition here.
+     */
+    bool udef = is_a64(env) ? smd : !secure && smd;
+
+    /* In NS EL1, HCR controlled routing to EL2 has priority over SMD.  */
+    if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
+        udef = false;
+    }
+
+    /* We've already checked that EL3 exists at translation time.  */
+    if (udef) {
+        env->exception.syndrome = syn_uncategorized();
+        raise_exception(env, EXCP_UDEF);
+    }
+    env->exception.syndrome = syndrome;
+    raise_exception(env, EXCP_SMC);
+}
+
 void HELPER(exception_return)(CPUARMState *env)
 {
     int cur_el = arm_current_pl(env);
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 5692dff..dca98168 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1454,6 +1454,16 @@ static void disas_exc(DisasContext *s, uint32_t insn)
             gen_helper_hvc(cpu_env, tmp);
             tcg_temp_free_i32(tmp);
             break;
+        case 3:
+            if (!arm_dc_feature(s, ARM_FEATURE_EL3) || s->current_pl == 0) {
+                unallocated_encoding(s);
+                break;
+            }
+            tmp = tcg_const_i32(syn_aa64_smc(imm16));
+            gen_a64_set_pc_im(s->pc);
+            gen_helper_smc(cpu_env, tmp);
+            tcg_temp_free_i32(tmp);
+            break;
         default:
             unallocated_encoding(s);
             break;
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v3 14/16] target-arm: A64: Emulate the SMC insn

[Greg Bellows]

[-- Attachment #1: Type: text/plain, Size: 5613 bytes --]

Reviewed-by: Greg Bellows <greg.bellows@linaro.org>


On 17 June 2014 03:45, 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           |  1 +
>  target-arm/helper-a64.c    |  1 +
>  target-arm/helper.c        |  6 ++++++
>  target-arm/helper.h        |  1 +
>  target-arm/internals.h     |  6 ++++++
>  target-arm/op_helper.c     | 27 +++++++++++++++++++++++++++
>  target-arm/translate-a64.c | 10 ++++++++++
>  7 files changed, 52 insertions(+)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 258bc09..42e0ed3 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -52,6 +52,7 @@
>  #define EXCP_KERNEL_TRAP     9   /* Jumped to kernel code page.  */
>  #define EXCP_STREX          10
>  #define EXCP_HVC            11   /* HyperVisor Call */
> +#define EXCP_SMC            12   /* Secure Monitor Call */
>
>  #define ARMV7M_EXCP_RESET   1
>  #define ARMV7M_EXCP_NMI     2
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 7382d0a..f0f33af 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -476,6 +476,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>      case EXCP_UDEF:
>      case EXCP_SWI:
>      case EXCP_HVC:
> +    case EXCP_SMC:
>          env->cp15.esr_el[new_el] = env->exception.syndrome;
>          break;
>      case EXCP_IRQ:
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 44e8d47..4945f67 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3306,6 +3306,12 @@ unsigned int arm_excp_target_el(CPUState *cs,
> unsigned int excp_idx)
>      case EXCP_HVC:
>          target_el = MAX(target_el, 2);
>          break;
> +    case EXCP_SMC:
> +        target_el = 3;
> +        if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
> +            target_el = 2;
> +        }
> +        break;
>      }
>      return target_el;
>  }
> diff --git a/target-arm/helper.h b/target-arm/helper.h
> index 70cfd28..4293453 100644
> --- a/target-arm/helper.h
> +++ b/target-arm/helper.h
> @@ -51,6 +51,7 @@ DEF_HELPER_3(exception_with_syndrome, void, env, i32,
> i32)
>  DEF_HELPER_1(wfi, void, env)
>  DEF_HELPER_1(wfe, void, env)
>  DEF_HELPER_2(hvc, void, env, i32)
> +DEF_HELPER_2(smc, void, env, i32)
>
>  DEF_HELPER_3(cpsr_write, void, env, i32, i32)
>  DEF_HELPER_1(cpsr_read, i32, env)
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index b68e6f9..f6b7156 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -54,6 +54,7 @@ static const char * const excnames[] = {
>      [EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage",
>      [EXCP_STREX] = "QEMU intercept of STREX",
>      [EXCP_HVC] = "Hypervisor Call",
> +    [EXCP_SMC] = "Secure Monitor Call",
>  };
>
>  static inline void arm_log_exception(int idx)
> @@ -210,6 +211,11 @@ static inline uint32_t syn_aa64_hvc(uint16_t imm16)
>      return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | imm16;
>  }
>
> +static inline uint32_t syn_aa64_smc(uint16_t imm16)
> +{
> +    return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | imm16;
> +}
> +
>  static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb)
>  {
>      return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0xffff)
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index c1fa797..11636e3 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -402,6 +402,33 @@ void HELPER(hvc)(CPUARMState *env, uint32_t syndrome)
>      raise_exception(env, EXCP_HVC);
>  }
>
> +void HELPER(smc)(CPUARMState *env, uint32_t syndrome)
> +{
> +    int cur_el = arm_current_pl(env);
> +    /* FIXME: Use real secure state.  */
> +    bool secure = false;
> +    bool smd = env->cp15.scr_el3 & SCR_SMD;
> +    /* On ARMv8 AArch32, SMD only applies to NS mode.
> +     * On ARMv7 SMD only applies to NS mode and only if EL2 is available.
> +     * For ARMv7 non EL2, we force SMD to zero so we don't need to
> re-check
> +     * the EL2 condition here.
> +     */
> +    bool udef = is_a64(env) ? smd : !secure && smd;
> +
> +    /* In NS EL1, HCR controlled routing to EL2 has priority over SMD.  */
> +    if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
> +        udef = false;
> +    }
> +
> +    /* We've already checked that EL3 exists at translation time.  */
> +    if (udef) {
> +        env->exception.syndrome = syn_uncategorized();
> +        raise_exception(env, EXCP_UDEF);
> +    }
> +    env->exception.syndrome = syndrome;
> +    raise_exception(env, EXCP_SMC);
> +}
> +
>  void HELPER(exception_return)(CPUARMState *env)
>  {
>      int cur_el = arm_current_pl(env);
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 5692dff..dca98168 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -1454,6 +1454,16 @@ static void disas_exc(DisasContext *s, uint32_t
> insn)
>              gen_helper_hvc(cpu_env, tmp);
>              tcg_temp_free_i32(tmp);
>              break;
> +        case 3:
> +            if (!arm_dc_feature(s, ARM_FEATURE_EL3) || s->current_pl ==
> 0) {
> +                unallocated_encoding(s);
> +                break;
> +            }
> +            tmp = tcg_const_i32(syn_aa64_smc(imm16));
> +            gen_a64_set_pc_im(s->pc);
> +            gen_helper_smc(cpu_env, tmp);
> +            tcg_temp_free_i32(tmp);
> +            break;
>          default:
>              unallocated_encoding(s);
>              break;
> --
> 1.8.3.2
>
>

[-- Attachment #2: Type: text/html, Size: 7066 bytes --]

Re: [Qemu-devel] [PATCH v3 14/16] target-arm: A64: Emulate the SMC insn

[Peter Maydell]

On 17 June 2014 09:45, 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           |  1 +
>  target-arm/helper-a64.c    |  1 +
>  target-arm/helper.c        |  6 ++++++
>  target-arm/helper.h        |  1 +
>  target-arm/internals.h     |  6 ++++++
>  target-arm/op_helper.c     | 27 +++++++++++++++++++++++++++
>  target-arm/translate-a64.c | 10 ++++++++++
>  7 files changed, 52 insertions(+)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 258bc09..42e0ed3 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -52,6 +52,7 @@
>  #define EXCP_KERNEL_TRAP     9   /* Jumped to kernel code page.  */
>  #define EXCP_STREX          10
>  #define EXCP_HVC            11   /* HyperVisor Call */
> +#define EXCP_SMC            12   /* Secure Monitor Call */
>
>  #define ARMV7M_EXCP_RESET   1
>  #define ARMV7M_EXCP_NMI     2
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 7382d0a..f0f33af 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -476,6 +476,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>      case EXCP_UDEF:
>      case EXCP_SWI:
>      case EXCP_HVC:
> +    case EXCP_SMC:
>          env->cp15.esr_el[new_el] = env->exception.syndrome;
>          break;
>      case EXCP_IRQ:
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 44e8d47..4945f67 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3306,6 +3306,12 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
>      case EXCP_HVC:
>          target_el = MAX(target_el, 2);
>          break;
> +    case EXCP_SMC:
> +        target_el = 3;
> +        if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
> +            target_el = 2;
> +        }
> +        break;
>      }
>      return target_el;
>  }
> diff --git a/target-arm/helper.h b/target-arm/helper.h
> index 70cfd28..4293453 100644
> --- a/target-arm/helper.h
> +++ b/target-arm/helper.h
> @@ -51,6 +51,7 @@ DEF_HELPER_3(exception_with_syndrome, void, env, i32, i32)
>  DEF_HELPER_1(wfi, void, env)
>  DEF_HELPER_1(wfe, void, env)
>  DEF_HELPER_2(hvc, void, env, i32)
> +DEF_HELPER_2(smc, void, env, i32)
>
>  DEF_HELPER_3(cpsr_write, void, env, i32, i32)
>  DEF_HELPER_1(cpsr_read, i32, env)
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index b68e6f9..f6b7156 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -54,6 +54,7 @@ static const char * const excnames[] = {
>      [EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage",
>      [EXCP_STREX] = "QEMU intercept of STREX",
>      [EXCP_HVC] = "Hypervisor Call",
> +    [EXCP_SMC] = "Secure Monitor Call",
>  };
>
>  static inline void arm_log_exception(int idx)
> @@ -210,6 +211,11 @@ static inline uint32_t syn_aa64_hvc(uint16_t imm16)
>      return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | imm16;
>  }
>
> +static inline uint32_t syn_aa64_smc(uint16_t imm16)

uint16_t again.

> +{
> +    return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | imm16;
> +}
> +
>  static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb)
>  {
>      return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0xffff)
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index c1fa797..11636e3 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -402,6 +402,33 @@ void HELPER(hvc)(CPUARMState *env, uint32_t syndrome)
>      raise_exception(env, EXCP_HVC);
>  }
>
> +void HELPER(smc)(CPUARMState *env, uint32_t syndrome)
> +{
> +    int cur_el = arm_current_pl(env);
> +    /* FIXME: Use real secure state.  */
> +    bool secure = false;
> +    bool smd = env->cp15.scr_el3 & SCR_SMD;
> +    /* On ARMv8 AArch32, SMD only applies to NS mode.
> +     * On ARMv7 SMD only applies to NS mode and only if EL2 is available.
> +     * For ARMv7 non EL2, we force SMD to zero so we don't need to re-check
> +     * the EL2 condition here.
> +     */
> +    bool udef = is_a64(env) ? smd : !secure && smd;
> +
> +    /* In NS EL1, HCR controlled routing to EL2 has priority over SMD.  */
> +    if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
> +        udef = false;
> +    }
> +
> +    /* We've already checked that EL3 exists at translation time.  */
> +    if (udef) {
> +        env->exception.syndrome = syn_uncategorized();
> +        raise_exception(env, EXCP_UDEF);
> +    }
> +    env->exception.syndrome = syndrome;
> +    raise_exception(env, EXCP_SMC);
> +}
> +
>  void HELPER(exception_return)(CPUARMState *env)
>  {
>      int cur_el = arm_current_pl(env);
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 5692dff..dca98168 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -1454,6 +1454,16 @@ static void disas_exc(DisasContext *s, uint32_t insn)
>              gen_helper_hvc(cpu_env, tmp);
>              tcg_temp_free_i32(tmp);
>              break;
> +        case 3:
> +            if (!arm_dc_feature(s, ARM_FEATURE_EL3) || s->current_pl == 0) {
> +                unallocated_encoding(s);
> +                break;
> +            }
> +            tmp = tcg_const_i32(syn_aa64_smc(imm16));
> +            gen_a64_set_pc_im(s->pc);
> +            gen_helper_smc(cpu_env, tmp);
> +            tcg_temp_free_i32(tmp);
> +            break;
>          default:
>              unallocated_encoding(s);
>              break;

Same remarks apply here as for HVC in previous patch.

thanks
-- PMM

[Qemu-devel] [PATCH v3 15/16] target-arm: Add IRQ and FIQ routing to EL2 and 3

[Edgar E. Iglesias]

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

Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h    | 12 ++++++++++++
 target-arm/helper.c | 13 +++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 42e0ed3..bb123bd 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1136,6 +1136,12 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
     CPUARMState *env = cs->env_ptr;
     unsigned int cur_el = arm_current_pl(env);
     unsigned int target_el = arm_excp_target_el(cs, excp_idx);
+    /* FIXME: Use actual secure state.  */
+    bool secure = false;
+    /* Interrupts can only be hypervised and routed to
+     * EL2 if we are in NS EL0/1.
+     */
+    bool irq_can_hyp = !secure && cur_el < 2 && target_el == 2;
 
     /* Don't take exceptions if they target a lower EL.  */
     if (cur_el > target_el) {
@@ -1144,8 +1150,14 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
 
     switch (excp_idx) {
     case EXCP_FIQ:
+        if (irq_can_hyp && (env->cp15.hcr_el2 & HCR_FMO)) {
+            return true;
+        }
         return !(env->daif & PSTATE_F);
     case EXCP_IRQ:
+        if (irq_can_hyp && (env->cp15.hcr_el2 & HCR_IMO)) {
+            return true;
+        }
         return ((IS_M(env) && env->regs[15] < 0xfffffff0)
                             || !(env->daif & PSTATE_I));
     default:
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 4945f67..3afcbb2 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3312,6 +3312,19 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
             target_el = 2;
         }
         break;
+    case EXCP_FIQ:
+    case EXCP_IRQ: {
+            const uint64_t hcr_mask = excp_idx == EXCP_FIQ ? HCR_FMO : HCR_IMO;
+            const uint32_t scr_mask = excp_idx == EXCP_FIQ ? SCR_FIQ : SCR_IRQ;
+
+            if (!secure && (env->cp15.hcr_el2 & hcr_mask)) {
+                target_el = 2;
+            }
+            if (env->cp15.scr_el3 & scr_mask) {
+                target_el = 3;
+            }
+            break;
+        }
     }
     return target_el;
 }
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v3 15/16] target-arm: Add IRQ and FIQ routing to EL2 and 3

[Peter Maydell]

On 17 June 2014 09:45, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3312,6 +3312,19 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
>              target_el = 2;
>          }
>          break;
> +    case EXCP_FIQ:
> +    case EXCP_IRQ: {

A trivial style nit, but I prefer the { to go on its own line when
opening a new scope for a case statement like this.

> +            const uint64_t hcr_mask = excp_idx == EXCP_FIQ ? HCR_FMO : HCR_IMO;
> +            const uint32_t scr_mask = excp_idx == EXCP_FIQ ? SCR_FIQ : SCR_IRQ;
> +
> +            if (!secure && (env->cp15.hcr_el2 & hcr_mask)) {
> +                target_el = 2;
> +            }
> +            if (env->cp15.scr_el3 & scr_mask) {
> +                target_el = 3;
> +            }
> +            break;
> +        }
>      }
>      return target_el;
>  }

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 15/16] target-arm: Add IRQ and FIQ routing to EL2 and 3

[Edgar E. Iglesias]

On Fri, Aug 01, 2014 at 03:27:44PM +0100, Peter Maydell wrote:
> On 17 June 2014 09:45, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -3312,6 +3312,19 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
> >              target_el = 2;
> >          }
> >          break;
> > +    case EXCP_FIQ:
> > +    case EXCP_IRQ: {
> 
> A trivial style nit, but I prefer the { to go on its own line when
> opening a new scope for a case statement like this.

I've changed this to your prefered style, thanks.


> 
> > +            const uint64_t hcr_mask = excp_idx == EXCP_FIQ ? HCR_FMO : HCR_IMO;
> > +            const uint32_t scr_mask = excp_idx == EXCP_FIQ ? SCR_FIQ : SCR_IRQ;
> > +
> > +            if (!secure && (env->cp15.hcr_el2 & hcr_mask)) {
> > +                target_el = 2;
> > +            }
> > +            if (env->cp15.scr_el3 & scr_mask) {
> > +                target_el = 3;
> > +            }
> > +            break;
> > +        }
> >      }
> >      return target_el;
> >  }
> 
> thanks
> -- PMM

[Qemu-devel] [PATCH v3 16/16] target-arm: Add support for VIRQ and VFIQ

[Edgar E. Iglesias]

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

Acked-by: Greg Bellows <greg.bellows@linaro.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 cpu-exec.c              | 12 ++++++++++++
 target-arm/cpu.c        | 20 ++++++++++++++++++--
 target-arm/cpu.h        | 24 ++++++++++++++++++++++--
 target-arm/helper-a64.c |  2 ++
 target-arm/helper.c     |  4 ++++
 target-arm/internals.h  |  2 ++
 6 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index a579ffc..baf5643 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -498,6 +498,18 @@ int cpu_exec(CPUArchState *env)
                         cc->do_interrupt(cpu);
                         next_tb = 0;
                     }
+                    if (interrupt_request & CPU_INTERRUPT_VIRQ
+                        && arm_excp_unmasked(cpu, EXCP_VIRQ)) {
+                        cpu->exception_index = EXCP_VIRQ;
+                        cc->do_interrupt(cpu);
+                        next_tb = 0;
+                    }
+                    if (interrupt_request & CPU_INTERRUPT_VFIQ
+                        && arm_excp_unmasked(cpu, EXCP_VFIQ)) {
+                        cpu->exception_index = EXCP_VFIQ;
+                        cc->do_interrupt(cpu);
+                        next_tb = 0;
+                    }
 #elif defined(TARGET_UNICORE32)
                     if (interrupt_request & CPU_INTERRUPT_HARD
                         && !(env->uncached_asr & ASR_I)) {
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 8e64c5a..cd7a5df 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -195,6 +195,20 @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)
             cpu_reset_interrupt(cs, CPU_INTERRUPT_FIQ);
         }
         break;
+    case ARM_CPU_VIRQ:
+        if (level) {
+            cpu_interrupt(cs, CPU_INTERRUPT_VIRQ);
+        } else {
+            cpu_reset_interrupt(cs, CPU_INTERRUPT_VIRQ);
+        }
+        break;
+    case ARM_CPU_VFIQ:
+        if (level) {
+            cpu_interrupt(cs, CPU_INTERRUPT_VFIQ);
+        } else {
+            cpu_reset_interrupt(cs, CPU_INTERRUPT_VFIQ);
+        }
+        break;
     default:
         hw_error("arm_cpu_set_irq: Bad interrupt line %d\n", irq);
     }
@@ -242,9 +256,11 @@ static void arm_cpu_initfn(Object *obj)
 #ifndef CONFIG_USER_ONLY
     /* Our inbound IRQ and FIQ lines */
     if (kvm_enabled()) {
-        qdev_init_gpio_in(DEVICE(cpu), arm_cpu_kvm_set_irq, 2);
+        /* VIRQ and VFIQ are unused with KVM but we add them to maintain
+           the same interface as non-KVM CPUs.  */
+        qdev_init_gpio_in(DEVICE(cpu), arm_cpu_kvm_set_irq, 4);
     } else {
-        qdev_init_gpio_in(DEVICE(cpu), arm_cpu_set_irq, 2);
+        qdev_init_gpio_in(DEVICE(cpu), arm_cpu_set_irq, 4);
     }
 
     cpu->gt_timer[GTIMER_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index bb123bd..df61bbd 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -53,6 +53,8 @@
 #define EXCP_STREX          10
 #define EXCP_HVC            11   /* HyperVisor Call */
 #define EXCP_SMC            12   /* Secure Monitor Call */
+#define EXCP_VIRQ           13
+#define EXCP_VFIQ           14
 
 #define ARMV7M_EXCP_RESET   1
 #define ARMV7M_EXCP_NMI     2
@@ -67,6 +69,8 @@
 
 /* ARM-specific interrupt pending bits.  */
 #define CPU_INTERRUPT_FIQ   CPU_INTERRUPT_TGT_EXT_1
+#define CPU_INTERRUPT_VIRQ  CPU_INTERRUPT_TGT_EXT_2
+#define CPU_INTERRUPT_VFIQ  CPU_INTERRUPT_TGT_EXT_3
 
 /* The usual mapping for an AArch64 system register to its AArch32
  * counterpart is for the 32 bit world to have access to the lower
@@ -85,6 +89,9 @@
 /* Meanings of the ARMCPU object's two inbound GPIO lines */
 #define ARM_CPU_IRQ 0
 #define ARM_CPU_FIQ 1
+#define ARM_CPU_VIRQ 2
+#define ARM_CPU_VFIQ 3
+
 
 typedef void ARMWriteCPFunc(void *opaque, int cp_info,
                             int srcreg, int operand, uint32_t value);
@@ -1142,6 +1149,8 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
      * EL2 if we are in NS EL0/1.
      */
     bool irq_can_hyp = !secure && cur_el < 2 && target_el == 2;
+    bool irq_unmasked = ((IS_M(env) && env->regs[15] < 0xfffffff0)
+                          || !(env->daif & PSTATE_I));
 
     /* Don't take exceptions if they target a lower EL.  */
     if (cur_el > target_el) {
@@ -1158,8 +1167,19 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
         if (irq_can_hyp && (env->cp15.hcr_el2 & HCR_IMO)) {
             return true;
         }
-        return ((IS_M(env) && env->regs[15] < 0xfffffff0)
-                            || !(env->daif & PSTATE_I));
+        return irq_unmasked;
+    case EXCP_VFIQ:
+        if (!secure && !(env->cp15.hcr_el2 & HCR_FMO)) {
+            /* VFIQs are only taken when hypervized and non-secure.  */
+            return false;
+        }
+        return !(env->daif & PSTATE_F);
+    case EXCP_VIRQ:
+        if (!secure && !(env->cp15.hcr_el2 & HCR_IMO)) {
+            /* VIRQs are only taken when hypervized and non-secure.  */
+            return false;
+        }
+        return irq_unmasked;
     default:
         g_assert_not_reached();
         break;
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index f0f33af..d7522b6 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -480,9 +480,11 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
         env->cp15.esr_el[new_el] = env->exception.syndrome;
         break;
     case EXCP_IRQ:
+    case EXCP_VIRQ:
         addr += 0x80;
         break;
     case EXCP_FIQ:
+    case EXCP_VFIQ:
         addr += 0x100;
         break;
     default:
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3afcbb2..182306c 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3325,6 +3325,10 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
             }
             break;
         }
+    case EXCP_VIRQ:
+    case EXCP_VFIQ:
+        target_el = 1;
+        break;
     }
     return target_el;
 }
diff --git a/target-arm/internals.h b/target-arm/internals.h
index f6b7156..a57aa7d 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -55,6 +55,8 @@ static const char * const excnames[] = {
     [EXCP_STREX] = "QEMU intercept of STREX",
     [EXCP_HVC] = "Hypervisor Call",
     [EXCP_SMC] = "Secure Monitor Call",
+    [EXCP_VIRQ] = "Virtual IRQ",
+    [EXCP_VFIQ] = "Virtual FIQ",
 };
 
 static inline void arm_log_exception(int idx)
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v3 16/16] target-arm: Add support for VIRQ and VFIQ

[Peter Maydell]

On 17 June 2014 09:45, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Acked-by: Greg Bellows <greg.bellows@linaro.org>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  cpu-exec.c              | 12 ++++++++++++
>  target-arm/cpu.c        | 20 ++++++++++++++++++--
>  target-arm/cpu.h        | 24 ++++++++++++++++++++++--
>  target-arm/helper-a64.c |  2 ++
>  target-arm/helper.c     |  4 ++++
>  target-arm/internals.h  |  2 ++
>  6 files changed, 60 insertions(+), 4 deletions(-)
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index a579ffc..baf5643 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -498,6 +498,18 @@ int cpu_exec(CPUArchState *env)
>                          cc->do_interrupt(cpu);
>                          next_tb = 0;
>                      }
> +                    if (interrupt_request & CPU_INTERRUPT_VIRQ
> +                        && arm_excp_unmasked(cpu, EXCP_VIRQ)) {
> +                        cpu->exception_index = EXCP_VIRQ;
> +                        cc->do_interrupt(cpu);
> +                        next_tb = 0;
> +                    }
> +                    if (interrupt_request & CPU_INTERRUPT_VFIQ
> +                        && arm_excp_unmasked(cpu, EXCP_VFIQ)) {
> +                        cpu->exception_index = EXCP_VFIQ;
> +                        cc->do_interrupt(cpu);
> +                        next_tb = 0;
> +                    }
>  #elif defined(TARGET_UNICORE32)
>                      if (interrupt_request & CPU_INTERRUPT_HARD
>                          && !(env->uncached_asr & ASR_I)) {
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 8e64c5a..cd7a5df 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -195,6 +195,20 @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)
>              cpu_reset_interrupt(cs, CPU_INTERRUPT_FIQ);
>          }
>          break;
> +    case ARM_CPU_VIRQ:
> +        if (level) {
> +            cpu_interrupt(cs, CPU_INTERRUPT_VIRQ);
> +        } else {
> +            cpu_reset_interrupt(cs, CPU_INTERRUPT_VIRQ);
> +        }
> +        break;
> +    case ARM_CPU_VFIQ:
> +        if (level) {
> +            cpu_interrupt(cs, CPU_INTERRUPT_VFIQ);
> +        } else {
> +            cpu_reset_interrupt(cs, CPU_INTERRUPT_VFIQ);
> +        }
> +        break;

With four cases this kind of wants a lookup of irq to CPU_INTERRUPT_
value I think, rather than the code duplication.

>      default:
>          hw_error("arm_cpu_set_irq: Bad interrupt line %d\n", irq);
>      }
> @@ -242,9 +256,11 @@ static void arm_cpu_initfn(Object *obj)
>  #ifndef CONFIG_USER_ONLY
>      /* Our inbound IRQ and FIQ lines */
>      if (kvm_enabled()) {
> -        qdev_init_gpio_in(DEVICE(cpu), arm_cpu_kvm_set_irq, 2);
> +        /* VIRQ and VFIQ are unused with KVM but we add them to maintain
> +           the same interface as non-KVM CPUs.  */

    /* Multiline comments
     * should look like this.
     */

> +        qdev_init_gpio_in(DEVICE(cpu), arm_cpu_kvm_set_irq, 4);
>      } else {
> -        qdev_init_gpio_in(DEVICE(cpu), arm_cpu_set_irq, 2);
> +        qdev_init_gpio_in(DEVICE(cpu), arm_cpu_set_irq, 4);
>      }
>
>      cpu->gt_timer[GTIMER_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index bb123bd..df61bbd 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -53,6 +53,8 @@
>  #define EXCP_STREX          10
>  #define EXCP_HVC            11   /* HyperVisor Call */
>  #define EXCP_SMC            12   /* Secure Monitor Call */
> +#define EXCP_VIRQ           13
> +#define EXCP_VFIQ           14
>
>  #define ARMV7M_EXCP_RESET   1
>  #define ARMV7M_EXCP_NMI     2
> @@ -67,6 +69,8 @@
>
>  /* ARM-specific interrupt pending bits.  */
>  #define CPU_INTERRUPT_FIQ   CPU_INTERRUPT_TGT_EXT_1
> +#define CPU_INTERRUPT_VIRQ  CPU_INTERRUPT_TGT_EXT_2
> +#define CPU_INTERRUPT_VFIQ  CPU_INTERRUPT_TGT_EXT_3
>
>  /* The usual mapping for an AArch64 system register to its AArch32
>   * counterpart is for the 32 bit world to have access to the lower
> @@ -85,6 +89,9 @@
>  /* Meanings of the ARMCPU object's two inbound GPIO lines */

You forgot to update this comment ^^^

>  #define ARM_CPU_IRQ 0
>  #define ARM_CPU_FIQ 1
> +#define ARM_CPU_VIRQ 2
> +#define ARM_CPU_VFIQ 3

>
>  typedef void ARMWriteCPFunc(void *opaque, int cp_info,
>                              int srcreg, int operand, uint32_t value);
> @@ -1142,6 +1149,8 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
>       * EL2 if we are in NS EL0/1.
>       */
>      bool irq_can_hyp = !secure && cur_el < 2 && target_el == 2;
> +    bool irq_unmasked = ((IS_M(env) && env->regs[15] < 0xfffffff0)
> +                          || !(env->daif & PSTATE_I));

The M profile stuff is starting to look increasingly weird here.

>
>      /* Don't take exceptions if they target a lower EL.  */
>      if (cur_el > target_el) {
> @@ -1158,8 +1167,19 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
>          if (irq_can_hyp && (env->cp15.hcr_el2 & HCR_IMO)) {
>              return true;
>          }
> -        return ((IS_M(env) && env->regs[15] < 0xfffffff0)
> -                            || !(env->daif & PSTATE_I));
> +        return irq_unmasked;
> +    case EXCP_VFIQ:
> +        if (!secure && !(env->cp15.hcr_el2 & HCR_FMO)) {
> +            /* VFIQs are only taken when hypervized and non-secure.  */
> +            return false;
> +        }
> +        return !(env->daif & PSTATE_F);
> +    case EXCP_VIRQ:
> +        if (!secure && !(env->cp15.hcr_el2 & HCR_IMO)) {
> +            /* VIRQs are only taken when hypervized and non-secure.  */
> +            return false;
> +        }
> +        return irq_unmasked;
>      default:
>          g_assert_not_reached();
>          break;
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index f0f33af..d7522b6 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -480,9 +480,11 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>          env->cp15.esr_el[new_el] = env->exception.syndrome;
>          break;
>      case EXCP_IRQ:
> +    case EXCP_VIRQ:
>          addr += 0x80;
>          break;
>      case EXCP_FIQ:
> +    case EXCP_VFIQ:
>          addr += 0x100;
>          break;
>      default:
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 3afcbb2..182306c 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3325,6 +3325,10 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
>              }
>              break;
>          }
> +    case EXCP_VIRQ:
> +    case EXCP_VFIQ:
> +        target_el = 1;
> +        break;
>      }
>      return target_el;
>  }
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index f6b7156..a57aa7d 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -55,6 +55,8 @@ static const char * const excnames[] = {
>      [EXCP_STREX] = "QEMU intercept of STREX",
>      [EXCP_HVC] = "Hypervisor Call",
>      [EXCP_SMC] = "Secure Monitor Call",
> +    [EXCP_VIRQ] = "Virtual IRQ",
> +    [EXCP_VFIQ] = "Virtual FIQ",
>  };
>
>  static inline void arm_log_exception(int idx)
> --
> 1.8.3.2

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 16/16] target-arm: Add support for VIRQ and VFIQ

[Edgar E. Iglesias]

On Fri, Aug 01, 2014 at 03:32:59PM +0100, Peter Maydell wrote:
> On 17 June 2014 09:45, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Acked-by: Greg Bellows <greg.bellows@linaro.org>
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  cpu-exec.c              | 12 ++++++++++++
> >  target-arm/cpu.c        | 20 ++++++++++++++++++--
> >  target-arm/cpu.h        | 24 ++++++++++++++++++++++--
> >  target-arm/helper-a64.c |  2 ++
> >  target-arm/helper.c     |  4 ++++
> >  target-arm/internals.h  |  2 ++
> >  6 files changed, 60 insertions(+), 4 deletions(-)
> >
> > diff --git a/cpu-exec.c b/cpu-exec.c
> > index a579ffc..baf5643 100644
> > --- a/cpu-exec.c
> > +++ b/cpu-exec.c
> > @@ -498,6 +498,18 @@ int cpu_exec(CPUArchState *env)
> >                          cc->do_interrupt(cpu);
> >                          next_tb = 0;
> >                      }
> > +                    if (interrupt_request & CPU_INTERRUPT_VIRQ
> > +                        && arm_excp_unmasked(cpu, EXCP_VIRQ)) {
> > +                        cpu->exception_index = EXCP_VIRQ;
> > +                        cc->do_interrupt(cpu);
> > +                        next_tb = 0;
> > +                    }
> > +                    if (interrupt_request & CPU_INTERRUPT_VFIQ
> > +                        && arm_excp_unmasked(cpu, EXCP_VFIQ)) {
> > +                        cpu->exception_index = EXCP_VFIQ;
> > +                        cc->do_interrupt(cpu);
> > +                        next_tb = 0;
> > +                    }
> >  #elif defined(TARGET_UNICORE32)
> >                      if (interrupt_request & CPU_INTERRUPT_HARD
> >                          && !(env->uncached_asr & ASR_I)) {
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index 8e64c5a..cd7a5df 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -195,6 +195,20 @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)
> >              cpu_reset_interrupt(cs, CPU_INTERRUPT_FIQ);
> >          }
> >          break;
> > +    case ARM_CPU_VIRQ:
> > +        if (level) {
> > +            cpu_interrupt(cs, CPU_INTERRUPT_VIRQ);
> > +        } else {
> > +            cpu_reset_interrupt(cs, CPU_INTERRUPT_VIRQ);
> > +        }
> > +        break;
> > +    case ARM_CPU_VFIQ:
> > +        if (level) {
> > +            cpu_interrupt(cs, CPU_INTERRUPT_VFIQ);
> > +        } else {
> > +            cpu_reset_interrupt(cs, CPU_INTERRUPT_VFIQ);
> > +        }
> > +        break;
> 
> With four cases this kind of wants a lookup of irq to CPU_INTERRUPT_
> value I think, rather than the code duplication.

Added a table lookup, thanks.

> 
> >      default:
> >          hw_error("arm_cpu_set_irq: Bad interrupt line %d\n", irq);
> >      }
> > @@ -242,9 +256,11 @@ static void arm_cpu_initfn(Object *obj)
> >  #ifndef CONFIG_USER_ONLY
> >      /* Our inbound IRQ and FIQ lines */
> >      if (kvm_enabled()) {
> > -        qdev_init_gpio_in(DEVICE(cpu), arm_cpu_kvm_set_irq, 2);
> > +        /* VIRQ and VFIQ are unused with KVM but we add them to maintain
> > +           the same interface as non-KVM CPUs.  */
> 
>     /* Multiline comments
>      * should look like this.
>      */


Fixed

> 
> > +        qdev_init_gpio_in(DEVICE(cpu), arm_cpu_kvm_set_irq, 4);
> >      } else {
> > -        qdev_init_gpio_in(DEVICE(cpu), arm_cpu_set_irq, 2);
> > +        qdev_init_gpio_in(DEVICE(cpu), arm_cpu_set_irq, 4);
> >      }
> >
> >      cpu->gt_timer[GTIMER_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index bb123bd..df61bbd 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -53,6 +53,8 @@
> >  #define EXCP_STREX          10
> >  #define EXCP_HVC            11   /* HyperVisor Call */
> >  #define EXCP_SMC            12   /* Secure Monitor Call */
> > +#define EXCP_VIRQ           13
> > +#define EXCP_VFIQ           14
> >
> >  #define ARMV7M_EXCP_RESET   1
> >  #define ARMV7M_EXCP_NMI     2
> > @@ -67,6 +69,8 @@
> >
> >  /* ARM-specific interrupt pending bits.  */
> >  #define CPU_INTERRUPT_FIQ   CPU_INTERRUPT_TGT_EXT_1
> > +#define CPU_INTERRUPT_VIRQ  CPU_INTERRUPT_TGT_EXT_2
> > +#define CPU_INTERRUPT_VFIQ  CPU_INTERRUPT_TGT_EXT_3
> >
> >  /* The usual mapping for an AArch64 system register to its AArch32
> >   * counterpart is for the 32 bit world to have access to the lower
> > @@ -85,6 +89,9 @@
> >  /* Meanings of the ARMCPU object's two inbound GPIO lines */
> 
> You forgot to update this comment ^^^

Done


> 
> >  #define ARM_CPU_IRQ 0
> >  #define ARM_CPU_FIQ 1
> > +#define ARM_CPU_VIRQ 2
> > +#define ARM_CPU_VFIQ 3
> 
> >
> >  typedef void ARMWriteCPFunc(void *opaque, int cp_info,
> >                              int srcreg, int operand, uint32_t value);
> > @@ -1142,6 +1149,8 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
> >       * EL2 if we are in NS EL0/1.
> >       */
> >      bool irq_can_hyp = !secure && cur_el < 2 && target_el == 2;
> > +    bool irq_unmasked = ((IS_M(env) && env->regs[15] < 0xfffffff0)
> > +                          || !(env->daif & PSTATE_I));
> 
> The M profile stuff is starting to look increasingly weird here.

Yes, not sure what to do about it though. I forgot to move the
comment in cpu-exec to here, will do that.

Cheers,
Edgar

Re: [Qemu-devel] [PATCH v3 00/16] target-arm: Parts of the AArch64 EL2/3 exception model

[Greg Bellows]

[-- Attachment #1: Type: text/plain, Size: 3065 bytes --]

I didn't get "[PATCH v3 13/16] target-arm: A64: Emulate the HVC insn" in
email, so I reviewed it in patchwork.  No apparent changes from v2.

Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
 [v3,13/16] target-arm: A64: Emulate the HVC insn


On 17 June 2014 03:45, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:

> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Hi,
>
> This is a second round of AArch64 EL2/3 patches working on the exception
> model. Among other things adding HVC/SMC, interrupt routing to EL2/3 and
> Virtual IRQs/FIQs. The VIRQ/VFIQ support only adds the external signal
> delivery method.
>
> Patch 3 is a bug fix.
> Patch 14 fails checkpatch, seems like a bug in checkpatch, CC:d Blue.
>
> This conflicts slightly with the PSCI emulation patches that Rob posted.
> A rebase should be trivial, hooking in the PSCI emulation calls in the
> HVC/SMC helpers.
>
> Cheers,
> Edgar
>
> v2 -> v3:
> * Add more HCR bitfield macros
> * Flush TLB on hcr_write change of HCR RW, DC and PTW.
> * Fix hvc helper, HVC is undefined in secure mode.
> * Remove uint16_t imm16 syndrome gen change.
> * Replace c1_scr with scr_el3
>
> v1 -> v2:
> * Avoid imm16 mask in syndrome generation
> * Use g_assert_not_reached() in arm_excp_unmasked()
> * Avoid some logic duplication in arm_excp_target_el and arm_excp_unmasked.
> * Put arm_excp_target_el in helper.c to start with.
> * Fix SMC disable (SMD or SCD) for ARMv7 only applies if EL2 exists
> * SCR_RES0_MASK -> SCR_MASK
> * HCR_RES0_MASK -> HCR_MASK
> * Fix SMC routing to EL2, only applies for NS EL1.
> * Fix CPreg defs for ESR_EL2/3
> * Fix SMC helper, SMC routing to EL2 and SCR.SMD for AArch32.
>
> Edgar E. Iglesias (16):
>   target-arm: A64: Break out aarch64_save/restore_sp
>   target-arm: A64: Respect SPSEL in ERET SP restore
>   target-arm: A64: Respect SPSEL when taking exceptions
>   target-arm: Make far_el1 an array
>   target-arm: Add ESR_EL2 and 3
>   target-arm: Add FAR_EL2 and 3
>   target-arm: Add HCR_EL2
>   target-arm: Add SCR_EL3
>   target-arm: A64: Refactor aarch64_cpu_do_interrupt
>   target-arm: Break out exception masking to a separate func
>   target-arm: Don't take interrupts targeting lower ELs
>   target-arm: A64: Correct updates to FAR and ESR on exceptions
>   target-arm: A64: Emulate the HVC insn
>   target-arm: A64: Emulate the SMC insn
>   target-arm: Add IRQ and FIQ routing to EL2 and 3
>   target-arm: Add support for VIRQ and VFIQ
>
>  cpu-exec.c                 |  17 +++++-
>  target-arm/cpu.c           |  22 ++++++-
>  target-arm/cpu.h           | 120 ++++++++++++++++++++++++++++++++++++-
>  target-arm/helper-a64.c    |  32 +++++-----
>  target-arm/helper.c        | 145
> ++++++++++++++++++++++++++++++++++++++++++---
>  target-arm/helper.h        |   2 +
>  target-arm/internals.h     |  43 +++++++++++---
>  target-arm/kvm64.c         |  13 +---
>  target-arm/op_helper.c     |  68 +++++++++++++++++++--
>  target-arm/translate-a64.c |  31 ++++++++--
>  10 files changed, 433 insertions(+), 60 deletions(-)
>
> --
> 1.8.3.2
>
>

[-- Attachment #2: Type: text/html, Size: 4057 bytes --]

Re: [Qemu-devel] [PATCH v3 00/16] target-arm: Parts of the AArch64 EL2/3 exception model

[Edgar E. Iglesias]

Ping!

PMM, are you expecting me to change something with this series that I've
missed or is it just busy times for review?

Cheers,
Edgar

On Tue, Jun 17, 2014 at 06:45:30PM +1000, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Hi,
> 
> This is a second round of AArch64 EL2/3 patches working on the exception
> model. Among other things adding HVC/SMC, interrupt routing to EL2/3 and
> Virtual IRQs/FIQs. The VIRQ/VFIQ support only adds the external signal
> delivery method.
> 
> Patch 3 is a bug fix.
> Patch 14 fails checkpatch, seems like a bug in checkpatch, CC:d Blue.
> 
> This conflicts slightly with the PSCI emulation patches that Rob posted.
> A rebase should be trivial, hooking in the PSCI emulation calls in the
> HVC/SMC helpers.
> 
> Cheers,
> Edgar
> 
> v2 -> v3:
> * Add more HCR bitfield macros
> * Flush TLB on hcr_write change of HCR RW, DC and PTW.
> * Fix hvc helper, HVC is undefined in secure mode.
> * Remove uint16_t imm16 syndrome gen change.
> * Replace c1_scr with scr_el3
> 
> v1 -> v2:
> * Avoid imm16 mask in syndrome generation
> * Use g_assert_not_reached() in arm_excp_unmasked()
> * Avoid some logic duplication in arm_excp_target_el and arm_excp_unmasked.
> * Put arm_excp_target_el in helper.c to start with.
> * Fix SMC disable (SMD or SCD) for ARMv7 only applies if EL2 exists
> * SCR_RES0_MASK -> SCR_MASK
> * HCR_RES0_MASK -> HCR_MASK
> * Fix SMC routing to EL2, only applies for NS EL1.
> * Fix CPreg defs for ESR_EL2/3
> * Fix SMC helper, SMC routing to EL2 and SCR.SMD for AArch32.
> 
> Edgar E. Iglesias (16):
>   target-arm: A64: Break out aarch64_save/restore_sp
>   target-arm: A64: Respect SPSEL in ERET SP restore
>   target-arm: A64: Respect SPSEL when taking exceptions
>   target-arm: Make far_el1 an array
>   target-arm: Add ESR_EL2 and 3
>   target-arm: Add FAR_EL2 and 3
>   target-arm: Add HCR_EL2
>   target-arm: Add SCR_EL3
>   target-arm: A64: Refactor aarch64_cpu_do_interrupt
>   target-arm: Break out exception masking to a separate func
>   target-arm: Don't take interrupts targeting lower ELs
>   target-arm: A64: Correct updates to FAR and ESR on exceptions
>   target-arm: A64: Emulate the HVC insn
>   target-arm: A64: Emulate the SMC insn
>   target-arm: Add IRQ and FIQ routing to EL2 and 3
>   target-arm: Add support for VIRQ and VFIQ
> 
>  cpu-exec.c                 |  17 +++++-
>  target-arm/cpu.c           |  22 ++++++-
>  target-arm/cpu.h           | 120 ++++++++++++++++++++++++++++++++++++-
>  target-arm/helper-a64.c    |  32 +++++-----
>  target-arm/helper.c        | 145 ++++++++++++++++++++++++++++++++++++++++++---
>  target-arm/helper.h        |   2 +
>  target-arm/internals.h     |  43 +++++++++++---
>  target-arm/kvm64.c         |  13 +---
>  target-arm/op_helper.c     |  68 +++++++++++++++++++--
>  target-arm/translate-a64.c |  31 ++++++++--
>  10 files changed, 433 insertions(+), 60 deletions(-)
> 
> -- 
> 1.8.3.2
> 

Re: [Qemu-devel] [PATCH v3 00/16] target-arm: Parts of the AArch64 EL2/3 exception model

[Peter Maydell]

On 11 July 2014 00:17, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> Ping!
>
> PMM, are you expecting me to change something with this series that I've
> missed or is it just busy times for review?

It's on my review list but since it's not for 2.1 it's a lower
priority right now...

thanks
-- PMM