efi_loader: Simplify efi_dp_from_mem()

This function should take a pointer, not an address. Update it along
with all users.

Series-to: u-boot
Series-cc: ilias, heinrich
Series-version: 5
Series-changes: 2
- Drop patch 'Convert efi_get_memory_map() to return pointers'
- Drop patch 'efi_loader: Make more use of ulong'
- Significantly expand and redirect the series

Series-changes: 3
- Add comment to struct efi_device_path_memory
- Use a pointer for the values in struct efi_device_path_memory

Series-changes: 5
- Drop the enum / mem_type patch as it is not needed

Series-links: 434801 3:434576 1:434151
Cover-letter:
efi: Tidy up confusion between pointers and addresses
The EFI-loader implementation converts things back and forth between
addresses and pointers, with not much consistency in how this is done.

Within most of U-Boot a pointer is a void * and an address is a ulong

This convention is very helpful, since it is obvious in common code as
to whether you need to call map_sysmem() and friends, or not.

As part of cleaning up the EFI memory-management, I found it almost
impossible to know in some cases whether something is an address or a
pointer. I decided to give up on that and come back to it when this is
resolved.

This series starts applying the normal ulong/void * convention to the
EFI_loader code, so making things easier to follow. For now, u64 is
often used instead of ulong, but the effect is the same.

The main changes are:
- Rather than using the external struct efi_mem_desc data-structure for
  internal bookkeeping, create a new one, so it can have different
  semantics
- Within efi_memory.c addresses are used, rather than addresses
  masquerading as pointers. The conversions are done in efi_boottime

Unforunately my foray into attempting to use enum for the memory type
failed. The problem is not just that the external interface cannot use
an enum. In fact the enum in the spec is really just confusing, since
values not mentioned in the enum are valid. While we could handle this
by declaring a few more values in enum efi_memory_type, it doesn't seem
worth it. For example, if we declare 0x6fffffff and -1 as valid values,
we get the correct range, but then we need to be careful about
conversion in efi_boottime. If we declare 0xffffffff as valid, then the
enum ends up being 64-bits wide! Yes, that would work, I suppose it
wouldn't matter, but at that point I believe using an enum is actually
more confusing than not. It also made passing SCT tricky, since invalid
values are passed in...

Link: https://lore.kernel.org/u-boot/20240725135629.3505072-1-sjg@chromium.org/
END

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

boot/bootmeth_efi.c

@@ -52,6 +52,40 @@ static bool bootmeth_uses_network(struct bootflow *bflow)
 	    device_get_uclass_id(media) == UCLASS_ETH;
 }
 
+static void set_efi_bootdev(struct blk_desc *desc, struct bootflow *bflow)
+{
+	const struct udevice *media_dev;
+	int size = bflow->size;
+	const char *dev_name;
+	char devnum_str[9];
+	char dirname[200];
+	char *last_slash;
+
+	/*
+	 * This is a horrible hack to tell EFI about this boot device. Once we
+	 * unify EFI with the rest of U-Boot we can clean this up. The same hack
+	 * exists in multiple places, e.g. in the fs, tftp and load commands.
+	 *
+	 * Once we can clean up the EFI code to make proper use of driver model,
+	 * this can go away.
+	 */
+	media_dev = dev_get_parent(bflow->dev);
+	snprintf(devnum_str, sizeof(devnum_str), "%x:%x",
+		 desc ? desc->devnum : dev_seq(media_dev),
+		 bflow->part);
+
+	strlcpy(dirname, bflow->fname, sizeof(dirname));
+	last_slash = strrchr(dirname, '/');
+	if (last_slash)
+		*last_slash = '\0';
+
+	dev_name = device_get_uclass_id(media_dev) == UCLASS_MASS_STORAGE ?
+		 "usb" : blk_get_uclass_name(device_get_uclass_id(media_dev));
+	log_debug("setting bootdev %s, %s, %s, %p, %x\n",
+		  dev_name, devnum_str, bflow->fname, bflow->buf, size);
+	efi_set_bootdev(dev_name, devnum_str, bflow->fname, bflow->buf, size);
+}
+
 static int efiload_read_file(struct bootflow *bflow, ulong addr)
 {
 	struct blk_desc *desc = NULL;
@@ -68,6 +102,8 @@ static int efiload_read_file(struct bootflow *bflow, ulong addr)
 		return log_msg_ret("rdf", ret);
 	bflow->buf = map_sysmem(addr, bflow->size);
 
+	set_efi_bootdev(desc, bflow);
+
 	return 0;
 }
 
@@ -210,7 +246,6 @@ static int distro_efi_read_bootflow_net(struct bootflow *bflow)
 	if (size <= 0)
 		return log_msg_ret("sz", -EINVAL);
 	bflow->size = size;
-	bflow->buf = map_sysmem(addr, size);
 
 	/* bootfile should be setup by dhcp */
 	bootfile_name = env_get("bootfile");
@@ -220,6 +255,10 @@ static int distro_efi_read_bootflow_net(struct bootflow *bflow)
 	if (!bflow->fname)
 		return log_msg_ret("fi0", -ENOMEM);
 
+	/* do the hideous EFI hack */
+	efi_set_bootdev("Net", "", bflow->fname, map_sysmem(addr, 0),
+			bflow->size);
+
 	/* read the DT file also */
 	fdt_addr_str = env_get("fdt_addr_r");
 	if (!fdt_addr_str)
@@ -293,10 +332,29 @@ static int distro_efi_boot(struct udevice *dev, struct bootflow *bflow)
 		if (bflow->flags & ~BOOTFLOWF_USE_BUILTIN_FDT)
 			fdt = bflow->fdt_addr;
 
+	} else {
+		/*
+		 * This doesn't actually work for network devices:
+		 *
+		 * do_bootefi_image() No UEFI binary known at 0x02080000
+		 *
+		 * But this is the same behaviour for distro boot, so it can be
+		 * fixed here.
+		 */
+		fdt = env_get_hex("fdt_addr_r", 0);
 	}
 
-	if (efi_bootflow_run(bflow))
-		return log_msg_ret("run", -EINVAL);
+	if (bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT) {
+		log_debug("Booting with built-in fdt\n");
+		if (efi_binary_run(map_sysmem(kernel, 0), bflow->size,
+				   EFI_FDT_USE_INTERNAL))
+			return log_msg_ret("run", -EINVAL);
+	} else {
+		log_debug("Booting with external fdt\n");
+		if (efi_binary_run(map_sysmem(kernel, 0), bflow->size,
+				   map_sysmem(fdt, 0)))
+			return log_msg_ret("run", -EINVAL);
+	}
 
 	return 0;
 }

include/efi_loader.h

@@ -19,7 +19,6 @@
 #include <linux/oid_registry.h>
 
 struct blk_desc;
-struct bootflow;
 struct jmp_buf_data;
 
 #if CONFIG_IS_ENABLED(EFI_LOADER)
@@ -545,15 +544,6 @@ efi_status_t efi_install_fdt(void *fdt);
 efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options);
 /* Run loaded UEFI image with given fdt */
 efi_status_t efi_binary_run(void *image, size_t size, void *fdt);
-
-/**
- * efi_bootflow_run() - Run a bootflow containing an EFI application
- *
- * @bootflow: Bootflow to run
- * Return: Status code, something went wrong
- */
-efi_status_t efi_bootflow_run(struct bootflow *bootflow);
-
 /* Initialize variable services */
 efi_status_t efi_init_variables(void);
 /* Notify ExitBootServices() is called */

lib/efi_loader/efi_bootbin.c

@@ -6,16 +6,13 @@
 
 #define LOG_CATEGORY LOGC_EFI
 
-#include <bootflow.h>
 #include <charset.h>
-#include <dm.h>
 #include <efi.h>
 #include <efi_loader.h>
 #include <env.h>
 #include <image.h>
 #include <log.h>
 #include <malloc.h>
-#include <mapmem.h>
 
 static struct efi_device_path *bootefi_image_path;
 static struct efi_device_path *bootefi_device_path;
@@ -47,46 +44,12 @@ void efi_clear_bootdev(void)
 	image_size = 0;
 }
 
