linux-efi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/19] EFI stub early spring cleaning part 2
@ 2020-02-10 16:02 Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 01/19] efi/libstub/x86: Remove pointless zeroing of apm_bios_info Ard Biesheuvel
                   ` (18 more replies)
  0 siblings, 19 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

This series is a second pass over the EFI stub code to clean it up and
prepare it for some future changes:
- loading the initrd in a different way, which should remove the need for
  bootloaders to have knowledge about the initrd allocation policies and
  about how to pass the base and size to the kernel proper,
- support for RISC-V will be arriving soon, which makes this a good time to
  do some janitorial work to avoid RISC-V from inheriting legacy that only
  exists for the benefit of other architectures.

First of all, code has been moved into separate source files where appropriate,
so that we can benefit from the fact that the EFI stub is a true static library,
and so only the objects that are needed to satisfy symbol dependencies are
incorporated into the final build. We also move x86's eboot.c into libstub/
so we can use the same cflags and make rules across the entire library.
(#5, #7, #9, #12, #13, #14)

Patches #6, #15 and #17 clean up the memory allocation, file I/O and command
line parsing routines that are shared by all architectures, to remove open
coded logic that is already implemented elsewhere, either in the firmware or
in code that we can incorporate from the kernel proper.

Patches #10, #11 and #16 deal with upper limits for memory allocations.

Patches #18 and #19 adds the plumbing to enable us to locate device paths and
invoke the EFI LoadFile2 protocol.

The remaining patches are cleanups or minor bug fixes across the board.

Note that this series applies onto Arvind's x86/boot cleanups [0]. Full
branch can be found here [1]

Cc: nivedita@alum.mit.edu
Cc: mingo@kernel.org
Cc: lukas@wunner.de
Cc: atish.patra@wdc.com

[0] https://lore.kernel.org/linux-efi/20200202171353.3736319-1-nivedita@alum.mit.edu/
[1] https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-spring-cleaning-part2

Ard Biesheuvel (19):
  efi/libstub/x86: Remove pointless zeroing of apm_bios_info
  efi/libstub/x86: Avoid overflowing code32_start on PE entry
  efi/libstub: Use hidden visiblity for all source files
  efi/libstub/arm: Relax FDT alignment requirement
  efi/libstub: Move memory map handling and allocation routines to mem.c
  efi/libstub: Simplify efi_high_alloc() and rename to
    efi_allocate_pages()
  efi/libstub/x86: Incorporate eboot.c into libstub
  efi/libstub: Use consistent type names for file I/O protocols
  efi/libstub: Move stub specific declarations into efistub.h
  efi/libstub/x86: Permit bootparams struct to be allocated above 4 GB
  efi/libstub/x86: Permit cmdline data to be allocated above 4 GB
  efi/libstub: Move efi_random_alloc() into separate source file
  efi/libstub: Move get_dram_base() into arm-stub.c
  efi/libstub: Move file I/O support code into separate file
  efi/libstub: Rewrite file I/O routine
  efi/libstub: Take soft and hard memory limits into account for initrd
    loading
  efi/libstub: Clean up command line parsing routine
  efi/libstub: Expose LocateDevicePath boot service
  efi/libstub: Make the LoadFile EFI protocol accessible

 arch/arm64/include/asm/efi.h                  |  10 -
 arch/x86/boot/compressed/Makefile             |   5 +-
 arch/x86/boot/compressed/eboot.h              |  31 -
 arch/x86/include/asm/efi.h                    |   9 +-
 drivers/firmware/efi/libstub/Makefile         |   6 +-
 drivers/firmware/efi/libstub/arm-stub.c       |  47 +-
 drivers/firmware/efi/libstub/arm64-stub.c     |   8 +-
 .../firmware/efi/libstub/efi-stub-helper.c    | 725 +-----------------
 drivers/firmware/efi/libstub/efistub.h        | 586 +++++++++++++-
 drivers/firmware/efi/libstub/fdt.c            |   7 +-
 drivers/firmware/efi/libstub/file.c           | 258 +++++++
 drivers/firmware/efi/libstub/hidden.h         |   6 +
 drivers/firmware/efi/libstub/mem.c            | 253 ++++++
 drivers/firmware/efi/libstub/random.c         | 114 ---
 drivers/firmware/efi/libstub/randomalloc.c    | 124 +++
 drivers/firmware/efi/libstub/string.c         |  63 ++
 .../firmware/efi/libstub/x86-stub.c           |  79 +-
 include/linux/efi.h                           | 516 +------------
 18 files changed, 1389 insertions(+), 1458 deletions(-)
 delete mode 100644 arch/x86/boot/compressed/eboot.h
 create mode 100644 drivers/firmware/efi/libstub/file.c
 create mode 100644 drivers/firmware/efi/libstub/hidden.h
 create mode 100644 drivers/firmware/efi/libstub/mem.c
 create mode 100644 drivers/firmware/efi/libstub/randomalloc.c
 rename arch/x86/boot/compressed/eboot.c => drivers/firmware/efi/libstub/x86-stub.c (94%)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 01/19] efi/libstub/x86: Remove pointless zeroing of apm_bios_info
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 02/19] efi/libstub/x86: Avoid overflowing code32_start on PE entry Ard Biesheuvel
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

We have some code in the EFI stub entry point that takes the address
of the apm_bios_info struct in the newly allocated and zeroed out
boot_params structure, only to zero it out again. This is pointless
so remove it.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/boot/compressed/eboot.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 32423e83ba8f..4745a1ee7953 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -358,7 +358,6 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 				   efi_system_table_t *sys_table_arg)
 {
 	struct boot_params *boot_params;
-	struct apm_bios_info *bi;
 	struct setup_header *hdr;
 	efi_loaded_image_t *image;
 	efi_guid_t proto = LOADED_IMAGE_PROTOCOL_GUID;
@@ -389,7 +388,6 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	memset(boot_params, 0x0, 0x4000);
 
 	hdr = &boot_params->hdr;
-	bi = &boot_params->apm_bios_info;
 
 	/* Copy the second sector to boot_params */
 	memcpy(&hdr->jump, image->image_base + 512, 512);
@@ -416,9 +414,6 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	hdr->ramdisk_image = 0;
 	hdr->ramdisk_size = 0;
 
-	/* Clear APM BIOS info */
-	memset(bi, 0, sizeof(*bi));
-
 	status = efi_parse_options(cmdline_ptr);
 	if (status != EFI_SUCCESS)
 		goto fail2;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 02/19] efi/libstub/x86: Avoid overflowing code32_start on PE entry
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 01/19] efi/libstub/x86: Remove pointless zeroing of apm_bios_info Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 03/19] efi/libstub: Use hidden visiblity for all source files Ard Biesheuvel
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

When using the native PE entry point (as opposed to the EFI handover
protocol entry point that is used more widely), we set code32_start,
which is a 32-bit wide field, to the effective symbol address of
startup_32, which could overflow given that the EFI loader may have
located the running image anywhere in memory, and we haven't reached
the point yet where we relocate ourselves.

Since we relocate ourselves if code32_start != pref_address, this
isn't likely to lead to problems in practice, given how unlikely
it is that the truncated effective address of startup_32 happens
to equal pref_address. But it is better to defer the assignment
of code32_start to after the relocation, when it is guaranteed to
fit.

While at it, move the call to efi_relocate_kernel() to an earlier
stage so it is more likely that our preferred offset in memory has
not been occupied by other memory allocations done in the mean time.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/boot/compressed/eboot.c | 40 +++++++++-----------
 1 file changed, 18 insertions(+), 22 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 4745a1ee7953..55637135b50c 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -439,8 +439,6 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	boot_params->ext_ramdisk_image = (u64)ramdisk_addr >> 32;
 	boot_params->ext_ramdisk_size  = (u64)ramdisk_size >> 32;
 
-	hdr->code32_start = (u32)(unsigned long)startup_32;
-
 	efi_stub_entry(handle, sys_table, boot_params);
 	/* not reached */
 
@@ -707,6 +705,7 @@ struct boot_params *efi_main(efi_handle_t handle,
 			     efi_system_table_t *sys_table_arg,
 			     struct boot_params *boot_params)
 {
+	unsigned long bzimage_addr = (unsigned long)startup_32;
 	struct setup_header *hdr = &boot_params->hdr;
 	efi_status_t status;
 	unsigned long cmdline_paddr;
@@ -717,6 +716,23 @@ struct boot_params *efi_main(efi_handle_t handle,
 	if (sys_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE)
 		goto fail;
 
+	/*
+	 * If the kernel isn't already loaded at the preferred load
+	 * address, relocate it.
+	 */
+	if (bzimage_addr != hdr->pref_address) {
+		status = efi_relocate_kernel(&bzimage_addr,
+					     hdr->init_size, hdr->init_size,
+					     hdr->pref_address,
+					     hdr->kernel_alignment,
+					     LOAD_PHYSICAL_ADDR);
+		if (status != EFI_SUCCESS) {
+			efi_printk("efi_relocate_kernel() failed!\n");
+			goto fail;
+		}
+	}
+	hdr->code32_start = (u32)bzimage_addr;
+
 	/*
 	 * make_boot_params() may have been called before efi_main(), in which
 	 * case this is the second time we parse the cmdline. This is ok,
@@ -746,26 +762,6 @@ struct boot_params *efi_main(efi_handle_t handle,
 
 	setup_quirks(boot_params);
 
-	/*
-	 * If the kernel isn't already loaded at the preferred load
-	 * address, relocate it.
-	 */
-	if (hdr->pref_address != hdr->code32_start) {
-		unsigned long bzimage_addr = hdr->code32_start;
-		status = efi_relocate_kernel(&bzimage_addr,
-					     hdr->init_size, hdr->init_size,
-					     hdr->pref_address,
-					     hdr->kernel_alignment,
-					     LOAD_PHYSICAL_ADDR);
-		if (status != EFI_SUCCESS) {
-			efi_printk("efi_relocate_kernel() failed!\n");
-			goto fail;
-		}
-
-		hdr->pref_address = hdr->code32_start;
-		hdr->code32_start = bzimage_addr;
-	}
-
 	status = exit_boot(boot_params, handle);
 	if (status != EFI_SUCCESS) {
 		efi_printk("exit_boot() failed!\n");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 03/19] efi/libstub: Use hidden visiblity for all source files
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 01/19] efi/libstub/x86: Remove pointless zeroing of apm_bios_info Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 02/19] efi/libstub/x86: Avoid overflowing code32_start on PE entry Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-24 23:15   ` Atish Patra
  2020-02-10 16:02 ` [PATCH 04/19] efi/libstub/arm: Relax FDT alignment requirement Ard Biesheuvel
                   ` (15 subsequent siblings)
  18 siblings, 1 reply; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

Instead of setting the visibility pragma for a small set of symbol
declarations that could result in absolute references that we cannot
support in the stub, declare hidden visibility for all code in the
EFI stub, which is more robust and future proof.

To ensure that the #pragma is taken into account before any other
includes are processed, put it in a header file of its own and
include it via the compiler command line using the -include option.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/include/asm/efi.h              | 3 ---
 drivers/firmware/efi/libstub/Makefile     | 2 +-
 drivers/firmware/efi/libstub/arm64-stub.c | 8 +-------
 drivers/firmware/efi/libstub/hidden.h     | 6 ++++++
 4 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index 44531a69d32b..56ae87401a26 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -107,9 +107,6 @@ static inline void free_screen_info(struct screen_info *si)
 {
 }
 
-/* redeclare as 'hidden' so the compiler will generate relative references */
-extern struct screen_info screen_info __attribute__((__visibility__("hidden")));
-
 static inline void efifb_setup_from_dmi(struct screen_info *si, const char *opt)
 {
 }
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index f14b7636323a..4efdbd711e8e 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -25,7 +25,7 @@ cflags-$(CONFIG_ARM)		:= $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \
 cflags-$(CONFIG_EFI_ARMSTUB)	+= -I$(srctree)/scripts/dtc/libfdt
 
 KBUILD_CFLAGS			:= $(cflags-y) -DDISABLE_BRANCH_PROFILING \
-				   -D__NO_FORTIFY \
+				   -include hidden.h -D__NO_FORTIFY \
 				   $(call cc-option,-ffreestanding) \
 				   $(call cc-option,-fno-stack-protector) \
 				   -D__DISABLE_EXPORTS
diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index 2915b44132e6..719d03a64329 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -6,17 +6,11 @@
  * Adapted from ARM version by Mark Salter <msalter@redhat.com>
  */
 
-/*
- * To prevent the compiler from emitting GOT-indirected (and thus absolute)
- * references to the section markers, override their visibility as 'hidden'
- */
-#pragma GCC visibility push(hidden)
-#include <asm/sections.h>
-#pragma GCC visibility pop
 
 #include <linux/efi.h>
 #include <asm/efi.h>
 #include <asm/memory.h>
+#include <asm/sections.h>
 #include <asm/sysreg.h>
 
 #include "efistub.h"
diff --git a/drivers/firmware/efi/libstub/hidden.h b/drivers/firmware/efi/libstub/hidden.h
new file mode 100644
index 000000000000..3493b041f419
--- /dev/null
+++ b/drivers/firmware/efi/libstub/hidden.h
@@ -0,0 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * To prevent the compiler from emitting GOT-indirected (and thus absolute)
+ * references to any global symbols, override their visibility as 'hidden'
+ */
+#pragma GCC visibility push(hidden)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 04/19] efi/libstub/arm: Relax FDT alignment requirement
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2020-02-10 16:02 ` [PATCH 03/19] efi/libstub: Use hidden visiblity for all source files Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 05/19] efi/libstub: Move memory map handling and allocation routines to mem.c Ard Biesheuvel
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

The arm64 kernel no longer requires the FDT blob to fit inside a
naturally aligned 2 MB memory block, so remove the code that aligns
the allocation to 2 MB.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/include/asm/efi.h       | 7 -------
 drivers/firmware/efi/libstub/fdt.c | 6 +-----
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index 56ae87401a26..45e821222774 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -57,13 +57,6 @@ efi_status_t __efi_rt_asm_wrapper(void *, const char *, ...);
 
 /* arch specific definitions used by the stub code */
 
-/*
- * AArch64 requires the DTB to be 8-byte aligned in the first 512MiB from
- * start of kernel and may not cross a 2MiB boundary. We set alignment to
- * 2MiB so we know it won't cross a 2MiB boundary.
- */
-#define EFI_FDT_ALIGN	SZ_2M   /* used by allocate_new_fdt_and_exit_boot() */
-
 /*
  * In some configurations (e.g. VMAP_STACK && 64K pages), stacks built into the
  * kernel need greater alignment than we require the segments to be padded to.
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index 0a91e5232127..f71cd54823b7 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -199,10 +199,6 @@ static efi_status_t update_fdt_memmap(void *fdt, struct efi_boot_memmap *map)
 	return EFI_SUCCESS;
 }
 
-#ifndef EFI_FDT_ALIGN
-# define EFI_FDT_ALIGN EFI_PAGE_SIZE
-#endif
-
 struct exit_boot_struct {
 	efi_memory_desc_t	*runtime_map;
 	int			*runtime_entry_count;
@@ -281,7 +277,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
 	pr_efi("Exiting boot services and installing virtual address map...\n");
 
 	map.map = &memory_map;
-	status = efi_high_alloc(MAX_FDT_SIZE, EFI_FDT_ALIGN,
+	status = efi_high_alloc(MAX_FDT_SIZE, EFI_PAGE_SIZE,
 				new_fdt_addr, max_addr);
 	if (status != EFI_SUCCESS) {
 		pr_efi_err("Unable to allocate memory for new device tree.\n");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 05/19] efi/libstub: Move memory map handling and allocation routines to mem.c
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
                   ` (3 preceding siblings ...)
  2020-02-10 16:02 ` [PATCH 04/19] efi/libstub/arm: Relax FDT alignment requirement Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 06/19] efi/libstub: Simplify efi_high_alloc() and rename to efi_allocate_pages() Ard Biesheuvel
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

Create a new source file mem.c to keep the routines involved in memory
allocation and deallocation and manipulation of the EFI memory map.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/Makefile          |   2 +-
 drivers/firmware/efi/libstub/efi-stub-helper.c | 313 -------------------
 drivers/firmware/efi/libstub/mem.c             | 319 ++++++++++++++++++++
 3 files changed, 320 insertions(+), 314 deletions(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 4efdbd711e8e..45d6eb657437 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -39,7 +39,7 @@ OBJECT_FILES_NON_STANDARD	:= y
 KCOV_INSTRUMENT			:= n
 
 lib-y				:= efi-stub-helper.o gop.o secureboot.o tpm.o \
-				   random.o pci.o
+				   mem.o random.o pci.o
 
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 74ddfb496140..60d13c7a2e92 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -53,8 +53,6 @@ bool __pure __efi_soft_reserve_enabled(void)
 	return !efi_nosoftreserve;
 }
 
-#define EFI_MMAP_NR_SLACK_SLOTS	8
-
 struct file_info {
 	efi_file_handle_t *handle;
 	u64 size;
@@ -77,64 +75,6 @@ void efi_printk(char *str)
 	}
 }
 
-static inline bool mmap_has_headroom(unsigned long buff_size,
-				     unsigned long map_size,
-				     unsigned long desc_size)
-{
-	unsigned long slack = buff_size - map_size;
-
-	return slack / desc_size >= EFI_MMAP_NR_SLACK_SLOTS;
-}
-
-efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
-{
-	efi_memory_desc_t *m = NULL;
-	efi_status_t status;
-	unsigned long key;
-	u32 desc_version;
-
-	*map->desc_size =	sizeof(*m);
-	*map->map_size =	*map->desc_size * 32;
-	*map->buff_size =	*map->map_size;
-again:
-	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA,
-			     *map->map_size, (void **)&m);
-	if (status != EFI_SUCCESS)
-		goto fail;
-
-	*map->desc_size = 0;
-	key = 0;
-	status = efi_bs_call(get_memory_map, map->map_size, m,
-			     &key, map->desc_size, &desc_version);
-	if (status == EFI_BUFFER_TOO_SMALL ||
-	    !mmap_has_headroom(*map->buff_size, *map->map_size,
-			       *map->desc_size)) {
-		efi_bs_call(free_pool, m);
-		/*
-		 * Make sure there is some entries of headroom so that the
-		 * buffer can be reused for a new map after allocations are
-		 * no longer permitted.  Its unlikely that the map will grow to
-		 * exceed this headroom once we are ready to trigger
-		 * ExitBootServices()
-		 */
-		*map->map_size += *map->desc_size * EFI_MMAP_NR_SLACK_SLOTS;
-		*map->buff_size = *map->map_size;
-		goto again;
-	}
-
-	if (status != EFI_SUCCESS)
-		efi_bs_call(free_pool, m);
-
-	if (map->key_ptr && status == EFI_SUCCESS)
-		*map->key_ptr = key;
-	if (map->desc_ver && status == EFI_SUCCESS)
-		*map->desc_ver = desc_version;
-
-fail:
-	*map->map = m;
-	return status;
-}
-
 
 unsigned long get_dram_base(void)
 {
@@ -170,192 +110,6 @@ unsigned long get_dram_base(void)
 	return membase;
 }
 
-/*
- * Allocate at the highest possible address that is not above 'max'.
- */
-efi_status_t efi_high_alloc(unsigned long size, unsigned long align,
-			    unsigned long *addr, unsigned long max)
-{
-	unsigned long map_size, desc_size, buff_size;
-	efi_memory_desc_t *map;
-	efi_status_t status;
-	unsigned long nr_pages;
-	u64 max_addr = 0;
-	int i;
-	struct efi_boot_memmap boot_map;
-
-	boot_map.map =		&map;
-	boot_map.map_size =	&map_size;
-	boot_map.desc_size =	&desc_size;
-	boot_map.desc_ver =	NULL;
-	boot_map.key_ptr =	NULL;
-	boot_map.buff_size =	&buff_size;
-
-	status = efi_get_memory_map(&boot_map);
-	if (status != EFI_SUCCESS)
-		goto fail;
-
-	/*
-	 * Enforce minimum alignment that EFI or Linux requires when
-	 * requesting a specific address.  We are doing page-based (or
-	 * larger) allocations, and both the address and size must meet
-	 * alignment constraints.
-	 */
-	if (align < EFI_ALLOC_ALIGN)
-		align = EFI_ALLOC_ALIGN;
-
-	size = round_up(size, EFI_ALLOC_ALIGN);
-	nr_pages = size / EFI_PAGE_SIZE;
-again:
-	for (i = 0; i < map_size / desc_size; i++) {
-		efi_memory_desc_t *desc;
-		unsigned long m = (unsigned long)map;
-		u64 start, end;
-
-		desc = efi_early_memdesc_ptr(m, desc_size, i);
-		if (desc->type != EFI_CONVENTIONAL_MEMORY)
-			continue;
-
-		if (efi_soft_reserve_enabled() &&
-		    (desc->attribute & EFI_MEMORY_SP))
-			continue;
-
-		if (desc->num_pages < nr_pages)
-			continue;
-
-		start = desc->phys_addr;
-		end = start + desc->num_pages * EFI_PAGE_SIZE;
-
-		if (end > max)
-			end = max;
-
-		if ((start + size) > end)
-			continue;
-
-		if (round_down(end - size, align) < start)
-			continue;
-
-		start = round_down(end - size, align);
-
-		/*
-		 * Don't allocate at 0x0. It will confuse code that
-		 * checks pointers against NULL.
-		 */
-		if (start == 0x0)
-			continue;
-
-		if (start > max_addr)
-			max_addr = start;
-	}
-
-	if (!max_addr)
-		status = EFI_NOT_FOUND;
-	else {
-		status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
-				     EFI_LOADER_DATA, nr_pages, &max_addr);
-		if (status != EFI_SUCCESS) {
-			max = max_addr;
-			max_addr = 0;
-			goto again;
-		}
-
-		*addr = max_addr;
-	}
-
-	efi_bs_call(free_pool, map);
-fail:
-	return status;
-}
-
-/*
- * Allocate at the lowest possible address that is not below 'min'.
- */
-efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
-				 unsigned long *addr, unsigned long min)
-{
-	unsigned long map_size, desc_size, buff_size;
-	efi_memory_desc_t *map;
-	efi_status_t status;
-	unsigned long nr_pages;
-	int i;
-	struct efi_boot_memmap boot_map;
-
-	boot_map.map =		&map;
-	boot_map.map_size =	&map_size;
-	boot_map.desc_size =	&desc_size;
-	boot_map.desc_ver =	NULL;
-	boot_map.key_ptr =	NULL;
-	boot_map.buff_size =	&buff_size;
-
-	status = efi_get_memory_map(&boot_map);
-	if (status != EFI_SUCCESS)
-		goto fail;
-
-	/*
-	 * Enforce minimum alignment that EFI or Linux requires when
-	 * requesting a specific address.  We are doing page-based (or
-	 * larger) allocations, and both the address and size must meet
-	 * alignment constraints.
-	 */
-	if (align < EFI_ALLOC_ALIGN)
-		align = EFI_ALLOC_ALIGN;
-
-	size = round_up(size, EFI_ALLOC_ALIGN);
-	nr_pages = size / EFI_PAGE_SIZE;
-	for (i = 0; i < map_size / desc_size; i++) {
-		efi_memory_desc_t *desc;
-		unsigned long m = (unsigned long)map;
-		u64 start, end;
-
-		desc = efi_early_memdesc_ptr(m, desc_size, i);
-
-		if (desc->type != EFI_CONVENTIONAL_MEMORY)
-			continue;
-
-		if (efi_soft_reserve_enabled() &&
-		    (desc->attribute & EFI_MEMORY_SP))
-			continue;
-
-		if (desc->num_pages < nr_pages)
-			continue;
-
-		start = desc->phys_addr;
-		end = start + desc->num_pages * EFI_PAGE_SIZE;
-
-		if (start < min)
-			start = min;
-
-		start = round_up(start, align);
-		if ((start + size) > end)
-			continue;
-
-		status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
-				     EFI_LOADER_DATA, nr_pages, &start);
-		if (status == EFI_SUCCESS) {
-			*addr = start;
-			break;
-		}
-	}
-
-	if (i == map_size / desc_size)
-		status = EFI_NOT_FOUND;
-
-	efi_bs_call(free_pool, map);
-fail:
-	return status;
-}
-
-void efi_free(unsigned long size, unsigned long addr)
-{
-	unsigned long nr_pages;
-
-	if (!size)
-		return;
-
-	nr_pages = round_up(size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
-	efi_bs_call(free_pages, addr, nr_pages);
-}
-
 static efi_status_t efi_file_size(void *__fh, efi_char16_t *filename_16,
 				  void **handle, u64 *file_sz)
 {
@@ -695,73 +449,6 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
 
 	return status;
 }
-/*
- * Relocate a kernel image, either compressed or uncompressed.
- * In the ARM64 case, all kernel images are currently
- * uncompressed, and as such when we relocate it we need to
- * allocate additional space for the BSS segment. Any low
- * memory that this function should avoid needs to be
- * unavailable in the EFI memory map, as if the preferred
- * address is not available the lowest available address will
- * be used.
- */
-efi_status_t efi_relocate_kernel(unsigned long *image_addr,
-				 unsigned long image_size,
-				 unsigned long alloc_size,
-				 unsigned long preferred_addr,
-				 unsigned long alignment,
-				 unsigned long min_addr)
-{
-	unsigned long cur_image_addr;
-	unsigned long new_addr = 0;
-	efi_status_t status;
-	unsigned long nr_pages;
-	efi_physical_addr_t efi_addr = preferred_addr;
-
-	if (!image_addr || !image_size || !alloc_size)
-		return EFI_INVALID_PARAMETER;
-	if (alloc_size < image_size)
-		return EFI_INVALID_PARAMETER;
-
-	cur_image_addr = *image_addr;
-
-	/*
-	 * The EFI firmware loader could have placed the kernel image
-	 * anywhere in memory, but the kernel has restrictions on the
-	 * max physical address it can run at.  Some architectures
-	 * also have a prefered address, so first try to relocate
-	 * to the preferred address.  If that fails, allocate as low
-	 * as possible while respecting the required alignment.
-	 */
-	nr_pages = round_up(alloc_size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
-	status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
-			     EFI_LOADER_DATA, nr_pages, &efi_addr);
-	new_addr = efi_addr;
-	/*
-	 * If preferred address allocation failed allocate as low as
-	 * possible.
-	 */
-	if (status != EFI_SUCCESS) {
-		status = efi_low_alloc_above(alloc_size, alignment, &new_addr,
-					     min_addr);
-	}
-	if (status != EFI_SUCCESS) {
-		pr_efi_err("Failed to allocate usable memory for kernel.\n");
-		return status;
-	}
-
-	/*
-	 * We know source/dest won't overlap since both memory ranges
-	 * have been allocated by UEFI, so we can safely use memcpy.
-	 */
-	memcpy((void *)new_addr, (void *)cur_image_addr, image_size);
-
-	/* Return the new address of the relocated image. */
-	*image_addr = new_addr;
-
-	return status;
-}
-
 /*
  * Get the number of UTF-8 bytes corresponding to an UTF-16 character.
  * This overestimates for surrogates, but that is okay.
diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c
new file mode 100644
index 000000000000..690648a7ca1e
--- /dev/null
+++ b/drivers/firmware/efi/libstub/mem.c
@@ -0,0 +1,319 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/efi.h>
+#include <asm/efi.h>
+
+#include "efistub.h"
+
+#define EFI_MMAP_NR_SLACK_SLOTS	8
+
+static inline bool mmap_has_headroom(unsigned long buff_size,
+				     unsigned long map_size,
+				     unsigned long desc_size)
+{
+	unsigned long slack = buff_size - map_size;
+
+	return slack / desc_size >= EFI_MMAP_NR_SLACK_SLOTS;
+}
+
+efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
+{
+	efi_memory_desc_t *m = NULL;
+	efi_status_t status;
+	unsigned long key;
+	u32 desc_version;
+
+	*map->desc_size =	sizeof(*m);
+	*map->map_size =	*map->desc_size * 32;
+	*map->buff_size =	*map->map_size;
+again:
+	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA,
+			     *map->map_size, (void **)&m);
+	if (status != EFI_SUCCESS)
+		goto fail;
+
+	*map->desc_size = 0;
+	key = 0;
+	status = efi_bs_call(get_memory_map, map->map_size, m,
+			     &key, map->desc_size, &desc_version);
+	if (status == EFI_BUFFER_TOO_SMALL ||
+	    !mmap_has_headroom(*map->buff_size, *map->map_size,
+			       *map->desc_size)) {
+		efi_bs_call(free_pool, m);
+		/*
+		 * Make sure there is some entries of headroom so that the
+		 * buffer can be reused for a new map after allocations are
+		 * no longer permitted.  Its unlikely that the map will grow to
+		 * exceed this headroom once we are ready to trigger
+		 * ExitBootServices()
+		 */
+		*map->map_size += *map->desc_size * EFI_MMAP_NR_SLACK_SLOTS;
+		*map->buff_size = *map->map_size;
+		goto again;
+	}
+
+	if (status != EFI_SUCCESS)
+		efi_bs_call(free_pool, m);
+
+	if (map->key_ptr && status == EFI_SUCCESS)
+		*map->key_ptr = key;
+	if (map->desc_ver && status == EFI_SUCCESS)
+		*map->desc_ver = desc_version;
+
+fail:
+	*map->map = m;
+	return status;
+}
+
+/*
+ * Allocate at the highest possible address that is not above 'max'.
+ */
+efi_status_t efi_high_alloc(unsigned long size, unsigned long align,
+			    unsigned long *addr, unsigned long max)
+{
+	unsigned long map_size, desc_size, buff_size;
+	efi_memory_desc_t *map;
+	efi_status_t status;
+	unsigned long nr_pages;
+	u64 max_addr = 0;
+	int i;
+	struct efi_boot_memmap boot_map;
+
+	boot_map.map =		&map;
+	boot_map.map_size =	&map_size;
+	boot_map.desc_size =	&desc_size;
+	boot_map.desc_ver =	NULL;
+	boot_map.key_ptr =	NULL;
+	boot_map.buff_size =	&buff_size;
+
+	status = efi_get_memory_map(&boot_map);
+	if (status != EFI_SUCCESS)
+		goto fail;
+
+	/*
+	 * Enforce minimum alignment that EFI or Linux requires when
+	 * requesting a specific address.  We are doing page-based (or
+	 * larger) allocations, and both the address and size must meet
+	 * alignment constraints.
+	 */
+	if (align < EFI_ALLOC_ALIGN)
+		align = EFI_ALLOC_ALIGN;
+
+	size = round_up(size, EFI_ALLOC_ALIGN);
+	nr_pages = size / EFI_PAGE_SIZE;
+again:
+	for (i = 0; i < map_size / desc_size; i++) {
+		efi_memory_desc_t *desc;
+		unsigned long m = (unsigned long)map;
+		u64 start, end;
+
+		desc = efi_early_memdesc_ptr(m, desc_size, i);
+		if (desc->type != EFI_CONVENTIONAL_MEMORY)
+			continue;
+
+		if (efi_soft_reserve_enabled() &&
+		    (desc->attribute & EFI_MEMORY_SP))
+			continue;
+
+		if (desc->num_pages < nr_pages)
+			continue;
+
+		start = desc->phys_addr;
+		end = start + desc->num_pages * EFI_PAGE_SIZE;
+
+		if (end > max)
+			end = max;
+
+		if ((start + size) > end)
+			continue;
+
+		if (round_down(end - size, align) < start)
+			continue;
+
+		start = round_down(end - size, align);
+
+		/*
+		 * Don't allocate at 0x0. It will confuse code that
+		 * checks pointers against NULL.
+		 */
+		if (start == 0x0)
+			continue;
+
+		if (start > max_addr)
+			max_addr = start;
+	}
+
+	if (!max_addr)
+		status = EFI_NOT_FOUND;
+	else {
+		status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
+				     EFI_LOADER_DATA, nr_pages, &max_addr);
+		if (status != EFI_SUCCESS) {
+			max = max_addr;
+			max_addr = 0;
+			goto again;
+		}
+
+		*addr = max_addr;
+	}
+
+	efi_bs_call(free_pool, map);
+fail:
+	return status;
+}
+
+/*
+ * Allocate at the lowest possible address that is not below 'min'.
+ */
+efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
+				 unsigned long *addr, unsigned long min)
+{
+	unsigned long map_size, desc_size, buff_size;
+	efi_memory_desc_t *map;
+	efi_status_t status;
+	unsigned long nr_pages;
+	int i;
+	struct efi_boot_memmap boot_map;
+
+	boot_map.map		= &map;
+	boot_map.map_size	= &map_size;
+	boot_map.desc_size	= &desc_size;
+	boot_map.desc_ver	= NULL;
+	boot_map.key_ptr	= NULL;
+	boot_map.buff_size	= &buff_size;
+
+	status = efi_get_memory_map(&boot_map);
+	if (status != EFI_SUCCESS)
+		goto fail;
+
+	/*
+	 * Enforce minimum alignment that EFI or Linux requires when
+	 * requesting a specific address.  We are doing page-based (or
+	 * larger) allocations, and both the address and size must meet
+	 * alignment constraints.
+	 */
+	if (align < EFI_ALLOC_ALIGN)
+		align = EFI_ALLOC_ALIGN;
+
+	size = round_up(size, EFI_ALLOC_ALIGN);
+	nr_pages = size / EFI_PAGE_SIZE;
+	for (i = 0; i < map_size / desc_size; i++) {
+		efi_memory_desc_t *desc;
+		unsigned long m = (unsigned long)map;
+		u64 start, end;
+
+		desc = efi_early_memdesc_ptr(m, desc_size, i);
+
+		if (desc->type != EFI_CONVENTIONAL_MEMORY)
+			continue;
+
+		if (efi_soft_reserve_enabled() &&
+		    (desc->attribute & EFI_MEMORY_SP))
+			continue;
+
+		if (desc->num_pages < nr_pages)
+			continue;
+
+		start = desc->phys_addr;
+		end = start + desc->num_pages * EFI_PAGE_SIZE;
+
+		if (start < min)
+			start = min;
+
+		start = round_up(start, align);
+		if ((start + size) > end)
+			continue;
+
+		status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
+				     EFI_LOADER_DATA, nr_pages, &start);
+		if (status == EFI_SUCCESS) {
+			*addr = start;
+			break;
+		}
+	}
+
+	if (i == map_size / desc_size)
+		status = EFI_NOT_FOUND;
+
+	efi_bs_call(free_pool, map);
+fail:
+	return status;
+}
+
+void efi_free(unsigned long size, unsigned long addr)
+{
+	unsigned long nr_pages;
+
+	if (!size)
+		return;
+
+	nr_pages = round_up(size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
+	efi_bs_call(free_pages, addr, nr_pages);
+}
+
+/*
+ * Relocate a kernel image, either compressed or uncompressed.
+ * In the ARM64 case, all kernel images are currently
+ * uncompressed, and as such when we relocate it we need to
+ * allocate additional space for the BSS segment. Any low
+ * memory that this function should avoid needs to be
+ * unavailable in the EFI memory map, as if the preferred
+ * address is not available the lowest available address will
+ * be used.
+ */
+efi_status_t efi_relocate_kernel(unsigned long *image_addr,
+				 unsigned long image_size,
+				 unsigned long alloc_size,
+				 unsigned long preferred_addr,
+				 unsigned long alignment,
+				 unsigned long min_addr)
+{
+	unsigned long cur_image_addr;
+	unsigned long new_addr = 0;
+	efi_status_t status;
+	unsigned long nr_pages;
+	efi_physical_addr_t efi_addr = preferred_addr;
+
+	if (!image_addr || !image_size || !alloc_size)
+		return EFI_INVALID_PARAMETER;
+	if (alloc_size < image_size)
+		return EFI_INVALID_PARAMETER;
+
+	cur_image_addr = *image_addr;
+
+	/*
+	 * The EFI firmware loader could have placed the kernel image
+	 * anywhere in memory, but the kernel has restrictions on the
+	 * max physical address it can run at.  Some architectures
+	 * also have a prefered address, so first try to relocate
+	 * to the preferred address.  If that fails, allocate as low
+	 * as possible while respecting the required alignment.
+	 */
+	nr_pages = round_up(alloc_size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
+	status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
+			     EFI_LOADER_DATA, nr_pages, &efi_addr);
+	new_addr = efi_addr;
+	/*
+	 * If preferred address allocation failed allocate as low as
+	 * possible.
+	 */
+	if (status != EFI_SUCCESS) {
+		status = efi_low_alloc_above(alloc_size, alignment, &new_addr,
+					     min_addr);
+	}
+	if (status != EFI_SUCCESS) {
+		pr_efi_err("Failed to allocate usable memory for kernel.\n");
+		return status;
+	}
+
+	/*
+	 * We know source/dest won't overlap since both memory ranges
+	 * have been allocated by UEFI, so we can safely use memcpy.
+	 */
+	memcpy((void *)new_addr, (void *)cur_image_addr, image_size);
+
+	/* Return the new address of the relocated image. */
+	*image_addr = new_addr;
+
+	return status;
+}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 06/19] efi/libstub: Simplify efi_high_alloc() and rename to efi_allocate_pages()
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
                   ` (4 preceding siblings ...)
  2020-02-10 16:02 ` [PATCH 05/19] efi/libstub: Move memory map handling and allocation routines to mem.c Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 07/19] efi/libstub/x86: Incorporate eboot.c into libstub Ard Biesheuvel
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

The implementation of efi_high_alloc() uses a complicated way of
traversing the memory map to find an available region that is located
as close as possible to the provided upper limit, and calls AllocatePages
subsequently to create the allocation at that exact address.

This is precisely what the EFI_ALLOCATE_MAX_ADDRESS allocation type
argument to AllocatePages() does, and considering that EFI_ALLOC_ALIGN
only exceeds EFI_PAGE_SIZE on arm64, let's use AllocatePages() directly
and implement the alignment using code that the compiler can remove if
it does not exceed EFI_PAGE_SIZE.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/efi-stub-helper.c |   5 +-
 drivers/firmware/efi/libstub/fdt.c             |   3 +-
 drivers/firmware/efi/libstub/mem.c             | 102 ++++----------------
 include/linux/efi.h                            |   4 +-
 4 files changed, 23 insertions(+), 91 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 60d13c7a2e92..7afe31357df3 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -385,8 +385,7 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
 		 * so allocate enough memory for all the files.  This is used
 		 * for loading multiple files.
 		 */
-		status = efi_high_alloc(file_size_total, 0x1000, &file_addr,
-					max_addr);
+		status = efi_allocate_pages(file_size_total, &file_addr, max_addr);
 		if (status != EFI_SUCCESS) {
 			pr_efi_err("Failed to alloc highmem for files\n");
 			goto close_handles;
@@ -536,7 +535,7 @@ char *efi_convert_cmdline(efi_loaded_image_t *image,
 
 	options_bytes++;	/* NUL termination */
 
-	status = efi_high_alloc(options_bytes, 0, &cmdline_addr,
+	status = efi_allocate_pages(options_bytes, &cmdline_addr,
 				MAX_CMDLINE_ADDRESS);
 	if (status != EFI_SUCCESS)
 		return NULL;
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index f71cd54823b7..46cffac7a5f1 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -277,8 +277,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
 	pr_efi("Exiting boot services and installing virtual address map...\n");
 
 	map.map = &memory_map;
-	status = efi_high_alloc(MAX_FDT_SIZE, EFI_PAGE_SIZE,
-				new_fdt_addr, max_addr);
+	status = efi_allocate_pages(MAX_FDT_SIZE, new_fdt_addr, max_addr);
 	if (status != EFI_SUCCESS) {
 		pr_efi_err("Unable to allocate memory for new device tree.\n");
 		goto fail;
diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c
index 690648a7ca1e..5808c8764e64 100644
--- a/drivers/firmware/efi/libstub/mem.c
+++ b/drivers/firmware/efi/libstub/mem.c
@@ -68,100 +68,34 @@ efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
 /*
  * Allocate at the highest possible address that is not above 'max'.
  */
-efi_status_t efi_high_alloc(unsigned long size, unsigned long align,
-			    unsigned long *addr, unsigned long max)
+efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
+				unsigned long max)
 {
-	unsigned long map_size, desc_size, buff_size;
-	efi_memory_desc_t *map;
+	efi_physical_addr_t alloc_addr = ALIGN_DOWN(max + 1, EFI_ALLOC_ALIGN) - 1;
+	int slack = EFI_ALLOC_ALIGN / EFI_PAGE_SIZE - 1;
 	efi_status_t status;
-	unsigned long nr_pages;
-	u64 max_addr = 0;
-	int i;
-	struct efi_boot_memmap boot_map;
-
-	boot_map.map =		&map;
-	boot_map.map_size =	&map_size;
-	boot_map.desc_size =	&desc_size;
-	boot_map.desc_ver =	NULL;
-	boot_map.key_ptr =	NULL;
-	boot_map.buff_size =	&buff_size;
-
-	status = efi_get_memory_map(&boot_map);
-	if (status != EFI_SUCCESS)
-		goto fail;
-
-	/*
-	 * Enforce minimum alignment that EFI or Linux requires when
-	 * requesting a specific address.  We are doing page-based (or
-	 * larger) allocations, and both the address and size must meet
-	 * alignment constraints.
-	 */
-	if (align < EFI_ALLOC_ALIGN)
-		align = EFI_ALLOC_ALIGN;
 
 	size = round_up(size, EFI_ALLOC_ALIGN);
-	nr_pages = size / EFI_PAGE_SIZE;
-again:
-	for (i = 0; i < map_size / desc_size; i++) {
-		efi_memory_desc_t *desc;
-		unsigned long m = (unsigned long)map;
-		u64 start, end;
-
-		desc = efi_early_memdesc_ptr(m, desc_size, i);
-		if (desc->type != EFI_CONVENTIONAL_MEMORY)
-			continue;
-
-		if (efi_soft_reserve_enabled() &&
-		    (desc->attribute & EFI_MEMORY_SP))
-			continue;
-
-		if (desc->num_pages < nr_pages)
-			continue;
-
-		start = desc->phys_addr;
-		end = start + desc->num_pages * EFI_PAGE_SIZE;
-
-		if (end > max)
-			end = max;
-
-		if ((start + size) > end)
-			continue;
-
-		if (round_down(end - size, align) < start)
-			continue;
-
-		start = round_down(end - size, align);
+	status = efi_bs_call(allocate_pages, EFI_ALLOCATE_MAX_ADDRESS,
+			     EFI_LOADER_DATA, size / EFI_PAGE_SIZE + slack,
+			     &alloc_addr);
+	if (status != EFI_SUCCESS)
+		return status;
 
-		/*
-		 * Don't allocate at 0x0. It will confuse code that
-		 * checks pointers against NULL.
-		 */
-		if (start == 0x0)
-			continue;
+	*addr = ALIGN((unsigned long)alloc_addr, EFI_ALLOC_ALIGN);
 
-		if (start > max_addr)
-			max_addr = start;
-	}
+	if (slack > 0) {
+		int l = (alloc_addr % EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
 
-	if (!max_addr)
-		status = EFI_NOT_FOUND;
-	else {
-		status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
-				     EFI_LOADER_DATA, nr_pages, &max_addr);
-		if (status != EFI_SUCCESS) {
-			max = max_addr;
-			max_addr = 0;
-			goto again;
+		if (l) {
+			efi_bs_call(free_pages, alloc_addr, slack - l + 1);
+			slack = l - 1;
 		}
-
-		*addr = max_addr;
+		if (slack)
+			efi_bs_call(free_pages, *addr + size, slack);
 	}
-
-	efi_bs_call(free_pool, map);
-fail:
-	return status;
+	return EFI_SUCCESS;
 }
-
 /*
  * Allocate at the lowest possible address that is not below 'min'.
  */
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 7efd7072cca5..7e231c3cfb6f 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1508,8 +1508,8 @@ efi_status_t efi_low_alloc(unsigned long size, unsigned long align,
 	return efi_low_alloc_above(size, align, addr, 0x8);
 }
 
-efi_status_t efi_high_alloc(unsigned long size, unsigned long align,
-			    unsigned long *addr, unsigned long max);
+efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
+				unsigned long max);
 
 efi_status_t efi_relocate_kernel(unsigned long *image_addr,
 				 unsigned long image_size,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 07/19] efi/libstub/x86: Incorporate eboot.c into libstub
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
                   ` (5 preceding siblings ...)
  2020-02-10 16:02 ` [PATCH 06/19] efi/libstub: Simplify efi_high_alloc() and rename to efi_allocate_pages() Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 08/19] efi/libstub: Use consistent type names for file I/O protocols Ard Biesheuvel
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

Most of the EFI stub source files of all architectures reside under
drivers/firmware/efi/libstub, where they share a Makefile with special
CFLAGS and an include file with declarations that are only relevant
for stub code.

Currently, we carry a lot of stub specific stuff in linux/efi.h only
because eboot.c in arch/x86 needs them as well. So let's move eboot.c
into libstub/, and move the contents of eboot.h that we still care
about into efistub.h

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/boot/compressed/Makefile                                           |  5 +---
 arch/x86/boot/compressed/eboot.h                                            | 31 --------------------
 drivers/firmware/efi/libstub/Makefile                                       |  1 +
 drivers/firmware/efi/libstub/efistub.h                                      | 16 ++++++++++
 arch/x86/boot/compressed/eboot.c => drivers/firmware/efi/libstub/x86-stub.c |  5 +---
 5 files changed, 19 insertions(+), 39 deletions(-)

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 26050ae0b27e..e51879bdc51c 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -87,10 +87,7 @@ endif
 
 vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o
 
-$(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone
-
-vmlinux-objs-$(CONFIG_EFI_STUB) += $(obj)/eboot.o \
-	$(objtree)/drivers/firmware/efi/libstub/lib.a
+vmlinux-objs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
 vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o
 
 # The compressed kernel is built with -fPIC/-fPIE so that a boot loader
diff --git a/arch/x86/boot/compressed/eboot.h b/arch/x86/boot/compressed/eboot.h
deleted file mode 100644
index 99f35343d443..000000000000
--- a/arch/x86/boot/compressed/eboot.h
+++ /dev/null
@@ -1,31 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef BOOT_COMPRESSED_EBOOT_H
-#define BOOT_COMPRESSED_EBOOT_H
-
-#define SEG_TYPE_DATA		(0 << 3)
-#define SEG_TYPE_READ_WRITE	(1 << 1)
-#define SEG_TYPE_CODE		(1 << 3)
-#define SEG_TYPE_EXEC_READ	(1 << 1)
-#define SEG_TYPE_TSS		((1 << 3) | (1 << 0))
-#define SEG_OP_SIZE_32BIT	(1 << 0)
-#define SEG_GRANULARITY_4KB	(1 << 0)
-
-#define DESC_TYPE_CODE_DATA	(1 << 0)
-
-typedef union efi_uga_draw_protocol efi_uga_draw_protocol_t;
-
-union efi_uga_draw_protocol {
-	struct {
-		efi_status_t (__efiapi *get_mode)(efi_uga_draw_protocol_t *,
-						  u32*, u32*, u32*, u32*);
-		void *set_mode;
-		void *blt;
-	};
-	struct {
-		u32 get_mode;
-		u32 set_mode;
-		u32 blt;
-	} mixed_mode;
-};
-
-#endif /* BOOT_COMPRESSED_EBOOT_H */
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 45d6eb657437..8e15634fa929 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -52,6 +52,7 @@ lib-$(CONFIG_EFI_ARMSTUB)	+= arm-stub.o fdt.o string.o \
 
 lib-$(CONFIG_ARM)		+= arm32-stub.o
 lib-$(CONFIG_ARM64)		+= arm64-stub.o
+lib-$(CONFIG_X86)		+= x86-stub.o
 CFLAGS_arm32-stub.o		:= -DTEXT_OFFSET=$(TEXT_OFFSET)
 CFLAGS_arm64-stub.o		:= -DTEXT_OFFSET=$(TEXT_OFFSET)
 
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index c244b165005e..55de118e8267 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -90,4 +90,20 @@ void *get_efi_config_table(efi_guid_t guid);
 	efi_rt_call(set_variable, (efi_char16_t *)(name),	\
 		    (efi_guid_t *)(vendor), __VA_ARGS__)
 
+typedef union efi_uga_draw_protocol efi_uga_draw_protocol_t;
+
+union efi_uga_draw_protocol {
+	struct {
+		efi_status_t (__efiapi *get_mode)(efi_uga_draw_protocol_t *,
+						  u32*, u32*, u32*, u32*);
+		void *set_mode;
+		void *blt;
+	};
+	struct {
+		u32 get_mode;
+		u32 set_mode;
+		u32 blt;
+	} mixed_mode;
+};
+
 #endif
diff --git a/arch/x86/boot/compressed/eboot.c b/drivers/firmware/efi/libstub/x86-stub.c
similarity index 99%
rename from arch/x86/boot/compressed/eboot.c
rename to drivers/firmware/efi/libstub/x86-stub.c
index 55637135b50c..7e7c50883cce 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -6,8 +6,6 @@
  *
  * ----------------------------------------------------------------------- */
 
-#pragma GCC visibility push(hidden)
-
 #include <linux/efi.h>
 #include <linux/pci.h>
 
@@ -17,8 +15,7 @@
 #include <asm/desc.h>
 #include <asm/boot.h>
 
-#include "../string.h"
-#include "eboot.h"
+#include "efistub.h"
 
 static efi_system_table_t *sys_table;
 extern const bool efi_is64;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 08/19] efi/libstub: Use consistent type names for file I/O protocols
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
                   ` (6 preceding siblings ...)
  2020-02-10 16:02 ` [PATCH 07/19] efi/libstub/x86: Incorporate eboot.c into libstub Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 09/19] efi/libstub: Move stub specific declarations into efistub.h Ard Biesheuvel
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

Align the naming of efi_file_io_interface_t and efi_file_handle_t with
the UEFI spec, and call them efi_simple_file_system_protocol_t and
efi_file_protocol_t, respectively, using the same convention we use
for all other type definitions that originate in the UEFI spec.

While at it, move the definitions to efistub.h, so they are only seen
by code that needs them.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/efi-stub-helper.c | 16 ++---
 drivers/firmware/efi/libstub/efistub.h         | 63 ++++++++++++++++++++
 include/linux/efi.h                            | 60 +------------------
 3 files changed, 72 insertions(+), 67 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 7afe31357df3..6db91655c743 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -54,7 +54,7 @@ bool __pure __efi_soft_reserve_enabled(void)
 }
 
 struct file_info {
-	efi_file_handle_t *handle;
+	efi_file_protocol_t *handle;
 	u64 size;
 };
 
@@ -113,7 +113,7 @@ unsigned long get_dram_base(void)
 static efi_status_t efi_file_size(void *__fh, efi_char16_t *filename_16,
 				  void **handle, u64 *file_sz)
 {
-	efi_file_handle_t *h, *fh = __fh;
+	efi_file_protocol_t *h, *fh = __fh;
 	efi_file_info_t *info;
 	efi_status_t status;
 	efi_guid_t info_guid = EFI_FILE_INFO_ID;
@@ -159,22 +159,22 @@ static efi_status_t efi_file_size(void *__fh, efi_char16_t *filename_16,
 	return status;
 }
 
-static efi_status_t efi_file_read(efi_file_handle_t *handle,
+static efi_status_t efi_file_read(efi_file_protocol_t *handle,
 				  unsigned long *size, void *addr)
 {
 	return handle->read(handle, size, addr);
 }
 
-static efi_status_t efi_file_close(efi_file_handle_t *handle)
+static efi_status_t efi_file_close(efi_file_protocol_t *handle)
 {
 	return handle->close(handle);
 }
 
 static efi_status_t efi_open_volume(efi_loaded_image_t *image,
-				    efi_file_handle_t **__fh)
+				    efi_file_protocol_t **__fh)
 {
-	efi_file_io_interface_t *io;
-	efi_file_handle_t *fh;
+	efi_simple_file_system_protocol_t *io;
+	efi_file_protocol_t *fh;
 	efi_guid_t fs_proto = EFI_FILE_SYSTEM_GUID;
 	efi_status_t status;
 	efi_handle_t handle = image->device_handle;
@@ -282,7 +282,7 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
 	struct file_info *files;
 	unsigned long file_addr;
 	u64 file_size_total;
-	efi_file_handle_t *fh = NULL;
+	efi_file_protocol_t *fh = NULL;
 	efi_status_t status;
 	int nr_files;
 	char *str;
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 55de118e8267..79cdb219f439 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -106,4 +106,67 @@ union efi_uga_draw_protocol {
 	} mixed_mode;
 };
 
+typedef struct efi_loaded_image {
+	u32			revision;
+	efi_handle_t		parent_handle;
+	efi_system_table_t	*system_table;
+	efi_handle_t		device_handle;
+	void			*file_path;
+	void			*reserved;
+	u32			load_options_size;
+	void			*load_options;
+	void			*image_base;
+	__aligned_u64		image_size;
+	unsigned int		image_code_type;
+	unsigned int		image_data_type;
+	efi_status_t		(__efiapi *unload)(efi_handle_t image_handle);
+} efi_loaded_image_t;
+
+typedef struct {
+	u64			size;
+	u64			file_size;
+	u64			phys_size;
+	efi_time_t		create_time;
+	efi_time_t		last_access_time;
+	efi_time_t		modification_time;
+	__aligned_u64		attribute;
+	efi_char16_t		filename[1];
+} efi_file_info_t;
+
+typedef struct efi_file_protocol efi_file_protocol_t;
+
+struct efi_file_protocol {
+	u64		revision;
+	efi_status_t	(__efiapi *open)	(efi_file_protocol_t *,
+						 efi_file_protocol_t **,
+						 efi_char16_t *, u64, u64);
+	efi_status_t	(__efiapi *close)	(efi_file_protocol_t *);
+	efi_status_t	(__efiapi *delete)	(efi_file_protocol_t *);
+	efi_status_t	(__efiapi *read)	(efi_file_protocol_t *,
+						 unsigned long *, void *);
+	efi_status_t	(__efiapi *write)	(efi_file_protocol_t *,
+						 unsigned long, void *);
+	efi_status_t	(__efiapi *get_position)(efi_file_protocol_t *, u64 *);
+	efi_status_t	(__efiapi *set_position)(efi_file_protocol_t *, u64);
+	efi_status_t	(__efiapi *get_info)	(efi_file_protocol_t *,
+						 efi_guid_t *, unsigned long *,
+						 void *);
+	efi_status_t	(__efiapi *set_info)	(efi_file_protocol_t *,
+						 efi_guid_t *, unsigned long,
+						 void *);
+	efi_status_t	(__efiapi *flush)	(efi_file_protocol_t *);
+};
+
+typedef struct efi_simple_file_system_protocol efi_simple_file_system_protocol_t;
+
+struct efi_simple_file_system_protocol {
+	u64	revision;
+	int	(__efiapi *open_volume)(efi_simple_file_system_protocol_t *,
+					efi_file_protocol_t **);
+};
+
+#define EFI_FILE_MODE_READ	0x0000000000000001
+#define EFI_FILE_MODE_WRITE	0x0000000000000002
+#define EFI_FILE_MODE_CREATE	0x8000000000000000
+
 #endif
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 7e231c3cfb6f..2b228df18407 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -796,65 +796,7 @@ struct efi_fdt_params {
 	u32 desc_ver;
 };
 
-typedef struct {
-	u32 revision;
-	efi_handle_t parent_handle;
-	efi_system_table_t *system_table;
-	efi_handle_t device_handle;
-	void *file_path;
-	void *reserved;
-	u32 load_options_size;
-	void *load_options;
-	void *image_base;
-	__aligned_u64 image_size;
-	unsigned int image_code_type;
-	unsigned int image_data_type;
-	efi_status_t ( __efiapi *unload)(efi_handle_t image_handle);
-} efi_loaded_image_t;
-
-typedef struct {
-	u64 size;
-	u64 file_size;
-	u64 phys_size;
-	efi_time_t create_time;
-	efi_time_t last_access_time;
-	efi_time_t modification_time;
-	__aligned_u64 attribute;
-	efi_char16_t filename[1];
-} efi_file_info_t;
-
-typedef struct efi_file_handle efi_file_handle_t;
-
-struct efi_file_handle {
-	u64 revision;
-	efi_status_t (__efiapi *open)(efi_file_handle_t *,
-				      efi_file_handle_t **,
-				      efi_char16_t *, u64, u64);
-	efi_status_t (__efiapi *close)(efi_file_handle_t *);
-	void *delete;
-	efi_status_t (__efiapi *read)(efi_file_handle_t *,
-				      unsigned long *, void *);
-	void *write;
-	void *get_position;
-	void *set_position;
-	efi_status_t (__efiapi *get_info)(efi_file_handle_t *,
-					  efi_guid_t *, unsigned long *,
-					  void *);
-	void *set_info;
-	void *flush;
-};
-
-typedef struct efi_file_io_interface efi_file_io_interface_t;
-
-struct efi_file_io_interface {
-	u64 revision;
-	int (__efiapi *open_volume)(efi_file_io_interface_t *,
-				    efi_file_handle_t **);
-};
-
-#define EFI_FILE_MODE_READ	0x0000000000000001
-#define EFI_FILE_MODE_WRITE	0x0000000000000002
-#define EFI_FILE_MODE_CREATE	0x8000000000000000
+typedef struct efi_loaded_image efi_loaded_image_t;
 
 typedef struct {
 	u32 version;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 09/19] efi/libstub: Move stub specific declarations into efistub.h
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
                   ` (7 preceding siblings ...)
  2020-02-10 16:02 ` [PATCH 08/19] efi/libstub: Use consistent type names for file I/O protocols Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 10/19] efi/libstub/x86: Permit bootparams struct to be allocated above 4 GB Ard Biesheuvel
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

Move all the declarations that are only used in stub code from
linux/efi.h to efistub.h which is only included locally.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/efistub.h | 507 ++++++++++++++++++--
 include/linux/efi.h                    | 456 +-----------------
 2 files changed, 479 insertions(+), 484 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 79cdb219f439..8bb46c1fd2cd 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -43,34 +43,6 @@ extern __pure efi_system_table_t  *efi_system_table(void);
 
 #define pr_efi_err(msg) efi_printk("EFI stub: ERROR: "msg)
 
-void efi_char16_printk(efi_char16_t *);
-void efi_char16_printk(efi_char16_t *);
-
-unsigned long get_dram_base(void);
-
-efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
-					    unsigned long *new_fdt_addr,
-					    unsigned long max_addr,
-					    u64 initrd_addr, u64 initrd_size,
-					    char *cmdline_ptr,
-					    unsigned long fdt_addr,
-					    unsigned long fdt_size);
-
-void *get_fdt(unsigned long *fdt_size);
-
-void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
-		     unsigned long desc_size, efi_memory_desc_t *runtime_map,
-		     int *count);
-
-efi_status_t efi_get_random_bytes(unsigned long size, u8 *out);
-
-efi_status_t efi_random_alloc(unsigned long size, unsigned long align,
-			      unsigned long *addr, unsigned long random_seed);
-
-efi_status_t check_platform_features(void);
-
-void *get_efi_config_table(efi_guid_t guid);
-
 /* Helper macros for the usual case of using simple C variables: */
 #ifndef fdt_setprop_inplace_var
 #define fdt_setprop_inplace_var(fdt, node_offset, name, var) \
@@ -90,6 +62,156 @@ void *get_efi_config_table(efi_guid_t guid);
 	efi_rt_call(set_variable, (efi_char16_t *)(name),	\
 		    (efi_guid_t *)(vendor), __VA_ARGS__)
 
+#define efi_get_handle_at(array, idx)					\
+	(efi_is_native() ? (array)[idx] 				\
+		: (efi_handle_t)(unsigned long)((u32 *)(array))[idx])
+
+#define efi_get_handle_num(size)					\
+	((size) / (efi_is_native() ? sizeof(efi_handle_t) : sizeof(u32)))
+
+#define for_each_efi_handle(handle, array, size, i)			\
+	for (i = 0;							\
+	     i < efi_get_handle_num(size) &&				\
+		((handle = efi_get_handle_at((array), i)) || true);	\
+	     i++)
+
+/*
+ * Allocation types for calls to boottime->allocate_pages.
+ */
+#define EFI_ALLOCATE_ANY_PAGES		0
+#define EFI_ALLOCATE_MAX_ADDRESS	1
+#define EFI_ALLOCATE_ADDRESS		2
+#define EFI_MAX_ALLOCATE_TYPE		3
+
+/*
+ * The type of search to perform when calling boottime->locate_handle
+ */
+#define EFI_LOCATE_ALL_HANDLES			0
+#define EFI_LOCATE_BY_REGISTER_NOTIFY		1
+#define EFI_LOCATE_BY_PROTOCOL			2
+
+struct efi_boot_memmap {
+	efi_memory_desc_t	**map;
+	unsigned long		*map_size;
+	unsigned long		*desc_size;
+	u32			*desc_ver;
+	unsigned long		*key_ptr;
+	unsigned long		*buff_size;
+};
+
+/*
+ * EFI Boot Services table
+ */
+union efi_boot_services {
+	struct {
+		efi_table_hdr_t hdr;
+		void *raise_tpl;
+		void *restore_tpl;
+		efi_status_t (__efiapi *allocate_pages)(int, int, unsigned long,
+							efi_physical_addr_t *);
+		efi_status_t (__efiapi *free_pages)(efi_physical_addr_t,
+						    unsigned long);
+		efi_status_t (__efiapi *get_memory_map)(unsigned long *, void *,
+							unsigned long *,
+							unsigned long *, u32 *);
+		efi_status_t (__efiapi *allocate_pool)(int, unsigned long,
+						       void **);
+		efi_status_t (__efiapi *free_pool)(void *);
+		void *create_event;
+		void *set_timer;
+		void *wait_for_event;
+		void *signal_event;
+		void *close_event;
+		void *check_event;
+		void *install_protocol_interface;
+		void *reinstall_protocol_interface;
+		void *uninstall_protocol_interface;
+		efi_status_t (__efiapi *handle_protocol)(efi_handle_t,
+							 efi_guid_t *, void **);
+		void *__reserved;
+		void *register_protocol_notify;
+		efi_status_t (__efiapi *locate_handle)(int, efi_guid_t *,
+						       void *, unsigned long *,
+						       efi_handle_t *);
+		void *locate_device_path;
+		efi_status_t (__efiapi *install_configuration_table)(efi_guid_t *,
+								     void *);
+		void *load_image;
+		void *start_image;
+		void *exit;
+		void *unload_image;
+		efi_status_t (__efiapi *exit_boot_services)(efi_handle_t,
+							    unsigned long);
+		void *get_next_monotonic_count;
+		void *stall;
+		void *set_watchdog_timer;
+		void *connect_controller;
+		efi_status_t (__efiapi *disconnect_controller)(efi_handle_t,
+							       efi_handle_t,
+							       efi_handle_t);
+		void *open_protocol;
+		void *close_protocol;
+		void *open_protocol_information;
+		void *protocols_per_handle;
+		void *locate_handle_buffer;
+		efi_status_t (__efiapi *locate_protocol)(efi_guid_t *, void *,
+							 void **);
+		void *install_multiple_protocol_interfaces;
+		void *uninstall_multiple_protocol_interfaces;
+		void *calculate_crc32;
+		void *copy_mem;
+		void *set_mem;
+		void *create_event_ex;
+	};
+	struct {
+		efi_table_hdr_t hdr;
+		u32 raise_tpl;
+		u32 restore_tpl;
+		u32 allocate_pages;
+		u32 free_pages;
+		u32 get_memory_map;
+		u32 allocate_pool;
+		u32 free_pool;
+		u32 create_event;
+		u32 set_timer;
+		u32 wait_for_event;
+		u32 signal_event;
+		u32 close_event;
+		u32 check_event;
+		u32 install_protocol_interface;
+		u32 reinstall_protocol_interface;
+		u32 uninstall_protocol_interface;
+		u32 handle_protocol;
+		u32 __reserved;
+		u32 register_protocol_notify;
+		u32 locate_handle;
+		u32 locate_device_path;
+		u32 install_configuration_table;
+		u32 load_image;
+		u32 start_image;
+		u32 exit;
+		u32 unload_image;
+		u32 exit_boot_services;
+		u32 get_next_monotonic_count;
+		u32 stall;
+		u32 set_watchdog_timer;
+		u32 connect_controller;
+		u32 disconnect_controller;
+		u32 open_protocol;
+		u32 close_protocol;
+		u32 open_protocol_information;
+		u32 protocols_per_handle;
+		u32 locate_handle_buffer;
+		u32 locate_protocol;
+		u32 install_multiple_protocol_interfaces;
+		u32 uninstall_multiple_protocol_interfaces;
+		u32 calculate_crc32;
+		u32 copy_mem;
+		u32 set_mem;
+		u32 create_event_ex;
+	} mixed_mode;
+};
+
 typedef union efi_uga_draw_protocol efi_uga_draw_protocol_t;
 
 union efi_uga_draw_protocol {
@@ -106,7 +228,81 @@ union efi_uga_draw_protocol {
 	} mixed_mode;
 };
 
-typedef struct efi_loaded_image {
+union efi_simple_text_output_protocol {
+	struct {
+		void *reset;
+		efi_status_t (__efiapi *output_string)(efi_simple_text_output_protocol_t *,
+						       efi_char16_t *);
+		void *test_string;
+	};
+	struct {
+		u32 reset;
+		u32 output_string;
+		u32 test_string;
+	} mixed_mode;
+};
+
+#define PIXEL_RGB_RESERVED_8BIT_PER_COLOR		0
+#define PIXEL_BGR_RESERVED_8BIT_PER_COLOR		1
+#define PIXEL_BIT_MASK					2
+#define PIXEL_BLT_ONLY					3
+#define PIXEL_FORMAT_MAX				4
+
+typedef struct {
+	u32 red_mask;
+	u32 green_mask;
+	u32 blue_mask;
+	u32 reserved_mask;
+} efi_pixel_bitmask_t;
+
+typedef struct {
+	u32 version;
+	u32 horizontal_resolution;
+	u32 vertical_resolution;
+	int pixel_format;
+	efi_pixel_bitmask_t pixel_information;
+	u32 pixels_per_scan_line;
+} efi_graphics_output_mode_info_t;
+
+typedef union efi_graphics_output_protocol_mode efi_graphics_output_protocol_mode_t;
+
+union efi_graphics_output_protocol_mode {
+	struct {
+		u32 max_mode;
+		u32 mode;
+		efi_graphics_output_mode_info_t *info;
+		unsigned long size_of_info;
+		efi_physical_addr_t frame_buffer_base;
+		unsigned long frame_buffer_size;
+	};
+	struct {
+		u32 max_mode;
+		u32 mode;
+		u32 info;
+		u32 size_of_info;
+		u64 frame_buffer_base;
+		u32 frame_buffer_size;
+	} mixed_mode;
+};
+
+typedef union efi_graphics_output_protocol efi_graphics_output_protocol_t;
+
+union efi_graphics_output_protocol {
+	struct {
+		void *query_mode;
+		void *set_mode;
+		void *blt;
+		efi_graphics_output_protocol_mode_t *mode;
+	};
+	struct {
+		u32 query_mode;
+		u32 set_mode;
+		u32 blt;
+		u32 mode;
+	} mixed_mode;
+};
+
+typedef struct {
 	u32			revision;
 	efi_handle_t		parent_handle;
 	efi_system_table_t	*system_table;
@@ -169,4 +365,257 @@ struct efi_simple_file_system_protocol {
 #define EFI_FILE_MODE_WRITE	0x0000000000000002
 #define EFI_FILE_MODE_CREATE	0x8000000000000000
 
+typedef enum {
+	EfiPciIoWidthUint8,
+	EfiPciIoWidthUint16,
+	EfiPciIoWidthUint32,
+	EfiPciIoWidthUint64,
+	EfiPciIoWidthFifoUint8,
+	EfiPciIoWidthFifoUint16,
+	EfiPciIoWidthFifoUint32,
+	EfiPciIoWidthFifoUint64,
+	EfiPciIoWidthFillUint8,
+	EfiPciIoWidthFillUint16,
+	EfiPciIoWidthFillUint32,
+	EfiPciIoWidthFillUint64,
+	EfiPciIoWidthMaximum
+} EFI_PCI_IO_PROTOCOL_WIDTH;
+
+typedef enum {
+	EfiPciIoAttributeOperationGet,
+	EfiPciIoAttributeOperationSet,
+	EfiPciIoAttributeOperationEnable,
+	EfiPciIoAttributeOperationDisable,
+	EfiPciIoAttributeOperationSupported,
+    EfiPciIoAttributeOperationMaximum
+} EFI_PCI_IO_PROTOCOL_ATTRIBUTE_OPERATION;
+
+typedef struct {
+	u32 read;
+	u32 write;
+} efi_pci_io_protocol_access_32_t;
+
+typedef union efi_pci_io_protocol efi_pci_io_protocol_t;
+
+typedef
+efi_status_t (__efiapi *efi_pci_io_protocol_cfg_t)(efi_pci_io_protocol_t *,
+						   EFI_PCI_IO_PROTOCOL_WIDTH,
+						   u32 offset,
+						   unsigned long count,
+						   void *buffer);
+
+typedef struct {
+	void *read;
+	void *write;
+} efi_pci_io_protocol_access_t;
+
+typedef struct {
+	efi_pci_io_protocol_cfg_t read;
+	efi_pci_io_protocol_cfg_t write;
+} efi_pci_io_protocol_config_access_t;
+
+union efi_pci_io_protocol {
+	struct {
+		void *poll_mem;
+		void *poll_io;
+		efi_pci_io_protocol_access_t mem;
+		efi_pci_io_protocol_access_t io;
+		efi_pci_io_protocol_config_access_t pci;
+		void *copy_mem;
+		void *map;
+		void *unmap;
+		void *allocate_buffer;
+		void *free_buffer;
+		void *flush;
+		efi_status_t (__efiapi *get_location)(efi_pci_io_protocol_t *,
+						      unsigned long *segment_nr,
+						      unsigned long *bus_nr,
+						      unsigned long *device_nr,
+						      unsigned long *func_nr);
+		void *attributes;
+		void *get_bar_attributes;
+		void *set_bar_attributes;
+		uint64_t romsize;
+		void *romimage;
+	};
+	struct {
+		u32 poll_mem;
+		u32 poll_io;
+		efi_pci_io_protocol_access_32_t mem;
+		efi_pci_io_protocol_access_32_t io;
+		efi_pci_io_protocol_access_32_t pci;
+		u32 copy_mem;
+		u32 map;
+		u32 unmap;
+		u32 allocate_buffer;
+		u32 free_buffer;
+		u32 flush;
+		u32 get_location;
+		u32 attributes;
+		u32 get_bar_attributes;
+		u32 set_bar_attributes;
+		u64 romsize;
+		u32 romimage;
+	} mixed_mode;
+};
+
+#define EFI_PCI_IO_ATTRIBUTE_ISA_MOTHERBOARD_IO 0x0001
+#define EFI_PCI_IO_ATTRIBUTE_ISA_IO 0x0002
+#define EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO 0x0004
+#define EFI_PCI_IO_ATTRIBUTE_VGA_MEMORY 0x0008
+#define EFI_PCI_IO_ATTRIBUTE_VGA_IO 0x0010
+#define EFI_PCI_IO_ATTRIBUTE_IDE_PRIMARY_IO 0x0020
+#define EFI_PCI_IO_ATTRIBUTE_IDE_SECONDARY_IO 0x0040
+#define EFI_PCI_IO_ATTRIBUTE_MEMORY_WRITE_COMBINE 0x0080
+#define EFI_PCI_IO_ATTRIBUTE_IO 0x0100
+#define EFI_PCI_IO_ATTRIBUTE_MEMORY 0x0200
+#define EFI_PCI_IO_ATTRIBUTE_BUS_MASTER 0x0400
+#define EFI_PCI_IO_ATTRIBUTE_MEMORY_CACHED 0x0800
+#define EFI_PCI_IO_ATTRIBUTE_MEMORY_DISABLE 0x1000
+#define EFI_PCI_IO_ATTRIBUTE_EMBEDDED_DEVICE 0x2000
+#define EFI_PCI_IO_ATTRIBUTE_EMBEDDED_ROM 0x4000
+#define EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE 0x8000
+#define EFI_PCI_IO_ATTRIBUTE_ISA_IO_16 0x10000
+#define EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO_16 0x20000
+#define EFI_PCI_IO_ATTRIBUTE_VGA_IO_16 0x40000
+
+struct efi_dev_path;
+
+typedef union apple_properties_protocol apple_properties_protocol_t;
+
+union apple_properties_protocol {
+	struct {
+		unsigned long version;
+		efi_status_t (__efiapi *get)(apple_properties_protocol_t *,
+					     struct efi_dev_path *,
+					     efi_char16_t *, void *, u32 *);
+		efi_status_t (__efiapi *set)(apple_properties_protocol_t *,
+					     struct efi_dev_path *,
+					     efi_char16_t *, void *, u32);
+		efi_status_t (__efiapi *del)(apple_properties_protocol_t *,
+					     struct efi_dev_path *,
+					     efi_char16_t *);
+		efi_status_t (__efiapi *get_all)(apple_properties_protocol_t *,
+						 void *buffer, u32 *);
+	};
+	struct {
+		u32 version;
+		u32 get;
+		u32 set;
+		u32 del;
+		u32 get_all;
+	} mixed_mode;
+};
+
+typedef u32 efi_tcg2_event_log_format;
+
+typedef union efi_tcg2_protocol efi_tcg2_protocol_t;
+
+union efi_tcg2_protocol {
+	struct {
+		void *get_capability;
+		efi_status_t (__efiapi *get_event_log)(efi_handle_t,
+						       efi_tcg2_event_log_format,
+						       efi_physical_addr_t *,
+						       efi_physical_addr_t *,
+						       efi_bool_t *);
+		void *hash_log_extend_event;
+		void *submit_command;
+		void *get_active_pcr_banks;
+		void *set_active_pcr_banks;
+		void *get_result_of_set_active_pcr_banks;
+	};
+	struct {
+		u32 get_capability;
+		u32 get_event_log;
+		u32 hash_log_extend_event;
+		u32 submit_command;
+		u32 get_active_pcr_banks;
+		u32 set_active_pcr_banks;
+		u32 get_result_of_set_active_pcr_banks;
+	} mixed_mode;
+};
+
+void efi_pci_disable_bridge_busmaster(void);
+
+typedef efi_status_t (*efi_exit_boot_map_processing)(
+	struct efi_boot_memmap *map,
+	void *priv);
+
+efi_status_t efi_exit_boot_services(void *handle,
+				    struct efi_boot_memmap *map,
+				    void *priv,
+				    efi_exit_boot_map_processing priv_func);
+
+void efi_char16_printk(efi_char16_t *);
+
+unsigned long get_dram_base(void);
+
+efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
+					    unsigned long *new_fdt_addr,
+					    unsigned long max_addr,
+					    u64 initrd_addr, u64 initrd_size,
+					    char *cmdline_ptr,
+					    unsigned long fdt_addr,
+					    unsigned long fdt_size);
+
+void *get_fdt(unsigned long *fdt_size);
+
+void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
+		     unsigned long desc_size, efi_memory_desc_t *runtime_map,
+		     int *count);
+
+efi_status_t efi_get_random_bytes(unsigned long size, u8 *out);
+
+efi_status_t efi_random_alloc(unsigned long size, unsigned long align,
+			      unsigned long *addr, unsigned long random_seed);
+
+efi_status_t check_platform_features(void);
+
+void *get_efi_config_table(efi_guid_t guid);
+
+void efi_printk(char *str);
+
+void efi_free(unsigned long size, unsigned long addr);
+
+char *efi_convert_cmdline(efi_loaded_image_t *image, int *cmd_line_len);
+
+efi_status_t efi_get_memory_map(struct efi_boot_memmap *map);
+
+efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
+				 unsigned long *addr, unsigned long min);
+
+static inline
+efi_status_t efi_low_alloc(unsigned long size, unsigned long align,
+			   unsigned long *addr)
+{
+	/*
+	 * Don't allocate at 0x0. It will confuse code that
+	 * checks pointers against NULL. Skip the first 8
+	 * bytes so we start at a nice even number.
+	 */
+	return efi_low_alloc_above(size, align, addr, 0x8);
+}
+
+efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
+				unsigned long max);
+
+efi_status_t efi_relocate_kernel(unsigned long *image_addr,
+				 unsigned long image_size,
+				 unsigned long alloc_size,
+				 unsigned long preferred_addr,
+				 unsigned long alignment,
+				 unsigned long min_addr);
+
+efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
+				  char *cmd_line, char *option_string,
+				  unsigned long max_addr,
+				  unsigned long *load_addr,
+				  unsigned long *load_size);
+
+efi_status_t efi_parse_options(char const *cmdline);
+
+efi_status_t efi_setup_gop(struct screen_info *si, efi_guid_t *proto,
+			   unsigned long size);
+
 #endif
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 2b228df18407..0e047d2738cd 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -56,19 +56,6 @@ typedef void *efi_handle_t;
 #define __efiapi
 #endif
 
-#define efi_get_handle_at(array, idx)					\
-	(efi_is_native() ? (array)[idx] 				\
-		: (efi_handle_t)(unsigned long)((u32 *)(array))[idx])
-
-#define efi_get_handle_num(size)					\
-	((size) / (efi_is_native() ? sizeof(efi_handle_t) : sizeof(u32)))
-
-#define for_each_efi_handle(handle, array, size, i)			\
-	for (i = 0;							\
-	     i < efi_get_handle_num(size) &&				\
-		((handle = efi_get_handle_at((array), i)) || true);	\
-	     i++)
-
 /*
  * The UEFI spec and EDK2 reference implementation both define EFI_GUID as
  * struct { u32 a; u16; b; u16 c; u8 d[8]; }; and so the implied alignment
@@ -157,15 +144,6 @@ typedef struct {
 	u32 imagesize;
 } efi_capsule_header_t;
 
-struct efi_boot_memmap {
-	efi_memory_desc_t	**map;
-	unsigned long		*map_size;
-	unsigned long		*desc_size;
-	u32			*desc_ver;
-	unsigned long		*key_ptr;
-	unsigned long		*buff_size;
-};
-
 /*
  * EFI capsule flags
  */
@@ -187,14 +165,6 @@ struct capsule_info {
 
 int __efi_capsule_setup_info(struct capsule_info *cap_info);
 
-/*
- * Allocation types for calls to boottime->allocate_pages.
- */
-#define EFI_ALLOCATE_ANY_PAGES		0
-#define EFI_ALLOCATE_MAX_ADDRESS	1
-#define EFI_ALLOCATE_ADDRESS		2
-#define EFI_MAX_ALLOCATE_TYPE		3
-
 typedef int (*efi_freemem_callback_t) (u64 start, u64 end, void *arg);
 
 /*
@@ -224,291 +194,7 @@ typedef struct {
 	u8 sets_to_zero;
 } efi_time_cap_t;
 
-typedef struct {
-	efi_table_hdr_t hdr;
-	u32 raise_tpl;
-	u32 restore_tpl;
-	u32 allocate_pages;
-	u32 free_pages;
-	u32 get_memory_map;
-	u32 allocate_pool;
-	u32 free_pool;
-	u32 create_event;
-	u32 set_timer;
-	u32 wait_for_event;
-	u32 signal_event;
-	u32 close_event;
-	u32 check_event;
-	u32 install_protocol_interface;
-	u32 reinstall_protocol_interface;
-	u32 uninstall_protocol_interface;
-	u32 handle_protocol;
-	u32 __reserved;
-	u32 register_protocol_notify;
-	u32 locate_handle;
-	u32 locate_device_path;
-	u32 install_configuration_table;
-	u32 load_image;
-	u32 start_image;
-	u32 exit;
-	u32 unload_image;
-	u32 exit_boot_services;
-	u32 get_next_monotonic_count;
-	u32 stall;
-	u32 set_watchdog_timer;
-	u32 connect_controller;
-	u32 disconnect_controller;
-	u32 open_protocol;
-	u32 close_protocol;
-	u32 open_protocol_information;
-	u32 protocols_per_handle;
-	u32 locate_handle_buffer;
-	u32 locate_protocol;
-	u32 install_multiple_protocol_interfaces;
-	u32 uninstall_multiple_protocol_interfaces;
-	u32 calculate_crc32;
-	u32 copy_mem;
-	u32 set_mem;
-	u32 create_event_ex;
-} __packed efi_boot_services_32_t;
-
-/*
- * EFI Boot Services table
- */
-typedef union {
-	struct {
-		efi_table_hdr_t hdr;
-		void *raise_tpl;
-		void *restore_tpl;
-		efi_status_t (__efiapi *allocate_pages)(int, int, unsigned long,
-							efi_physical_addr_t *);
-		efi_status_t (__efiapi *free_pages)(efi_physical_addr_t,
-						    unsigned long);
-		efi_status_t (__efiapi *get_memory_map)(unsigned long *, void *,
-							unsigned long *,
-							unsigned long *, u32 *);
-		efi_status_t (__efiapi *allocate_pool)(int, unsigned long,
-						       void **);
-		efi_status_t (__efiapi *free_pool)(void *);
-		void *create_event;
-		void *set_timer;
-		void *wait_for_event;
-		void *signal_event;
-		void *close_event;
-		void *check_event;
-		void *install_protocol_interface;
-		void *reinstall_protocol_interface;
-		void *uninstall_protocol_interface;
-		efi_status_t (__efiapi *handle_protocol)(efi_handle_t,
-							 efi_guid_t *, void **);
-		void *__reserved;
-		void *register_protocol_notify;
-		efi_status_t (__efiapi *locate_handle)(int, efi_guid_t *,
-						       void *, unsigned long *,
-						       efi_handle_t *);
-		void *locate_device_path;
-		efi_status_t (__efiapi *install_configuration_table)(efi_guid_t *,
-								     void *);
-		void *load_image;
-		void *start_image;
-		void *exit;
-		void *unload_image;
-		efi_status_t (__efiapi *exit_boot_services)(efi_handle_t,
-							    unsigned long);
-		void *get_next_monotonic_count;
-		void *stall;
-		void *set_watchdog_timer;
-		void *connect_controller;
-		efi_status_t (__efiapi *disconnect_controller)(efi_handle_t,
-							       efi_handle_t,
-							       efi_handle_t);
-		void *open_protocol;
-		void *close_protocol;
-		void *open_protocol_information;
-		void *protocols_per_handle;
-		void *locate_handle_buffer;
-		efi_status_t (__efiapi *locate_protocol)(efi_guid_t *, void *,
-							 void **);
-		void *install_multiple_protocol_interfaces;
-		void *uninstall_multiple_protocol_interfaces;
-		void *calculate_crc32;
-		void *copy_mem;
-		void *set_mem;
-		void *create_event_ex;
-	};
-	efi_boot_services_32_t mixed_mode;
-} efi_boot_services_t;
-
-typedef enum {
-	EfiPciIoWidthUint8,
-	EfiPciIoWidthUint16,
-	EfiPciIoWidthUint32,
-	EfiPciIoWidthUint64,
-	EfiPciIoWidthFifoUint8,
-	EfiPciIoWidthFifoUint16,
-	EfiPciIoWidthFifoUint32,
-	EfiPciIoWidthFifoUint64,
-	EfiPciIoWidthFillUint8,
-	EfiPciIoWidthFillUint16,
-	EfiPciIoWidthFillUint32,
-	EfiPciIoWidthFillUint64,
-	EfiPciIoWidthMaximum
-} EFI_PCI_IO_PROTOCOL_WIDTH;
-
-typedef enum {
-	EfiPciIoAttributeOperationGet,
-	EfiPciIoAttributeOperationSet,
-	EfiPciIoAttributeOperationEnable,
-	EfiPciIoAttributeOperationDisable,
-	EfiPciIoAttributeOperationSupported,
-    EfiPciIoAttributeOperationMaximum
-} EFI_PCI_IO_PROTOCOL_ATTRIBUTE_OPERATION;
-
-typedef struct {
-	u32 read;
-	u32 write;
-} efi_pci_io_protocol_access_32_t;
-
-typedef union efi_pci_io_protocol efi_pci_io_protocol_t;
-
-typedef
-efi_status_t (__efiapi *efi_pci_io_protocol_cfg_t)(efi_pci_io_protocol_t *,
-						   EFI_PCI_IO_PROTOCOL_WIDTH,
-						   u32 offset,
-						   unsigned long count,
-						   void *buffer);
-
-typedef struct {
-	void *read;
-	void *write;
-} efi_pci_io_protocol_access_t;
-
-typedef struct {
-	efi_pci_io_protocol_cfg_t read;
-	efi_pci_io_protocol_cfg_t write;
-} efi_pci_io_protocol_config_access_t;
-
-union efi_pci_io_protocol {
-	struct {
-		void *poll_mem;
-		void *poll_io;
-		efi_pci_io_protocol_access_t mem;
-		efi_pci_io_protocol_access_t io;
-		efi_pci_io_protocol_config_access_t pci;
-		void *copy_mem;
-		void *map;
-		void *unmap;
-		void *allocate_buffer;
-		void *free_buffer;
-		void *flush;
-		efi_status_t (__efiapi *get_location)(efi_pci_io_protocol_t *,
-						      unsigned long *segment_nr,
-						      unsigned long *bus_nr,
-						      unsigned long *device_nr,
-						      unsigned long *func_nr);
-		void *attributes;
-		void *get_bar_attributes;
-		void *set_bar_attributes;
-		uint64_t romsize;
-		void *romimage;
-	};
-	struct {
-		u32 poll_mem;
-		u32 poll_io;
-		efi_pci_io_protocol_access_32_t mem;
-		efi_pci_io_protocol_access_32_t io;
-		efi_pci_io_protocol_access_32_t pci;
-		u32 copy_mem;
-		u32 map;
-		u32 unmap;
-		u32 allocate_buffer;
-		u32 free_buffer;
-		u32 flush;
-		u32 get_location;
-		u32 attributes;
-		u32 get_bar_attributes;
-		u32 set_bar_attributes;
-		u64 romsize;
-		u32 romimage;
-	} mixed_mode;
-};
-
-#define EFI_PCI_IO_ATTRIBUTE_ISA_MOTHERBOARD_IO 0x0001
-#define EFI_PCI_IO_ATTRIBUTE_ISA_IO 0x0002
-#define EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO 0x0004
-#define EFI_PCI_IO_ATTRIBUTE_VGA_MEMORY 0x0008
-#define EFI_PCI_IO_ATTRIBUTE_VGA_IO 0x0010
-#define EFI_PCI_IO_ATTRIBUTE_IDE_PRIMARY_IO 0x0020
-#define EFI_PCI_IO_ATTRIBUTE_IDE_SECONDARY_IO 0x0040
-#define EFI_PCI_IO_ATTRIBUTE_MEMORY_WRITE_COMBINE 0x0080
-#define EFI_PCI_IO_ATTRIBUTE_IO 0x0100
-#define EFI_PCI_IO_ATTRIBUTE_MEMORY 0x0200
-#define EFI_PCI_IO_ATTRIBUTE_BUS_MASTER 0x0400
-#define EFI_PCI_IO_ATTRIBUTE_MEMORY_CACHED 0x0800
-#define EFI_PCI_IO_ATTRIBUTE_MEMORY_DISABLE 0x1000
-#define EFI_PCI_IO_ATTRIBUTE_EMBEDDED_DEVICE 0x2000
-#define EFI_PCI_IO_ATTRIBUTE_EMBEDDED_ROM 0x4000
-#define EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE 0x8000
-#define EFI_PCI_IO_ATTRIBUTE_ISA_IO_16 0x10000
-#define EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO_16 0x20000
-#define EFI_PCI_IO_ATTRIBUTE_VGA_IO_16 0x40000
-
-struct efi_dev_path;
-
-typedef union apple_properties_protocol apple_properties_protocol_t;
-
-union apple_properties_protocol {
-	struct {
-		unsigned long version;
-		efi_status_t (__efiapi *get)(apple_properties_protocol_t *,
-					     struct efi_dev_path *,
-					     efi_char16_t *, void *, u32 *);
-		efi_status_t (__efiapi *set)(apple_properties_protocol_t *,
-					     struct efi_dev_path *,
-					     efi_char16_t *, void *, u32);
-		efi_status_t (__efiapi *del)(apple_properties_protocol_t *,
-					     struct efi_dev_path *,
-					     efi_char16_t *);
-		efi_status_t (__efiapi *get_all)(apple_properties_protocol_t *,
-						 void *buffer, u32 *);
-	};
-	struct {
-		u32 version;
-		u32 get;
-		u32 set;
-		u32 del;
-		u32 get_all;
-	} mixed_mode;
-};
-
-typedef u32 efi_tcg2_event_log_format;
-
-typedef union efi_tcg2_protocol efi_tcg2_protocol_t;
-
-union efi_tcg2_protocol {
-	struct {
-		void *get_capability;
-		efi_status_t (__efiapi *get_event_log)(efi_handle_t,
-						       efi_tcg2_event_log_format,
-						       efi_physical_addr_t *,
-						       efi_physical_addr_t *,
-						       efi_bool_t *);
-		void *hash_log_extend_event;
-		void *submit_command;
-		void *get_active_pcr_banks;
-		void *set_active_pcr_banks;
-		void *get_result_of_set_active_pcr_banks;
-	};
-	struct {
-		u32 get_capability;
-		u32 get_event_log;
-		u32 hash_log_extend_event;
-		u32 submit_command;
-		u32 get_active_pcr_banks;
-		u32 set_active_pcr_banks;
-		u32 get_result_of_set_active_pcr_banks;
-	} mixed_mode;
-};
+typedef union efi_boot_services efi_boot_services_t;
 
 /*
  * Types and defines for EFI ResetSystem
@@ -796,8 +482,6 @@ struct efi_fdt_params {
 	u32 desc_ver;
 };
 
-typedef struct efi_loaded_image efi_loaded_image_t;
-
 typedef struct {
 	u32 version;
 	u32 length;
@@ -1130,13 +814,6 @@ extern int efi_status_to_err(efi_status_t status);
  */
 #define EFI_VARIABLE_GUID_LEN	UUID_STRING_LEN
 
-/*
- * The type of search to perform when calling boottime->locate_handle
- */
-#define EFI_LOCATE_ALL_HANDLES			0
-#define EFI_LOCATE_BY_REGISTER_NOTIFY		1
-#define EFI_LOCATE_BY_PROTOCOL			2
-
 /*
  * EFI Device Path information
  */
@@ -1254,80 +931,6 @@ struct efivar_entry {
 	bool deleting;
 };
 
-union efi_simple_text_output_protocol {
-	struct {
-		void *reset;
-		efi_status_t (__efiapi *output_string)(efi_simple_text_output_protocol_t *,
-						       efi_char16_t *);
-		void *test_string;
-	};
-	struct {
-		u32 reset;
-		u32 output_string;
-		u32 test_string;
-	} mixed_mode;
-};
-
-#define PIXEL_RGB_RESERVED_8BIT_PER_COLOR		0
-#define PIXEL_BGR_RESERVED_8BIT_PER_COLOR		1
-#define PIXEL_BIT_MASK					2
-#define PIXEL_BLT_ONLY					3
-#define PIXEL_FORMAT_MAX				4
-
-typedef struct {
-	u32 red_mask;
-	u32 green_mask;
-	u32 blue_mask;
-	u32 reserved_mask;
-} efi_pixel_bitmask_t;
-
-typedef struct {
-	u32 version;
-	u32 horizontal_resolution;
-	u32 vertical_resolution;
-	int pixel_format;
-	efi_pixel_bitmask_t pixel_information;
-	u32 pixels_per_scan_line;
-} efi_graphics_output_mode_info_t;
-
-typedef union efi_graphics_output_protocol_mode efi_graphics_output_protocol_mode_t;
-
-union efi_graphics_output_protocol_mode {
-	struct {
-		u32 max_mode;
-		u32 mode;
-		efi_graphics_output_mode_info_t *info;
-		unsigned long size_of_info;
-		efi_physical_addr_t frame_buffer_base;
-		unsigned long frame_buffer_size;
-	};
-	struct {
-		u32 max_mode;
-		u32 mode;
-		u32 info;
-		u32 size_of_info;
-		u64 frame_buffer_base;
-		u32 frame_buffer_size;
-	} mixed_mode;
-};
-
-typedef union efi_graphics_output_protocol efi_graphics_output_protocol_t;
-
-union efi_graphics_output_protocol {
-	struct {
-		void *query_mode;
-		void *set_mode;
-		void *blt;
-		efi_graphics_output_protocol_mode_t *mode;
-	};
-	struct {
-		u32 query_mode;
-		u32 set_mode;
-		u32 blt;
-		u32 mode;
-	} mixed_mode;
-};
-
 extern struct list_head efivar_sysfs_list;
 
 static inline void
@@ -1425,52 +1028,6 @@ static inline int efi_runtime_map_copy(void *buf, size_t bufsz)
 
 #endif
 
-/* prototypes shared between arch specific and generic stub code */
-
-void efi_printk(char *str);
-
-void efi_free(unsigned long size, unsigned long addr);
-
-char *efi_convert_cmdline(efi_loaded_image_t *image, int *cmd_line_len);
-
-efi_status_t efi_get_memory_map(struct efi_boot_memmap *map);
-
-efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
-				 unsigned long *addr, unsigned long min);
-
-static inline
-efi_status_t efi_low_alloc(unsigned long size, unsigned long align,
-			   unsigned long *addr)
-{
-	/*
-	 * Don't allocate at 0x0. It will confuse code that
-	 * checks pointers against NULL. Skip the first 8
-	 * bytes so we start at a nice even number.
-	 */
-	return efi_low_alloc_above(size, align, addr, 0x8);
-}
-
-efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
-				unsigned long max);
-
-efi_status_t efi_relocate_kernel(unsigned long *image_addr,
-				 unsigned long image_size,
-				 unsigned long alloc_size,
-				 unsigned long preferred_addr,
-				 unsigned long alignment,
-				 unsigned long min_addr);
-
-efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
-				  char *cmd_line, char *option_string,
-				  unsigned long max_addr,
-				  unsigned long *load_addr,
-				  unsigned long *load_size);
-
-efi_status_t efi_parse_options(char const *cmdline);
-
-efi_status_t efi_setup_gop(struct screen_info *si, efi_guid_t *proto,
-			   unsigned long size);
-
 #ifdef CONFIG_EFI
 extern bool efi_runtime_disabled(void);
 #else
@@ -1548,15 +1105,6 @@ void efi_retrieve_tpm2_eventlog(void);
 	arch_efi_call_virt_teardown();					\
 })
 
-typedef efi_status_t (*efi_exit_boot_map_processing)(
-	struct efi_boot_memmap *map,
-	void *priv);
-
-efi_status_t efi_exit_boot_services(void *handle,
-				    struct efi_boot_memmap *map,
-				    void *priv,
-				    efi_exit_boot_map_processing priv_func);
-
 #define EFI_RANDOM_SEED_SIZE		64U
 
 struct linux_efi_random_seed {
@@ -1643,6 +1191,4 @@ struct linux_efi_memreserve {
 #define EFI_MEMRESERVE_COUNT(size) (((size) - sizeof(struct linux_efi_memreserve)) \
 	/ sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
 
-void efi_pci_disable_bridge_busmaster(void);
-
 #endif /* _LINUX_EFI_H */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 10/19] efi/libstub/x86: Permit bootparams struct to be allocated above 4 GB
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
                   ` (8 preceding siblings ...)
  2020-02-10 16:02 ` [PATCH 09/19] efi/libstub: Move stub specific declarations into efistub.h Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 11/19] efi/libstub/x86: Permit cmdline data " Ard Biesheuvel
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

We now support bootparams structures that are located in memory that
is not 32-bit addressable, so relax the allocation limit on systems
where this feature is enabled.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/x86-stub.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 7e7c50883cce..9d60352baa0f 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -363,6 +363,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	char *cmdline_ptr;
 	unsigned long ramdisk_addr;
 	unsigned long ramdisk_size;
+	bool above4g;
 
 	sys_table = sys_table_arg;
 
@@ -376,7 +377,11 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 		return status;
 	}
 
-	status = efi_low_alloc(0x4000, 1, (unsigned long *)&boot_params);
+	hdr = &((struct boot_params *)image->image_base)->hdr;
+	above4g = hdr->xloadflags & XLF_CAN_BE_LOADED_ABOVE_4G;
+
+	status = efi_allocate_pages(0x4000, (unsigned long *)&boot_params,
+				    above4g ? ULONG_MAX : UINT_MAX);
 	if (status != EFI_SUCCESS) {
 		efi_printk("Failed to allocate lowmem for boot params\n");
 		return status;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 11/19] efi/libstub/x86: Permit cmdline data to be allocated above 4 GB
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
                   ` (9 preceding siblings ...)
  2020-02-10 16:02 ` [PATCH 10/19] efi/libstub/x86: Permit bootparams struct to be allocated above 4 GB Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 12/19] efi/libstub: Move efi_random_alloc() into separate source file Ard Biesheuvel
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

We now support cmdline data that is located in memory that is not
32-bit addressable, so relax the allocation limit on systems where
this feature is enabled.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/include/asm/efi.h                     | 2 --
 drivers/firmware/efi/libstub/arm-stub.c        | 2 +-
 drivers/firmware/efi/libstub/efi-stub-helper.c | 9 ++-------
 drivers/firmware/efi/libstub/efistub.h         | 3 ++-
 drivers/firmware/efi/libstub/x86-stub.c        | 5 +++--
 5 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 86169a24b0d8..85f79accd3f8 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -34,8 +34,6 @@ static inline bool efi_have_uv1_memmap(void)
 #define EFI32_LOADER_SIGNATURE	"EL32"
 #define EFI64_LOADER_SIGNATURE	"EL64"
 
-#define MAX_CMDLINE_ADDRESS	UINT_MAX
-
 #define ARCH_EFI_IRQ_FLAGS_MASK	X86_EFLAGS_IF
 
 /*
diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index d357393e680e..ebdf1964dd5c 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -161,7 +161,7 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table_arg,
 	 * protocol. We are going to copy the command line into the
 	 * device tree, so this can be allocated anywhere.
 	 */
-	cmdline_ptr = efi_convert_cmdline(image, &cmdline_size);
+	cmdline_ptr = efi_convert_cmdline(image, &cmdline_size, ULONG_MAX);
 	if (!cmdline_ptr) {
 		pr_efi_err("getting command line via LOADED_IMAGE_PROTOCOL\n");
 		goto fail;
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 6db91655c743..92ccd0a51ae6 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -497,17 +497,13 @@ static u8 *efi_utf16_to_utf8(u8 *dst, const u16 *src, int n)
 	return dst;
 }
 
-#ifndef MAX_CMDLINE_ADDRESS
-#define MAX_CMDLINE_ADDRESS	ULONG_MAX
-#endif
-
 /*
  * Convert the unicode UEFI command line to ASCII to pass to kernel.
  * Size of memory allocated return in *cmd_line_len.
  * Returns NULL on error.
  */
 char *efi_convert_cmdline(efi_loaded_image_t *image,
-			  int *cmd_line_len)
+			  int *cmd_line_len, unsigned long max_addr)
 {
 	const u16 *s2;
 	u8 *s1 = NULL;
@@ -535,8 +531,7 @@ char *efi_convert_cmdline(efi_loaded_image_t *image,
 
 	options_bytes++;	/* NUL termination */
 
-	status = efi_allocate_pages(options_bytes, &cmdline_addr,
-				MAX_CMDLINE_ADDRESS);
+	status = efi_allocate_pages(options_bytes, &cmdline_addr, max_addr);
 	if (status != EFI_SUCCESS)
 		return NULL;
 
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 8bb46c1fd2cd..b94c63d17a4f 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -578,7 +578,8 @@ void efi_printk(char *str);
 
 void efi_free(unsigned long size, unsigned long addr);
 
-char *efi_convert_cmdline(efi_loaded_image_t *image, int *cmd_line_len);
+char *efi_convert_cmdline(efi_loaded_image_t *image, int *cmd_line_len,
+			  unsigned long max_addr);
 
 efi_status_t efi_get_memory_map(struct efi_boot_memmap *map);
 
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 9d60352baa0f..f2dbf119837c 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -405,7 +405,8 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	hdr->type_of_loader = 0x21;
 
 	/* Convert unicode cmdline to ascii */
-	cmdline_ptr = efi_convert_cmdline(image, &options_size);
+	cmdline_ptr = efi_convert_cmdline(image, &options_size,
+					  above4g ? ULONG_MAX : UINT_MAX);
 	if (!cmdline_ptr)
 		goto fail;
 
@@ -445,7 +446,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	/* not reached */
 
 fail2:
-	efi_free(options_size, hdr->cmd_line_ptr);
+	efi_free(options_size, (unsigned long)cmdline_ptr);
 fail:
 	efi_free(0x4000, (unsigned long)boot_params);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 12/19] efi/libstub: Move efi_random_alloc() into separate source file
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
                   ` (10 preceding siblings ...)
  2020-02-10 16:02 ` [PATCH 11/19] efi/libstub/x86: Permit cmdline data " Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 13/19] efi/libstub: Move get_dram_base() into arm-stub.c Ard Biesheuvel
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

efi_random_alloc() is only used on arm64, but as it shares a source
file with efi_random_get_seed(), the latter will pull in the former
on other architectures as well.

Let's take advantage of the fact that libstub is a static library,
and so the linker will only incorporate objects that are needed to
satisfy dependencies in other objects. This means we can move the
random alloc code to a separate source file that gets built
unconditionally, but only used when needed.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/Makefile      |   2 +-
 drivers/firmware/efi/libstub/random.c      | 114 ------------------
 drivers/firmware/efi/libstub/randomalloc.c | 124 ++++++++++++++++++++
 3 files changed, 125 insertions(+), 115 deletions(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 8e15634fa929..ac0d0e1ef32d 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -39,7 +39,7 @@ OBJECT_FILES_NON_STANDARD	:= y
 KCOV_INSTRUMENT			:= n
 
 lib-y				:= efi-stub-helper.o gop.o secureboot.o tpm.o \
-				   mem.o random.o pci.o
+				   mem.o random.o randomalloc.o pci.o
 
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
diff --git a/drivers/firmware/efi/libstub/random.c b/drivers/firmware/efi/libstub/random.c
index 316ce9ff0193..21e7e9325219 100644
--- a/drivers/firmware/efi/libstub/random.c
+++ b/drivers/firmware/efi/libstub/random.c
@@ -4,7 +4,6 @@
  */
 
 #include <linux/efi.h>
-#include <linux/log2.h>
 #include <asm/efi.h>
 
 #include "efistub.h"
@@ -39,119 +38,6 @@ efi_status_t efi_get_random_bytes(unsigned long size, u8 *out)
 	return efi_call_proto(rng, get_rng, NULL, size, out);
 }
 
-/*
- * Return the number of slots covered by this entry, i.e., the number of
- * addresses it covers that are suitably aligned and supply enough room
- * for the allocation.
- */
-static unsigned long get_entry_num_slots(efi_memory_desc_t *md,
-					 unsigned long size,
-					 unsigned long align_shift)
-{
-	unsigned long align = 1UL << align_shift;
-	u64 first_slot, last_slot, region_end;
-
-	if (md->type != EFI_CONVENTIONAL_MEMORY)
-		return 0;
-
-	if (efi_soft_reserve_enabled() &&
-	    (md->attribute & EFI_MEMORY_SP))
-		return 0;
-
-	region_end = min((u64)ULONG_MAX, md->phys_addr + md->num_pages*EFI_PAGE_SIZE - 1);
-
-	first_slot = round_up(md->phys_addr, align);
-	last_slot = round_down(region_end - size + 1, align);
-
-	if (first_slot > last_slot)
-		return 0;
-
-	return ((unsigned long)(last_slot - first_slot) >> align_shift) + 1;
-}
-
-/*
- * The UEFI memory descriptors have a virtual address field that is only used
- * when installing the virtual mapping using SetVirtualAddressMap(). Since it
- * is unused here, we can reuse it to keep track of each descriptor's slot
- * count.
- */
-#define MD_NUM_SLOTS(md)	((md)->virt_addr)
-
-efi_status_t efi_random_alloc(unsigned long size,
-			      unsigned long align,
-			      unsigned long *addr,
-			      unsigned long random_seed)
-{
-	unsigned long map_size, desc_size, total_slots = 0, target_slot;
-	unsigned long buff_size;
-	efi_status_t status;
-	efi_memory_desc_t *memory_map;
-	int map_offset;
-	struct efi_boot_memmap map;
-
-	map.map =	&memory_map;
-	map.map_size =	&map_size;
-	map.desc_size =	&desc_size;
-	map.desc_ver =	NULL;
-	map.key_ptr =	NULL;
-	map.buff_size =	&buff_size;
-
-	status = efi_get_memory_map(&map);
-	if (status != EFI_SUCCESS)
-		return status;
-
-	if (align < EFI_ALLOC_ALIGN)
-		align = EFI_ALLOC_ALIGN;
-
-	/* count the suitable slots in each memory map entry */
-	for (map_offset = 0; map_offset < map_size; map_offset += desc_size) {
-		efi_memory_desc_t *md = (void *)memory_map + map_offset;
-		unsigned long slots;
-
-		slots = get_entry_num_slots(md, size, ilog2(align));
-		MD_NUM_SLOTS(md) = slots;
-		total_slots += slots;
-	}
-
-	/* find a random number between 0 and total_slots */
-	target_slot = (total_slots * (u16)random_seed) >> 16;
-
-	/*
-	 * target_slot is now a value in the range [0, total_slots), and so
-	 * it corresponds with exactly one of the suitable slots we recorded
-	 * when iterating over the memory map the first time around.
-	 *
-	 * So iterate over the memory map again, subtracting the number of
-	 * slots of each entry at each iteration, until we have found the entry
-	 * that covers our chosen slot. Use the residual value of target_slot
-	 * to calculate the randomly chosen address, and allocate it directly
-	 * using EFI_ALLOCATE_ADDRESS.
-	 */
-	for (map_offset = 0; map_offset < map_size; map_offset += desc_size) {
-		efi_memory_desc_t *md = (void *)memory_map + map_offset;
-		efi_physical_addr_t target;
-		unsigned long pages;
-
-		if (target_slot >= MD_NUM_SLOTS(md)) {
-			target_slot -= MD_NUM_SLOTS(md);
-			continue;
-		}
-
-		target = round_up(md->phys_addr, align) + target_slot * align;
-		pages = round_up(size, EFI_PAGE_SIZE) / EFI_PAGE_SIZE;
-
-		status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
-				     EFI_LOADER_DATA, pages, &target);
-		if (status == EFI_SUCCESS)
-			*addr = target;
-		break;
-	}
-
-	efi_bs_call(free_pool, memory_map);
-
-	return status;
-}
-
 efi_status_t efi_random_get_seed(void)
 {
 	efi_guid_t rng_proto = EFI_RNG_PROTOCOL_GUID;
diff --git a/drivers/firmware/efi/libstub/randomalloc.c b/drivers/firmware/efi/libstub/randomalloc.c
new file mode 100644
index 000000000000..4578f59e160c
--- /dev/null
+++ b/drivers/firmware/efi/libstub/randomalloc.c
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016 Linaro Ltd;  <ard.biesheuvel@linaro.org>
+ */
+
+#include <linux/efi.h>
+#include <linux/log2.h>
+#include <asm/efi.h>
+
+#include "efistub.h"
+
+/*
+ * Return the number of slots covered by this entry, i.e., the number of
+ * addresses it covers that are suitably aligned and supply enough room
+ * for the allocation.
+ */
+static unsigned long get_entry_num_slots(efi_memory_desc_t *md,
+					 unsigned long size,
+					 unsigned long align_shift)
+{
+	unsigned long align = 1UL << align_shift;
+	u64 first_slot, last_slot, region_end;
+
+	if (md->type != EFI_CONVENTIONAL_MEMORY)
+		return 0;
+
+	if (efi_soft_reserve_enabled() &&
+	    (md->attribute & EFI_MEMORY_SP))
+		return 0;
+
+	region_end = min(md->phys_addr + md->num_pages * EFI_PAGE_SIZE - 1,
+			 (u64)ULONG_MAX);
+
+	first_slot = round_up(md->phys_addr, align);
+	last_slot = round_down(region_end - size + 1, align);
+
+	if (first_slot > last_slot)
+		return 0;
+
+	return ((unsigned long)(last_slot - first_slot) >> align_shift) + 1;
+}
+
+/*
+ * The UEFI memory descriptors have a virtual address field that is only used
+ * when installing the virtual mapping using SetVirtualAddressMap(). Since it
+ * is unused here, we can reuse it to keep track of each descriptor's slot
+ * count.
+ */
+#define MD_NUM_SLOTS(md)	((md)->virt_addr)
+
+efi_status_t efi_random_alloc(unsigned long size,
+			      unsigned long align,
+			      unsigned long *addr,
+			      unsigned long random_seed)
+{
+	unsigned long map_size, desc_size, total_slots = 0, target_slot;
+	unsigned long buff_size;
+	efi_status_t status;
+	efi_memory_desc_t *memory_map;
+	int map_offset;
+	struct efi_boot_memmap map;
+
+	map.map =	&memory_map;
+	map.map_size =	&map_size;
+	map.desc_size =	&desc_size;
+	map.desc_ver =	NULL;
+	map.key_ptr =	NULL;
+	map.buff_size =	&buff_size;
+
+	status = efi_get_memory_map(&map);
+	if (status != EFI_SUCCESS)
+		return status;
+
+	if (align < EFI_ALLOC_ALIGN)
+		align = EFI_ALLOC_ALIGN;
+
+	/* count the suitable slots in each memory map entry */
+	for (map_offset = 0; map_offset < map_size; map_offset += desc_size) {
+		efi_memory_desc_t *md = (void *)memory_map + map_offset;
+		unsigned long slots;
+
+		slots = get_entry_num_slots(md, size, ilog2(align));
+		MD_NUM_SLOTS(md) = slots;
+		total_slots += slots;
+	}
+
+	/* find a random number between 0 and total_slots */
+	target_slot = (total_slots * (u16)random_seed) >> 16;
+
+	/*
+	 * target_slot is now a value in the range [0, total_slots), and so
+	 * it corresponds with exactly one of the suitable slots we recorded
+	 * when iterating over the memory map the first time around.
+	 *
+	 * So iterate over the memory map again, subtracting the number of
+	 * slots of each entry at each iteration, until we have found the entry
+	 * that covers our chosen slot. Use the residual value of target_slot
+	 * to calculate the randomly chosen address, and allocate it directly
+	 * using EFI_ALLOCATE_ADDRESS.
+	 */
+	for (map_offset = 0; map_offset < map_size; map_offset += desc_size) {
+		efi_memory_desc_t *md = (void *)memory_map + map_offset;
+		efi_physical_addr_t target;
+		unsigned long pages;
+
+		if (target_slot >= MD_NUM_SLOTS(md)) {
+			target_slot -= MD_NUM_SLOTS(md);
+			continue;
+		}
+
+		target = round_up(md->phys_addr, align) + target_slot * align;
+		pages = round_up(size, EFI_PAGE_SIZE) / EFI_PAGE_SIZE;
+
+		status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
+				     EFI_LOADER_DATA, pages, &target);
+		if (status == EFI_SUCCESS)
+			*addr = target;
+		break;
+	}
+
+	efi_bs_call(free_pool, memory_map);
+
+	return status;
+}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 13/19] efi/libstub: Move get_dram_base() into arm-stub.c
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
                   ` (11 preceding siblings ...)
  2020-02-10 16:02 ` [PATCH 12/19] efi/libstub: Move efi_random_alloc() into separate source file Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-17  1:17   ` Atish Patra
  2020-02-10 16:02 ` [PATCH 14/19] efi/libstub: Move file I/O support code into separate file Ard Biesheuvel
                   ` (5 subsequent siblings)
  18 siblings, 1 reply; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

get_dram_base() is only called from arm-stub.c so move it into
the same source file as its caller.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/arm-stub.c        | 33 ++++++++++++++++++
 drivers/firmware/efi/libstub/efi-stub-helper.c | 35 --------------------
 drivers/firmware/efi/libstub/efistub.h         |  2 --
 3 files changed, 33 insertions(+), 37 deletions(-)

diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index ebdf1964dd5c..fb5b2b35d3be 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -87,6 +87,39 @@ void install_memreserve_table(void)
 		pr_efi_err("Failed to install memreserve config table!\n");
 }
 
+static unsigned long get_dram_base(void)
+{
+	efi_status_t status;
+	unsigned long map_size, buff_size;
+	unsigned long membase  = EFI_ERROR;
+	struct efi_memory_map map;
+	efi_memory_desc_t *md;
+	struct efi_boot_memmap boot_map;
+
+	boot_map.map		= (efi_memory_desc_t **)&map.map;
+	boot_map.map_size	= &map_size;
+	boot_map.desc_size	= &map.desc_size;
+	boot_map.desc_ver	= NULL;
+	boot_map.key_ptr	= NULL;
+	boot_map.buff_size	= &buff_size;
+
+	status = efi_get_memory_map(&boot_map);
+	if (status != EFI_SUCCESS)
+		return membase;
+
+	map.map_end = map.map + map_size;
+
+	for_each_efi_memory_desc_in_map(&map, md) {
+		if (md->attribute & EFI_MEMORY_WB) {
+			if (membase > md->phys_addr)
+				membase = md->phys_addr;
+		}
+	}
+
+	efi_bs_call(free_pool, map.map);
+
+	return membase;
+}
 
 /*
  * This function handles the architcture specific differences between arm and
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 92ccd0a51ae6..1a8f2cf5a2bd 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -75,41 +75,6 @@ void efi_printk(char *str)
 	}
 }
 
-
-unsigned long get_dram_base(void)
-{
-	efi_status_t status;
-	unsigned long map_size, buff_size;
-	unsigned long membase  = EFI_ERROR;
-	struct efi_memory_map map;
-	efi_memory_desc_t *md;
-	struct efi_boot_memmap boot_map;
-
-	boot_map.map =		(efi_memory_desc_t **)&map.map;
-	boot_map.map_size =	&map_size;
-	boot_map.desc_size =	&map.desc_size;
-	boot_map.desc_ver =	NULL;
-	boot_map.key_ptr =	NULL;
-	boot_map.buff_size =	&buff_size;
-
-	status = efi_get_memory_map(&boot_map);
-	if (status != EFI_SUCCESS)
-		return membase;
-
-	map.map_end = map.map + map_size;
-
-	for_each_efi_memory_desc_in_map(&map, md) {
-		if (md->attribute & EFI_MEMORY_WB) {
-			if (membase > md->phys_addr)
-				membase = md->phys_addr;
-		}
-	}
-
-	efi_bs_call(free_pool, map.map);
-
-	return membase;
-}
-
 static efi_status_t efi_file_size(void *__fh, efi_char16_t *filename_16,
 				  void **handle, u64 *file_sz)
 {
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index b94c63d17a4f..5123def761e9 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -549,8 +549,6 @@ efi_status_t efi_exit_boot_services(void *handle,
 
 void efi_char16_printk(efi_char16_t *);
 
-unsigned long get_dram_base(void);
-
 efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
 					    unsigned long *new_fdt_addr,
 					    unsigned long max_addr,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 14/19] efi/libstub: Move file I/O support code into separate file
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
                   ` (12 preceding siblings ...)
  2020-02-10 16:02 ` [PATCH 13/19] efi/libstub: Move get_dram_base() into arm-stub.c Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 15/19] efi/libstub: Rewrite file I/O routine Ard Biesheuvel
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

Split off the file I/O support code into a separate source file so
it ends up in a separate object file in the static library, allowing
the linker to omit it if the routines are not used.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/Makefile          |   2 +-
 drivers/firmware/efi/libstub/efi-stub-helper.c | 294 +------------------
 drivers/firmware/efi/libstub/efistub.h         |   1 +
 drivers/firmware/efi/libstub/file.c            | 303 ++++++++++++++++++++
 4 files changed, 311 insertions(+), 289 deletions(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index ac0d0e1ef32d..74a097ef1780 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -39,7 +39,7 @@ OBJECT_FILES_NON_STANDARD	:= y
 KCOV_INSTRUMENT			:= n
 
 lib-y				:= efi-stub-helper.o gop.o secureboot.o tpm.o \
-				   mem.o random.o randomalloc.o pci.o
+				   file.o mem.o random.o randomalloc.o pci.o
 
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 1a8f2cf5a2bd..db23be5dc69b 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -12,23 +12,7 @@
 
 #include "efistub.h"
 
-/*
- * Some firmware implementations have problems reading files in one go.
- * A read chunk size of 1MB seems to work for most platforms.
- *
- * Unfortunately, reading files in chunks triggers *other* bugs on some
- * platforms, so we provide a way to disable this workaround, which can
- * be done by passing "efi=nochunk" on the EFI boot stub command line.
- *
- * If you experience issues with initrd images being corrupt it's worth
- * trying efi=nochunk, but chunking is enabled by default because there
- * are far more machines that require the workaround than those that
- * break with it enabled.
- */
-#define EFI_READ_CHUNK_SIZE	(1024 * 1024)
-
-static unsigned long efi_chunk_size = EFI_READ_CHUNK_SIZE;
-
+static bool __efistub_global efi_nochunk;
 static bool __efistub_global efi_nokaslr;
 static bool __efistub_global efi_quiet;
 static bool __efistub_global efi_novamap;
