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

["Nhi Pham" <nhi@os.amperecomputing.com>]

Hi Ard,

On 04/09/2021 00:50, Ard Biesheuvel wrote:
> 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?

I just sent out the v2 of this patch which updates the commit message to 
clarify the reason why we need to implement a new PlatformBootManagerLib 
for LinuxBoot.

Thanks for your reviewing!

-Nhi

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