Skip to content

Commit fbe1bf1

Browse files
committed
Revert "x86/smp: Put CPUs into INIT on shutdown if possible"
This reverts commit 45e34c8, and the two subsequent fixes to it: 3f874c9 ("x86/smp: Don't send INIT to non-present and non-booted CPUs") b1472a6 ("x86/smp: Don't send INIT to boot CPU") because it seems to result in hung machines at shutdown. Particularly some Dell machines, but Thomas says "The rest seems to be Lenovo and Sony with Alderlake/Raptorlake CPUs - at least that's what I could figure out from the various bug reports. I don't know which CPUs the DELL machines have, so I can't say it's a pattern. I agree with the revert for now" Ashok Raj chimes in: "There was a report (probably this same one), and it turns out it was a bug in the BIOS SMI handler. The client BIOS's were waiting for the lowest APICID to be the SMI rendevous master. If this is MeteorLake, the BSP wasn't the one with the lowest APIC and it triped here. The BIOS change is also being pushed to others for assimilation :) Server BIOS's had this correctly for a while now" and it does look likely to be some bad interaction between SMI and the non-BSP cores having put into INIT (and thus unresponsive until reset). Link: https://bbs.archlinux.org/viewtopic.php?pid=2124429 Link: https://www.reddit.com/r/openSUSE/comments/16qq99b/tumbleweed_shutdown_did_not_finish_completely/ Link: https://forum.artixlinux.org/index.php/topic,5997.0.html Link: https://bugzilla.redhat.com/show_bug.cgi?id=2241279 Acked-by: Thomas Gleixner <[email protected]> Cc: Ashok Raj <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 5720c43 commit fbe1bf1

File tree

3 files changed

+7
-60
lines changed

3 files changed

+7
-60
lines changed

arch/x86/include/asm/smp.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,6 @@ void native_smp_send_reschedule(int cpu);
129129
void native_send_call_func_ipi(const struct cpumask *mask);
130130
void native_send_call_func_single_ipi(int cpu);
131131

132-
bool smp_park_other_cpus_in_init(void);
133132
void smp_store_cpu_info(int id);
134133

135134
asmlinkage __visible void smp_reboot_interrupt(void);

arch/x86/kernel/smp.c

Lines changed: 7 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ static int smp_stop_nmi_callback(unsigned int val, struct pt_regs *regs)
131131
}
132132

133133
/*
134-
* Disable virtualization, APIC etc. and park the CPU in a HLT loop
134+
* this function calls the 'stop' function on all other CPUs in the system.
135135
*/
136136
DEFINE_IDTENTRY_SYSVEC(sysvec_reboot)
137137
{
@@ -172,17 +172,13 @@ static void native_stop_other_cpus(int wait)
172172
* 2) Wait for all other CPUs to report that they reached the
173173
* HLT loop in stop_this_cpu()
174174
*
175-
* 3) If the system uses INIT/STARTUP for CPU bringup, then
176-
* send all present CPUs an INIT vector, which brings them
177-
* completely out of the way.
175+
* 3) If #2 timed out send an NMI to the CPUs which did not
176+
* yet report
178177
*
179-
* 4) If #3 is not possible and #2 timed out send an NMI to the
180-
* CPUs which did not yet report
181-
*
182-
* 5) Wait for all other CPUs to report that they reached the
178+
* 4) Wait for all other CPUs to report that they reached the
183179
* HLT loop in stop_this_cpu()
184180
*
185-
* #4 can obviously race against a CPU reaching the HLT loop late.
181+
* #3 can obviously race against a CPU reaching the HLT loop late.
186182
* That CPU will have reported already and the "have all CPUs
187183
* reached HLT" condition will be true despite the fact that the
188184
* other CPU is still handling the NMI. Again, there is no
@@ -198,35 +194,15 @@ static void native_stop_other_cpus(int wait)
198194
/*
199195
* Don't wait longer than a second for IPI completion. The
200196
* wait request is not checked here because that would
201-
* prevent an NMI/INIT shutdown in case that not all
197+
* prevent an NMI shutdown attempt in case that not all
202198
* CPUs reach shutdown state.
203199
*/
204200
timeout = USEC_PER_SEC;
205201
while (!cpumask_empty(&cpus_stop_mask) && timeout--)
206202
udelay(1);
207203
}
208204

209-
/*
210-
* Park all other CPUs in INIT including "offline" CPUs, if
211-
* possible. That's a safe place where they can't resume execution
212-
* of HLT and then execute the HLT loop from overwritten text or
213-
* page tables.
214-
*
215-
* The only downside is a broadcast MCE, but up to the point where
216-
* the kexec() kernel brought all APs online again an MCE will just
217-
* make HLT resume and handle the MCE. The machine crashes and burns
218-
* due to overwritten text, page tables and data. So there is a
219-
* choice between fire and frying pan. The result is pretty much
220-
* the same. Chose frying pan until x86 provides a sane mechanism
221-
* to park a CPU.
222-
*/
223-
if (smp_park_other_cpus_in_init())
224-
goto done;
225-
226-
/*
227-
* If park with INIT was not possible and the REBOOT_VECTOR didn't
228-
* take all secondary CPUs offline, try with the NMI.
229-
*/
205+
/* if the REBOOT_VECTOR didn't work, try with the NMI */
230206
if (!cpumask_empty(&cpus_stop_mask)) {
231207
/*
232208
* If NMI IPI is enabled, try to register the stop handler
@@ -249,7 +225,6 @@ static void native_stop_other_cpus(int wait)
249225
udelay(1);
250226
}
251227

252-
done:
253228
local_irq_save(flags);
254229
disable_local_APIC();
255230
mcheck_cpu_clear(this_cpu_ptr(&cpu_info));

arch/x86/kernel/smpboot.c

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1240,33 +1240,6 @@ void arch_thaw_secondary_cpus_end(void)
12401240
cache_aps_init();
12411241
}
12421242

1243-
bool smp_park_other_cpus_in_init(void)
1244-
{
1245-
unsigned int cpu, this_cpu = smp_processor_id();
1246-
unsigned int apicid;
1247-
1248-
if (apic->wakeup_secondary_cpu_64 || apic->wakeup_secondary_cpu)
1249-
return false;
1250-
1251-
/*
1252-
* If this is a crash stop which does not execute on the boot CPU,
1253-
* then this cannot use the INIT mechanism because INIT to the boot
1254-
* CPU will reset the machine.
1255-
*/
1256-
if (this_cpu)
1257-
return false;
1258-
1259-
for_each_cpu_and(cpu, &cpus_booted_once_mask, cpu_present_mask) {
1260-
if (cpu == this_cpu)
1261-
continue;
1262-
apicid = apic->cpu_present_to_apicid(cpu);
1263-
if (apicid == BAD_APICID)
1264-
continue;
1265-
send_init_sequence(apicid);
1266-
}
1267-
return true;
1268-
}
1269-
12701243
/*
12711244
* Early setup to make printk work.
12721245
*/

0 commit comments

Comments
 (0)