New patch to review for coreboot: Turn CBMEM console into a ring buffer that can persist across reboots
Julius Werner (jwerner@chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/18301
-gerrit
commit f76a674a3ece3f83bb274d8fde10c13a6abf2c37 Author: Julius Werner jwerner@chromium.org Date: Thu Feb 2 17:32:00 2017 -0800
Turn CBMEM console into a ring buffer that can persist across reboots
This patch allows the CBMEM console to persist across reboots, which should greatly help post factum debugging of issues involving multiple reboots. In order to prevent the console from filling up, it will instead operate as a ring buffer that continues to evict the oldest lines once full.
The console control structure is modified in a sorta backwards-compatible way, so that new readers can continue to work with old console buffers and vice versa. When an old reader reads a new buffer that has already once overflowed (i.e. is operating in true ring buffer mode) it will print lines out of order, but it will at least still print out the whole console content and not do any illegal memory accesses (assuming it correctly implemented cursor overflow as it was already possible before this patch).
BUG=chromium:651966 TEST=Rebooted and confirmed output repeatedly on a Kevin and a Falco. Also confirmed correct behavior across suspend/resume for the latter.
Change-Id: Ifcbf59d58e1ad20995b98d111c4647281fbb45ff Signed-off-by: Julius Werner jwerner@chromium.org --- payloads/libpayload/drivers/cbmem_console.c | 26 +++- src/lib/cbmem_console.c | 202 +++++++++------------------- util/cbmem/cbmem.c | 53 ++++---- 3 files changed, 110 insertions(+), 171 deletions(-)
diff --git a/payloads/libpayload/drivers/cbmem_console.c b/payloads/libpayload/drivers/cbmem_console.c index 6873444..8e83224 100644 --- a/payloads/libpayload/drivers/cbmem_console.c +++ b/payloads/libpayload/drivers/cbmem_console.c @@ -32,7 +32,9 @@
struct cbmem_console { uint32_t size; - uint32_t cursor; + uint32_t cursor : 24; + uint32_t _reserved_flags : 7; + uint32_t overflowed : 1; uint8_t body[0]; } __attribute__ ((__packed__));
@@ -43,18 +45,30 @@ static struct console_output_driver cbmem_console_driver = .write = &cbmem_console_write, };
+static void do_write(const void *buffer, size_t count) +{ + memcpy(cbmem_console_p->body + cbmem_console_p->cursor, buffer, count); + cbmem_console_p->cursor += count; +} + void cbmem_console_init(void) { cbmem_console_p = lib_sysinfo.cbmem_cons; - if (cbmem_console_p) + if (cbmem_console_p && cbmem_console_p->size) console_add_output_driver(&cbmem_console_driver); }
void cbmem_console_write(const void *buffer, size_t count) { - if (cbmem_console_p->cursor + count >= cbmem_console_p->size) - return; + while (cbmem_console_p->cursor + count >= cbmem_console_p->size) { + uint32_t still_fits = cbmem_console_p->size - + cbmem_console_p->cursor; + do_write(buffer, still_fits); + cbmem_console_p->overflowed = 1; + cbmem_console_p->cursor = 0; + buffer += still_fits; + count -= still_fits; + }
- memcpy(cbmem_console_p->body + cbmem_console_p->cursor, buffer, count); - cbmem_console_p->cursor += count; + do_write(buffer, count); } diff --git a/src/lib/cbmem_console.c b/src/lib/cbmem_console.c index 2845066..e6564a8 100644 --- a/src/lib/cbmem_console.c +++ b/src/lib/cbmem_console.c @@ -23,21 +23,22 @@
/* * Structure describing console buffer. It is overlaid on a flat memory area, - * with body covering the extent of the memory. Once the buffer is - * full, the cursor keeps going but the data is dropped on the floor. This - * allows to tell how much data was lost in the process. + * with body covering the extent of the memory. Once the buffer is full, + * output will wrap back around to the start of the buffer. The high bit of the + * cursor field gets set to indicate that this happened. If the underlying + * storage allows this, the buffer will persist across multiple boots and append + * to the previous log. */ struct cbmem_console { u32 size; - u32 cursor; + u32 cursor : 24; + u32 _reserved_flags : 7; + u32 overflowed : 1; u8 body[0]; } __attribute__ ((__packed__));
static struct cbmem_console *cbmem_console_p CAR_GLOBAL;
-static void copy_console_buffer(struct cbmem_console *old_cons_p, - struct cbmem_console *new_cons_p); - #ifdef __PRE_RAM__ /* * While running from ROM, before DRAM is initialized, some area in cache as @@ -62,21 +63,24 @@ static void copy_console_buffer(struct cbmem_console *old_cons_p, static u8 static_console[STATIC_CONSOLE_SIZE]; #endif
-/* flags for init */ -#define CBMEMC_RESET (1<<0) -#define CBMEMC_APPEND (1<<1) - -static inline struct cbmem_console *current_console(void) +static struct cbmem_console *current_console(void) { return car_sync_var(cbmem_console_p); }
-static inline void current_console_set(struct cbmem_console *new_console_p) +static void current_console_set(struct cbmem_console *new_console_p) { car_set_var(cbmem_console_p, new_console_p); }
-static inline void init_console_ptr(void *storage, u32 total_space, int flags) +static int buffer_valid(struct cbmem_console *cbm_cons_p, u32 total_space) +{ + return cbm_cons_p->cursor < cbm_cons_p->size && + cbm_cons_p->_reserved_flags == 0 && + cbm_cons_p->size == total_space - sizeof(struct cbmem_console); +} + +static inline void init_console_ptr(void *storage, u32 total_space) { struct cbmem_console *cbm_cons_p = storage;
@@ -85,14 +89,11 @@ static inline void init_console_ptr(void *storage, u32 total_space, int flags) return; }
- if (flags & CBMEMC_RESET) { + if (!buffer_valid(cbm_cons_p, total_space)) { cbm_cons_p->size = total_space - sizeof(struct cbmem_console); cbm_cons_p->cursor = 0; - } - if (flags & CBMEMC_APPEND) { - struct cbmem_console *tmp_cons_p = current_console(); - if (tmp_cons_p) - copy_console_buffer(tmp_cons_p, cbm_cons_p); + cbm_cons_p->_reserved_flags = 0; + cbm_cons_p->overflowed = 0; }
current_console_set(cbm_cons_p); @@ -101,149 +102,66 @@ static inline void init_console_ptr(void *storage, u32 total_space, int flags) void cbmemc_init(void) { #ifdef __PRE_RAM__ - int flags = 0; - - /* If in bootblock always initialize the console first. */ - if (ENV_BOOTBLOCK) - flags = CBMEMC_RESET; - else if (ENV_ROMSTAGE) { - /* Initialize console for the first time in romstage when - * there's no prior stage that initialized it first. */ - if (!IS_ENABLED(CONFIG_VBOOT_STARTS_IN_BOOTBLOCK) && - !IS_ENABLED(CONFIG_BOOTBLOCK_CONSOLE)) - flags = CBMEMC_RESET; - } else if (ENV_VERSTAGE) { - /* - * Initialize console for the first time in verstage when - * there is no console in bootblock. Otherwise honor the - * bootblock console when verstage comes right after - * bootblock. - */ - if (IS_ENABLED(CONFIG_VBOOT_STARTS_IN_BOOTBLOCK) && - !IS_ENABLED(CONFIG_BOOTBLOCK_CONSOLE)) - flags = CBMEMC_RESET; - } - - init_console_ptr(_preram_cbmem_console, - _preram_cbmem_console_size, flags); + /* Pre-RAM environments use special buffer placed by linker script. */ + init_console_ptr(_preram_cbmem_console, _preram_cbmem_console_size); #else - /* - * Initializing before CBMEM is available, use static buffer to store - * the log. - */ - init_console_ptr(static_console, sizeof(static_console), CBMEMC_RESET); + /* Post-RAM uses static (BSS) buffer before CBMEM is reinitialized. */ + init_console_ptr(static_console, sizeof(static_console)); #endif }
void cbmemc_tx_byte(unsigned char data) { struct cbmem_console *cbm_cons_p = current_console(); - u32 cursor;
- if (!cbm_cons_p) + if (!cbm_cons_p || !cbm_cons_p->size) return;
- cursor = cbm_cons_p->cursor++; - if (cursor < cbm_cons_p->size) - cbm_cons_p->body[cursor] = data; + cbm_cons_p->body[cbm_cons_p->cursor++] = data; + if (cbm_cons_p->cursor >= cbm_cons_p->size) { + cbm_cons_p->overflowed = 1; + cbm_cons_p->cursor = 0; + } }
/* - * Copy the current console buffer (either from the cache as RAM area, or from - * the static buffer, pointed at by cbmem_console_p) into the CBMEM console - * buffer space (pointed at by new_cons_p), concatenating the copied data with - * the CBMEM console buffer contents. - * - * If there is overflow - add to the destination area a string, reporting the - * overflow and the number of dropped characters. + * Copy the current console buffer (either from the cache as RAM area or from + * the static buffer, pointed at by cons_p) into the newly initialized CBMEM + * console. The use of cbmemc_tx_byte() ensures that all special cases for the + * target console (e.g. overflow) will be handled. If there had been an + * overflow in the source console, log a message to that effect. */ -static void copy_console_buffer(struct cbmem_console *old_cons_p, - struct cbmem_console *new_cons_p) +static void copy_console_buffer(struct cbmem_console *src_cons_p) { - u32 copy_size, dropped_chars; - u32 cursor = new_cons_p->cursor; - - if (old_cons_p->cursor < old_cons_p->size) - copy_size = old_cons_p->cursor; - else - copy_size = old_cons_p->size; - - if (cursor > new_cons_p->size) - copy_size = 0; - else if (cursor + copy_size > new_cons_p->size) - copy_size = new_cons_p->size - cursor; - - dropped_chars = old_cons_p->cursor - copy_size; - if (dropped_chars) { - /* Reserve 80 chars to report overflow, if possible. */ - if (copy_size < 80) - return; - copy_size -= 80; - dropped_chars += 80; + u32 c; + + if (src_cons_p->overflowed) { + const char overflow_warning[] = "\n*** Pre-CBMEM console " + "overflowed, log truncated ***\n"; + for (c = 0; c < sizeof(overflow_warning) - 1; c++) + cbmemc_tx_byte(overflow_warning[c]); + for (c = src_cons_p->cursor; c < src_cons_p->size; c++) + cbmemc_tx_byte(src_cons_p->body[c]); }
- memcpy(new_cons_p->body + cursor, old_cons_p->body, - copy_size); - - cursor += copy_size; - - if (dropped_chars) { - const char loss_str1[] = "\n\n*** Log truncated, "; - const char loss_str2[] = " characters dropped. ***\n\n"; - - /* - * When running from ROM sprintf is not available, a simple - * itoa implementation is used instead. - */ - int got_first_digit = 0; - - /* Way more than possible number of dropped characters. */ - u32 mult = 100000; - - strcpy((char *)new_cons_p->body + cursor, loss_str1); - cursor += sizeof(loss_str1) - 1; - - while (mult) { - int digit = dropped_chars / mult; - if (got_first_digit || digit) { - new_cons_p->body[cursor++] = digit + '0'; - dropped_chars %= mult; - /* Excessive, but keeps it simple */ - got_first_digit = 1; - } - mult /= 10; - } + for (c = 0; c < src_cons_p->cursor; c++) + cbmemc_tx_byte(src_cons_p->body[c]);
- strcpy((char *)new_cons_p->body + cursor, loss_str2); - cursor += sizeof(loss_str2) - 1; - } - new_cons_p->cursor = cursor; + /* Invalidate the source console, so it will be reinitialized on the + next reboot. Otherwise, we might copy the same bytes again. */ + src_cons_p->size = 0; }
static void cbmemc_reinit(int is_recovery) { - struct cbmem_console *cbm_cons_p; const size_t size = CONFIG_CONSOLE_CBMEM_BUFFER_SIZE; - int flags = CBMEMC_APPEND; - - /* No appending when no preram console available and adding for - * the first time. */ - if (!ENV_RAMSTAGE && !ENV_POSTCAR && _preram_cbmem_console_size == 0) - flags = CBMEMC_RESET; - - /* Need to reset the newly added cbmem console in romstage. */ - if (ENV_ROMSTAGE) - flags |= CBMEMC_RESET; + /* If CBMEM entry already existed, old contents are not altered. */ + struct cbmem_console *cbmem_cons_p = cbmem_add(CBMEM_ID_CONSOLE, size); + struct cbmem_console *preram_cons_p = current_console();
- /* Need to reset the newly added cbmem console in ramstage - * when there was no console in preram environment. */ - if (ENV_RAMSTAGE && IS_ENABLED(CONFIG_LATE_CBMEM_INIT)) - flags |= CBMEMC_RESET; - - /* If CBMEM entry already existed, old contents is not altered. */ - cbm_cons_p = cbmem_add(CBMEM_ID_CONSOLE, size); - - init_console_ptr(cbm_cons_p, size, flags); + init_console_ptr(cbmem_cons_p, size); + if (preram_cons_p && current_console()) + copy_console_buffer(preram_cons_p); } ROMSTAGE_CBMEM_INIT_HOOK(cbmemc_reinit) RAMSTAGE_CBMEM_INIT_HOOK(cbmemc_reinit) @@ -253,13 +171,17 @@ POSTCAR_CBMEM_INIT_HOOK(cbmemc_reinit) void cbmem_dump_console(void) { struct cbmem_console *cbm_cons_p; - int cursor; + u32 cursor;
cbm_cons_p = current_console(); if (!cbm_cons_p) return;
uart_init(0); + if (cbm_cons_p->overflowed) + for (cursor = cbm_cons_p->cursor; + cursor < cbm_cons_p->size; cursor++) + uart_tx_byte(0, cbm_cons_p->body[cursor]); for (cursor = 0; cursor < cbm_cons_p->cursor; cursor++) uart_tx_byte(0, cbm_cons_p->body[cursor]); } diff --git a/util/cbmem/cbmem.c b/util/cbmem/cbmem.c index 99f25f8..b03e20c 100644 --- a/util/cbmem/cbmem.c +++ b/util/cbmem/cbmem.c @@ -601,52 +601,55 @@ static void dump_timestamps(int mach_readable) unmap_memory(); }
+struct cbmem_console { + u32 size; + u32 cursor : 24; + u32 _reserved_flags : 7; + u32 overflowed : 1; + u8 body[0]; +} __attribute__ ((__packed__)); + /* dump the cbmem console */ static void dump_console(void) { - void *console_p; + struct cbmem_console *console_p; char *console_c; - uint32_t size; - uint32_t cursor; + size_t size;
if (console.tag != LB_TAG_CBMEM_CONSOLE) { fprintf(stderr, "No console found in coreboot table.\n"); return; }
- console_p = map_memory_size((unsigned long)console.cbmem_addr, - 2 * sizeof(uint32_t), 1); - /* The in-memory format of the console area is: - * u32 size - * u32 cursor - * char console[size] - * Hence we have to add 8 to get to the actual console string. - */ - size = ((uint32_t *)console_p)[0]; - cursor = ((uint32_t *)console_p)[1]; - /* Cursor continues to go on even after no more data fits in - * the buffer but the data is dropped in this case. - */ - if (size > cursor) - size = cursor; - console_c = calloc(1, size + 1); + size = sizeof(*console_p); + console_p = map_memory_size((unsigned long)console.cbmem_addr, size, 1); + if (!console_p->overflowed && console_p->cursor < console_p->size) + size = console_p->cursor; + else + size = console_p->size; unmap_memory(); + + console_c = malloc(size + 1); if (!console_c) { fprintf(stderr, "Not enough memory for console.\n"); exit(1); } + console_c[size] = '\0';
console_p = map_memory_size((unsigned long)console.cbmem_addr, - size + sizeof(size) + sizeof(cursor), 1); - aligned_memcpy(console_c, console_p + 8, size); + size + sizeof(*console_p), 1); + if (console_p->overflowed) { + aligned_memcpy(console_c, console_p->body + console_p->cursor, + console_p->size - console_p->cursor); + aligned_memcpy(console_c + console_p->size - console_p->cursor, + console_p->body, console_p->cursor); + } else { + aligned_memcpy(console_c, console_p->body, size); + }
printf("%s\n", console_c); - if (size < cursor) - printf("%d %s lost\n", cursor - size, - (cursor - size) == 1 ? "byte":"bytes");
free(console_c); - unmap_memory(); }
-
Julius Werner