Re: [PATCH v4 3/3] dtc: Document the dynamic plugin internals

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Wed, Mar 04, 2015 at 03:16:32PM +0200, Pantelis Antoniou wrote:
> Hi David,
> 
> > On Mar 4, 2015, at 13:29 , David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> > 
> > On Fri, Feb 27, 2015 at 08:55:46PM +0200, 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 | 301 +++++++++++++++++++++++++++++++++++
> >> 1 file changed, 301 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..b5ce9b4
> >> --- /dev/null
> >> +++ b/Documentation/dt-object-internal.txt
> >> @@ -0,0 +1,301 @@
> >> +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. 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.
> > 
> > I really don't understand this requirement.  I mean, obviously it
> > makes sense for the "base" dt, since that can be used as-is, ignoring
> > the overlay stuff.  But the overlay fragments themselves are useless
> > if not properly processed, in which case what's the point of
> > maintaining the same dtb format.
> > 
> > It really seems like you're trying to force out-of-band information
> > into band.  The dt is flexible enough that you *can* do that, but that
> > doesn't mean it's a good idea.
> > 
> 
> It means that every piece of software that understands DT can be used as is
> without any changes to the format.

No, only those pieces of software that process the dtb format, but
don't actually do anything with the tree content continue to work.
Software which wants to actually make sense of the tree will *think*
they understand the format, but be horribly mistaken.

So, apart from dtc itself, what software in the first category did
you have in mind.

> And IMHO all information that is related to DT should be expressed using DT only
> constructs and not out of band data like memory ranges.

You're going to have to back up your HO with better reasons that
you've given so far..

> The existence of out-of-band data is owed to the easier processing of them by
> non-DT aware bootloaders, but that that’s anymore the case.
> 
> > 
> >> +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.
> >> +
> >> +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 {
> >> +		...
> >> +		linux,phandle = <0x00000001>;
> >> +		phandle = <0x00000001>;
> >> +		...
> >> +	};
> >> +	ocp {
> >> +		...
> >> +		linux,phandle = <0x00000002>;
> >> +		phandle = <0x00000002>;
> >> +		...
> >> +	};
> >> +	__symbols__ {
> >> +		res="/res";
> >> +		ocp="/ocp";
> > 
> > 
> > So, the __symbols__ node looks an awful lot like the OF-standard
> > aliases node.  Is there any reason you can't use /aliases for this
> > instead?  (Automatically generating aliases for nodes with a label is
> > a dtc feature I've had vaguely in mind since forever).
> > 
> 
> The point of the aliases node is to add explicitly defined shortcut 
> for locations in the tree, when the symbols node records all the label
> instances. If we were to go with the implicit aliases node fill up,
> should the existing aliases node definition by the user continue? Cause that
> might cause conflicts.

Automatically putting every node label into aliases might cause
problems, yes.  But it's also not clear that making every label
available for use by overlays is a good idea.  What I had in mind was
some way of tagging labels to say that they're exported, meaning they
go into aliases.

> 
> > 
> >> +	};
> >> +};
> >> +
> >> +Notice that all the nodes that had a label have been recorded, and that
> >> +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 labels
> >> +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:
> >> +
> >> +/plugin/;	/* allow undefined label 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 label
> >> +in the foo.dts file.
> >> +
> >> +$ dtc -@ -O dtb -o bar.dtbo -b 0 bar.dts
> >> +$ fdtdump bar.dtbo
> >> +...
> >> +/ {
> >> +	... /* properties */
> >> +	fragment@0 {
> >> +		target = <0xdeadbeef>;
> > 
> > The target property is specific to the overlay format.  So specifying
> > it as a nonsense phandle, which must then be fixed up is silly.  It
> > should be encoded directly as a symbolic name.
> 
> There is nothing specific to the target property for the overlay format per-se.
> The 0xdeadbeef value is used as an easily visually identified value that will
> be fixed up by resolution.

What I mean is that there isn't a previous standard defining the
format of the target property, which means you can format it however
you like.  In which case making it a phandle which will always needing
fixing up is stupid.  Make it be a symbolic name which you can resolve
directly.

> 
> > 
> >> +		__overlay__ {
> >> +			bar {
> >> +				compatible = "corp,bar";
> >> +				... /* various properties and child nodes */
> >> +			}
> >> +		};
> >> +	};
> >> +	__fixups__ {
> >> +	    ocp = "/fragment@0:target:0";
> >> +	};
> >> +};
> >> +
> >> +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 '0xdeadbeef',
> > 
> > 0xdeadbeef is not an illegal phandle value.  Use either 0 or -1 if you
> > need an illegal phandle value.
> 
> 0xdeadbeef is just used to be easily identified. 0 or -1 can easily be
> substituted.
>  
> > 
> >> 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>";
> > 
> > Encoding multiple things into parsed text strings, and encoding
> > numbers into text really doesn't sit well with normal conventions for
> > encoding dt properties.  You can mix \0 terminated strings and binary
> > encoded numbers into a single property if necessary, or use multiple
> > properties.
> > 
> > Worse, this encoding doesn't allow multiple overlay properties to be
> > fixed up to point to the same label.
> > 
> 
> My mistake, the documentation entry is not exactly accurate, there are
> multiple entries by label encoded as multiple zero encoded strings.
> 
> So that’s
> 
> <label> = "<local-full-path>:<property-name>:<offset>”
> 	[, "<local-full-path>:<property-name>:<offset>” …] ;

Ok.  The format still doesn't sit well with DT conventions.

> > 
> >> +<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.
> >> +
> >> +/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 = <0xdeadbeef>;
> >> +		__overlay__ {
> >> +			res_baz {
> >> +				....
> >> +				linux,phandle = <0x00000001>;
> >> +				phandle = <0x00000001>;
> >> +			};
> >> +		};
> >> +	};
> >> +	fragment@1 {
> >> +		target = <0xdeadbeef>;
> >> +		__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. Since phandles are allocated
> >> starting at 1
> > 
> > That's not guaranteed, it's just what the current dtc implementation does.
> > 
> 
> OK, I will remove the reference to the ‘1’ number. It’s only there to illustrate
> the theory of operation.
> 
> >> +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.
> > 
> > The __local_fixups__ has a totally different format from __fixups__
> > for doing a very similar operation.  You should be able to unify the
> > encoding for local and non-local fixups.
> > 
> 
> The __fixups__ and __local_fixups__ are used for completely different operations.
> 
> __local_fixups__ contain pointers in the local tree where phandle references are
> been taken, while __fixups__ contains pointers to the local tree where phandle
> resolution takes place.

> So no, they perform completely different operations.

I'm not sure quite what distinction you're making between "references
are been taken" and "phandle resolution".  But as far as I can tell
both local and non-local fixups involve taking a phandle value in the
local fragment and correcting it in some way, which sound like pretty
similar operations to me.

But even to the extent that they're different, your own description
above says both involve a pointer to the local tree.  But you've
encoded that pointer to the local tree in a completely different way
in the two cases, which is idiotic.

-- 
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: pgpqUFMYIXIKW.pgp
Description: PGP signature


[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux