[PATCH 1/6] x86, efi: Fix display detection in EFI boot stub

[PATCH 1/6] x86, efi: Fix display detection in EFI boot stub

[David Woodhouse]

From: David Woodhouse <David.Woodhouse-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

When booting under OVMF we have precisely one GOP device, and it
implements the ConOut protocol.

We break out of the loop when we look at it... and then promptly abort
because 'first_gop' never gets set. We should set first_gop *before*
breaking out of the loop. Yes, it doesn't really mean "first" any more,
but that doesn't matter. It's only a flag to indicate that a suitable
GOP was found.

In fact, we'd do just as well to initialise 'width' to zero in this
function, then just check *that* instead of first_gop. But I'll do the
minimal fix for now (and for stable@).

Signed-off-by: David Woodhouse <David.Woodhouse-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
---
 arch/x86/boot/compressed/eboot.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index b1942e2..03d8c9b 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -432,10 +432,9 @@ static efi_status_t setup_gop(struct screen_info *si, efi_guid_t *proto,
 			 * Once we've found a GOP supporting ConOut,
 			 * don't bother looking any further.
 			 */
+			first_gop = gop;
 			if (conout_found)
 				break;
-
-			first_gop = gop;
 		}
 	}
 
-- 
1.8.0.1

[PATCH 2/6] x86, efi: Fix 32-bit EFI handover protocol entry point

[David Woodhouse]

From: David Woodhouse <David.Woodhouse-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

If the bootloader calls the EFI handover entry point as a standard function
call, then it'll have a return address on the stack. We need to pop that
before calling efi_main(), or the arguments will all be out of position on
the stack.

Signed-off-by: David Woodhouse <David.Woodhouse-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 arch/x86/boot/compressed/head_32.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index aa4aaf1..ccb2f4a 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -50,8 +50,10 @@ ENTRY(startup_32)
 	pushl	%eax
 	pushl	%esi
 	pushl	%ecx
+	sub	$0x4, %esp
 
 	.org 0x30,0x90
+	add	$0x4, %esp
 	call	efi_main
 	cmpl	$0, %eax
 	movl	%eax, %esi
-- 
1.8.0.1

[PATCH 3/6] x86, efi: Fix PCI ROM handing in EFI boot stub, in 32-bit mode

[David Woodhouse]

From: David Woodhouse <David.Woodhouse-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

The 'Attributes' argument to pci->Attributes() function is 64-bit. So
when invoking in 32-bit mode it takes two registers, not just one.

This fixes memory corruption when booting via the 32-bit EFI boot stub.

Signed-off-by: David Woodhouse <David.Woodhouse-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 arch/x86/boot/compressed/eboot.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 03d8c9b..685d0b9 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -295,10 +295,15 @@ static efi_status_t setup_efi_pci(struct boot_params *params)
 		if (!pci)
 			continue;
 
+#ifdef CONFIG_X86_64
 		status = efi_call_phys4(pci->attributes, pci,
 					EfiPciIoAttributeOperationGet, 0,
 					&attributes);
-
+#else
+		status = efi_call_phys5(pci->attributes, pci,
+					EfiPciIoAttributeOperationGet, 0, 0,
+					&attributes);
+#endif
 		if (status != EFI_SUCCESS)
 			continue;
 
-- 
1.8.0.1

[PATCH 4/6] x86, efi: Add loadflags for EFI handover support in 32-bit and 64-bit mode

[David Woodhouse]

From: David Woodhouse <David.Woodhouse-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

As it is, the bootloader has no idea whether the kernel's EFI boot stub
is 32-bit or 64-bit. If it gets it wrong, we'll get bizarre crashes; it
won't even get the *entry* point right because of the weird implicit
added 0x200 for the 64-bit entry points.

This patch adds flags to indicate support for 32-bit vs. 64-bit mode.

In future, it would be possible to support both in the same kernel. We
can't make EFI runtime calls to 32-bit EFI from a 64-bit kernel, or
vice versa, but with the old entry point it used to at least *boot*, and
the EFI handover protocol ought to retain that functionality.

As an added bonus, stop setting handover_offset when CONFIG_EFI_STUB
isn't even set, and asking the bootloader to jump to where we *would*
have placed the entry point.

Signed-off-by: David Woodhouse <David.Woodhouse-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 Documentation/x86/boot.txt | 45 ++++++++++++++++++++++++++++++++++++++-------
 arch/x86/boot/header.S     |  6 ++++--
 2 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt
index 406d82d..0841b21 100644
--- a/Documentation/x86/boot.txt
+++ b/Documentation/x86/boot.txt
@@ -401,6 +401,14 @@ Protocol:	2.00+
 	- If 0, the protected-mode code is loaded at 0x10000.
 	- If 1, the protected-mode code is loaded at 0x100000.
 
+  Bit 1 (read):	EFI_STUB_32
+	- If 0, the EFI handover protocol is not supported in 32-bit mode.
+	- If 1, the EFI handover protocol is supported in 32-bit mode.
+
+  Bit 2 (read):	EFI_STUB_64
+	- If 0, the EFI handover protocol is not supported in 64-bit mode.
+	- If 1, the EFI handover protocol is supported in 64-bit mode.
+
   Bit 5 (write): QUIET_FLAG
 	- If 0, print early messages.
 	- If 1, suppress early messages.
@@ -702,12 +710,15 @@ Field name:	handover_offset
 Type:		read
 Offset/size:	0x264/4
 
-  This field is the offset from the beginning of the kernel image to
-  the EFI handover protocol entry point. Boot loaders using the EFI
-  handover protocol to boot the kernel should jump to this offset.
-
-  See EFI HANDOVER PROTOCOL below for more details.
+  This field indicates the location of the EFI handover protocol entry
+  point. See EFI HANDOVER PROTOCOL below for more details.
 
+  NOTE: For 32-bit support, this field contains the offset from the
+  beginning of the kernel image as one might expect. For 64-bit support,
+  for historical reasons 0x200 must be added to the value of this field.
+  For example, if the value of the handover_offset field is 0x30, the
+  64-bit EFI entry point is actually 0x230 bytes into the kernel code.
+  Do not ask why.
 
 **** THE IMAGE CHECKSUM
 
@@ -1034,8 +1045,23 @@ address of the struct boot_params; %ebp, %edi and %ebx must be zero.
 This protocol allows boot loaders to defer initialisation to the EFI
 boot stub. The boot loader is required to load the kernel/initrd(s)
 from the boot media and jump to the EFI handover protocol entry point
-which is hdr->handover_offset bytes from the beginning of
-startup_{32,64}.
+which is indicated by the hdr->handover_offset field.
+
+The appropriate (32-bit or 64-bit) EFI boot stub must be called according
+to the version of EFI running on the system. A kernel may support either,
+both, or neither entry point. The bootloader should check the EFI_STUB_32
+or EFI_STUB_64 flag in the load_flags field as appropriate. If the entry
+point appropriate for the system's implementation of EFI is not supported
+by the kernel, then the bootloader should invoke the 'legacy' entry point
+as normal.
+
+If invoking the 32-bit entry point, it is found at the offset indicated
+by the handover_offset field of the setup header, and obviously must be
+invoked with the CPU in 32-bit mode.
+
+If invoking the 64-bit entry point, it is found at an offset 0x200 greater
+than the value of the handover_offset field, and the CPU must be in 64-bit
+mode.
 
 The function prototype for the handover entry point looks like this,
 
@@ -1055,3 +1081,8 @@ The boot loader *must* fill out the following fields in bp,
     o hdr.ramdisk_size  (if applicable)
 
 All other fields should be zero.
+
+Note that the efi_main() entry point uses Linux/ELF calling conventions,
+not EFI calling conventions. So for x86_64, 'handle' is in %rdi, 'table'
+in %rsi and 'bp' in %rdx. For i386, the ABI is the same: the arguments
+(and return address) are on the stack.
diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 8c132a6..318264e 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -303,7 +303,8 @@ CAN_USE_HEAP	= 0x80			# If set, the loader also has set
 					# space behind setup.S can be used for
 					# heap purposes.
 					# Only the loader knows what is free
