| 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
| ||
|
Message-ID: <ad3affd0-58d3-41b3-8014-cb0fc953dc6d@amd.com>
Date: Wed, 25 Oct 2023 17:11:53 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: Thomas Gleixner <tglx@...utronix.de>,
David Lazar <dlazar@...il.com>
Cc: Hans de Goede <hdegoede@...hat.com>, kys@...rosoft.com,
hpa@...ux.intel.com, x86@...nel.org,
LKML <linux-kernel@...r.kernel.org>,
Borislav Petkov <bp@...en8.de>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Linux kernel regressions list <regressions@...ts.linux.dev>
Subject: Re:
On 10/25/2023 16:04, Thomas Gleixner wrote:
> 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: e179f6914152 ("x86, irq, pic: Probe for legacy PIC and set legacy_pic appropriately")
> Reported-by: David Lazar <dlazar@...il.com>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> Tested-by: David Lazar <dlazar@...il.com>
> Cc: stable@...r.kernel.org
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=218003
s/Link/Closes/
Presumably you will add a proper subject when this is committed?
With adding title and fixing that tag:
Reviewed-by: Mario Limonciello <mario.limonciello@....com>
> ---
> ---
> arch/x86/include/asm/i8259.h | 2 ++
> arch/x86/kernel/acpi/boot.c | 3 +++
> arch/x86/kernel/i8259.c | 38 ++++++++++++++++++++++++++++++--------
> 3 files changed, 35 insertions(+), 8 deletions(-)
>
> --- a/arch/x86/include/asm/i8259.h
> +++ b/arch/x86/include/asm/i8259.h
> @@ -69,6 +69,8 @@ struct legacy_pic {
> void (*make_irq)(unsigned int irq);
> };
>
> +void legacy_pic_pcat_compat(void);
> +
> extern struct legacy_pic *legacy_pic;
> extern struct legacy_pic null_legacy_pic;
>
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -148,6 +148,9 @@ static int __init acpi_parse_madt(struct
> pr_debug("Local APIC address 0x%08x\n", madt->address);
> }
>
> + if (madt->flags & ACPI_MADT_PCAT_COMPAT)
> + legacy_pic_pcat_compat();
> +
> /* ACPI 6.3 and newer support the online capable bit. */
> if (acpi_gbl_FADT.header.revision > 6 ||
> (acpi_gbl_FADT.header.revision == 6 &&
> --- a/arch/x86/kernel/i8259.c
> +++ b/arch/x86/kernel/i8259.c
> @@ -32,6 +32,7 @@
> */
> static void init_8259A(int auto_eoi);
>
> +static bool pcat_compat __ro_after_init;
> static int i8259A_auto_eoi;
> DEFINE_RAW_SPINLOCK(i8259A_lock);
>
> @@ -299,15 +300,32 @@ static void unmask_8259A(void)
>
> static int probe_8259A(void)
> {
> + unsigned char new_val, probe_val = ~(1 << PIC_CASCADE_IR);
> unsigned long flags;
> - unsigned char probe_val = ~(1 << PIC_CASCADE_IR);
> - unsigned char new_val;
> +
> + /*
> + * If MADT has the PCAT_COMPAT flag set, then do not bother probing
> + * for the PIC. Some BIOSes leave the PIC uninitialized and probing
> + * fails.
> + *
> + * Right now this causes problems as quite some code depends on
> + * nr_legacy_irqs() > 0 or has_legacy_pic() == true. This is silly
> + * when the system has an IO/APIC because then PIC is not required
> + * at all, except for really old machines where the timer interrupt
> + * must be routed through the PIC. So just pretend that the PIC is
> + * there and let legacy_pic->init() initialize it for nothing.
> + *
> + * Alternatively this could just try to initialize the PIC and
> + * repeat the probe, but for cases where there is no PIC that's
> + * just pointless.
> + */
> + if (pcat_compat)
> + return nr_legacy_irqs();
> +
> /*
> - * Check to see if we have a PIC.
> - * Mask all except the cascade and read
> - * back the value we just wrote. If we don't
> - * have a PIC, we will read 0xff as opposed to the
> - * value we wrote.
> + * Check to see if we have a PIC. Mask all except the cascade and
> + * read back the value we just wrote. If we don't have a PIC, we
> + * will read 0xff as opposed to the value we wrote.
> */
> raw_spin_lock_irqsave(&i8259A_lock, flags);
>
> @@ -429,5 +447,9 @@ static int __init i8259A_init_ops(void)
>
> return 0;
> }
> -
> device_initcall(i8259A_init_ops);
> +
> +void __init legacy_pic_pcat_compat(void)
> +{
> + pcat_compat = true;
> +}
Powered by blists - more mailing lists