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@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@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.