[PATCH 0/2] EFI earlyprintk support

[PATCH 0/2] EFI earlyprintk support

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

These two patches cleanup the #include <linux/efi.h> duplication and add
support for earlyprintk=efi, which is the only way users can debug early
boot crashes without special hardware.

Matt Fleming (2):
  x86/efi: Include linux/efi.h in asm/efi.h
  x86/efi: Add EFI framebuffer earlyprintk support

 Documentation/kernel-parameters.txt  |   8 +-
 arch/x86/Kconfig.debug               |   9 ++
 arch/x86/boot/compressed/eboot.c     |   1 -
 arch/x86/include/asm/efi.h           |   4 +
 arch/x86/kernel/early_printk.c       |   6 ++
 arch/x86/kernel/setup.c              |   1 -
 arch/x86/platform/efi/Makefile       |   1 +
 arch/x86/platform/efi/early_printk.c | 191 +++++++++++++++++++++++++++++++++++
 arch/x86/platform/efi/efi.c          |   1 -
 arch/x86/platform/efi/efi_32.c       |   1 -
 arch/x86/platform/efi/efi_64.c       |   1 -
 arch/x86/platform/uv/bios_uv.c       |   1 -
 12 files changed, 216 insertions(+), 9 deletions(-)
 create mode 100644 arch/x86/platform/efi/early_printk.c

-- 
1.8.1.4

[PATCH 1/2] x86/efi: Include linux/efi.h in asm/efi.h

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

Every file that includes asm/efi.h also includes linux/efi.h. Just
include linux/efi.h directly and avoid the duplication.

Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Suggested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/x86/boot/compressed/eboot.c | 1 -
 arch/x86/include/asm/efi.h       | 2 ++
 arch/x86/kernel/setup.c          | 1 -
 arch/x86/platform/efi/efi.c      | 1 -
 arch/x86/platform/efi/efi_32.c   | 1 -
 arch/x86/platform/efi/efi_64.c   | 1 -
 arch/x86/platform/uv/bios_uv.c   | 1 -
 7 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index b7388a4..3f1dae2 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -7,7 +7,6 @@
  *
  * ----------------------------------------------------------------------- */
 
-#include <linux/efi.h>
 #include <linux/pci.h>
 #include <asm/efi.h>
 #include <asm/setup.h>
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 0062a01..b10ea9e 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -1,6 +1,8 @@
 #ifndef _ASM_X86_EFI_H
 #define _ASM_X86_EFI_H
 
+#include <linux/efi.h>
+
 #ifdef CONFIG_X86_32
 
 #define EFI_LOADER_SIGNATURE	"EL32"
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f0de629..35e9883 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -37,7 +37,6 @@
 #include <linux/root_dev.h>
 #include <linux/highmem.h>
 #include <linux/module.h>
-#include <linux/efi.h>
 #include <linux/init.h>
 #include <linux/edd.h>
 #include <linux/iscsi_ibft.h>
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index c7e22ab..543a4d9 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -30,7 +30,6 @@
 
 #include <linux/kernel.h>
 #include <linux/init.h>
-#include <linux/efi.h>
 #include <linux/efi-bgrt.h>
 #include <linux/export.h>
 #include <linux/bootmem.h>
diff --git a/arch/x86/platform/efi/efi_32.c b/arch/x86/platform/efi/efi_32.c
index 40e4469..dd566d1 100644
--- a/arch/x86/platform/efi/efi_32.c
+++ b/arch/x86/platform/efi/efi_32.c
@@ -22,7 +22,6 @@
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/ioport.h>
-#include <linux/efi.h>
 
 #include <asm/io.h>
 #include <asm/desc.h>
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index 39a0e7f..f146de9 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -23,7 +23,6 @@
 #include <linux/bootmem.h>
 #include <linux/ioport.h>
 #include <linux/module.h>
-#include <linux/efi.h>
 #include <linux/uaccess.h>
 #include <linux/io.h>
 #include <linux/reboot.h>
diff --git a/arch/x86/platform/uv/bios_uv.c b/arch/x86/platform/uv/bios_uv.c
index 7666121..e55b074 100644
--- a/arch/x86/platform/uv/bios_uv.c
+++ b/arch/x86/platform/uv/bios_uv.c
@@ -19,7 +19,6 @@
  *  Copyright (c) Russ Anderson <rja@sgi.com>
  */
 
