Re: [PATCH v6 2/4] x86/irq: Use irq_chip_retrigger_hierarchy() in fixup_irqs()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] x86/irq: lost interrupts in fixup_irqs() due to bypassed irq_retrigger
--

commit a27ffeac3562f1a871ba38be5a47e5097ea4e9f4
Author: Naman Jain <namjain@linux.microsoft.com>

x86/irq: Use irq_chip_retrigger_hierarchy() in fixup_irqs()

fixup_irqs() re-injects a pending interrupt on its new target CPU by looking
at the outermost domain chip and invoking its irq_retrigger() callback
directly. This commit changes the implementation to use
irq_chip_retrigger_hierarchy() instead, which walks up the interrupt hierarchy
until it finds a chip that implements irq_retrigger(). It also changes
desc->lock to use a scoped guard.

> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
> index 30122f0b3af96..acdc1145cce6d 100644
> --- a/arch/x86/kernel/irq.c
> +++ b/arch/x86/kernel/irq.c
> @@ -490,21 +490,19 @@ void fixup_irqs(void)
>  	 * nothing else will touch it.
>  	 */
>  	for (vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) {
> -		if (IS_ERR_OR_NULL(__this_cpu_read(vector_irq[vector])))
> +		desc = __this_cpu_read(vector_irq[vector]);
> +
> +		if (IS_ERR_OR_NULL(desc))
>  			continue;
>  
>  		if (is_vector_pending(vector)) {
> -			desc = __this_cpu_read(vector_irq[vector]);
> -
> -			raw_spin_lock(&desc->lock);
> +			guard(raw_spinlock)(&desc->lock);
>  			data = irq_desc_get_irq_data(desc);
> -			chip = irq_data_get_irq_chip(data);
> -			if (chip->irq_retrigger) {
> -				chip->irq_retrigger(data);
> +			ret = irq_chip_retrigger_hierarchy(data);

[Severity: High]
Does this skip the outermost chip and potentially drop the interrupt?

In fixup_irqs(), data is passed to irq_chip_retrigger_hierarchy(), which
begins by immediately advancing to data->parent_data:

kernel/irq/chip.c:irq_chip_retrigger_hierarchy() {
	for (data = data->parent_data; data; data = data->parent_data)
		if (data->chip && data->chip->irq_retrigger)
			return data->chip->irq_retrigger(data);

	return 0;
}

If the outermost chip (such as lapic_controller for legacy IRQs) implements
irq_retrigger itself, the original irq_data is skipped and the loop will
terminate immediately if data->parent_data is NULL.

This can lead to hardware interrupts pending during CPU hot-unplug being
silently dropped. It appears this logic error is not fixed by subsequent
patches in the series either.

> +			if (ret)
>  				__this_cpu_write(vector_irq[vector], VECTOR_RETRIGGERED);
> -			}
> -			raw_spin_unlock(&desc->lock);
>  		}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923043023.3150498-1-namjain@linux.microsoft.com?part=2