-efi_status_t calculate_paths(const char *dev, const char *devnr, const char *path,
-			     struct efi_device_path **device_pathp,
-			     struct efi_device_path **image_pathp)
-
-{
-	struct efi_device_path *image, *device;
-	efi_status_t ret;
-
-	ret = efi_dp_from_name(dev, devnr, path, &device, &image);
-	if (ret != EFI_SUCCESS)
-		return ret;
-
-	*device_pathp = 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);
-	}
-	*image_pathp = image;
-	log_debug("- boot device %pD\n", device);
-	if (image)
-		log_debug("- image %pD\n", image);
-
-	return EFI_SUCCESS;
-}
-
 /**
  * efi_set_bootdev() - set boot device
  *
  * This function is called when a file is loaded, e.g. via the 'load' command.
  * We use the path to this file to inform the UEFI binary about the boot device.
  *
- * For a valid image, it sets:
- *    - image_addr to the provided buffer
- *    - image_size to the provided buffer_size
- *    - bootefi_device_path to the EFI device-path
- *    - bootefi_image_path to the EFI image-path
- *
  * @dev:		device, e.g. "MMC"
  * @devnr:		number of the device, e.g. "1:2"
  * @path:		path to file loaded
@@ -96,6 +59,7 @@ efi_status_t calculate_paths(const char *dev, const char *devnr, const char *pat
 void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
 		     void *buffer, size_t buffer_size)
 {
+	struct efi_device_path *device, *image;
 	efi_status_t ret;
 
 	log_debug("dev=%s, devnr=%s, path=%s, buffer=%p, size=%zx\n", dev,
@@ -129,9 +93,21 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
 	image_addr = buffer;
 	image_size = buffer_size;
 
-	ret = calculate_paths(dev, devnr, path, &bootefi_device_path,
-			      &bootefi_image_path);
-	if (ret) {
+	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;
+		log_debug("- boot device %pD\n", device);
+		if (image)
+			log_debug("- image %pD\n", image);
+	} else {
 		log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
 		efi_clear_bootdev();
 	}
@@ -142,21 +118,41 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
  *
  * @source_buffer:	memory address of the UEFI image
  * @source_size:	size of the UEFI image
- * @device:		EFI device-path
- * @image:		EFI image-path
  * Return:		status code
  */
-static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size,
-				  struct efi_device_path *device,
-				  struct efi_device_path *image)
+efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
 {
-	efi_handle_t handle;
-	struct efi_device_path *msg_path, *file_path;
+	efi_handle_t mem_handle = NULL, handle;
+	struct efi_device_path *file_path = NULL;
+	struct efi_device_path *msg_path;
 	efi_status_t ret;
 	u16 *load_options;
 
-	file_path = efi_dp_concat(device, image, 0);
-	msg_path = image;
+	if (!bootefi_device_path || !bootefi_image_path) {
+		log_debug("Not loaded from disk\n");
+		/*
+		 * Special case for efi payload not loaded from disk,
+		 * such as 'bootefi hello' or for example payload
+		 * loaded directly into memory via JTAG, etc:
+		 */
+		file_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
+					    source_buffer, source_size);
+		/*
+		 * Make sure that device for device_path exist
+		 * in load_image(). Otherwise, shell and grub will fail.
+		 */
+		ret = efi_install_multiple_protocol_interfaces(&mem_handle,
+							       &efi_guid_device_path,
+							       file_path, NULL);
+		if (ret != EFI_SUCCESS)
+			goto out;
+		msg_path = file_path;
+	} else {
+		file_path = efi_dp_concat(bootefi_device_path,
+					  bootefi_image_path, 0);
+		msg_path = bootefi_image_path;
+		log_debug("Loaded from disk\n");
+	}
 
 	log_info("Booting %pD\n", msg_path);
 
@@ -175,31 +171,19 @@ static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size,
 	ret = do_bootefi_exec(handle, load_options);
 
 out:
+	if (mem_handle) {
+		efi_status_t r;
+
+		r = efi_uninstall_multiple_protocol_interfaces(
+			mem_handle, &efi_guid_device_path, file_path, NULL);
+		if (r != EFI_SUCCESS)
+			log_err("Uninstalling protocol interfaces failed\n");
+	}
+	efi_free_pool(file_path);
 
 	return ret;
 }
 
