[U-Boot] [RFC v2 10/11] efi_loader: rework bootmgr/bootefi using load_image API

AKASHI Takahiro takahiro.akashi at linaro.org
Wed Mar 27 04:40:41 UTC 2019


In the current implementation, bootefi command and EFI boot manager
don't use load_image API, instead, use more primitive and internal
functions. This will introduce duplicated code and potentially
unknown bugs as well as inconsistent behaviours.

With this patch, do_efibootmgr() and do_boot_efi() are completely
overhauled and re-implemented using load_image API.

Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
---
 cmd/bootefi.c                 | 199 +++++++++++++++++++---------------
 include/efi_loader.h          |   5 +-
 lib/efi_loader/efi_bootmgr.c  |  43 ++++----
 lib/efi_loader/efi_boottime.c |   1 +
 4 files changed, 138 insertions(+), 110 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 39a0712af6ad..17dccd718008 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -224,88 +224,43 @@ static efi_status_t efi_install_fdt(const char *fdt_opt)
 /**
  * do_bootefi_exec() - execute EFI binary
  *
- * @efi:		address of the binary
- * @device_path:	path of the device from which the binary was loaded
- * @image_path:		device path of the binary
+ * @handle:		handle of loaded image
  * Return:		status code
  *
  * Load the EFI binary into a newly assigned memory unwinding the relocation
  * information, install the loaded image protocol, and call the binary.
  */
-static efi_status_t do_bootefi_exec(void *efi,
-				    struct efi_device_path *device_path,
-				    struct efi_device_path *image_path)
+static efi_status_t do_bootefi_exec(efi_handle_t handle)
 {
-	efi_handle_t mem_handle = NULL;
-	struct efi_device_path *memdp = NULL;
-	efi_status_t ret;
 	struct efi_loaded_image_obj *image_obj = NULL;
 	struct efi_loaded_image *loaded_image_info = NULL;
-
-	/*
-	 * Special case for efi payload not loaded from disk, such as
-	 * 'bootefi hello' or for example payload loaded directly into
-	 * memory via JTAG, etc:
-	 */
-	if (!device_path && !image_path) {
-		printf("WARNING: using memory device/image path, this may confuse some payloads!\n");
-		/* actual addresses filled in after efi_load_pe() */
-		memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0);
-		device_path = image_path = memdp;
-		/*
-		 * Grub expects that the device path of the loaded image is
-		 * installed on a handle.
-		 */
-		ret = efi_create_handle(&mem_handle);
-		if (ret != EFI_SUCCESS)
-			return ret; /* TODO: leaks device_path */
-		ret = efi_add_protocol(mem_handle, &efi_guid_device_path,
-				       device_path);
-		if (ret != EFI_SUCCESS)
-			goto err_add_protocol;
-	} else {
-		assert(device_path && image_path);
-	}
-
-	ret = efi_setup_loaded_image(device_path, image_path, &image_obj,
-				     &loaded_image_info);
-	if (ret)
-		goto err_prepare;
+	efi_status_t ret;
 
 	/* Transfer environment variable as load options */
-	set_load_options(loaded_image_info, "bootargs");
-
-	/* Load the EFI payload */
-	ret = efi_load_pe(image_obj, efi, loaded_image_info);
+	ret = EFI_CALL(systab.boottime->open_protocol(
+					handle,
+					&efi_guid_loaded_image,
+					(void **)&loaded_image_info,
+					NULL, NULL,
+					EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL));
 	if (ret != EFI_SUCCESS)
-		goto err_prepare;
-
-	if (memdp) {
-		struct efi_device_path_memory *mdp = (void *)memdp;
-		mdp->memory_type = loaded_image_info->image_code_type;
-		mdp->start_address = (uintptr_t)loaded_image_info->image_base;
-		mdp->end_address = mdp->start_address +
-				loaded_image_info->image_size;
-	}
+		goto err;
+	set_load_options(loaded_image_info, "bootargs");
 
 	/* we don't support much: */
 	env_set("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported",
 		"{ro,boot}(blob)0000000000000000");
 
 	/* Call our payload! */
+	image_obj = container_of(handle, struct efi_loaded_image_obj, header);
 	debug("%s: Jumping to 0x%p\n", __func__, image_obj->entry);
-	ret = EFI_CALL(efi_start_image(&image_obj->header, NULL, NULL));
+	ret = EFI_CALL(efi_start_image(handle, NULL, NULL));
 
