Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
From:   Zhang Rui <rui.zhang@intel.com>
To:     tglx@linutronix.de, bp@alien8.de, dave.hansen@linux.intel.com,
        mingo@redhat.com, rafael.j.wysocki@intel.com
Cc:     linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org,
        x86@kernel.org, peterz@infradead.org, jsperbeck@google.com,
        andres@anarazel.de
Subject: [PATCH] x86/acpi: Do strict X2APIC ID check only when LAPIC contains enabled CPUs
Date:   Sun, 10 Dec 2023 22:39:25 +0800
Message-Id: <20231210143925.38722-1-rui.zhang@intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Commit 8e9c42d776d6 ("x86/acpi: Ignore invalid x2APIC entries") does
strict X2APIC ID check when LAPIC contains valid CPUs. The code assumes
LAPIC contains valid CPUs when acpi_table_parse_madt(LAPIC) returns a
postive value.

This is wrong because acpi_table_parse_madt() return value only
represents the number of legal entries parsed. For example, LAPIC entry
with LAPIC ID 0xff is counted as a legal entry, but it doesn't describe
a valid CPU. And when all LAPIC entries have LAPIC ID 0xff, the return
value is positive but LAPIC does not contain any valid CPU. In this
case, kernel erroneously does strict X2APIC ID check and loses the CPUs
described in X2APIC entries.

Fix the problem by doing strict X2APIC ID check less aggressively, say
only when LAPIC contains enabled CPUs.

Fixes: 8e9c42d776d6 ("x86/acpi: Ignore invalid x2APIC entries")
Reported-by: John Sperbeck <jsperbeck@google.com>
Closes: https://lore.kernel.org/all/20231122221947.781812-1-jsperbeck@google.com/
Tested-by: John Sperbeck <jsperbeck@google.com>
Reported-by: Andres Freund <andres@anarazel.de>
Closes: https://lore.kernel.org/all/20231206065850.hs7k554v6wym7gw5@awork3.anarazel.de/
Tested-by: Andres Freund <andres@anarazel.de>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 arch/x86/kernel/acpi/boot.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 1a0dd80d81ac..c6495ebca91a 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -266,6 +266,7 @@ static int __init
 acpi_parse_lapic(union acpi_subtable_headers * header, const unsigned long end)
 {
 	struct acpi_madt_local_apic *processor = NULL;
+	int cpu;
 
 	processor = (struct acpi_madt_local_apic *)header;
 
@@ -289,9 +290,13 @@ acpi_parse_lapic(union acpi_subtable_headers * header, const unsigned long end)
 	 * to not preallocating memory for all NR_CPUS
 	 * when we use CPU hotplug.
 	 */
-	acpi_register_lapic(processor->id,	/* APIC ID */
-			    processor->processor_id, /* ACPI ID */
-			    processor->lapic_flags & ACPI_MADT_ENABLED);
+	cpu = acpi_register_lapic(processor->id,	/* APIC ID */
+				  processor->processor_id, /* ACPI ID */
+				  processor->lapic_flags & ACPI_MADT_ENABLED);
+
+	/* Do strict X2APIC ID check only when LAPIC contains enabled CPUs */
+	if (cpu >= 0)
+		has_lapic_cpus = true;
 
 	return 0;
 }
@@ -1134,7 +1139,6 @@ static int __init acpi_parse_madt_lapic_entries(void)
 	if (!count) {
 		count = acpi_table_parse_madt(ACPI_MADT_TYPE_LOCAL_APIC,
 					acpi_parse_lapic, MAX_LOCAL_APIC);
-		has_lapic_cpus = count > 0;
 		x2count = acpi_table_parse_madt(ACPI_MADT_TYPE_LOCAL_X2APIC,
 					acpi_parse_x2apic, MAX_LOCAL_APIC);
 	}
-- 
2.34.1