On Tue, May 24, 2016 at 03:53:07PM +0300, Pantelis Antoniou wrote:
> Hi David,
>
> > On May 24, 2016, at 13:50 , David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > On Tue, May 24, 2016 at 10:43:29AM +0300, Pantelis Antoniou wrote:
> >> Hi David,
> >>
> >>> On May 24, 2016, at 07:58 , David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> >>>
> >>> One small nit in the document itself.
> >>>
> >>> I have other comments, but they're about the overlay format itself,
> >>> rather than this patch as such.
> >>>
> >>
> >> OK.
> >>
> >>> On Thu, May 05, 2016 at 10:48:41PM +0300, Pantelis Antoniou wrote:
> >>>> Provides the document explaining the internal mechanics of
> >>>> plugins and options.
> >>>>
> >>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@xxxxxxxxxxxx>
> >>>> ---
> >>>> Documentation/dt-object-internal.txt | 318 +++++++++++++++++++++++++++++++++++
> >>>> 1 file changed, 318 insertions(+)
> >>>> create mode 100644 Documentation/dt-object-internal.txt
> >>>>
> >>>> diff --git a/Documentation/dt-object-internal.txt b/Documentation/dt-object-internal.txt
> >>>> new file mode 100644
> >>>> index 0000000..734f447
> >>>> --- /dev/null
> >>>> +++ b/Documentation/dt-object-internal.txt
> >>>> @@ -0,0 +1,318 @@
> >>>> +Device Tree Dynamic Object format internals
> >>>> +-------------------------------------------
> >>>> +
> >>>> +The Device Tree for most platforms is a static representation of
> >>>> +the hardware capabilities. This is insufficient for many platforms
> >>>> +that need to dynamically insert device tree fragments to the
> >>>> +running kernel's live tree.
> >>>> +
> >>>> +This document explains the the device tree object format and the
> >>>> +modifications made to the device tree compiler, which make it possible.
> >>>> +
> >>>> +1. Simplified Problem Definition
> >>>> +--------------------------------
> >>>> +
> >>>> +Assume we have a platform which boots using following simplified device tree.
> >>>> +
> >>>> +---- foo.dts -----------------------------------------------------------------
> >>>> + /* FOO platform */
> >>>> + / {
> >>>> + compatible = "corp,foo";
> >>>> +
> >>>> + /* shared resources */
> >>>> + res: res {
> >>>> + };
> >>>> +
> >>>> + /* On chip peripherals */
> >>>> + ocp: ocp {
> >>>> + /* peripherals that are always instantiated */
> >>>> + peripheral1 { ... };
> >>>> + };
> >>>> + };
> >>>> +---- foo.dts -----------------------------------------------------------------
> >>>> +
> >>>> +We have a number of peripherals that after probing (using some undefined method)
> >>>> +should result in different device tree configuration.
> >>>> +
> >>>> +We cannot boot with this static tree because due to the configuration of the
> >>>> +foo platform there exist multiple conficting peripherals DT fragments.
> >>>> +
> >>>> +So for the bar peripheral we would have this:
> >>>> +
> >>>> +---- foo+bar.dts -------------------------------------------------------------
> >>>> + /* FOO platform + bar peripheral */
> >>>> + / {
> >>>> + compatible = "corp,foo";
> >>>> +
> >>>> + /* shared resources */
> >>>> + res: res {
> >>>> + };
> >>>> +
> >>>> + /* On chip peripherals */
> >>>> + ocp: ocp {
> >>>> + /* peripherals that are always instantiated */
> >>>> + peripheral1 { ... };
> >>>> +
> >>>> + /* bar peripheral */
> >>>> + bar {
> >>>> + compatible = "corp,bar";
> >>>> + ... /* various properties and child nodes */
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> +---- foo+bar.dts -------------------------------------------------------------
> >>>> +
> >>>> +While for the baz peripheral we would have this:
> >>>> +
> >>>> +---- foo+baz.dts -------------------------------------------------------------
> >>>> + /* FOO platform + baz peripheral */
> >>>> + / {
> >>>> + compatible = "corp,foo";
> >>>> +
> >>>> + /* shared resources */
> >>>> + res: res {
> >>>> + /* baz resources */
> >>>> + baz_res: res_baz { ... };
> >>>> + };
> >>>> +
> >>>> + /* On chip peripherals */
> >>>> + ocp: ocp {
> >>>> + /* peripherals that are always instantiated */
> >>>> + peripheral1 { ... };
> >>>> +
> >>>> + /* baz peripheral */
> >>>> + baz {
> >>>> + compatible = "corp,baz";
> >>>> + /* reference to another point in the tree */
> >>>> + ref-to-res = <&baz_res>;
> >>>> + ... /* various properties and child nodes */
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> +---- foo+baz.dts -------------------------------------------------------------
> >>>> +
> >>>> +We note that the baz case is more complicated, since the baz peripheral needs to
> >>>> +reference another node in the DT tree.
> >>>> +
> >>>> +2. Device Tree Object Format Requirements
> >>>> +-----------------------------------------
> >>>> +
> >>>> +Since the device tree is used for booting a number of very different hardware
> >>>> +platforms it is imperative that we tread very carefully.
> >>>> +
> >>>> +2.a) No changes to the Device Tree binary format for the base tree. We cannot
> >>>> +modify the tree format at all and all the information we require should be
> >>>> +encoded using device tree itself. We can add nodes that can be safely ignored
> >>>> +by both bootloaders and the kernel. The plugin dtb's are optionally tagged
> >>>> +with a different magic number in the header but otherwise they too are simple
> >>>> +blobs.
> >>>> +
> >>>> +2.b) Changes to the DTS source format should be absolutely minimal, and should
> >>>> +only be needed for the DT fragment definitions, and not the base boot DT.
> >>>> +
> >>>> +2.c) An explicit option should be used to instruct DTC to generate the required
> >>>> +information needed for object resolution. Platforms that don't use the
> >>>> +dynamic object format can safely ignore it.
> >>>> +
> >>>> +2.d) Finally, DT syntax changes should be kept to a minimum. It should be
> >>>> +possible to express everything using the existing DT syntax.
> >>>> +
> >>>> +3. Implementation
> >>>> +-----------------
> >>>> +
> >>>> +The basic unit of addressing in Device Tree is the phandle. Turns out it's
> >>>> +relatively simple to extend the way phandles are generated and referenced
> >>>> +so that it's possible to dynamically convert symbolic references (labels)
> >>>> +to phandle values. This is a valid assumption as long as the author uses
> >>>> +reference syntax and does not assign phandle values manually (which might
> >>>> +be a problem with decompiled source files).
> >>>> +
> >>>> +We can roughly divide the operation into two steps.
> >>>> +
> >>>> +3.a) Compilation of the base board DTS file using the '-@' option
> >>>> +generates a valid DT blob with an added __symbols__ node at the root node,
> >>>> +containing a list of all nodes that are marked with a label.
> >>>> +
> >>>> +Using the foo.dts file above the following node will be generated;
> >>>> +
> >>>> +$ dtc -@ -O dtb -o foo.dtb -b 0 foo.dts
> >>>> +$ fdtdump foo.dtb
> >>>> +...
> >>>> +/ {
> >>>> + ...
> >>>> + res {
> >>>> + ...
> >>>> + phandle = <0x00000001>;
> >>>> + ...
> >>>> + };
> >>>> + ocp {
> >>>> + ...
> >>>> + phandle = <0x00000002>;
> >>>> + ...
> >>>> + };
> >>>> + __symbols__ {
> >>>> + res="/res";
> >>>> + ocp="/ocp";
> >>>> + };
> >>>> +};
> >>>> +
> >>>> +Notice that all the nodes that had a reference have been recorded, and that
> >>>
> >>> s/reference/label/
> >>>
> >>
> >> OK
> >>
> >>>> +phandles have been generated for them.
> >>>
> >>>
> >>>> +This blob can be used to boot the board normally, the __symbols__ node will
> >>>> +be safely ignored both by the bootloader and the kernel (the only loss will
> >>>> +be a few bytes of memory and disk space).
> >>>> +
> >>>> +3.b) The Device Tree fragments must be compiled with the same option but they
> >>>> +must also have a tag (/plugin/) that allows undefined references to nodes
> >>>> +that are not present at compilation time to be recorded so that the runtime
> >>>> +loader can fix them.
> >>>> +
> >>>> +So the bar peripheral's DTS format would be of the form:
> >>>> +
> >>>> +/dts-v1/ /plugin/; /* allow undefined references and record them */
> >>>> +/ {
> >>>> + .... /* various properties for loader use; i.e. part id etc. */
> >>>> + fragment@0 {
> >>>> + target = <&ocp>;
> >>>> + __overlay__ {
> >>>> + /* bar peripheral */
> >>>> + bar {
> >>>> + compatible = "corp,bar";
> >>>> + ... /* various properties and child nodes */
> >>>> + }
> >>>> + };
> >>>> + };
> >>>> +};
> >>>> +
> >>>> +Note that there's a target property that specifies the location where the
> >>>> +contents of the overlay node will be placed, and it references the node
> >>>> +in the foo.dts file.
> >>>
> >>> Ugh.. I really don't like the target stuff appearing in the dts like
> >>> this. I thought we were changing this so these appeared in the blob,
> >>> but in the source we just used the existing overlay syntax, so for the
> >>> above, something like:
> >>>
> >>> &ocp {
> >>> ...
> >>> };
> >>>
> >>
> >> This works, but it’s just syntactic sugar.
> >
> > Hmmm.... The target= property and fragment@ nodes are part of the
> > internal overlay glue, rather than actual DT content. So, I *really*
> > dislike including it inline in the dts file. Come to that, I dislike
> > including it in the dtb, but I can see the rationale and we're kind of
> > stuck with it anyway. The dts, not so much.
> >
>
> There are more target variants to come, but they don’t require any
> dtc changes.
>
> The &label { }; transforms to fragment@0 { target = <&label>; __overlay__ { }; };
>
> >> It does not cover the cases where the target is a path, or a different
> >> kind of target.
> >
> > Huh? It certainly covers the case of a path
> > &{/some/path} { ... }
> > What other sort of target did you have in mind?
> >
>
> That is just not supported right now. There is no phandle to resolve.
>
> I’ll take a stab at it, but it will transform to the target-path variant.
Right, but my point is there's an obvious dts syntex for
path-targetted fragments. My inclination is to create syntax for any
new fragment types, rather than expose the encoding of the overlay
metadata as if it were in-band information.
> >> Besides the sugary part, a target is something that doesn’t have anything to
> >> do with the plugin format.
> >>
> >>> Or have I gotten confused by the history of things.
> >>>
> >>
> >> It’s got a long history for sure :)
> >>
> >>>> +$ dtc -@ -O dtb -o bar.dtbo -b 0 bar.dts
> >>>> +$ fdtdump bar.dtbo
> >>>> +...
> >>>> +/ {
> >>>> + ... /* properties */
> >>>> + fragment@0 {
> >>>> + target = <0xffffffff>;
> >>>> + __overlay__ {
> >>>> + bar {
> >>>> + compatible = "corp,bar";
> >>>> + ... /* various properties and child nodes */
> >>>> + }
> >>>> + };
> >>>> + };
> >>>> + __fixups__ {
> >>>> + ocp = "/fragment@0:target:0";
> >>>
> >>> I still hate this parse-requiring string, but I guess we're stuck with
> >>> it.
> >>>
> >>>> + };
> >>>> +};
> >>>> +
> >>>> +No __symbols__ has been generated (no label in bar.dts).
> >>>> +Note that the target's ocp label is undefined, so the phandle handle
> >>>> +value is filled with the illegal value '0xffffffff', while a __fixups__
> >>>> +node has been generated, which marks the location in the tree where
> >>>> +the label lookup should store the runtime phandle value of the ocp node.
> >>>> +
> >>>> +The format of the __fixups__ node entry is
> >>>> +
> >>>> + <label> = "<local-full-path>:<property-name>:<offset>";
> >>>> +
> >>>> +<label> Is the label we're referring
> >>>> +<local-full-path> Is the full path of the node the reference is
> >>>> +<property-name> Is the name of the property containing the
> >>>> + reference
> >>>> +<offset> The offset (in bytes) of where the property's
> >>>> + phandle value is located.
> >>>> +
> >>>> +Doing the same with the baz peripheral's DTS format is a little bit more
> >>>> +involved, since baz contains references to local labels which require
> >>>> +local fixups.
> >>>> +
> >>>> +/dts-v1/ /plugin/; /* allow undefined label references and record them */
> >>>> +/ {
> >>>> + .... /* various properties for loader use; i.e. part id etc. */
> >>>> + fragment@0 {
> >>>> + target = <&res>;
> >>>> + __overlay__ {
> >>>> + /* baz resources */
> >>>> + baz_res: res_baz { ... };
> >>>> + };
> >>>> + };
> >>>> + fragment@1 {
> >>>> + target = <&ocp>;
> >>>> + __overlay__ {
> >>>> + /* baz peripheral */
> >>>> + baz {
> >>>> + compatible = "corp,baz";
> >>>> + /* reference to another point in the tree */
> >>>> + ref-to-res = <&baz_res>;
> >>>> + ... /* various properties and child nodes */
> >>>> + }
> >>>> + };
> >>>> + };
> >>>> +};
> >>>> +
> >>>> +Note that &bar_res reference.
> >>>> +
> >>>> +$ dtc -@ -O dtb -o baz.dtbo -b 0 baz.dts
> >>>> +$ fdtdump baz.dtbo
> >>>> +...
> >>>> +/ {
> >>>> + ... /* properties */
> >>>> + fragment@0 {
> >>>> + target = <0xffffffff>;
> >>>> + __overlay__ {
> >>>> + res_baz {
> >>>> + ....
> >>>> + phandle = <0x00000001>;
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> + fragment@1 {
> >>>> + target = <0xffffffff>;
> >>>> + __overlay__ {
> >>>> + baz {
> >>>> + compatible = "corp,baz";
> >>>> + ... /* various properties and child nodes */
> >>>> + ref-to-res = <0x00000001>;
> >>>> + }
> >>>> + };
> >>>> + };
> >>>> + __fixups__ {
> >>>> + res = "/fragment@0:target:0";
> >>>> + ocp = "/fragment@1:target:0";
> >>>> + };
> >>>> + __local_fixups__ {
> >>>> + fragment@1 {
> >>>> + __overlay__ {
> >>>> + baz {
> >>>> + ref-to-res = <0>;
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> +};
> >>>> +
> >>>> +This is similar to the bar case, but the reference of a local label by the
> >>>> +baz node generates a __local_fixups__ entry that records the place that the
> >>>> +local reference is being made. No matter how phandles are allocated from dtc
> >>>> +the run time loader must apply an offset to each phandle in every dynamic
> >>>> +DT object loaded. The __local_fixups__ node records the place of every
> >>>> +local reference so that the loader can apply the offset.
> >>>
> >>> I'm still utterly baffled why the __local_fixups__ encoding is totally
> >>> different from the __fixups__ encoding. But, again, stuck with it, I
> >>> guess.
> >>>
> >>> I'd really like to see a simplified, consolidated format defined and
> >>> deprecate this one, though.
> >>>
> >>
> >> I did explain it before, so here it goes again.
> >>
> >> Although the names are similar (fixups vs local fixups) the operation
> >> performed on them is completely different.
> >>
> >> The fixups are there so that we can resolve symbolic names to phandles, while
> >> the local fixups are there to track where local phandles exist in order to
> >> adjust them to be valid when the overlay is applied.
> >>
> >> So:
> >>
> >> fixups -> symbolic name to phandle
> >> local fixups -> local phandle locations to be adjusted on resolution
> >
> > I get that distinction, but that's not the point. While the method of
> > adjusting the phandle value is different, both types are a correction
> > to a single phandle value at a specific offset in a specific property.
> >
> > In both cases you need to convey the location of a specific phandle
> > value, but that's encoded in a completely different way in each case.
> > That's what bothers me.
> >
>
> It might be possible for the format to change but it’s just not going to be the same.
Not identical, no, but it could be much more similar than it is. For
example:
/{
fragment@0 {
target = <&somewhere>;
__overlay__ = {
dev0: dev@0 {
link = <&devx>;
};
dev@1 {
link = <&dev0>;
};
};
};
__fixups__ = {
somewhere = "/fragment@0:target:0";
devx = "/fragment@0/__overlay__/dev@0:link:0";
local,phandles = "/fragment@0/__overlay__/dev@1:link:0";
};
};
(',' is not a valid character for labels, so this is unambiguous).
> There’s no place to put the label reference in the fixup form so no code-reuse is
> going to happen.
I don't follow what you mean by this.
> >>>> +There is an alternative syntax to the expanded form for overlays with phandle
> >>>> +targets which makes the format similar to the one using in .dtsi include files.
> >>>> +
> >>>> +So for the &ocp target example above one can simply write:
> >>>> +
> >>>> +/dts-v1/ /plugin/;
> >>>> +&ocp {
> >>>> + /* bar peripheral */
> >>>> + bar {
> >>>> + compatible = "corp,bar";
> >>>> + ... /* various properties and child nodes */
> >>>> + }
> >>>> +};
> >>>> +
> >>>> +The resulting dtb object is identical.
> >>>
> >>
> >> Regards
> >>
> >> — Pantelis
> >>
> >
>
> 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
