Change in coreboot[master]: cpu/x86/smm: Fix getting the save state for multiple segments
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47081 )
Change subject: cpu/x86/smm: Fix getting the save state for multiple segments ......................................................................
cpu/x86/smm: Fix getting the save state for multiple segments
The map for for the base of smm save states is a bit more complicated when a system uses multiple segments. Instead of redoing the segment computation, copy over the map used in during relocation to the permanent handler, via relocatable module parameters.
Untested.
Change-Id: I480d00c55bedd0d2e4a7984cd577b7f5aaf6078d Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/x86/smm/smm_module_handler.c M src/cpu/x86/smm/smm_module_loader.c M src/include/cpu/x86/smm.h 3 files changed, 20 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/47081/1
diff --git a/src/cpu/x86/smm/smm_module_handler.c b/src/cpu/x86/smm/smm_module_handler.c index b6ef077..77432fd 100644 --- a/src/cpu/x86/smm/smm_module_handler.c +++ b/src/cpu/x86/smm/smm_module_handler.c @@ -18,6 +18,8 @@ static volatile __attribute__((aligned(4))) smi_semaphore smi_handler_status = SMI_UNLOCKED;
+const struct smm_handler_params params __attribute__((unused,section(".module_parameters"))); + static int smi_obtain_lock(void) { u8 ret = SMI_LOCKED; @@ -91,18 +93,17 @@
struct global_nvs *gnvs;
-/* TODO: handle multiple segments! */ +/* This returns the base of the save state allocated for a CPU. + * Note that if a size larger than the actual save state is allocated + * this is not the 'real' base of the save state as those are top aligned + * and not bottom aligned. + */ void *smm_get_save_state(int cpu) { - char *base; + if (cpu < CONFIG_MAX_CPUS) + return (void *)params.ss_start[cpu];
- /* This function assumes all save states start at top of default - * SMRAM size space and are staggered down by save state size. */ - base = (void *)(uintptr_t)smm_runtime->smbase; - base += SMM_DEFAULT_SIZE; - base -= (cpu + 1) * smm_runtime->save_state_size; - - return base; + return NULL; }
uint32_t smm_revision(void) diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index 66d0268..7c35433 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -562,6 +562,7 @@ void *fxsave_area; size_t total_size = 0; char *base; + struct smm_handler_params *handler_params;
if (size <= SMM_DEFAULT_SIZE) return -1; @@ -637,6 +638,10 @@
params->handler = rmodule_entry(&smm_mod); params->handler_arg = rmodule_parameters(&smm_mod); + handler_params = params->handler_arg; + /* Copy over the CPU save state map to the permanent handler. */ + for (int i = 0; i < CONFIG_MAX_CPUS; i++) + handler_params->ss_start[i] = cpus[i].ss_start;
printk(BIOS_DEBUG, "%s: smram_start: 0x%p\n", __func__, smram); diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index 7491d95..3d109a1 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -153,6 +153,11 @@ unsigned int smram_end; };
+/* These are the relocatable module parameters for the permanent handler */ +struct smm_handler_params { + uintptr_t ss_start[CONFIG_MAX_CPUS]; +}; + /* Both of these return 0 on success, < 0 on failure. */ int smm_setup_relocation_handler(struct smm_loader_params *params); int smm_load_module(void *smram, size_t size, struct smm_loader_params *params);
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47081 )
Change subject: cpu/x86/smm: Fix getting the save state for multiple segments ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47081/1/src/cpu/x86/smm/smm_module_... File src/cpu/x86/smm/smm_module_handler.c:
https://review.coreboot.org/c/coreboot/+/47081/1/src/cpu/x86/smm/smm_module_... PS1, Line 21: const struct smm_handler_params params __attribute__((unused,section(".module_parameters"))); space required after that ',' (ctx:VxV)
Arthur Heymans has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/47081 )
Change subject: cpu/x86/smm: Fix getting the save state for multiple segments ......................................................................
cpu/x86/smm: Fix getting the save state for multiple segments
The map for for the base of smm save states is a bit more complicated when a system uses multiple segments. Instead of redoing the segment computation, copy over the map used in during relocation to the permanent handler, via relocatable module parameters.
Untested.
Change-Id: I480d00c55bedd0d2e4a7984cd577b7f5aaf6078d Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/x86/smm/smm_module_handler.c M src/cpu/x86/smm/smm_module_loader.c M src/include/cpu/x86/smm.h 3 files changed, 20 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/47081/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47081 )
Change subject: cpu/x86/smm: Fix getting the save state for multiple segments ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47081/2/src/cpu/x86/smm/smm_module_... File src/cpu/x86/smm/smm_module_handler.c:
https://review.coreboot.org/c/coreboot/+/47081/2/src/cpu/x86/smm/smm_module_... PS2, Line 21: const struct smm_handler_params params __attribute__((unused,section(".module_parameters"))); space required after that ',' (ctx:VxV)
Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/47081 )
Change subject: cpu/x86/smm: Fix getting the save state for multiple segments ......................................................................
Abandoned
Done differently
-
Arthur Heymans (Code Review) -
build bot (Jenkins) (Code Review)