Re: [PATCH 1/4] x86: Do microcode updates at CPU_STARTING, not CPU_ONLINE

[Henrique de Moraes Holschuh <hmh@hmh.eng.br>]

On Thu, 14 Jun 2012, Borislav Petkov wrote:
> On Wed, Jun 13, 2012 at 01:20:39PM -0700, Andi Kleen wrote:
> >  	case CPU_ONLINE:
> >  	case CPU_ONLINE_FROZEN:
> > -		microcode_update_cpu(cpu);
> > +		/* Retry again in case we couldn't request early */
> > +		if (cpu_data(cpu).microcode < ucode_cpu_info[cpu].cpu_sig.rev)
> > +			microcode_update_cpu_late(cpu);
> 
> This implies newer ucode versions are numerically higher than
> what's currently present. And it is probably correct in the 99% of
> the cases but it would be more correct IMHO to have "!=" there.
> microcode_update_cpu_late takes care of checking the correct ucode
> version anyway down the path.

Indeed.  For Intel, the microcode rev is a *signed* value, and the
intended way for it to work is that you always install negative rev
microcode (used only internally at Intel, I am told), or install
microcode if rev(new microcode) > rev (old microcode) >= 0, and refuse
to install if rev(new microcode) == 0.

It is better to use != and have stricter checks in the vendor-specific
code.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh