[U-Boot] [PATCH v2 1/8] x86: Add implementations of setjmp() and longjmp()

[U-Boot] [PATCH v2 1/8] x86: Add implementations of setjmp() and longjmp()

[Simon Glass]

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
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH v2 2/8] efi: Use asmlinkage for EFIAPI

[Simon Glass]

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;
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH v2 2/8] efi: Use asmlinkage for EFIAPI

[Bin Meng]

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>

[U-Boot] [U-Boot,v2,2/8] efi: Use asmlinkage for EFIAPI

[Alexander Graf]

> 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 

[U-Boot] [PATCH v2 3/8] efi: Fix missing EFIAPI specifiers

[Simon Glass]

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)
 {
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [U-Boot,v2,3/8] efi: Fix missing EFIAPI specifiers

[Alexander Graf]

> 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 

[U-Boot] [PATCH v2 4/8] x86: Tidy up selection of building the EFI stub

[Simon Glass]

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
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH v2 4/8] x86: Tidy up selection of building the EFI stub

[Bin Meng]

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

[U-Boot] [PATCH v2 4/8] x86: Tidy up selection of building the EFI stub

[Simon Glass]

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

[U-Boot] [PATCH v2 4/8] x86: Tidy up selection of building the EFI stub

[Bin Meng]

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

[U-Boot] [PATCH v2 4/8] x86: Tidy up selection of building the EFI stub

[Simon Glass]

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

[U-Boot] [PATCH v2 4/8] x86: Tidy up selection of building the EFI stub

[Bin Meng]

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

[U-Boot] [PATCH v2 4/8] x86: Tidy up selection of building the EFI stub

[Simon Glass]

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

[U-Boot] [PATCH v2 4/8] x86: Tidy up selection of building the EFI stub

[Bin Meng]

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

[U-Boot] [PATCH v2 4/8] x86: Tidy up selection of building the EFI stub

[Alexander Graf]

> 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

[U-Boot] [PATCH v2 4/8] x86: Tidy up selection of building the EFI stub

[Bin Meng]

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

[U-Boot] [PATCH v2 4/8] x86: Tidy up selection of building the EFI stub

[Alexander Graf]

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

[U-Boot] [PATCH v2 4/8] x86: Tidy up selection of building the EFI stub

[Bin Meng]

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

[U-Boot] [PATCH v2 4/8] x86: Tidy up selection of building the EFI stub

[Alexander Graf]

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

[U-Boot] [PATCH v2 4/8] x86: Tidy up selection of building the EFI stub

[Bin Meng]

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

[U-Boot] [PATCH v2 5/8] arm: efi: Add a hello world test program

