From: "Ard Biesheuvel" <ardb@kernel.org>
To: Pete Batard <pete@akeo.ie>
Cc: Sunny Wang <Sunny.Wang@arm.com>,
edk2-devel-groups-io <devel@edk2.groups.io>,
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 17:22:02 +0200 [thread overview]
Message-ID: <CAMj1kXHMBFejByLtZN06HeB3z3YJ6u2mjd-Schmgs9atGXLjfQ@mail.gmail.com> (raw)
In-Reply-To: <b8d3227d-936c-da5e-5806-518653ec9d2f@akeo.ie>
On Tue, 13 Apr 2021 at 14:13, Pete Batard <pete@akeo.ie> wrote:
>
> 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>
Thanks Pete.
So which patch is the one I should be applying?
next prev parent reply other threads:[~2021-04-13 15:22 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
2021-04-13 15:22 ` Ard Biesheuvel [this message]
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=CAMj1kXHMBFejByLtZN06HeB3z3YJ6u2mjd-Schmgs9atGXLjfQ@mail.gmail.com \
--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