[Qemu-devel] [PATCH v2 0/6] target-arm: Implement various EL3 traps

[Qemu-devel] [PATCH v2 0/6] target-arm: Implement various EL3 traps

[Peter Maydell]

This patchset fixes or implements a lot of traps to EL3 as
listed in the ARM ARM section D1.15.4 "EL3 configurable controls".
Most of the rest we already had implemented.

NB: where the trap I was implementing for EL3 had an
obvious equivalent in EL2 I included the EL2 check in this
series, but I haven't attempted to implement traps which are
only EL2 and not EL3.

Changes v1->v2: rewrote patch 1 to use a new arm_highest_el()
function.

Patches 2..6 are already reviewed, only patch 1 needs doing.

thanks
-- PMM

Peter Maydell (6):
  target-arm: correct CNTFRQ access rights
  target-arm: Fix handling of SCR.SMD
  target-arm: Implement MDCR_EL3.TDOSA and MDCR_EL2.TDOSA traps
  target-arm: Implement MDCR_EL2.TDRA traps
  target-arm: Implement MDCR_EL3.TDA and MDCR_EL2.TDA traps
  target-arm: Report correct syndrome for FPEXC32_EL2 traps

 target-arm/cpu.h       |  29 ++++++++++++
 target-arm/helper.c    | 122 +++++++++++++++++++++++++++++++++++++++++--------
 target-arm/op_helper.c |  25 ++++++++--
 3 files changed, 153 insertions(+), 23 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v2 1/6] target-arm: correct CNTFRQ access rights

[Peter Maydell]

Correct some corner cases we were getting wrong for
CNTFRQ access rights:
 * should UNDEF from 32-bit Secure EL1
 * only writable from the highest implemented exception level,
   which might not be EL1 now

To clarify the code, provide a new utility function
arm_highest_el() which returns the highest implemented
exception level.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Rewritten to use arm_highest_el() to improve clarity
---
 target-arm/cpu.h    | 12 ++++++++++++
 target-arm/helper.c | 29 ++++++++++++++++++++++++++---
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 5137632..afbf366 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1255,6 +1255,18 @@ static inline bool cptype_valid(int cptype)
 #define PL1_RW (PL1_R | PL1_W)
 #define PL0_RW (PL0_R | PL0_W)
 
+/* Return the highest implemented Exception Level */
+static inline int arm_highest_el(CPUARMState *env)
+{
+    if (arm_feature(env, ARM_FEATURE_EL3)) {
+        return 3;
+    }
+    if (arm_feature(env, ARM_FEATURE_EL2)) {
+        return 2;
+    }
+    return 1;
+}
+
 /* Return the current Exception Level (as per ARMv8; note that this differs
  * from the ARMv7 Privilege Level).
  */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 2f9db72..4d27c00 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1218,10 +1218,33 @@ static const ARMCPRegInfo v6k_cp_reginfo[] = {
 static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo *ri,
                                        bool isread)
 {
-    /* CNTFRQ: not visible from PL0 if both PL0PCTEN and PL0VCTEN are zero */
-    if (arm_current_el(env) == 0 && !extract32(env->cp15.c14_cntkctl, 0, 2)) {
-        return CP_ACCESS_TRAP;
+    /* CNTFRQ: not visible from PL0 if both PL0PCTEN and PL0VCTEN are zero.
+     * Writable only at the highest implemented exception level.
+     */
+    int el = arm_current_el(env);
+
+    switch (el) {
+    case 0:
+        if (!extract32(env->cp15.c14_cntkctl, 0, 2)) {
+            return CP_ACCESS_TRAP;
+        }
+        break;
+    case 1:
+        if (!isread && ri->state == ARM_CP_STATE_AA32 &&
+            arm_is_secure_below_el3(env)) {
+            /* Accesses from 32-bit Secure EL1 UNDEF (*not* trap to EL3!) */
+            return CP_ACCESS_TRAP_UNCATEGORIZED;
+        }
+        break;
+    case 2:
+    case 3:
+        break;
     }
+
+    if (!isread && el < arm_highest_el(env)) {
+        return CP_ACCESS_TRAP_UNCATEGORIZED;
+    }
+
     return CP_ACCESS_OK;
 }
 
-- 
1.9.1

[Qemu-devel] [PATCH v2 2/6] target-arm: Fix handling of SCR.SMD

[Peter Maydell]

We weren't quite implementing the handling of SCR.SMD correctly.
The condition governing whether the SMD bit should apply only
for NS state is "is EL3 is AArch32", not "is the current EL AArch32".
Fix the condition, and clarify the comment both to reflect this and
to expand slightly on what's going on for the v7-no-Virtualization case.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>
---
The bit about forcing SMD to zero confused me, anyway, since I
expected it to mean "in this function", not elsewhere...
---
 target-arm/op_helper.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index bd48549..4c0980e 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -614,12 +614,14 @@ void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)
     int cur_el = arm_current_el(env);
     bool secure = arm_is_secure(env);
     bool smd = env->cp15.scr_el3 & SCR_SMD;
-    /* On ARMv8 AArch32, SMD only applies to NS state.
-     * On ARMv7 SMD only applies to NS state 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.
+    /* On ARMv8 with EL3 AArch64, SMD applies to both S and NS state.
+     * On ARMv8 with EL3 AArch32, or ARMv7 with the Virtualization
+     *  extensions, SMD only applies to NS state.
+     * On ARMv7 without the Virtualization extensions, the SMD bit
+     * doesn't exist, but we forbid the guest to set it to 1 in scr_write(),
+     * so we need not special case this here.
      */
-    bool undef = is_a64(env) ? smd : (!secure && smd);
+    bool undef = arm_feature(env, ARM_FEATURE_AARCH64) ? smd : smd && !secure;
 
     if (arm_is_psci_call(cpu, EXCP_SMC)) {
         /* If PSCI is enabled and this looks like a valid PSCI call then
-- 
1.9.1

[Qemu-devel] [PATCH v2 3/6] target-arm: Implement MDCR_EL3.TDOSA and MDCR_EL2.TDOSA traps

[Peter Maydell]

Implement the traps to EL2 and EL3 controlled by the bits
MDCR_EL2.TDOSA MDCR_EL3.TDOSA. These can configurably trap
accesses to the "powerdown debug" registers.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>
---
 target-arm/cpu.h    | 12 ++++++++++++
 target-arm/helper.c | 23 ++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index afbf366..77f9b51 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -595,6 +595,18 @@ void pmccntr_sync(CPUARMState *env);
 #define CPTR_TTA      (1U << 20)
 #define CPTR_TFP      (1U << 10)
 
+#define MDCR_EPMAD    (1U << 21)
+#define MDCR_EDAD     (1U << 20)
+#define MDCR_SPME     (1U << 17)
+#define MDCR_SDD      (1U << 16)
+#define MDCR_TDRA     (1U << 11)
+#define MDCR_TDOSA    (1U << 10)
+#define MDCR_TDA      (1U << 9)
+#define MDCR_TDE      (1U << 8)
+#define MDCR_HPME     (1U << 7)
+#define MDCR_TPM      (1U << 6)
+#define MDCR_TPMCR    (1U << 5)
+
 #define CPSR_M (0x1fU)
 #define CPSR_T (1U << 5)
 #define CPSR_F (1U << 6)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 4d27c00..b45d596 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -385,6 +385,24 @@ static CPAccessResult access_trap_aa32s_el1(CPUARMState *env,
     return CP_ACCESS_TRAP_UNCATEGORIZED;
 }
 
+/* Check for traps to "powerdown debug" registers, which are controlled
+ * by MDCR.TDOSA
+ */
+static CPAccessResult access_tdosa(CPUARMState *env, const ARMCPRegInfo *ri,
+                                   bool isread)
+{
+    int el = arm_current_el(env);
+
+    if (el < 2 && (env->cp15.mdcr_el2 & MDCR_TDOSA)
+        && !arm_is_secure_below_el3(env)) {
+        return CP_ACCESS_TRAP_EL2;
+    }
+    if (el < 3 && (env->cp15.mdcr_el3 & MDCR_TDOSA)) {
+        return CP_ACCESS_TRAP_EL3;
+    }
+    return CP_ACCESS_OK;
+}
+
 static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
 {
     ARMCPU *cpu = arm_env_get_cpu(env);
@@ -3778,15 +3796,18 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
     { .name = "OSLAR_EL1", .state = ARM_CP_STATE_BOTH,
       .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 4,
       .access = PL1_W, .type = ARM_CP_NO_RAW,
+      .accessfn = access_tdosa,
       .writefn = oslar_write },
     { .name = "OSLSR_EL1", .state = ARM_CP_STATE_BOTH,
       .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 4,
       .access = PL1_R, .resetvalue = 10,
+      .accessfn = access_tdosa,
       .fieldoffset = offsetof(CPUARMState, cp15.oslsr_el1) },
     /* Dummy OSDLR_EL1: 32-bit Linux will read this */
     { .name = "OSDLR_EL1", .state = ARM_CP_STATE_BOTH,
       .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 3, .opc2 = 4,
-      .access = PL1_RW, .type = ARM_CP_NOP },
+      .access = PL1_RW, .accessfn = access_tdosa,
+      .type = ARM_CP_NOP },
     /* Dummy DBGVCR: Linux wants to clear this on startup, but we don't
      * implement vector catch debug events yet.
      */
-- 
1.9.1

[Qemu-devel] [PATCH v2 4/6] target-arm: Implement MDCR_EL2.TDRA traps

[Peter Maydell]

Implement trapping of the "debug ROM" registers, which are controlled
by MDCR_EL2.TDRA for EL2 but by the more general MDCR_EL3.TDA for EL3.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>
---
 target-arm/helper.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index b45d596..11eb77a 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -403,6 +403,24 @@ static CPAccessResult access_tdosa(CPUARMState *env, const ARMCPRegInfo *ri,
     return CP_ACCESS_OK;
 }
 
+/* Check for traps to "debug ROM" registers, which are controlled
+ * by MDCR_EL2.TDRA for EL2 but by the more general MDCR_EL3.TDA for EL3.
+ */
+static CPAccessResult access_tdra(CPUARMState *env, const ARMCPRegInfo *ri,
+                                  bool isread)
+{
+    int el = arm_current_el(env);
+
+    if (el < 2 && (env->cp15.mdcr_el2 & MDCR_TDRA)
+        && !arm_is_secure_below_el3(env)) {
+        return CP_ACCESS_TRAP_EL2;
+    }
+    if (el < 3 && (env->cp15.mdcr_el3 & MDCR_TDA)) {
+        return CP_ACCESS_TRAP_EL3;
+    }
+    return CP_ACCESS_OK;
+}
+
 static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
 {
     ARMCPU *cpu = arm_env_get_cpu(env);
@@ -3773,12 +3791,15 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
      * accessor.
      */
     { .name = "DBGDRAR", .cp = 14, .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 0,
-      .access = PL0_R, .type = ARM_CP_CONST, .resetvalue = 0 },
+      .access = PL0_R, .accessfn = access_tdra,
+      .type = ARM_CP_CONST, .resetvalue = 0 },
     { .name = "MDRAR_EL1", .state = ARM_CP_STATE_AA64,
       .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 0,
-      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
+      .access = PL1_R, .accessfn = access_tdra,
+      .type = ARM_CP_CONST, .resetvalue = 0 },
     { .name = "DBGDSAR", .cp = 14, .crn = 2, .crm = 0, .opc1 = 0, .opc2 = 0,
-      .access = PL0_R, .type = ARM_CP_CONST, .resetvalue = 0 },
+      .access = PL0_R, .accessfn = access_tdra,
+      .type = ARM_CP_CONST, .resetvalue = 0 },
     /* Monitor debug system control register; the 32-bit alias is DBGDSCRext. */
     { .name = "MDSCR_EL1", .state = ARM_CP_STATE_BOTH,
       .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 2, .opc2 = 2,
-- 
1.9.1

[Qemu-devel] [PATCH v2 5/6] target-arm: Implement MDCR_EL3.TDA and MDCR_EL2.TDA traps

[Peter Maydell]

Implement the debug register traps controlled by MDCR_EL2.TDA
and MDCR_EL3.TDA.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>
---
 target-arm/helper.c | 39 ++++++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 11eb77a..e2b7238 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -421,6 +421,24 @@ static CPAccessResult access_tdra(CPUARMState *env, const ARMCPRegInfo *ri,
     return CP_ACCESS_OK;
 }
 
+/* Check for traps to general debug registers, which are controlled
+ * by MDCR_EL2.TDA for EL2 and MDCR_EL3.TDA for EL3.
+ */
+static CPAccessResult access_tda(CPUARMState *env, const ARMCPRegInfo *ri,
+                                  bool isread)
+{
+    int el = arm_current_el(env);
+
+    if (el < 2 && (env->cp15.mdcr_el2 & MDCR_TDA)
+        && !arm_is_secure_below_el3(env)) {
+        return CP_ACCESS_TRAP_EL2;
+    }
+    if (el < 3 && (env->cp15.mdcr_el3 & MDCR_TDA)) {
+        return CP_ACCESS_TRAP_EL3;
+    }
+    return CP_ACCESS_OK;
+}
+
 static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
 {
     ARMCPU *cpu = arm_env_get_cpu(env);
@@ -3384,7 +3402,8 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
       .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
     { .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
-      .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+      .access = PL2_RW, .accessfn = access_tda,
+      .type = ARM_CP_CONST, .resetvalue = 0 },
     { .name = "HPFAR_EL2", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
       .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
@@ -3803,7 +3822,7 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
     /* Monitor debug system control register; the 32-bit alias is DBGDSCRext. */
     { .name = "MDSCR_EL1", .state = ARM_CP_STATE_BOTH,
       .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 2, .opc2 = 2,
-      .access = PL1_RW,
+      .access = PL1_RW, .accessfn = access_tda,
       .fieldoffset = offsetof(CPUARMState, cp15.mdscr_el1),
       .resetvalue = 0 },
     /* MDCCSR_EL0, aka DBGDSCRint. This is a read-only mirror of MDSCR_EL1.
@@ -3812,7 +3831,7 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
     { .name = "MDCCSR_EL0", .state = ARM_CP_STATE_BOTH,
       .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 1, .opc2 = 0,
       .type = ARM_CP_ALIAS,
-      .access = PL1_R,
+      .access = PL1_R, .accessfn = access_tda,
       .fieldoffset = offsetof(CPUARMState, cp15.mdscr_el1), },
     { .name = "OSLAR_EL1", .state = ARM_CP_STATE_BOTH,
       .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 4,
@@ -3834,7 +3853,8 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
      */
     { .name = "DBGVCR",
       .cp = 14, .opc1 = 0, .crn = 0, .crm = 7, .opc2 = 0,
-      .access = PL1_RW, .type = ARM_CP_NOP },
+      .access = PL1_RW, .accessfn = access_tda,
+      .type = ARM_CP_NOP },
     REGINFO_SENTINEL
 };
 
@@ -4099,7 +4119,8 @@ static void define_debug_regs(ARMCPU *cpu)
     int wrps, brps, ctx_cmps;
     ARMCPRegInfo dbgdidr = {
         .name = "DBGDIDR", .cp = 14, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 0,
-        .access = PL0_R, .type = ARM_CP_CONST, .resetvalue = cpu->dbgdidr,
+        .access = PL0_R, .accessfn = access_tda,
+        .type = ARM_CP_CONST, .resetvalue = cpu->dbgdidr,
     };
 
     /* Note that all these register fields hold "number of Xs minus 1". */
@@ -4130,13 +4151,13 @@ static void define_debug_regs(ARMCPU *cpu)
         ARMCPRegInfo dbgregs[] = {
             { .name = "DBGBVR", .state = ARM_CP_STATE_BOTH,
               .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = i, .opc2 = 4,
-              .access = PL1_RW,
+              .access = PL1_RW, .accessfn = access_tda,
               .fieldoffset = offsetof(CPUARMState, cp15.dbgbvr[i]),
               .writefn = dbgbvr_write, .raw_writefn = raw_write
             },
             { .name = "DBGBCR", .state = ARM_CP_STATE_BOTH,
               .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = i, .opc2 = 5,
-              .access = PL1_RW,
+              .access = PL1_RW, .accessfn = access_tda,
               .fieldoffset = offsetof(CPUARMState, cp15.dbgbcr[i]),
               .writefn = dbgbcr_write, .raw_writefn = raw_write
             },
@@ -4149,13 +4170,13 @@ static void define_debug_regs(ARMCPU *cpu)
         ARMCPRegInfo dbgregs[] = {
             { .name = "DBGWVR", .state = ARM_CP_STATE_BOTH,
               .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = i, .opc2 = 6,
-              .access = PL1_RW,
+              .access = PL1_RW, .accessfn = access_tda,
               .fieldoffset = offsetof(CPUARMState, cp15.dbgwvr[i]),
               .writefn = dbgwvr_write, .raw_writefn = raw_write
             },
             { .name = "DBGWCR", .state = ARM_CP_STATE_BOTH,
               .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = i, .opc2 = 7,
-              .access = PL1_RW,
+              .access = PL1_RW, .accessfn = access_tda,
               .fieldoffset = offsetof(CPUARMState, cp15.dbgwcr[i]),
               .writefn = dbgwcr_write, .raw_writefn = raw_write
             },
-- 
1.9.1

[Qemu-devel] [PATCH v2 6/6] target-arm: Report correct syndrome for FPEXC32_EL2 traps

[Peter Maydell]

If access to FPEXC32_EL2 is trapped by CPTR_EL2.TFP or CPTR_EL3.TFP,
this should be reported with a syndrome register indicating an
FP access trap, not one indicating a system register access trap.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>
---
 target-arm/cpu.h       |  5 +++++
 target-arm/helper.c    |  4 ++--
 target-arm/op_helper.c | 13 +++++++++++++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 77f9b51..1623821 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1334,6 +1334,11 @@ typedef enum CPAccessResult {
     /* As CP_ACCESS_UNCATEGORIZED, but for traps directly to EL2 or EL3 */
     CP_ACCESS_TRAP_UNCATEGORIZED_EL2 = 5,
     CP_ACCESS_TRAP_UNCATEGORIZED_EL3 = 6,
+    /* Access fails and results in an exception syndrome for an FP access,
+     * trapped directly to EL2 or EL3
+     */
+    CP_ACCESS_TRAP_FP_EL2 = 7,
+    CP_ACCESS_TRAP_FP_EL3 = 8,
 } CPAccessResult;
 
 /* Access functions for coprocessor registers. These cannot fail and
diff --git a/target-arm/helper.c b/target-arm/helper.c
index e2b7238..bb913c6 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3011,10 +3011,10 @@ static CPAccessResult fpexc32_access(CPUARMState *env, const ARMCPRegInfo *ri,
                                      bool isread)
 {
     if ((env->cp15.cptr_el[2] & CPTR_TFP) && arm_current_el(env) == 2) {
-        return CP_ACCESS_TRAP_EL2;
+        return CP_ACCESS_TRAP_FP_EL2;
     }
     if (env->cp15.cptr_el[3] & CPTR_TFP) {
-        return CP_ACCESS_TRAP_EL3;
+        return CP_ACCESS_TRAP_FP_EL3;
     }
     return CP_ACCESS_OK;
 }
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 4c0980e..049b521 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -500,6 +500,19 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome,
         target_el = 3;
         syndrome = syn_uncategorized();
         break;
+    case CP_ACCESS_TRAP_FP_EL2:
+        target_el = 2;
+        /* Since we are an implementation that takes exceptions on a trapped
+         * conditional insn only if the insn has passed its condition code
+         * check, we take the IMPDEF choice to always report CV=1 COND=0xe
+         * (which is also the required value for AArch64 traps).
+         */
+        syndrome = syn_fp_access_trap(1, 0xe, false);
+        break;
+    case CP_ACCESS_TRAP_FP_EL3:
+        target_el = 3;
+        syndrome = syn_fp_access_trap(1, 0xe, false);
+        break;
     default:
         g_assert_not_reached();
     }
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 1/6] target-arm: correct CNTFRQ access rights

[Sergey Fedorov]

On 11.02.2016 19:03, Peter Maydell wrote:
> Correct some corner cases we were getting wrong for
> CNTFRQ access rights:
>  * should UNDEF from 32-bit Secure EL1
>  * only writable from the highest implemented exception level,
>    which might not be EL1 now
>
> To clarify the code, provide a new utility function
> arm_highest_el() which returns the highest implemented
> exception level.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>

> ---
> Rewritten to use arm_highest_el() to improve clarity
> ---
>  target-arm/cpu.h    | 12 ++++++++++++
>  target-arm/helper.c | 29 ++++++++++++++++++++++++++---
>  2 files changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 5137632..afbf366 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1255,6 +1255,18 @@ static inline bool cptype_valid(int cptype)
>  #define PL1_RW (PL1_R | PL1_W)
>  #define PL0_RW (PL0_R | PL0_W)
>  
> +/* Return the highest implemented Exception Level */
> +static inline int arm_highest_el(CPUARMState *env)
> +{
> +    if (arm_feature(env, ARM_FEATURE_EL3)) {
> +        return 3;
> +    }
> +    if (arm_feature(env, ARM_FEATURE_EL2)) {
> +        return 2;
> +    }
> +    return 1;
> +}
> +
>  /* Return the current Exception Level (as per ARMv8; note that this differs
>   * from the ARMv7 Privilege Level).
>   */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 2f9db72..4d27c00 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1218,10 +1218,33 @@ static const ARMCPRegInfo v6k_cp_reginfo[] = {
>  static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo *ri,
>                                         bool isread)
>  {
> -    /* CNTFRQ: not visible from PL0 if both PL0PCTEN and PL0VCTEN are zero */
> -    if (arm_current_el(env) == 0 && !extract32(env->cp15.c14_cntkctl, 0, 2)) {
> -        return CP_ACCESS_TRAP;
> +    /* CNTFRQ: not visible from PL0 if both PL0PCTEN and PL0VCTEN are zero.
> +     * Writable only at the highest implemented exception level.
> +     */
> +    int el = arm_current_el(env);
> +
> +    switch (el) {
> +    case 0:
> +        if (!extract32(env->cp15.c14_cntkctl, 0, 2)) {
> +            return CP_ACCESS_TRAP;
> +        }
> +        break;
> +    case 1:
> +        if (!isread && ri->state == ARM_CP_STATE_AA32 &&
> +            arm_is_secure_below_el3(env)) {
> +            /* Accesses from 32-bit Secure EL1 UNDEF (*not* trap to EL3!) */
> +            return CP_ACCESS_TRAP_UNCATEGORIZED;
> +        }
> +        break;
> +    case 2:
> +    case 3:
> +        break;
>      }
> +
> +    if (!isread && el < arm_highest_el(env)) {
> +        return CP_ACCESS_TRAP_UNCATEGORIZED;
> +    }
> +
>      return CP_ACCESS_OK;
>  }
>  

Re: [Qemu-devel] [PATCH v2 2/6] target-arm: Fix handling of SCR.SMD

[Edgar E. Iglesias]

On Thu, Feb 11, 2016 at 04:03:25PM +0000, Peter Maydell wrote:
> We weren't quite implementing the handling of SCR.SMD correctly.
> The condition governing whether the SMD bit should apply only
> for NS state is "is EL3 is AArch32", not "is the current EL AArch32".
> Fix the condition, and clarify the comment both to reflect this and
> to expand slightly on what's going on for the v7-no-Virtualization case.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>



> ---
> The bit about forcing SMD to zero confused me, anyway, since I
> expected it to mean "in this function", not elsewhere...
> ---
>  target-arm/op_helper.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index bd48549..4c0980e 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -614,12 +614,14 @@ void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)
>      int cur_el = arm_current_el(env);
>      bool secure = arm_is_secure(env);
>      bool smd = env->cp15.scr_el3 & SCR_SMD;
> -    /* On ARMv8 AArch32, SMD only applies to NS state.
> -     * On ARMv7 SMD only applies to NS state 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.
> +    /* On ARMv8 with EL3 AArch64, SMD applies to both S and NS state.
> +     * On ARMv8 with EL3 AArch32, or ARMv7 with the Virtualization
> +     *  extensions, SMD only applies to NS state.
> +     * On ARMv7 without the Virtualization extensions, the SMD bit
> +     * doesn't exist, but we forbid the guest to set it to 1 in scr_write(),
> +     * so we need not special case this here.
>       */
> -    bool undef = is_a64(env) ? smd : (!secure && smd);
> +    bool undef = arm_feature(env, ARM_FEATURE_AARCH64) ? smd : smd && !secure;
>  
>      if (arm_is_psci_call(cpu, EXCP_SMC)) {
>          /* If PSCI is enabled and this looks like a valid PSCI call then
> -- 
> 1.9.1
> 

Re: [Qemu-devel] [PATCH v2 1/6] target-arm: correct CNTFRQ access rights

[Edgar E. Iglesias]

On Thu, Feb 11, 2016 at 04:03:24PM +0000, Peter Maydell wrote:
> Correct some corner cases we were getting wrong for
> CNTFRQ access rights:
>  * should UNDEF from 32-bit Secure EL1
>  * only writable from the highest implemented exception level,
>    which might not be EL1 now
> 
> To clarify the code, provide a new utility function
> arm_highest_el() which returns the highest implemented
> exception level.

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Rewritten to use arm_highest_el() to improve clarity
> ---
>  target-arm/cpu.h    | 12 ++++++++++++
>  target-arm/helper.c | 29 ++++++++++++++++++++++++++---
>  2 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 5137632..afbf366 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1255,6 +1255,18 @@ static inline bool cptype_valid(int cptype)
>  #define PL1_RW (PL1_R | PL1_W)
>  #define PL0_RW (PL0_R | PL0_W)
>  
> +/* Return the highest implemented Exception Level */
> +static inline int arm_highest_el(CPUARMState *env)
> +{
> +    if (arm_feature(env, ARM_FEATURE_EL3)) {
> +        return 3;
> +    }
> +    if (arm_feature(env, ARM_FEATURE_EL2)) {
> +        return 2;
> +    }
> +    return 1;
> +}
> +
>  /* Return the current Exception Level (as per ARMv8; note that this differs
>   * from the ARMv7 Privilege Level).
>   */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 2f9db72..4d27c00 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1218,10 +1218,33 @@ static const ARMCPRegInfo v6k_cp_reginfo[] = {
>  static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo *ri,
>                                         bool isread)
>  {
> -    /* CNTFRQ: not visible from PL0 if both PL0PCTEN and PL0VCTEN are zero */
> -    if (arm_current_el(env) == 0 && !extract32(env->cp15.c14_cntkctl, 0, 2)) {
> -        return CP_ACCESS_TRAP;
> +    /* CNTFRQ: not visible from PL0 if both PL0PCTEN and PL0VCTEN are zero.
> +     * Writable only at the highest implemented exception level.
> +     */
> +    int el = arm_current_el(env);
> +
> +    switch (el) {
> +    case 0:
> +        if (!extract32(env->cp15.c14_cntkctl, 0, 2)) {
> +            return CP_ACCESS_TRAP;
> +        }
> +        break;
> +    case 1:
> +        if (!isread && ri->state == ARM_CP_STATE_AA32 &&
> +            arm_is_secure_below_el3(env)) {
> +            /* Accesses from 32-bit Secure EL1 UNDEF (*not* trap to EL3!) */
> +            return CP_ACCESS_TRAP_UNCATEGORIZED;
> +        }
> +        break;
> +    case 2:
> +    case 3:
> +        break;
>      }
> +
> +    if (!isread && el < arm_highest_el(env)) {
> +        return CP_ACCESS_TRAP_UNCATEGORIZED;
> +    }
> +
>      return CP_ACCESS_OK;
>  }
>  
> -- 
> 1.9.1
>