* [Qemu-devel] [PATCH v2 1/9] target-arm: Add exception target el infrastructure
2015-04-22 17:09 [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support Greg Bellows
@ 2015-04-22 17:09 ` Greg Bellows
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 2/9] target-arm: Extend helpers to route exceptions Greg Bellows
` (9 subsequent siblings)
10 siblings, 0 replies; 33+ messages in thread
From: Greg Bellows @ 2015-04-22 17:09 UTC (permalink / raw)
To: qemu-devel, peter.maydell, alex.bennee; +Cc: serge.fdrv, Greg Bellows
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>
---
v1 -> v2
- Add translate utility function for determining the default exception target
EL and replaced uses of MAX with it.
---
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 083211c..0b232ba 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -401,6 +401,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 7713022..72a973a 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.8.3.2
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH v2 2/9] target-arm: Extend helpers to route exceptions
2015-04-22 17:09 [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support Greg Bellows
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 1/9] target-arm: Add exception target el infrastructure Greg Bellows
@ 2015-04-22 17:09 ` Greg Bellows
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 3/9] target-arm: Update interrupt handling to use target EL Greg Bellows
` (8 subsequent siblings)
10 siblings, 0 replies; 33+ messages in thread
From: Greg Bellows @ 2015-04-22 17:09 UTC (permalink / raw)
To: qemu-devel, peter.maydell, alex.bennee; +Cc: serge.fdrv, Greg Bellows
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>
---
v1 -> v2
- Add utility function for determining the target exception EL.
- Replaced uses of MAX with the above function when setting the target EL.
---
target-arm/op_helper.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 72a973a..971edc7 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);
}
@@ -363,6 +378,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 +438,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 +469,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.8.3.2
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH v2 3/9] target-arm: Update interrupt handling to use target EL
2015-04-22 17:09 [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support Greg Bellows
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 1/9] target-arm: Add exception target el infrastructure Greg Bellows
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 2/9] target-arm: Extend helpers to route exceptions Greg Bellows
@ 2015-04-22 17:09 ` Greg Bellows
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 4/9] target-arm: Add AArch64 CPTR registers Greg Bellows
` (7 subsequent siblings)
10 siblings, 0 replies; 33+ messages in thread
From: Greg Bellows @ 2015-04-22 17:09 UTC (permalink / raw)
To: qemu-devel, peter.maydell, alex.bennee; +Cc: serge.fdrv, Greg Bellows
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>
---
v1 -> v2
- Remove need for MAX in aarch64_cpu_do_interrupt. Other changed eliminated
the need for it,
---
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 986f04c..4aa71a7 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 0b232ba..2178a1f 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1007,7 +1007,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);
@@ -1489,11 +1490,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 d77c6de..c83d9fc 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -4091,7 +4091,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;
}
@@ -4215,8 +4216,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);
@@ -4251,40 +4252,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.8.3.2
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH v2 4/9] target-arm: Add AArch64 CPTR registers
2015-04-22 17:09 [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support Greg Bellows
` (2 preceding siblings ...)
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 3/9] target-arm: Update interrupt handling to use target EL Greg Bellows
@ 2015-04-22 17:09 ` Greg Bellows
2015-05-18 17:32 ` Peter Maydell
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 5/9] target-arm: Extend FP checks to use an EL Greg Bellows
` (6 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Greg Bellows @ 2015-04-22 17:09 UTC (permalink / raw)
To: qemu-devel, peter.maydell, alex.bennee; +Cc: serge.fdrv, Greg Bellows
Adds CPTR_EL2/3 system registers definitions and access function.
Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
v2 -> v3
- Broke out cptr and cpacr access functions
- Added HCPTR register entry as alias of CPTR_EL2
- Added HCPTR and CPTR_EL2 no_el2 register entries.
- Fixed cptr_access comment
---
target-arm/cpu.h | 5 +++++
target-arm/helper.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 2178a1f..d61bb3f 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -202,6 +202,7 @@ typedef struct CPUARMState {
uint64_t sctlr_el[4];
};
uint64_t c1_coproc; /* Coprocessor access 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. */
@@ -575,6 +576,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 c83d9fc..e36c3d4 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -592,6 +592,34 @@ static void cpacr_write(CPUARMState *env, const ARMCPRegInfo *ri,
env->cp15.c1_coproc = value;
}
+static CPAccessResult cpacr_access(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+ if (arm_feature(env, ARM_FEATURE_V8) && arm_current_el(env) == 1) {
+ /* Check if CPACR accesses are to be trapped to EL2 */
+ if ((env->cp15.cptr_el[2] & CPTR_TCPAC) && !arm_is_secure(env)) {
+ env->exception.target_el = 2;
+ return CP_ACCESS_TRAP;
+ /* Check if CPACR accesses are to be trapped to EL3 */
+ } else if (env->cp15.cptr_el[3] & CPTR_TCPAC) {
+ env->exception.target_el = 3;
+ return CP_ACCESS_TRAP;
+ }
+ }
+
+ return CP_ACCESS_OK;
+}
+
+static CPAccessResult cptr_access(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+ /* Check it CPTR accesses are set to trap to EL3 */
+ if (arm_current_el(env) == 2 && (env->cp15.cptr_el[3] & CPTR_TCPAC)) {
+ env->exception.target_el = 3;
+ return CP_ACCESS_TRAP;
+ }
+
+ return CP_ACCESS_OK;
+}
+
static const ARMCPRegInfo v6_cp_reginfo[] = {
/* prefetch by MVA in v6, NOP in v7 */
{ .name = "MVA_prefetch",
@@ -614,7 +642,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.c1_coproc),
.resetvalue = 0, .writefn = cpacr_write },
REGINFO_SENTINEL
@@ -2470,6 +2498,14 @@ 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_AA64,
+ .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 2,
+ .access = PL2_RW, .accessfn = cptr_access, .resetvalue = 0,
+ .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
+ { .name = "HCPTR", .type = ARM_CP_ALIAS,
+ .cp = 15, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 2,
+ .access = PL2_RW, .accessfn = cptr_access, .resetvalue = 0,
+ .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
REGINFO_SENTINEL
};
@@ -2537,6 +2573,14 @@ 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_AA64,
+ .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]) },
+ { .name = "HCPTR", .type = ARM_CP_ALIAS,
+ .cp = 15, .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
};
@@ -2598,6 +2642,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.8.3.2
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v2 4/9] target-arm: Add AArch64 CPTR registers
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 4/9] target-arm: Add AArch64 CPTR registers Greg Bellows
@ 2015-05-18 17:32 ` Peter Maydell
0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2015-05-18 17:32 UTC (permalink / raw)
To: Greg Bellows; +Cc: Sergey Fedorov, Alex Bennée, QEMU Developers
On 22 April 2015 at 18:09, Greg Bellows <greg.bellows@linaro.org> wrote:
> Adds CPTR_EL2/3 system registers definitions and access function.
>
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
>
> ---
>
> v2 -> v3
> - Broke out cptr and cpacr access functions
> - Added HCPTR register entry as alias of CPTR_EL2
> - Added HCPTR and CPTR_EL2 no_el2 register entries.
> - Fixed cptr_access comment
> ---
> target-arm/cpu.h | 5 +++++
> target-arm/helper.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 54 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 2178a1f..d61bb3f 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -202,6 +202,7 @@ typedef struct CPUARMState {
> uint64_t sctlr_el[4];
> };
> uint64_t c1_coproc; /* Coprocessor access 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. */
> @@ -575,6 +576,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 c83d9fc..e36c3d4 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -592,6 +592,34 @@ static void cpacr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> env->cp15.c1_coproc = value;
> }
>
> +static CPAccessResult cpacr_access(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> + if (arm_feature(env, ARM_FEATURE_V8) && arm_current_el(env) == 1) {
> + /* Check if CPACR accesses are to be trapped to EL2 */
> + if ((env->cp15.cptr_el[2] & CPTR_TCPAC) && !arm_is_secure(env)) {
> + env->exception.target_el = 2;
> + return CP_ACCESS_TRAP;
> + /* Check if CPACR accesses are to be trapped to EL3 */
> + } else if (env->cp15.cptr_el[3] & CPTR_TCPAC) {
CPTR_EL3.TCPAC traps EL2 accesses to CPACR as well as EL1 accesses.
> + env->exception.target_el = 3;
> + return CP_ACCESS_TRAP;
> + }
> + }
> +
> + return CP_ACCESS_OK;
> +}
> +
> +static CPAccessResult cptr_access(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> + /* Check it CPTR accesses are set to trap to EL3 */
"if"
> + if (arm_current_el(env) == 2 && (env->cp15.cptr_el[3] & CPTR_TCPAC)) {
> + env->exception.target_el = 3;
> + return CP_ACCESS_TRAP;
> + }
> +
> + return CP_ACCESS_OK;
> +}
> +
> static const ARMCPRegInfo v6_cp_reginfo[] = {
> /* prefetch by MVA in v6, NOP in v7 */
> { .name = "MVA_prefetch",
> @@ -614,7 +642,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.c1_coproc),
> .resetvalue = 0, .writefn = cpacr_write },
> REGINFO_SENTINEL
> @@ -2470,6 +2498,14 @@ 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_AA64,
> + .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 2,
> + .access = PL2_RW, .accessfn = cptr_access, .resetvalue = 0,
> + .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> + { .name = "HCPTR", .type = ARM_CP_ALIAS,
> + .cp = 15, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 2,
> + .access = PL2_RW, .accessfn = cptr_access, .resetvalue = 0,
> + .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
These can be merged into a single ARM_CP_STATE_BOTH, using
ARM_CP_CONST is more efficient than using readfns to get the
RAZ/WI behaviour, and we don't need the access function because
it can never trigger for a CPU without EL2:
+ { .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
> };
>
> @@ -2537,6 +2573,14 @@ 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_AA64,
> + .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]) },
> + { .name = "HCPTR", .type = ARM_CP_ALIAS,
> + .cp = 15, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 2,
> + .access = PL2_RW, .accessfn = cptr_access, .resetvalue = 0,
> + .fieldoffset = offsetof(CPUARMState, cp15.cptr_el[2]) },
These also can be merged:
+ { .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
> };
>
> @@ -2598,6 +2642,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
> };
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v2 5/9] target-arm: Extend FP checks to use an EL
2015-04-22 17:09 [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support Greg Bellows
` (3 preceding siblings ...)
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 4/9] target-arm: Add AArch64 CPTR registers Greg Bellows
@ 2015-04-22 17:09 ` Greg Bellows
2015-05-18 17:40 ` Peter Maydell
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 6/9] target-arm: Add TTBR regime function and use Greg Bellows
` (5 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Greg Bellows @ 2015-04-22 17:09 UTC (permalink / raw)
To: qemu-devel, peter.maydell, alex.bennee; +Cc: serge.fdrv, Greg Bellows
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>
---
target-arm/cpu.h | 63 +++++++++++++++++++++++++++++++++++-----------
target-arm/translate-a64.c | 8 +++---
target-arm/translate.c | 17 ++++++-------
target-arm/translate.h | 2 +-
4 files changed, 61 insertions(+), 29 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index d61bb3f..c7808da 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1773,11 +1773,14 @@ static inline bool arm_singlestep_active(CPUARMState *env)
#define ARM_TBFLAG_NS_MASK (1 << ARM_TBFLAG_NS_SHIFT)
/* Bit usage when in AArch64 state */
+/* The floating-point mask in AArch64 is 2 bits to carry the target exception
+ * EL is not enabled.
+ */
#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_FPEN_MASK (0x3 << ARM_TBFLAG_AA64_FPEN_SHIFT)
+#define ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT 4
#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_SHIFT 5
#define ARM_TBFLAG_AA64_PSTATE_SS_MASK (1 << ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
/* some convenience accessor macros */
@@ -1814,24 +1817,55 @@ static inline bool arm_singlestep_active(CPUARMState *env)
#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)
+/* Function to determine if floating point is disabled and what EL FP
+ * operations should be trapped to. If FP is enabled 0 is returned.
+ */
+static inline int get_fp_exception_el(CPUARMState *env)
{
int fpen;
+ int cur_el = arm_current_el(env);
+ /* CPACR doesn't exist before v6, so VFP is always accessible */
if (arm_feature(env, ARM_FEATURE_V6)) {
fpen = extract32(env->cp15.c1_coproc, 20, 2);
- } else {
- /* CPACR doesn't exist before v6, so VFP is always accessible */
- fpen = 3;
+
+ /* If we are less than EL2, check if the CPACR has floating point
+ * enabled. If not, return a trap to EL1.
+ */
+ if ((cur_el == 0 && fpen == 1) ||
+ (cur_el < 2 && (fpen == 0 || fpen == 2))) {
+ return 1;
+ }
+
+ /* The CPTR registers only exist in ARMv8 */
+ if (arm_feature(env, ARM_FEATURE_V8)) {
+ /* Check whether floating point operations are trapped to EL2 */
+ if (cur_el < 2 && extract32(env->cp15.cptr_el[2], 10, 1)) {
+ return 2;
+ }
+
+ /* Check whether floating point operations are trapped to EL3 */
+ if (cur_el < 3 && extract32(env->cp15.cptr_el[3], 10, 1)) {
+ return 3;
+ }
+ }
}
+ return 0;
+}
+
+static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
+ target_ulong *cs_base, int *flags)
+{
+ int fp_excp_el = get_fp_exception_el(env);
+
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;
- }
+
+ /* Add the target FP exception EL to the flags */
+ *flags |= fp_excp_el << ARM_TBFLAG_AA64_FPEN_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
@@ -1859,9 +1893,10 @@ 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;
- }
+
+ /* Add the target FP exception EL to the flags */
+ *flags |= fp_excp_el << ARM_TBFLAG_CPACR_FPEN_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
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index b1f44c9..b4423ca 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_AA64_FPEN(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..0d9b856 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_CPACR_FPEN(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.8.3.2
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v2 5/9] target-arm: Extend FP checks to use an EL
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 5/9] target-arm: Extend FP checks to use an EL Greg Bellows
@ 2015-05-18 17:40 ` Peter Maydell
0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2015-05-18 17:40 UTC (permalink / raw)
To: Greg Bellows; +Cc: Sergey Fedorov, Alex Bennée, QEMU Developers
On 22 April 2015 at 18:09, Greg Bellows <greg.bellows@linaro.org> wrote:
> 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>
> ---
> target-arm/cpu.h | 63 +++++++++++++++++++++++++++++++++++-----------
> target-arm/translate-a64.c | 8 +++---
> target-arm/translate.c | 17 ++++++-------
> target-arm/translate.h | 2 +-
> 4 files changed, 61 insertions(+), 29 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index d61bb3f..c7808da 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1773,11 +1773,14 @@ static inline bool arm_singlestep_active(CPUARMState *env)
> #define ARM_TBFLAG_NS_MASK (1 << ARM_TBFLAG_NS_SHIFT)
>
> /* Bit usage when in AArch64 state */
> +/* The floating-point mask in AArch64 is 2 bits to carry the target exception
> + * EL is not enabled.
> + */
> #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_FPEN_MASK (0x3 << ARM_TBFLAG_AA64_FPEN_SHIFT)
> +#define ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT 4
> #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_SHIFT 5
> #define ARM_TBFLAG_AA64_PSTATE_SS_MASK (1 << ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
>
> /* some convenience accessor macros */
> @@ -1814,24 +1817,55 @@ static inline bool arm_singlestep_active(CPUARMState *env)
> #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)
> +/* Function to determine if floating point is disabled and what EL FP
> + * operations should be trapped to. If FP is enabled 0 is returned.
> + */
> +static inline int get_fp_exception_el(CPUARMState *env)
> {
> int fpen;
> + int cur_el = arm_current_el(env);
>
> + /* CPACR doesn't exist before v6, so VFP is always accessible */
> if (arm_feature(env, ARM_FEATURE_V6)) {
> fpen = extract32(env->cp15.c1_coproc, 20, 2);
this would be easier to read as
if (!v6) {
return 0;
}
rather than putting the bulk of the function in an else clause.
> - } else {
> - /* CPACR doesn't exist before v6, so VFP is always accessible */
> - fpen = 3;
> +
> + /* If we are less than EL2, check if the CPACR has floating point
> + * enabled. If not, return a trap to EL1.
> + */
> + if ((cur_el == 0 && fpen == 1) ||
> + (cur_el < 2 && (fpen == 0 || fpen == 2))) {
> + return 1;
> + }
> +
> + /* The CPTR registers only exist in ARMv8 */
> + if (arm_feature(env, ARM_FEATURE_V8)) {
> + /* Check whether floating point operations are trapped to EL2 */
> + if (cur_el < 2 && extract32(env->cp15.cptr_el[2], 10, 1)) {
> + return 2;
> + }
> +
> + /* Check whether floating point operations are trapped to EL3 */
> + if (cur_el < 3 && extract32(env->cp15.cptr_el[3], 10, 1)) {
> + return 3;
> + }
> + }
> }
>
> + return 0;
> +}
> +
> +static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
> + target_ulong *cs_base, int *flags)
> +{
> + int fp_excp_el = get_fp_exception_el(env);
> +
> 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;
> - }
> +
> + /* Add the target FP exception EL to the flags */
> + *flags |= fp_excp_el << ARM_TBFLAG_AA64_FPEN_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
> @@ -1859,9 +1893,10 @@ 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;
> - }
> +
> + /* Add the target FP exception EL to the flags */
> + *flags |= fp_excp_el << ARM_TBFLAG_CPACR_FPEN_SHIFT;
You didn't make this tb flags field 2 bits wide...
(We should probably bite the bullet and tidy up the flags fields a bit
so more of the common flags are really common.)
> +
> /* 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
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index b1f44c9..b4423ca 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_AA64_FPEN(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..0d9b856 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_CPACR_FPEN(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.8.3.2
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v2 6/9] target-arm: Add TTBR regime function and use
2015-04-22 17:09 [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support Greg Bellows
` (4 preceding siblings ...)
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 5/9] target-arm: Extend FP checks to use an EL Greg Bellows
@ 2015-04-22 17:09 ` Greg Bellows
2015-04-22 18:16 ` Sergey Fedorov
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 7/9] target-arm: Add EL3 and EL2 TCR checking Greg Bellows
` (4 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Greg Bellows @ 2015-04-22 17:09 UTC (permalink / raw)
To: qemu-devel, peter.maydell, alex.bennee; +Cc: serge.fdrv, Greg Bellows
Add a utility function for choosing the correct TTBR system register based on
the specified MMU index. Add use of function on physical address lookup.
Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
target-arm/helper.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index e36c3d4..69a5891 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -4899,6 +4899,21 @@ static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
return &env->cp15.tcr_el[regime_el(env, mmu_idx)];
}
+/* Return the TTBR associated with this translation regime */
+static inline uint32_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx,
+ int ttbrn)
+{
+ if (mmu_idx == ARMMMUIdx_S2NS) {
+ /* TODO: return VTTBR_EL2 */
+ g_assert_not_reached();
+ }
+ if (ttbrn == 0) {
+ return env->cp15.ttbr0_el[regime_el(env, mmu_idx)];
+ } else {
+ return env->cp15.ttbr1_el[regime_el(env, mmu_idx)];
+ }
+}
+
/* Return true if the translation regime is using LPAE format page tables */
static inline bool regime_using_lpae_format(CPUARMState *env,
ARMMMUIdx mmu_idx)
@@ -5097,7 +5112,6 @@ static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx,
uint32_t *table, uint32_t address)
{
/* Note that we can only get here for an AArch32 PL0/PL1 lookup */
- int el = regime_el(env, mmu_idx);
TCR *tcr = regime_tcr(env, mmu_idx);
if (address & tcr->mask) {
@@ -5105,13 +5119,13 @@ static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx,
/* Translation table walk disabled for TTBR1 */
return false;
}
- *table = env->cp15.ttbr1_el[el] & 0xffffc000;
+ *table = regime_ttbr(env, mmu_idx, 1) & 0xffffc000;
} else {
if (tcr->raw_tcr & TTBCR_PD0) {
/* Translation table walk disabled for TTBR0 */
return false;
}
- *table = env->cp15.ttbr0_el[el] & tcr->base_mask;
+ *table = regime_ttbr(env, mmu_idx, 0) & tcr->base_mask;
}
*table |= (address >> 18) & 0x3ffc;
return true;
@@ -5441,7 +5455,7 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
* we will always flush the TLB any time the ASID is changed).
*/
if (ttbr_select == 0) {
- ttbr = A32_BANKED_CURRENT_REG_GET(env, ttbr0);
+ ttbr = regime_ttbr(env, mmu_idx, 0);
epd = extract32(tcr->raw_tcr, 7, 1);
tsz = t0sz;
@@ -5453,7 +5467,7 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
granule_sz = 11;
}
} else {
- ttbr = A32_BANKED_CURRENT_REG_GET(env, ttbr1);
+ ttbr = regime_ttbr(env, mmu_idx, 1);
epd = extract32(tcr->raw_tcr, 23, 1);
tsz = t1sz;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v2 6/9] target-arm: Add TTBR regime function and use
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 6/9] target-arm: Add TTBR regime function and use Greg Bellows
@ 2015-04-22 18:16 ` Sergey Fedorov
2015-04-22 18:23 ` Greg Bellows
0 siblings, 1 reply; 33+ messages in thread
From: Sergey Fedorov @ 2015-04-22 18:16 UTC (permalink / raw)
To: Greg Bellows, qemu-devel, peter.maydell, alex.bennee
On 22.04.2015 10:09, Greg Bellows wrote:
> Add a utility function for choosing the correct TTBR system register based on
> the specified MMU index. Add use of function on physical address lookup.
>
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
> ---
> target-arm/helper.c | 24 +++++++++++++++++++-----
> 1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index e36c3d4..69a5891 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -4899,6 +4899,21 @@ static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
> return &env->cp15.tcr_el[regime_el(env, mmu_idx)];
> }
>
> +/* Return the TTBR associated with this translation regime */
> +static inline uint32_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx,
> + int ttbrn)
Should return uint64_t.
> +{
> + if (mmu_idx == ARMMMUIdx_S2NS) {
> + /* TODO: return VTTBR_EL2 */
> + g_assert_not_reached();
> + }
> + if (ttbrn == 0) {
> + return env->cp15.ttbr0_el[regime_el(env, mmu_idx)];
> + } else {
> + return env->cp15.ttbr1_el[regime_el(env, mmu_idx)];
> + }
> +}
> +
> /* Return true if the translation regime is using LPAE format page tables */
> static inline bool regime_using_lpae_format(CPUARMState *env,
> ARMMMUIdx mmu_idx)
> @@ -5097,7 +5112,6 @@ static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx,
> uint32_t *table, uint32_t address)
> {
> /* Note that we can only get here for an AArch32 PL0/PL1 lookup */
> - int el = regime_el(env, mmu_idx);
> TCR *tcr = regime_tcr(env, mmu_idx);
>
> if (address & tcr->mask) {
> @@ -5105,13 +5119,13 @@ static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx,
> /* Translation table walk disabled for TTBR1 */
> return false;
> }
> - *table = env->cp15.ttbr1_el[el] & 0xffffc000;
> + *table = regime_ttbr(env, mmu_idx, 1) & 0xffffc000;
> } else {
> if (tcr->raw_tcr & TTBCR_PD0) {
> /* Translation table walk disabled for TTBR0 */
> return false;
> }
> - *table = env->cp15.ttbr0_el[el] & tcr->base_mask;
> + *table = regime_ttbr(env, mmu_idx, 0) & tcr->base_mask;
> }
> *table |= (address >> 18) & 0x3ffc;
> return true;
> @@ -5441,7 +5455,7 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
> * we will always flush the TLB any time the ASID is changed).
> */
> if (ttbr_select == 0) {
> - ttbr = A32_BANKED_CURRENT_REG_GET(env, ttbr0);
> + ttbr = regime_ttbr(env, mmu_idx, 0);
> epd = extract32(tcr->raw_tcr, 7, 1);
> tsz = t0sz;
>
> @@ -5453,7 +5467,7 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
> granule_sz = 11;
> }
> } else {
> - ttbr = A32_BANKED_CURRENT_REG_GET(env, ttbr1);
> + ttbr = regime_ttbr(env, mmu_idx, 1);
> epd = extract32(tcr->raw_tcr, 23, 1);
> tsz = t1sz;
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v2 6/9] target-arm: Add TTBR regime function and use
2015-04-22 18:16 ` Sergey Fedorov
@ 2015-04-22 18:23 ` Greg Bellows
0 siblings, 0 replies; 33+ messages in thread
From: Greg Bellows @ 2015-04-22 18:23 UTC (permalink / raw)
To: Sergey Fedorov; +Cc: Peter Maydell, Alex Bennée, QEMU Developers
[-- Attachment #1: Type: text/plain, Size: 3589 bytes --]
On Wed, Apr 22, 2015 at 1:16 PM, Sergey Fedorov <serge.fdrv@gmail.com>
wrote:
> On 22.04.2015 10:09, Greg Bellows wrote:
> > Add a utility function for choosing the correct TTBR system register
> based on
> > the specified MMU index. Add use of function on physical address lookup.
> >
> > Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
> > ---
> > target-arm/helper.c | 24 +++++++++++++++++++-----
> > 1 file changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index e36c3d4..69a5891 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -4899,6 +4899,21 @@ static inline TCR *regime_tcr(CPUARMState *env,
> ARMMMUIdx mmu_idx)
> > return &env->cp15.tcr_el[regime_el(env, mmu_idx)];
> > }
> >
> > +/* Return the TTBR associated with this translation regime */
> > +static inline uint32_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx,
> > + int ttbrn)
>
> Should return uint64_t.
>
>
Yes, good catch... will fix in v3.
> > +{
> > + if (mmu_idx == ARMMMUIdx_S2NS) {
> > + /* TODO: return VTTBR_EL2 */
> > + g_assert_not_reached();
> > + }
> > + if (ttbrn == 0) {
> > + return env->cp15.ttbr0_el[regime_el(env, mmu_idx)];
> > + } else {
> > + return env->cp15.ttbr1_el[regime_el(env, mmu_idx)];
> > + }
> > +}
> > +
> > /* Return true if the translation regime is using LPAE format page
> tables */
> > static inline bool regime_using_lpae_format(CPUARMState *env,
> > ARMMMUIdx mmu_idx)
> > @@ -5097,7 +5112,6 @@ static bool get_level1_table_address(CPUARMState
> *env, ARMMMUIdx mmu_idx,
> > uint32_t *table, uint32_t address)
> > {
> > /* Note that we can only get here for an AArch32 PL0/PL1 lookup */
> > - int el = regime_el(env, mmu_idx);
> > TCR *tcr = regime_tcr(env, mmu_idx);
> >
> > if (address & tcr->mask) {
> > @@ -5105,13 +5119,13 @@ static bool get_level1_table_address(CPUARMState
> *env, ARMMMUIdx mmu_idx,
> > /* Translation table walk disabled for TTBR1 */
> > return false;
> > }
> > - *table = env->cp15.ttbr1_el[el] & 0xffffc000;
> > + *table = regime_ttbr(env, mmu_idx, 1) & 0xffffc000;
> > } else {
> > if (tcr->raw_tcr & TTBCR_PD0) {
> > /* Translation table walk disabled for TTBR0 */
> > return false;
> > }
> > - *table = env->cp15.ttbr0_el[el] & tcr->base_mask;
> > + *table = regime_ttbr(env, mmu_idx, 0) & tcr->base_mask;
> > }
> > *table |= (address >> 18) & 0x3ffc;
> > return true;
> > @@ -5441,7 +5455,7 @@ static int get_phys_addr_lpae(CPUARMState *env,
> target_ulong address,
> > * we will always flush the TLB any time the ASID is changed).
> > */
> > if (ttbr_select == 0) {
> > - ttbr = A32_BANKED_CURRENT_REG_GET(env, ttbr0);
> > + ttbr = regime_ttbr(env, mmu_idx, 0);
> > epd = extract32(tcr->raw_tcr, 7, 1);
> > tsz = t0sz;
> >
> > @@ -5453,7 +5467,7 @@ static int get_phys_addr_lpae(CPUARMState *env,
> target_ulong address,
> > granule_sz = 11;
> > }
> > } else {
> > - ttbr = A32_BANKED_CURRENT_REG_GET(env, ttbr1);
> > + ttbr = regime_ttbr(env, mmu_idx, 1);
> > epd = extract32(tcr->raw_tcr, 23, 1);
> > tsz = t1sz;
> >
>
>
[-- Attachment #2: Type: text/html, Size: 5204 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v2 7/9] target-arm: Add EL3 and EL2 TCR checking
2015-04-22 17:09 [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support Greg Bellows
` (5 preceding siblings ...)
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 6/9] target-arm: Add TTBR regime function and use Greg Bellows
@ 2015-04-22 17:09 ` Greg Bellows
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 8/9] target-arm: Add WFx syndrome function Greg Bellows
` (3 subsequent siblings)
10 siblings, 0 replies; 33+ messages in thread
From: Greg Bellows @ 2015-04-22 17:09 UTC (permalink / raw)
To: qemu-devel, peter.maydell, alex.bennee; +Cc: serge.fdrv, Greg Bellows
Updated get_phys_addr_lpae to check the appropriate TTBCR/TCR depending on the
current EL. Support includes using the different TCR format as well as checks to
insure TTBR1 is not used when in EL2 or EL3.
Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
target-arm/helper.c | 45 ++++++++++++++++++++++++++++++++-------------
1 file changed, 32 insertions(+), 13 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 69a5891..e7ab5c4 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -5397,21 +5397,34 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
int32_t tbi = 0;
TCR *tcr = regime_tcr(env, mmu_idx);
int ap, ns, xn, pxn;
+ uint32_t el = regime_el(env, mmu_idx);
+ bool ttbr1_valid = true;
/* TODO:
- * This code assumes we're either a 64-bit EL1 or a 32-bit PL1;
- * it doesn't handle the different format TCR for TCR_EL2, TCR_EL3,
- * and VTCR_EL2, or the fact that those regimes don't have a split
- * TTBR0/TTBR1. Attribute and permission bit handling should also
- * be checked when adding support for those page table walks.
+ * This code does not handle the different format TCR for VTCR_EL2.
+ * This code also does not support shareability levels.
+ * Attribute and permission bit handling should also be checked when adding
+ * support for those page table walks.
*/
- if (arm_el_is_aa64(env, regime_el(env, mmu_idx))) {
+ if (arm_el_is_aa64(env, el)) {
va_size = 64;
- if (extract64(address, 55, 1))
- tbi = extract64(tcr->raw_tcr, 38, 1);
- else
- tbi = extract64(tcr->raw_tcr, 37, 1);
+ if (el > 1) {
+ tbi = extract64(tcr->raw_tcr, 20, 1);
+ } else {
+ if (extract64(address, 55, 1)) {
+ tbi = extract64(tcr->raw_tcr, 38, 1);
+ } else {
+ tbi = extract64(tcr->raw_tcr, 37, 1);
+ }
+ }
tbi *= 8;
+
+ /* If we are in 64-bit EL2 or EL3 then there is no TTBR1, so mark it
+ * invalid.
+ */
+ if (el > 1) {
+ ttbr1_valid = false;
+ }
}
/* Determine whether this address is in the region controlled by
@@ -5432,13 +5445,14 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
if (t0sz && !extract64(address, va_size - t0sz, t0sz - tbi)) {
/* there is a ttbr0 region and we are in it (high bits all zero) */
ttbr_select = 0;
- } else if (t1sz && !extract64(~address, va_size - t1sz, t1sz - tbi)) {
+ } else if (ttbr1_valid && t1sz &&
+ !extract64(~address, va_size - t1sz, t1sz - tbi)) {
/* there is a ttbr1 region and we are in it (high bits all one) */
ttbr_select = 1;
} else if (!t0sz) {
/* ttbr0 region is "everything not in the ttbr1 region" */
ttbr_select = 0;
- } else if (!t1sz) {
+ } else if (!t1sz && ttbr1_valid) {
/* ttbr1 region is "everything not in the ttbr0 region" */
ttbr_select = 1;
} else {
@@ -5467,6 +5481,9 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
granule_sz = 11;
}
} else {
+ /* We should only be here if TTBR1 is valid */
+ assert(ttbr1_valid);
+
ttbr = regime_ttbr(env, mmu_idx, 1);
epd = extract32(tcr->raw_tcr, 23, 1);
tsz = t1sz;
@@ -5485,7 +5502,9 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
*/
if (epd) {
- /* Translation table walk disabled => Translation fault on TLB miss */
+ /* Translation table walk disabled => Translation fault on TLB miss
+ * Note: This is always 0 on 64-bit EL2 and EL3.
+ */
goto do_fault;
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH v2 8/9] target-arm: Add WFx syndrome function
2015-04-22 17:09 [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support Greg Bellows
` (6 preceding siblings ...)
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 7/9] target-arm: Add EL3 and EL2 TCR checking Greg Bellows
@ 2015-04-22 17:09 ` Greg Bellows
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support Greg Bellows
` (2 subsequent siblings)
10 siblings, 0 replies; 33+ messages in thread
From: Greg Bellows @ 2015-04-22 17:09 UTC (permalink / raw)
To: qemu-devel, peter.maydell, alex.bennee; +Cc: serge.fdrv, Greg Bellows
Adds a utility function for creating a WFx exception syndrome
Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
target-arm/internals.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/target-arm/internals.h b/target-arm/internals.h
index 2cc3017..de0a9c1 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -347,6 +347,12 @@ static inline uint32_t syn_breakpoint(int same_el)
| ARM_EL_IL | 0x22;
}
+static inline uint32_t syn_wfx(int cv, int cond, int ti)
+{
+ return (EC_WFX_TRAP << ARM_EL_EC_SHIFT) |
+ (cv << 24) | (cond << 20) | ti;
+}
+
/* Update a QEMU watchpoint based on the information the guest has set in the
* DBGWCR<n>_EL1 and DBGWVR<n>_EL1 registers.
*/
--
1.8.3.2
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support
2015-04-22 17:09 [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support Greg Bellows
` (7 preceding siblings ...)
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 8/9] target-arm: Add WFx syndrome function Greg Bellows
@ 2015-04-22 17:09 ` Greg Bellows
2015-04-23 2:49 ` Edgar E. Iglesias
2015-04-23 3:37 ` [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 " Edgar E. Iglesias
2015-05-18 17:53 ` Peter Maydell
10 siblings, 1 reply; 33+ messages in thread
From: Greg Bellows @ 2015-04-22 17:09 UTC (permalink / raw)
To: qemu-devel, peter.maydell, alex.bennee; +Cc: serge.fdrv, Greg Bellows
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>
---
v1 -> v2
- Replace check loop with simpler if checks.
- Changed WFx syncdrome function to take bool
- Changed return of uint32_t to int
- Added cdditional comments.
---
target-arm/internals.h | 2 +-
target-arm/op_helper.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 74 insertions(+), 4 deletions(-)
diff --git a/target-arm/internals.h b/target-arm/internals.h
index de0a9c1..8e208f7 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -347,7 +347,7 @@ static inline uint32_t syn_breakpoint(int same_el)
| ARM_EL_IL | 0x22;
}
-static inline uint32_t syn_wfx(int cv, int cond, int ti)
+static inline uint32_t syn_wfx(int cv, int cond, bool ti)
{
return (EC_WFX_TRAP << ARM_EL_EC_SHIFT) |
(cv << 24) | (cond << 20) | ti;
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 971edc7..43aa8ef 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -223,23 +223,93 @@ 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.
+ */
+ if (cur_el < 1) {
+ mask = is_wfe ? SCTLR_nTWE : SCTLR_nTWI;
+ if (arm_el_is_aa64(env, 1)) {
+ if (!(env->cp15.sctlr_el[1] & mask)) {
+ return 1;
+ }
+ } else if (arm_feature(env, ARM_FEATURE_V8)) {
+ /* SCTLR WFx SCTLR trap bits only exist in ARMv8 */
+ if (!(A32_BANKED_CURRENT_REG_GET(env, sctlr) & mask)) {
+ return 1;
+ }
+ }
+ }
+
+ /* We are not trapping to EL1 at this point.
+ * Check if we are trapping WFx to EL2 it present.
+ */
+ if (cur_el < 2) {
+ if (arm_feature(env, ARM_FEATURE_EL2) && !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 at this point.
+ * Check if we are trapping WFx to EL3 if present.
+ */
+ if (cur_el < 3) {
+ if (arm_feature(env, ARM_FEATURE_EL3) &&
+ arm_feature(env, ARM_FEATURE_V8)) {
+ 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);
- cs->exception_index = EXCP_HLT;
- cs->halted = 1;
+ if (target_el) {
+ cs->exception_index = EXCP_UDEF;
+ env->exception.syndrome = syn_wfx(1, 0xe, false);
+ env->exception.target_el = target_el;
+ env->pc -= 4;
+ } else {
+ cs->exception_index = EXCP_HLT;
+ cs->halted = 1;
+ }
cpu_loop_exit(cs);
}
void HELPER(wfe)(CPUARMState *env)
{
CPUState *cs = CPU(arm_env_get_cpu(env));
+ int target_el = check_wfx_trap(env, true);
/* Don't actually halt the CPU, just yield back to top
* level loop
*/
- cs->exception_index = EXCP_YIELD;
+ if (target_el) {
+ cs->exception_index = EXCP_UDEF;
+ env->exception.syndrome = syn_wfx(1, 0xe, true);
+ env->exception.target_el = target_el;
+ env->pc -= 4;
+ } else {
+ cs->exception_index = EXCP_YIELD;
+ }
cpu_loop_exit(cs);
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support Greg Bellows
@ 2015-04-23 2:49 ` Edgar E. Iglesias
2015-04-23 7:59 ` Peter Maydell
0 siblings, 1 reply; 33+ messages in thread
From: Edgar E. Iglesias @ 2015-04-23 2:49 UTC (permalink / raw)
To: Greg Bellows; +Cc: peter.maydell, alex.bennee, qemu-devel, serge.fdrv
On Wed, Apr 22, 2015 at 12:09:21PM -0500, Greg Bellows wrote:
> 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>
>
> ---
>
> v1 -> v2
> - Replace check loop with simpler if checks.
> - Changed WFx syncdrome function to take bool
> - Changed return of uint32_t to int
> - Added cdditional comments.
> ---
> target-arm/internals.h | 2 +-
> target-arm/op_helper.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 74 insertions(+), 4 deletions(-)
>
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index de0a9c1..8e208f7 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -347,7 +347,7 @@ static inline uint32_t syn_breakpoint(int same_el)
> | ARM_EL_IL | 0x22;
> }
>
> -static inline uint32_t syn_wfx(int cv, int cond, int ti)
> +static inline uint32_t syn_wfx(int cv, int cond, bool ti)
> {
> return (EC_WFX_TRAP << ARM_EL_EC_SHIFT) |
> (cv << 24) | (cond << 20) | ti;
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 971edc7..43aa8ef 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -223,23 +223,93 @@ 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.
> + */
> + if (cur_el < 1) {
> + mask = is_wfe ? SCTLR_nTWE : SCTLR_nTWI;
> + if (arm_el_is_aa64(env, 1)) {
> + if (!(env->cp15.sctlr_el[1] & mask)) {
> + return 1;
> + }
> + } else if (arm_feature(env, ARM_FEATURE_V8)) {
> + /* SCTLR WFx SCTLR trap bits only exist in ARMv8 */
> + if (!(A32_BANKED_CURRENT_REG_GET(env, sctlr) & mask)) {
> + return 1;
> + }
> + }
> + }
> +
> + /* We are not trapping to EL1 at this point.
> + * Check if we are trapping WFx to EL2 it present.
> + */
> + if (cur_el < 2) {
> + if (arm_feature(env, ARM_FEATURE_EL2) && !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 at this point.
> + * Check if we are trapping WFx to EL3 if present.
> + */
> + if (cur_el < 3) {
> + if (arm_feature(env, ARM_FEATURE_EL3) &&
> + arm_feature(env, ARM_FEATURE_V8)) {
> + 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);
>
> - cs->exception_index = EXCP_HLT;
> - cs->halted = 1;
> + if (target_el) {
> + cs->exception_index = EXCP_UDEF;
> + env->exception.syndrome = syn_wfx(1, 0xe, false);
> + env->exception.target_el = target_el;
> + env->pc -= 4;
> + } else {
> + cs->exception_index = EXCP_HLT;
> + cs->halted = 1;
> + }
> cpu_loop_exit(cs);
> }
>
> void HELPER(wfe)(CPUARMState *env)
> {
> CPUState *cs = CPU(arm_env_get_cpu(env));
> + int target_el = check_wfx_trap(env, true);
>
> /* Don't actually halt the CPU, just yield back to top
> * level loop
> */
> - cs->exception_index = EXCP_YIELD;
> + if (target_el) {
> + cs->exception_index = EXCP_UDEF;
> + env->exception.syndrome = syn_wfx(1, 0xe, true);
> + env->exception.target_el = target_el;
> + env->pc -= 4;
> + } else {
> + cs->exception_index = EXCP_YIELD;
> + }
> cpu_loop_exit(cs);
> }
Hi Greg,
Before trapping, don't you need to check that the CPU has no actual work?
e.g:
if (!cc->has_work(cs) && ..)
I think you can still EXCP_YIELD the core if it has work though as it's probably a good place to reschedule other CPUs in our single-threaded SMP model.
Cheers,
Edgar
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support
2015-04-23 2:49 ` Edgar E. Iglesias
@ 2015-04-23 7:59 ` Peter Maydell
2015-04-23 10:39 ` Edgar E. Iglesias
0 siblings, 1 reply; 33+ messages in thread
From: Peter Maydell @ 2015-04-23 7:59 UTC (permalink / raw)
To: Edgar E. Iglesias
Cc: Sergey Fedorov, Alex Bennée, QEMU Developers, Greg Bellows
On 23 April 2015 at 03:49, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Wed, Apr 22, 2015 at 12:09:21PM -0500, Greg Bellows wrote:
>> void HELPER(wfe)(CPUARMState *env)
>> {
>> CPUState *cs = CPU(arm_env_get_cpu(env));
>> + int target_el = check_wfx_trap(env, true);
>>
>> /* Don't actually halt the CPU, just yield back to top
>> * level loop
>> */
>> - cs->exception_index = EXCP_YIELD;
>> + if (target_el) {
>> + cs->exception_index = EXCP_UDEF;
>> + env->exception.syndrome = syn_wfx(1, 0xe, true);
>> + env->exception.target_el = target_el;
>> + env->pc -= 4;
>> + } else {
>> + cs->exception_index = EXCP_YIELD;
>> + }
>> cpu_loop_exit(cs);
>> }
>
>
> Hi Greg,
>
> Before trapping, don't you need to check that the CPU has no actual work?
> e.g:
> if (!cc->has_work(cs) && ..)
We don't track WFE-wakeup events, so we must always trap. (Well,
I suppose strictly we could also go for "never trap" on the basis
that really our implementation here is pretty much a NOP, but I
think always-trap is closer to reasonable.)
In theory you could maybe check has_work() for the WFI case,
since doing an EXCP_HLT really should cause us to stop until
has_work is true, but it seems a bit fragile -- could we really
guarantee that nothing would change between this point and
when we went back through the main loop that would change
whether has_work evaluates true or not? I think that it's better
there too to just always take the trap: setting EXCP_HLT is our
"going into a low power state" and so we should take the trap
if we would otherwise have done that.
> I think you can still EXCP_YIELD the core if it has work though
> as it's probably a good place to reschedule other CPUs in our
> single-threaded SMP model.
That is indeed the only reason we do anything with WFE at all
(it used to be a no-op, but yielding gives better performance).
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support
2015-04-23 7:59 ` Peter Maydell
@ 2015-04-23 10:39 ` Edgar E. Iglesias
2015-04-23 10:57 ` Peter Maydell
0 siblings, 1 reply; 33+ messages in thread
From: Edgar E. Iglesias @ 2015-04-23 10:39 UTC (permalink / raw)
To: Peter Maydell
Cc: Sergey Fedorov, Alex Bennée, QEMU Developers, Greg Bellows
[-- Attachment #1: Type: text/plain, Size: 2430 bytes --]
On 23/04/2015 6:00 pm, "Peter Maydell" <peter.maydell@linaro.org> wrote:
>
> On 23 April 2015 at 03:49, Edgar E. Iglesias <edgar.iglesias@gmail.com>
wrote:
> > On Wed, Apr 22, 2015 at 12:09:21PM -0500, Greg Bellows wrote:
> >> void HELPER(wfe)(CPUARMState *env)
> >> {
> >> CPUState *cs = CPU(arm_env_get_cpu(env));
> >> + int target_el = check_wfx_trap(env, true);
> >>
> >> /* Don't actually halt the CPU, just yield back to top
> >> * level loop
> >> */
> >> - cs->exception_index = EXCP_YIELD;
> >> + if (target_el) {
> >> + cs->exception_index = EXCP_UDEF;
> >> + env->exception.syndrome = syn_wfx(1, 0xe, true);
> >> + env->exception.target_el = target_el;
> >> + env->pc -= 4;
> >> + } else {
> >> + cs->exception_index = EXCP_YIELD;
> >> + }
> >> cpu_loop_exit(cs);
> >> }
> >
> >
> > Hi Greg,
> >
> > Before trapping, don't you need to check that the CPU has no actual
work?
> > e.g:
> > if (!cc->has_work(cs) && ..)
>
> We don't track WFE-wakeup events, so we must always trap. (Well,
> I suppose strictly we could also go for "never trap" on the basis
> that really our implementation here is pretty much a NOP, but I
> think always-trap is closer to reasonable.)
>
> In theory you could maybe check has_work() for the WFI case,
> since doing an EXCP_HLT really should cause us to stop until
> has_work is true, but it seems a bit fragile -- could we really
> guarantee that nothing would change between this point and
> when we went back through the main loop that would change
> whether has_work evaluates true or not? I think that it's better
> there too to just always take the trap: setting EXCP_HLT is our
> "going into a low power state" and so we should take the trap
> if we would otherwise have done that.
I think functional wise we are OK.
The implementation can AFAIK always choose to nop for whatever reason (e.g
has_work()). Only when we choose to enter low power, the trap comes into
play.
Maybe wfe is the most problematic one because it fires more frequently and
often when has_work() is true?
Cheers,
Edgar
>
> > I think you can still EXCP_YIELD the core if it has work though
> > as it's probably a good place to reschedule other CPUs in our
> > single-threaded SMP model.
>
> That is indeed the only reason we do anything with WFE at all
> (it used to be a no-op, but yielding gives better performance).
>
> -- PMM
[-- Attachment #2: Type: text/html, Size: 3341 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support
2015-04-23 10:39 ` Edgar E. Iglesias
@ 2015-04-23 10:57 ` Peter Maydell
2015-04-23 11:24 ` Edgar E. Iglesias
0 siblings, 1 reply; 33+ messages in thread
From: Peter Maydell @ 2015-04-23 10:57 UTC (permalink / raw)
To: Edgar E. Iglesias
Cc: Sergey Fedorov, Alex Bennée, QEMU Developers, Greg Bellows
On 23 April 2015 at 11:39, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
>
> On 23/04/2015 6:00 pm, "Peter Maydell" <peter.maydell@linaro.org> wrote:
>> In theory you could maybe check has_work() for the WFI case,
>> since doing an EXCP_HLT really should cause us to stop until
>> has_work is true, but it seems a bit fragile -- could we really
>> guarantee that nothing would change between this point and
>> when we went back through the main loop that would change
>> whether has_work evaluates true or not? I think that it's better
>> there too to just always take the trap: setting EXCP_HLT is our
>> "going into a low power state" and so we should take the trap
>> if we would otherwise have done that.
>
> I think functional wise we are OK.
> The implementation can AFAIK always choose to nop for whatever reason (e.g
> has_work()). Only when we choose to enter low power, the trap comes into
> play.
Ah, so in helper_wfi() do something like
if (!has_work()) {
if (trapping wfi) {
EXCP_UDEF code;
} else {
EXCP_HALT code;
}
}
/* otherwise just return, making this WFI a nop */
?
I think that would work.
> Maybe wfe is the most problematic one because it fires more frequently and
> often when has_work() is true?
Yes, I think we should start by not trapping on WFE and then look
at how good/bad perf is.
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support
2015-04-23 10:57 ` Peter Maydell
@ 2015-04-23 11:24 ` Edgar E. Iglesias
2015-04-23 11:28 ` Peter Maydell
0 siblings, 1 reply; 33+ messages in thread
From: Edgar E. Iglesias @ 2015-04-23 11:24 UTC (permalink / raw)
To: Peter Maydell
Cc: Sergey Fedorov, Alex Bennée, QEMU Developers, Greg Bellows
On Thu, Apr 23, 2015 at 11:57:10AM +0100, Peter Maydell wrote:
> On 23 April 2015 at 11:39, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> >
> > On 23/04/2015 6:00 pm, "Peter Maydell" <peter.maydell@linaro.org> wrote:
> >> In theory you could maybe check has_work() for the WFI case,
> >> since doing an EXCP_HLT really should cause us to stop until
> >> has_work is true, but it seems a bit fragile -- could we really
> >> guarantee that nothing would change between this point and
> >> when we went back through the main loop that would change
> >> whether has_work evaluates true or not? I think that it's better
> >> there too to just always take the trap: setting EXCP_HLT is our
> >> "going into a low power state" and so we should take the trap
> >> if we would otherwise have done that.
> >
> > I think functional wise we are OK.
> > The implementation can AFAIK always choose to nop for whatever reason (e.g
> > has_work()). Only when we choose to enter low power, the trap comes into
> > play.
>
> Ah, so in helper_wfi() do something like
>
> if (!has_work()) {
> if (trapping wfi) {
> EXCP_UDEF code;
> } else {
> EXCP_HALT code;
> }
> }
> /* otherwise just return, making this WFI a nop */
>
> ?
>
> I think that would work.
Yes.
Maybe we can consider YIELD instead of NOP when has_work() is true as a WFI
is probably a good hint from guests to reschedule QEMU CPUs.
>
> > Maybe wfe is the most problematic one because it fires more frequently and
> > often when has_work() is true?
>
> Yes, I think we should start by not trapping on WFE and then look
> at how good/bad perf is.
Sounds good to me.
Thanks!
Edgar
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support
2015-04-23 11:24 ` Edgar E. Iglesias
@ 2015-04-23 11:28 ` Peter Maydell
2015-04-23 11:34 ` Edgar E. Iglesias
0 siblings, 1 reply; 33+ messages in thread
From: Peter Maydell @ 2015-04-23 11:28 UTC (permalink / raw)
To: Edgar E. Iglesias
Cc: Sergey Fedorov, Alex Bennée, QEMU Developers, Greg Bellows
On 23 April 2015 at 12:24, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> Maybe we can consider YIELD instead of NOP when has_work() is true as a WFI
> is probably a good hint from guests to reschedule QEMU CPUs.
That I'm not convinced about. If we have a pending interrupt then
our best bet is probably to take it immediately on this CPU, not
yield to another CPU and take the interrupt when we eventually
get control back.
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support
2015-04-23 11:28 ` Peter Maydell
@ 2015-04-23 11:34 ` Edgar E. Iglesias
2015-04-23 14:26 ` Greg Bellows
0 siblings, 1 reply; 33+ messages in thread
From: Edgar E. Iglesias @ 2015-04-23 11:34 UTC (permalink / raw)
To: Peter Maydell
Cc: Sergey Fedorov, Alex Bennée, QEMU Developers, Greg Bellows
On Thu, Apr 23, 2015 at 12:28:43PM +0100, Peter Maydell wrote:
> On 23 April 2015 at 12:24, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > Maybe we can consider YIELD instead of NOP when has_work() is true as a WFI
> > is probably a good hint from guests to reschedule QEMU CPUs.
>
> That I'm not convinced about. If we have a pending interrupt then
> our best bet is probably to take it immediately on this CPU, not
> yield to another CPU and take the interrupt when we eventually
> get control back.
Yeah, true. It's actually a very bad YIELD point when has_work() is true
for a WFI.
For WFE too I guess, when interrupts are unmasked.
Good point.
Cheers,
Edgar
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support
2015-04-23 11:34 ` Edgar E. Iglesias
@ 2015-04-23 14:26 ` Greg Bellows
2015-04-23 14:30 ` Peter Maydell
0 siblings, 1 reply; 33+ messages in thread
From: Greg Bellows @ 2015-04-23 14:26 UTC (permalink / raw)
To: Edgar E. Iglesias
Cc: Peter Maydell, Alex Bennée, QEMU Developers, Sergey Fedorov
[-- Attachment #1: Type: text/plain, Size: 1133 bytes --]
On Thu, Apr 23, 2015 at 6:34 AM, Edgar E. Iglesias <edgar.iglesias@gmail.com
> wrote:
> On Thu, Apr 23, 2015 at 12:28:43PM +0100, Peter Maydell wrote:
> > On 23 April 2015 at 12:24, Edgar E. Iglesias <edgar.iglesias@gmail.com>
> wrote:
> > > Maybe we can consider YIELD instead of NOP when has_work() is true as
> a WFI
> > > is probably a good hint from guests to reschedule QEMU CPUs.
> >
> > That I'm not convinced about. If we have a pending interrupt then
> > our best bet is probably to take it immediately on this CPU, not
> > yield to another CPU and take the interrupt when we eventually
> > get control back.
>
> Yeah, true. It's actually a very bad YIELD point when has_work() is true
> for a WFI.
>
> For WFE too I guess, when interrupts are unmasked.
>
> Good point.
>
> Cheers,
> Edgar
>
Good catch Edgar, we shouldn't trap if we are "going into a low-power
state".
It sounds like we arrived at the following:
wfi- check has_work before taking either the UDEF or HLT exception hereby
making it a noop.
wfe - Never trap, making it always a noop. This includes undoing the
existing YIELD exception.
Sound right?
[-- Attachment #2: Type: text/html, Size: 2521 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support
2015-04-23 14:26 ` Greg Bellows
@ 2015-04-23 14:30 ` Peter Maydell
2015-04-23 14:41 ` Greg Bellows
0 siblings, 1 reply; 33+ messages in thread
From: Peter Maydell @ 2015-04-23 14:30 UTC (permalink / raw)
To: Greg Bellows
Cc: Edgar E. Iglesias, Alex Bennée, QEMU Developers, Sergey Fedorov
On 23 April 2015 at 15:26, Greg Bellows <greg.bellows@linaro.org> wrote:
> Good catch Edgar, we shouldn't trap if we are "going into a low-power
> state".
You mean "if we are not going into a low-power state".
> It sounds like we arrived at the following:
>
> wfi- check has_work before taking either the UDEF or HLT exception hereby
> making it a noop.
This is OK (as per the code sketch in my earlier message).
> wfe - Never trap, making it always a noop. This includes undoing the
> existing YIELD exception.
No, the YIELD behaviour is correct for WFE. This is important
to avoid us spinning for the rest of this vcpu's timeslice
(never making progress because the other vcpu which could
unblock us from this spinlock doesn't run). But we don't ever
want to trap to EL2.
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support
2015-04-23 14:30 ` Peter Maydell
@ 2015-04-23 14:41 ` Greg Bellows
2015-04-23 14:51 ` Peter Maydell
0 siblings, 1 reply; 33+ messages in thread
From: Greg Bellows @ 2015-04-23 14:41 UTC (permalink / raw)
To: Peter Maydell
Cc: Edgar E. Iglesias, Alex Bennée, QEMU Developers, Sergey Fedorov
[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]
On Thu, Apr 23, 2015 at 9:30 AM, Peter Maydell <peter.maydell@linaro.org>
wrote:
> On 23 April 2015 at 15:26, Greg Bellows <greg.bellows@linaro.org> wrote:
> > Good catch Edgar, we shouldn't trap if we are "going into a low-power
> > state".
>
> You mean "if we are not going into a low-power state".
>
Yes, sorry.
>
> > It sounds like we arrived at the following:
> >
> > wfi- check has_work before taking either the UDEF or HLT exception
> hereby
> > making it a noop.
>
> This is OK (as per the code sketch in my earlier message).
>
> > wfe - Never trap, making it always a noop. This includes undoing the
> > existing YIELD exception.
>
> No, the YIELD behaviour is correct for WFE. This is important
> to avoid us spinning for the rest of this vcpu's timeslice
> (never making progress because the other vcpu which could
> unblock us from this spinlock doesn't run). But we don't ever
> want to trap to EL2.
>
Ok it sounds like the primary concern is the performance impact of
constantly trapping wfe to EL2 (in which case HCR must be set to cause
this). Should we still honor EL3 and EL1 trapping ofr wfe? In other words
check if they are the target and allow it if !has_work?
>
> -- PMM
>
[-- Attachment #2: Type: text/html, Size: 2423 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support
2015-04-23 14:41 ` Greg Bellows
@ 2015-04-23 14:51 ` Peter Maydell
2015-04-23 15:00 ` Greg Bellows
0 siblings, 1 reply; 33+ messages in thread
From: Peter Maydell @ 2015-04-23 14:51 UTC (permalink / raw)
To: Greg Bellows
Cc: Edgar E. Iglesias, Alex Bennée, QEMU Developers, Sergey Fedorov
On 23 April 2015 at 15:41, Greg Bellows <greg.bellows@linaro.org> wrote:
> Ok it sounds like the primary concern is the performance impact of
> constantly trapping wfe to EL2 (in which case HCR must be set to cause
> this). Should we still honor EL3 and EL1 trapping ofr wfe? In other words
> check if they are the target and allow it if !has_work?
We should check what the architecture manual says, but I think
that in all the trap-WFE cases it says "traps if the CPU
would go into a low power state". QEMU *never* goes into a
low power state on WFE, so we never trap.
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support
2015-04-23 14:51 ` Peter Maydell
@ 2015-04-23 15:00 ` Greg Bellows
2015-04-23 15:11 ` Peter Maydell
0 siblings, 1 reply; 33+ messages in thread
From: Greg Bellows @ 2015-04-23 15:00 UTC (permalink / raw)
To: Peter Maydell
Cc: Edgar E. Iglesias, Alex Bennée, QEMU Developers, Sergey Fedorov
[-- Attachment #1: Type: text/plain, Size: 949 bytes --]
On Thu, Apr 23, 2015 at 9:51 AM, Peter Maydell <peter.maydell@linaro.org>
wrote:
> On 23 April 2015 at 15:41, Greg Bellows <greg.bellows@linaro.org> wrote:
> > Ok it sounds like the primary concern is the performance impact of
> > constantly trapping wfe to EL2 (in which case HCR must be set to cause
> > this). Should we still honor EL3 and EL1 trapping ofr wfe? In other
> words
> > check if they are the target and allow it if !has_work?
>
> We should check what the architecture manual says, but I think
> that in all the trap-WFE cases it says "traps if the CPU
> would go into a low power state". QEMU *never* goes into a
> low power state on WFE, so we never trap.
>
Right, that is exactly what it says. So if QEMU never goes into a low
power state than neither wfi or wfe should trap as they have the same
explanation. In which case the best solution is to pull the patches out.
That work?
>
> -- PMM
>
[-- Attachment #2: Type: text/html, Size: 1673 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support
2015-04-23 15:00 ` Greg Bellows
@ 2015-04-23 15:11 ` Peter Maydell
0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2015-04-23 15:11 UTC (permalink / raw)
To: Greg Bellows
Cc: Edgar E. Iglesias, Alex Bennée, QEMU Developers, Sergey Fedorov
On 23 April 2015 at 16:00, Greg Bellows <greg.bellows@linaro.org> wrote:
>
>
> On Thu, Apr 23, 2015 at 9:51 AM, Peter Maydell <peter.maydell@linaro.org>
> wrote:
>> We should check what the architecture manual says, but I think
>> that in all the trap-WFE cases it says "traps if the CPU
>> would go into a low power state". QEMU *never* goes into a
>> low power state on WFE, so we never trap.
>
>
> Right, that is exactly what it says. So if QEMU never goes into a low power
> state than neither wfi or wfe should trap as they have the same explanation.
> In which case the best solution is to pull the patches out. That work?
No, because for WFI we *do* go into a low-power state (EXCP_HLT),
so we must trap.
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support
2015-04-22 17:09 [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support Greg Bellows
` (8 preceding siblings ...)
2015-04-22 17:09 ` [Qemu-devel] [PATCH v2 9/9] target-arm: Add WFx instruction trap support Greg Bellows
@ 2015-04-23 3:37 ` Edgar E. Iglesias
2015-04-23 8:06 ` Peter Maydell
2015-04-23 10:10 ` Peter Maydell
2015-05-18 17:53 ` Peter Maydell
10 siblings, 2 replies; 33+ messages in thread
From: Edgar E. Iglesias @ 2015-04-23 3:37 UTC (permalink / raw)
To: Greg Bellows; +Cc: peter.maydell, alex.bennee, qemu-devel, serge.fdrv
On Wed, Apr 22, 2015 at 12:09:12PM -0500, Greg Bellows wrote:
> Initial patchset adding support for trapping to an EL other than EL1. Support
> includes changes to interfaces to allow specification of the target EL. Also
> includes the addition of the ARMv8 CPTR system registers used for controlling
> the trapping of features.
Hi Greg,
I'm not sure I see the value with the target_el member, it felt OK
to call the route function from the do_interrupt functions but I may
be missing something.
I had a comment on the trapping of WFX, I recall not checking for
has_work was causing a lot of exception round-trips when running
XEN. To the point were things almost stopped working.
I didn't check other registers and bit details from the ARM spec but
in general the series looks good.
Acked-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Thanks,
Edgar
>
> ---
>
> v1 -> v2
> - Removed use of MAX through out the patch when setting target_el
> - Updated FP trap support to pass a target exception EL rather than just an
> enable boolean.
> - Added utility functions for determining the target exception EL
> - Broke up cptr and cpacr access functions
> - Added HCPTR CP register entry and HCPTR/CPTR_EL2 zero entries
> - Broke out TCR changes into their own patch and added support for handling the
> lack of TTBR1
> - Simplified wfx checking
> - General comment cleanup
>
> Greg Bellows (9):
> 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 TTBR regime function and use
> target-arm: Add EL3 and EL2 TCR checking
> target-arm: Add WFx syndrome function
> target-arm: Add WFx instruction trap support
>
> target-arm/cpu.c | 61 +++++++++++------
> target-arm/cpu.h | 76 ++++++++++++++++-----
> target-arm/helper-a64.c | 2 +-
> target-arm/helper.c | 160 +++++++++++++++++++++++++++++----------------
> target-arm/helper.h | 2 +-
> target-arm/internals.h | 6 ++
> target-arm/op_helper.c | 98 +++++++++++++++++++++++++--
> target-arm/translate-a64.c | 40 +++++++-----
> target-arm/translate.c | 70 ++++++++++++--------
> target-arm/translate.h | 17 ++++-
> 10 files changed, 390 insertions(+), 142 deletions(-)
>
> --
> 1.8.3.2
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support
2015-04-23 3:37 ` [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 " Edgar E. Iglesias
@ 2015-04-23 8:06 ` Peter Maydell
2015-04-23 10:10 ` Peter Maydell
1 sibling, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2015-04-23 8:06 UTC (permalink / raw)
To: Edgar E. Iglesias
Cc: Sergey Fedorov, Alex Bennée, QEMU Developers, Greg Bellows
On 23 April 2015 at 04:37, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Wed, Apr 22, 2015 at 12:09:12PM -0500, Greg Bellows wrote:
>> Initial patchset adding support for trapping to an EL other than EL1. Support
>> includes changes to interfaces to allow specification of the target EL. Also
>> includes the addition of the ARMv8 CPTR system registers used for controlling
>> the trapping of features.
>
> Hi Greg,
>
> I'm not sure I see the value with the target_el member, it felt OK
> to call the route function from the do_interrupt functions but I may
> be missing something.
The problem is that for some exceptions (particularly the ones which are
"some trap bit means an insn causes a trap to EL2 or EL3") you can't
determine the target-el after the fact: all you have is "EXCP_UDEF"
and you don't know which of the various possible trappable insns it
was, so you don't know whether to take it to EL1, EL2 or EL3.
There were two solutions to this that we tossed around:
(1) this one, where you determine the target EL for the trap at
translate time (by encoding the trap bits into the TB flags,
or at least the target EL for each kind of trap)
(2) an approach which splits EXCP_UDEF up into effectively one
exception type for each case of trappable instructions, and
then check the trap bits at runtime to figure out the routing
Once you've decided on approach (1) you might as well use the new
exception.target_el member to avoid repeating the routing calculations
for IRQ, FIQ etc.
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support
2015-04-23 3:37 ` [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 " Edgar E. Iglesias
2015-04-23 8:06 ` Peter Maydell
@ 2015-04-23 10:10 ` Peter Maydell
2015-04-23 10:27 ` Edgar E. Iglesias
2015-04-23 14:05 ` Greg Bellows
1 sibling, 2 replies; 33+ messages in thread
From: Peter Maydell @ 2015-04-23 10:10 UTC (permalink / raw)
To: Edgar E. Iglesias
Cc: Sergey Fedorov, Alex Bennée, QEMU Developers, Greg Bellows
On 23 April 2015 at 04:37, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> I had a comment on the trapping of WFX, I recall not checking for
> has_work was causing a lot of exception round-trips when running
> XEN. To the point were things almost stopped working.
For traps on WFE, or traps on WFI? If the former, we should
probably implement that as "never trap" since we don't actually
go into a 'low power state' for WFE (execution will always
continue even if there's no pending WFE event).
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support
2015-04-23 10:10 ` Peter Maydell
@ 2015-04-23 10:27 ` Edgar E. Iglesias
2015-04-23 14:05 ` Greg Bellows
1 sibling, 0 replies; 33+ messages in thread
From: Edgar E. Iglesias @ 2015-04-23 10:27 UTC (permalink / raw)
To: Peter Maydell
Cc: Sergey Fedorov, Alex Bennée, QEMU Developers, Greg Bellows
[-- Attachment #1: Type: text/plain, Size: 974 bytes --]
---
Sent from my phone
On 23/04/2015 8:10 pm, "Peter Maydell" <peter.maydell@linaro.org> wrote:
>
> On 23 April 2015 at 04:37, Edgar E. Iglesias <edgar.iglesias@gmail.com>
wrote:
> > I had a comment on the trapping of WFX, I recall not checking for
> > has_work was causing a lot of exception round-trips when running
> > XEN. To the point were things almost stopped working.
>
> For traps on WFE, or traps on WFI? If the former, we should
> probably implement that as "never trap" since we don't actually
> go into a 'low power state' for WFE (execution will always
> continue even if there's no pending WFE event).
>
> -- PMM
Hi,
I think it was for both, I can retest it to double check.
Our (xilinx) tree tracks wfe wakeups.
I agree that without tracking we better not trap..
When running emulated SMP with multiple nested SMP guests there are a lot
of these events fired. The exception code paths down to hypervisors involve
fairly long round trips.
Cheers,
Edgar
[-- Attachment #2: Type: text/html, Size: 1355 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support
2015-04-23 10:10 ` Peter Maydell
2015-04-23 10:27 ` Edgar E. Iglesias
@ 2015-04-23 14:05 ` Greg Bellows
1 sibling, 0 replies; 33+ messages in thread
From: Greg Bellows @ 2015-04-23 14:05 UTC (permalink / raw)
To: Peter Maydell
Cc: Edgar E. Iglesias, Alex Bennée, QEMU Developers, Sergey Fedorov
[-- Attachment #1: Type: text/plain, Size: 1041 bytes --]
On Thu, Apr 23, 2015 at 5:10 AM, Peter Maydell <peter.maydell@linaro.org>
wrote:
> On 23 April 2015 at 04:37, Edgar E. Iglesias <edgar.iglesias@gmail.com>
> wrote:
> > I had a comment on the trapping of WFX, I recall not checking for
> > has_work was causing a lot of exception round-trips when running
> > XEN. To the point were things almost stopped working.
>
> For traps on WFE, or traps on WFI? If the former, we should
> probably implement that as "never trap" since we don't actually
> go into a 'low power state' for WFE (execution will always
> continue even if there's no pending WFE event).
>
> -- PMM
>
I did notice that issuing a wfi without trapping causes QEMU to hang. I
had to make sure to not test the non-trapping case in EL1. I didn't
investigate too deeply, but I figured that there was nothing to wake it up.
The WFx stuff was a good test case for the trap to ELn, but if it is
troublesome I can pull the patch until we arrive at a reasonable solution.
The FP stuff now also tests this.
[-- Attachment #2: Type: text/html, Size: 1763 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support
2015-04-22 17:09 [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 trap support Greg Bellows
` (9 preceding siblings ...)
2015-04-23 3:37 ` [Qemu-devel] [PATCH v2 0/9] target-arm: EL3 " Edgar E. Iglesias
@ 2015-05-18 17:53 ` Peter Maydell
10 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2015-05-18 17:53 UTC (permalink / raw)
To: Greg Bellows; +Cc: Sergey Fedorov, Alex Bennée, QEMU Developers
On 22 April 2015 at 18:09, Greg Bellows <greg.bellows@linaro.org> wrote:
> Initial patchset adding support for trapping to an EL other than EL1. Support
> includes changes to interfaces to allow specification of the target EL. Also
> includes the addition of the ARMv8 CPTR system registers used for controlling
> the trapping of features.
>
> ---
>
> v1 -> v2
> - Removed use of MAX through out the patch when setting target_el
> - Updated FP trap support to pass a target exception EL rather than just an
> enable boolean.
> - Added utility functions for determining the target exception EL
> - Broke up cptr and cpacr access functions
> - Added HCPTR CP register entry and HCPTR/CPTR_EL2 zero entries
> - Broke out TCR changes into their own patch and added support for handling the
> lack of TTBR1
> - Simplified wfx checking
> - General comment cleanup
>
> Greg Bellows (9):
> 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 TTBR regime function and use
> target-arm: Add EL3 and EL2 TCR checking
> target-arm: Add WFx syndrome function
> target-arm: Add WFx instruction trap support
Of this series, patches 1,2,3,6,7,8 look good so I'm applying
them to target-arm.next (with the uint64_t fixup Sergey spotted
for patch 6). I've sent comments on patches 4 and 5. 9 might be
OK but there was enough discussion on it to merit a respin anyway.
(Since Greg has moved on from Linaro I will be doing the respin.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread