[gPXE-devel] [PATCH] Run COM32 programs with a valid IDT
Geoff Lywood
glywood at vmware.com
Thu Jul 1 14:35:11 EDT 2010
I tried compiling this against a different version of binutils, and the assembler complained about "movzxb". I guess the correct mnemonic for that instruction is "movzbl", so that should be changed before this patch gets committed. (Is there documentation anywhere that explains these sorts of subtle differences between Intel and AT&T syntax?)
Thanks,
Geoff
> -----Original Message-----
> From: Daniel Verkamp [mailto:daniel at drv.nu]
> Sent: Thursday, July 01, 2010 8:26 AM
> To: Stefan Hajnoczi
> Cc: H. Peter Anvin; Geoff Lywood; gpxe-devel at etherboot.org
> Subject: Re: [gPXE-devel] [PATCH] Run COM32 programs with a valid IDT
>
> This looks good to me after a first reading; I'm happy with it
> assuming it has been tested (sounds like it has if mboot.c32 now
> works).
>
> CC'd hpa: if you could take a glance and see if it looks okay, that
> would be helpful too.
>
> As a side note, it might be nice if an IDT for COM32 would be
> mentioned as part of the interface in the syslinux documentation. I
> mostly followed doc/comboot.txt in the initial implementation, so I
> missed this entirely.
>
> Thanks,
> -- Daniel Verkamp
>
> On Tue, Jun 29, 2010 at 12:23 AM, Stefan Hajnoczi <stefanha at gmail.com>
> wrote:
> > CCed Daniel Verkamp, who wrote the COMBOOT implementation.
> >
> > 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 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
> >>
More information about the gPXE-devel
mailing list