[U-Boot] [PATCH 00/16] efi: Enable basic sandbox support for EFI loader

[U-Boot] [PATCH 00/16] efi: Enable basic sandbox support for EFI loader

[Simon Glass]

A limitation of the EFI loader at present is that it does not build with
sandbox. This makes it hard to write tests, since sandbox is used for most
testing in U-Boot.

This series enables the EFI loader feature. It allows sandbox to build and
run a trivial function which calls the EFI API to output a message.

Much work remains but this should serve as a basis for adding tests more
easily for EFI loader.

This series sits on top of Heinrich's recent EFI test series. It is
available at u-boot-dm/efi-working


Simon Glass (16):
  efi: Update efi_smbios_register() to return error code
  efi: Move the init check inside efi_init_obj_list()
  efi: Add error checking for efi_init_obj_list()
  efi: Add a TODO to efi_init_obj_list()
  efi: Correct header order in efi_memory
  efi: sandbox: Adjust memory setup for sandbox
  sandbox: smbios: Update to support sandbox
  sandbox: Add a setjmp() implementation
  efi: sandbox: Add required linker sections
  efi: sandbox: Add distroboot support
  Define board_quiesce_devices() in a shared location
  Add a comment for board_quiesce_devices()
  efi: sandbox: Add relocation constants
  efi: Add a comment about duplicated ELF constants
  efi: sandbox: Enable EFI loader builder for sandbox
  efi: sandbox: Add a simple 'bootefi test' command

 arch/arm/include/asm/u-boot-arm.h |  1 -
 arch/sandbox/cpu/cpu.c            | 13 ++++++++++
 arch/sandbox/cpu/os.c             | 17 ++++++++++++
 arch/sandbox/cpu/u-boot.lds       | 29 +++++++++++++++++++++
 arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++
 arch/sandbox/lib/Makefile         |  2 +-
 arch/sandbox/lib/sections.c       | 12 +++++++++
 arch/x86/include/asm/u-boot-x86.h |  1 -
 arch/x86/lib/bootm.c              |  4 ---
 cmd/bootefi.c                     | 54 ++++++++++++++++++++++++++++++++++-----
 common/bootm.c                    |  4 +++
 configs/sandbox_defconfig         |  1 +
 include/bootm.h                   |  8 ++++++
 include/config_distro_bootcmd.h   |  2 +-
 include/efi_loader.h              | 13 ++++++++--
 include/os.h                      | 21 +++++++++++++++
 lib/efi_loader/Kconfig            | 12 ++++++++-
 lib/efi_loader/Makefile           |  1 +
 lib/efi_loader/efi_boottime.c     |  4 +++
 lib/efi_loader/efi_memory.c       | 33 +++++++++++++-----------
 lib/efi_loader/efi_runtime.c      |  7 +++++
 lib/efi_loader/efi_smbios.c       |  6 +++--
 lib/efi_loader/efi_test.c         | 17 ++++++++++++
 lib/smbios.c                      | 38 ++++++++++++++++++++-------
 24 files changed, 277 insertions(+), 44 deletions(-)
 create mode 100644 arch/sandbox/include/asm/setjmp.h
 create mode 100644 arch/sandbox/lib/sections.c
 create mode 100644 lib/efi_loader/efi_test.c

-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 01/16] efi: Update efi_smbios_register() to return error code

[Simon Glass]

This function can fail but gives no indication of failure. Update it to
return an error when something goes wrong.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/efi_loader.h        | 10 ++++++++--
 lib/efi_loader/efi_smbios.c |  6 ++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 2051fc994e..79d2dad22c 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -150,8 +150,14 @@ int efi_disk_register(void);
 int efi_gop_register(void);
 /* Called by bootefi to make the network interface available */
 int efi_net_register(void **handle);
-/* Called by bootefi to make SMBIOS tables available */
-void efi_smbios_register(void);
+/**
+ * efi_smbios_register() - write out SMBIOS tables
+ *
+ * Called by bootefi to make SMBIOS tables available
+ *
+ * @return 0 if OK, -ENOMEM if no memory is available for the tables
+ */
+int efi_smbios_register(void);
 
 /* Called by networking code to memorize the dhcp ack package */
 void efi_net_set_dhcp_ack(void *pkt, int len);
diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
index ac412e7362..3b87294dc3 100644
--- a/lib/efi_loader/efi_smbios.c
+++ b/lib/efi_loader/efi_smbios.c
@@ -13,7 +13,7 @@
 
 static const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;
 
-void efi_smbios_register(void)
+int efi_smbios_register(void)
 {
 	/* Map within the low 32 bits, to allow for 32bit SMBIOS tables */
 	uint64_t dmi = 0xffffffff;
@@ -22,11 +22,13 @@ void efi_smbios_register(void)
 	int memtype = EFI_RUNTIME_SERVICES_DATA;
 
 	if (efi_allocate_pages(1, memtype, pages, &dmi) != EFI_SUCCESS)
-		return;
+		return -ENOMEM;
 
 	/* Generate SMBIOS tables */
 	write_smbios_table(dmi);
 
 	/* And expose them to our EFI payload */
 	efi_install_configuration_table(&smbios_guid, (void*)(uintptr_t)dmi);
+
+	return 0;
 }
-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 02/16] efi: Move the init check inside efi_init_obj_list()

[Simon Glass]

Rather than having the caller check this variable and the callee set it,
move all access to the variable inside the function. This reduces the
logic needed to call efi_init_obj_list().

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 cmd/bootefi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index abfab8fa98..2c9d31c5eb 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -109,6 +109,8 @@ static struct efi_object bootefi_device_obj = {
 /* Initialize and populate EFI object list */
 static void efi_init_obj_list(void)
 {
+	if (efi_obj_list_initalized)
+		return;
 	efi_obj_list_initalized = 1;
 
 	list_add_tail(&loaded_image_info_obj.link, &efi_obj_list);
@@ -256,8 +258,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
 		return -ENOENT;
 
 	/* Initialize and populate EFI object list */
-	if (!efi_obj_list_initalized)
-		efi_init_obj_list();
+	efi_init_obj_list();
 
 	/* Call our payload! */
 	debug("%s:%d Jumping to 0x%lx\n", __func__, __LINE__, (long)entry);
@@ -311,8 +312,7 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		 */
 		efi_save_gd();
 		/* Initialize and populate EFI object list */
-		if (!efi_obj_list_initalized)
-			efi_init_obj_list();
+		efi_init_obj_list();
 		loaded_image_info.device_handle = bootefi_device_path;
 		loaded_image_info.file_path = bootefi_image_path;
 		return efi_selftest(&loaded_image_info, &systab);
-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 03/16] efi: Add error checking for efi_init_obj_list()

[Simon Glass]

This function calls a function which can fail. Print a message in this
case and abort the boot, rather than silently continuing to boot, which
will certainly fail.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 cmd/bootefi.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 2c9d31c5eb..9aa588eb1b 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -106,11 +106,17 @@ static struct efi_object bootefi_device_obj = {
 	},
 };
 
-/* Initialize and populate EFI object list */
-static void efi_init_obj_list(void)
+/**
+ * efi_init_obj_list() - Initialize and populate EFI object list
+ *
+ * @return 0 if OK, -ve on error (in which case it prints a message)
+ */
+static int efi_init_obj_list(void)
 {
+	int ret;
+
 	if (efi_obj_list_initalized)
-		return;
+		return 0;
 	efi_obj_list_initalized = 1;
 
 	list_add_tail(&loaded_image_info_obj.link, &efi_obj_list);
@@ -132,12 +138,19 @@ static void efi_init_obj_list(void)
 		loaded_image_info.device_handle = bootefi_device_path;
 #endif
 #ifdef CONFIG_GENERATE_SMBIOS_TABLE
-	efi_smbios_register();
+	ret = efi_smbios_register();
+	if (ret)
+		goto error;
 #endif
 
 	/* Initialize EFI runtime services */
 	efi_reset_system_init();
 	efi_get_time_init();
+
+	return 0;
+error:
+	printf("Error: Cannot set up EFI object list (err=%d)\n", ret);
+	return ret;
 }
 
 static void *copy_fdt(void *fdt)
@@ -219,6 +232,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
 	ulong fdt_pages, fdt_size, fdt_start, fdt_end;
 	const efi_guid_t fdt_guid = EFI_FDT_GUID;
 	bootm_headers_t img = { 0 };
+	int ret;
 
 	/*
 	 * gd lives in a fixed register which may get clobbered while we execute
@@ -258,7 +272,9 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
 		return -ENOENT;
 
 	/* Initialize and populate EFI object list */
-	efi_init_obj_list();
+	ret = efi_init_obj_list();
+	if (ret)
+		return ret;
 
 	/* Call our payload! */
 	debug("%s:%d Jumping to 0x%lx\n", __func__, __LINE__, (long)entry);
@@ -312,7 +328,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		 */
 		efi_save_gd();
 		/* Initialize and populate EFI object list */
-		efi_init_obj_list();
+		if (efi_init_obj_list())
+			return CMD_RET_FAILURE;
+
 		loaded_image_info.device_handle = bootefi_device_path;
 		loaded_image_info.file_path = bootefi_image_path;
 		return efi_selftest(&loaded_image_info, &systab);
-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 04/16] efi: Add a TODO to efi_init_obj_list()

[Simon Glass]

This function repeats data structures provided by driver model. They are
only created once so can be stale if the EFI loader is called twice (e.g.
for testing or on boot failure).

Add a TODO to address this. It should be possible to attach EFI devices
and data structures to driver-model devices and avoid having a parallel
set of data structures.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 cmd/bootefi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 9aa588eb1b..ee07733e3e 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -109,6 +109,10 @@ static struct efi_object bootefi_device_obj = {
 /**
  * efi_init_obj_list() - Initialize and populate EFI object list
  *
+ * TODO(sjg at chromium.org): Move this to a dynamic list based on driver model,
+ * so that it does not need to be created before running EFI applications
+ * and updates when devices change.
+ *
  * @return 0 if OK, -ve on error (in which case it prints a message)
  */
 static int efi_init_obj_list(void)
-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 05/16] efi: Correct header order in efi_memory

[Simon Glass]

The headers are not in the correct order. Fix this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 lib/efi_loader/efi_memory.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index 9e079f1fa3..ad3d277be6 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -8,12 +8,11 @@
 
 #include <common.h>
 #include <efi_loader.h>
+#include <inttypes.h>
 #include <malloc.h>
+#include <watchdog.h>
 #include <asm/global_data.h>
-#include <libfdt_env.h>
 #include <linux/list_sort.h>
-#include <inttypes.h>
-#include <watchdog.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 06/16] efi: sandbox: Adjust memory setup for sandbox

[Simon Glass]

With sandbox the U-Boot code is not mapped into the sandbox memory range
so does not need to be excluded when allocating EFI memory. Update the EFI
memory init code to take account of that.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 lib/efi_loader/efi_memory.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index ad3d277be6..c1a080e2e9 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -459,18 +459,22 @@ int efi_memory_init(void)
 
 	efi_add_known_memory();
 
-	/* Add U-Boot */
-	uboot_start = (gd->start_addr_sp - uboot_stack_size) & ~EFI_PAGE_MASK;
-	uboot_pages = (gd->ram_top - uboot_start) >> EFI_PAGE_SHIFT;
-	efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA, false);
-
-	/* Add Runtime Services */
-	runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
-	runtime_end = (ulong)&__efi_runtime_stop;
-	runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
-	runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
-	efi_add_memory_map(runtime_start, runtime_pages,
-			   EFI_RUNTIME_SERVICES_CODE, false);
+	if (!IS_ENABLED(CONFIG_SANDBOX)) {
+		/* Add U-Boot */
+		uboot_start = (gd->start_addr_sp - uboot_stack_size) &
+				~EFI_PAGE_MASK;
+		uboot_pages = (gd->ram_top - uboot_start) >> EFI_PAGE_SHIFT;
+		efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA,
+				   false);
+
+		/* Add Runtime Services */
+		runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
+		runtime_end = (ulong)&__efi_runtime_stop;
+		runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
+		runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
+		efi_add_memory_map(runtime_start, runtime_pages,
+				   EFI_RUNTIME_SERVICES_CODE, false);
+	}
 
 #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
 	/* Request a 32bit 64MB bounce buffer region */
-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 07/16] sandbox: smbios: Update to support sandbox

[Simon Glass]

At present this code casts addresses to pointers so cannot be used with
sandbox. Update it to use mapmem instead.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 lib/smbios.c | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/lib/smbios.c b/lib/smbios.c
index 8f19ad89c1..56481d448d 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -7,6 +7,7 @@
  */
 
 #include <common.h>
+#include <mapmem.h>
 #include <smbios.h>
 #include <tables_csum.h>
 #include <version.h>
@@ -75,9 +76,10 @@ static int smbios_string_table_len(char *start)
 
 static int smbios_write_type0(ulong *current, int handle)
 {
-	struct smbios_type0 *t = (struct smbios_type0 *)*current;
+	struct smbios_type0 *t;
 	int len = sizeof(struct smbios_type0);
 
+	t = map_sysmem(*current, len);
 	memset(t, 0, sizeof(struct smbios_type0));
 	fill_smbios_header(t, SMBIOS_BIOS_INFORMATION, len, handle);
 	t->vendor = smbios_add_string(t->eos, "U-Boot");
@@ -104,16 +106,18 @@ static int smbios_write_type0(ulong *current, int handle)
 
 	len = t->length + smbios_string_table_len(t->eos);
 	*current += len;
+	unmap_sysmem(t);
 
 	return len;
 }
 
 static int smbios_write_type1(ulong *current, int handle)
 {
-	struct smbios_type1 *t = (struct smbios_type1 *)*current;
+	struct smbios_type1 *t;
 	int len = sizeof(struct smbios_type1);
 	char *serial_str = env_get("serial#");
 
+	t = map_sysmem(*current, len);
 	memset(t, 0, sizeof(struct smbios_type1));
 	fill_smbios_header(t, SMBIOS_SYSTEM_INFORMATION, len, handle);
 	t->manufacturer = smbios_add_string(t->eos, CONFIG_SMBIOS_MANUFACTURER);
@@ -125,15 +129,17 @@ static int smbios_write_type1(ulong *current, int handle)
 
 	len = t->length + smbios_string_table_len(t->eos);
 	*current += len;
+	unmap_sysmem(t);
 
 	return len;
 }
 
 static int smbios_write_type2(ulong *current, int handle)
 {
-	struct smbios_type2 *t = (struct smbios_type2 *)*current;
+	struct smbios_type2 *t;
 	int len = sizeof(struct smbios_type2);
 
+	t = map_sysmem(*current, len);
 	memset(t, 0, sizeof(struct smbios_type2));
 	fill_smbios_header(t, SMBIOS_BOARD_INFORMATION, len, handle);
 	t->manufacturer = smbios_add_string(t->eos, CONFIG_SMBIOS_MANUFACTURER);
@@ -143,15 +149,17 @@ static int smbios_write_type2(ulong *current, int handle)
 
 	len = t->length + smbios_string_table_len(t->eos);
 	*current += len;
+	unmap_sysmem(t);
 
 	return len;
 }
 
 static int smbios_write_type3(ulong *current, int handle)
 {
-	struct smbios_type3 *t = (struct smbios_type3 *)*current;
+	struct smbios_type3 *t;
 	int len = sizeof(struct smbios_type3);
 
+	t = map_sysmem(*current, len);
 	memset(t, 0, sizeof(struct smbios_type3));
 	fill_smbios_header(t, SMBIOS_SYSTEM_ENCLOSURE, len, handle);
 	t->manufacturer = smbios_add_string(t->eos, CONFIG_SMBIOS_MANUFACTURER);
@@ -163,6 +171,7 @@ static int smbios_write_type3(ulong *current, int handle)
 
 	len = t->length + smbios_string_table_len(t->eos);
 	*current += len;
+	unmap_sysmem(t);
 
 	return len;
 }
@@ -201,9 +210,10 @@ static void smbios_write_type4_dm(struct smbios_type4 *t)
 
 static int smbios_write_type4(ulong *current, int handle)
 {
-	struct smbios_type4 *t = (struct smbios_type4 *)*current;
+	struct smbios_type4 *t;
 	int len = sizeof(struct smbios_type4);
 
+	t = map_sysmem(*current, len);
 	memset(t, 0, sizeof(struct smbios_type4));
 	fill_smbios_header(t, SMBIOS_PROCESSOR_INFORMATION, len, handle);
 	t->processor_type = SMBIOS_PROCESSOR_TYPE_CENTRAL;
@@ -217,32 +227,37 @@ static int smbios_write_type4(ulong *current, int handle)
 
 	len = t->length + smbios_string_table_len(t->eos);
 	*current += len;
+	unmap_sysmem(t);
 
 	return len;
 }
 
 static int smbios_write_type32(ulong *current, int handle)
 {
-	struct smbios_type32 *t = (struct smbios_type32 *)*current;
+	struct smbios_type32 *t;
 	int len = sizeof(struct smbios_type32);
 
+	t = map_sysmem(*current, len);
 	memset(t, 0, sizeof(struct smbios_type32));
 	fill_smbios_header(t, SMBIOS_SYSTEM_BOOT_INFORMATION, len, handle);
 
 	*current += len;
+	unmap_sysmem(t);
 
 	return len;
 }
 
 static int smbios_write_type127(ulong *current, int handle)
 {
-	struct smbios_type127 *t = (struct smbios_type127 *)*current;
+	struct smbios_type127 *t;
 	int len = sizeof(struct smbios_type127);
 
+	t = map_sysmem(*current, len);
 	memset(t, 0, sizeof(struct smbios_type127));
 	fill_smbios_header(t, SMBIOS_END_OF_TABLE, len, handle);
 
 	*current += len;
+	unmap_sysmem(t);
 
 	return len;
 }
@@ -271,7 +286,7 @@ ulong write_smbios_table(ulong addr)
 	/* 16 byte align the table address */
 	addr = ALIGN(addr, 16);
 
-	se = (struct smbios_entry *)(uintptr_t)addr;
+	se = map_sysmem(addr, sizeof(struct smbios_entry));
 	memset(se, 0, sizeof(struct smbios_entry));
 
 	addr += sizeof(struct smbios_entry);
@@ -280,9 +295,13 @@ ulong write_smbios_table(ulong addr)
 
 	/* populate minimum required tables */
 	for (i = 0; i < ARRAY_SIZE(smbios_write_funcs); i++) {
-		int tmp = smbios_write_funcs[i]((ulong *)&addr, handle++);
+		ulong *ptr = map_sysmem(addr, 0);
+		int tmp;
+
+		tmp = smbios_write_funcs[i](ptr, handle++);
 		max_struct_size = max(max_struct_size, tmp);
 		len += tmp;
+		unmap_sysmem(ptr);
 	}
 
 	memcpy(se->anchor, "_SM_", 4);
@@ -300,6 +319,7 @@ ulong write_smbios_table(ulong addr)
 	isize = sizeof(struct smbios_entry) - SMBIOS_INTERMEDIATE_OFFSET;
 	se->intermediate_checksum = table_compute_checksum(istart, isize);
 	se->checksum = table_compute_checksum(se, sizeof(struct smbios_entry));
+	unmap_sysmem(se);
 
 	return addr;
 }
-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 08/16] sandbox: Add a setjmp() implementation

[Simon Glass]

Add an implementation of setjmp() and longjmp() which rely on the
underlying host C library. Since we cannot know how large the jump buffer
needs to be, pick something that should be suitable and check it at
runtime. At present we need access to the underlying struct as well.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/sandbox/cpu/cpu.c            | 13 +++++++++++++
 arch/sandbox/cpu/os.c             | 17 +++++++++++++++++
 arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++++++++
 include/os.h                      | 21 +++++++++++++++++++++
 4 files changed, 72 insertions(+)
 create mode 100644 arch/sandbox/include/asm/setjmp.h

diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
index 01991049cc..de5862a53b 100644
--- a/arch/sandbox/cpu/cpu.c
+++ b/arch/sandbox/cpu/cpu.c
@@ -9,6 +9,7 @@
 #include <libfdt.h>
 #include <os.h>
 #include <asm/io.h>
+#include <asm/setjmp.h>
 #include <asm/state.h>
 #include <dm/root.h>
 
@@ -154,3 +155,15 @@ ulong timer_get_boot_us(void)
 
 	return (count - base_count) / 1000;
 }
+
+int setjmp(jmp_buf jmp)
+{
+	return os_setjmp((ulong *)jmp, sizeof(jmp));
+}
+
+void longjmp(jmp_buf jmp, int ret)
+{
+	os_longjmp((ulong *)jmp, ret);
+	while (1)
+		;
+}
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index 22d6aab534..909034fa4b 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -7,6 +7,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <getopt.h>
+#include <setjmp.h>
 #include <stdio.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -609,3 +610,19 @@ void os_localtime(struct rtc_time *rt)
 	rt->tm_yday = tm->tm_yday;
 	rt->tm_isdst = tm->tm_isdst;
 }
+
+int os_setjmp(ulong *jmp, int size)
+{
+	if (size < sizeof(jmp_buf)) {
+		printf("setjmp: jmpbuf is too small (%d bytes, need %d)\n",
+		       size, sizeof(jmp_buf));
+		return -ENOSPC;
+	}
+
+	return setjmp((struct __jmp_buf_tag *)jmp);
+}
+
+void os_longjmp(ulong *jmp, int ret)
+{
+	longjmp((struct __jmp_buf_tag *)jmp, ret);
+}
diff --git a/arch/sandbox/include/asm/setjmp.h b/arch/sandbox/include/asm/setjmp.h
new file mode 100644
index 0000000000..e25f50107c
--- /dev/null
+++ b/arch/sandbox/include/asm/setjmp.h
@@ -0,0 +1,21 @@
+/*
+ * (C) Copyright 2016
+ * Alexander Graf <agraf@suse.de>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _SETJMP_H_
+#define _SETJMP_H_	1
+
+struct jmp_buf_data {
+	/* We're not sure how long this should be */
+	ulong data[32];
+};
+
+typedef struct jmp_buf_data jmp_buf[1];
+
+int setjmp(jmp_buf jmp);
+__noreturn void longjmp(jmp_buf jmp, int ret);
+
+#endif /* _SETJMP_H_ */
diff --git a/include/os.h b/include/os.h
index 2bf4bdb1b8..ad1836ac9f 100644
--- a/include/os.h
+++ b/include/os.h
@@ -310,4 +310,25 @@ int os_spl_to_uboot(const char *fname);
  */
 void os_localtime(struct rtc_time *rt);
 
+/**
+ * os_setjmp() - Call setjmp()
+ *
+ * Call the host system's setjmp() function.
+ *
+ * @jmp: Buffer to store current execution state
+ * @size: Size of buffer
+ * @return normal setjmp() value if OK, -ENOSPC if @size is too small
+ */
+int os_setjmp(ulong *jmp, int size);
+
+/**
+ * os_longjmp() - Call longjmp()
+ *
+ * Call the host system's longjmp() function.
+ *
+ * @jmp: Buffer where previous execution state was stored
+ * @ret: Value to pass to longjmp()
+ */
+void os_longjmp(ulong *jmp, int ret);
+
 #endif
-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 09/16] efi: sandbox: Add required linker sections

[Simon Glass]

The EFI loader code requires certain linker sections to exist. Add these
for sandbox so that the EFI loader code will link.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/sandbox/cpu/u-boot.lds | 29 +++++++++++++++++++++++++++++
 arch/sandbox/lib/Makefile   |  2 +-
 arch/sandbox/lib/sections.c | 12 ++++++++++++
 3 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 arch/sandbox/lib/sections.c

diff --git a/arch/sandbox/cpu/u-boot.lds b/arch/sandbox/cpu/u-boot.lds
index 7e92b4ac66..38a74bcaf4 100644
--- a/arch/sandbox/cpu/u-boot.lds
+++ b/arch/sandbox/cpu/u-boot.lds
@@ -19,6 +19,35 @@ SECTIONS
 	__u_boot_sandbox_option_end = .;
 
 	__bss_start = .;
+
+		.__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)
+	}
+
 }
 
 INSERT BEFORE .data;
diff --git a/arch/sandbox/lib/Makefile b/arch/sandbox/lib/Makefile
index 2e7802feac..d2171579b7 100644
--- a/arch/sandbox/lib/Makefile
+++ b/arch/sandbox/lib/Makefile
@@ -7,7 +7,7 @@
 # SPDX-License-Identifier:	GPL-2.0+
 #
 
-obj-y	+= interrupts.o
+obj-y	+= interrupts.o sections.o
 ifndef CONFIG_SPL_BUILD
 obj-$(CONFIG_PCI)	+= pci_io.o
 endif
diff --git a/arch/sandbox/lib/sections.c b/arch/sandbox/lib/sections.c
new file mode 100644
index 0000000000..6455e0f088
--- /dev/null
+++ b/arch/sandbox/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")));
-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 10/16] efi: sandbox: Add distroboot support

[Simon Glass]

With sandbox these values depend on the host system. Let's assume that it
is x86_64 for now.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/config_distro_bootcmd.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
index 9ed6b9892c..11cc771c5d 100644
--- a/include/config_distro_bootcmd.h
+++ b/include/config_distro_bootcmd.h
@@ -233,7 +233,7 @@
 #elif defined(CONFIG_ARM)
 #define BOOTENV_EFI_PXE_ARCH "0xa"
 #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00010:UNDI:003000"
-#elif defined(CONFIG_X86)
+#elif defined(CONFIG_X86) || defined(CONFIG_SANDBOX)
 /* Always assume we're running 64bit */
 #define BOOTENV_EFI_PXE_ARCH "0x7"
 #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00007:UNDI:003000"
-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 11/16] Define board_quiesce_devices() in a shared location

[Simon Glass]

This undocumented function relies on arch-specific code to declare a nop
weak version. Add the weak function in common code instead to avoid having
to duplicate the same function in each arch.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/arm/include/asm/u-boot-arm.h | 1 -
 arch/x86/include/asm/u-boot-x86.h | 1 -
 arch/x86/lib/bootm.c              | 4 ----
 common/bootm.c                    | 4 ++++
 include/bootm.h                   | 2 ++
 lib/efi_loader/efi_boottime.c     | 4 ++++
 6 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/u-boot-arm.h b/arch/arm/include/asm/u-boot-arm.h
index ef4fca68ee..73ccf41f8c 100644
--- a/arch/arm/include/asm/u-boot-arm.h
+++ b/arch/arm/include/asm/u-boot-arm.h
@@ -38,7 +38,6 @@ int	arch_early_init_r(void);
 
 /* board/.../... */
 int	board_init(void);
-void	board_quiesce_devices(void);
 
 /* cpu/.../interrupt.c */
 int	arch_interrupt_init	(void);
diff --git a/arch/x86/include/asm/u-boot-x86.h b/arch/x86/include/asm/u-boot-x86.h
index 187fe5fd8c..d2e1426042 100644
--- a/arch/x86/include/asm/u-boot-x86.h
+++ b/arch/x86/include/asm/u-boot-x86.h
@@ -85,7 +85,6 @@ static inline __attribute__((no_instrument_function)) uint64_t rdtsc(void)
 /* board/... */
 void timer_set_tsc_base(uint64_t new_base);
 uint64_t timer_get_tsc(void);
-void board_quiesce_devices(void);
 
 void quick_ram_check(void);
 
diff --git a/arch/x86/lib/bootm.c b/arch/x86/lib/bootm.c
index ecd4f4e6c6..d9063a76d7 100644
--- a/arch/x86/lib/bootm.c
+++ b/arch/x86/lib/bootm.c
@@ -33,10 +33,6 @@ int arch_fixup_fdt(void *blob)
 	return 0;
 }
 
-__weak void board_quiesce_devices(void)
-{
-}
-
 void bootm_announce_and_cleanup(void)
 {
 	printf("\nStarting kernel ...\n\n");
diff --git a/common/bootm.c b/common/bootm.c
index 32b3ea8e2d..f64742168c 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -47,6 +47,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
 				   char * const argv[], bootm_headers_t *images,
 				   ulong *os_data, ulong *os_len);
 
+__weak void board_quiesce_devices(void)
+{
+}
+
 #ifdef CONFIG_LMB
 static void boot_start_lmb(bootm_headers_t *images)
 {
diff --git a/include/bootm.h b/include/bootm.h
index 49813772ce..76b6ab42e6 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -73,4 +73,6 @@ int bootm_decomp_image(int comp, ulong load, ulong image_start, int type,
 		       void *load_buf, void *image_buf, ulong image_len,
 		       uint unc_len, ulong *load_end);
 
+void board_quiesce_devices(void);
+
 #endif
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index d832f48599..03b97d36ae 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -883,6 +883,10 @@ static void efi_exit_caches(void)
 #endif
 }
 
+__weak void board_quiesce_devices(void)
+{
+}
+
 static efi_status_t EFIAPI efi_exit_boot_services(void *image_handle,
 						  unsigned long map_key)
 {
-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 12/16] Add a comment for board_quiesce_devices()

[Simon Glass]

This exported function should have a comment describing what it does. Also
it should really be removed in favour of device_remove(), which handles
this sort of thing now. Add a comment with a TODO.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/bootm.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/bootm.h b/include/bootm.h
index 76b6ab42e6..caf331f54b 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -73,6 +73,12 @@ int bootm_decomp_image(int comp, ulong load, ulong image_start, int type,
 		       void *load_buf, void *image_buf, ulong image_len,
 		       uint unc_len, ulong *load_end);
 
+/*
+ * boards should define this to disable devices when EFI exits from boot
+ * services.
+ *
+ * TODO(sjg at chromium.org>): Update this to use driver model's device_remove().
+ */
 void board_quiesce_devices(void);
 
 #endif
-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 13/16] efi: sandbox: Add relocation constants

[Simon Glass]

Add these so that we can build the EFI loader for sandbox. The values are
for x86_64 so potentially bogus. But we don't support relocation within
sandbox anyway.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 lib/efi_loader/efi_runtime.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index ad7f3754bd..8ad1d2fc99 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -48,6 +48,9 @@ static efi_status_t __efi_runtime EFIAPI efi_invalid_parameter(void);
 #include <asm/elf.h>
 #define R_RELATIVE	R_386_RELATIVE
 #define R_MASK		0xffULL
+#elif defined(CONFIG_SANDBOX)
+#define R_RELATIVE	8
+#define R_MASK		0xffffffffULL
 #else
 #error Need to add relocation awareness
 #endif
-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 14/16] efi: Add a comment about duplicated ELF constants

[Simon Glass]

These constants are defined in arch-specific code but redefined here. Add
a TODO to clean this up.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 lib/efi_loader/efi_runtime.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 8ad1d2fc99..f52d8d71db 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -37,6 +37,10 @@ static efi_status_t __efi_runtime EFIAPI efi_invalid_parameter(void);
 #define EFI_CACHELINE_SIZE 128
 #endif
 
+/*
+ * TODO(sjg at chromium.org): These defines and structs should come from the elf.
+ * header for each arch (or a generic header) rather than being repeated here.
+ */
 #if defined(CONFIG_ARM64)
 #define R_RELATIVE	1027
 #define R_MASK		0xffffffffULL
-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 15/16] efi: sandbox: Enable EFI loader builder for sandbox

[Simon Glass]

This allows this feature to build within sandbox. This is for testing
purposes only since it is not possible for sandbox to load native code.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 lib/efi_loader/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index d2b6327119..dee0a96a98 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 (ARM || X86) && OF_LIBFDT
+	depends on (ARM || X86 || SANDBOX) && OF_LIBFDT
 	default y
 	help
 	  Select this option if you want to run EFI applications (like grub2)
-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 16/16] efi: sandbox: Add a simple 'bootefi test' command

[Simon Glass]

This jumps to test code which can call directly into the EFI support. It
does not need a separate image so it is easy to write tests with it.

For now the test just outputs a message. To try it:

./sandbox/u-boot -c "bootefi test"
U-Boot 2017.09-00204-g696c9855fe (Sep 17 2017 - 16:43:53 -0600)

DRAM:  128 MiB
MMC:
Using default environment

In:    serial
Out:   serial
Err:   serial
SCSI:  Net:   No ethernet found.
IDE:   Bus 0: not available
Found 0 disks
Hello, world!
Test passed

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 cmd/bootefi.c             | 18 ++++++++++++++++++
 configs/sandbox_defconfig |  1 +
 include/efi_loader.h      |  3 +++
 lib/efi_loader/Kconfig    | 10 ++++++++++
 lib/efi_loader/Makefile   |  1 +
 lib/efi_loader/efi_test.c | 17 +++++++++++++++++
 6 files changed, 50 insertions(+)
 create mode 100644 lib/efi_loader/efi_test.c

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index ee07733e3e..f499103d23 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -323,6 +323,24 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		memcpy((char *)addr, __efi_hello_world_begin, size);
 	} else
 #endif
