Date: Fri, 5 Jan 2018 16:28:17 +0100
From: Andrea Arcangeli <aarcange@redhat.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Borislav Petkov <bp@alien8.de>,
        Tim Chen <tim.c.chen@linux.intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Andy Lutomirski <luto@kernel.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Dave Hansen <dave.hansen@intel.com>,
        Andi Kleen <ak@linux.intel.com>,
        Arjan Van De Ven <arjan.van.de.ven@intel.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 7/7] x86/microcode: Recheck IBRS features on microcode
 reload
Message-ID: <20180105152817.GM26807@redhat.com>
References: <cover.1515086770.git.tim.c.chen@linux.intel.com>
 <a2092889d6d2198494d97a341ea0375a95d3b3bd.1515086770.git.tim.c.chen@linux.intel.com>
 <20180104182858.2nevvgeq5wdojz2w@pd.tnic>
 <20180104183430.GJ13348@redhat.com>
 <20180104185033.tsbijx32as6wqvye@pd.tnic>
 <20180105133217.GB12036@kroah.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180105133217.GB12036@kroah.com>
User-Agent: Mutt/1.9.2 (2017-12-15)
Sender: linux-kernel-owner@vger.kernel.org

Hello everyone,

On Fri, Jan 05, 2018 at 02:32:17PM +0100, Greg Kroah-Hartman wrote:
> On Thu, Jan 04, 2018 at 07:50:33PM +0100, Borislav Petkov wrote:
> > On Thu, Jan 04, 2018 at 07:34:30PM +0100, Andrea Arcangeli wrote:
> > > void spec_ctrl_rescan_cpuid(void)
> > > {
> > > 	int cpu;
> > > 
> > > 	if (use_ibp_disable)
> > > 		return;
> > > 	mutex_lock(&spec_ctrl_mutex);
> > > 	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL ||
> > > 	    boot_cpu_data.x86_vendor == X86_VENDOR_AMD) {
> > > 		/* detect spec ctrl related cpuid additions */
> > > 		init_scattered_cpuid_features(&boot_cpu_data);
> > 
> > You don't need to noodle through all the scattered features - just the
> > two bits.
> 
> Does it really matter?  Rescanning everything can't hurt here, and it
> shouldn't be noticable in any boot time chart.  I feel like arguing
> about tiny stuff like this takes away from the obvious other problems
> this whole patch series had :(

Yes IMHO it's not worth trying to filter those two bits, sharing the
same function looked much cleaner.

It always helps to get patches to perfection so reviews welcome, but
IMHO the best way forward is to release a kernel with PTI then in rc1
of the next apply ibrs ibpb solution for variant#2 in whatever shape
and form (not necessarily immediately hyper optimized using static key
for the entry code branching or static_cpu_has for once), so that it
activates the microcode IBRS IBPB. Then we can optimize it in rc2 and
later.

About reptoline, I think all reptoline talk it's a waste of time right
now. Reptoline is an attempt to optimize for old CPUs only, it's
hugely more complex to implement and deploy, and for future silicon
it's useless. Even reptoline still requires IBRS and some CPU has no
ibrs so it'd require yet another alternative there and repotline can't
fix skylake anyway, reptoline requires new gcc code that doesn't even
exist today to do a 2-way code emission with 2 different reptolines
for different CPUS with no kernel code that exists that can reconcile
it at boot time to patch it. There's no way to runtime disable it
after applied, and AMD fam 0x10 0x12 0x16 also wouldn't get any
benefit reptoline either, not just skylake wouldn't use it.

Thanks,
Andrea