On 2/11/2025 4:54 PM, Andrew Cooper wrote:
> If you're going to override the BIOS setting, then you need to
> explicitly set MSR_MISC_ENABLE.FAST_STRINGS.
>
> Otherwise you're claiming to Linux that REP is good even when hardware
> is prohibited from using optimisations.
>
I think the current checks have unnecessary overlap which makes them
confusing. We should be fine if we only rely on the architectural
MSR_MISC_ENABLE.FAST_STRINGS bit and rely just on the BIOS setting. My
justification is below.
The simplified version of the current checks is as follows:
Check 1 (Based on Family Model numbers):
> /*
> * Unconditionally set REP_GOOD on early Family 6 processors
> */
> if (IS_ENABLED(CONFIG_X86_64) &&
> (c->x86_vfm >= INTEL_PENTIUM_PRO && c->x86_vfm < INTEL_PENTIUM_M_DOTHAN))
> set_cpu_cap(c, X86_FEATURE_REP_GOOD);
This check is mostly redundant since it is targeted for 64 bit and very
few if any of those CPUs support 64 bit processing. I suggest that we
get rid of this check completely. The risk here is fairly limited as well.
Check 2 (Based on MISC_ENABLE.FAST_STRING):
> /*
> * If fast string is not enabled in IA32_MISC_ENABLE for any reason,
> * clear the fast string and enhanced fast string CPU capabilities.
> */
> if (c->x86_vfm >= INTEL_PENTIUM_M_DOTHAN) {
> rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> if (misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING) {
> /* X86_FEATURE_ERMS will be automatically set based on CPUID */
> set_cpu_cap(c, X86_FEATURE_REP_GOOD);
> } else {
> pr_info("Disabled fast string operations\n");
> setup_clear_cpu_cap(X86_FEATURE_REP_GOOD);
> setup_clear_cpu_cap(X86_FEATURE_ERMS);
> }
> }
This is the only real check that is needed and should likely suffice in
all meaningful scenarios.
Comments?