+	if (IS_ENABLED(CONFIG_BOOTEFI_TEST) && !strcmp(argv[1], "test")) {
+		int ret;
+
+		/* Initialize and populate EFI object list */
+		if (efi_init_obj_list())
+			return CMD_RET_FAILURE;
+
+		loaded_image_info.device_handle = bootefi_device_path;
+		loaded_image_info.file_path = bootefi_image_path;
+		ret = efi_test(&loaded_image_info, &systab);
+		if (ret) {
+			printf("Test failed: err=%d\n", ret);
+			return CMD_RET_FAILURE;
+		}
+		printf("Test passed\n");
+
+		return 0;
+	}
 #ifdef CONFIG_CMD_BOOTEFI_SELFTEST
 	if (!strcmp(argv[1], "selftest")) {
 		/*
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 72600afea8..ab63f639de 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -196,3 +196,4 @@ CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
 CONFIG_UT_ENV=y
 CONFIG_UT_OVERLAY=y
+CONFIG_CMD_BOOTEFI_SELFTEST=y
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 79d2dad22c..43919137b0 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -263,6 +263,9 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
 				 struct efi_system_table *systab);
 #endif
 
+/* Perform EFI tests */
+int efi_test(efi_handle_t image_handle, struct efi_system_table *systab);
+
 #else /* defined(EFI_LOADER) && !defined(CONFIG_SPL_BUILD) */
 
 /* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it out */
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index dee0a96a98..659b2b18f4 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -16,3 +16,13 @@ config EFI_LOADER_BOUNCE_BUFFER
 	  Some hardware does not support DMA to full 64bit addresses. For this
 	  hardware we can create a bounce buffer so that payloads don't have to
 	  worry about platform details.
+
+config BOOTEFI_TEST
+	bool "Provide a test for the EFI loader"
+	depends on EFI_LOADER && SANDBOX
+	default y
+	help
+	  Provides a test of the EFI loader functionality accessed via the
+	  command line ('bootefi test'). This runs within U-Boot so does not
+	  need a separate EFI application to work. It aims to include coverage
+	  of all EFI code which can be accessed within sandbox.
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 30bf343a36..69eb93518d 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -21,3 +21,4 @@ obj-$(CONFIG_DM_VIDEO) += efi_gop.o
 obj-$(CONFIG_PARTITIONS) += efi_disk.o
 obj-$(CONFIG_NET) += efi_net.o
 obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
+obj-$(CONFIG_BOOTEFI_TEST) += efi_test.o
diff --git a/lib/efi_loader/efi_test.c b/lib/efi_loader/efi_test.c
new file mode 100644
index 0000000000..3fdce78b05
--- /dev/null
+++ b/lib/efi_loader/efi_test.c
@@ -0,0 +1,17 @@
+/*
+ * Copyright (c) 2017, Google Inc. All rights reserved.
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#include <common.h>
+#include <efi_api.h>
+
+int efi_test(efi_handle_t image_handle, struct efi_system_table *systable)
+{
+	struct efi_simple_text_output_protocol *con_out = systable->con_out;
+
+	con_out->output_string(con_out, L"Hello, world!\n");
+
+	return 0;
+}
-- 
2.14.1.690.gbb1197296e-goog

[U-Boot] [PATCH 00/16] efi: Enable basic sandbox support for EFI loader

[Heinrich Schuchardt]

On 09/18/2017 12:59 AM, Simon Glass wrote:
> A limitation of the EFI loader at present is that it does not build with
> sandbox. This makes it hard to write tests, since sandbox is used for most
> testing in U-Boot.
> 
> This series enables the EFI loader feature. It allows sandbox to build and
> run a trivial function which calls the EFI API to output a message.
> 
> Much work remains but this should serve as a basis for adding tests more
> easily for EFI loader.
> 
> This series sits on top of Heinrich's recent EFI test series. It is
> available at u-boot-dm/efi-working
> 
> 
> Simon Glass (16):
>   efi: Update efi_smbios_register() to return error code
>   efi: Move the init check inside efi_init_obj_list()
>   efi: Add error checking for efi_init_obj_list()
>   efi: Add a TODO to efi_init_obj_list()
>   efi: Correct header order in efi_memory
>   efi: sandbox: Adjust memory setup for sandbox
>   sandbox: smbios: Update to support sandbox
>   sandbox: Add a setjmp() implementation
>   efi: sandbox: Add required linker sections
>   efi: sandbox: Add distroboot support
>   Define board_quiesce_devices() in a shared location
>   Add a comment for board_quiesce_devices()
>   efi: sandbox: Add relocation constants
>   efi: Add a comment about duplicated ELF constants
>   efi: sandbox: Enable EFI loader builder for sandbox
>   efi: sandbox: Add a simple 'bootefi test' command
> 
>  arch/arm/include/asm/u-boot-arm.h |  1 -
>  arch/sandbox/cpu/cpu.c            | 13 ++++++++++
>  arch/sandbox/cpu/os.c             | 17 ++++++++++++
>  arch/sandbox/cpu/u-boot.lds       | 29 +++++++++++++++++++++
>  arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++
>  arch/sandbox/lib/Makefile         |  2 +-
>  arch/sandbox/lib/sections.c       | 12 +++++++++
>  arch/x86/include/asm/u-boot-x86.h |  1 -
>  arch/x86/lib/bootm.c              |  4 ---
>  cmd/bootefi.c                     | 54 ++++++++++++++++++++++++++++++++++-----
>  common/bootm.c                    |  4 +++
>  configs/sandbox_defconfig         |  1 +
>  include/bootm.h                   |  8 ++++++
>  include/config_distro_bootcmd.h   |  2 +-
>  include/efi_loader.h              | 13 ++++++++--
>  include/os.h                      | 21 +++++++++++++++
>  lib/efi_loader/Kconfig            | 12 ++++++++-
>  lib/efi_loader/Makefile           |  1 +
>  lib/efi_loader/efi_boottime.c     |  4 +++
>  lib/efi_loader/efi_memory.c       | 33 +++++++++++++-----------
>  lib/efi_loader/efi_runtime.c      |  7 +++++
>  lib/efi_loader/efi_smbios.c       |  6 +++--
>  lib/efi_loader/efi_test.c         | 17 ++++++++++++
>  lib/smbios.c                      | 38 ++++++++++++++++++++-------
>  24 files changed, 277 insertions(+), 44 deletions(-)
>  create mode 100644 arch/sandbox/include/asm/setjmp.h
>  create mode 100644 arch/sandbox/lib/sections.c
>  create mode 100644 lib/efi_loader/efi_test.c
> 
Thanks for enabling efi_loader on sandbox. That will make many things
easier.

Unfortunately
efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
                                 struct efi_system_table *systab)
{
...
        boottime = systable->boottime;
...
        ret = boottime->allocate_pool(EFI_BOOT_SERVICES_DATA, map_size,
                                      (void **)&memory_map);
leads to a segmentation fault:

=> bootefi selftest

Testing EFI API implementation

Number of tests to execute: 3
<snip>
Setting up 'ExitBootServices'
Setting up 'ExitBootServices' succeeded
Segmentation fault
user at workstation:~/workspace/u-boot-odroid-c2/denx$

The problem does not exist with qemu-x86_defconfig without your patches.

qemu-x86_defconfig cannot be built with you patches:

  UPD     include/generated/asm-offsets.h
sh: echo: I/O error
Kbuild:47: recipe for target 'include/generated/generic-asm-offsets.h'
failed
make[1]: *** [include/generated/generic-asm-offsets.h] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1332: recipe for target 'prepare0' failed
make: *** [prepare0] Error 2

Best regards

Heinrich

[U-Boot] [PATCH 00/16] efi: Enable basic sandbox support for EFI loader

[Simon Glass]

Hi Heinrich,

On 17 September 2017 at 21:48, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 09/18/2017 12:59 AM, Simon Glass wrote:
>> A limitation of the EFI loader at present is that it does not build with
>> sandbox. This makes it hard to write tests, since sandbox is used for most
>> testing in U-Boot.
>>
>> This series enables the EFI loader feature. It allows sandbox to build and
>> run a trivial function which calls the EFI API to output a message.
>>
>> Much work remains but this should serve as a basis for adding tests more
>> easily for EFI loader.
>>
>> This series sits on top of Heinrich's recent EFI test series. It is
>> available at u-boot-dm/efi-working
>>
>>
>> Simon Glass (16):
>>   efi: Update efi_smbios_register() to return error code
>>   efi: Move the init check inside efi_init_obj_list()
>>   efi: Add error checking for efi_init_obj_list()
>>   efi: Add a TODO to efi_init_obj_list()
>>   efi: Correct header order in efi_memory
>>   efi: sandbox: Adjust memory setup for sandbox
>>   sandbox: smbios: Update to support sandbox
>>   sandbox: Add a setjmp() implementation
>>   efi: sandbox: Add required linker sections
>>   efi: sandbox: Add distroboot support
>>   Define board_quiesce_devices() in a shared location
>>   Add a comment for board_quiesce_devices()
>>   efi: sandbox: Add relocation constants
>>   efi: Add a comment about duplicated ELF constants
>>   efi: sandbox: Enable EFI loader builder for sandbox
>>   efi: sandbox: Add a simple 'bootefi test' command
>>
>>  arch/arm/include/asm/u-boot-arm.h |  1 -
>>  arch/sandbox/cpu/cpu.c            | 13 ++++++++++
>>  arch/sandbox/cpu/os.c             | 17 ++++++++++++
>>  arch/sandbox/cpu/u-boot.lds       | 29 +++++++++++++++++++++
>>  arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++
>>  arch/sandbox/lib/Makefile         |  2 +-
>>  arch/sandbox/lib/sections.c       | 12 +++++++++
>>  arch/x86/include/asm/u-boot-x86.h |  1 -
>>  arch/x86/lib/bootm.c              |  4 ---
>>  cmd/bootefi.c                     | 54 ++++++++++++++++++++++++++++++++++-----
>>  common/bootm.c                    |  4 +++
>>  configs/sandbox_defconfig         |  1 +
>>  include/bootm.h                   |  8 ++++++
>>  include/config_distro_bootcmd.h   |  2 +-
>>  include/efi_loader.h              | 13 ++++++++--
>>  include/os.h                      | 21 +++++++++++++++
>>  lib/efi_loader/Kconfig            | 12 ++++++++-
>>  lib/efi_loader/Makefile           |  1 +
>>  lib/efi_loader/efi_boottime.c     |  4 +++
>>  lib/efi_loader/efi_memory.c       | 33 +++++++++++++-----------
>>  lib/efi_loader/efi_runtime.c      |  7 +++++
>>  lib/efi_loader/efi_smbios.c       |  6 +++--
>>  lib/efi_loader/efi_test.c         | 17 ++++++++++++
>>  lib/smbios.c                      | 38 ++++++++++++++++++++-------
>>  24 files changed, 277 insertions(+), 44 deletions(-)
>>  create mode 100644 arch/sandbox/include/asm/setjmp.h
>>  create mode 100644 arch/sandbox/lib/sections.c
>>  create mode 100644 lib/efi_loader/efi_test.c
>>
> Thanks for enabling efi_loader on sandbox. That will make many things
> easier.
>
> Unfortunately
> efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>                                  struct efi_system_table *systab)
> {
> ...
>         boottime = systable->boottime;
> ...
>         ret = boottime->allocate_pool(EFI_BOOT_SERVICES_DATA, map_size,
>                                       (void **)&memory_map);
> leads to a segmentation fault:
>
> => bootefi selftest
>
> Testing EFI API implementation
>
> Number of tests to execute: 3
> <snip>
> Setting up 'ExitBootServices'
> Setting up 'ExitBootServices' succeeded
> Segmentation fault
> user at workstation:~/workspace/u-boot-odroid-c2/denx$
>
> The problem does not exist with qemu-x86_defconfig without your patches.
>
> qemu-x86_defconfig cannot be built with you patches:
>
>   UPD     include/generated/asm-offsets.h
> sh: echo: I/O error
> Kbuild:47: recipe for target 'include/generated/generic-asm-offsets.h'
> failed
> make[1]: *** [include/generated/generic-asm-offsets.h] Error 1
> make[1]: *** Waiting for unfinished jobs....
> Makefile:1332: recipe for target 'prepare0' failed
> make: *** [prepare0] Error 2

Are you able to bisect this to the commit which causes the problem.
I've had a look through and cannot figure it out my inspection.
Otherwise I should be able to look at it on Tuesday.

Regards,
Simon

[U-Boot] [PATCH 01/16] efi: Update efi_smbios_register() to return error code

[Heinrich Schuchardt]

On 09/18/2017 12:59 AM, Simon Glass wrote:
> This function can fail but gives no indication of failure. Update it to
> return an error when something goes wrong.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  include/efi_loader.h        | 10 ++++++++--
>  lib/efi_loader/efi_smbios.c |  6 ++++--
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 2051fc994e..79d2dad22c 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -150,8 +150,14 @@ int efi_disk_register(void);
>  int efi_gop_register(void);
>  /* Called by bootefi to make the network interface available */
>  int efi_net_register(void **handle);
> -/* Called by bootefi to make SMBIOS tables available */
> -void efi_smbios_register(void);
> +/**
> + * efi_smbios_register() - write out SMBIOS tables
> + *
> + * Called by bootefi to make SMBIOS tables available
> + *
> + * @return 0 if OK, -ENOMEM if no memory is available for the tables
> + */
> +int efi_smbios_register(void);
>  
>  /* Called by networking code to memorize the dhcp ack package */
>  void efi_net_set_dhcp_ack(void *pkt, int len);
> diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
> index ac412e7362..3b87294dc3 100644
> --- a/lib/efi_loader/efi_smbios.c
> +++ b/lib/efi_loader/efi_smbios.c
> @@ -13,7 +13,7 @@
>  
>  static const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;
>  
> -void efi_smbios_register(void)
> +int efi_smbios_register(void)

Please, use efi_status_t as return type.

>  {
>  	/* Map within the low 32 bits, to allow for 32bit SMBIOS tables */
>  	uint64_t dmi = 0xffffffff;
> @@ -22,11 +22,13 @@ void efi_smbios_register(void)
>  	int memtype = EFI_RUNTIME_SERVICES_DATA;
>  
>  	if (efi_allocate_pages(1, memtype, pages, &dmi) != EFI_SUCCESS)
> -		return;
> +		return -ENOMEM;

Use	return EFI_OUT_OF_RESOURCES
This matches the value returned by efi_install_configuration_table.

>  
>  	/* Generate SMBIOS tables */
>  	write_smbios_table(dmi);
>  
>  	/* And expose them to our EFI payload */
>  	efi_install_configuration_table(&smbios_guid, (void*)(uintptr_t)dmi);

This function can return EFI_OUT_OF_RESOURCES.

> +
> +	return 0;

Use	return EFI_SUCCESS;

Regards Heinrich

>  }
> 

[U-Boot] [PATCH 02/16] efi: Move the init check inside efi_init_obj_list()

[Heinrich Schuchardt]

On 09/18/2017 12:59 AM, Simon Glass wrote:
> Rather than having the caller check this variable and the callee set it,
> move all access to the variable inside the function. This reduces the
> logic needed to call efi_init_obj_list().
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  cmd/bootefi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index abfab8fa98..2c9d31c5eb 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -109,6 +109,8 @@ static struct efi_object bootefi_device_obj = {
>  /* Initialize and populate EFI object list */
>  static void efi_init_obj_list(void)
>  {
> +	if (efi_obj_list_initalized)
> +		return;
>  	efi_obj_list_initalized = 1;
>  
>  	list_add_tail(&loaded_image_info_obj.link, &efi_obj_list);
> @@ -256,8 +258,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
>  		return -ENOENT;
>  
>  	/* Initialize and populate EFI object list */
> -	if (!efi_obj_list_initalized)
> -		efi_init_obj_list();
> +	efi_init_obj_list();
>  
>  	/* Call our payload! */
>  	debug("%s:%d Jumping to 0x%lx\n", __func__, __LINE__, (long)entry);
> @@ -311,8 +312,7 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  		 */
>  		efi_save_gd();
>  		/* Initialize and populate EFI object list */
> -		if (!efi_obj_list_initalized)
> -			efi_init_obj_list();
> +		efi_init_obj_list();
>  		loaded_image_info.device_handle = bootefi_device_path;
>  		loaded_image_info.file_path = bootefi_image_path;
>  		return efi_selftest(&loaded_image_info, &systab);
> 

Reviewed: Heinrich Schuchardt <xypron.glpk@gmx.de>

[U-Boot] [PATCH 03/16] efi: Add error checking for efi_init_obj_list()

[Heinrich Schuchardt]

On 09/18/2017 12:59 AM, Simon Glass wrote:
> This function calls a function which can fail. Print a message in this
> case and abort the boot, rather than silently continuing to boot, which
> will certainly fail.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  cmd/bootefi.c | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 2c9d31c5eb..9aa588eb1b 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -106,11 +106,17 @@ static struct efi_object bootefi_device_obj = {
>  	},
>  };
>  
> -/* Initialize and populate EFI object list */
> -static void efi_init_obj_list(void)
> +/**
> + * efi_init_obj_list() - Initialize and populate EFI object list
> + *
> + * @return 0 if OK, -ve on error (in which case it prints a message)
> + */
> +static int efi_init_obj_list(void)

Use efi_status_t as return type.

>  {
> +	int ret;
> +
>  	if (efi_obj_list_initalized)
> -		return;
> +		return 0;
>  	efi_obj_list_initalized = 1;
>  
>  	list_add_tail(&loaded_image_info_obj.link, &efi_obj_list);
> @@ -132,12 +138,19 @@ static void efi_init_obj_list(void)
>  		loaded_image_info.device_handle = bootefi_device_path;
>  #endif
>  #ifdef CONFIG_GENERATE_SMBIOS_TABLE
> -	efi_smbios_register();
> +	ret = efi_smbios_register();
> +	if (ret)
> +		goto error;
>  #endif
>  
>  	/* Initialize EFI runtime services */
>  	efi_reset_system_init();
>  	efi_get_time_init();
> +
> +	return 0;
> +error:
> +	printf("Error: Cannot set up EFI object list (err=%d)\n", ret);
> +	return ret;
>  }
>  
>  static void *copy_fdt(void *fdt)
> @@ -219,6 +232,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
>  	ulong fdt_pages, fdt_size, fdt_start, fdt_end;
>  	const efi_guid_t fdt_guid = EFI_FDT_GUID;
>  	bootm_headers_t img = { 0 };
> +	int ret;
>  
>  	/*
>  	 * gd lives in a fixed register which may get clobbered while we execute
> @@ -258,7 +272,9 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
>  		return -ENOENT;
>  
>  	/* Initialize and populate EFI object list */
> -	efi_init_obj_list();
> +	ret = efi_init_obj_list();
> +	if (ret)
> +		return ret;
>  
>  	/* Call our payload! */
>  	debug("%s:%d Jumping to 0x%lx\n", __func__, __LINE__, (long)entry);
> @@ -312,7 +328,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  		 */
>  		efi_save_gd();
>  		/* Initialize and populate EFI object list */
> -		efi_init_obj_list();
> +		if (efi_init_obj_list())
> +			return CMD_RET_FAILURE;
> +

We are duplicating code here.

efi_save_gd and efi_init_obj_list should have been moved to the start of
do_bootefi_exec in my patch
efi_selftest: provide unit test for event services

>  		loaded_image_info.device_handle = bootefi_device_path;
>  		loaded_image_info.file_path = bootefi_image_path;
>  		return efi_selftest(&loaded_image_info, &systab);
> 

[U-Boot] [PATCH 04/16] efi: Add a TODO to efi_init_obj_list()

[Heinrich Schuchardt]

On 09/18/2017 12:59 AM, Simon Glass wrote:
> This function repeats data structures provided by driver model. They are
> only created once so can be stale if the EFI loader is called twice (e.g.
> for testing or on boot failure).
> 
> Add a TODO to address this. It should be possible to attach EFI devices
> and data structures to driver-model devices and avoid having a parallel
> set of data structures.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  cmd/bootefi.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 9aa588eb1b..ee07733e3e 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -109,6 +109,10 @@ static struct efi_object bootefi_device_obj = {
>  /**
>   * efi_init_obj_list() - Initialize and populate EFI object list
>   *
> + * TODO(sjg at chromium.org): Move this to a dynamic list based on driver model,
> + * so that it does not need to be created before running EFI applications
> + * and updates when devices change.
> + *

I am not quite sure if by dynamic list you refer to linker generated
lists or to hot plugging.

The UEFI spec 2.7 has a chapter on "Hot-Plug Events". This would add a
lot of complexity. I do not think that we currently need it.

The object list also gets new entries created by the EFI application via
calling InstallMultipleProtocolInterfaces of InstallProtocolInterface
with *handle == NULL.

>   * @return 0 if OK, -ve on error (in which case it prints a message)
>   */
>  static int efi_init_obj_list(void)
> 

[U-Boot] [PATCH 05/16] efi: Correct header order in efi_memory

[Heinrich Schuchardt]

On 09/18/2017 12:59 AM, Simon Glass wrote:
> The headers are not in the correct order. Fix this.

The commit message should tell that and explain why you are dropping
#include <libfdt_env.h>

> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  lib/efi_loader/efi_memory.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index 9e079f1fa3..ad3d277be6 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -8,12 +8,11 @@
>  
>  #include <common.h>
>  #include <efi_loader.h>
> +#include <inttypes.h>
>  #include <malloc.h>
> +#include <watchdog.h>
>  #include <asm/global_data.h>
> -#include <libfdt_env.h>
>  #include <linux/list_sort.h>
> -#include <inttypes.h>
> -#include <watchdog.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> 

[U-Boot] [PATCH 08/16] sandbox: Add a setjmp() implementation

[Heinrich Schuchardt]

On 09/18/2017 12:59 AM, Simon Glass wrote:
> Add an implementation of setjmp() and longjmp() which rely on the
> underlying host C library. Since we cannot know how large the jump buffer
> needs to be, pick something that should be suitable and check it at
> runtime. At present we need access to the underlying struct as well.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  arch/sandbox/cpu/cpu.c            | 13 +++++++++++++
>  arch/sandbox/cpu/os.c             | 17 +++++++++++++++++
>  arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++++++++
>  include/os.h                      | 21 +++++++++++++++++++++
>  4 files changed, 72 insertions(+)
>  create mode 100644 arch/sandbox/include/asm/setjmp.h
> 
> diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
> index 01991049cc..de5862a53b 100644
> --- a/arch/sandbox/cpu/cpu.c
> +++ b/arch/sandbox/cpu/cpu.c
> @@ -9,6 +9,7 @@
>  #include <libfdt.h>
>  #include <os.h>
>  #include <asm/io.h>
> +#include <asm/setjmp.h>
>  #include <asm/state.h>
>  #include <dm/root.h>
>  
> @@ -154,3 +155,15 @@ ulong timer_get_boot_us(void)
>  
>  	return (count - base_count) / 1000;
>  }
> +
> +int setjmp(jmp_buf jmp)
> +{
> +	return os_setjmp((ulong *)jmp, sizeof(jmp));
> +}
> +
> +void longjmp(jmp_buf jmp, int ret)
> +{
> +	os_longjmp((ulong *)jmp, ret);
> +	while (1)
> +		;
> +}
> diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
> index 22d6aab534..909034fa4b 100644
> --- a/arch/sandbox/cpu/os.c
> +++ b/arch/sandbox/cpu/os.c
> @@ -7,6 +7,7 @@
>  #include <errno.h>
>  #include <fcntl.h>
>  #include <getopt.h>
> +#include <setjmp.h>
>  #include <stdio.h>
>  #include <stdint.h>
>  #include <stdlib.h>
> @@ -609,3 +610,19 @@ void os_localtime(struct rtc_time *rt)
>  	rt->tm_yday = tm->tm_yday;
>  	rt->tm_isdst = tm->tm_isdst;
>  }
> +
> +int os_setjmp(ulong *jmp, int size)
> +{
> +	if (size < sizeof(jmp_buf)) {
> +		printf("setjmp: jmpbuf is too small (%d bytes, need %d)\n",
> +		       size, sizeof(jmp_buf));
> +		return -ENOSPC;
> +	}
> +
> +	return setjmp((struct __jmp_buf_tag *)jmp);
> +}
> +
> +void os_longjmp(ulong *jmp, int ret)
> +{
> +	longjmp((struct __jmp_buf_tag *)jmp, ret);
> +}
> diff --git a/arch/sandbox/include/asm/setjmp.h b/arch/sandbox/include/asm/setjmp.h
> new file mode 100644
> index 0000000000..e25f50107c
> --- /dev/null
> +++ b/arch/sandbox/include/asm/setjmp.h
> @@ -0,0 +1,21 @@
> +/*
> + * (C) Copyright 2016
> + * Alexander Graf <agraf@suse.de>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#ifndef _SETJMP_H_
> +#define _SETJMP_H_	1
> +
> +struct jmp_buf_data {
> +	/* We're not sure how long this should be */
> +	ulong data[32];
> +};
> +
> +typedef struct jmp_buf_data jmp_buf[1];
> +
> +int setjmp(jmp_buf jmp);
> +__noreturn void longjmp(jmp_buf jmp, int ret);
> +
> +#endif /* _SETJMP_H_ */
> diff --git a/include/os.h b/include/os.h
> index 2bf4bdb1b8..ad1836ac9f 100644
> --- a/include/os.h
> +++ b/include/os.h
> @@ -310,4 +310,25 @@ int os_spl_to_uboot(const char *fname);
>   */
>  void os_localtime(struct rtc_time *rt);
>  
> +/**
> + * os_setjmp() - Call setjmp()
> + *
> + * Call the host system's setjmp() function.
> + *
> + * @jmp: Buffer to store current execution state
> + * @size: Size of buffer
> + * @return normal setjmp() value if OK, -ENOSPC if @size is too small
> + */
> +int os_setjmp(ulong *jmp, int size);
> +
> +/**
> + * os_longjmp() - Call longjmp()
> + *
> + * Call the host system's longjmp() function.
> + *
> + * @jmp: Buffer where previous execution state was stored
> + * @ret: Value to pass to longjmp()
> + */
> +void os_longjmp(ulong *jmp, int ret);
> +
>  #endif
> 

Please, fix this warning:

arch/sandbox/cpu/cpu.c: In function ‘setjmp’:
arch/sandbox/cpu/cpu.c:161:39: warning: ‘sizeof’ on array function
parameter ‘jmp’ will return size of ‘struct jmp_buf_data *’
[-Wsizeof-array-argument]
  return os_setjmp((ulong *)jmp, sizeof(jmp));
                                       ^
