[PATCH v3 0/6] arm64 UEFI early FDT handling

[PATCH v3 0/6] 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.

Patches #1 and #2 introduce 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 #3 moves some UEFI+FDT init code around before making changes to it.

Patch #4 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 #5 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 #6 does the same as #5, but for memreserves and the /reserved-memory
node.

Changes since v2:
- instead of copying the generic implementation, turn
  early_init_dt_add_memory_arch() into a weak alias so that it is still
  accessible to overrides

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 (6):
  of/fdt: make generic early_init_dt_add_memory_arch() a weak alias
  arm64: override generic version of early_init_dt_add_memory_arch()
  efi: move FDT handling to separate object file
  arm64/efi: move EFI /chosen node parsing 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       |  2 +
 arch/arm64/kernel/efi.c            | 26 ++++--
 arch/arm64/kernel/setup.c          |  3 +
 arch/arm64/mm/init.c               | 12 ++-
 drivers/firmware/efi/Makefile      |  1 +
 drivers/firmware/efi/efi-fdt.c     | 73 +++++++++++++++++
 drivers/firmware/efi/efi.c         | 84 --------------------
 drivers/firmware/efi/libstub/fdt.c | 33 +-------
 drivers/of/fdt.c                   |  7 +-
 include/linux/efi.h                |  2 +-
 include/linux/of_fdt.h             |  1 +
 11 files changed, 116 insertions(+), 128 deletions(-)
 create mode 100644 drivers/firmware/efi/efi-fdt.c

-- 
1.9.1

[PATCH v3 1/6] of/fdt: make generic early_init_dt_add_memory_arch() a weak alias

[Ard Biesheuvel]

The function early_init_dt_add_memory_arch() is defined as __weak
so that archs can override it. However, in this override implementation,
it may still be useful to invoke the generic implementation, so instead,
rename the generic version to early_init_dt_add_memory() and turn the
original definition into a weak alias. This way, the generic version can
still be called even in the presence of an arch specific override.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/of/fdt.c       | 7 +++++--
 include/linux/of_fdt.h | 1 +
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 07496560e5b9..8b758e10c9f8 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -969,7 +969,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
 #ifdef CONFIG_HAVE_MEMBLOCK
 #define MAX_PHYS_ADDR	((phys_addr_t)~0)
 
-void __init __weak early_init_dt_add_memory_arch(u64 base, u64 size)
+void __init early_init_dt_add_memory(u64 base, u64 size)
 {
 	const u64 phys_offset = __pa(PAGE_OFFSET);
 
@@ -1027,7 +1027,7 @@ void * __init __weak early_init_dt_alloc_memory_arch(u64 size, u64 align)
 	return __va(memblock_alloc(size, align));
 }
 #else
-void __init __weak early_init_dt_add_memory_arch(u64 base, u64 size)
+void __init early_init_dt_add_memory(u64 base, u64 size)
 {
 	WARN_ON(1);
 }
@@ -1047,6 +1047,9 @@ void * __init __weak early_init_dt_alloc_memory_arch(u64 size, u64 align)
 }
 #endif
 
+__weak __alias(early_init_dt_add_memory)
+void early_init_dt_add_memory_arch(u64 base, u64 size);
+
 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..463a06ba87ac 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -65,6 +65,7 @@ extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
 				     int depth, void *data);
 extern void early_init_fdt_scan_reserved_mem(void);
 extern void early_init_fdt_reserve_self(void);
+extern void early_init_dt_add_memory(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);
-- 
1.9.1

[PATCH v3 2/6] arm64: override generic version of early_init_dt_add_memory_arch()

