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