LKML Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/3] x86, efi: EFI boot stub documentation
@ 2012-03-16 14:18 Matt Fleming
  2012-03-16 14:18 ` [PATCH 1/3] x86, efi: Only close open files in error path Matt Fleming
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Matt Fleming @ 2012-03-16 14:18 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Matthew Garret, Matt Fleming, linux-kernel, x86

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

Some people trying out the EFI stub have been confused about the
syntax for the initrd= option, so this series adds some information to
Documentation/x86. It also adds some helpful error messages, as
previously if the user specified an incorrect initrd filename the
machine just appeared to hang. Now at least it will print a message
saying that it couldn't open the file.

The first patch is actually a bug fix for an issue where the wrong
loop counter was used when trying to close initrd files in the error
path, which caused the machine to hang because a garbage pointer was
dereferenced.

Ideally I'd like to squeeze at least the last two patches in before
v3.3 is released.

Matt Fleming (3):
  x86, efi: Only close open files in error path
  x86, efi; Add EFI boot stub console support
  x86, efi: Add EFI boot stub documentation

 Documentation/x86/efi-stub.txt   |   65 ++++++++++++++++++++++++++++
 arch/x86/Kconfig                 |    2 +
 arch/x86/boot/compressed/eboot.c |   88 ++++++++++++++++++++++++++++++-------
 arch/x86/boot/compressed/eboot.h |    6 +++
 4 files changed, 144 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/x86/efi-stub.txt

-- 
1.7.4.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] x86, efi: Only close open files in error path
  2012-03-16 14:18 [PATCH 0/3] x86, efi: EFI boot stub documentation Matt Fleming
@ 2012-03-16 14:18 ` Matt Fleming
  2012-06-01 19:11   ` [tip:x86/efi] " tip-bot for Matt Fleming
  2012-03-16 14:18 ` [PATCH 2/3] x86, efi; Add EFI boot stub console support Matt Fleming
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Matt Fleming @ 2012-03-16 14:18 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Matthew Garret, Matt Fleming, linux-kernel, x86

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

The loop at the 'close_handles' label in handle_ramdisks() should be
using 'i', which represents the number of initrd files that were
successfully opened, not 'nr_initrds' which is the number of initrd=
arguments passed on the command line.

Currently, if we execute the loop to close all file handles and we
failed to open any initrds we'll try to call the close function on a
garbage pointer, causing the machine to hang.

Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Matthew Garrett <mjg@redhat.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/x86/boot/compressed/eboot.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index fec216f..fb810ce 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -674,7 +674,7 @@ free_initrd_total:
 	low_free(initrd_total, initrd_addr);
 
 close_handles:
-	for (k = j; k < nr_initrds; k++)
+	for (k = j; k < i; k++)
 		efi_call_phys1(fh->close, initrds[k].handle);
 free_initrds:
 	efi_call_phys1(sys_table->boottime->free_pool, initrds);
-- 
1.7.4.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/3] x86, efi; Add EFI boot stub console support
  2012-03-16 14:18 [PATCH 0/3] x86, efi: EFI boot stub documentation Matt Fleming
  2012-03-16 14:18 ` [PATCH 1/3] x86, efi: Only close open files in error path Matt Fleming
@ 2012-03-16 14:18 ` Matt Fleming
  2012-06-01 19:11   ` [tip:x86/efi] " tip-bot for Matt Fleming
  2012-03-16 14:18 ` [PATCH 3/3] x86, efi: Add EFI boot stub documentation Matt Fleming
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Matt Fleming @ 2012-03-16 14:18 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Matthew Garret, Matt Fleming, linux-kernel, x86

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

We need a way of printing useful messages to the user, for example
when we fail to open an initrd file, instead of just hanging the
machine without giving the user any indication of what went wrong. So
sprinkle some error messages throughout the EFI boot stub code to make
it easier for users to diagnose/report problems.

Reported-by: Keshav P R <the.ridikulus.rat@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Matthew Garrett <mjg@redhat.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/x86/boot/compressed/eboot.c |   86 +++++++++++++++++++++++++++++++-------
 arch/x86/boot/compressed/eboot.h |    6 +++
 2 files changed, 76 insertions(+), 16 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index fb810ce..90f6fbc 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -16,6 +16,27 @@
 
 static efi_system_table_t *sys_table;
 
+static void efi_printk(char *str)
+{
+	efi_char16_t *s16;
+	char *s8;
+
+	for (s8 = str; *s8; s8++) {
+		struct efi_simple_text_output_protocol *out;
+		efi_char16_t ch[2] = { 0 };
+
+		ch[0] = *s8;
+		out = (struct efi_simple_text_output_protocol *)sys_table->con_out;
+
+		if (*s8 == '\n') {
+			efi_char16_t nl[2] = { '\r', 0 };
+			efi_call_phys2(out->output_string, out, nl);
+		}
+
+		efi_call_phys2(out->output_string, out, ch);
+	}
+}
+
 static efi_status_t __get_map(efi_memory_desc_t **map, unsigned long *map_size,
 			      unsigned long *desc_size)
 {
@@ -531,8 +552,10 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
 				EFI_LOADER_DATA,
 				nr_initrds * sizeof(*initrds),
 				&initrds);
