[PATCH 30/32] fdt: Allow the devicetree to come from a bloblist

[Ilias Apalodimas]

Hi Michal,

On Mon, 11 Sept 2023 at 13:58, Michal Simek <michal.simek at amd.com> wrote:
>
> Hi Ilias,
>
> On 9/11/23 09:56, Ilias Apalodimas wrote:
> > Hi Michal,
> >
> > On Mon, 11 Sept 2023 at 09:38, Michal Simek <michal.simek at amd.com> wrote:
> >>
> >>
> >>
> >> On 9/11/23 08:17, Ilias Apalodimas wrote:
> >>> Hi Simon,
> >>>
> >>> On Sun, 10 Sept 2023 at 22:14, Simon Glass <sjg at chromium.org> wrote:
> >>>>
> >>>> Hi Ilias,
> >>>>
> >>>> On Mon, 4 Sept 2023 at 03:31, Ilias Apalodimas
> >>>> <ilias.apalodimas at linaro.org> wrote:
> >>>>>
> >>>>> Hi Simon,
> >>>>>
> >>>>> On Fri, 1 Sept 2023 at 18:51, Simon Glass <sjg at chromium.org> wrote:
> >>>>>>
> >>>>>> Hi Ilias,
> >>>>>>
> >>>>>> On Fri, 1 Sept 2023 at 01:50, Ilias Apalodimas <ilias.apalodimas at linaro.org> wrote:
> >>>>>>>
> >>>>>>> [...]
> >>>>>>>
> >>>>>>>>>
> >>>>>>>>>> +config OF_BLOBLIST
> >>>>>>>>>> +     bool "DTB is provided by a bloblist"
> >>>>>>>>>> +     help
> >>>>>>>>>> +       Select this to read the devicetree from the bloblist. This allows
> >>>>>>>>>> +       using a bloblist to transfer the devicetree between  U-Boot phases.
> >>>>>>>>>> +       The devicetree is stored in the bloblist by an early phase so that
> >>>>>>>>>> +       U-Boot can read it.
> >>>>>>>>>> +
> >>>>>>>>>
> >>>>>>>>> I dont think this is a good idea.  We used to have 4-5 different options
> >>>>>>>>> here, which we tried to clean up and ended up with two very discrete
> >>>>>>>>> options.  Why do we have to reintroduce a new one?  Doesn't that bloblist
> >>>>>>>>> have a header of some sort?  The bloblist literally comes from a previous
> >>>>>>>>> stage bootloader which is what OF_BOARD is here for. So why can't we just
> >>>>>>>>> read the header and figure out if the magic of the bloblist matches?
> >>>>>>>>
> >>>>>>>> No, OF_BOARD is a hack to allow boards to do what they like with the FDT.
> >>>>>>>>
> >>>>>>>> This patch is a standard mechanism to pass the DT from one firmware
> >>>>>>>> phase to the next. We have spent quite a bit of time creating a spec
> >>>>>>>> for it, and we should use it.
> >>>>>>>
> >>>>>>> Where exactly am I objecting using the spec?   Can you please re-read my email?
> >>>>>>> I am actually pointing out we should use the spec *properly*.  So
> >>>>>>> instead of having a Kconfig option for the DT, which is pretty
> >>>>>>> pointless,  we should parse the bloblist.  If the header defined by
> >>>>>>> the *spec* is found, we should just search for the DT in there.
> >>>>>>> What you are doing here, is take the spec, pick a very specific item
> >>>>>>> that the list contains, and create a Kconfig option out of it.  Which
> >>>>>>> basically ignores the discoverable options of the bloblist.  For
> >>>>>>> example, that bloblist can also contain an entry to a TPM eventlog.
> >>>>>>> Should we start creating Kconfig options for all the firmware handoff
> >>>>>>> entries that are defined on that spec?
> >>>>>>
> >>>>>> OK so that is a different thing. What should it do if it expects to find a bloblist but cannot? I want it to throw an error, because I am trying to make the boot deterministic. What do you think?
> >>>>>
> >>>>> That's fine by me.  You can even put that under IS_ENABLED for the
> >>>>> bloblist inside the existing OF_BOARD check.  So I was thinking
> >>>>> - If no bloblist is required in Kconfig options we do the hacks we used to
> >>>>> - if bloblist is selected and the config option is OF_BOARD, throw an
> >>>>> error and mention that the previous stage loader should hand over a DT
> >>>>>
> >>>>> Is that what you had in mind?
> >>>>
> >>>> Yes, that sounds good to me.
> >>>>
> >>>> But I still think we need an OF_BLOBLIST option to control whether the
> >>>> devicetree comes from there.
> >>>>    Otherwise we will end up with people
> >>>> using OF_BOARD to work around it. We also have the SPL case which does
> >>>> not pass the DT in a bloblist...in fact SPL typically doesn't even
> >>>> have the full DT.
> >>>
> >>> Wouldn't IS_ENABLED(BLOBLIST) || IS_ENABLED(SPL_BLOBLIST) be enough?
> >>> Inside the OF_BOARD portion of the function, search for a bloblist if
> >>> the option is enabled.  If you can't find a bloblist and the DT within
> >>> that bloblist, then error out
> >>
> >> Just my 2c here.
> >> I think all options should be possible to disable. It means I can imagine to
> >> disable u-boot not to take care about DT provided from previous stage. The same
> >> is for TPM event log. IMHO every stage should have an option to simply ignore
> >> data pass from previous stage. I don't really mind how it is done but there
> >> should be an option to by purpose say to ignore the part of pass data.
> >
> > That would be done by disabling BLOBLIST.  I don't think having a Kconfig to say
> > "I want a bloblist, but I only want the DT from there" is reasonable
> > (or for any other item the bloblist can carry).   OF_BOARD means the
> > DT will come from a previous stage loader. If bloblist is enabled then
> > the DT must come from there.
>
> I don't agree on this. If bloblist is enabled and DT is passed SW should have a
> freedom to ignore it.
>
> At start everything is likely in sync but later stages before U-Boot can stay at
> certain version but there could be a need to update u-boot where DT from
> previous stage could be broken.

But you *can* ignore it and load a different one later.  The only
restriction is that if you compile u-boot with the assumption an early
stage bootloader provides a DT you *must* find it.  But we will still
just keep 2 options in U-Boot of how you get a DT.
A previous loader provided it or U-Boot provided it.  Whether that
comes from a bloblist or a register is irrelevant no ?

[...]


Thanks

/Ilias