[efi] Disconnect existing drivers on a per-protocol basis

UEFI does not provide a direct method to disconnect the existing
driver of a specific protocol from a handle.  We currently use
DisconnectController() to remove all drivers from a handle that we
want to drive ourselves, and then rely on recursion in the call to
ConnectController() to reconnect any drivers that did not need to be
disconnected in the first place.

Experience shows that OEMs tend not to ever test the disconnection
code paths in their UEFI drivers, and it is common to find drivers
that refuse to disconnect, fail to close opened handles, fail to
function correctly after reconnection, or lock up the entire system.

Implement a more selective form of disconnection, in which we use
OpenProtocolInformation() to identify the driver associated with a
specific protocol, and then disconnect only that driver.

Perform disconnections in reverse order of attachment priority, since
this is the order likely to minimise the number of cascaded implicit
disconnections.

This allows our MNP driver to avoid performing any disconnections at
all, since it does not require exclusive access to the MNP protocol.
It also avoids performing unnecessary disconnections and reconnections
of unrelated drivers such as the "UEFI WiFi Connection Manager" that
attaches to wireless network interfaces in order to manage wireless
network associations.

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

6 files changed, 86 insertions, 7 deletions

diff --git a/src/drivers/net/efi/snp.c b/src/drivers/net/efi/snp.c
index 58b5ad546..7c4123677 100644
--- a/src/drivers/net/efi/snp.c
+++ b/src/drivers/net/efi/snp.c
@@ -59,6 +59,7 @@ static int nii_supported ( EFI_HANDLE device ) {
 /** EFI SNP driver */
 struct efi_driver snp_driver __efi_driver ( EFI_DRIVER_SNP ) = {
 	.name = "SNP",
+	.exclude = &efi_simple_network_protocol_guid,
 	.supported = snp_supported,
 	.start = snpnet_start,
 	.stop = snpnet_stop,
@@ -67,6 +68,7 @@ struct efi_driver snp_driver __efi_driver ( EFI_DRIVER_SNP ) = {
 /** EFI NII driver */
 struct efi_driver nii_driver __efi_driver ( EFI_DRIVER_NII ) = {
 	.name = "NII",
+	.exclude = &efi_nii31_protocol_guid,
 	.supported = nii_supported,
 	.start = nii_start,
 	.stop = nii_stop,
diff --git a/src/drivers/net/efi/snponly.c b/src/drivers/net/efi/snponly.c
index e40451885..267572e34 100644
--- a/src/drivers/net/efi/snponly.c
+++ b/src/drivers/net/efi/snponly.c
@@ -209,6 +209,7 @@ static int mnponly_supported ( EFI_HANDLE device ) {
 /** EFI SNP chainloading-device-only driver */
 struct efi_driver snponly_driver __efi_driver ( EFI_DRIVER_SNP ) = {
 	.name = "SNPONLY",
+	.exclude = &efi_simple_network_protocol_guid,
 	.supported = snponly_supported,
 	.start = snpnet_start,
 	.stop = snpnet_stop,
@@ -217,6 +218,7 @@ struct efi_driver snponly_driver __efi_driver ( EFI_DRIVER_SNP ) = {
 /** EFI NII chainloading-device-only driver */
 struct efi_driver niionly_driver __efi_driver ( EFI_DRIVER_NII ) = {
 	.name = "NIIONLY",
+	.exclude = &efi_nii31_protocol_guid,
 	.supported = niionly_supported,
 	.start = nii_start,
 	.stop = nii_stop,
diff --git a/src/drivers/usb/usbio.c b/src/drivers/usb/usbio.c
index e4dca7e87..991b290ad 100644
--- a/src/drivers/usb/usbio.c
+++ b/src/drivers/usb/usbio.c
@@ -1651,6 +1651,7 @@ static void usbio_stop ( struct efi_device *efidev ) {
 /** EFI USB I/O driver */
 struct efi_driver usbio_driver __efi_driver ( EFI_DRIVER_HARDWARE ) = {
 	.name = "USBIO",
+	.exclude = &efi_usb_io_protocol_guid,
 	.supported = usbio_supported,
 	.start = usbio_start,
 	.stop = usbio_stop,
diff --git a/src/include/ipxe/efi/efi_driver.h b/src/include/ipxe/efi/efi_driver.h
index e07bfd49d..4c2148919 100644
--- a/src/include/ipxe/efi/efi_driver.h
+++ b/src/include/ipxe/efi/efi_driver.h
@@ -33,6 +33,8 @@ struct efi_device {
 struct efi_driver {
 	/** Name */
 	const char *name;
+	/** Protocol to which exclusive access is required, if any */
+	EFI_GUID *exclude;
 	/**
 	 * Check if driver supports device
 	 *
diff --git a/src/interface/efi/efi_driver.c b/src/interface/efi/efi_driver.c
index 9f2f08846..8c5e00bfb 100644
--- a/src/interface/efi/efi_driver.c
+++ b/src/interface/efi/efi_driver.c
@@ -442,6 +442,68 @@ void efi_driver_uninstall ( void ) {
 }
 
 /**
+ * Try to disconnect an existing EFI driver
+ *
+ * @v device		EFI device
+ * @v protocol		Protocol GUID
+ * @ret rc		Return status code
+ */
+static int efi_driver_exclude ( EFI_HANDLE device, EFI_GUID *protocol ) {
+	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+	EFI_OPEN_PROTOCOL_INFORMATION_ENTRY *openers;
+	EFI_OPEN_PROTOCOL_INFORMATION_ENTRY *opener;
+	EFI_HANDLE driver;
+	UINTN count;
+	unsigned int i;
+	EFI_STATUS efirc;
+	int rc;
+
+	/* Retrieve list of openers */
+	if ( ( efirc = bs->OpenProtocolInformation ( device, protocol, &openers,
+						     &count ) ) != 0 ) {
+		rc = -EEFI ( efirc );
+		DBGC ( device, "EFIDRV %s could not list %s openers: %s\n",
+		       efi_handle_name ( device ), efi_guid_ntoa ( protocol ),
+		       strerror ( rc ) );
+		goto err_list;
+	}
+
+	/* Identify BY_DRIVER opener */
+	driver = NULL;
+	for ( i = 0 ; i < count ; i++ ) {
+		opener = &openers[i];
+		if ( opener->Attributes & EFI_OPEN_PROTOCOL_BY_DRIVER ) {
+			driver = opener->AgentHandle;
+			break;
+		}
+	}
+
+	/* Try to disconnect driver */
+	if ( driver ) {
+		DBGC ( device, "EFIDRV %s disconnecting %s driver ",
+		       efi_handle_name ( device ), efi_guid_ntoa ( protocol ) );
+		DBGC ( device, "%s\n", efi_handle_name ( driver ) );
+		if ( ( efirc = bs->DisconnectController ( device, driver,
+							  NULL ) ) != 0 ) {
+			rc = -EEFI ( efirc );
+			DBGC ( device, "EFIDRV %s could not disconnect ",
+			       efi_handle_name ( device ) );
+			DBGC ( device, "%s: %s\n",
+			       efi_handle_name ( driver ), strerror ( rc ) );
+			goto err_disconnect;
+		}
+	}
+
+	/* Success */
+	rc = 0;
+
+ err_disconnect:
+	bs->FreePool ( openers );
+ err_list:
+	return rc;
+}
+
+/**
  * Try to connect EFI driver
  *
  * @v device		EFI device
@@ -451,6 +513,8 @@ static int efi_driver_connect ( EFI_HANDLE device ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
 	EFI_HANDLE drivers[2] =
 		{ efi_driver_binding.DriverBindingHandle, NULL };
+	struct efi_driver *efidrv;
+	EFI_GUID *exclude;
 	EFI_STATUS efirc;
 	int rc;
 
@@ -468,13 +532,20 @@ static int efi_driver_connect ( EFI_HANDLE device ) {
 	DBGC ( device, "EFIDRV %s disconnecting existing drivers\n",
 	       efi_handle_name ( device ) );
 	efi_driver_disconnecting = 1;
-	if ( ( efirc = bs->DisconnectController ( device, NULL,
-						  NULL ) ) != 0 ) {
-		rc = -EEFI ( efirc );
-		DBGC ( device, "EFIDRV %s could not disconnect existing "
-		       "drivers: %s\n", efi_handle_name ( device ),
-		       strerror ( rc ) );
-		/* Ignore the error and attempt to connect our drivers */
+	for_each_table_entry_reverse ( efidrv, EFI_DRIVERS ) {
+		exclude = efidrv->exclude;
+		if ( ! exclude )
+			continue;
+		if ( ( rc = efidrv->supported ( device ) ) != 0 )
+			continue;
+		DBGC ( device, "EFIDRV %s disconnecting %s drivers\n",
+		       efi_handle_name ( device ), efi_guid_ntoa ( exclude ) );
+		if ( ( rc = efi_driver_exclude ( device, exclude ) ) != 0 ) {
+			DBGC ( device, "EFIDRV %s could not disconnect %s "
+			       "drivers: %s\n", efi_handle_name ( device ),
+			       efi_guid_ntoa ( exclude ), strerror ( rc ) );
+			/* Ignore the error and attempt to connect anyway */
+		}
 	}
 	efi_driver_disconnecting = 0;
 	DBGC2 ( device, "EFIDRV %s after disconnecting:\n",
diff --git a/src/interface/efi/efi_pci.c b/src/interface/efi/efi_pci.c
index 6bdc2d575..f98794c27 100644
--- a/src/interface/efi/efi_pci.c
+++ b/src/interface/efi/efi_pci.c
@@ -916,6 +916,7 @@ static void efipci_stop ( struct efi_device *efidev ) {
 /** EFI PCI driver */
 struct efi_driver efipci_driver __efi_driver ( EFI_DRIVER_HARDWARE ) = {
 	.name = "PCI",
+	.exclude = &efi_pci_io_protocol_guid,
 	.supported = efipci_supported,
 	.start = efipci_start,
 	.stop = efipci_stop,