From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web10.14668.1682089055054717993 for ; Fri, 21 Apr 2023 07:57:35 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=KvEFuaHy; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 7126D6114C for ; Fri, 21 Apr 2023 14:57:34 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D68C3C433EF for ; Fri, 21 Apr 2023 14:57:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1682089053; bh=2n6Nw57VYBZ9Ltf1t7xqH5d3t+WyZ9FQL+EzcPp2wY8=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=KvEFuaHy353l/CRQjUa6WWmtUEAl7zSTsNSt9uAJYwAMa5LE09TrMdAby9sq2HY8f /TQ668qax08CEDn6IMD61X/Oe/18a99AJ7Z12f1c5TAlio16GtgBjZ7llPqZ83uHz5 zzitPBYcGpVIlh22UpMfgjhbGtnDv8hZJGUmHZFzMzx8wLMWHBzCX0pz4+nefOnGBo 2L6ecHdmqDk8m1w0d35BTTmNlO1ZwGdXo7Pv2+7LBzAPLLib5KFZIrR/fuIrmO62SB kzFs1zIMU2VBB8liUa4n50IBPzbuFZpSrKyCqVJPdKwLN5uAJzjhDH1QD6IQvSDckX KAhIlUcXi9mWQ== Received: by mail-lf1-f50.google.com with SMTP id 2adb3069b0e04-4ec94eb6dcaso1839144e87.3 for ; Fri, 21 Apr 2023 07:57:33 -0700 (PDT) X-Gm-Message-State: AAQBX9fjyrnAxO1B3Ex7PPa551lBswWxHsOaWSXSL2j+KbgS7mC9xT+Y zGldHYe0ksSGivepGGrIermlXMqaTJvytqBsmpI= X-Google-Smtp-Source: AKy350ZJUoNPGjnauc0g7ZS9rryg5usWy0/cFDAu+W5wKHtB8vAHbEQRoUgk2QiLqJRW+Wa1O1AdNd4+p53/a+ZqAzI= X-Received: by 2002:a19:c518:0:b0:4eb:3b58:c558 with SMTP id w24-20020a19c518000000b004eb3b58c558mr1312282lfe.59.1682089051802; Fri, 21 Apr 2023 07:57:31 -0700 (PDT) MIME-Version: 1.0 References: <20230207090653.395992-1-pierre.gondois@arm.com> In-Reply-To: <20230207090653.395992-1-pierre.gondois@arm.com> From: "Ard Biesheuvel" Date: Fri, 21 Apr 2023 16:57:20 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp To: pierre.gondois@arm.com Cc: devel@edk2.groups.io, ardb+tianocore@kernel.org, leif@nuviainc.com, sami.mujawar@arm.com, patrik.berglund@arm.com Content-Type: text/plain; charset="UTF-8" On Tue, 7 Feb 2023 at 10:07, wrote: > > From: Pierre Gondois > > The UEFI Shell is a non-active boot option, at the opposite of UiApp. > If no valid boot option is found, UiApp is selected. UiApp requires a > human interaction. When installing a new EDKII image in CIs or when > scripting is required, this is problematic. > > If no valid boot option is discovered, add a path to directly go to > the UEFI Shell where the startup.nsh script is automatically executed. > The UEFI Shell is launched after connecting possible devices, but > before the reset that is meant to automatically make them visible. > > The new PcdUefiShellDefaultBootEnable must be set to TRUE to enable > this behaviour. The Pcd is set to false by default. > > Signed-off-by: Pierre Gondois > --- > ArmPkg/ArmPkg.dec | 9 ++- > .../PlatformBootManagerLib/PlatformBm.c | 69 ++++++++++++++++++- > .../PlatformBootManagerLib.inf | 4 +- > 3 files changed, 79 insertions(+), 3 deletions(-) > > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec > index f17ba913e6de..2444457ae58a 100644 > --- a/ArmPkg/ArmPkg.dec > +++ b/ArmPkg/ArmPkg.dec > @@ -2,7 +2,7 @@ > # ARM processor package. > # > # Copyright (c) 2009 - 2010, Apple Inc. All rights reserved.
> -# Copyright (c) 2011 - 2022, ARM Limited. All rights reserved. > +# Copyright (c) 2011 - 2023, ARM Limited. All rights reserved. > # Copyright (c) 2021, Ampere Computing LLC. All rights reserved. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > @@ -221,6 +221,13 @@ [PcdsFixedAtBuild.common] > # > gArmTokenSpaceGuid.PcdArmDmaDeviceOffset|0x0|UINT64|0x0000044 > > + # > + # Boot the Uefi Shell instead of UiApp when no valid boot option is found. > + # This is useful in CI environment so that startup.nsh can be launched. > + # The default value is FALSE. > + # > + gArmTokenSpaceGuid.PcdUefiShellDefaultBootEnable|FALSE|BOOLEAN|0x0000052 > + > [PcdsFixedAtBuild.common, PcdsPatchableInModule.common] > gArmTokenSpaceGuid.PcdFdBaseAddress|0|UINT64|0x0000002B > gArmTokenSpaceGuid.PcdFvBaseAddress|0|UINT64|0x0000002D > diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > index 2fb1a4aa4fb8..9bdc44d86b54 100644 > --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > @@ -2,7 +2,7 @@ > Implementation for PlatformBootManagerLib library class interfaces. > > Copyright (C) 2015-2016, Red Hat, Inc. > - Copyright (c) 2014 - 2021, ARM Ltd. All rights reserved.
> + Copyright (c) 2014 - 2023, Arm Ltd. All rights reserved.
> Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.
> Copyright (c) 2016, Linaro Ltd. All rights reserved.
> Copyright (c) 2021, Semihalf All rights reserved.
> @@ -470,6 +470,61 @@ PlatformRegisterFvBootOption ( > EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount); > } > > +/** Boot a Fv Boot Option. > + * > + * This function is useful for booting the UEFI Shell as it is loaded > + * as a non active boot option. > + * > + * @param[in] FileGuid The File GUID. > + * @param[in] Description String describing the Boot Option. > + */ > +STATIC > +VOID > +PlatformBootFvBootOption ( > + IN CONST EFI_GUID *FileGuid, > + IN CHAR16 *Description > + ) > +{ > + EFI_STATUS Status; > + EFI_BOOT_MANAGER_LOAD_OPTION NewOption; > + 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); > + This assumes that the UEFI shell file is in the same FV as the executable that incorporated this library. Can you include a comment on why that is a fair assumption? > + Status = EfiBootManagerInitializeLoadOption ( > + &NewOption, > + LoadOptionNumberUnassigned, > + LoadOptionTypeBoot, > + LOAD_OPTION_ACTIVE, > + Description, > + DevicePath, > + NULL, > + 0 > + ); > + ASSERT_EFI_ERROR (Status); > + FreePool (DevicePath); > + > + for ( ; ;) { > + EfiBootManagerBoot (&NewOption); Why the endless loop? > + } > +} > + > STATIC > VOID > GetPlatformOptions ( > @@ -1075,6 +1130,18 @@ PlatformBootManagerUnableToBoot ( > EfiBootManagerConnectAll (); > EfiBootManagerRefreshAllBootOption (); > > + // > + // Boot the 'UEFI Shell'. If the Pcd is not set, the UEFI Shell is not > + // an active boot option and must be manually selected through UiApp > + // (at least during the fist boot). > + // > + if (FixedPcdGetBool (PcdUefiShellDefaultBootEnable)) { > + PlatformBootFvBootOption ( > + &gUefiShellFileGuid, > + L"UEFI Shell (default)" > + ); > + } > + > // > // Record the updated number of boot configured boot options > // > diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > index 86751b45f82b..05ed46456cc4 100644 > --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > @@ -2,7 +2,7 @@ > # Implementation for PlatformBootManagerLib library class interfaces. > # > # Copyright (C) 2015-2016, Red Hat, Inc. > -# Copyright (c) 2014, ARM Ltd. All rights reserved.
> +# Copyright (c) 2014 - 2023, Arm Ltd. All rights reserved.
> # Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.
> # Copyright (c) 2016, Linaro Ltd. All rights reserved.
> # > @@ -29,6 +29,7 @@ [Sources] > PlatformBm.h > > [Packages] > + ArmPkg/ArmPkg.dec > EmbeddedPkg/EmbeddedPkg.dec > MdeModulePkg/MdeModulePkg.dec > MdePkg/MdePkg.dec > @@ -55,6 +56,7 @@ [FeaturePcd] > gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport > > [FixedPcd] > + gArmTokenSpaceGuid.PcdUefiShellDefaultBootEnable > gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString > gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate > -- > 2.25.1 >