Hi Michal, On Mon, 11 Sept 2023 at 13:58, Michal Simek <michal.si...@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.si...@amd.com> wrote: > >> > >> > >> > >> On 9/11/23 08:17, Ilias Apalodimas wrote: > >>> Hi Simon, > >>> > >>> On Sun, 10 Sept 2023 at 22:14, Simon Glass <s...@chromium.org> wrote: > >>>> > >>>> Hi Ilias, > >>>> > >>>> On Mon, 4 Sept 2023 at 03:31, Ilias Apalodimas > >>>> <ilias.apalodi...@linaro.org> wrote: > >>>>> > >>>>> Hi Simon, > >>>>> > >>>>> On Fri, 1 Sept 2023 at 18:51, Simon Glass <s...@chromium.org> wrote: > >>>>>> > >>>>>> Hi Ilias, > >>>>>> > >>>>>> On Fri, 1 Sept 2023 at 01:50, Ilias Apalodimas > >>>>>> <ilias.apalodi...@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