Re: [coreboot] r1057 - in coreboot-v3/arch/x86: . intel/core2

[Stefan Reinauer]

[e-mail] wrote:
> Author: hailfinger
> Date: 2008-11-26 00:39:31 +0100 (Wed, 26 Nov 2008)
> New Revision: 1057
>
> Modified:
>    coreboot-v3/arch/x86/intel/core2/stage0.S
>    coreboot-v3/arch/x86/stage1.c
> Log:
> The Core2Duo CAR code did set up the stack incorrectly. In combination
> with a wrong calling convention of stage1_phase1() this caused stage1 to
> assume BIST had failed.
>
> Signed-off-by: Carl-Daniel Hailfinger 
> Acked-by: Ronald G. Minnich

Can you please back this out again? With this code nothing seems to work
anymore.
> Modified: coreboot-v3/arch/x86/intel/core2/stage0.S
>
> --- coreboot-v3/arch/x86/intel/core2/stage0.S	2008-11-25 23:10:24 UTC (rev 1056)
> +++ coreboot-v3/arch/x86/intel/core2/stage0.S	2008-11-25 23:39:31 UTC (rev 1057)
> @@ -3,6 +3,7 @@
> Copyright (C) 2000,2007 Ronald G. Minnich
> Copyright (C) 2007-2008 coresystems GmbH
> Copyright (C) 2008 Carl-Daniel Hailfinger
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
>
> @@ -159,17 +160,26 @@
>    movl	$(CACHE_AS_RAM_BASE + CACHE_AS_RAM_SIZE - 4), %eax
>    movl	%eax, %esp
> /* Restore the BIST result */
> /* Store zero for the pointer to the global variables. */
> pushl   $0
> /* Restore the BIST result. */
> movl	%ebp, %eax
> /* We need to set ebp? No need. */
> movl	%esp, %ebp
> /* Second parameter: init_detected */
> /* Store zero for the unused init_detected parameter. */
> pushl	$0
> /* First parameter: bist */
> pushl	%eax
>
> -#if 0
> /* this will be interpreted as failed bist */
> port80_post(0x23)
>
> -#endif
>
> call	stage1_phase1
>
> /* We will not go back. */
>
> port80_post(0x2f)
>
> error:
> Modified: coreboot-v3/arch/x86/stage1.c
>
> --- coreboot-v3/arch/x86/stage1.c	2008-11-25 23:10:24 UTC (rev 1056)
> +++ coreboot-v3/arch/x86/stage1.c	2008-11-25 23:39:31 UTC (rev 1057)
> @@ -187,7 +187,7 @@
> 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_phase1(u32 bist, u32 init_detected)
> +void __attribute__((stdcall,regparm(0))) stage1_phase1(u32 bist, u32 init_detected)
>  {
>    struct global_vars globvars;
>    int ret;

r1057 - in coreboot-v3/arch/x86: . intel/core2

[Carl-Daniel Hailfinger]

On 26.11.2008 02:43, Stefan Reinauer wrote:
> [e-mail] wrote:
> > Author: hailfinger
> > Date: 2008-11-26 00:39:31 +0100 (Wed, 26 Nov 2008)
> > New Revision: 1057
> >
> > Modified:
> >    coreboot-v3/arch/x86/intel/core2/stage0.S
> >    coreboot-v3/arch/x86/stage1.c
> > Log:
> > The Core2Duo CAR code did set up the stack incorrectly. In combination
> > with a wrong calling convention of stage1_phase1() this caused stage1 to
> > assume BIST had failed.
> >
> > Signed-off-by: Carl-Daniel Hailfinger 
> > Acked-by: Ronald G. Minnich
>
> Can you please back this out again? With this code nothing seems to work
> anymore.

I kept the added comments and the two initial pushl $0. Committed in r1058.

If it still fails with svn HEAD, feel free to revert the additional
pushl $0. If the code does fail after that, the breakage was introduced
before my commit (or gcc/gas are malfunctioning).

Regards,
Carl-Daniel