[coreboot-gerrit] Change in coreboot[master]: Kahlee initial SMI review clean-up

frank vibrans (Code Review) gerrit at coreboot.org
Tue Aug 15 18:36:13 CEST 2017


frank vibrans has uploaded this change for review. ( https://review.coreboot.org/21020


Change subject: Kahlee initial SMI review clean-up
......................................................................

Kahlee initial SMI review clean-up

Replaced hard-coded values with defined constants and
made other suggested fixes.

Change-Id: I380e3620e5cbe2bb375f7df171944baacb645491
Signed-off-by: frank vibrans <frank.vibrans at scarletltd.com>
---
M src/cpu/x86/smm/smmhandler.S
M src/include/cpu/amd/amdfam15.h
M src/soc/amd/stoneyridge/Kconfig
M src/soc/amd/stoneyridge/include/soc/smi.h
M src/soc/amd/stoneyridge/model_15_init.c
M src/soc/amd/stoneyridge/smi.c
M src/soc/amd/stoneyridge/smihandler.c
7 files changed, 41 insertions(+), 44 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/21020/1

diff --git a/src/cpu/x86/smm/smmhandler.S b/src/cpu/x86/smm/smmhandler.S
index 0617a0d..cf520e0 100644
--- a/src/cpu/x86/smm/smmhandler.S
+++ b/src/cpu/x86/smm/smmhandler.S
@@ -143,7 +143,7 @@
 	 */
 #if IS_ENABLED(CONFIG_CPU_AMD_AGESA_FAMILY15_TN) \
 	|| IS_ENABLED(CONFIG_CPU_AMD_AGESA_FAMILY15_RL) \
-	|| IS_ENABLED(CONFIG_CPU_AMD_AGESA_BINARY_PI)
+	|| IS_ENABLED(CONFIG_SOC_AMD_PI)
 	/* LAPIC IDs start from 0x10; map that to the proper core index */
 	subl $0x10, %ecx
 #endif
diff --git a/src/include/cpu/amd/amdfam15.h b/src/include/cpu/amd/amdfam15.h
index 37fbc49..bc263e7 100644
--- a/src/include/cpu/amd/amdfam15.h
+++ b/src/include/cpu/amd/amdfam15.h
@@ -20,7 +20,9 @@
 
 #define MCI_STATUS			0x00000401
 #define MSR_SMM_BASE			0xC0010111
+#define MSR_TSEG_BASE			0xC0010112
 #define MSR_SMM_MASK			0xC0010113
+#define MSR_TSEG_MASK			0xC0010113
 #define HWCR_MSR			0xC0010015
 #define NB_CFG_MSR			0xC001001f
 
@@ -37,6 +39,19 @@
 
 #define CORE_PERF_BOOST_CTRL		0x15C
 
+#define SMM_LOCK_BIT			(1 << 0)
+#define SMM_ASEG_VALID			(1 << 0)
+#define SMM_TSEG_VALID			(1 << 1)
+#define SMM_ASEG_WB			(6 << 8)
+#define SMM_TSEG_WB			(6 << 12)
+#define SMM_SAVE_STATE_SIZE		0x400
+
+#define ENABLE_CF8_EXTCFG		(1 << (46 - 32))
+
+#define CPUID_FN_8K_8			0x80000008
+#define CPUID_FEAT_HTT			(1 << 28)
+#define CPUID_EXT_FEAT_CMP_LEGACY	(1 << (33 - 32))
+
 #if defined(__PRE_RAM__)
 void wait_all_core0_started(void);
 void wait_all_other_cores_started(u32 bsp_apicid);
diff --git a/src/soc/amd/stoneyridge/Kconfig b/src/soc/amd/stoneyridge/Kconfig
index 19aa1b2..2af58e8 100644
--- a/src/soc/amd/stoneyridge/Kconfig
+++ b/src/soc/amd/stoneyridge/Kconfig
@@ -49,6 +49,7 @@
 	select SOC_AMD_COMMON_BLOCK_CAR
 	select C_ENVIRONMENT_BOOTBLOCK
 	select BOOTBLOCK_CONSOLE
+	select HAVE_SMI_HANDLER
 
 config VBOOT
 	select AMDFW_OUTSIDE_CBFS
@@ -280,10 +281,6 @@
 	  controller 0 registers range from FEDC_6000h
 	  to FEDC_6FFFh. UART controller 1 registers
 	  range from FEDC_8000h to FEDC_8FFFh.
-
-config HAVE_SMI_HANDLER
-	bool "Enable SMM and SMI handling"
-	default y
 
 config SMM_TSEG_SIZE
 	hex
diff --git a/src/soc/amd/stoneyridge/include/soc/smi.h b/src/soc/amd/stoneyridge/include/soc/smi.h
index 016ea4a..72c8d80 100644
--- a/src/soc/amd/stoneyridge/include/soc/smi.h
+++ b/src/soc/amd/stoneyridge/include/soc/smi.h
@@ -15,6 +15,7 @@
  * polluting the namesace.
  */
 #define SMI_BASE		0xfed80200
+#define SMM_SAVE_STATE_SIZE	0x400
 
 #define SMI_REG_SMITRIG0	0x98
 #define SMITRG0_EOS		(1 << 28)
@@ -25,19 +26,14 @@
 #define SMI_REG_SMISTAT0	0x80
 #define SMI_REG_SMISTAT1	0x84
 #define SMI_REG_SMISTAT2	0x88
-#define SMI_REG_SMISTAT3	0x8C
+#define SMI_REG_SMISTAT3	0x8c
 #define SMI_REG_SMISTAT4	0x90
 
 #define SMI_REG_CONTROL0	0xa0
 
-#define SMI_REG_SMICTRL2	0xA8		/* Routes FakeSMIs reported in SMIx84 */
-#define SMICTRL2_FAKES_EN	0x00000054	/* Enable FakeSMIs reported in SMIx84 */
-
-#define SMI_REG_SMICTRL8	0xC0		/* Routes FakeSMIs reported in SMIx90 */
-#define SMICTRL8_FAKES_EN	0x01500000	/* Enable FakeSMIs reported in SMIx90 */
-
-#define SMI_REG_SMICTRL9	0xC4		/* Routes trap events reported in SMIx90 */
-#define SMICTRL9_MTRAP_EN	0x00010000	/* Enable memory trap SMI reported in SMIx90 */
+#define SMI_REG_SMICTRL2	0xa8		/* SMIs reported in SMIx84 */
+#define SMI_REG_SMICTRL8	0xc0		/* SMIs reported in SMIx90 */
+#define SMI_REG_SMICTRL9	0xc4		/* SMIs reported in SMIx90 */
 
 enum smi_mode {
 	SMI_MODE_DISABLE = 0,
diff --git a/src/soc/amd/stoneyridge/model_15_init.c b/src/soc/amd/stoneyridge/model_15_init.c
index 6003bef..06bb48a 100644
--- a/src/soc/amd/stoneyridge/model_15_init.c
+++ b/src/soc/amd/stoneyridge/model_15_init.c
@@ -55,9 +55,6 @@
 	u8 i;
 	msr_t msr;
 	int msrno;
-#if IS_ENABLED(CONFIG_HAVE_SMI_HANDLER)
-	unsigned int cpu_idx;
-#endif
 #if IS_ENABLED(CONFIG_LOGICAL_CPUS)
 	u32 siblings;
 #endif
@@ -97,15 +94,15 @@
 	setup_lapic();
 
 #if IS_ENABLED(CONFIG_LOGICAL_CPUS)
-	siblings = cpuid_ecx(0x80000008) & 0xff;
+	siblings = cpuid_ecx(CPUID_FN_8K_8) & 0xff;
 
 	if (siblings > 0) {
 		msr = rdmsr_amd(CPU_ID_FEATURES_MSR);
-		msr.lo |= 1 << 28;
+		msr.lo |= CPUID_FEAT_HTT;
 		wrmsr_amd(CPU_ID_FEATURES_MSR, msr);
 
 		msr = rdmsr_amd(CPU_ID_EXT_FEATURES_MSR);
-		msr.hi |= 1 << (33 - 32);
+		msr.hi |= CPUID_EXT_FEAT_CMP_LEGACY;
 		wrmsr_amd(CPU_ID_EXT_FEATURES_MSR, msr);
 	}
 	printk(BIOS_DEBUG, "siblings = %02d, ", siblings);
@@ -114,28 +111,28 @@
 
 	/* DisableCf8ExtCfg */
 	msr = rdmsr(NB_CFG_MSR);
-	msr.hi &= ~(1 << (46 - 32));
+	msr.hi &= ~ENABLE_CF8_EXTCFG;
 	wrmsr(NB_CFG_MSR, msr);
 
 
 	if (IS_ENABLED(CONFIG_HAVE_SMI_HANDLER)) {
-		cpu_idx = cpu_info()->index;
+		u32 cpu_idx = cpu_index();
 		printk(BIOS_INFO, "Initializing SMM for CPU %u\n", cpu_idx);
 
 		/* Set SMM base address for this CPU */
 		msr = rdmsr(MSR_SMM_BASE);
-		msr.lo = SMM_BASE - (cpu_idx * 0x400);
+		msr.lo = SMM_BASE - (cpu_idx * SMM_SAVE_STATE_SIZE);
 		wrmsr(MSR_SMM_BASE, msr);
 
-		/* Enable the SMM memory window */
+		/* Enable the SMM ASEG as read, write, WB */
 		msr = rdmsr(MSR_SMM_MASK);
-		msr.lo |= ((1 << 0) | (6 << 8)); /* Enable ASEG SMRAM Range as WB */
+		msr.lo |= SMM_ASEG_VALID | SMM_ASEG_WB;
 		wrmsr(MSR_SMM_MASK, msr);
 	}
 
 	/* Write protect SMM space with SMMLOCK. */
 	msr = rdmsr(HWCR_MSR);
-	msr.lo |= (1 << 0);
+	msr.lo |= SMM_LOCK_BIT;
 	wrmsr(HWCR_MSR, msr);
 }
 
diff --git a/src/soc/amd/stoneyridge/smi.c b/src/soc/amd/stoneyridge/smi.c
index e8c1180..a3d1153 100644
--- a/src/soc/amd/stoneyridge/smi.c
+++ b/src/soc/amd/stoneyridge/smi.c
@@ -13,17 +13,16 @@
 #include <cpu/x86/lapic.h>
 #include <cpu/x86/msr.h>
 #include <cpu/x86/mtrr.h>
-#include <cpu/amd/mtrr.h>
-#include <cpu/amd/msr.h>
 #include <cpu/x86/cache.h>
 #include <cpu/x86/smm.h>
+#include <cpu/amd/mtrr.h>
+#include <cpu/amd/msr.h>
 #include <soc/smi.h>
 #include <string.h>
 
-
 void smm_setup_structures(void *gnvs, void *tcg, void *smi1)
 {
-	printk(BIOS_DEBUG, "smm_setup_structures - STUB.\n");
+	printk(BIOS_DEBUG, "%s - STUB.\n", __func__);
 }
 
 /** Set the EOS bit and enable SMI generation from southbridge */
@@ -35,13 +34,12 @@
 	smi_write32(SMI_REG_SMITRIG0, reg);
 }
 
-/* Sets up ASEG MTRR and copies the SMM code to the ASEG. */
+/* Set up ASEG MTRR and copy the SMM code to the ASEG */
 void smm_init(void)
 {
-
 	msr_t msr, syscfg_orig, mtrr_aseg_orig;
 
-	printk(BIOS_DEBUG, "SMM_mem_init\n");
+	printk(BIOS_DEBUG, "%s\n", __func__);
 
 	/* Back up MSRs for later restore */
 	syscfg_orig = rdmsr(SYSCFG_MSR);
@@ -54,25 +52,22 @@
 
 	/* Allow changes to MTRR extended attributes */
 	msr.lo |= SYSCFG_MSR_MtrrFixDramModEn;
-	/* turn the extended attributes off until we fix
-	 * them so A0000 is routed to memory
-	 */
 	msr.lo &= ~SYSCFG_MSR_MtrrFixDramEn;
 	wrmsr(SYSCFG_MSR, msr);
 
-	/* set DRAM access to 0xa0000-0xbffff to read, write, UC */
+	/* Set ASEG access to read, write, UC */
 	msr.hi = msr.lo = 0x18181818;
 	wrmsr(MTRR_FIX_16K_A0000, msr);
 
-	/* enable the extended features */
+	/* Enable the extended features */
 	msr = syscfg_orig;
-	msr.lo |= SYSCFG_MSR_MtrrFixDramModEn;
+	msr.lo &= ~SYSCFG_MSR_MtrrFixDramModEn;
 	msr.lo |= SYSCFG_MSR_MtrrFixDramEn;
 	wrmsr(SYSCFG_MSR, msr);
 
 	enable_cache();
 
-	/* copy the real SMM handler */
+	/* Copy the real SMM handler */
 	memcpy((void *)SMM_BASE, _binary_smm_start,
 		_binary_smm_end - _binary_smm_start);
 	wbinvd();
@@ -84,7 +79,4 @@
 	wrmsr(MTRR_FIX_16K_A0000, mtrr_aseg_orig);
 
 	enable_cache();
-
-	/* CPU MSR are set in CPU init */
 }
-
diff --git a/src/soc/amd/stoneyridge/smihandler.c b/src/soc/amd/stoneyridge/smihandler.c
index cb67801..2932026 100644
--- a/src/soc/amd/stoneyridge/smihandler.c
+++ b/src/soc/amd/stoneyridge/smihandler.c
@@ -133,7 +133,7 @@
 }
 
 void southbridge_smi_handler(unsigned int node,
-	smm_state_save_area_t *state_save)
+			     smm_state_save_area_t *state_save)
 {
 	const uint16_t smi_src = smi_read16(0x94);
 

-- 
To view, visit https://review.coreboot.org/21020
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I380e3620e5cbe2bb375f7df171944baacb645491
Gerrit-Change-Number: 21020
Gerrit-PatchSet: 1
Gerrit-Owner: frank vibrans <frank.vibrans at scarletltd.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20170815/c9281fe7/attachment.html>


More information about the coreboot-gerrit mailing list