Change in coreboot[master]: cpu/amd/pi/00730F01: Clean up microcode update from __PRE_RAM__
Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35586 )
Change subject: cpu/amd/pi/00730F01: Clean up microcode update from __PRE_RAM__ ......................................................................
cpu/amd/pi/00730F01: Clean up microcode update from __PRE_RAM__
It is only called in ramstage. Even if it was called in romstage, execution flow is such that BSP and AP CPUs should not be able to enter update_microcode() routine concurrently.
Also the Kconfig guarding the spin_lock() calls are not selected nor are the lock variables declared for these platforms.
Change-Id: I1c2e106f10e8420e942b3ed082c677c0db95557c Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/amd/pi/00730F01/Makefile.inc M src/cpu/amd/pi/00730F01/microcode_fam16h.c 2 files changed, 1 insertion(+), 28 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/35586/1
diff --git a/src/cpu/amd/pi/00730F01/Makefile.inc b/src/cpu/amd/pi/00730F01/Makefile.inc index 5e49869..c4a92cf 100644 --- a/src/cpu/amd/pi/00730F01/Makefile.inc +++ b/src/cpu/amd/pi/00730F01/Makefile.inc @@ -12,8 +12,6 @@ #
romstage-y += fixme.c -romstage-y += update_microcode.c -romstage-y += microcode_fam16h.c
ramstage-y += fixme.c ramstage-y += chip_name.c diff --git a/src/cpu/amd/pi/00730F01/microcode_fam16h.c b/src/cpu/amd/pi/00730F01/microcode_fam16h.c index 1251c38..1f1dbd9 100644 --- a/src/cpu/amd/pi/00730F01/microcode_fam16h.c +++ b/src/cpu/amd/pi/00730F01/microcode_fam16h.c @@ -16,8 +16,6 @@ #include <cpu/x86/msr.h> #include <cpu/amd/microcode.h> #include <cbfs.h> -#include <arch/io.h> -#include <smp/spinlock.h>
/* * Values and header structure from: @@ -122,22 +120,12 @@ "Skipping microcode patch!\n"); return; } -#ifdef __PRE_RAM__ -#if CONFIG(HAVE_ROMSTAGE_MICROCODE_CBFS_SPINLOCK) - spin_lock(romstage_microcode_cbfs_lock()); -#endif -#endif 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"); -#ifdef __PRE_RAM__ -#if CONFIG(HAVE_ROMSTAGE_MICROCODE_CBFS_SPINLOCK) - spin_unlock(romstage_microcode_cbfs_lock()); -#endif -#endif return; }
@@ -145,21 +133,8 @@ ucode_len < F16H_MPB_DATA_OFFSET) { printk(BIOS_DEBUG, "microcode file invalid. Skipping " "updates.\n"); -#ifdef __PRE_RAM__ -#if CONFIG(HAVE_ROMSTAGE_MICROCODE_CBFS_SPINLOCK) - spin_unlock(romstage_microcode_cbfs_lock()); -#endif -#endif return; }
- amd_update_microcode(ucode, ucode_len, - equivalent_processor_rev_id); - -#ifdef __PRE_RAM__ -#if CONFIG(HAVE_ROMSTAGE_MICROCODE_CBFS_SPINLOCK) - spin_unlock(romstage_microcode_cbfs_lock()); -#endif -#endif - + amd_update_microcode(ucode, ucode_len, equivalent_processor_rev_id); }
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35586 )
Change subject: cpu/amd/pi/00730F01: Clean up microcode update from __PRE_RAM__ ......................................................................
Patch Set 2: Code-Review+2
Right, it is useless to compile ucode update for romstage for now, because AGESA overwrites the ucode regardless of the patchlevel currently loaded on CPU.
Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35586 )
Change subject: cpu/amd/pi/00730F01: Clean up microcode update from __PRE_RAM__ ......................................................................
cpu/amd/pi/00730F01: Clean up microcode update from __PRE_RAM__
It is only called in ramstage. Even if it was called in romstage, execution flow is such that BSP and AP CPUs should not be able to enter update_microcode() routine concurrently.
Also the Kconfig guarding the spin_lock() calls are not selected nor are the lock variables declared for these platforms.
Change-Id: I1c2e106f10e8420e942b3ed082c677c0db95557c Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35586 Reviewed-by: Michał Żygowski michal.zygowski@3mdeb.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/cpu/amd/pi/00730F01/Makefile.inc M src/cpu/amd/pi/00730F01/microcode_fam16h.c 2 files changed, 1 insertion(+), 28 deletions(-)
Approvals: build bot (Jenkins): Verified Michał Żygowski: Looks good to me, approved
diff --git a/src/cpu/amd/pi/00730F01/Makefile.inc b/src/cpu/amd/pi/00730F01/Makefile.inc index 5e49869..c4a92cf 100644 --- a/src/cpu/amd/pi/00730F01/Makefile.inc +++ b/src/cpu/amd/pi/00730F01/Makefile.inc @@ -12,8 +12,6 @@ #
romstage-y += fixme.c -romstage-y += update_microcode.c -romstage-y += microcode_fam16h.c
ramstage-y += fixme.c ramstage-y += chip_name.c diff --git a/src/cpu/amd/pi/00730F01/microcode_fam16h.c b/src/cpu/amd/pi/00730F01/microcode_fam16h.c index 1251c38..1f1dbd9 100644 --- a/src/cpu/amd/pi/00730F01/microcode_fam16h.c +++ b/src/cpu/amd/pi/00730F01/microcode_fam16h.c @@ -16,8 +16,6 @@ #include <cpu/x86/msr.h> #include <cpu/amd/microcode.h> #include <cbfs.h> -#include <arch/io.h> -#include <smp/spinlock.h>
/* * Values and header structure from: @@ -122,22 +120,12 @@ "Skipping microcode patch!\n"); return; } -#ifdef __PRE_RAM__ -#if CONFIG(HAVE_ROMSTAGE_MICROCODE_CBFS_SPINLOCK) - spin_lock(romstage_microcode_cbfs_lock()); -#endif -#endif 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"); -#ifdef __PRE_RAM__ -#if CONFIG(HAVE_ROMSTAGE_MICROCODE_CBFS_SPINLOCK) - spin_unlock(romstage_microcode_cbfs_lock()); -#endif -#endif return; }
@@ -145,21 +133,8 @@ ucode_len < F16H_MPB_DATA_OFFSET) { printk(BIOS_DEBUG, "microcode file invalid. Skipping " "updates.\n"); -#ifdef __PRE_RAM__ -#if CONFIG(HAVE_ROMSTAGE_MICROCODE_CBFS_SPINLOCK) - spin_unlock(romstage_microcode_cbfs_lock()); -#endif -#endif return; }
- amd_update_microcode(ucode, ucode_len, - equivalent_processor_rev_id); - -#ifdef __PRE_RAM__ -#if CONFIG(HAVE_ROMSTAGE_MICROCODE_CBFS_SPINLOCK) - spin_unlock(romstage_microcode_cbfs_lock()); -#endif -#endif - + amd_update_microcode(ucode, ucode_len, equivalent_processor_rev_id); }
-
Kyösti Mälkki (Code Review) -
Michał Żygowski (Code Review)