Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47072 )
Change subject: cpu/x86/mp_init.c: Allocate the smm relocation stack on the heap ......................................................................
cpu/x86/mp_init.c: Allocate the smm relocation stack on the heap
The smm_module_loader v2 places stack_top at DEFAULT_SMBASE + per_cpu_stack_size. The smm_stub assembly will put the stack for a CPU at stack_top - apicid * stack_size. So for loader v2 this means that areas below DEFAULT_SMBASE get used as a CPU stack which would cause lower memory corruption on S3 resume.
The solution is to use ramstage heap as a temporary place for the smm relocation stacks. This completely avoids using lower memory for CPU stacks.
On V1 all the CPU stacks would be properly allocated inside the DEFAULT_SMBASE but this change won't hurt either.
Tested: works fine on OCP/Deltalake.
Change-Id: I125d3aa6526c923e154c41e2d586557c7d8f56c3 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- 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 3 files changed, 32 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/47072/1
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 4870529..3deab6d 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -3,6 +3,7 @@ #include <console/console.h> #include <stddef.h> #include <stdint.h> +#include <stdlib.h> #include <string.h> #include <rmodule.h> #include <arch/cpu.h> @@ -764,19 +765,21 @@ runtime->apic_id_to_cpu[i] = cpu_get_apic_id(i); }
+void *smm_stacks; + static int install_relocation_handler(int num_cpus, size_t save_state_size) { - int cpus = num_cpus; -#if CONFIG(X86_SMM_LOADER_VERSION2) - /* Default SMRAM size is not big enough to concurrently - * handle relocation for more than ~32 CPU threads - * therefore, relocate 1 by 1. */ - cpus = 1; -#endif + smm_stacks = memalign(16, num_cpus * CONFIG_SMM_STUB_STACK_SIZE); + + if (smm_stacks == NULL) { + printk(BIOS_ERR, "%s: failed to allocate stacks.\n"); + return -1; + }
struct smm_loader_params smm_params = { + .stack_top = (void *)((uintptr_t)smm_stacks + num_cpus * CONFIG_SMM_STUB_STACK_SIZE), .per_cpu_stack_size = CONFIG_SMM_STUB_STACK_SIZE, - .num_concurrent_stacks = cpus, + .num_concurrent_stacks = num_cpus, .per_cpu_save_state_size = save_state_size, .num_concurrent_save_states = 1, .handler = smm_do_relocation, @@ -1104,6 +1107,8 @@
restore_default_smm_area(default_smm_area);
+ free(smm_stacks); + /* Signal callback on success if it's provided. */ if (ret == 0 && mp_state.ops.post_mp_init != NULL) mp_state.ops.post_mp_init(); diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index 876fde6..3c0ed7e 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -244,8 +244,16 @@ size += params->per_cpu_save_state_size; }
- /* Place the stacks in the lower half of SMRAM. */ - stacks_top = smm_stub_place_stacks(base, size, params); + /* Use the smbase as a proxy to know if we are installing the stub for relocation + * or for permanent handling. In case of relocation the SMM relocation stack will + * have been allocated on the ramstage heap and programmed in the smm loader params. + */ + if (smbase == (void *)SMM_DEFAULT_BASE) + stacks_top = params->stack_top; + else + /* Place the stacks in the lower half of SMRAM. */ + stacks_top = smm_stub_place_stacks(base, size, params); + if (stacks_top == NULL) return -1;
diff --git a/src/cpu/x86/smm/smm_module_loaderv2.c b/src/cpu/x86/smm/smm_module_loaderv2.c index 7a72c10..54e46d4 100644 --- a/src/cpu/x86/smm/smm_module_loaderv2.c +++ b/src/cpu/x86/smm/smm_module_loaderv2.c @@ -414,8 +414,15 @@ * of SMRAM which is TSEG base */ const total_stack_size = params->num_concurrent_stacks * params->per_cpu_stack_size; - stacks_top = smm_stub_place_stacks((char *)params->smram_start, total_stack_size, - params); + /* Use the smbase as a proxy to know if we are installing the stub for relocation + * or for permanent handling. In case of relocation the SMM relocation stack will + * have been allocated on the ramstage heap and programmed in the smm loader params. + */ + if (smbase == (void *)SMM_DEFAULT_BASE) + stacks_top = params->stack_top; + else + stacks_top = smm_stub_place_stacks((char *)params->smram_start, size, params); + if (stacks_top == NULL) { printk(BIOS_ERR, "%s: not enough space for stacks\n", __func__); printk(BIOS_ERR, "%s: ....need -> %p : available -> %zx\n", __func__,