Re: [coreboot] [PATCH] Fix geode lx VSA loading

[Patrick Georgi]

Am Freitag, den 04.06.2010, 16:25 +0100 schrieb Edwin Beasant:
> This patch fixes the option rom code that was buggy when it switched
> segment registers before restoring register values. This was breaking
> the Geode VSA, and probably would have hurt other option roms as well.
>
> Signed-off by: Edwin Beasant [e-mail]

Except for the first two parts (the magic values in ecx/edx and
whitespace), and assuming this is tested, the patch is

Acked-by: Patrick Georgi

[PATCH] Fix geode lx VSA loading

[Stefan Reinauer]

On 6/4/10 6:37 PM, Patrick Georgi wrote:
> Am Freitag, den 04.06.2010, 16:25 +0100 schrieb Edwin Beasant:
> > This patch fixes the option rom code that was buggy when it switched
> > segment registers before restoring register values. This was breaking
> > the Geode VSA, and probably would have hurt other option roms as well.
> >
> > Signed-off by: Edwin Beasant [e-mail]
>
> Except for the first two parts (the magic values in ecx/edx and
> whitespace), and assuming this is tested, the patch is
>
> Acked-by: Patrick Georgi

I did a new version of the patch with those issues fixed (and pushing
one register instead of all), see attachment

Signed-off-by: Edwin Beasant 
Signed-off-by: Stefan Reinauer

[PATCH] Fix geode lx VSA loading

[Myles Watson]

I don't see where ax gets set to 0.  Does the comment need to be updated, or
do you want all the segments set to ax?

+	/* initialize registers for option rom lcall */
+	movl	__registers +  0, %eax
+	movl	__registers +  4, %ebx
+	movl	__registers +  8, %ecx
+	movl	__registers + 12, %edx
+	movl	__registers + 16, %esi
+	movl	__registers + 20, %edi	
+
    /* Set all segments to 0x0000, ds to 0x0040 */
+	push	%ax
    mov	%ax, %es
    mov	%ax, %fs
    mov	%ax, %gs
    mov	$0x40, %ax
    mov	%ax, %ds
+	pop	%ax

Maybe you want a

mov $0, %ax

right after the push %ax?

Thanks,
Myles

[PATCH] Fix geode lx VSA loading

[Stefan Reinauer]

On 6/4/10 6:40 PM, Stefan Reinauer wrote:
> On 6/4/10 6:37 PM, Patrick Georgi wrote:
> > Am Freitag, den 04.06.2010, 16:25 +0100 schrieb Edwin Beasant:
> > > This patch fixes the option rom code that was buggy when it switched
> > > segment registers before restoring register values. This was breaking
> > > the Geode VSA, and probably would have hurt other option roms as well.
> > >
> > > Signed-off by: Edwin Beasant [e-mail]
> >
> > Except for the first two parts (the magic values in ecx/edx and
> > whitespace), and assuming this is tested, the patch is
> >
> > Acked-by: Patrick Georgi
>
> I did a new version of the patch with those issues fixed (and pushing
> one register instead of all), see attachment
>
> Signed-off-by: Edwin Beasant 
> Signed-off-by: Stefan Reinauer

Updated patch.

[PATCH] Fix geode lx VSA loading

[ron minnich]

On Fri, Jun 4, 2010 at 9:53 AM, Myles Watson  wrote:
> mov $0, %ax

use the classic:
xorl %ax, %ax

ron

[PATCH] Fix geode lx VSA loading

[Edwin Beasant]

Agreed - ax should be 0, it is only 0 in this case due to the argument passed in!
Before the segment setup, but after the _registers section, an xorl %ax, %al would be appropriate.
Thanks!
Edwin

-----Original Message-----
From: coreboot-bounces+edwin_beasant=[e-mail] [mailto:coreboot-bounces+edwin_beasant=[e-mail]] On Behalf Of ron minnich
Sent: 06 June 2010 05:50
To: Myles Watson
Cc: Stefan Reinauer; [e-mail]
Subject: Re: [coreboot] [PATCH] Fix geode lx VSA loading

On Fri, Jun 4, 2010 at 9:53 AM, Myles Watson  wrote:
> mov $0, %ax

use the classic:
xorl %ax, %ax

ron