From: "Nate DeSimone" <nathaniel.l.desimone@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"benjamin.doron00@gmail.com" <benjamin.doron00@gmail.com>
Cc: "Dong, Eric" <eric.dong@intel.com>,
Liming Gao <gaoliming@byosoft.com.cn>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH v2] BoardModulePkg/BoardBdsHookLib: Register UiApp as boot option
Date: Fri, 13 Aug 2021 06:17:59 +0000 [thread overview]
Message-ID: <MWHPR1101MB2160C62B8EC8FB08EF731FD6CDFA9@MWHPR1101MB2160.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20210728194616.6047-1-benjamin.doron00@gmail.com>
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] -=-=-=-=-=-=
next prev parent reply other threads:[~2021-08-13 6:18 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-28 19:46 [edk2-platforms][PATCH v2] BoardModulePkg/BoardBdsHookLib: Register UiApp as boot option Benjamin Doron
2021-08-13 6:17 ` Nate DeSimone [this message]
2021-08-13 18:14 ` [edk2-devel] " Benjamin Doron
2021-08-13 18:19 ` Benjamin Doron
-- strict thread matches above, loose matches on Subject: below --
2021-07-26 22:07 Benjamin Doron
2021-07-26 22:09 ` [edk2-devel] " Benjamin Doron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MWHPR1101MB2160C62B8EC8FB08EF731FD6CDFA9@MWHPR1101MB2160.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox