From 5cd28e17600458ee99e69a86856f2b7e8eddd0c0 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 3 Oct 2020 12:44:31 +0200 Subject: [PATCH 1/9] efi_loader: description EFI_LOAD_FILE2_PROTOCOL U-Boot offers a EFI_LOAD_FILE2_PROTOCOL which the Linux EFI stub can use to load an initial RAM disk. Update the function comments of the implementation. Signed-off-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas --- lib/efi_loader/efi_load_initrd.c | 42 +++++++++++++++++--------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c index 574a83d7e3..ff69e6eb79 100644 --- a/lib/efi_loader/efi_load_initrd.c +++ b/lib/efi_loader/efi_load_initrd.c @@ -47,9 +47,9 @@ static const struct efi_initrd_dp dp = { /** * get_file_size() - retrieve the size of initramfs, set efi status on error * - * @dev: device to read from. i.e "mmc" - * @part: device partition. i.e "0:1" - * @file: name fo file + * @dev: device to read from, e.g. "mmc" + * @part: device partition, e.g. "0:1" + * @file: name of file * @status: EFI exit code in case of failure * * Return: size of file @@ -78,15 +78,16 @@ out: } /** - * load_file2() - get information about random number generation + * efi_load_file2initrd() - load initial RAM disk + * + * This function implements the LoadFile service of the EFI_LOAD_FILE2_PROTOCOL + * in order to load an initial RAM disk requested by the Linux kernel stub. * - * This function implement the LoadFile2() service in order to load an initram - * disk requested by the Linux kernel stub. * See the UEFI spec for details. * - * @this: loadfile2 protocol instance - * @file_path: relative path of the file. "" in this case - * @boot_policy: must be false for Loadfile2 + * @this: EFI_LOAD_FILE2_PROTOCOL instance + * @file_path: media device path of the file, "" in this case + * @boot_policy: must be false * @buffer_size: size of allocated buffer * @buffer: buffer to load the file * @@ -128,7 +129,13 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this, goto out; } - /* expect something like 'mmc 0:1 initrd.cpio.gz' */ + /* + * expect a string with three space separated parts: + * + * * a block device type, e.g. "mmc" + * * a device and partition identifier, e.g. "0:1" + * * a file path on the block device, e.g. "/boot/initrd.cpio.gz" + */ dev = strsep(&s, " "); if (!dev) goto out; @@ -168,23 +175,20 @@ out: } /** - * efi_initrd_register() - Register a handle and loadfile2 protocol + * efi_initrd_register() - create handle for loading initial RAM disk * - * This function creates a new handle and installs a linux specific GUID - * to handle initram disk loading during boot. - * See the UEFI spec for details. + * This function creates a new handle and installs a Linux specific vendor + * device path and an EFI_LOAD_FILE_2_PROTOCOL. Linux uses the device path + * to identify the handle and then calls the LoadFile service of the + * EFI_LOAD_FILE_2_PROTOCOL to read the initial RAM disk. * - * Return: status code + * Return: status code */ efi_status_t efi_initrd_register(void) { efi_handle_t efi_initrd_handle = NULL; efi_status_t ret; - /* - * Set up the handle with the EFI_LOAD_FILE2_PROTOCOL which Linux may - * use to load the initial ramdisk. - */ ret = EFI_CALL(efi_install_multiple_protocol_interfaces (&efi_initrd_handle, /* initramfs */ From e2aff337ed53ada0abdbc38d0afa8d6f209bf4ab Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 3 Oct 2020 12:50:52 +0200 Subject: [PATCH 2/9] efi_loader: illegal free in EFI_LOAD_FILE2_PROTOCOL strsep() changes the address that its first argument points to. We cannot use the changed address as argument of free(). Signed-off-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas --- lib/efi_loader/efi_load_initrd.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c index ff69e6eb79..d517d686c3 100644 --- a/lib/efi_loader/efi_load_initrd.c +++ b/lib/efi_loader/efi_load_initrd.c @@ -98,19 +98,20 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this, struct efi_device_path *file_path, bool boot_policy, efi_uintn_t *buffer_size, void *buffer) { - const char *filespec = CONFIG_EFI_INITRD_FILESPEC; + char *filespec; efi_status_t status = EFI_NOT_FOUND; loff_t file_sz = 0, read_sz = 0; char *dev, *part, *file; - char *s; + char *pos; int ret; EFI_ENTRY("%p, %p, %d, %p, %p", this, file_path, boot_policy, buffer_size, buffer); - s = strdup(filespec); - if (!s) + filespec = strdup(CONFIG_EFI_INITRD_FILESPEC); + if (!filespec) goto out; + pos = filespec; if (!this || this != &efi_lf2_protocol || !buffer_size) { @@ -136,13 +137,13 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this, * * a device and partition identifier, e.g. "0:1" * * a file path on the block device, e.g. "/boot/initrd.cpio.gz" */ - dev = strsep(&s, " "); + dev = strsep(&pos, " "); if (!dev) goto out; - part = strsep(&s, " "); + part = strsep(&pos, " "); if (!part) goto out; - file = strsep(&s, " "); + file = strsep(&pos, " "); if (!file) goto out; @@ -170,7 +171,7 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this, } out: - free(s); + free(filespec); return EFI_EXIT(status); } From 9487683821e59d0156b8f0697d669ef62534a867 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 3 Oct 2020 13:12:03 +0200 Subject: [PATCH 3/9] efi_selftest: enable printing hexadecimal numbers Add code to use %x in efi_st_print(). Signed-off-by: Heinrich Schuchardt --- lib/efi_selftest/efi_selftest_console.c | 35 ++++++++++++++++--------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/lib/efi_selftest/efi_selftest_console.c b/lib/efi_selftest/efi_selftest_console.c index 13f3ee6bc1..0219bd70e0 100644 --- a/lib/efi_selftest/efi_selftest_console.c +++ b/lib/efi_selftest/efi_selftest_console.c @@ -44,25 +44,28 @@ static void mac(void *pointer, u16 **buf) } /* - * Print a pointer to an u16 string + * printx() - print hexadecimal number to an u16 string * - * @pointer: pointer - * @buf: pointer to buffer address - * on return position of terminating zero word + * @pointer: pointer + * @prec: minimum number of digits to print + * @buf: pointer to buffer address, + * on return position of terminating zero word + * @size: size of value to be printed in bytes */ -static void pointer(void *pointer, u16 **buf) +static void printx(u64 p, int prec, u16 **buf) { int i; u16 c; - uintptr_t p = (uintptr_t)pointer; u16 *pos = *buf; - for (i = 8 * sizeof(p) - 4; i >= 0; i -= 4) { - c = (p >> i) & 0x0f; - c += '0'; - if (c > '9') - c += 'a' - '9' - 1; - *pos++ = c; + for (i = 2 * sizeof(p) - 1; i >= 0; --i) { + c = (p >> (4 * i)) & 0x0f; + if (c || pos != *buf || !i || i < prec) { + c += '0'; + if (c > '9') + c += 'a' - '9' - 1; + *pos++ = c; + } } *pos = 0; *buf = pos; @@ -212,7 +215,9 @@ void efi_st_printc(int color, const char *fmt, ...) break; default: --c; - pointer(va_arg(args, void*), &pos); + printx((uintptr_t)va_arg(args, void *), + 2 * sizeof(void *), &pos); + break; } break; case 's': @@ -223,6 +228,10 @@ void efi_st_printc(int color, const char *fmt, ...) case 'u': uint2dec(va_arg(args, u32), prec, &pos); break; + case 'x': + printx((u64)va_arg(args, unsigned int), + prec, &pos); + break; default: break; } From dc374ab08f7432933083ea9a95a90d7f29a8852e Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 3 Oct 2020 13:43:45 +0200 Subject: [PATCH 4/9] efi_selftest: print CRC32 of initrd as hexadecimal Print the CRC32 loaded via the EFI_LOAD_FILE2_PROTOCOL as a hexadecimal number. Signed-off-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas --- lib/efi_selftest/efi_selftest_load_initrd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/efi_selftest/efi_selftest_load_initrd.c b/lib/efi_selftest/efi_selftest_load_initrd.c index e16163caca..fe060a6644 100644 --- a/lib/efi_selftest/efi_selftest_load_initrd.c +++ b/lib/efi_selftest/efi_selftest_load_initrd.c @@ -200,7 +200,7 @@ static int execute(void) efi_st_error("Could not determine CRC32\n"); return EFI_ST_FAILURE; } - efi_st_printf("CRC32 %u\n", (unsigned int)crc32); + efi_st_printf("CRC32 %.8x\n", (unsigned int)crc32); status = boottime->free_pool(buf); if (status != EFI_SUCCESS) { From eb0d1d83994a1e54d6720a6fc8eb1fd3ce2e56a9 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 30 Sep 2020 21:52:09 +0200 Subject: [PATCH 5/9] efi_selftest: avoid unnecessary reset When we do not execute a test requiring ExitBootServices do not reset the system after testing. Signed-off-by: Heinrich Schuchardt --- lib/efi_selftest/efi_selftest.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/lib/efi_selftest/efi_selftest.c b/lib/efi_selftest/efi_selftest.c index 165fa265f2..85e819bdfa 100644 --- a/lib/efi_selftest/efi_selftest.c +++ b/lib/efi_selftest/efi_selftest.c @@ -142,6 +142,27 @@ static int teardown(struct efi_unit_test *test, unsigned int *failures) return ret; } +/* + * Check that a test requiring reset exists. + * + * @testname: name of the test + * @return: test, or NULL if not found + */ +static bool need_reset(const u16 *testname) +{ + struct efi_unit_test *test; + + for (test = ll_entry_start(struct efi_unit_test, efi_unit_test); + test < ll_entry_end(struct efi_unit_test, efi_unit_test); ++test) { + if (testname && efi_st_strcmp_16_8(testname, test->name)) + continue; + if (test->phase == EFI_SETUP_BEFORE_BOOTTIME_EXIT || + test->phase == EFI_SETUP_AFTER_BOOTTIME_EXIT) + return true; + } + return false; +} + /* * Check that a test exists. * @@ -290,6 +311,16 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle, EFI_ST_SETUP | EFI_ST_EXECUTE | EFI_ST_TEARDOWN, &failures); + if (!need_reset(testname)) { + if (failures) + ret = EFI_PROTOCOL_ERROR; + + /* Give feedback */ + efi_st_printc(EFI_WHITE, "\nSummary: %u failures\n\n", + failures); + return ret; + } + /* Execute mixed tests */ efi_st_do_tests(testname, EFI_SETUP_BEFORE_BOOTTIME_EXIT, EFI_ST_SETUP, &failures); From f3866909e35074ea6f50226d40487a180de1132f Mon Sep 17 00:00:00 2001 From: Michael Walle Date: Tue, 29 Sep 2020 08:54:48 +0200 Subject: [PATCH 6/9] distro_bootcmd: call EFI bootmgr even without having /EFI/boot Currently, the EFI bootmgr is only called if there is a EFI binary inside the path for removable media is found, i.e. /EFI/boot/. This doesn't make sense. It is the duty of the bootmgr to find out the path and name of the EFI binary to boot. It should be called even if there is no /EFI/boot directory. Thus, call the bootmgr before we try to boot the EFI binary inside the removable media path. Also remove the ${fdtcontroladdr} parameter because the fallback is handled in cmd/bootefi.c and that already takes care of correct settings if the board has ACPI and thus no device tree at all. Signed-off-by: Michael Walle Reviewed-by: Heinrich Schuchardt --- include/config_distro_bootcmd.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index fc0935fa21..ff29ef5a90 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -123,12 +123,14 @@ #define BOOTENV_SHARED_EFI \ - "boot_efi_binary=" \ + "boot_efi_bootmgr=" \ "if fdt addr ${fdt_addr_r}; then " \ "bootefi bootmgr ${fdt_addr_r};" \ "else " \ - "bootefi bootmgr ${fdtcontroladdr};" \ - "fi;" \ + "bootefi bootmgr;" \ + "fi\0" \ + \ + "boot_efi_binary=" \ "load ${devtype} ${devnum}:${distro_bootpart} " \ "${kernel_addr_r} efi/boot/"BOOTEFI_NAME"; " \ "if fdt addr ${fdt_addr_r}; then " \ @@ -152,6 +154,7 @@ "run load_efi_dtb; " \ "fi;" \ "done;" \ + "run boot_efi_bootmgr;" \ "if test -e ${devtype} ${devnum}:${distro_bootpart} " \ "efi/boot/"BOOTEFI_NAME"; then " \ "echo Found EFI removable media binary " \ From 493a37fe8607bcbf4cb7c4a5c18fed5a86f9e5d2 Mon Sep 17 00:00:00 2001 From: Sean Anderson Date: Mon, 28 Sep 2020 12:08:37 -0400 Subject: [PATCH 7/9] efi: Fix typo in documentation There is an extra space. Signed-off-by: Sean Anderson Reviewed-by: Heinrich Schuchardt --- lib/efi_selftest/efi_selftest_set_virtual_address_map.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/efi_selftest/efi_selftest_set_virtual_address_map.c b/lib/efi_selftest/efi_selftest_set_virtual_address_map.c index a4e5a50f63..b097a81136 100644 --- a/lib/efi_selftest/efi_selftest_set_virtual_address_map.c +++ b/lib/efi_selftest/efi_selftest_set_virtual_address_map.c @@ -23,7 +23,7 @@ static u32 notify_call_count; static bool convert_pointer_failed; /** - * notify () - notification function + * notify() - notification function * * This function is called when the EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE event * occurs. The correct output of ConvertPointer() is checked. From 4b71f6dc4e2a6d98f24120dfc2f88d37c37d35e8 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 17 Sep 2020 19:09:23 +0200 Subject: [PATCH 8/9] efi_loader: QEMU CONFIG_EFI_GRUB_ARM32_WORKAROUND=n CONFIG_EFI_GRUB_ARM32 is only needed for architectures with caches that are not managed via CP15 (or for some outdated buggy versions of GRUB). It makes more sense to disable the setting per architecture than per defconfig. Move QEMU's CONFIG_EFI_GRUB_ARM32_WORKAROUND=n from defconfig to Kconfig. Signed-off-by: Heinrich Schuchardt --- configs/qemu_arm_defconfig | 1 - lib/efi_loader/Kconfig | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig index 1ffa727e63..278d8f41f4 100644 --- a/configs/qemu_arm_defconfig +++ b/configs/qemu_arm_defconfig @@ -56,4 +56,3 @@ CONFIG_USB=y CONFIG_DM_USB=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_PCI=y -# CONFIG_EFI_GRUB_ARM32_WORKAROUND is not set diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index bad1a29ba8..ab42f3ba75 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -164,6 +164,7 @@ config EFI_HAVE_RUNTIME_RESET config EFI_GRUB_ARM32_WORKAROUND bool "Workaround for GRUB on 32bit ARM" + default n if ARCH_QEMU default y depends on ARM && !ARM64 help From 4cbb2930bd8c67f40f848528941930cf4c2a1841 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 27 Aug 2020 12:52:20 +0200 Subject: [PATCH 9/9] efi_loader: consider no-map property of reserved memory The device tree may contain a /reserved-memory node. The no-map property of the sub-nodes signals if the memory may be accessed by the UEFI payload or not. In the EBBR specification (https://github.com/arm-software/ebbr) the modeling of the reserved memory has been clarified. If a reserved memory node in the device tree has the no-map property map, create a EfiReservedMemoryType memory map entry else use EfiBootServicesData. Signed-off-by: Heinrich Schuchardt --- cmd/bootefi.c | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 40d5ef2b3a..fdf909f8da 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -135,12 +135,29 @@ done: return ret; } -static void efi_reserve_memory(u64 addr, u64 size) +/** + * efi_reserve_memory() - add reserved memory to memory map + * + * @addr: start address of the reserved memory range + * @size: size of the reserved memory range + * @nomap: indicates that the memory range shall not be accessed by the + * UEFI payload + */ +static void efi_reserve_memory(u64 addr, u64 size, bool nomap) { + int type; + efi_uintn_t ret; + /* Convert from sandbox address space. */ addr = (uintptr_t)map_sysmem(addr, 0); - if (efi_add_memory_map(addr, size, - EFI_RESERVED_MEMORY_TYPE) != EFI_SUCCESS) + + if (nomap) + type = EFI_RESERVED_MEMORY_TYPE; + else + type = EFI_BOOT_SERVICES_DATA; + + ret = efi_add_memory_map(addr, size, type); + if (ret != EFI_SUCCESS) log_err("Reserved memory mapping failed addr %llx size %llx\n", addr, size); } @@ -166,7 +183,7 @@ static void efi_carve_out_dt_rsv(void *fdt) for (i = 0; i < nr_rsv; i++) { if (fdt_get_mem_rsv(fdt, i, &addr, &size) != 0) continue; - efi_reserve_memory(addr, size); + efi_reserve_memory(addr, size, false); } /* process reserved-memory */ @@ -186,8 +203,13 @@ static void efi_carve_out_dt_rsv(void *fdt) * a size instead of a reg property. */ if (fdt_addr != FDT_ADDR_T_NONE && - fdtdec_get_is_enabled(fdt, subnode)) - efi_reserve_memory(fdt_addr, fdt_size); + fdtdec_get_is_enabled(fdt, subnode)) { + bool nomap; + + nomap = !!fdt_getprop(fdt, subnode, "no-map", + NULL); + efi_reserve_memory(fdt_addr, fdt_size, nomap); + } subnode = fdt_next_subnode(fdt, subnode); } }