[coreboot] New patch to review for coreboot: 4cf3f38 oprom: Ensure that mode information is valid before putting it in the tables.

Stefan Reinauer (stefan.reinauer@coreboot.org) gerrit at coreboot.org
Wed Nov 7 01:36:50 CET 2012


Stefan Reinauer (stefan.reinauer at coreboot.org) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/1758

-gerrit

commit 4cf3f38265cac806e73673eaad9da7b46791d80c
Author: Gabe Black <gabeblack at google.com>
Date:   Sun Sep 30 04:47:48 2012 -0700

    oprom: Ensure that mode information is valid before putting it in the tables.
    
    At least when CONFIG_CHROMEOS is turned on, it's possible for
    CONFIG_FRAMEBUFFER_KEEP_VESA_MODE to be set but for there not to be any valid
    information to put into the framebuffer coreboot table. That means that what's
    put in there is junk, probably all zeroes from the uninitialized global
    variable the mode information is stored in (mode_info).
    
    When a payload uses libpayload and turns on the coreboot framebuffer console,
    that console will attempt to scroll at some point and decrease the cursor's y
    coordinate until it is less than the number of rows claimed by the console.
    The number of rows is computed by taking the vertical resolution of the
    framebuffer and dividing it by the height of the font. Because the mode
    information was all zeroes, the coreboot table info is all zeroes, and that
    means that the number of rows the console claims is zero. You can't get the
    unsigned y coordinate of the cursor to be less than zero, so libpayload gets
    stuck in an infinite loop.
    
    The solution this change implements is to add a new function,
    vbe_mode_info_valid, which simply returns whether or not mode_info has anything
    in it. If not, the framebuffer coreboot table is not created, and libpayload
    doesn't get stuck.
    
    Change-Id: I08f3ec628e4453f0cfe9e15c4d8dfd40327f91c9
    Signed-off-by: Gabe Black <gabeblack at google.com>
---
 src/arch/x86/boot/coreboot_table.c |  5 +++++
 src/devices/oprom/include/vbe.h    |  3 +++
 src/devices/oprom/x86.c            | 29 ++++++++++++++++++-----------
 src/devices/oprom/yabel/vbe.c      |  8 ++++++++
 src/devices/oprom/yabel/vbe.h      |  1 +
 5 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/src/arch/x86/boot/coreboot_table.c b/src/arch/x86/boot/coreboot_table.c
index d056837..18ec6d8 100644
--- a/src/arch/x86/boot/coreboot_table.c
+++ b/src/arch/x86/boot/coreboot_table.c
@@ -174,7 +174,12 @@ static void lb_framebuffer(struct lb_header *header)
 {
 #if CONFIG_FRAMEBUFFER_KEEP_VESA_MODE
 	void fill_lb_framebuffer(struct lb_framebuffer *framebuffer);
+	int vbe_mode_info_valid(void);
 
+	// If there isn't any mode info to put in the table, don't ask for it
+	// to be filled with junk.
+	if (!vbe_mode_info_valid())
+		return;
 	struct lb_framebuffer *framebuffer;
 	framebuffer = (struct lb_framebuffer *)lb_new_record(header);
 	framebuffer->tag = LB_TAG_FRAMEBUFFER;
diff --git a/src/devices/oprom/include/vbe.h b/src/devices/oprom/include/vbe.h
index f857b67..ab26d59 100644
--- a/src/devices/oprom/include/vbe.h
+++ b/src/devices/oprom/include/vbe.h
@@ -105,6 +105,9 @@ typedef struct {
 struct lb_framebuffer;
 
 void vbe_set_graphics(void);
+// A way to check if mode information collected by vbe_set_graphics is valid
+// and fill_lb_framebuffer will have real information to use.
+int vbe_mode_info_valid(void);
 void vbe_textmode_console(void);
 void fill_lb_framebuffer(struct lb_framebuffer *framebuffer);
 
diff --git a/src/devices/oprom/x86.c b/src/devices/oprom/x86.c
index cc70d74..1791a87 100644
--- a/src/devices/oprom/x86.c
+++ b/src/devices/oprom/x86.c
@@ -167,33 +167,40 @@ static void setup_realmode_idt(void)
 }
 
 #if CONFIG_FRAMEBUFFER_SET_VESA_MODE
-static u8 vbe_get_mode_info(vbe_mode_info_t * mode_info)
+vbe_mode_info_t mode_info;
+static int mode_info_valid;
+
+int vbe_mode_info_valid(void)
+{
+	return mode_info_valid;
+}
+
+static u8 vbe_get_mode_info(vbe_mode_info_t * mi)
 {
 	printk(BIOS_DEBUG, "VBE: Getting information about VESA mode %04x\n",
-		mode_info->video_mode);
+		mi->video_mode);
 	char *buffer = (char *)&__buffer;
 	u16 buffer_seg = (((unsigned long)buffer) >> 4) & 0xff00;
 	u16 buffer_adr = ((unsigned long)buffer) & 0xffff;
 	realmode_interrupt(0x10, VESA_GET_MODE_INFO, 0x0000,
-			mode_info->video_mode, 0x0000, buffer_seg, buffer_adr);
-	memcpy(mode_info->mode_info_block, buffer, sizeof(vbe_mode_info_t));
+			mi->video_mode, 0x0000, buffer_seg, buffer_adr);
+	memcpy(mi->mode_info_block, buffer, sizeof(vbe_mode_info_t));
+	mode_info_valid = 1;
 	return 0;
 }
 
-static u8 vbe_set_mode(vbe_mode_info_t * mode_info)
+static u8 vbe_set_mode(vbe_mode_info_t * mi)
 {
-	printk(BIOS_DEBUG, "VBE: Setting VESA mode %04x\n", mode_info->video_mode);
+	printk(BIOS_DEBUG, "VBE: Setting VESA mode %04x\n", mi->video_mode);
 	// request linear framebuffer mode
-	mode_info->video_mode |= (1 << 14);
+	mi->video_mode |= (1 << 14);
 	// request clearing of framebuffer
-	mode_info->video_mode &= ~(1 << 15);
-	realmode_interrupt(0x10, VESA_SET_MODE, mode_info->video_mode,
+	mi->video_mode &= ~(1 << 15);
+	realmode_interrupt(0x10, VESA_SET_MODE, mi->video_mode,
 			0x0000, 0x0000, 0x0000, 0x0000);
 	return 0;
 }
 
-vbe_mode_info_t mode_info;
-
 /* These two functions could probably even be generic between
  * yabel and x86 native. TBD later.
  */
diff --git a/src/devices/oprom/yabel/vbe.c b/src/devices/oprom/yabel/vbe.c
index a9cee30..9dbe07c 100644
--- a/src/devices/oprom/yabel/vbe.c
+++ b/src/devices/oprom/yabel/vbe.c
@@ -133,6 +133,13 @@ vbe_info(vbe_info_t * info)
 	return 0;
 }
 
+static int mode_info_valid;
+
+int vbe_mode_info_valid(void)
+{
+	return mode_info_valid;
+}
+
 // VBE Function 01h
 static u8
 vbe_get_mode_info(vbe_mode_info_t * mode_info)
@@ -167,6 +174,7 @@ vbe_get_mode_info(vbe_mode_info_t * mode_info)
 	memcpy(mode_info->mode_info_block,
 	       biosmem + ((M.x86.R_ES << 4) + M.x86.R_DI),
 	       sizeof(mode_info->mode_info_block));
+	mode_info_valid = 1;
 
 	//printf("Mode Info Dump:");
 	//dump(mode_info_block, 64);
diff --git a/src/devices/oprom/yabel/vbe.h b/src/devices/oprom/yabel/vbe.h
index 6ddeeef..bf286bc 100644
--- a/src/devices/oprom/yabel/vbe.h
+++ b/src/devices/oprom/yabel/vbe.h
@@ -16,6 +16,7 @@
 struct lb_framebuffer;
 
 void vbe_set_graphics(void);
+int vbe_mode_info_valid(void);
 void fill_lb_framebuffer(struct lb_framebuffer *framebuffer);
 void vbe_textmode_console(void);
 




More information about the coreboot mailing list