-	if (status != EFI_SUCCESS)
+	if (status != EFI_SUCCESS) {
+		efi_printk("Failed to alloc mem for initrds\n");
 		goto fail;
+	}
 
 	str = (char *)(unsigned long)hdr->cmd_line_ptr;
 	for (i = 0; i < nr_initrds; i++) {
@@ -575,32 +598,42 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
 
 			status = efi_call_phys3(boottime->handle_protocol,
 					image->device_handle, &fs_proto, &io);
-			if (status != EFI_SUCCESS)
+			if (status != EFI_SUCCESS) {
+				efi_printk("Failed to handle fs_proto\n");
 				goto free_initrds;
+			}
 
 			status = efi_call_phys2(io->open_volume, io, &fh);
-			if (status != EFI_SUCCESS)
+			if (status != EFI_SUCCESS) {
+				efi_printk("Failed to open volume\n");
 				goto free_initrds;
+			}
 		}
 
 		status = efi_call_phys5(fh->open, fh, &h, filename,
 					EFI_FILE_MODE_READ, (u64)0);
-		if (status != EFI_SUCCESS)
+		if (status != EFI_SUCCESS) {
+			efi_printk("Failed to open initrd file\n");
 			goto close_handles;
+		}
 
 		initrd->handle = h;
 
 		info_sz = 0;
 		status = efi_call_phys4(h->get_info, h, &info_guid,
 					&info_sz, NULL);
-		if (status != EFI_BUFFER_TOO_SMALL)
+		if (status != EFI_BUFFER_TOO_SMALL) {
+			efi_printk("Failed to get initrd info size\n");
 			goto close_handles;
+		}
 
 grow:
 		status = efi_call_phys3(sys_table->boottime->allocate_pool,
 					EFI_LOADER_DATA, info_sz, &info);
-		if (status != EFI_SUCCESS)
+		if (status != EFI_SUCCESS) {
+			efi_printk("Failed to alloc mem for initrd info\n");
 			goto close_handles;
+		}
 
 		status = efi_call_phys4(h->get_info, h, &info_guid,
 					&info_sz, info);
@@ -612,8 +645,10 @@ grow:
 		file_sz = info->file_size;
 		efi_call_phys1(sys_table->boottime->free_pool, info);
 
-		if (status != EFI_SUCCESS)
+		if (status != EFI_SUCCESS) {
+			efi_printk("Failed to get initrd info\n");
 			goto close_handles;
+		}
 
 		initrd->size = file_sz;
 		initrd_total += file_sz;
@@ -629,11 +664,14 @@ grow:
 		 */
 		status = high_alloc(initrd_total, 0x1000,
 				   &initrd_addr, hdr->initrd_addr_max);
-		if (status != EFI_SUCCESS)
+		if (status != EFI_SUCCESS) {
+			efi_printk("Failed to alloc highmem for initrds\n");
 			goto close_handles;
+		}
 
 		/* We've run out of free low memory. */
 		if (initrd_addr > hdr->initrd_addr_max) {
+			efi_printk("We've run out of free low memory\n");
 			status = EFI_INVALID_PARAMETER;
 			goto free_initrd_total;
 		}
@@ -652,8 +690,10 @@ grow:
 				status = efi_call_phys3(fh->read,
 							initrds[j].handle,
 							&chunksize, addr);
-				if (status != EFI_SUCCESS)
+				if (status != EFI_SUCCESS) {
+					efi_printk("Failed to read initrd\n");
 					goto free_initrd_total;
+				}
 				addr += chunksize;
 				size -= chunksize;
 			}
@@ -732,8 +772,10 @@ static efi_status_t make_boot_params(struct boot_params *boot_params,
 			options_size++;	/* NUL termination */
 
 			status = low_alloc(options_size, 1, &cmdline);
-			if (status != EFI_SUCCESS)
+			if (status != EFI_SUCCESS) {
+				efi_printk("Failed to alloc mem for cmdline\n");
 				goto fail;
+			}
 
 			s1 = (u8 *)(unsigned long)cmdline;
 			s2 = (u16 *)options;
@@ -895,12 +937,16 @@ struct boot_params *efi_main(void *handle, efi_system_table_t *_table)
 
 	status = efi_call_phys3(sys_table->boottime->handle_protocol,
 				handle, &proto, (void *)&image);
-	if (status != EFI_SUCCESS)
+	if (status != EFI_SUCCESS) {
+		efi_printk("Failed to get handle for LOADED_IMAGE_PROTOCOL\n");
 		goto fail;
+	}
 
 	status = low_alloc(0x4000, 1, (unsigned long *)&boot_params);
-	if (status != EFI_SUCCESS)
+	if (status != EFI_SUCCESS) {
+		efi_printk("Failed to alloc lowmem for boot params\n");
 		goto fail;
+	}
 
 	memset(boot_params, 0x0, 0x4000);
 
@@ -925,8 +971,10 @@ struct boot_params *efi_main(void *handle, efi_system_table_t *_table)
 	if (status != EFI_SUCCESS) {
 		status = low_alloc(hdr->init_size, hdr->kernel_alignment,
 				   &start);
-		if (status != EFI_SUCCESS)
+		if (status != EFI_SUCCESS) {
+			efi_printk("Failed to alloc mem for kernel\n");
 			goto fail;
+		}
 	}
 
 	hdr->code32_start = (__u32)start;
@@ -937,19 +985,25 @@ struct boot_params *efi_main(void *handle, efi_system_table_t *_table)
 	status = efi_call_phys3(sys_table->boottime->allocate_pool,
 				EFI_LOADER_DATA, sizeof(*gdt),
 				(void **)&gdt);
-	if (status != EFI_SUCCESS)
+	if (status != EFI_SUCCESS) {
+		efi_printk("Failed to alloc mem for gdt structure\n");
 		goto fail;
+	}
 
 	gdt->size = 0x800;
 	status = low_alloc(gdt->size, 8, (unsigned long *)&gdt->address);
-	if (status != EFI_SUCCESS)
+	if (status != EFI_SUCCESS) {
+		efi_printk("Failed to alloc mem for gdt\n");
 		goto fail;
+	}
 
 	status = efi_call_phys3(sys_table->boottime->allocate_pool,
 				EFI_LOADER_DATA, sizeof(*idt),
 				(void **)&idt);
-	if (status != EFI_SUCCESS)
+	if (status != EFI_SUCCESS) {
+		efi_printk("Failed to alloc mem for idt structure\n");
 		goto fail;
+	}
 
 	idt->size = 0;
 	idt->address = 0;
diff --git a/arch/x86/boot/compressed/eboot.h b/arch/x86/boot/compressed/eboot.h
index 3925166..3c9a2a4 100644
--- a/arch/x86/boot/compressed/eboot.h
+++ b/arch/x86/boot/compressed/eboot.h
@@ -58,4 +58,10 @@ struct efi_uga_draw_protocol {
 	void *blt;
 };
 
+struct efi_simple_text_output_protocol {
+	unsigned long reset;
+	unsigned long output_string;
+	unsigned long test_string;
+};
+
 #endif /* BOOT_COMPRESSED_EBOOT_H */
-- 
1.7.4.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/3] x86, efi: Add EFI boot stub documentation
  2012-03-16 14:18 [PATCH 0/3] x86, efi: EFI boot stub documentation Matt Fleming
  2012-03-16 14:18 ` [PATCH 1/3] x86, efi: Only close open files in error path Matt Fleming
  2012-03-16 14:18 ` [PATCH 2/3] x86, efi; Add EFI boot stub console support Matt Fleming
@ 2012-03-16 14:18 ` Matt Fleming
  2012-06-01 19:13   ` [tip:x86/efi] " tip-bot for Matt Fleming
  2012-03-20 23:57 ` [PATCH 0/3] x86, efi: " Shea Levy
  2012-05-04 10:27 ` Matt Fleming
  4 siblings, 1 reply; 10+ messages in thread
From: Matt Fleming @ 2012-03-16 14:18 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Matthew Garret, Matt Fleming, linux-kernel, x86, Randy Dunlap

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

Since we can't expect every user to read the EFI boot stub code it
seems prudent to have a couple of paragraphs explaining what it is and
how it works.

The "initrd=" option in particular is tricky because it only
understands absolute EFI-style paths (backslashes as directory
separators), and until now this hasn't been documented anywhere. This
has tripped up a couple of users.

Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Matthew Garrett <mjg@redhat.com>
Cc: Randy Dunlap <rdunlap@xenotime.net>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 Documentation/x86/efi-stub.txt |   65 ++++++++++++++++++++++++++++++++++++++++
 arch/x86/Kconfig               |    2 +
 2 files changed, 67 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/x86/efi-stub.txt

