public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices
@ 2018-03-13 21:22 Laszlo Ersek
  2018-03-13 21:22 ` [PATCH 1/6] OvmfPkg/QemuBootOrderLib: wrap overlong line Laszlo Ersek
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Laszlo Ersek @ 2018-03-13 21:22 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen, Shannon Zhao, Xiang Zheng

Repo:   https://github.com/lersek/edk2.git
Branch: qemu_bootorder_connect

Adding tens or hundreds of bootable devices to a QEMU VM config slows
the OVMF and ArmVirtQemu boots to a crawl, several people have reported
in the past.

There are at least two reasons for this (high pflash traffic due to
heavy nvvar massaging per device, and PCI config space access slowing
down on QEMU as the number of regions increases). However, part of the
pain is self-inflicted in our PlatformBootManagerLib instances: we
connect all bootable devices (for maximum compatibility with the user's
VM config) even if the user doesn't intend to boot off most of them.

It's oft repeated that the set of devices connected during boot is
platform policy, so this series replaces the culprit
EfiBootManagerConnectAll() calls with a bit smarter algorithm.

I sought to keep the commit messages under control.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Shannon Zhao <zhaoshenglong@huawei.com>
Cc: Xiang Zheng <xiang.zheng@linaro.org>

Thanks,
Laszlo

Laszlo Ersek (6):
  OvmfPkg/QemuBootOrderLib: wrap overlong line
  OvmfPkg/QemuBootOrderLib: add missing EFIAPI specifiers
  OvmfPkg/QemuBootOrderLib: clean up translation of virtio-net over MMIO
  OvmfPkg/QemuBootOrderLib: add ConnectDevicesFromQemu()
  OvmfPkg/PlatformBootManagerLib: minimize the set of connected devices
  ArmVirtPkg/PlatformBootManagerLib: minimize the set of connected
    devices

 ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c |  12 +-
 OvmfPkg/Include/Library/QemuBootOrderLib.h             |  41 ++++-
 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c   |  16 +-
 OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c    | 165 +++++++++++++++++++-
 4 files changed, 218 insertions(+), 16 deletions(-)

-- 
2.14.1.3.gb7cf6e02401b



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/6] OvmfPkg/QemuBootOrderLib: wrap overlong line
  2018-03-13 21:22 [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices Laszlo Ersek
@ 2018-03-13 21:22 ` Laszlo Ersek
  2018-03-13 21:22 ` [PATCH 2/6] OvmfPkg/QemuBootOrderLib: add missing EFIAPI specifiers Laszlo Ersek
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Laszlo Ersek @ 2018-03-13 21:22 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen, Shannon Zhao, Xiang Zheng

81 characters is too many.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Shannon Zhao <zhaoshenglong@huawei.com>
Cc: Xiang Zheng <xiang.zheng@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
index 366104adf535..682cc7a4dca5 100644
--- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
+++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
@@ -1909,7 +1909,8 @@ SetBootOrderFromQemu (
                     BootOrder.Data
                     );
     if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "%a: setting BootOrder: %r\n", __FUNCTION__, Status));
+      DEBUG ((DEBUG_ERROR, "%a: setting BootOrder: %r\n", __FUNCTION__,
+        Status));
       goto ErrorFreeExtraPciRoots;
     }
 
-- 
2.14.1.3.gb7cf6e02401b




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/6] OvmfPkg/QemuBootOrderLib: add missing EFIAPI specifiers
  2018-03-13 21:22 [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices Laszlo Ersek
  2018-03-13 21:22 ` [PATCH 1/6] OvmfPkg/QemuBootOrderLib: wrap overlong line Laszlo Ersek
@ 2018-03-13 21:22 ` Laszlo Ersek
  2018-03-13 21:22 ` [PATCH 3/6] OvmfPkg/QemuBootOrderLib: clean up translation of virtio-net over MMIO Laszlo Ersek
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Laszlo Ersek @ 2018-03-13 21:22 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen, Shannon Zhao, Xiang Zheng

Public library APIs should be declared as EFIAPI.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Shannon Zhao <zhaoshenglong@huawei.com>
Cc: Xiang Zheng <xiang.zheng@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/Include/Library/QemuBootOrderLib.h          | 2 ++
 OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/OvmfPkg/Include/Library/QemuBootOrderLib.h b/OvmfPkg/Include/Library/QemuBootOrderLib.h
index 743a71782471..874344a95d80 100644
--- a/OvmfPkg/Include/Library/QemuBootOrderLib.h
+++ b/OvmfPkg/Include/Library/QemuBootOrderLib.h
@@ -49,6 +49,7 @@
 
 **/
 RETURN_STATUS
+EFIAPI
 SetBootOrderFromQemu (
   VOID
   );
@@ -61,6 +62,7 @@ SetBootOrderFromQemu (
   @return  The TimeoutDefault argument for PlatformBdsEnterFrontPage().
 **/
 UINT16
+EFIAPI
 GetFrontPageTimeoutFromQemu (
   VOID
   );
diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
index 682cc7a4dca5..b699970b12ab 100644
--- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
+++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
@@ -1763,6 +1763,7 @@ PruneBootVariables (
 
 **/
 RETURN_STATUS
+EFIAPI
 SetBootOrderFromQemu (
   VOID
   )
@@ -1946,6 +1947,7 @@ ErrorFreeFwCfg:
   @return  The TimeoutDefault argument for PlatformBdsEnterFrontPage().
 **/
 UINT16
+EFIAPI
 GetFrontPageTimeoutFromQemu (
   VOID
   )
-- 
2.14.1.3.gb7cf6e02401b




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/6] OvmfPkg/QemuBootOrderLib: clean up translation of virtio-net over MMIO
  2018-03-13 21:22 [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices Laszlo Ersek
  2018-03-13 21:22 ` [PATCH 1/6] OvmfPkg/QemuBootOrderLib: wrap overlong line Laszlo Ersek
  2018-03-13 21:22 ` [PATCH 2/6] OvmfPkg/QemuBootOrderLib: add missing EFIAPI specifiers Laszlo Ersek
@ 2018-03-13 21:22 ` Laszlo Ersek
  2018-03-13 21:22 ` [PATCH 4/6] OvmfPkg/QemuBootOrderLib: add ConnectDevicesFromQemu() Laszlo Ersek
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Laszlo Ersek @ 2018-03-13 21:22 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen, Shannon Zhao, Xiang Zheng

The "/MAC(" suffix of the translated UEFI devpath prefix is unnecessary
for matching, because the virtio-mmio base address in VenHwString is
unique anyway. Furthermore, the partial string "MAC(" cannot be processed
by ConvertTextToDevicePath(), which will become relevant later in this
series. Remove "/MAC(".

While at it, remove a bogus comment on PCI.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Shannon Zhao <zhaoshenglong@huawei.com>
Cc: Xiang Zheng <xiang.zheng@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
index b699970b12ab..a4213cf6d267 100644
--- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
+++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
@@ -1217,14 +1217,14 @@ TranslateMmioOfwNodes (
     //                |                             fixed
     //                base address of virtio-mmio register block
     //
-    // UEFI device path prefix (dependent on presence of nonzero PCI function):
+    // UEFI device path prefix:
     //
-    //   <VenHwString>/MAC(
+    //   <VenHwString>
     //
     Written = UnicodeSPrintAsciiFormat (
                 Translated,
                 *TranslatedSize * sizeof (*Translated), // BufferSize in bytes
-                "%s/MAC(",
+                "%s",
                 VenHwString
                 );
   } else {
-- 
2.14.1.3.gb7cf6e02401b




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/6] OvmfPkg/QemuBootOrderLib: add ConnectDevicesFromQemu()
  2018-03-13 21:22 [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices Laszlo Ersek
                   ` (2 preceding siblings ...)
  2018-03-13 21:22 ` [PATCH 3/6] OvmfPkg/QemuBootOrderLib: clean up translation of virtio-net over MMIO Laszlo Ersek
@ 2018-03-13 21:22 ` Laszlo Ersek
  2018-03-13 21:22 ` [PATCH 5/6] OvmfPkg/PlatformBootManagerLib: minimize the set of connected devices Laszlo Ersek
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Laszlo Ersek @ 2018-03-13 21:22 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen, Shannon Zhao, Xiang Zheng

QemuBootOrderLib expects PlatformBootManagerLib to call the following
triplet:

(1) EfiBootManagerConnectAll(),
(2) EfiBootManagerRefreshAllBootOption(),
(3) SetBootOrderFromQemu().

This leads to bad performance, when many devices exist such that the
firmware can drive them, but they aren't marked for booting in the
"bootorder" fw_cfg file. Namely,

(1) EfiBootManagerConnectAll() talks to all hardware, which takes long.
    Plus some DriverBindingStart() functions write NV variables, which is
    also slow. (For example, the IP config policy for each NIC is stored
    in an NV var that is named after the MAC).

(2) EfiBootManagerRefreshAllBootOption() generates boot options from the
    protocol instances produced by (1). Writing boot options is slow.

(3) Under the above circumstances, SetBootOrderFromQemu() removes most of
    the boot options produced by (2). Erasing boot options is slow.

Introduce ConnectDevicesFromQemu() as a replacement for (1): only connect
devices that the QEMU user actually wants to boot off of.

(There's a slight loss of compatibility when a platform switches from
EfiBootManagerConnectAll() to ConnectDevicesFromQemu().
EfiBootManagerConnectAll() may produce UEFI device paths that are unknown
to QemuBootOrderLib (that is, for neither PCI- nor virtio-mmio-based
devices). The BootOrderComplete() function lets such unmatched boot
options survive at the end of the boot order. With
ConnectDevicesFromQemu(), these options will not be auto-generated in the
first place. They may still be produced by other means.

SetBootOrderFromQemu() is not modified in any way; reordering+filtering
boot options remains a separate task.)

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Shannon Zhao <zhaoshenglong@huawei.com>
Cc: Xiang Zheng <xiang.zheng@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/Include/Library/QemuBootOrderLib.h          |  39 ++++-
 OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c | 154 +++++++++++++++++++-
 2 files changed, 189 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/Include/Library/QemuBootOrderLib.h b/OvmfPkg/Include/Library/QemuBootOrderLib.h
index 874344a95d80..d17de3bb8fae 100644
--- a/OvmfPkg/Include/Library/QemuBootOrderLib.h
+++ b/OvmfPkg/Include/Library/QemuBootOrderLib.h
@@ -20,6 +20,41 @@
 #include <Base.h>
 
 
+/**
+  Connect devices based on the boot order retrieved from QEMU.
+
+  Attempt to retrieve the "bootorder" fw_cfg file from QEMU. Translate the
+  OpenFirmware device paths therein to UEFI device path fragments. Connect the
+  devices identified by the UEFI devpath prefixes as narrowly as possible, then
+  connect all their child devices, recursively.
+
+  If this function fails, then platform BDS should fall back to
+  EfiBootManagerConnectAll(), or some other method for connecting any expected
+  boot devices.
+
+  @retval RETURN_SUCCESS            The "bootorder" fw_cfg file has been
+                                    parsed, and the referenced device-subtrees
+                                    have been connected.
+
+  @retval RETURN_UNSUPPORTED        QEMU's fw_cfg is not supported.
+
+  @retval RETURN_NOT_FOUND          Empty or nonexistent "bootorder" fw_cfg
+                                    file.
+
+  @retval RETURN_INVALID_PARAMETER  Parse error in the "bootorder" fw_cfg file.
+
+  @retval RETURN_OUT_OF_RESOURCES   Memory allocation failed.
+
+  @return                           Error statuses propagated from underlying
+                                    functions.
+**/
+RETURN_STATUS
+EFIAPI
+ConnectDevicesFromQemu (
+  VOID
+  );
+
+
 /**
 
   Set the boot order based on configuration retrieved from QEMU.
@@ -29,8 +64,8 @@
   translated fragments against the current list of boot options, and rewrite
   the BootOrder NvVar so that it corresponds to the order described in fw_cfg.
 
-  Platform BDS should call this function after EfiBootManagerConnectAll () and
-  EfiBootManagerRefreshAllBootOption () return.
+  Platform BDS should call this function after connecting any expected boot
+  devices and calling EfiBootManagerRefreshAllBootOption ().
 
   @retval RETURN_SUCCESS            BootOrder NvVar rewritten.
 
diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
index a4213cf6d267..15e4c67e48c5 100644
--- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
+++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
@@ -1453,6 +1453,156 @@ TranslateOfwPath (
 }
 
 
+/**
+  Connect devices based on the boot order retrieved from QEMU.
+
+  Attempt to retrieve the "bootorder" fw_cfg file from QEMU. Translate the
+  OpenFirmware device paths therein to UEFI device path fragments. Connect the
+  devices identified by the UEFI devpath prefixes as narrowly as possible, then
+  connect all their child devices, recursively.
+
+  If this function fails, then platform BDS should fall back to
+  EfiBootManagerConnectAll(), or some other method for connecting any expected
+  boot devices.
+
+  @retval RETURN_SUCCESS            The "bootorder" fw_cfg file has been
+                                    parsed, and the referenced device-subtrees
+                                    have been connected.
+
+  @retval RETURN_UNSUPPORTED        QEMU's fw_cfg is not supported.
+
+  @retval RETURN_NOT_FOUND          Empty or nonexistent "bootorder" fw_cfg
+                                    file.
+
+  @retval RETURN_INVALID_PARAMETER  Parse error in the "bootorder" fw_cfg file.
+
+  @retval RETURN_OUT_OF_RESOURCES   Memory allocation failed.
+
+  @return                           Error statuses propagated from underlying
+                                    functions.
+**/
+RETURN_STATUS
+EFIAPI
+ConnectDevicesFromQemu (
+  VOID
+  )
+{
+  RETURN_STATUS        Status;
+  FIRMWARE_CONFIG_ITEM FwCfgItem;
+  UINTN                FwCfgSize;
+  CHAR8                *FwCfg;
+  EFI_STATUS           EfiStatus;
+  EXTRA_ROOT_BUS_MAP   *ExtraPciRoots;
+  CONST CHAR8          *FwCfgPtr;
+  UINTN                NumConnected;
+  UINTN                TranslatedSize;
+  CHAR16               Translated[TRANSLATION_OUTPUT_SIZE];
+
+  Status = QemuFwCfgFindFile ("bootorder", &FwCfgItem, &FwCfgSize);
+  if (RETURN_ERROR (Status)) {
+    return Status;
+  }
+
+  if (FwCfgSize == 0) {
+    return RETURN_NOT_FOUND;
+  }
+
+  FwCfg = AllocatePool (FwCfgSize);
+  if (FwCfg == NULL) {
+    return RETURN_OUT_OF_RESOURCES;
+  }
+
+  QemuFwCfgSelectItem (FwCfgItem);
+  QemuFwCfgReadBytes (FwCfgSize, FwCfg);
+  if (FwCfg[FwCfgSize - 1] != '\0') {
+    Status = RETURN_INVALID_PARAMETER;
+    goto FreeFwCfg;
+  }
+  DEBUG ((DEBUG_VERBOSE, "%a: FwCfg:\n", __FUNCTION__));
+  DEBUG ((DEBUG_VERBOSE, "%a\n", FwCfg));
+  DEBUG ((DEBUG_VERBOSE, "%a: FwCfg: <end>\n", __FUNCTION__));
+
+  if (FeaturePcdGet (PcdQemuBootOrderPciTranslation)) {
+    EfiStatus = CreateExtraRootBusMap (&ExtraPciRoots);
+    if (EFI_ERROR (EfiStatus)) {
+      Status = (RETURN_STATUS)EfiStatus;
+      goto FreeFwCfg;
+    }
+  } else {
+    ExtraPciRoots = NULL;
+  }
+
+  //
+  // Translate each OpenFirmware path to a UEFI devpath prefix.
+  //
+  FwCfgPtr = FwCfg;
+  NumConnected = 0;
+  TranslatedSize = ARRAY_SIZE (Translated);
+  Status = TranslateOfwPath (&FwCfgPtr, ExtraPciRoots, Translated,
+             &TranslatedSize);
+  while (!RETURN_ERROR (Status)) {
+    EFI_DEVICE_PATH_PROTOCOL *DevicePath;
+    EFI_HANDLE               Controller;
+
+    //
+    // Convert the UEFI devpath prefix to binary representation.
+    //
+    ASSERT (Translated[TranslatedSize] == L'\0');
+    DevicePath = ConvertTextToDevicePath (Translated);
+    if (DevicePath == NULL) {
+      Status = RETURN_OUT_OF_RESOURCES;
+      goto FreeExtraPciRoots;
+    }
+    //
+    // Advance along DevicePath, connecting the nodes individually, and asking
+    // drivers not to produce sibling nodes. Retrieve the controller handle
+    // associated with the full DevicePath -- this is the device that QEMU's
+    // OFW devpath refers to.
+    //
+    EfiStatus = EfiBootManagerConnectDevicePath (DevicePath, &Controller);
+    FreePool (DevicePath);
+    if (EFI_ERROR (EfiStatus)) {
+      Status = (RETURN_STATUS)EfiStatus;
+      goto FreeExtraPciRoots;
+    }
+    //
+    // Because QEMU's OFW devpaths have lesser expressive power than UEFI
+    // devpaths (i.e., DevicePath is considered a prefix), connect the tree
+    // rooted at Controller, recursively. If no children are produced
+    // (EFI_NOT_FOUND), that's OK.
+    //
+    EfiStatus = gBS->ConnectController (Controller, NULL, NULL, TRUE);
+    if (EFI_ERROR (EfiStatus) && EfiStatus != EFI_NOT_FOUND) {
+      Status = (RETURN_STATUS)EfiStatus;
+      goto FreeExtraPciRoots;
+    }
+    ++NumConnected;
+    //
+    // Move to the next OFW devpath.
+    //
+    TranslatedSize = ARRAY_SIZE (Translated);
+    Status = TranslateOfwPath (&FwCfgPtr, ExtraPciRoots, Translated,
+               &TranslatedSize);
+  }
+
+  if (Status == RETURN_NOT_FOUND && NumConnected > 0) {
+    DEBUG ((DEBUG_INFO, "%a: %Lu OpenFirmware device path(s) connected\n",
+      __FUNCTION__, (UINT64)NumConnected));
+    Status = RETURN_SUCCESS;
+  }
+
+FreeExtraPciRoots:
+  if (ExtraPciRoots != NULL) {
+    DestroyExtraRootBusMap (ExtraPciRoots);
+  }
+
+FreeFwCfg:
+  FreePool (FwCfg);
+
+  return Status;
+}
+
+
 /**
 
   Convert the UEFI DevicePath to full text representation with DevPathToText,
@@ -1743,8 +1893,8 @@ PruneBootVariables (
   translated fragments against the current list of boot options, and rewrite
   the BootOrder NvVar so that it corresponds to the order described in fw_cfg.
 
-  Platform BDS should call this function after EfiBootManagerConnectAll () and
-  EfiBootManagerRefreshAllBootOption () return.
+  Platform BDS should call this function after connecting any expected boot
+  devices and calling EfiBootManagerRefreshAllBootOption ().
 
   @retval RETURN_SUCCESS            BootOrder NvVar rewritten.
 
-- 
2.14.1.3.gb7cf6e02401b




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/6] OvmfPkg/PlatformBootManagerLib: minimize the set of connected devices
  2018-03-13 21:22 [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices Laszlo Ersek
                   ` (3 preceding siblings ...)
  2018-03-13 21:22 ` [PATCH 4/6] OvmfPkg/QemuBootOrderLib: add ConnectDevicesFromQemu() Laszlo Ersek
@ 2018-03-13 21:22 ` Laszlo Ersek
  2018-03-13 21:22 ` [PATCH 6/6] ArmVirtPkg/PlatformBootManagerLib: " Laszlo Ersek
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Laszlo Ersek @ 2018-03-13 21:22 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen, Shannon Zhao, Xiang Zheng

Prefer ConnectDevicesFromQemu() to EfiBootManagerConnectAll().

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Shannon Zhao <zhaoshenglong@huawei.com>
Cc: Xiang Zheng <xiang.zheng@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
index 025252e72b39..5ef0e828900d 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
@@ -1346,7 +1346,8 @@ Returns:
 
 --*/
 {
-  UINTN Index;
+  UINTN         Index;
+  RETURN_STATUS Status;
 
   DEBUG ((EFI_D_INFO, "PlatformBdsConnectSequence\n"));
 
@@ -1365,11 +1366,14 @@ Returns:
     Index++;
   }
 
-  //
-  // Just use the simple policy to connect all devices
-  //
-  DEBUG ((EFI_D_INFO, "EfiBootManagerConnectAll\n"));
-  EfiBootManagerConnectAll ();
+  Status = ConnectDevicesFromQemu ();
+  if (RETURN_ERROR (Status)) {
+    //
+    // Just use the simple policy to connect all devices
+    //
+    DEBUG ((DEBUG_INFO, "EfiBootManagerConnectAll\n"));
+    EfiBootManagerConnectAll ();
+  }
 
   PciAcpiInitialization ();
 }
-- 
2.14.1.3.gb7cf6e02401b




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/6] ArmVirtPkg/PlatformBootManagerLib: minimize the set of connected devices
  2018-03-13 21:22 [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices Laszlo Ersek
                   ` (4 preceding siblings ...)
  2018-03-13 21:22 ` [PATCH 5/6] OvmfPkg/PlatformBootManagerLib: minimize the set of connected devices Laszlo Ersek
@ 2018-03-13 21:22 ` Laszlo Ersek
  2018-03-14  9:01 ` [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices Ard Biesheuvel
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Laszlo Ersek @ 2018-03-13 21:22 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen, Shannon Zhao, Xiang Zheng

Prefer ConnectDevicesFromQemu() to EfiBootManagerConnectAll().

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Shannon Zhao <zhaoshenglong@huawei.com>
Cc: Xiang Zheng <xiang.zheng@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
index f9591964d577..36e0eed2384a 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -663,15 +663,23 @@ PlatformBootManagerAfterConsole (
   VOID
   )
 {
+  RETURN_STATUS Status;
+
   //
   // Show the splash screen.
   //
   BootLogoEnableLogo ();
 
   //
-  // Connect the rest of the devices.
+  // Connect the purported boot devices.
   //
-  EfiBootManagerConnectAll ();
+  Status = ConnectDevicesFromQemu ();
+  if (RETURN_ERROR (Status)) {
+    //
+    // Connect the rest of the devices.
+    //
+    EfiBootManagerConnectAll ();
+  }
 
   //
   // Process QEMU's -kernel command line option. Note that the kernel booted
-- 
2.14.1.3.gb7cf6e02401b



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices
  2018-03-13 21:22 [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices Laszlo Ersek
                   ` (5 preceding siblings ...)
  2018-03-13 21:22 ` [PATCH 6/6] ArmVirtPkg/PlatformBootManagerLib: " Laszlo Ersek
@ 2018-03-14  9:01 ` Ard Biesheuvel
  2018-03-14  9:16 ` zhengxiang (A)
  2018-03-14 10:30 ` Laszlo Ersek
  8 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2018-03-14  9:01 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: edk2-devel-01, Jordan Justen, Shannon Zhao, Xiang Zheng

On 13 March 2018 at 21:22, Laszlo Ersek <lersek@redhat.com> wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: qemu_bootorder_connect
>
> Adding tens or hundreds of bootable devices to a QEMU VM config slows
> the OVMF and ArmVirtQemu boots to a crawl, several people have reported
> in the past.
>
> There are at least two reasons for this (high pflash traffic due to
> heavy nvvar massaging per device, and PCI config space access slowing
> down on QEMU as the number of regions increases). However, part of the
> pain is self-inflicted in our PlatformBootManagerLib instances: we
> connect all bootable devices (for maximum compatibility with the user's
> VM config) even if the user doesn't intend to boot off most of them.
>
> It's oft repeated that the set of devices connected during boot is
> platform policy, so this series replaces the culprit
> EfiBootManagerConnectAll() calls with a bit smarter algorithm.
>
> I sought to keep the commit messages under control.
>

This is really nice. Most platforms I've worked with just connect
everything all the time, which is sloppy. I'm glad you fixed this for
*VMF

Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> # ArmVirtQemu
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices
  2018-03-13 21:22 [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices Laszlo Ersek
                   ` (6 preceding siblings ...)
  2018-03-14  9:01 ` [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices Ard Biesheuvel
@ 2018-03-14  9:16 ` zhengxiang (A)
  2018-03-14 10:30 ` Laszlo Ersek
  8 siblings, 0 replies; 10+ messages in thread
From: zhengxiang (A) @ 2018-03-14  9:16 UTC (permalink / raw)
  To: Laszlo Ersek, edk2-devel-01
  Cc: Ard Biesheuvel, Jordan Justen, Shannon Zhao, Xiang Zheng



On 2018/3/14 5:22, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: qemu_bootorder_connect
> 
> Adding tens or hundreds of bootable devices to a QEMU VM config slows
> the OVMF and ArmVirtQemu boots to a crawl, several people have reported
> in the past.
> 
> There are at least two reasons for this (high pflash traffic due to
> heavy nvvar massaging per device, and PCI config space access slowing
> down on QEMU as the number of regions increases). However, part of the
> pain is self-inflicted in our PlatformBootManagerLib instances: we
> connect all bootable devices (for maximum compatibility with the user's
> VM config) even if the user doesn't intend to boot off most of them.
> 
> It's oft repeated that the set of devices connected during boot is
> platform policy, so this series replaces the culprit
> EfiBootManagerConnectAll() calls with a bit smarter algorithm.
> 
> I sought to keep the commit messages under control.
> 

Thanks, Laszlo! It works for booting QEMU VM with 60-virtio-scsi and
24-virtio-net on ARM64.
The boot time reduces from more than one hour to ~90 seconds.

Tested-by: Xiang Zheng <xiang.zheng@linaro.org>

Thanks,
Xiang



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices
  2018-03-13 21:22 [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices Laszlo Ersek
                   ` (7 preceding siblings ...)
  2018-03-14  9:16 ` zhengxiang (A)
@ 2018-03-14 10:30 ` Laszlo Ersek
  8 siblings, 0 replies; 10+ messages in thread
From: Laszlo Ersek @ 2018-03-14 10:30 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Jordan Justen, Xiang Zheng, Ard Biesheuvel

On 03/13/18 22:22, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: qemu_bootorder_connect
> 
> Adding tens or hundreds of bootable devices to a QEMU VM config slows
> the OVMF and ArmVirtQemu boots to a crawl, several people have reported
> in the past.
> 
> There are at least two reasons for this (high pflash traffic due to
> heavy nvvar massaging per device, and PCI config space access slowing
> down on QEMU as the number of regions increases). However, part of the
> pain is self-inflicted in our PlatformBootManagerLib instances: we
> connect all bootable devices (for maximum compatibility with the user's
> VM config) even if the user doesn't intend to boot off most of them.
> 
> It's oft repeated that the set of devices connected during boot is
> platform policy, so this series replaces the culprit
> EfiBootManagerConnectAll() calls with a bit smarter algorithm.

Thanks everyone for the super quick feedback!

Commit range 12957e56d26d..ff1d0fbfbaec.

Cheers,
Laszlo


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-03-14 10:23 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-13 21:22 [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices Laszlo Ersek
2018-03-13 21:22 ` [PATCH 1/6] OvmfPkg/QemuBootOrderLib: wrap overlong line Laszlo Ersek
2018-03-13 21:22 ` [PATCH 2/6] OvmfPkg/QemuBootOrderLib: add missing EFIAPI specifiers Laszlo Ersek
2018-03-13 21:22 ` [PATCH 3/6] OvmfPkg/QemuBootOrderLib: clean up translation of virtio-net over MMIO Laszlo Ersek
2018-03-13 21:22 ` [PATCH 4/6] OvmfPkg/QemuBootOrderLib: add ConnectDevicesFromQemu() Laszlo Ersek
2018-03-13 21:22 ` [PATCH 5/6] OvmfPkg/PlatformBootManagerLib: minimize the set of connected devices Laszlo Ersek
2018-03-13 21:22 ` [PATCH 6/6] ArmVirtPkg/PlatformBootManagerLib: " Laszlo Ersek
2018-03-14  9:01 ` [PATCH 0/6] OvmfPkg, ArmVirtQemu: leaner platform BDS policy for connecting devices Ard Biesheuvel
2018-03-14  9:16 ` zhengxiang (A)
2018-03-14 10:30 ` Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox