All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 00/14] Various EL3 support/cleanup patches
@ 2015-05-19 18:33 Peter Maydell
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 01/14] target-arm: Add exception target el infrastructure Peter Maydell
                   ` (13 more replies)
  0 siblings, 14 replies; 36+ messages in thread
From: Peter Maydell @ 2015-05-19 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: edgar.iglesias, serge.fdrv, alex.bennee, agraf, patches

This patch series is basically the remaining patches from
Greg's EL3-trapping series, cleaned up by me based on review
comments and on various things I noticed as I went along.
(In particular it has the reworking of when we should set
exception.target_el &c that I mentioned in the comments on
patch 4 of Edgar's recent series, and fixes for all the bugs
that making that change brings to light.) I also threw in
some patches for obvious cleanup that I noticed as I was
doing this.

Greg Bellows (6):
  target-arm: Add exception target el infrastructure
  target-arm: Extend helpers to route exceptions
  target-arm: Update interrupt handling to use target EL
  target-arm: Add AArch64 CPTR registers
  target-arm: Extend FP checks to use an EL
  target-arm: Add WFx instruction trap support

Peter Maydell (8):
  target-arm: Set correct syndrome for faults on MSR DAIF*, imm
  target-arm: Move setting of exception info into tlb_fill
  target-arm: Set exception target EL in tlb_fill
  target-arm: Make raise_exception() take syndrome and target EL
  target-arm: Allow cp access functions to indicate traps to EL2 or EL3
  target-arm: Make singlestate TB flags common between AArch32/64
  target-arm: Move TB flags down to fill gap
  target-arm: Don't halt on WFI unless we don't have any work

 target-arm/cpu.c           |  78 ++++++++++++++-----
 target-arm/cpu.h           | 186 +++++++++++++++++++++++++++------------------
 target-arm/helper-a64.c    |   2 +-
 target-arm/helper.c        | 128 ++++++++++++-------------------
 target-arm/helper.h        |   2 +-
 target-arm/internals.h     |   3 +
 target-arm/op_helper.c     | 174 ++++++++++++++++++++++++++++++++++--------
 target-arm/translate-a64.c |  48 +++++++-----
 target-arm/translate.c     |  74 +++++++++++-------
 target-arm/translate.h     |  17 ++++-
 10 files changed, 465 insertions(+), 247 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 01/14] target-arm: Add exception target el infrastructure
  2015-05-19 18:33 [Qemu-devel] [PATCH 00/14] Various EL3 support/cleanup patches Peter Maydell
@ 2015-05-19 18:33 ` Peter Maydell
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 02/14] target-arm: Extend helpers to route exceptions Peter Maydell
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2015-05-19 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: edgar.iglesias, serge.fdrv, alex.bennee, agraf, patches

From: Greg Bellows <greg.bellows@linaro.org>

Add a CPU state exception target EL field that will be used for communicating
the EL to which an exception should be routed.

Add a disassembly context field for tracking the EL3 architecture needed for
determining the target exception EL.

Add a target EL argument to the generic exception helper for callers to specify
the EL to which the exception should be routed.  Extended the helper to set
the newly added CPU state exception target el.

Added a function for setting the target exception EL and updated calls to helpers
to call it.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
Acked-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Message-id: 1429722561-12651-2-git-send-email-greg.bellows@linaro.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h           |  1 +
 target-arm/helper.h        |  2 +-
 target-arm/op_helper.c     |  3 ++-
 target-arm/translate-a64.c | 34 +++++++++++++++---------
 target-arm/translate.c     | 65 ++++++++++++++++++++++++++++++----------------
 target-arm/translate.h     | 15 +++++++++++
 6 files changed, 83 insertions(+), 37 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index d4a5899..75d6c4c 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -396,6 +396,7 @@ typedef struct CPUARMState {
         uint32_t syndrome; /* AArch64 format syndrome register */
         uint32_t fsr; /* AArch32 format fault status register info */
         uint64_t vaddress; /* virtual addr associated with exception, if any */
+        uint32_t target_el; /* EL the exception should be targeted for */
         /* If we implement EL2 we will also need to store information
          * about the intermediate physical address for stage 2 faults.
          */
diff --git a/target-arm/helper.h b/target-arm/helper.h
index dec3728..fc885de 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -47,7 +47,7 @@ DEF_HELPER_FLAGS_2(usad8, TCG_CALL_NO_RWG_SE, i32, i32, i32)
 DEF_HELPER_FLAGS_3(sel_flags, TCG_CALL_NO_RWG_SE,
                    i32, i32, i32, i32)
 DEF_HELPER_2(exception_internal, void, env, i32)
-DEF_HELPER_3(exception_with_syndrome, void, env, i32, i32)
+DEF_HELPER_4(exception_with_syndrome, void, env, i32, i32, i32)
 DEF_HELPER_1(wfi, void, env)
 DEF_HELPER_1(wfe, void, env)
 DEF_HELPER_1(pre_hvc, void, env)
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 3df9c57..51b04bd 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -246,13 +246,14 @@ void HELPER(exception_internal)(CPUARMState *env, uint32_t excp)
 
 /* Raise an exception with the specified syndrome register value */
 void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp,
-                                     uint32_t syndrome)
+                                     uint32_t syndrome, uint32_t target_el)
 {
     CPUState *cs = CPU(arm_env_get_cpu(env));
 
     assert(!excp_is_internal(excp));
     cs->exception_index = excp;
     env->exception.syndrome = syndrome;
+    env->exception.target_el = target_el;
     cpu_loop_exit(cs);
 }
 
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 0b192a1..b1f44c9 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -197,12 +197,15 @@ static void gen_exception_internal(int excp)
     tcg_temp_free_i32(tcg_excp);
 }
 
-static void gen_exception(int excp, uint32_t syndrome)
+static void gen_exception(int excp, uint32_t syndrome, uint32_t target_el)
 {
     TCGv_i32 tcg_excp = tcg_const_i32(excp);
     TCGv_i32 tcg_syn = tcg_const_i32(syndrome);
+    TCGv_i32 tcg_el = tcg_const_i32(target_el);
 
-    gen_helper_exception_with_syndrome(cpu_env, tcg_excp, tcg_syn);
+    gen_helper_exception_with_syndrome(cpu_env, tcg_excp,
+                                       tcg_syn, tcg_el);
+    tcg_temp_free_i32(tcg_el);
     tcg_temp_free_i32(tcg_syn);
     tcg_temp_free_i32(tcg_excp);
 }
@@ -215,10 +218,10 @@ static void gen_exception_internal_insn(DisasContext *s, int offset, int excp)
 }
 
 static void gen_exception_insn(DisasContext *s, int offset, int excp,
-                               uint32_t syndrome)
+                               uint32_t syndrome, uint32_t target_el)
 {
     gen_a64_set_pc_im(s->pc - offset);
-    gen_exception(excp, syndrome);
+    gen_exception(excp, syndrome, target_el);
     s->is_jmp = DISAS_EXC;
 }
 
@@ -245,7 +248,8 @@ static void gen_step_complete_exception(DisasContext *s)
      * of the exception, and our syndrome information is always correct.
      */
     gen_ss_advance(s);
-    gen_exception(EXCP_UDEF, syn_swstep(s->ss_same_el, 1, s->is_ldex));
+    gen_exception(EXCP_UDEF, syn_swstep(s->ss_same_el, 1, s->is_ldex),
+                  default_exception_el(s));
     s->is_jmp = DISAS_EXC;
 }
 
@@ -292,7 +296,8 @@ static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest)
 static void unallocated_encoding(DisasContext *s)
 {
     /* Unallocated and reserved encodings are uncategorized */
-    gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized());
+    gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(),
+                       default_exception_el(s));
 }
 
 #define unsupported_encoding(s, insn)                                    \
@@ -971,7 +976,8 @@ static inline bool fp_access_check(DisasContext *s)
         return true;
     }
 
-    gen_exception_insn(s, 4, EXCP_UDEF, syn_fp_access_trap(1, 0xe, false));
+    gen_exception_insn(s, 4, EXCP_UDEF, syn_fp_access_trap(1, 0xe, false),
+                       default_exception_el(s));
     return false;
 }
 
@@ -1498,7 +1504,8 @@ static void disas_exc(DisasContext *s, uint32_t insn)
         switch (op2_ll) {
         case 1:
             gen_ss_advance(s);
-            gen_exception_insn(s, 0, EXCP_SWI, syn_aa64_svc(imm16));
+            gen_exception_insn(s, 0, EXCP_SWI, syn_aa64_svc(imm16),
+                               default_exception_el(s));
             break;
         case 2:
             if (s->current_el == 0) {
@@ -1511,7 +1518,7 @@ static void disas_exc(DisasContext *s, uint32_t insn)
             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));
+            gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16), 2);
             break;
         case 3:
             if (s->current_el == 0) {
@@ -1523,7 +1530,7 @@ static void disas_exc(DisasContext *s, uint32_t insn)
             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));
+            gen_exception_insn(s, 0, EXCP_SMC, syn_aa64_smc(imm16), 3);
             break;
         default:
             unallocated_encoding(s);
@@ -1536,7 +1543,8 @@ static void disas_exc(DisasContext *s, uint32_t insn)
             break;
         }
         /* BRK */
-        gen_exception_insn(s, 4, EXCP_BKPT, syn_aa64_bkpt(imm16));
+        gen_exception_insn(s, 4, EXCP_BKPT, syn_aa64_bkpt(imm16),
+                           default_exception_el(s));
         break;
     case 2:
         if (op2_ll != 0) {
@@ -10936,6 +10944,7 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
     dc->condjmp = 0;
 
     dc->aarch64 = 1;
+    dc->el3_is_aa64 = arm_el_is_aa64(env, 3);
     dc->thumb = 0;
     dc->bswap_code = 0;
     dc->condexec_mask = 0;
@@ -11031,7 +11040,8 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
              * bits should be zero.
              */
             assert(num_insns == 0);
-            gen_exception(EXCP_UDEF, syn_swstep(dc->ss_same_el, 0, 0));
+            gen_exception(EXCP_UDEF, syn_swstep(dc->ss_same_el, 0, 0),
+                          default_exception_el(dc));
             dc->is_jmp = DISAS_EXC;
             break;
         }
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 9116529..2bd5733 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -217,12 +217,16 @@ static void gen_exception_internal(int excp)
     tcg_temp_free_i32(tcg_excp);
 }
 
-static void gen_exception(int excp, uint32_t syndrome)
+static void gen_exception(int excp, uint32_t syndrome, uint32_t target_el)
 {
     TCGv_i32 tcg_excp = tcg_const_i32(excp);
     TCGv_i32 tcg_syn = tcg_const_i32(syndrome);
+    TCGv_i32 tcg_el = tcg_const_i32(target_el);
 
-    gen_helper_exception_with_syndrome(cpu_env, tcg_excp, tcg_syn);
+    gen_helper_exception_with_syndrome(cpu_env, tcg_excp,
+                                       tcg_syn, tcg_el);
+
+    tcg_temp_free_i32(tcg_el);
     tcg_temp_free_i32(tcg_syn);
     tcg_temp_free_i32(tcg_excp);
 }
@@ -250,7 +254,8 @@ static void gen_step_complete_exception(DisasContext *s)
      * of the exception, and our syndrome information is always correct.
      */
     gen_ss_advance(s);
-    gen_exception(EXCP_UDEF, syn_swstep(s->ss_same_el, 1, s->is_ldex));
+    gen_exception(EXCP_UDEF, syn_swstep(s->ss_same_el, 1, s->is_ldex),
+                  default_exception_el(s));
     s->is_jmp = DISAS_EXC;
 }
 
@@ -1013,11 +1018,12 @@ static void gen_exception_internal_insn(DisasContext *s, int offset, int excp)
     s->is_jmp = DISAS_JUMP;
 }
 
-static void gen_exception_insn(DisasContext *s, int offset, int excp, int syn)
+static void gen_exception_insn(DisasContext *s, int offset, int excp,
+                               int syn, uint32_t target_el)
 {
     gen_set_condexec(s);
     gen_set_pc_im(s, s->pc - offset);
-    gen_exception(excp, syn);
+    gen_exception(excp, syn, target_el);
     s->is_jmp = DISAS_JUMP;
 }
 
@@ -3040,7 +3046,8 @@ static int disas_vfp_insn(DisasContext *s, uint32_t insn)
      */
     if (!s->cpacr_fpen) {
         gen_exception_insn(s, 4, EXCP_UDEF,
-                           syn_fp_access_trap(1, 0xe, s->thumb));
+                           syn_fp_access_trap(1, 0xe, s->thumb),
+                           default_exception_el(s));
         return 0;
     }
 
@@ -4358,7 +4365,8 @@ static int disas_neon_ls_insn(DisasContext *s, uint32_t insn)
      */
     if (!s->cpacr_fpen) {
         gen_exception_insn(s, 4, EXCP_UDEF,
-                           syn_fp_access_trap(1, 0xe, s->thumb));
+                           syn_fp_access_trap(1, 0xe, s->thumb),
+                           default_exception_el(s));
         return 0;
     }
 
@@ -5096,7 +5104,8 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
      */
     if (!s->cpacr_fpen) {
         gen_exception_insn(s, 4, EXCP_UDEF,
-                           syn_fp_access_trap(1, 0xe, s->thumb));
+                           syn_fp_access_trap(1, 0xe, s->thumb),
+                           default_exception_el(s));
         return 0;
     }
 
@@ -7960,7 +7969,8 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
                 /* bkpt */
                 ARCH(5);
                 gen_exception_insn(s, 4, EXCP_BKPT,
-                                   syn_aa32_bkpt(imm16, false));
+                                   syn_aa32_bkpt(imm16, false),
+                                   default_exception_el(s));
                 break;
             case 2:
                 /* Hypervisor call (v7) */
@@ -9021,7 +9031,8 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
             break;
         default:
         illegal_op:
-            gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized());
+            gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(),
+                               default_exception_el(s));
             break;
         }
     }
@@ -10858,7 +10869,8 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
         {
             int imm8 = extract32(insn, 0, 8);
             ARCH(5);
-            gen_exception_insn(s, 2, EXCP_BKPT, syn_aa32_bkpt(imm8, true));
+            gen_exception_insn(s, 2, EXCP_BKPT, syn_aa32_bkpt(imm8, true),
+                               default_exception_el(s));
             break;
         }
 
@@ -11013,11 +11025,13 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
     }
     return;
 undef32:
-    gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized());
+    gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(),
+                       default_exception_el(s));
     return;
 illegal_op:
 undef:
-    gen_exception_insn(s, 2, EXCP_UDEF, syn_uncategorized());
+    gen_exception_insn(s, 2, EXCP_UDEF, syn_uncategorized(),
+                       default_exception_el(s));
 }
 
 /* generate intermediate code in gen_opc_buf and gen_opparam_buf for
@@ -11057,6 +11071,7 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
     dc->condjmp = 0;
 
     dc->aarch64 = 0;
+    dc->el3_is_aa64 = arm_el_is_aa64(env, 3);
     dc->thumb = ARM_TBFLAG_THUMB(tb->flags);
     dc->bswap_code = ARM_TBFLAG_BSWAP_CODE(tb->flags);
     dc->condexec_mask = (ARM_TBFLAG_CONDEXEC(tb->flags) & 0xf) << 1;
@@ -11216,7 +11231,8 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
              * bits should be zero.
              */
             assert(num_insns == 0);
-            gen_exception(EXCP_UDEF, syn_swstep(dc->ss_same_el, 0, 0));
+            gen_exception(EXCP_UDEF, syn_swstep(dc->ss_same_el, 0, 0),
+                          default_exception_el(dc));
             goto done_generating;
         }
 
@@ -11276,13 +11292,14 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
             gen_set_condexec(dc);
             if (dc->is_jmp == DISAS_SWI) {
                 gen_ss_advance(dc);
-                gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb));
+                gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb),
+                              default_exception_el(dc));
             } else if (dc->is_jmp == DISAS_HVC) {
                 gen_ss_advance(dc);
-                gen_exception(EXCP_HVC, syn_aa32_hvc(dc->svc_imm));
+                gen_exception(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2);
             } else if (dc->is_jmp == DISAS_SMC) {
                 gen_ss_advance(dc);
-                gen_exception(EXCP_SMC, syn_aa32_smc());
+                gen_exception(EXCP_SMC, syn_aa32_smc(), 3);
             } else if (dc->ss_active) {
                 gen_step_complete_exception(dc);
             } else {
@@ -11297,13 +11314,14 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
         gen_set_condexec(dc);
         if (dc->is_jmp == DISAS_SWI && !dc->condjmp) {
             gen_ss_advance(dc);
-            gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb));
+            gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb),
+                          default_exception_el(dc));
         } else if (dc->is_jmp == DISAS_HVC && !dc->condjmp) {
             gen_ss_advance(dc);
-            gen_exception(EXCP_HVC, syn_aa32_hvc(dc->svc_imm));
+            gen_exception(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2);
         } else if (dc->is_jmp == DISAS_SMC && !dc->condjmp) {
             gen_ss_advance(dc);
-            gen_exception(EXCP_SMC, syn_aa32_smc());
+            gen_exception(EXCP_SMC, syn_aa32_smc(), 3);
         } else if (dc->ss_active) {
             gen_step_complete_exception(dc);
         } else {
@@ -11341,13 +11359,14 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
             gen_helper_wfe(cpu_env);
             break;
         case DISAS_SWI:
-            gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb));
+            gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb),
+                          default_exception_el(dc));
             break;
         case DISAS_HVC:
-            gen_exception(EXCP_HVC, syn_aa32_hvc(dc->svc_imm));
+            gen_exception(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2);
             break;
         case DISAS_SMC:
-            gen_exception(EXCP_SMC, syn_aa32_smc());
+            gen_exception(EXCP_SMC, syn_aa32_smc(), 3);
             break;
         }
         if (dc->condjmp) {
diff --git a/target-arm/translate.h b/target-arm/translate.h
index 9829576..2eadcb7 100644
--- a/target-arm/translate.h
+++ b/target-arm/translate.h
@@ -23,6 +23,7 @@ typedef struct DisasContext {
     ARMMMUIdx mmu_idx; /* MMU index to use for normal loads/stores */
     bool ns;        /* Use non-secure CPREG bank on access */
     bool cpacr_fpen; /* FP enabled via CPACR.FPEN */
+    bool el3_is_aa64;  /* Flag indicating whether EL3 is AArch64 or not */
     bool vfp_enabled; /* FP enabled via FPSCR.EN */
     int vec_len;
     int vec_stride;
@@ -73,6 +74,20 @@ static inline int get_mem_index(DisasContext *s)
     return s->mmu_idx;
 }
 
+/* Function used to determine the target exception EL when otherwise not known
+ * or default.
+ */
+static inline int default_exception_el(DisasContext *s)
+{
+    /* If we are coming from secure EL0 in a system with a 32-bit EL3, then
+     * there is no secure EL1, so we route exceptions to EL3.  Otherwise,
+     * exceptions can only be routed to ELs above 1, so we target the higher of
+     * 1 or the current EL.
+     */
+    return (s->mmu_idx == ARMMMUIdx_S1SE0 && !s->el3_is_aa64)
+            ? 3 : MAX(1, s->current_el);
+}
+
 /* target-specific extra values for is_jmp */
 /* These instructions trap after executing, so the A32/T32 decoder must
  * defer them until after the conditional execution state has been updated.
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 02/14] target-arm: Extend helpers to route exceptions
  2015-05-19 18:33 [Qemu-devel] [PATCH 00/14] Various EL3 support/cleanup patches Peter Maydell
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 01/14] target-arm: Add exception target el infrastructure Peter Maydell
@ 2015-05-19 18:33 ` Peter Maydell
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 03/14] target-arm: Set correct syndrome for faults on MSR DAIF*, imm Peter Maydell
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2015-05-19 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: edgar.iglesias, serge.fdrv, alex.bennee, agraf, patches

From: Greg Bellows <greg.bellows@linaro.org>

Updated the various helper routines to set the target EL as needed using a
dedicated function.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
Acked-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Message-id: 1429722561-12651-3-git-send-email-greg.bellows@linaro.org
[PMM: Also set target_el in fault cases in access_check_cp_reg()]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/op_helper.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 51b04bd..43e3457 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -33,6 +33,20 @@ static void raise_exception(CPUARMState *env, int tt)
     cpu_loop_exit(cs);
 }
 
+static int exception_target_el(CPUARMState *env)
+{
+    int target_el = MAX(1, arm_current_el(env));
+
+    /* No such thing as secure EL1 if EL3 is aarch32, so update the target EL
+     * to EL3 in this case.
+     */
+    if (arm_is_secure(env) && !arm_el_is_aa64(env, 3) && target_el == 1) {
+        target_el = 3;
+    }
+
+    return target_el;
+}
+
 uint32_t HELPER(neon_tbl)(CPUARMState *env, uint32_t ireg, uint32_t def,
                           uint32_t rn, uint32_t maxindex)
 {
@@ -306,6 +320,7 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome)
     if (arm_feature(env, ARM_FEATURE_XSCALE) && ri->cp < 14
         && extract32(env->cp15.c15_cpar, ri->cp, 1) == 0) {
         env->exception.syndrome = syndrome;
+        env->exception.target_el = exception_target_el(env);
         raise_exception(env, EXCP_UDEF);
     }
 
@@ -318,9 +333,11 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome)
         return;
     case CP_ACCESS_TRAP:
         env->exception.syndrome = syndrome;
+        env->exception.target_el = exception_target_el(env);
         break;
     case CP_ACCESS_TRAP_UNCATEGORIZED:
         env->exception.syndrome = syn_uncategorized();
+        env->exception.target_el = exception_target_el(env);
         break;
     default:
         g_assert_not_reached();
@@ -363,6 +380,7 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
      * to catch that case at translate time.
      */
     if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UMA)) {
+        env->exception.target_el = exception_target_el(env);
         raise_exception(env, EXCP_UDEF);
     }
 
@@ -422,6 +440,7 @@ void HELPER(pre_hvc)(CPUARMState *env)
 
     if (undef) {
         env->exception.syndrome = syn_uncategorized();
+        env->exception.target_el = exception_target_el(env);
         raise_exception(env, EXCP_UDEF);
     }
 }
@@ -452,11 +471,13 @@ void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)
     } else if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
         /* In NS EL1, HCR controlled routing to EL2 has priority over SMD. */
         env->exception.syndrome = syndrome;
+        env->exception.target_el = 2;
         raise_exception(env, EXCP_HYP_TRAP);
     }
 
     if (undef) {
         env->exception.syndrome = syn_uncategorized();
+        env->exception.target_el = exception_target_el(env);
         raise_exception(env, EXCP_UDEF);
     }
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 03/14] target-arm: Set correct syndrome for faults on MSR DAIF*, imm
  2015-05-19 18:33 [Qemu-devel] [PATCH 00/14] Various EL3 support/cleanup patches Peter Maydell
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 01/14] target-arm: Add exception target el infrastructure Peter Maydell
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 02/14] target-arm: Extend helpers to route exceptions Peter Maydell
@ 2015-05-19 18:33 ` Peter Maydell
  2015-05-28  5:30   ` Edgar E. Iglesias
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 04/14] target-arm: Move setting of exception info into tlb_fill Peter Maydell
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2015-05-19 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: edgar.iglesias, serge.fdrv, alex.bennee, agraf, patches

If the SCTLR.UMA trap bit is set then attempts by EL0 to update
the PSTATE DAIF bits via "MSR DAIFSet, imm" and "MSR DAIFClr, imm"
instructions will raise an exception. We were failing to set
the syndrome information for this exception, which meant that
it would be reported as a repeat of whatever the previous
exception was. Set the correct syndrome information.
---
 target-arm/op_helper.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 43e3457..5af4a0e 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -381,6 +381,9 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
      */
     if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UMA)) {
         env->exception.target_el = exception_target_el(env);
+        env->exception.syndrome = syn_aa64_sysregtrap(0, extract32(op, 0, 3),
+                                                      extract32(op, 3, 3), 4,
+                                                      0x1f, imm, 0);
         raise_exception(env, EXCP_UDEF);
     }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 04/14] target-arm: Move setting of exception info into tlb_fill
  2015-05-19 18:33 [Qemu-devel] [PATCH 00/14] Various EL3 support/cleanup patches Peter Maydell
                   ` (2 preceding siblings ...)
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 03/14] target-arm: Set correct syndrome for faults on MSR DAIF*, imm Peter Maydell
@ 2015-05-19 18:33 ` Peter Maydell
  2015-05-28  5:32   ` Edgar E. Iglesias
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 05/14] target-arm: Set exception target EL in tlb_fill Peter Maydell
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2015-05-19 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: edgar.iglesias, serge.fdrv, alex.bennee, agraf, patches

Move the code which sets exception information out of
arm_cpu_handle_mmu_fault and into tlb_fill. tlb_fill
is the only caller which wants to raise_exception()
so it makes more sense for it to handle the whole of
the exception setup.

As part of this cleanup, move the user-mode-only
implementation function for the handle_mmu_fault CPU
method into cpu.c so we don't need to make it globally
visible, and rename the softmmu-only utility function
arm_cpu_handle_mmu_fault to arm_tlb_fill so it's clear
that it's not the same thing.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.c       | 17 +++++++++++++++++
 target-arm/cpu.h       |  2 --
 target-arm/helper.c    | 47 +++++++----------------------------------------
 target-arm/internals.h |  3 +++
 target-arm/op_helper.c | 27 +++++++++++++++++++++++++--
 5 files changed, 52 insertions(+), 44 deletions(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 3ca3fa8..798c689 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -1197,6 +1197,23 @@ static Property arm_cpu_properties[] = {
     DEFINE_PROP_END_OF_LIST()
 };
 
+#ifdef CONFIG_USER_ONLY
+static int arm_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int rw,
+                                    int mmu_idx)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+
+    env->exception.vaddress = address;
+    if (rw == 2) {
+        cs->exception_index = EXCP_PREFETCH_ABORT;
+    } else {
+        cs->exception_index = EXCP_DATA_ABORT;
+    }
+    return 1;
+}
+#endif
+
 static void arm_cpu_class_init(ObjectClass *oc, void *data)
 {
     ARMCPUClass *acc = ARM_CPU_CLASS(oc);
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 75d6c4c..6845666 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -504,8 +504,6 @@ static inline bool is_a64(CPUARMState *env)
    is returned if the signal was handled by the virtual CPU.  */
 int cpu_arm_signal_handler(int host_signum, void *pinfo,
                            void *puc);
-int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
-                             int mmu_idx);
 
 /**
  * pmccntr_sync
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 5d0f011..a9e85b9 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -4047,21 +4047,6 @@ uint32_t HELPER(rbit)(uint32_t x)
 
 #if defined(CONFIG_USER_ONLY)
 
-int arm_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int rw,
-                             int mmu_idx)
-{
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-
-    env->exception.vaddress = address;
-    if (rw == 2) {
-        cs->exception_index = EXCP_PREFETCH_ABORT;
-    } else {
-        cs->exception_index = EXCP_DATA_ABORT;
-    }
-    return 1;
-}
-
 /* These should probably raise undefined insn exceptions.  */
 void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val)
 {
@@ -5826,8 +5811,12 @@ static inline int get_phys_addr(CPUARMState *env, target_ulong address,
     }
 }
 
-int arm_cpu_handle_mmu_fault(CPUState *cs, vaddr address,
-                             int access_type, int mmu_idx)
+/* Walk the page table and (if the mapping exists) add the page
+ * to the TLB. Return 0 on success, or an ARM DFSR/IFSR fault
+ * register format value on failure.
+ */
+int arm_tlb_fill(CPUState *cs, vaddr address,
+                 int access_type, int mmu_idx)
 {
     ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;
@@ -5835,8 +5824,6 @@ int arm_cpu_handle_mmu_fault(CPUState *cs, vaddr address,
     target_ulong page_size;
     int prot;
     int ret;
-    uint32_t syn;
-    bool same_el = (arm_current_el(env) != 0);
     MemTxAttrs attrs = {};
 
     ret = get_phys_addr(env, address, access_type, mmu_idx, &phys_addr,
@@ -5850,27 +5837,7 @@ int arm_cpu_handle_mmu_fault(CPUState *cs, vaddr address,
         return 0;
     }
 
-    /* AArch64 syndrome does not have an LPAE bit */
-    syn = ret & ~(1 << 9);
-
-    /* For insn and data aborts we assume there is no instruction syndrome
-     * information; this is always true for exceptions reported to EL1.
-     */
-    if (access_type == 2) {
-        syn = syn_insn_abort(same_el, 0, 0, syn);
-        cs->exception_index = EXCP_PREFETCH_ABORT;
-    } else {
-        syn = syn_data_abort(same_el, 0, 0, 0, access_type == 1, syn);
-        if (access_type == 1 && arm_feature(env, ARM_FEATURE_V6)) {
-            ret |= (1 << 11);
-        }
-        cs->exception_index = EXCP_DATA_ABORT;
-    }
-
-    env->exception.syndrome = syn;
-    env->exception.vaddress = address;
-    env->exception.fsr = ret;
-    return 1;
+    return ret;
 }
 
 hwaddr arm_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
diff --git a/target-arm/internals.h b/target-arm/internals.h
index de0a9c1..1e5071e 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -387,4 +387,7 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type);
 void arm_handle_psci_call(ARMCPU *cpu);
 #endif
 
+/* Do a page table walk and add page to TLB if possible */
+int arm_tlb_fill(CPUState *cpu, vaddr address, int rw, int mmu_idx);
+
 #endif
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 5af4a0e..cda9767 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -80,16 +80,39 @@ void tlb_fill(CPUState *cs, target_ulong addr, int is_write, int mmu_idx,
 {
     int ret;
 
-    ret = arm_cpu_handle_mmu_fault(cs, addr, is_write, mmu_idx);
+    ret = arm_tlb_fill(cs, addr, is_write, mmu_idx);
     if (unlikely(ret)) {
         ARMCPU *cpu = ARM_CPU(cs);
         CPUARMState *env = &cpu->env;
+        uint32_t syn, exc;
+        bool same_el = (arm_current_el(env) != 0);
 
         if (retaddr) {
             /* now we have a real cpu fault */
             cpu_restore_state(cs, retaddr);
         }
-        raise_exception(env, cs->exception_index);
+
+        /* AArch64 syndrome does not have an LPAE bit */
+        syn = ret & ~(1 << 9);
+
+        /* For insn and data aborts we assume there is no instruction syndrome
+         * information; this is always true for exceptions reported to EL1.
+         */
+        if (is_write == 2) {
+            syn = syn_insn_abort(same_el, 0, 0, syn);
+            exc = EXCP_PREFETCH_ABORT;
+        } else {
+            syn = syn_data_abort(same_el, 0, 0, 0, is_write == 1, syn);
+            if (is_write == 1 && arm_feature(env, ARM_FEATURE_V6)) {
+                ret |= (1 << 11);
+            }
+            exc = EXCP_DATA_ABORT;
+        }
+
+        env->exception.syndrome = syn;
+        env->exception.vaddress = addr;
+        env->exception.fsr = ret;
+        raise_exception(env, exc);
     }
 }
 #endif
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 05/14] target-arm: Set exception target EL in tlb_fill
  2015-05-19 18:33 [Qemu-devel] [PATCH 00/14] Various EL3 support/cleanup patches Peter Maydell
                   ` (3 preceding siblings ...)
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 04/14] target-arm: Move setting of exception info into tlb_fill Peter Maydell
@ 2015-05-19 18:33 ` Peter Maydell
  2015-05-28  5:39   ` Edgar E. Iglesias
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 06/14] target-arm: Make raise_exception() take syndrome and target EL Peter Maydell
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2015-05-19 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: edgar.iglesias, serge.fdrv, alex.bennee, agraf, patches

Set the exception target EL for MMU faults in tlb_fill.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/op_helper.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index cda9767..cbb26c4 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -110,6 +110,7 @@ void tlb_fill(CPUState *cs, target_ulong addr, int is_write, int mmu_idx,
         }
 
         env->exception.syndrome = syn;
+        env->exception.target_el = exception_target_el(env);
         env->exception.vaddress = addr;
         env->exception.fsr = ret;
         raise_exception(env, exc);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 06/14] target-arm: Make raise_exception() take syndrome and target EL
  2015-05-19 18:33 [Qemu-devel] [PATCH 00/14] Various EL3 support/cleanup patches Peter Maydell
                   ` (4 preceding siblings ...)
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 05/14] target-arm: Set exception target EL in tlb_fill Peter Maydell
@ 2015-05-19 18:33 ` Peter Maydell
  2015-05-28  5:42   ` Edgar E. Iglesias
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 07/14] target-arm: Update interrupt handling to use " Peter Maydell
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2015-05-19 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: edgar.iglesias, serge.fdrv, alex.bennee, agraf, patches

Rather than making every caller of raise_exception set the
syndrome and target EL by hand, make these arguments to
raise_exception() and have that do the job.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/op_helper.c | 68 +++++++++++++++++++++-----------------------------
 1 file changed, 28 insertions(+), 40 deletions(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index cbb26c4..d693b01 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -24,12 +24,15 @@
 #define SIGNBIT (uint32_t)0x80000000
 #define SIGNBIT64 ((uint64_t)1 << 63)
 
-static void raise_exception(CPUARMState *env, int tt)
+static void raise_exception(CPUARMState *env, uint32_t excp,
+                            uint32_t syndrome, uint32_t target_el)
 {
-    ARMCPU *cpu = arm_env_get_cpu(env);
-    CPUState *cs = CPU(cpu);
+    CPUState *cs = CPU(arm_env_get_cpu(env));
 
-    cs->exception_index = tt;
+    assert(!excp_is_internal(excp));
+    cs->exception_index = excp;
+    env->exception.syndrome = syndrome;
+    env->exception.target_el = target_el;
     cpu_loop_exit(cs);
 }
 
@@ -109,11 +112,9 @@ void tlb_fill(CPUState *cs, target_ulong addr, int is_write, int mmu_idx,
             exc = EXCP_DATA_ABORT;
         }
 
-        env->exception.syndrome = syn;
-        env->exception.target_el = exception_target_el(env);
         env->exception.vaddress = addr;
         env->exception.fsr = ret;
-        raise_exception(env, exc);
+        raise_exception(env, exc, syn, exception_target_el(env));
     }
 }
 #endif
@@ -286,13 +287,7 @@ void HELPER(exception_internal)(CPUARMState *env, uint32_t excp)
 void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp,
                                      uint32_t syndrome, uint32_t target_el)
 {
-    CPUState *cs = CPU(arm_env_get_cpu(env));
-
-    assert(!excp_is_internal(excp));
-    cs->exception_index = excp;
-    env->exception.syndrome = syndrome;
-    env->exception.target_el = target_el;
-    cpu_loop_exit(cs);
+    raise_exception(env, excp, syndrome, target_el);
 }
 
 uint32_t HELPER(cpsr_read)(CPUARMState *env)
@@ -343,9 +338,7 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome)
 
     if (arm_feature(env, ARM_FEATURE_XSCALE) && ri->cp < 14
         && extract32(env->cp15.c15_cpar, ri->cp, 1) == 0) {
-        env->exception.syndrome = syndrome;
-        env->exception.target_el = exception_target_el(env);
-        raise_exception(env, EXCP_UDEF);
+        raise_exception(env, EXCP_UDEF, syndrome, exception_target_el(env));
     }
 
     if (!ri->accessfn) {
@@ -356,17 +349,15 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome)
     case CP_ACCESS_OK:
         return;
     case CP_ACCESS_TRAP:
-        env->exception.syndrome = syndrome;
-        env->exception.target_el = exception_target_el(env);
         break;
     case CP_ACCESS_TRAP_UNCATEGORIZED:
-        env->exception.syndrome = syn_uncategorized();
-        env->exception.target_el = exception_target_el(env);
+        syndrome = syn_uncategorized();
         break;
     default:
         g_assert_not_reached();
     }
-    raise_exception(env, EXCP_UDEF);
+
+    raise_exception(env, EXCP_UDEF, syndrome, exception_target_el(env));
 }
 
 void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value)
@@ -404,11 +395,10 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
      * to catch that case at translate time.
      */
     if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UMA)) {
-        env->exception.target_el = exception_target_el(env);
-        env->exception.syndrome = syn_aa64_sysregtrap(0, extract32(op, 0, 3),
-                                                      extract32(op, 3, 3), 4,
-                                                      0x1f, imm, 0);
-        raise_exception(env, EXCP_UDEF);
+        uint32_t syndrome = syn_aa64_sysregtrap(0, extract32(op, 0, 3),
+                                                extract32(op, 3, 3), 4,
+                                                0x1f, imm, 0);
+        raise_exception(env, EXCP_UDEF, syndrome, exception_target_el(env));
     }
 
     switch (op) {
@@ -466,9 +456,8 @@ void HELPER(pre_hvc)(CPUARMState *env)
     }
 
     if (undef) {
-        env->exception.syndrome = syn_uncategorized();
-        env->exception.target_el = exception_target_el(env);
-        raise_exception(env, EXCP_UDEF);
+        raise_exception(env, EXCP_UDEF, syn_uncategorized(),
+                        exception_target_el(env));
     }
 }
 
@@ -497,15 +486,12 @@ void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)
         undef = true;
     } else if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
         /* In NS EL1, HCR controlled routing to EL2 has priority over SMD. */
-        env->exception.syndrome = syndrome;
-        env->exception.target_el = 2;
-        raise_exception(env, EXCP_HYP_TRAP);
+        raise_exception(env, EXCP_HYP_TRAP, syndrome, 2);
     }
 
     if (undef) {
-        env->exception.syndrome = syn_uncategorized();
-        env->exception.target_el = exception_target_el(env);
-        raise_exception(env, EXCP_UDEF);
+        raise_exception(env, EXCP_UDEF, syn_uncategorized(),
+                        exception_target_el(env));
     }
 }
 
@@ -798,14 +784,15 @@ void arm_debug_excp_handler(CPUState *cs)
                 bool wnr = (wp_hit->flags & BP_WATCHPOINT_HIT_WRITE) != 0;
                 bool same_el = arm_debug_target_el(env) == arm_current_el(env);
 
-                env->exception.syndrome = syn_watchpoint(same_el, 0, wnr);
                 if (extended_addresses_enabled(env)) {
                     env->exception.fsr = (1 << 9) | 0x22;
                 } else {
                     env->exception.fsr = 0x2;
                 }
                 env->exception.vaddress = wp_hit->hitaddr;
-                raise_exception(env, EXCP_DATA_ABORT);
+                raise_exception(env, EXCP_DATA_ABORT,
+                                syn_watchpoint(same_el, 0, wnr),
+                                arm_debug_target_el(env));
             } else {
                 cpu_resume_from_signal(cs, NULL);
             }
@@ -813,14 +800,15 @@ void arm_debug_excp_handler(CPUState *cs)
     } else {
         if (check_breakpoints(cpu)) {
             bool same_el = (arm_debug_target_el(env) == arm_current_el(env));
-            env->exception.syndrome = syn_breakpoint(same_el);
             if (extended_addresses_enabled(env)) {
                 env->exception.fsr = (1 << 9) | 0x22;
             } else {
                 env->exception.fsr = 0x2;
             }
             /* FAR is UNKNOWN, so doesn't need setting */
-            raise_exception(env, EXCP_PREFETCH_ABORT);
+            raise_exception(env, EXCP_PREFETCH_ABORT,
+                            syn_breakpoint(same_el),
+                            arm_debug_target_el(env));
         }
     }
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 07/14] target-arm: Update interrupt handling to use target EL
  2015-05-19 18:33 [Qemu-devel] [PATCH 00/14] Various EL3 support/cleanup patches Peter Maydell
                   ` (5 preceding siblings ...)
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 06/14] target-arm: Make raise_exception() take syndrome and target EL Peter Maydell
@ 2015-05-19 18:33 ` Peter Maydell
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 08/14] target-arm: Allow cp access functions to indicate traps to EL2 or EL3 Peter Maydell
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2015-05-19 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: edgar.iglesias, serge.fdrv, alex.bennee, agraf, patches

From: Greg Bellows <greg.bellows@linaro.org>

Updated the interrupt handling to utilize and report through the target EL
exception field.  This includes consolidating and cleaning up code where
needed. Target EL is now calculated once in arm_cpu_exec_interrupt() and
do_interrupt was updated to use the target_el exception field.  The
necessary code from arm_excp_target_el() was merged in where needed and the
function removed.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
Acked-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Message-id: 1429722561-12651-4-git-send-email-greg.bellows@linaro.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.c        | 61 +++++++++++++++++++++++++++++++++----------------
 target-arm/cpu.h        |  7 +++---
 target-arm/helper-a64.c |  2 +-
 target-arm/helper.c     | 41 ++++-----------------------------
 4 files changed, 50 insertions(+), 61 deletions(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 798c689..4a888ab 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -206,31 +206,52 @@ static void arm_cpu_reset(CPUState *s)
 bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
 {
     CPUClass *cc = CPU_GET_CLASS(cs);
+    CPUARMState *env = cs->env_ptr;
+    uint32_t cur_el = arm_current_el(env);
+    bool secure = arm_is_secure(env);
+    uint32_t target_el;
+    uint32_t excp_idx;
     bool ret = false;
 
-    if (interrupt_request & CPU_INTERRUPT_FIQ
-        && arm_excp_unmasked(cs, EXCP_FIQ)) {
-        cs->exception_index = EXCP_FIQ;
-        cc->do_interrupt(cs);
-        ret = true;
+    if (interrupt_request & CPU_INTERRUPT_FIQ) {
+        excp_idx = EXCP_FIQ;
+        target_el = arm_phys_excp_target_el(cs, excp_idx, cur_el, secure);
+        if (arm_excp_unmasked(cs, excp_idx, target_el)) {
+            cs->exception_index = excp_idx;
+            env->exception.target_el = target_el;
+            cc->do_interrupt(cs);
+            ret = true;
+        }
     }
-    if (interrupt_request & CPU_INTERRUPT_HARD
-        && arm_excp_unmasked(cs, EXCP_IRQ)) {
-        cs->exception_index = EXCP_IRQ;
-        cc->do_interrupt(cs);
-        ret = true;
+    if (interrupt_request & CPU_INTERRUPT_HARD) {
+        excp_idx = EXCP_IRQ;
+        target_el = arm_phys_excp_target_el(cs, excp_idx, cur_el, secure);
+        if (arm_excp_unmasked(cs, excp_idx, target_el)) {
+            cs->exception_index = excp_idx;
+            env->exception.target_el = target_el;
+            cc->do_interrupt(cs);
+            ret = true;
+        }
     }
-    if (interrupt_request & CPU_INTERRUPT_VIRQ
-        && arm_excp_unmasked(cs, EXCP_VIRQ)) {
-        cs->exception_index = EXCP_VIRQ;
-        cc->do_interrupt(cs);
-        ret = true;
+    if (interrupt_request & CPU_INTERRUPT_VIRQ) {
+        excp_idx = EXCP_VIRQ;
+        target_el = 1;
+        if (arm_excp_unmasked(cs, excp_idx, target_el)) {
+            cs->exception_index = excp_idx;
+            env->exception.target_el = target_el;
+            cc->do_interrupt(cs);
+            ret = true;
+        }
     }
-    if (interrupt_request & CPU_INTERRUPT_VFIQ
-        && arm_excp_unmasked(cs, EXCP_VFIQ)) {
-        cs->exception_index = EXCP_VFIQ;
-        cc->do_interrupt(cs);
-        ret = true;
+    if (interrupt_request & CPU_INTERRUPT_VFIQ) {
+        excp_idx = EXCP_VFIQ;
+        target_el = 1;
+        if (arm_excp_unmasked(cs, excp_idx, target_el)) {
+            cs->exception_index = excp_idx;
+            env->exception.target_el = target_el;
+            cc->do_interrupt(cs);
+            ret = true;
+        }
     }
 
     return ret;
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 6845666..9119a94 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1000,7 +1000,8 @@ static inline bool access_secure_reg(CPUARMState *env)
                        (_val))
 
 void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf);
-unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx);
+uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
+                                 uint32_t cur_el, bool secure);
 
 /* Interface between CPU and Interrupt controller.  */
 void armv7m_nvic_set_pending(void *opaque, int irq);
@@ -1482,11 +1483,11 @@ 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)
+static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
+                                     unsigned int target_el)
 {
     CPUARMState *env = cs->env_ptr;
     unsigned int cur_el = arm_current_el(env);
-    unsigned int target_el = arm_excp_target_el(cs, excp_idx);
     bool secure = arm_is_secure(env);
     uint32_t scr;
     uint32_t hcr;
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 861f6fa..e30af06 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -463,7 +463,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
 {
     ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;
-    unsigned int new_el = arm_excp_target_el(cs, cs->exception_index);
+    unsigned int new_el = env->exception.target_el;
     target_ulong addr = env->cp15.vbar_el[new_el];
     unsigned int new_mode = aarch64_pstate_mode(new_el, true);
 
diff --git a/target-arm/helper.c b/target-arm/helper.c
index a9e85b9..abfd70e 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -4087,7 +4087,8 @@ 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)
+uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
+                                 uint32_t cur_el, bool secure)
 {
     return 1;
 }
@@ -4211,8 +4212,8 @@ const int8_t target_el_table[2][2][2][2][2][4] = {
 /*
  * Determine the target EL for physical exceptions
  */
-static inline uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
-                                        uint32_t cur_el, bool secure)
+uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
+                                 uint32_t cur_el, bool secure)
 {
     CPUARMState *env = cs->env_ptr;
     int rw = ((env->cp15.scr_el3 & SCR_RW) == SCR_RW);
@@ -4247,40 +4248,6 @@ static inline uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
     return target_el;
 }
 
-/*
- * Determine the target EL for a given exception type.
- */
-unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
-{
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-    unsigned int cur_el = arm_current_el(env);
-    unsigned int target_el;
-    bool secure = arm_is_secure(env);
-
-    switch (excp_idx) {
-    case EXCP_HVC:
-    case EXCP_HYP_TRAP:
-        target_el = 2;
-        break;
-    case EXCP_SMC:
-        target_el = 3;
-        break;
-    case EXCP_FIQ:
-    case EXCP_IRQ:
-        target_el = arm_phys_excp_target_el(cs, excp_idx, cur_el, secure);
-        break;
-    case EXCP_VIRQ:
-    case EXCP_VFIQ:
-        target_el = 1;
-        break;
-    default:
-        target_el = MAX(cur_el, 1);
-        break;
-    }
-    return target_el;
-}
-
 static void v7m_push(CPUARMState *env, uint32_t val)
 {
     CPUState *cs = CPU(arm_env_get_cpu(env));
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 08/14] target-arm: Allow cp access functions to indicate traps to EL2 or EL3
  2015-05-19 18:33 [Qemu-devel] [PATCH 00/14] Various EL3 support/cleanup patches Peter Maydell
                   ` (6 preceding siblings ...)
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 07/14] target-arm: Update interrupt handling to use " Peter Maydell
@ 2015-05-19 18:33 ` Peter Maydell
  2015-05-21  5:47   ` Edgar E. Iglesias
  2015-05-28 11:48   ` Edgar E. Iglesias
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 09/14] target-arm: Add AArch64 CPTR registers Peter Maydell
                   ` (5 subsequent siblings)
  13 siblings, 2 replies; 36+ messages in thread
From: Peter Maydell @ 2015-05-19 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: edgar.iglesias, serge.fdrv, alex.bennee, agraf, patches

Some coprocessor access functions will need to indicate that the
instruction should trap to EL2 or EL3 rather than the default
target exception level; add corresponding CPAccessResult enum
entries and handling code.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h       |  6 +++++-
 target-arm/op_helper.c | 14 +++++++++++++-
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 9119a94..e431372 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1252,7 +1252,8 @@ typedef enum CPAccessResult {
     /* Access fails due to a configurable trap or enable which would
      * result in a categorized exception syndrome giving information about
      * the failing instruction (ie syndrome category 0x3, 0x4, 0x5, 0x6,
-     * 0xc or 0x18).
+     * 0xc or 0x18). The exception is taken to the usual target EL (EL1 or
+     * PL1 if in EL0, otherwise to the current EL).
      */
     CP_ACCESS_TRAP = 1,
     /* Access fails and results in an exception syndrome 0x0 ("uncategorized").
@@ -1260,6 +1261,9 @@ typedef enum CPAccessResult {
      * result in this failure is specifically defined by the architecture.
      */
     CP_ACCESS_TRAP_UNCATEGORIZED = 2,
+    /* As CP_ACCESS_TRAP, but for traps directly to EL2 or EL3 */
+    CP_ACCESS_TRAP_EL2 = 3,
+    CP_ACCESS_TRAP_EL3 = 4,
 } CPAccessResult;
 
 /* Access functions for coprocessor registers. These cannot fail and
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index d693b01..5963f3b 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -335,6 +335,7 @@ void HELPER(set_user_reg)(CPUARMState *env, uint32_t regno, uint32_t val)
 void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome)
 {
     const ARMCPRegInfo *ri = rip;
+    int target_el;
 
     if (arm_feature(env, ARM_FEATURE_XSCALE) && ri->cp < 14
         && extract32(env->cp15.c15_cpar, ri->cp, 1) == 0) {
@@ -349,6 +350,17 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome)
     case CP_ACCESS_OK:
         return;
     case CP_ACCESS_TRAP:
+        target_el = exception_target_el(env);
+        break;
+    case CP_ACCESS_TRAP_EL2:
+        /* Requesting a trap to EL2 when we're in EL3 or S-EL0/1 is
+         * a bug in the access function.
+         */
+        assert(!arm_is_secure(env) && !arm_current_el(env) == 3);
+        target_el = 2;
+        break;
+    case CP_ACCESS_TRAP_EL3:
+        target_el = 3;
         break;
     case CP_ACCESS_TRAP_UNCATEGORIZED:
         syndrome = syn_uncategorized();
@@ -357,7 +369,7 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome)
         g_assert_not_reached();
     }
 
-    raise_exception(env, EXCP_UDEF, syndrome, exception_target_el(env));
+    raise_exception(env, EXCP_UDEF, syndrome, target_el);
 }
 
 void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 09/14] target-arm: Add AArch64 CPTR registers
  2015-05-19 18:33 [Qemu-devel] [PATCH 00/14] Various EL3 support/cleanup patches Peter Maydell
                   ` (7 preceding siblings ...)
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 08/14] target-arm: Allow cp access functions to indicate traps to EL2 or EL3 Peter Maydell
@ 2015-05-19 18:33 ` Peter Maydell
  2015-05-28 12:12   ` Edgar E. Iglesias
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 10/14] target-arm: Make singlestate TB flags common between AArch32/64 Peter Maydell
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2015-05-19 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: edgar.iglesias, serge.fdrv, alex.bennee, agraf, patches

From: Greg Bellows <greg.bellows@linaro.org>

Adds CPTR_EL2/3 system registers definitions and access function.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
[PMM: merge CPTR_EL2 and HCPTR definitions into a single
 def using STATE_BOTH;
 don't use readfn/writefn to implement RAZ/WI registers;
 don't use accessfn for the no-EL2 CPTR_EL2;
 fix cpacr_access logic to catch EL2 accesses to CPACR being
 trapped to EL3;
 use new CP_ACCESS_TRAP_EL[23] rather than setting
 exception.target_el directly]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h    |  5 +++++
 target-arm/helper.c | 40 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index e431372..8cc4bc9 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -197,6 +197,7 @@ typedef struct CPUARMState {
             uint64_t sctlr_el[4];
         };
         uint64_t cpacr_el1; /* Architectural feature access control register */
+        uint64_t cptr_el[4];  /* ARMv8 feature trap registers */
         uint32_t c1_xscaleauxcr; /* XScale auxiliary control register.  */
         uint64_t sder; /* Secure debug enable register. */
         uint32_t nsacr; /* Non-secure access control register. */
@@ -568,6 +569,10 @@ void pmccntr_sync(CPUARMState *env);
 #define SCTLR_AFE     (1U << 29)
 #define SCTLR_TE      (1U << 30)
 
+#define CPTR_TCPAC    (1U << 31)
+#define CPTR_TTA      (1U << 20)
+#define CPTR_TFP      (1U << 10)
+
 #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 abfd70e..1cc4993 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -592,6 +592,33 @@ static void cpacr_write(CPUARMState *env, const ARMCPRegInfo *ri,
     env->cp15.cpacr_el1 = value;
 }
 
+static CPAccessResult cpacr_access(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    if (arm_feature(env, ARM_FEATURE_V8)) {
+        /* Check if CPACR accesses are to be trapped to EL2 */
+        if (arm_current_el(env) == 1 &&
+            (env->cp15.cptr_el[2] & CPTR_TCPAC) && !arm_is_secure(env)) {
+            return CP_ACCESS_TRAP_EL2;
+        /* Check if CPACR accesses are to be trapped to EL3 */
+        } else if (arm_current_el(env) < 3 &&
+                   (env->cp15.cptr_el[3] & CPTR_TCPAC)) {
+            return CP_ACCESS_TRAP_EL3;
+        }
+    }
+
+    return CP_ACCESS_OK;
+}
+
+static CPAccessResult cptr_access(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    /* Check if CPTR accesses are set to trap to EL3 */
+    if (arm_current_el(env) == 2 && (env->cp15.cptr_el[3] & CPTR_TCPAC)) {
+        return CP_ACCESS_TRAP_EL3;
+    }
+
+    return CP_ACCESS_OK;
+}
+
 static const ARMCPRegInfo v6_cp_reginfo[] = {
     /* prefetch by MVA in v6, NOP in v7 */
     { .name = "MVA_prefetch",
@@ -614,7 +641,7 @@ static const ARMCPRegInfo v6_cp_reginfo[] = {
     { .name = "WFAR", .cp = 15, .crn = 6, .crm = 0, .opc1 = 0, .opc2 = 1,
       .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0, },
     { .name = "CPACR", .state = ARM_CP_STATE_BOTH, .opc0 = 3,
-      .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 2,
+      .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 2, .accessfn = cpacr_access,
       .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.cpacr_el1),
       .resetvalue = 0, .writefn = cpacr_write },
     REGINFO_SENTINEL
@@ -2481,6 +2508,9 @@ static const ARMCPRegInfo v8_el3_no_el2_cp_reginfo[] = {
       .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
       .access = PL2_RW,
       .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
+    { .name = "CPTR_EL2", .state = ARM_CP_STATE_BOTH,
+      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 2,
+      .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
     REGINFO_SENTINEL
 };
 
@@ -2548,6 +2578,10 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
       .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 1, .opc2 = 0,
       .access = PL3_RW, .type = ARM_CP_ALIAS,
       .fieldoffset = offsetof(CPUARMState, sp_el[2]) },
+    { .name = "CPTR_EL2", .state = ARM_CP_STATE_BOTH,
+      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 2,
+      .access = PL2_RW, .accessfn = cptr_access, .resetvalue = 0,
+      .fieldoffset = offsetof(CPUARMState, cp15.cptr_el[2]) },
     REGINFO_SENTINEL
 };
 
@@ -2609,6 +2643,10 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
       .access = PL3_RW, .writefn = vbar_write,
       .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[3]),
       .resetvalue = 0 },
+    { .name = "CPTR_EL3", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 2,
+      .access = PL3_RW, .accessfn = cptr_access, .resetvalue = 0,
+      .fieldoffset = offsetof(CPUARMState, cp15.cptr_el[3]) },
     REGINFO_SENTINEL
 };
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 10/14] target-arm: Make singlestate TB flags common between AArch32/64
  2015-05-19 18:33 [Qemu-devel] [PATCH 00/14] Various EL3 support/cleanup patches Peter Maydell
                   ` (8 preceding siblings ...)
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 09/14] target-arm: Add AArch64 CPTR registers Peter Maydell
@ 2015-05-19 18:33 ` Peter Maydell
  2015-05-28  5:51   ` Edgar E. Iglesias
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 11/14] target-arm: Extend FP checks to use an EL Peter Maydell
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2015-05-19 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: edgar.iglesias, serge.fdrv, alex.bennee, agraf, patches

Currently we keep the TB flags PSTATE_SS and SS_ACTIVE in different
bit positions for AArch64 and AArch32. Replace these separate
definitions with a single common flag in the upper part of the
flags word.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h           | 69 ++++++++++++++++++----------------------------
 target-arm/translate-a64.c |  4 +--
 2 files changed, 29 insertions(+), 44 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 8cc4bc9..8aeb8aa 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1737,6 +1737,10 @@ static inline bool arm_singlestep_active(CPUARMState *env)
 #define ARM_TBFLAG_AARCH64_STATE_MASK  (1U << ARM_TBFLAG_AARCH64_STATE_SHIFT)
 #define ARM_TBFLAG_MMUIDX_SHIFT 28
 #define ARM_TBFLAG_MMUIDX_MASK (0x7 << ARM_TBFLAG_MMUIDX_SHIFT)
+#define ARM_TBFLAG_SS_ACTIVE_SHIFT 27
+#define ARM_TBFLAG_SS_ACTIVE_MASK (1 << ARM_TBFLAG_SS_ACTIVE_SHIFT)
+#define ARM_TBFLAG_PSTATE_SS_SHIFT 26
+#define ARM_TBFLAG_PSTATE_SS_MASK (1 << ARM_TBFLAG_PSTATE_SS_SHIFT)
 
 /* Bit usage when in AArch32 state: */
 #define ARM_TBFLAG_THUMB_SHIFT      0
@@ -1753,10 +1757,6 @@ static inline bool arm_singlestep_active(CPUARMState *env)
 #define ARM_TBFLAG_BSWAP_CODE_MASK  (1 << ARM_TBFLAG_BSWAP_CODE_SHIFT)
 #define ARM_TBFLAG_CPACR_FPEN_SHIFT 17
 #define ARM_TBFLAG_CPACR_FPEN_MASK  (1 << ARM_TBFLAG_CPACR_FPEN_SHIFT)
-#define ARM_TBFLAG_SS_ACTIVE_SHIFT 18
-#define ARM_TBFLAG_SS_ACTIVE_MASK (1 << ARM_TBFLAG_SS_ACTIVE_SHIFT)
-#define ARM_TBFLAG_PSTATE_SS_SHIFT 19
-#define ARM_TBFLAG_PSTATE_SS_MASK (1 << ARM_TBFLAG_PSTATE_SS_SHIFT)
 /* We store the bottom two bits of the CPAR as TB flags and handle
  * checks on the other bits at runtime
  */
@@ -1772,16 +1772,16 @@ static inline bool arm_singlestep_active(CPUARMState *env)
 /* Bit usage when in AArch64 state */
 #define ARM_TBFLAG_AA64_FPEN_SHIFT  2
 #define ARM_TBFLAG_AA64_FPEN_MASK   (1 << ARM_TBFLAG_AA64_FPEN_SHIFT)
-#define ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT 3
-#define ARM_TBFLAG_AA64_SS_ACTIVE_MASK (1 << ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT)
-#define ARM_TBFLAG_AA64_PSTATE_SS_SHIFT 4
-#define ARM_TBFLAG_AA64_PSTATE_SS_MASK (1 << ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
 
 /* some convenience accessor macros */
 #define ARM_TBFLAG_AARCH64_STATE(F) \
     (((F) & ARM_TBFLAG_AARCH64_STATE_MASK) >> ARM_TBFLAG_AARCH64_STATE_SHIFT)
 #define ARM_TBFLAG_MMUIDX(F) \
     (((F) & ARM_TBFLAG_MMUIDX_MASK) >> ARM_TBFLAG_MMUIDX_SHIFT)
+#define ARM_TBFLAG_SS_ACTIVE(F) \
+    (((F) & ARM_TBFLAG_SS_ACTIVE_MASK) >> ARM_TBFLAG_SS_ACTIVE_SHIFT)
+#define ARM_TBFLAG_PSTATE_SS(F) \
+    (((F) & ARM_TBFLAG_PSTATE_SS_MASK) >> ARM_TBFLAG_PSTATE_SS_SHIFT)
 #define ARM_TBFLAG_THUMB(F) \
     (((F) & ARM_TBFLAG_THUMB_MASK) >> ARM_TBFLAG_THUMB_SHIFT)
 #define ARM_TBFLAG_VECLEN(F) \
@@ -1796,18 +1796,10 @@ static inline bool arm_singlestep_active(CPUARMState *env)
     (((F) & ARM_TBFLAG_BSWAP_CODE_MASK) >> ARM_TBFLAG_BSWAP_CODE_SHIFT)
 #define ARM_TBFLAG_CPACR_FPEN(F) \
     (((F) & ARM_TBFLAG_CPACR_FPEN_MASK) >> ARM_TBFLAG_CPACR_FPEN_SHIFT)
-#define ARM_TBFLAG_SS_ACTIVE(F) \
-    (((F) & ARM_TBFLAG_SS_ACTIVE_MASK) >> ARM_TBFLAG_SS_ACTIVE_SHIFT)
-#define ARM_TBFLAG_PSTATE_SS(F) \
-    (((F) & ARM_TBFLAG_PSTATE_SS_MASK) >> ARM_TBFLAG_PSTATE_SS_SHIFT)
 #define ARM_TBFLAG_XSCALE_CPAR(F) \
     (((F) & ARM_TBFLAG_XSCALE_CPAR_MASK) >> ARM_TBFLAG_XSCALE_CPAR_SHIFT)
 #define ARM_TBFLAG_AA64_FPEN(F) \
     (((F) & ARM_TBFLAG_AA64_FPEN_MASK) >> ARM_TBFLAG_AA64_FPEN_SHIFT)
-#define ARM_TBFLAG_AA64_SS_ACTIVE(F) \
-    (((F) & ARM_TBFLAG_AA64_SS_ACTIVE_MASK) >> ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT)
-#define ARM_TBFLAG_AA64_PSTATE_SS(F) \
-    (((F) & ARM_TBFLAG_AA64_PSTATE_SS_MASK) >> ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
 #define ARM_TBFLAG_NS(F) \
     (((F) & ARM_TBFLAG_NS_MASK) >> ARM_TBFLAG_NS_SHIFT)
 
@@ -1829,19 +1821,6 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
         if (fpen == 3 || (fpen == 1 && arm_current_el(env) != 0)) {
             *flags |= ARM_TBFLAG_AA64_FPEN_MASK;
         }
-        /* The SS_ACTIVE and PSTATE_SS bits correspond to the state machine
-         * states defined in the ARM ARM for software singlestep:
-         *  SS_ACTIVE   PSTATE.SS   State
-         *     0            x       Inactive (the TB flag for SS is always 0)
-         *     1            0       Active-pending
-         *     1            1       Active-not-pending
-         */
-        if (arm_singlestep_active(env)) {
-            *flags |= ARM_TBFLAG_AA64_SS_ACTIVE_MASK;
-            if (env->pstate & PSTATE_SS) {
-                *flags |= ARM_TBFLAG_AA64_PSTATE_SS_MASK;
-            }
-        }
     } else {
         *pc = env->regs[15];
         *flags = (env->thumb << ARM_TBFLAG_THUMB_SHIFT)
@@ -1859,24 +1838,30 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
         if (fpen == 3 || (fpen == 1 && arm_current_el(env) != 0)) {
             *flags |= ARM_TBFLAG_CPACR_FPEN_MASK;
         }
-        /* The SS_ACTIVE and PSTATE_SS bits correspond to the state machine
-         * states defined in the ARM ARM for software singlestep:
-         *  SS_ACTIVE   PSTATE.SS   State
-         *     0            x       Inactive (the TB flag for SS is always 0)
-         *     1            0       Active-pending
-         *     1            1       Active-not-pending
-         */
-        if (arm_singlestep_active(env)) {
-            *flags |= ARM_TBFLAG_SS_ACTIVE_MASK;
-            if (env->uncached_cpsr & PSTATE_SS) {
-                *flags |= ARM_TBFLAG_PSTATE_SS_MASK;
-            }
-        }
         *flags |= (extract32(env->cp15.c15_cpar, 0, 2)
                    << ARM_TBFLAG_XSCALE_CPAR_SHIFT);
     }
 
     *flags |= (cpu_mmu_index(env) << ARM_TBFLAG_MMUIDX_SHIFT);
+    /* The SS_ACTIVE and PSTATE_SS bits correspond to the state machine
+     * states defined in the ARM ARM for software singlestep:
+     *  SS_ACTIVE   PSTATE.SS   State
+     *     0            x       Inactive (the TB flag for SS is always 0)
+     *     1            0       Active-pending
+     *     1            1       Active-not-pending
+     */
+    if (arm_singlestep_active(env)) {
+        *flags |= ARM_TBFLAG_SS_ACTIVE_MASK;
+        if (is_a64(env)) {
+            if (env->pstate & PSTATE_SS) {
+                *flags |= ARM_TBFLAG_PSTATE_SS_MASK;
+            }
+        } else {
+            if (env->uncached_cpsr & PSTATE_SS) {
+                *flags |= ARM_TBFLAG_PSTATE_SS_MASK;
+            }
+        }
+    }
 
     *cs_base = 0;
 }
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index b1f44c9..b58778a 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -10975,8 +10975,8 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
      *   emit code to generate a software step exception
      *   end the TB
      */
-    dc->ss_active = ARM_TBFLAG_AA64_SS_ACTIVE(tb->flags);
-    dc->pstate_ss = ARM_TBFLAG_AA64_PSTATE_SS(tb->flags);
+    dc->ss_active = ARM_TBFLAG_SS_ACTIVE(tb->flags);
+    dc->pstate_ss = ARM_TBFLAG_PSTATE_SS(tb->flags);
     dc->is_ldex = false;
     dc->ss_same_el = (arm_debug_target_el(env) == dc->current_el);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 11/14] target-arm: Extend FP checks to use an EL
  2015-05-19 18:33 [Qemu-devel] [PATCH 00/14] Various EL3 support/cleanup patches Peter Maydell
                   ` (9 preceding siblings ...)
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 10/14] target-arm: Make singlestate TB flags common between AArch32/64 Peter Maydell
@ 2015-05-19 18:33 ` Peter Maydell
  2015-05-28 12:50   ` Edgar E. Iglesias
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 12/14] target-arm: Move TB flags down to fill gap Peter Maydell
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2015-05-19 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: edgar.iglesias, serge.fdrv, alex.bennee, agraf, patches

From: Greg Bellows <greg.bellows@linaro.org>

Extend the ARM disassemble context to take a target exception EL instead of a
boolean enable. This change reverses the polarity of the check making a value
of 0 indicate floating point enabled (no exception).

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
[PMM: Use a common TB flag field for AArch32 and AArch64;
 CPTR_EL2 exists in v7; CPTR_EL2 should trap for EL2 accesses;
 CPTR_EL2 should not trap for secure accesses; CPTR_EL3
 should trap for EL3 accesses; CPACR traps for secure
 accesses should trap to EL3 if EL3 is AArch32]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h           | 92 +++++++++++++++++++++++++++++++++++-----------
 target-arm/translate-a64.c |  8 ++--
 target-arm/translate.c     | 17 ++++-----
 target-arm/translate.h     |  2 +-
 4 files changed, 82 insertions(+), 37 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 8aeb8aa..647e0ba 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1741,6 +1741,9 @@ static inline bool arm_singlestep_active(CPUARMState *env)
 #define ARM_TBFLAG_SS_ACTIVE_MASK (1 << ARM_TBFLAG_SS_ACTIVE_SHIFT)
 #define ARM_TBFLAG_PSTATE_SS_SHIFT 26
 #define ARM_TBFLAG_PSTATE_SS_MASK (1 << ARM_TBFLAG_PSTATE_SS_SHIFT)
+/* Target EL if we take a floating-point-disabled exception */
+#define ARM_TBFLAG_FPEXC_EL_SHIFT 24
+#define ARM_TBFLAG_FPEXC_EL_MASK (0x3 << ARM_TBFLAG_FPEXC_EL_SHIFT)
 
 /* Bit usage when in AArch32 state: */
 #define ARM_TBFLAG_THUMB_SHIFT      0
@@ -1755,8 +1758,6 @@ static inline bool arm_singlestep_active(CPUARMState *env)
 #define ARM_TBFLAG_CONDEXEC_MASK    (0xff << ARM_TBFLAG_CONDEXEC_SHIFT)
 #define ARM_TBFLAG_BSWAP_CODE_SHIFT 16
 #define ARM_TBFLAG_BSWAP_CODE_MASK  (1 << ARM_TBFLAG_BSWAP_CODE_SHIFT)
-#define ARM_TBFLAG_CPACR_FPEN_SHIFT 17
-#define ARM_TBFLAG_CPACR_FPEN_MASK  (1 << ARM_TBFLAG_CPACR_FPEN_SHIFT)
 /* We store the bottom two bits of the CPAR as TB flags and handle
  * checks on the other bits at runtime
  */
@@ -1769,9 +1770,7 @@ static inline bool arm_singlestep_active(CPUARMState *env)
 #define ARM_TBFLAG_NS_SHIFT         22
 #define ARM_TBFLAG_NS_MASK          (1 << ARM_TBFLAG_NS_SHIFT)
 
-/* Bit usage when in AArch64 state */
-#define ARM_TBFLAG_AA64_FPEN_SHIFT  2
-#define ARM_TBFLAG_AA64_FPEN_MASK   (1 << ARM_TBFLAG_AA64_FPEN_SHIFT)
+/* Bit usage when in AArch64 state: currently we have no A64 specific bits */
 
 /* some convenience accessor macros */
 #define ARM_TBFLAG_AARCH64_STATE(F) \
@@ -1782,6 +1781,8 @@ static inline bool arm_singlestep_active(CPUARMState *env)
     (((F) & ARM_TBFLAG_SS_ACTIVE_MASK) >> ARM_TBFLAG_SS_ACTIVE_SHIFT)
 #define ARM_TBFLAG_PSTATE_SS(F) \
     (((F) & ARM_TBFLAG_PSTATE_SS_MASK) >> ARM_TBFLAG_PSTATE_SS_SHIFT)
+#define ARM_TBFLAG_FPEXC_EL(F) \
+    (((F) & ARM_TBFLAG_FPEXC_EL_MASK) >> ARM_TBFLAG_FPEXC_EL_SHIFT)
 #define ARM_TBFLAG_THUMB(F) \
     (((F) & ARM_TBFLAG_THUMB_MASK) >> ARM_TBFLAG_THUMB_SHIFT)
 #define ARM_TBFLAG_VECLEN(F) \
@@ -1794,33 +1795,82 @@ static inline bool arm_singlestep_active(CPUARMState *env)
     (((F) & ARM_TBFLAG_CONDEXEC_MASK) >> ARM_TBFLAG_CONDEXEC_SHIFT)
 #define ARM_TBFLAG_BSWAP_CODE(F) \
     (((F) & ARM_TBFLAG_BSWAP_CODE_MASK) >> ARM_TBFLAG_BSWAP_CODE_SHIFT)
-#define ARM_TBFLAG_CPACR_FPEN(F) \
-    (((F) & ARM_TBFLAG_CPACR_FPEN_MASK) >> ARM_TBFLAG_CPACR_FPEN_SHIFT)
 #define ARM_TBFLAG_XSCALE_CPAR(F) \
     (((F) & ARM_TBFLAG_XSCALE_CPAR_MASK) >> ARM_TBFLAG_XSCALE_CPAR_SHIFT)
-#define ARM_TBFLAG_AA64_FPEN(F) \
-    (((F) & ARM_TBFLAG_AA64_FPEN_MASK) >> ARM_TBFLAG_AA64_FPEN_SHIFT)
 #define ARM_TBFLAG_NS(F) \
     (((F) & ARM_TBFLAG_NS_MASK) >> ARM_TBFLAG_NS_SHIFT)
 
-static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
-                                        target_ulong *cs_base, int *flags)
+/* Return the exception level to which FP-disabled exceptions should
+ * be taken, or 0 if FP is enabled.
+ */
+static inline int fp_exception_el(CPUARMState *env)
 {
     int fpen;
+    int cur_el = arm_current_el(env);
 
-    if (arm_feature(env, ARM_FEATURE_V6)) {
-        fpen = extract32(env->cp15.cpacr_el1, 20, 2);
-    } else {
-        /* CPACR doesn't exist before v6, so VFP is always accessible */
-        fpen = 3;
+    /* CPACR and the CPTR registers don't exist before v6, so FP is
+     * always accessible
+     */
+    if (!arm_feature(env, ARM_FEATURE_V6)) {
+        return 0;
+    }
+
+    /* The CPACR controls traps to EL1, or PL1 if we're 32 bit:
+     * 0, 2 : trap EL0 and EL1/PL1 accesses
+     * 1    : trap only EL0 accesses
+     * 3    : trap no accesses
+     */
+    fpen = extract32(env->cp15.cpacr_el1, 20, 2);
+    switch (fpen) {
+    case 0:
+    case 2:
+        if (cur_el == 0 || cur_el == 1) {
+            /* Trap to PL1, which might be EL1 or EL3 */
+            if (arm_is_secure(env) && !arm_el_is_aa64(env, 3)) {
+                return 3;
+            }
+            return 1;
+        }
+        if (cur_el == 3 && !is_a64(env)) {
+            /* Secure PL1 running at EL3 */
+            return 3;
+        }
+        break;
+    case 1:
+        if (cur_el == 0) {
+            return 1;
+        }
+        break;
+    case 3:
+        break;
+    }
+
+    /* For the CPTR registers we don't need to guard with an ARM_FEATURE
+     * check because zero bits in the registers mean "don't trap".
+     */
+
+    /* CPTR_EL2 : present in v7VE or v8 */
+    if (cur_el <= 2 && extract32(env->cp15.cptr_el[2], 10, 1)
+        && arm_is_secure_below_el3(env)) {
+        /* Trap FP ops at EL2, NS-EL1 or NS-EL0 to EL2 */
+        return 2;
+    }
+
+    /* CPTR_EL3 : present in v8 */
+    if (extract32(env->cp15.cptr_el[3], 10, 1)) {
+        /* Trap all FP ops to EL3 */
+        return 3;
     }
 
+    return 0;
+}
+
+static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
+                                        target_ulong *cs_base, int *flags)
+{
     if (is_a64(env)) {
         *pc = env->pc;
         *flags = ARM_TBFLAG_AARCH64_STATE_MASK;
-        if (fpen == 3 || (fpen == 1 && arm_current_el(env) != 0)) {
-            *flags |= ARM_TBFLAG_AA64_FPEN_MASK;
-        }
     } else {
         *pc = env->regs[15];
         *flags = (env->thumb << ARM_TBFLAG_THUMB_SHIFT)
@@ -1835,9 +1885,6 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
             || arm_el_is_aa64(env, 1)) {
             *flags |= ARM_TBFLAG_VFPEN_MASK;
         }
-        if (fpen == 3 || (fpen == 1 && arm_current_el(env) != 0)) {
-            *flags |= ARM_TBFLAG_CPACR_FPEN_MASK;
-        }
         *flags |= (extract32(env->cp15.c15_cpar, 0, 2)
                    << ARM_TBFLAG_XSCALE_CPAR_SHIFT);
     }
@@ -1862,6 +1909,7 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
             }
         }
     }
+    *flags |= fp_exception_el(env) << ARM_TBFLAG_FPEXC_EL_SHIFT;
 
     *cs_base = 0;
 }
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index b58778a..8d08ccd 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -412,7 +412,7 @@ static TCGv_i64 read_cpu_reg_sp(DisasContext *s, int reg, int sf)
 static inline void assert_fp_access_checked(DisasContext *s)
 {
 #ifdef CONFIG_DEBUG_TCG
-    if (unlikely(!s->fp_access_checked || !s->cpacr_fpen)) {
+    if (unlikely(!s->fp_access_checked || s->fp_excp_el)) {
         fprintf(stderr, "target-arm: FP access check missing for "
                 "instruction 0x%08x\n", s->insn);
         abort();
@@ -972,12 +972,12 @@ static inline bool fp_access_check(DisasContext *s)
     assert(!s->fp_access_checked);
     s->fp_access_checked = true;
 
-    if (s->cpacr_fpen) {
+    if (!s->fp_excp_el) {
         return true;
     }
 
     gen_exception_insn(s, 4, EXCP_UDEF, syn_fp_access_trap(1, 0xe, false),
-                       default_exception_el(s));
+                       s->fp_excp_el);
     return false;
 }
 
@@ -10954,7 +10954,7 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
 #if !defined(CONFIG_USER_ONLY)
     dc->user = (dc->current_el == 0);
 #endif
-    dc->cpacr_fpen = ARM_TBFLAG_AA64_FPEN(tb->flags);
+    dc->fp_excp_el = ARM_TBFLAG_FPEXC_EL(tb->flags);
     dc->vec_len = 0;
     dc->vec_stride = 0;
     dc->cp_regs = cpu->cp_regs;
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 2bd5733..ed2c43d 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -3044,10 +3044,9 @@ static int disas_vfp_insn(DisasContext *s, uint32_t insn)
      * for invalid encodings; we will generate incorrect syndrome information
      * for attempts to execute invalid vfp/neon encodings with FP disabled.
      */
-    if (!s->cpacr_fpen) {
+    if (s->fp_excp_el) {
         gen_exception_insn(s, 4, EXCP_UDEF,
-                           syn_fp_access_trap(1, 0xe, s->thumb),
-                           default_exception_el(s));
+                           syn_fp_access_trap(1, 0xe, s->thumb), s->fp_excp_el);
         return 0;
     }
 
@@ -4363,10 +4362,9 @@ static int disas_neon_ls_insn(DisasContext *s, uint32_t insn)
      * for invalid encodings; we will generate incorrect syndrome information
      * for attempts to execute invalid vfp/neon encodings with FP disabled.
      */
-    if (!s->cpacr_fpen) {
+    if (s->fp_excp_el) {
         gen_exception_insn(s, 4, EXCP_UDEF,
-                           syn_fp_access_trap(1, 0xe, s->thumb),
-                           default_exception_el(s));
+                           syn_fp_access_trap(1, 0xe, s->thumb), s->fp_excp_el);
         return 0;
     }
 
@@ -5102,10 +5100,9 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
      * for invalid encodings; we will generate incorrect syndrome information
      * for attempts to execute invalid vfp/neon encodings with FP disabled.
      */
-    if (!s->cpacr_fpen) {
+    if (s->fp_excp_el) {
         gen_exception_insn(s, 4, EXCP_UDEF,
-                           syn_fp_access_trap(1, 0xe, s->thumb),
-                           default_exception_el(s));
+                           syn_fp_access_trap(1, 0xe, s->thumb), s->fp_excp_el);
         return 0;
     }
 
@@ -11082,7 +11079,7 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
     dc->user = (dc->current_el == 0);
 #endif
     dc->ns = ARM_TBFLAG_NS(tb->flags);
-    dc->cpacr_fpen = ARM_TBFLAG_CPACR_FPEN(tb->flags);
+    dc->fp_excp_el = ARM_TBFLAG_FPEXC_EL(tb->flags);
     dc->vfp_enabled = ARM_TBFLAG_VFPEN(tb->flags);
     dc->vec_len = ARM_TBFLAG_VECLEN(tb->flags);
     dc->vec_stride = ARM_TBFLAG_VECSTRIDE(tb->flags);
diff --git a/target-arm/translate.h b/target-arm/translate.h
index 2eadcb7..bcdcf11 100644
--- a/target-arm/translate.h
+++ b/target-arm/translate.h
@@ -22,7 +22,7 @@ typedef struct DisasContext {
 #endif
     ARMMMUIdx mmu_idx; /* MMU index to use for normal loads/stores */
     bool ns;        /* Use non-secure CPREG bank on access */
-    bool cpacr_fpen; /* FP enabled via CPACR.FPEN */
+    int fp_excp_el; /* FP exception EL or 0 if enabled */
     bool el3_is_aa64;  /* Flag indicating whether EL3 is AArch64 or not */
     bool vfp_enabled; /* FP enabled via FPSCR.EN */
     int vec_len;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 12/14] target-arm: Move TB flags down to fill gap
  2015-05-19 18:33 [Qemu-devel] [PATCH 00/14] Various EL3 support/cleanup patches Peter Maydell
                   ` (10 preceding siblings ...)
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 11/14] target-arm: Extend FP checks to use an EL Peter Maydell
@ 2015-05-19 18:33 ` Peter Maydell
  2015-05-28  5:53   ` Edgar E. Iglesias
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 13/14] target-arm: Don't halt on WFI unless we don't have any work Peter Maydell
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 14/14] target-arm: Add WFx instruction trap support Peter Maydell
  13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2015-05-19 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: edgar.iglesias, serge.fdrv, alex.bennee, agraf, patches

Deleting the now-unused ARM_TBFLAG_CPACR_FPEN left a gap in the
bit usage; move the following ARM_TBFLAG_XSCALE_CPAR and
ARM_TBFLAG_NS_SHIFT down 3 bits to fill the gap.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 647e0ba..dd7a90b 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1761,13 +1761,13 @@ static inline bool arm_singlestep_active(CPUARMState *env)
 /* We store the bottom two bits of the CPAR as TB flags and handle
  * checks on the other bits at runtime
  */
-#define ARM_TBFLAG_XSCALE_CPAR_SHIFT 20
+#define ARM_TBFLAG_XSCALE_CPAR_SHIFT 17
 #define ARM_TBFLAG_XSCALE_CPAR_MASK (3 << ARM_TBFLAG_XSCALE_CPAR_SHIFT)
 /* Indicates whether cp register reads and writes by guest code should access
  * the secure or nonsecure bank of banked registers; note that this is not
  * the same thing as the current security state of the processor!
  */
-#define ARM_TBFLAG_NS_SHIFT         22
+#define ARM_TBFLAG_NS_SHIFT         19
 #define ARM_TBFLAG_NS_MASK          (1 << ARM_TBFLAG_NS_SHIFT)
 
 /* Bit usage when in AArch64 state: currently we have no A64 specific bits */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 13/14] target-arm: Don't halt on WFI unless we don't have any work
  2015-05-19 18:33 [Qemu-devel] [PATCH 00/14] Various EL3 support/cleanup patches Peter Maydell
                   ` (11 preceding siblings ...)
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 12/14] target-arm: Move TB flags down to fill gap Peter Maydell
@ 2015-05-19 18:33 ` Peter Maydell
  2015-05-28  5:55   ` Edgar E. Iglesias
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 14/14] target-arm: Add WFx instruction trap support Peter Maydell
  13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2015-05-19 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: edgar.iglesias, serge.fdrv, alex.bennee, agraf, patches

Just NOP the WFI instruction if we have work to do.
This doesn't make much difference currently (though it does avoid
jumping out to the top level loop and immediately restarting),
but the distinction between "halt" and "don't halt" will become
more important when the decision to halt requires us to trap
to a higher exception level instead.

Suggested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/op_helper.c     | 7 +++++++
 target-arm/translate-a64.c | 4 ++++
 target-arm/translate.c     | 4 ++++
 3 files changed, 15 insertions(+)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 5963f3b..517dacc 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -252,6 +252,13 @@ void HELPER(wfi)(CPUARMState *env)
 {
     CPUState *cs = CPU(arm_env_get_cpu(env));
 
+    if (cpu_has_work(cs)) {
+        /* Don't bother to go into our "low power state" if
+         * we would just wake up immediately.
+         */
+        return;
+    }
+
     cs->exception_index = EXCP_HLT;
     cs->halted = 1;
     cpu_loop_exit(cs);
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 8d08ccd..ffa6cb8 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -11113,6 +11113,10 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
              */
             gen_a64_set_pc_im(dc->pc);
             gen_helper_wfi(cpu_env);
+            /* The helper doesn't necessarily throw an exception, but we
+             * must go back to the main loop to check for interrupts anyway.
+             */
+            tcg_gen_exit_tb(0);
             break;
         }
     }
diff --git a/target-arm/translate.c b/target-arm/translate.c
index ed2c43d..6493b9a 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -11351,6 +11351,10 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
             break;
         case DISAS_WFI:
             gen_helper_wfi(cpu_env);
+            /* The helper doesn't necessarily throw an exception, but we
+             * must go back to the main loop to check for interrupts anyway.
+             */
+            tcg_gen_exit_tb(0);
             break;
         case DISAS_WFE:
             gen_helper_wfe(cpu_env);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PATCH 14/14] target-arm: Add WFx instruction trap support
  2015-05-19 18:33 [Qemu-devel] [PATCH 00/14] Various EL3 support/cleanup patches Peter Maydell
                   ` (12 preceding siblings ...)
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 13/14] target-arm: Don't halt on WFI unless we don't have any work Peter Maydell
@ 2015-05-19 18:33 ` Peter Maydell
  2015-05-28  5:59   ` Edgar E. Iglesias
  13 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2015-05-19 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: edgar.iglesias, serge.fdrv, alex.bennee, agraf, patches

