[LinuxBIOS] patch for making system run past disable_car

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Thu Dec 13 01:50:59 CET 2007


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 <rminnich at gmail.com>
>
> 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




More information about the coreboot mailing list