diff mbox

PC-BSD installer does not boot with 1.7.4 (bisected)

Message ID 20140206040442.GA5486@morn.localdomain
State New
Headers show

Commit Message

Kevin O'Connor Feb. 6, 2014, 4:04 a.m. UTC 
On Wed, Feb 05, 2014 at 12:44:06PM -0500, Kevin O'Connor wrote:
> On Tue, Feb 04, 2014 at 04:33:19PM +0400, Michael Tokarev wrote:
> > We have a bugreport in debian, http://bugs.debian.org/737142,
> > stating that PC-BSD does not work with seabios-1.7.4 anymore.
> > 
> > I digged in, and found out that it fails only with -vga std
> > (cirrus works fine).  So I bisected the issue - only changing
> > vgabios-stdvga.bin, and found this:
[...]
> This seems to be similar to the freebsd problem reported last year -
> see:
> http://lists.gnu.org/archive/html/qemu-stable/2013-03/msg00037.html
[...]
> (*) The best I can think of is to try and construct some hand crafted
> assembler that can catch buggy x86emu emulators and force a failure..

I have put together some assember to try and run-time test for known
broken versions of x86emu.  This patch to SeaVGABIOS does seem to
catch the freebsd issue and in my tests it convinces freebsd to take a
different approach.  The patch is a bit ugly though.

Comments welcome.
-Kevin


commit e3caa553e940efb6184b30a5637134c4aa9f8b65
Author: Kevin O'Connor <kevin@koconnor.net>
Date:   Wed Feb 5 22:47:29 2014 -0500

    vgabios: Attempt to detect old x86emu and force a fault.
    
    Check for cases where the leal instruction does not work.  This
    instruction is known to not be emulated properly on old versions of
    x86emu.  If a broken version of x86emu is found, force a fault that
    x86emu will easily detect.  This should help prevent soft failures
    when running old software.
    
    Signed-off-by: Kevin O'Connor <kevin@koconnor.net>

Comments

Michael Tokarev Feb. 6, 2014, 12:44 p.m. UTC | #1 
[Changing subject to reflect reality]

I don't really understand what it is all about, as I initially said.
But I've been told on freebsd IRC channels to post the issue to
freebsd-emulation list, which I'm Cc'ing now, and if noone there
answers, also to John Baldwin <jhb@FreeBSD>.

To bring some context back, here are a few pointers:

 http://bugs.debian.org/737142 --
   talking about pc-bsd, but it appears that the prob affects other BSD
   kernels too
 http://thread.gmane.org/gmane.comp.emulators.qemu/254074 -- this thread
 http://lists.gnu.org/archive/html/qemu-stable/2013-03/msg00037.html - a
   weird issue with freebsd reported last december

If it's a bug in *BSD kernels, maybe someone there will be glad to fix
it.  And maybe together we can come out with a more elegant solution to
this issue, or maybe to confirm (or deny) that the proposed patch is
a right thing to do.

Thanks,

/mjt

06.02.2014 08:04, Kevin O'Connor wrote:
> On Wed, Feb 05, 2014 at 12:44:06PM -0500, Kevin O'Connor wrote:
>> On Tue, Feb 04, 2014 at 04:33:19PM +0400, Michael Tokarev wrote:
>>> We have a bugreport in debian, http://bugs.debian.org/737142,
>>> stating that PC-BSD does not work with seabios-1.7.4 anymore.
>>>
>>> I digged in, and found out that it fails only with -vga std
>>> (cirrus works fine).  So I bisected the issue - only changing
>>> vgabios-stdvga.bin, and found this:
> [...]
>> This seems to be similar to the freebsd problem reported last year -
>> see:
>> http://lists.gnu.org/archive/html/qemu-stable/2013-03/msg00037.html
> [...]
>> (*) The best I can think of is to try and construct some hand crafted
>> assembler that can catch buggy x86emu emulators and force a failure..
> 
> I have put together some assember to try and run-time test for known
> broken versions of x86emu.  This patch to SeaVGABIOS does seem to
> catch the freebsd issue and in my tests it convinces freebsd to take a
> different approach.  The patch is a bit ugly though.
> 
> Comments welcome.
> -Kevin
> 
> 
> commit e3caa553e940efb6184b30a5637134c4aa9f8b65
> Author: Kevin O'Connor <kevin@koconnor.net>
> Date:   Wed Feb 5 22:47:29 2014 -0500
> 
>     vgabios: Attempt to detect old x86emu and force a fault.
>     
>     Check for cases where the leal instruction does not work.  This
>     instruction is known to not be emulated properly on old versions of
>     x86emu.  If a broken version of x86emu is found, force a fault that
>     x86emu will easily detect.  This should help prevent soft failures
>     when running old software.
>     
>     Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
> 
> diff --git a/vgasrc/vgaentry.S b/vgasrc/vgaentry.S
> index 9854448..e246e7c 100644
> --- a/vgasrc/vgaentry.S
> +++ b/vgasrc/vgaentry.S
> @@ -45,9 +45,27 @@ _rom_header_signature:
>   * Entry points
>   ****************************************************************/
>  
> -        // This macro is the same as ENTRY_ARG except the "calll"
> -        // instruction is avoided to work around known issues in the
> -        // emulation of some versions of x86emu.
> +        // Force a fault if found to be running on broken x86emu versions.
> +        DECLFUNC x86emu_fault
> +x86emu_fault:
> +        int $0x03
> +1:      hlt
> +        jmp 1b
> +
> +        // This macro implements a call while avoiding instructions
> +        // that old versions of x86emu have problems with.
> +        .macro VGA_CALLL cfunc
> +        // Make sure leal instruction works.
> +        movl $0x8000, %ecx
> +        leal (%ecx, %ecx, 1), %ecx
> +        cmpl $0x10000, %ecx
> +        jne x86emu_fault
> +        // Use callw instead of calll
> +        push %ax
> +        callw \cfunc
> +        .endm
> +
> +        // This macro is the same as ENTRY_ARG except VGA_CALLL is used.
>          .macro ENTRY_ARG_VGA cfunc
>          cli
>          cld
> @@ -57,7 +75,7 @@ _rom_header_signature:
>          movl %esp, %ebx         // Backup %esp, then zero high bits
>          movzwl %sp, %esp
>          movl %esp, %eax         // First arg is pointer to struct bregs
> -        pushw %ax ; callw \cfunc
> +        VGA_CALLL \cfunc
>          movl %ebx, %esp         // Restore %esp (including high bits)
>          POPBREGS
>          .endm
> @@ -103,7 +121,7 @@ entry_10_extrastack:
>          movw %ds, %dx           // Setup %ss/%esp and call function
>          movw %dx, %ss
>          movl %eax, %esp
> -        pushw %ax ; callw handle_10
> +        VGA_CALLL handle_10
>  
>          movl %esp, %eax         // Restore registers and return
>          movw BREGS_size+4(%eax), %ss
>
Kevin O'Connor Feb. 10, 2014, 3:41 p.m. UTC | #2 
On Wed, Feb 05, 2014 at 11:04:42PM -0500, Kevin O'Connor wrote:
> On Wed, Feb 05, 2014 at 12:44:06PM -0500, Kevin O'Connor wrote:
> > On Tue, Feb 04, 2014 at 04:33:19PM +0400, Michael Tokarev wrote:
> > > We have a bugreport in debian, http://bugs.debian.org/737142,
> > > stating that PC-BSD does not work with seabios-1.7.4 anymore.
> > > 
> > > I digged in, and found out that it fails only with -vga std
> > > (cirrus works fine).  So I bisected the issue - only changing
> > > vgabios-stdvga.bin, and found this:
> [...]
> > This seems to be similar to the freebsd problem reported last year -
> > see:
> > http://lists.gnu.org/archive/html/qemu-stable/2013-03/msg00037.html
> [...]
> > (*) The best I can think of is to try and construct some hand crafted
> > assembler that can catch buggy x86emu emulators and force a failure..
> 
> I have put together some assember to try and run-time test for known
> broken versions of x86emu.  This patch to SeaVGABIOS does seem to
> catch the freebsd issue and in my tests it convinces freebsd to take a
> different approach.  The patch is a bit ugly though.

