Stefan Reinauer has submitted this change. ( https://review.coreboot.org/c/coreboot/+/55628?usp=email )
Change subject: cpu/x86/: Centralize MSEG location calculation ......................................................................
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 edmyers@tycho.nsa.gov Reviewed-on: https://review.coreboot.org/c/coreboot/+/55628 Reviewed-by: Stefan Reinauer stefan.reinauer@coreboot.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/cpu/x86/mp_init.c M src/cpu/x86/smm/smm_module_loader.c M src/cpu/x86/smm/smm_module_loaderv2.c M src/cpu/x86/smm/tseg_region.c M src/include/cpu/x86/smm.h 5 files changed, 20 insertions(+), 8 deletions(-)
Approvals: Stefan Reinauer: Looks good to me, approved build bot (Jenkins): Verified
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 57b8865..313fb34 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -756,9 +756,9 @@ 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 19acea6..dd00b7f 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -368,7 +368,7 @@ 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 04654f7..8d01b3c 100644 --- a/src/cpu/x86/smm/smm_module_loaderv2.c +++ b/src/cpu/x86/smm/smm_module_loaderv2.c @@ -586,8 +586,8 @@
/* 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 a8b8bb7..747dacd 100644 --- a/src/cpu/x86/smm/tseg_region.c +++ b/src/cpu/x86/smm/tseg_region.c @@ -17,6 +17,7 @@ #include <cpu/x86/smm.h> #include <stage_cache.h> #include <types.h> +#include <inttypes.h>
/* * 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 @@ 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 @@ 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 d8f6d0e..b449296 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -168,6 +168,8 @@ enum { /* SMM handler area. */ SMM_SUBREGION_HANDLER, + /* MSEG (STM). */ + SMM_SUBREGION_MSEG, /* SMM cache region. */ SMM_SUBREGION_CACHE, /* Chipset specific area. */