mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] genirq/PM: Re-enable chained interrupts after hibernation
@ 2026-09-05 15:27 Zhu Ling
  2026-09-08 20:07 ` Thomas Gleixner
  0 siblings, 1 reply; 3+ messages in thread
From: Zhu Ling @ 2026-09-05 15:27 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Zhu Ling, Rafael J. Wysocki, Radu Rendec, Pavel Machek,
	Len Brown, Greg Kroah-Hartman, Danilo Krummrich, linux-kernel,
	linux-pm, driver-core

Chained interrupts remain enabled across suspend because they can be part
of a wakeup path. Consequently, suspend_device_irq() does not set
IRQS_SUSPENDED for them, and they do not have IRQF_FORCE_RESUME accounting.

After restoring a hibernation image, the interrupt controller can have a
chained parent interrupt masked while the restored descriptor still marks
it started and enabled. For example, the restore kernel may initialize the
controller without probing the driver that installs the chained handler.
resume_irq() skips the parent in this state, leaving interrupts from the
devices below it blocked.

Pass the hibernation restore context from dpm_resume_noirq() to the IRQ PM
code and force active chained interrupts through the existing force-resume
path. Set the disabled and masked state before enabling the interrupt so
that cached descriptor state cannot suppress the irqchip callback. Calling
irq_startup() alone on an already started and enabled interrupt can skip
the hardware access when the descriptor also records it as unmasked.

Leave disabled and unstarted chained interrupts alone. Keep the existing
behavior for ordinary resume, hibernation thaw and recovery, and the early
syscore resume pass.

Fixes: 0a0c5168df27 ("PM: Introduce functions for suspending and resuming device interrupts")
Assisted-by: LLM
Signed-off-by: Zhu Ling <zhuling0805@qq.com>
---
Found by inspection and reproduced with a fake irq_chip in UML KUnit.
The reproducer installs a chained handler, runs the IRQ suspend path, and
sets the simulated hardware mask without changing the descriptor. On the
unpatched kernel, resume leaves the hardware masked with no enable/unmask
callback and no interrupt delivery. With the fix, delivery resumes.

Validation:
  - UML KUnit: 24 cases passed; the same suite failed 6 cases without the
    fix. Covers irq_enable and irq_unmask callbacks, disabled depth,
    shutdown, existing force-resume behavior and pending interrupt resend.
  - x86_64 defconfig (GCC 13.3) and arm64 defconfig (Clang 18.1):
    kernel/irq/pm.o and drivers/base/power/main.o built with W=1.
  - x86_64 tinyconfig (PM disabled): kernel/irq/chip.o built with W=1.
  - checkpatch.pl --strict and git apply --check passed.

No physical-board hibernation test has been performed. Sparse was attempted
but rejected by checker-valid.sh because the installed sparse 0.6.4 lacks
__typeof_unqual__ support.

AI assistance: an LLM inspected the supplied patch and local IRQ/PM code,
prepared the fix and changelog, and wrote and ran the fake-irqchip tests.

 drivers/base/power/main.c |  2 +-
 include/linux/interrupt.h |  2 +-
 kernel/irq/pm.c           | 29 ++++++++++++++++++++---------
 3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index e130da428141..370ce28fb7b3 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -952,7 +952,7 @@ void dpm_resume_noirq(pm_message_t state)
 {
 	dpm_noirq_resume_devices(state);
 
-	resume_device_irqs();
+	resume_device_irqs(state.event == PM_EVENT_RESTORE);
 	device_wakeup_disarm_wake_irqs();
 }
 
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 3bf969ad8fe0..1cf8808adf7f 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -253,7 +253,7 @@ extern int irq_inject_interrupt(unsigned int irq);
 
 /* The following three functions are for the core kernel use only. */
 extern void suspend_device_irqs(void);
-extern void resume_device_irqs(void);
+void resume_device_irqs(bool restore);
 extern void rearm_wake_irq(unsigned int irq);
 
 /**
diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
index 99ff65466d87..325b5dd4495e 100644
--- a/kernel/irq/pm.c
+++ b/kernel/irq/pm.c
@@ -141,7 +141,7 @@ void suspend_device_irqs(void)
 	}
 }
 
-static void resume_irq(struct irq_desc *desc)
+static void resume_irq(struct irq_desc *desc, bool restore)
 {
 	struct irq_data *irqd = &desc->irq_data;
 
@@ -160,9 +160,18 @@ static void resume_irq(struct irq_desc *desc)
 	if (desc->istate & IRQS_SUSPENDED)
 		goto resume;
 
-	/* Force resume the interrupt? */
-	if (!desc->force_resume_depth)
+	if (restore && irq_desc_is_chained(desc)) {
+		/*
+		 * Chained interrupts are not suspended to preserve wakeup paths.
+		 * After restoring a hibernation image, the controller may have
+		 * them disabled despite the restored descriptor state. Force
+		 * active chained interrupts through the hardware enable path.
+		 */
+		if (!irqd_is_started(irqd) || irqd_irq_disabled(irqd))
+			return;
+	} else if (!desc->force_resume_depth) {
 		return;
+	}
 
 	/* Pretend that it got disabled ! */
 	desc->depth++;
@@ -173,7 +182,7 @@ static void resume_irq(struct irq_desc *desc)
 	__enable_irq(desc);
 }
 
-static void resume_irqs(bool want_early)
+static void resume_irqs(bool want_early, bool restore)
 {
 	struct irq_desc *desc;
 	int irq;
@@ -187,7 +196,7 @@ static void resume_irqs(bool want_early)
 			continue;
 
 		guard(raw_spinlock_irqsave)(&desc->lock);
-		resume_irq(desc);
+		resume_irq(desc, restore);
 	}
 }
 
@@ -217,7 +226,7 @@ void rearm_wake_irq(unsigned int irq)
  */
 static void irq_pm_syscore_resume(void *data)
 {
-	resume_irqs(true);
+	resume_irqs(true, false);
 }
 
 static const struct syscore_ops irq_pm_syscore_ops = {
@@ -238,12 +247,14 @@ device_initcall(irq_pm_init_ops);
 
 /**
  * resume_device_irqs - enable interrupt lines disabled by suspend_device_irqs()
+ * @restore: Whether memory has been restored from a hibernation image
  *
  * Enable all non-%IRQF_EARLY_RESUME interrupt lines previously
  * disabled by suspend_device_irqs() that have the IRQS_SUSPENDED flag
- * set as well as those with %IRQF_FORCE_RESUME.
+ * set as well as those with %IRQF_FORCE_RESUME. Also re-enable active chained
+ * interrupts when restoring a hibernation image.
  */
-void resume_device_irqs(void)
+void resume_device_irqs(bool restore)
 {
-	resume_irqs(false);
+	resume_irqs(false, restore);
 }

base-commit: bc35965f6940a9bf834d54187b6088b8eb09206d
-- 
2.43.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] genirq/PM: Re-enable chained interrupts after hibernation
  2026-09-05 15:27 [PATCH] genirq/PM: Re-enable chained interrupts after hibernation Zhu Ling
@ 2026-09-08 20:07 ` Thomas Gleixner
  2026-10-24 14:46   ` Zhu Ling
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Gleixner @ 2026-09-08 20:07 UTC (permalink / raw)
  To: Zhu Ling
  Cc: Zhu Ling, Rafael J. Wysocki, Radu Rendec, Pavel Machek,
	Len Brown, Greg Kroah-Hartman, Danilo Krummrich, linux-kernel,
	linux-pm, driver-core

On Sat, Sep 05 2026 at 23:27, Zhu Ling wrote:
> Chained interrupts remain enabled across suspend because they can be part
> of a wakeup path. Consequently, suspend_device_irq() does not set
> IRQS_SUSPENDED for them, and they do not have IRQF_FORCE_RESUME accounting.
>
> After restoring a hibernation image, the interrupt controller can have a
> chained parent interrupt masked while the restored descriptor still marks
> it started and enabled. For example, the restore kernel may initialize the
> controller without probing the driver that installs the chained handler.
> resume_irq() skips the parent in this state, leaving interrupts from the
> devices below it blocked.
>
> Pass the hibernation restore context from dpm_resume_noirq() to the IRQ PM
> code and force active chained interrupts through the existing force-resume
> path. Set the disabled and masked state before enabling the interrupt so
> that cached descriptor state cannot suppress the irqchip callback. Calling
> irq_startup() alone on an already started and enabled interrupt can skip
> the hardware access when the descriptor also records it as unmasked.
>
> Leave disabled and unstarted chained interrupts alone. Keep the existing
> behavior for ordinary resume, hibernation thaw and recovery, and the early
> syscore resume pass.
>
> Fixes: 0a0c5168df27 ("PM: Introduce functions for suspending and resuming device interrupts")

This fixes nothing than a theoretical problem hallucinated by a LLM and
we don't fix hallucinations.

If that would be a real world problem then this change would cure the
symptom and not the root cause.

Thanks,

        tglx


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] genirq/PM: Re-enable chained interrupts after hibernation
  2026-09-08 20:07 ` Thomas Gleixner
