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 ---

(no changes since v3)

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 *

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 v3)

Changes in v3: - Show the returned address rather than the pointer

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

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

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 080e7f78ae3..f27b3827ed2 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> @@ -511,6 +512,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 = %llx\n", (u64)map_to_sysmem(*buffer)); + return EFI_EXIT(r); }

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) { }

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 f27b3827ed2..334a4d64b3f 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -415,9 +415,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. * @@ -426,14 +426,14 @@ static void EFIAPI efi_restore_tpl(efi_uintn_t old_tpl) * * Return: status code */ -static efi_status_t EFIAPI efi_allocate_pages_ext(int type, int memory_type, +static efi_status_t EFIAPI efi_allocate_pages_ext(int type, int mem_type, efi_uintn_t pages, - uint64_t *memory) + uint64_t *memoryp) { efi_status_t r;

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

diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index ee387e1dfd4..9c8cd35b97b 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -975,7 +975,7 @@ struct efi_device_path __maybe_unused *efi_dp_from_eth(void) }

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

(no changes since v1)

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

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

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

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

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

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

(no changes since v1)

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

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

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

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

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

(no changes since v1)

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

(no changes since v1)

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

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

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

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

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

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

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

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

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

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

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

When logging pool allocations, show the address of the pointer, not the pointer itself. This makes it easier to debug with sandbox

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

Changes in v4: - Add new patch to show the address for pool allocations

lib/efi_loader/efi_boottime.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 334a4d64b3f..3ea239fda41 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -533,7 +533,7 @@ static efi_status_t EFIAPI efi_free_pool_ext(void *buffer) { efi_status_t r;

- EFI_ENTRY("%p", buffer); + EFI_ENTRY("%llx", (u64)map_to_sysmem(buffer)); r = efi_free_pool(buffer); return EFI_EXIT(r); }

This collects together several v3 patches into one, to avoid any temporary test-breakage which would make future bisecting difficult.

For efi_memory, 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.

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

For lmb, now that efi_add_memory_map_pg() uses a address rather than a pointer cast to an int, we can simplify the code here.

For efi_reserve_memory(), use addresses rather than pointers, so that it doesn't have to use mapmem.

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.

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

Update the memset() parameter to be a char while we are here.

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

Changes in v4: - Combine the various pointer-to-address patches into one

include/efi_loader.h | 15 ++++++------ lib/efi_loader/efi_bootmgr.c | 3 ++- lib/efi_loader/efi_boottime.c | 39 ++++++++++++++++++++++--------- lib/efi_loader/efi_dt_fixup.c | 4 ---- lib/efi_loader/efi_image_loader.c | 3 ++- lib/efi_loader/efi_memory.c | 38 ++++++++++-------------------- lib/efi_loader/efi_runtime.c | 3 ++- lib/efi_loader/efi_var_mem.c | 6 ++--- lib/lmb.c | 10 +++----- 9 files changed, 59 insertions(+), 62 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h index 4c346addae3..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 */ @@ -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_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index bb635d77b53..30d4ce09e7e 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> @@ -1302,7 +1303,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 3ea239fda41..e81a6d38db8 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -417,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. * @@ -431,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. @@ -453,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); }

@@ -1951,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); @@ -1978,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)); @@ -2012,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; @@ -2050,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; }

@@ -2121,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; @@ -3322,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_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 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 b3ba67413ee..5a6794652d1 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...) @@ -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; @@ -626,7 +612,7 @@ efi_status_t efi_free_pool(void *buffer) if (!buffer) return EFI_INVALID_PARAMETER;

- ret = efi_check_allocated((uintptr_t)buffer, true); + ret = efi_check_allocated(map_to_sysmem(buffer), true); if (ret != EFI_SUCCESS) return ret;

@@ -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_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); 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; 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; }

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 30d4ce09e7e..9383adbd958 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -366,8 +366,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) { @@ -402,7 +400,6 @@ static efi_status_t efi_bootmgr_release_uridp(struct uridp_context *ctx)

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

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

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

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

(no changes since v3)

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")) {