[U-Boot] [PATCH] ARM: crt0: Pass malloc base address

[Albert ARIBAUD]

Hello Simon,

On Wed, 11 Nov 2015 14:49:05 -0700, Simon Glass <sjg at chromium.org>
wrote:
> Hi Fabio,
> 
> On 11 November 2015 at 14:24, Fabio Estevam <festevam at gmail.com>
> wrote:
> > Hi Simon,
> >
> > On Wed, Nov 11, 2015 at 7:08 PM, Simon Glass <sjg at chromium.org>
> > wrote:
> >
> >> That test is intended to avoid setting up simple malloc() if we
> >> plan to use full malloc() in SPL. Of course, full malloc() is set
> >> up a little later (in spl_init()). But we should not need both -
> >> either we use simple malloc() or full malloc().
> >>
> >> But for your board I see:
> >>
> >> $ grep CONFIG_SYS_SPL_MALLOC_SIZE b/mx6sabresd_spl/u-boot.cfg
> >> #define CONFIG_SYS_SPL_MALLOC_SIZE 0x3200000
> >>
> >> So you will not be able to use simple malloc(). I'd suggest calling
> >> spl_init() from board_init_f() if you need malloc() there. But it
> >
> > Yes, I do call spl_init() from board_init_f() prior to calling
> > malloc() and this has been working fine prior to 5ba534d247d418.
> >
> >> presumably needs to be done after you have SDRAM up. So perhaps
> >
> > The trick here is that I need malloc to work prior to have SDRAM
> > configured.
> >
> > On cgtqmx6eval we need to read SPI NOR to determine what kind of DDR
> > we will need to configure.
> >
> > Otavio has sent the SPL support for cgtqmx6eval, but it has not
> > reached U-boot mainline yet.
> >
> > I am reproducing the problem on a mx6sabresd_spl target with the
> > simple example code I sent previously.
> 
> I see. That's not a use case I have seen before.
> 
> I'd suggest changing the #ifdef to always set up early malloc() if
> CONFIG_SYS_MALLOC_F is set. There will of course be a new malloc()
> once spi_init() is called, but that does not matter.
> 
> In your patch, please be careful to add some docs to the README on
> this point (the early malloc() is only there to permit DRAM init,
> etc.). It could get quite confusing...

I'm not sure all details are clear to me so let me chime in on this.

IIUC, what Fabio needs is a working C runtime including malloc, based on
some IRAM rather than SDRAM.

This means there will be two phases where malloc can be used, the usual
one after SDRAM init, and a new one before SDRAM init.

Of course, the amount of memory reserved for the malloc arena cannot
be the same in IRAM as it will be in SDRAM, meaning that the routine
which reserves the arena must handle both cases, by detecting whether
it is running before or after SDRAM init and choosing the right malloc
size macro, or by just bein passed that info from the code responsible
for maintaining the C runtime environment (arch/arm/lib/crt0.C in the
ARM case) -- via the 'chunk_id' method I described earlier in
https://www.mail-archive.com/u-boot@lists.denx.de/msg191898.html

There is a *VERY* *BIG* *PROBLEM* to the whole idea of
malloc-before-SDRAM:

We could manage to transfer the pre-SDRAM-init maloc arena content to
the post-SDRAM-init malloc arena, thus preserving all pre- SDRAM-init
mallocs() across the stack switch.

*BUT*: all (variable-stored) pointers to pre-SDRAM-malloc'ed space will
become dangling pointers, because we have no easy way of telling where
these pointers are and relocating them.

This means that the mallocs() done before SDRAM init are lost and that
al pointers to them /will/ become dangling, including any copies and
derivatives of these; all code which depend these pointers and their
will have to handle the situation, which will prove *quite* complex or,
to be blunter, will prove not actually done, and will prove so through
weird bugs creeping up that we'll take some time to relate to malloc
issues, and to fix.

Of course, we could just decide that any malloc done before SDRAM init
is lost and that user code should deal with it. But I fear it will do so
just as badly as it would handle the "maloc transfer" alternative I just
described. Plus, if the malloc'ed data contains hardware module state
or any non-recoverable-again information, then we *cannot* just drop
it. 

Now, there is a systemic solution: that all code which stores a
malloc-arena-based pointer value in memory call a system-provided
function which, if running pre-SDRAM-init, would record the location of
the pointer in a list; upon C environment switch from pre- to
post-SDRAM contexts, the list would be run through and each pointer
would be 'relocated' from the old to the new maloc arena. Post-init,
the functon would not do any recording. That causes a slight
performance hit on mallocs, but I don't think it'll affect boot time
that much.

Only code which has to run before SDRAM init would have to use the
function.

The benefit of this approach is that maloc'ed space would remain
malloc'ed after SDRAM init and declared pointers to it would remain
valid. Code which has to malloc before SDRAM init would not have to
re-malloc or fix anything, or even realize the stack and malloc arena
have moved, as long as it has declared its malloc-related pointer(s).

Of course the list would be limited. But seeing as we'd be running in a
tight context and only to get SDRAM running, the list would not extend
much.

I imagine a scheme where the list is kept in the malloc arena. The
malloc space itself would still grow 'down' from the stack top while
the pointer list would grow 'up' from the stack 'bottom' limit.

I could whip up an RFC patch (with ARM support, to be extended to other
platforms as my recent experience showed I'm not that good at NIOS2 for
instance) if people are interested.

> Regards,
> Simon

Amicalement,
-- 
Albert.