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.web10.75.1618327336291844808 for ; Tue, 13 Apr 2021 08:22:16 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=p+onsmHA; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id 128D9610CA for ; Tue, 13 Apr 2021 15:22:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1618327335; bh=tz46BNriAZxm2uYmLf9tqy61DICRMiLaQA5brcS2v2E=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=p+onsmHAw6xbFUipgUARVZHxTDDz9pHJp22lDiZkIRdXNAlFOt2crQbKpWiDjMzH3 pyaexoSoxQJQdR6SxCq5EbixUBEN7Gymj6MKqYVLIulz89QR/EXc8CoryzXIsNnD+z 8y/vepR2omu9npMEeWmtyiTW/ExQMAWZziIJOXFk62/v3X2CXzpSWxV1BOQGR5389u jKuRxAJG8RiLOfKmRV+rTf9c3xUOFfnur726p5UjALx5JHCiVKhjYGkgB5ls1ossDR bJdppE8PE9k5k2FSk3uA4F9c1JzyeBEdsSQ0jG1c9L9bYuY3+jfJxDgn1BpmZelJ3k vctPaIlDSc8oA== Received: by mail-ot1-f53.google.com with SMTP id w21-20020a9d63950000b02901ce7b8c45b4so16409030otk.5 for ; Tue, 13 Apr 2021 08:22:15 -0700 (PDT) X-Gm-Message-State: AOAM532twcoIMq/7/wQIhM86mxRlhTrxQhGRjZFwblQdIInm/eG8C0jV MZ9FeN5vHV83byvMQcvRx3d/tguXzvE8YBPzDE0= X-Google-Smtp-Source: ABdhPJykS1xyqsFypVnoVwCtD/bbGu62yb64WK+NWSMFERfWXbkb7WJ/hOzzA8DgPQVSa+Aq0oviqz2hM5xztal0OKA= X-Received: by 2002:a9d:12cb:: with SMTP id g69mr28120456otg.77.1618327334294; Tue, 13 Apr 2021 08:22:14 -0700 (PDT) MIME-Version: 1.0 References: <20210413071449.383-1-Sunny.Wang@arm.com> In-Reply-To: From: "Ard Biesheuvel" Date: Tue, 13 Apr 2021 17:22:02 +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 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?