Messages in this thread | | | Subject | Re: [PATCH v3 2/3] x86: Remove vendor checks from prefer_mwait_c1_over_halt | From | Zhang Rui <> | Date | Fri, 20 May 2022 21:43:59 +0800 |
| |
On Thu, 2022-05-19 at 09:00 -0700, Dave Hansen wrote: > On 5/10/22 03:18, Wyes Karny wrote: > > 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;
I'm new to x86 code, a dumb question, what about the other vendors? with this patch, prefer_mwait_c1_over_halt() can return 1 for other vendors as well?
> > > > - 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; > > So, before, we pretty much just assume that all Intel CPUs with MWAIT > should use MWAIT C1. > > > - 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 least one > > + * MWAIT C1 substate present. > > + */ > > + return (edx & MWAIT_C1_SUBSTATE_MASK); > > } > > So, I guess the "If MWAIT extensions are not available" check is > consistent with the "always use it on Intel" behavior. > > But, this would change the behavior on Intel systems that both have > CPUID5_ECX_EXTENSIONS_SUPPORTED and do not set bits in > MWAIT_C1_SUBSTATE_MASK. > > Is that a problem or an improvement?
At least Intel processors since Nehalem have MWAIT C1 support. For elder ones, need to confirm with Len.
When no bits set in MWAIT_C1_SUBSTATE_MASK, it means MWAIT C1 is not available for some reason, let me check if I can make this happen or not in real life.
thanks, rui
| |