Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 11 Dec 2018 19:37:26 -0800 (PST)
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Borislav Petkov <bp@alien8.de>
cc:     "Lendacky, Thomas" <Thomas.Lendacky@amd.com>,
        "x86@kernel.org" <x86@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
        Jiri Kosina <jkosina@suse.cz>, Ingo Molnar <mingo@redhat.com>,
        Tim Chen <tim.c.chen@linux.intel.com>,
        David Woodhouse <dwmw@amazon.co.uk>
Subject: Re: [PATCH v2] x86/speculation: Add support for STIBP always-on
 preferred mode
In-Reply-To: <20181212000518.GX27375@zn.tnic>
Message-ID: <alpine.DEB.2.21.1812111910520.22893@nanos.tec.linutronix.de>
References: <20181211224615.5373.65736.stgit@tlendack-t1.amdoffice.net> <20181212000518.GX27375@zn.tnic>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, 12 Dec 2018, Borislav Petkov wrote:
> On Tue, Dec 11, 2018 at 10:46:16PM +0000, Lendacky, Thomas wrote:
> > +	/*
> > +	 * At this point, an STIBP mode other than "off" has been set.
> > +	 * If STIBP support is not being forced, check if STIBP always-on
> > +	 * is preferred.
> > +	 */
> > +	if (mode != SPECTRE_V2_USER_STRICT &&
> > +	    boot_cpu_has(X86_FEATURE_AMD_STIBP_ALWAYS_ON)) {
> > +		stibp_always_on = true;
> > +		mode = SPECTRE_V2_USER_STRICT;
> > +		pr_info("mitigation: STIBP always-on is preferred\n");
> > +	}
> > +
> >  	/* Initialize Indirect Branch Prediction Barrier */
> >  	if (boot_cpu_has(X86_FEATURE_IBPB)) {
> >  		setup_force_cpu_cap(X86_FEATURE_USE_IBPB);
> > @@ -1088,7 +1102,8 @@ static char *stibp_state(void)
> >  	case SPECTRE_V2_USER_NONE:
> >  		return ", STIBP: disabled";
> >  	case SPECTRE_V2_USER_STRICT:
> > -		return ", STIBP: forced";
> > +		return stibp_always_on ? ", STIBP: always-on"
> > +				       : ", STIBP: forced";
> 
> I still don't like that separate stibp_always_on variable when we can do
> all the querying just by using mode and X86_FEATURE_AMD_STIBP_ALWAYS_ON.

Hmmm. I've not seen the V1 of this (it's not in my inbox) but the v1->v2
changes contain:

> > - Removed explicit SPECTRE_V2_USER_STRICT_PREFERRED mode

Now I really have to ask why?

Neither the extra variable nor the cpu feature check are pretty. An
explicit mode is way better in terms of code clarity and you get the proper
printout via spectre_v2_user_strings.

Hmm?

	tglx