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?
Thanks
/Ilias
>
> >
> > Thanks
> > /Ilias
> >
> > >
> > > The patches to align bloblist with the spec have been sent, but there
> > > is a late-breaking change that we are trying to resolve. Once that is
> > > sorted out, I will send v2 of those patches.
> > >
> > > >
> > > > > config OF_BOARD
> > > > > bool "Provided by the board (e.g a previous loader) at runtime"
> > > > > default y if SANDBOX || OF_HAS_PRIOR_STAGE
> > > > > diff --git a/include/bloblist.h b/include/bloblist.h
> > > > > index 080cc46a126..e16d122f4fb 100644
> > > > > --- a/include/bloblist.h
> > > > > +++ b/include/bloblist.h
> > > > > @@ -103,6 +103,11 @@ enum bloblist_tag_t {
> > > > > BLOBLISTT_ACPI_TABLES = 0x104, /* ACPI tables for x86 */
> > > > > BLOBLISTT_SMBIOS_TABLES = 0x105, /* SMBIOS tables for x86 */
> > > > > BLOBLISTT_VBOOT_CTX = 0x106, /* Chromium OS verified boot
> > > > > context */
> > > > > + /*
> > > >
> > > > [...]
> > > >
> > > > >
> > > > [0] https://github.com/FirmwareHandoff/firmware_handoff
> > >
>
> Regards,
> Simon
