Skip to content

Commit 70523f3

Browse files
committed
Revert "x86/smp: Eliminate mwait_play_dead_cpuid_hint()"
Revert commit 96040f7 ("x86/smp: Eliminate mwait_play_dead_cpuid_hint()") because it introduced a significant power regression on systems that start with "nosmt" in the kernel command line. Namely, on such systems, SMT siblings permanently go offline early, when cpuidle has not been initialized yet, so after the above commit, hlt_play_dead() is called for them. Later on, when the processor attempts to enter a deep package C-state, including PC10 which is requisite for reaching minimum power in suspend-to-idle, it is not able to do that because of the SMT siblings staying in C1 (which they have been put into by HLT). As a result, the idle power (including power in suspend-to-idle) rises quite dramatically on those systems with all of the possible consequences, which (needless to say) may not be expected by their users. This issue is hard to debug and potentially dangerous, so it needs to be addressed as soon as possible in a way that will work for 6.15.y, hence the revert. Of course, after this revert, the issue that commit 96040f7 attempted to address will be back and it will need to be fixed again later. Fixes: 96040f7 ("x86/smp: Eliminate mwait_play_dead_cpuid_hint()") Reported-by: Todd Brandt <todd.e.brandt@linux.intel.com> Tested-by: Todd Brandt <todd.e.brandt@linux.intel.com> Cc: 6.15+ <stable@vger.kernel.org> # 6.15+ Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Acked-by: Dave Hansen <dave.hansen@linux.intel.com> Link: https://patch.msgid.link/12674167.O9o76ZdvQC@rjwysocki.net
1 parent 39cdf87 commit 70523f3

File tree

1 file changed

+47
-7
lines changed

1 file changed

+47
-7
lines changed

arch/x86/kernel/smpboot.c

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1238,10 +1238,6 @@ void play_dead_common(void)
12381238
local_irq_disable();
12391239
}
12401240

1241-
/*
1242-
* We need to flush the caches before going to sleep, lest we have
1243-
* dirty data in our caches when we come back up.
1244-
*/
12451241
void __noreturn mwait_play_dead(unsigned int eax_hint)
12461242
{
12471243
struct mwait_cpu_dead *md = this_cpu_ptr(&mwait_cpu_dead);
@@ -1287,6 +1283,50 @@ void __noreturn mwait_play_dead(unsigned int eax_hint)
12871283
}
12881284
}
12891285

1286+
/*
1287+
* We need to flush the caches before going to sleep, lest we have
1288+
* dirty data in our caches when we come back up.
1289+
*/
1290+
static inline void mwait_play_dead_cpuid_hint(void)
1291+
{
1292+
unsigned int eax, ebx, ecx, edx;
1293+
unsigned int highest_cstate = 0;
1294+
unsigned int highest_subcstate = 0;
1295+
int i;
1296+
1297+
if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD ||
1298+
boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
1299+
return;
1300+
if (!this_cpu_has(X86_FEATURE_MWAIT))
1301+
return;
1302+
if (!this_cpu_has(X86_FEATURE_CLFLUSH))
1303+
return;
1304+
1305+
eax = CPUID_LEAF_MWAIT;
1306+
ecx = 0;
1307+
native_cpuid(&eax, &ebx, &ecx, &edx);
1308+
1309+
/*
1310+
* eax will be 0 if EDX enumeration is not valid.
1311+
* Initialized below to cstate, sub_cstate value when EDX is valid.
1312+
*/
1313+
if (!(ecx & CPUID5_ECX_EXTENSIONS_SUPPORTED)) {
1314+
eax = 0;
1315+
} else {
1316+
edx >>= MWAIT_SUBSTATE_SIZE;
1317+
for (i = 0; i < 7 && edx; i++, edx >>= MWAIT_SUBSTATE_SIZE) {
1318+
if (edx & MWAIT_SUBSTATE_MASK) {
1319+
highest_cstate = i;
1320+
highest_subcstate = edx & MWAIT_SUBSTATE_MASK;
1321+
}
1322+
}
1323+
eax = (highest_cstate << MWAIT_SUBSTATE_SIZE) |
1324+
(highest_subcstate - 1);
1325+
}
1326+
1327+
mwait_play_dead(eax);
1328+
}
1329+
12901330
/*
12911331
* Kick all "offline" CPUs out of mwait on kexec(). See comment in
12921332
* mwait_play_dead().
@@ -1337,9 +1377,9 @@ void native_play_dead(void)
13371377
play_dead_common();
13381378
tboot_shutdown(TB_SHUTDOWN_WFS);
13391379

1340-
/* Below returns only on error. */
1341-
cpuidle_play_dead();
1342-
hlt_play_dead();
1380+
mwait_play_dead_cpuid_hint();
1381+
if (cpuidle_play_dead())
1382+
hlt_play_dead();
13431383
}
13441384

13451385
#else /* ... !CONFIG_HOTPLUG_CPU */

0 commit comments

Comments
 (0)