-#include <linux/efi.h>
 #include <linux/export.h>
 #include <asm/efi.h>
 #include <linux/io.h>
-- 
1.8.1.4

[PATCH v2 2/2] x86/efi: Add EFI framebuffer earlyprintk support

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

It's incredibly difficult to diagnose early EFI boot issues without
special hardware because earlyprintk=vga doesn't work on EFI systems.

Add support for writing to the EFI framebuffer, via earlyprintk=efi,
which will actually give users a chance of providing debug output.

Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Jones <pjones@redhat.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---

Changes in v2:

 - add asm/efi.h to efi/early_printk.c for console prototype
 - Kconfig.debug: clear up grammar
 - remove uneeded paranthesis, due to precedence
 - delete spaces around multiplication
 - pass unsigned char to early_efi_write_char()
 - make 'dst' u32 * and get rid of memcpy()
 - delete mask code and write color directly
 - move efi_x, efi_y to top of file
 - standardize on unsigned int over int
 - s++ on separate line
 - switch ~(u32)0 for -1
 - scroll instead of clearing screen
 - rename early_efi_clear_line() early_efi_clear_scanline()

 Documentation/kernel-parameters.txt  |   8 +-
 arch/x86/Kconfig.debug               |   9 ++
 arch/x86/include/asm/efi.h           |   2 +
 arch/x86/kernel/early_printk.c       |   6 ++
 arch/x86/platform/efi/Makefile       |   1 +
 arch/x86/platform/efi/early_printk.c | 191 +++++++++++++++++++++++++++++++++++
 6 files changed, 214 insertions(+), 3 deletions(-)
 create mode 100644 arch/x86/platform/efi/early_printk.c

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index fcbb736..c07cb09 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -847,6 +847,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 
 	earlyprintk=	[X86,SH,BLACKFIN,ARM]
 			earlyprintk=vga
+			earlyprintk=efi
 			earlyprintk=xen
 			earlyprintk=serial[,ttySn[,baudrate]]
 			earlyprintk=serial[,0x...[,baudrate]]
@@ -860,7 +861,8 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			Append ",keep" to not disable it when the real console
 			takes over.
 
-			Only vga or serial or usb debug port at a time.
+			Only one of vga, efi, serial, or usb debug port can
+			be used at a time.
 
 			Currently only ttyS0 and ttyS1 may be specified by
 			name.  Other I/O ports may be explicitly specified
@@ -874,8 +876,8 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			Interaction with the standard serial driver is not
 			very good.
 
-			The VGA output is eventually overwritten by the real
-			console.
+			The VGA and EFI output is eventually overwritten by
+			the real console.
 
 			The xen output can only be used by Xen PV guests.
 
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 78d91af..b6fe388 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -59,6 +59,15 @@ config EARLY_PRINTK_DBGP
 	  with klogd/syslogd or the X server. You should normally N here,
 	  unless you want to debug such a crash. You need usb debug device.
 
+config EARLY_PRINTK_EFI
+	bool "Early printk via the EFI framebuffer"
+	depends on EFI && EARLY_PRINTK && FONT_SUPPORT
+	---help---
+	  Write kernel log output directly into the EFI framebuffer.
+
+	  This is useful for kernel debugging when your machine crashes very
+	  early before the console code is initialized.
+
 config X86_PTDUMP
 	bool "Export kernel pagetable layout to userspace via debugfs"
 	depends on DEBUG_KERNEL
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index b10ea9e..6b65df0 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -111,6 +111,8 @@ static inline bool efi_is_native(void)
 	return IS_ENABLED(CONFIG_X86_64) == efi_enabled(EFI_64BIT);
 }
 
+extern struct console early_efi_console;
+
 #else
 /*
  * IF EFI is not configured, have the EFI calls return -ENOSYS.
diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
index d15f575..66f9d93 100644
--- a/arch/x86/kernel/early_printk.c
+++ b/arch/x86/kernel/early_printk.c
@@ -17,6 +17,7 @@
 #include <asm/mrst.h>
 #include <asm/pgtable.h>
 #include <linux/usb/ehci_def.h>
+#include <asm/efi.h>
 
 /* Simple VGA output */
 #define VGABASE		(__ISA_IO_base + 0xb8000)
