From 8904cd55f128941d53d9a8beef71fb32a920a92d Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Tue, 17 Feb 2009 00:47:35 +0000 Subject: [comboot] Allow for tail recursion of COMBOOT images Multi-level menus via COMBOOT rely on the COMBOOT program being able to exit and invoke a new COMBOOT program (the next menu). This works, but rapidly (within about five iterations) runs out of space in gPXE's internal stack, since each new image is executed in a new function context. Fix by allowing tail recursion between images; an image can now specify a replacement image for itself, and image_exec() will perform the necessary tail recursion. --- src/arch/i386/image/com32.c | 40 ++++--- src/arch/i386/image/comboot.c | 40 ++++--- src/arch/i386/include/bits/errfile.h | 1 + src/arch/i386/include/comboot.h | 14 +-- src/arch/i386/interface/syslinux/comboot_call.c | 143 ++++++++++++++---------- src/core/image.c | 30 ++++- src/image/script.c | 5 +- src/include/gpxe/image.h | 14 +++ 8 files changed, 176 insertions(+), 111 deletions(-) diff --git a/src/arch/i386/image/com32.c b/src/arch/i386/image/com32.c index da604625..27ccbc02 100644 --- a/src/arch/i386/image/com32.c +++ b/src/arch/i386/image/com32.c @@ -70,20 +70,20 @@ static int com32_exec ( struct image *image ) { } DBGC ( image, "COM32 %p: available memory top = 0x%x\n", - image, (int)avail_mem_top ); + image, avail_mem_top ); assert ( avail_mem_top != 0 ); com32_external_esp = phys_to_virt ( avail_mem_top ); /* Hook COMBOOT API interrupts */ - hook_comboot_interrupts( ); + hook_comboot_interrupts(); - /* Temporarily de-register image, so that a "boot" command - * doesn't throw us into an execution loop. Hold a reference - * to avoid the image's being freed. + /* Unregister image, so that a "boot" command doesn't + * throw us into an execution loop. We never + * reregister ourselves; COMBOOT images expect to be + * removed on exit. */ - image_get ( image ); unregister_image ( image ); __asm__ __volatile__ ( @@ -111,25 +111,31 @@ static int com32_exec ( struct image *image ) { /* %6 */ "r" ( COM32_START_PHYS ) : "memory" ); + DBGC ( image, "COM32 %p: returned\n", image ); break; - case COMBOOT_RETURN_RUN_KERNEL: - DBGC ( image, "COM32 %p: returned to run kernel...\n", image ); - comboot_run_kernel ( ); + case COMBOOT_EXIT: + DBGC ( image, "COM32 %p: exited\n", image ); break; - case COMBOOT_RETURN_EXIT: + case COMBOOT_EXIT_RUN_KERNEL: + DBGC ( image, "COM32 %p: exited to run kernel %p\n", + image, comboot_replacement_image ); + image->replacement = comboot_replacement_image; + image_autoload ( image->replacement ); break; - } - - comboot_force_text_mode ( ); + case COMBOOT_EXIT_COMMAND: + DBGC ( image, "COM32 %p: exited after executing command\n", + image ); + break; - DBGC ( image, "COM32 %p returned\n", image ); + default: + assert ( 0 ); + break; + } - /* Re-register image and return */ - register_image ( image ); - image_put ( image ); + comboot_force_text_mode(); return 0; } diff --git a/src/arch/i386/image/comboot.c b/src/arch/i386/image/comboot.c index 63d02c0f..3ca9d28f 100644 --- a/src/arch/i386/image/comboot.c +++ b/src/arch/i386/image/comboot.c @@ -142,16 +142,16 @@ static int comboot_exec ( struct image *image ) { comboot_init_psp ( image, seg_userptr ); /* Hook COMBOOT API interrupts */ - hook_comboot_interrupts ( ); + hook_comboot_interrupts(); DBGC ( image, "executing 16-bit COMBOOT image at %4x:0100\n", - COMBOOT_PSP_SEG ); + COMBOOT_PSP_SEG ); - /* Temporarily de-register image, so that a "boot" command - * doesn't throw us into an execution loop. Hold a reference - * to avoid the image's being freed. + /* Unregister image, so that a "boot" command doesn't + * throw us into an execution loop. We never + * reregister ourselves; COMBOOT images expect to be + * removed on exit. */ - image_get ( image ); unregister_image ( image ); /* Store stack segment at 0x38 and stack pointer at 0x3A @@ -180,26 +180,32 @@ static int comboot_exec ( struct image *image ) { "xorw %%bp, %%bp\n\t" "lret\n\t" ) : : "r" ( COMBOOT_PSP_SEG ) : "eax" ); + DBGC ( image, "COMBOOT %p: returned\n", image ); break; - case COMBOOT_RETURN_RUN_KERNEL: - DBGC ( image, "COMBOOT %p: returned to run kernel...\n", image ); - comboot_run_kernel ( ); + case COMBOOT_EXIT: + DBGC ( image, "COMBOOT %p: exited\n", image ); break; - case COMBOOT_RETURN_EXIT: + case COMBOOT_EXIT_RUN_KERNEL: + DBGC ( image, "COMBOOT %p: exited to run kernel %p\n", + image, comboot_replacement_image ); + image->replacement = comboot_replacement_image; + image_autoload ( image->replacement ); break; - } + case COMBOOT_EXIT_COMMAND: + DBGC ( image, "COMBOOT %p: exited after executing command\n", + image ); + break; - comboot_force_text_mode ( ); + default: + assert ( 0 ); + break; + } - DBGC ( image, "COMBOOT %p returned\n", image ); + comboot_force_text_mode(); - /* Re-register image and return */ - register_image ( image ); - image_put ( image ); - return 0; } diff --git a/src/arch/i386/include/bits/errfile.h b/src/arch/i386/include/bits/errfile.h index 1723063b..5ea8a318 100644 --- a/src/arch/i386/include/bits/errfile.h +++ b/src/arch/i386/include/bits/errfile.h @@ -23,6 +23,7 @@ #define ERRFILE_comboot ( ERRFILE_ARCH | ERRFILE_IMAGE | 0x00070000 ) #define ERRFILE_com32 ( ERRFILE_ARCH | ERRFILE_IMAGE | 0x00080000 ) #define ERRFILE_comboot_resolv ( ERRFILE_ARCH | ERRFILE_IMAGE | 0x00090000 ) +#define ERRFILE_comboot_call ( ERRFILE_ARCH | ERRFILE_IMAGE | 0x000a0000 ) #define ERRFILE_undi ( ERRFILE_ARCH | ERRFILE_NET | 0x00000000 ) #define ERRFILE_undiload ( ERRFILE_ARCH | ERRFILE_NET | 0x00010000 ) diff --git a/src/arch/i386/include/comboot.h b/src/arch/i386/include/comboot.h index 1fc3b718..4376650f 100644 --- a/src/arch/i386/include/comboot.h +++ b/src/arch/i386/include/comboot.h @@ -79,18 +79,14 @@ extern int comboot_resolv ( const char *name, struct in_addr *address ); /* setjmp/longjmp context buffer used to return after loading an image */ extern jmp_buf comboot_return; -/* Command line to execute when returning via comboot_return - * with COMBOOT_RETURN_RUN_KERNEL - */ -extern char *comboot_kernel_cmdline; - -/* Execute comboot_image_cmdline */ -extern void comboot_run_kernel ( ); +/* Replacement image when exiting with COMBOOT_EXIT_RUN_KERNEL */ +extern struct image *comboot_replacement_image; extern void *com32_external_esp; -#define COMBOOT_RETURN_EXIT 1 -#define COMBOOT_RETURN_RUN_KERNEL 2 +#define COMBOOT_EXIT 1 +#define COMBOOT_EXIT_RUN_KERNEL 2 +#define COMBOOT_EXIT_COMMAND 3 extern void comboot_force_text_mode ( void ); diff --git a/src/arch/i386/interface/syslinux/comboot_call.c b/src/arch/i386/interface/syslinux/comboot_call.c index 977d44f3..02b2250b 100644 --- a/src/arch/i386/interface/syslinux/comboot_call.c +++ b/src/arch/i386/interface/syslinux/comboot_call.c @@ -35,6 +35,8 @@ #include #include #include +#include +#include /** The "SYSLINUX" version string */ static char __data16_array ( syslinux_version, [] ) = "gPXE " VERSION; @@ -67,10 +69,8 @@ extern void int22_wrapper ( void ); /* setjmp/longjmp context buffer used to return after loading an image */ jmp_buf comboot_return; -/* Command line to execute when returning via comboot_return - * with COMBOOT_RETURN_RUN_KERNEL - */ -char *comboot_kernel_cmdline; +/* Replacement image when exiting with COMBOOT_EXIT_RUN_KERNEL */ +struct image *comboot_replacement_image; /* Mode flags set by INT 22h AX=0017h */ static uint16_t comboot_graphics_mode = 0; @@ -154,58 +154,81 @@ void comboot_force_text_mode ( void ) { /** - * Run the kernel specified in comboot_kernel_cmdline + * Fetch kernel and optional initrd */ -void comboot_run_kernel ( ) -{ - char *initrd; - - comboot_force_text_mode ( ); - - DBG ( "COMBOOT: executing image '%s'\n", comboot_kernel_cmdline ); +static int comboot_fetch_kernel ( char *kernel_file, char *cmdline ) { + struct image *kernel; + struct image *initrd = NULL; + char *initrd_file; + int rc; /* Find initrd= parameter, if any */ - if ( ( initrd = strstr ( comboot_kernel_cmdline, "initrd=" ) ) ) { - char old_char = '\0'; - char *initrd_end = strchr( initrd, ' ' ); - - /* Replace space after end of parameter - * with a nul terminator if this is not - * the last parameter - */ - if ( initrd_end ) { - old_char = *initrd_end; - *initrd_end = '\0'; - } + if ( ( initrd_file = strstr ( cmdline, "initrd=" ) ) != NULL ) { + char *initrd_end; - /* Replace = with space to get 'initrd filename' - * command suitable for system() - */ - initrd[6] = ' '; + /* skip "initrd=" */ + initrd_file += 7; - DBG( "COMBOOT: loading initrd '%s'\n", initrd ); + /* Find terminating space, if any, and replace with NUL */ + initrd_end = strchr ( initrd_file, ' ' ); + if ( initrd_end ) + *initrd_end = '\0'; - system ( initrd ); + DBG ( "COMBOOT: fetching initrd '%s'\n", initrd_file ); - /* Restore space after parameter */ - if ( initrd_end ) { - *initrd_end = old_char; + /* Allocate and fetch initrd */ + initrd = alloc_image(); + if ( ! initrd ) { + DBG ( "COMBOOT: could not allocate initrd\n" ); + rc = -ENOMEM; + goto err_alloc_initrd; + } + if ( ( rc = imgfetch ( initrd, initrd_file, + register_image ) ) != 0 ) { + DBG ( "COMBOOT: could not fetch initrd: %s\n", + strerror ( rc ) ); + goto err_fetch_initrd; } - /* Restore = */ - initrd[6] = '='; + /* Restore space after initrd name, if applicable */ + if ( initrd_end ) + *initrd_end = ' '; } - /* Load kernel */ - DBG ( "COMBOOT: loading kernel '%s'\n", comboot_kernel_cmdline ); - system ( comboot_kernel_cmdline ); + DBG ( "COMBOOT: fetching kernel '%s'\n", kernel_file ); - free ( comboot_kernel_cmdline ); + /* Allocate and fetch kernel */ + kernel = alloc_image(); + if ( ! kernel ) { + DBG ( "COMBOOT: could not allocate kernel\n" ); + rc = -ENOMEM; + goto err_alloc_kernel; + } + if ( ( rc = imgfetch ( kernel, kernel_file, + register_image ) ) != 0 ) { + DBG ( "COMBOOT: could not fetch kernel: %s\n", + strerror ( rc ) ); + goto err_fetch_kernel; + } + if ( ( rc = image_set_cmdline ( kernel, cmdline ) ) != 0 ) { + DBG ( "COMBOOT: could not set kernel command line: %s\n", + strerror ( rc ) ); + goto err_set_cmdline; + } - /* Boot */ - system ( "boot" ); + /* Store kernel as replacement image */ + comboot_replacement_image = kernel; - DBG ( "COMBOOT: back from executing command\n" ); + return 0; + + err_set_cmdline: + err_fetch_kernel: + image_put ( kernel ); + err_alloc_kernel: + err_fetch_initrd: + image_put ( initrd ); + err_alloc_initrd: + return rc; } @@ -213,7 +236,7 @@ void comboot_run_kernel ( ) * Terminate program interrupt handler */ static __asmcall void int20 ( struct i386_all_regs *ix86 __unused ) { - longjmp ( comboot_return, COMBOOT_RETURN_EXIT ); + longjmp ( comboot_return, COMBOOT_EXIT ); } @@ -226,7 +249,7 @@ static __asmcall void int21 ( struct i386_all_regs *ix86 ) { switch ( ix86->regs.ah ) { case 0x00: case 0x4C: /* Terminate program */ - longjmp ( comboot_return, COMBOOT_RETURN_EXIT ); + longjmp ( comboot_return, COMBOOT_EXIT ); break; case 0x01: /* Get Key with Echo */ @@ -323,17 +346,15 @@ static __asmcall void int22 ( struct i386_all_regs *ix86 ) { char cmd[len + 1]; copy_from_user ( cmd, cmd_u, 0, len + 1 ); DBG ( "COMBOOT: executing command '%s'\n", cmd ); - - comboot_kernel_cmdline = strdup ( cmd ); - - DBG ( "COMBOOT: returning to run image...\n" ); - longjmp ( comboot_return, COMBOOT_RETURN_RUN_KERNEL ); + system ( cmd ); + DBG ( "COMBOOT: exiting after executing command...\n" ); + longjmp ( comboot_return, COMBOOT_EXIT_COMMAND ); } break; case 0x0004: /* Run default command */ /* FIXME: just exit for now */ - longjmp ( comboot_return, COMBOOT_RETURN_EXIT ); + longjmp ( comboot_return, COMBOOT_EXIT_COMMAND ); break; case 0x0005: /* Force text mode */ @@ -518,21 +539,21 @@ static __asmcall void int22 ( struct i386_all_regs *ix86 ) { userptr_t cmd_u = real_to_user ( ix86->segs.es, ix86->regs.bx ); int file_len = strlen_user ( file_u, 0 ); int cmd_len = strlen_user ( cmd_u, 0 ); - char file[file_len + 1 + cmd_len + 7 + 1]; + char file[file_len + 1]; char cmd[cmd_len + 1]; - memcpy( file, "kernel ", 7 ); - copy_from_user ( file + 7, file_u, 0, file_len + 1 ); + copy_from_user ( file, file_u, 0, file_len + 1 ); copy_from_user ( cmd, cmd_u, 0, cmd_len + 1 ); - strcat ( file, " " ); - strcat ( file, cmd ); - - DBG ( "COMBOOT: run kernel image '%s'\n", file ); - comboot_kernel_cmdline = strdup ( file ); - - DBG ( "COMBOOT: returning to run image...\n" ); - longjmp ( comboot_return, COMBOOT_RETURN_RUN_KERNEL ); + DBG ( "COMBOOT: run kernel %s %s\n", file, cmd ); + comboot_fetch_kernel ( file, cmd ); + /* Technically, we should return if we + * couldn't load the kernel, but it's not safe + * to do that since we have just overwritten + * part of the COMBOOT program's memory space. + */ + DBG ( "COMBOOT: exiting to run kernel...\n" ); + longjmp ( comboot_return, COMBOOT_EXIT_RUN_KERNEL ); } break; diff --git a/src/core/image.c b/src/core/image.c index 440a68c9..741b0547 100644 --- a/src/core/image.c +++ b/src/core/image.c @@ -53,6 +53,7 @@ static void free_image ( struct refcnt *refcnt ) { uri_put ( image->uri ); ufree ( image->data ); + image_put ( image->replacement ); free ( image ); DBGC ( image, "IMAGE %p freed\n", image ); } @@ -142,9 +143,9 @@ int register_image ( struct image *image ) { * @v image Executable/loadable image */ void unregister_image ( struct image *image ) { + DBGC ( image, "IMAGE %p unregistered\n", image ); list_del ( &image->list ); image_put ( image ); - DBGC ( image, "IMAGE %p unregistered\n", image ); } /** @@ -237,6 +238,7 @@ int image_autoload ( struct image *image ) { * @ret rc Return status code */ int image_exec ( struct image *image ) { + struct image *replacement; struct uri *old_cwuri; int rc; @@ -257,18 +259,40 @@ int image_exec ( struct image *image ) { old_cwuri = uri_get ( cwuri ); churi ( image->uri ); + /* Take out a temporary reference to the image. This allows + * the image to unregister itself if necessary, without + * automatically freeing itself. + */ + image_get ( image ); + /* Try executing the image */ if ( ( rc = image->type->exec ( image ) ) != 0 ) { DBGC ( image, "IMAGE %p could not execute: %s\n", image, strerror ( rc ) ); - goto done; + /* Do not return yet; we still have clean-up to do */ } - done: + /* Pick up replacement image before we drop the original + * image's temporary reference. + */ + if ( ( replacement = image->replacement ) != NULL ) + image_get ( replacement ); + + /* Drop temporary reference to the original image */ + image_put ( image ); + /* Reset current working directory */ churi ( old_cwuri ); uri_put ( old_cwuri ); + /* Tail-recurse into replacement image, if one exists */ + if ( replacement ) { + DBGC ( image, "IMAGE %p replacing self with IMAGE %p\n", + image, replacement ); + if ( ( rc = image_exec ( replacement ) ) != 0 ) + return rc; + } + return rc; } diff --git a/src/image/script.c b/src/image/script.c index fe722885..2d242746 100644 --- a/src/image/script.c +++ b/src/image/script.c @@ -43,10 +43,8 @@ static int script_exec ( struct image *image ) { int rc; /* Temporarily de-register image, so that a "boot" command - * doesn't throw us into an execution loop. Hold a reference - * to avoid the image's being freed. + * doesn't throw us into an execution loop. */ - image_get ( image ); unregister_image ( image ); while ( offset < image->len ) { @@ -80,7 +78,6 @@ static int script_exec ( struct image *image ) { done: /* Re-register image and return */ register_image ( image ); - image_put ( image ); return rc; } diff --git a/src/include/gpxe/image.h b/src/include/gpxe/image.h index 76dc3b8f..0163b082 100644 --- a/src/include/gpxe/image.h +++ b/src/include/gpxe/image.h @@ -46,6 +46,16 @@ struct image { userptr_t user; unsigned long ul; } priv; + + /** Replacement image + * + * An image wishing to replace itself with another image (in a + * style similar to a Unix exec() call) should return from its + * exec() method with the replacement image set to point to + * the new image. The new image must already be in a suitable + * state for execution. + */ + struct image *replacement; }; /** Image is loaded */ @@ -79,6 +89,10 @@ struct image_type { * * @v image Loaded image * @ret rc Return status code + * + * Note that the image may be invalidated by the act of + * execution, i.e. an image is allowed to choose to unregister + * (and so potentially free) itself. */ int ( * exec ) ( struct image *image ); }; -- cgit v1.2.3-55-g7522