Attempt to clarify the purpose of setup_ioapic_dest(), as the comment ahead of it is outdated, and looks to be a verbatim copy from Linux from one of the code imports.
The function serves two purposes: shuffling the interrupts across CPUs after SMP bringup or re-assigning all interrupts to CPU#0 if no IRQ balancing is set at run time. However the function won't perform any of those functions correctly, as it was unconditionally using desc->arch.cpu_mask as the target CPU mask for interrupts (which is the current target anyway). Fix by adding a new `shuffle` parameter that's used to signal whether the function is intended to balance interrupts across CPUs, or to re-assign all interrupts to the BSP. Signed-off-by: Roger Pau Monné <[email protected]> --- I couldn't find a specific Fixes tag to use here, I think this has been broken all along. --- xen/arch/x86/include/asm/irq.h | 2 +- xen/arch/x86/io_apic.c | 13 +++++++------ xen/arch/x86/platform_hypercall.c | 2 +- xen/arch/x86/smpboot.c | 3 ++- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/xen/arch/x86/include/asm/irq.h b/xen/arch/x86/include/asm/irq.h index 7315150b66b4..df7b48c8653e 100644 --- a/xen/arch/x86/include/asm/irq.h +++ b/xen/arch/x86/include/asm/irq.h @@ -137,7 +137,7 @@ int i8259A_resume(void); void enable_IO_APIC(void); void setup_IO_APIC(void); void disable_IO_APIC(void); -void setup_ioapic_dest(void); +void setup_ioapic_dest(bool shuffle); vmask_t *io_apic_get_used_vector_map(unsigned int irq); extern unsigned int io_apic_irqs; diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c index 3c59bf0e15da..19960d291c47 100644 --- a/xen/arch/x86/io_apic.c +++ b/xen/arch/x86/io_apic.c @@ -717,12 +717,14 @@ static int __init find_isa_irq_apic(int irq, int type) static int pin_2_irq(int idx, int apic, int pin); /* - * This function currently is only a helper for the i386 smp boot process where - * we need to reprogram the ioredtbls to cater for the cpus which have come online - * so mask in all cases should simply be TARGET_CPUS + * This function serves two different purposes: shuffling the IO-APIC + * interrupts across CPUs after SMP bringup, or re-assigning all interrupts to + * the BSP if IRQ balancing is disabled at runtime. Such functional + * distinction is signaled by the `shuffle` parameter. */ -void /*__init*/ setup_ioapic_dest(void) +void setup_ioapic_dest(bool shuffle) { + const cpumask_t *mask = shuffle ? TARGET_CPUS : cpumask_of(0); int pin, ioapic, irq, irq_entry; if (skip_ioapic_setup) @@ -740,8 +742,7 @@ void /*__init*/ setup_ioapic_dest(void) desc = irq_to_desc(irq); spin_lock_irqsave(&desc->lock, flags); - BUG_ON(!cpumask_intersects(desc->arch.cpu_mask, &cpu_online_map)); - set_ioapic_affinity_irq(desc, desc->arch.cpu_mask); + set_ioapic_affinity_irq(desc, mask); spin_unlock_irqrestore(&desc->lock, flags); } } diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c index 79bb99e0b602..22fd65a6aa7e 100644 --- a/xen/arch/x86/platform_hypercall.c +++ b/xen/arch/x86/platform_hypercall.c @@ -337,7 +337,7 @@ ret_t do_platform_op( case QUIRK_NOIRQBALANCING: printk("Platform quirk -- Disabling IRQ balancing/affinity.\n"); opt_noirqbalance = 1; - setup_ioapic_dest(); + setup_ioapic_dest(false); break; case QUIRK_IOAPIC_BAD_REGSEL: dprintk(XENLOG_WARNING, diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c index 69cc9bbc6e2c..55ea62d7d67f 100644 --- a/xen/arch/x86/smpboot.c +++ b/xen/arch/x86/smpboot.c @@ -1433,7 +1433,8 @@ void __init smp_cpus_done(void) check_nmi_watchdog(); } - setup_ioapic_dest(); + if ( !opt_noirqbalance ) + setup_ioapic_dest(true); mtrr_save_state(); mtrr_aps_sync_end(); -- 2.51.0
