[coreboot] [RFC] v3: Stack switching abstraction for C7 and later Intel processors
c-d.hailfinger.devel.2006 at gmx.net
Wed Oct 15 19:04:20 CEST 2008
On 15.10.2008 17:31, ron minnich wrote:
> 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_*
>> 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;
>> - struct lb_memory *mem = (struct lb_memory*) faker;
>> - mem->tag = LB_TAG_MEMORY;
>> - mem->size = 28;
>> - mem->map.start.lo = mem->map.start.hi = 0;
>> - mem->map.size.lo = (32*1024*1024);
>> - mem->map.size.hi = 0;
>> - mem->map.type = LB_MEM_RAM;
>> -#endif /* CONFIG_PAYLOAD_ELF_LOADER */
>> /* 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 */
>> @@ -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);
>> + 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?
Absolutely not. You're right about this. However, right now my focus is
on keeping changes isolated and I'd take care of this in a separate patch.
> Or do you want the printk_buffer_move
> in stage1_phase3?
I'd prefer to have printk_buffer_move as first instruction in stage1_phase2.
> 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.
I'll be back in ~3 hours. If you have acked this in the meanwhile, I
will commit, otherwise I'll rework.
More information about the coreboot