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

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

[Benjamin Doron]

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.

The F2 hotkey can be used to enter UiApp.

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

diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
index a37139a0074e..0bcee7c9a4ba 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
@@ -1385,7 +1385,7 @@ BdsAfterConsoleReadyBeforeBootOptionCallback (
       break;
   }
 
-  Print (L"Press F7 for BootMenu!\n");
+  Print (L"Press F2 for Setup, or F7 for BootMenu!\n");
 
 
 }
diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
index 87138bdd79ff..e734e3ad15c3 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
@@ -15,6 +15,7 @@ BOOLEAN    mHotKeypressed = FALSE;
 EFI_EVENT  HotKeyEvent    = NULL;
 
 UINTN      mBootMenuOptionNumber;
+UINTN      mSetupOptionNumber;
 
 
 /**
@@ -361,20 +362,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
-
+  mSetupOptionNumber = RegisterFvBootOption (&mUiFile, L"Enter Setup", (UINTN) -1, LOAD_OPTION_CATEGORY_APP | LOAD_OPTION_ACTIVE | LOAD_OPTION_HIDDEN, NULL, 0);
 }
 
 /**
@@ -463,14 +455,13 @@ RegisterStaticHotkey (
   //
   // [F2]/[F7]
   //
-  F2.Key.ScanCode    = SCAN_F2;
-  F2.Key.UnicodeChar = CHAR_NULL;
-  F2.KeyState.KeyShiftState = EFI_SHIFT_STATE_VALID;
-  F2.KeyState.KeyToggleState = 0;
-  Status = EfiBootManagerGetBootManagerMenu (&BootOption);
-  ASSERT_EFI_ERROR (Status);
-  RegisterBootOptionHotkey ((UINT16) BootOption.OptionNumber, &F2.Key, TRUE);
-  EfiBootManagerFreeLoadOption (&BootOption);
+  if (mSetupOptionNumber) {
+    F2.Key.ScanCode    = SCAN_F2;
+    F2.Key.UnicodeChar = CHAR_NULL;
+    F2.KeyState.KeyShiftState = EFI_SHIFT_STATE_VALID;
+    F2.KeyState.KeyToggleState = 0;
+    RegisterBootOptionHotkey ((UINT16) mSetupOptionNumber, &F2.Key, TRUE);
+  }
 
   F7.Key.ScanCode    = SCAN_F7;
   F7.Key.UnicodeChar = CHAR_NULL;
-- 
2.31.1

[edk2-platforms][PATCH v2 2/2] BoardModulePkg/BoardBdsHookLib: Simplify hotkey registration

[Benjamin Doron]

Retrieve BootOption of BootManagerMenu for registering F7 hotkey, rather
than creating an additional boot option.

Tested, both F7 hotkey still opens the list of boot options.

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

diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
index e734e3ad15c3..7ac6c150f2e7 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
@@ -14,7 +14,6 @@ BOOLEAN    mPxeBoot       = FALSE;
 BOOLEAN    mHotKeypressed = FALSE;
 EFI_EVENT  HotKeyEvent    = NULL;
 
-UINTN      mBootMenuOptionNumber;
 UINTN      mSetupOptionNumber;
 
 
@@ -189,9 +188,6 @@ CreateFvBootOption (
 EFI_GUID mUiFile = {
   0x462CAA21, 0x7614, 0x4503, { 0x83, 0x6E, 0x8A, 0xB6, 0xF4, 0x66, 0x23, 0x31 }
 };
-EFI_GUID mBootMenuFile = {
-  0xEEC25BDC, 0x67F2, 0x4D95, { 0xB1, 0xD5, 0xF8, 0x1B, 0x20, 0x39, 0xD1, 0x1D }
-};
 
 
 /**
@@ -354,15 +350,6 @@ RegisterDefaultBootOption (
     ShellDataSize = 0;
     RegisterFvBootOption (&gUefiShellFileGuid,      INTERNAL_UEFI_SHELL_NAME, (UINTN) -1, LOAD_OPTION_ACTIVE, (UINT8 *)ShellData, ShellDataSize);
 
-  //
-  // Boot Menu
-  //
-  mBootMenuOptionNumber = RegisterFvBootOption (&mBootMenuFile, L"Boot Device List",   (UINTN) -1, LOAD_OPTION_CATEGORY_APP | LOAD_OPTION_ACTIVE | LOAD_OPTION_HIDDEN, NULL, 0);
-
-  if (mBootMenuOptionNumber == LoadOptionNumberUnassigned) {
-    DEBUG ((DEBUG_INFO, "BootMenuOptionNumber (%d) should not be same to LoadOptionNumberUnassigned(%d).\n", mBootMenuOptionNumber, LoadOptionNumberUnassigned));
-  }
-
   //
   // Boot Manager Menu
   //
@@ -468,8 +455,10 @@ RegisterStaticHotkey (
   F7.KeyState.KeyShiftState = EFI_SHIFT_STATE_VALID;
   F7.KeyState.KeyToggleState = 0;
   mBootMenuBoot  = !EnterSetup;
-  RegisterBootOptionHotkey ((UINT16) mBootMenuOptionNumber, &F7.Key, mBootMenuBoot);
-
+  Status = EfiBootManagerGetBootManagerMenu (&BootOption);
+  ASSERT_EFI_ERROR (Status);
+  RegisterBootOptionHotkey ((UINT16) BootOption.OptionNumber, &F7.Key, mBootMenuBoot);
+  EfiBootManagerFreeLoadOption (&BootOption);
 }
 
 
-- 
2.31.1

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

[Nate DeSimone]

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>

-----Original Message-----
From: Benjamin Doron <benjamin.doron00@gmail.com> 
Sent: Friday, August 13, 2021 5:43 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Subject: [edk2-platforms][PATCH v2 1/2] 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.

The F2 hotkey can be used to enter UiApp.

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

diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
index a37139a0074e..0bcee7c9a4ba 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
@@ -1385,7 +1385,7 @@ BdsAfterConsoleReadyBeforeBootOptionCallback (
       break;
   }
 
-  Print (L"Press F7 for BootMenu!\n");
+  Print (L"Press F2 for Setup, or F7 for BootMenu!\n");
 
 
 }
diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
index 87138bdd79ff..e734e3ad15c3 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
@@ -15,6 +15,7 @@ BOOLEAN    mHotKeypressed = FALSE;
 EFI_EVENT  HotKeyEvent    = NULL;
 
 UINTN      mBootMenuOptionNumber;
+UINTN      mSetupOptionNumber;
 
 
 /**
@@ -361,20 +362,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
-
+  mSetupOptionNumber = RegisterFvBootOption (&mUiFile, L"Enter Setup", (UINTN) -1, LOAD_OPTION_CATEGORY_APP | LOAD_OPTION_ACTIVE | LOAD_OPTION_HIDDEN, NULL, 0);
 }
 
 /**
@@ -463,14 +455,13 @@ RegisterStaticHotkey (
   //
   // [F2]/[F7]
   //
-  F2.Key.ScanCode    = SCAN_F2;
-  F2.Key.UnicodeChar = CHAR_NULL;
-  F2.KeyState.KeyShiftState = EFI_SHIFT_STATE_VALID;
-  F2.KeyState.KeyToggleState = 0;
-  Status = EfiBootManagerGetBootManagerMenu (&BootOption);
-  ASSERT_EFI_ERROR (Status);
-  RegisterBootOptionHotkey ((UINT16) BootOption.OptionNumber, &F2.Key, TRUE);
-  EfiBootManagerFreeLoadOption (&BootOption);
+  if (mSetupOptionNumber) {
+    F2.Key.ScanCode    = SCAN_F2;
+    F2.Key.UnicodeChar = CHAR_NULL;
+    F2.KeyState.KeyShiftState = EFI_SHIFT_STATE_VALID;
+    F2.KeyState.KeyToggleState = 0;
+    RegisterBootOptionHotkey ((UINT16) mSetupOptionNumber, &F2.Key, TRUE);
+  }
 
   F7.Key.ScanCode    = SCAN_F7;
   F7.Key.UnicodeChar = CHAR_NULL;
-- 
2.31.1

Re: [edk2-platforms][PATCH v2 2/2] BoardModulePkg/BoardBdsHookLib: Simplify hotkey registration

[Nate DeSimone]

Hi Benjamin,

In principle this is a good idea, unfortunately some platforms have been coded to do stuff like this:
https://github.com/tianocore/edk2-platforms/blob/784f7739f5afd268042d4d9e8ef570131620c82c/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc#L323

And this:
https://github.com/tianocore/edk2-platforms/blob/784f7739f5afd268042d4d9e8ef570131620c82c/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.dsc#L210

So the only way we can maintain a consistent user experience is to explicitly call out the GUID for the boot device selection menu. For now, I'll just push the first patch in this series.

Thanks,
Nate

-----Original Message-----
From: Benjamin Doron <benjamin.doron00@gmail.com> 
Sent: Friday, August 13, 2021 5:43 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Subject: [edk2-platforms][PATCH v2 2/2] BoardModulePkg/BoardBdsHookLib: Simplify hotkey registration

Retrieve BootOption of BootManagerMenu for registering F7 hotkey, rather than creating an additional boot option.

Tested, both F7 hotkey still opens the list of boot options.

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

diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
index e734e3ad15c3..7ac6c150f2e7 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
@@ -14,7 +14,6 @@ BOOLEAN    mPxeBoot       = FALSE;
 BOOLEAN    mHotKeypressed = FALSE;
 EFI_EVENT  HotKeyEvent    = NULL;
 
-UINTN      mBootMenuOptionNumber;
 UINTN      mSetupOptionNumber;
 
 
@@ -189,9 +188,6 @@ CreateFvBootOption (
 EFI_GUID mUiFile = {
   0x462CAA21, 0x7614, 0x4503, { 0x83, 0x6E, 0x8A, 0xB6, 0xF4, 0x66, 0x23, 0x31 }
 };
-EFI_GUID mBootMenuFile = {
-  0xEEC25BDC, 0x67F2, 0x4D95, { 0xB1, 0xD5, 0xF8, 0x1B, 0x20, 0x39, 0xD1, 0x1D }
-};
 
 
 /**
@@ -354,15 +350,6 @@ RegisterDefaultBootOption (
     ShellDataSize = 0;
     RegisterFvBootOption (&gUefiShellFileGuid,      INTERNAL_UEFI_SHELL_NAME, (UINTN) -1, LOAD_OPTION_ACTIVE, (UINT8 *)ShellData, ShellDataSize);
 
-  //
-  // Boot Menu
-  //
-  mBootMenuOptionNumber = RegisterFvBootOption (&mBootMenuFile, L"Boot Device List",   (UINTN) -1, LOAD_OPTION_CATEGORY_APP | LOAD_OPTION_ACTIVE | LOAD_OPTION_HIDDEN, NULL, 0);
-
-  if (mBootMenuOptionNumber == LoadOptionNumberUnassigned) {
-    DEBUG ((DEBUG_INFO, "BootMenuOptionNumber (%d) should not be same to LoadOptionNumberUnassigned(%d).\n", mBootMenuOptionNumber, LoadOptionNumberUnassigned));
-  }
-
   //
   // Boot Manager Menu
   //
@@ -468,8 +455,10 @@ RegisterStaticHotkey (
   F7.KeyState.KeyShiftState = EFI_SHIFT_STATE_VALID;
   F7.KeyState.KeyToggleState = 0;
   mBootMenuBoot  = !EnterSetup;
-  RegisterBootOptionHotkey ((UINT16) mBootMenuOptionNumber, &F7.Key, mBootMenuBoot);
-
+  Status = EfiBootManagerGetBootManagerMenu (&BootOption);
+  ASSERT_EFI_ERROR (Status);
+  RegisterBootOptionHotkey ((UINT16) BootOption.OptionNumber, &F7.Key, mBootMenuBoot);
+  EfiBootManagerFreeLoadOption (&BootOption);
 }
 
 
-- 
2.31.1

Re: [edk2-platforms][PATCH v2 2/2] BoardModulePkg/BoardBdsHookLib: Simplify hotkey registration

[Benjamin Doron]

[-- Attachment #1: Type: text/plain, Size: 4146 bytes --]

Hi Nate,
That makes sense. I had been concerned that UiApp might be used in some
places, but I didn't expect significant disparity between the boards (and I
didn't check all of them, my mistake).

Maybe trying to align them on some differences, someday, might be a
good idea? But not for now, in any case.


On Thu., Aug. 26, 2021, 7:41 p.m. Desimone, Nathaniel L, <
nathaniel.l.desimone@intel.com> wrote:

> Hi Benjamin,
>
> In principle this is a good idea, unfortunately some platforms have been
> coded to do stuff like this:
>
> https://github.com/tianocore/edk2-platforms/blob/784f7739f5afd268042d4d9e8ef570131620c82c/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc#L323
>
> And this:
>
> https://github.com/tianocore/edk2-platforms/blob/784f7739f5afd268042d4d9e8ef570131620c82c/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.dsc#L210
>
> So the only way we can maintain a consistent user experience is to
> explicitly call out the GUID for the boot device selection menu. For now,
> I'll just push the first patch in this series.
>
> Thanks,
> Nate
>
> -----Original Message-----
> From: Benjamin Doron <benjamin.doron00@gmail.com>
> Sent: Friday, August 13, 2021 5:43 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Subject: [edk2-platforms][PATCH v2 2/2] BoardModulePkg/BoardBdsHookLib:
> Simplify hotkey registration
>
> Retrieve BootOption of BootManagerMenu for registering F7 hotkey, rather
> than creating an additional boot option.
>
> Tested, both F7 hotkey still opens the list of boot options.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
> ---
> Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c |
> 19 ++++---------------
>  1 file changed, 4 insertions(+), 15 deletions(-)
>
> diff --git
> a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
> b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
> index e734e3ad15c3..7ac6c150f2e7 100644
> ---
> a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
> +++
> b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
> @@ -14,7 +14,6 @@ BOOLEAN    mPxeBoot       = FALSE;
>  BOOLEAN    mHotKeypressed = FALSE;
>  EFI_EVENT  HotKeyEvent    = NULL;
>
> -UINTN      mBootMenuOptionNumber;
>  UINTN      mSetupOptionNumber;
>
>
> @@ -189,9 +188,6 @@ CreateFvBootOption (
>  EFI_GUID mUiFile = {
>    0x462CAA21, 0x7614, 0x4503, { 0x83, 0x6E, 0x8A, 0xB6, 0xF4, 0x66, 0x23,
> 0x31 }
>  };
> -EFI_GUID mBootMenuFile = {
> -  0xEEC25BDC, 0x67F2, 0x4D95, { 0xB1, 0xD5, 0xF8, 0x1B, 0x20, 0x39, 0xD1,
> 0x1D }
> -};
>
>
>  /**
> @@ -354,15 +350,6 @@ RegisterDefaultBootOption (
>      ShellDataSize = 0;
>      RegisterFvBootOption (&gUefiShellFileGuid,
> INTERNAL_UEFI_SHELL_NAME, (UINTN) -1, LOAD_OPTION_ACTIVE, (UINT8
> *)ShellData, ShellDataSize);
>
> -  //
> -  // Boot Menu
> -  //
> -  mBootMenuOptionNumber = RegisterFvBootOption (&mBootMenuFile, L"Boot
> Device List",   (UINTN) -1, LOAD_OPTION_CATEGORY_APP | LOAD_OPTION_ACTIVE |
> LOAD_OPTION_HIDDEN, NULL, 0);
> -
> -  if (mBootMenuOptionNumber == LoadOptionNumberUnassigned) {
> -    DEBUG ((DEBUG_INFO, "BootMenuOptionNumber (%d) should not be same to
> LoadOptionNumberUnassigned(%d).\n", mBootMenuOptionNumber,
> LoadOptionNumberUnassigned));
> -  }
> -
>    //
>    // Boot Manager Menu
>    //
> @@ -468,8 +455,10 @@ RegisterStaticHotkey (
>    F7.KeyState.KeyShiftState = EFI_SHIFT_STATE_VALID;
>    F7.KeyState.KeyToggleState = 0;
>    mBootMenuBoot  = !EnterSetup;
> -  RegisterBootOptionHotkey ((UINT16) mBootMenuOptionNumber, &F7.Key,
> mBootMenuBoot);
> -
> +  Status = EfiBootManagerGetBootManagerMenu (&BootOption);
> +  ASSERT_EFI_ERROR (Status);
> +  RegisterBootOptionHotkey ((UINT16) BootOption.OptionNumber, &F7.Key,
> mBootMenuBoot);
> +  EfiBootManagerFreeLoadOption (&BootOption);
>  }
>
>
> --
> 2.31.1
>
>

[-- Attachment #2: Type: text/html, Size: 6041 bytes --]

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

[Nate DeSimone]

Pushed: https://github.com/tianocore/edk2-platforms/commit/b401dfd

-----Original Message-----
From: Benjamin Doron <benjamin.doron00@gmail.com> 
Sent: Friday, August 13, 2021 5:43 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Subject: [edk2-platforms][PATCH v2 1/2] 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.

The F2 hotkey can be used to enter UiApp.

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

diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
index a37139a0074e..0bcee7c9a4ba 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHook
+++ Lib.c
@@ -1385,7 +1385,7 @@ BdsAfterConsoleReadyBeforeBootOptionCallback (
       break;   } -  Print (L"Press F7 for BootMenu!\n");+  Print (L"Press F2 for Setup, or F7 for BootMenu!\n");   }diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
index 87138bdd79ff..e734e3ad15c3 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOpt
+++ ion.c
@@ -15,6 +15,7 @@ BOOLEAN    mHotKeypressed = FALSE;
 EFI_EVENT  HotKeyEvent    = NULL;  UINTN      mBootMenuOptionNumber;+UINTN      mSetupOptionNumber;   /**@@ -361,20 +362,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-+  mSetupOptionNumber = RegisterFvBootOption (&mUiFile, L"Enter Setup", (UINTN) -1, LOAD_OPTION_CATEGORY_APP | LOAD_OPTION_ACTIVE | LOAD_OPTION_HIDDEN, NULL, 0); }  /**@@ -463,14 +455,13 @@ RegisterStaticHotkey (
   //   // [F2]/[F7]   //-  F2.Key.ScanCode    = SCAN_F2;-  F2.Key.UnicodeChar = CHAR_NULL;-  F2.KeyState.KeyShiftState = EFI_SHIFT_STATE_VALID;-  F2.KeyState.KeyToggleState = 0;-  Status = EfiBootManagerGetBootManagerMenu (&BootOption);-  ASSERT_EFI_ERROR (Status);-  RegisterBootOptionHotkey ((UINT16) BootOption.OptionNumber, &F2.Key, TRUE);-  EfiBootManagerFreeLoadOption (&BootOption);+  if (mSetupOptionNumber) {+    F2.Key.ScanCode    = SCAN_F2;+    F2.Key.UnicodeChar = CHAR_NULL;+    F2.KeyState.KeyShiftState = EFI_SHIFT_STATE_VALID;+    F2.KeyState.KeyToggleState = 0;+    RegisterBootOptionHotkey ((UINT16) mSetupOptionNumber, &F2.Key, TRUE);+  }    F7.Key.ScanCode    = SCAN_F7;   F7.Key.UnicodeChar = CHAR_NULL;-- 
2.31.1

Re: [edk2-platforms][PATCH v2 2/2] BoardModulePkg/BoardBdsHookLib: Simplify hotkey registration

[Nate DeSimone]

[-- Attachment #1: Type: text/plain, Size: 4986 bytes --]

Yes agreed, making this more consistent would be a good improvement and one that I would support. There is always the perennial issue of finding the time to make those improvements and balancing that with everything else that needs to get done 😊.

From: Benjamin Doron <benjamin.doron00@gmail.com>
Sent: Thursday, August 26, 2021 4:50 PM
To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Cc: devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
Subject: Re: [edk2-platforms][PATCH v2 2/2] BoardModulePkg/BoardBdsHookLib: Simplify hotkey registration

Hi Nate,
That makes sense. I had been concerned that UiApp might be used in some places, but I didn't expect significant disparity between the boards (and I didn't check all of them, my mistake).

Maybe trying to align them on some differences, someday, might be a good idea? But not for now, in any case.

On Thu., Aug. 26, 2021, 7:41 p.m. Desimone, Nathaniel L, <nathaniel.l.desimone@intel.com<mailto:nathaniel.l.desimone@intel.com>> wrote:
Hi Benjamin,

In principle this is a good idea, unfortunately some platforms have been coded to do stuff like this:
https://github.com/tianocore/edk2-platforms/blob/784f7739f5afd268042d4d9e8ef570131620c82c/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc#L323

And this:
https://github.com/tianocore/edk2-platforms/blob/784f7739f5afd268042d4d9e8ef570131620c82c/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.dsc#L210

So the only way we can maintain a consistent user experience is to explicitly call out the GUID for the boot device selection menu. For now, I'll just push the first patch in this series.

Thanks,
Nate

-----Original Message-----
From: Benjamin Doron <benjamin.doron00@gmail.com<mailto:benjamin.doron00@gmail.com>>
Sent: Friday, August 13, 2021 5:43 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com<mailto:nathaniel.l.desimone@intel.com>>
Subject: [edk2-platforms][PATCH v2 2/2] BoardModulePkg/BoardBdsHookLib: Simplify hotkey registration

Retrieve BootOption of BootManagerMenu for registering F7 hotkey, rather than creating an additional boot option.

Tested, both F7 hotkey still opens the list of boot options.

Cc: Eric Dong <eric.dong@intel.com<mailto:eric.dong@intel.com>>
Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com<mailto:nathaniel.l.desimone@intel.com>>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com<mailto:benjamin.doron00@gmail.com>>
---
Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
index e734e3ad15c3..7ac6c150f2e7 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBootOption.c
@@ -14,7 +14,6 @@ BOOLEAN    mPxeBoot       = FALSE;
 BOOLEAN    mHotKeypressed = FALSE;
 EFI_EVENT  HotKeyEvent    = NULL;

-UINTN      mBootMenuOptionNumber;
 UINTN      mSetupOptionNumber;


@@ -189,9 +188,6 @@ CreateFvBootOption (
 EFI_GUID mUiFile = {
   0x462CAA21, 0x7614, 0x4503, { 0x83, 0x6E, 0x8A, 0xB6, 0xF4, 0x66, 0x23, 0x31 }
 };
-EFI_GUID mBootMenuFile = {
-  0xEEC25BDC, 0x67F2, 0x4D95, { 0xB1, 0xD5, 0xF8, 0x1B, 0x20, 0x39, 0xD1, 0x1D }
-};


 /**
@@ -354,15 +350,6 @@ RegisterDefaultBootOption (
     ShellDataSize = 0;
     RegisterFvBootOption (&gUefiShellFileGuid,      INTERNAL_UEFI_SHELL_NAME, (UINTN) -1, LOAD_OPTION_ACTIVE, (UINT8 *)ShellData, ShellDataSize);

-  //
-  // Boot Menu
-  //
-  mBootMenuOptionNumber = RegisterFvBootOption (&mBootMenuFile, L"Boot Device List",   (UINTN) -1, LOAD_OPTION_CATEGORY_APP | LOAD_OPTION_ACTIVE | LOAD_OPTION_HIDDEN, NULL, 0);
-
-  if (mBootMenuOptionNumber == LoadOptionNumberUnassigned) {
-    DEBUG ((DEBUG_INFO, "BootMenuOptionNumber (%d) should not be same to LoadOptionNumberUnassigned(%d).\n", mBootMenuOptionNumber, LoadOptionNumberUnassigned));
-  }
-
   //
   // Boot Manager Menu
   //
@@ -468,8 +455,10 @@ RegisterStaticHotkey (
   F7.KeyState.KeyShiftState = EFI_SHIFT_STATE_VALID;
   F7.KeyState.KeyToggleState = 0;
   mBootMenuBoot  = !EnterSetup;
-  RegisterBootOptionHotkey ((UINT16) mBootMenuOptionNumber, &F7.Key, mBootMenuBoot);
-
+  Status = EfiBootManagerGetBootManagerMenu (&BootOption);
+  ASSERT_EFI_ERROR (Status);
+  RegisterBootOptionHotkey ((UINT16) BootOption.OptionNumber, &F7.Key, mBootMenuBoot);
+  EfiBootManagerFreeLoadOption (&BootOption);
 }


--
2.31.1

[-- Attachment #2: Type: text/html, Size: 8862 bytes --]