14991 Large AMD microcode does not load at boot

14994 Panic on microcode update with UFS boot archive
14995 misleading messages on microcode update failure
14996 Restructure ucode data Makefiles
15003 Corrupt AMD microcode equivalence-table not properly handled
Reviewed by: Keith M Wesolowski <wesolows@oxide.computer>
Reviewed by: Marco van Wieringen <mvw@planets.elm.net>
Approved by: Dan McDonald <danmcd@mnx.io>

11 files changed, 310 insertions, 191 deletions

diff --git a/usr/src/common/fs/ufsops.c b/usr/src/common/fs/ufsops.c
index 2b6d1c5a05..00110a200d 100644
--- a/usr/src/common/fs/ufsops.c
+++ b/usr/src/common/fs/ufsops.c
@@ -23,6 +23,7 @@
  * Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  * Copyright (c) 2016 by Delphix. All rights reserved.
+ * Copyright 2022 Oxide Computer Company
  */
 
 #include <sys/types.h>
@@ -34,6 +35,7 @@
 #include <sys/sysmacros.h>
 #include <sys/bootvfs.h>
 #include <sys/filep.h>
+#include <sys/kmem.h>
 
 #ifdef	_BOOT
 #include "../common/util.h"
@@ -761,6 +763,17 @@ bufs_close(int fd)
 		cf_close(filep);
 		bkmem_free((char *)filep, sizeof (fileid_t));
 
+		/*
+		 * Some files are opened and closed in early boot, for example
+		 * when doing a microcode update on the boot CPU. In that case
+		 * the inode cache will contain memory allocated from boot
+		 * pages, which will be invalid once kmem is initialised.
+		 * Until kmem is ready, clear the inode cache when closing a
+		 * file.
+		 */
+		if (kmem_ready == 0)
+			free_cache();
+
 		return (0);
 	} else {
 		/* Big problem */
diff --git a/usr/src/data/ucode/Makefile b/usr/src/data/ucode/Makefile
index c4d40f1fbf..7b37890c24 100644
--- a/usr/src/data/ucode/Makefile
+++ b/usr/src/data/ucode/Makefile
@@ -10,45 +10,23 @@
 #
 
 #
-# Copyright 2019 Joyent, Inc.
-# Copyright 2022 OmniOS Community Edition (OmniOSce) Association.
+# Copyright 2022 Oxide Computer Company
 #
 
-include	$(SRC)/data/Makefile.data
+SUBDIRS = intel amd
 
-ROOTUCODEPATH = $(ROOT)/platform/i86pc/ucode
-ROOTAMDDIR = $(ROOTUCODEPATH)/AuthenticAMD
-ROOTINTELDIR = $(ROOTUCODEPATH)/GenuineIntel
+all :=		TARGET=all
+clean :=	TARGET=clean
+clobber :=	TARGET=clobber
+install :=	TARGET=install
 
-AMD_FILES :sh= (cd amd; print *-* container)
-INTEL_FILES :sh= (cd intel; print *-*)
-include $(SRC)/data/ucode/Makefile.links
+.PARALLEL: $(SUBDIRS)
 
-ROOTAMDFILES = $(AMD_FILES:%=$(ROOTAMDDIR)/%)
-ROOTINTELFILES = $(INTEL_FILES:%=$(ROOTINTELDIR)/%)
-ROOTINTELLINKS = $(INTEL_LINKS:%=$(ROOTINTELDIR)/%)
+.KEEP_STATE:
 
-$(ROOTAMDFILES) := FILEMODE = 444
-$(ROOTINTELFILES) := FILEMODE = 444
+all clean clobber install: $(SUBDIRS)
 
-.PARALLEL:
+$(SUBDIRS):	FRC
+		cd $@; pwd; $(MAKE) $(TARGET)
 
-all:
-
-install: $(ROOTAMDFILES) $(ROOTINTELFILES) $(ROOTINTELLINKS)
-
-clean:
-
-$(ROOTUCODEPATH):
-	$(INS.dir)
-
-$(ROOTINTELDIR) $(ROOTAMDDIR): $(ROOTUCODEPATH)
-	$(INS.dir)
-
-$(ROOTAMDDIR)/%: amd/% $(ROOTAMDDIR)
-	$(INS.file)
-
-$(ROOTINTELDIR)/%: intel/% $(ROOTINTELDIR)
-	$(INS.file)
-
-include	$(SRC)/data/Makefile.targ
+FRC:
diff --git a/usr/src/data/ucode/Makefile.com b/usr/src/data/ucode/Makefile.com
new file mode 100644
index 0000000000..030dedb94a
--- /dev/null
+++ b/usr/src/data/ucode/Makefile.com
@@ -0,0 +1,39 @@
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright 2019 Joyent, Inc.
+# Copyright 2022 OmniOS Community Edition (OmniOSce) Association.
+# Copyright 2022 Oxide Computer Company
+#
+
+include $(SRC)/data/Makefile.data
+
+ROOTINTELDIR = $(ROOTUCODEPATH)/GenuineIntel
+ROOTAMDDIR = $(ROOTUCODEPATH)/AuthenticAMD
+
+ROOTINTELFILES = $(INTEL_FILES:%=$(ROOTINTELDIR)/%)
+ROOTAMDFILES = $(AMD_FILES:%=$(ROOTAMDDIR)/%)
+
+$(ROOTINTELFILES) := FILEMODE = 444
+$(ROOTAMDFILES) := FILEMODE = 444
+
+$(ROOTUCODEPATH):
+	$(INS.dir)
+
+$(ROOTINTELDIR) $(ROOTAMDDIR): $(ROOTUCODEPATH)
+	$(INS.dir)
+
+$(ROOTINTELDIR)/%: % $(ROOTINTELDIR)
+	$(INS.file)
+
+$(ROOTAMDDIR)/%: % $(ROOTAMDDIR)
+	$(INS.file)
diff --git a/usr/src/data/ucode/README.ucode b/usr/src/data/ucode/README.ucode
index 30851457e5..ae06999833 100644
--- a/usr/src/data/ucode/README.ucode
+++ b/usr/src/data/ucode/README.ucode
@@ -52,6 +52,10 @@ The script "update.amd" in this directory can be used to help automate
 the update by automatically checking out the latest upstream firmware and
 inserting it into the tree and package manifest. Be careful about new files.
 
+NOTE: If any microcode file is larger than 8KiB (8192 bytes), then the size of
+      the ucode_file_amd_t structure in uts/common/sys/ucode.h will need
+      increasing to cater for this.
+
 AMD - see:
     usr/src/pkg/manifests/system-microcode-amd.p5m
 for exact current version
diff --git a/usr/src/data/ucode/amd/Makefile b/usr/src/data/ucode/amd/Makefile
new file mode 100644
index 0000000000..ea9c63cafd
--- /dev/null
+++ b/usr/src/data/ucode/amd/Makefile
@@ -0,0 +1,29 @@
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright 2019 Joyent, Inc.
+# Copyright 2022 OmniOS Community Edition (OmniOSce) Association.
+# Copyright 2022 Oxide Computer Company
+#
+
+ROOTUCODEPATH = $(ROOT)/platform/i86pc/ucode
+AMD_FILES :sh= (print *-* container)
+
+include ../Makefile.com
+
+.PARALLEL:
+
+all clean:
+
+install: $(ROOTAMDFILES)
+
+include $(SRC)/data/Makefile.targ
diff --git a/usr/src/data/ucode/intel/Makefile b/usr/src/data/ucode/intel/Makefile
new file mode 100644
index 0000000000..1650cc11dc
--- /dev/null
+++ b/usr/src/data/ucode/intel/Makefile
@@ -0,0 +1,32 @@
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright 2019 Joyent, Inc.
+# Copyright 2022 OmniOS Community Edition (OmniOSce) Association.
+# Copyright 2022 Oxide Computer Company
+#
+
+ROOTUCODEPATH = $(ROOT)/platform/i86pc/ucode
+INTEL_FILES :sh= (print *-*)
+
+include ../Makefile.com
+include Makefile.links
+
+ROOTINTELLINKS = $(INTEL_LINKS:%=$(ROOTINTELDIR)/%)
+
+.PARALLEL:
+
+all clean:
+
+install: $(ROOTINTELFILES) $(ROOTINTELLINKS)
+
+include $(SRC)/data/Makefile.targ
diff --git a/usr/src/data/ucode/Makefile.links b/usr/src/data/ucode/intel/Makefile.links
index cbf1906197..cbf1906197 100644
--- a/usr/src/data/ucode/Makefile.links
+++ b/usr/src/data/ucode/intel/Makefile.links
diff --git a/usr/src/data/ucode/update.amd b/usr/src/data/ucode/update.amd
index 59b0edee08..ca5a50f514 100755
--- a/usr/src/data/ucode/update.amd
+++ b/usr/src/data/ucode/update.amd
@@ -90,7 +90,7 @@ done
 
 # Now that everything is in place and verified, begin modifying the tree.
 
-rm -f amd/*
+rm -f amd/*-* amd/container
 
 cp $tmp/ucode/LICENSE.amd-ucode amd/THIRDPARTYLICENSE
 echo AMD Processor Microcode Data Files > amd/THIRDPARTYLICENSE.descrip
@@ -136,6 +136,7 @@ rm -f $mf.tmp
 for f in amd/*; do
 	bf=${f##*/}
 	[[ $bf = THIRDPARTYLICENSE* ]] && continue
+	[[ $bf = Makefile* ]] && continue
 	echo "file path=$FW/$bf group=sys mode=0444 reboot-needed=true" >> $mf
 done
 
diff --git a/usr/src/data/ucode/update.intel b/usr/src/data/ucode/update.intel
index e949922d5f..65898e4190 100755
--- a/usr/src/data/ucode/update.intel
+++ b/usr/src/data/ucode/update.intel
@@ -26,6 +26,7 @@ function errexit {
 ucodetar="$1"
 
 FW=platform/i86pc/ucode/GenuineIntel
+LINKSF=intel/Makefile.links
 
 export LC_ALL=C.UTF-8
 
@@ -78,12 +79,12 @@ mv $mf $mf.tmp
 egrep -v "(file|hardlink) path=$FW" $mf.tmp > $mf
 rm -f $mf.tmp
 
-rm -f intel/*
+rm -f intel/*-*
 
 cp $tmp/$path/license intel/THIRDPARTYLICENSE
 echo Intel Processor Microcode Data Files > intel/THIRDPARTYLICENSE.descrip
 
-rm -f Makefile.links
+rm -f $LINKSF
 
 typeset -A seen
 typeset -A inodes
@@ -105,6 +106,7 @@ done
 for f in intel/*; do
 	bf=$(basename $f)
 	[[ $bf = THIRDPARTYLICENSE* ]] && continue
+	[[ $bf = Makefile* ]] && continue
 	echo "file path=$FW/$bf group=sys mode=0444 reboot-needed=true" >> $mf
 done
 
@@ -115,11 +117,11 @@ done
 		echo "\t$i \\"
 	done | sed '$s/ .*//'
 	echo
-) > Makefile.links
+) > $LINKSF
 
 for i in "${!links[@]}"; do
 	echo "hardlink path=$FW/$i target=${links[$i]}" >> $mf
-	cat << EOM >> Makefile.links
+	cat << EOM >> $LINKSF
 \$(ROOTINTELDIR)/$i: \$(ROOTINTELDIR)/${links[$i]}
 	\$(RM) \$@; \$(LN) \$^ \$@
 
diff --git a/usr/src/uts/common/sys/ucode.h b/usr/src/uts/common/sys/ucode.h
index 7d2af4711a..3c5dd6e989 100644
--- a/usr/src/uts/common/sys/ucode.h
+++ b/usr/src/uts/common/sys/ucode.h
@@ -24,6 +24,7 @@
  *
  * Copyright 2021 OmniOS Community Edition (OmniOSce) Association.
  * Copyright 2022 Joyent, Inc.
+ * Copyright 2022 Oxide Computer Company
  */
 
 #ifndef	_SYS_UCODE_H
