[PATCH] powerpc: Check flat device tree version at boot

[Rob Herring]

On Fri, Aug 29, 2014 at 6:13 AM, Grant Likely <grant.likely at linaro.org> wrote:
> On 29 Aug 2014 02:56, "Michael Ellerman" <mpe at ellerman.id.au> wrote:
>>
>> On Thu, 2014-08-28 at 09:27 -0500, Rob Herring wrote:
>> > On Thu, Aug 28, 2014 at 3:40 AM, Michael Ellerman <mpe at ellerman.id.au> wrote:
>> > > In commit e6a6928c3ea1 "of/fdt: Convert FDT functions to use libfdt",
>> > > the kernel stopped supporting old flat device tree formats. The minimum
>> > > supported version is now 0x10.
>> >
>> > Ugg. Is that something which needs to be supported? Supporting it at
>> > this point would mean adding support to libfdt.
>>
>> Well it would have been nice to keep supporting v2, dropping it broke
>> kexec-tools for example. But it's done now, so we'll just deal with the
>> fallout.
>
> Umm, so we broke userspace? That's not okay. At the very least we
> should investigate how much work is needed to bring v2 support into
> libfdt, or up-rev the flat tree at runtime before we parse it.

Would up-reving work? kexec-tools is broken or booting a new kernel
with kexec is broken?

> We should be able to update it in-line. IIRC, the main difference
> between v2 and v0x10 is that only the leaf node name is encoded into
> the node instead of the full name. We can loop over the tree and
> truncate all the names while filling the unused bytes with FDT NOP
> tags. Should be simple. Something like the following:

There is also the name property in v1-v3. I'm guessing linux ignores
that already?

>
> fixup_old_device_tree(blob)
> {
>        for (offset = fdt_next_node(blob, -1, &depth);
>              offset >= 0 && depth >= 0 && !rc;
>              offset = fdt_next_node(blob, offset, &depth)) {
>                 char *pathp = fdt_get_name(blob, offset, NULL);
>                 if (*pathp == '/') {
>                         char *leaf = kbasename(pathp);
>                         int len = strlen(pathp);
>                         int newlen = strlen(leaf);
>                         strcpy(pathp, leaf); /* copying in place, need
> to check make sure this code is safe */
>                         node->size = newlen /* fixme - this is just
> pseudocode */
>                         newlen = FDT_TAGALIGN(newlen);
>                         while (newlen < len) {
>                                 *(pathp + newlen) = FDT_NOP;
>                                 newlen = FDT_TAGALIGN(newlen+1);
>                         }
>                 }
>        }
>
> }
>
> There's probable some more elegance that can be put into the above
> block, but you get the idea. That could be run in-place without
> copying the tree to another location, and it could possibly be done in
> the boot wrapper. Then we'd also be able to get rid of the v2
> compatibility code elsewhere in drivers/of/fdt.c because it would
> already be taken care of.

That's what got us into this problem. ;)

Rob