@@ -36,6 +20,10 @@ static bool __efistub_global efi_nosoftreserve;
 static bool __efistub_global efi_disable_pci_dma =
 					IS_ENABLED(CONFIG_EFI_DISABLE_PCI_DMA);
 
+bool __pure nochunk(void)
+{
+	return efi_nochunk;
+}
 bool __pure nokaslr(void)
 {
 	return efi_nokaslr;
@@ -53,11 +41,6 @@ bool __pure __efi_soft_reserve_enabled(void)
 	return !efi_nosoftreserve;
 }
 
-struct file_info {
-	efi_file_protocol_t *handle;
-	u64 size;
-};
-
 void efi_printk(char *str)
 {
 	char *s8;
@@ -75,90 +58,6 @@ void efi_printk(char *str)
 	}
 }
 
-static efi_status_t efi_file_size(void *__fh, efi_char16_t *filename_16,
-				  void **handle, u64 *file_sz)
-{
-	efi_file_protocol_t *h, *fh = __fh;
-	efi_file_info_t *info;
-	efi_status_t status;
-	efi_guid_t info_guid = EFI_FILE_INFO_ID;
-	unsigned long info_sz;
-
-	status = fh->open(fh, &h, filename_16, EFI_FILE_MODE_READ, 0);
-	if (status != EFI_SUCCESS) {
-		efi_printk("Failed to open file: ");
-		efi_char16_printk(filename_16);
-		efi_printk("\n");
-		return status;
-	}
-
-	*handle = h;
-
-	info_sz = 0;
-	status = h->get_info(h, &info_guid, &info_sz, NULL);
-	if (status != EFI_BUFFER_TOO_SMALL) {
-		efi_printk("Failed to get file info size\n");
-		return status;
-	}
-
-grow:
-	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, info_sz,
-			     (void **)&info);
-	if (status != EFI_SUCCESS) {
-		efi_printk("Failed to alloc mem for file info\n");
-		return status;
-	}
-
-	status = h->get_info(h, &info_guid, &info_sz, info);
-	if (status == EFI_BUFFER_TOO_SMALL) {
-		efi_bs_call(free_pool, info);
-		goto grow;
-	}
-
-	*file_sz = info->file_size;
-	efi_bs_call(free_pool, info);
-
-	if (status != EFI_SUCCESS)
-		efi_printk("Failed to get initrd info\n");
-
-	return status;
-}
-
-static efi_status_t efi_file_read(efi_file_protocol_t *handle,
-				  unsigned long *size, void *addr)
-{
-	return handle->read(handle, size, addr);
-}
-
-static efi_status_t efi_file_close(efi_file_protocol_t *handle)
-{
-	return handle->close(handle);
-}
-
-static efi_status_t efi_open_volume(efi_loaded_image_t *image,
-				    efi_file_protocol_t **__fh)
-{
-	efi_simple_file_system_protocol_t *io;
-	efi_file_protocol_t *fh;
-	efi_guid_t fs_proto = EFI_FILE_SYSTEM_GUID;
-	efi_status_t status;
-	efi_handle_t handle = image->device_handle;
-
-	status = efi_bs_call(handle_protocol, handle, &fs_proto, (void **)&io);
-	if (status != EFI_SUCCESS) {
-		efi_printk("Failed to handle fs_proto\n");
-		return status;
-	}
-
-	status = io->open_volume(io, &fh);
-	if (status != EFI_SUCCESS)
-		efi_printk("Failed to open volume\n");
-	else
-		*__fh = fh;
-
-	return status;
-}
-
 /*
  * Parse the ASCII string 'cmdline' for EFI options, denoted by the efi=
  * option, e.g. efi=nochunk.
@@ -197,7 +96,7 @@ efi_status_t efi_parse_options(char const *cmdline)
 	while (*str && *str != ' ') {
 		if (!strncmp(str, "nochunk", 7)) {
 			str += strlen("nochunk");
-			efi_chunk_size = -1UL;
+			efi_nochunk = true;
 		}
 
 		if (!strncmp(str, "novamap", 7)) {
@@ -232,187 +131,6 @@ efi_status_t efi_parse_options(char const *cmdline)
 	return EFI_SUCCESS;
 }
 
-/*
- * Check the cmdline for a LILO-style file= arguments.
- *
- * We only support loading a file from the same filesystem as
- * the kernel image.
- */
-efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
-				  char *cmd_line, char *option_string,
-				  unsigned long max_addr,
-				  unsigned long *load_addr,
-				  unsigned long *load_size)
-{
-	struct file_info *files;
-	unsigned long file_addr;
-	u64 file_size_total;
-	efi_file_protocol_t *fh = NULL;
-	efi_status_t status;
-	int nr_files;
-	char *str;
-	int i, j, k;
-
-	file_addr = 0;
-	file_size_total = 0;
-
-	str = cmd_line;
-
-	j = 0;			/* See close_handles */
-
-	if (!load_addr || !load_size)
-		return EFI_INVALID_PARAMETER;
-
-	*load_addr = 0;
-	*load_size = 0;
-
-	if (!str || !*str)
-		return EFI_SUCCESS;
-
-	for (nr_files = 0; *str; nr_files++) {
-		str = strstr(str, option_string);
-		if (!str)
-			break;
-
-		str += strlen(option_string);
-
-		/* Skip any leading slashes */
-		while (*str == '/' || *str == '\\')
-			str++;
-
-		while (*str && *str != ' ' && *str != '\n')
-			str++;
-	}
-
-	if (!nr_files)
-		return EFI_SUCCESS;
-
-	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA,
-			     nr_files * sizeof(*files), (void **)&files);
-	if (status != EFI_SUCCESS) {
-		pr_efi_err("Failed to alloc mem for file handle list\n");
-		goto fail;
-	}
-
-	str = cmd_line;
-	for (i = 0; i < nr_files; i++) {
-		struct file_info *file;
-		efi_char16_t filename_16[256];
-		efi_char16_t *p;
-
-		str = strstr(str, option_string);
-		if (!str)
-			break;
-
-		str += strlen(option_string);
-
-		file = &files[i];
-		p = filename_16;
-
-		/* Skip any leading slashes */
-		while (*str == '/' || *str == '\\')
-			str++;
-
-		while (*str && *str != ' ' && *str != '\n') {
-			if ((u8 *)p >= (u8 *)filename_16 + sizeof(filename_16))
-				break;
-
-			if (*str == '/') {
-				*p++ = '\\';
-				str++;
-			} else {
-				*p++ = *str++;
-			}
-		}
-
-		*p = '\0';
-
-		/* Only open the volume once. */
-		if (!i) {
-			status = efi_open_volume(image, &fh);
-			if (status != EFI_SUCCESS)
-				goto free_files;
-		}
-
-		status = efi_file_size(fh, filename_16, (void **)&file->handle,
-				       &file->size);
-		if (status != EFI_SUCCESS)
-			goto close_handles;
-
-		file_size_total += file->size;
-	}
-
-	if (file_size_total) {
-		unsigned long addr;
-
-		/*
-		 * Multiple files need to be at consecutive addresses in memory,
-		 * so allocate enough memory for all the files.  This is used
-		 * for loading multiple files.
-		 */
-		status = efi_allocate_pages(file_size_total, &file_addr, max_addr);
-		if (status != EFI_SUCCESS) {
-			pr_efi_err("Failed to alloc highmem for files\n");
-			goto close_handles;
-		}
-
-		/* We've run out of free low memory. */
-		if (file_addr > max_addr) {
-			pr_efi_err("We've run out of free low memory\n");
-			status = EFI_INVALID_PARAMETER;
-			goto free_file_total;
-		}
-
-		addr = file_addr;
-		for (j = 0; j < nr_files; j++) {
-			unsigned long size;
-
-			size = files[j].size;
-			while (size) {
-				unsigned long chunksize;
-
-				if (IS_ENABLED(CONFIG_X86) && size > efi_chunk_size)
-					chunksize = efi_chunk_size;
-				else
-					chunksize = size;
-
-				status = efi_file_read(files[j].handle,
-						       &chunksize,
-						       (void *)addr);
-				if (status != EFI_SUCCESS) {
-					pr_efi_err("Failed to read file\n");
-					goto free_file_total;
-				}
-				addr += chunksize;
-				size -= chunksize;
-			}
-
-			efi_file_close(files[j].handle);
-		}
-
-	}
-
-	efi_bs_call(free_pool, files);
-
-	*load_addr = file_addr;
-	*load_size = file_size_total;
-
-	return status;
-
-free_file_total:
-	efi_free(file_size_total, file_addr);
-
-close_handles:
-	for (k = j; k < i; k++)
-		efi_file_close(files[k].handle);
-free_files:
-	efi_bs_call(free_pool, files);
-fail:
-	*load_addr = 0;
-	*load_size = 0;
-
-	return status;
-}
 /*
  * Get the number of UTF-8 bytes corresponding to an UTF-16 character.
  * This overestimates for surrogates, but that is okay.
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 5123def761e9..e057d509d5d8 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -31,6 +31,7 @@
 #define __efistub_global
 #endif
 
+extern bool __pure nochunk(void);
 extern bool __pure nokaslr(void);
 extern bool __pure is_quiet(void);
 extern bool __pure novamap(void);
diff --git a/drivers/firmware/efi/libstub/file.c b/drivers/firmware/efi/libstub/file.c
new file mode 100644
index 000000000000..e0302f340962
--- /dev/null
+++ b/drivers/firmware/efi/libstub/file.c
@@ -0,0 +1,303 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Helper functions used by the EFI stub on multiple
+ * architectures. This should be #included by the EFI stub
+ * implementation files.
+ *
+ * Copyright 2011 Intel Corporation; author Matt Fleming
+ */
+
+#include <linux/efi.h>
+#include <asm/efi.h>
+
+#include "efistub.h"
+
+/*
+ * Some firmware implementations have problems reading files in one go.
+ * A read chunk size of 1MB seems to work for most platforms.
+ *
+ * Unfortunately, reading files in chunks triggers *other* bugs on some
+ * platforms, so we provide a way to disable this workaround, which can
+ * be done by passing "efi=nochunk" on the EFI boot stub command line.
+ *
+ * If you experience issues with initrd images being corrupt it's worth
+ * trying efi=nochunk, but chunking is enabled by default on x86 because
+ * there are far more machines that require the workaround than those that
+ * break with it enabled.
+ */
+#define EFI_READ_CHUNK_SIZE	SZ_1M
+
+struct file_info {
+	efi_file_protocol_t *handle;
+	u64 size;
+};
+
+static efi_status_t efi_file_size(void *__fh, efi_char16_t *filename_16,
+				  void **handle, u64 *file_sz)
+{
+	efi_file_protocol_t *h, *fh = __fh;
+	efi_file_info_t *info;
+	efi_status_t status;
+	efi_guid_t info_guid = EFI_FILE_INFO_ID;
+	unsigned long info_sz;
+
+	status = fh->open(fh, &h, filename_16, EFI_FILE_MODE_READ, 0);
+	if (status != EFI_SUCCESS) {
+		efi_printk("Failed to open file: ");
+		efi_char16_printk(filename_16);
+		efi_printk("\n");
+		return status;
+	}
+
+	*handle = h;
+
+	info_sz = 0;
+	status = h->get_info(h, &info_guid, &info_sz, NULL);
+	if (status != EFI_BUFFER_TOO_SMALL) {
+		efi_printk("Failed to get file info size\n");
+		return status;
+	}
+
+grow:
+	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, info_sz,
+			     (void **)&info);
+	if (status != EFI_SUCCESS) {
+		efi_printk("Failed to alloc mem for file info\n");
+		return status;
+	}
+
+	status = h->get_info(h, &info_guid, &info_sz, info);
+	if (status == EFI_BUFFER_TOO_SMALL) {
+		efi_bs_call(free_pool, info);
+		goto grow;
+	}
+
+	*file_sz = info->file_size;
+	efi_bs_call(free_pool, info);
+
+	if (status != EFI_SUCCESS)
+		efi_printk("Failed to get initrd info\n");
+
+	return status;
+}
+
+static efi_status_t efi_file_read(efi_file_protocol_t *handle,
+				  unsigned long *size, void *addr)
+{
+	return handle->read(handle, size, addr);
+}
+
+static efi_status_t efi_file_close(efi_file_protocol_t *handle)
+{
+	return handle->close(handle);
+}
+
+static efi_status_t efi_open_volume(efi_loaded_image_t *image,
+				    efi_file_protocol_t **__fh)
+{
+	efi_simple_file_system_protocol_t *io;
+	efi_file_protocol_t *fh;
+	efi_guid_t fs_proto = EFI_FILE_SYSTEM_GUID;
+	efi_status_t status;
+	efi_handle_t handle = image->device_handle;
+
+	status = efi_bs_call(handle_protocol, handle, &fs_proto, (void **)&io);
+	if (status != EFI_SUCCESS) {
+		efi_printk("Failed to handle fs_proto\n");
+		return status;
+	}
+
+	status = io->open_volume(io, &fh);
+	if (status != EFI_SUCCESS)
+		efi_printk("Failed to open volume\n");
+	else
+		*__fh = fh;
+
+	return status;
+}
+
+/*
+ * Check the cmdline for a LILO-style file= arguments.
+ *
+ * We only support loading a file from the same filesystem as
+ * the kernel image.
+ */
+efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
+				  char *cmd_line, char *option_string,
+				  unsigned long max_addr,
+				  unsigned long *load_addr,
+				  unsigned long *load_size)
+{
+	unsigned long efi_chunk_size = ULONG_MAX;
+	struct file_info *files;
+	unsigned long file_addr;
+	u64 file_size_total;
+	efi_file_protocol_t *fh = NULL;
+	efi_status_t status;
+	int nr_files;
+	char *str;
+	int i, j, k;
+
+	if (IS_ENABLED(CONFIG_X86) && !nochunk())
+		efi_chunk_size = EFI_READ_CHUNK_SIZE;
+
+	file_addr = 0;
+	file_size_total = 0;
+
+	str = cmd_line;
+
+	j = 0;			/* See close_handles */
+
+	if (!load_addr || !load_size)
+		return EFI_INVALID_PARAMETER;
+
+	*load_addr = 0;
+	*load_size = 0;
+
+	if (!str || !*str)
+		return EFI_SUCCESS;
+
+	for (nr_files = 0; *str; nr_files++) {
+		str = strstr(str, option_string);
+		if (!str)
+			break;
+
+		str += strlen(option_string);
+
+		/* Skip any leading slashes */
+		while (*str == '/' || *str == '\\')
+			str++;
+
+		while (*str && *str != ' ' && *str != '\n')
+			str++;
+	}
+
+	if (!nr_files)
+		return EFI_SUCCESS;
+
+	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA,
+			     nr_files * sizeof(*files), (void **)&files);
+	if (status != EFI_SUCCESS) {
+		pr_efi_err("Failed to alloc mem for file handle list\n");
+		goto fail;
+	}
+
+	str = cmd_line;
+	for (i = 0; i < nr_files; i++) {
+		struct file_info *file;
+		efi_char16_t filename_16[256];
+		efi_char16_t *p;
+
+		str = strstr(str, option_string);
+		if (!str)
+			break;
+
+		str += strlen(option_string);
+
+		file = &files[i];
+		p = filename_16;
+
+		/* Skip any leading slashes */
+		while (*str == '/' || *str == '\\')
+			str++;
+
+		while (*str && *str != ' ' && *str != '\n') {
+			if ((u8 *)p >= (u8 *)filename_16 + sizeof(filename_16))
+				break;
+
+			if (*str == '/') {
+				*p++ = '\\';
+				str++;
+			} else {
+				*p++ = *str++;
+			}
+		}
+
+		*p = '\0';
+
+		/* Only open the volume once. */
+		if (!i) {
+			status = efi_open_volume(image, &fh);
+			if (status != EFI_SUCCESS)
+				goto free_files;
+		}
+
+		status = efi_file_size(fh, filename_16, (void **)&file->handle,
+				       &file->size);
+		if (status != EFI_SUCCESS)
+			goto close_handles;
+
+		file_size_total += file->size;
+	}
+
+	if (file_size_total) {
+		unsigned long addr;
+
+		/*
+		 * Multiple files need to be at consecutive addresses in memory,
+		 * so allocate enough memory for all the files.  This is used
+		 * for loading multiple files.
+		 */
+		status = efi_allocate_pages(file_size_total, &file_addr, max_addr);
+		if (status != EFI_SUCCESS) {
+			pr_efi_err("Failed to alloc highmem for files\n");
+			goto close_handles;
+		}
+
+		/* We've run out of free low memory. */
+		if (file_addr > max_addr) {
+			pr_efi_err("We've run out of free low memory\n");
+			status = EFI_INVALID_PARAMETER;
+			goto free_file_total;
+		}
+
+		addr = file_addr;
+		for (j = 0; j < nr_files; j++) {
+			unsigned long size;
+
+			size = files[j].size;
+			while (size) {
+				unsigned long chunksize;
+
+				if (size > efi_chunk_size)
+					chunksize = efi_chunk_size;
+				else
+					chunksize = size;
+
+				status = efi_file_read(files[j].handle,
+						       &chunksize,
+						       (void *)addr);
+				if (status != EFI_SUCCESS) {
+					pr_efi_err("Failed to read file\n");
+					goto free_file_total;
+				}
+				addr += chunksize;
+				size -= chunksize;
+			}
+
+			efi_file_close(files[j].handle);
+		}
+
+	}
+
+	efi_bs_call(free_pool, files);
+
+	*load_addr = file_addr;
+	*load_size = file_size_total;
+
+	return status;
+
+free_file_total:
+	efi_free(file_size_total, file_addr);
+
+close_handles:
+	for (k = j; k < i; k++)
+		efi_file_close(files[k].handle);
+free_files:
+	efi_bs_call(free_pool, files);
+fail:
+	*load_addr = 0;
+	*load_size = 0;
+
+	return status;
+}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 15/19] efi/libstub: Rewrite file I/O routine
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
                   ` (13 preceding siblings ...)
  2020-02-10 16:02 ` [PATCH 14/19] efi/libstub: Move file I/O support code into separate file Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 16/19] efi/libstub: Take soft and hard memory limits into account for initrd loading Ard Biesheuvel
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

The file I/O routine that is used to load initrd or dtb files from
the EFI system partition suffers from a few issues:
- it converts the u8[] command line back to a UTF-16 string, which is
  pointless since we only handle initrd or dtb arguments provided via
  the loaded image protocol anyway, which is where we got the command
  line from in the first place when booting via the PE entry point,
- in the far majority of cases, only a single initrd= option is present,
  but it optimizes for multiple options, by going over the command line
  twice, allocating heap buffers for dynamically sized arrays, etc.
- the coding style is hard to follow, with few comments, and all logic
  including string parsing etc all conbined in a single routine.

Let's fix this by rewriting most of it, based on the idea that in the
case of multiple initrds, we can just allocate a new, bigger buffer
and copy over the data before freeing the old one.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/arm-stub.c |  12 +-
 drivers/firmware/efi/libstub/efistub.h  |  17 +-
 drivers/firmware/efi/libstub/file.c     | 356 +++++++++-----------
 drivers/firmware/efi/libstub/x86-stub.c |  12 +-
 4 files changed, 169 insertions(+), 228 deletions(-)

diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index fb5b2b35d3be..9b12f9b19533 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -148,7 +148,7 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table_arg,
 	unsigned long dram_base;
 	/* addr/point and size pairs for memory management*/
 	unsigned long initrd_addr;
