[PATCH 1/2] bloblist: Introduce BLOBLIST_PRIOR_STAGE options

[Tom Rini]

On Thu, Dec 12, 2024 at 07:08:34PM +0200, Ilias Apalodimas wrote:
> On Thu, 12 Dec 2024 at 19:02, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Thu, Dec 12, 2024 at 06:34:44PM +0200, Ilias Apalodimas wrote:
> > > Hi Tom,
> > >
> > > On Thu, Dec 12, 2024 at 09:11:41AM -0600, Tom Rini wrote:
> > > > Introduce an option to control if we expect that a prior stage has
> > > > created a bloblist already and thus it is safe to scan the address. We
> > > > need to have this be configurable because we do not (and cannot) know if
> > > > the address at CONFIG_BLOBLIST_ADDR is in regular DRAM or some special
> > > > on-chip memory and so it may or may not be safe to read from this
> > > > address this early.
> > >
> > > Yes this patch makes instead of making the OF_SEPARATE implicitly take
> > > priority
> > >
> > > >
> > > > Signed-off-by: Tom Rini <trini at konsulko.com>
> > > > ---
> > > > Cc: Simon Glass <sjg at chromium.org>
> > > > ---
> > > >  common/Kconfig | 32 ++++++++++++++++++++++++++++++++
> > > >  lib/fdtdec.c   | 11 +++--------
> > > >  2 files changed, 35 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/common/Kconfig b/common/Kconfig
> > > > index e8d89bf6eb9d..e8febad0f212 100644
> > > > --- a/common/Kconfig
> > > > +++ b/common/Kconfig
> > > > @@ -1077,6 +1077,14 @@ config BLOBLIST_SIZE
> > > >       is set up in the first part of U-Boot to run (TPL, SPL or U-Boot
> > > >       proper), and this sane bloblist is used for subsequent phases.
> > > >
> > > > +config BLOBLIST_PRIOR_STAGE
> > > > +   bool "Bloblist was created in a stage prior to U-Boot"
> > > > +   default y
> > > > +   depends on BLOBLIST_FIXED
> > > > +   help
> > > > +     Select this if the bloblist at CONFIG_BLOBLIST_ADDR was created
> > > > +     before U-Boot is started.
> > > > +
> > > >  config BLOBLIST_SIZE_RELOC
> > > >     hex "Size of bloblist after relocation"
> > > >     default BLOBLIST_SIZE if BLOBLIST_FIXED || BLOBLIST_ALLOC
> > > > @@ -1117,6 +1125,14 @@ config SPL_BLOBLIST_ALLOC
> > > >
> > > >  endchoice
> > > >
> > > > +config SPL_BLOBLIST_PRIOR_STAGE
> > > > +   bool "Bloblist was created in a stage prior to SPL"
> > > > +   default y
> > > > +   depends on SPL_BLOBLIST_FIXED
> > > > +   help
> > > > +     Select this if the bloblist at CONFIG_BLOBLIST_ADDR was created before
> > > > +     U-Boot SPL is started.
> > > > +
> > > >  endif # SPL_BLOBLIST
> > > >
> > > >  if TPL_BLOBLIST
> > > > @@ -1146,6 +1162,14 @@ config TPL_BLOBLIST_ALLOC
> > > >
> > > >  endchoice
> > > >
> > > > +config TPL_BLOBLIST_PRIOR_STAGE
> > > > +   bool "Bloblist was created in a stage prior to TPL"
> > > > +   default y
> > > > +   depends on TPL_BLOBLIST_FIXED
> > > > +   help
> > > > +     Select this if the bloblist at CONFIG_BLOBLIST_ADDR was created before
> > > > +     U-Boot TPL is started.
> > > > +
> > > >  endif # TPL_BLOBLIST
> > > >
> > > >  if VPL_BLOBLIST
> > > > @@ -1175,6 +1199,14 @@ config VPL_BLOBLIST_ALLOC
> > > >
> > > >  endchoice
> > > >
> > > > +config VPL_BLOBLIST_PRIOR_STAGE
> > > > +   bool "Bloblist was created in a stage prior to VPL"
> > > > +   default y
> > > > +   depends on VPL_BLOBLIST_FIXED
> > > > +   help
> > > > +     Select this if the bloblist at CONFIG_BLOBLIST_ADDR was created before
> > > > +     U-Boot VPL is started.
> > > > +
> > > >  endif # VPL_BLOBLIST
> > > >
> > > >  endmenu
> > > > diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> > > > index b06559880296..29bddab4150c 100644
> > > > --- a/lib/fdtdec.c
> > > > +++ b/lib/fdtdec.c
> > > > @@ -1669,15 +1669,10 @@ int fdtdec_setup(void)
> > > >     int ret = -ENOENT;
> > > >
> > > >     /*
> > > > -    * If allowing a bloblist, check that first. There was discussion about
> > > > -    * adding an OF_BLOBLIST Kconfig, but this was rejected.
> > > > -    *
> > > > -    * The necessary test is whether the previous phase passed a bloblist,
> > > > -    * not whether this phase creates one.
> > > > +    * Only scan for a bloblist and then if that bloblist contains a device
> > > > +    * tree if we have been configured to expect one.
> > > >      */
> > > > -   if (CONFIG_IS_ENABLED(BLOBLIST) &&
> > > > -       (xpl_prev_phase() != PHASE_TPL ||
> > > > -        !IS_ENABLED(CONFIG_TPL_BLOBLIST))) {
> > > > +   if (CONFIG_IS_ENABLED(BLOBLIST_PRIOR_STAGE)) {
> > >
> > > Without wider context I am not sure but if that check can fold in
> > > bloblist_maybe_init() and return -X if the config isn't enabled, it's going
> > > to be easier to read
> >
> > Hmm. We'd have:
> >         if (CONFIG_IS_ENABLED(BLOBLIST_PRIOR_STAGE) &&
> >             !(ret = bloblist_maybe_init()) {
> >                 /* We have a bloblist, see if it has FDT */
> 
> I mean move the check inside bloblist_maybe_init()

As we talked about on IRC, no, because the "maybe" in that function
means "find or create". So the expected usage here is that we have
platforms that create the bloblist in SPL (post DRAM init) and use that
with the HANDOFF mechanism to pass information to full U-Boot.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20241212/5d570747/attachment.sig>