[coreboot] [LinuxBIOS] Intel microcode revision code

[joe at smittys.pointclark.net]

Quoting Corey Osgood <corey.osgood at gmail.com>:

> 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?
>
> -Corey
>
I agree, it's silly. Fist of all it is just a comment. Second, I  
doesn't effect the actual code. Looks like a typo to me....

Acked-by: Joseph Smith <joe at smittys.pointclark.net>


Thanks - Joe