Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/50312 )
Change subject: intel/stm: Place resource list right below MSEG ......................................................................
intel/stm: Place resource list right below MSEG
Suggested by Nico Huber in CB:38765.
This placement makes the address calculation simpler and makes its location indepedent of the number of CPUs.
As part of the change in the BIOS resource list address calculation, the `size` variable was factored out of the conditional in line 361, thus eliminating the else.
Original-Change-Id: I9ee2747474df02b0306530048bdec75e95413b5d Original-Signed-off-by: Eugene D Myers cedarhouse@comcast.net Original-Reviewed-on: https://review.coreboot.org/c/coreboot/+/40437 Original-Reviewed-by: Nico Huber nico.h@gmx.de Original-Reviewed-by: Angel Pons th3fanbus@gmail.com Original-Tested-by: build bot (Jenkins) no-reply@coreboot.org
(cherry picked from commit 076605bc92730553e9adae543713f0d356a94709) Signed-off-by: Marc Jones marcjones@sysproconsulting.com
Change-Id: Ie62e2bdccd2d09084cc39a0f2fe32df236c08cd6 Reviewed-on: https://review.coreboot.org/c/coreboot/+/50312 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Stefan Reinauer stefan.reinauer@coreboot.org Reviewed-by: Jay Talbott JayTalbott@sysproconsulting.com Reviewed-by: Eugene Myers cedarhouse1@comcast.net Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net --- M src/cpu/x86/smm/smm_module_loader.c M src/security/intel/stm/StmPlatformSmm.c 2 files changed, 8 insertions(+), 18 deletions(-)
Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve Jay Talbott: Looks good to me, but someone else must approve Eugene Myers: Looks good to me, but someone else must approve
diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index 3ed20b7..19acea6 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -318,13 +318,13 @@ /* The SMM module is placed within the provided region in the following * manner: * +-----------------+ <- smram + size - * | stacks | - * +-----------------+ <- smram + size - total_stack_size - * | fxsave area | - * +-----------------+ <- smram + size - total_stack_size - fxsave_size * | BIOS resource | * | list (STM) | - * +-----------------+ <- .. - CONFIG_BIOS_RESOURCE_LIST_SIZE + * +-----------------+ <- smram + size - CONFIG_BIOS_RESOURCE_LIST_SIZE + * | stacks | + * +-----------------+ <- .. - total_stack_size + * | fxsave area | + * +-----------------+ <- .. - total_stack_size - fxsave_size * | ... | * +-----------------+ <- smram + handler_size + SMM_DEFAULT_SIZE * | handler | @@ -365,11 +365,10 @@
/* Stacks start at the top of the region. */ base = smram; + base += size;
if (CONFIG(STM)) - base += size - CONFIG_MSEG_SIZE; // take out the mseg - else - base += size; + base -= CONFIG_MSEG_SIZE + CONFIG_BIOS_RESOURCE_LIST_SIZE;
params->stack_top = base;
@@ -400,10 +399,6 @@ total_size = total_stack_size + handler_size; total_size += fxsave_size + SMM_DEFAULT_SIZE;
- // account for the bios resource list - if (CONFIG(STM)) - total_size += CONFIG_BIOS_RESOURCE_LIST_SIZE; - if (total_size > size) return -1;
diff --git a/src/security/intel/stm/StmPlatformSmm.c b/src/security/intel/stm/StmPlatformSmm.c index 45db0e0..248ccc0 100644 --- a/src/security/intel/stm/StmPlatformSmm.c +++ b/src/security/intel/stm/StmPlatformSmm.c @@ -177,12 +177,7 @@
// need to create the BIOS resource list once // first calculate the location in SMRAM - addr_calc = (mseg - (CONFIG_SMM_MODULE_STACK_SIZE * num_cpus)); - - if (CONFIG(SSE)) - addr_calc -= FXSAVE_SIZE * num_cpus; - - addr_calc -= CONFIG_BIOS_RESOURCE_LIST_SIZE; + addr_calc = mseg - CONFIG_BIOS_RESOURCE_LIST_SIZE; stm_resource_heap = (uint8_t *) addr_calc; printk(BIOS_DEBUG, "STM: stm_resource_heap located at %p\n", stm_resource_heap);