From: Simon Glass sjg@chromium.org

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 Signed-off-by: Alexander Graf agraf@suse.de --- include/config_distro_bootcmd.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)

diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index d672e8ebe6..1bd79ae3b8 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -251,6 +251,8 @@ #elif defined(CONFIG_ARM) #define BOOTENV_EFI_PXE_ARCH "0xa" #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00010:UNDI:003000" + +/* For sandbox we only support 64-bit x86 at present */ #elif defined(CONFIG_X86) /* Always assume we're running 64bit */ #define BOOTENV_EFI_PXE_ARCH "0x7" @@ -261,6 +263,17 @@ #elif defined(CONFIG_CPU_RISCV_64) #define BOOTENV_EFI_PXE_ARCH "0x1b" #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00027:UNDI:003000" +#elif defined(CONFIG_SANDBOX) +/* + * TODO(sjg@chromium.org): Consider providing a way to enable sandbox features + * based on the host architecture + */ +# ifndef __x86_64__ +# warning "sandbox EFI support is only tested on 64-bit x86" +# endif +/* To support other *host* architectures this should be changed */ +#define BOOTENV_EFI_PXE_ARCH "0x7" +#define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00007:UNDI:003000" #else #error Please specify an EFI client identifier #endif

The EFI image loader tries to determine which target architecture we're working with to only load PE binaries that match.

So far this has worked based on CONFIG defines, because the target CPU was always indicated by a config define. With sandbox however, this is not longer true as all sandbox targets only encompass a single CONFIG option and so we need to use compiler defines to determine the CPU architecture.

Signed-off-by: Alexander Graf agraf@suse.de --- lib/efi_loader/efi_image_loader.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index ecdb77e5b6..fdf40a62c8 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -19,25 +19,25 @@ const efi_guid_t efi_simple_file_system_protocol_guid = const efi_guid_t efi_file_info_guid = EFI_FILE_INFO_GUID;

