[PATCH 02/18] UefiPayloadPkg: Provide option to use Boot Splash

[PATCH 02/18] UefiPayloadPkg: Provide option to use Boot Splash

[Sean Rhodes]

Provide a build option to use a Boot Splash logo.

Signed-off-by: Sean Rhodes <sean@starlabs.systems>
---
 .../Library/PlatformBootManagerLib/PlatformBootManager.c   | 5 +++++
 .../PlatformBootManagerLib/PlatformBootManagerLib.inf      | 2 ++
 UefiPayloadPkg/UefiPayloadPkg.dec                          | 3 +++
 UefiPayloadPkg/UefiPayloadPkg.dsc                          | 7 +++++++
 UefiPayloadPkg/UefiPayloadPkg.fdf                          | 3 +++
 5 files changed, 20 insertions(+)

diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
index a8ead775ea..3bded489ef 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
@@ -243,6 +243,11 @@ PlatformBootManagerAfterConsole (
   Black.Blue = Black.Green = Black.Red = Black.Reserved = 0;
   White.Blue = White.Green = White.Red = White.Reserved = 0xFF;
 
+  if (FixedPcdGetBool (PcdBootSplashImage)) {
+    gST->ConOut->ClearScreen (gST->ConOut);
+    BootLogoEnableLogo ();
+  }
+
   EfiBootManagerConnectAll ();
   EfiBootManagerRefreshAllBootOption ();
 
diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 9c4a9da943..306bd33b7a 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -39,6 +39,7 @@
   UefiRuntimeServicesTableLib
   UefiLib
   UefiBootManagerLib
+  BootLogoLib
   PcdLib
   DxeServicesLib
   MemoryAllocationLib
@@ -73,3 +74,4 @@
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
   gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile
+  gUefiPayloadPkgTokenSpaceGuid.PcdBootSplashImage
diff --git a/UefiPayloadPkg/UefiPayloadPkg.dec b/UefiPayloadPkg/UefiPayloadPkg.dec
index 551f0a4915..4f5756d575 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dec
+++ b/UefiPayloadPkg/UefiPayloadPkg.dec
@@ -83,6 +83,9 @@ gUefiPayloadPkgTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x04000000|UINT32|0x
 
 gUefiPayloadPkgTokenSpaceGuid.PcdPcdDriverFile|{ 0x57, 0x72, 0xcf, 0x80, 0xab, 0x87, 0xf9, 0x47, 0xa3, 0xfe, 0xD5, 0x0B, 0x76, 0xd8, 0x95, 0x41 }|VOID*|0x00000018
 
+# BootSplash Image
+gUefiPayloadPkgTokenSpaceGuid.PcdBootSplashImage|TRUE|BOOLEAN|0x00000021
+
 ## FFS filename to find the default variable initial data file.
 # @Prompt FFS Name of variable initial data file
  gUefiPayloadPkgTokenSpaceGuid.PcdNvsDataFile |{ 0x1a, 0xf1, 0xb1, 0xae, 0x42, 0xcc, 0xcf, 0x4e, 0xac, 0x60, 0xdb, 0xab, 0xf6, 0xca, 0x69, 0xe6 }|VOID*|0x00000025
diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index fd5739499a..282435ceec 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -33,6 +33,7 @@
   DEFINE UNIVERSAL_PAYLOAD            = FALSE
   DEFINE SECURITY_STUB_ENABLE         = TRUE
   DEFINE SMM_SUPPORT                  = FALSE
+  DEFINE BOOTSPLASH_IMAGE             = FALSE
   #
   # SBL:      UEFI payload for Slim Bootloader
   # COREBOOT: UEFI payload for coreboot
@@ -209,6 +210,7 @@
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
   SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
   UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
+  BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
   CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
   FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
 
@@ -399,6 +401,8 @@
   gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask       | 0x1
 !endif
 
+  gUefiPayloadPkgTokenSpaceGuid.PcdBootSplashImage|$(BOOTSPLASH_IMAGE)
+
 [PcdsPatchableInModule.X64]
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister|$(RTC_INDEX_REGISTER)
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister|$(RTC_TARGET_REGISTER)
@@ -549,6 +553,9 @@
 !endif
   UefiCpuPkg/CpuDxe/CpuDxe.inf
   MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+!if $(BOOTSPLASH_IMAGE) == TRUE
+  MdeModulePkg/Logo/LogoDxe.inf
+!endif
   MdeModulePkg/Application/UiApp/UiApp.inf {
     <LibraryClasses>
       NULL|MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf
diff --git a/UefiPayloadPkg/UefiPayloadPkg.fdf b/UefiPayloadPkg/UefiPayloadPkg.fdf
index c7b04978ad..a71d655687 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.fdf
+++ b/UefiPayloadPkg/UefiPayloadPkg.fdf
@@ -158,6 +158,9 @@ INF MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
 INF UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
 
 INF MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
+!if $(BOOTSPLASH_IMAGE) == TRUE
+INF MdeModulePkg/Logo/LogoDxe.inf
+!endif
 #
 # PCI Support
 #
-- 
2.32.0

[PATCH 03/18] MdeModulePackage: Add option to follow BGRT spec

[Sean Rhodes]

Add option to centre the Boot Logo 38.2% from the top of screen, following
the BGRT specification.

Signed-off-by: Sean Rhodes <sean@starlabs.systems>
---
 MdeModulePkg/Library/BootLogoLib/BootLogoLib.c   | 7 ++++++-
 MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf | 3 +++
 MdeModulePkg/MdeModulePkg.dec                    | 3 +++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
index 478ec2d40e..928f37a2e1 100644
--- a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
+++ b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
@@ -176,7 +176,12 @@ BootLogoEnableLogo (
         break;
       case EdkiiPlatformLogoDisplayAttributeCenter:
         DestX = (SizeOfX - Image.Width) / 2;
-        DestY = (SizeOfY - Image.Height) / 2;
+        if (FixedPcdGetBool (PcdFollowBGRTSpec)) {
+          DestY = (SizeOfY * 382) / 1000 - Image.Height / 2;
+        } else {
+          DestY = (SizeOfY - Image.Height) / 2;
+        }
+
         break;
       case EdkiiPlatformLogoDisplayAttributeCenterRight:
         DestX = SizeOfX - Image.Width;
diff --git a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
index 7d50f2dfa3..03ff038f47 100644
--- a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
+++ b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
@@ -48,5 +48,8 @@
   gEfiUserManagerProtocolGuid                   ## CONSUMES
   gEdkiiPlatformLogoProtocolGuid                ## CONSUMES
 
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFollowBGRTSpec
+
 [FeaturePcd]
   gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport ## CONSUMES
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 463e889e9a..b4855431b0 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -2079,6 +2079,9 @@
   # @Prompt Enable PCIe Resizable BAR Capability support.
   gEfiMdeModulePkgTokenSpaceGuid.PcdPcieResizableBarSupport|FALSE|BOOLEAN|0x10000024
 
+  # Follow BGRT Specifcation
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFollowBGRTSpec|FALSE|BOOLEAN|0x00000025
+
 [PcdsPatchableInModule]
   ## Specify memory size with page number for PEI code when
   #  Loading Module at Fixed Address feature is enabled.
-- 
2.32.0

[PATCH 04/18] UefiPayloadPkg: Hookup BGRT build option

[Sean Rhodes]

Hook FOLLOW_BGRT_SPEC build option to FollowBGRTSpec PCD.

Signed-off-by: Sean Rhodes <sean@starlabs.systems>
---
 UefiPayloadPkg/UefiPayloadPkg.dsc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 282435ceec..82adf98a45 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -34,6 +34,7 @@
   DEFINE SECURITY_STUB_ENABLE         = TRUE
   DEFINE SMM_SUPPORT                  = FALSE
   DEFINE BOOTSPLASH_IMAGE             = FALSE
+  DEFINE FOLLOW_BGRT_SPEC             = FALSE
   #
   # SBL:      UEFI payload for Slim Bootloader
   # COREBOOT: UEFI payload for coreboot
@@ -400,6 +401,7 @@
 !if $(PERFORMANCE_MEASUREMENT_ENABLE)
   gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask       | 0x1
 !endif
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFollowBGRTSpec|$(FOLLOW_BGRT_SPEC)
 
   gUefiPayloadPkgTokenSpaceGuid.PcdBootSplashImage|$(BOOTSPLASH_IMAGE)
 
-- 
2.32.0

[PATCH 05/18] UefiPayloadPkg: Make Boot Manager Key configurable

[Sean Rhodes]

Provide a build option to use [Esc] instead of [F2] for devices
such as Chromebooks that don't have F-keys.

Signed-off-by: Sean Rhodes <sean@starlabs.systems>
---
 .../PlatformBootManager.c                     | 44 +++++++++++++------
 .../PlatformBootManagerLib.inf                |  2 +
 UefiPayloadPkg/UefiPayloadPkg.dec             |  3 ++
 UefiPayloadPkg/UefiPayloadPkg.dsc             |  4 ++
 4 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
index 3bded489ef..6174ca7896 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
@@ -164,7 +164,7 @@ PlatformBootManagerBeforeConsole (
   )
 {
   EFI_INPUT_KEY                 Enter;
-  EFI_INPUT_KEY                 F2;
+  EFI_INPUT_KEY                 CustomKey;
   EFI_INPUT_KEY                 Down;
   EFI_BOOT_MANAGER_LOAD_OPTION  BootOption;
   EFI_STATUS                    Status;
@@ -186,13 +186,22 @@ PlatformBootManagerBeforeConsole (
   Enter.UnicodeChar = CHAR_CARRIAGE_RETURN;
   EfiBootManagerRegisterContinueKeyOption (0, &Enter, NULL);
 
-  //
-  // Map F2 to Boot Manager Menu
-  //
-  F2.ScanCode    = SCAN_F2;
-  F2.UnicodeChar = CHAR_NULL;
+  if (FixedPcdGetBool (PcdBootManagerEscape)) {
+    //
+    // Map Esc to Boot Manager Menu
+    //
+    CustomKey.ScanCode    = SCAN_ESC;
+    CustomKey.UnicodeChar = CHAR_NULL;
+  } else {
+    //
+    // Map Esc to Boot Manager Menu
+    //
+    CustomKey.ScanCode    = SCAN_F2;
+    CustomKey.UnicodeChar = CHAR_NULL;
+  }
+
   EfiBootManagerGetBootManagerMenu (&BootOption);
-  EfiBootManagerAddKeyOptionVariable (NULL, (UINT16)BootOption.OptionNumber, 0, &F2, NULL);
+  EfiBootManagerAddKeyOptionVariable (NULL, (UINT16)BootOption.OptionNumber, 0, &CustomKey, NULL);
 
   //
   // Also add Down key to Boot Manager Menu since some serial terminals don't support F2 key.
@@ -256,12 +265,21 @@ PlatformBootManagerAfterConsole (
   //
   PlatformRegisterFvBootOption (PcdGetPtr (PcdShellFile), L"UEFI Shell", LOAD_OPTION_ACTIVE);
 
-  Print (
-    L"\n"
-    L"F2 or Down      to enter Boot Manager Menu.\n"
-    L"ENTER           to boot directly.\n"
-    L"\n"
-    );
+  if (FixedPcdGetBool (PcdBootManagerEscape)) {
+    Print (
+      L"\n"
+      L"Esc or Down      to enter Boot Manager Menu.\n"
+      L"ENTER           to boot directly.\n"
+      L"\n"
+      );
+  } else {
+    Print (
+      L"\n"
+      L"F2 or Down      to enter Boot Manager Menu.\n"
+      L"ENTER           to boot directly.\n"
+      L"\n"
+      );
+  }
 }
 
 /**
diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 306bd33b7a..c0bfed26cf 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -75,3 +75,5 @@
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
   gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile
   gUefiPayloadPkgTokenSpaceGuid.PcdBootSplashImage
+  gUefiPayloadPkgTokenSpaceGuid.PcdBootManagerEscape
+
diff --git a/UefiPayloadPkg/UefiPayloadPkg.dec b/UefiPayloadPkg/UefiPayloadPkg.dec
index 4f5756d575..b01b553cdd 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dec
+++ b/UefiPayloadPkg/UefiPayloadPkg.dec
@@ -83,6 +83,9 @@ gUefiPayloadPkgTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x04000000|UINT32|0x
 
 gUefiPayloadPkgTokenSpaceGuid.PcdPcdDriverFile|{ 0x57, 0x72, 0xcf, 0x80, 0xab, 0x87, 0xf9, 0x47, 0xa3, 0xfe, 0xD5, 0x0B, 0x76, 0xd8, 0x95, 0x41 }|VOID*|0x00000018
 
+# Boot Manager Key
+gUefiPayloadPkgTokenSpaceGuid.PcdBootManagerEscape|FALSE|BOOLEAN|0x00000020
+
 # BootSplash Image
 gUefiPayloadPkgTokenSpaceGuid.PcdBootSplashImage|TRUE|BOOLEAN|0x00000021
 
diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 82adf98a45..eaad7eee84 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -33,8 +33,10 @@
   DEFINE UNIVERSAL_PAYLOAD            = FALSE
   DEFINE SECURITY_STUB_ENABLE         = TRUE
   DEFINE SMM_SUPPORT                  = FALSE
+  DEFINE BOOT_MANAGER_ESCAPE          = FALSE
   DEFINE BOOTSPLASH_IMAGE             = FALSE
   DEFINE FOLLOW_BGRT_SPEC             = FALSE
+  DEFINE BOOT_MANAGER_ESCAPE          = FALSE
   #
   # SBL:      UEFI payload for Slim Bootloader
   # COREBOOT: UEFI payload for coreboot
@@ -405,6 +407,8 @@
 
   gUefiPayloadPkgTokenSpaceGuid.PcdBootSplashImage|$(BOOTSPLASH_IMAGE)
 
+  gUefiPayloadPkgTokenSpaceGuid.PcdBootManagerEscape|$(BOOT_MANAGER_ESCAPE)
+
 [PcdsPatchableInModule.X64]
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister|$(RTC_INDEX_REGISTER)
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister|$(RTC_TARGET_REGISTER)
-- 
2.32.0

[PATCH 06/18] UefiPayloadPkg: Add build option for Above 4G Memory

[Sean Rhodes]

When build option ABOVE_4G_MEMORY is set to true, nothing will change
and EDKII will use all available memory.

Setting it to false will create memory type information HOB in
payload entry, so that EDKII will reserve enough memory below 4G
for EDKII modules. This option is useful for bootloaders that are not
fully 64-bit aware such as Qubes R4.0.4 bootloader, Zorin and Proxmox.

Signed-off-by: Sean Rhodes <sean@starlabs.systems>
---
 .../UefiPayloadEntry/UefiPayloadEntry.c       | 39 +++++++++++++++++++
 .../UefiPayloadEntry/UefiPayloadEntry.inf     |  7 ++++
 UefiPayloadPkg/UefiPayloadPkg.dec             |  3 ++
 UefiPayloadPkg/UefiPayloadPkg.dsc             |  3 ++
 4 files changed, 52 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index 0fed1e3691..998895e201 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -5,10 +5,44 @@
 
 **/
 
+#include <Guid/MemoryTypeInformation.h>
 #include "UefiPayloadEntry.h"
 
 STATIC UINT32  mTopOfLowerUsableDram = 0;
 
+EFI_MEMORY_TYPE_INFORMATION  mDefaultMemoryTypeInformation[] = {
+  { EfiACPIReclaimMemory,   FixedPcdGet32 (PcdMemoryTypeEfiACPIReclaimMemory)   },
+  { EfiACPIMemoryNVS,       FixedPcdGet32 (PcdMemoryTypeEfiACPIMemoryNVS)       },
+  { EfiReservedMemoryType,  FixedPcdGet32 (PcdMemoryTypeEfiReservedMemoryType)  },
+  { EfiRuntimeServicesData, FixedPcdGet32 (PcdMemoryTypeEfiRuntimeServicesData) },
+  { EfiRuntimeServicesCode, FixedPcdGet32 (PcdMemoryTypeEfiRuntimeServicesCode) },
+  { EfiMaxMemoryType,       0                                                   }
+};
+
+/**
+   Function to reserve memory below 4GB for EDKII Modules.
+
+   This causes the DXE to dispatch everything under 4GB and allows Operating
+   System's that require EFI_LOADED_IMAGE to be under 4GB to start.
+   e.g. Xen hypervisor used in Qubes.
+**/
+VOID
+ForceModulesBelow4G (
+  VOID
+  )
+{
+  DEBUG ((DEBUG_INFO, "Building hob to restrict memory resorces to below 4G.\n"));
+
+  //
+  // Create Memory Type Information HOB
+  //
+  BuildGuidDataHob (
+    &gEfiMemoryTypeInformationGuid,
+    mDefaultMemoryTypeInformation,
+    sizeof (mDefaultMemoryTypeInformation)
+    );
+}
+
 /**
    Callback function to build resource descriptor HOB
 
@@ -438,6 +472,11 @@ _ModuleEntryPoint (
   // Build other HOBs required by DXE
   BuildGenericHob ();
 
+  // Create a HOB to make resources for EDKII modules below 4G
+  if (!FixedPcdGetBool (PcdAbove4GMemory)) {
+    ForceModulesBelow4G ();
+  }
+
   // Load the DXE Core
   Status = LoadDxeCore (&DxeCoreEntryPoint);
   ASSERT_EFI_ERROR (Status);
diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
index 1847d6481a..2ca47e3bb5 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
@@ -86,8 +86,15 @@
   gUefiPayloadPkgTokenSpaceGuid.PcdPayloadFdMemSize
   gUefiPayloadPkgTokenSpaceGuid.PcdBootloaderParameter
   gUefiPayloadPkgTokenSpaceGuid.PcdSystemMemoryUefiRegionSize
+  gUefiPayloadPkgTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS
+  gUefiPayloadPkgTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
+  gUefiPayloadPkgTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType
+  gUefiPayloadPkgTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData
+  gUefiPayloadPkgTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack               ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy       ## SOMETIMES_CONSUMES
 
+  gUefiPayloadPkgTokenSpaceGuid.PcdAbove4GMemory
+
diff --git a/UefiPayloadPkg/UefiPayloadPkg.dec b/UefiPayloadPkg/UefiPayloadPkg.dec
index b01b553cdd..652029c194 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dec
+++ b/UefiPayloadPkg/UefiPayloadPkg.dec
@@ -83,6 +83,9 @@ gUefiPayloadPkgTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x04000000|UINT32|0x
 
 gUefiPayloadPkgTokenSpaceGuid.PcdPcdDriverFile|{ 0x57, 0x72, 0xcf, 0x80, 0xab, 0x87, 0xf9, 0x47, 0xa3, 0xfe, 0xD5, 0x0B, 0x76, 0xd8, 0x95, 0x41 }|VOID*|0x00000018
 
+# Above 4G Memory
+gUefiPayloadPkgTokenSpaceGuid.PcdAbove4GMemory|TRUE|BOOLEAN|0x00000019
+
 # Boot Manager Key
 gUefiPayloadPkgTokenSpaceGuid.PcdBootManagerEscape|FALSE|BOOLEAN|0x00000020
 
diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index eaad7eee84..14b7cc55f5 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -33,6 +33,7 @@
   DEFINE UNIVERSAL_PAYLOAD            = FALSE
   DEFINE SECURITY_STUB_ENABLE         = TRUE
   DEFINE SMM_SUPPORT                  = FALSE
+  DEFINE ABOVE_4G_MEMORY              = TRUE
   DEFINE BOOT_MANAGER_ESCAPE          = FALSE
   DEFINE BOOTSPLASH_IMAGE             = FALSE
   DEFINE FOLLOW_BGRT_SPEC             = FALSE
@@ -409,6 +410,8 @@
 
   gUefiPayloadPkgTokenSpaceGuid.PcdBootManagerEscape|$(BOOT_MANAGER_ESCAPE)
 
+  gUefiPayloadPkgTokenSpaceGuid.PcdAbove4GMemory|$(ABOVE_4G_MEMORY)
+
 [PcdsPatchableInModule.X64]
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister|$(RTC_INDEX_REGISTER)
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister|$(RTC_TARGET_REGISTER)
-- 
2.32.0

[PATCH 08/18] UefiPayloadPkg: Fix case of protocol

[Sean Rhodes]

Fix case match in <Protocol/MpService.h> to avoid build failure on
Linu.

Signed-off-by: Sean Rhodes <sean@starlabs.systems>
---
 UefiPayloadPkg/BlSupportSmm/BlSupportSmm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiPayloadPkg/BlSupportSmm/BlSupportSmm.h b/UefiPayloadPkg/BlSupportSmm/BlSupportSmm.h
index 15e604d514..b82f632556 100644
--- a/UefiPayloadPkg/BlSupportSmm/BlSupportSmm.h
+++ b/UefiPayloadPkg/BlSupportSmm/BlSupportSmm.h
@@ -22,7 +22,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/PciLib.h>
 #include <Protocol/SmmSwDispatch2.h>
 #include <Protocol/SmmAccess2.h>
-#include <protocol/MpService.h>
+#include <Protocol/MpService.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Register/Intel/ArchitecturalMsr.h>
 #include <Guid/SmmRegisterInfoGuid.h>
-- 
2.32.0

Re: [edk2-devel] [PATCH 08/18] UefiPayloadPkg: Fix case of protocol

[Patrick Rudolph]

Hi,
can you please have a look at this patch?
It fixes a build failure.

Regards,
Patrick Rudolph

On Thu, Feb 10, 2022 at 10:24 PM Sean Rhodes <sean@starlabs.systems> wrote:
>
> Fix case match in <Protocol/MpService.h> to avoid build failure on
> Linu.
>
> Signed-off-by: Sean Rhodes <sean@starlabs.systems>
> ---
>  UefiPayloadPkg/BlSupportSmm/BlSupportSmm.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/UefiPayloadPkg/BlSupportSmm/BlSupportSmm.h b/UefiPayloadPkg/BlSupportSmm/BlSupportSmm.h
> index 15e604d514..b82f632556 100644
> --- a/UefiPayloadPkg/BlSupportSmm/BlSupportSmm.h
> +++ b/UefiPayloadPkg/BlSupportSmm/BlSupportSmm.h
> @@ -22,7 +22,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Library/PciLib.h>
>  #include <Protocol/SmmSwDispatch2.h>
>  #include <Protocol/SmmAccess2.h>
> -#include <protocol/MpService.h>
> +#include <Protocol/MpService.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Register/Intel/ArchitecturalMsr.h>
>  #include <Guid/SmmRegisterInfoGuid.h>
> --
> 2.32.0
>
>
>
> ------------
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#86579): https://edk2.groups.io/g/devel/message/86579
> Mute This Topic: https://groups.io/mt/89056082/2917327
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [patrick.rudolph@9elements.com]
> ------------
>
>

[PATCH 09/18] SdMmcPciDxe: Reduce timeout for SD card reset

[Sean Rhodes]

From: Matt DeVillier <matt.devillier@gmail.com>

Previous 1s timeout causeed stalls on boot splash with no benefit.
Reduced to 100ms and no interruptions to boot when no SD card inserted.

Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index 85e09cf114..81fae8e262 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -49,7 +49,7 @@ extern EDKII_SD_MMC_OVERRIDE  *mOverride;
 //
 // Generic time out value, 1 microsecond as unit.
 //
-#define SD_MMC_HC_GENERIC_TIMEOUT  1 * 1000 * 1000
+#define SD_MMC_HC_GENERIC_TIMEOUT  1 * 1000 * 100
 
 //
 // SD/MMC async transfer timer interval, set by experience.
-- 
2.32.0

Re: [PATCH 09/18] SdMmcPciDxe: Reduce timeout for SD card reset

[Wu, Hao A]

Hello,

I think we have encountered some devices that will take as long as 1 second to respond.

How about adding a new PCD called gEfiMdeModulePkgTokenSpaceGuid.PcdSdMmcGenericTimeoutValue (in microsecond unit) in MdeModulePkg.dec '[PcdsFixedAtBuild, PcdsPatchableInModule]' section?
Please leave the default PCD value as 1000000 (1 second) and you can customize the value in your own platform DSC file.

Also added MdeModulePkg maintainers to check if they have concern for adding a new PCD.

Best Regards,
Hao Wu

> -----Original Message-----
> From: Sean Rhodes <sean@starlabs.systems>
> Sent: Friday, February 11, 2022 5:25 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Matt DeVillier
> <matt.devillier@gmail.com>
> Subject: [PATCH 09/18] SdMmcPciDxe: Reduce timeout for SD card reset
> 
> From: Matt DeVillier <matt.devillier@gmail.com>
> 
> Previous 1s timeout causeed stalls on boot splash with no benefit.
> Reduced to 100ms and no interruptions to boot when no SD card inserted.
> 
> Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 85e09cf114..81fae8e262 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -49,7 +49,7 @@ extern EDKII_SD_MMC_OVERRIDE  *mOverride;
>  //
> 
>  // Generic time out value, 1 microsecond as unit.
> 
>  //
> 
> -#define SD_MMC_HC_GENERIC_TIMEOUT  1 * 1000 * 1000
> 
> +#define SD_MMC_HC_GENERIC_TIMEOUT  1 * 1000 * 100
> 
> 
> 
>  //
> 
>  // SD/MMC async transfer timer interval, set by experience.
> 
> --
> 2.32.0

[PATCH 10/18] BlSupportSmm: fix definition of SetSmrr()

[Sean Rhodes]

From: Matt DeVillier <matt.devillier@gmail.com>

Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Change-Id: Id5b077ad6c79717a9d97d4228145791ef062962c
---
 UefiPayloadPkg/BlSupportSmm/BlSupportSmm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/UefiPayloadPkg/BlSupportSmm/BlSupportSmm.c b/UefiPayloadPkg/BlSupportSmm/BlSupportSmm.c
index dcc4d60bb2..0d16aec8ef 100644
--- a/UefiPayloadPkg/BlSupportSmm/BlSupportSmm.c
+++ b/UefiPayloadPkg/BlSupportSmm/BlSupportSmm.c
@@ -191,6 +191,7 @@ SmmFeatureLockOnS3 (
   @param[in] ProcedureArgument  Pointer to SMRR_BASE_MASK structure.
 **/
 VOID
+EFIAPI
 SetSmrr (
   IN VOID  *ProcedureArgument
   )
-- 
2.32.0

Re: [edk2-devel] [PATCH 10/18] BlSupportSmm: fix definition of SetSmrr()

[Patrick Rudolph]

Hi,
Can you please have a look at this patch?
The BlSupportSmm currently doesn't compile and this patch fixes the issue.
It would be very nice if you could add build tests for every option
defaulting to false to catch those issues in the future.

Regards,
Patrick Rudolph

On Thu, Feb 10, 2022 at 10:25 PM Sean Rhodes <sean@starlabs.systems> wrote:
>
> From: Matt DeVillier <matt.devillier@gmail.com>
>
> Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> Change-Id: Id5b077ad6c79717a9d97d4228145791ef062962c
> ---
>  UefiPayloadPkg/BlSupportSmm/BlSupportSmm.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/UefiPayloadPkg/BlSupportSmm/BlSupportSmm.c b/UefiPayloadPkg/BlSupportSmm/BlSupportSmm.c
> index dcc4d60bb2..0d16aec8ef 100644
> --- a/UefiPayloadPkg/BlSupportSmm/BlSupportSmm.c
> +++ b/UefiPayloadPkg/BlSupportSmm/BlSupportSmm.c
> @@ -191,6 +191,7 @@ SmmFeatureLockOnS3 (
>    @param[in] ProcedureArgument  Pointer to SMRR_BASE_MASK structure.
>  **/
>  VOID
> +EFIAPI
>  SetSmrr (
>    IN VOID  *ProcedureArgument
>    )
> --
> 2.32.0
>
>
>
> ------------
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#86581): https://edk2.groups.io/g/devel/message/86581
> Mute This Topic: https://groups.io/mt/89056100/2917327
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [patrick.rudolph@9elements.com]
> ------------
>
>

[PATCH 11/18] UefiPayloadPkg: Make Boot Timeout configurable

[Sean Rhodes]

Signed-off-by: Sean Rhodes <sean@starlabs.systems>
---
 UefiPayloadPkg/UefiPayloadPkg.dsc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 14b7cc55f5..7b57310dfd 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -37,7 +37,7 @@
   DEFINE BOOT_MANAGER_ESCAPE          = FALSE
   DEFINE BOOTSPLASH_IMAGE             = FALSE
   DEFINE FOLLOW_BGRT_SPEC             = FALSE
-  DEFINE BOOT_MANAGER_ESCAPE          = FALSE
+  DEFINE PLATFORM_BOOT_TIMEOUT        = 3
   #
   # SBL:      UEFI payload for Slim Bootloader
   # COREBOOT: UEFI payload for coreboot
@@ -483,7 +483,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
-  gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
+  gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|$(PLATFORM_BOOT_TIMEOUT)
 !if $(VARIABLE_SUPPORT) == "SPI"
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize  |0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0
-- 
2.32.0

[PATCH 12/18] Ps2KbdCtrller: Make wait for SUCCESS after BAT non-fatal

[Sean Rhodes]

From: Matt DeVillier <matt.devillier@gmail.com>

Recent model Chromebooks only return ACK, but not
BAT_SUCCESS, which causes hanging and failed ps2k init.
To mitigate this, make the absence of BAT_SUCCESS reply
non-fatal, and reduce the no-reply timeout from 4s to 1s.

Tested on google/dracia and purism/librem_14

Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
---
 MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
index 77dc226222..981309f8b0 100644
--- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
+++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
@@ -1731,13 +1731,7 @@ InitKeyboard (
     //
     // wait for BAT completion code
     //
-    mWaitForValueTimeOut = KEYBOARD_BAT_TIMEOUT;
-
-    Status = KeyboardWaitForValue (ConsoleIn, KEYBOARD_8048_RETURN_8042_BAT_SUCCESS);
-    if (EFI_ERROR (Status)) {
-      KeyboardError (ConsoleIn, L"Keyboard self test failed!\n\r");
-      goto Done;
-    }
+    KeyboardWaitForValue (ConsoleIn, KEYBOARD_8048_RETURN_8042_BAT_SUCCESS);
 
     mWaitForValueTimeOut = KEYBOARD_WAITFORVALUE_TIMEOUT;
 
-- 
2.32.0

Re: [PATCH 12/18] Ps2KbdCtrller: Make wait for SUCCESS after BAT non-fatal

[Wu, Hao A]

Hello,

(Add Ray as the initial author for this driver.)
Inline comments below:


> -----Original Message-----
> From: Sean Rhodes <sean@starlabs.systems>
> Sent: Friday, February 11, 2022 5:26 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Matt DeVillier
> <matt.devillier@gmail.com>
> Subject: [PATCH 12/18] Ps2KbdCtrller: Make wait for SUCCESS after BAT non-
> fatal
> 
> From: Matt DeVillier <matt.devillier@gmail.com>
> 
> Recent model Chromebooks only return ACK, but not
> BAT_SUCCESS, which causes hanging and failed ps2k init.
> To mitigate this, make the absence of BAT_SUCCESS reply
> non-fatal, and reduce the no-reply timeout from 4s to 1s.
> 
> Tested on google/dracia and purism/librem_14
> 
> Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> ---
>  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
> b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
> index 77dc226222..981309f8b0 100644
> --- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
> +++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
> @@ -1731,13 +1731,7 @@ InitKeyboard (
>      //
> 
>      // wait for BAT completion code
> 
>      //
> 
> -    mWaitForValueTimeOut = KEYBOARD_BAT_TIMEOUT;
> 
> -
> 
> -    Status = KeyboardWaitForValue (ConsoleIn,
> KEYBOARD_8048_RETURN_8042_BAT_SUCCESS);
> 
> -    if (EFI_ERROR (Status)) {
> 
> -      KeyboardError (ConsoleIn, L"Keyboard self test failed!\n\r");
> 
> -      goto Done;
> 
> -    }
> 
> +    KeyboardWaitForValue (ConsoleIn,
> KEYBOARD_8048_RETURN_8042_BAT_SUCCESS);


Sorry for lacking of expertise in PS2 domain, I will try my best to provide comments based on my knowledge.

I am fine with removing the hard requirement on successful BAT (assuming problematic device will eventually fail on subsequent initialization steps).
I am also fine with reducing the timeout for BAT to 1 second, since I found the below information at link:
http://www-ug.eecg.toronto.edu/msl/nios_devices/datasheets/PS2%20Keyboard%20Protocol.htm
(But I am not sure if the above link is an official spec for PS2 keyboard)
"This BAT completion code must be sent 500~750 milliseconds after power-on."

But please help to update the patch by changing the value of KEYBOARD_BAT_TIMEOUT macro to 1 second instead of removing line:
mWaitForValueTimeOut = KEYBOARD_BAT_TIMEOUT;

@Ni, Ray, could you help to take a look at this patch as well? Thanks in advance.

Best Regards,
Hao Wu


> 
> 
> 
>      mWaitForValueTimeOut = KEYBOARD_WAITFORVALUE_TIMEOUT;
> 
> 
> 
> --
> 2.32.0

[PATCH 13/18] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting

[Sean Rhodes]

From: Matt DeVillier <matt.devillier@gmail.com>

No need to check the interface protocol then conditionally setting,
just set it to BOOT_PROTOCOL and check for error.

This is what Linux does for HID devices as some don't follow the USB spec.
One example is the Aspeed BMC HID keyboard device, which adds a massive
boot delay without this patch as it doesn't respond to 'GetProtocolRequest'.

Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
 MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
index 5a94a4dda7..56d249f937 100644
--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
@@ -863,12 +863,24 @@ InitUSBKeyboard (
   // Set boot protocol for the USB Keyboard.
   // This driver only supports boot protocol.
   //
-  if (Protocol != BOOT_PROTOCOL) {
-    UsbSetProtocolRequest (
-      UsbKeyboardDevice->UsbIo,
-      UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
-      BOOT_PROTOCOL
+  Status = UsbSetProtocolRequest (
+             UsbKeyboardDevice->UsbIo,
+             UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
+             BOOT_PROTOCOL
+             );
+  if (EFI_ERROR (Status)) {
+    //
+    // If protocol could not be set here, it means
+    // the keyboard interface has some errors and could
+    // not be initialized
+    //
+    REPORT_STATUS_CODE_WITH_DEVICE_PATH (
+      EFI_ERROR_CODE | EFI_ERROR_MINOR,
+      (EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_INTERFACE_ERROR),
+      UsbKeyboardDevice->DevicePath
       );
+
+    return EFI_DEVICE_ERROR;
   }
 
   UsbKeyboardDevice->CtrlOn    = FALSE;
-- 
2.32.0

Re: [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting

[Wu, Hao A]

Hello,

Inline comments below:


> -----Original Message-----
> From: Sean Rhodes <sean@starlabs.systems>
> Sent: Friday, February 11, 2022 5:27 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Matt DeVillier
> <matt.devillier@gmail.com>; Patrick Rudolph
> <patrick.rudolph@9elements.com>
> Subject: [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c: don't request
> protocol before setting
> 
> From: Matt DeVillier <matt.devillier@gmail.com>
> 
> No need to check the interface protocol then conditionally setting,
> just set it to BOOT_PROTOCOL and check for error.
> 
> This is what Linux does for HID devices as some don't follow the USB spec.
> One example is the Aspeed BMC HID keyboard device, which adds a massive
> boot delay without this patch as it doesn't respond to 'GetProtocolRequest'.
> 
> Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> ---
>  MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 22
> +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> index 5a94a4dda7..56d249f937 100644
> --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> @@ -863,12 +863,24 @@ InitUSBKeyboard (
>    // Set boot protocol for the USB Keyboard.
> 
>    // This driver only supports boot protocol.
> 
>    //
> 
> -  if (Protocol != BOOT_PROTOCOL) {
> 
> -    UsbSetProtocolRequest (
> 
> -      UsbKeyboardDevice->UsbIo,
> 
> -      UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> 
> -      BOOT_PROTOCOL
> 
> +  Status = UsbSetProtocolRequest (
> 
> +             UsbKeyboardDevice->UsbIo,
> 
> +             UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> 
> +             BOOT_PROTOCOL
> 
> +             );
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    //
> 
> +    // If protocol could not be set here, it means
> 
> +    // the keyboard interface has some errors and could
> 
> +    // not be initialized
> 
> +    //
> 
> +    REPORT_STATUS_CODE_WITH_DEVICE_PATH (
> 
> +      EFI_ERROR_CODE | EFI_ERROR_MINOR,
> 
> +      (EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_INTERFACE_ERROR),
> 
> +      UsbKeyboardDevice->DevicePath
> 
>        );
> 
> +
> 
> +    return EFI_DEVICE_ERROR;
> 
>    }


Sorry for a question, I do not understand why the proposed change will eliminate the boot delay.
For device that does not respond to GetProtocolRequest(), the origin flow is like:
a.1 Timeout occurs for GetProtocolRequest()
a.2 Conditionally execute UsbSetProtocolRequest()

The proposed new flow is like:
b.1 Timeout occurs for GetProtocolRequest()
b.2 Always execute UsbSetProtocolRequest()

Could you help to elaborate on the change? Thanks in advance.

Best Regards,
Hao Wu


> 
> 
> 
>    UsbKeyboardDevice->CtrlOn    = FALSE;
> 
> --
> 2.32.0

Re: [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting

[MrChromebox]

seems this patch got truncated during a rebase, here is the correct version.

For device that does not respond to GetProtocolRequest(), the original flow is:
a.1 Execute GetProtocolRequest()
a.2 Timeout occurs for GetProtocolRequest()
a.3 Conditionally execute UsbSetProtocolRequest()

The proposed new flow is:
b.1 Always execute UsbSetProtocolRequest()

The timeout delay for keyboards which do not support
GetProtocolRequest() that we've seen is significant (>5s, IIRC)

 MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 28 ++++++++++++++----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
index 5a94a4dda7..ad55fa3330 100644
--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
@@ -805,7 +805,6 @@ InitUSBKeyboard (
   )
 {
   UINT16      ConfigValue;
-  UINT8       Protocol;
   EFI_STATUS  Status;
   UINT32      TransferResult;

@@ -854,21 +853,28 @@ InitUSBKeyboard (
     }
   }

-  UsbGetProtocolRequest (
-    UsbKeyboardDevice->UsbIo,
-    UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
-    &Protocol
-    );
   //
   // Set boot protocol for the USB Keyboard.
   // This driver only supports boot protocol.
   //
-  if (Protocol != BOOT_PROTOCOL) {
-    UsbSetProtocolRequest (
-      UsbKeyboardDevice->UsbIo,
-      UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
-      BOOT_PROTOCOL
+  Status = UsbSetProtocolRequest (
+             UsbKeyboardDevice->UsbIo,
+             UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
+             BOOT_PROTOCOL
+             );
+  if (EFI_ERROR (Status)) {
+    //
+    // If protocol could not be set here, it means
+    // the keyboard interface has some errors and could
+    // not be initialized
+    //
+    REPORT_STATUS_CODE_WITH_DEVICE_PATH (
+      EFI_ERROR_CODE | EFI_ERROR_MINOR,
+      (EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_INTERFACE_ERROR),
+      UsbKeyboardDevice->DevicePath
       );
+
+    return EFI_DEVICE_ERROR;
   }

   UsbKeyboardDevice->CtrlOn    = FALSE;
-- 
2.32.0

On Thu, Feb 10, 2022 at 9:32 PM Wu, Hao A <hao.a.wu@intel.com> wrote:
>
> Hello,
>
> Inline comments below:
>
>
> > -----Original Message-----
> > From: Sean Rhodes <sean@starlabs.systems>
> > Sent: Friday, February 11, 2022 5:27 AM
> > To: devel@edk2.groups.io
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Matt DeVillier
> > <matt.devillier@gmail.com>; Patrick Rudolph
> > <patrick.rudolph@9elements.com>
> > Subject: [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c: don't request
> > protocol before setting
> >
> > From: Matt DeVillier <matt.devillier@gmail.com>
> >
> > No need to check the interface protocol then conditionally setting,
> > just set it to BOOT_PROTOCOL and check for error.
> >
> > This is what Linux does for HID devices as some don't follow the USB spec.
> > One example is the Aspeed BMC HID keyboard device, which adds a massive
> > boot delay without this patch as it doesn't respond to 'GetProtocolRequest'.
> >
> > Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> > ---
> >  MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 22
> > +++++++++++++++++-----
> >  1 file changed, 17 insertions(+), 5 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > index 5a94a4dda7..56d249f937 100644
> > --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > @@ -863,12 +863,24 @@ InitUSBKeyboard (
> >    // Set boot protocol for the USB Keyboard.
> >
> >    // This driver only supports boot protocol.
> >
> >    //
> >
> > -  if (Protocol != BOOT_PROTOCOL) {
> >
> > -    UsbSetProtocolRequest (
> >
> > -      UsbKeyboardDevice->UsbIo,
> >
> > -      UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> >
> > -      BOOT_PROTOCOL
> >
> > +  Status = UsbSetProtocolRequest (
> >
> > +             UsbKeyboardDevice->UsbIo,
> >
> > +             UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> >
> > +             BOOT_PROTOCOL
> >
> > +             );
> >
> > +  if (EFI_ERROR (Status)) {
> >
> > +    //
> >
> > +    // If protocol could not be set here, it means
> >
> > +    // the keyboard interface has some errors and could
> >
> > +    // not be initialized
> >
> > +    //
> >
> > +    REPORT_STATUS_CODE_WITH_DEVICE_PATH (
> >
> > +      EFI_ERROR_CODE | EFI_ERROR_MINOR,
> >
> > +      (EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_INTERFACE_ERROR),
> >
> > +      UsbKeyboardDevice->DevicePath
> >
> >        );
> >
> > +
> >
> > +    return EFI_DEVICE_ERROR;
> >
> >    }
>
>
> Sorry for a question, I do not understand why the proposed change will eliminate the boot delay.
> For device that does not respond to GetProtocolRequest(), the origin flow is like:
> a.1 Timeout occurs for GetProtocolRequest()
> a.2 Conditionally execute UsbSetProtocolRequest()
>
> The proposed new flow is like:
> b.1 Timeout occurs for GetProtocolRequest()
> b.2 Always execute UsbSetProtocolRequest()
>
> Could you help to elaborate on the change? Thanks in advance.
>
> Best Regards,
> Hao Wu
>
>
> >
> >
> >
> >    UsbKeyboardDevice->CtrlOn    = FALSE;
> >
> > --
> > 2.32.0
>

Re: [edk2-devel] [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting

[Wu, Hao A]

Inline comments below:


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> MrChromebox
> Sent: Saturday, February 12, 2022 6:46 AM
> To: Wu, Hao A <hao.a.wu@intel.com>
> Cc: Rhodes, Sean <sean@starlabs.systems>; Patrick Rudolph
> <patrick.rudolph@9elements.com>; devel@edk2.groups.io; Wang, Jian J
> <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray
> <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c:
> don't request protocol before setting
> 
> seems this patch got truncated during a rebase, here is the correct version.
> 
> For device that does not respond to GetProtocolRequest(), the original flow
> is:
> a.1 Execute GetProtocolRequest()
> a.2 Timeout occurs for GetProtocolRequest()
> a.3 Conditionally execute UsbSetProtocolRequest()
> 
> The proposed new flow is:
> b.1 Always execute UsbSetProtocolRequest()
> 
> The timeout delay for keyboards which do not support
> GetProtocolRequest() that we've seen is significant (>5s, IIRC)
> 
>  MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 28 ++++++++++++++----
> ------
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> index 5a94a4dda7..ad55fa3330 100644
> --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> @@ -805,7 +805,6 @@ InitUSBKeyboard (
>    )
>  {
>    UINT16      ConfigValue;
> -  UINT8       Protocol;
>    EFI_STATUS  Status;
>    UINT32      TransferResult;
> 
> @@ -854,21 +853,28 @@ InitUSBKeyboard (
>      }
>    }
> 
> -  UsbGetProtocolRequest (
> -    UsbKeyboardDevice->UsbIo,
> -    UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> -    &Protocol
> -    );
>    //
>    // Set boot protocol for the USB Keyboard.
>    // This driver only supports boot protocol.
>    //
> -  if (Protocol != BOOT_PROTOCOL) {
> -    UsbSetProtocolRequest (
> -      UsbKeyboardDevice->UsbIo,
> -      UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> -      BOOT_PROTOCOL
> +  Status = UsbSetProtocolRequest (
> +             UsbKeyboardDevice->UsbIo,
> +             UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> +             BOOT_PROTOCOL
> +             );
> +  if (EFI_ERROR (Status)) {
> +    //
> +    // If protocol could not be set here, it means
> +    // the keyboard interface has some errors and could
> +    // not be initialized
> +    //
> +    REPORT_STATUS_CODE_WITH_DEVICE_PATH (
> +      EFI_ERROR_CODE | EFI_ERROR_MINOR,
> +      (EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_INTERFACE_ERROR),
> +      UsbKeyboardDevice->DevicePath
>        );
> +
> +    return EFI_DEVICE_ERROR;
>    }


I am okay with unconditionally sending the Set_Protocol command.
Could you help to:
1. Just keep the previous behavior for calling UsbSetProtocolRequest() with no error handling logic?

2. Check if similar changes need to be applied in:
  * MdeModulePkg\Bus\Usb\UsbMouseAbsolutePointerDxe
  * MdeModulePkg\Bus\Usb\UsbMouseDxe
If you do not have device for verifying the change, I suggest to make the change only in UsbKbDxe.

3. Send out a new V2 patch rather than replying the previous patch mail.

Thanks in advance.

Best Regards,
Hao Wu


> 
>    UsbKeyboardDevice->CtrlOn    = FALSE;
> --
> 2.32.0
> 
> On Thu, Feb 10, 2022 at 9:32 PM Wu, Hao A <hao.a.wu@intel.com> wrote:
> >
> > Hello,
> >
> > Inline comments below:
> >
> >
> > > -----Original Message-----
> > > From: Sean Rhodes <sean@starlabs.systems>
> > > Sent: Friday, February 11, 2022 5:27 AM
> > > To: devel@edk2.groups.io
> > > Cc: Wu, Hao A <hao.a.wu@intel.com>; Matt DeVillier
> > > <matt.devillier@gmail.com>; Patrick Rudolph
> > > <patrick.rudolph@9elements.com>
> > > Subject: [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c: don't request
> > > protocol before setting
> > >
> > > From: Matt DeVillier <matt.devillier@gmail.com>
> > >
> > > No need to check the interface protocol then conditionally setting,
> > > just set it to BOOT_PROTOCOL and check for error.
> > >
> > > This is what Linux does for HID devices as some don't follow the USB spec.
> > > One example is the Aspeed BMC HID keyboard device, which adds a
> > > massive boot delay without this patch as it doesn't respond to
> 'GetProtocolRequest'.
> > >
> > > Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> > > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> > > ---
> > >  MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 22
> > > +++++++++++++++++-----
> > >  1 file changed, 17 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > > b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > > index 5a94a4dda7..56d249f937 100644
> > > --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > > +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > > @@ -863,12 +863,24 @@ InitUSBKeyboard (
> > >    // Set boot protocol for the USB Keyboard.
> > >
> > >    // This driver only supports boot protocol.
> > >
> > >    //
> > >
> > > -  if (Protocol != BOOT_PROTOCOL) {
> > >
> > > -    UsbSetProtocolRequest (
> > >
> > > -      UsbKeyboardDevice->UsbIo,
> > >
> > > -      UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> > >
> > > -      BOOT_PROTOCOL
> > >
> > > +  Status = UsbSetProtocolRequest (
> > >
> > > +             UsbKeyboardDevice->UsbIo,
> > >
> > > +
> > > + UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> > >
> > > +             BOOT_PROTOCOL
> > >
> > > +             );
> > >
> > > +  if (EFI_ERROR (Status)) {
> > >
> > > +    //
> > >
> > > +    // If protocol could not be set here, it means
> > >
> > > +    // the keyboard interface has some errors and could
> > >
> > > +    // not be initialized
> > >
> > > +    //
> > >
> > > +    REPORT_STATUS_CODE_WITH_DEVICE_PATH (
> > >
> > > +      EFI_ERROR_CODE | EFI_ERROR_MINOR,
> > >
> > > +      (EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_INTERFACE_ERROR),
> > >
> > > +      UsbKeyboardDevice->DevicePath
> > >
> > >        );
> > >
> > > +
> > >
> > > +    return EFI_DEVICE_ERROR;
> > >
> > >    }
> >
> >
> > Sorry for a question, I do not understand why the proposed change will
> eliminate the boot delay.
> > For device that does not respond to GetProtocolRequest(), the origin flow
> is like:
> > a.1 Timeout occurs for GetProtocolRequest()
> > a.2 Conditionally execute UsbSetProtocolRequest()
> >
> > The proposed new flow is like:
> > b.1 Timeout occurs for GetProtocolRequest()
> > b.2 Always execute UsbSetProtocolRequest()
> >
> > Could you help to elaborate on the change? Thanks in advance.
> >
> > Best Regards,
> > Hao Wu
> >
> >
> > >
> > >
> > >
> > >    UsbKeyboardDevice->CtrlOn    = FALSE;
> > >
> > > --
> > > 2.32.0
> >
> 
> 
> 
> 

[PATCH 14/18] UefiPayloadPkg: Add RNG support

[Sean Rhodes]

From: Patrick Rudolph <patrick.rudolph@9elements.com>

Uses the RDRAND instruction if available and install EfiRngProtocol.
The protocol may be used by iPXE or the Linux kernel to gather entropy.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
 UefiPayloadPkg/Library/BaseRngLib/BaseRng.c   | 199 ++++++++++++++++++
 .../Library/BaseRngLib/BaseRngLib.inf         |  32 +++
 .../Library/BaseRngLib/BaseRngLib.uni         |  17 ++
 UefiPayloadPkg/UefiPayloadPkg.dsc             |   8 +
 UefiPayloadPkg/UefiPayloadPkg.fdf             |   4 +
 5 files changed, 260 insertions(+)
 create mode 100644 UefiPayloadPkg/Library/BaseRngLib/BaseRng.c
 create mode 100644 UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.inf
 create mode 100644 UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.uni

diff --git a/UefiPayloadPkg/Library/BaseRngLib/BaseRng.c b/UefiPayloadPkg/Library/BaseRngLib/BaseRng.c
new file mode 100644
index 0000000000..c21e713cb0
--- /dev/null
+++ b/UefiPayloadPkg/Library/BaseRngLib/BaseRng.c
@@ -0,0 +1,199 @@
+/** @file
+  Random number generator services that uses RdRand instruction access
+  to provide high-quality random numbers.
+
+Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Register/Intel/Cpuid.h>
+
+STATIC BOOLEAN  mHasRdRand;
+
+//
+// Bit mask used to determine if RdRand instruction is supported.
+//
+#define RDRAND_MASK  BIT30
+
+//
+// Limited retry number when valid random data is returned.
+// Uses the recommended value defined in Section 7.3.17 of "Intel 64 and IA-32
+// Architectures Software Developer's Mannual".
+//
+#define RDRAND_RETRY_LIMIT  10
+
+/**
+  The constructor function checks whether or not RDRAND instruction is supported
+  by the host hardware.
+
+  The constructor function checks whether or not RDRAND instruction is supported.
+  It will always return RETURN_SUCCESS.
+
+  @retval RETURN_SUCCESS   The constructor always returns EFI_SUCCESS.
+
+**/
+RETURN_STATUS
+EFIAPI
+BaseRngLibConstructor (
+  VOID
+  )
+{
+  UINT32  RegEax;
+  UINT32  RegEcx;
+
+  AsmCpuid (CPUID_SIGNATURE, &RegEax, NULL, NULL, NULL);
+  if (RegEax < 1) {
+    mHasRdRand = FALSE;
+    return RETURN_SUCCESS;
+  }
+
+  //
+  // Determine RDRAND support by examining bit 30 of the ECX register returned by
+  // CPUID. A value of 1 indicates that processor support RDRAND instruction.
+  //
+  AsmCpuid (CPUID_VERSION_INFO, 0, 0, &RegEcx, 0);
+
+  mHasRdRand = ((RegEcx & RDRAND_MASK) == RDRAND_MASK);
+
+  return RETURN_SUCCESS;
+}
+
+/**
+  Generates a 16-bit random number.
+
+  if Rand is NULL, then ASSERT().
+
+  @param[out] Rand     Buffer pointer to store the 16-bit random value.
+
+  @retval TRUE         Random number generated successfully.
+  @retval FALSE        Failed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber16 (
+  OUT     UINT16  *Rand
+  )
+{
+  UINT32  Index;
+
+  ASSERT (Rand != NULL);
+
+  if (mHasRdRand) {
+    //
+    // A loop to fetch a 16 bit random value with a retry count limit.
+    //
+    for (Index = 0; Index < RDRAND_RETRY_LIMIT; Index++) {
+      if (AsmRdRand16 (Rand)) {
+        return TRUE;
+      }
+    }
+  }
+
+  return FALSE;
+}
+
+/**
+  Generates a 32-bit random number.
+
+  if Rand is NULL, then ASSERT().
+
+  @param[out] Rand     Buffer pointer to store the 32-bit random value.
+
+  @retval TRUE         Random number generated successfully.
+  @retval FALSE        Failed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber32 (
+  OUT     UINT32  *Rand
+  )
+{
+  UINT32  Index;
+
+  ASSERT (Rand != NULL);
+
+  if (mHasRdRand) {
+    //
+    // A loop to fetch a 32 bit random value with a retry count limit.
+    //
+    for (Index = 0; Index < RDRAND_RETRY_LIMIT; Index++) {
+      if (AsmRdRand32 (Rand)) {
+        return TRUE;
+      }
+    }
+  }
+
+  return FALSE;
+}
+
+/**
+  Generates a 64-bit random number.
+
+  if Rand is NULL, then ASSERT().
+
+  @param[out] Rand     Buffer pointer to store the 64-bit random value.
+
+  @retval TRUE         Random number generated successfully.
+  @retval FALSE        Failed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber64 (
+  OUT     UINT64  *Rand
+  )
+{
+  UINT32  Index;
+
+  ASSERT (Rand != NULL);
+
+  if (mHasRdRand) {
+    //
+    // A loop to fetch a 64 bit random value with a retry count limit.
+    //
+    for (Index = 0; Index < RDRAND_RETRY_LIMIT; Index++) {
+      if (AsmRdRand64 (Rand)) {
+        return TRUE;
+      }
+    }
+  }
+
+  return FALSE;
+}
+
+/**
+  Generates a 128-bit random number.
+
+  if Rand is NULL, then ASSERT().
+
+  @param[out] Rand     Buffer pointer to store the 128-bit random value.
+
+  @retval TRUE         Random number generated successfully.
+  @retval FALSE        Failed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber128 (
+  OUT     UINT64  *Rand
+  )
+{
+  ASSERT (Rand != NULL);
+
+  //
+  // Read first 64 bits
+  //
+  if (!GetRandomNumber64 (Rand)) {
+    return FALSE;
+  }
+
+  //
+  // Read second 64 bits
+  //
+  return GetRandomNumber64 (++Rand);
+}
diff --git a/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.inf b/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.inf
new file mode 100644
index 0000000000..67a91ccfff
--- /dev/null
+++ b/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.inf
@@ -0,0 +1,32 @@
+## @file
+#  Instance of RNG (Random Number Generator) Library.
+#
+#  Copyright (c) 2020 9elements Agency GmbH.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = BaseRngLib
+  MODULE_UNI_FILE                = BaseRngLib.uni
+  FILE_GUID                      = 05C48431-DE18-4550-931A-3350E8551498
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = RngLib
+  CONSTRUCTOR                    = BaseRngLibConstructor
+
+#
+#  VALID_ARCHITECTURES           = IA32 X64
+#
+
+[Sources.Ia32, Sources.X64]
+  BaseRng.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
diff --git a/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.uni b/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.uni
new file mode 100644
index 0000000000..f3ed954c52
--- /dev/null
+++ b/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.uni
@@ -0,0 +1,17 @@
+// /** @file
+// Instance of RNG (Random Number Generator) Library.
+//
+// BaseRng Library that uses CPU RdRand instruction access to provide
+// high-quality random numbers.
+//
+// Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// **/
+
+
+#string STR_MODULE_ABSTRACT             #language en-US "Instance of RNG Library"
+
+#string STR_MODULE_DESCRIPTION          #language en-US "BaseRng Library that uses CPU RdRand instruction access to provide high-quality random numbers"
+
diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 7b57310dfd..c8562d592b 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -723,6 +723,14 @@
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
 !endif
 
+  #
+  # Random Number Generator
+  #
+  SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf {
+      <LibraryClasses>
+      RngLib|UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.inf
+  }
+
   #------------------------------
   #  Build the shell
   #------------------------------
diff --git a/UefiPayloadPkg/UefiPayloadPkg.fdf b/UefiPayloadPkg/UefiPayloadPkg.fdf
index a71d655687..3462eded64 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.fdf
+++ b/UefiPayloadPkg/UefiPayloadPkg.fdf
@@ -232,6 +232,10 @@ INF MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouseDxe.inf
 #
 INF  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
 
+# Random Number Generator
+#
+INF SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+
 #
 # UEFI network modules
 #
-- 
2.32.0

Re: [PATCH 14/18] UefiPayloadPkg: Add RNG support

[Guo Dong]

should we put BaseRngLib into SecurityPkg so that it could be used by other platforms?

Thanks,
Guo

-----Original Message-----
From: Sean Rhodes <sean@starlabs.systems> 
Sent: Thursday, February 10, 2022 2:28 PM
To: devel@edk2.groups.io
Cc: Dong, Guo <guo.dong@intel.com>; Patrick Rudolph <patrick.rudolph@9elements.com>
Subject: [PATCH 14/18] UefiPayloadPkg: Add RNG support

From: Patrick Rudolph <patrick.rudolph@9elements.com>

Uses the RDRAND instruction if available and install EfiRngProtocol.
The protocol may be used by iPXE or the Linux kernel to gather entropy.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
 UefiPayloadPkg/Library/BaseRngLib/BaseRng.c   | 199 ++++++++++++++++++
 .../Library/BaseRngLib/BaseRngLib.inf         |  32 +++
 .../Library/BaseRngLib/BaseRngLib.uni         |  17 ++
 UefiPayloadPkg/UefiPayloadPkg.dsc             |   8 +
 UefiPayloadPkg/UefiPayloadPkg.fdf             |   4 +
 5 files changed, 260 insertions(+)
 create mode 100644 UefiPayloadPkg/Library/BaseRngLib/BaseRng.c
 create mode 100644 UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.inf
 create mode 100644 UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.uni

diff --git a/UefiPayloadPkg/Library/BaseRngLib/BaseRng.c b/UefiPayloadPkg/Library/BaseRngLib/BaseRng.c
new file mode 100644
index 0000000000..c21e713cb0
--- /dev/null
+++ b/UefiPayloadPkg/Library/BaseRngLib/BaseRng.c
@@ -0,0 +1,199 @@
+/** @file

+  Random number generator services that uses RdRand instruction access

+  to provide high-quality random numbers.

+

+Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>

+SPDX-License-Identifier: BSD-2-Clause-Patent

+

+**/

+

+#include <Library/BaseLib.h>

+#include <Library/DebugLib.h>

+#include <Register/Intel/Cpuid.h>

+

+STATIC BOOLEAN  mHasRdRand;

+

+//

+// Bit mask used to determine if RdRand instruction is supported.

+//

+#define RDRAND_MASK  BIT30

+

+//

+// Limited retry number when valid random data is returned.

+// Uses the recommended value defined in Section 7.3.17 of "Intel 64 and IA-32

+// Architectures Software Developer's Mannual".

+//

+#define RDRAND_RETRY_LIMIT  10

+

+/**

+  The constructor function checks whether or not RDRAND instruction is supported

+  by the host hardware.

+

+  The constructor function checks whether or not RDRAND instruction is supported.

+  It will always return RETURN_SUCCESS.

+

+  @retval RETURN_SUCCESS   The constructor always returns EFI_SUCCESS.

+

+**/

+RETURN_STATUS

+EFIAPI

+BaseRngLibConstructor (

+  VOID

+  )

+{

+  UINT32  RegEax;

+  UINT32  RegEcx;

+

+  AsmCpuid (CPUID_SIGNATURE, &RegEax, NULL, NULL, NULL);

+  if (RegEax < 1) {

+    mHasRdRand = FALSE;

+    return RETURN_SUCCESS;

+  }

+

+  //

+  // Determine RDRAND support by examining bit 30 of the ECX register returned by

+  // CPUID. A value of 1 indicates that processor support RDRAND instruction.

+  //

+  AsmCpuid (CPUID_VERSION_INFO, 0, 0, &RegEcx, 0);

+

+  mHasRdRand = ((RegEcx & RDRAND_MASK) == RDRAND_MASK);

+

+  return RETURN_SUCCESS;

+}

+

+/**

+  Generates a 16-bit random number.

+

+  if Rand is NULL, then ASSERT().

+

+  @param[out] Rand     Buffer pointer to store the 16-bit random value.

+

+  @retval TRUE         Random number generated successfully.

+  @retval FALSE        Failed to generate the random number.

+

+**/

+BOOLEAN

+EFIAPI

+GetRandomNumber16 (

+  OUT     UINT16  *Rand

+  )

+{

+  UINT32  Index;

+

+  ASSERT (Rand != NULL);

+

+  if (mHasRdRand) {

+    //

+    // A loop to fetch a 16 bit random value with a retry count limit.

+    //

+    for (Index = 0; Index < RDRAND_RETRY_LIMIT; Index++) {

+      if (AsmRdRand16 (Rand)) {

+        return TRUE;

+      }

+    }

+  }

+

+  return FALSE;

+}

+

+/**

+  Generates a 32-bit random number.

+

+  if Rand is NULL, then ASSERT().

+

+  @param[out] Rand     Buffer pointer to store the 32-bit random value.

+

+  @retval TRUE         Random number generated successfully.

+  @retval FALSE        Failed to generate the random number.

+

+**/

+BOOLEAN

+EFIAPI

+GetRandomNumber32 (

+  OUT     UINT32  *Rand

+  )

+{

+  UINT32  Index;

+

+  ASSERT (Rand != NULL);

+

+  if (mHasRdRand) {

+    //

+    // A loop to fetch a 32 bit random value with a retry count limit.

+    //

+    for (Index = 0; Index < RDRAND_RETRY_LIMIT; Index++) {

+      if (AsmRdRand32 (Rand)) {

+        return TRUE;

+      }

+    }

+  }

+

+  return FALSE;

+}

+

+/**

+  Generates a 64-bit random number.

+

+  if Rand is NULL, then ASSERT().

+

+  @param[out] Rand     Buffer pointer to store the 64-bit random value.

+

+  @retval TRUE         Random number generated successfully.

+  @retval FALSE        Failed to generate the random number.

+

+**/

+BOOLEAN

+EFIAPI

+GetRandomNumber64 (

+  OUT     UINT64  *Rand

+  )

+{

+  UINT32  Index;

+

+  ASSERT (Rand != NULL);

+

+  if (mHasRdRand) {

+    //

+    // A loop to fetch a 64 bit random value with a retry count limit.

+    //

+    for (Index = 0; Index < RDRAND_RETRY_LIMIT; Index++) {

+      if (AsmRdRand64 (Rand)) {

+        return TRUE;

+      }

+    }

+  }

+

+  return FALSE;

+}

+

+/**

+  Generates a 128-bit random number.

+

+  if Rand is NULL, then ASSERT().

+

+  @param[out] Rand     Buffer pointer to store the 128-bit random value.

+

+  @retval TRUE         Random number generated successfully.

+  @retval FALSE        Failed to generate the random number.

+

+**/

+BOOLEAN

+EFIAPI

+GetRandomNumber128 (

+  OUT     UINT64  *Rand

+  )

+{

+  ASSERT (Rand != NULL);

+

+  //

+  // Read first 64 bits

+  //

+  if (!GetRandomNumber64 (Rand)) {

+    return FALSE;

+  }

+

+  //

+  // Read second 64 bits

+  //

+  return GetRandomNumber64 (++Rand);

+}

diff --git a/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.inf b/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.inf
new file mode 100644
index 0000000000..67a91ccfff
--- /dev/null
+++ b/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.inf
@@ -0,0 +1,32 @@
+## @file

+#  Instance of RNG (Random Number Generator) Library.

+#

+#  Copyright (c) 2020 9elements Agency GmbH.<BR>

+#

+#  SPDX-License-Identifier: BSD-2-Clause-Patent

+#

+##

+

+[Defines]

+  INF_VERSION                    = 0x00010005

+  BASE_NAME                      = BaseRngLib

+  MODULE_UNI_FILE                = BaseRngLib.uni

+  FILE_GUID                      = 05C48431-DE18-4550-931A-3350E8551498

+  MODULE_TYPE                    = BASE

+  VERSION_STRING                 = 1.0

+  LIBRARY_CLASS                  = RngLib

+  CONSTRUCTOR                    = BaseRngLibConstructor

+

+#

+#  VALID_ARCHITECTURES           = IA32 X64

+#

+

+[Sources.Ia32, Sources.X64]

+  BaseRng.c

+

+[Packages]

+  MdePkg/MdePkg.dec

+

+[LibraryClasses]

+  BaseLib

+  DebugLib

diff --git a/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.uni b/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.uni
new file mode 100644
index 0000000000..f3ed954c52
--- /dev/null
+++ b/UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.uni
@@ -0,0 +1,17 @@
+// /** @file

+// Instance of RNG (Random Number Generator) Library.

+//

+// BaseRng Library that uses CPU RdRand instruction access to provide

+// high-quality random numbers.

+//

+// Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>

+//

+// SPDX-License-Identifier: BSD-2-Clause-Patent

+//

+// **/

+

+

+#string STR_MODULE_ABSTRACT             #language en-US "Instance of RNG Library"

+

+#string STR_MODULE_DESCRIPTION          #language en-US "BaseRng Library that uses CPU RdRand instruction access to provide high-quality random numbers"

+

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 7b57310dfd..c8562d592b 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -723,6 +723,14 @@
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf

 !endif

 

+  #

+  # Random Number Generator

+  #

+  SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf {

+      <LibraryClasses>

+      RngLib|UefiPayloadPkg/Library/BaseRngLib/BaseRngLib.inf

+  }

+

   #------------------------------

   #  Build the shell

   #------------------------------

diff --git a/UefiPayloadPkg/UefiPayloadPkg.fdf b/UefiPayloadPkg/UefiPayloadPkg.fdf
index a71d655687..3462eded64 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.fdf
+++ b/UefiPayloadPkg/UefiPayloadPkg.fdf
@@ -232,6 +232,10 @@ INF MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouseDxe.inf
 #

 INF  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf

 

+# Random Number Generator

+#

+INF SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf

+

 #

 # UEFI network modules

 #

-- 
2.32.0

[PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection

[Sean Rhodes]

From: Patrick Rudolph <patrick.rudolph@9elements.com>

This fixes an issue where the framebuffer provided by coreboot or
slimbootloader will only work on the primary VGA device. If the
framebuffer corresponds to a different device the screen will stay black.
In addition, the code doesn't work for multiple graphic cards, has
reference to non existing functions, and is a duplication of common code.

Call EfiBootManagerConnectVideoController on every display device found,
not only the legacy VGA device. This is the same as OvmfPkg does.

Allows to display output on the framebuffer set up by firmware, which might
not be the VGA device.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
 .../PlatformBootManagerLib/PlatformConsole.c   | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
index c93c7c99b5..bfaf89e74c 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
@@ -109,7 +109,7 @@ EFI_DEVICE_PATH_PROTOCOL  *gPlatformRootBridges[] = {
   NULL
 };
 
-BOOLEAN  mDetectVgaOnly;
+BOOLEAN  mDetectDisplayOnly;
 
 /**
   Add IsaKeyboard to ConIn; add IsaSerial to ConOut, ConIn, ErrOut.
@@ -482,7 +482,7 @@ DetectAndPreparePlatformPciDevicePath (
                     );
   ASSERT_EFI_ERROR (Status);
 
-  if (!mDetectVgaOnly) {
+  if (!mDetectDisplayOnly) {
     //
     // Here we decide whether it is LPC Bridge
     //
@@ -515,14 +515,14 @@ DetectAndPreparePlatformPciDevicePath (
   }
 
   //
-  // Here we decide which VGA device to enable in PCI bus
+  // Enable all display devices
   //
-  if (IS_PCI_VGA (Pci)) {
+  if (IS_PCI_DISPLAY (Pci)) {
     //
     // Add them to ConOut.
     //
-    DEBUG ((DEBUG_INFO, "Found PCI VGA device\n"));
-    PreparePciVgaDevicePath (Handle);
+    DEBUG ((DEBUG_INFO, "Found PCI Display device\n"));
+    EfiBootManagerConnectVideoController (Handle);
     return EFI_SUCCESS;
   }
 
@@ -532,7 +532,7 @@ DetectAndPreparePlatformPciDevicePath (
 /**
   Do platform specific PCI Device check and add them to ConOut, ConIn, ErrOut
 
-  @param[in]  DetectVgaOnly - Only detect VGA device if it's TRUE.
+  @param[in]  DetectDisplayOnly - Only detect display device if it's TRUE.
 
   @retval EFI_SUCCESS - PCI Device check and Console variable update successfully.
   @retval EFI_STATUS - PCI Device check or Console variable update fail.
@@ -540,10 +540,10 @@ DetectAndPreparePlatformPciDevicePath (
 **/
 EFI_STATUS
 DetectAndPreparePlatformPciDevicePaths (
-  BOOLEAN  DetectVgaOnly
+  BOOLEAN  DetectDisplayOnly
   )
 {
-  mDetectVgaOnly = DetectVgaOnly;
+  mDetectDisplayOnly = DetectDisplayOnly;
 
   EfiBootManagerUpdateConsoleVariable (
     ConIn,
-- 
2.32.0

Re: [PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection

[Guo Dong]

Reviewed-by: Guo Dong <guo.dong@intel.com>

-----Original Message-----
From: Sean Rhodes <sean@starlabs.systems> 
Sent: Thursday, February 10, 2022 2:28 PM
To: devel@edk2.groups.io
Cc: Dong, Guo <guo.dong@intel.com>; Patrick Rudolph <patrick.rudolph@9elements.com>
Subject: [PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection

From: Patrick Rudolph <patrick.rudolph@9elements.com>

This fixes an issue where the framebuffer provided by coreboot or slimbootloader will only work on the primary VGA device. If the framebuffer corresponds to a different device the screen will stay black.
In addition, the code doesn't work for multiple graphic cards, has reference to non existing functions, and is a duplication of common code.

Call EfiBootManagerConnectVideoController on every display device found, not only the legacy VGA device. This is the same as OvmfPkg does.

Allows to display output on the framebuffer set up by firmware, which might not be the VGA device.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
 .../PlatformBootManagerLib/PlatformConsole.c   | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
index c93c7c99b5..bfaf89e74c 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformConsole.c
@@ -109,7 +109,7 @@ EFI_DEVICE_PATH_PROTOCOL  *gPlatformRootBridges[] = {
   NULL }; -BOOLEAN  mDetectVgaOnly;+BOOLEAN  mDetectDisplayOnly;  /**   Add IsaKeyboard to ConIn; add IsaSerial to ConOut, ConIn, ErrOut.@@ -482,7 +482,7 @@ DetectAndPreparePlatformPciDevicePath (
                     );   ASSERT_EFI_ERROR (Status); -  if (!mDetectVgaOnly) {+  if (!mDetectDisplayOnly) {     //     // Here we decide whether it is LPC Bridge     //@@ -515,14 +515,14 @@ DetectAndPreparePlatformPciDevicePath (
   }    //-  // Here we decide which VGA device to enable in PCI bus+  // Enable all display devices   //-  if (IS_PCI_VGA (Pci)) {+  if (IS_PCI_DISPLAY (Pci)) {     //     // Add them to ConOut.     //-    DEBUG ((DEBUG_INFO, "Found PCI VGA device\n"));-    PreparePciVgaDevicePath (Handle);+    DEBUG ((DEBUG_INFO, "Found PCI Display device\n"));+    EfiBootManagerConnectVideoController (Handle);     return EFI_SUCCESS;   } @@ -532,7 +532,7 @@ DetectAndPreparePlatformPciDevicePath (
 /**   Do platform specific PCI Device check and add them to ConOut, ConIn, ErrOut -  @param[in]  DetectVgaOnly - Only detect VGA device if it's TRUE.+  @param[in]  DetectDisplayOnly - Only detect display device if it's TRUE.    @retval EFI_SUCCESS - PCI Device check and Console variable update successfully.   @retval EFI_STATUS - PCI Device check or Console variable update fail.@@ -540,10 +540,10 @@ DetectAndPreparePlatformPciDevicePath (
 **/ EFI_STATUS DetectAndPreparePlatformPciDevicePaths (-  BOOLEAN  DetectVgaOnly+  BOOLEAN  DetectDisplayOnly   ) {-  mDetectVgaOnly = DetectVgaOnly;+  mDetectDisplayOnly = DetectDisplayOnly;    EfiBootManagerUpdateConsoleVariable (     ConIn,-- 
2.32.0

Re: [edk2-devel] [PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection

[Sean Rhodes]

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

Hey Guo

Can I chase when this patch will be merged?

Many thanks

Sean

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

Re: [edk2-devel] [PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection

[Guo Dong]

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


Hi Sean,

Sorry for late response. I merged several UEFI payload packages yesterday including this one.
Please let me know if you have other patches pending to merge (The patches passed code review).

Thanks,
Guo

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean Rhodes
Sent: Monday, March 21, 2022 2:49 AM
To: Dong, Guo <guo.dong@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection

Hey Guo

Can I chase when this patch will be merged?

Many thanks

Sean


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

Re: [edk2-devel] [PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection

[Sean Rhodes]

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

Hey

No problem and thank you.

That have been reviewed, it's just these two:
https://github.com/tianocore/edk2/pull/2654
https://github.com/tianocore/edk2/pull/2485

Thanks

Sean


On Tue, 29 Mar 2022 at 18:32, Dong, Guo <guo.dong@intel.com> wrote:

>
>
> Hi Sean,
>
>
>
> Sorry for late response. I merged several UEFI payload packages yesterday
> including this one.
>
> Please let me know if you have other patches pending to merge (The patches
> passed code review).
>
>
>
> Thanks,
>
> Guo
>
>
>
> *From:* devel@edk2.groups.io <devel@edk2.groups.io> * On Behalf Of *Sean
> Rhodes
> *Sent:* Monday, March 21, 2022 2:49 AM
> *To:* Dong, Guo <guo.dong@intel.com>; devel@edk2.groups.io
> *Subject:* Re: [edk2-devel] [PATCH 15/18]
> UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection
>
>
>
> Hey Guo
>
> Can I chase when this patch will be merged?
>
> Many thanks
>
> Sean
>
> 
>

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

Re: [edk2-devel] [PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection

[Guo Dong]

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


Hi Sean,

I would help solve merge conflict and then merge https://github.com/tianocore/edk2/pull/2654.
For another patch, please contact MdeModulePkg maintainer to help merge it.

Thanks,
Guo

From: Sean Rhodes <sean@starlabs.systems>
Sent: Tuesday, March 29, 2022 10:35 AM
To: Dong, Guo <guo.dong@intel.com>
Cc: devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection

Hey

No problem and thank you.

That have been reviewed, it's just these two:
https://github.com/tianocore/edk2/pull/2654
https://github.com/tianocore/edk2/pull/2485

Thanks

Sean


On Tue, 29 Mar 2022 at 18:32, Dong, Guo <guo.dong@intel.com<mailto:guo.dong@intel.com>> wrote:

Hi Sean,

Sorry for late response. I merged several UEFI payload packages yesterday including this one.
Please let me know if you have other patches pending to merge (The patches passed code review).

Thanks,
Guo

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Sean Rhodes
Sent: Monday, March 21, 2022 2:49 AM
To: Dong, Guo <guo.dong@intel.com<mailto:guo.dong@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection

Hey Guo

Can I chase when this patch will be merged?

Many thanks

Sean


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

Re: [edk2-devel] [PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection

[Sean Rhodes]

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

Hi Guo

Merge conflict is resolved

Thanks

Sean


On Tue, 29 Mar 2022 at 19:01, Dong, Guo <guo.dong@intel.com> wrote:

>
>
> Hi Sean,
>
>
>
> I would help solve merge conflict and then merge
> https://github.com/tianocore/edk2/pull/2654.
>
> For another patch, please contact MdeModulePkg maintainer to help merge it.
>
>
>
> Thanks,
>
> Guo
>
>
>
> *From:* Sean Rhodes <sean@starlabs.systems>
> *Sent:* Tuesday, March 29, 2022 10:35 AM
> *To:* Dong, Guo <guo.dong@intel.com>
> *Cc:* devel@edk2.groups.io
> *Subject:* Re: [edk2-devel] [PATCH 15/18]
> UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection
>
>
>
> Hey
>
>
>
> No problem and thank you.
>
>
>
> That have been reviewed, it's just these two:
>
> https://github.com/tianocore/edk2/pull/2654
>
> https://github.com/tianocore/edk2/pull/2485
>
>
>
> Thanks
>
>
>
> Sean
>
>
>
>
>
> On Tue, 29 Mar 2022 at 18:32, Dong, Guo <guo.dong@intel.com> wrote:
>
>
>
> Hi Sean,
>
>
>
> Sorry for late response. I merged several UEFI payload packages yesterday
> including this one.
>
> Please let me know if you have other patches pending to merge (The patches
> passed code review).
>
>
>
> Thanks,
>
> Guo
>
>
>
> *From:* devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf Of *Sean
> Rhodes
> *Sent:* Monday, March 21, 2022 2:49 AM
> *To:* Dong, Guo <guo.dong@intel.com>; devel@edk2.groups.io
> *Subject:* Re: [edk2-devel] [PATCH 15/18]
> UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection
>
>
>
> Hey Guo
>
> Can I chase when this patch will be merged?
>
> Many thanks
>
> Sean
>
> 
>
>

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

Re: [edk2-devel] [PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection

[Guo Dong]

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


It was merged.

From: Sean Rhodes <sean@starlabs.systems>
Sent: Tuesday, March 29, 2022 11:05 AM
To: Dong, Guo <guo.dong@intel.com>
Cc: devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection

Hi Guo

Merge conflict is resolved

Thanks

Sean


On Tue, 29 Mar 2022 at 19:01, Dong, Guo <guo.dong@intel.com<mailto:guo.dong@intel.com>> wrote:

Hi Sean,

I would help solve merge conflict and then merge https://github.com/tianocore/edk2/pull/2654.
For another patch, please contact MdeModulePkg maintainer to help merge it.

Thanks,
Guo

From: Sean Rhodes <sean@starlabs.systems<mailto:sean@starlabs.systems>>
Sent: Tuesday, March 29, 2022 10:35 AM
To: Dong, Guo <guo.dong@intel.com<mailto:guo.dong@intel.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection

Hey

No problem and thank you.

That have been reviewed, it's just these two:
https://github.com/tianocore/edk2/pull/2654
https://github.com/tianocore/edk2/pull/2485

Thanks

Sean


On Tue, 29 Mar 2022 at 18:32, Dong, Guo <guo.dong@intel.com<mailto:guo.dong@intel.com>> wrote:

Hi Sean,

Sorry for late response. I merged several UEFI payload packages yesterday including this one.
Please let me know if you have other patches pending to merge (The patches passed code review).

Thanks,
Guo

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Sean Rhodes
Sent: Monday, March 21, 2022 2:49 AM
To: Dong, Guo <guo.dong@intel.com<mailto:guo.dong@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [PATCH 15/18] UefiPayloadPkg/Library/PlatformBootManagerLib: Remove broken VGA detection

Hey Guo

Can I chase when this patch will be merged?

Many thanks

Sean


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

[PATCH 16/18] UefiCpuPkg: Disable MTRR programming for UefiPayloadPkg

[Sean Rhodes]

From: Patrick Rudolph <patrick.rudolph@9elements.com>

The MTRRs have already been programmed by FSB.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
 UefiCpuPkg/Library/MtrrLib/MtrrLib.c   | 4 ++++
 UefiCpuPkg/Library/MtrrLib/MtrrLib.inf | 2 +-
 UefiCpuPkg/UefiCpuPkg.dec              | 4 ++++
 UefiCpuPkg/UefiCpuPkg.uni              | 8 ++++++--
 UefiPayloadPkg/UefiPayloadPkg.dsc      | 4 ++++
 5 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
index e5c862c83d..8734bae2f0 100644
--- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
+++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
@@ -2845,6 +2845,10 @@ IsMtrrSupported (
   CPUID_VERSION_INFO_EDX     Edx;
   MSR_IA32_MTRRCAP_REGISTER  MtrrCap;
 
+  if (PcdGetBool (PcdCpuDisableMtrrProgramming)) {
+    return FALSE;
+  }
+
   //
   // Check CPUID(1).EDX[12] for MTRR capability
   //
diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf b/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
index 4c9ea2def3..afc0bcc724 100644
--- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
+++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
@@ -15,7 +15,6 @@
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = MtrrLib
 
-
 #
 # The following information is for reference only and not required by the build tools.
 #
@@ -37,4 +36,5 @@
 
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuNumberOfReservedVariableMtrrs   ## SOMETIMES_CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuDisableMtrrProgramming          ## CONSUMES
 
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 7de66fde67..a2a4ecc094 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -217,6 +217,10 @@
   # @Prompt SMM Code Access Check.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmCodeAccessCheckEnable|TRUE|BOOLEAN|0x60000013
 
+  ## Disables MTRR programming in case it's already programmed by FSB.
+  # @Prompt Disable MTRR programming.
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuDisableMtrrProgramming|FALSE|BOOLEAN|0x00000017
+
   ## Specifies the number of variable MTRRs reserved for OS use. The default number of
   #  MTRRs reserved for OS use is 2.
   # @Prompt Number of reserved variable MTRRs.
diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
index 219c1963bf..3db0063f50 100644
--- a/UefiCpuPkg/UefiCpuPkg.uni
+++ b/UefiCpuPkg/UefiCpuPkg.uni
@@ -140,9 +140,13 @@
 
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuHotPlugDataAddress_HELP  #language en-US "Contains the pointer to a CPU Hot Plug Data structure if CPU hot-plug is supported."
 
-#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuNumberOfReservedVariableMtrrs_PROMPT  #language en-US "Number of reserved variable MTRRs"
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuDisableMtrrProgramming_PROMPT  #language en-US "Number of reserved variable MTRRs"
 
-#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuNumberOfReservedVariableMtrrs_HELP  #language en-US "Specifies the number of variable MTRRs reserved for OS use."
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuDisableMtrrProgramming_HELP  #language en-US "Specifies the number of variable MTRRs reserved for OS use."
+
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuNumberOfReservedVariableMtrrs_PROMPT  #language en-US "Disable MTRR programming."
+
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuNumberOfReservedVariableMtrrs_HELP  #language en-US "Disables MTRR programming in case it's already programmed by FSB."
 
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuApLoopMode_PROMPT  #language en-US "The AP wait loop state"
 
diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index c8562d592b..afec9109c1 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -37,6 +37,7 @@
   DEFINE BOOT_MANAGER_ESCAPE          = FALSE
   DEFINE BOOTSPLASH_IMAGE             = FALSE
   DEFINE FOLLOW_BGRT_SPEC             = FALSE
+  DEFINE MTRR_PROGRAMMING             = TRUE
   DEFINE PLATFORM_BOOT_TIMEOUT        = 3
   #
   # SBL:      UEFI payload for Slim Bootloader
@@ -412,6 +413,9 @@
 
   gUefiPayloadPkgTokenSpaceGuid.PcdAbove4GMemory|$(ABOVE_4G_MEMORY)
 
+  # Disable MTRR programming
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuDisableMtrrProgramming|$(MTRR_PROGRAMMING)
+
 [PcdsPatchableInModule.X64]
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister|$(RTC_INDEX_REGISTER)
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister|$(RTC_TARGET_REGISTER)
-- 
2.32.0

Re: [edk2-devel] [PATCH 16/18] UefiCpuPkg: Disable MTRR programming for UefiPayloadPkg

[Michael D Kinney]

Hi Sean,

By disabling the MTRR lib in this way, the UefiPayloadPkg has no means to
adjust cachability if it needs to in the future, and it breaks conformance 
with the PI Specification.

What modules in the UefiPayloadPkg are changing cachability?  Is that appropriate?
Should those modules be updates to not make those requests instead?

Just want to know if this alternate method of preventing MTRR changes was 
evaluated.

Thanks,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean Rhodes
> Sent: Thursday, February 10, 2022 1:28 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Patrick Rudolph <patrick.rudolph@9elements.com>
> Subject: [edk2-devel] [PATCH 16/18] UefiCpuPkg: Disable MTRR programming for UefiPayloadPkg
> 
> From: Patrick Rudolph <patrick.rudolph@9elements.com>
> 
> The MTRRs have already been programmed by FSB.
> 
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> ---
>  UefiCpuPkg/Library/MtrrLib/MtrrLib.c   | 4 ++++
>  UefiCpuPkg/Library/MtrrLib/MtrrLib.inf | 2 +-
>  UefiCpuPkg/UefiCpuPkg.dec              | 4 ++++
>  UefiCpuPkg/UefiCpuPkg.uni              | 8 ++++++--
>  UefiPayloadPkg/UefiPayloadPkg.dsc      | 4 ++++
>  5 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> index e5c862c83d..8734bae2f0 100644
> --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> @@ -2845,6 +2845,10 @@ IsMtrrSupported (
>    CPUID_VERSION_INFO_EDX     Edx;
> 
>    MSR_IA32_MTRRCAP_REGISTER  MtrrCap;
> 
> 
> 
> +  if (PcdGetBool (PcdCpuDisableMtrrProgramming)) {
> 
> +    return FALSE;
> 
> +  }
> 
> +
> 
>    //
> 
>    // Check CPUID(1).EDX[12] for MTRR capability
> 
>    //
> 
> diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf b/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> index 4c9ea2def3..afc0bcc724 100644
> --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> @@ -15,7 +15,6 @@
>    VERSION_STRING                 = 1.0
> 
>    LIBRARY_CLASS                  = MtrrLib
> 
> 
> 
> -
> 
>  #
> 
>  # The following information is for reference only and not required by the build tools.
> 
>  #
> 
> @@ -37,4 +36,5 @@
> 
> 
>  [Pcd]
> 
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuNumberOfReservedVariableMtrrs   ## SOMETIMES_CONSUMES
> 
> +  gUefiCpuPkgTokenSpaceGuid.PcdCpuDisableMtrrProgramming          ## CONSUMES
> 
> 
> 
> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
> index 7de66fde67..a2a4ecc094 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dec
> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> @@ -217,6 +217,10 @@
>    # @Prompt SMM Code Access Check.
> 
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmCodeAccessCheckEnable|TRUE|BOOLEAN|0x60000013
> 
> 
> 
> +  ## Disables MTRR programming in case it's already programmed by FSB.
> 
> +  # @Prompt Disable MTRR programming.
> 
> +  gUefiCpuPkgTokenSpaceGuid.PcdCpuDisableMtrrProgramming|FALSE|BOOLEAN|0x00000017
> 
> +
> 
>    ## Specifies the number of variable MTRRs reserved for OS use. The default number of
> 
>    #  MTRRs reserved for OS use is 2.
> 
>    # @Prompt Number of reserved variable MTRRs.
> 
> diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
> index 219c1963bf..3db0063f50 100644
> --- a/UefiCpuPkg/UefiCpuPkg.uni
> +++ b/UefiCpuPkg/UefiCpuPkg.uni
> @@ -140,9 +140,13 @@
> 
> 
>  #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuHotPlugDataAddress_HELP  #language en-US "Contains the pointer to a CPU Hot Plug
> Data structure if CPU hot-plug is supported."
> 
> 
> 
> -#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuNumberOfReservedVariableMtrrs_PROMPT  #language en-US "Number of reserved variable
> MTRRs"
> 
> +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuDisableMtrrProgramming_PROMPT  #language en-US "Number of reserved variable MTRRs"
> 
> 
> 
> -#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuNumberOfReservedVariableMtrrs_HELP  #language en-US "Specifies the number of
> variable MTRRs reserved for OS use."
> 
> +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuDisableMtrrProgramming_HELP  #language en-US "Specifies the number of variable
> MTRRs reserved for OS use."
> 
> +
> 
> +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuNumberOfReservedVariableMtrrs_PROMPT  #language en-US "Disable MTRR programming."
> 
> +
> 
> +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuNumberOfReservedVariableMtrrs_HELP  #language en-US "Disables MTRR programming in
> case it's already programmed by FSB."
> 
> 
> 
>  #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuApLoopMode_PROMPT  #language en-US "The AP wait loop state"
> 
> 
> 
> diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
> index c8562d592b..afec9109c1 100644
> --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> @@ -37,6 +37,7 @@
>    DEFINE BOOT_MANAGER_ESCAPE          = FALSE
> 
>    DEFINE BOOTSPLASH_IMAGE             = FALSE
> 
>    DEFINE FOLLOW_BGRT_SPEC             = FALSE
> 
> +  DEFINE MTRR_PROGRAMMING             = TRUE
> 
>    DEFINE PLATFORM_BOOT_TIMEOUT        = 3
> 
>    #
> 
>    # SBL:      UEFI payload for Slim Bootloader
> 
> @@ -412,6 +413,9 @@
> 
> 
>    gUefiPayloadPkgTokenSpaceGuid.PcdAbove4GMemory|$(ABOVE_4G_MEMORY)
> 
> 
> 
> +  # Disable MTRR programming
> 
> +  gUefiCpuPkgTokenSpaceGuid.PcdCpuDisableMtrrProgramming|$(MTRR_PROGRAMMING)
> 
> +
> 
>  [PcdsPatchableInModule.X64]
> 
>    gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister|$(RTC_INDEX_REGISTER)
> 
>    gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister|$(RTC_TARGET_REGISTER)
> 
> --
> 2.32.0
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#86587): https://edk2.groups.io/g/devel/message/86587
> Mute This Topic: https://groups.io/mt/89056162/1643496
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com]
> -=-=-=-=-=-=
> 

Re: [edk2-devel] [PATCH 16/18] UefiCpuPkg: Disable MTRR programming for UefiPayloadPkg

[Patrick Rudolph]

Hi Mike,
I don't see this covered by the PI specification (it's soo huge).
Which chapter describes this?
The original issue was a boot hang observed on real hardware as the
MtrrLib can't handle
specific corner cases. Not calling into MtrrLib fixes the issue.

An alternative approach would be to fix the corner case handling, or
introduce and use a MtrrLibNull where necessary.

Regards,
Patrick Rudolph

On Thu, Feb 10, 2022 at 11:06 PM Michael D Kinney
<michael.d.kinney@intel.com> wrote:
>
> Hi Sean,
>
> By disabling the MTRR lib in this way, the UefiPayloadPkg has no means to
> adjust cachability if it needs to in the future, and it breaks conformance
> with the PI Specification.
>
> What modules in the UefiPayloadPkg are changing cachability?  Is that appropriate?
> Should those modules be updates to not make those requests instead?
>
> Just want to know if this alternate method of preventing MTRR changes was
> evaluated.
>
> Thanks,
>
> Mike
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean Rhodes
> > Sent: Thursday, February 10, 2022 1:28 PM
> > To: devel@edk2.groups.io
> > Cc: Dong, Eric <eric.dong@intel.com>; Patrick Rudolph <patrick.rudolph@9elements.com>
> > Subject: [edk2-devel] [PATCH 16/18] UefiCpuPkg: Disable MTRR programming for UefiPayloadPkg
> >
> > From: Patrick Rudolph <patrick.rudolph@9elements.com>
> >
> > The MTRRs have already been programmed by FSB.
> >
> > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> > ---
> >  UefiCpuPkg/Library/MtrrLib/MtrrLib.c   | 4 ++++
> >  UefiCpuPkg/Library/MtrrLib/MtrrLib.inf | 2 +-
> >  UefiCpuPkg/UefiCpuPkg.dec              | 4 ++++
> >  UefiCpuPkg/UefiCpuPkg.uni              | 8 ++++++--
> >  UefiPayloadPkg/UefiPayloadPkg.dsc      | 4 ++++
> >  5 files changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> > index e5c862c83d..8734bae2f0 100644
> > --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> > +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> > @@ -2845,6 +2845,10 @@ IsMtrrSupported (
> >    CPUID_VERSION_INFO_EDX     Edx;
> >
> >    MSR_IA32_MTRRCAP_REGISTER  MtrrCap;
> >
> >
> >
> > +  if (PcdGetBool (PcdCpuDisableMtrrProgramming)) {
> >
> > +    return FALSE;
> >
> > +  }
> >
> > +
> >
> >    //
> >
> >    // Check CPUID(1).EDX[12] for MTRR capability
> >
> >    //
> >
> > diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf b/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> > index 4c9ea2def3..afc0bcc724 100644
> > --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> > +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> > @@ -15,7 +15,6 @@
> >    VERSION_STRING                 = 1.0
> >
> >    LIBRARY_CLASS                  = MtrrLib
> >
> >
> >
> > -
> >
> >  #
> >
> >  # The following information is for reference only and not required by the build tools.
> >
> >  #
> >
> > @@ -37,4 +36,5 @@
> >
> >
> >  [Pcd]
> >
> >    gUefiCpuPkgTokenSpaceGuid.PcdCpuNumberOfReservedVariableMtrrs   ## SOMETIMES_CONSUMES
> >
> > +  gUefiCpuPkgTokenSpaceGuid.PcdCpuDisableMtrrProgramming          ## CONSUMES
> >
> >
> >
> > diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
> > index 7de66fde67..a2a4ecc094 100644
> > --- a/UefiCpuPkg/UefiCpuPkg.dec
> > +++ b/UefiCpuPkg/UefiCpuPkg.dec
> > @@ -217,6 +217,10 @@
> >    # @Prompt SMM Code Access Check.
> >
> >    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmCodeAccessCheckEnable|TRUE|BOOLEAN|0x60000013
> >
> >
> >
> > +  ## Disables MTRR programming in case it's already programmed by FSB.
> >
> > +  # @Prompt Disable MTRR programming.
> >
> > +  gUefiCpuPkgTokenSpaceGuid.PcdCpuDisableMtrrProgramming|FALSE|BOOLEAN|0x00000017
> >
> > +
> >
> >    ## Specifies the number of variable MTRRs reserved for OS use. The default number of
> >
> >    #  MTRRs reserved for OS use is 2.
> >
> >    # @Prompt Number of reserved variable MTRRs.
> >
> > diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
> > index 219c1963bf..3db0063f50 100644
> > --- a/UefiCpuPkg/UefiCpuPkg.uni
> > +++ b/UefiCpuPkg/UefiCpuPkg.uni
> > @@ -140,9 +140,13 @@
> >
> >
> >  #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuHotPlugDataAddress_HELP  #language en-US "Contains the pointer to a CPU Hot Plug
> > Data structure if CPU hot-plug is supported."
> >
> >
> >
> > -#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuNumberOfReservedVariableMtrrs_PROMPT  #language en-US "Number of reserved variable
> > MTRRs"
> >
> > +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuDisableMtrrProgramming_PROMPT  #language en-US "Number of reserved variable MTRRs"
> >
> >
> >
> > -#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuNumberOfReservedVariableMtrrs_HELP  #language en-US "Specifies the number of
> > variable MTRRs reserved for OS use."
> >
> > +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuDisableMtrrProgramming_HELP  #language en-US "Specifies the number of variable
> > MTRRs reserved for OS use."
> >
> > +
> >
> > +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuNumberOfReservedVariableMtrrs_PROMPT  #language en-US "Disable MTRR programming."
> >
> > +
> >
> > +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuNumberOfReservedVariableMtrrs_HELP  #language en-US "Disables MTRR programming in
> > case it's already programmed by FSB."
> >
> >
> >
> >  #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuApLoopMode_PROMPT  #language en-US "The AP wait loop state"
> >
> >
> >
> > diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
> > index c8562d592b..afec9109c1 100644
> > --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> > +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> > @@ -37,6 +37,7 @@
> >    DEFINE BOOT_MANAGER_ESCAPE          = FALSE
> >
> >    DEFINE BOOTSPLASH_IMAGE             = FALSE
> >
> >    DEFINE FOLLOW_BGRT_SPEC             = FALSE
> >
> > +  DEFINE MTRR_PROGRAMMING             = TRUE
> >
> >    DEFINE PLATFORM_BOOT_TIMEOUT        = 3
> >
> >    #
> >
> >    # SBL:      UEFI payload for Slim Bootloader
> >
> > @@ -412,6 +413,9 @@
> >
> >
> >    gUefiPayloadPkgTokenSpaceGuid.PcdAbove4GMemory|$(ABOVE_4G_MEMORY)
> >
> >
> >
> > +  # Disable MTRR programming
> >
> > +  gUefiCpuPkgTokenSpaceGuid.PcdCpuDisableMtrrProgramming|$(MTRR_PROGRAMMING)
> >
> > +
> >
> >  [PcdsPatchableInModule.X64]
> >
> >    gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister|$(RTC_INDEX_REGISTER)
> >
> >    gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister|$(RTC_TARGET_REGISTER)
> >
> > --
> > 2.32.0
> >
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#86587): https://edk2.groups.io/g/devel/message/86587
> > Mute This Topic: https://groups.io/mt/89056162/1643496
> > Group Owner: devel+owner@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com]
> > -=-=-=-=-=-=
> >
>
>
>
> 
>
>

[PATCH 17/18] BmBoot: prioritize internal boot devices over external

[Sean Rhodes]

From: Matt DeVillier <matt.devillier@gmail.com>

This prevents some internally-attached USB devices
(card readers, etc) from defaulting ahead of SATA/NVMe/eMMC drives.

Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c       | 10 ++++++++--
 .../Library/UefiBootManagerLib/UefiBootManagerLib.inf  |  1 +
 MdeModulePkg/MdeModulePkg.dec                          |  3 +++
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 2da6d2381e..a41a69afd2 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -2216,8 +2216,14 @@ BmEnumerateBootOptions (
       //
       // Skip the fixed block io then the removable block io
       //
-      if (BlkIo->Media->RemovableMedia == ((Removable == 0) ? FALSE : TRUE)) {
-        continue;
+      if (FixedPcdGetBool (PcdPrioritizeInternal)) {
+        if (BlkIo->Media->RemovableMedia == (Removable == 0)) {
+          continue;
+        }
+      } else {
+        if (BlkIo->Media->RemovableMedia == ((Removable == 0) ? FALSE : TRUE)) {
+          continue;
+        }
       }
 
       Description = BmGetBootDescription (Handles[Index]);
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
index fe05d5f1cc..207420e334 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
+++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
@@ -119,3 +119,4 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile                     ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdDriverHealthConfigureForm               ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxRepairCount                          ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPrioritizeInternal
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index b4855431b0..38e2573a6d 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1077,6 +1077,9 @@
   # @Prompt Enable UEFI Stack Guard.
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x30001055
 
+  # Prioritize Internal Devices
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPrioritizeInternal|FALSE|BOOLEAN|0x00000031
+
 [PcdsFixedAtBuild, PcdsPatchableInModule]
   ## Dynamic type PCD can be registered callback function for Pcd setting action.
   #  PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum number of callback function
-- 
2.32.0

[PATCH 18/18] UefiPayloadPkg: Hookup Prioritise Internal build option

[Sean Rhodes]

Hook PRIORITISE_INTERNAL build option to PrioritiseInternal PCD.

Signed-off-by: Sean Rhodes <sean@starlabs.systems>
---
 UefiPayloadPkg/UefiPayloadPkg.dsc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index afec9109c1..86153d7eaa 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -39,6 +39,7 @@
   DEFINE FOLLOW_BGRT_SPEC             = FALSE
   DEFINE MTRR_PROGRAMMING             = TRUE
   DEFINE PLATFORM_BOOT_TIMEOUT        = 3
+  DEFINE PRIORITIZE_INTERNAL          = FALSE
   #
   # SBL:      UEFI payload for Slim Bootloader
   # COREBOOT: UEFI payload for coreboot
@@ -413,9 +414,10 @@
 
   gUefiPayloadPkgTokenSpaceGuid.PcdAbove4GMemory|$(ABOVE_4G_MEMORY)
 
-  # Disable MTRR programming
   gUefiCpuPkgTokenSpaceGuid.PcdCpuDisableMtrrProgramming|$(MTRR_PROGRAMMING)
 
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPrioritizeInternal|$(PRIORITISE_INTERNAL)
+
 [PcdsPatchableInModule.X64]
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister|$(RTC_INDEX_REGISTER)
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister|$(RTC_TARGET_REGISTER)
-- 
2.32.0