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

[Qemu-devel] [PATCH v6 00/10] 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.

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 code.

Cheers,
Edgar

v5 -> v6:
* Another try at SCR RES0/RES1
* Make SCR RES0 masks more readable
* Wake CPUs that receive VIRQs/VFIQs
* Make SCR an uint64_t
* Rebase with the single-step support. Split HVC/SMC to use a pre hvc/smc
  handler for cases where the exception is raised before advancing PC.

v4 -> v5:
* Fix RES0/1 masks for SCR
* Use scr_write for aarch32 SCR
* Revert the move of aarch32 SCR to the el3 cpreg defs to avoid breakage
  of existing code relying on SCR existing in current 32bit CPUs.
  The 32bit TZ series will need to address this and enable EL3 for 32bit CPUs.

v3 -> v4:
* Coding style changes.
* Add access spec for v8_el3_no_el2_cp_reginfo.HCR_EL2.
* Move SCR to the el3 cpreg defs and add NO_MIGRATE to SCR_EL3.
* Correct HCR.HCD and HCR.TSC RES0 behaviour.
* Comment on hcr_write TLB flush.
* Use uint32_t with explicit masking for imm16 in syndrome generator.
* Add table lookup of interrupt masks in arm_cpu_set_irq.
* Move M profile irq handling comment from cpu-exec.c to cpu.h.
* Correct trap address for disabled HVC/SMD and for SMC routed to EL2.

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 (10):
  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           |  29 ++++++----
 target-arm/cpu.h           | 130 +++++++++++++++++++++++++++++++++++++++++++-
 target-arm/helper-a64.c    |  31 ++++++-----
 target-arm/helper.c        | 132 ++++++++++++++++++++++++++++++++++++++++++++-
 target-arm/helper.h        |   2 +
 target-arm/internals.h     |  14 +++++
 target-arm/op_helper.c     |  58 ++++++++++++++++++++
 target-arm/translate-a64.c |  43 +++++++++++----
 9 files changed, 416 insertions(+), 40 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v6 01/10] target-arm: Add HCR_EL2

[Edgar E. Iglesias]

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

Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h    | 36 ++++++++++++++++++++++++++++++++++++
 target-arm/helper.c | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index d1e1ccb..36507f9 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.  */
@@ -565,6 +566,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 ece9673..40b0c5f 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2230,10 +2230,44 @@ 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,
+      .access = PL2_RW,
+      .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;
+    } else {
+        valid_mask &= ~HCR_TSC;
+    }
+
+    /* Clear RES0 bits.  */
+    value &= valid_mask;
+
+    /* These bits change the MMU setup:
+     * HCR_VM enables stage 2 translation
+     * HCR_PTW forbids certain page-table setups
+     * HCR_DC Disables stage1 and enables stage2 translation
+     */
+    if ((raw_read(env, ri) ^ value) & (HCR_VM | 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.9.1

[Qemu-devel] [PATCH v6 02/10] 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    | 19 ++++++++++++++++++-
 target-arm/helper.c | 35 +++++++++++++++++++++++++++++++++--
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 36507f9..c69d471 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 */
+        uint64_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.  */
@@ -601,6 +601,23 @@ 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_FW                (1U << 4)
+#define SCR_AW                (1U << 5)
+#define SCR_NET               (1U << 6)
+#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_AARCH32_MASK      (0x3fff & ~(SCR_RW | SCR_ST))
+#define SCR_AARCH64_MASK      (0x3fff & ~SCR_NET)
+
 /* 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 40b0c5f..8d0e056 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -747,6 +747,32 @@ static void vbar_write(CPUARMState *env, const ARMCPRegInfo *ri,
     raw_write(env, ri, value & ~0x1FULL);
 }
 
+static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
+{
+    /* We only mask off bits that are RES0 both for AArch64 and AArch32.
+     * For bits that vary between AArch32/64, code needs to check the
+     * current execution mode before directly using the feature bit.
+     */
+    uint32_t valid_mask = SCR_AARCH64_MASK | SCR_AARCH32_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;
+        }
+    }
+
+    /* Clear all-context RES0 bits.  */
+    value &= valid_mask;
+    raw_write(env, ri, value);
+}
+
 static uint64_t ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     ARMCPU *cpu = arm_env_get_cpu(env);
@@ -873,8 +899,8 @@ 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),
-      .resetvalue = 0, },
+      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3),
+      .resetvalue = 0, .writefn = scr_write },
     { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0,
       .access = PL1_R, .readfn = ccsidr_read, .type = ARM_CP_NO_MIGRATE },
@@ -2314,6 +2340,11 @@ 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,
+      .type = ARM_CP_NO_MIGRATE,
+      .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.9.1

[Qemu-devel] [PATCH v6 03/10] 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.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
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 c69d471..e2474d0 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -499,6 +499,12 @@ void pmccntr_sync(CPUARMState *env);
 #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.
@@ -754,6 +760,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 8d0e056..4431fbb 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3587,6 +3587,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.  */
@@ -3642,6 +3647,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.9.1

[Qemu-devel] [PATCH v6 04/10] 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 | 15 +++++++++++++++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index bd93165..d017588 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -596,7 +596,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;
@@ -611,8 +611,7 @@ int cpu_exec(CPUArchState *env)
                        We avoid this by disabling interrupts when
                        pc contains a magic address.  */
                     if (interrupt_request & CPU_INTERRUPT_HARD
-                        && !(env->daif & PSTATE_I)
-                        && (!IS_M(env) || env->regs[15] < 0xfffffff0)) {
+                        && 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 e2474d0..a5e8e0d 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1171,6 +1171,21 @@ 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 !(env->daif & PSTATE_I)
+               && (!IS_M(env) || env->regs[15] < 0xfffffff0);
+    default:
+        g_assert_not_reached();
+    }
+}
+
 static inline CPUARMState *cpu_init(const char *cpu_model)
 {
     ARMCPU *cpu = cpu_arm_init(cpu_model);
-- 
1.9.1

[Qemu-devel] [PATCH v6 05/10] 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>
Reviewed-by: Peter Maydell <peter.maydell@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 a5e8e0d..7f8a410 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1174,6 +1174,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.9.1

[Qemu-devel] [PATCH v6 06/10] 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 | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 4be0784..c6ef8e9 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -466,18 +466,17 @@ 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;
+        /* fall through */
     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.9.1

[Qemu-devel] [PATCH v6 07/10] 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     | 32 ++++++++++++++++++++++++++++++++
 target-arm/translate-a64.c | 30 +++++++++++++++++++++---------
 7 files changed, 89 insertions(+), 10 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 7f8a410..b553f3d 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 c6ef8e9..4e6ca26 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_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 4431fbb..504ff42 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3652,7 +3652,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 1d7003b..75fc1b3 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_1(pre_hvc, void, env)
 
 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 64751a0..afcc4e0 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)
@@ -215,6 +216,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(uint32_t imm16)
+{
+    return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff);
+}
+
 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 b956216..8441c27 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -374,6 +374,38 @@ void HELPER(clear_pstate_ss)(CPUARMState *env)
     env->pstate &= ~PSTATE_SS;
 }
 
+void HELPER(pre_hvc)(CPUARMState *env)
+{
+    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) {
+        /* UDEFs trap on the HVC, roll back to the PC to the HVC insn.  */
+        env->exception.syndrome = syn_uncategorized();
+        raise_exception(env, EXCP_UDEF);
+    }
+}
+
 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 8e66b6c..8eaa85f 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1473,20 +1473,32 @@ static void disas_exc(DisasContext *s, uint32_t insn)
 
     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) {
-            unallocated_encoding(s);
-            break;
-        }
         /* For SVC, HVC and SMC we advance the single-step state
          * machine before taking the exception. This is architecturally
          * mandated, to ensure that single-stepping a system call
          * instruction works properly.
          */
-        gen_ss_advance(s);
-        gen_exception_insn(s, 0, EXCP_SWI, syn_aa64_svc(imm16));
+        switch (op2_ll) {
+        case 1:
+            gen_ss_advance(s);
+            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;
+            }
+            /* The pre HVC helper handles cases when HVC gets trapped
+             * as an undefined insn by runtime configuration.  */
+            gen_a64_set_pc_im(s->pc - 4);
+            gen_helper_pre_hvc(cpu_env);
+            gen_ss_advance(s);
+            gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16));
+            break;
+        default:
+            unallocated_encoding(s);
+            break;
+        }
         break;
     case 1:
         if (op2_ll != 0) {
-- 
1.9.1

[Qemu-devel] [PATCH v6 08/10] 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     | 26 ++++++++++++++++++++++++++
 target-arm/translate-a64.c | 13 +++++++++++++
 7 files changed, 54 insertions(+)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index b553f3d..c24af40 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 4e6ca26..996dfea 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -477,6 +477,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 504ff42..719c95d 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3677,6 +3677,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 75fc1b3..dec3728 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_1(pre_hvc, void, env)
+DEF_HELPER_2(pre_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 afcc4e0..e15ae57 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)
@@ -221,6 +222,11 @@ static inline uint32_t syn_aa64_hvc(uint32_t imm16)
     return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff);
 }
 
+static inline uint32_t syn_aa64_smc(uint32_t imm16)
+{
+    return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff);
+}
+
 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 8441c27..e6a0656 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -406,6 +406,32 @@ void HELPER(pre_hvc)(CPUARMState *env)
     }
 }
 
+void HELPER(pre_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)) {
+        env->exception.syndrome = syndrome;
+        raise_exception(env, EXCP_SMC);
+    }
+
+    /* We've already checked that EL3 exists at translation time.  */
+    if (udef) {
+        env->exception.syndrome = syn_uncategorized();
+        raise_exception(env, EXCP_UDEF);
+    }
+}
+
 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 8eaa85f..9a58de8 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1470,6 +1470,7 @@ 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:
@@ -1495,6 +1496,18 @@ static void disas_exc(DisasContext *s, uint32_t insn)
             gen_ss_advance(s);
             gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16));
             break;
+        case 3:
+            if (!arm_dc_feature(s, ARM_FEATURE_EL3) || s->current_pl == 0) {
+                unallocated_encoding(s);
+                break;
+            }
+            gen_a64_set_pc_im(s->pc - 4);
+            tmp = tcg_const_i32(syn_aa64_smc(imm16));
+            gen_helper_pre_smc(cpu_env, tmp);
+            tcg_temp_free_i32(tmp);
+            gen_ss_advance(s);
+            gen_exception_insn(s, 0, EXCP_SMC, syn_aa64_smc(imm16));
+            break;
         default:
             unallocated_encoding(s);
             break;
-- 
1.9.1

[Qemu-devel] [PATCH v6 09/10] 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 | 14 ++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index c24af40..a5123f8 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1178,6 +1178,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) {
@@ -1186,8 +1192,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 !(env->daif & PSTATE_I)
                && (!IS_M(env) || env->regs[15] < 0xfffffff0);
     default:
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 719c95d..3a9d1fc 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3683,6 +3683,20 @@ 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.9.1

[Qemu-devel] [PATCH v6 10/10] 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        | 29 ++++++++++++++++++-----------
 target-arm/cpu.h        | 36 +++++++++++++++++++++++++++++++++---
 target-arm/helper-a64.c |  2 ++
 target-arm/helper.c     |  4 ++++
 target-arm/internals.h  |  2 ++
 6 files changed, 71 insertions(+), 14 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index d017588..6203ba5 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -616,6 +616,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 7ea12bd..d7adcb2 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -41,7 +41,9 @@ static void arm_cpu_set_pc(CPUState *cs, vaddr value)
 static bool arm_cpu_has_work(CPUState *cs)
 {
     return cs->interrupt_request &
-        (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_EXITTB);
+        (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD
+         | CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ
+         | CPU_INTERRUPT_EXITTB);
 }
 
 static void cp_reg_reset(gpointer key, gpointer value, gpointer opaque)
@@ -193,20 +195,22 @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)
 {
     ARMCPU *cpu = opaque;
     CPUState *cs = CPU(cpu);
+    static const int mask[] = {
+        [ARM_CPU_IRQ] = CPU_INTERRUPT_HARD,
+        [ARM_CPU_FIQ] = CPU_INTERRUPT_FIQ,
+        [ARM_CPU_VIRQ] = CPU_INTERRUPT_VIRQ,
+        [ARM_CPU_VFIQ] = CPU_INTERRUPT_VFIQ
+    };
 
     switch (irq) {
     case ARM_CPU_IRQ:
-        if (level) {
-            cpu_interrupt(cs, CPU_INTERRUPT_HARD);
-        } else {
-            cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
-        }
-        break;
     case ARM_CPU_FIQ:
+    case ARM_CPU_VIRQ:
+    case ARM_CPU_VFIQ:
         if (level) {
-            cpu_interrupt(cs, CPU_INTERRUPT_FIQ);
+            cpu_interrupt(cs, mask[irq]);
         } else {
-            cpu_reset_interrupt(cs, CPU_INTERRUPT_FIQ);
+            cpu_reset_interrupt(cs, mask[irq]);
         }
         break;
     default:
@@ -256,9 +260,12 @@ 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 a5123f8..0333de1 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
@@ -82,9 +86,12 @@
 #define offsetofhigh32(S, M) (offsetof(S, M) + sizeof(uint32_t))
 #endif
 
-/* Meanings of the ARMCPU object's two inbound GPIO lines */
+/* Meanings of the ARMCPU object's four 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);
@@ -1184,6 +1191,18 @@ 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;
+    /* ARMv7-M interrupt return works by loading a magic value
+     * into the PC.  On real hardware the load causes the
+     * return to occur.  The qemu implementation performs the
+     * jump normally, then does the exception return when the
+     * CPU tries to execute code at the magic address.
+     * This will cause the magic PC value to be pushed to
+     * the stack if an interrupt occurred at the wrong time.
+     * We avoid this by disabling interrupts when
+     * pc contains a magic address.
+     */
+    bool irq_unmasked = !(env->daif & PSTATE_I)
+                        && (!IS_M(env) || env->regs[15] < 0xfffffff0);
 
     /* Don't take exceptions if they target a lower EL.  */
     if (cur_el > target_el) {
@@ -1200,8 +1219,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 !(env->daif & PSTATE_I)
-               && (!IS_M(env) || env->regs[15] < 0xfffffff0);
+        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();
     }
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 996dfea..bd16fe3 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -481,9 +481,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 3a9d1fc..2f7b6e6 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3697,6 +3697,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 e15ae57..1e98102 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.9.1

Re: [Qemu-devel] [PATCH v6 02/10] target-arm: Add SCR_EL3

[Greg Bellows]

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

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

On 12 September 2014 21:29, 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    | 19 ++++++++++++++++++-
>  target-arm/helper.c | 35 +++++++++++++++++++++++++++++++++--
>  2 files changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 36507f9..c69d471 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 */
> +        uint64_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.  */
> @@ -601,6 +601,23 @@ 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_FW                (1U << 4)
> +#define SCR_AW                (1U << 5)
> +#define SCR_NET               (1U << 6)
> +#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_AARCH32_MASK      (0x3fff & ~(SCR_RW | SCR_ST))
> +#define SCR_AARCH64_MASK      (0x3fff & ~SCR_NET)
> +
>  /* 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 40b0c5f..8d0e056 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -747,6 +747,32 @@ static void vbar_write(CPUARMState *env, const
> ARMCPRegInfo *ri,
>      raw_write(env, ri, value & ~0x1FULL);
>  }
>
> +static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t
> value)
> +{
> +    /* We only mask off bits that are RES0 both for AArch64 and AArch32.
> +     * For bits that vary between AArch32/64, code needs to check the
> +     * current execution mode before directly using the feature bit.
> +     */
> +    uint32_t valid_mask = SCR_AARCH64_MASK | SCR_AARCH32_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;
> +        }
> +    }
> +
> +    /* Clear all-context RES0 bits.  */
> +    value &= valid_mask;
> +    raw_write(env, ri, value);
> +}
> +
>  static uint64_t ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
>  {
>      ARMCPU *cpu = arm_env_get_cpu(env);
> @@ -873,8 +899,8 @@ 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),
> -      .resetvalue = 0, },
> +      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState,
> cp15.scr_el3),
> +      .resetvalue = 0, .writefn = scr_write },
>      { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH,
>        .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0,
>        .access = PL1_R, .readfn = ccsidr_read, .type = ARM_CP_NO_MIGRATE },
> @@ -2314,6 +2340,11 @@ 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,
> +      .type = ARM_CP_NO_MIGRATE,
> +      .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.9.1
>
>

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

