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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 19 May 2022 09:00:32 -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 v3 2/3] x86: Remove vendor checks from
 prefer_mwait_c1_over_halt

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