From c61e22627deb0b236342ceb9643dd3cd3a78cdbb Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 18 Nov 2019 16:50:12 -0500 Subject: [PATCH] target/arm: Fix routing of singlestep exceptions When generating an architectural single-step exception we were routing it to the "default exception level", which is to say the same exception level we execute at except that EL0 exceptions go to EL1. This is incorrect because the debug exception level can be configured by the guest for situations such as single stepping of EL0 and EL1 code by EL2. We have to track the target debug exception level in the TB flags, because it is dependent on CPU state like HCR_EL2.TGE and MDCR_EL2.TDE. (That we were previously calling the arm_debug_target_el() function to determine dc->ss_same_el is itself a bug, though one that would only have manifested as incorrect syndrome information.) Since we are out of TB flag bits unless we want to expand into the cs_base field, we share some bits with the M-profile only HANDLER and STACKCHECK bits, since only A-profile has this singlestep. Fixes: https://bugs.launchpad.net/qemu/+bug/1838913 Backports commit 8bd587c1066f4456ddfe611b571d9439a947d74c from qemu --- qemu/target/arm/cpu.h | 5 +++++ qemu/target/arm/helper.c | 6 ++++++ qemu/target/arm/translate-a64.c | 2 +- qemu/target/arm/translate.c | 4 +++- qemu/target/arm/translate.h | 15 +++++++++++---- 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/qemu/target/arm/cpu.h b/qemu/target/arm/cpu.h index 87b0b401..7d321075 100644 --- a/qemu/target/arm/cpu.h +++ b/qemu/target/arm/cpu.h @@ -3101,6 +3101,11 @@ FIELD(TBFLAG_ANY, PSTATE_SS, 26, 1) /* Target EL if we take a floating-point-disabled exception */ FIELD(TBFLAG_ANY, FPEXC_EL, 24, 2) FIELD(TBFLAG_ANY, BE_DATA, 23, 1) +/* + * For A-profile only, target EL for debug exceptions. + * Note that this overlaps with the M-profile-only HANDLER and STACKCHECK bits. + */ +FIELD(TBFLAG_ANY, DEBUG_TARGET_EL, 21, 2) /* Bit usage when in AArch32 state: */ FIELD(TBFLAG_A32, THUMB, 0, 1) diff --git a/qemu/target/arm/helper.c b/qemu/target/arm/helper.c index b7996f2e..aa376e10 100644 --- a/qemu/target/arm/helper.c +++ b/qemu/target/arm/helper.c @@ -11034,6 +11034,12 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc, } } + if (!arm_feature(env, ARM_FEATURE_M)) { + int target_el = arm_debug_target_el(env); + + flags = FIELD_DP32(flags, TBFLAG_ANY, DEBUG_TARGET_EL, target_el); + } + *pflags = flags; *cs_base = 0; } diff --git a/qemu/target/arm/translate-a64.c b/qemu/target/arm/translate-a64.c index 28dec27c..06990c26 100644 --- a/qemu/target/arm/translate-a64.c +++ b/qemu/target/arm/translate-a64.c @@ -14523,7 +14523,7 @@ static void aarch64_tr_init_disas_context(DisasContextBase *dcbase, dc->ss_active = FIELD_EX32(tb_flags, TBFLAG_ANY, SS_ACTIVE); dc->pstate_ss = FIELD_EX32(tb_flags, TBFLAG_ANY, PSTATE_SS); dc->is_ldex = false; - dc->ss_same_el = (arm_debug_target_el(env) == dc->current_el); + dc->debug_target_el = FIELD_EX32(tb_flags, TBFLAG_ANY, DEBUG_TARGET_EL); /* Bound the number of insns to execute to those left on the page. */ bound = -(dc->base.pc_first | TARGET_PAGE_MASK) / 4; diff --git a/qemu/target/arm/translate.c b/qemu/target/arm/translate.c index 399a67fa..f9119521 100644 --- a/qemu/target/arm/translate.c +++ b/qemu/target/arm/translate.c @@ -12064,7 +12064,9 @@ static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) dc->ss_active = FIELD_EX32(tb_flags, TBFLAG_ANY, SS_ACTIVE); dc->pstate_ss = FIELD_EX32(tb_flags, TBFLAG_ANY, PSTATE_SS); dc->is_ldex = false; - dc->ss_same_el = false; /* Can't be true since EL_d must be AArch64 */ + if (!arm_feature(env, ARM_FEATURE_M)) { + dc->debug_target_el = FIELD_EX32(tb_flags, TBFLAG_ANY, DEBUG_TARGET_EL); + } dc->page_start = dc->base.pc_first & TARGET_PAGE_MASK; diff --git a/qemu/target/arm/translate.h b/qemu/target/arm/translate.h index 4be0b3a6..deaa95a8 100644 --- a/qemu/target/arm/translate.h +++ b/qemu/target/arm/translate.h @@ -49,6 +49,8 @@ typedef struct DisasContext { uint32_t svc_imm; int aarch64; int current_el; + /* Debug target exception level for single-step exceptions */ + int debug_target_el; GHashTable *cp_regs; uint64_t features; /* CPU features bits */ /* Because unallocated encodings generate different exception syndrome @@ -69,8 +71,6 @@ typedef struct DisasContext { * ie A64 LDX*, LDAX*, A32/T32 LDREX*, LDAEX*. */ bool is_ldex; - /* True if a single-step exception will be taken to the current EL */ - bool ss_same_el; /* True if v8.3-PAuth is active. */ bool pauth_active; /* Bottom two bits of XScale c15_cpar coprocessor access control reg */ @@ -258,8 +258,15 @@ static inline void gen_exception(DisasContext *s, int excp, uint32_t syndrome, /* Generate an architectural singlestep exception */ static inline void gen_swstep_exception(DisasContext *s, int isv, int ex) { - gen_exception(s, EXCP_UDEF, syn_swstep(s->ss_same_el, isv, ex), - default_exception_el(s)); + bool same_el = (s->debug_target_el == s->current_el); + + /* + * If singlestep is targeting a lower EL than the current one, + * then s->ss_active must be false and we can never get here. + */ + assert(s->debug_target_el >= s->current_el); + + gen_exception(s, EXCP_UDEF, syn_swstep(same_el, isv, ex), s->debug_target_el); } /*