From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f51.google.com (mail-wm1-f51.google.com [209.85.128.51]) by mx.groups.io with SMTP id smtpd.web11.108.1618327491377700915 for ; Tue, 13 Apr 2021 08:24:52 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=W8kgtLSS; spf=pass (domain: akeo.ie, ip: 209.85.128.51, mailfrom: pete@akeo.ie) Received: by mail-wm1-f51.google.com with SMTP id u20so4424436wmj.0 for ; Tue, 13 Apr 2021 08:24:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=Tv9UEV+8z42vIsItXgx6nSRlaCOTkMHTYuCJkOsm7/E=; b=W8kgtLSSfCVcEh/ik1m8ZXejoKD2qez/GRPq4sZoQFUWzOFU0AM97u+ogfFiVZM/72 DP/zOmK9UQHta23xVahURRMrjPRLRsKkzfMKSz2H6M0ZITSscY9SSFMTsu7wmx9MyCf1 7AfIAfLlZcr8YFYD6WOZq6qvJZW2Xb/VTrS/h/0noMMSYKIZD6Bk5CC0O1QUhAAT0Dhd IPwOkQ44CErC6Jhcg4pigFueOwGff/1Dpv05tsj8hYRF3B3y/AUKFn7GV1JL5QHWzKRv s2NgM0mK4Q4vv0xLa/Ob6vmcINSlLz4j9l9raX/vVNpSU00ni3tSx2zOKBl0pSnsph1c Sb5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=Tv9UEV+8z42vIsItXgx6nSRlaCOTkMHTYuCJkOsm7/E=; b=gMtsTokDQnEXaX4s21L+eVF9mJZlGKT8r0nhH7xuDi3qBey3qcdAPRPRHqJr4d0QVZ ip0t67t6g8CXLL9O9b5DN/vu97igkQlVF35aMhgKdQ0xSuAXNO41chT3e8MPfV0zEPIs Aobqw/p/3594IrOOtfW87s9nPnaPc2/A+nqREsFS7puEGsF/NszAcwdbHEKRhCspIt3o XGTGX4W2NCrBL++IYFdInANWWOmvSYgQpCRPecY564cz7rfj1Qrogrd0XoNxIaRHSmrQ fp0/eAIPC069TPX3hPZq2+GAqgG6e6rBPDA9pQrzBvyHEMPeywweXpdAGhmlhcoU9hnw Bn5Q== X-Gm-Message-State: AOAM532Z6lfRbIxJU926wP3XtSRoCnajtg8AiSnHyJ6nSde2TGZ897/c CyC0LYaVBjk8mFc819FmRJCuMQ== X-Google-Smtp-Source: ABdhPJwjjeEZpTprlvLrPrCbWlTWpksVIvQy/Xwzz6j3hkgbVu3TccTwm/vx2+D76R/dS4CF9dr3GA== X-Received: by 2002:a1c:bad6:: with SMTP id k205mr575824wmf.10.1618327489855; Tue, 13 Apr 2021 08:24:49 -0700 (PDT) Return-Path: Received: from [10.0.0.122] ([84.203.85.228]) by smtp.googlemail.com with ESMTPSA id t63sm2716792wma.20.2021.04.13.08.24.48 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 13 Apr 2021 08:24:49 -0700 (PDT) Subject: Re: [PATCH v2 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot To: Ard Biesheuvel Cc: Sunny Wang , edk2-devel-groups-io , Samer El-Haj-Mahmoud , Jeremy Linton , Sami Mujawar , Ard Biesheuvel References: <20210413071449.383-1-Sunny.Wang@arm.com> From: "Pete Batard" Message-ID: Date: Tue, 13 Apr 2021 16:24:48 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit 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. Regards, /Pete