* [PATCH v2 0/5] watchdog: qcom: Support NMI pretimeout warnings
@ 2026-08-29 0:59 Mayank Rungta
2026-08-29 0:59 ` [PATCH v2 1/5] genirq: Synchronize in-flight handlers during NMI teardown Mayank Rungta
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Mayank Rungta @ 2026-08-29 0:59 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Thomas Gleixner, Radu Rendec
Cc: linux-watchdog, linux-kernel, linux-arm-msm, Kirill A. Shutemov,
Douglas Anderson, 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 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
Note on Sashiko review:
Sashiko AI review on v1 noted a pre-existing issue regarding the lack of
desc->request_mutex acquisition in free_nmi() and the request_nmi() error
path.
This pre-existing behavior is orthogonal to this series and not affected
by exporting NMI symbols or in-flight NMI handler synchronization. To keep
this series focused on module export, request_mutex handling is left unchanged.
---
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 | 52 +++++++++++++++++++++++++++++++---
drivers/watchdog/watchdog_pretimeout.c | 45 ++++++++++++++++-------------
include/linux/interrupt.h | 1 +
include/linux/watchdog.h | 2 +-
kernel/irq/manage.c | 45 +++++++++++++++++++++++++----
5 files changed, 115 insertions(+), 30 deletions(-)
---
base-commit: 77ae27fd98f3b548797c9f22c10ab5cf1c4ada53
change-id: 20260724-qcom-wdt-nmi-series-06a48da7b415
Best regards,
--
Mayank Rungta <mrungta@google.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/5] genirq: Synchronize in-flight handlers during NMI teardown
2026-08-29 0:59 [PATCH v2 0/5] watchdog: qcom: Support NMI pretimeout warnings Mayank Rungta
@ 2026-08-29 0:59 ` Mayank Rungta
2026-08-29 1:14 ` sashiko-bot
2026-08-31 18:16 ` Doug Anderson
2026-08-29 0:59 ` [PATCH v2 2/5] genirq: Implement synchronous disable_nmi() Mayank Rungta
` (3 subsequent siblings)
4 siblings, 2 replies; 15+ messages in thread
From: Mayank Rungta @ 2026-08-29 0:59 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Thomas Gleixner, Radu Rendec
Cc: linux-watchdog, linux-kernel, linux-arm-msm, Kirill A. Shutemov,
Douglas Anderson, 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(), shut down the line, call __synchronize_hardirq(desc, true)
to wait for in-flight handlers, and only then tear down the NMI
configuration and deactivate the interrupt domain 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 | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 2fbff2618a1e..61384925b921 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;
}
@@ -2035,10 +2043,6 @@ static const void *__cleanup_nmi(unsigned int irq, struct irq_desc *desc)
const char *devname = NULL;
scoped_guard(raw_spinlock_irqsave, &desc->lock) {
- irq_nmi_teardown(desc);
-
- desc->istate &= ~IRQS_NMI;
-
if (!WARN_ON(desc->action == NULL)) {
action = desc->action;
irq_pm_remove_action(desc, action);
@@ -2047,11 +2051,20 @@ 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_shutdown(desc);
}
irq_proc_update_valid(desc);
+ /* Ensure all in-flight NMI handlers on other CPUs complete before freeing action */
+ __synchronize_hardirq(desc, true);
+
+ scoped_guard(raw_spinlock_irqsave, &desc->lock) {
+ irq_nmi_teardown(desc);
+ desc->istate &= ~IRQS_NMI;
+ irq_domain_deactivate_irq(&desc->irq_data);
+ }
+
if (action)
unregister_handler_proc(irq, action);
kfree(action);
@@ -2068,6 +2081,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.897.gb25b4bd76c-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/5] genirq: Implement synchronous disable_nmi()
2026-08-29 0:59 [PATCH v2 0/5] watchdog: qcom: Support NMI pretimeout warnings Mayank Rungta
2026-08-29 0:59 ` [PATCH v2 1/5] genirq: Synchronize in-flight handlers during NMI teardown Mayank Rungta
@ 2026-08-29 0:59 ` Mayank Rungta
2026-08-29 1:18 ` sashiko-bot
2026-08-31 18:16 ` Doug Anderson
2026-08-29 0:59 ` [PATCH v2 3/5] genirq: Export NMI APIs Mayank Rungta
` (2 subsequent siblings)
4 siblings, 2 replies; 15+ messages in thread
From: Mayank Rungta @ 2026-08-29 0:59 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Thomas Gleixner, Radu Rendec
Cc: linux-watchdog, linux-kernel, linux-arm-msm, Kirill A. Shutemov,
Douglas Anderson, 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
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 61384925b921..40013aa94481 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.897.gb25b4bd76c-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 3/5] genirq: Export NMI APIs
2026-08-29 0:59 [PATCH v2 0/5] watchdog: qcom: Support NMI pretimeout warnings Mayank Rungta
2026-08-29 0:59 ` [PATCH v2 1/5] genirq: Synchronize in-flight handlers during NMI teardown Mayank Rungta
2026-08-29 0:59 ` [PATCH v2 2/5] genirq: Implement synchronous disable_nmi() Mayank Rungta
@ 2026-08-29 0:59 ` Mayank Rungta
2026-08-29 1:15 ` sashiko-bot
2026-08-31 18:17 ` Doug Anderson
2026-08-29 0:59 ` [PATCH v2 4/5] watchdog: pretimeout: Protect governor access with RCU for NMI safety Mayank Rungta
2026-08-29 0:59 ` [PATCH v2 5/5] watchdog: qcom: Register pretimeout interrupt as NMI Mayank Rungta
4 siblings, 2 replies; 15+ messages in thread
From: Mayank Rungta @ 2026-08-29 0:59 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Thomas Gleixner, Radu Rendec
Cc: linux-watchdog, linux-kernel, linux-arm-msm, Kirill A. Shutemov,
Douglas Anderson, 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
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 40013aa94481..ded8cc33381c 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)
{
@@ -2110,6 +2113,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
@@ -2372,6 +2376,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.897.gb25b4bd76c-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 4/5] watchdog: pretimeout: Protect governor access with RCU for NMI safety
2026-08-29 0:59 [PATCH v2 0/5] watchdog: qcom: Support NMI pretimeout warnings Mayank Rungta
` (2 preceding siblings ...)
2026-08-29 0:59 ` [PATCH v2 3/5] genirq: Export NMI APIs Mayank Rungta
@ 2026-08-29 0:59 ` Mayank Rungta
2026-08-31 18:17 ` Doug Anderson
2026-08-29 0:59 ` [PATCH v2 5/5] watchdog: qcom: Register pretimeout interrupt as NMI Mayank Rungta
4 siblings, 1 reply; 15+ messages in thread
From: Mayank Rungta @ 2026-08-29 0:59 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Thomas Gleixner, Radu Rendec
Cc: linux-watchdog, linux-kernel, linux-arm-msm, Kirill A. Shutemov,
Douglas Anderson, 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
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.897.gb25b4bd76c-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 5/5] watchdog: qcom: Register pretimeout interrupt as NMI
2026-08-29 0:59 [PATCH v2 0/5] watchdog: qcom: Support NMI pretimeout warnings Mayank Rungta
` (3 preceding siblings ...)
2026-08-29 0:59 ` [PATCH v2 4/5] watchdog: pretimeout: Protect governor access with RCU for NMI safety Mayank Rungta
@ 2026-08-29 0:59 ` Mayank Rungta
2026-08-29 1:15 ` sashiko-bot
2026-08-31 18:17 ` Doug Anderson
4 siblings, 2 replies; 15+ messages in thread
From: Mayank Rungta @ 2026-08-29 0:59 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Thomas Gleixner, Radu Rendec
Cc: linux-watchdog, linux-kernel, linux-arm-msm, Kirill A. Shutemov,
Douglas Anderson, 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, register a devres cleanup action to free the NMI upon
driver unbind, and use enable_nmi() and disable_nmi() during start and
stop.
Assisted-by: Antigravity:gemini
Signed-off-by: Mayank Rungta <mrungta@google.com>
---
drivers/watchdog/qcom-wdt.c | 52 +++++++++++++++++++++++++++++++++++++++++----
1 file changed, 48 insertions(+), 4 deletions(-)
diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
index 4eb1bf979012..82eaf8786426 100644
--- a/drivers/watchdog/qcom-wdt.c
+++ b/drivers/watchdog/qcom-wdt.c
@@ -52,6 +52,8 @@ struct qcom_wdt {
unsigned long rate;
void __iomem *base;
const u32 *layout;
+ int irq;
+ bool is_nmi;
};
static void __iomem *wdt_addr(struct qcom_wdt *wdt, enum wdt_reg reg)
@@ -74,6 +76,28 @@ static irqreturn_t qcom_wdt_isr(int irq, void *arg)
return IRQ_HANDLED;
}
+static void qcom_wdt_enable_irq(struct qcom_wdt *wdt)
+{
+ if (wdt->is_nmi && wdt->irq > 0)
+ enable_nmi(wdt->irq);
+}
+
+static void qcom_wdt_disable_irq(struct qcom_wdt *wdt)
+{
+ if (wdt->is_nmi && wdt->irq > 0)
+ disable_nmi(wdt->irq);
+}
+
+static void qcom_wdt_free_nmi(void *arg)
+{
+ struct qcom_wdt *wdt = arg;
+
+ if (watchdog_active(&wdt->wdd))
+ qcom_wdt_disable_irq(wdt);
+
+ free_nmi(wdt->irq, &wdt->wdd);
+}
+
static int qcom_wdt_start(struct watchdog_device *wdd)
{
struct qcom_wdt *wdt = to_qcom_wdt(wdd);
@@ -84,6 +108,8 @@ static int qcom_wdt_start(struct watchdog_device *wdd)
writel(bark * wdt->rate, wdt_addr(wdt, WDT_BARK_TIME));
writel(wdd->timeout * wdt->rate, wdt_addr(wdt, WDT_BITE_TIME));
writel(QCOM_WDT_ENABLE, wdt_addr(wdt, WDT_EN));
+
+ qcom_wdt_enable_irq(wdt);
return 0;
}
@@ -91,6 +117,8 @@ static int qcom_wdt_stop(struct watchdog_device *wdd)
{
struct qcom_wdt *wdt = to_qcom_wdt(wdd);
+ qcom_wdt_disable_irq(wdt);
+
writel(0, wdt_addr(wdt, WDT_EN));
return 0;
}
@@ -256,6 +284,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 +340,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 {
+ wdt->is_nmi = true;
+ 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.897.gb25b4bd76c-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/5] genirq: Synchronize in-flight handlers during NMI teardown
2026-08-29 0:59 ` [PATCH v2 1/5] genirq: Synchronize in-flight handlers during NMI teardown Mayank Rungta
@ 2026-08-29 1:14 ` sashiko-bot
2026-08-31 18:16 ` Doug Anderson
1 sibling, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-08-29 1:14 UTC (permalink / raw)
To: Mayank Rungta; +Cc: linux-watchdog
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Splitting the atomic NMI teardown in `__cleanup_nmi()` without holding `desc->request_mutex` introduces a critical race condition with concurrent `request_nmi()` calls.
Pre-existing issues:
- [Critical] Setting `desc->action = NULL` before synchronizing in-flight NMIs creates a fatal race window where concurrent NMIs dereference a NULL pointer.
--
commit c7c9e712a226c1915918d074d948b4cf1845990f
Author: Mayank Rungta <mrungta@google.com>
genirq: Synchronize in-flight handlers during NMI teardown
This patch modifies __cleanup_nmi() to wait for in-flight handlers using
__synchronize_hardirq() before tearing down the NMI configuration and
deactivating the domain. It also enforces that interrupt controllers claiming
NMI support must implement ->irq_get_irqchip_state().
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 2fbff2618a1e2..61384925b921a 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
[ ... ]
> @@ -2047,11 +2051,20 @@ 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_shutdown(desc);
> }
>
> irq_proc_update_valid(desc);
>
> + /* Ensure all in-flight NMI handlers on other CPUs complete before freeing action */
> + __synchronize_hardirq(desc, true);
[Severity: Critical]
This is a pre-existing issue, but does setting desc->action to NULL before
synchronizing in-flight NMIs expose a fatal race window?
If an NMI is delivered to a CPU after desc->action is set to NULL, but before
the hardware line is disabled (or if an NMI is already in flight), the NMI
handler could unconditionally dereference the nullified action pointer.
Unlike normal IRQ handlers, handle_fasteoi_nmi() assumes the action is
always valid:
kernel/irq/chip.c:handle_fasteoi_nmi() {
...
/*
* NMIs cannot be shared, there is only one action.
*/
res = action->handler(irq, action->dev_id);
...
}
The attempt to fix the use-after-free via __synchronize_hardirq() appears
ineffective here because the pointer is nullified before the synchronization
barrier.
> +
> + scoped_guard(raw_spinlock_irqsave, &desc->lock) {
> + irq_nmi_teardown(desc);
> + desc->istate &= ~IRQS_NMI;
> + irq_domain_deactivate_irq(&desc->irq_data);
> + }
[Severity: High]
Does splitting the atomic NMI teardown without holding desc->request_mutex
introduce a race condition with concurrent request_nmi() calls?
Since the lock is yielded to wait in __synchronize_hardirq(), a concurrent
request_nmi() could execute. Unlike free_irq(), __cleanup_nmi() does not hold
desc->request_mutex. This means request_nmi() could acquire the mutex,
configure the new NMI, and attach the action.
When __cleanup_nmi() resumes in this block, would it blindly execute
irq_nmi_teardown() and irq_domain_deactivate_irq(), inadvertently destroying
the newly requested hardware state and leaving the new NMI broken?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260828-qcom-wdt-nmi-series-v2-0-363979fe6b6b@google.com?part=1
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 5/5] watchdog: qcom: Register pretimeout interrupt as NMI
2026-08-29 0:59 ` [PATCH v2 5/5] watchdog: qcom: Register pretimeout interrupt as NMI Mayank Rungta
@ 2026-08-29 1:15 ` sashiko-bot
2026-08-31 18:17 ` Doug Anderson
1 sibling, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-08-29 1:15 UTC (permalink / raw)
To: Mayank Rungta; +Cc: linux-watchdog
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Medium] Calling WDIOC_SETTIMEOUT or WDIOC_SETPRETIMEOUT on an active watchdog triggers an 'Unbalanced enable for IRQ' kernel warning.
- [Medium] Unbinding the driver or encountering a probe failure when the hardware watchdog is already running from the bootloader triggers a WARN_ON in free_nmi().
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but setting the timeout or pretimeout on a stopped watchdog unconditionally starts the hardware watchdog without updating the core state, eventually leading to an unexpected system reboot.
--
commit 90c184f58c3a63ad67e01452209c8185a52db049
Author: Mayank Rungta <mrungta@google.com>
watchdog: qcom: Register pretimeout interrupt as NMI
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.
This commit updates the driver to attempt requesting the pretimeout
interrupt as an NMI. It adds cleanup actions and handles enabling and
disabling the NMI during start and stop.
> diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
> index 4eb1bf9790125..82eaf8786426f 100644
> --- a/drivers/watchdog/qcom-wdt.c
> +++ b/drivers/watchdog/qcom-wdt.c
> @@ -52,6 +52,8 @@ struct qcom_wdt {
> unsigned long rate;
> void __iomem *base;
> const u32 *layout;
> + int irq;
> + bool is_nmi;
> };
>
> static void __iomem *wdt_addr(struct qcom_wdt *wdt, enum wdt_reg reg)
[ ... ]
> @@ -74,6 +76,28 @@ static irqreturn_t qcom_wdt_isr(int irq, void *arg)
> return IRQ_HANDLED;
> }
>
> +static void qcom_wdt_enable_irq(struct qcom_wdt *wdt)
> +{
> + if (wdt->is_nmi && wdt->irq > 0)
> + enable_nmi(wdt->irq);
> +}
> +
> +static void qcom_wdt_disable_irq(struct qcom_wdt *wdt)
> +{
> + if (wdt->is_nmi && wdt->irq > 0)
> + disable_nmi(wdt->irq);
> +}
> +
> +static void qcom_wdt_free_nmi(void *arg)
> +{
> + struct qcom_wdt *wdt = arg;
> +
> + if (watchdog_active(&wdt->wdd))
> + qcom_wdt_disable_irq(wdt);
> +
> + free_nmi(wdt->irq, &wdt->wdd);
> +}
[Severity: Medium]
Will this fail to disable the NMI if the watchdog was started by the
bootloader?
If the hardware watchdog is already running from the bootloader,
qcom_wdt_probe() calls qcom_wdt_start(), which enables the NMI and sets the
WDOG_HW_RUNNING flag, but the WDOG_ACTIVE flag remains unset:
drivers/watchdog/qcom-wdt.c:qcom_wdt_probe() {
...
if (qcom_wdt_is_running(&wdt->wdd)) {
qcom_wdt_start(&wdt->wdd);
set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
}
...
}
If the driver is later unbound or fails probe, qcom_wdt_free_nmi() only
checks watchdog_active(), which evaluates WDOG_ACTIVE.
Because the disable step is skipped, will free_nmi() be called while the NMI
is still enabled, resulting in a WARN_ON stack trace during cleanup?
> static int qcom_wdt_start(struct watchdog_device *wdd)
> {
> struct qcom_wdt *wdt = to_qcom_wdt(wdd);
> @@ -84,6 +108,8 @@ static int qcom_wdt_start(struct watchdog_device *wdd)
> writel(bark * wdt->rate, wdt_addr(wdt, WDT_BARK_TIME));
> writel(wdd->timeout * wdt->rate, wdt_addr(wdt, WDT_BITE_TIME));
> writel(QCOM_WDT_ENABLE, wdt_addr(wdt, WDT_EN));
> +
> + qcom_wdt_enable_irq(wdt);
> return 0;
> }
[Severity: Medium]
Does this code trigger an unbalanced enable warning when the timeout is changed?
When a user application changes the timeout via ioctl(WDIOC_SETTIMEOUT), the
core watchdog subsystem invokes qcom_wdt_set_timeout(), which unconditionally
calls qcom_wdt_start():
drivers/watchdog/qcom-wdt.c:qcom_wdt_set_timeout() {
...
wdd->timeout = timeout;
return qcom_wdt_start(wdd);
}
Because qcom_wdt_start() then blindly calls qcom_wdt_enable_irq() without
checking if the NMI is already enabled, will this trigger an 'Unbalanced
enable for IRQ' warning from the core IRQ subsystem?
[Severity: High]
This is a pre-existing issue, but does calling qcom_wdt_start() here
unconditionally start the hardware even if it was stopped by the user?
If a user application stops the watchdog and then sets the timeout,
qcom_wdt_set_timeout() blindly invokes qcom_wdt_start() as shown above.
This physically starts the hardware countdown without updating the core state.
Because the core state remains disabled, subsequent keepalive pings are
silently ignored by the core. Will this lead to an un-pinged hardware
watchdog that eventually unexpectedly reboots the system?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260828-qcom-wdt-nmi-series-v2-0-363979fe6b6b@google.com?part=5
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/5] genirq: Export NMI APIs
2026-08-29 0:59 ` [PATCH v2 3/5] genirq: Export NMI APIs Mayank Rungta
@ 2026-08-29 1:15 ` sashiko-bot
2026-08-31 18:17 ` Doug Anderson
1 sibling, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-08-29 1:15 UTC (permalink / raw)
To: Mayank Rungta; +Cc: linux-watchdog
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Exporting `free_nmi()` and `disable_nmi()` allows loadable modules to manage NMIs, but these APIs fundamentally fail to synchronize with in-flight NMI handlers, introducing a race condition that leads to kernel panics on module unload.
Pre-existing issues:
- [High] `__cleanup_nmi()` fails to acquire `desc->request_mutex` when tearing down an NMI. This allows concurrent IRQ request/free operations to race, corrupting IRQ resource state and bypassing hardware resource allocation.
- [High] Concurrent NMI delivery during `free_nmi()` teardown leads to a NULL pointer dereference in `handle_fasteoi_nmi()` because `desc->action` is cleared before `__synchronize_hardirq()` waits for in-flight NMIs.
- [Medium] `free_nmi()` ignores the provided `dev_id` parameter, bypassing ownership validation and allowing any module to inadvertently or maliciously unregister an NMI owned by another subsystem.
--
commit cf44d4cc82f985321cdf77cbbbd3469b85de0426
Author: Mayank Rungta <mrungta@google.com>
genirq: Export NMI APIs
This commit exports request_nmi(), free_nmi(), enable_nmi(),
disable_nmi_nosync(), and disable_nmi() with EXPORT_SYMBOL_GPL to allow
loadable kernel modules to register and manage NMIs.
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 40013aa94481e..ded8cc33381cc 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
[ ... ]
> @@ -2110,6 +2113,7 @@ const void *free_nmi(unsigned int irq, void *dev_id)
>
> return __cleanup_nmi(irq, desc);
> }
> +EXPORT_SYMBOL_GPL(free_nmi);
[Severity: High]
Is it safe to export free_nmi() and disable_nmi() given they can introduce
a race condition that leads to kernel panics on module unload?
When a module unloads, free_nmi() calls __cleanup_nmi(), which uses
__synchronize_hardirq() to wait for handlers on other CPUs to finish.
__synchronize_hardirq() checks the IRQD_IRQ_INPROGRESS flag:
kernel/irq/manage.c:__synchronize_hardirq() {
...
while (irqd_irq_inprogress(&desc->irq_data))
cpu_relax();
...
}
However, NMI handlers like handle_fasteoi_nmi() bypass standard IRQ locking
and never set IRQD_IRQ_INPROGRESS:
kernel/irq/chip.c:handle_fasteoi_nmi() {
...
res = action->handler(irq, action->dev_id);
...
}
Consequently, __synchronize_hardirq() returns immediately. The module's text
segment could then be unmapped while the NMI handler is still executing,
leading to a page fault.
[Severity: High]
This is a pre-existing issue, but can concurrent NMI delivery during
free_nmi() teardown lead to a NULL pointer dereference in
handle_fasteoi_nmi()?
During free_nmi(), __cleanup_nmi() sets desc->action = NULL and drops
desc->lock before calling __synchronize_hardirq():
kernel/irq/manage.c:__cleanup_nmi() {
...
desc->action = NULL;
irq_settings_clr_disable_unlazy(desc);
irq_shutdown(desc);
...
/* Ensure all in-flight NMI handlers on other CPUs complete before freeing action */
__synchronize_hardirq(desc, true);
...
}
If an NMI fires concurrently, the dispatcher handle_fasteoi_nmi() executes
on another CPU without taking desc->lock. It reads desc->action and
unconditionally dereferences it without checking for NULL:
kernel/irq/chip.c:handle_fasteoi_nmi() {
...
struct irqaction *action = desc->action;
...
res = action->handler(irq, action->dev_id);
...
}
[Severity: High]
This isn't a bug introduced by this patch, but it appears __cleanup_nmi()
fails to acquire desc->request_mutex when tearing down an NMI.
__cleanup_nmi() drops desc->lock and performs teardown (including
irq_release_resources()) without holding desc->request_mutex:
kernel/irq/manage.c:__cleanup_nmi() {
...
irq_release_resources(desc);
...
}
Conversely, __setup_irq() relies exclusively on desc->request_mutex for
serialization. If __cleanup_nmi() runs concurrently with __setup_irq() (e.g.
during a rapid module reload or concurrent requests), they can race.
__setup_irq() may read a non-NULL desc->action and skip resource allocation:
kernel/irq/manage.c:__setup_irq() {
...
if (!desc->action) {
ret = irq_request_resources(desc);
...
}
Could this lead to bypassed hardware resource allocation or corrupted hardware
state for IRQ resources?
[Severity: Medium]
This is a pre-existing issue, but I noticed free_nmi() ignores the provided
dev_id parameter.
The free_nmi() API accepts a dev_id parameter, but delegates to
__cleanup_nmi() which drops the token entirely:
kernel/irq/manage.c:free_nmi() {
...
return __cleanup_nmi(irq, desc);
}
Inside __cleanup_nmi(), the action is unconditionally unlinked and torn
down without checking if its dev_id matches the caller's cookie:
kernel/irq/manage.c:__cleanup_nmi() {
...
if (!WARN_ON(desc->action == NULL)) {
action = desc->action;
irq_pm_remove_action(desc, action);
...
}
Unlike free_irq(), which verifies action->dev_id == dev_id, this lacks
ownership validation. Could a buggy loadable module unilaterally destroy
another component's NMI by passing an incorrect IRQ number or a
NULL cookie to free_nmi()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260828-qcom-wdt-nmi-series-v2-0-363979fe6b6b@google.com?part=3
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/5] genirq: Implement synchronous disable_nmi()
2026-08-29 0:59 ` [PATCH v2 2/5] genirq: Implement synchronous disable_nmi() Mayank Rungta
@ 2026-08-29 1:18 ` sashiko-bot
2026-08-31 18:16 ` Doug Anderson
1 sibling, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-08-29 1:18 UTC (permalink / raw)
To: Mayank Rungta; +Cc: linux-watchdog
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Because NMIs do not set the IRQD_IRQ_INPROGRESS flag, __synchronize_hardirq() skips its cpu_relax() yield loop and tightly polls the hardware state, causing severe bus and lock contention.
--
commit 3611450f7345bc1611ae9b9599bb6e44ba2b6477
Author: Mayank Rungta <mrungta@google.com>
genirq: Implement synchronous disable_nmi()
This patch introduces a synchronous disable_nmi() interface. Drivers can use
this in process context to disable an NMI line and ensure running handlers
have completed on other CPUs by wrapping disable_irq().
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index 3bf969ad8fe07..f13b3ab3829af 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
[ ... ]
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 61384925b921a..40013aa94481e 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);
[Severity: High]
This is a pre-existing issue, but does wrapping disable_irq() here create a
severe hardware polling loop when synchronizing NMIs?
Because disable_irq() delegates to __synchronize_hardirq(), it expects to wait
for the IRQD_IRQ_INPROGRESS flag using a cpu_relax() loop:
kernel/irq/manage.c:__synchronize_hardirq() {
...
while (irqd_irq_inprogress(&desc->irq_data))
cpu_relax();
/* Ok, that indicated we're done: double-check carefully. */
guard(raw_spinlock_irqsave)(&desc->lock);
inprogress = irqd_irq_inprogress(&desc->irq_data);
...
}
However, NMI flow handlers (such as handle_fasteoi_nmi() in kernel/irq/chip.c)
execute locklessly and never set the IRQD_IRQ_INPROGRESS flag.
Since the flag is false, the cpu_relax() yield is skipped entirely. When an NMI
is executing concurrently, __synchronize_hardirq() will repeatedly acquire
desc->lock and query the hardware state via __irq_get_irqchip_state().
Will this cause a tight MMIO polling loop on the IRQ controller without
yielding, generating severe bus contention and potentially delaying the NMI
handler it is waiting for?
> +}
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260828-qcom-wdt-nmi-series-v2-0-363979fe6b6b@google.com?part=2
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/5] genirq: Synchronize in-flight handlers during NMI teardown
2026-08-29 0:59 ` [PATCH v2 1/5] genirq: Synchronize in-flight handlers during NMI teardown Mayank Rungta
2026-08-29 1:14 ` sashiko-bot
@ 2026-08-31 18:16 ` Doug Anderson
1 sibling, 0 replies; 15+ messages in thread
From: Doug Anderson @ 2026-08-31 18:16 UTC (permalink / raw)
To: Mayank Rungta
Cc: Wim Van Sebroeck, Guenter Roeck, Thomas Gleixner, Radu Rendec,
linux-watchdog, linux-kernel, linux-arm-msm, Kirill A. Shutemov
Hi,
On Fri, Aug 28, 2026 at 6:00 PM Mayank Rungta <mrungta@google.com> wrote:
>
> 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(), shut down the line, call __synchronize_hardirq(desc, true)
> to wait for in-flight handlers, and only then tear down the NMI
> configuration and deactivate the interrupt domain 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 | 25 ++++++++++++++++++++-----
> 1 file changed, 20 insertions(+), 5 deletions(-)
FWIW, Sashiko's feedback [1] again seems relevant, so probably a v3 is
worthwhile.
[1] https://lore.kernel.org/all/20260829011449.87AF11F000E9@smtp.kernel.org/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/5] genirq: Implement synchronous disable_nmi()
2026-08-29 0:59 ` [PATCH v2 2/5] genirq: Implement synchronous disable_nmi() Mayank Rungta
2026-08-29 1:18 ` sashiko-bot
@ 2026-08-31 18:16 ` Doug Anderson
1 sibling, 0 replies; 15+ messages in thread
From: Doug Anderson @ 2026-08-31 18:16 UTC (permalink / raw)
To: Mayank Rungta
Cc: Wim Van Sebroeck, Guenter Roeck, Thomas Gleixner, Radu Rendec,
linux-watchdog, linux-kernel, linux-arm-msm, Kirill A. Shutemov
Hi,
On Fri, Aug 28, 2026 at 6:00 PM Mayank Rungta <mrungta@google.com> wrote:
>
> 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
> Signed-off-by: Mayank Rungta <mrungta@google.com>
> ---
> include/linux/interrupt.h | 1 +
> kernel/irq/manage.c | 15 +++++++++++++++
> 2 files changed, 16 insertions(+)
This looks right to me.
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Sashiko had feedback [1] where it was worried about tightly looping
doing HW polling. To me, that doesn't seem like a big deal since there
shouldn't be many NMI users and NMIs shouldn't be freed often. Perhaps
Thomas has different thoughts.
[1] https://lore.kernel.org/all/20260829011853.0B9A41F000E9@smtp.kernel.org/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/5] genirq: Export NMI APIs
2026-08-29 0:59 ` [PATCH v2 3/5] genirq: Export NMI APIs Mayank Rungta
2026-08-29 1:15 ` sashiko-bot
@ 2026-08-31 18:17 ` Doug Anderson
1 sibling, 0 replies; 15+ messages in thread
From: Doug Anderson @ 2026-08-31 18:17 UTC (permalink / raw)
To: Mayank Rungta
Cc: Wim Van Sebroeck, Guenter Roeck, Thomas Gleixner, Radu Rendec,
linux-watchdog, linux-kernel, linux-arm-msm, Kirill A. Shutemov
Hi,
On Fri, Aug 28, 2026 at 6:00 PM Mayank Rungta <mrungta@google.com> 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.
>
> Assisted-by: Antigravity:gemini
> Signed-off-by: Mayank Rungta <mrungta@google.com>
> ---
> kernel/irq/manage.c | 5 +++++
> 1 file changed, 5 insertions(+)
This is no different than v1, so you probably should have carried my
Reviewed-by tag:
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Sashiko has feedback [1], but it seems to be mostly regugitating the
same feedback it had on patch #1. It also mentions that "`free_nmi()`
ignores the provided `dev_id` parameter", but (to me) that doesn't
seem like a big deal and is certainly not new.
[1] https://lore.kernel.org/all/20260829011550.ECF091F000E9@smtp.kernel.org/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 4/5] watchdog: pretimeout: Protect governor access with RCU for NMI safety
2026-08-29 0:59 ` [PATCH v2 4/5] watchdog: pretimeout: Protect governor access with RCU for NMI safety Mayank Rungta
@ 2026-08-31 18:17 ` Doug Anderson
0 siblings, 0 replies; 15+ messages in thread
From: Doug Anderson @ 2026-08-31 18:17 UTC (permalink / raw)
To: Mayank Rungta
Cc: Wim Van Sebroeck, Guenter Roeck, Thomas Gleixner, Radu Rendec,
linux-watchdog, linux-kernel, linux-arm-msm, Kirill A. Shutemov
Hi,
On Fri, Aug 28, 2026 at 6:00 PM Mayank Rungta <mrungta@google.com> wrote:
>
> 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
> 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(-)
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 5/5] watchdog: qcom: Register pretimeout interrupt as NMI
2026-08-29 0:59 ` [PATCH v2 5/5] watchdog: qcom: Register pretimeout interrupt as NMI Mayank Rungta
2026-08-29 1:15 ` sashiko-bot
@ 2026-08-31 18:17 ` Doug Anderson
1 sibling, 0 replies; 15+ messages in thread
From: Doug Anderson @ 2026-08-31 18:17 UTC (permalink / raw)
To: Mayank Rungta
Cc: Wim Van Sebroeck, Guenter Roeck, Thomas Gleixner, Radu Rendec,
linux-watchdog, linux-kernel, linux-arm-msm, Kirill A. Shutemov
Hi,
On Fri, Aug 28, 2026 at 6:00 PM Mayank Rungta <mrungta@google.com> 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.
>
> 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, register a devres cleanup action to free the NMI upon
> driver unbind, and use enable_nmi() and disable_nmi() during start and
> stop.
>
> Assisted-by: Antigravity:gemini
> Signed-off-by: Mayank Rungta <mrungta@google.com>
> ---
> drivers/watchdog/qcom-wdt.c | 52 +++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 48 insertions(+), 4 deletions(-)
Sashiko had some feedback here [1]. I think the right resolution is to
not try to enable/disable the NMI when the watchdog is
enabled/disabled, just like we do for normal IRQs.
For NMIs:
* We're forced to start out in "disabled" state (NOAUTOEN).
* We're forced to disable before we free or we get a WARN_ON.
None of those are really problems, though. After we request the NMI,
there's no problem enabling it right away, is there? Then, if we do
that, we can make qcom_wdt_free_nmi() unconditionally call
disable_nmi() before calling free_nmi(). Now everything is nice and
symmetric and simpler.
[1] https://lore.kernel.org/all/20260829011545.3873A1F000E9@smtp.kernel.org/
-Doug
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-08-31 18:17 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-29 0:59 [PATCH v2 0/5] watchdog: qcom: Support NMI pretimeout warnings Mayank Rungta
2026-08-29 0:59 ` [PATCH v2 1/5] genirq: Synchronize in-flight handlers during NMI teardown Mayank Rungta
2026-08-29 1:14 ` sashiko-bot
2026-08-31 18:16 ` Doug Anderson
2026-08-29 0:59 ` [PATCH v2 2/5] genirq: Implement synchronous disable_nmi() Mayank Rungta
2026-08-29 1:18 ` sashiko-bot
2026-08-31 18:16 ` Doug Anderson
2026-08-29 0:59 ` [PATCH v2 3/5] genirq: Export NMI APIs Mayank Rungta
2026-08-29 1:15 ` sashiko-bot
2026-08-31 18:17 ` Doug Anderson
2026-08-29 0:59 ` [PATCH v2 4/5] watchdog: pretimeout: Protect governor access with RCU for NMI safety Mayank Rungta
2026-08-31 18:17 ` Doug Anderson
2026-08-29 0:59 ` [PATCH v2 5/5] watchdog: qcom: Register pretimeout interrupt as NMI Mayank Rungta
2026-08-29 1:15 ` sashiko-bot
2026-08-31 18:17 ` Doug Anderson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.