Re: [PATCH v4 19/21] drivers/of: Support adding sub-tree

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Thu, 2015-05-14 at 10:04 +0300, Pantelis Antoniou wrote:

> Hmm, since you just want to transmit a whole subtree things are a bit
> simpler.
> 
> You don’t need any of the fixups, and your target node is known.
> 
> So your overlay is simply:
> 
> / {
> 	fragment@0 {
> 		target-path = “/foo”;
> 		__overlay__ {
> 			/* contents of the slot */
> 		};
> 	}; 
> };
>
> I think it’s possible to just bit-mangle a blob (in pseudo code).
> 
> 	const u8 template_overlay_blob[] = { <compiled blob of the above> };
> 
> 	flatten_slot(slot_blob);
> 
> 	overlay_blob = allocate_new_blob(template_overlay_blob, slot_blob);
> 
> 	overlay_node = find_node(overlay_blob, “/fragment@0/__overlay__);
> 	target_prop = find_prop(overlay_blob, “/fragment@0/target-path”);
> 
> 	inject_slot_blob(overlay_blob, overlay_node, slot_blob);
> 	modify_slot_target(overlay_blob, target_prop, slot_target);
> 	
> I don’t think you need to re-flatten anything, shuffling bits around with
> memmove should work.

Fairly gross :-)

But yeah generating the overlay doesn't necessarily scare me, I can
generate a temp tree that is the overlay in which I "copy" the subtree
(or in my internal ptr-based representation I could have a concept of
alias which I follow while flattening).

That leaves me with these problems:

 - No support for removing of nodes, so that needs to be added back to
the format and to Linux unless I continue removing by hand in the PCI
hotplug code itself

 - No support for "committing" the overlay which needs to be added as
well.

> >>> Now we could consider that subtree as a changeset that can be undone,
> >>> but that wouldn't work for boot time. And subsequent updates wouldn't
> >>> have that concept of "undoing" anyway.
> >>> 
> >> 
> >> I have posted another patch that does boot-time DT quirk which are
> >> non-revertable.
> >> 
> >> https://lkml.org/lkml/2015/2/18/258
> > 
> > Not sure how that applies in my case ... I can't change the
> > representation of the PCI subtree, this is standard OFW representation,
> > I can't change the FW to make it an overlay-like thing at boot time,
> > that would break existing kernels.
> > 
> 
> The idea is to append the ‘quirk’ to the already booting device tree blob.

I know but that's not how things work for me. At boot time the FW passes
me one tree that contains all the PCI stuff it has probed.

> Another idea floating around was to simple concatenate the booting blob with
> any overlay blobs you want applied at boot time.

Sure but I don't get overlay blobs at boot time.

> >>> IE. conceptually, what overlays do today is quite rooted around the idea
> >>> of having a fixed "base" DT and some pre-compiled DTB overlays that
> >>> get added/removed. The design completely ignore the idea of a FW that
> >>> maintains a "live" tree which we want to keep in sync, which is what we
> >>> want to do here, or what we could do with a "live" open firmware
> >>> implementation.
> >>> 
> >>> Now we might be able to reconcile them, but it feels to me that the
> >>> overlay/changeset stuff is too rooted in the first concept…
> >>> 
> >> 
> >> The first DT overlays use case (beaglebone capes) is what got the concept
> >> started.
> >> 
> >> Right now is a generic mechanism to apply modifications to the kernel
> >> live tree, with the possibility to revert them.
> > 
> > Yes but as I said it's not really thought in term of keeping the kernel
> > tree in sync with an external dynamically generated tree. Maybe we can
> > fix it, but it's more complex…
> > 
> 
> Yes it is, unfortunately.

Right. Which makes the solution of just passing my bit of tree as a blob
which I expand in Linux where I want it rather than an overlay tempting
if we can make Gavin patch more palatable (removing the hybrid stuff
etc...).

Cheers,
Ben.

> > Ben.
> > 
> >>> Ben.
> >>> 
> >>> 
> >> 
> >> Regards
> >> 
> >> — Pantelis
> > 
> > 
> 
> Regards
> 
> — Pantelis