[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-4.1?] target/arm: Deliver BKPT/BRK exception
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCH for-4.1?] target/arm: Deliver BKPT/BRK exceptions to correct exception level |
Date: |
Tue, 30 Jul 2019 17:31:09 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 |
On 7/30/19 3:25 PM, Peter Maydell wrote:
> Most Arm architectural debug exceptions (eg watchpoints) are ignored
> if the configured "debug exception level" is below the current
> exception level (so for example EL1 can't arrange to get debug exceptions
> for EL2 execution). Exceptions generated by the BRK or BPKT instructions
> are a special case -- they must always cause an exception, so if
> we're executing above the debug exception level then we
> must take them to the current exception level.
>
> This fixes a bug where executing BRK at EL2 could result in an
> exception being taken at EL1 (which is strictly forbidden by the
> architecture).
>
> Fixes: https://bugs.launchpad.net/qemu/+bug/1838277
> Signed-off-by: Peter Maydell <address@hidden>
> ---
> At this point in the release cycle I'm not sure we should put this
> into 4.1 -- it is definitely a bug but it's not a regression as
> we've been wrong like this for multiple releases, pretty much
> since we put in the debug handling code I suspect.
The fix is quite trivial, and the user reported using a release, so
having it in the next release would be nice.
Or as usual, wait for 'last-minute-bugfix-that-postpone-another-rc' and
squeeze this fix in.
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>
> target/arm/op_helper.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
> index 1ab91f915e4..5e1625a1c8a 100644
> --- a/target/arm/op_helper.c
> +++ b/target/arm/op_helper.c
> @@ -370,6 +370,9 @@ void HELPER(exception_with_syndrome)(CPUARMState *env,
> uint32_t excp,
> */
> 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
> @@ -377,7 +380,18 @@ void HELPER(exception_bkpt_insn)(CPUARMState *env,
> uint32_t syndrome)
> * exception/security level.
> */
> env->exception.vaddress = 0;
> - raise_exception(env, EXCP_BKPT, syndrome, arm_debug_target_el(env));
> + /*
> + * 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)
>