arch/sandbox/cpu/cpu.c:159:20: note: declared here
 int setjmp(jmp_buf jmp)

Regards

Heinrich

[U-Boot] [PATCH 06/16] efi: sandbox: Adjust memory setup for sandbox

[Heinrich Schuchardt]

On 09/18/2017 12:59 AM, Simon Glass wrote:
> With sandbox the U-Boot code is not mapped into the sandbox memory range
> so does not need to be excluded when allocating EFI memory. Update the EFI
> memory init code to take account of that.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  lib/efi_loader/efi_memory.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index ad3d277be6..c1a080e2e9 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -459,18 +459,22 @@ int efi_memory_init(void)
>  
>  	efi_add_known_memory();
>  
> -	/* Add U-Boot */
> -	uboot_start = (gd->start_addr_sp - uboot_stack_size) & ~EFI_PAGE_MASK;
> -	uboot_pages = (gd->ram_top - uboot_start) >> EFI_PAGE_SHIFT;
> -	efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA, false);
> -
> -	/* Add Runtime Services */
> -	runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
> -	runtime_end = (ulong)&__efi_runtime_stop;
> -	runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
> -	runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
> -	efi_add_memory_map(runtime_start, runtime_pages,
> -			   EFI_RUNTIME_SERVICES_CODE, false);
> +	if (!IS_ENABLED(CONFIG_SANDBOX)) {

We need the memory maps for:
GetMemoryMap
ExitBootServices

In efi_init_memory_init you define 8 MB memory starting at
0x0000000000000000 as available:
ram_start 0
start 0
pages 8000

I suggest you override efi_init_memory_init using malloc to assign at
least 128 MB contiguous memory with alignment EFI_PAGE_SIZE.

Regards

Heinrich

> +		/* Add U-Boot */
> +		uboot_start = (gd->start_addr_sp - uboot_stack_size) &
> +				~EFI_PAGE_MASK;
> +		uboot_pages = (gd->ram_top - uboot_start) >> EFI_PAGE_SHIFT;
> +		efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA,
> +				   false);
> +
> +		/* Add Runtime Services */
> +		runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
> +		runtime_end = (ulong)&__efi_runtime_stop;
> +		runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
> +		runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
> +		efi_add_memory_map(runtime_start, runtime_pages,
> +				   EFI_RUNTIME_SERVICES_CODE, false);
> +	}
>  
>  #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
>  	/* Request a 32bit 64MB bounce buffer region */
> 

[U-Boot] [PATCH 16/16] efi: sandbox: Add a simple 'bootefi test' command

[Heinrich Schuchardt]

On 09/18/2017 12:59 AM, Simon Glass wrote:
> This jumps to test code which can call directly into the EFI support. It
> does not need a separate image so it is easy to write tests with it.
> 
> For now the test just outputs a message. To try it:
> 
> ./sandbox/u-boot -c "bootefi test"
> U-Boot 2017.09-00204-g696c9855fe (Sep 17 2017 - 16:43:53 -0600)
> 
> DRAM:  128 MiB
> MMC:
> Using default environment
> 
> In:    serial
> Out:   serial
> Err:   serial
> SCSI:  Net:   No ethernet found.
> IDE:   Bus 0: not available
> Found 0 disks
> Hello, world!
> Test passed
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  cmd/bootefi.c             | 18 ++++++++++++++++++
>  configs/sandbox_defconfig |  1 +
>  include/efi_loader.h      |  3 +++
>  lib/efi_loader/Kconfig    | 10 ++++++++++
>  lib/efi_loader/Makefile   |  1 +
>  lib/efi_loader/efi_test.c | 17 +++++++++++++++++
>  6 files changed, 50 insertions(+)
>  create mode 100644 lib/efi_loader/efi_test.c
> 
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index ee07733e3e..f499103d23 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -323,6 +323,24 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  		memcpy((char *)addr, __efi_hello_world_begin, size);
>  	} else
>  #endif
> +	if (IS_ENABLED(CONFIG_BOOTEFI_TEST) && !strcmp(argv[1], "test")) {
> +		int ret;
> +
> +		/* Initialize and populate EFI object list */
> +		if (efi_init_obj_list())
> +			return CMD_RET_FAILURE;
> +
> +		loaded_image_info.device_handle = bootefi_device_path;
> +		loaded_image_info.file_path = bootefi_image_path;
> +		ret = efi_test(&loaded_image_info, &systab);
> +		if (ret) {
> +			printf("Test failed: err=%d\n", ret);
> +			return CMD_RET_FAILURE;
> +		}
> +		printf("Test passed\n");
> +
> +		return 0;
> +	}
>  #ifdef CONFIG_CMD_BOOTEFI_SELFTEST
>  	if (!strcmp(argv[1], "selftest")) {
>  		/*
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index 72600afea8..ab63f639de 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -196,3 +196,4 @@ CONFIG_UT_TIME=y
>  CONFIG_UT_DM=y
>  CONFIG_UT_ENV=y
>  CONFIG_UT_OVERLAY=y
> +CONFIG_CMD_BOOTEFI_SELFTEST=y
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 79d2dad22c..43919137b0 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -263,6 +263,9 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>  				 struct efi_system_table *systab);
>  #endif
>  
> +/* Perform EFI tests */
> +int efi_test(efi_handle_t image_handle, struct efi_system_table *systab);
> +
>  #else /* defined(EFI_LOADER) && !defined(CONFIG_SPL_BUILD) */
>  
>  /* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it out */
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index dee0a96a98..659b2b18f4 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -16,3 +16,13 @@ config EFI_LOADER_BOUNCE_BUFFER
>  	  Some hardware does not support DMA to full 64bit addresses. For this
>  	  hardware we can create a bounce buffer so that payloads don't have to
>  	  worry about platform details.
> +
> +config BOOTEFI_TEST
> +	bool "Provide a test for the EFI loader"
> +	depends on EFI_LOADER && SANDBOX
> +	default y
> +	help
> +	  Provides a test of the EFI loader functionality accessed via the
> +	  command line ('bootefi test'). This runs within U-Boot so does not
> +	  need a separate EFI application to work. It aims to include coverage
> +	  of all EFI code which can be accessed within sandbox.
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 30bf343a36..69eb93518d 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -21,3 +21,4 @@ obj-$(CONFIG_DM_VIDEO) += efi_gop.o
>  obj-$(CONFIG_PARTITIONS) += efi_disk.o
>  obj-$(CONFIG_NET) += efi_net.o
>  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
> +obj-$(CONFIG_BOOTEFI_TEST) += efi_test.o
> diff --git a/lib/efi_loader/efi_test.c b/lib/efi_loader/efi_test.c
> new file mode 100644
> index 0000000000..3fdce78b05
> --- /dev/null
> +++ b/lib/efi_loader/efi_test.c
> @@ -0,0 +1,17 @@
> +/*
> + * Copyright (c) 2017, Google Inc. All rights reserved.
> + *
> + * SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <efi_api.h>
> +
> +int efi_test(efi_handle_t image_handle, struct efi_system_table *systable)
> +{
> +	struct efi_simple_text_output_protocol *con_out = systable->con_out;
> +
> +	con_out->output_string(con_out, L"Hello, world!\n");
> +
> +	return 0;
> +}
> 

Hello Simon,

why do we need this patch?
You could just call bootefi selftest to demonstrate the same (after
fixing the memory map initialization for the sandbox).

Could you, please, review the framework that I have setup for bootefi
selftest. Does the design make sense to you?

Best regards

Heinrich

[U-Boot] [PATCH 13/16] efi: sandbox: Add relocation constants

[Heinrich Schuchardt]

On 09/18/2017 12:59 AM, Simon Glass wrote:
> Add these so that we can build the EFI loader for sandbox. The values are
> for x86_64 so potentially bogus. But we don't support relocation within
> sandbox anyway.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  lib/efi_loader/efi_runtime.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> index ad7f3754bd..8ad1d2fc99 100644
> --- a/lib/efi_loader/efi_runtime.c
> +++ b/lib/efi_loader/efi_runtime.c
> @@ -48,6 +48,9 @@ static efi_status_t __efi_runtime EFIAPI efi_invalid_parameter(void);
>  #include <asm/elf.h>
>  #define R_RELATIVE	R_386_RELATIVE
>  #define R_MASK		0xffULL
> +#elif defined(CONFIG_SANDBOX)

For the sandbox you should make your choice based on the host
architecture. I want to be able to work on my armv8 machine.

> +#define R_RELATIVE	8

You mean R_X86_64_RELATIVE (defined in arch/x86/include/asm/elf.h)?

Best regards

Heinrich

> +#define R_MASK		0xffffffffULL
>  #else
>  #error Need to add relocation awareness
>  #endif
> 

[U-Boot] [PATCH 00/16] efi: Enable basic sandbox support for EFI loader

[Rob Clark]

On Sun, Sep 17, 2017 at 11:48 PM, Heinrich Schuchardt
<xypron.glpk@gmx.de> wrote:
> On 09/18/2017 12:59 AM, Simon Glass wrote:
>> A limitation of the EFI loader at present is that it does not build with
>> sandbox. This makes it hard to write tests, since sandbox is used for most
>> testing in U-Boot.
>>
>> This series enables the EFI loader feature. It allows sandbox to build and
>> run a trivial function which calls the EFI API to output a message.
>>
>> Much work remains but this should serve as a basis for adding tests more
>> easily for EFI loader.
>>
>> This series sits on top of Heinrich's recent EFI test series. It is
>> available at u-boot-dm/efi-working
>>
>>
>> Simon Glass (16):
>>   efi: Update efi_smbios_register() to return error code
>>   efi: Move the init check inside efi_init_obj_list()
>>   efi: Add error checking for efi_init_obj_list()
>>   efi: Add a TODO to efi_init_obj_list()
>>   efi: Correct header order in efi_memory
>>   efi: sandbox: Adjust memory setup for sandbox
>>   sandbox: smbios: Update to support sandbox
>>   sandbox: Add a setjmp() implementation
>>   efi: sandbox: Add required linker sections
>>   efi: sandbox: Add distroboot support
>>   Define board_quiesce_devices() in a shared location
>>   Add a comment for board_quiesce_devices()
>>   efi: sandbox: Add relocation constants
>>   efi: Add a comment about duplicated ELF constants
>>   efi: sandbox: Enable EFI loader builder for sandbox
>>   efi: sandbox: Add a simple 'bootefi test' command
>>
>>  arch/arm/include/asm/u-boot-arm.h |  1 -
>>  arch/sandbox/cpu/cpu.c            | 13 ++++++++++
>>  arch/sandbox/cpu/os.c             | 17 ++++++++++++
>>  arch/sandbox/cpu/u-boot.lds       | 29 +++++++++++++++++++++
>>  arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++
>>  arch/sandbox/lib/Makefile         |  2 +-
>>  arch/sandbox/lib/sections.c       | 12 +++++++++
>>  arch/x86/include/asm/u-boot-x86.h |  1 -
>>  arch/x86/lib/bootm.c              |  4 ---
>>  cmd/bootefi.c                     | 54 ++++++++++++++++++++++++++++++++++-----
>>  common/bootm.c                    |  4 +++
>>  configs/sandbox_defconfig         |  1 +
>>  include/bootm.h                   |  8 ++++++
>>  include/config_distro_bootcmd.h   |  2 +-
>>  include/efi_loader.h              | 13 ++++++++--
>>  include/os.h                      | 21 +++++++++++++++
>>  lib/efi_loader/Kconfig            | 12 ++++++++-
>>  lib/efi_loader/Makefile           |  1 +
>>  lib/efi_loader/efi_boottime.c     |  4 +++
>>  lib/efi_loader/efi_memory.c       | 33 +++++++++++++-----------
>>  lib/efi_loader/efi_runtime.c      |  7 +++++
>>  lib/efi_loader/efi_smbios.c       |  6 +++--
>>  lib/efi_loader/efi_test.c         | 17 ++++++++++++
>>  lib/smbios.c                      | 38 ++++++++++++++++++++-------
>>  24 files changed, 277 insertions(+), 44 deletions(-)
>>  create mode 100644 arch/sandbox/include/asm/setjmp.h
>>  create mode 100644 arch/sandbox/lib/sections.c
>>  create mode 100644 lib/efi_loader/efi_test.c
>>
> Thanks for enabling efi_loader on sandbox. That will make many things
> easier.
>
> Unfortunately
> efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>                                  struct efi_system_table *systab)
> {
> ...
>         boottime = systable->boottime;
> ...
>         ret = boottime->allocate_pool(EFI_BOOT_SERVICES_DATA, map_size,
>                                       (void **)&memory_map);
> leads to a segmentation fault:

I'm seeing something similar, because:

(gdb) print gd->bd->bi_dram[0]
$2 = {start = 0, size = 134217728}

u-boot expects 1:1 phys:virt mapping, so that probably won't work.

> => bootefi selftest
>
> Testing EFI API implementation
>
> Number of tests to execute: 3
> <snip>
> Setting up 'ExitBootServices'
> Setting up 'ExitBootServices' succeeded
> Segmentation fault
> user at workstation:~/workspace/u-boot-odroid-c2/denx$
>
> The problem does not exist with qemu-x86_defconfig without your patches.

fwiw, qemu-x86 still works for me (I can still load Shell.efi) with
these patches..

BR,
-R

> qemu-x86_defconfig cannot be built with you patches:
>
>   UPD     include/generated/asm-offsets.h
> sh: echo: I/O error
> Kbuild:47: recipe for target 'include/generated/generic-asm-offsets.h'
> failed
> make[1]: *** [include/generated/generic-asm-offsets.h] Error 1
> make[1]: *** Waiting for unfinished jobs....
> Makefile:1332: recipe for target 'prepare0' failed
> make: *** [prepare0] Error 2
>
> Best regards
>
> Heinrich

[U-Boot] [PATCH 00/16] efi: Enable basic sandbox support for EFI loader

[Rob Clark]

On Mon, Sep 18, 2017 at 9:18 AM, Rob Clark <robdclark@gmail.com> wrote:
> On Sun, Sep 17, 2017 at 11:48 PM, Heinrich Schuchardt
> <xypron.glpk@gmx.de> wrote:
>> On 09/18/2017 12:59 AM, Simon Glass wrote:
>>> A limitation of the EFI loader at present is that it does not build with
>>> sandbox. This makes it hard to write tests, since sandbox is used for most
>>> testing in U-Boot.
>>>
>>> This series enables the EFI loader feature. It allows sandbox to build and
>>> run a trivial function which calls the EFI API to output a message.
>>>
>>> Much work remains but this should serve as a basis for adding tests more
>>> easily for EFI loader.
>>>
>>> This series sits on top of Heinrich's recent EFI test series. It is
>>> available at u-boot-dm/efi-working
>>>
>>>
>>> Simon Glass (16):
>>>   efi: Update efi_smbios_register() to return error code
>>>   efi: Move the init check inside efi_init_obj_list()
>>>   efi: Add error checking for efi_init_obj_list()
>>>   efi: Add a TODO to efi_init_obj_list()
>>>   efi: Correct header order in efi_memory
>>>   efi: sandbox: Adjust memory setup for sandbox
>>>   sandbox: smbios: Update to support sandbox
>>>   sandbox: Add a setjmp() implementation
>>>   efi: sandbox: Add required linker sections
>>>   efi: sandbox: Add distroboot support
>>>   Define board_quiesce_devices() in a shared location
>>>   Add a comment for board_quiesce_devices()
>>>   efi: sandbox: Add relocation constants
>>>   efi: Add a comment about duplicated ELF constants
>>>   efi: sandbox: Enable EFI loader builder for sandbox
>>>   efi: sandbox: Add a simple 'bootefi test' command
>>>
>>>  arch/arm/include/asm/u-boot-arm.h |  1 -
>>>  arch/sandbox/cpu/cpu.c            | 13 ++++++++++
>>>  arch/sandbox/cpu/os.c             | 17 ++++++++++++
>>>  arch/sandbox/cpu/u-boot.lds       | 29 +++++++++++++++++++++
>>>  arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++
>>>  arch/sandbox/lib/Makefile         |  2 +-
>>>  arch/sandbox/lib/sections.c       | 12 +++++++++
>>>  arch/x86/include/asm/u-boot-x86.h |  1 -
>>>  arch/x86/lib/bootm.c              |  4 ---
>>>  cmd/bootefi.c                     | 54 ++++++++++++++++++++++++++++++++++-----
>>>  common/bootm.c                    |  4 +++
>>>  configs/sandbox_defconfig         |  1 +
>>>  include/bootm.h                   |  8 ++++++
>>>  include/config_distro_bootcmd.h   |  2 +-
>>>  include/efi_loader.h              | 13 ++++++++--
>>>  include/os.h                      | 21 +++++++++++++++
>>>  lib/efi_loader/Kconfig            | 12 ++++++++-
>>>  lib/efi_loader/Makefile           |  1 +
>>>  lib/efi_loader/efi_boottime.c     |  4 +++
>>>  lib/efi_loader/efi_memory.c       | 33 +++++++++++++-----------
>>>  lib/efi_loader/efi_runtime.c      |  7 +++++
>>>  lib/efi_loader/efi_smbios.c       |  6 +++--
>>>  lib/efi_loader/efi_test.c         | 17 ++++++++++++
>>>  lib/smbios.c                      | 38 ++++++++++++++++++++-------
>>>  24 files changed, 277 insertions(+), 44 deletions(-)
>>>  create mode 100644 arch/sandbox/include/asm/setjmp.h
>>>  create mode 100644 arch/sandbox/lib/sections.c
>>>  create mode 100644 lib/efi_loader/efi_test.c
>>>
>> Thanks for enabling efi_loader on sandbox. That will make many things
>> easier.
>>
>> Unfortunately
>> efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>>                                  struct efi_system_table *systab)
>> {
>> ...
>>         boottime = systable->boottime;
>> ...
>>         ret = boottime->allocate_pool(EFI_BOOT_SERVICES_DATA, map_size,
>>                                       (void **)&memory_map);
>> leads to a segmentation fault:
>
> I'm seeing something similar, because:
>
> (gdb) print gd->bd->bi_dram[0]
> $2 = {start = 0, size = 134217728}
>
> u-boot expects 1:1 phys:virt mapping, so that probably won't work.

The following quick hack works.. something similar could probably be
smashed in to ""

--------
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index cddafe2d43..da2079a4b1 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -459,9 +459,10 @@ int efi_memory_init(void)
     unsigned long uboot_start, uboot_pages;
     unsigned long uboot_stack_size = 16 * 1024 * 1024;

-    efi_add_known_memory();

     if (!IS_ENABLED(CONFIG_SANDBOX)) {
+        efi_add_known_memory();
+
         /* Add U-Boot */
         uboot_start = (gd->start_addr_sp - uboot_stack_size) &
                 ~EFI_PAGE_MASK;
@@ -476,6 +477,12 @@ int efi_memory_init(void)
         runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
         efi_add_memory_map(runtime_start, runtime_pages,
                    EFI_RUNTIME_SERVICES_CODE, false);
+    } else {
+#define SZ_256M    0x10000000
+        size_t sz = SZ_256M;
+        void *ram = os_malloc(sz);
+        efi_add_memory_map((uintptr_t)ram, sz >> EFI_PAGE_SHIFT,
+                   EFI_CONVENTIONAL_MEMORY, false);
     }

 #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
--------

With that I'm at least getting further..  efi_allocate_pool()
eventually fails, possibly making every small memory allocation page
aligned means that 256m isn't enough..

BR,
-R

>> => bootefi selftest
>>
>> Testing EFI API implementation
>>
>> Number of tests to execute: 3
>> <snip>
>> Setting up 'ExitBootServices'
>> Setting up 'ExitBootServices' succeeded
>> Segmentation fault
>> user at workstation:~/workspace/u-boot-odroid-c2/denx$
>>
>> The problem does not exist with qemu-x86_defconfig without your patches.
>
> fwiw, qemu-x86 still works for me (I can still load Shell.efi) with
> these patches..
>
> BR,
> -R
>
>> qemu-x86_defconfig cannot be built with you patches:
>>
>>   UPD     include/generated/asm-offsets.h
>> sh: echo: I/O error
>> Kbuild:47: recipe for target 'include/generated/generic-asm-offsets.h'
>> failed
>> make[1]: *** [include/generated/generic-asm-offsets.h] Error 1
>> make[1]: *** Waiting for unfinished jobs....
>> Makefile:1332: recipe for target 'prepare0' failed
>> make: *** [prepare0] Error 2
>>
>> Best regards
>>
>> Heinrich

[U-Boot] [PATCH 00/16] efi: Enable basic sandbox support for EFI loader

[Rob Clark]

On Mon, Sep 18, 2017 at 9:31 AM, Rob Clark <robdclark@gmail.com> wrote:
> On Mon, Sep 18, 2017 at 9:18 AM, Rob Clark <robdclark@gmail.com> wrote:
>> On Sun, Sep 17, 2017 at 11:48 PM, Heinrich Schuchardt
>> <xypron.glpk@gmx.de> wrote:
>>> On 09/18/2017 12:59 AM, Simon Glass wrote:
>>>> A limitation of the EFI loader at present is that it does not build with
>>>> sandbox. This makes it hard to write tests, since sandbox is used for most
>>>> testing in U-Boot.
>>>>
>>>> This series enables the EFI loader feature. It allows sandbox to build and
>>>> run a trivial function which calls the EFI API to output a message.
>>>>
>>>> Much work remains but this should serve as a basis for adding tests more
>>>> easily for EFI loader.
>>>>
>>>> This series sits on top of Heinrich's recent EFI test series. It is
>>>> available at u-boot-dm/efi-working
>>>>
>>>>
>>>> Simon Glass (16):
>>>>   efi: Update efi_smbios_register() to return error code
>>>>   efi: Move the init check inside efi_init_obj_list()
>>>>   efi: Add error checking for efi_init_obj_list()
>>>>   efi: Add a TODO to efi_init_obj_list()
>>>>   efi: Correct header order in efi_memory
>>>>   efi: sandbox: Adjust memory setup for sandbox
>>>>   sandbox: smbios: Update to support sandbox
>>>>   sandbox: Add a setjmp() implementation
>>>>   efi: sandbox: Add required linker sections
>>>>   efi: sandbox: Add distroboot support
>>>>   Define board_quiesce_devices() in a shared location
>>>>   Add a comment for board_quiesce_devices()
>>>>   efi: sandbox: Add relocation constants
>>>>   efi: Add a comment about duplicated ELF constants
>>>>   efi: sandbox: Enable EFI loader builder for sandbox
>>>>   efi: sandbox: Add a simple 'bootefi test' command
>>>>
>>>>  arch/arm/include/asm/u-boot-arm.h |  1 -
>>>>  arch/sandbox/cpu/cpu.c            | 13 ++++++++++
>>>>  arch/sandbox/cpu/os.c             | 17 ++++++++++++
>>>>  arch/sandbox/cpu/u-boot.lds       | 29 +++++++++++++++++++++
>>>>  arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++
>>>>  arch/sandbox/lib/Makefile         |  2 +-
>>>>  arch/sandbox/lib/sections.c       | 12 +++++++++
>>>>  arch/x86/include/asm/u-boot-x86.h |  1 -
>>>>  arch/x86/lib/bootm.c              |  4 ---
>>>>  cmd/bootefi.c                     | 54 ++++++++++++++++++++++++++++++++++-----
>>>>  common/bootm.c                    |  4 +++
>>>>  configs/sandbox_defconfig         |  1 +
>>>>  include/bootm.h                   |  8 ++++++
>>>>  include/config_distro_bootcmd.h   |  2 +-
>>>>  include/efi_loader.h              | 13 ++++++++--
>>>>  include/os.h                      | 21 +++++++++++++++
>>>>  lib/efi_loader/Kconfig            | 12 ++++++++-
>>>>  lib/efi_loader/Makefile           |  1 +
>>>>  lib/efi_loader/efi_boottime.c     |  4 +++
>>>>  lib/efi_loader/efi_memory.c       | 33 +++++++++++++-----------
>>>>  lib/efi_loader/efi_runtime.c      |  7 +++++
>>>>  lib/efi_loader/efi_smbios.c       |  6 +++--
>>>>  lib/efi_loader/efi_test.c         | 17 ++++++++++++
>>>>  lib/smbios.c                      | 38 ++++++++++++++++++++-------
>>>>  24 files changed, 277 insertions(+), 44 deletions(-)
>>>>  create mode 100644 arch/sandbox/include/asm/setjmp.h
>>>>  create mode 100644 arch/sandbox/lib/sections.c
>>>>  create mode 100644 lib/efi_loader/efi_test.c
>>>>
>>> Thanks for enabling efi_loader on sandbox. That will make many things
>>> easier.
>>>
>>> Unfortunately
>>> efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>>>                                  struct efi_system_table *systab)
>>> {
>>> ...
>>>         boottime = systable->boottime;
>>> ...
>>>         ret = boottime->allocate_pool(EFI_BOOT_SERVICES_DATA, map_size,
>>>                                       (void **)&memory_map);
>>> leads to a segmentation fault:
>>
>> I'm seeing something similar, because:
>>
>> (gdb) print gd->bd->bi_dram[0]
>> $2 = {start = 0, size = 134217728}
>>
>> u-boot expects 1:1 phys:virt mapping, so that probably won't work.
>
> The following quick hack works.. something similar could probably be
> smashed in to ""
>
> --------
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index cddafe2d43..da2079a4b1 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -459,9 +459,10 @@ int efi_memory_init(void)
>      unsigned long uboot_start, uboot_pages;
>      unsigned long uboot_stack_size = 16 * 1024 * 1024;
>
> -    efi_add_known_memory();
>
>      if (!IS_ENABLED(CONFIG_SANDBOX)) {
> +        efi_add_known_memory();
> +
>          /* Add U-Boot */
>          uboot_start = (gd->start_addr_sp - uboot_stack_size) &
>                  ~EFI_PAGE_MASK;
> @@ -476,6 +477,12 @@ int efi_memory_init(void)
>          runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
>          efi_add_memory_map(runtime_start, runtime_pages,
>                     EFI_RUNTIME_SERVICES_CODE, false);
> +    } else {
> +#define SZ_256M    0x10000000
> +        size_t sz = SZ_256M;
> +        void *ram = os_malloc(sz);
> +        efi_add_memory_map((uintptr_t)ram, sz >> EFI_PAGE_SHIFT,
> +                   EFI_CONVENTIONAL_MEMORY, false);
>      }
>
>  #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> --------
>
> With that I'm at least getting further..  efi_allocate_pool()
> eventually fails, possibly making every small memory allocation page
> aligned means that 256m isn't enough..

Ok, still just as hacky, but works a bit better:

---------
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index cddafe2d43..b546b5e35d 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -14,6 +14,7 @@
 #include <linux/list_sort.h>
 #include <inttypes.h>
 #include <watchdog.h>
+#include <os.h>

 DECLARE_GLOBAL_DATA_PTR;

@@ -459,9 +460,9 @@ int efi_memory_init(void)
     unsigned long uboot_start, uboot_pages;
     unsigned long uboot_stack_size = 16 * 1024 * 1024;

-    efi_add_known_memory();
-
     if (!IS_ENABLED(CONFIG_SANDBOX)) {
+        efi_add_known_memory();
+
         /* Add U-Boot */
         uboot_start = (gd->start_addr_sp - uboot_stack_size) &
                 ~EFI_PAGE_MASK;
@@ -476,6 +477,14 @@ int efi_memory_init(void)
         runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
         efi_add_memory_map(runtime_start, runtime_pages,
                    EFI_RUNTIME_SERVICES_CODE, false);
+    } else {
+#define SZ_4K    0x00001000
+#define SZ_256M    0x10000000
+        size_t sz = SZ_256M;
+        uintptr_t ram = (uintptr_t)os_malloc(sz + SZ_4K) + SZ_4K;
+        efi_add_memory_map(ram & ~EFI_PAGE_MASK, sz >> EFI_PAGE_SHIFT,
+                   EFI_CONVENTIONAL_MEMORY, false);
+        gd->start_addr_sp = ~0;
     }

 #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
---------

At this point it crashes in efi_load_pe() when it first tries to
dereference the address of the image passed in, ie. I'm running:

  host bind 0 x86_64-sct.img
  load host 0:1 0x01000000 /efi/boot/shell.efi
  bootefi 0x01000000

Not sure if there is a better way to pick an address to load into.  Or
maybe just assuming that PA==VA isn't a good idea in sandbox?

(Maybe being able to do 'bootefi host 0:1 /efi/boot/shell.efi' and
have bootefi take care or memory allocation for the loaded image would
be convenient)

BR,
-R


> BR,
> -R
>
>>> => bootefi selftest
>>>
>>> Testing EFI API implementation
>>>
>>> Number of tests to execute: 3
>>> <snip>
>>> Setting up 'ExitBootServices'
>>> Setting up 'ExitBootServices' succeeded
>>> Segmentation fault
>>> user at workstation:~/workspace/u-boot-odroid-c2/denx$
>>>
>>> The problem does not exist with qemu-x86_defconfig without your patches.
>>
>> fwiw, qemu-x86 still works for me (I can still load Shell.efi) with
>> these patches..
>>
>> BR,
>> -R
>>
>>> qemu-x86_defconfig cannot be built with you patches:
>>>
>>>   UPD     include/generated/asm-offsets.h
>>> sh: echo: I/O error
>>> Kbuild:47: recipe for target 'include/generated/generic-asm-offsets.h'
>>> failed
>>> make[1]: *** [include/generated/generic-asm-offsets.h] Error 1
>>> make[1]: *** Waiting for unfinished jobs....
>>> Makefile:1332: recipe for target 'prepare0' failed
>>> make: *** [prepare0] Error 2
>>>
>>> Best regards
>>>
>>> Heinrich

[U-Boot] [PATCH 00/16] efi: Enable basic sandbox support for EFI loader

[Rob Clark]

On Mon, Sep 18, 2017 at 10:30 AM, Rob Clark <robdclark@gmail.com> wrote:
> On Mon, Sep 18, 2017 at 9:31 AM, Rob Clark <robdclark@gmail.com> wrote:
>> On Mon, Sep 18, 2017 at 9:18 AM, Rob Clark <robdclark@gmail.com> wrote:
>>> On Sun, Sep 17, 2017 at 11:48 PM, Heinrich Schuchardt
>>> <xypron.glpk@gmx.de> wrote:
>>>> On 09/18/2017 12:59 AM, Simon Glass wrote:
>>>>> A limitation of the EFI loader at present is that it does not build with
>>>>> sandbox. This makes it hard to write tests, since sandbox is used for most
>>>>> testing in U-Boot.
>>>>>
>>>>> This series enables the EFI loader feature. It allows sandbox to build and
>>>>> run a trivial function which calls the EFI API to output a message.
>>>>>
>>>>> Much work remains but this should serve as a basis for adding tests more
>>>>> easily for EFI loader.
>>>>>
>>>>> This series sits on top of Heinrich's recent EFI test series. It is
>>>>> available at u-boot-dm/efi-working
>>>>>
>>>>>
>>>>> Simon Glass (16):
>>>>>   efi: Update efi_smbios_register() to return error code
>>>>>   efi: Move the init check inside efi_init_obj_list()
>>>>>   efi: Add error checking for efi_init_obj_list()
>>>>>   efi: Add a TODO to efi_init_obj_list()
>>>>>   efi: Correct header order in efi_memory
>>>>>   efi: sandbox: Adjust memory setup for sandbox
>>>>>   sandbox: smbios: Update to support sandbox
>>>>>   sandbox: Add a setjmp() implementation
>>>>>   efi: sandbox: Add required linker sections
>>>>>   efi: sandbox: Add distroboot support
>>>>>   Define board_quiesce_devices() in a shared location
>>>>>   Add a comment for board_quiesce_devices()
>>>>>   efi: sandbox: Add relocation constants
>>>>>   efi: Add a comment about duplicated ELF constants
>>>>>   efi: sandbox: Enable EFI loader builder for sandbox
>>>>>   efi: sandbox: Add a simple 'bootefi test' command
>>>>>
>>>>>  arch/arm/include/asm/u-boot-arm.h |  1 -
>>>>>  arch/sandbox/cpu/cpu.c            | 13 ++++++++++
>>>>>  arch/sandbox/cpu/os.c             | 17 ++++++++++++
>>>>>  arch/sandbox/cpu/u-boot.lds       | 29 +++++++++++++++++++++
>>>>>  arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++
>>>>>  arch/sandbox/lib/Makefile         |  2 +-
>>>>>  arch/sandbox/lib/sections.c       | 12 +++++++++
>>>>>  arch/x86/include/asm/u-boot-x86.h |  1 -
>>>>>  arch/x86/lib/bootm.c              |  4 ---
>>>>>  cmd/bootefi.c                     | 54 ++++++++++++++++++++++++++++++++++-----
>>>>>  common/bootm.c                    |  4 +++
>>>>>  configs/sandbox_defconfig         |  1 +
>>>>>  include/bootm.h                   |  8 ++++++
>>>>>  include/config_distro_bootcmd.h   |  2 +-
>>>>>  include/efi_loader.h              | 13 ++++++++--
>>>>>  include/os.h                      | 21 +++++++++++++++
>>>>>  lib/efi_loader/Kconfig            | 12 ++++++++-
>>>>>  lib/efi_loader/Makefile           |  1 +
>>>>>  lib/efi_loader/efi_boottime.c     |  4 +++
>>>>>  lib/efi_loader/efi_memory.c       | 33 +++++++++++++-----------
>>>>>  lib/efi_loader/efi_runtime.c      |  7 +++++
>>>>>  lib/efi_loader/efi_smbios.c       |  6 +++--
>>>>>  lib/efi_loader/efi_test.c         | 17 ++++++++++++
>>>>>  lib/smbios.c                      | 38 ++++++++++++++++++++-------
>>>>>  24 files changed, 277 insertions(+), 44 deletions(-)
>>>>>  create mode 100644 arch/sandbox/include/asm/setjmp.h
>>>>>  create mode 100644 arch/sandbox/lib/sections.c
>>>>>  create mode 100644 lib/efi_loader/efi_test.c
>>>>>
>>>> Thanks for enabling efi_loader on sandbox. That will make many things
>>>> easier.
>>>>
>>>> Unfortunately
>>>> efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>>>>                                  struct efi_system_table *systab)
>>>> {
>>>> ...
>>>>         boottime = systable->boottime;
>>>> ...
>>>>         ret = boottime->allocate_pool(EFI_BOOT_SERVICES_DATA, map_size,
>>>>                                       (void **)&memory_map);
>>>> leads to a segmentation fault:
>>>
>>> I'm seeing something similar, because:
>>>
>>> (gdb) print gd->bd->bi_dram[0]
>>> $2 = {start = 0, size = 134217728}
>>>
>>> u-boot expects 1:1 phys:virt mapping, so that probably won't work.
>>
>> The following quick hack works.. something similar could probably be
>> smashed in to ""
>>
>> --------
>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>> index cddafe2d43..da2079a4b1 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -459,9 +459,10 @@ int efi_memory_init(void)
>>      unsigned long uboot_start, uboot_pages;
>>      unsigned long uboot_stack_size = 16 * 1024 * 1024;
>>
>> -    efi_add_known_memory();
>>
>>      if (!IS_ENABLED(CONFIG_SANDBOX)) {
>> +        efi_add_known_memory();
>> +
>>          /* Add U-Boot */
>>          uboot_start = (gd->start_addr_sp - uboot_stack_size) &
>>                  ~EFI_PAGE_MASK;
>> @@ -476,6 +477,12 @@ int efi_memory_init(void)
>>          runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
>>          efi_add_memory_map(runtime_start, runtime_pages,
>>                     EFI_RUNTIME_SERVICES_CODE, false);
>> +    } else {
>> +#define SZ_256M    0x10000000
>> +        size_t sz = SZ_256M;
>> +        void *ram = os_malloc(sz);
>> +        efi_add_memory_map((uintptr_t)ram, sz >> EFI_PAGE_SHIFT,
>> +                   EFI_CONVENTIONAL_MEMORY, false);
>>      }
>>
>>  #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
>> --------
>>
>> With that I'm at least getting further..  efi_allocate_pool()
>> eventually fails, possibly making every small memory allocation page
>> aligned means that 256m isn't enough..
>
> Ok, still just as hacky, but works a bit better:
>
> ---------
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index cddafe2d43..b546b5e35d 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -14,6 +14,7 @@
>  #include <linux/list_sort.h>
>  #include <inttypes.h>
>  #include <watchdog.h>
> +#include <os.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -459,9 +460,9 @@ int efi_memory_init(void)
>      unsigned long uboot_start, uboot_pages;
>      unsigned long uboot_stack_size = 16 * 1024 * 1024;
>
> -    efi_add_known_memory();
> -
>      if (!IS_ENABLED(CONFIG_SANDBOX)) {
> +        efi_add_known_memory();
> +
>          /* Add U-Boot */
>          uboot_start = (gd->start_addr_sp - uboot_stack_size) &
>                  ~EFI_PAGE_MASK;
> @@ -476,6 +477,14 @@ int efi_memory_init(void)
>          runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
>          efi_add_memory_map(runtime_start, runtime_pages,
>                     EFI_RUNTIME_SERVICES_CODE, false);
> +    } else {
> +#define SZ_4K    0x00001000
> +#define SZ_256M    0x10000000
> +        size_t sz = SZ_256M;
> +        uintptr_t ram = (uintptr_t)os_malloc(sz + SZ_4K) + SZ_4K;
> +        efi_add_memory_map(ram & ~EFI_PAGE_MASK, sz >> EFI_PAGE_SHIFT,
> +                   EFI_CONVENTIONAL_MEMORY, false);
> +        gd->start_addr_sp = ~0;
>      }
>
>  #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> ---------
>
> At this point it crashes in efi_load_pe() when it first tries to
> dereference the address of the image passed in, ie. I'm running:
>
>   host bind 0 x86_64-sct.img
>   load host 0:1 0x01000000 /efi/boot/shell.efi
>   bootefi 0x01000000
>
> Not sure if there is a better way to pick an address to load into.  Or
> maybe just assuming that PA==VA isn't a good idea in sandbox?
>

Ok, I realized there is map_sysmem().. which gets me further..
efi_loader really expects identity mapping (PA==VA), and iirc this is
what UEFI spec expects too so I wouldn't necessarily call it a bug in
efi_loader.

BR,
-R

[U-Boot] [PATCH 00/16] efi: Enable basic sandbox support for EFI loader

[Rob Clark]

On Mon, Sep 18, 2017 at 11:07 AM, Rob Clark <robdclark@gmail.com> wrote:
> On Mon, Sep 18, 2017 at 10:30 AM, Rob Clark <robdclark@gmail.com> wrote:
>> On Mon, Sep 18, 2017 at 9:31 AM, Rob Clark <robdclark@gmail.com> wrote:
>>> On Mon, Sep 18, 2017 at 9:18 AM, Rob Clark <robdclark@gmail.com> wrote:
>>>> On Sun, Sep 17, 2017 at 11:48 PM, Heinrich Schuchardt
>>>> <xypron.glpk@gmx.de> wrote:
>>>>> On 09/18/2017 12:59 AM, Simon Glass wrote:
>>>>>> A limitation of the EFI loader at present is that it does not build with
>>>>>> sandbox. This makes it hard to write tests, since sandbox is used for most
>>>>>> testing in U-Boot.
>>>>>>
>>>>>> This series enables the EFI loader feature. It allows sandbox to build and
>>>>>> run a trivial function which calls the EFI API to output a message.
>>>>>>
>>>>>> Much work remains but this should serve as a basis for adding tests more
>>>>>> easily for EFI loader.
>>>>>>
>>>>>> This series sits on top of Heinrich's recent EFI test series. It is
>>>>>> available at u-boot-dm/efi-working
>>>>>>
>>>>>>
>>>>>> Simon Glass (16):
>>>>>>   efi: Update efi_smbios_register() to return error code
>>>>>>   efi: Move the init check inside efi_init_obj_list()
>>>>>>   efi: Add error checking for efi_init_obj_list()
>>>>>>   efi: Add a TODO to efi_init_obj_list()
>>>>>>   efi: Correct header order in efi_memory
>>>>>>   efi: sandbox: Adjust memory setup for sandbox
>>>>>>   sandbox: smbios: Update to support sandbox
>>>>>>   sandbox: Add a setjmp() implementation
>>>>>>   efi: sandbox: Add required linker sections
>>>>>>   efi: sandbox: Add distroboot support
>>>>>>   Define board_quiesce_devices() in a shared location
>>>>>>   Add a comment for board_quiesce_devices()
>>>>>>   efi: sandbox: Add relocation constants
>>>>>>   efi: Add a comment about duplicated ELF constants
>>>>>>   efi: sandbox: Enable EFI loader builder for sandbox
>>>>>>   efi: sandbox: Add a simple 'bootefi test' command
>>>>>>
>>>>>>  arch/arm/include/asm/u-boot-arm.h |  1 -
>>>>>>  arch/sandbox/cpu/cpu.c            | 13 ++++++++++
>>>>>>  arch/sandbox/cpu/os.c             | 17 ++++++++++++
>>>>>>  arch/sandbox/cpu/u-boot.lds       | 29 +++++++++++++++++++++
>>>>>>  arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++
>>>>>>  arch/sandbox/lib/Makefile         |  2 +-
>>>>>>  arch/sandbox/lib/sections.c       | 12 +++++++++
>>>>>>  arch/x86/include/asm/u-boot-x86.h |  1 -
>>>>>>  arch/x86/lib/bootm.c              |  4 ---
>>>>>>  cmd/bootefi.c                     | 54 ++++++++++++++++++++++++++++++++++-----
>>>>>>  common/bootm.c                    |  4 +++
>>>>>>  configs/sandbox_defconfig         |  1 +
>>>>>>  include/bootm.h                   |  8 ++++++
>>>>>>  include/config_distro_bootcmd.h   |  2 +-
>>>>>>  include/efi_loader.h              | 13 ++++++++--
>>>>>>  include/os.h                      | 21 +++++++++++++++
>>>>>>  lib/efi_loader/Kconfig            | 12 ++++++++-
>>>>>>  lib/efi_loader/Makefile           |  1 +
>>>>>>  lib/efi_loader/efi_boottime.c     |  4 +++
>>>>>>  lib/efi_loader/efi_memory.c       | 33 +++++++++++++-----------
>>>>>>  lib/efi_loader/efi_runtime.c      |  7 +++++
>>>>>>  lib/efi_loader/efi_smbios.c       |  6 +++--
>>>>>>  lib/efi_loader/efi_test.c         | 17 ++++++++++++
>>>>>>  lib/smbios.c                      | 38 ++++++++++++++++++++-------
>>>>>>  24 files changed, 277 insertions(+), 44 deletions(-)
>>>>>>  create mode 100644 arch/sandbox/include/asm/setjmp.h
>>>>>>  create mode 100644 arch/sandbox/lib/sections.c
>>>>>>  create mode 100644 lib/efi_loader/efi_test.c
>>>>>>
>>>>> Thanks for enabling efi_loader on sandbox. That will make many things
>>>>> easier.
>>>>>
>>>>> Unfortunately
>>>>> efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>>>>>                                  struct efi_system_table *systab)
>>>>> {
>>>>> ...
>>>>>         boottime = systable->boottime;
>>>>> ...
>>>>>         ret = boottime->allocate_pool(EFI_BOOT_SERVICES_DATA, map_size,
>>>>>                                       (void **)&memory_map);
>>>>> leads to a segmentation fault:
>>>>
>>>> I'm seeing something similar, because:
>>>>
>>>> (gdb) print gd->bd->bi_dram[0]
>>>> $2 = {start = 0, size = 134217728}
>>>>
>>>> u-boot expects 1:1 phys:virt mapping, so that probably won't work.
>>>
>>> The following quick hack works.. something similar could probably be
>>> smashed in to ""
>>>
>>> --------
>>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>>> index cddafe2d43..da2079a4b1 100644
>>> --- a/lib/efi_loader/efi_memory.c
>>> +++ b/lib/efi_loader/efi_memory.c
>>> @@ -459,9 +459,10 @@ int efi_memory_init(void)
>>>      unsigned long uboot_start, uboot_pages;
>>>      unsigned long uboot_stack_size = 16 * 1024 * 1024;
>>>
>>> -    efi_add_known_memory();
>>>
>>>      if (!IS_ENABLED(CONFIG_SANDBOX)) {
>>> +        efi_add_known_memory();
>>> +
>>>          /* Add U-Boot */
>>>          uboot_start = (gd->start_addr_sp - uboot_stack_size) &
>>>                  ~EFI_PAGE_MASK;
>>> @@ -476,6 +477,12 @@ int efi_memory_init(void)
>>>          runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
>>>          efi_add_memory_map(runtime_start, runtime_pages,
>>>                     EFI_RUNTIME_SERVICES_CODE, false);
>>> +    } else {
>>> +#define SZ_256M    0x10000000
>>> +        size_t sz = SZ_256M;
>>> +        void *ram = os_malloc(sz);
>>> +        efi_add_memory_map((uintptr_t)ram, sz >> EFI_PAGE_SHIFT,
>>> +                   EFI_CONVENTIONAL_MEMORY, false);
>>>      }
>>>
>>>  #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
>>> --------
>>>
>>> With that I'm at least getting further..  efi_allocate_pool()
>>> eventually fails, possibly making every small memory allocation page
>>> aligned means that 256m isn't enough..
>>
>> Ok, still just as hacky, but works a bit better:
>>
>> ---------
>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>> index cddafe2d43..b546b5e35d 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -14,6 +14,7 @@
>>  #include <linux/list_sort.h>
>>  #include <inttypes.h>
>>  #include <watchdog.h>
>> +#include <os.h>
>>
>>  DECLARE_GLOBAL_DATA_PTR;
>>
>> @@ -459,9 +460,9 @@ int efi_memory_init(void)
>>      unsigned long uboot_start, uboot_pages;
>>      unsigned long uboot_stack_size = 16 * 1024 * 1024;
>>
>> -    efi_add_known_memory();
>> -
>>      if (!IS_ENABLED(CONFIG_SANDBOX)) {
>> +        efi_add_known_memory();
>> +
>>          /* Add U-Boot */
>>          uboot_start = (gd->start_addr_sp - uboot_stack_size) &
>>                  ~EFI_PAGE_MASK;
>> @@ -476,6 +477,14 @@ int efi_memory_init(void)
>>          runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
>>          efi_add_memory_map(runtime_start, runtime_pages,
>>                     EFI_RUNTIME_SERVICES_CODE, false);
>> +    } else {
>> +#define SZ_4K    0x00001000
>> +#define SZ_256M    0x10000000
>> +        size_t sz = SZ_256M;
>> +        uintptr_t ram = (uintptr_t)os_malloc(sz + SZ_4K) + SZ_4K;
>> +        efi_add_memory_map(ram & ~EFI_PAGE_MASK, sz >> EFI_PAGE_SHIFT,
>> +                   EFI_CONVENTIONAL_MEMORY, false);
>> +        gd->start_addr_sp = ~0;
>>      }
>>
>>  #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
>> ---------
>>
>> At this point it crashes in efi_load_pe() when it first tries to
>> dereference the address of the image passed in, ie. I'm running:
>>
>>   host bind 0 x86_64-sct.img
>>   load host 0:1 0x01000000 /efi/boot/shell.efi
>>   bootefi 0x01000000
>>
>> Not sure if there is a better way to pick an address to load into.  Or
>> maybe just assuming that PA==VA isn't a good idea in sandbox?
>>
>
> Ok, I realized there is map_sysmem().. which gets me further..
> efi_loader really expects identity mapping (PA==VA), and iirc this is
> what UEFI spec expects too so I wouldn't necessarily call it a bug in
> efi_loader.
>

So, I don't know x86(_64) asm or calling conventions as well as arm..
but I wonder if we are screwing up something long those lines:


0000000000000280 <.text>:
     280:       48 89 5c 24 08          mov    %rbx,0x8(%rsp)
     285:       57                      push   %rdi
     286:       48 83 ec 20             sub    $0x20,%rsp
     28a:       48 8b f9                mov    %rcx,%rdi
>>   28d:       e8 1e 00 00 00          callq  0x2b0
this jump is taken to 0x2b0

     292:       e8 2d 06 00 00          callq  0x8c4
     297:       48 8b cf                mov    %rdi,%rcx
     29a:       48 8b d8                mov    %rax,%rbx
     29d:       e8 ea 01 00 00          callq  0x48c
     2a2:       48 8b c3                mov    %rbx,%rax
     2a5:       48 8b 5c 24 30          mov    0x30(%rsp),%rbx
     2aa:       48 83 c4 20             add    $0x20,%rsp
     2ae:       5f                      pop    %rdi
     2af:       c3                      retq
>>   2b0:       40 53                   rex push %rbx
     2b2:       48 83 ec 20             sub    $0x20,%rsp
     2b6:       48 89 0d e3 b9 05 00    mov    %rcx,0x5b9e3(%rip)
  # 0x5bca0
     2bd:       4c 8d 05 f4 b9 05 00    lea    0x5b9f4(%rip),%r8
 # 0x5bcb8
>>   2c4:       48 8b 42 60             mov    0x60(%rdx),%rax

and at 0x2c4 %rdx is 0x2..  I always thought x86 asm syntax strange,
but I assume that is trying to write to value of %rdx + offset of
0x60??  But this is a register never written, so I assume it is
expected to be passed from efi_loader?

From https://en.wikipedia.org/wiki/X86_calling_conventions it seems
that MS calling convention expects 2nd arg in %rdx, but linux/gcc
calling convention expects 3rd arg in %rdx (there is no 3rd arg)..

BR,
-

[U-Boot] [PATCH 08/16] sandbox: Add a setjmp() implementation

[Rob Clark]

On Mon, Sep 18, 2017 at 2:01 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 09/18/2017 12:59 AM, Simon Glass wrote:
>> Add an implementation of setjmp() and longjmp() which rely on the
>> underlying host C library. Since we cannot know how large the jump buffer
>> needs to be, pick something that should be suitable and check it at
>> runtime. At present we need access to the underlying struct as well.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  arch/sandbox/cpu/cpu.c            | 13 +++++++++++++
>>  arch/sandbox/cpu/os.c             | 17 +++++++++++++++++
>>  arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++++++++
>>  include/os.h                      | 21 +++++++++++++++++++++
>>  4 files changed, 72 insertions(+)
>>  create mode 100644 arch/sandbox/include/asm/setjmp.h
>>
>> diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
>> index 01991049cc..de5862a53b 100644
>> --- a/arch/sandbox/cpu/cpu.c
>> +++ b/arch/sandbox/cpu/cpu.c
>> @@ -9,6 +9,7 @@
>>  #include <libfdt.h>
>>  #include <os.h>
>>  #include <asm/io.h>
>> +#include <asm/setjmp.h>
>>  #include <asm/state.h>
>>  #include <dm/root.h>
>>
>> @@ -154,3 +155,15 @@ ulong timer_get_boot_us(void)
>>
>>       return (count - base_count) / 1000;
>>  }
>> +
>> +int setjmp(jmp_buf jmp)
>> +{
>> +     return os_setjmp((ulong *)jmp, sizeof(jmp));
>> +}
>> +
>> +void longjmp(jmp_buf jmp, int ret)
>> +{
>> +     os_longjmp((ulong *)jmp, ret);
>> +     while (1)
>> +             ;
>> +}
>> diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
>> index 22d6aab534..909034fa4b 100644
>> --- a/arch/sandbox/cpu/os.c
>> +++ b/arch/sandbox/cpu/os.c
>> @@ -7,6 +7,7 @@
>>  #include <errno.h>
>>  #include <fcntl.h>
>>  #include <getopt.h>
>> +#include <setjmp.h>
>>  #include <stdio.h>
>>  #include <stdint.h>
>>  #include <stdlib.h>
>> @@ -609,3 +610,19 @@ void os_localtime(struct rtc_time *rt)
>>       rt->tm_yday = tm->tm_yday;
>>       rt->tm_isdst = tm->tm_isdst;
>>  }
>> +
>> +int os_setjmp(ulong *jmp, int size)
>> +{
>> +     if (size < sizeof(jmp_buf)) {
>> +             printf("setjmp: jmpbuf is too small (%d bytes, need %d)\n",
>> +                    size, sizeof(jmp_buf));
>> +             return -ENOSPC;
>> +     }
>> +
>> +     return setjmp((struct __jmp_buf_tag *)jmp);
>> +}
>> +
>> +void os_longjmp(ulong *jmp, int ret)
>> +{
>> +     longjmp((struct __jmp_buf_tag *)jmp, ret);
>> +}
>> diff --git a/arch/sandbox/include/asm/setjmp.h b/arch/sandbox/include/asm/setjmp.h
>> new file mode 100644
>> index 0000000000..e25f50107c
>> --- /dev/null
>> +++ b/arch/sandbox/include/asm/setjmp.h
>> @@ -0,0 +1,21 @@
>> +/*
>> + * (C) Copyright 2016
>> + * Alexander Graf <agraf@suse.de>
>> + *
>> + * SPDX-License-Identifier:  GPL-2.0+
>> + */
>> +
>> +#ifndef _SETJMP_H_
>> +#define _SETJMP_H_   1
>> +
>> +struct jmp_buf_data {
>> +     /* We're not sure how long this should be */
>> +     ulong data[32];
>> +};
>> +
>> +typedef struct jmp_buf_data jmp_buf[1];
>> +
>> +int setjmp(jmp_buf jmp);
>> +__noreturn void longjmp(jmp_buf jmp, int ret);
>> +
>> +#endif /* _SETJMP_H_ */
>> diff --git a/include/os.h b/include/os.h
>> index 2bf4bdb1b8..ad1836ac9f 100644
>> --- a/include/os.h
>> +++ b/include/os.h
>> @@ -310,4 +310,25 @@ int os_spl_to_uboot(const char *fname);
>>   */
>>  void os_localtime(struct rtc_time *rt);
>>
>> +/**
>> + * os_setjmp() - Call setjmp()
>> + *
>> + * Call the host system's setjmp() function.
>> + *
>> + * @jmp: Buffer to store current execution state
>> + * @size: Size of buffer
>> + * @return normal setjmp() value if OK, -ENOSPC if @size is too small
>> + */
>> +int os_setjmp(ulong *jmp, int size);
>> +
>> +/**
>> + * os_longjmp() - Call longjmp()
>> + *
>> + * Call the host system's longjmp() function.
>> + *
>> + * @jmp: Buffer where previous execution state was stored
>> + * @ret: Value to pass to longjmp()
>> + */
>> +void os_longjmp(ulong *jmp, int ret);
>> +
>>  #endif
>>
>
> Please, fix this warning:
>
> arch/sandbox/cpu/cpu.c: In function ‘setjmp’:
> arch/sandbox/cpu/cpu.c:161:39: warning: ‘sizeof’ on array function
> parameter ‘jmp’ will return size of ‘struct jmp_buf_data *’
> [-Wsizeof-array-argument]
>   return os_setjmp((ulong *)jmp, sizeof(jmp));
>                                        ^
> arch/sandbox/cpu/cpu.c:159:20: note: declared here
>  int setjmp(jmp_buf jmp)
>

