Re: [PATCH] powerpc/entry: Disable interrupts before irqentry_exit

[Shrikanth Hegde]

Hi Peter. Thanks for taking a look at it.

On 6/3/26 3:41 PM, Peter Zijlstra wrote:
> On Wed, Jun 03, 2026 at 03:25:21PM +0530, Shrikanth Hegde wrote:
> > Venkat reported a panic on powerpc-next tree where GENERIC_ENTRY has
> > been enabled.
> >
> > kernel BUG at kernel/sched/core.c:7512!
> > NIP  preempt_schedule_irq+0x44/0x118
> > LR   dynamic_irqentry_exit_cond_resched+0x40/0x1a4
> > Call Trace:
> >   dynamic_irqentry_exit_cond_resched+0x40/0x1a4
> >   do_page_fault+0xc0/0x104
> >   data_access_common_virt+0x210/0x220
> >
> > This happens since __do_page_fault ends up enabling the interrupts and
> > it could take significant time such that need_resched could be set. This
> > leads to schedule call in irqentry_exit leading to the bug.
> >
> > There are many such irq handlers which enables the interrupts.
> > Fix it by disabling the irq before calling irqentry_exit. The same
> > pattern exists today in interrupt_exit_kernel_prepare.
> >
> > While there, make those BUG_ON into WARN_ON. Interrupt is disabled right
> > after so it is not that severe. This will still help to catch the
> > offending callsites.
> >
> > Fixes: bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature")
> > Reported-by: Venkat Rao Bagalkote <venkat88@xxxxxxxxxxxxx>
> > Closes: https://lore.kernel.org/all/7904105b-9dfa-4efd-a5ef-bc0276ed255d@xxxxxxxxxxxxx/
> > Signed-off-by: Shrikanth Hegde <sshegde@xxxxxxxxxxxxx>
> > ---
> >
> > This applies on top on powerpc/next tree.
> > base: 6ed60999d33d '("powerpc: Remove unused functions")'
> >
> >   arch/powerpc/include/asm/entry-common.h | 9 +++++----
> >   1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/entry-common.h b/arch/powerpc/include/asm/entry-common.h
> > index de5601282755..7da373a56813 100644
> > --- a/arch/powerpc/include/asm/entry-common.h
> > +++ b/arch/powerpc/include/asm/entry-common.h
> > @@ -253,16 +253,17 @@ static inline void arch_interrupt_enter_prepare(struct pt_regs *regs)
> >   static inline void arch_interrupt_exit_prepare(struct pt_regs *regs)
> >   {
> >   	if (user_mode(regs)) {
> > -		BUG_ON(regs_is_unrecoverable(regs));
> > -		BUG_ON(regs_irqs_disabled(regs));
> > +		WARN_ON(regs_is_unrecoverable(regs));
> > +		WARN_ON(regs_irqs_disabled(regs));
>
> So while you will indeed disable IRQs righ below. This checks the IRQ
> state of regs, not the current state.
>
> What you are allowing through it a *userspace* state that has IRQs
> disabled. That is an invalid state.

Ok. I should have seen it closely. Let me keep them as is.

> >   		/*
> >   		 * We don't need to restore AMR on the way back to userspace for KUAP.
> >   		 * AMR can only have been unlocked if we interrupted the kernel.
> >   		 */
> >   		kuap_assert_locked();
> > -
> > -		local_irq_disable();
> >   	}
> > +
> > +	/* irqentry_exit expects to be called with interrupts disabled */
> > +	local_irq_disable();
> >   }
> >   
> >   static inline void arch_interrupt_async_enter_prepare(struct pt_regs *regs)
> > --
> > 2.47.3