[U-Boot] [PATCH v2 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 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 | 66 +++++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/setjmp.h | 24 ++++++++++++++++ 3 files changed, 91 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..7443274 --- /dev/null +++ b/arch/x86/cpu/setjmp.S @@ -0,0 +1,66 @@ +/* + * 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 + */ + +#include <asm/global_data.h> +#include <asm/msr-index.h> +#include <asm/processor-flags.h> + +#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 1, %eax + 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..deef67e --- /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); + +#endif
This is required for x86 and is also correct for ARM (since it is empty).
Signed-off-by: Simon Glass sjg@chromium.org ---
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 Mon, Sep 26, 2016 at 5:27 AM, Simon Glass sjg@chromium.org wrote:
This is required for x86 and is also correct for ARM (since it is empty).
Signed-off-by: Simon Glass sjg@chromium.org
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
Reviewed-by: Bin Meng bmeng.cn@gmail.com
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
Thanks, applied to
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 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 be6f5e8..798b566 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) {
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
Thanks, applied to
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 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
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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)
I don't understand why this is needed?
#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
Regards, Bin
Hi Bin,
On 26 September 2016 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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)
I don't understand why this is needed?
If building the 64-bit EFI stub, we need to use 64-bit ints for the stub, but 32-bits for the rest of U-Boot. So this header gets used both ways.
#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
Regards, Bin
Regards, Simon
Hi Simon,
On Tue, Sep 27, 2016 at 8:35 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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)
I don't understand why this is needed?
If building the 64-bit EFI stub, we need to use 64-bit ints for the stub, but 32-bits for the rest of U-Boot. So this header gets used both ways.
For 32-bit EFI stub or U-Boot itself, EFI_BITS_PER_LONG is defined as BITS_PER_LONG which is 32.
#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
Regards, Bin
Hi Bin,
On 26 September 2016 at 20:44, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Sep 27, 2016 at 8:35 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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)
I don't understand why this is needed?
If building the 64-bit EFI stub, we need to use 64-bit ints for the stub, but 32-bits for the rest of U-Boot. So this header gets used both ways.
For 32-bit EFI stub or U-Boot itself, EFI_BITS_PER_LONG is defined as BITS_PER_LONG which is 32.
Yes that's right. But in the case of the stub, it can be different from U-Boot itself. This case takes care of that.
#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
Regards, Simon
Hi Simon,
On Wed, Sep 28, 2016 at 1:55 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 20:44, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Sep 27, 2016 at 8:35 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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)
I don't understand why this is needed?
If building the 64-bit EFI stub, we need to use 64-bit ints for the stub, but 32-bits for the rest of U-Boot. So this header gets used both ways.
For 32-bit EFI stub or U-Boot itself, EFI_BITS_PER_LONG is defined as BITS_PER_LONG which is 32.
Yes that's right. But in the case of the stub, it can be different from U-Boot itself. This case takes care of that.
Sorry but I still don't get it. What's broken without this change?
Regards, Bin
Hi Bin,
On 27 September 2016 at 19:23, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Sep 28, 2016 at 1:55 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 20:44, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Sep 27, 2016 at 8:35 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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)
I don't understand why this is needed?
If building the 64-bit EFI stub, we need to use 64-bit ints for the stub, but 32-bits for the rest of U-Boot. So this header gets used both ways.
For 32-bit EFI stub or U-Boot itself, EFI_BITS_PER_LONG is defined as BITS_PER_LONG which is 32.
Yes that's right. But in the case of the stub, it can be different from U-Boot itself. This case takes care of that.
Sorry but I still don't get it. What's broken without this change?
When building U-Boot with CONFIG_EFI_STUB_64BIT enabled, at present EFI_BITS_PER_LONG will be 64.
This is fine for building the stub.
But for building U-Boot, we still want it to be 32.
At present the code overrides EFI_BITS_PER_LONG, setting it to 64 if CONFIG_EFI_STUB_64BIT is enabled.
This means that EFI_LOADER support does not build properly, since it uses 64 instead of 32.
You can try building without this commit to see the result.
Regards, Simon
Hi Simon,
On Wed, Sep 28, 2016 at 10:43 PM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 27 September 2016 at 19:23, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Sep 28, 2016 at 1:55 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 20:44, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Sep 27, 2016 at 8:35 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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)
I don't understand why this is needed?
If building the 64-bit EFI stub, we need to use 64-bit ints for the stub, but 32-bits for the rest of U-Boot. So this header gets used both ways.
For 32-bit EFI stub or U-Boot itself, EFI_BITS_PER_LONG is defined as BITS_PER_LONG which is 32.
Yes that's right. But in the case of the stub, it can be different from U-Boot itself. This case takes care of that.
Sorry but I still don't get it. What's broken without this change?
When building U-Boot with CONFIG_EFI_STUB_64BIT enabled, at present EFI_BITS_PER_LONG will be 64.
This is fine for building the stub.
Yes
But for building U-Boot, we still want it to be 32.
Yes
At present the code overrides EFI_BITS_PER_LONG, setting it to 64 if CONFIG_EFI_STUB_64BIT is enabled.
This means that EFI_LOADER support does not build properly, since it uses 64 instead of 32.
So you want CONFIG_EFI_STUB_64BIT and CONFIG_EFI_LOADER both be defined? I don't think it is a valid configuration.
You can try building without this commit to see the result.
Regards, Bin
Am 29.09.2016 um 05:37 schrieb Bin Meng bmeng.cn@gmail.com:
Hi Simon,
On Wed, Sep 28, 2016 at 10:43 PM, Simon Glass sjg@chromium.org wrote: Hi Bin,
On 27 September 2016 at 19:23, Bin Meng bmeng.cn@gmail.com wrote: Hi Simon,
On Wed, Sep 28, 2016 at 1:55 AM, Simon Glass sjg@chromium.org wrote: Hi Bin,
On 26 September 2016 at 20:44, Bin Meng bmeng.cn@gmail.com wrote: Hi Simon,
On Tue, Sep 27, 2016 at 8:35 AM, Simon Glass sjg@chromium.org wrote: Hi Bin,
> On 26 September 2016 at 02:50, Bin Meng bmeng.cn@gmail.com wrote: > Hi Simon, > >> On Mon, Sep 26, 2016 at 5:27 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 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) > > I don't understand why this is needed?
If building the 64-bit EFI stub, we need to use 64-bit ints for the stub, but 32-bits for the rest of U-Boot. So this header gets used both ways.
For 32-bit EFI stub or U-Boot itself, EFI_BITS_PER_LONG is defined as BITS_PER_LONG which is 32.
Yes that's right. But in the case of the stub, it can be different from U-Boot itself. This case takes care of that.
Sorry but I still don't get it. What's broken without this change?
When building U-Boot with CONFIG_EFI_STUB_64BIT enabled, at present EFI_BITS_PER_LONG will be 64.
This is fine for building the stub.
Yes
But for building U-Boot, we still want it to be 32.
Yes
At present the code overrides EFI_BITS_PER_LONG, setting it to 64 if CONFIG_EFI_STUB_64BIT is enabled.
This means that EFI_LOADER support does not build properly, since it uses 64 instead of 32.
So you want CONFIG_EFI_STUB_64BIT and CONFIG_EFI_LOADER both be defined? I don't think it is a valid configuration.
Why not?
Alex
Hi Alex,
On Thu, Sep 29, 2016 at 1:08 PM, Alexander Graf agraf@suse.de wrote:
Am 29.09.2016 um 05:37 schrieb Bin Meng bmeng.cn@gmail.com:
Hi Simon,
On Wed, Sep 28, 2016 at 10:43 PM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 27 September 2016 at 19:23, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Sep 28, 2016 at 1:55 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 20:44, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Sep 27, 2016 at 8:35 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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)
I don't understand why this is needed?
If building the 64-bit EFI stub, we need to use 64-bit ints for the
stub, but 32-bits for the rest of U-Boot. So this header gets used
both ways.
For 32-bit EFI stub or U-Boot itself, EFI_BITS_PER_LONG is defined as
BITS_PER_LONG which is 32.
Yes that's right. But in the case of the stub, it can be different
from U-Boot itself. This case takes care of that.
Sorry but I still don't get it. What's broken without this change?
When building U-Boot with CONFIG_EFI_STUB_64BIT enabled, at present
EFI_BITS_PER_LONG will be 64.
This is fine for building the stub.
Yes
But for building U-Boot, we still want it to be 32.
Yes
At present the code overrides EFI_BITS_PER_LONG, setting it to 64 if
CONFIG_EFI_STUB_64BIT is enabled.
This means that EFI_LOADER support does not build properly, since it
uses 64 instead of 32.
So you want CONFIG_EFI_STUB_64BIT and CONFIG_EFI_LOADER both be defined? I don't think it is a valid configuration.
Why not?
So the board has a 64-bit UEFI BIOS, and with CONFIG_EFI_STUB_64BIT we build U-Boot as a 64-bit UEFI payload and let the UEFI BIOS boot the payload (U-Boot), then with CONFIG_EFI_LOADER we are trying to provide the UEFI runtime environment within the U-Boot. What value are we looking for? This is asking for troubles.
Regards, Bin
On 09/29/2016 07:37 AM, Bin Meng wrote:
Hi Alex,
On Thu, Sep 29, 2016 at 1:08 PM, Alexander Graf agraf@suse.de wrote:
Am 29.09.2016 um 05:37 schrieb Bin Meng bmeng.cn@gmail.com:
Hi Simon,
On Wed, Sep 28, 2016 at 10:43 PM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 27 September 2016 at 19:23, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Sep 28, 2016 at 1:55 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 20:44, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Sep 27, 2016 at 8:35 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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)
I don't understand why this is needed?
If building the 64-bit EFI stub, we need to use 64-bit ints for the
stub, but 32-bits for the rest of U-Boot. So this header gets used
both ways.
For 32-bit EFI stub or U-Boot itself, EFI_BITS_PER_LONG is defined as
BITS_PER_LONG which is 32.
Yes that's right. But in the case of the stub, it can be different
from U-Boot itself. This case takes care of that.
Sorry but I still don't get it. What's broken without this change?
When building U-Boot with CONFIG_EFI_STUB_64BIT enabled, at present
EFI_BITS_PER_LONG will be 64.
This is fine for building the stub.
Yes
But for building U-Boot, we still want it to be 32.
Yes
At present the code overrides EFI_BITS_PER_LONG, setting it to 64 if
CONFIG_EFI_STUB_64BIT is enabled.
This means that EFI_LOADER support does not build properly, since it
uses 64 instead of 32.
So you want CONFIG_EFI_STUB_64BIT and CONFIG_EFI_LOADER both be defined? I don't think it is a valid configuration.
Why not?
So the board has a 64-bit UEFI BIOS, and with CONFIG_EFI_STUB_64BIT we build U-Boot as a 64-bit UEFI payload and let the UEFI BIOS boot the payload (U-Boot), then with CONFIG_EFI_LOADER we are trying to provide the UEFI runtime environment within the U-Boot. What value are we looking for? This is asking for troubles.
Why is this asking for trouble? The inner uefi payload has no idea that the outer uefi firmware exists. It only ever talks to u-boot. I would argue the other way around: If we can't make it work, we have a layering problem.
Alex
Hi Alex,
On Thu, Sep 29, 2016 at 3:13 PM, Alexander Graf agraf@suse.de wrote:
On 09/29/2016 07:37 AM, Bin Meng wrote:
Hi Alex,
On Thu, Sep 29, 2016 at 1:08 PM, Alexander Graf agraf@suse.de wrote:
Am 29.09.2016 um 05:37 schrieb Bin Meng bmeng.cn@gmail.com:
Hi Simon,
On Wed, Sep 28, 2016 at 10:43 PM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 27 September 2016 at 19:23, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Sep 28, 2016 at 1:55 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 20:44, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Sep 27, 2016 at 8:35 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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)
I don't understand why this is needed?
If building the 64-bit EFI stub, we need to use 64-bit ints for the
stub, but 32-bits for the rest of U-Boot. So this header gets used
both ways.
For 32-bit EFI stub or U-Boot itself, EFI_BITS_PER_LONG is defined as
BITS_PER_LONG which is 32.
Yes that's right. But in the case of the stub, it can be different
from U-Boot itself. This case takes care of that.
Sorry but I still don't get it. What's broken without this change?
When building U-Boot with CONFIG_EFI_STUB_64BIT enabled, at present
EFI_BITS_PER_LONG will be 64.
This is fine for building the stub.
Yes
But for building U-Boot, we still want it to be 32.
Yes
At present the code overrides EFI_BITS_PER_LONG, setting it to 64 if
CONFIG_EFI_STUB_64BIT is enabled.
This means that EFI_LOADER support does not build properly, since it
uses 64 instead of 32.
So you want CONFIG_EFI_STUB_64BIT and CONFIG_EFI_LOADER both be defined? I don't think it is a valid configuration.
Why not?
So the board has a 64-bit UEFI BIOS, and with CONFIG_EFI_STUB_64BIT we build U-Boot as a 64-bit UEFI payload and let the UEFI BIOS boot the payload (U-Boot), then with CONFIG_EFI_LOADER we are trying to provide the UEFI runtime environment within the U-Boot. What value are we looking for? This is asking for troubles.
Why is this asking for trouble? The inner uefi payload has no idea that the outer uefi firmware exists. It only ever talks to u-boot. I would argue the other way around: If we can't make it work, we have a layering problem.
This shows no value to me. In the end, providing EFI loader in the U-Boot is to load some EFI apps. But this can be done from the original UEFI BIOS without the need to have the middle-stage U-Boot payload. What layering problem do we want to fix here? Are you saying testing EFI loader in the U-Boot is not enough, so that we should support such configuration for additional testing?
Regards, Bin
On 09/29/2016 09:28 AM, Bin Meng wrote:
Hi Alex,
On Thu, Sep 29, 2016 at 3:13 PM, Alexander Graf agraf@suse.de wrote:
On 09/29/2016 07:37 AM, Bin Meng wrote:
Hi Alex,
On Thu, Sep 29, 2016 at 1:08 PM, Alexander Graf agraf@suse.de wrote:
Am 29.09.2016 um 05:37 schrieb Bin Meng bmeng.cn@gmail.com:
Hi Simon,
On Wed, Sep 28, 2016 at 10:43 PM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 27 September 2016 at 19:23, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Sep 28, 2016 at 1:55 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 20:44, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Sep 27, 2016 at 8:35 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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)
I don't understand why this is needed?
If building the 64-bit EFI stub, we need to use 64-bit ints for the
stub, but 32-bits for the rest of U-Boot. So this header gets used
both ways.
For 32-bit EFI stub or U-Boot itself, EFI_BITS_PER_LONG is defined as
BITS_PER_LONG which is 32.
Yes that's right. But in the case of the stub, it can be different
from U-Boot itself. This case takes care of that.
Sorry but I still don't get it. What's broken without this change?
When building U-Boot with CONFIG_EFI_STUB_64BIT enabled, at present
EFI_BITS_PER_LONG will be 64.
This is fine for building the stub.
Yes
But for building U-Boot, we still want it to be 32.
Yes
At present the code overrides EFI_BITS_PER_LONG, setting it to 64 if
CONFIG_EFI_STUB_64BIT is enabled.
This means that EFI_LOADER support does not build properly, since it
uses 64 instead of 32.
So you want CONFIG_EFI_STUB_64BIT and CONFIG_EFI_LOADER both be defined? I don't think it is a valid configuration.
Why not?
So the board has a 64-bit UEFI BIOS, and with CONFIG_EFI_STUB_64BIT we build U-Boot as a 64-bit UEFI payload and let the UEFI BIOS boot the payload (U-Boot), then with CONFIG_EFI_LOADER we are trying to provide the UEFI runtime environment within the U-Boot. What value are we looking for? This is asking for troubles.
Why is this asking for trouble? The inner uefi payload has no idea that the outer uefi firmware exists. It only ever talks to u-boot. I would argue the other way around: If we can't make it work, we have a layering problem.
This shows no value to me. In the end, providing EFI loader in the U-Boot is to load some EFI apps. But this can be done from the original UEFI BIOS without the need to have the middle-stage U-Boot payload.
You could say the same about running U-Boot on EFI. You can as well just load your payload from the original uEFI firmware :).
What layering problem do we want to fix here? Are you saying testing EFI loader in the U-Boot is not enough, so that we should support such configuration for additional testing?
I'm saying that I don't see anything that speaks against it working, so why should we disable it? At least for prototyping it's a convenient option to have and in general divergence is a bad thing.
Alex
Hi Alex,
On Thu, Sep 29, 2016 at 4:06 PM, Alexander Graf agraf@suse.de wrote:
On 09/29/2016 09:28 AM, Bin Meng wrote:
Hi Alex,
On Thu, Sep 29, 2016 at 3:13 PM, Alexander Graf agraf@suse.de wrote:
On 09/29/2016 07:37 AM, Bin Meng wrote:
Hi Alex,
On Thu, Sep 29, 2016 at 1:08 PM, Alexander Graf agraf@suse.de wrote:
Am 29.09.2016 um 05:37 schrieb Bin Meng bmeng.cn@gmail.com:
Hi Simon,
On Wed, Sep 28, 2016 at 10:43 PM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 27 September 2016 at 19:23, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Sep 28, 2016 at 1:55 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 20:44, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Sep 27, 2016 at 8:35 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 September 2016 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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)
I don't understand why this is needed?
If building the 64-bit EFI stub, we need to use 64-bit ints for the
stub, but 32-bits for the rest of U-Boot. So this header gets used
both ways.
For 32-bit EFI stub or U-Boot itself, EFI_BITS_PER_LONG is defined as
BITS_PER_LONG which is 32.
Yes that's right. But in the case of the stub, it can be different
from U-Boot itself. This case takes care of that.
Sorry but I still don't get it. What's broken without this change?
When building U-Boot with CONFIG_EFI_STUB_64BIT enabled, at present
EFI_BITS_PER_LONG will be 64.
This is fine for building the stub.
Yes
But for building U-Boot, we still want it to be 32.
Yes
At present the code overrides EFI_BITS_PER_LONG, setting it to 64 if
CONFIG_EFI_STUB_64BIT is enabled.
This means that EFI_LOADER support does not build properly, since it
uses 64 instead of 32.
So you want CONFIG_EFI_STUB_64BIT and CONFIG_EFI_LOADER both be defined? I don't think it is a valid configuration.
Why not?
So the board has a 64-bit UEFI BIOS, and with CONFIG_EFI_STUB_64BIT we build U-Boot as a 64-bit UEFI payload and let the UEFI BIOS boot the payload (U-Boot), then with CONFIG_EFI_LOADER we are trying to provide the UEFI runtime environment within the U-Boot. What value are we looking for? This is asking for troubles.
Why is this asking for trouble? The inner uefi payload has no idea that the outer uefi firmware exists. It only ever talks to u-boot. I would argue the other way around: If we can't make it work, we have a layering problem.
This shows no value to me. In the end, providing EFI loader in the U-Boot is to load some EFI apps. But this can be done from the original UEFI BIOS without the need to have the middle-stage U-Boot payload.
You could say the same about running U-Boot on EFI. You can as well just load your payload from the original uEFI firmware :).
Wait, they are different things. EFI loader in U-Boot assumes U-Boot is the bootloader and we want a EFI runtime environment to load EFI payload without the need to have a full UEFI BIOS. I see value in supporting EFI loader in U-Boot.
What layering problem do we want to fix here? Are you saying testing EFI loader in the U-Boot is not enough, so that we should support such configuration for additional testing?
I'm saying that I don't see anything that speaks against it working, so why should we disable it? At least for prototyping it's a convenient option to have and in general divergence is a bad thing.
But this one, value-wise, no, but yes, technically we can make such configuration work :)
Regards, Bin
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 v2: None
arch/arm/lib/HelloWorld32.efi | Bin 0 -> 11712 bytes arch/arm/lib/Makefile | 7 +++++++ cmd/Kconfig | 10 ++++++++++ cmd/bootefi.c | 26 ++++++++++++++++++++------ include/asm-generic/sections.h | 2 ++ scripts/Makefile.lib | 19 +++++++++++++++++++ 6 files changed, 58 insertions(+), 6 deletions(-) create mode 100644 arch/arm/lib/HelloWorld32.efi
diff --git a/arch/arm/lib/HelloWorld32.efi b/arch/arm/lib/HelloWorld32.efi new file mode 100644 index 0000000000000000000000000000000000000000..7708451c04a99752ed468842090a9b3f3506090e GIT binary patch literal 11712 zcmcI~3sjS5p7-<S0wDwmH=*L?4R{Gg<qd&=78L@##G(ZOFK4H-H6;;JLxLf+XnUrh zo%wn?dKQd3g6NpBGrQfLne)MH{XX3(2Re54%<R@(Xzd)go!!0(b+y<t6Q^`YE1La1 zZxTesX~(l0{lCxce}DeZ|G6Bt{RMgCU)Lj3-X*N7BIFm0^lk@g0$IQ$GK7rg(R$>k zkr2WH1YM-fqM&g9Z**-0kM=9?AB?uu59yz;dvpzYq01EHo|_Nh5&PD8Y@Hs^QX8pV zv2rdumXJN`tQL<2odN~u5G&D~{gr`yo!jPcHKB|8EvmCv&SDV)Jky-!*Q%;&+Rn-O z>=qTDHuP%CNz;P&w7iBV!IY4O*P?7g87J1B4RL(iFk@~Temk3x#|Y`7lq%qKhvM+< z4Ecu0y3`Lyup?AAR2zgkL3c<i)^fKL0HZ0CWBw;i&J!eP2}Puw5-GniOSyZNaz~Uh zU8LkgY35&R(m+`m)$3MB29BFTgF|FZI%GaROIZ@7jN6g}jJ%NC{EkNc#PIbt=(FN{ z+b|g!T||f<V1pfP)g;KAC#2+Dd`)&|%}`toS-zy@Cnnjuax+s%g6Be^YlJNs?gQ@2 zfRlg)Z%7B*szJ%#g_3jd;5O}^@bpxe8`8DpUPv(;G$~Jnr*DMS+t#4}dbB^$yk3)p z{*S_{Z4>)+82K<dA~SE+C@}JFm`WrgA0r-q1$k>06Y?qQTTos{`AwASbMafhyDt~i zZ-P1(@?_?GjSSTP0BZGc5P186_aR{IC&BVKK8f$)!_z+sYll`|SZ1!!ECcT8aCrI@ z=s$~>y}!s@smVs47k$Towc<jCxk{6Xo>#+}HOpF<;+IGXJo?m^GVtjJ#_|hE<|jlO z4}=SNmS4|TVEo{haeUE`c4%J<o#Vdhm6O7AEcz?I<oMz1!EQ40d+20bLdZ5i8{i0R zd<A9l8}T*q=VX7M-BKTtCI0)f%(3wF55m&HZ-!(yWbiok0rhzZaTlI06Lm~6|3uWF z1X>rvk{hr@W&Ww?;dSApTC%(-l+NuN+SKyoP<VPJFy1i9-jkWnY2snok}y;FyK5{T z8Xj0m$i9_?{1#;o^a-O>oC{C?I?QloL@^wZ83+2cu)6FIm&)lJ-oB%P1l_K7@ZSmk zf4oxy{_^+az<9qr4j7-_iAf+MF&W3f|5Z(d|9jxC7$zj`9QEpN?oh8Lm{)5OK>M$E zayyTP;<zS!GItQoess5ib*7tK{evTyFB8z5|Q@2NEhdzG&It9Txm9hvmF)ad>(x zTs29`XiUXu+7K1Y9-p5beI~Hbgi>7};I_W(R~PvF>T=(9Qs?taiO(-n`}}QW`1;at zKFYP9(0)o&1U#112U1E8`}LsLbH44A-`Azt4w~%`p$V3){Y2GvG%#1cGU1uP{W`Lv zrU2E4TL4{HcVMBa%Wquj^BX0;?J{*Ybm{U3yAX3xesgsf`Q4{K<ws?vfCuGQw7A=! z0p1y@zU`D3<w^N<fggc>g!4_1QJ<d?eEw6CZsGf>8sSu$M(CQ0^Ht;E@z+vv;~pCH z^+o$;byq>=o`wMNLEfOxFDdgeutzGxHi)_(^$DuatDqy~J6Dy&=TF4k$ylFX&iVW= z(tZ_uG1nj9xp>d%tAx^%of8avDe++r!5`a<@_Eom$#<^Hz)KEZO7KbqFZ(0#Dt`#C zWpj89!mm^Z*z_gUNe;Qt<4YQYgV3ACdDqWKFg_&bSrrLNLuHe4M0MFecKr;Qn8YD_ zFvuPsKHFDKv~P}(T*k}viTUFe&6^A_W3eJf)HUjJ*)7@o`l{)gTYURG_K=JZ!qbo4 zj_h0E>4mo=`&K;mknnW;?KrGwgP!gp>ay^(WLP~+`O$skL##o%k4z&cE#TAnWx)Sz zh8vnh9!fG-i+N~rCOrKK`X<n)Fq_4-EzFP;Bv;Eb-u;T&DW5wR8L~Pg9SmZP{&WQ) zp97X4AGMvk_*wi)*=e%;xGCX-S4{EmO52#VvYU)7YlLspR(^J^ZMYqNi91a+2~RR@ zYW^{PHP8H*338!Cp6QqH`=649m}T{oQYJXg@{+-};X~l7ctd^e+k^2xX$;98mmVj_ z$#Uj63C4xwgVaU_*;d>)YJWVql>ghBqBAN!vnO?m)MdA1@TwDqXY#zsC*)(9XGor6 zgsjRuktC2j66|_8Jnfk&8Ol79e__B}JN5tfpF|#eb|xcOmaH0KR!LT+oWR&nN*_}y zyG1<d#~C5>O!|pW<tgunr+3YmhdB7J;<om}cj57)DMvtEGgJG=@beibB&(8z@N`ua z#{!%~!1?Qxmw~f>CcG-+1heXUUob3T4V|ecGR{;?N^;YB9xuy0v&6e(AbY5KD82u$ zkegqZmEHXOa9SS;u6#2|C!dn<Ai)<7t<Xuwl)AFuz@b+IIJ+oogN*7@Xtyu*WJce@ zO3K&JNjXZ(*6M1;lCG@Q?HZF`AvzNLUKgt~kEIAYooQ^1jvrGA>veHg^tyfTuG1OD zI9=12RM@K9IOfnjK4#S|@si-NuCyx+pseYRyP`z7{7@3Uszd7n7L=y$CVZdj-mP<u ztq8QEUfiv?(x%%x=0^SP?q+;#-C2PSopo$9*x%i$`{r0mplXWPUcXiqWaK}>S0X=) zuS|XhUxoY>zG`{@wLE$9WmP{gJ2V%PPbLqfwk$lBD5&~W{iH>vSM@IJ`9IfGCs$uc z9(XTAMrDR%?@QNIeKM{tBzs5IbKu%)`R<XbAS<W-;T|vG6Lbr%C^e~l%q``H)Pbdy zX_ekD7<JlH6_ZSEMt@@2r`J+Xu95%E4M`lidGIimyW?7F-zk2H*SP4se?`zJr`)Q7 zU5P~aF`pgmN|ul7f|A6Vsjiwxk6NPYe{5V2jbw&~kZfzR_iCmF8jCt@RrOrWTqKYY zdT!=cMfSoJEg=PfEue`v<ObuV(whpB-1FWiq)H~J`W5=^pGc}yz1cxoa{5SZkX67M zMRrh<oQ0MSwXS4#{9isBm8g#`8qx*XL>dWXO~*|lP5dBxs;gEpZZ*|FOWTbqxiYsl zxK@!VRQ^FNO`McU|KW2wPFBPT8!m`6B>n@kLM|wC<pR;DbJe|!fbX%Sn=d91Zw{~H z&HSZM;V4cbcN!<<lK(*c*Efh#HvZ;KlB|G6L|Mm6H57*gCnv`%DTazq7<%ibRHhzi z8u~}zC5$O=hPfElZzd_up~~|ZK}_Fdl&YJIDm%y~!>@?-!tU%KlUx?$y0fRqOC-3a zdto1HF4&o<7<rjD`>Uq9Y^we|#E7f_C$Uq>$Kbih44hJVFHpYYABC*m6~N1wBG$6t zW8LcGukk+qhkTx2)yG{(?w9HHAz5j1?^3T^P=QYYTB>79y<|D%u0Ec0vonEGr1(_> zx(mtC8BOk?b0pJm$6Tv=$r=(Ay4a|Pj@;_z*(Jy(x4L*A%DuNPg-Fk(kgRK^hU7?k zmc5>PQ0i?uDD%E{Q0|>L80XD>A>P~cg2MaS3klwd7nI)DYr@lqZ$AlrXpF}q3KD1I ziAwxw*K3{2UZ*^XccPQ==Ds5F<{kj&19TQ--ac^Ib6W>JXFXIZdMLuBRMkUt%l<B_ zH{pZaATe!U%I2xA9N8i-*dEG(b#k5j$}3OQhjM$ePWD#4I8M9~SufW_;=pj5ET_>C z@h6?hE+KUwqi^XwIcSMjiM&FVr}nFlKeveb@uSd{-hIM@*=mV5N%!PfYX5VKYJ|JK z_lUl4m=|A(Ts4>TS@3?bcf-86NxD@c@8u8hKFeza8c`Y{V#!CL(%$UZdSPz{C?X?j z)IJJndn2s`v6U5Vo#pB3TX<YP)zw6F*azB`BTSAYC!CvgEMuyxKHDoBui=mJ@+o3T z5wZrpVaoEVg##C|`WErh%41(JB(>8tNpiCLBqo`-76Mhl_kMTna#h(M7!5NiXBPFA zj9li2J|{0-3Ry=}#upCk;Zyn-ElL}BUB=v!7}C7)my3r;Zwa5?&*tKeF%wUvJ@s>} zf(>JIjj6}d`mvu?ruEV_n=oEIM0%JUieLNXE1!j@mxAK40gCg_H>8pu%g8NBN?Nb^ zGRf!1NDr4wF~ie|z}kddwhg)KTY%%(qx(?)7xLN^K#uwsDE|n!%Pqo-%1s>K1-Y!q zEfPtATQqv&d8*^Wv9y7dlWCP{UW%JE-aJH(QMpV`M8|h-#q$z@Vlrd!xB?b_W2|cP zyHxA`5Mi0h{t&~Z^-`-<<4+HL<7cF&gwH->nv~|I_dj07<b<dH9z4|j3gnuszV6B| zhL}R>n#JRan-bOKs*HDI@+pS}<1fw1Uy~cr_Pe*9;v@P>5CKFe<pqBD+M+(pQ*NIL z^V0v=06e%Ttv_P(-^s*ziCP?<-UsRPML+X#cv=_YhVn14I4xY#(_3#&D5am|rkuoW zwl}>$rIKo2b1OWZ8QMMcY+S<onSJ4D<_qnGG*J9TpAL$IP<UE_(S30Vm+5E@MpYR7 zP@nX~o#`*bnFFCk;2F%iuB^#BnT>Z8*%<csTbFT~`Oly0aHb0W6&Z=`QKvDNp8)8- z#r|TyKv>@dj*=VLxAzMwl=K{rp-E=6$FCve3E$`(9N;HMad9>laarK{F76k)rDd#y zJu9%NGts&a_}!9dKk7`h?t2UUOtc?$My&r#pt8D2q@PjO2}JGFOUPM)-v1<;lmJm% zOl4ewfTyb*synXjqTW;|Ce7!kHf+v4G?6&Bz4FjR{M>flp^43F4&?%0Lq_)pz+<%r z_5UdwZiK_DA7;nJ&4YgEZh{>-)Fuk_-f^+k=U31>9qvSL{a@6_wgr@fxQpOEq9m-K zRBHqon<L1$T$J^KoVzIC?l++%bwZ;0E!2-*EhOZIj9m?!)q(^xQqV{+F2%TdtQx$B zuhV-BmG{;hc^4s14SAw0DigFa&?+HM33-aqqX8NC$iYVm*@>d;qgS&a?+Upziyy<8 zD=T(arS?#Lsh)T@y+NSQ+&d?gPvy{YNtAC4XYL&|9e65o^g4%oqm(`7C%(vi6i;YM zGP*smfDnXo!-PUCcTV8tS1G~$?cb?PwGZv@Tqo+&;{7xgaZB%==ubm`*5A@L#t7Ly zVFbO5rS<v=tO@YkIU(M4!7CB>!e7qeCj$-b`#$Z1K4j20@ja1OT_C2%=i#t2hWGz~ zrz`^==9Ys#zYB*a8O{`tl2m^}R2J@^DbhNX8sk}ndnO>|BtqI|mwy@86eyMY{2zlR zHQ(i5j6Mn1BrJu^czk_z8QT<Kgu{UpypO23qgVBtAcvg3TKbnh|6<c&zk(dSS`FP~ z9JQ@sf`dGUD~Ie9Kl~xuNjRxm%KB1}2hvoK&%htgQ2%lk=)pwvAkYK8V7Wbk4X}Oj z#V)@}ee^2k@ah%cCI3#yNSUMGva+UtdO7a)YQg_jzC_SKM+JNID#IQQT-=QKW2C|c z=)Ab_u>Y;iQh~mAltOpR_0<h{1Np_%QeheA6mb_b7%PwpzepwmDLd>>gRHc<vS1ft z=%hHes_$g!F}_R<`3zzOa%sMc@ufN}V=Drb7nQfTyi1ftV^!tz`4QujD$K6}vG7Ca z`R%&H{vV)x8FJ5}>?Vi(htx-)=jrQ|XKZaM;0p%tJ4Agk@K!#3mpXl2!a@G1B*1V| z<Xnj$DQgs@b-xrC->Zn*zo2uvh^{BR01Axy2gq?dp+lGW-a_Mmfi8%z2^wFq_m%YM zx~A_cG;Rs0OQY+X<`PV%lForT8Y9c#mkh)Ra)5%VI~<^~L<HZ2Iu~*HOP_y18S)Nf zoK5!mkBoNt7qEx@4CfFSt|D+TnGs(5J?3L{kHD0@9FUVn{~_4(8FXF%AJDu)h;Jg6 zU&LI7_-4TKN;rJuIJO%(*(1<#^*IlJXZq|+<hA3@^qHB+YexcJJMK*P%qZ{@{z07d zefknWg2wX$v-O<w^xYA6SMmzpJm|Z!4DZSd@#=ABx)WShUWmghbR71?wi%Ys;|uu5 z>6>pk31*%rBi*3;SHPh`@nvR!=w`<79#Q?_hadi)=3#<gXZ0xmCDs-#H{S|RD$5%h zs_Gk`uHv^mT~%LSThD46JQhz!JG+rx;m%W%{Prfd#naT1-`?!7w72JX*!J4<9d@V1 z?OJQI?kmVQb=V!&e3Q-6?68?!7PmFOzG`b#c|+CH)fE+mCHdv`+w!+rZQESd4u`F7 zUvqwWTbsk)WbxQt&U~}Y;c)G8xxryhwX({zqdLF5!{e&9IoCB2%>&xyk-O*waYHnK zGE~Y;$s#ah2dN@F*+N)iCN|>0KNsq|@O7hZ1qJf-?C&ehgb7snt@dZ!7WaYt9dLum z<?=Mx+|SvYZ0(JfXB;*RK6C(PG;e9OG~3J;r`2I&H5*y2_Bof`%C631xjfR`(D-1V zso{aXM^UuC!|AcN+UC{uA!AhC`Uf?76zMUqpsaJ-ogT2}UG7$k=K<Ta+8tY-Y<x10 zjgGHmxlS(f)90}pH?kb5a5>vu^SIB{v3IY{{Q$=(`a^uKHM`vQgD$7X;;6TQZ3m4> z%7e>eSFU8WJ8f<cBK6)u%$3RUpzb@GEtPJ|^Y>3HP}gn}=Y5^Y?rF8OJ)mN}>-jCt zy{>Y%+j8Jf4mQ{i+St;Kte!M@+_a&M7~SZqw|P3;&b*BqX>Yx4e}~;|vwro;y)UFq zmUc0M!QJj`#w^WFN2G_HXF9gpoXwsVwv>Ikxvt6jG;i;;S<5|QN_ZL_+bo^-){fR4 zPJ5HfYKyRmkdR7Sy9Ze$=J9*h!mQxANb^XKhKqJC+FG{3)!}Zk<*}Ps2!u3bv`7z$ z;YZr+T6PZMBj9gjqqfAnu<_4#15^~}Rm<Dm?VgB-^288~%`()tHMyMDd&Us19SA~1 zgBHK&s`(beb}g%29l=C(>nd(lULKpz>JhYeNh<Bn?JyR08VV>er8O4jvGWtNYwn|c zK-?Cm$L4lg9A;Z*MT^DV=-PoL4xdN0fcQ<D*up%r8v6>3E2tG&%T&2JO4T>C*!Oy% z*MmxZl?W&a0kKK=({oVX-ek8&G>J}LEGXv00(86^L@^4WfBi_Bmnf#;SA<E77S6g? zMfScx*H)Oz>r3eVF{|xH8uO8Wx-aPA*89+5<s6&mi~rTpg;I(<tmilq(<~w&l67`k z935C}G-=*9J~yK88@eZ=?;E>Er+L^$CU0Je4K|CrspTGo2f~g<-Mx0ioF~ePt))5M znxoLy4vbi&fZ(p%=P-h|v2^FxV>R23H166Pq0}4m@`&hUWLY{1u{zgj;fOZUqPcNz z9uUy^$a%lq-I~7xE7{)cv{{k<A7!uKW@~l150v9j7dhb3y?>rQaIeErs67%k+Hxwh z)#mhIi<#4)nIgLF*V`<P2*oB=EXiFNVl>iB$aV73A9Z^)DMk+lI`I0lEuuT3cx0hw z#46&(7~@V4%WhBY-iQ=vEACRU!^+*%tc{##qM`Y4wE6kWC4S(D!08JNg+<2VlJ(`L zipnaUkbL}2R54&1zzR4B@B;n@Fa)>=_&2~Iz(;^j0DlC;ttKQBuo6%JcpR_=&;YOi zS^=GaZopB%X~2&FgMgm{{u%HQ;17TqKmzU}nSd1lJ-`I`bAxK$DTr+dn2KF?t9bq} zln^cNur#-02EK(z;PmW^^z>LF76qJGlyI?l(#|4NoJf%6u`j-eEE(g%bpl;Hbmoy# zXNSYVwu>9sCXz&wl+iAFtlku@H+R^ro9^z~;=0!xX?l*#-@Ga7o}m?7N$1)XT;c4e zpmPORKwFW@{Ut$%H9z_(w6ZL%ZlE8Gl<X#mzq{5%&e4T2n<CaCAS20qj<&UKdoxaE zj%X~3Yc%g+ylB#dQ>o|f&1Um+Y3OLeWvQL)vbde}FtEe9&*^&J$yPfKw6)CkR@xjk zkB#Lmc1MTXHru_+ZJ(z%y1*M<F1Df7;&9BOV`<`|P)Yy(Z>!5<Wvkq7mz!*HK8F-% zMHX>eS|P<vb~xKR+S*)h56;Xc;6)iyzmO+g9u~RBdI0BDde4h8iD**ic6n?~l&rSH z!y;kRO9VZ8wv(N>OtjkA3YXj6(dLOYc|;LdvfFG{yM>5fwyDM9Yz7M|iFb84t)yIZ z5%y-MUF@f9+i>ln+OW7mw83$t5rN0LIy@p>J555sEqA!7t_aj<`<aMUTPn8IMKy2a z8)H?fzN^XQm~~8(tF;XWa$ICt+<e+`7b4iGJK*&L>`uGOAs*IAMSVq-qRMHdR!6LX zlDG=ntgN<Ry?EG+whTCWMiF_t!#3;xtuVU-djM{9t!-{stGykx;$$^6mRHoUeEE*8 zjT>T<QTv?Dz1QJ-p0zvKy&cXb@!GJ$vhHiVPdwz7GkXZGx3yaAPOHt0Y=TaEyGy)A z;^C0~!pcbburJ=>NIyW0zZs?Qq)8v8j8J@KzWZN$KDqlqvd)UI`xC!5j5QXd8?p?G z4Q9huL!IGG!>HkH!><hQ7{&~j4I#sfffODu>?=G~=qvnT;p>I(6l#ms6mdm`MI}W? zih7EU7xfkWwn!*a8`F)7^}E+UwSLcf>-sm=pI`sx`t%JSmtHUZv@}>cRSH3BLbNCb z^>66U>)+Im>fhG?O8<_2On=1CV>oW;Gn_Jrs<MRaM&-D%&v?q{Gyc%{y74E*LE{_7 z^Ts!gqsF(5zcRjK95Y@vUNQdGC>TFBUN?Sf3>v45A>)jZ6w8Vg#j0X;ae8r9@#12( aczLn5cug@FLO%YJ#tpkS9NEyb;lBZO+uTI}
literal 0 HcmV?d00001
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 d28da54..f12fcb8 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -181,6 +181,16 @@ config CMD_BOOTEFI help Boot an EFI image from memory.
+config CMD_BOOTEFI_HELLO + bool "Allow booting a standard EFI hello word for testing" + depends on CMD_BOOTEFI + default y if CMD_BOOTEFI && !ARM64 + 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 woring at a basic level, and for brining + 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..90faf34 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 Word application stored within U-Boot" +#endif + ; #endif
U_BOOT_CMD( diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h index 328bc62..54ccac9 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..2b31b1a 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 $<
On 25.09.16 23:27, 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 v2: None
arch/arm/lib/HelloWorld32.efi | Bin 0 -> 11712 bytes
IIRC U-Boot as a whole is GPL licensed, which means that any binaries shipped inside would also need to be GPL compatibly licensed which again means that the source code (and build instructions?) for this .efi file would need to be part of the tree, no?
Alex
On Mon, Sep 26, 2016 at 09:36:19AM +0200, Alexander Graf wrote:
On 25.09.16 23:27, 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 v2: None
arch/arm/lib/HelloWorld32.efi | Bin 0 -> 11712 bytes
IIRC U-Boot as a whole is GPL licensed, which means that any binaries shipped inside would also need to be GPL compatibly licensed which again means that the source code (and build instructions?) for this .efi file would need to be part of the tree, no?
Yeah, I'm not super comfortable with this.
Hi,
On 27 September 2016 at 15:28, Tom Rini trini@konsulko.com wrote:
On Mon, Sep 26, 2016 at 09:36:19AM +0200, Alexander Graf wrote:
On 25.09.16 23:27, 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 v2: None
arch/arm/lib/HelloWorld32.efi | Bin 0 -> 11712 bytes
IIRC U-Boot as a whole is GPL licensed, which means that any binaries shipped inside would also need to be GPL compatibly licensed which again means that the source code (and build instructions?) for this .efi file would need to be part of the tree, no?
Yeah, I'm not super comfortable with this.
Do you think we should drop these binary patches? I could always put the binaries somewhere along with instructions on how to get them.
I do think it is useful to be able to test the platform though.
Regards, Simon
Am 03.10.2016 um 23:50 schrieb Simon Glass sjg@chromium.org:
Hi,
On 27 September 2016 at 15:28, Tom Rini trini@konsulko.com wrote:
On Mon, Sep 26, 2016 at 09:36:19AM +0200, Alexander Graf wrote:
On 25.09.16 23:27, 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 v2: None
arch/arm/lib/HelloWorld32.efi | Bin 0 -> 11712 bytes
IIRC U-Boot as a whole is GPL licensed, which means that any binaries shipped inside would also need to be GPL compatibly licensed which again means that the source code (and build instructions?) for this .efi file would need to be part of the tree, no?
Yeah, I'm not super comfortable with this.
Do you think we should drop these binary patches? I could always put the binaries somewhere along with instructions on how to get them.
I think that's the best option, yes. You can always just add a url to the readme to point people into the right direction.
I do think it is useful to be able to test the platform though.
I don't disagree, but I would argue that for the average u-boot user it brings no additional value ;). And people like you who know how to enable a new architecture probably also know how to get a file into their target's memory.
Alex
Regards, Simon
Hi Alex,
On 3 October 2016 at 21:15, Alexander Graf agraf@suse.de wrote:
Am 03.10.2016 um 23:50 schrieb Simon Glass sjg@chromium.org:
Hi,
On 27 September 2016 at 15:28, Tom Rini trini@konsulko.com wrote:
On Mon, Sep 26, 2016 at 09:36:19AM +0200, Alexander Graf wrote:
On 25.09.16 23:27, 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 v2: None
arch/arm/lib/HelloWorld32.efi | Bin 0 -> 11712 bytes
IIRC U-Boot as a whole is GPL licensed, which means that any binaries
shipped inside would also need to be GPL compatibly licensed which again
means that the source code (and build instructions?) for this .efi file
would need to be part of the tree, no?
Yeah, I'm not super comfortable with this.
Do you think we should drop these binary patches? I could always put the binaries somewhere along with instructions on how to get them.
I think that's the best option, yes. You can always just add a url to the readme to point people into the right direction.
OK. One problem is that we cannot write a test for it unless we actually run an EFI application.
I do think it is useful to be able to test the platform though.
I don't disagree, but I would argue that for the average u-boot user it brings no additional value ;). And people like you who know how to enable a new architecture probably also know how to get a file into their target's memory.
I wonder if we can build our own hello world application? I think I did it once. But there is EFI library code that we would need to bring in (perhaps a small amount).
Regards, Simon
Am 04.10.2016 um 17:37 schrieb Simon Glass sjg@chromium.org:
Hi Alex,
On 3 October 2016 at 21:15, Alexander Graf agraf@suse.de wrote:
Am 03.10.2016 um 23:50 schrieb Simon Glass sjg@chromium.org:
Hi,
On 27 September 2016 at 15:28, Tom Rini trini@konsulko.com wrote:
On Mon, Sep 26, 2016 at 09:36:19AM +0200, Alexander Graf wrote:
On 25.09.16 23:27, 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 v2: None
arch/arm/lib/HelloWorld32.efi | Bin 0 -> 11712 bytes
IIRC U-Boot as a whole is GPL licensed, which means that any binaries
shipped inside would also need to be GPL compatibly licensed which again
means that the source code (and build instructions?) for this .efi file
would need to be part of the tree, no?
Yeah, I'm not super comfortable with this.
Do you think we should drop these binary patches? I could always put the binaries somewhere along with instructions on how to get them.
I think that's the best option, yes. You can always just add a url to the readme to point people into the right direction.
OK. One problem is that we cannot write a test for it unless we actually run an EFI application.
Well, you could always provide a binary disk image that you run in qemu as test case. That one doesn't have to be gpl compliant thn because it's not derived work :).
I do think it is useful to be able to test the platform though.
I don't disagree, but I would argue that for the average u-boot user it brings no additional value ;). And people like you who know how to enable a new architecture probably also know how to get a file into their target's memory.
I wonder if we can build our own hello world application? I think I did it once. But there is EFI library code that we would need to bring in (perhaps a small amount).
We could. The main problem is the PE header.
Maybe we can trick around that with bincopy -O binary though. Hmm :).
Alex
Regards, Simon
Hi Alex,
On 4 October 2016 at 09:50, Alexander Graf agraf@suse.de wrote:
Am 04.10.2016 um 17:37 schrieb Simon Glass sjg@chromium.org:
Hi Alex,
On 3 October 2016 at 21:15, Alexander Graf agraf@suse.de wrote:
Am 03.10.2016 um 23:50 schrieb Simon Glass sjg@chromium.org:
Hi,
On 27 September 2016 at 15:28, Tom Rini trini@konsulko.com wrote:
On Mon, Sep 26, 2016 at 09:36:19AM +0200, Alexander Graf wrote:
On 25.09.16 23:27, 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 v2: None
arch/arm/lib/HelloWorld32.efi | Bin 0 -> 11712 bytes
IIRC U-Boot as a whole is GPL licensed, which means that any binaries
shipped inside would also need to be GPL compatibly licensed which again
means that the source code (and build instructions?) for this .efi file
would need to be part of the tree, no?
Yeah, I'm not super comfortable with this.
Do you think we should drop these binary patches? I could always put
the binaries somewhere along with instructions on how to get them.
I think that's the best option, yes. You can always just add a url to the
readme to point people into the right direction.
OK. One problem is that we cannot write a test for it unless we actually run an EFI application.
Well, you could always provide a binary disk image that you run in qemu as test case. That one doesn't have to be gpl compliant thn because it's not derived work :).
I do think it is useful to be able to test the platform though.
I don't disagree, but I would argue that for the average u-boot user it
brings no additional value ;). And people like you who know how to enable a
new architecture probably also know how to get a file into their target's
memory.
I wonder if we can build our own hello world application? I think I did it once. But there is EFI library code that we would need to bring in (perhaps a small amount).
We could. The main problem is the PE header.
What is tricky about that?
Maybe we can trick around that with bincopy -O binary though. Hmm :).
Yes I think it is possible, and desirable. But for now I've gone with using an external patch. I may come back to this another time.
Regards, Simon
On 18/10/2016 22:37, Simon Glass wrote:
Hi Alex,
On 4 October 2016 at 09:50, Alexander Graf agraf@suse.de wrote:
Am 04.10.2016 um 17:37 schrieb Simon Glass sjg@chromium.org:
Hi Alex,
On 3 October 2016 at 21:15, Alexander Graf agraf@suse.de wrote:
Am 03.10.2016 um 23:50 schrieb Simon Glass sjg@chromium.org:
Hi,
On 27 September 2016 at 15:28, Tom Rini trini@konsulko.com wrote:
On Mon, Sep 26, 2016 at 09:36:19AM +0200, Alexander Graf wrote:
On 25.09.16 23:27, 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 v2: None
arch/arm/lib/HelloWorld32.efi | Bin 0 -> 11712 bytes
IIRC U-Boot as a whole is GPL licensed, which means that any binaries
shipped inside would also need to be GPL compatibly licensed which again
means that the source code (and build instructions?) for this .efi file
would need to be part of the tree, no?
Yeah, I'm not super comfortable with this.
Do you think we should drop these binary patches? I could always put
the binaries somewhere along with instructions on how to get them.
I think that's the best option, yes. You can always just add a url to the
readme to point people into the right direction.
OK. One problem is that we cannot write a test for it unless we actually run an EFI application.
Well, you could always provide a binary disk image that you run in qemu as test case. That one doesn't have to be gpl compliant thn because it's not derived work :).
I do think it is useful to be able to test the platform though.
I don't disagree, but I would argue that for the average u-boot user it
brings no additional value ;). And people like you who know how to enable a
new architecture probably also know how to get a file into their target's
memory.
I wonder if we can build our own hello world application? I think I did it once. But there is EFI library code that we would need to bring in (perhaps a small amount).
We could. The main problem is the PE header.
What is tricky about that?
Our compiler usually generates elf files, no PE binaries. So we'd have to assemble the PE header ourselves - or rely on a second compiler.
Alex
Hi Alex,
On 19 October 2016 at 01:07, Alexander Graf agraf@suse.de wrote:
On 18/10/2016 22:37, Simon Glass wrote:
Hi Alex,
On 4 October 2016 at 09:50, Alexander Graf agraf@suse.de wrote:
Am 04.10.2016 um 17:37 schrieb Simon Glass sjg@chromium.org:
Hi Alex,
On 3 October 2016 at 21:15, Alexander Graf agraf@suse.de wrote:
Am 03.10.2016 um 23:50 schrieb Simon Glass sjg@chromium.org:
Hi,
On 27 September 2016 at 15:28, Tom Rini trini@konsulko.com wrote:
On Mon, Sep 26, 2016 at 09:36:19AM +0200, Alexander Graf wrote:
On 25.09.16 23:27, 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 v2: None
arch/arm/lib/HelloWorld32.efi | Bin 0 -> 11712 bytes
IIRC U-Boot as a whole is GPL licensed, which means that any binaries
shipped inside would also need to be GPL compatibly licensed which again
means that the source code (and build instructions?) for this .efi file
would need to be part of the tree, no?
Yeah, I'm not super comfortable with this.
Do you think we should drop these binary patches? I could always put
the binaries somewhere along with instructions on how to get them.
I think that's the best option, yes. You can always just add a url to the
readme to point people into the right direction.
OK. One problem is that we cannot write a test for it unless we actually run an EFI application.
Well, you could always provide a binary disk image that you run in qemu as test case. That one doesn't have to be gpl compliant thn because it's not derived work :).
I do think it is useful to be able to test the platform though.
I don't disagree, but I would argue that for the average u-boot user it
brings no additional value ;). And people like you who know how to enable a
new architecture probably also know how to get a file into their target's
memory.
I wonder if we can build our own hello world application? I think I did it once. But there is EFI library code that we would need to bring in (perhaps a small amount).
We could. The main problem is the PE header.
What is tricky about that?
Our compiler usually generates elf files, no PE binaries. So we'd have to assemble the PE header ourselves - or rely on a second compiler.
I think I'm going to go with the first option which seems easy enough.
Regards, Simon
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 v2: None
arch/arm/lib/HelloWorld32.efi | Bin 0 -> 11712 bytes arch/arm/lib/Makefile | 7 +++++++ cmd/Kconfig | 10 ++++++++++ cmd/bootefi.c | 26 ++++++++++++++++++++------ include/asm-generic/sections.h | 2 ++ scripts/Makefile.lib | 19 +++++++++++++++++++ 6 files changed, 58 insertions(+), 6 deletions(-) create mode 100644 arch/arm/lib/HelloWorld32.efi
[snip]
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 d28da54..f12fcb8 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -181,6 +181,16 @@ config CMD_BOOTEFI help Boot an EFI image from memory.
+config CMD_BOOTEFI_HELLO
bool "Allow booting a standard EFI hello word for testing"
typo: world
Please fix all other typos which were pointed out in the v1 comment: http://patchwork.ozlabs.org/patch/656430/
depends on CMD_BOOTEFIdefault y if CMD_BOOTEFI && !ARM64helpThis adds a standard EFI hello world application to U-Boot so thatit can be used with the 'bootefi hello' command. This is usefulfor testing that EFI is woring at a basic level, and for briningup EFI support on a new architecture.
[snip]
Regards, Bin
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 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 798b566..e027bd3 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
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
Thanks, applied to
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 v2: None
arch/x86/lib/HelloWorld32.efi | Bin 0 -> 11168 bytes arch/x86/lib/Makefile | 1 + 2 files changed, 1 insertion(+) create mode 100644 arch/x86/lib/HelloWorld32.efi
diff --git a/arch/x86/lib/HelloWorld32.efi b/arch/x86/lib/HelloWorld32.efi new file mode 100644 index 0000000000000000000000000000000000000000..e59b84b4d1d5f5aeabb4f25094ec0724ac114b4e GIT binary patch literal 11168 zcmcIq3v?6LnI0KK1u+>zTxuF884^e=UY2B8wgV=9jROIJkdVn^9jaKuk|Rq*nhB=G ziRnluOefytoGmT8C3~EO?QwVM3EOo8%}J>p5)8Ci2%EIoxF;cLI2o^Kwn+&VgjD<8 zxg%MAkTlt|&bgZV_#fZ@-~ayipCOL>nXmlU{gpY#7@a)BtTW_~O`vy<x!!P{fnm6L z3{yLYF4g9tkM^x%I_B^oZQ@_=5=uwrXH8tx$A#CJpK7|-1bM)6ZZX3QOhxb&=avip zP651{jj${8%oWGM+~q#6;Kd~I1F=2x%%!UZwB=EMDAG!?W-yay?jPvtm;3w=iEY%D zj7{X3`$y<<$Dh~t$mcgVyQK>S%GbI145NrfrKKQoR+v@oHXgx5cl1`L)7`8tfROv| zL+3#_Qy#kaestHPE4^GOr+WMsA9!#}%dw2GB1J4vT8zzZwF9n^htw04GCm+U66qal zR6C|ID23ht1b3@8Oh6RoikzDR#8Gs9A7yj~c9E%LpbhLnPrX5(cS6U0s>3%y$61WW z={S=m7_Xy>&TpnVj%Rf|haNC(+z1^{OoJWAR3<CJkr;*@;fXpzS%M;$JI9XKsVv=& zlUW^W>3og@H7~Y|V4i_H-IifzcTE5r!3a05xM?Liu^ZsEhCnU4>g&{P$@7*0rRC~L zvH|X07YZ<&77Np2L0UBSJd^q?oel&+9$>KGRu$OtZ0|RTaa>|Rn4{F%@R;Rtxt(ce zJ}mBC3gW&(;cM!@%w?G5%sx(-t~wFGnhtFhidv*|23=u3<RWa;chl)KTs}WBEPj1s zU?-%s*988K0BywzB&$uJOz=jbyoWDzx&y?Y6rfxE36^wNg;T;Bul<aw>sNSVVtOf* zw6J>yG(c`l0nYvduu*kTOgBq4WXKR;pAcZ>fw^S))L%4U@=e4cDwicDLlC<HF?8Ci zDe*IuSZP?fe;*<L2xHg_*<=WEXW$7!eitPVQ}VIBkuDe&zCfw1+Q0Bdu>UN}PKFwL zA7L*Gyhx;%W!SZS)laEdpniw4-6(Y#!LynAxgk(WM1G3BCVt+m(_G?>2I1@OKnRQN zjg;nhI!!sHnV+~sobnW*;3>t=DaEr3;V1-K&rT+9v+6<s;0Sz1*D~Ae3cO9cc@JgW zNf))?Gj4XPUYG?qi2->?C{A3$;YKb=H<KVs3>?8C!>&$2gIk>e={N;)Dw9ZvUcSJo zMMQerQbe+P34q}X%K(D!nCIClDAs!N`DR5dbR*-aTL1vizIo{w%-$H-Ujl4*2Y!u* zdhRN9%}Ppg)97&oA0uFk3vU9|YD)D2rE)s4QSCTIMX#yNfm%>H-Rc$4_xOQOJEQK6 zMB#^^hr*kP!eW=EFz^=WsI<^TY4FTzYI`k&{S-u*GmE(B;ftKvGpjy>I3$*ymCq}V zB80PB{`FwexPmW;&0FCq?3yEW6&rRNg=xF*HB94+>yzgn&@hoqUI<tK-y>dBJcY-n zG0hFI!sS%&oDGGJi*Wt-WZSI_v->*vwV8wBIanmWsoMnZcBY3f29%;PG&q>dSPbq| zx}c(9bco^LzQV*X-AA4`%DgdYu?!@J+4w63@c7wJ_2WSTwb-deN`R!yfC!JP8?X#C za!NW^E2Rwc2W0+$#2?5baV^YV%Rbj~U|Ne&ZrPjs+7Q1N7#QdTx6q(=Qpt-EBtWVL zVVV|FVsUf8eKQpFWVMW3(X<Tl-%qC*W{BTMtV!0X72`!bHzvXz5QvC9SrI=T69Ha` zzzU=AxaEq5_$Se4*rT-u_W5~(=ZrJY+{S-j=8s^{WWG<~k06!JQ^Y={WpCmVyNB<t zemnZj?yhb|D54%@<G)6oqwr6TJboR+?E0W~0c=UlX5)Kjkj^6EdMP!FO<cuj<TQSU z_<o{si0>sRhWMw+NC76`(I8F0BBhqGaZ-(Kw@RtSY`m0oZ%7QV@%PY^QXCuq3=)#7 zrPLBO{@>_IsikcEBQj@`Qq|jUl2R5{x=$ls&c=VPQJC3yHD;1?rPMk${u^yB#Ky@5 zEv0<0pN$_Tit445n~mR!8)y4Bh#m1qrxOa8w-}v~A8I2n8=nCRDb>ox*J_=6+4#4> zKC)d~kFoJ{*tPAPl-kC|8#N?RP@w9RQf=sfXJnm5Dzb4i*4gi#*W`4v@f(SpGA!;o z%^1;r4M{{gZ5TPJJ@ZBewP*gwFSMs|<lnSs;mA+$#14T)<iHC3QdbEh&Qkbdg^~HE zlf?>OLVF9fm3jI~FP_pszRVYo_#s25!fc5)TQaf%v+@<`6i3CgjaL-UzG)uBlD4yw zYL;JH;@BspW^TXbVMqTg!wLB{neS(h)*;4bPV?*|VRiyxmV6y#REX>-ViWvK5()hj zEg+7ZN2)_;4Ds3#HSti<iydM{ZXvGw;G}dKrgdXNzI<u0Tbw1m`&sv^waGmBvV1~1 zS12yl44bQT6)P>pN~LrxUmhJy-;%st=_*lLN^)lI&?s-nP%4k-OUH0@i<5IQq?xG_ z`IPitzI4U#vnBI~NB>j$19xdKkLBMY0M^PU2GwZ`4ZHb60>HcjxY#`gC1dfWS8Bx@ zq?380Z%e;pj^M;FqwoCtvgA_vls4G)USTb^_zHtx@f((O5)m%W0ArpdeT2L(9On-) z&dtp|{7X*R(XXDNM_O_8OI^K;I9vXB<`v!lg0ZLTB~uC>hcADHFk&ocd&HLv$45uy zl<7?3EX$vfy80Q}b5NMm-F1i&t^=Xv?CQ}`1K%GX7G|XR{xsh!pZ=KvTLI;l264#V zgrrM@1~ze;bY970<H*g}uHNqRMz#_0v<}2%TS%16Cb49uQIv%E5`9pDyZX9aAEc4H zj+P=xf!%V_a@I1OiGqW~PPTDS=6grBLcS&qiJ_Q1jG%on8=A?v5Sp3O1C#2voE_p1 z!f3`aOl0+lFOJ-l5v0M=gPX#lgA$VRzeA1)xyn!B%*gzqo`!rmf1&z4jM&3}>S>rk zMmx#qc{eM)T%b4(xzN3yJv>03v&l~Z`MHk#%pyNC$&b<HY({wkYulOj+sOv1uFE8+ z*N&V9OwGbVMZ|F|mX7D+M2f}9>xs)m98b*1VhPE1ACt83ymLArLaH<0#U3ssooV=~ z&2t6@aus}(A_~mOM{=e2tS8bPInpsBJ0N|>eEFqa@8QmVyyu>@beY@EDYiWa7a%_t zNU7=D=PI_uUW|HLUaEeZO*{r9K;aC$m8<eGO(mCcKK7Kl|D}<C1id2eGl*J($3fRF zn>m@w25D8xx8ya(Bo>iqIBJJvT@sNp^1wI|ExL#s#zf54M0^5&Ld3`)@w`Sf92sGW ztd@O-mTTQN6wa@Rtd=R^=m%}&Fh~@~J_D(6ij-@h5_XszMM>dWaCBM8@C3&ly&En_ zZk;&0g3LN6%qH)eFmbeskeQ~M)%EF_Nxy#O)FSQZqKG*uJ9h8#?`90?^e%okDLG_E zT;k(-IH5=dVhN!o2ju5F5ihzlInjwh(lXi-Puitlk^v(wQ+f!A!p98>($Vw3mi!a^ zZecdk49<Tw-yeM=c?vxwv+6;%ft-CPG2l4XkZViUxWy)ge_C-ISi<+R&z&$ho&+8a zc*?X|<W{YaIv{(x+d0&Jd(&c{`q(Gr3~-948-)xh!TPXWO#G}6MUGpp@COWhMhK@k z`VuFF`6Q~@qsN@>2FTc`uA~yQI;dCiKzBd;+!=#MXJZpT&;kIgY+ON?f@n7r1RK@A zT-5Xv<&VHR`Ess=UGEa~vZGtPh3YQWnKk$u)xj*Y<t*T?pl~JBUE*Se??X{`8yMK5 z!)ft=TZ8LVd$K&q85-C07ZB=<{OPVgXv$i;S9tnaNsHQ+1BR9-i8%K88SK&Z2C-ae z=}Qa?OCfdeT_dE<9K6l3Ti!$Va-*ietuBQEMVo<NVb?S<PVSGMdkn&ZY5t(pb#R*K z>A5Fwg`-~&y%=_l_*nyfCk4KYS}OMt(d|oh@Mr!^cT1bqJdSNUn4AF~Jmpg(wV>Yh zOA<&VGDJHG2eKsyhp{bzH^n}@r;YPwvl^qqHL;mObJB4|1{o>9j;q7}lPUZAQ1)-6 zW&f+VLulE5FP2fP!!Q_vve?<q7>>0Y+$82^QAO%rD#AKCREr%DXq|6VcVR#DI^Pv| z2WK?!+pJgv6|1&T)-vgXTGTJ({lKV_PeD4LGqB_q@C|4APJXDv>081ZEoVo?1LS7w zCl~0Cv0ph~X0$i(60OKKDqVnHDD&^De*}N#W!M>LycO3}Szsd`Y9u51eHo;pQSK^` z`66B5hzFSP6$NixL;O`3iM#wN!t(Oii&g<X#;Jcx#gmI4u6z*hO#yN{2Yy5}%%&RD zASL4TS6aM>{uh=5Kg|-p|3_+MjT!IEmf@|^$_E)?X(BBwQkUrpsy~h5740hFBPBjn zaFFsfBwr=(=6Fk_ANiyUcsn$zZUocGo<=4w`{7T6JPamyM1$<2kY7b%LnYpyL1NR8 zbg#TQ1LT0tMu9xB2pn%@*>1}K`H!i9bP*u61jTP!jw$OF7ATDi$zFcsB%RTZFje%2 zJbBLj+UFq`d2hE<ZQ7?Q`exdGd*JC(IQ7T{%~xBMX_?n^x5kmL<HN_T%)4w4ag@d+ zQKm5-=|V{lf6@6Y`7lI3#HF1}wHVxiRC*DN*zk+mW+MdP&Gvlt6=H@45mP#+G3_+t ztIHEqKHN$_Jbr>P24f#k|C}Wl|KafgI<J3tEGJ?Adt&9;B#j92)$r-h`I}YecJr=H zTll6mTX>gigNrM53tmBtaW&kcsA(3nVA6_-DT}p6y+UiCEY=qC#$sine`~NT7Huu_ z`yRHI)r-N9ugu}|J0d<Y<S%pa>-ai1zvb?R23u9xn!0l9*rM~{wz9fy+d{!suMmuc z%U1hCp~&V)6asR(jeNc5?y@>jh}`WDFK=bWmWZFV@BeUZnkD^uR%_^X;{-AuhG&|X zH4MkBX8cSD|3%Q>jNd5wK4yXbuMh_62X2wKbdk?=C!VvIrbx8ID{u>HxYE)`BS9ax z*u<H?$a>gKj9M3bDC&*wDD%KIjpUO)CftILk4DQfLl{yoZr$pSPSVh}ro-FjU+oS1 zLjFmMpYQxB)YnHMg4-W`G}!8oZSp=8@&k&7YpMl+YIARzq_nNxJ!uZ!9s6r@O=kMi zNL^xB2zL0V1bdA&;MM-Vw62U3@N`ClVF7|lOFe7WZ(48S^o_rrGk2QlpT)%0)Np2| zArg*7rq~pH{u*0Wx;hdKemfEtydjq#{33~U;!m@QyZv^qwAmjOa013JBGT$ZlcUem z=5366w@(a8kS<%Lh0XH%pwQvnHfg(Dk?m{3TO)PRsCUQbF1mx?_H!$1I1A$zqGV7? z!*o-`<rl<g*i=(PW?lYA#9-9#gWEEGBdH@QY}iWaO)!plFyp%~wMF&bm|xo$vQL?g zwntfIbeDiJ7;c-WD+klmU~-u!dE@_%bWK?BN5kF_5N{6pgaEgaE3aZon?l~U7`*aW zDe()pleni!dsxldqsk0Rv`8z>5)&FaY2tS7L@vrmF?WJoqc$-s!(u4J#WZKEVrDb5 zXX#VqK&{gIZDP>3DzmE))UdkFWwDdAM%`i3Vv?zgwFZOw-gBknmKK?-I+ran+p<5_ z&P8S`8PU$fCxZ!Jnf_z*aUAL1Nq(?WbuWpXF>B~ivt?|e)@E_!pxqD+wgur`eQPz; zrfbR4jv19T-ms~qxW!iF$r!WQ8x13{I8XTDaAbR!yF0XFTOd2z=nwe?KiA|9hQz2p zJH0s?oMM*3<TgbjoV&vt3T4T0T#!o|$-f}4i+Ft;Zih&eSrdNL8^Uel^hUiM&=O@l z;h4B>TO=yrwAF)77f4PavpynlF2C2egK6}W6t4@RNSx7#;BO^t8$^MNYz1X3B1T~# z(~LCK;pZA6(Wtmh$PAkhMG(n3{Jx-<(SEtsfH&L*5kyl{L=5|wI?Y8~qdy$f=84#C z$czLHhupIbj$<|<@VJO5XnZk}7(iPWiV~~{RDJvqwQ5a+!>J?R)U+wnCGaDykx<q# zt&xsxC=_sqambl5WKqWLZx!M79b9uT64HtYroq*qbMRpwv6@-~khlf?KCaYSZDPho z<;b);VN)>V&-#BI%ocGEBk6Z+i$*$vG4N`eg&XG)yk+b_dkXExXfL9@fu^GU8Erbp zFmupeMt=dGOVFy&R-@gA7C;lwy3iD~J!t#Uo<n;9?HJl=w0F=xK>HZ&Gqi#QkcVbN zTZOh3?LM?Nv=~|(&1|uj+bZmpRn>L%4UK$LWAM>nETWYe<}aw-CX^2(3zGy42_Czx zv)t<PcX)$gpFfJk_NB8VkwnvV?qy47=v6WPvR&s7w`r+!OPjORx1}lA>G#zMT6wtz z6Ao`@utV(dgoCXSpP!1MEI0}=0SO{=&5kSIvbR+((G*v`e$SNVk}c!h+VL@Qt2iix zHk8YBhRM*=F}DoIBMagy(ARLfEg3J=a9`RDfT&&OrA@(TOrRdpb9{CQ=O+^Oja$Rf z5K(!c1VDPuT(^%QEidCr7gI`f*WF>h1D919XTkXyW66dJVJ7vI-DcvZX6BYo;GLu{ zbB(U{cQypP(M=H#DmD0AM*;P#R&h2Hvlx{JDFXnDRvQFd4k-M;8o{jsLU=N$FH!-J zP>|V#&)<X0{Q@NV=4FB+Cl-KlEQm53Aph$lX-cLH!Y>Gu4DTd}+=M{auG=b78)VVe zkeH_dH6iHXR#Hpkki*Ixo2QEZ#ri>6i6Y2iHczNJXsOxk4T=7#tCF0Ep1?XTq9?44 z!$c~B3G3s|LB?0Q$$oFNH874~QrMBGo4}>#3r$z%EaiAB2hd*~7@0@`!<gH11i{-l zx>Ms=%*9YO;7-)bDU*rOoQwjMY(l1Q?(~|OQaU2JajOXgnEYx<--B8$*cSHtkoTRq zQDd~u{8ASPd69PbJ0j5?b@)U;KNpY-Wr{g+yVMbd9saO@izR13@`GMrqi70IFso2% z0Thapj9_iYN&Yi;HtB!F+4}3DUVbfsEQb7jA8oDm4XbMX*jin_uDrSYnes#BFO;7z z|FHaWd8w_+=CQ?WKeGLk?X>M9+t(^eDwb5(DppjiskpboTM?+}tazg0>53m#9IiNC zak}ES71Qjq?049g+t=8g_7?kh?Xvw{`=|DsD;HHZR<5mls`BqE?NtY>p0Ao)T~h6- z-co&~MqRb$u79|z@2(f`(g5KfBAn>VGyk)BzNO64YzbNR+kR@hV#}|vR;;Xes^SL~ zA6BF)zF}WrFSBp3-)Dc;{;&35*x$6jXK$)>RCZQ=x3ahLt;%^-rYgL(Fbu~qe}l(f z%RWo5<$$Hna?o<f(r-CpdC@XpdD(Kp@*B$;%j=dmEx)&%v%GInEf*|*vRt&JELSaz zHQ#Eq7FY|dMb=_#iIuZ1w3b?zTFq7nobZ>2@gB6Ri)mY5Z!zbRe15$Ae|>fu``EPH KhvIm`-~R%zH!}tR
literal 0 HcmV?d00001
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
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 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 1cdd49d..30dfe90 100644 --- a/configs/efi-x86_defconfig +++ b/configs/efi-x86_defconfig @@ -35,3 +35,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 ba5bb99..9d3129a 100644 --- a/doc/README.x86 +++ b/doc/README.x86 @@ -1070,7 +1070,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)
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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 | 66 +++++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/setjmp.h | 24 ++++++++++++++++ 3 files changed, 91 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..7443274 --- /dev/null +++ b/arch/x86/cpu/setjmp.S @@ -0,0 +1,66 @@ +/*
- 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
- */
+#include <asm/global_data.h> +#include <asm/msr-index.h> +#include <asm/processor-flags.h>
I believe the above 3 includes are not needed.
+#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 1, %eax
This should be $1.
But I still think the setjmp/longjump codes in efi_loader is wrong. We should have longjump() to pass the return value to setjmp().
movl (%edx), %ebxmovl 4(%edx), %espmovl 8(%edx), %ebpmovl 12(%edx), %esimovl 16(%edx), %edijmp *20(%edx).size longjmp, .-longjmpdiff --git a/arch/x86/include/asm/setjmp.h b/arch/x86/include/asm/setjmp.h new file mode 100644 index 0000000..deef67e --- /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);
+#endif
Regards, Bin
On 26.09.16 09:00, Bin Meng wrote:
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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 | 66 +++++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/setjmp.h | 24 ++++++++++++++++ 3 files changed, 91 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..7443274 --- /dev/null +++ b/arch/x86/cpu/setjmp.S @@ -0,0 +1,66 @@ +/*
- 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
- */
+#include <asm/global_data.h> +#include <asm/msr-index.h> +#include <asm/processor-flags.h>
I believe the above 3 includes are not needed.
+#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 1, %eaxThis should be $1.
But I still think the setjmp/longjump codes in efi_loader is wrong. We should have longjump() to pass the return value to setjmp().
Why? Where's the difference?
Alex
Hi Alex,
On Mon, Sep 26, 2016 at 3:05 PM, Alexander Graf agraf@suse.de wrote:
On 26.09.16 09:00, Bin Meng wrote:
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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 | 66 +++++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/setjmp.h | 24 ++++++++++++++++ 3 files changed, 91 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..7443274 --- /dev/null +++ b/arch/x86/cpu/setjmp.S @@ -0,0 +1,66 @@ +/*
- 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
- */
+#include <asm/global_data.h> +#include <asm/msr-index.h> +#include <asm/processor-flags.h>
I believe the above 3 includes are not needed.
+#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 1, %eaxThis should be $1.
But I still think the setjmp/longjump codes in efi_loader is wrong. We should have longjump() to pass the return value to setjmp().
Why? Where's the difference?
longjump() does not have the setjmp() return value as the parameter, which concerns me as it does not conform to the standard longjump() implementation. This v2 hardcoded the return value to 1, which makes the following logic work in efi_loader.
if (setjmp(&info->exit_jmp)) { /* We returned from the child image */ return EFI_EXIT(info->exit_status); }
If we want such a simplified implementation in efi_loader, we should probably rename these functions to efi_setjmp() and efi_longjump().
Regards, Bin
On 26.09.16 09:21, Bin Meng wrote:
Hi Alex,
On Mon, Sep 26, 2016 at 3:05 PM, Alexander Graf agraf@suse.de wrote:
On 26.09.16 09:00, Bin Meng wrote:
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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 | 66 +++++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/setjmp.h | 24 ++++++++++++++++ 3 files changed, 91 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..7443274 --- /dev/null +++ b/arch/x86/cpu/setjmp.S @@ -0,0 +1,66 @@ +/*
- 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
- */
+#include <asm/global_data.h> +#include <asm/msr-index.h> +#include <asm/processor-flags.h>
I believe the above 3 includes are not needed.
+#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 1, %eaxThis should be $1.
But I still think the setjmp/longjump codes in efi_loader is wrong. We should have longjump() to pass the return value to setjmp().
Why? Where's the difference?
longjump() does not have the setjmp() return value as the parameter, which concerns me as it does not conform to the standard longjump() implementation. This v2 hardcoded the return value to 1, which makes the following logic work in efi_loader.
if (setjmp(&info->exit_jmp)) { /* We returned from the child image */ return EFI_EXIT(info->exit_status); }
If we want such a simplified implementation in efi_loader, we should probably rename these functions to efi_setjmp() and efi_longjump().
Ah, I see. The second parameter to longjmp() should be the the return value after the jump - which the code above actually implements. And then it clobbers it with a 1.
I guess that's my fault, because I skipped the second argument bit in my longjmp header definition. Please just add it to the longjmp prototype and explicitly pass "1" as return value in (don't forget to adapt the prototypes in arch/arm/include/asm/setjmp.h). I can fix up the arm code later to actually pass the argument.
Alex
Hi Alex,
On 26 September 2016 at 01:28, Alexander Graf agraf@suse.de wrote:
On 26.09.16 09:21, Bin Meng wrote:
Hi Alex,
On Mon, Sep 26, 2016 at 3:05 PM, Alexander Graf agraf@suse.de wrote:
On 26.09.16 09:00, Bin Meng wrote:
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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 | 66 +++++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/setjmp.h | 24 ++++++++++++++++ 3 files changed, 91 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..7443274 --- /dev/null +++ b/arch/x86/cpu/setjmp.S @@ -0,0 +1,66 @@ +/*
- 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
- */
+#include <asm/global_data.h> +#include <asm/msr-index.h> +#include <asm/processor-flags.h>
I believe the above 3 includes are not needed.
+#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 1, %eaxThis should be $1.
But I still think the setjmp/longjump codes in efi_loader is wrong. We should have longjump() to pass the return value to setjmp().
Why? Where's the difference?
longjump() does not have the setjmp() return value as the parameter, which concerns me as it does not conform to the standard longjump() implementation. This v2 hardcoded the return value to 1, which makes the following logic work in efi_loader.
if (setjmp(&info->exit_jmp)) { /* We returned from the child image */ return EFI_EXIT(info->exit_status); }
If we want such a simplified implementation in efi_loader, we should probably rename these functions to efi_setjmp() and efi_longjump().
Ah, I see. The second parameter to longjmp() should be the the return value after the jump - which the code above actually implements. And then it clobbers it with a 1.
I guess that's my fault, because I skipped the second argument bit in my longjmp header definition. Please just add it to the longjmp prototype and explicitly pass "1" as return value in (don't forget to adapt the prototypes in arch/arm/include/asm/setjmp.h). I can fix up the arm code later to actually pass the argument.
Would you mind doing the fix-up patch? I don't think we should change the prototype separate from the ARM code.
Regards, Simon
On Tue, Sep 27, 2016 at 8:34 AM, Simon Glass sjg@chromium.org wrote:
Hi Alex,
On 26 September 2016 at 01:28, Alexander Graf agraf@suse.de wrote:
On 26.09.16 09:21, Bin Meng wrote:
Hi Alex,
On Mon, Sep 26, 2016 at 3:05 PM, Alexander Graf agraf@suse.de wrote:
On 26.09.16 09:00, Bin Meng wrote:
Hi Simon,
On Mon, Sep 26, 2016 at 5:27 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 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 | 66 +++++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/setjmp.h | 24 ++++++++++++++++ 3 files changed, 91 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..7443274 --- /dev/null +++ b/arch/x86/cpu/setjmp.S @@ -0,0 +1,66 @@ +/*
- 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
- */
+#include <asm/global_data.h> +#include <asm/msr-index.h> +#include <asm/processor-flags.h>
I believe the above 3 includes are not needed.
+#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 1, %eaxThis should be $1.
But I still think the setjmp/longjump codes in efi_loader is wrong. We should have longjump() to pass the return value to setjmp().
Why? Where's the difference?
longjump() does not have the setjmp() return value as the parameter, which concerns me as it does not conform to the standard longjump() implementation. This v2 hardcoded the return value to 1, which makes the following logic work in efi_loader.
if (setjmp(&info->exit_jmp)) { /* We returned from the child image */ return EFI_EXIT(info->exit_status); }
If we want such a simplified implementation in efi_loader, we should probably rename these functions to efi_setjmp() and efi_longjump().
Ah, I see. The second parameter to longjmp() should be the the return value after the jump - which the code above actually implements. And then it clobbers it with a 1.
I guess that's my fault, because I skipped the second argument bit in my longjmp header definition. Please just add it to the longjmp prototype and explicitly pass "1" as return value in (don't forget to adapt the prototypes in arch/arm/include/asm/setjmp.h). I can fix up the arm code later to actually pass the argument.
Would you mind doing the fix-up patch? I don't think we should change the prototype separate from the ARM code.
Yes, we should fix the longjump() declaration as well as the ARM codes first. Then rework this x86 implementation to handle return value in the assembly codes.
Regards, Bin
participants (4)
-
Alexander Graf -
Bin Meng -
Simon Glass -
Tom Rini