From mboxrd@z Thu Jan  1 00:00:00 1970
Return-path: <tglx@linutronix.de>
Received: from hsi-kbw-5-158-153-52.hsi19.kabel-badenwuerttemberg.de
 ([5.158.153.52] helo=nanos.tec.linutronix.de)	by Galois.linutronix.de with
 esmtpsa (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256)	(Exim 4.80)	(envelope-from
 <tglx@linutronix.de>)	id 1fIrTX-0006y6-NQ	for speck@linutronix.de; Wed, 16
 May 2018 10:07:55 +0200
Date: Wed, 16 May 2018 10:07:55 +0200 (CEST)
From: Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [patch 03/15] Hidden 3
In-Reply-To: <20180516024930.GD18660@char.us.oracle.com>
Message-ID: <alpine.DEB.2.21.1805160955420.1627@nanos.tec.linutronix.de>
References: <20180513140048.543641807@linutronix.de>
 <20180513140538.470697861@linutronix.de>
 <20180516024930.GD18660@char.us.oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
To: speck@linutronix.de
List-ID: <speck.linutronix.de>

On Tue, 15 May 2018, speck for Konrad Rzeszutek Wilk wrote:
> On Sun, May 13, 2018 at 04:00:51PM +0200, speck for Thomas Gleixner wrote:
> > --- a/arch/x86/kernel/cpu/bugs.c
> > +++ b/arch/x86/kernel/cpu/bugs.c
> > @@ -64,7 +64,7 @@ void __init check_bugs(void)
> >  	 * have unknown values. AMD64_LS_CFG MSR is cached in the early AMD
> >  	 * init code as it is not enumerated and depends on the family.
> >  	 */
> > -	if (boot_cpu_has(X86_FEATURE_IBRS))
> > +	if (boot_cpu_has(X86_FEATURE_MSR_SPEC_CTRL))
> >  		rdmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
> >  
> >  	/* Select the proper spectre mitigation before patching alternatives */
> > @@ -145,7 +145,7 @@ u64 x86_spec_ctrl_get_default(void)
> >  {
> >  	u64 msrval = x86_spec_ctrl_base;
> >  
> > -	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
> > +	if (static_cpu_has(X86_FEATURE_SPEC_CTRL))
> 
> You are using the static_cpu_has which ends up doing alternative
> patching - neat.
> 
> But what if the machine at bootup has no SPEC_CTRL MSR support at all,
> and then we end up loading a new microcode with this and then suddenly
> SPEC_CTRL MSR is available?
> 
> Wouldn't the static_cpu_has in this scenario end up always returning false?
> 
> [And this may all be moot - perhaps we don't support this scenario?]

Allowing it after late micro code loading might work, but it just works
because x86_spec_ctrl_base is initialized to 0, which happens to be the
right default value. But it's not working by design.

Similarly, we don't reevaluate mitigation selection and command line
options after early boot, so just special casing it for virtualization does
not make much sense to me. 

We can change it back, but then then we need some way to check that
SPEC_CTRL_MSR reads 0 at detection time and audit everything else whether
it can run into weird inconsistent state as we have to deal with a
bunch of variants here.

I'm all for keeping it simple and just treat it like a CPU upgrade which
you can't do on a running system either.

Thanks,

	tglx