-	u64 initrd_size = 0;
+	unsigned long initrd_size = 0;
 	unsigned long fdt_addr = 0;  /* Original DTB */
 	unsigned long fdt_size = 0;
 	char *cmdline_ptr = NULL;
@@ -238,8 +238,7 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table_arg,
 		if (strstr(cmdline_ptr, "dtb="))
 			pr_efi("Ignoring DTB from command line.\n");
 	} else {
-		status = handle_cmdline_files(image, cmdline_ptr, "dtb=",
-					      ~0UL, &fdt_addr, &fdt_size);
+		status = efi_load_dtb(image, &fdt_addr, &fdt_size);
 
 		if (status != EFI_SUCCESS) {
 			pr_efi_err("Failed to load device tree!\n");
@@ -259,11 +258,8 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table_arg,
 	if (!fdt_addr)
 		pr_efi("Generating empty DTB\n");
 
-	status = handle_cmdline_files(image, cmdline_ptr, "initrd=",
-				      efi_get_max_initrd_addr(dram_base,
-							      *image_addr),
-				      (unsigned long *)&initrd_addr,
-				      (unsigned long *)&initrd_size);
+	status = efi_load_initrd(image, &initrd_addr, &initrd_size,
+				 efi_get_max_initrd_addr(dram_base, *image_addr));
 	if (status != EFI_SUCCESS)
 		pr_efi_err("Failed initrd from command line!\n");
 
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index e057d509d5d8..60d929469b8b 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -327,7 +327,7 @@ typedef struct {
 	efi_time_t		last_access_time;
 	efi_time_t		modification_time;
 	__aligned_u64		attribute;
-	efi_char16_t		filename[1];
+	efi_char16_t		filename[];
 } efi_file_info_t;
 
 typedef struct efi_file_protocol efi_file_protocol_t;
@@ -607,15 +607,18 @@ efi_status_t efi_relocate_kernel(unsigned long *image_addr,
 				 unsigned long alignment,
 				 unsigned long min_addr);
 
-efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
-				  char *cmd_line, char *option_string,
-				  unsigned long max_addr,
-				  unsigned long *load_addr,
-				  unsigned long *load_size);
-
 efi_status_t efi_parse_options(char const *cmdline);
 
 efi_status_t efi_setup_gop(struct screen_info *si, efi_guid_t *proto,
 			   unsigned long size);
 
+efi_status_t efi_load_dtb(efi_loaded_image_t *image,
+			  unsigned long *load_addr,
+			  unsigned long *load_size);
+
+efi_status_t efi_load_initrd(efi_loaded_image_t *image,
+			     unsigned long *load_addr,
+			     unsigned long *load_size,
+			     unsigned long max_addr);
+
 #endif
diff --git a/drivers/firmware/efi/libstub/file.c b/drivers/firmware/efi/libstub/file.c
index e0302f340962..0d67432ed067 100644
--- a/drivers/firmware/efi/libstub/file.c
+++ b/drivers/firmware/efi/libstub/file.c
@@ -12,6 +12,8 @@
 
 #include "efistub.h"
 
+#define MAX_FILENAME_SIZE	256
+
 /*
  * Some firmware implementations have problems reading files in one go.
  * A read chunk size of 1MB seems to work for most platforms.
@@ -27,277 +29,221 @@
  */
 #define EFI_READ_CHUNK_SIZE	SZ_1M
 
-struct file_info {
-	efi_file_protocol_t *handle;
-	u64 size;
-};
-
-static efi_status_t efi_file_size(void *__fh, efi_char16_t *filename_16,
-				  void **handle, u64 *file_sz)
+static efi_status_t efi_open_file(efi_file_protocol_t *volume,
+				  efi_char16_t *filename_16,
+				  efi_file_protocol_t **handle,
+				  unsigned long *file_size)
 {
-	efi_file_protocol_t *h, *fh = __fh;
-	efi_file_info_t *info;
-	efi_status_t status;
+	struct {
+		efi_file_info_t info;
+		efi_char16_t	filename[MAX_FILENAME_SIZE];
+	} finfo;
 	efi_guid_t info_guid = EFI_FILE_INFO_ID;
+	efi_file_protocol_t *fh;
 	unsigned long info_sz;
+	efi_status_t status;
 
-	status = fh->open(fh, &h, filename_16, EFI_FILE_MODE_READ, 0);
+	status = volume->open(volume, &fh, filename_16, EFI_FILE_MODE_READ, 0);
 	if (status != EFI_SUCCESS) {
-		efi_printk("Failed to open file: ");
+		pr_efi_err("Failed to open file: ");
 		efi_char16_printk(filename_16);
 		efi_printk("\n");
 		return status;
 	}
 
-	*handle = h;
-
-	info_sz = 0;
-	status = h->get_info(h, &info_guid, &info_sz, NULL);
-	if (status != EFI_BUFFER_TOO_SMALL) {
-		efi_printk("Failed to get file info size\n");
-		return status;
-	}
-
-grow:
-	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, info_sz,
-			     (void **)&info);
+	info_sz = sizeof(finfo);
+	status = fh->get_info(fh, &info_guid, &info_sz, &finfo);
 	if (status != EFI_SUCCESS) {
-		efi_printk("Failed to alloc mem for file info\n");
+		pr_efi_err("Failed to get file info\n");
+		fh->close(fh);
 		return status;
 	}
 
-	status = h->get_info(h, &info_guid, &info_sz, info);
-	if (status == EFI_BUFFER_TOO_SMALL) {
-		efi_bs_call(free_pool, info);
-		goto grow;
-	}
-
-	*file_sz = info->file_size;
-	efi_bs_call(free_pool, info);
-
-	if (status != EFI_SUCCESS)
-		efi_printk("Failed to get initrd info\n");
-
-	return status;
-}
-
-static efi_status_t efi_file_read(efi_file_protocol_t *handle,
-				  unsigned long *size, void *addr)
-{
-	return handle->read(handle, size, addr);
-}
-
-static efi_status_t efi_file_close(efi_file_protocol_t *handle)
-{
-	return handle->close(handle);
+	*handle = fh;
+	*file_size = finfo.info.file_size;
+	return EFI_SUCCESS;
 }
 
 static efi_status_t efi_open_volume(efi_loaded_image_t *image,
-				    efi_file_protocol_t **__fh)
+				    efi_file_protocol_t **fh)
 {
-	efi_simple_file_system_protocol_t *io;
-	efi_file_protocol_t *fh;
 	efi_guid_t fs_proto = EFI_FILE_SYSTEM_GUID;
+	efi_simple_file_system_protocol_t *io;
 	efi_status_t status;
-	efi_handle_t handle = image->device_handle;
 
-	status = efi_bs_call(handle_protocol, handle, &fs_proto, (void **)&io);
+	status = efi_bs_call(handle_protocol, image->device_handle, &fs_proto,
+			     (void **)&io);
 	if (status != EFI_SUCCESS) {
-		efi_printk("Failed to handle fs_proto\n");
+		pr_efi_err("Failed to handle fs_proto\n");
 		return status;
 	}
 
-	status = io->open_volume(io, &fh);
+	status = io->open_volume(io, fh);
 	if (status != EFI_SUCCESS)
-		efi_printk("Failed to open volume\n");
-	else
-		*__fh = fh;
+		pr_efi_err("Failed to open volume\n");
 
 	return status;
 }
 
+static int find_file_option(const efi_char16_t *cmdline, int cmdline_len,
+			    const efi_char16_t *prefix, int prefix_size,
+			    efi_char16_t *result, int result_len)
+{
+	int prefix_len = prefix_size / 2;
+	bool found = false;
+	int i;
+
+	for (i = prefix_len; i < cmdline_len; i++) {
+		if (!memcmp(&cmdline[i - prefix_len], prefix, prefix_size)) {
+			found = true;
+			break;
+		}
+	}
+
+	if (!found)
+		return 0;
+
+	while (--result_len > 0 && i < cmdline_len) {
+		if (cmdline[i] == L'\0' ||
+		    cmdline[i] == L'\n' ||
+		    cmdline[i] == L' ')
+			break;
+		*result++ = cmdline[i++];
+	}
+	*result = L'\0';
+	return i;
+}
+
 /*
  * Check the cmdline for a LILO-style file= arguments.
  *
  * We only support loading a file from the same filesystem as
  * the kernel image.
  */
-efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
-				  char *cmd_line, char *option_string,
-				  unsigned long max_addr,
-				  unsigned long *load_addr,
-				  unsigned long *load_size)
+static efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
+					 const efi_char16_t *optstr,
+					 int optstr_size,
+					 unsigned long max_addr,
+					 unsigned long *load_addr,
+					 unsigned long *load_size)
 {
+	const efi_char16_t *cmdline = image->load_options;
+	int cmdline_len = image->load_options_size / 2;
 	unsigned long efi_chunk_size = ULONG_MAX;
-	struct file_info *files;
-	unsigned long file_addr;
-	u64 file_size_total;
-	efi_file_protocol_t *fh = NULL;
+	efi_file_protocol_t *volume = NULL;
+	efi_file_protocol_t *file;
+	unsigned long alloc_addr;
+	unsigned long alloc_size;
 	efi_status_t status;
-	int nr_files;
-	char *str;
-	int i, j, k;
-
-	if (IS_ENABLED(CONFIG_X86) && !nochunk())
-		efi_chunk_size = EFI_READ_CHUNK_SIZE;
-
-	file_addr = 0;
-	file_size_total = 0;
-
-	str = cmd_line;
-
-	j = 0;			/* See close_handles */
+	int offset;
 
 	if (!load_addr || !load_size)
 		return EFI_INVALID_PARAMETER;
 
-	*load_addr = 0;
-	*load_size = 0;
-
-	if (!str || !*str)
-		return EFI_SUCCESS;
-
-	for (nr_files = 0; *str; nr_files++) {
-		str = strstr(str, option_string);
-		if (!str)
-			break;
-
-		str += strlen(option_string);
-
-		/* Skip any leading slashes */
-		while (*str == '/' || *str == '\\')
-			str++;
-
-		while (*str && *str != ' ' && *str != '\n')
-			str++;
-	}
-
-	if (!nr_files)
-		return EFI_SUCCESS;
+	if (IS_ENABLED(CONFIG_X86) && !nochunk())
+		efi_chunk_size = EFI_READ_CHUNK_SIZE;
 
-	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA,
-			     nr_files * sizeof(*files), (void **)&files);
-	if (status != EFI_SUCCESS) {
-		pr_efi_err("Failed to alloc mem for file handle list\n");
-		goto fail;
-	}
+	alloc_addr = alloc_size = 0;
+	do {
+		efi_char16_t filename[MAX_FILENAME_SIZE];
+		unsigned long size;
+		void *addr;
 
-	str = cmd_line;
-	for (i = 0; i < nr_files; i++) {
-		struct file_info *file;
-		efi_char16_t filename_16[256];
-		efi_char16_t *p;
+		offset = find_file_option(cmdline, cmdline_len,
+					  optstr, optstr_size,
+					  filename, ARRAY_SIZE(filename));
 
-		str = strstr(str, option_string);
-		if (!str)
+		if (!offset)
 			break;
 
-		str += strlen(option_string);
-
-		file = &files[i];
-		p = filename_16;
-
-		/* Skip any leading slashes */
-		while (*str == '/' || *str == '\\')
-			str++;
-
-		while (*str && *str != ' ' && *str != '\n') {
-			if ((u8 *)p >= (u8 *)filename_16 + sizeof(filename_16))
-				break;
-
-			if (*str == '/') {
-				*p++ = '\\';
-				str++;
-			} else {
-				*p++ = *str++;
-			}
-		}
-
-		*p = '\0';
+		cmdline += offset;
+		cmdline_len -= offset;
 
-		/* Only open the volume once. */
-		if (!i) {
-			status = efi_open_volume(image, &fh);
+		if (!volume) {
+			status = efi_open_volume(image, &volume);
 			if (status != EFI_SUCCESS)
-				goto free_files;
+				return status;
 		}
 
-		status = efi_file_size(fh, filename_16, (void **)&file->handle,
-				       &file->size);
+		status = efi_open_file(volume, filename, &file, &size);
 		if (status != EFI_SUCCESS)
-			goto close_handles;
-
-		file_size_total += file->size;
-	}
-
-	if (file_size_total) {
-		unsigned long addr;
+			goto err_close_volume;
 
 		/*
-		 * Multiple files need to be at consecutive addresses in memory,
-		 * so allocate enough memory for all the files.  This is used
-		 * for loading multiple files.
+		 * Check whether the existing allocation can contain the next
+		 * file. This condition will also trigger naturally during the
+		 * first (and typically only) iteration of the loop, given that
+		 * alloc_size == 0 in that case.
 		 */
-		status = efi_allocate_pages(file_size_total, &file_addr, max_addr);
-		if (status != EFI_SUCCESS) {
-			pr_efi_err("Failed to alloc highmem for files\n");
-			goto close_handles;
-		}
+		if (round_up(alloc_size + size, EFI_ALLOC_ALIGN) >
+		    round_up(alloc_size, EFI_ALLOC_ALIGN)) {
+			unsigned long old_addr = alloc_addr;
+
+			status = efi_allocate_pages(alloc_size + size, &alloc_addr,
+						    max_addr);
+			if (status != EFI_SUCCESS) {
+				pr_efi_err("Failed to reallocate memory for files\n");
+				goto err_close_file;
+			}
 
-		/* We've run out of free low memory. */
-		if (file_addr > max_addr) {
-			pr_efi_err("We've run out of free low memory\n");
-			status = EFI_INVALID_PARAMETER;
-			goto free_file_total;
+			if (old_addr != 0) {
+				/*
+				 * This is not the first time we've gone
+				 * around this loop, and so we are loading
+				 * multiple files that need to be concatenated
+				 * and returned in a single buffer.
+				 */
+				memcpy((void *)alloc_addr, (void *)old_addr, alloc_size);
+				efi_free(alloc_size, old_addr);
+			}
 		}
 
-		addr = file_addr;
-		for (j = 0; j < nr_files; j++) {
-			unsigned long size;
-
-			size = files[j].size;
-			while (size) {
-				unsigned long chunksize;
-
-				if (size > efi_chunk_size)
-					chunksize = efi_chunk_size;
-				else
-					chunksize = size;
-
-				status = efi_file_read(files[j].handle,
-						       &chunksize,
-						       (void *)addr);
-				if (status != EFI_SUCCESS) {
-					pr_efi_err("Failed to read file\n");
-					goto free_file_total;
-				}
-				addr += chunksize;
-				size -= chunksize;
-			}
+		addr = (void *)alloc_addr + alloc_size;
+		alloc_size += size;
 
-			efi_file_close(files[j].handle);
+		while (size) {
+			unsigned long chunksize = min(size, efi_chunk_size);
+
+			status = file->read(file, &chunksize, addr);
+			if (status != EFI_SUCCESS) {
+				pr_efi_err("Failed to read file\n");
+				goto err_close_file;
+			}
+			addr += chunksize;
+			size -= chunksize;
 		}
+		file->close(file);
+	} while (offset > 0);
 
-	}
+	*load_addr = alloc_addr;
+	*load_size = alloc_size;
 
-	efi_bs_call(free_pool, files);
+	if (volume)
+		volume->close(volume);
+	return EFI_SUCCESS;
 
-	*load_addr = file_addr;
-	*load_size = file_size_total;
+err_close_file:
+	file->close(file);
 
+err_close_volume:
+	volume->close(volume);
+	efi_free(alloc_size, alloc_addr);
 	return status;
+}
 
-free_file_total:
-	efi_free(file_size_total, file_addr);
-
-close_handles:
-	for (k = j; k < i; k++)
-		efi_file_close(files[k].handle);
-free_files:
-	efi_bs_call(free_pool, files);
-fail:
-	*load_addr = 0;
-	*load_size = 0;
+efi_status_t efi_load_dtb(efi_loaded_image_t *image,
+			  unsigned long *load_addr,
+			  unsigned long *load_size)
+{
+	return handle_cmdline_files(image, L"dtb=", sizeof(L"dtb=") - 2,
+				    ULONG_MAX, load_addr, load_size);
+}
 
-	return status;
+efi_status_t efi_load_initrd(efi_loaded_image_t *image,
+			     unsigned long *load_addr,
+			     unsigned long *load_size,
+			     unsigned long max_addr)
+{
+	return handle_cmdline_files(image, L"initrd=", sizeof(L"initrd=") - 2,
+				    max_addr, load_addr, load_size);
 }
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index f2dbf119837c..39d04735f1c5 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -421,18 +421,14 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	if (status != EFI_SUCCESS)
 		goto fail2;
 
-	status = handle_cmdline_files(image,
-				      (char *)(unsigned long)hdr->cmd_line_ptr,
-				      "initrd=", hdr->initrd_addr_max,
-				      &ramdisk_addr, &ramdisk_size);
+	status = efi_load_initrd(image, &ramdisk_addr, &ramdisk_size,
+				 hdr->initrd_addr_max);
 
 	if (status != EFI_SUCCESS &&
 	    hdr->xloadflags & XLF_CAN_BE_LOADED_ABOVE_4G) {
 		efi_printk("Trying to load files to higher address\n");
-		status = handle_cmdline_files(image,
-				      (char *)(unsigned long)hdr->cmd_line_ptr,
-				      "initrd=", -1UL,
-				      &ramdisk_addr, &ramdisk_size);
+		status = efi_load_initrd(image, &ramdisk_addr, &ramdisk_size,
+					 ULONG_MAX);
 	}
 
 	if (status != EFI_SUCCESS)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 16/19] efi/libstub: Take soft and hard memory limits into account for initrd loading
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
                   ` (14 preceding siblings ...)
  2020-02-10 16:02 ` [PATCH 15/19] efi/libstub: Rewrite file I/O routine Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 17/19] efi/libstub: Clean up command line parsing routine Ard Biesheuvel
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