@@ -234,6 +235,11 @@ static int __init setup_early_printk(char *buf)
 			early_console_register(&early_hsu_console, keep);
 		}
 #endif
+#ifdef CONFIG_EARLY_PRINTK_EFI
+		if (!strncmp(buf, "efi", 3))
+			early_console_register(&early_efi_console, keep);
+#endif
+
 		buf++;
 	}
 	return 0;
diff --git a/arch/x86/platform/efi/Makefile b/arch/x86/platform/efi/Makefile
index 6db1cc4..b7b0b35 100644
--- a/arch/x86/platform/efi/Makefile
+++ b/arch/x86/platform/efi/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_EFI) 		+= efi.o efi_$(BITS).o efi_stub_$(BITS).o
 obj-$(CONFIG_ACPI_BGRT) += efi-bgrt.o
+obj-$(CONFIG_EARLY_PRINTK_EFI)	+= early_printk.o
diff --git a/arch/x86/platform/efi/early_printk.c b/arch/x86/platform/efi/early_printk.c
new file mode 100644
index 0000000..b10c009
--- /dev/null
+++ b/arch/x86/platform/efi/early_printk.c
@@ -0,0 +1,191 @@
+/*
+ * Copyright (C) 2013 Intel Corporation; author Matt Fleming
+ *
+ *  This file is part of the Linux kernel, and is made available under
+ *  the terms of the GNU General Public License version 2.
+ */
+
+#include <linux/console.h>
+#include <linux/font.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <asm/efi.h>
+#include <asm/setup.h>
+
+static const struct font_desc *font;
+static u32 efi_x, efi_y;
+
+static __init void early_efi_clear_scanline(unsigned int y)
+{
+	unsigned long base, *dst;
+	u16 len;
+
+	base = boot_params.screen_info.lfb_base;
+	len = boot_params.screen_info.lfb_linelength;
+
+	dst = early_ioremap(base + y*len, len);
+	if (!dst)
+		return;
+
+	memset(dst, 0, len);
+	early_iounmap(dst, len);
+}
+
+static __init void early_efi_scroll_up(void)
+{
+	unsigned long base, *dst, *src;
+	u16 len;
+	u32 i, height;
+
+	base = boot_params.screen_info.lfb_base;
+	len = boot_params.screen_info.lfb_linelength;
+	height = boot_params.screen_info.lfb_height;
+
+	for (i = 0; i < height - font->height; i++) {
+		dst = early_ioremap(base + i*len, len);
+		if (!dst)
+			return;
+
+		src = early_ioremap(base + (i + font->height) * len, len);
+		if (!src) {
+			early_iounmap(dst, len);
+			return;
+		}
+
+		memmove(dst, src, len);
+
+		early_iounmap(src, len);
+		early_iounmap(dst, len);
+	}
+}
+
+static void early_efi_write_char(u32 *dst, unsigned char c, unsigned int h)
+{
+	const u32 color_black = 0x00000000;
+	const u32 color_grey = 0x00aaaaaa;
+	const u8 *src;
+	u8 s8;
+	int m;
+
+	src = font->data + c * font->height;
+	s8 = *(src + h);
+
+	for (m = 0; m < 8; m++) {
+		if ((s8 >> (7 - m)) & 1)
+			*dst = color_grey;
+		else
+			*dst = color_black;
+		dst++;
+	}
+}
+
+static __init void
+early_efi_write(struct console *con, const char *str, unsigned int num)
+{
+	struct screen_info *si;
+	unsigned long base;
+	unsigned int len;
+	const char *s;
+	void *dst;
+
+	base = boot_params.screen_info.lfb_base;
+	si = &boot_params.screen_info;
+	len = si->lfb_linelength;
+
+	while (num) {
+		unsigned int linemax;
+		unsigned int h, count = 0;
+
+		for (s = str; *s && *s != '\n'; s++) {
+			if (count == num)
+				break;
+			count++;
+		}
+
+		linemax = (si->lfb_width - efi_x) / font->width;
+		if (count > linemax)
+			count = linemax;
+
+		for (h = 0; h < font->height; h++) {
+			unsigned int n, x;
+
+			dst = early_ioremap(base + (efi_y + h) * len, len);
+			if (!dst)
+				return;
+
+			s = str;
+			n = count;
+			x = efi_x;
+
+			while (n-- > 0) {
+				early_efi_write_char(dst + x*4, *s, h);
+				x += font->width;
+				s++;
+			}
+
+			early_iounmap(dst, len);
+		}
+
+		num -= count;
+		efi_x += count * font->width;
+		str += count;
+
+		if (num > 0 && *s == '\n') {
+			efi_x = 0;
+			efi_y += font->height;
+			str++;
+			num--;
+		}
+
+		if (efi_x >= si->lfb_width) {
+			efi_x = 0;
+			efi_y += font->height;
+		}
+
+		if (efi_y + font->height >= si->lfb_height) {
+			u32 i;
+
+			efi_y -= font->height;
+			early_efi_scroll_up();
+
+			for (i = 0; i < font->height; i++)
+				early_efi_clear_scanline(efi_y + i);
+		}
+	}
+}
+
+static __init int early_efi_setup(struct console *con, char *options)
+{
+	struct screen_info *si;
+	u16 xres, yres;
+	u32 i;
+
+	si = &boot_params.screen_info;
+	xres = si->lfb_width;
+	yres = si->lfb_height;
+
+	/*
+	 * early_efi_write_char() implicitly assumes a framebuffer with
+	 * 32-bits per pixel.
+	 */
+	if (si->lfb_depth != 32)
+		return -ENODEV;
+
+	font = get_default_font(xres, yres, -1, -1);
+	if (!font)
+		return -ENODEV;
+
+	efi_y = rounddown(yres, font->height) - font->height;
+	for (i = 0; i < (yres - efi_y) / font->height; i++)
+		early_efi_scroll_up();
+
+	return 0;
+}
+
+struct console early_efi_console = {
+	.name =		"earlyefi",
+	.write =	early_efi_write,
+	.setup =	early_efi_setup,
+	.flags =	CON_PRINTBUFFER,
+	.index =	-1,
+};
-- 
1.8.1.4

