[U-Boot] passing info from SPL to U-Boot

[Simon Glass]

Hi Wolfgang,

On Fri, 15 Mar 2019 at 02:34, Wolfgang Denk <wd at denx.de> wrote:
>
> Dear Simon,
>
> In message <CAPnjgZ14Sq=OWRbM6hpqO2CLP=1eg=ftQw_1Bqu0DNe+6OxTkQ at mail.gmail.com> you wrote:
> >
> > I think it is a reasonable idea to allow the gd region to pass from
> > TPL -> SPL -> U-Boot. But we'll need to remove use of
> > CONFIG_IS_ENABLED(), or put shared things at the beginning of the
> > structure.
>
> Indeed. And/or split things up in "common" stuff and optional /
> config dependent things.
>
> > We need the concept of 'am I the first thing to run'. This is
> > implemented in bloblist as u_boot_first_phase() - see spl.h. If this
> > is true, we must set up the data structure. If false we must find one
> > set up by a previous phase and use it. Bloblist handles this, but
> > perhaps gd could as well?
>
> I wonder why we need 4 different ways of doing basically the same
> thing.
>
> First, we have GD, which exists since the dawn of U-Boot, which was
> intended to pass data between boot stages (by then, before and after
> relocation), but apparently it has never been used for passing
> information between SPL and U-Boot proper.

That's right. It is always zeroed these days at the start of each
phase. So I think work is needed if we want to use this.

Also I don't think we can assume that gd stays in the same place
through all phases of U-Boot. Probably we can keep it in the same
place in TPL, SPL and U-Boot pre-relocation, then move it to SDRAM
during relocation.

>
> Then you added the bloblist thingy.  It's not really clear what it's
> intentions are - I see the commits, but I can't find what you want
> to use it for or what design you have in mind.  It's too complicated
> for passing just a few data, but apparently you find it necessary to
> make it secure enough that you add version, magic and checksum
> (which makes it necessary to have CRC32 in SPL...).  Also, I wonder
> how the search mechanism effects boot time...

This is designed for things that need to past structs between phases.
For example, verified boot may start processing in TPL and then pass
information to SPL and then to U-Boot proper. This may involve quite a
bit of info, so it is in a C structure. It isn't really suitable to
put this entire structure in gd IMO, since that struct is pretty
small.

See doc/README.bloblist:

 A bloblist provides a way to store collections of binary information
(blobs) in
 a central structure. Each record of information is assigned a tag so that its
 owner can find it and update it. Each record is generally described by a C
 structure defined by the code that owns it.

Maybe this should have more detail?

The version, magic and checksum are to ensure that the data is not
corrupted by mistake, which as you know can happen very easily with
fixed-position data structures. The search is pretty quick once the
checksum is done, just running through a few pointers. I suppose we
could make the checksum optional.

>
> An then there is commit b0edea3c27 with the spl_handoff thing.  I
> can't decide whether this is intended as a general feature or a
> separate, SPL specific mechanism.  And more questions - if we pass
> the handoff pointer in GD, why all the effort - why don't we just
> make sure GD is passed properly?  The fact that there is no use case
> at all in mainline U-Boot makes it really hard to understand your
> intentions.

That actually uses the bloblist, putting an SPL struct in there,
intended to hold things like the SDRAM config or boot options
discovered in SPL. At present we have a few ad-hoc ways of

>
> And finally there is bootstage with it's own mechanism of
> information passing.

Yes, and this is ad-hoc too. I would like to move it to bloblist.

>
>
> Can we not unify these, and use one common method, please?

I think we might end up with gd (once this work is done) and bloblist.
For now I feel that bloblist has a purpose but let's see how it goes
with the gd work.

>
>
> > Also consider the scenario where there is a read-only TPL programmed
> > in manufacture that never changes, and a read-write SPL +  U-Boot that
> > can be upgraded in the field. In this case they may eventually end up
> > being built with different versions of U-Boot. The bloblist structure
> > is intended to handle this by at least checking that the size matches.
>
> You also have the version field there, right?  Who not (also)
> checking this?

Yes it should check this, at least once we actually have a version 1 :-)

>
> > Related, I feel that we should figure out how to use registers to pass
> > addresses from SPL to U-Boot. On ARM we could use r0 to pass the value
> > of gd, perhaps.
>
> There is no need to.  GD already has a well-defined register which
> has been reserved exclusively for this use - on ARM, it's R9.

Yes, but I'm not sure this is portable. E.g. on x86 this can't be used
in 64-bit mode, so passing info from 32-bit SPL to 64-bit U-Boot is
not possible (I believe). Using a normal register might be better, but
in any case this is going to be arch-specific.

In any case, I was not even aware that gd was intended to pass info
from SPL to U-Boot. It is possible that in my refactoring of
board_init() some years ago I actually broken this. Once we get it
running again we should add some tests and docs.

Regards,
Simon