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@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);
-
frank vibrans (Code Review)