diff --git a/Documentation/x86/efi-stub.txt b/Documentation/x86/efi-stub.txt
new file mode 100644
index 0000000..44e6bb6
--- /dev/null
+++ b/Documentation/x86/efi-stub.txt
@@ -0,0 +1,65 @@
+			  The EFI Boot Stub
+		     ---------------------------
+
+On the x86 platform, a bzImage can masquerade as a PE/COFF image,
+thereby convincing EFI firmware loaders to load it as an EFI
+executable. The code that modifies the bzImage header, along with the
+EFI-specific entry point that the firmware loader jumps to are
+collectively known as the "EFI boot stub", and live in
+arch/x86/boot/header.S and arch/x86/boot/compressed/eboot.c,
+respectively.
+
+By using the EFI boot stub it's possible to boot a Linux kernel
+without the use of a conventional EFI boot loader, such as grub or
+elilo. Since the EFI boot stub performs the jobs of a boot loader, in
+a certain sense it *IS* the boot loader.
+
+The EFI boot stub is enabled with the CONFIG_EFI_STUB kernel option.
+
+
+**** How to install bzImage.efi
+
+The bzImage located in arch/x86/boot/bzImage must be copied to the EFI
+System Partiion (ESP) and renamed with the extension ".efi". Without
+the extension the EFI firmware loader will refuse to execute it. It's
+not possible to execute bzImage.efi from the usual Linux file systems
+because EFI firmware doesn't have support for them.
+
+
+**** Passing kernel parameters from the EFI shell
+
+Arguments to the kernel can be passed after bzImage.efi, e.g.
+
+	fs0:> bzImage.efi console=ttyS0 root=/dev/sda4
+
+
+**** The "initrd=" option
+
+Like most boot loaders, the EFI stub allows the user to specify
+multiple initrd files using the "initrd=" option. This is the only EFI
+stub-specific command line parameter, everything else is passed to the
+kernel when it boots.
+
+The path to the initrd file must be an absolute path from the
+beginning of the ESP, relative path names do not work. Also, the path
+is an EFI-style path and directory elements must be separated with
+backslashes (\). For example, given the following directory layout,
+
+fs0:>
+	Kernels\
+			bzImage.efi
+			initrd-large.img
+
+	Ramdisks\
+			initrd-small.img
+			initrd-medium.img
+
+to boot with the initrd-large.img file if the current working
+directory is fs0:\Kernels, the following command must be used,
+
+	fs0:\Kernels> bzImage.efi initrd=\Kernels\initrd-large.img
+
+Notice how bzImage.efi can be specified with a relative path. That's
+because the image we're executing is interpreted by the EFI shell,
+which understands relative paths, whereas the rest of the command line
+is passed to bzImage.efi.
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 5bed94e..591c1a5 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1521,6 +1521,8 @@ config EFI_STUB
           This kernel feature allows a bzImage to be loaded directly
 	  by EFI firmware without the use of a bootloader.
 
+	  See Documentation/x86/efi-stub.txt for more information.
+
 config SECCOMP
 	def_bool y
 	prompt "Enable seccomp to safely compute untrusted bytecode"
-- 
1.7.4.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] x86, efi: EFI boot stub documentation
  2012-03-16 14:18 [PATCH 0/3] x86, efi: EFI boot stub documentation Matt Fleming
                   ` (2 preceding siblings ...)
  2012-03-16 14:18 ` [PATCH 3/3] x86, efi: Add EFI boot stub documentation Matt Fleming
@ 2012-03-20 23:57 ` Shea Levy
  2012-03-21  0:00   ` H. Peter Anvin
  2012-05-04 10:27 ` Matt Fleming
  4 siblings, 1 reply; 10+ messages in thread
From: Shea Levy @ 2012-03-20 23:57 UTC (permalink / raw)
  To: Matt Fleming
  Cc: H. Peter Anvin, Matthew Garret, Matt Fleming, linux-kernel, x86

On 03/16/2012 10:18 AM, Matt Fleming wrote:
> From: Matt Fleming<matt.fleming@intel.com>
>
> Some people trying out the EFI stub have been confused about the
> syntax for the initrd= option, so this series adds some information to
> Documentation/x86. It also adds some helpful error messages, as
> previously if the user specified an incorrect initrd filename the
> machine just appeared to hang. Now at least it will print a message
> saying that it couldn't open the file.
>
> The first patch is actually a bug fix for an issue where the wrong
> loop counter was used when trying to close initrd files in the error
> path, which caused the machine to hang because a garbage pointer was
> dereferenced.
>
> Ideally I'd like to squeeze at least the last two patches in before
> v3.3 is released.
>
> Matt Fleming (3):
>    x86, efi: Only close open files in error path
>    x86, efi; Add EFI boot stub console support
>    x86, efi: Add EFI boot stub documentation
>
>   Documentation/x86/efi-stub.txt   |   65 ++++++++++++++++++++++++++++
>   arch/x86/Kconfig                 |    2 +
>   arch/x86/boot/compressed/eboot.c |   88 ++++++++++++++++++++++++++++++-------
>   arch/x86/boot/compressed/eboot.h |    6 +++
>   4 files changed, 144 insertions(+), 17 deletions(-)
>   create mode 100644 Documentation/x86/efi-stub.txt
>

Anyone know if we can expect these to be in mainline by the end of the 
merge window?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] x86, efi: EFI boot stub documentation
  2012-03-20 23:57 ` [PATCH 0/3] x86, efi: " Shea Levy