-err_prepare:
-	/* image has returned, loaded-image obj goes *poof*: */
 	efi_restore_gd();
 	free(loaded_image_info->load_options);
-	efi_delete_handle(&image_obj->header);
-
-err_add_protocol:
-	if (mem_handle)
-		efi_delete_handle(mem_handle);
+	efi_free_pool(loaded_image_info);
 
+err:
 	return ret;
 }
 
@@ -319,8 +274,7 @@ err_add_protocol:
  */
 static int do_efibootmgr(const char *fdt_opt)
 {
-	struct efi_device_path *device_path, *file_path;
-	void *addr;
+	efi_handle_t handle;
 	efi_status_t ret;
 
 	/* Allow unaligned memory access */
@@ -340,19 +294,21 @@ static int do_efibootmgr(const char *fdt_opt)
 	if (ret != EFI_SUCCESS)
 		return CMD_RET_FAILURE;
 
-	addr = efi_bootmgr_load(&device_path, &file_path);
-	if (!addr)
-		return 1;
+	ret = efi_bootmgr_load(&handle);
+	if (ret != EFI_SUCCESS) {
+		printf("EFI boot manager: Cannot load any image\n");
+		return CMD_RET_FAILURE;
+	}
 
-	printf("## Starting EFI application at %p ...\n", addr);
-	ret = do_bootefi_exec(addr, device_path, file_path);
-	printf("## Application terminated, r = %lu\n",
-	       ret & ~EFI_ERROR_MASK);
+	ret = do_bootefi_exec(handle);
+	printf("## Application terminated, r = %lu\n", ret & ~EFI_ERROR_MASK);
+
+	EFI_CALL(efi_unload_image(handle));
 
 	if (ret != EFI_SUCCESS)
-		return 1;
+		return CMD_RET_FAILURE;
 
-	return 0;
+	return CMD_RET_SUCCESS;
 }
 
 /*
@@ -367,10 +323,14 @@ static int do_efibootmgr(const char *fdt_opt)
  */
 static int do_boot_efi(const char *image_opt, const char *fdt_opt)
 {
-	unsigned long addr;
-	char *saddr;
+	void *image_buf;
+	struct efi_device_path *device_path, *image_path;
+	struct efi_device_path *memdp = NULL, *file_path = NULL;
+	const char *saddr;
+	unsigned long addr, size;
+	const char *size_str;
+	efi_handle_t mem_handle = NULL, handle;
 	efi_status_t ret;
-	unsigned long fdt_addr;
 
 	/* Allow unaligned memory access */
 	allow_unaligned();
@@ -390,36 +350,96 @@ static int do_boot_efi(const char *image_opt, const char *fdt_opt)
 		return CMD_RET_FAILURE;
 
 #ifdef CONFIG_CMD_BOOTEFI_HELLO
-	if (!strcmp(argv[1], "hello")) {
-		ulong size = __efi_helloworld_end - __efi_helloworld_begin;
-
+	if (!strcmp(image_opt, "hello")) {
 		saddr = env_get("loadaddr");
+		size = __efi_helloworld_end - __efi_helloworld_begin;
+
 		if (saddr)
 			addr = simple_strtoul(saddr, NULL, 16);
 		else
 			addr = CONFIG_SYS_LOAD_ADDR;
-		memcpy(map_sysmem(addr, size), __efi_helloworld_begin, size);
+
+		image_buf = map_sysmem(addr, size);
+		memcpy(image_buf, __efi_helloworld_begin, size);
+
+		device_path = NULL;
+		image_path = NULL;
 	} else
 #endif
 	{
-		saddr = argv[1];
+		saddr = image_opt;
+		size_str = env_get("filesize");
+		if (size_str)
+			size = simple_strtoul(size_str, NULL, 16);
+		else
+			size = 0;
 
 		addr = simple_strtoul(saddr, NULL, 16);
 		/* Check that a numeric value was passed */
 		if (!addr && *saddr != '0')
 			return CMD_RET_USAGE;
+
+		image_buf = map_sysmem(addr, size);
+
+		device_path = bootefi_device_path;
+		image_path = bootefi_image_path;
+	}
+
+	if (!device_path && !image_path) {
+		/*
+		 * Special case for efi payload not loaded from disk,
+		 * such as 'bootefi hello' or for example payload
+		 * loaded directly into memory via JTAG, etc:
+		 */
+		printf("WARNING: using memory device/image path, this may confuse some payloads!\n");
+
+		/* actual addresses filled in after efi_load_image() */
+		memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
+					(uint64_t)image_buf, size);
+		file_path = memdp; /* append(memdp, NULL) */
+
+		/*
+		 * Make sure that device for device_path exist
+		 * in load_image(). Otherwise, shell and grub will fail.
+		 */
+		ret = efi_create_handle(&mem_handle);
+		if (ret != EFI_SUCCESS)
+			/* TODO: leaks device_path */
+			goto err_add_protocol;
+
+		ret = efi_add_protocol(mem_handle, &efi_guid_device_path,
+				       memdp);
+		if (ret != EFI_SUCCESS)
+			goto err_add_protocol;
+	} else {
+		assert(device_path && image_path);
+		file_path = efi_dp_append(device_path, image_path);
 	}
 
+	ret = EFI_CALL(efi_load_image(false, (void *)0x1 /* FIXME */,
+				      file_path, image_buf, size, &handle));
+	if (ret != EFI_SUCCESS)
+		goto err_prepare;
+
 	printf("## Starting EFI application at %08lx ...\n", addr);
-	ret = do_bootefi_exec(map_sysmem(addr, 0), bootefi_device_path,
-			      bootefi_image_path);
-	printf("## Application terminated, r = %lu\n",
-	       ret & ~EFI_ERROR_MASK);
+	ret = do_bootefi_exec(handle);
+	printf("## Application terminated, r = %lu\n", ret & ~EFI_ERROR_MASK);
+
+	EFI_CALL(efi_unload_image(handle));
+err_prepare:
+	if (device_path)
+		efi_free_pool(file_path);
+
+err_add_protocol:
+	if (mem_handle)
+		efi_delete_handle(mem_handle);
+	if (memdp)
+		efi_free_pool(memdp);
 
 	if (ret != EFI_SUCCESS)
 		return CMD_RET_FAILURE;
-	else
-		return CMD_RET_SUCCESS;
+
+	return CMD_RET_SUCCESS;
 }
 
 #ifdef CONFIG_CMD_BOOTEFI_SELFTEST
