[coreboot-gerrit] New patch to review for coreboot: coreinfo: Rewrite bootlog_module

Yasha Cherikovsky (yasha.che3@gmail.com) gerrit at coreboot.org
Sat Nov 14 18:11:16 CET 2015


Yasha Cherikovsky (yasha.che3 at gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/12440

-gerrit

commit 6d55fd59fe680f795380982ccea32d0d7f465ec0
Author: Yasha Cherikovsky <yasha.cherikovsky at gmail.com>
Date:   Sat Nov 14 18:52:35 2015 +0200

    coreinfo: Rewrite bootlog_module
    
    The old bootlog_module implementation was completely broken:
    - It assumed that the console buffer is located at address 0x90000,
      and of size 64K. It is not correct nowadays.
    - It displayed the buffer in a very hacky way, the code was riddled with
      TODOs and FIXMEs. Scrolling had sometimes unexpected behavior.
    
    The new implementation:
    - Uses the cbmem console as the source of data.
      It takes the console information from lib_sysinfo of libpayload, which is
      constructed from the coreboot tables (no more hardcoded adressess).
    - Properly sanitizes the console buffer for display, which makes
      scolling and display much easier to implement.
    
    Change-Id: I3f87ec920631da2acfd3f52273228703f22f469f
    Signed-off-by: Yasha Cherikovsky <yasha.che3 at gmail.com>
---
 payloads/coreinfo/bootlog_module.c | 209 +++++++++++++++++++++++++------------
 1 file changed, 142 insertions(+), 67 deletions(-)

diff --git a/payloads/coreinfo/bootlog_module.c b/payloads/coreinfo/bootlog_module.c
index 27653d3..9e72e70 100644
--- a/payloads/coreinfo/bootlog_module.c
+++ b/payloads/coreinfo/bootlog_module.c
@@ -17,33 +17,130 @@
 
 #if IS_ENABLED(CONFIG_MODULE_BOOTLOG)
 
-#define CONFIG_COREBOOT_PRINTK_BUFFER_ADDR 0x90000
-#define CONFIG_COREBOOT_PRINTK_BUFFER_SIZE 65536
+#define LINES_SHOWN 19
+#define TAB_WIDTH 2
 
-static char *buf;
-static s32 cursor = 0;
-static s32 cursor_max;
 
-static int bootlog_module_init(void)
-{
-	int i;
-	volatile unsigned long *ptr =
-		(void *)(CONFIG_COREBOOT_PRINTK_BUFFER_ADDR + 16); /* FIXME */
+/* Globals that are used for tracking screen state */
+static char *g_buf = NULL;
+static s32 g_line = 0;
+static s32 g_lines_count = 0;
+static s32 g_max_cursor_line = 0;
+
+
+/* Copied from libpayload/drivers/cbmem_console.c */
+struct cbmem_console {
+	uint32_t size;
+	uint32_t cursor;
+	uint8_t body[0];
+} __attribute__ ((__packed__));
+
+
+static u32 char_width(char c, u32 cursor, u32 screen_width) {
+	if (c == '\n') {
+		return screen_width - (cursor % screen_width);
+	} else if (c == '\t') {
+		return TAB_WIDTH;
+	} else if (isprint(c)) {
+		return 1;
+	}
+
+	return 0;
+}
+
+static u32 calculate_chars_count(char *str, u32 str_len, u32 screen_width, u32 screen_height) {
+	u32 i, count = 0;
+
+	for (i = 0; i < str_len; i++) {
+		count += char_width(str[i], count, screen_width);
+	}
+
+	/* Ensure that 'count' can occupy at least the whole screen */
+	if (count < screen_width * screen_height) {
+		count = screen_width * screen_height;
+	}
+
+	/* Pad to line end */
+	if (count % screen_width != 0) {
+		count += screen_width - (count % screen_width);
+	}
+
+	return count;
+}
+
+/*
+ * This method takes an input buffer and sanitizes it for display, which means:
+ *  - '\n' is converted to spaces until end of line
+ *  - Tabs are converted to spaces of size TAB_WIDTH
+ *  - Only printable characters are preserved
+ */
+static int sanitize_buffer_for_display(char *str, u32 str_len, char *out, u32 out_len, u32 screen_width) {
+	u32 cursor = 0;
+	u32 i;
+
+	for (i = 0; i < str_len && cursor < out_len; i++) {
+		u32 width = char_width(str[i], cursor, screen_width);
+		if (width == 1) {
+			out[cursor++] = str[i];
+		} else if (width > 1) {
+			while (width-- && cursor < out_len) {
+				out[cursor++] = ' ';
+			}
+		}
+	}
 
-	buf = malloc(CONFIG_COREBOOT_PRINTK_BUFFER_SIZE);
-	if (!buf) {
-		/* TODO */
+	/* Fill the rest of the out buffer with spaces */
+	while (cursor < out_len) {
+		out[cursor++] = ' ';
 	}
 
-	memcpy(buf, (char *)ptr, CONFIG_COREBOOT_PRINTK_BUFFER_SIZE);
+	return 0;
+}
+
+static int bootlog_module_init(void) {
+	/* Make sure that lib_sysinfo is initialized */
+	int ret = lib_get_sysinfo();
+	if (ret) {
+		return -1;
+	}
+
+	struct cbmem_console *console = lib_sysinfo.cbmem_cons;
+	if (console == NULL) {
+		return -1;
+	}
+	/* Extract console information */
+	char *buffer = (char *)(&(console->body));
+	u32 buffer_size = console->size;
+	u32 cursor = console->cursor;
+
+	/* The cursor may be bigger than buffer size when the buffer is full */
+	if (cursor >= buffer_size) {
+		cursor = buffer_size - 1;
+	}
+
+	/* Calculate how much characters will be displayed on screen */
+	u32 chars_count = calculate_chars_count(buffer, cursor + 1, SCREEN_X, LINES_SHOWN);
+
+	/* Sanity check, chars_count must be padded to full line */
+	if (chars_count % SCREEN_X != 0) {
+		return -2;
+	}
+
+	g_lines_count = chars_count / SCREEN_X;
+	g_max_cursor_line = MAX(g_lines_count - 1 - LINES_SHOWN, 0);
 
-	cursor_max = CONFIG_COREBOOT_PRINTK_BUFFER_SIZE;
-	for (i = 0; i < 20; i++) {
-		do {
-			cursor_max--;
-		} while (*(buf + cursor_max) != '\n');
+	g_buf = malloc(chars_count);
+	if (!g_buf) {
+		return -3;
+	}
+
+	if (sanitize_buffer_for_display(buffer, cursor + 1,
+									g_buf, chars_count,
+									SCREEN_X) < 0) {
+		free(g_buf);
+		g_buf = NULL;
+		return -4;
 	}
-	cursor_max++;	/* Stay _behind_ the newline. */
 
 	/* TODO: Maybe a _cleanup hook where we call free()? */
 
@@ -52,74 +149,52 @@ static int bootlog_module_init(void)
 
 static int bootlog_module_redraw(WINDOW *win)
 {
-	int x = 0, y = 0;
-	char *tmp = buf + cursor;
-
 	print_module_title(win, "Coreboot Bootlog");
 
-	/* FIXME: Handle lines longer than 80 characters. */
-	while (y <= 18) {
-		mvwaddch(win, y + 2, x, isprint(*tmp) ? *tmp : ' ');
-		x++;
-		tmp++;
-		if (*tmp == '\n') {
-			y++;
-			x = 0;
-			tmp++;		/* Skip the newline. */
+	if (!g_buf) {
+		return -1;
+	}
+
+	int x = 0, y = 0;
+	char *tmp = g_buf + g_line * SCREEN_X;
+
+	for (y = 0; y < LINES_SHOWN; y++) {
+		for (x = 0; x < SCREEN_X; x++) {
+			mvwaddch(win, y + 2, x, *tmp);
+			tmp++;
 		}
+
 	}
 
 	return 0;
 }
 
-/* TODO: Simplify code. */
 static int bootlog_module_handle(int key)
 {
-	int i;
+	if (!g_buf) {
+		return 0;
+	}
 
 	switch (key) {
 	case KEY_DOWN:
-		if (cursor == cursor_max)
-			return 0;
-		while (*(buf + cursor) != '\n')
-			cursor++;
-		cursor++;	/* Skip the newline. */
+		g_line++;
 		break;
 	case KEY_UP:
-		if (cursor == 0)
-			return 0;
-		cursor--;	/* Skip the newline. */
-		do {
-			cursor--;
-		} while (*(buf + cursor) != '\n');
-		cursor++;	/* Stay _behind_ the newline. */
+		g_line--;
 		break;
-	case KEY_NPAGE:
-		if (cursor == cursor_max)
-			return 0;
-		for (i = 0; i < 20; i++) {
-			while (*(buf + cursor) != '\n')
-				cursor++;
-			cursor++;	/* Skip the newline. */
-		}
+	case KEY_NPAGE: /* Page up */
+		g_line -= LINES_SHOWN;
 		break;
-	case KEY_PPAGE:
-		if (cursor == 0)
-			return 0;
-		for (i = 0; i < 20; i++) {
-			do {
-				cursor--;
-			} while (*(buf + cursor) != '\n');
-		}
-		cursor++;	/* Stay _behind_ the newline. */
+	case KEY_PPAGE: /* Page down */
+		g_line += LINES_SHOWN;
 		break;
 	}
 
-	if (cursor > cursor_max)
-		cursor = cursor_max;
+	if (g_line < 0)
+		g_line = 0;
 
-	if (cursor < 0)
-		cursor = 0;
+	if (g_line > g_max_cursor_line)
+		g_line = g_max_cursor_line;
 
 	return 1;
 }



More information about the coreboot-gerrit mailing list