linux-kernel - Re: [PATCH v3 2/3] x86: Remove vendor checks from prefer_mwait_c1_over

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f1872469-8a4d-5300-2367-0f56fafcb0de@amd.com>
Date:   Mon, 23 May 2022 21:19:22 +0530
From:   Wyes Karny <wyes.karny@....com>
To:     Zhang Rui <rui.zhang@...el.com>,
        Dave Hansen <dave.hansen@...el.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 v3 2/3] x86: Remove vendor checks from
 prefer_mwait_c1_over_halt

Hello Rui,

On 5/20/2022 7:13 PM, Zhang Rui wrote:
> 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?

This decision tree is based on cpuid features, so if the processor
advertises MWAIT C1 support we would be choosing MWAIT.
If any vendor wants to avoid choosing MWAIT (even cpuid shows MWAIT C1
support), they can explicitly mention. Will add exceptions form them.

> 
>>>  
>>> -	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
>