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

[U-Boot] [PATCH v10 00/10] efi: Enable 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.

Also included in v8 is support for running the full EFI self tests. These
run OK with some tweaks to a few parts of the code.

With v9, various EFI patches have been applied which change things. This
series includes a partial review of one, which makes 'bootefi test' work.
But there are still problems with 'bootefi selftest':

As of v10 two selftests fail. I fixed another one in this series. I think
have removed all paches not essential to getting things running. The
refactoring patches will be sent separately once these are sorted out.

$ sandbox/u-boot -D -c "bootefi selftest"
...
Executing 'real time clock'
/home/sjg/c/src/third_party/u-boot/files/lib/efi_selftest/efi_selftest_rtc.c(46):
ERROR: Could not read real time clock
/home/sjg/c/src/third_party/u-boot/files/lib/efi_selftest/efi_selftest.c(109):
ERROR: Executing 'real time clock' failed

Setting up 'simple network protocol'
/home/sjg/c/src/third_party/u-boot/files/lib/efi_selftest/efi_selftest_snp.c(209):
ERROR: Failed to locate simple network protocol
/home/sjg/c/src/third_party/u-boot/files/lib/efi_selftest/efi_selftest.c(84):
ERROR: Setting up 'simple network protocol' failed

This series is at u-boot-dm/efi-working

Changes in v10:
- Try to use compiler pre-defines to get the right settings for sandbox
- Explain the padding of the returned pointer in the commit message
- Add new patch to correct the operation of efi_file_write()
- Drop patches not essential to getting things working

Changes in v9:
- Fix 'thi' typo
- Drop fdt_end variable in efi_install_fdt()

Changes in v8:
- Rebase to master, bringing in all EFI changes
- Drop 'efi: sandbox: Add relocation constants'
- Drop 'efi: Adjust memory handling to support sandbox'
- Drop 'sandbox: smbios: Update to support sandbox'
- Expand series substantially to support bootefi selftest
- Rebase to master

Changes in v7:
- Drop an unnecessary comment
- Update patch subject s/builder/build/

Changes in v6:
- Warn about building sandbox EFI support on something other than __x86_64__

Changes in v4:
- Rebase to master
- Update SPDX tags

Changes in v3:
- Init the 'rows' and 'cols' vars to avoid a compiler error (gcc 4.8.4)
- Rebase to master

Changes in v2:
- Rebase to master

Alexander Graf (1):
  efi_loader: Pass address to fs_read()

Simon Glass (9):
  efi: sandbox: Add distroboot support
  efi: sandbox: Enable EFI loader build for sandbox
  sandbox: Align RAM buffer to the machine page size
  sandbox: Add support for calling abort()
  sandbox: Enhance map_to_sysmem() to handle foreign pointers
  efi: Relocate FDT to 127MB instead of 128MB
  efi: sandbox: Tidy up copy_fdt() to work with sandbox
  efi: Correct the operation of efi_file_write()
  efi: sandbox: Enable selftest command

 arch/sandbox/cpu/cpu.c           | 141 +++++++++++++++++++++++++++++--
 arch/sandbox/cpu/os.c            |  12 ++-
 arch/sandbox/cpu/state.c         |   8 ++
 arch/sandbox/include/asm/state.h |  21 +++++
 cmd/bootefi.c                    |  83 +++++++++++-------
 configs/sandbox_defconfig        |   1 +
 include/config_distro_bootcmd.h  |  16 ++--
 include/os.h                     |   4 +
 lib/efi_loader/Kconfig           |   2 +-
 lib/efi_loader/efi_file.c        |   5 +-
 10 files changed, 241 insertions(+), 52 deletions(-)

-- 
2.19.0.397.gdd90340f6a-goog

[U-Boot] [PATCH v10 01/10] 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>
---

Changes in v10:
- Try to use compiler pre-defines to get the right settings for sandbox

Changes in v9: None
Changes in v8: None
Changes in v7:
- Drop an unnecessary comment

Changes in v6:
- Warn about building sandbox EFI support on something other than __x86_64__

Changes in v4: None
Changes in v3: None
Changes in v2: None

 include/config_distro_bootcmd.h | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
index d672e8ebe65..373fee78a99 100644
--- a/include/config_distro_bootcmd.h
+++ b/include/config_distro_bootcmd.h
@@ -245,22 +245,26 @@
 #if defined(CONFIG_CMD_DHCP)
 #if defined(CONFIG_EFI_LOADER)
 /* http://www.iana.org/assignments/dhcpv6-parameters/dhcpv6-parameters.xml */
-#if defined(CONFIG_ARM64)
+#if defined(CONFIG_ARM64) || defined(__aarch64__)
 #define BOOTENV_EFI_PXE_ARCH "0xb"
 #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00011:UNDI:003000"
-#elif defined(CONFIG_ARM)
+#elif defined(CONFIG_ARM) || defined(__arm__)
 #define BOOTENV_EFI_PXE_ARCH "0xa"
 #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00010:UNDI:003000"
-#elif defined(CONFIG_X86)
-/* Always assume we're running 64bit */
+#elif defined(CONFIG_X86) || defined(__x86_64__)
 #define BOOTENV_EFI_PXE_ARCH "0x7"
 #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00007:UNDI:003000"
-#elif defined(CONFIG_CPU_RISCV_32)
+#elif defined(__i386__)
+#define BOOTENV_EFI_PXE_ARCH "0x6"
+#define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00006:UNDI:003000"
+#elif defined(CONFIG_CPU_RISCV_32) || ((defined(__riscv) && __riscv_xlen == 32))
 #define BOOTENV_EFI_PXE_ARCH "0x19"
 #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00025:UNDI:003000"
-#elif defined(CONFIG_CPU_RISCV_64)
+#elif defined(CONFIG_CPU_RISCV_64) || ((defined(__riscv) && __riscv_xlen == 64))
 #define BOOTENV_EFI_PXE_ARCH "0x1b"
 #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00027:UNDI:003000"
+#elif defined(CONFIG_SANDBOX)
+# error "sandbox EFI support is only supported on ARM and x86"
 #else
 #error Please specify an EFI client identifier
 #endif
-- 
2.19.0.397.gdd90340f6a-goog

[U-Boot] [PATCH v10 02/10] efi: sandbox: Enable EFI loader build for sandbox

[Simon Glass]

This allows this feature to build within sandbox. This is useful for
testing.

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

Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7:
- Update patch subject s/builder/build/

Changes in v6: None
Changes in v4: None
Changes in v3:
- Init the 'rows' and 'cols' vars to avoid a compiler error (gcc 4.8.4)

Changes in v2: None

 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 ce6a09f0b43..bfd7b19d791 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 || RISCV) && OF_LIBFDT
+	depends on (ARM || X86 || RISCV || SANDBOX) && OF_LIBFDT
 	# We need EFI_STUB_64BIT to be set on x86_64 with EFI_STUB
 	depends on !EFI_STUB || !X86_64 || EFI_STUB_64BIT
 	# We need EFI_STUB_32BIT to be set on x86_32 with EFI_STUB
-- 
2.19.0.397.gdd90340f6a-goog

[U-Boot] [PATCH v10 03/10] sandbox: Align RAM buffer to the machine page size

[Simon Glass]

At present the sandbox RAM buffer is not aligned to any particular
address boundary. This makes the internal pointers somewhat random with
respect to the associated RAM buffer addresses.

Align the buffer to the page size of the machine to help with this. Note
that there is a header at the start of the allocated pointer. To avoid
returning a pointer which is not aligned to a page boundary, we waste
almost an entire page of memory for each allocation.

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