@@ -114,12 +115,17 @@ typedef struct ucode_header_amd {
 
 typedef struct ucode_file_amd {
 #ifndef __xpv
+	/*
+	 * The combined size of these fields adds up to 8KiB (8192 bytes).
+	 * If support is needed for larger update files, increase the size of
+	 * the uf_encr element.
+	 */
 	ucode_header_amd_t uf_header;
 	uint8_t uf_data[896];
 	uint8_t uf_resv[896];
 	uint8_t uf_code_present;
 	uint8_t uf_code[191];
-	uint8_t uf_encr[2048];
+	uint8_t uf_encr[6144];
 #else
 	uint8_t *ucodep;
 	uint32_t usize;
diff --git a/usr/src/uts/i86pc/os/microcode.c b/usr/src/uts/i86pc/os/microcode.c
index cbcecadbef..3e2e1a260d 100644
--- a/usr/src/uts/i86pc/os/microcode.c
+++ b/usr/src/uts/i86pc/os/microcode.c
@@ -26,6 +26,7 @@
  * Copyright 2012 Nexenta Systems, Inc. All rights reserved.
  * Copyright (c) 2018, Joyent, Inc.
  * Copyright 2021 OmniOS Community Edition (OmniOSce) Association.
+ * Copyright 2022 Oxide Computer Company
  */
 
 #include <sys/asm_linkage.h>
@@ -56,6 +57,7 @@
  * AMD-specific equivalence table
  */
 static ucode_eqtbl_amd_t *ucode_eqtbl_amd;
+static uint_t ucode_eqtbl_amd_entries;
 
 /*
  * mcpu_ucode_info for the boot CPU.  Statically allocated.
@@ -67,67 +69,60 @@ static ucode_file_t ucodefile;
 static void* ucode_zalloc(processorid_t, size_t);
 static void ucode_free(processorid_t, void *, size_t);
 
-static int ucode_capable_amd(cpu_t *);
+static uint32_t ucode_load_intel(ucode_file_t *, cpu_ucode_info_t *, cpu_t *);
 static int ucode_capable_intel(cpu_t *);
+static ucode_errno_t ucode_extract_intel(ucode_update_t *, uint8_t *, int);
+static void ucode_file_reset_intel(ucode_file_t *, processorid_t);
+static ucode_errno_t ucode_locate_intel(cpu_t *, cpu_ucode_info_t *,
+    ucode_file_t *);
+static ucode_errno_t ucode_match_intel(int, cpu_ucode_info_t *,
+    ucode_header_intel_t *, ucode_ext_table_intel_t *);
+static void ucode_read_rev_intel(cpu_ucode_info_t *);
 
-static ucode_errno_t ucode_extract_amd(ucode_update_t *, uint8_t *, int);
-static ucode_errno_t ucode_extract_intel(ucode_update_t *, uint8_t *,
-    int);
 
+static int ucode_capable_amd(cpu_t *);
+static ucode_errno_t ucode_extract_amd(ucode_update_t *, uint8_t *, int);
 static void ucode_file_reset_amd(ucode_file_t *, processorid_t);
-static void ucode_file_reset_intel(ucode_file_t *, processorid_t);
-
 static uint32_t ucode_load_amd(ucode_file_t *, cpu_ucode_info_t *, cpu_t *);
-static uint32_t ucode_load_intel(ucode_file_t *, cpu_ucode_info_t *, cpu_t *);
-
-#ifdef	__xpv
-static void ucode_load_xpv(ucode_update_t *);
-static void ucode_chipset_amd(uint8_t *, int);
-#endif
-
 static int ucode_equiv_cpu_amd(cpu_t *, uint16_t *);
-
 static ucode_errno_t ucode_locate_amd(cpu_t *, cpu_ucode_info_t *,
     ucode_file_t *);
-static ucode_errno_t ucode_locate_intel(cpu_t *, cpu_ucode_info_t *,
-    ucode_file_t *);
-
 #ifndef __xpv
 static ucode_errno_t ucode_match_amd(uint16_t, cpu_ucode_info_t *,
     ucode_file_amd_t *, int);
+#else
+static void ucode_load_xpv(ucode_update_t *);
+static void ucode_chipset_amd(uint8_t *, int);
 #endif
-static ucode_errno_t ucode_match_intel(int, cpu_ucode_info_t *,
-    ucode_header_intel_t *, ucode_ext_table_intel_t *);
-
 static void ucode_read_rev_amd(cpu_ucode_info_t *);
-static void ucode_read_rev_intel(cpu_ucode_info_t *);
+
 
 static const struct ucode_ops ucode_amd = {
-	MSR_AMD_PATCHLOADER,
-	ucode_capable_amd,
-	ucode_file_reset_amd,
-	ucode_read_rev_amd,
-	ucode_load_amd,
-	ucode_validate_amd,
-	ucode_extract_amd,
-	ucode_locate_amd
+	.write_msr	= MSR_AMD_PATCHLOADER,
+	.capable	= ucode_capable_amd,
+	.file_reset	= ucode_file_reset_amd,
+	.read_rev	= ucode_read_rev_amd,
+	.load		= ucode_load_amd,
+	.validate	= ucode_validate_amd,
+	.extract	= ucode_extract_amd,
+	.locate		= ucode_locate_amd
 };
 
 static const struct ucode_ops ucode_intel = {
-	MSR_INTC_UCODE_WRITE,
-	ucode_capable_intel,
-	ucode_file_reset_intel,
-	ucode_read_rev_intel,
-	ucode_load_intel,
-	ucode_validate_intel,
-	ucode_extract_intel,
-	ucode_locate_intel
+	.write_msr	= MSR_INTC_UCODE_WRITE,
+	.capable	= ucode_capable_intel,
+	.file_reset	= ucode_file_reset_intel,
+	.read_rev	= ucode_read_rev_intel,
+	.load		= ucode_load_intel,
+	.validate	= ucode_validate_intel,
+	.extract	= ucode_extract_intel,
+	.locate		= ucode_locate_intel
 };
 
 const struct ucode_ops *ucode;
 
 static const char ucode_failure_fmt[] =
-	"cpu%d: failed to update microcode from version 0x%x to 0x%x\n";
+	"cpu%d: failed to update microcode from version 0x%x to 0x%x";
 static const char ucode_success_fmt[] =
 	"?cpu%d: microcode has been updated from version 0x%x to 0x%x\n";
 
@@ -181,7 +176,7 @@ ucode_cleanup()
 static void*
 ucode_zalloc(processorid_t id, size_t size)
 {
-	if (id)
+	if (id != 0)
 		return (kmem_zalloc(size, KM_NOSLEEP));
 
 	/* BOP_ALLOC() failure results in panic */
@@ -191,7 +186,7 @@ ucode_zalloc(processorid_t id, size_t size)
 static void
 ucode_free(processorid_t id, void* buf, size_t size)
 {
-	if (id)
+	if (id != 0)
 		kmem_free(buf, size);
 }
 
@@ -212,7 +207,6 @@ ucode_free(processorid_t id, void* buf, size_t size)
 #define	XPVDOMU_OR_HVM	\
 	((hwenv == HW_XEN_PV && !is_controldom()) || (hwenv & HW_VIRTUAL) != 0)
 
-/*ARGSUSED*/
 static int
 ucode_capable_amd(cpu_t *cp)
 {
@@ -285,76 +279,94 @@ static int
 ucode_equiv_cpu_amd(cpu_t *cp, uint16_t *eq_sig)
 {
 	char name[MAXPATHLEN];
-	intptr_t fd;
-	int count;
-	int offset = 0, cpi_sig = cpuid_getsig(cp);
-	ucode_eqtbl_amd_t *eqtbl = ucode_eqtbl_amd;
+	int cpi_sig = cpuid_getsig(cp);
 
 	(void) snprintf(name, MAXPATHLEN, "/%s/%s/equivalence-table",
 	    UCODE_INSTALL_PATH, cpuid_getvendorstr(cp));
 
-	/*
-	 * No kmem_zalloc() etc. available on boot cpu.
-	 */
 	if (cp->cpu_id == 0) {
+		/*
+		 * No kmem_zalloc() etc. available on boot cpu.
+		 */
+		ucode_eqtbl_amd_t eqtbl;
+		int count, offset = 0;
+		intptr_t fd;
+
 		if ((fd = kobj_open(name)) == -1)
 			return (EM_OPENFILE);
-		/* ucode_zalloc() cannot fail on boot cpu */
-		eqtbl = ucode_zalloc(cp->cpu_id, sizeof (*eqtbl));
-		ASSERT(eqtbl);
 		do {
-			count = kobj_read(fd, (int8_t *)eqtbl,
-			    sizeof (*eqtbl), offset);
-			if (count != sizeof (*eqtbl)) {
+			count = kobj_read(fd, (int8_t *)&eqtbl,
+			    sizeof (eqtbl), offset);
+			if (count != sizeof (eqtbl)) {
 				(void) kobj_close(fd);
 				return (EM_HIGHERREV);
 			}
 			offset += count;
-		} while (eqtbl->ue_inst_cpu && eqtbl->ue_inst_cpu != cpi_sig);
+		} while (eqtbl.ue_inst_cpu != 0 &&
+		    eqtbl.ue_inst_cpu != cpi_sig);
 		(void) kobj_close(fd);
-	}
-
-	/*
-	 * If not already done, load the equivalence table.
-	 * Not done on boot CPU.
-	 */
-	if (eqtbl == NULL) {
-		struct _buf *eq;
-		uint64_t size;
+		*eq_sig = eqtbl.ue_equiv_cpu;
+	} else {
+		ucode_eqtbl_amd_t *eqtbl;
 
-		if ((eq = kobj_open_file(name)) == (struct _buf *)-1)
-			return (EM_OPENFILE);
+		/*
+		 * If not already done, load the equivalence table.
+		 * Not done on boot CPU.
+		 */
+		if (ucode_eqtbl_amd == NULL) {
+			struct _buf *eq;
+			uint64_t size;
+			int count;
 
-		if (kobj_get_filesize(eq, &size) < 0) {
-			kobj_close_file(eq);
-			return (EM_OPENFILE);
-		}
+			if ((eq = kobj_open_file(name)) == (struct _buf *)-1)
+				return (EM_OPENFILE);
 
-		ucode_eqtbl_amd = kmem_zalloc(size, KM_NOSLEEP);
-		if (ucode_eqtbl_amd == NULL) {
-			kobj_close_file(eq);
-			return (EM_NOMEM);
-		}
+			if (kobj_get_filesize(eq, &size) < 0) {
+				kobj_close_file(eq);
+				return (EM_OPENFILE);
+			}
 
-		count = kobj_read_file(eq, (char *)ucode_eqtbl_amd, size, 0);
-		kobj_close_file(eq);
+			if (size == 0 ||
+			    size % sizeof (*ucode_eqtbl_amd) != 0) {
+				kobj_close_file(eq);
+				return (EM_HIGHERREV);
+			}
 
-		if (count != size)
-			return (EM_FILESIZE);
-	}
+			ucode_eqtbl_amd = kmem_zalloc(size, KM_NOSLEEP);
+			if (ucode_eqtbl_amd == NULL) {
+				kobj_close_file(eq);
+				return (EM_NOMEM);
+			}
+			count = kobj_read_file(eq, (char *)ucode_eqtbl_amd,
+			    size, 0);
+			kobj_close_file(eq);
 
-	/* Get the equivalent CPU id. */
-	if (cp->cpu_id)
-		for (eqtbl = ucode_eqtbl_amd;
-		    eqtbl->ue_inst_cpu && eqtbl->ue_inst_cpu != cpi_sig;
-		    eqtbl++)
-			;
+			if (count != size) {
+				ucode_eqtbl_amd_entries = 0;
+				return (EM_FILESIZE);
+			}
 
-	*eq_sig = eqtbl->ue_equiv_cpu;
+			ucode_eqtbl_amd_entries =
+			    size / sizeof (*ucode_eqtbl_amd);
+		}
 
-	/* No equivalent CPU id found, assume outdated microcode file. */
-	if (*eq_sig == 0)
+		eqtbl = ucode_eqtbl_amd;
+		*eq_sig = 0;
+		for (uint_t i = 0; i < ucode_eqtbl_amd_entries; i++, eqtbl++) {
+			if (eqtbl->ue_inst_cpu == 0) {
+				/* End of table */
+				return (EM_HIGHERREV);
+			}
+			if (eqtbl->ue_inst_cpu == cpi_sig) {
+				*eq_sig = eqtbl->ue_equiv_cpu;
+				return (EM_OK);
+			}
+		}
+		/*
+		 * No equivalent CPU id found, assume outdated microcode file.
+		 */
 		return (EM_HIGHERREV);
+	}
 
 	return (EM_OK);
 }
@@ -403,7 +415,6 @@ ucode_chipset_amd(uint8_t *buf, int size)
  *
  * Return EM_OK on success, corresponding error code on failure.
  */
-/*ARGSUSED*/
 static ucode_errno_t
 ucode_locate_amd(cpu_t *cp, cpu_ucode_info_t *uinfop, ucode_file_t *ufp)
 {
@@ -414,7 +425,6 @@ ucode_locate_amd(cpu_t *cp, cpu_ucode_info_t *uinfop, ucode_file_t *ufp)
 
 #ifndef __xpv
 	uint16_t eq_sig = 0;
-	int i;
 
 	/* get equivalent CPU id */
 	if ((rc = ucode_equiv_cpu_amd(cp, &eq_sig)) != EM_OK)
@@ -425,11 +435,13 @@ ucode_locate_amd(cpu_t *cp, cpu_ucode_info_t *uinfop, ucode_file_t *ufp)
 	 * allocated before, check for a matching microcode to avoid loading
 	 * the file again.
 	 */
-	if (ucodefp == NULL)
+
+	if (ucodefp == NULL) {
 		ucodefp = ucode_zalloc(cp->cpu_id, sizeof (*ucodefp));
-	else if (ucode_match_amd(eq_sig, uinfop, ucodefp, sizeof (*ucodefp))
-	    == EM_OK)
+	} else if (ucode_match_amd(eq_sig, uinfop, ucodefp, sizeof (*ucodefp))
+	    == EM_OK) {
 		return (EM_OK);
+	}
 
 	if (ucodefp == NULL)
 		return (EM_NOMEM);
@@ -444,7 +456,7 @@ ucode_locate_amd(cpu_t *cp, cpu_ucode_info_t *uinfop, ucode_file_t *ufp)
 	 * patch that matches.
 	 */
 
-	for (i = 0; i < 0xff; i++) {
+	for (uint_t i = 0; i < 0xff; i++) {
 		(void) snprintf(name, MAXPATHLEN, "/%s/%s/%04X-%02X",
 		    UCODE_INSTALL_PATH, cpuid_getvendorstr(cp), eq_sig, i);
 		if ((fd = kobj_open(name)) == -1)
@@ -663,8 +675,9 @@ ucode_match_amd(uint16_t eq_sig, cpu_ucode_info_t *uinfop,
 	 */
 	if (uh->uh_cpu_rev < 0x5000 &&
 	    size > offsetof(ucode_file_amd_t, uf_code_present) &&
-	    ucodefp->uf_code_present)
+	    ucodefp->uf_code_present) {
 		return (EM_NOMATCH);
+	}
 
 	if (eq_sig != uh->uh_cpu_rev)
 		return (EM_NOMATCH);
@@ -730,51 +743,6 @@ ucode_match_intel(int cpi_sig, cpu_ucode_info_t *uinfop,
 	return (EM_NOMATCH);
 }
 
-/*ARGSUSED*/
-static int
-ucode_write(xc_arg_t arg1, xc_arg_t unused2, xc_arg_t unused3)
-{
-	ucode_update_t *uusp = (ucode_update_t *)arg1;
-	cpu_ucode_info_t *uinfop = CPU->cpu_m.mcpu_ucode_info;
-#ifndef __xpv
-	on_trap_data_t otd;
-#endif
-
-	ASSERT(ucode);
-	ASSERT(uusp->ucodep);
-
-#ifndef	__xpv
-	/*
-	 * Check one more time to see if it is really necessary to update
-	 * microcode just in case this is a hyperthreaded processor where
-	 * the threads share the same microcode.
-	 */
-	if (!ucode_force_update) {
-		ucode->read_rev(uinfop);
-		uusp->new_rev = uinfop->cui_rev;
-		if (uinfop->cui_rev >= uusp->expected_rev)
-			return (0);
-	}
-
-	if (!on_trap(&otd, OT_DATA_ACCESS)) {
-		/*
-		 * On some platforms a cache invalidation is required for the
-		 * ucode update to be successful due to the parts of the
-		 * processor that the microcode is updating.
-		 */
-		invalidate_cache();
-		wrmsr(ucode->write_msr, (uintptr_t)uusp->ucodep);
-	}
-
-	no_trap();
-#endif
-	ucode->read_rev(uinfop);
-	uusp->new_rev = uinfop->cui_rev;
-
-	return (0);
-}
-
-/*ARGSUSED*/
 static uint32_t
 ucode_load_amd(ucode_file_t *ufp, cpu_ucode_info_t *uinfop, cpu_t *cp)
 {
@@ -792,14 +760,14 @@ ucode_load_amd(ucode_file_t *ufp, cpu_ucode_info_t *uinfop, cpu_t *cp)
 	kpreempt_disable();
 	if (on_trap(&otd, OT_DATA_ACCESS)) {
 		no_trap();
-		kpreempt_enable();
-		return (0);
+		goto out;
 	}
 	wrmsr(ucode->write_msr, (uintptr_t)ucodefp);
 	no_trap();
 	ucode->read_rev(uinfop);
-	kpreempt_enable();
 
+out:
+	kpreempt_enable();
 	return (ucodefp->uf_header.uh_patch_id);
 #else
 	uus.ucodep = ucodefp->ucodep;
@@ -812,7 +780,6 @@ ucode_load_amd(ucode_file_t *ufp, cpu_ucode_info_t *uinfop, cpu_t *cp)
 #endif
 }
 
-/*ARGSUSED2*/
 static uint32_t
 ucode_load_intel(ucode_file_t *ufp, cpu_ucode_info_t *uinfop, cpu_t *cp)
 {
@@ -1043,6 +1010,50 @@ ucode_extract_intel(ucode_update_t *uusp, uint8_t *ucodep, int size)
 
 	return (EM_OK);
 }
+
+static int
+ucode_write(xc_arg_t arg1, xc_arg_t unused2, xc_arg_t unused3)
+{
+	ucode_update_t *uusp = (ucode_update_t *)arg1;
+	cpu_ucode_info_t *uinfop = CPU->cpu_m.mcpu_ucode_info;
+#ifndef __xpv
+	on_trap_data_t otd;
+#endif
+
+	ASSERT(ucode);
+	ASSERT(uusp->ucodep);
+
+#ifndef	__xpv
+	/*
+	 * Check one more time to see if it is really necessary to update
+	 * microcode just in case this is a hyperthreaded processor where
+	 * the threads share the same microcode.
+	 */
+	if (!ucode_force_update) {
+		ucode->read_rev(uinfop);
+		uusp->new_rev = uinfop->cui_rev;
+		if (uinfop->cui_rev >= uusp->expected_rev)
+			return (0);
+	}
+
+	if (!on_trap(&otd, OT_DATA_ACCESS)) {
+		/*
+		 * On some platforms a cache invalidation is required for the
+		 * ucode update to be successful due to the parts of the
+		 * processor that the microcode is updating.
+		 */
+		invalidate_cache();
+		wrmsr(ucode->write_msr, (uintptr_t)uusp->ucodep);
+	}
+
+	no_trap();
+#endif
+	ucode->read_rev(uinfop);
+	uusp->new_rev = uinfop->cui_rev;
+
+	return (0);
+}
+
 /*
  * Entry point to microcode update from the ucode_drv driver.
  *
@@ -1156,10 +1167,8 @@ ucode_update(uint8_t *ucodep, int size)
 }
 
 /*
+ * Entry point to microcode update from mlsetup() and mp_startup()
  * Initialize mcpu_ucode_info, and perform microcode update if necessary.
- * This is the entry point from boot path where pointer to CPU structure
- * is available.
- *
  * cpuid_info must be initialized before ucode_check can be called.
  */
 void
@@ -1167,7 +1176,6 @@ ucode_check(cpu_t *cp)
 {
 	cpu_ucode_info_t *uinfop;
 	ucode_errno_t rc = EM_OK;
-	uint32_t new_rev = 0;
 
 	ASSERT(cp);
 	/*
@@ -1222,11 +1230,18 @@ ucode_check(cpu_t *cp)
 	 * Check to see if we need ucode update
 	 */
 	if ((rc = ucode->locate(cp, uinfop, &ucodefile)) == EM_OK) {
+		uint32_t old_rev, new_rev;
+
+		old_rev = uinfop->cui_rev;
 		new_rev = ucode->load(&ucodefile, uinfop, cp);
 
-		if (uinfop->cui_rev != new_rev)
+		if (uinfop->cui_rev != new_rev) {
 			cmn_err(CE_WARN, ucode_failure_fmt, cp->cpu_id,
-			    uinfop->cui_rev, new_rev);
+			    old_rev, new_rev);
+		} else {
+			cmn_err(CE_CONT, ucode_success_fmt, cp->cpu_id,
+			    old_rev, new_rev);
+		}
 	}
 
 	/*