* [PATCH v2 01/25] target/arm: Mark exception helpers as noreturn
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-07 2:47 ` [PATCH v2 02/25] target/arm: Add coproc parameter to syn_fp_access_trap Richard Henderson
` (23 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/helper.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index b1334e0c42..5161cdf73d 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -44,9 +44,9 @@ 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_4(exception_with_syndrome, void, env, i32, i32, i32)
-DEF_HELPER_2(exception_bkpt_insn, void, env, i32)
+DEF_HELPER_2(exception_internal, noreturn, env, i32)
+DEF_HELPER_4(exception_with_syndrome, noreturn, env, i32, i32, i32)
+DEF_HELPER_2(exception_bkpt_insn, noreturn, env, i32)
DEF_HELPER_2(exception_pc_alignment, noreturn, env, tl)
DEF_HELPER_1(setend, void, env)
DEF_HELPER_2(wfi, void, env, i32)
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 02/25] target/arm: Add coproc parameter to syn_fp_access_trap
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
2022-06-07 2:47 ` [PATCH v2 01/25] target/arm: Mark exception helpers as noreturn Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 15:55 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 03/25] target/arm: Move fp access syndrome adjust out of raise_exception Richard Henderson
` (22 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
With ARMv8, this field is always RES0.
With ARMv7, targeting EL2 and TA=0, it is always 0xA.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/syndrome.h | 7 ++++---
target/arm/translate-a64.c | 3 ++-
target/arm/translate-vfp.c | 14 ++++++++++++--
3 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/target/arm/syndrome.h b/target/arm/syndrome.h
index 0cb26dde7d..c105f9e6ba 100644
--- a/target/arm/syndrome.h
+++ b/target/arm/syndrome.h
@@ -185,12 +185,13 @@ static inline uint32_t syn_cp15_rrt_trap(int cv, int cond, int opc1, int crm,
| (rt2 << 10) | (rt << 5) | (crm << 1) | isread;
}
-static inline uint32_t syn_fp_access_trap(int cv, int cond, bool is_16bit)
+static inline uint32_t syn_fp_access_trap(int cv, int cond, bool is_16bit,
+ int coproc)
{
- /* AArch32 FP trap or any AArch64 FP/SIMD trap: TA == 0 coproc == 0xa */
+ /* AArch32 FP trap or any AArch64 FP/SIMD trap: TA == 0 */
return (EC_ADVSIMDFPACCESSTRAP << ARM_EL_EC_SHIFT)
| (is_16bit ? 0 : ARM_EL_IL)
- | (cv << 24) | (cond << 20) | 0xa;
+ | (cv << 24) | (cond << 20) | coproc;
}
static inline uint32_t syn_simd_access_trap(int cv, int cond, bool is_16bit)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 935e1929bb..14b90a37ea 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1162,7 +1162,8 @@ static bool fp_access_check(DisasContext *s)
s->fp_access_checked = true;
gen_exception_insn(s, s->pc_curr, EXCP_UDEF,
- syn_fp_access_trap(1, 0xe, false), s->fp_excp_el);
+ syn_fp_access_trap(1, 0xe, false, 0),
+ s->fp_excp_el);
return false;
}
s->fp_access_checked = true;
diff --git a/target/arm/translate-vfp.c b/target/arm/translate-vfp.c
index 40a513b822..0f797c56fd 100644
--- a/target/arm/translate-vfp.c
+++ b/target/arm/translate-vfp.c
@@ -219,8 +219,18 @@ static void gen_update_fp_context(DisasContext *s)
static bool vfp_access_check_a(DisasContext *s, bool ignore_vfp_enabled)
{
if (s->fp_excp_el) {
- gen_exception_insn(s, s->pc_curr, EXCP_UDEF,
- syn_fp_access_trap(1, 0xe, false), s->fp_excp_el);
+ /*
+ * The full syndrome is only used for HSR when HCPTR traps:
+ * For v8, when TA==0, coproc is RES0.
+ * For v7, any use of a Floating-point instruction or access
+ * to a Floating-point Extension register that is trapped to
+ * Hyp mode because of a trap configured in the HCPTR sets
+ * this field to 0xA.
+ */
+ int coproc = arm_dc_feature(s, ARM_FEATURE_V8) ? 0 : 0xa;
+ uint32_t syn = syn_fp_access_trap(1, 0xe, false, coproc);
+
+ gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn, s->fp_excp_el);
return false;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 03/25] target/arm: Move fp access syndrome adjust out of raise_exception
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
2022-06-07 2:47 ` [PATCH v2 01/25] target/arm: Mark exception helpers as noreturn Richard Henderson
2022-06-07 2:47 ` [PATCH v2 02/25] target/arm: Add coproc parameter to syn_fp_access_trap Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 15:59 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 04/25] target/arm: Move exception_target_el out of line Richard Henderson
` (21 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Create helper_exception_advsimdfp_access to handle both
the routing and the syndrome contents, depending on the
eventual target EL and mode.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/helper.h | 1 +
target/arm/translate.h | 9 +++++++++
target/arm/op_helper.c | 32 +++++++++++++++++++++++---------
target/arm/translate-a64.c | 5 ++---
target/arm/translate-vfp.c | 4 +++-
5 files changed, 38 insertions(+), 13 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index 5161cdf73d..a13007699d 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -46,6 +46,7 @@ DEF_HELPER_FLAGS_3(sel_flags, TCG_CALL_NO_RWG_SE,
i32, i32, i32, i32)
DEF_HELPER_2(exception_internal, noreturn, env, i32)
DEF_HELPER_4(exception_with_syndrome, noreturn, env, i32, i32, i32)
+DEF_HELPER_3(exception_advsimdfp_access, noreturn, env, i32, i32)
DEF_HELPER_2(exception_bkpt_insn, noreturn, env, i32)
DEF_HELPER_2(exception_pc_alignment, noreturn, env, tl)
DEF_HELPER_1(setend, void, env)
diff --git a/target/arm/translate.h b/target/arm/translate.h
index 9f0bb270c5..da5f89d49d 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -337,6 +337,15 @@ static inline void gen_exception(int excp, uint32_t syndrome,
tcg_constant_i32(target_el));
}
+static inline void gen_exception_advsimdfp_access(DisasContext *s,
+ uint32_t syndrome)
+{
+ gen_helper_exception_advsimdfp_access(cpu_env,
+ tcg_constant_i32(syndrome),
+ tcg_constant_i32(s->fp_excp_el));
+ s->base.is_jmp = DISAS_NORETURN;
+}
+
/* Generate an architectural singlestep exception */
static inline void gen_swstep_exception(DisasContext *s, int isv, int ex)
{
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index c4bd668870..67b7dbeaa9 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -34,16 +34,8 @@ void raise_exception(CPUARMState *env, uint32_t excp,
CPUState *cs = env_cpu(env);
if (target_el == 1 && (arm_hcr_el2_eff(env) & HCR_TGE)) {
- /*
- * Redirect NS EL1 exceptions to NS EL2. These are reported with
- * their original syndrome register value, with the exception of
- * SIMD/FP access traps, which are reported as uncategorized
- * (see DDI0478C.a D1.10.4)
- */
+ /* Redirect NS EL1 exceptions to NS EL2. */
target_el = 2;
- if (syn_get_ec(syndrome) == EC_ADVSIMDFPACCESSTRAP) {
- syndrome = syn_uncategorized();
- }
}
assert(!excp_is_internal(excp));
@@ -384,6 +376,28 @@ void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp,
raise_exception(env, excp, syndrome, target_el);
}
+/* Raise an exception with EC_ADVSIMDFPACCESS. */
+void HELPER(exception_advsimdfp_access)(CPUARMState *env,
+ uint32_t syndrome, uint32_t target_el)
+{
+ if (target_el == 1 && (arm_hcr_el2_eff(env) & HCR_TGE)) {
+ /*
+ * SIMD/FP access traps, when re-routed to EL2, are reported with
+ * uncategorized syndrome. See DDI0487H.a rule RJNBTN.
+ */
+ target_el = 2;
+ syndrome = syn_uncategorized();
+ } else if (arm_el_is_aa64(env, target_el)) {
+ /*
+ * From AArch32, we will have filled in TA and/or COPROC for use
+ * by aa32 HSR. But in aa64 ESR_ELx, the low 20 bits are RES0.
+ */
+ syndrome &= 0xfff00000;
+ }
+
+ raise_exception(env, EXCP_UDEF, syndrome, target_el);
+}
+
/* Raise an EXCP_BKPT with the specified syndrome register value,
* targeting the correct exception level for debug exceptions.
*/
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 14b90a37ea..3251c69180 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1161,9 +1161,8 @@ static bool fp_access_check(DisasContext *s)
assert(!s->fp_access_checked);
s->fp_access_checked = true;
- gen_exception_insn(s, s->pc_curr, EXCP_UDEF,
- syn_fp_access_trap(1, 0xe, false, 0),
- s->fp_excp_el);
+ gen_a64_set_pc_im(s->pc_curr);
+ gen_exception_advsimdfp_access(s, syn_fp_access_trap(1, 0xe, false, 0));
return false;
}
s->fp_access_checked = true;
diff --git a/target/arm/translate-vfp.c b/target/arm/translate-vfp.c
index 0f797c56fd..2a98387411 100644
--- a/target/arm/translate-vfp.c
+++ b/target/arm/translate-vfp.c
@@ -230,7 +230,9 @@ static bool vfp_access_check_a(DisasContext *s, bool ignore_vfp_enabled)
int coproc = arm_dc_feature(s, ARM_FEATURE_V8) ? 0 : 0xa;
uint32_t syn = syn_fp_access_trap(1, 0xe, false, coproc);
- gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn, s->fp_excp_el);
+ gen_set_condexec(s);
+ gen_set_pc_im(s, s->pc_curr);
+ gen_exception_advsimdfp_access(s, syn);
return false;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH v2 03/25] target/arm: Move fp access syndrome adjust out of raise_exception
2022-06-07 2:47 ` [PATCH v2 03/25] target/arm: Move fp access syndrome adjust out of raise_exception Richard Henderson
@ 2022-06-09 15:59 ` Peter Maydell
2022-06-09 17:09 ` Richard Henderson
0 siblings, 1 reply; 49+ messages in thread
From: Peter Maydell @ 2022-06-09 15:59 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Tue, 7 Jun 2022 at 03:50, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Create helper_exception_advsimdfp_access to handle both
> the routing and the syndrome contents, depending on the
> eventual target EL and mode.
Bit awkward to have "TGE means go to EL2, not EL1" in two places
now, but I'm going to assume that either (a) this gets cleaned up
later in the series or (b) that it's the least-worst option, so
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v2 03/25] target/arm: Move fp access syndrome adjust out of raise_exception
2022-06-09 15:59 ` Peter Maydell
@ 2022-06-09 17:09 ` Richard Henderson
0 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2022-06-09 17:09 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, qemu-arm
On 6/9/22 08:59, Peter Maydell wrote:
> On Tue, 7 Jun 2022 at 03:50, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Create helper_exception_advsimdfp_access to handle both
>> the routing and the syndrome contents, depending on the
>> eventual target EL and mode.
>
> Bit awkward to have "TGE means go to EL2, not EL1" in two places
> now, but I'm going to assume that either (a) this gets cleaned up
> later in the series or (b) that it's the least-worst option, so
Hmm, well. I had planned for it to go away from within raise_exception later in the
series, but it didn't work out, so I could drop this.
r~
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 04/25] target/arm: Move exception_target_el out of line
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (2 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 03/25] target/arm: Move fp access syndrome adjust out of raise_exception Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:00 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 05/25] target/arm: Move arm_singlestep_active " Richard Henderson
` (20 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Move the function to op_helper.c, near raise_exception.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/internals.h | 16 +---------------
target/arm/op_helper.c | 15 +++++++++++++++
2 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index b654bee468..36c33a6ac5 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1090,21 +1090,6 @@ typedef struct ARMVAParameters {
ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va,
ARMMMUIdx mmu_idx, bool data);
-static inline 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;
-}
-
/* Determine if allocation tags are available. */
static inline bool allocation_tag_access_enabled(CPUARMState *env, int el,
uint64_t sctlr)
@@ -1329,5 +1314,6 @@ void define_cortex_a72_a57_a53_cp_reginfo(ARMCPU *cpu);
#endif
void aa32_max_features(ARMCPU *cpu);
+int exception_target_el(CPUARMState *env);
#endif
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 67b7dbeaa9..00a082d1f7 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -28,6 +28,21 @@
#define SIGNBIT (uint32_t)0x80000000
#define SIGNBIT64 ((uint64_t)1 << 63)
+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;
+}
+
void raise_exception(CPUARMState *env, uint32_t excp,
uint32_t syndrome, uint32_t target_el)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 05/25] target/arm: Move arm_singlestep_active out of line
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (3 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 04/25] target/arm: Move exception_target_el out of line Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-07 2:47 ` [PATCH v2 06/25] target/arm: Move arm_generate_debug_exceptions " Richard Henderson
` (19 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Move the function to debug_helper.c, and the
declaration to internals.h.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu.h | 10 ----------
target/arm/internals.h | 1 +
target/arm/debug_helper.c | 12 ++++++++++++
3 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index c1865ad5da..2e115a0281 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3087,16 +3087,6 @@ static inline bool arm_generate_debug_exceptions(CPUARMState *env)
}
}
-/* Is single-stepping active? (Note that the "is EL_D AArch64?" check
- * implicitly means this always returns false in pre-v8 CPUs.)
- */
-static inline bool arm_singlestep_active(CPUARMState *env)
-{
- return extract32(env->cp15.mdscr_el1, 0, 1)
- && arm_el_is_aa64(env, arm_debug_target_el(env))
- && arm_generate_debug_exceptions(env);
-}
-
static inline bool arm_sctlr_b(CPUARMState *env)
{
return
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 36c33a6ac5..148f2a9252 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1315,5 +1315,6 @@ void define_cortex_a72_a57_a53_cp_reginfo(ARMCPU *cpu);
void aa32_max_features(ARMCPU *cpu);
int exception_target_el(CPUARMState *env);
+bool arm_singlestep_active(CPUARMState *env);
#endif
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index 46893697cc..1abf41c5f8 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -11,6 +11,18 @@
#include "exec/exec-all.h"
#include "exec/helper-proto.h"
+
+/*
+ * Is single-stepping active? (Note that the "is EL_D AArch64?" check
+ * implicitly means this always returns false in pre-v8 CPUs.)
+ */
+bool arm_singlestep_active(CPUARMState *env)
+{
+ return extract32(env->cp15.mdscr_el1, 0, 1)
+ && arm_el_is_aa64(env, arm_debug_target_el(env))
+ && arm_generate_debug_exceptions(env);
+}
+
/* Return true if the linked breakpoint entry lbn passes its checks */
static bool linked_bp_matches(ARMCPU *cpu, int lbn)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 06/25] target/arm: Move arm_generate_debug_exceptions out of line
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (4 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 05/25] target/arm: Move arm_singlestep_active " Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-07 2:47 ` [PATCH v2 07/25] target/arm: Use is_a64 in arm_generate_debug_exceptions Richard Henderson
` (18 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Move arm_generate_debug_exceptions and its two subroutines,
{aa32,aa64}_generate_debug_exceptions into debug_helper.c,
and the one interface declaration to internals.h.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu.h | 91 -------------------------------------
target/arm/internals.h | 1 +
target/arm/debug_helper.c | 94 +++++++++++++++++++++++++++++++++++++++
3 files changed, 95 insertions(+), 91 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 2e115a0281..92c9758e86 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2996,97 +2996,6 @@ static inline bool arm_v7m_csselr_razwi(ARMCPU *cpu)
return (cpu->clidr & R_V7M_CLIDR_CTYPE_ALL_MASK) != 0;
}
-/* See AArch64.GenerateDebugExceptionsFrom() in ARM ARM pseudocode */
-static inline bool aa64_generate_debug_exceptions(CPUARMState *env)
-{
- int cur_el = arm_current_el(env);
- int debug_el;
-
- if (cur_el == 3) {
- return false;
- }
-
- /* MDCR_EL3.SDD disables debug events from Secure state */
- if (arm_is_secure_below_el3(env)
- && extract32(env->cp15.mdcr_el3, 16, 1)) {
- return false;
- }
-
- /*
- * Same EL to same EL debug exceptions need MDSCR_KDE enabled
- * while not masking the (D)ebug bit in DAIF.
- */
- debug_el = arm_debug_target_el(env);
-
- if (cur_el == debug_el) {
- return extract32(env->cp15.mdscr_el1, 13, 1)
- && !(env->daif & PSTATE_D);
- }
-
- /* Otherwise the debug target needs to be a higher EL */
- return debug_el > cur_el;
-}
-
-static inline bool aa32_generate_debug_exceptions(CPUARMState *env)
-{
- int el = arm_current_el(env);
-
- if (el == 0 && arm_el_is_aa64(env, 1)) {
- return aa64_generate_debug_exceptions(env);
- }
-
- if (arm_is_secure(env)) {
- int spd;
-
- if (el == 0 && (env->cp15.sder & 1)) {
- /* SDER.SUIDEN means debug exceptions from Secure EL0
- * are always enabled. Otherwise they are controlled by
- * SDCR.SPD like those from other Secure ELs.
- */
- return true;
- }
-
- spd = extract32(env->cp15.mdcr_el3, 14, 2);
- switch (spd) {
- case 1:
- /* SPD == 0b01 is reserved, but behaves as 0b00. */
- case 0:
- /* For 0b00 we return true if external secure invasive debug
- * is enabled. On real hardware this is controlled by external
- * signals to the core. QEMU always permits debug, and behaves
- * as if DBGEN, SPIDEN, NIDEN and SPNIDEN are all tied high.
- */
- return true;
- case 2:
- return false;
- case 3:
- return true;
- }
- }
-
- return el != 2;
-}
-
-/* Return true if debugging exceptions are currently enabled.
- * This corresponds to what in ARM ARM pseudocode would be
- * if UsingAArch32() then
- * return AArch32.GenerateDebugExceptions()
- * else
- * return AArch64.GenerateDebugExceptions()
- * We choose to push the if() down into this function for clarity,
- * since the pseudocode has it at all callsites except for the one in
- * CheckSoftwareStep(), where it is elided because both branches would
- * always return the same value.
- */
-static inline bool arm_generate_debug_exceptions(CPUARMState *env)
-{
- if (env->aarch64) {
- return aa64_generate_debug_exceptions(env);
- } else {
- return aa32_generate_debug_exceptions(env);
- }
-}
-
static inline bool arm_sctlr_b(CPUARMState *env)
{
return
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 148f2a9252..e9ac54f244 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1316,5 +1316,6 @@ void define_cortex_a72_a57_a53_cp_reginfo(ARMCPU *cpu);
void aa32_max_features(ARMCPU *cpu);
int exception_target_el(CPUARMState *env);
bool arm_singlestep_active(CPUARMState *env);
+bool arm_generate_debug_exceptions(CPUARMState *env);
#endif
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index 1abf41c5f8..20a0e4261a 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -12,6 +12,100 @@
#include "exec/helper-proto.h"
+/* See AArch64.GenerateDebugExceptionsFrom() in ARM ARM pseudocode */
+static bool aa64_generate_debug_exceptions(CPUARMState *env)
+{
+ int cur_el = arm_current_el(env);
+ int debug_el;
+
+ if (cur_el == 3) {
+ return false;
+ }
+
+ /* MDCR_EL3.SDD disables debug events from Secure state */
+ if (arm_is_secure_below_el3(env)
+ && extract32(env->cp15.mdcr_el3, 16, 1)) {
+ return false;
+ }
+
+ /*
+ * Same EL to same EL debug exceptions need MDSCR_KDE enabled
+ * while not masking the (D)ebug bit in DAIF.
+ */
+ debug_el = arm_debug_target_el(env);
+
+ if (cur_el == debug_el) {
+ return extract32(env->cp15.mdscr_el1, 13, 1)
+ && !(env->daif & PSTATE_D);
+ }
+
+ /* Otherwise the debug target needs to be a higher EL */
+ return debug_el > cur_el;
+}
+
+static bool aa32_generate_debug_exceptions(CPUARMState *env)
+{
+ int el = arm_current_el(env);
+
+ if (el == 0 && arm_el_is_aa64(env, 1)) {
+ return aa64_generate_debug_exceptions(env);
+ }
+
+ if (arm_is_secure(env)) {
+ int spd;
+
+ if (el == 0 && (env->cp15.sder & 1)) {
+ /*
+ * SDER.SUIDEN means debug exceptions from Secure EL0
+ * are always enabled. Otherwise they are controlled by
+ * SDCR.SPD like those from other Secure ELs.
+ */
+ return true;
+ }
+
+ spd = extract32(env->cp15.mdcr_el3, 14, 2);
+ switch (spd) {
+ case 1:
+ /* SPD == 0b01 is reserved, but behaves as 0b00. */
+ case 0:
+ /*
+ * For 0b00 we return true if external secure invasive debug
+ * is enabled. On real hardware this is controlled by external
+ * signals to the core. QEMU always permits debug, and behaves
+ * as if DBGEN, SPIDEN, NIDEN and SPNIDEN are all tied high.
+ */
+ return true;
+ case 2:
+ return false;
+ case 3:
+ return true;
+ }
+ }
+
+ return el != 2;
+}
+
+/*
+ * Return true if debugging exceptions are currently enabled.
+ * This corresponds to what in ARM ARM pseudocode would be
+ * if UsingAArch32() then
+ * return AArch32.GenerateDebugExceptions()
+ * else
+ * return AArch64.GenerateDebugExceptions()
+ * We choose to push the if() down into this function for clarity,
+ * since the pseudocode has it at all callsites except for the one in
+ * CheckSoftwareStep(), where it is elided because both branches would
+ * always return the same value.
+ */
+bool arm_generate_debug_exceptions(CPUARMState *env)
+{
+ if (env->aarch64) {
+ return aa64_generate_debug_exceptions(env);
+ } else {
+ return aa32_generate_debug_exceptions(env);
+ }
+}
+
/*
* Is single-stepping active? (Note that the "is EL_D AArch64?" check
* implicitly means this always returns false in pre-v8 CPUs.)
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 07/25] target/arm: Use is_a64 in arm_generate_debug_exceptions
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (5 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 06/25] target/arm: Move arm_generate_debug_exceptions " Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-07 2:47 ` [PATCH v2 08/25] target/arm: Move exception_bkpt_insn to debug_helper.c Richard Henderson
` (17 subsequent siblings)
24 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Use the accessor rather than the raw structure member.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/debug_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index 20a0e4261a..a18a09a0c3 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -99,7 +99,7 @@ static bool aa32_generate_debug_exceptions(CPUARMState *env)
*/
bool arm_generate_debug_exceptions(CPUARMState *env)
{
- if (env->aarch64) {
+ if (is_a64(env)) {
return aa64_generate_debug_exceptions(env);
} else {
return aa32_generate_debug_exceptions(env);
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 08/25] target/arm: Move exception_bkpt_insn to debug_helper.c
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (6 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 07/25] target/arm: Use is_a64 in arm_generate_debug_exceptions Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:00 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 09/25] target/arm: Move arm_debug_exception_fsr " Richard Henderson
` (16 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/debug_helper.c | 31 +++++++++++++++++++++++++++++++
target/arm/op_helper.c | 29 -----------------------------
2 files changed, 31 insertions(+), 29 deletions(-)
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index a18a09a0c3..80dff0788b 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -430,6 +430,37 @@ void arm_debug_excp_handler(CPUState *cs)
}
}
+/*
+ * Raise an EXCP_BKPT with the specified syndrome register value,
+ * targeting the correct exception level for debug exceptions.
+ */
+void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome)
+{
+ int debug_el = arm_debug_target_el(env);
+ int cur_el = arm_current_el(env);
+
+ /* FSR will only be used if the debug target EL is AArch32. */
+ env->exception.fsr = arm_debug_exception_fsr(env);
+ /*
+ * FAR is UNKNOWN: clear vaddress to avoid potentially exposing
+ * values to the guest that it shouldn't be able to see at its
+ * exception/security level.
+ */
+ env->exception.vaddress = 0;
+ /*
+ * Other kinds of architectural debug exception are ignored if
+ * they target an exception level below the current one (in QEMU
+ * this is checked by arm_generate_debug_exceptions()). Breakpoint
+ * instructions are special because they always generate an exception
+ * to somewhere: if they can't go to the configured debug exception
+ * level they are taken to the current exception level.
+ */
+ if (debug_el < cur_el) {
+ debug_el = cur_el;
+ }
+ raise_exception(env, EXCP_BKPT, syndrome, debug_el);
+}
+
#if !defined(CONFIG_USER_ONLY)
vaddr arm_adjust_watchpoint_address(CPUState *cs, vaddr addr, int len)
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 00a082d1f7..ae0009d1a3 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -413,35 +413,6 @@ void HELPER(exception_advsimdfp_access)(CPUARMState *env,
raise_exception(env, EXCP_UDEF, syndrome, target_el);
}
-/* Raise an EXCP_BKPT with the specified syndrome register value,
- * targeting the correct exception level for debug exceptions.
- */
-void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome)
-{
- int debug_el = arm_debug_target_el(env);
- int cur_el = arm_current_el(env);
-
- /* FSR will only be used if the debug target EL is AArch32. */
- env->exception.fsr = arm_debug_exception_fsr(env);
- /* FAR is UNKNOWN: clear vaddress to avoid potentially exposing
- * values to the guest that it shouldn't be able to see at its
- * exception/security level.
- */
- env->exception.vaddress = 0;
- /*
- * Other kinds of architectural debug exception are ignored if
- * they target an exception level below the current one (in QEMU
- * this is checked by arm_generate_debug_exceptions()). Breakpoint
- * instructions are special because they always generate an exception
- * to somewhere: if they can't go to the configured debug exception
- * level they are taken to the current exception level.
- */
- if (debug_el < cur_el) {
- debug_el = cur_el;
- }
- raise_exception(env, EXCP_BKPT, syndrome, debug_el);
-}
-
uint32_t HELPER(cpsr_read)(CPUARMState *env)
{
return cpsr_read(env) & ~CPSR_EXEC;
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 09/25] target/arm: Move arm_debug_exception_fsr to debug_helper.c
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (7 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 08/25] target/arm: Move exception_bkpt_insn to debug_helper.c Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:01 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 10/25] target/arm: Rename helper_exception_with_syndrome Richard Henderson
` (15 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
This function now now only used in debug_helper.c, so there is
no reason to have a declaration in a header.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/internals.h | 25 -------------------------
target/arm/debug_helper.c | 26 ++++++++++++++++++++++++++
2 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index e9ac54f244..c8410ee801 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -793,31 +793,6 @@ static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
return &env->cp15.tcr_el[regime_el(env, mmu_idx)];
}
-/* Return the FSR value for a debug exception (watchpoint, hardware
- * breakpoint or BKPT insn) targeting the specified exception level.
- */
-static inline uint32_t arm_debug_exception_fsr(CPUARMState *env)
-{
- ARMMMUFaultInfo fi = { .type = ARMFault_Debug };
- int target_el = arm_debug_target_el(env);
- bool using_lpae = false;
-
- if (target_el == 2 || arm_el_is_aa64(env, target_el)) {
- using_lpae = true;
- } else {
- if (arm_feature(env, ARM_FEATURE_LPAE) &&
- (env->cp15.tcr_el[target_el].raw_tcr & TTBCR_EAE)) {
- using_lpae = true;
- }
- }
-
- if (using_lpae) {
- return arm_fi_to_lfsc(&fi);
- } else {
- return arm_fi_to_sfsc(&fi);
- }
-}
-
/**
* arm_num_brps: Return number of implemented breakpoints.
* Note that the ID register BRPS field is "number of bps - 1",
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index 80dff0788b..a743061e89 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -379,6 +379,32 @@ bool arm_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
return check_watchpoints(cpu);
}
+/*
+ * Return the FSR value for a debug exception (watchpoint, hardware
+ * breakpoint or BKPT insn) targeting the specified exception level.
+ */
+static uint32_t arm_debug_exception_fsr(CPUARMState *env)
+{
+ ARMMMUFaultInfo fi = { .type = ARMFault_Debug };
+ int target_el = arm_debug_target_el(env);
+ bool using_lpae = false;
+
+ if (target_el == 2 || arm_el_is_aa64(env, target_el)) {
+ using_lpae = true;
+ } else {
+ if (arm_feature(env, ARM_FEATURE_LPAE) &&
+ (env->cp15.tcr_el[target_el].raw_tcr & TTBCR_EAE)) {
+ using_lpae = true;
+ }
+ }
+
+ if (using_lpae) {
+ return arm_fi_to_lfsc(&fi);
+ } else {
+ return arm_fi_to_sfsc(&fi);
+ }
+}
+
void arm_debug_excp_handler(CPUState *cs)
{
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 10/25] target/arm: Rename helper_exception_with_syndrome
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (8 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 09/25] target/arm: Move arm_debug_exception_fsr " Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:01 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 11/25] target/arm: Introduce gen_exception_insn_el_v Richard Henderson
` (14 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Rename to helper_exception_with_syndrome_el, to emphasize
that the target el is a parameter.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/helper.h | 2 +-
target/arm/translate.h | 6 +++---
target/arm/op_helper.c | 6 +++---
target/arm/translate.c | 6 +++---
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index a13007699d..aca86612b4 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -45,7 +45,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, noreturn, env, i32)
-DEF_HELPER_4(exception_with_syndrome, noreturn, env, i32, i32, i32)
+DEF_HELPER_4(exception_with_syndrome_el, noreturn, env, i32, i32, i32)
DEF_HELPER_3(exception_advsimdfp_access, noreturn, env, i32, i32)
DEF_HELPER_2(exception_bkpt_insn, noreturn, env, i32)
DEF_HELPER_2(exception_pc_alignment, noreturn, env, tl)
diff --git a/target/arm/translate.h b/target/arm/translate.h
index da5f89d49d..1d6a8c7dca 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -332,9 +332,9 @@ static inline void gen_ss_advance(DisasContext *s)
static inline void gen_exception(int excp, uint32_t syndrome,
uint32_t target_el)
{
- gen_helper_exception_with_syndrome(cpu_env, tcg_constant_i32(excp),
- tcg_constant_i32(syndrome),
- tcg_constant_i32(target_el));
+ gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp),
+ tcg_constant_i32(syndrome),
+ tcg_constant_i32(target_el));
}
static inline void gen_exception_advsimdfp_access(DisasContext *s,
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index ae0009d1a3..1c4e6753f6 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -373,7 +373,7 @@ void HELPER(yield)(CPUARMState *env)
* those EXCP values which are special cases for QEMU to interrupt
* execution and not to be used for exceptions which are passed to
* the guest (those must all have syndrome information and thus should
- * use exception_with_syndrome).
+ * use exception_with_syndrome*).
*/
void HELPER(exception_internal)(CPUARMState *env, uint32_t excp)
{
@@ -385,8 +385,8 @@ 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 target_el)
+void HELPER(exception_with_syndrome_el)(CPUARMState *env, uint32_t excp,
+ uint32_t syndrome, uint32_t target_el)
{
raise_exception(env, excp, syndrome, target_el);
}
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 87a899d638..dc033600c0 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1119,9 +1119,9 @@ static void gen_exception_el(DisasContext *s, int excp, uint32_t syn,
{
gen_set_condexec(s);
gen_set_pc_im(s, s->pc_curr);
- gen_helper_exception_with_syndrome(cpu_env,
- tcg_constant_i32(excp),
- tcg_constant_i32(syn), tcg_el);
+ gen_helper_exception_with_syndrome_el(cpu_env,
+ tcg_constant_i32(excp),
+ tcg_constant_i32(syn), tcg_el);
s->base.is_jmp = DISAS_NORETURN;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 11/25] target/arm: Introduce gen_exception_insn_el_v
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (9 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 10/25] target/arm: Rename helper_exception_with_syndrome Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:05 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 12/25] target/arm: Rename gen_exception_insn to gen_exception_insn_el Richard Henderson
` (13 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Create a function below gen_exception_insn that takes
the target_el as a TCGv_i32, replacing gen_exception_el.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/translate.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index dc033600c0..9cb31663dd 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1086,8 +1086,8 @@ static void gen_exception_internal_insn(DisasContext *s, uint32_t pc, int excp)
s->base.is_jmp = DISAS_NORETURN;
}
-void gen_exception_insn(DisasContext *s, uint64_t pc, int excp,
- uint32_t syn, uint32_t target_el)
+static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp,
+ uint32_t syn, TCGv_i32 tcg_el)
{
if (s->aarch64) {
gen_a64_set_pc_im(pc);
@@ -1095,10 +1095,17 @@ void gen_exception_insn(DisasContext *s, uint64_t pc, int excp,
gen_set_condexec(s);
gen_set_pc_im(s, pc);
}
- gen_exception(excp, syn, target_el);
+ gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp),
+ tcg_constant_i32(syn), tcg_el);
s->base.is_jmp = DISAS_NORETURN;
}
+void gen_exception_insn(DisasContext *s, uint64_t pc, int excp,
+ uint32_t syn, uint32_t target_el)
+{
+ gen_exception_insn_el_v(s, pc, excp, syn, tcg_constant_i32(target_el));
+}
+
static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn)
{
gen_set_condexec(s);
@@ -1114,17 +1121,6 @@ void unallocated_encoding(DisasContext *s)
default_exception_el(s));
}
-static void gen_exception_el(DisasContext *s, int excp, uint32_t syn,
- TCGv_i32 tcg_el)
-{
- gen_set_condexec(s);
- gen_set_pc_im(s, s->pc_curr);
- gen_helper_exception_with_syndrome_el(cpu_env,
- tcg_constant_i32(excp),
- tcg_constant_i32(syn), tcg_el);
- s->base.is_jmp = DISAS_NORETURN;
-}
-
/* Force a TB lookup after an instruction that changes the CPU state. */
void gen_lookup_tb(DisasContext *s)
{
@@ -2847,7 +2843,8 @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn,
tcg_el = tcg_constant_i32(3);
}
- gen_exception_el(s, EXCP_UDEF, syn_uncategorized(), tcg_el);
+ gen_exception_insn_el_v(s, s->pc_curr, EXCP_UDEF,
+ syn_uncategorized(), tcg_el);
tcg_temp_free_i32(tcg_el);
return false;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 12/25] target/arm: Rename gen_exception_insn to gen_exception_insn_el
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (10 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 11/25] target/arm: Introduce gen_exception_insn_el_v Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:05 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 13/25] target/arm: Introduce gen_exception_insn Richard Henderson
` (12 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/translate.h | 4 ++--
target/arm/translate-a64.c | 30 +++++++++++++--------------
target/arm/translate-m-nocp.c | 16 +++++++-------
target/arm/translate-mve.c | 4 ++--
target/arm/translate-vfp.c | 4 ++--
target/arm/translate.c | 39 ++++++++++++++++++-----------------
6 files changed, 49 insertions(+), 48 deletions(-)
diff --git a/target/arm/translate.h b/target/arm/translate.h
index 1d6a8c7dca..c54db51d70 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -279,8 +279,8 @@ void arm_jump_cc(DisasCompare *cmp, TCGLabel *label);
void arm_gen_test_cc(int cc, TCGLabel *label);
MemOp pow2_align(unsigned i);
void unallocated_encoding(DisasContext *s);
-void gen_exception_insn(DisasContext *s, uint64_t pc, int excp,
- uint32_t syn, uint32_t target_el);
+void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp,
+ uint32_t syn, uint32_t target_el);
/* Return state of Alternate Half-precision flag, caller frees result */
static inline TCGv_i32 get_ahp_flag(void)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 3251c69180..cc96535fae 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1178,8 +1178,8 @@ bool sve_access_check(DisasContext *s)
assert(!s->sve_access_checked);
s->sve_access_checked = true;
- gen_exception_insn(s, s->pc_curr, EXCP_UDEF,
- syn_sve_access_trap(), s->sve_excp_el);
+ gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF,
+ syn_sve_access_trap(), s->sve_excp_el);
return false;
}
s->sve_access_checked = true;
@@ -1815,8 +1815,8 @@ static void gen_sysreg_undef(DisasContext *s, bool isread,
} else {
syndrome = syn_uncategorized();
}
- gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syndrome,
- default_exception_el(s));
+ gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, syndrome,
+ default_exception_el(s));
}
/* MRS - move from system register
@@ -2068,8 +2068,8 @@ static void disas_exc(DisasContext *s, uint32_t insn)
switch (op2_ll) {
case 1: /* SVC */
gen_ss_advance(s);
- gen_exception_insn(s, s->base.pc_next, EXCP_SWI,
- syn_aa64_svc(imm16), default_exception_el(s));
+ gen_exception_insn_el(s, s->base.pc_next, EXCP_SWI,
+ syn_aa64_svc(imm16), default_exception_el(s));
break;
case 2: /* HVC */
if (s->current_el == 0) {
@@ -2082,8 +2082,8 @@ static void disas_exc(DisasContext *s, uint32_t insn)
gen_a64_set_pc_im(s->pc_curr);
gen_helper_pre_hvc(cpu_env);
gen_ss_advance(s);
- gen_exception_insn(s, s->base.pc_next, EXCP_HVC,
- syn_aa64_hvc(imm16), 2);
+ gen_exception_insn_el(s, s->base.pc_next, EXCP_HVC,
+ syn_aa64_hvc(imm16), 2);
break;
case 3: /* SMC */
if (s->current_el == 0) {
@@ -2093,8 +2093,8 @@ static void disas_exc(DisasContext *s, uint32_t insn)
gen_a64_set_pc_im(s->pc_curr);
gen_helper_pre_smc(cpu_env, tcg_constant_i32(syn_aa64_smc(imm16)));
gen_ss_advance(s);
- gen_exception_insn(s, s->base.pc_next, EXCP_SMC,
- syn_aa64_smc(imm16), 3);
+ gen_exception_insn_el(s, s->base.pc_next, EXCP_SMC,
+ syn_aa64_smc(imm16), 3);
break;
default:
unallocated_encoding(s);
@@ -14724,8 +14724,8 @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
* Illegal execution state. This has priority over BTI
* exceptions, but comes after instruction abort exceptions.
*/
- gen_exception_insn(s, s->pc_curr, EXCP_UDEF,
- syn_illegalstate(), default_exception_el(s));
+ gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF,
+ syn_illegalstate(), default_exception_el(s));
return;
}
@@ -14756,9 +14756,9 @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
if (s->btype != 0
&& s->guarded_page
&& !btype_destination_ok(insn, s->bt, s->btype)) {
- gen_exception_insn(s, s->pc_curr, EXCP_UDEF,
- syn_btitrap(s->btype),
- default_exception_el(s));
+ gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF,
+ syn_btitrap(s->btype),
+ default_exception_el(s));
return;
}
} else {
diff --git a/target/arm/translate-m-nocp.c b/target/arm/translate-m-nocp.c
index 27363a7b4e..636bfb1788 100644
--- a/target/arm/translate-m-nocp.c
+++ b/target/arm/translate-m-nocp.c
@@ -143,8 +143,8 @@ static bool trans_VSCCLRM(DisasContext *s, arg_VSCCLRM *a)
tcg_gen_brcondi_i32(TCG_COND_EQ, sfpa, 0, s->condlabel);
if (s->fp_excp_el != 0) {
- gen_exception_insn(s, s->pc_curr, EXCP_NOCP,
- syn_uncategorized(), s->fp_excp_el);
+ gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP,
+ syn_uncategorized(), s->fp_excp_el);
return true;
}
@@ -376,7 +376,7 @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int regno,
if (!vfp_access_check_m(s, true)) {
/*
* This was only a conditional exception, so override
- * gen_exception_insn()'s default to DISAS_NORETURN
+ * gen_exception_insn_el()'s default to DISAS_NORETURN
*/
s->base.is_jmp = DISAS_NEXT;
break;
@@ -532,7 +532,7 @@ static bool gen_M_fp_sysreg_read(DisasContext *s, int regno,
if (!vfp_access_check_m(s, true)) {
/*
* This was only a conditional exception, so override
- * gen_exception_insn()'s default to DISAS_NORETURN
+ * gen_exception_insn_el()'s default to DISAS_NORETURN
*/
s->base.is_jmp = DISAS_NEXT;
break;
@@ -765,14 +765,14 @@ static bool trans_NOCP(DisasContext *s, arg_nocp *a)
}
if (a->cp != 10) {
- gen_exception_insn(s, s->pc_curr, EXCP_NOCP,
- syn_uncategorized(), default_exception_el(s));
+ gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP,
+ syn_uncategorized(), default_exception_el(s));
return true;
}
if (s->fp_excp_el != 0) {
- gen_exception_insn(s, s->pc_curr, EXCP_NOCP,
- syn_uncategorized(), s->fp_excp_el);
+ gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP,
+ syn_uncategorized(), s->fp_excp_el);
return true;
}
diff --git a/target/arm/translate-mve.c b/target/arm/translate-mve.c
index 4267d43cc7..5aec2a1555 100644
--- a/target/arm/translate-mve.c
+++ b/target/arm/translate-mve.c
@@ -100,8 +100,8 @@ bool mve_eci_check(DisasContext *s)
return true;
default:
/* Reserved value: INVSTATE UsageFault */
- gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(),
- default_exception_el(s));
+ gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(),
+ default_exception_el(s));
return false;
}
}
diff --git a/target/arm/translate-vfp.c b/target/arm/translate-vfp.c
index 2a98387411..f939d64101 100644
--- a/target/arm/translate-vfp.c
+++ b/target/arm/translate-vfp.c
@@ -262,8 +262,8 @@ bool vfp_access_check_m(DisasContext *s, bool skip_context_update)
* the encoding space handled by the patterns in m-nocp.decode,
* and for them we may need to raise NOCP here.
*/
- gen_exception_insn(s, s->pc_curr, EXCP_NOCP,
- syn_uncategorized(), s->fp_excp_el);
+ gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP,
+ syn_uncategorized(), s->fp_excp_el);
return false;
}
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 9cb31663dd..44f462a3a3 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1100,8 +1100,8 @@ static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp,
s->base.is_jmp = DISAS_NORETURN;
}
-void gen_exception_insn(DisasContext *s, uint64_t pc, int excp,
- uint32_t syn, uint32_t target_el)
+void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp,
+ uint32_t syn, uint32_t target_el)
{
gen_exception_insn_el_v(s, pc, excp, syn, tcg_constant_i32(target_el));
}
@@ -1117,8 +1117,8 @@ static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn)
void unallocated_encoding(DisasContext *s)
{
/* Unallocated and reserved encodings are uncategorized */
- gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),
- default_exception_el(s));
+ gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),
+ default_exception_el(s));
}
/* Force a TB lookup after an instruction that changes the CPU state. */
@@ -2869,8 +2869,8 @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn,
undef:
/* If we get here then some access check did not pass */
- gen_exception_insn(s, s->pc_curr, EXCP_UDEF,
- syn_uncategorized(), exc_target);
+ gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF,
+ syn_uncategorized(), exc_target);
return false;
}
@@ -5094,7 +5094,8 @@ static void gen_srs(DisasContext *s,
* For the UNPREDICTABLE cases we choose to UNDEF.
*/
if (s->current_el == 1 && !s->ns && mode == ARM_CPU_MODE_MON) {
- gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), 3);
+ gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF,
+ syn_uncategorized(), 3);
return;
}
@@ -8479,8 +8480,8 @@ static bool trans_WLS(DisasContext *s, arg_WLS *a)
* Do the check-and-raise-exception by hand.
*/
if (s->fp_excp_el) {
- gen_exception_insn(s, s->pc_curr, EXCP_NOCP,
- syn_uncategorized(), s->fp_excp_el);
+ gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP,
+ syn_uncategorized(), s->fp_excp_el);
return true;
}
}
@@ -8582,8 +8583,8 @@ static bool trans_LE(DisasContext *s, arg_LE *a)
tmp = load_cpu_field(v7m.ltpsize);
tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 4, skipexc);
tcg_temp_free_i32(tmp);
- gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(),
- default_exception_el(s));
+ gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(),
+ default_exception_el(s));
gen_set_label(skipexc);
}
@@ -9053,8 +9054,8 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
* UsageFault exception.
*/
if (arm_dc_feature(s, ARM_FEATURE_M)) {
- gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(),
- default_exception_el(s));
+ gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(),
+ default_exception_el(s));
return;
}
@@ -9063,8 +9064,8 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
* Illegal execution state. This has priority over BTI
* exceptions, but comes after instruction abort exceptions.
*/
- gen_exception_insn(s, s->pc_curr, EXCP_UDEF,
- syn_illegalstate(), default_exception_el(s));
+ gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF,
+ syn_illegalstate(), default_exception_el(s));
return;
}
@@ -9633,8 +9634,8 @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
* Illegal execution state. This has priority over BTI
* exceptions, but comes after instruction abort exceptions.
*/
- gen_exception_insn(dc, dc->pc_curr, EXCP_UDEF,
- syn_illegalstate(), default_exception_el(dc));
+ gen_exception_insn_el(dc, dc->pc_curr, EXCP_UDEF,
+ syn_illegalstate(), default_exception_el(dc));
return;
}
@@ -9707,8 +9708,8 @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
*/
tcg_remove_ops_after(dc->insn_eci_rewind);
dc->condjmp = 0;
- gen_exception_insn(dc, dc->pc_curr, EXCP_INVSTATE, syn_uncategorized(),
- default_exception_el(dc));
+ gen_exception_insn_el(dc, dc->pc_curr, EXCP_INVSTATE, syn_uncategorized(),
+ default_exception_el(dc));
}
arm_post_translate_insn(dc);
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 13/25] target/arm: Introduce gen_exception_insn
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (11 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 12/25] target/arm: Rename gen_exception_insn to gen_exception_insn_el Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:06 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 14/25] target/arm: Create helper_exception_swstep Richard Henderson
` (11 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Create a new wrapper function that passes the default
exception target to gen_exception_insn_el.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/translate.h | 1 +
target/arm/translate-a64.c | 15 ++++++---------
target/arm/translate-m-nocp.c | 3 +--
target/arm/translate-mve.c | 3 +--
target/arm/translate.c | 29 +++++++++++++----------------
5 files changed, 22 insertions(+), 29 deletions(-)
diff --git a/target/arm/translate.h b/target/arm/translate.h
index c54db51d70..04d45da54e 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -281,6 +281,7 @@ MemOp pow2_align(unsigned i);
void unallocated_encoding(DisasContext *s);
void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp,
uint32_t syn, uint32_t target_el);
+void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, uint32_t syn);
/* Return state of Alternate Half-precision flag, caller frees result */
static inline TCGv_i32 get_ahp_flag(void)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index cc96535fae..a5c66ce085 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1815,8 +1815,7 @@ static void gen_sysreg_undef(DisasContext *s, bool isread,
} else {
syndrome = syn_uncategorized();
}
- gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, syndrome,
- default_exception_el(s));
+ gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syndrome);
}
/* MRS - move from system register
@@ -2068,8 +2067,8 @@ static void disas_exc(DisasContext *s, uint32_t insn)
switch (op2_ll) {
case 1: /* SVC */
gen_ss_advance(s);
- gen_exception_insn_el(s, s->base.pc_next, EXCP_SWI,
- syn_aa64_svc(imm16), default_exception_el(s));
+ gen_exception_insn(s, s->base.pc_next, EXCP_SWI,
+ syn_aa64_svc(imm16));
break;
case 2: /* HVC */
if (s->current_el == 0) {
@@ -14724,8 +14723,7 @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
* Illegal execution state. This has priority over BTI
* exceptions, but comes after instruction abort exceptions.
*/
- gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF,
- syn_illegalstate(), default_exception_el(s));
+ gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_illegalstate());
return;
}
@@ -14756,9 +14754,8 @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
if (s->btype != 0
&& s->guarded_page
&& !btype_destination_ok(insn, s->bt, s->btype)) {
- gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF,
- syn_btitrap(s->btype),
- default_exception_el(s));
+ gen_exception_insn(s, s->pc_curr, EXCP_UDEF,
+ syn_btitrap(s->btype));
return;
}
} else {
diff --git a/target/arm/translate-m-nocp.c b/target/arm/translate-m-nocp.c
index 636bfb1788..4029d7fdd4 100644
--- a/target/arm/translate-m-nocp.c
+++ b/target/arm/translate-m-nocp.c
@@ -765,8 +765,7 @@ static bool trans_NOCP(DisasContext *s, arg_nocp *a)
}
if (a->cp != 10) {
- gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP,
- syn_uncategorized(), default_exception_el(s));
+ gen_exception_insn(s, s->pc_curr, EXCP_NOCP, syn_uncategorized());
return true;
}
diff --git a/target/arm/translate-mve.c b/target/arm/translate-mve.c
index 5aec2a1555..0cf1b5ea4f 100644
--- a/target/arm/translate-mve.c
+++ b/target/arm/translate-mve.c
@@ -100,8 +100,7 @@ bool mve_eci_check(DisasContext *s)
return true;
default:
/* Reserved value: INVSTATE UsageFault */
- gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(),
- default_exception_el(s));
+ gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized());
return false;
}
}
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 44f462a3a3..c7d422b541 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1106,6 +1106,11 @@ void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp,
gen_exception_insn_el_v(s, pc, excp, syn, tcg_constant_i32(target_el));
}
+void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, uint32_t syn)
+{
+ gen_exception_insn_el(s, pc, excp, syn, default_exception_el(s));
+}
+
static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn)
{
gen_set_condexec(s);
@@ -1117,8 +1122,7 @@ static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn)
void unallocated_encoding(DisasContext *s)
{
/* Unallocated and reserved encodings are uncategorized */
- gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),
- default_exception_el(s));
+ gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized());
}
/* Force a TB lookup after an instruction that changes the CPU state. */
@@ -2731,8 +2735,6 @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn,
* an exception and return false. Otherwise it will return true,
* and set *tgtmode and *regno appropriately.
*/
- int exc_target = default_exception_el(s);
-
/* These instructions are present only in ARMv8, or in ARMv7 with the
* Virtualization Extensions.
*/
@@ -2869,8 +2871,7 @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn,
undef:
/* If we get here then some access check did not pass */
- gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF,
- syn_uncategorized(), exc_target);
+ gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized());
return false;
}
@@ -8583,8 +8584,7 @@ static bool trans_LE(DisasContext *s, arg_LE *a)
tmp = load_cpu_field(v7m.ltpsize);
tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 4, skipexc);
tcg_temp_free_i32(tmp);
- gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(),
- default_exception_el(s));
+ gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized());
gen_set_label(skipexc);
}
@@ -9054,8 +9054,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
* UsageFault exception.
*/
if (arm_dc_feature(s, ARM_FEATURE_M)) {
- gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(),
- default_exception_el(s));
+ gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized());
return;
}
@@ -9064,8 +9063,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
* Illegal execution state. This has priority over BTI
* exceptions, but comes after instruction abort exceptions.
*/
- gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF,
- syn_illegalstate(), default_exception_el(s));
+ gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_illegalstate());
return;
}
@@ -9634,8 +9632,7 @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
* Illegal execution state. This has priority over BTI
* exceptions, but comes after instruction abort exceptions.
*/
- gen_exception_insn_el(dc, dc->pc_curr, EXCP_UDEF,
- syn_illegalstate(), default_exception_el(dc));
+ gen_exception_insn(dc, dc->pc_curr, EXCP_UDEF, syn_illegalstate());
return;
}
@@ -9708,8 +9705,8 @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
*/
tcg_remove_ops_after(dc->insn_eci_rewind);
dc->condjmp = 0;
- gen_exception_insn_el(dc, dc->pc_curr, EXCP_INVSTATE, syn_uncategorized(),
- default_exception_el(dc));
+ gen_exception_insn(dc, dc->pc_curr, EXCP_INVSTATE,
+ syn_uncategorized());
}
arm_post_translate_insn(dc);
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 14/25] target/arm: Create helper_exception_swstep
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (12 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 13/25] target/arm: Introduce gen_exception_insn Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:35 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 15/25] target/arm: Remove TBFLAG_ANY.DEBUG_TARGET_EL Richard Henderson
` (10 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Move the computation from gen_swstep_exception into a helper.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/helper.h | 1 +
target/arm/translate.h | 12 +++---------
target/arm/debug_helper.c | 16 ++++++++++++++++
3 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index aca86612b4..afc0f1a462 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -48,6 +48,7 @@ DEF_HELPER_2(exception_internal, noreturn, env, i32)
DEF_HELPER_4(exception_with_syndrome_el, noreturn, env, i32, i32, i32)
DEF_HELPER_3(exception_advsimdfp_access, noreturn, env, i32, i32)
DEF_HELPER_2(exception_bkpt_insn, noreturn, env, i32)
+DEF_HELPER_2(exception_swstep, noreturn, env, i32)
DEF_HELPER_2(exception_pc_alignment, noreturn, env, tl)
DEF_HELPER_1(setend, void, env)
DEF_HELPER_2(wfi, void, env, i32)
diff --git a/target/arm/translate.h b/target/arm/translate.h
index 04d45da54e..c720a7e26c 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -350,15 +350,9 @@ static inline void gen_exception_advsimdfp_access(DisasContext *s,
/* Generate an architectural singlestep exception */
static inline void gen_swstep_exception(DisasContext *s, int isv, int ex)
{
- 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(EXCP_UDEF, syn_swstep(same_el, isv, ex), s->debug_target_el);
+ /* Fill in the same_el field of the syndrome in the helper. */
+ uint32_t syn = syn_swstep(false, isv, ex);
+ gen_helper_exception_swstep(cpu_env, tcg_constant_i32(syn));
}
/*
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index a743061e89..a3a1b98de2 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -487,6 +487,22 @@ void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome)
raise_exception(env, EXCP_BKPT, syndrome, debug_el);
}
+void HELPER(exception_swstep)(CPUARMState *env, uint32_t syndrome)
+{
+ int debug_el = arm_debug_target_el(env);
+ int cur_el = arm_current_el(env);
+
+ /*
+ * If singlestep is targeting a lower EL than the current one, then
+ * DisasContext.ss_active must be false and we can never get here.
+ */
+ assert(debug_el >= cur_el);
+ if (debug_el == cur_el) {
+ syndrome |= 1 << ARM_EL_EC_SHIFT;
+ }
+ raise_exception(env, EXCP_UDEF, syndrome, debug_el);
+}
+
#if !defined(CONFIG_USER_ONLY)
vaddr arm_adjust_watchpoint_address(CPUState *cs, vaddr addr, int len)
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH v2 14/25] target/arm: Create helper_exception_swstep
2022-06-07 2:47 ` [PATCH v2 14/25] target/arm: Create helper_exception_swstep Richard Henderson
@ 2022-06-09 16:35 ` Peter Maydell
0 siblings, 0 replies; 49+ messages in thread
From: Peter Maydell @ 2022-06-09 16:35 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Tue, 7 Jun 2022 at 03:58, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Move the computation from gen_swstep_exception into a helper.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/helper.h | 1 +
> target/arm/translate.h | 12 +++---------
> target/arm/debug_helper.c | 16 ++++++++++++++++
> 3 files changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/target/arm/helper.h b/target/arm/helper.h
> index aca86612b4..afc0f1a462 100644
> --- a/target/arm/helper.h
> +++ b/target/arm/helper.h
> @@ -48,6 +48,7 @@ DEF_HELPER_2(exception_internal, noreturn, env, i32)
> DEF_HELPER_4(exception_with_syndrome_el, noreturn, env, i32, i32, i32)
> DEF_HELPER_3(exception_advsimdfp_access, noreturn, env, i32, i32)
> DEF_HELPER_2(exception_bkpt_insn, noreturn, env, i32)
> +DEF_HELPER_2(exception_swstep, noreturn, env, i32)
> DEF_HELPER_2(exception_pc_alignment, noreturn, env, tl)
> DEF_HELPER_1(setend, void, env)
> DEF_HELPER_2(wfi, void, env, i32)
> diff --git a/target/arm/translate.h b/target/arm/translate.h
> index 04d45da54e..c720a7e26c 100644
> --- a/target/arm/translate.h
> +++ b/target/arm/translate.h
> @@ -350,15 +350,9 @@ static inline void gen_exception_advsimdfp_access(DisasContext *s,
> /* Generate an architectural singlestep exception */
> static inline void gen_swstep_exception(DisasContext *s, int isv, int ex)
> {
> - 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(EXCP_UDEF, syn_swstep(same_el, isv, ex), s->debug_target_el);
> + /* Fill in the same_el field of the syndrome in the helper. */
> + uint32_t syn = syn_swstep(false, isv, ex);
> + gen_helper_exception_swstep(cpu_env, tcg_constant_i32(syn));
> }
>
> /*
> diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
> index a743061e89..a3a1b98de2 100644
> --- a/target/arm/debug_helper.c
> +++ b/target/arm/debug_helper.c
> @@ -487,6 +487,22 @@ void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome)
> raise_exception(env, EXCP_BKPT, syndrome, debug_el);
> }
>
> +void HELPER(exception_swstep)(CPUARMState *env, uint32_t syndrome)
> +{
> + int debug_el = arm_debug_target_el(env);
> + int cur_el = arm_current_el(env);
> +
> + /*
> + * If singlestep is targeting a lower EL than the current one, then
> + * DisasContext.ss_active must be false and we can never get here.
> + */
> + assert(debug_el >= cur_el);
This is a little trickier than it first looks, because in the old
setup the assert in gen_swstep_exception() is checking the translate
time state (which corresponds to the EL we executed the insn in),
whereas this assert is checked at runtime, so it happens after all
the effects of the insn have taken place, which might include a
change of exception level, if the insn is "eret". Similarly we'll
now calculate the "same_el" bit based on the EL after execution of
the eret, rather than the one before.
I think however that:
* the assertion is still fine, because we can only go down in EL
(going up in EL means taking an exception, in which case we won't
be here)
* setting the same-el bit based on the cur_el after the eret
changes it is actually fixing a bug in a corner case:
- EL_D is using MDSCR_EL1.KDE == 1 to enable debug exceptions
within EL_D itself
- we singlestep an eret from EL_D to some lower EL
Here the 'same EL' bit should be based on the EL we end up with
after the 'eret' (architecturally we only take the swstep
exception when we are in the Active-Pending state, which is
after we have completed execution of the instruction proper),
so it ought to be 0. But in the old code we calculate it using
the DisasContext::current_el, so it would incorrectly be 1.
(Writing a test case to demonstrate this theory is left as an
exercise for the reader :-))
So as far as the code changes are concerned,
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
but since this is pretty subtle we should probably discuss it in
the commit message, and definitely we should note that this is
fixing a bug.
thanks
-- PMM
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 15/25] target/arm: Remove TBFLAG_ANY.DEBUG_TARGET_EL
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (13 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 14/25] target/arm: Create helper_exception_swstep Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:36 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 16/25] target/arm: Move gen_exception to translate.c Richard Henderson
` (9 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
We no longer need this value during translation,
as it is now handled within the helpers.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu.h | 6 ++----
target/arm/translate.h | 2 --
target/arm/helper.c | 12 ++----------
target/arm/translate-a64.c | 1 -
target/arm/translate.c | 1 -
5 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 92c9758e86..5224dc2c85 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3085,11 +3085,9 @@ FIELD(TBFLAG_ANY, BE_DATA, 3, 1)
FIELD(TBFLAG_ANY, MMUIDX, 4, 4)
/* Target EL if we take a floating-point-disabled exception */
FIELD(TBFLAG_ANY, FPEXC_EL, 8, 2)
-/* For A-profile only, target EL for debug exceptions. */
-FIELD(TBFLAG_ANY, DEBUG_TARGET_EL, 10, 2)
/* Memory operations require alignment: SCTLR_ELx.A or CCR.UNALIGN_TRP */
-FIELD(TBFLAG_ANY, ALIGN_MEM, 12, 1)
-FIELD(TBFLAG_ANY, PSTATE__IL, 13, 1)
+FIELD(TBFLAG_ANY, ALIGN_MEM, 10, 1)
+FIELD(TBFLAG_ANY, PSTATE__IL, 11, 1)
/*
* Bit usage when in AArch32 state, both A- and M-profile.
diff --git a/target/arm/translate.h b/target/arm/translate.h
index c720a7e26c..4a93cac65f 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -59,8 +59,6 @@ typedef struct DisasContext {
*/
uint32_t svc_imm;
int current_el;
- /* Debug target exception level for single-step exceptions */
- int debug_target_el;
GHashTable *cp_regs;
uint64_t features; /* CPU features bits */
bool aarch64;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 40da63913c..eeca1a5dc8 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -13630,18 +13630,10 @@ static CPUARMTBFlags rebuild_hflags_m32(CPUARMState *env, int fp_el,
return rebuild_hflags_common_32(env, fp_el, mmu_idx, flags);
}
-static CPUARMTBFlags rebuild_hflags_aprofile(CPUARMState *env)
-{
- CPUARMTBFlags flags = {};
-
- DP_TBFLAG_ANY(flags, DEBUG_TARGET_EL, arm_debug_target_el(env));
- return flags;
-}
-
static CPUARMTBFlags rebuild_hflags_a32(CPUARMState *env, int fp_el,
ARMMMUIdx mmu_idx)
{
- CPUARMTBFlags flags = rebuild_hflags_aprofile(env);
+ CPUARMTBFlags flags = {};
int el = arm_current_el(env);
if (arm_sctlr(env, el) & SCTLR_A) {
@@ -13667,7 +13659,7 @@ static CPUARMTBFlags rebuild_hflags_a32(CPUARMState *env, int fp_el,
static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, int el, int fp_el,
ARMMMUIdx mmu_idx)
{
- CPUARMTBFlags flags = rebuild_hflags_aprofile(env);
+ CPUARMTBFlags flags = {};
ARMMMUIdx stage1 = stage_1_mmu_idx(mmu_idx);
uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr;
uint64_t sctlr;
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index a5c66ce085..b45039a124 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -14644,7 +14644,6 @@ static void aarch64_tr_init_disas_context(DisasContextBase *dcbase,
dc->ss_active = EX_TBFLAG_ANY(tb_flags, SS_ACTIVE);
dc->pstate_ss = EX_TBFLAG_ANY(tb_flags, PSTATE__SS);
dc->is_ldex = false;
- dc->debug_target_el = EX_TBFLAG_ANY(tb_flags, 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/target/arm/translate.c b/target/arm/translate.c
index c7d422b541..b8a8972bac 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -9350,7 +9350,6 @@ static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
dc->v7m_lspact = EX_TBFLAG_M32(tb_flags, LSPACT);
dc->mve_no_pred = EX_TBFLAG_M32(tb_flags, MVE_NO_PRED);
} else {
- dc->debug_target_el = EX_TBFLAG_ANY(tb_flags, DEBUG_TARGET_EL);
dc->sctlr_b = EX_TBFLAG_A32(tb_flags, SCTLR__B);
dc->hstr_active = EX_TBFLAG_A32(tb_flags, HSTR_ACTIVE);
dc->ns = EX_TBFLAG_A32(tb_flags, NS);
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 16/25] target/arm: Move gen_exception to translate.c
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (14 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 15/25] target/arm: Remove TBFLAG_ANY.DEBUG_TARGET_EL Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:37 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 17/25] target/arm: Rename gen_exception to gen_exception_el Richard Henderson
` (8 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
This function is not required by any other translation file.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/translate.h | 8 --------
target/arm/translate.c | 7 +++++++
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/target/arm/translate.h b/target/arm/translate.h
index 4a93cac65f..d7a9acf5a9 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -328,14 +328,6 @@ static inline void gen_ss_advance(DisasContext *s)
}
}
-static inline void gen_exception(int excp, uint32_t syndrome,
- uint32_t target_el)
-{
- gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp),
- tcg_constant_i32(syndrome),
- tcg_constant_i32(target_el));
-}
-
static inline void gen_exception_advsimdfp_access(DisasContext *s,
uint32_t syndrome)
{
diff --git a/target/arm/translate.c b/target/arm/translate.c
index b8a8972bac..fc5eafaeeb 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1086,6 +1086,13 @@ static void gen_exception_internal_insn(DisasContext *s, uint32_t pc, int excp)
s->base.is_jmp = DISAS_NORETURN;
}
+static void gen_exception(int excp, uint32_t syndrome, uint32_t target_el)
+{
+ gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp),
+ tcg_constant_i32(syndrome),
+ tcg_constant_i32(target_el));
+}
+
static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp,
uint32_t syn, TCGv_i32 tcg_el)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 17/25] target/arm: Rename gen_exception to gen_exception_el
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (15 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 16/25] target/arm: Move gen_exception to translate.c Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:37 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 18/25] target/arm: Introduce gen_exception Richard Henderson
` (7 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/translate.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index fc5eafaeeb..edb7d3f394 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1086,7 +1086,7 @@ static void gen_exception_internal_insn(DisasContext *s, uint32_t pc, int excp)
s->base.is_jmp = DISAS_NORETURN;
}
-static void gen_exception(int excp, uint32_t syndrome, uint32_t target_el)
+static void gen_exception_el(int excp, uint32_t syndrome, uint32_t target_el)
{
gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp),
tcg_constant_i32(syndrome),
@@ -9758,16 +9758,16 @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
switch (dc->base.is_jmp) {
case DISAS_SWI:
gen_ss_advance(dc);
- gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb),
- default_exception_el(dc));
+ gen_exception_el(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb),
+ default_exception_el(dc));
break;
case DISAS_HVC:
gen_ss_advance(dc);
- gen_exception(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2);
+ gen_exception_el(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2);
break;
case DISAS_SMC:
gen_ss_advance(dc);
- gen_exception(EXCP_SMC, syn_aa32_smc(), 3);
+ gen_exception_el(EXCP_SMC, syn_aa32_smc(), 3);
break;
case DISAS_NEXT:
case DISAS_TOO_MANY:
@@ -9828,14 +9828,14 @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
gen_helper_yield(cpu_env);
break;
case DISAS_SWI:
- gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb),
- default_exception_el(dc));
+ gen_exception_el(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), 2);
+ gen_exception_el(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2);
break;
case DISAS_SMC:
- gen_exception(EXCP_SMC, syn_aa32_smc(), 3);
+ gen_exception_el(EXCP_SMC, syn_aa32_smc(), 3);
break;
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 18/25] target/arm: Introduce gen_exception
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (16 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 17/25] target/arm: Rename gen_exception to gen_exception_el Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:38 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 19/25] target/arm: Introduce gen_exception_el_v Richard Henderson
` (6 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Create a new wrapper function that passes the default
exception target to gen_exception_el.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/translate.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index edb7d3f394..5a48937ede 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1093,6 +1093,11 @@ static void gen_exception_el(int excp, uint32_t syndrome, uint32_t target_el)
tcg_constant_i32(target_el));
}
+static void gen_exception(DisasContext *s, int excp, uint32_t syndrome)
+{
+ gen_exception_el(excp, syndrome, default_exception_el(s));
+}
+
static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp,
uint32_t syn, TCGv_i32 tcg_el)
{
@@ -9758,8 +9763,7 @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
switch (dc->base.is_jmp) {
case DISAS_SWI:
gen_ss_advance(dc);
- gen_exception_el(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb),
- default_exception_el(dc));
+ gen_exception(dc, EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb));
break;
case DISAS_HVC:
gen_ss_advance(dc);
@@ -9828,8 +9832,7 @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
gen_helper_yield(cpu_env);
break;
case DISAS_SWI:
- gen_exception_el(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb),
- default_exception_el(dc));
+ gen_exception(dc, EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb));
break;
case DISAS_HVC:
gen_exception_el(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2);
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 19/25] target/arm: Introduce gen_exception_el_v
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (17 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 18/25] target/arm: Introduce gen_exception Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:38 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 20/25] target/arm: Introduce helper_exception_with_syndrome Richard Henderson
` (5 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Split out a common helper function for gen_exception_el
and gen_exception_insn_el_v.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/translate.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 5a48937ede..fcb6ee648b 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1086,11 +1086,15 @@ static void gen_exception_internal_insn(DisasContext *s, uint32_t pc, int excp)
s->base.is_jmp = DISAS_NORETURN;
}
-static void gen_exception_el(int excp, uint32_t syndrome, uint32_t target_el)
+static void gen_exception_el_v(int excp, uint32_t syndrome, TCGv_i32 tcg_el)
{
gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp),
- tcg_constant_i32(syndrome),
- tcg_constant_i32(target_el));
+ tcg_constant_i32(syndrome), tcg_el);
+}
+
+static void gen_exception_el(int excp, uint32_t syndrome, uint32_t target_el)
+{
+ gen_exception_el_v(excp, syndrome, tcg_constant_i32(target_el));
}
static void gen_exception(DisasContext *s, int excp, uint32_t syndrome)
@@ -1107,8 +1111,7 @@ static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp,
gen_set_condexec(s);
gen_set_pc_im(s, pc);
}
- gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp),
- tcg_constant_i32(syn), tcg_el);
+ gen_exception_el_v(excp, syn, tcg_el);
s->base.is_jmp = DISAS_NORETURN;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 20/25] target/arm: Introduce helper_exception_with_syndrome
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (18 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 19/25] target/arm: Introduce gen_exception_el_v Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:42 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 21/25] target/arm: Remove default_exception_el Richard Henderson
` (4 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
With the helper we can use exception_target_el at runtime,
instead of default_exception_el at translate time.
While we're at it, remove the DisasContext parameter from
gen_exception, as it is no longer used.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/helper.h | 1 +
target/arm/op_helper.c | 10 ++++++++++
target/arm/translate.c | 18 +++++++++++++-----
3 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index afc0f1a462..7c8133f83a 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -45,6 +45,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, noreturn, env, i32)
+DEF_HELPER_3(exception_with_syndrome, noreturn, env, i32, i32)
DEF_HELPER_4(exception_with_syndrome_el, noreturn, env, i32, i32, i32)
DEF_HELPER_3(exception_advsimdfp_access, noreturn, env, i32, i32)
DEF_HELPER_2(exception_bkpt_insn, noreturn, env, i32)
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 1c4e6753f6..32e2a92ff4 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -391,6 +391,16 @@ void HELPER(exception_with_syndrome_el)(CPUARMState *env, uint32_t excp,
raise_exception(env, excp, syndrome, target_el);
}
+/*
+ * Raise an exception with the specified syndrome register value
+ * to the default target el.
+ */
+void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp,
+ uint32_t syndrome)
+{
+ raise_exception(env, excp, syndrome, exception_target_el(env));
+}
+
/* Raise an exception with EC_ADVSIMDFPACCESS. */
void HELPER(exception_advsimdfp_access)(CPUARMState *env,
uint32_t syndrome, uint32_t target_el)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index fcb6ee648b..81c27e7c70 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1097,9 +1097,10 @@ static void gen_exception_el(int excp, uint32_t syndrome, uint32_t target_el)
gen_exception_el_v(excp, syndrome, tcg_constant_i32(target_el));
}
-static void gen_exception(DisasContext *s, int excp, uint32_t syndrome)
+static void gen_exception(int excp, uint32_t syndrome)
{
- gen_exception_el(excp, syndrome, default_exception_el(s));
+ gen_helper_exception_with_syndrome(cpu_env, tcg_constant_i32(excp),
+ tcg_constant_i32(syndrome));
}
static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp,
@@ -1123,7 +1124,14 @@ void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp,
void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, uint32_t syn)
{
- gen_exception_insn_el(s, pc, excp, syn, default_exception_el(s));
+ if (s->aarch64) {
+ gen_a64_set_pc_im(pc);
+ } else {
+ gen_set_condexec(s);
+ gen_set_pc_im(s, pc);
+ }
+ gen_exception(excp, syn);
+ s->base.is_jmp = DISAS_NORETURN;
}
static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn)
@@ -9766,7 +9774,7 @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
switch (dc->base.is_jmp) {
case DISAS_SWI:
gen_ss_advance(dc);
- gen_exception(dc, EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb));
+ gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb));
break;
case DISAS_HVC:
gen_ss_advance(dc);
@@ -9835,7 +9843,7 @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
gen_helper_yield(cpu_env);
break;
case DISAS_SWI:
- gen_exception(dc, EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb));
+ gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb));
break;
case DISAS_HVC:
gen_exception_el(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2);
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 21/25] target/arm: Remove default_exception_el
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (19 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 20/25] target/arm: Introduce helper_exception_with_syndrome Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:43 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 22/25] target/arm: Create raise_exception_debug Richard Henderson
` (3 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
This function is no longer used. At the same time, remove
DisasContext.secure_routed_to_el3, as it in turn becomes unused.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/translate.h | 16 ----------------
target/arm/translate-a64.c | 5 -----
target/arm/translate.c | 5 -----
3 files changed, 26 deletions(-)
diff --git a/target/arm/translate.h b/target/arm/translate.h
index d7a9acf5a9..b01a58653e 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -43,8 +43,6 @@ typedef struct DisasContext {
int fp_excp_el; /* FP exception EL or 0 if enabled */
int sve_excp_el; /* SVE exception EL or 0 if enabled */
int sve_len; /* SVE vector length in bytes */
- /* Flag indicating that exceptions from secure mode are routed to EL3. */
- bool secure_routed_to_el3;
bool vfp_enabled; /* FP enabled via FPSCR.EN */
int vec_len;
int vec_stride;
@@ -199,20 +197,6 @@ static inline int get_mem_index(DisasContext *s)
return arm_to_core_mmu_idx(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_SE10_0 && s->secure_routed_to_el3)
- ? 3 : MAX(1, s->current_el);
-}
-
static inline void disas_set_insn_syndrome(DisasContext *s, uint32_t syn)
{
/* We don't need to save all of the syndrome so we mask and shift
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index b45039a124..8da2ca8324 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -14584,11 +14584,6 @@ static void aarch64_tr_init_disas_context(DisasContextBase *dcbase,
dc->condjmp = 0;
dc->aarch64 = true;
- /* 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.
- */
- dc->secure_routed_to_el3 = arm_feature(env, ARM_FEATURE_EL3) &&
- !arm_el_is_aa64(env, 3);
dc->thumb = false;
dc->sctlr_b = 0;
dc->be_data = EX_TBFLAG_ANY(tb_flags, BE_DATA) ? MO_BE : MO_LE;
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 81c27e7c70..6617de775f 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -9319,11 +9319,6 @@ static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
dc->condjmp = 0;
dc->aarch64 = false;
- /* 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.
- */
- dc->secure_routed_to_el3 = arm_feature(env, ARM_FEATURE_EL3) &&
- !arm_el_is_aa64(env, 3);
dc->thumb = EX_TBFLAG_AM32(tb_flags, THUMB);
dc->be_data = EX_TBFLAG_ANY(tb_flags, BE_DATA) ? MO_BE : MO_LE;
condexec = EX_TBFLAG_AM32(tb_flags, CONDEXEC);
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 22/25] target/arm: Create raise_exception_debug
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (20 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 21/25] target/arm: Remove default_exception_el Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:59 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 23/25] target/arm: Move arm_debug_target_el to debug_helper.c Richard Henderson
` (2 subsequent siblings)
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Handle the debug vs current el exception test in one place.
Leave EXCP_BKPT alone, since that treats debug < current differently.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/debug_helper.c | 44 +++++++++++++++++++++------------------
1 file changed, 24 insertions(+), 20 deletions(-)
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index a3a1b98de2..26004df99b 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -12,6 +12,26 @@
#include "exec/helper-proto.h"
+/*
+ * Raise an exception to the debug target el.
+ * Modify syndrome to indicate when origin and target EL are the same.
+ */
+G_NORETURN static void
+raise_exception_debug(CPUARMState *env, uint32_t excp, uint32_t syndrome)
+{
+ int debug_el = arm_debug_target_el(env);
+ int cur_el = arm_current_el(env);
+
+ /*
+ * If singlestep is targeting a lower EL than the current one, then
+ * DisasContext.ss_active must be false and we can never get here.
+ * Similarly for watchpoint and breakpoint matches.
+ */
+ assert(debug_el >= cur_el);
+ syndrome |= (debug_el == cur_el) << ARM_EL_EC_SHIFT;
+ raise_exception(env, excp, syndrome, debug_el);
+}
+
/* See AArch64.GenerateDebugExceptionsFrom() in ARM ARM pseudocode */
static bool aa64_generate_debug_exceptions(CPUARMState *env)
{
@@ -418,19 +438,16 @@ void arm_debug_excp_handler(CPUState *cs)
if (wp_hit) {
if (wp_hit->flags & BP_CPU) {
bool wnr = (wp_hit->flags & BP_WATCHPOINT_HIT_WRITE) != 0;
- bool same_el = arm_debug_target_el(env) == arm_current_el(env);
cs->watchpoint_hit = NULL;
env->exception.fsr = arm_debug_exception_fsr(env);
env->exception.vaddress = wp_hit->hitaddr;
- raise_exception(env, EXCP_DATA_ABORT,
- syn_watchpoint(same_el, 0, wnr),
- arm_debug_target_el(env));
+ raise_exception_debug(env, EXCP_DATA_ABORT,
+ syn_watchpoint(0, 0, wnr));
}
} else {
uint64_t pc = is_a64(env) ? env->pc : env->regs[15];
- bool same_el = (arm_debug_target_el(env) == arm_current_el(env));
/*
* (1) GDB breakpoints should be handled first.
@@ -450,9 +467,7 @@ void arm_debug_excp_handler(CPUState *cs)
* exception/security level.
*/
env->exception.vaddress = 0;
- raise_exception(env, EXCP_PREFETCH_ABORT,
- syn_breakpoint(same_el),
- arm_debug_target_el(env));
+ raise_exception_debug(env, EXCP_PREFETCH_ABORT, syn_breakpoint(0));
}
}
@@ -489,18 +504,7 @@ void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome)
void HELPER(exception_swstep)(CPUARMState *env, uint32_t syndrome)
{
- int debug_el = arm_debug_target_el(env);
- int cur_el = arm_current_el(env);
-
- /*
- * If singlestep is targeting a lower EL than the current one, then
- * DisasContext.ss_active must be false and we can never get here.
- */
- assert(debug_el >= cur_el);
- if (debug_el == cur_el) {
- syndrome |= 1 << ARM_EL_EC_SHIFT;
- }
- raise_exception(env, EXCP_UDEF, syndrome, debug_el);
+ raise_exception_debug(env, EXCP_UDEF, syndrome);
}
#if !defined(CONFIG_USER_ONLY)
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 23/25] target/arm: Move arm_debug_target_el to debug_helper.c
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (21 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 22/25] target/arm: Create raise_exception_debug Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:43 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 24/25] target/arm: Rearrange Secure PL1 test in arm_debug_target_el Richard Henderson
2022-06-07 2:47 ` [PATCH v2 25/25] target/arm: Fix Secure PL1 tests in fp_exception_el Richard Henderson
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
This function is no longer used outside debug_helper.c.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu.h | 21 ---------------------
target/arm/debug_helper.c | 21 +++++++++++++++++++++
2 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 5224dc2c85..5bc6382fce 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2967,27 +2967,6 @@ typedef enum ARMASIdx {
ARMASIdx_TagS = 3,
} ARMASIdx;
-/* Return the Exception Level targeted by debug exceptions. */
-static inline int arm_debug_target_el(CPUARMState *env)
-{
- bool secure = arm_is_secure(env);
- bool route_to_el2 = false;
-
- if (arm_is_el2_enabled(env)) {
- route_to_el2 = env->cp15.hcr_el2 & HCR_TGE ||
- env->cp15.mdcr_el2 & MDCR_TDE;
- }
-
- if (route_to_el2) {
- return 2;
- } else if (arm_feature(env, ARM_FEATURE_EL3) &&
- !arm_el_is_aa64(env, 3) && secure) {
- return 3;
- } else {
- return 1;
- }
-}
-
static inline bool arm_v7m_csselr_razwi(ARMCPU *cpu)
{
/* If all the CLIDR.Ctypem bits are 0 there are no caches, and
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index 26004df99b..b18a6bd3a2 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -12,6 +12,27 @@
#include "exec/helper-proto.h"
+/* Return the Exception Level targeted by debug exceptions. */
+static int arm_debug_target_el(CPUARMState *env)
+{
+ bool secure = arm_is_secure(env);
+ bool route_to_el2 = false;
+
+ if (arm_is_el2_enabled(env)) {
+ route_to_el2 = env->cp15.hcr_el2 & HCR_TGE ||
+ env->cp15.mdcr_el2 & MDCR_TDE;
+ }
+
+ if (route_to_el2) {
+ return 2;
+ } else if (arm_feature(env, ARM_FEATURE_EL3) &&
+ !arm_el_is_aa64(env, 3) && secure) {
+ return 3;
+ } else {
+ return 1;
+ }
+}
+
/*
* Raise an exception to the debug target el.
* Modify syndrome to indicate when origin and target EL are the same.
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v2 24/25] target/arm: Rearrange Secure PL1 test in arm_debug_target_el
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (22 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 23/25] target/arm: Move arm_debug_target_el to debug_helper.c Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:53 ` Peter Maydell
2022-06-07 2:47 ` [PATCH v2 25/25] target/arm: Fix Secure PL1 tests in fp_exception_el Richard Henderson
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Not a bug, because arm_is_el2_enabled tests for secure,
and SCR_EL3.EEL2 cannot be set for AArch32, however the
ordering of the tests looks odd. Mirror the structure
over in exception_target_el().
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/debug_helper.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index b18a6bd3a2..59dfcb5d5c 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -15,22 +15,24 @@
/* Return the Exception Level targeted by debug exceptions. */
static int arm_debug_target_el(CPUARMState *env)
{
- bool secure = arm_is_secure(env);
- bool route_to_el2 = false;
-
- if (arm_is_el2_enabled(env)) {
- route_to_el2 = env->cp15.hcr_el2 & HCR_TGE ||
- env->cp15.mdcr_el2 & MDCR_TDE;
- }
-
- if (route_to_el2) {
- return 2;
- } else if (arm_feature(env, ARM_FEATURE_EL3) &&
- !arm_el_is_aa64(env, 3) && secure) {
+ /*
+ * No such thing as secure EL1 if EL3 is AArch32.
+ * Remap Secure PL1 to EL3.
+ */
+ if (arm_is_secure(env) && !arm_el_is_aa64(env, 3)) {
return 3;
- } else {
- return 1;
}
+
+ /*
+ * HCR.TGE redirects EL0 exceptions from EL1 to EL2.
+ * MDCR.TDE redirects both EL0 and EL1 debug exceptions to EL2.
+ */
+ if (arm_is_el2_enabled(env) &&
+ (env->cp15.hcr_el2 & HCR_TGE || env->cp15.mdcr_el2 & MDCR_TDE)) {
+ return 2;
+ }
+
+ return 1;
}
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH v2 24/25] target/arm: Rearrange Secure PL1 test in arm_debug_target_el
2022-06-07 2:47 ` [PATCH v2 24/25] target/arm: Rearrange Secure PL1 test in arm_debug_target_el Richard Henderson
@ 2022-06-09 16:53 ` Peter Maydell
2022-06-09 19:49 ` Richard Henderson
0 siblings, 1 reply; 49+ messages in thread
From: Peter Maydell @ 2022-06-09 16:53 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Tue, 7 Jun 2022 at 04:06, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Not a bug, because arm_is_el2_enabled tests for secure,
> and SCR_EL3.EEL2 cannot be set for AArch32, however the
> ordering of the tests looks odd. Mirror the structure
> over in exception_target_el().
I think the code is following the ordering in the
DebugTarget() and DebugTargetFrom() pseudocode (or else some other
equivalent function in an older version of the Arm ARM...)
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/debug_helper.c | 30 ++++++++++++++++--------------
> 1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
> index b18a6bd3a2..59dfcb5d5c 100644
> --- a/target/arm/debug_helper.c
> +++ b/target/arm/debug_helper.c
> @@ -15,22 +15,24 @@
> /* Return the Exception Level targeted by debug exceptions. */
> static int arm_debug_target_el(CPUARMState *env)
> {
> - bool secure = arm_is_secure(env);
> - bool route_to_el2 = false;
> -
> - if (arm_is_el2_enabled(env)) {
> - route_to_el2 = env->cp15.hcr_el2 & HCR_TGE ||
> - env->cp15.mdcr_el2 & MDCR_TDE;
> - }
> -
> - if (route_to_el2) {
> - return 2;
> - } else if (arm_feature(env, ARM_FEATURE_EL3) &&
> - !arm_el_is_aa64(env, 3) && secure) {
> + /*
> + * No such thing as secure EL1 if EL3 is AArch32.
> + * Remap Secure PL1 to EL3.
> + */
I think you're also relying on there being no secure EL2
if EL3 is AArch32 (otherwise an exception from secure EL0
might need to be routed to secure EL2, not EL3).
> + if (arm_is_secure(env) && !arm_el_is_aa64(env, 3)) {
> return 3;
> - } else {
> - return 1;
> }
> +
> + /*
> + * HCR.TGE redirects EL0 exceptions from EL1 to EL2.
> + * MDCR.TDE redirects both EL0 and EL1 debug exceptions to EL2.
> + */
> + if (arm_is_el2_enabled(env) &&
> + (env->cp15.hcr_el2 & HCR_TGE || env->cp15.mdcr_el2 & MDCR_TDE)) {
> + return 2;
> + }
> +
> + return 1;
> }
Anyway, if you prefer this way around
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
though I think there is usually some value in following
the pseudocode's arrangement.
thanks
-- PMM
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH v2 24/25] target/arm: Rearrange Secure PL1 test in arm_debug_target_el
2022-06-09 16:53 ` Peter Maydell
@ 2022-06-09 19:49 ` Richard Henderson
0 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2022-06-09 19:49 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, qemu-arm
On 6/9/22 09:53, Peter Maydell wrote:
> On Tue, 7 Jun 2022 at 04:06, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Not a bug, because arm_is_el2_enabled tests for secure,
>> and SCR_EL3.EEL2 cannot be set for AArch32, however the
>> ordering of the tests looks odd. Mirror the structure
>> over in exception_target_el().
>
> I think the code is following the ordering in the
> DebugTarget() and DebugTargetFrom() pseudocode (or else some other
> equivalent function in an older version of the Arm ARM...)
Fair enough.
> I think you're also relying on there being no secure EL2
> if EL3 is AArch32 (otherwise an exception from secure EL0
> might need to be routed to secure EL2, not EL3).
Correct, though I don't imagine SEL2 will ever apply to A32.
I'll drop the patch though.
r~
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v2 25/25] target/arm: Fix Secure PL1 tests in fp_exception_el
2022-06-07 2:47 [PATCH v2 00/25] target/arm: tidy exception routing Richard Henderson
` (23 preceding siblings ...)
2022-06-07 2:47 ` [PATCH v2 24/25] target/arm: Rearrange Secure PL1 test in arm_debug_target_el Richard Henderson
@ 2022-06-07 2:47 ` Richard Henderson
2022-06-09 16:56 ` Peter Maydell
24 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2022-06-07 2:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
We were using arm_is_secure and is_a64, which are
tests against the current EL, as opposed to
arm_el_is_aa64 and arm_is_secure_below_el3, which
can be applied to a different EL than current.
Consolidate the two tests.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/helper.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index eeca1a5dc8..1d81a01c94 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -13400,27 +13400,22 @@ int fp_exception_el(CPUARMState *env, int cur_el)
int fpen = FIELD_EX64(env->cp15.cpacr_el1, CPACR_EL1, FPEN);
switch (fpen) {
+ case 1:
+ if (cur_el != 0) {
+ break;
+ }
+ /* fall through */
case 0:
case 2:
- if (cur_el == 0 || cur_el == 1) {
- /* Trap to PL1, which might be EL1 or EL3 */
- if (arm_is_secure(env) && !arm_el_is_aa64(env, 3)) {
- return 3;
- }
- return 1;
- }
- if (cur_el == 3 && !is_a64(env)) {
- /* Secure PL1 running at EL3 */
+ /* Trap from Secure PL0 or PL1 to Secure PL1. */
+ if (!arm_el_is_aa64(env, 3)
+ && (cur_el == 3 || arm_is_secure_below_el3(env))) {
return 3;
}
- break;
- case 1:
- if (cur_el == 0) {
+ if (cur_el <= 1) {
return 1;
}
break;
- case 3:
- break;
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 49+ messages in thread