Subrata Banik submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Subrata Banik: Looks good to me, approved Angel Pons: Looks good to me, approved
cpu/x86/mp_init.c: Fix HAVE_SMI_HANDLER

Fixes commit 29c7622 ("cpu/x86/mp_init.c: Fix building with no
smihandler") broke SMM init because is_smm_enable() was called before
smm_enable.

Rework the code a little to make it clear what codepaths are used with
CONFIG_HAVE_SMI_HANDLER.

TESTED: now prodrive/hermes boots again.

Change-Id: If4ce0dca2f29754d131dacf2da63e946be9a7b6d
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59912
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Subrata Banik <subrata.banik@intel.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
---
M src/cpu/x86/mp_init.c
1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c
index 709e7a2..507b5fe 100644
--- a/src/cpu/x86/mp_init.c
+++ b/src/cpu/x86/mp_init.c
@@ -1061,27 +1061,13 @@
return rmodule_memory_size(&smm_stub);
}

-static void fill_mp_state(struct mp_state *state, const struct mp_ops *ops)
+static void fill_mp_state_smm(struct mp_state *state, const struct mp_ops *ops)
{
- /*
- * Make copy of the ops so that defaults can be set in the non-const
- * structure if needed.
- */
- memcpy(&state->ops, ops, sizeof(*ops));
-
- if (ops->get_cpu_count != NULL)
- state->cpu_count = ops->get_cpu_count();
-
- if (!is_smm_enabled())
- return;
-
if (ops->get_smm_info != NULL)
ops->get_smm_info(&state->perm_smbase, &state->perm_smsize,
- &state->smm_real_save_state_size);
+ &state->smm_real_save_state_size);

- if (CONFIG(HAVE_SMI_HANDLER))
- state->smm_save_state_size = MAX(state->smm_real_save_state_size,
- smm_stub_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
@@ -1095,11 +1081,25 @@
* Default to smm_initiate_relocation() if trigger callback isn't
* provided.
*/
- if (CONFIG(HAVE_SMI_HANDLER) &&
- ops->per_cpu_smm_trigger == NULL)
+ if (ops->per_cpu_smm_trigger == NULL)
mp_state.ops.per_cpu_smm_trigger = smm_initiate_relocation;
}

+static void fill_mp_state(struct mp_state *state, const struct mp_ops *ops)
+{
+ /*
+ * Make copy of the ops so that defaults can be set in the non-const
+ * structure if needed.
+ */
+ memcpy(&state->ops, ops, sizeof(*ops));
+
+ if (ops->get_cpu_count != NULL)
+ state->cpu_count = ops->get_cpu_count();
+
+ if (CONFIG(HAVE_SMI_HANDLER))
+ fill_mp_state_smm(state, ops);
+}
+
static enum cb_err do_mp_init_with_smm(struct bus *cpu_bus, const struct mp_ops *mp_ops)
{
enum cb_err ret;

2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one.

To view, visit change 59912. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If4ce0dca2f29754d131dacf2da63e946be9a7b6d
Gerrit-Change-Number: 59912
Gerrit-PatchSet: 5
Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Arthur Heymans <arthur.heymans@9elements.com>
Gerrit-MessageType: merged