There hasn't been much comment, but I have pushed this patch to the
main seabios repo for now.

-Kevin
Xin Li Feb. 11, 2014, 10:34 p.m. UTC | #3 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 2/6/14, 4:44 AM, Michael Tokarev wrote:
> [Changing subject to reflect reality]
> 
> I don't really understand what it is all about, as I initially
> said. But I've been told on freebsd IRC channels to post the issue
> to freebsd-emulation list, which I'm Cc'ing now, and if noone
> there answers, also to John Baldwin <jhb@FreeBSD>.
> 
> To bring some context back, here are a few pointers:
> 
> http://bugs.debian.org/737142 -- talking about pc-bsd, but it
> appears that the prob affects other BSD kernels too 
> http://thread.gmane.org/gmane.comp.emulators.qemu/254074 -- this
> thread 
> http://lists.gnu.org/archive/html/qemu-stable/2013-03/msg00037.html
> - a weird issue with freebsd reported last december
> 
> If it's a bug in *BSD kernels, maybe someone there will be glad to
> fix it.  And maybe together we can come out with a more elegant
> solution to this issue, or maybe to confirm (or deny) that the
> proposed patch is a right thing to do.

Yes it does look like a bug in FreeBSD kernel.  Is there a PR already
or should I create one and assign to myself?  I will try to fix this
when I have some free cycles.

I assume this also happens when running FreeBSD under qemu on FreeBSD,
right?

Cheers,

> Thanks,
> 
> /mjt
> 
> 06.02.2014 08:04, Kevin O'Connor wrote:
>> On Wed, Feb 05, 2014 at 12:44:06PM -0500, Kevin O'Connor wrote:
>>> On Tue, Feb 04, 2014 at 04:33:19PM +0400, Michael Tokarev
>>> wrote:
>>>> We have a bugreport in debian,
>>>> http://bugs.debian.org/737142, stating that PC-BSD does not
>>>> work with seabios-1.7.4 anymore.
>>>> 
>>>> I digged in, and found out that it fails only with -vga std 
>>>> (cirrus works fine).  So I bisected the issue - only
>>>> changing vgabios-stdvga.bin, and found this:
>> [...]
>>> This seems to be similar to the freebsd problem reported last
>>> year - see: 
>>> http://lists.gnu.org/archive/html/qemu-stable/2013-03/msg00037.html
>>
>>> 
[...]
>>> (*) The best I can think of is to try and construct some hand
>>> crafted assembler that can catch buggy x86emu emulators and
>>> force a failure..
>> 
>> I have put together some assember to try and run-time test for
>> known broken versions of x86emu.  This patch to SeaVGABIOS does
>> seem to catch the freebsd issue and in my tests it convinces
>> freebsd to take a different approach.  The patch is a bit ugly
>> though.
>> 
>> Comments welcome. -Kevin
>> 
>> 
>> commit e3caa553e940efb6184b30a5637134c4aa9f8b65 Author: Kevin
>> O'Connor <kevin@koconnor.net> Date:   Wed Feb 5 22:47:29 2014
>> -0500
>> 
>> vgabios: Attempt to detect old x86emu and force a fault.
>> 
>> Check for cases where the leal instruction does not work.  This 
>> instruction is known to not be emulated properly on old versions
>> of x86emu.  If a broken version of x86emu is found, force a fault
>> that x86emu will easily detect.  This should help prevent soft
>> failures when running old software.
>> 
>> Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
>> 
>> diff --git a/vgasrc/vgaentry.S b/vgasrc/vgaentry.S index
>> 9854448..e246e7c 100644 --- a/vgasrc/vgaentry.S +++
>> b/vgasrc/vgaentry.S @@ -45,9 +45,27 @@ _rom_header_signature: *
>> Entry points 
>> ****************************************************************/
>>
>>  -        // This macro is the same as ENTRY_ARG except the
>> "calll" -        // instruction is avoided to work around known
>> issues in the -        // emulation of some versions of x86emu. +
>> // Force a fault if found to be running on broken x86emu
>> versions. +        DECLFUNC x86emu_fault +x86emu_fault: +
>> int $0x03 +1:      hlt +        jmp 1b + +        // This macro
>> implements a call while avoiding instructions +        // that
>> old versions of x86emu have problems with. +        .macro
>> VGA_CALLL cfunc +        // Make sure leal instruction works. +
>> movl $0x8000, %ecx +        leal (%ecx, %ecx, 1), %ecx +
>> cmpl $0x10000, %ecx +        jne x86emu_fault +        // Use
>> callw instead of calll +        push %ax +        callw \cfunc +
>> .endm + +        // This macro is the same as ENTRY_ARG except
>> VGA_CALLL is used. .macro ENTRY_ARG_VGA cfunc cli cld @@ -57,7
>> +75,7 @@ _rom_header_signature: movl %esp, %ebx         // Backup
>> %esp, then zero high bits movzwl %sp, %esp movl %esp, %eax
>> // First arg is pointer to struct bregs -        pushw %ax ;
>> callw \cfunc +        VGA_CALLL \cfunc movl %ebx, %esp         //
>> Restore %esp (including high bits) POPBREGS .endm @@ -103,7
>> +121,7 @@ entry_10_extrastack: movw %ds, %dx           // Setup
>> %ss/%esp and call function movw %dx, %ss movl %eax, %esp -
>> pushw %ax ; callw handle_10 +        VGA_CALLL handle_10
>> 
>> movl %esp, %eax         // Restore registers and return movw
>> BREGS_size+4(%eax), %ss
>> 
> 
> _______________________________________________ 
> freebsd-emulation@freebsd.org mailing list 
> http://lists.freebsd.org/mailman/listinfo/freebsd-emulation To
> unsubscribe, send any mail to
> "freebsd-emulation-unsubscribe@freebsd.org"
> 

-----BEGIN PGP SIGNATURE-----

iQIcBAEBCgAGBQJS+qWRAAoJEJW2GBstM+nsz3sP/AsGiEPWzHAbQP5nYR9alDiL
dmMd8RB4yVYLmJJXCxLldMjNw3kuHRcZ8IrvCaxsFvUHnzdQudmqCc6uxgcWdC3Y
gqSYP2/dnG5OXsUqmRHW0yKpfrBlbOA/utRDZz9MzTx4m7sudY8y8ZEUmb2OyJOZ
CjQrW5OyHIXUxzdKMlcpiAy5+edQVWk5TBdyz6h0WVrk4A9uuAjOw/DYse05lGVF
l1d/Eto+Wui38fMTjfdVJqVcPrfy859aODf/vRkmznujd0Zumr/8OMXmEsF+Q+iv
ktRNWab39DXIArMsYbCGr7Xg7Q0ZjZB0eAjB8zxW1a0p/7V4Spf9QQu9wNHVi4sf
DOwiaU2K+2/zz/BYIyicyZN+glQMLcq+t3u8uagm6GOcHMx1ZsyFeyIhj8Gbtjfl
VUPOVZfu25dogr5GY2U68tPHoiQStsSvdSg4+jaDYK5AZcs1BGZ5i5ncArjATV1Z
KIe3ISOzu20BMhjAXHALnTNMFuBWD6+Va6ExVHWkWuz445aQ0REx3hJDqWbm7iVd
DTD2N6Bs/uL7P3aVODgz8EqQ6EMhswFYgqSirMb9w38FCLnJ56aAA31kFU00U28p
OG8kzgc3dY0II1vVfNLhrJ5kGNDOKxZ62KsXbLHPdBl3cgKweqghd53WTjU/dSo6
0NVfAqheJ3ifZ0I0yB3d
=jTIz
-----END PGP SIGNATURE-----
Michael Tokarev Feb. 17, 2014, 5:35 p.m. UTC | #4 
10.02.2014 19:41, Kevin O'Connor wrote:
[]
> There hasn't been much comment, but I have pushed this patch to the
> main seabios repo for now.