From: Greg Bellows <greg.bellows@linaro.org>

Add support for trapping WFI and WFE instructions to the proper EL when
SCTLR/SCR/HCR settings apply.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
[PMM: removed unnecessary tweaking of syn_wfx() prototype;
 use raise_exception();
 don't trap on WFE (and add comment explaining why not);
 remove unnecessary ARM_FEATURE checks;
 trap to EL3, not EL1, if in S-EL0 and SCTLR check fires]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/op_helper.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 517dacc..c5ba9f0 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -248,9 +248,60 @@ uint32_t HELPER(usat16)(CPUARMState *env, uint32_t x, uint32_t shift)
     return res;
 }
 
+/* Function checks whether WFx (WFI/WFE) instructions are set up to be trapped.
+ * The function returns the target EL (1-3) if the instruction is to be trapped;
+ * otherwise it returns 0 indicating it is not trapped.
+ */
+static inline int check_wfx_trap(CPUARMState *env, bool is_wfe)
+{
+    int cur_el = arm_current_el(env);
+    uint64_t mask;
+
+    /* If we are currently in EL0 then we need to check if SCTLR is set up for
+     * WFx instructions being trapped to EL1. These trap bits don't exist in v7.
+     */
+    if (cur_el < 1 && arm_feature(env, ARM_FEATURE_V8)) {
+        int target_el;
+
+        mask = is_wfe ? SCTLR_nTWE : SCTLR_nTWI;
+        if (arm_is_secure_below_el3(env) && !arm_el_is_aa64(env, 3)) {
+            /* Secure EL0 and Secure PL1 is at EL3 */
+            target_el = 3;
+        } else {
+            target_el = 1;
+        }
+
+        if (!(env->cp15.sctlr_el[target_el] & mask)) {
+            return target_el;
+        }
+    }
+
+    /* We are not trapping to EL1; trap to EL2 if HCR_EL2 requires it
+     * No need for ARM_FEATURE check as if HCR_EL2 doesn't exist the
+     * bits will be zero indicating no trap.
+     */
+    if (cur_el < 2 && !arm_is_secure(env)) {
+        mask = (is_wfe) ? HCR_TWE : HCR_TWI;
+        if (env->cp15.hcr_el2 & mask) {
+            return 2;
+        }
+    }
+
+    /* We are not trapping to EL1 or EL2; trap to EL3 if SCR_EL3 requires it */
+    if (cur_el < 3) {
+        mask = (is_wfe) ? SCR_TWE : SCR_TWI;
+        if (env->cp15.scr_el3 & mask) {
+            return 3;
+        }
+    }
+
+    return 0;
+}
+
 void HELPER(wfi)(CPUARMState *env)
 {
     CPUState *cs = CPU(arm_env_get_cpu(env));
+    int target_el = check_wfx_trap(env, false);
 
     if (cpu_has_work(cs)) {
         /* Don't bother to go into our "low power state" if
@@ -259,6 +310,11 @@ void HELPER(wfi)(CPUARMState *env)
         return;
     }
 
+    if (target_el) {
+        env->pc -= 4;
+        raise_exception(env, EXCP_UDEF, syn_wfx(1, 0xe, 0), target_el);
+    }
+
     cs->exception_index = EXCP_HLT;
     cs->halted = 1;
     cpu_loop_exit(cs);
@@ -269,7 +325,9 @@ void HELPER(wfe)(CPUARMState *env)
     CPUState *cs = CPU(arm_env_get_cpu(env));
 
     /* Don't actually halt the CPU, just yield back to top
-     * level loop
+     * level loop. This is not going into a "low power state"
+     * (ie halting until some event occurs), so we never take
+     * a configurable trap to a different exception level.
      */
     cs->exception_index = EXCP_YIELD;
     cpu_loop_exit(cs);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 08/14] target-arm: Allow cp access functions to indicate traps to EL2 or EL3
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 08/14] target-arm: Allow cp access functions to indicate traps to EL2 or EL3 Peter Maydell
@ 2015-05-21  5:47   ` Edgar E. Iglesias
  2015-05-21  7:04     ` Peter Maydell
  2015-05-28 11:48   ` Edgar E. Iglesias
  1 sibling, 1 reply; 36+ messages in thread
From: Edgar E. Iglesias @ 2015-05-21  5:47 UTC (permalink / raw)
  To: Peter Maydell; +Cc: agraf, serge.fdrv, alex.bennee, qemu-devel, patches

On Tue, May 19, 2015 at 07:33:28PM +0100, Peter Maydell wrote:
> Some coprocessor access functions will need to indicate that the
> instruction should trap to EL2 or EL3 rather than the default
> target exception level; add corresponding CPAccessResult enum
> entries and handling code.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.h       |  6 +++++-
>  target-arm/op_helper.c | 14 +++++++++++++-
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 9119a94..e431372 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1252,7 +1252,8 @@ typedef enum CPAccessResult {
>      /* Access fails due to a configurable trap or enable which would
>       * result in a categorized exception syndrome giving information about
>       * the failing instruction (ie syndrome category 0x3, 0x4, 0x5, 0x6,
> -     * 0xc or 0x18).
> +     * 0xc or 0x18). The exception is taken to the usual target EL (EL1 or
> +     * PL1 if in EL0, otherwise to the current EL).
>       */
>      CP_ACCESS_TRAP = 1,
>      /* Access fails and results in an exception syndrome 0x0 ("uncategorized").
> @@ -1260,6 +1261,9 @@ typedef enum CPAccessResult {
>       * result in this failure is specifically defined by the architecture.
>       */
>      CP_ACCESS_TRAP_UNCATEGORIZED = 2,
> +    /* As CP_ACCESS_TRAP, but for traps directly to EL2 or EL3 */
> +    CP_ACCESS_TRAP_EL2 = 3,
> +    CP_ACCESS_TRAP_EL3 = 4,
>  } CPAccessResult;
>  
>  /* Access functions for coprocessor registers. These cannot fail and
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index d693b01..5963f3b 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -335,6 +335,7 @@ void HELPER(set_user_reg)(CPUARMState *env, uint32_t regno, uint32_t val)
>  void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome)
>  {
>      const ARMCPRegInfo *ri = rip;
> +    int target_el;
>  
>      if (arm_feature(env, ARM_FEATURE_XSCALE) && ri->cp < 14
>          && extract32(env->cp15.c15_cpar, ri->cp, 1) == 0) {
> @@ -349,6 +350,17 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome)
>      case CP_ACCESS_OK:
>          return;
>      case CP_ACCESS_TRAP:
> +        target_el = exception_target_el(env);
> +        break;
> +    case CP_ACCESS_TRAP_EL2:
> +        /* Requesting a trap to EL2 when we're in EL3 or S-EL0/1 is
> +         * a bug in the access function.
> +         */
> +        assert(!arm_is_secure(env) && !arm_current_el(env) == 3);
> +        target_el = 2;
> +        break;
> +    case CP_ACCESS_TRAP_EL3:
> +        target_el = 3;
>          break;
>      case CP_ACCESS_TRAP_UNCATEGORIZED:
>          syndrome = syn_uncategorized();

It looks like you need a:
target_el = exception_target_el(env);

here, no?


> @@ -357,7 +369,7 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome)
>          g_assert_not_reached();
>      }
>  
> -    raise_exception(env, EXCP_UDEF, syndrome, exception_target_el(env));
> +    raise_exception(env, EXCP_UDEF, syndrome, target_el);
>  }
>  
>  void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value)
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 08/14] target-arm: Allow cp access functions to indicate traps to EL2 or EL3
  2015-05-21  5:47   ` Edgar E. Iglesias
@ 2015-05-21  7:04     ` Peter Maydell
  0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2015-05-21  7:04 UTC (permalink / raw)
  To: Edgar E. Iglesias
  Cc: Alexander Graf, Sergey Fedorov, Alex Bennée,
	QEMU Developers, Patch Tracking

On 21 May 2015 at 06:47, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote:
> On Tue, May 19, 2015 at 07:33:28PM +0100, Peter Maydell wrote:
>>      case CP_ACCESS_TRAP_UNCATEGORIZED:
>>          syndrome = syn_uncategorized();
>
> It looks like you need a:
> target_el = exception_target_el(env);
>
> here, no?

Oops, yes.

-- PMM

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 03/14] target-arm: Set correct syndrome for faults on MSR DAIF*, imm
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 03/14] target-arm: Set correct syndrome for faults on MSR DAIF*, imm Peter Maydell
@ 2015-05-28  5:30   ` Edgar E. Iglesias
  2015-05-28  8:30     ` Peter Maydell
  0 siblings, 1 reply; 36+ messages in thread
From: Edgar E. Iglesias @ 2015-05-28  5:30 UTC (permalink / raw)
  To: Peter Maydell; +Cc: agraf, serge.fdrv, alex.bennee, qemu-devel, patches

On Tue, May 19, 2015 at 07:33:23PM +0100, Peter Maydell wrote:
> If the SCTLR.UMA trap bit is set then attempts by EL0 to update
> the PSTATE DAIF bits via "MSR DAIFSet, imm" and "MSR DAIFClr, imm"
> instructions will raise an exception. We were failing to set
> the syndrome information for this exception, which meant that
> it would be reported as a repeat of whatever the previous
> exception was. Set the correct syndrome information.
> ---
>  target-arm/op_helper.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 43e3457..5af4a0e 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -381,6 +381,9 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
>       */
>      if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UMA)) {
>          env->exception.target_el = exception_target_el(env);
> +        env->exception.syndrome = syn_aa64_sysregtrap(0, extract32(op, 0, 3),
> +                                                      extract32(op, 3, 3), 4,
> +                                                      0x1f, imm, 0);

Did you possibly reverse the argument order of 0x1f and imm?

Cheers,
Edgar


>          raise_exception(env, EXCP_UDEF);
>      }
>  
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 04/14] target-arm: Move setting of exception info into tlb_fill
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 04/14] target-arm: Move setting of exception info into tlb_fill Peter Maydell
@ 2015-05-28  5:32   ` Edgar E. Iglesias
  0 siblings, 0 replies; 36+ messages in thread
From: Edgar E. Iglesias @ 2015-05-28  5:32 UTC (permalink / raw)
  To: Peter Maydell; +Cc: agraf, serge.fdrv, alex.bennee, qemu-devel, patches

On Tue, May 19, 2015 at 07:33:24PM +0100, Peter Maydell wrote:
> Move the code which sets exception information out of
> arm_cpu_handle_mmu_fault and into tlb_fill. tlb_fill
> is the only caller which wants to raise_exception()
> so it makes more sense for it to handle the whole of
> the exception setup.
> 
> As part of this cleanup, move the user-mode-only
> implementation function for the handle_mmu_fault CPU
> method into cpu.c so we don't need to make it globally
> visible, and rename the softmmu-only utility function
> arm_cpu_handle_mmu_fault to arm_tlb_fill so it's clear
> that it's not the same thing.

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


> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.c       | 17 +++++++++++++++++
>  target-arm/cpu.h       |  2 --
>  target-arm/helper.c    | 47 +++++++----------------------------------------
>  target-arm/internals.h |  3 +++
>  target-arm/op_helper.c | 27 +++++++++++++++++++++++++--
>  5 files changed, 52 insertions(+), 44 deletions(-)
> 
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 3ca3fa8..798c689 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -1197,6 +1197,23 @@ static Property arm_cpu_properties[] = {
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> +#ifdef CONFIG_USER_ONLY
> +static int arm_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int rw,
> +                                    int mmu_idx)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +
> +    env->exception.vaddress = address;
> +    if (rw == 2) {
> +        cs->exception_index = EXCP_PREFETCH_ABORT;
> +    } else {
> +        cs->exception_index = EXCP_DATA_ABORT;
> +    }
> +    return 1;
> +}
> +#endif
> +
>  static void arm_cpu_class_init(ObjectClass *oc, void *data)
>  {
>      ARMCPUClass *acc = ARM_CPU_CLASS(oc);
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 75d6c4c..6845666 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -504,8 +504,6 @@ static inline bool is_a64(CPUARMState *env)
>     is returned if the signal was handled by the virtual CPU.  */
>  int cpu_arm_signal_handler(int host_signum, void *pinfo,
>                             void *puc);
> -int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
> -                             int mmu_idx);
>  
>  /**
>   * pmccntr_sync
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 5d0f011..a9e85b9 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -4047,21 +4047,6 @@ uint32_t HELPER(rbit)(uint32_t x)
>  
>  #if defined(CONFIG_USER_ONLY)
>  
> -int arm_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int rw,
> -                             int mmu_idx)
> -{
> -    ARMCPU *cpu = ARM_CPU(cs);
> -    CPUARMState *env = &cpu->env;
> -
> -    env->exception.vaddress = address;
> -    if (rw == 2) {
> -        cs->exception_index = EXCP_PREFETCH_ABORT;
> -    } else {
> -        cs->exception_index = EXCP_DATA_ABORT;
> -    }
> -    return 1;
> -}
> -
>  /* These should probably raise undefined insn exceptions.  */
>  void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val)
>  {
> @@ -5826,8 +5811,12 @@ static inline int get_phys_addr(CPUARMState *env, target_ulong address,
>      }
>  }
>  
> -int arm_cpu_handle_mmu_fault(CPUState *cs, vaddr address,
> -                             int access_type, int mmu_idx)
> +/* Walk the page table and (if the mapping exists) add the page
> + * to the TLB. Return 0 on success, or an ARM DFSR/IFSR fault
> + * register format value on failure.
> + */
> +int arm_tlb_fill(CPUState *cs, vaddr address,
> +                 int access_type, int mmu_idx)
>  {
>      ARMCPU *cpu = ARM_CPU(cs);
>      CPUARMState *env = &cpu->env;
> @@ -5835,8 +5824,6 @@ int arm_cpu_handle_mmu_fault(CPUState *cs, vaddr address,
>      target_ulong page_size;
>      int prot;
>      int ret;
> -    uint32_t syn;
> -    bool same_el = (arm_current_el(env) != 0);
>      MemTxAttrs attrs = {};
>  
>      ret = get_phys_addr(env, address, access_type, mmu_idx, &phys_addr,
> @@ -5850,27 +5837,7 @@ int arm_cpu_handle_mmu_fault(CPUState *cs, vaddr address,
>          return 0;
>      }
>  
> -    /* AArch64 syndrome does not have an LPAE bit */
> -    syn = ret & ~(1 << 9);
> -
> -    /* For insn and data aborts we assume there is no instruction syndrome
> -     * information; this is always true for exceptions reported to EL1.
> -     */
> -    if (access_type == 2) {
> -        syn = syn_insn_abort(same_el, 0, 0, syn);
> -        cs->exception_index = EXCP_PREFETCH_ABORT;
> -    } else {
> -        syn = syn_data_abort(same_el, 0, 0, 0, access_type == 1, syn);
> -        if (access_type == 1 && arm_feature(env, ARM_FEATURE_V6)) {
> -            ret |= (1 << 11);
> -        }
> -        cs->exception_index = EXCP_DATA_ABORT;
> -    }
> -
> -    env->exception.syndrome = syn;
> -    env->exception.vaddress = address;
> -    env->exception.fsr = ret;
> -    return 1;
> +    return ret;
>  }
>  
>  hwaddr arm_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index de0a9c1..1e5071e 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -387,4 +387,7 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type);
>  void arm_handle_psci_call(ARMCPU *cpu);
>  #endif
>  
> +/* Do a page table walk and add page to TLB if possible */
> +int arm_tlb_fill(CPUState *cpu, vaddr address, int rw, int mmu_idx);
> +
>  #endif
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 5af4a0e..cda9767 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -80,16 +80,39 @@ void tlb_fill(CPUState *cs, target_ulong addr, int is_write, int mmu_idx,
>  {
>      int ret;
>  
> -    ret = arm_cpu_handle_mmu_fault(cs, addr, is_write, mmu_idx);
> +    ret = arm_tlb_fill(cs, addr, is_write, mmu_idx);
>      if (unlikely(ret)) {
>          ARMCPU *cpu = ARM_CPU(cs);
>          CPUARMState *env = &cpu->env;
> +        uint32_t syn, exc;
> +        bool same_el = (arm_current_el(env) != 0);
>  
>          if (retaddr) {
>              /* now we have a real cpu fault */
>              cpu_restore_state(cs, retaddr);
>          }
> -        raise_exception(env, cs->exception_index);
> +
> +        /* AArch64 syndrome does not have an LPAE bit */
> +        syn = ret & ~(1 << 9);
> +
> +        /* For insn and data aborts we assume there is no instruction syndrome
> +         * information; this is always true for exceptions reported to EL1.
> +         */
> +        if (is_write == 2) {
> +            syn = syn_insn_abort(same_el, 0, 0, syn);
> +            exc = EXCP_PREFETCH_ABORT;
> +        } else {
> +            syn = syn_data_abort(same_el, 0, 0, 0, is_write == 1, syn);
> +            if (is_write == 1 && arm_feature(env, ARM_FEATURE_V6)) {
> +                ret |= (1 << 11);
> +            }
> +            exc = EXCP_DATA_ABORT;
> +        }
> +
> +        env->exception.syndrome = syn;
> +        env->exception.vaddress = addr;
> +        env->exception.fsr = ret;
> +        raise_exception(env, exc);
>      }
>  }
>  #endif
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 05/14] target-arm: Set exception target EL in tlb_fill
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 05/14] target-arm: Set exception target EL in tlb_fill Peter Maydell
@ 2015-05-28  5:39   ` Edgar E. Iglesias
  0 siblings, 0 replies; 36+ messages in thread
From: Edgar E. Iglesias @ 2015-05-28  5:39 UTC (permalink / raw)
  To: Peter Maydell; +Cc: agraf, serge.fdrv, alex.bennee, qemu-devel, patches

On Tue, May 19, 2015 at 07:33:25PM +0100, Peter Maydell wrote:
> Set the exception target EL for MMU faults in tlb_fill.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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


> ---
>  target-arm/op_helper.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index cda9767..cbb26c4 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -110,6 +110,7 @@ void tlb_fill(CPUState *cs, target_ulong addr, int is_write, int mmu_idx,
>          }
>  
>          env->exception.syndrome = syn;
> +        env->exception.target_el = exception_target_el(env);
>          env->exception.vaddress = addr;
>          env->exception.fsr = ret;
>          raise_exception(env, exc);
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 06/14] target-arm: Make raise_exception() take syndrome and target EL
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 06/14] target-arm: Make raise_exception() take syndrome and target EL Peter Maydell
@ 2015-05-28  5:42   ` Edgar E. Iglesias
  0 siblings, 0 replies; 36+ messages in thread
From: Edgar E. Iglesias @ 2015-05-28  5:42 UTC (permalink / raw)
  To: Peter Maydell; +Cc: agraf, serge.fdrv, alex.bennee, qemu-devel, patches

On Tue, May 19, 2015 at 07:33:26PM +0100, Peter Maydell wrote:
> Rather than making every caller of raise_exception set the
> syndrome and target EL by hand, make these arguments to
> raise_exception() and have that do the job.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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


