[efi] Allow network devices to be created on top of arbitrary SNP devices

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

11 files changed, 643 insertions, 397 deletions

diff --git a/src/drivers/net/efi/snp.c b/src/drivers/net/efi/snp.c
new file mode 100644
index 00000000..98619d66
--- /dev/null
+++ b/src/drivers/net/efi/snp.c
@@ -0,0 +1,219 @@
+/*
+ * Copyright (C) 2014 Michael Brown <mbrown@fensystems.co.uk>.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA.
+ */
+
+FILE_LICENCE ( GPL2_OR_LATER );
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+#include <ipxe/efi/efi.h>
+#include <ipxe/efi/Protocol/SimpleNetwork.h>
+#include <ipxe/efi/efi_driver.h>
+#include <ipxe/efi/efi_snp.h>
+#include <ipxe/efi/efi_pci.h>
+#include "snpnet.h"
+#include "snp.h"
+
+/** @file
+ *
+ * SNP driver
+ *
+ */
+
+/** EFI simple network protocol GUID */
+static EFI_GUID efi_simple_network_protocol_guid
+	= EFI_SIMPLE_NETWORK_PROTOCOL_GUID;
+
+/** EFI PCI I/O protocol GUID */
+static EFI_GUID efi_pci_io_protocol_guid
+	= EFI_PCI_IO_PROTOCOL_GUID;
+
+/**
+ * Check to see if driver supports a device
+ *
+ * @v device		EFI device handle
+ * @ret rc		Return status code
+ */
+static int snp_supported ( EFI_HANDLE device ) {
+	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+	EFI_STATUS efirc;
+
+	/* Check that this is not a device we are providing ourselves */
+	if ( find_snpdev ( device ) != NULL ) {
+		DBGCP ( device, "SNP %p %s is provided by this binary\n",
+			device, efi_handle_devpath_text ( device ) );
+		return -ENOTTY;
+	}
+
+	/* Test for presence of simple network protocol */
+	if ( ( efirc = bs->OpenProtocol ( device,
+					  &efi_simple_network_protocol_guid,
+					  NULL, efi_image_handle, device,
+					  EFI_OPEN_PROTOCOL_TEST_PROTOCOL))!=0){
+		DBGCP ( device, "SNP %p %s is not an SNP device\n",
+			device, efi_handle_devpath_text ( device ) );
+		return -EEFI ( efirc );
+	}
+	DBGC ( device, "SNP %p %s is an SNP device\n",
+	       device, efi_handle_devpath_text ( device ) );
+
+	return 0;
+}
+
+/**
+ * Get underlying PCI device information
+ *
+ * @v snpdev		SNP device
+ * @ret rc		Return status code
+ */
+static int snp_pci_info ( struct snp_device *snpdev ) {
+	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+	struct efi_device *efidev = snpdev->efidev;
+	EFI_DEVICE_PATH_PROTOCOL *devpath = efidev->path;
+	struct pci_device pci;
+	EFI_HANDLE device;
+	EFI_STATUS efirc;
+	int rc;
+
+	/* Check for presence of PCI I/O protocol */
+	if ( ( efirc = bs->LocateDevicePath ( &efi_pci_io_protocol_guid,
+					      &devpath, &device ) ) != 0 ) {
+		DBGC ( efidev->device, "SNP %p %s is not a PCI device\n",
+		       efidev->device, efi_devpath_text ( efidev->path ) );
+		return -EEFI ( efirc );
+	}
+
+	/* Get PCI device information */
+	if ( ( rc = efipci_info ( device, &pci ) ) != 0 ) {
+		DBGC ( efidev->device, "SNP %p %s could not get PCI "
+		       "information: %s\n", efidev->device,
+		       efi_devpath_text ( efidev->path ), strerror ( rc ) );
+		return rc;
+	}
+
+	/* Populate SNP device information */
+	memcpy ( &snpdev->dev.desc, &pci.dev.desc, sizeof ( snpdev->dev.desc ));
+	snprintf ( snpdev->dev.name, sizeof ( snpdev->dev.name ), "SNP-%s",
+		   pci.dev.name );
+
+	return 0;
+}
+
+/**
+ * Attach driver to device
+ *
+ * @v efidev		EFI device
+ * @ret rc		Return status code
+ */
+static int snp_start ( struct efi_device *efidev ) {
+	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+	EFI_HANDLE device = efidev->device;
+	struct snp_device *snpdev;
+	union {
+		EFI_SIMPLE_NETWORK_PROTOCOL *snp;
+		void *interface;
+	} snp;
+	EFI_STATUS efirc;
+	int rc;
+
+	/* Check that this is not a device we are providing ourselves */
+	if ( find_snpdev ( efidev->device ) != NULL ) {
+		DBGCP ( device, "SNP %p %s is provided by this binary\n",
+			device, efi_devpath_text ( efidev->path ) );
+		rc = -ENOTTY;
+		goto err_own;
+	}
+
+	/* Allocate and initialise structure */
+	snpdev = zalloc ( sizeof ( *snpdev ) );
+	if ( ! snpdev ) {
+		rc = -ENOMEM;
+		goto err_alloc;
+	}
+	snpdev->efidev = efidev;
+	snpdev->dev.driver_name = "SNP";
+	INIT_LIST_HEAD ( &snpdev->dev.children );
+
+	/* See if device is an SNP device */
+	if ( ( efirc = bs->OpenProtocol ( device,
+					  &efi_simple_network_protocol_guid,
+					  &snp.interface, efi_image_handle,
+					  device,
+					  ( EFI_OPEN_PROTOCOL_BY_DRIVER |
+					    EFI_OPEN_PROTOCOL_EXCLUSIVE )))!=0){
+		rc = -EEFI ( efirc );
+		DBGCP ( device, "SNP %p %s cannot open SNP protocol: %s\n",
+			device, efi_devpath_text ( efidev->path ),
+			strerror ( rc ) );
+		goto err_open_protocol;
+	}
+	snpdev->snp = snp.snp;
+
+	/* Get underlying device information */
+	if ( ( rc = snp_pci_info ( snpdev ) ) != 0 )
+		goto err_info;
+
+	/* Mark SNP device as a child of the EFI device */
+	snpdev->dev.parent = &efidev->dev;
+	list_add ( &snpdev->dev.siblings, &efidev->dev.children );
+
+	/* Create SNP network device */
+	if ( ( rc = snpnet_probe ( snpdev ) ) != 0 )
+		goto err_probe;
+
+	efidev_set_drvdata ( efidev, snpdev );
+	return 0;
+
+	snpnet_remove ( snpdev );
+ err_probe:
+	list_del ( &snpdev->dev.siblings );
+ err_info:
+	bs->CloseProtocol ( device, &efi_simple_network_protocol_guid,
+			    efi_image_handle, device );
+ err_open_protocol:
+	free ( snpdev );
+ err_alloc:
+ err_own:
+	return rc;
+}
+
+/**
+ * Detach driver from device
+ *
+ * @v efidev		EFI device
+  */
+static void snp_stop ( struct efi_device *efidev ) {
+	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+	struct snp_device *snpdev = efidev_get_drvdata ( efidev );
+
+	snpnet_remove ( snpdev );
+	list_del ( &snpdev->dev.siblings );
+	bs->CloseProtocol ( efidev->device, &efi_simple_network_protocol_guid,
+			    efi_image_handle, efidev->device );
+	free ( snpdev );
+}
+
+/** EFI SNP driver */
+struct efi_driver snp_driver __efi_driver ( EFI_DRIVER_NORMAL ) = {
+	.name = "SNP",
+	.supported = snp_supported,
+	.start = snp_start,
+	.stop = snp_stop,
+};
diff --git a/src/drivers/net/efi/snp.h b/src/drivers/net/efi/snp.h
index 4d6b1014..718d1231 100644
--- a/src/drivers/net/efi/snp.h
+++ b/src/drivers/net/efi/snp.h
@@ -28,22 +28,21 @@
 FILE_LICENCE ( GPL2_OR_LATER );
 
 #include <ipxe/device.h>
-#include <ipxe/netdevice.h>
+#include <ipxe/efi/efi.h>
 #include <ipxe/efi/Protocol/SimpleNetwork.h>
 
