soc/amd/common/block/cpu: Refactor ucode allocation

Move microcode load/unload to pre_mp_init and post_mp_init callbacks.
It allows to make sure that ucode is freed only if all APs updated
microcode.

BUG=b:278264488
TEST=Build and run with additional debug prints added
to confirm that data are correctly unmapped

Change-Id: I200d24df6157cc6d06bade34809faefea9f0090a
Signed-off-by: Grzegorz Bernacki <bernacki@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/74777
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>

7 files changed, 61 insertions, 39 deletions

diff --git a/src/soc/amd/cezanne/cpu.c b/src/soc/amd/cezanne/cpu.c
index 6149a84610..9261d54f87 100644
--- a/src/soc/amd/cezanne/cpu.c
+++ b/src/soc/amd/cezanne/cpu.c
@@ -39,7 +39,7 @@ static void zen_2_3_init(struct device *dev)
 	check_mca();
 	set_cstate_io_addr();
 
-	amd_update_microcode_from_cbfs();
+	amd_apply_microcode_patch();
 }
 
 static struct device_operations cpu_dev_ops = {
diff --git a/src/soc/amd/common/block/cpu/smm/smm_relocate.c b/src/soc/amd/common/block/cpu/smm/smm_relocate.c
index 1b929c7ffa..2e3bff189c 100644
--- a/src/soc/amd/common/block/cpu/smm/smm_relocate.c
+++ b/src/soc/amd/common/block/cpu/smm/smm_relocate.c
@@ -6,6 +6,7 @@
 #include <amdblocks/smm.h>
 #include <console/console.h>
 #include <cpu/amd/amd64_save_state.h>
+#include <cpu/amd/microcode.h>
 #include <cpu/amd/msr.h>
 #include <cpu/amd/mtrr.h>
 #include <cpu/cpu.h>
@@ -22,6 +23,8 @@ static void pre_mp_init(void)
 	else
 		x86_setup_mtrrs_with_detect();
 	x86_mtrr_check();
+	if (CONFIG(SOC_AMD_COMMON_BLOCK_UCODE))
+		amd_load_microcode_from_cbfs();
 }
 
 static void get_smm_info(uintptr_t *perm_smbase, size_t *perm_smsize,
@@ -83,10 +86,17 @@ static void smm_relocation_handler(void)
 	lock_smm();
 }
 
+static void post_mp_init(void)
+{
+	if (CONFIG(SOC_AMD_COMMON_BLOCK_UCODE))
+		amd_free_microcode();
+	global_smi_enable();
+}
+
 const struct mp_ops amd_mp_ops_with_smm = {
 	.pre_mp_init = pre_mp_init,
 	.get_cpu_count = get_cpu_count,
 	.get_smm_info = get_smm_info,
 	.per_cpu_smm_trigger = smm_relocation_handler,
-	.post_mp_init = global_smi_enable,
+	.post_mp_init = post_mp_init,
 };
diff --git a/src/soc/amd/common/block/cpu/update_microcode.c b/src/soc/amd/common/block/cpu/update_microcode.c
index 0edb9d29a9..14c4f36b98 100644
--- a/src/soc/amd/common/block/cpu/update_microcode.c
+++ b/src/soc/amd/common/block/cpu/update_microcode.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 
+#include <amdblocks/cpu.h>
 #include <cbfs.h>
 #include <commonlib/helpers.h>
 #include <console/console.h>
@@ -32,21 +33,30 @@ struct microcode {
 	uint8_t reserved2[4];
 } __packed;
 
-static void apply_microcode_patch(const struct microcode *m)
+static const struct microcode *ucode;
+
+/* This function requires the ucode variable to be initialized by amd_load_microcode_from_cbfs()
+   and then allocated memory should be freed by the amd_free_microcode(). */
+void amd_apply_microcode_patch(void)
 {
 	uint32_t new_patch_id;
 	msr_t msr;
 
-	msr.raw = (uintptr_t)m;
+	if (!ucode) {
+		printk(BIOS_ERR, "%s: NULL pointer to microcode\n", __func__);
+		return;
+	}
+
+	msr.raw = (uintptr_t)ucode;
 
 	wrmsr(MSR_PATCH_LOADER, msr);
 
-	printk(BIOS_DEBUG, "microcode: patch id to apply = 0x%08x\n", m->patch_id);
+	printk(BIOS_DEBUG, "microcode: patch id to apply = 0x%08x\n", ucode->patch_id);
 
 	msr = rdmsr(IA32_BIOS_SIGN_ID);
 	new_patch_id = msr.lo;
 
-	if (new_patch_id == m->patch_id)
+	if (new_patch_id == ucode->patch_id)
 		printk(BIOS_INFO, "microcode: being updated to patch id = 0x%08x succeeded\n",
 		       new_patch_id);
 	else
@@ -61,11 +71,11 @@ static uint16_t get_equivalent_processor_rev_id(void)
 	return (uint16_t)((cpuid_family & 0xff0000) >> 8 | (cpuid_family & 0xff));
 }
 
-static const struct microcode *find_microcode(const struct microcode *ucode,
+static const struct microcode *find_microcode(const struct microcode *microcode,
 					      uint16_t equivalent_processor_rev_id)
 {
-	if (ucode->processor_rev_id == equivalent_processor_rev_id)
-		return ucode;
+	if (microcode->processor_rev_id == equivalent_processor_rev_id)
+		return microcode;
 
 	printk(BIOS_WARNING, "Failed to find microcode for processor rev: %hx.\n",
 	       equivalent_processor_rev_id);
@@ -73,39 +83,41 @@ static const struct microcode *find_microcode(const struct microcode *ucode,
 	return NULL;
 }
 
-void amd_update_microcode_from_cbfs(void)
+void amd_load_microcode_from_cbfs(void)
 {
-	static const struct microcode *ucode_cache;
-	static bool cache_valid;
-
-	struct microcode *ucode;
 	char name[] = CPU_MICROCODE_BLOB_NAME;
 	uint16_t equivalent_processor_rev_id;
 
-	/* Cache the buffer so each CPU doesn't need to read the uCode from flash */
-	/* Note that this code assumes all cores are the same */
-	if (!cache_valid) {
-		timestamp_add_now(TS_READ_UCODE_START);
-		equivalent_processor_rev_id = get_equivalent_processor_rev_id();
-		snprintf(name, sizeof(name), CPU_MICROCODE_BLOB_FORMAT, equivalent_processor_rev_id);
-		ucode = cbfs_map(name, NULL);
-		if (!ucode) {
-			printk(BIOS_WARNING, "%s not found. Skipping updates.\n", name);
-			return;
-		}
-
-		ucode_cache = find_microcode(ucode, equivalent_processor_rev_id);
-
-		if (!ucode_cache) {
-			cbfs_unmap(ucode);
-			return;
-		}
-
-		cache_valid = true;
+	if (ucode)
+		return;
+
+	/* Store the pointer to the buffer in global variable, so each CPU doesn't need to read
+	 * the uCode from flash. Note that this code assumes all cores are the same */
+	timestamp_add_now(TS_READ_UCODE_START);
+	equivalent_processor_rev_id = get_equivalent_processor_rev_id();
+	snprintf(name, sizeof(name), CPU_MICROCODE_BLOB_FORMAT, equivalent_processor_rev_id);
+
+	ucode = cbfs_map(name, NULL);
+	if (!ucode) {
+		printk(BIOS_WARNING, "%s not found. Skipping updates.\n", name);
 		timestamp_add_now(TS_READ_UCODE_END);
+		return;
+	}
+
+	if (find_microcode(ucode, equivalent_processor_rev_id) == NULL) {
+		cbfs_unmap((void *)ucode);
+		ucode = NULL;
 	}
 
-	apply_microcode_patch(ucode_cache);
+	timestamp_add_now(TS_READ_UCODE_END);
+}
+
+void amd_free_microcode(void)
+{
+	if (ucode) {
+		cbfs_unmap((void *)ucode);
+		ucode = NULL;
+	}
 }
 
 void preload_microcode(void)
diff --git a/src/soc/amd/glinda/cpu.c b/src/soc/amd/glinda/cpu.c
index c745b73030..49cd11cdbf 100644
--- a/src/soc/amd/glinda/cpu.c
+++ b/src/soc/amd/glinda/cpu.c
@@ -42,7 +42,7 @@ static void zen_2_3_init(struct device *dev)
 	check_mca();
 	set_cstate_io_addr();
 
-	amd_update_microcode_from_cbfs();
+	amd_apply_microcode_patch();
 }
 
 static struct device_operations cpu_dev_ops = {
diff --git a/src/soc/amd/mendocino/cpu.c b/src/soc/amd/mendocino/cpu.c
index b9297b4323..c742db0e75 100644
--- a/src/soc/amd/mendocino/cpu.c
+++ b/src/soc/amd/mendocino/cpu.c
@@ -40,7 +40,7 @@ static void zen_2_3_init(struct device *dev)
 	check_mca();
 	set_cstate_io_addr();
 
-	amd_update_microcode_from_cbfs();
+	amd_apply_microcode_patch();
 }
 
 static struct device_operations cpu_dev_ops = {
diff --git a/src/soc/amd/phoenix/cpu.c b/src/soc/amd/phoenix/cpu.c
index 848f667bf1..2c8f77130a 100644
--- a/src/soc/amd/phoenix/cpu.c
+++ b/src/soc/amd/phoenix/cpu.c
@@ -42,7 +42,7 @@ static void zen_2_3_init(struct device *dev)
 	check_mca();
 	set_cstate_io_addr();
 
-	amd_update_microcode_from_cbfs();
+	amd_apply_microcode_patch();
 }
 
 static struct device_operations cpu_dev_ops = {
diff --git a/src/soc/amd/picasso/cpu.c b/src/soc/amd/picasso/cpu.c
index c5fc8a8bf9..c0b918e5c0 100644
--- a/src/soc/amd/picasso/cpu.c
+++ b/src/soc/amd/picasso/cpu.c
@@ -40,7 +40,7 @@ static void model_17_init(struct device *dev)
 	check_mca();
 	set_cstate_io_addr();
 
-	amd_update_microcode_from_cbfs();
+	amd_apply_microcode_patch();
 }
 
 static struct device_operations cpu_dev_ops = {