[coreboot] [patch] RE: Fam10 breakage

Bao, Zheng Zheng.Bao at amd.com
Mon Mar 1 09:00:09 CET 2010


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 at coreboot.org
[mailto:coreboot-bounces at coreboot.org] On Behalf Of Patrick Georgi
Sent: Monday, March 01, 2010 3:39 PM
To: coreboot at 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

-- 
coreboot mailing list: coreboot at coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot






More information about the coreboot mailing list