diff mbox

[v10,01/26] target-arm: extend async excp masking

Message ID 1415289073-14681-2-git-send-email-greg.bellows@linaro.org
State New
Headers show

Commit Message

Greg Bellows Nov. 6, 2014, 3:50 p.m. UTC
This patch extends arm_excp_unmasked() to use lookup tables for determining
whether IRQ and FIQ exceptions are masked.  The lookup tables are based on the
ARMv8 and ARMv7 specification physical interrupt masking tables.

If EL3 is using AArch64 IRQ/FIQ masking is ignored in all exception levels
other than EL3 if SCR.{FIQ|IRQ} is set to 1 (routed to EL3).

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>

---

v8 -> v9
- Undo the use of tables for exception masking and instead go with simplified
  logic based on the target EL lookup.
- Remove the masking tables

v7 -> v8
- Add IRQ and FIQ exeception masking lookup tables.
- Rewrite patch to use lookup tables for determining whether an excpetion is
  masked or not.

v5 -> v6
- Globally change Aarch# to AArch#
- Fixed comment termination

v4 -> v5
- Merge with v4 patch 10
---
 target-arm/cpu.h | 66 ++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 52 insertions(+), 14 deletions(-)

Comments

Peter Maydell Nov. 17, 2014, 2:46 p.m. UTC | #1
On 6 November 2014 15:50, Greg Bellows <greg.bellows@linaro.org> wrote:
> This patch extends arm_excp_unmasked() to use lookup tables for determining
> whether IRQ and FIQ exceptions are masked.  The lookup tables are based on the
> ARMv8 and ARMv7 specification physical interrupt masking tables.
>
> If EL3 is using AArch64 IRQ/FIQ masking is ignored in all exception levels
> other than EL3 if SCR.{FIQ|IRQ} is set to 1 (routed to EL3).
>
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
>
> ---
>
> v8 -> v9
> - Undo the use of tables for exception masking and instead go with simplified
>   logic based on the target EL lookup.
> - Remove the masking tables
>
> v7 -> v8
> - Add IRQ and FIQ exeception masking lookup tables.
> - Rewrite patch to use lookup tables for determining whether an excpetion is
>   masked or not.
>
> v5 -> v6
> - Globally change Aarch# to AArch#
> - Fixed comment termination
>
> v4 -> v5
> - Merge with v4 patch 10
> ---
>  target-arm/cpu.h | 66 ++++++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 52 insertions(+), 14 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 7f80090..cf30b2a 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1247,27 +1247,51 @@ 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);
>      unsigned int target_el = arm_excp_target_el(cs, excp_idx);
> -    /* FIXME: Use actual secure state.  */
> -    bool secure = false;
> -    /* If in EL1/0, Physical IRQ routing to EL2 only happens from NS state.  */
> -    bool irq_can_hyp = !secure && cur_el < 2 && target_el == 2;
> -
> -    /* Don't take exceptions if they target a lower EL.  */
> +    bool secure = arm_is_secure(env);
> +    uint32_t scr;
> +    uint32_t hcr;
> +    bool pstate_unmasked;
> +    int8_t unmasked = 0;
> +    bool is_aa64 = arm_el_is_aa64(env, 3);

If you keep this variable, call it el3_is_aa64 (but see comments below).

> +
> +    /* Don't take exceptions if they target a lower EL.
> +     * This check should catch any exceptions that would not be taken but left
> +     * pending.
> +     */
>      if (cur_el > target_el) {
>          return false;
>      }
>
>      switch (excp_idx) {
>      case EXCP_FIQ:
> -        if (irq_can_hyp && (env->cp15.hcr_el2 & HCR_FMO)) {
> -            return true;
> -        }
> -        return !(env->daif & PSTATE_F);
> +        /* 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 &= is_aa64 || !((env->cp15.scr_el3 & SCR_FW) && !hcr);
> +        pstate_unmasked = !(env->daif & PSTATE_F);
> +        break;
> +
>      case EXCP_IRQ:
> -        if (irq_can_hyp && (env->cp15.hcr_el2 & HCR_IMO)) {
> -            return true;
> -        }
> -        return !(env->daif & PSTATE_I);
> +        /* 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 = is_aa64 || (env->cp15.hcr_el2 & HCR_IMO);
> +        scr = false;
> +        pstate_unmasked = !(env->daif & PSTATE_I);
> +        break;
> +
>      case EXCP_VFIQ:
>          if (secure || !(env->cp15.hcr_el2 & HCR_FMO)) {
>              /* VFIQs are only taken when hypervized and non-secure.  */
> @@ -1283,6 +1307,20 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
>      default:
>          g_assert_not_reached();
>      }
> +
> +    /* Use the target EL, current execution state and SCR/HCR settings to
> +     * determine whether the corresponding CPSR bit is used to mask the
> +     * interrupt.
> +     */
> +    if ((target_el > cur_el) && (target_el != 1) && (scr || hcr) &&
> +        (is_aa64 || !secure)) {
> +        unmasked = 1;
> +    }

I think this logic is correct but I find it a little confusing.
In particular it is not obvious that most of this doesn't apply
for 64-bit EL3, because there is an "is_aa64 || " hidden in the
settings of scr and hcr, as well as in the check on !secure.

I would suggest pulling that out so:

    if ((target_el > cur_el) && (target_el != 1) {
        /* With 64-bit EL3 the logic is simple: we always ignore PSTATE masking
         * when taking an exception to a higher-and-not-1 exception level.
         * In a 32 bit EL3 there are some awkward special cases where the
         * SCR/HCR mask bits determine whether or not we should honour
         * PSTATE masking.
         */
        if ((arm_el_is_aa64(env, 3) || ((scr || hcr) && !secure)) {
            unmasked = 1;
        }
    }

(and dropping the "is_aa64 || " in the setting of scr/hcr.)

If you make that tweak you can add my reviewed-by tag.

thanks
-- PMM
diff mbox

Patch

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 7f80090..cf30b2a 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1247,27 +1247,51 @@  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);
     unsigned int target_el = arm_excp_target_el(cs, excp_idx);
-    /* FIXME: Use actual secure state.  */
-    bool secure = false;
-    /* If in EL1/0, Physical IRQ routing to EL2 only happens from NS state.  */
-    bool irq_can_hyp = !secure && cur_el < 2 && target_el == 2;
-
-    /* Don't take exceptions if they target a lower EL.  */
+    bool secure = arm_is_secure(env);
+    uint32_t scr;
+    uint32_t hcr;
+    bool pstate_unmasked;
+    int8_t unmasked = 0;
+    bool is_aa64 = arm_el_is_aa64(env, 3);
+
+    /* Don't take exceptions if they target a lower EL.
+     * This check should catch any exceptions that would not be taken but left
+     * pending.
+     */
     if (cur_el > target_el) {
         return false;
     }
 
     switch (excp_idx) {
     case EXCP_FIQ:
-        if (irq_can_hyp && (env->cp15.hcr_el2 & HCR_FMO)) {
-            return true;
-        }
-        return !(env->daif & PSTATE_F);
+        /* 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 &= is_aa64 || !((env->cp15.scr_el3 & SCR_FW) && !hcr);
+        pstate_unmasked = !(env->daif & PSTATE_F);
+        break;
+
     case EXCP_IRQ:
-        if (irq_can_hyp && (env->cp15.hcr_el2 & HCR_IMO)) {
-            return true;
-        }
-        return !(env->daif & PSTATE_I);
+        /* 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 = is_aa64 || (env->cp15.hcr_el2 & HCR_IMO);
+        scr = false;
+        pstate_unmasked = !(env->daif & PSTATE_I);
+        break;
+
     case EXCP_VFIQ:
         if (secure || !(env->cp15.hcr_el2 & HCR_FMO)) {
             /* VFIQs are only taken when hypervized and non-secure.  */
@@ -1283,6 +1307,20 @@  static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
     default:
         g_assert_not_reached();
     }
+
+    /* Use the target EL, current execution state and SCR/HCR settings to
+     * determine whether the corresponding CPSR bit is used to mask the
+     * interrupt.
+     */
+    if ((target_el > cur_el) && (target_el != 1) && (scr || hcr) &&
+        (is_aa64 || !secure)) {
+        unmasked = 1;
+    }
+
+    /* The PSTATE bits only mask the interrupt if we have not overriden the
+     * ability above.
+     */
+    return unmasked || pstate_unmasked;
 }
 
 static inline CPUARMState *cpu_init(const char *cpu_model)