MdeModulePkg: Optimize BmExpandPartitionDevicePath

Reference: https://github.com/tianocore/edk2/pull/4892

BmExpandPartitionDevicePath is called to expand "short-form" device paths
which are commonly used with OS boot options. To expand a device path, it
calls EfiBootManagerConnectAll to connect all the possible BlockIo
devices in the system to search for a matching partition. However, this
is sometimes unnecessary on certain platforms (such as OVMF/QEMU) because
the boot devices are previously explicity connected
(See: ConnectDevicesFromQemu).  EfiBootManagerConnectAll calls are
extremely costly in terms of boot time and resources and should be avoided
whenever feasible.

(

OVMF call tree:

  PlatformBootManagerAfterConsole()         [OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c]
    PlatformBdsConnectSequence()            [OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c]
      ConnectDevicesFromQemu()              [OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c]
        ...
    EfiBootManagerRefreshAllBootOption()    [MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c]
      ...
    SetBootOrderFromQemu()                  [OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c]
      Match()                               [OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c]
        EfiBootManagerGetLoadOptionBuffer() [MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c]
          BmGetNextLoadOptionBuffer()       [MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c]
            BmGetNextLoadOptionDevicePath() [MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c]
              BmExpandPartitionDevicePath() [MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c]

)

Therefore optimize BmExpandPartitionDevicePath to first search the
existing BlockIo handles for a match. If a match is not found, then
fallback to the original code to call EfiBootManagerConnectAll and search
again. Thus, this optimization should be extremely low-risk given the
fallback to previous behavior.

NOTE: The existing optimization in the code to use a "HDDP" variable to
save the last matched device paths does not cover the first time a boot
option is expanded (i.e. before the "HDDP" is created) nor when the device
configuration has changed (resulting in the boot device moving to a
different location in the PCI Bus/Dev hierarchy). This new optimization
covers both of these cases on requisite platforms which explicity connect
boot devices.

In our testing on OVMF/QEMU VMs with dozens of configured vnic devices,
these extraneous calls to EfiBootManagerConnectAll from
BmExpandPartitionDevicePath were found to cause many seconds (or even
minutes) of additional VM boot time in some cases - due to the vnics
being unnecessarily connected.

Cc: Zhichao Gao zhichao.gao@intel.com
Cc: Ray Ni ray.ni@intel.com
Signed-off-by: Aaron Young <aaron.young@oracle.com>
Message-Id: <20231010150644.37857-1-Aaron.Young@oracle.com>
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
[lersek@redhat.com: add OVMF call tree to commit message]

1 files changed, 56 insertions, 34 deletions

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index c3763c4483..7a97f7cdcc 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -880,6 +880,8 @@ BmExpandPartitionDevicePath (
   BOOLEAN                   NeedAdjust;

   EFI_DEVICE_PATH_PROTOCOL  *Instance;

   UINTN                     Size;

+  BOOLEAN                   MatchFound;

+  BOOLEAN                   ConnectAllAttempted;

 

   //

   // Check if there is prestore 'HDDP' variable.

@@ -974,49 +976,69 @@ BmExpandPartitionDevicePath (
   // If we get here we fail to find or 'HDDP' not exist, and now we need

   // to search all devices in the system for a matched partition

   //

-  EfiBootManagerConnectAll ();

-  Status = gBS->LocateHandleBuffer (ByProtocol, &gEfiBlockIoProtocolGuid, NULL, &BlockIoHandleCount, &BlockIoBuffer);

-  if (EFI_ERROR (Status)) {

-    BlockIoHandleCount = 0;

-    BlockIoBuffer      = NULL;

-  }

-

-  //

-  // Loop through all the device handles that support the BLOCK_IO Protocol

-  //

-  for (Index = 0; Index < BlockIoHandleCount; Index++) {

-    BlockIoDevicePath = DevicePathFromHandle (BlockIoBuffer[Index]);

-    if (BlockIoDevicePath == NULL) {

-      continue;

+  BlockIoBuffer       = NULL;

+  MatchFound          = FALSE;

+  ConnectAllAttempted = FALSE;

+  do {

+    if (BlockIoBuffer != NULL) {

+      FreePool (BlockIoBuffer);

     }

 

-    if (BmMatchPartitionDevicePathNode (BlockIoDevicePath, (HARDDRIVE_DEVICE_PATH *)FilePath)) {

-      //

-      // Find the matched partition device path

-      //

-      TempDevicePath = AppendDevicePath (BlockIoDevicePath, NextDevicePathNode (FilePath));

-      FullPath       = BmGetNextLoadOptionDevicePath (TempDevicePath, NULL);

-      FreePool (TempDevicePath);

+    Status = gBS->LocateHandleBuffer (ByProtocol, &gEfiBlockIoProtocolGuid, NULL, &BlockIoHandleCount, &BlockIoBuffer);

+    if (EFI_ERROR (Status)) {

+      BlockIoHandleCount = 0;

+      BlockIoBuffer      = NULL;

+    }

 

-      if (FullPath != NULL) {

-        BmCachePartitionDevicePath (&CachedDevicePath, BlockIoDevicePath);

+    //

+    // Loop through all the device handles that support the BLOCK_IO Protocol

+    //

+    for (Index = 0; Index < BlockIoHandleCount; Index++) {

+      BlockIoDevicePath = DevicePathFromHandle (BlockIoBuffer[Index]);

+      if (BlockIoDevicePath == NULL) {

+        continue;

+      }

 

+      if (BmMatchPartitionDevicePathNode (BlockIoDevicePath, (HARDDRIVE_DEVICE_PATH *)FilePath)) {

         //

-        // Save the matching Device Path so we don't need to do a connect all next time

-        // Failing to save only impacts performance next time expanding the short-form device path

+        // Find the matched partition device path

         //

-        Status = gRT->SetVariable (

-                        L"HDDP",

-                        &mBmHardDriveBootVariableGuid,

-                        EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE,

-                        GetDevicePathSize (CachedDevicePath),

-                        CachedDevicePath

-                        );

+        TempDevicePath = AppendDevicePath (BlockIoDevicePath, NextDevicePathNode (FilePath));

+        FullPath       = BmGetNextLoadOptionDevicePath (TempDevicePath, NULL);

+        FreePool (TempDevicePath);

 

-        break;

+        if (FullPath != NULL) {

+          BmCachePartitionDevicePath (&CachedDevicePath, BlockIoDevicePath);

+

+          //

+          // Save the matching Device Path so we don't need to do a connect all next time

+          // Failing to save only impacts performance next time expanding the short-form device path

+          //

+          Status = gRT->SetVariable (

+                          L"HDDP",

+                          &mBmHardDriveBootVariableGuid,

+                          EFI_VARIABLE_BOOTSERVICE_ACCESS |

+                          EFI_VARIABLE_NON_VOLATILE,

+                          GetDevicePathSize (CachedDevicePath),

+                          CachedDevicePath

+                          );

+          MatchFound = TRUE;

+          break;

+        }

       }

     }

-  }

+

+    //

+    // If we found a matching BLOCK_IO handle or we've already

+    // tried a ConnectAll, we are done searching.

+    //

+    if (MatchFound || ConnectAllAttempted) {

+      break;

+    }

+

+    EfiBootManagerConnectAll ();

+    ConnectAllAttempted = TRUE;

+  } while (1);

 

   if (CachedDevicePath != NULL) {

     FreePool (CachedDevicePath);