* [PATCH 0/8] Introduce fdtgrep for subsetting and hashing FDTs
@ 2013-01-21 20:59 Simon Glass
[not found] ` <1358801962-21707-1-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
0 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2013-01-21 20:59 UTC (permalink / raw)
To: Jon Loeliger; +Cc: Devicetree Discuss
This series adds a new function, fdt_find_regions() which maps FDT parts
such as nodes and properties to their regions in the FDT binary. The
function is then used to implement a grep utility for FDTs.
The function itself is quite simple and small, but this series adds tests
and a grep utility, so quite a bit of code is built on it.
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. The full FDT can be grepped to pull out the few things that are
needed - this can be automatic and does not require the FDT source code.
This first use makes it easy to implement an FDT grep. Options are
provided to search for matching nodes (by name or compatible string),
properties and also for any of the above. It is possible to search for
non-matches also (useful for excluding a particular property from the
FDT, for example). The output is like fdtdump, but only with the regions
selected by the grep. Options are also provided to print the string table,
memory reservation table, etc. The fdtgrep utility can output valid
source, which can be used by dtc, but it can also directly output a new
.dtb binary.
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 second use is the subject of a recent series sent to the U-Boot
mailing list, to enhance FIT images to support verified boot. Briefly,
this works by signing configurations (consisting of particular kernel
and FDT combinations) so that the boot loader can verify that these
combinations are valid and permitted. Since a FIT image is in fact an
FDT, we need to be able to hash particular regions of the FDT for the
signing and verification process. This is done by using fdt_find_regions()
to select the data that needs to be hashed for a particular configuration.
The fdtgrep utility can fairly easily replace all of the functions of
fdtdump, so this series turns fdtdump into a symlink.
Simon Glass (8):
Adjust util_is_printable_string() comment and fix test
Move property-printing into util
.gitignore: Add rule for *.patch
Export fdt_stringlist_contains()
libfdt: Add function to find regions in an FDT
Add fdtgrep to grep and subset FDTs
Remove fdtdump and use fdtgrep instead
RFC: Check offset in fdt_string()
.gitignore | 2 +
Makefile | 9 +-
Makefile.utils | 7 +
fdtdump.c | 172 -----------
fdtgrep.c | 789 ++++++++++++++++++++++++++++++++++++++++++++++++++
libfdt/fdt_ro.c | 7 +-
libfdt/fdt_wip.c | 311 ++++++++++++++++++++
libfdt/libfdt.h | 156 ++++++++++
tests/.gitignore | 1 +
tests/Makefile.tests | 3 +-
tests/grep.dts | 23 ++
tests/region_tree.c | 324 +++++++++++++++++++++
tests/run_tests.sh | 357 ++++++++++++++++++++++-
tests/tests.sh | 1 +
util.c | 37 +++
util.h | 22 ++-
16 files changed, 2036 insertions(+), 185 deletions(-)
delete mode 100644 fdtdump.c
create mode 100644 fdtgrep.c
create mode 100644 tests/grep.dts
create mode 100644 tests/region_tree.c
--
1.7.7.3
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/8] Adjust util_is_printable_string() comment and fix test
[not found] ` <1358801962-21707-1-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2013-01-21 20:59 ` Simon Glass
[not found] ` <1358801962-21707-2-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-01-21 20:59 ` [PATCH 2/8] Move property-printing into util Simon Glass
` (6 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2013-01-21 20:59 UTC (permalink / raw)
To: Jon Loeliger; +Cc: Devicetree Discuss
This commit which changed the behaviour of this function broke one
of the tests. Also the comment should be updated to reflect its new
behaviour.
Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
tests/run_tests.sh | 4 +---
util.h | 8 +++++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index dd7f217..e04512c 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -498,9 +498,7 @@ fdtget_tests () {
# run_fdtget_test <expected-result> [<flags>] <file> <node> <property>
run_fdtget_test "MyBoardName" $dtb / model
- run_fdtget_test "77 121 66 111 \
-97 114 100 78 97 109 101 0 77 121 66 111 97 114 100 70 97 109 105 \
-108 121 78 97 109 101 0" $dtb / compatible
+ run_fdtget_test "MyBoardName MyBoardFamilyName" $dtb / compatible
run_fdtget_test "MyBoardName MyBoardFamilyName" -t s $dtb / compatible
run_fdtget_test 32768 $dtb /cpus/PowerPC,970@1 d-cache-size
run_fdtget_test 8000 -tx $dtb /cpus/PowerPC,970@1 d-cache-size
diff --git a/util.h b/util.h
index c8eb45d..e9043be 100644
--- a/util.h
+++ b/util.h
@@ -57,12 +57,14 @@ extern char *xstrdup(const char *s);
extern char *join_path(const char *path, const char *name);
/**
- * Check a string of a given length to see if it is all printable and
- * has a valid terminator.
+ * Check a property of a given length to see if it is all printable and
+ * has a valid terminator. The property can contain either a single string,
+ * or multiple strings each of non-zero length.
*
* @param data The string to check
* @param len The string length including terminator
- * @return 1 if a valid printable string, 0 if not */
+ * @return 1 if a valid printable string, 0 if not
+ */
int util_is_printable_string(const void *data, int len);
/*
--
1.7.7.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/8] Move property-printing into util
[not found] ` <1358801962-21707-1-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-01-21 20:59 ` [PATCH 1/8] Adjust util_is_printable_string() comment and fix test Simon Glass
@ 2013-01-21 20:59 ` Simon Glass
[not found] ` <1358801962-21707-3-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-01-21 20:59 ` [PATCH 3/8] .gitignore: Add rule for *.patch Simon Glass
` (5 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2013-01-21 20:59 UTC (permalink / raw)
To: Jon Loeliger; +Cc: Devicetree Discuss
The function that prints a property can be useful to other programs,
so move it into util.
Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
fdtdump.c | 37 +------------------------------------
util.c | 37 +++++++++++++++++++++++++++++++++++++
util.h | 14 ++++++++++++++
3 files changed, 52 insertions(+), 36 deletions(-)
diff --git a/fdtdump.c b/fdtdump.c
index b2c5b37..03ea429 100644
--- a/fdtdump.c
+++ b/fdtdump.c
@@ -17,41 +17,6 @@
#define PALIGN(p, a) ((void *)(ALIGN((unsigned long)(p), (a))))
#define GET_CELL(p) (p += 4, *((const uint32_t *)(p-4)))
-static void print_data(const char *data, int len)
-{
- int i;
- const char *p = data;
- const char *s;
-
- /* no data, don't print */
- if (len == 0)
- return;
-
- if (util_is_printable_string(data, len)) {
- printf(" = ");
-
- s = data;
- do {
- printf("\"%s\"", s);
- s += strlen(s) + 1;
- if (s < data + len)
- printf(", ");
- } while (s < data + len);
-
- } else if ((len % 4) == 0) {
- printf(" = <");
- for (i = 0; i < len; i += 4)
- printf("0x%08x%s", fdt32_to_cpu(GET_CELL(p)),
- i < (len - 4) ? " " : "");
- printf(">");
- } else {
- printf(" = [");
- for (i = 0; i < len; i++)
- printf("%02x%s", *p++, i < len - 1 ? " " : "");
- printf("]");
- }
-}
-
static void dump_blob(void *blob)
{
struct fdt_header *bph = blob;
@@ -147,7 +112,7 @@ static void dump_blob(void *blob)
p = PALIGN(p + sz, 4);
printf("%*s%s", depth * shift, "", s);
- print_data(t, sz);
+ utilfdt_print_data(t, sz);
printf(";\n");
}
}
diff --git a/util.c b/util.c
index 45f186b..b081fa8 100644
--- a/util.c
+++ b/util.c
@@ -335,3 +335,40 @@ int utilfdt_decode_type(const char *fmt, int *type, int *size)
return -1;
return 0;
}
+
+void utilfdt_print_data(const char *data, int len)
+{
+ int i;
+ const char *p = data;
+ const char *s;
+
+ /* no data, don't print */
+ if (len == 0)
+ return;
+
+ if (util_is_printable_string(data, len)) {
+ printf(" = ");
+
+ s = data;
+ do {
+ printf("\"%s\"", s);
+ s += strlen(s) + 1;
+ if (s < data + len)
+ printf(", ");
+ } while (s < data + len);
+
+ } else if ((len % 4) == 0) {
+ const uint32_t *cell = (const uint32_t *)data;
+
+ printf(" = <");
+ for (i = 0; i < len; i += 4)
+ printf("0x%08x%s", fdt32_to_cpu(cell[i]),
+ i < (len - 4) ? " " : "");
+ printf(">");
+ } else {
+ printf(" = [");
+ for (i = 0; i < len; i++)
+ printf("%02x%s", *p++, i < len - 1 ? " " : "");
+ printf("]");
+ }
+}
diff --git a/util.h b/util.h
index e9043be..543a173 100644
--- a/util.h
+++ b/util.h
@@ -152,4 +152,18 @@ int utilfdt_decode_type(const char *fmt, int *type, int *size);
"\tOptional modifier prefix:\n" \
"\t\thh or b=byte, h=2 byte, l=4 byte (default)\n";
+/**
+ * Print property data in a readable format to stdout
+ *
+ * Properties that look like strings will be printed as strings. Otherwise
+ * the data will be displayed either as cells (if len is a multiple of 4
+ * bytes) or bytes.
+ *
+ * If len is 0 then this function does nothing.
+ *
+ * @param data Pointers to property data
+ * @param len Length of property data
+ */
+void utilfdt_print_data(const char *data, int len);
+
#endif /* _UTIL_H */
--
1.7.7.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/8] .gitignore: Add rule for *.patch
[not found] ` <1358801962-21707-1-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-01-21 20:59 ` [PATCH 1/8] Adjust util_is_printable_string() comment and fix test Simon Glass
2013-01-21 20:59 ` [PATCH 2/8] Move property-printing into util Simon Glass
@ 2013-01-21 20:59 ` Simon Glass
[not found] ` <1358801962-21707-4-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-01-21 20:59 ` [PATCH 4/8] Export fdt_stringlist_contains() Simon Glass
` (4 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2013-01-21 20:59 UTC (permalink / raw)
To: Jon Loeliger; +Cc: Devicetree Discuss
Ignore any patch files that we find, since these are likely to be
used when sending patches upstream.
Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
.gitignore | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/.gitignore b/.gitignore
index 7cabc49..545b899 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,6 +1,7 @@
*.o
*.d
*.a
+*.patch
*.so
*~
*.tab.[ch]
--
1.7.7.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/8] Export fdt_stringlist_contains()
[not found] ` <1358801962-21707-1-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
` (2 preceding siblings ...)
2013-01-21 20:59 ` [PATCH 3/8] .gitignore: Add rule for *.patch Simon Glass
@ 2013-01-21 20:59 ` Simon Glass
[not found] ` <1358801962-21707-5-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-01-21 20:59 ` [PATCH 5/8] libfdt: Add function to find regions in an FDT Simon Glass
` (3 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2013-01-21 20:59 UTC (permalink / raw)
To: Jon Loeliger; +Cc: Devicetree Discuss
This function is useful outside libfdt, so export it.
Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
libfdt/fdt_ro.c | 5 ++---
libfdt/libfdt.h | 14 ++++++++++++++
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
index 42da2bd..50007f6 100644
--- a/libfdt/fdt_ro.c
+++ b/libfdt/fdt_ro.c
@@ -515,8 +515,7 @@ int fdt_node_offset_by_phandle(const void *fdt, uint32_t phandle)
return offset; /* error from fdt_next_node() */
}
-static int _fdt_stringlist_contains(const char *strlist, int listlen,
- const char *str)
+int fdt_stringlist_contains(const char *strlist, int listlen, const char *str)
{
int len = strlen(str);
const char *p;
@@ -542,7 +541,7 @@ int fdt_node_check_compatible(const void *fdt, int nodeoffset,
prop = fdt_getprop(fdt, nodeoffset, "compatible", &len);
if (!prop)
return len;
- if (_fdt_stringlist_contains(prop, len, compatible))
+ if (fdt_stringlist_contains(prop, len, compatible))
return 0;
else
return 1;
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index 8e57a06..c0075e7 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -816,6 +816,20 @@ int fdt_node_check_compatible(const void *fdt, int nodeoffset,
int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
const char *compatible);
+/**
+ * fdt_stringlist_contains - check a string list property for a string
+ * @strlist: Property containing a list of strings to check
+ * @listlen: Length of property
+ * @str: String to search for
+ *
+ * This is a utility function provided for convenience. The list contains
+ * one or more strings, each terminated by \0, as is found in a device tree
+ * "compatible" property.
+ *
+ * @return: 1 if the string is found in the list, 0 not found, or invalid list
+ */
+int fdt_stringlist_contains(const char *strlist, int listlen, const char *str);
+
/**********************************************************************/
/* Write-in-place functions */
/**********************************************************************/
--
1.7.7.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 5/8] libfdt: Add function to find regions in an FDT
[not found] ` <1358801962-21707-1-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
` (3 preceding siblings ...)
2013-01-21 20:59 ` [PATCH 4/8] Export fdt_stringlist_contains() Simon Glass
@ 2013-01-21 20:59 ` Simon Glass
[not found] ` <1358801962-21707-6-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-01-21 20:59 ` [PATCH 6/8] Add fdtgrep to grep and subset FDTs Simon Glass
` (2 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2013-01-21 20:59 UTC (permalink / raw)
To: Jon Loeliger; +Cc: Devicetree Discuss
Given a set of nodes and properties, find the regions of the device tree
which describe those parts.
A test is provided which builds a tree while tracking where the regions
should be, then calls fdt_find_regions() to make sure that it agrees.
Further tests will come as part of fdtgrep.
Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
libfdt/fdt_wip.c | 311 ++++++++++++++++++++++++++++++++++++++++++++++++
libfdt/libfdt.h | 142 ++++++++++++++++++++++
tests/.gitignore | 1 +
tests/Makefile.tests | 3 +-
tests/region_tree.c | 324 ++++++++++++++++++++++++++++++++++++++++++++++++++
tests/run_tests.sh | 5 +
6 files changed, 785 insertions(+), 1 deletions(-)
create mode 100644 tests/region_tree.c
diff --git a/libfdt/fdt_wip.c b/libfdt/fdt_wip.c
index c5bbb68..ff0f940 100644
--- a/libfdt/fdt_wip.c
+++ b/libfdt/fdt_wip.c
@@ -116,3 +116,314 @@ int fdt_nop_node(void *fdt, int nodeoffset)
endoffset - nodeoffset);
return 0;
}
+
+/* Maximum depth that we can grep */
+#define FDT_MAX_DEPTH 32
+
+/* Decribes what we want to include from the current tag */
+enum want_t {
+ WANT_NOTHING,
+ WANT_NODES_ONLY,
+ WANT_NODES_AND_PROPS,
+};
+
+/* 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 */
+};
+
+/* The state of our finding algortihm */
+struct find_reg {
+ 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 */
+};
+
+/**
+ * fdt_add_region() - Add a new region to our list
+ *
+ * The region is added if there is space, but in any case we increment the
+ * count. If permitted, and the new region overlaps the last one, we merge
+ * them.
+ *
+ * @info: State information
+ * @offset: Start offset of region
+ * @size: Size of region
+ */
+static void fdt_add_region(struct find_reg *info, int offset, int size)
+{
+ struct fdt_region *reg = &info->region[info->count - 1];
+
+ if (info->can_merge && info->count &&
+ info->count < info->max_regions &&
+ offset <= reg->offset + reg->size) {
+ reg->size = offset + size - reg->offset;
+ } else if (info->count < info->max_regions) {
+ reg++;
+ reg->offset = offset;
+ reg->size = size;
+ info->count++;
+ }
+}
+
+/**
+ * fdt_include_supernodes() - Include supernodes required by this node
+ *
+ * When we decided to include a node or property which is not at the top
+ * level, this function forces the inclusion of higher level nodes. For
+ * example, given this tree:
+ *
+ * / {
+ * testing {
+ * }
+ * }
+ *
+ * If we decide to include testing then we need the root node to have a valid
+ * tree. This function adds those regions.
+ *
+ * @info: State information
+ * @depth: Current stack depth
+ */
+static void fdt_include_supernodes(struct find_reg *info, int depth)
+{
+ int base = fdt_off_dt_struct(info->fdt);
+ int start, stop_at;
+ int i;
+
+ /*
+ * Work down the stack looking for supernodes that we didn't include.
+ * The algortihm here is actually pretty simple, since we know that
+ * no previous subnode had to include these nodes, or if it did, we
+ * marked them as included (on the stack) already.
+ */
+ for (i = 0; i <= depth; i++) {
+ if (!info->stack[i].included) {
+ start = info->stack[i].offset;
+
+ /* Add the FDT_BEGIN_NODE tag of this supernode */
+ fdt_next_tag(info->fdt, start, &stop_at);
+ fdt_add_region(info, base + start, stop_at - start);
+
+ /* Remember that this supernode is now included */
+ info->stack[i].included = 1;
+ info->can_merge = 1;
+ }
+
+ /* Force (later) generation of the FDT_END_NODE tag */
+ if (!info->stack[i].want)
+ info->stack[i].want = WANT_NODES_ONLY;
+ }
+}
+
+int fdt_find_regions(const void *fdt,
+ int (*h_include)(void *priv, int type, const char *data,
+ int size),
+ void *priv,
+ struct fdt_region region[], int max_regions,
+ char *path, int path_len, int flags)
+{
+ int base = fdt_off_dt_struct(fdt);
+ struct find_reg info;
+ int nextoffset;
+ int start = -1;
+ int depth = -1;
+ const char *str;
+ enum want_t want = WANT_NOTHING;
+ uint32_t tag;
+ char *end;
+
+ /* Set up our state */
+ info.fdt = fdt;
+ info.can_merge = 1;
+ info.count = 0;
+ info.region = region;
+ info.max_regions = max_regions;
+
+ /* Add the memory reserve map into its own region */
+ if (flags & FDT_REG_ADD_MEM_RSVMAP) {
+ fdt_add_region(&info, fdt_off_mem_rsvmap(fdt),
+ fdt_off_dt_struct(fdt) - fdt_off_mem_rsvmap(fdt));
+ info.can_merge = 0; /* Don't allow merging with this */
+ }
+
+ end = path;
+ *end = '\0';
+ nextoffset = 0;
+
+ /*
+ * Work through the tags one by one, deciding whether each needs to
+ * be included or not. We set the variable 'include' to indicate our
+ * decision. 'want' is used to track what we want to include - it
+ * allows us to pick up all the properties (and/or subnode tags) of
+ * a node.
+ */
+ do {
+ const struct fdt_property *prop;
+ const char *name;
+ int include = 0;
+ int stop_at = 0;
+ int offset;
+ int val;
+ int len;
+
+ /*
+ * Find the tag, and the offset of the next one. If we need to
+ * stop including tags, then by default we stop *after*
+ * including the current tag
+ */
+ offset = nextoffset;
+ tag = fdt_next_tag(fdt, offset, &nextoffset);
+ stop_at = nextoffset;
+
+ switch (tag) {
+ case FDT_PROP:
+ stop_at = offset;
+ prop = fdt_get_property_by_offset(fdt, offset, NULL);
+ str = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
+ val = h_include(priv, FDT_IS_PROP, str,
+ strlen(str) + 1);
+ if (val == -1) {
+ include = want == WANT_NODES_AND_PROPS;
+ } else {
+ include = val;
+ /*
+ * Make sure we include the } for this block.
+ * It might be more correct to put this done
+ * by the call to fdt_include_supernodes() in
+ * the case where it adds the node we are
+ * currently in, but this is fairly
+ * equivalent.
+ */
+ if ((flags & FDT_REG_SUPERNODES) && val &&
+ !want)
+ want = WANT_NODES_ONLY;
+ }
+
+ /* Value grepping is not yet supported */
+ break;
+
+ case FDT_NOP:
+ include = want == WANT_NODES_AND_PROPS;
+ stop_at = offset;
+ break;
+
+ case FDT_BEGIN_NODE:
+ depth++;
+ if (depth == FDT_MAX_DEPTH)
+ return -FDT_ERR_BADSTRUCTURE;
+ name = fdt_get_name(fdt, offset, &len);
+ if (end - path + 2 + len >= path_len)
+ return -FDT_ERR_NOSPACE;
+
+ /* Build the full path of this node */
+ if (end != path + 1)
+ *end++ = '/';
+ strcpy(end, name);
+ end += len;
+ info.stack[depth].want = want;
+ info.stack[depth].offset = offset;
+
+ /*
+ * If we are not intending to include this node, make
+ * sure we stop *before* its tag
+ */
+ if (want == WANT_NODES_ONLY ||
+ !(flags & FDT_REG_DIRECT_SUBNODES)) {
+ stop_at = offset;
+ want = WANT_NOTHING;
+ }
+ val = h_include(priv, FDT_IS_NODE, path,
+ end - path + 1);
+
+ /*
+ * If no information about this, try the compatible
+ * property. Here we are looking ahead in the tree.
+ */
+ if (val == -1) {
+ str = fdt_getprop(fdt, offset,
+ "compatible", &len);
+ val = h_include(priv, FDT_IS_COMPAT, str,
+ len);
+ }
+
+ /* Include this if requested */
+ if (val)
+ want = WANT_NODES_AND_PROPS;
+
+ /* If not requested, decay our 'want' value */
+ else if (want)
+ want--;
+
+ /* Not including this tag, so stop now */
+ else
+ stop_at = offset;
+
+ /*
+ * Decide whether to include this tag, and update our
+ * stack with the state for this node
+ */
+ include = want;
+ info.stack[depth].included = include;
+ break;
+
+ case FDT_END_NODE:
+ include = want;
+ if (depth < 0)
+ return -FDT_ERR_BADSTRUCTURE;
+
+ /*
+ * If we don't want this node, stop right away, unless
+ * we are including subnodes
+ */
+ if (!want && !(flags & FDT_REG_DIRECT_SUBNODES))
+ stop_at = offset;
+ want = info.stack[depth].want;
+ depth--;
+ while (end > path && *--end != '/')
+ ;
+ *end = '\0';
+ break;
+
+ case FDT_END:
+ /* We always include the end tag */
+ include = 1;
+ break;
+ }
+
+ /* If this tag is to be included, mark it as region start */
+ if (include && start == -1) {
+ /* Include any supernodes required by this one */
+ if (flags & FDT_REG_SUPERNODES)
+ fdt_include_supernodes(&info, depth);
+ start = offset;
+ }
+
+ /*
+ * If this tag is not to be included, finish up the current
+ * region
+ */
+ if (!include && start != -1) {
+ fdt_add_region(&info, base + start, stop_at - start);
+ start = -1;
+ info.can_merge = 1;
+ }
+ } while (tag != FDT_END);
+
+ if (nextoffset != fdt_size_dt_struct(fdt))
+ return -FDT_ERR_BADLAYOUT;
+
+ /* Add a region for the END tag and a separate one for string table */
+ fdt_add_region(&info, base + start, nextoffset - start);
+ if (flags & FDT_REG_ADD_STRING_TAB) {
+ info.can_merge = 0;
+ fdt_add_region(&info, fdt_off_dt_strings(fdt),
+ fdt_size_dt_strings(fdt));
+ }
+
+ return info.count;
+}
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index c0075e7..0b3eacf 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -1489,4 +1489,146 @@ int fdt_del_node(void *fdt, int nodeoffset);
const char *fdt_strerror(int errval);
+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 the FDT_BEGIN_NODE tags of subnodes, including their names */
+#define FDT_REG_DIRECT_SUBNODES (1 << 1)
+
+/*
+ * Add all supernodes of a matching node/property, useful for creating a
+ * valid subset tree
+ */
+#define FDT_REG_SUPERNODES (1 << 2)
+
+/* Add a region for the mem_rsvmap table (always the first region) */
+#define FDT_REG_ADD_MEM_RSVMAP (1 << 3)
+
+/* Indicates what an fdt part is (node, property, value, compatible) */
+#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)
+
+#define FDT_IS_ANY 15
+
+/**
+ * fdt_find_regions() - 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.
+ *
+ * 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 - if no information is available
+ *
+ * For nodes, the function may be called twice, once with FDT_IS_NODE to check
+ * the node name. If this returns -1 then the function is immediately called
+ * again with FDT_IS_COMPAT to check the compatible string. Note that if
+ * there is no compatible string, then data == NULL and size == 0. This means
+ * that the inclusion of a node can be controlled either by a node name or
+ * its compatible string.
+ *
+ * There is no special handling of unit addresses, so the entire name must
+ * be given for nodes. Wildcards are not supported.
+ *
+ * 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()
+ * @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_find_regions(const void *fdt,
+ int (*h_include)(void *priv, int type, const char *data,
+ int size),
+ void *priv,
+ struct fdt_region region[], int max_regions,
+ char *path, int path_len, int flags);
+
#endif /* _LIBFDT_H */
diff --git a/tests/.gitignore b/tests/.gitignore
index e2aa24a..ecac691 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -41,6 +41,7 @@ tmp.*
/phandle_format
/propname_escapes
/references
+/region_tree
/root_node
/rw_tree1
/set_name
diff --git a/tests/Makefile.tests b/tests/Makefile.tests
index d59bff8..10bbf1b 100644
--- a/tests/Makefile.tests
+++ b/tests/Makefile.tests
@@ -20,7 +20,8 @@ LIB_TESTS_L = get_mem_rsv \
dtb_reverse dtbs_equal_unordered \
add_subnode_with_nops path_offset_aliases \
utilfdt_test \
- integer-expressions
+ integer-expressions \
+ region_tree
LIB_TESTS = $(LIB_TESTS_L:%=$(TESTS_PREFIX)%)
LIBTREE_TESTS_L = truncated_property
diff --git a/tests/region_tree.c b/tests/region_tree.c
new file mode 100644
index 0000000..7a5f09b
--- /dev/null
+++ b/tests/region_tree.c
@@ -0,0 +1,324 @@
+/*
+ * hash_tree - Testcase for fdt_find_regions()
+ *
+ * Copyright (c) 2013, Google Inc.
+ *
+ * Copyright (C) 2006 David Gibson, IBM Corporation.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <ctype.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <libfdt_env.h>
+#include <fdt.h>
+#include <libfdt.h>
+
+#include "tests.h"
+#include "testdata.h"
+
+#define SPACE 65536
+
+#define CHECK(code) \
+ { \
+ err = (code); \
+ if (err) \
+ FAIL(#code ": %s", fdt_strerror(err)); \
+ }
+
+/*
+ * Regions we expect to see returned from fdt_find_regions(). We build up a
+ * list of these as we make the tree, then check the results of
+ * fdt_find_regions() once we are done.
+ */
+static struct fdt_region expect[20];
+
+/* Number of expected regions */
+int expect_count;
+
+/* Mark the start of a new region */
+static void start(void *fdt)
+{
+ expect[expect_count].offset = fdt_size_dt_struct(fdt);
+ verbose_printf("[%d: %x ", expect_count,
+ fdt_off_dt_struct(fdt) + expect[expect_count].offset);
+}
+
+/* Mark the end of a region */
+static void stop(void *fdt)
+{
+ expect[expect_count].size = fdt_size_dt_struct(fdt) -
+ expect[expect_count].offset;
+ expect[expect_count].offset += fdt_off_dt_struct(fdt);
+ verbose_printf("%x]\n", expect[expect_count].offset +
+ expect[expect_count].size);
+ expect_count++;
+}
+
+/**
+ * build_tree() - Build a tree
+ *
+ * @fdt: Pointer to place to put tree, assumed to be large enough
+ * @flags: Flags to control the tree creation (FDT_REG_...)
+ * @space: Amount of space to create for later tree additions
+ *
+ * This creates a tree modelled on a U-Boot FIT image, with various nodes
+ * and properties which are useful for testing the hashing features of
+ * fdt_find_regions().
+ */
+static void build_tree(void *fdt, int flags, int space)
+{
+ int direct_subnodes = flags & FDT_REG_DIRECT_SUBNODES;
+ int supernodes = flags & FDT_REG_SUPERNODES;
+ int either = direct_subnodes || supernodes;
+ int err;
+
+ CHECK(fdt_create(fdt, SPACE));
+
+ CHECK(fdt_add_reservemap_entry(fdt, TEST_ADDR_1, TEST_SIZE_1));
+ CHECK(fdt_add_reservemap_entry(fdt, TEST_ADDR_2, TEST_SIZE_2));
+ CHECK(fdt_finish_reservemap(fdt));
+
+ start(fdt); /* region 0 */
+ CHECK(fdt_begin_node(fdt, ""));
+ CHECK(fdt_property_string(fdt, "description", "kernel image"));
+ CHECK(fdt_property_u32(fdt, "#address-cells", 1));
+
+ /* /images */
+ if (!either)
+ stop(fdt);
+ CHECK(fdt_begin_node(fdt, "images"));
+ if (either)
+ stop(fdt);
+ CHECK(fdt_property_u32(fdt, "image-prop", 1));
+
+ /* /images/kernel@1 */
+ start(fdt); /* region 1 */
+ CHECK(fdt_begin_node(fdt, "kernel@1"));
+ CHECK(fdt_property_string(fdt, "description", "exynos kernel"));
+ stop(fdt);
+ CHECK(fdt_property_string(fdt, "data", "this is the kernel image"));
+ start(fdt); /* region 2 */
+
+ /* /images/kernel/hash@1 */
+ CHECK(fdt_begin_node(fdt, "hash@1"));
+ CHECK(fdt_property_string(fdt, "algo", "sha1"));
+ CHECK(fdt_end_node(fdt));
+
+ /* /images/kernel/hash@2 */
+ if (!direct_subnodes)
+ stop(fdt);
+ CHECK(fdt_begin_node(fdt, "hash@2"));
+ if (direct_subnodes)
+ stop(fdt);
+ CHECK(fdt_property_string(fdt, "algo", "sha1"));
+ if (direct_subnodes)
+ start(fdt); /* region 3 */
+ CHECK(fdt_end_node(fdt));
+ if (!direct_subnodes)
+ start(fdt); /* region 3 */
+
+ CHECK(fdt_end_node(fdt));
+
+ /* /images/fdt@1 */
+ CHECK(fdt_begin_node(fdt, "fdt@1"));
+ CHECK(fdt_property_string(fdt, "description", "snow FDT"));
+ stop(fdt);
+ CHECK(fdt_property_string(fdt, "data", "FDT data"));
+ start(fdt); /* region 4 */
+
+ /* /images/kernel/hash@1 */
+ CHECK(fdt_begin_node(fdt, "hash@1"));
+ CHECK(fdt_property_string(fdt, "algo", "sha1"));
+ CHECK(fdt_end_node(fdt));
+
+ CHECK(fdt_end_node(fdt));
+
+ if (!either)
+ stop(fdt);
+ CHECK(fdt_end_node(fdt));
+
+ /* /configurations */
+ CHECK(fdt_begin_node(fdt, "configurations"));
+ if (either)
+ stop(fdt);
+ CHECK(fdt_property_string(fdt, "default", "conf@1"));
+
+ /* /configurations/conf@1 */
+ start(fdt); /* region 6 */
+ CHECK(fdt_begin_node(fdt, "conf@1"));
+ CHECK(fdt_property_string(fdt, "kernel", "kernel@1"));
+ CHECK(fdt_property_string(fdt, "fdt", "fdt@1"));
+ CHECK(fdt_end_node(fdt));
+ stop(fdt);
+
+ /* /configurations/conf@2 */
+ CHECK(fdt_begin_node(fdt, "conf@2"));
+ CHECK(fdt_property_string(fdt, "kernel", "kernel@1"));
+ CHECK(fdt_property_string(fdt, "fdt", "fdt@2"));
+ CHECK(fdt_end_node(fdt));
+
+ if (either)
+ start(fdt); /* region 7 */
+ CHECK(fdt_end_node(fdt));
+ if (!either)
+ start(fdt); /* region 7 */
+
+ CHECK(fdt_end_node(fdt));
+
+ CHECK(fdt_finish(fdt));
+ stop(fdt);
+
+ /* Add in the strings */
+ if (flags & FDT_REG_ADD_STRING_TAB) {
+ expect[expect_count].offset = fdt_off_dt_strings(fdt);
+ expect[expect_count].size = fdt_size_dt_strings(fdt);
+ expect_count++;
+ }
+
+ /* Make a bit of space */
+ if (space)
+ CHECK(fdt_open_into(fdt, fdt, fdt_totalsize(fdt) + space));
+
+ verbose_printf("Completed tree, totalsize = %d\n", fdt_totalsize(fdt));
+}
+
+/**
+ * strlist_contains() - Returns 1 if a string is contained in a list
+ *
+ * @list: List of strings
+ * @count: Number of strings in list
+ * @str: String to search for
+ */
+static int strlist_contains(const char * const list[], int count,
+ const char *str)
+{
+ int i;
+
+ for (i = 0; i < count; i++)
+ if (!strcmp(list[i], str))
+ return 1;
+
+ return 0;
+}
+
+/**
+ * h_include() - Our include handler for fdt_find_regions()
+ *
+ * This is very simple - we have a list of nodes we are looking for, and
+ * one property that we want to exclude.
+ */
+static int h_include(void *priv, int type, const char *data, int size)
+{
+ const char * const inc[] = {
+ "/",
+ "/images/kernel@1",
+ "/images/fdt@1",
+ "/configurations/conf@1",
+ "/images/kernel@1/hash@1",
+ "/images/fdt@1/hash@1",
+ };
+
+ switch (type) {
+ case FDT_IS_NODE:
+ return strlist_contains(inc, 6, data);
+ case FDT_IS_PROP:
+ return !strcmp(data, "data") ? 0 : -1;
+ }
+
+ return 0;
+}
+
+/**
+ * check_regions() - Check that the regions are as we expect
+ *
+ * Call fdt_find_regions() and check that the results are as we expect them,
+ * matching the list of expected regions we created at the same time as
+ * the tree.
+ *
+ * @fdt: Pointer to device tree to check
+ * @flags: Flags value (FDT_REG_...)
+ * @return 0 if ok, -1 on failure
+ */
+static int check_regions(const void *fdt, int flags)
+{
+ int count;
+ struct fdt_region region[20];
+ char path[1024];
+ int ret = 0;
+ int i;
+
+ count = fdt_find_regions(fdt, h_include, NULL, region,
+ ARRAY_SIZE(region), path, sizeof(path),
+ flags);
+ verbose_printf("Regions: %d\n", count);
+ for (i = 0; i < count; i++) {
+ struct fdt_region *reg = ®ion[i];
+ struct fdt_region *exp = &expect[i];
+
+ verbose_printf("%d: %-10x %-10x\n", i, reg->offset,
+ reg->offset + reg->size);
+ if (memcmp(exp, reg, sizeof(*reg))) {
+ ret = -1;
+ verbose_printf("exp: %-10x %-10x\n", exp->offset,
+ exp->offset + exp->size);
+ }
+ }
+
+ return ret;
+}
+
+int main(int argc, char *argv[])
+{
+ const char *fname = NULL;
+ int flags = 0;
+ int space = 0;
+ void *fdt;
+
+ test_init(argc, argv);
+ if (argc < 2) {
+ verbose_printf("Usage: %s <flag value> [<space>]"
+ " [<output_fname.dtb>]", argv[0]);
+ FAIL();
+ }
+ flags = atoi(argv[1]);
+ if (argc >= 3)
+ space = atoi(argv[2]);
+ if (argc >= 4)
+ fname = argv[3];
+
+ /*
+ * Allocate space for the tree and build it, creating a list of
+ * expected regions.
+ */
+ fdt = xmalloc(SPACE);
+ build_tree(fdt, flags, space);
+
+ /* Write the tree out if required */
+ if (fname)
+ save_blob(fname, fdt);
+
+ /* Check the regions are what we expect */
+ if (check_regions(fdt, flags))
+ FAIL();
+ else
+ PASS();
+
+ return 0;
+}
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index e04512c..59b6055 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -246,6 +246,11 @@ libfdt_tests () {
# Specific bug tests
run_test add_subnode_with_nops
+
+ # Tests for fdt_find_regions()
+ for flags in 0 1 2 3 4 5 6 7; do
+ run_test region_tree ${flags}
+ done
}
dtc_tests () {
--
1.7.7.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 6/8] Add fdtgrep to grep and subset FDTs
[not found] ` <1358801962-21707-1-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
` (4 preceding siblings ...)
2013-01-21 20:59 ` [PATCH 5/8] libfdt: Add function to find regions in an FDT Simon Glass
@ 2013-01-21 20:59 ` Simon Glass
2013-01-21 20:59 ` [PATCH 7/8] Remove fdtdump and use fdtgrep instead Simon Glass
2013-01-21 20:59 ` [PATCH 8/8] RFC: Check offset in fdt_string() Simon Glass
7 siblings, 0 replies; 21+ messages in thread
From: Simon Glass @ 2013-01-21 20:59 UTC (permalink / raw)
To: Jon Loeliger; +Cc: Devicetree Discuss
fdtgrep operates like a grep for device trees, working on binary .dtb
files.
fdtgrep can find nodes by name or compatible string. It can find properties
by name. Using a list of of nodes/properties/compatible strings to match,
fdtgrep creates either .dts text output containing just those matches, or
.dtb output. In the .dtb case, the output is a valid FDT which only includes
the selected nodes/properties. This is useful in resource-constrained systems
where a full FDT is too large to fit in available memory (e.g. U-Boot SPL).
fdtgrep also supports producing raw binary output, without the normal FDT
headers. This can be useful for hashing part of an FDT and making sure that
that part does not change, even if other parts do. This makes it possible to
detect an important change, while ignoring a change that is of no interest.
At its simplest fdtgrep can be used as follows;
fdtgrep /holiday tests/grep.dtb
and it produces just that node (with a skeleton to hold it):
/ {
holiday {
compatible = "ixtapa", "mexico";
weather = "sunny";
status = "okay";
};
};
The binary form of this is:
fdtgrep -n /holiday -O dtb tests/grep.dtb
which produces a valid DTB file with just the above nodes.
Various options are provided to search for properties, to ensure that
nodes/properties are not present, and to search for nodes by compatible
string.
Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
.gitignore | 1 +
Makefile | 4 +
Makefile.utils | 7 +
fdtgrep.c | 775 ++++++++++++++++++++++++++++++++++++++++++++++++++++
tests/grep.dts | 23 ++
tests/run_tests.sh | 348 +++++++++++++++++++++++-
tests/tests.sh | 1 +
7 files changed, 1158 insertions(+), 1 deletions(-)
create mode 100644 fdtgrep.c
create mode 100644 tests/grep.dts
diff --git a/.gitignore b/.gitignore
index 545b899..9d3a550 100644
--- a/.gitignore
+++ b/.gitignore
@@ -12,6 +12,7 @@ lex.yy.c
/convert-dtsv0
/version_gen.h
/fdtget
+/fdtgrep
/fdtput
/patches
/.pc
diff --git a/Makefile b/Makefile
index 1169e6c..96e0488 100644
--- a/Makefile
+++ b/Makefile
@@ -112,6 +112,7 @@ BIN += dtc
BIN += fdtdump
BIN += fdtget
BIN += fdtput
+BIN += fdtgrep
SCRIPTS = dtdiff
@@ -124,6 +125,7 @@ ifneq ($(DEPTARGETS),)
-include $(FDTDUMP_OBJS:%.o=%.d)
-include $(FDTGET_OBJS:%.o=%.d)
-include $(FDTPUT_OBJS:%.o=%.d)
+-include $(FDTGREP_OBJS:%.o=%.d)
endif
@@ -188,6 +190,7 @@ fdtget: $(FDTGET_OBJS) $(LIBFDT_archive)
fdtput: $(FDTPUT_OBJS) $(LIBFDT_archive)
+fdtgrep: $(FDTGREP_OBJS) $(LIBFDT_archive)
#
# Testsuite rules
@@ -198,6 +201,7 @@ TESTS_BIN += dtc
TESTS_BIN += convert-dtsv0
TESTS_BIN += fdtput
TESTS_BIN += fdtget
+TESTS_BIN += fdtgrep
include tests/Makefile.tests
diff --git a/Makefile.utils b/Makefile.utils
index 48ece49..f2a7001 100644
--- a/Makefile.utils
+++ b/Makefile.utils
@@ -22,3 +22,10 @@ FDTPUT_SRCS = \
util.c
FDTPUT_OBJS = $(FDTPUT_SRCS:%.c=%.o)
+
+
+FDTGREP_SRCS = \
+ fdtgrep.c \
+ util.c
+
+FDTGREP_OBJS = $(FDTGREP_SRCS:%.c=%.o)
diff --git a/fdtgrep.c b/fdtgrep.c
new file mode 100644
index 0000000..bb08f12
--- /dev/null
+++ b/fdtgrep.c
@@ -0,0 +1,775 @@
+/*
+ * Copyright (c) 2013, Google Inc.
+ *
+ * Perform a grep of an FDT either displaying the source subset or producing
+ * a new .dtb subset which can be used as required.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <assert.h>
+#include <ctype.h>
+#include <getopt.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <libfdt.h>
+#include <libfdt_internal.h>
+#include "util.h"
+
+/* Define DEBUG to get some debugging output on stderr */
+#ifdef DEBUG
+#define debug(a, b...) fprintf(stderr, a, ## b)
+#else
+#define debug(a, b...)
+#endif
+
+/* A linked list of values we are grepping for */
+struct value_node {
+ int type; /* Types this value matches (FDT_IS... mask) */
+ int include; /* 1 to include matches, 0 to exclude */
+ const char *string; /* String to match */
+ struct value_node *next; /* Pointer to next node, or NULL */
+};
+
+/* Output formats we support */
+enum output_t {
+ OUT_DTS, /* Device tree source */
+ OUT_DTB, /* Valid device tree binary */
+ OUT_BIN, /* Fragment of .dtb, for hashing */
+};
+
+/* Holds information which controls our output and options */
+struct display_info {
+ enum output_t output; /* Output format */
+ int all; /* Display all properties/nodes */
+ int colour; /* Display output in ANSI colour */
+ int region_list; /* Output a region list */
+ int flags; /* Flags (FDT_REG_...) */
+ int list_strings; /* List strings in string table */
+ int show_offset; /* Show address / offset */
+ int header; /* Output an FDT header */
+ int diff; /* Show +/- diff markers */
+ int show_dts_version; /* Put '/dts-v1/;' on the first line */
+ int types_inc; /* Mask of types that we include (FDT_IS...) */
+ int types_exc; /* Mask of types that we exclude (FDT_IS...) */
+ int invert; /* Invert polarity of match */
+ struct value_node *value_head; /* List of values to match */
+ const char *output_fname; /* Output filename */
+ FILE *fout; /* File to write dts/dtb output */
+};
+
+static void report_error(const char *where, int err)
+{
+ fprintf(stderr, "Error at '%s': %s\n", where, fdt_strerror(err));
+}
+
+/* Supported ANSI colours */
+enum {
+ COL_BLACK,
+ COL_RED,
+ COL_GREEN,
+ COL_YELLOW,
+ COL_BLUE,
+ COL_MAGENTA,
+ COL_CYAN,
+ COL_WHITE,
+
+ COL_NONE = -1,
+};
+
+/**
+ * print_ansi_colour() - Print out the ANSI sequence for a colour
+ *
+ * @fout: Output file
+ * @col: Colour to output (COL_...), or COL_NONE to reset colour
+ */
+static void print_ansi_colour(FILE *fout, int col)
+{
+ if (col == COL_NONE)
+ fprintf(fout, "\033[0m");
+ else
+ fprintf(fout, "\033[1;%dm", col + 30);
+}
+
+
+/**
+ * value_add() - Add a new value to our list of things to grep for
+ *
+ * @disp: Display structure, holding info about our options
+ * @headp: Pointer to header pointer of list
+ * @type: Type of this value (FDT_IS_...)
+ * @include: 1 if we want to include matches, 0 to exclude
+ * @str: String value to match
+ */
+static int value_add(struct display_info *disp, struct value_node **headp,
+ int type, int include, const char *str)
+{
+ struct value_node *node;
+
+ /*
+ * Keep track of which types we are excluding/including. We don't
+ * allow both including and excluding things, because it doesn't make
+ * sense. 'Including' means that everything not mentioned is
+ * excluded. 'Excluding' means that everything not mentioned is
+ * included. So using the two together would be meaningless.
+ */
+ if (include)
+ disp->types_inc |= type;
+ else
+ disp->types_exc |= type;
+ if (disp->types_inc & disp->types_exc & type) {
+ fprintf(stderr, "Cannot use both include and exclude"
+ " for '%s'\n", str);
+ return -1;
+ }
+
+ str = strdup(str);
+ node = malloc(sizeof(*node));
+ if (!str || !node) {
+ fprintf(stderr, "Out of memory\n");
+ return -1;
+ }
+ node->next = *headp;
+ node->type = type;
+ node->include = include;
+ node->string = str;
+ *headp = node;
+
+ return 0;
+}
+
+/**
+ * display_fdt_by_regions() - Display regions of an FDT source
+ *
+ * This dumps an FDT as source, but only certain regions of it. This is the
+ * final stage of the grep - we have a list of regions we want to display,
+ * and this function displays them.
+ *
+ * @disp: Display structure, holding info about our options
+ * @blob: FDT blob to display
+ * @region: List of regions to display
+ * @count: Number of regions
+ */
+static int display_fdt_by_regions(struct display_info *disp, const void *blob,
+ struct fdt_region region[], int count)
+{
+ struct fdt_region *reg = region, *reg_end = region + count;
+ uint32_t off_mem_rsvmap = fdt_off_mem_rsvmap(blob);
+ int base = fdt_off_dt_struct(blob);
+ int version = fdt_version(blob);
+ int offset, nextoffset;
+ int tag, depth, shift;
+ FILE *f = disp->fout;
+ uint64_t addr, size;
+ int in_region;
+ int file_ofs;
+ int i;
+
+ if (disp->show_dts_version)
+ fprintf(f, "/dts-v1/;\n");
+
+ if (disp->header) {
+ fprintf(f, "// magic:\t\t0x%x\n", fdt_magic(blob));
+ fprintf(f, "// totalsize:\t\t0x%x (%d)\n", fdt_totalsize(blob),
+ fdt_totalsize(blob));
+ fprintf(f, "// off_dt_struct:\t0x%x\n",
+ fdt_off_dt_struct(blob));
+ fprintf(f, "// off_dt_strings:\t0x%x\n",
+ fdt_off_dt_strings(blob));
+ fprintf(f, "// off_mem_rsvmap:\t0x%x\n", off_mem_rsvmap);
+ fprintf(f, "// version:\t\t%d\n", version);
+ fprintf(f, "// last_comp_version:\t%d\n",
+ fdt_last_comp_version(blob));
+ if (version >= 2) {
+ fprintf(f, "// boot_cpuid_phys:\t0x%x\n",
+ fdt_boot_cpuid_phys(blob));
+ }
+ if (version >= 3) {
+ fprintf(f, "// size_dt_strings:\t0x%x\n",
+ fdt_size_dt_strings(blob));
+ }
+ if (version >= 17) {
+ fprintf(f, "// size_dt_struct:\t0x%x\n",
+ fdt_size_dt_struct(blob));
+ }
+ fprintf(f, "\n");
+ }
+
+ if (disp->flags & FDT_REG_ADD_MEM_RSVMAP) {
+ const struct fdt_reserve_entry *p_rsvmap;
+
+ p_rsvmap = (const struct fdt_reserve_entry *)
+ ((const char *)blob + off_mem_rsvmap);
+ for (i = 0; ; i++) {
+ addr = fdt64_to_cpu(p_rsvmap[i].address);
+ size = fdt64_to_cpu(p_rsvmap[i].size);
+ if (addr == 0 && size == 0)
+ break;
+
+ fprintf(f, "/memreserve/ %llx %llx;\n",
+ (unsigned long long)addr, (unsigned long long)size);
+ }
+ }
+
+ depth = nextoffset = 0;
+ shift = 4; /* 4 spaces per indent */
+ do {
+ const struct fdt_property *prop;
+ const char *name;
+ int show;
+ int len;
+
+ offset = nextoffset;
+
+ /*
+ * Work out the file offset of this offset, and decide
+ * whether it is in the region list or not
+ */
+ file_ofs = base + offset;
+ if (reg < reg_end && file_ofs >= reg->offset + reg->size)
+ reg++;
+ in_region = reg < reg_end && file_ofs >= reg->offset &&
+ file_ofs < reg->offset + reg->size;
+ tag = fdt_next_tag(blob, offset, &nextoffset);
+
+ if (tag == FDT_END)
+ break;
+ show = in_region || disp->all;
+ if (show && disp->diff)
+ fprintf(f, "%c", in_region ? '+' : '-');
+
+ if (!show) {
+ /* Do this here to avoid 'if (show)' in every 'case' */
+ if (tag == FDT_BEGIN_NODE)
+ depth++;
+ else if (tag == FDT_END_NODE)
+ depth--;
+ continue;
+ }
+ if (disp->show_offset && tag != FDT_END)
+ fprintf(f, "%4x: ", file_ofs);
+
+ /* Green means included, red means excluded */
+ if (disp->colour)
+ print_ansi_colour(f, in_region ? COL_GREEN : COL_RED);
+
+ switch (tag) {
+ case FDT_PROP:
+ prop = fdt_get_property_by_offset(blob, offset, NULL);
+ name = fdt_string(blob, fdt32_to_cpu(prop->nameoff));
+ fprintf(f, "%*s%s", depth * shift, "", name);
+ utilfdt_print_data(prop->data,
+ fdt32_to_cpu(prop->len));
+ fprintf(f, ";");
+ break;
+
+ case FDT_NOP:
+ fprintf(f, "%*s// [NOP]", depth * shift, "");
+ break;
+
+ case FDT_BEGIN_NODE:
+ name = fdt_get_name(blob, offset, &len);
+ fprintf(f, "%*s%s {", depth++ * shift, "",
+ *name ? name : "/");
+ break;
+
+ case FDT_END_NODE:
+ fprintf(f, "%*s};", --depth * shift, "");
+ break;
+ }
+
+ /* Reset colour back to normal before end of line */
+ if (disp->colour)
+ print_ansi_colour(f, COL_NONE);
+ fprintf(f, "\n");
+ } while (1);
+
+ /* Print a list of strings if requested */
+ if (disp->list_strings) {
+ const char *str;
+ int offset;
+
+ for (offset = 0; offset < fdt_size_dt_strings(blob);
+ offset += strlen(str) + 1) {
+ str = fdt_string(blob, offset);
+ int len = strlen(str) + 1;
+ int show;
+
+ /* Only print strings that are in the region */
+ file_ofs = fdt_off_dt_strings(blob) + offset;
+ in_region = reg < reg_end &&
+ file_ofs >= reg->offset &&
+ file_ofs + len < reg->offset +
+ reg->size;
+ show = in_region || disp->all;
+ if (show && disp->diff)
+ printf("%c", in_region ? '+' : '-');
+ if (disp->show_offset)
+ printf("%4x: ", offset);
+ printf("%s\n", str);
+ }
+ }
+
+ return 0;
+}
+
+/**
+ * dump_fdt_regions() - Dump regions of an FDT as binary data
+ *
+ * This dumps an FDT as binary, but only certain regions of it. This is the
+ * final stage of the grep - we have a list of regions we want to dump,
+ * and this function dumps them.
+ *
+ * The output of this function may or may not be a valid FDT. To ensure it
+ * is, these disp->flags must be set:
+ *
+ * FDT_REG_SUPERNODES: ensures that subnodes are preceeded by their
+ * parents. Without this option, fragments of subnode data may be
+ * output without the supernodes above them. This is useful for
+ * hashing but cannot produce a valid FDT.
+ * FDT_REG_ADD_STRING_TAB: Adds a string table to the end of the FDT.
+ * Without this none of the properties will have names
+ * FDT_REG_ADD_MEM_RSVMAP: Adds a mem_rsvmap table - an FDT is invalid
+ * without this.
+ *
+ * @disp: Display structure, holding info about our options
+ * @blob: FDT blob to display
+ * @region: List of regions to display
+ * @count: Number of regions
+ */
+static int dump_fdt_regions(struct display_info *disp, const void *blob,
+ struct fdt_region region[], int count)
+{
+ struct fdt_header hdr, *fdt = &hdr;
+ int size, struct_start;
+ FILE *f = disp->fout;
+ int i;
+
+ /* Set up a basic header (even if we don't actually write it) */
+ memset(fdt, '\0', sizeof(*fdt));
+ fdt_set_magic(fdt, FDT_MAGIC);
+ struct_start = FDT_ALIGN(sizeof(struct fdt_header),
+ sizeof(struct fdt_reserve_entry));
+ fdt_set_off_mem_rsvmap(fdt, struct_start);
+ fdt_set_version(fdt, FDT_LAST_SUPPORTED_VERSION);
+ fdt_set_last_comp_version(fdt, FDT_FIRST_SUPPORTED_VERSION);
+
+ /*
+ * Calculate the total size of the regions we are writing out. The
+ * first will be the mem_rsvmap if the FDT_REG_ADD_MEM_RSVMAP flag
+ * is set. The last will be the string table if FDT_REG_ADD_STRING_TAB
+ * is set.
+ */
+ for (i = size = 0; i < count; i++)
+ size += region[i].size;
+
+ /* Bring in the mem_rsvmap section from the old file if requested */
+ if (count > 0 && (disp->flags & FDT_REG_ADD_MEM_RSVMAP)) {
+ struct_start += region[0].size;
+ size -= region[0].size;
+ }
+ fdt_set_off_dt_struct(fdt, struct_start);
+
+ /* Update the header to have the correct offsets/sizes */
+ if (count >= 2 && (disp->flags & FDT_REG_ADD_STRING_TAB)) {
+ int str_size;
+
+ str_size = region[count - 1].size;
+ fdt_set_size_dt_struct(fdt, size - str_size);
+ fdt_set_off_dt_strings(fdt, struct_start + size - str_size);
+ fdt_set_size_dt_strings(fdt, str_size);
+ fdt_set_totalsize(fdt, struct_start + size);
+ }
+
+ /* Write the header if required */
+ if (disp->header) {
+ if (sizeof(hdr) != fwrite(fdt, 1, sizeof(hdr), f))
+ return -1;
+ for (i = sizeof(hdr); i < fdt_off_mem_rsvmap(&hdr); i++)
+ fputc('\0', f);
+ }
+
+ /* Output all the nodes including any mem_rsvmap/string table */
+ for (i = size = 0; i < count; i++) {
+ struct fdt_region *reg = ®ion[i];
+
+ if (reg->size != fwrite((const char *)blob + reg->offset, 1,
+ reg->size, f))
+ return -1;
+ size += reg->size;
+ }
+
+ return 0;
+}
+
+/**
+ * show_region_list() - Print out a list of regions
+ *
+ * The list includes the region offset (absolute offset from start of FDT
+ * blob in bytes) and size
+ *
+ * @reg: List of regions to print
+ * @count: Number of regions
+ */
+static void show_region_list(struct fdt_region *reg, int count)
+{
+ int i;
+
+ printf("Regions: %d\n", count);
+ for (i = 0; i < count; i++, reg++) {
+ printf("%d: %-10x %-10x\n", i, reg->offset,
+ reg->offset + reg->size);
+ }
+}
+
+/**
+ * h_include() - Include handler function for fdt_find_regions()
+ *
+ * This function decides whether to include or exclude a node, property or
+ * compatible string. The function is defined by fdt_find_regions().
+ *
+ * The algorithm is documented in the code - disp->invert is 0 for normal
+ * operation, and 1 to invert the sense of all matches.
+ */
+static int h_include(void *priv, int type, const char *data, int size)
+{
+ struct display_info *disp = priv;
+ struct value_node *val;
+ int match, none_match = FDT_IS_ANY;
+
+ /* If none of our conditions mention this type, we know nothing */
+ debug("type=%x, data=%s\n", type, data);
+ if (!((disp->types_inc | disp->types_exc) & type)) {
+ debug(" - not in any condition\n");
+ return -1;
+ }
+
+ /*
+ * Go through the list of conditions. For inclusive conditions, we
+ * return 1 at the first match. For exclusive conditions, we must
+ * check that there are no matches.
+ */
+ for (val = disp->value_head; val; val = val->next) {
+ if (!(type & val->type))
+ continue;
+ match = fdt_stringlist_contains(data, size, val->string);
+ if (match && val->include) {
+ debug(" - match inc %s\n", val->string);
+ return !disp->invert;
+ }
+ if (match)
+ none_match &= ~val->type;
+ }
+
+ /*
+ * If this is an exclusive condition, and nothing matches, then we
+ * should return 1
+ */
+ if ((type & disp->types_exc) && (none_match & type)) {
+ /*
+ * Allow FDT_IS_COMPAT to make the final decision in the
+ * case where there is no specific type
+ */
+ if (type == FDT_IS_NODE && disp->types_exc == FDT_IS_ANY) {
+ debug(" - supressed exc node\n");
+ return -1;
+ }
+ debug(" - match exc\n");
+ return !disp->invert;
+ }
+
+ /*
+ * Allow FDT_IS_COMPAT to make the final decision in the
+ * case where there is no specific type (inclusive)
+ */
+ if (type == FDT_IS_NODE && disp->types_inc == FDT_IS_ANY)
+ return -1;
+
+ debug(" - no match, types_inc=%x, types_exc=%x, none_match=%x\n",
+ disp->types_inc, disp->types_exc, none_match);
+
+ return disp->invert;
+}
+
+/**
+ * Run the main fdtgrep operation, given a filename and valid arguments
+ *
+ * @param disp Display information / options
+ * @param filename Filename of blob file
+ * @param return 0 if ok, -ve on error
+ */
+static int do_fdtgrep(struct display_info *disp, const char *filename)
+{
+ struct fdt_region *region;
+ int max_regions;
+ int count = 100;
+ char path[1024];
+ char *blob;
+ int i, ret;
+
+ blob = utilfdt_read(filename);
+ if (!blob)
+ return -1;
+ ret = fdt_check_header(blob);
+ if (ret) {
+ fprintf(stderr, "Error: %s\n", fdt_strerror(ret));
+ return ret;
+ }
+
+ /* Allow old files, but they are untested */
+ if (fdt_version(blob) < 17 && disp->value_head) {
+ fprintf(stderr, "Warning: fdtgrep does not fully support"
+ " version %d files\n", fdt_version(blob));
+ }
+
+ /*
+ * We do two passes, since we don't know how many regions we need.
+ * The first pass will count the regions, but if it is too many,
+ * we do another pass to actually record them.
+ */
+ for (i = 0; i < 2; i++) {
+ region = malloc(count * sizeof(struct fdt_region));
+ if (!region) {
+ fprintf(stderr, "Out of memory for %d regions\n",
+ count);
+ return -1;
+ }
+ max_regions = count;
+ count = fdt_find_regions(blob,
+ h_include, disp,
+ region, max_regions, path, sizeof(path),
+ disp->flags);
+ if (count < 0) {
+ report_error("fdt_find_regions", count);
+ return -1;
+ }
+ if (count <= max_regions)
+ break;
+ free(region);
+ }
+
+ /* Optionally print a list of regions */
+ if (disp->region_list)
+ show_region_list(region, count);
+
+ /* Output either source .dts or binary .dtb */
+ if (disp->output == OUT_DTS) {
+ ret = display_fdt_by_regions(disp, blob, region, count);
+ } else {
+ ret = dump_fdt_regions(disp, blob, region, count);
+ if (ret)
+ fprintf(stderr, "Write failure\n");
+ }
+
+ free(blob);
+ free(region);
+
+ return ret;
+}
+
+static const char *usage_msg =
+ "fdtgrep - extract portions from device tree\n"
+ "\n"
+ "Usage:\n"
+ " fdtgrep <options> [<match nodes/prop/compat>...] <dt file>|-\n"
+ "Options:\n"
+ "\t-a\tDisplay address / offset\n"
+ "\t-A\tShow all nodes/tags, colour those that match\n"
+ "\t-c/-C <node>\tCompatible nodes to include/exclude in grep\n"
+ "\t-d\tDiff: Mark matching nodes with +, others with -\n"
+ "\t-g/-G <node>\tNode/property/compatible string to include/exclude\n"
+ "\t-H\tOutput a header\n"
+ "\t-l\tOutput a region list\n"
+ "\t-L\tList strings in string table\n"
+ "\t-m\tInclude mem_rsvmap section in binary output\n"
+ "\t-n/-N <node>\tNode to include/exclude in grep\n"
+ "\t-p/-P <node>\tProperty to include/exclude in grep\n"
+ "\t-s\tInclude direct subnode names of matching nodes\n"
+ "\t-S\tDon't include supernodes of matching nodes\n"
+ "\t-t\tInclude string table in binary output\n"
+ "\t-o <output file>\n"
+ "\t-O <output format>\n"
+ "\t\tOutput formats are:\n"
+ "\t\t\tdts - device tree source text\n"
+ "\t\t\tdtb - device tree blob (sets -Hmt automatically)\n"
+ "\t\t\tbin - device tree fragment (may not be a valid .dtb)\n"
+ "\t-v\tInvert the sense of matching, to select non-matching lines\n"
+ "\t-V\tPut \"/dts-v1/;\" on first line of dts output\n"
+ "\t-h\t\tPrint this help\n\n"
+ USAGE_TYPE_MSG;
+
+static void usage(const char *msg)
+{
+ if (msg)
+ fprintf(stderr, "Error: %s\n\n", msg);
+
+ fprintf(stderr, "%s", usage_msg);
+ exit(2);
+}
+
+static void scan_args(struct display_info *disp, int argc, char *argv[])
+{
+ for (;;) {
+ int c = getopt(argc, argv,
+ "haAc:C:dg:G:HlLmn:N:o:O:p:P:sStvV");
+ int type = 0;
+ int inc = 1;
+
+ if (c == -1)
+ break;
+
+ switch (c) {
+ case 'h':
+ case '?':
+ usage(NULL);
+ case 'a':
+ disp->show_offset = 1;
+ break;
+ case 'A':
+ disp->all = 1;
+ break;
+ case 'C':
+ inc = 0;
+ /* no break */
+ case 'c':
+ type = FDT_IS_COMPAT;
+ break;
+ case 'd':
+ disp->diff = 1;
+ break;
+ case 'G':
+ inc = 0;
+ /* no break */
+ case 'g':
+ type = FDT_IS_ANY;
+ break;
+ case 'H':
+ disp->header = 1;
+ break;
+ case 'l':
+ disp->region_list = 1;
+ break;
+ case 'L':
+ disp->list_strings = 1;
+ break;
+ case 'm':
+ disp->flags |= FDT_REG_ADD_MEM_RSVMAP;
+ break;
+ case 'N':
+ inc = 0;
+ /* no break */
+ case 'n':
+ type = FDT_IS_NODE;
+ break;
+ case 'o':
+ disp->output_fname = optarg;
+ break;
+ case 'O':
+ if (!strcmp(optarg, "dtb"))
+ disp->output = OUT_DTB;
+ else if (!strcmp(optarg, "dts"))
+ disp->output = OUT_DTS;
+ else if (!strcmp(optarg, "bin"))
+ disp->output = OUT_BIN;
+ else
+ usage("Unknown output format");
+ break;
+ case 'P':
+ inc = 0;
+ /* no break */
+ case 'p':
+ type = FDT_IS_PROP;
+ break;
+ case 's':
+ disp->flags |= FDT_REG_DIRECT_SUBNODES;
+ break;
+ case 'S':
+ disp->flags &= ~FDT_REG_SUPERNODES;
+ break;
+ case 't':
+ disp->flags |= FDT_REG_ADD_STRING_TAB;
+ break;
+ case 'v':
+ disp->invert = 1;
+ break;
+ case 'V':
+ disp->show_dts_version = 1;
+ break;
+ }
+
+ if (type && value_add(disp, &disp->value_head, type, inc,
+ optarg))
+ usage("Cannot add value");
+ }
+
+ if (disp->invert && disp->types_exc)
+ usage("-v has no meaning when used with 'exclude' conditions");
+}
+
+int main(int argc, char *argv[])
+{
+ char *filename = NULL;
+ struct display_info disp;
+ int ret;
+
+ /* set defaults */
+ memset(&disp, '\0', sizeof(disp));
+ disp.flags = FDT_REG_SUPERNODES; /* Default flags */
+
+ scan_args(&disp, argc, argv);
+
+ /* Show matched lines in colour if we can */
+ disp.colour = disp.all && isatty(0);
+
+ /* Any additional arguments can match anything, just like -g */
+ while (optind < argc - 1) {
+ if (value_add(&disp, &disp.value_head, FDT_IS_ANY, 1,
+ argv[optind++]))
+ usage("Cannot add value");
+ }
+
+ if (optind < argc)
+ filename = argv[optind++];
+ if (!filename)
+ usage("Missing filename");
+
+ /* If a valid .dtb is required, set flags to ensure we get one */
+ if (disp.output == OUT_DTB) {
+ disp.header = 1;
+ disp.flags |= FDT_REG_SUPERNODES;
+ disp.flags |= FDT_REG_ADD_MEM_RSVMAP | FDT_REG_ADD_STRING_TAB;
+ }
+
+ if (disp.output_fname) {
+ disp.fout = fopen(disp.output_fname, "w");
+ if (!disp.fout)
+ usage("Cannot open output file");
+ } else {
+ disp.fout = stdout;
+ }
+
+ /* Run the grep and output the results */
+ ret = do_fdtgrep(&disp, filename);
+ if (disp.output_fname)
+ fclose(disp.fout);
+ if (ret)
+ return 1;
+
+ return 0;
+}
diff --git a/tests/grep.dts b/tests/grep.dts
new file mode 100644
index 0000000..9600657
--- /dev/null
+++ b/tests/grep.dts
@@ -0,0 +1,23 @@
+/dts-v1/;
+/memreserve/ 1 2;
+/ {
+ model = "MyBoardName";
+ compatible = "MyBoardName", "MyBoardFamilyName";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ chosen {
+ bootargs = "root=/dev/sda2";
+ linux,platform = <0x600>;
+ };
+ holiday {
+ compatible = "ixtapa", "mexico";
+ weather = "sunny";
+ status = "okay";
+ flight@1 {
+ airline = "alaska";
+ };
+ flight@2 {
+ airline = "lan";
+ };
+ };
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 59b6055..19c6264 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -584,6 +584,349 @@ utilfdt_tests () {
run_test utilfdt_test
}
+# Add a property to a tree, then hash it and see if it changed
+# Args:
+# $1: 0 if we expect it to stay the same, 1 if we expect a change
+# $2: node to add a property to
+# $3: arguments for fdtget
+# $4: filename of device tree binary
+# $5: hash of unchanged file (empty to calculate it)
+# $6: node to add a property to ("testing" by default if empty)
+check_hash () {
+ local changed="$1"
+ local node="$2"
+ local args="$3"
+ local tree="$4"
+ local base="$5"
+ local nodename="$6"
+
+ if [ -z "$nodename" ]; then
+ nodename=testing
+ fi
+ if [ -z "$base" ]; then
+ base=$($DTGREP ${args} -O bin $tree | sha1sum)
+ fi
+ $DTPUT $tree $node $nodename 1
+ hash=$($DTGREP ${args} -O bin $tree | sha1sum)
+ if [ "$base" == "$hash" ]; then
+ if [ "$changed" == 1 ]; then
+ echo "$test: Hash should have changed"
+ echo base $base
+ echo hash $hash
+ false
+ fi
+ else
+ if [ "$changed" == 0 ]; then
+ echo "$test: Base hash is $base but it was changed to $hash"
+ false
+ fi
+ fi
+}
+
+# Check the number of lines generated matches what we expect
+# Args:
+# $1: Expected number of lines
+# $2...: Command line to run to generate output
+check_lines () {
+ local base="$1"
+
+ shift
+ lines=$($@ | wc -l)
+ if [ "$base" != "$lines" ]; then
+ echo "Expected $base lines but got $lines lines"
+ false
+ fi
+}
+
+# Check the number of bytes generated matches what we expect
+# Args:
+# $1: Expected number of bytes
+# $2...: Command line to run to generate output
+check_bytes () {
+ local base="$1"
+
+ shift
+ bytes=$($@ | wc -c)
+ if [ "$base" != "$bytes" ]; then
+ echo "Expected $base bytes but got $bytes bytes"
+ false
+ fi
+}
+
+# Check whether a command generates output which contains a string
+# Args:
+# $1: 0 to expect the string to be absent, 1 to expect it to be present
+# $2: text to grep for
+# $3...: Command to execute
+check_contains () {
+ contains="$1"
+ text="$2"
+
+ shift 2
+ if $@ | grep -q $text; then
+ if [ $contains -ne 1 ]; then
+ echo "Did not expect to find $text in output"
+ false
+ fi
+ else
+ if [ $contains -ne 0 ]; then
+ echo "Expected to find $text in output"
+ false
+ fi
+ fi
+}
+
+# Check that $2 and $3 are equal. $1 is the test name to display
+equal_test () {
+ echo -n "$1: "
+ if [ "$2" == "$3" ]; then
+ PASS
+ else
+ FAIL "$2 != $3"
+ fi
+}
+
+fdtgrep_tests () {
+ local addr
+ local all_lines # Total source lines in .dts file
+ local base
+ local dt_start
+ local lines
+ local node_lines # Number of lines of 'struct' output
+ local orig
+ local string_size
+ local tmp
+ local tree
+
+ tmp=/tmp/tests.$$
+ orig=region_tree.test.dtb
+ ./region_tree 0 1000 ${orig}
+
+ # Hash of partial tree
+ # - modify tree in various ways and check that hash is unaffected
+ tree=region_tree.mod.dtb
+ cp $orig $tree
+ args="-n /images/kernel@1"
+ run_wrap_test check_hash 0 /images "$args" $tree
+ run_wrap_test check_hash 0 /images/kernel@1/hash@1 "$args" $tree
+ run_wrap_test check_hash 0 / "$args" $tree
+ $DTPUT -c $tree /images/kernel@1/newnode
+ run_wrap_test check_hash 0 / "$args" $tree
+ run_wrap_test check_hash 1 /images/kernel@1 "$args" $tree
+
+ # Now hash immediate subnodes so we detect a new subnode added
+ cp $orig $tree
+ args="-n /images/kernel@1 -s"
+ run_wrap_test check_hash 0 /images "$args" $tree
+ run_wrap_test check_hash 0 /images/kernel@1/hash@1 "$args" $tree
+ run_wrap_test check_hash 0 / "$args" $tree
+ base=$($DTGREP $args -O bin $tree | sha1sum)
+ $DTPUT -c $tree /images/kernel@1/newnode
+ run_wrap_test check_hash 1 / "$args" $tree "$base"
+ cp $orig $tree
+ run_wrap_test check_hash 1 /images/kernel@1 "$args" $tree
+
+ # Hash the string table, which should change if we add a new property name
+ # (Adding an existing property name will just reuse that string)
+ cp $orig $tree
+ args="-t -n /images/kernel@1"
+ run_wrap_test check_hash 0 /images "$args" $tree "" data
+ run_wrap_test check_hash 1 /images/kernel@1 "$args" $tree
+
+ dts=grep.dts
+ dtb=grep.dtb
+ run_dtc_test -O dtb -p 0x1000 -o $dtb $dts
+
+ # Tests for each argument are roughly in alphabetical order
+ #
+ # First a sanity check that we can get back the source from the .dtb
+ all_lines=$(cat $dts | wc -l)
+ run_wrap_test check_lines ${all_lines} $DTGREP -Vm $dtb
+ node_lines=$(($all_lines - 2))
+
+ # Get the offset of the dt_struct start (also tests -H somewhat)
+ dt_start=$($DTGREP -H $dtb | awk '/off_dt_struct:/ {print $3}')
+ dt_size=$($DTGREP -H $dtb | awk '/size_dt_struct:/ {print $3}')
+
+ # Check -a: the first line should contain the offset of the dt_start
+ addr=$($DTGREP -a $dtb | head -1 | tr -d : | awk '{print $1}')
+ run_wrap_test equal_test "-a offset first" "$dt_start" "0x$addr"
+
+ # Last line should be 8 bytes less than the size (NODE, END tags)
+ addr=$($DTGREP -a $dtb | tail -1 | tr -d : | awk '{print $1}')
+ last=$(printf "%#x" $(($dt_start + $dt_size - 8)))
+ run_wrap_test equal_test "-a offset last" "$last" "0x$addr"
+
+ # Check that -A controls display of all lines
+ # The 'chosen' node should only have four output lines
+ run_wrap_test check_lines $node_lines $DTGREP -S -A -n /chosen $dtb
+ run_wrap_test check_lines 4 $DTGREP -S -n /chosen $dtb
+
+ # Check that -c picks out nodes
+ run_wrap_test check_lines 5 $DTGREP -S -c ixtapa $dtb
+ run_wrap_test check_lines $(($node_lines - 5)) $DTGREP -S -C ixtapa $dtb
+
+ # -d marks selected lines with +
+ run_wrap_test check_lines $node_lines $DTGREP -S -Ad -n /chosen $dtb
+ run_wrap_test check_lines 4 $DTGREP -S -Ad -n /chosen $dtb |grep +
+
+ # -g should find a node, property or compatible string
+ run_wrap_test check_lines 2 $DTGREP -S -g / $dtb
+ run_wrap_test check_lines 2 $DTGREP -S -g /chosen $dtb
+ run_wrap_test check_lines $(($node_lines - 2)) $DTGREP -S -G /chosen $dtb
+
+ run_wrap_test check_lines 1 $DTGREP -S -g bootargs $dtb
+ run_wrap_test check_lines $(($node_lines - 1)) $DTGREP -S -G bootargs $dtb
+
+ # We should find the /holiday node, so 1 line for 'holiday {', one for '}'
+ run_wrap_test check_lines 2 $DTGREP -S -g ixtapa $dtb
+ run_wrap_test check_lines $(($node_lines - 2)) $DTGREP -S -G ixtapa $dtb
+
+ run_wrap_test check_lines 3 $DTGREP -S -g ixtapa -g bootargs $dtb
+ run_wrap_test check_lines $(($node_lines - 3)) $DTGREP -S -G ixtapa \
+ -G bootargs $dtb
+
+ # -l outputs a,list of regions - here we should get 3: one for the header,
+ # one for the node and one for the 'end' tag.
+ run_wrap_test check_lines 3 $DTGREP -S -l -n /chosen $dtb -o $tmp
+
+ # -L outputs all the strings in the string table
+ cat >$tmp <<END
+ #address-cells
+ airline
+ bootargs
+ compatible
+ linux,platform
+ model
+ #size-cells
+ status
+ weather
+END
+ lines=$(cat $tmp | wc -l)
+ run_wrap_test check_lines $lines $DTGREP -S -L -n // $dtb
+
+ # Check that the -m flag works
+ run_wrap_test check_contains 1 memreserve $DTGREP -Vm $dtb
+ run_wrap_test check_contains 0 memreserve $DTGREP -V $dtb
+
+ # Test -n
+ run_wrap_test check_lines 0 $DTGREP -S -n // $dtb
+ run_wrap_test check_lines 0 $DTGREP -S -n chosen $dtb
+ run_wrap_test check_lines 0 $DTGREP -S -n holiday $dtb
+ run_wrap_test check_lines 0 $DTGREP -S -n \"\" $dtb
+ run_wrap_test check_lines 4 $DTGREP -S -n /chosen $dtb
+ run_wrap_test check_lines 5 $DTGREP -S -n /holiday $dtb
+ run_wrap_test check_lines 9 $DTGREP -S -n /chosen -n /holiday $dtb
+
+ # Test -N which should list everything except matching nodes
+ run_wrap_test check_lines $node_lines $DTGREP -S -N // $dtb
+ run_wrap_test check_lines $node_lines $DTGREP -S -N chosen $dtb
+ run_wrap_test check_lines $(($node_lines - 4)) $DTGREP -S -N /chosen $dtb
+ run_wrap_test check_lines $(($node_lines - 5)) $DTGREP -S -N /holiday $dtb
+ run_wrap_test check_lines $(($node_lines - 9)) $DTGREP -S -N /chosen \
+ -N /holiday $dtb
+
+ # Using -n and -N together is undefined, so we don't have tests for that
+ # The same applies for -p/-P and -c/-C.
+ run_wrap_error_test $DTGREP -n chosen -N holiday $dtb
+ run_wrap_error_test $DTGREP -c chosen -C holiday $dtb
+ run_wrap_error_test $DTGREP -p chosen -P holiday $dtb
+
+ # Test -o: this should output just the .dts file to a file
+ # Where there is non-dts output it should go to stdout
+ rm -f $tmp
+ run_wrap_test check_lines 0 $DTGREP $dtb -o $tmp
+ run_wrap_test check_lines $node_lines cat $tmp
+
+ # Here we expect a region list with a single entry, plus a header line
+ # on stdout
+ run_wrap_test check_lines 2 $DTGREP $dtb -o $tmp -l
+ run_wrap_test check_lines $node_lines cat $tmp
+
+ # Here we expect a list of strings on stdout
+ run_wrap_test check_lines ${lines} $DTGREP $dtb -o $tmp -L
+ run_wrap_test check_lines $node_lines cat $tmp
+
+ # Test -p: with -S we only get the compatible lines themselves
+ run_wrap_test check_lines 2 $DTGREP -S -p compatible -n // $dtb
+ run_wrap_test check_lines 1 $DTGREP -S -p bootargs -n // $dtb
+
+ # Without -S we also get the node containing these properties
+ run_wrap_test check_lines 6 $DTGREP -p compatible -n // $dtb
+ run_wrap_test check_lines 5 $DTGREP -p bootargs -n // $dtb
+
+ # Now similar tests for -P
+ # First get the number of property lines (containing '=')
+ lines=$(grep "=" $dts |wc -l)
+ run_wrap_test check_lines $(($lines - 2)) $DTGREP -S -P compatible \
+ -n // $dtb
+ run_wrap_test check_lines $(($lines - 1)) $DTGREP -S -P bootargs \
+ -n // $dtb
+ run_wrap_test check_lines $(($lines - 3)) $DTGREP -S -P compatible \
+ -P bootargs -n // $dtb
+
+ # Without -S we also get the node containing these properties
+ run_wrap_test check_lines $(($node_lines - 2)) $DTGREP -P compatible \
+ -n // $dtb
+ run_wrap_test check_lines $(($node_lines - 1)) $DTGREP -P bootargs \
+ -n // $dtb
+ run_wrap_test check_lines $(($node_lines - 3)) $DTGREP -P compatible \
+ -P bootargs -n // $dtb
+
+ # -s should bring in all sub-nodes
+ run_wrap_test check_lines 2 $DTGREP -p none -n / $dtb
+ run_wrap_test check_lines 6 $DTGREP -s -p none -n / $dtb
+ run_wrap_test check_lines 2 $DTGREP -S -p none -n /holiday $dtb
+ run_wrap_test check_lines 4 $DTGREP -p none -n /holiday $dtb
+ run_wrap_test check_lines 8 $DTGREP -s -p none -n /holiday $dtb
+
+ # -v inverts the polarity of any condition
+ run_wrap_test check_lines $(($node_lines - 2)) $DTGREP -Sv -p none \
+ -n / $dtb
+ run_wrap_test check_lines $(($node_lines - 2)) $DTGREP -Sv -p compatible \
+ -n // $dtb
+ run_wrap_test check_lines $(($node_lines - 2)) $DTGREP -Sv -g /chosen \
+ $dtb
+ run_wrap_test check_lines $node_lines $DTGREP -Sv -n // $dtb
+ run_wrap_test check_lines $node_lines $DTGREP -Sv -n chosen $dtb
+ run_wrap_error_test $DTGREP -v -N holiday $dtb
+
+ # Check that the -V flag works
+ run_wrap_test check_contains 1 dts-v1 $DTGREP -V $dtb
+ run_wrap_test check_contains 0 dts-v1 $DTGREP $dtb
+
+ # Now some dtb tests. The dts tests above have tested the basic grepping
+ # features so we only need to concern ourselves with things that are
+ # different about dtb/bin output.
+
+ # An empty node list should just give us the FDT_END tag
+ run_wrap_test check_bytes 4 $DTGREP -n // -O bin $dtb
+
+ # The mem_rsvmap is two entries of 16 bytes each
+ run_wrap_test check_bytes $((4 + 32)) $DTGREP -m -n // -O bin $dtb
+
+ # Check we can add the string table
+ string_size=$($DTGREP -H $dtb | awk '/size_dt_strings:/ {print $3}')
+ run_wrap_test check_bytes $((4 + $string_size)) $DTGREP -t -n // -O bin \
+ $dtb
+ run_wrap_test check_bytes $((4 + 32 + $string_size)) $DTGREP -tm \
+ -n // -O bin $dtb
+
+ # Check that a pass-through works ok. fdtgrep aligns the mem_rsvmap table
+ # to a 16-bytes boundary, but dtc uses 8 bytes so we expect the size to
+ # increase by 8 bytes...
+ run_dtc_test -O dtb -o $dtb $dts
+ base=$(stat -c %s $dtb)
+ run_wrap_test check_bytes $(($base + 8)) $DTGREP -O dtb $dtb
+
+ # ...but we should get the same output from fdtgrep in a second pass
+ run_wrap_test check_bytes 0 $DTGREP -O dtb $dtb -o $tmp
+ base=$(stat -c %s $tmp)
+ run_wrap_test check_bytes $base $DTGREP -O dtb $tmp
+
+ rm -f $tmp
+}
+
while getopts "vt:m" ARG ; do
case $ARG in
"v")
@@ -599,7 +942,7 @@ while getopts "vt:m" ARG ; do
done
if [ -z "$TESTSETS" ]; then
- TESTSETS="libfdt utilfdt dtc dtbs_equal fdtget fdtput"
+ TESTSETS="libfdt utilfdt dtc dtbs_equal fdtget fdtput fdtgrep"
fi
# Make sure we don't have stale blobs lying around
@@ -625,6 +968,9 @@ for set in $TESTSETS; do
"fdtput")
fdtput_tests
;;
+ "fdtgrep")
+ fdtgrep_tests
+ ;;
esac
done
diff --git a/tests/tests.sh b/tests/tests.sh
index 31530d5..8e4b65b 100644
--- a/tests/tests.sh
+++ b/tests/tests.sh
@@ -21,6 +21,7 @@ FAIL_IF_SIGNAL () {
DTC=../dtc
DTGET=../fdtget
DTPUT=../fdtput
+DTGREP=../fdtgrep
verbose_run () {
if [ -z "$QUIET_TEST" ]; then
--
1.7.7.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 7/8] Remove fdtdump and use fdtgrep instead
[not found] ` <1358801962-21707-1-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
` (5 preceding siblings ...)
2013-01-21 20:59 ` [PATCH 6/8] Add fdtgrep to grep and subset FDTs Simon Glass
@ 2013-01-21 20:59 ` Simon Glass
[not found] ` <1358801962-21707-8-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-01-21 20:59 ` [PATCH 8/8] RFC: Check offset in fdt_string() Simon Glass
7 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2013-01-21 20:59 UTC (permalink / raw)
To: Jon Loeliger; +Cc: Devicetree Discuss
Since fdtgrep does everything that fdtdump does now, perhaps we should
replace it with a symlink.
Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Makefile | 5 +-
fdtdump.c | 137 -------------------------------------------------------------
fdtgrep.c | 40 ++++++++++++------
3 files changed, 30 insertions(+), 152 deletions(-)
delete mode 100644 fdtdump.c
diff --git a/Makefile b/Makefile
index 96e0488..339b83c 100644
--- a/Makefile
+++ b/Makefile
@@ -122,7 +122,6 @@ all: $(BIN) libfdt
ifneq ($(DEPTARGETS),)
-include $(DTC_OBJS:%.o=%.d)
-include $(CONVERT_OBJS:%.o=%.d)
--include $(FDTDUMP_OBJS:%.o=%.d)
-include $(FDTGET_OBJS:%.o=%.d)
-include $(FDTPUT_OBJS:%.o=%.d)
-include $(FDTGREP_OBJS:%.o=%.d)
@@ -184,7 +183,9 @@ convert-dtsv0: $(CONVERT_OBJS)
@$(VECHO) LD $@
$(LINK.c) -o $@ $^
-fdtdump: $(FDTDUMP_OBJS)
+fdtdump: fdtgrep
+ rm -f $@
+ ln -s $< $@
fdtget: $(FDTGET_OBJS) $(LIBFDT_archive)
diff --git a/fdtdump.c b/fdtdump.c
deleted file mode 100644
index 03ea429..0000000
--- a/fdtdump.c
+++ /dev/null
@@ -1,137 +0,0 @@
-/*
- * fdtdump.c - Contributed by Pantelis Antoniou <pantelis.antoniou AT gmail.com>
- */
-
-#include <stdint.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <ctype.h>
-
-#include <libfdt_env.h>
-#include <fdt.h>
-
-#include "util.h"
-
-#define ALIGN(x, a) (((x) + ((a) - 1)) & ~((a) - 1))
-#define PALIGN(p, a) ((void *)(ALIGN((unsigned long)(p), (a))))
-#define GET_CELL(p) (p += 4, *((const uint32_t *)(p-4)))
-
-static void dump_blob(void *blob)
-{
- struct fdt_header *bph = blob;
- uint32_t off_mem_rsvmap = fdt32_to_cpu(bph->off_mem_rsvmap);
- uint32_t off_dt = fdt32_to_cpu(bph->off_dt_struct);
- uint32_t off_str = fdt32_to_cpu(bph->off_dt_strings);
- struct fdt_reserve_entry *p_rsvmap =
- (struct fdt_reserve_entry *)((char *)blob + off_mem_rsvmap);
- const char *p_struct = (const char *)blob + off_dt;
- const char *p_strings = (const char *)blob + off_str;
- uint32_t version = fdt32_to_cpu(bph->version);
- uint32_t totalsize = fdt32_to_cpu(bph->totalsize);
- uint32_t tag;
- const char *p, *s, *t;
- int depth, sz, shift;
- int i;
- uint64_t addr, size;
-
- depth = 0;
- shift = 4;
-
- printf("/dts-v1/;\n");
- printf("// magic:\t\t0x%x\n", fdt32_to_cpu(bph->magic));
- printf("// totalsize:\t\t0x%x (%d)\n", totalsize, totalsize);
- printf("// off_dt_struct:\t0x%x\n", off_dt);
- printf("// off_dt_strings:\t0x%x\n", off_str);
- printf("// off_mem_rsvmap:\t0x%x\n", off_mem_rsvmap);
- printf("// version:\t\t%d\n", version);
- printf("// last_comp_version:\t%d\n",
- fdt32_to_cpu(bph->last_comp_version));
- if (version >= 2)
- printf("// boot_cpuid_phys:\t0x%x\n",
- fdt32_to_cpu(bph->boot_cpuid_phys));
-
- if (version >= 3)
- printf("// size_dt_strings:\t0x%x\n",
- fdt32_to_cpu(bph->size_dt_strings));
- if (version >= 17)
- printf("// size_dt_struct:\t0x%x\n",
- fdt32_to_cpu(bph->size_dt_struct));
- printf("\n");
-
- for (i = 0; ; i++) {
- addr = fdt64_to_cpu(p_rsvmap[i].address);
- size = fdt64_to_cpu(p_rsvmap[i].size);
- if (addr == 0 && size == 0)
- break;
-
- printf("/memreserve/ %llx %llx;\n",
- (unsigned long long)addr, (unsigned long long)size);
- }
-
- p = p_struct;
- while ((tag = fdt32_to_cpu(GET_CELL(p))) != FDT_END) {
-
- /* printf("tag: 0x%08x (%d)\n", tag, p - p_struct); */
-
- if (tag == FDT_BEGIN_NODE) {
- s = p;
- p = PALIGN(p + strlen(s) + 1, 4);
-
- if (*s == '\0')
- s = "/";
-
- printf("%*s%s {\n", depth * shift, "", s);
-
- depth++;
- continue;
- }
-
- if (tag == FDT_END_NODE) {
- depth--;
-
- printf("%*s};\n", depth * shift, "");
- continue;
- }
-
- if (tag == FDT_NOP) {
- printf("%*s// [NOP]\n", depth * shift, "");
- continue;
- }
-
- if (tag != FDT_PROP) {
- fprintf(stderr, "%*s ** Unknown tag 0x%08x\n", depth * shift, "", tag);
- break;
- }
- sz = fdt32_to_cpu(GET_CELL(p));
- s = p_strings + fdt32_to_cpu(GET_CELL(p));
- if (version < 16 && sz >= 8)
- p = PALIGN(p, 8);
- t = p;
-
- p = PALIGN(p + sz, 4);
-
- printf("%*s%s", depth * shift, "", s);
- utilfdt_print_data(t, sz);
- printf(";\n");
- }
-}
-
-
-int main(int argc, char *argv[])
-{
- char *buf;
-
- if (argc < 2) {
- fprintf(stderr, "supply input filename\n");
- return 5;
- }
-
- buf = utilfdt_read(argv[1]);
- if (buf)
- dump_blob(buf);
- else
- return 10;
-
- return 0;
-}
diff --git a/fdtgrep.c b/fdtgrep.c
index bb08f12..3169e29 100644
--- a/fdtgrep.c
+++ b/fdtgrep.c
@@ -726,28 +726,42 @@ int main(int argc, char *argv[])
{
char *filename = NULL;
struct display_info disp;
+ const char *name;
int ret;
/* set defaults */
memset(&disp, '\0', sizeof(disp));
disp.flags = FDT_REG_SUPERNODES; /* Default flags */
- scan_args(&disp, argc, argv);
+ /* For fdtdump, use default args */
+ name = strrchr(argv[0], '/');
+ if (!strcmp(name ? name + 1 : argv[0], "fdtdump")) {
+ disp.show_dts_version = 1;
+ disp.header = 1;
+ disp.flags |= FDT_REG_ADD_MEM_RSVMAP;
+ if (argc < 2) {
+ fprintf(stderr, "supply input filename\n");
+ return 5;
+ }
+ filename = argv[1];
+ } else {
+ scan_args(&disp, argc, argv);
- /* Show matched lines in colour if we can */
- disp.colour = disp.all && isatty(0);
+ /* Show matched lines in colour if we can */
+ disp.colour = disp.all && isatty(0);
- /* Any additional arguments can match anything, just like -g */
- while (optind < argc - 1) {
- if (value_add(&disp, &disp.value_head, FDT_IS_ANY, 1,
- argv[optind++]))
- usage("Cannot add value");
- }
+ /* Any additional arguments can match anything, just like -g */
+ while (optind < argc - 1) {
+ if (value_add(&disp, &disp.value_head, FDT_IS_ANY, 1,
+ argv[optind++]))
+ usage("Cannot add value");
+ }
- if (optind < argc)
- filename = argv[optind++];
- if (!filename)
- usage("Missing filename");
+ if (optind < argc)
+ filename = argv[optind++];
+ if (!filename)
+ usage("Missing filename");
+ }
/* If a valid .dtb is required, set flags to ensure we get one */
if (disp.output == OUT_DTB) {
--
1.7.7.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 8/8] RFC: Check offset in fdt_string()
[not found] ` <1358801962-21707-1-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
` (6 preceding siblings ...)
2013-01-21 20:59 ` [PATCH 7/8] Remove fdtdump and use fdtgrep instead Simon Glass
@ 2013-01-21 20:59 ` Simon Glass
7 siblings, 0 replies; 21+ messages in thread
From: Simon Glass @ 2013-01-21 20:59 UTC (permalink / raw)
To: Jon Loeliger; +Cc: Devicetree Discuss
(We probably don't want this patch, and certainly can't apply it as is,
but I send it in order to find out the intent of fdt_string()).
At present fdt_string() says that returns:
- a pointer to the string, on success
- NULL, if stroffset is out of bounds
However it does not in fact return NULL. Changing it to do so also
breaks 15 tests (segfault).
What is the intended behaviour of this function, please?
Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
libfdt/fdt_ro.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
index 50007f6..cba8772 100644
--- a/libfdt/fdt_ro.c
+++ b/libfdt/fdt_ro.c
@@ -77,6 +77,8 @@ static int _fdt_nodename_eq(const void *fdt, int offset,
const char *fdt_string(const void *fdt, int stroffset)
{
+ if (stroffset < 0 || stroffset >= fdt_size_dt_strings(fdt))
+ return NULL;
return (const char *)fdt + fdt_off_dt_strings(fdt) + stroffset;
}
--
1.7.7.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/8] Adjust util_is_printable_string() comment and fix test
[not found] ` <1358801962-21707-2-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2013-01-22 5:33 ` David Gibson
[not found] ` <20130122053340.GJ23500-W9XWwYn+TF0XU02nzanrWNbf9cGiqdzd@public.gmane.org>
0 siblings, 1 reply; 21+ messages in thread
From: David Gibson @ 2013-01-22 5:33 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Discuss
[-- Attachment #1.1: Type: text/plain, Size: 1530 bytes --]
On Mon, Jan 21, 2013 at 12:59:15PM -0800, Simon Glass wrote:
> This commit which changed the behaviour of this function broke one
> of the tests. Also the comment should be updated to reflect its new
> behaviour.
>
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
Jon, please apply asap, regardless of what we do with the rest of this
series, since it fixes up the ugly test failure we have right now.
> ---
> tests/run_tests.sh | 4 +---
> util.h | 8 +++++---
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index dd7f217..e04512c 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -498,9 +498,7 @@ fdtget_tests () {
>
> # run_fdtget_test <expected-result> [<flags>] <file> <node> <property>
> run_fdtget_test "MyBoardName" $dtb / model
> - run_fdtget_test "77 121 66 111 \
> -97 114 100 78 97 109 101 0 77 121 66 111 97 114 100 70 97 109 105 \
> -108 121 78 97 109 101 0" $dtb / compatible
> + run_fdtget_test "MyBoardName MyBoardFamilyName" $dtb / compatible
I would also like to see a test of the old-style behaviour still
present, by explicitly enabling it with -t bu.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 192 bytes --]
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/8] Move property-printing into util
[not found] ` <1358801962-21707-3-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2013-01-22 5:35 ` David Gibson
2013-01-27 20:30 ` Jon Loeliger
1 sibling, 0 replies; 21+ messages in thread
From: David Gibson @ 2013-01-22 5:35 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Discuss
[-- Attachment #1.1: Type: text/plain, Size: 546 bytes --]
On Mon, Jan 21, 2013 at 12:59:16PM -0800, Simon Glass wrote:
> The function that prints a property can be useful to other programs,
> so move it into util.
>
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Straightforward enough.
Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 192 bytes --]
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/8] .gitignore: Add rule for *.patch
[not found] ` <1358801962-21707-4-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2013-01-22 5:35 ` David Gibson
2013-01-27 20:30 ` Jon Loeliger
1 sibling, 0 replies; 21+ messages in thread
From: David Gibson @ 2013-01-22 5:35 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Discuss
[-- Attachment #1.1: Type: text/plain, Size: 969 bytes --]
On Mon, Jan 21, 2013 at 12:59:17PM -0800, Simon Glass wrote:
> Ignore any patch files that we find, since these are likely to be
> used when sending patches upstream.
>
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Acked-by: David Gibson >david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
Not really related to this series at all, but I think this is a
reasonable addition. Jon, please apply, regardless of the rest of the
series.
> ---
> .gitignore | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 7cabc49..545b899 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -1,6 +1,7 @@
> *.o
> *.d
> *.a
> +*.patch
> *.so
> *~
> *.tab.[ch]
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 192 bytes --]
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/8] Export fdt_stringlist_contains()
[not found] ` <1358801962-21707-5-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2013-01-23 5:07 ` David Gibson
2013-01-27 20:31 ` Jon Loeliger
1 sibling, 0 replies; 21+ messages in thread
From: David Gibson @ 2013-01-23 5:07 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Discuss
[-- Attachment #1.1: Type: text/plain, Size: 621 bytes --]
On Mon, Jan 21, 2013 at 12:59:18PM -0800, Simon Glass wrote:
> This function is useful outside libfdt, so export it.
>
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
I didn't export this initially since the compatible test was the only
user I knew of. But exporting it seems like a generally good idea.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 192 bytes --]
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/8] Adjust util_is_printable_string() comment and fix test
[not found] ` <20130122053340.GJ23500-W9XWwYn+TF0XU02nzanrWNbf9cGiqdzd@public.gmane.org>
@ 2013-01-27 19:14 ` Simon Glass
0 siblings, 0 replies; 21+ messages in thread
From: Simon Glass @ 2013-01-27 19:14 UTC (permalink / raw)
To: David Gibson; +Cc: Devicetree Discuss
Hi David,
On Tue, Jan 22, 2013 at 6:33 PM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Mon, Jan 21, 2013 at 12:59:15PM -0800, Simon Glass wrote:
>> This commit which changed the behaviour of this function broke one
>> of the tests. Also the comment should be updated to reflect its new
>> behaviour.
>>
>> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>
> Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
>
> Jon, please apply asap, regardless of what we do with the rest of this
> series, since it fixes up the ugly test failure we have right now.
>
>> ---
>> tests/run_tests.sh | 4 +---
>> util.h | 8 +++++---
>> 2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
>> index dd7f217..e04512c 100755
>> --- a/tests/run_tests.sh
>> +++ b/tests/run_tests.sh
>> @@ -498,9 +498,7 @@ fdtget_tests () {
>>
>> # run_fdtget_test <expected-result> [<flags>] <file> <node> <property>
>> run_fdtget_test "MyBoardName" $dtb / model
>> - run_fdtget_test "77 121 66 111 \
>> -97 114 100 78 97 109 101 0 77 121 66 111 97 114 100 70 97 109 105 \
>> -108 121 78 97 109 101 0" $dtb / compatible
>> + run_fdtget_test "MyBoardName MyBoardFamilyName" $dtb / compatible
>
> I would also like to see a test of the old-style behaviour still
> present, by explicitly enabling it with -t bu.
OK - I have sent a v2 of this patch.
Regards,
Simon
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/8] Move property-printing into util
[not found] ` <1358801962-21707-3-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-01-22 5:35 ` David Gibson
@ 2013-01-27 20:30 ` Jon Loeliger
1 sibling, 0 replies; 21+ messages in thread
From: Jon Loeliger @ 2013-01-27 20:30 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Discuss
> The function that prints a property can be useful to other programs,
> so move it into util.
>
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Applied.
Thanks,
jdl
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/8] .gitignore: Add rule for *.patch
[not found] ` <1358801962-21707-4-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-01-22 5:35 ` David Gibson
@ 2013-01-27 20:30 ` Jon Loeliger
1 sibling, 0 replies; 21+ messages in thread
From: Jon Loeliger @ 2013-01-27 20:30 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Discuss
> Ignore any patch files that we find, since these are likely to be
> used when sending patches upstream.
>
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Applied.
Thanks,
jdl
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/8] Export fdt_stringlist_contains()
[not found] ` <1358801962-21707-5-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-01-23 5:07 ` David Gibson
@ 2013-01-27 20:31 ` Jon Loeliger
1 sibling, 0 replies; 21+ messages in thread
From: Jon Loeliger @ 2013-01-27 20:31 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Discuss
> This function is useful outside libfdt, so export it.
>
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Applied.
Thanks,
jdl
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 7/8] Remove fdtdump and use fdtgrep instead
[not found] ` <1358801962-21707-8-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2013-02-06 7:16 ` David Gibson
[not found] ` <20130206071606.GR821-W9XWwYn+TF0XU02nzanrWNbf9cGiqdzd@public.gmane.org>
0 siblings, 1 reply; 21+ messages in thread
From: David Gibson @ 2013-02-06 7:16 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Discuss
[-- Attachment #1.1: Type: text/plain, Size: 639 bytes --]
On Mon, Jan 21, 2013 at 12:59:21PM -0800, Simon Glass wrote:
> Since fdtgrep does everything that fdtdump does now, perhaps we should
> replace it with a symlink.
Nack. The point of fdtdump is not simply to be a tool for dumping
device trees - dtc -I dtb -O dts will do that already. The point is
to be a really simple, independent implementation of dumping the tree,
which can be used for analyzing bugs in the more complex tools.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 192 bytes --]
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 7/8] Remove fdtdump and use fdtgrep instead
[not found] ` <20130206071606.GR821-W9XWwYn+TF0XU02nzanrWNbf9cGiqdzd@public.gmane.org>
@ 2013-02-06 16:40 ` Simon Glass
0 siblings, 0 replies; 21+ messages in thread
From: Simon Glass @ 2013-02-06 16:40 UTC (permalink / raw)
To: David Gibson; +Cc: Devicetree Discuss
Hi David,
On Tue, Feb 5, 2013 at 11:16 PM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Mon, Jan 21, 2013 at 12:59:21PM -0800, Simon Glass wrote:
>> Since fdtgrep does everything that fdtdump does now, perhaps we should
>> replace it with a symlink.
>
> Nack. The point of fdtdump is not simply to be a tool for dumping
> device trees - dtc -I dtb -O dts will do that already. The point is
> to be a really simple, independent implementation of dumping the tree,
> which can be used for analyzing bugs in the more complex tools.
Sounds reasonable - I wasn't aware of that. I figured that if I didn't
send a patch to implement dump with grep then someone would ask for
it... But I agree it is nice to keep the simple code and I have made
the output function common anyway.
Regards,
Simon
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/8] libfdt: Add function to find regions in an FDT
[not found] ` <1358801962-21707-6-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2013-02-11 2:47 ` David Gibson
[not found] ` <20130211024736.GG4948-W9XWwYn+TF0XU02nzanrWNbf9cGiqdzd@public.gmane.org>
0 siblings, 1 reply; 21+ messages in thread
From: David Gibson @ 2013-02-11 2:47 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Discuss
[-- Attachment #1.1: Type: text/plain, Size: 19944 bytes --]
On Mon, Jan 21, 2013 at 12:59:19PM -0800, Simon Glass wrote:
> Given a set of nodes and properties, find the regions of the device tree
> which describe those parts.
>
> A test is provided which builds a tree while tracking where the regions
> should be, then calls fdt_find_regions() to make sure that it agrees.
>
> Further tests will come as part of fdtgrep.
Ok, for starters I don't want to include this before the next dtc
release. It's a pretty substantial addition.
Second, fdt_wip is definitely the wrong place for this. fdt_wip.c is
for the Write-In-Place code (i.e. functions which alter the tree
contents, but don't need to move any existing data around). AFAICT
this function only reads the tree, which would put it in fdt_ro.c.
However, it's such a big addition - and adds several new state
structures, I think it should go in an entirely new source file for
it. That way people who don't use this piece of the library won't get
it included, even if they don't have a compiler which does function
sections.
Third, I'm having a bit of trouble wrapping my head around exactly
what the search function does, and how the various flags and pieces
interact. A block comment with a general overview would be nice.
More specific comments below:
>
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
> libfdt/fdt_wip.c | 311 ++++++++++++++++++++++++++++++++++++++++++++++++
> libfdt/libfdt.h | 142 ++++++++++++++++++++++
> tests/.gitignore | 1 +
> tests/Makefile.tests | 3 +-
> tests/region_tree.c | 324 ++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/run_tests.sh | 5 +
> 6 files changed, 785 insertions(+), 1 deletions(-)
> create mode 100644 tests/region_tree.c
>
> diff --git a/libfdt/fdt_wip.c b/libfdt/fdt_wip.c
> index c5bbb68..ff0f940 100644
> --- a/libfdt/fdt_wip.c
> +++ b/libfdt/fdt_wip.c
> @@ -116,3 +116,314 @@ int fdt_nop_node(void *fdt, int nodeoffset)
> endoffset - nodeoffset);
> return 0;
> }
> +
> +/* Maximum depth that we can grep */
> +#define FDT_MAX_DEPTH 32
> +
> +/* Decribes what we want to include from the current tag */
> +enum want_t {
> + WANT_NOTHING,
> + WANT_NODES_ONLY,
> + WANT_NODES_AND_PROPS,
> +};
> +
> +/* 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 */
What's the distinction between 'included' and 'want' and how do they
interact?
> +};
> +
> +/* The state of our finding algortihm */
> +struct find_reg {
> + 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 */
> +};
> +
> +/**
> + * fdt_add_region() - Add a new region to our list
> + *
> + * The region is added if there is space, but in any case we increment the
> + * count. If permitted, and the new region overlaps the last one, we merge
> + * them.
> + *
> + * @info: State information
> + * @offset: Start offset of region
> + * @size: Size of region
> + */
> +static void fdt_add_region(struct find_reg *info, int offset, int size)
> +{
> + struct fdt_region *reg = &info->region[info->count - 1];
> +
> + if (info->can_merge && info->count &&
> + info->count < info->max_regions &&
Shouldn't this be <= for the merge case?
> + offset <= reg->offset + reg->size) {
> + reg->size = offset + size - reg->offset;
> + } else if (info->count < info->max_regions) {
> + reg++;
> + reg->offset = offset;
> + reg->size = size;
> + info->count++;
> + }
> +}
> +
> +/**
> + * fdt_include_supernodes() - Include supernodes required by this node
> + *
> + * When we decided to include a node or property which is not at the top
> + * level, this function forces the inclusion of higher level nodes. For
> + * example, given this tree:
> + *
> + * / {
> + * testing {
> + * }
> + * }
> + *
> + * If we decide to include testing then we need the root node to have a valid
> + * tree. This function adds those regions.
> + *
> + * @info: State information
> + * @depth: Current stack depth
> + */
> +static void fdt_include_supernodes(struct find_reg *info, int depth)
> +{
> + int base = fdt_off_dt_struct(info->fdt);
> + int start, stop_at;
> + int i;
> +
> + /*
> + * Work down the stack looking for supernodes that we didn't include.
> + * The algortihm here is actually pretty simple, since we know that
> + * no previous subnode had to include these nodes, or if it did, we
> + * marked them as included (on the stack) already.
> + */
> + for (i = 0; i <= depth; i++) {
> + if (!info->stack[i].included) {
> + start = info->stack[i].offset;
> +
> + /* Add the FDT_BEGIN_NODE tag of this supernode */
> + fdt_next_tag(info->fdt, start, &stop_at);
> + fdt_add_region(info, base + start, stop_at - start);
> +
> + /* Remember that this supernode is now included */
> + info->stack[i].included = 1;
> + info->can_merge = 1;
> + }
> +
> + /* Force (later) generation of the FDT_END_NODE tag */
> + if (!info->stack[i].want)
> + info->stack[i].want = WANT_NODES_ONLY;
> + }
> +}
> +
> +int fdt_find_regions(const void *fdt,
> + int (*h_include)(void *priv, int type, const char *data,
> + int size),
> + void *priv,
> + struct fdt_region region[], int max_regions,
> + char *path, int path_len, int flags)
So, in general I've tried to avoid having the libfdt interfaces
require the caller to buffer significant amounts of information. So,
if it's possible (and I don't immediately see a reason for it not to
be) I'd prefer to see first_region()/next_region() type functions,
rather than requiring the callers to gather the whole region list at
once.
> +{
> + int base = fdt_off_dt_struct(fdt);
> + struct find_reg info;
> + int nextoffset;
> + int start = -1;
> + int depth = -1;
> + const char *str;
> + enum want_t want = WANT_NOTHING;
> + uint32_t tag;
> + char *end;
> +
> + /* Set up our state */
> + info.fdt = fdt;
> + info.can_merge = 1;
> + info.count = 0;
> + info.region = region;
> + info.max_regions = max_regions;
> +
> + /* Add the memory reserve map into its own region */
> + if (flags & FDT_REG_ADD_MEM_RSVMAP) {
> + fdt_add_region(&info, fdt_off_mem_rsvmap(fdt),
> + fdt_off_dt_struct(fdt) - fdt_off_mem_rsvmap(fdt));
> + info.can_merge = 0; /* Don't allow merging with this */
> + }
> +
> + end = path;
> + *end = '\0';
> + nextoffset = 0;
> +
> + /*
> + * Work through the tags one by one, deciding whether each needs to
> + * be included or not. We set the variable 'include' to indicate our
> + * decision. 'want' is used to track what we want to include - it
> + * allows us to pick up all the properties (and/or subnode tags) of
> + * a node.
> + */
> + do {
> + const struct fdt_property *prop;
> + const char *name;
> + int include = 0;
> + int stop_at = 0;
> + int offset;
> + int val;
> + int len;
> +
> + /*
> + * Find the tag, and the offset of the next one. If we need to
> + * stop including tags, then by default we stop *after*
> + * including the current tag
> + */
> + offset = nextoffset;
> + tag = fdt_next_tag(fdt, offset, &nextoffset);
> + stop_at = nextoffset;
> +
> + switch (tag) {
> + case FDT_PROP:
> + stop_at = offset;
> + prop = fdt_get_property_by_offset(fdt, offset, NULL);
> + str = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
> + val = h_include(priv, FDT_IS_PROP, str,
> + strlen(str) + 1);
> + if (val == -1) {
> + include = want == WANT_NODES_AND_PROPS;
> + } else {
> + include = val;
> + /*
> + * Make sure we include the } for this block.
> + * It might be more correct to put this done
> + * by the call to fdt_include_supernodes() in
> + * the case where it adds the node we are
> + * currently in, but this is fairly
> + * equivalent.
> + */
> + if ((flags & FDT_REG_SUPERNODES) && val &&
> + !want)
> + want = WANT_NODES_ONLY;
> + }
> +
> + /* Value grepping is not yet supported */
> + break;
> +
> + case FDT_NOP:
> + include = want == WANT_NODES_AND_PROPS;
> + stop_at = offset;
> + break;
> +
> + case FDT_BEGIN_NODE:
> + depth++;
> + if (depth == FDT_MAX_DEPTH)
> + return -FDT_ERR_BADSTRUCTURE;
Hrm. Perhaps we should add a new error code for "too deep". After
all the structure of the tree is perfectly fine here, it's just this
search function can't cope with its depth.
> + name = fdt_get_name(fdt, offset, &len);
> + if (end - path + 2 + len >= path_len)
> + return -FDT_ERR_NOSPACE;
> +
> + /* Build the full path of this node */
> + if (end != path + 1)
> + *end++ = '/';
> + strcpy(end, name);
> + end += len;
> + info.stack[depth].want = want;
> + info.stack[depth].offset = offset;
> +
> + /*
> + * If we are not intending to include this node, make
> + * sure we stop *before* its tag
> + */
> + if (want == WANT_NODES_ONLY ||
> + !(flags & FDT_REG_DIRECT_SUBNODES)) {
> + stop_at = offset;
> + want = WANT_NOTHING;
> + }
> + val = h_include(priv, FDT_IS_NODE, path,
> + end - path + 1);
> +
> + /*
> + * If no information about this, try the compatible
> + * property. Here we are looking ahead in the tree.
> + */
> + if (val == -1) {
> + str = fdt_getprop(fdt, offset,
> + "compatible", &len);
> + val = h_include(priv, FDT_IS_COMPAT, str,
> + len);
> + }
> +
> + /* Include this if requested */
> + if (val)
> + want = WANT_NODES_AND_PROPS;
> +
> + /* If not requested, decay our 'want' value */
> + else if (want)
> + want--;
> +
> + /* Not including this tag, so stop now */
> + else
> + stop_at = offset;
> +
> + /*
> + * Decide whether to include this tag, and update our
> + * stack with the state for this node
> + */
> + include = want;
> + info.stack[depth].included = include;
> + break;
> +
> + case FDT_END_NODE:
> + include = want;
> + if (depth < 0)
> + return -FDT_ERR_BADSTRUCTURE;
> +
> + /*
> + * If we don't want this node, stop right away, unless
> + * we are including subnodes
> + */
> + if (!want && !(flags & FDT_REG_DIRECT_SUBNODES))
> + stop_at = offset;
> + want = info.stack[depth].want;
> + depth--;
> + while (end > path && *--end != '/')
> + ;
> + *end = '\0';
> + break;
> +
> + case FDT_END:
> + /* We always include the end tag */
> + include = 1;
> + break;
> + }
> +
> + /* If this tag is to be included, mark it as region start */
> + if (include && start == -1) {
> + /* Include any supernodes required by this one */
> + if (flags & FDT_REG_SUPERNODES)
> + fdt_include_supernodes(&info, depth);
> + start = offset;
> + }
> +
> + /*
> + * If this tag is not to be included, finish up the current
> + * region
> + */
> + if (!include && start != -1) {
> + fdt_add_region(&info, base + start, stop_at - start);
> + start = -1;
> + info.can_merge = 1;
> + }
> + } while (tag != FDT_END);
> +
> + if (nextoffset != fdt_size_dt_struct(fdt))
> + return -FDT_ERR_BADLAYOUT;
That should be BADSTRUCTURE, not BADLAYOUT. BADLAYOUT means
structure/strings/memreserve blocks are not in the preferred order
w.r.t. each other.
> +
> + /* Add a region for the END tag and a separate one for string table */
> + fdt_add_region(&info, base + start, nextoffset - start);
> + if (flags & FDT_REG_ADD_STRING_TAB) {
> + info.can_merge = 0;
> + fdt_add_region(&info, fdt_off_dt_strings(fdt),
> + fdt_size_dt_strings(fdt));
> + }
You may want to give BADLAYOUT here, though, if the strings block is
before the structure block.
> +
> + return info.count;
> +}
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index c0075e7..0b3eacf 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -1489,4 +1489,146 @@ int fdt_del_node(void *fdt, int nodeoffset);
>
> const char *fdt_strerror(int errval);
>
> +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 the FDT_BEGIN_NODE tags of subnodes, including their names */
> +#define FDT_REG_DIRECT_SUBNODES (1 << 1)
> +
> +/*
> + * Add all supernodes of a matching node/property, useful for creating a
> + * valid subset tree
> + */
> +#define FDT_REG_SUPERNODES (1 << 2)
> +
> +/* Add a region for the mem_rsvmap table (always the first region) */
> +#define FDT_REG_ADD_MEM_RSVMAP (1 << 3)
> +
> +/* Indicates what an fdt part is (node, property, value, compatible) */
> +#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)
> +
> +#define FDT_IS_ANY 15
> +
> +/**
> + * fdt_find_regions() - 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 even with this subsetting.
> + * 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 - if no information is available
You don't describe what this function does when told "no information
is available". Really the option seems a bit pointless to me, why not
just leave the include by default or exclude by default decision to
the include function.
> + *
> + * For nodes, the function may be called twice, once with FDT_IS_NODE to check
> + * the node name. If this returns -1 then the function is immediately called
> + * again with FDT_IS_COMPAT to check the compatible string. Note that if
> + * there is no compatible string, then data == NULL and size == 0. This means
> + * that the inclusion of a node can be controlled either by a node name or
> + * its compatible string.
This seems awkward to me. Why not supply the fdt pointer and node
offset when calling the inclusion function for a node and have
h_include get whatever properties it needs to make that decision.
> + *
> + * There is no special handling of unit addresses, so the entire name must
> + * be given for nodes. Wildcards are not supported.
Given where? The only names given in the interface are in the tree
itself. Wildcards could be supported by the h_include function AFAICT.
> + * 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()
> + * @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_find_regions(const void *fdt,
> + int (*h_include)(void *priv, int type, const char *data,
> + int size),
> + void *priv,
> + struct fdt_region region[], int max_regions,
> + char *path, int path_len, int flags);
> +
> #endif /* _LIBFDT_H */
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 192 bytes --]
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/8] libfdt: Add function to find regions in an FDT
[not found] ` <20130211024736.GG4948-W9XWwYn+TF0XU02nzanrWNbf9cGiqdzd@public.gmane.org>
@ 2013-02-15 22:47 ` Simon Glass
0 siblings, 0 replies; 21+ messages in thread
From: Simon Glass @ 2013-02-15 22:47 UTC (permalink / raw)
To: David Gibson; +Cc: Devicetree Discuss
Hi David,
On Sun, Feb 10, 2013 at 6:47 PM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Mon, Jan 21, 2013 at 12:59:19PM -0800, Simon Glass wrote:
>> Given a set of nodes and properties, find the regions of the device tree
>> which describe those parts.
>>
>> A test is provided which builds a tree while tracking where the regions
>> should be, then calls fdt_find_regions() to make sure that it agrees.
>>
>> Further tests will come as part of fdtgrep.
>
> Ok, for starters I don't want to include this before the next dtc
> release. It's a pretty substantial addition.
>
> Second, fdt_wip is definitely the wrong place for this. fdt_wip.c is
> for the Write-In-Place code (i.e. functions which alter the tree
> contents, but don't need to move any existing data around). AFAICT
> this function only reads the tree, which would put it in fdt_ro.c.
> However, it's such a big addition - and adds several new state
> structures, I think it should go in an entirely new source file for
> it. That way people who don't use this piece of the library won't get
> it included, even if they don't have a compiler which does function
> sections.
OK I misunderstood the name of that file - I will add a new file.
>
> Third, I'm having a bit of trouble wrapping my head around exactly
> what the search function does, and how the various flags and pieces
> interact. A block comment with a general overview would be nice.
>
> More specific comments below:
>
>>
>> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> ---
>> libfdt/fdt_wip.c | 311 ++++++++++++++++++++++++++++++++++++++++++++++++
>> libfdt/libfdt.h | 142 ++++++++++++++++++++++
>> tests/.gitignore | 1 +
>> tests/Makefile.tests | 3 +-
>> tests/region_tree.c | 324 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> tests/run_tests.sh | 5 +
>> 6 files changed, 785 insertions(+), 1 deletions(-)
>> create mode 100644 tests/region_tree.c
>>
>> diff --git a/libfdt/fdt_wip.c b/libfdt/fdt_wip.c
>> index c5bbb68..ff0f940 100644
>> --- a/libfdt/fdt_wip.c
>> +++ b/libfdt/fdt_wip.c
>> @@ -116,3 +116,314 @@ int fdt_nop_node(void *fdt, int nodeoffset)
>> endoffset - nodeoffset);
>> return 0;
>> }
>> +
>> +/* Maximum depth that we can grep */
>> +#define FDT_MAX_DEPTH 32
>> +
>> +/* Decribes what we want to include from the current tag */
>> +enum want_t {
>> + WANT_NOTHING,
>> + WANT_NODES_ONLY,
>> + WANT_NODES_AND_PROPS,
>> +};
>> +
>> +/* 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 */
>
> What's the distinction between 'included' and 'want' and how do they
> interact?
I will add a big comment for that.
>
>> +};
>> +
>> +/* The state of our finding algortihm */
>> +struct find_reg {
>> + 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 */
>> +};
>> +
>> +/**
>> + * fdt_add_region() - Add a new region to our list
>> + *
>> + * The region is added if there is space, but in any case we increment the
>> + * count. If permitted, and the new region overlaps the last one, we merge
>> + * them.
>> + *
>> + * @info: State information
>> + * @offset: Start offset of region
>> + * @size: Size of region
>> + */
>> +static void fdt_add_region(struct find_reg *info, int offset, int size)
>> +{
>> + struct fdt_region *reg = &info->region[info->count - 1];
>> +
>> + if (info->can_merge && info->count &&
>> + info->count < info->max_regions &&
>
> Shouldn't this be <= for the merge case?
Yes, thanks.
>
>> + offset <= reg->offset + reg->size) {
>> + reg->size = offset + size - reg->offset;
>> + } else if (info->count < info->max_regions) {
>> + reg++;
>> + reg->offset = offset;
>> + reg->size = size;
>> + info->count++;
>> + }
>> +}
>> +
>> +/**
>> + * fdt_include_supernodes() - Include supernodes required by this node
>> + *
>> + * When we decided to include a node or property which is not at the top
>> + * level, this function forces the inclusion of higher level nodes. For
>> + * example, given this tree:
>> + *
>> + * / {
>> + * testing {
>> + * }
>> + * }
>> + *
>> + * If we decide to include testing then we need the root node to have a valid
>> + * tree. This function adds those regions.
>> + *
>> + * @info: State information
>> + * @depth: Current stack depth
>> + */
>> +static void fdt_include_supernodes(struct find_reg *info, int depth)
>> +{
>> + int base = fdt_off_dt_struct(info->fdt);
>> + int start, stop_at;
>> + int i;
>> +
>> + /*
>> + * Work down the stack looking for supernodes that we didn't include.
>> + * The algortihm here is actually pretty simple, since we know that
>> + * no previous subnode had to include these nodes, or if it did, we
>> + * marked them as included (on the stack) already.
>> + */
>> + for (i = 0; i <= depth; i++) {
>> + if (!info->stack[i].included) {
>> + start = info->stack[i].offset;
>> +
>> + /* Add the FDT_BEGIN_NODE tag of this supernode */
>> + fdt_next_tag(info->fdt, start, &stop_at);
>> + fdt_add_region(info, base + start, stop_at - start);
>> +
>> + /* Remember that this supernode is now included */
>> + info->stack[i].included = 1;
>> + info->can_merge = 1;
>> + }
>> +
>> + /* Force (later) generation of the FDT_END_NODE tag */
>> + if (!info->stack[i].want)
>> + info->stack[i].want = WANT_NODES_ONLY;
>> + }
>> +}
>> +
>> +int fdt_find_regions(const void *fdt,
>> + int (*h_include)(void *priv, int type, const char *data,
>> + int size),
>> + void *priv,
>> + struct fdt_region region[], int max_regions,
>> + char *path, int path_len, int flags)
>
> So, in general I've tried to avoid having the libfdt interfaces
> require the caller to buffer significant amounts of information. So,
> if it's possible (and I don't immediately see a reason for it not to
> be) I'd prefer to see first_region()/next_region() type functions,
> rather than requiring the callers to gather the whole region list at
> once.
OK I have had a crack at this, although it took several hours and I'm
not entirely pleased with the state that I need to keep around. I will
send a patch and you can see what you think. It can be improved a
little I think, but mainly I would like to avoid adding lots of stuff
to libfdt.h.
>
>> +{
>> + int base = fdt_off_dt_struct(fdt);
>> + struct find_reg info;
>> + int nextoffset;
>> + int start = -1;
>> + int depth = -1;
>> + const char *str;
>> + enum want_t want = WANT_NOTHING;
>> + uint32_t tag;
>> + char *end;
>> +
>> + /* Set up our state */
>> + info.fdt = fdt;
>> + info.can_merge = 1;
>> + info.count = 0;
>> + info.region = region;
>> + info.max_regions = max_regions;
>> +
>> + /* Add the memory reserve map into its own region */
>> + if (flags & FDT_REG_ADD_MEM_RSVMAP) {
>> + fdt_add_region(&info, fdt_off_mem_rsvmap(fdt),
>> + fdt_off_dt_struct(fdt) - fdt_off_mem_rsvmap(fdt));
>> + info.can_merge = 0; /* Don't allow merging with this */
>> + }
>> +
>> + end = path;
>> + *end = '\0';
>> + nextoffset = 0;
>> +
>> + /*
>> + * Work through the tags one by one, deciding whether each needs to
>> + * be included or not. We set the variable 'include' to indicate our
>> + * decision. 'want' is used to track what we want to include - it
>> + * allows us to pick up all the properties (and/or subnode tags) of
>> + * a node.
>> + */
>> + do {
>> + const struct fdt_property *prop;
>> + const char *name;
>> + int include = 0;
>> + int stop_at = 0;
>> + int offset;
>> + int val;
>> + int len;
>> +
>> + /*
>> + * Find the tag, and the offset of the next one. If we need to
>> + * stop including tags, then by default we stop *after*
>> + * including the current tag
>> + */
>> + offset = nextoffset;
>> + tag = fdt_next_tag(fdt, offset, &nextoffset);
>> + stop_at = nextoffset;
>> +
>> + switch (tag) {
>> + case FDT_PROP:
>> + stop_at = offset;
>> + prop = fdt_get_property_by_offset(fdt, offset, NULL);
>> + str = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
>> + val = h_include(priv, FDT_IS_PROP, str,
>> + strlen(str) + 1);
>> + if (val == -1) {
>> + include = want == WANT_NODES_AND_PROPS;
>> + } else {
>> + include = val;
>> + /*
>> + * Make sure we include the } for this block.
>> + * It might be more correct to put this done
>> + * by the call to fdt_include_supernodes() in
>> + * the case where it adds the node we are
>> + * currently in, but this is fairly
>> + * equivalent.
>> + */
>> + if ((flags & FDT_REG_SUPERNODES) && val &&
>> + !want)
>> + want = WANT_NODES_ONLY;
>> + }
>> +
>> + /* Value grepping is not yet supported */
>> + break;
>> +
>> + case FDT_NOP:
>> + include = want == WANT_NODES_AND_PROPS;
>> + stop_at = offset;
>> + break;
>> +
>> + case FDT_BEGIN_NODE:
>> + depth++;
>> + if (depth == FDT_MAX_DEPTH)
>> + return -FDT_ERR_BADSTRUCTURE;
>
> Hrm. Perhaps we should add a new error code for "too deep". After
> all the structure of the tree is perfectly fine here, it's just this
> search function can't cope with its depth.
Done
>
>> + name = fdt_get_name(fdt, offset, &len);
>> + if (end - path + 2 + len >= path_len)
>> + return -FDT_ERR_NOSPACE;
>> +
>> + /* Build the full path of this node */
>> + if (end != path + 1)
>> + *end++ = '/';
>> + strcpy(end, name);
>> + end += len;
>> + info.stack[depth].want = want;
>> + info.stack[depth].offset = offset;
>> +
>> + /*
>> + * If we are not intending to include this node, make
>> + * sure we stop *before* its tag
>> + */
>> + if (want == WANT_NODES_ONLY ||
>> + !(flags & FDT_REG_DIRECT_SUBNODES)) {
>> + stop_at = offset;
>> + want = WANT_NOTHING;
>> + }
>> + val = h_include(priv, FDT_IS_NODE, path,
>> + end - path + 1);
>> +
>> + /*
>> + * If no information about this, try the compatible
>> + * property. Here we are looking ahead in the tree.
>> + */
>> + if (val == -1) {
>> + str = fdt_getprop(fdt, offset,
>> + "compatible", &len);
>> + val = h_include(priv, FDT_IS_COMPAT, str,
>> + len);
>> + }
>> +
>> + /* Include this if requested */
>> + if (val)
>> + want = WANT_NODES_AND_PROPS;
>> +
>> + /* If not requested, decay our 'want' value */
>> + else if (want)
>> + want--;
>> +
>> + /* Not including this tag, so stop now */
>> + else
>> + stop_at = offset;
>> +
>> + /*
>> + * Decide whether to include this tag, and update our
>> + * stack with the state for this node
>> + */
>> + include = want;
>> + info.stack[depth].included = include;
>> + break;
>> +
>> + case FDT_END_NODE:
>> + include = want;
>> + if (depth < 0)
>> + return -FDT_ERR_BADSTRUCTURE;
>> +
>> + /*
>> + * If we don't want this node, stop right away, unless
>> + * we are including subnodes
>> + */
>> + if (!want && !(flags & FDT_REG_DIRECT_SUBNODES))
>> + stop_at = offset;
>> + want = info.stack[depth].want;
>> + depth--;
>> + while (end > path && *--end != '/')
>> + ;
>> + *end = '\0';
>> + break;
>> +
>> + case FDT_END:
>> + /* We always include the end tag */
>> + include = 1;
>> + break;
>> + }
>> +
>> + /* If this tag is to be included, mark it as region start */
>> + if (include && start == -1) {
>> + /* Include any supernodes required by this one */
>> + if (flags & FDT_REG_SUPERNODES)
>> + fdt_include_supernodes(&info, depth);
>> + start = offset;
>> + }
>> +
>> + /*
>> + * If this tag is not to be included, finish up the current
>> + * region
>> + */
>> + if (!include && start != -1) {
>> + fdt_add_region(&info, base + start, stop_at - start);
>> + start = -1;
>> + info.can_merge = 1;
>> + }
>> + } while (tag != FDT_END);
>> +
>> + if (nextoffset != fdt_size_dt_struct(fdt))
>> + return -FDT_ERR_BADLAYOUT;
>
> That should be BADSTRUCTURE, not BADLAYOUT. BADLAYOUT means
> structure/strings/memreserve blocks are not in the preferred order
> w.r.t. each other.
OK, done
>
>> +
>> + /* Add a region for the END tag and a separate one for string table */
>> + fdt_add_region(&info, base + start, nextoffset - start);
>> + if (flags & FDT_REG_ADD_STRING_TAB) {
>> + info.can_merge = 0;
>> + fdt_add_region(&info, fdt_off_dt_strings(fdt),
>> + fdt_size_dt_strings(fdt));
>> + }
>
> You may want to give BADLAYOUT here, though, if the strings block is
> before the structure block.
OK, will add this.
>
>> +
>> + return info.count;
>> +}
>> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
>> index c0075e7..0b3eacf 100644
>> --- a/libfdt/libfdt.h
>> +++ b/libfdt/libfdt.h
>> @@ -1489,4 +1489,146 @@ int fdt_del_node(void *fdt, int nodeoffset);
>>
>> const char *fdt_strerror(int errval);
>>
>> +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 the FDT_BEGIN_NODE tags of subnodes, including their names */
>> +#define FDT_REG_DIRECT_SUBNODES (1 << 1)
>> +
>> +/*
>> + * Add all supernodes of a matching node/property, useful for creating a
>> + * valid subset tree
>> + */
>> +#define FDT_REG_SUPERNODES (1 << 2)
>> +
>> +/* Add a region for the mem_rsvmap table (always the first region) */
>> +#define FDT_REG_ADD_MEM_RSVMAP (1 << 3)
>> +
>> +/* Indicates what an fdt part is (node, property, value, compatible) */
>> +#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)
>> +
>> +#define FDT_IS_ANY 15
>> +
>> +/**
>> + * fdt_find_regions() - 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 even with this subsetting.
I will add this comment.
>
>> + * 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 - if no information is available
>
> You don't describe what this function does when told "no information
> is available". Really the option seems a bit pointless to me, why not
> just leave the include by default or exclude by default decision to
> the include function.
This is partly used to deal with the conflict between node name
and node compatible string which you picked up on later. But it is
also used to deal with properties. I will add a more detailed comment.
>
>> + *
>> + * For nodes, the function may be called twice, once with FDT_IS_NODE to check
>> + * the node name. If this returns -1 then the function is immediately called
>> + * again with FDT_IS_COMPAT to check the compatible string. Note that if
>> + * there is no compatible string, then data == NULL and size == 0. This means
>> + * that the inclusion of a node can be controlled either by a node name or
>> + * its compatible string.
>
> This seems awkward to me. Why not supply the fdt pointer and node
> offset when calling the inclusion function for a node and have
> h_include get whatever properties it needs to make that decision.
Yes it is awkward, and I have been thinking of how to tidy this
up. I think I will just go with what you suggest since the cost of
trying to make things simple for the caller (in terms of allowing a
simple strcmp() there for example) is much higher than the benefit.
>
>> + *
>> + * There is no special handling of unit addresses, so the entire name must
>> + * be given for nodes. Wildcards are not supported.
>
> Given where? The only names given in the interface are in the tree
> itself. Wildcards could be supported by the h_include function AFAICT.
This comment refers to an earlier interface that I originally tried
(passing node names to the function). I dropped it because it was
inflexible. I will drop this comment.
>
>> + * 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()
>> + * @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_find_regions(const void *fdt,
>> + int (*h_include)(void *priv, int type, const char *data,
>> + int size),
>> + void *priv,
>> + struct fdt_region region[], int max_regions,
>> + char *path, int path_len, int flags);
>> +
>> #endif /* _LIBFDT_H */
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
Regards,
Simon
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2013-02-15 22:47 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-21 20:59 [PATCH 0/8] Introduce fdtgrep for subsetting and hashing FDTs Simon Glass
[not found] ` <1358801962-21707-1-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-01-21 20:59 ` [PATCH 1/8] Adjust util_is_printable_string() comment and fix test Simon Glass
[not found] ` <1358801962-21707-2-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-01-22 5:33 ` David Gibson
[not found] ` <20130122053340.GJ23500-W9XWwYn+TF0XU02nzanrWNbf9cGiqdzd@public.gmane.org>
2013-01-27 19:14 ` Simon Glass
2013-01-21 20:59 ` [PATCH 2/8] Move property-printing into util Simon Glass
[not found] ` <1358801962-21707-3-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-01-22 5:35 ` David Gibson
2013-01-27 20:30 ` Jon Loeliger
2013-01-21 20:59 ` [PATCH 3/8] .gitignore: Add rule for *.patch Simon Glass
[not found] ` <1358801962-21707-4-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-01-22 5:35 ` David Gibson
2013-01-27 20:30 ` Jon Loeliger
2013-01-21 20:59 ` [PATCH 4/8] Export fdt_stringlist_contains() Simon Glass
[not found] ` <1358801962-21707-5-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-01-23 5:07 ` David Gibson
2013-01-27 20:31 ` Jon Loeliger
2013-01-21 20:59 ` [PATCH 5/8] libfdt: Add function to find regions in an FDT Simon Glass
[not found] ` <1358801962-21707-6-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-02-11 2:47 ` David Gibson
[not found] ` <20130211024736.GG4948-W9XWwYn+TF0XU02nzanrWNbf9cGiqdzd@public.gmane.org>
2013-02-15 22:47 ` Simon Glass
2013-01-21 20:59 ` [PATCH 6/8] Add fdtgrep to grep and subset FDTs Simon Glass
2013-01-21 20:59 ` [PATCH 7/8] Remove fdtdump and use fdtgrep instead Simon Glass
[not found] ` <1358801962-21707-8-git-send-email-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2013-02-06 7:16 ` David Gibson
[not found] ` <20130206071606.GR821-W9XWwYn+TF0XU02nzanrWNbf9cGiqdzd@public.gmane.org>
2013-02-06 16:40 ` Simon Glass
2013-01-21 20:59 ` [PATCH 8/8] RFC: Check offset in fdt_string() Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.