Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier.
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83067?usp=email )
Change subject: cpu/x86/smm: Move smmstore entry to a common place ......................................................................
cpu/x86/smm: Move smmstore entry to a common place
Now that SMM save states are handled in a common place so can the SMMSTORE calls entry.
Change-Id: Ieb8d01a2d33e9214c3df7f909dcfab49a05fffff Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/drivers/smmstore/smi.c M src/include/smmstore.h M src/soc/amd/common/block/cpu/smm/smi_apmc.c M src/soc/amd/stoneyridge/smihandler.c M src/soc/intel/baytrail/smihandler.c M src/soc/intel/braswell/smihandler.c M src/soc/intel/broadwell/pch/smihandler.c M src/soc/intel/common/block/smm/smihandler.c M src/southbridge/intel/common/smihandler.c M src/southbridge/intel/lynxpoint/smihandler.c 10 files changed, 34 insertions(+), 233 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/83067/1
diff --git a/src/drivers/smmstore/smi.c b/src/drivers/smmstore/smi.c index 4f7b4bd..c95ed82 100644 --- a/src/drivers/smmstore/smi.c +++ b/src/drivers/smmstore/smi.c @@ -6,6 +6,7 @@ #include <smmstore.h> #include <stddef.h> #include <stdint.h> +#include <cpu/x86/save_state.h>
/* * Check that the given range is legal. @@ -136,13 +137,33 @@ return ret; }
-uint32_t smmstore_exec(uint8_t command, void *param) +void apmc_smmstore_handler(void) { + uint8_t command; + uint16_t ax; + uint32_t ret, param; + const int apmc_node = get_apmc_node(APM_CNT_SMMSTORE); + + if (apmc_node < 0) + return; + + /* Command and return value in EAX */ + if (get_save_state_reg(RAX, apmc_node, &ax, sizeof(ax))) + return; + + command = (ax >> 8); + + /* Parameter buffer in EBX */ + if (get_save_state_reg(RBX, apmc_node, ¶m, sizeof(param))) + return; + if (command != SMMSTORE_CMD_CLEAR && !param) return SMMSTORE_RET_FAILURE;
if (CONFIG(SMMSTORE_V2)) - return smmstorev2_exec(command, param); + ret = smmstorev2_exec(command, (void *)param); else - return smmstorev1_exec(command, param); + ret = smmstorev1_exec(command, (void *)param); + + set_save_state_reg(RAX, apmc_node, &ret, sizeof(ret)); } diff --git a/src/include/smmstore.h b/src/include/smmstore.h index 6805cbc..30e714e 100644 --- a/src/include/smmstore.h +++ b/src/include/smmstore.h @@ -100,7 +100,7 @@
/* SMM handler */ -uint32_t smmstore_exec(uint8_t command, void *param); +void apmc_smmstore_handler(void);
/* Implementation of Version 1 */ int smmstore_read_region(void *buf, ssize_t *bufsize); diff --git a/src/soc/amd/common/block/cpu/smm/smi_apmc.c b/src/soc/amd/common/block/cpu/smm/smi_apmc.c index 63bc398..93bc3d1 100644 --- a/src/soc/amd/common/block/cpu/smm/smi_apmc.c +++ b/src/soc/amd/common/block/cpu/smm/smi_apmc.c @@ -17,59 +17,6 @@ #define SMM_IO_TRAP_RW (1 << 0) #define SMM_IO_TRAP_VALID (1 << 1)
-static inline u16 get_io_address(u32 info) -{ - return ((info >> SMM_IO_TRAP_PORT_OFFSET) & - SMM_IO_TRAP_PORT_ADDRESS_MASK); -} - -static void *find_save_state(int cmd) -{ - unsigned int core; - amd64_smm_state_save_area_t *state; - u32 smm_io_trap; - u8 reg_al; - - /* Check all nodes looking for the one that issued the IO */ - for (core = 0; core < CONFIG_MAX_CPUS; core++) { - state = smm_get_save_state(core); - smm_io_trap = state->smm_io_trap_offset; - /* Check for Valid IO Trap Word (bit1==1) */ - if (!(smm_io_trap & SMM_IO_TRAP_VALID)) - continue; - /* Make sure it was a write (bit0==0) */ - if (smm_io_trap & SMM_IO_TRAP_RW) - continue; - /* Check for APMC IO port */ - if (pm_acpi_smi_cmd_port() != get_io_address(smm_io_trap)) - continue; - /* Check AL against the requested command */ - reg_al = state->rax; - if (reg_al == cmd) - return state; - } - return NULL; -} - -void handle_smi_store(void) -{ - u8 sub_command; - amd64_smm_state_save_area_t *io_smi; - u32 reg_ebx; - - io_smi = find_save_state(APM_CNT_SMMSTORE); - if (!io_smi) - return; - /* Command and return value in EAX */ - sub_command = (io_smi->rax >> 8) & 0xff; - - /* Parameter buffer in EBX */ - reg_ebx = io_smi->rbx; - - /* drivers/smmstore/smi.c */ - io_smi->rax = smmstore_exec(sub_command, (void *)(uintptr_t)reg_ebx); -} - void fch_apmc_smi_handler(void) { const uint8_t cmd = apm_get_apmc(); @@ -88,7 +35,7 @@ break; case APM_CNT_SMMSTORE: if (CONFIG(SMMSTORE)) - handle_smi_store(); + apmc_smmstore_handler(); break; case APM_CNT_SMMINFO: psp_notify_smm(); diff --git a/src/soc/amd/stoneyridge/smihandler.c b/src/soc/amd/stoneyridge/smihandler.c index 5c13a57..57337f5 100644 --- a/src/soc/amd/stoneyridge/smihandler.c +++ b/src/soc/amd/stoneyridge/smihandler.c @@ -11,6 +11,7 @@ #include <cpu/x86/cache.h> #include <cpu/x86/smm.h> #include <elog.h> +#include <smmstore.h> #include <soc/smi.h> #include <soc/southbridge.h> #include <types.h> @@ -37,7 +38,7 @@ break; case APM_CNT_SMMSTORE: if (CONFIG(SMMSTORE)) - handle_smi_store(); + apmc_smmstore_handler(); break; }
diff --git a/src/soc/intel/baytrail/smihandler.c b/src/soc/intel/baytrail/smihandler.c index 7a5d222..6ef1adc 100644 --- a/src/soc/intel/baytrail/smihandler.c +++ b/src/soc/intel/baytrail/smihandler.c @@ -229,25 +229,6 @@ LPSS_ACPI_MODE_DISABLE(SPI); }
-static void southbridge_smi_store(void) -{ - u8 sub_command, ret; - em64t100_smm_state_save_area_t *io_smi = smi_apmc_find_state_save(APM_CNT_SMMSTORE); - uint32_t reg_ebx; - - if (!io_smi) - return; - /* Command and return value in EAX */ - sub_command = (io_smi->rax >> 8) & 0xff; - - /* Parameter buffer in EBX */ - reg_ebx = io_smi->rbx; - - /* drivers/smmstore/smi.c */ - ret = smmstore_exec(sub_command, (void *)reg_ebx); - io_smi->rax = ret; -} - static void southbridge_smi_apmc(void) { uint8_t reg8; @@ -269,7 +250,7 @@ break; case APM_CNT_SMMSTORE: if (CONFIG(SMMSTORE)) - southbridge_smi_store(); + apmc_smmstore_handler(); break; }
diff --git a/src/soc/intel/braswell/smihandler.c b/src/soc/intel/braswell/smihandler.c index 940c66d..b7baa61 100644 --- a/src/soc/intel/braswell/smihandler.c +++ b/src/soc/intel/braswell/smihandler.c @@ -199,25 +199,6 @@ return NULL; }
-static void southbridge_smi_store(void) -{ - u8 sub_command, ret; - em64t100_smm_state_save_area_t *io_smi = smi_apmc_find_state_save(APM_CNT_SMMSTORE); - uint32_t reg_ebx; - - if (!io_smi) - return; - /* Command and return value in EAX */ - sub_command = (io_smi->rax >> 8) & 0xff; - - /* Parameter buffer in EBX */ - reg_ebx = io_smi->rbx; - - /* drivers/smmstore/smi.c */ - ret = smmstore_exec(sub_command, (void *)reg_ebx); - io_smi->rax = ret; -} - static void southbridge_smi_apmc(void) { uint8_t reg8; @@ -236,7 +217,7 @@ break; case APM_CNT_SMMSTORE: if (CONFIG(SMMSTORE)) - southbridge_smi_store(); + apmc_smmstore_handler(); break; }
diff --git a/src/soc/intel/broadwell/pch/smihandler.c b/src/soc/intel/broadwell/pch/smihandler.c index 632775b..86bf732 100644 --- a/src/soc/intel/broadwell/pch/smihandler.c +++ b/src/soc/intel/broadwell/pch/smihandler.c @@ -251,26 +251,6 @@ return NULL; }
-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); - uint32_t reg_ebx; - - if (!io_smi) - return; - /* Command and return value in EAX */ - sub_command = (io_smi->rax >> 8) & 0xff; - - /* Parameter buffer in EBX */ - reg_ebx = io_smi->rbx; - - /* drivers/smmstore/smi.c */ - ret = smmstore_exec(sub_command, (void *)reg_ebx); - io_smi->rax = ret; -} - static void southbridge_smi_apmc(void) { u8 reg8; @@ -289,7 +269,7 @@ break; case APM_CNT_SMMSTORE: if (CONFIG(SMMSTORE)) - southbridge_smi_store(); + apmc_smmstore_handler(); break; }
diff --git a/src/soc/intel/common/block/smm/smihandler.c b/src/soc/intel/common/block/smm/smihandler.c index fc26dc4..27d9312 100644 --- a/src/soc/intel/common/block/smm/smihandler.c +++ b/src/soc/intel/common/block/smm/smihandler.c @@ -63,38 +63,6 @@
/* Common Functions */
-static void *find_save_state(const struct smm_save_state_ops *save_state_ops, - int cmd) -{ - int node; - void *state = NULL; - uint32_t io_misc_info; - uint8_t reg_al; - - /* 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); - - io_misc_info = save_state_ops->get_io_misc_info(state); - - /* Check for Synchronous IO (bit0==1) */ - if (!(io_misc_info & (1 << 0))) - continue; - /* Make sure it was a write (bit4==0) */ - if (io_misc_info & (1 << 4)) - continue; - /* Check for APMC IO port */ - if (((io_misc_info >> 16) & 0xff) != APM_CNT) - continue; - /* Check AL against the requested command */ - reg_al = save_state_ops->get_reg(state, RAX); - if (reg_al != cmd) - continue; - break; - } - return state; -} - /* Inherited from cpu/x86/smm.h resulting in a different signature */ void southbridge_smi_set_eos(void) { @@ -256,44 +224,6 @@ wrmsr(MSR_SPCL_CHIPSET_USAGE, msr); }
-static void southbridge_smi_store( - const struct smm_save_state_ops *save_state_ops) -{ - u8 sub_command, ret; - void *io_smi; - uint32_t reg_ebx; - - io_smi = find_save_state(save_state_ops, APM_CNT_SMMSTORE); - if (!io_smi) - return; - /* Command and return value in EAX */ - sub_command = (save_state_ops->get_reg(io_smi, RAX) >> 8) & 0xff; - - /* Parameter buffer in EBX */ - reg_ebx = save_state_ops->get_reg(io_smi, RBX); - - const bool wp_enabled = !fast_spi_wpd_status(); - if (wp_enabled) { - set_insmm_sts(true); - /* - * As per BWG, clearing "SPI_BIOS_CONTROL_SYNC_SS" - * bit is a must prior setting SPI_BIOS_CONTROL_WPD" bit - * to avoid 3-strike error. - */ - fast_spi_clear_sync_smi_status(); - fast_spi_disable_wp(); - } - - /* drivers/smmstore/smi.c */ - ret = smmstore_exec(sub_command, (void *)(uintptr_t)reg_ebx); - save_state_ops->set_reg(io_smi, RAX, ret); - - if (wp_enabled) { - fast_spi_enable_wp(); - set_insmm_sts(false); - } -} - __weak const struct gpio_lock_config *soc_gpio_lock_config(size_t *num) { *num = 0; @@ -366,7 +296,7 @@ break; case APM_CNT_SMMSTORE: if (CONFIG(SMMSTORE)) - southbridge_smi_store(save_state_ops); + apmc_smmstore_handler(); break; case APM_CNT_FINALIZE: finalize(); diff --git a/src/southbridge/intel/common/smihandler.c b/src/southbridge/intel/common/smihandler.c index 42351b0..5ac4b3a 100644 --- a/src/southbridge/intel/common/smihandler.c +++ b/src/southbridge/intel/common/smihandler.c @@ -228,26 +228,6 @@ return NULL; }
-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; - - if (!io_smi) - return; - /* Command and return value in EAX */ - sub_command = (io_smi->rax >> 8) & 0xff; - - /* Parameter buffer in EBX */ - reg_rbx = (uintptr_t)io_smi->rbx; - - /* drivers/smmstore/smi.c */ - ret = smmstore_exec(sub_command, (void *)reg_rbx); - io_smi->rax = ret; -} - static int mainboard_finalized = 0;
static void southbridge_smi_apmc(void) @@ -277,7 +257,7 @@ break; case APM_CNT_SMMSTORE: if (CONFIG(SMMSTORE)) - southbridge_smi_store(); + apmc_smmstore_handler(); break; }
diff --git a/src/southbridge/intel/lynxpoint/smihandler.c b/src/southbridge/intel/lynxpoint/smihandler.c index 8f2b5a3..4dc3086 100644 --- a/src/southbridge/intel/lynxpoint/smihandler.c +++ b/src/southbridge/intel/lynxpoint/smihandler.c @@ -200,26 +200,6 @@ return NULL; }
-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); - uint32_t reg_ebx; - - if (!io_smi) - return; - /* Command and return value in EAX */ - sub_command = (io_smi->rax >> 8) & 0xff; - - /* Parameter buffer in EBX */ - reg_ebx = io_smi->rbx; - - /* drivers/smmstore/smi.c */ - ret = smmstore_exec(sub_command, (void *)reg_ebx); - io_smi->rax = ret; -} - static void southbridge_smi_apmc(void) { u8 reg8; @@ -253,7 +233,7 @@ break; case APM_CNT_SMMSTORE: if (CONFIG(SMMSTORE)) - southbridge_smi_store(); + apmc_smmstore_handler(); break; }
-
Arthur Heymans (Code Review)