On x86, the preferred load address of the initrd is still below 4 GB,
even though in some cases, we can cope with an initrd that is loaded
above that.

To simplify the code, and to make it more straightforward to introduce
other ways to load the initrd, pass the soft and hard memory limits at
the same time, and let the code handling the initrd= command line option
deal with this.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/arm-stub.c |  2 +-
 drivers/firmware/efi/libstub/efistub.h  |  3 ++-
 drivers/firmware/efi/libstub/file.c     | 21 ++++++++++++++------
 drivers/firmware/efi/libstub/x86-stub.c | 11 ++--------
 4 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index 9b12f9b19533..2edc673ea06c 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -258,7 +258,7 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table_arg,
 	if (!fdt_addr)
 		pr_efi("Generating empty DTB\n");
 
-	status = efi_load_initrd(image, &initrd_addr, &initrd_size,
+	status = efi_load_initrd(image, &initrd_addr, &initrd_size, ULONG_MAX,
 				 efi_get_max_initrd_addr(dram_base, *image_addr));
 	if (status != EFI_SUCCESS)
 		pr_efi_err("Failed initrd from command line!\n");
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 60d929469b8b..d282d907cd33 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -619,6 +619,7 @@ efi_status_t efi_load_dtb(efi_loaded_image_t *image,
 efi_status_t efi_load_initrd(efi_loaded_image_t *image,
 			     unsigned long *load_addr,
 			     unsigned long *load_size,
-			     unsigned long max_addr);
+			     unsigned long soft_limit,
+			     unsigned long hard_limit);
 
 #endif
diff --git a/drivers/firmware/efi/libstub/file.c b/drivers/firmware/efi/libstub/file.c
index 0d67432ed067..be78f64f8d80 100644
--- a/drivers/firmware/efi/libstub/file.c
+++ b/drivers/firmware/efi/libstub/file.c
@@ -123,7 +123,8 @@ static int find_file_option(const efi_char16_t *cmdline, int cmdline_len,
 static efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
 					 const efi_char16_t *optstr,
 					 int optstr_size,
-					 unsigned long max_addr,
+					 unsigned long soft_limit,
+					 unsigned long hard_limit,
 					 unsigned long *load_addr,
 					 unsigned long *load_size)
 {
@@ -179,8 +180,15 @@ static efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
 		    round_up(alloc_size, EFI_ALLOC_ALIGN)) {
 			unsigned long old_addr = alloc_addr;
 
-			status = efi_allocate_pages(alloc_size + size, &alloc_addr,
-						    max_addr);
+			status = EFI_OUT_OF_RESOURCES;
+			if (soft_limit < hard_limit)
+				status = efi_allocate_pages(alloc_size + size,
+							    &alloc_addr,
+							    soft_limit);
+			if (status == EFI_OUT_OF_RESOURCES)
+				status = efi_allocate_pages(alloc_size + size,
+							    &alloc_addr,
+							    hard_limit);
 			if (status != EFI_SUCCESS) {
 				pr_efi_err("Failed to reallocate memory for files\n");
 				goto err_close_file;
@@ -236,14 +244,15 @@ efi_status_t efi_load_dtb(efi_loaded_image_t *image,
 			  unsigned long *load_size)
 {
 	return handle_cmdline_files(image, L"dtb=", sizeof(L"dtb=") - 2,
-				    ULONG_MAX, load_addr, load_size);
+				    ULONG_MAX, ULONG_MAX, load_addr, load_size);
 }
 
 efi_status_t efi_load_initrd(efi_loaded_image_t *image,
 			     unsigned long *load_addr,
 			     unsigned long *load_size,
-			     unsigned long max_addr)
+			     unsigned long soft_limit,
+			     unsigned long hard_limit)
 {
 	return handle_cmdline_files(image, L"initrd=", sizeof(L"initrd=") - 2,
-				    max_addr, load_addr, load_size);
+				    soft_limit, hard_limit, load_addr, load_size);
 }
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 39d04735f1c5..52a1a2df2071 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -422,15 +422,8 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 		goto fail2;
 
 	status = efi_load_initrd(image, &ramdisk_addr, &ramdisk_size,
-				 hdr->initrd_addr_max);
-
-	if (status != EFI_SUCCESS &&
-	    hdr->xloadflags & XLF_CAN_BE_LOADED_ABOVE_4G) {
-		efi_printk("Trying to load files to higher address\n");
-		status = efi_load_initrd(image, &ramdisk_addr, &ramdisk_size,
-					 ULONG_MAX);
-	}
-
+				 hdr->initrd_addr_max,
+				 above4g ? ULONG_MAX : hdr->initrd_addr_max);
 	if (status != EFI_SUCCESS)
 		goto fail2;
 	hdr->ramdisk_image = ramdisk_addr & 0xffffffff;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 17/19] efi/libstub: Clean up command line parsing routine
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
                   ` (15 preceding siblings ...)
  2020-02-10 16:02 ` [PATCH 16/19] efi/libstub: Take soft and hard memory limits into account for initrd loading Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 18/19] efi/libstub: Expose LocateDevicePath boot service Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 19/19] efi/libstub: Make the LoadFile EFI protocol accessible Ard Biesheuvel
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

We currently parse the command non-destructively, to avoid having to
allocate memory for a copy before passing it to the standard parsing
routines that are used by the core kernel, and which modify the input
to delineate the parsed tokens with NUL characters.

Instead, we call strstr() and strncmp() to go over the input multiple
times, and match prefixes rather than tokens, which implies that we
would match, e.g., 'nokaslrfoo' in the stub and disable KASLR, while
the kernel would disregard the option and run with KASLR enabled.

In order to avoid having to reason about whether and how this behavior
may be abused, let's clean up the parsing routines, and rebuild them
on top of the existing helpers.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/Makefile          |  3 +-
 drivers/firmware/efi/libstub/efi-stub-helper.c | 79 +++++++-------------
 drivers/firmware/efi/libstub/string.c          | 63 ++++++++++++++++
 3 files changed, 91 insertions(+), 54 deletions(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 74a097ef1780..82cb9f9c08ce 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -39,7 +39,8 @@ OBJECT_FILES_NON_STANDARD	:= y
 KCOV_INSTRUMENT			:= n
 
 lib-y				:= efi-stub-helper.o gop.o secureboot.o tpm.o \
-				   file.o mem.o random.o randomalloc.o pci.o
+				   file.o mem.o random.o randomalloc.o pci.o \
+				   lib-cmdline.o lib-ctype.o
 
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index db23be5dc69b..b3621ceb3480 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -68,66 +68,39 @@ void efi_printk(char *str)
  */
 efi_status_t efi_parse_options(char const *cmdline)
 {
-	char *str;
-
-	str = strstr(cmdline, "nokaslr");
-	if (str == cmdline || (str && str > cmdline && *(str - 1) == ' '))
-		efi_nokaslr = true;
-
-	str = strstr(cmdline, "quiet");
-	if (str == cmdline || (str && str > cmdline && *(str - 1) == ' '))
-		efi_quiet = true;
-
-	/*
-	 * If no EFI parameters were specified on the cmdline we've got
-	 * nothing to do.
-	 */
-	str = strstr(cmdline, "efi=");
-	if (!str)
-		return EFI_SUCCESS;
-
-	/* Skip ahead to first argument */
-	str += strlen("efi=");
-
-	/*
-	 * Remember, because efi= is also used by the kernel we need to
-	 * skip over arguments we don't understand.
-	 */
-	while (*str && *str != ' ') {
-		if (!strncmp(str, "nochunk", 7)) {
-			str += strlen("nochunk");
-			efi_nochunk = true;
-		}
+	size_t len = strlen(cmdline);
+	efi_status_t status;
+	char *str, *buf;
 
-		if (!strncmp(str, "novamap", 7)) {
-			str += strlen("novamap");
-			efi_novamap = true;
-		}
+	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, len, (void **)&buf);
+	if (status != EFI_SUCCESS)
+		return status;
 
-		if (IS_ENABLED(CONFIG_EFI_SOFT_RESERVE) &&
-		    !strncmp(str, "nosoftreserve", 7)) {
-			str += strlen("nosoftreserve");
-			efi_nosoftreserve = true;
-		}
+	str = skip_spaces(memcpy(buf, cmdline, len + 1));
 
-		if (!strncmp(str, "disable_early_pci_dma", 21)) {
-			str += strlen("disable_early_pci_dma");
-			efi_disable_pci_dma = true;
-		}
+	while (*str) {
+		char *param, *val;
 
-		if (!strncmp(str, "no_disable_early_pci_dma", 24)) {
-			str += strlen("no_disable_early_pci_dma");
-			efi_disable_pci_dma = false;
-		}
+		str = next_arg(str, &param, &val);
 
-		/* Group words together, delimited by "," */
-		while (*str && *str != ' ' && *str != ',')
-			str++;
+		if (!strcmp(param, "nokaslr")) {
+			efi_nokaslr = true;
+		} else if (!strcmp(param, "quiet")) {
+			efi_quiet = true;
+		} else if (!strcmp(param, "efi") && val) {
+			efi_nochunk = parse_option_str(val, "nochunk");
+			efi_novamap = parse_option_str(val, "novamap");
 
-		if (*str == ',')
-			str++;
-	}
+			efi_nosoftreserve = IS_ENABLED(CONFIG_EFI_SOFT_RESERVE) &&
+					    parse_option_str(val, "nosoftreserve");
 
+			if (parse_option_str(val, "disable_early_pci_dma"))
+				efi_disable_pci_dma = true;
+			if (parse_option_str(val, "no_disable_early_pci_dma"))
+				efi_disable_pci_dma = false;
+		}
+	}
+	efi_bs_call(free_pool, buf);
 	return EFI_SUCCESS;
 }
 
diff --git a/drivers/firmware/efi/libstub/string.c b/drivers/firmware/efi/libstub/string.c
index ed10e3f602c5..33dfb83be239 100644
--- a/drivers/firmware/efi/libstub/string.c
+++ b/drivers/firmware/efi/libstub/string.c
@@ -6,6 +6,7 @@
  *  Copyright (C) 1991, 1992  Linus Torvalds
  */
 
+#include <linux/ctype.h>
 #include <linux/types.h>
 #include <linux/string.h>
 
@@ -56,3 +57,65 @@ int strncmp(const char *cs, const char *ct, size_t count)
 	return 0;
 }
 #endif
+
+char *skip_spaces(const char *str)
+{
+	while (isspace(*str))
+		++str;
+	return (char *)str;
+}
+
+/* Works only for digits and letters, but small and fast */
+#define TOLOWER(x) ((x) | 0x20)
+
+static unsigned int simple_guess_base(const char *cp)
+{
+	if (cp[0] == '0') {
+		if (TOLOWER(cp[1]) == 'x' && isxdigit(cp[2]))
+			return 16;
+		else
+			return 8;
+	} else {
+		return 10;
+	}
+}
+
+/**
+ * simple_strtoull - convert a string to an unsigned long long
+ * @cp: The start of the string
+ * @endp: A pointer to the end of the parsed string will be placed here
+ * @base: The number base to use
+ */
+
+unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int base)
+{
+	unsigned long long result = 0;
+
+	if (!base)
+		base = simple_guess_base(cp);
+
+	if (base == 16 && cp[0] == '0' && TOLOWER(cp[1]) == 'x')
+		cp += 2;
+
+	while (isxdigit(*cp)) {
+		unsigned int value;
+
+		value = isdigit(*cp) ? *cp - '0' : TOLOWER(*cp) - 'a' + 10;
+		if (value >= base)
+			break;
+		result = result * base + value;
+		cp++;
+	}
+	if (endp)
+		*endp = (char *)cp;
+
+	return result;
+}
+
+long simple_strtol(const char *cp, char **endp, unsigned int base)
+{
+	if (*cp == '-')
+		return -simple_strtoull(cp + 1, endp, base);
+
+	return simple_strtoull(cp, endp, base);
+}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 18/19] efi/libstub: Expose LocateDevicePath boot service
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
                   ` (16 preceding siblings ...)
  2020-02-10 16:02 ` [PATCH 17/19] efi/libstub: Clean up command line parsing routine Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  2020-02-10 16:02 ` [PATCH 19/19] efi/libstub: Make the LoadFile EFI protocol accessible Ard Biesheuvel
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

We will be adding support for loading the initrd from a GUIDed
device path in a subsequent patch, so update the prototype of
the LocateDevicePath() boot service to make it callable from
our code.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/include/asm/efi.h             | 3 +++
 drivers/firmware/efi/libstub/efistub.h | 6 +++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 85f79accd3f8..9ced980b123b 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -283,6 +283,9 @@ static inline void *efi64_zero_upper(void *p)
 #define __efi64_argmap_locate_protocol(protocol, reg, interface)	\
 	((protocol), (reg), efi64_zero_upper(interface))
 
+#define __efi64_argmap_locate_device_path(protocol, path, handle)	\
+	((protocol), (path), efi64_zero_upper(handle))
+
 /* PCI I/O */
 #define __efi64_argmap_get_location(protocol, seg, bus, dev, func)	\
 	((protocol), efi64_zero_upper(seg), efi64_zero_upper(bus),	\
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index d282d907cd33..afa774a312f5 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -100,6 +100,8 @@ struct efi_boot_memmap {
 	unsigned long		*buff_size;
 };
 
+typedef struct efi_generic_dev_path efi_device_path_protocol_t;
+
 /*
  * EFI Boot Services table
  */
@@ -134,7 +136,9 @@ union efi_boot_services {
 		efi_status_t (__efiapi *locate_handle)(int, efi_guid_t *,
 						       void *, unsigned long *,
 						       efi_handle_t *);
-		void *locate_device_path;
+		efi_status_t (__efiapi *locate_device_path)(efi_guid_t *,
+							    efi_device_path_protocol_t **,
+							    efi_handle_t *);
 		efi_status_t (__efiapi *install_configuration_table)(efi_guid_t *,
 								     void *);
 		void *load_image;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 19/19] efi/libstub: Make the LoadFile EFI protocol accessible
  2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
                   ` (17 preceding siblings ...)
  2020-02-10 16:02 ` [PATCH 18/19] efi/libstub: Expose LocateDevicePath boot service Ard Biesheuvel
@ 2020-02-10 16:02 ` Ard Biesheuvel
  18 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-10 16:02 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, nivedita, mingo, lukas, atish.patra

Add the protocol definitions, GUIDs and mixed mode glue so that
the EFI loadfile protocol can be used from the stub. This will
be used in a future patch to load the initrd.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/include/asm/efi.h             |  4 ++++
 drivers/firmware/efi/libstub/efistub.h | 14 ++++++++++++++
 include/linux/efi.h                    |  2 ++
 3 files changed, 20 insertions(+)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 9ced980b123b..fcb21e3d13c5 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -291,6 +291,10 @@ static inline void *efi64_zero_upper(void *p)
 	((protocol), efi64_zero_upper(seg), efi64_zero_upper(bus),	\
 	 efi64_zero_upper(dev), efi64_zero_upper(func))
 
+/* LoadFile */
+#define __efi64_argmap_load_file(protocol, path, policy, bufsize, buf)	\
+	((protocol), (path), (policy), efi64_zero_upper(bufsize), (buf))
+
 /*
  * The macros below handle the plumbing for the argument mapping. To add a
  * mapping for a specific EFI method, simply define a macro
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index afa774a312f5..34fe3fad042f 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -541,6 +541,20 @@ union efi_tcg2_protocol {
 	} mixed_mode;
 };
 
+typedef union efi_load_file_protocol efi_load_file_protocol_t;
+typedef union efi_load_file_protocol efi_load_file2_protocol_t;
+
+union efi_load_file_protocol {
+	struct {
+		efi_status_t (__efiapi *load_file)(efi_load_file_protocol_t *,
+						   efi_device_path_protocol_t *,
+						   bool, unsigned long *, void *);
+	};
+	struct {
+		u32 load_file;
+	} mixed_mode;
+};
+
 void efi_pci_disable_bridge_busmaster(void);
 
 typedef efi_status_t (*efi_exit_boot_map_processing)(
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 0e047d2738cd..9ccf313fe9de 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -332,6 +332,8 @@ void efi_native_runtime_setup(void);
 #define EFI_CONSOLE_OUT_DEVICE_GUID		EFI_GUID(0xd3b36f2c, 0xd551, 0x11d4,  0x9a, 0x46, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 #define APPLE_PROPERTIES_PROTOCOL_GUID		EFI_GUID(0x91bd12fe, 0xf6c3, 0x44fb,  0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0)
 #define EFI_TCG2_PROTOCOL_GUID			EFI_GUID(0x607f766c, 0x7455, 0x42be,  0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f)
+#define EFI_LOAD_FILE_PROTOCOL_GUID		EFI_GUID(0x56ec3091, 0x954c, 0x11d2,  0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
+#define EFI_LOAD_FILE2_PROTOCOL_GUID		EFI_GUID(0x4006c0c1, 0xfcb3, 0x403e,  0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d)
 
 #define EFI_IMAGE_SECURITY_DATABASE_GUID	EFI_GUID(0xd719b2cb, 0x3d3a, 0x4596,  0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f)
 #define EFI_SHIM_LOCK_GUID			EFI_GUID(0x605dab50, 0xe046, 0x4300,  0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH 13/19] efi/libstub: Move get_dram_base() into arm-stub.c
  2020-02-10 16:02 ` [PATCH 13/19] efi/libstub: Move get_dram_base() into arm-stub.c Ard Biesheuvel
@ 2020-02-17  1:17   ` Atish Patra
  2020-02-17  8:37     ` Ard Biesheuvel
  0 siblings, 1 reply; 29+ messages in thread
From: Atish Patra @ 2020-02-17  1:17 UTC (permalink / raw)
  To: linux-efi, ardb; +Cc: mingo, nivedita, lukas

On Mon, 2020-02-10 at 17:02 +0100, Ard Biesheuvel wrote:
> get_dram_base() is only called from arm-stub.c so move it into
> the same source file as its caller.
> 

Just FYI: riscv efi stub port is also going to use get_dram_base().
However, I have renamed arm-stub.c to generic efi-stub.c so that arm,
arm64 and riscv can reuse it. Thus, Moving get_dram_base() into arm-
stub.c works for riscv as well. I will rebase my patches on top of this
series.


> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/firmware/efi/libstub/arm-stub.c        | 33
> ++++++++++++++++++
>  drivers/firmware/efi/libstub/efi-stub-helper.c | 35 ----------------
> ----
>  drivers/firmware/efi/libstub/efistub.h         |  2 --
>  3 files changed, 33 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c
> b/drivers/firmware/efi/libstub/arm-stub.c
> index ebdf1964dd5c..fb5b2b35d3be 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -87,6 +87,39 @@ void install_memreserve_table(void)
>  		pr_efi_err("Failed to install memreserve config
> table!\n");
>  }
>  
> +static unsigned long get_dram_base(void)
> +{
> +	efi_status_t status;
> +	unsigned long map_size, buff_size;
> +	unsigned long membase  = EFI_ERROR;
> +	struct efi_memory_map map;
> +	efi_memory_desc_t *md;
> +	struct efi_boot_memmap boot_map;
> +
> +	boot_map.map		= (efi_memory_desc_t **)&map.map;
> +	boot_map.map_size	= &map_size;
> +	boot_map.desc_size	= &map.desc_size;
> +	boot_map.desc_ver	= NULL;
> +	boot_map.key_ptr	= NULL;
> +	boot_map.buff_size	= &buff_size;
> +
> +	status = efi_get_memory_map(&boot_map);
> +	if (status != EFI_SUCCESS)
> +		return membase;
> +
> +	map.map_end = map.map + map_size;
> +
> +	for_each_efi_memory_desc_in_map(&map, md) {
> +		if (md->attribute & EFI_MEMORY_WB) {
> +			if (membase > md->phys_addr)
> +				membase = md->phys_addr;
> +		}
> +	}
> +
> +	efi_bs_call(free_pool, map.map);
> +
> +	return membase;
> +}
>  
>  /*
>   * This function handles the architcture specific differences
> between arm and
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c
> b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index 92ccd0a51ae6..1a8f2cf5a2bd 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -75,41 +75,6 @@ void efi_printk(char *str)
>  	}
>  }
>  
> -
> -unsigned long get_dram_base(void)
> -{
> -	efi_status_t status;
> -	unsigned long map_size, buff_size;
> -	unsigned long membase  = EFI_ERROR;
> -	struct efi_memory_map map;
> -	efi_memory_desc_t *md;
> -	struct efi_boot_memmap boot_map;
> -
> -	boot_map.map =		(efi_memory_desc_t **)&map.map;
> -	boot_map.map_size =	&map_size;
> -	boot_map.desc_size =	&map.desc_size;
> -	boot_map.desc_ver =	NULL;
> -	boot_map.key_ptr =	NULL;
> -	boot_map.buff_size =	&buff_size;
> -
> -	status = efi_get_memory_map(&boot_map);
> -	if (status != EFI_SUCCESS)
> -		return membase;
> -
> -	map.map_end = map.map + map_size;
> -
> -	for_each_efi_memory_desc_in_map(&map, md) {
> -		if (md->attribute & EFI_MEMORY_WB) {
> -			if (membase > md->phys_addr)
> -				membase = md->phys_addr;
> -		}
> -	}
> -
> -	efi_bs_call(free_pool, map.map);
> -
> -	return membase;
> -}
> -
>  static efi_status_t efi_file_size(void *__fh, efi_char16_t
> *filename_16,
>  				  void **handle, u64 *file_sz)
>  {
> diff --git a/drivers/firmware/efi/libstub/efistub.h
> b/drivers/firmware/efi/libstub/efistub.h
> index b94c63d17a4f..5123def761e9 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -549,8 +549,6 @@ efi_status_t efi_exit_boot_services(void *handle,
>  
>  void efi_char16_printk(efi_char16_t *);
>  
> -unsigned long get_dram_base(void);
> -
>  efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
>  					    unsigned long
> *new_fdt_addr,
>  					    unsigned long max_addr,

-- 
Regards,
Atish

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 13/19] efi/libstub: Move get_dram_base() into arm-stub.c
  2020-02-17  1:17   ` Atish Patra
@ 2020-02-17  8:37     ` Ard Biesheuvel
  2020-02-26 23:34       ` Atish Patra
  0 siblings, 1 reply; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-17  8:37 UTC (permalink / raw)
  To: Atish Patra; +Cc: linux-efi, mingo, nivedita, lukas

On Mon, 17 Feb 2020 at 02:17, Atish Patra <Atish.Patra@wdc.com> wrote:
>
> On Mon, 2020-02-10 at 17:02 +0100, Ard Biesheuvel wrote:
> > get_dram_base() is only called from arm-stub.c so move it into
> > the same source file as its caller.
> >
>
> Just FYI: riscv efi stub port is also going to use get_dram_base().
> However, I have renamed arm-stub.c to generic efi-stub.c so that arm,
> arm64 and riscv can reuse it. Thus, Moving get_dram_base() into arm-
> stub.c works for riscv as well. I will rebase my patches on top of this
> series.
>

Thanks Atish. I was hoping it would turn out like this, which is why I
am pushing this series now.

I haven't looked at your code yet, but please avoid using the command
line based initrd/dtb loading routines. I am proposing a cleaner way
to provide the initrd from firmware [0], and dtb loading by the stub
should not be done in the first place.

[0] https://lore.kernel.org/linux-efi/20200216141104.21477-1-ardb@kernel.org/

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 03/19] efi/libstub: Use hidden visiblity for all source files
  2020-02-10 16:02 ` [PATCH 03/19] efi/libstub: Use hidden visiblity for all source files Ard Biesheuvel
@ 2020-02-24 23:15   ` Atish Patra
  2020-02-24 23:36     ` Ard Biesheuvel
  0 siblings, 1 reply; 29+ messages in thread
From: Atish Patra @ 2020-02-24 23:15 UTC (permalink / raw)
  To: linux-efi, ardb; +Cc: mingo, nivedita, lukas

On Mon, 2020-02-10 at 17:02 +0100, Ard Biesheuvel wrote:
> Instead of setting the visibility pragma for a small set of symbol
> declarations that could result in absolute references that we cannot
> support in the stub, declare hidden visibility for all code in the
> EFI stub, which is more robust and future proof.
> 
> To ensure that the #pragma is taken into account before any other
> includes are processed, put it in a header file of its own and
> include it via the compiler command line using the -include option.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/arm64/include/asm/efi.h              | 3 ---
>  drivers/firmware/efi/libstub/Makefile     | 2 +-
>  drivers/firmware/efi/libstub/arm64-stub.c | 8 +-------
>  drivers/firmware/efi/libstub/hidden.h     | 6 ++++++
>  4 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/efi.h
> b/arch/arm64/include/asm/efi.h
> index 44531a69d32b..56ae87401a26 100644
> --- a/arch/arm64/include/asm/efi.h
> +++ b/arch/arm64/include/asm/efi.h
> @@ -107,9 +107,6 @@ static inline void free_screen_info(struct
> screen_info *si)
>  {
>  }
>  
> -/* redeclare as 'hidden' so the compiler will generate relative
> references */
> -extern struct screen_info screen_info
> __attribute__((__visibility__("hidden")));
> -
>  static inline void efifb_setup_from_dmi(struct screen_info *si,
> const char *opt)
>  {
>  }
> diff --git a/drivers/firmware/efi/libstub/Makefile
> b/drivers/firmware/efi/libstub/Makefile
> index f14b7636323a..4efdbd711e8e 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -25,7 +25,7 @@ cflags-$(CONFIG_ARM)		:= $(subst
> $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \
>  cflags-$(CONFIG_EFI_ARMSTUB)	+= -I$(srctree)/scripts/dtc/libfdt
>  
>  KBUILD_CFLAGS			:= $(cflags-y)
> -DDISABLE_BRANCH_PROFILING \
> -				   -D__NO_FORTIFY \
> +				   -include hidden.h -D__NO_FORTIFY \
>  				   $(call cc-option,-ffreestanding) \
>  				   $(call cc-option,-fno-stack-
> protector) \
>  				   -D__DISABLE_EXPORTS
> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c
> b/drivers/firmware/efi/libstub/arm64-stub.c
> index 2915b44132e6..719d03a64329 100644
> --- a/drivers/firmware/efi/libstub/arm64-stub.c
> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> @@ -6,17 +6,11 @@
>   * Adapted from ARM version by Mark Salter <msalter@redhat.com>
>   */
>  
> -/*
> - * To prevent the compiler from emitting GOT-indirected (and thus
> absolute)
> - * references to the section markers, override their visibility as
> 'hidden'
> - */
> -#pragma GCC visibility push(hidden)
> -#include <asm/sections.h>
> -#pragma GCC visibility pop
>  
>  #include <linux/efi.h>
>  #include <asm/efi.h>
>  #include <asm/memory.h>
> +#include <asm/sections.h>
>  #include <asm/sysreg.h>
>  
>  #include "efistub.h"
> diff --git a/drivers/firmware/efi/libstub/hidden.h
> b/drivers/firmware/efi/libstub/hidden.h
> new file mode 100644
> index 000000000000..3493b041f419
> --- /dev/null
> +++ b/drivers/firmware/efi/libstub/hidden.h
> @@ -0,0 +1,6 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * To prevent the compiler from emitting GOT-indirected (and thus
> absolute)
> + * references to any global symbols, override their visibility as
> 'hidden'
> + */
> +#pragma GCC visibility push(hidden)

I am getting following compiler errors because of this patch while
cross compiling for ARM, ARM64 and RISC-V.

cc1: fatal error: hidden.h: No such file or directory

Adding the following line to cflags solves the issue which indicates
that gcc is not looking at the source file directory for this
particularly include file.

-I$(srctree)/drivers/firmware/efi/libstub

I might have some trick in my build setup. Just posting here to see if
anybody else also seeing the same problem.


-- 
Regards,
Atish

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 03/19] efi/libstub: Use hidden visiblity for all source files
  2020-02-24 23:15   ` Atish Patra
@ 2020-02-24 23:36     ` Ard Biesheuvel
  2020-02-25 19:18       ` Atish Patra
  0 siblings, 1 reply; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-24 23:36 UTC (permalink / raw)
  To: Atish Patra; +Cc: linux-efi, mingo, nivedita, lukas

On Tue, 25 Feb 2020 at 00:15, Atish Patra <Atish.Patra@wdc.com> wrote:
>
> On Mon, 2020-02-10 at 17:02 +0100, Ard Biesheuvel wrote:
> > Instead of setting the visibility pragma for a small set of symbol
> > declarations that could result in absolute references that we cannot
> > support in the stub, declare hidden visibility for all code in the
> > EFI stub, which is more robust and future proof.
> >
> > To ensure that the #pragma is taken into account before any other
> > includes are processed, put it in a header file of its own and
> > include it via the compiler command line using the -include option.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  arch/arm64/include/asm/efi.h              | 3 ---
> >  drivers/firmware/efi/libstub/Makefile     | 2 +-
> >  drivers/firmware/efi/libstub/arm64-stub.c | 8 +-------
> >  drivers/firmware/efi/libstub/hidden.h     | 6 ++++++
> >  4 files changed, 8 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/efi.h
> > b/arch/arm64/include/asm/efi.h
> > index 44531a69d32b..56ae87401a26 100644
> > --- a/arch/arm64/include/asm/efi.h
> > +++ b/arch/arm64/include/asm/efi.h
> > @@ -107,9 +107,6 @@ static inline void free_screen_info(struct
> > screen_info *si)
> >  {
> >  }
> >
> > -/* redeclare as 'hidden' so the compiler will generate relative
> > references */
> > -extern struct screen_info screen_info
> > __attribute__((__visibility__("hidden")));
> > -
> >  static inline void efifb_setup_from_dmi(struct screen_info *si,
> > const char *opt)
> >  {
> >  }
> > diff --git a/drivers/firmware/efi/libstub/Makefile
> > b/drivers/firmware/efi/libstub/Makefile
> > index f14b7636323a..4efdbd711e8e 100644
> > --- a/drivers/firmware/efi/libstub/Makefile
> > +++ b/drivers/firmware/efi/libstub/Makefile
> > @@ -25,7 +25,7 @@ cflags-$(CONFIG_ARM)                := $(subst
> > $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \
> >  cflags-$(CONFIG_EFI_ARMSTUB) += -I$(srctree)/scripts/dtc/libfdt
> >
> >  KBUILD_CFLAGS                        := $(cflags-y)
> > -DDISABLE_BRANCH_PROFILING \
> > -                                -D__NO_FORTIFY \
> > +                                -include hidden.h -D__NO_FORTIFY \
> >                                  $(call cc-option,-ffreestanding) \
> >                                  $(call cc-option,-fno-stack-
> > protector) \
> >                                  -D__DISABLE_EXPORTS
> > diff --git a/drivers/firmware/efi/libstub/arm64-stub.c
> > b/drivers/firmware/efi/libstub/arm64-stub.c
> > index 2915b44132e6..719d03a64329 100644
> > --- a/drivers/firmware/efi/libstub/arm64-stub.c
> > +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> > @@ -6,17 +6,11 @@
> >   * Adapted from ARM version by Mark Salter <msalter@redhat.com>
> >   */
> >
> > -/*
> > - * To prevent the compiler from emitting GOT-indirected (and thus
> > absolute)
> > - * references to the section markers, override their visibility as
> > 'hidden'
> > - */
> > -#pragma GCC visibility push(hidden)
> > -#include <asm/sections.h>
> > -#pragma GCC visibility pop
> >
> >  #include <linux/efi.h>
> >  #include <asm/efi.h>
> >  #include <asm/memory.h>
> > +#include <asm/sections.h>
> >  #include <asm/sysreg.h>
> >
> >  #include "efistub.h"
> > diff --git a/drivers/firmware/efi/libstub/hidden.h
> > b/drivers/firmware/efi/libstub/hidden.h
> > new file mode 100644
> > index 000000000000..3493b041f419
> > --- /dev/null
> > +++ b/drivers/firmware/efi/libstub/hidden.h
> > @@ -0,0 +1,6 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * To prevent the compiler from emitting GOT-indirected (and thus
> > absolute)
> > + * references to any global symbols, override their visibility as
> > 'hidden'
> > + */
> > +#pragma GCC visibility push(hidden)
>
> I am getting following compiler errors because of this patch while
> cross compiling for ARM, ARM64 and RISC-V.
>
> cc1: fatal error: hidden.h: No such file or directory
>
> Adding the following line to cflags solves the issue which indicates
> that gcc is not looking at the source file directory for this
> particularly include file.
>
> -I$(srctree)/drivers/firmware/efi/libstub
>
> I might have some trick in my build setup. Just posting here to see if
> anybody else also seeing the same problem.
>

Thanks Atish. Heinrich reported the same issue. It didn't affect me or
any of the bots because it only hits if you build in-tree (i.e.,
$srctree == $objtree)

My latest efi/next branch carries a fix for this (similar to your
suggestion, but using -include)

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 03/19] efi/libstub: Use hidden visiblity for all source files
  2020-02-24 23:36     ` Ard Biesheuvel
