Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MdeModulePkg: Optimize BmExpandPartitionDevicePath to avoid ConnectAll #4892

Conversation

ajyoung-oracle
Copy link
Contributor

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 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.

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 appearing at a different PCI Bus/Dev). This new optimization covers both of these cases on requisite platforms which explicity connect boot devices (e.g. OVMF/QEMU).

In our testing on OVMF/QEMU VMs with dozens of configured vnic devices, these extraneous calls to EfiBootManagerConnectAll from BmExpandPartitionDevicePath were found to cause 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

BmExpandPartitionDevicePath calls EfiBootManagerConnectAll to expand
"short form" device paths - which are commonly used with OS boot options.
However on some platforms (such as OVMF/QEMU), this ConnectAll call
is often unnecessary as the BlockIo handles have already been created by
prior explicit boot device connection. ConnectAll calls are very
costly and should be avoided whenever feasible. So, optimze the
code to first search the existing BlockIo handles for a match.
If no match is found, then fallback to do a ConnectAll and search again.

NOTE: the existing optimization in the code - to use a "HDDP" variable
to store the last matching device path - doesn't cover the first time
a device path is expanded nor when a boot device's location changes.
In both of these cases, a ConnectAll call is still made from
BmExpandPartitionDevicePath. This new optimization fixes both of
these cases on requisite platforms.

Signed-off-by: Aaron Young <aaron.young@oracle.com>
lersek pushed a commit to lersek/edk2 that referenced this pull request Oct 24, 2023
Reference: tianocore#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]
@lersek
Copy link
Member

lersek commented Oct 24, 2023

personal CI run; superseded by #4948

@lersek lersek closed this Oct 24, 2023
mergify bot pushed a commit that referenced this pull request Oct 24, 2023
Reference: #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]
mdkinney pushed a commit to mdkinney/edk2-codereview that referenced this pull request Nov 7, 2023
Reference: tianocore/edk2#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]
mdkinney pushed a commit to mdkinney/edk2-codereview that referenced this pull request Nov 7, 2023
Reference: tianocore/edk2#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]
mdkinney pushed a commit to mdkinney/edk2-codereview that referenced this pull request Nov 7, 2023
Reference: tianocore/edk2#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]
MikhailKrichanov pushed a commit to acidanthera/audk that referenced this pull request Dec 11, 2023
Reference: tianocore/edk2#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]
mxu9 pushed a commit to tianocore/edk2-staging that referenced this pull request Jan 5, 2024
Reference: tianocore/edk2#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]
Wenxing-hou pushed a commit to Wenxing-hou/edk2-staging that referenced this pull request Mar 13, 2024
Reference: tianocore/edk2#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]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants