Revamp Intel microcode update code

- add GPLv2 + copyright header after talking to Ron
- "bits" in struct microcode served no real purpose but
  getting its address taken. Hence drop it
- use asm volatile instead of __asm__ volatile
- drop superfluous wrmsr (that seems to be harmless but
  is still wrong) in read_microcode_rev
- use u32 instead of unsigned int where appropriate
- make code usable both in bootblock and in ramstage
- drop ROMCC style print_debug statements
- drop microcode update copy in Sandybridge bootblock

Change-Id: Iec4d5c7bfac210194caf577e8d72446e6dfb4b86
Signed-off-by: Stefan Reinauer <reinauer@google.com>
Reviewed-on: http://review.coreboot.org/928
Tested-by: build bot (Jenkins)
Reviewed-by: Ronald G. Minnich <rminnich@gmail.com>

src/cpu/intel/microcode/microcode.c

@@ -1,6 +1,24 @@
-/* microcode.c:	Microcode update for PIII and later CPUS
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2000 Ronald G. Minnich
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+/* Microcode update for Intel PIII and later CPUs */
+
 #include <stdint.h>
 #include <console/console.h>
 #include <cpu/cpu.h>
@@ -9,30 +27,28 @@
 
 struct microcode {
 	u32 hdrver;	/* Header Version */
-	u32 rev;	/* Patch ID       */
+	u32 rev;	/* Update Revision */
-	u32 date;	/* DATE           */
+	u32 date;	/* Date */
-	u32 sig;	/* CPUID          */
+	u32 sig;	/* Processor Signature */
 
-	u32 cksum;	/* Checksum       */
+	u32 cksum;	/* Checksum */
-	u32 ldrver;	/* Loader Version */
+	u32 ldrver;	/* Loader Revision */
-	u32 pf;		/* Platform ID    */
+	u32 pf;		/* Processor Flags */
 
-	u32 data_size;	/* Data size      */
+	u32 data_size;	/* Data Size */
-	u32 total_size;	/* Total size     */
+	u32 total_size;	/* Total Size */
 
 	u32 reserved[3];
-	u32 bits[1012];
 };
 
 static inline u32 read_microcode_rev(void)
 {
-	/* Some Intel Cpus can be very finicky about the
+	/* Some Intel CPUs can be very finicky about the
 	 * CPUID sequence used.  So this is implemented in
 	 * assembly so that it works reliably.
 	 */
 	msr_t msr;
-	__asm__ volatile (
+	asm volatile (
-		"wrmsr\n\t"
 		"xorl %%eax, %%eax\n\t"
 		"xorl %%edx, %%edx\n\t"
 		"movl $0x8b, %%ecx\n\t"
@@ -52,14 +68,14 @@ static inline u32 read_microcode_rev(void)
 
 void intel_update_microcode(const void *microcode_updates)
 {
-	unsigned int eax;
+	u32 eax;
-	unsigned int pf, rev, sig;
+	u32 pf, rev, sig;
 	unsigned int x86_model, x86_family;
 	const struct microcode *m;
 	const char *c;
 	msr_t msr;
 
-	/* cpuid sets msr 0x8B iff a microcode update has been loaded. */
+	/* CPUID sets MSR 0x8B iff a microcode update has been loaded. */
 	msr.lo = 0;
 	msr.hi = 0;
 	wrmsr(0x8B, msr);
@@ -75,35 +91,38 @@ void intel_update_microcode(const void *microcode_updates)
 		msr = rdmsr(0x17);
 		pf = 1 << ((msr.hi >> 18) & 7);
 	}
-	print_debug("microcode_info: sig = 0x");
+#if !defined(__ROMCC__)
-	print_debug_hex32(sig);
+	/* If this code is compiled with ROMCC we're probably in
-	print_debug(" pf=0x");
+	 * the bootblock and don't have console output yet.
-	print_debug_hex32(pf);
+	 */
-	print_debug(" rev = 0x");
+	printk(BIOS_DEBUG, "microcode_info: sig=0x%08x pf=0x%08x rev=0x%08x\n",
-	print_debug_hex32(rev);
+			sig, pf, rev);
-	print_debug("\n");
+#endif
 
 	m = microcode_updates;
-	for(c = microcode_updates; m->hdrver;  m = (const struct microcode *)c) {
+	for(c = microcode_updates; m->hdrver; m = (const struct microcode *)c) {
 		if ((m->sig == sig) && (m->pf & pf)) {
 			unsigned int new_rev;
-			msr.lo = (unsigned long)(&m->bits) & 0xffffffff;
+			msr.lo = (unsigned long)c + sizeof(struct microcode);
 			msr.hi = 0;
 			wrmsr(0x79, msr);
 
 			/* Read back the new microcode version */
 			new_rev = read_microcode_rev();
 
-			print_debug("microcode updated to revision: ");
+#if !defined(__ROMCC__)
-			print_debug_hex32(new_rev);
+			printk(BIOS_DEBUG, "microcode updated to revision: "
-			print_debug(" from revision ");
+				    "%08x from revision %08x\n", new_rev, rev);
-			print_debug_hex32(rev);
+#endif
-			print_debug("\n");
 			break;
 		}
+
 		if (m->total_size) {
 			c += m->total_size;
 		} else {
+#if !defined(__ROMCC__)
+			printk(BIOS_WARNING, "Microcode has no valid size field!\n");
+#endif
 			c += 2048;
 		}
 	}

src/cpu/intel/model_206ax/bootblock.c

@@ -27,94 +27,7 @@ static const uint32_t microcode_updates[] = {
 	#include "x06_microcode.h"
 };
 
-struct microcode {
+#include <cpu/intel/microcode/microcode.c>
-	u32 hdrver;	/* Header Version */
-	u32 rev;	/* Patch ID       */
-	u32 date;	/* DATE           */
-	u32 sig;	/* CPUID          */
-
-	u32 cksum;	/* Checksum       */
-	u32 ldrver;	/* Loader Version */
-	u32 pf;		/* Platform ID    */
-
-	u32 data_size;	/* Data size      */
-	u32 total_size;	/* Total size     */
-
-	u32 reserved[3];
-	u32 bits[1012];
-};
-
-static inline u32 read_microcode_rev(void)
-{
-	/* Some Intel Cpus can be very finicky about the
-	 * CPUID sequence used.  So this is implemented in
-	 * assembly so that it works reliably.
-	 */
-	msr_t msr;
-	__asm__ volatile (
-		"wrmsr\n\t"
-		"xorl %%eax, %%eax\n\t"
-		"xorl %%edx, %%edx\n\t"
-		"movl $0x8b, %%ecx\n\t"
-		"wrmsr\n\t"
-		"movl $0x01, %%eax\n\t"
-		"cpuid\n\t"
-		"movl $0x08b, %%ecx\n\t"
-		"rdmsr \n\t"
-		: /* outputs */
-		"=a" (msr.lo), "=d" (msr.hi)
-		: /* inputs */
-		: /* trashed */
-		 "ecx"
-	);
-	return msr.hi;
-}
-
-void intel_update_microcode(const void *microcode_updates)
-{
-	unsigned int eax;
-	unsigned int pf, rev, sig;
-	unsigned int x86_model, x86_family;
-	const struct microcode *m;
-	const char *c;
-	msr_t msr;
-
-	/* cpuid sets msr 0x8B iff a microcode update has been loaded. */
-	msr.lo = 0;
-	msr.hi = 0;
-	wrmsr(0x8B, msr);
-	eax = cpuid_eax(1);
-	msr = rdmsr(0x8B);
-	rev = msr.hi;
-	x86_model = (eax >>4) & 0x0f;
-	x86_family = (eax >>8) & 0x0f;
-	sig = eax;
-
-	pf = 0;
-	if ((x86_model >= 5)||(x86_family>6)) {
-		msr = rdmsr(0x17);
-		pf = 1 << ((msr.hi >> 18) & 7);
-	}
-
-	m = microcode_updates;
-	for(c = microcode_updates; m->hdrver;  m = (const struct microcode *)c) {
-		if ((m->sig == sig) && (m->pf & pf)) {
-			unsigned int new_rev;
-			msr.lo = (unsigned long)(&m->bits) & 0xffffffff;
-			msr.hi = 0;
-			wrmsr(0x79, msr);
-
-			/* Read back the new microcode version */
-			new_rev = read_microcode_rev();
-			break;
-		}
-		if (m->total_size) {
-			c += m->total_size;
-		} else {
-			c += 2048;
-		}
-	}
-}
 
 static void set_var_mtrr(
 	unsigned reg, unsigned base, unsigned size, unsigned type)

src/include/cpu/intel/microcode.h

@@ -1 +1,22 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2000 Ronald G. Minnich
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#if !defined(__ROMCC__)
 void intel_update_microcode(const void *microcode_updates);
+#endif