Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36590 )
Change subject: [UNTESTED]sb/intel/common: Deal with different save states ......................................................................
[UNTESTED]sb/intel/common: Deal with different save states
Some CPUs have different save states and have to be dealt with accordingly.
updating the gnvs pointer is now done in a common place and fixes this on i82801xx targets.
Change-Id: I63f77432cf4a2ef484ad320bfc321e9a923a19ab Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- 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 7 files changed, 47 insertions(+), 56 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/36590/1
diff --git a/src/southbridge/intel/bd82x6x/smihandler.c b/src/southbridge/intel/bd82x6x/smihandler.c index ceac598..c4f1efb 100644 --- a/src/southbridge/intel/bd82x6x/smihandler.c +++ b/src/southbridge/intel/bd82x6x/smihandler.c @@ -211,16 +211,9 @@ xhci_sleep(slp_type); }
-void southbridge_update_gnvs(u8 apm_cnt, int *smm_done) +void set_gnvs_ptr(uintptr_t offset) { - 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); - } + gnvs = (global_nvs_t *)offset; }
void southbridge_finalize_all(void) diff --git a/src/southbridge/intel/common/pmutil.h b/src/southbridge/intel/common/pmutil.h index cea5c82..a5cd921 100644 --- a/src/southbridge/intel/common/pmutil.h +++ b/src/southbridge/intel/common/pmutil.h @@ -142,10 +142,10 @@ 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 set_gnvs_ptr(uintptr_t ptr); void southbridge_finalize_all(void); void southbridge_smi_monitor(void); -em64t101_smm_state_save_area_t *smi_apmc_find_state_save(u8 cmd); +int 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 5582051..e85031d 100644 --- a/src/southbridge/intel/common/smihandler.c +++ b/src/southbridge/intel/common/smihandler.c @@ -215,13 +215,17 @@ * 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) +int smi_apmc_find_state_save(u8 cmd) { em64t101_smm_state_save_area_t *state; int node;
/* Check all nodes looking for the one that issued the IO */ for (node = 0; node < CONFIG_MAX_CPUS; node++) { + if (get_save_state_type() == LEGACY) + if (((*(uintptr_t *)save_state_reg(node, SS_EAX)) & 0xff) != cmd) + continue; + state = smm_get_save_state(node);
/* Check for Synchronous IO (bit0 == 1) */ @@ -240,10 +244,11 @@ if ((state->rax & 0xff) != cmd) continue;
- return state; + return node; }
- return NULL; + printk(BIOS_WARNING, "APMC node not found!\n"); + return -1; }
#if CONFIG(ELOG_GSMI) @@ -251,18 +256,17 @@ { u32 *ret, *param; u8 sub_command; - em64t101_smm_state_save_area_t *io_smi = - smi_apmc_find_state_save(APM_CNT_ELOG_GSMI); + int apm_node = smi_apmc_find_state_save(APM_CNT_ELOG_GSMI);
- if (!io_smi) + if (apm_node < 0) return;
/* Command and return value in EAX */ - ret = (u32*)&io_smi->rax; + ret = (u32*)save_state_reg(apm_node, SS_EAX); sub_command = (u8)(*ret >> 8);
/* Parameter buffer in EBX */ - param = (u32*)&io_smi->rbx; + param = (u32*)save_state_reg(apm_node, SS_EBX);
/* drivers/elog/gsmi.c */ *ret = gsmi_exec(sub_command, param); @@ -272,21 +276,33 @@ 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); - uintptr_t reg_rbx; + int apm_node = smi_apmc_find_state_save(APM_CNT_SMMSTORE); + void *reg_rbx;
- if (!io_smi) + if (apm_node < 0) return; /* Command and return value in EAX */ - sub_command = (io_smi->rax >> 8) & 0xff; + sub_command = (*(uintptr_t *)save_state_reg(apm_node, SS_EAX) >> 8) & 0xff;
/* Parameter buffer in EBX */ - reg_rbx = (uintptr_t)io_smi->rbx; + reg_rbx = save_state_reg(apm_node, SS_EBX);
/* drivers/smmstore/smi.c */ - ret = smmstore_exec(sub_command, (void *)reg_rbx); - io_smi->rax = ret; + ret = smmstore_exec(sub_command, reg_rbx); + *(uintptr_t *)save_state_reg(apm_node, SS_EAX) = ret; +} + +static void southbridge_update_gnvs(u8 apm_cnt, int *smm_done) +{ + int apm_node = smi_apmc_find_state_save(apm_cnt); + uintptr_t gnvs; + if (apm_node > 0) { + /* EBX in the state save contains the GNVS pointer */ + gnvs = *(uintptr_t *)save_state_reg(apm_node, SS_EBX); + set_gnvs_ptr(gnvs); + *smm_done = 1; + printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs); + } }
static int mainboard_finalized = 0; diff --git a/src/southbridge/intel/i82801gx/smihandler.c b/src/southbridge/intel/i82801gx/smihandler.c index 16ceb13..c697cf5 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 set_gnvs_ptr(uintptr_t offset) { - gnvs = *(global_nvs_t **)0x500; - *smm_done = 1; + gnvs = (global_nvs_t *)offset; }
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..2edba39 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 set_gnvs_ptr(uintptr_t offset) { - gnvs = *(global_nvs_t **)0x500; - tcg = *(void **)0x504; - smi1 = *(void **)0x508; - *smm_done = 1; + gnvs = (global_nvs_t *)offset; }
void southbridge_smi_monitor(void) diff --git a/src/southbridge/intel/i82801jx/smihandler.c b/src/southbridge/intel/i82801jx/smihandler.c index 667a853..69a9e7a 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 set_gnvs_ptr(uintptr_t offset) { - gnvs = *(global_nvs_t **)0x500; - tcg = *(void **)0x504; - smi1 = *(void **)0x508; - *smm_done = 1; + gnvs = (global_nvs_t *)offset; }
void southbridge_smi_monitor(void) diff --git a/src/southbridge/intel/ibexpeak/smihandler.c b/src/southbridge/intel/ibexpeak/smihandler.c index 3668842..961d207 100644 --- a/src/southbridge/intel/ibexpeak/smihandler.c +++ b/src/southbridge/intel/ibexpeak/smihandler.c @@ -171,16 +171,9 @@ #undef IOTRAP }
-void southbridge_update_gnvs(u8 apm_cnt, int *smm_done) +void set_gnvs_ptr(uintptr_t offset) { - 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); - } + gnvs = (global_nvs_t *)offset; }
void southbridge_finalize_all(void)
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36590 )
Change subject: [UNTESTED]sb/intel/common: Deal with different save states ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36590/1/src/southbridge/intel/commo... File src/southbridge/intel/common/smihandler.c:
https://review.coreboot.org/c/coreboot/+/36590/1/src/southbridge/intel/commo... PS1, Line 265: ret = (u32*)save_state_reg(apm_node, SS_EAX); "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/36590/1/src/southbridge/intel/commo... PS1, Line 269: param = (u32*)save_state_reg(apm_node, SS_EBX); "(foo*)" should be "(foo *)"
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36590
to look at the new patch set (#2).
Change subject: [UNTESTED]sb/intel/common: Deal with different save states ......................................................................
[UNTESTED]sb/intel/common: Deal with different save states
Some CPUs have different save states and have to be dealt with accordingly.
updating the gnvs pointer is now done in a common place and fixes this on i82801xx targets.
Change-Id: I63f77432cf4a2ef484ad320bfc321e9a923a19ab Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- 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 7 files changed, 47 insertions(+), 56 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/36590/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36590 )
Change subject: [UNTESTED]sb/intel/common: Deal with different save states ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36590/2/src/southbridge/intel/commo... File src/southbridge/intel/common/smihandler.c:
https://review.coreboot.org/c/coreboot/+/36590/2/src/southbridge/intel/commo... PS2, Line 265: ret = (u32*)save_state_reg(apm_node, SS_EAX); "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/36590/2/src/southbridge/intel/commo... PS2, Line 269: param = (u32*)save_state_reg(apm_node, SS_EBX); "(foo*)" should be "(foo *)"
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36590
to look at the new patch set (#3).
Change subject: [UNTESTED]sb/intel/common: Deal with different save states ......................................................................
[UNTESTED]sb/intel/common: Deal with different save states
Some CPUs have different save states and have to be dealt with accordingly.
updating the gnvs pointer is now done in a common place and fixes this on i82801xx targets.
Change-Id: I63f77432cf4a2ef484ad320bfc321e9a923a19ab Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- 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 7 files changed, 47 insertions(+), 56 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/36590/3
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36590
to look at the new patch set (#4).
Change subject: [UNTESTED]sb/intel/common: Deal with different save states ......................................................................
[UNTESTED]sb/intel/common: Deal with different save states
Some CPUs have different save states and have to be dealt with accordingly.
updating the gnvs pointer is now done in a common place and fixes this on i82801xx targets.
Change-Id: I63f77432cf4a2ef484ad320bfc321e9a923a19ab Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- 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 7 files changed, 47 insertions(+), 56 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/36590/4
Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/36590 )
Change subject: [UNTESTED]sb/intel/common: Deal with different save states ......................................................................
Abandoned
code is in common now
-
Arthur Heymans (Code Review) -
build bot (Jenkins) (Code Review)