[PATCH v3 00/28] 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.
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
Link: https://lore.kernel.org/u-boot/20240725135629.3505072-1-sjg@chromium.org/
Changes in v3: - Adjust comments - Put the header-file in its own section - Add comment to struct efi_device_path_memory - Use a pointer for the values in struct efi_device_path_memory
Changes in v2: - Fix missing @ - Note that this holds pointers, not addresses - Add a new patch with comments where incorrect addresses are used - Use EFI_PRINT() instead of log_debug() - Rebase on early patch - Add new patch to add the EFI-loader API documentation - Drop the changes to the boottime API - Add new patch to use a separate stuct for memory nodes - 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
Simon Glass (28): efi: Define fields in struct efi_mem_desc efi_loader: Fix typos in enum efi_allocate_type efi_loader: Drop extra brackets in efi_mem_carve_out() efi_loader: Add comments where incorrect addresses are used efi_loader: Show the resulting memory address from an alloc efi_loader: Update startimage_exit self-test to check error efi_loader: Move some memory-function comments to header doc: efi: Add the EFI-loader API documentation efi_loader: Use the enum for memory type efi_loader: Use a separate struct for memory nodes efi_loader: Drop virtual_start from priv_mem_desc efi_loader: Drop reserved from priv_mem_desc efi_loader: Use the enum for the memory type in priv_mem_desc efi_loader: Avoid assigning desc in efi_mem_carve_out() efi_loader: Move struct efi_mem_list fields together efi_loader: Rename struct efi_mem_list to mem_node efi_loader: Rename physical_start to base efi_loader: Use correct type in efi_add_runtime_mmio() efi_loader: Update efi_add_memory_map() to use address lmb: Reduce mapmem contortions in lmb_map_update_notify() efi_loader: Use addresses in efi_reserve_memory() efi_loader: Use addresses in efi_memory efi_loader: Update efi_add_runtime_mmio() to pass an address efi_loader: Correct address-usage in copy_fdt() efi_loader: Drop comments about incorrect addresses efi_bootmgr: Avoid casts in try_load_from_uri_path() efi_loader: Don't try to add sandbox runtime code efi_loader: Simplify efi_dp_from_mem()
arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 2 +- arch/arm/mach-bcm283x/reset.c | 2 +- doc/api/efi.rst | 6 + include/efi.h | 23 +- include/efi_api.h | 10 + include/efi_loader.h | 102 ++++-- lib/efi_loader/efi_bootbin.c | 3 +- lib/efi_loader/efi_bootmgr.c | 10 +- lib/efi_loader/efi_boottime.c | 51 ++- lib/efi_loader/efi_device_path.c | 18 +- lib/efi_loader/efi_dt_fixup.c | 4 - lib/efi_loader/efi_helper.c | 4 +- lib/efi_loader/efi_image_loader.c | 3 +- lib/efi_loader/efi_memory.c | 299 +++++++----------- lib/efi_loader/efi_runtime.c | 7 +- lib/efi_loader/efi_var_mem.c | 6 +- .../efi_selftest_startimage_exit.c | 6 +- lib/lmb.c | 10 +- 18 files changed, 311 insertions(+), 255 deletions(-)
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 v3: - Adjust comments
Changes in v2: - Fix missing @ - Note that this holds pointers, not addresses
include/efi.h | 21 +++++++++++++++++++++ include/efi_loader.h | 6 ++++-- lib/efi_loader/efi_memory.c | 4 +++- 3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/include/efi.h b/include/efi.h index c559fda3004..348ff9efd16 100644 --- a/include/efi.h +++ b/include/efi.h @@ -273,6 +273,27 @@ 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 + * + * This field implements the EFI_MEMORY_DESCRIPTOR type of the UEFI + * specification. + * + * 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 virtual memory, which will be the + * same as @physical_start before where both addresses will always be the + * same before SetVirtualMemoryMap() is called as the UEFI specification + * requires identity mapping. + * @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 *
Fix 'indicatged' and 'adress' typos.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de ---
(no changes since v1)
include/efi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/efi.h b/include/efi.h index 348ff9efd16..76feefe6c50 100644 --- a/include/efi.h +++ b/include/efi.h @@ -175,7 +175,7 @@ enum efi_allocate_type { EFI_ALLOCATE_MAX_ADDRESS, /** * @EFI_ALLOCATE_ADDRESS: - * Allocate a memory block starting at the indicatged adress. + * Allocate a memory block starting at the indicated address. */ EFI_ALLOCATE_ADDRESS, /**
Simplify a few expressions in this function.
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 f1154f73e05..3b1c7528e92 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -206,11 +206,11 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, (carve_desc->num_pages << EFI_PAGE_SHIFT);
/* check whether we're overlapping */ - if ((carve_end <= map_start) || (carve_start >= map_end)) + if (carve_end <= map_start || carve_start >= map_end) return EFI_CARVE_NO_OVERLAP;
/* We're overlapping with non-RAM, warn the caller if desired */ - if (overlap_conventional && (map_desc->type != EFI_CONVENTIONAL_MEMORY)) + if (overlap_conventional && map_desc->type != EFI_CONVENTIONAL_MEMORY) return EFI_CARVE_OVERLAPS_NONRAM;
/* Sanitize carve_start and carve_end to lie within our bounds */
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 ---
(no changes since v2)
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 ---
(no changes since v2)
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; }
/*
Exported functions should be documented in the header file, not the implementation. We tend to make such updates on a piecemeal basis to avoid a 'flag day'. Move some comments related to memory allocation to follow the convention.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - Rebase on early patch
include/efi_loader.h | 77 +++++++++++++++++++++++++++++++---- lib/efi_loader/efi_memory.c | 81 ------------------------------------- 2 files changed, 69 insertions(+), 89 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index ee0cdd36500..00a1259c006 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -758,21 +758,68 @@ efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf, * Return: size in pages */ #define efi_size_in_pages(size) (((size) + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT) -/* Allocate boot service data pool memory */ + +/** + * efi_alloc() - allocate boot-services-data pool-memory + * + * Allocate memory from pool and zero it out. + * + * @size: number of bytes to allocate + * Return: pointer to allocated memory or NULL + */ void *efi_alloc(size_t len); -/* Allocate pages on the specified alignment */ + +/** + * efi_alloc_aligned_pages() - allocate aligned memory pages + * + * @len: len in bytes + * @memory_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); -/* More specific EFI memory allocator, called by EFI payloads */ + +/** + * efi_allocate_pages - allocate memory pages + * + * @type: type of allocation to be performed + * @memory_type: usage type of the allocated memory + * @pages: number of pages to be allocated + * @memory: 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); -/* EFI memory free function. */ + +/** + * efi_free_pages() - free memory pages + * + * @memory: start of the memory area to be freed + * @pages: number of pages to be freed + * Return: status code + */ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages); -/* EFI memory allocator for small allocations */ + +/** + * efi_allocate_pool - allocate memory from pool + * + * @pool_type: 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_uintn_t size, void **buffer); -/* EFI pool memory free function. */ + +/** + * efi_free_pool() - free memory from pool + * + * @buffer: start of memory to be freed + * Return: status code + */ efi_status_t efi_free_pool(void *buffer); + /* Allocate and retrieve EFI memory map */ efi_status_t efi_get_memory_map_alloc(efi_uintn_t *map_size, struct efi_mem_desc **memory_map); @@ -782,7 +829,21 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, efi_uintn_t *map_key, efi_uintn_t *descriptor_size, uint32_t *descriptor_version); -/* Adds a range into the EFI memory map */ + +/** + * 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) + * @size: length in bytes of the memory area + * @memory_type: 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);
/** @@ -793,7 +854,7 @@ 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: type of memory added + * @memory_type: EFI type of memory added * @overlap_conventional: region may only overlap free(conventional) * memory * Return: status code diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 3b1c7528e92..9d0f27dbb3e 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -257,17 +257,6 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, return EFI_CARVE_LOOP_AGAIN; }
-/** - * efi_add_memory_map_pg() - add pages to the memory map - * - * @start: start address, must be a multiple of - * EFI_PAGE_SIZE - * @pages: number of pages to add - * @memory_type: 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, bool overlap_conventional) @@ -381,20 +370,6 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, return EFI_SUCCESS; }
-/** - * 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) - * @size: length in bytes of the memory area - * @memory_type: 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) { u64 pages; @@ -440,15 +415,6 @@ static efi_status_t efi_check_allocated(u64 addr, bool must_be_allocated) return EFI_NOT_FOUND; }
-/** - * efi_allocate_pages - allocate memory pages - * - * @type: type of allocation to be performed - * @memory_type: usage type of the allocated memory - * @pages: number of pages to be allocated - * @memory: 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) @@ -516,13 +482,6 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type, return EFI_SUCCESS; }
-/** - * efi_free_pages() - free memory pages - * - * @memory: start of the memory area to be freed - * @pages: number of pages to be freed - * Return: status code - */ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages) { u64 len; @@ -556,14 +515,6 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages) return ret; }
-/** - * efi_alloc_aligned_pages() - allocate aligned memory pages - * - * @len: len in bytes - * @memory_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) { u64 req_pages = efi_size_in_pages(len); @@ -608,14 +559,6 @@ void *efi_alloc_aligned_pages(u64 len, int memory_type, size_t align) return (void *)(uintptr_t)aligned_mem; }
-/** - * efi_allocate_pool - allocate memory from pool - * - * @pool_type: 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_uintn_t size, void **buffer) { efi_status_t r; @@ -644,14 +587,6 @@ efi_status_t efi_allocate_pool(enum efi_memory_type pool_type, efi_uintn_t size, return r; }
-/** - * efi_alloc() - allocate boot services data pool memory - * - * Allocate memory from pool and zero it out. - * - * @size: number of bytes to allocate - * Return: pointer to allocated memory or NULL - */ void *efi_alloc(size_t size) { void *buf; @@ -666,12 +601,6 @@ void *efi_alloc(size_t size) return buf; }
-/** - * efi_free_pool() - free memory from pool - * - * @buffer: start of memory to be freed - * Return: status code - */ efi_status_t efi_free_pool(void *buffer) { efi_status_t ret; @@ -793,16 +722,6 @@ efi_status_t efi_get_memory_map_alloc(efi_uintn_t *map_size, return ret; }
-/** - * efi_add_known_memory() - add memory types to the EFI memory map - * - * This function is to be used to add different memory types other - * than EFI_CONVENTIONAL_MEMORY to the EFI memory map. The conventional - * memory is handled by the LMB module and gets added to the memory - * map through the LMB module. - * - * This function may be overridden for architectures specific purposes. - */ __weak void efi_add_known_memory(void) { }
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 v3: - Put the header-file in its own section
Changes in v2: - Add new patch to add the EFI-loader API documentation
doc/api/efi.rst | 6 ++++++ include/efi_loader.h | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/doc/api/efi.rst b/doc/api/efi.rst index 43d6f936fb0..49814bcf5ea 100644 --- a/doc/api/efi.rst +++ b/doc/api/efi.rst @@ -54,6 +54,12 @@ drivers, handles, loaded images, and the memory map). .. kernel-doc:: cmd/efidebug.c :internal:
+Overall API +----------- + +.. kernel-doc:: include/efi_loader.h + :internal: + Initialization of the UEFI sub-system -------------------------------------
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 ---
(no changes since v2)
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;
At present efi_memory uses struct efi_mem_desc for each node of its memory list. This is convenient but is not ideal, since:
- it means that the fields are under a 'desc' sub-structure - it includes an unused 'reserved' field - it includes virtual_start which is confusing as it is always the same as physical_start - we must use u64 to store pointers, since that is how they are returned by calls to efi_get_memory_map()
As a first step to tidying this up, create a new, private struct to hold these fields.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - Add new patch to use a separate stuct for memory nodes
lib/efi_loader/efi_memory.c | 49 +++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 7 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 3cece51f030..9e4158edfaf 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -27,9 +27,39 @@ DECLARE_GLOBAL_DATA_PTR;
efi_uintn_t efi_memory_map_key;
+/** + * struct priv_mem_desc - defines an memory region + * + * 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. + * + * @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 priv_mem_desc { + u32 type; + u32 reserved; + efi_physical_addr_t physical_start; + efi_virtual_addr_t virtual_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 efi_mem_desc desc; + struct priv_mem_desc desc; };
#define EFI_CARVE_NO_OVERLAP -1 @@ -116,7 +146,7 @@ static int efi_mem_cmp(void *priv, struct list_head *a, struct list_head *b) * @desc: memory descriptor * Return: end address + 1 */ -static uint64_t desc_get_end(struct efi_mem_desc *desc) +static uint64_t desc_get_end(struct priv_mem_desc *desc) { return desc->physical_start + (desc->num_pages << EFI_PAGE_SHIFT); } @@ -138,8 +168,8 @@ static void efi_mem_sort(void) while (merge_again) { merge_again = false; list_for_each_entry(lmem, &efi_mem, link) { - struct efi_mem_desc *prev; - struct efi_mem_desc *cur; + struct priv_mem_desc *prev; + struct priv_mem_desc *cur; uint64_t pages;
if (!prevmem) { @@ -194,11 +224,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 efi_mem_desc *carve_desc, + struct priv_mem_desc *carve_desc, bool overlap_conventional) { struct efi_mem_list *newmap; - struct efi_mem_desc *map_desc = &map->desc; + struct priv_mem_desc *map_desc = &map->desc; 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; @@ -680,7 +710,12 @@ 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 = lmem->desc; + 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; + memory_map->num_pages = lmem->desc.num_pages; + memory_map->attribute = lmem->desc.attribute; memory_map--; }
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--;
This field is not used. Drop it and set the value to 0 when the memory-map is requested.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
lib/efi_loader/efi_memory.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 518b8cff61d..81e40fc923d 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -35,7 +35,6 @@ efi_uintn_t efi_memory_map_key; * internal format is converted to the external struct efi_mem_desc format. * * @type (enum efi_memory_type): EFI memory-type - * @reserved: unused * @physical_start: Start address of region in physical memory * @num_pages: Number of EFI pages this record covers (each is EFI_PAGE_SIZE * bytes) @@ -43,7 +42,6 @@ efi_uintn_t efi_memory_map_key; */ struct priv_mem_desc { u32 type; - u32 reserved; efi_physical_addr_t physical_start; u64 num_pages; u64 attribute; @@ -705,7 +703,7 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, memory_map = &memory_map[map_entries - 1]; list_for_each_entry(lmem, &efi_mem, link) { memory_map->type = lmem->desc.type; - memory_map->reserved = lmem->desc.reserved; + memory_map->reserved = 0; memory_map->physical_start = lmem->desc.physical_start;
/* virtual and physical are always the same */
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;
Rather than assigning desc and then changing two fields, assign all four fields, for clarity.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
lib/efi_loader/efi_memory.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 83f2e32b9a4..b51bb367899 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -268,9 +268,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 = map->desc; + 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; /* Insert before current entry (descending address order) */ list_add_tail(&newmap->link, &map->link);
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--; }
This struct is really a node in the list, not a list itself. Also it doesn't need an efi_ prefix. Rename it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
lib/efi_loader/efi_memory.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 6e474c88784..dbcba99ae39 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -28,7 +28,7 @@ DECLARE_GLOBAL_DATA_PTR; efi_uintn_t efi_memory_map_key;
/** - * struct efi_mem_list - defines an EFI memory record + * struct mem_node - 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 @@ -43,7 +43,7 @@ efi_uintn_t efi_memory_map_key; * bytes) * @attribute: Memory attributes (see EFI_MEMORY...) */ -struct efi_mem_list { +struct mem_node { struct list_head link; enum efi_memory_type type; efi_physical_addr_t physical_start; @@ -118,8 +118,8 @@ static u64 checksum(struct efi_pool_allocation *alloc) */ 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); + 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) return 0; @@ -135,7 +135,7 @@ static int efi_mem_cmp(void *priv, struct list_head *a, struct list_head *b) * @node: memory node * Return: end address + 1 */ -static uint64_t desc_get_end(struct efi_mem_list *node) +static uint64_t desc_get_end(struct mem_node *node) { return node->physical_start + (node->num_pages << EFI_PAGE_SHIFT); } @@ -147,8 +147,8 @@ static uint64_t desc_get_end(struct efi_mem_list *node) */ static void efi_mem_sort(void) { - struct efi_mem_list *lmem; - struct efi_mem_list *prevmem = NULL; + struct mem_node *lmem; + struct mem_node *prevmem = NULL; bool merge_again = true;
list_sort(NULL, &efi_mem, efi_mem_cmp); @@ -157,8 +157,8 @@ static void efi_mem_sort(void) while (merge_again) { merge_again = false; list_for_each_entry(lmem, &efi_mem, link) { - struct efi_mem_list *prev; - struct efi_mem_list *cur; + struct mem_node *prev; + struct mem_node *cur; uint64_t pages;
if (!prevmem) { @@ -211,12 +211,11 @@ static void efi_mem_sort(void) * In case of EFI_CARVE_OVERLAPS_NONRAM it is the callers responsibility * to re-add the already carved out pages to the mapping. */ -static s64 efi_mem_carve_out(struct efi_mem_list *map, - struct efi_mem_list *carve_desc, +static s64 efi_mem_carve_out(struct mem_node *map, struct mem_node *carve_desc, bool overlap_conventional) { - struct efi_mem_list *newmap; - struct efi_mem_list *map_desc = map; + 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; @@ -278,8 +277,8 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, enum efi_memory_type mem_type, bool overlap_conventional) { - struct efi_mem_list *lmem; - struct efi_mem_list *newlist; + struct mem_node *lmem; + struct mem_node *newlist; bool carve_again; uint64_t carved_pages = 0; struct efi_event *evt; @@ -414,7 +413,7 @@ efi_status_t efi_add_memory_map(u64 start, u64 size, */ static efi_status_t efi_check_allocated(u64 addr, bool must_be_allocated) { - struct efi_mem_list *item; + struct mem_node *item;
list_for_each_entry(item, &efi_mem, link) { u64 start = item->physical_start; @@ -666,7 +665,7 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, { size_t map_entries; efi_uintn_t map_size = 0; - struct efi_mem_list *lmem; + struct mem_node *lmem; efi_uintn_t provided_map_size;
if (!memory_map_size)
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--;
This function is passed the address of a void * so update the argument to match. It is better to have casts in the caller than introduce confusion as to what is passed in.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 2 +- arch/arm/mach-bcm283x/reset.c | 2 +- include/efi_loader.h | 2 +- lib/efi_loader/efi_runtime.c | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c index d2d3e346a36..f9c379e9b06 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c @@ -1272,7 +1272,7 @@ void __efi_runtime EFIAPI efi_reset_system(
efi_status_t efi_reset_system_init(void) { - return efi_add_runtime_mmio(&rstcr, sizeof(*rstcr)); + return efi_add_runtime_mmio((void **)&rstcr, sizeof(*rstcr)); }
#endif diff --git a/arch/arm/mach-bcm283x/reset.c b/arch/arm/mach-bcm283x/reset.c index 1dc7ce50d1d..fd10f01d4f7 100644 --- a/arch/arm/mach-bcm283x/reset.c +++ b/arch/arm/mach-bcm283x/reset.c @@ -85,7 +85,7 @@ void __efi_runtime EFIAPI efi_reset_system( efi_status_t efi_reset_system_init(void) { wdog_regs = (struct bcm2835_wdog_regs *)BCM2835_WDOG_PHYSADDR; - return efi_add_runtime_mmio(&wdog_regs, sizeof(*wdog_regs)); + return efi_add_runtime_mmio((void **)&wdog_regs, sizeof(*wdog_regs)); }
#endif diff --git a/include/efi_loader.h b/include/efi_loader.h index a7228672f27..4c346addae3 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -73,7 +73,7 @@ struct jmp_buf_data; * Call this with mmio_ptr as the _pointer_ to a pointer to an MMIO region * to make it available at runtime */ -efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len); +efi_status_t efi_add_runtime_mmio(void **mmio_ptr, u64 len);
/* * Special case handler for error/abort that just tries to dtrt to get diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 05369c47b01..9d3b940afbd 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -927,7 +927,7 @@ out: * @len: size of the memory-mapped IO region * Returns: status code */ -efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len) +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; @@ -941,7 +941,7 @@ efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len) if (!newmmio) return EFI_OUT_OF_RESOURCES; newmmio->ptr = mmio_ptr; - newmmio->paddr = *(uintptr_t *)mmio_ptr; + newmmio->paddr = (uintptr_t)*(void **)mmio_ptr; newmmio->len = len; list_add_tail(&newmmio->link, &efi_runtime_mmio);
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) {
Now that efi_add_memory_map_pg() uses a address rather than a pointer cast to an int, we can simplify the code here.
This is fix#1/4 for: Update efi_add_memory_map() to use address
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
lib/lmb.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c index 14b9b8466ff..588787d2a90 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -10,7 +10,6 @@ #include <efi_loader.h> #include <event.h> #include <image.h> -#include <mapmem.h> #include <lmb.h> #include <log.h> #include <malloc.h> @@ -448,7 +447,6 @@ static bool lmb_should_notify(enum lmb_flags flags) static int lmb_map_update_notify(phys_addr_t addr, phys_size_t size, u8 op, enum lmb_flags flags) { - u64 efi_addr; u64 pages; efi_status_t status;
@@ -460,11 +458,10 @@ static int lmb_map_update_notify(phys_addr_t addr, phys_size_t size, u8 op, if (!lmb_should_notify(flags)) return 0;
- efi_addr = (uintptr_t)map_sysmem(addr, 0); - pages = efi_size_in_pages(size + (efi_addr & EFI_PAGE_MASK)); - efi_addr &= ~EFI_PAGE_MASK; + pages = efi_size_in_pages(size + (addr & EFI_PAGE_MASK)); + addr &= ~EFI_PAGE_MASK;
- status = efi_add_memory_map_pg(efi_addr, pages, + status = efi_add_memory_map_pg(addr, pages, op == MAP_OP_RESERVE ? EFI_BOOT_SERVICES_DATA : EFI_CONVENTIONAL_MEMORY, @@ -474,7 +471,6 @@ static int lmb_map_update_notify(phys_addr_t addr, phys_size_t size, u8 op, status & ~EFI_ERROR_MASK); return -1; } - unmap_sysmem((void *)(uintptr_t)efi_addr);
return 0; }
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
The efi_allocate_pool() call uses a pointer to refer to memory, but efi_allocate_pages() uses an int.
This causes quite a bit of confusion, since sandbox has a distinction between pointers and addresses.
Adjust efi_allocate/free_pages_ext() to handle the conversions.
For the efi_memory implementation, use addresses, to avoid the messy conversions.
In general this simplifies the code, but the main benefit is that casts are no-longer needed in most places, so the compiler can check that we are doing the right thing.
Update the memset() parameter to be a char while we are here.
This is fix#3/4 for: Update efi_add_memory_map() to use address
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
include/efi_loader.h | 4 ++-- lib/efi_loader/efi_bootmgr.c | 2 +- lib/efi_loader/efi_boottime.c | 40 ++++++++++++++++++++++--------- lib/efi_loader/efi_image_loader.c | 3 ++- lib/efi_loader/efi_memory.c | 38 ++++++++++------------------- lib/efi_loader/efi_var_mem.c | 6 ++--- 6 files changed, 49 insertions(+), 44 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index f1ae29b04d7..4e34e1caede 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -786,7 +786,7 @@ void *efi_alloc_aligned_pages(u64 len, enum efi_memory_type mem_type, * @type: type of allocation to be performed * @mem_type: usage type of the allocated memory * @pages: number of pages to be allocated - * @memoryp: returns a pointer to the allocated memory + * @memoryp: returns the allocated address * Return: status code */ efi_status_t efi_allocate_pages(enum efi_allocate_type type, @@ -796,7 +796,7 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type, /** * efi_free_pages() - free memory pages * - * @memory: start of the memory area to be freed + * @memory: start-address of the memory area to be freed * @pages: number of pages to be freed * Return: status code */ diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index bb635d77b53..b05d7e2ed54 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -1302,7 +1302,7 @@ efi_status_t efi_bootmgr_run(void *fdt) if (!IS_ENABLED(CONFIG_GENERATE_ACPI_TABLE)) { free(fdt_lo); if (fdt_distro) - efi_free_pages((uintptr_t)fdt_distro, + efi_free_pages(map_to_sysmem(fdt_distro), efi_size_in_pages(fdt_size)); }
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 8f311f95d1f..84acf17dc95 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -13,6 +13,7 @@ #include <irq_func.h> #include <log.h> #include <malloc.h> +#include <mapmem.h> #include <pe.h> #include <time.h> #include <u-boot/crc.h> @@ -416,7 +417,7 @@ static void EFIAPI efi_restore_tpl(efi_uintn_t old_tpl) * @type: type of allocation to be performed * @mem_type: usage type of the allocated memory * @pages: number of pages to be allocated - * @memoryp: allocated memory + * @memoryp: allocated memory (a pointer, cast to u64) * * This function implements the AllocatePages service. * @@ -430,15 +431,25 @@ static efi_status_t EFIAPI efi_allocate_pages_ext(int type, int mem_type, uint64_t *memoryp) { efi_status_t r; + u64 addr;
EFI_ENTRY("%d, %d, 0x%zx, %p", type, mem_type, pages, memoryp); - r = efi_allocate_pages(type, mem_type, pages, memoryp); + if (!memoryp) + return EFI_INVALID_PARAMETER; + + /* we should not read this unless type indicates it is being used */ + if (type == EFI_ALLOCATE_MAX_ADDRESS || type == EFI_ALLOCATE_ADDRESS) + addr = map_to_sysmem((void *)(uintptr_t)*memoryp); + r = efi_allocate_pages(type, mem_type, pages, &addr); + if (r == EFI_SUCCESS) + *memoryp = (uintptr_t)map_sysmem(addr, pages * EFI_PAGE_SIZE); + return EFI_EXIT(r); }
/** * efi_free_pages_ext() - Free memory pages. - * @memory: start of the memory area to be freed + * @memory: start of the memory area to be freed (a pointer, cast to u64) * @pages: number of pages to be freed * * This function implements the FreePages service. @@ -452,9 +463,12 @@ static efi_status_t EFIAPI efi_free_pages_ext(uint64_t memory, efi_uintn_t pages) { efi_status_t r; + u64 addr;
EFI_ENTRY("%llx, 0x%zx", memory, pages); - r = efi_free_pages(memory, pages); + addr = map_to_sysmem((void *)(uintptr_t)memory); + r = efi_free_pages(addr, pages); + return EFI_EXIT(r); }
@@ -1950,8 +1964,9 @@ efi_status_t efi_load_image_from_file(struct efi_device_path *file_path, { struct efi_file_handle *f; efi_status_t ret; - u64 addr; efi_uintn_t bs; + void *buf; + u64 addr;
/* Open file */ f = efi_file_from_path(file_path); @@ -1977,10 +1992,11 @@ efi_status_t efi_load_image_from_file(struct efi_device_path *file_path, }
/* Read file */ - EFI_CALL(ret = f->read(f, &bs, (void *)(uintptr_t)addr)); + buf = map_sysmem(addr, bs); + EFI_CALL(ret = f->read(f, &bs, buf)); if (ret != EFI_SUCCESS) efi_free_pages(addr, efi_size_in_pages(bs)); - *buffer = (void *)(uintptr_t)addr; + *buffer = buf; *size = bs; error: EFI_CALL(f->close(f)); @@ -2011,6 +2027,7 @@ efi_status_t efi_load_image_from_path(bool boot_policy, uint64_t addr, pages; const efi_guid_t *guid; struct efi_handler *handler; + void *buf;
/* In case of failure nothing is returned */ *buffer = NULL; @@ -2049,15 +2066,16 @@ efi_status_t efi_load_image_from_path(bool boot_policy, ret = EFI_OUT_OF_RESOURCES; goto out; } + buf = map_sysmem(addr, buffer_size); ret = EFI_CALL(load_file_protocol->load_file( load_file_protocol, rem, boot_policy, - &buffer_size, (void *)(uintptr_t)addr)); + &buffer_size, buf)); if (ret != EFI_SUCCESS) efi_free_pages(addr, pages); out: efi_close_protocol(device, guid, efi_root, NULL); if (ret == EFI_SUCCESS) { - *buffer = (void *)(uintptr_t)addr; + *buffer = buf; *size = buffer_size; }
@@ -2120,7 +2138,7 @@ efi_status_t EFIAPI efi_load_image(bool boot_policy, ret = efi_load_pe(*image_obj, dest_buffer, source_size, info); if (!source_buffer) /* Release buffer to which file was loaded */ - efi_free_pages((uintptr_t)dest_buffer, + efi_free_pages(map_to_sysmem(dest_buffer), efi_size_in_pages(source_size)); if (ret == EFI_SUCCESS || ret == EFI_SECURITY_VIOLATION) { info->system_table = &systab; @@ -3321,7 +3339,7 @@ close_next: } }
- efi_free_pages((uintptr_t)loaded_image_protocol->image_base, + efi_free_pages(map_to_sysmem(loaded_image_protocol->image_base), efi_size_in_pages(loaded_image_protocol->image_size)); efi_delete_handle(&image_obj->header);
diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index 0ddf69a0918..2f2587b32a6 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -13,6 +13,7 @@ #include <efi_loader.h> #include <log.h> #include <malloc.h> +#include <mapmem.h> #include <pe.h> #include <sort.h> #include <crypto/mscode.h> @@ -970,7 +971,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, /* Run through relocations */ if (efi_loader_relocate(rel, rel_size, efi_reloc, (unsigned long)image_base) != EFI_SUCCESS) { - efi_free_pages((uintptr_t) efi_reloc, + efi_free_pages(map_to_sysmem(efi_reloc), (virt_size + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT); ret = EFI_LOAD_ERROR; goto err; diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 157ba5ec50c..a33c025fa20 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -36,9 +36,7 @@ efi_uintn_t efi_memory_map_key; * * @link: Link to prev/next node in list * @type: EFI memory-type - * @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 + * @base: 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...) @@ -298,7 +296,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 = (uintptr_t)map_sysmem(start, pages * EFI_PAGE_SIZE); + newlist->base = start; newlist->num_pages = pages;
switch (mem_type) { @@ -435,7 +433,7 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type, enum efi_memory_type mem_type, efi_uintn_t pages, uint64_t *memoryp) { - u64 efi_addr, len; + u64 len; uint flags; efi_status_t ret; phys_addr_t addr; @@ -462,8 +460,7 @@ 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)*memoryp); - addr = (u64)lmb_alloc_base_flags(len, EFI_PAGE_SIZE, addr, + addr = (u64)lmb_alloc_base_flags(len, EFI_PAGE_SIZE, *memoryp, flags); if (!addr) return EFI_OUT_OF_RESOURCES; @@ -472,8 +469,7 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type, if (*memoryp & EFI_PAGE_MASK) return EFI_NOT_FOUND;
- addr = map_to_sysmem((void *)(uintptr_t)*memoryp); - addr = (u64)lmb_alloc_addr_flags(addr, len, flags); + addr = (u64)lmb_alloc_addr_flags(*memoryp, len, flags); if (!addr) return EFI_NOT_FOUND; break; @@ -482,17 +478,15 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type, return EFI_INVALID_PARAMETER; }
- 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, mem_type, true); + ret = efi_add_memory_map_pg(addr, pages, mem_type, true); if (ret != EFI_SUCCESS) { /* Map would overlap, bail out */ lmb_free_flags(addr, (u64)pages << EFI_PAGE_SHIFT, flags); - unmap_sysmem((void *)(uintptr_t)efi_addr); return EFI_OUT_OF_RESOURCES; }
- *memoryp = efi_addr; + *memoryp = addr;
return EFI_SUCCESS; } @@ -515,18 +509,10 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages) }
len = (u64)pages << EFI_PAGE_SHIFT; - /* - * The 'memory' variable for sandbox holds a pointer which has already - * been mapped with map_sysmem() from efi_allocate_pages(). Convert - * it back to an address LMB understands - */ - status = lmb_free_flags(map_to_sysmem((void *)(uintptr_t)memory), len, - LMB_NOOVERWRITE); + status = lmb_free_flags(memory, len, LMB_NOOVERWRITE); if (status) return EFI_NOT_FOUND;
- unmap_sysmem((void *)(uintptr_t)memory); - return ret; }
@@ -551,7 +537,7 @@ void *efi_alloc_aligned_pages(u64 len, enum efi_memory_type mem_type, if (align < EFI_PAGE_SIZE) { r = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, mem_type, req_pages, &mem); - return (r == EFI_SUCCESS) ? (void *)(uintptr_t)mem : NULL; + return (r == EFI_SUCCESS) ? map_sysmem(mem, len) : NULL; }
r = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, mem_type, @@ -572,7 +558,7 @@ void *efi_alloc_aligned_pages(u64 len, enum efi_memory_type mem_type, efi_free_pages(mem, free_pages); }
- return (void *)(uintptr_t)aligned_mem; + return map_sysmem(aligned_mem, len); }
efi_status_t efi_allocate_pool(enum efi_memory_type mem_type, efi_uintn_t size, @@ -595,7 +581,7 @@ efi_status_t efi_allocate_pool(enum efi_memory_type mem_type, efi_uintn_t size, 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; + alloc = map_sysmem(addr, size); alloc->num_pages = num_pages; alloc->checksum = checksum(alloc); *buffer = alloc->data; @@ -641,7 +627,7 @@ efi_status_t efi_free_pool(void *buffer) /* Avoid double free */ alloc->checksum = 0;
- ret = efi_free_pages((uintptr_t)alloc, alloc->num_pages); + ret = efi_free_pages(map_to_sysmem(alloc), alloc->num_pages);
return ret; } diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c index 5c69a1e0f3e..a96a1805865 100644 --- a/lib/efi_loader/efi_var_mem.c +++ b/lib/efi_loader/efi_var_mem.c @@ -7,6 +7,7 @@
#include <efi_loader.h> #include <efi_variable.h> +#include <mapmem.h> #include <u-boot/crc.h>
/* @@ -227,9 +228,8 @@ efi_status_t efi_var_mem_init(void) 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 = map_sysmem(memory, EFI_VAR_BUF_SIZE); + memset(efi_var_buf, '\0', EFI_VAR_BUF_SIZE); efi_var_buf->magic = EFI_VAR_FILE_MAGIC; efi_var_buf->length = (uintptr_t)efi_var_buf->var - (uintptr_t)efi_var_buf;
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);
Update this function to work correctly with sandbox
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
lib/efi_loader/efi_helper.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c index fd89ef6036f..72902a5ade3 100644 --- a/lib/efi_loader/efi_helper.c +++ b/lib/efi_loader/efi_helper.c @@ -486,12 +486,11 @@ 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; + new_fdt = map_sysmem(new_fdt_addr, fdt_size); memcpy(new_fdt, fdt, fdt_totalsize(fdt)); fdt_set_totalsize(new_fdt, fdt_size);
- *fdtp = (void *)(uintptr_t)new_fdt_addr; + *fdtp = new_fdt; done: return ret; }
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;
Update this function to use map_sysmem() so that it can work correctly on sandbox
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
lib/efi_loader/efi_bootmgr.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index b4ea6b817b6..98799aead84 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -14,6 +14,7 @@ #include <efi.h> #include <log.h> #include <malloc.h> +#include <mapmem.h> #include <net.h> #include <efi_loader.h> #include <efi_variable.h> @@ -497,6 +498,7 @@ static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp, * If the file is PE-COFF image, load the downloaded file. */ uri_len = strlen(uridp->uri); + source_buffer = map_sysmem(image_addr, image_size); if (!strncmp(&uridp->uri[uri_len - 4], ".iso", 4) || !strncmp(&uridp->uri[uri_len - 4], ".img", 4)) { ret = prepare_loaded_image(lo_label, image_addr, image_size, @@ -506,21 +508,19 @@ 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) { + } else if (efi_check_pe(source_buffer, image_size, NULL) == + EFI_SUCCESS) { /* * loaded_dp must exist until efi application returns, * will be freed in return_to_efibootmgr event callback. */ loaded_dp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, - (uintptr_t)image_addr, image_size); + image_addr, image_size); ret = efi_install_multiple_protocol_interfaces( &mem_handle, &efi_guid_device_path, loaded_dp, NULL); if (ret != EFI_SUCCESS) goto err;
- source_buffer = (void *)image_addr; source_size = image_size; } else { log_err("Error: file type is not supported\n");
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)
This function should take a pointer, not an address. Update it along with all users.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: - Add comment to struct efi_device_path_memory - Use a pointer for the values in struct efi_device_path_memory
Changes in v2: - 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
include/efi_api.h | 10 ++++++++++ include/efi_loader.h | 5 ++--- lib/efi_loader/efi_bootbin.c | 3 +-- lib/efi_loader/efi_bootmgr.c | 2 +- lib/efi_loader/efi_device_path.c | 20 ++++++++++---------- 5 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/include/efi_api.h b/include/efi_api.h index f07d074f93b..f7da915b0e4 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -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; diff --git a/include/efi_loader.h b/include/efi_loader.h index 4e34e1caede..1269907fa3c 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -919,9 +919,8 @@ 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(enum efi_memory_type mem_type, - uint64_t start_address, - size_t size); +struct efi_device_path *efi_dp_from_mem(enum efi_memory_type 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( const struct efi_device_path *dp); diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index a87006b3c0e..7e7a6bf31aa 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -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. diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 98799aead84..e3b8dfb6013 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -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) diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 9c8cd35b97b..5a93e10fb8e 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -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,9 +976,8 @@ 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(enum efi_memory_type mem_type, - uint64_t start_address, - size_t size) +struct efi_device_path *efi_dp_from_mem(enum efi_memory_type memory_type, + void *start_ptr, size_t size) { struct efi_device_path_memory *mdp; void *buf, *start; @@ -990,9 +990,9 @@ struct efi_device_path *efi_dp_from_mem(enum efi_memory_type mem_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 = mem_type; - mdp->start_address = start_address; - mdp->end_address = start_address + size; + mdp->memory_type = memory_type; + 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")) {
participants (1)
-
Simon Glass