Re: [PATCH] x86: Add check for P5 to microcode_intel_early

["Bryan O'Donoghue" <bryan.odonoghue.lkml@nexus-software.ie>]

On 19/04/13 20:11, Borislav Petkov wrote:
> On Fri, Apr 19, 2013 at 06:23:03PM +0100, Bryan O'Donoghue wrote:
>> Architectural MSRs associated with microcode are for P6 or higher.
>> Add a check to early microcode to detect<  P6.
>>
>> Without a check for<  P6 - we end up reading from unimplemented MSRs
>> on Pentium.
>
> Is this something you're actually seeing on some box or just found by
> staring at the code?

We actually see this on a P5 alright.
The code path is reachable, no question.

> In any case, the family checks should go into the ucode driver entry
> points in arch/x86/kernel/microcode_core_early.c. AFAICT, x86_vendor()
> is a good candidate to be taught to read out the family too and return
> X86_VENDOR_UNKNOWN if<  P6. Or something to that effect.

Hmm

Just returning X86_VENDOR_UNKNOWN - won't fix the bug though - after all 
MSR_IA32_UCODE_REV is also x86_family() >= 6

x86 = get_x86_family(csig.sig);
x86_model = get_x86_model(csig.sig);

if (x86 < 6)
	return UCODE_ERROR;

if ((x86_model >= 5) || (x86 > 6)) {
	/* get processor flags from MSR 0x17 */
	native_rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]);
	csig.pf = 1 << ((val[1] >> 18) & 7);
}

native_wrmsr(MSR_IA32_UCODE_REV, 0, 0);

/* As documented in the SDM: Do a CPUID 1 here */
sync_core();

/* get the current revision from MSR 0x8B */
native_rdmsr(MSR_IA32_UCODE_REV, val[0], val[1]);

Peter - what's your take ?