-/** A network device that consumes the EFI Simple Network Protocol */
+/** An SNP device */
 struct snp_device {
-	/** Underlying simple network protocol instance */
+	/** EFI device */
+	struct efi_device *efidev;
+	/** Simple network protocol */
 	EFI_SIMPLE_NETWORK_PROTOCOL *snp;
-
 	/** Generic device */
 	struct device dev;
-
 	/** Network device */
 	struct net_device *netdev;
-
-	/** State to put the snp in when removing the device */
-	uint32 removal_state;
+	/** State to restore when removing the device */
+	UINT32 removal_state;
 };
 
 #endif /* _SNP_H */
diff --git a/src/include/ipxe/device.h b/src/include/ipxe/device.h
index c59697c0..89509734 100644
--- a/src/include/ipxe/device.h
+++ b/src/include/ipxe/device.h
@@ -57,6 +57,9 @@ struct device_description {
 /** TAP bus type */
 #define BUS_TYPE_TAP 6
 
+/** EFI bus type */
+#define BUS_TYPE_EFI 7
+
 /** A hardware device */
 struct device {
 	/** Name */
diff --git a/src/include/ipxe/efi/efi_driver.h b/src/include/ipxe/efi/efi_driver.h
index 600d304f..21bff0e5 100644
--- a/src/include/ipxe/efi/efi_driver.h
+++ b/src/include/ipxe/efi/efi_driver.h
@@ -8,10 +8,25 @@
 
 FILE_LICENCE ( GPL2_OR_LATER );
 
+#include <ipxe/device.h>
 #include <ipxe/tables.h>
 #include <ipxe/efi/efi.h>
 #include <ipxe/efi/Protocol/DevicePath.h>
 
+/** An EFI device */
+struct efi_device {
+	/** Generic device */
+	struct device dev;
+	/** EFI device handle */
+	EFI_HANDLE device;
+	/** Device path */
+	EFI_DEVICE_PATH_PROTOCOL *path;
+	/** Driver for this device */
+	struct efi_driver *driver;
+	/** Driver-private data */
+	void *priv;
+};
+
 /** An EFI driver */
 struct efi_driver {
 	/** Name */
@@ -19,23 +34,23 @@ struct efi_driver {
 	/**
 	 * Check if driver supports device
 	 *
-	 * @v device		Device
+	 * @v device		EFI device handle
 	 * @ret rc		Return status code
 	 */
 	int ( * supported ) ( EFI_HANDLE device );
 	/**
 	 * Attach driver to device
 	 *
-	 * @v device		Device
+	 * @v efidev		EFI device
 	 * @ret rc		Return status code
 	 */
-	int ( * start ) ( EFI_HANDLE device );
+	int ( * start ) ( struct efi_device *efidev );
 	/**
 	 * Detach driver from device
 	 *
-	 * @v device		Device
+	 * @v efidev		EFI device
 	 */
-	void ( * stop ) ( EFI_HANDLE device );
+	void ( * stop ) ( struct efi_device *efidev );
 };
 
 /** EFI driver table */
@@ -48,8 +63,32 @@ struct efi_driver {
 #define EFI_DRIVER_NORMAL	02	/**< Normal drivers */
 #define EFI_DRIVER_LATE		03	/**< Late drivers */
 
+/**
+ * Set EFI driver-private data
+ *
+ * @v efidev		EFI device
+ * @v priv		Private data
+ */
+static inline void efidev_set_drvdata ( struct efi_device *efidev,
+					void *priv ) {
+	efidev->priv = priv;
+}
+
+/**
+ * Get EFI driver-private data
+ *
+ * @v efidev		EFI device
+ * @ret priv		Private data
+ */
+static inline void * efidev_get_drvdata ( struct efi_device *efidev ) {
+	return efidev->priv;
+}
+
 extern EFI_DEVICE_PATH_PROTOCOL *
 efi_devpath_end ( EFI_DEVICE_PATH_PROTOCOL *path );
+extern struct efi_device * efidev_parent ( struct device *dev );
+extern int efidev_child_add ( struct efi_device *efidev, EFI_HANDLE device );
+extern void efidev_child_del ( struct efi_device *efidev, EFI_HANDLE device );
 extern int efi_driver_install ( void );
 extern void efi_driver_uninstall ( void );
 extern int efi_driver_connect_all ( void );
diff --git a/src/include/ipxe/efi/efi_pci.h b/src/include/ipxe/efi/efi_pci.h
index c5ff366c..af36613d 100644
--- a/src/include/ipxe/efi/efi_pci.h
+++ b/src/include/ipxe/efi/efi_pci.h
@@ -8,40 +8,18 @@
 
 FILE_LICENCE ( GPL2_OR_LATER );
 
+#include <ipxe/pci.h>
 #include <ipxe/efi/efi.h>
 #include <ipxe/efi/Protocol/PciIo.h>
-#include <ipxe/efi/Protocol/DevicePath.h>
 
 /* PciRootBridgeIo.h uses LShiftU64(), which isn't defined anywhere else */
 static inline EFIAPI uint64_t LShiftU64 ( UINT64 value, UINTN shift ) {
 	return ( value << shift );
 }
 
-struct device;
-
-/** An EFI PCI device */
-struct efi_pci_device {
-	/** List of EFI PCI devices */
-	struct list_head list;
-	/** iPXE PCI device */
-	struct pci_device pci;
-	/** Underlying EFI device */
-	EFI_HANDLE device;
-	/** PCI I/O protocol */
-	EFI_PCI_IO_PROTOCOL *pci_io;
-	/** Device path */
-	EFI_DEVICE_PATH_PROTOCOL *path;
-};
-
-extern int efipci_create ( EFI_HANDLE device, UINT32 attributes,
-			   struct efi_pci_device **efipci );
-extern int efipci_enable ( struct efi_pci_device *efipci );
-extern struct efi_pci_device * efipci_find_efi ( EFI_HANDLE device );
-extern struct efi_pci_device * efipci_find ( struct device *dev );
-extern int efipci_child_add ( struct efi_pci_device *efipci,
-			      EFI_HANDLE device );
-extern void efipci_child_del ( struct efi_pci_device *efipci,
-			       EFI_HANDLE device );
-extern void efipci_destroy ( struct efi_pci_device *efipci );
+extern int efipci_open ( EFI_HANDLE device, UINT32 attributes,
+			 struct pci_device *pci );
+extern void efipci_close ( EFI_HANDLE device );
+extern int efipci_info ( EFI_HANDLE device, struct pci_device *pci );
 
 #endif /* _IPXE_EFI_PCI_H */
diff --git a/src/include/ipxe/efi/efi_snp.h b/src/include/ipxe/efi/efi_snp.h
index a468165c..06483cfb 100644
--- a/src/include/ipxe/efi/efi_snp.h
+++ b/src/include/ipxe/efi/efi_snp.h
@@ -24,8 +24,8 @@ struct efi_snp_device {
 	struct list_head list;
 	/** The underlying iPXE network device */
 	struct net_device *netdev;
-	/** The underlying EFI PCI device */
-	struct efi_pci_device *efipci;
+	/** The underlying EFI device */
+	struct efi_device *efidev;
 	/** EFI device handle */
 	EFI_HANDLE handle;
 	/** The SNP structure itself */
@@ -76,6 +76,7 @@ struct efi_snp_device {
 
 extern int efi_snp_hii_install ( struct efi_snp_device *snpdev );
 extern void efi_snp_hii_uninstall ( struct efi_snp_device *snpdev );
+extern struct efi_snp_device * find_snpdev ( EFI_HANDLE handle );
 extern struct efi_snp_device * last_opened_snpdev ( void );
 extern void efi_snp_claim ( void );
 extern void efi_snp_release ( void );
diff --git a/src/include/ipxe/errfile.h b/src/include/ipxe/errfile.h
index e3b6b87e..a8b3a8e4 100644
--- a/src/include/ipxe/errfile.h
+++ b/src/include/ipxe/errfile.h
@@ -154,6 +154,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #define ERRFILE_intel		     ( ERRFILE_DRIVER | 0x00650000 )
 #define ERRFILE_myson		     ( ERRFILE_DRIVER | 0x00660000 )
 #define ERRFILE_intelx		     ( ERRFILE_DRIVER | 0x00670000 )
+#define ERRFILE_snp		     ( ERRFILE_DRIVER | 0x00680000 )
 
 #define ERRFILE_scsi		     ( ERRFILE_DRIVER | 0x00700000 )
 #define ERRFILE_arbel		     ( ERRFILE_DRIVER | 0x00710000 )
diff --git a/src/interface/efi/efi_bofm.c b/src/interface/efi/efi_bofm.c
index 2a258a9b..e49dbe40 100644
--- a/src/interface/efi/efi_bofm.c
+++ b/src/interface/efi/efi_bofm.c
@@ -155,38 +155,28 @@ static EFI_GUID bofm2_protocol_guid =
 /**
  * Check if device is supported
  *
- * @v device		EFI device
+ * @v device		EFI device handle
  * @ret rc		Return status code
  */
 static int efi_bofm_supported ( EFI_HANDLE device ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+	struct pci_device pci;
 	union {
 		IBM_BOFM_DRIVER_CONFIGURATION_PROTOCOL *bofm1;
 		void *interface;
 	} bofm1;
-	struct efi_pci_device *efipci;
 	EFI_STATUS efirc;
 	int rc;
 
-	/* Do nothing if we are already driving this device */
-	efipci = efipci_find_efi ( device );
-	if ( efipci ) {
-		DBGCP ( device, "EFIBOFM %p %s already started\n",
-			device, efi_devpath_text ( efipci->path ) );
-		rc = -EALREADY;
-		goto err_already_started;
-	}
-
-	/* Create corresponding PCI device, if any */
-	if ( ( rc = efipci_create ( device, EFI_OPEN_PROTOCOL_GET_PROTOCOL,
-				    &efipci ) ) != 0 )
-		goto err_create;
+	/* Get PCI device information */
+	if ( ( rc = efipci_info ( device, &pci ) ) != 0 )
+		return rc;
 
 	/* Look for a BOFM driver */
-	if ( ( rc = bofm_find_driver ( &efipci->pci ) ) != 0 ) {
+	if ( ( rc = bofm_find_driver ( &pci ) ) != 0 ) {
 		DBGCP ( device, "EFIBOFM %p %s has no driver\n",
-			device, efi_devpath_text ( efipci->path ) );
-		goto err_no_driver;
+			device, efi_handle_devpath_text ( device ) );
+		return rc;
 	}
 
 	/* Locate BOFM protocol */
@@ -194,8 +184,8 @@ static int efi_bofm_supported ( EFI_HANDLE device ) {
 					    &bofm1.interface ) ) != 0 ) {
 		rc = -EEFI ( efirc );
 		DBGC ( device, "EFIBOFM %p %s cannot find BOFM protocol\n",
-		       device, efi_devpath_text ( efipci->path ) );
-		goto err_not_bofm;
+		       device, efi_handle_devpath_text ( device ) );
+		return rc;
 	}
 
 	/* Register support for this device */
@@ -205,36 +195,25 @@ static int efi_bofm_supported ( EFI_HANDLE device ) {
 						      0x02 /* Version */ ))!=0){
 		rc = -EEFI ( efirc );
 		DBGC ( device, "EFIBOFM %p %s could not register support: %s\n",
-		       device, efi_devpath_text ( efipci->path ),
+		       device, efi_handle_devpath_text ( device ),
 		       strerror ( rc ) );
-		goto err_cannot_register;
+		return rc;
 	}
 
-	DBGC ( device, "EFIBOFM %p %s has driver \"%s\"\n", device,
-	       efi_devpath_text ( efipci->path ), efipci->pci.id->name );
-
-	/* Destroy temporary PCI device */
-	efipci_destroy ( efipci );
-
+	DBGC ( device, "EFIBOFM %p %s has driver \"%s\"\n",
+	       device, efi_handle_devpath_text ( device ), pci.id->name );
 	return 0;
-
- err_cannot_register:
- err_not_bofm:
- err_no_driver:
-	efipci_destroy ( efipci );
- err_create:
- err_already_started:
-	return rc;
 }
 
 /**
  * Attach driver to device
  *
- * @v device		EFI device
+ * @v efidev		EFI device
  * @ret rc		Return status code
  */
-static int efi_bofm_start ( EFI_HANDLE device ) {
+static int efi_bofm_start ( struct efi_device *efidev ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+	EFI_HANDLE device = efidev->device;
 	union {
 		IBM_BOFM_DRIVER_CONFIGURATION_PROTOCOL *bofm1;
 		void *interface;
@@ -243,42 +222,29 @@ static int efi_bofm_start ( EFI_HANDLE device ) {
 		IBM_BOFM_DRIVER_CONFIGURATION_PROTOCOL2 *bofm2;
 		void *interface;
 	} bofm2;
-	struct efi_pci_device *efipci;
+	struct pci_device pci;
 	IBM_BOFM_TABLE *bofmtab;
 	IBM_BOFM_TABLE *bofmtab2;
 	int bofmrc;
 	EFI_STATUS efirc;
 	int rc;
 
-	/* Do nothing if we are already driving this device */
-	efipci = efipci_find_efi ( device );
-	if ( efipci ) {
-		DBGCP ( device, "EFIPCI %p %s already started\n",
-			device, efi_devpath_text ( efipci->path ) );
-		rc = -EALREADY;
-		goto err_already_started;
-	}
-
-	/* Create corresponding PCI device */
-	if ( ( rc = efipci_create ( device, EFI_OPEN_PROTOCOL_GET_PROTOCOL,
-				    &efipci ) ) != 0 )
-		goto err_create;
-
-	/* Enable PCI device */
-	if ( ( rc = efipci_enable ( efipci ) ) != 0 )
-		goto err_enable;
+	/* Open PCI device, if possible */
+	if ( ( rc = efipci_open ( device, EFI_OPEN_PROTOCOL_GET_PROTOCOL,
+				  &pci ) ) != 0 )
+		goto err_open;
 
 	/* Locate BOFM protocol */
 	if ( ( efirc = bs->LocateProtocol ( &bofm1_protocol_guid, NULL,
 					    &bofm1.interface ) ) != 0 ) {
 		rc = -EEFI ( efirc );
 		DBGC ( device, "EFIBOFM %p %s cannot find BOFM protocol\n",
-		       device, efi_devpath_text ( efipci->path ) );
+		       device, efi_devpath_text ( efidev->path ) );
 		goto err_locate_bofm;
 	}
 	bofmtab = &bofm1.bofm1->BofmTable;
 	DBGC ( device, "EFIBOFM %p %s found version 1 BOFM table at %p+%04x\n",
-	       device, efi_devpath_text ( efipci->path ), bofmtab,
+	       device, efi_devpath_text ( efidev->path ), bofmtab,
 	       bofmtab->Parameters.Length );
 
 	/* Locate BOFM2 protocol, if available */
@@ -286,36 +252,35 @@ static int efi_bofm_start ( EFI_HANDLE device ) {
 					    &bofm2.interface ) ) == 0 ) {
 		bofmtab2 = &bofm2.bofm2->BofmTable;
 		DBGC ( device, "EFIBOFM %p %s found version 2 BOFM table at "
-		       "%p+%04x\n", device, efi_devpath_text ( efipci->path ),
+		       "%p+%04x\n", device, efi_devpath_text ( efidev->path ),
 		       bofmtab2, bofmtab2->Parameters.Length );
 		assert ( bofm2.bofm2->RegisterSupport ==
 			 bofm1.bofm1->RegisterSupport );
 	} else {
 		DBGC ( device, "EFIBOFM %p %s cannot find BOFM2 protocol\n",
-		       device, efi_devpath_text ( efipci->path ) );
+		       device, efi_devpath_text ( efidev->path ) );
 		/* Not a fatal error; may be a BOFM1-only system */
 		bofmtab2 = NULL;
 	}
 
 	/* Process BOFM table */
 	DBGC2 ( device, "EFIBOFM %p %s version 1 before processing:\n",
-		device, efi_devpath_text ( efipci->path ) );
+		device, efi_devpath_text ( efidev->path ) );
 	DBGC2_HD ( device, bofmtab, bofmtab->Parameters.Length );
 	if ( bofmtab2 ) {
 		DBGC2 ( device, "EFIBOFM %p %s version 2 before processing:\n",
-			device, efi_devpath_text ( efipci->path ) );
+			device, efi_devpath_text ( efidev->path ) );
 		DBGC2_HD ( device, bofmtab2, bofmtab2->Parameters.Length );
 	}
