* [PATCH v2 0/5] MdeModulePkg, OvmfPkg, ArmVirtPkg: more visible boot progress reporting
@ 2019-02-20 8:16 Laszlo Ersek
2019-02-20 8:16 ` [PATCH v2 1/5] MdeModulePkg/UefiBootManagerLib: fix LoadImage/StartImage status code rep Laszlo Ersek
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Laszlo Ersek @ 2019-02-20 8:16 UTC (permalink / raw)
To: edk2-devel
Cc: Anthony Perard, Ard Biesheuvel, Dandan Bi, Hao Wu, Jian J Wang,
Jordan Justen, Julien Grall, Ray Ni, Sean Brogan, Star Zeng
Repo: https://github.com/lersek/edk2.git
Branch: boot_diags_v2
This is version 2 of the series
[edk2] [PATCH 0/5] MdeModulePkg, OvmfPkg: more easily visible boot progress reporting
which was originally posted at
http://mid.mail-archive.com/20171122235849.4177-1-lersek@redhat.com
https://lists.01.org/pipermail/edk2-devel/2017-November/017850.html
The most important changes are listed on the v2 patches individually.
(It doesn't make much sense to compare v1 and v2 code-wise, because core
infrastructure has changed significantly since v1.)
The first patch is a bugfix for MdeModulePkg/UefiBootManagerLib; the
rest enables the feature in OvmfPkg, and in ArmVirtPkg/ArmVirtQemu*.
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@linaro.org>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Star Zeng <star.zeng@intel.com>
Thanks,
Laszlo
Laszlo Ersek (5):
MdeModulePkg/UefiBootManagerLib: fix LoadImage/StartImage status code
rep.
OvmfPkg: add library to track boot option loading/starting on the
console
OvmfPkg/PlatformBootManagerLib: display boot option loading/starting
ArmVirtPkg/ArmVirtQemu*: enable minimal Status Code Routing in DXE
ArmVirtPkg/PlatformBootManagerLib: display boot option
loading/starting
ArmVirtPkg/ArmVirtQemu.dsc | 11 +
ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 5 +
ArmVirtPkg/ArmVirtQemuKernel.dsc | 11 +
ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 3 +
ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 +
MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 69 +++--
MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h | 1 +
OvmfPkg/Include/Library/PlatformBmPrintScLib.h | 41 +++
OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf | 66 +++++
OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c | 310 ++++++++++++++++++++
OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 3 +
OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 +
OvmfPkg/OvmfPkg.dec | 5 +
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
16 files changed, 512 insertions(+), 18 deletions(-)
create mode 100644 OvmfPkg/Include/Library/PlatformBmPrintScLib.h
create mode 100644 OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
create mode 100644 OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c
--
2.19.1.3.g30247aa5d201
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/5] MdeModulePkg/UefiBootManagerLib: fix LoadImage/StartImage status code rep.
2019-02-20 8:16 [PATCH v2 0/5] MdeModulePkg, OvmfPkg, ArmVirtPkg: more visible boot progress reporting Laszlo Ersek
@ 2019-02-20 8:16 ` Laszlo Ersek
2019-02-20 13:17 ` Ni, Ray
2019-02-20 8:16 ` [PATCH v2 2/5] OvmfPkg: add library to track boot option loading/starting on the console Laszlo Ersek
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Laszlo Ersek @ 2019-02-20 8:16 UTC (permalink / raw)
To: edk2-devel; +Cc: Dandan Bi, Hao Wu, Jian J Wang, Ray Ni, Sean Brogan, Star Zeng
In the EFI_RETURN_STATUS_EXTENDED_DATA structure from PI-1.7, there may be
padding between the DataHeader and ReturnStatus members. The
REPORT_STATUS_CODE_EX() macro starts populating the structure immediately
after DataHeader, therefore the source data must provide for the padding.
Extract the BmReportImageFailure() function from EfiBootManagerBoot(),
prepare a zero padding (if any) in a temporary
EFI_RETURN_STATUS_EXTENDED_DATA object, and fix the
REPORT_STATUS_CODE_EX() macro invocation.
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Star Zeng <star.zeng@intel.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1539
Fixes: c2cf8720a5aad74230767a1f11bade2d86de3745
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- new in v2
MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h | 1 +
MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 69 +++++++++++++++-----
2 files changed, 52 insertions(+), 18 deletions(-)
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index 978fbff966f6..0fef63fceedf 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
+++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
@@ -51,6 +51,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Guid/MemoryTypeInformation.h>
#include <Guid/FileInfo.h>
#include <Guid/GlobalVariable.h>
+#include <Guid/StatusCodeDataTypeId.h>
#include <Guid/StatusCodeDataTypeVariable.h>
#include <Library/PrintLib.h>
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 9be1633b7480..ffb98c6c9b83 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1667,6 +1667,55 @@ BmIsBootManagerMenuFilePath (
return FALSE;
}
+/**
+ Report status code with EFI_RETURN_STATUS_EXTENDED_DATA about LoadImage() or
+ StartImage() failure.
+
+ @param[in] ErrorCode An Error Code in the Software Class, DXE Boot
+ Service Driver Subclass. ErrorCode will be used to
+ compose the Value parameter for status code
+ reporting. Must be one of
+ EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR and
+ EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED.
+
+ @param[in] FailureStatus The failure status returned by the boot service
+ that should be reported.
+**/
+VOID
+BmReportImageFailure (
+ IN UINT32 ErrorCode,
+ IN EFI_STATUS FailureStatus
+ )
+{
+ EFI_RETURN_STATUS_EXTENDED_DATA ExtendedData;
+ VOID *PaddingStart;
+ UINTN PaddingSize;
+
+ if (!ReportErrorCodeEnabled ()) {
+ return;
+ }
+
+ ASSERT (
+ (ErrorCode == EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR) ||
+ (ErrorCode == EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED)
+ );
+
+ PaddingStart = &ExtendedData.DataHeader + 1;
+ PaddingSize = (UINTN) &ExtendedData.ReturnStatus - (UINTN) PaddingStart;
+ ZeroMem (PaddingStart, PaddingSize);
+ ExtendedData.ReturnStatus = FailureStatus;
+
+ REPORT_STATUS_CODE_EX (
+ (EFI_ERROR_CODE | EFI_ERROR_MINOR),
+ (EFI_SOFTWARE_DXE_BS_DRIVER | ErrorCode),
+ 0,
+ NULL,
+ NULL,
+ PaddingStart,
+ PaddingSize + sizeof (ExtendedData.ReturnStatus)
+ );
+}
+
/**
Attempt to boot the EFI boot option. This routine sets L"BootCurent" and
also signals the EFI ready to boot event. If the device path for the option
@@ -1822,15 +1871,7 @@ EfiBootManagerBoot (
//
// Report Status Code with the failure status to indicate that the failure to load boot option
//
- REPORT_STATUS_CODE_EX (
- EFI_ERROR_CODE | EFI_ERROR_MINOR,
- (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR),
- 0,
- NULL,
- NULL,
- &Status,
- sizeof (EFI_STATUS)
- );
+ BmReportImageFailure (EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR, Status);
BootOption->Status = Status;
//
// Destroy the RAM disk
@@ -1911,15 +1952,7 @@ EfiBootManagerBoot (
//
// Report Status Code with the failure status to indicate that boot failure
//
- REPORT_STATUS_CODE_EX (
- EFI_ERROR_CODE | EFI_ERROR_MINOR,
- (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED),
- 0,
- NULL,
- NULL,
- &Status,
- sizeof (EFI_STATUS)
- );
+ BmReportImageFailure (EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED, Status);
}
PERF_END_EX (gImageHandle, "BdsAttempt", NULL, 0, (UINT32) OptionNumber);
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/5] OvmfPkg: add library to track boot option loading/starting on the console
2019-02-20 8:16 [PATCH v2 0/5] MdeModulePkg, OvmfPkg, ArmVirtPkg: more visible boot progress reporting Laszlo Ersek
2019-02-20 8:16 ` [PATCH v2 1/5] MdeModulePkg/UefiBootManagerLib: fix LoadImage/StartImage status code rep Laszlo Ersek
@ 2019-02-20 8:16 ` Laszlo Ersek
2019-02-20 9:21 ` Ard Biesheuvel
2019-02-20 10:04 ` Laszlo Ersek
2019-02-20 8:16 ` [PATCH v2 3/5] OvmfPkg/PlatformBootManagerLib: display boot option loading/starting Laszlo Ersek
` (2 subsequent siblings)
4 siblings, 2 replies; 12+ messages in thread
From: Laszlo Ersek @ 2019-02-20 8:16 UTC (permalink / raw)
To: edk2-devel
Cc: Anthony Perard, Ard Biesheuvel, Jordan Justen, Julien Grall,
Ray Ni
Introduce the Platform Boot Manager Print Status Code Library (for short,
PlatformBmPrintScLib) class for catching and printing the LoadImage() /
StartImage() preparations, and return statuses, that are reported by
UefiBootManagerLib.
In the primary library instance, catch only such status codes that
UefiBootManagerLib reports from the same module that contains
PlatformBmPrintScLib. The intent is to establish a reporting-printing
channel within BdsDxe, between UefiBootManagerLib and
PlatformBmPrintScLib. Ignore status codes originating elsewhence, e.g.
from UiApp's copy of UefiBootManagerLib.
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@linaro.org>
Cc: Ray Ni <ray.ni@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1515418
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- Split the status code handling to a separate library, so that it's
easy to reuse in ArmVirtPkg.
- Rework the logic based on
<https://bugzilla.tianocore.org/show_bug.cgi?id=1398> and
<https://mantis.uefi.org/mantis/view.php?id=1885>, and follow Ray's
advice in
<http://mid.mail-archive.com/734D49CCEBEEF84792F5B80ED585239D5BACE29B@SHSMSX104.ccr.corp.intel.com>:
- The boot option details are fetched via BootCurrent.
- For reporting LoadImage() and StartImage() preparations, replace the
originally proposed PcdDebugCodeOsLoaderDetail status code with the
existent (edk2-specific) PcdProgressCodeOsLoaderLoad and
PcdProgressCodeOsLoaderStart status codes.
- For reporting LoadImage() and StartImage() return values, replace
the originally proposed PcdDebugCodeOsLoaderDetail status code with
the standard EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR and
EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED status codes.
- For all four kinds of reports, replace the originally proposed "OS
Loader Detail" structure (and GUID) with the recently standardized
EFI_RETURN_STATUS_EXTENDED_DATA structure.
OvmfPkg/OvmfPkg.dec | 5 +
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/Include/Library/PlatformBmPrintScLib.h | 41 +++
OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf | 66 +++++
OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c | 310 ++++++++++++++++++++
7 files changed, 425 insertions(+)
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 7666297cf8f1..e50c6179a249 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -45,6 +45,11 @@ [LibraryClasses]
# access.
PciCapPciSegmentLib|Include/Library/PciCapPciSegmentLib.h
+ ## @libraryclass Register a status code handler for printing the Boot
+ # Manager's LoadImage() and StartImage() preparations, and
+ # return codes, to the UEFI console.
+ PlatformBmPrintScLib|Include/Library/PlatformBmPrintScLib.h
+
## @libraryclass Access QEMU's firmware configuration interface
#
QemuFwCfgLib|Include/Library/QemuFwCfgLib.h
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index f9216af479f4..5b885590b275 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -348,6 +348,7 @@ [LibraryClasses.common.DXE_DRIVER]
UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+ PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
!if $(SMM_REQUIRE) == TRUE
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 1e470de74434..bbf0853ee6b9 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -353,6 +353,7 @@ [LibraryClasses.common.DXE_DRIVER]
UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+ PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
!if $(SMM_REQUIRE) == TRUE
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index e4929d8cf4a8..d81460f52041 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -353,6 +353,7 @@ [LibraryClasses.common.DXE_DRIVER]
UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+ PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
!if $(SMM_REQUIRE) == TRUE
diff --git a/OvmfPkg/Include/Library/PlatformBmPrintScLib.h b/OvmfPkg/Include/Library/PlatformBmPrintScLib.h
new file mode 100644
index 000000000000..1777f9d7c947
--- /dev/null
+++ b/OvmfPkg/Include/Library/PlatformBmPrintScLib.h
@@ -0,0 +1,41 @@
+/** @file
+ Register a status code handler for printing the Boot Manager's LoadImage()
+ and StartImage() preparations, and return codes, to the UEFI console.
+
+ This feature enables users that are not accustomed to analyzing the firmware
+ log to glean some information about UEFI boot option processing (loading and
+ starting).
+
+ Copyright (C) 2019, Red Hat, Inc.
+
+ This program and the accompanying materials are licensed and made available
+ under the terms and conditions of the BSD License which accompanies this
+ distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+ WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+**/
+
+#ifndef __PLATFORM_BM_PRINT_SC_LIB__
+#define __PLATFORM_BM_PRINT_SC_LIB__
+
+#include <Uefi/UefiBaseType.h>
+
+/**
+ Register a status code handler for printing the Boot Manager's LoadImage()
+ and StartImage() preparations, and return codes, to the UEFI console.
+
+ @retval EFI_SUCCESS The status code handler has been successfully
+ registered.
+
+ @return Error codes propagated from boot services and from
+ EFI_RSC_HANDLER_PROTOCOL.
+**/
+EFI_STATUS
+EFIAPI
+PlatformBmPrintScRegisterHandler (
+ VOID
+ );
+
+#endif // __PLATFORM_BM_PRINT_SC_LIB__
diff --git a/OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf b/OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
new file mode 100644
index 000000000000..8f02e0b48236
--- /dev/null
+++ b/OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
@@ -0,0 +1,66 @@
+## @file
+# Register a status code handler for printing the Boot Manager's LoadImage()
+# and StartImage() preparations, and return codes, to the UEFI console.
+#
+# This feature enables users that are not accustomed to analyzing the firmware
+# log to glean some information about UEFI boot option processing (loading and
+# starting).
+#
+# This library instance filters out (ignores) status codes that are not
+# reported by the containing firmware module. The intent is to link this
+# library instance into BdsDxe via PlatformBootManagerLib (which BdsDxe depends
+# upon), then catch only those status codes that BdsDxe reports (which happens
+# via UefiBootManagerLib). Status codes reported by other modules (such as
+# UiApp), via UefiBootManagerLib or otherwise, are meant to be ignored.
+#
+# Copyright (C) 2019, Red Hat, Inc.
+#
+# This program and the accompanying materials are licensed and made available
+# under the terms and conditions of the BSD License which accompanies this
+# distribution. The full text of the license may be found at
+# http://opensource.org/licenses/bsd-license.php
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+##
+
+[Defines]
+ INF_VERSION = 1.27
+ BASE_NAME = PlatformBmPrintScLib
+ FILE_GUID = 3417c705-903e-41a7-9485-3fafebf60917
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = PlatformBmPrintScLib|DXE_DRIVER
+
+[Sources]
+ StatusCodeHandler.c
+
+[Packages]
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ BaseMemoryLib
+ DebugLib
+ DevicePathLib
+ MemoryAllocationLib
+ PcdLib
+ PrintLib
+ UefiBootManagerLib
+ UefiBootServicesTableLib
+ UefiLib
+ UefiRuntimeServicesTableLib
+
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdProgressCodeOsLoaderLoad ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdProgressCodeOsLoaderStart ## CONSUMES
+
+[Protocols]
+ gEfiRscHandlerProtocolGuid ## CONSUMES
+
+[Guids]
+ gEfiGlobalVariableGuid ## CONSUMES
+ gEfiStatusCodeSpecificDataGuid ## CONSUMES
+
+[Depex.common.DXE_DRIVER]
+ gEfiRscHandlerProtocolGuid AND gEfiVariableArchProtocolGuid
diff --git a/OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c b/OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c
new file mode 100644
index 000000000000..9806d3c7411e
--- /dev/null
+++ b/OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c
@@ -0,0 +1,310 @@
+/** @file
+ Register a status code handler for printing the Boot Manager's LoadImage()
+ and StartImage() preparations, and return codes, to the UEFI console.
+
+ This feature enables users that are not accustomed to analyzing the firmware
+ log to glean some information about UEFI boot option processing (loading and
+ starting).
+
+ This library instance filters out (ignores) status codes that are not
+ reported by the containing firmware module. The intent is to link this
+ library instance into BdsDxe via PlatformBootManagerLib (which BdsDxe depends
+ upon), then catch only those status codes that BdsDxe reports (which happens
+ via UefiBootManagerLib). Status codes reported by other modules (such as
+ UiApp), via UefiBootManagerLib or otherwise, are meant to be ignored.
+
+ Copyright (C) 2019, Red Hat, Inc.
+
+ This program and the accompanying materials are licensed and made available
+ under the terms and conditions of the BSD License which accompanies this
+ distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+ WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+**/
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DevicePathLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PrintLib.h>
+#include <Library/UefiBootManagerLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+
+#include <Protocol/ReportStatusCodeHandler.h>
+
+#include <Guid/GlobalVariable.h>
+#include <Guid/StatusCodeDataTypeId.h>
+
+#include <Pi/PiStatusCode.h>
+
+
+//
+// Convenience variables for the status codes that are relevant for LoadImage()
+// and StartImage() preparations and return codes.
+//
+STATIC EFI_STATUS_CODE_VALUE mLoadPrep;
+STATIC EFI_STATUS_CODE_VALUE mLoadFail;
+STATIC EFI_STATUS_CODE_VALUE mStartPrep;
+STATIC EFI_STATUS_CODE_VALUE mStartFail;
+
+
+/**
+ Handle status codes reported through ReportStatusCodeLib /
+ EFI_STATUS_CODE_PROTOCOL.ReportStatusCode(). Format matching status codes to
+ the system console.
+
+ The highest TPL at which this handler can be registered with
+ EFI_RSC_HANDLER_PROTOCOL.Register() is TPL_NOTIFY. That's because Print()
+ uses EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL internally.
+
+ The parameter list of this function precisely matches that of
+ EFI_STATUS_CODE_PROTOCOL.ReportStatusCode().
+
+ The return status of this function is ignored by the caller, but the function
+ still returns sensible codes:
+
+ @retval EFI_SUCCESS The status code has been processed; either
+ as a no-op, due to filtering, or by
+ formatting it to the system console.
+
+ @retval EFI_INVALID_PARAMETER Unknown or malformed contents have been
+ detected in Data.
+
+ @retval EFI_INCOMPATIBLE_VERSION Unexpected UEFI variable behavior has been
+ encountered.
+
+ @return Error codes propagated from underlying
+ services.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+HandleStatusCode (
+ IN EFI_STATUS_CODE_TYPE CodeType,
+ IN EFI_STATUS_CODE_VALUE Value,
+ IN UINT32 Instance,
+ IN EFI_GUID *CallerId,
+ IN EFI_STATUS_CODE_DATA *Data
+ )
+{
+ UINTN VariableSize;
+ UINT16 BootCurrent;
+ EFI_STATUS Status;
+ CHAR16 BootOptionName[ARRAY_SIZE (L"Boot####")];
+ EFI_BOOT_MANAGER_LOAD_OPTION BmBootOption;
+ BOOLEAN DevPathStringIsDynamic;
+ CHAR16 *DevPathString;
+
+ //
+ // Ignore all status codes that are irrelevant for LoadImage() and
+ // StartImage() preparations and return codes.
+ //
+ if (Value != mLoadPrep && Value != mLoadFail &&
+ Value != mStartPrep && Value != mStartFail) {
+ return EFI_SUCCESS;
+ }
+ //
+ // Ignore status codes that are not reported by the same containing module.
+ //
+ if (!CompareGuid (CallerId, &gEfiCallerIdGuid)) {
+ return EFI_SUCCESS;
+ }
+
+ //
+ // Sanity-check Data in case of failure reports.
+ //
+ if ((Value == mLoadFail || Value == mStartFail) &&
+ (Data == NULL ||
+ Data->HeaderSize != sizeof *Data ||
+ Data->Size != sizeof (EFI_RETURN_STATUS_EXTENDED_DATA) - sizeof *Data ||
+ !CompareGuid (&Data->Type, &gEfiStatusCodeSpecificDataGuid))) {
+ DEBUG ((DEBUG_ERROR, "%a:%a: malformed Data\n", gEfiCallerBaseName,
+ __FUNCTION__));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Get the number of the Boot#### option that the status code applies to.
+ //
+ VariableSize = sizeof BootCurrent;
+ Status = gRT->GetVariable (EFI_BOOT_CURRENT_VARIABLE_NAME,
+ &gEfiGlobalVariableGuid, NULL /* Attributes */,
+ &VariableSize, &BootCurrent);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a:%a: failed to get %g:\"%s\": %r\n",
+ gEfiCallerBaseName, __FUNCTION__, &gEfiGlobalVariableGuid,
+ EFI_BOOT_CURRENT_VARIABLE_NAME, Status));
+ return Status;
+ }
+ if (VariableSize != sizeof BootCurrent) {
+ DEBUG ((DEBUG_ERROR, "%a:%a: got %Lu bytes for %g:\"%s\", expected %Lu\n",
+ gEfiCallerBaseName, __FUNCTION__, (UINT64)VariableSize,
+ &gEfiGlobalVariableGuid, EFI_BOOT_CURRENT_VARIABLE_NAME,
+ (UINT64)sizeof BootCurrent));
+ return EFI_INCOMPATIBLE_VERSION;
+ }
+
+ //
+ // Get the Boot#### option that the status code applies to.
+ //
+ UnicodeSPrint (BootOptionName, sizeof BootOptionName, L"Boot%04x",
+ BootCurrent);
+ Status = EfiBootManagerVariableToLoadOption (BootOptionName, &BmBootOption);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR,
+ "%a:%a: EfiBootManagerVariableToLoadOption(\"%s\"): %r\n",
+ gEfiCallerBaseName, __FUNCTION__, BootOptionName, Status));
+ return Status;
+ }
+
+ //
+ // Format the device path.
+ //
+ DevPathStringIsDynamic = TRUE;
+ DevPathString = ConvertDevicePathToText (
+ BmBootOption.FilePath,
+ FALSE, // DisplayOnly
+ FALSE // AllowShortcuts
+ );
+ if (DevPathString == NULL) {
+ DevPathStringIsDynamic = FALSE;
+ DevPathString = L"<out of memory while formatting device path>";
+ }
+
+ //
+ // Print the message to the console.
+ //
+ if (Value == mLoadPrep || Value == mStartPrep) {
+ Print (
+ L"%a: %a %s \"%s\" from %s\n",
+ gEfiCallerBaseName,
+ Value == mLoadPrep ? "loading" : "starting",
+ BootOptionName,
+ BmBootOption.Description,
+ DevPathString
+ );
+ } else {
+ Print (
+ L"%a: failed to %a %s \"%s\" from %s: %r\n",
+ gEfiCallerBaseName,
+ Value == mLoadFail ? "load" : "start",
+ BootOptionName,
+ BmBootOption.Description,
+ DevPathString,
+ ((EFI_RETURN_STATUS_EXTENDED_DATA *)Data)->ReturnStatus
+ );
+ }
+
+ //
+ // Done.
+ //
+ if (DevPathStringIsDynamic) {
+ FreePool (DevPathString);
+ }
+ EfiBootManagerFreeLoadOption (&BmBootOption);
+ return EFI_SUCCESS;
+}
+
+
+/**
+ Unregister HandleStatusCode() at ExitBootServices().
+
+ (See EFI_RSC_HANDLER_PROTOCOL in Volume 3 of the Platform Init spec.)
+
+ @param[in] Event Event whose notification function is being invoked.
+
+ @param[in] Context Pointer to EFI_RSC_HANDLER_PROTOCOL, originally looked up
+ when HandleStatusCode() was registered.
+**/
+STATIC
+VOID
+EFIAPI
+UnregisterAtExitBootServices (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ EFI_RSC_HANDLER_PROTOCOL *StatusCodeRouter;
+
+ StatusCodeRouter = Context;
+ StatusCodeRouter->Unregister (HandleStatusCode);
+}
+
+
+/**
+ Register a status code handler for printing the Boot Manager's LoadImage()
+ and StartImage() preparations, and return codes, to the UEFI console.
+
+ @retval EFI_SUCCESS The status code handler has been successfully
+ registered.
+
+ @return Error codes propagated from boot services and from
+ EFI_RSC_HANDLER_PROTOCOL.
+**/
+EFI_STATUS
+EFIAPI
+PlatformBmPrintScRegisterHandler (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ EFI_RSC_HANDLER_PROTOCOL *StatusCodeRouter;
+ EFI_EVENT ExitBootEvent;
+
+ Status = gBS->LocateProtocol (&gEfiRscHandlerProtocolGuid,
+ NULL /* Registration */, (VOID **)&StatusCodeRouter);
+ ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ //
+ // Set the EFI_STATUS_CODE_VALUE convenience variables.
+ //
+ mLoadPrep = PcdGet32 (PcdProgressCodeOsLoaderLoad);
+ mLoadFail = (EFI_SOFTWARE_DXE_BS_DRIVER |
+ EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR);
+ mStartPrep = PcdGet32 (PcdProgressCodeOsLoaderStart);
+ mStartFail = (EFI_SOFTWARE_DXE_BS_DRIVER |
+ EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED);
+
+ //
+ // Register the handler callback.
+ //
+ Status = StatusCodeRouter->Register (HandleStatusCode, TPL_CALLBACK);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a:%a: failed to register status code handler: %r\n",
+ gEfiCallerBaseName, __FUNCTION__, Status));
+ return Status;
+ }
+
+ //
+ // Status code reporting and routing/handling extend into OS runtime. Since
+ // we don't want our handler to survive the BDS phase, we have to unregister
+ // the callback at ExitBootServices(). (See EFI_RSC_HANDLER_PROTOCOL in
+ // Volume 3 of the Platform Init spec.)
+ //
+ Status = gBS->CreateEvent (
+ EVT_SIGNAL_EXIT_BOOT_SERVICES, // Type
+ TPL_CALLBACK, // NotifyTpl
+ UnregisterAtExitBootServices, // NotifyFunction
+ StatusCodeRouter, // NotifyContext
+ &ExitBootEvent // Event
+ );
+ if (EFI_ERROR (Status)) {
+ //
+ // We have to unregister the callback right now, and fail the function.
+ //
+ DEBUG ((DEBUG_ERROR, "%a:%a: failed to create ExitBootServices() event: "
+ "%r\n", gEfiCallerBaseName, __FUNCTION__, Status));
+ StatusCodeRouter->Unregister (HandleStatusCode);
+ return Status;
+ }
+
+ return EFI_SUCCESS;
+}
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/5] OvmfPkg/PlatformBootManagerLib: display boot option loading/starting
2019-02-20 8:16 [PATCH v2 0/5] MdeModulePkg, OvmfPkg, ArmVirtPkg: more visible boot progress reporting Laszlo Ersek
2019-02-20 8:16 ` [PATCH v2 1/5] MdeModulePkg/UefiBootManagerLib: fix LoadImage/StartImage status code rep Laszlo Ersek
2019-02-20 8:16 ` [PATCH v2 2/5] OvmfPkg: add library to track boot option loading/starting on the console Laszlo Ersek
@ 2019-02-20 8:16 ` Laszlo Ersek
2019-02-20 8:16 ` [PATCH v2 4/5] ArmVirtPkg/ArmVirtQemu*: enable minimal Status Code Routing in DXE Laszlo Ersek
2019-02-20 8:16 ` [PATCH v2 5/5] ArmVirtPkg/PlatformBootManagerLib: display boot option loading/starting Laszlo Ersek
4 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2019-02-20 8:16 UTC (permalink / raw)
To: edk2-devel
Cc: Anthony Perard, Ard Biesheuvel, Jordan Justen, Julien Grall,
Ray Ni
Consume PlatformBmPrintScLib, added earlier in this series. When
BdsDxe+UefiBootManagerLib report LoadImage() / StartImage() preparations
and return statuses, print the reports to the UEFI console. This allows
end-users better visibility into the boot process.
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@linaro.org>
Cc: Ray Ni <ray.ni@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1515418
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- Split the OvmfPkg/PlatformBootManagerLib change to a separate patch.
- Drop general messages from PlatformBootManagerAfterConsole(), which
would report calls of EfiBootManagerRefreshAllBootOption(),
RemoveStaleFvFileOptions(), and SetBootOrderFromQemu(). Those messages
weren't really helpful for diagnosing boot problems.
OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 +
OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 5d9b53bf2d1e..c41aaeef3fc3 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -58,6 +58,7 @@ [LibraryClasses]
QemuBootOrderLib
ReportStatusCodeLib
UefiLib
+ PlatformBmPrintScLib
Tcg2PhysicalPresenceLib
[Pcd]
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
index b2faa797c61b..12303fb0f1f3 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
@@ -16,6 +16,7 @@
#include <Guid/XenInfo.h>
#include <Guid/RootBridgesConnectedEventGroup.h>
#include <Protocol/FirmwareVolume2.h>
+#include <Library/PlatformBmPrintScLib.h>
#include <Library/Tcg2PhysicalPresenceLib.h>
@@ -1542,6 +1543,8 @@ PlatformBootManagerAfterConsole (
RemoveStaleFvFileOptions ();
SetBootOrderFromQemu ();
+
+ PlatformBmPrintScRegisterHandler ();
}
/**
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 4/5] ArmVirtPkg/ArmVirtQemu*: enable minimal Status Code Routing in DXE
2019-02-20 8:16 [PATCH v2 0/5] MdeModulePkg, OvmfPkg, ArmVirtPkg: more visible boot progress reporting Laszlo Ersek
` (2 preceding siblings ...)
2019-02-20 8:16 ` [PATCH v2 3/5] OvmfPkg/PlatformBootManagerLib: display boot option loading/starting Laszlo Ersek
@ 2019-02-20 8:16 ` Laszlo Ersek
2019-02-20 8:16 ` [PATCH v2 5/5] ArmVirtPkg/PlatformBootManagerLib: display boot option loading/starting Laszlo Ersek
4 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2019-02-20 8:16 UTC (permalink / raw)
To: edk2-devel
The EFI_RETURN_STATUS_EXTENDED_DATA feature from PI-1.7
(<https://mantis.uefi.org/mantis/view.php?id=1885>) enables platform code
to learn about boot option failures (loading and launching) via status
codes reported by the UEFI Boot Manager.
In commit 59541d41633c, we removed all status code support from
ArmVirtPkg. Reenable that support now, minimally, just to the extent so we
can benefit from the PI-1.7 feature mentioned above:
(1) Include the ReportStatusCodeRouterRuntimeDxe driver.
This driver produces two protocols, EFI_STATUS_CODE_PROTOCOL and
EFI_RSC_HANDLER_PROTOCOL. The former allows DXE phase modules and
runtime modules to report (produce) status codes. The latter allows
the same types of modules to register callbacks for status code
handling (consumption).
(Handler registration occurs only at boot time. Status codes are
delivered to each handler at runtime as well, unless the handler is
unregistered at ExitBootServices().)
(2) Resolve ReportStatusCodeLib to a non-Null instance, for DXE_DRIVER
modules only. This way DXE_DRIVER modules that use the
REPORT_STATUS_CODE_EX() macro and friends will reach
EFI_STATUS_CODE_PROTOCOL from point (1).
(3) Set PcdReportStatusCodePropertyMask to 3 (the default value is 0).
This causes the REPORT_STATUS_CODE_EX() macro and friends to let
Progress Codes (bit#0) and Error Codes (bit#1) through to point (1).
Debug Codes (bit#2) are filtered out.
(4) Include no driver, for now, that registers any status code handler via
EFI_RSC_HANDLER_PROTOCOL, from point (1). Status codes that reach
ReportStatusCodeRouterRuntimeDxe will be thrown away.
(5) Modify only the ArmVirtQemu* platforms. A status code handler will
be added to "ArmVirtPkg/Library/PlatformBootManagerLib" in the next patch,
and this library instance is not consumed by ArmVirtXen.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Julien Grall <julien.grall@linaro.org>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1515418
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- new in v2
ArmVirtPkg/ArmVirtQemu.dsc | 10 ++++++++++
ArmVirtPkg/ArmVirtQemuKernel.dsc | 10 ++++++++++
ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 5 +++++
3 files changed, 25 insertions(+)
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 8cc31fda7a37..ec97d5a14b3a 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -69,6 +69,9 @@ [LibraryClasses.common]
[LibraryClasses.common.PEIM]
ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
+[LibraryClasses.common.DXE_DRIVER]
+ ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
+
[LibraryClasses.common.UEFI_DRIVER]
UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
@@ -155,6 +158,8 @@ [PcdsFixedAtBuild.common]
gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
!endif
+ gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|3
+
[PcdsFixedAtBuild.AARCH64]
# Clearing BIT0 in this PCD prevents installing a 32-bit SMBIOS entry point,
# if the entry point version is >= 3.0. AARCH64 OSes cannot assume the
@@ -304,6 +309,11 @@ [Components.common]
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
+ #
+ # Status Code Routing
+ #
+ MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportStatusCodeRouterRuntimeDxe.inf
+
#
# Platform Driver
#
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index c3e0c9bf2549..e8c065229b21 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -66,6 +66,9 @@ [LibraryClasses.common]
PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
+[LibraryClasses.common.DXE_DRIVER]
+ ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
+
[LibraryClasses.common.UEFI_DRIVER]
UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
@@ -149,6 +152,8 @@ [PcdsFixedAtBuild.common]
gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
!endif
+ gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|3
+
[PcdsPatchableInModule.common]
#
# This will be overridden in the code
@@ -288,6 +293,11 @@ [Components.common]
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
+ #
+ # Status Code Routing
+ #
+ MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportStatusCodeRouterRuntimeDxe.inf
+
#
# Platform Driver
#
diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
index dc6a0a2bcdc7..098d40b61b15 100644
--- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
+++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
@@ -91,6 +91,11 @@ [FV.FvMain]
INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
INF MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
+ #
+ # Status Code Routing
+ #
+ INF MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportStatusCodeRouterRuntimeDxe.inf
+
#
# Platform Driver
#
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 5/5] ArmVirtPkg/PlatformBootManagerLib: display boot option loading/starting
2019-02-20 8:16 [PATCH v2 0/5] MdeModulePkg, OvmfPkg, ArmVirtPkg: more visible boot progress reporting Laszlo Ersek
` (3 preceding siblings ...)
2019-02-20 8:16 ` [PATCH v2 4/5] ArmVirtPkg/ArmVirtQemu*: enable minimal Status Code Routing in DXE Laszlo Ersek
@ 2019-02-20 8:16 ` Laszlo Ersek
4 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2019-02-20 8:16 UTC (permalink / raw)
To: edk2-devel
Consume PlatformBmPrintScLib, added earlier in this series. When
BdsDxe+UefiBootManagerLib report LoadImage() / StartImage() preparations
and return statuses, print the reports to the UEFI console. This allows
end-users better visibility into the boot process.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1515418
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- new in v2
ArmVirtPkg/ArmVirtQemu.dsc | 1 +
ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 +
ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 +
ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 3 +++
4 files changed, 6 insertions(+)
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index ec97d5a14b3a..a77d71bcea36 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -58,6 +58,7 @@ [LibraryClasses.common]
CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+ PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index e8c065229b21..1e5388ae708f 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -58,6 +58,7 @@ [LibraryClasses.common]
CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+ PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 0cbc82f5d27d..9a8a70379e67 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -50,6 +50,7 @@ [LibraryClasses]
DevicePathLib
MemoryAllocationLib
PcdLib
+ PlatformBmPrintScLib
PrintLib
QemuBootOrderLib
QemuFwCfgLib
diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
index 534357eff46b..0e8d7501fb1d 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -20,6 +20,7 @@
#include <Library/BootLogoLib.h>
#include <Library/DevicePathLib.h>
#include <Library/PcdLib.h>
+#include <Library/PlatformBmPrintScLib.h>
#include <Library/QemuBootOrderLib.h>
#include <Library/UefiBootManagerLib.h>
#include <Protocol/DevicePath.h>
@@ -833,6 +834,8 @@ PlatformBootManagerAfterConsole (
RemoveStaleFvFileOptions ();
SetBootOrderFromQemu ();
+
+ PlatformBmPrintScRegisterHandler ();
}
/**
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/5] OvmfPkg: add library to track boot option loading/starting on the console
2019-02-20 8:16 ` [PATCH v2 2/5] OvmfPkg: add library to track boot option loading/starting on the console Laszlo Ersek
@ 2019-02-20 9:21 ` Ard Biesheuvel
2019-02-20 12:01 ` Laszlo Ersek
2019-02-20 10:04 ` Laszlo Ersek
1 sibling, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2019-02-20 9:21 UTC (permalink / raw)
To: Laszlo Ersek
Cc: edk2-devel@lists.01.org, Anthony Perard, Jordan Justen,
Julien Grall, Ray Ni
On Wed, 20 Feb 2019 at 09:16, Laszlo Ersek <lersek@redhat.com> wrote:
>
> Introduce the Platform Boot Manager Print Status Code Library (for short,
> PlatformBmPrintScLib) class for catching and printing the LoadImage() /
> StartImage() preparations, and return statuses, that are reported by
> UefiBootManagerLib.
>
> In the primary library instance, catch only such status codes that
> UefiBootManagerLib reports from the same module that contains
> PlatformBmPrintScLib. The intent is to establish a reporting-printing
> channel within BdsDxe, between UefiBootManagerLib and
> PlatformBmPrintScLib. Ignore status codes originating elsewhence, e.g.
> from UiApp's copy of UefiBootManagerLib.
>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Julien Grall <julien.grall@linaro.org>
> Cc: Ray Ni <ray.ni@intel.com>
> Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1515418
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> v2:
>
> - Split the status code handling to a separate library, so that it's
> easy to reuse in ArmVirtPkg.
>
> - Rework the logic based on
> <https://bugzilla.tianocore.org/show_bug.cgi?id=1398> and
> <https://mantis.uefi.org/mantis/view.php?id=1885>, and follow Ray's
> advice in
> <http://mid.mail-archive.com/734D49CCEBEEF84792F5B80ED585239D5BACE29B@SHSMSX104.ccr.corp.intel.com>:
>
> - The boot option details are fetched via BootCurrent.
>
> - For reporting LoadImage() and StartImage() preparations, replace the
> originally proposed PcdDebugCodeOsLoaderDetail status code with the
> existent (edk2-specific) PcdProgressCodeOsLoaderLoad and
> PcdProgressCodeOsLoaderStart status codes.
>
> - For reporting LoadImage() and StartImage() return values, replace
> the originally proposed PcdDebugCodeOsLoaderDetail status code with
> the standard EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR and
> EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED status codes.
>
> - For all four kinds of reports, replace the originally proposed "OS
> Loader Detail" structure (and GUID) with the recently standardized
> EFI_RETURN_STATUS_EXTENDED_DATA structure.
>
> OvmfPkg/OvmfPkg.dec | 5 +
> OvmfPkg/OvmfPkgIa32.dsc | 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> OvmfPkg/OvmfPkgX64.dsc | 1 +
> OvmfPkg/Include/Library/PlatformBmPrintScLib.h | 41 +++
> OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf | 66 +++++
> OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c | 310 ++++++++++++++++++++
> 7 files changed, 425 insertions(+)
>
...
> diff --git a/OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c b/OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c
> new file mode 100644
> index 000000000000..9806d3c7411e
> --- /dev/null
> +++ b/OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c
> @@ -0,0 +1,310 @@
> +/** @file
> + Register a status code handler for printing the Boot Manager's LoadImage()
> + and StartImage() preparations, and return codes, to the UEFI console.
> +
> + This feature enables users that are not accustomed to analyzing the firmware
> + log to glean some information about UEFI boot option processing (loading and
> + starting).
> +
> + This library instance filters out (ignores) status codes that are not
> + reported by the containing firmware module. The intent is to link this
> + library instance into BdsDxe via PlatformBootManagerLib (which BdsDxe depends
> + upon), then catch only those status codes that BdsDxe reports (which happens
> + via UefiBootManagerLib). Status codes reported by other modules (such as
> + UiApp), via UefiBootManagerLib or otherwise, are meant to be ignored.
> +
> + Copyright (C) 2019, Red Hat, Inc.
> +
> + This program and the accompanying materials are licensed and made available
> + under the terms and conditions of the BSD License which accompanies this
> + distribution. The full text of the license may be found at
> + http://opensource.org/licenses/bsd-license.php
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +**/
> +
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/PrintLib.h>
> +#include <Library/UefiBootManagerLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +
> +#include <Protocol/ReportStatusCodeHandler.h>
> +
> +#include <Guid/GlobalVariable.h>
> +#include <Guid/StatusCodeDataTypeId.h>
> +
> +#include <Pi/PiStatusCode.h>
> +
> +
> +//
> +// Convenience variables for the status codes that are relevant for LoadImage()
> +// and StartImage() preparations and return codes.
> +//
> +STATIC EFI_STATUS_CODE_VALUE mLoadPrep;
> +STATIC EFI_STATUS_CODE_VALUE mLoadFail;
> +STATIC EFI_STATUS_CODE_VALUE mStartPrep;
> +STATIC EFI_STATUS_CODE_VALUE mStartFail;
> +
> +
> +/**
> + Handle status codes reported through ReportStatusCodeLib /
> + EFI_STATUS_CODE_PROTOCOL.ReportStatusCode(). Format matching status codes to
> + the system console.
> +
> + The highest TPL at which this handler can be registered with
> + EFI_RSC_HANDLER_PROTOCOL.Register() is TPL_NOTIFY. That's because Print()
> + uses EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL internally.
> +
> + The parameter list of this function precisely matches that of
> + EFI_STATUS_CODE_PROTOCOL.ReportStatusCode().
> +
> + The return status of this function is ignored by the caller, but the function
> + still returns sensible codes:
> +
> + @retval EFI_SUCCESS The status code has been processed; either
> + as a no-op, due to filtering, or by
> + formatting it to the system console.
> +
> + @retval EFI_INVALID_PARAMETER Unknown or malformed contents have been
> + detected in Data.
> +
> + @retval EFI_INCOMPATIBLE_VERSION Unexpected UEFI variable behavior has been
> + encountered.
> +
> + @return Error codes propagated from underlying
> + services.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +HandleStatusCode (
> + IN EFI_STATUS_CODE_TYPE CodeType,
> + IN EFI_STATUS_CODE_VALUE Value,
> + IN UINT32 Instance,
> + IN EFI_GUID *CallerId,
> + IN EFI_STATUS_CODE_DATA *Data
> + )
> +{
> + UINTN VariableSize;
> + UINT16 BootCurrent;
> + EFI_STATUS Status;
> + CHAR16 BootOptionName[ARRAY_SIZE (L"Boot####")];
> + EFI_BOOT_MANAGER_LOAD_OPTION BmBootOption;
> + BOOLEAN DevPathStringIsDynamic;
> + CHAR16 *DevPathString;
> +
> + //
> + // Ignore all status codes that are irrelevant for LoadImage() and
> + // StartImage() preparations and return codes.
> + //
> + if (Value != mLoadPrep && Value != mLoadFail &&
> + Value != mStartPrep && Value != mStartFail) {
> + return EFI_SUCCESS;
> + }
> + //
> + // Ignore status codes that are not reported by the same containing module.
> + //
> + if (!CompareGuid (CallerId, &gEfiCallerIdGuid)) {
> + return EFI_SUCCESS;
> + }
> +
> + //
> + // Sanity-check Data in case of failure reports.
> + //
> + if ((Value == mLoadFail || Value == mStartFail) &&
> + (Data == NULL ||
> + Data->HeaderSize != sizeof *Data ||
> + Data->Size != sizeof (EFI_RETURN_STATUS_EXTENDED_DATA) - sizeof *Data ||
> + !CompareGuid (&Data->Type, &gEfiStatusCodeSpecificDataGuid))) {
> + DEBUG ((DEBUG_ERROR, "%a:%a: malformed Data\n", gEfiCallerBaseName,
> + __FUNCTION__));
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + //
> + // Get the number of the Boot#### option that the status code applies to.
> + //
> + VariableSize = sizeof BootCurrent;
> + Status = gRT->GetVariable (EFI_BOOT_CURRENT_VARIABLE_NAME,
> + &gEfiGlobalVariableGuid, NULL /* Attributes */,
> + &VariableSize, &BootCurrent);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "%a:%a: failed to get %g:\"%s\": %r\n",
> + gEfiCallerBaseName, __FUNCTION__, &gEfiGlobalVariableGuid,
> + EFI_BOOT_CURRENT_VARIABLE_NAME, Status));
> + return Status;
> + }
> + if (VariableSize != sizeof BootCurrent) {
> + DEBUG ((DEBUG_ERROR, "%a:%a: got %Lu bytes for %g:\"%s\", expected %Lu\n",
> + gEfiCallerBaseName, __FUNCTION__, (UINT64)VariableSize,
> + &gEfiGlobalVariableGuid, EFI_BOOT_CURRENT_VARIABLE_NAME,
> + (UINT64)sizeof BootCurrent));
> + return EFI_INCOMPATIBLE_VERSION;
> + }
> +
> + //
> + // Get the Boot#### option that the status code applies to.
> + //
> + UnicodeSPrint (BootOptionName, sizeof BootOptionName, L"Boot%04x",
> + BootCurrent);
> + Status = EfiBootManagerVariableToLoadOption (BootOptionName, &BmBootOption);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR,
> + "%a:%a: EfiBootManagerVariableToLoadOption(\"%s\"): %r\n",
> + gEfiCallerBaseName, __FUNCTION__, BootOptionName, Status));
> + return Status;
> + }
> +
> + //
> + // Format the device path.
> + //
> + DevPathStringIsDynamic = TRUE;
> + DevPathString = ConvertDevicePathToText (
> + BmBootOption.FilePath,
> + FALSE, // DisplayOnly
> + FALSE // AllowShortcuts
> + );
> + if (DevPathString == NULL) {
> + DevPathStringIsDynamic = FALSE;
> + DevPathString = L"<out of memory while formatting device path>";
> + }
> +
> + //
> + // Print the message to the console.
> + //
> + if (Value == mLoadPrep || Value == mStartPrep) {
> + Print (
> + L"%a: %a %s \"%s\" from %s\n",
> + gEfiCallerBaseName,
> + Value == mLoadPrep ? "loading" : "starting",
> + BootOptionName,
> + BmBootOption.Description,
> + DevPathString
> + );
> + } else {
> + Print (
> + L"%a: failed to %a %s \"%s\" from %s: %r\n",
> + gEfiCallerBaseName,
> + Value == mLoadFail ? "load" : "start",
> + BootOptionName,
> + BmBootOption.Description,
> + DevPathString,
> + ((EFI_RETURN_STATUS_EXTENDED_DATA *)Data)->ReturnStatus
> + );
> + }
> +
> + //
> + // Done.
> + //
> + if (DevPathStringIsDynamic) {
> + FreePool (DevPathString);
> + }
> + EfiBootManagerFreeLoadOption (&BmBootOption);
> + return EFI_SUCCESS;
> +}
> +
> +
> +/**
> + Unregister HandleStatusCode() at ExitBootServices().
> +
> + (See EFI_RSC_HANDLER_PROTOCOL in Volume 3 of the Platform Init spec.)
> +
> + @param[in] Event Event whose notification function is being invoked.
> +
> + @param[in] Context Pointer to EFI_RSC_HANDLER_PROTOCOL, originally looked up
> + when HandleStatusCode() was registered.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +UnregisterAtExitBootServices (
> + IN EFI_EVENT Event,
> + IN VOID *Context
> + )
> +{
> + EFI_RSC_HANDLER_PROTOCOL *StatusCodeRouter;
> +
> + StatusCodeRouter = Context;
> + StatusCodeRouter->Unregister (HandleStatusCode);
> +}
> +
> +
> +/**
> + Register a status code handler for printing the Boot Manager's LoadImage()
> + and StartImage() preparations, and return codes, to the UEFI console.
> +
> + @retval EFI_SUCCESS The status code handler has been successfully
> + registered.
> +
> + @return Error codes propagated from boot services and from
> + EFI_RSC_HANDLER_PROTOCOL.
> +**/
> +EFI_STATUS
> +EFIAPI
> +PlatformBmPrintScRegisterHandler (
> + VOID
> + )
> +{
> + EFI_STATUS Status;
> + EFI_RSC_HANDLER_PROTOCOL *StatusCodeRouter;
> + EFI_EVENT ExitBootEvent;
> +
> + Status = gBS->LocateProtocol (&gEfiRscHandlerProtocolGuid,
> + NULL /* Registration */, (VOID **)&StatusCodeRouter);
> + ASSERT_EFI_ERROR (Status);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + //
> + // Set the EFI_STATUS_CODE_VALUE convenience variables.
> + //
> + mLoadPrep = PcdGet32 (PcdProgressCodeOsLoaderLoad);
> + mLoadFail = (EFI_SOFTWARE_DXE_BS_DRIVER |
> + EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR);
> + mStartPrep = PcdGet32 (PcdProgressCodeOsLoaderStart);
> + mStartFail = (EFI_SOFTWARE_DXE_BS_DRIVER |
> + EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED);
> +
This bit looks somewhat dodgy to me, but I suppose the asymmetry is
'prior art' from EDK2, no?
In any case, this looks good to me otherwise, so for the series
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> + //
> + // Register the handler callback.
> + //
> + Status = StatusCodeRouter->Register (HandleStatusCode, TPL_CALLBACK);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "%a:%a: failed to register status code handler: %r\n",
> + gEfiCallerBaseName, __FUNCTION__, Status));
> + return Status;
> + }
> +
> + //
> + // Status code reporting and routing/handling extend into OS runtime. Since
> + // we don't want our handler to survive the BDS phase, we have to unregister
> + // the callback at ExitBootServices(). (See EFI_RSC_HANDLER_PROTOCOL in
> + // Volume 3 of the Platform Init spec.)
> + //
> + Status = gBS->CreateEvent (
> + EVT_SIGNAL_EXIT_BOOT_SERVICES, // Type
> + TPL_CALLBACK, // NotifyTpl
> + UnregisterAtExitBootServices, // NotifyFunction
> + StatusCodeRouter, // NotifyContext
> + &ExitBootEvent // Event
> + );
> + if (EFI_ERROR (Status)) {
> + //
> + // We have to unregister the callback right now, and fail the function.
> + //
> + DEBUG ((DEBUG_ERROR, "%a:%a: failed to create ExitBootServices() event: "
> + "%r\n", gEfiCallerBaseName, __FUNCTION__, Status));
> + StatusCodeRouter->Unregister (HandleStatusCode);
> + return Status;
> + }
> +
> + return EFI_SUCCESS;
> +}
> --
> 2.19.1.3.g30247aa5d201
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/5] OvmfPkg: add library to track boot option loading/starting on the console
2019-02-20 8:16 ` [PATCH v2 2/5] OvmfPkg: add library to track boot option loading/starting on the console Laszlo Ersek
2019-02-20 9:21 ` Ard Biesheuvel
@ 2019-02-20 10:04 ` Laszlo Ersek
2019-02-20 10:06 ` Ard Biesheuvel
1 sibling, 1 reply; 12+ messages in thread
From: Laszlo Ersek @ 2019-02-20 10:04 UTC (permalink / raw)
To: edk2-devel; +Cc: Anthony Perard, Jordan Justen
On 02/20/19 09:16, Laszlo Ersek wrote:
> Introduce the Platform Boot Manager Print Status Code Library (for short,
> PlatformBmPrintScLib) class for catching and printing the LoadImage() /
> StartImage() preparations, and return statuses, that are reported by
> UefiBootManagerLib.
>
> In the primary library instance, catch only such status codes that
> UefiBootManagerLib reports from the same module that contains
> PlatformBmPrintScLib. The intent is to establish a reporting-printing
> channel within BdsDxe, between UefiBootManagerLib and
> PlatformBmPrintScLib. Ignore status codes originating elsewhence, e.g.
> from UiApp's copy of UefiBootManagerLib.
>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Julien Grall <julien.grall@linaro.org>
> Cc: Ray Ni <ray.ni@intel.com>
> Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1515418
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> v2:
>
> - Split the status code handling to a separate library, so that it's
> easy to reuse in ArmVirtPkg.
>
> - Rework the logic based on
> <https://bugzilla.tianocore.org/show_bug.cgi?id=1398> and
> <https://mantis.uefi.org/mantis/view.php?id=1885>, and follow Ray's
> advice in
> <http://mid.mail-archive.com/734D49CCEBEEF84792F5B80ED585239D5BACE29B@SHSMSX104.ccr.corp.intel.com>:
>
> - The boot option details are fetched via BootCurrent.
>
> - For reporting LoadImage() and StartImage() preparations, replace the
> originally proposed PcdDebugCodeOsLoaderDetail status code with the
> existent (edk2-specific) PcdProgressCodeOsLoaderLoad and
> PcdProgressCodeOsLoaderStart status codes.
>
> - For reporting LoadImage() and StartImage() return values, replace
> the originally proposed PcdDebugCodeOsLoaderDetail status code with
> the standard EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR and
> EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED status codes.
>
> - For all four kinds of reports, replace the originally proposed "OS
> Loader Detail" structure (and GUID) with the recently standardized
> EFI_RETURN_STATUS_EXTENDED_DATA structure.
>
> OvmfPkg/OvmfPkg.dec | 5 +
> OvmfPkg/OvmfPkgIa32.dsc | 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> OvmfPkg/OvmfPkgX64.dsc | 1 +
> OvmfPkg/Include/Library/PlatformBmPrintScLib.h | 41 +++
> OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf | 66 +++++
> OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c | 310 ++++++++++++++++++++
> 7 files changed, 425 insertions(+)
>
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 7666297cf8f1..e50c6179a249 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -45,6 +45,11 @@ [LibraryClasses]
> # access.
> PciCapPciSegmentLib|Include/Library/PciCapPciSegmentLib.h
>
> + ## @libraryclass Register a status code handler for printing the Boot
> + # Manager's LoadImage() and StartImage() preparations, and
> + # return codes, to the UEFI console.
> + PlatformBmPrintScLib|Include/Library/PlatformBmPrintScLib.h
> +
> ## @libraryclass Access QEMU's firmware configuration interface
> #
> QemuFwCfgLib|Include/Library/QemuFwCfgLib.h
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index f9216af479f4..5b885590b275 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -348,6 +348,7 @@ [LibraryClasses.common.DXE_DRIVER]
> UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
> DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
> PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> + PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
> QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
> CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
> !if $(SMM_REQUIRE) == TRUE
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 1e470de74434..bbf0853ee6b9 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -353,6 +353,7 @@ [LibraryClasses.common.DXE_DRIVER]
> UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
> DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
> PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> + PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
> QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
> CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
> !if $(SMM_REQUIRE) == TRUE
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index e4929d8cf4a8..d81460f52041 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -353,6 +353,7 @@ [LibraryClasses.common.DXE_DRIVER]
> UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
> DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
> PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> + PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
> QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
> CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
> !if $(SMM_REQUIRE) == TRUE
> diff --git a/OvmfPkg/Include/Library/PlatformBmPrintScLib.h b/OvmfPkg/Include/Library/PlatformBmPrintScLib.h
> new file mode 100644
> index 000000000000..1777f9d7c947
> --- /dev/null
> +++ b/OvmfPkg/Include/Library/PlatformBmPrintScLib.h
> @@ -0,0 +1,41 @@
> +/** @file
> + Register a status code handler for printing the Boot Manager's LoadImage()
> + and StartImage() preparations, and return codes, to the UEFI console.
> +
> + This feature enables users that are not accustomed to analyzing the firmware
> + log to glean some information about UEFI boot option processing (loading and
> + starting).
> +
> + Copyright (C) 2019, Red Hat, Inc.
> +
> + This program and the accompanying materials are licensed and made available
> + under the terms and conditions of the BSD License which accompanies this
> + distribution. The full text of the license may be found at
> + http://opensource.org/licenses/bsd-license.php
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +**/
> +
> +#ifndef __PLATFORM_BM_PRINT_SC_LIB__
> +#define __PLATFORM_BM_PRINT_SC_LIB__
> +
> +#include <Uefi/UefiBaseType.h>
> +
> +/**
> + Register a status code handler for printing the Boot Manager's LoadImage()
> + and StartImage() preparations, and return codes, to the UEFI console.
> +
> + @retval EFI_SUCCESS The status code handler has been successfully
> + registered.
> +
> + @return Error codes propagated from boot services and from
> + EFI_RSC_HANDLER_PROTOCOL.
> +**/
> +EFI_STATUS
> +EFIAPI
> +PlatformBmPrintScRegisterHandler (
> + VOID
> + );
> +
> +#endif // __PLATFORM_BM_PRINT_SC_LIB__
> diff --git a/OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf b/OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
> new file mode 100644
> index 000000000000..8f02e0b48236
> --- /dev/null
> +++ b/OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
> @@ -0,0 +1,66 @@
> +## @file
> +# Register a status code handler for printing the Boot Manager's LoadImage()
> +# and StartImage() preparations, and return codes, to the UEFI console.
> +#
> +# This feature enables users that are not accustomed to analyzing the firmware
> +# log to glean some information about UEFI boot option processing (loading and
> +# starting).
> +#
> +# This library instance filters out (ignores) status codes that are not
> +# reported by the containing firmware module. The intent is to link this
> +# library instance into BdsDxe via PlatformBootManagerLib (which BdsDxe depends
> +# upon), then catch only those status codes that BdsDxe reports (which happens
> +# via UefiBootManagerLib). Status codes reported by other modules (such as
> +# UiApp), via UefiBootManagerLib or otherwise, are meant to be ignored.
> +#
> +# Copyright (C) 2019, Red Hat, Inc.
> +#
> +# This program and the accompanying materials are licensed and made available
> +# under the terms and conditions of the BSD License which accompanies this
> +# distribution. The full text of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +##
> +
> +[Defines]
> + INF_VERSION = 1.27
> + BASE_NAME = PlatformBmPrintScLib
> + FILE_GUID = 3417c705-903e-41a7-9485-3fafebf60917
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = PlatformBmPrintScLib|DXE_DRIVER
> +
> +[Sources]
> + StatusCodeHandler.c
> +
> +[Packages]
> + MdeModulePkg/MdeModulePkg.dec
> + MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> + BaseMemoryLib
> + DebugLib
> + DevicePathLib
> + MemoryAllocationLib
> + PcdLib
> + PrintLib
> + UefiBootManagerLib
> + UefiBootServicesTableLib
> + UefiLib
> + UefiRuntimeServicesTableLib
> +
> +[Pcd]
> + gEfiMdeModulePkgTokenSpaceGuid.PcdProgressCodeOsLoaderLoad ## CONSUMES
> + gEfiMdeModulePkgTokenSpaceGuid.PcdProgressCodeOsLoaderStart ## CONSUMES
> +
> +[Protocols]
> + gEfiRscHandlerProtocolGuid ## CONSUMES
> +
> +[Guids]
> + gEfiGlobalVariableGuid ## CONSUMES
> + gEfiStatusCodeSpecificDataGuid ## CONSUMES
> +
> +[Depex.common.DXE_DRIVER]
> + gEfiRscHandlerProtocolGuid AND gEfiVariableArchProtocolGuid
> diff --git a/OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c b/OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c
> new file mode 100644
> index 000000000000..9806d3c7411e
> --- /dev/null
> +++ b/OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c
> @@ -0,0 +1,310 @@
> +/** @file
> + Register a status code handler for printing the Boot Manager's LoadImage()
> + and StartImage() preparations, and return codes, to the UEFI console.
> +
> + This feature enables users that are not accustomed to analyzing the firmware
> + log to glean some information about UEFI boot option processing (loading and
> + starting).
> +
> + This library instance filters out (ignores) status codes that are not
> + reported by the containing firmware module. The intent is to link this
> + library instance into BdsDxe via PlatformBootManagerLib (which BdsDxe depends
> + upon), then catch only those status codes that BdsDxe reports (which happens
> + via UefiBootManagerLib). Status codes reported by other modules (such as
> + UiApp), via UefiBootManagerLib or otherwise, are meant to be ignored.
> +
> + Copyright (C) 2019, Red Hat, Inc.
> +
> + This program and the accompanying materials are licensed and made available
> + under the terms and conditions of the BSD License which accompanies this
> + distribution. The full text of the license may be found at
> + http://opensource.org/licenses/bsd-license.php
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +**/
> +
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/PrintLib.h>
> +#include <Library/UefiBootManagerLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +
> +#include <Protocol/ReportStatusCodeHandler.h>
> +
> +#include <Guid/GlobalVariable.h>
> +#include <Guid/StatusCodeDataTypeId.h>
> +
> +#include <Pi/PiStatusCode.h>
> +
> +
> +//
> +// Convenience variables for the status codes that are relevant for LoadImage()
> +// and StartImage() preparations and return codes.
> +//
> +STATIC EFI_STATUS_CODE_VALUE mLoadPrep;
> +STATIC EFI_STATUS_CODE_VALUE mLoadFail;
> +STATIC EFI_STATUS_CODE_VALUE mStartPrep;
> +STATIC EFI_STATUS_CODE_VALUE mStartFail;
> +
> +
> +/**
> + Handle status codes reported through ReportStatusCodeLib /
> + EFI_STATUS_CODE_PROTOCOL.ReportStatusCode(). Format matching status codes to
> + the system console.
> +
> + The highest TPL at which this handler can be registered with
> + EFI_RSC_HANDLER_PROTOCOL.Register() is TPL_NOTIFY. That's because Print()
> + uses EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL internally.
Sorry, this is a stale comment from v1; I should have updated it. The
callback now uses the variable services, and those are restricted to <=
TPL_CALLBACK.
The code is correct (it does use TPL_CALLBACK), so if there's no other
change necessary, I could update the comment on push too. I can also
post v3 just for this, if that's preferred.
Thanks
Laszlo
> +
> + The parameter list of this function precisely matches that of
> + EFI_STATUS_CODE_PROTOCOL.ReportStatusCode().
> +
> + The return status of this function is ignored by the caller, but the function
> + still returns sensible codes:
> +
> + @retval EFI_SUCCESS The status code has been processed; either
> + as a no-op, due to filtering, or by
> + formatting it to the system console.
> +
> + @retval EFI_INVALID_PARAMETER Unknown or malformed contents have been
> + detected in Data.
> +
> + @retval EFI_INCOMPATIBLE_VERSION Unexpected UEFI variable behavior has been
> + encountered.
> +
> + @return Error codes propagated from underlying
> + services.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +HandleStatusCode (
> + IN EFI_STATUS_CODE_TYPE CodeType,
> + IN EFI_STATUS_CODE_VALUE Value,
> + IN UINT32 Instance,
> + IN EFI_GUID *CallerId,
> + IN EFI_STATUS_CODE_DATA *Data
> + )
> +{
> + UINTN VariableSize;
> + UINT16 BootCurrent;
> + EFI_STATUS Status;
> + CHAR16 BootOptionName[ARRAY_SIZE (L"Boot####")];
> + EFI_BOOT_MANAGER_LOAD_OPTION BmBootOption;
> + BOOLEAN DevPathStringIsDynamic;
> + CHAR16 *DevPathString;
> +
> + //
> + // Ignore all status codes that are irrelevant for LoadImage() and
> + // StartImage() preparations and return codes.
> + //
> + if (Value != mLoadPrep && Value != mLoadFail &&
> + Value != mStartPrep && Value != mStartFail) {
> + return EFI_SUCCESS;
> + }
> + //
> + // Ignore status codes that are not reported by the same containing module.
> + //
> + if (!CompareGuid (CallerId, &gEfiCallerIdGuid)) {
> + return EFI_SUCCESS;
> + }
> +
> + //
> + // Sanity-check Data in case of failure reports.
> + //
> + if ((Value == mLoadFail || Value == mStartFail) &&
> + (Data == NULL ||
> + Data->HeaderSize != sizeof *Data ||
> + Data->Size != sizeof (EFI_RETURN_STATUS_EXTENDED_DATA) - sizeof *Data ||
> + !CompareGuid (&Data->Type, &gEfiStatusCodeSpecificDataGuid))) {
> + DEBUG ((DEBUG_ERROR, "%a:%a: malformed Data\n", gEfiCallerBaseName,
> + __FUNCTION__));
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + //
> + // Get the number of the Boot#### option that the status code applies to.
> + //
> + VariableSize = sizeof BootCurrent;
> + Status = gRT->GetVariable (EFI_BOOT_CURRENT_VARIABLE_NAME,
> + &gEfiGlobalVariableGuid, NULL /* Attributes */,
> + &VariableSize, &BootCurrent);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "%a:%a: failed to get %g:\"%s\": %r\n",
> + gEfiCallerBaseName, __FUNCTION__, &gEfiGlobalVariableGuid,
> + EFI_BOOT_CURRENT_VARIABLE_NAME, Status));
> + return Status;
> + }
> + if (VariableSize != sizeof BootCurrent) {
> + DEBUG ((DEBUG_ERROR, "%a:%a: got %Lu bytes for %g:\"%s\", expected %Lu\n",
> + gEfiCallerBaseName, __FUNCTION__, (UINT64)VariableSize,
> + &gEfiGlobalVariableGuid, EFI_BOOT_CURRENT_VARIABLE_NAME,
> + (UINT64)sizeof BootCurrent));
> + return EFI_INCOMPATIBLE_VERSION;
> + }
> +
> + //
> + // Get the Boot#### option that the status code applies to.
> + //
> + UnicodeSPrint (BootOptionName, sizeof BootOptionName, L"Boot%04x",
> + BootCurrent);
> + Status = EfiBootManagerVariableToLoadOption (BootOptionName, &BmBootOption);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR,
> + "%a:%a: EfiBootManagerVariableToLoadOption(\"%s\"): %r\n",
> + gEfiCallerBaseName, __FUNCTION__, BootOptionName, Status));
> + return Status;
> + }
> +
> + //
> + // Format the device path.
> + //
> + DevPathStringIsDynamic = TRUE;
> + DevPathString = ConvertDevicePathToText (
> + BmBootOption.FilePath,
> + FALSE, // DisplayOnly
> + FALSE // AllowShortcuts
> + );
> + if (DevPathString == NULL) {
> + DevPathStringIsDynamic = FALSE;
> + DevPathString = L"<out of memory while formatting device path>";
> + }
> +
> + //
> + // Print the message to the console.
> + //
> + if (Value == mLoadPrep || Value == mStartPrep) {
> + Print (
> + L"%a: %a %s \"%s\" from %s\n",
> + gEfiCallerBaseName,
> + Value == mLoadPrep ? "loading" : "starting",
> + BootOptionName,
> + BmBootOption.Description,
> + DevPathString
> + );
> + } else {
> + Print (
> + L"%a: failed to %a %s \"%s\" from %s: %r\n",
> + gEfiCallerBaseName,
> + Value == mLoadFail ? "load" : "start",
> + BootOptionName,
> + BmBootOption.Description,
> + DevPathString,
> + ((EFI_RETURN_STATUS_EXTENDED_DATA *)Data)->ReturnStatus
> + );
> + }
> +
> + //
> + // Done.
> + //
> + if (DevPathStringIsDynamic) {
> + FreePool (DevPathString);
> + }
> + EfiBootManagerFreeLoadOption (&BmBootOption);
> + return EFI_SUCCESS;
> +}
> +
> +
> +/**
> + Unregister HandleStatusCode() at ExitBootServices().
> +
> + (See EFI_RSC_HANDLER_PROTOCOL in Volume 3 of the Platform Init spec.)
> +
> + @param[in] Event Event whose notification function is being invoked.
> +
> + @param[in] Context Pointer to EFI_RSC_HANDLER_PROTOCOL, originally looked up
> + when HandleStatusCode() was registered.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +UnregisterAtExitBootServices (
> + IN EFI_EVENT Event,
> + IN VOID *Context
> + )
> +{
> + EFI_RSC_HANDLER_PROTOCOL *StatusCodeRouter;
> +
> + StatusCodeRouter = Context;
> + StatusCodeRouter->Unregister (HandleStatusCode);
> +}
> +
> +
> +/**
> + Register a status code handler for printing the Boot Manager's LoadImage()
> + and StartImage() preparations, and return codes, to the UEFI console.
> +
> + @retval EFI_SUCCESS The status code handler has been successfully
> + registered.
> +
> + @return Error codes propagated from boot services and from
> + EFI_RSC_HANDLER_PROTOCOL.
> +**/
> +EFI_STATUS
> +EFIAPI
> +PlatformBmPrintScRegisterHandler (
> + VOID
> + )
> +{
> + EFI_STATUS Status;
> + EFI_RSC_HANDLER_PROTOCOL *StatusCodeRouter;
> + EFI_EVENT ExitBootEvent;
> +
> + Status = gBS->LocateProtocol (&gEfiRscHandlerProtocolGuid,
> + NULL /* Registration */, (VOID **)&StatusCodeRouter);
> + ASSERT_EFI_ERROR (Status);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + //
> + // Set the EFI_STATUS_CODE_VALUE convenience variables.
> + //
> + mLoadPrep = PcdGet32 (PcdProgressCodeOsLoaderLoad);
> + mLoadFail = (EFI_SOFTWARE_DXE_BS_DRIVER |
> + EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR);
> + mStartPrep = PcdGet32 (PcdProgressCodeOsLoaderStart);
> + mStartFail = (EFI_SOFTWARE_DXE_BS_DRIVER |
> + EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED);
> +
> + //
> + // Register the handler callback.
> + //
> + Status = StatusCodeRouter->Register (HandleStatusCode, TPL_CALLBACK);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "%a:%a: failed to register status code handler: %r\n",
> + gEfiCallerBaseName, __FUNCTION__, Status));
> + return Status;
> + }
> +
> + //
> + // Status code reporting and routing/handling extend into OS runtime. Since
> + // we don't want our handler to survive the BDS phase, we have to unregister
> + // the callback at ExitBootServices(). (See EFI_RSC_HANDLER_PROTOCOL in
> + // Volume 3 of the Platform Init spec.)
> + //
> + Status = gBS->CreateEvent (
> + EVT_SIGNAL_EXIT_BOOT_SERVICES, // Type
> + TPL_CALLBACK, // NotifyTpl
> + UnregisterAtExitBootServices, // NotifyFunction
> + StatusCodeRouter, // NotifyContext
> + &ExitBootEvent // Event
> + );
> + if (EFI_ERROR (Status)) {
> + //
> + // We have to unregister the callback right now, and fail the function.
> + //
> + DEBUG ((DEBUG_ERROR, "%a:%a: failed to create ExitBootServices() event: "
> + "%r\n", gEfiCallerBaseName, __FUNCTION__, Status));
> + StatusCodeRouter->Unregister (HandleStatusCode);
> + return Status;
> + }
> +
> + return EFI_SUCCESS;
> +}
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/5] OvmfPkg: add library to track boot option loading/starting on the console
2019-02-20 10:04 ` Laszlo Ersek
@ 2019-02-20 10:06 ` Ard Biesheuvel
0 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2019-02-20 10:06 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel@lists.01.org, Anthony Perard, Jordan Justen
On Wed, 20 Feb 2019 at 11:04, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 02/20/19 09:16, Laszlo Ersek wrote:
> > Introduce the Platform Boot Manager Print Status Code Library (for short,
> > PlatformBmPrintScLib) class for catching and printing the LoadImage() /
> > StartImage() preparations, and return statuses, that are reported by
> > UefiBootManagerLib.
> >
> > In the primary library instance, catch only such status codes that
> > UefiBootManagerLib reports from the same module that contains
> > PlatformBmPrintScLib. The intent is to establish a reporting-printing
> > channel within BdsDxe, between UefiBootManagerLib and
> > PlatformBmPrintScLib. Ignore status codes originating elsewhence, e.g.
> > from UiApp's copy of UefiBootManagerLib.
> >
> > Cc: Anthony Perard <anthony.perard@citrix.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Julien Grall <julien.grall@linaro.org>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1515418
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> > ---
> >
> > Notes:
> > v2:
> >
> > - Split the status code handling to a separate library, so that it's
> > easy to reuse in ArmVirtPkg.
> >
> > - Rework the logic based on
> > <https://bugzilla.tianocore.org/show_bug.cgi?id=1398> and
> > <https://mantis.uefi.org/mantis/view.php?id=1885>, and follow Ray's
> > advice in
> > <http://mid.mail-archive.com/734D49CCEBEEF84792F5B80ED585239D5BACE29B@SHSMSX104.ccr.corp.intel.com>:
> >
> > - The boot option details are fetched via BootCurrent.
> >
> > - For reporting LoadImage() and StartImage() preparations, replace the
> > originally proposed PcdDebugCodeOsLoaderDetail status code with the
> > existent (edk2-specific) PcdProgressCodeOsLoaderLoad and
> > PcdProgressCodeOsLoaderStart status codes.
> >
> > - For reporting LoadImage() and StartImage() return values, replace
> > the originally proposed PcdDebugCodeOsLoaderDetail status code with
> > the standard EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR and
> > EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED status codes.
> >
> > - For all four kinds of reports, replace the originally proposed "OS
> > Loader Detail" structure (and GUID) with the recently standardized
> > EFI_RETURN_STATUS_EXTENDED_DATA structure.
> >
> > OvmfPkg/OvmfPkg.dec | 5 +
> > OvmfPkg/OvmfPkgIa32.dsc | 1 +
> > OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> > OvmfPkg/OvmfPkgX64.dsc | 1 +
> > OvmfPkg/Include/Library/PlatformBmPrintScLib.h | 41 +++
> > OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf | 66 +++++
> > OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c | 310 ++++++++++++++++++++
> > 7 files changed, 425 insertions(+)
> >
> > diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> > index 7666297cf8f1..e50c6179a249 100644
> > --- a/OvmfPkg/OvmfPkg.dec
> > +++ b/OvmfPkg/OvmfPkg.dec
> > @@ -45,6 +45,11 @@ [LibraryClasses]
> > # access.
> > PciCapPciSegmentLib|Include/Library/PciCapPciSegmentLib.h
> >
> > + ## @libraryclass Register a status code handler for printing the Boot
> > + # Manager's LoadImage() and StartImage() preparations, and
> > + # return codes, to the UEFI console.
> > + PlatformBmPrintScLib|Include/Library/PlatformBmPrintScLib.h
> > +
> > ## @libraryclass Access QEMU's firmware configuration interface
> > #
> > QemuFwCfgLib|Include/Library/QemuFwCfgLib.h
> > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> > index f9216af479f4..5b885590b275 100644
> > --- a/OvmfPkg/OvmfPkgIa32.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32.dsc
> > @@ -348,6 +348,7 @@ [LibraryClasses.common.DXE_DRIVER]
> > UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
> > DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
> > PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> > + PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
> > QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
> > CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
> > !if $(SMM_REQUIRE) == TRUE
> > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> > index 1e470de74434..bbf0853ee6b9 100644
> > --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> > @@ -353,6 +353,7 @@ [LibraryClasses.common.DXE_DRIVER]
> > UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
> > DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
> > PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> > + PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
> > QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
> > CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
> > !if $(SMM_REQUIRE) == TRUE
> > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> > index e4929d8cf4a8..d81460f52041 100644
> > --- a/OvmfPkg/OvmfPkgX64.dsc
> > +++ b/OvmfPkg/OvmfPkgX64.dsc
> > @@ -353,6 +353,7 @@ [LibraryClasses.common.DXE_DRIVER]
> > UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
> > DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
> > PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> > + PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
> > QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
> > CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
> > !if $(SMM_REQUIRE) == TRUE
> > diff --git a/OvmfPkg/Include/Library/PlatformBmPrintScLib.h b/OvmfPkg/Include/Library/PlatformBmPrintScLib.h
> > new file mode 100644
> > index 000000000000..1777f9d7c947
> > --- /dev/null
> > +++ b/OvmfPkg/Include/Library/PlatformBmPrintScLib.h
> > @@ -0,0 +1,41 @@
> > +/** @file
> > + Register a status code handler for printing the Boot Manager's LoadImage()
> > + and StartImage() preparations, and return codes, to the UEFI console.
> > +
> > + This feature enables users that are not accustomed to analyzing the firmware
> > + log to glean some information about UEFI boot option processing (loading and
> > + starting).
> > +
> > + Copyright (C) 2019, Red Hat, Inc.
> > +
> > + This program and the accompanying materials are licensed and made available
> > + under the terms and conditions of the BSD License which accompanies this
> > + distribution. The full text of the license may be found at
> > + http://opensource.org/licenses/bsd-license.php
> > +
> > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> > + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +**/
> > +
> > +#ifndef __PLATFORM_BM_PRINT_SC_LIB__
> > +#define __PLATFORM_BM_PRINT_SC_LIB__
> > +
> > +#include <Uefi/UefiBaseType.h>
> > +
> > +/**
> > + Register a status code handler for printing the Boot Manager's LoadImage()
> > + and StartImage() preparations, and return codes, to the UEFI console.
> > +
> > + @retval EFI_SUCCESS The status code handler has been successfully
> > + registered.
> > +
> > + @return Error codes propagated from boot services and from
> > + EFI_RSC_HANDLER_PROTOCOL.
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +PlatformBmPrintScRegisterHandler (
> > + VOID
> > + );
> > +
> > +#endif // __PLATFORM_BM_PRINT_SC_LIB__
> > diff --git a/OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf b/OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
> > new file mode 100644
> > index 000000000000..8f02e0b48236
> > --- /dev/null
> > +++ b/OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
> > @@ -0,0 +1,66 @@
> > +## @file
> > +# Register a status code handler for printing the Boot Manager's LoadImage()
> > +# and StartImage() preparations, and return codes, to the UEFI console.
> > +#
> > +# This feature enables users that are not accustomed to analyzing the firmware
> > +# log to glean some information about UEFI boot option processing (loading and
> > +# starting).
> > +#
> > +# This library instance filters out (ignores) status codes that are not
> > +# reported by the containing firmware module. The intent is to link this
> > +# library instance into BdsDxe via PlatformBootManagerLib (which BdsDxe depends
> > +# upon), then catch only those status codes that BdsDxe reports (which happens
> > +# via UefiBootManagerLib). Status codes reported by other modules (such as
> > +# UiApp), via UefiBootManagerLib or otherwise, are meant to be ignored.
> > +#
> > +# Copyright (C) 2019, Red Hat, Inc.
> > +#
> > +# This program and the accompanying materials are licensed and made available
> > +# under the terms and conditions of the BSD License which accompanies this
> > +# distribution. The full text of the license may be found at
> > +# http://opensource.org/licenses/bsd-license.php
> > +#
> > +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> > +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +##
> > +
> > +[Defines]
> > + INF_VERSION = 1.27
> > + BASE_NAME = PlatformBmPrintScLib
> > + FILE_GUID = 3417c705-903e-41a7-9485-3fafebf60917
> > + MODULE_TYPE = DXE_DRIVER
> > + VERSION_STRING = 1.0
> > + LIBRARY_CLASS = PlatformBmPrintScLib|DXE_DRIVER
> > +
> > +[Sources]
> > + StatusCodeHandler.c
> > +
> > +[Packages]
> > + MdeModulePkg/MdeModulePkg.dec
> > + MdePkg/MdePkg.dec
> > +
> > +[LibraryClasses]
> > + BaseMemoryLib
> > + DebugLib
> > + DevicePathLib
> > + MemoryAllocationLib
> > + PcdLib
> > + PrintLib
> > + UefiBootManagerLib
> > + UefiBootServicesTableLib
> > + UefiLib
> > + UefiRuntimeServicesTableLib
> > +
> > +[Pcd]
> > + gEfiMdeModulePkgTokenSpaceGuid.PcdProgressCodeOsLoaderLoad ## CONSUMES
> > + gEfiMdeModulePkgTokenSpaceGuid.PcdProgressCodeOsLoaderStart ## CONSUMES
> > +
> > +[Protocols]
> > + gEfiRscHandlerProtocolGuid ## CONSUMES
> > +
> > +[Guids]
> > + gEfiGlobalVariableGuid ## CONSUMES
> > + gEfiStatusCodeSpecificDataGuid ## CONSUMES
> > +
> > +[Depex.common.DXE_DRIVER]
> > + gEfiRscHandlerProtocolGuid AND gEfiVariableArchProtocolGuid
> > diff --git a/OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c b/OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c
> > new file mode 100644
> > index 000000000000..9806d3c7411e
> > --- /dev/null
> > +++ b/OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c
> > @@ -0,0 +1,310 @@
> > +/** @file
> > + Register a status code handler for printing the Boot Manager's LoadImage()
> > + and StartImage() preparations, and return codes, to the UEFI console.
> > +
> > + This feature enables users that are not accustomed to analyzing the firmware
> > + log to glean some information about UEFI boot option processing (loading and
> > + starting).
> > +
> > + This library instance filters out (ignores) status codes that are not
> > + reported by the containing firmware module. The intent is to link this
> > + library instance into BdsDxe via PlatformBootManagerLib (which BdsDxe depends
> > + upon), then catch only those status codes that BdsDxe reports (which happens
> > + via UefiBootManagerLib). Status codes reported by other modules (such as
> > + UiApp), via UefiBootManagerLib or otherwise, are meant to be ignored.
> > +
> > + Copyright (C) 2019, Red Hat, Inc.
> > +
> > + This program and the accompanying materials are licensed and made available
> > + under the terms and conditions of the BSD License which accompanies this
> > + distribution. The full text of the license may be found at
> > + http://opensource.org/licenses/bsd-license.php
> > +
> > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> > + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +**/
> > +
> > +#include <Library/BaseMemoryLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/DevicePathLib.h>
> > +#include <Library/MemoryAllocationLib.h>
> > +#include <Library/PcdLib.h>
> > +#include <Library/PrintLib.h>
> > +#include <Library/UefiBootManagerLib.h>
> > +#include <Library/UefiBootServicesTableLib.h>
> > +#include <Library/UefiLib.h>
> > +#include <Library/UefiRuntimeServicesTableLib.h>
> > +
> > +#include <Protocol/ReportStatusCodeHandler.h>
> > +
> > +#include <Guid/GlobalVariable.h>
> > +#include <Guid/StatusCodeDataTypeId.h>
> > +
> > +#include <Pi/PiStatusCode.h>
> > +
> > +
> > +//
> > +// Convenience variables for the status codes that are relevant for LoadImage()
> > +// and StartImage() preparations and return codes.
> > +//
> > +STATIC EFI_STATUS_CODE_VALUE mLoadPrep;
> > +STATIC EFI_STATUS_CODE_VALUE mLoadFail;
> > +STATIC EFI_STATUS_CODE_VALUE mStartPrep;
> > +STATIC EFI_STATUS_CODE_VALUE mStartFail;
> > +
> > +
> > +/**
> > + Handle status codes reported through ReportStatusCodeLib /
> > + EFI_STATUS_CODE_PROTOCOL.ReportStatusCode(). Format matching status codes to
> > + the system console.
> > +
> > + The highest TPL at which this handler can be registered with
> > + EFI_RSC_HANDLER_PROTOCOL.Register() is TPL_NOTIFY. That's because Print()
> > + uses EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL internally.
>
> Sorry, this is a stale comment from v1; I should have updated it. The
> callback now uses the variable services, and those are restricted to <=
> TPL_CALLBACK.
>
> The code is correct (it does use TPL_CALLBACK), so if there's no other
> change necessary, I could update the comment on push too. I can also
> post v3 just for this, if that's preferred.
>
I don't mind folding in that change before applying.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/5] OvmfPkg: add library to track boot option loading/starting on the console
2019-02-20 9:21 ` Ard Biesheuvel
@ 2019-02-20 12:01 ` Laszlo Ersek
0 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2019-02-20 12:01 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org, Anthony Perard, Jordan Justen,
Julien Grall, Ray Ni
On 02/20/19 10:21, Ard Biesheuvel wrote:
> On Wed, 20 Feb 2019 at 09:16, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> Introduce the Platform Boot Manager Print Status Code Library (for short,
>> PlatformBmPrintScLib) class for catching and printing the LoadImage() /
>> StartImage() preparations, and return statuses, that are reported by
>> UefiBootManagerLib.
>>
>> In the primary library instance, catch only such status codes that
>> UefiBootManagerLib reports from the same module that contains
>> PlatformBmPrintScLib. The intent is to establish a reporting-printing
>> channel within BdsDxe, between UefiBootManagerLib and
>> PlatformBmPrintScLib. Ignore status codes originating elsewhence, e.g.
>> from UiApp's copy of UefiBootManagerLib.
>>
>> Cc: Anthony Perard <anthony.perard@citrix.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Julien Grall <julien.grall@linaro.org>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1515418
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>> v2:
>>
>> - Split the status code handling to a separate library, so that it's
>> easy to reuse in ArmVirtPkg.
>>
>> - Rework the logic based on
>> <https://bugzilla.tianocore.org/show_bug.cgi?id=1398> and
>> <https://mantis.uefi.org/mantis/view.php?id=1885>, and follow Ray's
>> advice in
>> <http://mid.mail-archive.com/734D49CCEBEEF84792F5B80ED585239D5BACE29B@SHSMSX104.ccr.corp.intel.com>:
>>
>> - The boot option details are fetched via BootCurrent.
>>
>> - For reporting LoadImage() and StartImage() preparations, replace the
>> originally proposed PcdDebugCodeOsLoaderDetail status code with the
>> existent (edk2-specific) PcdProgressCodeOsLoaderLoad and
>> PcdProgressCodeOsLoaderStart status codes.
>>
>> - For reporting LoadImage() and StartImage() return values, replace
>> the originally proposed PcdDebugCodeOsLoaderDetail status code with
>> the standard EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR and
>> EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED status codes.
>>
>> - For all four kinds of reports, replace the originally proposed "OS
>> Loader Detail" structure (and GUID) with the recently standardized
>> EFI_RETURN_STATUS_EXTENDED_DATA structure.
>>
>> OvmfPkg/OvmfPkg.dec | 5 +
>> OvmfPkg/OvmfPkgIa32.dsc | 1 +
>> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
>> OvmfPkg/OvmfPkgX64.dsc | 1 +
>> OvmfPkg/Include/Library/PlatformBmPrintScLib.h | 41 +++
>> OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf | 66 +++++
>> OvmfPkg/Library/PlatformBmPrintScLib/StatusCodeHandler.c | 310 ++++++++++++++++++++
>> 7 files changed, 425 insertions(+)
>>
[...]
>> + //
>> + // Set the EFI_STATUS_CODE_VALUE convenience variables.
>> + //
>> + mLoadPrep = PcdGet32 (PcdProgressCodeOsLoaderLoad);
>> + mLoadFail = (EFI_SOFTWARE_DXE_BS_DRIVER |
>> + EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR);
>> + mStartPrep = PcdGet32 (PcdProgressCodeOsLoaderStart);
>> + mStartFail = (EFI_SOFTWARE_DXE_BS_DRIVER |
>> + EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED);
>> +
>
> This bit looks somewhat dodgy to me, but I suppose the asymmetry is
> 'prior art' from EDK2, no?
Yes, that's the case. All four status code values are taken verbatim
from EfiBootManagerBoot()
[MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c], where they are
reported / produced.
I use module-global variables here because (a) I need no generality wrt.
status codes values in this module (I really only care for these four),
and (b) the original expressions are simply unbearably long; considering
the frequent use of these status code values in the patch.
Regarding the reporting in EfiBootManagerBoot(): the status code values
- (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR)
- (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED)
are from the PI spec. If we expand the macros a bit, we get,
respectively:
- EFI_SOFTWARE | 0x00050000 | EFI_SUBCLASS_SPECIFIC | 0x00000002
- EFI_SOFTWARE | 0x00050000 | EFI_SUBCLASS_SPECIFIC | 0x00000003
So we are in the "software class", the "DXE Boot Service Driver"
subclass, and we report values 2 and 3, which are meant to be unique
only within that subclass.
Conversely, the "prep" status code values are edk2 extensions. The PCDs
allow a platform, in theory anyway, to tweak the exact values. But in
practice, that should never be necessary. Let's check their default
values, in "MdeModulePkg/MdeModulePkg.dec":
> ## Progress Code for OS Loader LoadImage start.<BR><BR>
> # PROGRESS_CODE_OS_LOADER_LOAD = (EFI_SOFTWARE_DXE_BS_DRIVER | (EFI_OEM_SPECIFIC | 0x00000000)) = 0x03058000<BR>
> # @Prompt Progress Code for OS Loader LoadImage start.
> # @ValidList 0x80000003 | 0x03058000
> gEfiMdeModulePkgTokenSpaceGuid.PcdProgressCodeOsLoaderLoad|0x03058000|UINT32|0x30001030
>
> ## Progress Code for OS Loader StartImage start.<BR><BR>
> # PROGRESS_CODE_OS_LOADER_START = (EFI_SOFTWARE_DXE_BS_DRIVER | (EFI_OEM_SPECIFIC | 0x00000001)) = 0x03058001<BR>
> # @Prompt Progress Code for OS Loader StartImage start.
> # @ValidList 0x80000003 | 0x03058001
> gEfiMdeModulePkgTokenSpaceGuid.PcdProgressCodeOsLoaderStart|0x03058001|UINT32|0x30001031
Meaning
- EFI_SOFTWARE | 0x00050000 | EFI_OEM_SPECIFIC | 0x00000000
- EFI_SOFTWARE | 0x00050000 | EFI_OEM_SPECIFIC | 0x00000001
We stay within the same class & subclass, but use OEM-specific values 0
and 1, rather than standard values 2 and 3 that are specific to the
subclass.
I'd prefer if these weren't even PCDs, and the
PROGRESS_CODE_OS_LOADER_LOAD and PROGRESS_CODE_OS_LOADER_START macros
actually existed in some header file. That would be similarly clear
about the values being edk2 extensions, but without muddying the picture
with (academic?) platform overrides.
> In any case, this looks good to me otherwise, so for the series
>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Thank you!
Laszlo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/5] MdeModulePkg/UefiBootManagerLib: fix LoadImage/StartImage status code rep.
2019-02-20 8:16 ` [PATCH v2 1/5] MdeModulePkg/UefiBootManagerLib: fix LoadImage/StartImage status code rep Laszlo Ersek
@ 2019-02-20 13:17 ` Ni, Ray
2019-02-21 8:36 ` Laszlo Ersek
0 siblings, 1 reply; 12+ messages in thread
From: Ni, Ray @ 2019-02-20 13:17 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel
Cc: Dandan Bi, Hao Wu, Jian J Wang, Sean Brogan, Star Zeng
On 2/20/2019 4:16 PM, Laszlo Ersek wrote:
> In the EFI_RETURN_STATUS_EXTENDED_DATA structure from PI-1.7, there may be
> padding between the DataHeader and ReturnStatus members. The
> REPORT_STATUS_CODE_EX() macro starts populating the structure immediately
> after DataHeader, therefore the source data must provide for the padding.
>
> Extract the BmReportImageFailure() function from EfiBootManagerBoot(),
> prepare a zero padding (if any) in a temporary
> EFI_RETURN_STATUS_EXTENDED_DATA object, and fix the
> REPORT_STATUS_CODE_EX() macro invocation.
>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1539
> Fixes: c2cf8720a5aad74230767a1f11bade2d86de3745
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> v2:
> - new in v2
>
> MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h | 1 +
> MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 69 +++++++++++++++-----
> 2 files changed, 52 insertions(+), 18 deletions(-)
>
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> index 978fbff966f6..0fef63fceedf 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> @@ -51,6 +51,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> #include <Guid/MemoryTypeInformation.h>
> #include <Guid/FileInfo.h>
> #include <Guid/GlobalVariable.h>
> +#include <Guid/StatusCodeDataTypeId.h>
> #include <Guid/StatusCodeDataTypeVariable.h>
>
> #include <Library/PrintLib.h>
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 9be1633b7480..ffb98c6c9b83 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -1667,6 +1667,55 @@ BmIsBootManagerMenuFilePath (
> return FALSE;
> }
>
> +/**
> + Report status code with EFI_RETURN_STATUS_EXTENDED_DATA about LoadImage() or
> + StartImage() failure.
> +
> + @param[in] ErrorCode An Error Code in the Software Class, DXE Boot
> + Service Driver Subclass. ErrorCode will be used to
> + compose the Value parameter for status code
> + reporting. Must be one of
> + EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR and
> + EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED.
> +
> + @param[in] FailureStatus The failure status returned by the boot service
> + that should be reported.
> +**/
> +VOID
> +BmReportImageFailure (
Laszlo,
Thanks for quick fixing this issue.
To match the status code it reports, how about rename the function as
"BmReportLoadFailure"?
Another minor comments in below.
> + IN UINT32 ErrorCode,
> + IN EFI_STATUS FailureStatus
> + )
> +{
> + EFI_RETURN_STATUS_EXTENDED_DATA ExtendedData;
> + VOID *PaddingStart;
> + UINTN PaddingSize;
> +
> + if (!ReportErrorCodeEnabled ()) {
> + return;
> + }
> +
> + ASSERT (
> + (ErrorCode == EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR) ||
> + (ErrorCode == EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED)
> + );
> +
> + PaddingStart = &ExtendedData.DataHeader + 1;
> + PaddingSize = (UINTN) &ExtendedData.ReturnStatus - (UINTN) PaddingStart;
> + ZeroMem (PaddingStart, PaddingSize);
I prefer "ZeroMem (&ExtendedData, sizeof (ExtendedData))" instead of
introducing local variables such as PaddingStart/PaddingSize.
This makes code more good-looking:).
> + ExtendedData.ReturnStatus = FailureStatus;
> +
> + REPORT_STATUS_CODE_EX (
> + (EFI_ERROR_CODE | EFI_ERROR_MINOR),
> + (EFI_SOFTWARE_DXE_BS_DRIVER | ErrorCode),
> + 0,
> + NULL,
> + NULL,
> + PaddingStart,
&ExtendedData.DataHeader + 1
> + PaddingSize + sizeof (ExtendedData.ReturnStatus)
sizeof (ExtendedData) - sizeof (ExtendedData.DataHeader)
> + );
> +}
> +
> /**
> Attempt to boot the EFI boot option. This routine sets L"BootCurent" and
> also signals the EFI ready to boot event. If the device path for the option
> @@ -1822,15 +1871,7 @@ EfiBootManagerBoot (
> //
> // Report Status Code with the failure status to indicate that the failure to load boot option
> //
> - REPORT_STATUS_CODE_EX (
> - EFI_ERROR_CODE | EFI_ERROR_MINOR,
> - (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR),
> - 0,
> - NULL,
> - NULL,
> - &Status,
> - sizeof (EFI_STATUS)
> - );
> + BmReportImageFailure (EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR, Status);
> BootOption->Status = Status;
> //
> // Destroy the RAM disk
> @@ -1911,15 +1952,7 @@ EfiBootManagerBoot (
> //
> // Report Status Code with the failure status to indicate that boot failure
> //
> - REPORT_STATUS_CODE_EX (
> - EFI_ERROR_CODE | EFI_ERROR_MINOR,
> - (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED),
> - 0,
> - NULL,
> - NULL,
> - &Status,
> - sizeof (EFI_STATUS)
> - );
> + BmReportImageFailure (EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED, Status);
> }
> PERF_END_EX (gImageHandle, "BdsAttempt", NULL, 0, (UINT32) OptionNumber);
>
>
--
Thanks,
Ray
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/5] MdeModulePkg/UefiBootManagerLib: fix LoadImage/StartImage status code rep.
2019-02-20 13:17 ` Ni, Ray
@ 2019-02-21 8:36 ` Laszlo Ersek
0 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2019-02-21 8:36 UTC (permalink / raw)
To: Ni, Ray, edk2-devel
Cc: Dandan Bi, Hao Wu, Jian J Wang, Sean Brogan, Star Zeng
On 02/20/19 14:17, Ni, Ray wrote:
> On 2/20/2019 4:16 PM, Laszlo Ersek wrote:
>> In the EFI_RETURN_STATUS_EXTENDED_DATA structure from PI-1.7, there
>> may be
>> padding between the DataHeader and ReturnStatus members. The
>> REPORT_STATUS_CODE_EX() macro starts populating the structure immediately
>> after DataHeader, therefore the source data must provide for the padding.
>>
>> Extract the BmReportImageFailure() function from EfiBootManagerBoot(),
>> prepare a zero padding (if any) in a temporary
>> EFI_RETURN_STATUS_EXTENDED_DATA object, and fix the
>> REPORT_STATUS_CODE_EX() macro invocation.
>>
>> Cc: Dandan Bi <dandan.bi@intel.com>
>> Cc: Hao Wu <hao.a.wu@intel.com>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Cc: Sean Brogan <sean.brogan@microsoft.com>
>> Cc: Star Zeng <star.zeng@intel.com>
>> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1539
>> Fixes: c2cf8720a5aad74230767a1f11bade2d86de3745
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>> v2:
>> - new in v2
>>
>> MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h | 1 +
>> MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 69
>> +++++++++++++++-----
>> 2 files changed, 52 insertions(+), 18 deletions(-)
>>
>> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
>> b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
>> index 978fbff966f6..0fef63fceedf 100644
>> --- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
>> +++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
>> @@ -51,6 +51,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
>> EITHER EXPRESS OR IMPLIED.
>> #include <Guid/MemoryTypeInformation.h>
>> #include <Guid/FileInfo.h>
>> #include <Guid/GlobalVariable.h>
>> +#include <Guid/StatusCodeDataTypeId.h>
>> #include <Guid/StatusCodeDataTypeVariable.h>
>> #include <Library/PrintLib.h>
>> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>> index 9be1633b7480..ffb98c6c9b83 100644
>> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>> @@ -1667,6 +1667,55 @@ BmIsBootManagerMenuFilePath (
>> return FALSE;
>> }
>> +/**
>> + Report status code with EFI_RETURN_STATUS_EXTENDED_DATA about
>> LoadImage() or
>> + StartImage() failure.
>> +
>> + @param[in] ErrorCode An Error Code in the Software Class, DXE
>> Boot
>> + Service Driver Subclass. ErrorCode will
>> be used to
>> + compose the Value parameter for status code
>> + reporting. Must be one of
>> + EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR and
>> + EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED.
>> +
>> + @param[in] FailureStatus The failure status returned by the boot
>> service
>> + that should be reported.
>> +**/
>> +VOID
>> +BmReportImageFailure (
> Laszlo,
> Thanks for quick fixing this issue.
> To match the status code it reports, how about rename the function as
> "BmReportLoadFailure"?
Sure, I can do that.
> Another minor comments in below.
>> + IN UINT32 ErrorCode,
>> + IN EFI_STATUS FailureStatus
>> + )
>> +{
>> + EFI_RETURN_STATUS_EXTENDED_DATA ExtendedData;
>> + VOID *PaddingStart;
>> + UINTN PaddingSize;
>> +
>> + if (!ReportErrorCodeEnabled ()) {
>> + return;
>> + }
>> +
>> + ASSERT (
>> + (ErrorCode == EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR) ||
>> + (ErrorCode == EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED)
>> + );
>> +
>> + PaddingStart = &ExtendedData.DataHeader + 1;
>> + PaddingSize = (UINTN) &ExtendedData.ReturnStatus - (UINTN)
>> PaddingStart;
>> + ZeroMem (PaddingStart, PaddingSize);
>
> I prefer "ZeroMem (&ExtendedData, sizeof (ExtendedData))" instead of
> introducing local variables such as PaddingStart/PaddingSize.
> This makes code more good-looking:).
I agree that that looks more natural. I didn't know how much you'd like
me to "optimize" this logic. Because,
(1) an optimizing compiler can eliminate PaddingSize from the ZeroMem()
call (replacing it with a constant); in other words, the PaddingSize
calculation would have no runtime cost,
(2) the ZeroMem() call would have to clear fewer bytes.
I hesitated between the smallest possible ZeroMem() and the
easiest-to-read ZeroMem(). I will rework it in v3.
>
>> + ExtendedData.ReturnStatus = FailureStatus;
>> +
>> + REPORT_STATUS_CODE_EX (
>> + (EFI_ERROR_CODE | EFI_ERROR_MINOR),
>> + (EFI_SOFTWARE_DXE_BS_DRIVER | ErrorCode),
>> + 0,
>> + NULL,
>> + NULL,
>> + PaddingStart,
> &ExtendedData.DataHeader + 1
>
>> + PaddingSize + sizeof (ExtendedData.ReturnStatus)
> sizeof (ExtendedData) - sizeof (ExtendedData.DataHeader)
Yes.
Thanks,
Laszlo
>> + );
>> +}
>> +
>> /**
>> Attempt to boot the EFI boot option. This routine sets
>> L"BootCurent" and
>> also signals the EFI ready to boot event. If the device path for
>> the option
>> @@ -1822,15 +1871,7 @@ EfiBootManagerBoot (
>> //
>> // Report Status Code with the failure status to indicate that
>> the failure to load boot option
>> //
>> - REPORT_STATUS_CODE_EX (
>> - EFI_ERROR_CODE | EFI_ERROR_MINOR,
>> - (EFI_SOFTWARE_DXE_BS_DRIVER |
>> EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR),
>> - 0,
>> - NULL,
>> - NULL,
>> - &Status,
>> - sizeof (EFI_STATUS)
>> - );
>> + BmReportImageFailure (EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR,
>> Status);
>> BootOption->Status = Status;
>> //
>> // Destroy the RAM disk
>> @@ -1911,15 +1952,7 @@ EfiBootManagerBoot (
>> //
>> // Report Status Code with the failure status to indicate that
>> boot failure
>> //
>> - REPORT_STATUS_CODE_EX (
>> - EFI_ERROR_CODE | EFI_ERROR_MINOR,
>> - (EFI_SOFTWARE_DXE_BS_DRIVER |
>> EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED),
>> - 0,
>> - NULL,
>> - NULL,
>> - &Status,
>> - sizeof (EFI_STATUS)
>> - );
>> + BmReportImageFailure (EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED, Status);
>> }
>> PERF_END_EX (gImageHandle, "BdsAttempt", NULL, 0, (UINT32)
>> OptionNumber);
>>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-02-21 8:36 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-20 8:16 [PATCH v2 0/5] MdeModulePkg, OvmfPkg, ArmVirtPkg: more visible boot progress reporting Laszlo Ersek
2019-02-20 8:16 ` [PATCH v2 1/5] MdeModulePkg/UefiBootManagerLib: fix LoadImage/StartImage status code rep Laszlo Ersek
2019-02-20 13:17 ` Ni, Ray
2019-02-21 8:36 ` Laszlo Ersek
2019-02-20 8:16 ` [PATCH v2 2/5] OvmfPkg: add library to track boot option loading/starting on the console Laszlo Ersek
2019-02-20 9:21 ` Ard Biesheuvel
2019-02-20 12:01 ` Laszlo Ersek
2019-02-20 10:04 ` Laszlo Ersek
2019-02-20 10:06 ` Ard Biesheuvel
2019-02-20 8:16 ` [PATCH v2 3/5] OvmfPkg/PlatformBootManagerLib: display boot option loading/starting Laszlo Ersek
2019-02-20 8:16 ` [PATCH v2 4/5] ArmVirtPkg/ArmVirtQemu*: enable minimal Status Code Routing in DXE Laszlo Ersek
2019-02-20 8:16 ` [PATCH v2 5/5] ArmVirtPkg/PlatformBootManagerLib: display boot option loading/starting Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox