From e7e2bd2a593e95fb8fe7b4020a6822ce9cd8a1ff Mon Sep 17 00:00:00 2001 From: Eugene Myers Date: Wed, 12 May 2021 07:55:28 -0400 Subject: [PATCH] cpu/x86/: Centralize MSEG location calculation This patch centralizes the MSEG location calculation. In the current implementation, the calculation happens in smm_module_loader and mp_init. When smm_module_loaderv2 was added, this calculation became broken as the original calculation made assumptions based on perm_smbase. The calculation is now located in smm_subregion (tseg_region.c), as the MSEG is located within the TSEG (or SMM); These patches have been tested on a Purism librem-l1um server. Change-Id: Ic17e1a505401c3b2a218826dffae6fe12a5c15c6 Signed-off-by: Eugene Myers Reviewed-on: https://review.coreboot.org/c/coreboot/+/55628 Reviewed-by: Stefan Reinauer Tested-by: build bot (Jenkins) --- src/cpu/x86/mp_init.c | 4 ++-- src/cpu/x86/smm/smm_module_loader.c | 2 +- src/cpu/x86/smm/smm_module_loaderv2.c | 4 ++-- src/cpu/x86/smm/tseg_region.c | 16 +++++++++++++--- src/include/cpu/x86/smm.h | 2 ++ 5 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 57b88653c5..313fb34113 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -756,9 +756,9 @@ static void asmlinkage smm_do_relocation(void *arg) if (CONFIG(STM)) { if (is_smm_enabled()) { uintptr_t mseg; + size_t mseg_size; - mseg = mp_state.perm_smbase + - (mp_state.perm_smsize - CONFIG_MSEG_SIZE); + smm_subregion(SMM_SUBREGION_MSEG, &mseg, &mseg_size); stm_setup(mseg, p->cpu, runtime->num_cpus, perm_smbase, diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index 19acea60d6..dd00b7f05c 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -368,7 +368,7 @@ int smm_load_module(void *smram, size_t size, struct smm_loader_params *params) base += size; if (CONFIG(STM)) - base -= CONFIG_MSEG_SIZE + CONFIG_BIOS_RESOURCE_LIST_SIZE; + base -= CONFIG_BIOS_RESOURCE_LIST_SIZE; params->stack_top = base; diff --git a/src/cpu/x86/smm/smm_module_loaderv2.c b/src/cpu/x86/smm/smm_module_loaderv2.c index 04654f77c9..8d01b3c0ee 100644 --- a/src/cpu/x86/smm/smm_module_loaderv2.c +++ b/src/cpu/x86/smm/smm_module_loaderv2.c @@ -586,8 +586,8 @@ int smm_load_module(void *smram, size_t size, struct smm_loader_params *params) /* MSEG starts at the top of SMRAM and works down */ if (CONFIG(STM)) { - base -= CONFIG_MSEG_SIZE + CONFIG_BIOS_RESOURCE_LIST_SIZE; - total_size += CONFIG_MSEG_SIZE + CONFIG_BIOS_RESOURCE_LIST_SIZE; + base -= CONFIG_BIOS_RESOURCE_LIST_SIZE; + total_size += CONFIG_BIOS_RESOURCE_LIST_SIZE; } /* FXSAVE goes below MSEG */ diff --git a/src/cpu/x86/smm/tseg_region.c b/src/cpu/x86/smm/tseg_region.c index a8b8bb7b9a..747dacdca6 100644 --- a/src/cpu/x86/smm/tseg_region.c +++ b/src/cpu/x86/smm/tseg_region.c @@ -17,6 +17,7 @@ #include #include #include +#include /* * Subregions within SMM @@ -25,6 +26,8 @@ * +-------------------------+ * | External Stage Cache | SMM_RESERVED_SIZE * +-------------------------+ + * | STM | MSEG_SIZE + * +-------------------------+ * | code and data | * | (TSEG) | * +-------------------------+ TSEG @@ -35,17 +38,24 @@ int smm_subregion(int sub, uintptr_t *start, size_t *size) size_t sub_size; const size_t ied_size = CONFIG_IED_REGION_SIZE; const size_t cache_size = CONFIG_SMM_RESERVED_SIZE; + const size_t mseg_size = CONFIG_MSEG_SIZE; smm_region(&sub_base, &sub_size); ASSERT(IS_ALIGNED(sub_base, sub_size)); - ASSERT(sub_size > (cache_size + ied_size)); + ASSERT(sub_size > (cache_size + ied_size + mseg_size)); switch (sub) { case SMM_SUBREGION_HANDLER: /* Handler starts at the base of TSEG. */ sub_size -= ied_size; sub_size -= cache_size; + sub_size -= mseg_size; + break; + case SMM_SUBREGION_MSEG: + /* MSEG follows the SMM HANDLER subregion */ + sub_base += sub_size - (ied_size + cache_size + mseg_size); + sub_size = mseg_size; break; case SMM_SUBREGION_CACHE: /* External cache is in the middle of TSEG. */ @@ -88,11 +98,11 @@ void smm_list_regions(void) return; printk(BIOS_DEBUG, "SMM Memory Map\n"); - printk(BIOS_DEBUG, "SMRAM : 0x%zx 0x%zx\n", base, size); + printk(BIOS_DEBUG, "SMRAM : 0x%" PRIxPTR " 0x%zx\n", base, size); for (i = 0; i < SMM_SUBREGION_NUM; i++) { if (smm_subregion(i, &base, &size)) continue; - printk(BIOS_DEBUG, " Subregion %d: 0x%zx 0x%zx\n", i, base, size); + printk(BIOS_DEBUG, " Subregion %d: 0x%" PRIxPTR " 0x%zx\n", i, base, size); } } diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index d8f6d0e627..b4492963b3 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -168,6 +168,8 @@ void smm_region(uintptr_t *start, size_t *size); enum { /* SMM handler area. */ SMM_SUBREGION_HANDLER, + /* MSEG (STM). */ + SMM_SUBREGION_MSEG, /* SMM cache region. */ SMM_SUBREGION_CACHE, /* Chipset specific area. */