| Message ID | 1444327729-4120-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
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 >
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; + } } }
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(-)