Re: [PATCH 1/2] x86/microcode: Update late microcode in parallel

[Raj, Ashok]

On Mon, Aug 26, 2019 at 08:53:05AM -0400, Boris Ostrovsky wrote:
> On 8/24/19 4:53 AM, Borislav Petkov wrote:
> >  
> > +wait_for_siblings:
> > +	if (__wait_for_cpus(&late_cpus_out, NSEC_PER_SEC))
> > +		panic("Timeout during microcode update!\n");
> > +
> >  	/*
> > -	 * Increase the wait timeout to a safe value here since we're
> > -	 * serializing the microcode update and that could take a while on a
> > -	 * large number of CPUs. And that is fine as the *actual* timeout will
> > -	 * be determined by the last CPU finished updating and thus cut short.
> > +	 * At least one thread has completed update on each core.
> > +	 * For others, simply call the update to make sure the
> > +	 * per-cpu cpuinfo can be updated with right microcode
> > +	 * revision.
> 
> 
> What is the advantage of having those other threads go through
> find_patch() and (in Intel case) intel_get_microcode_revision() (which
> involves two MSR accesses) vs. having the master sibling update slaves'
> microcode revisions? There are only two things that need to be updated,
> uci->cpu_sig.rev and c->microcode.
> 

True, yes we could do that. But there is some warm and comfy feeling
that you really read the revision from the thread local copy of the revision
and it matches what was updated in the other thread sibling rather than
just hardcoding the fixup's. The code looks clean in the sense it looks like
you are attempting to upgrade but the new revision is reflected correctly
and you skip the update.

But if you feel complelled, i'm not opposed to it as long as Boris is 
happy with the changes :-).

Cheers,
Ashok