-		.byte	LOADED_HIGH
+EFIFLAGS = IS_ENABLED(CONFIG_EFI_STUB) << (1 + IS_ENABLED(CONFIG_X86_64))
+		.byte	LOADED_HIGH + EFIFLAGS
 
 setup_move_size: .word  0x8000		# size to move, when setup is not
 					# loaded at 0x90000. We will move setup
@@ -397,7 +398,8 @@ pref_address:		.quad LOAD_PHYSICAL_ADDR	# preferred load addr
 #define INIT_SIZE VO_INIT_SIZE
 #endif
 init_size:		.long INIT_SIZE		# kernel initialization size
-handover_offset:	.long 0x30		# offset to the handover
+handover_offset:	.long 0x30 * IS_ENABLED(CONFIG_EFI_STUB)
+						# offset to the handover
 						# protocol entry point
 
 # End of setup header #####################################################
-- 
1.8.0.1

[PATCH 5/6] x86, boot: Lead us not into temptation

[David Woodhouse]

From: David Woodhouse <David.Woodhouse-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Avoid hard-coded addresses in compressed startup code

We have historically hard-coded entry points in head.S just so it's easy
to build the executable/bzImage headers with references to them.

Unfortunately, this leads to boot loaders abusing these "known" addresses
even when they are *explicitly* told that they "should look at the ELF
header to find this address, as it may change in the future". And even
when the address in question *has* actually been changed in the past,
without fanfare or thought to compatibility.

Thus we have bootloaders doing stunningly broken things like jumping to
offset 0x200 in the kernel startup code in 64-bit mode, *hoping* that
startup_64 is still there. And hoping that it's actually a 64-bit kernel
despite the fact that we don't give them any indication of that fact.

Yes, this abomination has been seen in the wild, and yes the offending
bootloaders just jump right into the compressed payload of a 32-bit
bzImage, in 64-bit mode. And no, it doesn't work.

This patch should hopefully remove the temptation to abuse internal
addresses, where sternly worded comments have not sufficed. Instead of
having hard-coded addresses and saying "please don't abuse these", we
actually pull the addresses out of the ELF payload into zoffset.h, and
make build.c shove them back into the right places.

Rather than including zoffset.h into build.c and thus having to rebuild
the tool for every kernel build, we parse it instead. The parsing code
is small and simple.

Signed-off-by: David Woodhouse <David.Woodhouse-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 Documentation/x86/boot.txt         | 14 +++++++
 arch/x86/boot/Makefile             |  4 +-
 arch/x86/boot/compressed/head_32.S |  6 +--
 arch/x86/boot/compressed/head_64.S |  8 ++--
 arch/x86/boot/header.S             |  5 ++-
 arch/x86/boot/tools/build.c        | 81 +++++++++++++++++++++++++++++---------
 6 files changed, 89 insertions(+), 29 deletions(-)

diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt
index 0841b21..62acb5b 100644
--- a/Documentation/x86/boot.txt
+++ b/Documentation/x86/boot.txt
@@ -1003,6 +1003,20 @@ IMPORTANT: All the hooks are required to preserve %esp, %ebp, %esi and
 	(relocated, if appropriate.)
 
 
+**** 64-bit BOOT PROTOCOL
+
+There is no 64-bit boot protocol when using the bzImage format; a
+bootloader cannot even reliably tell a 64-bit from a 32-bit kernel.
+64-bit bootloaders, unless using the 64-bit EFI handover protocol (see
+below), must switch the CPU into 32-bit mode and use the 32-bit boot
+protocolas described below.
+
+Special-purpose bootloaders using an ELF image file (such as is found
+in arch/x86/boot/compressed/vmlinux after a kernel build) may use the
+entry point indicated by the ELF headers, exactly as documented for
+the 32-bit boot protocol below, except in 64-bit mode.
+
+
 **** 32-bit BOOT PROTOCOL
 
 For machine with some new BIOS other than legacy BIOS, such as EFI,
diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index ccce0ed..379814b 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -71,7 +71,7 @@ GCOV_PROFILE := n
 $(obj)/bzImage: asflags-y  := $(SVGA_MODE)
 
 quiet_cmd_image = BUILD   $@
-cmd_image = $(obj)/tools/build $(obj)/setup.bin $(obj)/vmlinux.bin > $@
+cmd_image = $(obj)/tools/build $(obj)/setup.bin $(obj)/vmlinux.bin $(obj)/zoffset.h > $@
 
 $(obj)/bzImage: $(obj)/setup.bin $(obj)/vmlinux.bin $(obj)/tools/build FORCE
 	$(call if_changed,image)
@@ -92,7 +92,7 @@ targets += voffset.h
 $(obj)/voffset.h: vmlinux FORCE
 	$(call if_changed,voffset)
 
-sed-zoffset := -e 's/^\([0-9a-fA-F]*\) . \(startup_32\|input_data\|_end\|z_.*\)$$/\#define ZO_\2 0x\1/p'
+sed-zoffset := -e 's/^\([0-9a-fA-F]*\) . \(startup_32\|startup_64\|efi_pe_entry\|efi_stub_entry\|input_data\|_end\|z_.*\)$$/\#define ZO_\2 0x\1/p'
 
 quiet_cmd_zoffset = ZOFFSET $@
       cmd_zoffset = $(NM) $< | sed -n $(sed-zoffset) > $@
diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index ccb2f4a..1e3184f 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -35,11 +35,11 @@ ENTRY(startup_32)
 #ifdef CONFIG_EFI_STUB
 	jmp	preferred_addr
 
-	.balign	0x10
 	/*
 	 * We don't need the return address, so set up the stack so
-	 * efi_main() can find its arugments.
+	 * efi_main() can find its arguments.
 	 */
+ENTRY(efi_pe_entry)
 	add	$0x4, %esp
 
 	call	make_boot_params
@@ -52,7 +52,7 @@ ENTRY(startup_32)
 	pushl	%ecx
 	sub	$0x4, %esp
 
-	.org 0x30,0x90
+ENTRY(efi_stub_entry)
 	add	$0x4, %esp
 	call	efi_main
 	cmpl	$0, %eax
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 2c4b171..f5d1aaa 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -201,12 +201,12 @@ ENTRY(startup_64)
 	 */
 #ifdef CONFIG_EFI_STUB
 	/*
-	 * The entry point for the PE/COFF executable is 0x210, so only
-	 * legacy boot loaders will execute this jmp.
+	 * The entry point for the PE/COFF executable is efi_pe_entry, so
+	 * only legacy boot loaders will execute this jmp.
 	 */
 	jmp	preferred_addr
 
-	.org 0x210
+ENTRY(efi_pe_entry)
 	mov	%rcx, %rdi
 	mov	%rdx, %rsi
 	pushq	%rdi
@@ -218,7 +218,7 @@ ENTRY(startup_64)
 	popq	%rsi
 	popq	%rdi
 
-	.org 0x230,0x90
+ENTRY(efi_stub_entry)
 	call	efi_main
 	movq	%rax,%rsi
 	cmpq	$0,%rax
diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 318264e..ad3b7f8 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -398,9 +398,10 @@ pref_address:		.quad LOAD_PHYSICAL_ADDR	# preferred load addr
 #define INIT_SIZE VO_INIT_SIZE
 #endif
 init_size:		.long INIT_SIZE		# kernel initialization size
-handover_offset:	.long 0x30 * IS_ENABLED(CONFIG_EFI_STUB)
-						# offset to the handover
+
+handover_offset:	.long 0			# offset to the EFI handover
 						# protocol entry point
+						# (filled in by build.c)
 
 # End of setup header #####################################################
 
diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
index 4b8e165..94c5446 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -52,6 +52,10 @@ int is_big_kernel;
 
 #define PECOFF_RELOC_RESERVE 0x20
 
