lkml.org 
[lkml]   [2018]   [Jan]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC 05/10] x86/speculation: Add basic IBRS support infrastructure
From
Date
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).

[unhandled content-type:application/x-pkcs7-signature]
\
 
 \ /
  Last update: 2018-01-24 10:04    [W:0.145 / U:3.716 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site