-static efi_status_t efi_binary_run_(void *image_ptr, size_t size, void *fdt,
-				    struct efi_device_path *device,
-				    struct efi_device_path *image)
-{
-	efi_status_t ret;
-
-	/* Initialize EFI drivers */
-	ret = efi_init_obj_list();
-	if (ret != EFI_SUCCESS) {
-		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
-			ret & ~EFI_ERROR_MASK);
-		return -1;
-	}
-
-	ret = efi_install_fdt(fdt);
-	if (ret != EFI_SUCCESS)
-		return ret;
-
-	return efi_run_image(image_ptr, size, device, image);
-}
-
 /**
  * efi_binary_run() - run loaded UEFI image
  *
@@ -214,116 +198,19 @@ static efi_status_t efi_binary_run_(void *image_ptr, size_t size, void *fdt,
  */
 efi_status_t efi_binary_run(void *image, size_t size, void *fdt)
 {
-	efi_handle_t mem_handle = NULL;
-	struct efi_device_path *file_path = NULL;
 	efi_status_t ret;
 
-	if (!bootefi_device_path || !bootefi_image_path) {
-		log_debug("Not loaded from disk\n");
-		/*
-		 * Special case for efi payload not loaded from disk,
-		 * such as 'bootefi hello' or for example payload
-		 * loaded directly into memory via JTAG, etc:
-		 */
-		file_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, image,
-					    size);
-		/*
-		 * Make sure that device for device_path exist
-		 * in load_image(). Otherwise, shell and grub will fail.
-		 */
-		ret = efi_install_multiple_protocol_interfaces(&mem_handle,
-							       &efi_guid_device_path,
-							       file_path, NULL);
-		if (ret != EFI_SUCCESS)
-			goto out;
-	} else {
-		log_debug("Loaded from disk\n");
+	/* Initialize EFI drivers */
+	ret = efi_init_obj_list();
+	if (ret != EFI_SUCCESS) {
+		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+			ret & ~EFI_ERROR_MASK);
+		return -1;
 	}
 
-	ret = efi_binary_run_(image, size, fdt, bootefi_device_path,
-			      bootefi_image_path);
-out:
-	if (mem_handle) {
-		efi_status_t r;
+	ret = efi_install_fdt(fdt);
+	if (ret != EFI_SUCCESS)
+		return ret;
 
-		r = efi_uninstall_multiple_protocol_interfaces(mem_handle,
-					&efi_guid_device_path, file_path, NULL);
-		if (r != EFI_SUCCESS)
-			log_err("Uninstalling protocol interfaces failed\n");
-	}
-	efi_free_pool(file_path);
-
-	return ret;
+	return efi_run_image(image, size);
 }
-
-/**
- * calc_dev_name() - Calculate the device name to give to EFI
- *
- * If not supported, this shows an error.
- *
- * Return name, or NULL if not supported
- */
-static const char *calc_dev_name(struct bootflow *bflow)
-{
-	const struct udevice *media_dev;
-
-	media_dev = dev_get_parent(bflow->dev);
-
-	if (!bflow->blk) {
-		if (device_get_uclass_id(media_dev) == UCLASS_ETH)
-			return "Net";
-
-		log_err("Cannot boot EFI app on media '%s'\n",
-			dev_get_uclass_name(media_dev));
-
-		return NULL;
-	}
-
-	if (device_get_uclass_id(media_dev) == UCLASS_MASS_STORAGE)
-		return "usb";
-
-	return blk_get_uclass_name(device_get_uclass_id(media_dev));
-}
-
-efi_status_t efi_bootflow_run(struct bootflow *bflow)
-{
-	struct efi_device_path *device, *image;
-	const struct udevice *media_dev;
-	struct blk_desc *desc = NULL;
-	const char *dev_name;
-	char devnum_str[9];
-	efi_status_t ret;
-	void *fdt;
-
-	media_dev = dev_get_parent(bflow->dev);
-	if (bflow->blk) {
-		desc = dev_get_uclass_plat(bflow->blk);
-
-		snprintf(devnum_str, sizeof(devnum_str), "%x:%x",
-			 desc ? desc->devnum : dev_seq(media_dev), bflow->part);
-	} else {
-		*devnum_str = '\0';
-	}
-
-	dev_name = calc_dev_name(bflow);
-	log_debug("dev_name '%s' devnum_str '%s' fname '%s' media_dev '%s'\n",
-		  dev_name, devnum_str, bflow->fname, media_dev->name);
-	if (!dev_name)
-		return EFI_UNSUPPORTED;
-	ret = calculate_paths(dev_name, devnum_str, bflow->fname, &device,
-			      &image);
-	if (ret)
-		return EFI_UNSUPPORTED;
-
-	if (bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT) {
-		log_debug("Booting with built-in fdt\n");
-		fdt = EFI_FDT_USE_INTERNAL;
-	} else {
-		log_debug("Booting with external fdt\n");
-		fdt = map_sysmem(bflow->fdt_addr, 0);
-	}
-	ret = efi_binary_run_(bflow->buf, bflow->size, fdt, device, image);
-
-	return ret;
-}
-