[Qemu-devel] [BUG] Windows 7 fails to start with new vgabios binaries

[Qemu-devel] [BUG] Windows 7 fails to start with new vgabios binaries

[Gal Hammer]

A Windows 7 (32-bit) VM running with QXL device fails to start with the 
new updated vgabios binaries (commit 
6eefccc0bb9c34051b1e21880fc3a1c1c8686edd).

qemu's command line:

/usr/bin/qemu-kvm -m 1G -smp 1 -enable-kvm -usb -device usb-tablet \
  -device ide-drive,drive=drive-virtio0-0-0,id=virtio0-0-0,bootindex=1 \
  -drive 
file=win7_32.qcow2,if=none,id=drive-virtio0-0-0,format=qcow2,werror=stop,rerror=stop,cache=none 
\
  -monitor stdio \
  -vga qxl -spice id=on,disable-ticketing,port=5903

FYI,

     Gal.

Re: [Qemu-devel] [BUG] Windows 7 fails to start with new vgabios binaries

[Gerd Hoffmann]

On Mo, 2013-12-09 at 14:18 +0200, Gal Hammer wrote:
> A Windows 7 (32-bit) VM running with QXL device fails to start with the 
> new updated vgabios binaries (commit 
> 6eefccc0bb9c34051b1e21880fc3a1c1c8686edd).
> 
> qemu's command line:
> 
> /usr/bin/qemu-kvm -m 1G -smp 1 -enable-kvm -usb -device usb-tablet \
>   -device ide-drive,drive=drive-virtio0-0-0,id=virtio0-0-0,bootindex=1 \
>   -drive 
> file=win7_32.qcow2,if=none,id=drive-virtio0-0-0,format=qcow2,werror=stop,rerror=stop,cache=none 
> \
>   -monitor stdio \
>   -vga qxl -spice id=on,disable-ticketing,port=5903

[ full quote, adding seabios to Cc ]

Tracked down to the new vgabios stack switching.  With
CONFIG_VGA_ALLOCATE_EXTRA_STACK=n everything is fine again.

cheers,
  Gerd

Re: [Qemu-devel] [SeaBIOS] [BUG] Windows 7 fails to start with new vgabios binaries

[Kevin O'Connor]

On Mon, Dec 09, 2013 at 02:20:59PM +0100, Gerd Hoffmann wrote:
> On Mo, 2013-12-09 at 14:18 +0200, Gal Hammer wrote:
> > A Windows 7 (32-bit) VM running with QXL device fails to start with the 
> > new updated vgabios binaries (commit 
> > 6eefccc0bb9c34051b1e21880fc3a1c1c8686edd).
> > 
> > qemu's command line:
> > 
> > /usr/bin/qemu-kvm -m 1G -smp 1 -enable-kvm -usb -device usb-tablet \
> >   -device ide-drive,drive=drive-virtio0-0-0,id=virtio0-0-0,bootindex=1 \
> >   -drive 
> > file=win7_32.qcow2,if=none,id=drive-virtio0-0-0,format=qcow2,werror=stop,rerror=stop,cache=none 
> > \
> >   -monitor stdio \
> >   -vga qxl -spice id=on,disable-ticketing,port=5903
> 
> [ full quote, adding seabios to Cc ]
> 
> Tracked down to the new vgabios stack switching.  With
> CONFIG_VGA_ALLOCATE_EXTRA_STACK=n everything is fine again.

Ughh.  WinXP doesn't have the problem, but I can also reproduce on
Win7 beta.

I'll change the default for CONFIG_VGA_ALLOCATE_EXTRA_STACK to off.

Thanks,
-Kevin

Re: [Qemu-devel] [SeaBIOS] [BUG] Windows 7 fails to start with new vgabios binaries

[Kevin O'Connor]

On Mon, Dec 09, 2013 at 10:57:25AM -0500, Kevin O'Connor wrote:
> On Mon, Dec 09, 2013 at 02:20:59PM +0100, Gerd Hoffmann wrote:
> > On Mo, 2013-12-09 at 14:18 +0200, Gal Hammer wrote:
> > > A Windows 7 (32-bit) VM running with QXL device fails to start with the 
> > > new updated vgabios binaries (commit 
> > Tracked down to the new vgabios stack switching.  With
> > CONFIG_VGA_ALLOCATE_EXTRA_STACK=n everything is fine again.
> 
> Ughh.  WinXP doesn't have the problem, but I can also reproduce on
> Win7 beta.
> 
> I'll change the default for CONFIG_VGA_ALLOCATE_EXTRA_STACK to off.

I tracked this down further.  The problem is the Windows x86 emulator
doesn't correctly handle memory accesses relative to the %esp
register.  Julian reported this some time back and we worked around it
then.  However, the recent "extra stack" assembler code inserted a few
of these instructions.

I think the proper fix is to leave CONFIG_VGA_ALLOCATE_EXTRA_STACK on
and use slightly different assembler so as not to aggravate win7.

I tested the seabios patch below and my win7 beta now boots okay.

-Kevin


--- a/vgasrc/vgaentry.S
+++ b/vgasrc/vgaentry.S
@@ -97,12 +97,9 @@ entry_10_extrastack:
         movl %ecx, BREGS_ecx(%eax)
         movw %es, BREGS_es(%eax)
         movl %esp, BREGS_size+0(%eax)
-        movzwl %sp, %esp
         movw %ss, BREGS_size+4(%eax)
-        movl (%esp), %edx
-        movl %edx, BREGS_code(%eax)
-        movw 4(%esp), %dx
-        movw %dx, BREGS_flags(%eax)
+        popl BREGS_code(%eax)
+        popw BREGS_flags(%eax)
 
         movw %ds, %dx           // Setup %ss/%esp and call function
         movw %dx, %ss

Re: [Qemu-devel] [SeaBIOS] [BUG] Windows 7 fails to start with new vgabios binaries

[Gal Hammer]

Kevin,

The patch was verified and it solves the problem on Windows 7 official release as well.

Thanks!

    Gal.

----- Original Message -----
From: "Kevin O'Connor" <kevin@koconnor.net>
To: "Gerd Hoffmann" <kraxel@redhat.com>
Cc: "Gal Hammer" <ghammer@redhat.com>, "seabios" <seabios@seabios.org>, qemu-devel@nongnu.org, "Julian Pidancet" <julian.pidancet@gmail.com>
Sent: Tuesday, December 10, 2013 12:12:51 AM
Subject: Re: [SeaBIOS] [BUG] Windows 7 fails to start with new vgabios binaries

On Mon, Dec 09, 2013 at 10:57:25AM -0500, Kevin O'Connor wrote:
> On Mon, Dec 09, 2013 at 02:20:59PM +0100, Gerd Hoffmann wrote:
> > On Mo, 2013-12-09 at 14:18 +0200, Gal Hammer wrote:
> > > A Windows 7 (32-bit) VM running with QXL device fails to start with the 
> > > new updated vgabios binaries (commit 
> > Tracked down to the new vgabios stack switching.  With
> > CONFIG_VGA_ALLOCATE_EXTRA_STACK=n everything is fine again.
> 
> Ughh.  WinXP doesn't have the problem, but I can also reproduce on
> Win7 beta.
> 
> I'll change the default for CONFIG_VGA_ALLOCATE_EXTRA_STACK to off.

I tracked this down further.  The problem is the Windows x86 emulator
doesn't correctly handle memory accesses relative to the %esp
register.  Julian reported this some time back and we worked around it
then.  However, the recent "extra stack" assembler code inserted a few
of these instructions.

I think the proper fix is to leave CONFIG_VGA_ALLOCATE_EXTRA_STACK on
and use slightly different assembler so as not to aggravate win7.

I tested the seabios patch below and my win7 beta now boots okay.

-Kevin


--- a/vgasrc/vgaentry.S
+++ b/vgasrc/vgaentry.S
@@ -97,12 +97,9 @@ entry_10_extrastack:
         movl %ecx, BREGS_ecx(%eax)
         movw %es, BREGS_es(%eax)
         movl %esp, BREGS_size+0(%eax)
-        movzwl %sp, %esp
         movw %ss, BREGS_size+4(%eax)
-        movl (%esp), %edx
-        movl %edx, BREGS_code(%eax)
-        movw 4(%esp), %dx
-        movw %dx, BREGS_flags(%eax)
+        popl BREGS_code(%eax)
+        popw BREGS_flags(%eax)
 
         movw %ds, %dx           // Setup %ss/%esp and call function
         movw %dx, %ss

Re: [Qemu-devel] [SeaBIOS] [BUG] Windows 7 fails to start with new vgabios binaries

[Kevin O'Connor]

On Tue, Dec 10, 2013 at 04:55:36AM -0500, Gal Hammer wrote:
> Kevin,
> 
> The patch was verified and it solves the problem on Windows 7 official release as well.

Thanks.

I pushed this patch to seabios git master.

-Kevin


> 
> ----- Original Message -----
> From: "Kevin O'Connor" <kevin@koconnor.net>
> To: "Gerd Hoffmann" <kraxel@redhat.com>
> Cc: "Gal Hammer" <ghammer@redhat.com>, "seabios" <seabios@seabios.org>, qemu-devel@nongnu.org, "Julian Pidancet" <julian.pidancet@gmail.com>
> Sent: Tuesday, December 10, 2013 12:12:51 AM
> Subject: Re: [SeaBIOS] [BUG] Windows 7 fails to start with new vgabios binaries
> 
> On Mon, Dec 09, 2013 at 10:57:25AM -0500, Kevin O'Connor wrote:
> > On Mon, Dec 09, 2013 at 02:20:59PM +0100, Gerd Hoffmann wrote:
> > > On Mo, 2013-12-09 at 14:18 +0200, Gal Hammer wrote:
> > > > A Windows 7 (32-bit) VM running with QXL device fails to start with the 
> > > > new updated vgabios binaries (commit 
> > > Tracked down to the new vgabios stack switching.  With
> > > CONFIG_VGA_ALLOCATE_EXTRA_STACK=n everything is fine again.
> > 
> > Ughh.  WinXP doesn't have the problem, but I can also reproduce on
> > Win7 beta.
> > 
> > I'll change the default for CONFIG_VGA_ALLOCATE_EXTRA_STACK to off.
> 
> I tracked this down further.  The problem is the Windows x86 emulator
> doesn't correctly handle memory accesses relative to the %esp
> register.  Julian reported this some time back and we worked around it
> then.  However, the recent "extra stack" assembler code inserted a few
> of these instructions.
> 
> I think the proper fix is to leave CONFIG_VGA_ALLOCATE_EXTRA_STACK on
> and use slightly different assembler so as not to aggravate win7.
> 
> I tested the seabios patch below and my win7 beta now boots okay.
> 
> -Kevin
> 
> 
> --- a/vgasrc/vgaentry.S
> +++ b/vgasrc/vgaentry.S
> @@ -97,12 +97,9 @@ entry_10_extrastack:
>          movl %ecx, BREGS_ecx(%eax)
>          movw %es, BREGS_es(%eax)
>          movl %esp, BREGS_size+0(%eax)
> -        movzwl %sp, %esp
>          movw %ss, BREGS_size+4(%eax)
> -        movl (%esp), %edx
> -        movl %edx, BREGS_code(%eax)
> -        movw 4(%esp), %dx
> -        movw %dx, BREGS_flags(%eax)
> +        popl BREGS_code(%eax)
> +        popw BREGS_flags(%eax)
>  
>          movw %ds, %dx           // Setup %ss/%esp and call function
>          movw %dx, %ss