Re: [Qemu-devel] [PATCH v6 07/10] target-arm: A64: Emulate the HVC insn

[Greg Bellows]

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

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

On 12 September 2014 21:29, 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     | 32 ++++++++++++++++++++++++++++++++
>  target-arm/translate-a64.c | 30 +++++++++++++++++++++---------
>  7 files changed, 89 insertions(+), 10 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 7f8a410..b553f3d 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 c6ef8e9..4e6ca26 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_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 4431fbb..504ff42 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3652,7 +3652,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 1d7003b..75fc1b3 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_1(pre_hvc, void, env)
>
>  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 64751a0..afcc4e0 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)
> @@ -215,6 +216,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(uint32_t imm16)
> +{
> +    return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 &
> 0xffff);
> +}
> +
>  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 b956216..8441c27 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -374,6 +374,38 @@ void HELPER(clear_pstate_ss)(CPUARMState *env)
>      env->pstate &= ~PSTATE_SS;
>  }
>
> +void HELPER(pre_hvc)(CPUARMState *env)
> +{
> +    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) {
> +        /* UDEFs trap on the HVC, roll back to the PC to the HVC insn.  */
> +        env->exception.syndrome = syn_uncategorized();
> +        raise_exception(env, EXCP_UDEF);
> +    }
> +}
> +
>  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 8e66b6c..8eaa85f 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -1473,20 +1473,32 @@ static void disas_exc(DisasContext *s, uint32_t
> insn)
>
>      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) {
> -            unallocated_encoding(s);
> -            break;
> -        }
>          /* For SVC, HVC and SMC we advance the single-step state
>           * machine before taking the exception. This is architecturally
>           * mandated, to ensure that single-stepping a system call
>           * instruction works properly.
>           */
> -        gen_ss_advance(s);
> -        gen_exception_insn(s, 0, EXCP_SWI, syn_aa64_svc(imm16));
> +        switch (op2_ll) {
> +        case 1:
> +            gen_ss_advance(s);
> +            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;
> +            }
> +            /* The pre HVC helper handles cases when HVC gets trapped
> +             * as an undefined insn by runtime configuration.  */
> +            gen_a64_set_pc_im(s->pc - 4);
> +            gen_helper_pre_hvc(cpu_env);
> +            gen_ss_advance(s);
> +            gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16));
> +            break;
> +        default:
> +            unallocated_encoding(s);
> +            break;
> +        }
>          break;
>      case 1:
>          if (op2_ll != 0) {
> --
> 1.9.1
>
>

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

Re: [Qemu-devel] [PATCH v6 08/10] target-arm: A64: Emulate the SMC insn

[Greg Bellows]

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

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

On 12 September 2014 21:29, 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     | 26 ++++++++++++++++++++++++++
>  target-arm/translate-a64.c | 13 +++++++++++++
>  7 files changed, 54 insertions(+)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index b553f3d..c24af40 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 4e6ca26..996dfea 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -477,6 +477,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 504ff42..719c95d 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3677,6 +3677,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 75fc1b3..dec3728 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_1(pre_hvc, void, env)
> +DEF_HELPER_2(pre_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 afcc4e0..e15ae57 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)
> @@ -221,6 +222,11 @@ static inline uint32_t syn_aa64_hvc(uint32_t imm16)
>      return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 &
> 0xffff);
>  }
>
> +static inline uint32_t syn_aa64_smc(uint32_t imm16)
> +{
> +    return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 &
> 0xffff);
> +}
> +
>  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 8441c27..e6a0656 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -406,6 +406,32 @@ void HELPER(pre_hvc)(CPUARMState *env)
>      }
>  }
>
> +void HELPER(pre_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)) {
> +        env->exception.syndrome = syndrome;
> +        raise_exception(env, EXCP_SMC);
> +    }
> +
> +    /* We've already checked that EL3 exists at translation time.  */
> +    if (udef) {
> +        env->exception.syndrome = syn_uncategorized();
> +        raise_exception(env, EXCP_UDEF);
> +    }
> +}
> +
>  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 8eaa85f..9a58de8 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -1470,6 +1470,7 @@ 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:
> @@ -1495,6 +1496,18 @@ static void disas_exc(DisasContext *s, uint32_t
> insn)
>              gen_ss_advance(s);
>              gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16));
>              break;
> +        case 3:
> +            if (!arm_dc_feature(s, ARM_FEATURE_EL3) || s->current_pl ==
> 0) {
> +                unallocated_encoding(s);
> +                break;
> +            }
> +            gen_a64_set_pc_im(s->pc - 4);
> +            tmp = tcg_const_i32(syn_aa64_smc(imm16));
> +            gen_helper_pre_smc(cpu_env, tmp);
> +            tcg_temp_free_i32(tmp);
> +            gen_ss_advance(s);
> +            gen_exception_insn(s, 0, EXCP_SMC, syn_aa64_smc(imm16));
> +            break;
>          default:
>              unallocated_encoding(s);
>              break;
> --
> 1.9.1
>
>

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

Re: [Qemu-devel] [PATCH v6 02/10] target-arm: Add SCR_EL3

[Peter Maydell]

On 13 September 2014 05:29, 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    | 19 ++++++++++++++++++-
>  target-arm/helper.c | 35 +++++++++++++++++++++++++++++++++--
>  2 files changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 36507f9..c69d471 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 */
> +        uint64_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.  */

> @@ -873,8 +899,8 @@ 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),
> -      .resetvalue = 0, },
> +      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3),
> +      .resetvalue = 0, .writefn = scr_write },

Still wrong, I'm afraid. For a 32 bit register with a 64
bit struct field you have to use offsetoflow32(), otherwise
you'll get the wrong half on bigendian hosts.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v6 07/10] target-arm: A64: Emulate the HVC insn

[Peter Maydell]

On 13 September 2014 05:29, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:

> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3652,7 +3652,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;

Everywhere else that we start with a CPUState* and need
a CPUARMState* we do it with:

    ARMCPU *cpu = ARM_CPU(cs);
    CPUARMState *env = &cpu->env;

I think we should be consistent.

> +    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;
> +    }

HCR.TGE isn't actually relevant for some exceptions
(eg SMC), and the HVC handling you have below
effectively ends up ignoring the route_to_el2
information. That suggests to me that you should put
this code in a default: case in the switch below.

> +    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 1d7003b..75fc1b3 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_1(pre_hvc, void, env)
>
>  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 64751a0..afcc4e0 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)
> @@ -215,6 +216,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(uint32_t imm16)
> +{
> +    return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff);
> +}
> +
>  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 b956216..8441c27 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -374,6 +374,38 @@ void HELPER(clear_pstate_ss)(CPUARMState *env)
>      env->pstate &= ~PSTATE_SS;
>  }
>
> +void HELPER(pre_hvc)(CPUARMState *env)
> +{
> +    int cur_el = arm_current_pl(env);
> +    /* FIXME: Use actual secure state.  */
> +    bool secure = false;
> +    bool udef;

The standard spelling is "UNDEF" (except in the constant name
"EXCP_UDEF", which we should probably change, though I won't
actually do that now since it will cause a bunch of pointless
conflicts for you. Another tempting rename is EXCP_SWI and
DISAS_SWI to EXCP_SVC and DISAS_SVC, though in that case the
old names have the virtue of having been right once.)

> +
> +    /* 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) {
> +        /* UDEFs trap on the HVC, roll back to the PC to the HVC insn.  */

"with the PC pointing to the HVC insn" ?  At any rate, nothing
in this function is rolling anything back.

> +        env->exception.syndrome = syn_uncategorized();
> +        raise_exception(env, EXCP_UDEF);
> +    }
> +}
> +
>  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 8e66b6c..8eaa85f 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -1473,20 +1473,32 @@ static void disas_exc(DisasContext *s, uint32_t insn)
>
>      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) {
> -            unallocated_encoding(s);
> -            break;
> -        }
>          /* For SVC, HVC and SMC we advance the single-step state
>           * machine before taking the exception. This is architecturally
>           * mandated, to ensure that single-stepping a system call
>           * instruction works properly.
>           */
> -        gen_ss_advance(s);
> -        gen_exception_insn(s, 0, EXCP_SWI, syn_aa64_svc(imm16));
> +        switch (op2_ll) {
> +        case 1:
> +            gen_ss_advance(s);
> +            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;
> +            }
> +            /* The pre HVC helper handles cases when HVC gets trapped
> +             * as an undefined insn by runtime configuration.  */

"*/" on a line of its own, please.

> +            gen_a64_set_pc_im(s->pc - 4);
> +            gen_helper_pre_hvc(cpu_env);
> +            gen_ss_advance(s);
> +            gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16));
> +            break;
> +        default:
> +            unallocated_encoding(s);
> +            break;
> +        }
>          break;
>      case 1:
>          if (op2_ll != 0) {
> --
> 1.9.1
>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v6 08/10] target-arm: A64: Emulate the SMC insn

[Peter Maydell]

On 13 September 2014 05:29, 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     | 26 ++++++++++++++++++++++++++
>  target-arm/translate-a64.c | 13 +++++++++++++
>  7 files changed, 54 insertions(+)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index b553f3d..c24af40 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 4e6ca26..996dfea 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -477,6 +477,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 504ff42..719c95d 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3677,6 +3677,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 75fc1b3..dec3728 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_1(pre_hvc, void, env)
> +DEF_HELPER_2(pre_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 afcc4e0..e15ae57 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)
> @@ -221,6 +222,11 @@ static inline uint32_t syn_aa64_hvc(uint32_t imm16)
>      return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff);
>  }
>
> +static inline uint32_t syn_aa64_smc(uint32_t imm16)
> +{
> +    return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff);
> +}
> +
>  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 8441c27..e6a0656 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -406,6 +406,32 @@ void HELPER(pre_hvc)(CPUARMState *env)
>      }
>  }
>
> +void HELPER(pre_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.
> +     */

NS isn't a mode, it's a state. (S vs NS state is orthogonal
to exception levels and mostly orthogonal to AArch32 modes.)

> +    bool udef = is_a64(env) ? smd : !secure && smd;

What precedence did you intend here between the ?: and
the && operators? More brackets would be nice...

> +    /* In NS EL1, HCR controlled routing to EL2 has priority over SMD.  */
> +    if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
> +        env->exception.syndrome = syndrome;
> +        raise_exception(env, EXCP_SMC);

Shouldn't this just be returning so that the generated
code immediately following can raise the SMC exception
with the correct syndrome, PC and singlestep state?
(would also save you passing in the syndrome argument
to this fn).

> +    }
> +
> +    /* We've already checked that EL3 exists at translation time.  */
> +    if (udef) {
> +        env->exception.syndrome = syn_uncategorized();
> +        raise_exception(env, EXCP_UDEF);
> +    }
> +}
> +
>  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 8eaa85f..9a58de8 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -1470,6 +1470,7 @@ 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:
> @@ -1495,6 +1496,18 @@ static void disas_exc(DisasContext *s, uint32_t insn)
>              gen_ss_advance(s);
>              gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16));
>              break;
> +        case 3:
> +            if (!arm_dc_feature(s, ARM_FEATURE_EL3) || s->current_pl == 0) {
> +                unallocated_encoding(s);
> +                break;
> +            }
> +            gen_a64_set_pc_im(s->pc - 4);
> +            tmp = tcg_const_i32(syn_aa64_smc(imm16));
> +            gen_helper_pre_smc(cpu_env, tmp);
> +            tcg_temp_free_i32(tmp);
> +            gen_ss_advance(s);
> +            gen_exception_insn(s, 0, EXCP_SMC, syn_aa64_smc(imm16));
> +            break;
>          default:
>              unallocated_encoding(s);
>              break;
> --
> 1.9.1

thanks
-- PMM

Re: [Qemu-devel] [PATCH v6 09/10] target-arm: Add IRQ and FIQ routing to EL2 and 3

[Peter Maydell]

On 13 September 2014 05:29, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> 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 | 14 ++++++++++++++
>  2 files changed, 26 insertions(+)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index c24af40..a5123f8 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1178,6 +1178,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.

The logic is correct but I don't think the comment is.
We can take interrupts to EL2 if we're in EL2 as well,
it's just that in that case we honour the PSTATE mask bits.

> +     */
> +    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) {
> @@ -1186,8 +1192,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;
> +        }

This doesn't seem to be implementing the "if HCR_EL2.TGE
is 1 then AMO/IMO/FMO are treated as being 1" behaviour
described in the footnote to Table D1-14.

>          return !(env->daif & PSTATE_I)
>                 && (!IS_M(env) || env->regs[15] < 0xfffffff0);
>      default:
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 719c95d..3a9d1fc 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3683,6 +3683,20 @@ 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;
>  }

thanks
-- PMM

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

[Peter Maydell]

On 13 September 2014 05:29, 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        | 29 ++++++++++++++++++-----------
>  target-arm/cpu.h        | 36 +++++++++++++++++++++++++++++++++---
>  target-arm/helper-a64.c |  2 ++
>  target-arm/helper.c     |  4 ++++
>  target-arm/internals.h  |  2 ++
>  6 files changed, 71 insertions(+), 14 deletions(-)
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index d017588..6203ba5 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -616,6 +616,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;
> +                    }

NB that this is going to conflict with RTH's
patches to refactor this cpu-exec.c ifdef ladder,
though not in a very hard to fix up way.

