| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250512173250.1.If5c00cf9f08732f4af5f104ae59b8785c7f69536@changeid>
Date: Mon, 12 May 2025 17:32:52 -0700
From: Douglas Anderson <dianders@...omium.org>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: chintanpandya@...gle.com,
Douglas Anderson <dianders@...omium.org>,
Marc Zyngier <maz@...nel.org>,
Maulik Shah <quic_mkshah@...cinc.com>,
linux-kernel@...r.kernel.org
Subject: [PATCH] genirq/PM: Fix IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND if depth > 1
The IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag doesn't work properly if the
IRQ disable depth is not 0 or 1 at suspend time. In this case, the
IRQ's depth will be decremented but the IRQ won't be enabled to wake
the system up. Add a special case for when we're suspending and always
enable the IRQ in that case.
A few notes:
* As part of this, irq_startup() can no longer force the depth to 0.
Change irq_startup() to decrement by 1 and make all other callers of
irq_startup() initialize the depth to 1 to keep them working the
same.
* In order to avoid turning __enable_irq() into spaghetti code for the
special case, swap from a "switch" statement to a handful of
"if/else". Nicely, this eliminates an old "goto".
Fixes: 90428a8eb494 ("genirq/PM: Introduce IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND flag")
Signed-off-by: Douglas Anderson <dianders@...omium.org>
---
I fully realize there are rough edges to this patch. Specifically,
it's a bit ugly to init the depth to 1 in all the callers. Things
could also get dicey of any code tries to enable/disable the IRQ
_after_ we've set the "IRQD_IRQ_ENABLED_ON_SUSPEND", though I hope
that doesn't happen.
If you hate this patch, feel free to simply treat it as a bug report.
I'm happy if someone wants to write an alternative patch or I can make
whatever cleanups are needed and send a v2/v3/etc. Let me know!
This patch has only been lightly tested.
kernel/irq/chip.c | 3 ++-
kernel/irq/cpuhotplug.c | 4 +++-
kernel/irq/manage.c | 29 ++++++++++++++++-------------
kernel/irq/pm.c | 2 +-
4 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 36cf1b09cc84..24a5958c09e4 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -272,7 +272,7 @@ int irq_startup(struct irq_desc *desc, bool resend, bool force)
const struct cpumask *aff = irq_data_get_affinity_mask(d);
int ret = 0;
- desc->depth = 0;
+ desc->depth--;
if (irqd_is_started(d)) {
irq_enable(desc);
@@ -313,6 +313,7 @@ int irq_activate_and_startup(struct irq_desc *desc, bool resend)
{
if (WARN_ON(irq_activate(desc)))
return 0;
+ desc->depth = 1;
return irq_startup(desc, resend, IRQ_START_FORCE);
}
diff --git a/kernel/irq/cpuhotplug.c b/kernel/irq/cpuhotplug.c
index 15a7654eff68..8d98a2961e91 100644
--- a/kernel/irq/cpuhotplug.c
+++ b/kernel/irq/cpuhotplug.c
@@ -218,8 +218,10 @@ static void irq_restore_affinity_of_irq(struct irq_desc *desc, unsigned int cpu)
if (desc->istate & IRQS_SUSPENDED)
return;
- if (irqd_is_managed_and_shutdown(data))
+ if (irqd_is_managed_and_shutdown(data)) {
+ desc->depth = 1;
irq_startup(desc, IRQ_RESEND, IRQ_START_COND);
+ }
/*
* If the interrupt can only be directed to a single target
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 753eef8e041c..a8c9b645fe49 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -688,7 +688,14 @@ EXPORT_SYMBOL_GPL(irq_set_vcpu_affinity);
void __disable_irq(struct irq_desc *desc)
{
- if (!desc->depth++)
+ struct irq_data *irqd = &desc->irq_data;
+
+ /*
+ * Always increase the disable depth; actually do the disable if
+ * the depth was 0 _or_ we temporarily enabled during the suspend
+ * path.
+ */
+ if (!desc->depth++ || irqd_is_enabled_on_suspend(irqd))
irq_disable(desc);
}
@@ -786,15 +793,12 @@ void disable_nmi_nosync(unsigned int irq)
void __enable_irq(struct irq_desc *desc)
{
- switch (desc->depth) {
- case 0:
- err_out:
+ struct irq_data *irqd = &desc->irq_data;
+
+ if (desc->depth == 0 || desc->istate & IRQS_SUSPENDED) {
WARN(1, KERN_WARNING "Unbalanced enable for IRQ %d\n",
irq_desc_get_irq(desc));
- break;
- case 1: {
- if (desc->istate & IRQS_SUSPENDED)
- goto err_out;
+ } else if (desc->depth == 1 || irqd_is_enabled_on_suspend(irqd)) {
/* Prevent probing on this irq: */
irq_settings_set_noprobe(desc);
/*
@@ -809,9 +813,7 @@ void __enable_irq(struct irq_desc *desc)
* invoke irq_enable() under the hood.
*/
irq_startup(desc, IRQ_RESEND, IRQ_START_FORCE);
- break;
- }
- default:
+ } else {
desc->depth--;
}
}
@@ -1774,6 +1776,9 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
irqd_set(&desc->irq_data, IRQD_NO_BALANCING);
}
+ /* Undo nested disables: */
+ desc->depth = 1;
+
if (!(new->flags & IRQF_NO_AUTOEN) &&
irq_settings_can_autoenable(desc)) {
irq_startup(desc, IRQ_RESEND, IRQ_START_COND);
@@ -1785,8 +1790,6 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
* interrupts forever.
*/
WARN_ON_ONCE(new->flags & IRQF_SHARED);
- /* Undo nested disables: */
- desc->depth = 1;
}
} else if (new->flags & IRQF_TRIGGER_MASK) {
diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
index c556bc49d213..b151a94f6233 100644
--- a/kernel/irq/pm.c
+++ b/kernel/irq/pm.c
@@ -86,8 +86,8 @@ static bool suspend_device_irq(struct irq_desc *desc)
* Enable interrupt here to unmask/enable in irqchip
* to be able to resume with such interrupts.
*/
- __enable_irq(desc);
irqd_set(irqd, IRQD_IRQ_ENABLED_ON_SUSPEND);
+ __enable_irq(desc);
}
/*
* We return true here to force the caller to issue
--
2.49.0.1045.g170613ef41-goog
Powered by blists - more mailing lists