Felix Held submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved
amd/00730F01: Clean the Microcode updating

According to the comments of
https://review.coreboot.org/c/coreboot/+/41719
, which is about Microcode patch for amd/picasso.
Change the code with the same way.

The changes include:
1. combine the microcode_xxx.c and update_microcode.c
into one source.
2. Redefine the microcode updating function to eliminate
the parameter. Get the revision ID in the black box.
Reduce the depth of function calls.
3. Get the revision ID by bitwise calculation instead of
lookup table.
4. Reduce the confusing type casts.
5. Squash some lines.

We do not change the way it used to be. The code assume
only one microcode is integrated in CBFS. If needed in future,
41719 is the example of integrating multiple binaries.

And, 41719 depends on the definition in this patch.

Change-Id: I8b0da99db0d3189058f75e199f05492c4e6c5881
Signed-off-by: Zheng Bao <zheng.bao@amd.com>
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/42094
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
---
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, 118 insertions(+), 160 deletions(-)

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..11e2531 100644
--- a/src/cpu/amd/pi/00730F01/update_microcode.c
+++ b/src/cpu/amd/pi/00730F01/update_microcode.c
@@ -2,42 +2,131 @@

#include <stdint.h>
#include <cpu/amd/microcode.h>
+#include <commonlib/helpers.h>
+#include <console/console.h>
+#include <cpu/x86/msr.h>
+#include <cpu/amd/msr.h>
+#include <cbfs.h>

-struct id_mapping {
- uint32_t orig_id;
- uint16_t new_id;
-};
+/*
+ * Values and header structure from:
+ * BKDG for AMD Family 16h Models 30h-3Fh Processors
+ * 52740 Rev 3.06 - March 18, 2016
+ */

-static u16 get_equivalent_processor_rev_id(u32 orig_id)
+#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];
+} __packed;
+
+static void apply_microcode_patch(const struct microcode *m)
{
- static const struct id_mapping id_mapping_table[] = {
- /* Family 16h */
+ uint32_t new_patch_id;
+ msr_t msr;

- /* TODO This equivalent processor revisions ID needs verification */
- { 0x730f01, 0x7301 },
+ msr.hi = (uint64_t)(uintptr_t)m >> 32;
+ msr.lo = (uintptr_t)m & 0xffffffff;

- /* Array terminator */
- { 0xffffff, 0x0000 },
- };
+ wrmsr(MSR_PATCH_LOADER, msr);

- u32 new_id;
- int i;
+ printk(BIOS_DEBUG, "microcode: patch id to apply = 0x%08x\n",
+ m->patch_id);

- new_id = 0;
+ msr = rdmsr(MSR_PATCH_LEVEL);
+ new_patch_id = msr.lo;

- for (i = 0; id_mapping_table[i].orig_id != 0xffffff; i++) {
- if (id_mapping_table[i].orig_id == orig_id) {
- new_id = id_mapping_table[i].new_id;
- break;
- }
- }
-
- return new_id;
+ if (new_patch_id == m->patch_id)
+ printk(BIOS_INFO, "microcode: being updated to patch id = 0x%08x succeeded\n",
+ new_patch_id);
+ else
+ printk(BIOS_ERR, "microcode: being updated to patch id = 0x%08x failed\n",
+ new_patch_id);
}

-void update_microcode(u32 cpu_deviceid)
+static uint16_t get_equivalent_processor_rev_id(void)
{
- u16 equivalent_processor_rev_id =
- get_equivalent_processor_rev_id(cpu_deviceid);
- amd_update_microcode_from_cbfs(equivalent_processor_rev_id);
+ uint32_t cpuid_family = cpuid_eax(1);
+
+ return (uint16_t)((cpuid_family & 0xff0000) >> 8 | (cpuid_family & 0xff));
+}
+
+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;
+
+ /* Assume cpu_microcode_blob only contains one microcode. */
+ if (m->processor_rev_id == equivalent_processor_rev_id)
+ apply_microcode_patch(m);
+}
+
+void amd_update_microcode_from_cbfs(void)
+{
+ const void *ucode;
+ size_t ucode_len;
+ uint16_t equivalent_processor_rev_id = get_equivalent_processor_rev_id();
+
+ ucode = cbfs_boot_map_with_leak("cpu_microcode_blob.bin",
+ CBFS_TYPE_MICROCODE, &ucode_len);
+ if (!ucode) {
+ printk(BIOS_WARNING, "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 */

To view, visit change 42094. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8b0da99db0d3189058f75e199f05492c4e6c5881
Gerrit-Change-Number: 42094
Gerrit-PatchSet: 11
Gerrit-Owner: Bao Zheng <fishbaozi@gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: Zheng Bao <zheng.bao@amd.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-MessageType: merged