From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-il1-f169.google.com (mail-il1-f169.google.com [209.85.166.169]) by mx.groups.io with SMTP id smtpd.web08.2024.1634956482849404966 for ; Fri, 22 Oct 2021 19:34:43 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@google.com header.s=20210112 header.b=VJw7U/1I; spf=pass (domain: google.com, ip: 209.85.166.169, mailfrom: moritzf@google.com) Received: by mail-il1-f169.google.com with SMTP id i6so6191071ila.12 for ; Fri, 22 Oct 2021 19:34:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=+N87pKoynNxPZGPniq97RlO01L8k5XIvcSf2RNmevxI=; b=VJw7U/1InW3FXzRzH2tvCHd/0ZQkLMhB3AhTltkGCCABTSC/Zp4VM7E6rErAsDO5H2 DqADK3QT4bwFbF3smIq8+b8TmTjoiuHVn4dIOivEXaGnG8j+MIDiWQOSE1/1/ZTGgcqd y7kFQCn/5/uMWAhqbk8zaVs5jr92t2CGHaG4zWAptDOtV3Q+v1Wex9uf+xy7bTPm61ZF fQMj0Q/vfxGAtGbiRqI5F6d40AtjaacZ5N1a2Qse9EL7PZWYD5zSW+ajEuBC4J7uddjU dOWe5Zq6Y12mDC82FGIuH6OWlBso+ZlY1witAbljhRZpzWkuXOEXfHa531NhUcOEsYAg UNGw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=+N87pKoynNxPZGPniq97RlO01L8k5XIvcSf2RNmevxI=; b=kUW5ISi1+wnTu4pHGrsPYxgr1rllXmLe3N5298lAZilcTQRZsdvrs6tZaIsv2LH4BT ZlSMhzWtml3R6L5ctmlyw7RuVaWsS0Z+vxlkt0x158O5bkVDdQZpsSMZtXn5bdQUqDR/ WNWHBb/lYnjcGd8WSl37Z3j6yjByg18gn4dnewx8i5cfPmaWV4kZcUABiyPru++pEiCT ZoHvD/j8zCV/aEKVMrBiuI7tfvXbiPqmGHnGardhcF0Lxd/U7LlyI4C/6wS+1bCLa63Y /6/aZAwVFlvc1Ahd7TvYN1bhFEfn3qksCneaHgbJ/0l4kwLreuKbCg3IUA+GeLHmV4HS 96WQ== X-Gm-Message-State: AOAM530v0CxzHRIp3wuFTLqRreGXRuxo90JYbaLfNSQbRUOsONUve6cX qzupeC/4o+Xdide/9osF0u5LV5pK9LboGmB2BS4RmMBkAuE= X-Google-Smtp-Source: ABdhPJzY52RFNAtXo9iX3bZuIPhkCEMP5LENcCoeTBs/vzeXpGWMeTnDf2MPf8sn5wa5XJksTxKCwF2nWF2wb3D2aEE= X-Received: by 2002:a05:6e02:1402:: with SMTP id n2mr2107844ilo.160.1634956481771; Fri, 22 Oct 2021 19:34:41 -0700 (PDT) MIME-Version: 1.0 References: <20210907033857.18848-1-nhi@os.amperecomputing.com> In-Reply-To: From: "Moritz Fischer" Date: Fri, 22 Oct 2021 19:34:30 -0700 Message-ID: Subject: Re: [edk2-devel] [PATCH v2 1/1] ArmPkg: Implement PlatformBootManagerLib for LinuxBoot To: devel@edk2.groups.io, ardb@kernel.org Cc: Samer El-Haj-Mahmoud , Nhi Pham , "patches@amperecomputing.com" , Leif Lindholm , Ard Biesheuvel , Jeff Booher-Kaeding Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, Oct 22, 2021 at 11:08 AM Ard Biesheuvel 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 > wrote: > > > > Ackd-by: Samer El-Haj-Mahmoud Acked-by: Moritz Fischer > > > > Any update on getting this reviewed/merged? We have downstream platform= s that depend on this and would like to avoid duplication of similar functi= onality in platform code. > > > > Thanks! > > --Samer > > > > > > > -----Original Message----- > > > From: devel@edk2.groups.io On Behalf Of Ard > > > Biesheuvel via groups.io > > > Sent: Wednesday, September 22, 2021 7:49 AM > > > To: Nhi Pham > > > Cc: edk2-devel-groups-io ; > > > patches@amperecomputing.com; Leif Lindholm ; Ard > > > Biesheuvel > > > Subject: Re: [edk2-devel] [PATCH v2 1/1] ArmPkg: Implement > > > PlatformBootManagerLib for LinuxBoot > > > > > > On Tue, 7 Sept 2021 at 05:40, Nhi Pham > > > wrote: > > > > > > > > LinuxBoot is a firmware that replaces specific firmware functionali= ty > > > > like the UEFI DXE phase with a Linux kernel and runtime. It is buil= t-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 She= ll > > > > Application and replace it with the LinuxBoot payload without > > > > recompiling platform EDK2 (There might be an issue with a few syste= ms > > > > 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 th= at > > > > 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 Linu= xBoot > > > > specific BDS implementation. In addition, with making the BDS > > > > implementation simpler, we can reduce many DXE drivers which we thi= nk 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 Linu= xBoot > > > > quickly by skipping the UiApp and UEFI Shell. > > > > > > > > The PlatformBootManagerLib library derived from > > > > ArmPkg/Library/PlatformBootManagerLib. > > > > > > > > Cc: Leif Lindholm > > > > Cc: Ard Biesheuvel > > > > > > > > Signed-off-by: Nhi Pham > > > > > > Acked-by: Ard Biesheuvel > > > > > > > --- > > > > 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.
> > > > # 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 contain= s > > > > + # 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.
> > > > # Copyright (c) 2016, Linaro Ltd. All rights reserved.
> > > > # Copyright (c) Microsoft Corporation.
> > > > +# 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 interfa= ces. > > > > +# > > > > +# Copyright (C) 2015-2016, Red Hat, Inc. > > > > +# Copyright (c) 2014, ARM Ltd. All rights reserved.
> > > > +# Copyright (c) 2007 - 2014, Intel Corporation. All rights reserv= ed.
> > > > +# Copyright (c) 2016, Linaro Ltd. All rights reserved.
> > > > +# Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights > > > reserved.
> > > > +# > > > > +# SPDX-License-Identifier: BSD-2-Clause-Patent > > > > +# > > > > +## > > > > + > > > > +[Defines] > > > > + INF_VERSION =3D 0x0001001B > > > > + BASE_NAME =3D LinuxBootBootManagerLib > > > > + FILE_GUID =3D 1FA91547-DB23-4F6A-8AF8-3B978= 2A7F917 > > > > + MODULE_TYPE =3D DXE_DRIVER > > > > + VERSION_STRING =3D 1.0 > > > > + LIBRARY_CLASS =3D PlatformBootManagerLib|DXE_DR= IVER > > > > + > > > > +# > > > > +# The following information is for reference only and not required= by the > > > build tools. > > > > +# > > > > +# VALID_ARCHITECTURES =3D 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 interfac= es. > > > > + > > > > + Copyright (C) 2015-2016, Red Hat, Inc. > > > > + Copyright (c) 2014 - 2019, ARM Ltd. All rights reserved.
> > > > + Copyright (c) 2004 - 2018, Intel Corporation. All rights reserve= d.
> > > > + Copyright (c) 2016, Linaro Ltd. All rights reserved.
> > > > + Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights > > > reserved.
> > > > + > > > > + SPDX-License-Identifier: BSD-2-Clause-Patent > > > > + > > > > +**/ > > > > + > > > > +#include > > > > + > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > + > > > > +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 =3D gBS->HandleProtocol ( > > > > + gImageHandle, > > > > + &gEfiLoadedImageProtocolGuid, > > > > + (VOID **)&LoadedImage > > > > + ); > > > > + ASSERT_EFI_ERROR (Status); > > > > + > > > > + EfiInitializeFwVolDevicepathNode (&FileNode, FileGuid); > > > > + DevicePath =3D DevicePathFromHandle (LoadedImage->DeviceHandle); > > > > + ASSERT (DevicePath !=3D NULL); > > > > + DevicePath =3D AppendDevicePathNode ( > > > > + DevicePath, > > > > + (EFI_DEVICE_PATH_PROTOCOL *)&FileNode > > > > + ); > > > > + ASSERT (DevicePath !=3D NULL); > > > > + > > > > + Status =3D EfiBootManagerInitializeLoadOption ( > > > > + &NewOption, > > > > + LoadOptionNumberUnassigned, > > > > + LoadOptionTypeBoot, > > > > + Attributes, > > > > + Description, > > > > + DevicePath, > > > > + NULL, > > > > + 0 > > > > + ); > > > > + ASSERT_EFI_ERROR (Status); > > > > + FreePool (DevicePath); > > > > + > > > > + BootOptions =3D EfiBootManagerGetLoadOptions ( > > > > + &BootOptionCount, > > > > + LoadOptionTypeBoot > > > > + ); > > > > + > > > > + OptionIndex =3D EfiBootManagerFindLoadOption ( > > > > + &NewOption, > > > > + BootOptions, > > > > + BootOptionCount > > > > + ); > > > > + > > > > + if (OptionIndex =3D=3D -1) { > > > > + Status =3D 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 wait= s 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 appl= ications > > > > + 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 co= nfidential and may also be privileged. If you are not the intended recipien= t, 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. > > >=20 > >