* [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot
@ 2021-04-12 9:05 Sunny Wang
2021-04-12 13:03 ` Pete Batard
2021-04-12 21:28 ` Samer El-Haj-Mahmoud
0 siblings, 2 replies; 9+ messages in thread
From: Sunny Wang @ 2021-04-12 9:05 UTC (permalink / raw)
To: devel
Cc: Sunny Wang, Samer El-Haj-Mahmoud, Jeremy Linton, Pete Batard,
Ard Biesheuvel, Sunny Wang
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@arm.com>
Cc: Jeremy Linton <jeremy.linton@arm.com>
Cc: Pete Batard <pete@akeo.ie>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Signed-off-by: Sunny Wang <sunny.wang@arm.com>
---
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 11 ++++++++++-
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 ++-
.../Drivers/ConfigDxe/ConfigDxeHii.uni | 10 +++++++++-
.../Drivers/ConfigDxe/ConfigDxeHii.vfr | 15 ++++++++++++++-
Platform/RaspberryPi/Include/ConfigVars.h | 12 +++++++++++-
.../Library/PlatformBootManagerLib/PlatformBm.c | 16 ++++++++++++++--
.../PlatformBootManagerLib.inf | 1 +
Platform/RaspberryPi/RPi3/RPi3.dsc | 10 +++++++++-
Platform/RaspberryPi/RPi4/RPi4.dsc | 9 ++++++++-
Platform/RaspberryPi/RaspberryPi.dec | 2 ++
10 files changed, 80 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@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@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..7b14fdf39f 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@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 discoverd for reducing "
+ "the boot time."
+ "When Full Discovery is selected, all the devices will be discoverd for some "
+ "scenarios such as system deployement 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..5dc558ec08 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@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);
@@ -220,6 +225,14 @@ formset
minsize = 0,
maxsize = ASSET_TAG_STR_MAX_LEN,
endstring;
+
+ 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;
endform;
form formid = 0x1003,
diff --git a/Platform/RaspberryPi/Include/ConfigVars.h b/Platform/RaspberryPi/Include/ConfigVars.h
index 142317985a..3347f899df 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@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 Discovrey (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..abbe4fb3d0 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@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
@@ -26,9 +26,11 @@
#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 +635,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..ddb03e405f 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@gmail.com>
@@ -512,6 +512,14 @@
gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0
gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSetGuid|0x0|0
+ #
+ # Boot Policy
+ # 0 - Fast Boot
+ # 1 - Full Discovrey (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..8ee1922a44 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@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 Discovrey (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@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
--
2.31.0.windows.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot
2021-04-12 9:05 [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot Sunny Wang
@ 2021-04-12 13:03 ` Pete Batard
2021-04-12 17:27 ` Ard Biesheuvel
2021-04-13 4:33 ` Sunny Wang
2021-04-12 21:28 ` Samer El-Haj-Mahmoud
1 sibling, 2 replies; 9+ messages in thread
From: Pete Batard @ 2021-04-12 13:03 UTC (permalink / raw)
To: Sunny Wang, devel; +Cc: Samer El-Haj-Mahmoud, Jeremy Linton, Ard Biesheuvel
Hi Sunny,
Functionaly, I see no issues with this patch, and it's indeed a good
solution to the problem of trying to satisfy everyone while fixing the
issues we are seeing.
There are however a few minor typos and whitespace issues, that I'm
detailing below. So could you please send a v2 to address these?
On 2021.04.12 10:05, 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@arm.com>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> ---
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 11 ++++++++++-
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 ++-
> .../Drivers/ConfigDxe/ConfigDxeHii.uni | 10 +++++++++-
> .../Drivers/ConfigDxe/ConfigDxeHii.vfr | 15 ++++++++++++++-
> Platform/RaspberryPi/Include/ConfigVars.h | 12 +++++++++++-
> .../Library/PlatformBootManagerLib/PlatformBm.c | 16 ++++++++++++++--
> .../PlatformBootManagerLib.inf | 1 +
> Platform/RaspberryPi/RPi3/RPi3.dsc | 10 +++++++++-
> Platform/RaspberryPi/RPi4/RPi4.dsc | 9 ++++++++-
> Platform/RaspberryPi/RaspberryPi.dec | 2 ++
> 10 files changed, 80 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@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@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..7b14fdf39f 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@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"
>
>
Personally, I would move the Boot Policy section between "System Table
Selection" and "ACPI Fan Control" if we try to keep the advanced
settings in the order we think they will be most used.
>
> +#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 discoverd for reducing "
"discoverd" -> "discovered"
>
> + "the boot time."
Missing a space after "the boot time.". It should be "the boot time. ",
else the help text will display as "(...) the boot time.When Full
Discovery (...)"
>
> + "When Full Discovery is selected, all the devices will be discoverd for some "
"discoverd" -> "discovered"
>
> + "scenarios such as system deployement and diagnostic tests"
"deployement" -> "deployment"
>
> +#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..5dc558ec08 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@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);
>
> @@ -220,6 +225,14 @@ formset
> minsize = 0,
>
> maxsize = ASSET_TAG_STR_MAX_LEN,
>
> endstring;
>
> +
>
> + 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;
"STRING_TOKEN(STR_FAST_BOOT )" -> "STRING_TOKEN(STR_FAST_BOOT)"
>
> + option text = STRING_TOKEN(STR_FULL_DISCOVERY), value = FULL_DISCOVERY, flags = DEFAULT;
>
> + endoneof;
>
> endform;
>
>
>
> form formid = 0x1003,
>
> diff --git a/Platform/RaspberryPi/Include/ConfigVars.h b/Platform/RaspberryPi/Include/ConfigVars.h
> index 142317985a..3347f899df 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@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 Discovrey (Connect All)
"Discovrey" -> "Discovery"
>
> + */
>
> + 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..abbe4fb3d0 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@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
>
> @@ -26,9 +26,11 @@
> #include <Guid/EventGroup.h>
>
> #include <Guid/TtyTerm.h>
>
>
>
> +#include <ConfigVars.h>
I'd remove the extra line between <Guid/TtyTerm.h> and <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"
Why are you adding the carriage return? Was there an issue with the
display of this prompt?
>
>
>
> #define DP_NODE_LEN(Type) { (UINT8)sizeof (Type), (UINT8)(sizeof (Type) >> 8) }
>
>
>
> @@ -633,6 +635,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"));
"All" -> "all"
>
> + 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..ddb03e405f 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@gmail.com>
>
> @@ -512,6 +512,14 @@
> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0
>
> gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSetGuid|0x0|0
>
>
>
> + #
>
> + # Boot Policy
>
> + # 0 - Fast Boot
>
> + # 1 - Full Discovrey (Connect All)
"Discovrey" -> "Discovery"
>
> + #
>
> + gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeFormSetGuid|0x0|1
>
> +
>
> +
Please remove the extra line. There should be just one blank like
between the sections.
>
> #
>
> # Reset-related.
>
> #
>
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index ff802d8347..8ee1922a44 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@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 Discovrey (Connect All)
"Discovrey" -> "Discovery"
Regards,
/Pete
>
> + #
>
> + 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@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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot
2021-04-12 13:03 ` Pete Batard
@ 2021-04-12 17:27 ` Ard Biesheuvel
2021-04-13 6:01 ` Sunny Wang
[not found] ` <16755592E0799AE6.2444@groups.io>
2021-04-13 4:33 ` Sunny Wang
1 sibling, 2 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2021-04-12 17:27 UTC (permalink / raw)
To: Pete Batard
Cc: Sunny Wang, edk2-devel-groups-io, Samer El-Haj-Mahmoud,
Jeremy Linton, Ard Biesheuvel
On Mon, 12 Apr 2021 at 15:03, Pete Batard <pete@akeo.ie> wrote:
>
> Hi Sunny,
>
> Functionaly, I see no issues with this patch, and it's indeed a good
> solution to the problem of trying to satisfy everyone while fixing the
> issues we are seeing.
>
> There are however a few minor typos and whitespace issues, that I'm
> detailing below. So could you please send a v2 to address these?
>
Agree with Pete. Thanks for providing a fix for this, and please
incorporate the changes suggested by Pete.
Then, could we *please* figure out a way to get this patch on the
mailing list without Windows whitespace damage? Not sure if anything
changed recently, but not a single EDK2 patch has arrived in my
mailbox recently in a form that I could apply with 'git am'.
Thanks,
Ard.
> On 2021.04.12 10:05, 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@arm.com>
> > Cc: Jeremy Linton <jeremy.linton@arm.com>
> > Cc: Pete Batard <pete@akeo.ie>
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> > ---
> > .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 11 ++++++++++-
> > .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 ++-
> > .../Drivers/ConfigDxe/ConfigDxeHii.uni | 10 +++++++++-
> > .../Drivers/ConfigDxe/ConfigDxeHii.vfr | 15 ++++++++++++++-
> > Platform/RaspberryPi/Include/ConfigVars.h | 12 +++++++++++-
> > .../Library/PlatformBootManagerLib/PlatformBm.c | 16 ++++++++++++++--
> > .../PlatformBootManagerLib.inf | 1 +
> > Platform/RaspberryPi/RPi3/RPi3.dsc | 10 +++++++++-
> > Platform/RaspberryPi/RPi4/RPi4.dsc | 9 ++++++++-
> > Platform/RaspberryPi/RaspberryPi.dec | 2 ++
> > 10 files changed, 80 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@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@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..7b14fdf39f 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@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"
> >
> >
>
> Personally, I would move the Boot Policy section between "System Table
> Selection" and "ACPI Fan Control" if we try to keep the advanced
> settings in the order we think they will be most used.
>
> >
> > +#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 discoverd for reducing "
>
> "discoverd" -> "discovered"
>
> >
> > + "the boot time."
>
> Missing a space after "the boot time.". It should be "the boot time. ",
> else the help text will display as "(...) the boot time.When Full
> Discovery (...)"
>
> >
> > + "When Full Discovery is selected, all the devices will be discoverd for some "
>
> "discoverd" -> "discovered"
>
> >
> > + "scenarios such as system deployement and diagnostic tests"
>
> "deployement" -> "deployment"
>
> >
> > +#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..5dc558ec08 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@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);
> >
> > @@ -220,6 +225,14 @@ formset
> > minsize = 0,
> >
> > maxsize = ASSET_TAG_STR_MAX_LEN,
> >
> > endstring;
> >
> > +
> >
> > + 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;
>
> "STRING_TOKEN(STR_FAST_BOOT )" -> "STRING_TOKEN(STR_FAST_BOOT)"
>
> >
> > + option text = STRING_TOKEN(STR_FULL_DISCOVERY), value = FULL_DISCOVERY, flags = DEFAULT;
> >
> > + endoneof;
> >
> > endform;
> >
> >
> >
> > form formid = 0x1003,
> >
> > diff --git a/Platform/RaspberryPi/Include/ConfigVars.h b/Platform/RaspberryPi/Include/ConfigVars.h
> > index 142317985a..3347f899df 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@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 Discovrey (Connect All)
>
> "Discovrey" -> "Discovery"
>
> >
> > + */
> >
> > + 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..abbe4fb3d0 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@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
> >
> > @@ -26,9 +26,11 @@
> > #include <Guid/EventGroup.h>
> >
> > #include <Guid/TtyTerm.h>
> >
> >
> >
> > +#include <ConfigVars.h>
>
> I'd remove the extra line between <Guid/TtyTerm.h> and <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"
>
> Why are you adding the carriage return? Was there an issue with the
> display of this prompt?
>
> >
> >
> >
> > #define DP_NODE_LEN(Type) { (UINT8)sizeof (Type), (UINT8)(sizeof (Type) >> 8) }
> >
> >
> >
> > @@ -633,6 +635,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"));
>
> "All" -> "all"
>
> >
> > + 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..ddb03e405f 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@gmail.com>
> >
> > @@ -512,6 +512,14 @@
> > gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0
> >
> > gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSetGuid|0x0|0
> >
> >
> >
> > + #
> >
> > + # Boot Policy
> >
> > + # 0 - Fast Boot
> >
> > + # 1 - Full Discovrey (Connect All)
>
> "Discovrey" -> "Discovery"
>
> >
> > + #
> >
> > + gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeFormSetGuid|0x0|1
> >
> > +
> >
> > +
>
> Please remove the extra line. There should be just one blank like
> between the sections.
>
> >
> > #
> >
> > # Reset-related.
> >
> > #
> >
> > diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
> > index ff802d8347..8ee1922a44 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@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 Discovrey (Connect All)
>
> "Discovrey" -> "Discovery"
>
> Regards,
>
> /Pete
>
> >
> > + #
> >
> > + 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@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
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot
2021-04-12 9:05 [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot Sunny Wang
2021-04-12 13:03 ` Pete Batard
@ 2021-04-12 21:28 ` Samer El-Haj-Mahmoud
2021-04-13 7:51 ` Sunny Wang
2021-04-16 13:45 ` Sunny Wang
1 sibling, 2 replies; 9+ messages in thread
From: Samer El-Haj-Mahmoud @ 2021-04-12 21:28 UTC (permalink / raw)
To: Sunny Wang, devel@edk2.groups.io
Cc: Sunny Wang, Jeremy Linton, Pete Batard, Ard Biesheuvel,
Sunny Wang, Samer El-Haj-Mahmoud
Sunny,
Thanks for sending this!
This was tested by several RPi4 users and confirmed to fix at least the following issue:
https://github.com/pftf/RPi4/issues/144
https://github.com/pftf/RPi4/issues/114
It *may* also fix the following issues (to be confirmed):
https://github.com/pftf/RPi4/issues/117
https://github.com/pftf/RPi4/issues/136
Please add references to these issues (at least confirmed ones) in the commit message when you send the v2 of the patch.
With that,
Acked-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> -----Original Message-----
> From: Sunny Wang <Sunny.Wang@arm.com>
> Sent: Monday, April 12, 2021 5:06 AM
> To: devel@edk2.groups.io
> Cc: Sunny Wang <Sunny.Wang@arm.com>; Samer El-Haj-Mahmoud
> <Samer.El-Haj-Mahmoud@arm.com>; Jeremy Linton
> <Jeremy.Linton@arm.com>; Pete Batard <pete@akeo.ie>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Sunny Wang <Sunny.Wang@arm.com>
> Subject: [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast
> Boot
>
> 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@arm.com>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> ---
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 11 ++++++++++-
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 ++-
> .../Drivers/ConfigDxe/ConfigDxeHii.uni | 10 +++++++++-
> .../Drivers/ConfigDxe/ConfigDxeHii.vfr | 15 ++++++++++++++-
> Platform/RaspberryPi/Include/ConfigVars.h | 12 +++++++++++-
> .../Library/PlatformBootManagerLib/PlatformBm.c | 16 ++++++++++++++--
> .../PlatformBootManagerLib.inf | 1 +
> Platform/RaspberryPi/RPi3/RPi3.dsc | 10 +++++++++-
> Platform/RaspberryPi/RPi4/RPi4.dsc | 9 ++++++++-
> Platform/RaspberryPi/RaspberryPi.dec | 2 ++
> 10 files changed, 80 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@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@gmail.com> # # SPDX-
> License-Identifier: BSD-2-Clause-Patent@@ -93,6 +93,7 @@
> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB
> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio
> gRaspberryPiTokenSpaceGuid.PcdFanTemp+
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy [Depex] gPcdProtocolGuid
> AND gRaspberryPiFirmwareProtocolGuiddiff --git
> a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> index 466fa852cb..7b14fdf39f 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@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 discoverd for
> reducing "+ "the boot time."+
> "When Full Discovery is selected, all the devices will be discoverd for some "+
> "scenarios such as system deployement 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..5dc558ec08 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@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);@@ -220,6 +225,14 @@ formset
> minsize = 0, maxsize = ASSET_TAG_STR_MAX_LEN,
> endstring;++ 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; endform; form
> formid = 0x1003,diff --git a/Platform/RaspberryPi/Include/ConfigVars.h
> b/Platform/RaspberryPi/Include/ConfigVars.h
> index 142317985a..3347f899df 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@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 Discovrey (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..abbe4fb3d0 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@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@@ -26,9
> +26,11 @@ #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 +635,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/PlatformBootMan
> agerLib.inf
> b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMan
> agerLib.inf
> index 88f6f8fe09..fbf510ab96 100644
> ---
> a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMan
> agerLib.inf
> +++
> b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMa
> +++ nagerLib.inf
> @@ -63,6 +63,7 @@
> [Pcd] gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
> gRaspberryPiTokenSpaceGuid.PcdSdIsArasan+
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy [Guids] gEfiFileInfoGuiddiff --
> git a/Platform/RaspberryPi/RPi3/RPi3.dsc
> b/Platform/RaspberryPi/RPi3/RPi3.dsc
> index 0961133ae9..ddb03e405f 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@gmail.com>@@ -512,6 +512,14 @@
>
> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFor
> mSetGuid|0x0|0
> gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormS
> etGuid|0x0|0 + #+ # Boot Policy+ # 0 - Fast Boot+ # 1 - Full Discovrey
> (Connect All)+ #+
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeFor
> mSetGuid|0x0|1++ # # Reset-related. #diff --git
> a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index ff802d8347..8ee1922a44 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@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"|gConfigDxeFor
> mSetGuid|0x0|0
> gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormS
> etGuid|0x0|60 + #+ # Boot Policy+ # 0 - Fast Boot+ # 1 - Full Discovrey
> (Connect All)+ #+
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeFor
> mSetGuid|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@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|0x0000001
> E
> gRaspberryPiTokenSpaceGuid.PcdMmcEnableDma|0|UINT32|0x0000001F+
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy|0|UINT32|0x00000020--
> 2.31.0.windows.1
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot
2021-04-12 13:03 ` Pete Batard
2021-04-12 17:27 ` Ard Biesheuvel
@ 2021-04-13 4:33 ` Sunny Wang
1 sibling, 0 replies; 9+ messages in thread
From: Sunny Wang @ 2021-04-13 4:33 UTC (permalink / raw)
To: Pete Batard, devel@edk2.groups.io
Cc: Samer El-Haj-Mahmoud, Jeremy Linton, Ard Biesheuvel, Sunny Wang
Thanks for the review, Pete. I just sent v2 for addressing all your comments except the one below.
> -#define BOOT_PROMPT L"ESC (setup), F1 (shell), ENTER (boot)"
>
> +#define BOOT_PROMPT L"ESC (setup), F1 (shell), ENTER (boot)\n"
Why are you adding the carriage return? Was there an issue with the display of this prompt?
[Sunny] Yeah, I saw a display issue in the serial log. This message would be stuck to the next message (either screen or serial message) like the following:
- ESC (setup), F1 (shell), ENTER (boot)UsbBusRecursivelyConnectWantedUsbIo: TPL before connect is 4
- ESC (setup), F1 (shell), ENTER (boot)Boot Policy is Full Discovery. Connect all devices
Best Regards,
Sunny Wang
-----Original Message-----
From: Pete Batard <pete@akeo.ie>
Sent: Monday, April 12, 2021 9:04 PM
To: Sunny Wang <Sunny.Wang@arm.com>; devel@edk2.groups.io
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Jeremy Linton <Jeremy.Linton@arm.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>
Subject: Re: [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot
Hi Sunny,
Functionaly, I see no issues with this patch, and it's indeed a good solution to the problem of trying to satisfy everyone while fixing the issues we are seeing.
There are however a few minor typos and whitespace issues, that I'm detailing below. So could you please send a v2 to address these?
On 2021.04.12 10:05, 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@arm.com>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> ---
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 11 ++++++++++-
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 ++-
> .../Drivers/ConfigDxe/ConfigDxeHii.uni | 10 +++++++++-
> .../Drivers/ConfigDxe/ConfigDxeHii.vfr | 15 ++++++++++++++-
> Platform/RaspberryPi/Include/ConfigVars.h | 12 +++++++++++-
> .../Library/PlatformBootManagerLib/PlatformBm.c | 16 ++++++++++++++--
> .../PlatformBootManagerLib.inf | 1 +
> Platform/RaspberryPi/RPi3/RPi3.dsc | 10 +++++++++-
> Platform/RaspberryPi/RPi4/RPi4.dsc | 9 ++++++++-
> Platform/RaspberryPi/RaspberryPi.dec | 2 ++
> 10 files changed, 80 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@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@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..7b14fdf39f 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@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"
>
>
Personally, I would move the Boot Policy section between "System Table Selection" and "ACPI Fan Control" if we try to keep the advanced settings in the order we think they will be most used.
>
> +#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 discoverd for reducing "
"discoverd" -> "discovered"
>
> + "the boot time."
Missing a space after "the boot time.". It should be "the boot time. ", else the help text will display as "(...) the boot time.When Full Discovery (...)"
>
> + "When Full Discovery is selected, all the devices will be discoverd for some "
"discoverd" -> "discovered"
>
> + "scenarios such as system deployement and diagnostic tests"
"deployement" -> "deployment"
>
> +#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..5dc558ec08 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@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);
>
> @@ -220,6 +225,14 @@ formset
> minsize = 0,
>
> maxsize = ASSET_TAG_STR_MAX_LEN,
>
> endstring;
>
> +
>
> + 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;
"STRING_TOKEN(STR_FAST_BOOT )" -> "STRING_TOKEN(STR_FAST_BOOT)"
>
> + option text = STRING_TOKEN(STR_FULL_DISCOVERY), value =
> + FULL_DISCOVERY, flags = DEFAULT;
>
> + endoneof;
>
> endform;
>
>
>
> form formid = 0x1003,
>
> diff --git a/Platform/RaspberryPi/Include/ConfigVars.h
> b/Platform/RaspberryPi/Include/ConfigVars.h
> index 142317985a..3347f899df 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@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 Discovrey (Connect All)
"Discovrey" -> "Discovery"
>
> + */
>
> + 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..abbe4fb3d0 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@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
>
> @@ -26,9 +26,11 @@
> #include <Guid/EventGroup.h>
>
> #include <Guid/TtyTerm.h>
>
>
>
> +#include <ConfigVars.h>
I'd remove the extra line between <Guid/TtyTerm.h> and <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"
Why are you adding the carriage return? Was there an issue with the display of this prompt?
>
>
>
> #define DP_NODE_LEN(Type) { (UINT8)sizeof (Type), (UINT8)(sizeof
> (Type) >> 8) }
>
>
>
> @@ -633,6 +635,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"));
"All" -> "all"
>
> + 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/PlatformBootMana
> gerLib.inf
> b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMana
> gerLib.inf
> index 88f6f8fe09..fbf510ab96 100644
> ---
> a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMana
> gerLib.inf
> +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBoot
> +++ ManagerLib.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..ddb03e405f 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@gmail.com>
>
> @@ -512,6 +512,14 @@
>
> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSet
> Guid|0x0|0
>
>
> gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSetGuid
> |0x0|0
>
>
>
> + #
>
> + # Boot Policy
>
> + # 0 - Fast Boot
>
> + # 1 - Full Discovrey (Connect All)
"Discovrey" -> "Discovery"
>
> + #
>
> +
> + gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeFor
> + mSetGuid|0x0|1
>
> +
>
> +
Please remove the extra line. There should be just one blank like between the sections.
>
> #
>
> # Reset-related.
>
> #
>
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc
> b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index ff802d8347..8ee1922a44 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@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"|gConfigDxeFormSet
> Guid|0x0|0
>
>
> gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSetGuid
> |0x0|60
>
>
>
> + #
>
> + # Boot Policy
>
> + # 0 - Fast Boot
>
> + # 1 - Full Discovrey (Connect All)
"Discovrey" -> "Discovery"
Regards,
/Pete
>
> + #
>
> +
> + gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeFor
> + mSetGuid|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@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
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot
2021-04-12 17:27 ` Ard Biesheuvel
@ 2021-04-13 6:01 ` Sunny Wang
[not found] ` <16755592E0799AE6.2444@groups.io>
1 sibling, 0 replies; 9+ messages in thread
From: Sunny Wang @ 2021-04-13 6:01 UTC (permalink / raw)
To: Ard Biesheuvel, Pete Batard
Cc: edk2-devel-groups-io, Samer El-Haj-Mahmoud, Jeremy Linton,
Ard Biesheuvel, Sunny Wang
Thanks, Ard.
I checked and fixed Windows whitespace damage in v2. If you still see the issue, feel free to let me know.
The issue was caused by my git transfer encoding setting. It was somehow using "quoted-printable". Then, I used "--transfer-encoding=" to override the setting to "base64". Now the issue seems to be resolved.
Best Regards,
Sunny Wang
-----Original Message-----
From: Ard Biesheuvel <ardb@kernel.org>
Sent: Tuesday, April 13, 2021 1:27 AM
To: Pete Batard <pete@akeo.ie>
Cc: Sunny Wang <Sunny.Wang@arm.com>; edk2-devel-groups-io <devel@edk2.groups.io>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Jeremy Linton <Jeremy.Linton@arm.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>
Subject: Re: [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot
On Mon, 12 Apr 2021 at 15:03, Pete Batard <pete@akeo.ie> wrote:
>
> Hi Sunny,
>
> Functionaly, I see no issues with this patch, and it's indeed a good
> solution to the problem of trying to satisfy everyone while fixing the
> issues we are seeing.
>
> There are however a few minor typos and whitespace issues, that I'm
> detailing below. So could you please send a v2 to address these?
>
Agree with Pete. Thanks for providing a fix for this, and please incorporate the changes suggested by Pete.
Then, could we *please* figure out a way to get this patch on the mailing list without Windows whitespace damage? Not sure if anything changed recently, but not a single EDK2 patch has arrived in my mailbox recently in a form that I could apply with 'git am'.
Thanks,
Ard.
> On 2021.04.12 10:05, 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@arm.com>
> > Cc: Jeremy Linton <jeremy.linton@arm.com>
> > Cc: Pete Batard <pete@akeo.ie>
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> > ---
> > .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 11 ++++++++++-
> > .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 ++-
> > .../Drivers/ConfigDxe/ConfigDxeHii.uni | 10 +++++++++-
> > .../Drivers/ConfigDxe/ConfigDxeHii.vfr | 15 ++++++++++++++-
> > Platform/RaspberryPi/Include/ConfigVars.h | 12 +++++++++++-
> > .../Library/PlatformBootManagerLib/PlatformBm.c | 16 ++++++++++++++--
> > .../PlatformBootManagerLib.inf | 1 +
> > Platform/RaspberryPi/RPi3/RPi3.dsc | 10 +++++++++-
> > Platform/RaspberryPi/RPi4/RPi4.dsc | 9 ++++++++-
> > Platform/RaspberryPi/RaspberryPi.dec | 2 ++
> > 10 files changed, 80 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@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@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..7b14fdf39f 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@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"
> >
> >
>
> Personally, I would move the Boot Policy section between "System Table
> Selection" and "ACPI Fan Control" if we try to keep the advanced
> settings in the order we think they will be most used.
>
> >
> > +#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 discoverd for reducing "
>
> "discoverd" -> "discovered"
>
> >
> > + "the boot time."
>
> Missing a space after "the boot time.". It should be "the boot time.
> ", else the help text will display as "(...) the boot time.When Full
> Discovery (...)"
>
> >
> > + "When Full Discovery is selected, all the devices will be discoverd for some "
>
> "discoverd" -> "discovered"
>
> >
> > + "scenarios such as system deployement and diagnostic tests"
>
> "deployement" -> "deployment"
>
> >
> > +#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..5dc558ec08 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@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);
> >
> > @@ -220,6 +225,14 @@ formset
> > minsize = 0,
> >
> > maxsize = ASSET_TAG_STR_MAX_LEN,
> >
> > endstring;
> >
> > +
> >
> > + 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;
>
> "STRING_TOKEN(STR_FAST_BOOT )" -> "STRING_TOKEN(STR_FAST_BOOT)"
>
> >
> > + option text = STRING_TOKEN(STR_FULL_DISCOVERY), value =
> > + FULL_DISCOVERY, flags = DEFAULT;
> >
> > + endoneof;
> >
> > endform;
> >
> >
> >
> > form formid = 0x1003,
> >
> > diff --git a/Platform/RaspberryPi/Include/ConfigVars.h
> > b/Platform/RaspberryPi/Include/ConfigVars.h
> > index 142317985a..3347f899df 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@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 Discovrey (Connect All)
>
> "Discovrey" -> "Discovery"
>
> >
> > + */
> >
> > + 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..abbe4fb3d0 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@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
> >
> > @@ -26,9 +26,11 @@
> > #include <Guid/EventGroup.h>
> >
> > #include <Guid/TtyTerm.h>
> >
> >
> >
> > +#include <ConfigVars.h>
>
> I'd remove the extra line between <Guid/TtyTerm.h> and <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"
>
> Why are you adding the carriage return? Was there an issue with the
> display of this prompt?
>
> >
> >
> >
> > #define DP_NODE_LEN(Type) { (UINT8)sizeof (Type), (UINT8)(sizeof
> > (Type) >> 8) }
> >
> >
> >
> > @@ -633,6 +635,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"));
>
> "All" -> "all"
>
> >
> > + 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/PlatformBootMa
> > nagerLib.inf
> > b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMa
> > nagerLib.inf
> > index 88f6f8fe09..fbf510ab96 100644
> > ---
> > a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMa
> > nagerLib.inf
> > +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBo
> > +++ otManagerLib.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..ddb03e405f 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@gmail.com>
> >
> > @@ -512,6 +512,14 @@
> >
> > gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormS
> > etGuid|0x0|0
> >
> >
> > gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSetGu
> > id|0x0|0
> >
> >
> >
> > + #
> >
> > + # Boot Policy
> >
> > + # 0 - Fast Boot
> >
> > + # 1 - Full Discovrey (Connect All)
>
> "Discovrey" -> "Discovery"
>
> >
> > + #
> >
> > +
> > + gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeF
> > + ormSetGuid|0x0|1
> >
> > +
> >
> > +
>
> Please remove the extra line. There should be just one blank like
> between the sections.
>
> >
> > #
> >
> > # Reset-related.
> >
> > #
> >
> > diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc
> > b/Platform/RaspberryPi/RPi4/RPi4.dsc
> > index ff802d8347..8ee1922a44 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@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"|gConfigDxeFormS
> > etGuid|0x0|0
> >
> >
> > gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSetGu
> > id|0x0|60
> >
> >
> >
> > + #
> >
> > + # Boot Policy
> >
> > + # 0 - Fast Boot
> >
> > + # 1 - Full Discovrey (Connect All)
>
> "Discovrey" -> "Discovery"
>
> Regards,
>
> /Pete
>
> >
> > + #
> >
> > +
> > + gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeF
> > + ormSetGuid|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@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
> >
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot
[not found] ` <16755592E0799AE6.2444@groups.io>
@ 2021-04-13 7:22 ` Sunny Wang
0 siblings, 0 replies; 9+ messages in thread
From: Sunny Wang @ 2021-04-13 7:22 UTC (permalink / raw)
To: devel@edk2.groups.io, Sunny Wang, Ard Biesheuvel, Pete Batard
Cc: Samer El-Haj-Mahmoud, Jeremy Linton, Ard Biesheuvel
It looks like "base64" would cause a display issue with https://edk2.groups.io/g/devel/message/74011, so I re-sent v2 with the default "8bit" setting. So far, "8bit" seems working fine.
Best Regards,
Sunny Wang
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sunny Wang via groups.io
Sent: Tuesday, April 13, 2021 2:02 PM
To: Ard Biesheuvel <ardb@kernel.org>; Pete Batard <pete@akeo.ie>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Jeremy Linton <Jeremy.Linton@arm.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Sunny Wang <Sunny.Wang@arm.com>
Subject: Re: [edk2-devel] [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot
Thanks, Ard.
I checked and fixed Windows whitespace damage in v2. If you still see the issue, feel free to let me know.
The issue was caused by my git transfer encoding setting. It was somehow using "quoted-printable". Then, I used "--transfer-encoding=" to override the setting to "base64". Now the issue seems to be resolved.
Best Regards,
Sunny Wang
-----Original Message-----
From: Ard Biesheuvel <ardb@kernel.org>
Sent: Tuesday, April 13, 2021 1:27 AM
To: Pete Batard <pete@akeo.ie>
Cc: Sunny Wang <Sunny.Wang@arm.com>; edk2-devel-groups-io <devel@edk2.groups.io>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Jeremy Linton <Jeremy.Linton@arm.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>
Subject: Re: [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot
On Mon, 12 Apr 2021 at 15:03, Pete Batard <pete@akeo.ie> wrote:
>
> Hi Sunny,
>
> Functionaly, I see no issues with this patch, and it's indeed a good
> solution to the problem of trying to satisfy everyone while fixing the
> issues we are seeing.
>
> There are however a few minor typos and whitespace issues, that I'm
> detailing below. So could you please send a v2 to address these?
>
Agree with Pete. Thanks for providing a fix for this, and please incorporate the changes suggested by Pete.
Then, could we *please* figure out a way to get this patch on the mailing list without Windows whitespace damage? Not sure if anything changed recently, but not a single EDK2 patch has arrived in my mailbox recently in a form that I could apply with 'git am'.
Thanks,
Ard.
> On 2021.04.12 10:05, 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@arm.com>
> > Cc: Jeremy Linton <jeremy.linton@arm.com>
> > Cc: Pete Batard <pete@akeo.ie>
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> > ---
> > .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 11 ++++++++++-
> > .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 ++-
> > .../Drivers/ConfigDxe/ConfigDxeHii.uni | 10 +++++++++-
> > .../Drivers/ConfigDxe/ConfigDxeHii.vfr | 15 ++++++++++++++-
> > Platform/RaspberryPi/Include/ConfigVars.h | 12 +++++++++++-
> > .../Library/PlatformBootManagerLib/PlatformBm.c | 16 ++++++++++++++--
> > .../PlatformBootManagerLib.inf | 1 +
> > Platform/RaspberryPi/RPi3/RPi3.dsc | 10 +++++++++-
> > Platform/RaspberryPi/RPi4/RPi4.dsc | 9 ++++++++-
> > Platform/RaspberryPi/RaspberryPi.dec | 2 ++
> > 10 files changed, 80 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@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@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..7b14fdf39f 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@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"
> >
> >
>
> Personally, I would move the Boot Policy section between "System Table
> Selection" and "ACPI Fan Control" if we try to keep the advanced
> settings in the order we think they will be most used.
>
> >
> > +#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 discoverd for reducing "
>
> "discoverd" -> "discovered"
>
> >
> > + "the boot time."
>
> Missing a space after "the boot time.". It should be "the boot time.
> ", else the help text will display as "(...) the boot time.When Full
> Discovery (...)"
>
> >
> > + "When Full Discovery is selected, all the devices will be discoverd for some "
>
> "discoverd" -> "discovered"
>
> >
> > + "scenarios such as system deployement and diagnostic tests"
>
> "deployement" -> "deployment"
>
> >
> > +#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..5dc558ec08 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@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);
> >
> > @@ -220,6 +225,14 @@ formset
> > minsize = 0,
> >
> > maxsize = ASSET_TAG_STR_MAX_LEN,
> >
> > endstring;
> >
> > +
> >
> > + 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;
>
> "STRING_TOKEN(STR_FAST_BOOT )" -> "STRING_TOKEN(STR_FAST_BOOT)"
>
> >
> > + option text = STRING_TOKEN(STR_FULL_DISCOVERY), value =
> > + FULL_DISCOVERY, flags = DEFAULT;
> >
> > + endoneof;
> >
> > endform;
> >
> >
> >
> > form formid = 0x1003,
> >
> > diff --git a/Platform/RaspberryPi/Include/ConfigVars.h
> > b/Platform/RaspberryPi/Include/ConfigVars.h
> > index 142317985a..3347f899df 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@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 Discovrey (Connect All)
>
> "Discovrey" -> "Discovery"
>
> >
> > + */
> >
> > + 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..abbe4fb3d0 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@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
> >
> > @@ -26,9 +26,11 @@
> > #include <Guid/EventGroup.h>
> >
> > #include <Guid/TtyTerm.h>
> >
> >
> >
> > +#include <ConfigVars.h>
>
> I'd remove the extra line between <Guid/TtyTerm.h> and <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"
>
> Why are you adding the carriage return? Was there an issue with the
> display of this prompt?
>
> >
> >
> >
> > #define DP_NODE_LEN(Type) { (UINT8)sizeof (Type), (UINT8)(sizeof
> > (Type) >> 8) }
> >
> >
> >
> > @@ -633,6 +635,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"));
>
> "All" -> "all"
>
> >
> > + 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/PlatformBootMa
> > nagerLib.inf
> > b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMa
> > nagerLib.inf
> > index 88f6f8fe09..fbf510ab96 100644
> > ---
> > a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMa
> > nagerLib.inf
> > +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBo
> > +++ otManagerLib.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..ddb03e405f 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@gmail.com>
> >
> > @@ -512,6 +512,14 @@
> >
> > gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormS
> > etGuid|0x0|0
> >
> >
> > gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSetGu
> > id|0x0|0
> >
> >
> >
> > + #
> >
> > + # Boot Policy
> >
> > + # 0 - Fast Boot
> >
> > + # 1 - Full Discovrey (Connect All)
>
> "Discovrey" -> "Discovery"
>
> >
> > + #
> >
> > +
> > + gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeF
> > + ormSetGuid|0x0|1
> >
> > +
> >
> > +
>
> Please remove the extra line. There should be just one blank like
> between the sections.
>
> >
> > #
> >
> > # Reset-related.
> >
> > #
> >
> > diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc
> > b/Platform/RaspberryPi/RPi4/RPi4.dsc
> > index ff802d8347..8ee1922a44 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@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"|gConfigDxeFormS
> > etGuid|0x0|0
> >
> >
> > gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSetGu
> > id|0x0|60
> >
> >
> >
> > + #
> >
> > + # Boot Policy
> >
> > + # 0 - Fast Boot
> >
> > + # 1 - Full Discovrey (Connect All)
>
> "Discovrey" -> "Discovery"
>
> Regards,
>
> /Pete
>
> >
> > + #
> >
> > +
> > + gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeF
> > + ormSetGuid|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@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
> >
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot
2021-04-12 21:28 ` Samer El-Haj-Mahmoud
@ 2021-04-13 7:51 ` Sunny Wang
2021-04-16 13:45 ` Sunny Wang
1 sibling, 0 replies; 9+ messages in thread
From: Sunny Wang @ 2021-04-13 7:51 UTC (permalink / raw)
To: Samer El-Haj-Mahmoud, devel@edk2.groups.io
Cc: Jeremy Linton, Pete Batard, Ard Biesheuvel, Sunny Wang
Oops, I already sent v2. Sorry for missing your request (checking and confirming other issues and adding them to the commit message), Samer.
I will check this later and send v3 or re-send v2.
Please not push this until I address this comment. Thanks!
Best Regards,
Sunny Wang
-----Original Message-----
From: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Sent: Tuesday, April 13, 2021 5:29 AM
To: Sunny Wang <Sunny.Wang@arm.com>; devel@edk2.groups.io
Cc: Sunny Wang <Sunny.Wang@arm.com>; Jeremy Linton <Jeremy.Linton@arm.com>; Pete Batard <pete@akeo.ie>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Sunny Wang <Sunny.Wang@arm.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Subject: RE: [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot
Sunny,
Thanks for sending this!
This was tested by several RPi4 users and confirmed to fix at least the following issue:
https://github.com/pftf/RPi4/issues/144
https://github.com/pftf/RPi4/issues/114
It *may* also fix the following issues (to be confirmed):
https://github.com/pftf/RPi4/issues/117
https://github.com/pftf/RPi4/issues/136
Please add references to these issues (at least confirmed ones) in the commit message when you send the v2 of the patch.
With that,
Acked-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> -----Original Message-----
> From: Sunny Wang <Sunny.Wang@arm.com>
> Sent: Monday, April 12, 2021 5:06 AM
> To: devel@edk2.groups.io
> Cc: Sunny Wang <Sunny.Wang@arm.com>; Samer El-Haj-Mahmoud
> <Samer.El-Haj-Mahmoud@arm.com>; Jeremy Linton <Jeremy.Linton@arm.com>;
> Pete Batard <pete@akeo.ie>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Sunny Wang <Sunny.Wang@arm.com>
> Subject: [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling
> Fast Boot
>
> 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@arm.com>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> ---
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 11 ++++++++++-
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 ++-
> .../Drivers/ConfigDxe/ConfigDxeHii.uni | 10 +++++++++-
> .../Drivers/ConfigDxe/ConfigDxeHii.vfr | 15 ++++++++++++++-
> Platform/RaspberryPi/Include/ConfigVars.h | 12 +++++++++++-
> .../Library/PlatformBootManagerLib/PlatformBm.c | 16 ++++++++++++++--
> .../PlatformBootManagerLib.inf | 1 +
> Platform/RaspberryPi/RPi3/RPi3.dsc | 10 +++++++++-
> Platform/RaspberryPi/RPi4/RPi4.dsc | 9 ++++++++-
> Platform/RaspberryPi/RaspberryPi.dec | 2 ++
> 10 files changed, 80 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@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@gmail.com> # # SPDX-
> License-Identifier: BSD-2-Clause-Patent@@ -93,6 +93,7 @@
> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB
> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio
> gRaspberryPiTokenSpaceGuid.PcdFanTemp+
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy [Depex] gPcdProtocolGuid
> AND gRaspberryPiFirmwareProtocolGuiddiff --git
> a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> index 466fa852cb..7b14fdf39f 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@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 discoverd for
> reducing "+ "the boot time."+
> "When Full Discovery is selected, all the devices will be discoverd
> for some "+ "scenarios such as system deployement 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..5dc558ec08 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@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);@@ -220,6 +225,14 @@ formset
> minsize = 0, maxsize = ASSET_TAG_STR_MAX_LEN,
> endstring;++ 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; endform; form
> formid = 0x1003,diff --git a/Platform/RaspberryPi/Include/ConfigVars.h
> b/Platform/RaspberryPi/Include/ConfigVars.h
> index 142317985a..3347f899df 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@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 Discovrey (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..abbe4fb3d0 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@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@@ -26,9
> +26,11 @@ #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 +635,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/PlatformBootMan
> agerLib.inf
> b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMan
> agerLib.inf
> index 88f6f8fe09..fbf510ab96 100644
> ---
> a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMan
> agerLib.inf
> +++
> b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMa
> +++ nagerLib.inf
> @@ -63,6 +63,7 @@
> [Pcd] gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
> gRaspberryPiTokenSpaceGuid.PcdSdIsArasan+
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy [Guids] gEfiFileInfoGuiddiff --
> git a/Platform/RaspberryPi/RPi3/RPi3.dsc
> b/Platform/RaspberryPi/RPi3/RPi3.dsc
> index 0961133ae9..ddb03e405f 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@gmail.com>@@ -512,6 +512,14 @@
>
> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFor
> mSetGuid|0x0|0
> gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormS
> etGuid|0x0|0 + #+ # Boot Policy+ # 0 - Fast Boot+ # 1 - Full
> etGuid|0x0|Discovrey
> (Connect All)+ #+
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeFor
> mSetGuid|0x0|1++ # # Reset-related. #diff --git
> a/Platform/RaspberryPi/RPi4/RPi4.dsc
> b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index ff802d8347..8ee1922a44 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@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"|gConfigDxeFor
> mSetGuid|0x0|0
> gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormS
> etGuid|0x0|60 + #+ # Boot Policy+ # 0 - Fast Boot+ # 1 - Full
> etGuid|0x0|Discovrey
> (Connect All)+ #+
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeFor
> mSetGuid|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@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|0x0000001
> E
> gRaspberryPiTokenSpaceGuid.PcdMmcEnableDma|0|UINT32|0x0000001F+
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy|0|UINT32|0x00000020--
> 2.31.0.windows.1
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot
2021-04-12 21:28 ` Samer El-Haj-Mahmoud
2021-04-13 7:51 ` Sunny Wang
@ 2021-04-16 13:45 ` Sunny Wang
1 sibling, 0 replies; 9+ messages in thread
From: Sunny Wang @ 2021-04-16 13:45 UTC (permalink / raw)
To: Samer El-Haj-Mahmoud, devel@edk2.groups.io
Cc: Jeremy Linton, Pete Batard, Ard Biesheuvel, Sunny Wang
Hi Samer,
Thanks for checking and putting all related issues here. However, I missed your comment in v2. Sorry about that. Therefore, I directly added the references to the commit.
https://github.com/tianocore/edk2-platforms/commit/efdc159ef7c9f15581a0f63d755a1530ff475156
This commit turned out to fix three issues below. I also added comments/updates to these four issues and closed the following three.
https://github.com/pftf/RPi4/issues/144
https://github.com/pftf/RPi4/issues/114
https://github.com/pftf/RPi4/issues/117
Best Regards,
Sunny Wang
-----Original Message-----
From: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Sent: Tuesday, April 13, 2021 5:29 AM
To: Sunny Wang <Sunny.Wang@arm.com>; devel@edk2.groups.io
Cc: Sunny Wang <Sunny.Wang@arm.com>; Jeremy Linton <Jeremy.Linton@arm.com>; Pete Batard <pete@akeo.ie>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Sunny Wang <Sunny.Wang@arm.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Subject: RE: [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot
Sunny,
Thanks for sending this!
This was tested by several RPi4 users and confirmed to fix at least the following issue:
https://github.com/pftf/RPi4/issues/144
https://github.com/pftf/RPi4/issues/114
It *may* also fix the following issues (to be confirmed):
https://github.com/pftf/RPi4/issues/117
https://github.com/pftf/RPi4/issues/136
Please add references to these issues (at least confirmed ones) in the commit message when you send the v2 of the patch.
With that,
Acked-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> -----Original Message-----
> From: Sunny Wang <Sunny.Wang@arm.com>
> Sent: Monday, April 12, 2021 5:06 AM
> To: devel@edk2.groups.io
> Cc: Sunny Wang <Sunny.Wang@arm.com>; Samer El-Haj-Mahmoud
> <Samer.El-Haj-Mahmoud@arm.com>; Jeremy Linton <Jeremy.Linton@arm.com>;
> Pete Batard <pete@akeo.ie>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Sunny Wang <Sunny.Wang@arm.com>
> Subject: [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling
> Fast Boot
>
> 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@arm.com>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> ---
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 11 ++++++++++-
> .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 3 ++-
> .../Drivers/ConfigDxe/ConfigDxeHii.uni | 10 +++++++++-
> .../Drivers/ConfigDxe/ConfigDxeHii.vfr | 15 ++++++++++++++-
> Platform/RaspberryPi/Include/ConfigVars.h | 12 +++++++++++-
> .../Library/PlatformBootManagerLib/PlatformBm.c | 16 ++++++++++++++--
> .../PlatformBootManagerLib.inf | 1 +
> Platform/RaspberryPi/RPi3/RPi3.dsc | 10 +++++++++-
> Platform/RaspberryPi/RPi4/RPi4.dsc | 9 ++++++++-
> Platform/RaspberryPi/RaspberryPi.dec | 2 ++
> 10 files changed, 80 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@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@gmail.com> # # SPDX-
> License-Identifier: BSD-2-Clause-Patent@@ -93,6 +93,7 @@
> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB
> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio
> gRaspberryPiTokenSpaceGuid.PcdFanTemp+
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy [Depex] gPcdProtocolGuid
> AND gRaspberryPiFirmwareProtocolGuiddiff --git
> a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> index 466fa852cb..7b14fdf39f 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@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 discoverd for
> reducing "+ "the boot time."+
> "When Full Discovery is selected, all the devices will be discoverd
> for some "+ "scenarios such as system deployement 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..5dc558ec08 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@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);@@ -220,6 +225,14 @@ formset
> minsize = 0, maxsize = ASSET_TAG_STR_MAX_LEN,
> endstring;++ 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; endform; form
> formid = 0x1003,diff --git a/Platform/RaspberryPi/Include/ConfigVars.h
> b/Platform/RaspberryPi/Include/ConfigVars.h
> index 142317985a..3347f899df 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@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 Discovrey (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..abbe4fb3d0 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@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@@ -26,9
> +26,11 @@ #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 +635,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/PlatformBootMan
> agerLib.inf
> b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMan
> agerLib.inf
> index 88f6f8fe09..fbf510ab96 100644
> ---
> a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMan
> agerLib.inf
> +++
> b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBootMa
> +++ nagerLib.inf
> @@ -63,6 +63,7 @@
> [Pcd] gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
> gRaspberryPiTokenSpaceGuid.PcdSdIsArasan+
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy [Guids] gEfiFileInfoGuiddiff --
> git a/Platform/RaspberryPi/RPi3/RPi3.dsc
> b/Platform/RaspberryPi/RPi3/RPi3.dsc
> index 0961133ae9..ddb03e405f 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@gmail.com>@@ -512,6 +512,14 @@
>
> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFor
> mSetGuid|0x0|0
> gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormS
> etGuid|0x0|0 + #+ # Boot Policy+ # 0 - Fast Boot+ # 1 - Full
> etGuid|0x0|Discovrey
> (Connect All)+ #+
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeFor
> mSetGuid|0x0|1++ # # Reset-related. #diff --git
> a/Platform/RaspberryPi/RPi4/RPi4.dsc
> b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index ff802d8347..8ee1922a44 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@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"|gConfigDxeFor
> mSetGuid|0x0|0
> gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormS
> etGuid|0x0|60 + #+ # Boot Policy+ # 0 - Fast Boot+ # 1 - Full
> etGuid|0x0|Discovrey
> (Connect All)+ #+
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy|L"BootPolicy"|gConfigDxeFor
> mSetGuid|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@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|0x0000001
> E
> gRaspberryPiTokenSpaceGuid.PcdMmcEnableDma|0|UINT32|0x0000001F+
> gRaspberryPiTokenSpaceGuid.PcdBootPolicy|0|UINT32|0x00000020--
> 2.31.0.windows.1
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-04-16 13:45 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-04-12 9:05 [PATCH 1/1] Platform/RaspberryPi: Setup option for disabling Fast Boot Sunny Wang
2021-04-12 13:03 ` Pete Batard
2021-04-12 17:27 ` Ard Biesheuvel
2021-04-13 6:01 ` Sunny Wang
[not found] ` <16755592E0799AE6.2444@groups.io>
2021-04-13 7:22 ` [edk2-devel] " Sunny Wang
2021-04-13 4:33 ` Sunny Wang
2021-04-12 21:28 ` Samer El-Haj-Mahmoud
2021-04-13 7:51 ` Sunny Wang
2021-04-16 13:45 ` Sunny Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox