From: Michael Brown Date: Mon, 14 Jul 2025 11:17:11 +0000 (+0100) Subject: [efi] Drop to external TPL for calls to ConnectController() X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c3376f86457120e360161ce19e6862f31980c2f2;p=thirdparty%2Fipxe.git [efi] Drop to external TPL for calls to ConnectController() 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 --- diff --git a/src/include/ipxe/efi/efi.h b/src/include/ipxe/efi/efi.h index 03081f6f4..33278abed 100644 --- a/src/include/ipxe/efi/efi.h +++ b/src/include/ipxe/efi/efi.h @@ -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 diff --git a/src/include/ipxe/errfile.h b/src/include/ipxe/errfile.h index eee71bfaa..0e3634f9e 100644 --- a/src/include/ipxe/errfile.h +++ b/src/include/ipxe/errfile.h @@ -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 ) diff --git a/src/interface/efi/efi_block.c b/src/interface/efi/efi_block.c index aa5ec4e0f..cc93edb85 100644 --- a/src/interface/efi/efi_block.c +++ b/src/interface/efi/efi_block.c @@ -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 index 000000000..5aa8dc689 --- /dev/null +++ b/src/interface/efi/efi_connect.c @@ -0,0 +1,110 @@ +/* + * Copyright (C) 2025 Michael Brown . + * + * 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 +#include +#include + +/* 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; +} diff --git a/src/interface/efi/efi_driver.c b/src/interface/efi/efi_driver.c index ce1db228d..b1ff404be 100644 --- a/src/interface/efi/efi_driver.c +++ b/src/interface/efi/efi_driver.c @@ -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; } diff --git a/src/interface/efi/efi_usb.c b/src/interface/efi/efi_usb.c index 28dfc8680..b09272f58 100644 --- a/src/interface/efi/efi_usb.c +++ b/src/interface/efi/efi_usb.c @@ -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; diff --git a/src/interface/efi/efi_veto.c b/src/interface/efi/efi_veto.c index 3f9c9940e..da585db60 100644 --- a/src/interface/efi/efi_veto.c +++ b/src/interface/efi/efi_veto.c @@ -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",