[gPXE-devel] [PATCH] Run COM32 programs with a valid IDT
Daniel Verkamp
daniel at drv.nu
Thu Jul 1 11:26:04 EDT 2010
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