Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/63602 )
Change subject: [WIP]cpu/x86/smm: Use struct region to check overlapping sections ......................................................................
[WIP]cpu/x86/smm: Use struct region to check overlapping sections
This allows for some runtime checks on all SMM elements and removes the need for manually checks.
Change-Id: Ib7e2e3ae16c223ecfd8d5bce6ff6c17c53496925 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/x86/smm/smm_module_loader.c 1 file changed, 109 insertions(+), 39 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/63602/1
diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index 441ecf8..5744624 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -1,15 +1,18 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <acpi/acpi_gnvs.h> +#include <cbmem.h> +#include <commonlib/helpers.h> +#include <commonlib/region.h> +#include <console/console.h> +#include <cpu/x86/smm.h> +#include <rmodule.h> +#include <security/intel/stm/SmmStm.h> #include <stddef.h> #include <stdint.h> +#include <stdio.h> +#include <stdlib.h> #include <string.h> -#include <rmodule.h> -#include <cbmem.h> -#include <cpu/x86/smm.h> -#include <commonlib/helpers.h> -#include <console/console.h> -#include <security/intel/stm/SmmStm.h>
#define FXSAVE_SIZE 512 #define SMM_CODE_SEGMENT_SIZE 0x10000 @@ -114,8 +117,8 @@
for (unsigned int i = 0; i < num_cpus; i++) { if (i % cpus_per_segment == 0) - printk(BIOS_DEBUG, "-------------NEW CODE SEGMENT --------------\n"); - printk(BIOS_DEBUG, "CPU 0x%x\n", i); + printk(BIOS_SPEW, "-------------NEW CODE SEGMENT --------------\n"); + printk(BIOS_SPEW, "CPU 0x%x\n", i); /* We copy the same stub for each CPU so they all need the same 'smbase'. */ const size_t segment_number = i / cpus_per_segment; cpus[i].code_start = smbase - SMM_CODE_SEGMENT_SIZE * segment_number @@ -125,9 +128,9 @@ cpus[i].ss_top = cpus[i].code_start + SMM_ENTRY_OFFSET; cpus[i].ss_start = cpus[i].ss_top - params->cpu_save_state_size; cpus[i].code_end = cpus[i].code_start + stub_size; - printk(BIOS_DEBUG, " Stub [0x%lx-0x%lx[\n", cpus[i].code_start, + printk(BIOS_SPEW, " Stub [0x%lx-0x%lx[\n", cpus[i].code_start, cpus[i].code_end); - printk(BIOS_DEBUG, " Save state [0x%lx-0x%lx[\n", cpus[i].ss_start, + printk(BIOS_SPEW, " Save state [0x%lx-0x%lx[\n", cpus[i].ss_start, cpus[i].ss_top); cpus[i].active = 1; } @@ -353,6 +356,57 @@ mod_params->save_state_top[i] = cpus[i].ss_top; }
+struct region_list { + struct region region; + struct region_list *next; +}; + +static struct region_list *list; + +static int check_region(const struct region smram, + const struct region new_region, + const char *region_name) +{ + if (!region_is_subregion(&smram, &new_region)) { + printk(BIOS_ERR, "%s not in SMM\n", region_name); + return 1; + } + + struct region_list *test; + for (test = list; test != NULL; test = test->next) { + if (region_overlap(&test->region, &new_region)) { + printk(BIOS_ERR, "%s overlaps with a previous region\n", region_name); + return 1; + } + } + /* All is fine. Add the new region to the linked list */ + struct region_list *new_entry = malloc(sizeof(struct region_list)); + if (new_entry == NULL) { + printk(BIOS_ERR, "Out of memory\n"); + return 1; + } + + test->next = new_entry; + new_entry->region = new_region; + new_entry->next = NULL; + printk(BIOS_DEBUG, "%-12s [0x%lx-0x%lx]\n", region_name, region_offset(&new_region), + region_end(&new_region)); + return 0; +} + +static void free_list(void) +{ + struct region_list *entry = list; + struct region_list *next_entry; + while (1) { + if (entry == NULL) + return; + next_entry = entry->next; + free(entry); + entry = next_entry; + } +} + /* *The SMM module is placed within the provided region in the following * manner: @@ -389,62 +443,78 @@ if (rmodule_parse(&_binary_smm_start, &smi_handler)) return -1;
- const uintptr_t smram_top = smram_base + smram_size; + const struct region smram = { .offset = smram_base, .size = smram_size }; + const uintptr_t smram_top = region_end(&smram);
const size_t stm_size = CONFIG(STM) ? CONFIG_MSEG_SIZE - CONFIG_BIOS_RESOURCE_LIST_SIZE : 0; - const uintptr_t stm_base = CONFIG(STM) ? smram_top - stm_size : 0; - if (stm_size) { - printk(BIOS_DEBUG, "STM [0x%lx-0x%lx[\n", stm_base, - stm_base + stm_size);
+ struct region stm = {}; + if (CONFIG(STPM)) { + stm.offset = smram_top - stm_size; + stm.size = stm_size; + if (check_region(smram, stm, "STM")) + return 1; printk(BIOS_DEBUG, "MSEG size 0x%x\n", CONFIG_MSEG_SIZE); printk(BIOS_DEBUG, "BIOS res list 0x%x\n", CONFIG_BIOS_RESOURCE_LIST_SIZE); } + const size_t fx_save_area_size = CONFIG(SSE) ? FXSAVE_SIZE * params->num_cpus : 0; - const uintptr_t fx_save_area_base = - CONFIG(SSE) ? smram_top - stm_size - fx_save_area_size : 0; - if (fx_save_area_size) - printk(BIOS_DEBUG, "fx_save [0x%lx-0x%lx[\n", fx_save_area_base, - fx_save_area_base + fx_save_area_size); + struct region fx_save = {}; + if (CONFIG(SSE)) { + fx_save.offset = smram_top - stm_size - fx_save_area_size; + if (check_region(smram, fx_save, "STM")) + return 1; + } const size_t handler_size = rmodule_memory_size(&smi_handler); const size_t handler_alignment = rmodule_load_alignment(&smi_handler); const uintptr_t handler_base = ALIGN_DOWN(smram_top - stm_size - fx_save_area_size - handler_size, handler_alignment); - printk(BIOS_DEBUG, "smihandler [0x%lx-0x%lx[\n", handler_base, - handler_base + handler_size); - - if (handler_base <= smram_base) { - printk(BIOS_ERR, "Permanent handler won't FIT smram\n"); - return -1; - } + struct region handler = { + .offset = handler_base, + .size = handler_size + }; + if (check_region(smram, handler, "HANDLER")) + return 1;
const uintptr_t stub_segment_base = handler_base - SMM_CODE_SEGMENT_SIZE; if (!smm_create_map(stub_segment_base, params->num_concurrent_save_states, params)) { printk(BIOS_ERR, "%s: Error creating CPU map\n", __func__); return -1; } - - const uintptr_t lowest_stub = cpus[params->num_concurrent_save_states - 1].code_start; - const uintptr_t smm_stack_top = - smram_base + params->num_concurrent_save_states * CONFIG_SMM_MODULE_STACK_SIZE; - printk(BIOS_DEBUG, "cpu stacks [0x%lx-0x%lx[\n", smram_base, smm_stack_top); - if (lowest_stub < smm_stack_top) { - printk(BIOS_ERR, "SMM stubs won't fit in SMRAM 0x%lx\n", - cpus[params->num_concurrent_save_states - 1].code_start); - return -1; + for (unsigned int i = 0; i < params->num_concurrent_save_states; i++) { + printk(BIOS_DEBUG, "\nCPU %d\n", i); + struct region ss = { .offset = cpus[i].ss_start, .size = cpus[i].ss_top - cpus[i].ss_start }; + char string[13]; + snprintf(string, sizeof(string), " ss%d", i); + if (check_region(smram, ss, string)) + return 1; + struct region stub = {.offset = cpus[i].code_start, + .size = cpus[i].code_end - cpus[i].code_start}; + snprintf(string, sizeof(string), " stub%d", i); + if (check_region(smram, stub, string)) + return 1; }
- if (rmodule_load((void *)handler_base, &smi_handler)) - return -1; + struct region stacks = { + .offset = smram_base, + .size = params->num_concurrent_save_states * CONFIG_SMM_MODULE_STACK_SIZE + }; + printk(BIOS_DEBUG, "\n"); + if (check_region(smram, stacks, "stacks")) + return 1; + + if (rmodule_load((void *)handler_base, &smi_handler)) return -1;
struct smm_runtime *smihandler_params = rmodule_parameters(&smi_handler); params->handler = rmodule_entry(&smi_handler); setup_smihandler_params(smihandler_params, smram_base, smram_size, params);
+ free_list(); + return smm_module_setup_stub(stub_segment_base, smram_size, params, - (void *)fx_save_area_base); + (void *)region_offset(&fx_save)); }
/*