[U-Boot] [PATCH v3 0/9] efi_loader: fix memory handling
The Sandbox uses an address space that is neither the physical nor the virtual address space of the operating system.
In the EFI subsystem we should only use addresses that can be used by an EFI application. With the patch series we eliminate the Sandbox addresses from the EFI memory map.
In the flattened device tree the Sandbox needs its own address space. So make sure that this is always used here.
v3: merge two preexisting patch series resequence the patches to avoid changes to be undone later fix the value of fdtcontroladdr on the sandbox rebase patches
Heinrich Schuchardt (9): efi_loader: eliminate sandbox addresses efi_selftest: add test for memory allocation efi_selftest: building sandbox with EFI_SELFTEST efi_loader: macro efi_size_in_pages() fdt: sandbox: correct use of ${fdtcontroladdr} fdt_support: fdt reservations on the sandbox efi_loader: fix memory mapping for sandbox efi_loader: create fdt reservation before copy efi_selftest: check fdt is marked as runtime data
cmd/bootefi.c | 56 ++++---- common/board_r.c | 3 +- common/fdt_support.c | 3 +- include/efi_loader.h | 11 +- lib/efi_loader/efi_memory.c | 15 +- lib/efi_selftest/Kconfig | 2 +- lib/efi_selftest/Makefile | 1 + lib/efi_selftest/efi_selftest_memory.c | 187 +++++++++++++++++++++++++ lib/fdtdec.c | 6 +- test/py/tests/test_efi_selftest.py | 10 +- 10 files changed, 246 insertions(+), 48 deletions(-) create mode 100644 lib/efi_selftest/efi_selftest_memory.c
Do not use the sandbox's virtual address space for the internal structures of the memory map. This way we can eliminate a whole lot of unnecessary conversions.
The only conversion remaining is the one when adding known memory.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 rebase the patch to be the first in the series --- lib/efi_loader/efi_memory.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index f225a9028c5..c3f0b966f41 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -384,7 +384,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 = (uintptr_t)map_sysmem(addr, len); + *memory = addr; } else { /* Map would overlap, bail out */ r = EFI_OUT_OF_RESOURCES; @@ -418,12 +418,11 @@ 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(addr, pages, EFI_CONVENTIONAL_MEMORY, false); + r = efi_add_memory_map(memory, pages, EFI_CONVENTIONAL_MEMORY, false); /* Merging of adjacent free regions is missing */
- if (r == addr) + if (r == memory) return EFI_SUCCESS;
return EFI_NOT_FOUND; @@ -557,7 +556,7 @@ __weak void efi_add_known_memory(void) for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { u64 ram_end, ram_start, pages;
- ram_start = gd->bd->bi_dram[i].start; + ram_start = (uintptr_t)map_sysmem(gd->bd->bi_dram[i].start, 0); ram_end = ram_start + gd->bd->bi_dram[i].size;
/* Remove partial pages */
This unit test checks the following runtime services: AllocatePages, FreePages, GetMemoryMap
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3: no change --- lib/efi_selftest/Makefile | 1 + lib/efi_selftest/efi_selftest_memory.c | 163 +++++++++++++++++++++++++ 2 files changed, 164 insertions(+) create mode 100644 lib/efi_selftest/efi_selftest_memory.c
diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile index 4b1c0bb84b1..743b4820449 100644 --- a/lib/efi_selftest/Makefile +++ b/lib/efi_selftest/Makefile @@ -27,6 +27,7 @@ efi_selftest_fdt.o \ efi_selftest_gop.o \ efi_selftest_loaded_image.o \ efi_selftest_manageprotocols.o \ +efi_selftest_memory.o \ efi_selftest_rtc.o \ efi_selftest_snp.o \ efi_selftest_textinput.o \ diff --git a/lib/efi_selftest/efi_selftest_memory.c b/lib/efi_selftest/efi_selftest_memory.c new file mode 100644 index 00000000000..0623e8e62da --- /dev/null +++ b/lib/efi_selftest/efi_selftest_memory.c @@ -0,0 +1,163 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * efi_selftest_memory + * + * Copyright (c) 2018 Heinrich Schuchardt xypron.glpk@gmx.de + * + * This unit test checks the following runtime services: + * AllocatePages, FreePages, GetMemoryMap + */ + +#include <efi_selftest.h> + +#define EFI_ST_NUM_PAGES 8 + +static struct efi_boot_services *boottime; + +/** + * setup() - setup unit test + * + * @handle: handle of the loaded image + * @systable: system table + * Return: EFI_ST_SUCCESS for success + */ +static int setup(const efi_handle_t handle, + const struct efi_system_table *systable) +{ + boottime = systable->boottime; + + return EFI_ST_SUCCESS; +} + +/** + * find_in_memory_map() - check matching memory map entry exists + * + * @memory_map: memory map + * @desc_size: number of memory map entries + * @addr: physical address to find in the map + * @type: expected memory type + * Return: EFI_ST_SUCCESS for success + */ +static int find_in_memory_map(efi_uintn_t map_size, + struct efi_mem_desc *memory_map, + efi_uintn_t desc_size, + u64 addr, int memory_type) +{ + efi_uintn_t i; + bool found = false; + + for (i = 0; map_size; ++i, map_size -= desc_size) { + struct efi_mem_desc *entry = &memory_map[i]; + + if (addr >= entry->physical_start && + addr < entry->physical_start + + (entry->num_pages << EFI_PAGE_SHIFT)) { + if (found) { + efi_st_error("Duplicate memory map entry\n"); + return EFI_ST_FAILURE; + } + found = true; + if (memory_type != entry->type) { + efi_st_error + ("Wrong memory type %d, expected %d\n", + entry->type, memory_type); + return EFI_ST_FAILURE; + } + } + } + if (!found) { + efi_st_error("Missing memory map entry\n"); + return EFI_ST_FAILURE; + } + return EFI_ST_SUCCESS; +} + +/* + * execute() - execute unit test + * + * Return: EFI_ST_SUCCESS for success + */ +static int execute(void) +{ + u64 p1; + u64 p2; + efi_uintn_t map_size = 0; + efi_uintn_t map_key; + efi_uintn_t desc_size; + u32 desc_version; + struct efi_mem_desc *memory_map; + efi_status_t ret; + + /* Allocate two page ranges with different memory type */ + ret = boottime->allocate_pages(EFI_ALLOCATE_ANY_PAGES, + EFI_RUNTIME_SERVICES_CODE, + EFI_ST_NUM_PAGES, &p1); + if (ret != EFI_SUCCESS) { + efi_st_error("AllocatePages did not return EFI_SUCCESS\n"); + return EFI_ST_FAILURE; + } + ret = boottime->allocate_pages(EFI_ALLOCATE_ANY_PAGES, + EFI_RUNTIME_SERVICES_DATA, + EFI_ST_NUM_PAGES, &p2); + if (ret != EFI_SUCCESS) { + efi_st_error("AllocatePages did not return EFI_SUCCESS\n"); + return EFI_ST_FAILURE; + } + + /* Load memory map */ + ret = boottime->get_memory_map(&map_size, NULL, &map_key, &desc_size, + &desc_version); + if (ret != EFI_BUFFER_TOO_SMALL) { + efi_st_error + ("GetMemoryMap did not return EFI_BUFFER_TOO_SMALL\n"); + return EFI_ST_FAILURE; + } + /* Allocate extra space for newly allocated memory */ + map_size += sizeof(struct efi_mem_desc); + ret = boottime->allocate_pool(EFI_BOOT_SERVICES_DATA, map_size, + (void **)&memory_map); + if (ret != EFI_SUCCESS) { + efi_st_error("AllocatePool did not return EFI_SUCCESS\n"); + return EFI_ST_FAILURE; + } + ret = boottime->get_memory_map(&map_size, memory_map, &map_key, + &desc_size, &desc_version); + if (ret != EFI_SUCCESS) { + efi_st_error("GetMemoryMap did not return EFI_SUCCESS\n"); + return EFI_ST_FAILURE; + } + + /* Check memory map entries */ + if (find_in_memory_map(map_size, memory_map, desc_size, p1, + EFI_RUNTIME_SERVICES_CODE) != EFI_ST_SUCCESS) + return EFI_ST_FAILURE; + if (find_in_memory_map(map_size, memory_map, desc_size, p2, + EFI_RUNTIME_SERVICES_DATA) != EFI_ST_SUCCESS) + return EFI_ST_FAILURE; + + /* Free memory */ + ret = boottime->free_pages(p1, EFI_ST_NUM_PAGES); + if (ret != EFI_SUCCESS) { + efi_st_error("FreePages did not return EFI_SUCCESS\n"); + return EFI_ST_FAILURE; + } + ret = boottime->free_pages(p2, EFI_ST_NUM_PAGES); + if (ret != EFI_SUCCESS) { + efi_st_error("FreePages did not return EFI_SUCCESS\n"); + return EFI_ST_FAILURE; + } + ret = boottime->free_pool(memory_map); + if (ret != EFI_SUCCESS) { + efi_st_error("FreePool did not return EFI_SUCCESS\n"); + return EFI_ST_FAILURE; + } + + return EFI_ST_SUCCESS; +} + +EFI_UNIT_TEST(memory) = { + .name = "memory", + .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT, + .setup = setup, + .execute = execute, +};
Enable building the sandbox with CONFIG_EFI_SELFTEST.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 rebase patch --- lib/efi_selftest/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_selftest/Kconfig b/lib/efi_selftest/Kconfig index b52696778dd..59f9f36801c 100644 --- a/lib/efi_selftest/Kconfig +++ b/lib/efi_selftest/Kconfig @@ -1,6 +1,6 @@ config CMD_BOOTEFI_SELFTEST bool "Allow booting an EFI efi_selftest" - depends on CMD_BOOTEFI && !SANDBOX + depends on CMD_BOOTEFI imply FAT imply FAT_WRITE help
When allocating EFI memory pages the size in bytes has to be converted to pages.
Provide a macro efi_size_in_pages() for this conversion. Use it in the EFI subsystem and correct related comments.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 rebase the patch fix a comment --- cmd/bootefi.c | 15 ++++++--------- include/efi_loader.h | 11 ++++++++++- lib/efi_loader/efi_memory.c | 6 +++--- 3 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index c73e6228d3e..2c9b2eb8b6f 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -154,7 +154,7 @@ static void set_load_options(struct efi_loaded_image *loaded_image_info, * 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 + * Additional 12 KiB are 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 @@ -182,14 +182,12 @@ static efi_status_t copy_fdt(ulong *fdt_addrp) }
/* - * 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. + * Give us at least 12 KiB of breathing room in case the device tree + * needs to be expanded later. */ 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; + fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000); + fdt_size = fdt_pages << EFI_PAGE_SHIFT;
/* Safe fdt location is at 127MB */ new_fdt_addr = fdt_ram_start + (127 * 1024 * 1024) + fdt_size; @@ -287,8 +285,7 @@ static void efi_carve_out_dt_rsv(void *fdt) if (addr == (uintptr_t)fdt) continue;
- pages = ALIGN(size + (addr & EFI_PAGE_MASK), EFI_PAGE_SIZE) >> - EFI_PAGE_SHIFT; + pages = efi_size_in_pages(size + (addr & EFI_PAGE_MASK)); addr &= ~EFI_PAGE_MASK; if (!efi_add_memory_map(addr, pages, EFI_RESERVED_MEMORY_TYPE, false)) diff --git a/include/efi_loader.h b/include/efi_loader.h index bdb806cfce4..244e754e8fd 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -350,7 +350,16 @@ struct efi_simple_file_system_protocol *efi_simple_file_system( /* open file from device-path: */ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp);
- +/** + * efi_size_in_pages() - convert size in bytes to size in pages + * + * This macro returns the number of EFI memory pages required to hold 'size' + * bytes. + * + * @size: size in bytes + * Return: size in pages + */ +#define efi_size_in_pages(size) ((size + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT) /* Generic EFI memory allocator, call this to get memory */ void *efi_alloc(uint64_t len, int memory_type); /* More specific EFI memory allocator, called by EFI payloads */ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index c3f0b966f41..fad4edf9ab6 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -397,7 +397,7 @@ efi_status_t efi_allocate_pages(int type, int memory_type, void *efi_alloc(uint64_t len, int memory_type) { uint64_t ret = 0; - uint64_t pages = (len + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT; + uint64_t pages = efi_size_in_pages(len); efi_status_t r;
r = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, memory_type, pages, @@ -440,8 +440,8 @@ efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer) { efi_status_t r; struct efi_pool_allocation *alloc; - u64 num_pages = (size + sizeof(struct efi_pool_allocation) + - EFI_PAGE_MASK) >> EFI_PAGE_SHIFT; + u64 num_pages = efi_size_in_pages(size + + sizeof(struct efi_pool_allocation));
if (!buffer) return EFI_INVALID_PARAMETER;
The sandbox uses a virtual address space that is neither the physical nor the virtual address space of the operating system. All address used on the command line live in this address space. So also the environment variable ${fdtcontroladdr} has to be in this address space.
Commands like bootefi and booti receive the fdt address as parameter. Without the patch ${fdtcontroladdr} cannot be used as parameter value on the sandbox.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 new patch --- common/board_r.c | 3 ++- lib/fdtdec.c | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index c55e33eec27..745212be71a 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -453,7 +453,8 @@ static int initr_env(void) else set_default_env(NULL, 0); #ifdef CONFIG_OF_CONTROL - env_set_addr("fdtcontroladdr", gd->fdt_blob); + env_set_hex("fdtcontroladdr", + (unsigned long)map_to_sysmem(gd->fdt_blob)); #endif
/* Initialize from environment */ diff --git a/lib/fdtdec.c b/lib/fdtdec.c index a420ba18854..e4c1f6af157 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -11,6 +11,7 @@ #include <errno.h> #include <fdtdec.h> #include <fdt_support.h> +#include <mapmem.h> #include <linux/libfdt.h> #include <serial.h> #include <asm/sections.h> @@ -1252,8 +1253,9 @@ int fdtdec_setup(void) # if CONFIG_IS_ENABLED(OF_PRIOR_STAGE) gd->fdt_blob = (void *)prior_stage_fdt_address; # else - gd->fdt_blob = (void *)env_get_ulong("fdtcontroladdr", 16, - (uintptr_t)gd->fdt_blob); + gd->fdt_blob = map_sysmem + (env_get_ulong("fdtcontroladdr", 16, + (unsigned long)map_to_sysmem(gd->fdt_blob)), 0); # endif # endif
On the sandbox the memory addresses in the device tree refer to the virtual address space of the sandbox. This implies that the memory reservations for the fdt also have to be converted to this address space.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 map the correct parameter of fdt_add_mem_rsv --- common/fdt_support.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/common/fdt_support.c b/common/fdt_support.c index e6daa67990d..3440e42a257 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -7,6 +7,7 @@ */
#include <common.h> +#include <mapmem.h> #include <stdio_dev.h> #include <linux/ctype.h> #include <linux/types.h> @@ -633,7 +634,7 @@ int fdt_shrink_to_minimum(void *blob, uint extrasize) fdt_set_totalsize(blob, actualsize);
/* Add the new reservation */ - ret = fdt_add_mem_rsv(blob, (uintptr_t)blob, actualsize); + ret = fdt_add_mem_rsv(blob, map_to_sysmem(blob), actualsize); if (ret < 0) return ret;
The sandbox is using a virtual address space which is neither the physical address space of the operating system nor the virtual address space in which Linux aplications live. The addresses used insided the flattened device tree use this sandbox virtual address space. The EFI subsystem uses the virtual address space of the operating system and this is where the fdt is stored.
Fix all incorrect addresses for the fdt in cmd/bootefi.cmd.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 rebase patch efi_install_fdt() is called with a sandbox address. This was not evident until fdtcontroladdr was fixed. --- cmd/bootefi.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 2c9b2eb8b6f..bbba3382a32 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -153,15 +153,16 @@ static void set_load_options(struct efi_loaded_image *loaded_image_info, /** * 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. + * The FDT is copied to a suitable location within the EFI memory map. * Additional 12 KiB are 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 + * @fdtp: On entry a pointer to the flattened device tree. + * On exit a pointer to the copy of the flattened device tree. * FDT start * Return: status code */ -static efi_status_t copy_fdt(ulong *fdt_addrp) +static efi_status_t copy_fdt(void **fdtp) { unsigned long fdt_ram_start = -1L, fdt_pages; efi_status_t ret = 0; @@ -185,12 +186,16 @@ static efi_status_t copy_fdt(ulong *fdt_addrp) * Give us at least 12 KiB of breathing room in case the device tree * needs to be expanded later. */ - fdt = map_sysmem(*fdt_addrp, 0); + fdt = *fdtp; fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000); fdt_size = fdt_pages << EFI_PAGE_SHIFT;
- /* Safe fdt location is at 127MB */ - new_fdt_addr = fdt_ram_start + (127 * 1024 * 1024) + fdt_size; + /* + * Safe fdt location is at 127 MiB. + * On the sandbox convert from the sandbox address space. + */ + new_fdt_addr = (uintptr_t)map_sysmem(fdt_ram_start + 0x7f00000 + + fdt_size, 0); ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, EFI_RUNTIME_SERVICES_DATA, fdt_pages, &new_fdt_addr); @@ -205,12 +210,11 @@ static efi_status_t copy_fdt(ulong *fdt_addrp) goto done; } } - - new_fdt = map_sysmem(new_fdt_addr, fdt_size); + new_fdt = (void *)(uintptr_t)new_fdt_addr; memcpy(new_fdt, fdt, fdt_totalsize(fdt)); fdt_set_totalsize(new_fdt, fdt_size);
- *fdt_addrp = new_fdt_addr; + *fdtp = (void *)(uintptr_t)new_fdt_addr; done: return ret; } @@ -278,6 +282,9 @@ static void efi_carve_out_dt_rsv(void *fdt) if (fdt_get_mem_rsv(fdt, i, &addr, &size) != 0) continue;
+ /* Convert from sandbox address space. */ + addr = (uintptr_t)map_sysmem(addr, 0); + /* * Do not carve out the device tree. It is already marked as * EFI_RUNTIME_SERVICES_DATA @@ -306,12 +313,10 @@ static efi_status_t efi_install_fdt(ulong fdt_addr) }
/* Prepare fdt for payload */ - ret = copy_fdt(&fdt_addr); + ret = copy_fdt(&fdt); if (ret) return ret;
- unmap_sysmem(fdt); - fdt = map_sysmem(fdt_addr, 0); if (image_setup_libfdt(&img, fdt, 0, NULL)) { printf("ERROR: failed to process device tree\n"); return EFI_LOAD_ERROR;
When copying the device we must ensure that the copy does not fall into a memory area reserved by the same.
So let's change the sequence: first create memory reservations and then copy the device tree.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 rebase patch --- cmd/bootefi.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index bbba3382a32..122b7f4b113 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -285,13 +285,6 @@ static void efi_carve_out_dt_rsv(void *fdt) /* Convert from sandbox address space. */ addr = (uintptr_t)map_sysmem(addr, 0);
- /* - * Do not carve out the device tree. It is already marked as - * EFI_RUNTIME_SERVICES_DATA - */ - if (addr == (uintptr_t)fdt) - continue; - pages = efi_size_in_pages(size + (addr & EFI_PAGE_MASK)); addr &= ~EFI_PAGE_MASK; if (!efi_add_memory_map(addr, pages, EFI_RESERVED_MEMORY_TYPE, @@ -312,6 +305,9 @@ static efi_status_t efi_install_fdt(ulong fdt_addr) return EFI_INVALID_PARAMETER; }
+ /* Create memory reservation as indicated by the device tree */ + efi_carve_out_dt_rsv(fdt); + /* Prepare fdt for payload */ ret = copy_fdt(&fdt); if (ret) @@ -322,8 +318,6 @@ static efi_status_t efi_install_fdt(ulong fdt_addr) return EFI_LOAD_ERROR; }
- efi_carve_out_dt_rsv(fdt); - /* Link to it in the efi tables */ ret = efi_install_configuration_table(&efi_guid_fdt, fdt); if (ret != EFI_SUCCESS)
Check that the memory area containing the device tree is marked as runtime data.
Update the Python test to pass ${fdtcontroladdr} to bootefi.
Update the description of the Python test.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 update python test too --- lib/efi_selftest/efi_selftest_memory.c | 24 ++++++++++++++++++++++++ test/py/tests/test_efi_selftest.py | 10 ++++++---- 2 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/lib/efi_selftest/efi_selftest_memory.c b/lib/efi_selftest/efi_selftest_memory.c index 0623e8e62da..24b4438ce4f 100644 --- a/lib/efi_selftest/efi_selftest_memory.c +++ b/lib/efi_selftest/efi_selftest_memory.c @@ -6,13 +6,17 @@ * * This unit test checks the following runtime services: * AllocatePages, FreePages, GetMemoryMap + * + * The memory type used for the device tree is checked. */
#include <efi_selftest.h>
#define EFI_ST_NUM_PAGES 8
+static const efi_guid_t fdt_guid = EFI_FDT_GUID; static struct efi_boot_services *boottime; +static u64 fdt_addr;
/** * setup() - setup unit test @@ -24,8 +28,20 @@ static struct efi_boot_services *boottime; static int setup(const efi_handle_t handle, const struct efi_system_table *systable) { + size_t i; + boottime = systable->boottime;
+ for (i = 0; i < systable->nr_tables; ++i) { + if (!efi_st_memcmp(&systable->tables[i].guid, &fdt_guid, + sizeof(efi_guid_t))) { + if (fdt_addr) { + efi_st_error("Duplicate device tree\n"); + return EFI_ST_FAILURE; + } + fdt_addr = (uintptr_t)systable->tables[i].table; + } + } return EFI_ST_SUCCESS; }
@@ -152,6 +168,14 @@ static int execute(void) return EFI_ST_FAILURE; }
+ /* Check memory reservation for the device tree */ + if (fdt_addr && + find_in_memory_map(map_size, memory_map, desc_size, fdt_addr, + EFI_RUNTIME_SERVICES_DATA) != EFI_ST_SUCCESS) { + efi_st_error + ("Device tree not marked as runtime services data\n"); + return EFI_ST_FAILURE; + } return EFI_ST_SUCCESS; }
diff --git a/test/py/tests/test_efi_selftest.py b/test/py/tests/test_efi_selftest.py index e0833ffe22c..36b35ee536b 100644 --- a/test/py/tests/test_efi_selftest.py +++ b/test/py/tests/test_efi_selftest.py @@ -8,12 +8,14 @@ import u_boot_utils
@pytest.mark.buildconfigspec('cmd_bootefi_selftest') def test_efi_selftest(u_boot_console): - """ - Run bootefi selftest - """ + """Test the UEFI implementation + + :param u_boot_console: U-Boot console
+ This function executes all selftests that are not marked as on request. + """ u_boot_console.run_command(cmd='setenv efi_selftest') - u_boot_console.run_command(cmd='bootefi selftest', wait_for_prompt=False) + u_boot_console.run_command(cmd='bootefi selftest ${fdtcontroladdr}', wait_for_prompt=False) m = u_boot_console.p.expect(['Summary: 0 failures', 'Press any key']) if m != 0: raise Exception('Failures occurred during the EFI selftest')
Hi Heinrich,
On Sun, 18 Nov 2018 at 09:59, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
The Sandbox uses an address space that is neither the physical nor the virtual address space of the operating system.
In the EFI subsystem we should only use addresses that can be used by an EFI application. With the patch series we eliminate the Sandbox addresses from the EFI memory map.
In the flattened device tree the Sandbox needs its own address space. So make sure that this is always used here.
v3: merge two preexisting patch series resequence the patches to avoid changes to be undone later fix the value of fdtcontroladdr on the sandbox rebase patches
Heinrich Schuchardt (9): efi_loader: eliminate sandbox addresses efi_selftest: add test for memory allocation efi_selftest: building sandbox with EFI_SELFTEST efi_loader: macro efi_size_in_pages() fdt: sandbox: correct use of ${fdtcontroladdr} fdt_support: fdt reservations on the sandbox efi_loader: fix memory mapping for sandbox efi_loader: create fdt reservation before copy efi_selftest: check fdt is marked as runtime data
cmd/bootefi.c | 56 ++++---- common/board_r.c | 3 +- common/fdt_support.c | 3 +- include/efi_loader.h | 11 +- lib/efi_loader/efi_memory.c | 15 +- lib/efi_selftest/Kconfig | 2 +- lib/efi_selftest/Makefile | 1 + lib/efi_selftest/efi_selftest_memory.c | 187 +++++++++++++++++++++++++ lib/fdtdec.c | 6 +- test/py/tests/test_efi_selftest.py | 10 +- 10 files changed, 246 insertions(+), 48 deletions(-) create mode 100644 lib/efi_selftest/efi_selftest_memory.c
I'm holding off on looking at this too closely until my refactor series is in. But the approach looks good to me.
Regards, Simon
participants (2)
-
Heinrich Schuchardt -
Simon Glass