12 Dec
2007
12 Dec
'07
5:19 p.m.
On Dec 12, 2007 12:28 AM, <Martin.Karlsson(a)emerson.com> wrote:
Hello,
Yesterday I wrote some code for the Syslinux boot loader that is meant to
boot different images based on which revision of the CPU microcode that is
currently loaded in the CPU (yes, this is a bit strange but we have a very
good reason for it). I then compared my implementation with a few others an
ran across yours, in
http://www.openbios.org/viewvc/trunk/LinuxBIOSv2/src/cpu/intel/microcode/mi…
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 :-)
We'll have to figure out if this code was ever (a) used and (b) worked :-)
Thanks again, this is great!
ron
20 Feb
20 Feb
5:19 p.m.
New subject: [LinuxBIOS] Intel microcode revision code
* ron minnich <rminnich(a)gmail.com> [071212 17:19]:
Here's a patch that resolves the issue.
--
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info(a)coresystems.de • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866
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 :-) 
5:59 p.m.
New subject: [LinuxBIOS] Intel microcode revision code
On 20.02.2008 17:19, Stefan Reinauer wrote:
* ron minnich <rminnich(a)gmail.com> [071212
17:19]:
Here's a patch that resolves the issue.
Signed-off-by: Stefan Reinauer <stepan(a)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.
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 :-)
"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/
6:52 p.m.
New subject: [LinuxBIOS] Intel microcode revision code
Carl-Daniel Hailfinger wrote:
On 20.02.2008 17:19, Stefan Reinauer wrote:
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.
Stefan
--
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info(a)coresystems.de • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866
* ron minnich <rminnich(a)gmail.com> [071212
17:19]:
Here's a patch that resolves the issue.
Signed-off-by: Stefan Reinauer <stepan(a)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.
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
:-)
"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
7:09 p.m.
New subject: [LinuxBIOS] Intel microcode revision code
On 20.02.2008 18:52, Stefan Reinauer wrote:
Carl-Daniel Hailfinger wrote:
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
--
http://www.hailfinger.org/
On 20.02.2008 17:19, Stefan Reinauer wrote:
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.
* ron minnich <rminnich(a)gmail.com> [071212
17:19]:
ACK.
> 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(a)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"
"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
7:29 p.m.
New subject: [LinuxBIOS] Intel microcode revision code
On Wed, Feb 20, 2008 at 1:09 PM, Carl-Daniel Hailfinger <
c-d.hailfinger.devel.2006(a)gmx.net> wrote:
On 20.02.2008 18:52, Stefan Reinauer wrote:
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
Carl-Daniel Hailfinger wrote:
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
On 20.02.2008 17:19, Stefan Reinauer wrote:
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 * ron minnich <rminnich(a)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(a)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
7:38 p.m.
New subject: [LinuxBIOS] Intel microcode revision code
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(a)gmx.net> wrote:
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/
On 20.02.2008 18:52, Stefan Reinauer wrote:
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?
Carl-Daniel Hailfinger wrote:
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
On 20.02.2008 17:19, Stefan Reinauer wrote:
> * ron minnich <rminnich(a)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(a)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
9:35 p.m.
New subject: [LinuxBIOS] Intel microcode revision code
Quoting Corey Osgood <corey.osgood(a)gmail.com>om>:
On Wed, Feb 20, 2008 at 1:09 PM, Carl-Daniel
Hailfinger <
c-d.hailfinger.devel.2006(a)gmx.net> wrote:
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(a)smittys.pointclark.net>
Thanks - Joe
On 20.02.2008 18:52, Stefan Reinauer wrote:
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
Carl-Daniel Hailfinger wrote:
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
On 20.02.2008 17:19, Stefan Reinauer wrote:
> * ron minnich <rminnich(a)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(a)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
4720
days inactive
4790
days old
7 comments
5 participants
participants (5)
-
Carl-Daniel Hailfinger -
Corey Osgood -
joe＠smittys.pointclark.net -
ron minnich -
Stefan Reinauer