On Tue, Nov 29, 2016 at 01:09:11PM +0200, Pantelis Antoniou wrote: > Hi David, > > > On Nov 29, 2016, at 04:10 , David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote: > > > > On Mon, Nov 28, 2016 at 02:10:35PM +0200, Pantelis Antoniou wrote: > >> > >>> On Nov 28, 2016, at 06:12 , David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote: > >>> > >>> On Fri, Nov 25, 2016 at 02:32:10PM +0200, Pantelis Antoniou wrote: > >>>> This patch 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. > >>>> > >>>> The __fixups__ node make possible the dynamic resolution of phandle > >>>> references which are present in the plugin tree but lie in the > >>>> tree that are applying the overlay against. > >>>> > >>>> While there is a new magic number for dynamic device tree/overlays blobs > >>>> it is by default enabled. Remember to use -M to generate compatible > >>>> blobs. > >>>> > >>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@xxxxxxxxxxxx> > >>>> Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> > >>>> Signed-off-by: Jan Luebbe <jlu@xxxxxxxxxxxxxx> > >>>> --- > >>>> Documentation/manual.txt | 25 +++++- > >>>> checks.c | 8 +- > >>>> dtc-lexer.l | 5 ++ > >>>> dtc-parser.y | 50 +++++++++-- > >>>> dtc.c | 39 +++++++- > >>>> dtc.h | 20 ++++- > >>>> fdtdump.c | 2 +- > >>>> flattree.c | 17 ++-- > >>>> fstree.c | 2 +- > >>>> libfdt/fdt.c | 2 +- > >>>> libfdt/fdt.h | 3 +- > >>>> livetree.c | 225 ++++++++++++++++++++++++++++++++++++++++++++++- > >>>> tests/mangle-layout.c | 7 +- > >>>> 13 files changed, 375 insertions(+), 30 deletions(-) > >>>> > >>>> diff --git a/Documentation/manual.txt b/Documentation/manual.txt > >>>> index 398de32..094893b 100644 > >>>> --- a/Documentation/manual.txt > >>>> +++ b/Documentation/manual.txt > >>>> @@ -119,6 +119,24 @@ 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. > >>>> + > >>>> + When using the /plugin/ tag all unresolved label references to > >>>> + be tracked in the __fixups__ node, making dynamic resolution possible. > >>>> + > >>>> + -A > >>>> + Generate automatically aliases for all node labels. This is similar to > >>>> + the -@ option (the __symbols__ node contain identical information) but > >>>> + the semantics are slightly different since no phandles are automatically > >>>> + generated for labeled nodes. > >>>> + > >>>> + -M > >>>> + Generate blobs with the old FDT magic number for device tree objects. > >>>> + By default blobs use the DTBO FDT magic number instead. > >>>> + > >>>> -S <bytes> > >>>> Ensure the blob at least <bytes> long, adding additional > >>>> space if needed. > >>>> @@ -146,13 +164,18 @@ Additionally, dtc performs various sanity checks on the tree. > >>>> Here is a very rough overview of the layout of a DTS source file: > >>>> > >>>> > >>>> - sourcefile: list_of_memreserve devicetree > >>>> + sourcefile: versioninfo plugindecl list_of_memreserve devicetree > >>>> > >>>> memreserve: label 'memreserve' ADDR ADDR ';' > >>>> | label 'memreserve' ADDR '-' ADDR ';' > >>>> > >>>> devicetree: '/' nodedef > >>>> > >>>> + versioninfo: '/' 'dts-v1' '/' ';' > >>>> + > >>>> + plugindecl: '/' 'plugin' '/' ';' > >>>> + | /* empty */ > >>>> + > >>>> nodedef: '{' list_of_property list_of_subnode '}' ';' > >>>> > >>>> property: label PROPNAME '=' propdata ';' > >>>> diff --git a/checks.c b/checks.c > >>>> index 2bd27a4..4292f4b 100644 > >>>> --- a/checks.c > >>>> +++ b/checks.c > >>>> @@ -487,8 +487,12 @@ static void fixup_phandle_references(struct check *c, struct boot_info *bi, > >>>> > >>>> refnode = get_node_by_ref(dt, m->ref); > >>>> if (! refnode) { > >>>> - FAIL(c, "Reference to non-existent node or label \"%s\"\n", > >>>> - m->ref); > >>>> + if (!(bi->versionflags & VF_PLUGIN)) > >>>> + FAIL(c, "Reference to non-existent node or " > >>>> + "label \"%s\"\n", m->ref); > >>>> + else /* mark the entry as unresolved */ > >>>> + *((cell_t *)(prop->val.val + m->offset)) = > >>>> + cpu_to_fdt32(0xffffffff); > >>>> continue; > >>>> } > >>>> > >>>> diff --git a/dtc-lexer.l b/dtc-lexer.l > >>>> index 790fbf6..40bbc87 100644 > >>>> --- a/dtc-lexer.l > >>>> +++ b/dtc-lexer.l > >>>> @@ -121,6 +121,11 @@ static void lexical_error(const char *fmt, ...); > >>>> return DT_V1; > >>>> } > >>>> > >>>> +<*>"/plugin/" { > >>>> + DPRINT("Keyword: /plugin/\n"); > >>>> + return DT_PLUGIN; > >>>> + } > >>>> + > >>>> <*>"/memreserve/" { > >>>> DPRINT("Keyword: /memreserve/\n"); > >>>> BEGIN_DEFAULT(); > >>>> diff --git a/dtc-parser.y b/dtc-parser.y > >>>> index 14aaf2e..1a1f660 100644 > >>>> --- a/dtc-parser.y > >>>> +++ b/dtc-parser.y > >>>> @@ -19,6 +19,7 @@ > >>>> */ > >>>> %{ > >>>> #include <stdio.h> > >>>> +#include <inttypes.h> > >>>> > >>>> #include "dtc.h" > >>>> #include "srcpos.h" > >>>> @@ -33,6 +34,7 @@ extern void yyerror(char const *s); > >>>> > >>>> extern struct boot_info *the_boot_info; > >>>> extern bool treesource_error; > >>>> + > >>> > >>> Extraneous whitespace change here > >>> > >> > >> OK. > >> > >>>> %} > >>>> > >>>> %union { > >>>> @@ -52,9 +54,11 @@ extern bool treesource_error; > >>>> struct node *nodelist; > >>>> struct reserve_info *re; > >>>> uint64_t integer; > >>>> + unsigned int flags; > >>>> } > >>>> > >>>> %token DT_V1 > >>>> +%token DT_PLUGIN > >>>> %token DT_MEMRESERVE > >>>> %token DT_LSHIFT DT_RSHIFT DT_LE DT_GE DT_EQ DT_NE DT_AND DT_OR > >>>> %token DT_BITS > >>>> @@ -71,6 +75,8 @@ extern bool treesource_error; > >>>> > >>>> %type <data> propdata > >>>> %type <data> propdataprefix > >>>> +%type <flags> versioninfo > >>>> +%type <flags> plugindecl > >>>> %type <re> memreserve > >>>> %type <re> memreserves > >>>> %type <array> arrayprefix > >>>> @@ -101,16 +107,34 @@ extern bool treesource_error; > >>>> %% > >>>> > >>>> sourcefile: > >>>> - v1tag memreserves devicetree > >>>> + versioninfo plugindecl memreserves devicetree > >>>> + { > >>>> + the_boot_info = build_boot_info($1 | $2, $3, $4, > >>>> + guess_boot_cpuid($4)); > >>>> + } > >>>> + ; > >>>> + > >>>> +versioninfo: > >>>> + v1tag > >>>> { > >>>> - the_boot_info = build_boot_info($2, $3, > >>>> - guess_boot_cpuid($3)); > >>>> + $$ = VF_DT_V1; > >>>> } > >>>> ; > >>>> > >>>> v1tag: > >>>> DT_V1 ';' > >>>> + | DT_V1 > >>>> | DT_V1 ';' v1tag > >>>> + > >>>> +plugindecl: > >>>> + DT_PLUGIN ';' > >>>> + { > >>>> + $$ = VF_PLUGIN; > >>>> + } > >>>> + | /* empty */ > >>>> + { > >>>> + $$ = 0; > >>>> + } > >>>> ; > >>>> > >>>> memreserves: > >>>> @@ -161,10 +185,19 @@ devicetree: > >>>> { > >>>> struct node *target = get_node_by_ref($1, $2); > >>>> > >>>> - if (target) > >>>> + if (target) { > >>>> merge_nodes(target, $3); > >>>> - else > >>>> - ERROR(&@2, "Label or path %s not found", $2); > >>>> + } else { > >>>> + /* > >>>> + * We rely on the rule being always: > >>>> + * versioninfo plugindecl memreserves devicetree > >>>> + * so $-1 is what we want (plugindecl) > >>>> + */ > >>>> + if ($<flags>-1 & VF_PLUGIN) > >>> > >>> o_O... ok. I've never seen negative value references before. Can you > >>> provide a link to some documentation saying this is actually supported > >>> usage in bison? I wasn't able to find it when I looked. > >>> > >> > >> There is a section about inherited attributes in the flex & bison book by O’Reily. > >> > >> https://books.google.gr/books?id=3Sr1V5J9_qMC&lpg=PP1&dq=flex%20bison&hl=el&pg=PP1#v=onepage&q=flex%20bison&f=false > >> > >> There’s a direct link to the 2nd Edition of lex & yacc: > >> > >> https://books.google.gr/books?id=fMPxfWfe67EC&lpg=PA183&ots=RcRSji2NAT&dq=yacc%20inherited%20attributes&hl=el&pg=PA183#v=onepage&q=yacc%20inherited%20attributes&f=false > > > > Thanks for the link. I still think moving the fragment assembly out > > of the parser will be a better idea long term, but this does address > > the main concern I had, so it will do for now. > > > >>>> + add_orphan_node($1, $3, $2); > >>>> + else > >>>> + ERROR(&@2, "Label or path %s not found", $2); > >>>> + } > >>>> $$ = $1; > >>>> } > >>>> | devicetree DT_DEL_NODE DT_REF ';' > >>>> @@ -179,6 +212,11 @@ devicetree: > >>>> > >>>> $$ = $1; > >>>> } > >>>> + | /* empty */ > >>>> + { > >>>> + /* build empty node */ > >>>> + $$ = name_node(build_node(NULL, NULL), ""); > >>>> + } > >>>> ; > >>>> > >>>> nodedef: > >>>> diff --git a/dtc.c b/dtc.c > >>>> index 9dcf640..06e91bc 100644 > >>>> --- a/dtc.c > >>>> +++ b/dtc.c > >>>> @@ -32,6 +32,9 @@ int minsize; /* Minimum blob size */ > >>>> int padsize; /* Additional padding to blob */ > >>>> int alignsize; /* Additional padding to blob accroding to the alignsize */ > >>>> int phandle_format = PHANDLE_BOTH; /* Use linux,phandle or phandle properties */ > >>>> +int symbol_fixup_support; /* enable symbols & fixup support */ > >>>> +int auto_label_aliases; /* auto generate labels -> aliases */ > >>>> +int no_dtbo_magic; /* use old FDT magic values for objects */ > >>>> > >>>> static int is_power_of_2(int x) > >>>> { > >>>> @@ -59,7 +62,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:a:fb:i:H:sW:E:hv"; > >>>> +static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AMhv"; > >>>> static struct option const usage_long_opts[] = { > >>>> {"quiet", no_argument, NULL, 'q'}, > >>>> {"in-format", a_argument, NULL, 'I'}, > >>>> @@ -78,6 +81,9 @@ 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, '@'}, > >>>> + {"auto-alias", no_argument, NULL, 'A'}, > >>>> + {"no-dtbo-magic", no_argument, NULL, 'M'}, > >>>> {"help", no_argument, NULL, 'h'}, > >>>> {"version", no_argument, NULL, 'v'}, > >>>> {NULL, no_argument, NULL, 0x0}, > >>>> @@ -109,6 +115,9 @@ 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\tEnable auto-alias of labels", > >>>> + "\n\tDo not use DTBO magic value for plugin objects", > >>>> "\n\tPrint this help and exit", > >>>> "\n\tPrint version and exit", > >>>> NULL, > >>>> @@ -153,7 +162,7 @@ static const char *guess_input_format(const char *fname, const char *fallback) > >>>> fclose(f); > >>>> > >>>> magic = fdt32_to_cpu(magic); > >>>> - if (magic == FDT_MAGIC) > >>>> + if (magic == FDT_MAGIC || magic == FDT_MAGIC_DTBO) > >>>> return "dtb"; > >>>> > >>>> return guess_type_by_name(fname, fallback); > >>>> @@ -172,6 +181,7 @@ int main(int argc, char *argv[]) > >>>> FILE *outf = NULL; > >>>> int outversion = DEFAULT_FDT_VERSION; > >>>> long long cmdline_boot_cpuid = -1; > >>>> + fdt32_t out_magic = FDT_MAGIC; > >>>> > >>>> quiet = 0; > >>>> reservenum = 0; > >>>> @@ -249,6 +259,16 @@ int main(int argc, char *argv[]) > >>>> parse_checks_option(false, true, optarg); > >>>> break; > >>>> > >>>> + case '@': > >>>> + symbol_fixup_support = 1; > >>>> + break; > >>>> + case 'A': > >>>> + auto_label_aliases = 1; > >>>> + break; > >>>> + case 'M': > >>>> + no_dtbo_magic = 1; > >>>> + break; > >>>> + > >>>> case 'h': > >>>> usage(NULL); > >>>> default: > >>>> @@ -306,6 +326,14 @@ int main(int argc, char *argv[]) > >>>> fill_fullpaths(bi->dt, ""); > >>>> process_checks(force, bi); > >>>> > >>>> + if (auto_label_aliases) > >>>> + generate_label_tree(bi->dt, "aliases", false); > >>>> + > >>>> + if (symbol_fixup_support) { > >>>> + generate_label_tree(bi->dt, "__symbols__", true); > >>>> + generate_fixups_tree(bi->dt); > >>> > >>> Hang on.. this doesn't seem right. I thought -@ controlled the > >>> __symbols__ side (i.e. the part upon which we overlay) rather than the > >>> fixups side (the part which overlays). A dtbo could certainly have > >>> both, of course, but for base trees, wouldn't you have symbols without > >>> fixups? And should it be illegal to try to build a /plugin/ without > >>> -@? > >> > >> It does control both for now. For base trees having the fixup nodes > >> will allow us to do probe order dependency tracking in the future. > > > > Erm.. how? > > > >> For plugins we need the __symbols__ node to support stacked overlays, i.e. > >> overlays referring label that were introduced by a previous overlay. > > > > Yes, I realise that an overlay may well want __symbols__ as well. But > > they still seem conceptually different. I think -@ should control > > __symbols__ whereas /plugin/ should control __fixups__. > > > > It is easily done. Although using /plugin/ as an auto-magic option does both > just fine. Sorry, I don't follow what you're saying. > >> For plugins there is no requirement for now to actually contain references to > >> be resolved. It can easily be enforced though. > > > > Sure, but I don't see the relevance of that here. You could just omit > > the __fixups__ node if there's nothing to go into them. > > > > Hmm, yeah. > > > Regards > > — Pantelis > -- 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:
signature.asc
Description: PGP signature