[efi] Run ExitBootServices shutdown hook at TPL_NOTIFY

On some systems (observed with the Thunderbolt ports on a ThinkPad X1
Extreme Gen3 and a ThinkPad P53), if the IOMMU is enabled then the
system firmware will install an ExitBootServices notification event
that disables bus mastering on the Thunderbolt xHCI controller and all
PCI bridges, and destroys any extant IOMMU mappings.  This leaves the
xHCI controller unable to perform any DMA operations.

As described in commit 236299b ("[xhci] Avoid DMA during shutdown if
firmware has disabled bus mastering"), any subsequent DMA operation
attempted by the xHCI controller will end up completing after the
operating system kernel has reenabled bus mastering, resulting in a
DMA operation to an area of memory that the hardware is no longer
permitted to access and, on Windows with the Driver Verifier enabled,
a STOP 0xE6 (DRIVER_VERIFIER_DMA_VIOLATION).

That commit avoids triggering any DMA attempts during the shutdown of
the xHCI controller itself.  However, this is not a complete solution
since any attached and opened USB device (e.g. a USB NIC) may
asynchronously trigger DMA attempts that happen to occur after bus
mastering has been disabled but before we reset the xHCI controller.

Avoid this problem by installing our own ExitBootServices notification
event at TPL_NOTIFY, thereby causing it to be invoked before the
firmware's own ExitBootServices notification event that disables bus
mastering.

This unsurprisingly causes the shutdown hook itself to be invoked at
TPL_NOTIFY, which causes a fatal error when later code attempts to
raise the TPL to TPL_CALLBACK (which is a lower TPL).  Work around
this problem by redefining the "internal" iPXE TPL to be variable, and
set this internal TPL to TPL_NOTIFY when the shutdown hook is invoked.

Avoid calling into an underlying SNP protocol instance from within our
shutdown hook at TPL_NOTIFY, since the underlying SNP driver may
attempt to raise the TPL to TPL_CALLBACK (which would cause a fatal
error).  Failing to shut down the underlying SNP device is safe to do
since the underlying device must, in any case, have installed its own
ExitBootServices hook if any shutdown actions are required.

Reported-by: Andreas Hammarskjöld <junior@2PintSoftware.com>
Tested-by: Andreas Hammarskjöld <junior@2PintSoftware.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/drivers/net/efi/nii.c b/src/drivers/net/efi/nii.c
index b9f34650eaf174c34bdda46b99877d2990afc9b5..833462e77f9f0a97d3112a5d6cb410e99ef15ae0 100644 (file)
--- a/src/drivers/net/efi/nii.c
+++ b/src/drivers/net/efi/nii.c
@@ -576,7 +576,7 @@ static int nii_issue_cpb_db ( struct nii_nic *nii, unsigned int op, void *cpb,
        cdb.IFnum = nii->nii->IfNum;
 
        /* Raise task priority level */
-       tpl = bs->RaiseTPL ( TPL_CALLBACK );
+       tpl = bs->RaiseTPL ( efi_internal_tpl );
 
        /* Issue command */
        DBGC2 ( nii, "NII %s issuing %02x:%04x ifnum %d%s%s\n",

diff --git a/src/drivers/net/efi/snpnet.c b/src/drivers/net/efi/snpnet.c
index fb52402771b70dc34c3d2719239f56dd1020fcbc..69ec6f5e554e7b39f0fab73d0b800ec2034dcb13 100644 (file)
--- a/src/drivers/net/efi/snpnet.c
+++ b/src/drivers/net/efi/snpnet.c
@@ -164,6 +164,10 @@ static int snpnet_transmit ( struct net_device *netdev,
        EFI_STATUS efirc;
        int rc;
 
+       /* Do nothing if shutdown is in progress */
+       if ( efi_shutdown_in_progress )
+               return -ECANCELED;
+
        /* Defer the packet if there is already a transmission in progress */
        if ( snp->txbuf ) {
                netdev_tx_defer ( netdev, iobuf );
@@ -283,6 +287,10 @@ static void snpnet_poll_rx ( struct net_device *netdev ) {
  */
 static void snpnet_poll ( struct net_device *netdev ) {
 
+       /* Do nothing if shutdown is in progress */
+       if ( efi_shutdown_in_progress )
+               return;
+
        /* Process any TX completions */
        snpnet_poll_tx ( netdev );
 
@@ -426,8 +434,9 @@ static void snpnet_close ( struct net_device *netdev ) {
        EFI_STATUS efirc;
        int rc;
 
-       /* Shut down NIC */
-       if ( ( efirc = snp->snp->Shutdown ( snp->snp ) ) != 0 ) {
+       /* Shut down NIC (unless whole system shutdown is in progress) */
+       if ( ( ! efi_shutdown_in_progress ) &&
+            ( ( efirc = snp->snp->Shutdown ( snp->snp ) ) != 0 ) ) {
                rc = -EEFI ( efirc );
                DBGC ( snp, "SNP %s could not shut down: %s\n",
                       netdev->name, strerror ( rc ) );
@@ -589,8 +598,9 @@ void snpnet_stop ( struct efi_device *efidev ) {
        /* Unregister network device */
        unregister_netdev ( netdev );
 
-       /* Stop SNP protocol */
-       if ( ( efirc = snp->snp->Stop ( snp->snp ) ) != 0 ) {
+       /* Stop SNP protocol (unless whole system shutdown is in progress) */
+       if ( ( ! efi_shutdown_in_progress ) &&
+            ( ( efirc = snp->snp->Stop ( snp->snp ) ) != 0 ) ) {
                rc = -EEFI ( efirc );
                DBGC ( device, "SNP %s could not stop: %s\n",
                       efi_handle_name ( device ), strerror ( rc ) );

diff --git a/src/include/ipxe/efi/efi.h b/src/include/ipxe/efi/efi.h
index a83fa0f276e64990509b70bea9abe1925fd1e521..1dd0d44538031e14781964798e9f330572fc9d22 100644 (file)
--- a/src/include/ipxe/efi/efi.h
+++ b/src/include/ipxe/efi/efi.h
@@ -223,6 +223,7 @@ extern EFI_HANDLE efi_image_handle;
 extern EFI_LOADED_IMAGE_PROTOCOL *efi_loaded_image;
 extern EFI_DEVICE_PATH_PROTOCOL *efi_loaded_image_path;
 extern EFI_SYSTEM_TABLE *efi_systab;
+extern EFI_TPL efi_internal_tpl;
 extern EFI_TPL efi_external_tpl;
 extern int efi_shutdown_in_progress;
 

diff --git a/src/interface/efi/efi_entropy.c b/src/interface/efi/efi_entropy.c
index 70cd062939c60f745b0cbca5571dbb3ba77016ad..1e8ddfb68d45f2eda50d9d8062d42d40a77dfbf9 100644 (file)
--- a/src/interface/efi/efi_entropy.c
+++ b/src/interface/efi/efi_entropy.c
@@ -104,8 +104,8 @@ static void efi_entropy_disable ( void ) {
        /* Close timer tick event */
        bs->CloseEvent ( tick );
 
-       /* Return to TPL_CALLBACK */
-       bs->RaiseTPL ( TPL_CALLBACK );
+       /* Return to internal TPL */
+       bs->RaiseTPL ( efi_internal_tpl );
 }
 
 /**

diff --git a/src/interface/efi/efi_init.c b/src/interface/efi/efi_init.c
index 1c6e9d440469c900078f1fae5af59b4bb4184f00..5d98f9ff7b8d7fb310f6b7276cecdb3ef1d89639 100644 (file)
--- a/src/interface/efi/efi_init.c
+++ b/src/interface/efi/efi_init.c
@@ -47,6 +47,9 @@ EFI_DEVICE_PATH_PROTOCOL *efi_loaded_image_path;
  */
 EFI_SYSTEM_TABLE * _C2 ( PLATFORM, _systab );
 
+/** Internal task priority level */
+EFI_TPL efi_internal_tpl = TPL_CALLBACK;
+
 /** External task priority level */
 EFI_TPL efi_external_tpl = TPL_APPLICATION;
 
@@ -79,6 +82,17 @@ static EFI_STATUS EFIAPI efi_unload ( EFI_HANDLE image_handle );
 static EFIAPI void efi_shutdown_hook ( EFI_EVENT event __unused,
                                       void *context __unused ) {
 
+       /* This callback is invoked at TPL_NOTIFY in order to ensure
+        * that we have an opportunity to shut down cleanly before
+        * other shutdown hooks perform destructive operations such as
+        * disabling the IOMMU.
+        *
+        * Modify the internal task priority level so that no code
+        * attempts to raise from TPL_NOTIFY to TPL_CALLBACK (which
+        * would trigger a fatal exception).
+        */
+       efi_internal_tpl = TPL_NOTIFY;
+
        /* Mark shutdown as being in progress, to indicate that large
         * parts of the system (e.g. timers) are no longer functional.
         */
@@ -273,7 +287,7 @@ EFI_STATUS efi_init ( EFI_HANDLE image_handle,
         * bother doing so when ExitBootServices() is called.
         */
        if ( ( efirc = bs->CreateEvent ( EVT_SIGNAL_EXIT_BOOT_SERVICES,
-                                        TPL_CALLBACK, efi_shutdown_hook,
+                                        TPL_NOTIFY, efi_shutdown_hook,
                                         NULL, &efi_shutdown_event ) ) != 0 ) {
                rc = -EEFI ( efirc );
                DBGC ( systab, "EFI could not create ExitBootServices event: "
@@ -373,7 +387,7 @@ __attribute__ (( noreturn )) void __stack_chk_fail ( void ) {
 }
 
 /**
- * Raise task priority level to TPL_CALLBACK
+ * Raise task priority level to internal level
  *
  * @v tpl              Saved TPL
  */
@@ -384,7 +398,7 @@ void efi_raise_tpl ( struct efi_saved_tpl *tpl ) {
        tpl->previous = efi_external_tpl;
 
        /* Raise TPL and record previous TPL as new external TPL */
-       tpl->current = bs->RaiseTPL ( TPL_CALLBACK );
+       tpl->current = bs->RaiseTPL ( efi_internal_tpl );
        efi_external_tpl = tpl->current;
 }
 

diff --git a/src/interface/efi/efi_timer.c b/src/interface/efi/efi_timer.c
index 405cd3454f430c32d53d22baf2b772123122f068..6427eb1d82bc94617b261177a627611ddcd31f63 100644 (file)
--- a/src/interface/efi/efi_timer.c
+++ b/src/interface/efi/efi_timer.c
@@ -137,7 +137,7 @@ static unsigned long efi_currticks ( void ) {
                efi_jiffies++;
        } else {
                bs->RestoreTPL ( efi_external_tpl );
-               bs->RaiseTPL ( TPL_CALLBACK );
+               bs->RaiseTPL ( efi_internal_tpl );
        }
 
        return ( efi_jiffies * ( TICKS_PER_SEC / EFI_JIFFIES_PER_SEC ) );