[U-Boot] [PATCH v5 00/10] sandbox: efi_loader support

[U-Boot] [PATCH v5 00/10] sandbox: efi_loader support

[Alexander Graf]

This patch set finalized the last remaining bits to get efi_loader
working properly in sandbox. It depends on the current efi-next
tree to function properly.

With this patch set in place, I can successfully run the selftest suite
as well as an aarch64 grub.efi and an x86_64 grub.efi binary that has
been patched to not directly access port io.

I hope this patch set concludes the sandbox efforts and we can finally
move on doing productive work.

Alex

... -> v4:

  - changelog dropped, as the scope of the patch set slimmed down

v4 -> v5:

  - drop patches that are applied to efi-next
  - Replace runtime mprotect() mechanism with mmap() flag
  - Use system setjmp/longjmp directly from target code
  - Add Simon's smbios patch and adapt to efi_allocate_pages()
  - Include Simon's patches for fs_read(), map_to_sysmem(), RAM alignment


Alexander Graf (5):
  efi.h: Do not use config options
  distro: Move to compiler based target architecture determination
  efi_loader: Pass address to fs_read()
  sandbox: Fix setjmp/longjmp
  sandbox: Allow to execute from RAM

Simon Glass (5):
  sandbox: Align RAM buffer to the machine page size
  sandbox: smbios: Update to support sandbox
  sandbox: Add support for calling abort()
  sandbox: Enhance map_to_sysmem() to handle foreign pointers
  efi: sandbox: Enable EFI loader for sandbox

 arch/sandbox/cpu/cpu.c            | 153 ++++++++++++++++++++++++++++++++------
 arch/sandbox/cpu/os.c             |  27 ++-----
 arch/sandbox/cpu/state.c          |   8 ++
 arch/sandbox/include/asm/setjmp.h |   5 ++
 arch/sandbox/include/asm/state.h  |  21 ++++++
 include/config_distro_bootcmd.h   |  17 +++--
 include/efi.h                     |  24 +++---
 include/os.h                      |  20 +----
 lib/efi/Makefile                  |   4 +-
 lib/efi_loader/Kconfig            |   2 +-
 lib/efi_loader/efi_file.c         |   5 +-
 lib/efi_loader/efi_smbios.c       |  18 ++++-
 lib/smbios.c                      |  32 ++++++--
 13 files changed, 241 insertions(+), 95 deletions(-)

-- 
2.12.3

[U-Boot] [PATCH v5 01/10] efi.h: Do not use config options

[Alexander Graf]

Currently efi.h determines a few bits of its environment according to
config options. This falls apart with the efi stub support which may
result in efi.h getting pulled into the stub as well as real U-Boot
code. In that case, one may be 32bit while the other one is 64bit.

This patch changes the conditionals to use compiler provided defines
instead. That way we always adhere to the build environment we're in
and the definitions adjust automatically.

Signed-off-by: Alexander Graf <agraf@suse.de>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>
[bmeng: added some comments to describe the __x86_64__ check]
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 include/efi.h    | 24 +++++++++++-------------
 lib/efi/Makefile |  4 ++--
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/include/efi.h b/include/efi.h
index d4da65ce53..41530a7537 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -19,8 +19,15 @@
 #include <linux/string.h>
 #include <linux/types.h>
 
-#if CONFIG_EFI_STUB_64BIT || (!defined(CONFIG_EFI_STUB) && defined(__x86_64__))
-/* EFI uses the Microsoft ABI which is not the default for GCC */
+/*
+ * EFI on x86_64 uses the Microsoft ABI which is not the default for GCC.
+ *
+ * There are two scenarios for EFI on x86_64: building a 64-bit EFI stub
+ * codes (CONFIG_EFI_STUB_64BIT) and building a 64-bit U-Boot (CONFIG_X86_64).
+ * Either needs to be properly built with the '-m64' compiler flag, and hence
+ * it is enough to only check the compiler provided define __x86_64__ here.
+ */
+#ifdef __x86_64__
 #define EFIAPI __attribute__((ms_abi))
 #define efi_va_list __builtin_ms_va_list
 #define efi_va_start __builtin_ms_va_start
@@ -32,7 +39,7 @@
 #define efi_va_start va_start
 #define efi_va_arg va_arg
 #define efi_va_end va_end
-#endif
+#endif /* __x86_64__ */
 
 struct efi_device_path;
 
@@ -40,16 +47,7 @@ typedef struct {
 	u8 b[16];
 } efi_guid_t;
 
-#define EFI_BITS_PER_LONG	BITS_PER_LONG
-
-/*
- * With 64-bit EFI stub, EFI_BITS_PER_LONG has to be 64. EFI_STUB is set
- * in lib/efi/Makefile, when building the stub.
- */
-#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB)
-#undef EFI_BITS_PER_LONG
-#define EFI_BITS_PER_LONG	64
-#endif
+#define EFI_BITS_PER_LONG	(sizeof(long) * 8)
 
 /* Bit mask for EFI status code with error */
 #define EFI_ERROR_MASK (1UL << (EFI_BITS_PER_LONG - 1))
diff --git a/lib/efi/Makefile b/lib/efi/Makefile
index f1a3929e32..a790d2d554 100644
--- a/lib/efi/Makefile
+++ b/lib/efi/Makefile
@@ -7,11 +7,11 @@ obj-$(CONFIG_EFI_STUB) += efi_info.o
 
 CFLAGS_REMOVE_efi_stub.o := -mregparm=3 \
 	$(if $(CONFIG_EFI_STUB_64BIT),-march=i386 -m32)
-CFLAGS_efi_stub.o := -fpic -fshort-wchar -DEFI_STUB \
+CFLAGS_efi_stub.o := -fpic -fshort-wchar \
 	$(if $(CONFIG_EFI_STUB_64BIT),-m64)
 CFLAGS_REMOVE_efi.o := -mregparm=3 \
 	$(if $(CONFIG_EFI_STUB_64BIT),-march=i386 -m32)
-CFLAGS_efi.o := -fpic -fshort-wchar -DEFI_STUB \
+CFLAGS_efi.o := -fpic -fshort-wchar \
 	$(if $(CONFIG_EFI_STUB_64BIT),-m64)
 
 extra-$(CONFIG_EFI_STUB) += efi_stub.o efi.o
-- 
2.12.3

[U-Boot] [PATCH v5 02/10] distro: Move to compiler based target architecture determination

[Alexander Graf]

Thanks to CONFIG_SANDBOX, we can not rely on config options to tell us
what CPU architecture we're running on.

The compiler however does know that, so let's just move the ifdefs over
to compiler based defines rather than kconfig based options.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v3 -> v4:

  - Compile fix for dts
---
 include/config_distro_bootcmd.h | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
index d672e8ebe6..c35a42f6c5 100644
--- a/include/config_distro_bootcmd.h
+++ b/include/config_distro_bootcmd.h
@@ -245,22 +245,27 @@
 #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(__aarch64__)
 #define BOOTENV_EFI_PXE_ARCH "0xb"
 #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00011:UNDI:003000"
-#elif defined(CONFIG_ARM)
+#elif 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(__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 "0x0"
+#define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00000:UNDI:003000"
+#elif 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(__riscv) && (__riscv_xlen == 64)
 #define BOOTENV_EFI_PXE_ARCH "0x1b"
 #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00027:UNDI:003000"
+#elif defined(__ASSEMBLY__)
+#define BOOTENV_EFI_PXE_ARCH "0xff"
+#define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00255:UNDI:003000"
 #else
 #error Please specify an EFI client identifier
 #endif
-- 
2.12.3

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

[Alexander Graf]

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>
---
 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 e6a15bcb52..2107730ba5 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.12.3

[U-Boot] [PATCH v5 04/10] sandbox: Fix setjmp/longjmp

[Alexander Graf]

In sandbox, longjmp returns to itself in an endless loop because
os_longjmp() calls into longjmp() which is provided by U-Boot which
again calls os_longjmp().

Setjmp on the other hand must not return because otherwise the
return freees up stack elements that we need during longjmp().

The only straight forward fix that doesn't involve nasty hacks I
could find is to directly link against the system setjmp/longjmp
implementations. That means we just provide the compiler with
hints that the symbol will be available and actually fill them
out with versions from libc.

This approach should be reasonably platform agnostic

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v4 -> v5:

  - Use system setjmp/longjmp directly from target code
---
 arch/sandbox/cpu/cpu.c            | 12 ------------
 arch/sandbox/cpu/os.c             | 22 ----------------------
 arch/sandbox/include/asm/setjmp.h |  5 +++++
 include/os.h                      | 21 ---------------------
 4 files changed, 5 insertions(+), 55 deletions(-)

diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
index cde0b055a6..e7d3c272a4 100644
--- a/arch/sandbox/cpu/cpu.c
+++ b/arch/sandbox/cpu/cpu.c
@@ -165,15 +165,3 @@ 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 5839932b00..fc2f9dbc7a 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -629,25 +629,3 @@ 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)
-{
-	jmp_buf dummy;
-
-	/*
-	 * We cannot rely on the struct name that jmp_buf uses, so use a
-	 * local variable here
-	 */
-	if (size < sizeof(dummy)) {
-		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
index 1fe37c91cc..001c7ea322 100644
--- a/arch/sandbox/include/asm/setjmp.h
+++ b/arch/sandbox/include/asm/setjmp.h
@@ -24,6 +24,11 @@ struct jmp_buf_data {
 
 typedef struct jmp_buf_data jmp_buf[1];
 
+/*
+ * We have to directly link with the system versions of
+ * setjmp/longjmp, because setjmp must not return as otherwise
+ * the stack may become invalid.
+ */
 int setjmp(jmp_buf jmp);
 __noreturn void longjmp(jmp_buf jmp, int ret);
 
diff --git a/include/os.h b/include/os.h
index c8e0f52d30..64e89a06c9 100644
--- a/include/os.h
+++ b/include/os.h
@@ -330,25 +330,4 @@ 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.12.3

[U-Boot] [PATCH v5 05/10] sandbox: Allow to execute from RAM

[Alexander Graf]

With efi_loader, we may want to execute payloads from RAM. By default,
permissions on the RAM region don't allow us to execute from there though.

So let's change the default allocation scheme for RAM to also allow
execution from it. That way payloads that live in U-Boot RAM can be
directly executed.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v4 -> v5:

  - Replace runtime mprotect() mechanism with mmap() flag
---
 arch/sandbox/cpu/os.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index fc2f9dbc7a..e3fe017b5f 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -144,7 +144,8 @@ void *os_malloc(size_t length)
 {
 	struct os_mem_hdr *hdr;
 
-	hdr = mmap(NULL, length + sizeof(*hdr), PROT_READ | PROT_WRITE,
+	hdr = mmap(NULL, length + sizeof(*hdr),
+		   PROT_READ | PROT_WRITE | PROT_EXEC,
 		   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 	if (hdr == MAP_FAILED)
 		return NULL;
-- 
2.12.3

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

[Alexander Graf]

From: Simon Glass <sjg@chromium.org>

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.

Signed-off-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/sandbox/cpu/os.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index e3fe017b5f..3067503d51 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -143,15 +143,16 @@ 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),
+	hdr = mmap(NULL, length + page_size,
 		   PROT_READ | PROT_WRITE | PROT_EXEC,
 		   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.12.3

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

[Alexander Graf]

From: Simon Glass <sjg@chromium.org>

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>
[agraf: Adapt efi smbios api usage]
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 lib/efi_loader/efi_smbios.c | 18 ++++++++++++++----
 lib/smbios.c                | 32 ++++++++++++++++++++++++--------
 2 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
index 932f7582ec..469068d34d 100644
--- a/lib/efi_loader/efi_smbios.c
+++ b/lib/efi_loader/efi_smbios.c
@@ -8,6 +8,7 @@
 #include <common.h>
 #include <efi_loader.h>
 #include <inttypes.h>
+#include <mapmem.h>
 #include <smbios.h>
 
 static const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;
@@ -21,6 +22,8 @@ efi_status_t efi_smbios_register(void)
 {
 	/* Map within the low 32 bits, to allow for 32bit SMBIOS tables */
 	u64 dmi = U32_MAX;
+	u64 dmi_addr;
+	void *dmi_ptr;
 	efi_status_t ret;
 
 	/* Reserve 4kiB page for SMBIOS */
@@ -37,14 +40,21 @@ efi_status_t efi_smbios_register(void)
 	}
 
 	/*
+	 * efi_allocate_pages() returns the allocated pointer in dmi,
+	 * which we need to convert back into a U-Boot address for
+	 * write_smbios_table()
+	 */
+	dmi_ptr = (void *)(uintptr_t)dmi;
+	dmi_addr = map_to_sysmem(dmi_ptr);
+
+	/*
 	 * Generate SMBIOS tables - we know that efi_allocate_pages() returns
 	 * a 4k-aligned address, so it is safe to assume that
 	 * write_smbios_table() will write the table at that address.
 	 */
-	assert(!(dmi & 0xf));
-	write_smbios_table(dmi);
+	assert(!(dmi_addr & 0xf));
+	write_smbios_table(dmi_addr);
 
 	/* And expose them to our EFI payload */
-	return efi_install_configuration_table(&smbios_guid,
-					       (void *)(uintptr_t)dmi);
+	return efi_install_configuration_table(&smbios_guid, dmi_ptr);
 }
diff --git a/lib/smbios.c b/lib/smbios.c
index df3d26b071..fc3dabcbc1 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -6,6 +6,7 @@
  */
 
 #include <common.h>
+#include <mapmem.h>
 #include <smbios.h>
 #include <tables_csum.h>
 #include <version.h>
@@ -72,9 +73,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");
@@ -101,16 +103,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);
@@ -122,15 +126,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);
@@ -140,15 +146,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);
@@ -160,6 +168,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;
 }
@@ -198,9 +207,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;
@@ -214,32 +224,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;
 }
