| Message ID | 20221022052207.471328-1-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | powerpc/64s/interrupt: Fix clear of PACA_IRQS_HARD_DIS when returning to soft-masked context | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 10 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 10 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 6 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 23 jobs. |
> On 22-Oct-2022, at 10:52 AM, Nicholas Piggin <npiggin@gmail.com> wrote: > > Commit a4cb3651a1743 ("powerpc/64s/interrupt: Fix lost interrupts when > returning to soft-masked context") fixed the problem of pending irqs > pending cleared when clearing the HARD_DIS bit, but then it didn't clear > the bit at all. This change clears HARD_DIS without affecting other bits > in the mask. > > When an interrupt hits in a soft-masked section that has MSR[EE]=1, it > can hard disable and set PACA_IRQS_HARD_DIS,which must be cleared > when returning to the EE=1 caller (unless it was set due to a > MUST_HARD_MASK interrupt becoming pending). Failure to clear this leaves > the returned-to context running with MSR[EE]=1 and PACA_IRQS_HARD_DIS, > which confuses irq assertions and could be dangerous for code that might > test the flag. > > This was observed in a hash MMU kernel where a kernel hash fault hits in > a local_irqs_disabled region that has EE=1. The hash fault also runs > with EE=1, then as it returns, a decrementer hits in the restart section > and the irq restart code hard-masks which sets the PACA_IRQ_HARD_DIS > flag, which is not clear when the original context is returned to. > > Reported-by: Sachin Sant <sachinp@linux.ibm.com> > Fixes: a4cb3651a1743 ("powerpc/64s/interrupt: Fix lost interrupts when returning to soft-masked context") > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- Thanks Nick. The patch fixes the reported problem Tested-by: Sachin Sant <sachinp@linux.ibm.com>
On Sat, 22 Oct 2022 15:22:07 +1000, Nicholas Piggin wrote: > Commit a4cb3651a1743 ("powerpc/64s/interrupt: Fix lost interrupts when > returning to soft-masked context") fixed the problem of pending irqs > pending cleared when clearing the HARD_DIS bit, but then it didn't clear > the bit at all. This change clears HARD_DIS without affecting other bits > in the mask. > > When an interrupt hits in a soft-masked section that has MSR[EE]=1, it > can hard disable and set PACA_IRQS_HARD_DIS,which must be cleared > when returning to the EE=1 caller (unless it was set due to a > MUST_HARD_MASK interrupt becoming pending). Failure to clear this leaves > the returned-to context running with MSR[EE]=1 and PACA_IRQS_HARD_DIS, > which confuses irq assertions and could be dangerous for code that might > test the flag. > > [...] Applied to powerpc/fixes. [1/1] powerpc/64s/interrupt: Fix clear of PACA_IRQS_HARD_DIS when returning to soft-masked context https://git.kernel.org/powerpc/c/65722736c3baf29e02e964a09e85c9ef71c48e8d cheers
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S index 978a173eb339..a019ed6fc839 100644 --- a/arch/powerpc/kernel/interrupt_64.S +++ b/arch/powerpc/kernel/interrupt_64.S @@ -532,15 +532,24 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_kernel) * Returning to soft-disabled context. * Check if a MUST_HARD_MASK interrupt has become pending, in which * case we need to disable MSR[EE] in the return context. + * + * The MSR[EE] check catches among other things the short incoherency + * in hard_irq_disable() between clearing MSR[EE] and setting + * PACA_IRQ_HARD_DIS. */ ld r12,_MSR(r1) andi. r10,r12,MSR_EE beq .Lfast_kernel_interrupt_return_\srr\() // EE already disabled lbz r11,PACAIRQHAPPENED(r13) andi. r10,r11,PACA_IRQ_MUST_HARD_MASK - beq .Lfast_kernel_interrupt_return_\srr\() // No HARD_MASK pending + bne 1f // HARD_MASK is pending + // No HARD_MASK pending, clear possible HARD_DIS set by interrupt + andi. r11,r11,(~PACA_IRQ_HARD_DIS)@l + stb r11,PACAIRQHAPPENED(r13) + b .Lfast_kernel_interrupt_return_\srr\() + - /* Must clear MSR_EE from _MSR */ +1: /* Must clear MSR_EE from _MSR */ #ifdef CONFIG_PPC_BOOK3S li r10,0 /* Clear valid before changing _MSR */
Commit a4cb3651a1743 ("powerpc/64s/interrupt: Fix lost interrupts when returning to soft-masked context") fixed the problem of pending irqs pending cleared when clearing the HARD_DIS bit, but then it didn't clear the bit at all. This change clears HARD_DIS without affecting other bits in the mask. When an interrupt hits in a soft-masked section that has MSR[EE]=1, it can hard disable and set PACA_IRQS_HARD_DIS,which must be cleared when returning to the EE=1 caller (unless it was set due to a MUST_HARD_MASK interrupt becoming pending). Failure to clear this leaves the returned-to context running with MSR[EE]=1 and PACA_IRQS_HARD_DIS, which confuses irq assertions and could be dangerous for code that might test the flag. This was observed in a hash MMU kernel where a kernel hash fault hits in a local_irqs_disabled region that has EE=1. The hash fault also runs with EE=1, then as it returns, a decrementer hits in the restart section and the irq restart code hard-masks which sets the PACA_IRQ_HARD_DIS flag, which is not clear when the original context is returned to. Reported-by: Sachin Sant <sachinp@linux.ibm.com> Fixes: a4cb3651a1743 ("powerpc/64s/interrupt: Fix lost interrupts when returning to soft-masked context") Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/kernel/interrupt_64.S | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)