On Fri, Feb 27, 2015 at 08:55:44PM +0200, Pantelis Antoniou wrote:
> Enable the generation of symbols & local fixup information
> for trees compiled with the -@ (--symbols) option.
>
> Using this patch labels in the tree and their users emit information
> in __symbols__ and __local_fixups__ nodes. Using this information
> it is possible to implement run time dynamic tree loading.
>
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@xxxxxxxxxxxx>
> ---
> Documentation/manual.txt | 5 ++
> checks.c | 61 +++++++++++++++++++++
> dtc.c | 9 ++-
> dtc.h | 28 ++++++++++
> flattree.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 240 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/manual.txt b/Documentation/manual.txt
> index 398de32..4359958 100644
> --- a/Documentation/manual.txt
> +++ b/Documentation/manual.txt
> @@ -119,6 +119,11 @@ Options:
> Make space for <number> reserve map entries
> Relevant for dtb and asm output only.
>
> + -@
> + Generates a __symbols__ node at the root node of the resulting blob
> + for any node labels used, and for any local references using phandles
> + it also generates a __local_fixups__ node that tracks them.
> +
> -S <bytes>
> Ensure the blob at least <bytes> long, adding additional
> space if needed.
> diff --git a/checks.c b/checks.c
> index e81a8c7..103ca52 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -458,6 +458,7 @@ static void fixup_phandle_references(struct check *c, struct node *dt,
> struct node *node, struct property *prop)
> {
> struct marker *m = prop->val.markers;
> + struct fixup_entry *fe, **fep;
> struct node *refnode;
> cell_t phandle;
>
> @@ -471,9 +472,28 @@ static void fixup_phandle_references(struct check *c, struct node *dt,
> continue;
> }
>
> + /* if it's a local reference, we need to record it */
> + if (symbol_fixup_support) {
> +
> + /* allocate a new local fixup entry */
> + fe = xmalloc(sizeof(*fe));
> +
> + fe->node = node;
> + fe->prop = prop;
> + fe->offset = m->offset;
> + fe->next = NULL;
You're not initializing the local_fixup_generated field.
> +
> + /* append it to the local fixups */
> + fep = &dt->local_fixups;
> + while (*fep)
> + fep = &(*fep)->next;
> + *fep = fe;
> + }
> +
> phandle = get_node_phandle(dt, refnode);
> *((cell_t *)(prop->val.val + m->offset)) = cpu_to_fdt32(phandle);
> }
> +
> }
> ERROR(phandle_references, NULL, NULL, fixup_phandle_references, NULL,
> &duplicate_node_names, &explicit_phandles);
> @@ -652,6 +672,45 @@ static void check_obsolete_chosen_interrupt_controller(struct check *c,
> }
> TREE_WARNING(obsolete_chosen_interrupt_controller, NULL);
>
> +static void check_auto_label_phandles(struct check *c, struct node *dt,
> + struct node *node)
This should be called fixup_<something>. The naming convention is to
distinguish checks that actually check something, from fixups which
use the check infrastructure to traverse the tree and adjust things
but don't actually check anything.
> +{
> + struct label *l;
> + struct symbol *s, **sp;
> + int has_label;
> +
> + if (!symbol_fixup_support)
> + return;
> +
> + has_label = 0;
> + for_each_label(node->labels, l) {
> + has_label = 1;
> + break;
> + }
> +
> + if (!has_label)
> + return;
> +
> + /* force allocation of a phandle for this node */
> + (void)get_node_phandle(dt, node);
> +
> + /* add the symbol */
> + for_each_label(node->labels, l) {
> +
> + s = xmalloc(sizeof(*s));
> + s->label = l;
> + s->node = node;
> + s->next = NULL;
> +
> + /* add it to the symbols list */
> + sp = &dt->symbols;
> + while (*sp)
> + sp = &((*sp)->next);
> + *sp = s;
> + }
> +}
> +NODE_WARNING(auto_label_phandles, NULL);
This almost certainly should have some prereq checks/fixups.
> static struct check *check_table[] = {
> &duplicate_node_names, &duplicate_property_names,
> &node_name_chars, &node_name_format, &property_name_chars,
> @@ -670,6 +729,8 @@ static struct check *check_table[] = {
> &avoid_default_addr_size,
> &obsolete_chosen_interrupt_controller,
>
> + &auto_label_phandles,
> +
> &always_fail,
> };
>
> diff --git a/dtc.c b/dtc.c
> index 8c4add6..91e91e7 100644
> --- a/dtc.c
> +++ b/dtc.c
> @@ -29,6 +29,7 @@ int reservenum; /* Number of memory reservation slots */
> int minsize; /* Minimum blob size */
> int padsize; /* Additional padding to blob */
> int phandle_format = PHANDLE_BOTH; /* Use linux,phandle or phandle properties */
> +int symbol_fixup_support = 0;
>
> static void fill_fullpaths(struct node *tree, const char *prefix)
> {
> @@ -51,7 +52,7 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
> #define FDT_VERSION(version) _FDT_VERSION(version)
> #define _FDT_VERSION(version) #version
> static const char usage_synopsis[] = "dtc [options] <input file>";
> -static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:fb:i:H:sW:E:hv";
> +static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:fb:i:H:sW:E:@hv";
> static struct option const usage_long_opts[] = {
> {"quiet", no_argument, NULL, 'q'},
> {"in-format", a_argument, NULL, 'I'},
> @@ -69,6 +70,7 @@ static struct option const usage_long_opts[] = {
> {"phandle", a_argument, NULL, 'H'},
> {"warning", a_argument, NULL, 'W'},
> {"error", a_argument, NULL, 'E'},
> + {"symbols", no_argument, NULL, '@'},
> {"help", no_argument, NULL, 'h'},
> {"version", no_argument, NULL, 'v'},
> {NULL, no_argument, NULL, 0x0},
> @@ -99,6 +101,7 @@ static const char * const usage_opts_help[] = {
> "\t\tboth - Both \"linux,phandle\" and \"phandle\" properties",
> "\n\tEnable/disable warnings (prefix with \"no-\")",
> "\n\tEnable/disable errors (prefix with \"no-\")",
> + "\n\tEnable symbols/fixup support",
> "\n\tPrint this help and exit",
> "\n\tPrint version and exit",
> NULL,
> @@ -186,7 +189,9 @@ int main(int argc, char *argv[])
> case 'E':
> parse_checks_option(false, true, optarg);
> break;
> -
> + case '@':
> + symbol_fixup_support = 1;
> + break;
> case 'h':
> usage(NULL);
> default:
> diff --git a/dtc.h b/dtc.h
> index 56212c8..16354fa 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -54,6 +54,7 @@ extern int reservenum; /* Number of memory reservation slots */
> extern int minsize; /* Minimum blob size */
> extern int padsize; /* Additional padding to blob */
> extern int phandle_format; /* Use linux,phandle or phandle properties */
> +extern int symbol_fixup_support;/* enable symbols & fixup support */
>
> #define PHANDLE_LEGACY 0x1
> #define PHANDLE_EPAPR 0x2
> @@ -132,6 +133,20 @@ struct label {
> struct label *next;
> };
>
> +struct fixup_entry {
> + int offset;
> + struct node *node;
> + struct property *prop;
> + struct fixup_entry *next;
> + bool local_fixup_generated;
> +};
> +
> +struct symbol {
> + struct label *label;
> + struct node *node;
> + struct symbol *next;
> +};
> +
> struct property {
> bool deleted;
> char *name;
> @@ -158,6 +173,10 @@ struct node {
> int addr_cells, size_cells;
>
> struct label *labels;
> +
> + struct symbol *symbols;
> + struct fixup_entry *local_fixups;
You've put these fields in the structure for every node, but you're
only using them on the root node.
> + bool emit_local_fixup_node;
> };
>
> #define for_each_label_withdel(l0, l) \
> @@ -181,6 +200,15 @@ struct node {
> for_each_child_withdel(n, c) \
> if (!(c)->deleted)
>
> +#define for_each_fixup_entry(f, fe) \
> + for ((fe) = (f)->entries; (fe); (fe) = (fe)->next)
This macro is never used.
> +#define for_each_symbol(n, s) \
> + for ((s) = (n)->symbols; (s); (s) = (s)->next)
> +
> +#define for_each_local_fixup_entry(n, fe) \
> + for ((fe) = (n)->local_fixups; (fe); (fe) = (fe)->next)
> +
> void add_label(struct label **labels, char *label);
> void delete_labels(struct label **labels);
>
> diff --git a/flattree.c b/flattree.c
> index bd99fa2..3a58949 100644
> --- a/flattree.c
> +++ b/flattree.c
> @@ -255,6 +255,142 @@ static int stringtable_insert(struct data *d, const char *str)
> return i;
> }
>
> +static void emit_local_fixups(struct node *tree, struct emitter *emit,
> + void *etarget, struct data *strbuf, struct version_info *vi,
> + struct node *node)
Constructing these nodes during the emit code in flattree.c makes life
unnecessarily difficult, and means they'll be missing from -O dts
output. Instead you should construct the fixup nodes in the live
tree, then emit them in the usual way.
> +{
> + struct fixup_entry *fe, *fen;
> + struct node *child;
> + int nameoff, count;
> + cell_t *buf;
> + struct data d;
> +
> + if (node->emit_local_fixup_node) {
> +
> + /* emit the external fixups (do not emit /) */
> + if (node != tree) {
> + emit->beginnode(etarget, NULL);
> + emit->string(etarget, node->name, 0);
> + emit->align(etarget, sizeof(cell_t));
> + }
> +
> + for_each_local_fixup_entry(tree, fe) {
> + if (fe->node != node || fe->local_fixup_generated)
> + continue;
> +
> + /* count the number of fixup entries */
> + count = 0;
> + for_each_local_fixup_entry(tree, fen) {
> + if (fen->prop != fe->prop)
> + continue;
> + fen->local_fixup_generated = true;
> + count++;
> + }
> +
> + /* allocate buffer */
> + buf = xmalloc(count * sizeof(cell_t));
> +
> + /* collect all the offsets in buffer */
> + count = 0;
> + for_each_local_fixup_entry(tree, fen) {
> + if (fen->prop != fe->prop)
> + continue;
> + fen->local_fixup_generated = true;
> + buf[count++] = cpu_to_fdt32(fen->offset);
> + }
> + d = empty_data;
> + d.len = count * sizeof(cell_t);
> + d.val = (char *)buf;
> +
> + nameoff = stringtable_insert(strbuf, fe->prop->name);
> + emit->property(etarget, fe->prop->labels);
> + emit->cell(etarget, count * sizeof(cell_t));
> + emit->cell(etarget, nameoff);
> +
> + if ((vi->flags & FTF_VARALIGN) &&
> + (count * sizeof(cell_t)) >= 8)
> + emit->align(etarget, 8);
> +
> + emit->data(etarget, d);
> + emit->align(etarget, sizeof(cell_t));
> +
> + free(buf);
> + }
> + }
> +
> + for_each_child(node, child)
> + emit_local_fixups(tree, emit, etarget, strbuf, vi, child);
> +
> + if (node->emit_local_fixup_node && node != tree)
> + emit->endnode(etarget, tree->labels);
> +}
> +
> +static void emit_symbols_node(struct node *tree, struct emitter *emit,
> + void *etarget, struct data *strbuf,
> + struct version_info *vi)
> +{
> + struct symbol *sym;
> + int nameoff, vallen;
> +
> + /* do nothing if no symbols */
> + if (!tree->symbols)
> + return;
> +
> + emit->beginnode(etarget, NULL);
> + emit->string(etarget, "__symbols__", 0);
> + emit->align(etarget, sizeof(cell_t));
> +
> + for_each_symbol(tree, sym) {
> +
> + vallen = strlen(sym->node->fullpath);
> +
> + nameoff = stringtable_insert(strbuf, sym->label->label);
> +
> + emit->property(etarget, NULL);
> + emit->cell(etarget, vallen + 1);
> + emit->cell(etarget, nameoff);
> +
> + if ((vi->flags & FTF_VARALIGN) && vallen >= 8)
> + emit->align(etarget, 8);
> +
> + emit->string(etarget, sym->node->fullpath,
> + strlen(sym->node->fullpath));
> + emit->align(etarget, sizeof(cell_t));
> + }
> +
> + emit->endnode(etarget, NULL);
> +}
> +
> +static void emit_local_fixups_node(struct node *tree, struct emitter *emit,
> + void *etarget, struct data *strbuf,
> + struct version_info *vi)
> +{
> + struct fixup_entry *fe;
> + struct node *node;
> +
> + /* do nothing if no local fixups */
> + if (!tree->local_fixups)
> + return;
> +
> + /* mark all nodes that need a local fixup generated (and parents) */
> + for_each_local_fixup_entry(tree, fe) {
> + node = fe->node;
> + while (node != NULL && !node->emit_local_fixup_node) {
> + node->emit_local_fixup_node = true;
> + node = node->parent;
> + }
> + }
> +
> + /* emit the local fixups node now */
> + emit->beginnode(etarget, NULL);
> + emit->string(etarget, "__local_fixups__", 0);
> + emit->align(etarget, sizeof(cell_t));
> +
> + emit_local_fixups(tree, emit, etarget, strbuf, vi, tree);
> +
> + emit->endnode(etarget, tree->labels);
> +}
> +
> static void flatten_tree(struct node *tree, struct emitter *emit,
> void *etarget, struct data *strbuf,
> struct version_info *vi)
> @@ -310,6 +446,9 @@ static void flatten_tree(struct node *tree, struct emitter *emit,
> flatten_tree(child, emit, etarget, strbuf, vi);
> }
>
> + emit_symbols_node(tree, emit, etarget, strbuf, vi);
> + emit_local_fixups_node(tree, emit, etarget, strbuf, vi);
> +
> emit->endnode(etarget, tree->labels);
> }
>
--
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:
pgp961n9tOrqQ.pgp
Description: PGP signature
