[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>

diff --git a/src/drivers/net/efi/snp.c b/src/drivers/net/efi/snp.c
index 58b5ad546195445a87eb03b3608b321a81436b27..7c4123677a4b7b1ab04c3edacc221acac36f6254 100644 (file)
--- 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 e40451885ecd84a068827fc9e023c917d3d165d7..267572e34a55a489ca91a4480afa3d8388ad0b8f 100644 (file)
--- 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 e4dca7e8718bc1acb9d9a1e35b790e8ef926b61f..991b290adcffb34d7fd78a843711dd27cbdc3d76 100644 (file)
--- 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 e07bfd49d6527cdbe2f1d4b16e8ca5cd59fe831f..4c21489199548ba5d43efb04797210dfe456a26e 100644 (file)
--- 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 9f2f08846b378415b5b188bcc02297bce8320f49..8c5e00bfbff7fb23ba87a194e176022c595de37e 100644 (file)
--- a/src/interface/efi/efi_driver.c
+++ b/src/interface/efi/efi_driver.c
@@ -441,6 +441,68 @@ void efi_driver_uninstall ( void ) {
                &efi_component_name2_protocol_guid, &efi_wtf, NULL );
 }
 
+/**
+ * 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
  *
@@ -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 6bdc2d57506c52a7113e21517bacbe71a070e3c2..f98794c2715323e8f537305f65527933f0184ea8 100644 (file)
--- 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,