[efi] Ensure that all drivers are shut down before the OS boots

Reported-by: Itay Gazit <itayg@mellanox.co.il>
Suggested-by: Michael R Turner <mikeyt@us.ibm.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/image/efi_image.c b/src/image/efi_image.c
index ac54fdf6c78d2a616011c655b73be414b9f9db2a..0575496cbcf7c0feaceaad367adb1ad07cf1dd82 100644 (file)
--- a/src/image/efi_image.c
+++ b/src/image/efi_image.c
@@ -26,20 +26,6 @@ FILE_LICENCE ( GPL2_OR_LATER );
 
 FEATURE ( FEATURE_IMAGE, "EFI", DHCP_EB_FEATURE_EFI, 1 );
 
-/** Event used to signal shutdown */
-static EFI_EVENT efi_shutdown_event;
-
-/**
- * Shut down in preparation for booting an OS.
- *
- * This hook gets called at ExitBootServices time in order to make sure that
- * the network cards are properly shut down before the OS takes over.
- */
-static EFIAPI void efi_shutdown_hook ( EFI_EVENT event __unused,
-                                      void *context __unused ) {
-       shutdown_boot();
-}
-
 /**
  * Execute EFI image
  *
@@ -64,30 +50,14 @@ static int efi_image_exec ( struct image *image ) {
                return -ENOEXEC;
        }
 
-       /* Be sure to shut down the NIC at ExitBootServices time, or else
-        * DMA from the card can corrupt the OS.
-        */
-       efirc = bs->CreateEvent ( EVT_SIGNAL_EXIT_BOOT_SERVICES,
-                                 TPL_CALLBACK, efi_shutdown_hook,
-                                 NULL, &efi_shutdown_event );
-       if ( efirc ) {
-               rc = EFIRC_TO_RC ( efirc );
-               goto done;
-       }
-
        /* Start the image */
        if ( ( efirc = bs->StartImage ( handle, &exit_data_size,
                                        &exit_data ) ) != 0 ) {
                DBGC ( image, "EFIIMAGE %p returned with status %s\n",
                       image, efi_strerror ( efirc ) );
        }
-
        rc = EFIRC_TO_RC ( efirc );
 
-       /* Remove the shutdown hook */
-       bs->CloseEvent ( efi_shutdown_event );
-
-done:
        /* Unload the image.  We can't leave it loaded, because we
         * have no "unload" operation.
         */

diff --git a/src/interface/efi/efi_init.c b/src/interface/efi/efi_init.c
index 029bc06fc6b09b3e4d63274902cb166b49749e1d..6c7b4955f6b2056f97822a627da94ea85993072c 100644 (file)
--- a/src/interface/efi/efi_init.c
+++ b/src/interface/efi/efi_init.c
@@ -22,6 +22,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #include <ipxe/efi/efi.h>
 #include <ipxe/efi/Protocol/LoadedImage.h>
 #include <ipxe/uuid.h>
+#include <ipxe/init.h>
 
 /** Image handle passed to entry point */
 EFI_HANDLE efi_image_handle;
@@ -36,6 +37,21 @@ EFI_SYSTEM_TABLE *efi_systab;
 static EFI_GUID efi_loaded_image_protocol_guid
        = EFI_LOADED_IMAGE_PROTOCOL_GUID;
 
+/** Event used to signal shutdown */
+static EFI_EVENT efi_shutdown_event;
+
+/**
+ * Shut down in preparation for booting an OS.
+ *
+ * This hook gets called at ExitBootServices time in order to make
+ * sure that everything is properly shut down before the OS takes
+ * over.
+ */
+static EFIAPI void efi_shutdown_hook ( EFI_EVENT event __unused,
+                                      void *context __unused ) {
+       shutdown_boot();
+}
+
 /**
  * Look up EFI configuration table
  *
@@ -129,5 +145,18 @@ EFI_STATUS efi_init ( EFI_HANDLE image_handle,
                }
        }
 
+       /* EFI is perfectly capable of gracefully shutting down any
+        * loaded devices if it decides to fall back to a legacy boot.
+        * For no particularly comprehensible reason, it doesn't
+        * bother doing so when ExitBootServices() is called.
+        */
+       if ( ( efirc = bs->CreateEvent ( EVT_SIGNAL_EXIT_BOOT_SERVICES,
+                                        TPL_CALLBACK, efi_shutdown_hook,
+                                        NULL, &efi_shutdown_event ) ) != 0 ) {
+               DBGC ( systab, "EFI could not create ExitBootServices event: "
+                      "%s\n", efi_strerror ( efirc ) );
+               return efirc;
+       }
+
        return 0;
 }

diff --git a/src/interface/efi/efi_pci.c b/src/interface/efi/efi_pci.c
index 3b393fcbe4538b62d98815dad0f0d4101bb60953..fa71e7d847502a0889aebec610e28a747de5b31f 100644 (file)
--- a/src/interface/efi/efi_pci.c
+++ b/src/interface/efi/efi_pci.c
@@ -489,7 +489,7 @@ static struct efi_driver efipci_driver =
  * Install EFI PCI driver
  *
  */
-static void efipci_driver_init ( void ) {
+static void efipci_driver_startup ( void ) {
        struct efi_driver *efidrv = &efipci_driver;
        EFI_STATUS efirc;
 
@@ -503,7 +503,27 @@ static void efipci_driver_init ( void ) {
        DBGC ( efidrv, "EFIPCI driver installed\n" );
 }
 
+/**
+ * Shut down EFI PCI driver
+ *
+ * @v booting          System is shutting down for OS boot
+ */
+static void efipci_driver_shutdown ( int booting __unused ) {
+       struct efi_driver *efidrv = &efipci_driver;
+       struct efi_pci_device *efipci;
+       struct efi_pci_device *tmp;
+
+       /* Shut down any remaining devices */
+       list_for_each_entry_safe ( efipci, tmp, &efi_pci_devices, list ) {
+               DBGC ( efipci, "EFIPCI " PCI_FMT " still active at shutdown; "
+                      "forcing close\n", PCI_ARGS ( &efipci->pci ) );
+               pci_remove ( &efipci->pci );
+               efipci_destroy ( efidrv, efipci );
+       }
+}
+
 /** EFI PCI startup function */
 struct startup_fn startup_pci __startup_fn ( STARTUP_NORMAL ) = {
-       .startup = efipci_driver_init,
+       .startup = efipci_driver_startup,
+       .shutdown = efipci_driver_shutdown,
 };