@@ -268,7 +283,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);
@@ -297,6 +312,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.12.3

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

[Alexander Graf]

From: Simon Glass <sjg@chromium.org>

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>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/sandbox/cpu/os.c | 5 +++++
 include/os.h          | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index 3067503d51..9fbcb9ef92 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -631,3 +631,8 @@ void os_localtime(struct rtc_time *rt)
 	rt->tm_yday = tm->tm_yday;
 	rt->tm_isdst = tm->tm_isdst;
 }
+
+void os_abort(void)
+{
+	abort();
+}
diff --git a/include/os.h b/include/os.h
index 64e89a06c9..8fb9d309d0 100644
--- a/include/os.h
+++ b/include/os.h
@@ -330,4 +330,9 @@ int os_spl_to_uboot(const char *fname);
  */
 void os_localtime(struct rtc_time *rt);
 
+/**
+ * os_abort() - Raise SIGABRT to exit sandbox (e.g. to debugger)
+ */
+void os_abort(void);
+
 #endif
-- 
2.12.3

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

[Alexander Graf]

From: Simon Glass <sjg@chromium.org>

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>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 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 e7d3c272a4..456cfa3faa 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 thi sbuffer, 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 cc50819ab9..04a11fed55 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 7ed4b512d2..a612ce8944 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.12.3

[U-Boot] [PATCH v5 10/10] efi: sandbox: Enable EFI loader for sandbox

[Alexander Graf]

From: Simon Glass <sjg@chromium.org>

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>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 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 ce6a09f0b4..bfd7b19d79 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.12.3

[U-Boot] [PATCH v5 05/10] sandbox: Allow to execute from RAM

[Simon Glass]

On 22 June 2018 at 06:44, Alexander Graf <agraf@suse.de> wrote:
> With efi_loader, we may want to execute payloads from RAM. By default,
> permissions on the RAM region don't allow us to execute from there though.
>
> So let's change the default allocation scheme for RAM to also allow
> execution from it. That way payloads that live in U-Boot RAM can be
> directly executed.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
>
> ---
>
> v4 -> v5:
>
>   - Replace runtime mprotect() mechanism with mmap() flag
> ---
>  arch/sandbox/cpu/os.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

That looks a lot better to me.

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

[U-Boot] [PATCH v5 04/10] sandbox: Fix setjmp/longjmp

[Simon Glass]

Hi Alex,

On 22 June 2018 at 06:44, Alexander Graf <agraf@suse.de> wrote:
> In sandbox, longjmp returns to itself in an endless loop because
> os_longjmp() calls into longjmp() which is provided by U-Boot which
> again calls os_longjmp().
>
> Setjmp on the other hand must not return because otherwise the
> return freees up stack elements that we need during longjmp().
>
> The only straight forward fix that doesn't involve nasty hacks I
> could find is to directly link against the system setjmp/longjmp
> implementations. That means we just provide the compiler with
> hints that the symbol will be available and actually fill them
> out with versions from libc.
>
> This approach should be reasonably platform agnostic
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
>
> ---
>
> v4 -> v5:
>
>   - Use system setjmp/longjmp directly from target code
> ---
>  arch/sandbox/cpu/cpu.c            | 12 ------------
>  arch/sandbox/cpu/os.c             | 22 ----------------------
>  arch/sandbox/include/asm/setjmp.h |  5 +++++
>  include/os.h                      | 21 ---------------------
>  4 files changed, 5 insertions(+), 55 deletions(-)

I was wondering if that would work. It seems much better to me.

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

Regards,
Simon

[U-Boot] [PATCH v5 04/10] sandbox: Fix setjmp/longjmp

[Alexander Graf]

