Commit 512718bb authored by Mark Rutland's avatar Mark Rutland Committed by Thomas Gleixner
Browse files

genirq/chip: Don't call add_interrupt_randomness() for NMIs 

Recently handle_percpu_devid_irq() was changed to call
add_interrupt_randomness(). This introduced a potential deadlock when
handle_percpu_devid_irq() is used to handle an NMI, which can be
detected with lockdep, e.g.

    ================================
    WARNING: inconsistent lock state
    7.1.0-rc2-pnmi #465 Not tainted
    --------------------------------
    inconsistent {INITIAL USE} -> {IN-NMI} usage.
    perf/695 [HC1[1]:SC0[0]:HE0:SE1] takes:
    ffff00837dfd3a18 (&base->lock){-.-.}-{2:2}, at: lock_timer_base+0x6c/0xac
    {INITIAL USE} state was registered at:
      _raw_spin_lock_irqsave+0x68/0xb0
      lock_timer_base+0x6c/0xac
      __mod_timer+0x100/0x32c
      add_timer_global+0x2c/0x40
      __queue_delayed_work+0xf0/0x140
      queue_delayed_work_on+0x134/0x138
      mem_cgroup_css_online+0x30c/0x310
      online_css+0x34/0x10c
      cgroup_init_subsys+0x158/0x1c8
      cgroup_init+0x440/0x524
      start_kernel+0x888/0x998

    other info that might help us debug this:
    Possible unsafe locking scenario:
           CPU0
           ----
      lock(&base->lock);
      <Interrupt>
        lock(&base->lock);
        *** DEADLOCK ***

    Call trace:
     _raw_spin_lock_irqsave+0x68/0xb0
     lock_timer_base+0x6c/0xac
     add_timer_on+0x78/0x16c
     add_interrupt_randomness+0x124/0x134
     handle_percpu_devid_irq+0xd4/0x16c
     handle_irq_desc+0x40/0x58
     generic_handle_domain_nmi+0x28/0x50
     __gic_handle_nmi.isra.0+0x4c/0xa0
     gic_handle_irq+0x38/0x2bc
     call_on_irq_stack+0x30/0x48
     do_interrupt_handler+0x80/0x98
     el1_interrupt+0x90/0xac
     el1h_64_irq_handler+0x18/0x24
     el1h_64_irq+0x80/0x84
     [...]

During review, Thomas pointed out it wouldn't be safe for
handle_percpu_devid_irq() to call add_interrupt_randomness() if it was
used to handle NMIs:

  https://lore.kernel.org/lkml/87bjgik042.ffs@tglx/



... but evidently people missed that handle_percpu_devid_irq() *is* used
for NMIs.

While it might seem that NMIs should be handled with a separate
handle_percpu_devid_nmi() function, for various structural reasons this was
impractical, and handle_percpu_devid_irq() has been expected to be used for
NMIs since commits:

  21bbbc50 ("irqchip/gic-v3: Switch high priority PPIs over to handle_percpu_devid_irq()")
  5ff78c8d ("genirq: Kill handle_percpu_devid_fasteoi_nmi()")

Taking the above into account, avoid the deadlock by not calling
add_interrupt_randomness() when handle_percpu_devid_irq() is called in an
NMI context. This is consistent with other NNI handling flows, which do not
call add_interrupt_randomness().

At the same time, update the kernel-doc comment to make it clear that
handle_percpu_devid_irq() can be called in NMI context. The rest of
handle_percpu_devid_irq() is currently NMI safe and doesn't need to change.

Fixes: fd7400cf ("genirq/chip: Invoke add_interrupt_randomness() in handle_percpu_devid_irq()")
Reported-by: default avatarAda Couprie Diaz <ada.coupriediaz@arm.com>
Signed-off-by: default avatarMark Rutland <mark.rutland@arm.com>
Signed-off-by: default avatarThomas Gleixner <tglx@kernel.org>
Reviewed-by: default avatarJinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: default avatarMarc Zyngier <maz@kernel.org>
Link: https://patch.msgid.link/20260507110518.3128248-1-mark.rutland@arm.com
parent a7c7e426

kernel/irq/chip.c

+7 −2
Original line number Diff line number Diff line
@@ -14,6 +14,7 @@ 
#include <linux/interrupt.h>

#include <linux/kernel_stat.h>

#include <linux/irqdomain.h>

#include <linux/preempt.h>

#include <linux/random.h>



#include <trace/events/irq.h>
@@ -893,7 +894,10 @@ void handle_percpu_irq(struct irq_desc *desc) 
 *

 * action->percpu_dev_id is a pointer to percpu variables which

 * contain the real device id for the cpu on which this handler is

 * called

 * called.

 *

 * May be used for NMI interrupt lines, and so may be called in IRQ or NMI

 * context.

 */

void handle_percpu_devid_irq(struct irq_desc *desc)

{
@@ -930,6 +934,7 @@ void handle_percpu_devid_irq(struct irq_desc *desc) 
			    enabled ? " and unmasked" : "", irq, cpu);

	}



	if (!in_nmi())

		add_interrupt_randomness(irq);



	if (chip->irq_eoi)