On 11.12.2007 05:43, ron minnich wrote:
> note that the stage1_main code calls disable_car, then stage1_next.
> But disable_car calls stage1_next too! how
> can this works?
>
> The redundant call ensures this code works no
> matter whether the chip-dependent disable_car calls stage1_next or not.
>   

I don't like this. disable_car() should just be able to return without
problems. If it can't do that, fix it.
Otherwise we have to document the call to stage1_end() as a requirement
of any diable_car() function and *justify* it.

> This change makes it possible for an lx to get through CAR to stage2. 
>
> It's kind of ugly, but what it does is require the CAR code to call 
> a function, stage1_end. Why might this be needed? Because, at the end 
> of CAR, we invalidate the stack, which in turn *on some systems* will lose 
> the return address. We are thus losing our stack in disable_car, and 
> to compensate, we are jumping. 
>   

Why exactly do we invalidate the stack? If the stack is clobbered, the
contents of variables should be in a clobbered state as well. In that
case, I suspect parts of our v3 code would misbehave as well.

Can we make sure the stack contents are preserved?

> Signed-off-by: Ronald G. Minnich <[EMAIL PROTECTED]>
>
> Index: arch/x86/stage1.c
> ===================================================================
> --- arch/x86/stage1.c (revision 542)
> +++ arch/x86/stage1.c (working copy)
> @@ -73,23 +73,12 @@
>   */
>  void __attribute__((stdcall)) stage1_main(u32 bist)
>  {
> +     void stage1_end(void);
>       int ret;
> -     struct mem_file archive, result;
> +     struct mem_file archive;
>       int elfboot_mem(struct lb_memory *mem, void *where, int size);
> -     void *entry;
>  
> -     /* we can't 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;
> -
> -
>       post_code(0x02);
>  
>       // before we do anything, we want to stop if we dont run
> @@ -159,7 +148,27 @@
>  
>       /* Turn off Cache-As-Ram */
>       disable_car();
> +     stage1_end();
> +}
>  
> +void stage1_end(void)
> +{
> +     int ret;
> +     struct mem_file archive, result;
> +     int elfboot_mem(struct lb_memory *mem, void *where, int size);
> +     void *entry;
> +     /* we can't 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;
> +
> +
>       entry = load_file_segments(&archive, "normal/stage2");
>       if (entry == (void *)-1)
>               die("FATAL: Failed loading stage2.");
> Index: arch/x86/geodelx/stage1.c
> ===================================================================
> --- arch/x86/geodelx/stage1.c (revision 542)
> +++ arch/x86/geodelx/stage1.c (working copy)
> @@ -55,9 +55,11 @@
>  void disable_car(void)
>  {
>       int i;
> +     void stage1_end(void);
>   

If we really want to call stage1_end from disable_car(), we have to move
that to a header file.

>  
>       for (i = 0; i < ARRAY_SIZE(msr_table); i++)
>               wrmsr(msr_table[i].msrnum, msr_table[i].msr);
>  
>       __asm__("wbinvd\n");
> +     stage1_end();
>   

Disagreed, se above.

>  }

Carl-Daniel

-- 
linuxbios mailing list
linuxbios@linuxbios.org
http://www.linuxbios.org/mailman/listinfo/linuxbios

Reply via email to