x86/ioapic: Restore the ignore IRQ2 routing entry logic
Subsystems (2)
cover x86/ioapic: Restore the ignore IRQ2 routing entry logic
Cure for the IRQ2 ignore logic which got lost during the irqdomain conversion and a related sanity check.
Thanks,
tglx
PATCH 1/2 x86/ioapic: Ignore IRQ2 again
Vitali ran into an issue with hotplugging CPU0 on an Amazon instance where the matrix allocator claimed to be out of vectors. He analyzed it down to the point that IRQ2, the PIC cascade interrupt, which is supposed to be not ever routed to the IO/APIC ended up having an interrupt vector assigned which got moved during unplug of CPU0.
The underlying issue is that IRQ2 for various reasons (see commit af174783b925 ("x86: I/O APIC: Never configure IRQ2" for details) is treated as a reserved system vector by the vector core code and is not accounted as a regular vector. The Amazon BIOS has an routing entry of pin2 to IRQ2 which causes the IO/APIC setup to claim that interrupt which is granted by the vector domain because there is no sanity check. As a consequence the allocation counter of CPU0 underflows which causes a subsequent unplug to fail with:
[ ... ] CPU 0 has 4294967295 vectors, 589 available. Cannot disable CPU
There is another sanity check missing in the matrix allocator, but the underlying root cause is that the IO/APIC code lost the IRQ2 ignore logic during the conversion to irqdomains.
For almost 6 years nobody complained about this wreckage, which might indicate that this requirement could be lifted, but for any system which actually has a PIC IRQ2 is unusable by design so any routing entry has no effect and the interrupt cannot be connected to a device anyway.
Due to that and due to history biased paranoia reasons restore the IRQ2 ignore logic and treat it as non existent despite a routing entry claiming otherwise.
--- arch/x86/kernel/apic/io_apic.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
arch/x86/kernel/apic/io_apic.c +10 -0
@@ -1032,6 +1032,16 @@ static int mp_map_pin_to_irq(u32 gsi, inif (idx >=0 &&test_bit (mp_irqs[idx].srcbus, mp_bus_not_pci)) { irq = mp_irqs[idx].srcbusirq; legacy =mp_is_legacy_irq (irq); +/* +* IRQ2 is unusable for historical reasons on systems which +* have a legacy PIC. See the comment vs. IRQ2 further down. +* +* If this gets removed at some point then the related code +* in lapic_assign_system_vectors() needs to be adjusted as +* well. +*/ +if (legacy && irq ==PIC_CASCADE_IR ) +return -EINVAL ; }mutex_lock (&ioapic_mutex);
[tip: x86/urgent] x86/ioapic: Ignore IRQ2 again
The following commit has been merged into the x86/urgent branch of tip:
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: a501b048a95b79e1e34f03cac3c87ff1e9f229ad Gitweb: https://git.kernel.org/tip/a501b048a95b79e1e34f03cac3c87ff1e9f229ad Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 18 Mar 2021 20:26:47 +01:00 Committer: Thomas Gleixner <[email protected]> CommitterDate: Fri, 19 Mar 2021 12:43:41 +01:00
x86/ioapic: Ignore IRQ2 again
Vitaly ran into an issue with hotplugging CPU0 on an Amazon instance where the matrix allocator claimed to be out of vectors. He analyzed it down to the point that IRQ2, the PIC cascade interrupt, which is supposed to be not ever routed to the IO/APIC ended up having an interrupt vector assigned which got moved during unplug of CPU0.
The underlying issue is that IRQ2 for various reasons (see commit af174783b925 ("x86: I/O APIC: Never configure IRQ2" for details) is treated as a reserved system vector by the vector core code and is not accounted as a regular vector. The Amazon BIOS has an routing entry of pin2 to IRQ2 which causes the IO/APIC setup to claim that interrupt which is granted by the vector domain because there is no sanity check. As a consequence the allocation counter of CPU0 underflows which causes a subsequent unplug to fail with:
[ ... ] CPU 0 has 4294967295 vectors, 589 available. Cannot disable CPU
There is another sanity check missing in the matrix allocator, but the underlying root cause is that the IO/APIC code lost the IRQ2 ignore logic during the conversion to irqdomains.
For almost 6 years nobody complained about this wreckage, which might indicate that this requirement could be lifted, but for any system which actually has a PIC IRQ2 is unusable by design so any routing entry has no effect and the interrupt cannot be connected to a device anyway.
Due to that and due to history biased paranoia reasons restore the IRQ2 ignore logic and treat it as non existent despite a routing entry claiming otherwise.
--- arch/x86/kernel/apic/io_apic.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
arch/x86/kernel/apic/io_apic.c +10 -0
@@ -1032,6 +1032,16 @@ static int mp_map_pin_to_irq(u32 gsi, int idx, int ioapic, int pin,if (idx >=0 &&test_bit (mp_irqs[idx].srcbus, mp_bus_not_pci)) { irq = mp_irqs[idx].srcbusirq; legacy =mp_is_legacy_irq (irq); +/* +* IRQ2 is unusable for historical reasons on systems which +* have a legacy PIC. See the comment vs. IRQ2 further down. +* +* If this gets removed at some point then the related code +* in lapic_assign_system_vectors() needs to be adjusted as +* well. +*/ +if (legacy && irq ==PIC_CASCADE_IR ) +return -EINVAL ; }mutex_lock (&ioapic_mutex);
PATCH 2/2 x86/vector: Add a sanity check to prevent IRQ2 allocations
In reply to Thomas GleixnerTo prevent another incidental removal of the IRQ2 ignore logic in the IO/APIC code going unnoticed add a sanity check. Add some commentry at the other place which ignores IRQ2 while at it.
--- arch/x86/kernel/apic/vector.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
arch/x86/kernel/apic/vector.c +13 -0
@@ -543,6 +543,14 @@ static int x86_vector_alloc_irqs(structif ((info->flags &X86_IRQ_ALLOC_CONTIGUOUS_VECTORS ) && nr_irqs >1 )return -ENOSYS ; +/* +* Catch any attempt to touch the cascade interrupt on a PIC +* equipped system. +*/ +if (WARN_ON_ONCE (info->flags &X86_IRQ_ALLOC_LEGACY && + virq ==PIC_CASCADE_IR )) +return -EINVAL ; +for (i =0 ; i < nr_irqs; i++) { irqd =irq_domain_get_irq_data (domain, virq + i);BUG_ON (!irqd);
@@ -745,6 +753,11 @@ void __init lapic_assign_system_vectors(/* Mark the preallocated legacy interrupts */ for (i =0 ; i <nr_legacy_irqs (); i++) { +/* +* Don't touch the cascade interrupt. It's unusable +* on PIC equipped machines. See the large comment +* in the IO/APIC code. +*/ if (i !=PIC_CASCADE_IR )irq_matrix_assign (vector_matrix,ISA_IRQ_VECTOR (i)); }
[tip: x86/apic] x86/vector: Add a sanity check to prevent IRQ2 allocations
The following commit has been merged into the x86/apic branch of tip:
The following commit has been merged into the x86/apic branch of tip:
Commit-ID: 9a98bc2cf08a095367449b3548c3d9ad4ad2cd20 Gitweb: https://git.kernel.org/tip/9a98bc2cf08a095367449b3548c3d9ad4ad2cd20 Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 18 Mar 2021 20:26:48 +01:00 Committer: Thomas Gleixner <[email protected]> CommitterDate: Tue, 30 Mar 2021 00:39:12 +02:00
x86/vector: Add a sanity check to prevent IRQ2 allocations
To prevent another incidental removal of the IRQ2 ignore logic in the IO/APIC code going unnoticed add a sanity check. Add some commentry at the other place which ignores IRQ2 while at it.
--- arch/x86/kernel/apic/vector.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
arch/x86/kernel/apic/vector.c +13 -0
@@ -543,6 +543,14 @@ static int x86_vector_alloc_irqs(struct irq_domain *domain, unsigned int virq,if ((info->flags &X86_IRQ_ALLOC_CONTIGUOUS_VECTORS ) && nr_irqs >1 )return -ENOSYS ; +/* +* Catch any attempt to touch the cascade interrupt on a PIC +* equipped system. +*/ +if (WARN_ON_ONCE (info->flags &X86_IRQ_ALLOC_LEGACY && + virq ==PIC_CASCADE_IR )) +return -EINVAL ; +for (i =0 ; i < nr_irqs; i++) { irqd =irq_domain_get_irq_data (domain, virq + i);BUG_ON (!irqd);
@@ -745,6 +753,11 @@ void __init lapic_assign_system_vectors(void)/* Mark the preallocated legacy interrupts */ for (i =0 ; i <nr_legacy_irqs (); i++) { +/* +* Don't touch the cascade interrupt. It's unusable +* on PIC equipped machines. See the large comment +* in the IO/APIC code. +*/ if (i !=PIC_CASCADE_IR )irq_matrix_assign (vector_matrix,ISA_IRQ_VECTOR (i)); }