Re: [PATCH 1/2] x86/efi: Include linux/efi.h in asm/efi.h

[H. Peter Anvin]

The patch description doesn't match what the patch does.  We do not normally have the asm file include the linux file, which is what the patch seems to do.

Matt Fleming <matt@console-pimps.org> wrote:
>From: Matt Fleming <matt.fleming@intel.com>
>
>Every file that includes asm/efi.h also includes linux/efi.h. Just
>include linux/efi.h directly and avoid the duplication.
>
>Cc: H. Peter Anvin <hpa@zytor.com>
>Cc: Thomas Gleixner <tglx@linutronix.de>
>Suggested-by: Ingo Molnar <mingo@kernel.org>
>Signed-off-by: Matt Fleming <matt.fleming@intel.com>
>---
> arch/x86/boot/compressed/eboot.c | 1 -
> arch/x86/include/asm/efi.h       | 2 ++
> arch/x86/kernel/setup.c          | 1 -
> arch/x86/platform/efi/efi.c      | 1 -
> arch/x86/platform/efi/efi_32.c   | 1 -
> arch/x86/platform/efi/efi_64.c   | 1 -
> arch/x86/platform/uv/bios_uv.c   | 1 -
> 7 files changed, 2 insertions(+), 6 deletions(-)
>
>diff --git a/arch/x86/boot/compressed/eboot.c
>b/arch/x86/boot/compressed/eboot.c
>index b7388a4..3f1dae2 100644
>--- a/arch/x86/boot/compressed/eboot.c
>+++ b/arch/x86/boot/compressed/eboot.c
>@@ -7,7 +7,6 @@
>  *
>*
>-----------------------------------------------------------------------
>*/
> 
>-#include <linux/efi.h>
> #include <linux/pci.h>
> #include <asm/efi.h>
> #include <asm/setup.h>
>diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
>index 0062a01..b10ea9e 100644
>--- a/arch/x86/include/asm/efi.h
>+++ b/arch/x86/include/asm/efi.h
>@@ -1,6 +1,8 @@
> #ifndef _ASM_X86_EFI_H
> #define _ASM_X86_EFI_H
> 
>+#include <linux/efi.h>
>+
> #ifdef CONFIG_X86_32
> 
> #define EFI_LOADER_SIGNATURE	"EL32"
>diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
>index f0de629..35e9883 100644
>--- a/arch/x86/kernel/setup.c
>+++ b/arch/x86/kernel/setup.c
>@@ -37,7 +37,6 @@
> #include <linux/root_dev.h>
> #include <linux/highmem.h>
> #include <linux/module.h>
>-#include <linux/efi.h>
> #include <linux/init.h>
> #include <linux/edd.h>
> #include <linux/iscsi_ibft.h>
>diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
>index c7e22ab..543a4d9 100644
>--- a/arch/x86/platform/efi/efi.c
>+++ b/arch/x86/platform/efi/efi.c
>@@ -30,7 +30,6 @@
> 
> #include <linux/kernel.h>
> #include <linux/init.h>
>-#include <linux/efi.h>
> #include <linux/efi-bgrt.h>
> #include <linux/export.h>
> #include <linux/bootmem.h>
>diff --git a/arch/x86/platform/efi/efi_32.c
>b/arch/x86/platform/efi/efi_32.c
>index 40e4469..dd566d1 100644
>--- a/arch/x86/platform/efi/efi_32.c
>+++ b/arch/x86/platform/efi/efi_32.c
>@@ -22,7 +22,6 @@
> #include <linux/kernel.h>
> #include <linux/types.h>
> #include <linux/ioport.h>
>-#include <linux/efi.h>
> 
> #include <asm/io.h>
> #include <asm/desc.h>
>diff --git a/arch/x86/platform/efi/efi_64.c
>b/arch/x86/platform/efi/efi_64.c
>index 39a0e7f..f146de9 100644
>--- a/arch/x86/platform/efi/efi_64.c
>+++ b/arch/x86/platform/efi/efi_64.c
>@@ -23,7 +23,6 @@
> #include <linux/bootmem.h>
> #include <linux/ioport.h>
> #include <linux/module.h>
>-#include <linux/efi.h>
> #include <linux/uaccess.h>
> #include <linux/io.h>
> #include <linux/reboot.h>
>diff --git a/arch/x86/platform/uv/bios_uv.c
>b/arch/x86/platform/uv/bios_uv.c
>index 7666121..e55b074 100644
>--- a/arch/x86/platform/uv/bios_uv.c
>+++ b/arch/x86/platform/uv/bios_uv.c
>@@ -19,7 +19,6 @@
>  *  Copyright (c) Russ Anderson <rja@sgi.com>
>  */
> 
>-#include <linux/efi.h>
> #include <linux/export.h>
> #include <asm/efi.h>
> #include <linux/io.h>

