From: "Pete Batard" <pete@akeo.ie>
To: Ard Biesheuvel <ardb@kernel.org>
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 16:24:48 +0100 [thread overview]
Message-ID: <e15b6865-03a1-cd32-0a44-9ebaca36803f@akeo.ie> (raw)
In-Reply-To: <CAMj1kXHMBFejByLtZN06HeB3z3YJ6u2mjd-Schmgs9atGXLjfQ@mail.gmail.com>
On 2021.04.13 16:22, Ard Biesheuvel wrote:
> 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?
>
The one I reviewed is the most recent I saw on the mailing list.
I believe both v2 are identical though.
Regards,
/Pete
next prev parent reply other threads:[~2021-04-13 15:24 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
2021-04-13 15:24 ` Pete Batard [this message]
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=e15b6865-03a1-cd32-0a44-9ebaca36803f@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