> ---
>  target-arm/op_helper.c | 68 +++++++++++++++++++++-----------------------------
>  1 file changed, 28 insertions(+), 40 deletions(-)
> 
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index cbb26c4..d693b01 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -24,12 +24,15 @@
>  #define SIGNBIT (uint32_t)0x80000000
>  #define SIGNBIT64 ((uint64_t)1 << 63)
>  
> -static void raise_exception(CPUARMState *env, int tt)
> +static void raise_exception(CPUARMState *env, uint32_t excp,
> +                            uint32_t syndrome, uint32_t target_el)
>  {
> -    ARMCPU *cpu = arm_env_get_cpu(env);
> -    CPUState *cs = CPU(cpu);
> +    CPUState *cs = CPU(arm_env_get_cpu(env));
>  
> -    cs->exception_index = tt;
> +    assert(!excp_is_internal(excp));
> +    cs->exception_index = excp;
> +    env->exception.syndrome = syndrome;
> +    env->exception.target_el = target_el;
>      cpu_loop_exit(cs);
>  }
>  
> @@ -109,11 +112,9 @@ void tlb_fill(CPUState *cs, target_ulong addr, int is_write, int mmu_idx,
>              exc = EXCP_DATA_ABORT;
>          }
>  
> -        env->exception.syndrome = syn;
> -        env->exception.target_el = exception_target_el(env);
>          env->exception.vaddress = addr;
>          env->exception.fsr = ret;
> -        raise_exception(env, exc);
> +        raise_exception(env, exc, syn, exception_target_el(env));
>      }
>  }
>  #endif
> @@ -286,13 +287,7 @@ void HELPER(exception_internal)(CPUARMState *env, uint32_t excp)
>  void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp,
>                                       uint32_t syndrome, uint32_t target_el)
>  {
> -    CPUState *cs = CPU(arm_env_get_cpu(env));
> -
> -    assert(!excp_is_internal(excp));
> -    cs->exception_index = excp;
> -    env->exception.syndrome = syndrome;
> -    env->exception.target_el = target_el;
> -    cpu_loop_exit(cs);
> +    raise_exception(env, excp, syndrome, target_el);
>  }
>  
>  uint32_t HELPER(cpsr_read)(CPUARMState *env)
> @@ -343,9 +338,7 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome)
>  
>      if (arm_feature(env, ARM_FEATURE_XSCALE) && ri->cp < 14
>          && extract32(env->cp15.c15_cpar, ri->cp, 1) == 0) {
> -        env->exception.syndrome = syndrome;
> -        env->exception.target_el = exception_target_el(env);
> -        raise_exception(env, EXCP_UDEF);
> +        raise_exception(env, EXCP_UDEF, syndrome, exception_target_el(env));
>      }
>  
>      if (!ri->accessfn) {
> @@ -356,17 +349,15 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome)
>      case CP_ACCESS_OK:
>          return;
>      case CP_ACCESS_TRAP:
> -        env->exception.syndrome = syndrome;
> -        env->exception.target_el = exception_target_el(env);
>          break;
>      case CP_ACCESS_TRAP_UNCATEGORIZED:
> -        env->exception.syndrome = syn_uncategorized();
> -        env->exception.target_el = exception_target_el(env);
> +        syndrome = syn_uncategorized();
>          break;
>      default:
>          g_assert_not_reached();
>      }
> -    raise_exception(env, EXCP_UDEF);
> +
> +    raise_exception(env, EXCP_UDEF, syndrome, exception_target_el(env));
>  }
>  
>  void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value)
> @@ -404,11 +395,10 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
>       * to catch that case at translate time.
>       */
>      if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UMA)) {
> -        env->exception.target_el = exception_target_el(env);
> -        env->exception.syndrome = syn_aa64_sysregtrap(0, extract32(op, 0, 3),
> -                                                      extract32(op, 3, 3), 4,
> -                                                      0x1f, imm, 0);
> -        raise_exception(env, EXCP_UDEF);
> +        uint32_t syndrome = syn_aa64_sysregtrap(0, extract32(op, 0, 3),
> +                                                extract32(op, 3, 3), 4,
> +                                                0x1f, imm, 0);
> +        raise_exception(env, EXCP_UDEF, syndrome, exception_target_el(env));
>      }
>  
>      switch (op) {
> @@ -466,9 +456,8 @@ void HELPER(pre_hvc)(CPUARMState *env)
>      }
>  
>      if (undef) {
> -        env->exception.syndrome = syn_uncategorized();
> -        env->exception.target_el = exception_target_el(env);
> -        raise_exception(env, EXCP_UDEF);
> +        raise_exception(env, EXCP_UDEF, syn_uncategorized(),
> +                        exception_target_el(env));
>      }
>  }
>  
> @@ -497,15 +486,12 @@ void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)
>          undef = true;
>      } else if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
>          /* In NS EL1, HCR controlled routing to EL2 has priority over SMD. */
> -        env->exception.syndrome = syndrome;
> -        env->exception.target_el = 2;
> -        raise_exception(env, EXCP_HYP_TRAP);
> +        raise_exception(env, EXCP_HYP_TRAP, syndrome, 2);
>      }
>  
>      if (undef) {
> -        env->exception.syndrome = syn_uncategorized();
> -        env->exception.target_el = exception_target_el(env);
> -        raise_exception(env, EXCP_UDEF);
> +        raise_exception(env, EXCP_UDEF, syn_uncategorized(),
> +                        exception_target_el(env));
>      }
>  }
>  
> @@ -798,14 +784,15 @@ void arm_debug_excp_handler(CPUState *cs)
>                  bool wnr = (wp_hit->flags & BP_WATCHPOINT_HIT_WRITE) != 0;
>                  bool same_el = arm_debug_target_el(env) == arm_current_el(env);
>  
> -                env->exception.syndrome = syn_watchpoint(same_el, 0, wnr);
>                  if (extended_addresses_enabled(env)) {
>                      env->exception.fsr = (1 << 9) | 0x22;
>                  } else {
>                      env->exception.fsr = 0x2;
>                  }
>                  env->exception.vaddress = wp_hit->hitaddr;
> -                raise_exception(env, EXCP_DATA_ABORT);
> +                raise_exception(env, EXCP_DATA_ABORT,
> +                                syn_watchpoint(same_el, 0, wnr),
> +                                arm_debug_target_el(env));
>              } else {
>                  cpu_resume_from_signal(cs, NULL);
>              }
> @@ -813,14 +800,15 @@ void arm_debug_excp_handler(CPUState *cs)
>      } else {
>          if (check_breakpoints(cpu)) {
>              bool same_el = (arm_debug_target_el(env) == arm_current_el(env));
> -            env->exception.syndrome = syn_breakpoint(same_el);
>              if (extended_addresses_enabled(env)) {
>                  env->exception.fsr = (1 << 9) | 0x22;
>              } else {
>                  env->exception.fsr = 0x2;
>              }
>              /* FAR is UNKNOWN, so doesn't need setting */
> -            raise_exception(env, EXCP_PREFETCH_ABORT);
> +            raise_exception(env, EXCP_PREFETCH_ABORT,
> +                            syn_breakpoint(same_el),
> +                            arm_debug_target_el(env));
>          }
>      }
>  }
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 10/14] target-arm: Make singlestate TB flags common between AArch32/64
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 10/14] target-arm: Make singlestate TB flags common between AArch32/64 Peter Maydell
@ 2015-05-28  5:51   ` Edgar E. Iglesias
  0 siblings, 0 replies; 36+ messages in thread
From: Edgar E. Iglesias @ 2015-05-28  5:51 UTC (permalink / raw)
  To: Peter Maydell; +Cc: agraf, serge.fdrv, alex.bennee, qemu-devel, patches

On Tue, May 19, 2015 at 07:33:30PM +0100, Peter Maydell wrote:
> Currently we keep the TB flags PSTATE_SS and SS_ACTIVE in different
> bit positions for AArch64 and AArch32. Replace these separate
> definitions with a single common flag in the upper part of the
> flags word.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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


> ---
>  target-arm/cpu.h           | 69 ++++++++++++++++++----------------------------
>  target-arm/translate-a64.c |  4 +--
>  2 files changed, 29 insertions(+), 44 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 8cc4bc9..8aeb8aa 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1737,6 +1737,10 @@ static inline bool arm_singlestep_active(CPUARMState *env)
>  #define ARM_TBFLAG_AARCH64_STATE_MASK  (1U << ARM_TBFLAG_AARCH64_STATE_SHIFT)
>  #define ARM_TBFLAG_MMUIDX_SHIFT 28
>  #define ARM_TBFLAG_MMUIDX_MASK (0x7 << ARM_TBFLAG_MMUIDX_SHIFT)
> +#define ARM_TBFLAG_SS_ACTIVE_SHIFT 27
> +#define ARM_TBFLAG_SS_ACTIVE_MASK (1 << ARM_TBFLAG_SS_ACTIVE_SHIFT)
> +#define ARM_TBFLAG_PSTATE_SS_SHIFT 26
> +#define ARM_TBFLAG_PSTATE_SS_MASK (1 << ARM_TBFLAG_PSTATE_SS_SHIFT)
>  
>  /* Bit usage when in AArch32 state: */
>  #define ARM_TBFLAG_THUMB_SHIFT      0
> @@ -1753,10 +1757,6 @@ static inline bool arm_singlestep_active(CPUARMState *env)
>  #define ARM_TBFLAG_BSWAP_CODE_MASK  (1 << ARM_TBFLAG_BSWAP_CODE_SHIFT)
>  #define ARM_TBFLAG_CPACR_FPEN_SHIFT 17
>  #define ARM_TBFLAG_CPACR_FPEN_MASK  (1 << ARM_TBFLAG_CPACR_FPEN_SHIFT)
> -#define ARM_TBFLAG_SS_ACTIVE_SHIFT 18
> -#define ARM_TBFLAG_SS_ACTIVE_MASK (1 << ARM_TBFLAG_SS_ACTIVE_SHIFT)
> -#define ARM_TBFLAG_PSTATE_SS_SHIFT 19
> -#define ARM_TBFLAG_PSTATE_SS_MASK (1 << ARM_TBFLAG_PSTATE_SS_SHIFT)
>  /* We store the bottom two bits of the CPAR as TB flags and handle
>   * checks on the other bits at runtime
>   */
> @@ -1772,16 +1772,16 @@ static inline bool arm_singlestep_active(CPUARMState *env)
>  /* Bit usage when in AArch64 state */
>  #define ARM_TBFLAG_AA64_FPEN_SHIFT  2
>  #define ARM_TBFLAG_AA64_FPEN_MASK   (1 << ARM_TBFLAG_AA64_FPEN_SHIFT)
> -#define ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT 3
> -#define ARM_TBFLAG_AA64_SS_ACTIVE_MASK (1 << ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT)
> -#define ARM_TBFLAG_AA64_PSTATE_SS_SHIFT 4
> -#define ARM_TBFLAG_AA64_PSTATE_SS_MASK (1 << ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
>  
>  /* some convenience accessor macros */
>  #define ARM_TBFLAG_AARCH64_STATE(F) \
>      (((F) & ARM_TBFLAG_AARCH64_STATE_MASK) >> ARM_TBFLAG_AARCH64_STATE_SHIFT)
>  #define ARM_TBFLAG_MMUIDX(F) \
>      (((F) & ARM_TBFLAG_MMUIDX_MASK) >> ARM_TBFLAG_MMUIDX_SHIFT)
> +#define ARM_TBFLAG_SS_ACTIVE(F) \
> +    (((F) & ARM_TBFLAG_SS_ACTIVE_MASK) >> ARM_TBFLAG_SS_ACTIVE_SHIFT)
> +#define ARM_TBFLAG_PSTATE_SS(F) \
> +    (((F) & ARM_TBFLAG_PSTATE_SS_MASK) >> ARM_TBFLAG_PSTATE_SS_SHIFT)
>  #define ARM_TBFLAG_THUMB(F) \
>      (((F) & ARM_TBFLAG_THUMB_MASK) >> ARM_TBFLAG_THUMB_SHIFT)
>  #define ARM_TBFLAG_VECLEN(F) \
> @@ -1796,18 +1796,10 @@ static inline bool arm_singlestep_active(CPUARMState *env)
>      (((F) & ARM_TBFLAG_BSWAP_CODE_MASK) >> ARM_TBFLAG_BSWAP_CODE_SHIFT)
>  #define ARM_TBFLAG_CPACR_FPEN(F) \
>      (((F) & ARM_TBFLAG_CPACR_FPEN_MASK) >> ARM_TBFLAG_CPACR_FPEN_SHIFT)
> -#define ARM_TBFLAG_SS_ACTIVE(F) \
> -    (((F) & ARM_TBFLAG_SS_ACTIVE_MASK) >> ARM_TBFLAG_SS_ACTIVE_SHIFT)
> -#define ARM_TBFLAG_PSTATE_SS(F) \
> -    (((F) & ARM_TBFLAG_PSTATE_SS_MASK) >> ARM_TBFLAG_PSTATE_SS_SHIFT)
>  #define ARM_TBFLAG_XSCALE_CPAR(F) \
>      (((F) & ARM_TBFLAG_XSCALE_CPAR_MASK) >> ARM_TBFLAG_XSCALE_CPAR_SHIFT)
>  #define ARM_TBFLAG_AA64_FPEN(F) \
>      (((F) & ARM_TBFLAG_AA64_FPEN_MASK) >> ARM_TBFLAG_AA64_FPEN_SHIFT)
> -#define ARM_TBFLAG_AA64_SS_ACTIVE(F) \
> -    (((F) & ARM_TBFLAG_AA64_SS_ACTIVE_MASK) >> ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT)
> -#define ARM_TBFLAG_AA64_PSTATE_SS(F) \
> -    (((F) & ARM_TBFLAG_AA64_PSTATE_SS_MASK) >> ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
>  #define ARM_TBFLAG_NS(F) \
>      (((F) & ARM_TBFLAG_NS_MASK) >> ARM_TBFLAG_NS_SHIFT)
>  
> @@ -1829,19 +1821,6 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
>          if (fpen == 3 || (fpen == 1 && arm_current_el(env) != 0)) {
>              *flags |= ARM_TBFLAG_AA64_FPEN_MASK;
>          }
> -        /* The SS_ACTIVE and PSTATE_SS bits correspond to the state machine
> -         * states defined in the ARM ARM for software singlestep:
> -         *  SS_ACTIVE   PSTATE.SS   State
> -         *     0            x       Inactive (the TB flag for SS is always 0)
> -         *     1            0       Active-pending
> -         *     1            1       Active-not-pending
> -         */
> -        if (arm_singlestep_active(env)) {
> -            *flags |= ARM_TBFLAG_AA64_SS_ACTIVE_MASK;
> -            if (env->pstate & PSTATE_SS) {
> -                *flags |= ARM_TBFLAG_AA64_PSTATE_SS_MASK;
> -            }
> -        }
>      } else {
>          *pc = env->regs[15];
>          *flags = (env->thumb << ARM_TBFLAG_THUMB_SHIFT)
> @@ -1859,24 +1838,30 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
>          if (fpen == 3 || (fpen == 1 && arm_current_el(env) != 0)) {
>              *flags |= ARM_TBFLAG_CPACR_FPEN_MASK;
>          }
> -        /* The SS_ACTIVE and PSTATE_SS bits correspond to the state machine
> -         * states defined in the ARM ARM for software singlestep:
> -         *  SS_ACTIVE   PSTATE.SS   State
> -         *     0            x       Inactive (the TB flag for SS is always 0)
> -         *     1            0       Active-pending
> -         *     1            1       Active-not-pending
> -         */
> -        if (arm_singlestep_active(env)) {
> -            *flags |= ARM_TBFLAG_SS_ACTIVE_MASK;
> -            if (env->uncached_cpsr & PSTATE_SS) {
> -                *flags |= ARM_TBFLAG_PSTATE_SS_MASK;
> -            }
> -        }
>          *flags |= (extract32(env->cp15.c15_cpar, 0, 2)
>                     << ARM_TBFLAG_XSCALE_CPAR_SHIFT);
>      }
>  
>      *flags |= (cpu_mmu_index(env) << ARM_TBFLAG_MMUIDX_SHIFT);
> +    /* The SS_ACTIVE and PSTATE_SS bits correspond to the state machine
> +     * states defined in the ARM ARM for software singlestep:
> +     *  SS_ACTIVE   PSTATE.SS   State
> +     *     0            x       Inactive (the TB flag for SS is always 0)
> +     *     1            0       Active-pending
> +     *     1            1       Active-not-pending
> +     */
> +    if (arm_singlestep_active(env)) {
> +        *flags |= ARM_TBFLAG_SS_ACTIVE_MASK;
> +        if (is_a64(env)) {
> +            if (env->pstate & PSTATE_SS) {
> +                *flags |= ARM_TBFLAG_PSTATE_SS_MASK;
> +            }
> +        } else {
> +            if (env->uncached_cpsr & PSTATE_SS) {
> +                *flags |= ARM_TBFLAG_PSTATE_SS_MASK;
> +            }
> +        }
> +    }
>  
>      *cs_base = 0;
>  }
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index b1f44c9..b58778a 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -10975,8 +10975,8 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
>       *   emit code to generate a software step exception
>       *   end the TB
>       */
> -    dc->ss_active = ARM_TBFLAG_AA64_SS_ACTIVE(tb->flags);
> -    dc->pstate_ss = ARM_TBFLAG_AA64_PSTATE_SS(tb->flags);
> +    dc->ss_active = ARM_TBFLAG_SS_ACTIVE(tb->flags);
> +    dc->pstate_ss = ARM_TBFLAG_PSTATE_SS(tb->flags);
>      dc->is_ldex = false;
>      dc->ss_same_el = (arm_debug_target_el(env) == dc->current_el);
>  
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 12/14] target-arm: Move TB flags down to fill gap
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 12/14] target-arm: Move TB flags down to fill gap Peter Maydell
@ 2015-05-28  5:53   ` Edgar E. Iglesias
  0 siblings, 0 replies; 36+ messages in thread
From: Edgar E. Iglesias @ 2015-05-28  5:53 UTC (permalink / raw)
  To: Peter Maydell; +Cc: agraf, serge.fdrv, alex.bennee, qemu-devel, patches

On Tue, May 19, 2015 at 07:33:32PM +0100, Peter Maydell wrote:
> Deleting the now-unused ARM_TBFLAG_CPACR_FPEN left a gap in the
> bit usage; move the following ARM_TBFLAG_XSCALE_CPAR and
> ARM_TBFLAG_NS_SHIFT down 3 bits to fill the gap.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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


> ---
>  target-arm/cpu.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 647e0ba..dd7a90b 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1761,13 +1761,13 @@ static inline bool arm_singlestep_active(CPUARMState *env)
>  /* We store the bottom two bits of the CPAR as TB flags and handle
>   * checks on the other bits at runtime
>   */
> -#define ARM_TBFLAG_XSCALE_CPAR_SHIFT 20
> +#define ARM_TBFLAG_XSCALE_CPAR_SHIFT 17
>  #define ARM_TBFLAG_XSCALE_CPAR_MASK (3 << ARM_TBFLAG_XSCALE_CPAR_SHIFT)
>  /* Indicates whether cp register reads and writes by guest code should access
>   * the secure or nonsecure bank of banked registers; note that this is not
>   * the same thing as the current security state of the processor!
>   */
> -#define ARM_TBFLAG_NS_SHIFT         22
> +#define ARM_TBFLAG_NS_SHIFT         19
>  #define ARM_TBFLAG_NS_MASK          (1 << ARM_TBFLAG_NS_SHIFT)
>  
>  /* Bit usage when in AArch64 state: currently we have no A64 specific bits */
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 13/14] target-arm: Don't halt on WFI unless we don't have any work
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 13/14] target-arm: Don't halt on WFI unless we don't have any work Peter Maydell
@ 2015-05-28  5:55   ` Edgar E. Iglesias
  0 siblings, 0 replies; 36+ messages in thread
From: Edgar E. Iglesias @ 2015-05-28  5:55 UTC (permalink / raw)
  To: Peter Maydell; +Cc: agraf, serge.fdrv, alex.bennee, qemu-devel, patches

On Tue, May 19, 2015 at 07:33:33PM +0100, Peter Maydell wrote:
> Just NOP the WFI instruction if we have work to do.
> This doesn't make much difference currently (though it does avoid
> jumping out to the top level loop and immediately restarting),
> but the distinction between "halt" and "don't halt" will become
> more important when the decision to halt requires us to trap
> to a higher exception level instead.
> 
> Suggested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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


> ---
>  target-arm/op_helper.c     | 7 +++++++
>  target-arm/translate-a64.c | 4 ++++
>  target-arm/translate.c     | 4 ++++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 5963f3b..517dacc 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -252,6 +252,13 @@ void HELPER(wfi)(CPUARMState *env)
>  {
>      CPUState *cs = CPU(arm_env_get_cpu(env));
>  
> +    if (cpu_has_work(cs)) {
> +        /* Don't bother to go into our "low power state" if
> +         * we would just wake up immediately.
> +         */
> +        return;
> +    }
> +
>      cs->exception_index = EXCP_HLT;
>      cs->halted = 1;
>      cpu_loop_exit(cs);
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 8d08ccd..ffa6cb8 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -11113,6 +11113,10 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
>               */
>              gen_a64_set_pc_im(dc->pc);
>              gen_helper_wfi(cpu_env);
> +            /* The helper doesn't necessarily throw an exception, but we
> +             * must go back to the main loop to check for interrupts anyway.
> +             */
> +            tcg_gen_exit_tb(0);
>              break;
>          }
>      }
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index ed2c43d..6493b9a 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -11351,6 +11351,10 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
>              break;
>          case DISAS_WFI:
>              gen_helper_wfi(cpu_env);
> +            /* The helper doesn't necessarily throw an exception, but we
> +             * must go back to the main loop to check for interrupts anyway.
> +             */
> +            tcg_gen_exit_tb(0);
>              break;
>          case DISAS_WFE:
>              gen_helper_wfe(cpu_env);
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 14/14] target-arm: Add WFx instruction trap support
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 14/14] target-arm: Add WFx instruction trap support Peter Maydell
@ 2015-05-28  5:59   ` Edgar E. Iglesias
  0 siblings, 0 replies; 36+ messages in thread
From: Edgar E. Iglesias @ 2015-05-28  5:59 UTC (permalink / raw)
  To: Peter Maydell; +Cc: agraf, serge.fdrv, alex.bennee, qemu-devel, patches

On Tue, May 19, 2015 at 07:33:34PM +0100, Peter Maydell wrote:
> From: Greg Bellows <greg.bellows@linaro.org>
> 
> Add support for trapping WFI and WFE instructions to the proper EL when
> SCTLR/SCR/HCR settings apply.
> 
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
> [PMM: removed unnecessary tweaking of syn_wfx() prototype;
>  use raise_exception();
>  don't trap on WFE (and add comment explaining why not);
>  remove unnecessary ARM_FEATURE checks;
>  trap to EL3, not EL1, if in S-EL0 and SCTLR check fires]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


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


