[image] Simplify image management

Refactor the {load,exec} image operations as {probe,exec}.  This makes
the probe mechanism cleaner, eliminates some forward declarations,
avoids holding magic state in image->priv, eliminates the possibility
of screwing up between the "load" and "exec" stages, and makes the
documentation simpler since the concept of "loading" (as distinct from
"executing") no longer needs to be explained.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

src/arch/i386/image/bzimage.c

@@ -41,8 +41,6 @@ FILE_LICENCE ( GPL2_OR_LATER );
 
 FEATURE ( FEATURE_IMAGE, "bzImage", DHCP_EB_FEATURE_BZIMAGE, 1 );
 
-struct image_type bzimage_image_type __image_type ( PROBE_NORMAL );
-
 /**
  * bzImage context
  */
@@ -111,8 +109,8 @@ static int bzimage_parse_header ( struct image *image,
 			 sizeof ( bzimg->bzhdr ) );
 
 	/* Calculate size of real-mode portion */
-	bzimg->rm_filesz =
-		( ( bzimg->bzhdr.setup_sects ? bzimg->bzhdr.setup_sects : 4 ) + 1 ) << 9;
+	bzimg->rm_filesz = ( ( ( bzimg->bzhdr.setup_sects ?
+				 bzimg->bzhdr.setup_sects : 4 ) + 1 ) << 9 );
 	if ( bzimg->rm_filesz > image->len ) {
 		DBGC ( image, "bzImage %p too short for %zd byte of setup\n",
 		       image, bzimg->rm_filesz );
@@ -455,11 +453,33 @@ static int bzimage_exec ( struct image *image ) {
 	const char *cmdline = ( image->cmdline ? image->cmdline : "" );
 	int rc;
 
-	/* Read and parse header from loaded kernel */
+	/* Read and parse header from image */
 	if ( ( rc = bzimage_parse_header ( image, &bzimg,
-					   image->priv.user ) ) != 0 )
+					   image->data ) ) != 0 )
+		return rc;
+
+	/* Prepare segments */
+	if ( ( rc = prep_segment ( bzimg.rm_kernel, bzimg.rm_filesz,
+				   bzimg.rm_memsz ) ) != 0 ) {
+		DBGC ( image, "bzImage %p could not prepare RM segment: %s\n",
+		       image, strerror ( rc ) );
+		return rc;
+	}
+	if ( ( rc = prep_segment ( bzimg.pm_kernel, bzimg.pm_sz,
+				   bzimg.pm_sz ) ) != 0 ) {
+		DBGC ( image, "bzImage %p could not prepare PM segment: %s\n",
+		       image, strerror ( rc ) );
 		return rc;
-	assert ( bzimg.rm_kernel == image->priv.user );
+	}
+
+	/* Load segments */
+	memcpy_user ( bzimg.rm_kernel, 0, image->data,
+		      0, bzimg.rm_filesz );
+	memcpy_user ( bzimg.pm_kernel, 0, image->data,
+		      bzimg.rm_filesz, bzimg.pm_sz );
+
+	/* Update and write out header */
+	bzimage_update_header ( image, &bzimg, bzimg.rm_kernel );
 
 	/* Parse command line for bootloader parameters */
 	if ( ( rc = bzimage_parse_cmdline ( image, &bzimg, cmdline ) ) != 0)
@@ -506,12 +526,12 @@ static int bzimage_exec ( struct image *image ) {
 }
 
 /**
- * Load bzImage image into memory
+ * Probe bzImage image
  *
  * @v image		bzImage file
  * @ret rc		Return status code
  */
-int bzimage_load ( struct image *image ) {
+int bzimage_probe ( struct image *image ) {
 	struct bzimage_context bzimg;
 	int rc;
 
@@ -520,42 +540,12 @@ int bzimage_load ( struct image *image ) {
 					   image->data ) ) != 0 )
 		return rc;
 
-	/* This is a bzImage image, valid or otherwise */
-	if ( ! image->type )
-		image->type = &bzimage_image_type;
-
-	/* Prepare segments */
-	if ( ( rc = prep_segment ( bzimg.rm_kernel, bzimg.rm_filesz,
-				   bzimg.rm_memsz ) ) != 0 ) {
-		DBGC ( image, "bzImage %p could not prepare RM segment: %s\n",
-		       image, strerror ( rc ) );
-		return rc;
-	}
-	if ( ( rc = prep_segment ( bzimg.pm_kernel, bzimg.pm_sz,
-				   bzimg.pm_sz ) ) != 0 ) {
-		DBGC ( image, "bzImage %p could not prepare PM segment: %s\n",
-		       image, strerror ( rc ) );
-		return rc;
-	}
-
-	/* Load segments */
-	memcpy_user ( bzimg.rm_kernel, 0, image->data,
-		      0, bzimg.rm_filesz );
-	memcpy_user ( bzimg.pm_kernel, 0, image->data,
-		      bzimg.rm_filesz, bzimg.pm_sz );
-
-	/* Update and write out header */
-	bzimage_update_header ( image, &bzimg, bzimg.rm_kernel );
-
-	/* Record real-mode segment in image private data field */
-	image->priv.user = bzimg.rm_kernel;
-
 	return 0;
 }
 
 /** Linux bzImage image type */
 struct image_type bzimage_image_type __image_type ( PROBE_NORMAL ) = {
 	.name = "bzImage",
-	.load = bzimage_load,
+	.probe = bzimage_probe,
 	.exec = bzimage_exec,
 };

src/arch/i386/image/com32.c

@@ -40,8 +40,6 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #include <ipxe/init.h>
 #include <ipxe/io.h>
 
-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
@@ -55,7 +53,7 @@ struct idt_register com32_internal_idtr;
  * @v image		COM32 image
  * @ret rc		Return status code
  */
-static int com32_exec ( struct image *image ) {
+static int com32_exec_loop ( struct image *image ) {
 	struct memory_map memmap;
 	unsigned int i;
 	int state;
@@ -137,7 +135,6 @@ static int com32_exec ( struct image *image ) {
 		       image, comboot_replacement_image );
 		image->replacement = comboot_replacement_image;
 		comboot_replacement_image = NULL;
-		image_autoload ( image->replacement );
 		break;
 
 	case COMBOOT_EXIT_COMMAND:
@@ -207,7 +204,7 @@ static int com32_identify ( struct image *image ) {
  * @v image		COM32 image
  * @ret rc		Return status code
  */
-static int comboot_load_image ( struct image *image ) {
+static int com32_load_image ( struct image *image ) {
 	physaddr_t com32_irq_wrapper_phys;
 	struct idt_descriptor *idt;
 	struct ijb_entry *ijb;
@@ -262,7 +259,7 @@ static int comboot_load_image ( struct image *image ) {
  * @v image		COM32 image
  * @ret rc		Return status code
  */
-static int comboot_prepare_bounce_buffer ( struct image * image ) {
+static int com32_prepare_bounce_buffer ( struct image * image ) {
 	unsigned int seg;
 	userptr_t seg_userptr;
 	size_t filesz, memsz;
@@ -286,12 +283,12 @@ static int comboot_prepare_bounce_buffer ( struct image * image ) {
 }
 
 /**
- * Load COM32 image into memory
+ * Probe COM32 image
  *
  * @v image		COM32 image
  * @ret rc		Return status code
  */
-static int com32_load ( struct image *image ) {
+static int com32_probe ( struct image *image ) {
 	int rc;
 
 	DBGC ( image, "COM32 %p: name '%s', cmdline '%s'\n",
@@ -302,26 +299,34 @@ static int com32_load ( struct image *image ) {
 		return rc;
 	}
 
-	/* This is a COM32 image, valid or otherwise */
-	if ( ! image->type )
-		image->type = &com32_image_type;
+	return 0;
+}
+
+/**
+ * Execute COMBOOT image
+ *
+ * @v image		COM32 image
+ * @ret rc		Return status code
+ */
+static int com32_exec ( struct image *image ) {
+	int rc;
 
 	/* Load image */
-	if ( ( rc = comboot_load_image ( image ) ) != 0 ) {
+	if ( ( rc = com32_load_image ( image ) ) != 0 ) {
 		return rc;
 	}
 
 	/* Prepare bounce buffer segment */
-	if ( ( rc = comboot_prepare_bounce_buffer ( image ) ) != 0 ) {
+	if ( ( rc = com32_prepare_bounce_buffer ( image ) ) != 0 ) {
 		return rc;
 	}
 
-	return 0;
+	return com32_exec_loop ( image );
 }
 
 /** SYSLINUX COM32 image type */
 struct image_type com32_image_type __image_type ( PROBE_NORMAL ) = {
 	.name = "COM32",
-	.load = com32_load,
+	.probe = com32_probe,
 	.exec = com32_exec,
 };

src/arch/i386/image/comboot.c

@@ -42,8 +42,6 @@ FILE_LICENCE ( GPL2_OR_LATER );
 
 FEATURE ( FEATURE_IMAGE, "COMBOOT", DHCP_EB_FEATURE_COMBOOT, 1 );
 
-struct image_type comboot_image_type __image_type ( PROBE_NORMAL );
-
 /**
  * COMBOOT PSP, copied to offset 0 of code segment
  */
@@ -131,7 +129,7 @@ static void comboot_init_psp ( struct image * image, userptr_t seg_userptr ) {
  * @v image		COMBOOT image
  * @ret rc		Return status code
  */
-static int comboot_exec ( struct image *image ) {
+static int comboot_exec_loop ( struct image *image ) {
 	userptr_t seg_userptr = real_to_user ( COMBOOT_PSP_SEG, 0 );
 	int state;
 
@@ -194,7 +192,6 @@ static int comboot_exec ( struct image *image ) {
 		       image, comboot_replacement_image );
 		image->replacement = comboot_replacement_image;
 		comboot_replacement_image = NULL;
-		image_autoload ( image->replacement );
 		break;
 
 	case COMBOOT_EXIT_COMMAND:
@@ -278,12 +275,12 @@ static int comboot_prepare_segment ( struct image *image )
 }
 
 /**
- * Load COMBOOT image into memory
+ * Probe COMBOOT image
  *
  * @v image		COMBOOT image
  * @ret rc		Return status code
  */
-static int comboot_load ( struct image *image ) {
+static int comboot_probe ( struct image *image ) {
 	int rc;
 
 	DBGC ( image, "COMBOOT %p: name '%s'\n",
@@ -295,9 +292,17 @@ static int comboot_load ( struct image *image ) {
 		return rc;
 	}
 
-	/* This is a 16-bit COMBOOT image, valid or otherwise */
-	if ( ! image->type )
-		image->type = &comboot_image_type;
+	return 0;
+}
+
+/**
+ * Execute COMBOOT image
+ *
+ * @v image		COMBOOT image
+ * @ret rc		Return status code
+ */
+static int comboot_exec ( struct image *image ) {
+	int rc;
 	
 	/* Sanity check for filesize */
 	if( image->len >= 0xFF00 ) {
@@ -311,12 +316,12 @@ static int comboot_load ( struct image *image ) {
 		return rc;
 	}
 
-	return 0;
+	return comboot_exec_loop ( image );
 }
 
 /** SYSLINUX COMBOOT (16-bit) image type */
 struct image_type comboot_image_type __image_type ( PROBE_NORMAL ) = {
 	.name = "COMBOOT",
-	.load = comboot_load,
+	.probe = comboot_probe,
 	.exec = comboot_exec,
 };

src/arch/i386/image/elfboot.c

@@ -34,8 +34,6 @@ FILE_LICENCE ( GPL2_OR_LATER );
 
 FEATURE ( FEATURE_IMAGE, "ELF", DHCP_EB_FEATURE_ELF, 1 );
 
-struct image_type elfboot_image_type __image_type ( PROBE_NORMAL );
-
 /**
  * Execute ELF image
  *
@@ -43,7 +41,15 @@ struct image_type elfboot_image_type __image_type ( PROBE_NORMAL );
  * @ret rc		Return status code
  */
 static int elfboot_exec ( struct image *image ) {
-	physaddr_t entry = image->priv.phys;
+	physaddr_t entry;
+	int rc;
+
+	/* Load the image using core ELF support */
+	if ( ( rc = elf_load ( image, &entry ) ) != 0 ) {
+		DBGC ( image, "ELF %p could not load: %s\n",
+		       image, strerror ( rc ) );
+		return rc;
+	}
 
 	/* An ELF image has no callback interface, so we need to shut
 	 * down before invoking it.
@@ -66,12 +72,12 @@ static int elfboot_exec ( struct image *image ) {
 }
 
 /**
- * Load ELF image into memory
+ * Probe ELF image
  *
  * @v image		ELF file
  * @ret rc		Return status code
  */
-static int elfboot_load ( struct image *image ) {
+static int elfboot_probe ( struct image *image ) {
 	Elf32_Ehdr ehdr;
 	static const uint8_t e_ident[] = {
 		[EI_MAG0]	= ELFMAG0,
@@ -82,7 +88,6 @@ static int elfboot_load ( struct image *image ) {
 		[EI_DATA]	= ELFDATA2LSB,
 		[EI_VERSION]	= EV_CURRENT,
 	};
-	int rc;
 
 	/* Read ELF header */
 	copy_from_user ( &ehdr, image->data, 0, sizeof ( ehdr ) );
@@ -91,23 +96,12 @@ static int elfboot_load ( struct image *image ) {
 		return -ENOEXEC;
 	}
 
-	/* This is an ELF image, valid or otherwise */
-	if ( ! image->type )
-		image->type = &elfboot_image_type;
-
-	/* Load the image using core ELF support */
-	if ( ( rc = elf_load ( image ) ) != 0 ) {
-		DBGC ( image, "ELF %p could not load: %s\n",
-		       image, strerror ( rc ) );
-		return rc;
-	}
-
 	return 0;
 }
 
 /** ELF image type */
 struct image_type elfboot_image_type __image_type ( PROBE_NORMAL ) = {
 	.name = "ELF",
-	.load = elfboot_load,
+	.probe = elfboot_probe,
 	.exec = elfboot_exec,
 };

src/arch/i386/image/multiboot.c

@@ -40,8 +40,6 @@ FILE_LICENCE ( GPL2_OR_LATER );
 
 FEATURE ( FEATURE_IMAGE, "MBOOT", DHCP_EB_FEATURE_MULTIBOOT, 1 );
 
-struct image_type multiboot_image_type __image_type ( PROBE_MULTIBOOT );
-
 /**
  * Maximum number of modules we will allow for
  *
@@ -254,57 +252,6 @@ static struct multiboot_memory_map
 static struct multiboot_module __bss16_array ( mbmodules, [MAX_MODULES] );
 #define mbmodules __use_data16 ( mbmodules )
 
-/**
- * Execute multiboot image
- *
- * @v image		Multiboot image
- * @ret rc		Return status code
- */
-static int multiboot_exec ( struct image *image ) {
-	physaddr_t entry = image->priv.phys;
-
-	/* Populate multiboot information structure */
-	memset ( &mbinfo, 0, sizeof ( mbinfo ) );
-	mbinfo.flags = ( MBI_FLAG_LOADER | MBI_FLAG_MEM | MBI_FLAG_MMAP |
-			 MBI_FLAG_CMDLINE | MBI_FLAG_MODS );
-	mb_cmdline_offset = 0;
-	mbinfo.cmdline = multiboot_add_cmdline ( image->name, image->cmdline );
-	mbinfo.mods_count = multiboot_build_module_list ( image, mbmodules,
-				( sizeof(mbmodules) / sizeof(mbmodules[0]) ) );
-	mbinfo.mods_addr = virt_to_phys ( mbmodules );
-	mbinfo.mmap_addr = virt_to_phys ( mbmemmap );
-	mbinfo.boot_loader_name = virt_to_phys ( mb_bootloader_name );
-
-	/* Multiboot images may not return and have no callback
-	 * interface, so shut everything down prior to booting the OS.
-	 */
-	shutdown_boot();
-
-	/* Build memory map after unhiding bootloader memory regions as part of
-	 * shutting everything down.
-	 */
-	multiboot_build_memmap ( image, &mbinfo, mbmemmap,
-				 ( sizeof(mbmemmap) / sizeof(mbmemmap[0]) ) );
-
-	/* Jump to OS with flat physical addressing */
-	DBGC ( image, "MULTIBOOT %p starting execution at %lx\n",
-	       image, entry );
-	__asm__ __volatile__ ( PHYS_CODE ( "pushl %%ebp\n\t"
-					   "call *%%edi\n\t"
-					   "popl %%ebp\n\t" )
-			       : : "a" ( MULTIBOOT_BOOTLOADER_MAGIC ),
-			           "b" ( virt_to_phys ( &mbinfo ) ),
-			           "D" ( entry )
-			       : "ecx", "edx", "esi", "memory" );
-
-	DBGC ( image, "MULTIBOOT %p returned\n", image );
-
-	/* It isn't safe to continue after calling shutdown() */
-	while ( 1 ) {}
-
-	return -ECANCELED;  /* -EIMPOSSIBLE, anyone? */
-}
-
 /**
  * Find multiboot header
  *
@@ -357,10 +304,12 @@ static int multiboot_find_header ( struct image *image,
  *
  * @v image		Multiboot file
  * @v hdr		Multiboot header descriptor
+ * @ret entry		Entry point
  * @ret rc		Return status code
  */
 static int multiboot_load_raw ( struct image *image,
-				struct multiboot_header_info *hdr ) {
+				struct multiboot_header_info *hdr,
+				physaddr_t *entry ) {
 	size_t offset;
 	size_t filesz;
 	size_t memsz;
@@ -391,8 +340,8 @@ static int multiboot_load_raw ( struct image *image,
 	/* Copy image to segment */
 	memcpy_user ( buffer, 0, image->data, offset, filesz );
 
-	/* Record execution entry point in image private data field */
-	image->priv.phys = hdr->mb.entry_addr;
+	/* Record execution entry point */
+	*entry = hdr->mb.entry_addr;
 
 	return 0;
 }
@@ -401,13 +350,14 @@ static int multiboot_load_raw ( struct image *image,
  * Load ELF multiboot image into memory
  *
  * @v image		Multiboot file
+ * @ret entry		Entry point
  * @ret rc		Return status code
  */
-static int multiboot_load_elf ( struct image *image ) {
+static int multiboot_load_elf ( struct image *image, physaddr_t *entry ) {
 	int rc;
 
 	/* Load ELF image*/
-	if ( ( rc = elf_load ( image ) ) != 0 ) {
+	if ( ( rc = elf_load ( image, entry ) ) != 0 ) {
 		DBGC ( image, "MULTIBOOT %p ELF image failed to load: %s\n",
 		       image, strerror ( rc ) );
 		return rc;
@@ -417,13 +367,14 @@ static int multiboot_load_elf ( struct image *image ) {
 }
 
 /**
- * Load multiboot image into memory
+ * Execute multiboot image
  *
- * @v image		Multiboot file
+ * @v image		Multiboot image
  * @ret rc		Return status code
  */
-static int multiboot_load ( struct image *image ) {
+static int multiboot_exec ( struct image *image ) {
 	struct multiboot_header_info hdr;
+	physaddr_t entry;
 	int rc;
 
 	/* Locate multiboot header, if present */
@@ -432,12 +383,6 @@ static int multiboot_load ( struct image *image ) {
 		       image );
 		return rc;
 	}
-	DBGC ( image, "MULTIBOOT %p found header with flags %08x\n",
-	       image, hdr.mb.flags );
-
-	/* This is a multiboot image, valid or otherwise */
-	if ( ! image->type )
-		image->type = &multiboot_image_type;
 
 	/* Abort if we detect flags that we cannot support */
 	if ( hdr.mb.flags & MB_UNSUPPORTED_FLAGS ) {
@@ -451,16 +396,77 @@ static int multiboot_load ( struct image *image ) {
 	 * the ELF header if present, and Solaris relies on this
 	 * behaviour.
 	 */
-	if ( ( ( rc = multiboot_load_elf ( image ) ) != 0 ) &&
-	     ( ( rc = multiboot_load_raw ( image, &hdr ) ) != 0 ) )
+	if ( ( ( rc = multiboot_load_elf ( image, &entry ) ) != 0 ) &&
+	     ( ( rc = multiboot_load_raw ( image, &hdr, &entry ) ) != 0 ) )
 		return rc;
 
+	/* Populate multiboot information structure */
+	memset ( &mbinfo, 0, sizeof ( mbinfo ) );
+	mbinfo.flags = ( MBI_FLAG_LOADER | MBI_FLAG_MEM | MBI_FLAG_MMAP |
+			 MBI_FLAG_CMDLINE | MBI_FLAG_MODS );
+	mb_cmdline_offset = 0;
+	mbinfo.cmdline = multiboot_add_cmdline ( image->name, image->cmdline );
+	mbinfo.mods_count = multiboot_build_module_list ( image, mbmodules,
+				( sizeof(mbmodules) / sizeof(mbmodules[0]) ) );
+	mbinfo.mods_addr = virt_to_phys ( mbmodules );
+	mbinfo.mmap_addr = virt_to_phys ( mbmemmap );
+	mbinfo.boot_loader_name = virt_to_phys ( mb_bootloader_name );
+
+	/* Multiboot images may not return and have no callback
+	 * interface, so shut everything down prior to booting the OS.
+	 */
+	shutdown_boot();
+
+	/* Build memory map after unhiding bootloader memory regions as part of
+	 * shutting everything down.
+	 */
+	multiboot_build_memmap ( image, &mbinfo, mbmemmap,
+				 ( sizeof(mbmemmap) / sizeof(mbmemmap[0]) ) );
+
+	/* Jump to OS with flat physical addressing */
+	DBGC ( image, "MULTIBOOT %p starting execution at %lx\n",
+	       image, entry );
+	__asm__ __volatile__ ( PHYS_CODE ( "pushl %%ebp\n\t"
+					   "call *%%edi\n\t"
+					   "popl %%ebp\n\t" )
+			       : : "a" ( MULTIBOOT_BOOTLOADER_MAGIC ),
+			           "b" ( virt_to_phys ( &mbinfo ) ),
+			           "D" ( entry )
+			       : "ecx", "edx", "esi", "memory" );
+
+	DBGC ( image, "MULTIBOOT %p returned\n", image );
+
+	/* It isn't safe to continue after calling shutdown() */
+	while ( 1 ) {}
+
+	return -ECANCELED;  /* -EIMPOSSIBLE, anyone? */
+}
+
+/**
+ * Probe multiboot image
+ *
+ * @v image		Multiboot file
+ * @ret rc		Return status code
+ */
+static int multiboot_probe ( struct image *image ) {
+	struct multiboot_header_info hdr;
+	int rc;
+
+	/* Locate multiboot header, if present */
+	if ( ( rc = multiboot_find_header ( image, &hdr ) ) != 0 ) {
+		DBGC ( image, "MULTIBOOT %p has no multiboot header\n",
+		       image );
+		return rc;
+	}
+	DBGC ( image, "MULTIBOOT %p found header with flags %08x\n",
+	       image, hdr.mb.flags );
+
 	return 0;
 }
 
 /** Multiboot image type */
 struct image_type multiboot_image_type __image_type ( PROBE_MULTIBOOT ) = {
 	.name = "Multiboot",
-	.load = multiboot_load,
+	.probe = multiboot_probe,
 	.exec = multiboot_exec,
 };

src/arch/i386/image/nbi.c

@@ -28,8 +28,6 @@
 
 FEATURE ( FEATURE_IMAGE, "NBI", DHCP_EB_FEATURE_NBI, 1 );
 
-struct image_type nbi_image_type __image_type ( PROBE_NORMAL );
-
 /**
  * An NBI image header
  *
@@ -240,57 +238,6 @@ static int nbi_process_segments ( struct image *image,
 	return 0;
 }
 
-/**
- * Load an NBI image into memory
- *
- * @v image		NBI image
- * @ret rc		Return status code
- */
-static int nbi_load ( struct image *image ) {
-	struct imgheader imgheader;
-	int rc;
-
-	/* If we don't have enough data give up */
-	if ( image->len < NBI_HEADER_LENGTH ) {
-		DBGC ( image, "NBI %p too short for an NBI image\n", image );
-		return -ENOEXEC;
-	}
-
-	/* Check image header */
-	copy_from_user ( &imgheader, image->data, 0, sizeof ( imgheader ) );
-	if ( imgheader.magic != NBI_MAGIC ) {
-		DBGC ( image, "NBI %p has no NBI signature\n", image );
-		return -ENOEXEC;
-	}
-
-	/* This is an NBI image, valid or otherwise */
-	if ( ! image->type )
-		image->type = &nbi_image_type;
-
-	DBGC ( image, "NBI %p placing header at %hx:%hx\n", image,
-	       imgheader.location.segment, imgheader.location.offset );
-
-	/* NBI files can have overlaps between segments; the bss of
-	 * one segment may overlap the initialised data of another.  I
-	 * assume this is a design flaw, but there are images out
-	 * there that we need to work with.  We therefore do two
-	 * passes: first to initialise the segments, then to copy the
-	 * data.  This avoids zeroing out already-copied data.
-	 */
-	if ( ( rc = nbi_process_segments ( image, &imgheader,
-					   nbi_prepare_segment ) ) != 0 )
-		return rc;
-	if ( ( rc = nbi_process_segments ( image, &imgheader,
-					   nbi_load_segment ) ) != 0 )
-		return rc;
-
-	/* Record header address in image private data field */
-	image->priv.user = real_to_user ( imgheader.location.segment,
-					  imgheader.location.offset );
-
-	return 0;
-}
-
 /**
  * Boot a 16-bit NBI image
  *
@@ -396,8 +343,25 @@ static int nbi_exec ( struct image *image ) {
 	int may_return;
 	int rc;
 
-	copy_from_user ( &imgheader, image->priv.user, 0,
-			 sizeof ( imgheader ) );
+	/* Retrieve image header */
+	copy_from_user ( &imgheader, image->data, 0, sizeof ( imgheader ) );
+
+	DBGC ( image, "NBI %p placing header at %hx:%hx\n", image,
+	       imgheader.location.segment, imgheader.location.offset );
+
+	/* NBI files can have overlaps between segments; the bss of
+	 * one segment may overlap the initialised data of another.  I
+	 * assume this is a design flaw, but there are images out
+	 * there that we need to work with.  We therefore do two
+	 * passes: first to initialise the segments, then to copy the
+	 * data.  This avoids zeroing out already-copied data.
+	 */
+	if ( ( rc = nbi_process_segments ( image, &imgheader,
+					   nbi_prepare_segment ) ) != 0 )
+		return rc;
+	if ( ( rc = nbi_process_segments ( image, &imgheader,
+					   nbi_load_segment ) ) != 0 )
+		return rc;
 
 	/* Prepare DHCP option block */
 	if ( ( rc = nbi_prepare_dhcp ( image ) ) != 0 )
@@ -427,9 +391,34 @@ static int nbi_exec ( struct image *image ) {
 	return rc;
 }
 
+/**
+ * Probe NBI image
+ *
+ * @v image		NBI image
+ * @ret rc		Return status code
+ */
+static int nbi_probe ( struct image *image ) {
+	struct imgheader imgheader;
+
+	/* If we don't have enough data give up */
+	if ( image->len < NBI_HEADER_LENGTH ) {
+		DBGC ( image, "NBI %p too short for an NBI image\n", image );
+		return -ENOEXEC;
+	}
+
+	/* Check image header */
+	copy_from_user ( &imgheader, image->data, 0, sizeof ( imgheader ) );
+	if ( imgheader.magic != NBI_MAGIC ) {
+		DBGC ( image, "NBI %p has no NBI signature\n", image );
+		return -ENOEXEC;
+	}
+
+	return 0;
+}
+
 /** NBI image type */
 struct image_type nbi_image_type __image_type ( PROBE_NORMAL ) = {
 	.name = "NBI",
-	.load = nbi_load,
+	.probe = nbi_probe,
 	.exec = nbi_exec,
 };

src/arch/i386/image/pxe_image.c

@@ -35,8 +35,6 @@ FILE_LICENCE ( GPL2_OR_LATER );
 
 FEATURE ( FEATURE_IMAGE, "PXE", DHCP_EB_FEATURE_PXE, 1 );
 
-struct image_type pxe_image_type __image_type ( PROBE_PXE );
-
 /**
  * Execute PXE image
  *
@@ -44,9 +42,20 @@ struct image_type pxe_image_type __image_type ( PROBE_PXE );
  * @ret rc		Return status code
  */
 static int pxe_exec ( struct image *image ) {
+	userptr_t buffer = real_to_user ( 0, 0x7c00 );
 	struct net_device *netdev;
 	int rc;
 
+	/* Verify and prepare segment */
+	if ( ( rc = prep_segment ( buffer, image->len, image->len ) ) != 0 ) {
+		DBGC ( image, "IMAGE %p could not prepare segment: %s\n",
+		       image, strerror ( rc ) );
+		return rc;
+	}
+
+	/* Copy image to segment */
+	memcpy_user ( buffer, 0, image->data, 0, image->len );
+
 	/* Arbitrarily pick the most recently opened network device */
 	if ( ( netdev = last_opened_netdev() ) == NULL ) {
 		DBGC ( image, "IMAGE %p could not locate PXE net device\n",
@@ -67,51 +76,33 @@ static int pxe_exec ( struct image *image ) {
 }
 
 /**
- * Load PXE image into memory
+ * Probe PXE image
  *
  * @v image		PXE file
  * @ret rc		Return status code
  */
-int pxe_load ( struct image *image ) {
-	userptr_t buffer = real_to_user ( 0, 0x7c00 );
-	size_t filesz = image->len;
-	size_t memsz = image->len;
-	int rc;
+int pxe_probe ( struct image *image ) {
 
 	/* Images too large to fit in base memory cannot be PXE
 	 * images.  We include this check to help prevent unrecognised
 	 * images from being marked as PXE images, since PXE images
 	 * have no signature we can check against.
 	 */
-	if ( filesz > ( 0xa0000 - 0x7c00 ) )
+	if ( image->len > ( 0xa0000 - 0x7c00 ) )
 		return -ENOEXEC;
 
 	/* Rejecting zero-length images is also useful, since these
 	 * end up looking to the user like bugs in iPXE.
 	 */
-	if ( ! filesz )
+	if ( ! image->len )
 		return -ENOEXEC;
 
-	/* There are no signature checks for PXE; we will accept anything */
-	if ( ! image->type )
-		image->type = &pxe_image_type;
-
-	/* Verify and prepare segment */
-	if ( ( rc = prep_segment ( buffer, filesz, memsz ) ) != 0 ) {
-		DBGC ( image, "IMAGE %p could not prepare segment: %s\n",
-		       image, strerror ( rc ) );
-		return rc;
-	}
-
-	/* Copy image to segment */
-	memcpy_user ( buffer, 0, image->data, 0, filesz );
-
 	return 0;
 }
 
 /** PXE image type */
 struct image_type pxe_image_type __image_type ( PROBE_PXE ) = {
 	.name = "PXE",
-	.load = pxe_load,
+	.probe = pxe_probe,
 	.exec = pxe_exec,
 };

src/arch/i386/interface/syslinux/comboot_call.c

@@ -217,7 +217,7 @@ static int comboot_fetch_kernel ( char *kernel_file, char *cmdline ) {
 		goto out;
 	}
 	if ( ( rc = imgfetch ( kernel, kernel_file,
-			       register_image ) ) != 0 ) {
+			       register_and_select_image ) ) != 0 ) {
 		DBG ( "COMBOOT: could not fetch kernel: %s\n",
 		      strerror ( rc ) );
 		goto out;

src/core/downloader.c

@@ -218,8 +218,7 @@ static struct interface_descriptor downloader_job_desc =
  * @ret rc		Return status code
  *
  * Instantiates a downloader object to download the specified URI into
- * the specified image object.  If the download is successful, the
- * image registration routine @c register_image() will be called.
+ * the specified image object.
  */
 int create_downloader ( struct interface *job, struct image *image,
 			int type, ... ) {

src/core/image.c

@@ -32,7 +32,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
 
 /** @file
  *
- * Executable/loadable images
+ * Executable images
  *
  */
 
@@ -40,7 +40,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
 struct list_head images = LIST_HEAD_INIT ( images );
 
 /**
- * Free executable/loadable image
+ * Free executable image
  *
  * @v refcnt		Reference counter
  */
@@ -52,13 +52,13 @@ static void free_image ( struct refcnt *refcnt ) {
 	ufree ( image->data );
 	image_put ( image->replacement );
 	free ( image );
-	DBGC ( image, "IMAGE %p freed\n", image );
+	DBGC ( image, "IMAGE %s freed\n", image->name );
 }
 
 /**
- * Allocate executable/loadable image
+ * Allocate executable image
  *
- * @ret image		Executable/loadable image
+ * @ret image		Executable image
  */
 struct image * alloc_image ( void ) {
 	struct image *image;
@@ -75,12 +75,11 @@ struct image * alloc_image ( void ) {
  *
  * @v image		Image
  * @v URI		New image URI
- * @ret rc		Return status code
  *
  * If no name is set, the name will be updated to the base name of the
  * URI path (if any).
  */
-int image_set_uri ( struct image *image, struct uri *uri ) {
+void image_set_uri ( struct image *image, struct uri *uri ) {
 	const char *path = uri->path;
 
 	/* Replace URI reference */
@@ -90,8 +89,6 @@ int image_set_uri ( struct image *image, struct uri *uri ) {
 	/* Set name if none already specified */
 	if ( path && ( ! image->name[0] ) )
 		image_set_name ( image, basename ( ( char * ) path ) );
-
-	return 0;
 }
 
 /**
@@ -110,9 +107,9 @@ int image_set_cmdline ( struct image *image, const char *cmdline ) {
 }
 
 /**
- * Register executable/loadable image
+ * Register executable image
  *
- * @v image		Executable/loadable image
+ * @v image		Executable image
  * @ret rc		Return status code
  */
 int register_image ( struct image *image ) {
@@ -127,20 +124,20 @@ int register_image ( struct image *image ) {
 	/* Add to image list */
 	image_get ( image );
 	list_add_tail ( &image->list, &images );
-	DBGC ( image, "IMAGE %p at [%lx,%lx) registered as %s\n",
-	       image, user_to_phys ( image->data, 0 ),
-	       user_to_phys ( image->data, image->len ), image->name );
+	DBGC ( image, "IMAGE %s at [%lx,%lx) registered\n",
+	       image->name, user_to_phys ( image->data, 0 ),
+	       user_to_phys ( image->data, image->len ) );
 
 	return 0;
 }
 
 /**
- * Unregister executable/loadable image
+ * Unregister executable image
  *
- * @v image		Executable/loadable image
+ * @v image		Executable image
  */
 void unregister_image ( struct image *image ) {
-	DBGC ( image, "IMAGE %p unregistered\n", image );
+	DBGC ( image, "IMAGE %s unregistered\n", image->name );
 	list_del ( &image->list );
 	image_put ( image );
 }
@@ -149,7 +146,7 @@ void unregister_image ( struct image *image ) {
  * Find image by name
  *
  * @v name		Image name
- * @ret image		Executable/loadable image, or NULL
+ * @ret image		Executable image, or NULL
  */
 struct image * find_image ( const char *name ) {
 	struct image *image;
@@ -163,75 +160,39 @@ struct image * find_image ( const char *name ) {
 }
 
 /**
- * Load executable/loadable image into memory
- *
- * @v image		Executable/loadable image
- * @v type		Executable/loadable image type
- * @ret rc		Return status code
- */
-static int image_load_type ( struct image *image, struct image_type *type ) {
-	int rc;
-
-	/* Check image is actually loadable */
-	if ( ! type->load )
-		return -ENOEXEC;
-
-	/* Try the image loader */
-	if ( ( rc = type->load ( image ) ) != 0 ) {
-		DBGC ( image, "IMAGE %p could not load as %s: %s\n",
-		       image, type->name, strerror ( rc ) );
-		return rc;
-	}
-
-	/* Flag as loaded */
-	image->flags |= IMAGE_LOADED;
-	return 0;
-}
-
-/**
- * Load executable/loadable image into memory
- *
- * @v image		Executable/loadable image
- * @ret rc		Return status code
- */
-int image_load ( struct image *image ) {
-
-	assert ( image->type != NULL );
-
-	return image_load_type ( image, image->type );
-}
-
-/**
- * Autodetect image type and load executable/loadable image into memory
+ * Determine image type
  *
- * @v image		Executable/loadable image
+ * @v image		Executable image
  * @ret rc		Return status code
  */
-int image_autoload ( struct image *image ) {
+int image_probe ( struct image *image ) {
 	struct image_type *type;
 	int rc;
 
-	/* If image already has a type, use it */
+	/* Succeed if we already have a type */
 	if ( image->type )
-		return image_load ( image );
+		return 0;
 
-	/* Otherwise probe for a suitable type */
+	/* Try each type in turn */
 	for_each_table_entry ( type, IMAGE_TYPES ) {
-		DBGC ( image, "IMAGE %p trying type %s\n", image, type->name );
-		rc = image_load_type ( image, type );
-		if ( image->type == NULL )
-			continue;
-		return rc;
+		if ( ( rc = type->probe ( image ) ) == 0 ) {
+			image->type = type;
+			DBGC ( image, "IMAGE %s is %s\n",
+			       image->name, type->name );
+			return 0;
+		}
+		DBGC ( image, "IMAGE %s is not %s: %s\n", image->name,
+		       type->name, strerror ( rc ) );
 	}
 
-	DBGC ( image, "IMAGE %p format not recognised\n", image );
+	DBGC ( image, "IMAGE %s format not recognised\n", image->name );
 	return -ENOEXEC;
 }
 
 /**
- * Execute loaded image
+ * Execute image
  *
- * @v image		Loaded image
+ * @v image		Executable image
  * @ret rc		Return status code
  */
 int image_exec ( struct image *image ) {
@@ -239,18 +200,9 @@ int image_exec ( struct image *image ) {
 	struct uri *old_cwuri;
 	int rc;
 
-	/* Image must be loaded first */
-	if ( ! ( image->flags & IMAGE_LOADED ) ) {
-		DBGC ( image, "IMAGE %p could not execute: not loaded\n",
-		       image );
-		return -ENOTTY;
-	}
-
-	assert ( image->type != NULL );
-
-	/* Check that image is actually executable */
-	if ( ! image->type->exec )
-		return -ENOEXEC;
+	/* Check that this image can be executed */
+	if ( ( rc = image_probe ( image ) ) != 0 )
+		return rc;
 
 	/* Switch current working directory to be that of the image itself */
 	old_cwuri = uri_get ( cwuri );
@@ -264,8 +216,8 @@ int image_exec ( struct image *image ) {
 
 	/* Try executing the image */
 	if ( ( rc = image->type->exec ( image ) ) != 0 ) {
-		DBGC ( image, "IMAGE %p could not execute: %s\n",
-		       image, strerror ( rc ) );
+		DBGC ( image, "IMAGE %s could not execute: %s\n",
+		       image->name, strerror ( rc ) );
 		/* Do not return yet; we still have clean-up to do */
 	}
 
@@ -283,8 +235,8 @@ int image_exec ( struct image *image ) {
 
 	/* Tail-recurse into replacement image, if one exists */
 	if ( replacement ) {
-		DBGC ( image, "IMAGE %p replacing self with IMAGE %p\n",
-		       image, replacement );
+		DBGC ( image, "IMAGE %s replacing self with IMAGE %s\n",
+		       image->name, replacement->name );
 		if ( ( rc = image_exec ( replacement ) ) != 0 )
 			return rc;
 	}
@@ -293,33 +245,75 @@ int image_exec ( struct image *image ) {
 }
 
 /**
- * Register and autoload an image
+ * Select image for execution
  *
- * @v image		Image
+ * @v image		Executable image
  * @ret rc		Return status code
  */
-int register_and_autoload_image ( struct image *image ) {
+int image_select ( struct image *image ) {
+	struct image *tmp;
+	int rc;
+
+	/* Unselect all other images */
+	for_each_image ( tmp )
+		tmp->flags &= ~IMAGE_SELECTED;
+
+	/* Check that this image can be executed */
+	if ( ( rc = image_probe ( image ) ) != 0 )
+		return rc;
+
+	/* Mark image as selected */
+	image->flags |= IMAGE_SELECTED;
+
+	return 0;
+}
+
+/**
+ * Find selected image
+ *
+ * @ret image		Executable image, or NULL
+ */
+struct image * image_find_selected ( void ) {
+	struct image *image;
+
+	for_each_image ( image ) {
+		if ( image->flags & IMAGE_SELECTED )
+			return image;
+	}
+	return NULL;
+}
+
+/**
+ * Register and select an image
+ *
+ * @v image		Executable image
+ * @ret rc		Return status code
+ */
+int register_and_select_image ( struct image *image ) {
 	int rc;
 
 	if ( ( rc = register_image ( image ) ) != 0 )
 		return rc;
 
-	if ( ( rc = image_autoload ( image ) ) != 0 )
+	if ( ( rc = image_probe ( image ) ) != 0 )
+		return rc;
+
+	if ( ( rc = image_select ( image ) ) != 0 )
 		return rc;
 
 	return 0;
 }
 
 /**
- * Register and autoexec an image
+ * Register and boot an image
  *
  * @v image		Image
  * @ret rc		Return status code
  */
-int register_and_autoexec_image ( struct image *image ) {
+int register_and_boot_image ( struct image *image ) {
 	int rc;
 
-	if ( ( rc = register_and_autoload_image ( image ) ) != 0 )
+	if ( ( rc = register_and_select_image ( image ) ) != 0 )
 		return rc;
 
 	if ( ( rc = image_exec ( image ) ) != 0 )

src/hci/commands/image_cmd.c

@@ -86,28 +86,18 @@ static struct command_descriptor imgfetch_cmd =
 	COMMAND_DESC ( struct imgfetch_options, imgfetch_opts, 1, MAX_ARGUMENTS,
 		       "[--name <name>] <uri> [<arguments>...]" );
 
-/** "kernel" command descriptor */
-static struct command_descriptor kernel_cmd =
-	COMMAND_DESC ( struct imgfetch_options, imgfetch_opts, 1, MAX_ARGUMENTS,
-		       "[--name <name>] <uri> [<arguments>...]" );
-
-/** "chain" command descriptor */
-static struct command_descriptor chain_cmd =
-	COMMAND_DESC ( struct imgfetch_options, imgfetch_opts, 1, MAX_ARGUMENTS,
-		       "[--name <name>] <uri> [<arguments>...]" );
-
 /**
  * The "imgfetch" and friends command body
  *
  * @v argc		Argument count
  * @v argv		Argument list
  * @v cmd		Command descriptor
- * @v image_register	Image registration action
+ * @v action		Action to take upon a successful download
  * @ret rc		Return status code
  */
 static int imgfetch_core_exec ( int argc, char **argv,
 				struct command_descriptor *cmd,
-				int ( * image_register ) ( struct image * ) ) {
+				int ( * action ) ( struct image *image ) ) {
 	struct imgfetch_options opts;
 	struct image *image;
 	char *uri_string;
@@ -139,7 +129,7 @@ static int imgfetch_core_exec ( int argc, char **argv,
 		return rc;
 
 	/* Fetch the image */
-	if ( ( rc = imgfetch ( image, uri_string, image_register ) ) != 0 ) {
+	if ( ( rc = imgfetch ( image, uri_string, action ) ) != 0 ) {
 		printf ( "Could not fetch %s: %s\n",
 			 uri_string, strerror ( rc ) );
 		image_put ( image );
@@ -172,8 +162,8 @@ static int imgfetch_exec ( int argc, char **argv ) {
  */
 static int kernel_exec ( int argc, char **argv ) {
 
-	return imgfetch_core_exec ( argc, argv, &kernel_cmd,
-				    register_and_autoload_image );
+	return imgfetch_core_exec ( argc, argv, &imgfetch_cmd,
+				    register_and_select_image );
 }
 
 /**
@@ -185,34 +175,35 @@ static int kernel_exec ( int argc, char **argv ) {
  */
 static int chain_exec ( int argc, char **argv) {
 
-	return imgfetch_core_exec ( argc, argv, &chain_cmd,
-				    register_and_autoexec_image );
+	return imgfetch_core_exec ( argc, argv, &imgfetch_cmd,
+				    register_and_boot_image );
 }
 
-/** "imgload" options */
-struct imgload_options {};
+/** "imgselect" options */
+struct imgselect_options {};
 
-/** "imgload" option list */
-static struct option_descriptor imgload_opts[] = {};
+/** "imgselect" option list */
+static struct option_descriptor imgselect_opts[] = {};
 
-/** "imgload" command descriptor */
-static struct command_descriptor imgload_cmd =
-	COMMAND_DESC ( struct imgload_options, imgload_opts, 1, 1, "<image>" );
+/** "imgselect" command descriptor */
+static struct command_descriptor imgselect_cmd =
+	COMMAND_DESC ( struct imgselect_options, imgselect_opts, 1, 1,
+		       "<image>" );
 
 /**
- * The "imgload" command
+ * The "imgselect" command
  *
  * @v argc		Argument count
  * @v argv		Argument list
  * @ret rc		Return status code
  */
-static int imgload_exec ( int argc, char **argv ) {
-	struct imgload_options opts;
+static int imgselect_exec ( int argc, char **argv ) {
+	struct imgselect_options opts;
 	struct image *image;
 	int rc;
 
 	/* Parse options */
-	if ( ( rc = parse_options ( argc, argv, &imgload_cmd, &opts ) ) != 0 )
+	if ( ( rc = parse_options ( argc, argv, &imgselect_cmd, &opts ) ) != 0 )
 		return rc;
 
 	/* Parse image name */
@@ -220,8 +211,8 @@ static int imgload_exec ( int argc, char **argv ) {
 		return rc;
 
 	/* Load image */
-	if ( ( rc = imgload ( image ) ) != 0 ) {
-		printf ( "Could not load %s: %s\n",
+	if ( ( rc = imgselect ( image ) ) != 0 ) {
+		printf ( "Could not select %s: %s\n",
 			 image->name, strerror ( rc ) );
 		return rc;
 	}
@@ -302,8 +293,9 @@ static int imgexec_exec ( int argc, char **argv ) {
 	} else {
 		image = imgautoselect();
 		if ( ! image ) {
-			printf ( "No (unique) loaded image\n" );
-			return -ENOTTY;
+			rc = -ENOTTY;
+			printf ( "No image selected: %s\n", strerror ( rc ) );
+			return rc;
 		}
 	}
 
@@ -417,8 +409,12 @@ struct command image_commands[] __command = {
 		.exec = chain_exec,
 	},
 	{
-		.name = "imgload",
-		.exec = imgload_exec,
+		.name = "imgselect",
+		.exec = imgselect_exec,
+	},
+	{
+		.name = "imgload", /* synonym for "imgselect" */
+		.exec = imgselect_exec,
 	},
 	{
 		.name = "imgargs",

src/image/efi_image.c

@@ -26,8 +26,6 @@ FILE_LICENCE ( GPL2_OR_LATER );
 
 FEATURE ( FEATURE_IMAGE, "EFI", DHCP_EB_FEATURE_EFI, 1 );
 
-struct image_type efi_image_type __image_type ( PROBE_NORMAL );
-
 /** Event used to signal shutdown */
 static EFI_EVENT efi_shutdown_event;
 
@@ -99,12 +97,12 @@ done:
 }
 
 /**
- * Load EFI image into memory
+ * Probe EFI image
  *
  * @v image		EFI file
  * @ret rc		Return status code
  */
-static int efi_image_load ( struct image *image ) {
+static int efi_image_probe ( struct image *image ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
 	EFI_HANDLE handle;
 	EFI_STATUS efirc;
@@ -119,10 +117,6 @@ static int efi_image_load ( struct image *image ) {
 		return -ENOEXEC;
 	}
 
-	/* This is an EFI image */
-	if ( ! image->type )
-		image->type = &efi_image_type;
-
 	/* Unload the image.  We can't leave it loaded, because we
 	 * have no "unload" operation.
 	 */
@@ -134,6 +128,6 @@ static int efi_image_load ( struct image *image ) {
 /** EFI image type */
 struct image_type efi_image_type __image_type ( PROBE_NORMAL ) = {
 	.name = "EFI",
-	.load = efi_image_load,
+	.probe = efi_image_probe,
 	.exec = efi_image_exec,
 };

src/image/elf.c

@@ -45,10 +45,11 @@ typedef Elf32_Off	Elf_Off;
  * @v image		ELF file
  * @v phdr		ELF program header
  * @v ehdr		ELF executable header
+ * @ret entry		Entry point, if found
  * @ret rc		Return status code
  */
 static int elf_load_segment ( struct image *image, Elf_Phdr *phdr,
-			      Elf_Ehdr *ehdr ) {
+			      Elf_Ehdr *ehdr, physaddr_t *entry ) {
 	physaddr_t dest;
 	userptr_t buffer;
 	unsigned long e_offset;
@@ -96,15 +97,15 @@ static int elf_load_segment ( struct image *image, Elf_Phdr *phdr,
 
 	/* Set execution address, if it lies within this segment */
 	if ( ( e_offset = ( ehdr->e_entry - dest ) ) < phdr->p_filesz ) {
-		image->priv.phys = ehdr->e_entry;
+		*entry = ehdr->e_entry;
 		DBGC ( image, "ELF %p found physical entry point at %lx\n",
-		       image, image->priv.phys );
+		       image, *entry );
 	} else if ( ( e_offset = ( ehdr->e_entry - phdr->p_vaddr ) )
 		    < phdr->p_filesz ) {
-		if ( ! image->priv.phys ) {
-			image->priv.phys = ( dest + e_offset );
+		if ( ! *entry ) {
+			*entry = ( dest + e_offset );
 			DBGC ( image, "ELF %p found virtual entry point at %lx"
-			       " (virt %lx)\n", image, image->priv.phys,
+			       " (virt %lx)\n", image, *entry,
 			       ( ( unsigned long ) ehdr->e_entry ) );
 		}
 	}
@@ -116,18 +117,16 @@ static int elf_load_segment ( struct image *image, Elf_Phdr *phdr,
  * Load ELF image into memory
  *
  * @v image		ELF file
+ * @ret entry		Entry point
  * @ret rc		Return status code
  */
-int elf_load ( struct image *image ) {
+int elf_load ( struct image *image, physaddr_t *entry ) {
 	Elf_Ehdr ehdr;
 	Elf_Phdr phdr;
 	Elf_Off phoff;
 	unsigned int phnum;
 	int rc;
 
-	/* Image type must already have been set by caller */
-	assert ( image->type != NULL );
-
 	/* Read ELF header */
 	copy_from_user ( &ehdr, image->data, 0, sizeof ( ehdr ) );
 	if ( memcmp ( &ehdr.e_ident[EI_MAG0], ELFMAG, SELFMAG ) != 0 ) {
@@ -135,8 +134,8 @@ int elf_load ( struct image *image ) {
 		return -ENOEXEC;
 	}
 
-	/* Invalidate execution address */
-	image->priv.phys = 0;
+	/* Invalidate entry point */
+	*entry = 0;
 
 	/* Read ELF program headers */
 	for ( phoff = ehdr.e_phoff , phnum = ehdr.e_phnum ; phnum ;
@@ -147,12 +146,14 @@ int elf_load ( struct image *image ) {
 			return -ENOEXEC;
 		}
 		copy_from_user ( &phdr, image->data, phoff, sizeof ( phdr ) );
-		if ( ( rc = elf_load_segment ( image, &phdr, &ehdr ) ) != 0 )
+		if ( ( rc = elf_load_segment ( image, &phdr, &ehdr,
+					       entry ) ) != 0 ) {
 			return rc;
+		}
 	}
 
 	/* Check for a valid execution address */
-	if ( ! image->priv.phys ) {
+	if ( ! *entry ) {
 		DBGC ( image, "ELF %p entry point %lx outside image\n",
 		       image, ( ( unsigned long ) ehdr.e_entry ) );
 		return -ENOEXEC;

src/image/embedded.c

@@ -76,10 +76,10 @@ static void embedded_init ( void ) {
 		}
 	}
 
-	/* Load the first image */
+	/* Select the first image */
 	image = &embedded_images[0];
-	if ( ( rc = image_autoload ( image ) ) != 0 ) {
-		DBG ( "Could not load embedded image \"%s\": %s\n",
+	if ( ( rc = image_select ( image ) ) != 0 ) {
+		DBG ( "Could not select embedded image \"%s\": %s\n",
 		      image->name, strerror ( rc ) );
 		return;
 	}

src/image/script.c

@@ -36,8 +36,6 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #include <ipxe/image.h>
 #include <ipxe/shell.h>
 
-struct image_type script_image_type __image_type ( PROBE_NORMAL );
-
 /** Currently running script
  *
  * This is a global in order to allow goto_exec() to update the
@@ -165,12 +163,12 @@ static int script_exec ( struct image *image ) {
 }
 
 /**
- * Load script into memory
+ * Probe script image
  *
  * @v image		Script
  * @ret rc		Return status code
  */
-static int script_load ( struct image *image ) {
+static int script_probe ( struct image *image ) {
 	static const char ipxe_magic[] = "#!ipxe";
 	static const char gpxe_magic[] = "#!gpxe";
 	linker_assert ( sizeof ( ipxe_magic ) == sizeof ( gpxe_magic ),
@@ -193,20 +191,13 @@ static int script_load ( struct image *image ) {
 		return -ENOEXEC;
 	}
 
-	/* This is a script */
-	image->type = &script_image_type;
-
-	/* We don't actually load it anywhere; we will pick the lines
-	 * out of the image as we need them.
-	 */
-
 	return 0;
 }
 
 /** Script image type */
 struct image_type script_image_type __image_type ( PROBE_NORMAL ) = {
 	.name = "script",
-	.load = script_load,
+	.probe = script_probe,
 	.exec = script_exec,
 };
 

src/include/ipxe/elf.h

@@ -12,6 +12,6 @@ FILE_LICENCE ( GPL2_OR_LATER );
 
 #include <elf.h>
 
-extern int elf_load ( struct image *image );
+extern int elf_load ( struct image *image, physaddr_t *entry );
 
 #endif /* _IPXE_ELF_H */

src/include/ipxe/image.h

@@ -4,7 +4,7 @@
 /**
  * @file
  *
- * Executable/loadable images
+ * Executable images
  *
  */
 
@@ -18,7 +18,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
 struct uri;
 struct image_type;
 
-/** An executable or loadable image */
+/** An executable image */
 struct image {
 	/** Reference count */
 	struct refcnt refcnt;
@@ -42,20 +42,13 @@ struct image {
 
 	/** Image type, if known */
 	struct image_type *type;
-	/** Image type private data */
-	union {
-		physaddr_t phys;
-		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 (i.e. loaded).
+	 * the new image.
 	 *
 	 * If an image unregisters itself as a result of being
 	 * executed, it must make sure that its replacement image (if
@@ -65,41 +58,26 @@ struct image {
 	struct image *replacement;
 };
 
-/** Image is loaded */
-#define IMAGE_LOADED 0x0001
+/** Image is selected for execution */
+#define IMAGE_SELECTED 0x0001
 
-/** An executable or loadable image type */
+/** An executable image type */
 struct image_type {
 	/** Name of this image type */
 	char *name;
-	/**
-	 * Load image into memory
+	/** Probe image
 	 *
-	 * @v image		Executable/loadable image
+	 * @v image		Executable image
 	 * @ret rc		Return status code
 	 *
-	 * Load the image into memory at the correct location as
-	 * determined by the file format.
-	 *
-	 * If the file image is in the correct format, the method must
-	 * update @c image->type to point to its own type (unless @c
-	 * image->type is already set).  This allows the autoloading
-	 * code to disambiguate between "this is not my image format"
-	 * and "there is something wrong with this image".  In
-	 * particular, setting @c image->type and then returning an
-	 * error will cause image_autoload() to abort and return an
-	 * error, rather than continuing to the next image type.
+	 * Return success if the image is of this image type.
 	 */
-	int ( * load ) ( struct image *image );
+	int ( * probe ) ( struct image *image );
 	/**
-	 * Execute loaded image
+	 * Execute image
 	 *
-	 * @v image		Loaded image
+	 * @v image		Executable 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 );
 };
@@ -125,10 +103,10 @@ struct image_type {
  */
 #define PROBE_PXE 03
 
-/** Executable or loadable image type table */
+/** Executable image type table */
 #define IMAGE_TYPES __table ( struct image_type, "image_types" )
 
-/** An executable or loadable image type */
+/** An executable image type */
 #define __image_type( probe_order ) __table_entry ( IMAGE_TYPES, probe_order )
 
 extern struct list_head images;
@@ -147,17 +125,17 @@ static inline int have_images ( void ) {
 }
 
 extern struct image * alloc_image ( void );
-extern int image_set_uri ( struct image *image, struct uri *uri );
+extern void image_set_uri ( struct image *image, struct uri *uri );
 extern int image_set_cmdline ( struct image *image, const char *cmdline );
 extern int register_image ( struct image *image );
 extern void unregister_image ( struct image *image );
-extern void promote_image ( struct image *image );
 struct image * find_image ( const char *name );
-extern int image_load ( struct image *image );
-extern int image_autoload ( struct image *image );
+extern int image_probe ( struct image *image );
 extern int image_exec ( struct image *image );
-extern int register_and_autoload_image ( struct image *image );
-extern int register_and_autoexec_image ( struct image *image );
+extern int image_select ( struct image *image );
+extern struct image * image_find_selected ( void );
+extern int register_and_select_image ( struct image *image );
+extern int register_and_boot_image ( struct image *image );
 
 /**
  * Increment reference count on an image

src/include/usr/imgmgmt.h

@@ -9,16 +9,42 @@
 
 FILE_LICENCE ( GPL2_OR_LATER );
 
-struct image;
+#include <ipxe/image.h>
 
 extern int imgdownload ( struct image *image, struct uri *uri,
 			 int ( * action ) ( struct image *image ) );
 extern int imgfetch ( struct image *image, const char *uri_string,
 		      int ( * action ) ( struct image *image ) );
-extern int imgload ( struct image *image );
-extern int imgexec ( struct image *image );
-extern struct image * imgautoselect ( void );
 extern void imgstat ( struct image *image );
 extern void imgfree ( struct image *image );
 
+/**
+ * Select an image for execution
+ *
+ * @v image		Image
+ * @ret rc		Return status code
+ */
+static inline int imgselect ( struct image *image ) {
+	return image_select ( image );
+}
+
+/**
+ * Find the previously-selected image
+ *
+ * @ret image		Image, or NULL
+ */
+static inline struct image * imgautoselect ( void ) {
+	return image_find_selected();
+}
+
+/**
+ * Execute an image
+ *
+ * @v image		Image
+ * @ret rc		Return status code
+ */
+static inline int imgexec ( struct image *image ) {
+	return image_exec ( image );
+}
+
 #endif /* _USR_IMGMGMT_H */

src/usr/autoboot.c

@@ -184,7 +184,7 @@ int uriboot ( struct uri *filename, struct uri *root_path ) {
 	/* Attempt filename boot if applicable */
 	if ( filename ) {
 		if ( ( rc = imgdownload ( image, filename,
-					  register_and_autoexec_image ) ) !=0){
+					  register_and_boot_image ) ) != 0 ) {
 			printf ( "\nCould not chain image: %s\n",
 				 strerror ( rc ) );
 			/* Fall through to (possibly) attempt a SAN boot

src/usr/imgmgmt.c

@@ -99,63 +99,17 @@ int imgfetch ( struct image *image, const char *uri_string,
 	return rc;
 }
 
-/**
- * Load an image
- *
- * @v image		Image
- * @ret rc		Return status code
- */
-int imgload ( struct image *image ) {
-	int rc;
-
-	/* Try to load image */
-	if ( ( rc = image_autoload ( image ) ) != 0 )
-		return rc;
-
-	return 0;
-}
-
-/**
- * Execute an image
- *
- * @v image		Image
- * @ret rc		Return status code
- */
-int imgexec ( struct image *image ) {
-	return image_exec ( image );
-}
-
-/**
- * Identify the only loaded image
- *
- * @ret image		Image, or NULL if 0 or >1 images are loaded
- */
-struct image * imgautoselect ( void ) {
-	struct image *image;
-	struct image *selected_image = NULL;
-	int flagged_images = 0;
-
-	for_each_image ( image ) {
-		if ( image->flags & IMAGE_LOADED ) {
-			selected_image = image;
-			flagged_images++;
-		}
-	}
-
-	return ( ( flagged_images == 1 ) ? selected_image : NULL );
-}
-
 /**
  * Display status of an image
  *
  * @v image		Executable/loadable image
  */
 void imgstat ( struct image *image ) {
-	printf ( "%s: %zd bytes", image->name, image->len );
+	printf ( "%s : %zd bytes", image->name, image->len );
 	if ( image->type )
 		printf ( " [%s]", image->type->name );
-	if ( image->flags & IMAGE_LOADED )
-		printf ( " [LOADED]" );
+	if ( image->flags & IMAGE_SELECTED )
+		printf ( " [SELECTED]" );
 	if ( image->cmdline )
 		printf ( " \"%s\"", image->cmdline );
 	printf ( "\n" );