[PATCH RFC] genirq/kexec: Do not take the bus lock of an interrupt chip on the crash path

[PATCH RFC] genirq/kexec: Do not take the bus lock of an interrupt chip on the crash path

[Igor Velkov via B4 Relay]

From: Igor Velkov <iav@iav.lv>

machine_kexec_mask_interrupts() calls irq_set_irqchip_state() for every
started interrupt. That function takes the bus lock of the interrupt
chip and syncs the chip when it drops the lock. On the crash path
interrupts are off and the other CPUs are stopped, so a chip that needs
its bus for the sync never returns.

On RK3399 boards with an RK808 PMIC the RTC alarm interrupt belongs to
the regmap-irq chip of the RK808, which sits on I2C. rk808_irq_chip sets
init_ack_masked, so regmap_irq_sync_unlock() writes the ack register on
every unlock, and the I2C controller needs its own interrupt to finish
the transfer. A panic with a crash kernel loaded prints

  SMP: stopping secondary CPUs

and then nothing until the watchdog resets the board. With a print
before and after the call, the last line is

  kexec-dbg: irq 54 chip rk808 buslock 1: set state

Skip the call for chips that have irq_bus_lock. It clears the active
state of an interrupt forwarded to a VM; the EOI and irq_shutdown() that
follow still run for these interrupts.

Fixes: 78fd584cdec0 ("arm64: kdump: implement machine_crash_shutdown()")
Assisted-by: LLM
Signed-off-by: Igor Velkov <iav@iav.lv>
---
RFC: the hang is real, but I am not sure this is the right place to fix
it. Two other places would also do:
- irq_set_irqchip_state() could look for a chip that implements the
  callback before it takes the bus lock;
- regmap-irq could stop writing the ack register on every sync.

Of the 57 drivers that set irq_bus_lock, plus regmap-irq, only
drivers/cdx/cdx_msi.c also has a parent chip that implements
irq_set_irqchip_state (the ITS); the others returned -EINVAL from the
call anyway. irq-imx-irqsteer.c uses irq_bus_lock for runtime PM rather
than a slow bus.

Tested with sysrq-c and kdump-tools on 7.3-rc5:
- Kobol Helios64 (RK3399, RK808): hangs 3 of 3 without the patch, saves
  the dump 3 of 3 with it, both kernels from one tree;
- Radxa ROCK Pi 4A (RK3399, RK808): hangs 2 of 2 without, saves the dump
  2 of 2 with it.
With the rk808-rtc driver unbound the unpatched kernel saves the dump.
---
 kernel/irq/kexec.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/irq/kexec.c b/kernel/irq/kexec.c
index 1a3deffe6b5b..c6c745cc76a2 100644
--- a/kernel/irq/kexec.c
+++ b/kernel/irq/kexec.c
@@ -20,7 +20,11 @@ void machine_kexec_mask_interrupts(void)
 		if (!chip || !irqd_is_started(&desc->irq_data))
 			continue;
 
-		if (IS_ENABLED(CONFIG_GENERIC_IRQ_KEXEC_CLEAR_VM_FORWARD)) {
+		/*
+		 * irq_set_irqchip_state() takes the bus lock, and a chip that
+		 * has one may sleep or talk to its bus when the lock is dropped.
+		 */
+		if (IS_ENABLED(CONFIG_GENERIC_IRQ_KEXEC_CLEAR_VM_FORWARD) && !chip->irq_bus_lock) {
 			/*
 			 * First try to remove the active state from an interrupt which is forwarded
 			 * to a VM. If the interrupt is not forwarded, try to EOI the interrupt.

---
base-commit: a0e1fdb96578ea562a03c487f70673d228824d0a
change-id: 20261005-kexec-irq-bus-lock-f48d5966443d

Best regards,
-- 
Igor Velkov <iav@iav.lv>

Re: [PATCH RFC] genirq/kexec: Do not take the bus lock of an interrupt chip on the crash path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] irq/kexec: incomplete slow bus check allows hang via irq_bus_sync_unlock
--

commit a12343156827b9035af6e3982ac65b3437921502
Author: Igor Velkov <iav@iav.lv>

genirq/kexec: Do not take the bus lock of an interrupt chip on the crash path

machine_kexec_mask_interrupts() calls irq_set_irqchip_state() for every
started interrupt. That function takes the bus lock of the interrupt
chip and syncs the chip when it drops the lock. On the crash path
interrupts are off and the other CPUs are stopped, so a chip that needs
its bus for the sync never returns.

> diff --git a/kernel/irq/kexec.c b/kernel/irq/kexec.c
> index 1a3deffe6b5b6..c6c745cc76a25 100644
> --- a/kernel/irq/kexec.c
> +++ b/kernel/irq/kexec.c
> @@ -20,7 +20,11 @@ void machine_kexec_mask_interrupts(void)
>  		if (!chip || !irqd_is_started(&desc->irq_data))
>  			continue;
>  
> -		if (IS_ENABLED(CONFIG_GENERIC_IRQ_KEXEC_CLEAR_VM_FORWARD)) {
> +		/*
> +		 * irq_set_irqchip_state() takes the bus lock, and a chip that
> +		 * has one may sleep or talk to its bus when the lock is dropped.
> +		 */
> +		if (IS_ENABLED(CONFIG_GENERIC_IRQ_KEXEC_CLEAR_VM_FORWARD) && !chip->irq_bus_lock) {

[Severity: Medium]
Does this check comprehensively protect against slow bus chips?

If an irqchip implements irq_bus_sync_unlock but omits irq_bus_lock, this
condition evaluates to true and irq_set_irqchip_state() is called.
Since irq_set_irqchip_state() ultimately unlocks the descriptor via
__irq_put_desc_unlock(), it unconditionally invokes the sync callback:

kernel/irq/irqdesc.c:__irq_put_desc_unlock() {
    ...
    if (bus)
        chip_bus_sync_unlock(desc);
    ...
}

Executing the sync callback on the crash path without interrupts could hang the
system, defeating the purpose of this patch.

Other places in the subsystem check for both callbacks to identify slow bus
locking/syncing, for instance:

kernel/irq/manage.c:irq_supports_nmi() {
    ...
    if (d->chip->irq_bus_lock || d->chip->irq_bus_sync_unlock)
        return false;
    ...
}

Should this check also ensure !chip->irq_bus_sync_unlock?

>  			/*
>  			 * First try to remove the active state from an interrupt which is forwarded
>  			 * to a VM. If the interrupt is not forwarded, try to EOI the interrupt.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005-kexec-irq-bus-lock-v1-1-09f6b5323460@iav.lv?part=1

Re: [PATCH RFC] genirq/kexec: Do not take the bus lock of an interrupt chip on the crash path

[Igor Velkov]

On Mon, 05 Oct 2026 01:41:55 +0000, sashiko-bot@kernel.org wrote:
> If an irqchip implements irq_bus_sync_unlock but omits irq_bus_lock, this
> condition evaluates to true and irq_set_irqchip_state() is called.
[...]
> Should this check also ensure !chip->irq_bus_sync_unlock?

Yes. The next version will test both callbacks, as irq_supports_nmi()
does. No chip in tip/irq/core sets only one of them, as far as I can
grep, so nothing changes for current users.

Igor