[PATCH v3 0/5] watchdog: qcom: Support NMI pretimeout warnings

[PATCH v3 0/5] watchdog: qcom: Support NMI pretimeout warnings

[Mayank Rungta]

On ARM64 Qualcomm SoCs, when a system freezes completely due to hard
locked CPUs with standard interrupts disabled, a standard watchdog
pretimeout warning interrupt (bark) fails to fire. To diagnose total
system lockups, we need to transition the Qualcomm hardware watchdog
pretimeout bark interrupt into an NMI (or pseudo-NMI).

Enabling NMI pretimeout handlers within a loadable driver module requires
addressing NMI teardown synchronization, exporting NMI registration
APIs, and ensuring watchdog pretimeout governor dispatch is NMI-safe.

This 5-patch series achieves NMI pretimeout enablement for qcom-wdt:
 1) Enforces that interrupt controllers claiming NMI support must
    implement ->irq_get_irqchip_state(), and synchronizes in-flight NMI
    handlers during teardown (__cleanup_nmi) to prevent use-after-free
    bugs during module unload or driver unbind.
 2) Implements synchronous disable_nmi() to ensure in-flight handlers on
    other CPUs complete before returning.
 3) Exports request_nmi(), free_nmi(), enable_nmi(), disable_nmi(), and
    disable_nmi_nosync() to GPL loadable kernel modules.
 4) Replaces spinlocks in watchdog_notify_pretimeout() with RCU to
    guarantee safe governor execution from NMI context.
 5) Updates qcom-wdt to request its pretimeout bark interrupt as an
    NMI (or pseudo-NMI) with fallback to standard IRQ.

Testing & Verification:
 - Built and tested on ARM64 Qualcomm Snapdragon SoC (Google CoachZ)
   loadable module configurations (CONFIG_QCOM_WDT=m).
 - With GICv3 pseudo-NMI enabled, simulated hard CPU lockups and
   IRQ-disabled hang conditions via lkdtm. Confirmed that qcom-wdt traps
   the watchdog pretimeout bark interrupt as a pseudo-NMI and safely
   executes watchdog_notify_pretimeout() without deadlocks.
 - Verified that runtime transitions between pretimeout governors in
   sysfs execute safely.
 - Booted with pseudo-NMIs disabled and confirmed that qcom-wdt detects
   unsupported NMI and cleanly falls back to standard IRQ.

Signed-off-by: Mayank Rungta <mrungta@google.com>
---
Changes in v3:
- In __cleanup_nmi() (Patch 1), serialized teardown under desc->request_mutex
  matching __free_irq(), preventing concurrent request_nmi() from racing with
  teardown.
- In __cleanup_nmi() (Patch 1), kept desc->action intact across
  __synchronize_hardirq() to prevent lockless NMI handlers on other CPUs
  from encountering a NULL pointer.
- In __cleanup_nmi() (Patch 1), moved irq_proc_update_valid() under
  desc->lock in the second phase so it observes desc->action == NULL.
- In qcom-wdt (Patch 5), enabled the NMI once during probe (enable_nmi())
  to balance IRQF_NO_AUTOEN and removed interrupt enable/disable calls
  from qcom_wdt_start() / qcom_wdt_stop(), avoiding unbalanced enable
  warnings on WDIOC_SETTIMEOUT / WDIOC_SETPRETIMEOUT (Douglas Anderson).
- In qcom-wdt (Patch 5), simplified teardown to unconditionally call
  disable_nmi() and free_nmi(), dropping is_nmi tracking and watchdog_active()
  checks.
- Link to v2: https://lore.kernel.org/r/20260828-qcom-wdt-nmi-series-v2-0-363979fe6b6b@google.com

Changes in v2:
- Added patch to require ->irq_get_irqchip_state() for NMI-capable
  controllers in irq_supports_nmi(), and synchronize in-flight NMI
  handlers via __synchronize_hardirq() in __cleanup_nmi() to prevent
  use-after-free races during teardown.
- Added patch implementing synchronous disable_nmi() wrapping
  disable_irq().
- Exported disable_nmi() alongside other NMI APIs in genirq export patch.
- Re-ordered series to cluster genirq core changes (patches 1-3) followed
  by watchdog core and driver changes (patches 4-5).
- Fixed compiler warnings in watchdog pretimeout RCU patch by adding const
  qualifiers to local governor pointers.
- Dropped `irq_enabled` tracking from struct qcom_wdt in patch 5 since
  watchdog_dev.c strictly pairs ops->start and ops->stop calls.
- Used disable_nmi() in qcom_wdt_disable_irq() for synchronous stop.
- Link to v1: https://lore.kernel.org/r/20260730-qcom-wdt-nmi-series-v1-0-3aa86d162914@google.com

---
Mayank Rungta (5):
      genirq: Synchronize in-flight handlers during NMI teardown
      genirq: Implement synchronous disable_nmi()
      genirq: Export NMI APIs
      watchdog: pretimeout: Protect governor access with RCU for NMI safety
      watchdog: qcom: Register pretimeout interrupt as NMI

 drivers/watchdog/qcom-wdt.c            | 33 +++++++++++++++++++---
 drivers/watchdog/watchdog_pretimeout.c | 45 +++++++++++++++++-------------
 include/linux/interrupt.h              |  1 +
 include/linux/watchdog.h               |  2 +-
 kernel/irq/manage.c                    | 51 ++++++++++++++++++++++++++++++----
 5 files changed, 101 insertions(+), 31 deletions(-)
---
base-commit: 77ae27fd98f3b548797c9f22c10ab5cf1c4ada53
change-id: 20260724-qcom-wdt-nmi-series-06a48da7b415

Best regards,
-- 
Mayank Rungta <mrungta@google.com>

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

[Mayank Rungta]

When freeing an interrupt requested via request_nmi(), __cleanup_nmi()
currently tears down the NMI and frees the irqaction structure without
waiting for in-flight instances of the handler on other CPUs to complete,
which can lead to use-after-free conditions on module unload.

