Skip to content

Commit 128b0c9

Browse files
committed
x86/i8259: Skip probing when ACPI/MADT advertises PCAT compatibility
David and a few others reported that on certain newer systems some legacy interrupts fail to work correctly. Debugging revealed that the BIOS of these systems leaves the legacy PIC in uninitialized state which makes the PIC detection fail and the kernel switches to a dummy implementation. Unfortunately this fallback causes quite some code to fail as it depends on checks for the number of legacy PIC interrupts or the availability of the real PIC. In theory there is no reason to use the PIC on any modern system when IO/APIC is available, but the dependencies on the related checks cannot be resolved trivially and on short notice. This needs lots of analysis and rework. The PIC detection has been added to avoid quirky checks and force selection of the dummy implementation all over the place, especially in VM guest scenarios. So it's not an option to revert the relevant commit as that would break a lot of other scenarios. One solution would be to try to initialize the PIC on detection fail and retry the detection, but that puts the burden on everything which does not have a PIC. Fortunately the ACPI/MADT table header has a flag field, which advertises in bit 0 that the system is PCAT compatible, which means it has a legacy 8259 PIC. Evaluate that bit and if set avoid the detection routine and keep the real PIC installed, which then gets initialized (for nothing) and makes the rest of the code with all the dependencies work again. Fixes: e179f69 ("x86, irq, pic: Probe for legacy PIC and set legacy_pic appropriately") Reported-by: David Lazar <[email protected]> Signed-off-by: Thomas Gleixner <[email protected]> Tested-by: David Lazar <[email protected]> Reviewed-by: Hans de Goede <[email protected]> Reviewed-by: Mario Limonciello <[email protected]> Cc: [email protected] Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218003 Link: https://lore.kernel.org/r/875y2u5s8g.ffs@tglx
1 parent b99d70c commit 128b0c9

File tree

3 files changed

+35
-8
lines changed

3 files changed

+35
-8
lines changed

arch/x86/include/asm/i8259.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ struct legacy_pic {
6969
void (*make_irq)(unsigned int irq);
7070
};
7171

72+
void legacy_pic_pcat_compat(void);
73+
7274
extern struct legacy_pic *legacy_pic;
7375
extern struct legacy_pic null_legacy_pic;
7476

arch/x86/kernel/acpi/boot.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,9 @@ static int __init acpi_parse_madt(struct acpi_table_header *table)
148148
pr_debug("Local APIC address 0x%08x\n", madt->address);
149149
}
150150

151+
if (madt->flags & ACPI_MADT_PCAT_COMPAT)
152+
legacy_pic_pcat_compat();
153+
151154
/* ACPI 6.3 and newer support the online capable bit. */
152155
if (acpi_gbl_FADT.header.revision > 6 ||
153156
(acpi_gbl_FADT.header.revision == 6 &&

arch/x86/kernel/i8259.c

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
*/
3333
static void init_8259A(int auto_eoi);
3434

35+
static bool pcat_compat __ro_after_init;
3536
static int i8259A_auto_eoi;
3637
DEFINE_RAW_SPINLOCK(i8259A_lock);
3738

@@ -299,15 +300,32 @@ static void unmask_8259A(void)
299300

300301
static int probe_8259A(void)
301302
{
303+
unsigned char new_val, probe_val = ~(1 << PIC_CASCADE_IR);
302304
unsigned long flags;
303-
unsigned char probe_val = ~(1 << PIC_CASCADE_IR);
304-
unsigned char new_val;
305+
306+
/*
307+
* If MADT has the PCAT_COMPAT flag set, then do not bother probing
308+
* for the PIC. Some BIOSes leave the PIC uninitialized and probing
309+
* fails.
310+
*
311+
* Right now this causes problems as quite some code depends on
312+
* nr_legacy_irqs() > 0 or has_legacy_pic() == true. This is silly
313+
* when the system has an IO/APIC because then PIC is not required
314+
* at all, except for really old machines where the timer interrupt
315+
* must be routed through the PIC. So just pretend that the PIC is
316+
* there and let legacy_pic->init() initialize it for nothing.
317+
*
318+
* Alternatively this could just try to initialize the PIC and
319+
* repeat the probe, but for cases where there is no PIC that's
320+
* just pointless.
321+
*/
322+
if (pcat_compat)
323+
return nr_legacy_irqs();
324+
305325
/*
306-
* Check to see if we have a PIC.
307-
* Mask all except the cascade and read
308-
* back the value we just wrote. If we don't
309-
* have a PIC, we will read 0xff as opposed to the
310-
* value we wrote.
326+
* Check to see if we have a PIC. Mask all except the cascade and
327+
* read back the value we just wrote. If we don't have a PIC, we
328+
* will read 0xff as opposed to the value we wrote.
311329
*/
312330
raw_spin_lock_irqsave(&i8259A_lock, flags);
313331

@@ -429,5 +447,9 @@ static int __init i8259A_init_ops(void)
429447

430448
return 0;
431449
}
432-
433450
device_initcall(i8259A_init_ops);
451+
452+
void __init legacy_pic_pcat_compat(void)
453+
{
454+
pcat_compat = true;
455+
}

0 commit comments

Comments
 (0)