@ 2012-03-21  0:00   ` H. Peter Anvin
  0 siblings, 0 replies; 10+ messages in thread
From: H. Peter Anvin @ 2012-03-21  0:00 UTC (permalink / raw)
  To: Shea Levy; +Cc: Matt Fleming, Matthew Garret, Matt Fleming, linux-kernel, x86

On 03/20/2012 04:57 PM, Shea Levy wrote:
> Anyone know if we can expect these to be in mainline by the end of the
> merge window?

I think we can justify one as a fix and the rest as documentation.

	-hpa

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] x86, efi: EFI boot stub documentation
  2012-03-16 14:18 [PATCH 0/3] x86, efi: EFI boot stub documentation Matt Fleming
                   ` (3 preceding siblings ...)
  2012-03-20 23:57 ` [PATCH 0/3] x86, efi: " Shea Levy
@ 2012-05-04 10:27 ` Matt Fleming
  4 siblings, 0 replies; 10+ messages in thread
From: Matt Fleming @ 2012-05-04 10:27 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Matthew Garret, linux-kernel, x86

Ping? I can't find these patches in tip.

On Fri, 2012-03-16 at 14:18 +0000, Matt Fleming wrote:
> From: Matt Fleming <matt.fleming@intel.com>
> 
> Some people trying out the EFI stub have been confused about the
> syntax for the initrd= option, so this series adds some information to
> Documentation/x86. It also adds some helpful error messages, as
> previously if the user specified an incorrect initrd filename the
> machine just appeared to hang. Now at least it will print a message
> saying that it couldn't open the file.
> 
> The first patch is actually a bug fix for an issue where the wrong
> loop counter was used when trying to close initrd files in the error
> path, which caused the machine to hang because a garbage pointer was
> dereferenced.
> 
> Ideally I'd like to squeeze at least the last two patches in before
> v3.3 is released.
> 
> Matt Fleming (3):
>   x86, efi: Only close open files in error path
>   x86, efi; Add EFI boot stub console support
>   x86, efi: Add EFI boot stub documentation
> 
>  Documentation/x86/efi-stub.txt   |   65 ++++++++++++++++++++++++++++
>  arch/x86/Kconfig                 |    2 +
>  arch/x86/boot/compressed/eboot.c |   88 ++++++++++++++++++++++++++++++-------
>  arch/x86/boot/compressed/eboot.h |    6 +++
>  4 files changed, 144 insertions(+), 17 deletions(-)
>  create mode 100644 Documentation/x86/efi-stub.txt
> 




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [tip:x86/efi] x86, efi: Only close open files in error path
  2012-03-16 14:18 ` [PATCH 1/3] x86, efi: Only close open files in error path Matt Fleming
@ 2012-06-01 19:11   ` tip-bot for Matt Fleming
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Matt Fleming @ 2012-06-01 19:11 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, mjg, hpa, mingo, tglx, matt.fleming

Commit-ID:  30dc0d0fe5d08396dbdaa2d70972149131340960
Gitweb:     http://git.kernel.org/tip/30dc0d0fe5d08396dbdaa2d70972149131340960
Author:     Matt Fleming <matt.fleming@intel.com>
AuthorDate: Thu, 15 Mar 2012 19:13:25 +0000
Committer:  H. Peter Anvin <hpa@zytor.com>
CommitDate: Fri, 1 Jun 2012 09:11:10 -0700

x86, efi: Only close open files in error path

The loop at the 'close_handles' label in handle_ramdisks() should be
using 'i', which represents the number of initrd files that were
successfully opened, not 'nr_initrds' which is the number of initrd=
arguments passed on the command line.

Currently, if we execute the loop to close all file handles and we
failed to open any initrds we'll try to call the close function on a
garbage pointer, causing the machine to hang.

Cc: Matthew Garrett <mjg@redhat.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
Link: http://lkml.kernel.org/r/1331907517-3985-2-git-send-email-matt@console-pimps.org
Signed-off-by: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/boot/compressed/eboot.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 2c14e76..52a4e66 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -674,7 +674,7 @@ free_initrd_total:
 	low_free(initrd_total, initrd_addr);
 
 close_handles:
-	for (k = j; k < nr_initrds; k++)
+	for (k = j; k < i; k++)
 		efi_call_phys1(fh->close, initrds[k].handle);
 free_initrds:
 	efi_call_phys1(sys_table->boottime->free_pool, initrds);

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [tip:x86/efi] x86, efi; Add EFI boot stub console support
  2012-03-16 14:18 ` [PATCH 2/3] x86, efi; Add EFI boot stub console support Matt Fleming
@ 2012-06-01 19:11   ` tip-bot for Matt Fleming
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Matt Fleming @ 2012-06-01 19:11 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, mjg, hpa, mingo, the.ridikulus.rat, tglx, matt.fleming

Commit-ID:  9fa7dedad3d30345c843bd82db02c4d6169e5f61
Gitweb:     http://git.kernel.org/tip/9fa7dedad3d30345c843bd82db02c4d6169e5f61
Author:     Matt Fleming <matt.fleming@intel.com>
AuthorDate: Mon, 20 Feb 2012 13:20:59 +0000
Committer:  H. Peter Anvin <hpa@zytor.com>
CommitDate: Fri, 1 Jun 2012 09:11:26 -0700

