[PATCH V4 0/6] ADD ARM EFI stub

[PATCH V4 0/6] ADD ARM EFI stub

[Roy Franz]

This patch series adds EFI stub support for the ARM architecture.  The 
stub for ARM is implemented in a similar manner to x86 in that it is a 
shim layer between EFI and the normal zImage/bzImage boot process, and 
that an image with the stub configured is bootable as both a zImage and 
EFI application.  


Changes since v3:
* Common and x86 EFI stub changes have been broken out and submitted separately.
  This patch series depends on the "[PATCH V5 00/18] ARM EFI stub common code"
  series as merged by Matt Fleming. (Patch 10 of that series omitted)
* Re-added -fno-stack-protector.  Needed for compilation with 
  CONFIG_CC_STACKPROTECTOR option.  Cannot easily add support for stack 
  protection since there are no functions in stub that never return, which 
  is a requirement for use.  (x86 stub also disables stack protector.)
  Note that even in the absense of -fno-stack-protector, stack protection
  was never present in the decompressor, as the required functions are not
  implemented.
* Fixed long summaries in commit messages.
* Further cleanups of assembly in head.S


Changes since v2:
* Now depends on "arm: Add [U]EFI runtime services support" patches by leif.lindholm@linaro.org.
  The EFI memory map is the only memory map passed to the kernel, so EFI support
  is now required.
* remove "-fno-stack-protector" from decompressor Makefile.  The current code doesn't
  trigger the stack protection, so the decompressor now compiles with it still
  on.  Note that there has never been any stack protection in the decompressor
  since the stack usage doesn't trigger the heuristic in GCC, so right now
  the "-fno-stack-protector" is a noop.
* Changed EFI command line handling to not have a fixed limit.
* Change FDT memory allocation to retry with a larger allocation if
  first educated guess is inadequate.
* Correctly set 'SizeOfCode' in PE/COFF header.
* Reviewed ".setup" section that is in x86 PE/COFF header.  This is used for x86
  to account for code that is not in the .text section.  We don't need this
  for ARM, as all of our code is in the .text section, or in the PE/COFF header
  itself.
* Moved EFI_STUB_ERROR #define to header file to share between stub C and ASM.
* Variety of cleanups and fixes in head.S.
* Changed update_fdt_and_exit_boot() to just update the device tree, and
  renamed appropriately.  Memory allocations moved out of this function as
  well, which enables the retries if the initial FDT size is too small.
  Note that in order to do the retried allocations, the original FDT and 
  command line memory regions are left allocated.  This is OK since the kernel
  has the memory map and will free these allocations along with the initrd
  and new fdt allocations.
* Added prefix to all prints, reduced number of prints, and reviewed all
  messages.
* Change mixed usage of dtb/fdt to all be fdt or "device tree" in efi-stub.c
* remove unnecessary zimage_size variable from relocate_kernel()
* correct return types on EFI functions - should be efi_status_t, not int.



Changes since V1:
* Updated head.S based on feedback from Dave Martin.  ARM/THUMB
  switches now much cleaner.

Roy Franz (6):
  efi-stub.txt updates for ARM
  Add shared update_fdt() function for ARM/ARM64
  Add strstr to compressed string.c for ARM.
  Add EFI stub for ARM
  Disable stack protection for decompressor/stub
  Add config EFI_STUB for ARM to Kconfig

 Documentation/efi-stub.txt             |   27 ++-
 arch/arm/Kconfig                       |   11 ++
 arch/arm/boot/compressed/Makefile      |   17 +-
 arch/arm/boot/compressed/efi-header.S  |  111 ++++++++++++
 arch/arm/boot/compressed/efi-stub.c    |  288 ++++++++++++++++++++++++++++++++
 arch/arm/boot/compressed/efi-stub.h    |    5 +
 arch/arm/boot/compressed/head.S        |   83 ++++++++-
 arch/arm/boot/compressed/string.c      |   21 +++
 drivers/firmware/efi/efi-stub-helper.c |   86 ++++++++++
 9 files changed, 633 insertions(+), 16 deletions(-)
 create mode 100644 arch/arm/boot/compressed/efi-header.S
 create mode 100644 arch/arm/boot/compressed/efi-stub.c
 create mode 100644 arch/arm/boot/compressed/efi-stub.h

-- 
1.7.10.4

[PATCH 1/6] efi-stub.txt updates for ARM

[Roy Franz]

Update efi-stub.txt documentation to be more general
and not x86 specific.  Add ARM only "dtb=" command
line option description.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
 Documentation/efi-stub.txt |   27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/Documentation/efi-stub.txt b/Documentation/efi-stub.txt
index 44e6bb6..19e897c 100644
--- a/Documentation/efi-stub.txt
+++ b/Documentation/efi-stub.txt
@@ -1,13 +1,16 @@
 			  The EFI Boot Stub
 		     ---------------------------
 
-On the x86 platform, a bzImage can masquerade as a PE/COFF image,
-thereby convincing EFI firmware loaders to load it as an EFI
-executable. The code that modifies the bzImage header, along with the
-EFI-specific entry point that the firmware loader jumps to are
-collectively known as the "EFI boot stub", and live in
+On the x86 and ARM platforms, a kernel zImage/bzImage can masquerade
+as a PE/COFF image, thereby convincing EFI firmware loaders to load
+it as an EFI executable. The code that modifies the bzImage header,
+along with the EFI-specific entry point that the firmware loader
+jumps to are collectively known as the "EFI boot stub", and live in
 arch/x86/boot/header.S and arch/x86/boot/compressed/eboot.c,
-respectively.
+respectively.  For ARM the EFI stub is implemented in
+arch/arm/boot/compressed/efi-header.S and
+arch/arm/boot/compressed/efi-stub.c.  EFI stub code that is shared
+between architectures is in drivers/firmware/efi/efi-stub-helper.c.
 
 By using the EFI boot stub it's possible to boot a Linux kernel
 without the use of a conventional EFI boot loader, such as grub or
@@ -23,7 +26,9 @@ The bzImage located in arch/x86/boot/bzImage must be copied to the EFI
 System Partiion (ESP) and renamed with the extension ".efi". Without
 the extension the EFI firmware loader will refuse to execute it. It's
 not possible to execute bzImage.efi from the usual Linux file systems
-because EFI firmware doesn't have support for them.
+because EFI firmware doesn't have support for them.  For ARM the
+arch/arm/boot/zImage should be copied to the system partition, and it
+may not need to be renamed.
 
 
 **** Passing kernel parameters from the EFI shell
@@ -63,3 +68,11 @@ Notice how bzImage.efi can be specified with a relative path. That's
 because the image we're executing is interpreted by the EFI shell,
 which understands relative paths, whereas the rest of the command line
 is passed to bzImage.efi.
+
+
+**** The "dtb=" option
+
+For the ARM architecture, we also need to be able to provide a device
+tree to the kernel.  This is done with the "dtb=" command line option,
+and is process in the same manner as the "initrd=" option that is described
+above.
-- 
1.7.10.4

[PATCH 2/6] Add shared update_fdt() function for ARM/ARM64

[Roy Franz]

Both ARM and ARM64 stubs will update the device tree
that they pass to the kernel.  In both cases they
primarily need to add the same UEFI related information,
so the function can be shared.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
 drivers/firmware/efi/efi-stub-helper.c |   86 ++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/drivers/firmware/efi/efi-stub-helper.c b/drivers/firmware/efi/efi-stub-helper.c
index cc0581d..d3448a9 100644
--- a/drivers/firmware/efi/efi-stub-helper.c
+++ b/drivers/firmware/efi/efi-stub-helper.c
@@ -4,6 +4,7 @@
  * implementation files.
  *
  * Copyright 2011 Intel Corporation; author Matt Fleming
+ * Copyright 2013 Linaro Limited; author Roy Franz
  *
  * This file is part of the Linux kernel, and is made available
  * under the terms of the GNU General Public License version 2.
@@ -636,3 +637,88 @@ static char *efi_convert_cmdline_to_ascii(efi_system_table_t *sys_table_arg,
 	*cmd_line_len = options_size;
 	return (char *)cmdline_addr;
 }
+
+#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
+static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
+			       void *fdt, int new_fdt_size, char *cmdline_ptr,
+			       u64 initrd_addr, u64 initrd_size,
+			       efi_memory_desc_t *memory_map,
+			       unsigned long map_size, unsigned long desc_size,
+			       u32 desc_ver)
+{
+	int node;
+	int status;
+	u32 fdt_val32;
+	u64 fdt_val64;
+
+	status = fdt_open_into(orig_fdt, fdt, new_fdt_size);
+	if (status != 0)
+		goto fdt_set_fail;
+
+	node = fdt_subnode_offset(fdt, 0, "chosen");
+	if (node < 0) {
+		node = fdt_add_subnode(fdt, 0, "chosen");
+		if (node < 0) {
+			status = node; /* node is error code when negative */
+			goto fdt_set_fail;
+		}
+	}
+
+	if ((cmdline_ptr != NULL) && (strlen(cmdline_ptr) > 0)) {
+		status = fdt_setprop(fdt, node, "bootargs", cmdline_ptr,
+				     strlen(cmdline_ptr) + 1);
+		if (status)
+			goto fdt_set_fail;
+	}
+
+	/* Set intird address/end in device tree, if present */
+	if (initrd_size != 0) {
+		u64 initrd_image_end;
+		u64 initrd_image_start = cpu_to_fdt64(initrd_addr);
+		status = fdt_setprop(fdt, node, "linux,initrd-start",
+				     &initrd_image_start, sizeof(u64));
+		if (status)
+			goto fdt_set_fail;
+		initrd_image_end = cpu_to_fdt64(initrd_addr + initrd_size);
+		status = fdt_setprop(fdt, node, "linux,initrd-end",
+				     &initrd_image_end, sizeof(u64));
+		if (status)
+			goto fdt_set_fail;
+	}
+
+	/* Add FDT entries for EFI runtime services in chosen node. */
+	node = fdt_subnode_offset(fdt, 0, "chosen");
+	fdt_val64 = cpu_to_fdt64((u64)(unsigned long)sys_table);
+	status = fdt_setprop(fdt, node, "linux,efi-system-table",
+			     &fdt_val64, sizeof(fdt_val64));
+	if (status)
+		goto fdt_set_fail;
+
+	fdt_val32 = cpu_to_fdt32(desc_size);
+	status = fdt_setprop(fdt, node, "linux,efi-mmap-desc-size",
+			     &fdt_val32, sizeof(fdt_val32));
+	if (status)
+		goto fdt_set_fail;
+
+	fdt_val32 = cpu_to_fdt32(desc_ver);
+	status = fdt_setprop(fdt, node, "linux,efi-mmap-desc-version",
+			     &fdt_val32, sizeof(fdt_val32));
+	if (status)
+		goto fdt_set_fail;
+
+
+	/* Stuff the whole memory map into FDT */
+	status = fdt_setprop(fdt, node, "linux,efi-mmap",
+			     memory_map, map_size);
+	if (status)
+		goto fdt_set_fail;
+
+	return EFI_SUCCESS;
+
+fdt_set_fail:
+	if (status == -FDT_ERR_NOSPACE)
+		return EFI_BUFFER_TOO_SMALL;
+
+	return EFI_LOAD_ERROR;
+}
+#endif
-- 
1.7.10.4

[PATCH 3/6] Add strstr to compressed string.c for ARM.

[Roy Franz]

The shared efi-stub-helper.c functions require a strstr
implementation.
Implementation copied from arch/x86/boot/string.c

Signed-off-by: Roy Franz <roy.franz@linaro.org>
Reviewed-by: Grant Likely <grant.likely@linaro.org>
---
 arch/arm/boot/compressed/string.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/compressed/string.c b/arch/arm/boot/compressed/string.c
index 36e53ef..5397792 100644
--- a/arch/arm/boot/compressed/string.c
+++ b/arch/arm/boot/compressed/string.c
@@ -111,6 +111,27 @@ char *strchr(const char *s, int c)
 	return (char *)s;
 }
 
