[coreboot] [patch] RE: Fam10 breakage

Patrick Georgi patrick at georgi-clan.de
Mon Mar 1 08:39:17 CET 2010


Am 01.03.2010 06:42, schrieb Bao, Zheng:
> It doesn't matter whether it is a huge stack. The _estack - _stack
> should be MAX_CPUS * STACK_SIZE. The MAX_CPU could be set as 4, and
> STACK_SIZE could be 0x2000. So the stack is only 0x8000. Is it huge?
> But checking my coreboot_ram.map, _estack - _stack is only ONE
> STACK_SIZE. But the loader has no idea about that. The stack will
> overlap other section of data. I believe many other build machine
> will produce the same result with me.
The main issue I see is that weird rule to decide whether to use one
stack or many - it doesn't really make sense.

The only place where multiple stacks seem to be setup is
src/cpu/x86/lapic/lapic_cpu_init.c (look for estack in that file). That
place really indicates that the ldscript rule does the wrong thing:
No matter if that weird conditional holds true, multiple stacks are set
up, just in different ways, with special behaviour if the stacks are
split between <1MB and >1MB memory.

So I can only support Myles' proposal to make the linker script line
 . += CONFIG_MAX_CPUS * CONFIG_STACK_SIZE
without any cleverness. I believe that the line as it is in the linker
script is wrong. From looking at the behaviour in the mentioned C file,
it should decide between MAX_CPUS * STACK_SIZE and some more complex
rule that creates a stack at >1MB. Something like:

(I'm not proposing to add this to the linker script!)
. += ((CONFIG_RAMTOP>0x100000) && (CONFIG_RAMBASE < 0x100000) &&
((CONFIG_CONSOLE_VGA==1) || (CONFIG_PCI_ROM_RUN == 1)))?(0x100000+(20480
+ CONFIG_STACK_SIZE)*CONFIG_MAX_CPUS - (CONFIG_STACK_SIZE*index) -
.):(CONFIG_STACK_SIZE*CONFIG_MAX_CPUS);

There are a couple of boards with RAMBASE<1MB and RAMTOP >1MB.
(amd_db800, amd_norwich, artecgroup_dbe61, bcom_winnetp680,
digitallogic_msm800sev, iei_pcisa-lx-800-r10, jetway_j7f24,
lippert_roadrunner-lx, lippert_spacerunner-lx, pcengines_alix1c,
technexion_tim5690, via_epia-cn, via_epia, via_epia-m700, via_epia-m,
via_epia-n, via_pc2500e, via_vt8454c, winent_pl6064)

Only one of them actually has MAX_CPUS > 1, technexion/tim5690, so once
that one is changed to have RAMBASE >= 1MB, we could drop this special
rule without side effect (except for avoiding a linker bug and being
able to simplify the code)


Patrick




More information about the coreboot mailing list