From: "Pete Batard" <pete@akeo.ie>
To: Sunny Wang <Sunny.Wang@arm.com>, devel@edk2.groups.io
Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>,
Jeremy Linton <jeremy.linton@arm.com>,
Sami Mujawar <sami.mujawar@arm.com>,
Ard Biesheuvel <ardb+tianocore@kernel.org>
Subject: Re: [PATCH v2 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot
Date: Tue, 13 Apr 2021 13:12:57 +0100 [thread overview]
Message-ID: <b8d3227d-936c-da5e-5806-518653ec9d2f@akeo.ie> (raw)
In-Reply-To: <20210413071449.383-1-Sunny.Wang@arm.com>
Thanks for the v2. Everything looks good now.
On 2021.04.13 08:14, Sunny Wang wrote:
> This is a fix for https://github.com/pftf/RPi4/issues/114.
>
> Changes:
> 1. Add a setup option called Boot Policy and consume the setting
> during boot to whether perform or skip ConnectAll.
> 2. The Default setting is set to Full discovery because it is not
> worth enabling Fast boot by default on RaspberryPi systems.
> Enabling it just saves boot time about 1 second, but caused a
> lot of issues.
>
> Testing Done:
> - Booted to Standalone UEFI shell on SD card and use drivers
> command to check the result with Fast Boot and Full discovery
> settings. Then, child/device handles are created as expected.
>
> Note and to-do items:
> - The root cause looks like that boot loaders and some tools like
> grub and iPXE haven't supported selective connect/Fast boot.
> However, system firmware should still provide a setup option for
> user to enable Fast boot with old version boot loaders and tools
> , which is why we proposed this change. We will also report this
> issue to boot loader and tool vendors/open source GitHubs.
> - We Will add more options for connecting specific type devices so
> that we can still have the shortest boot time for all use cases.
>
> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> ---
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 11 ++++++++++-
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 ++-
> .../Drivers/ConfigDxe/ConfigDxeHii.uni | 10 +++++++++-
> .../Drivers/ConfigDxe/ConfigDxeHii.vfr | 16 +++++++++++++++-
> Platform/RaspberryPi/Include/ConfigVars.h | 12 +++++++++++-
> .../Library/PlatformBootManagerLib/PlatformBm.c | 15 +++++++++++++--
> .../PlatformBootManagerLib.inf | 1 +
> Platform/RaspberryPi/RPi3/RPi3.dsc | 9 ++++++++-
> Platform/RaspberryPi/RPi4/RPi4.dsc | 9 ++++++++-
> Platform/RaspberryPi/RaspberryPi.dec | 2 ++
> 10 files changed, 79 insertions(+), 9 deletions(-)
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index 22f86d4d44..d3c5869949 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> @@ -1,6 +1,6 @@
> /** @file
> *
> - * Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
> + * Copyright (c) 2019 - 2021, ARM Limited. All rights reserved.
> * Copyright (c) 2018 - 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
> *
> * SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -281,6 +281,15 @@ SetupVariables (
> );
> }
>
> + Size = sizeof (UINT32);
> + Status = gRT->GetVariable (L"BootPolicy",
> + &gConfigDxeFormSetGuid,
> + NULL, &Size, &Var32);
> + if (EFI_ERROR (Status)) {
> + Status = PcdSet32S (PcdBootPolicy, PcdGet32 (PcdBootPolicy));
> + ASSERT_EFI_ERROR (Status);
> + }
> +
> Size = sizeof (UINT32);
> Status = gRT->GetVariable (L"SdIsArasan",
> &gConfigDxeFormSetGuid,
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> index d51e54e010..032e40b0c3 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> @@ -2,7 +2,7 @@
> #
> # Component description file for the RasbperryPi DXE platform config driver.
> #
> -# Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
> +# Copyright (c) 2019 - 2021, ARM Limited. All rights reserved.
> # Copyright (c) 2018 - 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -93,6 +93,7 @@
> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB
> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio
> gRaspberryPiTokenSpaceGuid.PcdFanTemp
> + gRaspberryPiTokenSpaceGuid.PcdBootPolicy
>
> [Depex]
> gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> index 466fa852cb..81761d64bb 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> @@ -1,7 +1,7 @@
> /** @file
> *
> * Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
> - * Copyright (c) 2020, ARM Limited. All rights reserved.
> + * Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
> *
> * SPDX-License-Identifier: BSD-2-Clause-Patent
> *
> @@ -60,6 +60,14 @@
> #string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
> #string STR_ADVANCED_ASSET_TAG_HELP #language en-US "Set the system Asset Tag"
>
> +#string STR_BOOT_POLICY_PROMPT #language en-US "Boot Policy"
> +#string STR_BOOT_POLICY_HELP #language en-US "When Fast Boot is selected, only required devices will be discovered for reducing "
> + "the boot time. "
> + "When Full Discovery is selected, all the devices will be discovered for some "
> + "scenarios such as system deployment and diagnostic tests."
> +#string STR_FAST_BOOT #language en-US "Fast Boot"
> +#string STR_FULL_DISCOVERY #language en-US "Full Discovery"
> +
> /*
> * MMC/SD configuration.
> */
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> index cc7a09cfb7..aa124e4e31 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> @@ -1,7 +1,7 @@
> /** @file
> *
> * Copyright (c) 2018 Andrei Warkentin <andrey.warkentin@gmail.com>
> - * Copyright (c) 2020, ARM Limited. All rights reserved.
> + * Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
> *
> * SPDX-License-Identifier: BSD-2-Clause-Patent
> *
> @@ -116,6 +116,11 @@ formset
> name = DisplayEnableSShot,
> guid = CONFIGDXE_FORM_SET_GUID;
>
> + efivarstore BOOT_POLICY_VARSTORE_DATA,
> + attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
> + name = BootPolicy,
> + guid = CONFIGDXE_FORM_SET_GUID;
> +
> form formid = 1,
> title = STRING_TOKEN(STR_FORM_SET_TITLE);
> subtitle text = STRING_TOKEN(STR_NULL_STRING);
> @@ -190,6 +195,14 @@ formset
> option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_DT), value = SYSTEM_TABLE_MODE_DT, flags = DEFAULT;
> endoneof;
>
> + oneof varid = BootPolicy.BootPolicy,
> + prompt = STRING_TOKEN(STR_BOOT_POLICY_PROMPT),
> + help = STRING_TOKEN(STR_BOOT_POLICY_HELP),
> + flags = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
> + option text = STRING_TOKEN(STR_FAST_BOOT), value = FAST_BOOT , flags = 0;
> + option text = STRING_TOKEN(STR_FULL_DISCOVERY), value = FULL_DISCOVERY, flags = DEFAULT;
> + endoneof;
> +
> #if (RPI_MODEL == 4)
> grayoutif NOT ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
> oneof varid = FanOnGpio.Enabled,
> @@ -220,6 +233,7 @@ formset
> minsize = 0,
> maxsize = ASSET_TAG_STR_MAX_LEN,
> endstring;
> +
> endform;
>
> form formid = 0x1003,
> diff --git a/Platform/RaspberryPi/Include/ConfigVars.h b/Platform/RaspberryPi/Include/ConfigVars.h
> index 142317985a..9ef62b7a6e 100644
> --- a/Platform/RaspberryPi/Include/ConfigVars.h
> +++ b/Platform/RaspberryPi/Include/ConfigVars.h
> @@ -1,7 +1,7 @@
> /** @file
> *
> * Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
> - * Copyright (c) 2020, ARM Limited. All rights reserved.
> + * Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
> *
> * SPDX-License-Identifier: BSD-2-Clause-Patent
> *
> @@ -143,4 +143,14 @@ typedef struct {
> UINT32 EnableDma;
> } MMC_EMMC_DMA_VARSTORE_DATA;
>
> +#define FAST_BOOT 0
> +#define FULL_DISCOVERY 1
> +typedef struct {
> + /*
> + * 0 - Fast Boot
> + * 1 - Full Discovery (Connect All)
> + */
> + UINT32 BootPolicy;
> +} BOOT_POLICY_VARSTORE_DATA;
> +
> #endif /* CONFIG_VARS_H */
> diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> index c2fc40b8ea..d081fdae63 100644
> --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -4,7 +4,7 @@
> * Copyright (c) 2017-2018, Andrei Warkentin <andrey.warkentin@gmail.com>
> * Copyright (c) 2016, Linaro Ltd. All rights reserved.
> * Copyright (c) 2015-2016, Red Hat, Inc.
> - * Copyright (c) 2014-2020, ARM Ltd. All rights reserved.
> + * Copyright (c) 2014-2021, ARM Ltd. All rights reserved.
> * Copyright (c) 2004-2016, Intel Corporation. All rights reserved.
> *
> * SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -25,10 +25,11 @@
> #include <Protocol/LoadedImage.h>
> #include <Guid/EventGroup.h>
> #include <Guid/TtyTerm.h>
> +#include <ConfigVars.h>
>
> #include "PlatformBm.h"
>
> -#define BOOT_PROMPT L"ESC (setup), F1 (shell), ENTER (boot)"
> +#define BOOT_PROMPT L"ESC (setup), F1 (shell), ENTER (boot)\n"
>
> #define DP_NODE_LEN(Type) { (UINT8)sizeof (Type), (UINT8)(sizeof (Type) >> 8) }
>
> @@ -633,6 +634,16 @@ PlatformBootManagerAfterConsole (
> Print (BOOT_PROMPT);
> }
>
> + //
> + // Connect the rest of the devices if the boot polcy is set to Full discovery
> + //
> + if (PcdGet32 (PcdBootPolicy) == FULL_DISCOVERY) {
> + DEBUG ((DEBUG_INFO, "Boot Policy is Full Discovery. Connect all devices\n"));
> + EfiBootManagerConnectAll ();
> + } else if (PcdGet32 (PcdBootPolicy) == FAST_BOOT) {
> + DEBUG ((DEBUG_INFO, "Boot Policy is Fast Boot. Skip connecting all devices\n"));
> + }
> +
> Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL, (VOID**)&EsrtManagement);
> if (!EFI_ERROR (Status)) {
> EsrtManagement->SyncEsrtFmp ();
> diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index 88f6f8fe09..fbf510ab96 100644
> --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -63,6 +63,7 @@
> [Pcd]
> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
> gRaspberryPiTokenSpaceGuid.PcdSdIsArasan
> + gRaspberryPiTokenSpaceGuid.PcdBootPolicy
>
> [Guids]
> gEfiFileInfoGuid
> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
> index 0961133ae9..425c7ff9ec 100644
> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc
> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
> @@ -1,6 +1,6 @@
> # @file
> #
> -# Copyright (c) 2011 - 2020, ARM Limited. All rights reserved.
> +# Copyright (c) 2011 - 2021, ARM Limited. All rights reserved.
> # Copyright (c) 2014, Linaro Limited. All rights reserved.
> # Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.
> # Copyright (c) 2017 - 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
> @@ -512,6 +512,13 @@
> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0
> gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSetGuid|0x0|0
>
> + #
> + # Boot Policy
> + # 0 - Fast Boot
> + # 1 - Full Discovery (Connect All)
> + #
> + gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeFormSetGuid|0x0|1
> +
> #
> # Reset-related.
> #
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index ff802d8347..5c6783eae7 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -1,6 +1,6 @@
> # @file
> #
> -# Copyright (c) 2011 - 2020, ARM Limited. All rights reserved.
> +# Copyright (c) 2011 - 2021, ARM Limited. All rights reserved.
> # Copyright (c) 2017 - 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
> # Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.
> # Copyright (c) 2014, Linaro Limited. All rights reserved.
> @@ -528,6 +528,13 @@
> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0
> gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSetGuid|0x0|60
>
> + #
> + # Boot Policy
> + # 0 - Fast Boot
> + # 1 - Full Discovery (Connect All)
> + #
> + gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeFormSetGuid|0x0|1
> +
> #
> # Reset-related.
> #
> diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec
> index 08135717ed..8eb1c2bac7 100644
> --- a/Platform/RaspberryPi/RaspberryPi.dec
> +++ b/Platform/RaspberryPi/RaspberryPi.dec
> @@ -2,6 +2,7 @@
> #
> # Copyright (c) 2016, Linaro, Ltd. All rights reserved.
> # Copyright (c) 2017-2018, Andrei Warkentin <andrey.warkentin@gmail.com>
> +# Copyright (c) 2021, ARM Limited. All rights reserved.
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -70,3 +71,4 @@
> gRaspberryPiTokenSpaceGuid.PcdFanTemp|0|UINT32|0x0000001D
> gRaspberryPiTokenSpaceGuid.PcdPlatformResetDelay|0|UINT32|0x0000001E
> gRaspberryPiTokenSpaceGuid.PcdMmcEnableDma|0|UINT32|0x0000001F
> + gRaspberryPiTokenSpaceGuid.PcdBootPolicy|0|UINT32|0x00000020
>
Reviewed-by: Pete Batard <pete@akeo.ie>
next prev parent reply other threads:[~2021-04-13 12:13 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-13 7:14 [PATCH v2 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot Sunny Wang
2021-04-13 12:12 ` Pete Batard [this message]
2021-04-13 15:22 ` Ard Biesheuvel
2021-04-13 15:24 ` Pete Batard
2021-04-15 18:50 ` Ard Biesheuvel
-- strict thread matches above, loose matches on Subject: below --
2021-04-13 4:20 Sunny Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b8d3227d-936c-da5e-5806-518653ec9d2f@akeo.ie \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox