On 2019/10/18 15:12, Peter Zijlstra wrote:
On Fri, Oct 18, 2019 at 11:45:54AM +0800, Zhenzhong Duan wrote:
If 'idle=nomwait' is specified or process matching what's in
processor_idle_dmi_table, we should't use MWAIT at bootup stage before
cpuidle driver loaded, even if it's preferred by default on Intel.
Add a check so that HALT instruction is used in those cases.
The comment in idle_setup():
/*
* If the boot option of "idle=nomwait" is added,
* it means that mwait will be disabled for CPU C2/C3
* states. In such case it won't touch the variable
* of boot_option_idle_override.
*/
boot_option_idle_override = IDLE_NOMWAIT;
explicitly states this option is for C2+
Yea, this is confusing. Other place referencing
boot_option_idle_override tell me
"idle=nomwait" means not using mwait for all cstates. Maybe 'C2/C3' could be
removed from above comment?
See drivers/acpi/processor_idle.c:
if (cx.type == ACPI_STATE_C1 &&
(boot_option_idle_override == IDLE_NOMWAIT)) {
/*
* In most cases the C1 space_id
obtained from
* _CST object is FIXED_HARDWARE access
mode.
* But when the option of idle=halt is
added,
* the entry_method type should be
changed from
* CSTATE_FFH to CSTATE_HALT.
* When the option of idle=nomwait is
added,
* the C1 entry_method type should be
* CSTATE_HALT.
*/
cx.entry_method = ACPI_CSTATE_HALT;
snprintf(cx.desc, ACPI_CX_DESC_LEN,
"ACPI HLT");
}
and drivers/acpi/processor_pdc.c:
if (boot_option_idle_override == IDLE_NOMWAIT) {
/*
* If mwait is disabled for CPU C-states, the C2C3_FFH
access
* mode will be disabled in the parameter of _PDC object.
* Of course C1_FFH access mode will also be disabled.
*/
union acpi_object *obj;
u32 *buffer = NULL;
obj = pdc_in->pointer;
buffer = (u32 *)(obj->buffer.pointer);
buffer[2] &= ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH);
}
Signed-off-by: Zhenzhong Duan <zhenzhong.d...@oracle.com>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: Borislav Petkov <b...@alien8.de>
Cc: Ingo Molnar <mi...@redhat.com>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: Boris Ostrovsky <boris.ostrov...@oracle.com>
---
arch/x86/kernel/process.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 5e94c43..37fc577 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -667,6 +667,10 @@ static void amd_e400_idle(void)
*/
static int prefer_mwait_c1_over_halt(const struct cpuinfo_x86 *c)
{
+ /* Don't use MWAIT-C1 if explicitly requested */
+ if (boot_option_idle_override == IDLE_NOMWAIT)
+ return 0;
And this is very much about C1...
OTOH, "idle=halt" should be forcing HLT over MWAIT, so did you want to
write:
if (boot_option_idle_override == IDLE_HALT)
return 0;
instead?
I think it's not necessory, if 'idle=halt' specified,
select_idle_routine() returns early,
prefer_mwait_c1_over_halt() is never called.
Zhenzhong
