[coreboot] global_vars

[Carl-Daniel Hailfinger]

On 09.09.2008 03:18, Kevin O'Connor wrote:
> Hi,
>
> I was looking through the v3 code and came across this:
>
> void *bottom_of_stack(void)
> {
>         /* -4 because CONFIG_CARBASE + CONFIG_CARSIZE - 4 is initial %esp */
>         return (void *)(CONFIG_CARBASE + CONFIG_CARSIZE - 4);
> }
>
> struct global_vars *global_vars(void)
> {
>         return *(struct global_vars **)(bottom_of_stack() - sizeof(struct global_vars *));
> }
>
> [...]
>
> void __attribute__((stdcall)) stage1_main(u32 bist, u32 init_detected)
> {
>         struct global_vars globvars;
>
>
> If I understand the code correctly, the global_vars() function is
> trying to get at the stack variable globvars declared in stage1_main.
>   

No, that's not what the code does. global_vars() reads the address of
globvars from the bottom of stack where that address has been stored.
Note the double indirection.

> It's not valid to do that - the compiler is free to re-order stack
> variables, change them, or otherwise do funky things.
>   

Correct. That's why we do it the right way.

> A better way to do this would be to have some assembler code prior to
> stage1_main() reserve the space for globvars.
>   

We had that model in the past and it was too difficult to maintain.
Especially because creating assembler code which knows the size of
struct global_vars is difficult. The compiler is free to perform any
padding and reordering it wants, so it's almost guaranteed to break
during runtime.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/