Fix EFI multi device boot support

Fix EFI boot support when presented with multiple valid boot partitions
across multiple devices.

It now prefers to boot from partitions that are present on the underlying
device that the boot1 image was loaded from. This means that it will boot
from the partitions on device the user chose from EFI boot menu in
preference to those on other devices.

Also fixed is the recovery from a failed attempt to boot, from a seemingly
valid partition, by continuing to trying all other available partitions
no matter what the error.

boot1 now use * to signify a partition what was accepted from the preferred
device and + otherwise.

Finally some error messages where improved and DPRINTF's with slowed boot
to aid debugging.

ZFS will still be preferred over UFS when both are available on the boot
device.

Reviewed by:	imp
MFC after:	1 week
Sponsored by:	Multiplay
Differential Revision:	https://reviews.freebsd.org/D5108

5 files changed, 517 insertions, 181 deletions

diff --git a/sys/boot/efi/boot1/boot1.c b/sys/boot/efi/boot1/boot1.c
index c326c79..1161b0a 100644
--- a/sys/boot/efi/boot1/boot1.c
+++ b/sys/boot/efi/boot1/boot1.c
@@ -50,9 +50,6 @@ static const boot_module_t *boot_modules[] =
 void putchar(int c);
 EFI_STATUS efi_main(EFI_HANDLE Ximage, EFI_SYSTEM_TABLE* Xsystab);
 
-static void try_load(const boot_module_t* mod);
-static EFI_STATUS probe_handle(EFI_HANDLE h);
-
 EFI_SYSTEM_TABLE *systab;
 EFI_BOOT_SERVICES *bs;
 static EFI_HANDLE *image;
@@ -85,20 +82,300 @@ Free(void *buf, const char *file __unused, int line __unused)
 }
 
 /*
- * This function only returns if it fails to load the kernel. If it
- * succeeds, it simply boots the kernel.
+ * nodes_match returns TRUE if the imgpath isn't NULL and the nodes match,
+ * FALSE otherwise.
  */
-void
-try_load(const boot_module_t *mod)
+static BOOLEAN
+nodes_match(EFI_DEVICE_PATH *imgpath, EFI_DEVICE_PATH *devpath)
+{
+	int len;
+
+	if (imgpath == NULL || imgpath->Type != devpath->Type ||
+	    imgpath->SubType != devpath->SubType)
+		return (FALSE);
+
+	len = DevicePathNodeLength(imgpath);
+	if (len != DevicePathNodeLength(devpath))
+		return (FALSE);
+
+	return (memcmp(imgpath, devpath, (size_t)len) == 0);
+}
+
+/*
+ * device_paths_match returns TRUE if the imgpath isn't NULL and all nodes
+ * in imgpath and devpath match up to their respect occurances of a media
+ * node, FALSE otherwise.
+ */
+static BOOLEAN
+device_paths_match(EFI_DEVICE_PATH *imgpath, EFI_DEVICE_PATH *devpath)
+{
+
+	if (imgpath == NULL)
+		return (FALSE);
+
+	while (!IsDevicePathEnd(imgpath) && !IsDevicePathEnd(devpath)) {
+		if (IsDevicePathType(imgpath, MEDIA_DEVICE_PATH) &&
+		    IsDevicePathType(devpath, MEDIA_DEVICE_PATH))
+			return (TRUE);
+
+		if (!nodes_match(imgpath, devpath))
+			return (FALSE);
+
+		imgpath = NextDevicePathNode(imgpath);
+		devpath = NextDevicePathNode(devpath);
+	}
+
+	return (FALSE);
+}
+
+/*
+ * devpath_last returns the last non-path end node in devpath.
+ */
+static EFI_DEVICE_PATH *
+devpath_last(EFI_DEVICE_PATH *devpath)
+{
+
+	while (!IsDevicePathEnd(NextDevicePathNode(devpath)))
+		devpath = NextDevicePathNode(devpath);
+
+	return (devpath);
+}
+
+/*
+ * devpath_node_str is a basic output method for a devpath node which
+ * only understands a subset of the available sub types.
+ *
+ * If we switch to UEFI 2.x then we should update it to use:
+ * EFI_DEVICE_PATH_TO_TEXT_PROTOCOL.
+ */
+static int
+devpath_node_str(char *buf, size_t size, EFI_DEVICE_PATH *devpath)
+{
+
+	switch (devpath->Type) {
+	case MESSAGING_DEVICE_PATH:
+		switch (devpath->SubType) {
+		case MSG_ATAPI_DP: {
+			ATAPI_DEVICE_PATH *atapi;
+
+			atapi = (ATAPI_DEVICE_PATH *)(void *)devpath;
+			return snprintf(buf, size, "ata(%s,%s,0x%x)",
+			    (atapi->PrimarySecondary == 1) ?  "Sec" : "Pri",
+			    (atapi->SlaveMaster == 1) ?  "Slave" : "Master",
+			    atapi->Lun);
+		}
+		case MSG_USB_DP: {
+			USB_DEVICE_PATH *usb;
+
+			usb = (USB_DEVICE_PATH *)devpath;
+			return snprintf(buf, size, "usb(0x%02x,0x%02x)",
+			    usb->ParentPortNumber, usb->InterfaceNumber);
+		}
+		case MSG_SCSI_DP: {
+			SCSI_DEVICE_PATH *scsi;
+
+			scsi = (SCSI_DEVICE_PATH *)(void *)devpath;
+			return snprintf(buf, size, "scsi(0x%02x,0x%02x)",
+			    scsi->Pun, scsi->Lun);
+		}
+		case MSG_SATA_DP: {
+			SATA_DEVICE_PATH *sata;
+
+			sata = (SATA_DEVICE_PATH *)(void *)devpath;
+			return snprintf(buf, size, "sata(0x%x,0x%x,0x%x)",
+			    sata->HBAPortNumber, sata->PortMultiplierPortNumber,
+			    sata->Lun);
+		}
+		default:
+			return snprintf(buf, size, "msg(0x%02x)",
+			    devpath->SubType);
+		}
+		break;
+	case HARDWARE_DEVICE_PATH:
+		switch (devpath->SubType) {
+		case HW_PCI_DP: {
+			PCI_DEVICE_PATH *pci;
+
+			pci = (PCI_DEVICE_PATH *)devpath;
+			return snprintf(buf, size, "pci(0x%02x,0x%02x)",
+			    pci->Device, pci->Function);
+		}
+		default:
+			return snprintf(buf, size, "hw(0x%02x)",
+			    devpath->SubType);
+		}
+		break;
+	case ACPI_DEVICE_PATH: {
+		ACPI_HID_DEVICE_PATH *acpi;
+
+		acpi = (ACPI_HID_DEVICE_PATH *)(void *)devpath;
+		if ((acpi->HID & PNP_EISA_ID_MASK) == PNP_EISA_ID_CONST) {
+			switch (EISA_ID_TO_NUM(acpi->HID)) {
+			case 0x0a03:
+				return snprintf(buf, size, "pciroot(0x%x)",
+				    acpi->UID);
+			case 0x0a08:
+				return snprintf(buf, size, "pcieroot(0x%x)",
+				    acpi->UID);
+			case 0x0604:
+				return snprintf(buf, size, "floppy(0x%x)",
+				    acpi->UID);
+			case 0x0301:
+				return snprintf(buf, size, "keyboard(0x%x)",
+				    acpi->UID);
+			case 0x0501:
+				return snprintf(buf, size, "serial(0x%x)",
+				    acpi->UID);
+			case 0x0401:
+				return snprintf(buf, size, "parallelport(0x%x)",
+				    acpi->UID);
+			default:
+				return snprintf(buf, size, "acpi(pnp%04x,0x%x)",
+				    EISA_ID_TO_NUM(acpi->HID), acpi->UID);
+			}
+		}
+
+		return snprintf(buf, size, "acpi(0x%08x,0x%x)", acpi->HID,
+		    acpi->UID);
+	}
+	case MEDIA_DEVICE_PATH:
+		switch (devpath->SubType) {
+		case MEDIA_CDROM_DP: {
+			CDROM_DEVICE_PATH *cdrom;
+
+			cdrom = (CDROM_DEVICE_PATH *)(void *)devpath;
+			return snprintf(buf, size, "cdrom(%x)",
+			    cdrom->BootEntry);
+		}
+		case MEDIA_HARDDRIVE_DP: {
+			HARDDRIVE_DEVICE_PATH *hd;
+
+			hd = (HARDDRIVE_DEVICE_PATH *)(void *)devpath;
+			return snprintf(buf, size, "hd(%x)",
+			    hd->PartitionNumber);
+		}
+		default:
+			return snprintf(buf, size, "media(0x%02x)",
+			    devpath->SubType);
+		}
+	case BBS_DEVICE_PATH:
+		return snprintf(buf, size, "bbs(0x%02x)", devpath->SubType);
+	case END_DEVICE_PATH_TYPE:
+		return (0);
+	}
+
+	return snprintf(buf, size, "type(0x%02x, 0x%02x)", devpath->Type,
+	    devpath->SubType);
+}
+
+/*
+ * devpath_strlcat appends a text description of devpath to buf but not more
+ * than size - 1 characters followed by NUL-terminator.
+ */
+int
+devpath_strlcat(char *buf, size_t size, EFI_DEVICE_PATH *devpath)
+{
+	size_t len, used;
+	const char *sep;
+
+	sep = "";
+	used = 0;
+	while (!IsDevicePathEnd(devpath)) {
+		len = snprintf(buf, size - used, "%s", sep);
+		used += len;
+		if (used > size)
+			return (used);
+		buf += len;
+
+		len = devpath_node_str(buf, size - used, devpath);
+		used += len;
+		if (used > size)
+			return (used);
+		buf += len;
+		devpath = NextDevicePathNode(devpath);
+		sep = ":";
+	}
+
+	return (used);
+}
+
+/*
+ * devpath_str is convenience method which returns the text description of
+ * devpath using a static buffer, so it isn't thread safe!
+ */
+char *
+devpath_str(EFI_DEVICE_PATH *devpath)
+{
+	static char buf[256];
+
+	devpath_strlcat(buf, sizeof(buf), devpath);
+
+	return buf;
+}
+
+/*
+ * load_loader attempts to load the loader image data.
+ *
+ * It tries each module and its respective devices, identified by mod->probe,
+ * in order until a successful load occurs at which point it returns EFI_SUCCESS
+ * and EFI_NOT_FOUND otherwise.
+ *
+ * Only devices which have preferred matching the preferred parameter are tried.
+ */
+static EFI_STATUS
+load_loader(const boot_module_t **modp, dev_info_t **devinfop, void **bufp,
+    size_t *bufsize, BOOLEAN preferred)
 {
-	size_t bufsize, cmdsize;
-	void *buf;
+	UINTN i;
+	dev_info_t *dev;
+	const boot_module_t *mod;
+
+	for (i = 0; i < NUM_BOOT_MODULES; i++) {
+		if (boot_modules[i] == NULL)
+			continue;
+		mod = boot_modules[i];
+		for (dev = mod->devices(); dev != NULL; dev = dev->next) {
+			if (dev->preferred != preferred)
+				continue;
+
+			if (mod->load(PATH_LOADER_EFI, dev, bufp, bufsize) ==
+			    EFI_SUCCESS) {
+				*devinfop = dev;
+				*modp = mod;
+				return (EFI_SUCCESS);
+			}
+		}
+	}
+
+	return (EFI_NOT_FOUND);
+}
+
+/*
+ * try_boot only returns if it fails to load the loader. If it succeeds
+ * it simply boots, otherwise it returns the status of last EFI call.
+ */
+static EFI_STATUS
+try_boot()
+{
+	size_t bufsize, loadersize, cmdsize;
+	void *buf, *loaderbuf;
 	char *cmd;
 	dev_info_t *dev;
+	const boot_module_t *mod;
 	EFI_HANDLE loaderhandle;
 	EFI_LOADED_IMAGE *loaded_image;
 	EFI_STATUS status;
 
+	status = load_loader(&mod, &dev, &loaderbuf, &loadersize, TRUE);
+	if (status != EFI_SUCCESS) {
+		status = load_loader(&mod, &dev, &loaderbuf, &loadersize,
+		    FALSE);
+		if (status != EFI_SUCCESS) {
+			printf("Failed to load '%s'\n", PATH_LOADER_EFI);
+			return (status);
+		}
+	}
+
 	/*
 	 * Read in and parse the command line from /boot.config or /boot/config,
 	 * if present. We'll pass it the next stage via a simple ASCII
@@ -111,67 +388,183 @@ try_load(const boot_module_t *mod)
 	 */
 	cmd = NULL;
 	cmdsize = 0;
-	status = mod->load(PATH_DOTCONFIG, &dev, &buf, &bufsize);
+	status = mod->load(PATH_DOTCONFIG, dev, &buf, &bufsize);
 	if (status == EFI_NOT_FOUND)
-		status = mod->load(PATH_CONFIG, &dev, &buf, &bufsize);
+		status = mod->load(PATH_CONFIG, dev, &buf, &bufsize);
 	if (status == EFI_SUCCESS) {
 		cmdsize = bufsize + 1;
 		cmd = malloc(cmdsize);
-		if (cmd == NULL) {
-			free(buf);
-			return;
-		}
+		if (cmd == NULL)
+			goto errout;
 		memcpy(cmd, buf, bufsize);
 		cmd[bufsize] = '\0';
 		free(buf);
+		buf = NULL;
 	}
 
-	status = mod->load(PATH_LOADER_EFI, &dev, &buf, &bufsize);
-	if (status == EFI_NOT_FOUND)
-		return;
-
-	if (status != EFI_SUCCESS) {
-		printf("%s failed to load %s (%lu)\n", mod->name,
-		    PATH_LOADER_EFI, EFI_ERROR_CODE(status));
-		return;
-	}
-
-	if ((status = bs->LoadImage(TRUE, image, dev->devpath, buf, bufsize,
-	    &loaderhandle)) != EFI_SUCCESS) {
+	if ((status = bs->LoadImage(TRUE, image, devpath_last(dev->devpath),
+	    loaderbuf, loadersize, &loaderhandle)) != EFI_SUCCESS) {
 		printf("Failed to load image provided by %s, size: %zu, (%lu)\n",
 		     mod->name, bufsize, EFI_ERROR_CODE(status));
-		return;
+		goto errout;
 	}
 
-	if (cmd != NULL)
-		printf("    command args: %s\n", cmd);
-
 	if ((status = bs->HandleProtocol(loaderhandle, &LoadedImageGUID,
 	    (VOID**)&loaded_image)) != EFI_SUCCESS) {
 		printf("Failed to query LoadedImage provided by %s (%lu)\n",
 		    mod->name, EFI_ERROR_CODE(status));
-		return;
+		goto errout;
 	}
 
+	if (cmd != NULL)
+		printf("    command args: %s\n", cmd);
+
 	loaded_image->DeviceHandle = dev->devhandle;
 	loaded_image->LoadOptionsSize = cmdsize;
 	loaded_image->LoadOptions = cmd;
 
+	DPRINTF("Starting '%s' in 5 seconds...", PATH_LOADER_EFI);
+	DSTALL(1000000);
+	DPRINTF(".");
+	DSTALL(1000000);
+	DPRINTF(".");
+	DSTALL(1000000);
+	DPRINTF(".");
+	DSTALL(1000000);
+	DPRINTF(".");
+	DSTALL(1000000);
+	DPRINTF(".\n");
+
 	if ((status = bs->StartImage(loaderhandle, NULL, NULL)) !=
 	    EFI_SUCCESS) {
 		printf("Failed to start image provided by %s (%lu)\n",
 		    mod->name, EFI_ERROR_CODE(status));
-		free(cmd);
 		loaded_image->LoadOptionsSize = 0;
 		loaded_image->LoadOptions = NULL;
-		return;
 	}
+
+errout:
+	if (cmd != NULL)
+		free(cmd);
+	if (buf != NULL)
+		free(buf);
+	if (loaderbuf != NULL)
+		free(loaderbuf);
+
+	return (status);
+}
+
+/*
+ * probe_handle determines if the passed handle represents a logical partition
+ * if it does it uses each module in order to probe it and if successful it
+ * returns EFI_SUCCESS.
+ */
+static EFI_STATUS
+probe_handle(EFI_HANDLE h, EFI_DEVICE_PATH *imgpath, BOOLEAN *preferred)
+{
+	dev_info_t *devinfo;
+	EFI_BLOCK_IO *blkio;
+	EFI_DEVICE_PATH *devpath;
+	EFI_STATUS status;
+	UINTN i;
+
+	/* Figure out if we're dealing with an actual partition. */
+	status = bs->HandleProtocol(h, &DevicePathGUID, (void **)&devpath);
+	if (status == EFI_UNSUPPORTED)
+		return (status);
+
+	if (status != EFI_SUCCESS) {
+		DPRINTF("\nFailed to query DevicePath (%lu)\n",
+		    EFI_ERROR_CODE(status));
+		return (status);
+	}
+
+	DPRINTF("probing: %s\n", devpath_str(devpath));
+
+	status = bs->HandleProtocol(h, &BlockIoProtocolGUID, (void **)&blkio);
+	if (status == EFI_UNSUPPORTED)
+		return (status);
+
+	if (status != EFI_SUCCESS) {
+		DPRINTF("\nFailed to query BlockIoProtocol (%lu)\n",
+		    EFI_ERROR_CODE(status));
+		return (status);
+	}
+
+	if (!blkio->Media->LogicalPartition)
+		return (EFI_UNSUPPORTED);
+
+	*preferred = device_paths_match(imgpath, devpath);
+
+	/* Run through each module, see if it can load this partition */
+	for (i = 0; i < NUM_BOOT_MODULES; i++) {
+		if (boot_modules[i] == NULL)
+			continue;
+
+		if ((status = bs->AllocatePool(EfiLoaderData,
+		    sizeof(*devinfo), (void **)&devinfo)) !=
+		    EFI_SUCCESS) {
+			DPRINTF("\nFailed to allocate devinfo (%lu)\n",
+			    EFI_ERROR_CODE(status));
+			continue;
+		}
+		devinfo->dev = blkio;
+		devinfo->devpath = devpath;
+		devinfo->devhandle = h;
+		devinfo->devdata = NULL;
+		devinfo->preferred = *preferred;
+		devinfo->next = NULL;
+
+		status = boot_modules[i]->probe(devinfo);
+		if (status == EFI_SUCCESS)
+			return (EFI_SUCCESS);
+		(void)bs->FreePool(devinfo);
+	}
+
+	return (EFI_UNSUPPORTED);
+}
+
+/*
+ * probe_handle_status calls probe_handle and outputs the returned status
+ * of the call.
+ */
+static void
+probe_handle_status(EFI_HANDLE h, EFI_DEVICE_PATH *imgpath)
+{
+	EFI_STATUS status;
+	BOOLEAN preferred;
+
+	status = probe_handle(h, imgpath, &preferred);
+	
+	DPRINTF("probe: ");
+	switch (status) {
+	case EFI_UNSUPPORTED:
+		printf(".");
+		DPRINTF(" not supported\n");
+		break;
+	case EFI_SUCCESS:
+		if (preferred) {
+			printf("%c", '*');
+			DPRINTF(" supported (preferred)\n");
+		} else {
+			printf("%c", '+');
+			DPRINTF(" supported\n");
+		}
+		break;
+	default:
+		printf("x");
+		DPRINTF(" error (%lu)\n", EFI_ERROR_CODE(status));
+		break;
+	}
+	DSTALL(500000);
 }
 
 EFI_STATUS
 efi_main(EFI_HANDLE Ximage, EFI_SYSTEM_TABLE *Xsystab)
 {
 	EFI_HANDLE *handles;
+	EFI_LOADED_IMAGE *img;
+	EFI_DEVICE_PATH *imgpath;
 	EFI_STATUS status;
 	EFI_CONSOLE_CONTROL_PROTOCOL *ConsoleControl = NULL;
 	SIMPLE_TEXT_OUTPUT_INTERFACE *conout = NULL;
@@ -254,20 +647,22 @@ efi_main(EFI_HANDLE Ximage, EFI_SYSTEM_TABLE *Xsystab)
 	/* Scan all partitions, probing with all modules. */
 	nhandles = hsize / sizeof(*handles);
 	printf("   Probing %zu block devices...", nhandles);
-	for (i = 0; i < nhandles; i++) {
-		status = probe_handle(handles[i]);
-		switch (status) {
-		case EFI_UNSUPPORTED:
-			printf(".");
-			break;
-		case EFI_SUCCESS:
-			printf("+");
-			break;
-		default:
-			printf("x");
-			break;
-		}
+	DPRINTF("\n");
+
+	/* Determine the devpath of our image so we can prefer it. */
+	status = bs->HandleProtocol(image, &LoadedImageGUID, (VOID**)&img);
+	imgpath = NULL;
+	if (status == EFI_SUCCESS) {
+		status = bs->HandleProtocol(img->DeviceHandle, &DevicePathGUID,
+		    (void **)&imgpath);
+		if (status != EFI_SUCCESS)
+			DPRINTF("Failed to get image DevicePath (%lu)\n",
+			    EFI_ERROR_CODE(status));
+		DPRINTF("boot1 imagepath: %s\n", devpath_str(imgpath));
 	}
+
+	for (i = 0; i < nhandles; i++)
+		probe_handle_status(handles[i], imgpath);
 	printf(" done\n");
 
 	/* Status summary. */
@@ -278,78 +673,15 @@ efi_main(EFI_HANDLE Ximage, EFI_SYSTEM_TABLE *Xsystab)
 		}
 	}
 
-	/* Select a partition to boot by trying each module in order. */
-	for (i = 0; i < NUM_BOOT_MODULES; i++)
-		if (boot_modules[i] != NULL)
-			try_load(boot_modules[i]);
+	try_boot();
 
 	/* If we get here, we're out of luck... */
 	panic("No bootable partitions found!");
 }
 
-static EFI_STATUS
-probe_handle(EFI_HANDLE h)
-{
-	dev_info_t *devinfo;
-	EFI_BLOCK_IO *blkio;
-	EFI_DEVICE_PATH *devpath;
-	EFI_STATUS status;
-	UINTN i;
-
-	/* Figure out if we're dealing with an actual partition. */
-	status = bs->HandleProtocol(h, &DevicePathGUID, (void **)&devpath);
-	if (status == EFI_UNSUPPORTED)
-		return (status);
-
-	if (status != EFI_SUCCESS) {
-		DPRINTF("\nFailed to query DevicePath (%lu)\n",
-		    EFI_ERROR_CODE(status));
-		return (status);
-	}
-
-	while (!IsDevicePathEnd(NextDevicePathNode(devpath)))
-		devpath = NextDevicePathNode(devpath);
-
-	status = bs->HandleProtocol(h, &BlockIoProtocolGUID, (void **)&blkio);
-	if (status == EFI_UNSUPPORTED)
-		return (status);
-
-	if (status != EFI_SUCCESS) {
-		DPRINTF("\nFailed to query BlockIoProtocol (%lu)\n",
-		    EFI_ERROR_CODE(status));
-		return (status);
-	}
-
-	if (!blkio->Media->LogicalPartition)
-		return (EFI_UNSUPPORTED);
-
-	/* Run through each module, see if it can load this partition */
-	for (i = 0; i < NUM_BOOT_MODULES; i++) {
-		if (boot_modules[i] == NULL)
-			continue;
-
-		if ((status = bs->AllocatePool(EfiLoaderData,
-		    sizeof(*devinfo), (void **)&devinfo)) !=
-		    EFI_SUCCESS) {
-			DPRINTF("\nFailed to allocate devinfo (%lu)\n",
-			    EFI_ERROR_CODE(status));
-			continue;
-		}
-		devinfo->dev = blkio;
-		devinfo->devpath = devpath;
-		devinfo->devhandle = h;
-		devinfo->devdata = NULL;
-		devinfo->next = NULL;
-
-		status = boot_modules[i]->probe(devinfo);
-		if (status == EFI_SUCCESS)
-			return (EFI_SUCCESS);
-		(void)bs->FreePool(devinfo);
-	}
-
-	return (EFI_UNSUPPORTED);
-}
-
+/*
+ * add_device adds a device to the passed devinfo list.
+ */
 void
 add_device(dev_info_t **devinfop, dev_info_t *devinfo)
 {
diff --git a/sys/boot/efi/boot1/boot_module.h b/sys/boot/efi/boot1/boot_module.h
index 2c158f6..296d5a6 100644
--- a/sys/boot/efi/boot1/boot_module.h
+++ b/sys/boot/efi/boot1/boot_module.h
@@ -36,9 +36,11 @@
 #include <eficonsctl.h>
 
 #ifdef EFI_DEBUG
-#define DPRINTF(fmt, ...) printf(fmt, __VA_ARGS__)
+#define DPRINTF(fmt, args...) printf(fmt, ##args)
+#define DSTALL(d) bs->Stall(d)
 #else
 #define DPRINTF(fmt, ...) {}
+#define DSTALL(d) {}
 #endif
 
 /* EFI device info */
@@ -48,6 +50,7 @@ typedef struct dev_info
 	EFI_DEVICE_PATH *devpath;
 	EFI_HANDLE *devhandle;
 	void *devdata;
+	BOOLEAN preferred;
 	struct dev_info *next;
 } dev_info_t;
 
