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

[Frank Rowand]

On 08/16/17 08:09, Tom Rini wrote:
> On Tue, Aug 15, 2017 at 08:22:01PM -0700, Frank Rowand wrote:
>> On 08/15/17 17:42, Tom Rini wrote:
>>> On Tue, Aug 15, 2017 at 04:50:40PM -0700, Frank Rowand wrote:
>>>> On 08/15/17 14:15, Tom Rini 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,
>>>>> 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.
>>>>
>>>> I also was hoping that other people might have ideas for additional
>>>> approaches.
>>>
>>> Yes, please.
>>>
>>>>> 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.
>>>>
>>>> I hope that dtb contents are independent of CONFIG options, but I don't
>>>> feel confident is stating that there is not such dependency.  (Of course,
>>>> whether to build a dtb can be dependent on a CONFIG option in the Makefile,
>>>> but that is not the same concept.)
>>>>
>>>> The only existing rule that I am aware of that helps avoid a dts dependency
>>>> on kernel CONFIG options is that included files can not be from general kernel
>>>> header files; they must be in include/dt-bindings/.
>>>
>>> I'm fairly certain for in-kernel stuff at least, the assumption is
>>> correct.
>>>
>>>> Should we add text to Documentation/devicetree/bindings/submitting-patches.txt
>>>> that explicitly states that dts files are not allowed to contain any
>>>> dependency on kernel CONFIG options?
>>>
>>> Certainly can't hurt.
>>>
>>>>> Second, I think this is out of step with how a lot of overlay usage will
>>>>> occur.  My thinking is that with maximally useful overlays being
>>>>> available in mainline, lots of use-cases that we have today that result
>>>>> in a number of DTS files being included can become just overlays.  This
>>>>
>>>> I disagree with this.  My _opinion_ is that overlays should be the exception,
>>>> not the common case.  Overlays require extra complexity in the various
>>>> subsystems that interact with device trees.  For an overlay to work, these
>>>> subsystems must be able to react to changes made to the device tree by
>>>> an overlay.  The current mechanism is via notifiers, which only exist
>>>> for a few subsystems.
>>>
>>> Ah.  Now, I can't blame you for thinking with your kernel hat on, but,
>>> you're thinking with your kernel hat on :)  (And taking mine off for a
>>> minute is why I changed my mind between when we talked on IRC, and what
>>> I posted).  Kernel run-time apply an overlay has various use cases that
>>> I don't want to discount, but don't want to try and go in detail on
>>> either.
>>>
>>> At heart, one of the issues here is that the Linux kernel is the
>>> authoritative source of dts and dtb files.  Assembling a dtb and N
>>> overlays at some point prior to booting Linux, in order to give it a
>>> complete and valid system is going to be a common case.  Even setting
>>
>> Yes.  When discussing overlay issues and technologies we should be very
>> clear about whether the context is post-boot run time loading of an
>> overlay or using overlays applied on top of a base dtb to create a
>> dtb to be fed to the kernel for booting.  These are very different
>> domains.
>>
>> For the case of constructing a boot time dtb from a base dtb and one
>> or more overlays my primary concerns are related to the complexity
>> (and again fragility) of the approach.  It is already quite difficult
>> to read source dts files and dts include files and determine what the
>> final dtb will contain.  This has been the motivation for several of
>> my debugging tools.  Adding overlays to the mix will increase the
>> complexity of understanding the final product (dtb), where the
>> individual items originated, and what component to modify to result
>> in a change to the final product.  It will add the further challenge
>> that applying overlays in different orders may result in different
>> final dtbs (depending on how strict the rules for applying an overlay
>> are).  I agree that this technology has some valuable upsides and
>> use cases, but I also fear the negative impacts.  The trick lies
>> in finding a way to meet the technology needs in a manner that
>> successfully balances the costs and the benefits.
>>
>> I mentioned my concerns about the robustness of overlays applied to
>> a running Linux system in another reply.  It seems to me that
>> applying overlays pre-boot is more robust as far as the driver and
>> subsystem initialization is concerned, because the normal boot
>> path will be used to process the resulting dtb instead of using
>> an overlay loading path in the kernel.
>>
>>
>>> aside all of the hobbyist board families out there, SoMs are a big
>>> thing.  Eval modules are a big thing.  This turns not just enabling
>>> these as a vendor but using them as a developer into a much less error
>>> prone system.
>>
>> If I understand the eval modules concept, then using overlays to
>> develop and test device tree changes is somewhat analogous to using
>> kernel modules to develop and test drivers.  Yes, this may make
>> some device tree source development more efficient.  And in other
>> cases may actually make it less efficient.  I don't see the impact
>> as being very large one direction or the other.
>>
>>
>>> To try and re-state my rational, if the Linux kernel needs some
>>> safe-guards or other mechanisms to restrict what can be done on top of
>>> OF_OVERLAY (which is not widely enabled in mainline), OK, that's
>>> certainly a discussion to have and think about implications of.  But,
>>> the Linux kernel is the producer of most dtbs that will be consumed by a
>>> variety of platforms.  I feel it needs to generate the dtb that can be
>>> used for all consumers, since it is the source of these resources.
>>
>> This has gone far off topic from the original issue you brought up,
>> which is how, in the context of the Linux kernel build environment, to
>> enable building a base dtb that contains the symbols that are needed to
>> load an overlay.  Can we return to the base topic?  And if anyone wants
>> to discuss the other issues start a new thread?
> 
> Well, how about this?  There certainly should be some discussion about
> how in the context of the Linux kernel run-time, the various impacts of
> overlays need to be handled.  But for out of kernel pre-boot cases, I
> believe __symbols__ always is the only answer that won't bring everyone
> more pain down the line.  What needs to happen before we can get to that
> point, since the Linux kernel is the gate-keeper to how dtbs are built?

And I believe the opposite.  Symbols should only be included in a dtb if
there is an expectation they will be useful for that specific dtb.  I
see overlays as being useful in a minority of cases, not the majority.