linux-kernel - [PATCH] x86/i8259: Work around buggy legacy PIC

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-next>] [day] [month] [year] [list]

Message-Id: <20210512210459.1983026-1-luzmaximilian@gmail.com>
Date:   Wed, 12 May 2021 23:04:59 +0200
From:   Maximilian Luz <luzmaximilian@...il.com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>
Cc:     Maximilian Luz <luzmaximilian@...il.com>,
        "H. Peter Anvin" <hpa@...or.com>, Sachi King <nakato@...ato.io>,
        x86@...nel.org, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org
Subject: [PATCH] x86/i8259: Work around buggy legacy PIC

The legacy PIC on the AMD variant of the Microsoft Surface Laptop 4 has
some problems on boot. For some reason it consistently does not respond
on the first try, requiring a couple more tries before it finally
responds.

This currently leads to the PIC not being properly recognized, which
prevents interrupt handling down the line. Ultimately, this also leads
to the pinctrl-amd driver failing to probe due to platform_get_irq()
returning -EINVAL for its base IRQ. That, in turn, means that several
interrupts are not available and device drivers relying on those will
defer probing indefinitely, as querying those interrupts returns
-EPROBE_DEFER.

Add a quirk table and a retry-loop to work around that.

Also switch to pr_info() due to complaints by checkpatch and add a
pr_fmt() definition for completeness.

Cc: <stable@...r.kernel.org> # 5.10+
Co-developed-by: Sachi King <nakato@...ato.io>
Signed-off-by: Sachi King <nakato@...ato.io>
Signed-off-by: Maximilian Luz <luzmaximilian@...il.com>
---
 arch/x86/kernel/i8259.c | 51 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 46 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/i8259.c b/arch/x86/kernel/i8259.c
index 282b4ee1339f..0da757c6b292 100644
--- a/arch/x86/kernel/i8259.c
+++ b/arch/x86/kernel/i8259.c
@@ -1,4 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
+
+#define pr_fmt(fmt) "i8259: " fmt
+
 #include <linux/linkage.h>
 #include <linux/errno.h>
 #include <linux/signal.h>
@@ -16,6 +19,7 @@
 #include <linux/io.h>
 #include <linux/delay.h>
 #include <linux/pgtable.h>
+#include <linux/dmi.h>
 
 #include <linux/atomic.h>
 #include <asm/timer.h>
@@ -298,11 +302,39 @@ static void unmask_8259A(void)
 	raw_spin_unlock_irqrestore(&i8259A_lock, flags);
 }
 
+/*
+ * DMI table to identify devices with quirky probe behavior. See comment in
+ * probe_8259A() for more details.
+ */
+static const struct dmi_system_id retry_probe_quirk_table[] = {
+	{
+		.ident = "Microsoft Surface Laptop 4 (AMD)",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+			DMI_MATCH(DMI_PRODUCT_SKU, "Surface_Laptop_4_1952:1953")
+		},
+	},
+	{}
+};
+
 static int probe_8259A(void)
 {
 	unsigned long flags;
 	unsigned char probe_val = ~(1 << PIC_CASCADE_IR);
 	unsigned char new_val;
+	unsigned int i, imax = 1;
+
+	/*
+	 * Some systems have a legacy PIC that doesn't immediately respond
+	 * after boot. We know it's there, we know it should respond and is
+	 * required for proper interrupt handling later on, so let's try a
+	 * couple of times.
+	 */
+	if (dmi_check_system(retry_probe_quirk_table)) {
+		pr_warn("system with broken legacy PIC detected, re-trying multiple times if necessary\n");
+		imax = 10;
+	}
+
 	/*
 	 * Check to see if we have a PIC.
 	 * Mask all except the cascade and read
@@ -312,15 +344,24 @@ static int probe_8259A(void)
 	 */
 	raw_spin_lock_irqsave(&i8259A_lock, flags);
 
-	outb(0xff, PIC_SLAVE_IMR);	/* mask all of 8259A-2 */
-	outb(probe_val, PIC_MASTER_IMR);
-	new_val = inb(PIC_MASTER_IMR);
-	if (new_val != probe_val) {
-		printk(KERN_INFO "Using NULL legacy PIC\n");
+	for (i = 0; i < imax; i++) {
+		outb(0xff, PIC_SLAVE_IMR);	/* mask all of 8259A-2 */
+		outb(probe_val, PIC_MASTER_IMR);
+		new_val = inb(PIC_MASTER_IMR);
+		if (new_val == probe_val)
+			break;
+	}
+
+	if (i == imax) {
+		pr_info("using NULL legacy PIC\n");
 		legacy_pic = &null_legacy_pic;
 	}
 
 	raw_spin_unlock_irqrestore(&i8259A_lock, flags);
+
+	if (imax > 1 && i < imax)
+		pr_info("got legacy PIC after %d tries\n", i + 1);
+
 	return nr_legacy_irqs();
 }
 
-- 
2.31.1