[coreboot] [LinuxBIOS] Intel microcode revision code

[Carl-Daniel Hailfinger]

On 20.02.2008 17:19, Stefan Reinauer wrote:
> * ron minnich <rminnich at gmail.com> [071212 17:19]:
>   
>>> Question to you guys: why is the first wrmsr instruction there? From my
>>> understanding, by not properly initialising ECX, EAX and EDX this will
>>> overwrite whatever is in the MSR pointed to by ECX?!
>>>
>>> BTW I tried out your code on our target hardware (Intel Celeron M, 600 MHz)
>>> and with that first wrmsr line in place it hangs and without it, it runs
>>> just fine.
>>>       
>> Thanks Martin. That looks like quite a nice bug catch you've done :-)
>>     
>
> Here's a patch that resolves the issue.
>
> Signed-off-by: Stefan Reinauer <stepan at coresystems.de>
>
> Index: src/cpu/intel/microcode/microcode.c
> ===================================================================
> --- src/cpu/intel/microcode/microcode.c	(revision 3111)
> +++ src/cpu/intel/microcode/microcode.c	(working copy)
> @@ -33,7 +33,6 @@
>  	 */
>  	msr_t msr;
>  	__asm__ volatile (
> -		"wrmsr\n\t"
>   

ACK.

>  		"xorl %%eax, %%eax\n\t"
>  		"xorl %%edx, %%edx\n\t"
>  		"movl $0x8b, %%ecx\n\t"
> @@ -60,7 +59,7 @@
>  	char *c;
>  	msr_t msr;
>  	
> -	/* cpuid sets msr 0x8B iff a microcode update has been loaded. */
> +	/* cpuid sets msr 0x8B if a microcode update has been loaded. */
>   

NACK. "IFF" is shorthand for "if and only if", see 
http://en.wikipedia.org/wiki/If_and_only_if

>  	msr.lo = 0;
>  	msr.hi = 0;
>  	wrmsr(0x8B, msr);
>   


Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/