[U-Boot] [PATCH 0/5] libfdt: make lib/libfdt/ more synced with scripts/dtc/libfdt/

My goal is to make lib/libfdt/ only contain wrapper files to scripts/dtc/libfdt/.
This will ease to pull-in improvement from the upstream. Linux also does this; scripts/dtc/ is the mirror to the upstream DTC.
U-Boot has lib/libfdt/fdt_region.c, but it is not a part of libfdt. I want to move it out of lib/libfdt/ directory.
Masahiro Yamada (5): libfdt: move FDT_RAMDISK_OVERHEAD to image-fdt.c fdt_region: remove unneeded fdt_internal.h inclusion image.h: add forward declaration of struct fdt_region fdt_region: move fdt_region.c to common/ from lib/libfdt/ libfdt: split fdt_region declarations out to <fdt_region.h>
common/Makefile | 2 + {lib/libfdt => common}/fdt_region.c | 3 +- common/image-fdt.c | 3 + common/image-sig.c | 1 + include/fdt_region.h | 303 ++++++++++++++++++++++++++++++++++++ include/image.h | 1 + include/linux/libfdt.h | 302 ----------------------------------- lib/libfdt/Makefile | 3 - tools/Makefile | 5 +- tools/fdtgrep.c | 1 + tools/image-host.c | 1 + 11 files changed, 316 insertions(+), 309 deletions(-) rename {lib/libfdt => common}/fdt_region.c (99%) create mode 100644 include/fdt_region.h

This macro is locally referenced in common/image-fdt.c
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
common/image-fdt.c | 3 +++ include/linux/libfdt.h | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/common/image-fdt.c b/common/image-fdt.c index 25103ba..3dc02a1 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -21,6 +21,9 @@ #define CONFIG_SYS_FDT_PAD 0x3000 #endif
+/* adding a ramdisk needs 0x44 bytes in version 2008.10 */ +#define FDT_RAMDISK_OVERHEAD 0x80 + DECLARE_GLOBAL_DATA_PTR;
static void fdt_error(const char *msg) diff --git a/include/linux/libfdt.h b/include/linux/libfdt.h index 9e6eead..eeb2344 100644 --- a/include/linux/libfdt.h +++ b/include/linux/libfdt.h @@ -309,7 +309,4 @@ int fdt_add_alias_regions(const void *fdt, struct fdt_region *region, int count,
extern struct fdt_header *working_fdt; /* Pointer to the working fdt */
-/* adding a ramdisk needs 0x44 bytes in version 2008.10 */ -#define FDT_RAMDISK_OVERHEAD 0x80 - #endif /* _INCLUDE_LIBFDT_H_ */

On 21 March 2018 at 03:03, Masahiro Yamada yamada.masahiro@socionext.com wrote:
This macro is locally referenced in common/image-fdt.c
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
common/image-fdt.c | 3 +++ include/linux/libfdt.h | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On 23 March 2018 at 22:30, Simon Glass sjg@chromium.org wrote:
On 21 March 2018 at 03:03, Masahiro Yamada yamada.masahiro@socionext.com wrote:
This macro is locally referenced in common/image-fdt.c
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
common/image-fdt.c | 3 +++ include/linux/libfdt.h | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm, thanks!

fdt_region.c does not depend on anything in libfdt_internal.h
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
lib/libfdt/fdt_region.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/lib/libfdt/fdt_region.c b/lib/libfdt/fdt_region.c index 70914a4..054c4b3 100644 --- a/lib/libfdt/fdt_region.c +++ b/lib/libfdt/fdt_region.c @@ -14,8 +14,6 @@ #include "fdt_host.h" #endif
-#include "libfdt_internal.h" - #define FDT_MAX_DEPTH 32
static int str_in_list(const char *str, char * const list[], int count)

On 21 March 2018 at 03:03, Masahiro Yamada yamada.masahiro@socionext.com wrote:
fdt_region.c does not depend on anything in libfdt_internal.h
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
lib/libfdt/fdt_region.c | 2 -- 1 file changed, 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On 23 March 2018 at 22:30, Simon Glass sjg@chromium.org wrote:
On 21 March 2018 at 03:03, Masahiro Yamada yamada.masahiro@socionext.com wrote:
fdt_region.c does not depend on anything in libfdt_internal.h
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
lib/libfdt/fdt_region.c | 2 -- 1 file changed, 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm, thanks!

This header needs to know 'fdt_region' is a struct for the fit_region_make_list() prototype.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
include/image.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/image.h b/include/image.h index 621abf6..a6f82ae 100644 --- a/include/image.h +++ b/include/image.h @@ -21,6 +21,7 @@
/* Define this to avoid #ifdefs later on */ struct lmb; +struct fdt_region;
#ifdef USE_HOSTCC #include <sys/types.h>

On 21 March 2018 at 03:03, Masahiro Yamada yamada.masahiro@socionext.com wrote:
This header needs to know 'fdt_region' is a struct for the fit_region_make_list() prototype.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
include/image.h | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Simon Glass sjg@chromium.org

On 23 March 2018 at 22:30, Simon Glass sjg@chromium.org wrote:
On 21 March 2018 at 03:03, Masahiro Yamada yamada.masahiro@socionext.com wrote:
This header needs to know 'fdt_region' is a struct for the fit_region_make_list() prototype.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
include/image.h | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm, thanks!

My goal is to sync lib/libfdt/ with scripts/dtc/libfdt/, that is, make lib/libfdt/ contain only wrapper files.
fdt_region.c was written only for U-Boot to implement the verified boot. So, this belongs to the same group as common/fdt_support.c, which is a collection of U-Boot own fdt helpers.
Move lib/libfdt/fdt_region.c to common/fdt_region.c . This is necessary only when CONFIG_(SPL_)_FIT_SIGNATURE is enabled.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
common/Makefile | 2 ++ {lib/libfdt => common}/fdt_region.c | 0 lib/libfdt/Makefile | 3 --- tools/Makefile | 5 +++-- 4 files changed, 5 insertions(+), 5 deletions(-) rename {lib/libfdt => common}/fdt_region.c (100%)
diff --git a/common/Makefile b/common/Makefile index 7011dad..bf1713b 100644 --- a/common/Makefile +++ b/common/Makefile @@ -31,6 +31,7 @@ obj-$(CONFIG_CMD_BOOTI) += bootm.o bootm_os.o
obj-$(CONFIG_CMD_BEDBUG) += bedbug.o obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += fdt_support.o +obj-$(CONFIG_FIT_SIGNATURE) += fdt_region.o
obj-$(CONFIG_MII) += miiphyutil.o obj-$(CONFIG_CMD_MII) += miiphyutil.o @@ -75,6 +76,7 @@ obj-$(CONFIG_SPL_YMODEM_SUPPORT) += xyzModem.o obj-$(CONFIG_SPL_LOAD_FIT) += common_fit.o obj-$(CONFIG_SPL_NET_SUPPORT) += miiphyutil.o obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += fdt_support.o +obj-$(CONFIG_SPL_FIT_SIGNATURE) += fdt_region.o ifdef CONFIG_SPL_USB_HOST_SUPPORT obj-$(CONFIG_SPL_USB_SUPPORT) += usb.o usb_hub.o obj-$(CONFIG_USB_STORAGE) += usb_storage.o diff --git a/lib/libfdt/fdt_region.c b/common/fdt_region.c similarity index 100% rename from lib/libfdt/fdt_region.c rename to common/fdt_region.c diff --git a/lib/libfdt/Makefile b/lib/libfdt/Makefile index edd8e64..dd30d52 100644 --- a/lib/libfdt/Makefile +++ b/lib/libfdt/Makefile @@ -21,7 +21,4 @@ obj-$(CONFIG_OF_LIBFDT_OVERLAY) += fdt_overlay.o # TODO: split out the local modifiction. obj-y += fdt_ro.o
-# U-Boot own file -obj-y += fdt_region.o - ccflags-y := -I$(srctree)/scripts/dtc/libfdt diff --git a/tools/Makefile b/tools/Makefile index 55efb74..e7f701a 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -67,7 +67,7 @@ LIBFDT_SRCS_SYNCED := fdt.c fdt_wip.c fdt_sw.c fdt_rw.c \ fdt_strerror.c fdt_empty_tree.c fdt_addresses.c fdt_overlay.c # The following files are locally modified for U-Boot (unfotunately). # Use U-Boot own versions from lib/libfdt/. -LIBFDT_SRCS_UNSYNCED := fdt_ro.c fdt_region.c +LIBFDT_SRCS_UNSYNCED := fdt_ro.c
LIBFDT_OBJS := $(addprefix libfdt/, $(patsubst %.c, %.o, $(LIBFDT_SRCS_SYNCED))) \ $(addprefix lib/libfdt/, $(patsubst %.c, %.o, $(LIBFDT_SRCS_UNSYNCED))) @@ -82,6 +82,7 @@ ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o dumpimage-mkimage-objs := aisimage.o \ atmelimage.o \ $(FIT_SIG_OBJS-y) \ + common/fdt_region.o \ common/bootm.o \ lib/crc32.o \ default_image.o \ @@ -192,7 +193,7 @@ hostprogs-$(CONFIG_STATIC_RELA) += relocate-rela hostprogs-$(CONFIG_RISCV) += prelink-riscv
hostprogs-y += fdtgrep -fdtgrep-objs += $(LIBFDT_OBJS) fdtgrep.o +fdtgrep-objs += $(LIBFDT_OBJS) common/fdt_region.o fdtgrep.o
hostprogs-$(CONFIG_MIPS) += mips-relocs

Hi Masahiro,
On 21 March 2018 at 03:03, Masahiro Yamada yamada.masahiro@socionext.com wrote:
My goal is to sync lib/libfdt/ with scripts/dtc/libfdt/, that is, make lib/libfdt/ contain only wrapper files.
fdt_region.c was written only for U-Boot to implement the verified boot. So, this belongs to the same group as common/fdt_support.c, which is a collection of U-Boot own fdt helpers.
Actually it was written for the fdtgrep tool as well, which was sent upstream, but not accepted.
Perhaps I should try to send that again.
Move lib/libfdt/fdt_region.c to common/fdt_region.c . This is necessary only when CONFIG_(SPL_)_FIT_SIGNATURE is enabled.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
common/Makefile | 2 ++ {lib/libfdt => common}/fdt_region.c | 0 lib/libfdt/Makefile | 3 --- tools/Makefile | 5 +++-- 4 files changed, 5 insertions(+), 5 deletions(-) rename {lib/libfdt => common}/fdt_region.c (100%)
Regards, Simon

fdt_region APIs are not part of libfdt. They are U-Boot extension for the verified boot. Split the declartions related to fdt_region out ot <fdt_region.h>. This allows <linux/libfdt.h> to become a simple wrapper file, like Linux does.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
common/fdt_region.c | 1 + common/image-sig.c | 1 + include/fdt_region.h | 303 +++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/libfdt.h | 299 ------------------------------------------------ tools/fdtgrep.c | 1 + tools/image-host.c | 1 + 6 files changed, 307 insertions(+), 299 deletions(-) create mode 100644 include/fdt_region.h
diff --git a/common/fdt_region.c b/common/fdt_region.c index 054c4b3..4a02ef4 100644 --- a/common/fdt_region.c +++ b/common/fdt_region.c @@ -6,6 +6,7 @@ */
#include <linux/libfdt_env.h> +#include <fdt_region.h>
#ifndef USE_HOSTCC #include <fdt.h> diff --git a/common/image-sig.c b/common/image-sig.c index d9f712f..921f378 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -12,6 +12,7 @@ #include <malloc.h> DECLARE_GLOBAL_DATA_PTR; #endif /* !USE_HOSTCC*/ +#include <fdt_region.h> #include <image.h> #include <u-boot/rsa.h> #include <u-boot/rsa-checksum.h> diff --git a/include/fdt_region.h b/include/fdt_region.h new file mode 100644 index 0000000..2a5ca7d --- /dev/null +++ b/include/fdt_region.h @@ -0,0 +1,303 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _FDT_REGION_H +#define _FDT_REGION_H + +#ifndef SWIG /* Not available in Python */ +struct fdt_region { + int offset; + int size; +}; + +/* + * Flags for fdt_find_regions() + * + * Add a region for the string table (always the last region) + */ +#define FDT_REG_ADD_STRING_TAB (1 << 0) + +/* + * Add all supernodes of a matching node/property, useful for creating a + * valid subset tree + */ +#define FDT_REG_SUPERNODES (1 << 1) + +/* Add the FDT_BEGIN_NODE tags of subnodes, including their names */ +#define FDT_REG_DIRECT_SUBNODES (1 << 2) + +/* Add all subnodes of a matching node */ +#define FDT_REG_ALL_SUBNODES (1 << 3) + +/* Add a region for the mem_rsvmap table (always the first region) */ +#define FDT_REG_ADD_MEM_RSVMAP (1 << 4) + +/* Indicates what an fdt part is (node, property, value) */ +#define FDT_IS_NODE (1 << 0) +#define FDT_IS_PROP (1 << 1) +#define FDT_IS_VALUE (1 << 2) /* not supported */ +#define FDT_IS_COMPAT (1 << 3) /* used internally */ +#define FDT_NODE_HAS_PROP (1 << 4) /* node contains prop */ + +#define FDT_ANY_GLOBAL (FDT_IS_NODE | FDT_IS_PROP | FDT_IS_VALUE | \ + FDT_IS_COMPAT) +#define FDT_IS_ANY 0x1f /* all the above */ + +/* We set a reasonable limit on the number of nested nodes */ +#define FDT_MAX_DEPTH 32 + +/* Decribes what we want to include from the current tag */ +enum want_t { + WANT_NOTHING, + WANT_NODES_ONLY, /* No properties */ + WANT_NODES_AND_PROPS, /* Everything for one level */ + WANT_ALL_NODES_AND_PROPS /* Everything for all levels */ +}; + +/* Keeps track of the state at parent nodes */ +struct fdt_subnode_stack { + int offset; /* Offset of node */ + enum want_t want; /* The 'want' value here */ + int included; /* 1 if we included this node, 0 if not */ +}; + +struct fdt_region_ptrs { + int depth; /* Current tree depth */ + int done; /* What we have completed scanning */ + enum want_t want; /* What we are currently including */ + char *end; /* Pointer to end of full node path */ + int nextoffset; /* Next node offset to check */ +}; + +/* The state of our finding algortihm */ +struct fdt_region_state { + struct fdt_subnode_stack stack[FDT_MAX_DEPTH]; /* node stack */ + struct fdt_region *region; /* Contains list of regions found */ + int count; /* Numnber of regions found */ + const void *fdt; /* FDT blob */ + int max_regions; /* Maximum regions to find */ + int can_merge; /* 1 if we can merge with previous region */ + int start; /* Start position of current region */ + struct fdt_region_ptrs ptrs; /* Pointers for what we are up to */ +}; + +/** + * fdt_find_regions() - find regions in device tree + * + * Given a list of nodes to include and properties to exclude, find + * the regions of the device tree which describe those included parts. + * + * The intent is to get a list of regions which will be invariant provided + * those parts are invariant. For example, if you request a list of regions + * for all nodes but exclude the property "data", then you will get the + * same region contents regardless of any change to "data" properties. + * + * This function can be used to produce a byte-stream to send to a hashing + * function to verify that critical parts of the FDT have not changed. + * + * Nodes which are given in 'inc' are included in the region list, as + * are the names of the immediate subnodes nodes (but not the properties + * or subnodes of those subnodes). + * + * For eaxample "/" means to include the root node, all root properties + * and the FDT_BEGIN_NODE and FDT_END_NODE of all subnodes of /. The latter + * ensures that we capture the names of the subnodes. In a hashing situation + * it prevents the root node from changing at all Any change to non-excluded + * properties, names of subnodes or number of subnodes would be detected. + * + * When used with FITs this provides the ability to hash and sign parts of + * the FIT based on different configurations in the FIT. Then it is + * impossible to change anything about that configuration (include images + * attached to the configuration), but it may be possible to add new + * configurations, new images or new signatures within the existing + * framework. + * + * Adding new properties to a device tree may result in the string table + * being extended (if the new property names are different from those + * already added). This function can optionally include a region for + * the string table so that this can be part of the hash too. + * + * The device tree header is not included in the list. + * + * @fdt: Device tree to check + * @inc: List of node paths to included + * @inc_count: Number of node paths in list + * @exc_prop: List of properties names to exclude + * @exc_prop_count: Number of properties in exclude list + * @region: Returns list of regions + * @max_region: Maximum length of region list + * @path: Pointer to a temporary string for the function to use for + * building path names + * @path_len: Length of path, must be large enough to hold the longest + * path in the tree + * @add_string_tab: 1 to add a region for the string table + * @return number of regions in list. If this is >max_regions then the + * region array was exhausted. You should increase max_regions and try + * the call again. + */ +int fdt_find_regions(const void *fdt, char * const inc[], int inc_count, + char * const exc_prop[], int exc_prop_count, + struct fdt_region region[], int max_regions, + char *path, int path_len, int add_string_tab); + +/** + * fdt_first_region() - find regions in device tree + * + * Given a nodes and properties to include and properties to exclude, find + * the regions of the device tree which describe those included parts. + * + * The use for this function is twofold. Firstly it provides a convenient + * way of performing a structure-aware grep of the tree. For example it is + * possible to grep for a node and get all the properties associated with + * that node. Trees can be subsetted easily, by specifying the nodes that + * are required, and then writing out the regions returned by this function. + * This is useful for small resource-constrained systems, such as boot + * loaders, which want to use an FDT but do not need to know about all of + * it. + * + * Secondly it makes it easy to hash parts of the tree and detect changes. + * The intent is to get a list of regions which will be invariant provided + * those parts are invariant. For example, if you request a list of regions + * for all nodes but exclude the property "data", then you will get the + * same region contents regardless of any change to "data" properties. + * + * This function can be used to produce a byte-stream to send to a hashing + * function to verify that critical parts of the FDT have not changed. + * Note that semantically null changes in order could still cause false + * hash misses. Such reordering might happen if the tree is regenerated + * from source, and nodes are reordered (the bytes-stream will be emitted + * in a different order and many hash functions will detect this). However + * if an existing tree is modified using libfdt functions, such as + * fdt_add_subnode() and fdt_setprop(), then this problem is avoided. + * + * The nodes/properties to include/exclude are defined by a function + * provided by the caller. This function is called for each node and + * property, and must return: + * + * 0 - to exclude this part + * 1 - to include this part + * -1 - for FDT_IS_PROP only: no information is available, so include + * if its containing node is included + * + * The last case is only used to deal with properties. Often a property is + * included if its containing node is included - this is the case where + * -1 is returned.. However if the property is specifically required to be + * included/excluded, then 0 or 1 can be returned. Note that including a + * property when the FDT_REG_SUPERNODES flag is given will force its + * containing node to be included since it is not valid to have a property + * that is not in a node. + * + * Using the information provided, the inclusion of a node can be controlled + * either by a node name or its compatible string, or any other property + * that the function can determine. + * + * As an example, including node "/" means to include the root node and all + * root properties. A flag provides a way of also including supernodes (of + * which there is none for the root node), and another flag includes + * immediate subnodes, so in this case we would get the FDT_BEGIN_NODE and + * FDT_END_NODE of all subnodes of /. + * + * The subnode feature helps in a hashing situation since it prevents the + * root node from changing at all. Any change to non-excluded properties, + * names of subnodes or number of subnodes would be detected. + * + * When used with FITs this provides the ability to hash and sign parts of + * the FIT based on different configurations in the FIT. Then it is + * impossible to change anything about that configuration (include images + * attached to the configuration), but it may be possible to add new + * configurations, new images or new signatures within the existing + * framework. + * + * Adding new properties to a device tree may result in the string table + * being extended (if the new property names are different from those + * already added). This function can optionally include a region for + * the string table so that this can be part of the hash too. This is always + * the last region. + * + * The FDT also has a mem_rsvmap table which can also be included, and is + * always the first region if so. + * + * The device tree header is not included in the region list. Since the + * contents of the FDT are changing (shrinking, often), the caller will need + * to regenerate the header anyway. + * + * @fdt: Device tree to check + * @h_include: Function to call to determine whether to include a part or + * not: + * + * @priv: Private pointer as passed to fdt_find_regions() + * @fdt: Pointer to FDT blob + * @offset: Offset of this node / property + * @type: Type of this part, FDT_IS_... + * @data: Pointer to data (node name, property name, compatible + * string, value (not yet supported) + * @size: Size of data, or 0 if none + * @return 0 to exclude, 1 to include, -1 if no information is + * available + * @priv: Private pointer passed to h_include + * @region: Returns list of regions, sorted by offset + * @max_regions: Maximum length of region list + * @path: Pointer to a temporary string for the function to use for + * building path names + * @path_len: Length of path, must be large enough to hold the longest + * path in the tree + * @flags: Various flags that control the region algortihm, see + * FDT_REG_... + * @return number of regions in list. If this is >max_regions then the + * region array was exhausted. You should increase max_regions and try + * the call again. Only the first max_regions elements are available in the + * array. + * + * On error a -ve value is return, which can be: + * + * -FDT_ERR_BADSTRUCTURE (too deep or more END tags than BEGIN tags + * -FDT_ERR_BADLAYOUT + * -FDT_ERR_NOSPACE (path area is too small) + */ +int fdt_first_region(const void *fdt, + int (*h_include)(void *priv, const void *fdt, int offset, + int type, const char *data, int size), + void *priv, struct fdt_region *region, + char *path, int path_len, int flags, + struct fdt_region_state *info); + +/** fdt_next_region() - find next region + * + * See fdt_first_region() for full description. This function finds the + * next region according to the provided parameters, which must be the same + * as passed to fdt_first_region(). + * + * This function can additionally return -FDT_ERR_NOTFOUND when there are no + * more regions + */ +int fdt_next_region(const void *fdt, + int (*h_include)(void *priv, const void *fdt, int offset, + int type, const char *data, int size), + void *priv, struct fdt_region *region, + char *path, int path_len, int flags, + struct fdt_region_state *info); + +/** + * fdt_add_alias_regions() - find aliases that point to existing regions + * + * Once a device tree grep is complete some of the nodes will be present + * and some will have been dropped. This function checks all the alias nodes + * to figure out which points point to nodes which are still present. These + * aliases need to be kept, along with the nodes they reference. + * + * Given a list of regions function finds the aliases that still apply and + * adds more regions to the list for these. This function is called after + * fdt_next_region() has finished returning regions and requires the same + * state. + * + * @fdt: Device tree file to reference + * @region: List of regions that will be kept + * @count: Number of regions + * @max_regions: Number of entries that can fit in @region + * @info: Region state as returned from fdt_next_region() + * @return new number of regions in @region (i.e. count + the number added) + * or -FDT_ERR_NOSPACE if there was not enough space. + */ +int fdt_add_alias_regions(const void *fdt, struct fdt_region *region, int count, + int max_regions, struct fdt_region_state *info); +#endif /* SWIG */ + +#endif /* _FDT_REGION_H */ diff --git a/include/linux/libfdt.h b/include/linux/libfdt.h index eeb2344..39dbc88 100644 --- a/include/linux/libfdt.h +++ b/include/linux/libfdt.h @@ -8,305 +8,6 @@ #include "../../scripts/dtc/libfdt/libfdt.h"
/* U-Boot local hacks */ - -#ifndef SWIG /* Not available in Python */ -struct fdt_region { - int offset; - int size; -}; - -/* - * Flags for fdt_find_regions() - * - * Add a region for the string table (always the last region) - */ -#define FDT_REG_ADD_STRING_TAB (1 << 0) - -/* - * Add all supernodes of a matching node/property, useful for creating a - * valid subset tree - */ -#define FDT_REG_SUPERNODES (1 << 1) - -/* Add the FDT_BEGIN_NODE tags of subnodes, including their names */ -#define FDT_REG_DIRECT_SUBNODES (1 << 2) - -/* Add all subnodes of a matching node */ -#define FDT_REG_ALL_SUBNODES (1 << 3) - -/* Add a region for the mem_rsvmap table (always the first region) */ -#define FDT_REG_ADD_MEM_RSVMAP (1 << 4) - -/* Indicates what an fdt part is (node, property, value) */ -#define FDT_IS_NODE (1 << 0) -#define FDT_IS_PROP (1 << 1) -#define FDT_IS_VALUE (1 << 2) /* not supported */ -#define FDT_IS_COMPAT (1 << 3) /* used internally */ -#define FDT_NODE_HAS_PROP (1 << 4) /* node contains prop */ - -#define FDT_ANY_GLOBAL (FDT_IS_NODE | FDT_IS_PROP | FDT_IS_VALUE | \ - FDT_IS_COMPAT) -#define FDT_IS_ANY 0x1f /* all the above */ - -/* We set a reasonable limit on the number of nested nodes */ -#define FDT_MAX_DEPTH 32 - -/* Decribes what we want to include from the current tag */ -enum want_t { - WANT_NOTHING, - WANT_NODES_ONLY, /* No properties */ - WANT_NODES_AND_PROPS, /* Everything for one level */ - WANT_ALL_NODES_AND_PROPS /* Everything for all levels */ -}; - -/* Keeps track of the state at parent nodes */ -struct fdt_subnode_stack { - int offset; /* Offset of node */ - enum want_t want; /* The 'want' value here */ - int included; /* 1 if we included this node, 0 if not */ -}; - -struct fdt_region_ptrs { - int depth; /* Current tree depth */ - int done; /* What we have completed scanning */ - enum want_t want; /* What we are currently including */ - char *end; /* Pointer to end of full node path */ - int nextoffset; /* Next node offset to check */ -}; - -/* The state of our finding algortihm */ -struct fdt_region_state { - struct fdt_subnode_stack stack[FDT_MAX_DEPTH]; /* node stack */ - struct fdt_region *region; /* Contains list of regions found */ - int count; /* Numnber of regions found */ - const void *fdt; /* FDT blob */ - int max_regions; /* Maximum regions to find */ - int can_merge; /* 1 if we can merge with previous region */ - int start; /* Start position of current region */ - struct fdt_region_ptrs ptrs; /* Pointers for what we are up to */ -}; - -/** - * fdt_find_regions() - find regions in device tree - * - * Given a list of nodes to include and properties to exclude, find - * the regions of the device tree which describe those included parts. - * - * The intent is to get a list of regions which will be invariant provided - * those parts are invariant. For example, if you request a list of regions - * for all nodes but exclude the property "data", then you will get the - * same region contents regardless of any change to "data" properties. - * - * This function can be used to produce a byte-stream to send to a hashing - * function to verify that critical parts of the FDT have not changed. - * - * Nodes which are given in 'inc' are included in the region list, as - * are the names of the immediate subnodes nodes (but not the properties - * or subnodes of those subnodes). - * - * For eaxample "/" means to include the root node, all root properties - * and the FDT_BEGIN_NODE and FDT_END_NODE of all subnodes of /. The latter - * ensures that we capture the names of the subnodes. In a hashing situation - * it prevents the root node from changing at all Any change to non-excluded - * properties, names of subnodes or number of subnodes would be detected. - * - * When used with FITs this provides the ability to hash and sign parts of - * the FIT based on different configurations in the FIT. Then it is - * impossible to change anything about that configuration (include images - * attached to the configuration), but it may be possible to add new - * configurations, new images or new signatures within the existing - * framework. - * - * Adding new properties to a device tree may result in the string table - * being extended (if the new property names are different from those - * already added). This function can optionally include a region for - * the string table so that this can be part of the hash too. - * - * The device tree header is not included in the list. - * - * @fdt: Device tree to check - * @inc: List of node paths to included - * @inc_count: Number of node paths in list - * @exc_prop: List of properties names to exclude - * @exc_prop_count: Number of properties in exclude list - * @region: Returns list of regions - * @max_region: Maximum length of region list - * @path: Pointer to a temporary string for the function to use for - * building path names - * @path_len: Length of path, must be large enough to hold the longest - * path in the tree - * @add_string_tab: 1 to add a region for the string table - * @return number of regions in list. If this is >max_regions then the - * region array was exhausted. You should increase max_regions and try - * the call again. - */ -int fdt_find_regions(const void *fdt, char * const inc[], int inc_count, - char * const exc_prop[], int exc_prop_count, - struct fdt_region region[], int max_regions, - char *path, int path_len, int add_string_tab); - -/** - * fdt_first_region() - find regions in device tree - * - * Given a nodes and properties to include and properties to exclude, find - * the regions of the device tree which describe those included parts. - * - * The use for this function is twofold. Firstly it provides a convenient - * way of performing a structure-aware grep of the tree. For example it is - * possible to grep for a node and get all the properties associated with - * that node. Trees can be subsetted easily, by specifying the nodes that - * are required, and then writing out the regions returned by this function. - * This is useful for small resource-constrained systems, such as boot - * loaders, which want to use an FDT but do not need to know about all of - * it. - * - * Secondly it makes it easy to hash parts of the tree and detect changes. - * The intent is to get a list of regions which will be invariant provided - * those parts are invariant. For example, if you request a list of regions - * for all nodes but exclude the property "data", then you will get the - * same region contents regardless of any change to "data" properties. - * - * This function can be used to produce a byte-stream to send to a hashing - * function to verify that critical parts of the FDT have not changed. - * Note that semantically null changes in order could still cause false - * hash misses. Such reordering might happen if the tree is regenerated - * from source, and nodes are reordered (the bytes-stream will be emitted - * in a different order and many hash functions will detect this). However - * if an existing tree is modified using libfdt functions, such as - * fdt_add_subnode() and fdt_setprop(), then this problem is avoided. - * - * The nodes/properties to include/exclude are defined by a function - * provided by the caller. This function is called for each node and - * property, and must return: - * - * 0 - to exclude this part - * 1 - to include this part - * -1 - for FDT_IS_PROP only: no information is available, so include - * if its containing node is included - * - * The last case is only used to deal with properties. Often a property is - * included if its containing node is included - this is the case where - * -1 is returned.. However if the property is specifically required to be - * included/excluded, then 0 or 1 can be returned. Note that including a - * property when the FDT_REG_SUPERNODES flag is given will force its - * containing node to be included since it is not valid to have a property - * that is not in a node. - * - * Using the information provided, the inclusion of a node can be controlled - * either by a node name or its compatible string, or any other property - * that the function can determine. - * - * As an example, including node "/" means to include the root node and all - * root properties. A flag provides a way of also including supernodes (of - * which there is none for the root node), and another flag includes - * immediate subnodes, so in this case we would get the FDT_BEGIN_NODE and - * FDT_END_NODE of all subnodes of /. - * - * The subnode feature helps in a hashing situation since it prevents the - * root node from changing at all. Any change to non-excluded properties, - * names of subnodes or number of subnodes would be detected. - * - * When used with FITs this provides the ability to hash and sign parts of - * the FIT based on different configurations in the FIT. Then it is - * impossible to change anything about that configuration (include images - * attached to the configuration), but it may be possible to add new - * configurations, new images or new signatures within the existing - * framework. - * - * Adding new properties to a device tree may result in the string table - * being extended (if the new property names are different from those - * already added). This function can optionally include a region for - * the string table so that this can be part of the hash too. This is always - * the last region. - * - * The FDT also has a mem_rsvmap table which can also be included, and is - * always the first region if so. - * - * The device tree header is not included in the region list. Since the - * contents of the FDT are changing (shrinking, often), the caller will need - * to regenerate the header anyway. - * - * @fdt: Device tree to check - * @h_include: Function to call to determine whether to include a part or - * not: - * - * @priv: Private pointer as passed to fdt_find_regions() - * @fdt: Pointer to FDT blob - * @offset: Offset of this node / property - * @type: Type of this part, FDT_IS_... - * @data: Pointer to data (node name, property name, compatible - * string, value (not yet supported) - * @size: Size of data, or 0 if none - * @return 0 to exclude, 1 to include, -1 if no information is - * available - * @priv: Private pointer passed to h_include - * @region: Returns list of regions, sorted by offset - * @max_regions: Maximum length of region list - * @path: Pointer to a temporary string for the function to use for - * building path names - * @path_len: Length of path, must be large enough to hold the longest - * path in the tree - * @flags: Various flags that control the region algortihm, see - * FDT_REG_... - * @return number of regions in list. If this is >max_regions then the - * region array was exhausted. You should increase max_regions and try - * the call again. Only the first max_regions elements are available in the - * array. - * - * On error a -ve value is return, which can be: - * - * -FDT_ERR_BADSTRUCTURE (too deep or more END tags than BEGIN tags - * -FDT_ERR_BADLAYOUT - * -FDT_ERR_NOSPACE (path area is too small) - */ -int fdt_first_region(const void *fdt, - int (*h_include)(void *priv, const void *fdt, int offset, - int type, const char *data, int size), - void *priv, struct fdt_region *region, - char *path, int path_len, int flags, - struct fdt_region_state *info); - -/** fdt_next_region() - find next region - * - * See fdt_first_region() for full description. This function finds the - * next region according to the provided parameters, which must be the same - * as passed to fdt_first_region(). - * - * This function can additionally return -FDT_ERR_NOTFOUND when there are no - * more regions - */ -int fdt_next_region(const void *fdt, - int (*h_include)(void *priv, const void *fdt, int offset, - int type, const char *data, int size), - void *priv, struct fdt_region *region, - char *path, int path_len, int flags, - struct fdt_region_state *info); - -/** - * fdt_add_alias_regions() - find aliases that point to existing regions - * - * Once a device tree grep is complete some of the nodes will be present - * and some will have been dropped. This function checks all the alias nodes - * to figure out which points point to nodes which are still present. These - * aliases need to be kept, along with the nodes they reference. - * - * Given a list of regions function finds the aliases that still apply and - * adds more regions to the list for these. This function is called after - * fdt_next_region() has finished returning regions and requires the same - * state. - * - * @fdt: Device tree file to reference - * @region: List of regions that will be kept - * @count: Number of regions - * @max_regions: Number of entries that can fit in @region - * @info: Region state as returned from fdt_next_region() - * @return new number of regions in @region (i.e. count + the number added) - * or -FDT_ERR_NOSPACE if there was not enough space. - */ -int fdt_add_alias_regions(const void *fdt, struct fdt_region *region, int count, - int max_regions, struct fdt_region_state *info); -#endif /* SWIG */ - extern struct fdt_header *working_fdt; /* Pointer to the working fdt */
#endif /* _INCLUDE_LIBFDT_H_ */ diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c index 8d33205..bbdefe6 100644 --- a/tools/fdtgrep.c +++ b/tools/fdtgrep.c @@ -18,6 +18,7 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> +#include <fdt_region.h>
#include "fdt_host.h" #include "libfdt_internal.h" diff --git a/tools/image-host.c b/tools/image-host.c index 8a7469e..fc63c9e 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -11,6 +11,7 @@
#include "mkimage.h" #include <bootm.h> +#include <fdt_region.h> #include <image.h> #include <version.h>

Hi Masahiro,
On 21 March 2018 at 03:03, Masahiro Yamada yamada.masahiro@socionext.com wrote:
fdt_region APIs are not part of libfdt. They are U-Boot extension for the verified boot. Split the declartions related to fdt_region out ot <fdt_region.h>. This allows <linux/libfdt.h> to become a simple wrapper file, like Linux does.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
common/fdt_region.c | 1 + common/image-sig.c | 1 + include/fdt_region.h | 303 +++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/libfdt.h | 299 ------------------------------------------------ tools/fdtgrep.c | 1 + tools/image-host.c | 1 + 6 files changed, 307 insertions(+), 299 deletions(-) create mode 100644 include/fdt_region.h
I think moving the code into a separate header is fine, but I'm not keen on putting the implementation in common/ as it is a feature of libfdt. How does this actually help us?
Regards, Simon

Hi Simon,
2018-03-23 23:31 GMT+09:00 Simon Glass sjg@chromium.org:
Hi Masahiro,
On 21 March 2018 at 03:03, Masahiro Yamada yamada.masahiro@socionext.com wrote:
fdt_region APIs are not part of libfdt. They are U-Boot extension for the verified boot. Split the declartions related to fdt_region out ot <fdt_region.h>. This allows <linux/libfdt.h> to become a simple wrapper file, like Linux does.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
common/fdt_region.c | 1 + common/image-sig.c | 1 + include/fdt_region.h | 303 +++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/libfdt.h | 299 ------------------------------------------------ tools/fdtgrep.c | 1 + tools/image-host.c | 1 + 6 files changed, 307 insertions(+), 299 deletions(-) create mode 100644 include/fdt_region.h
I think moving the code into a separate header is fine, but I'm not keen on putting the implementation in common/ as it is a feature of libfdt. How does this actually help us?
I want to clarify the boundary between the upstream code and U-Boot own code.
If somebody tries to put new code into lib/libfdt/, he is doing wrong.

Hi Masahiro,
On 23 March 2018 at 09:57, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Hi Simon,
2018-03-23 23:31 GMT+09:00 Simon Glass sjg@chromium.org:
Hi Masahiro,
On 21 March 2018 at 03:03, Masahiro Yamada yamada.masahiro@socionext.com wrote:
fdt_region APIs are not part of libfdt. They are U-Boot extension for the verified boot. Split the declartions related to fdt_region out ot <fdt_region.h>. This allows <linux/libfdt.h> to become a simple wrapper file, like Linux does.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
common/fdt_region.c | 1 + common/image-sig.c | 1 + include/fdt_region.h | 303 +++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/libfdt.h | 299 ------------------------------------------------ tools/fdtgrep.c | 1 + tools/image-host.c | 1 + 6 files changed, 307 insertions(+), 299 deletions(-) create mode 100644 include/fdt_region.h
I think moving the code into a separate header is fine, but I'm not keen on putting the implementation in common/ as it is a feature of libfdt. How does this actually help us?
I want to clarify the boundary between the upstream code and U-Boot own code.
If somebody tries to put new code into lib/libfdt/, he is doing wrong.
Will still do have some patches locally that are not fully upstream, at least last time I looked. I'll take another look.
Regards, Simon
participants (2)
-
Masahiro Yamada
-
Simon Glass