Messages in this thread | | | Date | Thu, 5 May 2022 10:04:55 -0700 | Subject | Re: [PATCH v2 2/3] x86: Remove vendor checks from prefer_mwait_c1_over_halt | From | Dave Hansen <> |
| |
On 5/5/22 04:01, Wyes Karny wrote: > - 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; > + > + /* Monitor has a bug. Fallback to HALT */ > + if (boot_cpu_has_bug(X86_BUG_MONITOR)) > return 0; > > - if (!cpu_has(c, X86_FEATURE_MWAIT) || boot_cpu_has_bug(X86_BUG_MONITOR)) > + if (c->cpuid_level < CPUID_MWAIT_LEAF) > return 0;
First of all, thanks for all the detail in this new version of the patches!
In arch/x86/kernel/cpu/common.c, we have:
cpuid_dependent_features[] = { { X86_FEATURE_MWAIT, 0x00000005 }, ...
Shouldn't that clear X86_FEATURE_MWAIT on all systems without CPUID_MWAIT_LEAF? That would make the c->cpuid_level check above unnecessary.
> + /* > + * If ECX doesn't have extended info about MWAIT, > + * don't need to check substates. > + */ > + if (!(ecx & CPUID5_ECX_EXTENSIONS_SUPPORTED)) > + return 1;
Could you explain a bit more about this? I don't know this CPUID leaf off the top of my head and the line after this is checking EDX. It's not apparent from this comment why "!ECX_EXTENSIONS_SUPPORTED" means that MWAIT should be preferred.
> + /* Check, whether at least 1 MWAIT C1 substate is present */ > + return (edx & MWAIT_C1_SUBSTATE_MASK);
| |