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. > > Regarding DT part. We are experimenting with TF-A injecting for example DDR > memory reservation to DT. > Injection is working properly if you are using single DT but in SOM case we > are > using FIT image with a lot of DTBs inside (issue with checksums calculation). > I see couple of platforms (IIRC renesas/imx) which are using DT overlays and > passing it via specific registers. For these using bloblist might be right way > to go. Yes that's in fact the purpose of that spec. > I wasn't aware about the whole fdt bloblist discussion but based on my > experiments you can create multiple FDT entries that's why I expect that there > could be DT overlays from different stages. And I even think that all of them > can be overlays without base DT which can be select later on. If the bloblist has overlays, then we shouldn't be compiling with OF_BOARD. You will get a DT from u-boot and tghe adding this overlays, is an matter of parsing the bloblist and appending the, Thanks /Ilias > > Thanks, > Michal > >