x86, efi; Add EFI boot stub console support

We need a way of printing useful messages to the user, for example
when we fail to open an initrd file, instead of just hanging the
machine without giving the user any indication of what went wrong. So
sprinkle some error messages throughout the EFI boot stub code to make
it easier for users to diagnose/report problems.

Reported-by: Keshav P R <the.ridikulus.rat@gmail.com>
Cc: Matthew Garrett <mjg@redhat.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
Link: http://lkml.kernel.org/r/1331907517-3985-3-git-send-email-matt@console-pimps.org
Signed-off-by: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/boot/compressed/eboot.c |   85 ++++++++++++++++++++++++++++++-------
 arch/x86/boot/compressed/eboot.h |    6 +++
 2 files changed, 75 insertions(+), 16 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 52a4e66..4e85f5f 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -16,6 +16,26 @@
 
 static efi_system_table_t *sys_table;
 
+static void efi_printk(char *str)
+{
+	char *s8;
+
+	for (s8 = str; *s8; s8++) {
+		struct efi_simple_text_output_protocol *out;
+		efi_char16_t ch[2] = { 0 };
+
+		ch[0] = *s8;
+		out = (struct efi_simple_text_output_protocol *)sys_table->con_out;
+
+		if (*s8 == '\n') {
+			efi_char16_t nl[2] = { '\r', 0 };
+			efi_call_phys2(out->output_string, out, nl);
+		}
+
+		efi_call_phys2(out->output_string, out, ch);
+	}
+}
+
 static efi_status_t __get_map(efi_memory_desc_t **map, unsigned long *map_size,
 			      unsigned long *desc_size)
 {
@@ -531,8 +551,10 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
 				EFI_LOADER_DATA,
 				nr_initrds * sizeof(*initrds),
 				&initrds);
-	if (status != EFI_SUCCESS)
+	if (status != EFI_SUCCESS) {
+		efi_printk("Failed to alloc mem for initrds\n");
 		goto fail;
+	}
 
 	str = (char *)(unsigned long)hdr->cmd_line_ptr;
 	for (i = 0; i < nr_initrds; i++) {
@@ -575,32 +597,42 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
 
 			status = efi_call_phys3(boottime->handle_protocol,
 					image->device_handle, &fs_proto, &io);
-			if (status != EFI_SUCCESS)
+			if (status != EFI_SUCCESS) {
+				efi_printk("Failed to handle fs_proto\n");
 				goto free_initrds;
+			}
 
 			status = efi_call_phys2(io->open_volume, io, &fh);
-			if (status != EFI_SUCCESS)
+			if (status != EFI_SUCCESS) {
+				efi_printk("Failed to open volume\n");
 				goto free_initrds;
+			}
 		}
 
 		status = efi_call_phys5(fh->open, fh, &h, filename_16,
 					EFI_FILE_MODE_READ, (u64)0);
-		if (status != EFI_SUCCESS)
+		if (status != EFI_SUCCESS) {
+			efi_printk("Failed to open initrd file\n");
 			goto close_handles;
+		}
 
 		initrd->handle = h;
 
 		info_sz = 0;
 		status = efi_call_phys4(h->get_info, h, &info_guid,
 					&info_sz, NULL);
-		if (status != EFI_BUFFER_TOO_SMALL)
+		if (status != EFI_BUFFER_TOO_SMALL) {
+			efi_printk("Failed to get initrd info size\n");
 			goto close_handles;
+		}
 
 grow:
 		status = efi_call_phys3(sys_table->boottime->allocate_pool,
 					EFI_LOADER_DATA, info_sz, &info);
-		if (status != EFI_SUCCESS)
+		if (status != EFI_SUCCESS) {
+			efi_printk("Failed to alloc mem for initrd info\n");
 			goto close_handles;
+		}
 
 		status = efi_call_phys4(h->get_info, h, &info_guid,
 					&info_sz, info);
@@ -612,8 +644,10 @@ grow:
 		file_sz = info->file_size;
 		efi_call_phys1(sys_table->boottime->free_pool, info);
 
-		if (status != EFI_SUCCESS)
+		if (status != EFI_SUCCESS) {
+			efi_printk("Failed to get initrd info\n");
 			goto close_handles;
+		}
 
 		initrd->size = file_sz;
 		initrd_total += file_sz;
@@ -629,11 +663,14 @@ grow:
 		 */
 		status = high_alloc(initrd_total, 0x1000,
 				   &initrd_addr, hdr->initrd_addr_max);
-		if (status != EFI_SUCCESS)
+		if (status != EFI_SUCCESS) {
+			efi_printk("Failed to alloc highmem for initrds\n");
 			goto close_handles;
+		}
 
 		/* We've run out of free low memory. */
 		if (initrd_addr > hdr->initrd_addr_max) {
+			efi_printk("We've run out of free low memory\n");
 			status = EFI_INVALID_PARAMETER;
 			goto free_initrd_total;
 		}
