[PATCH v4 2/2] kexec: Prevent redundant IRQ masking by checking state before shutdown
From: Eliav Farber <hidden>
Date: 2024-11-29 11:31:29
Also in:
kexec, linux-arm-kernel, linux-riscv, lkml
Subsystem:
irq subsystem, kexec, the rest · Maintainers:
Thomas Gleixner, Andrew Morton, Baoquan He, Mike Rapoport, Pasha Tatashin, Pratyush Yadav, Linus Torvalds
During machine kexec, the function machine_kexec_mask_interrupts() is responsible for disabling or masking all interrupts. While the irq_disable hook ensures that an already-disabled IRQ is not disabled again, the current implementation unconditionally invokes the irq_mask() function for every interrupt descriptor, even when the interrupt is already masked. A specific issue was observed in the crash kernel flow after unbinding a device (prior to kexec) that used a GPIO as an IRQ source. The warning was triggered by the gpiochip_disable_irq() function, which attempted to clear the FLAG_IRQ_IS_ENABLED flag when FLAG_USED_AS_IRQ was not set:
void gpiochip_disable_irq(struct gpio_chip *gc, unsigned int offset)
{
struct gpio_desc *desc = gpiochip_get_desc(gc, offset);
if (!IS_ERR(desc) &&
!WARN_ON(!test_bit(FLAG_USED_AS_IRQ, &desc->flags)))
clear_bit(FLAG_IRQ_IS_ENABLED, &desc->flags);
}
This issue surfaced after commit a8173820f441 ("gpio: gpiolib: Allow GPIO
IRQs to lazy disable") introduced lazy disablement for GPIO IRQs. It
replaced disable/enable hooks with mask/unmask hooks. Unlike the disable
hook, the mask hook doesn't handle already-masked IRQs.
When a GPIO-IRQ driver is unbound, the IRQ is released, triggering
__irq_disable() and irq_state_set_masked(). A subsequent call to
machine_kexec_mask_interrupts() re-invokes chip->irq_mask(). This results
in a call chain, including gpiochip_irq_mask() and gpiochip_disable_irq().
Since FLAG_USED_AS_IRQ was cleared earlier, a warning occurs.
This patch addresses the issue by:
- Replacing the calls to irq_mask() and irq_disable() hooks with a
simplified call to irq_shutdown().
- Checking if the interrupt is started (irqd_is_started) before calling
the shutdown.
As part of this change, the irq_shutdown() declaration was moved from
kernel/irq/internals.h to include/linux/irq.h to make it accessible
outside the kernel/irq/ directory, as the former can only be included
within that directory.
Signed-off-by: Eliav Farber <redacted>
---
V4 -> V3: Add missing <linux/irq.h> include.
include/linux/irq.h | 3 +++
kernel/irq/internals.h | 1 -
kernel/kexec_core.c | 9 +++------
3 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/include/linux/irq.h b/include/linux/irq.h
index fa711f80957b..48a3df728c47 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h@@ -694,6 +694,9 @@ extern int irq_chip_request_resources_parent(struct irq_data *data); extern void irq_chip_release_resources_parent(struct irq_data *data); #endif +/* Shut down the interrupt */ +extern void irq_shutdown(struct irq_desc *desc); + /* Handling of unhandled and spurious interrupts: */ extern void note_interrupt(struct irq_desc *desc, irqreturn_t action_ret);
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index fe0272cd84a5..1f9287b1ccb7 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h@@ -88,7 +88,6 @@ extern int irq_activate(struct irq_desc *desc); extern int irq_activate_and_startup(struct irq_desc *desc, bool resend); extern int irq_startup(struct irq_desc *desc, bool resend, bool force); -extern void irq_shutdown(struct irq_desc *desc); extern void irq_shutdown_and_deactivate(struct irq_desc *desc); extern void irq_enable(struct irq_desc *desc); extern void irq_disable(struct irq_desc *desc);
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 777191458544..09c8e9814cd2 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c@@ -41,6 +41,7 @@ #include <linux/objtool.h> #include <linux/kmsg_dump.h> #include <linux/irqdesc.h> +#include <linux/irq.h> #include <asm/page.h> #include <asm/sections.h>
@@ -1084,7 +1085,7 @@ void machine_kexec_mask_interrupts(void) int check_eoi = 1; chip = irq_desc_get_chip(desc); - if (!chip) + if (!chip || !irqd_is_started(&desc->irq_data)) continue; if (IS_ENABLED(CONFIG_ARM64)) {
@@ -1098,10 +1099,6 @@ void machine_kexec_mask_interrupts(void) if (check_eoi && chip->irq_eoi && irqd_irq_inprogress(&desc->irq_data)) chip->irq_eoi(&desc->irq_data); - if (chip->irq_mask) - chip->irq_mask(&desc->irq_data); - - if (chip->irq_disable && !irqd_irq_disabled(&desc->irq_data)) - chip->irq_disable(&desc->irq_data); + irq_shutdown(desc); } }
--
2.40.1