From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 27BCFC0032E for ; Wed, 25 Oct 2023 21:04:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229799AbjJYVEW (ORCPT ); Wed, 25 Oct 2023 17:04:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57244 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229573AbjJYVEV (ORCPT ); Wed, 25 Oct 2023 17:04:21 -0400 Received: from galois.linutronix.de (Galois.linutronix.de [IPv6:2a0a:51c0:0:12e:550::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AB4FD132 for ; Wed, 25 Oct 2023 14:04:18 -0700 (PDT) From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1698267856; h=from:from:reply-to:subject:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references; bh=TPuwtI1kuhVX5ITa/AAnqFvkxKf7BGaubhPzT2UNE74=; b=oPvdKVbHmRxor0QJdAoEkesozaykgzHyFM1Y8mU3ePFVB69v3gQLnMFAkRbhLlpN5CS55z qWen4rc0tqgxN46mTmAYGmoxo5MEBtcnIcqNkAfm10cnxWbUVWkKyNBmdCQMQ8h2SRViui BiHqQ24w7t9E42iO4ITFd9k/zjukwBAFS+X7Q/e0SbBMX76YTdZmy+Ydkni/i65DZDG8Dr eodkcIKfYLffamwhPx+mSg76M8C912BdJn94wyj8kLvYCEqw2/aqKi1bcVLaTpawAsYHqz EflhvV1qmlW4rfSQQiISBkKL01pbyVuiUX+jsYyBlCiwS7xAEfaLYJ4nqn22WA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1698267856; h=from:from:reply-to:subject:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references; bh=TPuwtI1kuhVX5ITa/AAnqFvkxKf7BGaubhPzT2UNE74=; b=9HWGUBsXPYxnzZJLR8evvLGcbzm0V7W3Y8sB5wd7WaVo/Q6mTIsupHnaHMydd5NP0E8BBG mUQNgRi2DUsivlDg== To: David Lazar , Mario Limonciello Cc: Hans de Goede , kys@microsoft.com, hpa@linux.intel.com, x86@kernel.org, LKML , Borislav Petkov , "Rafael J. Wysocki" Subject: [PATCH] x86/i8259: Skip probing when ACPI/MADT advertises PCAT compatibility Subject: In-Reply-To: <87jzra6235.ffs@tglx> References: <878r7z4kb4.ffs@tglx> <87ttqhcotn.ffs@tglx> <87v8avawe0.ffs@tglx> <32bcaa8a-0413-4aa4-97a0-189830da8654@amd.com> <87jzra6235.ffs@tglx> Date: Wed, 25 Oct 2023 23:04:15 +0200 Message-ID: <875y2u5s8g.ffs@tglx> MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 Signed-off-by: Thomas Gleixner Tested-by: David Lazar Cc: stable@vger.kernel.org Link: https://bugzilla.kernel.org/show_bug.cgi?id=218003 --- --- 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; +}