+/**
+ * strstr - Find the first substring in a %NUL terminated string
+ * @s1: The string to be searched
+ * @s2: The string to search for
+ */
+char *strstr(const char *s1, const char *s2)
+{
+	size_t l1, l2;
+
+	l2 = strlen(s2);
+	if (!l2)
+		return (char *)s1;
+	l1 = strlen(s1);
+	while (l1 >= l2) {
+		l1--;
+		if (!memcmp(s1, s2, l2))
+			return (char *)s1;
+		s1++;
+	}
+	return NULL;
+}
 #undef memset
 
 void *memset(void *s, int c, size_t count)
-- 
1.7.10.4

[PATCH 4/6] Add EFI stub for ARM

[Roy Franz]

This patch adds EFI stub support for the ARM Linux kernel.  The EFI stub
operates similarly to the x86 stub: it is a shim between the EFI firmware
and the normal zImage entry point, and sets up the environment that the
zImage is expecting.  This includes loading the initrd (optionaly) and
device tree from the system partition based on the kernel command line.
The stub updates the device tree as necessary, adding entries for EFI
runtime services. The PE/COFF "MZ" header at offset 0 results in the
first instruction being an add that corrupts r5, which is not used by
the zImage interface.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
 arch/arm/boot/compressed/Makefile     |   15 +-
 arch/arm/boot/compressed/efi-header.S |  111 +++++++++++++
 arch/arm/boot/compressed/efi-stub.c   |  288 +++++++++++++++++++++++++++++++++
 arch/arm/boot/compressed/efi-stub.h   |    5 +
 arch/arm/boot/compressed/head.S       |   83 +++++++++-
 5 files changed, 494 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm/boot/compressed/efi-header.S
 create mode 100644 arch/arm/boot/compressed/efi-stub.c
 create mode 100644 arch/arm/boot/compressed/efi-stub.h

diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index 7ac1610..5fad8bd 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -103,11 +103,22 @@ libfdt_objs	:= $(addsuffix .o, $(basename $(libfdt)))
 $(addprefix $(obj)/,$(libfdt) $(libfdt_hdrs)): $(obj)/%: $(srctree)/scripts/dtc/libfdt/%
 	$(call cmd,shipped)
 
-$(addprefix $(obj)/,$(libfdt_objs) atags_to_fdt.o): \
+$(addprefix $(obj)/,$(libfdt_objs) atags_to_fdt.o efi-stub.o): \
 	$(addprefix $(obj)/,$(libfdt_hdrs))
 
 ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
-OBJS	+= $(libfdt_objs) atags_to_fdt.o
+OBJS	+= atags_to_fdt.o
+USE_LIBFDT = y
+endif
+
+ifeq ($(CONFIG_EFI_STUB),y)
+CFLAGS_efi-stub.o += -DTEXT_OFFSET=$(TEXT_OFFSET)
+OBJS	+= efi-stub.o
+USE_LIBFDT = y
+endif
+
+ifeq ($(USE_LIBFDT),y)
+OBJS	+= $(libfdt_objs)
 endif
 
 targets       := vmlinux vmlinux.lds \
diff --git a/arch/arm/boot/compressed/efi-header.S b/arch/arm/boot/compressed/efi-header.S
new file mode 100644
index 0000000..6965e0f
--- /dev/null
+++ b/arch/arm/boot/compressed/efi-header.S
@@ -0,0 +1,111 @@
+@ Copyright (C) 2013 Linaro Ltd;  <roy.franz@linaro.org>
+@
+@ This file contains the PE/COFF header that is part of the
+@ EFI stub.
+@
+
+	.org	0x3c
+	@
+	@ The PE header can be anywhere in the file, but for
+	@ simplicity we keep it together with the MSDOS header
+	@ The offset to the PE/COFF header needs to be at offset
+	@ 0x3C in the MSDOS header.
+	@ The only 2 fields of the MSDOS header that are used are this
+	@ PE/COFF offset, and the "MZ" bytes at offset 0x0.
+	@
+	.long	pe_header			@ Offset to the PE header.
+
+      .align 3
+pe_header:
+	.ascii	"PE"
+	.short 	0
+
+coff_header:
+	.short	0x01c2				@ ARM or Thumb
+	.short	2				@ nr_sections
+	.long	0 				@ TimeDateStamp
+	.long	0				@ PointerToSymbolTable
+	.long	1				@ NumberOfSymbols
+	.short	section_table - optional_header	@ SizeOfOptionalHeader
+	.short	0x306				@ Characteristics.
+						@ IMAGE_FILE_32BIT_MACHINE |
+						@ IMAGE_FILE_DEBUG_STRIPPED |
+						@ IMAGE_FILE_EXECUTABLE_IMAGE |
+						@ IMAGE_FILE_LINE_NUMS_STRIPPED
+
+optional_header:
+	.short	0x10b				@ PE32 format
+	.byte	0x02				@ MajorLinkerVersion
+	.byte	0x14				@ MinorLinkerVersion
+
+	.long	_edata - efi_stub_entry		@ SizeOfCode
+
+	.long	0				@ SizeOfInitializedData
+	.long	0				@ SizeOfUninitializedData
+
+	.long	efi_stub_entry			@ AddressOfEntryPoint
+	.long	efi_stub_entry			@ BaseOfCode
+	.long	0				@ data
+
+extra_header_fields:
+	.long	0				@ ImageBase
+	.long	0x20				@ SectionAlignment
+	.long	0x20				@ FileAlignment
+	.short	0				@ MajorOperatingSystemVersion
+	.short	0				@ MinorOperatingSystemVersion
+	.short	0				@ MajorImageVersion
+	.short	0				@ MinorImageVersion
+	.short	0				@ MajorSubsystemVersion
+	.short	0				@ MinorSubsystemVersion
+	.long	0				@ Win32VersionValue
+
+	.long	_edata				@ SizeOfImage
+
+	@ Everything before the entry point is considered part of the header
+	.long	efi_stub_entry			@ SizeOfHeaders
+	.long	0				@ CheckSum
+	.short	0xa				@ Subsystem (EFI application)
+	.short	0				@ DllCharacteristics
+	.long	0				@ SizeOfStackReserve
+	.long	0				@ SizeOfStackCommit
+	.long	0				@ SizeOfHeapReserve
+	.long	0				@ SizeOfHeapCommit
+	.long	0				@ LoaderFlags
+	.long	0x0				@ NumberOfRvaAndSizes
+
+	# Section table
+section_table:
+
+	#
+	# The EFI application loader requires a relocation section
+	# because EFI applications must be relocatable.  This is a
+	# dummy section as far as we are concerned.
+	#
+	.ascii	".reloc"
+	.byte	0
+	.byte	0			@ end of 0 padding of section name
+	.long	0
+	.long	0
+	.long	0			@ SizeOfRawData
+	.long	0			@ PointerToRawData
+	.long	0			@ PointerToRelocations
+	.long	0			@ PointerToLineNumbers
+	.short	0			@ NumberOfRelocations
+	.short	0			@ NumberOfLineNumbers
+	.long	0x42100040		@ Characteristics (section flags)
+
+
+	.ascii	".text"
+	.byte	0
+	.byte	0
+	.byte	0        		@ end of 0 padding of section name
+	.long	_edata - efi_stub_entry		@ VirtualSize
+	.long	efi_stub_entry			@ VirtualAddress
+	.long	_edata - efi_stub_entry		@ SizeOfRawData
+	.long	efi_stub_entry			@ PointerToRawData
+
+	.long	0		@ PointerToRelocations (0 for executables)
+	.long	0		@ PointerToLineNumbers (0 for executables)
+	.short	0		@ NumberOfRelocations  (0 for executables)
+	.short	0		@ NumberOfLineNumbers  (0 for executables)
+	.long	0xe0500020	@ Characteristics (section flags)
diff --git a/arch/arm/boot/compressed/efi-stub.c b/arch/arm/boot/compressed/efi-stub.c
new file mode 100644
index 0000000..a77cc4f
--- /dev/null
+++ b/arch/arm/boot/compressed/efi-stub.c
@@ -0,0 +1,288 @@
+/*
+ * linux/arch/arm/boot/compressed/efi-stub.c
+ *
+ * Copyright (C) 2013 Linaro Ltd;  <roy.franz@linaro.org>
+ *
+ * This file implements the EFI boot stub for the ARM kernel
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+#include <linux/efi.h>
+#include <libfdt.h>
+#include "efi-stub.h"
+
+/* EFI function call wrappers.  These are not required for
+ * ARM, but wrappers are required for X86 to convert between
+ * ABIs.  These wrappers are provided to allow code sharing
+ * between X86 and ARM.  Since these wrappers directly invoke the
+ * EFI function pointer, the function pointer type must be properly
+ * defined, which is not the case for X86  One advantage of this is
+ * it allows for type checking of arguments, which is not
+ * possible with the X86 wrappers.
+ */
+#define efi_call_phys0(f)			f()
+#define efi_call_phys1(f, a1)			f(a1)
+#define efi_call_phys2(f, a1, a2)		f(a1, a2)
+#define efi_call_phys3(f, a1, a2, a3)		f(a1, a2, a3)
+#define efi_call_phys4(f, a1, a2, a3, a4)	f(a1, a2, a3, a4)
+#define efi_call_phys5(f, a1, a2, a3, a4, a5)	f(a1, a2, a3, a4, a5)
+
+/* The maximum uncompressed kernel size is 32 MBytes, so we will reserve
+ * that for the decompressed kernel.  We have no easy way to tell what
+ * the actuall size of code + data the uncompressed kernel will use.
+ */
+#define MAX_UNCOMP_KERNEL_SIZE	0x02000000
+
+/* The kernel zImage should be located between 32 Mbytes
+ * and 128 MBytes from the base of DRAM.  The min
+ * address leaves space for a maximal size uncompressed image,
+ * and the max address is due to how the zImage decompressor
+ * picks a destination address.
+ */
+#define ZIMAGE_OFFSET_LIMIT	0x08000000
+#define MIN_ZIMAGE_OFFSET	MAX_UNCOMP_KERNEL_SIZE
+
+#define PRINTK_PREFIX		"EFIstub: "
+
+struct fdt_region {
+	u64 base;
+	u64 size;
+};
+
+
+/* Include shared EFI stub code */
+#include "../../../../drivers/firmware/efi/efi-stub-helper.c"
+
+
+int efi_entry(void *handle, efi_system_table_t *sys_table,
+	      unsigned long *zimage_addr)
+{
+	efi_loaded_image_t *image;
+	int status;
+	unsigned long nr_pages;
+	const struct fdt_region *region;
+
+	void *fdt;
+	int err;
+	int node;
+	unsigned long zimage_size = 0;
+	unsigned long dram_base;
+	/* addr/point and size pairs for memory management*/
+	unsigned long initrd_addr;
+	unsigned long initrd_size = 0;
+	unsigned long fdt_addr;
+	unsigned long fdt_size = 0;
+	efi_physical_addr_t kernel_reserve_addr;
+	unsigned long kernel_reserve_size = 0;
+	char *cmdline_ptr;
+	int cmdline_size = 0;
+
+	unsigned long map_size, desc_size;
+	u32 desc_ver;
+	unsigned long mmap_key;
+	efi_memory_desc_t *memory_map;
+
+	unsigned long new_fdt_size;
+	unsigned long new_fdt_addr;
+
+	efi_guid_t proto = LOADED_IMAGE_PROTOCOL_GUID;
+
+	/* Check if we were booted by the EFI firmware */
+	if (sys_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE)
+		goto fail;
+
+	efi_printk(sys_table, PRINTK_PREFIX"Booting Linux using EFI stub.\n");
+
+
+	/* get the command line from EFI, using the LOADED_IMAGE protocol */
+	status = efi_call_phys3(sys_table->boottime->handle_protocol,
+				handle, &proto, (void *)&image);
+	if (status != EFI_SUCCESS) {
+		efi_printk(sys_table, PRINTK_PREFIX"ERROR: Failed to get handle for LOADED_IMAGE_PROTOCOL.\n");
+		goto fail;
+	}
+
+	/* We are going to copy the command line into the device tree,
+	 * so this memory just needs to not conflict with boot protocol
+	 * requirements.
+	 */
+	cmdline_ptr = efi_convert_cmdline_to_ascii(sys_table, image,
+						   &cmdline_size);
+	if (!cmdline_ptr) {
+		efi_printk(sys_table, PRINTK_PREFIX"ERROR: Unable to allocate memory for command line.\n");
+		goto fail;
+	}
+
+	/* We first load the device tree, as we need to get the base address of
+	 * DRAM from the device tree.  The zImage, device tree, and initrd
+	 * have address restrictions that are relative to the base of DRAM.
+	 */
+	status = handle_cmdline_files(sys_table, image, cmdline_ptr, "dtb=",
+				      0xffffffff, &fdt_addr, &fdt_size);
+	if (status != EFI_SUCCESS) {
+		efi_printk(sys_table, PRINTK_PREFIX"ERROR: Unable to load device tree blob.\n");
+		goto fail_free_cmdline;
+	}
+
+	err = fdt_check_header((void *)fdt_addr);
+	if (err != 0) {
+		efi_printk(sys_table, PRINTK_PREFIX"ERROR: Device tree header not valid.\n");
+		goto fail_free_fdt;
+	}
+	if (fdt_totalsize((void *)fdt_addr) > fdt_size) {
+		efi_printk(sys_table, PRINTK_PREFIX"ERROR: Incomplete device tree.\n");
+		goto fail_free_fdt;
+
+	}
+
+
+	/* Look up the base of DRAM from the device tree. */
+	fdt = (void *)fdt_addr;
+	node = fdt_subnode_offset(fdt, 0, "memory");
+	region = fdt_getprop(fdt, node, "reg", NULL);
+	if (region) {
+		dram_base = fdt64_to_cpu(region->base);
+	} else {
+		/* There is no way to get amount or addresses of physical
+		 * memory installed using EFI calls.  If the device tree
+		 * we read from disk doesn't have this, there is no way
+		 * for us to construct this informaion.
+		 */
+		efi_printk(sys_table, PRINTK_PREFIX"ERROR: No 'memory' node in device tree.\n");
+		goto fail_free_fdt;
+	}
+
+	/* Reserve memory for the uncompressed kernel image. This is
+	 * all that prevents any future allocations from conflicting
+	 * with the kernel.  Since we can't tell from the compressed
+	 * image how much DRAM the kernel actually uses (due to BSS
+	 * size uncertainty) we allocate the maximum possible size.
+	 */
+	kernel_reserve_addr = dram_base;
+	kernel_reserve_size = MAX_UNCOMP_KERNEL_SIZE;
+	nr_pages = round_up(kernel_reserve_size, EFI_PAGE_SIZE) / EFI_PAGE_SIZE;
+	status = efi_call_phys4(sys_table->boottime->allocate_pages,
+				EFI_ALLOCATE_ADDRESS, EFI_LOADER_DATA,
+				nr_pages, &kernel_reserve_addr);
+	if (status != EFI_SUCCESS) {
+		efi_printk(sys_table, PRINTK_PREFIX"ERROR: Unable to allocate memory for uncompressed kernel.\n");
+		goto fail_free_fdt;
+	}
+
+	/* Relocate the zImage, if required.  ARM doesn't have a
+	 * preferred address, so we set it to 0, as we want to allocate
+	 * as low in memory as possible.
+	 */
+	zimage_size = image->image_size;
+	status = efi_relocate_kernel(sys_table, zimage_addr, zimage_size,
+				     zimage_size, 0, 0);
+	if (status != EFI_SUCCESS) {
+		efi_printk(sys_table, PRINTK_PREFIX"ERROR: Failed to relocate kernel.\n");
+		goto fail_free_kernel_reserve;
+	}
+
+	/* Check to see if we were able to allocate memory low enough
+	 * in memory.
+	 */
+	if (*zimage_addr + zimage_size > dram_base + ZIMAGE_OFFSET_LIMIT) {
+		efi_printk(sys_table, PRINTK_PREFIX"ERROR: Failed to relocate kernel, no low memory available.\n");
+		goto fail_free_zimage;
+	}
+	status = handle_cmdline_files(sys_table, image, cmdline_ptr, "initrd=",
+				      dram_base + ZIMAGE_OFFSET_LIMIT,
+				      &initrd_addr, &initrd_size);
+	if (status != EFI_SUCCESS) {
+		efi_printk(sys_table, PRINTK_PREFIX"ERROR: Unable to load initrd.\n");
+		goto fail_free_zimage;
+	}
+
+	/* Estimate size of new FDT, and allocate memory for it. We
+	 * will allocate a bigger buffer if this ends up being too
+	 * small, so a rough guess is OK here.*/
+	new_fdt_size = fdt_size + cmdline_size + 0x800;
+	while (1) {
+		status = efi_high_alloc(sys_table, new_fdt_size, 0,
+					&new_fdt_addr,
+					dram_base + ZIMAGE_OFFSET_LIMIT);
+		if (status != EFI_SUCCESS) {
+			efi_printk(sys_table, PRINTK_PREFIX"ERROR: Unable to allocate memory for new device tree.\n");
+			goto fail_free_initrd;
+		}
+
+		/* Now that we have done our final memory allocation (and free)
+		 * we can get the memory map key  needed for
+		 * exit_boot_services().
+		 */
+		status = efi_get_memory_map(sys_table, &memory_map, &map_size,
+					    &desc_size, &desc_ver, &mmap_key);
+		if (status != EFI_SUCCESS)
+			goto fail_free_new_fdt;
+
+		status = update_fdt(sys_table,
+				    fdt, (void *)new_fdt_addr, new_fdt_size,
+				    cmdline_ptr,
+				    initrd_addr, initrd_size,
+				    memory_map, map_size, desc_size, desc_ver);
+
+		/* Succeeding the first time is the expected case. */
+		if (status == EFI_SUCCESS)
+			break;
+
+		if (status == EFI_BUFFER_TOO_SMALL) {
+			/* We need to allocate more space for the new
+			 * device tree, so free existing buffer that is
+			 * too small.  Also free memory map, as we will need
+			 * to get new one that reflects the free/alloc we do
+			 * on the device tree buffer. */
+			efi_free(sys_table, new_fdt_size, new_fdt_addr);
+			efi_call_phys1(sys_table->boottime->free_pool,
+				       memory_map);
+			new_fdt_size += new_fdt_size / 4;
+		} else {
+			efi_printk(sys_table, PRINTK_PREFIX"ERROR: Unable to constuct new device tree.\n");
+			goto fail_free_mmap;
+		}
+	}
+
+	/* Now we are ready to exit_boot_services.*/
+	status = efi_call_phys2(sys_table->boottime->exit_boot_services,
+				handle, mmap_key);
+
+	if (status != EFI_SUCCESS) {
+		efi_printk(sys_table, PRINTK_PREFIX"ERROR: Exit boot services failed.\n");
+		goto fail_free_mmap;
+	}
+
+
+	/* Now we need to return the FDT address to the calling
+	 * assembly to this can be used as part of normal boot.
+	 */
+	return new_fdt_addr;
+
+fail_free_mmap:
+	efi_call_phys1(sys_table->boottime->free_pool, memory_map);
+
+fail_free_new_fdt:
+	efi_free(sys_table, new_fdt_size, new_fdt_addr);
+
+fail_free_initrd:
+	efi_free(sys_table, initrd_size, initrd_addr);
+
+fail_free_zimage:
+	efi_free(sys_table, zimage_size, *zimage_addr);
+
+fail_free_kernel_reserve:
+	efi_free(sys_table, kernel_reserve_size, kernel_reserve_addr);
+
+fail_free_fdt:
+	efi_free(sys_table, fdt_size, fdt_addr);
+
+fail_free_cmdline:
+	efi_free(sys_table, cmdline_size, (u32)cmdline_ptr);
+
+fail:
+	return EFI_STUB_ERROR;
+}
diff --git a/arch/arm/boot/compressed/efi-stub.h b/arch/arm/boot/compressed/efi-stub.h
new file mode 100644
index 0000000..0fe9376
--- /dev/null
+++ b/arch/arm/boot/compressed/efi-stub.h
@@ -0,0 +1,5 @@
+#ifndef _ARM_EFI_STUB_H
+#define _ARM_EFI_STUB_H
+/* Error code returned to ASM code instead of valid FDT address. */
+#define EFI_STUB_ERROR		(~0)
+#endif
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 75189f1..aae3c75 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -10,6 +10,7 @@
  */
 #include <linux/linkage.h>
 #include <asm/assembler.h>
+#include "efi-stub.h"
 
 	.arch	armv7-a
 /*
@@ -120,21 +121,92 @@
  */
 		.align
 		.arm				@ Always enter in ARM state
+		.text
 start:
 		.type	start,#function
-		.rept	7
+#ifdef CONFIG_EFI_STUB
+		@ Magic MSDOS signature for PE/COFF + ADD opcode
+		@ the EFI stub only supports little endian, as the EFI functions
+		@ it invokes are little endian.
+		.word	0x62805a4d
+#else
+		mov	r0, r0
+#endif
+		.rept	5
 		mov	r0, r0
 		.endr
-   ARM(		mov	r0, r0		)
-   ARM(		b	1f		)
- THUMB(		adr	r12, BSYM(1f)	)
- THUMB(		bx	r12		)
+
+		adrl	r12, BSYM(zimage_continue)
+ ARM(		mov     pc, r12 )
+ THUMB(		bx	r12     )
+		@ zimage_continue will be in ARM or thumb mode as configured
 
 		.word	0x016f2818		@ Magic numbers to help the loader
 		.word	start			@ absolute load/run zImage address
 		.word	_edata			@ zImage end address
+
+#ifdef CONFIG_EFI_STUB
+		@ Portions of the MSDOS file header must be at offset
+		@ 0x3c from the start of the file.  All PE/COFF headers
+		@ are kept contiguous for simplicity.
+#include "efi-header.S"
+
+efi_stub_entry:
+		@ The EFI stub entry point is not at a fixed address, however
+		@ this address must be set in the PE/COFF header.
+		@ EFI entry point is in A32 mode, switch to T32 if configured.
+ THUMB(		adr	r12, BSYM(1f)	)
+ THUMB(		bx	r12		)
  THUMB(		.thumb			)
 1:
