Re: [PATCH] devicetree: Enable generation of __symbols__ in all dtb files

[Rob Herring]

On Tue, Aug 15, 2017 at 5:49 PM, Tom Rini <trini@xxxxxxxxxxxx> wrote:
> On Tue, Aug 15, 2017 at 05:36:11PM -0500, Rob Herring wrote:
>> On Tue, Aug 15, 2017 at 4:15 PM, Tom Rini <trini@xxxxxxxxxxxx> wrote:
>> > With support for stacked overlays being part of libfdt it is now
>> > possible and likely that overlays which require __symbols__ will be
>> > applied to the dtb files generated by the kernel.  This is done by
>> > passing -@ to dtc.  This does increase the filesize (and resident memory
>> > usage) based on the number of __symbol__ entries added to match the
>> > contents of the dts.
>> >
>> > Cc: Rob Herring <robh+dt@xxxxxxxxxx>
>> > Cc: Frank Rowand <frowand.list@xxxxxxxxx>
>> > Cc: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
>> > Cc: Michal Marek <mmarek@xxxxxxxx>
>> > Cc: Pantelis Antoniou <pantelis.antoniou@xxxxxxxxxxxx>
>> > Cc: devicetree@xxxxxxxxxxxxxxx
>> > Cc: linux-kernel@xxxxxxxxxxxxxxx
>> > CC: linux-kbuild@xxxxxxxxxxxxxxx
>> > Signed-off-by: Tom Rini <trini@xxxxxxxxxxxx>
>> > ---
>> > In order for a dtb file to be useful with all types of overlays, it
>> > needs to be generated with the -@ flag passed to dtc so that __symbols__
>> > are generated.  This however is not free, and increases the resulting
>> > dtb file by up to approximately 50% today.  In the current worst case
>> > this is moving from 88KiB to 133KiB.  In talking with Frank about this,
>>
>> Plus some amount for the unflattened tree in memory, too.
>>
>> > he outlined 3 possible ways (with the 4th option of something else
>> > entirely).
>> >
>> > 1. Make passing -@ to dtc be dependent upon some CONFIG symbol.
>> > 2. In the kernel, if the kernel does not have overlay support, discard
>> > the __symbols__ information that we've been passed.
>> > 3. Have the bootloader pass in, or not, __symbols__ information.
>> >
>> > This patch is an attempt to implement something between the 3rd option
>> > and a different, 4th option.  Frank was thinking that we might introduce
>> > a new symbol to control generation of __symbol__ information for option
>> > 1.  I think this gets the usage backwards and will lead to confusion
>> > among users and developers.
>> >
>> > My proposal is that we do not want __symbols__ existence to be dependent
>> > on some part of the kernel configuration for a number of reasons.
>> > First, this is out of step with the rest of how dtbs are created today
>> > and more importantly, thought about.  Today, all dtb content is
>> > independent of CONFIG options.  If you build a dtb from a given kernel
>> > tree, everyone will agree on the result.  This is part of the "contract"
>> > on passing old kernels and new dtb files even.
>>
>> Agree completely. I don't even like that building dtbs depends on the ARCH.
>>
>> However, option 2 may still be useful. There's no point exposing what
>> can't be used. Furthermore, exposing __symbols__ in /proc/device-tree
>> at all may be a bad idea. We should consider if it should always be
>> hidden. That would also allow storing the __symbols__ data however we
>> want internally (i.e. with less memory usage). The complication is
>> always kexec which I haven't thought about too much here.
>
> A further patch to the kernel at run-time, OK.  If you give me some
> crumbs I'll see if I can figure out the next steps.
>
>> Also, perhaps we need finer grain control of __symbols__ generation.
>
> Here I have to disagree.
>
>> We really don't want userspace to be able to modify anything in the DT
>> at any point in time. That's a big can of worms and we don't want to
>> start there. The problem is labels are widely used just for
>> convenience and weren't part of the ABI. With overlays that changes,
>> so we either need to restrict labels usage or define another way. It
>> could be as simple as defining some prefix for label names for labels
>> to export.
>
> I think there needs to be a difference noted between "here is what
> policy the kernel is going to enforce about run time changes" and "here
> is what the user is going to assemble a system to look like".  Again,
> stemming from the part where the Linux kernel is where dts files reside
> and are generated from normally.  If we have it in __symbols__, someone
> can make use of it in hardware design (again, think of the SoM + carrier
> + custom) bit, I've seen so many real life products now that would be
> simplified in this manner).

I agree the usecase is an important one and one we should target, but
I think there are other issues to solve first before we get to the
trivial change needing to enable __symbols__. Do we have any dts files
actually structured for the SoM + carrier use case? I guess it's done
with includes ATM if we do. The run-time restrictions aren't just
kernel policy. The SoM itself is going to have restrictions defined by
its pinout. I think those need to be described in DT via a connector
binding. I worry about leaving things wide open and having overlays
just be a DT configuration tool with every platform structuring things
however they want. From what I've looked at on RPi, I'm very concerned
about having things like CMA overlays to set the CMA size. (On the
flip side as a user, it was very nice to just apply the RPi 1-wire
gpio overlay and things just worked.)

Rob