@ 2026-10-24 14:46   ` Zhu Ling
  0 siblings, 0 replies; 3+ messages in thread
From: Zhu Ling @ 2026-10-24 14:46 UTC (permalink / raw)
  To: tglx
  Cc: Zhu Ling, Marc Zyngier, Radu Rendec, Rafael J . Wysocki,
	linux-arm-kernel, linux-pm, linux-kernel

Hi Thomas,

Thank you for the review. I am withdrawing the previous genirq/PM
proposal. Please do not apply it: forcing chained IRQs through generic
resume treats the symptom rather than restoring controller state.

I have since reproduced the failure on the Phytium E2000Q platform
using upstream base 17bb0e2f6c8043f70a693a5f3797471cbed354c8.
GPIOLIB is disabled and no vendor GPIO driver is involved. An independent
module registers either an ordinary or chained handler on an idle
GICv3 SPI (INTID 189) and injects a real GIC pending event before and
after hibernation. It has no PM callbacks or automatic recovery.

On separate fresh boots, the ordinary IRQ delivers both injections.
The chained IRQ delivers only the first. After real S4, its descriptor
still reports started=1, disabled=0 and sw_masked=0, but the hardware
enable bit is clear and the second event remains pending. An explicit
unmask delivers that queued event without another injection or handler
reinstallation.

A GICv3-side SPI context save/restore revision passes the same test
in both modes: hits=2, injected=2, unexpected=0 and rescued=0. Each
mode was tested for one real S4 cycle on a fresh boot, with CPU0,
disk=shutdown and pm_test=none. No introducing commit is claimed.

The test kernels contain a separate 8-line USB host enablement, but
the unpatched test leaves the IRQ/GIC/PM paths unchanged. The test
module sets the out-of-tree taint bit; both conditions are disclosed
in the new cover letter.

I will send v2 as a separate thread, linking this discussion. It
preserves SPI distributor configuration in GICv3 and does not change
genirq policy. It is not a claim of complete GICv3/ITS S4 support.

Thanks,
Zhu Ling


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-24 14:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-05 15:27 [PATCH] genirq/PM: Re-enable chained interrupts after hibernation Zhu Ling
2026-09-08 20:07 ` Thomas Gleixner
2026-10-24 14:46   ` Zhu Ling

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®