From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f47.google.com (mail-wm1-f47.google.com [209.85.128.47]) by mx.groups.io with SMTP id smtpd.web11.8162.1618315981263029317 for ; Tue, 13 Apr 2021 05:13:01 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=FCTDxDU0; spf=pass (domain: akeo.ie, ip: 209.85.128.47, mailfrom: pete@akeo.ie) Received: by mail-wm1-f47.google.com with SMTP id y204so7210744wmg.2 for ; Tue, 13 Apr 2021 05:13:01 -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=gheTx5ML1ydU79z95kSsv+tpTAVPc1i4E04EODL20JU=; b=FCTDxDU0dc0yDJc01rCYFbPMsOWUx7PhLluktHpPJk0923MGCBmAXzS+1J/C84tTwx dL5V6EbnAqs3SS1ECQB0mk+obvXXKXM/JArOWP1yuFED12S5G8aCAAy3qw+Axdx+4tDq 8w10FHCInhPa5x4skAHGhXEvPhdUU0ckCFLTCg6uyjlInDl3N8vxKhVOFR6K2El6gMoQ f7+DTB1mVITFtMHGLqO3wepjPakM+MKvy/HpbYoezRBdFOpOFJxWhaDzQFHxTdFfG92e aT9tBRMYZ/3Sb1muFoats2hWAj0VSNs3aUv/0wPdy1wwDS3NbOifUWOBWWv4EAxFIxn2 g7LA== 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=gheTx5ML1ydU79z95kSsv+tpTAVPc1i4E04EODL20JU=; b=Z89m1ZW+B9mIRr85I0Nr/BT3qYx0KRoCcW4693C0t86HdsbYDorxH6ayPxlGB77ZN0 6IOMmAhszQamx3RK3XFrye+9fv2BGxPYatBQBo6VvwOW44VdBIcmSOfC8XsKUlTkPEZx n6wNNnfOOvDhGvFc9gzunMKfz1fLjsNFfGcOJdwoGzB0Rl1G5KCYE620dfaS9/boxFKL WMVZmI9851xv2RgffBqwbRbM/09OVfcSQDRh8q6C3LUTrdSOT/XCVTwQ45LE00at+ptE tSYTaql2hKS3Xqr3Zz9TLzRM9A5/bVLBl6WkP3fnp09MX/Erak3EurIkV2NJYY0YjTl/ sFXA== X-Gm-Message-State: AOAM533PFD7LOAxCk5NxCbE3rEJ8L9E4j+lbkpXXODONUHlVDSS0stwI dmmBGeZXbllFMt17xJuJoY/+eg== X-Google-Smtp-Source: ABdhPJwCWXctdHtWxfbFhdWvG2bpVe9DFoXHl8rhSCUi+03RxclgnG1hzQd1tp0VKsMjn32GOJM6aw== X-Received: by 2002:a05:600c:4f14:: with SMTP id l20mr3798720wmq.71.1618315979453; Tue, 13 Apr 2021 05:12:59 -0700 (PDT) Return-Path: Received: from [10.0.0.122] ([84.203.85.228]) by smtp.googlemail.com with ESMTPSA id u8sm20879669wrr.42.2021.04.13.05.12.58 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 13 Apr 2021 05:12:58 -0700 (PDT) Subject: Re: [PATCH v2 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot To: Sunny Wang , devel@edk2.groups.io Cc: 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 13:12:57 +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: <20210413071449.383-1-Sunny.Wang@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit 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