@@ -652,8 +689,10 @@ grow:
 				status = efi_call_phys3(fh->read,
 							initrds[j].handle,
 							&chunksize, addr);
-				if (status != EFI_SUCCESS)
+				if (status != EFI_SUCCESS) {
+					efi_printk("Failed to read initrd\n");
 					goto free_initrd_total;
+				}
 				addr += chunksize;
 				size -= chunksize;
 			}
@@ -732,8 +771,10 @@ static efi_status_t make_boot_params(struct boot_params *boot_params,
 			options_size++;	/* NUL termination */
 
 			status = low_alloc(options_size, 1, &cmdline);
-			if (status != EFI_SUCCESS)
+			if (status != EFI_SUCCESS) {
+				efi_printk("Failed to alloc mem for cmdline\n");
 				goto fail;
+			}
 
 			s1 = (u8 *)(unsigned long)cmdline;
 			s2 = (u16 *)options;
@@ -895,12 +936,16 @@ struct boot_params *efi_main(void *handle, efi_system_table_t *_table)
 
 	status = efi_call_phys3(sys_table->boottime->handle_protocol,
 				handle, &proto, (void *)&image);
-	if (status != EFI_SUCCESS)
+	if (status != EFI_SUCCESS) {
+		efi_printk("Failed to get handle for LOADED_IMAGE_PROTOCOL\n");
 		goto fail;
+	}
 
 	status = low_alloc(0x4000, 1, (unsigned long *)&boot_params);
-	if (status != EFI_SUCCESS)
+	if (status != EFI_SUCCESS) {
+		efi_printk("Failed to alloc lowmem for boot params\n");
 		goto fail;
+	}
 
 	memset(boot_params, 0x0, 0x4000);
 
@@ -933,8 +978,10 @@ struct boot_params *efi_main(void *handle, efi_system_table_t *_table)
 	if (status != EFI_SUCCESS) {
 		status = low_alloc(hdr->init_size, hdr->kernel_alignment,
 				   &start);
-		if (status != EFI_SUCCESS)
+		if (status != EFI_SUCCESS) {
+			efi_printk("Failed to alloc mem for kernel\n");
 			goto fail;
+		}
 	}
 
 	hdr->code32_start = (__u32)start;
@@ -945,19 +992,25 @@ struct boot_params *efi_main(void *handle, efi_system_table_t *_table)
 	status = efi_call_phys3(sys_table->boottime->allocate_pool,
 				EFI_LOADER_DATA, sizeof(*gdt),
 				(void **)&gdt);
-	if (status != EFI_SUCCESS)
+	if (status != EFI_SUCCESS) {
+		efi_printk("Failed to alloc mem for gdt structure\n");
 		goto fail;
+	}
 
 	gdt->size = 0x800;
 	status = low_alloc(gdt->size, 8, (unsigned long *)&gdt->address);
-	if (status != EFI_SUCCESS)
+	if (status != EFI_SUCCESS) {
+		efi_printk("Failed to alloc mem for gdt\n");
 		goto fail;
+	}
 
 	status = efi_call_phys3(sys_table->boottime->allocate_pool,
 				EFI_LOADER_DATA, sizeof(*idt),
 				(void **)&idt);
-	if (status != EFI_SUCCESS)
+	if (status != EFI_SUCCESS) {
+		efi_printk("Failed to alloc mem for idt structure\n");
 		goto fail;
+	}
 
 	idt->size = 0;
 	idt->address = 0;
diff --git a/arch/x86/boot/compressed/eboot.h b/arch/x86/boot/compressed/eboot.h
index 3925166..3b6e156 100644
--- a/arch/x86/boot/compressed/eboot.h
+++ b/arch/x86/boot/compressed/eboot.h
@@ -58,4 +58,10 @@ struct efi_uga_draw_protocol {
 	void *blt;
 };
 
+struct efi_simple_text_output_protocol {
+	void *reset;
+	void *output_string;
+	void *test_string;
+};
+
 #endif /* BOOT_COMPRESSED_EBOOT_H */

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [tip:x86/efi] x86, efi: Add EFI boot stub documentation
  2012-03-16 14:18 ` [PATCH 3/3] x86, efi: Add EFI boot stub documentation Matt Fleming
@ 2012-06-01 19:13   ` tip-bot for Matt Fleming
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Matt Fleming @ 2012-06-01 19:13 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, mjg, hpa, mingo, rdunlap, tglx, matt.fleming

Commit-ID:  0c7596621e313bfcfbacb288e768c7150f5de9e0
Gitweb:     http://git.kernel.org/tip/0c7596621e313bfcfbacb288e768c7150f5de9e0
Author:     Matt Fleming <matt.fleming@intel.com>
AuthorDate: Fri, 16 Mar 2012 12:03:13 +0000
Committer:  H. Peter Anvin <hpa@zytor.com>
CommitDate: Fri, 1 Jun 2012 09:11:41 -0700

x86, efi: Add EFI boot stub documentation

Since we can't expect every user to read the EFI boot stub code it
seems prudent to have a couple of paragraphs explaining what it is and
how it works.

The "initrd=" option in particular is tricky because it only
understands absolute EFI-style paths (backslashes as directory
separators), and until now this hasn't been documented anywhere. This
has tripped up a couple of users.

Cc: Matthew Garrett <mjg@redhat.com>
Cc: Randy Dunlap <rdunlap@xenotime.net>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
Link: http://lkml.kernel.org/r/1331907517-3985-4-git-send-email-matt@console-pimps.org
Signed-off-by: H. Peter Anvin <hpa@zytor.com>
---
 Documentation/x86/efi-stub.txt |   65 ++++++++++++++++++++++++++++++++++++++++
 arch/x86/Kconfig               |    2 +
 2 files changed, 67 insertions(+), 0 deletions(-)

diff --git a/Documentation/x86/efi-stub.txt b/Documentation/x86/efi-stub.txt
new file mode 100644
index 0000000..44e6bb6
--- /dev/null
+++ b/Documentation/x86/efi-stub.txt
@@ -0,0 +1,65 @@
+			  The EFI Boot Stub
+		     ---------------------------
+
+On the x86 platform, a bzImage can masquerade as a PE/COFF image,
+thereby convincing EFI firmware loaders to load it as an EFI
+executable. The code that modifies the bzImage header, along with the
+EFI-specific entry point that the firmware loader jumps to are
+collectively known as the "EFI boot stub", and live in
+arch/x86/boot/header.S and arch/x86/boot/compressed/eboot.c,
+respectively.
+
+By using the EFI boot stub it's possible to boot a Linux kernel
+without the use of a conventional EFI boot loader, such as grub or
+elilo. Since the EFI boot stub performs the jobs of a boot loader, in
+a certain sense it *IS* the boot loader.
+
+The EFI boot stub is enabled with the CONFIG_EFI_STUB kernel option.
+
+
+**** How to install bzImage.efi
+
+The bzImage located in arch/x86/boot/bzImage must be copied to the EFI
+System Partiion (ESP) and renamed with the extension ".efi". Without
+the extension the EFI firmware loader will refuse to execute it. It's
+not possible to execute bzImage.efi from the usual Linux file systems
+because EFI firmware doesn't have support for them.
+
+
+**** Passing kernel parameters from the EFI shell
+
+Arguments to the kernel can be passed after bzImage.efi, e.g.
+
+	fs0:> bzImage.efi console=ttyS0 root=/dev/sda4
+
+
+**** The "initrd=" option
+
+Like most boot loaders, the EFI stub allows the user to specify
+multiple initrd files using the "initrd=" option. This is the only EFI
+stub-specific command line parameter, everything else is passed to the
+kernel when it boots.
+
+The path to the initrd file must be an absolute path from the
+beginning of the ESP, relative path names do not work. Also, the path
+is an EFI-style path and directory elements must be separated with
+backslashes (\). For example, given the following directory layout,
+
+fs0:>
+	Kernels\
+			bzImage.efi
+			initrd-large.img
+
+	Ramdisks\
+			initrd-small.img
+			initrd-medium.img
+
+to boot with the initrd-large.img file if the current working
+directory is fs0:\Kernels, the following command must be used,
+
+	fs0:\Kernels> bzImage.efi initrd=\Kernels\initrd-large.img
+
+Notice how bzImage.efi can be specified with a relative path. That's
+because the image we're executing is interpreted by the EFI shell,
+which understands relative paths, whereas the rest of the command line
+is passed to bzImage.efi.
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d700811..c70684f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1506,6 +1506,8 @@ config EFI_STUB
           This kernel feature allows a bzImage to be loaded directly
 	  by EFI firmware without the use of a bootloader.
 
+	  See Documentation/x86/efi-stub.txt for more information.
+
 config SECCOMP
 	def_bool y
 	prompt "Enable seccomp to safely compute untrusted bytecode"

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-16 14:18 [PATCH 0/3] x86, efi: EFI boot stub documentation Matt Fleming
2012-03-16 14:18 ` [PATCH 1/3] x86, efi: Only close open files in error path Matt Fleming
2012-06-01 19:11   ` [tip:x86/efi] " tip-bot for Matt Fleming
2012-03-16 14:18 ` [PATCH 2/3] x86, efi; Add EFI boot stub console support Matt Fleming
2012-06-01 19:11   ` [tip:x86/efi] " tip-bot for Matt Fleming
2012-03-16 14:18 ` [PATCH 3/3] x86, efi: Add EFI boot stub documentation Matt Fleming
2012-06-01 19:13   ` [tip:x86/efi] " tip-bot for Matt Fleming
2012-03-20 23:57 ` [PATCH 0/3] x86, efi: " Shea Levy
2012-03-21  0:00   ` H. Peter Anvin
2012-05-04 10:27 ` Matt Fleming

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git