Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35783 )
Change subject: [WIP]sb/intel/common: Handle legacy save states properly ......................................................................
[WIP]sb/intel/common: Handle legacy save states properly
This moves southbridge_update_gnvs to a common place.
Older 32bit only CPU's have a different SMM save state, requiring a different handling of those.
Change-Id: I08391d4ee361ce75507c9de508cd75cc07249fd7 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/intel/smm/gen1/smmrelocate.c M src/include/cpu/intel/em64t101_save_state.h M src/southbridge/intel/bd82x6x/smihandler.c M src/southbridge/intel/common/pmutil.h M src/southbridge/intel/common/smihandler.c M src/southbridge/intel/i82801gx/smihandler.c M src/southbridge/intel/i82801ix/smihandler.c M src/southbridge/intel/i82801jx/smihandler.c M src/southbridge/intel/ibexpeak/smihandler.c 9 files changed, 140 insertions(+), 75 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/35783/1
diff --git a/src/cpu/intel/smm/gen1/smmrelocate.c b/src/cpu/intel/smm/gen1/smmrelocate.c index 3eb869a..5350d1c 100644 --- a/src/cpu/intel/smm/gen1/smmrelocate.c +++ b/src/cpu/intel/smm/gen1/smmrelocate.c @@ -168,6 +168,9 @@ if (smm_reloc_params.ied_size) setup_ied_area(&smm_reloc_params);
+ /* This may not be be correct for older CPU's supported by this code, + but given that em64t101_smm_state_save_area_t is larger than the + save_state of these CPU's it works. */ *smm_save_state_size = sizeof(em64t101_smm_state_save_area_t); }
@@ -191,6 +194,8 @@ { msr_t mtrr_cap; struct smm_relocation_params *relo_params = &smm_reloc_params; + /* The em64t101 save state is sufficiently compatible with older + save states with regards of smbase, smm_revision. */ em64t101_smm_state_save_area_t *save_state; u32 smbase = staggered_smbase; u32 iedbase = relo_params->ied_base; @@ -208,7 +213,10 @@ save_state = (void *)(curr_smbase + SMM_DEFAULT_SIZE - sizeof(*save_state)); save_state->smbase = smbase; - save_state->iedbase = iedbase; + + printk(BIOS_SPEW, "SMM revision: 0x%08x\n", save_state->smm_revision); + if (save_state->smm_revision == 0x00030101) + save_state->iedbase = iedbase;
/* Write EMRR and SMRR MSRs based on indicated support. */ mtrr_cap = rdmsr(MTRR_CAP_MSR); diff --git a/src/include/cpu/intel/em64t101_save_state.h b/src/include/cpu/intel/em64t101_save_state.h index b8bb2db..7493c85 100644 --- a/src/include/cpu/intel/em64t101_save_state.h +++ b/src/include/cpu/intel/em64t101_save_state.h @@ -20,6 +20,7 @@
/* Intel Revision 30101 SMM State-Save Area * The following processor architectures use this: + * - Nehalem * - SandyBridge * - IvyBridge * - Haswell diff --git a/src/southbridge/intel/bd82x6x/smihandler.c b/src/southbridge/intel/bd82x6x/smihandler.c index ceac598..fe4f80e 100644 --- a/src/southbridge/intel/bd82x6x/smihandler.c +++ b/src/southbridge/intel/bd82x6x/smihandler.c @@ -21,7 +21,6 @@ #include <cpu/x86/cache.h> #include <device/pci_def.h> #include <cpu/x86/smm.h> -#include <cpu/intel/em64t101_save_state.h> #include <northbridge/intel/sandybridge/sandybridge.h> #include <southbridge/intel/bd82x6x/me.h> #include <southbridge/intel/common/gpio.h> @@ -38,6 +37,11 @@ return gnvs; }
+void smm_set_gnvs(uintptr_t gnvs_ptr) +{ + gnvs = (global_nvs_t *)gnvs_ptr; +} + int southbridge_io_trap_handler(int smif) { switch (smif) { @@ -211,18 +215,6 @@ xhci_sleep(slp_type); }
-void southbridge_update_gnvs(u8 apm_cnt, int *smm_done) -{ - em64t101_smm_state_save_area_t *state = - smi_apmc_find_state_save(apm_cnt); - if (state) { - /* EBX in the state save contains the GNVS pointer */ - gnvs = (global_nvs_t *)((u32)state->rbx); - *smm_done = 1; - printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs); - } -} - void southbridge_finalize_all(void) { intel_me_finalize_smm(); diff --git a/src/southbridge/intel/common/pmutil.h b/src/southbridge/intel/common/pmutil.h index cea5c82..b2f1dc4 100644 --- a/src/southbridge/intel/common/pmutil.h +++ b/src/southbridge/intel/common/pmutil.h @@ -18,7 +18,9 @@ #define INTEL_COMMON_PMUTIL_H
#include <cpu/x86/smm.h> +#include <cpu/x86/legacy_save_state.h> #include <cpu/intel/em64t101_save_state.h> +#include <cpu/intel/em64t_save_state.h>
#define D31F0_PMBASE 0x40 #define D31F0_GEN_PMCON_1 0xa0 @@ -142,10 +144,11 @@ void southbridge_smm_xhci_sleep(u8 slp_type); void gpi_route_interrupt(u8 gpi, u8 mode); void southbridge_gate_memory_reset(void); -void southbridge_update_gnvs(u8 apm_cnt, int *smm_done); +void smm_set_gnvs_ptr(uintptr_t gnvs); void southbridge_finalize_all(void); void southbridge_smi_monitor(void); -em64t101_smm_state_save_area_t *smi_apmc_find_state_save(u8 cmd); + +void *smi_apmc_find_state_save(u8 cmd); void pch_log_state(void);
#endif /*INTEL_COMMON_PMUTIL_H */ diff --git a/src/southbridge/intel/common/smihandler.c b/src/southbridge/intel/common/smihandler.c index d61238c..731856a 100644 --- a/src/southbridge/intel/common/smihandler.c +++ b/src/southbridge/intel/common/smihandler.c @@ -210,37 +210,77 @@ } }
+enum save_state_type { + EM64T, + EM64T101, + LEGACY +}; + +static enum save_state_type get_save_state_type(void) +{ + /* Is geting this on node 0 fine??? */ + void *save_state = smm_get_save_state(0); + u32 smm_revision = ((em64t101_smm_state_save_area_t *)state)->smm_revision; + switch (smm_revision) { + case 0x00030002: + case 0x00030007: + return LEGACY; + case 0x00030100: + return EM64T; + default: + printk(BIOS_WARNING, "Unknown smm_revision...\n"); + /* Fallthrough */ + case 0x00030101: /* Nahalem, SandyBridge, IvyBridge, and Haswell */ + return EM64T101; + } +} + /* * Look for Synchronous IO SMI and use save state from that * core in case we are not running on the same core that * initiated the IO transaction. */ -em64t101_smm_state_save_area_t *smi_apmc_find_state_save(u8 cmd) +void *smi_apmc_find_state_save(u8 cmd) { - em64t101_smm_state_save_area_t *state; int node; + enum save_state_type save_state_type = get_save_state_type();
- /* Check all nodes looking for the one that issued the IO */ - for (node = 0; node < CONFIG_MAX_CPUS; node++) { - state = smm_get_save_state(node); + if (save_state_type == EM64T || save_state_type == EM64T101) { + /* Check all nodes looking for the one that issued the IO. + TODO: Check against actual amount of CPUs!! */ + for (node = 0; node < CONFIG_MAX_CPUS; node++) { + em64t101_smm_state_save_area_t *state = + (em64t101_smm_state_save_area_t *)smm_get_save_state(node);
- /* Check for Synchronous IO (bit0 == 1) */ - if (!(state->io_misc_info & (1 << 0))) - continue; + /* Check for Synchronous IO (bit0 == 1) */ + if (!(state->io_misc_info & (1 << 0))) + continue;
- /* Make sure it was a write (bit4 == 0) */ - if (state->io_misc_info & (1 << 4)) - continue; + /* Make sure it was a write (bit4 == 0) */ + if (state->io_misc_info & (1 << 4)) + continue;
- /* Check for APMC IO port */ - if (((state->io_misc_info >> 16) & 0xff) != APM_CNT) - continue; + /* Check for APMC IO port */ + if (((state->io_misc_info >> 16) & 0xff) != APM_CNT) + continue;
- /* Check AX against the requested command */ - if ((state->rax & 0xff) != cmd) - continue; + /* Check AX against the requested command */ + if ((state->rax & 0xff) != cmd) + continue;
- return state; + return state; + } + } + + if (save_state_type == LEGACY) { + for (node = 0; node < CONFIG_MAX_CPUS; node++) { + legacy_smm_state_save_area_t *state = + (legacy_smm_state_save_area_t *)smm_get_save_state(node); + + /* Check AX against the requested command */ + if ((state->eax & 0xff) == cmd) + return state; + } }
return NULL; @@ -272,23 +312,62 @@ static void southbridge_smi_store(void) { u8 sub_command, ret; - em64t101_smm_state_save_area_t *io_smi = - smi_apmc_find_state_save(APM_CNT_SMMSTORE); + void *state = smi_apmc_find_state_save(APM_CNT_SMMSTORE); uint32_t reg_ebx;
- if (!io_smi) + if (!state) return; /* Command and return value in EAX */ - sub_command = (io_smi->rax >> 8) & 0xff; + switch (get_save_state_type()) { + case EM64T: + case EM64T101: + sub_command = ((u32)((em64t101_smm_state_save_area_t *)state)->rax >> 8) & 0xff;
- /* Parameter buffer in EBX */ - reg_ebx = io_smi->rbx; + reg_ebx = (u32)((em64t101_smm_state_save_area_t *)state)->rbx; + break; + case LEGACY: + sub_command = (((legacy_smm_state_save_area_t *)state)->eax >> 8) & 0xff; + reg_ebx = ((legacy_smm_state_save_area_t *)state)->ebx; + break; + }
- /* drivers/smmstore/smi.c */ + /* drivers/smmstore/smi.c, parameter buffer in EBX */ ret = smmstore_exec(sub_command, (uintptr_t *)reg_ebx); - io_smi->rax = ret; + + switch (get_save_state_type()) { + case EM64T: + case EM64T101: + ((em64t101_smm_state_save_area_t *)state)->rax = ret; + break; + case LEGACY: + ((legacy_smm_state_save_area_t *)state)->eax = ret; + break; + } }
+static void southbridge_update_gnvs(u8 apm_cnt, int *smm_done) +{ + uintptr_t gnvs = 0; + void *state = smi_apmc_find_state_save(apm_cnt); + if (state) { + switch (get_save_state_type()) { + case EM64T: + case EM64T101: + /* EBX in the state save contains the GNVS pointer */ + gnvs = (u32)((em64t101_smm_state_save_area_t *)state)->rbx; + break; + case LEGACY: + gnvs = ((legacy_smm_state_save_area_t *)state)->ebx; + break; + } + *smm_done = 1; + printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs); + } + if (gnvs) + smm_set_gnvs_ptr(gnvs); +} + + static int mainboard_finalized = 0;
static void southbridge_smi_apmc(void) diff --git a/src/southbridge/intel/i82801gx/smihandler.c b/src/southbridge/intel/i82801gx/smihandler.c index 16ceb13..fccc67d 100644 --- a/src/southbridge/intel/i82801gx/smihandler.c +++ b/src/southbridge/intel/i82801gx/smihandler.c @@ -41,12 +41,11 @@ /* GNVS needs to be updated by an 0xEA PM Trap (B2) after it has been located * by coreboot. */ -global_nvs_t *gnvs = (global_nvs_t *)0x0; +static global_nvs_t *gnvs;
-void southbridge_update_gnvs(u8 apm_cnt, int *smm_done) +void smm_set_gnvs_ptr(uintptr_t gnvs_ptr) { - gnvs = *(global_nvs_t **)0x500; - *smm_done = 1; + gnvs = (global_nvs_t *)gnvs; }
int southbridge_io_trap_handler(int smif) diff --git a/src/southbridge/intel/i82801ix/smihandler.c b/src/southbridge/intel/i82801ix/smihandler.c index 8090a09..7135425 100644 --- a/src/southbridge/intel/i82801ix/smihandler.c +++ b/src/southbridge/intel/i82801ix/smihandler.c @@ -28,9 +28,7 @@ /* GNVS needs to be updated by an 0xEA PM Trap (B2) after it has been located * by coreboot. */ -global_nvs_t *gnvs = (global_nvs_t *)0x0; -void *tcg = (void *)0x0; -void *smi1 = (void *)0x0; +static global_nvs_t *gnvs;
int southbridge_io_trap_handler(int smif) { @@ -49,12 +47,9 @@ return 0; }
-void southbridge_update_gnvs(u8 apm_cnt, int *smm_done) +void smm_set_gnvs_ptr(uintptr_t gnvs_ptr) { - gnvs = *(global_nvs_t **)0x500; - tcg = *(void **)0x504; - smi1 = *(void **)0x508; - *smm_done = 1; + gnvs = (global_nvs_t *)gnvs; }
void southbridge_smi_monitor(void) diff --git a/src/southbridge/intel/i82801jx/smihandler.c b/src/southbridge/intel/i82801jx/smihandler.c index 667a853..d9e944d 100644 --- a/src/southbridge/intel/i82801jx/smihandler.c +++ b/src/southbridge/intel/i82801jx/smihandler.c @@ -35,9 +35,7 @@ /* GNVS needs to be updated by an 0xEA PM Trap (B2) after it has been located * by coreboot. */ -global_nvs_t *gnvs = (global_nvs_t *)0x0; -void *tcg = (void *)0x0; -void *smi1 = (void *)0x0; +static global_nvs_t *gnvs;
int southbridge_io_trap_handler(int smif) { @@ -56,12 +54,9 @@ return 0; }
-void southbridge_update_gnvs(u8 apm_cnt, int *smm_done) +void smm_set_gnvs_ptr(uintptr_t gnvs_ptr) { - gnvs = *(global_nvs_t **)0x500; - tcg = *(void **)0x504; - smi1 = *(void **)0x508; - *smm_done = 1; + gnvs = (global_nvs_t *)gnvs; }
void southbridge_smi_monitor(void) diff --git a/src/southbridge/intel/ibexpeak/smihandler.c b/src/southbridge/intel/ibexpeak/smihandler.c index 3668842..cc32127 100644 --- a/src/southbridge/intel/ibexpeak/smihandler.c +++ b/src/southbridge/intel/ibexpeak/smihandler.c @@ -21,7 +21,6 @@ #include <cpu/x86/cache.h> #include <device/pci_def.h> #include <cpu/x86/smm.h> -#include <cpu/intel/em64t101_save_state.h> #include <halt.h> #include <pc80/mc146818rtc.h> #include <cpu/intel/model_2065x/model_2065x.h> @@ -44,11 +43,17 @@ * by coreboot. */ static global_nvs_t *gnvs; + global_nvs_t *smm_get_gnvs(void) { return gnvs; }
+void smm_set_gnvs_ptr(uintptr_t gnvs_ptr) +{ + gnvs = (global_nvs_t *)gnvs; +} + int southbridge_io_trap_handler(int smif) { switch (smif) { @@ -171,18 +176,6 @@ #undef IOTRAP }
-void southbridge_update_gnvs(u8 apm_cnt, int *smm_done) -{ - em64t101_smm_state_save_area_t *state = - smi_apmc_find_state_save(apm_cnt); - if (state) { - /* EBX in the state save contains the GNVS pointer */ - gnvs = (global_nvs_t *)((u32)state->rbx); - *smm_done = 1; - printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs); - } -} - void southbridge_finalize_all(void) { intel_me_finalize_smm();