Change in coreboot[master]: Add console deinit API, use in SMM handler - coreboot-gerrit

16 Jul 2021

Deomid "rojer" Ryabkov has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56386 )
Change subject: Add console deinit API, use in SMM handler
......................................................................
Add console deinit API, use in SMM handler
When CONFIG_DEBUG_SMI is enabled SMM handler performs console
hardware initialization that may interfere with OS.
SMM handler should return shared hardware to the state it was at the
time of the SMI call.
Specifically, this has been observed with SMM handler output goint to UART:
uart8250_init disables interrupts because it uses polling but the Linux kernel uses interrupts.
As a result, SMI call leaves interrupts disabled and causes OS console I/O issues.
For 8250, we save all the registers we touch in init and restore them in deinit.
Other deinit functions are provided as stubs only but should probably be
lokked into as well.
Change-Id: Ia5e51f385f83cb998c244ca1d1ffc10339d3a714
---
M src/console/console.c
M src/console/init.c
M src/cpu/x86/smm/smm_module_handler.c
M src/drivers/uart/pl011.c
M src/drivers/uart/sifive.c
M src/drivers/uart/uart8250io.c
M src/drivers/uart/uart8250mem.c
M src/include/console/cbmem_console.h
M src/include/console/console.h
M src/include/console/flash.h
M src/include/console/ne2k.h
M src/include/console/qemu_debugcon.h
M src/include/console/spi.h
M src/include/console/spkmodem.h
M src/include/console/streams.h
M src/include/console/system76_ec.h
M src/include/console/uart.h
M src/include/console/usb.h
18 files changed, 122 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/56386/1

diff --git a/src/console/console.c b/src/console/console.c
index 67da107..17e905a2 100644
--- a/src/console/console.c
+++ b/src/console/console.c
@@ -16,7 +16,6 @@
    __cbmemc_init();
    __spkmodem_init();
    __qemu_debugcon_init();
-
    __uart_init();
    __ne2k_init();
    __usbdebug_init();
@@ -25,6 +24,19 @@
    __system76_ec_init();
 }
+void console_hw_deinit(void)
+{
+	__cbmemc_deinit();
+	__spkmodem_deinit();
+	__qemu_debugcon_deinit();
+	__uart_deinit();
+	__ne2k_deinit();
+	__usbdebug_deinit();
+	__spiconsole_deinit();
+	__flashconsole_deinit();
+	__system76_ec_deinit();
+}
+
 void console_tx_byte(unsigned char byte)
 {
    __cbmemc_tx_byte(byte);
diff --git a/src/console/init.c b/src/console/init.c
index a2ceb58..19f1452 100644
--- a/src/console/init.c
+++ b/src/console/init.c
@@ -63,3 +63,9 @@
           coreboot_version, coreboot_extra_version, coreboot_build,
           get_log_level());
 }
+
+void console_deinit(void)
+{
+	console_inited = 0;
+	console_hw_deinit();
+}
diff --git a/src/cpu/x86/smm/smm_module_handler.c b/src/cpu/x86/smm/smm_module_handler.c
index f9ebba4..8524ac8 100644
--- a/src/cpu/x86/smm/smm_module_handler.c
+++ b/src/cpu/x86/smm/smm_module_handler.c
@@ -179,6 +179,11 @@
    		die("SMM Handler caused a stack overflow\n");
    }
+	/* Detach from console, restore hardware state to the same it was before.
+	 * In particular, the interrupt mask: console_init disables interrupts
+	 * but the OS may be using interrupt mode. */
+	console_deinit();
+
    smi_release_lock();
/* De-assert SMI# signal to allow another SMI */
diff --git a/src/drivers/uart/pl011.c b/src/drivers/uart/pl011.c
index 0a73d82..d53ce95 100644
--- a/src/drivers/uart/pl011.c
+++ b/src/drivers/uart/pl011.c
@@ -9,6 +9,10 @@
 {
 }
+void uart_deinit(unsigned int idx)
+{
+}
+
 void uart_tx_byte(unsigned int idx, unsigned char data)
 {
    struct pl011_uart *regs = uart_platform_baseptr(idx);
diff --git a/src/drivers/uart/sifive.c b/src/drivers/uart/sifive.c
index 31181aa..d3f04b1 100644
--- a/src/drivers/uart/sifive.c
+++ b/src/drivers/uart/sifive.c
@@ -53,6 +53,11 @@
    sifive_uart_init(uart_platform_baseptr(idx), div);
 }