Because NMIs cannot acquire desc->lock or set IRQD_IRQ_INPROGRESS without
risking deadlocks, synchronizing in-flight NMIs on other CPUs during
teardown (__synchronize_hardirq) relies on querying the hardware
controller state via __irq_get_irqchip_state(IRQCHIP_STATE_ACTIVE).

Enforce that any interrupt controller claiming NMI support via
IRQCHIP_SUPPORTS_NMI must implement ->irq_get_irqchip_state(). In
__cleanup_nmi(), serialize teardown under desc->request_mutex, shut down
the line, call __synchronize_hardirq(desc, true) while keeping desc->action
valid to avoid racing with lockless in-flight handlers, and only then clear
desc->action, tear down the NMI configuration, deactivate the interrupt
domain, and update proc status before freeing the action structure.

Also add a WARN(in_interrupt()) check in free_nmi() matching __free_irq()
to prevent freeing NMIs from atomic contexts since synchronization and
resource cleanup can sleep and spin.

Assisted-by: Antigravity:gemini
Signed-off-by: Mayank Rungta <mrungta@google.com>
---
 kernel/irq/manage.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

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);
 
+	scoped_guard(raw_spinlock_irqsave, &desc->lock) {
 		if (!WARN_ON(desc->action == NULL)) {
 			action = desc->action;
 			irq_pm_remove_action(desc, action);
@@ -2046,12 +2063,12 @@ static const void *__cleanup_nmi(unsigned int irq, struct irq_desc *desc)
 		}
 		desc->action = NULL;
 
-		irq_settings_clr_disable_unlazy(desc);
-		irq_shutdown_and_deactivate(desc);
+		irq_nmi_teardown(desc);
+		desc->istate &= ~IRQS_NMI;
+		irq_domain_deactivate_irq(&desc->irq_data);
+		irq_proc_update_valid(desc);
 	}
 
-	irq_proc_update_valid(desc);
-
 	if (action)
 		unregister_handler_proc(irq, action);
 	kfree(action);
@@ -2068,6 +2085,8 @@ const void *free_nmi(unsigned int irq, void *dev_id)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
 
+	WARN(in_interrupt(), "Trying to free NMI %d from IRQ context!\n", irq);
+
 	if (!desc || WARN_ON(!irq_is_nmi(desc)))
 		return NULL;
 

-- 
2.55.0.970.g62bdec98f9-goog

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

[Thomas Gleixner]

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?

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@kernel.org> 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.

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

[Doug Anderson]

Hi,

On Fri, Sep 4, 2026 at 6:38 AM Marc Zyngier <maz@kernel.org> wrote:
>
> On Fri, 04 Sep 2026 10:17:42 +0100,
> Thomas Gleixner <tglx@kernel.org> 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...

The overall goal is to take the watchdog bark interrupt (which is a
GIC Shared Peripheral Interrupt) and promote it to NMI. An example of
that interrupt in the device tree for one Qualcomm board:

interrupts = <GIC_SPI 0 IRQ_TYPE_EDGE_RISING>;

The patches in this series seem to accomplish that. With the current
arm64 pseudo-NMI implemnttion, requesting as NMI just bumps up the
priority of an interrupt to "NMI" level and that works fine on SPIs as
well. On arm64/GIC interrupts are routed to a single CPU anyway, so
requesting it with `IRQF_PERCPU | IRQF_NOBALANCING` didn't seem
absurd.

The biggest problem we had was properly cleaning up the NMI at module
"unload" time. To make that work, we needed a way to synchronize the
NMIs to ensure they were quiescent before the module was unloaded.
That's what this patch is attempting, and is mostly attempting to
respond to Sashiko feedback on earlier patches.

I don't think Mayank is tied to any particular implementation and the
overall goal here is to allow the watchdog bark interrupt to run at an
elevated level so it can produce good backtraces even if all the CPUs
in the system are locked up and the buddy lockup detector doesn't
fire. Is this something you think should be doable? If so, what's the
best way for Mayank to go about doing it?


-Doug

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

[Marc Zyngier]

Hey Doug,

On Fri, 04 Sep 2026 15:34:12 +0100,
Doug Anderson <dianders@chromium.org> wrote:
> 
> Hi,
> 
> On Fri, Sep 4, 2026 at 6:38 AM Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Fri, 04 Sep 2026 10:17:42 +0100,
> > Thomas Gleixner <tglx@kernel.org> wrote:
> > >
> > > 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...
> 
> The overall goal is to take the watchdog bark interrupt (which is a
> GIC Shared Peripheral Interrupt) and promote it to NMI. An example of
> that interrupt in the device tree for one Qualcomm board:
> 
> interrupts = <GIC_SPI 0 IRQ_TYPE_EDGE_RISING>;
> 
> The patches in this series seem to accomplish that. With the current
> arm64 pseudo-NMI implemnttion, requesting as NMI just bumps up the
> priority of an interrupt to "NMI" level and that works fine on SPIs as
> well. On arm64/GIC interrupts are routed to a single CPU anyway, so
> requesting it with `IRQF_PERCPU | IRQF_NOBALANCING` didn't seem
> absurd.

Not absurd, but the implementation as suggested has the potential to
break per-CPU NMIs if you check for the active state on the wrong CPU.

Also, checking for that state doesn't mean the kernel is free of
reference on this interrupt, as you look at the HW state, not the
kernel's.

> The biggest problem we had was properly cleaning up the NMI at module
> "unload" time. To make that work, we needed a way to synchronize the
> NMIs to ensure they were quiescent before the module was unloaded.
> That's what this patch is attempting, and is mostly attempting to
> respond to Sashiko feedback on earlier patches.
>
> I don't think Mayank is tied to any particular implementation and the
> overall goal here is to allow the watchdog bark interrupt to run at an
> elevated level so it can produce good backtraces even if all the CPUs
> in the system are locked up and the buddy lockup detector doesn't
> fire. Is this something you think should be doable? If so, what's the
> best way for Mayank to go about doing it?

The only thing I can think of right now is some equivalent to
IRQD_IRQ_INPROGRESS done in a lockless manner. It doesn't have to be
atomic if you can guarantee that it is checked on the target CPU only,
after having disabled the interrupt.

But this is rather ugly, and I'm pretty sure Thomas will haete this
just as much.

	M.

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

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

[Doug Anderson]

Hi,

On Fri, Sep 4, 2026 at 7:54 AM Marc Zyngier <maz@kernel.org> wrote:
>
> Hey Doug,
>
> On Fri, 04 Sep 2026 15:34:12 +0100,
> Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Fri, Sep 4, 2026 at 6:38 AM Marc Zyngier <maz@kernel.org> wrote:
> > >
> > > On Fri, 04 Sep 2026 10:17:42 +0100,
> > > Thomas Gleixner <tglx@kernel.org> wrote:
> > > >
> > > > 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...
> >
> > The overall goal is to take the watchdog bark interrupt (which is a
> > GIC Shared Peripheral Interrupt) and promote it to NMI. An example of
> > that interrupt in the device tree for one Qualcomm board:
> >
> > interrupts = <GIC_SPI 0 IRQ_TYPE_EDGE_RISING>;
> >
> > The patches in this series seem to accomplish that. With the current
> > arm64 pseudo-NMI implemnttion, requesting as NMI just bumps up the
> > priority of an interrupt to "NMI" level and that works fine on SPIs as
> > well. On arm64/GIC interrupts are routed to a single CPU anyway, so
> > requesting it with `IRQF_PERCPU | IRQF_NOBALANCING` didn't seem
> > absurd.
>
> Not absurd, but the implementation as suggested has the potential to
> break per-CPU NMIs if you check for the active state on the wrong CPU.

Ah, got it. So if someone decides to call the synchronous version of
disable_nmi() for an actual per-CPU NMI then it would go boom. That
makes sense.


> Also, checking for that state doesn't mean the kernel is free of
> reference on this interrupt, as you look at the HW state, not the
> kernel's.

OK, fair enough.


> > The biggest problem we had was properly cleaning up the NMI at module
> > "unload" time. To make that work, we needed a way to synchronize the
> > NMIs to ensure they were quiescent before the module was unloaded.
> > That's what this patch is attempting, and is mostly attempting to
> > respond to Sashiko feedback on earlier patches.
> >
> > I don't think Mayank is tied to any particular implementation and the
> > overall goal here is to allow the watchdog bark interrupt to run at an
> > elevated level so it can produce good backtraces even if all the CPUs
> > in the system are locked up and the buddy lockup detector doesn't
> > fire. Is this something you think should be doable? If so, what's the
> > best way for Mayank to go about doing it?
>
> The only thing I can think of right now is some equivalent to
> IRQD_IRQ_INPROGRESS done in a lockless manner. It doesn't have to be
> atomic if you can guarantee that it is checked on the target CPU only,
> after having disabled the interrupt.
>
> But this is rather ugly, and I'm pretty sure Thomas will haete this
> just as much.

Thanks for your insights! Hmmm, maybe it makes sense for Thomas to
weigh in before attempting this. ...or perhaps he has some other great
idea that would make this work.

-Doug

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

[Thomas Gleixner]

On Fri, Sep 04 2026 at 14:40, Marc Zyngier wrote:
> On Fri, 04 Sep 2026 10:17:42 +0100,
> Thomas Gleixner <tglx@kernel.org> wrote:
>> > +	/*
>> > +	 * 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...

As I suspected.

The driver change is here:

   [1] https://lore.kernel.org/all/20260902-qcom-wdt-nmi-series-v3-5-f3999362a9ea@google.com/

>  @@ -311,10 +321,25 @@ static int qcom_wdt_probe(struct platform_device *pdev)
> 		 if (irq < 0 && irq != -ENXIO)
> 			 return irq;
> 		 if (irq > 0) {
>  -			ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
>  -					       "wdt_bark", &wdt->wdd);
>  -			if (ret)
>  -				return ret;
>  +			wdt->irq = irq;
>  +			irq_flags = IRQF_PERCPU | IRQF_NOBALANCING |
>  +				    IRQF_NO_AUTOEN | IRQF_NO_THREAD;
>  +
>  +			ret = request_nmi(irq, qcom_wdt_isr, irq_flags,
>  +					  "wdt_bark", &wdt->wdd);

Of course this is a regular global interrupt otherwise it couldn't be
requested normally. So this sets IRQF_PERCPU (it has to otherwise
request_nmi() would fail), but that looks like a hack to me.

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

[Doug Anderson]

Hi,

On Fri, Sep 4, 2026 at 8:11 AM Thomas Gleixner <tglx@kernel.org> wrote:
>
> The driver change is here:
>
>    [1] https://lore.kernel.org/all/20260902-qcom-wdt-nmi-series-v3-5-f3999362a9ea@google.com/
>
> >  @@ -311,10 +321,25 @@ static int qcom_wdt_probe(struct platform_device *pdev)
> >                if (irq < 0 && irq != -ENXIO)
> >                        return irq;
> >                if (irq > 0) {
> >  -                    ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
> >  -                                           "wdt_bark", &wdt->wdd);
> >  -                    if (ret)
> >  -                            return ret;
> >  +                    wdt->irq = irq;
> >  +                    irq_flags = IRQF_PERCPU | IRQF_NOBALANCING |
> >  +                                IRQF_NO_AUTOEN | IRQF_NO_THREAD;
> >  +
> >  +                    ret = request_nmi(irq, qcom_wdt_isr, irq_flags,
> >  +                                      "wdt_bark", &wdt->wdd);
>
> Of course this is a regular global interrupt otherwise it couldn't be
> requested normally. So this sets IRQF_PERCPU (it has to otherwise
> request_nmi() would fail), but that looks like a hack to me.

At least on GIC, promoting a regular global interrupt to NMI (or
pseudo-NMI) level is possible, and Mayank's use case here seems
legitimate. Any suggestions you have for making this look less like a
hack are appreciated! :-)

-Doug

[PATCH v3 2/5] genirq: Implement synchronous disable_nmi()

[Mayank Rungta]

Currently, disable_nmi_nosync() is the only interface to disable an NMI
line. Drivers stopping their devices in process context need a synchronous
variant that guarantees any running instance of the NMI handler has
completed on other CPUs before returning.

Implement disable_nmi() by wrapping disable_irq().

Assisted-by: Antigravity:gemini
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Mayank Rungta <mrungta@google.com>
---
 include/linux/interrupt.h |  1 +
 kernel/irq/manage.c       | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 3bf969ad8fe0..f13b3ab3829a 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -243,6 +243,7 @@ DEFINE_LOCK_GUARD_1(disable_irq, int,
 		    disable_irq(*_T->lock), enable_irq(*_T->lock))
 
 extern void disable_nmi_nosync(unsigned int irq);
+extern void disable_nmi(unsigned int irq);
 extern void disable_percpu_nmi(unsigned int irq);
 extern void enable_nmi(unsigned int irq);
 extern void enable_percpu_nmi(unsigned int irq, unsigned int type);
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index a9973b61163a..009633b5b654 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -767,6 +767,21 @@ void disable_nmi_nosync(unsigned int irq)
 	disable_irq_nosync(irq);
 }
 
+/**
+ * disable_nmi - disable an nmi and wait for any pending handlers
+ * @irq: Interrupt to disable
+ *
+ * Disable the selected interrupt line. Disables and enables are nested.
+ *
+ * The interrupt to disable must have been requested through request_nmi.
+ * This function ensures existing instances of the NMI handler have
+ * completed before returning.
+ */
+void disable_nmi(unsigned int irq)
+{
+	disable_irq(irq);
+}
+
 void __enable_irq(struct irq_desc *desc)
 {
 	switch (desc->depth) {

-- 
2.55.0.970.g62bdec98f9-goog

[PATCH v3 3/5] genirq: Export NMI APIs

[Mayank Rungta]

Currently, request_nmi(), free_nmi(), enable_nmi(), disable_nmi_nosync(),
and disable_nmi() are restricted to built-in kernel code because they are
not exported.

Export these symbols with EXPORT_SYMBOL_GPL so loadable kernel modules can
register and manage NMIs.

Assisted-by: Antigravity:gemini
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Mayank Rungta <mrungta@google.com>
---
 kernel/irq/manage.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 009633b5b654..cb6bad666779 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -766,6 +766,7 @@ void disable_nmi_nosync(unsigned int irq)
 {
 	disable_irq_nosync(irq);
 }
+EXPORT_SYMBOL_GPL(disable_nmi_nosync);
 
 /**
  * disable_nmi - disable an nmi and wait for any pending handlers
@@ -781,6 +782,7 @@ void disable_nmi(unsigned int irq)
 {
 	disable_irq(irq);
 }
+EXPORT_SYMBOL_GPL(disable_nmi);
 
 void __enable_irq(struct irq_desc *desc)
 {
@@ -848,6 +850,7 @@ void enable_nmi(unsigned int irq)
 {
 	enable_irq(irq);
 }
+EXPORT_SYMBOL_GPL(enable_nmi);
 
 static int set_irq_wake_real(unsigned int irq, unsigned int on)
 {
@@ -2114,6 +2117,7 @@ const void *free_nmi(unsigned int irq, void *dev_id)
 
 	return __cleanup_nmi(irq, desc);
 }
+EXPORT_SYMBOL_GPL(free_nmi);
 
 /**
  * request_threaded_irq - allocate an interrupt line
@@ -2376,6 +2380,7 @@ int request_nmi(unsigned int irq, irq_handler_t handler,
 
 	return retval;
 }
+EXPORT_SYMBOL_GPL(request_nmi);
 
 void enable_percpu_irq(unsigned int irq, unsigned int type)
 {

-- 
2.55.0.970.g62bdec98f9-goog

Re: [PATCH v3 3/5] genirq: Export NMI APIs

[Thomas Gleixner]

On Wed, Sep 02 2026 at 18:54, Mayank Rungta wrote:
> Currently, request_nmi(), free_nmi(), enable_nmi(), disable_nmi_nosync(),
> and disable_nmi() are restricted to built-in kernel code because they are
> not exported.
>
> Export these symbols with EXPORT_SYMBOL_GPL so loadable kernel modules can
> register and manage NMIs.

I'm not really fond of that.

Funny enough there is already a driver which claims it can be built as
module which uses these interfaces. Oh well...

> Assisted-by: Antigravity:gemini

I'm impressed that AI is required to add four EXPORT_SYMBOL_GPL()
instances.

Thanks,

        tglx

Re: [PATCH v3 3/5] genirq: Export NMI APIs

[Doug Anderson]

Hi,

On Fri, Sep 4, 2026 at 2:29 AM Thomas Gleixner <tglx@kernel.org> wrote:
>
> On Wed, Sep 02 2026 at 18:54, Mayank Rungta wrote:
> > Currently, request_nmi(), free_nmi(), enable_nmi(), disable_nmi_nosync(),
> > and disable_nmi() are restricted to built-in kernel code because they are
> > not exported.
> >
> > Export these symbols with EXPORT_SYMBOL_GPL so loadable kernel modules can
> > register and manage NMIs.
>
> I'm not really fond of that.

Is this to be taken a NAK on the whole idea, then?

The last patch in the series [1] wants to promote a HW watchdog bark
interrupt to NMI. This seems like a pretty useful thing to do and an
appropriate use of NMI, but the HW watchdog driver is a kernel module.
Do you have another suggestion for solving that?

[1] https://lore.kernel.org/all/20260902-qcom-wdt-nmi-series-v3-5-f3999362a9ea@google.com/


> Funny enough there is already a driver which claims it can be built as
> module which uses these interfaces. Oh well...
>
> > Assisted-by: Antigravity:gemini
>
> I'm impressed that AI is required to add four EXPORT_SYMBOL_GPL()
> instances.

Hmm, I guess it's hard to figure out the guidance here. I think Mayank
is only adding this tag to try match what's documented in
`coding-assistants.rst`. If you've got an AI helping you with a
series, it's pretty much touching all the patches. I guess you're
suggesting that, even if AI touched it, trivial patches should remove
the tag?

Re: [PATCH v3 3/5] genirq: Export NMI APIs

[Thomas Gleixner]

On Fri, Sep 04 2026 at 07:07, Doug Anderson wrote:
> On Fri, Sep 4, 2026 at 2:29 AM Thomas Gleixner <tglx@kernel.org> wrote:
>> On Wed, Sep 02 2026 at 18:54, Mayank Rungta wrote:
>> > Currently, request_nmi(), free_nmi(), enable_nmi(), disable_nmi_nosync(),
>> > and disable_nmi() are restricted to built-in kernel code because they are
>> > not exported.
>> >
>> > Export these symbols with EXPORT_SYMBOL_GPL so loadable kernel modules can
>> > register and manage NMIs.
>>
>> I'm not really fond of that.
>
> Is this to be taken a NAK on the whole idea, then?
>
> The last patch in the series [1] wants to promote a HW watchdog bark
> interrupt to NMI. This seems like a pretty useful thing to do and an
> appropriate use of NMI, but the HW watchdog driver is a kernel module.
> Do you have another suggestion for solving that?

I understand the intent and why it might be a good idea, but letting
random modules convert interrupts into NMIs is a scary thought.

We can export it in a restricted way though to limit the exposure and
force people to go through a thorough review of these use cases.

That could be a subsystem restricted export or an explicit restriction
via EXPORT..FOR_MODULES().

>> Funny enough there is already a driver which claims it can be built as
>> module which uses these interfaces. Oh well...
>>
>> > Assisted-by: Antigravity:gemini
>>
>> I'm impressed that AI is required to add four EXPORT_SYMBOL_GPL()
>> instances.
>
> Hmm, I guess it's hard to figure out the guidance here. I think Mayank
> is only adding this tag to try match what's documented in
> `coding-assistants.rst`. If you've got an AI helping you with a
> series, it's pretty much touching all the patches. I guess you're
> suggesting that, even if AI touched it, trivial patches should remove
> the tag?

I really have no strong opinion about it. But it just amused me and I
couldn't help my self from commenting.

Thanks,

        tglx

Re: [PATCH v3 3/5] genirq: Export NMI APIs

[Mayank Rungta]

On Fri, Sep 4, 2026 at 8:05 AM Thomas Gleixner <tglx@kernel.org> wrote:
>
> On Fri, Sep 04 2026 at 07:07, Doug Anderson wrote:
> > On Fri, Sep 4, 2026 at 2:29 AM Thomas Gleixner <tglx@kernel.org> wrote:
> >> On Wed, Sep 02 2026 at 18:54, Mayank Rungta wrote:
> >> > Currently, request_nmi(), free_nmi(), enable_nmi(), disable_nmi_nosync(),
> >> > and disable_nmi() are restricted to built-in kernel code because they are
> >> > not exported.
> >> >
> >> > Export these symbols with EXPORT_SYMBOL_GPL so loadable kernel modules can
> >> > register and manage NMIs.
> >>
> >> I'm not really fond of that.
> >
> > Is this to be taken a NAK on the whole idea, then?
> >
> > The last patch in the series [1] wants to promote a HW watchdog bark
> > interrupt to NMI. This seems like a pretty useful thing to do and an
> > appropriate use of NMI, but the HW watchdog driver is a kernel module.
> > Do you have another suggestion for solving that?
>
> I understand the intent and why it might be a good idea, but letting
> random modules convert interrupts into NMIs is a scary thought.
>
> We can export it in a restricted way though to limit the exposure and
> force people to go through a thorough review of these use cases.
>
> That could be a subsystem restricted export or an explicit restriction
> via EXPORT..FOR_MODULES().
>

That makes sense. Do you have a preference between
`EXPORT_SYMBOL_NS_GPL(..., WATCHDOG)` and
`EXPORT_SYMBOL_FOR_MODULES(..., "qcom_wdt")`? Happy to go with
whichever you prefer for v4 (which I'll hold off on sending until we
reach an agreement on the NMI teardown discussion in patch 1).

> >> Funny enough there is already a driver which claims it can be built as
> >> module which uses these interfaces. Oh well...
> >>
> >> > Assisted-by: Antigravity:gemini
> >>
> >> I'm impressed that AI is required to add four EXPORT_SYMBOL_GPL()
> >> instances.
> >
> > Hmm, I guess it's hard to figure out the guidance here. I think Mayank
> > is only adding this tag to try match what's documented in
> > `coding-assistants.rst`. If you've got an AI helping you with a
> > series, it's pretty much touching all the patches. I guess you're
> > suggesting that, even if AI touched it, trivial patches should remove
> > the tag?
>
> I really have no strong opinion about it. But it just amused me and I
> couldn't help my self from commenting.
>

Actually, I was able to add the first 4 exports by myself, it was the
5th one I really needed help with! :)

Jokes aside, since the whole series went through AI review and
different maintainers have different expectations around AI
attribution right now, I conservatively added the tag across all
patches. I'll drop `Assisted-by:` from trivial patches like this in
the next revision.

Thanks,
Mayank

[PATCH v3 4/5] watchdog: pretimeout: Protect governor access with RCU for NMI safety

[Mayank Rungta]

When watchdog pretimeout interrupts are configured as NMIs (or
pseudo-NMIs), pretimeout handlers execute in NMI context. Accessing
the watchdog governor via spinlocks in watchdog_notify_pretimeout() is
unsafe in NMI context and can cause deadlocks if an NMI arrives while a
spinlock is held.

Protect governor assignment and dereference with RCU instead of
spinlocks, allowing lockless, NMI-safe governor notifications while
preserving safe runtime governor switching.

Assisted-by: Antigravity:gemini
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Mayank Rungta <mrungta@google.com>
---
 drivers/watchdog/watchdog_pretimeout.c | 45 +++++++++++++++++++---------------
 include/linux/watchdog.h               |  2 +-
 2 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/watchdog/watchdog_pretimeout.c b/drivers/watchdog/watchdog_pretimeout.c
index 02e09b9e396d..7fb586c77403 100644
--- a/drivers/watchdog/watchdog_pretimeout.c
+++ b/drivers/watchdog/watchdog_pretimeout.c
@@ -4,6 +4,7 @@
  */
 
 #include <linux/list.h>
+#include <linux/rcupdate.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/string.h>
@@ -67,12 +68,14 @@ int watchdog_pretimeout_available_governors_get(char *buf)
 
 int watchdog_pretimeout_governor_get(struct watchdog_device *wdd, char *buf)
 {
+	const struct watchdog_governor *gov;
 	int count = 0;
 
-	spin_lock_irq(&pretimeout_lock);
-	if (wdd->gov)
-		count = sysfs_emit(buf, "%s\n", wdd->gov->name);
-	spin_unlock_irq(&pretimeout_lock);
+	rcu_read_lock();
+	gov = rcu_dereference(wdd->gov);
+	if (gov)
+		count = sysfs_emit(buf, "%s\n", gov->name);
+	rcu_read_unlock();
 
 	return count;
 }
@@ -91,7 +94,7 @@ int watchdog_pretimeout_governor_set(struct watchdog_device *wdd,
 	}
 
 	spin_lock_irq(&pretimeout_lock);
-	wdd->gov = priv->gov;
+	rcu_assign_pointer(wdd->gov, priv->gov);
 	spin_unlock_irq(&pretimeout_lock);
 
 	mutex_unlock(&governor_lock);
@@ -101,16 +104,13 @@ int watchdog_pretimeout_governor_set(struct watchdog_device *wdd,
 
 void watchdog_notify_pretimeout(struct watchdog_device *wdd)
 {
-	unsigned long flags;
+	const struct watchdog_governor *gov;
 
-	spin_lock_irqsave(&pretimeout_lock, flags);
-	if (!wdd->gov) {
-		spin_unlock_irqrestore(&pretimeout_lock, flags);
-		return;
-	}
-
-	wdd->gov->pretimeout(wdd);
-	spin_unlock_irqrestore(&pretimeout_lock, flags);
+	rcu_read_lock();
+	gov = rcu_dereference(wdd->gov);
+	if (gov)
+		gov->pretimeout(wdd);
+	rcu_read_unlock();
 }
 EXPORT_SYMBOL_GPL(watchdog_notify_pretimeout);
 
@@ -140,8 +140,8 @@ int watchdog_register_governor(struct watchdog_governor *gov)
 		default_gov = gov;
 
 		list_for_each_entry(p, &pretimeout_list, entry)
-			if (!p->wdd->gov)
-				p->wdd->gov = default_gov;
+			if (!rcu_access_pointer(p->wdd->gov))
+				rcu_assign_pointer(p->wdd->gov, default_gov);
 		spin_unlock_irq(&pretimeout_lock);
 	}
 
@@ -170,11 +170,14 @@ void watchdog_unregister_governor(struct watchdog_governor *gov)
 	if (default_gov == gov)
 		default_gov = NULL;
 	list_for_each_entry(p, &pretimeout_list, entry)
-		if (p->wdd->gov == gov)
-			p->wdd->gov = default_gov;
+		if (rcu_dereference_protected(p->wdd->gov,
+					      lockdep_is_held(&pretimeout_lock)) == gov)
+			rcu_assign_pointer(p->wdd->gov, default_gov);
 	spin_unlock_irq(&pretimeout_lock);
 
 	mutex_unlock(&governor_lock);
+
+	synchronize_rcu();
 }
 EXPORT_SYMBOL(watchdog_unregister_governor);
 
@@ -192,7 +195,7 @@ int watchdog_register_pretimeout(struct watchdog_device *wdd)
 	spin_lock_irq(&pretimeout_lock);
 	list_add(&p->entry, &pretimeout_list);
 	p->wdd = wdd;
-	wdd->gov = default_gov;
+	rcu_assign_pointer(wdd->gov, default_gov);
 	spin_unlock_irq(&pretimeout_lock);
 
 	return 0;
@@ -206,7 +209,7 @@ void watchdog_unregister_pretimeout(struct watchdog_device *wdd)
 		return;
 
 	spin_lock_irq(&pretimeout_lock);
-	wdd->gov = NULL;
+	rcu_assign_pointer(wdd->gov, NULL);
 
 	list_for_each_entry_safe(p, t, &pretimeout_list, entry) {
 		if (p->wdd == wdd) {
@@ -216,4 +219,6 @@ void watchdog_unregister_pretimeout(struct watchdog_device *wdd)
 		}
 	}
 	spin_unlock_irq(&pretimeout_lock);
+
+	synchronize_rcu();
 }
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 29cd03686154..5a3c35968cc8 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -105,7 +105,7 @@ struct watchdog_device {
 	const struct attribute_group **groups;
 	const struct watchdog_info *info;
 	const struct watchdog_ops *ops;
-	const struct watchdog_governor *gov;
+	const struct watchdog_governor __rcu *gov;
 	unsigned int bootstatus;
 	unsigned int timeout;
 	unsigned int pretimeout;

-- 
2.55.0.970.g62bdec98f9-goog

[PATCH v3 5/5] watchdog: qcom: Register pretimeout interrupt as NMI

[Mayank Rungta]

When a system is completely unresponsive due to an interrupt storm or
deadlocked CPU cores with standard interrupts disabled, a standard
watchdog pretimeout bark interrupt will fail to execute, preventing the
pretimeout governor from capturing CPU backtraces before the hardware
reset bite.

Attempt to request the Qualcomm watchdog pretimeout bark interrupt as an
NMI (or pseudo-NMI) using request_nmi(). If NMI registration is not
supported on the platform (e.g. pseudo-NMIs are disabled), gracefully
fall back to a standard interrupt via devm_request_irq().

When NMI is used, enable the NMI during probe to balance IRQF_NO_AUTOEN
and register a devres cleanup action to disable and free the NMI upon
driver unbind.

Assisted-by: Antigravity:gemini
Signed-off-by: Mayank Rungta <mrungta@google.com>
---
 drivers/watchdog/qcom-wdt.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
index 4eb1bf979012..2849dcb1f717 100644
--- a/drivers/watchdog/qcom-wdt.c
+++ b/drivers/watchdog/qcom-wdt.c
@@ -52,6 +52,7 @@ struct qcom_wdt {
 	unsigned long		rate;
 	void __iomem		*base;
 	const u32		*layout;
+	int			irq;
 };
 
 static void __iomem *wdt_addr(struct qcom_wdt *wdt, enum wdt_reg reg)
@@ -74,6 +75,14 @@ static irqreturn_t qcom_wdt_isr(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
+static void qcom_wdt_free_nmi(void *arg)
+{
+	struct qcom_wdt *wdt = arg;
+
+	disable_nmi(wdt->irq);
+	free_nmi(wdt->irq, &wdt->wdd);
+}
+
 static int qcom_wdt_start(struct watchdog_device *wdd)
 {
 	struct qcom_wdt *wdt = to_qcom_wdt(wdd);
@@ -256,6 +265,7 @@ static int qcom_wdt_probe(struct platform_device *pdev)
 	u32 percpu_offset;
 	int irq, ret;
 	struct clk *clk;
+	unsigned long irq_flags;
 
 	data = of_device_get_match_data(dev);
 	if (!data) {
@@ -311,10 +321,25 @@ static int qcom_wdt_probe(struct platform_device *pdev)
 		if (irq < 0 && irq != -ENXIO)
 			return irq;
 		if (irq > 0) {
-			ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
-					       "wdt_bark", &wdt->wdd);
-			if (ret)
-				return ret;
+			wdt->irq = irq;
+			irq_flags = IRQF_PERCPU | IRQF_NOBALANCING |
+				    IRQF_NO_AUTOEN | IRQF_NO_THREAD;
+
+			ret = request_nmi(irq, qcom_wdt_isr, irq_flags,
+					  "wdt_bark", &wdt->wdd);
+			if (ret) {
+				/* Fallback to normal interrupt if NMI not supported */
+				ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
+						       "wdt_bark", &wdt->wdd);
+				if (ret)
+					return ret;
+			} else {
+				enable_nmi(irq);
+				ret = devm_add_action_or_reset(dev, qcom_wdt_free_nmi,
+							       wdt);
+				if (ret)
+					return ret;
+			}
 
 			wdt->wdd.info = &qcom_wdt_pt_info;
 			wdt->wdd.pretimeout = 1;

-- 
2.55.0.970.g62bdec98f9-goog

Re: [PATCH v3 5/5] watchdog: qcom: Register pretimeout interrupt as NMI

[Konrad Dybcio]

On 9/3/26 3:54 AM, Mayank Rungta wrote:
> When a system is completely unresponsive due to an interrupt storm or
> deadlocked CPU cores with standard interrupts disabled, a standard
> watchdog pretimeout bark interrupt will fail to execute, preventing the
> pretimeout governor from capturing CPU backtraces before the hardware
> reset bite.

[...]

> -			ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
> -					       "wdt_bark", &wdt->wdd);
> -			if (ret)
> -				return ret;
> +			wdt->irq = irq;
> +			irq_flags = IRQF_PERCPU | IRQF_NOBALANCING |
> +				    IRQF_NO_AUTOEN | IRQF_NO_THREAD;
> +
> +			ret = request_nmi(irq, qcom_wdt_isr, irq_flags,
> +					  "wdt_bark", &wdt->wdd);
> +			if (ret) {
> +				/* Fallback to normal interrupt if NMI not supported */
> +				ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
> +						       "wdt_bark", &wdt->wdd);

GPT noticed this IRQ is requested but never enabled (NO_AUTOEN)

Konrad

Re: [PATCH v3 5/5] watchdog: qcom: Register pretimeout interrupt as NMI

[Mayank Rungta]

Hi,

> > When a system is completely unresponsive due to an interrupt storm or
> > deadlocked CPU cores with standard interrupts disabled, a standard
> > watchdog pretimeout bark interrupt will fail to execute, preventing the
> > pretimeout governor from capturing CPU backtraces before the hardware
> > reset bite.
>
> [...]
>
> > -                     ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
> > -                                            "wdt_bark", &wdt->wdd);
> > -                     if (ret)
> > -                             return ret;
> > +                     wdt->irq = irq;
> > +                     irq_flags = IRQF_PERCPU | IRQF_NOBALANCING |
> > +                                 IRQF_NO_AUTOEN | IRQF_NO_THREAD;
> > +
> > +                     ret = request_nmi(irq, qcom_wdt_isr, irq_flags,
> > +                                       "wdt_bark", &wdt->wdd);
> > +                     if (ret) {
> > +                             /* Fallback to normal interrupt if NMI not supported */
> > +                             ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
> > +                                                    "wdt_bark", &wdt->wdd);
>
> GPT noticed this IRQ is requested but never enabled (NO_AUTOEN)

For the NMI path: request_nmi() requires IRQF_NO_AUTOEN. To balance
this, enable_nmi(irq) is called immediately upon successful
registration in the probe function:

For the IRQ fallback: devm_request_irq() passes 0 (not irq_flags). In
__setup_irq(), when IRQF_NO_AUTOEN is absent, the core calls
irq_startup(), automatically enabling the interrupt line as it has
always done in mainline.

Both paths are therefore enabled upon probe.

Thanks,
Mayank

Re: [PATCH v3 5/5] watchdog: qcom: Register pretimeout interrupt as NMI

[Konrad Dybcio]

On 9/3/26 10:58 PM, Mayank Rungta wrote:
> Hi,
> 
>>> When a system is completely unresponsive due to an interrupt storm or
>>> deadlocked CPU cores with standard interrupts disabled, a standard
>>> watchdog pretimeout bark interrupt will fail to execute, preventing the
>>> pretimeout governor from capturing CPU backtraces before the hardware
>>> reset bite.
>>
>> [...]
>>
>>> -                     ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
>>> -                                            "wdt_bark", &wdt->wdd);
>>> -                     if (ret)
>>> -                             return ret;
>>> +                     wdt->irq = irq;
>>> +                     irq_flags = IRQF_PERCPU | IRQF_NOBALANCING |
>>> +                                 IRQF_NO_AUTOEN | IRQF_NO_THREAD;
>>> +
>>> +                     ret = request_nmi(irq, qcom_wdt_isr, irq_flags,
>>> +                                       "wdt_bark", &wdt->wdd);
>>> +                     if (ret) {
>>> +                             /* Fallback to normal interrupt if NMI not supported */
>>> +                             ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
>>> +                                                    "wdt_bark", &wdt->wdd);
>>
>> GPT noticed this IRQ is requested but never enabled (NO_AUTOEN)
> 
> For the NMI path: request_nmi() requires IRQF_NO_AUTOEN. To balance
> this, enable_nmi(irq) is called immediately upon successful
> registration in the probe function:
> 
> For the IRQ fallback: devm_request_irq() passes 0 (not irq_flags). In

