[PATCH edk2-platforms 0/4] Fix the boot order and the boot menu

[PATCH edk2-platforms 0/4] Fix the boot order and the boot menu

[Gary Lin]

After switching to MdeModulePkg BdsDxe, the boot option created by OS
was always put at the end of BootOrder after reboot. Besides, the PXE
boot options were always the first boot options.

This series fixes the priority of boot options and stops sorting the
boot options after the first boot. The missing BootManagerMenuApp is
also added back.

The last patch cleans up PlatformBootOption.c to remove the unused
variables and function.

The patches are also available in my github branch:
https://github.com/lcp/edk2-platforms/tree/fix-minnowboard-bds

Gary Lin (4):
  Vlv2TbltDevicePkg: Adjust the device priority
  Vlv2TbltDevicePkg: Add the missing BootManagerMenuApp
  Vlv2TbltDevicePkg: Only sort boot options when necessary
  Vlv2TbltDevicePkg: Clean up the unused variables and function

 Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/BdsPlatform.c        |  9 +--
 Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/PlatformBootOption.c | 85 ++++++--------------
 Platform/Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf                                        |  3 +-
 Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc                                    |  1 +
 Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc                                     |  1 +
 5 files changed, 33 insertions(+), 66 deletions(-)

-- 
2.22.0

[PATCH edk2-platforms 1/4] Vlv2TbltDevicePkg: Adjust the device priority

[Gary Lin]

In BootOptionPriority(), the higher number implies the lower boot
priority. This commit lowers the priority of network booting and raises
the priority of the local devices. Besides, SD and EMMC are also added
to BootOptionPriority() since Minnowboard comes with a SD card slot.

Cc: Zailiang Sun <zailiang.sun@intel.com>
Cc: Yi Qian <yi.qian@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Gary Lin <glin@suse.com>
---
 Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/PlatformBootOption.c | 28 ++++++++++----------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/PlatformBootOption.c b/Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/PlatformBootOption.c
