[PATCH] x86/cpu: Probe CPUID leaf 6 even when cpuid_level == 6

[PATCH] x86/cpu: Probe CPUID leaf 6 even when cpuid_level == 6

[Andy Lutomirski]

A typo (or mis-merge?) resulted in leaf 6 only being probed if
cpuid_level >= 7.

Fixes: 2ccd71f1b278 ("x86/cpufeature: Move some of the scattered feature bits to x86_capability")
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---

I don't know what CPUs, if any, are affected, and the bug is minor anyway,
so the major advantage of applying this is to remove a head-scratcher when
reading the code, I think.

 arch/x86/kernel/cpu/common.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index cc9e980c68ec..d61722821b7b 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -667,13 +667,14 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
 		c->x86_capability[CPUID_1_EDX] = edx;
 	}
 
+	/* Thermal and Power Management Leaf: level 0x00000006 (eax) */
+	if (c->cpuid_level >= 0x00000006)
+		c->x86_capability[CPUID_6_EAX] = cpuid_eax(0x00000006);
+
 	/* Additional Intel-defined flags: level 0x00000007 */
 	if (c->cpuid_level >= 0x00000007) {
 		cpuid_count(0x00000007, 0, &eax, &ebx, &ecx, &edx);
-
 		c->x86_capability[CPUID_7_0_EBX] = ebx;
-
-		c->x86_capability[CPUID_6_EAX] = cpuid_eax(0x00000006);
 		c->x86_capability[CPUID_7_ECX] = ecx;
 	}
 
-- 
2.9.3

Re: [PATCH] x86/cpu: Probe CPUID leaf 6 even when cpuid_level == 6

[Borislav Petkov]

On Thu, Dec 15, 2016 at 10:14:42AM -0800, Andy Lutomirski wrote:
> A typo (or mis-merge?) resulted in leaf 6 only being probed if
> cpuid_level >= 7.
> 
> Fixes: 2ccd71f1b278 ("x86/cpufeature: Move some of the scattered feature bits to x86_capability")
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
> 
> I don't know what CPUs, if any, are affected, and the bug is minor anyway,
> so the major advantage of applying this is to remove a head-scratcher when
> reading the code, I think.
> 
>  arch/x86/kernel/cpu/common.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index cc9e980c68ec..d61722821b7b 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -667,13 +667,14 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
>  		c->x86_capability[CPUID_1_EDX] = edx;
>  	}
>  
> +	/* Thermal and Power Management Leaf: level 0x00000006 (eax) */
> +	if (c->cpuid_level >= 0x00000006)
> +		c->x86_capability[CPUID_6_EAX] = cpuid_eax(0x00000006);
> +
>  	/* Additional Intel-defined flags: level 0x00000007 */
>  	if (c->cpuid_level >= 0x00000007) {
>  		cpuid_count(0x00000007, 0, &eax, &ebx, &ecx, &edx);
> -
>  		c->x86_capability[CPUID_7_0_EBX] = ebx;
> -
> -		c->x86_capability[CPUID_6_EAX] = cpuid_eax(0x00000006);
>  		c->x86_capability[CPUID_7_ECX] = ecx;
>  	}
>  
> -- 

Yeah,

Acked-by: Borislav Petkov <bp@suse.de>

However, as we talked already, this whole code needs a serious cleanup
so that it reads the max leaf and then read *all* and cache those we
need and do it only once.

Future work anyway.

Thanks.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[tip:x86/urgent] x86/cpu: Probe CPUID leaf 6 even when cpuid_level == 6

[tip-bot for Andy Lutomirski]

Commit-ID:  3df8d9208569ef0b2313e516566222d745f3b94b
Gitweb:     http://git.kernel.org/tip/3df8d9208569ef0b2313e516566222d745f3b94b
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Thu, 15 Dec 2016 10:14:42 -0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 19 Dec 2016 11:50:24 +0100

x86/cpu: Probe CPUID leaf 6 even when cpuid_level == 6

A typo (or mis-merge?) resulted in leaf 6 only being probed if
cpuid_level >= 7.

Fixes: 2ccd71f1b278 ("x86/cpufeature: Move some of the scattered feature bits to x86_capability")
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Acked-by: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Link: http://lkml.kernel.org/r/6ea30c0e9daec21e488b54761881a6dfcf3e04d0.1481825597.git.luto@kernel.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/kernel/cpu/common.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 1f6b50a..dc1697c 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -667,13 +667,14 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
 		c->x86_capability[CPUID_1_EDX] = edx;
 	}
 
+	/* Thermal and Power Management Leaf: level 0x00000006 (eax) */
+	if (c->cpuid_level >= 0x00000006)
+		c->x86_capability[CPUID_6_EAX] = cpuid_eax(0x00000006);
+
 	/* Additional Intel-defined flags: level 0x00000007 */
 	if (c->cpuid_level >= 0x00000007) {
 		cpuid_count(0x00000007, 0, &eax, &ebx, &ecx, &edx);
-
 		c->x86_capability[CPUID_7_0_EBX] = ebx;
-
-		c->x86_capability[CPUID_6_EAX] = cpuid_eax(0x00000006);
 		c->x86_capability[CPUID_7_ECX] = ecx;
 	}
 