+		@ Save lr on stack for possible return to EFI firmware.
+		@ Don't care about fp, but need 64 bit alignment....
+		stmfd	sp!, {fp, lr}
+
+		@ allocate space on stack for passing current zImage address
+		@ and for the EFI stub to return of new entry point of
+		@ zImage, as EFI stub may copy the kernel.  Pointer address
+		@ is passed in r2.  r0 and r1 are passed through from the
+		@ EFI firmware to efi_entry
+		adr	r3, start
+		str	r3, [sp, #-8]!
+		mov	r2, sp			@ pass pointer in r2
+		bl	efi_entry
+		ldr	r3, [sp], #8	@ get new zImage address from stack
+
+		@ Check for error return from EFI stub.  r0 has FDT address
+		@ or EFI_STUB_ERROR error code.
+		cmp	r0, #EFI_STUB_ERROR
+		beq	efi_load_fail
+
+		@ Save return values of efi_entry
+		stmfd	sp!, {r0, r3}
+		bl	cache_clean_flush
+		bl	cache_off
+		ldmfd   sp!, {r0, r3}
+
+		@ Set parameters for booting zImage according to boot protocol
+		@ put FDT address in r2, it was returned by efi_entry()
+		@ r1 is FDT machine type, and r0 needs to be 0
+		mov	r2, r0
+		mov	r1, #0xFFFFFFFF
+		mov	r0, #0
+
+		@ Branch to (possibly) relocated zImage that is in r3
+		@ Make sure we are in A32 mode, as zImage requires
+ THUMB(		bx	r3		)
+ ARM(		mov	pc, r3		)
+
+efi_load_fail:
+		@ Return EFI_LOAD_ERROR to EFI firmware on error.
+		@ Switch back to ARM mode for EFI is done based on
+		@ return address on stack in case we are in THUMB mode
+		ldr	r0, =0x80000001
+		ldmfd	sp!, {fp, pc}		@ put lr from stack into pc
+#endif
+
+ THUMB(		.thumb			)
+zimage_continue:
 		mrs	r9, cpsr
 #ifdef CONFIG_ARM_VIRT_EXT
 		bl	__hyp_stub_install	@ get into SVC mode, reversibly
@@ -167,7 +239,6 @@ not_angel:
 		 * by the linker here, but it should preserve r7, r8, and r9.
 		 */
 
-		.text
 
 #ifdef CONFIG_AUTO_ZRELADDR
 		@ determine final kernel image address
-- 
1.7.10.4

[PATCH 5/6] Disable stack protection for decompressor/stub

[Roy Franz]

The ARM decompressor/EFI stub do not implement the functions
(__stack_chk_guard_setup, etc) that are required for support of
stack protection.  The actual enablement of stack protection is
controlled by heuristics in GCC, which the code added for the EFI
stub triggers when CONFIG_STACKPROTECTOR is set.  Even with
CONFIG_STACKPROTECTOR set, the decompressor was not compiled
with stack protection actually enabled. Adding -fno-stack-protector
to the decompressor/stub build keeps it building without stack
protection as it has always been built.
The x86 decompressor/stub is also build with -fno-stack-protector.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
 arch/arm/boot/compressed/Makefile |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index 5fad8bd..551dbf9 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -136,7 +136,7 @@ ORIG_CFLAGS := $(KBUILD_CFLAGS)
 KBUILD_CFLAGS = $(subst -pg, , $(ORIG_CFLAGS))
 endif
 
-ccflags-y := -fpic -mno-single-pic-base -fno-builtin -I$(obj)
+ccflags-y := -fpic -mno-single-pic-base -fno-builtin -fno-stack-protector -I$(obj)
 asflags-y := -DZIMAGE
 
 # Supply kernel BSS size to the decompressor via a linker symbol.
-- 
1.7.10.4

[PATCH 6/6] Add config EFI_STUB for ARM to Kconfig

[Roy Franz]

Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
 arch/arm/Kconfig |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 43594d5..9c84429 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1805,6 +1805,17 @@ config UACCESS_WITH_MEMCPY
 	  However, if the CPU data cache is using a write-allocate mode,
 	  this option is unlikely to provide any performance gain.
 
+config EFI_STUB
+	bool "EFI stub support"
+	depends on EFI && !CPU_BIG_ENDIAN
+	---help---
+	  This kernel feature allows a zImage to be loaded directly
+	  by EFI firmware without the use of a bootloader.  A PE/COFF
+	  header is added to the zImage in a way that makes the binary
+	  both a Linux zImage and an PE/COFF executable that can be
+	  executed directly by EFI firmware.
+	  See Documentation/efi-stub.txt for more information.
+
 config SECCOMP
 	bool
 	prompt "Enable seccomp to safely compute untrusted bytecode"
-- 
1.7.10.4

Re: [PATCH 1/6] efi-stub.txt updates for ARM

[Matt Fleming]

On Sat, 28 Sep, at 09:23:18AM, Roy Franz wrote:
> Update efi-stub.txt documentation to be more general
> and not x86 specific.  Add ARM only "dtb=" command
> line option description.
> 
> Signed-off-by: Roy Franz <roy.franz@linaro.org>
> ---
>  Documentation/efi-stub.txt |   27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)

Looks good to me.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 2/6] Add shared update_fdt() function for ARM/ARM64

[Matt Fleming]

On Sat, 28 Sep, at 09:23:19AM, Roy Franz wrote:
> Both ARM and ARM64 stubs will update the device tree
> that they pass to the kernel.  In both cases they
> primarily need to add the same UEFI related information,
> so the function can be shared.
> 
> Signed-off-by: Roy Franz <roy.franz@linaro.org>
> ---
>  drivers/firmware/efi/efi-stub-helper.c |   86 ++++++++++++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
> 
> diff --git a/drivers/firmware/efi/efi-stub-helper.c b/drivers/firmware/efi/efi-stub-helper.c
> index cc0581d..d3448a9 100644
> --- a/drivers/firmware/efi/efi-stub-helper.c
> +++ b/drivers/firmware/efi/efi-stub-helper.c
> @@ -4,6 +4,7 @@
>   * implementation files.
>   *
>   * Copyright 2011 Intel Corporation; author Matt Fleming
> + * Copyright 2013 Linaro Limited; author Roy Franz
>   *
>   * This file is part of the Linux kernel, and is made available
>   * under the terms of the GNU General Public License version 2.
> @@ -636,3 +637,88 @@ static char *efi_convert_cmdline_to_ascii(efi_system_table_t *sys_table_arg,
>  	*cmd_line_len = options_size;
>  	return (char *)cmdline_addr;
>  }
> +
> +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
> +static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
> +			       void *fdt, int new_fdt_size, char *cmdline_ptr,
> +			       u64 initrd_addr, u64 initrd_size,
> +			       efi_memory_desc_t *memory_map,
> +			       unsigned long map_size, unsigned long desc_size,
> +			       u32 desc_ver)

Hmm... does this function really belong in efi-stub-helper.c? That file
should be for architecture independent functionality only.

While it currently does include this little snippet,

#ifdef CONFIG_ARM
	/*
	 * For ARM, allocate at a high address to avoid reserved
	 * regions at low addresses that we don't know the specfics of
	 * at the time we are processing the command line.
	 */
	status = efi_high_alloc(sys_table_arg, options_size, 0,
			    &cmdline_addr, 0xfffff000);
#else
	status = efi_low_alloc(sys_table_arg, options_size, 0,
			    &cmdline_addr);
#endif

I think that single #ifdef is justifiable. But including entire
functions that are only used by ARM/ARM64 or x86 isn't.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 2/6] Add shared update_fdt() function for ARM/ARM64

[Mark Salter]

On Thu, 2013-10-03 at 10:52 +0100, Matt Fleming wrote:
> > +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
> > +static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
> > +                            void *fdt, int new_fdt_size, char *cmdline_ptr,
> > +                            u64 initrd_addr, u64 initrd_size,
> > +                            efi_memory_desc_t *memory_map,
> > +                            unsigned long map_size, unsigned long desc_size,
> > +                            u32 desc_ver)
> 
> Hmm... does this function really belong in efi-stub-helper.c? That file
> should be for architecture independent functionality only.
> 

It isn't really arm-specific although arm is the only user right now.
We're using the FDT to pass EFI boot info that is passed in the boot
params block on x86. So potentially other architectures could use the
same code. How about making it someting like:

