lkml.org 
[lkml]   [2024]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectAW: x86/topology: Handle bogus ACPI tables correctly
Date
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div class="elementToProof" style="font-family: Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
Hi Thomas,</div>
<div class="elementToProof" style="font-family: Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
<br>
</div>
<div class="elementToProof" style="font-family: Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
I can confirm that your revised patch works as well as your first one.</div>
<div class="elementToProof" style="font-family: Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
<br>
</div>
<div class="elementToProof" style="font-family: Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
Thanks!</div>
<div class="elementToProof" style="font-family: Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
Carsten</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>Von:</b> Thomas Gleixner &lt;tglx@linutronix.de&gt;<br>
<b>Gesendet:</b> Freitag, 17. Mai 2024 16:40<br>
<b>An:</b> Carsten Tolkmit &lt;ctolkmit@tng.de&gt;<br>
<b>Cc:</b> LKML &lt;linux-kernel@vger.kernel.org&gt;; x86@kernel.org &lt;x86@kernel.org&gt;<br>
<b>Betreff:</b> x86/topology: Handle bogus ACPI tables correctly</font>
<div>&nbsp;</div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">The ACPI specification clearly states how the processors should be<br>
enumerated in the MADT:<br>
<br>
&nbsp;&quot;To ensure that the boot processor is supported post initialization,<br>
&nbsp; two guidelines should be followed. The first is that OSPM should<br>
&nbsp; initialize processors in the order that they appear in the MADT. The<br>
&nbsp; second is that platform firmware should list the boot processor as the<br>
&nbsp; first processor entry in the MADT.<br>
&nbsp; ...<br>
&nbsp; Failure of OSPM implementations and platform firmware to abide by<br>
&nbsp; these guidelines can result in both unpredictable and non optimal<br>
&nbsp; platform operation.&quot;<br>
<br>
The kernel relies on that ordering to detect the real BSP on crash kernels<br>
which is important to avoid sending a INIT IPI to it as that would cause a<br>
full machine reset.<br>
<br>
On a Dell XPS 16 9640 the BIOS ignores this rule and enumerates the CPUs in<br>
the wrong order. As a consequence the kernel falsely detects a crash kernel<br>
and disables the corresponding CPU.<br>
<br>
Prevent this by checking the IA32_APICBASE MSR for the BSP bit on the boot<br>
CPU. If that bit is set, then the MADT based BSP detection can be safely<br>
ignored. If the kernel detects a mismatch between the BSP bit and the first<br>
enumerated MADT entry then emit a firmware bug message.<br>
<br>
This obviously also has to be taken into account when the boot APIC ID and<br>
the first enumerated APIC ID match. If the boot CPU does not have the BSP<br>
bit set in the APICBASE MSR then there is no way for the boot CPU to<br>
determine which of the CPUs is the real BSP. Sending an INIT to the real<br>
BSP would reset the machine so the only sane way to deal with that is to<br>
limit the number of CPUs to one and emit a corresponding warning message.<br>
<br>
Fixes: 5c5682b9f87a (&quot;x86/cpu: Detect real BSP on crash kernels&quot;)<br>
Reported-by: Carsten Tolkmit &lt;ctolkmit@ennit.de&gt;<br>
Signed-off-by: Thomas Gleixner &lt;tglx@linutronix.de&gt;<br>
Cc: stable@vger.kernel.org<br>
Closes: <a href="https://bugzilla.kernel.org/show_bug.cgi?id=218837">https://bugzilla.kernel.org/show_bug.cgi?id=218837</a><br>
---<br>
<br>
This is a slightly different solution than the initial patch I provided in<br>
the bugzilla. Carsten, can you please test that again?<br>
---<br>
&nbsp;arch/x86/kernel/cpu/topology.c |&nbsp;&nbsp; 43 ++++++++++++++++++++++++++++++++++++++---<br>
&nbsp;1 file changed, 40 insertions(+), 3 deletions(-)<br>
<br>
--- a/arch/x86/kernel/cpu/topology.c<br>
+++ b/arch/x86/kernel/cpu/topology.c<br>
@@ -128,6 +128,9 @@ static void topo_set_cpuids(unsigned int<br>
&nbsp;<br>
&nbsp;static __init bool check_for_real_bsp(u32 apic_id)<br>
&nbsp;{<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; bool is_bsp = false, has_apic_base = boot_cpu_data.x86 &gt;= 6;<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; u64 msr;<br>
+<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; /*<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * There is no real good way to detect whether this a kdump()<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * kernel, but except on the Voyager SMP monstrosity which is not<br>
@@ -144,17 +147,51 @@ static __init bool check_for_real_bsp(u3<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; if (topo_info.real_bsp_apic_id != BAD_APICID)<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; return false;<br>
&nbsp;<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; /*<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * Check whether the enumeration order is broken by evaluating the<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * BSP bit in the APICBASE MSR. If the CPU does not have the<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * APICBASE MSR then the BSP detection is not possible and the<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * kernel must rely on the firmware enumeration order.<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; */<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; if (has_apic_base) {<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; rdmsrl(MSR_IA32_APICBASE, msr);<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; is_bsp = !!(msr &amp; MSR_IA32_APICBASE_BSP);<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; }<br>
+<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; if (apic_id == topo_info.boot_cpu_apic_id) {<br>
-&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; topo_info.real_bsp_apic_id = apic_id;<br>
-&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; return false;<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; if (is_bsp || !has_apic_base) {<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; topo_info.real_bsp_apic_id = apic_id;<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; return false;<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; }<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; /*<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * If the boot APIC is enumerated first, but the APICBASE<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * MSR does not have the BSP bit set, then there is no way<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * to discover the real BSP here. Assume a crash kernel and<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * limit the number of CPUs to 1 as an INIT to the real BSP<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * would reset the machine.<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; */<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; pr_warn(&quot;Enumerated BSP APIC %x is not marked in APICBASE MSR\n&quot;, apic_id);<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; pr_warn(&quot;Assuming crash kernel. Limiting to one CPU to prevent machine INIT\n&quot;);<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; set_nr_cpu_ids(1);<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; goto fwbug;<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; }<br>
&nbsp;<br>
-&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; pr_warn(&quot;Boot CPU APIC ID not the first enumerated APIC ID: %x &gt; %x\n&quot;,<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; pr_warn(&quot;Boot CPU APIC ID not the first enumerated APIC ID: %x != %x\n&quot;,<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; topo_info.boot_cpu_apic_id, apic_id);<br>
+<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; if (is_bsp &amp;&amp; has_apic_base) {<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; topo_info.real_bsp_apic_id = topo_info.boot_cpu_apic_id;<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; goto fwbug;<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; }<br>
+<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; pr_warn(&quot;Crash kernel detected. Disabling real BSP to prevent machine INIT\n&quot;);<br>
&nbsp;<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; topo_info.real_bsp_apic_id = apic_id;<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; return true;<br>
+<br>
+fwbug:<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; pr_warn(FW_BUG &quot;APIC enumeration order not specification compliant\n&quot;);<br>
+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; return false;<br>
&nbsp;}<br>
&nbsp;<br>
&nbsp;static unsigned int topo_unit_count(u32 lvlid, enum x86_topology_domains at_level,<br>
<br>
</div>
</span></font></div>
<font style="font-family: Calibri, sans-serif; font-size: 11pt;">https://www.tng.de
<br>
<p>Executive board (Gesch&auml;ftsf&uuml;hrer):<br>
Dr. Sven Willert (CEO/Vorsitz), <br>
Gunnar Peter, Sven Schade,<br>
Carsten Tolkmit, Bernd Sontheimer<br>
</p>
<p>Amtsgericht Kiel HRB 6002 KI<br>
USt-ID: DE225201428<br>
Die Information &uuml;ber die Verarbeitung Ihrer Daten<br>
gem&auml;&szlig; Artikel 12 DSGVO k&ouml;nnen Sie unter https://www.tng.de/datenschutz/ abrufen.<br>
______________________________________________________________________ </p>
</font>
</body>
</html>[unhandled content-type:application/pkcs7-signature]
\
 
 \ /
  Last update: 2024-05-17 22:19    [W:0.142 / U:0.916 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site