[edk2-devel] [PATCH v2 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot

Ard Biesheuvel ardb at kernel.org
Thu Apr 15 18:50:40 UTC 2021


On Tue, 13 Apr 2021 at 17:24, Pete Batard <pete at akeo.ie> wrote:
>
> On 2021.04.13 16:22, Ard Biesheuvel wrote:
> > On Tue, 13 Apr 2021 at 14:13, Pete Batard <pete at 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 at arm.com>
> >>> Cc: Jeremy Linton <jeremy.linton at arm.com>
> >>> Cc: Sami Mujawar <sami.mujawar at arm.com>
> >>> Cc: Pete Batard <pete at akeo.ie>
> >>> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
> >>> Signed-off-by: Sunny Wang <sunny.wang at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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.
>

OK

Pushed as 7545558886de..efdc159ef7c9

Thanks all,


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74137): https://edk2.groups.io/g/devel/message/74137
Mute This Topic: https://groups.io/mt/82057467/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list