[PATCH v2 33/35] RFC: Revert "bloblist: Load the bloblist from the previous loader"

Mon Feb 17 15:08:51 CET 2025

On Mon, Feb 17, 2025 at 06:16:12AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Sun, 16 Feb 2025 at 08:01, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Sun, Feb 16, 2025 at 07:10:31AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Sat, 15 Feb 2025 at 11:28, Tom Rini <trini at konsulko.com> wrote:
> > > >
> > > > On Sat, Feb 15, 2025 at 10:24:50AM -0700, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Sat, 15 Feb 2025 at 08:01, Tom Rini <trini at konsulko.com> wrote:
> > > > > >
> > > > > > On Sat, Feb 15, 2025 at 05:11:45AM -0700, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Sat, 15 Feb 2025 at 05:06, Simon Glass <sjg at chromium.org> wrote:
> > > > > > > >
> > > > > > > > Hi Tom,
> > > > > > > >
> > > > > > > > On Tue, 11 Feb 2025 at 06:59, Tom Rini <trini at konsulko.com> wrote:
> > > > > > > > >
> > > > > > > > > On Tue, Feb 11, 2025 at 06:10:54AM -0700, Simon Glass wrote:
> > > > > > > > > > Hi Tom,
> > > > > > > > > >
> > > > > > > > > > On Mon, 10 Feb 2025 at 07:33, Tom Rini <trini at konsulko.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Sun, Feb 09, 2025 at 02:14:54PM -0700, Simon Glass wrote:
> > > > > > > > > > >
> > > > > > > > > > > > The logic of this has become too confusing.
> > > > > > > > > > > >
> > > > > > > > > > > > The primary issue with the patch is that U-Boot needs to set up a
> > > > > > > > > > > > bloblist in the first phase where BLOBLIST is enabled. Subsequent
> > > > > > > > > > > > phases can then use that bloblist.
> > > > > > > > > > > >
> > > > > > > > > > > > But the first phase of U-Boot cannot assume that one exists.
> > > > > > > > > > > >
> > > > > > > > > > > > Reverting this commit seems like a better starting point for getting
> > > > > > > > > > > > things working for all use-cases.
> > > > > > > > > > > >
> > > > > > > > > > > > Note: The work to tidy this up is apparently underway. For this series,
> > > > > > > > > > > > a revert is the easiest path.
> > > > > > > > > > > >
> > > > > > > > > > > > This reverts commit 66131310d8ff1ba228f989b41bd8812f43be41c3.
> > > > > > > > > > > >
> > > > > > > > > > > > https://lore.kernel.org/u-boot/CAPnjgZ3hMHtiH=f5ZKXNniOfV_-vFryq1Gn7QZ5hKU8Wjo8igw@mail.gmail.com/
> > > > > > > > > > > >
> > > > > > > > > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > > > > > > > > ---
> > > > > > > > > > > >
> > > > > > > > > > > > (no changes since v1)
> > > > > > > > > > > >
> > > > > > > > > > > >  common/bloblist.c  | 64 ++++++++++++++--------------------------------
> > > > > > > > > > > >  include/bloblist.h | 10 --------
> > > > > > > > > > > >  2 files changed, 19 insertions(+), 55 deletions(-)
> > > > > > > > > > >
> > > > > > > > > > > We aren't reverting this change so if you plan to move this series out of
> > > > > > > > > > > your downstream fork you should start from that and stop posting it,
> > > > > > > > > > > that just leads to confusion.
> > > > > > > > > >
> > > > > > > > > > Who is 'we', and start from what?
> > > > > > > > >
> > > > > > > > > We, being the community, in here, the community mainline U-Boot project
> > > > > > > > > tree.
> > > > > > > > >
> > > > > > > > > > My goal with this series is to have something that actually boots on a
> > > > > > > > > > real board, so the bloblist changes are needed for that, particularly
> > > > > > > > > > as the 'vbe' board in the lab tests this on the hardware. I
> > > > > > > > > > deliberately put these two patches at the end of the series so you can
> > > > > > > > > > ignore them if you'd like.
> > > > > > > > > >
> > > > > > > > > > But for now, as I understand it, there are no users of standard
> > > > > > > > > > passage in tree, so actually it would be fine to apply them.
> > > > > > > > >
> > > > > > > > > And you can ignore all the feedback you like for your downstream fork,
> > > > > > > > > sure.
> > > > > > > > >
> > > > > > > > > Part of the feedback in this series already was "Yes, we can clean this
> > > > > > > > > up a bit more if bloblist will do its own thing instead".
> > > > > > > >
> > > > > > > > So I see that Raymond's patch is a significant rewrite of what is
> > > > > > > > there today, with multiple aims and changes. I would rather add my
> > > > > > > > revert and start from a cleaner place.
> > > > > > >
> > > > > > > Also, I forgot to mention that U-Boot doesn't seem to support the
> > > > > > > firmware handoff protocol in SPL? If so, could you point me to the
> > > > > > > code?
> > > > > >
> > > > > > Yes, you never implemented handoff between stages in U-Boot via
> > > > > > register. That's not a use case for the platforms that don't use U-Boot
> > > > > > for SPL.
> > > > >
> > > > > If you let me own bloblist, I'll own it, but for now I am staying away.
> > > >
> > > > Would you be working against mainline again, and not breaking standard
> > > > passage (as it's been split off) ?
> > >
> > > Sure. The only reason I have my tree is because (I feel that) parts of
> > > U-Boot have become no-go for me. So if you'd like me to maintain
> > > bloblist again, I would be delighted to do so.
> > >
> > > Steps I see:
> > > - Get my boards working again
> >
> > Already done. I'm waiting for Kever to pick up Jonas' small series that
> > brings back output prior to that statement from DM.
> 
> OK good.
> 
> >
> > > - Add standard passage from SPL->PPL
> >
> > By who? PPL (and xPL) already reads standard passage via register.
> 
> I mean implement it in SPL, so it can create the info and pass it to PPL.
> 
> >
> > > - Figure out what to do on x86 to also use standard passage
> >
> > Standard passage, or bloblist?
> 
> Well, bloblist today, but I hope we can come up with a standard
> passage for x86 too.

My problem is "standard passage" has a meaning now, so it would be
confusing to call "standard passage" one thing for arm and another for
x86.

> > > I'm not quite sure that BLOBLIST_FIXED can be removed, as it needs
> > > more thought. The phase that creates the bloblist initially needs to
> > > know where to put it.
> >
> > When a phase is running it already has to know what memory is available
> > right now or not, and can allocate as much of a list as it needs now.
> > Later phases can move/grow it.
> 
> So you mean xPL should use malloc() / simple_malloc() rather than a
> fixed address? Or are you suggesting doing something in
> board_init_f_alloc_reserve()?

My high level point is that a phase must already know what memory it can
and cannot touch, at run time.

-- 
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/20250217/efb7f8fa/attachment.sig>