Hello Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Nathaniel L Desimone, Subrata Banik, Marshall Dawson, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/35324
to review the following change.
Change subject: Revert "intel/fsp2_0: Move temporary RAM to .bss with FSP_USES_CB_STACK" ......................................................................
Revert "intel/fsp2_0: Move temporary RAM to .bss with FSP_USES_CB_STACK"
This reverts commit c9871505f115f0e5722bf23b13f4390c8d76d0da.
Reason for revert: <INSERT REASONING HERE>
Change-Id: I46889a3860a0704f9011c8227005207dcb740d0e --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/memory_init.c 2 files changed, 12 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35324/1
diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index 541ec47..3fb39d5 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -152,11 +152,6 @@ without reinitializing stack pointer. This feature is supported Icelake onwards.
-config FSP_TEMP_RAM_SIZE - hex - default 0x10000 - depends on FSP_USES_CB_STACK - config VERIFY_HOBS bool "Verify the FSP hand-off-blocks" default n diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index 9789c96..0797c2e 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -34,8 +34,6 @@ #include <fsp/memory_init.h> #include <types.h>
-static uint8_t temp_ram[CONFIG_FSP_TEMP_RAM_SIZE] __aligned(sizeof(uint64_t)); - /* TPM MRC hash functionality depends on vboot starting before memory init. */ _Static_assert(!CONFIG(FSP2_0_USES_TPM_MRC_HASH) || CONFIG(VBOOT_STARTS_IN_BOOTBLOCK), @@ -163,7 +161,6 @@
return CB_SUCCESS; } - static enum cb_err setup_fsp_stack_frame(FSPM_ARCH_UPD *arch_upd, const struct memranges *memmap) { @@ -171,6 +168,17 @@ uintptr_t stack_end;
/* + * FSP 2.1 version would use same stack as coreboot instead of + * setting up seprate stack frame. FSP 2.1 would not relocate stack + * top and does not reinitialize stack pointer. + */ + if (CONFIG(FSP_USES_CB_STACK)) { + arch_upd->StackBase = (void *)_car_stack_start; + arch_upd->StackSize = CONFIG_DCACHE_BSP_STACK_SIZE; + return CB_SUCCESS; + } + + /* * FSPM_UPD passed here is populated with default values * provided by the blob itself. We let FSPM use top of CAR * region of the size it requests. @@ -189,19 +197,8 @@ bool s3wake, uint32_t fsp_version, const struct memranges *memmap) { - /* - * FSP 2.1 version would use same stack as coreboot instead of - * setting up separate stack frame. FSP 2.1 would not relocate stack - * top and does not reinitialize stack pointer. The parameters passed - * as StackBase and StackSize are actually for temporary RAM and HOBs - * and are not related to FSP stack at all. - */ - if (CONFIG(FSP_USES_CB_STACK)) { - arch_upd->StackBase = temp_ram; - arch_upd->StackSize = sizeof(temp_ram); - } else if (setup_fsp_stack_frame(arch_upd, memmap)) { + if (setup_fsp_stack_frame(arch_upd, memmap)) return CB_ERR; - }
fsp_fill_mrc_cache(arch_upd, fsp_version);