Second attempt -- the first one got lost due to some local mail client
problems...
---------- Forwarded message ----------
Date: Fri, 1 Dec 2000 11:43:10 +0000 (GMT)
From: Tigran Aivazian <[email protected]>
To: Linus Torvalds <[email protected]>
Cc: [email protected]
Subject: [patch-2.4.0-test12-pre3] microcode update for P4
Hi Linus,
Here is the patch to microcode update driver to support the new P4
CPU.
Regards,
Tigran
diff -urN -X dontdiff linux/arch/i386/kernel/microcode.c ucode/arch/i386/kernel/microcode.c
--- linux/arch/i386/kernel/microcode.c Mon Oct 30 22:44:29 2000
+++ ucode/arch/i386/kernel/microcode.c Fri Dec 1 09:45:47 2000
@@ -4,13 +4,13 @@
* Copyright (C) 2000 Tigran Aivazian
*
* This driver allows to upgrade microcode on Intel processors
- * belonging to P6 family - PentiumPro, Pentium II,
- * Pentium III, Xeon etc.
+ * belonging to IA-32 family - PentiumPro, Pentium II,
+ * Pentium III, Xeon, Pentium 4, etc.
*
- * Reference: Section 8.10 of Volume III, Intel Pentium III Manual,
- * Order Number 243192 or free download from:
+ * Reference: Section 8.10 of Volume III, Intel Pentium 4 Manual,
+ * Order Number 245472 or free download from:
*
- * http://developer.intel.com/design/pentiumii/manuals/243192.htm
+ * http://developer.intel.com/design/pentium4/manuals/245472.htm
*
* For more information, go to http://www.urbanmyth.org/microcode
*
@@ -44,6 +44,9 @@
* to be 0 on my machine which is why it worked even when I
* disabled update by the BIOS)
* Thanks to Eric W. Biederman <[email protected]> for the fix.
+ * 1.08 01 Dec 2000, Richard Schaal <[email protected]> and
+ * Tigran Aivazian <[email protected]>
+ * Intel P4 processor support.
*/
#include <linux/init.h>
@@ -58,12 +61,20 @@
#include <asm/uaccess.h>
#include <asm/processor.h>
-#define MICROCODE_VERSION "1.07"
+#define MICROCODE_VERSION "1.08"
-MODULE_DESCRIPTION("Intel CPU (P6) microcode update driver");
+MODULE_DESCRIPTION("Intel CPU (IA-32) microcode update driver");
MODULE_AUTHOR("Tigran Aivazian <[email protected]>");
EXPORT_NO_SYMBOLS;
+#define MICRO_DEBUG 0
+
+#if MICRO_DEBUG
+#define Dprintk(x...) printk(##x)
+#else
+#define Dprintk(x...)
+#endif
+
/* VFS interface */
static int microcode_open(struct inode *, struct file *);
static ssize_t microcode_read(struct file *, char *, size_t, loff_t *);
@@ -114,7 +125,10 @@
printk(KERN_ERR "microcode: failed to devfs_register()\n");
return -EINVAL;
}
- printk(KERN_INFO "P6 Microcode Update Driver v%s\n", MICROCODE_VERSION);
+ printk(KERN_INFO
+ "IA-32 Microcode Update Driver: v%s <[email protected]>\n",
+ MICROCODE_VERSION);
+
return 0;
}
@@ -124,12 +138,12 @@
devfs_unregister(devfs_handle);
if (mc_applied)
kfree(mc_applied);
- printk(KERN_INFO "P6 Microcode Update Driver v%s unregistered\n",
+ printk(KERN_INFO "IA-32 Microcode Update Driver v%s unregistered\n",
MICROCODE_VERSION);
}
-module_init(microcode_init);
-module_exit(microcode_exit);
+module_init(microcode_init)
+module_exit(microcode_exit)
static int microcode_open(struct inode *unused1, struct file *unused2)
{
@@ -177,16 +191,18 @@
req->err = 1; /* assume the worst */
- if (c->x86_vendor != X86_VENDOR_INTEL || c->x86 != 6){
- printk(KERN_ERR "microcode: CPU%d not an Intel P6\n", cpu_num);
+ if (c->x86_vendor != X86_VENDOR_INTEL || c->x86 < 6 ||
+ test_bit(X86_FEATURE_IA64, &c->x86_capability)){
+ printk(KERN_ERR "microcode: CPU%d not a capable Intel processor\n", cpu_num);
return;
}
sig = c->x86_mask + (c->x86_model<<4) + (c->x86<<8);
- if (c->x86_model >= 5) {
- /* get processor flags from BBL_CR_OVRD MSR (0x17) */
- rdmsr(0x17, val[0], val[1]);
+ if ((c->x86_model >= 5) || (c->x86 > 6))
+ {
+ /* get processor flags from MSR 0x17 */
+ rdmsr(IA32_PLATFORM_ID, val[0], val[1]);
pf = 1 << ((val[1] >> 18) & 7);
}
@@ -195,13 +211,32 @@
microcode[i].ldrver == 1 && microcode[i].hdrver == 1) {
found=1;
- wrmsr(0x8B, 0, 0);
+
+ Dprintk("Microcode\n");
+ Dprintk(" Header Revision %d\n",microcode[i].hdrver);
+ Dprintk(" Date %x/%x/%x\n",
+ ((microcode[i].date >> 24 ) & 0xff),
+ ((microcode[i].date >> 16 ) & 0xff),
+ (microcode[i].date & 0xFFFF));
+ Dprintk(" Type %x Family %x Model %x Stepping %x\n",
+ ((microcode[i].sig >> 12) & 0x3),
+ ((microcode[i].sig >> 8) & 0xf),
+ ((microcode[i].sig >> 4) & 0xf),
+ ((microcode[i].sig & 0xf)));
+ Dprintk(" Checksum %x\n",microcode[i].cksum);
+ Dprintk(" Loader Revision %x\n",microcode[i].ldrver);
+ Dprintk(" Processor Flags %x\n\n",microcode[i].pf);
+
+ /* trick, to work even if there was no prior update by the BIOS */
+ wrmsr(IA32_UCODE_REV, 0, 0);
__asm__ __volatile__ ("cpuid" : : : "ax", "bx", "cx", "dx");
- rdmsr(0x8B, val[0], rev);
+
+ /* get current (on-cpu) revision into rev (ignore val[0]) */
+ rdmsr(IA32_UCODE_REV, val[0], rev);
if (microcode[i].rev < rev) {
printk(KERN_ERR
"microcode: CPU%d not 'upgrading' to earlier revision"
- " %d (current=%d)\n", cpu_num, microcode[i].rev, rev);
+ " %x (current=%x)\n", cpu_num, microcode[i].rev, rev);
} else if (microcode[i].rev == rev) {
printk(KERN_ERR
"microcode: CPU%d already up-to-date (revision %d)\n",
@@ -219,9 +254,14 @@
break;
}
- wrmsr(0x79, (unsigned int)(m->bits), 0);
+ /* write microcode via MSR 0x79 */
+ wrmsr(IA32_UCODE_WRITE, (unsigned int)(m->bits), 0);
+
+ /* serialize */
__asm__ __volatile__ ("cpuid" : : : "ax", "bx", "cx", "dx");
- rdmsr(0x8B, val[0], val[1]);
+
+ /* get the current revision from MSR 0x8B */
+ rdmsr(IA32_UCODE_REV, val[0], val[1]);
req->err = 0;
req->slot = i;
@@ -267,8 +307,8 @@
mc_applied = kmalloc(smp_num_cpus*sizeof(struct microcode),
GFP_KERNEL);
if (!mc_applied) {
- printk(KERN_ERR "microcode: out of memory for saved microcode\n");
up_write(µcode_rwsem);
+ printk(KERN_ERR "microcode: out of memory for saved microcode\n");
return -ENOMEM;
}
}
diff -urN -X dontdiff linux/include/asm-i386/msr.h ucode/include/asm-i386/msr.h
--- linux/include/asm-i386/msr.h Thu Oct 7 18:17:09 1999
+++ ucode/include/asm-i386/msr.h Fri Dec 1 09:38:59 2000
@@ -30,3 +30,7 @@
: "=a" (low), "=d" (high) \
: "c" (counter))
+/* symbolic names for some interesting MSRs */
+#define IA32_PLATFORM_ID 0x17
+#define IA32_UCODE_WRITE 0x79
+#define IA32_UCODE_REV 0x8B
Followup to: <[email protected]>
By author: Tigran Aivazian <[email protected]>
In newsgroup: linux.dev.kernel
>
> Second attempt -- the first one got lost due to some local mail client
> problems...
>
> Hi Linus,
>
> Here is the patch to microcode update driver to support the new P4
> CPU.
>
> --- linux/include/asm-i386/msr.h Thu Oct 7 18:17:09 1999
> +++ ucode/include/asm-i386/msr.h Fri Dec 1 09:38:59 2000
> @@ -30,3 +30,7 @@
> : "=a" (low), "=d" (high) \
> : "c" (counter))
>
> +/* symbolic names for some interesting MSRs */
> +#define IA32_PLATFORM_ID 0x17
> +#define IA32_UCODE_WRITE 0x79
> +#define IA32_UCODE_REV 0x8B
>
Please call these MSR_* instead, "IA32_*" isn't very descriptive,
besides, the preferred prefix in existing locations in the Linux
kernel is "X86_", e.g. X86_EFLAGS_IF or X86_CR4_PSE. I think there
are standard symbolic names for most MSRs in volume 3 of the Intel
processor manuals; I would suggest we use those.
-hpa
--
<[email protected]> at work, <[email protected]> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt
> Please call these MSR_* instead, "IA32_*" isn't very descriptive,
> besides, the preferred prefix in existing locations in the Linux
> kernel is "X86_", e.g. X86_EFLAGS_IF or X86_CR4_PSE. I think there
I think I agree with Tigran's naming. These are IA32 registers not X86 ones ;)
On 1 Dec 2000, H. Peter Anvin wrote:
> > +/* symbolic names for some interesting MSRs */
> > +#define IA32_PLATFORM_ID 0x17
> > +#define IA32_UCODE_WRITE 0x79
> > +#define IA32_UCODE_REV 0x8B
> >
>
> Please call these MSR_* instead, "IA32_*" isn't very descriptive,
> besides, the preferred prefix in existing locations in the Linux
> kernel is "X86_", e.g. X86_EFLAGS_IF or X86_CR4_PSE. I think there
> are standard symbolic names for most MSRs in volume 3 of the Intel
> processor manuals; I would suggest we use those.
~~~~~~~~~~~~~~~~~~~~~~~~~~~
which is exactly what I did. The old name for 0x17 was something like
BBL_CR_OVRD which had absolutely no meaning (or no meaning that I could
discern) so in the past I use just the number 0x17. Now (see P4 manuals,
already on developer.intel.com) they renamed to a very meaningful
IA32_PLATFORM_ID so I used that one. It is more important to match the
naming of the original specs than to be consistent with other naming used
in the kernel. To prove this point I suggest you look at NFSv2/NFSv3
naming used in the Linux kernel -- it matches rfc1094/rfc1813 which I
liked very much because it simplified reading kernel code (I assume
everyone first studies the specs and then reads Linux implementation).
Now that was about 0x17. As for 0x79 and 0x8B I made up my own names using
the Intel's one for 0x17 as a guideline. So they "look just like" as if
they were from the manual too ;) (and they are meaningful, which is a
bonus)
I am also glad Alan agrees with me.
Regards,
Tigran
Alan Cox wrote:
>
> > Please call these MSR_* instead, "IA32_*" isn't very descriptive,
> > besides, the preferred prefix in existing locations in the Linux
> > kernel is "X86_", e.g. X86_EFLAGS_IF or X86_CR4_PSE. I think there
>
> I think I agree with Tigran's naming. These are IA32 registers not X86 ones ;)
They are MSRs, most of all. His naming didn't reflect that, and quite
frankly, I'd much rather use the names (all starting with MSR_) that the
Intel documentation uses.
-hpa
--
<[email protected]> at work, <[email protected]> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt
Tigran Aivazian wrote:
>
> On Sat, 2 Dec 2000, H. Peter Anvin wrote:
>
> > Alan Cox wrote:
> > >
> > > > Please call these MSR_* instead, "IA32_*" isn't very descriptive,
> > > > besides, the preferred prefix in existing locations in the Linux
> > > > kernel is "X86_", e.g. X86_EFLAGS_IF or X86_CR4_PSE. I think there
> > >
> > > I think I agree with Tigran's naming. These are IA32 registers not X86 ones ;)
> >
> > They are MSRs, most of all. His naming didn't reflect that, and quite
> > frankly, I'd much rather use the names (all starting with MSR_) that the
> > Intel documentation uses.
> >
>
> Peter,
>
> you probably missed the message I sent to you earlier. I have already
> explained that I did use the names which Intel documentation uses. You may
> have an old (Pentium III) version of the manual but the current
> (P4) is already available (albeit a preliminary) and that is what I used
> as a guidance.
>
> It makes sense to check the facts before stating the same wrong statement
> twice.
>
OK, fair enough. Let me make a new statement then: I suggest we preface
these with MSR_ anyway so we can tell what they really are.
-hpa
--
<[email protected]> at work, <[email protected]> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt
Tigran Aivazian wrote:
>
> On Sat, 2 Dec 2000, H. Peter Anvin wrote:
> >
> > OK, fair enough. Let me make a new statement then: I suggest we preface
> > these with MSR_ anyway so we can tell what they really are.
> >
>
> That is much better. Actually, I accept your suggestion. (because I have
> just found a few more cleanups to be done in the code but they were not
> worth a patch on their own but together with the above it is worth
> remaking the patch).
>
> So, unless Linus already applied it I will resend a new one to him
> shortly.
>
> Regards,
> Tigran
>
> PS. Btw, the proof of my statement is found at:
>
> http://developer.intel.com/design/pentium4/manuals/245472.htm
>
> and download the PDF, then read the section 8.11.1 on page 8-32.
I believe you :) Pardon me now, I have a foot to get unstuck from this
here mouth of mine...
-hpa
--
<[email protected]> at work, <[email protected]> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt
Tigran Aivazian wrote:
>
> On Sat, 2 Dec 2000, Tigran Aivazian wrote:
>
> > On Sat, 2 Dec 2000, H. Peter Anvin wrote:
> > >
> > > OK, fair enough. Let me make a new statement then: I suggest we preface
> > > these with MSR_ anyway so we can tell what they really are.
> > >
> >
> > That is much better. Actually, I accept your suggestion.
>
> on the other hand -- that makes them much longer and it is always obvious
> from the context what they are, i.e.:
>
> a) if they appear in the code then it is unlikely they are outside of
> rdmsr()/wrmsr() which makes their meaning obvious.
>
> b) if they are in the header, the name of the header asm/msr.h and the
> comment above their definition explains what they are.
>
> I don't know -- if people really think MSR_ is needed then it can be
> done.
>
> I think my intuitive IA32_ naming was adequate but if you really believe
> we should prefix it with MSR_ then so be it.
>
I really think so... after all, it might be obvious when you use them in
the correct context, but it definitely isn't obvious when you're using
them in the wrong context. I am also worried about namespace collisions
doing back things.
-hpa
--
<[email protected]> at work, <[email protected]> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt
Peter,
Some minor mods to test12-pre5 (and earlier) arch/i386/kernel/setup.c:
please pass on to Linus if you approve.
1. Your "features" was reverted to "flags", but an extra tab is needed.
2. identify_cpu() re-evaluates x86_capability, which left cpu_has_tsc true
(and cpu MHz shown as 0.000) in non-SMP "notsc" case: #ifdef CONFIG_TSC
was bogus. And set X86_CR4_TSD here when testing this cpu's capability,
not where cpu_init() tests cpu_has_tsc (boot_cpu's adjusted capability).
I have removed the "FIX-HPA" comment line: of course, that's none of my
business, but if you approve the patch I imagine you'd want that to go too
(I agree it's a bit ugly there, but safest to disable cpu_has_tsc soonest).
Hugh
--- test12-pre5/arch/i386/kernel/setup.c Tue Dec 5 17:25:55 2000
+++ linux/arch/i386/kernel/setup.c Tue Dec 5 17:56:35 2000
@@ -1999,10 +1999,14 @@
* we do "generic changes."
*/
+#ifndef CONFIG_X86_TSC
/* TSC disabled? */
-#ifdef CONFIG_TSC
- if ( tsc_disable )
- clear_bit(X86_FEATURE_TSC, &c->x86_capability);
+ if ( test_bit(X86_FEATURE_TSC, &c->x86_capability) ) {
+ if (tsc_disable || !cpu_has_tsc) {
+ clear_bit(X86_FEATURE_TSC, &c->x86_capability);
+ set_in_cr4(X86_CR4_TSD);
+ }
+ }
#endif
/* Disable the PN if appropriate */
@@ -2172,7 +2176,7 @@
"fpu_exception\t: %s\n"
"cpuid level\t: %d\n"
"wp\t\t: %s\n"
- "flags\t:",
+ "flags\t\t:",
c->fdiv_bug ? "yes" : "no",
c->hlt_works_ok ? "no" : "yes",
c->f00f_bug ? "yes" : "no",
@@ -2218,9 +2222,7 @@
#ifndef CONFIG_X86_TSC
if (tsc_disable && cpu_has_tsc) {
printk("Disabling TSC...\n");
- /**** FIX-HPA: DOES THIS REALLY BELONG HERE? ****/
clear_bit(X86_FEATURE_TSC, boot_cpu_data.x86_capability);
- set_in_cr4(X86_CR4_TSD);
}
#endif