]> git.ipfire.org Git - thirdparty/ipxe.git/commitdiff
[efi] Drop to external TPL for calls to ConnectController()
authorMichael Brown <mcb30@ipxe.org>
Mon, 14 Jul 2025 11:17:11 +0000 (12:17 +0100)
committerMichael Brown <mcb30@ipxe.org>
Mon, 14 Jul 2025 11:19:15 +0000 (12:19 +0100)
There is nothing in the current versions of the UEFI specification
that limits the TPL at which we may call ConnectController() or
DisconnectController().  However, at least some platforms (observed
with a Lenovo ThinkPad T14s Gen 5) will occasionally and unpredictably
lock up before returning from ConnectController() if called at a TPL
higher than TPL_APPLICATION.

Work around whatever defect is present on these systems by dropping to
the current external TPL for all calls to ConnectController() or
DisconnectController().

Signed-off-by: Michael Brown <mcb30@ipxe.org>
src/include/ipxe/efi/efi.h
src/include/ipxe/errfile.h
src/interface/efi/efi_block.c
src/interface/efi/efi_connect.c [new file with mode: 0644]
src/interface/efi/efi_driver.c
src/interface/efi/efi_usb.c
src/interface/efi/efi_veto.c

index 03081f6f40ce2e4af342b5f94a8349e88d34a988..33278abed979e00c39988cdbfc1c2ece5ad0edc7 100644 (file)
@@ -413,6 +413,8 @@ extern int efi_open_by_child_untyped ( EFI_HANDLE handle, EFI_GUID *protocol,
                                       EFI_HANDLE child, void **interface );
 extern void efi_close_by_child ( EFI_HANDLE handle, EFI_GUID *protocol,
                                 EFI_HANDLE child );
+extern int efi_connect ( EFI_HANDLE device, EFI_HANDLE driver );
+extern int efi_disconnect ( EFI_HANDLE device, EFI_HANDLE driver );
 
 /**
  * Test protocol existence
index eee71bfaa6e50796e9aaea097e480fb2b955436c..0e3634f9e6da22b3a1aa53e8fbd4e722352c0294 100644 (file)
@@ -86,6 +86,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #define ERRFILE_null_smbios           ( ERRFILE_CORE | 0x002e0000 )
 #define ERRFILE_efi_open              ( ERRFILE_CORE | 0x002f0000 )
 #define ERRFILE_efi_table             ( ERRFILE_CORE | 0x00300000 )
+#define ERRFILE_efi_connect           ( ERRFILE_CORE | 0x00310000 )
 
 #define ERRFILE_eisa                ( ERRFILE_DRIVER | 0x00000000 )
 #define ERRFILE_isa                 ( ERRFILE_DRIVER | 0x00010000 )
index aa5ec4e0fa0398b83f50bd113c97982a76ef6d40..cc93edb85c28d5183e3900cdcc5cb1ebdbd7048a 100644 (file)
@@ -218,14 +218,10 @@ efi_block_io_flush ( EFI_BLOCK_IO_PROTOCOL *block_io ) {
  * @v handle           Block device handle
  */
 static void efi_block_connect ( unsigned int drive, EFI_HANDLE handle ) {
-       EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
-       EFI_STATUS efirc;
        int rc;
 
        /* Try to connect all possible drivers to this block device */
-       if ( ( efirc = bs->ConnectController ( handle, NULL, NULL,
-                                              TRUE ) ) != 0 ) {
-               rc = -EEFI ( efirc );
+       if ( ( rc = efi_connect ( handle, NULL ) ) != 0 ) {
                DBGC ( drive, "EFIBLK %#02x could not connect drivers: %s\n",
                       drive, strerror ( rc ) );
                /* May not be an error; may already be connected */
diff --git a/src/interface/efi/efi_connect.c b/src/interface/efi/efi_connect.c
new file mode 100644 (file)
index 0000000..5aa8dc6
--- /dev/null
@@ -0,0 +1,110 @@
+/*
+ * Copyright (C) 2025 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.
+ *
+ * You can also choose to distribute this program under the terms of
+ * the Unmodified Binary Distribution Licence (as given in the file
+ * COPYING.UBDL), provided that you have satisfied its requirements.
+ */
+
+FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
+
+/** @file
+ *
+ * EFI driver connection and disconnection
+ *
+ */
+
+#include <errno.h>
+#include <string.h>
+#include <ipxe/efi/efi.h>
+
+/* Disambiguate the various error causes */
+#define EINFO_EEFI_CONNECT                                             \
+       __einfo_uniqify ( EINFO_EPLATFORM, 0x01,                        \
+                         "Could not connect controllers" )
+#define EINFO_EEFI_CONNECT_PROHIBITED                                  \
+       __einfo_platformify ( EINFO_EEFI_CONNECT,                       \
+                             EFI_SECURITY_VIOLATION,                   \
+                             "Connecting controllers prohibited by "   \
+                             "security policy" )
+#define EEFI_CONNECT_PROHIBITED                                                \
+       __einfo_error ( EINFO_EEFI_CONNECT_PROHIBITED )
+#define EEFI_CONNECT( efirc ) EPLATFORM ( EINFO_EEFI_CONNECT, efirc,   \
+                                         EEFI_CONNECT_PROHIBITED )
+
+/**
+ * Connect UEFI driver(s)
+ *
+ * @v device           EFI device handle
+ * @v driver           EFI driver handle, or NULL
+ * @ret rc             Return status code
+ */
+int efi_connect ( EFI_HANDLE device, EFI_HANDLE driver ) {
+       EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+       EFI_HANDLE driverlist[2] = { driver, NULL };
+       EFI_HANDLE *drivers = ( driver ? driverlist : NULL );
+       EFI_STATUS efirc;
+       int rc;
+
+       /* Attempt connection at external TPL */
+       DBGC ( device, "EFI %s connecting ", efi_handle_name ( device ) );
+       DBGC ( device, "%s driver at %s TPL\n",
+              ( driver ? efi_handle_name ( driver ) : "any" ),
+              efi_tpl_name ( efi_external_tpl ) );
+       bs->RestoreTPL ( efi_external_tpl );
+       efirc = bs->ConnectController ( device, drivers, NULL, TRUE );
+       bs->RaiseTPL ( efi_internal_tpl );
+       if ( efirc != 0 ) {
+               rc = -EEFI_CONNECT ( efirc );
+               DBGC ( device, "EFI %s could not connect: %s\n",
+                      efi_handle_name ( device ), strerror ( rc ) );
+               return rc;
+       }
+
+       return 0;
+}
+
+/**
+ * Disconnect UEFI driver(s)
+ *
+ * @v device           EFI device handle
+ * @v driver           EFI driver handle, or NULL
+ * @ret rc             Return status code
+ */
+int efi_disconnect ( EFI_HANDLE device, EFI_HANDLE driver ) {
+       EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+       EFI_STATUS efirc;
+       int rc;
+
+       /* Attempt disconnection at external TPL */
+       DBGC ( device, "EFI %s disconnecting ", efi_handle_name ( device ) );
+       DBGC ( device, "%s driver at %s TPL\n",
+              ( driver ? efi_handle_name ( driver ) : "any" ),
+              efi_tpl_name ( efi_external_tpl ) );
+       bs->RestoreTPL ( efi_external_tpl );
+       efirc = bs->DisconnectController ( device, driver, NULL );
+       bs->RaiseTPL ( efi_internal_tpl );
+       if ( ( efirc != 0 ) && ( efirc != EFI_NOT_FOUND ) ) {
+               rc = -EEFI ( efirc );
+               DBGC ( device, "EFI %s could not disconnect: %s\n",
+                      efi_handle_name ( device ), strerror ( rc ) );
+               return rc;
+       }
+
+       return 0;
+}
index ce1db228dcd9404afb6adec699ef51841df99e2d..b1ff404beb5e663f51593c7a637a60eedda354b6 100644 (file)
@@ -39,20 +39,6 @@ FILE_LICENCE ( GPL2_OR_LATER );
  *
  */
 
-/* Disambiguate the various error causes */
-#define EINFO_EEFI_CONNECT                                             \
-       __einfo_uniqify ( EINFO_EPLATFORM, 0x01,                        \
-                         "Could not connect controllers" )
-#define EINFO_EEFI_CONNECT_PROHIBITED                                  \
-       __einfo_platformify ( EINFO_EEFI_CONNECT,                       \
-                             EFI_SECURITY_VIOLATION,                   \
-                             "Connecting controllers prohibited by "   \
-                             "security policy" )
-#define EEFI_CONNECT_PROHIBITED                                                \
-       __einfo_error ( EINFO_EEFI_CONNECT_PROHIBITED )
-#define EEFI_CONNECT( efirc ) EPLATFORM ( EINFO_EEFI_CONNECT, efirc,   \
-                                         EEFI_CONNECT_PROHIBITED )
-
 static EFI_DRIVER_BINDING_PROTOCOL efi_driver_binding;
 
 /** List of controlled EFI devices */
@@ -485,9 +471,7 @@ int efi_driver_exclude ( EFI_HANDLE device, EFI_GUID *protocol ) {
                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 );
+               if ( ( rc = efi_disconnect ( device, driver ) ) != 0 ) {
                        DBGC ( device, "EFIDRV %s could not disconnect ",
                               efi_handle_name ( device ) );
                        DBGC ( device, "%s: %s\n",
@@ -512,9 +496,7 @@ int efi_driver_exclude ( EFI_HANDLE device, EFI_GUID *protocol ) {
  * @ret rc             Return status code
  */
 static int efi_driver_connect ( EFI_HANDLE device ) {
-       EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
-       EFI_HANDLE drivers[2] =
-               { efi_driver_binding.DriverBindingHandle, NULL };
+       EFI_HANDLE driver = efi_driver_binding.DriverBindingHandle;
        struct efi_driver *efidrv;
        EFI_STATUS efirc;
        int rc;
@@ -553,16 +535,14 @@ static int efi_driver_connect ( EFI_HANDLE device ) {
        /* Connect our driver */
        DBGC ( device, "EFIDRV %s connecting new drivers\n",
               efi_handle_name ( device ) );
-       if ( ( efirc = bs->ConnectController ( device, drivers, NULL,
-                                              TRUE ) ) != 0 ) {
-               rc = -EEFI_CONNECT ( efirc );
+       if ( ( rc = efi_connect ( device, driver ) ) != 0 ) {
                DBGC ( device, "EFIDRV %s could not connect new drivers: "
                       "%s\n", efi_handle_name ( device ), strerror ( rc ) );
                DBGC ( device, "EFIDRV %s connecting driver directly\n",
                       efi_handle_name ( device ) );
                if ( ( efirc = efi_driver_start ( &efi_driver_binding, device,
                                                  NULL ) ) != 0 ) {
-                       rc = -EEFI_CONNECT ( efirc );
+                       rc = -EEFI ( efirc );
                        DBGC ( device, "EFIDRV %s could not connect driver "
                               "directly: %s\n", efi_handle_name ( device ),
                               strerror ( rc ) );
@@ -583,14 +563,13 @@ static int efi_driver_connect ( EFI_HANDLE device ) {
  * @ret rc             Return status code
  */
 static int efi_driver_disconnect ( EFI_HANDLE device ) {
-       EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+       EFI_HANDLE driver = efi_driver_binding.DriverBindingHandle;
 
        /* Disconnect our driver */
        efi_driver_disconnecting = 1;
-       bs->DisconnectController ( device,
-                                  efi_driver_binding.DriverBindingHandle,
-                                  NULL );
+       efi_disconnect ( device, driver );
        efi_driver_disconnecting = 0;
+
        return 0;
 }
 
@@ -601,10 +580,9 @@ static int efi_driver_disconnect ( EFI_HANDLE device ) {
  * @ret rc             Return status code
  */
 static int efi_driver_reconnect ( EFI_HANDLE device ) {
-       EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
 
        /* Reconnect any available driver */
-       bs->ConnectController ( device, NULL, NULL, TRUE );
+       efi_connect ( device, NULL );
 
        return 0;
 }
index 28dfc8680c4762ddbcd6b7406347ce2ce5bf699a..b09272f5851b1536028ec00636eae79f2cef8970 100644 (file)
@@ -1198,7 +1198,7 @@ static void efi_usb_uninstall ( struct efi_usb_interface *usbintf ) {
         * when uninstalling protocols.
         */
        if ( ! efi_shutdown_in_progress )
-               bs->DisconnectController ( usbintf->handle, NULL, NULL );
+               efi_disconnect ( usbintf->handle, NULL );
 
        /* Uninstall protocols */
        if ( ( ! efi_shutdown_in_progress ) &&
@@ -1260,7 +1260,6 @@ static void efi_usb_uninstall_all ( struct efi_usb_device *efiusb ) {
  */
 static int efi_usb_probe ( struct usb_function *func,
                           struct usb_configuration_descriptor *config ) {
-       EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
        struct usb_device *usb = func->usb;
        struct efi_usb_device *usbdev;
        struct efi_usb_interface *usbintf;
@@ -1318,7 +1317,7 @@ static int efi_usb_probe ( struct usb_function *func,
 
        /* Connect any external drivers */
        list_for_each_entry ( usbintf, &usbdev->interfaces, list )
-               bs->ConnectController ( usbintf->handle, NULL, NULL, TRUE );
+               efi_connect ( usbintf->handle, NULL );
 
        return 0;
 
index 3f9c9940e67833ae5b63d2310f1e57938e0e860a..da585db6046c3ef09fb78032386b0509e56b5761 100644 (file)
@@ -122,9 +122,7 @@ static int efi_veto_disconnect ( struct efi_veto *veto ) {
        /* Disconnect driver from all handles, in reverse order */
        for ( i = 0 ; i < count ; i++ ) {
                handle = handles[ count - i - 1 ];
-               efirc = bs->DisconnectController ( handle, driver, NULL );
-               if ( ( efirc != 0 ) && ( efirc != EFI_NOT_FOUND ) ) {
-                       rc = -EEFI ( efirc );
+               if ( ( rc = efi_disconnect ( handle, driver ) ) != 0 ) {
                        DBGC ( driver, "EFIVETO %s could not disconnect",
                               efi_handle_name ( driver ) );
                        DBGC ( driver, " %s: %s\n",