static int machines[] = { -#if defined(CONFIG_ARM64) +#if defined(__aarch64__) IMAGE_FILE_MACHINE_ARM64, -#elif defined(CONFIG_ARM) +#elif defined(__arm__) IMAGE_FILE_MACHINE_ARM, IMAGE_FILE_MACHINE_THUMB, IMAGE_FILE_MACHINE_ARMNT, #endif

-#if defined(CONFIG_X86_64) +#if defined(__x86_64__) IMAGE_FILE_MACHINE_AMD64, -#elif defined(CONFIG_X86) +#elif defined(__i386__) IMAGE_FILE_MACHINE_I386, #endif

-#if defined(CONFIG_CPU_RISCV_32) +#if defined(__riscv) && (__riscv_xlen == 32) IMAGE_FILE_MACHINE_RISCV32, #endif

-#if defined(CONFIG_CPU_RISCV_64) +#if defined(__riscv) && (__riscv_xlen == 64) IMAGE_FILE_MACHINE_RISCV64, #endif 0 };

The bootefi command gets a few addresses as values passed in. In sandbox, these values are in U-Boot address space, so we need to make sure we explicitly call map_sysmem() on them to be able to access them.

Signed-off-by: Alexander Graf agraf@suse.de Reviewed-by: Simon Glass sjg@chromium.org --- cmd/bootefi.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c index f55a40dc84..a86a2bd4a9 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -14,6 +14,7 @@ #include <errno.h> #include <linux/libfdt.h> #include <linux/libfdt_env.h> +#include <mapmem.h> #include <memalign.h> #include <asm/global_data.h> #include <asm-generic/sections.h> @@ -389,7 +390,8 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) unsigned long addr; char *saddr; efi_status_t r; - void *fdt_addr; + unsigned long fdt_addr; + void *fdt;

/* Allow unaligned memory access */ allow_unaligned(); @@ -406,11 +408,12 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return CMD_RET_USAGE;

if (argc > 2) { - fdt_addr = (void *)simple_strtoul(argv[2], NULL, 16); + fdt_addr = simple_strtoul(argv[2], NULL, 16); if (!fdt_addr && *argv[2] != '0') return CMD_RET_USAGE; /* Install device tree */ - r = efi_install_fdt(fdt_addr); + fdt = map_sysmem(fdt_addr, 0); + r = efi_install_fdt(fdt); if (r != EFI_SUCCESS) { printf("ERROR: failed to install device tree\n"); return CMD_RET_FAILURE; @@ -429,7 +432,7 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) addr = simple_strtoul(saddr, NULL, 16); else addr = CONFIG_SYS_LOAD_ADDR; - memcpy((char *)addr, __efi_helloworld_begin, size); + memcpy(map_sysmem(addr, size), __efi_helloworld_begin, size); } else #endif #ifdef CONFIG_CMD_BOOTEFI_SELFTEST @@ -475,7 +478,7 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) }

printf("## Starting EFI application at %08lx ...\n", addr); - r = do_bootefi_exec((void *)addr, bootefi_device_path, + r = do_bootefi_exec(map_sysmem(addr, 0), bootefi_device_path, bootefi_image_path); printf("## Application terminated, r = %lu\n", r & ~EFI_ERROR_MASK);

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 Signed-off-by: Bin Meng bmeng.cn@gmail.com --- include/efi.h | 17 ++++------------- lib/efi/Makefile | 4 ++-- 2 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/include/efi.h b/include/efi.h index e30a3c51c6..826d484977 100644 --- a/include/efi.h +++ b/include/efi.h @@ -19,12 +19,12 @@ #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 */ +#ifdef __x86_64__ #define EFIAPI __attribute__((ms_abi)) #else #define EFIAPI asmlinkage -#endif +#endif /* __x86_64__ */

struct efi_device_path;

@@ -32,16 +32,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 18d081ac46..ece7907227 100644 --- a/lib/efi/Makefile +++ b/lib/efi/Makefile @@ -7,9 +7,9 @@ obj-$(CONFIG_EFI_STUB) += efi_info.o

CFLAGS_REMOVE_efi_stub.o := -mregparm=3 \ $(if $(CONFIG_EFI_STUB_64BIT),-march=i386 -m32) -CFLAGS_efi_stub.o := -fpic -fshort-wchar -DEFI_STUB +CFLAGS_efi_stub.o := -fpic -fshort-wchar 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

extra-$(CONFIG_EFI_STUB) += efi_stub.o efi.o

With efi_loader we do not control payload applications, so we can not teach them about the difference between virtual and physical addresses.

Instead, let's just always map host virtual addresses in the efi memory map. That way we can be sure that all memory allocation functions always return consumable pointers.

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

---

v1 -> v2:

- only compile efi_add_known_memory if efi_loader is enabled

v3 -> v4:

- don't compile efi mapping code in for spl --- arch/sandbox/cpu/cpu.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)

diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c index cde0b055a6..29dac8dfda 100644 --- a/arch/sandbox/cpu/cpu.c +++ b/arch/sandbox/cpu/cpu.c @@ -5,6 +5,7 @@ #define DEBUG #include <common.h> #include <dm.h> +#include <efi_loader.h> #include <errno.h> #include <linux/libfdt.h> #include <os.h> @@ -177,3 +178,22 @@ void longjmp(jmp_buf jmp, int ret) while (1) ; } + +#if CONFIG_IS_ENABLED(EFI_LOADER) + +/* + * In sandbox, we don't have a 1:1 map, so we need to expose + * process addresses instead of U-Boot addresses + */ +void efi_add_known_memory(void) +{ + u64 ram_start = (uintptr_t)map_sysmem(0, gd->ram_size); + u64 ram_size = gd->ram_size; + u64 start = (ram_start + EFI_PAGE_MASK) & ~EFI_PAGE_MASK; + u64 pages = (ram_size + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT; + + efi_add_memory_map(start, pages, EFI_CONVENTIONAL_MEMORY, + false); +} + +#endif

From: Heinrich Schuchardt xypron.glpk@gmx.de

When running on the sandbox the stack is not necessarily at a higher memory address than the highest free memory.

There is no reason why the checking of the highest memory address should be more restrictive for EFI_ALLOCATE_ANY_PAGES than for EFI_ALLOCATE_MAX_ADDRESS.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de [agraf: use -1ULL instead] Signed-off-by: Alexander Graf agraf@suse.de --- lib/efi_loader/efi_memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index ec66af98ea..ce29bcc6a3 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -295,7 +295,7 @@ efi_status_t efi_allocate_pages(int type, int memory_type, switch (type) { case EFI_ALLOCATE_ANY_PAGES: /* Any page */ - addr = efi_find_free_memory(len, gd->start_addr_sp); + addr = efi_find_free_memory(len, -1ULL); if (!addr) { r = EFI_NOT_FOUND; break;

In the sandbox environment we can not easily build efi stub binaries right now, so let's disable the respective test cases for the efi selftest suite.

Signed-off-by: Alexander Graf agraf@suse.de Reviewed-by: Simon Glass sjg@chromium.org --- lib/efi_selftest/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile index 4fe404d88d..bf5c8199cb 100644 --- a/lib/efi_selftest/Makefile +++ b/lib/efi_selftest/Makefile @@ -41,7 +41,7 @@ endif

# TODO: As of v2018.01 the relocation code for the EFI application cannot # be built on x86_64. -ifeq ($(CONFIG_X86_64),) +ifeq ($(CONFIG_X86_64)$(CONFIG_SANDBOX),)

ifneq ($(CONFIG_CMD_BOOTEFI_SELFTEST),)

The fs_read() and fs_write() functions are internal interfaces that naturally want to get pointers as arguments. Most users so far even have pointers and explicitly cast them into integers just to be able to pass them into the function.

Convert them over to instead take a pointer argument for the buffer. That way any sandbox mapping gets greatly simplified and users of the API intuitively know what to do.

Signed-off-by: Alexander Graf agraf@suse.de --- board/BuR/common/common.c | 2 +- board/gdsys/p1022/controlcenterd-id.c | 10 +++++----- cmd/mvebu/bubt.c | 4 ++-- common/splash_source.c | 4 +++- drivers/bootcount/bootcount_ext.c | 12 ++++++------ drivers/fpga/zynqpl.c | 8 +++++--- fs/fs.c | 20 ++++++++++---------- include/fs.h | 12 ++++++------ lib/efi_loader/efi_file.c | 6 ++---- 9 files changed, 40 insertions(+), 38 deletions(-)

diff --git a/board/BuR/common/common.c b/board/BuR/common/common.c index 9df19791c2..ab9d9c51cf 100644 --- a/board/BuR/common/common.c +++ b/board/BuR/common/common.c @@ -269,7 +269,7 @@ static int load_devicetree(void) puts("load_devicetree: set_blk_dev failed.\n"); return -1; } - rc = fs_read(dtbname, (u32)dtbaddr, 0, 0, &dtbsize); + rc = fs_read(dtbname, (u_char *)dtbaddr, 0, 0, &dtbsize); #endif if (rc == 0) { gd->fdt_blob = (void *)dtbaddr; diff --git a/board/gdsys/p1022/controlcenterd-id.c b/board/gdsys/p1022/controlcenterd-id.c index 7e082dff05..2f01f7b7eb 100644 --- a/board/gdsys/p1022/controlcenterd-id.c +++ b/board/gdsys/p1022/controlcenterd-id.c @@ -874,7 +874,7 @@ static struct key_program *load_key_chunk(const char *ifname,

if (fs_set_blk_dev(ifname, dev_part_str, fs_type)) goto failure; - if (fs_read(path, (ulong)buf, 0, 12, &i) < 0) + if (fs_read(path, buf, 0, 12, &i) < 0) goto failure; if (i < 12) goto failure; @@ -890,7 +890,7 @@ static struct key_program *load_key_chunk(const char *ifname, goto failure; if (fs_set_blk_dev(ifname, dev_part_str, fs_type)) goto failure; - if (fs_read(path, (ulong)result, 0, + if (fs_read(path, result, 0, sizeof(struct key_program) + header.code_size, &i) < 0) goto failure; if (i <= 0) @@ -1019,7 +1019,7 @@ static int second_stage_init(void) struct key_program *hmac_blob = NULL; const char *image_path = "/ccdm.itb"; char *mac_path = NULL; - ulong image_addr; + u8 *image_addr; loff_t image_size; uint32_t err;

@@ -1059,7 +1059,7 @@ static int second_stage_init(void) strcat(mac_path, mac_suffix);

/* read image from mmcdev (ccdm.itb) */ - image_addr = (ulong)get_image_location(); + image_addr = get_image_location(); if (fs_set_blk_dev("mmc", mmcdev, FS_TYPE_EXT)) goto failure; if (fs_read(image_path, image_addr, 0, 0, &image_size) < 0) @@ -1077,7 +1077,7 @@ static int second_stage_init(void) puts("corrupted mac file\n"); goto failure; } - if (check_hmac(hmac_blob, (u8 *)image_addr, image_size)) { + if (check_hmac(hmac_blob, image_addr, image_size)) { puts("image integrity could not be verified\n"); goto failure; } diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c index b4d371f305..29fff898fa 100644 --- a/cmd/mvebu/bubt.c +++ b/cmd/mvebu/bubt.c @@ -209,7 +209,7 @@ static size_t mmc_read_file(const char *file_name) }

/* Perfrom file read */ - rc = fs_read(file_name, get_load_addr(), 0, 0, &act_read); + rc = fs_read(file_name, (void *)get_load_addr(), 0, 0, &act_read); if (rc) return 0;

@@ -392,7 +392,7 @@ static size_t usb_read_file(const char *file_name) }

/* Perfrom file read */ - rc = fs_read(file_name, get_load_addr(), 0, 0, &act_read); + rc = fs_read(file_name, (void *)get_load_addr(), 0, 0, &act_read); if (rc) return 0;

diff --git a/common/splash_source.c b/common/splash_source.c index 62763b9ebd..79dbea12fc 100644 --- a/common/splash_source.c +++ b/common/splash_source.c @@ -11,6 +11,7 @@ #include <fs.h> #include <fdt_support.h> #include <image.h> +#include <mapmem.h> #include <nand.h> #include <sata.h> #include <spi.h> @@ -252,7 +253,8 @@ static int splash_load_fs(struct splash_location *location, u32 bmp_load_addr) }

splash_select_fs_dev(location); - res = fs_read(splash_file, bmp_load_addr, 0, 0, &actread); + res = fs_read(splash_file, map_sysmem(bmp_load_addr, bmp_size), + 0, 0, &actread);

out: if (location->ubivol != NULL) diff --git a/drivers/bootcount/bootcount_ext.c b/drivers/bootcount/bootcount_ext.c index 075e590896..4a46f17c15 100644 --- a/drivers/bootcount/bootcount_ext.c +++ b/drivers/bootcount/bootcount_ext.c @@ -24,10 +24,10 @@ void bootcount_store(ulong a) buf = map_sysmem(CONFIG_SYS_BOOTCOUNT_ADDR, 2); buf[0] = BC_MAGIC; buf[1] = (a & 0xff); - unmap_sysmem(buf);

- ret = fs_write(CONFIG_SYS_BOOTCOUNT_EXT_NAME, - CONFIG_SYS_BOOTCOUNT_ADDR, 0, 2, &len); + ret = fs_write(CONFIG_SYS_BOOTCOUNT_EXT_NAME, buf, 0, 2, &len); + + unmap_sysmem(buf); if (ret != 0) puts("Error storing bootcount\n"); } @@ -44,14 +44,14 @@ ulong bootcount_load(void) return 0; }

- ret = fs_read(CONFIG_SYS_BOOTCOUNT_EXT_NAME, CONFIG_SYS_BOOTCOUNT_ADDR, - 0, 2, &len_read); + buf = map_sysmem(CONFIG_SYS_BOOTCOUNT_ADDR, 2); + + ret = fs_read(CONFIG_SYS_BOOTCOUNT_EXT_NAME, buf, 0, 2, &len_read); if (ret != 0 || len_read != 2) { puts("Error loading bootcount\n"); return 0; }

- buf = map_sysmem(CONFIG_SYS_BOOTCOUNT_ADDR, 2); if (buf[0] == BC_MAGIC) ret = buf[1];

diff --git a/drivers/fpga/zynqpl.c b/drivers/fpga/zynqpl.c index fd37d18c7f..2fba77d45f 100644 --- a/drivers/fpga/zynqpl.c +++ b/drivers/fpga/zynqpl.c @@ -431,7 +431,7 @@ static int zynq_loadfs(xilinx_desc *desc, const void *buf, size_t bsize, if (fs_set_blk_dev(interface, dev_part, fstype)) return FPGA_FAIL;

- if (fs_read(filename, (u32) buf, pos, blocksize, &actread) < 0) + if (fs_read(filename, (void *)buf, pos, blocksize, &actread) < 0) return FPGA_FAIL;

if (zynq_validate_bitstream(desc, buf, bsize, blocksize, &swap, @@ -454,10 +454,12 @@ static int zynq_loadfs(xilinx_desc *desc, const void *buf, size_t bsize, return FPGA_FAIL;

if (bsize > blocksize) { - if (fs_read(filename, (u32) buf, pos, blocksize, &actread) < 0) + if (fs_read(filename, (void *)buf, pos, blocksize, + &actread) < 0) return FPGA_FAIL; } else { - if (fs_read(filename, (u32) buf, pos, bsize, &actread) < 0) + if (fs_read(filename, (void *)buf, pos, bsize, + &actread) < 0) return FPGA_FAIL; } } while (bsize > blocksize); diff --git a/fs/fs.c b/fs/fs.c index 33808d549e..27ce9259d2 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -402,20 +402,17 @@ int fs_size(const char *filename, loff_t *size) return ret; }

-int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len, +int fs_read(const char *filename, void *buf, loff_t offset, loff_t len, loff_t *actread) { struct fstype_info *info = fs_get_info(fs_type); - void *buf; int ret;

/* * We don't actually know how many bytes are being read, since len==0 * means read the whole file. */ - buf = map_sysmem(addr, len); ret = info->read(filename, buf, offset, len, actread); - unmap_sysmem(buf);

/* If we requested a specific number of bytes, check we got it */ if (ret == 0 && len && *actread != len) @@ -425,16 +422,13 @@ int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len, return ret; }

-int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len, +int fs_write(const char *filename, void *buf, loff_t offset, loff_t len, loff_t *actwrite) { struct fstype_info *info = fs_get_info(fs_type); - void *buf; int ret;

- buf = map_sysmem(addr, len); ret = info->write(filename, buf, offset, len, actwrite); - unmap_sysmem(buf);

if (ret < 0 && len != *actwrite) { printf("** Unable to write file %s **\n", filename); @@ -529,6 +523,7 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], int ret; unsigned long time; char *ep; + void *buf;

if (argc < 2) return CMD_RET_USAGE; @@ -567,9 +562,11 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], else pos = 0;

+ buf = map_sysmem(addr, bytes); time = get_timer(0); - ret = fs_read(filename, addr, pos, bytes, &len_read); + ret = fs_read(filename, buf, pos, bytes, &len_read); time = get_timer(time); + unmap_sysmem(buf); if (ret < 0) return 1;

@@ -623,6 +620,7 @@ int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], loff_t len; int ret; unsigned long time; + void *buf;

if (argc < 6 || argc > 7) return CMD_RET_USAGE; @@ -638,9 +636,11 @@ int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], else pos = 0;

+ buf = map_sysmem(addr, bytes); time = get_timer(0); - ret = fs_write(filename, addr, pos, bytes, &len); + ret = fs_write(filename, buf, pos, bytes, &len); time = get_timer(time); + unmap_sysmem(buf); if (ret < 0) return 1;

diff --git a/include/fs.h b/include/fs.h index 163da103b4..647b0c2ed2 100644 --- a/include/fs.h +++ b/include/fs.h @@ -76,27 +76,27 @@ int fs_size(const char *filename, loff_t *size); * Note that not all filesystem types support either/both offset!=0 or len!=0. * * @filename: Name of file to read from - * @addr: The address to read into + * @buf: The buffer to read into * @offset: The offset in file to read from * @len: The number of bytes to read. Maybe 0 to read entire file * @actread: Returns the actual number of bytes read * @return 0 if ok with valid *actread, -1 on error conditions */ -int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len, +int fs_read(const char *filename, void *buf, loff_t offset, loff_t len, loff_t *actread);

/* * fs_write - Write file to the partition previously set by fs_set_blk_dev() * Note that not all filesystem types support offset!=0. * - * @filename: Name of file to read from - * @addr: The address to read into - * @offset: The offset in file to read from. Maybe 0 to write to start of file + * @filename: Name of file to write to + * @buf: The buffer to read from + * @offset: The offset in file to write to. Maybe 0 to write to start of file * @len: The number of bytes to write * @actwrite: Returns the actual number of bytes written * @return 0 if ok with valid *actwrite, -1 on error conditions */ -int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len, +int fs_write(const char *filename, void *buf, loff_t offset, loff_t len, loff_t *actwrite);

/* diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c index e6a15bcb52..7bd061f395 100644 --- a/lib/efi_loader/efi_file.c +++ b/lib/efi_loader/efi_file.c @@ -233,8 +233,7 @@ static efi_status_t file_read(struct file_handle *fh, u64 *buffer_size, { loff_t actread;

- if (fs_read(fh->path, (ulong)buffer, fh->offset, - *buffer_size, &actread)) + if (fs_read(fh->path, buffer, fh->offset, *buffer_size, &actread)) return EFI_DEVICE_ERROR;

*buffer_size = actread; @@ -363,8 +362,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, - &actwrite)) { + if (fs_write(fh->path, buffer, fh->offset, *buffer_size, &actwrite)) { ret = EFI_DEVICE_ERROR; goto error; }

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 df58e633d1..d471e6f4a4 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 do not support bootefi booting ARMv7 in non-secure mode depends on !ARMV7_NONSEC # We need EFI_STUB_64BIT to be set on x86_64 with EFI_STUB

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 --- lib/efi_loader/efi_runtime.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 388dfb9840..bc44e43745 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -32,18 +32,18 @@ static efi_status_t __efi_runtime EFIAPI efi_invalid_parameter(void); * TODO(sjg@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) +#if defined(__aarch64__) #define R_RELATIVE 1027 #define R_MASK 0xffffffffULL #define IS_RELA 1 -#elif defined(CONFIG_ARM) +#elif defined(__arm__) #define R_RELATIVE 23 #define R_MASK 0xffULL -#elif defined(CONFIG_X86) +#elif defined(__x86_64__) || defined(__i386__) #include <asm/elf.h> #define R_RELATIVE R_386_RELATIVE #define R_MASK 0xffULL -#elif defined(CONFIG_RISCV) +#elif defined(__riscv) #include <elf.h> #define R_RELATIVE R_RISCV_RELATIVE #define R_MASK 0xffULL @@ -55,25 +55,15 @@ struct dyn_sym { u32 foo2; u32 foo3; }; -#ifdef CONFIG_CPU_RISCV_32 +#if (__riscv_xlen == 32) #define R_ABSOLUTE R_RISCV_32 #define SYM_INDEX 8 -#else +#elif (__riscv_xlen == 64) #define R_ABSOLUTE R_RISCV_64 #define SYM_INDEX 32 +#else +#error unknown riscv target #endif - -/* For sandbox we only support 64-bit x86 at present */ -#elif defined(CONFIG_SANDBOX) -/* - * TODO(sjg@chromium.org): Consider providing a way to enable sandbox features - * based on the host architecture - */ -# ifndef __x86_64__ -# warning "sandbox EFI support is only tested on 64-bit x86" -# endif -#define R_RELATIVE 8 -#define R_MASK 0xffffffffULL #else #error Need to add relocation awareness #endif

In sandbox, longjmp returns to itself in an endless loop. Cut this through by calling the real OS function.

Setjmp on the other hand must not return. So here we have to call the OS setjmp function straight from the code where the setjmp call happens.

Signed-off-by: Alexander Graf agraf@suse.de --- arch/sandbox/cpu/cpu.c | 5 ----- arch/sandbox/cpu/os.c | 20 ++------------------ arch/sandbox/include/asm/setjmp.h | 4 +++- 3 files changed, 5 insertions(+), 24 deletions(-)

diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c index 29dac8dfda..9087026d71 100644 --- a/arch/sandbox/cpu/cpu.c +++ b/arch/sandbox/cpu/cpu.c @@ -167,11 +167,6 @@ 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); diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index 5839932b00..9cfdc9f31f 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -630,24 +630,8 @@ void os_localtime(struct rtc_time *rt) 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); + /* Call the OS longjmp function directly */ + _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..e103d170e0 100644 --- a/arch/sandbox/include/asm/setjmp.h +++ b/arch/sandbox/include/asm/setjmp.h @@ -24,7 +24,9 @@ struct jmp_buf_data {

typedef struct jmp_buf_data jmp_buf[1];

-int setjmp(jmp_buf jmp); +/* Call the OS setjmp function directly, so that we don't introduce returns */ +#define setjmp _setjmp +int _setjmp(jmp_buf jmp); __noreturn void longjmp(jmp_buf jmp, int ret);

#endif /* _SETJMP_H_ */

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 whenever we get into the efi_loader case, let's mark RAM as executable. That way we still protect normal cases, but allow for efi binaries to directly get executed from within RAM.

For this, we hook into the already existing allow_unaligned() call which also transitions the system over into semantics required by the UEFI specification.

Signed-off-by: Alexander Graf agraf@suse.de --- arch/sandbox/cpu/cpu.c | 14 ++++++++++++++ arch/sandbox/cpu/os.c | 14 ++++++++++++++ include/os.h | 19 +++++++++++++++++++ 3 files changed, 47 insertions(+)

diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c index 9087026d71..641b66a0a7 100644 --- a/arch/sandbox/cpu/cpu.c +++ b/arch/sandbox/cpu/cpu.c @@ -192,3 +192,17 @@ void efi_add_known_memory(void) }

#endif + +#if CONFIG_IS_ENABLED(EFI_LOADER) + +void allow_unaligned(void) +{ + int r; + + r = os_mprotect(gd->arch.ram_buf, gd->ram_size, + OS_PROT_READ | OS_PROT_WRITE | OS_PROT_EXEC); + + assert(!r); +} + +#endif diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index 9cfdc9f31f..9fa79a8843 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -183,6 +183,20 @@ void *os_realloc(void *ptr, size_t length) return buf; }

+int os_mprotect(void *ptr, size_t length, int prot) +{ + int p = 0; + + if (prot & OS_PROT_READ) + p |= PROT_READ; + if (prot & OS_PROT_WRITE) + p |= PROT_WRITE; + if (prot & OS_PROT_EXEC) + p |= PROT_EXEC; + + return mprotect(ptr, length, p); +} + void os_usleep(unsigned long usec) { usleep(usec); diff --git a/include/os.h b/include/os.h index c8e0f52d30..d451e12064 100644 --- a/include/os.h +++ b/include/os.h @@ -157,6 +157,25 @@ void os_free(void *ptr); void *os_realloc(void *ptr, size_t length);

/** + * Modify protection of a memory region + * + * This function changes the memory protection scheme of a given memory + * region. Using it you can for example allow execution of memory that + * would otherwise prohibit it. + * + * \param ptr Pointer to memory region to modify + * \param length New length for memory block + * \param prot New protection scheme (ORed OS_PROT_ values) + * \return 0 on success, -1 otherwise. + */ +int os_mprotect(void *ptr, size_t length, int prot); + +/* Defines for "prot" in os_mprotect() */ +#define OS_PROT_READ 0x1 +#define OS_PROT_WRITE 0x2 +#define OS_PROT_EXEC 0x4 + +/** * Access to the usleep function of the os * * \param usec Time to sleep in micro seconds

We currently expose host addresses in the EFI memory map. That can be bad if we ever want to use sandbox to boot strap a real kernel, because then the kernel would fetch its memory table from our host virtual address map. But to make that use case work, we would need to have full control over the address space the EFI application sees.

So let's expose only U-Boot addresses to the guest until we get to the point of allocation. EFI's allocation functions are fun - they can take U-Boot addresses as input values for hints and return host addresses as allocation results through the same uint64_t * parameter. So we need to be extra careful on what to pass in when.

With this patch I am successfully able to run the efi selftest suite as well as grub.efi on aarch64.

Signed-off-by: Alexander Graf agraf@suse.de --- arch/sandbox/cpu/cpu.c | 19 ------------------- lib/efi_loader/efi_memory.c | 12 ++++++------ 2 files changed, 6 insertions(+), 25 deletions(-)

diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c index 641b66a0a7..be88ab2f1c 100644 --- a/arch/sandbox/cpu/cpu.c +++ b/arch/sandbox/cpu/cpu.c @@ -176,25 +176,6 @@ void longjmp(jmp_buf jmp, int ret)

#if CONFIG_IS_ENABLED(EFI_LOADER)

-/* - * In sandbox, we don't have a 1:1 map, so we need to expose - * process addresses instead of U-Boot addresses - */ -void efi_add_known_memory(void) -{ - u64 ram_start = (uintptr_t)map_sysmem(0, gd->ram_size); - u64 ram_size = gd->ram_size; - u64 start = (ram_start + EFI_PAGE_MASK) & ~EFI_PAGE_MASK; - u64 pages = (ram_size + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT; - - efi_add_memory_map(start, pages, EFI_CONVENTIONAL_MEMORY, - false); -} - -#endif - -#if CONFIG_IS_ENABLED(EFI_LOADER) - void allow_unaligned(void) { int r; diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 19492df518..64d2b8f7fa 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -326,7 +326,7 @@ efi_status_t efi_allocate_pages(int type, int memory_type, /* Reserve that map in our memory maps */ ret = efi_add_memory_map(addr, pages, memory_type, true); if (ret == addr) { - *memory = addr; + *memory = (uintptr_t)map_sysmem(addr, pages * EFI_PAGE_SIZE); } else { /* Map would overlap, bail out */ r = EFI_OUT_OF_RESOURCES; @@ -360,11 +360,12 @@ void *efi_alloc(uint64_t len, int memory_type) efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages) { uint64_t r = 0; + uint64_t addr = map_to_sysmem((void*)(uintptr_t)memory);

- r = efi_add_memory_map(memory, pages, EFI_CONVENTIONAL_MEMORY, false); + r = efi_add_memory_map(addr, pages, EFI_CONVENTIONAL_MEMORY, false); /* Merging of adjacent free regions is missing */

- if (r == memory) + if (r == addr) return EFI_SUCCESS;

return EFI_NOT_FOUND; @@ -381,9 +382,9 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages) efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer) { efi_status_t r; - efi_physical_addr_t t; u64 num_pages = (size + sizeof(struct efi_pool_allocation) + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT; + struct efi_pool_allocation *alloc;

if (size == 0) { *buffer = NULL; @@ -391,10 +392,9 @@ efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer) }

r = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, pool_type, num_pages, - &t); + (uint64_t*)&alloc);

if (r == EFI_SUCCESS) { - struct efi_pool_allocation *alloc = (void *)(uintptr_t)t; alloc->num_pages = num_pages; *buffer = alloc->data; }

On 06/18/2018 05:22 PM, Alexander Graf wrote:

This patch set augments Simon's patch set for efi_loader support in sandbox[1], but cuts off the memory allocation scheme at a different point.

According to the UEFI spec, efi_allocate_pages() takes a uint64_t * argument. Via this argument, we get a physical address as input, but emit a pointer as output.

With this patch set in place, I can successfully run the selftest suite as well as an aarch64 grub.efi binary. X86_64 grub.efi doesn't work because that one requires inl instructions to work.

I've assembled a quick grub.efi that does work in sandbox as it no longer accesses I/O ports directly. Patch for it is below.

http://csgraf.de/tmp2/grub.efi

When building your own, make sure to exclude coreboot (cb*) modules - they seem to do something dirty and segfault for me. The other modules seem to work fine for me so far.

Alex

diff --git a/grub-core/kern/i386/tsc.c b/grub-core/kern/i386/tsc.c index f266eb131..99fc9f7fb 100644 --- a/grub-core/kern/i386/tsc.c +++ b/grub-core/kern/i386/tsc.c @@ -68,7 +68,7 @@ grub_tsc_init (void)  #ifdef GRUB_MACHINE_XEN    (void) (grub_tsc_calibrate_from_xen () || calibrate_tsc_hardcode());  #elif defined (GRUB_MACHINE_EFI) -  (void) (grub_tsc_calibrate_from_pmtimer () || grub_tsc_calibrate_from_pit () || grub_tsc_calibrate_from_efi() || calibrate_tsc_hardcode()); +  (void) (grub_tsc_calibrate_from_efi() || calibrate_tsc_hardcode());  #elif defined (GRUB_MACHINE_COREBOOT)    (void) (grub_tsc_calibrate_from_pmtimer () || grub_tsc_calibrate_from_pit () || calibrate_tsc_hardcode());  #else diff --git a/include/grub/i386/io.h b/include/grub/i386/io.h index ae12a3e3d..9fbeef916 100644 --- a/include/grub/i386/io.h +++ b/include/grub/i386/io.h @@ -26,47 +26,40 @@ typedef unsigned short int grub_port_t;  static __inline unsigned char  grub_inb (unsigned short int port)  { -  unsigned char _v; +  unsigned char _v = 0;

-  __asm__ __volatile__ ("inb %w1,%0":"=a" (_v):"Nd" (port));    return _v;  }

static __inline unsigned short int  grub_inw (unsigned short int port)  { -  unsigned short _v; +  unsigned short _v = 0;

-  __asm__ __volatile__ ("inw %w1,%0":"=a" (_v):"Nd" (port));    return _v;  }

static __inline unsigned int  grub_inl (unsigned short int port)  { -  unsigned int _v; +  unsigned int _v = 0;

-  __asm__ __volatile__ ("inl %w1,%0":"=a" (_v):"Nd" (port));    return _v;  }

static __inline void  grub_outb (unsigned char value, unsigned short int port)  { -  __asm__ __volatile__ ("outb %b0,%w1": :"a" (value), "Nd" (port));  }

static __inline void  grub_outw (unsigned short int value, unsigned short int port)  { -  __asm__ __volatile__ ("outw %w0,%w1": :"a" (value), "Nd" (port)); -  }

static __inline void  grub_outl (unsigned int value, unsigned short int port)  { -  __asm__ __volatile__ ("outl %0,%w1": :"a" (value), "Nd" (port));  }

#endif /* _SYS_IO_H */