diff mbox series

[3/3] efi/arm: rewrite FDT param discovery routines

Message ID 20200219152440.11561-4-ardb@kernel.org (mailing list archive)
State New, archived
Headers show
Series efi/arm: refactor DT EFI param parsing | expand

Commit Message

Ard Biesheuvel Feb. 19, 2020, 3:24 p.m. UTC
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(-)

Comments

Leif Lindholm Feb. 20, 2020, 6:30 p.m. UTC | #1
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
>
Ard Biesheuvel Feb. 20, 2020, 7:20 p.m. UTC | #2
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
> >
diff mbox series

Patch

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;
-}