diff mbox

[v3,10/16] target-arm: Break out exception masking to a separate func

Message ID 1402994746-8328-11-git-send-email-edgar.iglesias@gmail.com
State New
Headers show

Commit Message

Edgar E. Iglesias June 17, 2014, 8:45 a.m. UTC 
From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 cpu-exec.c       |  5 ++---
 target-arm/cpu.h | 16 ++++++++++++++++
 2 files changed, 18 insertions(+), 3 deletions(-)

Comments

Peter Maydell Aug. 1, 2014, 1:51 p.m. UTC | #1 
On 17 June 2014 09:45, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
>
> +static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
> +{
> +    CPUARMState *env = cs->env_ptr;
> +
> +    switch (excp_idx) {
> +    case EXCP_FIQ:
> +        return !(env->daif & PSTATE_F);
> +    case EXCP_IRQ:
> +        return ((IS_M(env) && env->regs[15] < 0xfffffff0)
> +                            || !(env->daif & PSTATE_I));
> +    default:
> +        g_assert_not_reached();
> +        break;

You don't need a break, we've just asserted that this isn't
reachable. (Conversely if it was possible to get past the
assert we'd be falling out of the function without returning
a value, so break is wrong either way. But "just don't
put in the break" is what we do elsewhere in target-arm.)

> +    }
> +}

thanks
-- PMM
Edgar E. Iglesias Aug. 4, 2014, 1:57 a.m. UTC | #2 
On Fri, Aug 01, 2014 at 02:51:44PM +0100, Peter Maydell wrote:
> On 17 June 2014 09:45, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> >
> > +static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
> > +{
> > +    CPUARMState *env = cs->env_ptr;
> > +
> > +    switch (excp_idx) {
> > +    case EXCP_FIQ:
> > +        return !(env->daif & PSTATE_F);
> > +    case EXCP_IRQ:
> > +        return ((IS_M(env) && env->regs[15] < 0xfffffff0)
> > +                            || !(env->daif & PSTATE_I));
> > +    default:
> > +        g_assert_not_reached();
> > +        break;
> 
> You don't need a break, we've just asserted that this isn't
> reachable. (Conversely if it was possible to get past the
> assert we'd be falling out of the function without returning
> a value, so break is wrong either way. But "just don't
> put in the break" is what we do elsewhere in target-arm.)

I've removed the break, thanks.


> 
> > +    }
> > +}
> 
> thanks
> -- PMM
diff mbox

Patch

diff --git a/cpu-exec.c b/cpu-exec.c
index 38e5f02..a579ffc 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -478,7 +478,7 @@  int cpu_exec(CPUArchState *env)
                     }
 #elif defined(TARGET_ARM)
                     if (interrupt_request & CPU_INTERRUPT_FIQ
-                        && !(env->daif & PSTATE_F)) {
+                        && arm_excp_unmasked(cpu, EXCP_FIQ)) {
                         cpu->exception_index = EXCP_FIQ;
                         cc->do_interrupt(cpu);
                         next_tb = 0;
@@ -493,8 +493,7 @@  int cpu_exec(CPUArchState *env)
                        We avoid this by disabling interrupts when
                        pc contains a magic address.  */
                     if (interrupt_request & CPU_INTERRUPT_HARD
-                        && ((IS_M(env) && env->regs[15] < 0xfffffff0)
-                            || !(env->daif & PSTATE_I))) {
+                        && arm_excp_unmasked(cpu, EXCP_IRQ)) {
                         cpu->exception_index = EXCP_IRQ;
                         cc->do_interrupt(cpu);
                         next_tb = 0;
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index cd2f74a..9d21361 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1129,6 +1129,22 @@  bool write_cpustate_to_list(ARMCPU *cpu);
 #  define TARGET_VIRT_ADDR_SPACE_BITS 32
 #endif
 
+static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
+{
+    CPUARMState *env = cs->env_ptr;
+
+    switch (excp_idx) {
+    case EXCP_FIQ:
+        return !(env->daif & PSTATE_F);
+    case EXCP_IRQ:
+        return ((IS_M(env) && env->regs[15] < 0xfffffff0)
+                            || !(env->daif & PSTATE_I));
+    default:
+        g_assert_not_reached();
+        break;
+    }
+}
+
 static inline CPUARMState *cpu_init(const char *cpu_model)
 {
     ARMCPU *cpu = cpu_arm_init(cpu_model);