Change in coreboot[master]: cpu/x86/smm,lib/cbmem_console: Enable CBMEMC when using DEBUG_SMI

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/62355 )

Change subject: cpu/x86/smm,lib/cbmem_console: Enable CBMEMC when using DEBUG_SMI
......................................................................

cpu/x86/smm,lib/cbmem_console: Enable CBMEMC when using DEBUG_SMI

This change will allow the SMI handler to write to the cbmem console
buffer. Normally SMIs can only be debugged using some kind of serial
port (UART). By storing the SMI logs into cbmem we can debug SMIs using
`cbmem -1`. Now that these logs are available to the OS we could also
verify there were no errors in the SMI handler.

Since SMM can write to all of DRAM, we can't trust any pointers
provided by cbmem after the OS has booted. For this reason we store the
cbmem console pointer as part of the SMM runtime parameters. The cbmem
console is implemented as a circular buffer so it will never write
outside of this area.

BUG=b:221231786
TEST=Boot non-serial FW with DEBUG_SMI and verified SMI messages are
visible when running `cbmem -1`. Perform a suspend/resume cycle and
verify new SMI events are written to the cbmem console log.

Signed-off-by: Raul E Rangel 
Change-Id: Ia1e310a12ca2f54210ccfaee58807cb808cfff79
Reviewed-on: https://review.coreboot.org/c/coreboot/+/62355
Tested-by: build bot (Jenkins) 
Reviewed-by: Karthik Ramasubramanian 
---
M src/cpu/x86/smm/smm_module_handler.c
M src/cpu/x86/smm/smm_module_loader.c
M src/include/console/cbmem_console.h
M src/include/cpu/x86/smm.h
M src/lib/Makefile.inc
M src/lib/cbmem_console.c
6 files changed, 36 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Karthik Ramasubramanian: Looks good to me, approved

diff --git a/src/cpu/x86/smm/smm_module_handler.c b/src/cpu/x86/smm/smm_module_handler.c
index cab691d..bd4149a 100644
--- a/src/cpu/x86/smm/smm_module_handler.c
+++ b/src/cpu/x86/smm/smm_module_handler.c
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

#include <arch/io.h>
+#include <console/cbmem_console.h>
 #include <console/console.h>
 #include <commonlib/region.h>
 #include <cpu/x86/smm.h>
@@ -48,6 +49,12 @@
    );
 }

+void smm_get_cbmemc_buffer(void **buffer_out, size_t *size_out)
+{
+	*buffer_out = smm_runtime.cbmemc;
+	*size_out = smm_runtime.cbmemc_size;
+}
+
 void io_trap_handler(int smif)
 {
    /* If a handler function handled a given IO trap, it
diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c
index 14e2aa3..17c6779 100644
--- a/src/cpu/x86/smm/smm_module_loader.c
+++ b/src/cpu/x86/smm/smm_module_loader.c
@@ -5,6 +5,7 @@
 #include <stdint.h>
 #include <string.h>
 #include <rmodule.h>
+#include <cbmem.h>
 #include <cpu/x86/smm.h>
 #include <commonlib/helpers.h>
 #include <console/console.h>
@@ -479,6 +480,7 @@
    void *fxsave_area;
    size_t total_size = 0;
    uintptr_t base; /* The base for the permanent handler */
+	const struct cbmem_entry *cbmemc;

if (CONFIG(SMM_ASEG))
    	return smm_load_module_aseg(smram_base, smram_size, params);
@@ -557,6 +559,14 @@
    handler_mod_params->num_cpus = params->num_cpus;
    handler_mod_params->gnvs_ptr = (uintptr_t)acpi_get_gnvs();

+	if (CONFIG(CONSOLE_CBMEM) && (cbmemc = cbmem_entry_find(CBMEM_ID_CONSOLE))) {
+		handler_mod_params->cbmemc = cbmem_entry_start(cbmemc);
+		handler_mod_params->cbmemc_size = cbmem_entry_size(cbmemc);
+	} else {
+		handler_mod_params->cbmemc = 0;
+		handler_mod_params->cbmemc_size = 0;
+	}
+
    printk(BIOS_DEBUG, "%s: smram_start: 0x%lx\n",  __func__, smram_base);
    printk(BIOS_DEBUG, "%s: smram_end: %lx\n", __func__, smram_base + smram_size);
    printk(BIOS_DEBUG, "%s: handler start %p\n",
@@ -577,6 +587,8 @@
    printk(BIOS_DEBUG, "%s: per_cpu_save_state_size = 0x%x\n", __func__,
           handler_mod_params->save_state_size);
    printk(BIOS_DEBUG, "%s: num_cpus = 0x%x\n", __func__, handler_mod_params->num_cpus);
+	printk(BIOS_DEBUG, "%s: cbmemc = %p, cbmemc_size = %#x\n", __func__,
+	       handler_mod_params->cbmemc, handler_mod_params->cbmemc_size);
    printk(BIOS_DEBUG, "%s: total_save_state_size = 0x%x\n", __func__,
           (handler_mod_params->save_state_size * handler_mod_params->num_cpus));

diff --git a/src/include/console/cbmem_console.h b/src/include/console/cbmem_console.h
index 4f03a45..9a814b9 100644
--- a/src/include/console/cbmem_console.h
+++ b/src/include/console/cbmem_console.h
@@ -10,7 +10,8 @@

#define __CBMEM_CONSOLE_ENABLE__	(CONFIG(CONSOLE_CBMEM) && \
    (ENV_RAMSTAGE || ENV_SEPARATE_VERSTAGE || ENV_POSTCAR  || \
-	 ENV_ROMSTAGE || (ENV_BOOTBLOCK && CONFIG(BOOTBLOCK_CONSOLE))))
+	 ENV_ROMSTAGE || (ENV_BOOTBLOCK && CONFIG(BOOTBLOCK_CONSOLE)) || \
+	 (ENV_SMM && CONFIG(DEBUG_SMI))))

#if __CBMEM_CONSOLE_ENABLE__
 static inline void __cbmemc_init(void)	{ cbmemc_init(); }
@@ -28,3 +29,6 @@
 void cbmem_dump_console_to_uart(void);
 void cbmem_dump_console(void);
 #endif
+
+/* Retrieves the location of the CBMEM Console buffer in SMM mode */
+void smm_get_cbmemc_buffer(void **buffer_out, size_t *size_out);
diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h
index 8ea8336..28d95e1 100644
--- a/src/include/cpu/x86/smm.h
+++ b/src/include/cpu/x86/smm.h
@@ -61,6 +61,8 @@
    u32 save_state_size;
    u32 num_cpus;
    u32 gnvs_ptr;
+	u32 cbmemc_size;
+	void *cbmemc;
    uintptr_t save_state_top[CONFIG_MAX_CPUS];
 } __packed;

diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc
index 5e3ce50..f3da503 100644
--- a/src/lib/Makefile.inc
+++ b/src/lib/Makefile.inc
@@ -220,6 +220,9 @@
 smm-y += fmap.c
 smm-y += cbfs.c memcmp.c
 smm-$(CONFIG_GENERIC_UDELAY) += timer.c
+ifeq ($(CONFIG_DEBUG_SMI),y)
+smm-$(CONFIG_CONSOLE_CBMEM) += cbmem_console.c
+endif

bootblock-y += version.c
 romstage-y += version.c
diff --git a/src/lib/cbmem_console.c b/src/lib/cbmem_console.c
index 0c56095..783f336 100644
--- a/src/lib/cbmem_console.c
+++ b/src/lib/cbmem_console.c
@@ -84,6 +84,13 @@
    if (ENV_ROMSTAGE_OR_BEFORE) {
    	/* Pre-RAM environments use special buffer placed by linker script. */
    	init_console_ptr(_preram_cbmem_console, REGION_SIZE(preram_cbmem_console));
+	} else if (ENV_SMM) {
+		void *cbmemc = NULL;
+		size_t cbmemc_size = 0;
+
+		smm_get_cbmemc_buffer(&cbmemc, &cbmemc_size);
+
+		init_console_ptr(cbmemc, cbmemc_size);
    } else {
    	/* Post-RAM uses static (BSS) buffer before CBMEM is reinitialized. */
    	init_console_ptr(static_console, sizeof(static_console));