-	bofmrc = bofm ( virt_to_user ( bofmtab2 ? bofmtab2 : bofmtab ),
-			&efipci->pci );
+	bofmrc = bofm ( virt_to_user ( bofmtab2 ? bofmtab2 : bofmtab ), &pci );
 	DBGC ( device, "EFIBOFM %p %s status %08x\n",
-	       device, efi_devpath_text ( efipci->path ), bofmrc );
+	       device, efi_devpath_text ( efidev->path ), bofmrc );
 	DBGC2 ( device, "EFIBOFM %p %s version 1 after processing:\n",
-		device, efi_devpath_text ( efipci->path ) );
+		device, efi_devpath_text ( efidev->path ) );
 	DBGC2_HD ( device, bofmtab, bofmtab->Parameters.Length );
 	if ( bofmtab2 ) {
 		DBGC2 ( device, "EFIBOFM %p %s version 2 after processing:\n",
-			device, efi_devpath_text ( efipci->path ) );
+			device, efi_devpath_text ( efidev->path ) );
 		DBGC2_HD ( device, bofmtab2, bofmtab2->Parameters.Length );
 	}
 
@@ -326,7 +291,7 @@ static int efi_bofm_start ( EFI_HANDLE device ) {
 			rc = -EEFI ( efirc );
 			DBGC ( device, "EFIBOFM %p %s could not set BOFM2 "
 			       "status: %s\n",
-			       device, efi_devpath_text ( efipci->path ),
+			       device, efi_devpath_text ( efidev->path ),
 			       strerror ( rc ) );
 			goto err_set_status;
 		}
@@ -336,24 +301,19 @@ static int efi_bofm_start ( EFI_HANDLE device ) {
 			rc = -EEFI ( efirc );
 			DBGC ( device, "EFIBOFM %p %s could not set BOFM "
 			       "status: %s\n",
-			       device, efi_devpath_text ( efipci->path ),
+			       device, efi_devpath_text ( efidev->path ),
 			       strerror ( rc ) );
 			goto err_set_status;
 		}
 	}
 
-	/* Destroy the PCI device anyway; we have no further use for it */
-	efipci_destroy ( efipci );
-
 	/* BOFM (ab)uses the "start" method to mean "process and exit" */
-	return -EAGAIN;
+	rc = -EAGAIN;
 
  err_set_status:
  err_locate_bofm:
- err_enable:
-	efipci_destroy ( efipci );
- err_create:
- err_already_started:
+	efipci_close ( device );
+ err_open:
 	return rc;
 }
 
@@ -362,8 +322,10 @@ static int efi_bofm_start ( EFI_HANDLE device ) {
  *
  * @v device		EFI device
  */
-static void efi_bofm_stop ( EFI_HANDLE device __unused ) {
-	/* Nothing to do */
+static void efi_bofm_stop ( struct efi_device *efidev __unused ) {
+
+	/* Should never happen */
+	assert ( 0 );
 }
 
 /** EFI BOFM driver */
diff --git a/src/interface/efi/efi_driver.c b/src/interface/efi/efi_driver.c
index 8ef47133..8d0c9354 100644
--- a/src/interface/efi/efi_driver.c
+++ b/src/interface/efi/efi_driver.c
@@ -20,6 +20,7 @@
 FILE_LICENCE ( GPL2_OR_LATER );
 
 #include <stddef.h>
+#include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
 #include <errno.h>
@@ -27,6 +28,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #include <ipxe/efi/efi.h>
 #include <ipxe/efi/Protocol/DriverBinding.h>
 #include <ipxe/efi/Protocol/ComponentName2.h>
+#include <ipxe/efi/Protocol/DevicePath.h>
 #include <ipxe/efi/efi_strings.h>
 #include <ipxe/efi/efi_driver.h>
 
@@ -46,6 +48,13 @@ static EFI_GUID efi_driver_binding_protocol_guid
 static EFI_GUID efi_component_name2_protocol_guid
 	= EFI_COMPONENT_NAME2_PROTOCOL_GUID;
 
+/** EFI device path protocol GUID */
+static EFI_GUID efi_device_path_protocol_guid
+	= EFI_DEVICE_PATH_PROTOCOL_GUID;
+
+/** List of controlled EFI devices */
+static LIST_HEAD ( efi_devices );
+
 /**
  * Find end of device path
  *
@@ -65,6 +74,99 @@ EFI_DEVICE_PATH_PROTOCOL * efi_devpath_end ( EFI_DEVICE_PATH_PROTOCOL *path ) {
 }
 
 /**
+ * Find EFI device
+ *
+ * @v device		EFI device handle
+ * @ret efidev		EFI device, or NULL if not found
+ */
+static struct efi_device * efidev_find ( EFI_HANDLE device ) {
+	struct efi_device *efidev;
+
+	/* Look for an existing EFI device */
+	list_for_each_entry ( efidev, &efi_devices, dev.siblings ) {
+		if ( efidev->device == device )
+			return efidev;
+	}
+
+	return NULL;
+}
+
+/**
+ * Get parent EFI device
+ *
+ * @v dev		Generic device
+ * @ret efidev		Parent EFI device, or NULL
+ */
+struct efi_device * efidev_parent ( struct device *dev ) {
+	struct device *parent = dev->parent;
+	struct efi_device *efidev;
+
+	/* Check that parent exists and is an EFI device */
+	if ( ! parent )
+		return NULL;
+	if ( parent->desc.bus_type != BUS_TYPE_EFI )
+		return NULL;
+
+	/* Get containing EFI device */
+	efidev = container_of ( parent, struct efi_device, dev );
+	return efidev;
+}
+
+/**
+ * Add EFI device as child of EFI device
+ *
+ * @v efidev		EFI device
+ * @v device		EFI child device handle
+ * @ret efirc		EFI status code
+ */
+int efidev_child_add ( struct efi_device *efidev, EFI_HANDLE device ) {
+	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+	void *devpath;
+	EFI_STATUS efirc;
+	int rc;
+
+	/* Re-open the device path protocol */
+	if ( ( efirc = bs->OpenProtocol ( efidev->device,
+					  &efi_device_path_protocol_guid,
+					  &devpath,
+					  efi_image_handle, device,
+					  EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
+					  ) ) != 0 ) {
+		rc = -EEFI ( efirc );
+		DBGC ( efidev->device, "EFIDRV %p %s could not add child",
+		       efidev->device, efi_devpath_text ( efidev->path ) );
+		DBGC ( efidev->device, " %p %s: %s\n", device,
+		       efi_handle_devpath_text ( device ), strerror ( rc ) );
+		return rc;
+	}
+
+	DBGC2 ( efidev->device, "EFIDRV %p %s added child",
+		efidev->device, efi_devpath_text ( efidev->path ) );
+	DBGC2 ( efidev->device, " %p %s\n",
+		device, efi_handle_devpath_text ( device ) );
+	return 0;
+}
+
+/**
+ * Remove EFI device as child of EFI device
+ *
+ * @v efidev		EFI device
+ * @v device		EFI child device handle
+ * @ret efirc		EFI status code
+ */
+void efidev_child_del ( struct efi_device *efidev, EFI_HANDLE device ) {
+	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+
+	bs->CloseProtocol ( efidev->device,
+			    &efi_device_path_protocol_guid,
+			    efi_image_handle, device );
+	DBGC2 ( efidev->device, "EFIDRV %p %s removed child",
+		efidev->device, efi_devpath_text ( efidev->path ) );
+	DBGC2 ( efidev->device, " %p %s\n",
+		device, efi_handle_devpath_text ( device ) );
+}
+
+/**
  * Check to see if driver supports a device
  *
  * @v driver		EFI driver
@@ -84,6 +186,13 @@ efi_driver_supported ( EFI_DRIVER_BINDING_PROTOCOL *driver __unused,
 		DBGCP ( device, " (child %s)", efi_devpath_text ( child ) );
 	DBGCP ( device, "\n" );
 
+	/* Do nothing if we are already driving this device */
+	if ( efidev_find ( device ) != NULL ) {
+		DBGCP ( device, "EFIDRV %p %s is already started\n",
+			device, efi_handle_devpath_text ( device ) );
+		return EFI_ALREADY_STARTED;
+	}
+
 	/* Look for a driver claiming to support this device */
 	for_each_table_entry ( efidrv, EFI_DRIVERS ) {
 		if ( ( rc = efidrv->supported ( device ) ) == 0 ) {
@@ -110,7 +219,14 @@ efi_driver_supported ( EFI_DRIVER_BINDING_PROTOCOL *driver __unused,
 static EFI_STATUS EFIAPI
 efi_driver_start ( EFI_DRIVER_BINDING_PROTOCOL *driver __unused,
 		   EFI_HANDLE device, EFI_DEVICE_PATH_PROTOCOL *child ) {
+	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
 	struct efi_driver *efidrv;
+	struct efi_device *efidev;
+	union {
+		EFI_DEVICE_PATH_PROTOCOL *devpath;
+		void *interface;
+	} devpath;
+	EFI_STATUS efirc;
 	int rc;
 
 	DBGC ( device, "EFIDRV %p %s DRIVER_START",
@@ -119,20 +235,61 @@ efi_driver_start ( EFI_DRIVER_BINDING_PROTOCOL *driver __unused,
 		DBGC ( device, " (child %s)", efi_devpath_text ( child ) );
 	DBGC ( device, "\n" );
 
+	/* Do nothing if we are already driving this device */
+	efidev = efidev_find ( device );
+	if ( efidev ) {
+		DBGCP ( device, "EFIDRV %p %s is already started\n",
+			device, efi_devpath_text ( efidev->path ) );
+		efirc = EFI_ALREADY_STARTED;
+		goto err_already_started;
+	}
+
+	/* Allocate and initialise structure */
+	efidev = zalloc ( sizeof ( *efidev ) );
+	if ( ! efidev ) {
+		efirc = EFI_OUT_OF_RESOURCES;
+		goto err_alloc;
+	}
+	efidev->device = device;
+	efidev->dev.desc.bus_type = BUS_TYPE_EFI;
+	INIT_LIST_HEAD ( &efidev->dev.children );
+	list_add ( &efidev->dev.siblings, &efi_devices );
+
+	/* Open device path protocol */
+	if ( ( efirc = bs->OpenProtocol ( device,
+					  &efi_device_path_protocol_guid,
+					  &devpath.interface,
+					  efi_image_handle, device,
+					  EFI_OPEN_PROTOCOL_BY_DRIVER ) ) != 0){
+		DBGCP ( device, "EFIDRV %p %s has no device path\n",
+			device, efi_handle_devpath_text ( device ) );
+		goto err_no_device_path;
+	}
+	efidev->path = devpath.devpath;
+
 	/* Try to start this device */
 	for_each_table_entry ( efidrv, EFI_DRIVERS ) {
-		if ( ( rc = efidrv->start ( device ) ) == 0 ) {
+		if ( ( rc = efidrv->start ( efidev ) ) == 0 ) {
+			efidev->driver = efidrv;
 			DBGC ( device, "EFIDRV %p %s using driver \"%s\"\n",
-			       device, efi_handle_devpath_text ( device ),
-			       efidrv->name );
+			       device, efi_devpath_text ( efidev->path ),
+			       efidev->driver->name );
 			return 0;
 		}
 		DBGC ( device, "EFIDRV %p %s could not start driver \"%s\": "
-		       "%s\n", device, efi_handle_devpath_text ( device ),
+		       "%s\n", device, efi_devpath_text ( efidev->path ),
 		       efidrv->name, strerror ( rc ) );
 	}
-
-	return EFI_UNSUPPORTED;
+	efirc = EFI_UNSUPPORTED;
+
+	bs->CloseProtocol ( device, &efi_device_path_protocol_guid,
+			    efi_image_handle, device );
+ err_no_device_path:
+	list_del ( &efidev->dev.siblings );
+	free ( efidev );
+ err_alloc:
+ err_already_started:
+	return efirc;
 }
 
 /**
@@ -149,7 +306,9 @@ static EFI_STATUS EFIAPI
 efi_driver_stop ( EFI_DRIVER_BINDING_PROTOCOL *driver __unused,
 		  EFI_HANDLE device, UINTN num_children,
 		  EFI_HANDLE *children ) {
+	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
 	struct efi_driver *efidrv;
+	struct efi_device *efidev;
 	UINTN i;
 
 	DBGC ( device, "EFIDRV %p %s DRIVER_STOP",
@@ -160,9 +319,22 @@ efi_driver_stop ( EFI_DRIVER_BINDING_PROTOCOL *driver __unused,
 	}
 	DBGC ( device, "\n" );
 
-	/* Try to stop this device */
-	for_each_table_entry ( efidrv, EFI_DRIVERS )
-		efidrv->stop ( device );
+	/* Do nothing unless we are driving this device */
+	efidev = efidev_find ( device );
+	if ( ! efidev ) {
+		DBGCP ( device, "EFIDRV %p %s is not started\n",
+			device, efi_devpath_text ( efidev->path ) );
+		return 0;
+	}
+
+	/* Stop this device */
+	efidrv = efidev->driver;
+	assert ( efidrv != NULL );
+	efidrv->stop ( efidev );
+	bs->CloseProtocol ( efidev->device, &efi_device_path_protocol_guid,
+			    efi_image_handle, efidev->device );
+	list_del ( &efidev->dev.siblings );
+	free ( efidev );
 
 	return 0;
 }
diff --git a/src/interface/efi/efi_pci.c b/src/interface/efi/efi_pci.c
index dd65b20f..7132263b 100644
--- a/src/interface/efi/efi_pci.c
+++ b/src/interface/efi/efi_pci.c
@@ -37,7 +37,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
 /* Disambiguate the various error causes */
 #define EINFO_EEFI_PCI							\
 	__einfo_uniqify ( EINFO_EPLATFORM, 0x01,			\
-			  "Could not open PCI I/O protocols" )
+			  "Could not open PCI I/O protocol" )
 #define EINFO_EEFI_PCI_NOT_PCI						\
 	__einfo_platformify ( EINFO_EEFI_PCI, EFI_UNSUPPORTED,		\
 			      "Not a PCI device" )
@@ -124,49 +124,28 @@ PROVIDE_PCIAPI_INLINE ( efi, pci_write_config_dword );
 static EFI_GUID efi_pci_io_protocol_guid
 	= EFI_PCI_IO_PROTOCOL_GUID;
 
-/** EFI device path protocol GUID */
-static EFI_GUID efi_device_path_protocol_guid
-	= EFI_DEVICE_PATH_PROTOCOL_GUID;
-
-/** EFI PCI devices */
-static LIST_HEAD ( efi_pci_devices );
-
 /**
- * Create EFI PCI device
+ * Open EFI PCI device
  *
- * @v device		EFI device
+ * @v device		EFI device handle
  * @v attributes	Protocol opening attributes
- * @v efipci		EFI PCI device to fill in
+ * @v pci		PCI device to fill in
  * @ret rc		Return status code
  */
-int efipci_create ( EFI_HANDLE device, UINT32 attributes,
-		    struct efi_pci_device **efipci ) {
+int efipci_open ( EFI_HANDLE device, UINT32 attributes,
+		  struct pci_device *pci ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
 	union {
 		EFI_PCI_IO_PROTOCOL *pci_io;
 		void *interface;
 	} pci_io;
-	union {
-		EFI_DEVICE_PATH_PROTOCOL *path;
-		void *interface;
-	} path;
 	UINTN pci_segment, pci_bus, pci_dev, pci_fn;
 	EFI_STATUS efirc;
 	int rc;
 
-	/* Allocate PCI device */
-	*efipci = zalloc ( sizeof ( **efipci ) );
-	if ( ! *efipci ) {
-		rc = -ENOMEM;
-		goto err_zalloc;
-	}
-	(*efipci)->device = device;
-
 	/* See if device is a PCI device */
-	if ( ( efirc = bs->OpenProtocol ( device,
-					  &efi_pci_io_protocol_guid,
-					  &pci_io.interface,
-					  efi_image_handle,
+	if ( ( efirc = bs->OpenProtocol ( device, &efi_pci_io_protocol_guid,
+					  &pci_io.interface, efi_image_handle,
 					  device, attributes ) ) != 0 ) {
 		rc = -EEFI_PCI ( efirc );
 		DBGCP ( device, "EFIPCI %p %s cannot open PCI protocols: %s\n",
@@ -174,16 +153,14 @@ int efipci_create ( EFI_HANDLE device, UINT32 attributes,
 			strerror ( rc ) );
 		goto err_open_protocol;
 	}
-	(*efipci)->pci_io = pci_io.pci_io;
 
 	/* Get PCI bus:dev.fn address */
-	if ( ( efirc = pci_io.pci_io->GetLocation ( pci_io.pci_io,
-						    &pci_segment,
+	if ( ( efirc = pci_io.pci_io->GetLocation ( pci_io.pci_io, &pci_segment,
 						    &pci_bus, &pci_dev,
 						    &pci_fn ) ) != 0 ) {
 		rc = -EEFI ( efirc );
-		DBGC ( device, "EFIPCI %p %s could not get PCI location: "
-		       "%s\n", device, efi_handle_devpath_text ( device ),
+		DBGC ( device, "EFIPCI %p %s could not get PCI location: %s\n",
+		       device, efi_handle_devpath_text ( device ),
 		       strerror ( rc ) );
 		goto err_get_location;
 	}
@@ -192,172 +169,72 @@ int efipci_create ( EFI_HANDLE device, UINT32 attributes,
 		( ( unsigned long ) pci_segment ), ( ( unsigned long ) pci_bus),
 		( ( unsigned long ) pci_dev ), ( ( unsigned long ) pci_fn ) );
 
+	/* Try to enable I/O cycles, memory cycles, and bus mastering.
+	 * Some platforms will 'helpfully' report errors if these bits
+	 * can't be enabled (for example, if the card doesn't actually
+	 * support I/O cycles).  Work around any such platforms by
+	 * enabling bits individually and simply ignoring any errors.
+	 */
+	pci_io.pci_io->Attributes ( pci_io.pci_io,
+				    EfiPciIoAttributeOperationEnable,
+				    EFI_PCI_IO_ATTRIBUTE_IO, NULL );
+	pci_io.pci_io->Attributes ( pci_io.pci_io,
+				    EfiPciIoAttributeOperationEnable,
+				    EFI_PCI_IO_ATTRIBUTE_MEMORY, NULL );
+	pci_io.pci_io->Attributes ( pci_io.pci_io,
+				    EfiPciIoAttributeOperationEnable,
+				    EFI_PCI_IO_ATTRIBUTE_BUS_MASTER, NULL );
+
 	/* Populate PCI device */
-	pci_init ( &(*efipci)->pci, PCI_BUSDEVFN ( pci_bus, pci_dev, pci_fn ) );
-	if ( ( rc = pci_read_config ( &(*efipci)->pci ) ) != 0 ) {
+	pci_init ( pci, PCI_BUSDEVFN ( pci_bus, pci_dev, pci_fn ) );
+	if ( ( rc = pci_read_config ( pci ) ) != 0 ) {
 		DBGC ( device, "EFIPCI %p %s cannot read PCI configuration: "
 		       "%s\n", device, efi_handle_devpath_text ( device ),
 		       strerror ( rc ) );
 		goto err_pci_read_config;
 	}
 
-	/* Retrieve device path */
-	if ( ( efirc = bs->OpenProtocol ( device,
-					  &efi_device_path_protocol_guid,
-					  &path.interface, efi_image_handle,
-					  device, attributes ) ) != 0 ) {
-		rc = -EEFI ( efirc );
-		DBGC ( device, "EFIPCI %p %s has no device path\n",
-		       device, efi_handle_devpath_text ( device ) );
-		goto err_no_device_path;
-	}
-	(*efipci)->path = path.path;
-
-	/* Add to list of PCI devices */
-	list_add ( &(*efipci)->list, &efi_pci_devices );
-
 	return 0;
 
-	bs->CloseProtocol ( device, &efi_device_path_protocol_guid,
-			    efi_image_handle, device );
- err_no_device_path:
  err_pci_read_config:
  err_get_location:
 	bs->CloseProtocol ( device, &efi_pci_io_protocol_guid,
 			    efi_image_handle, device );
  err_open_protocol:
-	free ( *efipci );
- err_zalloc:
 	return rc;
 }
 
 /**
- * Enable EFI PCI device
- *
- * @v efipci		EFI PCI device
- * @ret rc		Return status code
- */
-int efipci_enable ( struct efi_pci_device *efipci ) {
-	EFI_PCI_IO_PROTOCOL *pci_io = efipci->pci_io;
-
-	/* Try to enable I/O cycles, memory cycles, and bus mastering.
-	 * Some platforms will 'helpfully' report errors if these bits
-	 * can't be enabled (for example, if the card doesn't actually
-	 * support I/O cycles).  Work around any such platforms by
-	 * enabling bits individually and simply ignoring any errors.
-	 */
-	pci_io->Attributes ( pci_io, EfiPciIoAttributeOperationEnable,
-			     EFI_PCI_IO_ATTRIBUTE_IO, NULL );
-	pci_io->Attributes ( pci_io, EfiPciIoAttributeOperationEnable,
-			     EFI_PCI_IO_ATTRIBUTE_MEMORY, NULL );
-	pci_io->Attributes ( pci_io, EfiPciIoAttributeOperationEnable,
-			     EFI_PCI_IO_ATTRIBUTE_BUS_MASTER, NULL );
-
-	return 0;
-}
-
-/**
- * Find EFI PCI device by EFI device
- *
- * @v device		EFI device
- * @ret efipci		EFI PCI device, or NULL
- */
-struct efi_pci_device * efipci_find_efi ( EFI_HANDLE device ) {
-	struct efi_pci_device *efipci;
-
-	list_for_each_entry ( efipci, &efi_pci_devices, list ) {
-		if ( efipci->device == device )
-			return efipci;
-	}
-	return NULL;
-}
-
-/**
- * Find EFI PCI device by iPXE device
+ * Close EFI PCI device
  *
- * @v dev		Device
- * @ret efipci		EFI PCI device, or NULL
+ * @v device		EFI device handle
  */
-struct efi_pci_device * efipci_find ( struct device *dev ) {
-	struct efi_pci_device *efipci;
+void efipci_close ( EFI_HANDLE device ) {
+	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
 
-	list_for_each_entry ( efipci, &efi_pci_devices, list ) {
-		if ( &efipci->pci.dev == dev )
-			return efipci;
-	}
-	return NULL;
+	bs->CloseProtocol ( device, &efi_pci_io_protocol_guid,
+			    efi_image_handle, device );
 }
 
 /**
- * Add EFI device as child of EFI PCI device
+ * Get EFI PCI device information
  *
- * @v efipci		EFI PCI device
- * @v device		EFI child device
- * @ret efirc		EFI status code
+ * @v device		EFI device handle
+ * @v pci		PCI device to fill in
+ * @ret rc		Return status code
  */
-int efipci_child_add ( struct efi_pci_device *efipci, EFI_HANDLE device ) {
-	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
-	union {
-		EFI_PCI_IO_PROTOCOL *pci_io;
-		void *interface;
-	} pci_io;
-	EFI_STATUS efirc;
+int efipci_info ( EFI_HANDLE device, struct pci_device *pci ) {
 	int rc;
 
-	/* Re-open the PCI_IO_PROTOCOL */
-	if ( ( efirc = bs->OpenProtocol ( efipci->device,
-					  &efi_pci_io_protocol_guid,
-					  &pci_io.interface,
-					  efi_image_handle, device,
-					  EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
-					  ) ) != 0 ) {
-		rc = -EEFI ( efirc );
-		DBGC ( efipci->device, "EFIPCI %p %s could not add child",
-		       efipci->device, efi_devpath_text ( efipci->path ) );
-		DBGC ( efipci->device, " %p %s: %s\n", device,
-		       efi_handle_devpath_text ( device ), strerror ( rc ) );
+	/* Open PCI device, if possible */
+	if ( ( rc = efipci_open ( device, EFI_OPEN_PROTOCOL_GET_PROTOCOL,
+				  pci ) ) != 0 )
 		return rc;
-	}
-
-	DBGC2 ( efipci->device, "EFIPCI %p %s added child",
-		efipci->device, efi_devpath_text ( efipci->path ) );
-	DBGC2 ( efipci->device, " %p %s\n",
-		device, efi_handle_devpath_text ( device ) );
-	return 0;
-}
-
-/**
- * Remove EFI device as child of PCI device
- *
- * @v efipci		EFI PCI device
- * @v device		EFI child device
- * @ret efirc		EFI status code
- */
-void efipci_child_del ( struct efi_pci_device *efipci, EFI_HANDLE device ) {
-	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
-
-	bs->CloseProtocol ( efipci->device, &efi_pci_io_protocol_guid,
-			    efi_image_handle, device );
-	DBGC2 ( efipci->device, "EFIPCI %p %s removed child",
-		efipci->device, efi_devpath_text ( efipci->path ) );
-	DBGC2 ( efipci->device, " %p %s\n",
-		device, efi_handle_devpath_text ( device ) );
-}
 
-/**
- * Destroy EFI PCI device
- *
- * @v efipci		EFI PCI device
- */
-void efipci_destroy ( struct efi_pci_device *efipci ) {
-	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+	/* Close PCI device */
+	efipci_close ( device );
 
-	list_del ( &efipci->list );
-	bs->CloseProtocol ( efipci->device, &efi_device_path_protocol_guid,
-			    efi_image_handle, efipci->device );
-	bs->CloseProtocol ( efipci->device, &efi_pci_io_protocol_guid,
-			    efi_image_handle, efipci->device );
-	free ( efipci );
+	return 0;
 }
 
 /******************************************************************************
@@ -370,125 +247,105 @@ void efipci_destroy ( struct efi_pci_device *efipci ) {
 /**
  * Check to see if driver supports a device
  *
- * @v device		EFI device
+ * @v device		EFI device handle
  * @ret rc		Return status code
  */
 static int efipci_supported ( EFI_HANDLE device ) {
-	struct efi_pci_device *efipci;
+	struct pci_device pci;
 	int rc;
 
-	/* Do nothing if we are already driving this device */
-	efipci = efipci_find_efi ( device );
-	if ( efipci ) {
-		DBGCP ( device, "EFIPCI %p %s already started\n",
-			device, efi_devpath_text ( efipci->path ) );
-		rc = -EALREADY;
-		goto err_already_started;
-	}
-
-	/* Create temporary corresponding PCI device, if any */
-	if ( ( rc = efipci_create ( device, EFI_OPEN_PROTOCOL_GET_PROTOCOL,
-				    &efipci ) ) != 0 )
-		goto err_create;
+	/* Get PCI device information */
+	if ( ( rc = efipci_info ( device, &pci ) ) != 0 )
+		return rc;
 
 	/* Look for a driver */
-	if ( ( rc = pci_find_driver ( &efipci->pci ) ) != 0 ) {
+	if ( ( rc = pci_find_driver ( &pci ) ) != 0 ) {
 		DBGCP ( device, "EFIPCI %p %s has no driver\n",
-			device, efi_devpath_text ( efipci->path ) );
-		goto err_no_driver;
+			device, efi_handle_devpath_text ( device ) );
+		return rc;
 	}
-
-	DBGC ( device, "EFIPCI %p %s has driver \"%s\"\n", device,
-	       efi_devpath_text ( efipci->path ), efipci->pci.id->name );
-
-	/* Destroy temporary PCI device */
-	efipci_destroy ( efipci );
+	DBGC ( device, "EFIPCI %p %s has driver \"%s\"\n",
+	       device, efi_handle_devpath_text ( device ), pci.id->name );
 
 	return 0;
-
- err_no_driver:
-	efipci_destroy ( efipci );
- err_create:
- err_already_started:
-	return rc;
 }
 
 /**
  * Attach driver to device
  *
- * @v device		EFI device
+ * @v efidev		EFI device
  * @ret rc		Return status code
  */
-static int efipci_start ( EFI_HANDLE device ) {
-	struct efi_pci_device *efipci;
+static int efipci_start ( struct efi_device *efidev ) {
+	EFI_HANDLE device = efidev->device;
+	struct pci_device *pci;
 	int rc;
 
-	/* Do nothing if we are already driving this device */
-	efipci = efipci_find_efi ( device );
-	if ( efipci ) {
-		DBGCP ( device, "EFIPCI %p %s already started\n",
-			device, efi_devpath_text ( efipci->path ) );
-		rc = -EALREADY;
-		goto err_already_started;
+	/* Allocate PCI device */
+	pci = zalloc ( sizeof ( *pci ) );
+	if ( ! pci ) {
+		rc = -ENOMEM;
+		goto err_alloc;
 	}
 
-	/* Create corresponding PCI device */
-	if ( ( rc = efipci_create ( device, ( EFI_OPEN_PROTOCOL_BY_DRIVER |
-					      EFI_OPEN_PROTOCOL_EXCLUSIVE ),
-				    &efipci ) ) != 0 )
-		goto err_create;
+	/* Open PCI device */
+	if ( ( rc = efipci_open ( device, ( EFI_OPEN_PROTOCOL_BY_DRIVER |
+					    EFI_OPEN_PROTOCOL_EXCLUSIVE ),
+				  pci ) ) != 0 ) {
+		DBGC ( device, "EFIPCI %p %s could not open PCI device: %s\n",
+		       device, efi_devpath_text ( efidev->path ),
+		       strerror ( rc ) );
+		goto err_open;
+	}
 
 	/* Find driver */
-	if ( ( rc = pci_find_driver ( &efipci->pci ) ) != 0 ) {
+	if ( ( rc = pci_find_driver ( pci ) ) != 0 ) {
 		DBGC ( device, "EFIPCI %p %s has no driver\n",
-		       device, efi_devpath_text ( efipci->path ) );
+		       device, efi_devpath_text ( efidev->path ) );
 		goto err_find_driver;
 	}
 
-	/* Enable PCI device */
-	if ( ( rc = efipci_enable ( efipci ) ) != 0 )
-		goto err_enable;
+	/* Mark PCI device as a child of the EFI device */
+	pci->dev.parent = &efidev->dev;
+	list_add ( &pci->dev.siblings, &efidev->dev.children );
 
 	/* Probe driver */
-	if ( ( rc = pci_probe ( &efipci->pci ) ) != 0 ) {
+	if ( ( rc = pci_probe ( pci ) ) != 0 ) {
 		DBGC ( device, "EFIPCI %p %s could not probe driver \"%s\": "
-		       "%s\n", device, efi_devpath_text ( efipci->path ),
-		       efipci->pci.id->name, strerror ( rc ) );
+		       "%s\n", device, efi_devpath_text ( efidev->path ),
+		       pci->id->name, strerror ( rc ) );
 		goto err_probe;
 	}
 	DBGC ( device, "EFIPCI %p %s using driver \"%s\"\n", device,
-	       efi_devpath_text ( efipci->path ), efipci->pci.id->name );
+	       efi_devpath_text ( efidev->path ), pci->id->name );
 
+	efidev_set_drvdata ( efidev, pci );
 	return 0;
 
-	pci_remove ( &efipci->pci );
+	pci_remove ( pci );
  err_probe:
- err_enable:
+	list_del ( &pci->dev.siblings );
  err_find_driver:
-	efipci_destroy ( efipci );
- err_create:
- err_already_started:
+	efipci_close ( device );
+ err_open:
+	free ( pci );
+ err_alloc:
 	return rc;
 }
 
 /**
  * Detach driver from device
  *
- * @v device		EFI device
+ * @v efidev		EFI device
   */
-static void efipci_stop ( EFI_HANDLE device ) {
-	struct efi_pci_device *efipci;
-
-	/* Find PCI device */
-	efipci = efipci_find_efi ( device );
-	if ( ! efipci )
-		return;
-
-	/* Remove device */
-	pci_remove ( &efipci->pci );
-
-	/* Delete EFI PCI device */
-	efipci_destroy ( efipci );
+static void efipci_stop ( struct efi_device *efidev ) {
+	struct pci_device *pci = efidev_get_drvdata ( efidev );
+	EFI_HANDLE device = efidev->device;
+
+	pci_remove ( pci );
+	list_del ( &pci->dev.siblings );
+	efipci_close ( device );
+	free ( pci );
 }
 
 /** EFI PCI driver */
diff --git a/src/interface/efi/efi_snp.c b/src/interface/efi/efi_snp.c
index 9e7b8dcd..fd3aad4c 100644
--- a/src/interface/efi/efi_snp.c
+++ b/src/interface/efi/efi_snp.c
@@ -27,10 +27,8 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #include <ipxe/netdevice.h>
 #include <ipxe/iobuf.h>
 #include <ipxe/in.h>
-#include <ipxe/pci.h>
 #include <ipxe/version.h>
 #include <ipxe/efi/efi.h>
-#include <ipxe/efi/efi_pci.h>
 #include <ipxe/efi/efi_driver.h>
 #include <ipxe/efi/efi_strings.h>
 #include <ipxe/efi/efi_snp.h>
@@ -348,7 +346,7 @@ efi_snp_station_address ( EFI_SIMPLE_NETWORK_PROTOCOL *snp, BOOLEAN reset,
 	/* MAC address changes take effect only on netdev_open() */
 	if ( netdev_is_open ( snpdev->netdev ) ) {
 		DBGC ( snpdev, "SNPDEV %p MAC address changed while net "
-		       "devive open\n", snpdev );
+		       "device open\n", snpdev );
 	}
 
 	return 0;
@@ -922,7 +920,7 @@ static struct efi_snp_device * efi_snp_demux ( struct net_device *netdev ) {
  */
 static int efi_snp_probe ( struct net_device *netdev ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
-	struct efi_pci_device *efipci;
+	struct efi_device *efidev;
 	struct efi_snp_device *snpdev;
 	EFI_DEVICE_PATH_PROTOCOL *path_end;
 	MAC_ADDR_DEVICE_PATH *macpath;
@@ -930,18 +928,18 @@ static int efi_snp_probe ( struct net_device *netdev ) {
 	EFI_STATUS efirc;
 	int rc;
 
-	/* Find EFI PCI device */
-	efipci = efipci_find ( netdev->dev );
-	if ( ! efipci ) {
-		DBG ( "SNP skipping non-PCI device %s\n", netdev->name );
+	/* Find parent EFI device */
+	efidev = efidev_parent ( netdev->dev );
+	if ( ! efidev ) {
+		DBG ( "SNP skipping non-EFI device %s\n", netdev->name );
 		rc = 0;
-		goto err_no_pci;
+		goto err_no_efidev;
 	}
 
 	/* Calculate device path prefix length */
-	path_end = efi_devpath_end ( efipci->path );
+	path_end = efi_devpath_end ( efidev->path );
 	path_prefix_len = ( ( ( void * ) path_end ) -
-			    ( ( void * ) efipci->path ) );
+			    ( ( void * ) efidev->path ) );
 
 	/* Allocate the SNP device */
 	snpdev = zalloc ( sizeof ( *snpdev ) + path_prefix_len +
@@ -951,7 +949,7 @@ static int efi_snp_probe ( struct net_device *netdev ) {
 		goto err_alloc_snp;
 	}
 	snpdev->netdev = netdev_get ( netdev );
-	snpdev->efipci = efipci;
+	snpdev->efidev = efidev;
 
 	/* Sanity check */
 	if ( netdev->ll_protocol->ll_addr_len > sizeof ( EFI_MAC_ADDRESS ) ) {
@@ -1009,7 +1007,7 @@ static int efi_snp_probe ( struct net_device *netdev ) {
 		       "%s", netdev->name );
 
 	/* Populate the device path */
-	memcpy ( &snpdev->path, efipci->path, path_prefix_len );
+	memcpy ( &snpdev->path, efidev->path, path_prefix_len );
 	macpath = ( ( ( void * ) &snpdev->path ) + path_prefix_len );
 	path_end = ( ( void * ) ( macpath + 1 ) );
 	memset ( macpath, 0, sizeof ( *macpath ) );
@@ -1040,12 +1038,12 @@ static int efi_snp_probe ( struct net_device *netdev ) {
 		goto err_install_protocol_interface;
 	}
 
-	/* Add as child of PCI device */
-	if ( ( rc = efipci_child_add ( efipci, snpdev->handle ) ) != 0 ) {
-		DBGC ( snpdev, "SNPDEV %p could not become child of " PCI_FMT
-		       ": %s\n", snpdev, PCI_ARGS ( &efipci->pci ),
-		       strerror ( rc ) );
-		goto err_efipci_child_add;
+	/* Add as child of EFI parent device */
+	if ( ( rc = efidev_child_add ( efidev, snpdev->handle ) ) != 0 ) {
+		DBGC ( snpdev, "SNPDEV %p could not become child of %p %s: "
+		       "%s\n", snpdev, efidev->device,
+		       efi_devpath_text ( efidev->path ), strerror ( rc ) );
+		goto err_efidev_child_add;
 	}
 
 	/* Install HII */
@@ -1058,14 +1056,15 @@ static int efi_snp_probe ( struct net_device *netdev ) {
 	/* Add to list of SNP devices */
 	list_add ( &snpdev->list, &efi_snp_devices );
 
-	DBGC ( snpdev, "SNPDEV %p installed for %s as device %p\n",
-	       snpdev, netdev->name, snpdev->handle );
+	DBGC ( snpdev, "SNPDEV %p installed for %s as device %p %s\n",
+	       snpdev, netdev->name, snpdev->handle,
+	       efi_devpath_text ( &snpdev->path ) );
 	return 0;
 
 	efi_snp_hii_uninstall ( snpdev );
  err_hii_install:
-	efipci_child_del ( efipci, snpdev->handle );
- err_efipci_child_add:
+	efidev_child_del ( efidev, snpdev->handle );
+ err_efidev_child_add:
 	bs->UninstallMultipleProtocolInterfaces (
 			snpdev->handle,
 			&efi_simple_network_protocol_guid, &snpdev->snp,
@@ -1082,7 +1081,7 @@ static int efi_snp_probe ( struct net_device *netdev ) {
 	netdev_put ( netdev );
 	free ( snpdev );
  err_alloc_snp:
- err_no_pci:
+ err_no_efidev:
 	return rc;
 }
 
@@ -1129,7 +1128,7 @@ static void efi_snp_remove ( struct net_device *netdev ) {
 
 	/* Uninstall the SNP */
 	efi_snp_hii_uninstall ( snpdev );
-	efipci_child_del ( snpdev->efipci, snpdev->handle );
+	efidev_child_del ( snpdev->efidev, snpdev->handle );
 	list_del ( &snpdev->list );
 	bs->UninstallMultipleProtocolInterfaces (
 			snpdev->handle,
@@ -1154,6 +1153,22 @@ struct net_driver efi_snp_driver __net_driver = {
 };
 
 /**
+ * Find SNP device by EFI device handle
+ *
+ * @v handle		EFI device handle
+ * @ret snpdev		SNP device, or NULL
+ */
+struct efi_snp_device * find_snpdev ( EFI_HANDLE handle ) {
+	struct efi_snp_device *snpdev;
+
+	list_for_each_entry ( snpdev, &efi_snp_devices, list ) {
+		if ( snpdev->handle == handle )
+			return snpdev;
+	}
+	return NULL;
+}
+
+/**
  * Get most recently opened SNP device
  *
  * @ret snpdev		Most recently opened SNP device, or NULL