* Re: [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp
2023-02-07 9:06 [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp PierreGondois
@ 2023-02-09 16:57 ` Ard Biesheuvel
2023-02-13 8:39 ` PierreGondois
` (2 more replies)
2023-02-09 17:14 ` Marcin Juszkiewicz
` (3 subsequent siblings)
4 siblings, 3 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2023-02-09 16:57 UTC (permalink / raw)
To: pierre.gondois; +Cc: devel, ardb+tianocore, leif, sami.mujawar, patrik.berglund
On Tue, 7 Feb 2023 at 10:07, <pierre.gondois@arm.com> wrote:
>
> From: Pierre Gondois <pierre.gondois@arm.com>
>
> 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.
>
Is this similar to how we implemented this on RPi4? IIRC, a similar
issue came up there as well.
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
> 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.<BR>
> -# 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.<BR>
> + Copyright (c) 2014 - 2023, 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) 2021, Semihalf All rights reserved.<BR>
> @@ -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);
> +
> + Status = EfiBootManagerInitializeLoadOption (
> + &NewOption,
> + LoadOptionNumberUnassigned,
> + LoadOptionTypeBoot,
> + LOAD_OPTION_ACTIVE,
> + Description,
> + DevicePath,
> + NULL,
> + 0
> + );
> + ASSERT_EFI_ERROR (Status);
> + FreePool (DevicePath);
> +
> + for ( ; ;) {
> + EfiBootManagerBoot (&NewOption);
> + }
> +}
> +
> 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.<BR>
> +# Copyright (c) 2014 - 2023, 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>
> #
> @@ -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
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp
2023-02-09 16:57 ` Ard Biesheuvel
@ 2023-02-13 8:39 ` PierreGondois
2023-03-31 8:47 ` PierreGondois
2023-04-14 12:45 ` PierreGondois
[not found] ` <1755CDCBA23EBCF9.16809@groups.io>
2 siblings, 1 reply; 12+ messages in thread
From: PierreGondois @ 2023-02-13 8:39 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: devel, ardb+tianocore, leif, sami.mujawar, patrik.berglund,
marcin.juszkiewicz
Hello Ard,
On 2/9/23 17:57, Ard Biesheuvel wrote:
> On Tue, 7 Feb 2023 at 10:07, <pierre.gondois@arm.com> wrote:
>>
>> From: Pierre Gondois <pierre.gondois@arm.com>
>>
>> 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.
>>
>
> Is this similar to how we implemented this on RPi4? IIRC, a similar
> issue came up there as well.
I'm not finding an equivalent for the Rpi4. I see that the
BootDiscoveryPolicy was added, but this regards which devices to connect
during boot IIUC. The UEFI Shell also seems to have been added to the
Boot Manager Menu, but this should not select the UEFI Shell by default.
Regards,
Pierre
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp
2023-02-13 8:39 ` PierreGondois
@ 2023-03-31 8:47 ` PierreGondois
2023-04-14 12:22 ` [edk2-devel] " PierreGondois
0 siblings, 1 reply; 12+ messages in thread
From: PierreGondois @ 2023-03-31 8:47 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: devel, ardb+tianocore, leif, sami.mujawar, patrik.berglund,
marcin.juszkiewicz
Hello Ard,
On 2/13/23 09:39, Pierre Gondois wrote:
> Hello Ard,
>
> On 2/9/23 17:57, Ard Biesheuvel wrote:
>> On Tue, 7 Feb 2023 at 10:07, <pierre.gondois@arm.com> wrote:
>>>
>>> From: Pierre Gondois <pierre.gondois@arm.com>
>>>
>>> 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.
>>>
>>
>> Is this similar to how we implemented this on RPi4? IIRC, a similar
>> issue came up there as well.
>
> I'm not finding an equivalent for the Rpi4. I see that the
> BootDiscoveryPolicy was added, but this regards which devices to connect
> during boot IIUC. The UEFI Shell also seems to have been added to the
> Boot Manager Menu, but this should not select the UEFI Shell by default.
>
Are you referring to the following ?
https://edk2.groups.io/g/devel/message/60719
If yes I don't think this patch aims to achieve the same thing.
The goal here is to reach the UEFI shell (i.e. make it an active option).
I believe the patch mentioned above doesn't modify that and is more about
making it visible in UiApp BootManager.
Please let me know if you were referring to another modification,
Regards,
Pierre
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp
2023-03-31 8:47 ` PierreGondois
@ 2023-04-14 12:22 ` PierreGondois
0 siblings, 0 replies; 12+ messages in thread
From: PierreGondois @ 2023-04-14 12:22 UTC (permalink / raw)
To: PierreGondois, devel
[-- Attachment #1: Type: text/plain, Size: 635 bytes --]
Hello Ard,
KvmTool, Qemu, Xen and CloudHv use the following implementation:
PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
Testing:
- using kvmtool
- not pressing any key during the process
I end-up in the boot menu (so not the UEFI shell).
When trying with a zero-ed flash.img, kvmtool exits with the following:
'PlatformBootManagerUnableToBoot: rebooting after refreshing all boot options'
Then the flash is populated and the second attempt reaches the boot menu.
With the new PcdUefiShellDefaultBootEnable set to TRUE, we directly reach
the UEFI shell.
Regards,
Pierre
[-- Attachment #2: Type: text/html, Size: 703 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp
2023-02-09 16:57 ` Ard Biesheuvel
2023-02-13 8:39 ` PierreGondois
@ 2023-04-14 12:45 ` PierreGondois
[not found] ` <1755CDCBA23EBCF9.16809@groups.io>
2 siblings, 0 replies; 12+ messages in thread
From: PierreGondois @ 2023-04-14 12:45 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: devel, leif, sami.mujawar, patrik.berglund, Pierre Gondois
Hello Ard,
On 2/9/23 17:57, Ard Biesheuvel wrote:
> On Tue, 7 Feb 2023 at 10:07, <pierre.gondois@arm.com> wrote:
>>
>> From: Pierre Gondois <pierre.gondois@arm.com>
>>
>> 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.
>>
>
> Is this similar to how we implemented this on RPi4? IIRC, a similar
> issue came up there as well.
KvmTool, Qemu, Xen and CloudHv use the following implementation:
PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
Testing:
- using kvmtool
- not pressing any key during the process
I end-up in the boot menu (so not the UEFI shell).
When trying with a zero-ed flash.img, kvmtool exits with the following:
'PlatformBootManagerUnableToBoot: rebooting after refreshing all boot options'
Then the flash is populated and the second attempt reaches the boot menu.
With the new PcdUefiShellDefaultBootEnable set to TRUE, we directly reach
the UEFI shell.
Regards,
Pierre
^ permalink raw reply [flat|nested] 12+ messages in thread[parent not found: <1755CDCBA23EBCF9.16809@groups.io>]
* Re: [edk2-devel] [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp
[not found] ` <1755CDCBA23EBCF9.16809@groups.io>
@ 2023-04-21 8:51 ` PierreGondois
0 siblings, 0 replies; 12+ messages in thread
From: PierreGondois @ 2023-04-21 8:51 UTC (permalink / raw)
To: devel, Ard Biesheuvel; +Cc: leif, sami.mujawar, patrik.berglund
Hi Ard,
Just a ping in case this was forgotten,
Regards,
Pierre
On 4/14/23 14:45, PierreGondois via groups.io wrote:
> Hello Ard,
>
> On 2/9/23 17:57, Ard Biesheuvel wrote:
>> On Tue, 7 Feb 2023 at 10:07, <pierre.gondois@arm.com> wrote:
>>>
>>> From: Pierre Gondois <pierre.gondois@arm.com>
>>>
>>> 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.
>>>
>>
>> Is this similar to how we implemented this on RPi4? IIRC, a similar
>> issue came up there as well.
>
> KvmTool, Qemu, Xen and CloudHv use the following implementation:
> PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>
> Testing:
> - using kvmtool
> - not pressing any key during the process
> I end-up in the boot menu (so not the UEFI shell).
>
> When trying with a zero-ed flash.img, kvmtool exits with the following:
> 'PlatformBootManagerUnableToBoot: rebooting after refreshing all boot options'
> Then the flash is populated and the second attempt reaches the boot menu.
>
> With the new PcdUefiShellDefaultBootEnable set to TRUE, we directly reach
> the UEFI shell.
>
> Regards,
> Pierre
>
>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp
2023-02-07 9:06 [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp PierreGondois
2023-02-09 16:57 ` Ard Biesheuvel
@ 2023-02-09 17:14 ` Marcin Juszkiewicz
2023-02-13 14:27 ` Patrik Berglund
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Marcin Juszkiewicz @ 2023-02-09 17:14 UTC (permalink / raw)
To: devel, pierre.gondois
W dniu 7.02.2023 o 10:06, PierreGondois pisze:
> 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.
I like it. Much better option than having copy of shell.efi as
\boot\bootaa64.efi binary.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp
2023-02-07 9:06 [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp PierreGondois
2023-02-09 16:57 ` Ard Biesheuvel
2023-02-09 17:14 ` Marcin Juszkiewicz
@ 2023-02-13 14:27 ` Patrik Berglund
2023-02-14 13:36 ` Patrik Berglund
2023-04-21 14:57 ` Ard Biesheuvel
4 siblings, 0 replies; 12+ messages in thread
From: Patrik Berglund @ 2023-02-13 14:27 UTC (permalink / raw)
To: pierre.gondois, devel; +Cc: ardb+tianocore, leif, sami.mujawar
Tested the patch on a N1SDP board and it works as intended with
PcdUefiShellDefaultBootEnable as TRUE and FALSE.
Regards,
Patrik
On 2023-02-07 09:06, pierre.gondois@arm.com wrote:
> From: Pierre Gondois <pierre.gondois@arm.com>
>
> 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 <pierre.gondois@arm.com>
> ---
> 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.<BR>
> -# 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.<BR>
> + Copyright (c) 2014 - 2023, 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) 2021, Semihalf All rights reserved.<BR>
> @@ -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);
> +
> + Status = EfiBootManagerInitializeLoadOption (
> + &NewOption,
> + LoadOptionNumberUnassigned,
> + LoadOptionTypeBoot,
> + LOAD_OPTION_ACTIVE,
> + Description,
> + DevicePath,
> + NULL,
> + 0
> + );
> + ASSERT_EFI_ERROR (Status);
> + FreePool (DevicePath);
> +
> + for ( ; ;) {
> + EfiBootManagerBoot (&NewOption);
> + }
> +}
> +
> 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.<BR>
> +# Copyright (c) 2014 - 2023, 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>
> #
> @@ -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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp
2023-02-07 9:06 [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp PierreGondois
` (2 preceding siblings ...)
2023-02-13 14:27 ` Patrik Berglund
@ 2023-02-14 13:36 ` Patrik Berglund
2023-04-21 14:57 ` Ard Biesheuvel
4 siblings, 0 replies; 12+ messages in thread
From: Patrik Berglund @ 2023-02-14 13:36 UTC (permalink / raw)
To: pierre.gondois, devel; +Cc: ardb+tianocore, leif, sami.mujawar
Tested-by: Patrik Berglund <patrik.berglund@arm.com>
Regards,
Patrik
On 2023-02-07 09:06, pierre.gondois@arm.com wrote:
> From: Pierre Gondois <pierre.gondois@arm.com>
>
> 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 <pierre.gondois@arm.com>
> ---
> 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.<BR>
> -# 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.<BR>
> + Copyright (c) 2014 - 2023, 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) 2021, Semihalf All rights reserved.<BR>
> @@ -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);
> +
> + Status = EfiBootManagerInitializeLoadOption (
> + &NewOption,
> + LoadOptionNumberUnassigned,
> + LoadOptionTypeBoot,
> + LOAD_OPTION_ACTIVE,
> + Description,
> + DevicePath,
> + NULL,
> + 0
> + );
> + ASSERT_EFI_ERROR (Status);
> + FreePool (DevicePath);
> +
> + for ( ; ;) {
> + EfiBootManagerBoot (&NewOption);
> + }
> +}
> +
> 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.<BR>
> +# Copyright (c) 2014 - 2023, 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>
> #
> @@ -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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp
2023-02-07 9:06 [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp PierreGondois
` (3 preceding siblings ...)
2023-02-14 13:36 ` Patrik Berglund
@ 2023-04-21 14:57 ` Ard Biesheuvel
2023-04-25 6:57 ` PierreGondois
4 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2023-04-21 14:57 UTC (permalink / raw)
To: pierre.gondois; +Cc: devel, ardb+tianocore, leif, sami.mujawar, patrik.berglund
On Tue, 7 Feb 2023 at 10:07, <pierre.gondois@arm.com> wrote:
>
> From: Pierre Gondois <pierre.gondois@arm.com>
>
> 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 <pierre.gondois@arm.com>
> ---
> 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.<BR>
> -# 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.<BR>
> + Copyright (c) 2014 - 2023, 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) 2021, Semihalf All rights reserved.<BR>
> @@ -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.<BR>
> +# Copyright (c) 2014 - 2023, 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>
> #
> @@ -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
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp
2023-04-21 14:57 ` Ard Biesheuvel
@ 2023-04-25 6:57 ` PierreGondois
0 siblings, 0 replies; 12+ messages in thread
From: PierreGondois @ 2023-04-25 6:57 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: devel, ardb+tianocore, leif, sami.mujawar, patrik.berglund
On 4/21/23 16:57, Ard Biesheuvel wrote:
> On Tue, 7 Feb 2023 at 10:07, <pierre.gondois@arm.com> wrote:
>>
>> From: Pierre Gondois <pierre.gondois@arm.com>
>>
>> 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 <pierre.gondois@arm.com>
>> ---
>> 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.<BR>
>> -# 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.<BR>
>> + Copyright (c) 2014 - 2023, 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) 2021, Semihalf All rights reserved.<BR>
>> @@ -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?
Yes sure, I'm adding the following:
// The UEFI Shell was registered in PlatformRegisterFvBootOption ()
// previously, thus it must still be available in this FV.
>
>> + 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?
It is indeed not necessary, if this fails, we should continue the normal
process.
Regards,
Pierre
^ permalink raw reply [flat|nested] 12+ messages in thread