Changes in v10:
- Explain the padding of the returned pointer in the commit message

Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/sandbox/cpu/os.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index 5839932b005..a1a982af2de 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -143,14 +143,15 @@ void os_tty_raw(int fd, bool allow_sigs)
 void *os_malloc(size_t length)
 {
 	struct os_mem_hdr *hdr;
+	int page_size = getpagesize();
 
-	hdr = mmap(NULL, length + sizeof(*hdr), PROT_READ | PROT_WRITE,
-		   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	hdr = mmap(NULL, length + page_size,
+		   PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 	if (hdr == MAP_FAILED)
 		return NULL;
 	hdr->length = length;
 
-	return hdr + 1;
+	return (void *)hdr + page_size;
 }
 
 void os_free(void *ptr)
-- 
2.19.0.397.gdd90340f6a-goog

[U-Boot] [PATCH v10 04/10] sandbox: Add support for calling abort()

[Simon Glass]

This function is useful to signal that the application needs to exit
immediate. It can be caught with a debugger (e.g. gdb). Add a stub for it
so that it can be called from within sandbox when an internal error
occurs.

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

Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/sandbox/cpu/os.c | 5 +++++
 include/os.h          | 4 ++++
 2 files changed, 9 insertions(+)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index a1a982af2de..ca8d924cd65 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -652,3 +652,8 @@ void os_longjmp(ulong *jmp, int ret)
 {
 	longjmp((struct __jmp_buf_tag *)jmp, ret);
 }
+
+void os_abort(void)
+{
+	abort();
+}
diff --git a/include/os.h b/include/os.h
index c8e0f52d306..e850f879ec3 100644
--- a/include/os.h
+++ b/include/os.h
@@ -351,4 +351,8 @@ int os_setjmp(ulong *jmp, int size);
  */
 void os_longjmp(ulong *jmp, int ret);
 
+/**
+ * os_abort() - Raise SIGABRT to exit sandbox (e.g. to debugger)
+ */
+void os_abort(void);
 #endif
-- 
2.19.0.397.gdd90340f6a-goog

[U-Boot] [PATCH v10 05/10] sandbox: Enhance map_to_sysmem() to handle foreign pointers

[Simon Glass]

At present map_sysmem() maps an address into the sandbox RAM buffer,
return a pointer, while map_to_sysmem() goes the other way.

The mapping is currently just 1:1 since a case was not found where a more
flexible mapping was needed. PCI does have a separate and more complex
mapping, but uses its own mechanism.

However this arrange cannot handle one important case, which is where a
test declares a stack variable and passes a pointer to it into a U-Boot
function which uses map_to_sysmem() to turn it into a address. Since the
pointer is not inside emulated DRAM, this will fail.

Add a mapping feature which can handle any such pointer, mapping it to a
simple tag value which can be passed around in U-Boot as an address.

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

Changes in v10: None
Changes in v9:
- Fix 'thi' typo

Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/sandbox/cpu/cpu.c           | 141 +++++++++++++++++++++++++++++--
 arch/sandbox/cpu/state.c         |   8 ++
 arch/sandbox/include/asm/state.h |  21 +++++
 3 files changed, 161 insertions(+), 9 deletions(-)

diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
index cde0b055a67..e523223ebf8 100644
--- a/arch/sandbox/cpu/cpu.c
+++ b/arch/sandbox/cpu/cpu.c
@@ -57,14 +57,104 @@ int cleanup_before_linux_select(int flags)
 	return 0;
 }
 
+/**
+ * is_in_sandbox_mem() - Checks if a pointer is within sandbox's emulated DRAM
+ *
+ * This provides a way to check if a pointer is owned by sandbox (and is within
+ * its RAM) or not. Sometimes pointers come from a test which conceptually runs
+ * output sandbox, potentially with direct access to the C-library malloc()
+ * function, or the sandbox stack (which is not actually within the emulated
+ * DRAM.
+ *
+ * Such pointers obviously cannot be mapped into sandbox's DRAM, so we must
+ * detect them an process them separately, by recording a mapping to a tag,
+ * which we can use to map back to the pointer later.
+ *
+ * @ptr: Pointer to check
+ * @return true if this is within sandbox emulated DRAM, false if not
+ */
+static bool is_in_sandbox_mem(const void *ptr)
+{
+	return (const uint8_t *)ptr >= gd->arch.ram_buf &&
+		(const uint8_t *)ptr < gd->arch.ram_buf + gd->ram_size;
+}
+
+/**
+ * phys_to_virt() - Converts a sandbox RAM address to a pointer
+ *
+ * Sandbox uses U-Boot addresses from 0 to the size of DRAM. These index into
+ * the emulated DRAM buffer used by sandbox. This function converts such an
+ * address to a pointer into this buffer, which can be used to access the
+ * memory.
+ *
+ * If the address is outside this range, it is assumed to be a tag
+ */
 void *phys_to_virt(phys_addr_t paddr)
 {
-	return (void *)(gd->arch.ram_buf + paddr);
+	struct sandbox_mapmem_entry *mentry;
+	struct sandbox_state *state;
+
+	/* If the address is within emulated DRAM, calculate the value */
+	if (paddr < gd->ram_size)
+		return (void *)(gd->arch.ram_buf + paddr);
+
+	/*
+	 * Otherwise search out list of tags for the correct pointer previously
+	 * created by map_to_sysmem()
+	 */
+	state = state_get_current();
+	list_for_each_entry(mentry, &state->mapmem_head, sibling_node) {
+		if (mentry->tag == paddr) {
+			printf("%s: Used map from %lx to %p\n", __func__,
+			       (ulong)paddr, mentry->ptr);
+			return mentry->ptr;
+		}
+	}
+
+	printf("%s: Cannot map sandbox address %lx (SDRAM from 0 to %lx)\n",
+	       __func__, (ulong)paddr, (ulong)gd->ram_size);
+	os_abort();
+
+	/* Not reached */
+	return NULL;
+}
+
+struct sandbox_mapmem_entry *find_tag(const void *ptr)
+{
+	struct sandbox_mapmem_entry *mentry;
+	struct sandbox_state *state = state_get_current();
+
+	list_for_each_entry(mentry, &state->mapmem_head, sibling_node) {
+		if (mentry->ptr == ptr) {
+			debug("%s: Used map from %p to %lx\n", __func__, ptr,
+			      mentry->tag);
+			return mentry;
+		}
+	}
+	return NULL;
 }
 
-phys_addr_t virt_to_phys(void *vaddr)
+phys_addr_t virt_to_phys(void *ptr)
 {
-	return (phys_addr_t)((uint8_t *)vaddr - gd->arch.ram_buf);
+	struct sandbox_mapmem_entry *mentry;
+
+	/*
+	 * If it is in emulated RAM, don't bother looking for a tag. Just
+	 * calculate the pointer using the provides offset into the RAM buffer.
+	 */
+	if (is_in_sandbox_mem(ptr))
+		return (phys_addr_t)((uint8_t *)ptr - gd->arch.ram_buf);
+
+	mentry = find_tag(ptr);
+	if (!mentry) {
+		/* Abort so that gdb can be used here */
+		printf("%s: Cannot map sandbox address %p (SDRAM from 0 to %lx)\n",
+		       __func__, ptr, (ulong)gd->ram_size);
+		os_abort();
+	}
+	printf("%s: Used map from %p to %lx\n", __func__, ptr, mentry->tag);
+
+	return mentry->tag;
 }
 
 void *map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
@@ -87,24 +177,57 @@ void *map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
 	return phys_to_virt(paddr);
 }
 
-void unmap_physmem(const void *vaddr, unsigned long flags)
+void unmap_physmem(const void *ptr, unsigned long flags)
 {
 #ifdef CONFIG_PCI
 	if (map_dev) {
-		pci_unmap_physmem(vaddr, map_len, map_dev);
+		pci_unmap_physmem(ptr, map_len, map_dev);
 		map_dev = NULL;
 	}
 #endif
 }
 
-void sandbox_set_enable_pci_map(int enable)
+phys_addr_t map_to_sysmem(const void *ptr)
 {
-	enable_pci_map = enable;
+	struct sandbox_mapmem_entry *mentry;
+
+	/*
+	 * If it is in emulated RAM, don't bother creating a tag. Just return
+	 * the offset into the RAM buffer.
+	 */
+	if (is_in_sandbox_mem(ptr))
+		return (u8 *)ptr - gd->arch.ram_buf;
+
+	/*
+	 * See if there is an existing tag with this pointer. If not, set up a
+	 * new one.
+	 */
+	mentry = find_tag(ptr);
+	if (!mentry) {
+		struct sandbox_state *state = state_get_current();
+
+		mentry = malloc(sizeof(*mentry));
+		if (!mentry) {
+			printf("%s: Error: Out of memory\n", __func__);
+			os_exit(ENOMEM);
+		}
+		mentry->tag = state->next_tag++;
+		mentry->ptr = (void *)ptr;
+		list_add_tail(&mentry->sibling_node, &state->mapmem_head);
+		debug("%s: Added map from %p to %lx\n", __func__, ptr,
+		      (ulong)mentry->tag);
+	}
+
+	/*
+	 * Return the tag as the address to use. A later call to map_sysmem()
+	 * will return ptr
+	 */
+	return mentry->tag;
 }
 
-phys_addr_t map_to_sysmem(const void *ptr)
+void sandbox_set_enable_pci_map(int enable)
 {
-	return (u8 *)ptr - gd->arch.ram_buf;
+	enable_pci_map = enable;
 }
 
 void flush_dcache_range(unsigned long start, unsigned long stop)
diff --git a/arch/sandbox/cpu/state.c b/arch/sandbox/cpu/state.c
index cc50819ab93..04a11fed559 100644
--- a/arch/sandbox/cpu/state.c
+++ b/arch/sandbox/cpu/state.c
@@ -359,6 +359,14 @@ void state_reset_for_test(struct sandbox_state *state)
 
 	memset(&state->wdt, '\0', sizeof(state->wdt));
 	memset(state->spi, '\0', sizeof(state->spi));
+
+	/*
+	 * Set up the memory tag list. Use the top of emulated SDRAM for the
+	 * first tag number, since that address offset is outside the legal
+	 * range, and can be assumed to be a tag.
+	 */
+	INIT_LIST_HEAD(&state->mapmem_head);
+	state->next_tag = state->ram_size;
 }
 
 int state_init(void)
diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h
index 7ed4b512d2e..a612ce89447 100644
--- a/arch/sandbox/include/asm/state.h
+++ b/arch/sandbox/include/asm/state.h
@@ -9,6 +9,7 @@
 #include <config.h>
 #include <sysreset.h>
 #include <stdbool.h>
