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:7.604 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site