Messages in this thread | | | Date | Wed, 25 May 2022 18:55:48 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v4 2/3] x86: Remove vendor checks from prefer_mwait_c1_over_halt |
| |
On Mon, May 23, 2022 at 10:25:50PM +0530, Wyes Karny wrote:
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c > index 4e0178b066c5..7bf4d73c9522 100644 > --- a/arch/x86/kernel/process.c > +++ b/arch/x86/kernel/process.c > @@ -813,28 +813,43 @@ static void amd_e400_idle(void) > } > > /* > - * Intel Core2 and older machines prefer MWAIT over HALT for C1. > - * We can't rely on cpuidle installing MWAIT, because it will not load > - * on systems that support only C1 -- so the boot default must be MWAIT. > + * Prefer MWAIT over HALT if MWAIT is supported, MWAIT_CPUID leaf > + * exists and whenever MONITOR/MWAIT extensions are present there is at > + * least one C1 substate. > * > - * Some AMD machines are the opposite, they depend on using HALT. > - * > - * So for default C1, which is used during boot until cpuidle loads, > - * use MWAIT-C1 on Intel HW that has it, else use HALT. > + * Do not prefer MWAIT if MONITOR instruction has a bug or idle=nomwait > + * is passed to kernel commandline parameter. > */ > static int prefer_mwait_c1_over_halt(const struct cpuinfo_x86 *c) > { > + u32 eax, ebx, ecx, edx; > + > /* User has disallowed the use of MWAIT. Fallback to HALT */ > if (boot_option_idle_override == IDLE_NOMWAIT) > return 0; > > - if (c->x86_vendor != X86_VENDOR_INTEL) > + /* MWAIT is not supported on this platform. Fallback to HALT */ > + if (!cpu_has(c, X86_FEATURE_MWAIT)) > return 0; > > - if (!cpu_has(c, X86_FEATURE_MWAIT) || boot_cpu_has_bug(X86_BUG_MONITOR)) > + /* Monitor has a bug. Fallback to HALT */ > + if (boot_cpu_has_bug(X86_BUG_MONITOR)) > return 0; > > - return 1; > + cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &edx); > + > + /* > + * If MWAIT extensions are not available, it is safe to use MWAIT > + * with EAX=0, ECX=0. > + */ > + if (!(ecx & CPUID5_ECX_EXTENSIONS_SUPPORTED)) > + return 1; > + > + /* > + * If MWAIT extensions are available, there should be at least one > + * MWAIT C1 substate present. > + */ > + return (edx & MWAIT_C1_SUBSTATE_MASK); > }
Seems reasonable enough to me,
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
| |