There is quite a bit of confusion in the EFI code as to whether a field contains an address or a pointer. As a first step towards resolving this, document the memory-descriptor struct, indicating that it holds pointers, not addresses.

Dro the same for efi_add_memory_map() as it is widely used, as well as efi_add_memory_map_pg() which is only used by lmb

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

Changes in v2: - Fix missing @ - Note that this holds pointers, not addresses

include/efi.h | 15 +++++++++++++++ include/efi_loader.h | 6 ++++-- lib/efi_loader/efi_memory.c | 4 +++- 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/include/efi.h b/include/efi.h index c559fda3004..3fce281233c 100644 --- a/include/efi.h +++ b/include/efi.h @@ -273,6 +273,21 @@ enum efi_memory_type { #define EFI_PAGE_SIZE (1ULL << EFI_PAGE_SHIFT) #define EFI_PAGE_MASK (EFI_PAGE_SIZE - 1)

+/** + * struct efi_mem_desc - defines an EFI memory record + * + * Note that this struct is for use outside U-Boot, so the two 'start' fields + * are pointers, not addresses. Use map_to_sysmem() to convert to an address, so + * that sandbox operates correctly. + * + * @type (enum efi_memory_type): EFI memory-type + * @reserved: unused + * @physical_start: Start address of region in physical memory + * @virtual_start: Start address of region in physical memory + * @num_pages: Number of EFI pages this record covers (each is EFI_PAGE_SIZE + * bytes) + * @attribute: Memory attributes (see EFI_MEMORY_...) + */ struct efi_mem_desc { u32 type; u32 reserved; diff --git a/include/efi_loader.h b/include/efi_loader.h index 39809eac1bc..ee0cdd36500 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -788,8 +788,10 @@ efi_status_t efi_add_memory_map(u64 start, u64 size, int memory_type); /** * efi_add_memory_map_pg() - add pages to the memory map * - * @start: start address, must be a multiple of - * EFI_PAGE_SIZE + * @start: start address, must be a multiple of EFI_PAGE_SIZE. Note that this + * is actually a pointer provided as an integer. To pass in an address, pass + * in (ulong)map_to_sysmem(addr) + * * @pages: number of pages to add * @memory_type: type of memory added * @overlap_conventional: region may only overlap free(conventional) diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index e493934c713..f1154f73e05 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -384,7 +384,9 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, /** * efi_add_memory_map() - add memory area to the memory map * - * @start: start address of the memory area + * @start: start address of the memory area. Note that this is + * actually a pointer provided as an integer. To pass in + * an address, pass in (ulong)map_to_sysmem(addr) * @size: length in bytes of the memory area * @memory_type: type of memory added *

Some functions are passing addresses instead of pointers to the efi_add_memory_map() function. This confusion is understandable since the function arguments indicate an address.

Make a note of the 8 places where there are problems, which would break usage in sandbox tests.

Future work will resolve these problems.

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

Changes in v2: - Add a new patch with comments where incorrect addresses are used

lib/efi_loader/efi_acpi.c | 2 ++ lib/efi_loader/efi_bootmgr.c | 5 +++++ lib/efi_loader/efi_helper.c | 1 + lib/efi_loader/efi_smbios.c | 6 +++++- lib/efi_loader/efi_var_mem.c | 2 ++ 5 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_acpi.c b/lib/efi_loader/efi_acpi.c index 67bd7f8ca24..ffb360d461b 100644 --- a/lib/efi_loader/efi_acpi.c +++ b/lib/efi_loader/efi_acpi.c @@ -28,12 +28,14 @@ efi_status_t efi_acpi_register(void) /* Mark space used for tables */ start = ALIGN_DOWN(gd->arch.table_start, EFI_PAGE_MASK); end = ALIGN(gd->arch.table_end, EFI_PAGE_MASK); + /* TODO(sjg): This should use (ulong)map_sysmem(start, ...) */ ret = efi_add_memory_map(start, end - start, EFI_ACPI_RECLAIM_MEMORY); if (ret != EFI_SUCCESS) return ret; if (gd->arch.table_start_high) { start = ALIGN_DOWN(gd->arch.table_start_high, EFI_PAGE_MASK); end = ALIGN(gd->arch.table_end_high, EFI_PAGE_MASK); + /* TODO(sjg): This should use (ulong)map_sysmem(start, ...) */ ret = efi_add_memory_map(start, end - start, EFI_ACPI_RECLAIM_MEMORY); if (ret != EFI_SUCCESS) diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 8c51a6ef2ed..bb635d77b53 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -365,6 +365,8 @@ static efi_status_t prepare_loaded_image(u16 *label, ulong addr, ulong size, * TODO: expose the ramdisk to OS. * Need to pass the ramdisk information by the architecture-specific * methods such as 'pmem' device-tree node. + * + * TODO(sjg): This should use (ulong)map_sysmem(addr) */ ret = efi_add_memory_map(addr, size, EFI_RESERVED_MEMORY_TYPE); if (ret != EFI_SUCCESS) { @@ -399,6 +401,7 @@ static efi_status_t efi_bootmgr_release_uridp(struct uridp_context *ctx)

/* cleanup for iso or img image */ if (ctx->ramdisk_blk_dev) { + /* TODO(sjg): This should use (ulong)map_sysmem(...) */ ret = efi_add_memory_map(ctx->image_addr, ctx->image_size, EFI_CONVENTIONAL_MEMORY); if (ret != EFI_SUCCESS) @@ -506,6 +509,8 @@ static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp,

source_buffer = NULL; source_size = 0; + + /* TODO(sjg): This does not work on sandbox */ } else if (efi_check_pe((void *)image_addr, image_size, NULL) == EFI_SUCCESS) { /* * loaded_dp must exist until efi application returns, diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c index bf96f61d3d0..fd89ef6036f 100644 --- a/lib/efi_loader/efi_helper.c +++ b/lib/efi_loader/efi_helper.c @@ -486,6 +486,7 @@ static efi_status_t copy_fdt(void **fdtp) log_err("Failed to reserve space for FDT\n"); goto done; } + /* TODO(sjg): This does not work on sandbox */ new_fdt = (void *)(uintptr_t)new_fdt_addr; memcpy(new_fdt, fdt, fdt_totalsize(fdt)); fdt_set_totalsize(new_fdt, fdt_size); diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c index 8d2ef6deb51..02d4a5dd045 100644 --- a/lib/efi_loader/efi_smbios.c +++ b/lib/efi_loader/efi_smbios.c @@ -40,7 +40,11 @@ efi_status_t efi_smbios_register(void) return EFI_NOT_FOUND; }

- /* Mark space used for tables */ + /* + * Mark space used for tables/ + * + * TODO(sjg): This should use (ulong)map_sysmem(addr, ...) + */ ret = efi_add_memory_map(addr, TABLE_SIZE, EFI_RUNTIME_SERVICES_DATA); if (ret) return ret; diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c index 139e16aad7c..5c69a1e0f3e 100644 --- a/lib/efi_loader/efi_var_mem.c +++ b/lib/efi_loader/efi_var_mem.c @@ -226,6 +226,8 @@ efi_status_t efi_var_mem_init(void) &memory); if (ret != EFI_SUCCESS) return ret; + + /* TODO(sjg): This does not work on sandbox */ efi_var_buf = (struct efi_var_file *)(uintptr_t)memory; memset(efi_var_buf, 0, EFI_VAR_BUF_SIZE); efi_var_buf->magic = EFI_VAR_FILE_MAGIC;

Update efi_allocate_pool_ext() to log the pointer returned from this call, which can be helpful when debugging.

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

Changes in v2: - Use EFI_PRINT() instead of log_debug()

lib/efi_loader/efi_boottime.c | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 080e7f78ae3..be8e4f41ad2 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -511,6 +511,9 @@ static efi_status_t EFIAPI efi_allocate_pool_ext(int pool_type,

EFI_ENTRY("%d, %zu, %p", pool_type, size, buffer); r = efi_allocate_pool(pool_type, size, buffer); + if (r == EFI_SUCCESS) + EFI_PRINT("*buffer = %p\n", *buffer); + return EFI_EXIT(r); }

Check for an error returned from the decompress() function, just in case.

Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org ---

(no changes since v1)

lib/efi_selftest/efi_selftest_startimage_exit.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/efi_selftest/efi_selftest_startimage_exit.c b/lib/efi_selftest/efi_selftest_startimage_exit.c index b65a10b7a4b..8d119f054c5 100644 --- a/lib/efi_selftest/efi_selftest_startimage_exit.c +++ b/lib/efi_selftest/efi_selftest_startimage_exit.c @@ -84,13 +84,15 @@ static efi_status_t decompress(u8 **image) static int setup(const efi_handle_t handle, const struct efi_system_table *systable) { + efi_status_t ret; + image_handle = handle; boottime = systable->boottime;

/* Load the application image into memory */ - decompress(&image); + ret = decompress(&image);

- return EFI_ST_SUCCESS; + return ret; }

/*

Bring in the documentation from the efi_loader.h header file, so we can see the API defined there.

Fix efi_alloc() to avoid a warning.

Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Heinrich Schuchardt xypron.glpk@gmx.de ---

Changes in v2: - Add new patch to add the EFI-loader API documentation

doc/api/efi.rst | 3 +++ include/efi_loader.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/doc/api/efi.rst b/doc/api/efi.rst index 43d6f936fb0..57da7f615a6 100644 --- a/doc/api/efi.rst +++ b/doc/api/efi.rst @@ -75,6 +75,9 @@ Image relocation Memory services ~~~~~~~~~~~~~~~

+.. kernel-doc:: include/efi_loader.h + :internal: + .. kernel-doc:: lib/efi_loader/efi_memory.c :internal:

diff --git a/include/efi_loader.h b/include/efi_loader.h index 00a1259c006..3f75f2efcb6 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -764,7 +764,7 @@ efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf, * * Allocate memory from pool and zero it out. * - * @size: number of bytes to allocate + * @len: number of bytes to allocate * Return: pointer to allocated memory or NULL */ void *efi_alloc(size_t len);

Rather than an integer, it is better to use the enum provided, when referring to an EFI memory-type. Update existing uses.

Call the value 'mem_type' consistently. Fix up one instance of upper-case hex.

Fix up the calls in struct efi_boot_services so that they use the same enum, adding the missing parameter names and enum efi_allocate_type.

While we are here, rename the 'memory' parameter to 'memoryp' so that it is clear it is a return value.

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

Changes in v2: - Drop the changes to the boottime API

include/efi_loader.h | 29 ++++++++++---------- lib/efi_loader/efi_boottime.c | 12 ++++----- lib/efi_loader/efi_device_path.c | 4 +-- lib/efi_loader/efi_memory.c | 46 +++++++++++++++++--------------- 4 files changed, 47 insertions(+), 44 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h index 3f75f2efcb6..a7228672f27 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -773,24 +773,25 @@ void *efi_alloc(size_t len); * efi_alloc_aligned_pages() - allocate aligned memory pages * * @len: len in bytes - * @memory_type: usage type of the allocated memory + * @mem_type: usage type of the allocated memory * @align: alignment in bytes * Return: aligned memory or NULL */ -void *efi_alloc_aligned_pages(u64 len, int memory_type, size_t align); +void *efi_alloc_aligned_pages(u64 len, enum efi_memory_type mem_type, + size_t align);

/** * efi_allocate_pages - allocate memory pages * * @type: type of allocation to be performed - * @memory_type: usage type of the allocated memory + * @mem_type: usage type of the allocated memory * @pages: number of pages to be allocated - * @memory: returns a pointer to the allocated memory + * @memoryp: returns a pointer to the allocated memory * Return: status code */ efi_status_t efi_allocate_pages(enum efi_allocate_type type, - enum efi_memory_type memory_type, - efi_uintn_t pages, uint64_t *memory); + enum efi_memory_type mem_type, + efi_uintn_t pages, uint64_t *memoryp);

/** * efi_free_pages() - free memory pages @@ -804,12 +805,12 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages); /** * efi_allocate_pool - allocate memory from pool * - * @pool_type: type of the pool from which memory is to be allocated + * @mem_type: memory type of the pool from which memory is to be allocated * @size: number of bytes to be allocated * @buffer: allocated memory * Return: status code */ -efi_status_t efi_allocate_pool(enum efi_memory_type pool_type, +efi_status_t efi_allocate_pool(enum efi_memory_type mem_type, efi_uintn_t size, void **buffer);

/** @@ -837,14 +838,14 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, * actually a pointer provided as an integer. To pass in * an address, pass in (ulong)map_to_sysmem(addr) * @size: length in bytes of the memory area - * @memory_type: type of memory added - * + * @mem_type: EFI type of memory added * Return: status code * * This function automatically aligns the start and size of the memory area * to EFI_PAGE_SIZE. */ -efi_status_t efi_add_memory_map(u64 start, u64 size, int memory_type); +efi_status_t efi_add_memory_map(u64 start, u64 size, + enum efi_memory_type mem_type);

/** * efi_add_memory_map_pg() - add pages to the memory map @@ -854,13 +855,13 @@ efi_status_t efi_add_memory_map(u64 start, u64 size, int memory_type); * in (ulong)map_to_sysmem(addr) * * @pages: number of pages to add - * @memory_type: EFI type of memory added + * @mem_type: EFI type of memory added * @overlap_conventional: region may only overlap free(conventional) * memory * Return: status code */ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, - int memory_type, + enum efi_memory_type mem_type, bool overlap_conventional);

/* Called by board init to initialize the EFI drivers */ @@ -919,7 +920,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, +struct efi_device_path *efi_dp_from_mem(enum efi_memory_type mem_type, uint64_t start_address, size_t size); /* Determine the last device path node that is not the end node. */ diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index be8e4f41ad2..8f311f95d1f 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -414,9 +414,9 @@ static void EFIAPI efi_restore_tpl(efi_uintn_t old_tpl) /** * efi_allocate_pages_ext() - allocate memory pages * @type: type of allocation to be performed - * @memory_type: usage type of the allocated memory + * @mem_type: usage type of the allocated memory * @pages: number of pages to be allocated - * @memory: allocated memory + * @memoryp: allocated memory * * This function implements the AllocatePages service. * @@ -425,14 +425,14 @@ static void EFIAPI efi_restore_tpl(efi_uintn_t old_tpl) * * Return: status code */ -static efi_status_t EFIAPI efi_allocate_pages_ext(int type, int memory_type, +static efi_status_t EFIAPI efi_allocate_pages_ext(int type, int mem_type, efi_uintn_t pages, - uint64_t *memory) + uint64_t *memoryp) { efi_status_t r;

- EFI_ENTRY("%d, %d, 0x%zx, %p", type, memory_type, pages, memory); - r = efi_allocate_pages(type, memory_type, pages, memory); + EFI_ENTRY("%d, %d, 0x%zx, %p", type, mem_type, pages, memoryp); + r = efi_allocate_pages(type, mem_type, pages, memoryp); return EFI_EXIT(r); }

diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index ee387e1dfd4..9c8cd35b97b 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -975,7 +975,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, +struct efi_device_path *efi_dp_from_mem(enum efi_memory_type mem_type, uint64_t start_address, size_t size) { @@ -990,7 +990,7 @@ struct efi_device_path *efi_dp_from_mem(uint32_t memory_type, mdp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; mdp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MEMORY; mdp->dp.length = sizeof(*mdp); - mdp->memory_type = memory_type; + mdp->memory_type = mem_type; mdp->start_address = start_address; mdp->end_address = start_address + size; buf = &mdp[1]; diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 9d0f27dbb3e..3cece51f030 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -258,7 +258,7 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, }

efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, - int memory_type, + enum efi_memory_type mem_type, bool overlap_conventional) { struct efi_mem_list *lmem; @@ -268,10 +268,10 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, struct efi_event *evt;

EFI_PRINT("%s: 0x%llx 0x%llx %d %s\n", __func__, - start, pages, memory_type, overlap_conventional ? + start, pages, mem_type, overlap_conventional ? "yes" : "no");

- if (memory_type >= EFI_MAX_MEMORY_TYPE) + if (mem_type >= EFI_MAX_MEMORY_TYPE) return EFI_INVALID_PARAMETER;

if (!pages) @@ -281,12 +281,12 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, newlist = calloc(1, sizeof(*newlist)); if (!newlist) return EFI_OUT_OF_RESOURCES; - newlist->desc.type = memory_type; + newlist->desc.type = mem_type; newlist->desc.physical_start = start; newlist->desc.virtual_start = start; newlist->desc.num_pages = pages;

- switch (memory_type) { + switch (mem_type) { case EFI_RUNTIME_SERVICES_CODE: case EFI_RUNTIME_SERVICES_DATA: newlist->desc.attribute = EFI_MEMORY_WB | EFI_MEMORY_RUNTIME; @@ -370,14 +370,15 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, return EFI_SUCCESS; }

-efi_status_t efi_add_memory_map(u64 start, u64 size, int memory_type) +efi_status_t efi_add_memory_map(u64 start, u64 size, + enum efi_memory_type mem_type) { u64 pages;

pages = efi_size_in_pages(size + (start & EFI_PAGE_MASK)); start &= ~EFI_PAGE_MASK;

- return efi_add_memory_map_pg(start, pages, memory_type, false); + return efi_add_memory_map_pg(start, pages, mem_type, false); }

/** @@ -416,8 +417,8 @@ static efi_status_t efi_check_allocated(u64 addr, bool must_be_allocated) }

efi_status_t efi_allocate_pages(enum efi_allocate_type type, - enum efi_memory_type memory_type, - efi_uintn_t pages, uint64_t *memory) + enum efi_memory_type mem_type, + efi_uintn_t pages, uint64_t *memoryp) { u64 efi_addr, len; uint flags; @@ -425,10 +426,9 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type, phys_addr_t addr;

/* Check import parameters */ - if (memory_type >= EFI_PERSISTENT_MEMORY_TYPE && - memory_type <= 0x6FFFFFFF) + if (mem_type >= EFI_PERSISTENT_MEMORY_TYPE && mem_type <= 0x6fffffff) return EFI_INVALID_PARAMETER; - if (!memory) + if (!memoryp) return EFI_INVALID_PARAMETER; len = (u64)pages << EFI_PAGE_SHIFT; /* Catch possible overflow on 64bit systems */ @@ -447,17 +447,17 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type, break; case EFI_ALLOCATE_MAX_ADDRESS: /* Max address */ - addr = map_to_sysmem((void *)(uintptr_t)*memory); + addr = map_to_sysmem((void *)(uintptr_t)*memoryp); addr = (u64)lmb_alloc_base_flags(len, EFI_PAGE_SIZE, addr, flags); if (!addr) return EFI_OUT_OF_RESOURCES; break; case EFI_ALLOCATE_ADDRESS: - if (*memory & EFI_PAGE_MASK) + if (*memoryp & EFI_PAGE_MASK) return EFI_NOT_FOUND;

- addr = map_to_sysmem((void *)(uintptr_t)*memory); + addr = map_to_sysmem((void *)(uintptr_t)*memoryp); addr = (u64)lmb_alloc_addr_flags(addr, len, flags); if (!addr) return EFI_NOT_FOUND; @@ -469,7 +469,7 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type,

efi_addr = (u64)(uintptr_t)map_sysmem(addr, 0); /* Reserve that map in our memory maps */ - ret = efi_add_memory_map_pg(efi_addr, pages, memory_type, true); + ret = efi_add_memory_map_pg(efi_addr, pages, mem_type, true); if (ret != EFI_SUCCESS) { /* Map would overlap, bail out */ lmb_free_flags(addr, (u64)pages << EFI_PAGE_SHIFT, flags); @@ -477,7 +477,7 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type, return EFI_OUT_OF_RESOURCES; }

- *memory = efi_addr; + *memoryp = efi_addr;

return EFI_SUCCESS; } @@ -515,7 +515,8 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages) return ret; }

-void *efi_alloc_aligned_pages(u64 len, int memory_type, size_t align) +void *efi_alloc_aligned_pages(u64 len, enum efi_memory_type mem_type, + size_t align) { u64 req_pages = efi_size_in_pages(len); u64 true_pages = req_pages + efi_size_in_pages(align) - 1; @@ -533,12 +534,12 @@ void *efi_alloc_aligned_pages(u64 len, int memory_type, size_t align) return NULL;

if (align < EFI_PAGE_SIZE) { - r = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, memory_type, + r = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, mem_type, req_pages, &mem); return (r == EFI_SUCCESS) ? (void *)(uintptr_t)mem : NULL; }

- r = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, memory_type, + r = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, mem_type, true_pages, &mem); if (r != EFI_SUCCESS) return NULL; @@ -559,7 +560,8 @@ void *efi_alloc_aligned_pages(u64 len, int memory_type, size_t align) return (void *)(uintptr_t)aligned_mem; }

