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 <[email protected]>
---
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
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 <[email protected]>
> ---
>
> 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 <[email protected]>
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.
Commit-ID: 3df8d9208569ef0b2313e516566222d745f3b94b
Gitweb: http://git.kernel.org/tip/3df8d9208569ef0b2313e516566222d745f3b94b
Author: Andy Lutomirski <[email protected]>
AuthorDate: Thu, 15 Dec 2016 10:14:42 -0800
Committer: Thomas Gleixner <[email protected]>
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 <[email protected]>
Acked-by: Borislav Petkov <[email protected]>
Cc: Brian Gerst <[email protected]>
Link: http://lkml.kernel.org/r/6ea30c0e9daec21e488b54761881a6dfcf3e04d0.1481825597.git.luto@kernel.org
Signed-off-by: Thomas Gleixner <[email protected]>
---
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;
}
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 <[email protected]>
> AuthorDate: Thu, 15 Dec 2016 10:14:42 -0800
> Committer: Thomas Gleixner <[email protected]>
> 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 <[email protected]>
> Acked-by: Borislav Petkov <[email protected]>
> Cc: Brian Gerst <[email protected]>
> Link: http://lkml.kernel.org/r/6ea30c0e9daec21e488b54761881a6dfcf3e04d0.1481825597.git.luto@kernel.org
> Signed-off-by: Thomas Gleixner <[email protected]>
>
> ---
> 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.
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