Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/50767 )
Change subject: cpu/x86/smm_stub.S: Drop smm_runtime from the stub ......................................................................
cpu/x86/smm_stub.S: Drop smm_runtime from the stub
The parameters that the permanent handler requires are pushed directly to the permanent handlers relocatable module params.
The paremeters that the relocation handler requires are not passed on via arguments but are copied inside the ramstage. This is ok as the relocation handler calls into ramstage.
Change-Id: Ice311d05e2eb0e95122312511d83683d7f0dee58 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 M src/cpu/x86/smm/smm_stub.S M src/include/cpu/x86/smm.h 5 files changed, 1 insertion(+), 50 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/50767/1
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 281dff6..f42c1a2 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -709,15 +709,12 @@ static void asmlinkage smm_do_relocation(void *arg) { const struct smm_module_params *p; - const struct smm_runtime *runtime; int cpu; - uintptr_t curr_smbase; + const uintptr_t curr_smbase = SMM_DEFAULT_BASE; uintptr_t perm_smbase;
p = arg; - runtime = p->runtime; cpu = p->cpu; - curr_smbase = runtime->smbase;
if (cpu >= CONFIG_MAX_CPUS) { printk(BIOS_CRIT, diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index 322373a..d0bb64a 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -252,11 +252,6 @@ stub_params->c_handler_arg = (uintptr_t)params->handler_arg; stub_params->fxsave_area = (uintptr_t)fxsave_area; stub_params->fxsave_area_size = FXSAVE_SIZE; - stub_params->runtime.smbase = (uintptr_t)smbase; - stub_params->runtime.smm_size = smm_size; - stub_params->runtime.save_state_size = params->per_cpu_save_state_size; - stub_params->runtime.num_cpus = params->num_concurrent_stacks; - stub_params->runtime.gnvs_ptr = (uintptr_t)acpi_get_gnvs();
/* Initialize the APIC id to CPU number table to be 1:1 */ for (i = 0; i < params->num_concurrent_stacks; i++) diff --git a/src/cpu/x86/smm/smm_module_loaderv2.c b/src/cpu/x86/smm/smm_module_loaderv2.c index 6320454..8eef2f6 100644 --- a/src/cpu/x86/smm/smm_module_loaderv2.c +++ b/src/cpu/x86/smm/smm_module_loaderv2.c @@ -342,7 +342,6 @@ size_t i; struct smm_stub_params *stub_params; struct rmodule smm_stub; - unsigned int total_size_all; base = smbase; size = smm_size;
@@ -432,11 +431,6 @@ stub_params->c_handler_arg = (uintptr_t)params->handler_arg; stub_params->fxsave_area = (uintptr_t)fxsave_area; stub_params->fxsave_area_size = FXSAVE_SIZE; - stub_params->runtime.smbase = (uintptr_t)smbase; - stub_params->runtime.smm_size = smm_size; - stub_params->runtime.save_state_size = params->per_cpu_save_state_size; - stub_params->runtime.num_cpus = params->num_concurrent_stacks; - stub_params->runtime.gnvs_ptr = (uintptr_t)acpi_get_gnvs();
printk(BIOS_DEBUG, "%s: stack_end = 0x%lx\n", __func__, stub_params->stack_top - total_stack_size); @@ -444,24 +438,10 @@ "%s: stack_top = 0x%x\n", __func__, stub_params->stack_top); printk(BIOS_DEBUG, "%s: stack_size = 0x%x\n", __func__, stub_params->stack_size); - printk(BIOS_DEBUG, "%s: runtime.smbase = 0x%x\n", - __func__, stub_params->runtime.smbase); printk(BIOS_DEBUG, "%s: runtime.start32_offset = 0x%x\n", __func__, stub_params->start32_offset); printk(BIOS_DEBUG, "%s: runtime.smm_size = 0x%zx\n", __func__, smm_size); - printk(BIOS_DEBUG, "%s: per_cpu_save_state_size = 0x%x\n", - __func__, stub_params->runtime.save_state_size); - printk(BIOS_DEBUG, "%s: num_cpus = 0x%x\n", __func__, - stub_params->runtime.num_cpus); - printk(BIOS_DEBUG, "%s: total_save_state_size = 0x%x\n", - __func__, (stub_params->runtime.save_state_size * - stub_params->runtime.num_cpus)); - total_size_all = stub_params->stack_size + - (stub_params->runtime.save_state_size * - stub_params->runtime.num_cpus); - printk(BIOS_DEBUG, "%s: total_size_all = 0x%x\n", __func__, - total_size_all);
/* Initialize the APIC id to CPU number table to be 1:1 */ for (i = 0; i < params->num_concurrent_stacks; i++) diff --git a/src/cpu/x86/smm/smm_stub.S b/src/cpu/x86/smm/smm_stub.S index f5b4a3b..37dd9ca 100644 --- a/src/cpu/x86/smm/smm_stub.S +++ b/src/cpu/x86/smm/smm_stub.S @@ -36,20 +36,6 @@ start32_offset: .long smm_trampoline32 - _start
-/* struct smm_runtime begins here. */ -smm_runtime: -smbase: -.long 0 -smm_size: -.long 0 -save_state_size: -.long 0 -num_cpus: -.long 0 -gnvs_ptr: -.long 0 -/* end struct smm_runtime */ - .data /* Provide fallback stack to use when a valid CPU number cannot be found. */ fallback_stack_bottom: @@ -203,7 +189,6 @@ */ #ifdef __x86_64__ push %rbx /* uintptr_t *canary */ - push $(smm_runtime) push %rcx /* size_t cpu */ push c_handler_arg /* void *arg */
@@ -230,11 +215,7 @@ wrmsr
#else - push $0x0 /* Padding */ - push $0x0 /* Padding */ - push $0x0 /* Padding */ push %ebx /* uintptr_t *canary */ - push $(smm_runtime) push %ecx /* size_t cpu */ push c_handler_arg /* void *arg */ push %esp /* smm_module_params *arg (allocated on stack). */ diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index 7098e7b..71227f4 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -65,7 +65,6 @@ struct smm_module_params { void *arg; size_t cpu; - const struct smm_runtime *runtime; /* A canary value that has been placed at the end of the stack. * If (uintptr_t)canary != *canary then a stack overflow has occurred. */ @@ -90,7 +89,6 @@ u8 apic_id_to_cpu[CONFIG_MAX_CPUS]; /* STM's 32bit entry into SMI handler */ u32 start32_offset; - struct smm_runtime runtime; } __packed;
/* smm_handler_t is called with arg of smm_module_params pointer. */
-
Arthur Heymans (Code Review)