with the sizeof changed to sizeof(jmp_buf), I'm getting:

...
EFI: Entry efi_exit(00007fffffffc198, 2147483662, 0, 0000000000000000)

Program received signal SIGSEGV, Segmentation fault.
os_longjmp (jmp=0x7fffffffc200,
    jmp at entry=<error reading variable: DWARF-2 expression error: Loop
detected (257).>, ret=1,
    ret at entry=<error reading variable: DWARF-2 expression error: Loop
detected (257).>)
    at ../arch/sandbox/cpu/os.c:627
627        longjmp((struct __jmp_buf_tag *)jmp, ret);


So it seems like something not quite right still..

BR,
-R

[U-Boot] [PATCH 00/16] efi: Enable basic sandbox support for EFI loader

[Simon Glass]

Hi Rob,

On 18 September 2017 at 11:03, Rob Clark <robdclark@gmail.com> wrote:
> On Mon, Sep 18, 2017 at 11:07 AM, Rob Clark <robdclark@gmail.com> wrote:
>> On Mon, Sep 18, 2017 at 10:30 AM, Rob Clark <robdclark@gmail.com> wrote:
>>> On Mon, Sep 18, 2017 at 9:31 AM, Rob Clark <robdclark@gmail.com> wrote:
>>>> On Mon, Sep 18, 2017 at 9:18 AM, Rob Clark <robdclark@gmail.com> wrote:
>>>>> On Sun, Sep 17, 2017 at 11:48 PM, Heinrich Schuchardt
>>>>> <xypron.glpk@gmx.de> wrote:
>>>>>> On 09/18/2017 12:59 AM, Simon Glass wrote:
>>>>>>> A limitation of the EFI loader at present is that it does not build with
>>>>>>> sandbox. This makes it hard to write tests, since sandbox is used for most
>>>>>>> testing in U-Boot.
>>>>>>>
>>>>>>> This series enables the EFI loader feature. It allows sandbox to build and
>>>>>>> run a trivial function which calls the EFI API to output a message.
>>>>>>>
>>>>>>> Much work remains but this should serve as a basis for adding tests more
>>>>>>> easily for EFI loader.
>>>>>>>
>>>>>>> This series sits on top of Heinrich's recent EFI test series. It is
>>>>>>> available at u-boot-dm/efi-working
>>>>>>>
>>>>>>>
>>>>>>> Simon Glass (16):
>>>>>>>   efi: Update efi_smbios_register() to return error code
>>>>>>>   efi: Move the init check inside efi_init_obj_list()
>>>>>>>   efi: Add error checking for efi_init_obj_list()
>>>>>>>   efi: Add a TODO to efi_init_obj_list()
>>>>>>>   efi: Correct header order in efi_memory
>>>>>>>   efi: sandbox: Adjust memory setup for sandbox
>>>>>>>   sandbox: smbios: Update to support sandbox
>>>>>>>   sandbox: Add a setjmp() implementation
>>>>>>>   efi: sandbox: Add required linker sections
>>>>>>>   efi: sandbox: Add distroboot support
>>>>>>>   Define board_quiesce_devices() in a shared location
>>>>>>>   Add a comment for board_quiesce_devices()
>>>>>>>   efi: sandbox: Add relocation constants
>>>>>>>   efi: Add a comment about duplicated ELF constants
>>>>>>>   efi: sandbox: Enable EFI loader builder for sandbox
>>>>>>>   efi: sandbox: Add a simple 'bootefi test' command
>>>>>>>
>>>>>>>  arch/arm/include/asm/u-boot-arm.h |  1 -
>>>>>>>  arch/sandbox/cpu/cpu.c            | 13 ++++++++++
>>>>>>>  arch/sandbox/cpu/os.c             | 17 ++++++++++++
>>>>>>>  arch/sandbox/cpu/u-boot.lds       | 29 +++++++++++++++++++++
>>>>>>>  arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++
>>>>>>>  arch/sandbox/lib/Makefile         |  2 +-
>>>>>>>  arch/sandbox/lib/sections.c       | 12 +++++++++
>>>>>>>  arch/x86/include/asm/u-boot-x86.h |  1 -
>>>>>>>  arch/x86/lib/bootm.c              |  4 ---
>>>>>>>  cmd/bootefi.c                     | 54 ++++++++++++++++++++++++++++++++++-----
>>>>>>>  common/bootm.c                    |  4 +++
>>>>>>>  configs/sandbox_defconfig         |  1 +
>>>>>>>  include/bootm.h                   |  8 ++++++
>>>>>>>  include/config_distro_bootcmd.h   |  2 +-
>>>>>>>  include/efi_loader.h              | 13 ++++++++--
>>>>>>>  include/os.h                      | 21 +++++++++++++++
>>>>>>>  lib/efi_loader/Kconfig            | 12 ++++++++-
>>>>>>>  lib/efi_loader/Makefile           |  1 +
>>>>>>>  lib/efi_loader/efi_boottime.c     |  4 +++
>>>>>>>  lib/efi_loader/efi_memory.c       | 33 +++++++++++++-----------
>>>>>>>  lib/efi_loader/efi_runtime.c      |  7 +++++
>>>>>>>  lib/efi_loader/efi_smbios.c       |  6 +++--
>>>>>>>  lib/efi_loader/efi_test.c         | 17 ++++++++++++
>>>>>>>  lib/smbios.c                      | 38 ++++++++++++++++++++-------
>>>>>>>  24 files changed, 277 insertions(+), 44 deletions(-)
>>>>>>>  create mode 100644 arch/sandbox/include/asm/setjmp.h
>>>>>>>  create mode 100644 arch/sandbox/lib/sections.c
>>>>>>>  create mode 100644 lib/efi_loader/efi_test.c
>>>>>>>
>>>>>> Thanks for enabling efi_loader on sandbox. That will make many things
>>>>>> easier.
>>>>>>
>>>>>> Unfortunately
>>>>>> efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>>>>>>                                  struct efi_system_table *systab)
>>>>>> {
>>>>>> ...
>>>>>>         boottime = systable->boottime;
>>>>>> ...
>>>>>>         ret = boottime->allocate_pool(EFI_BOOT_SERVICES_DATA, map_size,
>>>>>>                                       (void **)&memory_map);
>>>>>> leads to a segmentation fault:
>>>>>
>>>>> I'm seeing something similar, because:
>>>>>
>>>>> (gdb) print gd->bd->bi_dram[0]
>>>>> $2 = {start = 0, size = 134217728}
>>>>>
>>>>> u-boot expects 1:1 phys:virt mapping, so that probably won't work.
>>>>
>>>> The following quick hack works.. something similar could probably be
>>>> smashed in to ""
>>>>
>>>> --------
>>>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>>>> index cddafe2d43..da2079a4b1 100644
>>>> --- a/lib/efi_loader/efi_memory.c
>>>> +++ b/lib/efi_loader/efi_memory.c
>>>> @@ -459,9 +459,10 @@ int efi_memory_init(void)
>>>>      unsigned long uboot_start, uboot_pages;
>>>>      unsigned long uboot_stack_size = 16 * 1024 * 1024;
>>>>
>>>> -    efi_add_known_memory();
>>>>
>>>>      if (!IS_ENABLED(CONFIG_SANDBOX)) {
>>>> +        efi_add_known_memory();
>>>> +
>>>>          /* Add U-Boot */
>>>>          uboot_start = (gd->start_addr_sp - uboot_stack_size) &
>>>>                  ~EFI_PAGE_MASK;
>>>> @@ -476,6 +477,12 @@ int efi_memory_init(void)
>>>>          runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
>>>>          efi_add_memory_map(runtime_start, runtime_pages,
>>>>                     EFI_RUNTIME_SERVICES_CODE, false);
>>>> +    } else {
>>>> +#define SZ_256M    0x10000000
>>>> +        size_t sz = SZ_256M;
>>>> +        void *ram = os_malloc(sz);
>>>> +        efi_add_memory_map((uintptr_t)ram, sz >> EFI_PAGE_SHIFT,
>>>> +                   EFI_CONVENTIONAL_MEMORY, false);
>>>>      }
>>>>
>>>>  #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
>>>> --------
>>>>
>>>> With that I'm at least getting further..  efi_allocate_pool()
>>>> eventually fails, possibly making every small memory allocation page
>>>> aligned means that 256m isn't enough..
>>>
>>> Ok, still just as hacky, but works a bit better:
>>>
>>> ---------
>>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>>> index cddafe2d43..b546b5e35d 100644
>>> --- a/lib/efi_loader/efi_memory.c
>>> +++ b/lib/efi_loader/efi_memory.c
>>> @@ -14,6 +14,7 @@
>>>  #include <linux/list_sort.h>
>>>  #include <inttypes.h>
>>>  #include <watchdog.h>
>>> +#include <os.h>
>>>
>>>  DECLARE_GLOBAL_DATA_PTR;
>>>
>>> @@ -459,9 +460,9 @@ int efi_memory_init(void)
>>>      unsigned long uboot_start, uboot_pages;
>>>      unsigned long uboot_stack_size = 16 * 1024 * 1024;
>>>
>>> -    efi_add_known_memory();
>>> -
>>>      if (!IS_ENABLED(CONFIG_SANDBOX)) {
>>> +        efi_add_known_memory();
>>> +
>>>          /* Add U-Boot */
>>>          uboot_start = (gd->start_addr_sp - uboot_stack_size) &
>>>                  ~EFI_PAGE_MASK;
>>> @@ -476,6 +477,14 @@ int efi_memory_init(void)
>>>          runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
>>>          efi_add_memory_map(runtime_start, runtime_pages,
>>>                     EFI_RUNTIME_SERVICES_CODE, false);
>>> +    } else {
>>> +#define SZ_4K    0x00001000
>>> +#define SZ_256M    0x10000000
>>> +        size_t sz = SZ_256M;
>>> +        uintptr_t ram = (uintptr_t)os_malloc(sz + SZ_4K) + SZ_4K;
>>> +        efi_add_memory_map(ram & ~EFI_PAGE_MASK, sz >> EFI_PAGE_SHIFT,
>>> +                   EFI_CONVENTIONAL_MEMORY, false);
>>> +        gd->start_addr_sp = ~0;
>>>      }
>>>
>>>  #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
>>> ---------
>>>
>>> At this point it crashes in efi_load_pe() when it first tries to
>>> dereference the address of the image passed in, ie. I'm running:
>>>
>>>   host bind 0 x86_64-sct.img
>>>   load host 0:1 0x01000000 /efi/boot/shell.efi
>>>   bootefi 0x01000000
>>>
>>> Not sure if there is a better way to pick an address to load into.  Or
>>> maybe just assuming that PA==VA isn't a good idea in sandbox?
>>>
>>
>> Ok, I realized there is map_sysmem().. which gets me further..
>> efi_loader really expects identity mapping (PA==VA), and iirc this is
>> what UEFI spec expects too so I wouldn't necessarily call it a bug in
>> efi_loader.
>>
>
> So, I don't know x86(_64) asm or calling conventions as well as arm..
> but I wonder if we are screwing up something long those lines:
>
>
> 0000000000000280 <.text>:
>      280:       48 89 5c 24 08          mov    %rbx,0x8(%rsp)
>      285:       57                      push   %rdi
>      286:       48 83 ec 20             sub    $0x20,%rsp
>      28a:       48 8b f9                mov    %rcx,%rdi
>>>   28d:       e8 1e 00 00 00          callq  0x2b0
> this jump is taken to 0x2b0
>
>      292:       e8 2d 06 00 00          callq  0x8c4
>      297:       48 8b cf                mov    %rdi,%rcx
>      29a:       48 8b d8                mov    %rax,%rbx
>      29d:       e8 ea 01 00 00          callq  0x48c
>      2a2:       48 8b c3                mov    %rbx,%rax
>      2a5:       48 8b 5c 24 30          mov    0x30(%rsp),%rbx
>      2aa:       48 83 c4 20             add    $0x20,%rsp
>      2ae:       5f                      pop    %rdi
>      2af:       c3                      retq
>>>   2b0:       40 53                   rex push %rbx
>      2b2:       48 83 ec 20             sub    $0x20,%rsp
>      2b6:       48 89 0d e3 b9 05 00    mov    %rcx,0x5b9e3(%rip)
>   # 0x5bca0
>      2bd:       4c 8d 05 f4 b9 05 00    lea    0x5b9f4(%rip),%r8
>  # 0x5bcb8
>>>   2c4:       48 8b 42 60             mov    0x60(%rdx),%rax
>
> and at 0x2c4 %rdx is 0x2..  I always thought x86 asm syntax strange,
> but I assume that is trying to write to value of %rdx + offset of
> 0x60??  But this is a register never written, so I assume it is
> expected to be passed from efi_loader?
>
> From https://en.wikipedia.org/wiki/X86_calling_conventions it seems
> that MS calling convention expects 2nd arg in %rdx, but linux/gcc
> calling convention expects 3rd arg in %rdx (there is no 3rd arg)..

I don't know it well either. But so long as functions are properly
declared in header files I don't really understand how we can have a
mismatch.

Regards,
Simon

[U-Boot] [PATCH 08/16] sandbox: Add a setjmp() implementation

[Simon Glass]

Hi,

On 18 September 2017 at 13:10, Rob Clark <robdclark@gmail.com> wrote:
> On Mon, Sep 18, 2017 at 2:01 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> On 09/18/2017 12:59 AM, Simon Glass wrote:
>>> Add an implementation of setjmp() and longjmp() which rely on the
>>> underlying host C library. Since we cannot know how large the jump buffer
>>> needs to be, pick something that should be suitable and check it at
>>> runtime. At present we need access to the underlying struct as well.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>>  arch/sandbox/cpu/cpu.c            | 13 +++++++++++++
>>>  arch/sandbox/cpu/os.c             | 17 +++++++++++++++++
>>>  arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++++++++
>>>  include/os.h                      | 21 +++++++++++++++++++++
>>>  4 files changed, 72 insertions(+)
>>>  create mode 100644 arch/sandbox/include/asm/setjmp.h
>>>
>>> diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
>>> index 01991049cc..de5862a53b 100644
>>> --- a/arch/sandbox/cpu/cpu.c
>>> +++ b/arch/sandbox/cpu/cpu.c
>>> @@ -9,6 +9,7 @@
>>>  #include <libfdt.h>
>>>  #include <os.h>
>>>  #include <asm/io.h>
>>> +#include <asm/setjmp.h>
>>>  #include <asm/state.h>
>>>  #include <dm/root.h>
>>>
>>> @@ -154,3 +155,15 @@ ulong timer_get_boot_us(void)
>>>
>>>       return (count - base_count) / 1000;
>>>  }
>>> +
>>> +int setjmp(jmp_buf jmp)
>>> +{
>>> +     return os_setjmp((ulong *)jmp, sizeof(jmp));
>>> +}
>>> +
>>> +void longjmp(jmp_buf jmp, int ret)
>>> +{
>>> +     os_longjmp((ulong *)jmp, ret);
>>> +     while (1)
>>> +             ;
>>> +}
>>> diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
>>> index 22d6aab534..909034fa4b 100644
>>> --- a/arch/sandbox/cpu/os.c
>>> +++ b/arch/sandbox/cpu/os.c
>>> @@ -7,6 +7,7 @@
>>>  #include <errno.h>
>>>  #include <fcntl.h>
>>>  #include <getopt.h>
>>> +#include <setjmp.h>
>>>  #include <stdio.h>
>>>  #include <stdint.h>
>>>  #include <stdlib.h>
>>> @@ -609,3 +610,19 @@ void os_localtime(struct rtc_time *rt)
>>>       rt->tm_yday = tm->tm_yday;
>>>       rt->tm_isdst = tm->tm_isdst;
>>>  }
>>> +
>>> +int os_setjmp(ulong *jmp, int size)
>>> +{
>>> +     if (size < sizeof(jmp_buf)) {
>>> +             printf("setjmp: jmpbuf is too small (%d bytes, need %d)\n",
>>> +                    size, sizeof(jmp_buf));
>>> +             return -ENOSPC;
>>> +     }
>>> +
>>> +     return setjmp((struct __jmp_buf_tag *)jmp);
>>> +}
>>> +
>>> +void os_longjmp(ulong *jmp, int ret)
>>> +{
>>> +     longjmp((struct __jmp_buf_tag *)jmp, ret);
>>> +}
>>> diff --git a/arch/sandbox/include/asm/setjmp.h b/arch/sandbox/include/asm/setjmp.h
>>> new file mode 100644
>>> index 0000000000..e25f50107c
>>> --- /dev/null
>>> +++ b/arch/sandbox/include/asm/setjmp.h
>>> @@ -0,0 +1,21 @@
>>> +/*
>>> + * (C) Copyright 2016
>>> + * Alexander Graf <agraf@suse.de>
>>> + *
>>> + * SPDX-License-Identifier:  GPL-2.0+
>>> + */
>>> +
>>> +#ifndef _SETJMP_H_
>>> +#define _SETJMP_H_   1
>>> +
>>> +struct jmp_buf_data {
>>> +     /* We're not sure how long this should be */
>>> +     ulong data[32];
>>> +};
>>> +
>>> +typedef struct jmp_buf_data jmp_buf[1];
>>> +
>>> +int setjmp(jmp_buf jmp);
>>> +__noreturn void longjmp(jmp_buf jmp, int ret);
>>> +
>>> +#endif /* _SETJMP_H_ */
>>> diff --git a/include/os.h b/include/os.h
>>> index 2bf4bdb1b8..ad1836ac9f 100644
>>> --- a/include/os.h
>>> +++ b/include/os.h
>>> @@ -310,4 +310,25 @@ int os_spl_to_uboot(const char *fname);
>>>   */
>>>  void os_localtime(struct rtc_time *rt);
>>>
>>> +/**
>>> + * os_setjmp() - Call setjmp()
>>> + *
>>> + * Call the host system's setjmp() function.
>>> + *
>>> + * @jmp: Buffer to store current execution state
>>> + * @size: Size of buffer
>>> + * @return normal setjmp() value if OK, -ENOSPC if @size is too small
>>> + */
>>> +int os_setjmp(ulong *jmp, int size);
>>> +
>>> +/**
>>> + * os_longjmp() - Call longjmp()
>>> + *
>>> + * Call the host system's longjmp() function.
>>> + *
>>> + * @jmp: Buffer where previous execution state was stored
>>> + * @ret: Value to pass to longjmp()
>>> + */
>>> +void os_longjmp(ulong *jmp, int ret);
>>> +
>>>  #endif
>>>
>>
>> Please, fix this warning:
>>
>> arch/sandbox/cpu/cpu.c: In function ‘setjmp’:
>> arch/sandbox/cpu/cpu.c:161:39: warning: ‘sizeof’ on array function
>> parameter ‘jmp’ will return size of ‘struct jmp_buf_data *’
>> [-Wsizeof-array-argument]
>>   return os_setjmp((ulong *)jmp, sizeof(jmp));
>>                                        ^
>> arch/sandbox/cpu/cpu.c:159:20: note: declared here
>>  int setjmp(jmp_buf jmp)
>>
>
> with the sizeof changed to sizeof(jmp_buf), I'm getting:
>
> ...
> EFI: Entry efi_exit(00007fffffffc198, 2147483662, 0, 0000000000000000)
>
> Program received signal SIGSEGV, Segmentation fault.
> os_longjmp (jmp=0x7fffffffc200,
>     jmp at entry=<error reading variable: DWARF-2 expression error: Loop
> detected (257).>, ret=1,
>     ret at entry=<error reading variable: DWARF-2 expression error: Loop
> detected (257).>)
>     at ../arch/sandbox/cpu/os.c:627
> 627        longjmp((struct __jmp_buf_tag *)jmp, ret);
>
>
> So it seems like something not quite right still..