@@ -577,7 +597,7 @@ static char bootefi_help_text[] =
 	"    Use environment variable efi_selftest to select a single test.\n"
 	"    Use 'setenv efi_selftest list' to enumerate all tests.\n"
 #endif
-	"bootefi bootmgr [fdt addr]\n"
+	"bootefi bootmgr [fdt address]\n"
 	"  - load and boot EFI payload based on BootOrder/BootXXXX variables.\n"
 	"\n"
 	"    If specified, the device tree located at <fdt address> gets\n"
@@ -602,6 +622,13 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path)
 	ret = efi_dp_from_name(dev, devnr, path, &device, &image);
 	if (ret == EFI_SUCCESS) {
 		bootefi_device_path = device;
+		if (image) {
+			/* FIXME: image should not contain device */
+			struct efi_device_path *image_tmp = image;
+
+			efi_dp_split_file_path(image, &device, &image);
+			efi_free_pool(image_tmp);
+		}
 		bootefi_image_path = image;
 	} else {
 		bootefi_device_path = NULL;
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 6dfa2fef3cf0..6c09a7f40d02 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -410,8 +410,6 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path,
 				    struct efi_device_path *file_path,
 				    struct efi_loaded_image_obj **handle_ptr,
 				    struct efi_loaded_image **info_ptr);
-efi_status_t efi_load_image_from_path(struct efi_device_path *file_path,
-				      void **buffer, efi_uintn_t *size);
 /* Print information about all loaded images */
 void efi_print_image_infos(void *pc);
 