index 84aa097d58..a73d54a97d 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/PlatformBootOption.c
+++ b/Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/PlatformBootOption.c
@@ -499,11 +499,10 @@ BootOptionType (
   Returns the priority number.
   OptionType                 EFI
   ------------------------------------
-  PXE                         2
-  DVD                         4
-  USB                         6
-  NVME                        7
-  HDD                         8
+  HDD                         2
+  USB                         4
+  SATA/NVME/SD                6
+  PXE/HTTP                    8
   EFI Shell                   9
   Others                      100
 
@@ -518,21 +517,22 @@ BootOptionPriority (
     // EFI boot options
     //
     switch (BootOptionType (BootOption->FilePath)) {
-    case MSG_MAC_ADDR_DP:
-    case MSG_VLAN_DP:
-    case MSG_IPv4_DP:
-    case MSG_IPv6_DP:
-      return 2;
+    case MSG_USB_DP:
+      return 4;
 
     case MSG_SATA_DP:
     case MSG_ATAPI_DP:
     case MSG_UFS_DP:
     case MSG_NVME_NAMESPACE_DP:
-      return 4;
-
-    case MSG_USB_DP:
+    case MSG_SD_DP:
+    case MSG_EMMC_DP:
       return 6;
 
+    case MSG_MAC_ADDR_DP:
+    case MSG_VLAN_DP:
+    case MSG_IPv4_DP:
+    case MSG_IPv6_DP:
+      return 8;
     }
     if (StrCmp (BootOption->Description, INTERNAL_UEFI_SHELL_NAME) == 0) {
       if (PcdGetBool (PcdBootToShellOnly)) {
@@ -541,7 +541,7 @@ BootOptionPriority (
       return 9;
     }
     if (StrCmp (BootOption->Description, UEFI_HARD_DRIVE_NAME) == 0) {
-      return 8;
+      return 2;
     }
     return 100;
 }
-- 
2.22.0

[PATCH edk2-platforms 2/4] Vlv2TbltDevicePkg: Add the missing BootManagerMenuApp

[Gary Lin]

BootManagerMenuApp wasn't imported so the Boot Device Menu never showed
correctly.

Also fix the description of UiApp.

Cc: Zailiang Sun <zailiang.sun@intel.com>
Cc: Yi Qian <yi.qian@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Gary Lin <glin@suse.com>
---
 Platform/Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf     | 3 ++-
 Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 1 +
 Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc  | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf
index 48058b663a..55a309444f 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf
@@ -417,7 +417,8 @@ INF UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
 INF MdeModulePkg/Universal/Metronome/Metronome.inf
 INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
 INF MdeModulePkg/Logo/LogoDxe.inf
-INF MdeModulePkg/Application/UiApp/UiApp.inf
+INF RuleOverride = UI MdeModulePkg/Application/UiApp/UiApp.inf
+INF MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuApp.inf
 INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
 INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
 INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
index 30b47d8f32..463b952e65 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
@@ -924,6 +924,7 @@
       NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf
       NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
   }
+  MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuApp.inf
 
   MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
   MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
index 57316d487b..ee18b45c97 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
@@ -939,6 +939,7 @@
       NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf
       NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
   }
+  MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuApp.inf
 
   MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
   MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
-- 
2.22.0

[PATCH edk2-platforms 3/4] Vlv2TbltDevicePkg: Only sort boot options when necessary

[Gary Lin]

* PlatformBootManagerAfterConsole() always sorted the boot options at
  the end, and this would invalidate the BootOrder created by the user.
  Besides, the current implementation would sort the boot options twice,
  and this is not efficient. We should just sort the boot options once
  at the end of the function only for the very first boot.

* Update the string in IsNeedSortBootOption() to match the descriptions
  of BootManagerMenuApp.

Cc: Zailiang Sun <zailiang.sun@intel.com>
Cc: Yi Qian <yi.qian@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Gary Lin <glin@suse.com>
---
 Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/BdsPlatform.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/BdsPlatform.c b/Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/BdsPlatform.c
index 7fbbf281c6..a5423013a5 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/BdsPlatform.c
+++ b/Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/BdsPlatform.c
@@ -1299,7 +1299,7 @@ IsNeedSortBootOption (
   //
   if ((BootOptionCount > 1) &&
       (((StrnCmp (BootOptions->Description, L"Enter Setup", StrLen (L"Enter Setup"))) == 0) ||
-       ((StrnCmp (BootOptions->Description, L"BootManagerMenuApp", StrLen (L"BootManagerMenuApp"))) == 0))) {
+       ((StrnCmp (BootOptions->Description, L"Boot Device List", StrLen (L"Boot Device List"))) == 0))) {
     return TRUE;
   }
 
@@ -1374,9 +1374,6 @@ PlatformBootManagerAfterConsole (
     //
     EfiBootManagerRefreshAllBootOption ();
 
-    if (IsNeedSortBootOption()) {
-      EfiBootManagerSortLoadOptionVariable (LoadOptionTypeBoot, CompareBootOption);
-    }
     //
     // PXE boot option may appear after boot option enumeration
     //
@@ -1400,7 +1397,9 @@ PlatformBootManagerAfterConsole (
   Print (L"Press F7 for BootMenu!\n");
 
   EfiBootManagerRefreshAllBootOption ();
-  EfiBootManagerSortLoadOptionVariable (LoadOptionTypeBoot, CompareBootOption);
+  if (IsNeedSortBootOption()) {
+    EfiBootManagerSortLoadOptionVariable (LoadOptionTypeBoot, CompareBootOption);
+  }
 }
 
 /**
-- 
2.22.0

[PATCH edk2-platforms 4/4] Vlv2TbltDevicePkg: Clean up the unused variables and function

[Gary Lin]

In PlatformBootOption.c:

* mContinueBoot and mBootMenuBoot were assigned as "!EnterSetup" before
  being used. However, EnterSetup is always "FALSE", so it's meaningless
  to keep those variables.

* mPxeBoot is defined but never used.

* DetectKeypressCallback() is never used. So we can remove it altogether
  with mHotKeypressed and HotKeyEvent.

Cc: Zailiang Sun <zailiang.sun@intel.com>
Cc: Yi Qian <yi.qian@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Gary Lin <glin@suse.com>
---
 Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/PlatformBootOption.c | 57 ++++----------------
 1 file changed, 11 insertions(+), 46 deletions(-)

diff --git a/Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/PlatformBootOption.c b/Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/PlatformBootOption.c
index a73d54a97d..539127e93f 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/PlatformBootOption.c
+++ b/Platform/Intel/Vlv2TbltDevicePkg/Library/DxePlatformBootManagerLib/PlatformBootOption.c
@@ -10,12 +10,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include <Library/PcdLib.h>
 
-BOOLEAN    mContinueBoot  = FALSE;
-BOOLEAN    mBootMenuBoot  = FALSE;
-BOOLEAN    mPxeBoot       = FALSE;
-BOOLEAN    mHotKeypressed = FALSE;
-EFI_EVENT  HotKeyEvent    = NULL;
-
 UINTN      mBootMenuOptionNumber;
 
 EFI_DEVICE_PATH_PROTOCOL *
@@ -396,21 +390,6 @@ RegisterBootOptionHotkey (
   }
 }
 
-EFI_STATUS
-EFIAPI
-DetectKeypressCallback (
-  IN EFI_KEY_DATA     *KeyData
-)
-{
-  mHotKeypressed = TRUE;
-
-  if (HotKeyEvent != NULL) {
-    gBS->SignalEvent(HotKeyEvent);
-  }
-
-  return EFI_SUCCESS;
-}
-
 /**
   This function is called after all the boot options are enumerated and ordered properly.
 **/
@@ -419,46 +398,32 @@ RegisterStaticHotkey (
   VOID
   )
 {
-
   EFI_INPUT_KEY                 Enter;
-  EFI_KEY_DATA                  F2;
-  EFI_KEY_DATA                  F7;
-  BOOLEAN                       EnterSetup;
+  EFI_INPUT_KEY                 F2;
+  EFI_INPUT_KEY                 F7;
   EFI_STATUS                    Status;
   EFI_BOOT_MANAGER_LOAD_OPTION  BootOption;
 
-  EnterSetup = FALSE;
-
   //
   // [Enter]
   //
-  mContinueBoot = !EnterSetup;
-  if (mContinueBoot) {
-    Enter.ScanCode    = SCAN_NULL;
-    Enter.UnicodeChar = CHAR_CARRIAGE_RETURN;
-    EfiBootManagerRegisterContinueKeyOption (0, &Enter, NULL);
-  }
-
+  Enter.ScanCode    = SCAN_NULL;
+  Enter.UnicodeChar = CHAR_CARRIAGE_RETURN;
+  EfiBootManagerRegisterContinueKeyOption (0, &Enter, NULL);
 
   //
   // [F2]/[F7]
   //
-  F2.Key.ScanCode    = SCAN_F2;
-  F2.Key.UnicodeChar = CHAR_NULL;
-  F2.KeyState.KeyShiftState = EFI_SHIFT_STATE_VALID;
-  F2.KeyState.KeyToggleState = 0;
+  F2.ScanCode    = SCAN_F2;
+  F2.UnicodeChar = CHAR_NULL;
   Status = EfiBootManagerGetBootManagerMenu (&BootOption);
   ASSERT_EFI_ERROR (Status);
-  RegisterBootOptionHotkey ((UINT16) BootOption.OptionNumber, &F2.Key, TRUE);
+  RegisterBootOptionHotkey ((UINT16) BootOption.OptionNumber, &F2, TRUE);
   EfiBootManagerFreeLoadOption (&BootOption);
 
-  F7.Key.ScanCode    = SCAN_F7;
-  F7.Key.UnicodeChar = CHAR_NULL;
-  F7.KeyState.KeyShiftState = EFI_SHIFT_STATE_VALID;
-  F7.KeyState.KeyToggleState = 0;
-  mBootMenuBoot  = !EnterSetup;
-  RegisterBootOptionHotkey ((UINT16) mBootMenuOptionNumber, &F7.Key, mBootMenuBoot);
-
+  F7.ScanCode    = SCAN_F7;
+  F7.UnicodeChar = CHAR_NULL;
+  RegisterBootOptionHotkey ((UINT16) mBootMenuOptionNumber, &F7, TRUE);
 }
 
 UINT8
-- 
2.22.0

Re: [edk2-devel] [PATCH edk2-platforms 2/4] Vlv2TbltDevicePkg: Add the missing BootManagerMenuApp

[Gary Lin]

On Fri, Jul 26, 2019 at 07:06:39AM +0000,  Gary Lin  wrote:
> BootManagerMenuApp wasn't imported so the Boot Device Menu never showed
> correctly.
> 
I should add more details for the fix below.

> Also fix the description of UiApp.
In the "Rule.Common.UEFI_APPLICATION.UI" section, it intends to change
the description of UiApp to "Enter Setup" to make the boot option
self-explaining in the Boot Menu. But this didn't happen since the rule
wasn't assigned to UiApp correctly, so the Boot Menu only showed "UiApp"
which is not understandable to users. This patch adds "RuleOverride = UI"
to UiApp.inf to change the description.


I'll add the above paragraph in case there is a v2 for this series.

Gary Lin

> Cc: Zailiang Sun <zailiang.sun@intel.com>
> Cc: Yi Qian <yi.qian@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Gary Lin <glin@suse.com>
> ---
>  Platform/Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf     | 3 ++-
>  Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 1 +
>  Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc  | 1 +
>  3 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf
> index 48058b663a..55a309444f 100644
> --- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf
> +++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkg.fdf
> @@ -417,7 +417,8 @@ INF UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
>  INF MdeModulePkg/Universal/Metronome/Metronome.inf
>  INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
>  INF MdeModulePkg/Logo/LogoDxe.inf
> -INF MdeModulePkg/Application/UiApp/UiApp.inf
> +INF RuleOverride = UI MdeModulePkg/Application/UiApp/UiApp.inf
> +INF MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuApp.inf
>  INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
>  INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
>  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
> index 30b47d8f32..463b952e65 100644
> --- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
> +++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
> @@ -924,6 +924,7 @@
>        NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf
>        NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
>    }
> +  MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuApp.inf
>
>    MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
>    MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
> diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
> index 57316d487b..ee18b45c97 100644
> --- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
> +++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
> @@ -939,6 +939,7 @@
>        NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf
>        NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
>    }
> +  MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuApp.inf
>
>    MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
>    MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
> -- 
> 2.22.0
> 
> 
> 
> 
>