Re: [PATCH] genirq/chip: Don't call add_interrupt_randomness() for NMIs

[Marc Zyngier]

On Thu, 07 May 2026 12:05:18 +0100,
Mark Rutland <mark.rutland@xxxxxxx> wrote:
> 
> 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:
>       lock_acquire+0x260/0x40c
>       _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
>       __primary_switched+0x88/0x90
>     irq event stamp: 62068
>     hardirqs last  enabled at (62067): [<ffff8000801cc0ec>] seqcount_lockdep_reader_access.constprop.0+0xf0/0xfc
>     hardirqs last disabled at (62068): [<ffff80008150e0ac>] _raw_spin_lock_irqsave+0x94/0xb0
>     softirqs last  enabled at (62050): [<ffff800080017614>] put_cpu_fpsimd_context+0x1c/0x4c
>     softirqs last disabled at (62048): [<ffff8000800175c8>] get_cpu_fpsimd_context+0x1c/0x4c
>         other info that might help us debug this:
>     Possible unsafe locking scenario:
>            CPU0
>            ----
>       lock(&base->lock);
>       <Interrupt>
>         lock(&base->lock);
>         *** DEADLOCK ***
>     3 locks held by perf/695:
>      #0: ffff0080146cd2c8 (&type->i_mutex_dir_key#6){++++}-{4:4}, at: lookup_slow+0x30/0x68
>      #1: ffff80008332b858 (rcu_read_lock){....}-{1:3}, at: blk_mq_run_hw_queue+0xf4/0x1fc
>      #2: ffff008000b6aa18 (&host->lock){-.-.}-{3:3}, at: ata_scsi_queuecmd+0x28/0x88
>     stack backtrace:
>     CPU: 3 UID: 0 PID: 695 Comm: perf Not tainted 7.1.0-rc2-pnmi #465 PREEMPT
>     Hardware name: ARM LTD Morello System Development Platform, BIOS EDK II Mar  7 2024
>     Call trace:
>      show_stack+0x18/0x24 (C)
>      dump_stack_lvl+0xc4/0x148
>      dump_stack+0x18/0x24
>      print_usage_bug.part.0+0x29c/0x364
>      lock_acquire+0x364/0x40c
>      _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 we should handle NMIs 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:
> 
>   21bbbc50f398f ("irqchip/gic-v3: Switch high priority PPIs over to handle_percpu_devid_irq()")
>   5ff78c8de9d83 ("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 our other NNI handling flows,
> which do not call add_interrupt_randomness().
> 
> At the same time, update the kerneldoc 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: fd7400cfcbaa ("genirq/chip: Invoke add_interrupt_randomness() in handle_percpu_devid_irq()")
> Reported-by: Ada Couprie Diaz <ada.coupriediaz@xxxxxxx>
> Signed-off-by: Mark Rutland <mark.rutland@xxxxxxx>
> Cc: Justin He <justin.he@xxxxxxx>
> Cc: Marc Zyngier <maz@xxxxxxxxxx>
> Cc: Michael Kelley <mhklinux@xxxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxx>
> Cc: Vladimir Murzin <vladimir.murzin@xxxxxxx>

Thanks for catching that one. FWIW:

Reviewed-by: Marc Zyngier <maz@xxxxxxxxxx>

	M.

-- 
Without deviation from the norm, progress is not possible.