Attention is currently required from: Raul Rangel, Yu-Ping Wu. Hello Raul Rangel, Yu-Ping Wu,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/62506
to review the following change.
Change subject: libpayload: cbmem_console: Drop loglevel markers from snapshot ......................................................................
libpayload: cbmem_console: Drop loglevel markers from snapshot
coreboot recently introduced non-printable loglevel markers in the CBMEM console. Payloads were generally unaffected since they don't use log levels and it is still legal to append lines without a marker to the log. However, payloads using cbmem_console_snapshot() to display existing logs from coreboot have started seeing '?' characters in place of the markers. This patch fixes the issue by filtering out marker characters.
BUG=b:221909874
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I4a9e5d464508320cf43ea572d62896d38c2a128d --- M payloads/libpayload/drivers/cbmem_console.c 1 file changed, 24 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/62506/1
diff --git a/payloads/libpayload/drivers/cbmem_console.c b/payloads/libpayload/drivers/cbmem_console.c index 22d5312..df40ad0 100644 --- a/payloads/libpayload/drivers/cbmem_console.c +++ b/payloads/libpayload/drivers/cbmem_console.c @@ -80,11 +80,28 @@ do_write(buffer, count); }
+static void snapshot_putc(char *console, uint32_t *cursor, char c) +{ + /* This is BIOS_LOG_IS_MARKER() from coreboot. Due to stupid + licensing restrictions, we can't use it directly. */ + if (c >= 0x10 && c <= 0x18) + return; + + /* Slight memory corruption may occur between reboots and give us a few + unprintable characters like '\0'. Replace them with '?' on output. */ + if (!isprint(c) && !isspace(c)) + console[*cursor] = '?'; + else + console[*cursor] = c; + + *cursor += 1; +} + char *cbmem_console_snapshot(void) { const struct cbmem_console *const console_p = phys_to_virt(cbmem_console_p); char *console_c; - uint32_t size, cursor, overflow; + uint32_t size, cursor, overflow, newc, oldc;
if (!console_p) { printf("ERROR: No cbmem console found in coreboot table\n"); @@ -104,24 +121,19 @@ size); return NULL; } - console_c[size] = '\0';
+ newc = 0; if (overflow) { if (cursor >= size) { printf("ERROR: CBMEM console struct is corrupted\n"); return NULL; } - memcpy(console_c, console_p->body + cursor, size - cursor); - memcpy(console_c + size - cursor, console_p->body, cursor); - } else { - memcpy(console_c, console_p->body, size); + for (oldc = cursor; oldc < size; oldc++) + snapshot_putc(console_c, &newc, console_p->body[oldc]); } - - /* Slight memory corruption may occur between reboots and give us a few - unprintable characters like '\0'. Replace them with '?' on output. */ - for (cursor = 0; cursor < size; cursor++) - if (!isprint(console_c[cursor]) && !isspace(console_c[cursor])) - console_c[cursor] = '?'; + for (oldc = 0; oldc < size && oldc < cursor; oldc++) + snapshot_putc(console_c, &newc, console_p->body[oldc]); + console_c[newc] = '\0';
return console_c; }