[U-Boot] [PATCH v3 1/8] x86: Add implementations of setjmp() and longjmp()
Bring in these functions from Linux v4.4. They will be needed for EFI loader support.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: - Add a parameter to longjmp() - Drop unused header files
Changes in v2: - Drop irrelevant comment - Add a comment about .size - Drop unnecessary .text directive - Make longjmp() always cause setjmp() to return 1
arch/x86/cpu/Makefile | 2 +- arch/x86/cpu/setjmp.S | 61 +++++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/setjmp.h | 24 +++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 arch/x86/cpu/setjmp.S create mode 100644 arch/x86/include/asm/setjmp.h
diff --git a/arch/x86/cpu/Makefile b/arch/x86/cpu/Makefile index 2667e0b..f5b8c9e 100644 --- a/arch/x86/cpu/Makefile +++ b/arch/x86/cpu/Makefile @@ -10,7 +10,7 @@
extra-y = start.o obj-$(CONFIG_X86_RESET_VECTOR) += resetvec.o start16.o -obj-y += interrupts.o cpu.o cpu_x86.o call64.o +obj-y += interrupts.o cpu.o cpu_x86.o call64.o setjmp.o
AFLAGS_REMOVE_call32.o := -mregparm=3 \ $(if $(CONFIG_EFI_STUB_64BIT),-march=i386 -m32) diff --git a/arch/x86/cpu/setjmp.S b/arch/x86/cpu/setjmp.S new file mode 100644 index 0000000..2ea1c6c --- /dev/null +++ b/arch/x86/cpu/setjmp.S @@ -0,0 +1,61 @@ +/* + * Written by H. Peter Anvin hpa@zytor.com + * Brought in from Linux v4.4 and modified for U-Boot + * From Linux arch/um/sys-i386/setjmp.S + * + * SPDX-License-Identifier: GPL-2.0 + */ + +#define _REGPARM + +/* + * The jmp_buf is assumed to contain the following, in order: + * %ebx + * %esp + * %ebp + * %esi + * %edi + * <return address> + */ + + .text + .align 4 + .globl setjmp + .type setjmp, @function +setjmp: +#ifdef _REGPARM + movl %eax, %edx +#else + movl 4(%esp), %edx +#endif + popl %ecx /* Return address, and adjust the stack */ + xorl %eax, %eax /* Return value */ + movl %ebx, (%edx) + movl %esp, 4(%edx) /* Post-return %esp! */ + pushl %ecx /* Make the call/return stack happy */ + movl %ebp, 8(%edx) + movl %esi, 12(%edx) + movl %edi, 16(%edx) + movl %ecx, 20(%edx) /* Return address */ + ret + + /* Provide function size if needed */ + .size setjmp, .-setjmp + + .align 4 + .globl longjmp + .type longjmp, @function +longjmp: +#ifdef _REGPARM + xchgl %eax, %edx +#else + movl 4(%esp), %edx /* jmp_ptr address */ +#endif + movl (%edx), %ebx + movl 4(%edx), %esp + movl 8(%edx), %ebp + movl 12(%edx), %esi + movl 16(%edx), %edi + jmp *20(%edx) + + .size longjmp, .-longjmp diff --git a/arch/x86/include/asm/setjmp.h b/arch/x86/include/asm/setjmp.h new file mode 100644 index 0000000..1feaa89 --- /dev/null +++ b/arch/x86/include/asm/setjmp.h @@ -0,0 +1,24 @@ +/* + * Written by H. Peter Anvin hpa@zytor.com + * Brought in from Linux v4.4 and modified for U-Boot + * From Linux arch/um/sys-i386/setjmp.S + * + * SPDX-License-Identifier: GPL-2.0 + */ + +#ifndef __setjmp_h +#define __setjmp_h + +struct jmp_buf_data { + unsigned int __ebx; + unsigned int __esp; + unsigned int __ebp; + unsigned int __esi; + unsigned int __edi; + unsigned int __eip; +}; + +int setjmp(struct jmp_buf_data *jmp_buf); +void longjmp(struct jmp_buf_data *jmp_buf, int val); + +#endif
This is required for x86 and is also correct for ARM (since it is empty).
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: None Changes in v2: - Move efi.h changes to a new patch
arch/avr32/include/asm/linkage.h | 0 arch/m68k/include/asm/linkage.h | 0 arch/microblaze/include/asm/linkage.h | 0 arch/mips/include/asm/linkage.h | 0 arch/nios2/include/asm/linkage.h | 0 arch/openrisc/include/asm/linkage.h | 0 arch/sandbox/include/asm/linkage.h | 0 arch/sh/include/asm/linkage.h | 0 arch/sparc/include/asm/linkage.h | 0 include/efi.h | 3 ++- 10 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 arch/avr32/include/asm/linkage.h create mode 100644 arch/m68k/include/asm/linkage.h create mode 100644 arch/microblaze/include/asm/linkage.h create mode 100644 arch/mips/include/asm/linkage.h create mode 100644 arch/nios2/include/asm/linkage.h create mode 100644 arch/openrisc/include/asm/linkage.h create mode 100644 arch/sandbox/include/asm/linkage.h create mode 100644 arch/sh/include/asm/linkage.h create mode 100644 arch/sparc/include/asm/linkage.h
diff --git a/arch/avr32/include/asm/linkage.h b/arch/avr32/include/asm/linkage.h new file mode 100644 index 0000000..e69de29 diff --git a/arch/m68k/include/asm/linkage.h b/arch/m68k/include/asm/linkage.h new file mode 100644 index 0000000..e69de29 diff --git a/arch/microblaze/include/asm/linkage.h b/arch/microblaze/include/asm/linkage.h new file mode 100644 index 0000000..e69de29 diff --git a/arch/mips/include/asm/linkage.h b/arch/mips/include/asm/linkage.h new file mode 100644 index 0000000..e69de29 diff --git a/arch/nios2/include/asm/linkage.h b/arch/nios2/include/asm/linkage.h new file mode 100644 index 0000000..e69de29 diff --git a/arch/openrisc/include/asm/linkage.h b/arch/openrisc/include/asm/linkage.h new file mode 100644 index 0000000..e69de29 diff --git a/arch/sandbox/include/asm/linkage.h b/arch/sandbox/include/asm/linkage.h new file mode 100644 index 0000000..e69de29 diff --git a/arch/sh/include/asm/linkage.h b/arch/sh/include/asm/linkage.h new file mode 100644 index 0000000..e69de29 diff --git a/arch/sparc/include/asm/linkage.h b/arch/sparc/include/asm/linkage.h new file mode 100644 index 0000000..e69de29 diff --git a/include/efi.h b/include/efi.h index 5a3b8cf..d07187c 100644 --- a/include/efi.h +++ b/include/efi.h @@ -15,6 +15,7 @@ #ifndef _EFI_H #define _EFI_H
+#include <linux/linkage.h> #include <linux/string.h> #include <linux/types.h>
@@ -22,7 +23,7 @@ /* EFI uses the Microsoft ABI which is not the default for GCC */ #define EFIAPI __attribute__((ms_abi)) #else -#define EFIAPI +#define EFIAPI asmlinkage #endif
struct efi_device_path;
On 10/18/2016 04:29 AM, Simon Glass wrote:
This is required for x86 and is also correct for ARM (since it is empty).
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
(Replying here in lack for a cover letter)
Could you please rebase your patches on top of
https://github.com/agraf/u-boot.git efi-next
so that all the patches that I already queued are not repeated in the patch set and we don't get any conflicts?
Thanks!
Alex
Hi Alex,
On 18 October 2016 at 01:12, Alexander Graf agraf@suse.de wrote:
On 10/18/2016 04:29 AM, Simon Glass wrote:
This is required for x86 and is also correct for ARM (since it is empty).
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
(Replying here in lack for a cover letter)
Could you please rebase your patches on top of
https://github.com/agraf/u-boot.git efi-next
so that all the patches that I already queued are not repeated in the patch set and we don't get any conflicts?
I can do that - but is this targeting -next? I was expecting these patches to land in master.
Regards, Simon
On 18/10/2016 22:37, Simon Glass wrote:
Hi Alex,
On 18 October 2016 at 01:12, Alexander Graf agraf@suse.de wrote:
On 10/18/2016 04:29 AM, Simon Glass wrote:
This is required for x86 and is also correct for ARM (since it is empty).
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
(Replying here in lack for a cover letter)
Could you please rebase your patches on top of
https://github.com/agraf/u-boot.git efi-next
so that all the patches that I already queued are not repeated in the patch set and we don't get any conflicts?
I can do that - but is this targeting -next? I was expecting these patches to land in master.
Sorry, they are on their way to master. It's just old habit wrt my naming scheme:
-next: current development branch, basically staging for master -release-number: future patches that I already applied proactively for the next release, or backports :)
As it stands, I've only taken the base patches, not all of them. Would you expect all of the x86 efi enablement to land in 2016.11?
Alex
Hi Alex,
On 19 October 2016 at 01:11, Alexander Graf agraf@suse.de wrote:
On 18/10/2016 22:37, Simon Glass wrote:
Hi Alex,
On 18 October 2016 at 01:12, Alexander Graf agraf@suse.de wrote:
On 10/18/2016 04:29 AM, Simon Glass wrote:
This is required for x86 and is also correct for ARM (since it is empty).
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
(Replying here in lack for a cover letter)
Could you please rebase your patches on top of
https://github.com/agraf/u-boot.git efi-next
so that all the patches that I already queued are not repeated in the patch set and we don't get any conflicts?
I can do that - but is this targeting -next? I was expecting these patches to land in master.
Sorry, they are on their way to master. It's just old habit wrt my naming scheme:
-next: current development branch, basically staging for master
I'd suggest calling that 'master'.
-release-number: future patches that I already applied proactively for the next release, or backports :)
As it stands, I've only taken the base patches, not all of them. Would you expect all of the x86 efi enablement to land in 2016.11?
No, let's target the release after that.
Regards, Simon
These are missing in some functions. Add them to keep things consistent.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Reviewed-by: Alexander Graf agraf@suse.de ---
Changes in v3: None Changes in v2: None
cmd/bootefi.c | 7 +++++-- include/efi_loader.h | 2 +- lib/efi_loader/efi_boottime.c | 5 +++-- lib/efi_loader/efi_disk.c | 13 +++++++------ lib/efi_loader/efi_net.c | 4 ++-- 5 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 21fe42c..7b2e0b5 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -15,6 +15,8 @@ #include <libfdt_env.h> #include <memalign.h> #include <asm/global_data.h> +#include <asm-generic/sections.h> +#include <linux/linkage.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -52,7 +54,7 @@ static struct efi_device_path_file_path bootefi_device_path[] = { } };
-static efi_status_t bootefi_open_dp(void *handle, efi_guid_t *protocol, +static efi_status_t EFIAPI bootefi_open_dp(void *handle, efi_guid_t *protocol, void **protocol_interface, void *agent_handle, void *controller_handle, uint32_t attributes) { @@ -145,7 +147,8 @@ static void *copy_fdt(void *fdt) */ static unsigned long do_bootefi_exec(void *efi, void *fdt) { - ulong (*entry)(void *image_handle, struct efi_system_table *st); + ulong (*entry)(void *image_handle, struct efi_system_table *st) + asmlinkage; ulong fdt_pages, fdt_size, fdt_start, fdt_end; bootm_headers_t img = { 0 };
diff --git a/include/efi_loader.h b/include/efi_loader.h index 9738835..aa4ae0e 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -93,7 +93,7 @@ void efi_net_set_dhcp_ack(void *pkt, int len); * Stub implementation for a protocol opener that just returns the handle as * interface */ -efi_status_t efi_return_handle(void *handle, +efi_status_t EFIAPI efi_return_handle(void *handle, efi_guid_t *protocol, void **protocol_interface, void *agent_handle, void *controller_handle, uint32_t attributes); diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 792db39..01a9b8f 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -159,7 +159,7 @@ static struct { u32 trigger_time; u64 trigger_next; unsigned long notify_tpl; - void (*notify_function) (void *event, void *context); + void (EFIAPI *notify_function) (void *event, void *context); void *notify_context; } efi_event = { /* Disable timers on bootup */ @@ -168,7 +168,8 @@ static struct {
static efi_status_t EFIAPI efi_create_event( enum efi_event_type type, ulong notify_tpl, - void (*notify_function) (void *event, void *context), + void (EFIAPI *notify_function) (void *event, + void *context), void *notify_context, void **event) { EFI_ENTRY("%d, 0x%lx, %p, %p", type, notify_tpl, notify_function, diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index d8ddcc9..1e3dca4 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -35,9 +35,10 @@ struct efi_disk_obj { const struct blk_desc *desc; };
-static efi_status_t efi_disk_open_block(void *handle, efi_guid_t *protocol, - void **protocol_interface, void *agent_handle, - void *controller_handle, uint32_t attributes) +static efi_status_t EFIAPI efi_disk_open_block(void *handle, + efi_guid_t *protocol, void **protocol_interface, + void *agent_handle, void *controller_handle, + uint32_t attributes) { struct efi_disk_obj *diskobj = handle;
@@ -46,7 +47,7 @@ static efi_status_t efi_disk_open_block(void *handle, efi_guid_t *protocol, return EFI_SUCCESS; }
-static efi_status_t efi_disk_open_dp(void *handle, efi_guid_t *protocol, +static efi_status_t EFIAPI efi_disk_open_dp(void *handle, efi_guid_t *protocol, void **protocol_interface, void *agent_handle, void *controller_handle, uint32_t attributes) { @@ -108,7 +109,7 @@ static efi_status_t EFIAPI efi_disk_rw_blocks(struct efi_block_io *this, return EFI_EXIT(EFI_SUCCESS); }
-static efi_status_t efi_disk_read_blocks(struct efi_block_io *this, +static efi_status_t EFIAPI efi_disk_read_blocks(struct efi_block_io *this, u32 media_id, u64 lba, unsigned long buffer_size, void *buffer) { @@ -143,7 +144,7 @@ static efi_status_t efi_disk_read_blocks(struct efi_block_io *this, return EFI_EXIT(r); }
-static efi_status_t efi_disk_write_blocks(struct efi_block_io *this, +static efi_status_t EFIAPI efi_disk_write_blocks(struct efi_block_io *this, u32 media_id, u64 lba, unsigned long buffer_size, void *buffer) { diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index dd3b485..9199518 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -191,7 +191,7 @@ static efi_status_t EFIAPI efi_net_receive(struct efi_simple_network *this, return EFI_EXIT(EFI_SUCCESS); }
-static efi_status_t efi_net_open_dp(void *handle, efi_guid_t *protocol, +static efi_status_t EFIAPI efi_net_open_dp(void *handle, efi_guid_t *protocol, void **protocol_interface, void *agent_handle, void *controller_handle, uint32_t attributes) { @@ -203,7 +203,7 @@ static efi_status_t efi_net_open_dp(void *handle, efi_guid_t *protocol, return EFI_SUCCESS; }
-static efi_status_t efi_net_open_pxe(void *handle, efi_guid_t *protocol, +static efi_status_t EFIAPI efi_net_open_pxe(void *handle, efi_guid_t *protocol, void **protocol_interface, void *agent_handle, void *controller_handle, uint32_t attributes) {
At present we use a CONFIG option in efi.h to determine whether we are building the EFI stub or not. This means that the same header cannot be used for EFI_LOADER support. The CONFIG option will be enabled for the whole build, even when not building the stub.
Use a different define instead, set up just for the files that make up the stub.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: None Changes in v2: - Add new patch to tidy up selection of building the EFI stub
include/efi.h | 7 +++++-- lib/efi/Makefile | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/include/efi.h b/include/efi.h index d07187c..3d58780 100644 --- a/include/efi.h +++ b/include/efi.h @@ -30,8 +30,11 @@ struct efi_device_path;
#define EFI_BITS_PER_LONG BITS_PER_LONG
-/* With 64-bit EFI stub, EFI_BITS_PER_LONG has to be 64 */ -#ifdef CONFIG_EFI_STUB_64BIT +/* + * With 64-bit EFI stub, EFI_BITS_PER_LONG has to be 64. EFI_STUB is set + * in lib/efi/Makefile, when building the stub. + */ +#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB) #undef EFI_BITS_PER_LONG #define EFI_BITS_PER_LONG 64 #endif diff --git a/lib/efi/Makefile b/lib/efi/Makefile index e32dc14..9449600 100644 --- a/lib/efi/Makefile +++ b/lib/efi/Makefile @@ -9,9 +9,9 @@ obj-$(CONFIG_EFI_STUB) += efi_info.o
CFLAGS_REMOVE_efi_stub.o := -mregparm=3 \ $(if $(CONFIG_EFI_STUB_64BIT),-march=i386 -m32) -CFLAGS_efi_stub.o := -fpic -fshort-wchar +CFLAGS_efi_stub.o := -fpic -fshort-wchar -DEFI_STUB CFLAGS_REMOVE_efi.o := -mregparm=3 \ $(if $(CONFIG_EFI_STUB_64BIT),-march=i386 -m32) -CFLAGS_efi.o := -fpic -fshort-wchar +CFLAGS_efi.o := -fpic -fshort-wchar -DEFI_STUB
extra-$(CONFIG_EFI_STUB) += efi_stub.o efi.o
On Tue, Oct 18, 2016 at 10:29 AM, Simon Glass sjg@chromium.org wrote:
At present we use a CONFIG option in efi.h to determine whether we are building the EFI stub or not. This means that the same header cannot be used for EFI_LOADER support. The CONFIG option will be enabled for the whole build, even when not building the stub.
Use a different define instead, set up just for the files that make up the stub.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3: None Changes in v2:
- Add new patch to tidy up selection of building the EFI stub
include/efi.h | 7 +++++-- lib/efi/Makefile | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
It is useful to have a basic sanity check for EFI loader support. Add a 'bootefi hello' command which loads HelloWord.efi and runs it under U-Boot.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: - Include a link to the program instead of adding it to the tree - Fix several typos - Align backslashes to the same column
Changes in v2: None
arch/arm/lib/Makefile | 7 +++++++ cmd/Kconfig | 9 +++++++++ cmd/bootefi.c | 26 ++++++++++++++++++++------ doc/README.efi | 22 ++++++++++++++++++++++ include/asm-generic/sections.h | 2 ++ scripts/Makefile.lib | 19 +++++++++++++++++++ 6 files changed, 79 insertions(+), 6 deletions(-)
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index caa62c6..64378e1 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -30,6 +30,13 @@ obj-$(CONFIG_CMD_BOOTI) += bootm.o obj-$(CONFIG_CMD_BOOTM) += bootm.o obj-$(CONFIG_CMD_BOOTZ) += bootm.o zimage.o obj-$(CONFIG_SYS_L2_PL310) += cache-pl310.o +ifdef CONFIG_ARM64 +# This option does not work for arm64, as there is no binary. +# TODO(sjg@chromium.org): Add this once it is possible to build one +obj-$(CONFIG_CMD_BOOTEFI_HELLO) += HelloWorld64.o +else +obj-$(CONFIG_CMD_BOOTEFI_HELLO) += HelloWorld32.o +endif obj-$(CONFIG_USE_ARCH_MEMSET) += memset.o obj-$(CONFIG_USE_ARCH_MEMCPY) += memcpy.o else diff --git a/cmd/Kconfig b/cmd/Kconfig index e339d86..a5d030b 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -181,6 +181,15 @@ config CMD_BOOTEFI help Boot an EFI image from memory.
+config CMD_BOOTEFI_HELLO + bool "Allow booting a standard EFI hello world for testing" + depends on CMD_BOOTEFI + help + This adds a standard EFI hello world application to U-Boot so that + it can be used with the 'bootefi hello' command. This is useful + for testing that EFI is working at a basic level, and for bringing + up EFI support on a new architecture. + config CMD_ELF bool "bootelf, bootvx" default y diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 7b2e0b5..7c2d9fc 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -230,13 +230,22 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
if (argc < 2) return CMD_RET_USAGE; - saddr = argv[1]; +#ifdef CONFIG_CMD_BOOTEFI_HELLO + if (!strcmp(argv[1], "hello")) { + addr = CONFIG_SYS_LOAD_ADDR; + memcpy((char *)addr, __efi_hello_world_begin, + __efi_hello_world_end - __efi_hello_world_begin); + } else +#endif + { + saddr = argv[1];
- addr = simple_strtoul(saddr, NULL, 16); + addr = simple_strtoul(saddr, NULL, 16);
- if (argc > 2) { - sfdt = argv[2]; - fdt_addr = simple_strtoul(sfdt, NULL, 16); + if (argc > 2) { + sfdt = argv[2]; + fdt_addr = simple_strtoul(sfdt, NULL, 16); + } }
printf("## Starting EFI application at 0x%08lx ...\n", addr); @@ -254,7 +263,12 @@ static char bootefi_help_text[] = "<image address> [fdt address]\n" " - boot EFI payload stored at address <image address>.\n" " If specified, the device tree located at <fdt address> gets\n" - " exposed as EFI configuration table.\n"; + " exposed as EFI configuration table.\n" +#ifdef CONFIG_CMD_BOOTEFI_HELLO + "hello\n" + " - boot a sample Hello World application stored within U-Boot" +#endif + ; #endif
U_BOOT_CMD( diff --git a/doc/README.efi b/doc/README.efi index 1fd3f00..6e76310 100644 --- a/doc/README.efi +++ b/doc/README.efi @@ -310,6 +310,28 @@ Removable media booting (search for /efi/boot/boota{a64,arm}.efi) is supported. Simple use cases like "Plug this SD card into my ARM device and it just boots into grub which boots into Linux", work very well.
+ +Running HelloWord.efi +--------------------- + +You can run a simple 'hello world' EFI program in U-Boot. This is not +distributed in the U-Boot source, but you can download this patch: + + https://goo.gl/vihiZS + +Then apply it, e.g.: + + $ git am ~/Downloads/0001-efi-Add-test-program-binaries.patch + +and enable the option CONFIG_CMD_BOOTEFI_HELLO + +Then you can boot into U-Boot and type: + + > bootefi hello + +The 'hello world EFI' program will then run, print a message and exit. + + Future work -----------
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h index d69bc60..daf021b 100644 --- a/include/asm-generic/sections.h +++ b/include/asm-generic/sections.h @@ -22,6 +22,8 @@ extern char __kprobes_text_start[], __kprobes_text_end[]; extern char __entry_text_start[], __entry_text_end[]; extern char __initdata_begin[], __initdata_end[]; extern char __start_rodata[], __end_rodata[]; +extern char __efi_hello_world_begin[]; +extern char __efi_hello_world_end[];
/* Start and end of .ctors section - used for constructor calls. */ extern char __ctors_start[], __ctors_end[]; diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 45a0e1d..1f534b7 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -321,6 +321,25 @@ cmd_S_ttf= \ $(obj)/%.S: $(src)/%.ttf $(call cmd,S_ttf)
+# EFI Hello World application +# --------------------------------------------------------------------------- + +# Generate an assembly file to wrap the EFI app +cmd_S_efi= \ +( \ + echo '.section .rodata.efi.init,"a"'; \ + echo '.balign 16'; \ + echo '.global __efi_hello_world_begin'; \ + echo '__efi_hello_world_begin:'; \ + echo '.incbin "$<" '; \ + echo '__efi_hello_world_end:'; \ + echo '.global __efi_hello_world_end'; \ + echo '.balign 16'; \ +) > $@ + +$(obj)/%.S: $(src)/%.efi + $(call cmd,S_efi) + # ACPI # --------------------------------------------------------------------------- quiet_cmd_acpi_c_asl= ASL $<
Add the required pieces to support the EFI loader on x86.
Since U-Boot only builds for 32-bit on x86, only a 32-bit EFI application is supported. If a 64-bit kernel must be booted, U-Boot supports this directly using FIT (see doc/uImage.FIT/kernel.its). U-Boot can act as a payload for both 32-bit and 64-bit EFI.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: None Changes in v2: None
arch/x86/cpu/u-boot.lds | 36 +++++++++++++++++++++++++++++++++++- arch/x86/lib/Makefile | 1 + arch/x86/lib/sections.c | 12 ++++++++++++ lib/efi_loader/efi_boottime.c | 9 +++++++++ lib/efi_loader/efi_runtime.c | 4 ++++ 5 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 arch/x86/lib/sections.c
diff --git a/arch/x86/cpu/u-boot.lds b/arch/x86/cpu/u-boot.lds index 36f59ea..cca536b 100644 --- a/arch/x86/cpu/u-boot.lds +++ b/arch/x86/cpu/u-boot.lds @@ -28,7 +28,10 @@ SECTIONS }
. = ALIGN(4); - .rodata : { *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) } + .rodata : { + *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) + KEEP(*(.rodata.efi.init)); + }
. = ALIGN(4); .data : { *(.data*) } @@ -40,6 +43,37 @@ SECTIONS .got : { *(.got*) }
. = ALIGN(4); + + .__efi_runtime_start : { + *(.__efi_runtime_start) + } + + .efi_runtime : { + *(efi_runtime_text) + *(efi_runtime_data) + } + + .__efi_runtime_stop : { + *(.__efi_runtime_stop) + } + + .efi_runtime_rel_start : + { + *(.__efi_runtime_rel_start) + } + + .efi_runtime_rel : { + *(.relefi_runtime_text) + *(.relefi_runtime_data) + } + + .efi_runtime_rel_stop : + { + *(.__efi_runtime_rel_stop) + } + + . = ALIGN(4); + __data_end = .; __init_end = .;
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile index e17f0bb..e46e7f1 100644 --- a/arch/x86/lib/Makefile +++ b/arch/x86/lib/Makefile @@ -28,6 +28,7 @@ obj-y += pirq_routing.o obj-y += relocate.o obj-y += physmem.o obj-$(CONFIG_X86_RAMTEST) += ramtest.o +obj-y += sections.o obj-y += sfi.o obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o obj-y += string.o diff --git a/arch/x86/lib/sections.c b/arch/x86/lib/sections.c new file mode 100644 index 0000000..6455e0f --- /dev/null +++ b/arch/x86/lib/sections.c @@ -0,0 +1,12 @@ +/* + * Copyright 2013 Albert ARIBAUD albert.u.boot@aribaud.net + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +char __efi_runtime_start[0] __attribute__((section(".__efi_runtime_start"))); +char __efi_runtime_stop[0] __attribute__((section(".__efi_runtime_stop"))); +char __efi_runtime_rel_start[0] + __attribute__((section(".__efi_runtime_rel_start"))); +char __efi_runtime_rel_stop[0] + __attribute__((section(".__efi_runtime_rel_stop"))); diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 01a9b8f..d16fb4e 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -39,6 +39,7 @@ static bool efi_is_direct_boot = true; */ static struct efi_configuration_table EFI_RUNTIME_DATA efi_conf_table[1];
+#ifdef CONFIG_ARM /* * The "gd" pointer lives in a register on ARM and AArch64 that we declare * fixed when compiling U-Boot. However, the payload does not know about that @@ -46,16 +47,20 @@ static struct efi_configuration_table EFI_RUNTIME_DATA efi_conf_table[1]; * EFI callback entry/exit. */ static volatile void *efi_gd, *app_gd; +#endif
/* Called from do_bootefi_exec() */ void efi_save_gd(void) { +#ifdef CONFIG_ARM efi_gd = gd; +#endif }
/* Called on every callback entry */ void efi_restore_gd(void) { +#ifdef CONFIG_ARM /* Only restore if we're already in EFI context */ if (!efi_gd) return; @@ -63,12 +68,16 @@ void efi_restore_gd(void) if (gd != efi_gd) app_gd = gd; gd = efi_gd; +#endif }
/* Called on every callback exit */ efi_status_t efi_exit_func(efi_status_t ret) { +#ifdef CONFIG_ARM gd = app_gd; +#endif + return ret; }
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 99b5ef1..3793471 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -34,6 +34,10 @@ static efi_status_t EFI_RUNTIME_TEXT EFIAPI efi_invalid_parameter(void); #elif defined(CONFIG_ARM) #define R_RELATIVE 23 #define R_MASK 0xffULL +#elif defined(CONFIG_X86) +#include <asm/elf.h> +#define R_RELATIVE R_386_RELATIVE +#define R_MASK 0xffULL #else #error Need to add relocation awareness #endif
It is useful to have a basic sanity check for EFI loader support. Add a 'bootefi hello' command which loads HelloWord.efi and runs it under U-Boot.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: - Include a link to the program instead of adding it to the tree
Changes in v2: None
arch/x86/lib/Makefile | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile index e46e7f1..1d9ebc0 100644 --- a/arch/x86/lib/Makefile +++ b/arch/x86/lib/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o obj-y += cmd_boot.o obj-$(CONFIG_SEABIOS) += coreboot_table.o obj-$(CONFIG_EFI) += efi/ +obj-$(CONFIG_CMD_BOOTEFI_HELLO) += HelloWorld32.o obj-y += e820.o obj-y += gcc.o obj-y += init_helpers.o
On Tue, Oct 18, 2016 at 10:29 AM, Simon Glass sjg@chromium.org wrote:
It is useful to have a basic sanity check for EFI loader support. Add a 'bootefi hello' command which loads HelloWord.efi and runs it under U-Boot.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Include a link to the program instead of adding it to the tree
Changes in v2: None
arch/x86/lib/Makefile | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
On 10/18/2016 04:29 AM, Simon Glass wrote:
It is useful to have a basic sanity check for EFI loader support. Add a 'bootefi hello' command which loads HelloWord.efi and runs it under U-Boot.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Include a link to the program instead of adding it to the tree
So, uh, where is the link?
I'm really not convinced this command buys us anything yet. I do agree that we want automated testing - but can't we get that using QEMU and a downloadable image file that we pass in as disk and have the distro boot do its magic?
Alex
Hi Alex,
On 18 October 2016 at 01:14, Alexander Graf agraf@suse.de wrote:
On 10/18/2016 04:29 AM, Simon Glass wrote:
It is useful to have a basic sanity check for EFI loader support. Add a 'bootefi hello' command which loads HelloWord.efi and runs it under U-Boot.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Include a link to the program instead of adding it to the tree
So, uh, where is the link?
I put it in the README (see the arm patch).
I'm really not convinced this command buys us anything yet. I do agree that we want automated testing - but can't we get that using QEMU and a downloadable image file that we pass in as disk and have the distro boot do its magic?
That seems very heavyweight as a sanity check, although I agree it is useful.
Here I am just making sure that EFI programs can start, print output and exit. It is a test that we can easily run without a lot of overhead, much less than a full distro boot.
Regards, Simon
On 18/10/2016 22:37, Simon Glass wrote:
Hi Alex,
On 18 October 2016 at 01:14, Alexander Graf agraf@suse.de wrote:
On 10/18/2016 04:29 AM, Simon Glass wrote:
It is useful to have a basic sanity check for EFI loader support. Add a 'bootefi hello' command which loads HelloWord.efi and runs it under U-Boot.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Include a link to the program instead of adding it to the tree
So, uh, where is the link?
I put it in the README (see the arm patch).
I'm really not convinced this command buys us anything yet. I do agree that we want automated testing - but can't we get that using QEMU and a downloadable image file that we pass in as disk and have the distro boot do its magic?
That seems very heavyweight as a sanity check, although I agree it is useful.
It's not really much more heavy weight. The "image file" could simply contain your hello world binary. But with this we don't just verify whether "bootefi" works, but also whether the default boot path works ok.
Here I am just making sure that EFI programs can start, print output and exit. It is a test that we can easily run without a lot of overhead, much less than a full distro boot.
Again, I don't think it's much more overhead and I do believe it gives us much cleaner separation between responsibilities of code (tests go where tests are).
Alex
Hi Alex,
On 19 October 2016 at 01:09, Alexander Graf agraf@suse.de wrote:
On 18/10/2016 22:37, Simon Glass wrote:
Hi Alex,
On 18 October 2016 at 01:14, Alexander Graf agraf@suse.de wrote:
On 10/18/2016 04:29 AM, Simon Glass wrote:
It is useful to have a basic sanity check for EFI loader support. Add a 'bootefi hello' command which loads HelloWord.efi and runs it under U-Boot.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Include a link to the program instead of adding it to the tree
So, uh, where is the link?
I put it in the README (see the arm patch).
I'm really not convinced this command buys us anything yet. I do agree that we want automated testing - but can't we get that using QEMU and a downloadable image file that we pass in as disk and have the distro boot do its magic?
That seems very heavyweight as a sanity check, although I agree it is useful.
It's not really much more heavy weight. The "image file" could simply contain your hello world binary. But with this we don't just verify whether "bootefi" works, but also whether the default boot path works ok.
I don't think I understand what you mean by 'image file'. Is it something other than the .efi file? Do you mean a disk image?
Here I am just making sure that EFI programs can start, print output and exit. It is a test that we can easily run without a lot of overhead, much less than a full distro boot.
Again, I don't think it's much more overhead and I do believe it gives us much cleaner separation between responsibilities of code (tests go where tests are).
You are talking about a functional test, something that tests things end to end. I prefer to at least start with a smaller test. Granted it takes a little more work but it means there are fewer things to hunt through when something goes wrong.
Regards, Simon
On 07/11/2016 10:46, Simon Glass wrote:
Hi Alex,
On 19 October 2016 at 01:09, Alexander Graf agraf@suse.de wrote:
On 18/10/2016 22:37, Simon Glass wrote:
Hi Alex,
On 18 October 2016 at 01:14, Alexander Graf agraf@suse.de wrote:
On 10/18/2016 04:29 AM, Simon Glass wrote:
It is useful to have a basic sanity check for EFI loader support. Add a 'bootefi hello' command which loads HelloWord.efi and runs it under U-Boot.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Include a link to the program instead of adding it to the tree
So, uh, where is the link?
I put it in the README (see the arm patch).
I'm really not convinced this command buys us anything yet. I do agree that we want automated testing - but can't we get that using QEMU and a downloadable image file that we pass in as disk and have the distro boot do its magic?
That seems very heavyweight as a sanity check, although I agree it is useful.
It's not really much more heavy weight. The "image file" could simply contain your hello world binary. But with this we don't just verify whether "bootefi" works, but also whether the default boot path works ok.
I don't think I understand what you mean by 'image file'. Is it something other than the .efi file? Do you mean a disk image?
Yes. For reasonable test coverage, we should also verify that the distro defaults wrote a sane boot script that automatically searches for a default EFI binary in /efi/boot/bootx86.efi on the first partition of all devices and runs it.
So if we just provide an SD card image or hard disk image to QEMU which contains a hello world .efi binary as that default boot file, we don't only test whether the "bootefi" command works, but also whether the distro boot script works.
Here I am just making sure that EFI programs can start, print output and exit. It is a test that we can easily run without a lot of overhead, much less than a full distro boot.
Again, I don't think it's much more overhead and I do believe it gives us much cleaner separation between responsibilities of code (tests go where tests are).
You are talking about a functional test, something that tests things end to end. I prefer to at least start with a smaller test. Granted it takes a little more work but it means there are fewer things to hunt through when something goes wrong.
Yes, I personally find unit tests terribly annoying and unproductive and functional tests very helpful :). And in this case, the effort to write it is about the same for both, just that the functional test actually tells you that things work or don't work at the end of the day.
With a code base like U-Boot, a simple functional test like the above plus git bisect should get you to an offending patch very quickly.
Alex
Hi Alex,
On 7 November 2016 at 09:32, Alexander Graf agraf@suse.de wrote:
On 07/11/2016 10:46, Simon Glass wrote:
Hi Alex,
On 19 October 2016 at 01:09, Alexander Graf agraf@suse.de wrote:
On 18/10/2016 22:37, Simon Glass wrote:
Hi Alex,
On 18 October 2016 at 01:14, Alexander Graf agraf@suse.de wrote:
On 10/18/2016 04:29 AM, Simon Glass wrote:
It is useful to have a basic sanity check for EFI loader support. Add a 'bootefi hello' command which loads HelloWord.efi and runs it under U-Boot.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Include a link to the program instead of adding it to the tree
So, uh, where is the link?
I put it in the README (see the arm patch).
I'm really not convinced this command buys us anything yet. I do agree that we want automated testing - but can't we get that using QEMU and a downloadable image file that we pass in as disk and have the distro boot do its magic?
That seems very heavyweight as a sanity check, although I agree it is useful.
It's not really much more heavy weight. The "image file" could simply contain your hello world binary. But with this we don't just verify whether "bootefi" works, but also whether the default boot path works ok.
I don't think I understand what you mean by 'image file'. Is it something other than the .efi file? Do you mean a disk image?
Yes. For reasonable test coverage, we should also verify that the distro defaults wrote a sane boot script that automatically searches for a default EFI binary in /efi/boot/bootx86.efi on the first partition of all devices and runs it.
So if we just provide an SD card image or hard disk image to QEMU which contains a hello world .efi binary as that default boot file, we don't only test whether the "bootefi" command works, but also whether the distro boot script works.
That's right.
Here I am just making sure that EFI programs can start, print output and exit. It is a test that we can easily run without a lot of overhead, much less than a full distro boot.
Again, I don't think it's much more overhead and I do believe it gives us much cleaner separation between responsibilities of code (tests go where tests are).
You are talking about a functional test, something that tests things end to end. I prefer to at least start with a smaller test. Granted it takes a little more work but it means there are fewer things to hunt through when something goes wrong.
Yes, I personally find unit tests terribly annoying and unproductive and functional tests very helpful :). And in this case, the effort to write it is about the same for both, just that the functional test actually tells you that things work or don't work at the end of the day.
With a code base like U-Boot, a simple functional test like the above plus git bisect should get you to an offending patch very quickly.
This is not a unit test - in fact the EFI stuff has no unit tests. I suppose if we are trying to find a name this is a small functional test since it exercises the general functionality.
I am much keener on small tests than large ones for finding simple bugs. Of course you can generally bisect to find a bug, but the more layers of software you need to look for the harder this is.
We could definitely use a pytest which checks an EFI boot into an image, but I don't think this obviates the need for a smaller targeted test like this one.
Regards, Simon
Am 11.11.2016 um 17:17 schrieb Simon Glass sjg@chromium.org:
Hi Alex,
On 7 November 2016 at 09:32, Alexander Graf agraf@suse.de wrote:
On 07/11/2016 10:46, Simon Glass wrote:
Hi Alex,
On 19 October 2016 at 01:09, Alexander Graf agraf@suse.de wrote:
On 18/10/2016 22:37, Simon Glass wrote:
Hi Alex,
On 18 October 2016 at 01:14, Alexander Graf agraf@suse.de wrote:
> On 10/18/2016 04:29 AM, Simon Glass wrote: > > > It is useful to have a basic sanity check for EFI loader support. Add > a > 'bootefi hello' command which loads HelloWord.efi and runs it under > U-Boot. > > Signed-off-by: Simon Glass sjg@chromium.org > --- > > Changes in v3: > - Include a link to the program instead of adding it to the tree
So, uh, where is the link?
I put it in the README (see the arm patch).
I'm really not convinced this command buys us anything yet. I do agree that we want automated testing - but can't we get that using QEMU and a downloadable image file that we pass in as disk and have the distro boot do its magic?
That seems very heavyweight as a sanity check, although I agree it is useful.
It's not really much more heavy weight. The "image file" could simply contain your hello world binary. But with this we don't just verify whether "bootefi" works, but also whether the default boot path works ok.
I don't think I understand what you mean by 'image file'. Is it something other than the .efi file? Do you mean a disk image?
Yes. For reasonable test coverage, we should also verify that the distro defaults wrote a sane boot script that automatically searches for a default EFI binary in /efi/boot/bootx86.efi on the first partition of all devices and runs it.
So if we just provide an SD card image or hard disk image to QEMU which contains a hello world .efi binary as that default boot file, we don't only test whether the "bootefi" command works, but also whether the distro boot script works.
That's right.
Here I am just making sure that EFI programs can start, print output and exit. It is a test that we can easily run without a lot of overhead, much less than a full distro boot.
Again, I don't think it's much more overhead and I do believe it gives us much cleaner separation between responsibilities of code (tests go where tests are).
You are talking about a functional test, something that tests things end to end. I prefer to at least start with a smaller test. Granted it takes a little more work but it means there are fewer things to hunt through when something goes wrong.
Yes, I personally find unit tests terribly annoying and unproductive and functional tests very helpful :). And in this case, the effort to write it is about the same for both, just that the functional test actually tells you that things work or don't work at the end of the day.
With a code base like U-Boot, a simple functional test like the above plus git bisect should get you to an offending patch very quickly.
This is not a unit test - in fact the EFI stuff has no unit tests. I suppose if we are trying to find a name this is a small functional test since it exercises the general functionality.
I am much keener on small tests than large ones for finding simple bugs. Of course you can generally bisect to find a bug, but the more layers of software you need to look for the harder this is.
We could definitely use a pytest which checks an EFI boot into an image, but I don't think this obviates the need for a smaller targeted test like this one.
I think arguing over this is moot :). More tests is usually a good thing, so whoever gets to write them gets to push them ;). As long as the licenses are sound at least.
Alex
Hi Alex,
On 11 November 2016 at 23:23, Alexander Graf agraf@suse.de wrote:
Am 11.11.2016 um 17:17 schrieb Simon Glass sjg@chromium.org:
Hi Alex,
On 7 November 2016 at 09:32, Alexander Graf agraf@suse.de wrote:
On 07/11/2016 10:46, Simon Glass wrote:
Hi Alex,
On 19 October 2016 at 01:09, Alexander Graf agraf@suse.de wrote:
On 18/10/2016 22:37, Simon Glass wrote:
Hi Alex,
> On 18 October 2016 at 01:14, Alexander Graf agraf@suse.de wrote: > >> On 10/18/2016 04:29 AM, Simon Glass wrote: >> >> >> It is useful to have a basic sanity check for EFI loader support. Add >> a >> 'bootefi hello' command which loads HelloWord.efi and runs it under >> U-Boot. >> >> Signed-off-by: Simon Glass sjg@chromium.org >> --- >> >> Changes in v3: >> - Include a link to the program instead of adding it to the tree > > > > So, uh, where is the link?
I put it in the README (see the arm patch).
> > I'm really not convinced this command buys us anything yet. I do agree > that > we want automated testing - but can't we get that using QEMU and a > downloadable image file that we pass in as disk and have the distro > boot do > its magic?
That seems very heavyweight as a sanity check, although I agree it is useful.
It's not really much more heavy weight. The "image file" could simply contain your hello world binary. But with this we don't just verify whether "bootefi" works, but also whether the default boot path works ok.
I don't think I understand what you mean by 'image file'. Is it something other than the .efi file? Do you mean a disk image?
Yes. For reasonable test coverage, we should also verify that the distro defaults wrote a sane boot script that automatically searches for a default EFI binary in /efi/boot/bootx86.efi on the first partition of all devices and runs it.
So if we just provide an SD card image or hard disk image to QEMU which contains a hello world .efi binary as that default boot file, we don't only test whether the "bootefi" command works, but also whether the distro boot script works.
That's right.
Here I am just making sure that EFI programs can start, print output and exit. It is a test that we can easily run without a lot of overhead, much less than a full distro boot.
Again, I don't think it's much more overhead and I do believe it gives us much cleaner separation between responsibilities of code (tests go where tests are).
You are talking about a functional test, something that tests things end to end. I prefer to at least start with a smaller test. Granted it takes a little more work but it means there are fewer things to hunt through when something goes wrong.
Yes, I personally find unit tests terribly annoying and unproductive and functional tests very helpful :). And in this case, the effort to write it is about the same for both, just that the functional test actually tells you that things work or don't work at the end of the day.
With a code base like U-Boot, a simple functional test like the above plus git bisect should get you to an offending patch very quickly.
This is not a unit test - in fact the EFI stuff has no unit tests. I suppose if we are trying to find a name this is a small functional test since it exercises the general functionality.
I am much keener on small tests than large ones for finding simple bugs. Of course you can generally bisect to find a bug, but the more layers of software you need to look for the harder this is.
We could definitely use a pytest which checks an EFI boot into an image, but I don't think this obviates the need for a smaller targeted test like this one.
I think arguing over this is moot :). More tests is usually a good thing, so whoever gets to write them gets to push them ;). As long as the licenses are sound at least.
OK good, well please can you review this at some point? Also, are you planning to write the 'larger' test? How do you test this all in suse?
Regards, Simon
On 14/11/2016 21:44, Simon Glass wrote:
Hi Alex,
On 11 November 2016 at 23:23, Alexander Graf agraf@suse.de wrote:
Am 11.11.2016 um 17:17 schrieb Simon Glass sjg@chromium.org:
Hi Alex,
On 7 November 2016 at 09:32, Alexander Graf agraf@suse.de wrote:
On 07/11/2016 10:46, Simon Glass wrote:
Hi Alex,
On 19 October 2016 at 01:09, Alexander Graf agraf@suse.de wrote:
> On 18/10/2016 22:37, Simon Glass wrote: > > Hi Alex, > >> On 18 October 2016 at 01:14, Alexander Graf agraf@suse.de wrote: >> >>> On 10/18/2016 04:29 AM, Simon Glass wrote: >>> >>> >>> It is useful to have a basic sanity check for EFI loader support. Add >>> a >>> 'bootefi hello' command which loads HelloWord.efi and runs it under >>> U-Boot. >>> >>> Signed-off-by: Simon Glass sjg@chromium.org >>> --- >>> >>> Changes in v3: >>> - Include a link to the program instead of adding it to the tree >> >> >> >> So, uh, where is the link? > > > I put it in the README (see the arm patch). > >> >> I'm really not convinced this command buys us anything yet. I do agree >> that >> we want automated testing - but can't we get that using QEMU and a >> downloadable image file that we pass in as disk and have the distro >> boot do >> its magic? > > > That seems very heavyweight as a sanity check, although I agree it is > useful.
It's not really much more heavy weight. The "image file" could simply contain your hello world binary. But with this we don't just verify whether "bootefi" works, but also whether the default boot path works ok.
I don't think I understand what you mean by 'image file'. Is it something other than the .efi file? Do you mean a disk image?
Yes. For reasonable test coverage, we should also verify that the distro defaults wrote a sane boot script that automatically searches for a default EFI binary in /efi/boot/bootx86.efi on the first partition of all devices and runs it.
So if we just provide an SD card image or hard disk image to QEMU which contains a hello world .efi binary as that default boot file, we don't only test whether the "bootefi" command works, but also whether the distro boot script works.
That's right.
> Here I am just making sure that EFI programs can start, print output > and exit. It is a test that we can easily run without a lot of > overhead, much less than a full distro boot.
Again, I don't think it's much more overhead and I do believe it gives us much cleaner separation between responsibilities of code (tests go where tests are).
You are talking about a functional test, something that tests things end to end. I prefer to at least start with a smaller test. Granted it takes a little more work but it means there are fewer things to hunt through when something goes wrong.
Yes, I personally find unit tests terribly annoying and unproductive and functional tests very helpful :). And in this case, the effort to write it is about the same for both, just that the functional test actually tells you that things work or don't work at the end of the day.
With a code base like U-Boot, a simple functional test like the above plus git bisect should get you to an offending patch very quickly.
This is not a unit test - in fact the EFI stuff has no unit tests. I suppose if we are trying to find a name this is a small functional test since it exercises the general functionality.
I am much keener on small tests than large ones for finding simple bugs. Of course you can generally bisect to find a bug, but the more layers of software you need to look for the harder this is.
We could definitely use a pytest which checks an EFI boot into an image, but I don't think this obviates the need for a smaller targeted test like this one.
I think arguing over this is moot :). More tests is usually a good thing, so whoever gets to write them gets to push them ;). As long as the licenses are sound at least.
OK good, well please can you review this at some point?
Review what exactly?
Also, are you planning to write the 'larger' test? How do you test this all in suse?
Planning yes, but I'm very good at not writing tests :).
Currently I'm testing this all in suse by running systems which rely on the code to work.
Alex
Hi Alex,
On 14 November 2016 at 13:46, Alexander Graf agraf@suse.de wrote:
On 14/11/2016 21:44, Simon Glass wrote:
Hi Alex,
On 11 November 2016 at 23:23, Alexander Graf agraf@suse.de wrote:
Am 11.11.2016 um 17:17 schrieb Simon Glass sjg@chromium.org:
Hi Alex,
On 7 November 2016 at 09:32, Alexander Graf agraf@suse.de wrote:
On 07/11/2016 10:46, Simon Glass wrote:
Hi Alex,
> On 19 October 2016 at 01:09, Alexander Graf agraf@suse.de wrote: > > > >> On 18/10/2016 22:37, Simon Glass wrote: >> >> Hi Alex, >> >>> On 18 October 2016 at 01:14, Alexander Graf agraf@suse.de wrote: >>> >>>> On 10/18/2016 04:29 AM, Simon Glass wrote: >>>> >>>> >>>> It is useful to have a basic sanity check for EFI loader support. >>>> Add >>>> a >>>> 'bootefi hello' command which loads HelloWord.efi and runs it >>>> under >>>> U-Boot. >>>> >>>> Signed-off-by: Simon Glass sjg@chromium.org >>>> --- >>>> >>>> Changes in v3: >>>> - Include a link to the program instead of adding it to the tree >>> >>> >>> >>> >>> So, uh, where is the link? >> >> >> >> I put it in the README (see the arm patch). >> >>> >>> I'm really not convinced this command buys us anything yet. I do >>> agree >>> that >>> we want automated testing - but can't we get that using QEMU and a >>> downloadable image file that we pass in as disk and have the distro >>> boot do >>> its magic? >> >> >> >> That seems very heavyweight as a sanity check, although I agree it >> is >> useful. > > > > It's not really much more heavy weight. The "image file" could simply > contain your hello world binary. But with this we don't just verify > whether "bootefi" works, but also whether the default boot path works > ok.
I don't think I understand what you mean by 'image file'. Is it something other than the .efi file? Do you mean a disk image?
Yes. For reasonable test coverage, we should also verify that the distro defaults wrote a sane boot script that automatically searches for a default EFI binary in /efi/boot/bootx86.efi on the first partition of all devices and runs it.
So if we just provide an SD card image or hard disk image to QEMU which contains a hello world .efi binary as that default boot file, we don't only test whether the "bootefi" command works, but also whether the distro boot script works.
That's right.
> >> Here I am just making sure that EFI programs can start, print output >> and exit. It is a test that we can easily run without a lot of >> overhead, much less than a full distro boot. > > > > Again, I don't think it's much more overhead and I do believe it > gives > us much cleaner separation between responsibilities of code (tests go > where tests are).
You are talking about a functional test, something that tests things end to end. I prefer to at least start with a smaller test. Granted it takes a little more work but it means there are fewer things to hunt through when something goes wrong.
Yes, I personally find unit tests terribly annoying and unproductive and functional tests very helpful :). And in this case, the effort to write it is about the same for both, just that the functional test actually tells you that things work or don't work at the end of the day.
With a code base like U-Boot, a simple functional test like the above plus git bisect should get you to an offending patch very quickly.
This is not a unit test - in fact the EFI stuff has no unit tests. I suppose if we are trying to find a name this is a small functional test since it exercises the general functionality.
I am much keener on small tests than large ones for finding simple bugs. Of course you can generally bisect to find a bug, but the more layers of software you need to look for the harder this is.
We could definitely use a pytest which checks an EFI boot into an image, but I don't think this obviates the need for a smaller targeted test like this one.
I think arguing over this is moot :). More tests is usually a good thing, so whoever gets to write them gets to push them ;). As long as the licenses are sound at least.
OK good, well please can you review this at some point?
Review what exactly?
I mean the patches. There should be ~14 in your queue.
Also, are you planning to write the 'larger' test? How do you test this all in suse?
Planning yes, but I'm very good at not writing tests :).
Currently I'm testing this all in suse by running systems which rely on the code to work.
OK I see.
Regards, Simon
On 14/11/2016 21:58, Simon Glass wrote:
Hi Alex,
On 14 November 2016 at 13:46, Alexander Graf agraf@suse.de wrote:
On 14/11/2016 21:44, Simon Glass wrote:
Hi Alex,
On 11 November 2016 at 23:23, Alexander Graf agraf@suse.de wrote:
Am 11.11.2016 um 17:17 schrieb Simon Glass sjg@chromium.org:
Hi Alex,
On 7 November 2016 at 09:32, Alexander Graf agraf@suse.de wrote:
> On 07/11/2016 10:46, Simon Glass wrote: > > Hi Alex, > >> On 19 October 2016 at 01:09, Alexander Graf agraf@suse.de wrote: >> >> >> >>> On 18/10/2016 22:37, Simon Glass wrote: >>> >>> Hi Alex, >>> >>>> On 18 October 2016 at 01:14, Alexander Graf agraf@suse.de wrote: >>>> >>>>> On 10/18/2016 04:29 AM, Simon Glass wrote: >>>>> >>>>> >>>>> It is useful to have a basic sanity check for EFI loader support. >>>>> Add >>>>> a >>>>> 'bootefi hello' command which loads HelloWord.efi and runs it >>>>> under >>>>> U-Boot. >>>>> >>>>> Signed-off-by: Simon Glass sjg@chromium.org >>>>> --- >>>>> >>>>> Changes in v3: >>>>> - Include a link to the program instead of adding it to the tree >>>> >>>> >>>> >>>> >>>> So, uh, where is the link? >>> >>> >>> >>> I put it in the README (see the arm patch). >>> >>>> >>>> I'm really not convinced this command buys us anything yet. I do >>>> agree >>>> that >>>> we want automated testing - but can't we get that using QEMU and a >>>> downloadable image file that we pass in as disk and have the distro >>>> boot do >>>> its magic? >>> >>> >>> >>> That seems very heavyweight as a sanity check, although I agree it >>> is >>> useful. >> >> >> >> It's not really much more heavy weight. The "image file" could simply >> contain your hello world binary. But with this we don't just verify >> whether "bootefi" works, but also whether the default boot path works >> ok. > > > > I don't think I understand what you mean by 'image file'. Is it > something other than the .efi file? Do you mean a disk image?
Yes. For reasonable test coverage, we should also verify that the distro defaults wrote a sane boot script that automatically searches for a default EFI binary in /efi/boot/bootx86.efi on the first partition of all devices and runs it.
So if we just provide an SD card image or hard disk image to QEMU which contains a hello world .efi binary as that default boot file, we don't only test whether the "bootefi" command works, but also whether the distro boot script works.
That's right.
> >> >>> Here I am just making sure that EFI programs can start, print output >>> and exit. It is a test that we can easily run without a lot of >>> overhead, much less than a full distro boot. >> >> >> >> Again, I don't think it's much more overhead and I do believe it >> gives >> us much cleaner separation between responsibilities of code (tests go >> where tests are). > > > > You are talking about a functional test, something that tests things > end to end. I prefer to at least start with a smaller test. Granted it > takes a little more work but it means there are fewer things to hunt > through when something goes wrong.
Yes, I personally find unit tests terribly annoying and unproductive and functional tests very helpful :). And in this case, the effort to write it is about the same for both, just that the functional test actually tells you that things work or don't work at the end of the day.
With a code base like U-Boot, a simple functional test like the above plus git bisect should get you to an offending patch very quickly.
This is not a unit test - in fact the EFI stuff has no unit tests. I suppose if we are trying to find a name this is a small functional test since it exercises the general functionality.
I am much keener on small tests than large ones for finding simple bugs. Of course you can generally bisect to find a bug, but the more layers of software you need to look for the harder this is.
We could definitely use a pytest which checks an EFI boot into an image, but I don't think this obviates the need for a smaller targeted test like this one.
I think arguing over this is moot :). More tests is usually a good thing, so whoever gets to write them gets to push them ;). As long as the licenses are sound at least.
OK good, well please can you review this at some point?
Review what exactly?
I mean the patches. There should be ~14 in your queue.
Interesting. I see them on patchwork, but neither in my U-Boot folder, my inbox nor my spam folder. I wonder what happened there.
Alex
Hi Alex,
On 14 November 2016 at 14:24, Alexander Graf agraf@suse.de wrote:
On 14/11/2016 21:58, Simon Glass wrote:
Hi Alex,
On 14 November 2016 at 13:46, Alexander Graf agraf@suse.de wrote:
On 14/11/2016 21:44, Simon Glass wrote:
Hi Alex,
On 11 November 2016 at 23:23, Alexander Graf agraf@suse.de wrote:
Am 11.11.2016 um 17:17 schrieb Simon Glass sjg@chromium.org:
Hi Alex,
> On 7 November 2016 at 09:32, Alexander Graf agraf@suse.de wrote: > > >> On 07/11/2016 10:46, Simon Glass wrote: >> >> Hi Alex, >> >>> On 19 October 2016 at 01:09, Alexander Graf agraf@suse.de wrote: >>> >>> >>> >>>> On 18/10/2016 22:37, Simon Glass wrote: >>>> >>>> Hi Alex, >>>> >>>>> On 18 October 2016 at 01:14, Alexander Graf agraf@suse.de >>>>> wrote: >>>>> >>>>>> On 10/18/2016 04:29 AM, Simon Glass wrote: >>>>>> >>>>>> >>>>>> It is useful to have a basic sanity check for EFI loader >>>>>> support. >>>>>> Add >>>>>> a >>>>>> 'bootefi hello' command which loads HelloWord.efi and runs it >>>>>> under >>>>>> U-Boot. >>>>>> >>>>>> Signed-off-by: Simon Glass sjg@chromium.org >>>>>> --- >>>>>> >>>>>> Changes in v3: >>>>>> - Include a link to the program instead of adding it to the tree >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> So, uh, where is the link? >>>> >>>> >>>> >>>> >>>> I put it in the README (see the arm patch). >>>> >>>>> >>>>> I'm really not convinced this command buys us anything yet. I do >>>>> agree >>>>> that >>>>> we want automated testing - but can't we get that using QEMU and >>>>> a >>>>> downloadable image file that we pass in as disk and have the >>>>> distro >>>>> boot do >>>>> its magic? >>>> >>>> >>>> >>>> >>>> That seems very heavyweight as a sanity check, although I agree it >>>> is >>>> useful. >>> >>> >>> >>> >>> It's not really much more heavy weight. The "image file" could >>> simply >>> contain your hello world binary. But with this we don't just verify >>> whether "bootefi" works, but also whether the default boot path >>> works >>> ok. >> >> >> >> >> I don't think I understand what you mean by 'image file'. Is it >> something other than the .efi file? Do you mean a disk image? > > > > > Yes. For reasonable test coverage, we should also verify that the > distro > defaults wrote a sane boot script that automatically searches for a > default > EFI binary in /efi/boot/bootx86.efi on the first partition of all > devices > and runs it. > > So if we just provide an SD card image or hard disk image to QEMU > which > contains a hello world .efi binary as that default boot file, we > don't > only > test whether the "bootefi" command works, but also whether the distro > boot > script works.
That's right.
> >> >>> >>>> Here I am just making sure that EFI programs can start, print >>>> output >>>> and exit. It is a test that we can easily run without a lot of >>>> overhead, much less than a full distro boot. >>> >>> >>> >>> >>> Again, I don't think it's much more overhead and I do believe it >>> gives >>> us much cleaner separation between responsibilities of code (tests >>> go >>> where tests are). >> >> >> >> >> You are talking about a functional test, something that tests things >> end to end. I prefer to at least start with a smaller test. Granted >> it >> takes a little more work but it means there are fewer things to hunt >> through when something goes wrong. > > > > > Yes, I personally find unit tests terribly annoying and unproductive > and > functional tests very helpful :). And in this case, the effort to > write > it > is about the same for both, just that the functional test actually > tells you > that things work or don't work at the end of the day. > > With a code base like U-Boot, a simple functional test like the above > plus > git bisect should get you to an offending patch very quickly.
This is not a unit test - in fact the EFI stuff has no unit tests. I suppose if we are trying to find a name this is a small functional test since it exercises the general functionality.
I am much keener on small tests than large ones for finding simple bugs. Of course you can generally bisect to find a bug, but the more layers of software you need to look for the harder this is.
We could definitely use a pytest which checks an EFI boot into an image, but I don't think this obviates the need for a smaller targeted test like this one.
I think arguing over this is moot :). More tests is usually a good thing, so whoever gets to write them gets to push them ;). As long as the licenses are sound at least.
OK good, well please can you review this at some point?
Review what exactly?
I mean the patches. There should be ~14 in your queue.
Interesting. I see them on patchwork, but neither in my U-Boot folder, my inbox nor my spam folder. I wonder what happened there.
I'm really not sure but I have seen similar things. I will see if I can figure it out.
Regards, Simon
Enable this so that EFI applications (notably grub) can be run under U-Boot on x86 platforms.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: None Changes in v2: - Remove EFI support from README.x86 TODO list
configs/efi-x86_defconfig | 1 + doc/README.x86 | 1 - lib/efi_loader/Kconfig | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/configs/efi-x86_defconfig b/configs/efi-x86_defconfig index b31c73b..1fe6142 100644 --- a/configs/efi-x86_defconfig +++ b/configs/efi-x86_defconfig @@ -34,3 +34,4 @@ CONFIG_USB=y CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y CONFIG_EFI=y +# CONFIG_EFI_LOADER is not set diff --git a/doc/README.x86 b/doc/README.x86 index 6799559..a38cc1b 100644 --- a/doc/README.x86 +++ b/doc/README.x86 @@ -1077,7 +1077,6 @@ TODO List --------- - Audio - Chrome OS verified boot -- Support for CONFIG_EFI_LOADER - Building U-Boot to run in 64-bit mode
References diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 37a0dd6..8e7e8a5 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -1,6 +1,6 @@ config EFI_LOADER bool "Support running EFI Applications in U-Boot" - depends on (ARM64 || ARM) && OF_LIBFDT + depends on (ARM64 || ARM || X86) && OF_LIBFDT default y help Select this option if you want to run EFI applications (like grub2)
On Tue, Oct 18, 2016 at 10:29 AM, Simon Glass sjg@chromium.org wrote:
Bring in these functions from Linux v4.4. They will be needed for EFI loader support.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Add a parameter to longjmp()
- Drop unused header files
Changes in v2:
- Drop irrelevant comment
- Add a comment about .size
- Drop unnecessary .text directive
- Make longjmp() always cause setjmp() to return 1
arch/x86/cpu/Makefile | 2 +- arch/x86/cpu/setjmp.S | 61 +++++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/setjmp.h | 24 +++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 arch/x86/cpu/setjmp.S create mode 100644 arch/x86/include/asm/setjmp.h
Reviewed-by: Bin Meng bmeng.cn@gmail.com
On Tue, Oct 18, 2016 at 11:02 AM, Bin Meng bmeng.cn@gmail.com wrote:
On Tue, Oct 18, 2016 at 10:29 AM, Simon Glass sjg@chromium.org wrote:
Bring in these functions from Linux v4.4. They will be needed for EFI loader support.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Add a parameter to longjmp()
- Drop unused header files
Changes in v2:
- Drop irrelevant comment
- Add a comment about .size
- Drop unnecessary .text directive
- Make longjmp() always cause setjmp() to return 1
arch/x86/cpu/Makefile | 2 +- arch/x86/cpu/setjmp.S | 61 +++++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/setjmp.h | 24 +++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 arch/x86/cpu/setjmp.S create mode 100644 arch/x86/include/asm/setjmp.h
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Checked patchwork record, looks Alex has applied v2 series patch#2, #3, #6 into his efi-next tree (https://github.com/agraf/u-boot/tree/efi-next)
I will only pick up this v3 patch#1 via u-boot-x86 tree, while leaving other patches in v3 to Alex to handle.
applied to u-boot-x86, thanks!
participants (3)
-
Alexander Graf -
Bin Meng -
Simon Glass