* [Qemu-devel] [PATCH] target-arm: Fix "no 64-bit EL2" assumption in arm_excp_unmasked()
@ 2015-10-08 18:08 Peter Maydell
2015-10-14 16:05 ` Edgar E. Iglesias
0 siblings, 1 reply; 2+ messages in thread
From: Peter Maydell @ 2015-10-08 18:08 UTC (permalink / raw)
To: qemu-devel; +Cc: Sergey Sorokin, Edgar E. Iglesias, patches
The code in arm_excp_unmasked() suppresses the ability of PSTATE.AIF
to mask exceptions from a lower EL targeting EL2 or EL3 if the
CPU is 64-bit. This is correct for a target of EL3, but not correct
for targeting EL2. Further, we go to some effort to calculate
scr and hcr values which are not used at all for the 64-bit CPU
case.
Rearrange the code to correctly implement the 64-bit CPU logic
and keep the hcr/scr calculations in the 32-bit CPU codepath.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
As promised, here's the patch that fixes the assumption about
64-bit implying no EL2 in arm_excp_unmasked(). This applies on
top of my current target-arm.next tree, since it has Sergey's
patch in it which touches the same code.
target-arm/cpu.h | 82 +++++++++++++++++++++++++++++++++++---------------------
1 file changed, 52 insertions(+), 30 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 99b34c4..6b6e4e0 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1524,8 +1524,6 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
CPUARMState *env = cs->env_ptr;
unsigned int cur_el = arm_current_el(env);
bool secure = arm_is_secure(env);
- bool scr;
- bool hcr;
bool pstate_unmasked;
int8_t unmasked = 0;
@@ -1539,31 +1537,10 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
switch (excp_idx) {
case EXCP_FIQ:
- /* If FIQs are routed to EL3 or EL2 then there are cases where we
- * override the CPSR.F in determining if the exception is masked or
- * not. If neither of these are set then we fall back to the CPSR.F
- * setting otherwise we further assess the state below.
- */
- hcr = (env->cp15.hcr_el2 & HCR_FMO);
- scr = (env->cp15.scr_el3 & SCR_FIQ);
-
- /* When EL3 is 32-bit, the SCR.FW bit controls whether the CPSR.F bit
- * masks FIQ interrupts when taken in non-secure state. If SCR.FW is
- * set then FIQs can be masked by CPSR.F when non-secure but only
- * when FIQs are only routed to EL3.
- */
- scr = scr && !((env->cp15.scr_el3 & SCR_FW) && !hcr);
pstate_unmasked = !(env->daif & PSTATE_F);
break;
case EXCP_IRQ:
- /* When EL3 execution state is 32-bit, if HCR.IMO is set then we may
- * override the CPSR.I masking when in non-secure state. The SCR.IRQ
- * setting has already been taken into consideration when setting the
- * target EL, so it does not have a further affect here.
- */
- hcr = (env->cp15.hcr_el2 & HCR_IMO);
- scr = false;
pstate_unmasked = !(env->daif & PSTATE_I);
break;
@@ -1588,13 +1565,58 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
* interrupt.
*/
if ((target_el > cur_el) && (target_el != 1)) {
- /* ARM_FEATURE_AARCH64 enabled means the highest EL is AArch64.
- * This code currently assumes that EL2 is not implemented
- * (and so that highest EL will be 3 and the target_el also 3).
- */
- if (arm_feature(env, ARM_FEATURE_AARCH64) ||
- ((scr || hcr) && (!secure))) {
- unmasked = 1;
+ /* Exceptions targeting a higher EL may not be maskable */
+ if (arm_feature(env, ARM_FEATURE_AARCH64)) {
+ /* 64-bit masking rules are simple: exceptions to EL3
+ * can't be masked, and exceptions to EL2 can only be
+ * masked from Secure state. The HCR and SCR settings
+ * don't affect the masking logic, only the interrupt routing.
+ */
+ if (target_el == 3 || !secure) {
+ unmasked = 1;
+ }
+ } else {
+ /* The old 32-bit-only environment has a more complicated
+ * masking setup. HCR and SCR bits not only affect interrupt
+ * routing but also change the behaviour of masking.
+ */
+ bool hcr, scr;
+
+ switch (excp_idx) {
+ case EXCP_FIQ:
+ /* If FIQs are routed to EL3 or EL2 then there are cases where
+ * we override the CPSR.F in determining if the exception is
+ * masked or not. If neither of these are set then we fall back
+ * to the CPSR.F setting otherwise we further assess the state
+ * below.
+ */
+ hcr = (env->cp15.hcr_el2 & HCR_FMO);
+ scr = (env->cp15.scr_el3 & SCR_FIQ);
+
+ /* When EL3 is 32-bit, the SCR.FW bit controls whether the
+ * CPSR.F bit masks FIQ interrupts when taken in non-secure
+ * state. If SCR.FW is set then FIQs can be masked by CPSR.F
+ * when non-secure but only when FIQs are only routed to EL3.
+ */
+ scr = scr && !((env->cp15.scr_el3 & SCR_FW) && !hcr);
+ break;
+ case EXCP_IRQ:
+ /* When EL3 execution state is 32-bit, if HCR.IMO is set then
+ * we may override the CPSR.I masking when in non-secure state.
+ * The SCR.IRQ setting has already been taken into consideration
+ * when setting the target EL, so it does not have a further
+ * affect here.
+ */
+ hcr = (env->cp15.hcr_el2 & HCR_IMO);
+ scr = false;
+ break;
+ default:
+ g_assert_not_reached();
+ }
+
+ if ((scr || hcr) && !secure) {
+ unmasked = 1;
+ }
}
}
--
1.9.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH] target-arm: Fix "no 64-bit EL2" assumption in arm_excp_unmasked()
2015-10-08 18:08 [Qemu-devel] [PATCH] target-arm: Fix "no 64-bit EL2" assumption in arm_excp_unmasked() Peter Maydell
@ 2015-10-14 16:05 ` Edgar E. Iglesias
0 siblings, 0 replies; 2+ messages in thread
From: Edgar E. Iglesias @ 2015-10-14 16:05 UTC (permalink / raw)
To: Peter Maydell; +Cc: Sergey Sorokin, qemu-devel, patches
On Thu, Oct 08, 2015 at 07:08:49PM +0100, Peter Maydell wrote:
> The code in arm_excp_unmasked() suppresses the ability of PSTATE.AIF
> to mask exceptions from a lower EL targeting EL2 or EL3 if the
> CPU is 64-bit. This is correct for a target of EL3, but not correct
> for targeting EL2. Further, we go to some effort to calculate
> scr and hcr values which are not used at all for the 64-bit CPU
> case.
>
> Rearrange the code to correctly implement the 64-bit CPU logic
> and keep the hcr/scr calculations in the 32-bit CPU codepath.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Looks good to me! I gave it a test run together with
Sergey's eb4ac7e08b342ae20661a4feac885b8e2e966418 from
target-arm.next.
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
> As promised, here's the patch that fixes the assumption about
> 64-bit implying no EL2 in arm_excp_unmasked(). This applies on
> top of my current target-arm.next tree, since it has Sergey's
> patch in it which touches the same code.
>
>
>
> target-arm/cpu.h | 82 +++++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 52 insertions(+), 30 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 99b34c4..6b6e4e0 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1524,8 +1524,6 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
> CPUARMState *env = cs->env_ptr;
> unsigned int cur_el = arm_current_el(env);
> bool secure = arm_is_secure(env);
> - bool scr;
> - bool hcr;
> bool pstate_unmasked;
> int8_t unmasked = 0;
>
> @@ -1539,31 +1537,10 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
>
> switch (excp_idx) {
> case EXCP_FIQ:
> - /* If FIQs are routed to EL3 or EL2 then there are cases where we
> - * override the CPSR.F in determining if the exception is masked or
> - * not. If neither of these are set then we fall back to the CPSR.F
> - * setting otherwise we further assess the state below.
> - */
> - hcr = (env->cp15.hcr_el2 & HCR_FMO);
> - scr = (env->cp15.scr_el3 & SCR_FIQ);
> -
> - /* When EL3 is 32-bit, the SCR.FW bit controls whether the CPSR.F bit
> - * masks FIQ interrupts when taken in non-secure state. If SCR.FW is
> - * set then FIQs can be masked by CPSR.F when non-secure but only
> - * when FIQs are only routed to EL3.
> - */
> - scr = scr && !((env->cp15.scr_el3 & SCR_FW) && !hcr);
> pstate_unmasked = !(env->daif & PSTATE_F);
> break;
>
> case EXCP_IRQ:
> - /* When EL3 execution state is 32-bit, if HCR.IMO is set then we may
> - * override the CPSR.I masking when in non-secure state. The SCR.IRQ
> - * setting has already been taken into consideration when setting the
> - * target EL, so it does not have a further affect here.
> - */
> - hcr = (env->cp15.hcr_el2 & HCR_IMO);
> - scr = false;
> pstate_unmasked = !(env->daif & PSTATE_I);
> break;
>
> @@ -1588,13 +1565,58 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
> * interrupt.
> */
> if ((target_el > cur_el) && (target_el != 1)) {
> - /* ARM_FEATURE_AARCH64 enabled means the highest EL is AArch64.
> - * This code currently assumes that EL2 is not implemented
> - * (and so that highest EL will be 3 and the target_el also 3).
> - */
> - if (arm_feature(env, ARM_FEATURE_AARCH64) ||
> - ((scr || hcr) && (!secure))) {
> - unmasked = 1;
> + /* Exceptions targeting a higher EL may not be maskable */
> + if (arm_feature(env, ARM_FEATURE_AARCH64)) {
> + /* 64-bit masking rules are simple: exceptions to EL3
> + * can't be masked, and exceptions to EL2 can only be
> + * masked from Secure state. The HCR and SCR settings
> + * don't affect the masking logic, only the interrupt routing.
> + */
> + if (target_el == 3 || !secure) {
> + unmasked = 1;
> + }
> + } else {
> + /* The old 32-bit-only environment has a more complicated
> + * masking setup. HCR and SCR bits not only affect interrupt
> + * routing but also change the behaviour of masking.
> + */
> + bool hcr, scr;
> +
> + switch (excp_idx) {
> + case EXCP_FIQ:
> + /* If FIQs are routed to EL3 or EL2 then there are cases where
> + * we override the CPSR.F in determining if the exception is
> + * masked or not. If neither of these are set then we fall back
> + * to the CPSR.F setting otherwise we further assess the state
> + * below.
> + */
> + hcr = (env->cp15.hcr_el2 & HCR_FMO);
> + scr = (env->cp15.scr_el3 & SCR_FIQ);
> +
> + /* When EL3 is 32-bit, the SCR.FW bit controls whether the
> + * CPSR.F bit masks FIQ interrupts when taken in non-secure
> + * state. If SCR.FW is set then FIQs can be masked by CPSR.F
> + * when non-secure but only when FIQs are only routed to EL3.
> + */
> + scr = scr && !((env->cp15.scr_el3 & SCR_FW) && !hcr);
> + break;
> + case EXCP_IRQ:
> + /* When EL3 execution state is 32-bit, if HCR.IMO is set then
> + * we may override the CPSR.I masking when in non-secure state.
> + * The SCR.IRQ setting has already been taken into consideration
> + * when setting the target EL, so it does not have a further
> + * affect here.
> + */
> + hcr = (env->cp15.hcr_el2 & HCR_IMO);
> + scr = false;
> + break;
> + default:
> + g_assert_not_reached();
> + }
> +
> + if ((scr || hcr) && !secure) {
> + unmasked = 1;
> + }
> }
> }
>
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-10-14 16:05 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-08 18:08 [Qemu-devel] [PATCH] target-arm: Fix "no 64-bit EL2" assumption in arm_excp_unmasked() Peter Maydell
2015-10-14 16:05 ` Edgar E. Iglesias
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).