linux-efi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] efi/arm: refactor DT EFI param parsing
@ 2020-02-19 15:24 Ard Biesheuvel
  2020-02-19 15:24 ` [PATCH 1/3] efi/arm: move FDT param discovery code out of efi.c Ard Biesheuvel
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2020-02-19 15:24 UTC (permalink / raw)
  To: linux-efi; +Cc: linux-arm-kernel, atish.patra, leif, Ard Biesheuvel

In preparation for another arrival (RISC-V), do some janitorial work on
the FDT param parsing code for EFI as well. The code sits in an #ifdef
block in efi.c, which is usually a good indicator that it should be moved
into its own source file. Then, we simplify the DT handling, by switching
to the FDT library, instead of going through the more high level early OF
enumeration code, which in our case will be doing libfdt calls under the
hood as well, as EFI on ARM specifically uses *flattened* DT. (note that
the EFI stub for ARM uses libfdt as well to populate the DT properties
that we read back here)

Ard Biesheuvel (3):
  efi/arm: move FDT param discovery code out of efi.c
  efi/arm: move FDT specific definitions into fdtparams.c
  efi/arm: rewrite FDT param discovery routines

 drivers/firmware/efi/Makefile    |   1 +
 drivers/firmware/efi/arm-init.c  |  17 +--
 drivers/firmware/efi/efi.c       | 135 --------------------
 drivers/firmware/efi/fdtparams.c | 125 ++++++++++++++++++
 include/linux/efi.h              |  10 +-
 5 files changed, 133 insertions(+), 155 deletions(-)
 create mode 100644 drivers/firmware/efi/fdtparams.c

-- 
2.17.1


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

* [PATCH 1/3] efi/arm: move FDT param discovery code out of efi.c
  2020-02-19 15:24 [PATCH 0/3] efi/arm: refactor DT EFI param parsing Ard Biesheuvel
@ 2020-02-19 15:24 ` Ard Biesheuvel
  2020-02-19 15:24 ` [PATCH 2/3] efi/arm: move FDT specific definitions into fdtparams.c Ard Biesheuvel
  2020-02-19 15:24 ` [PATCH 3/3] efi/arm: rewrite FDT param discovery routines Ard Biesheuvel
  2 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2020-02-19 15:24 UTC (permalink / raw)
  To: linux-efi; +Cc: linux-arm-kernel, atish.patra, leif, Ard Biesheuvel

On ARM systems, we discover the UEFI system table address and memory
map address from the /chosen node in the device tree, or in the Xen
case, from a similar node under /hypervisor.

Before making some functional changes to that code, move it into its
own file that only gets built if CONFIG_EFI_PARAMS_FROM_FDT=y.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/Makefile    |   1 +
 drivers/firmware/efi/efi.c       | 135 -------------------
 drivers/firmware/efi/fdtparams.c | 142 ++++++++++++++++++++
 3 files changed, 143 insertions(+), 135 deletions(-)

diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index 554d795270d9..3c5a9690de04 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -13,6 +13,7 @@ KASAN_SANITIZE_runtime-wrappers.o	:= n
 obj-$(CONFIG_ACPI_BGRT) 		+= efi-bgrt.o
 obj-$(CONFIG_EFI)			+= efi.o vars.o reboot.o memattr.o tpm.o
 obj-$(CONFIG_EFI)			+= capsule.o memmap.o
+obj-$(CONFIG_EFI_PARAMS_FROM_FDT)	+= fdtparams.o
 obj-$(CONFIG_EFI_VARS)			+= efivars.o
 obj-$(CONFIG_EFI_ESRT)			+= esrt.o
 obj-$(CONFIG_EFI_VARS_PSTORE)		+= efi-pstore.o
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 55570ed751e8..69a585106d30 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -20,7 +20,6 @@
 #include <linux/device.h>
 #include <linux/efi.h>
 #include <linux/of.h>
-#include <linux/of_fdt.h>
 #include <linux/io.h>
 #include <linux/kexec.h>
 #include <linux/platform_device.h>
@@ -656,140 +655,6 @@ void __init efi_systab_report_header(const efi_table_hdr_t *systab_hdr,
 		vendor);
 }
 
-#ifdef CONFIG_EFI_PARAMS_FROM_FDT
-
-#define UEFI_PARAM(name, prop, field)			   \
-	{						   \
-		{ name },				   \
-		{ prop },				   \
-		offsetof(struct efi_fdt_params, field),    \
-		sizeof_field(struct efi_fdt_params, field) \
-	}
-
-struct params {
-	const char name[32];
-	const char propname[32];
-	int offset;
-	int size;
-};
-
-static __initdata struct params fdt_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)
-};
-
-static __initdata struct params xen_fdt_params[] = {
-	UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
-	UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
-	UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
-	UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", desc_size),
-	UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", desc_ver)
-};
-
-#define EFI_FDT_PARAMS_SIZE	ARRAY_SIZE(fdt_params)
-
-static __initdata struct {
-	const char *uname;
-	const char *subnode;
-	struct params *params;
-} dt_params[] = {
-	{ "hypervisor", "uefi", xen_fdt_params },
-	{ "chosen", NULL, fdt_params },
-};
-
-struct param_info {
-	int found;
-	void *params;
-	const char *missing;
-};
-
-static int __init __find_uefi_params(unsigned long node,
-				     struct param_info *info,
-				     struct params *params)
-{
-	const void *prop;
-	void *dest;
-	u64 val;
-	int i, len;
-
-	for (i = 0; i < EFI_FDT_PARAMS_SIZE; i++) {
-		prop = of_get_flat_dt_prop(node, params[i].propname, &len);
-		if (!prop) {
-			info->missing = params[i].name;
-			return 0;
-		}
-
-		dest = info->params + params[i].offset;
-		info->found++;
-
-		val = of_read_number(prop, len / sizeof(u32));
-
-		if (params[i].size == sizeof(u32))
-			*(u32 *)dest = val;
-		else
-			*(u64 *)dest = val;
-
-		if (efi_enabled(EFI_DBG))
-			pr_info("  %s: 0x%0*llx\n", params[i].name,
-				params[i].size * 2, val);
-	}
-
-	return 1;
-}
-
-static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
-				       int depth, void *data)
-{
-	struct param_info *info = data;
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(dt_params); i++) {
-		const char *subnode = dt_params[i].subnode;
-
-		if (depth != 1 || strcmp(uname, dt_params[i].uname) != 0) {
-			info->missing = dt_params[i].params[0].name;
-			continue;
-		}
-
-		if (subnode) {
-			int err = of_get_flat_dt_subnode_by_name(node, subnode);
-
-			if (err < 0)
-				return 0;
-
-			node = err;
-		}
-
-		return __find_uefi_params(node, info, dt_params[i].params);
-	}
-
-	return 0;
-}
-
-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",
-		       info.missing);
-
-	return ret;
-}
-#endif /* CONFIG_EFI_PARAMS_FROM_FDT */
-
 static __initdata char memory_type_name[][20] = {
 	"Reserved",
 	"Loader Code",
diff --git a/drivers/firmware/efi/fdtparams.c b/drivers/firmware/efi/fdtparams.c
new file mode 100644
index 000000000000..3de343faffc6
--- /dev/null
+++ b/drivers/firmware/efi/fdtparams.c
@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#define pr_fmt(fmt) "efi: " fmt
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/efi.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+
+#include <asm/early_ioremap.h>
+
+#define UEFI_PARAM(name, prop, field)			   \
+	{						   \
+		{ name },				   \
+		{ prop },				   \
+		offsetof(struct efi_fdt_params, field),    \
+		sizeof_field(struct efi_fdt_params, field) \
+	}
+
+struct params {
+	const char name[32];
+	const char propname[32];
+	int offset;
+	int size;
+};
+
+static __initdata struct params fdt_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)
+};
+
+static __initdata struct params xen_fdt_params[] = {
+	UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
+	UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
+	UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
+	UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", desc_size),
+	UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", desc_ver)
+};
+
+#define EFI_FDT_PARAMS_SIZE	ARRAY_SIZE(fdt_params)
+
+static __initdata struct {
+	const char *uname;
+	const char *subnode;
+	struct params *params;
+} dt_params[] = {
+	{ "hypervisor", "uefi", xen_fdt_params },
+	{ "chosen", NULL, fdt_params },
+};
+
+struct param_info {
+	int found;
+	void *params;
+	const char *missing;
+};
+
+static int __init __find_uefi_params(unsigned long node,
+				     struct param_info *info,
+				     struct params *params)
+{
+	const void *prop;
+	void *dest;
+	u64 val;
+	int i, len;
+
+	for (i = 0; i < EFI_FDT_PARAMS_SIZE; i++) {
+		prop = of_get_flat_dt_prop(node, params[i].propname, &len);
+		if (!prop) {
+			info->missing = params[i].name;
+			return 0;
+		}
+
+		dest = info->params + params[i].offset;
+		info->found++;
+
+		val = of_read_number(prop, len / sizeof(u32));
+
+		if (params[i].size == sizeof(u32))
+			*(u32 *)dest = val;
+		else
+			*(u64 *)dest = val;
+
+		if (efi_enabled(EFI_DBG))
+			pr_info("  %s: 0x%0*llx\n", params[i].name,
+				params[i].size * 2, val);
+	}
+
+	return 1;
+}
+
+static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
+				       int depth, void *data)
+{
+	struct param_info *info = data;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dt_params); i++) {
+		const char *subnode = dt_params[i].subnode;
+
+		if (depth != 1 || strcmp(uname, dt_params[i].uname) != 0) {
+			info->missing = dt_params[i].params[0].name;
+			continue;
+		}
+
+		if (subnode) {
+			int err = of_get_flat_dt_subnode_by_name(node, subnode);
+
+			if (err < 0)
+				return 0;
+
+			node = err;
+		}
+
+		return __find_uefi_params(node, info, dt_params[i].params);
+	}
+
+	return 0;
+}
+
+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",
+		       info.missing);
+
+	return ret;
+}
-- 
2.17.1


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

* [PATCH 2/3] efi/arm: move FDT specific definitions into fdtparams.c
  2020-02-19 15:24 [PATCH 0/3] efi/arm: refactor DT EFI param parsing Ard Biesheuvel
  2020-02-19 15:24 ` [PATCH 1/3] efi/arm: move FDT param discovery code out of efi.c Ard Biesheuvel
@ 2020-02-19 15:24 ` Ard Biesheuvel
  2020-02-19 15:24 ` [PATCH 3/3] efi/arm: rewrite FDT param discovery routines Ard Biesheuvel
  2 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2020-02-19 15:24 UTC (permalink / raw)
  To: linux-efi; +Cc: linux-arm-kernel, atish.patra, leif, Ard Biesheuvel

Push the FDT params specific types and definition into fdtparams.c,
and instead, pass a reference to the memory map data structure and
populate it directly, and return the system table address as the
return value.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/arm-init.c  | 17 ++++-------
 drivers/firmware/efi/fdtparams.c | 30 +++++++++++++++-----
 include/linux/efi.h              | 10 +------
 3 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index 76bf5b22e49e..2791a8048f30 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -205,17 +205,13 @@ static __init void reserve_regions(void)
 void __init efi_init(void)
 {
 	struct efi_memory_map_data data;
-	struct efi_fdt_params params;
+	u64 efi_system_table;
 
 	/* Grab UEFI information placed in FDT by stub */
-	if (!efi_get_fdt_params(&params))
+	efi_system_table = efi_get_fdt_params(&data);
+	if (!efi_system_table)
 		return;
 
-	data.desc_version = params.desc_ver;
-	data.desc_size = params.desc_size;
-	data.size = params.mmap_size;
-	data.phys_map = params.mmap;
-
 	if (efi_memmap_init_early(&data) < 0) {
 		/*
 		* If we are booting via UEFI, the UEFI memory map is the only
@@ -229,7 +225,7 @@ void __init efi_init(void)
 	     "Unexpected EFI_MEMORY_DESCRIPTOR version %ld",
 	      efi.memmap.desc_version);
 
-	if (uefi_init(params.system_table) < 0) {
+	if (uefi_init(efi_system_table) < 0) {
 		efi_memmap_unmap();
 		return;
 	}
@@ -237,9 +233,8 @@ void __init efi_init(void)
 	reserve_regions();
 	efi_esrt_init();
 
-	memblock_reserve(params.mmap & PAGE_MASK,
-			 PAGE_ALIGN(params.mmap_size +
-				    (params.mmap & ~PAGE_MASK)));
+	memblock_reserve(data.phys_map & PAGE_MASK,
+			 PAGE_ALIGN(data.size + (data.phys_map & ~PAGE_MASK)));
 
 	init_screen_info();
 
diff --git a/drivers/firmware/efi/fdtparams.c b/drivers/firmware/efi/fdtparams.c
index 3de343faffc6..7a384b307c56 100644
--- a/drivers/firmware/efi/fdtparams.c
+++ b/drivers/firmware/efi/fdtparams.c
@@ -18,6 +18,14 @@
 		sizeof_field(struct efi_fdt_params, field) \
 	}
 
+struct efi_fdt_params {
+	u64 system_table;
+	u64 mmap;
+	u32 mmap_size;
+	u32 desc_size;
+	u32 desc_ver;
+};
+
 struct params {
 	const char name[32];
 	const char propname[32];
@@ -121,22 +129,30 @@ static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
 	return 0;
 }
 
-int __init efi_get_fdt_params(struct efi_fdt_params *params)
+u64 __init efi_get_fdt_params(struct efi_memory_map_data *memmap)
 {
+	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;
+	info.params = &params;
 
 	ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
-	if (!info.found)
+	if (!info.found) {
 		pr_info("UEFI not found.\n");
-	else if (!ret)
-		pr_err("Can't find '%s' in device tree!\n",
-		       info.missing);
+		return 0;
+	} else if (!ret) {
+		pr_err("Can't find '%s' in device tree!\n", info.missing);
+		return 0;
+	}
+
+	memmap->desc_version	= params.desc_ver;
+	memmap->desc_size	= params.desc_size;
+	memmap->size		= params.mmap_size;
+	memmap->phys_map	= params.mmap;
 
-	return ret;
+	return params.system_table;
 }
diff --git a/include/linux/efi.h b/include/linux/efi.h
index a36bbd20e3ac..2ab33d5d6ca5 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -478,14 +478,6 @@ struct efi_mem_range {
 	u64 attribute;
 };
 
-struct efi_fdt_params {
-	u64 system_table;
-	u64 mmap;
-	u32 mmap_size;
-	u32 desc_size;
-	u32 desc_ver;
-};
-
 typedef struct {
 	u32 version;
 	u32 length;
@@ -661,7 +653,7 @@ extern void efi_mem_reserve(phys_addr_t addr, u64 size);
 extern int efi_mem_reserve_persistent(phys_addr_t addr, u64 size);
 extern void efi_initialize_iomem_resources(struct resource *code_resource,
 		struct resource *data_resource, struct resource *bss_resource);
-extern int efi_get_fdt_params(struct efi_fdt_params *params);
+extern u64 efi_get_fdt_params(struct efi_memory_map_data *data);
 extern struct kobject *efi_kobj;
 
 extern int efi_reboot_quirk_mode;
-- 
2.17.1


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

* [PATCH 3/3] efi/arm: rewrite FDT param discovery routines
  2020-02-19 15:24 [PATCH 0/3] efi/arm: refactor DT EFI param parsing Ard Biesheuvel
  2020-02-19 15:24 ` [PATCH 1/3] efi/arm: move FDT param discovery code out of efi.c Ard Biesheuvel
  2020-02-19 15:24 ` [PATCH 2/3] efi/arm: move FDT specific definitions into fdtparams.c Ard Biesheuvel
@ 2020-02-19 15:24 ` Ard Biesheuvel
  2020-02-20 18:30   ` Leif Lindholm
  2 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2020-02-19 15:24 UTC (permalink / raw)
  To: linux-efi; +Cc: linux-arm-kernel, atish.patra, leif, Ard Biesheuvel

The efi_get_fdt_params() routine uses the early OF device tree
traversal helpers, that iterate over each node in the DT and invoke
a caller provided callback that can inspect the node's contents and
look for the required data. This requires a special param struct to
be passed around, with pointers into param enumeration structs that
contain (and duplicate) property names and offsets into yet another
struct that carries the collected data.

Since we know the data we look for is either under /hypervisor/uefi
or under /chosen, it is much simpler to use the libfdt routines
directly, and try to grab a reference to either node directly, and
read each property in sequence.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/fdtparams.c | 203 ++++++++------------
 1 file changed, 85 insertions(+), 118 deletions(-)

diff --git a/drivers/firmware/efi/fdtparams.c b/drivers/firmware/efi/fdtparams.c
index 7a384b307c56..23af4062e913 100644
--- a/drivers/firmware/efi/fdtparams.c
+++ b/drivers/firmware/efi/fdtparams.c
@@ -5,154 +5,121 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/efi.h>
-#include <linux/of.h>
+#include <linux/libfdt.h>
 #include <linux/of_fdt.h>
 
-#include <asm/early_ioremap.h>
+#include <asm/unaligned.h>
 
-#define UEFI_PARAM(name, prop, field)			   \
-	{						   \
-		{ name },				   \
-		{ prop },				   \
-		offsetof(struct efi_fdt_params, field),    \
-		sizeof_field(struct efi_fdt_params, field) \
-	}
-
-struct efi_fdt_params {
-	u64 system_table;
-	u64 mmap;
-	u32 mmap_size;
-	u32 desc_size;
-	u32 desc_ver;
-};
-
-struct params {
-	const char name[32];
-	const char propname[32];
-	int offset;
-	int size;
+enum {
+	SYSTAB,
+	MMBASE,
+	MMSIZE,
+	DCSIZE,
+	DCVERS,
 };
 
-static __initdata struct params fdt_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)
+static __initconst const char name[][22] = {
+	[SYSTAB] = "System Table         ",
+	[MMBASE] = "MemMap Address       ",
+	[MMSIZE] = "MemMap Size          ",
+	[DCSIZE] = "MemMap Desc. Size    ",
+	[DCVERS] = "MemMap Desc. Version ",
 };
 
-static __initdata struct params xen_fdt_params[] = {
-	UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
-	UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
-	UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
-	UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", desc_size),
-	UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", desc_ver)
-};
-
-#define EFI_FDT_PARAMS_SIZE	ARRAY_SIZE(fdt_params)
-
-static __initdata struct {
-	const char *uname;
-	const char *subnode;
-	struct params *params;
+static __initconst const struct {
+	const char	path[17];
+	const char	params[5][26];
 } dt_params[] = {
-	{ "hypervisor", "uefi", xen_fdt_params },
-	{ "chosen", NULL, fdt_params },
-};
-
-struct param_info {
-	int found;
-	void *params;
-	const char *missing;
+#ifdef CONFIG_XEN
+	{
+		"/hypervisor/uefi",
+		{
+			[SYSTAB] = "xen,uefi-system-table",
+			[MMBASE] = "xen,uefi-mmap-start",
+			[MMSIZE] = "xen,uefi-mmap-size",
+			[DCSIZE] = "xen,uefi-mmap-desc-size",
+			[DCVERS] = "xen,uefi-mmap-desc-ver",
+		}
+	},
+#endif
+	{
+		"/chosen",
+		{
+			[SYSTAB] = "linux,uefi-system-table",
+			[MMBASE] = "linux,uefi-mmap-start",
+			[MMSIZE] = "linux,uefi-mmap-size",
+			[DCSIZE] = "linux,uefi-mmap-desc-size",
+			[DCVERS] = "linux,uefi-mmap-desc-ver",
+		}
+	}
 };
 
-static int __init __find_uefi_params(unsigned long node,
-				     struct param_info *info,
-				     struct params *params)
+static int __init efi_get_fdt_prop(const void *fdt, int node, int dtp, int pp,
+				   void *var, int size)
 {
 	const void *prop;
-	void *dest;
+	int len;
 	u64 val;
-	int i, len;
 
-	for (i = 0; i < EFI_FDT_PARAMS_SIZE; i++) {
-		prop = of_get_flat_dt_prop(node, params[i].propname, &len);
-		if (!prop) {
-			info->missing = params[i].name;
-			return 0;
-		}
-
-		dest = info->params + params[i].offset;
-		info->found++;
+	prop = fdt_getprop(fdt, node, dt_params[dtp].params[pp], &len);
+	if (!prop) {
+		pr_err("Can't find property '%s' in device tree!\n",
+		       dt_params[dtp].params[pp]);
+		return 1;
+	}
 
-		val = of_read_number(prop, len / sizeof(u32));
+	val = (len == 4) ? (u64)be32_to_cpup(prop) : get_unaligned_be64(prop);
 
-		if (params[i].size == sizeof(u32))
-			*(u32 *)dest = val;
-		else
-			*(u64 *)dest = val;
+	if (size == 8)
+		*(u64 *)var = val;
+	else
+		*(u32 *)var = (val <= U32_MAX) ? val : U32_MAX; // saturate
 
-		if (efi_enabled(EFI_DBG))
-			pr_info("  %s: 0x%0*llx\n", params[i].name,
-				params[i].size * 2, val);
-	}
+	if (efi_enabled(EFI_DBG))
+		pr_info("  %s: 0x%0*llx\n", name[pp], size * 2, val);
 
-	return 1;
+	return 0;
 }
 
-static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
-				       int depth, void *data)
+u64 __init efi_get_fdt_params(struct efi_memory_map_data *mm)
 {
-	struct param_info *info = data;
+	const void *fdt = initial_boot_params;
+	unsigned long systab;
+	struct {
+		void	*var;
+		int	size;
+	} target[] = {
+		[SYSTAB] = { &systab,		sizeof(systab) },
+		[MMBASE] = { &mm->phys_map,	sizeof(mm->phys_map) },
+		[MMSIZE] = { &mm->size,		sizeof(mm->size) },
+		[DCSIZE] = { &mm->desc_size,	sizeof(mm->desc_size) },
+		[DCVERS] = { &mm->desc_version,	sizeof(mm->desc_version) },
+	};
 	int i;
 
+	BUILD_BUG_ON(ARRAY_SIZE(target) != ARRAY_SIZE(name));
+	BUILD_BUG_ON(ARRAY_SIZE(target) != ARRAY_SIZE(dt_params[0].params));
+
 	for (i = 0; i < ARRAY_SIZE(dt_params); i++) {
-		const char *subnode = dt_params[i].subnode;
+		int node;
+		int j;
 
-		if (depth != 1 || strcmp(uname, dt_params[i].uname) != 0) {
-			info->missing = dt_params[i].params[0].name;
+		node = fdt_path_offset(fdt, dt_params[i].path);
+		if (node < 0)
 			continue;
-		}
 
-		if (subnode) {
-			int err = of_get_flat_dt_subnode_by_name(node, subnode);
+		if (efi_enabled(EFI_DBG))
+			pr_info("Getting UEFI parameters from %s in DT:\n",
+				dt_params[i].path);
 
-			if (err < 0)
+		for (j = 0; j < ARRAY_SIZE(target); j++) {
+			if (efi_get_fdt_prop(fdt, node, i, j, target[j].var,
+					     target[j].size)) {
 				return 0;
-
-			node = err;
+			}
 		}
-
-		return __find_uefi_params(node, info, dt_params[i].params);
+		return systab;
 	}
-
+	pr_info("UEFI not found.\n");
 	return 0;
 }
-
-u64 __init efi_get_fdt_params(struct efi_memory_map_data *memmap)
-{
-	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");
-		return 0;
-	} else if (!ret) {
-		pr_err("Can't find '%s' in device tree!\n", info.missing);
-		return 0;
-	}
-
-	memmap->desc_version	= params.desc_ver;
-	memmap->desc_size	= params.desc_size;
-	memmap->size		= params.mmap_size;
-	memmap->phys_map	= params.mmap;
-
-	return params.system_table;
-}
-- 
2.17.1


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

* Re: [PATCH 3/3] efi/arm: rewrite FDT param discovery routines
  2020-02-19 15:24 ` [PATCH 3/3] efi/arm: rewrite FDT param discovery routines Ard Biesheuvel
@ 2020-02-20 18:30   ` Leif Lindholm
  2020-02-20 19:20     ` Ard Biesheuvel
  0 siblings, 1 reply; 6+ messages in thread
From: Leif Lindholm @ 2020-02-20 18:30 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-efi, linux-arm-kernel, atish.patra

On Wed, Feb 19, 2020 at 16:24:40 +0100, Ard Biesheuvel wrote:
> The efi_get_fdt_params() routine uses the early OF device tree
> traversal helpers, that iterate over each node in the DT and invoke
> a caller provided callback that can inspect the node's contents and
> look for the required data. This requires a special param struct to
> be passed around, with pointers into param enumeration structs that
> contain (and duplicate) property names and offsets into yet another
> struct that carries the collected data.
> 
> Since we know the data we look for is either under /hypervisor/uefi
> or under /chosen, it is much simpler to use the libfdt routines
> directly, and try to grab a reference to either node directly, and
> read each property in sequence.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/firmware/efi/fdtparams.c | 203 ++++++++------------
>  1 file changed, 85 insertions(+), 118 deletions(-)
> 
> diff --git a/drivers/firmware/efi/fdtparams.c b/drivers/firmware/efi/fdtparams.c
> index 7a384b307c56..23af4062e913 100644
> --- a/drivers/firmware/efi/fdtparams.c
> +++ b/drivers/firmware/efi/fdtparams.c
> @@ -5,154 +5,121 @@
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/efi.h>
> -#include <linux/of.h>
> +#include <linux/libfdt.h>
>  #include <linux/of_fdt.h>
>  
> -#include <asm/early_ioremap.h>
> +#include <asm/unaligned.h>
>  
> -#define UEFI_PARAM(name, prop, field)			   \
> -	{						   \
> -		{ name },				   \
> -		{ prop },				   \
> -		offsetof(struct efi_fdt_params, field),    \
> -		sizeof_field(struct efi_fdt_params, field) \
> -	}
> -
> -struct efi_fdt_params {
> -	u64 system_table;
> -	u64 mmap;
> -	u32 mmap_size;
> -	u32 desc_size;
> -	u32 desc_ver;
> -};
> -
> -struct params {
> -	const char name[32];
> -	const char propname[32];
> -	int offset;
> -	int size;
> +enum {
> +	SYSTAB,
> +	MMBASE,
> +	MMSIZE,
> +	DCSIZE,
> +	DCVERS,
>  };
>  
> -static __initdata struct params fdt_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)
> +static __initconst const char name[][22] = {

I was going to complain about that 22, and I still think it looks a
bit iffy, but I can't find a compiler that doesn't throw a warning if
the value changes, so I guess I'm being over sensitive.

> +	[SYSTAB] = "System Table         ",
> +	[MMBASE] = "MemMap Address       ",
> +	[MMSIZE] = "MemMap Size          ",
> +	[DCSIZE] = "MemMap Desc. Size    ",
> +	[DCVERS] = "MemMap Desc. Version ",
>  };
>  
> -static __initdata struct params xen_fdt_params[] = {
> -	UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
> -	UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
> -	UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
> -	UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", desc_size),
> -	UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", desc_ver)
> -};
> -
> -#define EFI_FDT_PARAMS_SIZE	ARRAY_SIZE(fdt_params)
> -
> -static __initdata struct {
> -	const char *uname;
> -	const char *subnode;
> -	struct params *params;
> +static __initconst const struct {
> +	const char	path[17];
> +	const char	params[5][26];

And I guess the same is true here, but the 5 *could* theoretically be
replaced by a compile-time calculation of DCVERS - SYSTAB + 1. That
may not be less error prone however. Finishing the enum on a
somethingMAX entry might make it slightly better?

But the error checking isn't really what I'm after, but getting rid of
the opaqueness. Could we have comments about which specific string
lengths we're matching against here for us bears of very little brain?

/
    Leif

>  } dt_params[] = {
> -	{ "hypervisor", "uefi", xen_fdt_params },
> -	{ "chosen", NULL, fdt_params },
> -};
> -
> -struct param_info {
> -	int found;
> -	void *params;
> -	const char *missing;
> +#ifdef CONFIG_XEN
> +	{
> +		"/hypervisor/uefi",
> +		{
> +			[SYSTAB] = "xen,uefi-system-table",
> +			[MMBASE] = "xen,uefi-mmap-start",
> +			[MMSIZE] = "xen,uefi-mmap-size",
> +			[DCSIZE] = "xen,uefi-mmap-desc-size",
> +			[DCVERS] = "xen,uefi-mmap-desc-ver",
> +		}
> +	},
> +#endif
> +	{
> +		"/chosen",
> +		{
> +			[SYSTAB] = "linux,uefi-system-table",
> +			[MMBASE] = "linux,uefi-mmap-start",
> +			[MMSIZE] = "linux,uefi-mmap-size",
> +			[DCSIZE] = "linux,uefi-mmap-desc-size",
> +			[DCVERS] = "linux,uefi-mmap-desc-ver",
> +		}
> +	}
>  };
>  
> -static int __init __find_uefi_params(unsigned long node,
> -				     struct param_info *info,
> -				     struct params *params)
> +static int __init efi_get_fdt_prop(const void *fdt, int node, int dtp, int pp,
> +				   void *var, int size)
>  {
>  	const void *prop;
> -	void *dest;
> +	int len;
>  	u64 val;
> -	int i, len;
>  
> -	for (i = 0; i < EFI_FDT_PARAMS_SIZE; i++) {
> -		prop = of_get_flat_dt_prop(node, params[i].propname, &len);
> -		if (!prop) {
> -			info->missing = params[i].name;
> -			return 0;
> -		}
> -
> -		dest = info->params + params[i].offset;
> -		info->found++;
> +	prop = fdt_getprop(fdt, node, dt_params[dtp].params[pp], &len);
> +	if (!prop) {
> +		pr_err("Can't find property '%s' in device tree!\n",
> +		       dt_params[dtp].params[pp]);
> +		return 1;
> +	}
>  
> -		val = of_read_number(prop, len / sizeof(u32));
> +	val = (len == 4) ? (u64)be32_to_cpup(prop) : get_unaligned_be64(prop);
>  
> -		if (params[i].size == sizeof(u32))
> -			*(u32 *)dest = val;
> -		else
> -			*(u64 *)dest = val;
> +	if (size == 8)
> +		*(u64 *)var = val;
> +	else
> +		*(u32 *)var = (val <= U32_MAX) ? val : U32_MAX; // saturate
>  
> -		if (efi_enabled(EFI_DBG))
> -			pr_info("  %s: 0x%0*llx\n", params[i].name,
> -				params[i].size * 2, val);
> -	}
> +	if (efi_enabled(EFI_DBG))
> +		pr_info("  %s: 0x%0*llx\n", name[pp], size * 2, val);
>  
> -	return 1;
> +	return 0;
>  }
>  
> -static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
> -				       int depth, void *data)
> +u64 __init efi_get_fdt_params(struct efi_memory_map_data *mm)
>  {
> -	struct param_info *info = data;
> +	const void *fdt = initial_boot_params;
> +	unsigned long systab;
> +	struct {
> +		void	*var;
> +		int	size;
> +	} target[] = {
> +		[SYSTAB] = { &systab,		sizeof(systab) },
> +		[MMBASE] = { &mm->phys_map,	sizeof(mm->phys_map) },
> +		[MMSIZE] = { &mm->size,		sizeof(mm->size) },
> +		[DCSIZE] = { &mm->desc_size,	sizeof(mm->desc_size) },
> +		[DCVERS] = { &mm->desc_version,	sizeof(mm->desc_version) },
> +	};
>  	int i;
>  
> +	BUILD_BUG_ON(ARRAY_SIZE(target) != ARRAY_SIZE(name));
> +	BUILD_BUG_ON(ARRAY_SIZE(target) != ARRAY_SIZE(dt_params[0].params));
> +
>  	for (i = 0; i < ARRAY_SIZE(dt_params); i++) {
> -		const char *subnode = dt_params[i].subnode;
> +		int node;
> +		int j;
>  
> -		if (depth != 1 || strcmp(uname, dt_params[i].uname) != 0) {
> -			info->missing = dt_params[i].params[0].name;
> +		node = fdt_path_offset(fdt, dt_params[i].path);
> +		if (node < 0)
>  			continue;
> -		}
>  
> -		if (subnode) {
> -			int err = of_get_flat_dt_subnode_by_name(node, subnode);
> +		if (efi_enabled(EFI_DBG))
> +			pr_info("Getting UEFI parameters from %s in DT:\n",
> +				dt_params[i].path);
>  
> -			if (err < 0)
> +		for (j = 0; j < ARRAY_SIZE(target); j++) {
> +			if (efi_get_fdt_prop(fdt, node, i, j, target[j].var,
> +					     target[j].size)) {
>  				return 0;
> -
> -			node = err;
> +			}
>  		}
> -
> -		return __find_uefi_params(node, info, dt_params[i].params);
> +		return systab;
>  	}
> -
> +	pr_info("UEFI not found.\n");
>  	return 0;
>  }
> -
> -u64 __init efi_get_fdt_params(struct efi_memory_map_data *memmap)
> -{
> -	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");
> -		return 0;
> -	} else if (!ret) {
> -		pr_err("Can't find '%s' in device tree!\n", info.missing);
> -		return 0;
> -	}
> -
> -	memmap->desc_version	= params.desc_ver;
> -	memmap->desc_size	= params.desc_size;
> -	memmap->size		= params.mmap_size;
> -	memmap->phys_map	= params.mmap;
> -
> -	return params.system_table;
> -}
> -- 
> 2.17.1
> 

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

