Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/61305 )
Change subject: lib/cbmem_console,console: Resurrect CONSOLE_CBMEM_DUMP_TO_UART ......................................................................
lib/cbmem_console,console: Resurrect CONSOLE_CBMEM_DUMP_TO_UART
Chromebooks normally run with non-serial enabled firmware because writing to the UART console is very slow. This unfortunately makes debugging boot errors more difficult. We tend to rely on port 80s and/or the vboot recovery code.
When CONSOLE_CBMEM_DUMP_TO_UART is selected it will dump the entire cbmem console to the UART whenever `vboot_reboot()` is called. We don't incur any boot time penalty in the happy path, but still retain the ability to access the logs when an error occurs.
The previous implementation was using a hard coded UART index and `get_uart_baudrate` was always returning 0 since `CONFIG_TTYS0_BAUD` wasn't defined. This change makes it so the UART console properties are available when CONSOLE_CBMEM_DUMP_TO_UART is set. This results in the following .config diff:
+CONFIG_UART_FOR_CONSOLE=0 +CONFIG_TTYS0_BASE=0x3f8 +CONFIG_TTYS0_LCS=3 +CONFIG_CONSOLE_SERIAL_115200=y +CONFIG_TTYS0_BAUD=115200
This functionality is especially helpful on Guybrush. PSP Verstage is run on S0i3 resume. Today, if there is an error, the cbmem console is lost since it lives in the PSP SRAM.
BUG=b:213828947, b:215599230 TEST=Build non-serial guybrush FW and verify no serial output happens in happy path. Inject a vboot error and perform an S0i3 suspend/resume. Verify CBMEM console gets dumped to the correct UART.
Signed-off-by: Raul E Rangel rrangel@chromium.org Change-Id: I997942204603362e51876a9ae25e493fe527437b Reviewed-on: https://review.coreboot.org/c/coreboot/+/61305 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/console/Kconfig M src/lib/cbmem_console.c 2 files changed, 18 insertions(+), 7 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve
diff --git a/src/console/Kconfig b/src/console/Kconfig index 60b27c4..f80d2e4 100644 --- a/src/console/Kconfig +++ b/src/console/Kconfig @@ -47,7 +47,7 @@ specific UART has to be used (e.g. when the platform code performs dangerous configurations).
-if CONSOLE_SERIAL +if CONSOLE_SERIAL || CONSOLE_CBMEM_DUMP_TO_UART
comment "I/O mapped, 8250-compatible" depends on DRIVERS_UART_8250IO diff --git a/src/lib/cbmem_console.c b/src/lib/cbmem_console.c index 8e190d72..2faa5d5 100644 --- a/src/lib/cbmem_console.c +++ b/src/lib/cbmem_console.c @@ -171,16 +171,27 @@ void cbmem_dump_console_to_uart(void) { u32 cursor; + unsigned int console_index; + if (!current_console) return;
- uart_init(0); - if (current_console->cursor & OVERFLOW) + console_index = get_uart_for_console(); + + uart_init(console_index); + if (current_console->cursor & OVERFLOW) { for (cursor = current_console->cursor & CURSOR_MASK; - cursor < current_console->size; cursor++) - uart_tx_byte(0, current_console->body[cursor]); - for (cursor = 0; cursor < (current_console->cursor & CURSOR_MASK); cursor++) - uart_tx_byte(0, current_console->body[cursor]); + cursor < current_console->size; cursor++) { + if (current_console->body[cursor] == '\n') + uart_tx_byte(console_index, '\r'); + uart_tx_byte(console_index, current_console->body[cursor]); + } + } + for (cursor = 0; cursor < (current_console->cursor & CURSOR_MASK); cursor++) { + if (current_console->body[cursor] == '\n') + uart_tx_byte(console_index, '\r'); + uart_tx_byte(console_index, current_console->body[cursor]); + } } #endif