Compare commits

...

1 Commits
efih2 ... efil5

Author SHA1 Message Date
Simon Glass
a9331de55b 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>
2024-12-11 06:52:47 -07:00
5 changed files with 21 additions and 13 deletions

View File

@@ -573,6 +573,16 @@ struct efi_mac_addr {
# define DEVICE_PATH_SUB_TYPE_VENDOR 0x04
# define DEVICE_PATH_SUB_TYPE_CONTROLLER 0x05
/**
* struct efi_device_path_memory - 'Memory Mapped Device Path' object
*
* @dp: header for device-path protocol
* @memory_type: see enum efi_memory_type
* @start_address: start address of the memory; note that this is provided to
* the EFI application so must be a pointer cast to u64
* @end_address: end address of the memory; note that this is provided to
* the EFI application so must be a pointer cast to u64
*/
struct efi_device_path_memory {
struct efi_device_path dp;
u32 memory_type;

View File

@@ -918,8 +918,7 @@ struct efi_device_path *efi_dp_part_node(struct blk_desc *desc, int part);
struct efi_device_path *efi_dp_from_file(const struct efi_device_path *dp,
const char *path);
struct efi_device_path *efi_dp_from_eth(void);
struct efi_device_path *efi_dp_from_mem(uint32_t mem_type,
uint64_t start_address,
struct efi_device_path *efi_dp_from_mem(uint32_t mem_type, void *start_ptr,
size_t size);
/* Determine the last device path node that is not the end node. */
const struct efi_device_path *efi_dp_last_node(

View File

@@ -136,8 +136,7 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
* loaded directly into memory via JTAG, etc:
*/
file_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
(uintptr_t)source_buffer,
source_size);
source_buffer, source_size);
/*
* Make sure that device for device_path exist
* in load_image(). Otherwise, shell and grub will fail.

View File

@@ -515,7 +515,7 @@ static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp,
* will be freed in return_to_efibootmgr event callback.
*/
loaded_dp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
image_addr, image_size);
source_buffer, image_size);
ret = efi_install_multiple_protocol_interfaces(
&mem_handle, &efi_guid_device_path, loaded_dp, NULL);
if (ret != EFI_SUCCESS)

View File

@@ -11,6 +11,7 @@
#include <dm.h>
#include <dm/root.h>
#include <log.h>
#include <mapmem.h>
#include <net.h>
#include <usb.h>
#include <mmc.h>
@@ -975,8 +976,7 @@ struct efi_device_path __maybe_unused *efi_dp_from_eth(void)
}
/* Construct a device-path for memory-mapped image */
struct efi_device_path *efi_dp_from_mem(uint32_t memory_type,
uint64_t start_address,
struct efi_device_path *efi_dp_from_mem(uint32_t memory_type, void *start_ptr,
size_t size)
{
struct efi_device_path_memory *mdp;
@@ -991,8 +991,8 @@ struct efi_device_path *efi_dp_from_mem(uint32_t memory_type,
mdp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MEMORY;
mdp->dp.length = sizeof(*mdp);
mdp->memory_type = memory_type;
mdp->start_address = start_address;
mdp->end_address = start_address + size;
mdp->start_address = (uintptr_t)start_ptr;
mdp->end_address = mdp->start_address + size;
buf = &mdp[1];
*((struct efi_device_path *)buf) = END;
@@ -1061,7 +1061,7 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
struct efi_device_path *dp;
struct disk_partition fs_partition;
size_t image_size;
void *image_addr;
void *image_ptr;
int part = 0;
if (path && !file)
@@ -1070,10 +1070,10 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
if (IS_ENABLED(CONFIG_EFI_BINARY_EXEC) &&
(!strcmp(dev, "Mem") || !strcmp(dev, "hostfs"))) {
/* loadm command and semihosting */
efi_get_image_parameters(&image_addr, &image_size);
efi_get_image_parameters(&image_ptr, &image_size);
dp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
(uintptr_t)image_addr, image_size);
dp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, image_ptr,
image_size);
} else if (IS_ENABLED(CONFIG_NETDEVICES) && !strcmp(dev, "Net")) {
dp = efi_dp_from_eth();
} else if (!strcmp(dev, "Uart")) {