[Simon Glass]

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 at 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 at 7jN6g}jJ%NC{EkNc#PIbt=(FN{
z+b|g!T||f<V1pfP)g;KAC#2+Dd`)&|%}`toS-zy at 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 at 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 at 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 at ZSmk
zf4oxy{_^+az<9qr4j7-_iAf+MF&W3f|5Z(d|9jxC7$zj`9QEpN?oh8Lm{)5OK>M$E
zayyTP;<zS!<GItQoess5ib*7tK{evTyFB8z5|Q@2NE>hdzG&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 at 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 at 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 at b+IIJ+oogN*7 at Xtyu*WJce@
zO3K&JNjXZ(*6M1;lCG@Q?HZF`AvzNLUKgt~kEIAYooQ^1jvrGA>veHg^tyfTuG1OD
zI9=12RM at K9IOfnjK4#S|@si-NuCyx+pseYRyP`z7{7 at 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 at 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 at 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 at 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 at h6?hE+KUwqi^XwIcSMjiM&FVr}nFlKeveb at uSd{-hIM@*=mV5N%!PfYX5VKYJ|JK
z_lUl4m=|A(Ts4>TS at 3?bcf-86NxD at 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 at s>}
zf(>JIjj6}d`mvu?ruEV_n=oEIM0%JUieLNXE1!j at 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 at 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 at +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 at 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 at 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 at 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 at y+NSQ+&d?gPvy{YNtAC4XYL&|9e65o^g4%oqm(`7C%(vi6i;YM
zGP*smfDnXo!-PUCcTV8tS1G~$?cb?PwGZv at Tqo+&;{7xgaZB%==ubm`*5A at L#t7Ly
zVFbO5rS<v=tO@YkIU(M4!7CB>!e7qeCj$-b`#$Z1K4j20 at ja1OT_C2%=i#t2hWGz~
zrz`^==9Ys#zYB*a8O{`tl2m^}R2J@^DbhNX8sk}ndnO>|BtqI|mwy at 86eyMY{2zlR
zHQ(i5j6Mn1BrJu^czk_z8QT<Kgu{UpypO23qgVBtAcvg3TKbnh|6<c&zk(dSS`FP~
z9JQ at 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 at K!#3mpXl2!a at 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 at 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 at F*+N)iCN|>0KNsq|@O7hZ1qJf-?C&ehgb7snt at 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 at +_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 at 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 at H+R^ro9^z~;=0!xX?l*#- at 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 at 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 at f9+i>ln+OW7mw83$t5rN0LIy at 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 at 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 at 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 at 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     $<
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH v2 5/8] arm: efi: Add a hello world test program

[Alexander Graf]

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

[U-Boot] [PATCH v2 5/8] arm: efi: Add a hello world test program

[Tom Rini]

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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160927/f1f71347/attachment.sig>

[U-Boot] [PATCH v2 5/8] arm: efi: Add a hello world test program

[Simon Glass]

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

[U-Boot] [PATCH v2 5/8] arm: efi: Add a hello world test program

[Alexander Graf]

> 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

[U-Boot] [PATCH v2 5/8] arm: efi: Add a hello world test program

[Simon Glass]

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

[U-Boot] [PATCH v2 5/8] arm: efi: Add a hello world test program

[Alexander Graf]

> 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

[U-Boot] [PATCH v2 5/8] arm: efi: Add a hello world test program

[Simon Glass]

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

[U-Boot] [PATCH v2 5/8] arm: efi: Add a hello world test program

[Alexander Graf]

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

[U-Boot] [PATCH v2 5/8] arm: efi: Add a hello world test program

[Simon Glass]

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

[U-Boot] [PATCH v2 5/8] arm: efi: Add a hello world test program

[Bin Meng]

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 at 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_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.
> +

[snip]

Regards,
Bin

[U-Boot] [PATCH v2 6/8] x86: efi: Add EFI loader support for x86

[Simon Glass]

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
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [U-Boot, v2, 6/8] x86: efi: Add EFI loader support for x86

[Alexander Graf]

> 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 

[U-Boot] [PATCH v2 7/8] x86: efi: Add a hello world test program

[Simon Glass]

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 at 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 at oHXgx5cl1`L)7`8tfROv|
zL+3#_Qy#kaestHPE4^GOr+WMsA9!#}%dw2GB1J4vT8zzZwF9n^htw04GCm+U66qal
zR6C|ID23ht1b3 at 8Oh6RoikzDR#8Gs9A7yj~c9E%LpbhLnPrX5(cS6U0s>3%y$61WW
z={S=m7_Xy>&TpnVj%Rf|haNC(+z1^{OoJWAR3<CJkr;*@;fXpzS%M;$JI9XKsVv=&
zlUW^W>3og at 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 at 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 at 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 at RtSY`m0oZ%7QV@%PY^QXCuq3=)#7
zrPLBO{@>_IsikcEBQj@`Qq|jUl2R5{x=$ls&c=VPQJC3yHD;1?rPMk${u^yB#Ky at 5
zEv0<0pN$_Tit445n~mR!8)y4Bh#m1qrxOa8w-}v~A8I2n8=nCRDb>ox*J_=6+4#4>
zKC)d~kFoJ{*tPAPl-kC|8#N?RP at w9RQf=sfXJnm5Dzb4i*4gi#*W`4v at f(SpGA!;o
z%^1;r4M{{gZ5TPJJ at 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 at i<5IQq?xG_
z`IPitzI4U#vnBI~NB>j$19xdKkLBMY0M^PU2GwZ`4ZHb60>HcjxY#`gC1dfWS8Bx@
zq?380Z%e;pj^M;FqwoCtvgA_vls4G)USTb^_zHtx at 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 at 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 at 8QmVyyu>@beY at EDYiWa7a%_t
zNU7=D=PI_uUW|HLUaEeZO*{r9K;aC$m8<eGO(mCcKK7Kl|D}<C1id2eGl*J($3fRF
zn>m at w25D8xx8ya(Bo>iqIBJJvT at sNp^1wI|ExL#s#zf54M0^5&Ld3`)@w`Sf92sGW
ztd at O-mTTQN6wa@Rtd=R^=m%}&Fh~@~J_D(6ij- at h5_XszMM>dWaCBM8 at 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 at 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 at 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 at 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 at d+
zQKm5-=|V{lf6 at 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 at 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 at O=kMi
zNL^xB2zL0V1bdA&;MM-Vw62U3 at N`ClVF7|lOFe7WZ(48S^o_rrGk2QlpT)%0)Np2|
zArg*7rq~pH{u*0Wx;hdKemfEtydjq#{33~U;!m at 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 at 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 at 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 at 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 at fu^GU8Erbp
zFmupeMt=dGOVFy&R- at 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 at 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 at 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 at 2(f`(g5KfBAn>VGyk)BzNO64YzbNR+kR at 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 at 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
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH v2 8/8] x86: Enable EFI loader support

[Simon Glass]

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)
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH v2 1/8] x86: Add implementations of setjmp() and longjmp()

[Bin Meng]

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), %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
> --

Regards,
Bin

[U-Boot] [PATCH v2 1/8] x86: Add implementations of setjmp() and longjmp()

[Alexander Graf]

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, %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().

Why? Where's the difference?


Alex

[U-Boot] [PATCH v2 1/8] x86: Add implementations of setjmp() and longjmp()

[Bin Meng]

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, %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().
>
> 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

[U-Boot] [PATCH v2 1/8] x86: Add implementations of setjmp() and longjmp()

[Alexander Graf]

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, %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().
>>
>> 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

[U-Boot] [PATCH v2 1/8] x86: Add implementations of setjmp() and longjmp()

[Simon Glass]

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, %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().
>>>
>>> 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

[U-Boot] [PATCH v2 1/8] x86: Add implementations of setjmp() and longjmp()

[Bin Meng]

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, %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().
>>>>
>>>> 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