[coreboot] [LinuxBIOS] Intel microcode revision code

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Wed Feb 20 19:38:51 CET 2008


On 20.02.2008 19:29, Corey Osgood wrote:
> On Wed, Feb 20, 2008 at 1:09 PM, Carl-Daniel Hailfinger <
> c-d.hailfinger.devel.2006 at gmx.net> wrote:
>
>   
>> On 20.02.2008 18:52, Stefan Reinauer wrote:
>>     
>>> Carl-Daniel Hailfinger wrote:
>>>       
>>>> 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
>>>>
>>>>         
>>> That's silly. This is not a mathematical expression nor a
>>> philosophical disquisition but a sentence. I am not even convinced
>>> that it was meant that way rather than being just a typo. If you have
>>> reasons to assume it means "If and only if" then let's write it that
>>>       
>> way.
>>
>> Merriam-Webster agrees with me that "iff" is a word:
>> http://www.merriam-webster.com/dictionary/iff
>> So this is a valid sentence and I see no reason to change it. Unless the
>> bit can be set even of no microcode update has been uploaded, "iff" is
>> the only correct word.
>>
>> Regards,
>> Carl-Daniel
>>
>>     
>
> Meriam-Webster and wikipedia might know, but not everyone does. Why can't we
> just change it to "if and only if" instead of using an obscure word that
> looks like a typo?
>   

OK, I was just used to the word from my Linux kernel and math background 
and thought everybody would understand it. "If and only if" is a longer 
phrase, but it seems to be understood by a wider audience, so I have 
dropped my objections against the change to that phrase.

Regards,
Carl-Daniel

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





More information about the coreboot mailing list