Re: [PATCH v10 6/7] x86/arch_prctl: Add ARCH_[GET|SET]_CPUID

From: Thomas Gleixner
Date: Wed Nov 09 2016 - 08:38:19 EST


On Wed, 9 Nov 2016, Borislav Petkov wrote:

> On Tue, Nov 08, 2016 at 09:06:31PM +0100, Thomas Gleixner wrote:
> > The upcoming ring3 mwait stuff can add its magic to tweak that MSR into
> > this function.
> >
> > Stick the call at the end of init_scattered_cpuid_features() for now. I
> > still need to figure out a proper place for it.
>
> So Thomas and I discussed this more on IRC and I think we can get rid
> of the MSR iterating in scattered.c and integrate both the R3 MWAIT and
> CPUID faulting like this:

I agree mostly.

> ---
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index fcd484d2bb03..5c38a85af2e7 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -452,6 +457,39 @@ static void intel_bsp_resume(struct cpuinfo_x86 *c)
> init_intel_energy_perf(c);
> }
>
> +static void init_misc_enables(struct cpuinfo_x86 *c)
> +{
> + u64 val, misc_en;
> +
> + if (rdmsrl_safe(MSR_MISC_FEATURES_ENABLES, &misc_en))
> + return;
> +
> + misc_en &= ~MSR_MISC_ENABLES_CPUID_FAULT_ENABLE;

I rather prefer to write this MSR to 0 right away and just enable the bits
which we really support.

Whatever Intel comes up with next, e.g. faulting of random other
instructions or whatever (mis)feature they think is valuable, will lead to
a debugging nightmare if some incompetent BIOS writer sets the bit and the
kernel does not know about it.

Yes, I know that there might be bits forced to 1 at some point in the
future, but let's deal with that when it happens.

Right now I can enable the CPUID FAULT bit on my broadwell and watch user
space programs die unexpectedly without a hint why. Simply because it's not
documented in the SDM. So we rather be safe than surprised.

@hpa: Thoughts?

Thanks,

tglx