-- 
Sent from my mobile phone.  Please pardon brevity and lack of formatting.

Re: [PATCH 1/2] x86/efi: Include linux/efi.h in asm/efi.h

[Matt Fleming]

On Fri, 11 Oct, at 08:25:26AM, H. Peter Anvin wrote:
> The patch description doesn't match what the patch does. 

Oh, bah. I see I wrote the patch description in a weird way. It should
have said something like,

  "Every file that includes asm/efi.h also includes linux/efi.h. Move
   the inclusion of the linux file into asm/efi.h so that users only
   need to include one header."

> We do not normally have the asm file include the linux file, which is
> what the patch seems to do.

Ingo suggested this because no file in arch/x86 includes asm/efi.h
without also including linux/efi.h, because asm/efi.h makes use of the
definitions in linux/efi.h. Admittedly this whole thing is a little
backwards.

I know that the usual idiom is to include the linux file, but x86 is the
only architecture to provide an asm/efi.h header, which means that to
stick with the usual idiom, we'd need an #ifdef CONFIG_X86 in
linux/efi.h to automatically include the asm file.

Unless we have an empty efi.h in include/asm-generic?

In fact, now I think about it, that would be a much better solution
because there's already a bunch of x86-specific defintions in
linux/efi.h, so the asm-generic/efi.h wouldn't be empty because we could
split out things like...


#ifdef CONFIG_X86
extern void efi_late_init(void);
extern void efi_free_boot_services(void);
extern efi_status_t efi_query_variable_store(u32 attributes, unsigned long size);
#else
static inline void efi_late_init(void) {}
static inline void efi_free_boot_services(void) {}

