From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Laszlo Ersek <lersek@redhat.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
Anthony Perard <anthony.perard@citrix.com>,
Jordan Justen <jordan.l.justen@intel.com>
Subject: Re: [PATCH v2 2/5] OvmfPkg: add library to track boot option loading/starting on the console
Date: Wed, 20 Feb 2019 11:06:52 +0100 [thread overview]
Message-ID: <CAKv+Gu-zLzrAZOrofyNPB4ub4WeNhTkLfJ2XoS5QEZcp8gmFmA@mail.gmail.com> (raw)
In-Reply-To: <cfa670f6-0fd9-6c6d-37a6-bcb46344edeb@redhat.com>
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.
next prev parent reply other threads:[~2019-02-20 10:07 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKv+Gu-zLzrAZOrofyNPB4ub4WeNhTkLfJ2XoS5QEZcp8gmFmA@mail.gmail.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox