[coreboot] [PATCH] v3: introduce generic global variable storage
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Sun Aug 10 14:03:22 CEST 2008
Hi Segher,
can you answer the question near the end of this mail with your gcc hat
on? Thanks.
On 10.08.2008 03:49, ron minnich wrote:
> On Sat, Aug 9, 2008 at 5:45 PM, Carl-Daniel Hailfinger wrote:
>
>> +struct global_vars {
>> +#ifdef CONFIG_CONSOLE_BUFFER
>> + struct printk_buffer *printk_buffer;
>> +#endif
>> +};
>>
>
> I think you should always leave the struct the same size and let it
> have struct members that are in some cases unused.
>
If we do that, we also have to keep the definition of struct
printk_buffer outside #ifdef CONFIG_CONSOLE_BUFFER and that is not
really clean. But I see your point there.
> Why not have, e.g, a page sized global area and allocate out of it?
>
Please NO! One page (4k) is really beyond what we need and with a
minimum CAR size of 4k, we will have exactly zero stack left for
function calls.
> Hmm. we just recreated malloc.
>
You're free to use malloc in stage2 and beyond, but please keep that
complexity out of stage1. malloc becomes ugly really fast if it can fail
easily due to size constraints.
> hmm. Why not when we allocate stack have a constant that defines a
> 'base of stack' area that is some fixed size, which matches struct
> global_vars in size? just random thoughts.
>
I could move the area allocation to be local to stage1_main. That makes
some code more complicated, but individual stage0 asm files can be left
alone when changing the global var struct.
However I'd like a compiler expert to tell me that gcc will NEVER
optimize away or reuse that stack allocation (local variable
"global_buffer") even if that variable is completely unused in the
respective function.
int stage1_main()
{
char global_buffer[1234];
some_function();
some_more_stuff;
return 0;
}
> This is close but Peter's comment is important.
>
I'll push my explanation from my mail answering him into the code.
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
More information about the coreboot
mailing list