Yes, you're right. Both GPT and I were fooled!

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v3 5/5] watchdog: qcom: Register pretimeout interrupt as NMI

[Doug Anderson]

Hi,

On Thu, Sep 3, 2026 at 1:58 PM Mayank Rungta <mrungta@google.com> wrote:
>
> Hi,
>
> > > When a system is completely unresponsive due to an interrupt storm or
> > > deadlocked CPU cores with standard interrupts disabled, a standard
> > > watchdog pretimeout bark interrupt will fail to execute, preventing the
> > > pretimeout governor from capturing CPU backtraces before the hardware
> > > reset bite.
> >
> > [...]
> >
> > > -                     ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
> > > -                                            "wdt_bark", &wdt->wdd);
> > > -                     if (ret)
> > > -                             return ret;
> > > +                     wdt->irq = irq;
> > > +                     irq_flags = IRQF_PERCPU | IRQF_NOBALANCING |
> > > +                                 IRQF_NO_AUTOEN | IRQF_NO_THREAD;
> > > +
> > > +                     ret = request_nmi(irq, qcom_wdt_isr, irq_flags,
> > > +                                       "wdt_bark", &wdt->wdd);
> > > +                     if (ret) {
> > > +                             /* Fallback to normal interrupt if NMI not supported */
> > > +                             ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
> > > +                                                    "wdt_bark", &wdt->wdd);
> >
> > GPT noticed this IRQ is requested but never enabled (NO_AUTOEN)
>
> For the NMI path: request_nmi() requires IRQF_NO_AUTOEN. To balance
> this, enable_nmi(irq) is called immediately upon successful
> registration in the probe function:
>
> For the IRQ fallback: devm_request_irq() passes 0 (not irq_flags). In
> __setup_irq(), when IRQF_NO_AUTOEN is absent, the core calls
> irq_startup(), automatically enabling the interrupt line as it has
> always done in mainline.
>
> Both paths are therefore enabled upon probe.

