Re: [PATCH v3 1/5] genirq: Synchronize in-flight handlers during NMI teardown

[Marc Zyngier]

On Fri, 04 Sep 2026 10:17:42 +0100,
Thomas Gleixner <tglx@xxxxxxxxxx> wrote:
> 
> On Wed, Sep 02 2026 at 18:54, Mayank Rungta wrote:
> > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> > index 2fbff2618a1e..a9973b61163a 100644
> > --- a/kernel/irq/manage.c
> > +++ b/kernel/irq/manage.c
> > @@ -1379,6 +1379,14 @@ static bool irq_supports_nmi(struct irq_desc *desc)
> >  	if (d->chip->irq_bus_lock || d->chip->irq_bus_sync_unlock)
> >  		return false;
> >  
> > +	/*
> > +	 * NMIs cannot set IRQD_IRQ_INPROGRESS because they cannot acquire
> > +	 * spinlocks. Synchronous disable and teardown require querying the
> > +	 * hardware state via ->irq_get_irqchip_state().
> > +	 */
> > +	if (!d->chip->irq_get_irqchip_state)
> > +		return false;
> > +
> >  	return d->chip->flags & IRQCHIP_SUPPORTS_NMI;
> >  }
> >  
> > @@ -2034,11 +2042,20 @@ static const void *__cleanup_nmi(unsigned int irq, struct irq_desc *desc)
> >  	struct irqaction *action = NULL;
> >  	const char *devname = NULL;
> >  
> > +	guard(mutex)(&desc->request_mutex);
> > +
> >  	scoped_guard(raw_spinlock_irqsave, &desc->lock) {
> > -		irq_nmi_teardown(desc);
> > +		irq_settings_clr_disable_unlazy(desc);
> > +		irq_shutdown(desc);
> > +	}
> >  
> > -		desc->istate &= ~IRQS_NMI;
> > +	/*
> > +	 * Ensure all in-flight NMI handlers on other CPUs complete before
> > +	 * clearing desc->action or tearing down NMI state.
> > +	 */
> > +	__synchronize_hardirq(desc, true);
> 
> NMIs are strictly per CPU interrupts. So how is this supposed to work
> correctly when looking at irqchip_state(ACTIVE) ?
> 
> Marc?

I have no idea what this is trying to achieve.

This can only work for a global interrupt, not for a CPU-private
interrupt, since in general you can't observe the state of the
interrupt on another CPU.

My guess is that the OP is trying to trigger an NMI using a global
interrupt, which we never intended to be supported. That's not to say
that it cannot be supported, but this patch seems to be breaking the
core use case...

	M.

-- 
Jazz isn't dead. It just smells funny.