[coreboot] [RFC] v3: Stack switching abstraction for C7 and later Intel processors

[ron minnich]

On Wed, Oct 15, 2008 at 4:46 AM, Carl-Daniel Hailfinger
<c-d.hailfinger.devel.2006 at gmx.net> wrote:
> On 14.10.2008 21:15, Peter Stuge wrote:
>> Carl-Daniel Hailfinger wrote:
>>
>>>>> I believe the stage0_main name is misleading. After all, stage0
>>>>> is pure asm and lives in its own .S file.
>>>>>
>>>> let's call it stage1 then and main()
>>>>
>>> Works for me.
>>>
>>
>> I'm afraid I don't like that.
>>
>> Please suggest something that makes the timeline obvious.
>> I think we already have other problems like this in v3.
>>
>> I would be OK with adding phases to stage1 e.g. but I have also
>> contemplated flattening the stage/phase tree to only have stages and
>> no phases - though that doesn't have to happen right now.
>>
>
> OK, so can we settle on phases?
>
> old -> new
> stage1_main before disable_car-> stage1_phase1
> disable_car and stack switch -> stage1_phase2
> stage1_main after stack switch -> stage1_phase3

sounds good.

>
> And yes, I'm aware that the patch below doesn't change the corresponding
> asm code yet. I'll work on that as soon as we have agreed on stage1_*
> naming.
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>
> Index: corebootv3-stackrebuild/arch/x86/stage1.c
> ===================================================================
> --- corebootv3-stackrebuild/arch/x86/stage1.c   (Revision 925)
> +++ corebootv3-stackrebuild/arch/x86/stage1.c   (Arbeitskopie)
> @@ -143,6 +143,9 @@
>        return ret;
>  }
>
> +void stage1_phase2(void);
> +void __attribute__((stdcall)) stage1_phase3(void);
> +
>  /**
>  * This function is called from assembler code with its argument on the
>  * stack. Force the compiler to generate always correct code for this case.
> @@ -154,29 +157,13 @@
>  * that we are restarting after some sort of reconfiguration. Note that we could use it on geode but
>  * do not at present.
>  */
> -void __attribute__((stdcall)) stage1_main(u32 bist, u32 init_detected)
> +void __attribute__((stdcall)) stage1_phase1(u32 bist, u32 init_detected)
>  {
>        struct global_vars globvars;
>        int ret;
>        struct mem_file archive;
> -       void *entry;
>        struct node_core_id me;
> -#ifdef CONFIG_PAYLOAD_ELF_LOADER
> -       struct mem_file result;
> -       int elfboot_mem(struct lb_memory *mem, void *where, int size);
>
> -       /* Why can't we statically init this hack? */
> -       unsigned char faker[64];
> -       struct lb_memory *mem = (struct lb_memory*) faker;
> -
> -       mem->tag = LB_TAG_MEMORY;
> -       mem->size = 28;
> -       mem->map[0].start.lo = mem->map[0].start.hi = 0;
> -       mem->map[0].size.lo = (32*1024*1024);
> -       mem->map[0].size.hi = 0;
> -       mem->map[0].type = LB_MEM_RAM;
> -#endif /* CONFIG_PAYLOAD_ELF_LOADER */
> -
>        post_code(POST_STAGE1_MAIN);
>
>        /* before we do anything, we want to stop if we do not run
> @@ -234,6 +221,29 @@
>
>        printk(BIOS_DEBUG, "Done RAM init code\n");
>
> +       /* Switch the stack location from CAR to RAM, rebuild the stack,
> +        * disable CAR and continue at stage1_phase3(). This is all wrapped in
> +        * stage1_phase2() to make the code easier to follow.
> +        * We will NEVER return.
> +        */
> +       stage1_phase2();
> +
> +       /* If we reach this point, something went terribly wrong. */
> +       die("The world is broken.\n");
> +}
> +
> +/**
> + * This function is called to take care of switching and rebuilding the stack
> + * so that we can cope with processors which don't support a CAR area at low
> + * addresses where CAR could be copied to RAM without problems.
> + * After the stack has been rebuilt, we switch the stack pointer to the new
> + * location, move the printk buffer and cotinue at stage1_phase3().
> + *
> + * NOTE: switch_stack may need to be reimplemented in processor-specific asm.
> + * TODO: Reevaluate whether printk_buffer_move should come before disable_car()
> + */
> +void stage1_phase2()
> +{
>        /* Turn off Cache-As-Ram */
>        disable_car();
>
> @@ -241,7 +251,37 @@
>        /* Move the printk buffer to PRINTK_BUF_ADDR_RAM */
>        printk_buffer_move((void *)PRINTK_BUF_ADDR_RAM, PRINTK_BUF_SIZE_RAM);
>  #endif
> +       stage1_phase3();
> +}

per our earlier discussion, do you really want to try to call more
functions  in stage1_phase2 after you've changed the stack, given
assumptions gcc may have made? Or do you want the printk_buffer_move
in stage1_phase3?

Either way, we have to move forward, let's see the assembly, I'll test
on geode to make sure there is no breakage, then we can move forward
for Corey.

ron

ron