[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. Note that for now, all that our version does is invoke the generic
implementation.

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

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index ad87ce826cce..d88e83ebf60d 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -374,3 +374,8 @@ static int __init keepinitrd_setup(char *__unused)
 
 __setup("keepinitrd", keepinitrd_setup);
 #endif
+
+void __init early_init_dt_add_memory_arch(u64 base, u64 size)
+{
+	early_init_dt_add_memory(base, size);
+}
-- 
1.9.1

[PATCH v3 3/6] 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>
Acked-by: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/Makefile  |  1 +
 drivers/firmware/efi/efi-fdt.c | 91 ++++++++++++++++++++
 drivers/firmware/efi/efi.c     | 84 ------------------
 3 files changed, 92 insertions(+), 84 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..8f3ce66e2b02
--- /dev/null
+++ b/drivers/firmware/efi/efi-fdt.c
@@ -0,0 +1,91 @@
+/*
+ * 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 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 (efi_enabled(EFI_DBG))
+			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)
+{
+	struct param_info info;
+	int ret;
+
+	pr_info("Getting EFI parameters from FDT:\n");
+
+	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 f38384b13254..3e1671ffd6ab 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>
 
@@ -468,88 +466,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 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 (efi_enabled(EFI_DBG))
-			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)
-{
-	struct param_info info;
-	int ret;
-
-	pr_info("Getting EFI parameters from FDT:\n");
-
-	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 v3 4/6] arm64/efi: move EFI /chosen node parsing 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 part of 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   |  2 +
 arch/arm64/kernel/efi.c        | 24 +++++--
 arch/arm64/kernel/setup.c      |  3 +
 drivers/firmware/efi/efi-fdt.c | 72 ++++++++------------
 include/linux/efi.h            |  2 +-
 5 files changed, 50 insertions(+), 53 deletions(-)

diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index ef572206f1c3..7cc8df68c638 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -6,8 +6,10 @@
 
 #ifdef CONFIG_EFI
 extern void efi_init(void);
+extern void efi_parse_fdt(void *fdt);
 #else
 #define efi_init()
+#define efi_parse_fdt(x)
 #endif
 
 #define efi_call_virt(f, ...)						\
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index ab5eeb63e2ca..a9e63122e32d 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -192,16 +192,14 @@ static __init void reserve_regions(void)
 		if (efi_enabled(EFI_DBG))
 			pr_cont("\n");
 	}
-
-	set_bit(EFI_MEMMAP, &efi.flags);
 }
 
-void __init efi_init(void)
+void __init efi_parse_fdt(void *fdt)
 {
 	struct efi_fdt_params params;
 
 	/* Grab UEFI information placed in FDT by stub */
-	if (!efi_get_fdt_params(&params))
+	if (!efi_get_fdt_params(fdt, &params))
 		return;
 
 	efi_system_table = params.system_table;
@@ -209,16 +207,28 @@ void __init efi_init(void)
 	memblock_reserve(params.mmap & PAGE_MASK,
 			 PAGE_ALIGN(params.mmap_size + (params.mmap & ~PAGE_MASK)));
 	memmap.phys_map = (void *)params.mmap;
-	memmap.map = early_memremap(params.mmap, params.mmap_size);
-	memmap.map_end = memmap.map + params.mmap_size;
 	memmap.desc_size = params.desc_size;
 	memmap.desc_version = params.desc_ver;
+	memmap.nr_map = params.mmap_size / params.desc_size;
+
+	set_bit(EFI_MEMMAP, &efi.flags);
+}
+
+void __init efi_init(void)
+{
+	int mmap_size = memmap.nr_map * memmap.desc_size;
+
+	if (!efi_enabled(EFI_MEMMAP))
+		return;
+
+	memmap.map = early_memremap((u64)memmap.phys_map, mmap_size);
+	memmap.map_end = memmap.map + mmap_size;
 
 	if (uefi_init() < 0)
 		return;
 
 	reserve_regions();
-	early_memunmap(memmap.map, params.mmap_size);
+	early_memunmap(memmap.map, mmap_size);
 }
 
 static bool __init efi_virtmap_init(void)
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index f3067d4d4e35..e0ff2cca510a 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -317,6 +317,9 @@ static void __init setup_machine_fdt(phys_addr_t dt_phys)
 {
 	void *dt_virt = fixmap_remap_fdt(dt_phys);
 
+	if (dt_virt)
+		efi_parse_fdt(dt_virt);
+
 	if (!dt_virt || !early_init_dt_scan(dt_virt)) {
 		pr_crit("\n"
 			"Error: invalid device tree blob@physical address %pa (virtual address 0x%p)\n"
diff --git a/drivers/firmware/efi/efi-fdt.c b/drivers/firmware/efi/efi-fdt.c
index 8f3ce66e2b02..e6622d3182ae 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,60 +31,43 @@ static __initdata struct {
 	UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
 };
 
-struct param_info {
-	int found;
-	void *params;
-};
-
-static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
-				       int depth, void *data)
+int __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;
+	}
 
 	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 || len != dt_params[i].size)
+			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 (efi_enabled(EFI_DBG))
-			pr_info("  %s: 0x%0*llx\n", dt_params[i].name,
-				dt_params[i].size * 2, val);
+		pr_info("  %s: 0x%0*llx\n", dt_params[i].name,
+			dt_params[i].size * 2, val);
 	}
-	return 1;
-}
+	return true;
 
-int __init efi_get_fdt_params(struct efi_fdt_params *params)
-{
-	struct param_info info;
-	int ret;
-
-	pr_info("Getting EFI parameters from FDT:\n");
-
-	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 4677d8a1bfd0..4a3ab85a60d7 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -902,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);
+extern int 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 v3 5/6] 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.

Acked-by: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/kernel/efi.c            |  2 +-
 arch/arm64/mm/init.c               |  6 ++++-
 drivers/firmware/efi/libstub/fdt.c | 24 +-------------------
 3 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index a9e63122e32d..1baa75a0df20 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -181,7 +181,7 @@ static __init void reserve_regions(void)
 		size = npages << PAGE_SHIFT;
 
 		if (is_normal_ram(md))
-			early_init_dt_add_memory_arch(paddr, size);
+			early_init_dt_add_memory(paddr, size);
 
 		if (is_reserve_region(md)) {
 			memblock_reserve(paddr, size);
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index d88e83ebf60d..d8ce7f51dba1 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -377,5 +377,9 @@ __setup("keepinitrd", keepinitrd_setup);
 
 void __init early_init_dt_add_memory_arch(u64 base, u64 size)
 {
-	early_init_dt_add_memory(base, size);
+	/*
+	 * Ignore DT memory nodes if we are booting via UEFI.
+	 */
+	if (!efi_enabled(EFI_MEMMAP))
+		early_init_dt_add_memory(base, size);
 }
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 v3 6/6] arm64/efi: ignore DT memreserve entries instead of removing them

[Ard Biesheuvel]

Now that the reservation of the FDT image itself is split off from
the processing of memory reservations described by the device tree,
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.

Note that this reverts 0ceac9e094b0 ("efi/arm64: Fix fdt-related
memory reservation").

Acked-by: Leif Lindholm <leif.lindholm@linaro.org>
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 d8ce7f51dba1..c74ff920540a 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 v3 0/6] arm64 UEFI early FDT handling

[Ard Biesheuvel]

(+ Grant)

On 22 September 2015 at 02:21, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 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

After reading Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
again, I think simply ignoring the reserved-memory node is not the way
to go. The reason is that it may contain dynamic allocations that are
referenced by other nodes in the DT, and there is no good technical
reason IMO to disallow those. OTOH, static allocations may conflict
with the UEFI memory map, so those need to be dropped or at least
checked against the memory map. The problem here is that static nodes
may also be referenced by phandle, so we need to handle the referring
node in some way as well.

So I think we have a number of options:
- ignore /memreserve/s and reject static allocations in
/reserved-memory (*) but honor dynamic ones
- ignore /memreserve/s and honor all of /reserved-memory after
checking that static allocations don't conflict
- honor all /memreserve/s and /reserved-memory nodes and check all for conflicts
- ...

(*) static allocations for regions that the UEFI memory map does not
describe should be OK, though

I personally prefer the first one, since a dynamic allocation
implicitly conveys that the region does not contain anything special
when coming out of boot, and there is very little we need to do other
than perform the actual reservation. Static allocations are ambiguous
in the sense that there is no annotation that explains the choice of
address.

Thoughts, please?

-- 
Ard.


> - 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.
>
> Patches #1 and #2 introduce 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 #3 moves some UEFI+FDT init code around before making changes to it.
>
> Patch #4 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 #5 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 #6 does the same as #5, but for memreserves and the /reserved-memory
> node.
>
> Changes since v2:
> - instead of copying the generic implementation, turn
>   early_init_dt_add_memory_arch() into a weak alias so that it is still
>   accessible to overrides
>
> 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 (6):
>   of/fdt: make generic early_init_dt_add_memory_arch() a weak alias
>   arm64: override generic version of early_init_dt_add_memory_arch()
>   efi: move FDT handling to separate object file
>   arm64/efi: move EFI /chosen node parsing 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       |  2 +
>  arch/arm64/kernel/efi.c            | 26 ++++--
>  arch/arm64/kernel/setup.c          |  3 +
>  arch/arm64/mm/init.c               | 12 ++-
>  drivers/firmware/efi/Makefile      |  1 +
>  drivers/firmware/efi/efi-fdt.c     | 73 +++++++++++++++++
>  drivers/firmware/efi/efi.c         | 84 --------------------
>  drivers/firmware/efi/libstub/fdt.c | 33 +-------
>  drivers/of/fdt.c                   |  7 +-
>  include/linux/efi.h                |  2 +-
>  include/linux/of_fdt.h             |  1 +
>  11 files changed, 116 insertions(+), 128 deletions(-)
>  create mode 100644 drivers/firmware/efi/efi-fdt.c
>
> --
> 1.9.1
>

