From cf2b0f102cdf912eedb87b10271fa0ad582cf2c1 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Mon, 17 Nov 2014 13:46:44 +0100 Subject: efi: efistub: allow allocation alignment larger than EFI_PAGE_SIZE On systems with 64 KB pages, it is preferable for UEFI memory map entries to be 64 KB aligned multiples of 64 KB, because it relieves us of having to deal with the residues. So, if EFI_ALLOC_ALIGN is #define'd by the platform, use it to round up all memory allocations made. Acked-by: Matt Fleming Acked-by: Borislav Petkov Tested-by: Leif Lindholm Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/libstub/efi-stub-helper.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) (limited to 'drivers/firmware/efi/libstub/efi-stub-helper.c') diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c index a920fec8fe88..e766df60fbfb 100644 --- a/drivers/firmware/efi/libstub/efi-stub-helper.c +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c @@ -32,6 +32,15 @@ static unsigned long __chunk_size = EFI_READ_CHUNK_SIZE; +/* + * Allow the platform to override the allocation granularity: this allows + * systems that have the capability to run with a larger page size to deal + * with the allocations for initrd and fdt more efficiently. + */ +#ifndef EFI_ALLOC_ALIGN +#define EFI_ALLOC_ALIGN EFI_PAGE_SIZE +#endif + struct file_info { efi_file_handle_t *handle; u64 size; @@ -150,10 +159,10 @@ efi_status_t efi_high_alloc(efi_system_table_t *sys_table_arg, * a specific address. We are doing page-based allocations, * so we must be aligned to a page. */ - if (align < EFI_PAGE_SIZE) - align = EFI_PAGE_SIZE; + if (align < EFI_ALLOC_ALIGN) + align = EFI_ALLOC_ALIGN; - nr_pages = round_up(size, EFI_PAGE_SIZE) / EFI_PAGE_SIZE; + nr_pages = round_up(size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE; again: for (i = 0; i < map_size / desc_size; i++) { efi_memory_desc_t *desc; @@ -235,10 +244,10 @@ efi_status_t efi_low_alloc(efi_system_table_t *sys_table_arg, * a specific address. We are doing page-based allocations, * so we must be aligned to a page. */ - if (align < EFI_PAGE_SIZE) - align = EFI_PAGE_SIZE; + if (align < EFI_ALLOC_ALIGN) + align = EFI_ALLOC_ALIGN; - nr_pages = round_up(size, EFI_PAGE_SIZE) / EFI_PAGE_SIZE; + nr_pages = round_up(size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE; for (i = 0; i < map_size / desc_size; i++) { efi_memory_desc_t *desc; unsigned long m = (unsigned long)map; @@ -292,7 +301,7 @@ void efi_free(efi_system_table_t *sys_table_arg, unsigned long size, if (!size) return; - nr_pages = round_up(size, EFI_PAGE_SIZE) / EFI_PAGE_SIZE; + nr_pages = round_up(size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE; efi_call_early(free_pages, addr, nr_pages); } @@ -561,7 +570,7 @@ efi_status_t efi_relocate_kernel(efi_system_table_t *sys_table_arg, * to the preferred address. If that fails, allocate as low * as possible while respecting the required alignment. */ - nr_pages = round_up(alloc_size, EFI_PAGE_SIZE) / EFI_PAGE_SIZE; + nr_pages = round_up(alloc_size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE; status = efi_call_early(allocate_pages, EFI_ALLOCATE_ADDRESS, EFI_LOADER_DATA, nr_pages, &efi_addr); -- cgit v1.2.3 From ddeeefe2dfbe1fa6b116b9362b1bec465b64c873 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Mon, 12 Jan 2015 20:28:20 +0000 Subject: arm64/efi: efistub: Apply __init annotation This ensures all stub component are freed when the kernel proper is done booting, by prefixing the names of all ELF sections that have the SHF_ALLOC attribute with ".init". This approach ensures that even implicitly emitted allocated data (like initializer values and string literals) are covered. At the same time, remove some __init annotations in the stub that have now become redundant, and add the __init annotation to handle_kernel_image which will now trigger a section mismatch warning without it. Signed-off-by: Ard Biesheuvel Signed-off-by: Matt Fleming --- arch/arm64/kernel/efi-stub.c | 14 +++++++------- drivers/firmware/efi/libstub/Makefile | 14 ++++++++++++++ drivers/firmware/efi/libstub/arm-stub.c | 8 ++++---- drivers/firmware/efi/libstub/efi-stub-helper.c | 2 +- 4 files changed, 26 insertions(+), 12 deletions(-) (limited to 'drivers/firmware/efi/libstub/efi-stub-helper.c') diff --git a/arch/arm64/kernel/efi-stub.c b/arch/arm64/kernel/efi-stub.c index d27dd982ff26..f5374065ad53 100644 --- a/arch/arm64/kernel/efi-stub.c +++ b/arch/arm64/kernel/efi-stub.c @@ -13,13 +13,13 @@ #include #include -efi_status_t handle_kernel_image(efi_system_table_t *sys_table, - unsigned long *image_addr, - unsigned long *image_size, - unsigned long *reserve_addr, - unsigned long *reserve_size, - unsigned long dram_base, - efi_loaded_image_t *image) +efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table, + unsigned long *image_addr, + unsigned long *image_size, + unsigned long *reserve_addr, + unsigned long *reserve_size, + unsigned long dram_base, + efi_loaded_image_t *image) { efi_status_t status; unsigned long kernel_size, kernel_memsize = 0; diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile index b14bc2b9fb4d..8902f52e0998 100644 --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/libstub/Makefile @@ -24,3 +24,17 @@ lib-y := efi-stub-helper.o lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o CFLAGS_fdt.o += -I$(srctree)/scripts/dtc/libfdt/ + +# +# arm64 puts the stub in the kernel proper, which will unnecessarily retain all +# code indefinitely unless it is annotated as __init/__initdata/__initconst etc. +# So let's apply the __init annotations at the section level, by prefixing +# the section names directly. This will ensure that even all the inline string +# literals are covered. +# +extra-$(CONFIG_ARM64) := $(lib-y) +lib-$(CONFIG_ARM64) := $(patsubst %.o,%.init.o,$(lib-y)) + +OBJCOPYFLAGS := --prefix-alloc-sections=.init +$(obj)/%.init.o: $(obj)/%.o FORCE + $(call if_changed,objcopy) diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c index 75ee05964cbc..a1fda71c425a 100644 --- a/drivers/firmware/efi/libstub/arm-stub.c +++ b/drivers/firmware/efi/libstub/arm-stub.c @@ -17,10 +17,10 @@ #include "efistub.h" -static int __init efi_secureboot_enabled(efi_system_table_t *sys_table_arg) +static int efi_secureboot_enabled(efi_system_table_t *sys_table_arg) { - static efi_guid_t const var_guid __initconst = EFI_GLOBAL_VARIABLE_GUID; - static efi_char16_t const var_name[] __initconst = { + static efi_guid_t const var_guid = EFI_GLOBAL_VARIABLE_GUID; + static efi_char16_t const var_name[] = { 'S', 'e', 'c', 'u', 'r', 'e', 'B', 'o', 'o', 't', 0 }; efi_get_variable_t *f_getvar = sys_table_arg->runtime->get_variable; @@ -164,7 +164,7 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table, * for both archictectures, with the arch-specific code provided in the * handle_kernel_image() function. */ -unsigned long __init efi_entry(void *handle, efi_system_table_t *sys_table, +unsigned long efi_entry(void *handle, efi_system_table_t *sys_table, unsigned long *image_addr) { efi_loaded_image_t *image; diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c index a920fec8fe88..9bd9fbb5bea8 100644 --- a/drivers/firmware/efi/libstub/efi-stub-helper.c +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c @@ -101,7 +101,7 @@ fail: } -unsigned long __init get_dram_base(efi_system_table_t *sys_table_arg) +unsigned long get_dram_base(efi_system_table_t *sys_table_arg) { efi_status_t status; unsigned long map_size; -- cgit v1.2.3 From d1a8d66b9177105e898e73716f97eb61842c457a Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Thu, 8 Jan 2015 17:51:47 +0000 Subject: efi/libstub: Call get_memory_map() to obtain map and desc sizes This fixes two minor issues in the implementation of get_memory_map(): - Currently, it assumes that sizeof(efi_memory_desc_t) == desc_size, which is usually true, but not mandated by the spec. (This was added intentionally to allow future additions to the definition of efi_memory_desc_t). The way the loop is implemented currently, the added slack space may be insufficient if desc_size is larger, which in some corner cases could result in the loop never terminating. - It allocates 32 efi_memory_desc_t entries first (again, using the size of the struct instead of desc_size), and frees and reallocates if it turns out to be insufficient. Few implementations of UEFI have such small memory maps, which results in a unnecessary allocate/free pair on each invocation. Fix this by calling the get_memory_map() boot service first with a '0' input value for map size to retrieve the map size and desc size from the firmware and only then perform the allocation, using desc_size rather than sizeof(efi_memory_desc_t). Signed-off-by: Ard Biesheuvel Signed-off-by: Matt Fleming --- drivers/firmware/efi/libstub/efi-stub-helper.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'drivers/firmware/efi/libstub/efi-stub-helper.c') diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c index 9bd9fbb5bea8..d073e3946383 100644 --- a/drivers/firmware/efi/libstub/efi-stub-helper.c +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c @@ -66,25 +66,29 @@ efi_status_t efi_get_memory_map(efi_system_table_t *sys_table_arg, unsigned long key; u32 desc_version; - *map_size = sizeof(*m) * 32; -again: + *map_size = 0; + *desc_size = 0; + key = 0; + status = efi_call_early(get_memory_map, map_size, NULL, + &key, desc_size, &desc_version); + if (status != EFI_BUFFER_TOO_SMALL) + return EFI_LOAD_ERROR; + /* * Add an additional efi_memory_desc_t because we're doing an * allocation which may be in a new descriptor region. */ - *map_size += sizeof(*m); + *map_size += *desc_size; status = efi_call_early(allocate_pool, EFI_LOADER_DATA, *map_size, (void **)&m); if (status != EFI_SUCCESS) goto fail; - *desc_size = 0; - key = 0; status = efi_call_early(get_memory_map, map_size, m, &key, desc_size, &desc_version); if (status == EFI_BUFFER_TOO_SMALL) { efi_call_early(free_pool, m); - goto again; + return EFI_LOAD_ERROR; } if (status != EFI_SUCCESS) -- cgit v1.2.3 From 43a9f69692b232d1c64c913a27507eb14a1c47fd Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Fri, 13 Feb 2015 15:46:56 +0000 Subject: Revert "efi/libstub: Call get_memory_map() to obtain map and desc sizes" This reverts commit d1a8d66b9177105e898e73716f97eb61842c457a. Ard reported a boot failure when running UEFI under Qemu and Xen and experimenting with various Tianocore build options, "As it turns out, when allocating room for the UEFI memory map using UEFI's AllocatePool (), it may result in two new memory map entries being created, for instance, when using Tianocore's preallocated region feature. For example, the following region 0x00005ead5000-0x00005ebfffff [Conventional Memory| | | | | |WB|WT|WC|UC] may be split like this 0x00005ead5000-0x00005eae2fff [Conventional Memory| | | | | |WB|WT|WC|UC] 0x00005eae3000-0x00005eae4fff [Loader Data | | | | | |WB|WT|WC|UC] 0x00005eae5000-0x00005ebfffff [Conventional Memory| | | | | |WB|WT|WC|UC] if the preallocated Loader Data region was chosen to be right in the middle of the original free space. After patch d1a8d66b9177 ("efi/libstub: Call get_memory_map() to obtain map and desc sizes"), this is not being dealt with correctly anymore, as the existing logic to allocate room for a single additional entry has become insufficient." Mark requested to reinstate the old loop we had before commit d1a8d66b9177, which grows the memory map buffer until it's big enough to hold the EFI memory map. Acked-by: Ard Biesheuvel Acked-by: Mark Rutland Signed-off-by: Matt Fleming --- drivers/firmware/efi/libstub/efi-stub-helper.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) (limited to 'drivers/firmware/efi/libstub/efi-stub-helper.c') diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c index d073e3946383..9bd9fbb5bea8 100644 --- a/drivers/firmware/efi/libstub/efi-stub-helper.c +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c @@ -66,29 +66,25 @@ efi_status_t efi_get_memory_map(efi_system_table_t *sys_table_arg, unsigned long key; u32 desc_version; - *map_size = 0; - *desc_size = 0; - key = 0; - status = efi_call_early(get_memory_map, map_size, NULL, - &key, desc_size, &desc_version); - if (status != EFI_BUFFER_TOO_SMALL) - return EFI_LOAD_ERROR; - + *map_size = sizeof(*m) * 32; +again: /* * Add an additional efi_memory_desc_t because we're doing an * allocation which may be in a new descriptor region. */ - *map_size += *desc_size; + *map_size += sizeof(*m); status = efi_call_early(allocate_pool, EFI_LOADER_DATA, *map_size, (void **)&m); if (status != EFI_SUCCESS) goto fail; + *desc_size = 0; + key = 0; status = efi_call_early(get_memory_map, map_size, m, &key, desc_size, &desc_version); if (status == EFI_BUFFER_TOO_SMALL) { efi_call_early(free_pool, m); - return EFI_LOAD_ERROR; + goto again; } if (status != EFI_SUCCESS) -- cgit v1.2.3