Re: [tip:x86/urgent] x86/cpu: Probe CPUID leaf 6 even when cpuid_level == 6

[H. Peter Anvin]

On 12/19/16 02:56, tip-bot for Andy Lutomirski wrote:
> Commit-ID:  3df8d9208569ef0b2313e516566222d745f3b94b
> Gitweb:     http://git.kernel.org/tip/3df8d9208569ef0b2313e516566222d745f3b94b
> Author:     Andy Lutomirski <luto@kernel.org>
> AuthorDate: Thu, 15 Dec 2016 10:14:42 -0800
> Committer:  Thomas Gleixner <tglx@linutronix.de>
> CommitDate: Mon, 19 Dec 2016 11:50:24 +0100
> 
> x86/cpu: Probe CPUID leaf 6 even when cpuid_level == 6
> 
> A typo (or mis-merge?) resulted in leaf 6 only being probed if
> cpuid_level >= 7.
> 
> Fixes: 2ccd71f1b278 ("x86/cpufeature: Move some of the scattered feature bits to x86_capability")
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> Acked-by: Borislav Petkov <bp@alien8.de>
> Cc: Brian Gerst <brgerst@gmail.com>
> Link: http://lkml.kernel.org/r/6ea30c0e9daec21e488b54761881a6dfcf3e04d0.1481825597.git.luto@kernel.org
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> 
> ---
>  arch/x86/kernel/cpu/common.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 1f6b50a..dc1697c 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -667,13 +667,14 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
>  		c->x86_capability[CPUID_1_EDX] = edx;
>  	}
>  
> +	/* Thermal and Power Management Leaf: level 0x00000006 (eax) */
> +	if (c->cpuid_level >= 0x00000006)
> +		c->x86_capability[CPUID_6_EAX] = cpuid_eax(0x00000006);
> +
>  	/* Additional Intel-defined flags: level 0x00000007 */
>  	if (c->cpuid_level >= 0x00000007) {
>  		cpuid_count(0x00000007, 0, &eax, &ebx, &ecx, &edx);
> -
>  		c->x86_capability[CPUID_7_0_EBX] = ebx;
> -
> -		c->x86_capability[CPUID_6_EAX] = cpuid_eax(0x00000006);
>  		c->x86_capability[CPUID_7_ECX] = ecx;
>  	}

Perhaps we should have something like:

void cpuid_cond(u32 leaf, u32 subleaf,
	 	u32 *eax, u32 *ebx, u32 *ecx, u32 *edx)
{
	const struct cpuinfo_x86 *c = this_cpu_ptr(&cpu_info);
	u32 maxleaf;
	int cpuid_level = c->cpuid_level;

	switch (leaf >> 16) {
		case 0x0000:
			maxleaf = c->cpuid_level;
			break;
		case 0x8000:
			maxleaf = c->extended_cpuid_level;
			break;
		case 0x4000 .. 0x40ff:
			/* Not ideal, can we do better? */
			maxleaf = cpu_has(X86_FEATURE_HYPERVISOR)
				  ? 0x40ffffff : 0;
			break;
		/* Add Transmeta 0x8086 and VIA/Cyrix 0xc000? */
		default:
			/* Can we do better? */
			if (c->x86_vendor_id) == X86_VENDOR_INTEL)
				maxleaf = 0;
			else
				maxleaf = leaf | 0xffff;
			break;
	}

	if (cpuid_level >= 0 && leaf >= maxleaf) {
		cpuid_count(leaf, subleaf, eax, ebx, ecx, edx);
	} else {
		*eax = *ebx = *ecx = *edx = 0;
	}
}

	-hpa

P.S. I would love to (a) move the CPUID bits into a structure instead of
passing all those crazy pointers around, and (b) stop passing struct
cpuinfo * around when we only use it for the current processor anyway (a
lot of these functions are in fact completely invalid if we don't); we
could define this_cpu_info as (*this_cpu_ptr(&cpu_info)) -- basically
what I have above -- or directly use percpu functions to access these
variables.

Re: [tip:x86/urgent] x86/cpu: Probe CPUID leaf 6 even when cpuid_level == 6

[Thomas Gleixner]

On Mon, 19 Dec 2016, H. Peter Anvin wrote:
> P.S. I would love to (a) move the CPUID bits into a structure instead of
> passing all those crazy pointers around, and (b) stop passing struct
> cpuinfo * around when we only use it for the current processor anyway (a
> lot of these functions are in fact completely invalid if we don't); we
> could define this_cpu_info as (*this_cpu_ptr(&cpu_info)) -- basically
> what I have above -- or directly use percpu functions to access these
> variables.

The whole cpu identify code wants to be rewritten completely. It's full of
conditionals and places which read the same cpuid leaf over and over. The
whole thing is just insane.

The proper solution is to have a single function which reads all available
leafs into a data structure and then do all the conditional computations
and decisions based on that.

Thanks,

	tglx