gm45: re-add mitigations for no-microcode setup
libreboot will still include microcode updates by default, but mitigations against broken speedstep and reboot (when microcode updates are excluded) were removed following the merge with osboot this patch restores those mitigations; the patch reverts coreboot to older smrr code (which works fine, it isn't critical to use the new behaviour) and disables peci (pointless feature) i'll probably re-tool this later to apply the changes conditionally to whether ucode is present this is not a change in policy. policy says: include cpu microcode updates by default policy also says: libreboot must be configurable microcode removal via cbfstool remove -n, counts as configuration, and in practise is not possible on gm45 patches in current libreboot; this patch corrects that problem, allowing the machines to work somewhat well (same stability issues as before, like MCE errors resulting in kernel panic on high CPU/memory usage, but i digress) happy... hackingfsdg20230625
Leah Rowe
parent
8fb54e801f
commit
bd4ea9a028
|
|
@ -0,0 +1,47 @@
|
|||
From 3cf315fd59f1388d60cce9290eb52bccb7b29625 Mon Sep 17 00:00:00 2001
|
||||
From: Leah Rowe <leah@libreboot.org>
|
||||
Date: Wed, 1 Dec 2021 02:53:00 +0000
|
||||
Subject: [PATCH 1/2] fix speedstep on x200/t400: Revert
|
||||
"cpu/intel/model_1067x: enable PECI"
|
||||
|
||||
This reverts commit 70fea013c7ebd6d85a7806748233fcfd76802f5f.
|
||||
|
||||
Enabling PECI without microcode updates loaded causes the CPUID feature set
|
||||
to become corrupted. And one consequence is broken SpeedStep. At least, that's
|
||||
my understanding looking at Intel Errata. This revert is not a fix, because
|
||||
upstream is correct (upstream assumes microcode updates). We will simply
|
||||
maintain this revert patch in Libreboot, from now on.
|
||||
---
|
||||
src/cpu/intel/model_1067x/model_1067x_init.c | 9 ---------
|
||||
1 file changed, 9 deletions(-)
|
||||
|
||||
diff --git a/src/cpu/intel/model_1067x/model_1067x_init.c b/src/cpu/intel/model_1067x/model_1067x_init.c
|
||||
index 315e7c36fc..1423fd72bc 100644
|
||||
--- a/src/cpu/intel/model_1067x/model_1067x_init.c
|
||||
+++ b/src/cpu/intel/model_1067x/model_1067x_init.c
|
||||
@@ -141,8 +141,6 @@ static void configure_emttm_tables(void)
|
||||
wrmsr(MSR_EMTTM_CR_TABLE(5), msr);
|
||||
}
|
||||
|
||||
-#define IA32_PECI_CTL 0x5a0
|
||||
-
|
||||
static void configure_misc(const int eist, const int tm2, const int emttm)
|
||||
{
|
||||
msr_t msr;
|
||||
@@ -185,13 +183,6 @@ static void configure_misc(const int eist, const int tm2, const int emttm)
|
||||
msr.lo |= (1 << 20); /* Lock Enhanced SpeedStep Enable */
|
||||
wrmsr(IA32_MISC_ENABLE, msr);
|
||||
}
|
||||
-
|
||||
- /* Enable PECI
|
||||
- WARNING: due to Erratum AW67 described in Intel document #318733
|
||||
- the microcode must be updated before this MSR is written to. */
|
||||
- msr = rdmsr(IA32_PECI_CTL);
|
||||
- msr.lo |= 1;
|
||||
- wrmsr(IA32_PECI_CTL, msr);
|
||||
}
|
||||
|
||||
#define PIC_SENS_CFG 0x1aa
|
||||
--
|
||||
2.40.0
|
||||
|
||||
|
|
@ -0,0 +1,173 @@
|
|||
From 651292a204b00d7a39d8722f9d26fd9d7178fba2 Mon Sep 17 00:00:00 2001
|
||||
From: Leah Rowe <leah@libreboot.org>
|
||||
Date: Mon, 17 Apr 2023 15:49:57 +0100
|
||||
Subject: [PATCH 1/1] GM45-type CPUs: don't enable alternative SMRR
|
||||
|
||||
This reverts the changes in coreboot revision:
|
||||
df7aecd92643d207feaf7fd840f8835097346644
|
||||
|
||||
While this fix is *technically correct*, the one in
|
||||
coreboot, it breaks rebooting as tested on several
|
||||
GM45 ThinkPads e.g. X200, T400, when microcode
|
||||
updates are not applied.
|
||||
|
||||
Since November 2022, Libreboot includes microcode
|
||||
updates by default, but it tells users how to remove
|
||||
it from the ROM (with cbfstool) if they wish.
|
||||
|
||||
Well, with Libreboot 20221214, 20230319 and 20230413,
|
||||
mitigations present in Libreboot 20220710 (which did
|
||||
not have microcode updates) do not exist.
|
||||
|
||||
This patch, along with the other patch to remove PECI
|
||||
support (which breaks speedstep when microcode updates
|
||||
are not applied) have now been re-added to Libreboot.
|
||||
|
||||
It is still best to use microcode updates by default.
|
||||
These patches in coreboot are not critically urgent,
|
||||
and you can use the machines with or without them,
|
||||
regardless of ucode.
|
||||
|
||||
I'll probably re-write this and the other patch at
|
||||
some point, applying the change conditionally upon
|
||||
whether or not microcode is applied.
|
||||
|
||||
Pragmatism is a good thing. I recommend it.
|
||||
---
|
||||
src/cpu/intel/model_1067x/model_1067x_init.c | 4 +++
|
||||
src/cpu/intel/model_1067x/mp_init.c | 26 --------------------
|
||||
src/cpu/intel/model_106cx/model_106cx_init.c | 4 +++
|
||||
src/cpu/intel/model_6ex/model_6ex_init.c | 4 +++
|
||||
src/cpu/intel/model_6fx/model_6fx_init.c | 4 +++
|
||||
5 files changed, 16 insertions(+), 26 deletions(-)
|
||||
|
||||
diff --git a/src/cpu/intel/model_1067x/model_1067x_init.c b/src/cpu/intel/model_1067x/model_1067x_init.c
|
||||
index 1423fd72bc..d1f98ca43a 100644
|
||||
--- a/src/cpu/intel/model_1067x/model_1067x_init.c
|
||||
+++ b/src/cpu/intel/model_1067x/model_1067x_init.c
|
||||
@@ -8,6 +8,7 @@
|
||||
#include <cpu/x86/cache.h>
|
||||
#include <cpu/x86/name.h>
|
||||
#include <cpu/intel/smm_reloc.h>
|
||||
+#include <cpu/intel/common/common.h>
|
||||
|
||||
#define MSR_BBL_CR_CTL3 0x11e
|
||||
|
||||
@@ -234,6 +235,9 @@ static void model_1067x_init(struct device *cpu)
|
||||
fill_processor_name(processor_name);
|
||||
printk(BIOS_INFO, "CPU: %s.\n", processor_name);
|
||||
|
||||
+ /* Set virtualization based on Kconfig option */
|
||||
+ set_vmx_and_lock();
|
||||
+
|
||||
/* Configure C States */
|
||||
configure_c_states(quad);
|
||||
|
||||
diff --git a/src/cpu/intel/model_1067x/mp_init.c b/src/cpu/intel/model_1067x/mp_init.c
|
||||
index bc53214310..72f40f6762 100644
|
||||
--- a/src/cpu/intel/model_1067x/mp_init.c
|
||||
+++ b/src/cpu/intel/model_1067x/mp_init.c
|
||||
@@ -43,34 +43,8 @@ static void pre_mp_smm_init(void)
|
||||
smm_initialize();
|
||||
}
|
||||
|
||||
-#define SMRR_SUPPORTED (1 << 11)
|
||||
-
|
||||
static void per_cpu_smm_trigger(void)
|
||||
{
|
||||
- msr_t mtrr_cap = rdmsr(MTRR_CAP_MSR);
|
||||
- if (cpu_has_alternative_smrr() && mtrr_cap.lo & SMRR_SUPPORTED) {
|
||||
- set_feature_ctrl_vmx();
|
||||
- msr_t ia32_ft_ctrl = rdmsr(IA32_FEATURE_CONTROL);
|
||||
- /* We don't care if the lock is already setting
|
||||
- as our smm relocation handler is able to handle
|
||||
- setups where SMRR is not enabled here. */
|
||||
- if (ia32_ft_ctrl.lo & (1 << 0)) {
|
||||
- /* IA32_FEATURE_CONTROL locked. If we set it again we
|
||||
- get an illegal instruction. */
|
||||
- printk(BIOS_DEBUG, "IA32_FEATURE_CONTROL already locked\n");
|
||||
- printk(BIOS_DEBUG, "SMRR status: %senabled\n",
|
||||
- ia32_ft_ctrl.lo & (1 << 3) ? "" : "not ");
|
||||
- } else {
|
||||
- if (!CONFIG(SET_IA32_FC_LOCK_BIT))
|
||||
- printk(BIOS_INFO,
|
||||
- "Overriding CONFIG(SET_IA32_FC_LOCK_BIT) to enable SMRR\n");
|
||||
- ia32_ft_ctrl.lo |= (1 << 3) | (1 << 0);
|
||||
- wrmsr(IA32_FEATURE_CONTROL, ia32_ft_ctrl);
|
||||
- }
|
||||
- } else {
|
||||
- set_vmx_and_lock();
|
||||
- }
|
||||
-
|
||||
/* Relocate the SMM handler. */
|
||||
smm_relocate();
|
||||
}
|
||||
diff --git a/src/cpu/intel/model_106cx/model_106cx_init.c b/src/cpu/intel/model_106cx/model_106cx_init.c
|
||||
index 05f5f327cc..0450c2ad83 100644
|
||||
--- a/src/cpu/intel/model_106cx/model_106cx_init.c
|
||||
+++ b/src/cpu/intel/model_106cx/model_106cx_init.c
|
||||
@@ -7,6 +7,7 @@
|
||||
#include <cpu/intel/speedstep.h>
|
||||
#include <cpu/x86/cache.h>
|
||||
#include <cpu/x86/name.h>
|
||||
+#include <cpu/intel/common/common.h>
|
||||
|
||||
#define HIGHEST_CLEVEL 3
|
||||
static void configure_c_states(void)
|
||||
@@ -66,6 +67,9 @@ static void model_106cx_init(struct device *cpu)
|
||||
fill_processor_name(processor_name);
|
||||
printk(BIOS_INFO, "CPU: %s.\n", processor_name);
|
||||
|
||||
+ /* Set virtualization based on Kconfig option */
|
||||
+ set_vmx_and_lock();
|
||||
+
|
||||
/* Configure C States */
|
||||
configure_c_states();
|
||||
|
||||
diff --git a/src/cpu/intel/model_6ex/model_6ex_init.c b/src/cpu/intel/model_6ex/model_6ex_init.c
|
||||
index 5bd1c32815..f3bb08cde3 100644
|
||||
--- a/src/cpu/intel/model_6ex/model_6ex_init.c
|
||||
+++ b/src/cpu/intel/model_6ex/model_6ex_init.c
|
||||
@@ -7,6 +7,7 @@
|
||||
#include <cpu/intel/speedstep.h>
|
||||
#include <cpu/x86/cache.h>
|
||||
#include <cpu/x86/name.h>
|
||||
+#include <cpu/intel/common/common.h>
|
||||
|
||||
#define HIGHEST_CLEVEL 3
|
||||
static void configure_c_states(void)
|
||||
@@ -105,6 +106,9 @@ static void model_6ex_init(struct device *cpu)
|
||||
/* Setup Page Attribute Tables (PAT) */
|
||||
// TODO set up PAT
|
||||
|
||||
+ /* Set virtualization based on Kconfig option */
|
||||
+ set_vmx_and_lock();
|
||||
+
|
||||
/* Configure C States */
|
||||
configure_c_states();
|
||||
|
||||
diff --git a/src/cpu/intel/model_6fx/model_6fx_init.c b/src/cpu/intel/model_6fx/model_6fx_init.c
|
||||
index 535fb8fae7..f7b05facd2 100644
|
||||
--- a/src/cpu/intel/model_6fx/model_6fx_init.c
|
||||
+++ b/src/cpu/intel/model_6fx/model_6fx_init.c
|
||||
@@ -7,6 +7,7 @@
|
||||
#include <cpu/intel/speedstep.h>
|
||||
#include <cpu/x86/cache.h>
|
||||
#include <cpu/x86/name.h>
|
||||
+#include <cpu/intel/common/common.h>
|
||||
|
||||
#define HIGHEST_CLEVEL 3
|
||||
static void configure_c_states(void)
|
||||
@@ -118,6 +119,9 @@ static void model_6fx_init(struct device *cpu)
|
||||
/* Setup Page Attribute Tables (PAT) */
|
||||
// TODO set up PAT
|
||||
|
||||
+ /* Set virtualization based on Kconfig option */
|
||||
+ set_vmx_and_lock();
|
||||
+
|
||||
/* Configure C States */
|
||||
configure_c_states();
|
||||
|
||||
--
|
||||
2.40.0
|
||||
|
||||
Loading…
Reference in New Issue