* Re: [PATCH 3/3] efi/arm: rewrite FDT param discovery routines
  2020-02-20 18:30   ` Leif Lindholm
@ 2020-02-20 19:20     ` Ard Biesheuvel
  0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2020-02-20 19:20 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: linux-efi, linux-arm-kernel, Atish Patra

On Thu, 20 Feb 2020 at 19:30, Leif Lindholm <leif@nuviainc.com> wrote:
>
> On Wed, Feb 19, 2020 at 16:24:40 +0100, Ard Biesheuvel wrote:
> > The efi_get_fdt_params() routine uses the early OF device tree
> > traversal helpers, that iterate over each node in the DT and invoke
> > a caller provided callback that can inspect the node's contents and
> > look for the required data. This requires a special param struct to
> > be passed around, with pointers into param enumeration structs that
> > contain (and duplicate) property names and offsets into yet another
> > struct that carries the collected data.
> >
> > Since we know the data we look for is either under /hypervisor/uefi
> > or under /chosen, it is much simpler to use the libfdt routines
> > directly, and try to grab a reference to either node directly, and
> > read each property in sequence.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  drivers/firmware/efi/fdtparams.c | 203 ++++++++------------
> >  1 file changed, 85 insertions(+), 118 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/fdtparams.c b/drivers/firmware/efi/fdtparams.c
> > index 7a384b307c56..23af4062e913 100644
> > --- a/drivers/firmware/efi/fdtparams.c
> > +++ b/drivers/firmware/efi/fdtparams.c
> > @@ -5,154 +5,121 @@
> >  #include <linux/module.h>
> >  #include <linux/init.h>
> >  #include <linux/efi.h>
> > -#include <linux/of.h>
> > +#include <linux/libfdt.h>
> >  #include <linux/of_fdt.h>
> >
> > -#include <asm/early_ioremap.h>
> > +#include <asm/unaligned.h>
> >
> > -#define UEFI_PARAM(name, prop, field)                           \
> > -     {                                                  \
> > -             { name },                                  \
> > -             { prop },                                  \
> > -             offsetof(struct efi_fdt_params, field),    \
> > -             sizeof_field(struct efi_fdt_params, field) \
> > -     }
> > -
> > -struct efi_fdt_params {
> > -     u64 system_table;
> > -     u64 mmap;
> > -     u32 mmap_size;
> > -     u32 desc_size;
> > -     u32 desc_ver;
> > -};
> > -
> > -struct params {
> > -     const char name[32];
> > -     const char propname[32];
> > -     int offset;
> > -     int size;
> > +enum {
> > +     SYSTAB,
> > +     MMBASE,
> > +     MMSIZE,
> > +     DCSIZE,
> > +     DCVERS,
> >  };
> >
> > -static __initdata struct params fdt_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)
> > +static __initconst const char name[][22] = {
>
> I was going to complain about that 22, and I still think it looks a
> bit iffy, but I can't find a compiler that doesn't throw a warning if
> the value changes, so I guess I'm being over sensitive.
>

Yeah, 32 is just as arbitrary, and wastes more space :-)

> > +     [SYSTAB] = "System Table         ",
> > +     [MMBASE] = "MemMap Address       ",
> > +     [MMSIZE] = "MemMap Size          ",
> > +     [DCSIZE] = "MemMap Desc. Size    ",
> > +     [DCVERS] = "MemMap Desc. Version ",
> >  };
> >
> > -static __initdata struct params xen_fdt_params[] = {
> > -     UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
> > -     UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
> > -     UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
> > -     UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", desc_size),
> > -     UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", desc_ver)
> > -};
> > -
> > -#define EFI_FDT_PARAMS_SIZE  ARRAY_SIZE(fdt_params)
> > -
> > -static __initdata struct {
> > -     const char *uname;
> > -     const char *subnode;
> > -     struct params *params;
> > +static __initconst const struct {
> > +     const char      path[17];
> > +     const char      params[5][26];
>
> And I guess the same is true here, but the 5 *could* theoretically be
> replaced by a compile-time calculation of DCVERS - SYSTAB + 1. That
> may not be less error prone however. Finishing the enum on a
> somethingMAX entry might make it slightly better?
>

Yeah that make sense - I'll change that.

> But the error checking isn't really what I'm after, but getting rid of
> the opaqueness. Could we have comments about which specific string
> lengths we're matching against here for us bears of very little brain?
>

Sure.


> >  } dt_params[] = {
> > -     { "hypervisor", "uefi", xen_fdt_params },
> > -     { "chosen", NULL, fdt_params },
> > -};
> > -
> > -struct param_info {
> > -     int found;
> > -     void *params;
> > -     const char *missing;
> > +#ifdef CONFIG_XEN
> > +     {
> > +             "/hypervisor/uefi",
> > +             {
> > +                     [SYSTAB] = "xen,uefi-system-table",
> > +                     [MMBASE] = "xen,uefi-mmap-start",
> > +                     [MMSIZE] = "xen,uefi-mmap-size",
> > +                     [DCSIZE] = "xen,uefi-mmap-desc-size",
> > +                     [DCVERS] = "xen,uefi-mmap-desc-ver",
> > +             }
> > +     },
> > +#endif
> > +     {
> > +             "/chosen",
> > +             {
> > +                     [SYSTAB] = "linux,uefi-system-table",
> > +                     [MMBASE] = "linux,uefi-mmap-start",
> > +                     [MMSIZE] = "linux,uefi-mmap-size",
> > +                     [DCSIZE] = "linux,uefi-mmap-desc-size",
> > +                     [DCVERS] = "linux,uefi-mmap-desc-ver",
> > +             }
> > +     }
> >  };
> >
> > -static int __init __find_uefi_params(unsigned long node,
> > -                                  struct param_info *info,
> > -                                  struct params *params)
> > +static int __init efi_get_fdt_prop(const void *fdt, int node, int dtp, int pp,
> > +                                void *var, int size)
> >  {
> >       const void *prop;
> > -     void *dest;
> > +     int len;
> >       u64 val;
> > -     int i, len;
> >
> > -     for (i = 0; i < EFI_FDT_PARAMS_SIZE; i++) {
> > -             prop = of_get_flat_dt_prop(node, params[i].propname, &len);
> > -             if (!prop) {
> > -                     info->missing = params[i].name;
> > -                     return 0;
> > -             }
> > -
> > -             dest = info->params + params[i].offset;
> > -             info->found++;
> > +     prop = fdt_getprop(fdt, node, dt_params[dtp].params[pp], &len);
> > +     if (!prop) {
> > +             pr_err("Can't find property '%s' in device tree!\n",
> > +                    dt_params[dtp].params[pp]);
> > +             return 1;
> > +     }
> >
> > -             val = of_read_number(prop, len / sizeof(u32));
> > +     val = (len == 4) ? (u64)be32_to_cpup(prop) : get_unaligned_be64(prop);
> >
> > -             if (params[i].size == sizeof(u32))
> > -                     *(u32 *)dest = val;
> > -             else
> > -                     *(u64 *)dest = val;
> > +     if (size == 8)
> > +             *(u64 *)var = val;
> > +     else
> > +             *(u32 *)var = (val <= U32_MAX) ? val : U32_MAX; // saturate
> >
> > -             if (efi_enabled(EFI_DBG))
> > -                     pr_info("  %s: 0x%0*llx\n", params[i].name,
> > -                             params[i].size * 2, val);
> > -     }
> > +     if (efi_enabled(EFI_DBG))
> > +             pr_info("  %s: 0x%0*llx\n", name[pp], size * 2, val);
> >
> > -     return 1;
> > +     return 0;
> >  }
> >
> > -static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
> > -                                    int depth, void *data)
> > +u64 __init efi_get_fdt_params(struct efi_memory_map_data *mm)
> >  {
> > -     struct param_info *info = data;
> > +     const void *fdt = initial_boot_params;
> > +     unsigned long systab;
> > +     struct {
> > +             void    *var;
> > +             int     size;
> > +     } target[] = {
> > +             [SYSTAB] = { &systab,           sizeof(systab) },
> > +             [MMBASE] = { &mm->phys_map,     sizeof(mm->phys_map) },
> > +             [MMSIZE] = { &mm->size,         sizeof(mm->size) },
> > +             [DCSIZE] = { &mm->desc_size,    sizeof(mm->desc_size) },
> > +             [DCVERS] = { &mm->desc_version, sizeof(mm->desc_version) },
> > +     };
> >       int i;
> >
> > +     BUILD_BUG_ON(ARRAY_SIZE(target) != ARRAY_SIZE(name));
> > +     BUILD_BUG_ON(ARRAY_SIZE(target) != ARRAY_SIZE(dt_params[0].params));
> > +
> >       for (i = 0; i < ARRAY_SIZE(dt_params); i++) {
> > -             const char *subnode = dt_params[i].subnode;
> > +             int node;
> > +             int j;
> >
> > -             if (depth != 1 || strcmp(uname, dt_params[i].uname) != 0) {
> > -                     info->missing = dt_params[i].params[0].name;
> > +             node = fdt_path_offset(fdt, dt_params[i].path);
> > +             if (node < 0)
> >                       continue;
> > -             }
> >
> > -             if (subnode) {
> > -                     int err = of_get_flat_dt_subnode_by_name(node, subnode);
> > +             if (efi_enabled(EFI_DBG))
> > +                     pr_info("Getting UEFI parameters from %s in DT:\n",
> > +                             dt_params[i].path);
> >
> > -                     if (err < 0)
> > +             for (j = 0; j < ARRAY_SIZE(target); j++) {
> > +                     if (efi_get_fdt_prop(fdt, node, i, j, target[j].var,
> > +                                          target[j].size)) {
> >                               return 0;
> > -
> > -                     node = err;
> > +                     }
> >               }
> > -
> > -             return __find_uefi_params(node, info, dt_params[i].params);
> > +             return systab;
> >       }
> > -
> > +     pr_info("UEFI not found.\n");
> >       return 0;
> >  }
> > -
> > -u64 __init efi_get_fdt_params(struct efi_memory_map_data *memmap)
> > -{
> > -     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");
> > -             return 0;
> > -     } else if (!ret) {
> > -             pr_err("Can't find '%s' in device tree!\n", info.missing);
> > -             return 0;
> > -     }
> > -
> > -     memmap->desc_version    = params.desc_ver;
> > -     memmap->desc_size       = params.desc_size;
> > -     memmap->size            = params.mmap_size;
> > -     memmap->phys_map        = params.mmap;
> > -
> > -     return params.system_table;
> > -}
> > --
> > 2.17.1
> >

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

end of thread, other threads:[~2020-02-20 19:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-19 15:24 [PATCH 0/3] efi/arm: refactor DT EFI param parsing Ard Biesheuvel
2020-02-19 15:24 ` [PATCH 1/3] efi/arm: move FDT param discovery code out of efi.c Ard Biesheuvel
2020-02-19 15:24 ` [PATCH 2/3] efi/arm: move FDT specific definitions into fdtparams.c Ard Biesheuvel
2020-02-19 15:24 ` [PATCH 3/3] efi/arm: rewrite FDT param discovery routines Ard Biesheuvel
2020-02-20 18:30   ` Leif Lindholm
2020-02-20 19:20     ` Ard Biesheuvel

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