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

Wed Mar 13 18:19:17 UTC 2019

On Wed, Mar 13, 2019 at 08:44:45AM +0100, Heiko Schocher wrote:
> Hello Wolfgang,
> 
> Am 12.03.2019 um 18:46 schrieb Wolfgang Denk:
> >Dear Tom,
> >
> >In message <20190312173125.GP4690 at bill-the-cat> you wrote:
> >>
> >>>I think you were misled by Heiko's description.  What he really ment
> >>>was just that the addresses where the boot ROM stored the
> >>>information about the boot device etc. gets overwriteen when the SPL
> >>
> >>For clarity, that's not _quite_ it.  The ROM passes the value in a
> >>register and we move that to scratch space, see
> >>arch/arm/mach-omap2/lowlevel_init.S and save_boot_params.  This is done
> >>on every 32bit Cortex-A TI platform.
> >...
> >>OK, but here's the problem.  We define off, iirc, 1KiB of that SRAM
> >>space as not-stack but scratch space to store stuff in.  The first
> >>problem you will hit, if something else touches that scratch space is
> >>what Heiko found, the boot value got blown away.
> >
> >I see.  Well, I think it's best if Heiko explains in detail; what he
> >has observed, and what when which part of the information got lost.
> >I was just interpreting his mail, so I may easily have misunderstood
> >this.
> >
> >@ Heiko:  Can you please elucidate?
> 
> arch/arm/include/asm/arch-am33xx/omap.h
> 
>  19 #ifdef CONFIG_AM33XX
>  20 #define NON_SECURE_SRAM_START   0x402F0400
>  21 #define NON_SECURE_SRAM_END     0x40310000
>  22 #define NON_SECURE_SRAM_IMG_END 0x4030B800
>  23 #elif defined(CONFIG_TI816X) || defined(CONFIG_TI814X)
>  24 #define NON_SECURE_SRAM_START   0x40300000
>  25 #define NON_SECURE_SRAM_END     0x40320000
>  26 #define NON_SECURE_SRAM_IMG_END 0x4031B800
>  27 #elif defined(CONFIG_AM43XX)
>  28 #define NON_SECURE_SRAM_START   0x402F0400
>  29 #define NON_SECURE_SRAM_END     0x40340000
>  30 #define NON_SECURE_SRAM_IMG_END 0x40337DE0
>  31 #define QSPI_BASE              0x47900000
>  32 #endif
>  33 #define SRAM_SCRATCH_SPACE_ADDR (NON_SECURE_SRAM_IMG_END - SZ_1K)
>  34

For reference, the AM437x TRM is at https://www.ti.com/lit/pdf/spruhl7
and as of this writing is at rev H and section 5.2.3 "Memory Map" under
section 5 which is "Initialization" is helpful to understand these
values.  The AM335x TRM is at https://www.ti.com/lit/pdf/spruh73 and
also helpful to understand how and why things are written how they are.
The other parts of this family have similar chapters and breakdowns.

So, lets comment on these, with the TRM too.  We can see from the table
that we use 0x402F0400 for NON_SECURE_SRAM_START as it's the best value
for all silicon versions as we must not touch that HS area.

Lets also take an aside to note that while the AM335x TRM does not note
that some areas are reserved for ROM the AM437x TRM does and it's a good
assumption that it's better documenting things that are likely true on
older SoCs.

We say that 0x40340000 is NON_SECURE_SRAM_END is the top of SRAM (as the
HS area is below the non-HS area).  Finally we say 0x4031B800 is
NON_SECURE_SRAM_IMG_END as that is the top of "Downloaded image" area.

Finally, we put the scratch space into the download area and give
ourselves a "generous" 1K for future use.  This is because of either
concerns or actual problems with using space elsewhere in SRAM and being
able to access it later.  Or just to try and separate out the area
between stack and this scratch space as much as possible.

> and with ./include/configs/ti_armv7_common.h
> 
>  69 #ifndef CONFIG_SYS_INIT_SP_ADDR
>  70 #define CONFIG_SYS_INIT_SP_ADDR         (NON_SECURE_SRAM_END - \
>  71                                                 GENERATED_GBL_DATA_SIZE)
>  72 #endif

Here's where some troubles now start.  As this was written versus the
am335x TRM, I decided that "public stack", which those TRMs did not say
was "reserved for ROM use", along with anything else outside the
download area was fair game.  We cannot download anything larger than
that otherwise the ROM fails.  This is why I put the stack pointer
there.  One could argue that's wrong now, given the guidance of the
AM437x TRM.

>  73
> 
> include/generated/generic-asm-offsets.h
> 
>   9 #define GENERATED_GBL_DATA_SIZE 224 /* (sizeof(struct global_data) + 15) & ~15  @ */
>  10 #define GENERATED_BD_INFO_SIZE 112 /* (sizeof(struct bd_info) + 15) & ~15       @ */
>  11 #define GD_SIZE 224 /* sizeof(struct global_data)       @ */
> 
> 
> -> CONFIG_SYS_INIT_SP_ADDR = 0x40340000 - 0xe0  = 0x4033ff20
> -> SRAM_SCRATCH_SPACE_ADDR = 0x40337DE0 - 0x400 = 0x403379e0
> 
> ./arch/arm/include/asm/omap_common.h:
> 816 #define OMAP_SRAM_SCRATCH_BOOT_PARAMS   (SRAM_SCRATCH_SPACE_ADDR + 0x24)
> 
> OMAP_SRAM_SCRATCH_BOOT_PARAMS = 0x40337a04
> 
> So stack is on a higher address than the scratch space. Stack
> addresses decrement on usage, so may they overwrite scratch
> space, as SPL functionality grows more and more ...

Yes, there is a trade-off between image features and space constraints.

> Hmm... I see, the NON_SECURE_SRAM_IMG_END is fix defined, and we cannot
> move it.
> 
> Ok, looking on my own now on the hardware:
> 
> => md 40337a04
> 40337a04: 40338dc4 8f943b1e 8138beca 4ea1b2c2    ..3 at .;....8....
>           ^
>           pointer to bootparms struct
> 
> => md.b 40338dc4
> 40338dc4: ff ff ff ff 08 8f 33 40 07 01 00 00 00 00 00 00    ......3 at ........
>                                   ^^
>                                   bootmode 0x07 -> mmc0
> 
> Nothing overwritten here! Puuh...
> 
> But I have a bad feeling reading the bootmode again from U-Boot, as
> the boot_params info is may in space, where stack can overwrite it ...

Now, here's where you at least have a possibly easy answer.  AM437x has
the most SRAM available in the "download image" area, so you could
indeed move the stack pointer to be below the scratch area, and checks
so that we know that we've reserved 8KiB (or something) for stack in
there too.  The instructions in doc/README.SPL for estimating stack
usage are still correct I'm pretty sure.  The problems start to come on
AM335x where non-HS has 110KiB and HS has 46KiB.

> >>I agree here.  Fixing things up such that we can pass things we know
> >>=66rom one stage to another in a defined manner, rather than ad-hoc
> >>manner, is good.  We could even, probably, re-work most of that use of
> >>scratch space to be done in another way, or make it safe to re-use
> >>later.
> >
> >Thanks a lot!  Let's go for it.
> 
> To be safe here, we should really pass the bootmode (or more common,
> the infos collected already in GD) from SPL to U-Boot ...

I don't object here, but you're going to run into memory constraints I
strongly suspect if we want to use bloblist and there's some safety
checking needed to make sure that if we pass GD along that it's a valid
thing and not garbage in a register.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190313/c265c406/attachment.sig>