Messages in this thread Patch in this message | | | Date | Wed, 25 May 2011 12:50:57 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH 1/3] x86, intel: Output microcode revision |
| |
On Wed, May 25, 2011 at 11:05:01AM +0200, Ingo Molnar wrote: > > 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.
Ok, that's a good point, actually.
> Sure we have /sys/devices/system/cpu/cpuN/microcode/version, but > that's both privileged to get
Not only that but you have to load the ucode driver to be able to read it. /proc/cpuinfo looks like the easiest and most generic place.
> and also has to be asked for separately.
yes.
> > 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.
Right.
Btw, can we dump the ucode version in hex since ours are much easier to read that way:
[86483.770976] microcode: CPU0: patch_level=0x010000c4 [86483.826987] microcode: CPU1: patch_level=0x010000c4 [86483.835071] microcode: CPU2: patch_level=0x010000c4 ...
I guess for Intel the ucode version format won't matter that much.
> > > > @@ -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.
even better.
Thanks.
-- Regards/Gruss, Boris.
Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632
| |