I wonder if it would make sense to either:

1. Rename the variable to 'nmi_flags'?

2. Get rid of the variable and just directly pass the flags to the request_nmi()

That would avoid the confusion. While I agree that your code now is
correct, I can understand why GPT (and humans) would get confused...


-Doug

Re: [PATCH v3 5/5] watchdog: qcom: Register pretimeout interrupt as NMI

[Mayank Rungta]

On Fri, Sep 4, 2026 at 7:38 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Thu, Sep 3, 2026 at 1:58 PM Mayank Rungta <mrungta@google.com> wrote:
> >
> > Hi,
> >
> > > > When a system is completely unresponsive due to an interrupt storm or
> > > > deadlocked CPU cores with standard interrupts disabled, a standard
> > > > watchdog pretimeout bark interrupt will fail to execute, preventing the
> > > > pretimeout governor from capturing CPU backtraces before the hardware
> > > > reset bite.
> > >
> > > [...]
> > >
> > > > -                     ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
> > > > -                                            "wdt_bark", &wdt->wdd);
> > > > -                     if (ret)
> > > > -                             return ret;
> > > > +                     wdt->irq = irq;
> > > > +                     irq_flags = IRQF_PERCPU | IRQF_NOBALANCING |
> > > > +                                 IRQF_NO_AUTOEN | IRQF_NO_THREAD;
> > > > +
> > > > +                     ret = request_nmi(irq, qcom_wdt_isr, irq_flags,
> > > > +                                       "wdt_bark", &wdt->wdd);
> > > > +                     if (ret) {
> > > > +                             /* Fallback to normal interrupt if NMI not supported */
> > > > +                             ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
> > > > +                                                    "wdt_bark", &wdt->wdd);
> > >
> > > GPT noticed this IRQ is requested but never enabled (NO_AUTOEN)
> >
> > For the NMI path: request_nmi() requires IRQF_NO_AUTOEN. To balance
> > this, enable_nmi(irq) is called immediately upon successful
> > registration in the probe function:
> >
> > For the IRQ fallback: devm_request_irq() passes 0 (not irq_flags). In
> > __setup_irq(), when IRQF_NO_AUTOEN is absent, the core calls
> > irq_startup(), automatically enabling the interrupt line as it has
> > always done in mainline.
> >
> > Both paths are therefore enabled upon probe.
>
> I wonder if it would make sense to either:
>
> 1. Rename the variable to 'nmi_flags'?
>
> 2. Get rid of the variable and just directly pass the flags to the request_nmi()
>
> That would avoid the confusion. While I agree that your code now is
> correct, I can understand why GPT (and humans) would get confused...
>
>
> -Doug

Agreed, renaming `irq_flags` to `nmi_flags` is a nice readability improvement.

However, since the way NMIs are requested is likely going to change
based on the ongoing discussion in patch 1, specifically regarding the
`IRQF_PERCPU` flag, I'll hold off on making changes here until we have
an agreement in place.

Thanks,
Mayank