From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web08.1812.1618512653499200880 for ; Thu, 15 Apr 2021 11:50:53 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=MPtc2qIc; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id AE8F8610FA for ; Thu, 15 Apr 2021 18:50:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1618512652; bh=lKDJXtQdTpiLHpbMrJJ8+vlL8H+x/oL27w7StepOAG8=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=MPtc2qIcslCDBmhg/sbI4eL1S8M7oSFjijRgGQsrL577UUv8YY9h16Oe4lcSVVPJr mXxB1FcL2526JA5brxdyIkmjvwLpBfvid/uBxWPhZH5QEqWjI3MRFvGCj8s+yL3nyo +vZPWPvdh7N3CVtt1h/R7Wpwz/J3AvjNJdgatkWUg3S+c1f7eP1Hix80oaKf45UTiD xnhVaEd6m6TN11+7J5GMi7ASKejqwiXWI/UgbH5W/eUlMEGjIm/A9sUsDddoIoVmxu AiiKe3DaJUNoDxx9euRUlzEDYAgCsV9JAMbVGi69QIeJxFD1XBc2PUdz0EFF/6149N 7csA6Pr8aRsWQ== Received: by mail-ot1-f41.google.com with SMTP id o13-20020a9d404d0000b029028e0a0ae6b4so4466331oti.10 for ; Thu, 15 Apr 2021 11:50:52 -0700 (PDT) X-Gm-Message-State: AOAM531pkv4OCMQCy0himAjVXcU7vAzphi02N8YBEwt9BlN7bBT/XwRp +mkhcz08+onb4grmgggq7aTUFDdebpBgRY7Qdto= X-Google-Smtp-Source: ABdhPJw8j2YYAlVpsJhodmQLAcAPmO0Mao7PqKBcCwwX4P6gHLxZPT1airniNqTGkWa85czt4xvWt8wn1ZE+Rk5g5eY= X-Received: by 2002:a9d:12cb:: with SMTP id g69mr539784otg.77.1618512651909; Thu, 15 Apr 2021 11:50:51 -0700 (PDT) MIME-Version: 1.0 References: <20210413071449.383-1-Sunny.Wang@arm.com> In-Reply-To: From: "Ard Biesheuvel" Date: Thu, 15 Apr 2021 20:50:40 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot To: Pete Batard Cc: Sunny Wang , edk2-devel-groups-io , Samer El-Haj-Mahmoud , Jeremy Linton , Sami Mujawar , Ard Biesheuvel Content-Type: text/plain; charset="UTF-8" On Tue, 13 Apr 2021 at 17:24, Pete Batard wrote: > > On 2021.04.13 16:22, Ard Biesheuvel wrote: > > On Tue, 13 Apr 2021 at 14:13, Pete Batard 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 > >>> Cc: Jeremy Linton > >>> Cc: Sami Mujawar > >>> Cc: Pete Batard > >>> Cc: Ard Biesheuvel > >>> Signed-off-by: Sunny Wang > >>> --- > >>> .../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 > >>> * > >>> * 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 > >>> # > >>> # 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 > >>> - * 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 > >>> - * 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 > >>> - * 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 > >>> * 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 > >>> #include > >>> #include > >>> +#include > >>> > >>> #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 > >>> @@ -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 > >>> # 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 > >>> +# 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 > > > > 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,