[coreboot] v3 qemu and gcc-4.3.0

[Jordan Crouse]

On 22/03/08 04:47 +0100, Devils-Hawk wrote:
> > You are probably crashing on the lret at the bottom of the
> > entry function in i386/head.S.  If you set up gdb, you should be able
> > to stop execution right before the lret (if you set a breakpoint at the
> > ElF entrypoint, the offending instruction is only 7 or 8 instructions past
> > the entry).  I would like to know what is on the stack at this point (
> > dump the two dwords at %esp).  I'm guessing that the stack is wrong, and
> > thats causing the triple fault.
> >
> > Jordan
> Yes, it did crash exactly at the lret of the _entry function. After trying to 
> understand some of the funkiness in head.S i came up with following patch 
> which did fix the problem but i should probably still look whats going wrong 
> with the original code. Its get getting quite late around here so I'll just 
> post what i have and get some sleep first. ;)
> 
> Let the linker figure out the correct address and just CALL the start_main 
> entry point.
> 
> Signed-off-by: Klaus Schnass <dev at stuffit.at>

I appreciate your efforts, but since the previous code worked fine on
hardware and SimNow (and most versions of qemu, for that matter), we're
ignoring a flaw in Qemu that should be investigated.  I would hate to
cover something up that would come back to bite us later.

That said, the code below is good, with the changes below:

> Index: i386/head.S
> ===================================================================
> --- i386/head.S	(revision 3185)
> +++ i386/head.S	(working copy)
> @@ -50,31 +50,26 @@
>  	/* No interrupts, please. */
>  	cli
>  
> -	/* Get the current stack pointer. */
> +	/* store current stack pointer  */
>  	movl %esp, %esi
>  
> +	/* Setup new stack */
>  	movl _istack, %ebx

Get rid of this.

> -	/* lret needs %cs in the stack, so copy it over. */
> -	movw %cs, 4(%ebx)
> +	movl (%ebx), %esp

If we go with this method, then we don't need the initial stack
structure at all - replace this with

	movl _stack, %esp

> -	/*
> -	 * Exchange the current stack pointer for the one in the initial
> -	 * stack (which happens to be the new stack pointer).
> -	 */
> -	xchgl %esi, 16(%ebx)
> +	/* Save old stack pointer */
> +	pushl %esi
>  
> -	/* Set the new stack pointer. */
> -	movl %esi, %esp
> -
> -	/* Return into the main entry function and go. */
> -	lret
> -
> +	/* let's rock */
> +	call start_main
> +	
>  _leave:
> -	movl _istack, %ebx
> +	/* get old stack pointer */
> +	popl %ebx
>  
> -	/* Restore the stack pointer from the storage area. */
> -	movl 16(%ebx), %esp
> +	/* restore old stack */
> +	movl %esp, %ebx
	
	wrong direction - it should be
	movl %ebx, %esp

>  	/* Return to the original context. */
> 	lret

With the changes, this should be just a ret

> Index: i386/main.c
> ===================================================================
> --- i386/main.c	(revision 3185)
> +++ i386/main.c	(working copy)
> @@ -36,17 +36,12 @@
>   * stack we store the original stack pointer from the calling application.
>   */
>  
> -static void start_main(void);
>  extern void _leave(void);
>  
>  static struct {
> -	uint32_t eip[2];
> -	uint32_t raddr[2];
>  	uint32_t esp;
> -} initial_stack __attribute__ ((section(".istack"))) = {
> -	{ (uint32_t) start_main, 0 },
> -	{ (uint32_t) _leave, 0 },
> -	(uint32_t) & initial_stack,
> +} initial_stack  __attribute__ ((section(".istack"))) = {
> +  (uint32_t) &initial_stack,
>  };
>  
>  void *_istack = &initial_stack;

Get rid of all this.

> @@ -55,7 +50,7 @@
>   * This is our C entry function - set up the system
>   * and jump into the payload entry point.
>   */
> -static void start_main(void)
> +void start_main(void)
>  {
>  	extern int main(void);
>  

And finally, remove the istack segment from the linker script.

Thanks,
Jordan