@@ -75,19 +78,21 @@ typedef struct boot_module_t
 
 	/*
 	 * load should select the best out of a set of devices that probe
-	 * indicated were loadable and load it.
+	 * indicated were loadable and load the specified file.
 	 *
 	 * Return codes:
 	 * EFI_SUCCESS = The module can handle the device.
 	 * EFI_NOT_FOUND = The module can not handle the device.
 	 * Other = The module encountered an error.
 	 */
-	EFI_STATUS (*load)(const char *loader_path, dev_info_t **devinfo,
+	EFI_STATUS (*load)(const char *filepath, dev_info_t *devinfo,
 	    void **buf, size_t *bufsize);
 
 	/* status outputs information about the probed devices. */
 	void (*status)();
 
+	/* valid devices as found by probe. */
+	dev_info_t *(*devices)();
 } boot_module_t;
 
 /* Standard boot modules. */
@@ -107,4 +112,6 @@ extern int vsnprintf(char *str, size_t sz, const char *fmt, va_list ap);
 extern EFI_SYSTEM_TABLE *systab;
 extern EFI_BOOT_SERVICES *bs;
 
+extern int devpath_strlcat(char *buf, size_t size, EFI_DEVICE_PATH *devpath);
+extern char *devpath_str(EFI_DEVICE_PATH *devpath);
 #endif
diff --git a/sys/boot/efi/boot1/ufs_module.c b/sys/boot/efi/boot1/ufs_module.c
index 07c7152..63087ea 100644
--- a/sys/boot/efi/boot1/ufs_module.c
+++ b/sys/boot/efi/boot1/ufs_module.c
@@ -93,7 +93,7 @@ probe(dev_info_t* dev)
 }
 
 static EFI_STATUS
-try_load(dev_info_t *dev, const char *loader_path, void **bufp, size_t *bufsize)
+load(const char *filepath, dev_info_t *dev, void **bufp, size_t *bufsize)
 {
 	ufs_ino_t ino;
 	EFI_STATUS status;
@@ -101,59 +101,46 @@ try_load(dev_info_t *dev, const char *loader_path, void **bufp, size_t *bufsize)
 	ssize_t read;
 	void *buf;
 
-	if (init_dev(dev) < 0)
+	DPRINTF("Loading '%s' from %s\n", filepath, devpath_str(dev->devpath));
+
+	if (init_dev(dev) < 0) {
+		DPRINTF("Failed to init device\n");
 		return (EFI_UNSUPPORTED);
+	}
 
-	if ((ino = lookup(loader_path)) == 0)
+	if ((ino = lookup(filepath)) == 0) {
+		DPRINTF("Failed to lookup '%s' (file not found?)\n", filepath);
 		return (EFI_NOT_FOUND);
+	}
 
 	if (fsread_size(ino, NULL, 0, &size) < 0 || size <= 0) {
-		printf("Failed to read size of '%s' ino: %d\n", loader_path,
-		    ino);
+		printf("Failed to read size of '%s' ino: %d\n", filepath, ino);
 		return (EFI_INVALID_PARAMETER);
 	}
 
 	if ((status = bs->AllocatePool(EfiLoaderData, size, &buf)) !=
 	    EFI_SUCCESS) {
-		printf("Failed to allocate read buffer (%lu)\n",
-		    EFI_ERROR_CODE(status));
+		printf("Failed to allocate read buffer %zu for '%s' (%lu)\n",
+		    size, filepath, EFI_ERROR_CODE(status));
 		return (status);
 	}
 
 	read = fsread(ino, buf, size);
 	if ((size_t)read != size) {
-		printf("Failed to read '%s' (%zd != %zu)\n", loader_path, read,
+		printf("Failed to read '%s' (%zd != %zu)\n", filepath, read,
 		    size);
 		(void)bs->FreePool(buf);
 		return (EFI_INVALID_PARAMETER);
 	}
 
+	DPRINTF("Load complete\n");
+
 	*bufp = buf;
 	*bufsize = size;
 
 	return (EFI_SUCCESS);
 }
 
-static EFI_STATUS
-load(const char *loader_path, dev_info_t **devinfop, void **buf,
-    size_t *bufsize)
-{
-	dev_info_t *dev;
-	EFI_STATUS status;
-
-	for (dev = devices; dev != NULL; dev = dev->next) {
-		status = try_load(dev, loader_path, buf, bufsize);
-		if (status == EFI_SUCCESS) {
-			*devinfop = dev;
-			return (EFI_SUCCESS);
-		} else if (status != EFI_NOT_FOUND) {
-			return (status);
-		}
-	}
-
-	return (EFI_NOT_FOUND);
-}
-
 static void
 status()
 {
@@ -176,10 +163,18 @@ status()
 	}
 }
 
+static dev_info_t *
+_devices()
+{
+
+	return (devices);
+}
+
 const boot_module_t ufs_module =
 {
 	.name = "UFS",
 	.probe = probe,
 	.load = load,
-	.status = status
+	.status = status,
+	.devices = _devices
 };
diff --git a/sys/boot/efi/boot1/zfs_module.c b/sys/boot/efi/boot1/zfs_module.c
index 96eec33..4e2c5c4 100644
--- a/sys/boot/efi/boot1/zfs_module.c
+++ b/sys/boot/efi/boot1/zfs_module.c
@@ -91,7 +91,7 @@ probe(dev_info_t *dev)
 }
 
 static EFI_STATUS
-try_load(dev_info_t *devinfo, const char *loader_path, void **bufp, size_t *bufsize)
+load(const char *filepath, dev_info_t *devinfo, void **bufp, size_t *bufsize)
 {
 	spa_t *spa;
 	struct zfsmount zfsmount;
@@ -102,32 +102,41 @@ try_load(dev_info_t *devinfo, const char *loader_path, void **bufp, size_t *bufs
 	EFI_STATUS status;
 
 	spa = devinfo->devdata;
-	if (zfs_spa_init(spa) != 0) {
-		/* Init failed, don't report this loudly. */
+
+	DPRINTF("load: '%s' spa: '%s', devpath: %s\n", filepath, spa->spa_name,
+	    devpath_str(devinfo->devpath));
+
+	if ((err = zfs_spa_init(spa)) != 0) {
+		DPRINTF("Failed to load pool '%s' (%d)\n", spa->spa_name, err);
 		return (EFI_NOT_FOUND);
 	}
 
-	if (zfs_mount(spa, 0, &zfsmount) != 0) {
-		/* Mount failed, don't report this loudly. */
+	if ((err = zfs_mount(spa, 0, &zfsmount)) != 0) {
+		DPRINTF("Failed to mount pool '%s' (%d)\n", spa->spa_name, err);
 		return (EFI_NOT_FOUND);
 	}
 
-	if ((err = zfs_lookup(&zfsmount, loader_path, &dn)) != 0) {
-		printf("Failed to lookup %s on pool %s (%d)\n", loader_path,
+	if ((err = zfs_lookup(&zfsmount, filepath, &dn)) != 0) {
+		if (err == ENOENT) {
+			DPRINTF("Failed to find '%s' on pool '%s' (%d)\n",
+			    filepath, spa->spa_name, err);
+			return (EFI_NOT_FOUND);
+		}
+		printf("Failed to lookup '%s' on pool '%s' (%d)\n", filepath,
 		    spa->spa_name, err);
 		return (EFI_INVALID_PARAMETER);
 	}
 
 	if ((err = zfs_dnode_stat(spa, &dn, &st)) != 0) {
-		printf("Failed to lookup %s on pool %s (%d)\n", loader_path,
+		printf("Failed to stat '%s' on pool '%s' (%d)\n", filepath,
 		    spa->spa_name, err);
 		return (EFI_INVALID_PARAMETER);
 	}
 
 	if ((status = bs->AllocatePool(EfiLoaderData, (UINTN)st.st_size, &buf))
 	    != EFI_SUCCESS) {
-		printf("Failed to allocate load buffer for pool %s (%lu)\n",
-		    spa->spa_name, EFI_ERROR_CODE(status));
+		printf("Failed to allocate load buffer %zu for pool '%s' for '%s' "
+		    "(%lu)\n", st.st_size, spa->spa_name, filepath, EFI_ERROR_CODE(status));
 		return (EFI_INVALID_PARAMETER);
 	}
 
@@ -144,26 +153,6 @@ try_load(dev_info_t *devinfo, const char *loader_path, void **bufp, size_t *bufs
 	return (EFI_SUCCESS);
 }
 
-static EFI_STATUS
-load(const char *loader_path, dev_info_t **devinfop, void **bufp,
-    size_t *bufsize)
-{
-	dev_info_t *devinfo;
-	EFI_STATUS status;
-
-	for (devinfo = devices; devinfo != NULL; devinfo = devinfo->next) {
-		status = try_load(devinfo, loader_path, bufp, bufsize);
-		if (status == EFI_SUCCESS) {
-			*devinfop = devinfo;
-			return (EFI_SUCCESS);
-		} else if (status != EFI_NOT_FOUND) {
-			return (status);
-		}
-	}
-
-	return (EFI_NOT_FOUND);
-}
-
 static void
 status()
 {
@@ -189,11 +178,19 @@ init()
 	zfs_init();
 }
 
+static dev_info_t *
+_devices()
+{
+
+	return (devices);
+}
+
 const boot_module_t zfs_module =
 {
 	.name = "ZFS",
 	.init = init,
 	.probe = probe,
 	.load = load,
-	.status = status
+	.status = status,
+	.devices = _devices
 };
diff --git a/sys/boot/efi/include/efidevp.h b/sys/boot/efi/include/efidevp.h
index f0f49ef..dda79de7 100644
--- a/sys/boot/efi/include/efidevp.h
+++ b/sys/boot/efi/include/efidevp.h
@@ -40,9 +40,7 @@ typedef struct _EFI_DEVICE_PATH {
 #define EFI_DP_TYPE_MASK                    0x7F
 #define EFI_DP_TYPE_UNPACKED                0x80
 
-//#define END_DEVICE_PATH_TYPE                0xff
 #define END_DEVICE_PATH_TYPE                0x7f
-//#define END_DEVICE_PATH_TYPE_UNPACKED       0x7f
 
 #define END_ENTIRE_DEVICE_PATH_SUBTYPE      0xff
 #define END_INSTANCE_DEVICE_PATH_SUBTYPE    0x01
@@ -56,8 +54,8 @@ typedef struct _EFI_DEVICE_PATH {
 #define DevicePathSubType(a)        ( (a)->SubType )
 #define DevicePathNodeLength(a)     ( ((a)->Length[0]) | ((a)->Length[1] << 8) )
 #define NextDevicePathNode(a)       ( (EFI_DEVICE_PATH *) ( ((UINT8 *) (a)) + DevicePathNodeLength(a)))
-//#define IsDevicePathEndType(a)      ( DevicePathType(a) == END_DEVICE_PATH_TYPE_UNPACKED )
-#define IsDevicePathEndType(a)      ( DevicePathType(a) == END_DEVICE_PATH_TYPE )
+#define IsDevicePathType(a, t)      ( DevicePathType(a) == t )
+#define IsDevicePathEndType(a)      IsDevicePathType(a, END_DEVICE_PATH_TYPE)
 #define IsDevicePathEndSubType(a)   ( (a)->SubType == END_ENTIRE_DEVICE_PATH_SUBTYPE )
 #define IsDevicePathEnd(a)          ( IsDevicePathEndType(a) && IsDevicePathEndSubType(a) )
 #define IsDevicePathUnpacked(a)     ( (a)->Type & EFI_DP_TYPE_UNPACKED )
@@ -285,6 +283,13 @@ typedef struct _UART_DEVICE_PATH {
 #define DEVICE_PATH_MESSAGING_VT_UTF8 \
     { 0xad15a0d6, 0x8bec, 0x4acf, {0xa0, 0x73, 0xd0, 0x1d, 0xe7, 0x7e, 0x2d, 0x88} }
 
+#define MSG_SATA_DP			0x12
+typedef struct _SATA_DEVICE_PATH {
+	EFI_DEVICE_PATH			Header;
+	UINT16				HBAPortNumber;
+	UINT16				PortMultiplierPortNumber;
+	UINT16				Lun;
+} SATA_DEVICE_PATH;
 
 #define MEDIA_DEVICE_PATH               0x04