Marc Jones submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved Jay Talbott: Looks good to me, but someone else must approve
soc/intel/xeon_sp/: Fix SMI_LOCK setting

Move the SMI_LOCK to post SMM setup. Also, use the correct access
method for SMI_LOCK. GEN_PMCON_A is in PCI config space and not
in MMIO space on this PCH.

Change-Id: Ibbb183ef61ca7330198c1243ecfc2d4df51e652b
Signed-off-by: Marc Jones <marcjones@sysproconsulting.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51452
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Jay Talbott <JayTalbott@sysproconsulting.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
---
M src/soc/intel/xeon_sp/cpx/cpu.c
M src/soc/intel/xeon_sp/include/soc/pm.h
M src/soc/intel/xeon_sp/lockdown.c
M src/soc/intel/xeon_sp/pmc.c
4 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/src/soc/intel/xeon_sp/cpx/cpu.c b/src/soc/intel/xeon_sp/cpx/cpu.c
index b3ab236..339bf09 100644
--- a/src/soc/intel/xeon_sp/cpx/cpu.c
+++ b/src/soc/intel/xeon_sp/cpx/cpu.c
@@ -16,8 +16,10 @@
#include <cpu/x86/mtrr.h>
#include <intelblocks/cpulib.h>
#include <intelblocks/mp_init.h>
+#include <intelpch/lockdown.h>
#include <soc/cpu.h>
#include <soc/msr.h>
+#include <soc/pm.h>
#include <soc/soc_util.h>
#include <soc/smmrelocate.h>
#include <soc/util.h>
@@ -175,8 +177,11 @@
/* Set Max Ratio */
set_max_turbo_freq();

- if (CONFIG(HAVE_SMI_HANDLER))
+ if (CONFIG(HAVE_SMI_HANDLER)) {
global_smi_enable();
+ if (get_lockdown_config() == CHIPSET_LOCKDOWN_COREBOOT)
+ pmc_lock_smi();
+ }
}

static const struct mp_ops mp_ops = {
diff --git a/src/soc/intel/xeon_sp/include/soc/pm.h b/src/soc/intel/xeon_sp/include/soc/pm.h
index 2673320..8c26c6a 100644
--- a/src/soc/intel/xeon_sp/include/soc/pm.h
+++ b/src/soc/intel/xeon_sp/include/soc/pm.h
@@ -125,4 +125,6 @@

uint16_t get_pmbase(void);

+void pmc_lock_smi(void);
+
#endif
diff --git a/src/soc/intel/xeon_sp/lockdown.c b/src/soc/intel/xeon_sp/lockdown.c
index 0e21680..3a12110 100644
--- a/src/soc/intel/xeon_sp/lockdown.c
+++ b/src/soc/intel/xeon_sp/lockdown.c
@@ -16,18 +16,6 @@
}
}

-static void pmc_lock_smi(void)
-{
- uint8_t *pmcbase;
- uint8_t reg8;
-
- pmcbase = pmc_mmio_regs();
-
- reg8 = read8(pmcbase + GEN_PMCON_A);
- reg8 |= SMI_LOCK;
- write8(pmcbase + GEN_PMCON_A, reg8);
-}
-
static void pmc_lockdown_config(int chipset_lockdown)
{
uint8_t *pmcbase;
@@ -41,9 +29,6 @@

/* Make sure payload/OS can't trigger global reset */
pmc_global_reset_disable_and_lock();
-
- if (chipset_lockdown == CHIPSET_LOCKDOWN_COREBOOT)
- pmc_lock_smi();
}

void soc_lockdown_config(int chipset_lockdown)
diff --git a/src/soc/intel/xeon_sp/pmc.c b/src/soc/intel/xeon_sp/pmc.c
index b4f86db..7f59555 100644
--- a/src/soc/intel/xeon_sp/pmc.c
+++ b/src/soc/intel/xeon_sp/pmc.c
@@ -163,6 +163,12 @@
return ACPI_BASE_ADDRESS;
}

+void pmc_lock_smi(void)
+{
+ printk(BIOS_DEBUG, "Locking SMM enable.\n");
+ pci_or_config32(PCH_DEV_PMC, GEN_PMCON_A, SMI_LOCK);
+}
+
const char *const *soc_smi_sts_array(size_t *smi_arr)
{
static const char *const smi_sts_bits[] = {

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

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibbb183ef61ca7330198c1243ecfc2d4df51e652b
Gerrit-Change-Number: 51452
Gerrit-PatchSet: 7
Gerrit-Owner: Marc Jones <marc@marcjonesconsulting.com>
Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Jay Talbott <JayTalbott@sysproconsulting.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang@fb.com>
Gerrit-Reviewer: Marc Jones <marc@marcjonesconsulting.com>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus@gmail.com>
Gerrit-MessageType: merged