-efi_status_t efi_allocate_pool(enum efi_memory_type pool_type, efi_uintn_t size, void **buffer) +efi_status_t efi_allocate_pool(enum efi_memory_type mem_type, efi_uintn_t size, + void **buffer) { efi_status_t r; u64 addr; @@ -575,7 +577,7 @@ efi_status_t efi_allocate_pool(enum efi_memory_type pool_type, efi_uintn_t size, return EFI_SUCCESS; }

- r = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, pool_type, num_pages, + r = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, mem_type, num_pages, &addr); if (r == EFI_SUCCESS) { alloc = (struct efi_pool_allocation *)(uintptr_t)addr;

This field is always the same as physical_start, so keeping track of it separately is unnecessary and confusing. Drop it.

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

(no changes since v1)

lib/efi_loader/efi_memory.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 9e4158edfaf..518b8cff61d 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -37,7 +37,6 @@ efi_uintn_t efi_memory_map_key; * @type (enum efi_memory_type): EFI memory-type * @reserved: unused * @physical_start: Start address of region in physical memory - * @virtual_start: Start address of region in physical memory * @num_pages: Number of EFI pages this record covers (each is EFI_PAGE_SIZE * bytes) * @attribute: Memory attributes (see EFI_MEMORY...) @@ -46,7 +45,6 @@ struct priv_mem_desc { u32 type; u32 reserved; efi_physical_addr_t physical_start; - efi_virtual_addr_t virtual_start; u64 num_pages; u64 attribute; }; @@ -187,7 +185,6 @@ static void efi_mem_sort(void) pages = cur->num_pages; prev->num_pages += pages; prev->physical_start -= pages << EFI_PAGE_SHIFT; - prev->virtual_start -= pages << EFI_PAGE_SHIFT; list_del(&lmem->link); free(lmem);

@@ -255,7 +252,6 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, free(map); } else { map->desc.physical_start = carve_end; - map->desc.virtual_start = carve_end; map->desc.num_pages = (map_end - carve_end) >> EFI_PAGE_SHIFT; } @@ -276,7 +272,6 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, return EFI_CARVE_OUT_OF_RESOURCES; newmap->desc = map->desc; newmap->desc.physical_start = carve_start; - newmap->desc.virtual_start = carve_start; newmap->desc.num_pages = (map_end - carve_start) >> EFI_PAGE_SHIFT; /* Insert before current entry (descending address order) */ list_add_tail(&newmap->link, &map->link); @@ -313,7 +308,6 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, return EFI_OUT_OF_RESOURCES; newlist->desc.type = mem_type; newlist->desc.physical_start = start; - newlist->desc.virtual_start = start; newlist->desc.num_pages = pages;

