linux-kernel - Re: [PATCH RFC 1/2] x86/apic: Do not make an exception for PIC_CASCADE_IR when marking legacy irqs in irq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <874kh9xuid.fsf@nanos.tec.linutronix.de>
Date:   Wed, 17 Mar 2021 23:52:58 +0100
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Vitaly Kuznetsov <vkuznets@...hat.com>, x86@...nel.org
Cc:     Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC 1/2] x86/apic: Do not make an exception for PIC_CASCADE_IR when marking legacy irqs in irq_matrix

On Wed, Mar 17 2021 at 22:40, Thomas Gleixner wrote:
> af174783b925 ("x86: I/O APIC: Never configure IRQ2")
>
> has a very nice explanation why.
>
> Back then the logic was quite different. All legacy PIC interrupts
> (0-15) were bound to the legacy vectors at boot and never moved away.
>
> There was a check in the back then setup routing which prevented the
> IOAPIC routing of IRQ2 which got lost at some point. Haven't figured out
> yet where this might be. Still digging in those ancient horrors.

So the commit in question is:

  d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")

For almost 6 years nobody complained about this wreckage, which might
indicate that we could lift this requirement, but OTOH for any system
which actually has a PIC IRQ2 is unusable by design.

And my history based paranoia vs. these kind of changes just makes me
tend to the safe side, so restoring the original behaviour makes more
sense than just papering over it.

Just to be clear: The reservation of PIC pin2 (aka. IRQ2) as a permanent
system vector cannot go away for any system which has a PIC. If that
ever fires then the PIC is hosed and you really want the warning about
the unhandled vector which is easy to identify because it is fixed.

You might think that's not relevant for modern CPUs where the PIC is
emulated, but I've dealt with reports where that emulation goes south so
I rather keep those sanity checks around as long as that legacy mess is
relevant. No idea why it still exists, but that's a different story.

Thanks,

        tglx
---
Subject: x86/ioapic: Ignore IRQ2 again
From: Thomas Gleixner <tglx@...utronix.de>
Date: Wed, 17 Mar 2021 23:10:02 +0100

Why did I have to do the archaeology?

< INSERT PROPER CHANGELOG >

Fixes: d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
---
 arch/x86/kernel/apic/io_apic.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1032,6 +1032,16 @@ static int mp_map_pin_to_irq(u32 gsi, in
 	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);