[PATCH v3 0/6] arm64 UEFI early FDT handling

[Will Deacon]

Hi Ard,

On Tue, Sep 29, 2015 at 08:38:57AM +0100, Ard Biesheuvel wrote:
> (+ Grant)
> 
> On 22 September 2015 at 02:21, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > 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
> 
> After reading Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> again, I think simply ignoring the reserved-memory node is not the way
> to go. The reason is that it may contain dynamic allocations that are
> referenced by other nodes in the DT, and there is no good technical
> reason IMO to disallow those. OTOH, static allocations may conflict
> with the UEFI memory map, so those need to be dropped or at least
> checked against the memory map. The problem here is that static nodes
> may also be referenced by phandle, so we need to handle the referring
> node in some way as well.
> 
> So I think we have a number of options:
> - ignore /memreserve/s and reject static allocations in
> /reserved-memory (*) but honor dynamic ones
> - ignore /memreserve/s and honor all of /reserved-memory after
> checking that static allocations don't conflict
> - honor all /memreserve/s and /reserved-memory nodes and check all for conflicts
> - ...
> 
> (*) static allocations for regions that the UEFI memory map does not
> describe should be OK, though
> 
> I personally prefer the first one, since a dynamic allocation
> implicitly conveys that the region does not contain anything special
> when coming out of boot, and there is very little we need to do other
> than perform the actual reservation. Static allocations are ambiguous
> in the sense that there is no annotation that explains the choice of
> address.
> 
> Thoughts, please?

What's the status of this series? It was on my "list of patches to watch"
that I'm just refreshing for 4.5, but I can't see any comments on-list
about it.

Cheers,

Will

[PATCH v3 0/6] arm64 UEFI early FDT handling

[Ard Biesheuvel]

On 16 November 2015 at 11:43, Will Deacon <will.deacon@arm.com> wrote:
> Hi Ard,
>
> On Tue, Sep 29, 2015 at 08:38:57AM +0100, Ard Biesheuvel wrote:
>> (+ Grant)
>>
>> On 22 September 2015 at 02:21, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> > 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
>>
>> After reading Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>> again, I think simply ignoring the reserved-memory node is not the way
>> to go. The reason is that it may contain dynamic allocations that are
>> referenced by other nodes in the DT, and there is no good technical
>> reason IMO to disallow those. OTOH, static allocations may conflict
>> with the UEFI memory map, so those need to be dropped or at least
>> checked against the memory map. The problem here is that static nodes
>> may also be referenced by phandle, so we need to handle the referring
>> node in some way as well.
>>
>> So I think we have a number of options:
>> - ignore /memreserve/s and reject static allocations in
>> /reserved-memory (*) but honor dynamic ones
>> - ignore /memreserve/s and honor all of /reserved-memory after
>> checking that static allocations don't conflict
>> - honor all /memreserve/s and /reserved-memory nodes and check all for conflicts
>> - ...
>>
>> (*) static allocations for regions that the UEFI memory map does not
>> describe should be OK, though
>>
>> I personally prefer the first one, since a dynamic allocation
>> implicitly conveys that the region does not contain anything special
>> when coming out of boot, and there is very little we need to do other
>> than perform the actual reservation. Static allocations are ambiguous
>> in the sense that there is no annotation that explains the choice of
>> address.
>>
>> Thoughts, please?
>
> What's the status of this series? It was on my "list of patches to watch"
> that I'm just refreshing for 4.5, but I can't see any comments on-list
> about it.
>

Good question. I got very little feedback (except from Ganapatrao) so
I didn't think people actually cared about this.
I followed up with a series that validates the /reserved-memory node
rather than removes it (which means we can keep both static and
dynamic allocations) but no response to that one either, apart from
some comments on implementation details

So we need to have the discussion first before looking at patches
again, I think, since the way we deal with these nodes should be based
on a comprehensive approach regarding memory nodes, memreserves and
/reserved-memory when booting via UEFI.

-- 
Ard.

[PATCH v3 0/6] arm64 UEFI early FDT handling

