Re: [PATCH] x86/cpuid: Fix up "virtual" IBRS/IBPB/STIBP feature bits on Intel

[Thomas Gleixner]

On Tue, 30 Jan 2018, David Woodhouse wrote:
> On Tue, 2018-01-30 at 12:18 +0100, Borislav Petkov wrote:
> > On Tue, Jan 30, 2018 at 11:03:50AM +0000, David Woodhouse wrote:
> > > 
> > > I pondered that, but I didn't like it. I didn't want to always *force*
> > > those features on, for all CPUs, just because they happened to be
> > > discovered at boot time on the first CPU (which *did* have its
> > > microcode updated by the crappy BIOS, while the others didn't).
> > > 
> > > I strongly suspect that's purely an academic concern, and we mostly
> > > check boot_cpu_has() and never even *notice* if secondary CPUs don't
> > > match. I just didn't want to make that *worse*. It tickled my OCD.
> >
> > Well, you need to do it because those bits are AMD-specific and they are
> > not set in the Intel CPUID leaf and identify_cpu() towards the end takes
> > care of "ironing" all those bits out which are not part of the common
> > feature set and which get_cpu_cap() has *not* read out from CPUID.
> 
> I need to set them for each CPU which has the Intel hardware bits set,
> sure. I don't need to use setup_force_cpu_cap() to do it. The patch I
> sent was doing it for each CPU.
> 
> > It is one of those I-told-you-so moments when I suggested to make the
> > visible feature bits the artificial ones and have the *actual* hardware
> > ones set those.
> 
> We don't have artificial ones for the hardware capability, but yes I
> could add another three. I could add X86_FEATURE_IBRS which is a
> virtual bit, set when *either* X86_FEATURE_SPEC_CTRL (on Intel) or
> X86_FEATURE_AMD_IBRS (on AMD) is set.
> 
> But actually... that doesn't help, does it? Because early_init_intel()
> is still only called *once* for the boot CPU. Those software bits would
> be set... and perhaps not later cleared when identify_boot_cpu()
> happens later, but would they ever get set for secondary CPUs? The code
> to set those virtual bits would *still* need to live somewhere that
> will get called for secondary CPUs, as I've done in this patch.
> 
> I could use setup_force_cpu_cap() but I still don't like that, as
> discussed.
> 
> So no, I don't see why inventing three more "virtual" bits to precisely
> parallel the AMD bits would really make much difference.

In any case, if there is ucode mismatch between CPUs the whole thing is
hosed anyway no matter what. So can you please agree on a solution so we
can unbreak the current state of affairs?

Thanks,

	tglx