[PATCH v2 0/5] arm64 UEFI early FDT handling

[PATCH v2 0/5] arm64 UEFI early FDT handling

[Ard Biesheuvel]

This is a followup to the "arm64: update/clarify/relax Image and FDT placement
rules" series I sent a while ago:
(http://article.gmane.org/gmane.linux.ports.arm.kernel/407148)

This has now been split in two series: this first series deals with the
early FDT handling, primarily in the context of UEFI, but not exclusively.

A number of minor issues exist in the early UEFI/FDT handling path, such as:
- when booting via UEFI, memreserve entries are removed from the device tree but
  the /reserved-memory node is not
- memory nodes are removed from the device tree in a way that is not officially
  supported by the libfdt API (i.e., you cannot delete nodes while traversing
  the tree)
- removal of memory nodes may discard annotations (such as NUMA topology) that
  should ideally be retained, or may corrupt the tree by discarding nodes
  referenced by phandles.

Patch #1 introduces an arm64 specific version of early_init_dt_add_memory_arch()
so that we can modify it later to ignore DT memory nodes if booting via UEFI.

Patch #2 moves some UEFI+FDT init code around before making changes to it.

Patch #3 moves the UEFI initialization to before the early FDT scanning so we
know at that point whether we are booting via UEFI or not.

Patch #4 changes the UEFI init code so that memory nodes are simply ignored, so
that they don't have to be removed by the stub anymore.

Patch #5 does the same as #6, but for memreserves and the /reserved-memory
node.

Changes since v1:
- dropped first two patches, they have been merged into v4.2-rc1
- dropped last patch regarding FDT placement by the stub, this is not entirely
  relevant to the primary issue targeted by this series
- rebased onto for-next/core (arm64) as of today

Ard Biesheuvel (5):
  arm64: clone early_init_dt_add_memory_arch() to override default
  efi: move FDT handling to separate object file
  arm64/efi: move EFI init before early FDT processing
  arm64/efi: ignore DT memory nodes instead of removing them
  arm64/efi: ignore DT memreserve entries instead of removing them

 arch/arm64/include/asm/efi.h       |  4 +-
 arch/arm64/kernel/efi.c            | 14 +---
 arch/arm64/kernel/setup.c          |  4 +-
 arch/arm64/mm/init.c               | 54 +++++++++++-
 drivers/firmware/efi/Makefile      |  1 +
 drivers/firmware/efi/efi-fdt.c     | 77 ++++++++++++++++++
 drivers/firmware/efi/efi.c         | 86 --------------------
 drivers/firmware/efi/libstub/fdt.c | 33 +-------
 include/linux/efi.h                |  3 +-
 9 files changed, 143 insertions(+), 133 deletions(-)
 create mode 100644 drivers/firmware/efi/efi-fdt.c

-- 
1.9.1

[PATCH v2 1/5] arm64: clone early_init_dt_add_memory_arch() to override default

[Ard Biesheuvel]

Override the __weak early_init_dt_add_memory_arch() with our own
version. We need this in a subsequent patch to make the handling of
the memory nodes conditional on whether we are booting via UEFI or
not.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/mm/init.c | 41 ++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index f5c0680d17d9..ab25fde7397c 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -374,3 +374,44 @@ static int __init keepinitrd_setup(char *__unused)
 
 __setup("keepinitrd", keepinitrd_setup);
 #endif
+
+void __init early_init_dt_add_memory_arch(u64 base, u64 size)
+{
+	const u64 phys_offset = __pa(PAGE_OFFSET);
+
+	if (!PAGE_ALIGNED(base)) {
+		if (size < PAGE_SIZE - (base & ~PAGE_MASK)) {
+			pr_warn("Ignoring memory block 0x%llx - 0x%llx\n",
+				base, base + size);
+			return;
+		}
+		size -= PAGE_SIZE - (base & ~PAGE_MASK);
+		base = PAGE_ALIGN(base);
+	}
+	size &= PAGE_MASK;
+
+	if (base > MAX_MEMBLOCK_ADDR) {
+		pr_warning("Ignoring memory block 0x%llx - 0x%llx\n",
+				base, base + size);
+		return;
+	}
+
+	if (base + size - 1 > MAX_MEMBLOCK_ADDR) {
+		pr_warning("Ignoring memory range 0x%llx - 0x%llx\n",
+				((u64)MAX_MEMBLOCK_ADDR) + 1, base + size);
+		size = MAX_MEMBLOCK_ADDR - base + 1;
+	}
+
+	if (base + size < phys_offset) {
+		pr_warning("Ignoring memory block 0x%llx - 0x%llx\n",
+			   base, base + size);
+		return;
+	}
+	if (base < phys_offset) {
+		pr_warning("Ignoring memory range 0x%llx - 0x%llx\n",
+			   base, phys_offset);
+		size -= phys_offset - base;
+		base = phys_offset;
+	}
+	memblock_add(base, size);
+}
-- 
1.9.1

[PATCH v2 2/5] efi: move FDT handling to separate object file

[Ard Biesheuvel]

The EFI specific FDT handling is compiled conditionally, and is
logically independent of the rest of efi.o. So move it to a separate
file before making changes to it in subsequent patches.

Acked-by: Matt Fleming <matt.fleming@intel.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/Makefile  |  1 +
 drivers/firmware/efi/efi-fdt.c | 93 ++++++++++++++++++++
 drivers/firmware/efi/efi.c     | 86 ------------------
 3 files changed, 94 insertions(+), 86 deletions(-)

diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index 6fd3da938717..d9140208fc1c 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_UEFI_CPER)			+= cper.o
 obj-$(CONFIG_EFI_RUNTIME_MAP)		+= runtime-map.o
 obj-$(CONFIG_EFI_RUNTIME_WRAPPERS)	+= runtime-wrappers.o
 obj-$(CONFIG_EFI_STUB)			+= libstub/
