What I keep trying to make everyone understand is not what the rules we should use to decide the stack size. What I worry is the bug in the crosstool will make the rule do the wrong thing, even if the rule is perfect. So far, no one seems to support me that there is a bug in the toolchain. I admit it seems ridiculous But the it is quite clear.
Zheng
-----Original Message----- From: coreboot-bounces@coreboot.org [mailto:coreboot-bounces@coreboot.org] On Behalf Of Patrick Georgi Sent: Monday, March 01, 2010 3:39 PM To: coreboot@coreboot.org Subject: Re: [coreboot] [patch] RE: Fam10 breakage
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