linux-kernel - Re: [patch 28/30] x86/microcode: Handle "offline" CPUs correctly

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZNVbc7qy8k49Dwhi@araj-mobl.amr.corp.intel.com>
Date:   Thu, 10 Aug 2023 14:49:39 -0700
From:   Ashok Raj <ashok.raj@...el.com>
To:     Peter Zijlstra <peterz@...radead.org>
CC:     Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>, <x86@...nel.org>,
        Borislav Petkov <bp@...en8.de>,
        Arjan van de Ven <arjan@...ux.intel.com>,
        Ashok Raj <ashok.raj@...el.com>
Subject: Re: [patch 28/30] x86/microcode: Handle "offline" CPUs correctly

On Thu, Aug 10, 2023 at 11:05:11PM +0200, Peter Zijlstra wrote:
> On Thu, Aug 10, 2023 at 01:50:17PM -0700, Ashok Raj wrote:
> > On Thu, Aug 10, 2023 at 10:46:05PM +0200, Peter Zijlstra wrote:
> > > On Thu, Aug 10, 2023 at 08:38:07PM +0200, Thomas Gleixner wrote:
> > > 
> > > >  	for_each_cpu_and(cpu, cpu_present_mask, &cpus_booted_once_mask) {
> > > > +		/*
> > > > +		 * Offline CPUs sit in one of the play_dead() functions
> > > > +		 * with interrupts disabled, but they still react on NMIs
> > > > +		 * and execute arbitrary code. Also MWAIT being updated
> > > > +		 * while the offline CPU sits there is not necessarily safe
> > > > +		 * on all CPU variants.
> > > > +		 *
> > > > +		 * Mark them in the offline_cpus mask which will be handled
> > > > +		 * by CPU0 later in the update process.
> > > > +		 *
> > > > +		 * Ensure that the primary thread is online so that it is
> > > > +		 * guaranteed that all cores are updated.
> > > > +		 */
> > > >  		if (!cpu_online(cpu)) {
> > > > +			if (topology_is_primary_thread(cpu) || !allow_smt_offline) {
> > > > +				pr_err("CPU %u not online, loading aborted\n", cpu);
> > > 
> > > We could make the NMI handler do the ucode load, no? Also, you just need
> > > any thread online, don't particularly care about primary thread or not
> > > afaict.
> > 
> > Patch 25 does that load in NMI. You are right, we just need "a" CPU in each
> > core online. 
> 
> Patch 25 does it for online CPUs, offline CPUs are having a separate
> code path:
> 
>   microcode_nmi_handler()
> 
> vs
> 
>   microcode_offline_nmi_handler()

Since the code enforces all primary CPUs to be ONLINE, the secondaries are the
other thread of the same core. So they automatically get the update when
primary does it. 

The secondaries are parked in NMI just to avoid the risk of executing code
that might be patched by primary.

Or maybe you had something else in mind.