+#include <linux/list.h>
 #include <linux/stringify.h>
 
 /**
@@ -45,6 +46,23 @@ struct sandbox_wdt_info {
 	bool running;
 };
 
+/**
+ * struct sandbox_mapmem_entry - maps pointers to/from U-Boot addresses
+ *
+ * When map_to_sysmem() is called with an address outside sandbox's emulated
+ * RAM, a record is created with a tag that can be used to reference that
+ * pointer. When map_sysmem() is called later with that tag, the pointer will
+ * be returned, just as it would for a normal sandbox address.
+ *
+ * @tag: Address tag (a value which U-Boot uses to refer to the address)
+ * @ptr: Associated pointer for that tag
+ */
+struct sandbox_mapmem_entry {
+	ulong tag;
+	void *ptr;
+	struct list_head sibling_node;
+};
+
 /* The complete state of the test system */
 struct sandbox_state {
 	const char *cmd;		/* Command to execute */
@@ -78,6 +96,9 @@ struct sandbox_state {
 
 	/* Information about Watchdog */
 	struct sandbox_wdt_info wdt;
+
+	ulong next_tag;			/* Next address tag to allocate */
+	struct list_head mapmem_head;	/* struct sandbox_mapmem_entry */
 };
 
 /* Minimum space we guarantee in the state FDT when calling read/write*/
-- 
2.19.0.397.gdd90340f6a-goog

[U-Boot] [PATCH v10 06/10] efi: Relocate FDT to 127MB instead of 128MB

[Simon Glass]

Sandbox only has 128MB of memory so we cannot relocate the device tree up
to start at 128MB. Use 127MB instead, which should be safe.

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

Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

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

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index b60c151fb4a..c4797481595 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -158,8 +158,8 @@ static void *copy_fdt(void *fdt)
 	fdt_size = ALIGN(fdt_size + EFI_PAGE_SIZE - 1, EFI_PAGE_SIZE);
 	fdt_pages = fdt_size >> EFI_PAGE_SHIFT;
 
-	/* Safe fdt location is at 128MB */
-	new_fdt_addr = fdt_ram_start + (128 * 1024 * 1024) + fdt_size;
+	/* Safe fdt location is at 127MB */
+	new_fdt_addr = fdt_ram_start + (127 * 1024 * 1024) + fdt_size;
 	if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
 			       EFI_RUNTIME_SERVICES_DATA, fdt_pages,
 			       &new_fdt_addr) != EFI_SUCCESS) {
-- 
2.19.0.397.gdd90340f6a-goog

[U-Boot] [PATCH v10 07/10] efi: sandbox: Tidy up copy_fdt() to work with sandbox

[Simon Glass]

At present this function takes a pointer as its argument, then passes this
to efi_allocate_pages(), which actually takes an address. It uses casts,
which are not supported on sandbox.

Also the function calculates the FDT size rounded up to the neared EFI
page size, then its caller recalculates the size and adds a bit more to
it.

This function is much better written as something that works with
addresses only, and returns both the address and the size of the relocated
FDT.

Also, copy_fdt() returns NULL on error, but really should propagate the
error from efi_allocate_pages(). To do this it needs to return an
efi_status_t, not a void *.

Update the code in this way, so that it is easier to follow, and also
supports sandbox.

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

Changes in v10: None
Changes in v9:
- Drop fdt_end variable in efi_install_fdt()

Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 cmd/bootefi.c | 79 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 50 insertions(+), 29 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index c4797481595..e169f5edc0d 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -131,17 +131,30 @@ static void set_load_options(struct efi_loaded_image *loaded_image_info,
 	loaded_image_info->load_options_size = size * 2;
 }
 
-static void *copy_fdt(void *fdt)
+/**
+ * copy_fdt() - Copy the device tree to a new location available to EFI
+ *
+ * The FDT is relocated into a suitable location within the EFI memory map.
+ * An additional 12KB is added to the space in case the device tree needs to be
+ * expanded later with fdt_open_into().
+ *
+ * @fdt_addr: On entry, address of start of FDT. On exit, address of relocated
+ *	FDT start
+ * @fdt_sizep: Returns new size of FDT, including
+ * @return new relocated address of FDT
+ */
+static efi_status_t copy_fdt(ulong *fdt_addrp, ulong *fdt_sizep)
 {
-	u64 fdt_size = fdt_totalsize(fdt);
 	unsigned long fdt_ram_start = -1L, fdt_pages;
+	efi_status_t ret = 0;
+	void *fdt, *new_fdt;
 	u64 new_fdt_addr;
-	void *new_fdt;
+	uint fdt_size;
 	int i;
 
-        for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
-                u64 ram_start = gd->bd->bi_dram[i].start;
-                u64 ram_size = gd->bd->bi_dram[i].size;
+	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
+		u64 ram_start = gd->bd->bi_dram[i].start;
+		u64 ram_size = gd->bd->bi_dram[i].size;
 
 		if (!ram_size)
 			continue;
@@ -154,30 +167,37 @@ static void *copy_fdt(void *fdt)
 	 * Give us at least 4KB of breathing room in case the device tree needs
 	 * to be expanded later. Round up to the nearest EFI page boundary.
 	 */
-	fdt_size += 4096;
+	fdt = map_sysmem(*fdt_addrp, 0);
+	fdt_size = fdt_totalsize(fdt);
+	fdt_size += 4096 * 3;
 	fdt_size = ALIGN(fdt_size + EFI_PAGE_SIZE - 1, EFI_PAGE_SIZE);
 	fdt_pages = fdt_size >> EFI_PAGE_SHIFT;
 
 	/* Safe fdt location is at 127MB */
 	new_fdt_addr = fdt_ram_start + (127 * 1024 * 1024) + fdt_size;
-	if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
-			       EFI_RUNTIME_SERVICES_DATA, fdt_pages,
-			       &new_fdt_addr) != EFI_SUCCESS) {
+	ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
+				 EFI_RUNTIME_SERVICES_DATA, fdt_pages,
+				 &new_fdt_addr);
+	if (ret != EFI_SUCCESS) {
 		/* If we can't put it there, put it somewhere */
 		new_fdt_addr = (ulong)memalign(EFI_PAGE_SIZE, fdt_size);
-		if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
-				       EFI_RUNTIME_SERVICES_DATA, fdt_pages,
-				       &new_fdt_addr) != EFI_SUCCESS) {
+		ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
+					 EFI_RUNTIME_SERVICES_DATA, fdt_pages,
+					 &new_fdt_addr);
+		if (ret != EFI_SUCCESS) {
 			printf("ERROR: Failed to reserve space for FDT\n");
-			return NULL;
+			goto done;
 		}
 	}
 
