[coreboot-gerrit] Change in coreboot[master]: [UNTESTED] intel/stm: Place resource list right below MSEG

[Nico Huber (Code Review)]

Hello Kyösti Mälkki, Aaron Durbin, Arthur Heymans, [e-mail],

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/38765

to review the following change.

Change subject: [UNTESTED] intel/stm: Place resource list right below MSEG
......................................................................

[UNTESTED] intel/stm: Place resource list right below MSEG

Can we do this? what do I miss?

Change-Id: I9c5970433a3851b5c9662c8870efaf2943498d27
Signed-off-by: Nico Huber 
---
M src/cpu/x86/smm/smm_module_loader.c
M src/security/intel/stm/StmPlatformSmm.c
2 files changed, 8 insertions(+), 19 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/38765/1

diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c
index 81020a4..2ef11340 100644
--- a/src/cpu/x86/smm/smm_module_loader.c
+++ b/src/cpu/x86/smm/smm_module_loader.c
@@ -315,13 +315,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      |
@@ -362,11 +362,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;

@@ -396,11 +395,6 @@
    /* Does the required amount of memory exceed the SMRAM region size? */
    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 d7064b0..eb69301 100644
--- a/src/security/intel/stm/StmPlatformSmm.c
+++ b/src/security/intel/stm/StmPlatformSmm.c
@@ -166,12 +166,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);