+void uart_deinit(unsigned int idx)
+{
+	/* TODO */
+}
+
 static bool uart_can_tx(struct sifive_uart_registers *regs)
 {
    return !(read32(&regs->txdata) & TXDATA_FULL);
diff --git a/src/drivers/uart/uart8250io.c b/src/drivers/uart/uart8250io.c
index aa8c969..0c64779 100644
--- a/src/drivers/uart/uart8250io.c
+++ b/src/drivers/uart/uart8250io.c
@@ -51,9 +51,23 @@
    	return 0x0;
 }
+struct uart8250_state {
+	uint8_t IER, FCR, MCR, LCR, DLL, DLM;
+};
+
+static struct uart8250_state s_uart8250_state;
+
 static void uart8250_init(unsigned int base_port, unsigned int divisor)
 {
-	/* Disable interrupts */
+	/* Save previous state for restoring later. */
+	s_uart8250_state.IER = inb(base_port + UART8250_IER);
+	s_uart8250_state.FCR = inb(base_port + UART8250_FCR);
+	s_uart8250_state.MCR = inb(base_port + UART8250_MCR);
+	s_uart8250_state.LCR = inb(base_port + UART8250_LCR);
+	s_uart8250_state.DLL = inb(base_port + UART8250_DLL);
+	s_uart8250_state.DLM = inb(base_port + UART8250_DLM);
+
+	/* Disable interrupts. */
    outb(0x0, base_port + UART8250_IER);
    /* Enable FIFOs */
    outb(UART8250_FCR_FIFO_EN, base_port + UART8250_FCR);
@@ -72,6 +86,16 @@
    outb(CONFIG_TTYS0_LCS, base_port + UART8250_LCR);
 }
+static void uart8250_deinit(unsigned int base_port)
+{
+	outb(s_uart8250_state.DLL, base_port + UART8250_DLL);
+	outb(s_uart8250_state.DLM, base_port + UART8250_DLM);
+	outb(s_uart8250_state.FCR, base_port + UART8250_FCR);
+	outb(s_uart8250_state.MCR, base_port + UART8250_MCR);
+	outb(s_uart8250_state.LCR, base_port + UART8250_LCR);
+	outb(s_uart8250_state.IER, base_port + UART8250_IER);
+}
+
 static const unsigned int bases[] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
uintptr_t uart_platform_base(unsigned int idx)
@@ -91,6 +115,13 @@
    }
 }
+void uart_deinit(unsigned int idx)
+{
+	if (!CONFIG(DRIVERS_UART_8250IO_SKIP_INIT)) {
+		uart8250_deinit(uart_platform_base(idx));
+	}
+}
+
 void uart_tx_byte(unsigned int idx, unsigned char data)
 {
    uart8250_tx_byte(uart_platform_base(idx), data);
diff --git a/src/drivers/uart/uart8250mem.c b/src/drivers/uart/uart8250mem.c
index 1834095..17c8c90 100644
--- a/src/drivers/uart/uart8250mem.c
+++ b/src/drivers/uart/uart8250mem.c
@@ -77,8 +77,22 @@
    	return 0x0;
 }
+struct uart8250_state {
+	uint8_t IER, FCR, MCR, LCR, DLL, DLM;
+};
+
+static struct uart8250_state s_uart8250_state;
+
 static void uart8250_mem_init(void *base, unsigned int divisor)
 {
+	/* Save previous state for restoring later. */
+	s_uart8250_state.IER = uart8250_read(base, UART8250_IER);
+	s_uart8250_state.FCR = uart8250_read(base, UART8250_FCR);
+	s_uart8250_state.MCR = uart8250_read(base, UART8250_MCR);
+	s_uart8250_state.LCR = uart8250_read(base, UART8250_LCR);
+	s_uart8250_state.DLL = uart8250_read(base, UART8250_DLL);
+	s_uart8250_state.DLM = uart8250_read(base, UART8250_DLM);
+
    /* Disable interrupts */
    uart8250_write(base, UART8250_IER, 0x0);
    /* Enable FIFOs */
@@ -97,6 +111,16 @@
    uart8250_write(base, UART8250_LCR, CONFIG_TTYS0_LCS);
 }
+static void uart8250_mem_deinit(void *base)
+{
+	uart8250_write(base, UART8250_DLL, s_uart8250_state.DLL);
+	uart8250_write(base, UART8250_DLM, s_uart8250_state.DLM);
+	uart8250_write(base, UART8250_FCR, s_uart8250_state.FCR);
+	uart8250_write(base, UART8250_MCR, s_uart8250_state.MCR);
+	uart8250_write(base, UART8250_LCR, s_uart8250_state.LCR);
+	uart8250_write(base, UART8250_IER, s_uart8250_state.IER);
+}
+
 void uart_init(unsigned int idx)
 {
    void *base = uart_platform_baseptr(idx);
@@ -109,6 +133,11 @@
    uart8250_mem_init(base, div);
 }