On 22.06.18 21:28, Simon Glass wrote:
> Hi Alex,
> 
> On 22 June 2018 at 06:44, Alexander Graf <agraf@suse.de> wrote:
>> In sandbox, longjmp returns to itself in an endless loop because
>> os_longjmp() calls into longjmp() which is provided by U-Boot which
>> again calls os_longjmp().
>>
>> Setjmp on the other hand must not return because otherwise the
>> return freees up stack elements that we need during longjmp().
>>
>> The only straight forward fix that doesn't involve nasty hacks I
>> could find is to directly link against the system setjmp/longjmp
>> implementations. That means we just provide the compiler with
>> hints that the symbol will be available and actually fill them
>> out with versions from libc.
>>
>> This approach should be reasonably platform agnostic
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>
>> ---
>>
>> v4 -> v5:
>>
>>   - Use system setjmp/longjmp directly from target code
>> ---
>>  arch/sandbox/cpu/cpu.c            | 12 ------------
>>  arch/sandbox/cpu/os.c             | 22 ----------------------
>>  arch/sandbox/include/asm/setjmp.h |  5 +++++
>>  include/os.h                      | 21 ---------------------
>>  4 files changed, 5 insertions(+), 55 deletions(-)
> 
> I was wondering if that would work. It seems much better to me.
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>

Awesome ;).

So the way forward that I would envision is that once the merge window
is open, I'll send out a pull request for efi-next to Tom. Pretty much
all the foundational work should be in upstream by then. The only
remaining parts are x86, distro and sandbox specific.

For these (basically all the patches of this patch set) I think it makes
sense if you send them via your tree then.

Beware that currently travis complains about sandbox failing with this
patch set applied. I assume it's your mao_to_sysmem() patch though:

  https://travis-ci.org/agraf/u-boot/jobs/395446382


Alex

[U-Boot] [PATCH v5 04/10] sandbox: Fix setjmp/longjmp

[Simon Glass]

Hi Alex,

On 23 June 2018 at 01:36, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 22.06.18 21:28, Simon Glass wrote:
>> Hi Alex,
>>
>> On 22 June 2018 at 06:44, Alexander Graf <agraf@suse.de> wrote:
>>> In sandbox, longjmp returns to itself in an endless loop because
>>> os_longjmp() calls into longjmp() which is provided by U-Boot which
>>> again calls os_longjmp().
>>>
>>> Setjmp on the other hand must not return because otherwise the
>>> return freees up stack elements that we need during longjmp().
>>>
>>> The only straight forward fix that doesn't involve nasty hacks I
>>> could find is to directly link against the system setjmp/longjmp
>>> implementations. That means we just provide the compiler with
>>> hints that the symbol will be available and actually fill them
>>> out with versions from libc.
>>>
>>> This approach should be reasonably platform agnostic
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>
>>> ---
>>>
>>> v4 -> v5:
>>>
>>>   - Use system setjmp/longjmp directly from target code
>>> ---
>>>  arch/sandbox/cpu/cpu.c            | 12 ------------
>>>  arch/sandbox/cpu/os.c             | 22 ----------------------
>>>  arch/sandbox/include/asm/setjmp.h |  5 +++++
>>>  include/os.h                      | 21 ---------------------
>>>  4 files changed, 5 insertions(+), 55 deletions(-)
>>
>> I was wondering if that would work. It seems much better to me.
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Awesome ;).
>
> So the way forward that I would envision is that once the merge window
> is open, I'll send out a pull request for efi-next to Tom. Pretty much
> all the foundational work should be in upstream by then. The only
> remaining parts are x86, distro and sandbox specific.
>
> For these (basically all the patches of this patch set) I think it makes
> sense if you send them via your tree then.
>
> Beware that currently travis complains about sandbox failing with this
> patch set applied. I assume it's your mao_to_sysmem() patch though:
>
>   https://travis-ci.org/agraf/u-boot/jobs/395446382

Yes that looks like a bug although I don't see it with my series.

Regards,
Simon

[U-Boot] [U-Boot,v5,04/10] sandbox: Fix setjmp/longjmp

[Alexander Graf]

> In sandbox, longjmp returns to itself in an endless loop because
> os_longjmp() calls into longjmp() which is provided by U-Boot which
> again calls os_longjmp().
> 
> Setjmp on the other hand must not return because otherwise the
> return freees up stack elements that we need during longjmp().
> 
> The only straight forward fix that doesn't involve nasty hacks I
> could find is to directly link against the system setjmp/longjmp
> implementations. That means we just provide the compiler with
> hints that the symbol will be available and actually fill them
> out with versions from libc.
> 
> This approach should be reasonably platform agnostic
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Thanks, applied to efi-next

Alex

[U-Boot] [U-Boot,v5,05/10] sandbox: Allow to execute from RAM

[Alexander Graf]

> With efi_loader, we may want to execute payloads from RAM. By default,
> permissions on the RAM region don't allow us to execute from there though.
> 
> So let's change the default allocation scheme for RAM to also allow
> execution from it. That way payloads that live in U-Boot RAM can be
> directly executed.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Thanks, applied to efi-next

Alex