static inline efi_status_t efi_query_variable_store(u32 attributes, unsigned long size)
{
	return EFI_SUCCESS;
}
#endif


Thoughts?
 
[ Patch included because I dropped Ingo from original Cc list ]

> Matt Fleming <matt@console-pimps.org> wrote:
> >From: Matt Fleming <matt.fleming@intel.com>
> >
> >Every file that includes asm/efi.h also includes linux/efi.h. Just
> >include linux/efi.h directly and avoid the duplication.
> >
> >Cc: H. Peter Anvin <hpa@zytor.com>
> >Cc: Thomas Gleixner <tglx@linutronix.de>
> >Suggested-by: Ingo Molnar <mingo@kernel.org>
> >Signed-off-by: Matt Fleming <matt.fleming@intel.com>
> >---
> > arch/x86/boot/compressed/eboot.c | 1 -
> > arch/x86/include/asm/efi.h       | 2 ++
> > arch/x86/kernel/setup.c          | 1 -
> > arch/x86/platform/efi/efi.c      | 1 -
> > arch/x86/platform/efi/efi_32.c   | 1 -
> > arch/x86/platform/efi/efi_64.c   | 1 -
> > arch/x86/platform/uv/bios_uv.c   | 1 -
> > 7 files changed, 2 insertions(+), 6 deletions(-)
> >
> >diff --git a/arch/x86/boot/compressed/eboot.c
> >b/arch/x86/boot/compressed/eboot.c
> >index b7388a4..3f1dae2 100644
> >--- a/arch/x86/boot/compressed/eboot.c
> >+++ b/arch/x86/boot/compressed/eboot.c
> >@@ -7,7 +7,6 @@
> >  *
> >*
> >-----------------------------------------------------------------------
> >*/
> > 
> >-#include <linux/efi.h>
> > #include <linux/pci.h>
> > #include <asm/efi.h>
> > #include <asm/setup.h>
> >diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
> >index 0062a01..b10ea9e 100644
> >--- a/arch/x86/include/asm/efi.h
> >+++ b/arch/x86/include/asm/efi.h
> >@@ -1,6 +1,8 @@
> > #ifndef _ASM_X86_EFI_H
> > #define _ASM_X86_EFI_H
> > 
> >+#include <linux/efi.h>
> >+
> > #ifdef CONFIG_X86_32
> > 
> > #define EFI_LOADER_SIGNATURE	"EL32"
> >diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> >index f0de629..35e9883 100644
> >--- a/arch/x86/kernel/setup.c
> >+++ b/arch/x86/kernel/setup.c
> >@@ -37,7 +37,6 @@
> > #include <linux/root_dev.h>
> > #include <linux/highmem.h>
> > #include <linux/module.h>
> >-#include <linux/efi.h>
> > #include <linux/init.h>
> > #include <linux/edd.h>
> > #include <linux/iscsi_ibft.h>
> >diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> >index c7e22ab..543a4d9 100644
> >--- a/arch/x86/platform/efi/efi.c
> >+++ b/arch/x86/platform/efi/efi.c
> >@@ -30,7 +30,6 @@
> > 
> > #include <linux/kernel.h>
> > #include <linux/init.h>
> >-#include <linux/efi.h>
> > #include <linux/efi-bgrt.h>
> > #include <linux/export.h>
> > #include <linux/bootmem.h>
> >diff --git a/arch/x86/platform/efi/efi_32.c
> >b/arch/x86/platform/efi/efi_32.c
> >index 40e4469..dd566d1 100644
> >--- a/arch/x86/platform/efi/efi_32.c
> >+++ b/arch/x86/platform/efi/efi_32.c
> >@@ -22,7 +22,6 @@
> > #include <linux/kernel.h>
> > #include <linux/types.h>
> > #include <linux/ioport.h>
> >-#include <linux/efi.h>
> > 
> > #include <asm/io.h>
> > #include <asm/desc.h>
> >diff --git a/arch/x86/platform/efi/efi_64.c
> >b/arch/x86/platform/efi/efi_64.c
> >index 39a0e7f..f146de9 100644
> >--- a/arch/x86/platform/efi/efi_64.c
> >+++ b/arch/x86/platform/efi/efi_64.c
> >@@ -23,7 +23,6 @@
> > #include <linux/bootmem.h>
> > #include <linux/ioport.h>
> > #include <linux/module.h>
> >-#include <linux/efi.h>
> > #include <linux/uaccess.h>
> > #include <linux/io.h>
> > #include <linux/reboot.h>
> >diff --git a/arch/x86/platform/uv/bios_uv.c
> >b/arch/x86/platform/uv/bios_uv.c
> >index 7666121..e55b074 100644
> >--- a/arch/x86/platform/uv/bios_uv.c
> >+++ b/arch/x86/platform/uv/bios_uv.c
> >@@ -19,7 +19,6 @@
> >  *  Copyright (c) Russ Anderson <rja@sgi.com>
> >  */
> > 
> >-#include <linux/efi.h>
> > #include <linux/export.h>
> > #include <asm/efi.h>
> > #include <linux/io.h>
> 
> -- 
> Sent from my mobile phone.  Please pardon brevity and lack of formatting.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH v2 2/2] x86/efi: Add EFI framebuffer earlyprintk support

[Ingo Molnar]

* Matt Fleming <matt@console-pimps.org> wrote:

> From: Matt Fleming <matt.fleming@intel.com>
> 
> It's incredibly difficult to diagnose early EFI boot issues without
> special hardware because earlyprintk=vga doesn't work on EFI systems.
> 
> Add support for writing to the EFI framebuffer, via earlyprintk=efi,
> which will actually give users a chance of providing debug output.
> 
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Peter Jones <pjones@redhat.com>
> Signed-off-by: Matt Fleming <matt.fleming@intel.com>
> 
>  Documentation/kernel-parameters.txt  |   8 +-
>  arch/x86/Kconfig.debug               |   9 ++
>  arch/x86/include/asm/efi.h           |   2 +
>  arch/x86/kernel/early_printk.c       |   6 ++
>  arch/x86/platform/efi/Makefile       |   1 +
>  arch/x86/platform/efi/early_printk.c | 191 +++++++++++++++++++++++++++++++++++
>  6 files changed, 214 insertions(+), 3 deletions(-)
>  create mode 100644 arch/x86/platform/efi/early_printk.c

Looks really nice now!

Reviewed-by: Ingo Molnar <mingo@kernel.org>

One detail:

> +static void early_efi_write_char(u32 *dst, unsigned char c, unsigned int h)
> +{
> +	const u32 color_black = 0x00000000;
> +	const u32 color_grey = 0x00aaaaaa;

I still think this should be white - that should be more visible than 
grey, especially in brightly lit places. (And I'm really sorry about nerds 
in the basement getting eye damage.)

Also, what is the highest byte typically, can that ever be part of 
multiple framebuffers and can it mean an alpha channel? If yes then '0x00' 
(read: fully translucent) might not be the best of choices for console 
output. In that case -1 (0xffffffff) would work in a natural way.

(If it's reserved bits then it has to be all zeroes.)

Thanks,

	Ingo

Re: [PATCH v2 2/2] x86/efi: Add EFI framebuffer earlyprintk support

[Matt Fleming]

On Sat, 12 Oct, at 07:56:15PM, Ingo Molnar wrote:
> > +static void early_efi_write_char(u32 *dst, unsigned char c, unsigned int h)
> > +{
> > +	const u32 color_black = 0x00000000;
> > +	const u32 color_grey = 0x00aaaaaa;
> 
> I still think this should be white - that should be more visible than 
> grey, especially in brightly lit places. (And I'm really sorry about nerds 
> in the basement getting eye damage.)
> 
> Also, what is the highest byte typically, can that ever be part of 
> multiple framebuffers and can it mean an alpha channel? If yes then '0x00' 
> (read: fully translucent) might not be the best of choices for console 
> output. In that case -1 (0xffffffff) would work in a natural way.
> 
> (If it's reserved bits then it has to be all zeroes.)

The GOP part of the spec says that the top byte is reserved, and must be
set to zero, and looking at the efifb driver it disables transparency
support when allocating the color map,

int fb_alloc_cmap(struct fb_cmap *cmap, int len, int transp)

[...]

	if ((err = fb_alloc_cmap(&info->cmap, 256, 0)) < 0) {
		printk(KERN_ERR "efifb: cannot allocate colormap\n");
		goto err_unmap;
	}


-- 
Matt Fleming, Intel Open Source Technology Center