Skip to contentSkip to conversation

x86/microcode: 4.11 queue

PATCH New Borislav Petkov 2 participants 33 messages Jan 20, 2017 – Jan 23, 2017

Subsystems (2)
lore
Share & tools
lore
author maintainer
#13 lore raw

PATCH v2 7/16 x86/microcode/AMD: Rework container parsing

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

It was pretty clumsy before and the whole work of parsing the microcode containers was spread around the functions wrongly.

Clean it up so that there's a main scan_containers() function which iterates over the microcode blob and picks apart the containers glued together. For each container, it calls a parse_container() helper which concentrates on one container only: sanity-checking, parsing, counting microcode patches in there, etc.

It makes much more sense now and it is actually very readable. Oh, and we luvz a diffstat removing more crap than adding.

---
 arch/x86/kernel/cpu/microcode/amd.c | 238 ++++++++++++++++--------------------
 1 file changed, 105 insertions(+), 133 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +105 -133
index 4e2238345308..9d5f4f3626f7 100644
@@ -64,43 +64,6 @@ static u16 this_equiv_id;
 static const char
 ucode_path[] __maybe_unused = "kernel/x86/microcode/AuthenticAMD.bin";
 
-static size_t compute_container_size(u8 *data, u32 total_size)
-{
-	size_t size = 0;
-	u32 *header = (u32 *)data;
-
-	if (header[0] != UCODE_MAGIC ||
-	    header[1] != UCODE_EQUIV_CPU_TABLE_TYPE || /* type */
-	    header[2] == 0)                            /* size */
-		return size;
-
-	size = header[2] + CONTAINER_HDR_SZ;
-	total_size -= size;
-	data += size;
-
-	while (total_size) {
-		u16 patch_size;
-
-		header = (u32 *)data;
-
-		if (header[0] != UCODE_UCODE_TYPE)
-			break;
-
-		/*
-		 * Sanity-check patch size.
-		 */
-		patch_size = header[1];
-		if (patch_size > PATCH_MAX_SIZE)
-			break;
-
-		size	   += patch_size + SECTION_HDR_SIZE;
-		data	   += patch_size + SECTION_HDR_SIZE;
-		total_size -= patch_size + SECTION_HDR_SIZE;
-	}
-
-	return size;
-}
-
 static u16 find_equiv_id(struct equiv_cpu_entry *equiv_table, u32 sig)
 {
 	for (; equiv_table && equiv_table->installed_cpu; equiv_table++) {
@@ -115,80 +78,106 @@ static u16 find_equiv_id(struct equiv_cpu_entry *equiv_table, u32 sig)
  * This scans the ucode blob for the proper container as we can have multiple
  * containers glued together. Returns the equivalence ID from the equivalence
  * table or 0 if none found.
+ * Returns the amount of bytes consumed while scanning. @desc contains all the
+ * data we're going to use in later stages of the application.
  */
-static u16
-find_proper_container(u8 *ucode, size_t size, struct cont_desc *desc)
+static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
 {
-	struct cont_desc ret = { 0 };
-	u32 eax, ebx, ecx, edx;
 	struct equiv_cpu_entry *eq;
-	int offset, left;
-	u16 eq_id = 0;
-	u32 *header;
-	u8 *data;
+	ssize_t orig_size = size;
+	u32 *hdr = (u32 *)ucode;
+	u32 eax, ebx, ecx, edx;
+	u16 eq_id;
+	u8 *buf;
 
-	data   = ucode;
-	left   = size;
-	header = (u32 *)data;
+	/* Am I looking at an equivalence table header? */
+	if (hdr[0] != UCODE_MAGIC ||
+	    hdr[1] != UCODE_EQUIV_CPU_TABLE_TYPE ||
+	    hdr[2] == 0) {
+		desc->eq_id = 0;
+		return CONTAINER_HDR_SZ;
+	}
 
+	buf = ucode;
 
-	/* find equiv cpu table */
-	if (header[0] != UCODE_MAGIC ||
-	    header[1] != UCODE_EQUIV_CPU_TABLE_TYPE || /* type */
-	    header[2] == 0)                            /* size */
-		return eq_id;
+	eq = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
 
-	eax = 0x00000001;
+	eax = 1;
 	ecx = 0;
 	native_cpuid(&eax, &ebx, &ecx, &edx);
 
-	while (left > 0) {
-		eq = (struct equiv_cpu_entry *)(data + CONTAINER_HDR_SZ);
-
-		ret.data = data;
+	/* Find the equivalence ID of our CPU in this table: */
+	eq_id = find_equiv_id(eq, eax);
 
-		/* Advance past the container header */
-		offset = header[2] + CONTAINER_HDR_SZ;
-		data  += offset;
-		left  -= offset;
+	buf  += hdr[2] + CONTAINER_HDR_SZ;
+	size -= hdr[2] + CONTAINER_HDR_SZ;
 
-		eq_id = find_equiv_id(eq, eax);
-		if (eq_id) {
-			ret.size = compute_container_size(ret.data, left + offset);
+	/*
+	 * Scan through the rest of the container to find where it ends. We do
+	 * some basic sanity-checking too.
+	 */
+	while (size > 0) {
+		struct microcode_amd *mc;
+		u32 patch_size;
 
-			/*
-			 * truncate how much we need to iterate over in the
-			 * ucode update loop below
-			 */
-			left = ret.size - offset;
+		hdr = (u32 *)buf;
 
-			*desc = ret;
-			return eq_id;
-		}
+		if (hdr[0] != UCODE_UCODE_TYPE)
+			break;
 
-		/*
-		 * support multiple container files appended together. if this
-		 * one does not have a matching equivalent cpu entry, we fast
-		 * forward to the next container file.
-		 */
-		while (left > 0) {
-			header = (u32 *)data;
+		/* Sanity-check patch size. */
+		patch_size = hdr[1];
+		if (patch_size > PATCH_MAX_SIZE)
+			break;
 
-			if (header[0] == UCODE_MAGIC &&
-			    header[1] == UCODE_EQUIV_CPU_TABLE_TYPE)
-				break;
+		/* Skip patch section header: */
+		buf  += SECTION_HDR_SIZE;
+		size -= SECTION_HDR_SIZE;
 
-			offset = header[1] + SECTION_HDR_SIZE;
-			data  += offset;
-			left  -= offset;
+		mc = (struct microcode_amd *)buf;
+		if (eq_id == mc->hdr.processor_rev_id) {
+			desc->psize = patch_size;
+			desc->mc = mc;
 		}
 
-		/* mark where the next microcode container file starts */
-		offset    = data - (u8 *)ucode;
-		ucode     = data;
+		buf  += patch_size;
+		size -= patch_size;
 	}
 
-	return eq_id;
+	/*
+	 * If we have found a patch (desc->mc), it means we're looking at the
+	 * container which has a patch for this CPU so return 0 to mean, @ucode
+	 * already points to the proper container. Otherwise, we return the size
+	 * we scanned so that we can advance to the next container in the
+	 * buffer.
+	 */
+	if (desc->mc) {
+		desc->eq_id = eq_id;
+		desc->data  = ucode;
+		desc->size  = orig_size - size;
+
+		return 0;
+	}
+
+	return orig_size - size;
+}
+
+/*
+ * Scan the ucode blob for the proper container as we can have multiple
+ * containers glued together.
+ */
+static void scan_containers(u8 *ucode, size_t size, struct cont_desc *desc)
+{
+	ssize_t rem = size;
+
+	while (rem >= 0) {
+		ssize_t s = parse_container(ucode, rem, desc);
+		if (!s)
+			return;
+
+		ucode += s;
+		rem   -= s;
+	}
 }
 
 static int __apply_microcode_amd(struct microcode_amd *mc)
@@ -214,17 +203,16 @@ static int __apply_microcode_amd(struct microcode_amd *mc)
  * load_microcode_amd() to save equivalent cpu table and microcode patches in
  * kernel heap memory.
  *
- * Returns true if container found (sets @ret_cont), false otherwise.
+ * Returns true if container found (sets @desc), false otherwise.
  */
 static bool apply_microcode_early_amd(void *ucode, size_t size, bool save_patch,
-				      struct cont_desc *desc)
+				      struct cont_desc *ret_desc)
 {
+	struct cont_desc desc = { 0 };
 	u8 (*patch)[PATCH_MAX_SIZE];
-	u32 rev, *header, *new_rev;
-	struct cont_desc ret;
-	int offset, left;
-	u16 eq_id = 0;
-	u8  *data;
+	struct microcode_amd *mc;
+	u32 rev, *new_rev;
+	bool ret = false;
 
 #ifdef CONFIG_X86_32
 	new_rev = (u32 *)__pa_nodebug(&ucode_new_rev);
@@ -237,47 +225,31 @@ static bool apply_microcode_early_amd(void *ucode, size_t size, bool save_patch,
 	if (check_current_patch_level(&rev, true))
 		return false;
 
-	eq_id = find_proper_container(ucode, size, &ret);
-	if (!eq_id)
-		return false;
-
-	this_equiv_id = eq_id;
-	header = (u32 *)ret.data;
-
-	/* We're pointing to an equiv table, skip over it. */
-	data = ret.data +  header[2] + CONTAINER_HDR_SZ;
-	left = ret.size - (header[2] + CONTAINER_HDR_SZ);
-
-	while (left > 0) {
-		struct microcode_amd *mc;
-
-		header = (u32 *)data;
-		if (header[0] != UCODE_UCODE_TYPE || /* type */
-		    header[1] == 0)                  /* size */
-			break;
+	scan_containers(ucode, size, &desc);
+	if (!desc.eq_id)
+		return ret;
 
-		mc = (struct microcode_amd *)(data + SECTION_HDR_SIZE);
+	this_equiv_id = desc.eq_id;
 
-		if (eq_id == mc->hdr.processor_rev_id && rev < mc->hdr.patch_id) {
+	mc = desc.mc;
+	if (!mc)
+		return ret;
 
-			if (!__apply_microcode_amd(mc)) {
-				rev = mc->hdr.patch_id;
-				*new_rev = rev;
+	if (rev >= mc->hdr.patch_id)
+		return ret;
 
-				if (save_patch)
-					memcpy(patch, mc, min_t(u32, header[1], PATCH_MAX_SIZE));
-			}
-		}
+	if (!__apply_microcode_amd(mc)) {
+		*new_rev = mc->hdr.patch_id;
+		ret      = true;
 
-		offset  = header[1] + SECTION_HDR_SIZE;
-		data   += offset;
-		left   -= offset;
+		if (save_patch)
+			memcpy(patch, mc, min_t(u32, desc.psize, PATCH_MAX_SIZE));
 	}
 
-	if (desc)
-		*desc = ret;
+	if (ret_desc)
+		*ret_desc = desc;
 
-	return true;
+	return ret;
 }
 
 static bool get_builtin_microcode(struct cpio_data *cp, unsigned int family)
@@ -396,6 +368,7 @@ void load_ucode_amd_ap(unsigned int family)
 		}
 
 		if (!apply_microcode_early_amd(cp.data, cp.size, false, &cont)) {
+			cont.data = NULL;
 			cont.size = -1;
 			return;
 		}
@@ -434,7 +407,6 @@ int __init save_microcode_in_initrd_amd(unsigned int fam)
 {
 	enum ucode_state ret;
 	int retval = 0;
-	u16 eq_id;
 
 	if (!cont.data) {
 		if (IS_ENABLED(CONFIG_X86_32) && (cont.size != -1)) {
@@ -450,8 +422,8 @@ int __init save_microcode_in_initrd_amd(unsigned int fam)
 				return -EINVAL;
 			}
 
-			eq_id = find_proper_container(cp.data, cp.size, &cont);
-			if (!eq_id) {
+			scan_containers(cp.data, cp.size, &cont);
+			if (!cont.eq_id) {
 				cont.size = -1;
 				return -EINVAL;
 			}
Signature · 1 line
2.11.0
author maintainer
#23 lore raw

PATCH v2 12/16 x86/microcode/AMD: Check patch level only on the BSP

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

Check final patch levels for AMD only on the BSP. This way, we decide early and only once whether to continue loading or to leave the loader disabled on such systems.

Simplify a lot.

---
 arch/x86/include/asm/microcode_amd.h |  2 -
 arch/x86/kernel/cpu/microcode/amd.c  | 78 +++++-------------------------------
 arch/x86/kernel/cpu/microcode/core.c | 41 +++++++++++++++++++
 3 files changed, 52 insertions(+), 69 deletions(-)
arch/x86/include/asm/microcode_amd.h +0 -2
index 3e3e20be829a..3d57009e168b 100644
@@ -54,6 +54,4 @@ static inline int __init
 save_microcode_in_initrd_amd(unsigned int family) { return -EINVAL; }
 void reload_ucode_amd(void) {}
 #endif
-
-extern bool check_current_patch_level(u32 *rev, bool early);
 #endif /* _ASM_X86_MICROCODE_AMD_H */
arch/x86/kernel/cpu/microcode/amd.c +11 -67

139 lines not on this page · raw message

arch/x86/kernel/cpu/microcode/core.c +41 -0

53 lines not on this page · raw message

Signature · 1 line
2.11.0
author maintainer
#15 lore raw

PATCH v2 8/16 x86/microcode: Decrease CPUID use

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

Get CPUID(1).EAX value once per CPU and propagate value into the callers instead of conveniently calling it every time.

---
 arch/x86/kernel/cpu/microcode/amd.c  | 52 +++++++++++++++++-------------------
 arch/x86/kernel/cpu/microcode/core.c | 38 ++++++++++----------------
 2 files changed, 38 insertions(+), 52 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +24 -28
index 9d5f4f3626f7..9fb398e64b0a 100644
@@ -47,6 +47,7 @@ static struct equiv_cpu_entry *equiv_cpu_table;
  */
 static struct cont_desc {
 	struct microcode_amd *mc;
+	u32		     cpuid_1_eax;
 	u32		     psize;
 	u16		     eq_id;
 	u8		     *data;
@@ -86,7 +87,6 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
 	struct equiv_cpu_entry *eq;
 	ssize_t orig_size = size;
 	u32 *hdr = (u32 *)ucode;
-	u32 eax, ebx, ecx, edx;
 	u16 eq_id;
 	u8 *buf;
 
@@ -102,12 +102,8 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
 
 	eq = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
 
-	eax = 1;
-	ecx = 0;
-	native_cpuid(&eax, &ebx, &ecx, &edx);
-
 	/* Find the equivalence ID of our CPU in this table: */
-	eq_id = find_equiv_id(eq, eax);
+	eq_id = find_equiv_id(eq, desc->cpuid_1_eax);
 
 	buf  += hdr[2] + CONTAINER_HDR_SZ;
 	size -= hdr[2] + CONTAINER_HDR_SZ;
@@ -205,8 +201,9 @@ static int __apply_microcode_amd(struct microcode_amd *mc)
  *
  * Returns true if container found (sets @desc), false otherwise.
  */
-static bool apply_microcode_early_amd(void *ucode, size_t size, bool save_patch,
-				      struct cont_desc *ret_desc)
+static bool
+apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size,
+			  bool save_patch, struct cont_desc *ret_desc)
 {
 	struct cont_desc desc = { 0 };
 	u8 (*patch)[PATCH_MAX_SIZE];
@@ -225,6 +222,8 @@ static bool apply_microcode_early_amd(void *ucode, size_t size, bool save_patch,
 	if (check_current_patch_level(&rev, true))
 		return false;
 
+	desc.cpuid_1_eax = cpuid_1_eax;
+
 	scan_containers(ucode, size, &desc);
 	if (!desc.eq_id)
 		return ret;
@@ -267,10 +266,9 @@ static bool get_builtin_microcode(struct cpio_data *cp, unsigned int family)
 #endif
 }
 
-void __init load_ucode_amd_bsp(unsigned int family)
+void __init load_ucode_amd_bsp(unsigned int cpuid_1_eax)
 {
 	struct ucode_cpu_info *uci;
-	u32 eax, ebx, ecx, edx;
 	struct cpio_data cp;
 	const char *path;
 	bool use_pa;
@@ -285,19 +283,16 @@ void __init load_ucode_amd_bsp(unsigned int family)
 		use_pa	= false;
 	}
 
-	if (!get_builtin_microcode(&cp, family))
+	if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)))
 		cp = find_microcode_in_initrd(path, use_pa);
 
 	if (!(cp.data && cp.size))
 		return;
 
-	/* Get BSP's CPUID.EAX(1), needed in load_microcode_amd() */
-	eax = 1;
-	ecx = 0;
-	native_cpuid(&eax, &ebx, &ecx, &edx);
-	uci->cpu_sig.sig = eax;
+	/* Needed in load_microcode_amd() */
+	uci->cpu_sig.sig = cpuid_1_eax;
 
-	apply_microcode_early_amd(cp.data, cp.size, true, NULL);
+	apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, true, NULL);
 }
 
 #ifdef CONFIG_X86_32
@@ -308,7 +303,7 @@ void __init load_ucode_amd_bsp(unsigned int family)
  * In save_microcode_in_initrd_amd() BSP's patch is copied to amd_ucode_patch,
  * which is used upon resume from suspend.
  */
-void load_ucode_amd_ap(unsigned int family)
+void load_ucode_amd_ap(unsigned int cpuid_1_eax)
 {
 	struct microcode_amd *mc;
 	struct cpio_data cp;
@@ -319,7 +314,7 @@ void load_ucode_amd_ap(unsigned int family)
 		return;
 	}
 
-	if (!get_builtin_microcode(&cp, family))
+	if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)))
 		cp = find_microcode_in_initrd((const char *)__pa_nodebug(ucode_path), true);
 
 	if (!(cp.data && cp.size))
@@ -329,14 +324,14 @@ void load_ucode_amd_ap(unsigned int family)
 	 * This would set amd_ucode_patch above so that the following APs can
 	 * use it directly instead of going down this path again.
 	 */
-	apply_microcode_early_amd(cp.data, cp.size, true, NULL);
+	apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, true, NULL);
 }
 #else
-void load_ucode_amd_ap(unsigned int family)
+void load_ucode_amd_ap(unsigned int cpuid_1_eax)
 {
 	struct equiv_cpu_entry *eq;
 	struct microcode_amd *mc;
-	u32 rev, eax;
+	u32 rev;
 	u16 eq_id;
 
 	/* 64-bit runs with paging enabled, thus early==false. */
@@ -351,7 +346,7 @@ void load_ucode_amd_ap(unsigned int family)
 			return;
 
 reget:
-		if (!get_builtin_microcode(&cp, family)) {
+		if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax))) {
 #ifdef CONFIG_BLK_DEV_INITRD
 			cp = find_cpio_data(ucode_path, (void *)initrd_start,
 					    initrd_end - initrd_start, NULL);
@@ -367,17 +362,16 @@ void load_ucode_amd_ap(unsigned int family)
 			}
 		}
 
-		if (!apply_microcode_early_amd(cp.data, cp.size, false, &cont)) {
+		if (!apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, false, &cont)) {
 			cont.data = NULL;
 			cont.size = -1;
 			return;
 		}
 	}
 
-	eax = cpuid_eax(0x00000001);
 	eq  = (struct equiv_cpu_entry *)(cont.data + CONTAINER_HDR_SZ);
 
-	eq_id = find_equiv_id(eq, eax);
+	eq_id = find_equiv_id(eq, cpuid_1_eax);
 	if (!eq_id)
 		return;
 
@@ -403,7 +397,7 @@ void load_ucode_amd_ap(unsigned int family)
 static enum ucode_state
 load_microcode_amd(int cpu, u8 family, const u8 *data, size_t size);
 
-int __init save_microcode_in_initrd_amd(unsigned int fam)
+int __init save_microcode_in_initrd_amd(unsigned int cpuid_1_eax)
 {
 	enum ucode_state ret;
 	int retval = 0;
@@ -422,6 +416,8 @@ int __init save_microcode_in_initrd_amd(unsigned int fam)
 				return -EINVAL;
 			}
 
+			cont.cpuid_1_eax = cpuid_1_eax;
+
 			scan_containers(cp.data, cp.size, &cont);
 			if (!cont.eq_id) {
 				cont.size = -1;
@@ -432,7 +428,7 @@ int __init save_microcode_in_initrd_amd(unsigned int fam)
 			return -EINVAL;
 	}
 
-	ret = load_microcode_amd(smp_processor_id(), fam, cont.data, cont.size);
+	ret = load_microcode_amd(smp_processor_id(), x86_family(cpuid_1_eax), cont.data, cont.size);
 	if (ret != UCODE_OK)
 		retval = -EINVAL;
 
arch/x86/kernel/cpu/microcode/core.c +14 -24
index 2af69d27da62..437996c9be67 100644
@@ -64,10 +64,6 @@ static DEFINE_MUTEX(microcode_mutex);
 
 struct ucode_cpu_info		ucode_cpu_info[NR_CPUS];
 
-/*
- * Operations that are run on a target cpu:
- */
-
 struct cpu_info_ctx {
 	struct cpu_signature	*cpu_sig;
 	int			err;
@@ -76,7 +72,6 @@ struct cpu_info_ctx {
 static bool __init check_loader_disabled_bsp(void)
 {
 	static const char *__dis_opt_str = "dis_ucode_ldr";
-	u32 a, b, c, d;
 
 #ifdef CONFIG_X86_32
 	const char *cmdline = (const char *)__pa_nodebug(boot_command_line);
@@ -92,16 +87,12 @@ static bool __init check_loader_disabled_bsp(void)
 	if (!have_cpuid_p())
 		return *res;
 
-	a = 1;
-	c = 0;
-	native_cpuid(&a, &b, &c, &d);
-
 	/*
 	 * CPUID(1).ECX[31]: reserved for hypervisor use. This is still not
 	 * completely accurate as xen pv guests don't see that CPUID bit set but
 	 * that's good enough as they don't land on the BSP path anyway.
 	 */
-	if (c & BIT(31))
+	if (native_cpuid_ecx(1) & BIT(31))
 		return *res;
 
 	if (cmdline_find_option_bool(cmdline, option) <= 0)
@@ -131,23 +122,22 @@ bool get_builtin_firmware(struct cpio_data *cd, const char *name)
 
 void __init load_ucode_bsp(void)
 {
-	int vendor;
-	unsigned int family;
+	unsigned int vendor, cpuid_1_eax;
 
 	if (check_loader_disabled_bsp())
 		return;
 
-	vendor = x86_cpuid_vendor();
-	family = x86_cpuid_family();
+	vendor	    = x86_cpuid_vendor();
+	cpuid_1_eax = native_cpuid_eax(1);
 
 	switch (vendor) {
 	case X86_VENDOR_INTEL:
-		if (family >= 6)
+		if (x86_family(cpuid_1_eax) >= 6)
 			load_ucode_intel_bsp();
 		break;
 	case X86_VENDOR_AMD:
-		if (family >= 0x10)
-			load_ucode_amd_bsp(family);
+		if (x86_family(cpuid_1_eax) >= 0x10)
+			load_ucode_amd_bsp(cpuid_1_eax);
 		break;
 	default:
 		break;
@@ -165,22 +155,22 @@ static bool check_loader_disabled_ap(void)
 
 void load_ucode_ap(void)
 {
-	int vendor, family;
+	unsigned int vendor, cpuid_1_eax;
 
 	if (check_loader_disabled_ap())
 		return;
 
-	vendor = x86_cpuid_vendor();
-	family = x86_cpuid_family();
+	vendor	    = x86_cpuid_vendor();
+	cpuid_1_eax = native_cpuid_eax(1);
 
 	switch (vendor) {
 	case X86_VENDOR_INTEL:
-		if (family >= 6)
+		if (x86_family(cpuid_1_eax) >= 6)
 			load_ucode_intel_ap();
 		break;
 	case X86_VENDOR_AMD:
-		if (family >= 0x10)
-			load_ucode_amd_ap(family);
+		if (x86_family(cpuid_1_eax) >= 0x10)
+			load_ucode_amd_ap(cpuid_1_eax);
 		break;
 	default:
 		break;
@@ -198,7 +188,7 @@ static int __init save_microcode_in_initrd(void)
 		break;
 	case X86_VENDOR_AMD:
 		if (c->x86 >= 0x10)
-			return save_microcode_in_initrd_amd(c->x86);
+			return save_microcode_in_initrd_amd(cpuid_eax(1));
 		break;
 	default:
 		break;
Signature · 1 line
2.11.0
author maintainer
#25 lore raw

PATCH v2 13/16 x86/microcode/AMD: Unify load_ucode_amd_ap()

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

Use a version for both bitness by adding a helper which does the actual container finding and parsing which can be used on any CPU - BSP or AP. Streamlines the paths more.

---
 arch/x86/kernel/cpu/microcode/amd.c | 81 ++++++++++++++-----------------------
 1 file changed, 31 insertions(+), 50 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +31 -50
index 61743476c25b..fe9e865480af 100644
@@ -261,7 +261,7 @@ static bool get_builtin_microcode(struct cpio_data *cp, unsigned int family)
 #endif
 }
 
-void __init load_ucode_amd_bsp(unsigned int cpuid_1_eax)
+void __load_ucode_amd(unsigned int cpuid_1_eax, struct cpio_data *ret)
 {
 	struct ucode_cpu_info *uci;
 	struct cpio_data cp;
@@ -281,89 +281,71 @@ void __init load_ucode_amd_bsp(unsigned int cpuid_1_eax)
 	if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)))
 		cp = find_microcode_in_initrd(path, use_pa);
 
-	if (!(cp.data && cp.size))
-		return;
-
 	/* Needed in load_microcode_amd() */
 	uci->cpu_sig.sig = cpuid_1_eax;
 
-	apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, true, NULL);
+	*ret = cp;
 }
 
-#ifdef CONFIG_X86_32
-/*
- * On 32-bit, since AP's early load occurs before paging is turned on, we
- * cannot traverse cpu_equiv_table and microcode_cache in kernel heap memory.
- * So during cold boot, AP will apply_ucode_in_initrd() just like the BSP.
- * In save_microcode_in_initrd_amd() BSP's patch is copied to amd_ucode_patch,
- * which is used upon resume from suspend.
- */
-void load_ucode_amd_ap(unsigned int cpuid_1_eax)
+void __init load_ucode_amd_bsp(unsigned int cpuid_1_eax)
 {
-	struct microcode_amd *mc;
-	struct cpio_data cp;
+	struct cpio_data cp = { };
 
-	mc = (struct microcode_amd *)__pa_nodebug(amd_ucode_patch);
-	if (mc->hdr.patch_id && mc->hdr.processor_rev_id) {
-		__apply_microcode_amd(mc);
-		return;
-	}
-
-	if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)))
-		cp = find_microcode_in_initrd((const char *)__pa_nodebug(ucode_path), true);
+	__load_ucode_amd(cpuid_1_eax, &cp);
 
 	if (!(cp.data && cp.size))
 		return;
 
-	/*
-	 * This would set amd_ucode_patch above so that the following APs can
-	 * use it directly instead of going down this path again.
-	 */
 	apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, true, NULL);
 }
-#else
+
 void load_ucode_amd_ap(unsigned int cpuid_1_eax)
 {
 	struct equiv_cpu_entry *eq;
 	struct microcode_amd *mc;
+	struct cont_desc *desc;
 	u16 eq_id;
 
+	if (IS_ENABLED(CONFIG_X86_32)) {
+		mc   = (struct microcode_amd *)__pa_nodebug(amd_ucode_patch);
+		desc = (struct cont_desc *)__pa_nodebug(&cont);
+	} else {
+		mc   = (struct microcode_amd *)amd_ucode_patch;
+		desc = &cont;
+	}
+
 	/* First AP hasn't cached it yet, go through the blob. */
-	if (!cont.data) {
-		struct cpio_data cp;
+	if (!desc->data) {
+		struct cpio_data cp = { };
 
-		if (cont.size == -1)
+		if (desc->size == -1)
 			return;
 
 reget:
-		if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax))) {
-			cp = find_microcode_in_initrd(ucode_path, false);
-
-			if (!(cp.data && cp.size)) {
-				/*
-				 * Mark it so that other APs do not scan again
-				 * for no real reason and slow down boot
-				 * needlessly.
-				 */
-				cont.size = -1;
-				return;
-			}
+		__load_ucode_amd(cpuid_1_eax, &cp);
+		if (!(cp.data && cp.size)) {
+			/*
+			 * Mark it so that other APs do not scan again for no
+			 * real reason and slow down boot needlessly.
+			 */
+			desc->size = -1;
+			return;
 		}
 
-		if (!apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, false, &cont)) {
-			cont.data = NULL;
-			cont.size = -1;
+		if (!apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, false, desc)) {
+			desc->data = NULL;
+			desc->size = -1;
 			return;
 		}
 	}
 
-	eq  = (struct equiv_cpu_entry *)(cont.data + CONTAINER_HDR_SZ);
+	eq  = (struct equiv_cpu_entry *)(desc->data + CONTAINER_HDR_SZ);
 
 	eq_id = find_equiv_id(eq, cpuid_1_eax);
 	if (!eq_id)
 		return;
 
-	if (eq_id == cont.eq_id) {
+	if (eq_id == desc->eq_id) {
 		u32 rev, dummy;
 
 		native_rdmsr(MSR_AMD64_PATCH_LEVEL, rev, dummy);
@@ -384,7 +366,6 @@ void load_ucode_amd_ap(unsigned int cpuid_1_eax)
 		goto reget;
 	}
 }
-#endif /* CONFIG_X86_32 */
 
 static enum ucode_state
 load_microcode_amd(int cpu, u8 family, const u8 *data, size_t size);
Signature · 1 line
2.11.0
author maintainer
#29 lore raw

PATCH v2 15/16 x86/microcode/AMD: Remove AP scanning optimization

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

The idea was to not scan the microcode blob on each AP (Application Processor) during boot and thus save us some milliseconds. However, on architectures where the microcode engine is shared between threads, this doesn't work. Here's why:

The microcode on CPU0, i.e., the first thread, gets updated. The second thread, i.e., CPU1, i.e., the first AP walks into load_ucode_amd_ap(), sees that there's no container cached and goes and scans for the proper blob.

It finds it and as a last step of apply_microcode_early_amd(), it tries to apply the patch but that core has already the updated microcode revision which it has received through CPU0's update. So it returns false and we do desc->size = -1 to prevent other APs from scanning.

However, the next AP, CPU2, has a different microcode engine which hasn't been updated yet. The desc->size == -1 test prevents it from scanning the blob anew and we fail to update it.

The fix is much more straight-forward than it looks: the BSP (BootStrapping Processor), i.e., CPU0, caches the microcode patch in amd_ucode_patch. We use that on the AP and try to apply it. In the 99.9999% of cases where we have homogeneous cores - *not* mixed-steppings - the application will be successful and we're good to go.

In the remaining small set of systems, we will simply rescan the blob and find (or not, if none present) the proper patch and apply it then.

---
 arch/x86/kernel/cpu/microcode/amd.c | 78 +++++++++----------------------------
 1 file changed, 18 insertions(+), 60 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +18 -60
index 2a194e384207..5e1b57747c2f 100644
@@ -45,14 +45,14 @@ static struct equiv_cpu_entry *equiv_cpu_table;
  * save from the initrd/builtin before jettisoning its contents. @mc is the
  * microcode patch we found to match.
  */
-static struct cont_desc {
+struct cont_desc {
 	struct microcode_amd *mc;
 	u32		     cpuid_1_eax;
 	u32		     psize;
 	u16		     eq_id;
 	u8		     *data;
 	size_t		     size;
-} cont;
+};
 
 static u32 ucode_new_rev;
 static u8 amd_ucode_patch[PATCH_MAX_SIZE];
@@ -201,8 +201,7 @@ static int __apply_microcode_amd(struct microcode_amd *mc)
  * Returns true if container found (sets @desc), false otherwise.
  */
 static bool
-apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size,
-			  bool save_patch, struct cont_desc *ret_desc)
+apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size, bool save_patch)
 {
 	struct cont_desc desc = { 0 };
 	u8 (*patch)[PATCH_MAX_SIZE];
@@ -240,9 +239,6 @@ apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size,
 			memcpy(patch, mc, min_t(u32, desc.psize, PATCH_MAX_SIZE));
 	}
 
-	if (ret_desc)
-		*ret_desc = desc;
-
 	return ret;
 }
 
@@ -292,79 +288,41 @@ void __init load_ucode_amd_bsp(unsigned int cpuid_1_eax)
 	struct cpio_data cp = { };
 
 	__load_ucode_amd(cpuid_1_eax, &cp);
-
 	if (!(cp.data && cp.size))
 		return;
 
-	apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, true, NULL);
+	apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, true);
 }
 
 void load_ucode_amd_ap(unsigned int cpuid_1_eax)
 {
-	struct equiv_cpu_entry *eq;
 	struct microcode_amd *mc;
-	struct cont_desc *desc;
-	u16 eq_id;
+	struct cpio_data cp;
+	u32 *new_rev, rev, dummy;
 
 	if (IS_ENABLED(CONFIG_X86_32)) {
-		mc   = (struct microcode_amd *)__pa_nodebug(amd_ucode_patch);
-		desc = (struct cont_desc *)__pa_nodebug(&cont);
+		mc	= (struct microcode_amd *)__pa_nodebug(amd_ucode_patch);
+		new_rev = (u32 *)__pa_nodebug(&ucode_new_rev);
 	} else {
-		mc   = (struct microcode_amd *)amd_ucode_patch;
-		desc = &cont;
+		mc	= (struct microcode_amd *)amd_ucode_patch;
+		new_rev = &ucode_new_rev;
 	}
 
-	/* First AP hasn't cached it yet, go through the blob. */
-	if (!desc->data) {
-		struct cpio_data cp = { };
-
-		if (desc->size == -1)
-			return;
-
-reget:
-		__load_ucode_amd(cpuid_1_eax, &cp);
-		if (!(cp.data && cp.size)) {
-			/*
-			 * Mark it so that other APs do not scan again for no
-			 * real reason and slow down boot needlessly.
-			 */
-			desc->size = -1;
-			return;
-		}
+	native_rdmsr(MSR_AMD64_PATCH_LEVEL, rev, dummy);
 
-		if (!apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, false, desc)) {
-			desc->data = NULL;
-			desc->size = -1;
+	/* Check whether we have saved a new patch already: */
+	if (*new_rev && rev < mc->hdr.patch_id) {
+		if (!__apply_microcode_amd(mc)) {
+			*new_rev = mc->hdr.patch_id;
 			return;
 		}
 	}
 
-	eq  = (struct equiv_cpu_entry *)(desc->data + CONTAINER_HDR_SZ);
-
-	eq_id = find_equiv_id(eq, cpuid_1_eax);
-	if (!eq_id)
+	__load_ucode_amd(cpuid_1_eax, &cp);
+	if (!(cp.data && cp.size))
 		return;
 
-	if (eq_id == desc->eq_id) {
-		u32 rev, dummy;
-
-		native_rdmsr(MSR_AMD64_PATCH_LEVEL, rev, dummy);
-
-		mc = (struct microcode_amd *)amd_ucode_patch;
-
-		if (mc && rev < mc->hdr.patch_id) {
-			if (!__apply_microcode_amd(mc))
-				ucode_new_rev = mc->hdr.patch_id;
-		}
-
-	} else {
-
-		/*
-		 * AP has a different equivalence ID than BSP, looks like
-		 * mixed-steppings silicon so go through the ucode blob anew.
-		 */
-		goto reget;
-	}
+	apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, false);
 }
 
 static enum ucode_state
Signature · 1 line
2.11.0
author maintainer
#27 lore raw

PATCH v2 14/16 x86/microcode/AMD: Simplify saving from initrd

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

No need to use the previously stashed info in the container - simply go ahead and parse the initrd once more. It simplifies and streamlines the code a whole lot.

---
 arch/x86/kernel/cpu/microcode/amd.c | 43 +++++++++++--------------------------
 1 file changed, 13 insertions(+), 30 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +13 -30
index fe9e865480af..2a194e384207 100644
@@ -372,43 +372,26 @@ load_microcode_amd(int cpu, u8 family, const u8 *data, size_t size);
 
 int __init save_microcode_in_initrd_amd(unsigned int cpuid_1_eax)
 {
+	struct cont_desc desc = { 0 };
 	enum ucode_state ret;
-	int retval = 0;
-
-	if (!cont.data) {
-		if (IS_ENABLED(CONFIG_X86_32) && (cont.size != -1)) {
-			struct cpio_data cp;
-
-			cp = find_microcode_in_initrd(ucode_path, false);
-			if (!(cp.data && cp.size)) {
-				cont.size = -1;
-				return -EINVAL;
-			}
+	struct cpio_data cp;
 
-			cont.cpuid_1_eax = cpuid_1_eax;
+	cp = find_microcode_in_initrd(ucode_path, false);
+	if (!(cp.data && cp.size))
+		return -EINVAL;
 
-			scan_containers(cp.data, cp.size, &cont);
-			if (!cont.eq_id) {
-				cont.size = -1;
-				return -EINVAL;
-			}
+	desc.cpuid_1_eax = cpuid_1_eax;
 
-		} else
-			return -EINVAL;
-	}
+	scan_containers(cp.data, cp.size, &desc);
+	if (!desc.eq_id)
+		return -EINVAL;
 
-	ret = load_microcode_amd(smp_processor_id(), x86_family(cpuid_1_eax), cont.data, cont.size);
+	ret = load_microcode_amd(smp_processor_id(), x86_family(cpuid_1_eax),
+				 desc.data, desc.size);
 	if (ret != UCODE_OK)
-		retval = -EINVAL;
-
-	/*
-	 * This will be freed any msec now, stash patches for the current
-	 * family and switch to patch cache for cpu hotplug, etc later.
-	 */
-	cont.data = NULL;
-	cont.size = 0;
+		return -EINVAL;
 
-	return retval;
+	return 0;
 }
 
 void reload_ucode_amd(void)
Signature · 1 line
2.11.0
author maintainer
#17 lore raw

PATCH v2 9/16 x86/microcode/AMD: Get rid of global this_equiv_id

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

We have a container which we update/prepare each time before applying a microcode patch instead of using a global.

---
 arch/x86/kernel/cpu/microcode/amd.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +1 -4
index 9fb398e64b0a..4cbfe70cf458 100644
@@ -56,7 +56,6 @@ static struct cont_desc {
 
 static u32 ucode_new_rev;
 static u8 amd_ucode_patch[PATCH_MAX_SIZE];
-static u16 this_equiv_id;
 
 /*
  * Microcode patch container file is prepended to the initrd in cpio
@@ -228,8 +227,6 @@ apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size,
 	if (!desc.eq_id)
 		return ret;
 
-	this_equiv_id = desc.eq_id;
-
 	mc = desc.mc;
 	if (!mc)
 		return ret;
@@ -375,7 +372,7 @@ void load_ucode_amd_ap(unsigned int cpuid_1_eax)
 	if (!eq_id)
 		return;
 
-	if (eq_id == this_equiv_id) {
+	if (eq_id == cont.eq_id) {
 		mc = (struct microcode_amd *)amd_ucode_patch;
 
 		if (mc && rev < mc->hdr.patch_id) {
Signature · 1 line
2.11.0
author maintainer
#1 lore raw

PATCH v2 1/16 x86/microcode/intel: Drop stashed AP patch pointer optimization

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

This was meant to save us the scanning of the microcode containter in the initrd since the first AP had already done that but it can also hurt us:

Imagine a single hyperthreaded CPU (Intel(R) Atom(TM) CPU N270, for example) which updates the microcode on the BSP but since the microcode engine is shared between the two threads, the update on CPU1 doesn't happen because it has already happened on CPU0 and we don't find a newer microcode revision on CPU1.

Which doesn't set the intel_ucode_patch pointer and at initrd jettisoning time we don't save the microcode patch for later application.

Now, when we suspend to RAM, the loaded microcode gets cleared so we need to reload but there's no patch saved in the cache.

Removing the optimization fixes this issue and all is fine and dandy.

Fixes 06b8534cb728 ("x86/microcode: Rework microcode loading")
---
 arch/x86/kernel/cpu/microcode/intel.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)
arch/x86/kernel/cpu/microcode/intel.c +1 -8
index 3f329b74e040..8325d8a09ab0 100644
@@ -41,7 +41,7 @@
 
 static const char ucode_path[] = "kernel/x86/microcode/GenuineIntel.bin";
 
-/* Current microcode patch used in early patching */
+/* Current microcode patch used in early patching on the APs. */
 struct microcode_intel *intel_ucode_patch;
 
 static inline bool cpu_signatures_match(unsigned int s1, unsigned int p1,
@@ -607,12 +607,6 @@ int __init save_microcode_in_initrd_intel(void)
 	struct ucode_cpu_info uci;
 	struct cpio_data cp;
 
-	/*
-	 * AP loading didn't find any microcode patch, no need to save anything.
-	 */
-	if (!intel_ucode_patch || IS_ERR(intel_ucode_patch))
-		return 0;
-
 	if (!load_builtin_intel_microcode(&cp))
 		cp = find_microcode_in_initrd(ucode_path, false);
 
@@ -628,7 +622,6 @@ int __init save_microcode_in_initrd_intel(void)
 	return 0;
 }
 
-
 /*
  * @res_patch, output: a pointer to the patch we found.
  */
Signature · 1 line
2.11.0
author maintainer
#31 lore raw

PATCH v2 16/16 x86/microcode/AMD: Remove struct cont_desc.eq_id

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

The equivalence ID was needed outside of the container scanning logic but now, after this has been cleaned up, not anymore. Now, cont_desc.mc is used to denote whether the container we're looking at has the proper microcode patch for this CPU or not.

---
 arch/x86/kernel/cpu/microcode/amd.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +4 -10
index 5e1b57747c2f..7889ae492af0 100644
@@ -49,7 +49,6 @@ struct cont_desc {
 	struct microcode_amd *mc;
 	u32		     cpuid_1_eax;
 	u32		     psize;
-	u16		     eq_id;
 	u8		     *data;
 	size_t		     size;
 };
@@ -92,10 +91,8 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
 	/* Am I looking at an equivalence table header? */
 	if (hdr[0] != UCODE_MAGIC ||
 	    hdr[1] != UCODE_EQUIV_CPU_TABLE_TYPE ||
-	    hdr[2] == 0) {
-		desc->eq_id = 0;
+	    hdr[2] == 0)
 		return CONTAINER_HDR_SZ;
-	}
 
 	buf = ucode;
 
@@ -147,9 +144,8 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
 	 * buffer.
 	 */
 	if (desc->mc) {
-		desc->eq_id = eq_id;
-		desc->data  = ucode;
-		desc->size  = orig_size - size;
+		desc->data = ucode;
+		desc->size = orig_size - size;
 
 		return 0;
 	}
@@ -220,8 +216,6 @@ apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size, bool save_p
 	desc.cpuid_1_eax = cpuid_1_eax;
 
 	scan_containers(ucode, size, &desc);
-	if (!desc.eq_id)
-		return ret;
 
 	mc = desc.mc;
 	if (!mc)
@@ -341,7 +335,7 @@ int __init save_microcode_in_initrd_amd(unsigned int cpuid_1_eax)
 	desc.cpuid_1_eax = cpuid_1_eax;
 
 	scan_containers(cp.data, cp.size, &desc);
-	if (!desc.eq_id)
+	if (!desc.mc)
 		return -EINVAL;
 
 	ret = load_microcode_amd(smp_processor_id(), x86_family(cpuid_1_eax),
Signature · 1 line
2.11.0
author maintainer
#21 lore raw

PATCH v2 11/16 x86/microcode: Remove local vendor variable

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

Use x86_cpuid_vendor() directly.

No functionality change.

---
 arch/x86/kernel/cpu/microcode/core.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
arch/x86/kernel/cpu/microcode/core.c +4 -6
index 437996c9be67..dc54518299c4 100644
@@ -122,15 +122,14 @@ bool get_builtin_firmware(struct cpio_data *cd, const char *name)
 
 void __init load_ucode_bsp(void)
 {
-	unsigned int vendor, cpuid_1_eax;
+	unsigned int cpuid_1_eax;
 
 	if (check_loader_disabled_bsp())
 		return;
 
-	vendor	    = x86_cpuid_vendor();
 	cpuid_1_eax = native_cpuid_eax(1);
 
-	switch (vendor) {
+	switch (x86_cpuid_vendor()) {
 	case X86_VENDOR_INTEL:
 		if (x86_family(cpuid_1_eax) >= 6)
 			load_ucode_intel_bsp();
@@ -155,15 +154,14 @@ static bool check_loader_disabled_ap(void)
 
 void load_ucode_ap(void)
 {
-	unsigned int vendor, cpuid_1_eax;
+	unsigned int cpuid_1_eax;
 
 	if (check_loader_disabled_ap())
 		return;
 
-	vendor	    = x86_cpuid_vendor();
 	cpuid_1_eax = native_cpuid_eax(1);
 
-	switch (vendor) {
+	switch (x86_cpuid_vendor()) {
 	case X86_VENDOR_INTEL:
 		if (x86_family(cpuid_1_eax) >= 6)
 			load_ucode_intel_ap();
Signature · 1 line
2.11.0
author maintainer
#11 lore raw

PATCH v2 6/16 x86/microcode/AMD: Extend the container struct

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

Make it into a container descriptor which is being passed around and stores important info like the matching container and the patch for the current CPU. Make it static too.

Later patches will use this and thus get rid of a double container parsing.

---
 arch/x86/kernel/cpu/microcode/amd.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +15 -11
index 1691f41fcfdb..4e2238345308 100644
@@ -42,11 +42,15 @@ static struct equiv_cpu_entry *equiv_cpu_table;
 
 /*
  * This points to the current valid container of microcode patches which we will
- * save from the initrd/builtin before jettisoning its contents.
+ * save from the initrd/builtin before jettisoning its contents. @mc is the
+ * microcode patch we found to match.
  */
-struct container {
-	u8 *data;
-	size_t size;
+static struct cont_desc {
+	struct microcode_amd *mc;
+	u32		     psize;
+	u16		     eq_id;
+	u8		     *data;
+	size_t		     size;
 } cont;
 
 static u32 ucode_new_rev;
@@ -113,9 +117,9 @@ static u16 find_equiv_id(struct equiv_cpu_entry *equiv_table, u32 sig)
  * table or 0 if none found.
  */
 static u16
-find_proper_container(u8 *ucode, size_t size, struct container *ret_cont)
+find_proper_container(u8 *ucode, size_t size, struct cont_desc *desc)
 {
-	struct container ret = { NULL, 0 };
+	struct cont_desc ret = { 0 };
 	u32 eax, ebx, ecx, edx;
 	struct equiv_cpu_entry *eq;
 	int offset, left;
@@ -158,7 +162,7 @@ find_proper_container(u8 *ucode, size_t size, struct container *ret_cont)
 			 */
 			left = ret.size - offset;
 
-			*ret_cont = ret;
+			*desc = ret;
 			return eq_id;
 		}
 
@@ -213,11 +217,11 @@ static int __apply_microcode_amd(struct microcode_amd *mc)
  * Returns true if container found (sets @ret_cont), false otherwise.
  */
 static bool apply_microcode_early_amd(void *ucode, size_t size, bool save_patch,
-				      struct container *ret_cont)
+				      struct cont_desc *desc)
 {
 	u8 (*patch)[PATCH_MAX_SIZE];
 	u32 rev, *header, *new_rev;
-	struct container ret;
+	struct cont_desc ret;
 	int offset, left;
 	u16 eq_id = 0;
 	u8  *data;
@@ -270,8 +274,8 @@ static bool apply_microcode_early_amd(void *ucode, size_t size, bool save_patch,
 		left   -= offset;
 	}
 
-	if (ret_cont)
-		*ret_cont = ret;
+	if (desc)
+		*desc = ret;
 
 	return true;
 }
Signature · 1 line
2.11.0
author maintainer
#19 lore raw

PATCH v2 10/16 x86/microcode/AMD: Use find_microcode_in_initrd()

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

Use the generic helper instead of semi-open-coding the procedure.

---
 arch/x86/kernel/cpu/microcode/amd.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +5 -11
index 4cbfe70cf458..7727f278de58 100644
@@ -337,17 +337,15 @@ void load_ucode_amd_ap(unsigned int cpuid_1_eax)
 
 	/* First AP hasn't cached it yet, go through the blob. */
 	if (!cont.data) {
-		struct cpio_data cp = { NULL, 0, "" };
+		struct cpio_data cp;
 
 		if (cont.size == -1)
 			return;
 
 reget:
 		if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax))) {
-#ifdef CONFIG_BLK_DEV_INITRD
-			cp = find_cpio_data(ucode_path, (void *)initrd_start,
-					    initrd_end - initrd_start, NULL);
-#endif
+			cp = find_microcode_in_initrd(ucode_path, false);
+
 			if (!(cp.data && cp.size)) {
 				/*
 				 * Mark it so that other APs do not scan again
@@ -401,13 +399,9 @@ int __init save_microcode_in_initrd_amd(unsigned int cpuid_1_eax)
 
 	if (!cont.data) {
 		if (IS_ENABLED(CONFIG_X86_32) && (cont.size != -1)) {
-			struct cpio_data cp = { NULL, 0, "" };
-
-#ifdef CONFIG_BLK_DEV_INITRD
-			cp = find_cpio_data(ucode_path, (void *)initrd_start,
-					    initrd_end - initrd_start, NULL);
-#endif
+			struct cpio_data cp;
 
+			cp = find_microcode_in_initrd(ucode_path, false);
 			if (!(cp.data && cp.size)) {
 				cont.size = -1;
 				return -EINVAL;
Signature · 1 line
2.11.0
author maintainer
#9 lore raw

PATCH v2 5/16 x86/microcode/AMD: Shorten function parameter's name

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

The whole driver calls this "mc", do that here too.

No functionality change.

---
 arch/x86/kernel/cpu/microcode/amd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +3 -3
index 5c1509a38048..1691f41fcfdb 100644
@@ -187,15 +187,15 @@ find_proper_container(u8 *ucode, size_t size, struct container *ret_cont)
 	return eq_id;
 }
 
-static int __apply_microcode_amd(struct microcode_amd *mc_amd)
+static int __apply_microcode_amd(struct microcode_amd *mc)
 {
 	u32 rev, dummy;
 
-	native_wrmsrl(MSR_AMD64_PATCH_LOADER, (u64)(long)&mc_amd->hdr.data_code);
+	native_wrmsrl(MSR_AMD64_PATCH_LOADER, (u64)(long)&mc->hdr.data_code);
 
 	/* verify patch application was successful */
 	native_rdmsr(MSR_AMD64_PATCH_LEVEL, rev, dummy);
-	if (rev != mc_amd->hdr.patch_id)
+	if (rev != mc->hdr.patch_id)
 		return -1;
 
 	return 0;
Signature · 1 line
2.11.0

Borislav Petkov

author maintainer
#0 lore raw

v2 cover x86/microcode: 4.11 queue

From: Borislav Petkov <[email protected]>

Hi,

here's v2 incorporating all feedback from tglx. As a result, the final version has gotten even cleaner. To the point that I might even start sharing code between the Intel and AMD side. We'll see how ugly it gets.

Btw, 1/16 needs to go to tip/x86/urgent as it is another fix for the loading rework that went in in 4.10.

Please queue the rest for 4.11.

Thanks.

Changelog:

v1:

so this is more of a lessons-learned pile after the rewriting of the whole patch loading method and not caching addresses. It largely simplifies the loader - just look at the diffstat - without any functionality loss (I hope :-)). The driver is also very much readable now with unified, common paths where possible.

What is more, 6/13 reworks the whole AMD container parsing into much more readable separation of functionality and functions doing only one thing properly.

13/13 is also another nice simplification for the AP update path which looks almost straightforward in comparison with what we had before.

All has been tested on a lot of boxes and in different configurations:

* builtin vs initrd microcode
* old microcode_ctl application method for Intel
* "echo 1 > /sys/devices/system/cpu/microcode/reload" late method
* CONFIG_RANDOMIZE_MEMORY

both on Intel and AMD machines.

Borislav Petkov (16):
x86/microcode/intel: Drop stashed AP patch pointer optimization x86/MSR: Carve out bare minimum accessors x86/microcode: Convert to bare minimum MSR accessors x86/microcode/AMD: Clean up find_equiv_id() x86/microcode/AMD: Shorten function parameter's name x86/microcode/AMD: Extend the container struct x86/microcode/AMD: Rework container parsing x86/microcode: Decrease CPUID use
x86/microcode/AMD: Get rid of global this_equiv_id x86/microcode/AMD: Use find_microcode_in_initrd() x86/microcode: Remove local vendor variable x86/microcode/AMD: Check patch level only on the BSP x86/microcode/AMD: Unify load_ucode_amd_ap() x86/microcode/AMD: Simplify saving from initrd x86/microcode/AMD: Remove AP scanning optimization x86/microcode/AMD: Remove struct cont_desc.eq_id

 arch/x86/include/asm/apic.h           |   2 +-
 arch/x86/include/asm/microcode.h      |   9 +-
 arch/x86/include/asm/microcode_amd.h  |   2 -
 arch/x86/include/asm/msr.h            |  51 ++--
 arch/x86/kernel/cpu/microcode/amd.c   | 500 +++++++++++-----------------------
 arch/x86/kernel/cpu/microcode/core.c  |  81 ++++--
 arch/x86/kernel/cpu/microcode/intel.c |   9 +-
 7 files changed, 250 insertions(+), 404 deletions(-)
Signature · 1 line
2.11.0
author maintainer
#7 lore raw

PATCH v2 4/16 x86/microcode/AMD: Clean up find_equiv_id()

From: Borislav Petkov <[email protected]>

No need to have it marked "inline" - let gcc decide. Also, shorten the argument name and simplify while-test.

While at it, make it into a proper for-loop and simplify it even more, as tglx suggests.

No functionality change.

---
 arch/x86/kernel/cpu/microcode/amd.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +5 -12
index 6a31e2691f3a..5c1509a38048 100644
@@ -97,20 +97,13 @@ static size_t compute_container_size(u8 *data, u32 total_size)
 	return size;
 }
 
-static inline u16 find_equiv_id(struct equiv_cpu_entry *equiv_cpu_table,
-				unsigned int sig)
+static u16 find_equiv_id(struct equiv_cpu_entry *equiv_table, u32 sig)
 {
-	int i = 0;
-
-	if (!equiv_cpu_table)
-		return 0;
-
-	while (equiv_cpu_table[i].installed_cpu != 0) {
-		if (sig == equiv_cpu_table[i].installed_cpu)
-			return equiv_cpu_table[i].equiv_cpu;
-
-		i++;
+	for (; equiv_table && equiv_table->installed_cpu; equiv_table++) {
+		if (sig == equiv_table->installed_cpu)
+			return equiv_table->equiv_cpu;
 	}
+
 	return 0;
 }
 
Signature · 1 line
2.11.0
author maintainer
#3 lore raw

PATCH v2 2/16 x86/MSR: Carve out bare minimum accessors

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

Add __rdmsr() and __wrmsr() which *only* read and write an MSR with exception handling. Those are going to be used in early code, like the microcode loader, which cannot stomach tracing code piggybacking on the MSR operation.

While at it, get rid of __native_write_msr_notrace().

---
 arch/x86/include/asm/apic.h |  2 +-
 arch/x86/include/asm/msr.h  | 51 +++++++++++++++++++++++++++------------------
 2 files changed, 32 insertions(+), 21 deletions(-)
arch/x86/include/asm/apic.h +1 -1
index 0c5fbc68e82d..eff8e36aaf72 100644
@@ -195,7 +195,7 @@ static inline void native_apic_msr_write(u32 reg, u32 v)
 
 static inline void native_apic_msr_eoi_write(u32 reg, u32 v)
 {
-	wrmsr_notrace(APIC_BASE_MSR + (APIC_EOI >> 4), APIC_EOI_ACK, 0);
+	__wrmsr(APIC_BASE_MSR + (APIC_EOI >> 4), APIC_EOI_ACK, 0);
 }
 
 static inline u32 native_apic_msr_read(u32 reg)
arch/x86/include/asm/msr.h +31 -20

78 lines not on this page · raw message

Signature · 1 line
2.11.0
author maintainer
#5 lore raw

PATCH v2 3/16 x86/microcode: Convert to bare minimum MSR accessors

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

Having tracepoints to the MSR accessors makes them unsuitable for early microcode loading: think 32-bit before paging is enabled and us chasing pointers to test whether a tracepoint is enabled or not. Results in a reliable triple fault.

Convert to the bare ones.

---
 arch/x86/include/asm/microcode.h | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
arch/x86/include/asm/microcode.h +4 -5
index 38711df3bcb5..90b22bbdfce9 100644
@@ -7,18 +7,17 @@
 
 #define native_rdmsr(msr, val1, val2)			\
 do {							\
-	u64 __val = native_read_msr((msr));		\
+	u64 __val = __rdmsr((msr));			\
 	(void)((val1) = (u32)__val);			\
 	(void)((val2) = (u32)(__val >> 32));		\
 } while (0)
 
 #define native_wrmsr(msr, low, high)			\
-	native_write_msr(msr, low, high)
+	__wrmsr(msr, low, high)
 
 #define native_wrmsrl(msr, val)				\
-	native_write_msr((msr),				\
-			 (u32)((u64)(val)),		\
-			 (u32)((u64)(val) >> 32))
+	__wrmsr((msr), (u32)((u64)(val)),		\
+		       (u32)((u64)(val) >> 32))
 
 struct ucode_patch {
 	struct list_head plist;
Signature · 1 line
2.11.0
#2 lore raw

[tip:x86/mce] x86/microcode/intel: Drop stashed AP patch pointer optimization

In reply to Borislav Petkov
x86/microcode/intel: Drop stashed AP patch pointer optimization
Commit-ID:  c26665ab5c49ad3e142e0f054ca3204f259ba09c
Gitweb:     http://git.kernel.org/tip/c26665ab5c49ad3e142e0f054ca3204f259ba09c
Author:     Borislav Petkov <[email protected]>
AuthorDate: Fri, 20 Jan 2017 21:29:40 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 23 Jan 2017 09:39:55 +0100

x86/microcode/intel: Drop stashed AP patch pointer optimization

This was meant to save us the scanning of the microcode containter in the initrd since the first AP had already done that but it can also hurt us:

Imagine a single hyperthreaded CPU (Intel(R) Atom(TM) CPU N270, for example) which updates the microcode on the BSP but since the microcode engine is shared between the two threads, the update on CPU1 doesn't happen because it has already happened on CPU0 and we don't find a newer microcode revision on CPU1.

Which doesn't set the intel_ucode_patch pointer and at initrd jettisoning time we don't save the microcode patch for later application.

Now, when we suspend to RAM, the loaded microcode gets cleared so we need to reload but there's no patch saved in the cache.

Removing the optimization fixes this issue and all is fine and dandy.

Fixes 06b8534cb728 ("x86/microcode: Rework microcode loading")
---
 arch/x86/kernel/cpu/microcode/intel.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)
arch/x86/kernel/cpu/microcode/intel.c +1 -8
index 3f329b7..8325d8a 100644
@@ -41,7 +41,7 @@
 
 static const char ucode_path[] = "kernel/x86/microcode/GenuineIntel.bin";
 
-/* Current microcode patch used in early patching */
+/* Current microcode patch used in early patching on the APs. */
 struct microcode_intel *intel_ucode_patch;
 
 static inline bool cpu_signatures_match(unsigned int s1, unsigned int p1,
@@ -607,12 +607,6 @@ int __init save_microcode_in_initrd_intel(void)
 	struct ucode_cpu_info uci;
 	struct cpio_data cp;
 
-	/*
-	 * AP loading didn't find any microcode patch, no need to save anything.
-	 */
-	if (!intel_ucode_patch || IS_ERR(intel_ucode_patch))
-		return 0;
-
 	if (!load_builtin_intel_microcode(&cp))
 		cp = find_microcode_in_initrd(ucode_path, false);
 
@@ -628,7 +622,6 @@ int __init save_microcode_in_initrd_intel(void)
 	return 0;
 }
 
-
 /*
  * @res_patch, output: a pointer to the patch we found.
  */
#4 lore raw

[tip:x86/mce] x86/MSR: Carve out bare minimum accessors

In reply to Borislav Petkov
x86/MSR: Carve out bare minimum accessors
Commit-ID:  a585df8edabdb47ae25214ebb3a627ca7ce800d3
Gitweb:     http://git.kernel.org/tip/a585df8edabdb47ae25214ebb3a627ca7ce800d3
Author:     Borislav Petkov <[email protected]>
AuthorDate: Fri, 20 Jan 2017 21:29:41 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 23 Jan 2017 10:02:45 +0100

x86/MSR: Carve out bare minimum accessors

Add __rdmsr() and __wrmsr() which *only* read and write an MSR with exception handling. Those are going to be used in early code, like the microcode loader, which cannot stomach tracing code piggybacking on the MSR operation.

While at it, get rid of __native_write_msr_notrace().

---
 arch/x86/include/asm/apic.h |  2 +-
 arch/x86/include/asm/msr.h  | 51 +++++++++++++++++++++++++++------------------
 2 files changed, 32 insertions(+), 21 deletions(-)
arch/x86/include/asm/apic.h +1 -1
index 0c5fbc6..eff8e36 100644
@@ -195,7 +195,7 @@ static inline void native_apic_msr_write(u32 reg, u32 v)
 
 static inline void native_apic_msr_eoi_write(u32 reg, u32 v)
 {
-	wrmsr_notrace(APIC_BASE_MSR + (APIC_EOI >> 4), APIC_EOI_ACK, 0);
+	__wrmsr(APIC_BASE_MSR + (APIC_EOI >> 4), APIC_EOI_ACK, 0);
 }
 
 static inline u32 native_apic_msr_read(u32 reg)
arch/x86/include/asm/msr.h +31 -20

78 lines not on this page · raw message

#6 lore raw

[tip:x86/mce] x86/microcode: Convert to bare minimum MSR accessors

In reply to Borislav Petkov
x86/microcode: Convert to bare minimum MSR accessors
Commit-ID:  0c12d18ab96e4da9f3e963bc242689bdeaaf2330
Gitweb:     http://git.kernel.org/tip/0c12d18ab96e4da9f3e963bc242689bdeaaf2330
Author:     Borislav Petkov <[email protected]>
AuthorDate: Fri, 20 Jan 2017 21:29:42 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 23 Jan 2017 10:02:45 +0100

x86/microcode: Convert to bare minimum MSR accessors

Having tracepoints to the MSR accessors makes them unsuitable for early microcode loading: think 32-bit before paging is enabled and us chasing pointers to test whether a tracepoint is enabled or not. Results in a reliable triple fault.

Convert to the bare ones.

---
 arch/x86/include/asm/microcode.h | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
arch/x86/include/asm/microcode.h +4 -5
index 38711df..90b22bb 100644
@@ -7,18 +7,17 @@
 
 #define native_rdmsr(msr, val1, val2)			\
 do {							\
-	u64 __val = native_read_msr((msr));		\
+	u64 __val = __rdmsr((msr));			\
 	(void)((val1) = (u32)__val);			\
 	(void)((val2) = (u32)(__val >> 32));		\
 } while (0)
 
 #define native_wrmsr(msr, low, high)			\
-	native_write_msr(msr, low, high)
+	__wrmsr(msr, low, high)
 
 #define native_wrmsrl(msr, val)				\
-	native_write_msr((msr),				\
-			 (u32)((u64)(val)),		\
-			 (u32)((u64)(val) >> 32))
+	__wrmsr((msr), (u32)((u64)(val)),		\
+		       (u32)((u64)(val) >> 32))
 
 struct ucode_patch {
 	struct list_head plist;
#8 lore raw

[tip:x86/mce] x86/microcode/AMD: Clean up find_equiv_id()

In reply to Borislav Petkov
x86/microcode/AMD: Clean up find_equiv_id()
Commit-ID:  1f02ac0682055418753fe30e6433dcb11fd03e1c
Gitweb:     http://git.kernel.org/tip/1f02ac0682055418753fe30e6433dcb11fd03e1c
Author:     Borislav Petkov <[email protected]>
AuthorDate: Fri, 20 Jan 2017 21:29:43 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 23 Jan 2017 10:02:46 +0100

x86/microcode/AMD: Clean up find_equiv_id()

No need to have it marked "inline" - let gcc decide. Also, shorten the argument name and simplify while-test.

While at it, make it into a proper for-loop and simplify it even more, as tglx suggests.

No functionality change.

---
 arch/x86/kernel/cpu/microcode/amd.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +5 -12
index 6a31e26..5c1509a3 100644
@@ -97,20 +97,13 @@ static size_t compute_container_size(u8 *data, u32 total_size)
 	return size;
 }
 
-static inline u16 find_equiv_id(struct equiv_cpu_entry *equiv_cpu_table,
-				unsigned int sig)
+static u16 find_equiv_id(struct equiv_cpu_entry *equiv_table, u32 sig)
 {
-	int i = 0;
-
-	if (!equiv_cpu_table)
-		return 0;
-
-	while (equiv_cpu_table[i].installed_cpu != 0) {
-		if (sig == equiv_cpu_table[i].installed_cpu)
-			return equiv_cpu_table[i].equiv_cpu;
-
-		i++;
+	for (; equiv_table && equiv_table->installed_cpu; equiv_table++) {
+		if (sig == equiv_table->installed_cpu)
+			return equiv_table->equiv_cpu;
 	}
+
 	return 0;
 }
 
#10 lore raw

[tip:x86/mce] x86/microcode/AMD: Shorten function parameter's name

In reply to Borislav Petkov
x86/microcode/AMD: Shorten function parameter's name
Commit-ID:  ef901dc33dfa9abffe7d954628b5ec7ba7f62aec
Gitweb:     http://git.kernel.org/tip/ef901dc33dfa9abffe7d954628b5ec7ba7f62aec
Author:     Borislav Petkov <[email protected]>
AuthorDate: Fri, 20 Jan 2017 21:29:44 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 23 Jan 2017 10:02:46 +0100

x86/microcode/AMD: Shorten function parameter's name

The whole driver calls this "mc", do that here too.

No functionality change.

---
 arch/x86/kernel/cpu/microcode/amd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +3 -3
index 5c1509a3..1691f41 100644
@@ -187,15 +187,15 @@ find_proper_container(u8 *ucode, size_t size, struct container *ret_cont)
 	return eq_id;
 }
 
-static int __apply_microcode_amd(struct microcode_amd *mc_amd)
+static int __apply_microcode_amd(struct microcode_amd *mc)
 {
 	u32 rev, dummy;
 
-	native_wrmsrl(MSR_AMD64_PATCH_LOADER, (u64)(long)&mc_amd->hdr.data_code);
+	native_wrmsrl(MSR_AMD64_PATCH_LOADER, (u64)(long)&mc->hdr.data_code);
 
 	/* verify patch application was successful */
 	native_rdmsr(MSR_AMD64_PATCH_LEVEL, rev, dummy);
-	if (rev != mc_amd->hdr.patch_id)
+	if (rev != mc->hdr.patch_id)
 		return -1;
 
 	return 0;
#12 lore raw

[tip:x86/mce] x86/microcode/AMD: Extend the container struct

In reply to Borislav Petkov
x86/microcode/AMD: Extend the container struct
Commit-ID:  f454177f739e92620d84a1f42f91b03007a1cdd0
Gitweb:     http://git.kernel.org/tip/f454177f739e92620d84a1f42f91b03007a1cdd0
Author:     Borislav Petkov <[email protected]>
AuthorDate: Fri, 20 Jan 2017 21:29:45 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 23 Jan 2017 10:02:47 +0100

x86/microcode/AMD: Extend the container struct

Make it into a container descriptor which is being passed around and stores important info like the matching container and the patch for the current CPU. Make it static too.

Later patches will use this and thus get rid of a double container parsing.

---
 arch/x86/kernel/cpu/microcode/amd.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +15 -11
index 1691f41..4e22383 100644
@@ -42,11 +42,15 @@ static struct equiv_cpu_entry *equiv_cpu_table;
 
 /*
  * This points to the current valid container of microcode patches which we will
- * save from the initrd/builtin before jettisoning its contents.
+ * save from the initrd/builtin before jettisoning its contents. @mc is the
+ * microcode patch we found to match.
  */
-struct container {
-	u8 *data;
-	size_t size;
+static struct cont_desc {
+	struct microcode_amd *mc;
+	u32		     psize;
+	u16		     eq_id;
+	u8		     *data;
+	size_t		     size;
 } cont;
 
 static u32 ucode_new_rev;
@@ -113,9 +117,9 @@ static u16 find_equiv_id(struct equiv_cpu_entry *equiv_table, u32 sig)
  * table or 0 if none found.
  */
 static u16
-find_proper_container(u8 *ucode, size_t size, struct container *ret_cont)
+find_proper_container(u8 *ucode, size_t size, struct cont_desc *desc)
 {
-	struct container ret = { NULL, 0 };
+	struct cont_desc ret = { 0 };
 	u32 eax, ebx, ecx, edx;
 	struct equiv_cpu_entry *eq;
 	int offset, left;
@@ -158,7 +162,7 @@ find_proper_container(u8 *ucode, size_t size, struct container *ret_cont)
 			 */
 			left = ret.size - offset;
 
-			*ret_cont = ret;
+			*desc = ret;
 			return eq_id;
 		}
 
@@ -213,11 +217,11 @@ static int __apply_microcode_amd(struct microcode_amd *mc)
  * Returns true if container found (sets @ret_cont), false otherwise.
  */
 static bool apply_microcode_early_amd(void *ucode, size_t size, bool save_patch,
-				      struct container *ret_cont)
+				      struct cont_desc *desc)
 {
 	u8 (*patch)[PATCH_MAX_SIZE];
 	u32 rev, *header, *new_rev;
-	struct container ret;
+	struct cont_desc ret;
 	int offset, left;
 	u16 eq_id = 0;
 	u8  *data;
@@ -270,8 +274,8 @@ static bool apply_microcode_early_amd(void *ucode, size_t size, bool save_patch,
 		left   -= offset;
 	}
 
-	if (ret_cont)
-		*ret_cont = ret;
+	if (desc)
+		*desc = ret;
 
 	return true;
 }
#14 lore raw

[tip:x86/mce] x86/microcode/AMD: Rework container parsing

In reply to Borislav Petkov
x86/microcode/AMD: Rework container parsing
Commit-ID:  8801b3fcb5744dc536272ba4ccfd6faca9b46705
Gitweb:     http://git.kernel.org/tip/8801b3fcb5744dc536272ba4ccfd6faca9b46705
Author:     Borislav Petkov <[email protected]>
AuthorDate: Fri, 20 Jan 2017 21:29:46 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 23 Jan 2017 10:02:47 +0100

x86/microcode/AMD: Rework container parsing

It was pretty clumsy before and the whole work of parsing the microcode containers was spread around the functions wrongly.

Clean it up so that there's a main scan_containers() function which iterates over the microcode blob and picks apart the containers glued together. For each container, it calls a parse_container() helper which concentrates on one container only: sanity-checking, parsing, counting microcode patches in there, etc.

It makes much more sense now and it is actually very readable. Oh, and we luvz a diffstat removing more crap than adding.

---
 arch/x86/kernel/cpu/microcode/amd.c | 238 ++++++++++++++++--------------------
 1 file changed, 105 insertions(+), 133 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +105 -133
index 4e22383..9d5f4f3 100644
@@ -64,43 +64,6 @@ static u16 this_equiv_id;
 static const char
 ucode_path[] __maybe_unused = "kernel/x86/microcode/AuthenticAMD.bin";
 
-static size_t compute_container_size(u8 *data, u32 total_size)
-{
-	size_t size = 0;
-	u32 *header = (u32 *)data;
-
-	if (header[0] != UCODE_MAGIC ||
-	    header[1] != UCODE_EQUIV_CPU_TABLE_TYPE || /* type */
-	    header[2] == 0)                            /* size */
-		return size;
-
-	size = header[2] + CONTAINER_HDR_SZ;
-	total_size -= size;
-	data += size;
-
-	while (total_size) {
-		u16 patch_size;
-
-		header = (u32 *)data;
-
-		if (header[0] != UCODE_UCODE_TYPE)
-			break;
-
-		/*
-		 * Sanity-check patch size.
-		 */
-		patch_size = header[1];
-		if (patch_size > PATCH_MAX_SIZE)
-			break;
-
-		size	   += patch_size + SECTION_HDR_SIZE;
-		data	   += patch_size + SECTION_HDR_SIZE;
-		total_size -= patch_size + SECTION_HDR_SIZE;
-	}
-
-	return size;
-}
-
 static u16 find_equiv_id(struct equiv_cpu_entry *equiv_table, u32 sig)
 {
 	for (; equiv_table && equiv_table->installed_cpu; equiv_table++) {
@@ -115,80 +78,106 @@ static u16 find_equiv_id(struct equiv_cpu_entry *equiv_table, u32 sig)
  * This scans the ucode blob for the proper container as we can have multiple
  * containers glued together. Returns the equivalence ID from the equivalence
  * table or 0 if none found.
+ * Returns the amount of bytes consumed while scanning. @desc contains all the
+ * data we're going to use in later stages of the application.
  */
-static u16
-find_proper_container(u8 *ucode, size_t size, struct cont_desc *desc)
+static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
 {
-	struct cont_desc ret = { 0 };
-	u32 eax, ebx, ecx, edx;
 	struct equiv_cpu_entry *eq;
-	int offset, left;
-	u16 eq_id = 0;
-	u32 *header;
-	u8 *data;
+	ssize_t orig_size = size;
+	u32 *hdr = (u32 *)ucode;
+	u32 eax, ebx, ecx, edx;
+	u16 eq_id;
+	u8 *buf;
 
-	data   = ucode;
-	left   = size;
-	header = (u32 *)data;
+	/* Am I looking at an equivalence table header? */
+	if (hdr[0] != UCODE_MAGIC ||
+	    hdr[1] != UCODE_EQUIV_CPU_TABLE_TYPE ||
+	    hdr[2] == 0) {
+		desc->eq_id = 0;
+		return CONTAINER_HDR_SZ;
+	}
 
+	buf = ucode;
 
-	/* find equiv cpu table */
-	if (header[0] != UCODE_MAGIC ||
-	    header[1] != UCODE_EQUIV_CPU_TABLE_TYPE || /* type */
-	    header[2] == 0)                            /* size */
-		return eq_id;
+	eq = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
 
-	eax = 0x00000001;
+	eax = 1;
 	ecx = 0;
 	native_cpuid(&eax, &ebx, &ecx, &edx);
 
-	while (left > 0) {
-		eq = (struct equiv_cpu_entry *)(data + CONTAINER_HDR_SZ);
-
-		ret.data = data;
+	/* Find the equivalence ID of our CPU in this table: */
+	eq_id = find_equiv_id(eq, eax);
 
-		/* Advance past the container header */
-		offset = header[2] + CONTAINER_HDR_SZ;
-		data  += offset;
-		left  -= offset;
+	buf  += hdr[2] + CONTAINER_HDR_SZ;
+	size -= hdr[2] + CONTAINER_HDR_SZ;
 
-		eq_id = find_equiv_id(eq, eax);
-		if (eq_id) {
-			ret.size = compute_container_size(ret.data, left + offset);
+	/*
+	 * Scan through the rest of the container to find where it ends. We do
+	 * some basic sanity-checking too.
+	 */
+	while (size > 0) {
+		struct microcode_amd *mc;
+		u32 patch_size;
 
-			/*
-			 * truncate how much we need to iterate over in the
-			 * ucode update loop below
-			 */
-			left = ret.size - offset;
+		hdr = (u32 *)buf;
 
-			*desc = ret;
-			return eq_id;
-		}
+		if (hdr[0] != UCODE_UCODE_TYPE)
+			break;
 
-		/*
-		 * support multiple container files appended together. if this
-		 * one does not have a matching equivalent cpu entry, we fast
-		 * forward to the next container file.
-		 */
-		while (left > 0) {
-			header = (u32 *)data;
+		/* Sanity-check patch size. */
+		patch_size = hdr[1];
+		if (patch_size > PATCH_MAX_SIZE)
+			break;
 
-			if (header[0] == UCODE_MAGIC &&
-			    header[1] == UCODE_EQUIV_CPU_TABLE_TYPE)
-				break;
+		/* Skip patch section header: */
+		buf  += SECTION_HDR_SIZE;
+		size -= SECTION_HDR_SIZE;
 
-			offset = header[1] + SECTION_HDR_SIZE;
-			data  += offset;
-			left  -= offset;
+		mc = (struct microcode_amd *)buf;
+		if (eq_id == mc->hdr.processor_rev_id) {
+			desc->psize = patch_size;
+			desc->mc = mc;
 		}
 
-		/* mark where the next microcode container file starts */
-		offset    = data - (u8 *)ucode;
-		ucode     = data;
+		buf  += patch_size;
+		size -= patch_size;
 	}
 
-	return eq_id;
+	/*
+	 * If we have found a patch (desc->mc), it means we're looking at the
+	 * container which has a patch for this CPU so return 0 to mean, @ucode
+	 * already points to the proper container. Otherwise, we return the size
+	 * we scanned so that we can advance to the next container in the
+	 * buffer.
+	 */
+	if (desc->mc) {
+		desc->eq_id = eq_id;
+		desc->data  = ucode;
+		desc->size  = orig_size - size;
+
+		return 0;
+	}
+
+	return orig_size - size;
+}
+
+/*
+ * Scan the ucode blob for the proper container as we can have multiple
+ * containers glued together.
+ */
+static void scan_containers(u8 *ucode, size_t size, struct cont_desc *desc)
+{
+	ssize_t rem = size;
+
+	while (rem >= 0) {
+		ssize_t s = parse_container(ucode, rem, desc);
+		if (!s)
+			return;
+
+		ucode += s;
+		rem   -= s;
+	}
 }
 
 static int __apply_microcode_amd(struct microcode_amd *mc)
@@ -214,17 +203,16 @@ static int __apply_microcode_amd(struct microcode_amd *mc)
  * load_microcode_amd() to save equivalent cpu table and microcode patches in
  * kernel heap memory.
  *
- * Returns true if container found (sets @ret_cont), false otherwise.
+ * Returns true if container found (sets @desc), false otherwise.
  */
 static bool apply_microcode_early_amd(void *ucode, size_t size, bool save_patch,
-				      struct cont_desc *desc)
+				      struct cont_desc *ret_desc)
 {
+	struct cont_desc desc = { 0 };
 	u8 (*patch)[PATCH_MAX_SIZE];
-	u32 rev, *header, *new_rev;
-	struct cont_desc ret;
-	int offset, left;
-	u16 eq_id = 0;
-	u8  *data;
+	struct microcode_amd *mc;
+	u32 rev, *new_rev;
+	bool ret = false;
 
 #ifdef CONFIG_X86_32
 	new_rev = (u32 *)__pa_nodebug(&ucode_new_rev);
@@ -237,47 +225,31 @@ static bool apply_microcode_early_amd(void *ucode, size_t size, bool save_patch,
 	if (check_current_patch_level(&rev, true))
 		return false;
 
-	eq_id = find_proper_container(ucode, size, &ret);
-	if (!eq_id)
-		return false;
-
-	this_equiv_id = eq_id;
-	header = (u32 *)ret.data;
-
-	/* We're pointing to an equiv table, skip over it. */
-	data = ret.data +  header[2] + CONTAINER_HDR_SZ;
-	left = ret.size - (header[2] + CONTAINER_HDR_SZ);
-
-	while (left > 0) {
-		struct microcode_amd *mc;
-
-		header = (u32 *)data;
-		if (header[0] != UCODE_UCODE_TYPE || /* type */
-		    header[1] == 0)                  /* size */
-			break;
+	scan_containers(ucode, size, &desc);
+	if (!desc.eq_id)
+		return ret;
 
-		mc = (struct microcode_amd *)(data + SECTION_HDR_SIZE);
+	this_equiv_id = desc.eq_id;
 
-		if (eq_id == mc->hdr.processor_rev_id && rev < mc->hdr.patch_id) {
+	mc = desc.mc;
+	if (!mc)
+		return ret;
 
-			if (!__apply_microcode_amd(mc)) {
-				rev = mc->hdr.patch_id;
-				*new_rev = rev;
+	if (rev >= mc->hdr.patch_id)
+		return ret;
 
-				if (save_patch)
-					memcpy(patch, mc, min_t(u32, header[1], PATCH_MAX_SIZE));
-			}
-		}
+	if (!__apply_microcode_amd(mc)) {
+		*new_rev = mc->hdr.patch_id;
+		ret      = true;
 
-		offset  = header[1] + SECTION_HDR_SIZE;
-		data   += offset;
-		left   -= offset;
+		if (save_patch)
+			memcpy(patch, mc, min_t(u32, desc.psize, PATCH_MAX_SIZE));
 	}
 
-	if (desc)
-		*desc = ret;
+	if (ret_desc)
+		*ret_desc = desc;
 
-	return true;
+	return ret;
 }
 
 static bool get_builtin_microcode(struct cpio_data *cp, unsigned int family)
@@ -396,6 +368,7 @@ reget:
 		}
 
 		if (!apply_microcode_early_amd(cp.data, cp.size, false, &cont)) {
+			cont.data = NULL;
 			cont.size = -1;
 			return;
 		}
@@ -434,7 +407,6 @@ int __init save_microcode_in_initrd_amd(unsigned int fam)
 {
 	enum ucode_state ret;
 	int retval = 0;
-	u16 eq_id;
 
 	if (!cont.data) {
 		if (IS_ENABLED(CONFIG_X86_32) && (cont.size != -1)) {
@@ -450,8 +422,8 @@ int __init save_microcode_in_initrd_amd(unsigned int fam)
 				return -EINVAL;
 			}
 
-			eq_id = find_proper_container(cp.data, cp.size, &cont);
-			if (!eq_id) {
+			scan_containers(cp.data, cp.size, &cont);
+			if (!cont.eq_id) {
 				cont.size = -1;
 				return -EINVAL;
 			}
#16 lore raw

[tip:x86/mce] x86/microcode: Decrease CPUID use

In reply to Borislav Petkov
x86/microcode: Decrease CPUID use
Commit-ID:  309aac77768c0c689eac4900ca8a6e9ef470035c
Gitweb:     http://git.kernel.org/tip/309aac77768c0c689eac4900ca8a6e9ef470035c
Author:     Borislav Petkov <[email protected]>
AuthorDate: Fri, 20 Jan 2017 21:29:47 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 23 Jan 2017 10:02:47 +0100

x86/microcode: Decrease CPUID use

Get CPUID(1).EAX value once per CPU and propagate value into the callers instead of conveniently calling it every time.

---
 arch/x86/kernel/cpu/microcode/amd.c  | 52 +++++++++++++++++-------------------
 arch/x86/kernel/cpu/microcode/core.c | 38 ++++++++++----------------
 2 files changed, 38 insertions(+), 52 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +24 -28
index 9d5f4f3..9fb398e 100644
@@ -47,6 +47,7 @@ static struct equiv_cpu_entry *equiv_cpu_table;
  */
 static struct cont_desc {
 	struct microcode_amd *mc;
+	u32		     cpuid_1_eax;
 	u32		     psize;
 	u16		     eq_id;
 	u8		     *data;
@@ -86,7 +87,6 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
 	struct equiv_cpu_entry *eq;
 	ssize_t orig_size = size;
 	u32 *hdr = (u32 *)ucode;
-	u32 eax, ebx, ecx, edx;
 	u16 eq_id;
 	u8 *buf;
 
@@ -102,12 +102,8 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
 
 	eq = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
 
-	eax = 1;
-	ecx = 0;
-	native_cpuid(&eax, &ebx, &ecx, &edx);
-
 	/* Find the equivalence ID of our CPU in this table: */
-	eq_id = find_equiv_id(eq, eax);
+	eq_id = find_equiv_id(eq, desc->cpuid_1_eax);
 
 	buf  += hdr[2] + CONTAINER_HDR_SZ;
 	size -= hdr[2] + CONTAINER_HDR_SZ;
@@ -205,8 +201,9 @@ static int __apply_microcode_amd(struct microcode_amd *mc)
  *
  * Returns true if container found (sets @desc), false otherwise.
  */
-static bool apply_microcode_early_amd(void *ucode, size_t size, bool save_patch,
-				      struct cont_desc *ret_desc)
+static bool
+apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size,
+			  bool save_patch, struct cont_desc *ret_desc)
 {
 	struct cont_desc desc = { 0 };
 	u8 (*patch)[PATCH_MAX_SIZE];
@@ -225,6 +222,8 @@ static bool apply_microcode_early_amd(void *ucode, size_t size, bool save_patch,
 	if (check_current_patch_level(&rev, true))
 		return false;
 
+	desc.cpuid_1_eax = cpuid_1_eax;
+
 	scan_containers(ucode, size, &desc);
 	if (!desc.eq_id)
 		return ret;
@@ -267,10 +266,9 @@ static bool get_builtin_microcode(struct cpio_data *cp, unsigned int family)
 #endif
 }
 
-void __init load_ucode_amd_bsp(unsigned int family)
+void __init load_ucode_amd_bsp(unsigned int cpuid_1_eax)
 {
 	struct ucode_cpu_info *uci;
-	u32 eax, ebx, ecx, edx;
 	struct cpio_data cp;
 	const char *path;
 	bool use_pa;
@@ -285,19 +283,16 @@ void __init load_ucode_amd_bsp(unsigned int family)
 		use_pa	= false;
 	}
 
-	if (!get_builtin_microcode(&cp, family))
+	if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)))
 		cp = find_microcode_in_initrd(path, use_pa);
 
 	if (!(cp.data && cp.size))
 		return;
 
-	/* Get BSP's CPUID.EAX(1), needed in load_microcode_amd() */
-	eax = 1;
-	ecx = 0;
-	native_cpuid(&eax, &ebx, &ecx, &edx);
-	uci->cpu_sig.sig = eax;
+	/* Needed in load_microcode_amd() */
+	uci->cpu_sig.sig = cpuid_1_eax;
 
-	apply_microcode_early_amd(cp.data, cp.size, true, NULL);
+	apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, true, NULL);
 }
 
 #ifdef CONFIG_X86_32
@@ -308,7 +303,7 @@ void __init load_ucode_amd_bsp(unsigned int family)
  * In save_microcode_in_initrd_amd() BSP's patch is copied to amd_ucode_patch,
  * which is used upon resume from suspend.
  */
-void load_ucode_amd_ap(unsigned int family)
+void load_ucode_amd_ap(unsigned int cpuid_1_eax)
 {
 	struct microcode_amd *mc;
 	struct cpio_data cp;
@@ -319,7 +314,7 @@ void load_ucode_amd_ap(unsigned int family)
 		return;
 	}
 
-	if (!get_builtin_microcode(&cp, family))
+	if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)))
 		cp = find_microcode_in_initrd((const char *)__pa_nodebug(ucode_path), true);
 
 	if (!(cp.data && cp.size))
@@ -329,14 +324,14 @@ void load_ucode_amd_ap(unsigned int family)
 	 * This would set amd_ucode_patch above so that the following APs can
 	 * use it directly instead of going down this path again.
 	 */
-	apply_microcode_early_amd(cp.data, cp.size, true, NULL);
+	apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, true, NULL);
 }
 #else
-void load_ucode_amd_ap(unsigned int family)
+void load_ucode_amd_ap(unsigned int cpuid_1_eax)
 {
 	struct equiv_cpu_entry *eq;
 	struct microcode_amd *mc;
-	u32 rev, eax;
+	u32 rev;
 	u16 eq_id;
 
 	/* 64-bit runs with paging enabled, thus early==false. */
@@ -351,7 +346,7 @@ void load_ucode_amd_ap(unsigned int family)
 			return;
 
 reget:
-		if (!get_builtin_microcode(&cp, family)) {
+		if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax))) {
 #ifdef CONFIG_BLK_DEV_INITRD
 			cp = find_cpio_data(ucode_path, (void *)initrd_start,
 					    initrd_end - initrd_start, NULL);
@@ -367,17 +362,16 @@ reget:
 			}
 		}
 
-		if (!apply_microcode_early_amd(cp.data, cp.size, false, &cont)) {
+		if (!apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, false, &cont)) {
 			cont.data = NULL;
 			cont.size = -1;
 			return;
 		}
 	}
 
-	eax = cpuid_eax(0x00000001);
 	eq  = (struct equiv_cpu_entry *)(cont.data + CONTAINER_HDR_SZ);
 
-	eq_id = find_equiv_id(eq, eax);
+	eq_id = find_equiv_id(eq, cpuid_1_eax);
 	if (!eq_id)
 		return;
 
@@ -403,7 +397,7 @@ reget:
 static enum ucode_state
 load_microcode_amd(int cpu, u8 family, const u8 *data, size_t size);
 
-int __init save_microcode_in_initrd_amd(unsigned int fam)
+int __init save_microcode_in_initrd_amd(unsigned int cpuid_1_eax)
 {
 	enum ucode_state ret;
 	int retval = 0;
@@ -422,6 +416,8 @@ int __init save_microcode_in_initrd_amd(unsigned int fam)
 				return -EINVAL;
 			}
 
+			cont.cpuid_1_eax = cpuid_1_eax;
+
 			scan_containers(cp.data, cp.size, &cont);
 			if (!cont.eq_id) {
 				cont.size = -1;
@@ -432,7 +428,7 @@ int __init save_microcode_in_initrd_amd(unsigned int fam)
 			return -EINVAL;
 	}
 
-	ret = load_microcode_amd(smp_processor_id(), fam, cont.data, cont.size);
+	ret = load_microcode_amd(smp_processor_id(), x86_family(cpuid_1_eax), cont.data, cont.size);
 	if (ret != UCODE_OK)
 		retval = -EINVAL;
 
arch/x86/kernel/cpu/microcode/core.c +14 -24
index 2af69d2..437996c 100644
@@ -64,10 +64,6 @@ static DEFINE_MUTEX(microcode_mutex);
 
 struct ucode_cpu_info		ucode_cpu_info[NR_CPUS];
 
-/*
- * Operations that are run on a target cpu:
- */
-
 struct cpu_info_ctx {
 	struct cpu_signature	*cpu_sig;
 	int			err;
@@ -76,7 +72,6 @@ struct cpu_info_ctx {
 static bool __init check_loader_disabled_bsp(void)
 {
 	static const char *__dis_opt_str = "dis_ucode_ldr";
-	u32 a, b, c, d;
 
 #ifdef CONFIG_X86_32
 	const char *cmdline = (const char *)__pa_nodebug(boot_command_line);
@@ -92,16 +87,12 @@ static bool __init check_loader_disabled_bsp(void)
 	if (!have_cpuid_p())
 		return *res;
 
-	a = 1;
-	c = 0;
-	native_cpuid(&a, &b, &c, &d);
-
 	/*
 	 * CPUID(1).ECX[31]: reserved for hypervisor use. This is still not
 	 * completely accurate as xen pv guests don't see that CPUID bit set but
 	 * that's good enough as they don't land on the BSP path anyway.
 	 */
-	if (c & BIT(31))
+	if (native_cpuid_ecx(1) & BIT(31))
 		return *res;
 
 	if (cmdline_find_option_bool(cmdline, option) <= 0)
@@ -131,23 +122,22 @@ bool get_builtin_firmware(struct cpio_data *cd, const char *name)
 
 void __init load_ucode_bsp(void)
 {
-	int vendor;
-	unsigned int family;
+	unsigned int vendor, cpuid_1_eax;
 
 	if (check_loader_disabled_bsp())
 		return;
 
-	vendor = x86_cpuid_vendor();
-	family = x86_cpuid_family();
+	vendor	    = x86_cpuid_vendor();
+	cpuid_1_eax = native_cpuid_eax(1);
 
 	switch (vendor) {
 	case X86_VENDOR_INTEL:
-		if (family >= 6)
+		if (x86_family(cpuid_1_eax) >= 6)
 			load_ucode_intel_bsp();
 		break;
 	case X86_VENDOR_AMD:
-		if (family >= 0x10)
-			load_ucode_amd_bsp(family);
+		if (x86_family(cpuid_1_eax) >= 0x10)
+			load_ucode_amd_bsp(cpuid_1_eax);
 		break;
 	default:
 		break;
@@ -165,22 +155,22 @@ static bool check_loader_disabled_ap(void)
 
 void load_ucode_ap(void)
 {
-	int vendor, family;
+	unsigned int vendor, cpuid_1_eax;
 
 	if (check_loader_disabled_ap())
 		return;
 
-	vendor = x86_cpuid_vendor();
-	family = x86_cpuid_family();
+	vendor	    = x86_cpuid_vendor();
+	cpuid_1_eax = native_cpuid_eax(1);
 
 	switch (vendor) {
 	case X86_VENDOR_INTEL:
-		if (family >= 6)
+		if (x86_family(cpuid_1_eax) >= 6)
 			load_ucode_intel_ap();
 		break;
 	case X86_VENDOR_AMD:
-		if (family >= 0x10)
-			load_ucode_amd_ap(family);
+		if (x86_family(cpuid_1_eax) >= 0x10)
+			load_ucode_amd_ap(cpuid_1_eax);
 		break;
 	default:
 		break;
@@ -198,7 +188,7 @@ static int __init save_microcode_in_initrd(void)
 		break;
 	case X86_VENDOR_AMD:
 		if (c->x86 >= 0x10)
-			return save_microcode_in_initrd_amd(c->x86);
+			return save_microcode_in_initrd_amd(cpuid_eax(1));
 		break;
 	default:
 		break;
#18 lore raw

[tip:x86/mce] x86/microcode/AMD: Get rid of global this_equiv_id

In reply to Borislav Petkov
x86/microcode/AMD: Get rid of global this_equiv_id
Commit-ID:  3da9b41794590022d09caa345aaa7c812ac22971
Gitweb:     http://git.kernel.org/tip/3da9b41794590022d09caa345aaa7c812ac22971
Author:     Borislav Petkov <[email protected]>
AuthorDate: Fri, 20 Jan 2017 21:29:48 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 23 Jan 2017 10:02:48 +0100

x86/microcode/AMD: Get rid of global this_equiv_id

We have a container which we update/prepare each time before applying a microcode patch instead of using a global.

---
 arch/x86/kernel/cpu/microcode/amd.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +1 -4
index 9fb398e..4cbfe70 100644
@@ -56,7 +56,6 @@ static struct cont_desc {
 
 static u32 ucode_new_rev;
 static u8 amd_ucode_patch[PATCH_MAX_SIZE];
-static u16 this_equiv_id;
 
 /*
  * Microcode patch container file is prepended to the initrd in cpio
@@ -228,8 +227,6 @@ apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size,
 	if (!desc.eq_id)
 		return ret;
 
-	this_equiv_id = desc.eq_id;
-
 	mc = desc.mc;
 	if (!mc)
 		return ret;
@@ -375,7 +372,7 @@ reget:
 	if (!eq_id)
 		return;
 
-	if (eq_id == this_equiv_id) {
+	if (eq_id == cont.eq_id) {
 		mc = (struct microcode_amd *)amd_ucode_patch;
 
 		if (mc && rev < mc->hdr.patch_id) {
#20 lore raw

[tip:x86/mce] x86/microcode/AMD: Use find_microcode_in_initrd()

In reply to Borislav Petkov
x86/microcode/AMD: Use find_microcode_in_initrd()
Commit-ID:  8cc26e0b4c49246564f773edbbefa3d5dc91d56e
Gitweb:     http://git.kernel.org/tip/8cc26e0b4c49246564f773edbbefa3d5dc91d56e
Author:     Borislav Petkov <[email protected]>
AuthorDate: Fri, 20 Jan 2017 21:29:49 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 23 Jan 2017 10:02:48 +0100

x86/microcode/AMD: Use find_microcode_in_initrd()

Use the generic helper instead of semi-open-coding the procedure.

---
 arch/x86/kernel/cpu/microcode/amd.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +5 -11
index 4cbfe70..7727f27 100644
@@ -337,17 +337,15 @@ void load_ucode_amd_ap(unsigned int cpuid_1_eax)
 
 	/* First AP hasn't cached it yet, go through the blob. */
 	if (!cont.data) {
-		struct cpio_data cp = { NULL, 0, "" };
+		struct cpio_data cp;
 
 		if (cont.size == -1)
 			return;
 
 reget:
 		if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax))) {
-#ifdef CONFIG_BLK_DEV_INITRD
-			cp = find_cpio_data(ucode_path, (void *)initrd_start,
-					    initrd_end - initrd_start, NULL);
-#endif
+			cp = find_microcode_in_initrd(ucode_path, false);
+
 			if (!(cp.data && cp.size)) {
 				/*
 				 * Mark it so that other APs do not scan again
@@ -401,13 +399,9 @@ int __init save_microcode_in_initrd_amd(unsigned int cpuid_1_eax)
 
 	if (!cont.data) {
 		if (IS_ENABLED(CONFIG_X86_32) && (cont.size != -1)) {
-			struct cpio_data cp = { NULL, 0, "" };
-
-#ifdef CONFIG_BLK_DEV_INITRD
-			cp = find_cpio_data(ucode_path, (void *)initrd_start,
-					    initrd_end - initrd_start, NULL);
-#endif
+			struct cpio_data cp;
 
+			cp = find_microcode_in_initrd(ucode_path, false);
 			if (!(cp.data && cp.size)) {
 				cont.size = -1;
 				return -EINVAL;
#22 lore raw

[tip:x86/mce] x86/microcode: Remove local vendor variable

In reply to Borislav Petkov
x86/microcode: Remove local vendor variable
Commit-ID:  7a93a40be23e5557934d773cc89b7b3627c08097
Gitweb:     http://git.kernel.org/tip/7a93a40be23e5557934d773cc89b7b3627c08097
Author:     Borislav Petkov <[email protected]>
AuthorDate: Fri, 20 Jan 2017 21:29:50 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 23 Jan 2017 10:02:49 +0100

x86/microcode: Remove local vendor variable

Use x86_cpuid_vendor() directly.

No functionality change.

---
 arch/x86/kernel/cpu/microcode/core.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
arch/x86/kernel/cpu/microcode/core.c +4 -6
index 437996c..dc54518 100644
@@ -122,15 +122,14 @@ bool get_builtin_firmware(struct cpio_data *cd, const char *name)
 
 void __init load_ucode_bsp(void)
 {
-	unsigned int vendor, cpuid_1_eax;
+	unsigned int cpuid_1_eax;
 
 	if (check_loader_disabled_bsp())
 		return;
 
-	vendor	    = x86_cpuid_vendor();
 	cpuid_1_eax = native_cpuid_eax(1);
 
-	switch (vendor) {
+	switch (x86_cpuid_vendor()) {
 	case X86_VENDOR_INTEL:
 		if (x86_family(cpuid_1_eax) >= 6)
 			load_ucode_intel_bsp();
@@ -155,15 +154,14 @@ static bool check_loader_disabled_ap(void)
 
 void load_ucode_ap(void)
 {
-	unsigned int vendor, cpuid_1_eax;
+	unsigned int cpuid_1_eax;
 
 	if (check_loader_disabled_ap())
 		return;
 
-	vendor	    = x86_cpuid_vendor();
 	cpuid_1_eax = native_cpuid_eax(1);
 
-	switch (vendor) {
+	switch (x86_cpuid_vendor()) {
 	case X86_VENDOR_INTEL:
 		if (x86_family(cpuid_1_eax) >= 6)
 			load_ucode_intel_ap();
#24 lore raw

[tip:x86/mce] x86/microcode/AMD: Check patch level only on the BSP

In reply to Borislav Petkov
x86/microcode/AMD: Check patch level only on the BSP
Commit-ID:  f3ad136d6ef966c8ba9090770c2bfe7e85f18471
Gitweb:     http://git.kernel.org/tip/f3ad136d6ef966c8ba9090770c2bfe7e85f18471
Author:     Borislav Petkov <[email protected]>
AuthorDate: Fri, 20 Jan 2017 21:29:51 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 23 Jan 2017 10:02:50 +0100

x86/microcode/AMD: Check patch level only on the BSP

Check final patch levels for AMD only on the BSP. This way, we decide early and only once whether to continue loading or to leave the loader disabled on such systems.

Simplify a lot.

---
 arch/x86/include/asm/microcode_amd.h |  2 -
 arch/x86/kernel/cpu/microcode/amd.c  | 78 +++++-------------------------------
 arch/x86/kernel/cpu/microcode/core.c | 41 +++++++++++++++++++
 3 files changed, 52 insertions(+), 69 deletions(-)
arch/x86/include/asm/microcode_amd.h +0 -2
index 3e3e20b..3d57009 100644
@@ -54,6 +54,4 @@ static inline int __init
 save_microcode_in_initrd_amd(unsigned int family) { return -EINVAL; }
 void reload_ucode_amd(void) {}
 #endif
-
-extern bool check_current_patch_level(u32 *rev, bool early);
 #endif /* _ASM_X86_MICROCODE_AMD_H */
arch/x86/kernel/cpu/microcode/amd.c +11 -67

139 lines not on this page · raw message

arch/x86/kernel/cpu/microcode/core.c +41 -0

53 lines not on this page · raw message

#30 lore raw

[tip:x86/mce] x86/microcode/AMD: Remove AP scanning optimization

In reply to Borislav Petkov
x86/microcode/AMD: Remove AP scanning optimization
Commit-ID:  69f5f983001f6d097aac774a9e917f44657f3367
Gitweb:     http://git.kernel.org/tip/69f5f983001f6d097aac774a9e917f44657f3367
Author:     Borislav Petkov <[email protected]>
AuthorDate: Fri, 20 Jan 2017 21:29:54 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 23 Jan 2017 10:02:51 +0100

x86/microcode/AMD: Remove AP scanning optimization

The idea was to not scan the microcode blob on each AP (Application Processor) during boot and thus save us some milliseconds. However, on architectures where the microcode engine is shared between threads, this doesn't work. Here's why:

The microcode on CPU0, i.e., the first thread, gets updated. The second thread, i.e., CPU1, i.e., the first AP walks into load_ucode_amd_ap(), sees that there's no container cached and goes and scans for the proper blob.

It finds it and as a last step of apply_microcode_early_amd(), it tries to apply the patch but that core has already the updated microcode revision which it has received through CPU0's update. So it returns false and we do desc->size = -1 to prevent other APs from scanning.

However, the next AP, CPU2, has a different microcode engine which hasn't been updated yet. The desc->size == -1 test prevents it from scanning the blob anew and we fail to update it.

The fix is much more straight-forward than it looks: the BSP (BootStrapping Processor), i.e., CPU0, caches the microcode patch in amd_ucode_patch. We use that on the AP and try to apply it. In the 99.9999% of cases where we have homogeneous cores - *not* mixed-steppings - the application will be successful and we're good to go.

In the remaining small set of systems, we will simply rescan the blob and find (or not, if none present) the proper patch and apply it then.

---
 arch/x86/kernel/cpu/microcode/amd.c | 78 +++++++++----------------------------
 1 file changed, 18 insertions(+), 60 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +18 -60
index 2a194e3..5e1b577 100644
@@ -45,14 +45,14 @@ static struct equiv_cpu_entry *equiv_cpu_table;
  * save from the initrd/builtin before jettisoning its contents. @mc is the
  * microcode patch we found to match.
  */
-static struct cont_desc {
+struct cont_desc {
 	struct microcode_amd *mc;
 	u32		     cpuid_1_eax;
 	u32		     psize;
 	u16		     eq_id;
 	u8		     *data;
 	size_t		     size;
-} cont;
+};
 
 static u32 ucode_new_rev;
 static u8 amd_ucode_patch[PATCH_MAX_SIZE];
@@ -201,8 +201,7 @@ static int __apply_microcode_amd(struct microcode_amd *mc)
  * Returns true if container found (sets @desc), false otherwise.
  */
 static bool
-apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size,
-			  bool save_patch, struct cont_desc *ret_desc)
+apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size, bool save_patch)
 {
 	struct cont_desc desc = { 0 };
 	u8 (*patch)[PATCH_MAX_SIZE];
@@ -240,9 +239,6 @@ apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size,
 			memcpy(patch, mc, min_t(u32, desc.psize, PATCH_MAX_SIZE));
 	}
 
-	if (ret_desc)
-		*ret_desc = desc;
-
 	return ret;
 }
 
@@ -292,79 +288,41 @@ void __init load_ucode_amd_bsp(unsigned int cpuid_1_eax)
 	struct cpio_data cp = { };
 
 	__load_ucode_amd(cpuid_1_eax, &cp);
-
 	if (!(cp.data && cp.size))
 		return;
 
-	apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, true, NULL);
+	apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, true);
 }
 
 void load_ucode_amd_ap(unsigned int cpuid_1_eax)
 {
-	struct equiv_cpu_entry *eq;
 	struct microcode_amd *mc;
-	struct cont_desc *desc;
-	u16 eq_id;
+	struct cpio_data cp;
+	u32 *new_rev, rev, dummy;
 
 	if (IS_ENABLED(CONFIG_X86_32)) {
-		mc   = (struct microcode_amd *)__pa_nodebug(amd_ucode_patch);
-		desc = (struct cont_desc *)__pa_nodebug(&cont);
+		mc	= (struct microcode_amd *)__pa_nodebug(amd_ucode_patch);
+		new_rev = (u32 *)__pa_nodebug(&ucode_new_rev);
 	} else {
-		mc   = (struct microcode_amd *)amd_ucode_patch;
-		desc = &cont;
+		mc	= (struct microcode_amd *)amd_ucode_patch;
+		new_rev = &ucode_new_rev;
 	}
 
-	/* First AP hasn't cached it yet, go through the blob. */
-	if (!desc->data) {
-		struct cpio_data cp = { };
-
-		if (desc->size == -1)
-			return;
-
-reget:
-		__load_ucode_amd(cpuid_1_eax, &cp);
-		if (!(cp.data && cp.size)) {
-			/*
-			 * Mark it so that other APs do not scan again for no
-			 * real reason and slow down boot needlessly.
-			 */
-			desc->size = -1;
-			return;
-		}
+	native_rdmsr(MSR_AMD64_PATCH_LEVEL, rev, dummy);
 
-		if (!apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, false, desc)) {
-			desc->data = NULL;
-			desc->size = -1;
+	/* Check whether we have saved a new patch already: */
+	if (*new_rev && rev < mc->hdr.patch_id) {
+		if (!__apply_microcode_amd(mc)) {
+			*new_rev = mc->hdr.patch_id;
 			return;
 		}
 	}
 
-	eq  = (struct equiv_cpu_entry *)(desc->data + CONTAINER_HDR_SZ);
-
-	eq_id = find_equiv_id(eq, cpuid_1_eax);
-	if (!eq_id)
+	__load_ucode_amd(cpuid_1_eax, &cp);
+	if (!(cp.data && cp.size))
 		return;
 
-	if (eq_id == desc->eq_id) {
-		u32 rev, dummy;
-
-		native_rdmsr(MSR_AMD64_PATCH_LEVEL, rev, dummy);
-
-		mc = (struct microcode_amd *)amd_ucode_patch;
-
-		if (mc && rev < mc->hdr.patch_id) {
-			if (!__apply_microcode_amd(mc))
-				ucode_new_rev = mc->hdr.patch_id;
-		}
-
-	} else {
-
-		/*
-		 * AP has a different equivalence ID than BSP, looks like
-		 * mixed-steppings silicon so go through the ucode blob anew.
-		 */
-		goto reget;
-	}
+	apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, false);
 }
 
 static enum ucode_state
#32 lore raw

[tip:x86/mce] x86/microcode/AMD: Remove struct cont_desc.eq_id

In reply to Borislav Petkov
x86/microcode/AMD: Remove struct cont_desc.eq_id
Commit-ID:  da0aa3dde05108e180eecd76534c55f43ea4b9c8
Gitweb:     http://git.kernel.org/tip/da0aa3dde05108e180eecd76534c55f43ea4b9c8
Author:     Borislav Petkov <[email protected]>
AuthorDate: Fri, 20 Jan 2017 21:29:55 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 23 Jan 2017 10:02:51 +0100

x86/microcode/AMD: Remove struct cont_desc.eq_id

The equivalence ID was needed outside of the container scanning logic but now, after this has been cleaned up, not anymore. Now, cont_desc.mc is used to denote whether the container we're looking at has the proper microcode patch for this CPU or not.

---
 arch/x86/kernel/cpu/microcode/amd.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +4 -10
index 5e1b577..7889ae4 100644
@@ -49,7 +49,6 @@ struct cont_desc {
 	struct microcode_amd *mc;
 	u32		     cpuid_1_eax;
 	u32		     psize;
-	u16		     eq_id;
 	u8		     *data;
 	size_t		     size;
 };
@@ -92,10 +91,8 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
 	/* Am I looking at an equivalence table header? */
 	if (hdr[0] != UCODE_MAGIC ||
 	    hdr[1] != UCODE_EQUIV_CPU_TABLE_TYPE ||
-	    hdr[2] == 0) {
-		desc->eq_id = 0;
+	    hdr[2] == 0)
 		return CONTAINER_HDR_SZ;
-	}
 
 	buf = ucode;
 
@@ -147,9 +144,8 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
 	 * buffer.
 	 */
 	if (desc->mc) {
-		desc->eq_id = eq_id;
-		desc->data  = ucode;
-		desc->size  = orig_size - size;
+		desc->data = ucode;
+		desc->size = orig_size - size;
 
 		return 0;
 	}
@@ -220,8 +216,6 @@ apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size, bool save_p
 	desc.cpuid_1_eax = cpuid_1_eax;
 
 	scan_containers(ucode, size, &desc);
-	if (!desc.eq_id)
-		return ret;
 
 	mc = desc.mc;
 	if (!mc)
@@ -341,7 +335,7 @@ int __init save_microcode_in_initrd_amd(unsigned int cpuid_1_eax)
 	desc.cpuid_1_eax = cpuid_1_eax;
 
 	scan_containers(cp.data, cp.size, &desc);
-	if (!desc.eq_id)
+	if (!desc.mc)
 		return -EINVAL;
 
 	ret = load_microcode_amd(smp_processor_id(), x86_family(cpuid_1_eax),
#28 lore raw

[tip:x86/mce] x86/microcode/AMD: Simplify saving from initrd

In reply to Borislav Petkov
x86/microcode/AMD: Simplify saving from initrd
Commit-ID:  72edfe950b36308353e27cdc02f334431239938a
Gitweb:     http://git.kernel.org/tip/72edfe950b36308353e27cdc02f334431239938a
Author:     Borislav Petkov <[email protected]>
AuthorDate: Fri, 20 Jan 2017 21:29:53 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 23 Jan 2017 10:02:50 +0100

x86/microcode/AMD: Simplify saving from initrd

No need to use the previously stashed info in the container - simply go ahead and parse the initrd once more. It simplifies and streamlines the code a whole lot.

---
 arch/x86/kernel/cpu/microcode/amd.c | 43 +++++++++++--------------------------
 1 file changed, 13 insertions(+), 30 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +13 -30
index fe9e865..2a194e3 100644
@@ -372,43 +372,26 @@ load_microcode_amd(int cpu, u8 family, const u8 *data, size_t size);
 
 int __init save_microcode_in_initrd_amd(unsigned int cpuid_1_eax)
 {
+	struct cont_desc desc = { 0 };
 	enum ucode_state ret;
-	int retval = 0;
-
-	if (!cont.data) {
-		if (IS_ENABLED(CONFIG_X86_32) && (cont.size != -1)) {
-			struct cpio_data cp;
-
-			cp = find_microcode_in_initrd(ucode_path, false);
-			if (!(cp.data && cp.size)) {
-				cont.size = -1;
-				return -EINVAL;
-			}
+	struct cpio_data cp;
 
-			cont.cpuid_1_eax = cpuid_1_eax;
+	cp = find_microcode_in_initrd(ucode_path, false);
+	if (!(cp.data && cp.size))
+		return -EINVAL;
 
-			scan_containers(cp.data, cp.size, &cont);
-			if (!cont.eq_id) {
-				cont.size = -1;
-				return -EINVAL;
-			}
+	desc.cpuid_1_eax = cpuid_1_eax;
 
-		} else
-			return -EINVAL;
-	}
+	scan_containers(cp.data, cp.size, &desc);
+	if (!desc.eq_id)
+		return -EINVAL;
 
-	ret = load_microcode_amd(smp_processor_id(), x86_family(cpuid_1_eax), cont.data, cont.size);
+	ret = load_microcode_amd(smp_processor_id(), x86_family(cpuid_1_eax),
+				 desc.data, desc.size);
 	if (ret != UCODE_OK)
-		retval = -EINVAL;
-
-	/*
-	 * This will be freed any msec now, stash patches for the current
-	 * family and switch to patch cache for cpu hotplug, etc later.
-	 */
-	cont.data = NULL;
-	cont.size = 0;
+		return -EINVAL;
 
-	return retval;
+	return 0;
 }
 
 void reload_ucode_amd(void)
#26 lore raw

[tip:x86/mce] x86/microcode/AMD: Unify load_ucode_amd_ap()

In reply to Borislav Petkov
x86/microcode/AMD: Unify load_ucode_amd_ap()
Commit-ID:  e71bb4ec073901ad50bfa86fed74fce7ac3210fe
Gitweb:     http://git.kernel.org/tip/e71bb4ec073901ad50bfa86fed74fce7ac3210fe
Author:     Borislav Petkov <[email protected]>
AuthorDate: Fri, 20 Jan 2017 21:29:52 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 23 Jan 2017 10:02:50 +0100

x86/microcode/AMD: Unify load_ucode_amd_ap()

Use a version for both bitness by adding a helper which does the actual container finding and parsing which can be used on any CPU - BSP or AP. Streamlines the paths more.

---
 arch/x86/kernel/cpu/microcode/amd.c | 81 ++++++++++++++-----------------------
 1 file changed, 31 insertions(+), 50 deletions(-)
arch/x86/kernel/cpu/microcode/amd.c +31 -50
index 6174347..fe9e865 100644
@@ -261,7 +261,7 @@ static bool get_builtin_microcode(struct cpio_data *cp, unsigned int family)
 #endif
 }
 
-void __init load_ucode_amd_bsp(unsigned int cpuid_1_eax)
+void __load_ucode_amd(unsigned int cpuid_1_eax, struct cpio_data *ret)
 {
 	struct ucode_cpu_info *uci;
 	struct cpio_data cp;
@@ -281,89 +281,71 @@ void __init load_ucode_amd_bsp(unsigned int cpuid_1_eax)
 	if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)))
 		cp = find_microcode_in_initrd(path, use_pa);
 
-	if (!(cp.data && cp.size))
-		return;
-
 	/* Needed in load_microcode_amd() */
 	uci->cpu_sig.sig = cpuid_1_eax;
 
-	apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, true, NULL);
+	*ret = cp;
 }
 
-#ifdef CONFIG_X86_32
-/*
- * On 32-bit, since AP's early load occurs before paging is turned on, we
- * cannot traverse cpu_equiv_table and microcode_cache in kernel heap memory.
- * So during cold boot, AP will apply_ucode_in_initrd() just like the BSP.
- * In save_microcode_in_initrd_amd() BSP's patch is copied to amd_ucode_patch,
- * which is used upon resume from suspend.
- */
-void load_ucode_amd_ap(unsigned int cpuid_1_eax)
+void __init load_ucode_amd_bsp(unsigned int cpuid_1_eax)
 {
-	struct microcode_amd *mc;
-	struct cpio_data cp;
+	struct cpio_data cp = { };
 
-	mc = (struct microcode_amd *)__pa_nodebug(amd_ucode_patch);
-	if (mc->hdr.patch_id && mc->hdr.processor_rev_id) {
-		__apply_microcode_amd(mc);
-		return;
-	}
-
-	if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)))
-		cp = find_microcode_in_initrd((const char *)__pa_nodebug(ucode_path), true);
+	__load_ucode_amd(cpuid_1_eax, &cp);
 
 	if (!(cp.data && cp.size))
 		return;
 
-	/*
-	 * This would set amd_ucode_patch above so that the following APs can
-	 * use it directly instead of going down this path again.
-	 */
 	apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, true, NULL);
 }
-#else
+
 void load_ucode_amd_ap(unsigned int cpuid_1_eax)
 {
 	struct equiv_cpu_entry *eq;
 	struct microcode_amd *mc;
+	struct cont_desc *desc;
 	u16 eq_id;
 
+	if (IS_ENABLED(CONFIG_X86_32)) {
+		mc   = (struct microcode_amd *)__pa_nodebug(amd_ucode_patch);
+		desc = (struct cont_desc *)__pa_nodebug(&cont);
+	} else {
+		mc   = (struct microcode_amd *)amd_ucode_patch;
+		desc = &cont;
+	}
+
 	/* First AP hasn't cached it yet, go through the blob. */
-	if (!cont.data) {
-		struct cpio_data cp;
+	if (!desc->data) {
+		struct cpio_data cp = { };
 
-		if (cont.size == -1)
+		if (desc->size == -1)
 			return;
 
 reget:
-		if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax))) {
-			cp = find_microcode_in_initrd(ucode_path, false);
-
-			if (!(cp.data && cp.size)) {
-				/*
-				 * Mark it so that other APs do not scan again
-				 * for no real reason and slow down boot
-				 * needlessly.
-				 */
-				cont.size = -1;
-				return;
-			}
+		__load_ucode_amd(cpuid_1_eax, &cp);
+		if (!(cp.data && cp.size)) {
+			/*
+			 * Mark it so that other APs do not scan again for no
+			 * real reason and slow down boot needlessly.
+			 */
+			desc->size = -1;
+			return;
 		}
 
-		if (!apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, false, &cont)) {
-			cont.data = NULL;
-			cont.size = -1;
+		if (!apply_microcode_early_amd(cpuid_1_eax, cp.data, cp.size, false, desc)) {
+			desc->data = NULL;
+			desc->size = -1;
 			return;
 		}
 	}
 
-	eq  = (struct equiv_cpu_entry *)(cont.data + CONTAINER_HDR_SZ);
+	eq  = (struct equiv_cpu_entry *)(desc->data + CONTAINER_HDR_SZ);
 
 	eq_id = find_equiv_id(eq, cpuid_1_eax);
 	if (!eq_id)
 		return;
 
-	if (eq_id == cont.eq_id) {
+	if (eq_id == desc->eq_id) {
 		u32 rev, dummy;
 
 		native_rdmsr(MSR_AMD64_PATCH_LEVEL, rev, dummy);
@@ -384,7 +366,6 @@ reget:
 		goto reget;
 	}
 }
-#endif /* CONFIG_X86_32 */
 
 static enum ucode_state
 load_microcode_amd(int cpu, u8 family, const u8 *data, size_t size);