Change in coreboot[master]: cpu/x86/mp_init: Allow stub sizes larger than the save state size

[Arthur Heymans (Code Review)]

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/50770 )

Change subject: cpu/x86/mp_init: Allow stub sizes larger than the save state size
......................................................................

cpu/x86/mp_init: Allow stub sizes larger than the save state size

The permanent handler module argument 'save_state_size' now holds the
meaning of the real save state size which is then substracted from the
CPUs save state 'top' to get the save state base.

TESTED with qemu Q35 on x86_64 where the stub size exceed the AMD64
save state size.

Change-Id: I55d7611a17b6d0a39aee1c56318539232a9bb781
Signed-off-by: Arthur Heymans 
---
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/include/cpu/x86/smm.h
4 files changed, 35 insertions(+), 19 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/50770/1

diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c
index f42c1a2..a6c7a16 100644
--- a/src/cpu/x86/mp_init.c
+++ b/src/cpu/x86/mp_init.c
@@ -685,6 +685,9 @@
    int cpu_count;
    uintptr_t perm_smbase;
    size_t perm_smsize;
+	/* Size of the real CPU save state */
+	size_t smm_real_save_state_size;
+	/* Size of allocated CPU save state, MAX(real save state size, stub size) */
    size_t smm_save_state_size;
    uintptr_t reloc_start32_offset;
    int do_smm;
@@ -766,7 +769,8 @@
    	stub_params->apic_id_to_cpu[i] = cpu_get_apic_id(i);
 }

-static int install_relocation_handler(int num_cpus, size_t save_state_size)
+static int install_relocation_handler(int num_cpus, size_t real_save_state_size,
+				      size_t save_state_size)
 {
    int cpus = num_cpus;
 #if CONFIG(X86_SMM_LOADER_VERSION2)
@@ -779,6 +783,7 @@
    struct smm_loader_params smm_params = {
    	.per_cpu_stack_size = CONFIG_SMM_STUB_STACK_SIZE,
    	.num_concurrent_stacks = cpus,
+		.real_cpu_save_state_size = real_save_state_size,
    	.per_cpu_save_state_size = save_state_size,
    	.num_concurrent_save_states = 1,
    	.handler = smm_do_relocation,
@@ -800,7 +805,8 @@
 }

static int install_permanent_handler(int num_cpus, uintptr_t smbase,
-					size_t smsize, size_t save_state_size)
+				     size_t smsize, size_t real_save_state_size,
+				     size_t save_state_size)
 {
    /*
     * All the CPUs will relocate to permanaent handler now. Set parameters
@@ -812,6 +818,7 @@
    struct smm_loader_params smm_params = {
    	.per_cpu_stack_size = CONFIG_SMM_MODULE_STACK_SIZE,
    	.num_concurrent_stacks = num_cpus,
+		.real_cpu_save_state_size = real_save_state_size,
    	.per_cpu_save_state_size = save_state_size,
    	.num_concurrent_save_states = num_cpus,
    };
@@ -833,6 +840,7 @@
 /* Load SMM handlers as part of MP flight record. */
 static void load_smm_handlers(void)
 {
+	size_t real_save_state_size = mp_state.smm_real_save_state_size;
    size_t smm_save_state_size = mp_state.smm_save_state_size;

/* Do nothing if SMM is disabled.*/
@@ -841,13 +849,15 @@

/* Install handlers. */
    if (install_relocation_handler(mp_state.cpu_count,
-		smm_save_state_size) < 0) {
+				       real_save_state_size,
+				       smm_save_state_size) < 0) {
    	printk(BIOS_ERR, "Unable to install SMM relocation handler.\n");
    	smm_disable();
    }

if (install_permanent_handler(mp_state.cpu_count, mp_state.perm_smbase,
-		mp_state.perm_smsize, smm_save_state_size) < 0) {
+				      mp_state.perm_smsize, real_save_state_size,
+				      smm_save_state_size) < 0) {
    	printk(BIOS_ERR, "Unable to install SMM permanent handler.\n");
    	smm_disable();
    }
@@ -1035,6 +1045,21 @@
    MP_FR_BLOCK_APS(ap_wait_for_instruction, NULL),
 };

+static size_t smm_stub_size(void)
+{
+	extern unsigned char _binary_smmstub_start[];
+	struct rmodule smm_stub;
+
+	if (rmodule_parse(&_binary_smmstub_start, &smm_stub)) {
+		printk(BIOS_ERR, "%s: unable to get SMM module size\n", __func__);
+		return 0;
+	}
+
+	size_t size = rmodule_memory_size(&smm_stub);
+	printk(BIOS_DEBUG, "stub_size 0x%lx\n\n\n", size);
+	return size;
+}
+
 static void fill_mp_state(struct mp_state *state, const struct mp_ops *ops)
 {
    /*
@@ -1048,7 +1073,9 @@

if (ops->get_smm_info != NULL)
    	ops->get_smm_info(&state->perm_smbase, &state->perm_smsize,
-					&state->smm_save_state_size);
+					&state->smm_real_save_state_size);
+
+	state->smm_save_state_size = MAX(state->smm_real_save_state_size, smm_stub_size());

/*
     * Make sure there is enough room for the SMM descriptor
diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c
index e2ef2dc..9b025a8 100644
--- a/src/cpu/x86/smm/smm_module_loader.c
+++ b/src/cpu/x86/smm/smm_module_loader.c
@@ -198,12 +198,6 @@
    smm_stub_size = rmodule_memory_size(&smm_stub);
    stub_entry_offset = rmodule_entry_offset(&smm_stub);

-	if (smm_stub_size > params->per_cpu_save_state_size) {
-		printk(BIOS_ERR, "SMM Module: SMM stub size larger than save state size\n");
-		printk(BIOS_ERR, "SMM Module: Staggered entry points will overlap stub\n");
-		return -1;
-	}
-
    /* Assume the stub is always small enough to live within upper half of
     * SMRAM region after the save state space has been allocated. */
    smm_stub_loc = &base[SMM_ENTRY_OFFSET];
@@ -391,7 +385,7 @@
    params->handler_arg = (void *)handler_mod_params;
    handler_mod_params->smbase = (uintptr_t)smram;
    handler_mod_params->smm_size = size;
-	handler_mod_params->save_state_size = params->per_cpu_save_state_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();

diff --git a/src/cpu/x86/smm/smm_module_loaderv2.c b/src/cpu/x86/smm/smm_module_loaderv2.c
index a24edd5..1ce5b87 100644
--- a/src/cpu/x86/smm/smm_module_loaderv2.c
+++ b/src/cpu/x86/smm/smm_module_loaderv2.c
@@ -123,12 +123,6 @@
    	return 0;
    }

-	if (stub_size > ss_size) {
-		printk(BIOS_ERR, "%s: Save state larger than SMM stub size\n", __func__);
-		printk(BIOS_ERR, "    Decrease stub size or increase the size allocated for the save state\n");
-		return 0;
-	}
-
    for (i = 0; i < num_cpus; i++) {
    	cpus[i].smbase = base;
    	cpus[i].entry = base + smm_entry_offset;
@@ -593,7 +587,7 @@
    params->handler_arg = (void *)handler_mod_params;
    handler_mod_params->smbase = (uintptr_t)smram;
    handler_mod_params->smm_size = size;
-	handler_mod_params->save_state_size = params->per_cpu_save_state_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();

diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h
index e1cc064..ace3a8f 100644
--- a/src/include/cpu/x86/smm.h
+++ b/src/include/cpu/x86/smm.h
@@ -149,6 +149,7 @@
    size_t per_cpu_stack_size;
    size_t num_concurrent_stacks;

+	size_t real_cpu_save_state_size;
    size_t per_cpu_save_state_size;
    size_t num_concurrent_save_states;