[Catalin Marinas]

On Mon, Nov 16, 2015 at 10:43:52AM +0000, Will Deacon wrote:
> On Tue, Sep 29, 2015 at 08:38:57AM +0100, Ard Biesheuvel wrote:
> > On 22 September 2015 at 02:21, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > > 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
> > 
> > After reading Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > again, I think simply ignoring the reserved-memory node is not the way
> > to go. The reason is that it may contain dynamic allocations that are
> > referenced by other nodes in the DT, and there is no good technical
> > reason IMO to disallow those. OTOH, static allocations may conflict
> > with the UEFI memory map, so those need to be dropped or at least
> > checked against the memory map. The problem here is that static nodes
> > may also be referenced by phandle, so we need to handle the referring
> > node in some way as well.
> > 
> > So I think we have a number of options:
> > - ignore /memreserve/s and reject static allocations in
> > /reserved-memory (*) but honor dynamic ones
> > - ignore /memreserve/s and honor all of /reserved-memory after
> > checking that static allocations don't conflict
> > - honor all /memreserve/s and /reserved-memory nodes and check all for conflicts
> > - ...
> > 
> > (*) static allocations for regions that the UEFI memory map does not
> > describe should be OK, though
> > 
> > I personally prefer the first one, since a dynamic allocation
> > implicitly conveys that the region does not contain anything special
> > when coming out of boot, and there is very little we need to do other
> > than perform the actual reservation. Static allocations are ambiguous
> > in the sense that there is no annotation that explains the choice of
> > address.
> > 
> > Thoughts, please?
> 
> What's the status of this series? It was on my "list of patches to watch"
> that I'm just refreshing for 4.5, but I can't see any comments on-list
> about it.

I thought it's being taken over by this series:

http://lkml.kernel.org/r/1446126059-25336-1-git-send-email-ard.biesheuvel at linaro.org

-- 
Catalin

[PATCH v3 0/6] arm64 UEFI early FDT handling

[Ard Biesheuvel]

On 16 November 2015 at 11:57, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Mon, Nov 16, 2015 at 10:43:52AM +0000, Will Deacon wrote:
>> On Tue, Sep 29, 2015 at 08:38:57AM +0100, Ard Biesheuvel wrote:
>> > On 22 September 2015 at 02:21, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> > > 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
>> >
>> > After reading Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>> > again, I think simply ignoring the reserved-memory node is not the way
>> > to go. The reason is that it may contain dynamic allocations that are
>> > referenced by other nodes in the DT, and there is no good technical
>> > reason IMO to disallow those. OTOH, static allocations may conflict
>> > with the UEFI memory map, so those need to be dropped or at least
>> > checked against the memory map. The problem here is that static nodes
>> > may also be referenced by phandle, so we need to handle the referring
>> > node in some way as well.
>> >
>> > So I think we have a number of options:
>> > - ignore /memreserve/s and reject static allocations in
>> > /reserved-memory (*) but honor dynamic ones
>> > - ignore /memreserve/s and honor all of /reserved-memory after
>> > checking that static allocations don't conflict
>> > - honor all /memreserve/s and /reserved-memory nodes and check all for conflicts
>> > - ...
>> >
>> > (*) static allocations for regions that the UEFI memory map does not
>> > describe should be OK, though
>> >
>> > I personally prefer the first one, since a dynamic allocation
>> > implicitly conveys that the region does not contain anything special
>> > when coming out of boot, and there is very little we need to do other
>> > than perform the actual reservation. Static allocations are ambiguous
>> > in the sense that there is no annotation that explains the choice of
>> > address.
>> >
>> > Thoughts, please?
>>
>> What's the status of this series? It was on my "list of patches to watch"
>> that I'm just refreshing for 4.5, but I can't see any comments on-list
>> about it.
>
> I thought it's being taken over by this series:
>
> http://lkml.kernel.org/r/1446126059-25336-1-git-send-email-ard.biesheuvel at linaro.org
>

No, that is a separate thing entirely.

That series deals with ways to remove ranges from the linear mapping
without losing the annotation that it is memory (to ensure that things
like page_is_ram() do the right thing)

This series deals with policy regarding memory nodes, memreserves and
the /reserved-memory node (which are all different things, mind you)
when booting via UEFI, since UEFI has its own method of conveying the
same information. The primary problem is that, currently, the
/reserved-memory node is *not* ignored, while its contents may
conflict with reservation in the UEFI memory map.

-- 
Ard.