Carl-Daniel Hailfinger wrote:
On 18.02.2008 23:55, Marc Jones wrote:
Carl-Daniel Hailfinger wrote:
it seems that executing VSA requires vm86 to be useful. Since we
unconditionally execute the VSA, we should unconditionally require vm86
support (PCI_OPTION_ROM_RUN_VM86) via Kconfig for Geode targets. Not
doing so will either cause compile failures or runtime failures.

Adding
select PCI_OPTION_ROM_RUN_VM86
below
config CPU_AMD_GEODELX
did not work out for me.
Sorry I missed this.

VSA requires the GDT that is in vm86.c. VSA loads similar to an option ROM so the loader does go into VM86 mode. All the other stuff like interrupt support and PCI BIOS isn't needed by VSA. I think that the GDT at the top of vm86.c can be moved to a header file, gdt.h or something like that.

northbridge/amd/geodelx/vsmsetup.c uses util/x86emu/vm86.c:setup_realmode_idt() but it seems most/all of the setup there is not needed at all for VSA. Pulling in setup_realmode_idt pulls in the rest of vm86 through direct and indirect dependencies.

Care to make a patch? :)


I am also leaning towards removing the IDT for VSA init. There is a risk if either an exception happens or a software interrupt is used you will get unexpected results. What probably happens is that you jump off to something that will eventually cause a triple fault and reboot. You may think this is bad (and it is) but it is the same risk that coreboot runs today. If coreboot had a generic IDT to handle exceptions, VSA init would use the same IDT. Note that hardware INT (even timers) should never happen as they are always masked.

I have built with no PCI_OPTION_ROM_RUN_VM86 and run this to filo.

Marc







--
Marc Jones
Senior Firmware Engineer
(970) 226-9684 Office
mailto:[EMAIL PROTECTED]
http://www.amd.com/embeddedprocessors
Reduce the amount of compilation errors for Geode LX targets if x86emu 
or no emulation is selected instead of vm86.
Factor out GDT code from vm86.c to vm86_gdt.c
Remove IDT setup for VSA init.

Signed-off-by: Carl-Daniel Hailfinger <[EMAIL PROTECTED]>
Signed-of-by: Marc Jones <[EMAIL PROTECTED]>

Index: coreboot-v3/northbridge/amd/geodelx/Makefile
===================================================================
--- coreboot-v3.orig/northbridge/amd/geodelx/Makefile   2008-02-19 
10:46:51.000000000 -0700
+++ coreboot-v3/northbridge/amd/geodelx/Makefile        2008-02-19 
10:53:54.000000000 -0700
@@ -22,6 +22,7 @@
 ifeq ($(CONFIG_NORTHBRIDGE_AMD_GEODELX),y)
 
 STAGE2_CHIPSET_OBJ += $(obj)/northbridge/amd/geodelx/geodelx.o \
-                     $(obj)/northbridge/amd/geodelx/vsmsetup.o
+                     $(obj)/northbridge/amd/geodelx/vsmsetup.o \
+                     $(obj)/util/x86emu/vm86_gdt.o
 
 endif
Index: coreboot-v3/util/x86emu/pcbios/pcibios.c
===================================================================
--- coreboot-v3.orig/util/x86emu/pcbios/pcibios.c       2008-01-28 
16:34:03.000000000 -0700
+++ coreboot-v3/util/x86emu/pcbios/pcibios.c    2008-02-19 10:53:54.000000000 
-0700
@@ -61,7 +61,7 @@
                break;
        case FIND_PCI_DEVICE:
                /* FixME: support SI != 0 */
-               dev = dev_find_device(X86_DX, X86_CX, dev);
+               dev = dev_find_pci_device(X86_DX, X86_CX, dev);
                if (dev != 0) {
                        X86_BH = dev->bus->secondary;
                        X86_BL = dev->path.u.pci.devfn;
Index: coreboot-v3/util/x86emu/vm86_gdt.c
===================================================================
--- /dev/null   1970-01-01 00:00:00.000000000 +0000
+++ coreboot-v3/util/x86emu/vm86_gdt.c  2008-02-19 10:53:54.000000000 -0700
@@ -0,0 +1,95 @@
+/*
+ *  Erik Arjan Hendriks <[EMAIL PROTECTED]>
+ *  Copyright (C) 2000 Scyld.
+ *  Copyright (C) 2000 Scyld Computing Corporation
+ *  Copyright (C) 2001 University of California.  LA-CC Number 01-67.
+ *  Copyright (C) 2005 [EMAIL PROTECTED]
+ *  Copyright (C) 2007 coresystems GmbH
+ *
+ *  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
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ */
+
+
+/* Declare a temporary global descriptor table - 
+ * necessary because the core part of the bios 
+ * no longer sets up any 16 bit segments 
+ */
+
+__asm__ (
+       /* pointer to original gdt */
+       "       .globl gdtarg\n"
+       "gdtarg:                        \n"
+       "       .word   gdt_limit       \n"
+       "       .long   gdtptr          \n"             
+
+       /* compute the table limit */
+       "__mygdt_limit = __mygdt_end - __mygdt - 1      \n"
+       "       .globl __mygdtaddr\n"
+       "__mygdtaddr:                   \n"
+       "       .word   __mygdt_limit   \n"
+       "       .long   __mygdt         \n"
+
+       "       .globl __mygdt\n"
+       "__mygdt:                       \n"
+       /* selgdt 0, unused */
+       "       .word   0x0000, 0x0000  \n"
+       "       .byte   0x00, 0x00, 0x00, 0x00  \n"
+
+       /* selgdt 8, unused */
+       "       .word   0x0000, 0x0000          \n"
+       "       .byte   0x00, 0x00, 0x00, 0x00  \n"
+
+       /* selgdt 0x10, flat code segment */
+       "       .word   0xffff, 0x0000          \n"
+       "       .byte   0x00, 0x9b, 0xcf, 0x00  \n"     
+
+       /* selgdt 0x18, flat data segment */
+       "       .word   0xffff, 0x0000          \n"
+       "       .byte   0x00, 0x93, 0xcf, 0x00  \n"
+
+       /* selgdt 0x20, unused */
+       "       .word   0x0000, 0x0000          \n"
+       "       .byte   0x00, 0x00, 0x00, 0x00  \n"
+
+        /* selgdt 0x28 16-bit 64k code at 0x00000000 */
+       "       .word   0xffff, 0x0000          \n"
+       "       .byte   0, 0x9a, 0, 0           \n"
+
+       /* selgdt 0x30 16-bit 64k data at 0x00000000 */
+       "       .word   0xffff, 0x0000          \n"
+       "       .byte   0, 0x92, 0, 0           \n"
+
+       "__mygdt_end:                           \n"
+
+       /* FIXME: This does probably not belong here */
+       "       .globl idtarg\n"
+       "idtarg:\n"
+       "       .word   _idt_end - _idt - 1\n"     /* limit */
+       "       .long   _idt\n"
+       "       .word   0\n"
+       "_idt:\n"
+       "       .fill   20, 8, 0\n" //       # idt is unitiailzed
+       "_idt_end:\n"
+
+       /* Declare a pointer to where our idt is going to be i.e. at mem zero */
+       "       .globl __myidt\n"
+        "__myidt:              \n"
+        /* 16-bit limit */
+        "      .word 1023      \n"
+        /* 24-bit base */
+        "      .long 0         \n"
+        "      .word 0         \n"
+);
Index: coreboot-v3/util/x86emu/vm86.c
===================================================================
--- coreboot-v3.orig/util/x86emu/vm86.c 2008-02-13 15:52:10.000000000 -0700
+++ coreboot-v3/util/x86emu/vm86.c      2008-02-19 10:53:54.000000000 -0700
@@ -30,77 +30,6 @@
 #include <io.h>
 
 
-/* Declare a temporary global descriptor table - 
- * necessary because the core part of the bios 
- * no longer sets up any 16 bit segments 
- */
-
-__asm__ (
-       /* pointer to original gdt */
-       "       .globl gdtarg\n"
-       "gdtarg:                        \n"
-       "       .word   gdt_limit       \n"
-       "       .long   gdtptr          \n"             
-
-       /* compute the table limit */
-       "__mygdt_limit = __mygdt_end - __mygdt - 1      \n"
-       "       .globl __mygdtaddr\n"
-       "__mygdtaddr:                   \n"
-       "       .word   __mygdt_limit   \n"
-       "       .long   __mygdt         \n"
-
-       "       .globl __mygdt\n"
-       "__mygdt:                       \n"
-       /* selgdt 0, unused */
-       "       .word   0x0000, 0x0000  \n"
-       "       .byte   0x00, 0x00, 0x00, 0x00  \n"
-
-       /* selgdt 8, unused */
-       "       .word   0x0000, 0x0000          \n"
-       "       .byte   0x00, 0x00, 0x00, 0x00  \n"
-
-       /* selgdt 0x10, flat code segment */
-       "       .word   0xffff, 0x0000          \n"
-       "       .byte   0x00, 0x9b, 0xcf, 0x00  \n"     
-
-       /* selgdt 0x18, flat data segment */
-       "       .word   0xffff, 0x0000          \n"
-       "       .byte   0x00, 0x93, 0xcf, 0x00  \n"
-
-       /* selgdt 0x20, unused */
-       "       .word   0x0000, 0x0000          \n"
-       "       .byte   0x00, 0x00, 0x00, 0x00  \n"
-
-        /* selgdt 0x28 16-bit 64k code at 0x00000000 */
-       "       .word   0xffff, 0x0000          \n"
-       "       .byte   0, 0x9a, 0, 0           \n"
-
-       /* selgdt 0x30 16-bit 64k data at 0x00000000 */
-       "       .word   0xffff, 0x0000          \n"
-       "       .byte   0, 0x92, 0, 0           \n"
-
-       "__mygdt_end:                           \n"
-
-       /* FIXME: This does probably not belong here */
-       "       .globl idtarg\n"
-       "idtarg:\n"
-       "       .word   _idt_end - _idt - 1\n"     /* limit */
-       "       .long   _idt\n"
-       "       .word   0\n"
-       "_idt:\n"
-       "       .fill   20, 8, 0\n" //       # idt is unitiailzed
-       "_idt_end:\n"
-
-       /* Declare a pointer to where our idt is going to be i.e. at mem zero */
-       "       .globl __myidt\n"
-        "__myidt:              \n"
-        /* 16-bit limit */
-        "      .word 1023      \n"
-        /* 24-bit base */
-        "      .long 0         \n"
-        "      .word 0         \n"
-);
-
 /* The address arguments to this function are PHYSICAL ADDRESSES */ 
 static void real_mode_switch_call_vga(unsigned long devfn)
 {
Index: coreboot-v3/util/x86emu/Makefile
===================================================================
--- coreboot-v3.orig/util/x86emu/Makefile       2008-01-28 16:34:04.000000000 
-0700
+++ coreboot-v3/util/x86emu/Makefile    2008-02-19 10:53:54.000000000 -0700
@@ -20,7 +20,7 @@
 
 X86EMU_OBJ  = debug.o decode.o fpu.o ops.o ops2.o prim_ops.o sys.o
 BIOSEMU_OBJ = biosemu.o pcbios/pcibios.o
-VM86_OBJ    = vm86.o
+VM86_OBJ    = vm86.o vm86_gdt.o
 
 ifeq ($(CONFIG_PCI_OPTION_ROM_RUN_X86EMU),y)
 LIBX86EMU_OBJS = $(patsubst %,$(obj)/util/x86emu/x86emu/%,$(X86EMU_OBJ)) \
Index: coreboot-v3/northbridge/amd/geodelx/geodelx.c
===================================================================
--- coreboot-v3.orig/northbridge/amd/geodelx/geodelx.c  2008-02-19 
11:48:15.000000000 -0700
+++ coreboot-v3/northbridge/amd/geodelx/geodelx.c       2008-02-19 
14:34:28.000000000 -0700
@@ -30,7 +30,6 @@
 extern void chipsetinit(void);
 extern u32 get_systop(void);
 extern void northbridge_init_early(void);
-extern void setup_realmode_idt(void);
 
 
 /**
@@ -155,8 +154,6 @@
 //     northbridge_init_early();
        chipsetinit();
 
-       setup_realmode_idt();
-
        printk(BIOS_SPEW, "Before VSA:\n");
        /* print_conf(); */
        /* Do the magic stuff here, so prepare your tambourine ;) */
Index: coreboot-v3/northbridge/amd/geodelx/vsmsetup.c
===================================================================
--- coreboot-v3.orig/northbridge/amd/geodelx/vsmsetup.c 2008-02-19 
14:12:21.000000000 -0700
+++ coreboot-v3/northbridge/amd/geodelx/vsmsetup.c      2008-02-19 
14:13:40.000000000 -0700
@@ -89,10 +89,6 @@
                "       mov     %%ax, %%ss      \n"
                "       movl    $0x1000, %%eax  \n"
                "       movl    %%eax, %%esp    \n"
-               /* Load our 16 it idt */
-               "       xor     %%ax, %%ax      \n"
-               "       mov     %%ax, %%ds      \n"
-               "       lidt    __myidt         \n"
                /* Dump zeros in the other segregs */
                "       mov     %%ax, %%es      \n"
                /* FixMe: Big real mode for gs, fs? */
@@ -120,9 +116,8 @@
                "       mov     %%ax, %%fs      \n"
                "       mov     %%ax, %%gs      \n"
                "       mov     %%ax, %%ss      \n"
-               /* restore proper gdt and idt */
+               /* restore proper gdt */
                "       lgdt    %%cs:gdtarg     \n"
-               "       lidt    idtarg          \n"
                ".globl vsm_exit                \n"
                "vsm_exit:                      \n"
                "       mov     __stack, %%esp  \n"
-- 
coreboot mailing list
[email protected]
http://www.coreboot.org/mailman/listinfo/coreboot

Reply via email to