+unsigned long efi_stub_entry;
+unsigned long efi_pe_entry;
+unsigned long startup_64;
+
 /*----------------------------------------------------------------------*/
 
 static const u32 crctab32[] = {
@@ -132,7 +136,7 @@ static void die(const char * str, ...)
 
 static void usage(void)
 {
-	die("Usage: build setup system [> image]");
+	die("Usage: build setup system [zoffset.h] [> image]");
 }
 
 #ifdef CONFIG_EFI_STUB
@@ -206,30 +210,54 @@ static void update_pecoff_text(unsigned int text_start, unsigned int file_sz)
 	 */
 	put_unaligned_le32(file_sz - 512, &buf[pe_header + 0x1c]);
 
-#ifdef CONFIG_X86_32
 	/*
-	 * Address of entry point.
-	 *
-	 * The EFI stub entry point is +16 bytes from the start of
-	 * the .text section.
+	 * Address of entry point for PE/COFF executable
 	 */
-	put_unaligned_le32(text_start + 16, &buf[pe_header + 0x28]);
-#else
-	/*
-	 * Address of entry point. startup_32 is at the beginning and
-	 * the 64-bit entry point (startup_64) is always 512 bytes
-	 * after. The EFI stub entry point is 16 bytes after that, as
-	 * the first instruction allows legacy loaders to jump over
-	 * the EFI stub initialisation
-	 */
-	put_unaligned_le32(text_start + 528, &buf[pe_header + 0x28]);
-#endif /* CONFIG_X86_32 */
+	put_unaligned_le32(text_start + efi_pe_entry, &buf[pe_header + 0x28]);
 
 	update_pecoff_section_header(".text", text_start, text_sz);
 }
 
 #endif /* CONFIG_EFI_STUB */
 
+
+/*
+ * Parse zoffset.h and find the entry points. We could just #include zoffset.h
+ * but that would mean tools/build would have to be rebuilt every time. It's
+ * not as if parsing it is hard...
+ */
+#define PARSE_ZOFS(p, sym) do { \
+	if (!strncmp(p, "#define ZO_" #sym " ", 11+sizeof(#sym)))	\
+		sym = strtoul(p + 11 + sizeof(#sym), NULL, 16);		\
+} while (0)
+
+static void parse_zoffset(char *fname)
+{
+	FILE *file;
+	char *p;
+	int c;
+
+	file = fopen(fname, "r");
+	if (!file)
+		die("Unable to open `%s': %m", fname);
+	c = fread(buf, 1, sizeof(buf) - 1, file);
+	if (ferror(file))
+		die("read-error on `zoffset.h'");
+	buf[c] = 0;
+
+	p = (char *)buf;
+
+	while (p && *p) {
+		PARSE_ZOFS(p, efi_stub_entry);
+		PARSE_ZOFS(p, efi_pe_entry);
+		PARSE_ZOFS(p, startup_64);
+
+		p = strchr(p, '\n');
+		while (p && (*p == '\r' || *p == '\n'))
+			p++;
+	}
+}
+
 int main(int argc, char ** argv)
 {
 	unsigned int i, sz, setup_sectors;
@@ -241,7 +269,19 @@ int main(int argc, char ** argv)
 	void *kernel;
 	u32 crc = 0xffffffffUL;
 
-	if (argc != 3)
+	/* Defaults for old kernel */
+#ifdef CONFIG_X86_32
+	efi_pe_entry = 0x10;
+	efi_stub_entry = 0x30;
+#else
+	efi_pe_entry = 0x210;
+	efi_stub_entry = 0x230;
+	startup_64 = 0x200;
+#endif
+
+	if (argc == 4)
+		parse_zoffset(argv[3]);
+	else if (argc != 3)
 		usage();
 
 	/* Copy the setup code */
@@ -299,6 +339,11 @@ int main(int argc, char ** argv)
 
 #ifdef CONFIG_EFI_STUB
 	update_pecoff_text(setup_sectors * 512, sz + i + ((sys_size * 16) - sz));
+
+#ifdef CONFIG_X86_64 /* Yes, this is really how we defined it :( */
+	efi_stub_entry -= 0x200;
+#endif
+	put_unaligned_le32(efi_stub_entry, &buf[0x264]);
 #endif
 
 	crc = partial_crc32(buf, i, crc);
-- 
1.8.0.1

[PATCH 6/6] x86, boot: Deliver us from evil

[David Woodhouse]

From: David Woodhouse <David.Woodhouse-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Check for broken bootloaders jumping into the kernel image at 0x200, which
was never a defined entry point. It was *always* broken for a 32-bit kernel,
and there's no sane way to tell a 32-bit bzImage from a 64-bit one. (Anyone
who suggests looking at the kernel_alignment field, go and stand in the
naughty corner. Besides, the offending bootloaders *didn't* do this.)

Signed-off-by: David Woodhouse <David.Woodhouse-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 arch/x86/boot/compressed/head_64.S | 49 +++++++++++++++++++++++---------------
 arch/x86/kernel/setup.c            |  6 +++++
 2 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index f5d1aaa..a7d8b7e 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -185,27 +185,28 @@ no_longmode:
 #include "../../kernel/verify_cpu.S"
 
 	/*
-	 * Be careful here startup_64 needs to be at a predictable
-	 * address so I can export it in an ELF header.  Bootloaders
-	 * should look at the ELF header to find this address, as
-	 * it may change in the future.
+	 * startup_64 was once here, and some very naughty bootloaders decided
+	 * to jump directly to it despite the fact that it was clearly marked
+	 * 'this may change' and in fact had already moved once without
+	 * fanfare. And despite the fact that they were not even checking for
+	 * 32-bit vs. 64-bit kernels, so they were just jumping right into
+	 * the compressed payload when asked to boot the former!
+	 *
+	 * These bootloaders are so *egregiously* broken, and for 32-bit
+	 * kernels they broken in the "it just crashes instead of booting"
+	 * sense of the word rather than just the "you got away with it, but
+	 * someone needs to be whipped for this" sense of the word. So we don't
+	 * feel the need to pander to them and retroactively "define" the
+	 * entry point to be at 0x200. But for a *little* while let's catch it
+	 * and give a nasty warning message during boot that their bootloader
+	 * needs to be updated.
 	 */
 	.code64
 	.org 0x200
-ENTRY(startup_64)
-	/*
-	 * We come here either from startup_32 or directly from a
-	 * 64bit bootloader.  If we come here from a bootloader we depend on
-	 * an identity mapped page table being provied that maps our
-	 * entire text+data+bss and hopefully all of memory.
-	 */
-#ifdef CONFIG_EFI_STUB
-	/*
-	 * The entry point for the PE/COFF executable is efi_pe_entry, so
-	 * only legacy boot loaders will execute this jmp.
-	 */
+	movl	$0xbad10ad, %edx
 	jmp	preferred_addr
 
+#ifdef CONFIG_EFI_STUB
 ENTRY(efi_pe_entry)
 	mov	%rcx, %rdi
 	mov	%rdx, %rsi
@@ -234,12 +235,19 @@ ENTRY(efi_stub_entry)
 	subq	$3b, %rax
 	subq	BP_pref_address(%rsi), %rax
 	add	BP_code32_start(%esi), %eax
-	leaq	preferred_addr(%rax), %rax
+	leaq	startup_64(%rax), %rax
 	jmp	*%rax
-
-preferred_addr:
 #endif
+	/*
+	 * We come here either from startup_32 or directly from a
+	 * 64bit bootloader.  If we come here from a bootloader we depend on
+	 * an identity mapped page table being provied that maps our
+	 * entire text+data+bss and hopefully all of memory.
+	 */
+ENTRY(startup_64)
+	movl	$0, %edx
 
+preferred_addr:
 	/* Setup data segments. */
 	xorl	%eax, %eax
 	movl	%eax, %ds
@@ -251,6 +259,9 @@ preferred_addr:
 	movl    $0x20, %eax
 	ltr	%ax
 
+	/* Store 'bad load' flag in BP */
+	movl	%edx, BP_scratch(%rsi)
+
 	/*
 	 * Compute the decompressed kernel start address.  It is where
 	 * we were loaded at aligned to a 2M boundary. %rbp contains the
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 23ddd55..02ce7d9 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1061,6 +1061,12 @@ void __init setup_arch(char **cmdline_p)
 		efi_enabled = 0;
 	}
 #endif
+#ifdef CONFIG_X86_64
+	if (boot_params.scratch == 0xbad10ad) {
+		WARN(1, "Broken bootloader jumped to wrong entry point in the kernel image. "
+		     "Please update your bootloader or new kernels may not boot.");
+	}
+#endif
 }
 
 #ifdef CONFIG_X86_32
-- 
1.8.0.1

Re: [PATCH 2/6] x86, efi: Fix 32-bit EFI handover protocol entry point

[Matt Fleming]

On Thu, 2013-01-10 at 18:04 +0000, David Woodhouse wrote:
> From: David Woodhouse <David.Woodhouse-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> If the bootloader calls the EFI handover entry point as a standard function
> call, then it'll have a return address on the stack. We need to pop that
> before calling efi_main(), or the arguments will all be out of position on
> the stack.
> 
> Signed-off-by: David Woodhouse <David.Woodhouse-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  arch/x86/boot/compressed/head_32.S | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
> index aa4aaf1..ccb2f4a 100644
> --- a/arch/x86/boot/compressed/head_32.S
> +++ b/arch/x86/boot/compressed/head_32.S
> @@ -50,8 +50,10 @@ ENTRY(startup_32)
>  	pushl	%eax
>  	pushl	%esi
>  	pushl	%ecx
> +	sub	$0x4, %esp
>  
>  	.org 0x30,0x90
> +	add	$0x4, %esp
>  	call	efi_main
>  	cmpl	$0, %eax
>  	movl	%eax, %esi

It's worth pointing out that efilinux doesn't invoke the handover
protocol as a function, it pushes arguments onto the stack and jumps. So
this change will actually break that boot loader.

Is there a particular reason why you need to invoke it as a standard
function on 32-bits?

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 2/6] x86, efi: Fix 32-bit EFI handover protocol entry point

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1839 bytes --]

On Wed, 2013-01-23 at 16:26 +0000, Matt Fleming wrote:
> It's worth pointing out that efilinux doesn't invoke the handover
> protocol as a function, it pushes arguments onto the stack and jumps.
> So this change will actually break that boot loader.
> 
> Is there a particular reason why you need to invoke it as a standard
> function on 32-bits?

Oh, for fuck's sake. It's clearly documented as a *FUNCTION* in
Documentation/x86/boot.txt:

>>>>>>>>>>>>>>.
The function prototype for the handover entry point looks like this,

    efi_main(void *handle, efi_system_table_t *table, struct boot_params *bp)
<<<<<<<<<<<<<<

This whole thing is a complete clusterfuck. To recap just *some* of the
more egregious parts of the breakage:

- We put the handover_offset field into kernels with !CONFIG_EFI_STUB.
- We have a magical 0x200 delta for handover_offset on a 64-bit kernel.
- We don't *indicate* which kernels are 64-bit and which are 32-bit, 
  which means that not only does the bootloader not know whether to
  add that brain-dead magical 0x200 offset, it *also* doesn't know which
  mode the CPU needs to be in (and which wordsize of EFI needs to be
  supported by the system).

Quite frankly, anything trying to invoke the EFI boot stub right now is
broken. Actually broken in the "it doesn't boot" sense, for at least 50%
of kernels. Anybody looking at incorporating bootloader support for it
should have done what I did, and throw their hands up in horror and
*fix* it before they could consider using it.

How about we write off the existing handover_offset field and consider
it a failed experiment in psychotic drugs, then introduce a *new* one
with sane and documented semantics? Hell, it only needs to be one byte
(handover_offset >> 4) rather than a whole four bytes....

-- 
dwmw2


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]