We were doing a fresh CPUID for every single bit in every single output
register. Do it once and then harvest *all* the bits we want.
We were also doing the max_level check with a new CPUID invocation for
every single bit. Stop that too.
Signed-off-by: David Woodhouse <[email protected]>
---
Spotted this in my travels; it offended me.
arch/x86/kernel/cpu/scattered.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index df11f5d..26c15fb 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -23,8 +23,8 @@ static const struct cpuid_bit cpuid_bits[] = {
{ X86_FEATURE_EPB, CPUID_ECX, 3, 0x00000006, 0 },
{ X86_FEATURE_CAT_L3, CPUID_EBX, 1, 0x00000010, 0 },
{ X86_FEATURE_CAT_L2, CPUID_EBX, 2, 0x00000010, 0 },
- { X86_FEATURE_CDP_L3, CPUID_ECX, 2, 0x00000010, 1 },
{ X86_FEATURE_MBA, CPUID_EBX, 3, 0x00000010, 0 },
+ { X86_FEATURE_CDP_L3, CPUID_ECX, 2, 0x00000010, 1 },
{ X86_FEATURE_HW_PSTATE, CPUID_EDX, 7, 0x80000007, 0 },
{ X86_FEATURE_CPB, CPUID_EDX, 9, 0x80000007, 0 },
{ X86_FEATURE_PROC_FEEDBACK, CPUID_EDX, 11, 0x80000007, 0 },
@@ -38,20 +38,35 @@ void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
u32 regs[4];
const struct cpuid_bit *cb;
+ /* Max level for basic CPUID leaves */
+ max_level = cpuid_eax(0);
+
for (cb = cpuid_bits; cb->feature; cb++) {
+ /* Are we on to extended CPUID leaves yet? */
+ if (cb->level >= 0x80000000 && max_level < 0x80000000) {
+ max_level = cpuid_eax(0x80000000);
+ if (max_level < 0x80000000 || max_level > 0x8000ffff) {
+ /* No extended leaves are supported */
+ break;
+ }
+ }
+
/* Verify that the level is valid */
- max_level = cpuid_eax(cb->level & 0xffff0000);
- if (max_level < cb->level ||
- max_level > (cb->level | 0xffff))
+ if (cb->level > max_level)
continue;
cpuid_count(cb->level, cb->sub_leaf, ®s[CPUID_EAX],
®s[CPUID_EBX], ®s[CPUID_ECX],
®s[CPUID_EDX]);
- if (regs[cb->reg] & (1 << cb->bit))
- set_cpu_cap(c, cb->feature);
+ do {
+ if (regs[cb->reg] & (1 << cb->bit))
+ set_cpu_cap(c, cb->feature);
+
+ } while (cb[1].level == cb[0].level &&
+ cb[1].sub_leaf == cb[0].sub_leaf &&
+ cb++);
}
}
--
2.7.4
On Tue, Jan 23, 2018 at 02:53:33PM +0000, David Woodhouse wrote:
> We were doing a fresh CPUID for every single bit in every single output
> register. Do it once and then harvest *all* the bits we want.
>
> We were also doing the max_level check with a new CPUID invocation for
> every single bit. Stop that too.
>
> Signed-off-by: David Woodhouse <[email protected]>
> ---
> Spotted this in my travels; it offended me.
Ok, I see that it's itching so let's scratch it properly :-)
If we're going to optimize scattered.c, let's do something like this:
* do CPUID for each function once.
* for each set bit in there, set feature flag
which means we'd have to change the data structure.
struct cpuid_leaf {
u32 level;
u32 sub_leaf;
struct cpuid_bit bits[];
};
and that last thing is:
struct cpuid_bit {
u16 feature;
u8 reg;
u8 bit;
};
So that you have something like (for example with leaf 0x10):
struct cpuid_leaf leafs[] = {
...
{
.level = 0x00000010,
.sub_leaf = 0,
.bits = {
{ X86_FEATURE_CAT_L3, CPUID_EBX, 1 },
{ X86_FEATURE_CAT_L2, CPUID_EBX, 2 },
{ X86_FEATURE_MBA , CPUID_EBX, 3 },
{ 0 }
}
}
...
}
This way you get the CPUID only once and then iterate over bits[] and
you can do the cleaner max level computation
cpuid_eax(level & 0xffff0000);
without having to do the extended level checks.
Anyway, something like that.
It's probably not even worth doing anything though - I doubt the speedup
is visible at all.
But I certainly understand the intent to fix an annoying thing like that. :-))
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
On Tue, Jan 23, 2018 at 06:32:16PM +0100, Borislav Petkov wrote:
> It's probably not even worth doing anything though - I doubt the speedup
> is visible at all.
One more thing I forgot to mention yesterday: I'm working on changing
the CPUID parsing we do now and we'll probably end up simply reading in
all CPUID leafs so scattered.c will disappear eventually. So I wouldn't
waste my energy on it.
:-)
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.