switch (mem_type) { @@ -713,7 +707,9 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, memory_map->type = lmem->desc.type; memory_map->reserved = lmem->desc.reserved; memory_map->physical_start = lmem->desc.physical_start; - memory_map->virtual_start = lmem->desc.virtual_start; + + /* virtual and physical are always the same */ + memory_map->virtual_start = lmem->desc.physical_start; memory_map->num_pages = lmem->desc.num_pages; memory_map->attribute = lmem->desc.attribute; memory_map--;

We can make use of the enum now, since this struct is not exported to the EFI app.

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

(no changes since v1)

lib/efi_loader/efi_memory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 81e40fc923d..83f2e32b9a4 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -34,14 +34,14 @@ efi_uintn_t efi_memory_map_key; * EFI application, other than through calls to efi_get_memory_map(), where this * internal format is converted to the external struct efi_mem_desc format. * - * @type (enum efi_memory_type): EFI memory-type + * @type: EFI memory-type * @physical_start: Start address of region in physical memory * @num_pages: Number of EFI pages this record covers (each is EFI_PAGE_SIZE * bytes) * @attribute: Memory attributes (see EFI_MEMORY...) */ struct priv_mem_desc { - u32 type; + enum efi_memory_type type; efi_physical_addr_t physical_start; u64 num_pages; u64 attribute;

We don't need a separate struct for the values in this node. Move everything in together, so that there is just one struct to consider.

Add a comment about physical_start, so it is clear that it is really a pointer.

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

(no changes since v1)

lib/efi_loader/efi_memory.c | 85 +++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 46 deletions(-)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index b51bb367899..6e474c88784 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -28,36 +28,29 @@ DECLARE_GLOBAL_DATA_PTR; efi_uintn_t efi_memory_map_key;

/** - * struct priv_mem_desc - defines an memory region + * struct efi_mem_list - defines an EFI memory record * * Note that this struct is for use inside U-Boot and is not visible to the * EFI application, other than through calls to efi_get_memory_map(), where this * internal format is converted to the external struct efi_mem_desc format. * + * @link: Link to prev/next node in list * @type: EFI memory-type - * @physical_start: Start address of region in physical memory + * @physical_start: Start address of region in physical memory. Note that this + * is really a pointer stored as an address, so use map_to_sysmem() to + * convert it to an address if needed * @num_pages: Number of EFI pages this record covers (each is EFI_PAGE_SIZE * bytes) * @attribute: Memory attributes (see EFI_MEMORY...) */ -struct priv_mem_desc { +struct efi_mem_list { + struct list_head link; enum efi_memory_type type; efi_physical_addr_t physical_start; u64 num_pages; u64 attribute; };

-/** - * struct efi_mem_list - defines an EFI memory record - * - * @link: Link to prev/next node in list - * @desc: Memory information about this node - */ -struct efi_mem_list { - struct list_head link; - struct priv_mem_desc desc; -}; - #define EFI_CARVE_NO_OVERLAP -1 #define EFI_CARVE_LOOP_AGAIN -2 #define EFI_CARVE_OVERLAPS_NONRAM -3 @@ -128,9 +121,9 @@ static int efi_mem_cmp(void *priv, struct list_head *a, struct list_head *b) struct efi_mem_list *mema = list_entry(a, struct efi_mem_list, link); struct efi_mem_list *memb = list_entry(b, struct efi_mem_list, link);

- if (mema->desc.physical_start == memb->desc.physical_start) + if (mema->physical_start == memb->physical_start) return 0; - else if (mema->desc.physical_start < memb->desc.physical_start) + else if (mema->physical_start < memb->physical_start) return 1; else return -1; @@ -139,12 +132,12 @@ static int efi_mem_cmp(void *priv, struct list_head *a, struct list_head *b) /** * desc_get_end() - get end address of memory area * - * @desc: memory descriptor + * @node: memory node * Return: end address + 1 */ -static uint64_t desc_get_end(struct priv_mem_desc *desc) +static uint64_t desc_get_end(struct efi_mem_list *node) { - return desc->physical_start + (desc->num_pages << EFI_PAGE_SHIFT); + return node->physical_start + (node->num_pages << EFI_PAGE_SHIFT); }

/** @@ -164,8 +157,8 @@ static void efi_mem_sort(void) while (merge_again) { merge_again = false; list_for_each_entry(lmem, &efi_mem, link) { - struct priv_mem_desc *prev; - struct priv_mem_desc *cur; + struct efi_mem_list *prev; + struct efi_mem_list *cur; uint64_t pages;

if (!prevmem) { @@ -173,8 +166,8 @@ static void efi_mem_sort(void) continue; }

- cur = &lmem->desc; - prev = &prevmem->desc; + cur = lmem; + prev = prevmem;

if ((desc_get_end(cur) == prev->physical_start) && (prev->type == cur->type) && @@ -219,11 +212,11 @@ static void efi_mem_sort(void) * to re-add the already carved out pages to the mapping. */ static s64 efi_mem_carve_out(struct efi_mem_list *map, - struct priv_mem_desc *carve_desc, + struct efi_mem_list *carve_desc, bool overlap_conventional) { struct efi_mem_list *newmap; - struct priv_mem_desc *map_desc = &map->desc; + struct efi_mem_list *map_desc = map; uint64_t map_start = map_desc->physical_start; uint64_t map_end = map_start + (map_desc->num_pages << EFI_PAGE_SHIFT); uint64_t carve_start = carve_desc->physical_start; @@ -249,8 +242,8 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, list_del(&map->link); free(map); } else { - map->desc.physical_start = carve_end; - map->desc.num_pages = (map_end - carve_end) + map->physical_start = carve_end; + map->num_pages = (map_end - carve_end) >> EFI_PAGE_SHIFT; }

@@ -268,10 +261,10 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, newmap = calloc(1, sizeof(*newmap)); if (!newmap) return EFI_CARVE_OUT_OF_RESOURCES; - newmap->desc.type = map->desc.type; - newmap->desc.physical_start = carve_start; - newmap->desc.num_pages = (map_end - carve_start) >> EFI_PAGE_SHIFT; - newmap->desc.attribute = map->desc.attribute; + newmap->type = map->type; + newmap->physical_start = carve_start; + newmap->num_pages = (map_end - carve_start) >> EFI_PAGE_SHIFT; + newmap->attribute = map->attribute; /* Insert before current entry (descending address order) */ list_add_tail(&newmap->link, &map->link);

@@ -305,20 +298,20 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, newlist = calloc(1, sizeof(*newlist)); if (!newlist) return EFI_OUT_OF_RESOURCES; - newlist->desc.type = mem_type; - newlist->desc.physical_start = start; - newlist->desc.num_pages = pages; + newlist->type = mem_type; + newlist->physical_start = start; + newlist->num_pages = pages;

switch (mem_type) { case EFI_RUNTIME_SERVICES_CODE: case EFI_RUNTIME_SERVICES_DATA: - newlist->desc.attribute = EFI_MEMORY_WB | EFI_MEMORY_RUNTIME; + newlist->attribute = EFI_MEMORY_WB | EFI_MEMORY_RUNTIME; break; case EFI_MMAP_IO: - newlist->desc.attribute = EFI_MEMORY_RUNTIME; + newlist->attribute = EFI_MEMORY_RUNTIME; break; default: - newlist->desc.attribute = EFI_MEMORY_WB; + newlist->attribute = EFI_MEMORY_WB; break; }

@@ -328,7 +321,7 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, list_for_each_entry(lmem, &efi_mem, link) { s64 r;

- r = efi_mem_carve_out(lmem, &newlist->desc, + r = efi_mem_carve_out(lmem, newlist, overlap_conventional); switch (r) { case EFI_CARVE_OUT_OF_RESOURCES: @@ -424,12 +417,12 @@ static efi_status_t efi_check_allocated(u64 addr, bool must_be_allocated) struct efi_mem_list *item;

list_for_each_entry(item, &efi_mem, link) { - u64 start = item->desc.physical_start; - u64 end = start + (item->desc.num_pages << EFI_PAGE_SHIFT); + u64 start = item->physical_start; + u64 end = start + (item->num_pages << EFI_PAGE_SHIFT);

if (addr >= start && addr < end) { if (must_be_allocated ^ - (item->desc.type == EFI_CONVENTIONAL_MEMORY)) + (item->type == EFI_CONVENTIONAL_MEMORY)) return EFI_SUCCESS; else return EFI_NOT_FOUND; @@ -703,14 +696,14 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, /* Return the list in ascending order */ memory_map = &memory_map[map_entries - 1]; list_for_each_entry(lmem, &efi_mem, link) { - memory_map->type = lmem->desc.type; + memory_map->type = lmem->type; memory_map->reserved = 0; - memory_map->physical_start = lmem->desc.physical_start; + memory_map->physical_start = lmem->physical_start;

/* virtual and physical are always the same */ - memory_map->virtual_start = lmem->desc.physical_start; - memory_map->num_pages = lmem->desc.num_pages; - memory_map->attribute = lmem->desc.attribute; + memory_map->virtual_start = lmem->physical_start; + memory_map->num_pages = lmem->num_pages; + memory_map->attribute = lmem->attribute; memory_map--; }

The word 'physical' suggests that there is an associated virtual address but there is not. Use 'base' since this is the base address of a region.

Change some uint64_t and remove some brackets to keep checkpatch happy.

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

(no changes since v1)

lib/efi_loader/efi_memory.c | 38 ++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index dbcba99ae39..89f22388fc4 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -36,7 +36,7 @@ efi_uintn_t efi_memory_map_key; * * @link: Link to prev/next node in list * @type: EFI memory-type - * @physical_start: Start address of region in physical memory. Note that this + * @base: Start address of region in physical memory. Note that this * is really a pointer stored as an address, so use map_to_sysmem() to * convert it to an address if needed * @num_pages: Number of EFI pages this record covers (each is EFI_PAGE_SIZE @@ -46,7 +46,7 @@ efi_uintn_t efi_memory_map_key; struct mem_node { struct list_head link; enum efi_memory_type type; - efi_physical_addr_t physical_start; + efi_physical_addr_t base; u64 num_pages; u64 attribute; }; @@ -121,9 +121,9 @@ static int efi_mem_cmp(void *priv, struct list_head *a, struct list_head *b) struct mem_node *mema = list_entry(a, struct mem_node, link); struct mem_node *memb = list_entry(b, struct mem_node, link);

- if (mema->physical_start == memb->physical_start) + if (mema->base == memb->base) return 0; - else if (mema->physical_start < memb->physical_start) + else if (mema->base < memb->base) return 1; else return -1; @@ -137,7 +137,7 @@ static int efi_mem_cmp(void *priv, struct list_head *a, struct list_head *b) */ static uint64_t desc_get_end(struct mem_node *node) { - return node->physical_start + (node->num_pages << EFI_PAGE_SHIFT); + return node->base + (node->num_pages << EFI_PAGE_SHIFT); }

/** @@ -169,13 +169,13 @@ static void efi_mem_sort(void) cur = lmem; prev = prevmem;

- if ((desc_get_end(cur) == prev->physical_start) && - (prev->type == cur->type) && - (prev->attribute == cur->attribute)) { + if (desc_get_end(cur) == prev->base && + prev->type == cur->type && + prev->attribute == cur->attribute) { /* There is an existing map before, reuse it */ pages = cur->num_pages; prev->num_pages += pages; - prev->physical_start -= pages << EFI_PAGE_SHIFT; + prev->base -= pages << EFI_PAGE_SHIFT; list_del(&lmem->link); free(lmem);

@@ -216,10 +216,10 @@ static s64 efi_mem_carve_out(struct mem_node *map, struct mem_node *carve_desc, { struct mem_node *newmap; struct mem_node *map_desc = map; - uint64_t map_start = map_desc->physical_start; - uint64_t map_end = map_start + (map_desc->num_pages << EFI_PAGE_SHIFT); - uint64_t carve_start = carve_desc->physical_start; - uint64_t carve_end = carve_start + + u64 map_start = map_desc->base; + u64 map_end = map_start + (map_desc->num_pages << EFI_PAGE_SHIFT); + u64 carve_start = carve_desc->base; + u64 carve_end = carve_start + (carve_desc->num_pages << EFI_PAGE_SHIFT);

/* check whether we're overlapping */ @@ -241,7 +241,7 @@ static s64 efi_mem_carve_out(struct mem_node *map, struct mem_node *carve_desc, list_del(&map->link); free(map); } else { - map->physical_start = carve_end; + map->base = carve_end; map->num_pages = (map_end - carve_end) >> EFI_PAGE_SHIFT; } @@ -261,7 +261,7 @@ static s64 efi_mem_carve_out(struct mem_node *map, struct mem_node *carve_desc, if (!newmap) return EFI_CARVE_OUT_OF_RESOURCES; newmap->type = map->type; - newmap->physical_start = carve_start; + newmap->base = carve_start; newmap->num_pages = (map_end - carve_start) >> EFI_PAGE_SHIFT; newmap->attribute = map->attribute; /* Insert before current entry (descending address order) */ @@ -298,7 +298,7 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, if (!newlist) return EFI_OUT_OF_RESOURCES; newlist->type = mem_type; - newlist->physical_start = start; + newlist->base = start; newlist->num_pages = pages;

switch (mem_type) { @@ -416,7 +416,7 @@ static efi_status_t efi_check_allocated(u64 addr, bool must_be_allocated) struct mem_node *item;

list_for_each_entry(item, &efi_mem, link) { - u64 start = item->physical_start; + u64 start = item->base; u64 end = start + (item->num_pages << EFI_PAGE_SHIFT);

if (addr >= start && addr < end) { @@ -697,10 +697,10 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, list_for_each_entry(lmem, &efi_mem, link) { memory_map->type = lmem->type; memory_map->reserved = 0; - memory_map->physical_start = lmem->physical_start; + memory_map->physical_start = lmem->base;

/* virtual and physical are always the same */ - memory_map->virtual_start = lmem->physical_start; + memory_map->virtual_start = lmem->base; memory_map->num_pages = lmem->num_pages; memory_map->attribute = lmem->attribute; memory_map--;

Update this function and its friend to use an address rather than a pointer cast to an integer.

The callers are updated in later patches, as marked, to reduce the diff, but we may which to squash some patches.

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

(no changes since v1)

include/efi_loader.h | 11 +++++------ lib/efi_loader/efi_memory.c | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h index 4c346addae3..f1ae29b04d7 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -834,9 +834,9 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, /** * efi_add_memory_map() - add memory area to the memory map * - * @start: start address of the memory area. Note that this is - * actually a pointer provided as an integer. To pass in - * an address, pass in (ulong)map_to_sysmem(addr) + * @start: start address, must be a multiple of EFI_PAGE_SIZE. Note + * that this is an address, not a pointer. Use + * map_to_sysmem(ptr) if you need to pass in a pointer * @size: length in bytes of the memory area * @mem_type: EFI type of memory added * Return: status code @@ -851,9 +851,8 @@ efi_status_t efi_add_memory_map(u64 start, u64 size, * efi_add_memory_map_pg() - add pages to the memory map * * @start: start address, must be a multiple of EFI_PAGE_SIZE. Note that this - * is actually a pointer provided as an integer. To pass in an address, pass - * in (ulong)map_to_sysmem(addr) - * + * is an address, not a pointer. Use map_to_sysmem(ptr) if you need to pass + * in a pointer * @pages: number of pages to add * @mem_type: EFI type of memory added * @overlap_conventional: region may only overlap free(conventional) diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 89f22388fc4..157ba5ec50c 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -298,7 +298,7 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, if (!newlist) return EFI_OUT_OF_RESOURCES; newlist->type = mem_type; - newlist->base = start; + newlist->base = (uintptr_t)map_sysmem(start, pages * EFI_PAGE_SIZE); newlist->num_pages = pages;

switch (mem_type) {

Adjust this function to use addresses rather than pointers, so that it doesn't have to use mapmem.

This is fix#2/4 for: Update efi_add_memory_map() to use address

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

(no changes since v1)

lib/efi_loader/efi_dt_fixup.c | 4 ---- 1 file changed, 4 deletions(-)

diff --git a/lib/efi_loader/efi_dt_fixup.c b/lib/efi_loader/efi_dt_fixup.c index 0dac94b0c6c..72d5d432a42 100644 --- a/lib/efi_loader/efi_dt_fixup.c +++ b/lib/efi_loader/efi_dt_fixup.c @@ -9,7 +9,6 @@ #include <efi_loader.h> #include <efi_rng.h> #include <fdtdec.h> -#include <mapmem.h>

const efi_guid_t efi_guid_dt_fixup_protocol = EFI_DT_FIXUP_PROTOCOL_GUID;

@@ -26,9 +25,6 @@ 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 (nomap) type = EFI_RESERVED_MEMORY_TYPE; else

Now that efi_add_memory_map() takes an address rather than a pointer, do the mapping in this function.

This is fix#4/4 for: Update efi_add_memory_map() to use address

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

(no changes since v1)

lib/efi_loader/efi_runtime.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 9d3b940afbd..37ed5963e2b 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -13,6 +13,7 @@ #include <efi_variable.h> #include <log.h> #include <malloc.h> +#include <mapmem.h> #include <rtc.h> #include <asm/global_data.h> #include <u-boot/crc.h> @@ -930,7 +931,7 @@ out: efi_status_t efi_add_runtime_mmio(void **mmio_ptr, u64 len) { struct efi_runtime_mmio_list *newmmio; - uint64_t addr = *(uintptr_t *)mmio_ptr; + u64 addr = map_to_sysmem(*mmio_ptr); efi_status_t ret;

ret = efi_add_memory_map(addr, len, EFI_MMAP_IO);

Now that these problems have been resolved, drop the comments.

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

(no changes since v1)

lib/efi_loader/efi_acpi.c | 2 -- lib/efi_loader/efi_bootmgr.c | 3 --- lib/efi_loader/efi_smbios.c | 6 +----- 3 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/lib/efi_loader/efi_acpi.c b/lib/efi_loader/efi_acpi.c index ffb360d461b..67bd7f8ca24 100644 --- a/lib/efi_loader/efi_acpi.c +++ b/lib/efi_loader/efi_acpi.c @@ -28,14 +28,12 @@ efi_status_t efi_acpi_register(void) /* Mark space used for tables */ start = ALIGN_DOWN(gd->arch.table_start, EFI_PAGE_MASK); end = ALIGN(gd->arch.table_end, EFI_PAGE_MASK); - /* TODO(sjg): This should use (ulong)map_sysmem(start, ...) */ ret = efi_add_memory_map(start, end - start, EFI_ACPI_RECLAIM_MEMORY); if (ret != EFI_SUCCESS) return ret; if (gd->arch.table_start_high) { start = ALIGN_DOWN(gd->arch.table_start_high, EFI_PAGE_MASK); end = ALIGN(gd->arch.table_end_high, EFI_PAGE_MASK); - /* TODO(sjg): This should use (ulong)map_sysmem(start, ...) */ ret = efi_add_memory_map(start, end - start, EFI_ACPI_RECLAIM_MEMORY); if (ret != EFI_SUCCESS) diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index b05d7e2ed54..b4ea6b817b6 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -365,8 +365,6 @@ static efi_status_t prepare_loaded_image(u16 *label, ulong addr, ulong size, * TODO: expose the ramdisk to OS. * Need to pass the ramdisk information by the architecture-specific * methods such as 'pmem' device-tree node. - * - * TODO(sjg): This should use (ulong)map_sysmem(addr) */ ret = efi_add_memory_map(addr, size, EFI_RESERVED_MEMORY_TYPE); if (ret != EFI_SUCCESS) { @@ -401,7 +399,6 @@ static efi_status_t efi_bootmgr_release_uridp(struct uridp_context *ctx)

/* cleanup for iso or img image */ if (ctx->ramdisk_blk_dev) { - /* TODO(sjg): This should use (ulong)map_sysmem(...) */ ret = efi_add_memory_map(ctx->image_addr, ctx->image_size, EFI_CONVENTIONAL_MEMORY); if (ret != EFI_SUCCESS) diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c index 02d4a5dd045..8d2ef6deb51 100644 --- a/lib/efi_loader/efi_smbios.c +++ b/lib/efi_loader/efi_smbios.c @@ -40,11 +40,7 @@ efi_status_t efi_smbios_register(void) return EFI_NOT_FOUND; }

- /* - * Mark space used for tables/ - * - * TODO(sjg): This should use (ulong)map_sysmem(addr, ...) - */ + /* Mark space used for tables */ ret = efi_add_memory_map(addr, TABLE_SIZE, EFI_RUNTIME_SERVICES_DATA); if (ret) return ret;

This cannot work since the code is not present in the emulated memory. In any case, sandbox cannot make use of the runtime code.

For now, just drop it from sandbox. We can always adjust things to copy it into memory, if needed.

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

(no changes since v1)

lib/efi_loader/efi_memory.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index a33c025fa20..796fa99f4fb 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -755,16 +755,22 @@ static void add_u_boot_and_runtime(void) runtime_mask = SZ_64K - 1; #endif

- /* - * Add Runtime Services. We mark surrounding boottime code as runtime as - * well to fulfill the runtime alignment constraints but avoid padding. - */ - runtime_start = (uintptr_t)__efi_runtime_start & ~runtime_mask; - runtime_end = (uintptr_t)__efi_runtime_stop; - runtime_end = (runtime_end + runtime_mask) & ~runtime_mask; - runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT; - efi_add_memory_map_pg(runtime_start, runtime_pages, - EFI_RUNTIME_SERVICES_CODE, false); + if (!IS_ENABLED(CONFIG_SANDBOX)) { + /* + * Add Runtime Services. We mark surrounding boottime code as + * runtime as well to fulfill the runtime alignment constraints + * but avoid padding. + * + * This is not enabled for sandbox, since we cannot map the + * sandbox code into emulated SDRAM + */ + runtime_start = (uintptr_t)__efi_runtime_start & ~runtime_mask; + runtime_end = (uintptr_t)__efi_runtime_stop; + runtime_end = (runtime_end + runtime_mask) & ~runtime_mask; + runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT; + efi_add_memory_map_pg(runtime_start, runtime_pages, + EFI_RUNTIME_SERVICES_CODE, false); + } }

int efi_memory_init(void)