Re: [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