+void uart_deinit(unsigned int idx)
+{
+	uart8250_mem_deinit(base);
+}
+
 void uart_tx_byte(unsigned int idx, unsigned char data)
 {
    void *base = uart_platform_baseptr(idx);
diff --git a/src/include/console/cbmem_console.h b/src/include/console/cbmem_console.h
index 3eb278d..a9a0a51 100644
--- a/src/include/console/cbmem_console.h
+++ b/src/include/console/cbmem_console.h
@@ -13,9 +13,11 @@
#if __CBMEM_CONSOLE_ENABLE__
 static inline void __cbmemc_init(void)	{ cbmemc_init(); }
+static inline void __cbmemc_deinit(void)	{ }
 static inline void __cbmemc_tx_byte(u8 data)	{ cbmemc_tx_byte(data); }
 #else
 static inline void __cbmemc_init(void)	{}
+static inline void __cbmemc_deinit(void)	{}
 static inline void __cbmemc_tx_byte(u8 data)	{}
 #endif
diff --git a/src/include/console/console.h b/src/include/console/console.h
index 17448fe..3836f45 100644
--- a/src/include/console/console.h
+++ b/src/include/console/console.h
@@ -49,6 +49,7 @@
#if __CONSOLE_ENABLE__
 asmlinkage void console_init(void);
+void console_deinit(void);
 int console_log_level(int msg_level);
int printk(int msg_level, const char *fmt, ...) __attribute__((format(printf, 2, 3)));
diff --git a/src/include/console/flash.h b/src/include/console/flash.h
index 9a0dc63..2e08d66 100644
--- a/src/include/console/flash.h
+++ b/src/include/console/flash.h
@@ -13,6 +13,7 @@
#if __CONSOLE_FLASH_ENABLE__
 static inline void __flashconsole_init(void)	{ flashconsole_init(); }
+static inline void __flashconsole_deinit(void)	{}
 static inline void __flashconsole_tx_byte(u8 data)
 {
    flashconsole_tx_byte(data);
@@ -23,6 +24,7 @@
 }
 #else
 static inline void __flashconsole_init(void)	{}
+static inline void __flashconsole_deinit(void)	{}
 static inline void __flashconsole_tx_byte(u8 data)	{}
 static inline void __flashconsole_tx_flush(void)	{}
 #endif /* __CONSOLE_FLASH_ENABLE__ */
diff --git a/src/include/console/ne2k.h b/src/include/console/ne2k.h
index ca8d147..5bef2e0 100644
--- a/src/include/console/ne2k.h
+++ b/src/include/console/ne2k.h
@@ -14,6 +14,9 @@
 {
    ne2k_init(CONFIG_CONSOLE_NE2K_IO_PORT);
 }
+static inline void __ne2k_deinit(void)
+{
+}
 static inline void __ne2k_tx_byte(u8 data)
 {
    ne2k_append_data(&data, 1, CONFIG_CONSOLE_NE2K_IO_PORT);
@@ -24,6 +27,7 @@
 }
 #else
 static inline void __ne2k_init(void)		{}
+static inline void __ne2k_deinit(void)		{}
 static inline void __ne2k_tx_byte(u8 data)	{}
 static inline void __ne2k_tx_flush(void)	{}
 #endif
diff --git a/src/include/console/qemu_debugcon.h b/src/include/console/qemu_debugcon.h
index f0252ee..1214bef 100644
--- a/src/include/console/qemu_debugcon.h
+++ b/src/include/console/qemu_debugcon.h
@@ -9,12 +9,14 @@
 #if CONFIG(CONSOLE_QEMU_DEBUGCON) && \
    (ENV_ROMSTAGE || ENV_RAMSTAGE || ENV_POSTCAR || ENV_BOOTBLOCK)
 static inline void __qemu_debugcon_init(void)	{ qemu_debugcon_init(); }
+static inline void __qemu_debugcon_deinit(void)	{}
 static inline void __qemu_debugcon_tx_byte(u8 data)
 {
    qemu_debugcon_tx_byte(data);
 }
 #else
 static inline void __qemu_debugcon_init(void)	{}
+static inline void __qemu_debugcon_deinit(void)	{}
 static inline void __qemu_debugcon_tx_byte(u8 data)	{}
 #endif
diff --git a/src/include/console/spi.h b/src/include/console/spi.h
index 2eff0a8..1326102 100644
--- a/src/include/console/spi.h
+++ b/src/include/console/spi.h
@@ -13,12 +13,14 @@
#if __CONSOLE_SPI_ENABLE__
 static inline void __spiconsole_init(void)	{ spiconsole_init(); }
+static inline void __spiconsole_deinit(void)	{}
 static inline void __spiconsole_tx_byte(u8 data)
 {
    spiconsole_tx_byte(data);
 }
 #else
 static inline void __spiconsole_init(void)	{}
+static inline void __spiconsole_deinit(void)	{}
 static inline void __spiconsole_tx_byte(u8 data)	{}
 #endif /* __CONSOLE_SPI_ENABLE__ */
diff --git a/src/include/console/spkmodem.h b/src/include/console/spkmodem.h
index c8c142f..7905567 100644
--- a/src/include/console/spkmodem.h
+++ b/src/include/console/spkmodem.h
@@ -8,9 +8,11 @@
#if CONFIG(SPKMODEM) && (ENV_ROMSTAGE || ENV_RAMSTAGE)
 static inline void __spkmodem_init(void)		{ spkmodem_init(); }
+static inline void __spkmodem_deinit(void)		{}
 static inline void __spkmodem_tx_byte(u8 data)	{ spkmodem_tx_byte(data); }
 #else
 static inline void __spkmodem_init(void)		{}
+static inline void __spkmodem_deinit(void)		{}
 static inline void __spkmodem_tx_byte(u8 data)	{}
 #endif
diff --git a/src/include/console/streams.h b/src/include/console/streams.h
index 44d96e2..5dc6132 100644
--- a/src/include/console/streams.h
+++ b/src/include/console/streams.h
@@ -7,6 +7,7 @@
 #include <stdint.h>
void console_hw_init(void);
+void console_hw_deinit(void);
 void console_tx_byte(unsigned char byte);
 void console_tx_flush(void);
diff --git a/src/include/console/system76_ec.h b/src/include/console/system76_ec.h
index 616e46f..eb363c2 100644
--- a/src/include/console/system76_ec.h
+++ b/src/include/console/system76_ec.h
@@ -18,6 +18,9 @@
 {
    system76_ec_init();
 }
+static inline void __system76_ec_deinit(void)
+{
+}
 static inline void __system76_ec_tx_flush(void)
 {
    system76_ec_flush();
@@ -28,6 +31,7 @@
 }
 #else
 static inline void __system76_ec_init(void) {}
+static inline void __system76_ec_deinit(void) {}
 static inline void __system76_ec_tx_flush(void) {}
 static inline void __system76_ec_tx_byte(unsigned char byte) {}
 #endif
diff --git a/src/include/console/uart.h b/src/include/console/uart.h
index 2e23d43..20ced3e 100644
--- a/src/include/console/uart.h
+++ b/src/include/console/uart.h
@@ -48,6 +48,7 @@
 void uart_bitbang_tx_byte(unsigned char data, void (*set_tx)(int line_state));
void uart_init(unsigned int idx);
+void uart_deinit(unsigned int idx);
 void uart_tx_byte(unsigned int idx, unsigned char data);
 void uart_tx_flush(unsigned int idx);
 unsigned char uart_rx_byte(unsigned int idx);
@@ -70,6 +71,10 @@
 {
    uart_init(get_uart_for_console());
 }
+static inline void __uart_deinit(void)
+{
+	uart_deinit(get_uart_for_console());
+}
 static inline void __uart_tx_byte(u8 data)
 {
    uart_tx_byte(get_uart_for_console(), data);
@@ -80,6 +85,7 @@
 }
 #else
 static inline void __uart_init(void)		{}
+static inline void __uart_deinit(void)		{}
 static inline void __uart_tx_byte(u8 data)	{}
 static inline void __uart_tx_flush(void)	{}
 #endif
diff --git a/src/include/console/usb.h b/src/include/console/usb.h
index 30591c2..fa4e5f9 100644
--- a/src/include/console/usb.h
+++ b/src/include/console/usb.h
@@ -25,6 +25,7 @@
#if __CONSOLE_USB_ENABLE__
 static inline void __usbdebug_init(void)	{ usbdebug_init(); }
+static inline void __usbdebug_deinit(void)	{}
 static inline void __usb_tx_byte(u8 data)
 {
    usb_tx_byte(USB_PIPE_FOR_CONSOLE, data);
@@ -32,6 +33,7 @@
 static inline void __usb_tx_flush(void)	{ usb_tx_flush(USB_PIPE_FOR_CONSOLE); }
 #else
 static inline void __usbdebug_init(void)	{}
+static inline void __usbdebug_deinit(void)	{}
 static inline void __usb_tx_byte(u8 data)	{}
 static inline void __usb_tx_flush(void)	{}
 #endif
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/56386
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia5e51f385f83cb998c244ca1d1ffc10339d3a714
Gerrit-Change-Number: 56386
Gerrit-PatchSet: 1
Gerrit-Owner: Deomid "rojer" Ryabkov rojer9@fb.com
Gerrit-MessageType: newchange



    