> ---
>  target-arm/op_helper.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 517dacc..c5ba9f0 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -248,9 +248,60 @@ uint32_t HELPER(usat16)(CPUARMState *env, uint32_t x, uint32_t shift)
>      return res;
>  }
>  
> +/* Function checks whether WFx (WFI/WFE) instructions are set up to be trapped.
> + * The function returns the target EL (1-3) if the instruction is to be trapped;
> + * otherwise it returns 0 indicating it is not trapped.
> + */
> +static inline int check_wfx_trap(CPUARMState *env, bool is_wfe)
> +{
> +    int cur_el = arm_current_el(env);
> +    uint64_t mask;
> +
> +    /* If we are currently in EL0 then we need to check if SCTLR is set up for
> +     * WFx instructions being trapped to EL1. These trap bits don't exist in v7.
> +     */
> +    if (cur_el < 1 && arm_feature(env, ARM_FEATURE_V8)) {
> +        int target_el;
> +
> +        mask = is_wfe ? SCTLR_nTWE : SCTLR_nTWI;
> +        if (arm_is_secure_below_el3(env) && !arm_el_is_aa64(env, 3)) {
> +            /* Secure EL0 and Secure PL1 is at EL3 */
> +            target_el = 3;
> +        } else {
> +            target_el = 1;
> +        }
> +
> +        if (!(env->cp15.sctlr_el[target_el] & mask)) {
> +            return target_el;
> +        }
> +    }
> +
> +    /* We are not trapping to EL1; trap to EL2 if HCR_EL2 requires it
> +     * No need for ARM_FEATURE check as if HCR_EL2 doesn't exist the
> +     * bits will be zero indicating no trap.
> +     */
> +    if (cur_el < 2 && !arm_is_secure(env)) {
> +        mask = (is_wfe) ? HCR_TWE : HCR_TWI;
> +        if (env->cp15.hcr_el2 & mask) {
> +            return 2;
> +        }
> +    }
> +
> +    /* We are not trapping to EL1 or EL2; trap to EL3 if SCR_EL3 requires it */
> +    if (cur_el < 3) {
> +        mask = (is_wfe) ? SCR_TWE : SCR_TWI;
> +        if (env->cp15.scr_el3 & mask) {
> +            return 3;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>  void HELPER(wfi)(CPUARMState *env)
>  {
>      CPUState *cs = CPU(arm_env_get_cpu(env));
> +    int target_el = check_wfx_trap(env, false);
>  
>      if (cpu_has_work(cs)) {
>          /* Don't bother to go into our "low power state" if
> @@ -259,6 +310,11 @@ void HELPER(wfi)(CPUARMState *env)
>          return;
>      }
>  
> +    if (target_el) {
> +        env->pc -= 4;
> +        raise_exception(env, EXCP_UDEF, syn_wfx(1, 0xe, 0), target_el);
> +    }
> +
>      cs->exception_index = EXCP_HLT;
>      cs->halted = 1;
>      cpu_loop_exit(cs);
> @@ -269,7 +325,9 @@ void HELPER(wfe)(CPUARMState *env)
>      CPUState *cs = CPU(arm_env_get_cpu(env));
>  
>      /* Don't actually halt the CPU, just yield back to top
> -     * level loop
> +     * level loop. This is not going into a "low power state"
> +     * (ie halting until some event occurs), so we never take
> +     * a configurable trap to a different exception level.
>       */
>      cs->exception_index = EXCP_YIELD;
>      cpu_loop_exit(cs);
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 03/14] target-arm: Set correct syndrome for faults on MSR DAIF*, imm
  2015-05-28  5:30   ` Edgar E. Iglesias
@ 2015-05-28  8:30     ` Peter Maydell
  2015-05-28 11:40       ` Peter Maydell
  0 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2015-05-28  8:30 UTC (permalink / raw)
  To: Edgar E. Iglesias
  Cc: Alexander Graf, Sergey Fedorov, Alex Bennée,
	QEMU Developers, Patch Tracking

On 28 May 2015 at 06:30, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote:
> On Tue, May 19, 2015 at 07:33:23PM +0100, Peter Maydell wrote:
>> If the SCTLR.UMA trap bit is set then attempts by EL0 to update
>> the PSTATE DAIF bits via "MSR DAIFSet, imm" and "MSR DAIFClr, imm"
>> instructions will raise an exception. We were failing to set
>> the syndrome information for this exception, which meant that
>> it would be reported as a repeat of whatever the previous
>> exception was. Set the correct syndrome information.
>> ---
>>  target-arm/op_helper.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
>> index 43e3457..5af4a0e 100644
>> --- a/target-arm/op_helper.c
>> +++ b/target-arm/op_helper.c
>> @@ -381,6 +381,9 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
>>       */
>>      if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UMA)) {
>>          env->exception.target_el = exception_target_el(env);
>> +        env->exception.syndrome = syn_aa64_sysregtrap(0, extract32(op, 0, 3),
>> +                                                      extract32(op, 3, 3), 4,
>> +                                                      0x1f, imm, 0);
>
> Did you possibly reverse the argument order of 0x1f and imm?

Ah, you're right; I was confused because the argument order of our
syn_aa64_sysregtrap() and the pseudocode AArch64.SystemRegisterTrap
is different (the latter follows the field order in the syndrome
register and ours doesn't).

-- PMM

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 03/14] target-arm: Set correct syndrome for faults on MSR DAIF*, imm
  2015-05-28  8:30     ` Peter Maydell
@ 2015-05-28 11:40       ` Peter Maydell
  2015-05-28 11:44         ` Edgar E. Iglesias
  0 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2015-05-28 11:40 UTC (permalink / raw)
  To: Edgar E. Iglesias
  Cc: Alexander Graf, Sergey Fedorov, Alex Bennée,
	QEMU Developers, Patch Tracking

On 28 May 2015 at 09:30, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 28 May 2015 at 06:30, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote:
>>> @@ -381,6 +381,9 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
>>>       */
>>>      if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UMA)) {
>>>          env->exception.target_el = exception_target_el(env);
>>> +        env->exception.syndrome = syn_aa64_sysregtrap(0, extract32(op, 0, 3),
>>> +                                                      extract32(op, 3, 3), 4,
>>> +                                                      0x1f, imm, 0);
>>
>> Did you possibly reverse the argument order of 0x1f and imm?
>
> Ah, you're right; I was confused because the argument order of our
> syn_aa64_sysregtrap() and the pseudocode AArch64.SystemRegisterTrap
> is different (the latter follows the field order in the syndrome
> register and ours doesn't).

I also managed to forget the signoff:
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

If this is the only fix in this series (I think you have one or two
patches still left to review) then I propose to add it as I put
the patches in target-arm.next:

-                                                      0x1f, imm, 0);
+                                                      imm, 0x1f, 0);

thanks
-- PMM

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 03/14] target-arm: Set correct syndrome for faults on MSR DAIF*, imm
  2015-05-28 11:40       ` Peter Maydell
@ 2015-05-28 11:44         ` Edgar E. Iglesias
  2015-05-28 11:54           ` Peter Maydell
  0 siblings, 1 reply; 36+ messages in thread
From: Edgar E. Iglesias @ 2015-05-28 11:44 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Alexander Graf, Sergey Fedorov, Alex Bennée,
	QEMU Developers, Patch Tracking

On Thu, May 28, 2015 at 12:40:42PM +0100, Peter Maydell wrote:
> On 28 May 2015 at 09:30, Peter Maydell <peter.maydell@linaro.org> wrote:
> > On 28 May 2015 at 06:30, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote:
> >>> @@ -381,6 +381,9 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
> >>>       */
> >>>      if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UMA)) {
> >>>          env->exception.target_el = exception_target_el(env);
> >>> +        env->exception.syndrome = syn_aa64_sysregtrap(0, extract32(op, 0, 3),
> >>> +                                                      extract32(op, 3, 3), 4,
> >>> +                                                      0x1f, imm, 0);
> >>
> >> Did you possibly reverse the argument order of 0x1f and imm?
> >
> > Ah, you're right; I was confused because the argument order of our
> > syn_aa64_sysregtrap() and the pseudocode AArch64.SystemRegisterTrap
> > is different (the latter follows the field order in the syndrome
> > register and ours doesn't).
> 
> I also managed to forget the signoff:
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> 
> If this is the only fix in this series (I think you have one or two
> patches still left to review) then I propose to add it as I put
> the patches in target-arm.next:
> 
> -                                                      0x1f, imm, 0);
> +                                                      imm, 0x1f, 0);


Yes, with these changes:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

One of the patches had a missing target_el assigment for
CP_ACCESS_TRAP_UNCATEGORIZED that I commented on a while back.

I'll have a look again.

Thanks,
Edgar



> 
> thanks
> -- PMM

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 08/14] target-arm: Allow cp access functions to indicate traps to EL2 or EL3
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 08/14] target-arm: Allow cp access functions to indicate traps to EL2 or EL3 Peter Maydell
  2015-05-21  5:47   ` Edgar E. Iglesias
@ 2015-05-28 11:48   ` Edgar E. Iglesias
  2015-05-28 11:56     ` Peter Maydell
  1 sibling, 1 reply; 36+ messages in thread
From: Edgar E. Iglesias @ 2015-05-28 11:48 UTC (permalink / raw)
  To: Peter Maydell; +Cc: agraf, serge.fdrv, alex.bennee, qemu-devel, patches

On Tue, May 19, 2015 at 07:33:28PM +0100, Peter Maydell wrote:
> Some coprocessor access functions will need to indicate that the
> instruction should trap to EL2 or EL3 rather than the default
> target exception level; add corresponding CPAccessResult enum
> entries and handling code.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.h       |  6 +++++-
>  target-arm/op_helper.c | 14 +++++++++++++-
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 9119a94..e431372 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1252,7 +1252,8 @@ typedef enum CPAccessResult {
>      /* Access fails due to a configurable trap or enable which would
>       * result in a categorized exception syndrome giving information about
>       * the failing instruction (ie syndrome category 0x3, 0x4, 0x5, 0x6,
> -     * 0xc or 0x18).
> +     * 0xc or 0x18). The exception is taken to the usual target EL (EL1 or
> +     * PL1 if in EL0, otherwise to the current EL).
>       */
>      CP_ACCESS_TRAP = 1,
>      /* Access fails and results in an exception syndrome 0x0 ("uncategorized").
> @@ -1260,6 +1261,9 @@ typedef enum CPAccessResult {
>       * result in this failure is specifically defined by the architecture.
>       */
>      CP_ACCESS_TRAP_UNCATEGORIZED = 2,
> +    /* As CP_ACCESS_TRAP, but for traps directly to EL2 or EL3 */
> +    CP_ACCESS_TRAP_EL2 = 3,
> +    CP_ACCESS_TRAP_EL3 = 4,
>  } CPAccessResult;
>  
>  /* Access functions for coprocessor registers. These cannot fail and
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index d693b01..5963f3b 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -335,6 +335,7 @@ void HELPER(set_user_reg)(CPUARMState *env, uint32_t regno, uint32_t val)
>  void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome)
>  {
>      const ARMCPRegInfo *ri = rip;
> +    int target_el;
>  
>      if (arm_feature(env, ARM_FEATURE_XSCALE) && ri->cp < 14
>          && extract32(env->cp15.c15_cpar, ri->cp, 1) == 0) {
> @@ -349,6 +350,17 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome)
>      case CP_ACCESS_OK:
>          return;
>      case CP_ACCESS_TRAP:
> +        target_el = exception_target_el(env);
> +        break;
> +    case CP_ACCESS_TRAP_EL2:
> +        /* Requesting a trap to EL2 when we're in EL3 or S-EL0/1 is
> +         * a bug in the access function.
> +         */
> +        assert(!arm_is_secure(env) && !arm_current_el(env) == 3);
> +        target_el = 2;
> +        break;
> +    case CP_ACCESS_TRAP_EL3:
> +        target_el = 3;
>          break;
>      case CP_ACCESS_TRAP_UNCATEGORIZED:
>          syndrome = syn_uncategorized();

Here it was, this needs target_el = exception_target_el(env);

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

Cheers,
Edgar



> @@ -357,7 +369,7 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome)
>          g_assert_not_reached();
>      }
>  
> -    raise_exception(env, EXCP_UDEF, syndrome, exception_target_el(env));
> +    raise_exception(env, EXCP_UDEF, syndrome, target_el);
>  }
>  
>  void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value)
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 03/14] target-arm: Set correct syndrome for faults on MSR DAIF*, imm
  2015-05-28 11:44         ` Edgar E. Iglesias
@ 2015-05-28 11:54           ` Peter Maydell
  0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2015-05-28 11:54 UTC (permalink / raw)
  To: Edgar E. Iglesias
  Cc: Alexander Graf, Sergey Fedorov, Alex Bennée,
	QEMU Developers, Patch Tracking

On 28 May 2015 at 12:44, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote:
> One of the patches had a missing target_el assigment for
> CP_ACCESS_TRAP_UNCATEGORIZED that I commented on a while back.

That's fixed in this series, I think.

Patches in this series I don't think I've seen a review or ack for:

target-arm: Allow cp access functions to indicate traps to EL2 or EL3
target-arm: Add AArch64 CPTR registers
target-arm: Extend FP checks to use an EL

thanks
-- PMM

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 08/14] target-arm: Allow cp access functions to indicate traps to EL2 or EL3
  2015-05-28 11:48   ` Edgar E. Iglesias
@ 2015-05-28 11:56     ` Peter Maydell
  0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2015-05-28 11:56 UTC (permalink / raw)
  To: Edgar E. Iglesias
  Cc: Alexander Graf, Sergey Fedorov, Alex Bennée,
	QEMU Developers, Patch Tracking

On 28 May 2015 at 12:48, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote:
> On Tue, May 19, 2015 at 07:33:28PM +0100, Peter Maydell wrote:
>> Some coprocessor access functions will need to indicate that the
>> instruction should trap to EL2 or EL3 rather than the default
>> target exception level; add corresponding CPAccessResult enum
>> entries and handling code.

>>      case CP_ACCESS_TRAP_UNCATEGORIZED:
>>          syndrome = syn_uncategorized();
>
> Here it was, this needs target_el = exception_target_el(env);
>
> With that change:
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Ah, I fixed this up in my working copy when you reviewed it,
but forgot to say so on list! No wonder I was confused...

-- PMM

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 09/14] target-arm: Add AArch64 CPTR registers
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 09/14] target-arm: Add AArch64 CPTR registers Peter Maydell
@ 2015-05-28 12:12   ` Edgar E. Iglesias
  0 siblings, 0 replies; 36+ messages in thread
From: Edgar E. Iglesias @ 2015-05-28 12:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: agraf, serge.fdrv, alex.bennee, qemu-devel, patches

On Tue, May 19, 2015 at 07:33:29PM +0100, Peter Maydell wrote:
> From: Greg Bellows <greg.bellows@linaro.org>
> 
> Adds CPTR_EL2/3 system registers definitions and access function.
> 
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
> [PMM: merge CPTR_EL2 and HCPTR definitions into a single
>  def using STATE_BOTH;
>  don't use readfn/writefn to implement RAZ/WI registers;
>  don't use accessfn for the no-EL2 CPTR_EL2;
>  fix cpacr_access logic to catch EL2 accesses to CPACR being
>  trapped to EL3;
>  use new CP_ACCESS_TRAP_EL[23] rather than setting
>  exception.target_el directly]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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


> ---
>  target-arm/cpu.h    |  5 +++++
>  target-arm/helper.c | 40 +++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index e431372..8cc4bc9 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -197,6 +197,7 @@ typedef struct CPUARMState {
>              uint64_t sctlr_el[4];
>          };
>          uint64_t cpacr_el1; /* Architectural feature access control register */
> +        uint64_t cptr_el[4];  /* ARMv8 feature trap registers */
>          uint32_t c1_xscaleauxcr; /* XScale auxiliary control register.  */
>          uint64_t sder; /* Secure debug enable register. */
>          uint32_t nsacr; /* Non-secure access control register. */
> @@ -568,6 +569,10 @@ void pmccntr_sync(CPUARMState *env);
>  #define SCTLR_AFE     (1U << 29)
>  #define SCTLR_TE      (1U << 30)
>  
> +#define CPTR_TCPAC    (1U << 31)
> +#define CPTR_TTA      (1U << 20)
> +#define CPTR_TFP      (1U << 10)
> +
>  #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 abfd70e..1cc4993 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -592,6 +592,33 @@ static void cpacr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>      env->cp15.cpacr_el1 = value;
>  }
>  
> +static CPAccessResult cpacr_access(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    if (arm_feature(env, ARM_FEATURE_V8)) {
> +        /* Check if CPACR accesses are to be trapped to EL2 */
> +        if (arm_current_el(env) == 1 &&
> +            (env->cp15.cptr_el[2] & CPTR_TCPAC) && !arm_is_secure(env)) {
> +            return CP_ACCESS_TRAP_EL2;
> +        /* Check if CPACR accesses are to be trapped to EL3 */
> +        } else if (arm_current_el(env) < 3 &&
> +                   (env->cp15.cptr_el[3] & CPTR_TCPAC)) {
> +            return CP_ACCESS_TRAP_EL3;
> +        }
> +    }
> +
> +    return CP_ACCESS_OK;
> +}
> +
> +static CPAccessResult cptr_access(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    /* Check if CPTR accesses are set to trap to EL3 */
> +    if (arm_current_el(env) == 2 && (env->cp15.cptr_el[3] & CPTR_TCPAC)) {
> +        return CP_ACCESS_TRAP_EL3;
> +    }
> +
> +    return CP_ACCESS_OK;
> +}
> +
>  static const ARMCPRegInfo v6_cp_reginfo[] = {
>      /* prefetch by MVA in v6, NOP in v7 */
>      { .name = "MVA_prefetch",
> @@ -614,7 +641,7 @@ static const ARMCPRegInfo v6_cp_reginfo[] = {
>      { .name = "WFAR", .cp = 15, .crn = 6, .crm = 0, .opc1 = 0, .opc2 = 1,
>        .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0, },
>      { .name = "CPACR", .state = ARM_CP_STATE_BOTH, .opc0 = 3,
> -      .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 2,
> +      .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 2, .accessfn = cpacr_access,
>        .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.cpacr_el1),
>        .resetvalue = 0, .writefn = cpacr_write },
>      REGINFO_SENTINEL
> @@ -2481,6 +2508,9 @@ static const ARMCPRegInfo v8_el3_no_el2_cp_reginfo[] = {
>        .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
>        .access = PL2_RW,
>        .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> +    { .name = "CPTR_EL2", .state = ARM_CP_STATE_BOTH,
> +      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 2,
> +      .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
>      REGINFO_SENTINEL
>  };
>  
> @@ -2548,6 +2578,10 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
>        .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 1, .opc2 = 0,
>        .access = PL3_RW, .type = ARM_CP_ALIAS,
>        .fieldoffset = offsetof(CPUARMState, sp_el[2]) },
> +    { .name = "CPTR_EL2", .state = ARM_CP_STATE_BOTH,
> +      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 2,
> +      .access = PL2_RW, .accessfn = cptr_access, .resetvalue = 0,
> +      .fieldoffset = offsetof(CPUARMState, cp15.cptr_el[2]) },
>      REGINFO_SENTINEL
>  };
>  
> @@ -2609,6 +2643,10 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>        .access = PL3_RW, .writefn = vbar_write,
>        .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[3]),
>        .resetvalue = 0 },
> +    { .name = "CPTR_EL3", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 2,
> +      .access = PL3_RW, .accessfn = cptr_access, .resetvalue = 0,
> +      .fieldoffset = offsetof(CPUARMState, cp15.cptr_el[3]) },
>      REGINFO_SENTINEL
>  };
>  
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 11/14] target-arm: Extend FP checks to use an EL
  2015-05-19 18:33 ` [Qemu-devel] [PATCH 11/14] target-arm: Extend FP checks to use an EL Peter Maydell
@ 2015-05-28 12:50   ` Edgar E. Iglesias
  2015-05-28 13:19     ` Peter Maydell
  0 siblings, 1 reply; 36+ messages in thread
From: Edgar E. Iglesias @ 2015-05-28 12:50 UTC (permalink / raw)
  To: Peter Maydell; +Cc: agraf, serge.fdrv, alex.bennee, qemu-devel, patches

