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.
Signed-off-by: Zhenzhong Duan <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Boris Ostrovsky <[email protected]>
---
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;
+
if (c->x86_vendor != X86_VENDOR_INTEL)
return 0;
--
1.8.3.1
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+
> Signed-off-by: Zhenzhong Duan <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Boris Ostrovsky <[email protected]>
> ---
> 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?
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 <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> Cc: Borislav Petkov <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: "H. Peter Anvin" <[email protected]>
>> Cc: Boris Ostrovsky <[email protected]>
>> ---
>> 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