@@ -565,8 +563,7 @@ struct efi_load_option {
 
 void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
 unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
-void *efi_bootmgr_load(struct efi_device_path **device_path,
-		       struct efi_device_path **file_path);
+efi_status_t efi_bootmgr_load(efi_handle_t *handle);
 
 #else /* CONFIG_IS_ENABLED(EFI_LOADER) */
 
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index 4fccadc5483d..13ec79b2098b 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -120,14 +120,14 @@ static void *get_var(u16 *name, const efi_guid_t *vendor,
  * if successful.  This checks that the EFI_LOAD_OPTION is active (enabled)
  * and that the specified file to boot exists.
  */
-static void *try_load_entry(uint16_t n, struct efi_device_path **device_path,
-			    struct efi_device_path **file_path)
+static efi_status_t try_load_entry(u16 n, efi_handle_t *handle)
 {
 	struct efi_load_option lo;
 	u16 varname[] = L"Boot0000";
 	u16 hexmap[] = L"0123456789ABCDEF";
-	void *load_option, *image = NULL;
+	void *load_option;
 	efi_uintn_t size;
+	efi_status_t ret;
 
 	varname[4] = hexmap[(n & 0xf000) >> 12];
 	varname[5] = hexmap[(n & 0x0f00) >> 8];
@@ -136,19 +136,19 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path,
 
 	load_option = get_var(varname, &efi_global_variable_guid, &size);
 	if (!load_option)
-		return NULL;
+		return EFI_LOAD_ERROR;
 
 	efi_deserialize_load_option(&lo, load_option);
 
 	if (lo.attributes & LOAD_OPTION_ACTIVE) {
 		u32 attributes;
-		efi_status_t ret;
 
 		debug("%s: trying to load \"%ls\" from %pD\n",
 		      __func__, lo.label, lo.file_path);
 
-		ret = efi_load_image_from_path(lo.file_path, &image, &size);
-
+		ret = EFI_CALL(efi_load_image(false, (void *)0x1 /* FIXME */,
+					      lo.file_path, NULL, 0,
+					      handle));
 		if (ret != EFI_SUCCESS)
 			goto error;
 
@@ -159,17 +159,22 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path,
 				L"BootCurrent",
 				(efi_guid_t *)&efi_global_variable_guid,
 				attributes, size, &n));
-		if (ret != EFI_SUCCESS)
+		if (ret != EFI_SUCCESS) {
+			if (EFI_CALL(efi_unload_image(*handle))
+			    != EFI_SUCCESS)
+				printf("Unloading image failed\n");
 			goto error;
+		}
 
 		printf("Booting: %ls\n", lo.label);
-		efi_dp_split_file_path(lo.file_path, device_path, file_path);
+	} else {
+		ret = EFI_LOAD_ERROR;
 	}
 
 error:
 	free(load_option);
 
-	return image;
+	return ret;
 }
 
 /*
@@ -177,12 +182,10 @@ error:
  * EFI variable, the available load-options, finding and returning
  * the first one that can be loaded successfully.
  */
-void *efi_bootmgr_load(struct efi_device_path **device_path,
-		       struct efi_device_path **file_path)
+efi_status_t efi_bootmgr_load(efi_handle_t *handle)
 {
 	u16 bootnext, *bootorder;
 	efi_uintn_t size;
-	void *image = NULL;
 	int i, num;
 	efi_status_t ret;
 
@@ -209,10 +212,9 @@ void *efi_bootmgr_load(struct efi_device_path **device_path,
 		/* load BootNext */
 		if (ret == EFI_SUCCESS) {
 			if (size == sizeof(u16)) {
-				image = try_load_entry(bootnext, device_path,
-						       file_path);
-				if (image)
-					return image;
+				ret = try_load_entry(bootnext, handle);
+				if (ret == EFI_SUCCESS)
+					return ret;
 			}
 		} else {
 			printf("Deleting BootNext failed\n");
@@ -223,19 +225,20 @@ void *efi_bootmgr_load(struct efi_device_path **device_path,
 	bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size);
 	if (!bootorder) {
 		printf("BootOrder not defined\n");
+		ret = EFI_NOT_FOUND;
 		goto error;
 	}
 
 	num = size / sizeof(uint16_t);
 	for (i = 0; i < num; i++) {
 		debug("%s: trying to load Boot%04X\n", __func__, bootorder[i]);
-		image = try_load_entry(bootorder[i], device_path, file_path);
-		if (image)
+		ret = try_load_entry(bootorder[i], handle);
+		if (ret == EFI_SUCCESS)
 			break;
 	}
 
 	free(bootorder);
 
 error:
-	return image;
+	return ret;
 }
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 74da6b01054a..8e2fa0111591 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1610,6 +1610,7 @@ failure:
  * @size:	size of the loaded image
  * Return:	status code
  */
+static
 efi_status_t efi_load_image_from_path(struct efi_device_path *file_path,
 				      void **buffer, efi_uintn_t *size)
 {
-- 
2.20.1



More information about the U-Boot mailing list