Re: [PATCH 1/1] ArmPkg: Implement PlatformBootManagerLib for LinuxBoot

["Ard Biesheuvel" <ardb@kernel.org>]

On Fri, 3 Sept 2021 at 18:07, Nhi Pham <nhi@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 as it will be executed at the end of DXE
> phase.
>
> This PlatformBootManagerLib instance registers only a GUID defined by
> gArmTokenSpaceGuid.PcdLinuxBootFileGuid for LinuxBoot Payload as active
> boot option. It allows BDS to jump to the LinuxBoot Shell immediately
> with skipping the UiApp Setup Screen or UEFI Shell.
>
> The PlatformBootManagerLib library derived from
> ArmPkg/Library/PlatformBootManagerLib.
>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>
> Signed-off-by: Nhi Pham <nhi@os.amperecomputing.com>

I *think* I know the answer, but could you please clarify in the
commit log why this justifies/requires a completely different
BootManagerLib implementation?

> ---
>  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
>