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

["Benjamin Doron" <benjamin.doron00@gmail.com>]

[-- 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 --]