[U-Boot] [PATCH v2 05/71] dm: Add a function to create a 'live' device tree

Simon Glass sjg at chromium.org
Tue May 16 00:18:39 UTC 2017


Hi Lothar,

On 15 May 2017 at 01:34, Lothar Waßmann <LW at karo-electronics.de> wrote:
> Hi,
>
> On Sun, 14 May 2017 21:03:23 -0600 Simon Glass wrote:
>> Hi Lothar,
>>
>> On 11 May 2017 at 08:59, Lothar Waßmann <LW at karo-electronics.de> wrote:
>> > Hi,
>> >
>> > On Wed, 10 May 2017 08:20:44 -0600 Simon Glass wrote:
>> >> This function converts the flat device tree into a hierarchical one with
>> >> C structures and pointers. This is easier to access.
>> >>
>> >> Signed-off-by: Simon Glass <sjg at chromium.org>
>> >> ---
>> >>
>> >> Changes in v2: None
>> >>
>> >>  include/of_live.h |  24 ++++
>> >>  lib/Makefile      |   1 +
>> >>  lib/of_live.c     | 333 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >>  3 files changed, 358 insertions(+)
>> >>  create mode 100644 include/of_live.h
>> >>  create mode 100644 lib/of_live.c
>> >>
>> >> diff --git a/include/of_live.h b/include/of_live.h
>> >> new file mode 100644
>> >> index 0000000000..f5303bb018
>> >> --- /dev/null
>> >> +++ b/include/of_live.h
>> >> @@ -0,0 +1,24 @@
>> >> +/*
>> >> + * Copyright (c) 2017 Google, Inc
>> >> + * Written by Simon Glass <sjg at chromium.org>
>> >> + *
>> >> + * SPDX-License-Identifier:  GPL-2.0+
>> >> + *
>> >> + * Support for a 'live' (as opposed to flat) device tree
>> >> + */
>> >> +
>> >> +#ifndef _OF_LIVE_H
>> >> +#define _OF_LIVE_H
>> >> +
>> >> +struct device_node;
>> >> +
>> >> +/**
>> >> + * of_live_build() - build a live (hierarchical) tree from a flat DT
>> >> + *
>> >> + * @fdt_blob: Input tree to convert
>> >> + * @rootp: Returns live tree that was created
>> >> + * @return 0 if OK, -ve on error
>> >> + */
>> >> +int of_live_build(const void *fdt_blob, struct device_node **rootp);
>> >> +
>> >> +#endif
>> >> diff --git a/lib/Makefile b/lib/Makefile
>> >> index 23e9f1ef11..bc2fb0a361 100644
>> >> --- a/lib/Makefile
>> >> +++ b/lib/Makefile
>> >> @@ -15,6 +15,7 @@ obj-$(CONFIG_ZLIB) += zlib/
>> >>  obj-$(CONFIG_BZIP2) += bzip2/
>> >>  obj-$(CONFIG_TIZEN) += tizen/
>> >>  obj-$(CONFIG_FIT) += libfdt/
>> >> +obj-$(CONFIG_OF_LIVE) += of_live.o
>> >>  obj-$(CONFIG_CMD_DHRYSTONE) += dhry/
>> >>
>> >>  obj-$(CONFIG_AES) += aes.o
>> >> diff --git a/lib/of_live.c b/lib/of_live.c
>> >> new file mode 100644
>> >> index 0000000000..51927f9e91
>> >> --- /dev/null
>> >> +++ b/lib/of_live.c
>> >> @@ -0,0 +1,333 @@
>> >> +/*
>> >> + * Copyright 2009 Benjamin Herrenschmidt, IBM Corp
>> >> + * benh at kernel.crashing.org
>> >> + *
>> >> + * Based on parts of drivers/of/fdt.c from Linux v4.9
>> >> + * Modifications for U-Boot
>> >> + * Copyright (c) 2017 Google, Inc
>> >> + *
>> >> + * SPDX-License-Identifier:  GPL-2.0+
>> >> + */
>> >> +
>> >> +#include <common.h>
>> >> +#include <libfdt.h>
>> >> +#include <of_live.h>
>> >> +#include <malloc.h>
>> >> +#include <dm/of_access.h>
>> >> +#include <linux/err.h>
>> >> +
>> >> +DECLARE_GLOBAL_DATA_PTR;
>> >> +
>> >> +static void *unflatten_dt_alloc(void **mem, unsigned long size,
>> >> +                             unsigned long align)
>> >> +{
>> >> +     void *res;
>> >> +
>> >> +     *mem = PTR_ALIGN(*mem, align);
>> >> +     res = *mem;
>> >> +     *mem += size;
>> >> +
>> >> +     return res;
>> >> +}
>> >> +
>> >> +/**
>> >> + * unflatten_dt_node() - Alloc and populate a device_node from the flat tree
>> >> + * @blob: The parent device tree blob
>> >> + * @mem: Memory chunk to use for allocating device nodes and properties
>> >> + * @poffset: pointer to node in flat tree
>> >> + * @dad: Parent struct device_node
>> >> + * @nodepp: The device_node tree created by the call
>> >> + * @fpsize: Size of the node path up at t05he current depth.
>> >> + * @dryrun: If true, do not allocate device nodes but still calculate needed
>> >> + * memory size
>> >> + */
>> >> +static void *unflatten_dt_node(const void *blob, void *mem, int *poffset,
>> >> +                            struct device_node *dad,
>> >> +                            struct device_node **nodepp,
>> >> +                            unsigned long fpsize, bool dryrun)
>> >> +{
>> >> +     const __be32 *p;
>> >> +     struct device_node *np;
>> >> +     struct property *pp, **prev_pp = NULL;
>> >> +     const char *pathp;
>> >> +     int l;
>> >> +     unsigned int allocl;
>> >> +     static int depth;
>> >> +     int old_depth;
>> >> +     int offset;
>> >> +     int has_name = 0;
>> >> +     int new_format = 0;
>> >> +
>> >> +     pathp = fdt_get_name(blob, *poffset, &l);
>> >> +     if (!pathp)
>> >> +             return mem;
>> >> +
>> >> +     allocl = ++l;
>> >> +
>> >> +     /*
>> >> +      * version 0x10 has a more compact unit name here instead of the full
>> >> +      * path. we accumulate the full path size using "fpsize", we'll rebuild
>> >> +      * it later. We detect this because the first character of the name is
>> >> +      * not '/'.
>> >> +      */
>> >> +     if ((*pathp) != '/') {
>> >> +             new_format = 1;
>> >> +             if (fpsize == 0) {
>> >> +                     /*
>> >> +                      * root node: special case. fpsize accounts for path
>> >> +                      * plus terminating zero. root node only has '/', so
>> >> +                      * fpsize should be 2, but we want to avoid the first
>> >> +                      * level nodes to have two '/' so we use fpsize 1 here
>> >> +                      */
>> >> +                     fpsize = 1;
>> >> +                     allocl = 2;
>> >> +                     l = 1;
>> >> +                     pathp = "";
>> >> +             } else {
>> >> +                     /*
>> >> +                      * account for '/' and path size minus terminal 0
>> >> +                      * already in 'l'
>> >> +                      */
>> >> +                     fpsize += l;
>> >> +                     allocl = fpsize;
>> >> +             }
>> >> +     }
>> >> +
>> >> +     np = unflatten_dt_alloc(&mem, sizeof(struct device_node) + allocl,
>> >> +                             __alignof__(struct device_node));
>> >> +     if (!dryrun) {
>> >> +             char *fn;
>> >> +
>> >> +             fn = (char *)np + sizeof(*np);
>> >> +             np->full_name = fn;
>> >> +             if (new_format) {
>> >> +                     /* rebuild full path for new format */
>> >> +                     if (dad && dad->parent) {
>> >> +                             strcpy(fn, dad->full_name);
>> >>
>> > strcpy() is EVIL! How do you ascertain, that the string
>> > at dad->full_name cannot overflow the buffer at fn?
>>
>> The size has been calculated the code above. This is a two-pass
>> approach - one pass figures out the total size of the required buffer,
>> and the next pass actually copies in the data.
>>
>> If the length were wrong that would indicate a bug in the code above.
>> See the part where it sets new_format to 1.
>>
> dad->full_name is touched nowhere else in the code, so it is not
> apparent from the code that its strlen() is accounted for in any way.

Well it is somewhat strange code, but I've brought it in from Linux
and am trying to avoid changing it too much.

Regards,
Simon


More information about the U-Boot mailing list