[gPXE-devel] [PATCH] Run COM32 programs with a valid IDT

Geoff Lywood glywood at vmware.com
Wed Jul 7 15:49:32 EDT 2010


This patch adds a fair amount of code size... ~300 bytes I think. Is there some kind of script I can run that will tell me exactly how much it has changed?

I can see some obvious optimizations I can do to eliminate some of this added code, and I'll send out a new patch soon.

Thanks,
Geoff


> -----Original Message-----
> From: Stefan Hajnoczi [mailto:stefanha at gmail.com]
> Sent: Wednesday, July 07, 2010 12:42 PM
> To: Geoff Lywood
> Cc: gpxe-devel at etherboot.org
> Subject: Re: [gPXE-devel] [PATCH] Run COM32 programs with a valid IDT
> 
> On Tue, Jun 29, 2010 at 2:18 AM, Geoff Lywood <glywood at vmware.com> wrote:
> > COM32 binaries generally expect to run with interrupts enabled. Syslinux
> does
> > so, and COM32 programs will execute cli/sti pairs when running a
> critical
> > section, to provide mutual exclusion against BIOS interrupt handlers.
> > Previously, under gPXE, the IDT was not valid, so any interrupt (e.g. a
> timer
> > tick) would generally cause the machine to triple fault.
> >
> > This change introduces code to:
> > - Create a valid IDT at the same location that syslinux uses
> > - Create an "interrupt jump buffer", which contains small pieces of code
> that
> >  simply record the vector number and jump to a common handler
> > - Thunk down to real mode and execute the BIOS's interrupt handler
> whenever
> >  an interrupt is received in a COM32 program
> > - Switch IDTs and enable/disable interrupts when context switching to
> and from
> >  COM32 binaries
> >
> > Testing done:
> > - Booted VMware ESX using a COM32 multiboot loader (mboot.c32)
> > - Built with GDBSERIAL enabled, and tested breakpoints on int22 and
> com32_irq
> > - Put the following code in a COM32 program:
> >    asm volatile ( "sti" );
> >    while ( 1 );
> >  Before this change, the machine would triple fault immediately. After
> this
> >  change, it hangs as expected. Under Bochs, it is possible to see the
> >  interrupt handler run, and the current time in the BIOS data area gets
> >  incremented.
> 
> I think we should merge this patch as it enables mboot.c32.  Thanks
> for describing the testing you have done.
> 
> I have skimmed the code but not reviewed in depth.
> 
> Stefan
> 
> > I have a feeling that Outlook is going to mangle this patch, so I'm
> attaching it as well.
> >
> > VMware, Inc. is providing this patch to you under the terms of the GPL
> version 2 and any later version. This patch is provided as is, with no
> warranties or support. VMware disclaims all liability in connection with
> the use/inability to use this patch. Any use of the attached is considered
> acceptance of the above.
> >
> > Signed-off-by: Geoff Lywood <glywood at vmware.com>
> > ---
> >  src/arch/i386/image/com32.c                      |   73
> +++++++++++++++++++++-
> >  src/arch/i386/include/comboot.h                  |   45 +++++++++++++
> >  src/arch/i386/interface/syslinux/com32_call.c    |   17 +++++
> >  src/arch/i386/interface/syslinux/com32_wrapper.S |   28 ++++++++
> >  4 files changed, 162 insertions(+), 1 deletions(-)
> >
> > diff --git a/src/arch/i386/image/com32.c b/src/arch/i386/image/com32.c
> > index 6ab347c..0f01027 100644
> > --- a/src/arch/i386/image/com32.c
> > +++ b/src/arch/i386/image/com32.c
> > @@ -42,6 +42,13 @@ FILE_LICENCE ( GPL2_OR_LATER );
> >
> >  struct image_type com32_image_type __image_type ( PROBE_NORMAL );
> >
> > +struct idt_register com32_external_idtr = {
> > +       .limit = COM32_NUM_IDT_ENTRIES * sizeof ( struct idt_descriptor
> ) - 1,
> > +       .base = COM32_IDT
> > +};
> > +
> > +struct idt_register com32_internal_idtr;
> > +
> >  /**
> >  * Execute COMBOOT image
> >  *
> > @@ -89,9 +96,12 @@ static int com32_exec ( struct image *image ) {
> >                unregister_image ( image );
> >
> >                __asm__ __volatile__ (
> > +                       "sidt com32_internal_idtr\n\t"
> > +                       "lidt com32_external_idtr\n\t"         /* Set up
> IDT */
> >                        "movl %%esp, (com32_internal_esp)\n\t" /* Save
> internal virtual address space ESP */
> >                        "movl (com32_external_esp), %%esp\n\t" /* Switch
> to COM32 ESP (top of available memory) */
> >                        "call _virt_to_phys\n\t"               /* Switch
> to flat physical address space */
> > +                       "sti\n\t"                              /* Enable
> interrupts */
> >                        "pushl %0\n\t"                         /* Pointer
> to CDECL helper function */
> >                        "pushl %1\n\t"                         /* Pointer
> to FAR call helper function */
> >                        "pushl %2\n\t"                         /* Size of
> low memory bounce buffer */
> > @@ -100,7 +110,9 @@ static int com32_exec ( struct image *image ) {
> >                        "pushl %5\n\t"                         /* Pointer
> to the command line arguments */
> >                        "pushl $6\n\t"                         /* Number
> of additional arguments */
> >                        "call *%6\n\t"                         /* Execute
> image */
> > -                       "call _phys_to_virt\n\t"               /* Switch
> back to internal virtual address space */
> > +                       "cli\n\t"                              /*
> Disable interrupts */
> > +                       "call _phys_to_virt\n\t"               /* Switch
> back to internal virtual address space */
> > +                       "lidt com32_internal_idtr\n\t"         /* Switch
> back to internal IDT (for debugging) */
> >                        "movl (com32_internal_esp), %%esp\n\t" /* Switch
> back to internal stack */
> >                :
> >                :
> > @@ -216,6 +228,60 @@ static int comboot_load_image ( struct image *image
> ) {
> >  }
> >
> >  /**
> > + * Prepare COM32 Interrupt Descriptor Table and Interrupt Jump Buffer
> > + * @v image            COM32 image
> > + * @ret rc             Return status code
> > + */
> > +static int comboot_prepare_idt ( struct image * image ) {
> > +       struct idt_descriptor *idt;
> > +       struct ijb_entry *ijb;
> > +       physaddr_t com32_irq_wrapper_phys;
> > +       userptr_t buffer;
> > +       size_t memsz;
> > +       int i, rc;
> > +
> > +       buffer = phys_to_user ( COM32_IDT );
> > +       memsz = COM32_NUM_IDT_ENTRIES * sizeof ( struct idt_descriptor
> );
> > +       rc = prep_segment ( buffer, memsz, memsz );
> > +       if ( rc != 0 ) {
> > +               DBGC ( image, "COM32 %p: could not prepare IDT segment:
> %s\n",
> > +                      image, strerror ( rc ) );
> > +               return rc;
> > +       }
> > +
> > +       buffer = phys_to_user ( COM32_IJB );
> > +       memsz = COM32_NUM_IDT_ENTRIES * sizeof ( struct ijb_entry );
> > +       rc = prep_segment ( buffer, memsz, memsz );
> > +       if ( rc != 0 ) {
> > +               DBGC ( image, "COM32 %p: could not prepare IJB segment:
> %s\n",
> > +                      image, strerror ( rc ) );
> > +               return rc;
> > +       }
> > +
> > +       idt = phys_to_virt ( COM32_IDT );
> > +       ijb = phys_to_virt ( COM32_IJB );
> > +       com32_irq_wrapper_phys = virt_to_phys ( com32_irq_wrapper );
> > +
> > +       for ( i = 0; i < COM32_NUM_IDT_ENTRIES; i++ ) {
> > +               uint32_t ijb_address = virt_to_phys ( &ijb[i] );
> > +
> > +               idt[i].offset_low = ijb_address & 0xFFFF;
> > +               idt[i].selector = PHYSICAL_CS;
> > +               idt[i].flags = IDT_INTERRUPT_GATE_FLAGS;
> > +               idt[i].offset_high = ijb_address >> 16;
> > +
> > +               ijb[i].pusha_instruction = IJB_PUSHA;
> > +               ijb[i].mov_instruction = IJB_MOV_AL_IMM8;
> > +               ijb[i].mov_value = i;
> > +               ijb[i].jump_instruction = IJB_JMP_REL32;
> > +               ijb[i].jump_destination = com32_irq_wrapper_phys -
> > +                       virt_to_phys ( &ijb[i + 1] );
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +/**
> >  * Prepare COM32 low memory bounce buffer
> >  * @v image            COM32 image
> >  * @ret rc             Return status code
> > @@ -269,6 +335,11 @@ static int com32_load ( struct image *image ) {
> >                return rc;
> >        }
> >
> > +       /* Set up IDT */
> > +       if ( ( rc = comboot_prepare_idt ( image ) ) != 0 ) {
> > +               return rc;
> > +       }
> > +
> >        /* Prepare bounce buffer segment */
> >        if ( ( rc = comboot_prepare_bounce_buffer ( image ) ) != 0 ) {
> >                return rc;
> > diff --git a/src/arch/i386/include/comboot.h
> b/src/arch/i386/include/comboot.h
> > index 1232f0a..ce6990d 100644
> > --- a/src/arch/i386/include/comboot.h
> > +++ b/src/arch/i386/include/comboot.h
> > @@ -13,6 +13,50 @@ FILE_LICENCE ( GPL2_OR_LATER );
> >  #include <setjmp.h>
> >  #include <gpxe/in.h>
> >
> > +/** Descriptor in a 32-bit IDT */
> > +struct idt_descriptor {
> > +       uint16_t offset_low;
> > +       uint16_t selector;
> > +       uint16_t flags;
> > +       uint16_t offset_high;
> > +} PACKED;
> > +
> > +/** Operand for the LIDT instruction */
> > +struct idt_register {
> > +       uint16_t limit;
> > +       uint32_t base;
> > +} PACKED;
> > +
> > +/** Entry in the interrupt jump buffer */
> > +struct ijb_entry {
> > +       uint8_t pusha_instruction;
> > +       uint8_t mov_instruction;
> > +       uint8_t mov_value;
> > +       uint8_t jump_instruction;
> > +       uint32_t jump_destination;
> > +} PACKED;
> > +
> > +/** The x86 opcode for "pushal" */
> > +#define IJB_PUSHA 0x60
> > +
> > +/** The x86 opcode for "movb $imm8,%al" */
> > +#define IJB_MOV_AL_IMM8 0xB0
> > +
> > +/** The x86 opcode for "jmp rel32" */
> > +#define IJB_JMP_REL32 0xE9
> > +
> > +/** Flags that specify a 32-bit interrupt gate with DPL=0 */
> > +#define IDT_INTERRUPT_GATE_FLAGS 0x8E00
> > +
> > +/** Address of COM32 interrupt descriptor table */
> > +#define COM32_IDT 0x100000
> > +
> > +/** Number of entries in a fully populated IDT */
> > +#define COM32_NUM_IDT_ENTRIES 256
> > +
> > +/** Address of COM32 interrupt jump buffer */
> > +#define COM32_IJB 0x100800
> > +
> >  /** Segment used for COMBOOT PSP and image */
> >  #define COMBOOT_PSP_SEG 0x07C0
> >
> > @@ -109,6 +153,7 @@ extern void unhook_comboot_interrupts ( );
> >  extern void com32_intcall_wrapper ( );
> >  extern void com32_farcall_wrapper ( );
> >  extern void com32_cfarcall_wrapper ( );
> > +extern void com32_irq_wrapper ( );
> >
> >  /* Resolve a hostname to an (IPv4) address */
> >  extern int comboot_resolv ( const char *name, struct in_addr *address
> );
> > diff --git a/src/arch/i386/interface/syslinux/com32_call.c
> b/src/arch/i386/interface/syslinux/com32_call.c
> > index d2c3f91..d271cc7 100644
> > --- a/src/arch/i386/interface/syslinux/com32_call.c
> > +++ b/src/arch/i386/interface/syslinux/com32_call.c
> > @@ -188,3 +188,20 @@ int __asmcall com32_cfarcall ( uint32_t proc,
> physaddr_t stack, size_t stacksz )
> >
> >        return eax;
> >  }
> > +
> > +/**
> > + * IRQ handler
> > + */
> > +void __asmcall com32_irq ( uint32_t vector ) {
> > +       uint32_t *ivt_entry = phys_to_virt( vector * 4 );
> > +
> > +       __asm__ __volatile__ (
> > +               REAL_CODE ( "pushfw\n\t"
> > +                           "pushw %%cs\n\t"
> > +                           "pushw $com32_irq_return\n\t"
> > +                           "pushl %0\n\t"
> > +                           "retf\n"
> > +                           "com32_irq_return:\n\t" )
> > +               : /* no outputs */
> > +               : "r" ( *ivt_entry ) );
> > +}
> > diff --git a/src/arch/i386/interface/syslinux/com32_wrapper.S
> b/src/arch/i386/interface/syslinux/com32_wrapper.S
> > index 5c5bd13..84d1571 100644
> > --- a/src/arch/i386/interface/syslinux/com32_wrapper.S
> > +++ b/src/arch/i386/interface/syslinux/com32_wrapper.S
> > @@ -22,6 +22,26 @@ FILE_LICENCE ( GPL2_OR_LATER )
> >        .arch i386
> >        .code32
> >
> > +       /*
> > +        * This code is entered after running the following sequence out
> of
> > +        * the interrupt jump buffer:
> > +        *
> > +        * pushal
> > +        * movb $vector, %al
> > +        * jmp com32_irq_wrapper
> > +        */
> > +
> > +       .globl com32_irq_wrapper
> > +com32_irq_wrapper:
> > +
> > +       movzxb %al,%eax
> > +       pushl %eax
> > +       movl $com32_irq, %eax
> > +       call com32_wrapper
> > +       popl %eax
> > +       popal
> > +       iret
> > +
> >        .globl com32_farcall_wrapper
> >  com32_farcall_wrapper:
> >
> > @@ -43,10 +63,14 @@ com32_intcall_wrapper:
> >        /*jmp com32_wrapper*/ /* fall through */
> >
> >  com32_wrapper:
> > +       cli
> >
> >        /* Switch to internal virtual address space */
> >        call _phys_to_virt
> >
> > +       /* Switch to internal IDT (if we have one for debugging) */
> > +       lidt com32_internal_idtr
> > +
> >        mov %eax, (com32_helper_function)
> >
> >        /* Save external COM32 stack pointer */
> > @@ -74,9 +98,13 @@ com32_wrapper:
> >        movl %esp, (com32_internal_esp)
> >        movl (com32_external_esp), %esp
> >
> > +       /* Switch to com32 IDT */
> > +       lidt com32_external_idtr
> > +
> >        /* Switch to external flat physical address space */
> >        call _virt_to_phys
> >
> > +       sti
> >        ret
> >
> >
> > _______________________________________________
> > gPXE-devel mailing list
> > gPXE-devel at etherboot.org
> > http://etherboot.org/mailman/listinfo/gpxe-devel
> >
> >


More information about the gPXE-devel mailing list