From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::144; helo=mail-it1-x144.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x144.google.com (mail-it1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 5635B201B0384 for ; Wed, 20 Feb 2019 02:07:04 -0800 (PST) Received: by mail-it1-x144.google.com with SMTP id x131so13905009itc.3 for ; Wed, 20 Feb 2019 02:07:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=/OGwCPzegH2PPYbacduYYyKUW1o26pupQWJG9o8sB/I=; b=MnBNGxDOI53hPfuD2q8HNlMeEZt4XYqNNFKLye/iULicrWikneCsL0Ze3OHp/G+XwU wTiUJyvwI4MYrNkCGjiyAOpDFkVaiO9501oAOSpBHDdZ7zPF0P6KHjm9VcWBZ99WP0Ga ysRO1hToN6bLCpUt7pnYK9tgJ2DuLiwKOgUebxXYmqeDhkQM6yxc16UWXCDlTFofx18Z CT6YCU5cqCT4Cwfs/lbnvqQts4xxB+RocsznwRq4dDyfDcrKYnyKzcKm51RM7FWHpEzo wf/yMj4A8S1YB45lrr7TuUJBLcXzV+dmi2D7pmrXmu+CAWKKDYpvkBmz2UGi9K9XH1zf Vpgg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=/OGwCPzegH2PPYbacduYYyKUW1o26pupQWJG9o8sB/I=; b=kZG01v8gX+2VMqDhx2IW1mu5ol5Vmywe3DP/3iAD02clibPlwl2ZRT4r1eKkawdkUV 6VVe303B5xlSG48akBTUuU4tGH+frkYVrS+lKm62jN2BCTeswK5f+Mf31RahZeZNI7qZ m+ID/d1J32weaHv2XcByOtbISG+zXUUcgjhp6ygg3G+WOa6ExsYh0acf1LJd3dRlv9HO Si++iCrv6/5bGElph0dHEJVoq8PWVI5gN0oCH8zCmPDM5dCg45y4F9QOB6sES4qniLgW FYqYtN6kgvBsNWCl76IyqIMT10wQygaHzI36CYVog9lSRSBhtN/qso9lFBdnyNdySjzz JWdA== X-Gm-Message-State: AHQUAuYtmj0Wo7rtrJ08rmh46CBNbvRL7qtlxTmXPBW94UcVTYlkWIEw paHd3X7+iycA8l2UgaI+xsos9Mla7Fxn1M6VzvGX0A== X-Google-Smtp-Source: AHgI3IbWpesmcCYKdopgSbHBh97YbS7Rm8w1i5VfVsnMq8pMX1B2mheNtR58o3fEFPxXulrJAxFCA/8pXH4pIjT3dlk= X-Received: by 2002:a6b:6511:: with SMTP id z17mr19358776iob.173.1550657223394; Wed, 20 Feb 2019 02:07:03 -0800 (PST) MIME-Version: 1.0 References: <20190220081644.8238-1-lersek@redhat.com> <20190220081644.8238-3-lersek@redhat.com> In-Reply-To: From: Ard Biesheuvel Date: Wed, 20 Feb 2019 11:06:52 +0100 Message-ID: To: Laszlo Ersek Cc: "edk2-devel@lists.01.org" , Anthony Perard , Jordan Justen Subject: Re: [PATCH v2 2/5] OvmfPkg: add library to track boot option loading/starting on the console X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Feb 2019 10:07:04 -0000 Content-Type: text/plain; charset="UTF-8" On Wed, 20 Feb 2019 at 11:04, Laszlo Ersek 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 > > Cc: Ard Biesheuvel > > Cc: Jordan Justen > > Cc: Julien Grall > > Cc: Ray Ni > > Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1515418 > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Laszlo Ersek > > --- > > > > 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 > > and > > , and follow Ray's > > advice in > > : > > > > - 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 > > + > > +/** > > + 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 > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > + > > +#include > > +#include > > + > > +#include > > + > > + > > +// > > +// 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.