>  #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 7ea12bd..d7adcb2 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -41,7 +41,9 @@ static void arm_cpu_set_pc(CPUState *cs, vaddr value)
>  static bool arm_cpu_has_work(CPUState *cs)
>  {
>      return cs->interrupt_request &
> -        (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_EXITTB);
> +        (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD
> +         | CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ
> +         | CPU_INTERRUPT_EXITTB);
>  }
>
>  static void cp_reg_reset(gpointer key, gpointer value, gpointer opaque)
> @@ -193,20 +195,22 @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)
>  {
>      ARMCPU *cpu = opaque;
>      CPUState *cs = CPU(cpu);
> +    static const int mask[] = {
> +        [ARM_CPU_IRQ] = CPU_INTERRUPT_HARD,
> +        [ARM_CPU_FIQ] = CPU_INTERRUPT_FIQ,
> +        [ARM_CPU_VIRQ] = CPU_INTERRUPT_VIRQ,
> +        [ARM_CPU_VFIQ] = CPU_INTERRUPT_VFIQ
> +    };
>
>      switch (irq) {
>      case ARM_CPU_IRQ:
> -        if (level) {
> -            cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> -        } else {
> -            cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> -        }
> -        break;
>      case ARM_CPU_FIQ:
> +    case ARM_CPU_VIRQ:
> +    case ARM_CPU_VFIQ:
>          if (level) {
> -            cpu_interrupt(cs, CPU_INTERRUPT_FIQ);
> +            cpu_interrupt(cs, mask[irq]);
>          } else {
> -            cpu_reset_interrupt(cs, CPU_INTERRUPT_FIQ);
> +            cpu_reset_interrupt(cs, mask[irq]);
>          }

It seems like a bad idea to inject the VIRQ and VFIQ
interrupts if the CPU doesn't implement them (ie if
it doesn't implement EL2). We should probably hw_error()
this case. (At least, that's what we'll do if we get one
when KVM is enabled, so we might as well be consistent...)

>          break;
>      default:
> @@ -256,9 +260,12 @@ 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 a5123f8..0333de1 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
> @@ -82,9 +86,12 @@
>  #define offsetofhigh32(S, M) (offsetof(S, M) + sizeof(uint32_t))
>  #endif
>
> -/* Meanings of the ARMCPU object's two inbound GPIO lines */
> +/* Meanings of the ARMCPU object's four inbound GPIO lines */
>  #define ARM_CPU_IRQ 0
>  #define ARM_CPU_FIQ 1
> +#define ARM_CPU_VIRQ 2
> +#define ARM_CPU_VFIQ 3
> +
>

Spurious extra blank line.

>  typedef void ARMWriteCPFunc(void *opaque, int cp_info,
>                              int srcreg, int operand, uint32_t value);
> @@ -1184,6 +1191,18 @@ 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;
> +    /* ARMv7-M interrupt return works by loading a magic value
> +     * into the PC.  On real hardware the load causes the
> +     * return to occur.  The qemu implementation performs the
> +     * jump normally, then does the exception return when the
> +     * CPU tries to execute code at the magic address.
> +     * This will cause the magic PC value to be pushed to
> +     * the stack if an interrupt occurred at the wrong time.
> +     * We avoid this by disabling interrupts when
> +     * pc contains a magic address.
> +     */
> +    bool irq_unmasked = !(env->daif & PSTATE_I)
> +                        && (!IS_M(env) || env->regs[15] < 0xfffffff0);
>
>      /* Don't take exceptions if they target a lower EL.  */
>      if (cur_el > target_el) {
> @@ -1200,8 +1219,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 !(env->daif & PSTATE_I)
> -               && (!IS_M(env) || env->regs[15] < 0xfffffff0);
> +        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();
>      }
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 996dfea..bd16fe3 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -481,9 +481,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 3a9d1fc..2f7b6e6 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3697,6 +3697,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 e15ae57..1e98102 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.9.1

Where is the code to generate VIRQ and VFIQ based on
HCR_EL2.VI and VF ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v6 02/10] target-arm: Add SCR_EL3

[Greg Bellows]

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

Is it necessary to make SCR 64-bit if it is 32-bit on both v7 and v8?

On 25 September 2014 13:15, Peter Maydell <peter.maydell@linaro.org> wrote:

> On 13 September 2014 05:29, 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    | 19 ++++++++++++++++++-
> >  target-arm/helper.c | 35 +++++++++++++++++++++++++++++++++--
> >  2 files changed, 51 insertions(+), 3 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 36507f9..c69d471 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 */
> > +        uint64_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.  */
>
> > @@ -873,8 +899,8 @@ 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),
> > -      .resetvalue = 0, },
> > +      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState,
> cp15.scr_el3),
> > +      .resetvalue = 0, .writefn = scr_write },
>
> Still wrong, I'm afraid. For a 32 bit register with a 64
> bit struct field you have to use offsetoflow32(), otherwise
> you'll get the wrong half on bigendian hosts.
>
> thanks
> -- PMM
>

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

Re: [Qemu-devel] [PATCH v6 02/10] target-arm: Add SCR_EL3

[Peter Maydell]

On 25 September 2014 20:49, Greg Bellows <greg.bellows@linaro.org> wrote:
> Is it necessary to make SCR 64-bit if it is 32-bit on both v7 and v8?

See previous round of reviews -- all AArch64 system registers
are 64 bits ("32 bit" in the ARM ARM is just a shorthand for
"64 bit with the top half RES0"; there is no "read 32 bit
system register instruction) and so the struct fields must
be uint64_t.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v6 02/10] target-arm: Add SCR_EL3

[Greg Bellows]

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

Thanks for the clarification.

On 25 September 2014 14:53, Peter Maydell <peter.maydell@linaro.org> wrote:

> On 25 September 2014 20:49, Greg Bellows <greg.bellows@linaro.org> wrote:
> > Is it necessary to make SCR 64-bit if it is 32-bit on both v7 and v8?
>
> See previous round of reviews -- all AArch64 system registers
> are 64 bits ("32 bit" in the ARM ARM is just a shorthand for
> "64 bit with the top half RES0"; there is no "read 32 bit
> system register instruction) and so the struct fields must
> be uint64_t.
>
> thanks
> -- PMM
>

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

Re: [Qemu-devel] [PATCH v6 02/10] target-arm: Add SCR_EL3

[Edgar E. Iglesias]

On Thu, Sep 25, 2014 at 07:15:29PM +0100, Peter Maydell wrote:
> On 13 September 2014 05:29, 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    | 19 ++++++++++++++++++-
> >  target-arm/helper.c | 35 +++++++++++++++++++++++++++++++++--
> >  2 files changed, 51 insertions(+), 3 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 36507f9..c69d471 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 */
> > +        uint64_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.  */
> 
> > @@ -873,8 +899,8 @@ 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),
> > -      .resetvalue = 0, },
> > +      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3),
> > +      .resetvalue = 0, .writefn = scr_write },
> 
> Still wrong, I'm afraid. For a 32 bit register with a 64
> bit struct field you have to use offsetoflow32(), otherwise
> you'll get the wrong half on bigendian hosts.

Fixed for v7, thanks.

Re: [Qemu-devel] [PATCH v6 07/10] target-arm: A64: Emulate the HVC insn

[Edgar E. Iglesias]

On Thu, Sep 25, 2014 at 07:39:32PM +0100, Peter Maydell wrote:
> On 13 September 2014 05:29, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> 
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -3652,7 +3652,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;
> 
> Everywhere else that we start with a CPUState* and need
> a CPUARMState* we do it with:
> 
>     ARMCPU *cpu = ARM_CPU(cs);
>     CPUARMState *env = &cpu->env;
> 
> I think we should be consistent.
> 
> > +    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;
> > +    }
> 
> HCR.TGE isn't actually relevant for some exceptions
> (eg SMC), and the HVC handling you have below
> effectively ends up ignoring the route_to_el2
> information. That suggests to me that you should put
> this code in a default: case in the switch below.

I don't really test nor support TGE so I'll just drop the TGE part.

> 
> > +    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 1d7003b..75fc1b3 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_1(pre_hvc, void, env)
> >
> >  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 64751a0..afcc4e0 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)
> > @@ -215,6 +216,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(uint32_t imm16)
> > +{
> > +    return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff);
> > +}
> > +
> >  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 b956216..8441c27 100644
> > --- a/target-arm/op_helper.c
> > +++ b/target-arm/op_helper.c
> > @@ -374,6 +374,38 @@ void HELPER(clear_pstate_ss)(CPUARMState *env)
> >      env->pstate &= ~PSTATE_SS;
> >  }
> >
> > +void HELPER(pre_hvc)(CPUARMState *env)
> > +{
> > +    int cur_el = arm_current_pl(env);
> > +    /* FIXME: Use actual secure state.  */
> > +    bool secure = false;
> > +    bool udef;
> 
> The standard spelling is "UNDEF" (except in the constant name
> "EXCP_UDEF", which we should probably change, though I won't
> actually do that now since it will cause a bunch of pointless
> conflicts for you. Another tempting rename is EXCP_SWI and
> DISAS_SWI to EXCP_SVC and DISAS_SVC, though in that case the
> old names have the virtue of having been right once.)
> 
> > +
> > +    /* 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) {
> > +        /* UDEFs trap on the HVC, roll back to the PC to the HVC insn.  */
> 
> "with the PC pointing to the HVC insn" ?  At any rate, nothing
> in this function is rolling anything back.

This is a hang-over from v5 prior to the singlestepping support
when the code was rolling back the pc.

HVCs that lead to undef are taken with the prefered exception
return pointing to the HVC, i.e before advancing the PC.

> 
> > +        env->exception.syndrome = syn_uncategorized();
> > +        raise_exception(env, EXCP_UDEF);
> > +    }
> > +}
> > +
> >  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 8e66b6c..8eaa85f 100644
> > --- a/target-arm/translate-a64.c
> > +++ b/target-arm/translate-a64.c
> > @@ -1473,20 +1473,32 @@ static void disas_exc(DisasContext *s, uint32_t insn)
> >
> >      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) {
> > -            unallocated_encoding(s);
> > -            break;
> > -        }
> >          /* For SVC, HVC and SMC we advance the single-step state
> >           * machine before taking the exception. This is architecturally
> >           * mandated, to ensure that single-stepping a system call
> >           * instruction works properly.
> >           */
> > -        gen_ss_advance(s);
> > -        gen_exception_insn(s, 0, EXCP_SWI, syn_aa64_svc(imm16));
> > +        switch (op2_ll) {
> > +        case 1:
> > +            gen_ss_advance(s);
> > +            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;
> > +            }
> > +            /* The pre HVC helper handles cases when HVC gets trapped
> > +             * as an undefined insn by runtime configuration.  */
> 
> "*/" on a line of its own, please.
> 
> > +            gen_a64_set_pc_im(s->pc - 4);
> > +            gen_helper_pre_hvc(cpu_env);
> > +            gen_ss_advance(s);
> > +            gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16));
> > +            break;
> > +        default:
> > +            unallocated_encoding(s);
> > +            break;
> > +        }
> >          break;
> >      case 1:
> >          if (op2_ll != 0) {
> > --
> > 1.9.1
> >
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v6 08/10] target-arm: A64: Emulate the SMC insn

[Edgar E. Iglesias]

On Thu, Sep 25, 2014 at 07:47:16PM +0100, Peter Maydell wrote:
> On 13 September 2014 05:29, 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     | 26 ++++++++++++++++++++++++++
> >  target-arm/translate-a64.c | 13 +++++++++++++
> >  7 files changed, 54 insertions(+)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index b553f3d..c24af40 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 4e6ca26..996dfea 100644
> > --- a/target-arm/helper-a64.c
> > +++ b/target-arm/helper-a64.c
> > @@ -477,6 +477,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 504ff42..719c95d 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -3677,6 +3677,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 75fc1b3..dec3728 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_1(pre_hvc, void, env)
> > +DEF_HELPER_2(pre_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 afcc4e0..e15ae57 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)
> > @@ -221,6 +222,11 @@ static inline uint32_t syn_aa64_hvc(uint32_t imm16)
> >      return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff);
> >  }
> >
> > +static inline uint32_t syn_aa64_smc(uint32_t imm16)
> > +{
> > +    return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff);
> > +}
> > +
> >  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 8441c27..e6a0656 100644
> > --- a/target-arm/op_helper.c
> > +++ b/target-arm/op_helper.c
> > @@ -406,6 +406,32 @@ void HELPER(pre_hvc)(CPUARMState *env)
> >      }
> >  }
> >
> > +void HELPER(pre_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.
> > +     */
> 
> NS isn't a mode, it's a state. (S vs NS state is orthogonal
> to exception levels and mostly orthogonal to AArch32 modes.)
> 
> > +    bool udef = is_a64(env) ? smd : !secure && smd;
> 
> What precedence did you intend here between the ?: and
> the && operators? More brackets would be nice...

Will change it to:

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)) {
> > +        env->exception.syndrome = syndrome;
> > +        raise_exception(env, EXCP_SMC);
> 
> Shouldn't this just be returning so that the generated
> code immediately following can raise the SMC exception
> with the correct syndrome, PC and singlestep state?
> (would also save you passing in the syndrome argument
> to this fn).

When routing SMCs to EL2, the exception happens before advancing the
PC. It's similar to the undef cases for HVC (and SMC).


> 
> > +    }
> > +
> > +    /* We've already checked that EL3 exists at translation time.  */
> > +    if (udef) {
> > +        env->exception.syndrome = syn_uncategorized();
> > +        raise_exception(env, EXCP_UDEF);
> > +    }
> > +}
> > +
> >  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 8eaa85f..9a58de8 100644
> > --- a/target-arm/translate-a64.c
> > +++ b/target-arm/translate-a64.c
> > @@ -1470,6 +1470,7 @@ 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:
> > @@ -1495,6 +1496,18 @@ static void disas_exc(DisasContext *s, uint32_t insn)
> >              gen_ss_advance(s);
> >              gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16));
> >              break;
> > +        case 3:
> > +            if (!arm_dc_feature(s, ARM_FEATURE_EL3) || s->current_pl == 0) {
> > +                unallocated_encoding(s);
> > +                break;
> > +            }
> > +            gen_a64_set_pc_im(s->pc - 4);
> > +            tmp = tcg_const_i32(syn_aa64_smc(imm16));
> > +            gen_helper_pre_smc(cpu_env, tmp);
> > +            tcg_temp_free_i32(tmp);
> > +            gen_ss_advance(s);
> > +            gen_exception_insn(s, 0, EXCP_SMC, syn_aa64_smc(imm16));
> > +            break;
> >          default:
> >              unallocated_encoding(s);
> >              break;
> > --
> > 1.9.1
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v6 07/10] target-arm: A64: Emulate the HVC insn

[Peter Maydell]

On 25 September 2014 23:20, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Thu, Sep 25, 2014 at 07:39:32PM +0100, Peter Maydell wrote:
>> HCR.TGE isn't actually relevant for some exceptions
>> (eg SMC), and the HVC handling you have below
>> effectively ends up ignoring the route_to_el2
>> information. That suggests to me that you should put
>> this code in a default: case in the switch below.
>
> I don't really test nor support TGE so I'll just drop the TGE part.

I'd rather we just implemented it properly...

-- PMM

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

[Edgar E. Iglesias]

On Thu, Sep 25, 2014 at 08:36:41PM +0100, Peter Maydell wrote:
> On 13 September 2014 05:29, 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        | 29 ++++++++++++++++++-----------
> >  target-arm/cpu.h        | 36 +++++++++++++++++++++++++++++++++---
> >  target-arm/helper-a64.c |  2 ++
> >  target-arm/helper.c     |  4 ++++
> >  target-arm/internals.h  |  2 ++
> >  6 files changed, 71 insertions(+), 14 deletions(-)
> >
> > diff --git a/cpu-exec.c b/cpu-exec.c
> > index d017588..6203ba5 100644
> > --- a/cpu-exec.c
> > +++ b/cpu-exec.c
> > @@ -616,6 +616,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;
> > +                    }
> 
> NB that this is going to conflict with RTH's
> patches to refactor this cpu-exec.c ifdef ladder,
> though not in a very hard to fix up way.

Right, thanks for the heads up.


> >  #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 7ea12bd..d7adcb2 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -41,7 +41,9 @@ static void arm_cpu_set_pc(CPUState *cs, vaddr value)
> >  static bool arm_cpu_has_work(CPUState *cs)
> >  {
> >      return cs->interrupt_request &
> > -        (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_EXITTB);
> > +        (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD
> > +         | CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ
> > +         | CPU_INTERRUPT_EXITTB);
> >  }
> >
> >  static void cp_reg_reset(gpointer key, gpointer value, gpointer opaque)
> > @@ -193,20 +195,22 @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)
> >  {
> >      ARMCPU *cpu = opaque;
> >      CPUState *cs = CPU(cpu);
> > +    static const int mask[] = {
> > +        [ARM_CPU_IRQ] = CPU_INTERRUPT_HARD,
> > +        [ARM_CPU_FIQ] = CPU_INTERRUPT_FIQ,
> > +        [ARM_CPU_VIRQ] = CPU_INTERRUPT_VIRQ,
> > +        [ARM_CPU_VFIQ] = CPU_INTERRUPT_VFIQ
> > +    };
> >
> >      switch (irq) {
> >      case ARM_CPU_IRQ:
> > -        if (level) {
> > -            cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> > -        } else {
> > -            cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> > -        }
> > -        break;
> >      case ARM_CPU_FIQ:
> > +    case ARM_CPU_VIRQ:
> > +    case ARM_CPU_VFIQ:
> >          if (level) {
> > -            cpu_interrupt(cs, CPU_INTERRUPT_FIQ);
> > +            cpu_interrupt(cs, mask[irq]);
> >          } else {
> > -            cpu_reset_interrupt(cs, CPU_INTERRUPT_FIQ);
> > +            cpu_reset_interrupt(cs, mask[irq]);
> >          }
> 
> It seems like a bad idea to inject the VIRQ and VFIQ
> interrupts if the CPU doesn't implement them (ie if
> it doesn't implement EL2). We should probably hw_error()
> this case. (At least, that's what we'll do if we get one
> when KVM is enabled, so we might as well be consistent...)

Sounds good.

> 
> >          break;
> >      default:
> > @@ -256,9 +260,12 @@ 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 a5123f8..0333de1 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
> > @@ -82,9 +86,12 @@
> >  #define offsetofhigh32(S, M) (offsetof(S, M) + sizeof(uint32_t))
> >  #endif
> >
> > -/* Meanings of the ARMCPU object's two inbound GPIO lines */
> > +/* Meanings of the ARMCPU object's four inbound GPIO lines */
> >  #define ARM_CPU_IRQ 0
> >  #define ARM_CPU_FIQ 1
> > +#define ARM_CPU_VIRQ 2
> > +#define ARM_CPU_VFIQ 3
> > +
> >
> 
> Spurious extra blank line.

Removed.

> 
> >  typedef void ARMWriteCPFunc(void *opaque, int cp_info,
> >                              int srcreg, int operand, uint32_t value);
> > @@ -1184,6 +1191,18 @@ 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;
> > +    /* ARMv7-M interrupt return works by loading a magic value
> > +     * into the PC.  On real hardware the load causes the
> > +     * return to occur.  The qemu implementation performs the
> > +     * jump normally, then does the exception return when the
> > +     * CPU tries to execute code at the magic address.
> > +     * This will cause the magic PC value to be pushed to
> > +     * the stack if an interrupt occurred at the wrong time.
> > +     * We avoid this by disabling interrupts when
> > +     * pc contains a magic address.
> > +     */
> > +    bool irq_unmasked = !(env->daif & PSTATE_I)
> > +                        && (!IS_M(env) || env->regs[15] < 0xfffffff0);
> >
> >      /* Don't take exceptions if they target a lower EL.  */
> >      if (cur_el > target_el) {
> > @@ -1200,8 +1219,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 !(env->daif & PSTATE_I)
> > -               && (!IS_M(env) || env->regs[15] < 0xfffffff0);
> > +        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();
> >      }
> > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> > index 996dfea..bd16fe3 100644
> > --- a/target-arm/helper-a64.c
> > +++ b/target-arm/helper-a64.c
> > @@ -481,9 +481,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 3a9d1fc..2f7b6e6 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -3697,6 +3697,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 e15ae57..1e98102 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.9.1
> 
> Where is the code to generate VIRQ and VFIQ based on
> HCR_EL2.VI and VF ?

I've scoped that out as future work (mentioned in the cover-letter).

Best regards,
Edgar

Re: [Qemu-devel] [PATCH v6 07/10] target-arm: A64: Emulate the HVC insn

[Edgar E. Iglesias]

On Fri, Sep 26, 2014 at 12:01:11AM +0100, Peter Maydell wrote:
> On 25 September 2014 23:20, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > On Thu, Sep 25, 2014 at 07:39:32PM +0100, Peter Maydell wrote:
> >> HCR.TGE isn't actually relevant for some exceptions
> >> (eg SMC), and the HVC handling you have below
> >> effectively ends up ignoring the route_to_el2
> >> information. That suggests to me that you should put
> >> this code in a default: case in the switch below.
> >
> > I don't really test nor support TGE so I'll just drop the TGE part.
> 
> I'd rather we just implemented it properly...

IMO, It's not about implementing it properly. It's about implementing the
EL3/2 support incrementally. The spec is too big to add all the features
at once.

Cheers,
Edgar

Re: [Qemu-devel] [PATCH v6 08/10] target-arm: A64: Emulate the SMC insn

[Peter Maydell]

On 25 September 2014 23:55, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Thu, Sep 25, 2014 at 07:47:16PM +0100, Peter Maydell wrote:
>> > +    /* In NS EL1, HCR controlled routing to EL2 has priority over SMD.  */
>> > +    if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
>> > +        env->exception.syndrome = syndrome;
>> > +        raise_exception(env, EXCP_SMC);
>>
>> Shouldn't this just be returning so that the generated
>> code immediately following can raise the SMC exception
>> with the correct syndrome, PC and singlestep state?
>> (would also save you passing in the syndrome argument
>> to this fn).
>
> When routing SMCs to EL2, the exception happens before advancing the
> PC. It's similar to the undef cases for HVC (and SMC).

Oh, yes, that's the trap enable bit. In that case we shouldn't
be using EXCP_SMC: this isn't routing the SMC exception, it's
taking a Hyp trap exception, and in AArch32 the vector
entry point is different. (Granted, you can't get to AArch32
by taking an exception from AArch64, but we should use the
right EXCP_ value to avoid the code looking gratuitously
different for the two cases.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v6 07/10] target-arm: A64: Emulate the HVC insn

[Peter Maydell]

On 26 September 2014 00:06, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Fri, Sep 26, 2014 at 12:01:11AM +0100, Peter Maydell wrote:
>> On 25 September 2014 23:20, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
>> > On Thu, Sep 25, 2014 at 07:39:32PM +0100, Peter Maydell wrote:
>> >> HCR.TGE isn't actually relevant for some exceptions
>> >> (eg SMC), and the HVC handling you have below
>> >> effectively ends up ignoring the route_to_el2
>> >> information. That suggests to me that you should put
>> >> this code in a default: case in the switch below.
>> >
>> > I don't really test nor support TGE so I'll just drop the TGE part.
>>
>> I'd rather we just implemented it properly...
>
> IMO, It's not about implementing it properly. It's about implementing the
> EL3/2 support incrementally. The spec is too big to add all the features
> at once.

Well, maybe. My point still stands that the code you have there
to figure out route_to_el2 is not going to be implementable as
a generic code fragment that doesn't care about the
exception type, and so you should stick to having the code
inside each switch case.

-- PMM

Re: [Qemu-devel] [PATCH v6 08/10] target-arm: A64: Emulate the SMC insn

[Edgar E. Iglesias]

On Fri, Sep 26, 2014 at 12:17:59AM +0100, Peter Maydell wrote:
> On 25 September 2014 23:55, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > On Thu, Sep 25, 2014 at 07:47:16PM +0100, Peter Maydell wrote:
> >> > +    /* In NS EL1, HCR controlled routing to EL2 has priority over SMD.  */
> >> > +    if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
> >> > +        env->exception.syndrome = syndrome;
> >> > +        raise_exception(env, EXCP_SMC);
> >>
> >> Shouldn't this just be returning so that the generated
> >> code immediately following can raise the SMC exception
> >> with the correct syndrome, PC and singlestep state?
> >> (would also save you passing in the syndrome argument
> >> to this fn).
> >
> > When routing SMCs to EL2, the exception happens before advancing the
> > PC. It's similar to the undef cases for HVC (and SMC).
> 
> Oh, yes, that's the trap enable bit. In that case we shouldn't
> be using EXCP_SMC: this isn't routing the SMC exception, it's
> taking a Hyp trap exception, and in AArch32 the vector
> entry point is different. (Granted, you can't get to AArch32
> by taking an exception from AArch64, but we should use the
> right EXCP_ value to avoid the code looking gratuitously
> different for the two cases.)

I see. I hadn't thought much about the AArch32 case here. For
AArch64, the pseudo code referes to this as route_to_el2.
Anyway, your comment makes sense to avoid diff between a32/a64
and I think it actually makes the AArch64 code a bit cleaner
aswell.

I'll add EXCP_HYP_TRAP.

Thanks,
Edgar

Re: [Qemu-devel] [PATCH v6 08/10] target-arm: A64: Emulate the SMC insn

[Peter Maydell]

On 26 September 2014 00:31, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Fri, Sep 26, 2014 at 12:17:59AM +0100, Peter Maydell wrote:
>> Oh, yes, that's the trap enable bit. In that case we shouldn't
>> be using EXCP_SMC: this isn't routing the SMC exception, it's
>> taking a Hyp trap exception, and in AArch32 the vector
>> entry point is different. (Granted, you can't get to AArch32
>> by taking an exception from AArch64, but we should use the
>> right EXCP_ value to avoid the code looking gratuitously
>> different for the two cases.)
>
> I see. I hadn't thought much about the AArch32 case here. For
> AArch64, the pseudo code referes to this as route_to_el2.

Mmm, but the pseudocode keeps AArch64 and AArch32 exception
paths a lot more separate than we do, and so it doesn't need
any information about the AArch32 exception when it's dealing
with a from-AArch64 exception. Our implementation routes
both paths in common, and so it's slightly clearer to always
retain the correct info for both cases (if nothing else, it's
slightly more accurate when we print out debug log about
what exceptions we're taking).

> Anyway, your comment makes sense to avoid diff between a32/a64
> and I think it actually makes the AArch64 code a bit cleaner
> aswell.
>
> I'll add EXCP_HYP_TRAP.

Thanks.

-- PMM

Re: [Qemu-devel] [PATCH v6 08/10] target-arm: A64: Emulate the SMC insn

[Edgar E. Iglesias]

On Fri, Sep 26, 2014 at 12:43:40AM +0100, Peter Maydell wrote:
> On 26 September 2014 00:31, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > On Fri, Sep 26, 2014 at 12:17:59AM +0100, Peter Maydell wrote:
> >> Oh, yes, that's the trap enable bit. In that case we shouldn't
> >> be using EXCP_SMC: this isn't routing the SMC exception, it's
> >> taking a Hyp trap exception, and in AArch32 the vector
> >> entry point is different. (Granted, you can't get to AArch32
> >> by taking an exception from AArch64, but we should use the
> >> right EXCP_ value to avoid the code looking gratuitously
> >> different for the two cases.)
> >
> > I see. I hadn't thought much about the AArch32 case here. For
> > AArch64, the pseudo code referes to this as route_to_el2.
> 
> Mmm, but the pseudocode keeps AArch64 and AArch32 exception
> paths a lot more separate than we do, and so it doesn't need
> any information about the AArch32 exception when it's dealing
> with a from-AArch64 exception. Our implementation routes
> both paths in common, and so it's slightly clearer to always
> retain the correct info for both cases (if nothing else, it's
> slightly more accurate when we print out debug log about
> what exceptions we're taking).

Agreed, your suggestion makes sense and will save some
pain for the A32 case. Good catch, thanks.

> 
> > Anyway, your comment makes sense to avoid diff between a32/a64
> > and I think it actually makes the AArch64 code a bit cleaner
> > aswell.
> >
> > I'll add EXCP_HYP_TRAP.
> 
> Thanks.
> 
> -- PMM

Re: [Qemu-devel] [PATCH v6 08/10] target-arm: A64: Emulate the SMC insn

[Edgar E. Iglesias]

On Fri, Sep 26, 2014 at 12:43:40AM +0100, Peter Maydell wrote:
> On 26 September 2014 00:31, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > On Fri, Sep 26, 2014 at 12:17:59AM +0100, Peter Maydell wrote:
> >> Oh, yes, that's the trap enable bit. In that case we shouldn't
> >> be using EXCP_SMC: this isn't routing the SMC exception, it's
> >> taking a Hyp trap exception, and in AArch32 the vector
> >> entry point is different. (Granted, you can't get to AArch32
> >> by taking an exception from AArch64, but we should use the
> >> right EXCP_ value to avoid the code looking gratuitously
> >> different for the two cases.)
> >
> > I see. I hadn't thought much about the AArch32 case here. For
> > AArch64, the pseudo code referes to this as route_to_el2.
> 
> Mmm, but the pseudocode keeps AArch64 and AArch32 exception
> paths a lot more separate than we do, and so it doesn't need
> any information about the AArch32 exception when it's dealing
> with a from-AArch64 exception. Our implementation routes
> both paths in common, and so it's slightly clearer to always
> retain the correct info for both cases (if nothing else, it's
> slightly more accurate when we print out debug log about
> what exceptions we're taking).
> 
> > Anyway, your comment makes sense to avoid diff between a32/a64
> > and I think it actually makes the AArch64 code a bit cleaner
> > aswell.
> >
> > I'll add EXCP_HYP_TRAP.
> 
> Thanks.

Hi,

I've just sent a v7. I think I've addressed your comments.

I'll see if I get some HCR.TGE test-cases going and send follow-up
patches on that later.

Cheers,
Edgar