Hello Zheng Bao, Zheng Bao,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/42094
to review the following change.
Change subject: amd/00730F01: Clean the Microcode updating. ......................................................................
amd/00730F01: Clean the Microcode updating.
According to the comments of https://review.coreboot.org/c/coreboot/+/41719 , Change the code with the same way. Also, 41719 should be based on this.
Change-Id: I8b0da99db0d3189058f75e199f05492c4e6c5881 Signed-off-by: Zheng Bao zheng.bao@amd.com Signed-off-by: Zheng Bao fishbaozi@gmail.com --- M src/cpu/amd/pi/00730F01/Makefile.inc D src/cpu/amd/pi/00730F01/microcode_fam16h.c M src/cpu/amd/pi/00730F01/model_16_init.c M src/cpu/amd/pi/00730F01/update_microcode.c M src/include/cpu/amd/microcode.h 5 files changed, 132 insertions(+), 148 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/42094/1
diff --git a/src/cpu/amd/pi/00730F01/Makefile.inc b/src/cpu/amd/pi/00730F01/Makefile.inc index 45ec1d9..406fafb9 100644 --- a/src/cpu/amd/pi/00730F01/Makefile.inc +++ b/src/cpu/amd/pi/00730F01/Makefile.inc @@ -6,7 +6,6 @@ ramstage-y += chip_name.c ramstage-y += model_16_init.c ramstage-y += update_microcode.c -ramstage-y += microcode_fam16h.c
subdirs-y += ../../mtrr subdirs-y += ../../../x86/tsc diff --git a/src/cpu/amd/pi/00730F01/microcode_fam16h.c b/src/cpu/amd/pi/00730F01/microcode_fam16h.c deleted file mode 100644 index 7d47258..0000000 --- a/src/cpu/amd/pi/00730F01/microcode_fam16h.c +++ /dev/null @@ -1,129 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <stdint.h> -#include <console/console.h> -#include <cpu/x86/msr.h> -#include <cpu/amd/microcode.h> -#include <cbfs.h> - -/* - * Values and header structure from: - * BKDG for AMD Family 16h Models 30h-3Fh Processors - * 52740 Rev 3.06 - March 18, 2016 - */ - -#define F16H_MPB_MAX_SIZE 3458 -#define F16H_MPB_DATA_OFFSET 32 - - /* - * STRUCTURE OF A MICROCODE (UCODE) FILE FOR FAM16h - * Microcode Patch Block - * Microcode Header - * Microcode "Blob" - * ... - * ... - * (end of file) - * - * - * MICROCODE HEADER (offset 0 bytes from start of file) - * Total size = 32 bytes - * [0:3] Date code (32 bits) - * [4:7] Patch level (32 bits) - * [8:9] Microcode patch data ID (16 bits) - * [10:15] Reserved (48 bits) - * [16:19] Chipset 1 device ID (32 bits) - * [20:23] Chipset 2 device ID (32 bits) - * [24:25] Processor Revisions ID (16 bits) - * [26] Chipset 1 revision ID (8 bits) - * [27] Chipset 2 revision ID (8 bits) - * [28:31] Reserved (32 bits) - * - * MICROCODE BLOB (offset += 32) - * Total size = m bytes - * - */ - -struct microcode { - uint32_t date_code; - uint32_t patch_id; - - uint16_t mc_patch_data_id; - uint8_t reserved1[6]; - - uint32_t chipset1_dev_id; - uint32_t chipset2_dev_id; - - uint16_t processor_rev_id; - - uint8_t chipset1_rev_id; - uint8_t chipset2_rev_id; - - uint8_t reserved2[4]; - - uint8_t m_patch_data[F16H_MPB_MAX_SIZE-F16H_MPB_DATA_OFFSET]; - -}; - -static void apply_microcode_patch(const struct microcode *m) -{ - uint32_t new_patch_id; - msr_t msr; - - /* apply patch */ - msr.hi = 0; - msr.lo = (uint32_t)m; - - wrmsr(0xc0010020, msr); - - printk(BIOS_DEBUG, "microcode: patch id to apply = 0x%08x\n", - m->patch_id); - - /* patch authentication */ - msr = rdmsr(0x8b); - new_patch_id = msr.lo; - - printk(BIOS_DEBUG, "microcode: updated to patch id = 0x%08x %s\n", - new_patch_id, - (new_patch_id == m->patch_id) ? "success" : "fail"); -} - -static void amd_update_microcode(const void *ucode, size_t ucode_len, - uint16_t equivalent_processor_rev_id) -{ - const struct microcode *m; - const uint8_t *c = ucode; - - m = (struct microcode *)c; - - if (m->processor_rev_id == equivalent_processor_rev_id) - apply_microcode_patch(m); -} - -void amd_update_microcode_from_cbfs(uint16_t equivalent_processor_rev_id) -{ - const void *ucode; - size_t ucode_len; - - if (equivalent_processor_rev_id == 0) { - printk(BIOS_DEBUG, "microcode: rev id not found. " - "Skipping microcode patch!\n"); - return; - } - ucode = cbfs_boot_map_with_leak("cpu_microcode_blob.bin", - CBFS_TYPE_MICROCODE, - &ucode_len); - if (!ucode) { - printk(BIOS_DEBUG, "cpu_microcode_blob.bin not found. " - "Skipping updates.\n"); - return; - } - - if (ucode_len > F16H_MPB_MAX_SIZE || - ucode_len < F16H_MPB_DATA_OFFSET) { - printk(BIOS_DEBUG, "microcode file invalid. Skipping " - "updates.\n"); - return; - } - - amd_update_microcode(ucode, ucode_len, equivalent_processor_rev_id); -} diff --git a/src/cpu/amd/pi/00730F01/model_16_init.c b/src/cpu/amd/pi/00730F01/model_16_init.c index bdbb37d..2883b17 100644 --- a/src/cpu/amd/pi/00730F01/model_16_init.c +++ b/src/cpu/amd/pi/00730F01/model_16_init.c @@ -87,7 +87,7 @@ msr.lo |= (1 << 0); wrmsr(HWCR_MSR, msr);
- update_microcode(cpuid_eax(1)); + amd_update_microcode_from_cbfs(); }
static struct device_operations cpu_dev_ops = { diff --git a/src/cpu/amd/pi/00730F01/update_microcode.c b/src/cpu/amd/pi/00730F01/update_microcode.c index e3f0c3b..b1cb5af 100644 --- a/src/cpu/amd/pi/00730F01/update_microcode.c +++ b/src/cpu/amd/pi/00730F01/update_microcode.c @@ -2,6 +2,92 @@
#include <stdint.h> #include <cpu/amd/microcode.h> +#include <commonlib/helpers.h> +#include <console/console.h> +#include <cpu/x86/msr.h> +#include <cbfs.h> +#include <cpu/amd/msr.h> + +/* + * Values and header structure from: + * BKDG for AMD Family 16h Models 30h-3Fh Processors + * 52740 Rev 3.06 - March 18, 2016 + */ + +#define F16H_MPB_MAX_SIZE 3458 +#define F16H_MPB_DATA_OFFSET 32 + + /* + * STRUCTURE OF A MICROCODE (UCODE) FILE FOR FAM16h + * Microcode Patch Block + * Microcode Header + * Microcode "Blob" + * ... + * ... + * (end of file) + * + * + * MICROCODE HEADER (offset 0 bytes from start of file) + * Total size = 32 bytes + * [0:3] Date code (32 bits) + * [4:7] Patch level (32 bits) + * [8:9] Microcode patch data ID (16 bits) + * [10:15] Reserved (48 bits) + * [16:19] Chipset 1 device ID (32 bits) + * [20:23] Chipset 2 device ID (32 bits) + * [24:25] Processor Revisions ID (16 bits) + * [26] Chipset 1 revision ID (8 bits) + * [27] Chipset 2 revision ID (8 bits) + * [28:31] Reserved (32 bits) + * + * MICROCODE BLOB (offset += 32) + * Total size = m bytes + * + */ + +struct microcode { + uint32_t date_code; + uint32_t patch_id; + + uint16_t mc_patch_data_id; + uint8_t reserved1[6]; + + uint32_t chipset1_dev_id; + uint32_t chipset2_dev_id; + + uint16_t processor_rev_id; + + uint8_t chipset1_rev_id; + uint8_t chipset2_rev_id; + + uint8_t reserved2[4]; + + uint8_t m_patch_data[F16H_MPB_MAX_SIZE-F16H_MPB_DATA_OFFSET]; + +}; + +static void apply_microcode_patch(const struct microcode *m) +{ + uint32_t new_patch_id; + msr_t msr; + + /* apply patch */ + msr.hi = (uint64_t)(uintptr_t)m >> 32; + msr.lo = (uintptr_t)m & 0xFFFFFFFF; + + wrmsr(MSR_PATCH_LOADER, msr); + + printk(BIOS_DEBUG, "microcode: patch id to apply = 0x%08x\n", + m->patch_id); + + /* patch authentication */ + msr = rdmsr(MSR_PATCH_LEVEL); + new_patch_id = msr.lo; + + printk(BIOS_DEBUG, "microcode: updated to patch id = 0x%08x %s\n", + new_patch_id, + (new_patch_id == m->patch_id) ? "success" : "fail"); +}
struct id_mapping { uint32_t orig_id; @@ -12,20 +98,16 @@ { static const struct id_mapping id_mapping_table[] = { /* Family 16h */ - - /* TODO This equivalent processor revisions ID needs verification */ - { 0x730f01, 0x7301 }, - - /* Array terminator */ - { 0xffffff, 0x0000 }, + /* TODO This equivalent processor revisions ID needs verification */ + { 0x810f81, 0x8181 }, + { 0x820f01, 0x8201 }, + //{ 0x810f81, 0x8180 }, };
- u32 new_id; - int i; + u32 new_id = 0; + u8 i;
- new_id = 0; - - for (i = 0; id_mapping_table[i].orig_id != 0xffffff; i++) { + for (i = 0; i < ARRAY_SIZE(id_mapping_table); i++) { if (id_mapping_table[i].orig_id == orig_id) { new_id = id_mapping_table[i].new_id; break; @@ -35,9 +117,42 @@ return new_id; }
-void update_microcode(u32 cpu_deviceid) +static void amd_update_microcode(const void *ucode, size_t ucode_len, + uint16_t equivalent_processor_rev_id) { - u16 equivalent_processor_rev_id = - get_equivalent_processor_rev_id(cpu_deviceid); - amd_update_microcode_from_cbfs(equivalent_processor_rev_id); + const struct microcode *m; + const uint8_t *c = ucode; + + m = (struct microcode *)c; + + if (m->processor_rev_id == equivalent_processor_rev_id) + apply_microcode_patch(m); +} + +void amd_update_microcode_from_cbfs() +{ + const void *ucode; + size_t ucode_len; + uint16_t equivalent_processor_rev_id = get_equivalent_processor_rev_id(cpuid_eax(1));; + + if (equivalent_processor_rev_id == 0) { + printk(BIOS_DEBUG, "microcode: rev id not found. Skipping microcode patch!\n"); + return; + } + ucode = cbfs_boot_map_with_leak("cpu_microcode_blob.bin", + CBFS_TYPE_MICROCODE, + &ucode_len); + if (!ucode) { + printk(BIOS_DEBUG, "cpu_microcode_blob.bin not found. Skipping updates.\n"); + return; + } + + if (ucode_len > F16H_MPB_MAX_SIZE || + ucode_len < F16H_MPB_DATA_OFFSET) { + printk(BIOS_DEBUG, "microcode file invalid. Skipping " + "updates.\n"); + return; + } + + amd_update_microcode(ucode, ucode_len, equivalent_processor_rev_id); } diff --git a/src/include/cpu/amd/microcode.h b/src/include/cpu/amd/microcode.h index 29b5576..800661b 100644 --- a/src/include/cpu/amd/microcode.h +++ b/src/include/cpu/amd/microcode.h @@ -1,7 +1,6 @@ #ifndef CPU_AMD_MICROCODE_H #define CPU_AMD_MICROCODE_H
-void update_microcode(u32 cpu_deviceid); -void amd_update_microcode_from_cbfs(u16 equivalent_processor_rev_id); +void amd_update_microcode_from_cbfs(void);
#endif /* CPU_AMD_MICROCODE_H */