[coreboot] STACK_SIZE pcengines apu1

[Sheng-Liang Song]

It is better to review the pcengines apu1 boot code--pay attention on any
function that uses >= 64B stack storage
(function arguments and local variables).  Try reduce the stack size of
such functions instead of increasing the STACK_SIZE.

On Thu, Sep 10, 2015 at 7:02 AM, Aaron Durbin <adurbin at google.com> wrote:

> On Sun, Sep 6, 2015 at 9:10 AM, Maxime de Roucy
> <maxime.deroucy at gmail.com> wrote:
> > Hello,
> >
> > I am building a coreboot rom for my pcengines apu1.
> > A bug is logged during the boot process :
> >
> http://review.coreboot.org/gitweb?p=board-status.git;a=blob;f=pcengines/apu1/4.0-9873-g7b9762f/2015-06-04T15:16:28Z/coreboot_console.txt;hb=HEAD#l1281
> >
> > In order to solve it I applied this changed :
> >
> >     diff --git a/src/Kconfig b/src/Kconfig
> >     index 9c01687..c8b8ad2 100644
> >     --- a/src/Kconfig
> >     +++ b/src/Kconfig
> >     @@ -427,7 +427,7 @@ config HEAP_SIZE
> >      config STACK_SIZE
> >             hex
> >             default 0x0 if (ARCH_RAMSTAGE_ARM || ARCH_RAMSTAGE_MIPS ||
> ARCH_RAMSTAGE_RISCV)
> >     -       default 0x1000
> >     +       default 0x2000
> >
> >      config MAX_CPUS
> >             int
> >
> > I don't see the bug line anymore, instead I see :
> >
> >     CPU0: stack: 00148000 - 0014a000, lowest used address 00148d34,
> stack used: 4812 bytes
> >
> > I now the patch is not good since it change the default stack size for
> > all the boards. I didn't found the right place the change the stack
> > size only for pcengines apu1 board.
> > But maybe those informations can help developers to improve coreboot.
>
> That's quite the stack usage.  It'd be interesting to know what's
> using all that stack. Could you apply this patch and run w/ it? It'd
> help narrow down at what point in the boot where the stack gets used
> up. Also, that you used 4812 bytes just means you overwrote the other
> CPU's stack at some point when STACK_SIZE == 4KiB.
>
>
> diff --git a/src/lib/hardwaremain.c b/src/lib/hardwaremain.c
> index b3c0c35..8a241b2 100644
> --- a/src/lib/hardwaremain.c
> +++ b/src/lib/hardwaremain.c
> @@ -378,6 +378,8 @@ static void bs_walk_state_machine(void)
>                 bs_report_time(state);
>
>                 state->complete = 1;
> +
> +               checkstack(_estack, 0);
>         }
>  }
>
>
> > --
> > Regards
> > Maxime de Roucy
> > --
> > coreboot mailing list: coreboot at coreboot.org
> > http://www.coreboot.org/mailman/listinfo/coreboot
>
> --
> coreboot mailing list: coreboot at coreboot.org
> http://www.coreboot.org/mailman/listinfo/coreboot
>



-- 
Sheng-Liang Song | Software Engineer | ssl at google.com | 650-537-5017
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20150910/19e8e0b8/attachment.html>