On Tue, 7 Jun 2022 at 04:06, Richard Henderson
<richard.hender...@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.hender...@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.mayd...@linaro.org>
though I think there is usually some value in following
the pseudocode's arrangement.
thanks
-- PMM
