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

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20110525105057.GA21830@gere.osrc.amd.com>
Date:	Wed, 25 May 2011 12:50:57 +0200
From:	Borislav Petkov <bp@...64.org>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Borislav Petkov <bp@...en8.de>, Andi Kleen <andi@...stfloor.org>,
	x86@...nel.org, linux-kernel@...r.kernel.org,
	Andi Kleen <ak@...ux.intel.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/