Re: [PATCH 06/12] x86/i8259: redo workaround for AMD spurious PIC interrupts

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.11.2025 09:35, Roger Pau Monné wrote:
> On Thu, Nov 20, 2025 at 04:05:38PM +0100, Jan Beulich wrote:
>> On 20.11.2025 10:58, Roger Pau Monne wrote:
>>> --- a/xen/arch/x86/i8259.c
>>> +++ b/xen/arch/x86/i8259.c
>>> @@ -407,21 +407,14 @@ void __init init_IRQ(void)
>>>          per_cpu(vector_irq, cpu)[LEGACY_VECTOR(irq)] = irq;
>>>  
>>>          /*
>>> -         * The interrupt affinity logic never targets interrupts to offline
>>> -         * CPUs, hence it's safe to use cpumask_all here.
>>> -         *
>>>           * Legacy PIC interrupts are only targeted to CPU0, but depending 
>>> on
>>>           * the platform they can be distributed to any online CPU in 
>>> hardware.
>>> -         * Note this behavior has only been observed on AMD hardware. In 
>>> order
>>> -         * to cope install all active legacy vectors on all CPUs.
>>> -         *
>>> -         * IO-APIC will change the destination mask if/when taking 
>>> ownership of
>>> -         * the interrupt.
>>> +         * Note this behavior has only been observed on AMD hardware. Set 
>>> the
>>> +         * target CPU as expected here, and cope with the possibly spurious
>>> +         * interrupts in do_IRQ().  This behavior has only been observed
>>> +         * during AP bringup.
>>>           */
>>> -        cpumask_copy(desc->arch.cpu_mask,
>>> -                     (boot_cpu_data.x86_vendor &
>>> -                      (X86_VENDOR_AMD | X86_VENDOR_HYGON) ? &cpumask_all
>>> -                                                          : 
>>> cpumask_of(cpu)));
>>> +        cpumask_copy(desc->arch.cpu_mask, cpumask_of(cpu));
>>>          desc->arch.vector = LEGACY_VECTOR(irq);
>>>      }
>>
>> Doesn't this collide with what setup_vector_irq() does (see also patch 04)? 
>> If
>> you don't set all bits here, not all CPUs will have the vector_irq[] slot set
>> correctly for do_IRQ() to actually be able to associate the vector with the
>> right IRQ.
> 
> For the AMD workaround I've only ever saw the spurious vector
> triggering on CPUs different than the BSP at bringup,

Besides this possibly being an artifact (the issue occurring once during boot
may hide it otherwise potentially also occurring later), I think we want to be
conservative and (continue to) cover all possible cases unless we have an
explanation for why the issue would be AP-bringup-time-only.

Jan

> I don't think we
> strictly need to bind all legacy vectors on all possible CPUs.  Well
> behaved PIC interrupts will only target the BSP, and that's properly
> setup.
> 
> Thanks, Roger.