Hmm I'm not sure what, yet.

Regards,
Simon

[U-Boot] [PATCH 16/16] efi: sandbox: Add a simple 'bootefi test' command

[Simon Glass]

Hi Heinrich,

On 18 September 2017 at 05:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 09/18/2017 12:59 AM, Simon Glass wrote:
>> This jumps to test code which can call directly into the EFI support. It
>> does not need a separate image so it is easy to write tests with it.
>>
>> For now the test just outputs a message. To try it:
>>
>> ./sandbox/u-boot -c "bootefi test"
>> U-Boot 2017.09-00204-g696c9855fe (Sep 17 2017 - 16:43:53 -0600)
>>
>> DRAM:  128 MiB
>> MMC:
>> Using default environment
>>
>> In:    serial
>> Out:   serial
>> Err:   serial
>> SCSI:  Net:   No ethernet found.
>> IDE:   Bus 0: not available
>> Found 0 disks
>> Hello, world!
>> Test passed
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  cmd/bootefi.c             | 18 ++++++++++++++++++
>>  configs/sandbox_defconfig |  1 +
>>  include/efi_loader.h      |  3 +++
>>  lib/efi_loader/Kconfig    | 10 ++++++++++
>>  lib/efi_loader/Makefile   |  1 +
>>  lib/efi_loader/efi_test.c | 17 +++++++++++++++++
>>  6 files changed, 50 insertions(+)
>>  create mode 100644 lib/efi_loader/efi_test.c
>>
>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> index ee07733e3e..f499103d23 100644
>> --- a/cmd/bootefi.c
>> +++ b/cmd/bootefi.c
>> @@ -323,6 +323,24 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>               memcpy((char *)addr, __efi_hello_world_begin, size);
>>       } else
>>  #endif
>> +     if (IS_ENABLED(CONFIG_BOOTEFI_TEST) && !strcmp(argv[1], "test")) {
>> +             int ret;
>> +
>> +             /* Initialize and populate EFI object list */
>> +             if (efi_init_obj_list())
>> +                     return CMD_RET_FAILURE;
>> +
>> +             loaded_image_info.device_handle = bootefi_device_path;
>> +             loaded_image_info.file_path = bootefi_image_path;
>> +             ret = efi_test(&loaded_image_info, &systab);
>> +             if (ret) {
>> +                     printf("Test failed: err=%d\n", ret);
>> +                     return CMD_RET_FAILURE;
>> +             }
>> +             printf("Test passed\n");
>> +
>> +             return 0;
>> +     }
>>  #ifdef CONFIG_CMD_BOOTEFI_SELFTEST
>>       if (!strcmp(argv[1], "selftest")) {
>>               /*
>> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
>> index 72600afea8..ab63f639de 100644
>> --- a/configs/sandbox_defconfig
>> +++ b/configs/sandbox_defconfig
>> @@ -196,3 +196,4 @@ CONFIG_UT_TIME=y
>>  CONFIG_UT_DM=y
>>  CONFIG_UT_ENV=y
>>  CONFIG_UT_OVERLAY=y
>> +CONFIG_CMD_BOOTEFI_SELFTEST=y
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index 79d2dad22c..43919137b0 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -263,6 +263,9 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>>                                struct efi_system_table *systab);
>>  #endif
>>
>> +/* Perform EFI tests */
>> +int efi_test(efi_handle_t image_handle, struct efi_system_table *systab);
>> +
>>  #else /* defined(EFI_LOADER) && !defined(CONFIG_SPL_BUILD) */
>>
>>  /* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it out */
>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>> index dee0a96a98..659b2b18f4 100644
>> --- a/lib/efi_loader/Kconfig
>> +++ b/lib/efi_loader/Kconfig
>> @@ -16,3 +16,13 @@ config EFI_LOADER_BOUNCE_BUFFER
>>         Some hardware does not support DMA to full 64bit addresses. For this
>>         hardware we can create a bounce buffer so that payloads don't have to
>>         worry about platform details.
>> +
>> +config BOOTEFI_TEST
>> +     bool "Provide a test for the EFI loader"
>> +     depends on EFI_LOADER && SANDBOX
>> +     default y
>> +     help
>> +       Provides a test of the EFI loader functionality accessed via the
>> +       command line ('bootefi test'). This runs within U-Boot so does not
>> +       need a separate EFI application to work. It aims to include coverage
>> +       of all EFI code which can be accessed within sandbox.
>> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
>> index 30bf343a36..69eb93518d 100644
>> --- a/lib/efi_loader/Makefile
>> +++ b/lib/efi_loader/Makefile
>> @@ -21,3 +21,4 @@ obj-$(CONFIG_DM_VIDEO) += efi_gop.o
>>  obj-$(CONFIG_PARTITIONS) += efi_disk.o
>>  obj-$(CONFIG_NET) += efi_net.o
>>  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
>> +obj-$(CONFIG_BOOTEFI_TEST) += efi_test.o
>> diff --git a/lib/efi_loader/efi_test.c b/lib/efi_loader/efi_test.c
>> new file mode 100644
>> index 0000000000..3fdce78b05
>> --- /dev/null
>> +++ b/lib/efi_loader/efi_test.c
>> @@ -0,0 +1,17 @@
>> +/*
>> + * Copyright (c) 2017, Google Inc. All rights reserved.
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <efi_api.h>
>> +
>> +int efi_test(efi_handle_t image_handle, struct efi_system_table *systable)
>> +{
>> +     struct efi_simple_text_output_protocol *con_out = systable->con_out;
>> +
>> +     con_out->output_string(con_out, L"Hello, world!\n");
>> +
>> +     return 0;
>> +}
>>
>
> Hello Simon,
>
> why do we need this patch?
> You could just call bootefi selftest to demonstrate the same (after
> fixing the memory map initialization for the sandbox).

Yes, but I'd like to use sandbox for testing where possible.

>
> Could you, please, review the framework that I have setup for bootefi
> selftest. Does the design make sense to you?

Yes it looks good, now reviewed. I think we need both:

- sandbox test for ease of use (but will not provide 100% coverage,
e.g. I'm not sure we can implement exit boot services)
- selftest to actually test the API calls properly (should be able to
test nearly everything here)

Regards,
Simon

[U-Boot] [PATCH 00/16] efi: Enable basic sandbox support for EFI loader

[Simon Glass]

Hi Rob,

On 18 September 2017 at 09:07, Rob Clark <robdclark@gmail.com> wrote:
> On Mon, Sep 18, 2017 at 10:30 AM, Rob Clark <robdclark@gmail.com> wrote:
>> On Mon, Sep 18, 2017 at 9:31 AM, Rob Clark <robdclark@gmail.com> wrote:
>>> On Mon, Sep 18, 2017 at 9:18 AM, Rob Clark <robdclark@gmail.com> wrote:
>>>> On Sun, Sep 17, 2017 at 11:48 PM, Heinrich Schuchardt
>>>> <xypron.glpk@gmx.de> wrote:
>>>>> On 09/18/2017 12:59 AM, Simon Glass wrote:
>>>>>> A limitation of the EFI loader at present is that it does not build with
>>>>>> sandbox. This makes it hard to write tests, since sandbox is used for most
>>>>>> testing in U-Boot.
>>>>>>
>>>>>> This series enables the EFI loader feature. It allows sandbox to build and
>>>>>> run a trivial function which calls the EFI API to output a message.
>>>>>>
>>>>>> Much work remains but this should serve as a basis for adding tests more
>>>>>> easily for EFI loader.
>>>>>>
>>>>>> This series sits on top of Heinrich's recent EFI test series. It is
>>>>>> available at u-boot-dm/efi-working
>>>>>>
>>>>>>
>>>>>> Simon Glass (16):
>>>>>>   efi: Update efi_smbios_register() to return error code
>>>>>>   efi: Move the init check inside efi_init_obj_list()
>>>>>>   efi: Add error checking for efi_init_obj_list()
>>>>>>   efi: Add a TODO to efi_init_obj_list()
>>>>>>   efi: Correct header order in efi_memory
>>>>>>   efi: sandbox: Adjust memory setup for sandbox
>>>>>>   sandbox: smbios: Update to support sandbox
>>>>>>   sandbox: Add a setjmp() implementation
>>>>>>   efi: sandbox: Add required linker sections
>>>>>>   efi: sandbox: Add distroboot support
>>>>>>   Define board_quiesce_devices() in a shared location
>>>>>>   Add a comment for board_quiesce_devices()
>>>>>>   efi: sandbox: Add relocation constants
>>>>>>   efi: Add a comment about duplicated ELF constants
>>>>>>   efi: sandbox: Enable EFI loader builder for sandbox
>>>>>>   efi: sandbox: Add a simple 'bootefi test' command
>>>>>>
>>>>>>  arch/arm/include/asm/u-boot-arm.h |  1 -
>>>>>>  arch/sandbox/cpu/cpu.c            | 13 ++++++++++
>>>>>>  arch/sandbox/cpu/os.c             | 17 ++++++++++++
>>>>>>  arch/sandbox/cpu/u-boot.lds       | 29 +++++++++++++++++++++
>>>>>>  arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++
>>>>>>  arch/sandbox/lib/Makefile         |  2 +-
>>>>>>  arch/sandbox/lib/sections.c       | 12 +++++++++
>>>>>>  arch/x86/include/asm/u-boot-x86.h |  1 -
>>>>>>  arch/x86/lib/bootm.c              |  4 ---
>>>>>>  cmd/bootefi.c                     | 54 ++++++++++++++++++++++++++++++++++-----
>>>>>>  common/bootm.c                    |  4 +++
>>>>>>  configs/sandbox_defconfig         |  1 +
>>>>>>  include/bootm.h                   |  8 ++++++
>>>>>>  include/config_distro_bootcmd.h   |  2 +-
>>>>>>  include/efi_loader.h              | 13 ++++++++--
>>>>>>  include/os.h                      | 21 +++++++++++++++
>>>>>>  lib/efi_loader/Kconfig            | 12 ++++++++-
>>>>>>  lib/efi_loader/Makefile           |  1 +
>>>>>>  lib/efi_loader/efi_boottime.c     |  4 +++
>>>>>>  lib/efi_loader/efi_memory.c       | 33 +++++++++++++-----------
>>>>>>  lib/efi_loader/efi_runtime.c      |  7 +++++
>>>>>>  lib/efi_loader/efi_smbios.c       |  6 +++--
>>>>>>  lib/efi_loader/efi_test.c         | 17 ++++++++++++
>>>>>>  lib/smbios.c                      | 38 ++++++++++++++++++++-------
>>>>>>  24 files changed, 277 insertions(+), 44 deletions(-)
>>>>>>  create mode 100644 arch/sandbox/include/asm/setjmp.h
>>>>>>  create mode 100644 arch/sandbox/lib/sections.c
>>>>>>  create mode 100644 lib/efi_loader/efi_test.c
>>>>>>
>>>>> Thanks for enabling efi_loader on sandbox. That will make many things
>>>>> easier.
>>>>>
>>>>> Unfortunately
>>>>> efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>>>>>                                  struct efi_system_table *systab)
>>>>> {
>>>>> ...
>>>>>         boottime = systable->boottime;
>>>>> ...
>>>>>         ret = boottime->allocate_pool(EFI_BOOT_SERVICES_DATA, map_size,
>>>>>                                       (void **)&memory_map);
>>>>> leads to a segmentation fault:
>>>>
>>>> I'm seeing something similar, because:
>>>>
>>>> (gdb) print gd->bd->bi_dram[0]
>>>> $2 = {start = 0, size = 134217728}
>>>>
>>>> u-boot expects 1:1 phys:virt mapping, so that probably won't work.
>>>
>>> The following quick hack works.. something similar could probably be
>>> smashed in to ""
>>>
>>> --------
>>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>>> index cddafe2d43..da2079a4b1 100644
>>> --- a/lib/efi_loader/efi_memory.c
>>> +++ b/lib/efi_loader/efi_memory.c
>>> @@ -459,9 +459,10 @@ int efi_memory_init(void)
>>>      unsigned long uboot_start, uboot_pages;
>>>      unsigned long uboot_stack_size = 16 * 1024 * 1024;
>>>
>>> -    efi_add_known_memory();
>>>
>>>      if (!IS_ENABLED(CONFIG_SANDBOX)) {
>>> +        efi_add_known_memory();
>>> +
>>>          /* Add U-Boot */
>>>          uboot_start = (gd->start_addr_sp - uboot_stack_size) &
>>>                  ~EFI_PAGE_MASK;
>>> @@ -476,6 +477,12 @@ int efi_memory_init(void)
>>>          runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
>>>          efi_add_memory_map(runtime_start, runtime_pages,
>>>                     EFI_RUNTIME_SERVICES_CODE, false);
>>> +    } else {
>>> +#define SZ_256M    0x10000000
>>> +        size_t sz = SZ_256M;
>>> +        void *ram = os_malloc(sz);
>>> +        efi_add_memory_map((uintptr_t)ram, sz >> EFI_PAGE_SHIFT,
>>> +                   EFI_CONVENTIONAL_MEMORY, false);
>>>      }
>>>
>>>  #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
>>> --------
>>>
>>> With that I'm at least getting further..  efi_allocate_pool()
>>> eventually fails, possibly making every small memory allocation page
>>> aligned means that 256m isn't enough..
>>
>> Ok, still just as hacky, but works a bit better:
>>
>> ---------
>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>> index cddafe2d43..b546b5e35d 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -14,6 +14,7 @@
>>  #include <linux/list_sort.h>
>>  #include <inttypes.h>
>>  #include <watchdog.h>
>> +#include <os.h>
>>
>>  DECLARE_GLOBAL_DATA_PTR;
>>
>> @@ -459,9 +460,9 @@ int efi_memory_init(void)
>>      unsigned long uboot_start, uboot_pages;
>>      unsigned long uboot_stack_size = 16 * 1024 * 1024;
>>
>> -    efi_add_known_memory();
>> -
>>      if (!IS_ENABLED(CONFIG_SANDBOX)) {
>> +        efi_add_known_memory();
>> +
>>          /* Add U-Boot */
>>          uboot_start = (gd->start_addr_sp - uboot_stack_size) &
>>                  ~EFI_PAGE_MASK;
>> @@ -476,6 +477,14 @@ int efi_memory_init(void)
>>          runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
>>          efi_add_memory_map(runtime_start, runtime_pages,
>>                     EFI_RUNTIME_SERVICES_CODE, false);
>> +    } else {
>> +#define SZ_4K    0x00001000
>> +#define SZ_256M    0x10000000
>> +        size_t sz = SZ_256M;
>> +        uintptr_t ram = (uintptr_t)os_malloc(sz + SZ_4K) + SZ_4K;
>> +        efi_add_memory_map(ram & ~EFI_PAGE_MASK, sz >> EFI_PAGE_SHIFT,
>> +                   EFI_CONVENTIONAL_MEMORY, false);
>> +        gd->start_addr_sp = ~0;
>>      }
>>
>>  #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
>> ---------
>>
>> At this point it crashes in efi_load_pe() when it first tries to
>> dereference the address of the image passed in, ie. I'm running:
>>
>>   host bind 0 x86_64-sct.img
>>   load host 0:1 0x01000000 /efi/boot/shell.efi
>>   bootefi 0x01000000
>>
>> Not sure if there is a better way to pick an address to load into.  Or
>> maybe just assuming that PA==VA isn't a good idea in sandbox?
>>
>
> Ok, I realized there is map_sysmem().. which gets me further..
> efi_loader really expects identity mapping (PA==VA), and iirc this is
> what UEFI spec expects too so I wouldn't necessarily call it a bug in
> efi_loader.

