[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 5 May 2022 10:04:55 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: Wyes Karny <wyes.karny@....com>, linux-kernel@...r.kernel.org
Cc: Lewis.Carroll@....com, Mario.Limonciello@....com,
gautham.shenoy@....com, Ananth.Narayan@....com, bharata@....com,
len.brown@...el.com, x86@...nel.org, tglx@...utronix.de,
mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
hpa@...or.com, peterz@...radead.org, chang.seok.bae@...el.com,
keescook@...omium.org, metze@...ba.org, zhengqi.arch@...edance.com,
mark.rutland@....com, puwen@...on.cn, rafael.j.wysocki@...el.com,
andrew.cooper3@...rix.com, jing2.liu@...el.com,
jmattson@...gle.com, pawan.kumar.gupta@...ux.intel.com
Subject: Re: [PATCH v2 2/3] x86: Remove vendor checks from
prefer_mwait_c1_over_halt
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);
Powered by blists - more mailing lists