powerpc/irq: Fix another case of lazy IRQ state getting out of sync

So we have another case of paca->irq_happened getting out of
sync with the HW irq state. This can happen when a perfmon
interrupt occurs while soft disabled, as it will return to a
soft disabled but hard enabled context while leaving a stale
PACA_IRQ_HARD_DIS flag set.

This patch fixes it, and also adds a test for the condition
of those flags being out of sync in arch_local_irq_restore()
when CONFIG_TRACE_IRQFLAGS is enabled.

This helps catching those gremlins faster (and so far I
can't seem see any anymore, so that's good news).

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index fc60150..ef2074c 100644 (file)
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -588,23 +588,19 @@ _GLOBAL(ret_from_except_lite)
 fast_exc_return_irq:
 restore:
        /*
-        * This is the main kernel exit path, we first check if we
-        * have to change our interrupt state.
+        * This is the main kernel exit path. First we check if we
+        * are about to re-enable interrupts
         */
        ld      r5,SOFTE(r1)
        lbz     r6,PACASOFTIRQEN(r13)
-       cmpwi   cr1,r5,0
-       cmpw    cr0,r5,r6
-       beq     cr0,4f
+       cmpwi   cr0,r5,0
+       beq     restore_irq_off
 
-       /* We do, handle disable first, which is easy */
-       bne     cr1,3f;
-       li      r0,0
-       stb     r0,PACASOFTIRQEN(r13);
-       TRACE_DISABLE_INTS
-       b       4f
+       /* We are enabling, were we already enabled ? Yes, just return */
+       cmpwi   cr0,r6,1
+       beq     cr0,do_restore
 
-3:     /*
+       /*
         * We are about to soft-enable interrupts (we are hard disabled
         * at this point). We check if there's anything that needs to
         * be replayed first.
@@ -626,7 +622,7 @@ restore_no_replay:
        /*
         * Final return path. BookE is handled in a different file
         */
-4:
+do_restore:
 #ifdef CONFIG_PPC_BOOK3E
        b       .exception_return_book3e
 #else
@@ -700,6 +696,25 @@ fast_exception_return:
 #endif /* CONFIG_PPC_BOOK3E */
 
        /*
+        * We are returning to a context with interrupts soft disabled.
+        *
+        * However, we may also about to hard enable, so we need to
+        * make sure that in this case, we also clear PACA_IRQ_HARD_DIS
+        * or that bit can get out of sync and bad things will happen
+        */
+restore_irq_off:
+       ld      r3,_MSR(r1)
+       lbz     r7,PACAIRQHAPPENED(r13)
+       andi.   r0,r3,MSR_EE
+       beq     1f
+       rlwinm  r7,r7,0,~PACA_IRQ_HARD_DIS
+       stb     r7,PACAIRQHAPPENED(r13)
+1:     li      r0,0
+       stb     r0,PACASOFTIRQEN(r13);
+       TRACE_DISABLE_INTS
+       b       do_restore
+
+       /*
         * Something did happen, check if a re-emit is needed
         * (this also clears paca->irq_happened)
         */
@@ -748,6 +763,9 @@ restore_check_irq_replay:
 #endif /* CONFIG_PPC_BOOK3E */
 1:     b       .ret_from_except /* What else to do here ? */
  
+
+
+3:
 do_work:
 #ifdef CONFIG_PREEMPT
        andi.   r0,r3,MSR_PR    /* Returning to user mode? */

diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index c6c6f3b..641da9e 100644 (file)
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -229,6 +229,19 @@ notrace void arch_local_irq_restore(unsigned long en)
         */
        if (unlikely(irq_happened != PACA_IRQ_HARD_DIS))
                __hard_irq_disable();
+#ifdef CONFIG_TRACE_IRQFLAG
+       else {
+               /*
+                * We should already be hard disabled here. We had bugs
+                * where that wasn't the case so let's dbl check it and
+                * warn if we are wrong. Only do that when IRQ tracing
+                * is enabled as mfmsr() can be costly.
+                */
+               if (WARN_ON(mfmsr() & MSR_EE))
+                       __hard_irq_disable();
+       }
+#endif /* CONFIG_TRACE_IRQFLAG */
+
        set_soft_enabled(0);
 
        /*