From c9871505f115f0e5722bf23b13f4390c8d76d0da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ky=C3=B6sti=20M=C3=A4lkki?= Date: Tue, 3 Sep 2019 07:03:39 +0300 Subject: [PATCH] intel/fsp2_0: Move temporary RAM to .bss with FSP_USES_CB_STACK MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The documentation for StackBase and StackSize in FSPM_ARCH_UPD is confusing. Previously the region was shared for heap and stack, starting with FSP2.1 only for heap (or 'temporary RAM') for HOBs. Moving the allocation outside DCACHE_BSP_STACK_SIZE allows use of stack guards and reduces amount of reserved CAR for bootblock and verstage, as the new allocation in .bss is only taken in romstage. BUG=b:140268415 Change-Id: I4cffcc73a89cb97ab7759dd373196ce9753a6307 Signed-off-by: Kyösti Mälkki Reviewed-on: https://review.coreboot.org/c/coreboot/+/35233 Tested-by: build bot (Jenkins) Reviewed-by: Furquan Shaikh --- src/drivers/intel/fsp2_0/Kconfig | 5 +++++ src/drivers/intel/fsp2_0/memory_init.c | 27 ++++++++++++++------------ 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index 3fb39d5076..541ec47b46 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -152,6 +152,11 @@ config FSP_USES_CB_STACK 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 0797c2e3fc..9789c9653a 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -34,6 +34,8 @@ #include #include +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), @@ -161,23 +163,13 @@ static enum cb_err check_region_overlap(const struct memranges *ranges, return CB_SUCCESS; } + static enum cb_err setup_fsp_stack_frame(FSPM_ARCH_UPD *arch_upd, const struct memranges *memmap) { uintptr_t stack_begin; 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 @@ -197,8 +189,19 @@ static enum cb_err fsp_fill_common_arch_params(FSPM_ARCH_UPD *arch_upd, bool s3wake, uint32_t fsp_version, const struct memranges *memmap) { - if (setup_fsp_stack_frame(arch_upd, 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)) { return CB_ERR; + } fsp_fill_mrc_cache(arch_upd, fsp_version);