@ 2020-02-25 19:18       ` Atish Patra
  0 siblings, 0 replies; 29+ messages in thread
From: Atish Patra @ 2020-02-25 19:18 UTC (permalink / raw)
  To: ardb; +Cc: linux-efi, mingo, nivedita, lukas

On Tue, 2020-02-25 at 00:36 +0100, Ard Biesheuvel wrote:
> On Tue, 25 Feb 2020 at 00:15, Atish Patra <Atish.Patra@wdc.com>
> wrote:
> > On Mon, 2020-02-10 at 17:02 +0100, Ard Biesheuvel wrote:
> > > Instead of setting the visibility pragma for a small set of
> > > symbol
> > > declarations that could result in absolute references that we
> > > cannot
> > > support in the stub, declare hidden visibility for all code in
> > > the
> > > EFI stub, which is more robust and future proof.
> > > 
> > > To ensure that the #pragma is taken into account before any other
> > > includes are processed, put it in a header file of its own and
> > > include it via the compiler command line using the -include
> > > option.
> > > 
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > ---
> > >  arch/arm64/include/asm/efi.h              | 3 ---
> > >  drivers/firmware/efi/libstub/Makefile     | 2 +-
> > >  drivers/firmware/efi/libstub/arm64-stub.c | 8 +-------
> > >  drivers/firmware/efi/libstub/hidden.h     | 6 ++++++
> > >  4 files changed, 8 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/arch/arm64/include/asm/efi.h
> > > b/arch/arm64/include/asm/efi.h
> > > index 44531a69d32b..56ae87401a26 100644
> > > --- a/arch/arm64/include/asm/efi.h
> > > +++ b/arch/arm64/include/asm/efi.h
> > > @@ -107,9 +107,6 @@ static inline void free_screen_info(struct
> > > screen_info *si)
> > >  {
> > >  }
> > > 
> > > -/* redeclare as 'hidden' so the compiler will generate relative
> > > references */
> > > -extern struct screen_info screen_info
> > > __attribute__((__visibility__("hidden")));
> > > -
> > >  static inline void efifb_setup_from_dmi(struct screen_info *si,
> > > const char *opt)
> > >  {
> > >  }
> > > diff --git a/drivers/firmware/efi/libstub/Makefile
> > > b/drivers/firmware/efi/libstub/Makefile
> > > index f14b7636323a..4efdbd711e8e 100644
> > > --- a/drivers/firmware/efi/libstub/Makefile
> > > +++ b/drivers/firmware/efi/libstub/Makefile
> > > @@ -25,7 +25,7 @@ cflags-$(CONFIG_ARM)                := $(subst
> > > $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \
> > >  cflags-$(CONFIG_EFI_ARMSTUB) += -I$(srctree)/scripts/dtc/libfdt
> > > 
> > >  KBUILD_CFLAGS                        := $(cflags-y)
> > > -DDISABLE_BRANCH_PROFILING \
> > > -                                -D__NO_FORTIFY \
> > > +                                -include hidden.h -D__NO_FORTIFY
> > > \
> > >                                  $(call cc-option,-ffreestanding) 
> > > \
> > >                                  $(call cc-option,-fno-stack-
> > > protector) \
> > >                                  -D__DISABLE_EXPORTS
> > > diff --git a/drivers/firmware/efi/libstub/arm64-stub.c
> > > b/drivers/firmware/efi/libstub/arm64-stub.c
> > > index 2915b44132e6..719d03a64329 100644
> > > --- a/drivers/firmware/efi/libstub/arm64-stub.c
> > > +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> > > @@ -6,17 +6,11 @@
> > >   * Adapted from ARM version by Mark Salter <msalter@redhat.com>
> > >   */
> > > 
> > > -/*
> > > - * To prevent the compiler from emitting GOT-indirected (and
> > > thus
> > > absolute)
> > > - * references to the section markers, override their visibility
> > > as
> > > 'hidden'
> > > - */
> > > -#pragma GCC visibility push(hidden)
> > > -#include <asm/sections.h>
> > > -#pragma GCC visibility pop
> > > 
> > >  #include <linux/efi.h>
> > >  #include <asm/efi.h>
> > >  #include <asm/memory.h>
> > > +#include <asm/sections.h>
> > >  #include <asm/sysreg.h>
> > > 
> > >  #include "efistub.h"
> > > diff --git a/drivers/firmware/efi/libstub/hidden.h
> > > b/drivers/firmware/efi/libstub/hidden.h
> > > new file mode 100644
> > > index 000000000000..3493b041f419
> > > --- /dev/null
> > > +++ b/drivers/firmware/efi/libstub/hidden.h
> > > @@ -0,0 +1,6 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/*
> > > + * To prevent the compiler from emitting GOT-indirected (and
> > > thus
> > > absolute)
> > > + * references to any global symbols, override their visibility
> > > as
> > > 'hidden'
> > > + */
> > > +#pragma GCC visibility push(hidden)
> > 
> > I am getting following compiler errors because of this patch while
> > cross compiling for ARM, ARM64 and RISC-V.
> > 
> > cc1: fatal error: hidden.h: No such file or directory
> > 
> > Adding the following line to cflags solves the issue which
> > indicates
> > that gcc is not looking at the source file directory for this
> > particularly include file.
> > 
> > -I$(srctree)/drivers/firmware/efi/libstub
> > 
> > I might have some trick in my build setup. Just posting here to see
> > if
> > anybody else also seeing the same problem.
> > 
> 
> Thanks Atish. Heinrich reported the same issue. It didn't affect me
> or
> any of the bots because it only hits if you build in-tree (i.e.,
> $srctree == $objtree)
> 
> My latest efi/next branch carries a fix for this (similar to your
> suggestion, but using -include)

Thanks. That solves the problem. I should have looked in the efi-next
first :(.

-- 
Regards,
Atish

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 13/19] efi/libstub: Move get_dram_base() into arm-stub.c
  2020-02-17  8:37     ` Ard Biesheuvel
@ 2020-02-26 23:34       ` Atish Patra
  2020-02-27  7:38         ` Ard Biesheuvel
  0 siblings, 1 reply; 29+ messages in thread
From: Atish Patra @ 2020-02-26 23:34 UTC (permalink / raw)
  To: ardb; +Cc: linux-efi, mingo, nivedita, lukas

On Mon, 2020-02-17 at 09:37 +0100, Ard Biesheuvel wrote:
> On Mon, 17 Feb 2020 at 02:17, Atish Patra <Atish.Patra@wdc.com>
> wrote:
> > On Mon, 2020-02-10 at 17:02 +0100, Ard Biesheuvel wrote:
> > > get_dram_base() is only called from arm-stub.c so move it into
> > > the same source file as its caller.
> > > 
> > 
> > Just FYI: riscv efi stub port is also going to use get_dram_base().
> > However, I have renamed arm-stub.c to generic efi-stub.c so that
> > arm,
> > arm64 and riscv can reuse it. Thus, Moving get_dram_base() into
> > arm-
> > stub.c works for riscv as well. I will rebase my patches on top of
> > this
> > series.
> > 
> 
> Thanks Atish. I was hoping it would turn out like this, which is why
> I
> am pushing this series now.
> 
> I haven't looked at your code yet, but please avoid using the command
> line based initrd/dtb loading routines. I am proposing a cleaner way
> to provide the initrd from firmware [0], and dtb loading by the stub
> should not be done in the first place.
> 
> [0] 
> https://lore.kernel.org/linux-efi/20200216141104.21477-1-ardb@kernel.org/

Sorry I missed this email earlier. Yes I am not using initrd or dtb
loading from U-Boot command line.

-- 
Regards,
Atish

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 13/19] efi/libstub: Move get_dram_base() into arm-stub.c
  2020-02-26 23:34       ` Atish Patra
@ 2020-02-27  7:38         ` Ard Biesheuvel
  2020-02-27  7:48           ` Atish Patra
  0 siblings, 1 reply; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-27  7:38 UTC (permalink / raw)
  To: Atish Patra; +Cc: linux-efi, mingo, nivedita, lukas

On Thu, 27 Feb 2020 at 00:35, Atish Patra <Atish.Patra@wdc.com> wrote:
>
> On Mon, 2020-02-17 at 09:37 +0100, Ard Biesheuvel wrote:
> > On Mon, 17 Feb 2020 at 02:17, Atish Patra <Atish.Patra@wdc.com>
> > wrote:
> > > On Mon, 2020-02-10 at 17:02 +0100, Ard Biesheuvel wrote:
> > > > get_dram_base() is only called from arm-stub.c so move it into
> > > > the same source file as its caller.
> > > >
> > >
> > > Just FYI: riscv efi stub port is also going to use get_dram_base().
> > > However, I have renamed arm-stub.c to generic efi-stub.c so that
> > > arm,
> > > arm64 and riscv can reuse it. Thus, Moving get_dram_base() into
> > > arm-
> > > stub.c works for riscv as well. I will rebase my patches on top of
> > > this
> > > series.
> > >
> >
> > Thanks Atish. I was hoping it would turn out like this, which is why
> > I
> > am pushing this series now.
> >
> > I haven't looked at your code yet, but please avoid using the command
> > line based initrd/dtb loading routines. I am proposing a cleaner way
> > to provide the initrd from firmware [0], and dtb loading by the stub
> > should not be done in the first place.
> >
> > [0]
> > https://lore.kernel.org/linux-efi/20200216141104.21477-1-ardb@kernel.org/
>
> Sorry I missed this email earlier. Yes I am not using initrd or dtb
> loading from U-Boot command line.
>

If you use arm-stub.c mostly unmodified, you will be using initrd=
command line loading, and it would be better to disable that, and only
enable it if there is a request for it (which I doubt will happen).

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 13/19] efi/libstub: Move get_dram_base() into arm-stub.c
  2020-02-27  7:38         ` Ard Biesheuvel
@ 2020-02-27  7:48           ` Atish Patra
  2020-02-27  7:50             ` Ard Biesheuvel
  0 siblings, 1 reply; 29+ messages in thread
From: Atish Patra @ 2020-02-27  7:48 UTC (permalink / raw)
  To: ardb; +Cc: linux-efi, mingo, nivedita, lukas

On Thu, 2020-02-27 at 08:38 +0100, Ard Biesheuvel wrote:
> On Thu, 27 Feb 2020 at 00:35, Atish Patra <Atish.Patra@wdc.com>
> wrote:
> > On Mon, 2020-02-17 at 09:37 +0100, Ard Biesheuvel wrote:
> > > On Mon, 17 Feb 2020 at 02:17, Atish Patra <Atish.Patra@wdc.com>
> > > wrote:
> > > > On Mon, 2020-02-10 at 17:02 +0100, Ard Biesheuvel wrote:
> > > > > get_dram_base() is only called from arm-stub.c so move it
> > > > > into
> > > > > the same source file as its caller.
> > > > > 
> > > > 
> > > > Just FYI: riscv efi stub port is also going to use
> > > > get_dram_base().
> > > > However, I have renamed arm-stub.c to generic efi-stub.c so
> > > > that
> > > > arm,
> > > > arm64 and riscv can reuse it. Thus, Moving get_dram_base() into
> > > > arm-
> > > > stub.c works for riscv as well. I will rebase my patches on top
> > > > of
> > > > this
> > > > series.
> > > > 
> > > 
> > > Thanks Atish. I was hoping it would turn out like this, which is
> > > why
> > > I
> > > am pushing this series now.
> > > 
> > > I haven't looked at your code yet, but please avoid using the
> > > command
> > > line based initrd/dtb loading routines. I am proposing a cleaner
> > > way
> > > to provide the initrd from firmware [0], and dtb loading by the
> > > stub
> > > should not be done in the first place.
> > > 
> > > [0]
> > > https://lore.kernel.org/linux-efi/20200216141104.21477-1-ardb@kernel.org/
> > 
> > Sorry I missed this email earlier. Yes I am not using initrd or dtb
> > loading from U-Boot command line.
> > 
> 
> If you use arm-stub.c mostly unmodified, you will be using initrd=
> command line loading, 

Do you mean the code path enters to "initrd=" loading part ?
I think that code path has a minor issue where it prints even if the
there was no "initrd=" supplied in the command line.

"Loaded initrd from command line option\n" 

This happens because efi_load_initrd returns EFI_SUCCESS in absense of
command string as well.

https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/tree/drivers/firmware/efi/libstub/arm-stub.c?h=next#n281

> and it would be better to disable that, and only
> enable it if there is a request for it (which I doubt will happen).

-- 
Regards,
Atish

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 13/19] efi/libstub: Move get_dram_base() into arm-stub.c
  2020-02-27  7:48           ` Atish Patra
@ 2020-02-27  7:50             ` Ard Biesheuvel
  0 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2020-02-27  7:50 UTC (permalink / raw)
  To: Atish Patra; +Cc: linux-efi, mingo, nivedita, lukas

On Thu, 27 Feb 2020 at 08:48, Atish Patra <Atish.Patra@wdc.com> wrote:
>
> On Thu, 2020-02-27 at 08:38 +0100, Ard Biesheuvel wrote:
> > On Thu, 27 Feb 2020 at 00:35, Atish Patra <Atish.Patra@wdc.com>
> > wrote:
> > > On Mon, 2020-02-17 at 09:37 +0100, Ard Biesheuvel wrote:
> > > > On Mon, 17 Feb 2020 at 02:17, Atish Patra <Atish.Patra@wdc.com>
> > > > wrote:
> > > > > On Mon, 2020-02-10 at 17:02 +0100, Ard Biesheuvel wrote:
> > > > > > get_dram_base() is only called from arm-stub.c so move it
> > > > > > into
> > > > > > the same source file as its caller.
> > > > > >
> > > > >
> > > > > Just FYI: riscv efi stub port is also going to use
> > > > > get_dram_base().
> > > > > However, I have renamed arm-stub.c to generic efi-stub.c so
> > > > > that
> > > > > arm,
> > > > > arm64 and riscv can reuse it. Thus, Moving get_dram_base() into
> > > > > arm-
> > > > > stub.c works for riscv as well. I will rebase my patches on top
> > > > > of
> > > > > this
> > > > > series.
> > > > >
> > > >
> > > > Thanks Atish. I was hoping it would turn out like this, which is
> > > > why
> > > > I
> > > > am pushing this series now.
> > > >
> > > > I haven't looked at your code yet, but please avoid using the
> > > > command
> > > > line based initrd/dtb loading routines. I am proposing a cleaner
> > > > way
> > > > to provide the initrd from firmware [0], and dtb loading by the
> > > > stub
> > > > should not be done in the first place.
> > > >
> > > > [0]
> > > > https://lore.kernel.org/linux-efi/20200216141104.21477-1-ardb@kernel.org/
> > >
> > > Sorry I missed this email earlier. Yes I am not using initrd or dtb
> > > loading from U-Boot command line.
> > >
> >
> > If you use arm-stub.c mostly unmodified, you will be using initrd=
> > command line loading,
>
> Do you mean the code path enters to "initrd=" loading part ?
> I think that code path has a minor issue where it prints even if the
> there was no "initrd=" supplied in the command line.
>
> "Loaded initrd from command line option\n"
>
> This happens because efi_load_initrd returns EFI_SUCCESS in absense of
> command string as well.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/tree/drivers/firmware/efi/libstub/arm-stub.c?h=next#n281
>

Ah, thanks for pointing that out.

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2020-02-27  7:50 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 01/19] efi/libstub/x86: Remove pointless zeroing of apm_bios_info Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 02/19] efi/libstub/x86: Avoid overflowing code32_start on PE entry Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 03/19] efi/libstub: Use hidden visiblity for all source files Ard Biesheuvel
2020-02-24 23:15   ` Atish Patra
2020-02-24 23:36     ` Ard Biesheuvel
2020-02-25 19:18       ` Atish Patra
2020-02-10 16:02 ` [PATCH 04/19] efi/libstub/arm: Relax FDT alignment requirement Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 05/19] efi/libstub: Move memory map handling and allocation routines to mem.c Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 06/19] efi/libstub: Simplify efi_high_alloc() and rename to efi_allocate_pages() Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 07/19] efi/libstub/x86: Incorporate eboot.c into libstub Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 08/19] efi/libstub: Use consistent type names for file I/O protocols Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 09/19] efi/libstub: Move stub specific declarations into efistub.h Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 10/19] efi/libstub/x86: Permit bootparams struct to be allocated above 4 GB Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 11/19] efi/libstub/x86: Permit cmdline data " Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 12/19] efi/libstub: Move efi_random_alloc() into separate source file Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 13/19] efi/libstub: Move get_dram_base() into arm-stub.c Ard Biesheuvel
2020-02-17  1:17   ` Atish Patra
2020-02-17  8:37     ` Ard Biesheuvel
2020-02-26 23:34       ` Atish Patra
2020-02-27  7:38         ` Ard Biesheuvel
2020-02-27  7:48           ` Atish Patra
2020-02-27  7:50             ` Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 14/19] efi/libstub: Move file I/O support code into separate file Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 15/19] efi/libstub: Rewrite file I/O routine Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 16/19] efi/libstub: Take soft and hard memory limits into account for initrd loading Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 17/19] efi/libstub: Clean up command line parsing routine Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 18/19] efi/libstub: Expose LocateDevicePath boot service Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 19/19] efi/libstub: Make the LoadFile EFI protocol accessible Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).