On Wed, 2018-01-24 at 09:47 +0100, Peter Zijlstra wrote:
> Typically tglx likes to use x86_match_cpu() for these things; see also
> commit: bd9240a18edfb ("x86/apic: Add TSC_DEADLINE quirk due to
> errata").
Thanks, will fix. I think we might also end up in whitelist mode,
adding "known good" microcodes to the list as they get released or
retroactively blessed.
I would really have liked a new bit in IA32_ARCH_CAPABILITIES to say
that it's safe, but that's not possible for *existing* microcode which
actually turns out to be OK in the end.
That means the whitelist ends up basically empty right now. Should I
add a command line parameter to override it? Otherwise we end up having
to rebuild the kernel every time there's a microcode release which
covers a new CPU SKU (which is why I kind of hate the whitelist, but
Arjan is very insistent...)
I'm kind of tempted to turn it into a whitelist just by adding 1 to the
microcode revision in each table entry. Sure, that N+1 might be another
microcode build that also has issues but never saw the light of day...
but that's OK as long it never *does*. And yes we'd have to tweak it if
revisions that are blacklisted in the Intel doc are subsequently
cleared. But at least it'd require *less* tweaking.
> >
> > +
> > +static int bad_spectre_microcode(struct cpuinfo_x86 *c)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(spectre_bad_microcodes); i++) {
> > + if (c->x86_model == spectre_bad_microcodes[i].model &&
> > + c->x86_mask == spectre_bad_microcodes[i].stepping)
> > + return (c->microcode <= spectre_bad_microcodes[i].microcode);
> > + }
> > + return 0;
> > +}
> The above is Intel only, you should check vendor too I think.
It's in intel.c, called from early_init_intel(). Isn't that sufficient?
> >
> > static void early_init_intel(struct cpuinfo_x86 *c)
> > {
> > u64 misc_enable;
> > @@ -122,6 +173,18 @@ static void early_init_intel(struct cpuinfo_x86 *c)
> > if (c->x86 >= 6 && !cpu_has(c, X86_FEATURE_IA64))
> > c->microcode = intel_get_microcode_revision();
> >
> > + if ((cpu_has(c, X86_FEATURE_SPEC_CTRL) ||
> > + cpu_has(c, X86_FEATURE_AMD_SPEC_CTRL) ||
> > + cpu_has(c, X86_FEATURE_AMD_PRED_CMD) ||
> > + cpu_has(c, X86_FEATURE_AMD_STIBP)) && bad_spectre_microcode(c)) {
> > + pr_warn("Intel Spectre v2 broken microcode detected; disabling SPEC_CTRL\n");
> > + clear_cpu_cap(c, X86_FEATURE_SPEC_CTRL);
> > + clear_cpu_cap(c, X86_FEATURE_STIBP);
> > + clear_cpu_cap(c, X86_FEATURE_AMD_SPEC_CTRL);
> > + clear_cpu_cap(c, X86_FEATURE_AMD_PRED_CMD);
> > + clear_cpu_cap(c, X86_FEATURE_AMD_STIBP);
> > + }
> And since its Intel only, what are those AMD features doing there?
Hypervisors which only want to expose PRED_CMD may do so using the AMD
feature bit. SPEC_CTRL requires save/restore and live migration
support, and isn't needed with retpoline anyway (since guests won't be
calling directly into firmware).
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