Xin Li (Cc'ed) replied to this email.

Meanwhile I tried your patch, and it indeed fixes the reported issues
with FreeBSD and PCBSD.

Even if FreeBSD fixes this bug, we still have to support runnin old,
unfixed, systems in qemu/kvm, so seabios-side change is needed anyway.

Oh well.

Thanks,

/mjt
Kevin O'Connor Feb. 17, 2014, 7:07 p.m. UTC | #5 
On Mon, Feb 17, 2014 at 09:35:30PM +0400, Michael Tokarev wrote:
> 10.02.2014 19:41, Kevin O'Connor wrote:
> []
> > There hasn't been much comment, but I have pushed this patch to the
> > main seabios repo for now.
> 
> Xin Li (Cc'ed) replied to this email.

Yes.  Thanks.  It would be good for FreeBSD to also fix the underlying
issue.

> Meanwhile I tried your patch, and it indeed fixes the reported issues
> with FreeBSD and PCBSD.
> 
> Even if FreeBSD fixes this bug, we still have to support runnin old,
> unfixed, systems in qemu/kvm, so seabios-side change is needed anyway.

Yeah.  That's why I went ahead and committed the SeaVGABIOS
workaround.  The workaround also makes early versions of X11 behave
better (they fail outright now instead of hanging).

-Kevin
diff mbox

Patch

diff --git a/vgasrc/vgaentry.S b/vgasrc/vgaentry.S
index 9854448..e246e7c 100644
--- a/vgasrc/vgaentry.S
+++ b/vgasrc/vgaentry.S
@@ -45,9 +45,27 @@  _rom_header_signature:
  * Entry points
  ****************************************************************/
 
-        // This macro is the same as ENTRY_ARG except the "calll"
-        // instruction is avoided to work around known issues in the
-        // emulation of some versions of x86emu.
+        // Force a fault if found to be running on broken x86emu versions.
+        DECLFUNC x86emu_fault
+x86emu_fault:
+        int $0x03
+1:      hlt
+        jmp 1b
+
+        // This macro implements a call while avoiding instructions
+        // that old versions of x86emu have problems with.
+        .macro VGA_CALLL cfunc
+        // Make sure leal instruction works.
+        movl $0x8000, %ecx
+        leal (%ecx, %ecx, 1), %ecx
+        cmpl $0x10000, %ecx
+        jne x86emu_fault
+        // Use callw instead of calll
+        push %ax
+        callw \cfunc
+        .endm
+
+        // This macro is the same as ENTRY_ARG except VGA_CALLL is used.
         .macro ENTRY_ARG_VGA cfunc
         cli
         cld
@@ -57,7 +75,7 @@  _rom_header_signature:
         movl %esp, %ebx         // Backup %esp, then zero high bits
         movzwl %sp, %esp
         movl %esp, %eax         // First arg is pointer to struct bregs
-        pushw %ax ; callw \cfunc
+        VGA_CALLL \cfunc
         movl %ebx, %esp         // Restore %esp (including high bits)
         POPBREGS
         .endm
@@ -103,7 +121,7 @@  entry_10_extrastack:
         movw %ds, %dx           // Setup %ss/%esp and call function
         movw %dx, %ss
         movl %eax, %esp
-        pushw %ax ; callw handle_10
+        VGA_CALLL handle_10
 
         movl %esp, %eax         // Restore registers and return
         movw BREGS_size+4(%eax), %ss