#ifdef ARCH_NEEDS_EFI_FDT
static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
...
#endif

so the architecture can decide whether to include it or not.

--Mark

Re: [PATCH 2/6] Add shared update_fdt() function for ARM/ARM64

[Matt Fleming]

On Thu, 03 Oct, at 09:43:24AM, Mark Salter wrote:
> On Thu, 2013-10-03 at 10:52 +0100, Matt Fleming wrote:
> > > +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
> > > +static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
> > > +                            void *fdt, int new_fdt_size, char *cmdline_ptr,
> > > +                            u64 initrd_addr, u64 initrd_size,
> > > +                            efi_memory_desc_t *memory_map,
> > > +                            unsigned long map_size, unsigned long desc_size,
> > > +                            u32 desc_ver)
> > 
> > Hmm... does this function really belong in efi-stub-helper.c? That file
> > should be for architecture independent functionality only.
> > 
> 
> It isn't really arm-specific although arm is the only user right now.
> We're using the FDT to pass EFI boot info that is passed in the boot
> params block on x86. So potentially other architectures could use the
> same code.

It's not EFI-specific either, apart from the fdt property names.

> How about making it someting like:
> 
> #ifdef ARCH_NEEDS_EFI_FDT
> static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
> ...
> #endif
> 
> so the architecture can decide whether to include it or not.

Is one implementation of this function going to fit all architectures?
Can we be sure of that ahead of time?

I'd prefer this to be kept in arch/ for now, and possibly moved into
efi-stub-helper.c at a later date if indeed it turns out to be generally
useful.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 2/6] Add shared update_fdt() function for ARM/ARM64

[Mark Salter]

On Thu, 2013-10-03 at 15:27 +0100, Matt Fleming wrote:
> On Thu, 03 Oct, at 09:43:24AM, Mark Salter wrote:
> > On Thu, 2013-10-03 at 10:52 +0100, Matt Fleming wrote:
> > > > +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
> > > > +static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
> > > > +                            void *fdt, int new_fdt_size, char *cmdline_ptr,
> > > > +                            u64 initrd_addr, u64 initrd_size,
> > > > +                            efi_memory_desc_t *memory_map,
> > > > +                            unsigned long map_size, unsigned long desc_size,
> > > > +                            u32 desc_ver)
> > > 
> > > Hmm... does this function really belong in efi-stub-helper.c? That file
> > > should be for architecture independent functionality only.
> > > 
> > 
> > It isn't really arm-specific although arm is the only user right now.
> > We're using the FDT to pass EFI boot info that is passed in the boot
> > params block on x86. So potentially other architectures could use the
> > same code.
> 
> It's not EFI-specific either, apart from the fdt property names.

Not EFI-specific in the sense that it uses the EFI services, but it is
EFI-specific in its function.

> 
> > How about making it someting like:
> > 
> > #ifdef ARCH_NEEDS_EFI_FDT
> > static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
> > ...
> > #endif
> > 
> > so the architecture can decide whether to include it or not.
> 
> Is one implementation of this function going to fit all architectures?
> Can we be sure of that ahead of time?
> 
> I'd prefer this to be kept in arch/ for now, and possibly moved into
> efi-stub-helper.c at a later date if indeed it turns out to be generally
> useful.

Fair enough. I don't have a strong opinion about it either way. Just a
natural instinct to share the code if possible, but it is easy enough
to change later if others want to use it.

--Mark

Re: [PATCH 2/6] Add shared update_fdt() function for ARM/ARM64

[Roy Franz]

On Thu, Oct 3, 2013 at 7:27 AM, Matt Fleming <matt@console-pimps.org> wrote:
> On Thu, 03 Oct, at 09:43:24AM, Mark Salter wrote:
>> On Thu, 2013-10-03 at 10:52 +0100, Matt Fleming wrote:
>> > > +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
>> > > +static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
>> > > +                            void *fdt, int new_fdt_size, char *cmdline_ptr,
>> > > +                            u64 initrd_addr, u64 initrd_size,
>> > > +                            efi_memory_desc_t *memory_map,
>> > > +                            unsigned long map_size, unsigned long desc_size,
>> > > +                            u32 desc_ver)
>> >
>> > Hmm... does this function really belong in efi-stub-helper.c? That file
>> > should be for architecture independent functionality only.
>> >
>>
>> It isn't really arm-specific although arm is the only user right now.
>> We're using the FDT to pass EFI boot info that is passed in the boot
>> params block on x86. So potentially other architectures could use the
>> same code.
>
> It's not EFI-specific either, apart from the fdt property names.
>
>> How about making it someting like:
>>
>> #ifdef ARCH_NEEDS_EFI_FDT
>> static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
>> ...
>> #endif
>>
>> so the architecture can decide whether to include it or not.
>
> Is one implementation of this function going to fit all architectures?
> Can we be sure of that ahead of time?
>
> I'd prefer this to be kept in arch/ for now, and possibly moved into
> efi-stub-helper.c at a later date if indeed it turns out to be generally
> useful.
>
> --
> Matt Fleming, Intel Open Source Technology Center

Hi Matt,

   This  function is shared between ARM and ARM64, which are separate
architectures so to keep this in arch code would require it to be
duplicated for ARM/ARM64.  Would moving this function to a new
"efi-stub-fdt.c" file be satisfactory?  This file would just be used
the ARM and ARM64, so this would remove
the need for the top-level #ifdefs.

Thanks,
Roy

Re: [PATCH 2/6] Add shared update_fdt() function for ARM/ARM64

[Matt Fleming]

On Thu, 03 Oct, at 12:28:03PM, Roy Franz wrote:
> Hi Matt,
> 
>    This  function is shared between ARM and ARM64, which are separate
> architectures so to keep this in arch code would require it to be
> duplicated for ARM/ARM64.  Would moving this function to a new
> "efi-stub-fdt.c" file be satisfactory?  This file would just be used
> the ARM and ARM64, so this would remove
> the need for the top-level #ifdefs.

Fair enough, move it to drivers/firmware/efi/fdt.c - there's no need to
prefix it with "efi-" (efi-pstore.c has historic baggage).

-- 
Matt Fleming, Intel Open Source Technology Center