[v2] BoardModulePkg/BoardBdsHookLib: Register UiApp as boot option

[edk2-devel] [edk2-platforms][PATCH v2] BoardModulePkg/BoardBdsHookLib: Register UiApp as boot option

Posted by Benjamin Doron 2 years, 9 months ago

BootManagerMenuApp is the default PcdBootManagerMenuFile. It allows
choosing a boot device, but system configuration is performed in UiApp.
Therefore, un-comment and fix UiApp boot option registration.

Tested, UiApp can be entered through the new boot option.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
---
 Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
index 87138bdd79ff..2dd0b250d44e 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
@@ -361,20 +361,11 @@ RegisterDefaultBootOption (
   if (mBootMenuOptionNumber == LoadOptionNumberUnassigned) {
     DEBUG ((DEBUG_INFO, "BootMenuOptionNumber (%d) should not be same to LoadOptionNumberUnassigned(%d).\n", mBootMenuOptionNumber, LoadOptionNumberUnassigned));
   }
-#if 0
+
   //
   // Boot Manager Menu
   //
-  EfiInitializeFwVolDevicepathNode (&FileNode, &mUiFile);
-
-  gBS->HandleProtocol (
-         gImageHandle,
-         &gEfiLoadedImageProtocolGuid,
-         (VOID **) &LoadedImage
-         );
-  DevicePath = AppendDevicePathNode (DevicePathFromHandle (LoadedImage->DeviceHandle), (EFI_DEVICE_PATH_PROTOCOL *) &FileNode);
-#endif
-
+  RegisterFvBootOption (&mUiFile, L"Platform Configuration", (UINTN) -1, LOAD_OPTION_ACTIVE, NULL, 0);
 }
 
 /**
-- 
2.31.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78309): https://edk2.groups.io/g/devel/message/78309
Mute This Topic: https://groups.io/mt/84469836/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH v2] BoardModulePkg/BoardBdsHookLib: Register UiApp as boot option

Posted by Nate DeSimone 2 years, 8 months ago

Hi Benjamin,

The implementation logic looks good. We do have to consider the case of a system without BIOS setup, in which case UiApp will not be present in the image. However, that appears to be well handled by new implementation. If UiApp does not exist in the FV that contains BdsDxe then the file will not be found and the new boot option will not be added.

I do have two comments however.

First, if UiApp does exist in the FV, could you change the F2 hot key to go straight into the setup menu? The hot key is registered here: https://github.com/tianocore/edk2-platforms/blob/c9fff3e9efd2d5daab76b703cc94fd7cbf2ac0b2/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c#L472

Normally F2 is used to enter setup but the current implementation is designed to handle the case of a system not having setup rather lazily... it just routes both F2 and F7 to the boot option menu 100% of the time. Could you modify this implementation so that if UiApp exists in the FV, then F2 goes straight into setup instead?

My second comment is kind of a nitpick, we generally use the string L"Enter Setup" for UiApp instead of L"Platform Configuration" and sadly some of the old code has special handling of that string. For example: https://github.com/tianocore/edk2-platforms/blob/c9fff3e9efd2d5daab76b703cc94fd7cbf2ac0b2/Platform/Intel/PurleyOpenBoardPkg/Override/Platform/Intel/MinPlatformPkg/Bds/Library/DxePlatformBootManagerLib/BdsPlatform.c#L1244

Could you please change that string to L"Enter Setup"?

Thanks,
Nate

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Benjamin Doron
Sent: Wednesday, July 28, 2021 12:46 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
Subject: [edk2-devel] [edk2-platforms][PATCH v2] BoardModulePkg/BoardBdsHookLib: Register UiApp as boot option

BootManagerMenuApp is the default PcdBootManagerMenuFile. It allows choosing a boot device, but system configuration is performed in UiApp.
Therefore, un-comment and fix UiApp boot option registration.

Tested, UiApp can be entered through the new boot option.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
---
 Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
index 87138bdd79ff..2dd0b250d44e 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOpt
+++ ion.c
@@ -361,20 +361,11 @@ RegisterDefaultBootOption (
   if (mBootMenuOptionNumber == LoadOptionNumberUnassigned) {     DEBUG ((DEBUG_INFO, "BootMenuOptionNumber (%d) should not be same to LoadOptionNumberUnassigned(%d).\n", mBootMenuOptionNumber, LoadOptionNumberUnassigned));   }-#if 0+   //   // Boot Manager Menu   //-  EfiInitializeFwVolDevicepathNode (&FileNode, &mUiFile);--  gBS->HandleProtocol (-         gImageHandle,-         &gEfiLoadedImageProtocolGuid,-         (VOID **) &LoadedImage-         );-  DevicePath = AppendDevicePathNode (DevicePathFromHandle (LoadedImage->DeviceHandle), (EFI_DEVICE_PATH_PROTOCOL *) &FileNode);-#endif-+  RegisterFvBootOption (&mUiFile, L"Platform Configuration", (UINTN) -1, LOAD_OPTION_ACTIVE, NULL, 0); }  /**-- 
2.31.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78309): https://edk2.groups.io/g/devel/message/78309
Mute This Topic: https://groups.io/mt/84469836/1767664
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [nathaniel.l.desimone@intel.com] -=-=-=-=-=-=




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79251): https://edk2.groups.io/g/devel/message/79251
Mute This Topic: https://groups.io/mt/84469836/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH v2] BoardModulePkg/BoardBdsHookLib: Register UiApp as boot option

Posted by Benjamin Doron 2 years, 8 months ago

Hi Nate,
Sure, I can do that. The UiApp boot option can then also be marked hidden.

Also, because of the way this patch would change hotkey handling, I'll drop https://edk2.groups.io/g/devel/topic/84671368.

Regards,
Benjamin


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79288): https://edk2.groups.io/g/devel/message/79288
Mute This Topic: https://groups.io/mt/84469836/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH v2] BoardModulePkg/BoardBdsHookLib: Register UiApp as boot option

Posted by Benjamin Doron 2 years, 8 months ago

Actually, mBootMenuOptionNumber could be left alone for this patch, so it would be best to keep the patches separate: get UiApp working, then cleanup BootManagerMenuApp.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79289): https://edk2.groups.io/g/devel/message/79289
Mute This Topic: https://groups.io/mt/84469836/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-