From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web09.1125.1634926096987644002 for ; Fri, 22 Oct 2021 11:08:17 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=rJfTzzDg; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id AD1266120D for ; Fri, 22 Oct 2021 18:08:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1634926094; bh=hPZyaaMUoFfOUh1oCVFLCvjqaI5/mirx9cHwJT/qrco=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=rJfTzzDgPzVzjHVS7smp7KtzNkkQfDBuUdiSrs6piNBgM+sIYnvohPMR9AIiOgNaY uQZ1VuocC+hR4Iov2xGdNH56bx2kLsmFRlbOfUNKGhjEIRcR0ZCWqt6gfEDN3SPTGB T4qvoJIGnCrLrlMOIXcqmto378qzO+XTPfNLMAhPai3VJ+ebRyHaFYReCJ+eywN0fn I31uMp756k2w9FS2hCVsUfPiUUfPJ2mblV+FFzKTVeaPXdLLpEVFcAL1ixj5g9dW6S K5aw4Wvq0AFD9lamKvepjpV0ELWj3s+Z8+1UCShKBWPd5Wl56F9ARFNztPF/tlqMXM 06UYwZDljClvQ== Received: by mail-ot1-f47.google.com with SMTP id 34-20020a9d0325000000b00552cae0decbso5443141otv.0 for ; Fri, 22 Oct 2021 11:08:14 -0700 (PDT) X-Gm-Message-State: AOAM533JMlha9k/OhG0zzJAGsIwrRPF7CPKTin0Lqcw1rxXBdzVcyByj N4uLl1jMrZpN5LdTnmpCYbSnqyX8KK7SXxCjeiY= X-Google-Smtp-Source: ABdhPJzrtzfnHfd1cAOF7VROzshyQQlLSL1VsHDEFcBSCZ9nTa1qlBa4GsYlSNBvDasQKX48hVspcsCc6T1CBOTHuVo= X-Received: by 2002:a9d:5911:: with SMTP id t17mr1158037oth.30.1634926093926; Fri, 22 Oct 2021 11:08:13 -0700 (PDT) MIME-Version: 1.0 References: <20210907033857.18848-1-nhi@os.amperecomputing.com> In-Reply-To: From: "Ard Biesheuvel" Date: Fri, 22 Oct 2021 20:08:02 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH v2 1/1] ArmPkg: Implement PlatformBootManagerLib for LinuxBoot To: Samer El-Haj-Mahmoud Cc: "devel@edk2.groups.io" , Nhi Pham , "patches@amperecomputing.com" , Leif Lindholm , Ard Biesheuvel , Jeff Booher-Kaeding Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable This patch triggers CI failures https://github.com/tianocore/edk2/pull/2114 Please take a look and resubmit if there is anything to fix. On Wed, 13 Oct 2021 at 20:43, Samer El-Haj-Mahmoud wrote: > > Ackd-by: Samer El-Haj-Mahmoud > > Any update on getting this reviewed/merged? We have downstream platforms = that depend on this and would like to avoid duplication of similar function= ality 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 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 fo= rce > > > 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 LinuxB= oot > > > 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 LinuxB= oot > > > 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 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.
> > > # 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.in= f > > > 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 interface= s. > > > +# > > > +# Copyright (C) 2015-2016, Red Hat, Inc. > > > +# Copyright (c) 2014, ARM Ltd. All rights reserved.
> > > +# Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved= .
> > > +# 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-3B9782A= 7F917 > > > + MODULE_TYPE =3D DXE_DRIVER > > > + VERSION_STRING =3D 1.0 > > > + LIBRARY_CLASS =3D PlatformBootManagerLib|DXE_DRIV= ER > > > + > > > +# > > > +# The following information is for reference only and not required b= y 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 interfaces= . > > > + > > > + Copyright (C) 2015-2016, Red Hat, Inc. > > > + Copyright (c) 2014 - 2019, ARM Ltd. All rights reserved.
> > > + Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.=
> > > + 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 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 applic= ations > > > + built into firmware volumes. > > > + > > > + If this function returns, BDS attempts to enter an infinite loop. > > > +**/ > > > +VOID > > > +EFIAPI > > > +PlatformBootManagerUnableToBoot ( > > > + VOID > > > + ) > > > +{ > > > + return; > > > +} > > > -- > > > 2.17.1 > > > > > > > > >=20 > > > > IMPORTANT NOTICE: The contents of this email and any attachments are conf= idential and may also be privileged. If you are not the intended recipient,= please notify the sender immediately and do not disclose the contents to a= ny other person, use it for any purpose, or store or copy the information i= n any medium. Thank you.