Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58697 )
Change subject: cpu/x86/smm: Retype variables ......................................................................
cpu/x86/smm: Retype variables
Change-Id: I85750282ab274f52bc176a1ac151ef2f9e0dd15d Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/58697 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: HAOUAS Elyes ehaouas@noos.fr --- M src/cpu/x86/mp_init.c M src/cpu/x86/smm/smm_module_loader.c M src/include/cpu/x86/smm.h 3 files changed, 44 insertions(+), 48 deletions(-)
Approvals: build bot (Jenkins): Verified HAOUAS Elyes: Looks good to me, approved
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index acbe579..888c97f 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -777,7 +777,7 @@ if (mp_state.ops.adjust_smm_params != NULL) mp_state.ops.adjust_smm_params(&smm_params, 0);
- if (smm_setup_relocation_handler((void *)perm_smbase, &smm_params)) { + if (smm_setup_relocation_handler(perm_smbase, &smm_params)) { printk(BIOS_ERR, "%s: smm setup failed\n", __func__); return CB_ERR; } @@ -813,7 +813,7 @@
printk(BIOS_DEBUG, "Installing permanent SMM handler to 0x%08lx\n", smbase);
- if (smm_load_module((void *)smbase, smsize, &smm_params)) + if (smm_load_module(smbase, smsize, &smm_params)) return CB_ERR;
adjust_smm_apic_id_map(&smm_params); diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index 101b7c5..ba179ca 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -243,10 +243,10 @@ * Place stacks in base -> base + size region, but ensure the stacks don't * overlap the staggered entry points. */ -static void *smm_stub_place_stacks(char *base, struct smm_loader_params *params) +static uintptr_t smm_stub_place_stacks(const uintptr_t base, struct smm_loader_params *params) { size_t total_stack_size; - char *stacks_top; + uintptr_t stacks_top;
/* If stack space is requested assume the space lives in the lower * half of SMRAM. */ @@ -258,11 +258,11 @@
/* There has to be at least one stack user. */ if (params->num_concurrent_stacks < 1) - return NULL; + return 0;
/* Stacks extend down to SMBASE */ - stacks_top = &base[total_stack_size]; - printk(BIOS_DEBUG, "%s: exit, stack_top %p\n", __func__, stacks_top); + stacks_top = base + total_stack_size; + printk(BIOS_DEBUG, "%s: exit, stack_top %lx\n", __func__, stacks_top);
return stacks_top; } @@ -312,17 +312,17 @@ * by the permanent handler can be used during relocation. This is done via the * smram_start argument. */ -static int smm_module_setup_stub(void *const smbase, const size_t smm_size, +static int smm_module_setup_stub(const uintptr_t smbase, const size_t smm_size, struct smm_loader_params *params, void *const fxsave_area, - void *const smram_start) + const uintptr_t smram_start) { size_t total_save_state_size; size_t smm_stub_size; - char *smm_stub_loc; - void *stacks_top; + uintptr_t smm_stub_loc; + uintptr_t stacks_top; size_t size; - char *base; + uintptr_t base; size_t i; struct smm_stub_params *stub_params; struct rmodule smm_stub; @@ -368,11 +368,10 @@ return -1; }
- smm_stub_loc = NULL; smm_stub_size = rmodule_memory_size(&smm_stub);
/* Put the stub at the main entry point */ - smm_stub_loc = &base[SMM_ENTRY_OFFSET]; + smm_stub_loc = base + SMM_ENTRY_OFFSET;
/* Stub is too big to fit. */ if (smm_stub_size > (size - SMM_ENTRY_OFFSET)) { @@ -385,18 +384,18 @@ * of SMRAM which is TSEG base */ stacks_top = smm_stub_place_stacks(smram_start, params); - if (stacks_top == NULL) { + if (stacks_top == 0) { printk(BIOS_ERR, "%s: error assigning stacks\n", __func__); return -1; } params->stack_top = stacks_top; /* Load the stub. */ - if (rmodule_load(smm_stub_loc, &smm_stub)) { + if (rmodule_load((void *)smm_stub_loc, &smm_stub)) { printk(BIOS_ERR, "%s: load module failed\n", __func__); return -1; }
- if (!smm_stub_place_staggered_entry_points(base, params, &smm_stub)) { + if (!smm_stub_place_staggered_entry_points((void *)base, params, &smm_stub)) { printk(BIOS_ERR, "%s: staggered entry points failed\n", __func__); return -1; } @@ -429,7 +428,7 @@ /* Allow the initiator to manipulate SMM stub parameters. */ params->stub_params = stub_params;
- printk(BIOS_DEBUG, "SMM Module: stub loaded at %p. Will call %p\n", + printk(BIOS_DEBUG, "SMM Module: stub loaded at %lx. Will call %p\n", smm_stub_loc, params->handler); return 0; } @@ -440,9 +439,9 @@ * assumption is that the stub will be entered from the default SMRAM * location: 0x30000 -> 0x40000. */ -int smm_setup_relocation_handler(void * const perm_smram, struct smm_loader_params *params) +int smm_setup_relocation_handler(const uintptr_t perm_smram, struct smm_loader_params *params) { - void *smram = (void *)(SMM_DEFAULT_BASE); + uintptr_t smram = SMM_DEFAULT_BASE; printk(BIOS_SPEW, "%s: enter\n", __func__); /* There can't be more than 1 concurrent save state for the relocation * handler because all CPUs default to 0x30000 as SMBASE. */ @@ -489,7 +488,8 @@ * expects a region large enough to encompass the handler and stacks * as well as the SMM_DEFAULT_SIZE. */ -int smm_load_module(void *smram, size_t size, struct smm_loader_params *params) +int smm_load_module(const uintptr_t smram_base, const size_t smram_size, + struct smm_loader_params *params) { struct rmodule smm_mod; struct smm_runtime *handler_mod_params; @@ -500,16 +500,16 @@ size_t fxsave_size; void *fxsave_area; size_t total_size = 0; - char *base; - void *smram_start = smram; - if (size <= SMM_DEFAULT_SIZE) + uintptr_t base; /* The base for the permanent handler */ + + if (smram_size <= SMM_DEFAULT_SIZE) return -1;
/* Load main SMI handler at the top of SMRAM * everything else will go below */ - base = smram; - base += size; + base = smram_base; + base += smram_size;
/* Fail if can't parse the smm rmodule. */ if (rmodule_parse(&_binary_smm_start, &smm_mod)) @@ -517,13 +517,13 @@
/* Clear SMM region */ if (CONFIG(DEBUG_SMI)) - memset(smram, 0xcd, size); + memset((void *)smram_base, 0xcd, smram_size);
total_stack_size = params->per_cpu_stack_size * params->num_concurrent_stacks; total_size += total_stack_size; /* Stacks are the base of SMRAM */ - params->stack_top = smram + total_stack_size; + params->stack_top = smram_base + total_stack_size;
/* MSEG starts at the top of SMRAM and works down */ if (CONFIG(STM)) { @@ -534,7 +534,7 @@ /* FXSAVE goes below MSEG */ if (CONFIG(SSE)) { fxsave_size = FXSAVE_SIZE * params->num_concurrent_stacks; - fxsave_area = base - fxsave_size; + fxsave_area = (char *)base - fxsave_size; base -= fxsave_size; total_size += fxsave_size; } else { @@ -546,8 +546,7 @@ base -= handler_size; total_size += handler_size; module_alignment = rmodule_load_alignment(&smm_mod); - alignment_size = module_alignment - - ((uintptr_t)base % module_alignment); + alignment_size = module_alignment - (base % module_alignment); if (alignment_size != module_alignment) { handler_size += alignment_size; base += alignment_size; @@ -555,10 +554,10 @@
printk(BIOS_DEBUG, "%s: total_smm_space_needed %zx, available -> %zx\n", - __func__, total_size, size); + __func__, total_size, smram_size);
/* Does the required amount of memory exceed the SMRAM region size? */ - if (total_size > size) { + if (total_size > smram_size) { printk(BIOS_ERR, "%s: need more SMRAM\n", __func__); return -1; } @@ -568,22 +567,20 @@ return -1; }
- if (rmodule_load(base, &smm_mod)) + if (rmodule_load((void *)base, &smm_mod)) return -1;
params->handler = rmodule_entry(&smm_mod); handler_mod_params = rmodule_parameters(&smm_mod); - handler_mod_params->smbase = (uintptr_t)smram; - handler_mod_params->smm_size = size; + handler_mod_params->smbase = smram_base; + handler_mod_params->smm_size = smram_size; handler_mod_params->save_state_size = params->real_cpu_save_state_size; handler_mod_params->num_cpus = params->num_concurrent_stacks; handler_mod_params->gnvs_ptr = (uintptr_t)acpi_get_gnvs();
- printk(BIOS_DEBUG, "%s: smram_start: 0x%p\n", - __func__, smram); - printk(BIOS_DEBUG, "%s: smram_end: %p\n", - __func__, smram + size); - printk(BIOS_DEBUG, "%s: stack_top: %p\n", + printk(BIOS_DEBUG, "%s: smram_start: 0x%lx\n", __func__, smram_base); + printk(BIOS_DEBUG, "%s: smram_end: %lx\n", __func__, smram_base + smram_size); + printk(BIOS_DEBUG, "%s: stack_top: %lx\n", __func__, params->stack_top); printk(BIOS_DEBUG, "%s: handler start %p\n", __func__, params->handler); @@ -610,10 +607,9 @@ * will be staggered below */ base -= SMM_CODE_SEGMENT_SIZE; - printk(BIOS_DEBUG, "%s: cpu0 entry: %p\n", - __func__, base); + printk(BIOS_DEBUG, "%s: cpu0 entry: %lx\n", __func__, base);
- if (!smm_create_map((uintptr_t)base, params->num_concurrent_save_states, params)) { + if (!smm_create_map(base, params->num_concurrent_save_states, params)) { printk(BIOS_ERR, "%s: Error creating CPU map\n", __func__); return -1; } @@ -623,5 +619,5 @@ cpus[i].ss_start + params->per_cpu_save_state_size; }
- return smm_module_setup_stub(base, size, params, fxsave_area, smram_start); + return smm_module_setup_stub(base, smram_size, params, fxsave_area, smram_base); } diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index 08404d0..848996f 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -135,7 +135,7 @@ * handle sparse APIC id space. */ struct smm_loader_params { - void *stack_top; + uintptr_t stack_top; size_t per_cpu_stack_size; size_t num_concurrent_stacks;
@@ -149,8 +149,8 @@ };
/* Both of these return 0 on success, < 0 on failure. */ -int smm_setup_relocation_handler(void * const perm_smram, struct smm_loader_params *params); -int smm_load_module(void *smram, size_t size, struct smm_loader_params *params); +int smm_setup_relocation_handler(const uintptr_t perm_smram, struct smm_loader_params *params); +int smm_load_module(uintptr_t smram_base, size_t smram_size, struct smm_loader_params *params);
u32 smm_get_cpu_smbase(unsigned int cpu_num);
-
Kyösti Mälkki (Code Review)