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, 17 Sep 2019 08:37:10 +0200 (CEST)
From:   Thomas Gleixner <tglx@linutronix.de>
To:     "Raj, Ashok" <ashok.raj@intel.com>
cc:     Johannes Erdfelt <johannes@erdfelt.com>,
        Borislav Petkov <bp@alien8.de>,
        Boris Ostrovsky <boris.ostrovsky@oracle.com>,
        Mihai Carabas <mihai.carabas@oracle.com>,
        "H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@redhat.com>,
        Jon Grimm <Jon.Grimm@amd.com>, kanth.ghatraju@oracle.com,
        konrad.wilk@oracle.com, patrick.colp@oracle.com,
        Tom Lendacky <thomas.lendacky@amd.com>,
        x86-ml <x86@kernel.org>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86/microcode: Add an option to reload microcode even
 if revision is unchanged
In-Reply-To: <20190917003122.GA3005@otc-nc-03>
Message-ID: <alpine.DEB.2.21.1909170824220.2066@nanos.tec.linutronix.de>
References: <20190905072029.GB19246@zn.tnic> <20190905194044.GA3663@otc-nc-03> <alpine.DEB.2.21.1909052316130.1902@nanos.tec.linutronix.de> <20190905222706.GA4422@otc-nc-03> <alpine.DEB.2.21.1909061431330.1902@nanos.tec.linutronix.de> <20190906144039.GA29569@sventech.com>
 <alpine.DEB.2.21.1909062237580.1902@nanos.tec.linutronix.de> <20190907003338.GA14807@araj-mobl1.jf.intel.com> <alpine.DEB.2.21.1909071236120.1902@nanos.tec.linutronix.de> <alpine.DEB.2.21.1909161227060.10731@nanos.tec.linutronix.de>
 <20190917003122.GA3005@otc-nc-03>
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

Raj,

On Mon, 16 Sep 2019, Raj, Ashok wrote:
> On Mon, Sep 16, 2019 at 12:36:11PM +0200, Thomas Gleixner wrote:
> > That said, there is also a distinct lack of information about micro code
> > loading in a safe way in general. We absolutely do not know whether a micro
> > code update affects any instruction which might be in use during the update
> > on a sibling. Right now it's all load and pray and the SDM is not really
> > helpful with that either.
> > 
> 
> Guilty as charged :-). In general we do not expect microcode updates to 
> remove any cpuid bits (Not that it hasn't happened, but it slipped through
> the cracks). 
> 
> microode updates should be of 3 types.
> 
> - Only loadable from BIOS (Only via FIT tables)
> - Suitable for early load (things that take cpuid bits for e.g.)
> - Suitable for late-load. (Where no cpuid bits should change etc).
> 
> Today the way we load after a stop_machine() all threads in the system are
> held hostage until all the cores have done the update. The thread sibling
> is also in the rendezvous loop. 

I know. See below.
 
> Do you think we still have that risk with a sibling thread? 
> (Assuming future ucodes don't do weird things like what happened in 
> that case where a cpuid was removed via an update)

Well, yes. The sibling executes a limited set of instructions in a loop,
but it might be hit by an NMI or MCE which executes even more instructions.

So what happens if the ucode update "fixes" one of the executed
instructions on the fly? Is that guaranteed to be safe? There is nothing
which says so.

A decade ago I experimented with putting the spinning CPUs into MWAIT,
which caused havoc. Did neither have time nor the stomach to dig into that
further, but the ucode update _did_ fix an issue with MWAIT according to
the version history.

That's why I'm worried about instructions being "fixed" which are executed
in parallel on the sibling.

An authorative statement vs. that would be appreciated. Preferrably in form
of an extension of the SDM, but an upfront statement in this thread would
be a good start.

Thanks,

	tglx