On Tue, May 19, 2015 at 07:33:31PM +0100, Peter Maydell wrote:
> From: Greg Bellows <greg.bellows@linaro.org>
> 
> Extend the ARM disassemble context to take a target exception EL instead of a
> boolean enable. This change reverses the polarity of the check making a value
> of 0 indicate floating point enabled (no exception).
> 
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
> [PMM: Use a common TB flag field for AArch32 and AArch64;
>  CPTR_EL2 exists in v7; CPTR_EL2 should trap for EL2 accesses;
>  CPTR_EL2 should not trap for secure accesses; CPTR_EL3
>  should trap for EL3 accesses; CPACR traps for secure
>  accesses should trap to EL3 if EL3 is AArch32]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.h           | 92 +++++++++++++++++++++++++++++++++++-----------
>  target-arm/translate-a64.c |  8 ++--
>  target-arm/translate.c     | 17 ++++-----
>  target-arm/translate.h     |  2 +-
>  4 files changed, 82 insertions(+), 37 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 8aeb8aa..647e0ba 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1741,6 +1741,9 @@ static inline bool arm_singlestep_active(CPUARMState *env)
>  #define ARM_TBFLAG_SS_ACTIVE_MASK (1 << ARM_TBFLAG_SS_ACTIVE_SHIFT)
>  #define ARM_TBFLAG_PSTATE_SS_SHIFT 26
>  #define ARM_TBFLAG_PSTATE_SS_MASK (1 << ARM_TBFLAG_PSTATE_SS_SHIFT)
> +/* Target EL if we take a floating-point-disabled exception */
> +#define ARM_TBFLAG_FPEXC_EL_SHIFT 24
> +#define ARM_TBFLAG_FPEXC_EL_MASK (0x3 << ARM_TBFLAG_FPEXC_EL_SHIFT)
>  
>  /* Bit usage when in AArch32 state: */
>  #define ARM_TBFLAG_THUMB_SHIFT      0
> @@ -1755,8 +1758,6 @@ static inline bool arm_singlestep_active(CPUARMState *env)
>  #define ARM_TBFLAG_CONDEXEC_MASK    (0xff << ARM_TBFLAG_CONDEXEC_SHIFT)
>  #define ARM_TBFLAG_BSWAP_CODE_SHIFT 16
>  #define ARM_TBFLAG_BSWAP_CODE_MASK  (1 << ARM_TBFLAG_BSWAP_CODE_SHIFT)
> -#define ARM_TBFLAG_CPACR_FPEN_SHIFT 17
> -#define ARM_TBFLAG_CPACR_FPEN_MASK  (1 << ARM_TBFLAG_CPACR_FPEN_SHIFT)
>  /* We store the bottom two bits of the CPAR as TB flags and handle
>   * checks on the other bits at runtime
>   */
> @@ -1769,9 +1770,7 @@ static inline bool arm_singlestep_active(CPUARMState *env)
>  #define ARM_TBFLAG_NS_SHIFT         22
>  #define ARM_TBFLAG_NS_MASK          (1 << ARM_TBFLAG_NS_SHIFT)
>  
> -/* Bit usage when in AArch64 state */
> -#define ARM_TBFLAG_AA64_FPEN_SHIFT  2
> -#define ARM_TBFLAG_AA64_FPEN_MASK   (1 << ARM_TBFLAG_AA64_FPEN_SHIFT)
> +/* Bit usage when in AArch64 state: currently we have no A64 specific bits */
>  
>  /* some convenience accessor macros */
>  #define ARM_TBFLAG_AARCH64_STATE(F) \
> @@ -1782,6 +1781,8 @@ static inline bool arm_singlestep_active(CPUARMState *env)
>      (((F) & ARM_TBFLAG_SS_ACTIVE_MASK) >> ARM_TBFLAG_SS_ACTIVE_SHIFT)
>  #define ARM_TBFLAG_PSTATE_SS(F) \
>      (((F) & ARM_TBFLAG_PSTATE_SS_MASK) >> ARM_TBFLAG_PSTATE_SS_SHIFT)
> +#define ARM_TBFLAG_FPEXC_EL(F) \
> +    (((F) & ARM_TBFLAG_FPEXC_EL_MASK) >> ARM_TBFLAG_FPEXC_EL_SHIFT)
>  #define ARM_TBFLAG_THUMB(F) \
>      (((F) & ARM_TBFLAG_THUMB_MASK) >> ARM_TBFLAG_THUMB_SHIFT)
>  #define ARM_TBFLAG_VECLEN(F) \
> @@ -1794,33 +1795,82 @@ static inline bool arm_singlestep_active(CPUARMState *env)
>      (((F) & ARM_TBFLAG_CONDEXEC_MASK) >> ARM_TBFLAG_CONDEXEC_SHIFT)
>  #define ARM_TBFLAG_BSWAP_CODE(F) \
>      (((F) & ARM_TBFLAG_BSWAP_CODE_MASK) >> ARM_TBFLAG_BSWAP_CODE_SHIFT)
> -#define ARM_TBFLAG_CPACR_FPEN(F) \
> -    (((F) & ARM_TBFLAG_CPACR_FPEN_MASK) >> ARM_TBFLAG_CPACR_FPEN_SHIFT)
>  #define ARM_TBFLAG_XSCALE_CPAR(F) \
>      (((F) & ARM_TBFLAG_XSCALE_CPAR_MASK) >> ARM_TBFLAG_XSCALE_CPAR_SHIFT)
> -#define ARM_TBFLAG_AA64_FPEN(F) \
> -    (((F) & ARM_TBFLAG_AA64_FPEN_MASK) >> ARM_TBFLAG_AA64_FPEN_SHIFT)
>  #define ARM_TBFLAG_NS(F) \
>      (((F) & ARM_TBFLAG_NS_MASK) >> ARM_TBFLAG_NS_SHIFT)
>  
> -static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
> -                                        target_ulong *cs_base, int *flags)
> +/* Return the exception level to which FP-disabled exceptions should
> + * be taken, or 0 if FP is enabled.
> + */
> +static inline int fp_exception_el(CPUARMState *env)
>  {
>      int fpen;
> +    int cur_el = arm_current_el(env);
>  
> -    if (arm_feature(env, ARM_FEATURE_V6)) {
> -        fpen = extract32(env->cp15.cpacr_el1, 20, 2);
> -    } else {
> -        /* CPACR doesn't exist before v6, so VFP is always accessible */
> -        fpen = 3;
> +    /* CPACR and the CPTR registers don't exist before v6, so FP is
> +     * always accessible
> +     */
> +    if (!arm_feature(env, ARM_FEATURE_V6)) {
> +        return 0;
> +    }
> +
> +    /* The CPACR controls traps to EL1, or PL1 if we're 32 bit:
> +     * 0, 2 : trap EL0 and EL1/PL1 accesses
> +     * 1    : trap only EL0 accesses
> +     * 3    : trap no accesses
> +     */
> +    fpen = extract32(env->cp15.cpacr_el1, 20, 2);
> +    switch (fpen) {
> +    case 0:
> +    case 2:
> +        if (cur_el == 0 || cur_el == 1) {
> +            /* Trap to PL1, which might be EL1 or EL3 */
> +            if (arm_is_secure(env) && !arm_el_is_aa64(env, 3)) {
> +                return 3;
> +            }
> +            return 1;
> +        }
> +        if (cur_el == 3 && !is_a64(env)) {
> +            /* Secure PL1 running at EL3 */
> +            return 3;
> +        }
> +        break;
> +    case 1:
> +        if (cur_el == 0) {
> +            return 1;
> +        }
> +        break;
> +    case 3:
> +        break;
> +    }
> +
> +    /* For the CPTR registers we don't need to guard with an ARM_FEATURE
> +     * check because zero bits in the registers mean "don't trap".
> +     */
> +
> +    /* CPTR_EL2 : present in v7VE or v8 */
> +    if (cur_el <= 2 && extract32(env->cp15.cptr_el[2], 10, 1)
> +        && arm_is_secure_below_el3(env)) {

I think this should be !arm_is_secure_below_el3(env)

Also the ARMv7 manual, the part describing CPACR cpn bits, indicates that
HCTPR has prio over CPACR. I didn't find any thing like that for
ARMv8... There might be a difference between v8/v7 here, do you know?

Cheers,
Edgar



> +        /* Trap FP ops at EL2, NS-EL1 or NS-EL0 to EL2 */
> +        return 2;
> +    }
> +
> +    /* CPTR_EL3 : present in v8 */
> +    if (extract32(env->cp15.cptr_el[3], 10, 1)) {
> +        /* Trap all FP ops to EL3 */
> +        return 3;
>      }
>  
> +    return 0;
> +}
> +
> +static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
> +                                        target_ulong *cs_base, int *flags)
> +{
>      if (is_a64(env)) {
>          *pc = env->pc;
>          *flags = ARM_TBFLAG_AARCH64_STATE_MASK;
> -        if (fpen == 3 || (fpen == 1 && arm_current_el(env) != 0)) {
> -            *flags |= ARM_TBFLAG_AA64_FPEN_MASK;
> -        }
>      } else {
>          *pc = env->regs[15];
>          *flags = (env->thumb << ARM_TBFLAG_THUMB_SHIFT)
> @@ -1835,9 +1885,6 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
>              || arm_el_is_aa64(env, 1)) {
>              *flags |= ARM_TBFLAG_VFPEN_MASK;
>          }
> -        if (fpen == 3 || (fpen == 1 && arm_current_el(env) != 0)) {
> -            *flags |= ARM_TBFLAG_CPACR_FPEN_MASK;
> -        }
>          *flags |= (extract32(env->cp15.c15_cpar, 0, 2)
>                     << ARM_TBFLAG_XSCALE_CPAR_SHIFT);
>      }
> @@ -1862,6 +1909,7 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
>              }
>          }
>      }
> +    *flags |= fp_exception_el(env) << ARM_TBFLAG_FPEXC_EL_SHIFT;
>  
>      *cs_base = 0;
>  }
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index b58778a..8d08ccd 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -412,7 +412,7 @@ static TCGv_i64 read_cpu_reg_sp(DisasContext *s, int reg, int sf)
>  static inline void assert_fp_access_checked(DisasContext *s)
>  {
>  #ifdef CONFIG_DEBUG_TCG
> -    if (unlikely(!s->fp_access_checked || !s->cpacr_fpen)) {
> +    if (unlikely(!s->fp_access_checked || s->fp_excp_el)) {
>          fprintf(stderr, "target-arm: FP access check missing for "
>                  "instruction 0x%08x\n", s->insn);
>          abort();
> @@ -972,12 +972,12 @@ static inline bool fp_access_check(DisasContext *s)
>      assert(!s->fp_access_checked);
>      s->fp_access_checked = true;
>  
> -    if (s->cpacr_fpen) {
> +    if (!s->fp_excp_el) {
>          return true;
>      }
>  
>      gen_exception_insn(s, 4, EXCP_UDEF, syn_fp_access_trap(1, 0xe, false),
> -                       default_exception_el(s));
> +                       s->fp_excp_el);
>      return false;
>  }
>  
> @@ -10954,7 +10954,7 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
>  #if !defined(CONFIG_USER_ONLY)
>      dc->user = (dc->current_el == 0);
>  #endif
> -    dc->cpacr_fpen = ARM_TBFLAG_AA64_FPEN(tb->flags);
> +    dc->fp_excp_el = ARM_TBFLAG_FPEXC_EL(tb->flags);
>      dc->vec_len = 0;
>      dc->vec_stride = 0;
>      dc->cp_regs = cpu->cp_regs;
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 2bd5733..ed2c43d 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -3044,10 +3044,9 @@ static int disas_vfp_insn(DisasContext *s, uint32_t insn)
>       * for invalid encodings; we will generate incorrect syndrome information
>       * for attempts to execute invalid vfp/neon encodings with FP disabled.
>       */
> -    if (!s->cpacr_fpen) {
> +    if (s->fp_excp_el) {
>          gen_exception_insn(s, 4, EXCP_UDEF,
> -                           syn_fp_access_trap(1, 0xe, s->thumb),
> -                           default_exception_el(s));
> +                           syn_fp_access_trap(1, 0xe, s->thumb), s->fp_excp_el);
>          return 0;
>      }
>  
> @@ -4363,10 +4362,9 @@ static int disas_neon_ls_insn(DisasContext *s, uint32_t insn)
>       * for invalid encodings; we will generate incorrect syndrome information
>       * for attempts to execute invalid vfp/neon encodings with FP disabled.
>       */
> -    if (!s->cpacr_fpen) {
> +    if (s->fp_excp_el) {
>          gen_exception_insn(s, 4, EXCP_UDEF,
> -                           syn_fp_access_trap(1, 0xe, s->thumb),
> -                           default_exception_el(s));
> +                           syn_fp_access_trap(1, 0xe, s->thumb), s->fp_excp_el);
>          return 0;
>      }
>  
> @@ -5102,10 +5100,9 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
>       * for invalid encodings; we will generate incorrect syndrome information
>       * for attempts to execute invalid vfp/neon encodings with FP disabled.
>       */
> -    if (!s->cpacr_fpen) {
> +    if (s->fp_excp_el) {
>          gen_exception_insn(s, 4, EXCP_UDEF,
> -                           syn_fp_access_trap(1, 0xe, s->thumb),
> -                           default_exception_el(s));
> +                           syn_fp_access_trap(1, 0xe, s->thumb), s->fp_excp_el);
>          return 0;
>      }
>  
> @@ -11082,7 +11079,7 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
>      dc->user = (dc->current_el == 0);
>  #endif
>      dc->ns = ARM_TBFLAG_NS(tb->flags);
> -    dc->cpacr_fpen = ARM_TBFLAG_CPACR_FPEN(tb->flags);
> +    dc->fp_excp_el = ARM_TBFLAG_FPEXC_EL(tb->flags);
>      dc->vfp_enabled = ARM_TBFLAG_VFPEN(tb->flags);
>      dc->vec_len = ARM_TBFLAG_VECLEN(tb->flags);
>      dc->vec_stride = ARM_TBFLAG_VECSTRIDE(tb->flags);
> diff --git a/target-arm/translate.h b/target-arm/translate.h
> index 2eadcb7..bcdcf11 100644
> --- a/target-arm/translate.h
> +++ b/target-arm/translate.h
> @@ -22,7 +22,7 @@ typedef struct DisasContext {
>  #endif
>      ARMMMUIdx mmu_idx; /* MMU index to use for normal loads/stores */
>      bool ns;        /* Use non-secure CPREG bank on access */
> -    bool cpacr_fpen; /* FP enabled via CPACR.FPEN */
> +    int fp_excp_el; /* FP exception EL or 0 if enabled */
>      bool el3_is_aa64;  /* Flag indicating whether EL3 is AArch64 or not */
>      bool vfp_enabled; /* FP enabled via FPSCR.EN */
>      int vec_len;
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 11/14] target-arm: Extend FP checks to use an EL
  2015-05-28 12:50   ` Edgar E. Iglesias
@ 2015-05-28 13:19     ` Peter Maydell
  2015-05-28 13:27       ` Edgar E. Iglesias
  0 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2015-05-28 13:19 UTC (permalink / raw)
  To: Edgar E. Iglesias
  Cc: Alexander Graf, Sergey Fedorov, Alex Bennée,
	QEMU Developers, Patch Tracking

On 28 May 2015 at 13:50, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote:
> On Tue, May 19, 2015 at 07:33:31PM +0100, Peter Maydell wrote:
>> +    /* For the CPTR registers we don't need to guard with an ARM_FEATURE
>> +     * check because zero bits in the registers mean "don't trap".
>> +     */
>> +
>> +    /* CPTR_EL2 : present in v7VE or v8 */
>> +    if (cur_el <= 2 && extract32(env->cp15.cptr_el[2], 10, 1)
>> +        && arm_is_secure_below_el3(env)) {
>
> I think this should be !arm_is_secure_below_el3(env)

Yes.

> Also the ARMv7 manual, the part describing CPACR cpn bits, indicates that
> HCTPR has prio over CPACR. I didn't find any thing like that for
> ARMv8... There might be a difference between v8/v7 here, do you know?

For v8, this is described in "G1.11.2 Exception priority order" I think:
exceptions taken to EL1 because of configurable access to insns
are item 8, which has higher priority than HCPTR traps (item 10).
This matches the code I have in this patch.

I believe that v7 is the same, and the v7 ARM ARM is just slightly
confusingly worded. The HCPTR documentation says that setting TCPn
to 1 traps "any otherwise-valid Non-secure access to CPn". The
CheckAdvSIMDOrVFPEnabled() pseudocode also clearly checks CPACR
first and HCPTR second.

-- PMM

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 11/14] target-arm: Extend FP checks to use an EL
  2015-05-28 13:19     ` Peter Maydell
@ 2015-05-28 13:27       ` Edgar E. Iglesias
  2015-05-28 14:48         ` Peter Maydell
  0 siblings, 1 reply; 36+ messages in thread
From: Edgar E. Iglesias @ 2015-05-28 13:27 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Alexander Graf, Sergey Fedorov, Alex Bennée,
	QEMU Developers, Patch Tracking

On Thu, May 28, 2015 at 02:19:42PM +0100, Peter Maydell wrote:
> On 28 May 2015 at 13:50, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote:
> > On Tue, May 19, 2015 at 07:33:31PM +0100, Peter Maydell wrote:
> >> +    /* For the CPTR registers we don't need to guard with an ARM_FEATURE
> >> +     * check because zero bits in the registers mean "don't trap".
> >> +     */
> >> +
> >> +    /* CPTR_EL2 : present in v7VE or v8 */
> >> +    if (cur_el <= 2 && extract32(env->cp15.cptr_el[2], 10, 1)
> >> +        && arm_is_secure_below_el3(env)) {
> >
> > I think this should be !arm_is_secure_below_el3(env)
> 
> Yes.
> 
> > Also the ARMv7 manual, the part describing CPACR cpn bits, indicates that
> > HCTPR has prio over CPACR. I didn't find any thing like that for
> > ARMv8... There might be a difference between v8/v7 here, do you know?
> 
> For v8, this is described in "G1.11.2 Exception priority order" I think:
> exceptions taken to EL1 because of configurable access to insns
> are item 8, which has higher priority than HCPTR traps (item 10).
> This matches the code I have in this patch.
> 
> I believe that v7 is the same, and the v7 ARM ARM is just slightly
> confusingly worded. The HCPTR documentation says that setting TCPn
> to 1 traps "any otherwise-valid Non-secure access to CPn". The
> CheckAdvSIMDOrVFPEnabled() pseudocode also clearly checks CPACR
> first and HCPTR second.

Thanks for clarifying that. With the !arm_is_secure_below_el3(env)
change:

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

Cheers,
Edgar

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PATCH 11/14] target-arm: Extend FP checks to use an EL
  2015-05-28 13:27       ` Edgar E. Iglesias
@ 2015-05-28 14:48         ` Peter Maydell
  0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2015-05-28 14:48 UTC (permalink / raw)
  To: Edgar E. Iglesias
  Cc: Alexander Graf, Sergey Fedorov, Alex Bennée,
	QEMU Developers, Patch Tracking

On 28 May 2015 at 14:27, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote:
> Thanks for clarifying that. With the !arm_is_secure_below_el3(env)
> change:
>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Thanks. Since there have only been a few trivial fixups I've applied
the series to target-arm.next.

-- PMM

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2015-05-28 14:48 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-19 18:33 [Qemu-devel] [PATCH 00/14] Various EL3 support/cleanup patches Peter Maydell
2015-05-19 18:33 ` [Qemu-devel] [PATCH 01/14] target-arm: Add exception target el infrastructure Peter Maydell
2015-05-19 18:33 ` [Qemu-devel] [PATCH 02/14] target-arm: Extend helpers to route exceptions Peter Maydell
2015-05-19 18:33 ` [Qemu-devel] [PATCH 03/14] target-arm: Set correct syndrome for faults on MSR DAIF*, imm Peter Maydell
2015-05-28  5:30   ` Edgar E. Iglesias
2015-05-28  8:30     ` Peter Maydell
2015-05-28 11:40       ` Peter Maydell
2015-05-28 11:44         ` Edgar E. Iglesias
2015-05-28 11:54           ` Peter Maydell
2015-05-19 18:33 ` [Qemu-devel] [PATCH 04/14] target-arm: Move setting of exception info into tlb_fill Peter Maydell
2015-05-28  5:32   ` Edgar E. Iglesias
2015-05-19 18:33 ` [Qemu-devel] [PATCH 05/14] target-arm: Set exception target EL in tlb_fill Peter Maydell
2015-05-28  5:39   ` Edgar E. Iglesias
2015-05-19 18:33 ` [Qemu-devel] [PATCH 06/14] target-arm: Make raise_exception() take syndrome and target EL Peter Maydell
2015-05-28  5:42   ` Edgar E. Iglesias
2015-05-19 18:33 ` [Qemu-devel] [PATCH 07/14] target-arm: Update interrupt handling to use " Peter Maydell
2015-05-19 18:33 ` [Qemu-devel] [PATCH 08/14] target-arm: Allow cp access functions to indicate traps to EL2 or EL3 Peter Maydell
2015-05-21  5:47   ` Edgar E. Iglesias
2015-05-21  7:04     ` Peter Maydell
2015-05-28 11:48   ` Edgar E. Iglesias
2015-05-28 11:56     ` Peter Maydell
2015-05-19 18:33 ` [Qemu-devel] [PATCH 09/14] target-arm: Add AArch64 CPTR registers Peter Maydell
2015-05-28 12:12   ` Edgar E. Iglesias
2015-05-19 18:33 ` [Qemu-devel] [PATCH 10/14] target-arm: Make singlestate TB flags common between AArch32/64 Peter Maydell
2015-05-28  5:51   ` Edgar E. Iglesias
2015-05-19 18:33 ` [Qemu-devel] [PATCH 11/14] target-arm: Extend FP checks to use an EL Peter Maydell
2015-05-28 12:50   ` Edgar E. Iglesias
2015-05-28 13:19     ` Peter Maydell
2015-05-28 13:27       ` Edgar E. Iglesias
2015-05-28 14:48         ` Peter Maydell
2015-05-19 18:33 ` [Qemu-devel] [PATCH 12/14] target-arm: Move TB flags down to fill gap Peter Maydell
2015-05-28  5:53   ` Edgar E. Iglesias
2015-05-19 18:33 ` [Qemu-devel] [PATCH 13/14] target-arm: Don't halt on WFI unless we don't have any work Peter Maydell
2015-05-28  5:55   ` Edgar E. Iglesias
2015-05-19 18:33 ` [Qemu-devel] [PATCH 14/14] target-arm: Add WFx instruction trap support Peter Maydell
2015-05-28  5:59   ` Edgar E. Iglesias

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.