Re: [SeaBIOS] vgabios testing

[Kevin O'Connor]

On Mon, Mar 05, 2012 at 04:41:27PM +0000, Julian Pidancet wrote:
> On Mon, Mar 5, 2012 at 4:21 PM, Kevin O'Connor  wrote:
> > On Mon, Mar 05, 2012 at 04:05:11PM +0000, Julian Pidancet wrote:
> > > Replacing instructions and handling displacement is probably going to
> > > be a huge pain.
> >
> > I don't think that will be an issue.  One can tell gcc to generate
> > assembler and then post-process that.  The gcc created assembler is
> > still label based so no positional issues should arise.
>
> Yes you're right. Post-processing the intermediate assembly will
> definitely be a huge win.

I put together the below as a quick hack.  It boots to DOS okay.  I'm
not sure the exact test case for x86emu and I didn't test that.  It's
just a quick hack, but it should highlight the idea.

-Kevin

diff --git a/Makefile b/Makefile
index 5d834b7..6cba6d5 100644
--- a/Makefile
+++ b/Makefile
@@ -31,7 +31,7 @@ COMMONCFLAGS = -I$(OUT) -Os -MD -g \
     -Wall -Wno-strict-aliasing -Wold-style-definition \
     $(call cc-option,$(CC),-Wtype-limits,) \
     -m32 -march=i386 -mregparm=3 -mpreferred-stack-boundary=2 \
-    -mrtd -minline-all-stringops \
+    -minline-all-stringops \
     -freg-struct-return -ffreestanding -fno-delete-null-pointer-checks \
     -ffunction-sections -fdata-sections -fno-common
 COMMONCFLAGS += $(call cc-option,$(CC),-nopie,)
@@ -193,7 +193,12 @@ SRCVGA=src/output.c src/util.c src/pci.c \

CFLAGS16VGA = $(CFLAGS16INC) -Isrc

-$(OUT)vgaccode16.o: $(OUT)autoconf.h ; $(call whole-compile, $(CFLAGS16VGA), $(SRCVGA),$@)
+$(OUT)vgaccode16.raw.s: $(OUT)autoconf.h ; $(call whole-compile, $(CFLAGS16VGA) -S, $(SRCVGA),$@)
+
+$(OUT)vgaccode16.o: $(OUT)vgaccode16.raw.s
+	@echo "  Fixup VGA rom assembler"
+	$(Q)$(PYTHON) ./tools/vgafixup.py $< $(OUT)vgaccode16.s
+	$(Q)as --32 src/code16gcc.s $(OUT)vgaccode16.s -o $@

$(OUT)vgaentry.o: vgaentry.S $(OUT)autoconf.h
    @echo "  Compiling (16bit) $@"
diff --git a/tools/vgafixup.py b/tools/vgafixup.py
new file mode 100644
index 0000000..db37037
--- /dev/null
+++ b/tools/vgafixup.py
@@ -0,0 +1,28 @@
+#!/usr/bin/env python
+# Work around x86emu bugs by replacing problematic instructions.
+#
+# Copyright (C) 2012  Kevin O'Connor 
+#
+# This file may be distributed under the terms of the GNU GPLv3 license.
+
+import sys
+
+def main():
+    infilename, outfilename = sys.argv[1:]
+    infile = open(infilename, 'rb')
+    out = []
+    for line in infile:
+        sline = line.strip()
+        if sline == 'ret':
+            out.append('retw $2\n')
+        elif sline == 'leave':
+            out.append('movl %ebp, %esp ; popl %ebp\n')
+        else:
+            out.append(line)
+    infile.close()
+    outfile = open(outfilename, 'wb')
+    outfile.write(''.join(out))
+    outfile.close()
+
+if __name__ == '__main__':
+    main()
diff --git a/vgasrc/clext.c b/vgasrc/clext.c
index e5dce35..fc5459a 100644
--- a/vgasrc/clext.c
+++ b/vgasrc/clext.c
@@ -526,7 +526,7 @@ ASM16(
     "a0h_callback:"
     "cli\n"
     "hlt\n"
-    "retf");
+    "lretw");

static void
 clext_1012a0(struct bregs *regs)

vgabios testing

[Julian Pidancet]

On Mon, Mar 5, 2012 at 5:18 PM, Kevin O'Connor  wrote:
> On Mon, Mar 05, 2012 at 04:41:27PM +0000, Julian Pidancet wrote:
> > On Mon, Mar 5, 2012 at 4:21 PM, Kevin O'Connor  wrote:
> > > On Mon, Mar 05, 2012 at 04:05:11PM +0000, Julian Pidancet wrote:
> > > > Replacing instructions and handling displacement is probably going to
> > > > be a huge pain.
> > >
> > > I don't think that will be an issue.  One can tell gcc to generate
> > > assembler and then post-process that.  The gcc created assembler is
> > > still label based so no positional issues should arise.
> >
> > Yes you're right. Post-processing the intermediate assembly will
> > definitely be a huge win.
>
> I put together the below as a quick hack.  It boots to DOS okay.  I'm
> not sure the exact test case for x86emu and I didn't test that.  It's
> just a quick hack, but it should highlight the idea.
>
> -Kevin
>
> diff --git a/Makefile b/Makefile
> index 5d834b7..6cba6d5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -31,7 +31,7 @@ COMMONCFLAGS = -I$(OUT) -Os -MD -g \
>     -Wall -Wno-strict-aliasing -Wold-style-definition \
>     $(call cc-option,$(CC),-Wtype-limits,) \
>     -m32 -march=i386 -mregparm=3 -mpreferred-stack-boundary=2 \
> -mrtd -minline-all-stringops \
> -minline-all-stringops \
>
> -freg-struct-return -ffreestanding -fno-delete-null-pointer-checks \
>     -ffunction-sections -fdata-sections -fno-common
>  COMMONCFLAGS += $(call cc-option,$(CC),-nopie,)
> @@ -193,7 +193,12 @@ SRCVGA=src/output.c src/util.c src/pci.c \
>
> CFLAGS16VGA = $(CFLAGS16INC) -Isrc
>
> -$(OUT)vgaccode16.o: $(OUT)autoconf.h ; $(call whole-compile, $(CFLAGS16VGA), $(SRCVGA),$@)
> +$(OUT)vgaccode16.raw.s: $(OUT)autoconf.h ; $(call whole-compile, $(CFLAGS16VGA) -S, $(SRCVGA),$@)
>
> +$(OUT)vgaccode16.o: $(OUT)vgaccode16.raw.s
> @echo "  Fixup VGA rom assembler"
> $(Q)$(PYTHON) ./tools/vgafixup.py $< $(OUT)vgaccode16.s
> $(Q)as --32 src/code16gcc.s $(OUT)vgaccode16.s -o $@
>
> $(OUT)vgaentry.o: vgaentry.S $(OUT)autoconf.h
>        @echo "  Compiling (16bit) $@"
> diff --git a/tools/vgafixup.py b/tools/vgafixup.py
> new file mode 100644
> index 0000000..db37037
> --- /dev/null
> +++ b/tools/vgafixup.py
> @@ -0,0 +1,28 @@
> +#!/usr/bin/env python
> +# Work around x86emu bugs by replacing problematic instructions.
> +#
> +# Copyright (C) 2012  Kevin O'Connor 
> +#
> +# This file may be distributed under the terms of the GNU GPLv3 license.
>
> +import sys
>
> +def main():
> infilename, outfilename = sys.argv[1:]
> infile = open(infilename, 'rb')
> out = []
> for line in infile:
> sline = line.strip()
> if sline == 'ret':
> out.append('retw $2\n')
> elif sline == 'leave':
> out.append('movl %ebp, %esp ; popl %ebp\n')
> else:
> out.append(line)
> infile.close()
> outfile = open(outfilename, 'wb')
> outfile.write(''.join(out))
> outfile.close()
>
> +if __name__ == '__main__':
> main()
>
> diff --git a/vgasrc/clext.c b/vgasrc/clext.c
> index e5dce35..fc5459a 100644
> --- a/vgasrc/clext.c
> +++ b/vgasrc/clext.c
> @@ -526,7 +526,7 @@ ASM16(
>     "a0h_callback:"
>     "cli\n"
>     "hlt\n"
> "retf");
> "lretw");
>
> static void
>  clext_1012a0(struct bregs *regs)

Great job !

I just tried you patch and it seems to do the job.
I am not using the latest master branch though, so it would be good to have a
confirmation that it works fine by someone else.

Thanks for working so quickly. I think you can ship it.

PS: I identified why Xorg was complaining about the ROM checksum, and
recently posted a trivial patch to fix it. Could you have a look at this one ?

vgabios testing

[Gerd Hoffmann]

On 03/05/12 18:18, Kevin O'Connor wrote:
> I put together the below as a quick hack.  It boots to DOS okay.  I'm
> not sure the exact test case for x86emu and I didn't test that.  It's
> just a quick hack, but it should highlight the idea.

Gets Xorg going as-is, looks like it catches all critical instructions
are catched already.

cheers,
  Gerd