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

Stefan Hajnoczi stefanha at gmail.com
Wed Jul 7 16:06:18 EDT 2010


On Wed, Jul 7, 2010 at 8:49 PM, Geoff Lywood <glywood at vmware.com> wrote:
> 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?

$ size bin/com32.o

or

$ make bin/gpxe.hd.sizes

or

util/diffsize.pl

Take your pick!

Stefan

> 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