On Fri, Oct 22, 2021 at 11:08 AM Ard Biesheuvel <a...@kernel.org> wrote:
>
> This patch triggers CI failures
>
> https://github.com/tianocore/edk2/pull/2114
>
> Please take a look and resubmit if there is anything to fix.
Looks like a missing comment?
>
> On Wed, 13 Oct 2021 at 20:43, Samer El-Haj-Mahmoud
> <samer.el-haj-mahm...@arm.com> wrote:
> >
> > Ackd-by: Samer El-Haj-Mahmoud <samer.el-haj-mahm...@arm.com>
Acked-by: Moritz Fischer <mori...@google.com>
> >
> > Any update on getting this reviewed/merged? We have downstream platforms
> > that depend on this and would like to avoid duplication of similar
> > functionality in platform code.
> >
> > Thanks!
> > --Samer
> >
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> > > Biesheuvel via groups.io
> > > Sent: Wednesday, September 22, 2021 7:49 AM
> > > To: Nhi Pham <n...@os.amperecomputing.com>
> > > Cc: edk2-devel-groups-io <devel@edk2.groups.io>;
> > > patc...@amperecomputing.com; Leif Lindholm <l...@nuviainc.com>; Ard
> > > Biesheuvel <ardb+tianoc...@kernel.org>
> > > Subject: Re: [edk2-devel] [PATCH v2 1/1] ArmPkg: Implement
> > > PlatformBootManagerLib for LinuxBoot
> > >
> > > On Tue, 7 Sept 2021 at 05:40, Nhi Pham <n...@os.amperecomputing.com>
> > > wrote:
> > > >
> > > > LinuxBoot is a firmware that replaces specific firmware functionality
> > > > like the UEFI DXE phase with a Linux kernel and runtime. It is built-in
> > > > UEFI image like an application, which is executed at the end of DXE
> > > > phase.
> > > >
> > > > To achieve the LinuxBoot boot flow "SEC->PEI->DXE->BDS->LinuxBoot",
> > > > today we use the common well-known GUID of UEFI Shell for LinuxBoot
> > > > payload, so LinuxBoot developers can effortlessly find the UEFI Shell
> > > > Application and replace it with the LinuxBoot payload without
> > > > recompiling platform EDK2 (There might be an issue with a few systems
> > > > that don't have a UEFI Shell). Also, we have a hard requirement to force
> > > > the BDS to boot into the LinuxBoot as it is essentially required that
> > > > only the LinuxBoot boot option is permissible and UEFI is an
> > > > intermediate bootstrap phase. Considering all the above, it is
> > > > reasonable to just have a new GUID for LinuxBoot and require a LinuxBoot
> > > > specific BDS implementation. In addition, with making the BDS
> > > > implementation simpler, we can reduce many DXE drivers which we think it
> > > > is not necessary for LinuxBoot booting.
> > > >
> > > > This patch adds a new PlatformBootManagerLib implementation which
> > > > registers only the gArmTokenSpaceGuid.PcdLinuxBootFileGuid for
> > > LinuxBoot
> > > > payload as an active boot option. It allows BDS to jump to the LinuxBoot
> > > > quickly by skipping the UiApp and UEFI Shell.
> > > >
> > > > The PlatformBootManagerLib library derived from
> > > > ArmPkg/Library/PlatformBootManagerLib.
> > > >
> > > > Cc: Leif Lindholm <l...@nuviainc.com>
> > > > Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>
> > > >
> > > > Signed-off-by: Nhi Pham <n...@os.amperecomputing.com>
> > >
> > > Acked-by: Ard Biesheuvel <a...@kernel.org>
> > >
> > > > ---
> > > > ArmPkg/ArmPkg.dec |
> > > > 8 +
> > > > ArmPkg/ArmPkg.dsc |
> > > > 2 +
> > > > ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBootManagerLib.inf
> > > | 58 +++++++
> > > > ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBm.c |
> > > > 178
> > > ++++++++++++++++++++
> > > > 4 files changed, 246 insertions(+)
> > > >
> > > > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> > > > index 214b2f589217..f68e6ee00860 100644
> > > > --- a/ArmPkg/ArmPkg.dec
> > > > +++ b/ArmPkg/ArmPkg.dec
> > > > @@ -3,6 +3,7 @@
> > > > #
> > > > # Copyright (c) 2009 - 2010, Apple Inc. All rights reserved.<BR>
> > > > # Copyright (c) 2011 - 2021, ARM Limited. All rights reserved.
> > > > +# Copyright (c) 2021, Ampere Computing LLC. All rights reserved.
> > > > #
> > > > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > #
> > > > @@ -382,3 +383,10 @@ [PcdsFixedAtBuild.common,
> > > PcdsDynamic.common]
> > > > #
> > > > gArmTokenSpaceGuid.PcdPciBusMin|0x0|UINT32|0x00000059
> > > > gArmTokenSpaceGuid.PcdPciBusMax|0x0|UINT32|0x0000005A
> > > > +
> > > > +[PcdsDynamicEx]
> > > > + #
> > > > + # This dynamic PCD hold the GUID of a firmware FFS which contains
> > > > + # the LinuxBoot payload.
> > > > + #
> > > > + gArmTokenSpaceGuid.PcdLinuxBootFileGuid|{0x0}|VOID*|0x0000005C
> > > > diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
> > > > index 926986cf7fbb..ffb1c261861e 100644
> > > > --- a/ArmPkg/ArmPkg.dsc
> > > > +++ b/ArmPkg/ArmPkg.dsc
> > > > @@ -5,6 +5,7 @@
> > > > # Copyright (c) 2011 - 2021, Arm Limited. All rights reserved.<BR>
> > > > # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
> > > > # Copyright (c) Microsoft Corporation.<BR>
> > > > +# Copyright (c) 2021, Ampere Computing LLC. All rights reserved.
> > > > #
> > > > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > #
> > > > @@ -150,6 +151,7 @@ [Components.common]
> > > >
> > > ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
> > > >
> > > ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
> > > > ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> > > > +
> > > ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBootManagerLib.inf
> > > >
> > > > ArmPkg/Drivers/ArmCrashDumpDxe/ArmCrashDumpDxe.inf
> > > > ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf
> > > > diff --git
> > > a/ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBootManagerLib.inf
> > > b/ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBootManagerLib.inf
> > > > new file mode 100644
> > > > index 000000000000..139b6171990a
> > > > --- /dev/null
> > > > +++
> > > b/ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBootManagerLib.inf
> > > > @@ -0,0 +1,58 @@
> > > > +## @file
> > > > +# Implementation for PlatformBootManagerLib library class interfaces.
> > > > +#
> > > > +# Copyright (C) 2015-2016, Red Hat, Inc.
> > > > +# Copyright (c) 2014, ARM Ltd. All rights reserved.<BR>
> > > > +# Copyright (c) 2007 - 2014, Intel Corporation. All rights
> > > > reserved.<BR>
> > > > +# Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
> > > > +# Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights
> > > reserved.<BR>
> > > > +#
> > > > +# SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > +#
> > > > +##
> > > > +
> > > > +[Defines]
> > > > + INF_VERSION = 0x0001001B
> > > > + BASE_NAME = LinuxBootBootManagerLib
> > > > + FILE_GUID = 1FA91547-DB23-4F6A-8AF8-3B9782A7F917
> > > > + MODULE_TYPE = DXE_DRIVER
> > > > + VERSION_STRING = 1.0
> > > > + LIBRARY_CLASS = PlatformBootManagerLib|DXE_DRIVER
> > > > +
> > > > +#
> > > > +# The following information is for reference only and not required by
> > > > the
> > > build tools.
> > > > +#
> > > > +# VALID_ARCHITECTURES = ARM AARCH64
> > > > +#
> > > > +
> > > > +[Sources]
> > > > + LinuxBootBm.c
> > > > +
> > > > +[Packages]
> > > > + ArmPkg/ArmPkg.dec
> > > > + MdeModulePkg/MdeModulePkg.dec
> > > > + MdePkg/MdePkg.dec
> > > > + ShellPkg/ShellPkg.dec
> > > > +
> > > > +[LibraryClasses]
> > > > + BaseLib
> > > > + BaseMemoryLib
> > > > + DebugLib
> > > > + MemoryAllocationLib
> > > > + PcdLib
> > > > + PrintLib
> > > > + UefiBootManagerLib
> > > > + UefiBootServicesTableLib
> > > > + UefiLib
> > > > + UefiRuntimeServicesTableLib
> > > > +
> > > > +[Pcd]
> > > > + gArmTokenSpaceGuid.PcdLinuxBootFileGuid
> > > > +
> > > > +[Guids]
> > > > + gEfiEndOfDxeEventGroupGuid
> > > > + gUefiShellFileGuid
> > > > + gZeroGuid
> > > > +
> > > > +[Protocols]
> > > > + gEfiLoadedImageProtocolGuid
> > > > diff --git a/ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBm.c
> > > b/ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBm.c
> > > > new file mode 100644
> > > > index 000000000000..f4941780efcd
> > > > --- /dev/null
> > > > +++ b/ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBm.c
> > > > @@ -0,0 +1,178 @@
> > > > +/** @file
> > > > + Implementation for PlatformBootManagerLib library class interfaces.
> > > > +
> > > > + Copyright (C) 2015-2016, Red Hat, Inc.
> > > > + Copyright (c) 2014 - 2019, ARM Ltd. All rights reserved.<BR>
> > > > + Copyright (c) 2004 - 2018, Intel Corporation. All rights
> > > > reserved.<BR>
> > > > + Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
> > > > + Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights
> > > reserved.<BR>
> > > > +
> > > > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > +
> > > > +**/
> > > > +
> > > > +#include <Uefi.h>
> > > > +
> > > > +#include <Guid/EventGroup.h>
> > > > +#include <Library/BaseLib.h>
> > > > +#include <Library/BaseMemoryLib.h>
> > > > +#include <Library/DebugLib.h>
> > > > +#include <Library/DevicePathLib.h>
> > > > +#include <Library/MemoryAllocationLib.h>
> > > > +#include <Library/PcdLib.h>
> > > > +#include <Library/UefiBootManagerLib.h>
> > > > +#include <Library/UefiBootServicesTableLib.h>
> > > > +#include <Library/UefiLib.h>
> > > > +#include <Library/UefiRuntimeServicesTableLib.h>
> > > > +#include <Protocol/LoadedImage.h>
> > > > +#include <Protocol/PlatformBootManager.h>
> > > > +
> > > > +STATIC
> > > > +VOID
> > > > +PlatformRegisterFvBootOption (
> > > > + CONST EFI_GUID *FileGuid,
> > > > + CHAR16 *Description,
> > > > + UINT32 Attributes
> > > > + )
> > > > +{
> > > > + EFI_STATUS Status;
> > > > + INTN OptionIndex;
> > > > + EFI_BOOT_MANAGER_LOAD_OPTION NewOption;
> > > > + EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions;
> > > > + UINTN BootOptionCount;
> > > > + MEDIA_FW_VOL_FILEPATH_DEVICE_PATH FileNode;
> > > > + EFI_LOADED_IMAGE_PROTOCOL *LoadedImage;
> > > > + EFI_DEVICE_PATH_PROTOCOL *DevicePath;
> > > > +
> > > > + Status = gBS->HandleProtocol (
> > > > + gImageHandle,
> > > > + &gEfiLoadedImageProtocolGuid,
> > > > + (VOID **)&LoadedImage
> > > > + );
> > > > + ASSERT_EFI_ERROR (Status);
> > > > +
> > > > + EfiInitializeFwVolDevicepathNode (&FileNode, FileGuid);
> > > > + DevicePath = DevicePathFromHandle (LoadedImage->DeviceHandle);
> > > > + ASSERT (DevicePath != NULL);
> > > > + DevicePath = AppendDevicePathNode (
> > > > + DevicePath,
> > > > + (EFI_DEVICE_PATH_PROTOCOL *)&FileNode
> > > > + );
> > > > + ASSERT (DevicePath != NULL);
> > > > +
> > > > + Status = EfiBootManagerInitializeLoadOption (
> > > > + &NewOption,
> > > > + LoadOptionNumberUnassigned,
> > > > + LoadOptionTypeBoot,
> > > > + Attributes,
> > > > + Description,
> > > > + DevicePath,
> > > > + NULL,
> > > > + 0
> > > > + );
> > > > + ASSERT_EFI_ERROR (Status);
> > > > + FreePool (DevicePath);
> > > > +
> > > > + BootOptions = EfiBootManagerGetLoadOptions (
> > > > + &BootOptionCount,
> > > > + LoadOptionTypeBoot
> > > > + );
> > > > +
> > > > + OptionIndex = EfiBootManagerFindLoadOption (
> > > > + &NewOption,
> > > > + BootOptions,
> > > > + BootOptionCount
> > > > + );
> > > > +
> > > > + if (OptionIndex == -1) {
> > > > + Status = EfiBootManagerAddLoadOptionVariable (&NewOption,
> > > MAX_UINTN);
> > > > + ASSERT_EFI_ERROR (Status);
> > > > + }
> > > > + EfiBootManagerFreeLoadOption (&NewOption);
> > > > + EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> > > > +}
> > > > +
> > > > +/**
> > > > + Do the platform specific action before the console is connected.
> > > > +
> > > > + Such as:
> > > > + Update console variable;
> > > > + Register new Driver#### or Boot####;
> > > > + Signal ReadyToLock event.
> > > > +**/
> > > > +VOID
> > > > +EFIAPI
> > > > +PlatformBootManagerBeforeConsole (
> > > > + VOID
> > > > + )
> > > > +{
> > > > + //
> > > > + // Signal EndOfDxe PI Event
> > > > + //
> > > > + EfiEventGroupSignal (&gEfiEndOfDxeEventGroupGuid);
> > > > +}
> > > > +
> > > > +/**
> > > > + Do the platform specific action after the console is connected.
> > > > +
> > > > + Such as:
> > > > + Dynamically switch output mode;
> > > > + Signal console ready platform customized event;
> > > > + Run diagnostics like memory testing;
> > > > + Connect certain devices;
> > > > + Dispatch additional option roms.
> > > > +**/
> > > > +VOID
> > > > +EFIAPI
> > > > +PlatformBootManagerAfterConsole (
> > > > + VOID
> > > > + )
> > > > +{
> > > > + EFI_GUID LinuxBootFileGuid;
> > > > +
> > > > + CopyGuid (&LinuxBootFileGuid, PcdGetPtr (PcdLinuxBootFileGuid));
> > > > +
> > > > + if (!CompareGuid (&LinuxBootFileGuid, &gZeroGuid)) {
> > > > + //
> > > > + // Register LinuxBoot
> > > > + //
> > > > + PlatformRegisterFvBootOption (
> > > > + &LinuxBootFileGuid,
> > > > + L"LinuxBoot",
> > > > + LOAD_OPTION_ACTIVE
> > > > + );
> > > > + } else {
> > > > + DEBUG ((DEBUG_ERROR, "%a: PcdLinuxBootFileGuid was not set!\n",
> > > __FUNCTION__));
> > > > + }
> > > > +}
> > > > +
> > > > +/**
> > > > + This function is called each second during the boot manager waits the
> > > > + timeout.
> > > > +
> > > > + @param TimeoutRemain The remaining timeout.
> > > > +**/
> > > > +VOID
> > > > +EFIAPI
> > > > +PlatformBootManagerWaitCallback (
> > > > + UINT16 TimeoutRemain
> > > > + )
> > > > +{
> > > > + return;
> > > > +}
> > > > +
> > > > +/**
> > > > + The function is called when no boot option could be launched,
> > > > + including platform recovery options and options pointing to
> > > > applications
> > > > + built into firmware volumes.
> > > > +
> > > > + If this function returns, BDS attempts to enter an infinite loop.
> > > > +**/
> > > > +VOID
> > > > +EFIAPI
> > > > +PlatformBootManagerUnableToBoot (
> > > > + VOID
> > > > + )
> > > > +{
> > > > + return;
> > > > +}
> > > > --
> > > > 2.17.1
> > > >
> > >
> > >
> > >
> > >
> >
> > IMPORTANT NOTICE: The contents of this email and any attachments are
> > confidential and may also be privileged. If you are not the intended
> > recipient, please notify the sender immediately and do not disclose the
> > contents to any other person, use it for any purpose, or store or copy the
> > information in any medium. Thank you.
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82564): https://edk2.groups.io/g/devel/message/82564
Mute This Topic: https://groups.io/mt/85428309/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
