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