Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79738?usp=email )
Change subject: smmstorev2: Load the communication buffer at SMM setup ......................................................................
smmstorev2: Load the communication buffer at SMM setup
This removes the runtime SMI call to set up the communication buffer for SMMSTORE in favor of setting this buffer up during the installation of the smihandler.
The reason is that it's less code in the handler and a time costly SMI is also avoided in ramstage.
Signed-off-by: Arthur Heymans arthur@aheymans.xyz Change-Id: I94dce77711f37f87033530f5ae48cb850a39341b --- M Documentation/drivers/smmstorev2.md M src/cpu/x86/smm/smm_module_handler.c M src/cpu/x86/smm/smm_module_loader.c M src/drivers/smmstore/ramstage.c M src/drivers/smmstore/smi.c M src/drivers/smmstore/store.c M src/include/cpu/x86/smm.h M src/include/smmstore.h 8 files changed, 55 insertions(+), 87 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/79738/1
diff --git a/Documentation/drivers/smmstorev2.md b/Documentation/drivers/smmstorev2.md index 70aa60d..40fba612 100644 --- a/Documentation/drivers/smmstorev2.md +++ b/Documentation/drivers/smmstorev2.md @@ -124,25 +124,9 @@ **NOTE**: The size of the struct entries are in the native word size of smihandler. This means 32 bits in almost all cases.
-#### - SMMSTORE_CMD_INIT = 4 +#### - SMMSTORE_CMD_INIT_DEPRECATED = 4
-This installs the communication buffer to use and thus enables the -SMMSTORE handler. This command can only be executed once and is done -by the firmware. Calling this function at runtime has no effect. - -The additional parameter buffer `%ebx` contains a pointer to the -following struct: - -```C -struct smmstore_params_init { - uint32_t com_buffer; - uint32_t com_buffer_size; -} __packed; -``` - -INPUT: -- `com_buffer`: Physical address of the communication buffer (CBMEM) -- `com_buffer_size`: Size in bytes of the communication buffer +Unused, returns SMMSTORE_REG_UNSUPPORTED.
#### - SMMSTORE_CMD_RAW_READ = 5
diff --git a/src/cpu/x86/smm/smm_module_handler.c b/src/cpu/x86/smm/smm_module_handler.c index 0daae00..b16fa81 100644 --- a/src/cpu/x86/smm/smm_module_handler.c +++ b/src/cpu/x86/smm/smm_module_handler.c @@ -60,6 +60,14 @@ } #endif
+#if CONFIG(SMMSTORE_V2) +void smm_get_smmstore_com_buffer(uintptr_t *base, size_t *size) +{ + *base = smm_runtime.smmstore_com_buffer_base; + *size = smm_runtime.smmstore_com_buffer_size; +} +#endif + void smm_get_cbmemc_buffer(void **buffer_out, size_t *size_out) { *buffer_out = smm_runtime.cbmemc; diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index f827e7b..8179cbc 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -8,7 +8,9 @@ #include <cpu/cpu.h> #include <cpu/x86/smm.h> #include <device/device.h> +#include <smmstore.h> #include <rmodule.h> +#include <stdint.h> #include <stdio.h> #include <string.h> #include <types.h> @@ -345,6 +347,22 @@
if (CONFIG(SMM_PCI_RESOURCE_STORE)) smm_pci_resource_store_init(mod_params); + + if (CONFIG(SMMSTORE_V2)) { + struct smmstore_params_info info; + if (smmstore_get_info(&info) < 0) { + printk(BIOS_INFO, "SMMSTORE: Failed to get meta data\n"); + return; + } + + void *ptr = cbmem_add(CBMEM_ID_SMM_COMBUFFER, info.block_size); + if (!ptr) { + printk(BIOS_ERR, "SMMSTORE: Failed to add com buffer\n"); + return; + } + mod_params->smmstore_com_buffer_base = (uintptr_t)ptr; + mod_params->smmstore_com_buffer_size = info.block_size; + } }
static void print_region(const char *name, const struct region region) diff --git a/src/drivers/smmstore/ramstage.c b/src/drivers/smmstore/ramstage.c index ef80e22..5a81d48 100644 --- a/src/drivers/smmstore/ramstage.c +++ b/src/drivers/smmstore/ramstage.c @@ -9,8 +9,6 @@ #include <types.h> #include <cbmem.h>
-static struct smmstore_params_info info; - void lb_smmstorev2(struct lb_header *header) { struct lb_record *rec; @@ -21,6 +19,9 @@ if (!e) return;
+ struct smmstore_params_info info; + smmstore_get_info(&info); + rec = lb_new_record(header); store = (struct lb_smmstorev2 *)rec;
@@ -33,44 +34,3 @@ store->block_size = info.block_size; store->apm_cmd = APM_CNT_SMMSTORE; } - -static void init_store(void *unused) -{ - struct smmstore_params_init args; - uint32_t eax = ~0; - uint32_t ebx; - - if (smmstore_get_info(&info) < 0) { - printk(BIOS_INFO, "SMMSTORE: Failed to get meta data\n"); - return; - } - - void *ptr = cbmem_add(CBMEM_ID_SMM_COMBUFFER, info.block_size); - if (!ptr) { - printk(BIOS_ERR, "SMMSTORE: Failed to add com buffer\n"); - return; - } - - args.com_buffer = (uintptr_t)ptr; - args.com_buffer_size = info.block_size; - ebx = (uintptr_t)&args; - - printk(BIOS_INFO, "SMMSTORE: Setting up SMI handler\n"); - - /* Issue SMI using APM to update the com buffer and to lock the SMMSTORE */ - __asm__ __volatile__ ( - "outb %%al, %%dx" - : "=a" (eax) - : "a" ((SMMSTORE_CMD_INIT << 8) | APM_CNT_SMMSTORE), - "b" (ebx), - "d" (APM_CNT) - : "memory"); - - if (eax != SMMSTORE_RET_SUCCESS) { - printk(BIOS_ERR, "SMMSTORE: Failed to install com buffer\n"); - return; - } -} - -/* The SMI APM handler is installed at DEV_INIT phase */ -BOOT_STATE_INIT_ENTRY(BS_DEV_INIT, BS_ON_EXIT, init_store, NULL); diff --git a/src/drivers/smmstore/smi.c b/src/drivers/smmstore/smi.c index 6ea447f..f0f5390 100644 --- a/src/drivers/smmstore/smi.c +++ b/src/drivers/smmstore/smi.c @@ -78,24 +78,25 @@ static uint32_t smmstorev2_exec(uint8_t command, void *param) { uint32_t ret = SMMSTORE_RET_FAILURE; + static bool initialized = false; + + if (!initialized) { + uintptr_t base; + size_t size; + smm_get_smmstore_com_buffer(&base, &size); + + if (base == 0 || size == 0) + return SMMSTORE_RET_FAILURE; + + if (range_check((void *)base, size)) + return SMMSTORE_RET_FAILURE; + + if (smmstore_init((void *)base, size)) + return SMMSTORE_RET_FAILURE; + initialized = true; + }
switch (command) { - case SMMSTORE_CMD_INIT: { - printk(BIOS_DEBUG, "Init SMM store\n"); - struct smmstore_params_init *params = param; - - if (range_check(params, sizeof(*params)) != 0) - break; - - void *buf = (void *)(uintptr_t)params->com_buffer; - - if (range_check(buf, params->com_buffer_size) != 0) - break; - - if (smmstore_init(buf, params->com_buffer_size) == 0) - ret = SMMSTORE_RET_SUCCESS; - break; - } case SMMSTORE_CMD_RAW_READ: { printk(BIOS_DEBUG, "Raw read from SMM store, param = %p\n", param); struct smmstore_params_raw_read *params = param; diff --git a/src/drivers/smmstore/store.c b/src/drivers/smmstore/store.c index bc3dcdc..3fca45b 100644 --- a/src/drivers/smmstore/store.c +++ b/src/drivers/smmstore/store.c @@ -283,14 +283,10 @@
/* Implementation of Version 2 */
-static bool store_initialized; static struct region_device mdev_com_buf;
static int smmstore_rdev_chain(struct region_device *rdev) { - if (!store_initialized) - return -1; - return rdev_chain_full(rdev, &mdev_com_buf); }
@@ -303,13 +299,8 @@ if (!buf || len < SMM_BLOCK_SIZE) return -1;
- if (store_initialized) - return -1; - rdev_chain_mem_rw(&mdev_com_buf, buf, len);
- store_initialized = true; - return 0; }
diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index a240ac2..e94fb59 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -91,6 +91,10 @@ #endif uintptr_t save_state_top[CONFIG_MAX_CPUS]; int smm_log_level; +#if CONFIG(SMMSTORE_V2) + uintptr_t smmstore_com_buffer_base; + size_t smmstore_com_buffer_size; +#endif } __packed;
struct smm_module_params { @@ -227,4 +231,6 @@
void smm_pci_resource_store_init(struct smm_runtime *smm_runtime);
+void smm_get_smmstore_com_buffer(uintptr_t *base, size_t *size); + #endif /* CPU_X86_SMM_H */ diff --git a/src/include/smmstore.h b/src/include/smmstore.h index d3940ab..6805cbc 100644 --- a/src/include/smmstore.h +++ b/src/include/smmstore.h @@ -16,7 +16,7 @@ #define SMMSTORE_CMD_APPEND 3
/* Version 2 */ -#define SMMSTORE_CMD_INIT 4 +#define SMMSTORE_CMD_INIT_DEPRECATED 4 #define SMMSTORE_CMD_RAW_READ 5 #define SMMSTORE_CMD_RAW_WRITE 6 #define SMMSTORE_CMD_RAW_CLEAR 7
-
Arthur Heymans (Code Review)