Well we need to make it work :-)

In general casting an address to a pointer (or vice versa) should be
avoided in U-Boot now that we have map_sysmem(). It is incompatible
with sandbox.

Regards,
Simon

[U-Boot] [PATCH 03/16] efi: Add error checking for efi_init_obj_list()

[Simon Glass]

Hi Heinrich,

On 17 September 2017 at 22:33, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 09/18/2017 12:59 AM, Simon Glass wrote:
> > This function calls a function which can fail. Print a message in this
> > case and abort the boot, rather than silently continuing to boot, which
> > will certainly fail.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  cmd/bootefi.c | 30 ++++++++++++++++++++++++------
> >  1 file changed, 24 insertions(+), 6 deletions(-)
> >
> > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > index 2c9d31c5eb..9aa588eb1b 100644
> > --- a/cmd/bootefi.c
> > +++ b/cmd/bootefi.c
> > @@ -106,11 +106,17 @@ static struct efi_object bootefi_device_obj = {
> >       },
> >  };
> >
> > -/* Initialize and populate EFI object list */
> > -static void efi_init_obj_list(void)
> > +/**
> > + * efi_init_obj_list() - Initialize and populate EFI object list
> > + *
> > + * @return 0 if OK, -ve on error (in which case it prints a message)
> > + */
> > +static int efi_init_obj_list(void)
>
> Use efi_status_t as return type.
>
> >  {
> > +     int ret;
> > +
> >       if (efi_obj_list_initalized)
> > -             return;
> > +             return 0;
> >       efi_obj_list_initalized = 1;
> >
> >       list_add_tail(&loaded_image_info_obj.link, &efi_obj_list);
> > @@ -132,12 +138,19 @@ static void efi_init_obj_list(void)
> >               loaded_image_info.device_handle = bootefi_device_path;
> >  #endif
> >  #ifdef CONFIG_GENERATE_SMBIOS_TABLE
> > -     efi_smbios_register();
> > +     ret = efi_smbios_register();
> > +     if (ret)
> > +             goto error;
> >  #endif
> >
> >       /* Initialize EFI runtime services */
> >       efi_reset_system_init();
> >       efi_get_time_init();
> > +
> > +     return 0;
> > +error:
> > +     printf("Error: Cannot set up EFI object list (err=%d)\n", ret);
> > +     return ret;
> >  }
> >
> >  static void *copy_fdt(void *fdt)
> > @@ -219,6 +232,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
> >       ulong fdt_pages, fdt_size, fdt_start, fdt_end;
> >       const efi_guid_t fdt_guid = EFI_FDT_GUID;
> >       bootm_headers_t img = { 0 };
> > +     int ret;
> >
> >       /*
> >        * gd lives in a fixed register which may get clobbered while we execute
> > @@ -258,7 +272,9 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
> >               return -ENOENT;
> >
> >       /* Initialize and populate EFI object list */
> > -     efi_init_obj_list();
> > +     ret = efi_init_obj_list();
> > +     if (ret)
> > +             return ret;
> >
> >       /* Call our payload! */
> >       debug("%s:%d Jumping to 0x%lx\n", __func__, __LINE__, (long)entry);
> > @@ -312,7 +328,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >                */
> >               efi_save_gd();
> >               /* Initialize and populate EFI object list */
> > -             efi_init_obj_list();
> > +             if (efi_init_obj_list())
> > +                     return CMD_RET_FAILURE;
> > +
>
> We are duplicating code here.
>
> efi_save_gd and efi_init_obj_list should have been moved to the start of
> do_bootefi_exec in my patch
> efi_selftest: provide unit test for event services
>

Yes I see that but I wonder if we should wait until your series is
applied before figuring this out?

>
> >               loaded_image_info.device_handle = bootefi_device_path;
> >               loaded_image_info.file_path = bootefi_image_path;
> >               return efi_selftest(&loaded_image_info, &systab);
> >
>

Regards,
Simon

[U-Boot] [PATCH 04/16] efi: Add a TODO to efi_init_obj_list()

[Simon Glass]

Hi Heinrich,

On 17 September 2017 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 09/18/2017 12:59 AM, Simon Glass wrote:
>> This function repeats data structures provided by driver model. They are
>> only created once so can be stale if the EFI loader is called twice (e.g.
>> for testing or on boot failure).
>>
>> Add a TODO to address this. It should be possible to attach EFI devices
>> and data structures to driver-model devices and avoid having a parallel
>> set of data structures.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  cmd/bootefi.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> index 9aa588eb1b..ee07733e3e 100644
>> --- a/cmd/bootefi.c
>> +++ b/cmd/bootefi.c
>> @@ -109,6 +109,10 @@ static struct efi_object bootefi_device_obj = {
>>  /**
>>   * efi_init_obj_list() - Initialize and populate EFI object list
>>   *
>> + * TODO(sjg at chromium.org): Move this to a dynamic list based on driver model,
>> + * so that it does not need to be created before running EFI applications
>> + * and updates when devices change.
>> + *
>
> I am not quite sure if by dynamic list you refer to linker generated
> lists or to hot plugging.
>
> The UEFI spec 2.7 has a chapter on "Hot-Plug Events". This would add a
> lot of complexity. I do not think that we currently need it.
>
> The object list also gets new entries created by the EFI application via
> calling InstallMultipleProtocolInterfaces of InstallProtocolInterface
> with *handle == NULL.

Here I am referring to what I see as duplicate device tables, brought
in from driver model. I am wondering if we can make the EFI info come
from the driver-model directly.

Regards,
Simon

[U-Boot] [PATCH 06/16] efi: sandbox: Adjust memory setup for sandbox

[Simon Glass]

Hi Heinrich,

On 18 September 2017 at 00:12, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 09/18/2017 12:59 AM, Simon Glass wrote:
>> With sandbox the U-Boot code is not mapped into the sandbox memory range
>> so does not need to be excluded when allocating EFI memory. Update the EFI
>> memory init code to take account of that.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  lib/efi_loader/efi_memory.c | 28 ++++++++++++++++------------
>>  1 file changed, 16 insertions(+), 12 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>> index ad3d277be6..c1a080e2e9 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -459,18 +459,22 @@ int efi_memory_init(void)
>>
>>       efi_add_known_memory();
>>
>> -     /* Add U-Boot */
>> -     uboot_start = (gd->start_addr_sp - uboot_stack_size) & ~EFI_PAGE_MASK;
>> -     uboot_pages = (gd->ram_top - uboot_start) >> EFI_PAGE_SHIFT;
>> -     efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA, false);
>> -
>> -     /* Add Runtime Services */
>> -     runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
>> -     runtime_end = (ulong)&__efi_runtime_stop;
>> -     runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
>> -     runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
>> -     efi_add_memory_map(runtime_start, runtime_pages,
>> -                        EFI_RUNTIME_SERVICES_CODE, false);
>> +     if (!IS_ENABLED(CONFIG_SANDBOX)) {
>
> We need the memory maps for:
> GetMemoryMap
> ExitBootServices
>
> In efi_init_memory_init you define 8 MB memory starting at
> 0x0000000000000000 as available:
> ram_start 0
> start 0
> pages 8000
>
> I suggest you override efi_init_memory_init using malloc to assign at
> least 128 MB contiguous memory with alignment EFI_PAGE_SIZE.

Do you mean outside the sandbox memory region? Sandbox works by having
a memory region from which all allocation comes.

Regards,
Simon

[U-Boot] [PATCH 13/16] efi: sandbox: Add relocation constants

[Simon Glass]

Hi Heinrich,

On 18 September 2017 at 05:13, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 09/18/2017 12:59 AM, Simon Glass wrote:
>> Add these so that we can build the EFI loader for sandbox. The values are
>> for x86_64 so potentially bogus. But we don't support relocation within
>> sandbox anyway.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  lib/efi_loader/efi_runtime.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
>> index ad7f3754bd..8ad1d2fc99 100644
>> --- a/lib/efi_loader/efi_runtime.c
>> +++ b/lib/efi_loader/efi_runtime.c
>> @@ -48,6 +48,9 @@ static efi_status_t __efi_runtime EFIAPI efi_invalid_parameter(void);
>>  #include <asm/elf.h>
>>  #define R_RELATIVE   R_386_RELATIVE
>>  #define R_MASK               0xffULL
>> +#elif defined(CONFIG_SANDBOX)
>
> For the sandbox you should make your choice based on the host
> architecture. I want to be able to work on my armv8 machine.

Sure, but I'm not sure how to do that. We need to know the host and
adjust EFI accordingly I think. Perhaps something to do later.

>
>> +#define R_RELATIVE   8
>
> You mean R_X86_64_RELATIVE (defined in arch/x86/include/asm/elf.h)?

Regards,
Simon