+obj-$(CONFIG_EFI_PARAMS_FROM_FDT)	+= efi-fdt.o
diff --git a/drivers/firmware/efi/efi-fdt.c b/drivers/firmware/efi/efi-fdt.c
new file mode 100644
index 000000000000..f0e7ef2ae0e9
--- /dev/null
+++ b/drivers/firmware/efi/efi-fdt.c
@@ -0,0 +1,93 @@
+/*
+ * Copyright (C) 2013 - 2015 Linaro Ltd.
+ *
+ * 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/init.h>
+#include <linux/efi.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+
+#define UEFI_PARAM(name, prop, field)			   \
+	{						   \
+		{ name },				   \
+		{ prop },				   \
+		offsetof(struct efi_fdt_params, field),    \
+		FIELD_SIZEOF(struct efi_fdt_params, field) \
+	}
+
+static __initdata struct {
+	const char name[32];
+	const char propname[32];
+	int offset;
+	int size;
+} dt_params[] = {
+	UEFI_PARAM("System Table", "linux,uefi-system-table", system_table),
+	UEFI_PARAM("MemMap Address", "linux,uefi-mmap-start", mmap),
+	UEFI_PARAM("MemMap Size", "linux,uefi-mmap-size", mmap_size),
+	UEFI_PARAM("MemMap Desc. Size", "linux,uefi-mmap-desc-size", desc_size),
+	UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
+};
+
+struct param_info {
+	int verbose;
+	int found;
+	void *params;
+};
+
+static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
+				       int depth, void *data)
+{
+	struct param_info *info = data;
+	const void *prop;
+	void *dest;
+	u64 val;
+	int i, len;
+
+	if (depth != 1 || strcmp(uname, "chosen") != 0)
+		return 0;
+
+	for (i = 0; i < ARRAY_SIZE(dt_params); i++) {
+		prop = of_get_flat_dt_prop(node, dt_params[i].propname, &len);
+		if (!prop)
+			return 0;
+		dest = info->params + dt_params[i].offset;
+		info->found++;
+
+		val = of_read_number(prop, len / sizeof(u32));
+
+		if (dt_params[i].size == sizeof(u32))
+			*(u32 *)dest = val;
+		else
+			*(u64 *)dest = val;
+
+		if (info->verbose)
+			pr_info("  %s: 0x%0*llx\n", dt_params[i].name,
+				dt_params[i].size * 2, val);
+	}
+	return 1;
+}
+
+int __init efi_get_fdt_params(struct efi_fdt_params *params, int verbose)
+{
+	struct param_info info;
+	int ret;
+
+	pr_info("Getting EFI parameters from FDT:\n");
+
+	info.verbose = verbose;
+	info.found = 0;
+	info.params = params;
+
+	ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
+	if (!info.found)
+		pr_info("UEFI not found.\n");
+	else if (!ret)
+		pr_err("Can't find '%s' in device tree!\n",
+		       dt_params[info.found].name);
+
+	return ret;
+}
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 9fa8084a7c8d..f975a1a45b80 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -20,8 +20,6 @@
 #include <linux/init.h>
 #include <linux/device.h>
 #include <linux/efi.h>
-#include <linux/of.h>
-#include <linux/of_fdt.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
 
@@ -460,90 +458,6 @@ static int __init efi_load_efivars(void)
 device_initcall(efi_load_efivars);
 #endif
 
-#ifdef CONFIG_EFI_PARAMS_FROM_FDT
-
-#define UEFI_PARAM(name, prop, field)			   \
-	{						   \
-		{ name },				   \
-		{ prop },				   \
-		offsetof(struct efi_fdt_params, field),    \
-		FIELD_SIZEOF(struct efi_fdt_params, field) \
-	}
-
-static __initdata struct {
-	const char name[32];
-	const char propname[32];
-	int offset;
-	int size;
-} dt_params[] = {
-	UEFI_PARAM("System Table", "linux,uefi-system-table", system_table),
-	UEFI_PARAM("MemMap Address", "linux,uefi-mmap-start", mmap),
-	UEFI_PARAM("MemMap Size", "linux,uefi-mmap-size", mmap_size),
-	UEFI_PARAM("MemMap Desc. Size", "linux,uefi-mmap-desc-size", desc_size),
-	UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
-};
-
-struct param_info {
-	int verbose;
-	int found;
-	void *params;
-};
-
-static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
-				       int depth, void *data)
-{
-	struct param_info *info = data;
-	const void *prop;
-	void *dest;
-	u64 val;
-	int i, len;
-
-	if (depth != 1 || strcmp(uname, "chosen") != 0)
-		return 0;
-
-	for (i = 0; i < ARRAY_SIZE(dt_params); i++) {
-		prop = of_get_flat_dt_prop(node, dt_params[i].propname, &len);
-		if (!prop)
-			return 0;
-		dest = info->params + dt_params[i].offset;
-		info->found++;
-
-		val = of_read_number(prop, len / sizeof(u32));
-
-		if (dt_params[i].size == sizeof(u32))
-			*(u32 *)dest = val;
-		else
-			*(u64 *)dest = val;
-
-		if (info->verbose)
-			pr_info("  %s: 0x%0*llx\n", dt_params[i].name,
-				dt_params[i].size * 2, val);
-	}
-	return 1;
-}
-
-int __init efi_get_fdt_params(struct efi_fdt_params *params, int verbose)
-{
-	struct param_info info;
-	int ret;
-
-	pr_info("Getting EFI parameters from FDT:\n");
-
-	info.verbose = verbose;
-	info.found = 0;
-	info.params = params;
-
-	ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
-	if (!info.found)
-		pr_info("UEFI not found.\n");
-	else if (!ret)
-		pr_err("Can't find '%s' in device tree!\n",
-		       dt_params[info.found].name);
-
-	return ret;
-}
-#endif /* CONFIG_EFI_PARAMS_FROM_FDT */
-
 static __initdata char memory_type_name[][20] = {
 	"Reserved",
 	"Loader Code",
-- 
1.9.1

[PATCH v2 3/5] arm64/efi: move EFI init before early FDT processing

[Ard Biesheuvel]

The early FDT processing is responsible for enumerating the
DT memory nodes and installing them as memblocks. This should
only be done if we are not booting via EFI, but at this point,
we don't know yet if that is the case or not.

So move the EFI init to before the early FDT processing. This involves
making some changes to the way EFI discovers the locations of the
EFI system table and the memory map, since those values are retrieved
from the FDT as well. Instead the of_scan infrastructure, it now uses
libfdt directly to access the /chosen node.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/include/asm/efi.h   |  4 +-
 arch/arm64/kernel/efi.c        | 14 ++--
 arch/arm64/kernel/setup.c      |  4 +-
 drivers/firmware/efi/efi-fdt.c | 74 ++++++++------------
 include/linux/efi.h            |  3 +-
 5 files changed, 40 insertions(+), 59 deletions(-)

diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index ef572206f1c3..635101f36720 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -5,9 +5,9 @@
 #include <asm/neon.h>
 
 #ifdef CONFIG_EFI
-extern void efi_init(void);
+extern void efi_init_fdt(void *fdt);
 #else
-#define efi_init()
+#define efi_init_fdt(x)
 #endif
 
 #define efi_call_virt(f, ...)						\
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 9d4aa18f2a82..0e3dbe2cb752 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -51,14 +51,7 @@ static struct mm_struct efi_mm = {
 	INIT_MM_CONTEXT(efi_mm)
 };
 
-static int uefi_debug __initdata;
-static int __init uefi_debug_setup(char *str)
-{
-	uefi_debug = 1;
-
-	return 0;
-}
-early_param("uefi_debug", uefi_debug_setup);
+static bool uefi_debug __initdata;
 
 static int __init is_normal_ram(efi_memory_desc_t *md)
 {
@@ -205,14 +198,15 @@ static __init void reserve_regions(void)
 	set_bit(EFI_MEMMAP, &efi.flags);
 }
 
-void __init efi_init(void)
+void __init efi_init_fdt(void *fdt)
 {
 	struct efi_fdt_params params;
 
 	/* Grab UEFI information placed in FDT by stub */
-	if (!efi_get_fdt_params(&params, uefi_debug))
+	if (!efi_get_fdt_params(fdt, &params))
 		return;
 
+	uefi_debug = params.verbose;
 	efi_system_table = params.system_table;
 
 	memblock_reserve(params.mmap & PAGE_MASK,
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index fddae2c15ad2..8fdde97c975c 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -298,6 +298,9 @@ static void __init setup_machine_fdt(phys_addr_t dt_phys)
 {
 	void *dt_virt = fixmap_remap_fdt(dt_phys);
 
+	if (dt_virt)
+		efi_init_fdt(dt_virt);
+
 	if (!dt_virt || !early_init_dt_scan(dt_virt)) {
 		pr_crit("\n"
 			"Error: invalid device tree blob at physical address %pa (virtual address 0x%p)\n"
@@ -366,7 +369,6 @@ void __init setup_arch(char **cmdline_p)
 	 */
 	local_async_enable();
 
-	efi_init();
 	arm64_memblock_init();
 
 	/* Parse the ACPI tables for possible boot-time configuration */
diff --git a/drivers/firmware/efi/efi-fdt.c b/drivers/firmware/efi/efi-fdt.c
index f0e7ef2ae0e9..2e0e1a5a3fbb 100644
--- a/drivers/firmware/efi/efi-fdt.c
+++ b/drivers/firmware/efi/efi-fdt.c
@@ -8,8 +8,7 @@
 
 #include <linux/init.h>
 #include <linux/efi.h>
-#include <linux/of.h>
-#include <linux/of_fdt.h>
+#include <linux/libfdt.h>
 
 #define UEFI_PARAM(name, prop, field)			   \
 	{						   \
@@ -32,62 +31,47 @@ static __initdata struct {
 	UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
 };
 
-struct param_info {
-	int verbose;
-	int found;
-	void *params;
-};
-
-static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
-				       int depth, void *data)
+bool __init efi_get_fdt_params(void *fdt, struct efi_fdt_params *params)
 {
-	struct param_info *info = data;
 	const void *prop;
-	void *dest;
-	u64 val;
-	int i, len;
+	int node, i;
+
+	pr_info("Getting EFI parameters from FDT:\n");
 
-	if (depth != 1 || strcmp(uname, "chosen") != 0)
-		return 0;
+	node = fdt_path_offset(fdt, "/chosen");
+	if (node < 0) {
+		pr_err("/chosen node not found!\n");
+		return false;
+	}
+
+	prop = fdt_getprop(fdt, node, "bootargs", NULL);
+	params->verbose = prop && strstr(prop, "uefi_debug");
 
 	for (i = 0; i < ARRAY_SIZE(dt_params); i++) {
-		prop = of_get_flat_dt_prop(node, dt_params[i].propname, &len);
-		if (!prop)
-			return 0;
-		dest = info->params + dt_params[i].offset;
-		info->found++;
+		void *dest;
+		int len;
+		u64 val;
 
-		val = of_read_number(prop, len / sizeof(u32));
+		prop = fdt_getprop(fdt, node, dt_params[i].propname, &len);
+		if (!prop)
+			goto not_found;
+		dest = (void *)params + dt_params[i].offset;
 
 		if (dt_params[i].size == sizeof(u32))
-			*(u32 *)dest = val;
+			val = *(u32 *)dest = be32_to_cpup(prop);
 		else
-			*(u64 *)dest = val;
+			val = *(u64 *)dest = be64_to_cpup(prop);
 
-		if (info->verbose)
+		if (params->verbose)
 			pr_info("  %s: 0x%0*llx\n", dt_params[i].name,
 				dt_params[i].size * 2, val);
 	}
-	return 1;
-}
-
-int __init efi_get_fdt_params(struct efi_fdt_params *params, int verbose)
-{
-	struct param_info info;
-	int ret;
+	return true;
 
-	pr_info("Getting EFI parameters from FDT:\n");
-
-	info.verbose = verbose;
-	info.found = 0;
-	info.params = params;
-
-	ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
-	if (!info.found)
+not_found:
+	if (i == 0)
 		pr_info("UEFI not found.\n");
-	else if (!ret)
-		pr_err("Can't find '%s' in device tree!\n",
-		       dt_params[info.found].name);
-
-	return ret;
+	else
+		pr_err("Can't find '%s' in device tree!\n", dt_params[i].name);
+	return false;
 }
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 85ef051ac6fb..faafa1ad6ea7 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -690,6 +690,7 @@ struct efi_fdt_params {
 	u32 mmap_size;
 	u32 desc_size;
 	u32 desc_ver;
+	bool verbose;
 };
 
 typedef struct {
@@ -901,7 +902,7 @@ extern void efi_initialize_iomem_resources(struct resource *code_resource,
 		struct resource *data_resource, struct resource *bss_resource);
 extern void efi_get_time(struct timespec *now);
 extern void efi_reserve_boot_services(void);
-extern int efi_get_fdt_params(struct efi_fdt_params *params, int verbose);
+extern bool efi_get_fdt_params(void *fdt, struct efi_fdt_params *params);
 extern struct efi_memory_map memmap;
 extern struct kobject *efi_kobj;
 
-- 
1.9.1

[PATCH v2 4/5] arm64/efi: ignore DT memory nodes instead of removing them

[Ard Biesheuvel]

There are two problems with the UEFI stub DT memory node removal
routine:
- it deletes nodes as it traverses the tree, which happens to work
  but is not supported, as deletion invalidates the node iterator;
- deleting memory nodes entirely may discard annotations in the form
  of additional properties on the nodes.

Now that the UEFI initialization has moved to an earlier stage, we can
actually just ignore any memblocks that are installed after we have
processed the UEFI memory map. This way, it is no longer necessary to
remove the nodes, so we can remove that logic from the stub as well.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/mm/init.c               | 10 ++++++++
 drivers/firmware/efi/libstub/fdt.c | 24 +-------------------
 2 files changed, 11 insertions(+), 23 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index ab25fde7397c..ecbc051bc66b 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -379,6 +379,16 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)
 {
 	const u64 phys_offset = __pa(PAGE_OFFSET);
 
+	/*
+	 * This callback will be invoked both when booting via UEFI and when
+	 * booting via DT only. In the former case, we need to ignore memory
+	 * nodes in the DT since UEFI is authoritative when it comes to the
+	 * memory map. So ignore any invocations of this callback after
+	 * EFI_MEMMAP has been set.
+	 */
+	if (efi_enabled(EFI_MEMMAP))
+		return;
+
 	if (!PAGE_ALIGNED(base)) {
 		if (size < PAGE_SIZE - (base & ~PAGE_MASK)) {
 			pr_warn("Ignoring memory block 0x%llx - 0x%llx\n",
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index ef5d764e2a27..343e7992bd8f 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -24,7 +24,7 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
 			unsigned long map_size, unsigned long desc_size,
 			u32 desc_ver)
 {
-	int node, prev, num_rsv;
+	int node, num_rsv;
 	int status;
 	u32 fdt_val32;
 	u64 fdt_val64;
@@ -54,28 +54,6 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
 		goto fdt_set_fail;
 
 	/*
-	 * Delete any memory nodes present. We must delete nodes which
-	 * early_init_dt_scan_memory may try to use.
-	 */
-	prev = 0;
-	for (;;) {
-		const char *type;
-		int len;
-
-		node = fdt_next_node(fdt, prev, NULL);
-		if (node < 0)
-			break;
-
-		type = fdt_getprop(fdt, node, "device_type", &len);
-		if (type && strncmp(type, "memory", len) == 0) {
-			fdt_del_node(fdt, node);
-			continue;
-		}
-
-		prev = node;
-	}
-
-	/*
 	 * Delete all memory reserve map entries. When booting via UEFI,
 	 * kernel will use the UEFI memory map to find reserved regions.
 	 */
-- 
1.9.1

[PATCH v2 5/5] arm64/efi: ignore DT memreserve entries instead of removing them

[Ard Biesheuvel]

Now that the reservation of the FDT image itself is split off, we
can make the DT scanning of memreserves conditional on whether we
booted via UEFI and have its memory map available. This allows us
to drop deletion of these memreserves in the stub. It also fixes
the issue where the /reserved-memory/ node (which offers another
way of reserving memory ranges) was not being ignored under UEFI.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/mm/init.c               |  3 ++-
 drivers/firmware/efi/libstub/fdt.c | 11 +----------
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index ecbc051bc66b..0c33670ed1b6 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -170,7 +170,8 @@ void __init arm64_memblock_init(void)
 		memblock_reserve(__virt_to_phys(initrd_start), initrd_end - initrd_start);
 #endif
 
-	early_init_fdt_scan_reserved_mem();
+	if (!efi_enabled(EFI_MEMMAP))
+		early_init_fdt_scan_reserved_mem();
 
 	/* 4GB maximum for 32-bit only capable devices */
 	if (IS_ENABLED(CONFIG_ZONE_DMA))
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index 343e7992bd8f..a7e87cd582f2 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -24,8 +24,7 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
 			unsigned long map_size, unsigned long desc_size,
 			u32 desc_ver)
 {
-	int node, num_rsv;
-	int status;
+	int node, status;
 	u32 fdt_val32;
 	u64 fdt_val64;
 
@@ -53,14 +52,6 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
 	if (status != 0)
 		goto fdt_set_fail;
 
-	/*
-	 * Delete all memory reserve map entries. When booting via UEFI,
-	 * kernel will use the UEFI memory map to find reserved regions.
-	 */
-	num_rsv = fdt_num_mem_rsv(fdt);
-	while (num_rsv-- > 0)
-		fdt_del_mem_rsv(fdt, num_rsv);
-
 	node = fdt_subnode_offset(fdt, 0, "chosen");
 	if (node < 0) {
 		node = fdt_add_subnode(fdt, 0, "chosen");
-- 
1.9.1

[PATCH v2 1/5] arm64: clone early_init_dt_add_memory_arch() to override default

[Leif Lindholm]

On Wed, Aug 26, 2015 at 10:06:27AM +0200, Ard Biesheuvel wrote:
> Override the __weak early_init_dt_add_memory_arch() with our own
> version. We need this in a subsequent patch to make the handling of
> the memory nodes conditional on whether we are booting via UEFI or
> not.

Worth clarifying that this is a direct copy?
("Our own version" can be read as there being modifications here.)

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm64/mm/init.c | 41 ++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index f5c0680d17d9..ab25fde7397c 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -374,3 +374,44 @@ static int __init keepinitrd_setup(char *__unused)
>  
>  __setup("keepinitrd", keepinitrd_setup);
>  #endif
> +
> +void __init early_init_dt_add_memory_arch(u64 base, u64 size)
> +{
> +	const u64 phys_offset = __pa(PAGE_OFFSET);
> +
> +	if (!PAGE_ALIGNED(base)) {
> +		if (size < PAGE_SIZE - (base & ~PAGE_MASK)) {
> +			pr_warn("Ignoring memory block 0x%llx - 0x%llx\n",
> +				base, base + size);
> +			return;
> +		}
> +		size -= PAGE_SIZE - (base & ~PAGE_MASK);
> +		base = PAGE_ALIGN(base);
> +	}
> +	size &= PAGE_MASK;
> +
> +	if (base > MAX_MEMBLOCK_ADDR) {
> +		pr_warning("Ignoring memory block 0x%llx - 0x%llx\n",
> +				base, base + size);
> +		return;
> +	}
> +
> +	if (base + size - 1 > MAX_MEMBLOCK_ADDR) {
> +		pr_warning("Ignoring memory range 0x%llx - 0x%llx\n",
> +				((u64)MAX_MEMBLOCK_ADDR) + 1, base + size);
> +		size = MAX_MEMBLOCK_ADDR - base + 1;
> +	}
> +
> +	if (base + size < phys_offset) {
> +		pr_warning("Ignoring memory block 0x%llx - 0x%llx\n",
> +			   base, base + size);
> +		return;
> +	}
> +	if (base < phys_offset) {
> +		pr_warning("Ignoring memory range 0x%llx - 0x%llx\n",
> +			   base, phys_offset);
> +		size -= phys_offset - base;
> +		base = phys_offset;
> +	}
> +	memblock_add(base, size);
> +}
> -- 
> 1.9.1

Would it be too crazy to do this via an added call to a weak
memblock_use_dt() function in the original instead, to avoid
duplicating this entire function?

We need the functionality for 4/5 (nice cleanup) and 5/5 (bugfix), so
happy with whatever works.

/
    Leif

[PATCH v2 2/5] efi: move FDT handling to separate object file

[Leif Lindholm]

On Wed, Aug 26, 2015 at 10:06:28AM +0200, Ard Biesheuvel wrote:
> The EFI specific FDT handling is compiled conditionally, and is
> logically independent of the rest of efi.o. So move it to a separate
> file before making changes to it in subsequent patches.
> 
> Acked-by: Matt Fleming <matt.fleming@intel.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  drivers/firmware/efi/Makefile  |  1 +
>  drivers/firmware/efi/efi-fdt.c | 93 ++++++++++++++++++++
>  drivers/firmware/efi/efi.c     | 86 ------------------
>  3 files changed, 94 insertions(+), 86 deletions(-)

Acked-by: Leif Lindholm <leif.lindholm@linaro.org>

> diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
> index 6fd3da938717..d9140208fc1c 100644
> --- a/drivers/firmware/efi/Makefile
> +++ b/drivers/firmware/efi/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_UEFI_CPER)			+= cper.o
>  obj-$(CONFIG_EFI_RUNTIME_MAP)		+= runtime-map.o
>  obj-$(CONFIG_EFI_RUNTIME_WRAPPERS)	+= runtime-wrappers.o
>  obj-$(CONFIG_EFI_STUB)			+= libstub/
> +obj-$(CONFIG_EFI_PARAMS_FROM_FDT)	+= efi-fdt.o
> diff --git a/drivers/firmware/efi/efi-fdt.c b/drivers/firmware/efi/efi-fdt.c
> new file mode 100644
> index 000000000000..f0e7ef2ae0e9
> --- /dev/null
> +++ b/drivers/firmware/efi/efi-fdt.c
> @@ -0,0 +1,93 @@
> +/*
> + * Copyright (C) 2013 - 2015 Linaro Ltd.
> + *
> + * 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/init.h>
> +#include <linux/efi.h>
> +#include <linux/of.h>
> +#include <linux/of_fdt.h>
> +
> +#define UEFI_PARAM(name, prop, field)			   \
> +	{						   \
> +		{ name },				   \
> +		{ prop },				   \
> +		offsetof(struct efi_fdt_params, field),    \
> +		FIELD_SIZEOF(struct efi_fdt_params, field) \
> +	}
> +
> +static __initdata struct {
> +	const char name[32];
> +	const char propname[32];
> +	int offset;
> +	int size;
> +} dt_params[] = {
> +	UEFI_PARAM("System Table", "linux,uefi-system-table", system_table),
> +	UEFI_PARAM("MemMap Address", "linux,uefi-mmap-start", mmap),
> +	UEFI_PARAM("MemMap Size", "linux,uefi-mmap-size", mmap_size),
> +	UEFI_PARAM("MemMap Desc. Size", "linux,uefi-mmap-desc-size", desc_size),
> +	UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
> +};
> +
> +struct param_info {
> +	int verbose;
> +	int found;
> +	void *params;
> +};
> +
> +static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
> +				       int depth, void *data)
> +{
> +	struct param_info *info = data;
> +	const void *prop;
> +	void *dest;
> +	u64 val;
> +	int i, len;
> +
> +	if (depth != 1 || strcmp(uname, "chosen") != 0)
> +		return 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(dt_params); i++) {
> +		prop = of_get_flat_dt_prop(node, dt_params[i].propname, &len);
> +		if (!prop)
> +			return 0;
> +		dest = info->params + dt_params[i].offset;
> +		info->found++;
> +
> +		val = of_read_number(prop, len / sizeof(u32));
> +
> +		if (dt_params[i].size == sizeof(u32))
> +			*(u32 *)dest = val;
> +		else
> +			*(u64 *)dest = val;
> +
> +		if (info->verbose)
> +			pr_info("  %s: 0x%0*llx\n", dt_params[i].name,
> +				dt_params[i].size * 2, val);
> +	}
> +	return 1;
> +}
> +
> +int __init efi_get_fdt_params(struct efi_fdt_params *params, int verbose)
> +{
> +	struct param_info info;
> +	int ret;
> +
> +	pr_info("Getting EFI parameters from FDT:\n");
> +
> +	info.verbose = verbose;
> +	info.found = 0;
> +	info.params = params;
> +
> +	ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
> +	if (!info.found)
> +		pr_info("UEFI not found.\n");
> +	else if (!ret)
> +		pr_err("Can't find '%s' in device tree!\n",
> +		       dt_params[info.found].name);
> +
> +	return ret;
> +}
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 9fa8084a7c8d..f975a1a45b80 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -20,8 +20,6 @@
>  #include <linux/init.h>
>  #include <linux/device.h>
>  #include <linux/efi.h>
> -#include <linux/of.h>
> -#include <linux/of_fdt.h>
>  #include <linux/io.h>
>  #include <linux/platform_device.h>
>  
> @@ -460,90 +458,6 @@ static int __init efi_load_efivars(void)
>  device_initcall(efi_load_efivars);
>  #endif
>  
> -#ifdef CONFIG_EFI_PARAMS_FROM_FDT
> -
> -#define UEFI_PARAM(name, prop, field)			   \
> -	{						   \
> -		{ name },				   \
> -		{ prop },				   \
> -		offsetof(struct efi_fdt_params, field),    \
> -		FIELD_SIZEOF(struct efi_fdt_params, field) \
> -	}
> -
> -static __initdata struct {
> -	const char name[32];
> -	const char propname[32];
> -	int offset;
> -	int size;
> -} dt_params[] = {
> -	UEFI_PARAM("System Table", "linux,uefi-system-table", system_table),
> -	UEFI_PARAM("MemMap Address", "linux,uefi-mmap-start", mmap),
> -	UEFI_PARAM("MemMap Size", "linux,uefi-mmap-size", mmap_size),
> -	UEFI_PARAM("MemMap Desc. Size", "linux,uefi-mmap-desc-size", desc_size),
> -	UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
> -};
> -
> -struct param_info {
> -	int verbose;
> -	int found;
> -	void *params;
> -};
> -
> -static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
> -				       int depth, void *data)
> -{
> -	struct param_info *info = data;
> -	const void *prop;
> -	void *dest;
> -	u64 val;
> -	int i, len;
> -
> -	if (depth != 1 || strcmp(uname, "chosen") != 0)
> -		return 0;
> -
> -	for (i = 0; i < ARRAY_SIZE(dt_params); i++) {
> -		prop = of_get_flat_dt_prop(node, dt_params[i].propname, &len);
> -		if (!prop)
> -			return 0;
> -		dest = info->params + dt_params[i].offset;
> -		info->found++;
> -
> -		val = of_read_number(prop, len / sizeof(u32));
> -
> -		if (dt_params[i].size == sizeof(u32))
> -			*(u32 *)dest = val;
> -		else
> -			*(u64 *)dest = val;
> -
> -		if (info->verbose)
> -			pr_info("  %s: 0x%0*llx\n", dt_params[i].name,
> -				dt_params[i].size * 2, val);
> -	}
> -	return 1;
> -}
> -
> -int __init efi_get_fdt_params(struct efi_fdt_params *params, int verbose)
> -{
> -	struct param_info info;
> -	int ret;
> -
> -	pr_info("Getting EFI parameters from FDT:\n");
> -
> -	info.verbose = verbose;
> -	info.found = 0;
> -	info.params = params;
> -
> -	ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
> -	if (!info.found)
> -		pr_info("UEFI not found.\n");
> -	else if (!ret)
> -		pr_err("Can't find '%s' in device tree!\n",
> -		       dt_params[info.found].name);
> -
> -	return ret;
> -}
> -#endif /* CONFIG_EFI_PARAMS_FROM_FDT */
> -
>  static __initdata char memory_type_name[][20] = {
>  	"Reserved",
>  	"Loader Code",
> -- 
> 1.9.1

[PATCH v2 1/5] arm64: clone early_init_dt_add_memory_arch() to override default

[Ard Biesheuvel]

On 26 August 2015 at 16:43, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Wed, Aug 26, 2015 at 10:06:27AM +0200, Ard Biesheuvel wrote:
>> Override the __weak early_init_dt_add_memory_arch() with our own
>> version. We need this in a subsequent patch to make the handling of
>> the memory nodes conditional on whether we are booting via UEFI or
>> not.
>
> Worth clarifying that this is a direct copy?
> ("Our own version" can be read as there being modifications here.)
>

Yes. In fact, it wasn't before, so hence the choice of words here.

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  arch/arm64/mm/init.c | 41 ++++++++++++++++++++
>>  1 file changed, 41 insertions(+)
>>
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index f5c0680d17d9..ab25fde7397c 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -374,3 +374,44 @@ static int __init keepinitrd_setup(char *__unused)
>>
>>  __setup("keepinitrd", keepinitrd_setup);
>>  #endif
>> +
>> +void __init early_init_dt_add_memory_arch(u64 base, u64 size)
>> +{
>> +     const u64 phys_offset = __pa(PAGE_OFFSET);
>> +
>> +     if (!PAGE_ALIGNED(base)) {
>> +             if (size < PAGE_SIZE - (base & ~PAGE_MASK)) {
>> +                     pr_warn("Ignoring memory block 0x%llx - 0x%llx\n",
>> +                             base, base + size);
>> +                     return;
>> +             }
>> +             size -= PAGE_SIZE - (base & ~PAGE_MASK);
>> +             base = PAGE_ALIGN(base);
>> +     }
>> +     size &= PAGE_MASK;
>> +
>> +     if (base > MAX_MEMBLOCK_ADDR) {
>> +             pr_warning("Ignoring memory block 0x%llx - 0x%llx\n",
>> +                             base, base + size);
>> +             return;
>> +     }
>> +
>> +     if (base + size - 1 > MAX_MEMBLOCK_ADDR) {
>> +             pr_warning("Ignoring memory range 0x%llx - 0x%llx\n",
>> +                             ((u64)MAX_MEMBLOCK_ADDR) + 1, base + size);
>> +             size = MAX_MEMBLOCK_ADDR - base + 1;
>> +     }
>> +
>> +     if (base + size < phys_offset) {
>> +             pr_warning("Ignoring memory block 0x%llx - 0x%llx\n",
>> +                        base, base + size);
>> +             return;
>> +     }
>> +     if (base < phys_offset) {
>> +             pr_warning("Ignoring memory range 0x%llx - 0x%llx\n",
>> +                        base, phys_offset);
>> +             size -= phys_offset - base;
>> +             base = phys_offset;
>> +     }
>> +     memblock_add(base, size);
>> +}
>> --
>> 1.9.1
>
> Would it be too crazy to do this via an added call to a weak
> memblock_use_dt() function in the original instead, to avoid
> duplicating this entire function?
>

This keeps coming up: I have a series in the freezer that decouples
the kernel mapping from the linear mapping, which makes another round
of changes to this function. So that is why I cloned the function in
the first place, only to reuse this patch for this series as well.

The following does the trick as well, but I am not sure if the alias
attribute is allowed in the kernel

"""
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index ad87ce826cce..af2f30cbf357 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -374,3 +374,8

 __setup("keepinitrd", keepinitrd_setup);
 #endif
+
+void __init early_init_dt_add_memory_arch(u64 base, u64 size)
+{
+       __early_init_dt_add_memory_arch(base, size);
+}
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 07496560e5b9..2da75619ed78 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -1047,6 +1047,9
 }
 #endif

+void __early_init_dt_add_memory_arch(u64 base, u64 size)
+                       __attribute__((alias("early_init_dt_add_memory_arch")));
+
 bool __init early_init_dt_verify(void *params)
 {
        if (!params)
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index df9ef3801812..64fcb143f5f4 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -66,6 +66,7
 extern void early_init_fdt_scan_reserved_mem(void);
 extern void early_init_fdt_reserve_self(void);
 extern void early_init_dt_add_memory_arch(u64 base, u64 size);
+extern void __early_init_dt_add_memory_arch(u64 base, u64 size);
 extern int early_init_dt_reserve_memory_arch(phys_addr_t base,
phys_addr_t size,
                                             bool no_map);
 extern void * early_init_dt_alloc_memory_arch(u64 size, u64 align);
"""

> We need the functionality for 4/5 (nice cleanup) and 5/5 (bugfix), so
> happy with whatever works.
>

Indeed.

[PATCH v2 3/5] arm64/efi: move EFI init before early FDT processing

[Leif Lindholm]

On Wed, Aug 26, 2015 at 10:06:29AM +0200, Ard Biesheuvel wrote:
> The early FDT processing is responsible for enumerating the
> DT memory nodes and installing them as memblocks. This should
> only be done if we are not booting via EFI, but at this point,
> we don't know yet if that is the case or not.
> 
> So move the EFI init to before the early FDT processing. This involves
> making some changes to the way EFI discovers the locations of the
> EFI system table and the memory map, since those values are retrieved
> from the FDT as well. Instead the of_scan infrastructure, it now uses
> libfdt directly to access the /chosen node.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm64/include/asm/efi.h   |  4 +-
>  arch/arm64/kernel/efi.c        | 14 ++--
>  arch/arm64/kernel/setup.c      |  4 +-
>  drivers/firmware/efi/efi-fdt.c | 74 ++++++++------------
>  include/linux/efi.h            |  3 +-
>  5 files changed, 40 insertions(+), 59 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> index ef572206f1c3..635101f36720 100644
> --- a/arch/arm64/include/asm/efi.h
> +++ b/arch/arm64/include/asm/efi.h
> @@ -5,9 +5,9 @@
>  #include <asm/neon.h>
>  
>  #ifdef CONFIG_EFI
> -extern void efi_init(void);
> +extern void efi_init_fdt(void *fdt);
>  #else
> -#define efi_init()
> +#define efi_init_fdt(x)
>  #endif
>  
>  #define efi_call_virt(f, ...)						\
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 9d4aa18f2a82..0e3dbe2cb752 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -51,14 +51,7 @@ static struct mm_struct efi_mm = {
>  	INIT_MM_CONTEXT(efi_mm)
>  };
>  
> -static int uefi_debug __initdata;
> -static int __init uefi_debug_setup(char *str)
> -{
> -	uefi_debug = 1;
> -
> -	return 0;
> -}
> -early_param("uefi_debug", uefi_debug_setup);
> +static bool uefi_debug __initdata;

So, this obviously clashes with the patches I sent out recently, but
then they were triggered by the awkwardness of the interface passing
the verbosity flag around...
  
>  static int __init is_normal_ram(efi_memory_desc_t *md)
>  {
> @@ -205,14 +198,15 @@ static __init void reserve_regions(void)
>  	set_bit(EFI_MEMMAP, &efi.flags);
>  }
>  
> -void __init efi_init(void)
> +void __init efi_init_fdt(void *fdt)

(Nitpick: efi_init_fdt here, efi_virtmap_init below - naming consistency)

>  {
>  	struct efi_fdt_params params;
>  
>  	/* Grab UEFI information placed in FDT by stub */
> -	if (!efi_get_fdt_params(&params, uefi_debug))
> +	if (!efi_get_fdt_params(fdt, &params))
>  		return;
>  
> +	uefi_debug = params.verbose;
>  	efi_system_table = params.system_table;

Here is where it gets problematic.
Because this is now called before parse_early_params(), we lose the
ability to do debug output. Now, the details in the DT parsing is
maybe not that critical, but what follow is:
  
>  	memblock_reserve(params.mmap & PAGE_MASK,

... and we _really_ need to be able to dump the memory regions, with
attributes, for debugging purposes.

> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index fddae2c15ad2..8fdde97c975c 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -298,6 +298,9 @@ static void __init setup_machine_fdt(phys_addr_t dt_phys)
>  {
>  	void *dt_virt = fixmap_remap_fdt(dt_phys);
>  
> +	if (dt_virt)
> +		efi_init_fdt(dt_virt);
> +
>  	if (!dt_virt || !early_init_dt_scan(dt_virt)) {
>  		pr_crit("\n"
>  			"Error: invalid device tree blob at physical address %pa (virtual address 0x%p)\n"
> @@ -366,7 +369,6 @@ void __init setup_arch(char **cmdline_p)
>  	 */
>  	local_async_enable();
>  
> -	efi_init();

So can we just keep this call and terminate efi_init_fdt before
memblock_reserve?

>  	arm64_memblock_init();
>  
>  	/* Parse the ACPI tables for possible boot-time configuration */
> diff --git a/drivers/firmware/efi/efi-fdt.c b/drivers/firmware/efi/efi-fdt.c
> index f0e7ef2ae0e9..2e0e1a5a3fbb 100644
> --- a/drivers/firmware/efi/efi-fdt.c
> +++ b/drivers/firmware/efi/efi-fdt.c
> @@ -8,8 +8,7 @@
>  
>  #include <linux/init.h>
>  #include <linux/efi.h>
> -#include <linux/of.h>
> -#include <linux/of_fdt.h>
> +#include <linux/libfdt.h>
>  
>  #define UEFI_PARAM(name, prop, field)			   \
>  	{						   \
> @@ -32,62 +31,47 @@ static __initdata struct {
>  	UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
>  };
>  
> -struct param_info {
> -	int verbose;
> -	int found;
> -	void *params;
> -};
> -
> -static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
> -				       int depth, void *data)
> +bool __init efi_get_fdt_params(void *fdt, struct efi_fdt_params *params)
>  {
> -	struct param_info *info = data;
>  	const void *prop;
> -	void *dest;
> -	u64 val;
> -	int i, len;
> +	int node, i;
> +
> +	pr_info("Getting EFI parameters from FDT:\n");
>  
> -	if (depth != 1 || strcmp(uname, "chosen") != 0)
> -		return 0;
> +	node = fdt_path_offset(fdt, "/chosen");
> +	if (node < 0) {
> +		pr_err("/chosen node not found!\n");
> +		return false;
> +	}
> +
> +	prop = fdt_getprop(fdt, node, "bootargs", NULL);
> +	params->verbose = prop && strstr(prop, "uefi_debug");
>  
>  	for (i = 0; i < ARRAY_SIZE(dt_params); i++) {
> -		prop = of_get_flat_dt_prop(node, dt_params[i].propname, &len);
> -		if (!prop)
> -			return 0;
> -		dest = info->params + dt_params[i].offset;
> -		info->found++;
> +		void *dest;
> +		int len;
> +		u64 val;
>  
> -		val = of_read_number(prop, len / sizeof(u32));
> +		prop = fdt_getprop(fdt, node, dt_params[i].propname, &len);
> +		if (!prop)
> +			goto not_found;
> +		dest = (void *)params + dt_params[i].offset;
>  
>  		if (dt_params[i].size == sizeof(u32))
> -			*(u32 *)dest = val;
> +			val = *(u32 *)dest = be32_to_cpup(prop);
>  		else
> -			*(u64 *)dest = val;
> +			val = *(u64 *)dest = be64_to_cpup(prop);
>  
> -		if (info->verbose)
> +		if (params->verbose)
>  			pr_info("  %s: 0x%0*llx\n", dt_params[i].name,
>  				dt_params[i].size * 2, val);
>  	}
> -	return 1;
> -}
> -
> -int __init efi_get_fdt_params(struct efi_fdt_params *params, int verbose)
> -{
> -	struct param_info info;
> -	int ret;
> +	return true;
>  
> -	pr_info("Getting EFI parameters from FDT:\n");
> -
> -	info.verbose = verbose;
> -	info.found = 0;
> -	info.params = params;
> -
> -	ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
> -	if (!info.found)
> +not_found:
> +	if (i == 0)
>  		pr_info("UEFI not found.\n");
> -	else if (!ret)
> -		pr_err("Can't find '%s' in device tree!\n",
> -		       dt_params[info.found].name);
> -
> -	return ret;
> +	else
> +		pr_err("Can't find '%s' in device tree!\n", dt_params[i].name);
> +	return false;
>  }
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 85ef051ac6fb..faafa1ad6ea7 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -690,6 +690,7 @@ struct efi_fdt_params {
>  	u32 mmap_size;
>  	u32 desc_size;
>  	u32 desc_ver;
> +	bool verbose;
>  };
>  
>  typedef struct {
> @@ -901,7 +902,7 @@ extern void efi_initialize_iomem_resources(struct resource *code_resource,
>  		struct resource *data_resource, struct resource *bss_resource);
>  extern void efi_get_time(struct timespec *now);
>  extern void efi_reserve_boot_services(void);
> -extern int efi_get_fdt_params(struct efi_fdt_params *params, int verbose);
> +extern bool efi_get_fdt_params(void *fdt, struct efi_fdt_params *params);
>  extern struct efi_memory_map memmap;
>  extern struct kobject *efi_kobj;
>  
> -- 
> 1.9.1
> 

[PATCH v2 4/5] arm64/efi: ignore DT memory nodes instead of removing them

[Leif Lindholm]

On Wed, Aug 26, 2015 at 10:06:30AM +0200, Ard Biesheuvel wrote:
> There are two problems with the UEFI stub DT memory node removal
> routine:
> - it deletes nodes as it traverses the tree, which happens to work
>   but is not supported, as deletion invalidates the node iterator;
> - deleting memory nodes entirely may discard annotations in the form
>   of additional properties on the nodes.
> 
> Now that the UEFI initialization has moved to an earlier stage, we can
> actually just ignore any memblocks that are installed after we have
> processed the UEFI memory map. This way, it is no longer necessary to
> remove the nodes, so we can remove that logic from the stub as well.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm64/mm/init.c               | 10 ++++++++
>  drivers/firmware/efi/libstub/fdt.c | 24 +-------------------
>  2 files changed, 11 insertions(+), 23 deletions(-)

Nice cleanup.

Acked-by: Leif Lindholm <leif.lindholm@linaro.org>
 
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index ab25fde7397c..ecbc051bc66b 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -379,6 +379,16 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)
>  {
>  	const u64 phys_offset = __pa(PAGE_OFFSET);
>  
> +	/*
> +	 * This callback will be invoked both when booting via UEFI and when
> +	 * booting via DT only. In the former case, we need to ignore memory
> +	 * nodes in the DT since UEFI is authoritative when it comes to the
> +	 * memory map. So ignore any invocations of this callback after
> +	 * EFI_MEMMAP has been set.
> +	 */
> +	if (efi_enabled(EFI_MEMMAP))
> +		return;
> +
>  	if (!PAGE_ALIGNED(base)) {
>  		if (size < PAGE_SIZE - (base & ~PAGE_MASK)) {
>  			pr_warn("Ignoring memory block 0x%llx - 0x%llx\n",
> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> index ef5d764e2a27..343e7992bd8f 100644
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -24,7 +24,7 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
>  			unsigned long map_size, unsigned long desc_size,
>  			u32 desc_ver)
>  {
> -	int node, prev, num_rsv;
> +	int node, num_rsv;
>  	int status;
>  	u32 fdt_val32;
>  	u64 fdt_val64;
> @@ -54,28 +54,6 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
>  		goto fdt_set_fail;
>  
>  	/*
> -	 * Delete any memory nodes present. We must delete nodes which
> -	 * early_init_dt_scan_memory may try to use.
> -	 */
> -	prev = 0;
> -	for (;;) {
> -		const char *type;
> -		int len;
> -
> -		node = fdt_next_node(fdt, prev, NULL);
> -		if (node < 0)
> -			break;
> -
> -		type = fdt_getprop(fdt, node, "device_type", &len);
> -		if (type && strncmp(type, "memory", len) == 0) {
> -			fdt_del_node(fdt, node);
> -			continue;
> -		}
> -
> -		prev = node;
> -	}
> -
> -	/*
>  	 * Delete all memory reserve map entries. When booting via UEFI,
>  	 * kernel will use the UEFI memory map to find reserved regions.
>  	 */
> -- 
> 1.9.1
> 

[PATCH v2 5/5] arm64/efi: ignore DT memreserve entries instead of removing them

[Leif Lindholm]

On Wed, Aug 26, 2015 at 10:06:31AM +0200, Ard Biesheuvel wrote:
> Now that the reservation of the FDT image itself is split off, we
> can make the DT scanning of memreserves conditional on whether we
> booted via UEFI and have its memory map available. This allows us
> to drop deletion of these memreserves in the stub. It also fixes
> the issue where the /reserved-memory/ node (which offers another
> way of reserving memory ranges) was not being ignored under UEFI.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm64/mm/init.c               |  3 ++-
>  drivers/firmware/efi/libstub/fdt.c | 11 +----------
>  2 files changed, 3 insertions(+), 11 deletions(-)

Acked-by: Leif Lindholm <leif.lindholm@linaro.org>

And possibly a reference to 0ceac9e094b0?

As it is a bugfix, it would be nice to see this one go into 4.3, but...

> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index ecbc051bc66b..0c33670ed1b6 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -170,7 +170,8 @@ void __init arm64_memblock_init(void)
>  		memblock_reserve(__virt_to_phys(initrd_start), initrd_end - initrd_start);
>  #endif
>  
> -	early_init_fdt_scan_reserved_mem();
> +	if (!efi_enabled(EFI_MEMMAP))
> +		early_init_fdt_scan_reserved_mem();
>  
>  	/* 4GB maximum for 32-bit only capable devices */
>  	if (IS_ENABLED(CONFIG_ZONE_DMA))
> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> index 343e7992bd8f..a7e87cd582f2 100644
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -24,8 +24,7 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
>  			unsigned long map_size, unsigned long desc_size,
>  			u32 desc_ver)
>  {
> -	int node, num_rsv;
> -	int status;
> +	int node, status;
>  	u32 fdt_val32;
>  	u64 fdt_val64;
>  
> @@ -53,14 +52,6 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
>  	if (status != 0)
>  		goto fdt_set_fail;
>  
> -	/*
> -	 * Delete all memory reserve map entries. When booting via UEFI,
> -	 * kernel will use the UEFI memory map to find reserved regions.
> -	 */
> -	num_rsv = fdt_num_mem_rsv(fdt);
> -	while (num_rsv-- > 0)
> -		fdt_del_mem_rsv(fdt, num_rsv);
> -
>  	node = fdt_subnode_offset(fdt, 0, "chosen");
>  	if (node < 0) {
>  		node = fdt_add_subnode(fdt, 0, "chosen");
> -- 
> 1.9.1
>