* [PATCH] x86: Don't use MWAIT if explicitly requested @ 2019-10-18 3:45 Zhenzhong Duan 2019-10-18 7:12 ` Peter Zijlstra 0 siblings, 1 reply; 3+ messages in thread From: Zhenzhong Duan @ 2019-10-18 3:45 UTC (permalink / raw) To: linux-kernel Cc: x86, tim.c.chen, Zhenzhong Duan, Thomas Gleixner, Borislav Petkov, Ingo Molnar, H. Peter Anvin, Boris Ostrovsky 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 <zhenzhong.duan@oracle.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Borislav Petkov <bp@alien8.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Boris Ostrovsky <boris.ostrovsky@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; + if (c->x86_vendor != X86_VENDOR_INTEL) return 0; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] x86: Don't use MWAIT if explicitly requested 2019-10-18 3:45 [PATCH] x86: Don't use MWAIT if explicitly requested Zhenzhong Duan @ 2019-10-18 7:12 ` Peter Zijlstra 2019-10-18 8:47 ` Zhenzhong Duan 0 siblings, 1 reply; 3+ messages in thread From: Peter Zijlstra @ 2019-10-18 7:12 UTC (permalink / raw) To: Zhenzhong Duan Cc: linux-kernel, x86, tim.c.chen, Thomas Gleixner, Borislav Petkov, Ingo Molnar, H. Peter Anvin, Boris Ostrovsky 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 <zhenzhong.duan@oracle.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Boris Ostrovsky <boris.ostrovsky@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? ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] x86: Don't use MWAIT if explicitly requested 2019-10-18 7:12 ` Peter Zijlstra @ 2019-10-18 8:47 ` Zhenzhong Duan 0 siblings, 0 replies; 3+ messages in thread From: Zhenzhong Duan @ 2019-10-18 8:47 UTC (permalink / raw) To: Peter Zijlstra Cc: linux-kernel, x86, tim.c.chen, Thomas Gleixner, Borislav Petkov, Ingo Molnar, H. Peter Anvin, Boris Ostrovsky 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.duan@oracle.com> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Borislav Petkov <bp@alien8.de> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: "H. Peter Anvin" <hpa@zytor.com> >> Cc: Boris Ostrovsky <boris.ostrovsky@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 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-10-18 8:48 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-18 3:45 [PATCH] x86: Don't use MWAIT if explicitly requested Zhenzhong Duan 2019-10-18 7:12 ` Peter Zijlstra 2019-10-18 8:47 ` Zhenzhong Duan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.