mb/prodrive/hermes: Ensure VMX setting is applied
VMX is enabled through a bit in the IA32_FEATURE_CONTROL MSR, which can
be locked. The MSR remains locked after a non-power cycle reset, though.
If the MSR is locked, coreboot bails out and leaves VMX in the state it
was found. Because of this, changes to the VMX enable option in the BMC
only take effect after the system is power cycled.
This behaviour is highly undesirable because users are likely not aware
that a power cycle is required for changes to VMX state to take effect.
So, if VMX is supported, the IA32_FEATURE_CONTROL MSR is locked and the
current VMX state does not match the requested state, then issue a full
reset. This will power cycle the system and unlock the MSR, so that the
desired VMX state can be programmed into the MSR. This is checked early
to avoid needlessly doing time-consuming operations (running FSP) twice
if we know we will need to power cycle the system anyway.
Note that a user may change the VMX setting after the newly-added check
but before the setting is read in ramstage to program the MSR, but this
is a non-issue as firmware settings need a reset to take effect anyway.
TEST: Toggle VMX setting in BMC and reboot without power cycle, observe
coreboot automatically issues a power cycle reset because the MSR
is locked and the VMX state differs. Verify that the system boots
properly with VMX in the correct state after having power cycled.
Change-Id: Id9061ba896a7062da45a86fb26eeb58927184dcb
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/75141
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Lean Sheng Tan <sheng.tan@9elements.com>
diff --git a/src/mainboard/prodrive/hermes/romstage.c b/src/mainboard/prodrive/hermes/romstage.c
index 9853426..a479bd5 100644
--- a/src/mainboard/prodrive/hermes/romstage.c
+++ b/src/mainboard/prodrive/hermes/romstage.c
@@ -1,7 +1,12 @@
/* SPDX-License-Identifier: GPL-2.0-only */
+#include <cf9_reset.h>
+#include <console/console.h>
+#include <cpu/cpu.h>
+#include <cpu/x86/msr.h>
#include <soc/cnl_memcfg_init.h>
#include <soc/romstage.h>
+#include <types.h>
#include "eeprom.h"
@@ -40,14 +45,44 @@
.ect = 0,
};
+static bool vmx_needs_full_reset(const bool enable_vmx)
+{
+ const uint32_t feature_flag = cpu_get_feature_flags_ecx();
+
+ if (!(feature_flag & (CPUID_VMX | CPUID_SMX))) {
+ /* CPU does not support VMX, no reset needed */
+ return false;
+ }
+
+ const msr_t msr = rdmsr(IA32_FEATURE_CONTROL);
+
+ const bool msr_locked = msr.lo & (1 << 0);
+ const bool vmx_enabled = msr.lo & (1 << 2);
+
+ /* Reset if the MSR is locked and VMX state does not match requested state */
+ return msr_locked && (vmx_enabled != enable_vmx);
+}
+
void mainboard_memory_init_params(FSPM_UPD *memupd)
{
+ const struct eeprom_board_settings *board_cfg = get_board_settings();
+
+ /*
+ * IA32_FEATURE_CONTROL is only unlocked by a full reset.
+ * Check early to avoid wasting time if we need to reset.
+ */
+ const bool enable_vmx = !board_cfg->vtx_disabled;
+ if (vmx_needs_full_reset(enable_vmx)) {
+ printk(BIOS_INFO, "IA32_FEATURE_CONTROL locked; full reset needed to %s VMX\n",
+ enable_vmx ? "enable" : "disable");
+ full_reset();
+ }
+
memupd->FspmConfig.UserBd = BOARD_TYPE_SERVER;
memupd->FspmTestConfig.SmbusSpdWriteDisable = 0;
cannonlake_memcfg_init(&memupd->FspmConfig, &baseboard_mem_cfg);
/* Tell FSP-M about the desired primary video adapter so that GGC is set up properly */
- const struct eeprom_board_settings *board_cfg = get_board_settings();
if (board_cfg && board_cfg->primary_video == PRIMARY_VIDEO_INTEL)
memupd->FspmConfig.PrimaryDisplay = 0; /* iGPU is primary */