-	new_fdt = (void*)(ulong)new_fdt_addr;
+	new_fdt = map_sysmem(new_fdt_addr, fdt_size);
 	memcpy(new_fdt, fdt, fdt_totalsize(fdt));
 	fdt_set_totalsize(new_fdt, fdt_size);
 
-	return new_fdt;
+	*fdt_addrp = new_fdt_addr;
+	*fdt_sizep = fdt_size;
+done:
+	return ret;
 }
 
 static efi_status_t efi_do_enter(
@@ -250,22 +270,27 @@ static void efi_carve_out_dt_rsv(void *fdt)
 	}
 }
 
-static efi_status_t efi_install_fdt(void *fdt)
+static efi_status_t efi_install_fdt(ulong fdt_addr)
 {
 	bootm_headers_t img = { 0 };
-	ulong fdt_pages, fdt_size, fdt_start, fdt_end;
+	ulong fdt_pages, fdt_size, fdt_start;
 	efi_status_t ret;
+	void *fdt;
 
+	fdt = map_sysmem(fdt_addr, 0);
 	if (fdt_check_header(fdt)) {
 		printf("ERROR: invalid device tree\n");
 		return EFI_INVALID_PARAMETER;
 	}
 
 	/* Prepare fdt for payload */
-	fdt = copy_fdt(fdt);
-	if (!fdt)
-		return EFI_OUT_OF_RESOURCES;
+	ret = copy_fdt(&fdt_addr, &fdt_size);
+	if (ret)
+		return ret;
 
+	unmap_sysmem(fdt);
+	fdt = map_sysmem(fdt_addr, 0);
+	fdt_size = fdt_totalsize(fdt);
 	if (image_setup_libfdt(&img, fdt, 0, NULL)) {
 		printf("ERROR: failed to process device tree\n");
 		return EFI_LOAD_ERROR;
@@ -279,14 +304,12 @@ static efi_status_t efi_install_fdt(void *fdt)
 		return EFI_OUT_OF_RESOURCES;
 
 	/* And reserve the space in the memory map */
-	fdt_start = ((ulong)fdt) & ~EFI_PAGE_MASK;
-	fdt_end = ((ulong)fdt) + fdt_totalsize(fdt);
-	fdt_size = (fdt_end - fdt_start) + EFI_PAGE_MASK;
+	fdt_start = fdt_addr;
 	fdt_pages = fdt_size >> EFI_PAGE_SHIFT;
-	/* Give a bootloader the chance to modify the device tree */
-	fdt_pages += 2;
+
 	ret = efi_add_memory_map(fdt_start, fdt_pages,
 				 EFI_BOOT_SERVICES_DATA, true);
+
 	return ret;
 }
 
@@ -443,7 +466,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	char *saddr;
 	efi_status_t r;
 	unsigned long fdt_addr;
-	void *fdt;
 
 	/* Allow unaligned memory access */
 	allow_unaligned();
@@ -464,8 +486,7 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		if (!fdt_addr && *argv[2] != '0')
 			return CMD_RET_USAGE;
 		/* Install device tree */
-		fdt = map_sysmem(fdt_addr, 0);
-		r = efi_install_fdt(fdt);
+		r = efi_install_fdt(fdt_addr);
 		if (r != EFI_SUCCESS) {
 			printf("ERROR: failed to install device tree\n");
 			return CMD_RET_FAILURE;
-- 
2.19.0.397.gdd90340f6a-goog

[U-Boot] [PATCH v10 08/10] efi_loader: Pass address to fs_read()

[Simon Glass]

From: Alexander Graf <agraf@suse.de>

The fs_read() function wants to get an address rather than the
pointer to a buffer.

So let's convert the passed buffer from pointer back a the address
to make efi_loader on sandbox happier.

Signed-off-by: Alexander Graf <agraf@suse.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 lib/efi_loader/efi_file.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
index e6a15bcb523..2107730ba5a 100644
--- a/lib/efi_loader/efi_file.c
+++ b/lib/efi_loader/efi_file.c
@@ -9,6 +9,7 @@
 #include <charset.h>
 #include <efi_loader.h>
 #include <malloc.h>
+#include <mapmem.h>
 #include <fs.h>
 
 /* GUID for file system information */
@@ -232,8 +233,10 @@ static efi_status_t file_read(struct file_handle *fh, u64 *buffer_size,
 		void *buffer)
 {
 	loff_t actread;
+	/* fs_read expects buffer address, not pointer */
+	uintptr_t buffer_addr = (uintptr_t)map_to_sysmem(buffer);
 
-	if (fs_read(fh->path, (ulong)buffer, fh->offset,
+	if (fs_read(fh->path, buffer_addr, fh->offset,
 		    *buffer_size, &actread))
 		return EFI_DEVICE_ERROR;
 
-- 
2.19.0.397.gdd90340f6a-goog

[U-Boot] [PATCH v10 09/10] efi: Correct the operation of efi_file_write()

[Simon Glass]

We should not directly cast between pointers and addresses since it breaks
sandbox. Fix this and simplify the code in file_read().

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

Changes in v10:
- Add new patch to correct the operation of efi_file_write()

Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 lib/efi_loader/efi_file.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
index 2107730ba5a..789d9eff175 100644
--- a/lib/efi_loader/efi_file.c
+++ b/lib/efi_loader/efi_file.c
@@ -233,10 +233,8 @@ static efi_status_t file_read(struct file_handle *fh, u64 *buffer_size,
 		void *buffer)
 {
 	loff_t actread;
-	/* fs_read expects buffer address, not pointer */
-	uintptr_t buffer_addr = (uintptr_t)map_to_sysmem(buffer);
 
-	if (fs_read(fh->path, buffer_addr, fh->offset,
+	if (fs_read(fh->path, map_to_sysmem(buffer), fh->offset,
 		    *buffer_size, &actread))
 		return EFI_DEVICE_ERROR;
 
@@ -366,7 +364,7 @@ static efi_status_t EFIAPI efi_file_write(struct efi_file_handle *file,
 		goto error;
 	}
 
-	if (fs_write(fh->path, (ulong)buffer, fh->offset, *buffer_size,
+	if (fs_write(fh->path, map_to_sysmem(buffer), fh->offset, *buffer_size,
 		     &actwrite)) {
 		ret = EFI_DEVICE_ERROR;
 		goto error;
-- 
2.19.0.397.gdd90340f6a-goog

[U-Boot] [PATCH v10 10/10] efi: sandbox: Enable selftest command

[Simon Glass]

Enable this for sandbox since it almost passes now.

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

Changes in v10:
- Drop patches not essential to getting things working

Changes in v9: None
Changes in v8:
- Rebase to master, bringing in all EFI changes
- Drop 'efi: sandbox: Add relocation constants'
- Drop 'efi: Adjust memory handling to support sandbox'
- Drop 'sandbox: smbios: Update to support sandbox'
- Expand series substantially to support bootefi selftest
- Rebase to master

Changes in v7: None
Changes in v6: None
Changes in v4:
- Rebase to master
- Update SPDX tags

Changes in v3:
- Rebase to master

Changes in v2:
- Rebase to master

 configs/sandbox_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index ca121182d65..0b209686bf9 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -23,6 +23,7 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
 CONFIG_CMD_BOOTZ=y
+CONFIG_CMD_BOOTEFI_SELFTEST=y
 # CONFIG_CMD_ELF is not set
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
-- 
2.19.0.397.gdd90340f6a-goog

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

[Simon Glass]

Hi Alex,

On 15 September 2018 at 08:50, Simon Glass <sjg@chromium.org> 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.
>
> Also included in v8 is support for running the full EFI self tests. These
> run OK with some tweaks to a few parts of the code.
>
> With v9, various EFI patches have been applied which change things. This
> series includes a partial review of one, which makes 'bootefi test' work.
> But there are still problems with 'bootefi selftest':
>
> As of v10 two selftests fail. I fixed another one in this series. I think
> have removed all paches not essential to getting things running. The
> refactoring patches will be sent separately once these are sorted out.

The only master branch I could find in your tree was a few years old
so I could not rebase on efi. Let me know if you update it. For now
you should be able to skip any patches you have already applied.

Regards,
Simon

[U-Boot] [U-Boot, v10, 07/10] efi: sandbox: Tidy up copy_fdt() to work with sandbox

[Alexander Graf]

> At present this function takes a pointer as its argument, then passes this
> to efi_allocate_pages(), which actually takes an address. It uses casts,
> which are not supported on sandbox.
> 
> Also the function calculates the FDT size rounded up to the neared EFI
> page size, then its caller recalculates the size and adds a bit more to
> it.
> 
> This function is much better written as something that works with
> addresses only, and returns both the address and the size of the relocated
> FDT.
> 
> Also, copy_fdt() returns NULL on error, but really should propagate the
> error from efi_allocate_pages(). To do this it needs to return an
> efi_status_t, not a void *.
> 
> Update the code in this way, so that it is easier to follow, and also
> supports sandbox.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Thanks, applied to efi-next

Alex

[U-Boot] [U-Boot, v10, 04/10] sandbox: Add support for calling abort()

[Alexander Graf]

> This function is useful to signal that the application needs to exit
> immediate. It can be caught with a debugger (e.g. gdb). Add a stub for it
> so that it can be called from within sandbox when an internal error
> occurs.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Thanks, applied to efi-next

Alex

[U-Boot] [U-Boot, v10, 06/10] efi: Relocate FDT to 127MB instead of 128MB

[Alexander Graf]

> Sandbox only has 128MB of memory so we cannot relocate the device tree up
> to start at 128MB. Use 127MB instead, which should be safe.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Thanks, applied to efi-next

Alex

[U-Boot] [U-Boot, v10, 09/10] efi: Correct the operation of efi_file_write()

[Alexander Graf]

> We should not directly cast between pointers and addresses since it breaks
> sandbox. Fix this and simplify the code in file_read().
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Thanks, applied to efi-next

Alex

[U-Boot] [U-Boot, v10, 01/10] efi: sandbox: Add distroboot support

[Alexander Graf]

> 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>

Thanks, applied to efi-next

Alex

[U-Boot] [U-Boot, v10, 05/10] sandbox: Enhance map_to_sysmem() to handle foreign pointers

[Alexander Graf]

> At present map_sysmem() maps an address into the sandbox RAM buffer,
> return a pointer, while map_to_sysmem() goes the other way.
> 
> The mapping is currently just 1:1 since a case was not found where a more
> flexible mapping was needed. PCI does have a separate and more complex
> mapping, but uses its own mechanism.
> 
> However this arrange cannot handle one important case, which is where a
> test declares a stack variable and passes a pointer to it into a U-Boot
> function which uses map_to_sysmem() to turn it into a address. Since the
> pointer is not inside emulated DRAM, this will fail.
> 
> Add a mapping feature which can handle any such pointer, mapping it to a
> simple tag value which can be passed around in U-Boot as an address.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Thanks, applied to efi-next

Alex

[U-Boot] [U-Boot, v10, 10/10] efi: sandbox: Enable selftest command

[Alexander Graf]

> Enable this for sandbox since it almost passes now.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Thanks, applied to efi-next

Alex

[U-Boot] [U-Boot, v10, 02/10] efi: sandbox: Enable EFI loader build for sandbox

[Alexander Graf]

> This allows this feature to build within sandbox. This is useful for
> testing.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Thanks, applied to efi-next

Alex

[U-Boot] [U-Boot, v10, 03/10] sandbox: Align RAM buffer to the machine page size

[Alexander Graf]

> At present the sandbox RAM buffer is not aligned to any particular
> address boundary. This makes the internal pointers somewhat random with
> respect to the associated RAM buffer addresses.
> 
> Align the buffer to the page size of the machine to help with this. Note
> that there is a header at the start of the allocated pointer. To avoid
> returning a pointer which is not aligned to a page boundary, we waste
> almost an entire page of memory for each allocation.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Thanks, applied to efi-next

Alex