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>
diff --git a/src/include/cpu/amd/microcode.h b/src/include/cpu/amd/microcode.h
index 9e88936..b6b158c 100644
--- a/src/include/cpu/amd/microcode.h
+++ b/src/include/cpu/amd/microcode.h
@@ -4,6 +4,9 @@
#define CPU_AMD_MICROCODE_H
void amd_update_microcode_from_cbfs(void);
+void amd_load_microcode_from_cbfs(void);
+void amd_free_microcode(void);
+void amd_apply_microcode_patch(void);
void preload_microcode(void);
#endif /* CPU_AMD_MICROCODE_H */
diff --git a/src/soc/amd/cezanne/cpu.c b/src/soc/amd/cezanne/cpu.c
index 6149a84..9261d54 100644
--- a/src/soc/amd/cezanne/cpu.c
+++ b/src/soc/amd/cezanne/cpu.c
@@ -39,7 +39,7 @@
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 1b929c7..2e3bff1 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 @@
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 @@
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 0edb9d2..14c4f36 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 @@
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 @@
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 @@
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;
- }
+ if (ucode)
+ return;
- ucode_cache = find_microcode(ucode, equivalent_processor_rev_id);
+ /* 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);
- if (!ucode_cache) {
- cbfs_unmap(ucode);
- return;
- }
-
- cache_valid = true;
+ 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;
}
- apply_microcode_patch(ucode_cache);
+ if (find_microcode(ucode, equivalent_processor_rev_id) == NULL) {
+ cbfs_unmap((void *)ucode);
+ ucode = NULL;
+ }
+
+ 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 c745b73..49cd11c 100644
--- a/src/soc/amd/glinda/cpu.c
+++ b/src/soc/amd/glinda/cpu.c
@@ -42,7 +42,7 @@
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 b9297b4..c742db0 100644
--- a/src/soc/amd/mendocino/cpu.c
+++ b/src/soc/amd/mendocino/cpu.c
@@ -40,7 +40,7 @@
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 848f667..2c8f7713 100644
--- a/src/soc/amd/phoenix/cpu.c
+++ b/src/soc/amd/phoenix/cpu.c
@@ -42,7 +42,7 @@
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 c5fc8a8..c0b918e 100644
--- a/src/soc/amd/picasso/cpu.c
+++ b/src/soc/amd/picasso/cpu.c
@@ -40,7 +40,7 @@
check_mca();
set_cstate_io_addr();
- amd_update_microcode_from_cbfs();
+ amd_apply_microcode_patch();
}
static struct device_operations cpu_dev_ops = {