Re: [PATCH 1/3] x86, intel: Output microcode revision

[Ingo Molnar <mingo@elte.hu>]

* Borislav Petkov <bp@alien8.de> wrote:

> On Wed, May 25, 2011 at 08:54:51AM +0200, Ingo Molnar wrote:
> > 
> > * Andi Kleen <andi@firstfloor.org> wrote:
> > 
> > > From: Andi Kleen <ak@linux.intel.com>
> > > 
> > > I got a request to make it easier to determine the microcode update level
> > > on Intel CPUs. This patch adds a new "cpu update" field to /proc/cpuinfo,
> > > which I added at the end to minimize impact on parsers.
> > 
> > Agreed, that is a good idea, adding this to cpuinfo makes sense.
> 
> Frankly, I'm not even 100% persuaded this is needed. The coretemp.c 
> jump-through-hoops to get the ucode revision is maybe the only case 
> that warrants adding that field to /proc/cpuinfo.

I've often wondered whether the CPU involved in a particular 
bugreport has the latest microcode installed.

Sure we have /sys/devices/system/cpu/cpuN/microcode/version, but 
that's both privileged to get and also has to be asked for 
separately.

Arguably the microcode version is a natural extension to the existing 
family/model/stepping sequence:

 cpu family	: 6
 model		: 26
 stepping	: 5

We'd now see:

 cpu family	: 6
 model		: 26
 stepping	: 5
 ucode_version  : 17

Where 'stepping' is a hardware revison number and 'ucode_version' is 
a dual software/hw revision number.

> > > @@ -111,6 +111,8 @@ struct cpuinfo_x86 {
> > >  	/* Index into per_cpu list: */
> > >  	u16			cpu_index;
> > >  #endif
> > > +	/* CPU update signature */
> > > +	u32			x86_cpu_update;
> > 
> > This should be cpu_microcode_version instead. We already know its x86 so the 
> > x86_ prefix is superfluous. 'cpu_update' is also rather ambigious and does not 
> > describe much.
> 
> Or shorter: 'cpu_ucode_version'.

We already know it's a cpu data structure, since it's called 'struct 
cpuinfo_x86' and the local variable is named 'c' which is the typical 
shortcut for that data structure.

so c->ucode_version is the right name here.

Thanks,

	Ingo