linux-kernel - Re: [PATCH 04/11] x86/acpi: Check MWAIT feature instead of CPUID level

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]

Message-ID: <Zz9S352550TZSKBQ@google.com>
Date: Thu, 21 Nov 2024 07:33:51 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Dave Hansen <dave.hansen@...ux.intel.com>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org, tglx@...utronix.de, 
	bp@...en8.de, rafael@...nel.org, lenb@...nel.org, dave.jiang@...el.com, 
	irenic.rajneesh@...il.com, david.e.box@...el.com
Subject: Re: [PATCH 04/11] x86/acpi: Check MWAIT feature instead of CPUID level

On Wed, Nov 20, 2024, Dave Hansen wrote:
> 
> From: Dave Hansen <dave.hansen@...ux.intel.com>
> 
> I think this code is possibly buggy.  The CPU could have a high
> c->cpuid_level and not support MWAIT at all.

Reading CPUID.0x5 is totally fine in that case though.  Wasteful and pointless,
but functionally ok.  If the CPU provides non-zero values when MWAIT is unsupported,
then that's a broken CPU.

> It is much more clear to
> just check for MWAIT support directly.  Also, because of the CPU level
> dependency code, any CPU that has X86_FEATURE_MWAIT also has a
> high-enough CPUID level.

No?  The MWAIT feature flag is in leaf 1.  1 < 5, and I don't see any cpuid_level
magic that would force it beyond '5'.

Why not check both?

> Check X86_FEATURE_MWAIT instead of the CPUID level.
> 
> Signed-off-by: Dave Hansen <dave.hansen@...ux.intel.com>
> ---
> 
>  b/arch/x86/kernel/acpi/cstate.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff -puN arch/x86/kernel/acpi/cstate.c~mwait-leaf-checks-3 arch/x86/kernel/acpi/cstate.c
> --- a/arch/x86/kernel/acpi/cstate.c~mwait-leaf-checks-3	2024-11-20 11:44:17.225650902 -0800
> +++ b/arch/x86/kernel/acpi/cstate.c	2024-11-20 11:44:17.225650902 -0800
> @@ -173,7 +173,7 @@ int acpi_processor_ffh_cstate_probe(unsi
>  	struct cpuinfo_x86 *c = &cpu_data(cpu);
>  	long retval;
>  
> -	if (!cpu_cstate_entry || c->cpuid_level < CPUID_MWAIT_LEAF)
> +	if (!cpu_cstate_entry || cpu_has(c, X86_FEATURE_MWAIT))

Someone didn't test this :-)

>  		return -1;
>  
>  	if (reg->bit_offset != NATIVE_CSTATE_BEYOND_HALT)
> _