lkml.org 
[lkml]   [2022]   [May]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2 2/3] x86: Remove vendor checks from prefer_mwait_c1_over_halt
From
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);

\
 
 \ /
  Last update: 2022-05-05 19:06    [W:0.235 / U:0.556 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site