Attention is currently required from: Eugene Myers.

Marc Jones would like Eugene Myers to review this change.

View Change

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
---
M src/cpu/x86/smm/smm_module_loader.c
M src/security/intel/stm/StmPlatformSmm.c
2 files changed, 8 insertions(+), 18 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/50312/1
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);

To view, visit change 50312. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: 4.11_branch
Gerrit-Change-Id: Ie62e2bdccd2d09084cc39a0f2fe32df236c08cd6
Gerrit-Change-Number: 50312
Gerrit-PatchSet: 1
Gerrit-Owner: Marc Jones <marc@marcjonesconsulting.com>
Gerrit-Reviewer: Eugene Myers <cedarhouse@comcast.net>
Gerrit-Attention: Eugene Myers <cedarhouse@comcast.net>
Gerrit-MessageType: newchange