[PATCH 0/6] UefiPayloadPkg: LinuxBoot Support in UefiPayload

[PATCH 0/6] UefiPayloadPkg: LinuxBoot Support in UefiPayload

[Cheng-Chieh Huang]

These are necessary patches to Support LinuxBoot in UefiPayload.
With these paches, we can boot to ESXi and Windows from a linux in QEMU.

LinuxBoot README:
https://github.com/linuxboot/edk2/blob/uefipayload/UefiPayloadPkg/README.md

PR to tianocore:
https://github.com/tianocore/edk2/pull/1820

Cheng-Chieh Huang (5):
  Add LINUXBOOT payload target
  Use legacy timer in Linuxboot payload
  Update maximum logic processor to 256
  Reserve Payload config in runtime services data
  Add DISABLE_MMX_SSE to avoid generating floating points operation

Trammell Hudson (1):
  LinuxBoot: use a text format for the configuration block.

 UefiPayloadPkg/UefiPayloadPkg.dsc             |  29 +-
 UefiPayloadPkg/UefiPayloadPkg.fdf             |   5 +
 .../Library/LbParseLib/LbParseLib.inf         |  39 ++
 UefiPayloadPkg/Include/Linuxboot.h            |  58 +++
 .../Library/LbParseLib/LbParseLib.c           | 348 ++++++++++++++++++
 .../PciHostBridgeLib/PciHostBridgeSupport.c   |   6 +-
 .../UefiPayloadEntry/UefiPayloadEntry.c       |   2 +
 CryptoPkg/Library/OpensslLib/openssl          |   2 +-
 8 files changed, 480 insertions(+), 9 deletions(-)
 create mode 100644 UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf
 create mode 100644 UefiPayloadPkg/Include/Linuxboot.h
 create mode 100644 UefiPayloadPkg/Library/LbParseLib/LbParseLib.c

Cc: Cheng-Chieh Huang <chengchieh@google.com>
Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
Cc: Trammell Hudson <hudson@trmm.net>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
-- 
2.32.0.402.g57bb445576-goog

[PATCH v1 1/6] UefiPayloadPkg: Add LINUXBOOT payload target

[Cheng-Chieh Huang]

Initial commit to support linuxboot payload.

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
 UefiPayloadPkg/UefiPayloadPkg.dsc                              |  16 +-
 UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf               |  39 +++++
 UefiPayloadPkg/Include/Linuxboot.h                             |  47 ++++++
 UefiPayloadPkg/Library/LbParseLib/LbParseLib.c                 | 168 ++++++++++++++++++++
 UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c |   6 +-
 CryptoPkg/Library/OpensslLib/openssl                           |   2 +-
 6 files changed, 270 insertions(+), 8 deletions(-)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index bcedf1c746b4..54576ba485b7 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -33,6 +33,7 @@ [Defines]
   #
   # SBL:      UEFI payload for Slim Bootloader
   # COREBOOT: UEFI payload for coreboot
+  # LINUXBOOT: UEFI payload for linuxboot
   #
   DEFINE   BOOTLOADER = SBL
 
@@ -93,6 +94,9 @@ [Defines]
 
 [BuildOptions]
   *_*_*_CC_FLAGS                 = -D DISABLE_NEW_DEPRECATED_INTERFACES
+!if $(BOOTLOADER) == "LINUXBOOT"
+  *_*_*_CC_FLAGS                 = -D LINUXBOOT_PAYLOAD
+!endif
   GCC:*_UNIXGCC_*_CC_FLAGS       = -DMDEPKG_NDEBUG
   GCC:RELEASE_*_*_CC_FLAGS       = -DMDEPKG_NDEBUG
   INTEL:RELEASE_*_*_CC_FLAGS     = /D MDEPKG_NDEBUG
@@ -222,11 +226,13 @@ [LibraryClasses]
 !endif
   PlatformSupportLib|UefiPayloadPkg/Library/PlatformSupportLibNull/PlatformSupportLibNull.inf
 !if $(UNIVERSAL_PAYLOAD) == FALSE
-  !if $(BOOTLOADER) == "COREBOOT"
-    BlParseLib|UefiPayloadPkg/Library/CbParseLib/CbParseLib.inf
-  !else
-    BlParseLib|UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
-  !endif
+ !if $(BOOTLOADER) == "COREBOOT"
+   BlParseLib|UefiPayloadPkg/Library/CbParseLib/CbParseLib.inf
+ !elseif $(BOOTLOADER) == "LINUXBOOT"
+   BlParseLib|UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf
+ !else
+   BlParseLib|UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
+ !endif
 !endif
 
   DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
diff --git a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf
new file mode 100644
index 000000000000..d75ba8db8cf3
--- /dev/null
+++ b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf
@@ -0,0 +1,39 @@
+## @file
+#  Linuxboot Table Parse Library.
+#
+#  Copyright (c) 2021, the u-root Authors. All rights reserved.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = LbParseLib
+  FILE_GUID                      = DBA15E1E-4C16-47DF-93C0-AB5888ED14C3
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = BlParseLib
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64
+#
+
+[Sources]
+  LbParseLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  UefiPayloadPkg/UefiPayloadPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  IoLib
+  DebugLib
+  PcdLib
+
+[Pcd]
+  gUefiPayloadPkgTokenSpaceGuid.PcdPayloadFdMemBase
diff --git a/UefiPayloadPkg/Include/Linuxboot.h b/UefiPayloadPkg/Include/Linuxboot.h
new file mode 100644
index 000000000000..34ca18069983
--- /dev/null
+++ b/UefiPayloadPkg/Include/Linuxboot.h
@@ -0,0 +1,47 @@
+/** @file
+  LinuxBoot PEI module include file.
+**/
+#ifndef _LINUXBOOT_PEI_H_INCLUDED_
+#define _LINUXBOOT_PEI_H_INCLUDED_
+
+#if defined(_MSC_VER)
+#pragma warning(disable : 4200)
+#endif
+
+#pragma pack(1)
+typedef struct SerialPortConfigStruct {
+  UINT32 Type;
+  UINT32 BaseAddr;
+  UINT32 Baud;
+  UINT32 RegWidth;
+  UINT32 InputHertz;
+  UINT32 UartPciAddr;
+} SerialPortConfig;
+
+typedef struct MemoryMapEntryStruct {
+  UINT64 Start;
+  UINT64 End;
+  UINT32 Type;
+} MemoryMapEntry;
+
+typedef struct UefiPayloadConfigStruct {
+  UINT64 Version;
+  UINT64 AcpiBase;
+  UINT64 AcpiSize;
+  UINT64 SmbiosBase;
+  UINT64 SmbiosSize;
+  SerialPortConfig SerialConfig;
+  UINT32 NumMemoryMapEntries;
+  MemoryMapEntry MemoryMapEntries[0];
+} UefiPayloadConfig;
+#pragma pack()
+
+#define UEFI_PAYLOAD_CONFIG_VERSION 1
+
+#define LINUXBOOT_MEM_RAM 1
+#define LINUXBOOT_MEM_DEFAULT 2
+#define LINUXBOOT_MEM_ACPI 3
+#define LINUXBOOT_MEM_NVS 4
+#define LINUXBOOT_MEM_RESERVED 5
+
+#endif  // _LINUXBOOT_PEI_H_INCLUDED_
diff --git a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
new file mode 100644
index 000000000000..34bfb6a1073f
--- /dev/null
+++ b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
@@ -0,0 +1,168 @@
+/** @file
+  This library will parse the linuxboot table in memory and extract those required
+  information.
+
+  Copyright (c) 2021, the u-root Authors. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+
+#include <IndustryStandard/Acpi.h>
+#include <IndustryStandard/SmBios.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/BlParseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/IoLib.h>
+#include <Library/PcdLib.h>
+#include <Linuxboot.h>
+#include <Uefi/UefiBaseType.h>
+
+// Retrieve UefiPayloadConfig from Linuxboot's uefiboot
+UefiPayloadConfig* GetUefiPayLoadConfig() {
+  UefiPayloadConfig* config =
+      (UefiPayloadConfig*)(UINTN)(PcdGet32(PcdPayloadFdMemBase) - SIZE_64KB);
+  if (config->Version != UEFI_PAYLOAD_CONFIG_VERSION) {
+    DEBUG((DEBUG_ERROR, "Expect payload config version: %d, but get %d\n",
+           UEFI_PAYLOAD_CONFIG_VERSION, config->Version));
+    CpuDeadLoop ();
+  }
+  return config;
+}
+
+// Align the address and add memory rang to MemInfoCallback
+void AddMemoryRange(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN UINTN start,
+                    IN UINTN end, IN int type) {
+  MEMROY_MAP_ENTRY MemoryMap;
+  UINTN AlignedStart;
+  UINTN AlignedEnd;
+  AlignedStart = ALIGN_VALUE(start, SIZE_4KB);
+  AlignedEnd = ALIGN_VALUE(end, SIZE_4KB);
+  // Conservative adjustment on Memory map. This should happen when booting from
+  // non UEFI bios and it may report a memory region less than 4KB.
+  if (AlignedStart > start && type != LINUXBOOT_MEM_RAM) {
+    AlignedStart -= SIZE_4KB;
+  }
+  if (AlignedEnd > end + 1 && type == LINUXBOOT_MEM_RAM) {
+    AlignedEnd -= SIZE_4KB;
+  }
+  MemoryMap.Base = AlignedStart;
+  MemoryMap.Size = AlignedEnd - AlignedStart;
+  MemoryMap.Type = type;
+  MemoryMap.Flag = 0;
+  MemInfoCallback(&MemoryMap, NULL);
+}
+
+/**
+  Acquire the memory information from the linuxboot table in memory.
+
+  @param  MemInfoCallback     The callback routine
+  @param  Params              Pointer to the callback routine parameter
+
+  @retval RETURN_SUCCESS     Successfully find out the memory information.
+  @retval RETURN_NOT_FOUND   Failed to find the memory information.
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseMemoryInfo(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN VOID* Params) {
+  UefiPayloadConfig* config;
+  int i;
+
+  config = GetUefiPayLoadConfig();
+
+  DEBUG((DEBUG_INFO, "MemoryMap #entries: %d\n", config->NumMemoryMapEntries));
+
+  MemoryMapEntry* entry = &config->MemoryMapEntries[0];
+  for (i = 0; i < config->NumMemoryMapEntries; i++) {
+    DEBUG((DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n", entry->Start,
+           entry->End, entry->Type));
+    AddMemoryRange(MemInfoCallback, entry->Start, entry->End, entry->Type);
+    entry++;
+  }
+  return RETURN_SUCCESS;
+}
+
+/**
+  Acquire acpi table and smbios table from linuxboot
+
+  @param  SystemTableInfo          Pointer to the system table info
+
+  @retval RETURN_SUCCESS            Successfully find out the tables.
+  @retval RETURN_NOT_FOUND          Failed to find the tables.
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseSystemTable(OUT SYSTEM_TABLE_INFO* SystemTableInfo) {
+  UefiPayloadConfig* config;
+
+  config = GetUefiPayLoadConfig();
+  SystemTableInfo->AcpiTableBase = config->AcpiBase;
+  SystemTableInfo->AcpiTableSize = config->AcpiSize;
+
+  SystemTableInfo->SmbiosTableBase = config->SmbiosBase;
+  SystemTableInfo->SmbiosTableSize = config->SmbiosSize;
+
+  return RETURN_SUCCESS;
+}
+
+/**
+  Find the serial port information
+
+  @param  SERIAL_PORT_INFO   Pointer to serial port info structure
+
+  @retval RETURN_SUCCESS     Successfully find the serial port information.
+  @retval RETURN_NOT_FOUND   Failed to find the serial port information .
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseSerialInfo(OUT SERIAL_PORT_INFO* SerialPortInfo) {
+  UefiPayloadConfig* config;
+  config = GetUefiPayLoadConfig();
+
+  SerialPortInfo->BaseAddr = config->SerialConfig.BaseAddr;
+  SerialPortInfo->RegWidth = config->SerialConfig.RegWidth;
+  SerialPortInfo->Type = config->SerialConfig.Type;
+  SerialPortInfo->Baud = config->SerialConfig.Baud;
+  SerialPortInfo->InputHertz = config->SerialConfig.InputHertz;
+  SerialPortInfo->UartPciAddr = config->SerialConfig.UartPciAddr;
+
+  return RETURN_SUCCESS;
+}
+
+/**
+  Find the video frame buffer information
+
+  @param  GfxInfo             Pointer to the EFI_PEI_GRAPHICS_INFO_HOB structure
+
+  @retval RETURN_SUCCESS     Successfully find the video frame buffer
+information.
+  @retval RETURN_NOT_FOUND   Failed to find the video frame buffer information .
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseGfxInfo(OUT EFI_PEI_GRAPHICS_INFO_HOB* GfxInfo) {
+  // Not supported
+  return RETURN_NOT_FOUND;
+}
+
+/**
+  Find the video frame buffer device information
+
+  @param  GfxDeviceInfo      Pointer to the EFI_PEI_GRAPHICS_DEVICE_INFO_HOB
+structure
+
+  @retval RETURN_SUCCESS     Successfully find the video frame buffer
+information.
+  @retval RETURN_NOT_FOUND   Failed to find the video frame buffer information.
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseGfxDeviceInfo(OUT EFI_PEI_GRAPHICS_DEVICE_INFO_HOB* GfxDeviceInfo) {
+  return RETURN_NOT_FOUND;
+}
diff --git a/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c b/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
index b0268f05069c..a4f714f765ea 100644
--- a/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
+++ b/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
@@ -40,8 +40,9 @@ AdjustRootBridgeResource (
   IN  PCI_ROOT_BRIDGE_APERTURE *PMemAbove4G
 )
 {
+#ifndef LINUXBOOT_PAYLOAD
   UINT64  Mask;
-
+#endif
   //
   // For now try to downgrade everything into MEM32 since
   // - coreboot does not assign resource above 4GB
@@ -80,7 +81,7 @@ AdjustRootBridgeResource (
     PMemAbove4G->Base  = MAX_UINT64;
     PMemAbove4G->Limit = 0;
   }
-
+#ifndef LINUXBOOT_PAYLOAD
   //
   // Align IO  resource at 4K  boundary
   //
@@ -98,6 +99,7 @@ AdjustRootBridgeResource (
   if (Mem->Base != MAX_UINT64) {
     Mem->Base &= ~Mask;
   }
+#endif
 }
 
 /**
diff --git a/CryptoPkg/Library/OpensslLib/openssl b/CryptoPkg/Library/OpensslLib/openssl
index 52c587d60be6..e2e09d9fba11 160000
--- a/CryptoPkg/Library/OpensslLib/openssl
+++ b/CryptoPkg/Library/OpensslLib/openssl
@@ -1 +1 @@
-Subproject commit 52c587d60be67c337364b830dd3fdc15404a2f04
+Subproject commit e2e09d9fba1187f8d6aafaa34d4172f56f1ffb72
-- 
2.32.0.402.g57bb445576-goog

Re: [edk2-devel] [PATCH v1 1/6] UefiPayloadPkg: Add LINUXBOOT payload target

[Guo Dong]

Move Linuxboot.h under UefiPayloadPkg/Library/LbParseLib since this header file is only used by this library.
Update LbParseLib.c following https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/
e.g. 
+  UefiPayloadConfig* config;
+  int i;
Change to:
+  UefiPayloadConfig   *Config;
+  UINTN                         Index;

And split below code into multiple lines for better readable.
+// Align the address and add memory rang to MemInfoCallback void 
+AddMemoryRange(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN UINTN start,
+                    IN UINTN end, IN int type) {

Update OpenSsl submodule in a separate patch if required.

Thanks,
Guo
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.groups.io
Cc: Cheng-Chieh Huang <chengchieh@google.com>
Subject: [edk2-devel] [PATCH v1 1/6] UefiPayloadPkg: Add LINUXBOOT payload target

Initial commit to support linuxboot payload.

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
 UefiPayloadPkg/UefiPayloadPkg.dsc                              |  16 +-
 UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf               |  39 +++++
 UefiPayloadPkg/Include/Linuxboot.h                             |  47 ++++++
 UefiPayloadPkg/Library/LbParseLib/LbParseLib.c                 | 168 ++++++++++++++++++++
 UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c |   6 +-
 CryptoPkg/Library/OpensslLib/openssl                           |   2 +-
 6 files changed, 270 insertions(+), 8 deletions(-)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index bcedf1c746b4..54576ba485b7 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -33,6 +33,7 @@ [Defines]
   #
   # SBL:      UEFI payload for Slim Bootloader
   # COREBOOT: UEFI payload for coreboot
+  # LINUXBOOT: UEFI payload for linuxboot
   #
   DEFINE   BOOTLOADER = SBL
 
@@ -93,6 +94,9 @@ [Defines]
 
 [BuildOptions]
   *_*_*_CC_FLAGS                 = -D DISABLE_NEW_DEPRECATED_INTERFACES
+!if $(BOOTLOADER) == "LINUXBOOT"
+  *_*_*_CC_FLAGS                 = -D LINUXBOOT_PAYLOAD
+!endif
   GCC:*_UNIXGCC_*_CC_FLAGS       = -DMDEPKG_NDEBUG
   GCC:RELEASE_*_*_CC_FLAGS       = -DMDEPKG_NDEBUG
   INTEL:RELEASE_*_*_CC_FLAGS     = /D MDEPKG_NDEBUG
@@ -222,11 +226,13 @@ [LibraryClasses]
 !endif
   PlatformSupportLib|UefiPayloadPkg/Library/PlatformSupportLibNull/PlatformSupportLibNull.inf
 !if $(UNIVERSAL_PAYLOAD) == FALSE
-  !if $(BOOTLOADER) == "COREBOOT"
-    BlParseLib|UefiPayloadPkg/Library/CbParseLib/CbParseLib.inf
-  !else
-    BlParseLib|UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
-  !endif
+ !if $(BOOTLOADER) == "COREBOOT"
+   BlParseLib|UefiPayloadPkg/Library/CbParseLib/CbParseLib.inf
+ !elseif $(BOOTLOADER) == "LINUXBOOT"
+   BlParseLib|UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf
+ !else
+   BlParseLib|UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
+ !endif
 !endif
 
   DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
diff --git a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf
new file mode 100644
index 000000000000..d75ba8db8cf3
--- /dev/null
+++ b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf
@@ -0,0 +1,39 @@
+## @file
+#  Linuxboot Table Parse Library.
+#
+#  Copyright (c) 2021, the u-root Authors. All rights reserved.<BR> #  
+SPDX-License-Identifier: BSD-2-Clause-Patent # ##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = LbParseLib
+  FILE_GUID                      = DBA15E1E-4C16-47DF-93C0-AB5888ED14C3
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = BlParseLib
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64
+#
+
+[Sources]
+  LbParseLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  UefiPayloadPkg/UefiPayloadPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  IoLib
+  DebugLib
+  PcdLib
+
+[Pcd]
+  gUefiPayloadPkgTokenSpaceGuid.PcdPayloadFdMemBase
diff --git a/UefiPayloadPkg/Include/Linuxboot.h b/UefiPayloadPkg/Include/Linuxboot.h
new file mode 100644
index 000000000000..34ca18069983
--- /dev/null
+++ b/UefiPayloadPkg/Include/Linuxboot.h
@@ -0,0 +1,47 @@
+/** @file
+  LinuxBoot PEI module include file.
+**/
+#ifndef _LINUXBOOT_PEI_H_INCLUDED_
+#define _LINUXBOOT_PEI_H_INCLUDED_
+
+#if defined(_MSC_VER)
+#pragma warning(disable : 4200)
+#endif
+
+#pragma pack(1)
+typedef struct SerialPortConfigStruct {
+  UINT32 Type;
+  UINT32 BaseAddr;
+  UINT32 Baud;
+  UINT32 RegWidth;
+  UINT32 InputHertz;
+  UINT32 UartPciAddr;
+} SerialPortConfig;
+
+typedef struct MemoryMapEntryStruct {
+  UINT64 Start;
+  UINT64 End;
+  UINT32 Type;
+} MemoryMapEntry;
+
+typedef struct UefiPayloadConfigStruct {
+  UINT64 Version;
+  UINT64 AcpiBase;
+  UINT64 AcpiSize;
+  UINT64 SmbiosBase;
+  UINT64 SmbiosSize;
+  SerialPortConfig SerialConfig;
+  UINT32 NumMemoryMapEntries;
+  MemoryMapEntry MemoryMapEntries[0];
+} UefiPayloadConfig;
+#pragma pack()
+
+#define UEFI_PAYLOAD_CONFIG_VERSION 1
+
+#define LINUXBOOT_MEM_RAM 1
+#define LINUXBOOT_MEM_DEFAULT 2
+#define LINUXBOOT_MEM_ACPI 3
+#define LINUXBOOT_MEM_NVS 4
+#define LINUXBOOT_MEM_RESERVED 5
+
+#endif  // _LINUXBOOT_PEI_H_INCLUDED_
diff --git a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
new file mode 100644
index 000000000000..34bfb6a1073f
--- /dev/null
+++ b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
@@ -0,0 +1,168 @@
+/** @file
+  This library will parse the linuxboot table in memory and extract 
+those required
+  information.
+
+  Copyright (c) 2021, the u-root Authors. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+
+#include <IndustryStandard/Acpi.h>
+#include <IndustryStandard/SmBios.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/BlParseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/IoLib.h>
+#include <Library/PcdLib.h>
+#include <Linuxboot.h>
+#include <Uefi/UefiBaseType.h>
+
+// Retrieve UefiPayloadConfig from Linuxboot's uefiboot
+UefiPayloadConfig* GetUefiPayLoadConfig() {
+  UefiPayloadConfig* config =
+      (UefiPayloadConfig*)(UINTN)(PcdGet32(PcdPayloadFdMemBase) - 
+SIZE_64KB);
+  if (config->Version != UEFI_PAYLOAD_CONFIG_VERSION) {
+    DEBUG((DEBUG_ERROR, "Expect payload config version: %d, but get %d\n",
+           UEFI_PAYLOAD_CONFIG_VERSION, config->Version));
+    CpuDeadLoop ();
+  }
+  return config;
+}
+
+// Align the address and add memory rang to MemInfoCallback void 
+AddMemoryRange(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN UINTN start,
+                    IN UINTN end, IN int type) {
+  MEMROY_MAP_ENTRY MemoryMap;
+  UINTN AlignedStart;
+  UINTN AlignedEnd;
+  AlignedStart = ALIGN_VALUE(start, SIZE_4KB);
+  AlignedEnd = ALIGN_VALUE(end, SIZE_4KB);
+  // Conservative adjustment on Memory map. This should happen when 
+booting from
+  // non UEFI bios and it may report a memory region less than 4KB.
+  if (AlignedStart > start && type != LINUXBOOT_MEM_RAM) {
+    AlignedStart -= SIZE_4KB;
+  }
+  if (AlignedEnd > end + 1 && type == LINUXBOOT_MEM_RAM) {
+    AlignedEnd -= SIZE_4KB;
+  }
+  MemoryMap.Base = AlignedStart;
+  MemoryMap.Size = AlignedEnd - AlignedStart;
+  MemoryMap.Type = type;
+  MemoryMap.Flag = 0;
+  MemInfoCallback(&MemoryMap, NULL);
+}
+
+/**
+  Acquire the memory information from the linuxboot table in memory.
+
+  @param  MemInfoCallback     The callback routine
+  @param  Params              Pointer to the callback routine parameter
+
+  @retval RETURN_SUCCESS     Successfully find out the memory information.
+  @retval RETURN_NOT_FOUND   Failed to find the memory information.
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseMemoryInfo(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN VOID* 
+Params) {
+  UefiPayloadConfig* config;
+  int i;
+
+  config = GetUefiPayLoadConfig();
+
+  DEBUG((DEBUG_INFO, "MemoryMap #entries: %d\n", 
+ config->NumMemoryMapEntries));
+
+  MemoryMapEntry* entry = &config->MemoryMapEntries[0];
+  for (i = 0; i < config->NumMemoryMapEntries; i++) {
+    DEBUG((DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n", entry->Start,
+           entry->End, entry->Type));
+    AddMemoryRange(MemInfoCallback, entry->Start, entry->End, entry->Type);
+    entry++;
+  }
+  return RETURN_SUCCESS;
+}
+
+/**
+  Acquire acpi table and smbios table from linuxboot
+
+  @param  SystemTableInfo          Pointer to the system table info
+
+  @retval RETURN_SUCCESS            Successfully find out the tables.
+  @retval RETURN_NOT_FOUND          Failed to find the tables.
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseSystemTable(OUT SYSTEM_TABLE_INFO* SystemTableInfo) {
+  UefiPayloadConfig* config;
+
+  config = GetUefiPayLoadConfig();
+  SystemTableInfo->AcpiTableBase = config->AcpiBase;  
+ SystemTableInfo->AcpiTableSize = config->AcpiSize;
+
+  SystemTableInfo->SmbiosTableBase = config->SmbiosBase;  
+ SystemTableInfo->SmbiosTableSize = config->SmbiosSize;
+
+  return RETURN_SUCCESS;
+}
+
+/**
+  Find the serial port information
+
+  @param  SERIAL_PORT_INFO   Pointer to serial port info structure
+
+  @retval RETURN_SUCCESS     Successfully find the serial port information.
+  @retval RETURN_NOT_FOUND   Failed to find the serial port information .
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseSerialInfo(OUT SERIAL_PORT_INFO* SerialPortInfo) {
+  UefiPayloadConfig* config;
+  config = GetUefiPayLoadConfig();
+
+  SerialPortInfo->BaseAddr = config->SerialConfig.BaseAddr;  
+ SerialPortInfo->RegWidth = config->SerialConfig.RegWidth;  
+ SerialPortInfo->Type = config->SerialConfig.Type;  
+ SerialPortInfo->Baud = config->SerialConfig.Baud;  
+ SerialPortInfo->InputHertz = config->SerialConfig.InputHertz;  
+ SerialPortInfo->UartPciAddr = config->SerialConfig.UartPciAddr;
+
+  return RETURN_SUCCESS;
+}
+
+/**
+  Find the video frame buffer information
+
+  @param  GfxInfo             Pointer to the EFI_PEI_GRAPHICS_INFO_HOB structure
+
+  @retval RETURN_SUCCESS     Successfully find the video frame buffer
+information.
+  @retval RETURN_NOT_FOUND   Failed to find the video frame buffer information .
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseGfxInfo(OUT EFI_PEI_GRAPHICS_INFO_HOB* GfxInfo) {
+  // Not supported
+  return RETURN_NOT_FOUND;
+}
+
+/**
+  Find the video frame buffer device information
+
+  @param  GfxDeviceInfo      Pointer to the EFI_PEI_GRAPHICS_DEVICE_INFO_HOB
+structure
+
+  @retval RETURN_SUCCESS     Successfully find the video frame buffer
+information.
+  @retval RETURN_NOT_FOUND   Failed to find the video frame buffer information.
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseGfxDeviceInfo(OUT EFI_PEI_GRAPHICS_DEVICE_INFO_HOB* GfxDeviceInfo) 
+{
+  return RETURN_NOT_FOUND;
+}
diff --git a/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c b/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
index b0268f05069c..a4f714f765ea 100644
--- a/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
+++ b/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
@@ -40,8 +40,9 @@ AdjustRootBridgeResource (
   IN  PCI_ROOT_BRIDGE_APERTURE *PMemAbove4G
 )
 {
+#ifndef LINUXBOOT_PAYLOAD
   UINT64  Mask;
-
+#endif
   //
   // For now try to downgrade everything into MEM32 since
   // - coreboot does not assign resource above 4GB @@ -80,7 +81,7 @@ AdjustRootBridgeResource (
     PMemAbove4G->Base  = MAX_UINT64;
     PMemAbove4G->Limit = 0;
   }
-
+#ifndef LINUXBOOT_PAYLOAD
   //
   // Align IO  resource at 4K  boundary
   //
@@ -98,6 +99,7 @@ AdjustRootBridgeResource (
   if (Mem->Base != MAX_UINT64) {
     Mem->Base &= ~Mask;
   }
+#endif
 }
 
 /**
diff --git a/CryptoPkg/Library/OpensslLib/openssl b/CryptoPkg/Library/OpensslLib/openssl
index 52c587d60be6..e2e09d9fba11 160000
--- a/CryptoPkg/Library/OpensslLib/openssl
+++ b/CryptoPkg/Library/OpensslLib/openssl
@@ -1 +1 @@
-Subproject commit 52c587d60be67c337364b830dd3fdc15404a2f04
+Subproject commit e2e09d9fba1187f8d6aafaa34d4172f56f1ffb72
--
2.32.0.402.g57bb445576-goog

[PATCH v1 2/6] UefiPayloadPkg: Use legacy timer in Linuxboot payload

[Cheng-Chieh Huang]

HPET timer may fail to init after prior linux taking over.

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
 UefiPayloadPkg/UefiPayloadPkg.dsc | 6 ++++++
 UefiPayloadPkg/UefiPayloadPkg.fdf | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 54576ba485b7..e56e6f4a5379 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -438,7 +438,13 @@ [Components.X64]
       NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
   }
 
+!if $(BOOTLOADER) == "LINUXBOOT"
+  OvmfPkg/8254TimerDxe/8254Timer.inf
+  OvmfPkg/8259InterruptControllerDxe/8259.inf
+!else
   PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
+!endif
+
   MdeModulePkg/Universal/Metronome/Metronome.inf
   MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
   MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
diff --git a/UefiPayloadPkg/UefiPayloadPkg.fdf b/UefiPayloadPkg/UefiPayloadPkg.fdf
index 041fed842cd8..f57a8b4bf3d3 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.fdf
+++ b/UefiPayloadPkg/UefiPayloadPkg.fdf
@@ -101,7 +101,12 @@ [FV.DXEFV]
 INF UefiCpuPkg/CpuDxe/CpuDxe.inf
 INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
 INF MdeModulePkg/Application/UiApp/UiApp.inf
+!if $(BOOTLOADER) != "LINUXBOOT"
 INF PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
+!else
+INF OvmfPkg/8254TimerDxe/8254Timer.inf
+INF OvmfPkg/8259InterruptControllerDxe/8259.inf
+!endif
 INF MdeModulePkg/Universal/Metronome/Metronome.inf
 INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
 INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
-- 
2.32.0.402.g57bb445576-goog

Re: [edk2-devel] [PATCH v1 2/6] UefiPayloadPkg: Use legacy timer in Linuxboot payload

[Guo Dong]

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

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.groups.io
Cc: Cheng-Chieh Huang <chengchieh@google.com>
Subject: [edk2-devel] [PATCH v1 2/6] UefiPayloadPkg: Use legacy timer in Linuxboot payload

HPET timer may fail to init after prior linux taking over.

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
 UefiPayloadPkg/UefiPayloadPkg.dsc | 6 ++++++  UefiPayloadPkg/UefiPayloadPkg.fdf | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 54576ba485b7..e56e6f4a5379 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -438,7 +438,13 @@ [Components.X64]
       NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
   }
 
+!if $(BOOTLOADER) == "LINUXBOOT"
+  OvmfPkg/8254TimerDxe/8254Timer.inf
+  OvmfPkg/8259InterruptControllerDxe/8259.inf
+!else
   PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
+!endif
+
   MdeModulePkg/Universal/Metronome/Metronome.inf
   MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
   MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
diff --git a/UefiPayloadPkg/UefiPayloadPkg.fdf b/UefiPayloadPkg/UefiPayloadPkg.fdf
index 041fed842cd8..f57a8b4bf3d3 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.fdf
+++ b/UefiPayloadPkg/UefiPayloadPkg.fdf
@@ -101,7 +101,12 @@ [FV.DXEFV]
 INF UefiCpuPkg/CpuDxe/CpuDxe.inf
 INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
 INF MdeModulePkg/Application/UiApp/UiApp.inf
+!if $(BOOTLOADER) != "LINUXBOOT"
 INF PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
+!else
+INF OvmfPkg/8254TimerDxe/8254Timer.inf
+INF OvmfPkg/8259InterruptControllerDxe/8259.inf
+!endif
 INF MdeModulePkg/Universal/Metronome/Metronome.inf
 INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
 INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
--
2.32.0.402.g57bb445576-goog

Re: [edk2-devel] [PATCH v1 2/6] UefiPayloadPkg: Use legacy timer in Linuxboot payload

[Guo Dong]

A typo copy, should be:
Reviewed-by: Guo Dong <guo.dong@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Guo Dong
Sent: Tuesday, August 3, 2021 7:22 PM
To: devel@edk2.groups.io; chengchieh@google.com
Subject: Re: [edk2-devel] [PATCH v1 2/6] UefiPayloadPkg: Use legacy timer in Linuxboot payload


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

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.groups.io
Cc: Cheng-Chieh Huang <chengchieh@google.com>
Subject: [edk2-devel] [PATCH v1 2/6] UefiPayloadPkg: Use legacy timer in Linuxboot payload

HPET timer may fail to init after prior linux taking over.

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
 UefiPayloadPkg/UefiPayloadPkg.dsc | 6 ++++++  UefiPayloadPkg/UefiPayloadPkg.fdf | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 54576ba485b7..e56e6f4a5379 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -438,7 +438,13 @@ [Components.X64]
       NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
   }
 
+!if $(BOOTLOADER) == "LINUXBOOT"
+  OvmfPkg/8254TimerDxe/8254Timer.inf
+  OvmfPkg/8259InterruptControllerDxe/8259.inf
+!else
   PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
+!endif
+
   MdeModulePkg/Universal/Metronome/Metronome.inf
   MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
   MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
diff --git a/UefiPayloadPkg/UefiPayloadPkg.fdf b/UefiPayloadPkg/UefiPayloadPkg.fdf
index 041fed842cd8..f57a8b4bf3d3 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.fdf
+++ b/UefiPayloadPkg/UefiPayloadPkg.fdf
@@ -101,7 +101,12 @@ [FV.DXEFV]
 INF UefiCpuPkg/CpuDxe/CpuDxe.inf
 INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
 INF MdeModulePkg/Application/UiApp/UiApp.inf
+!if $(BOOTLOADER) != "LINUXBOOT"
 INF PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
+!else
+INF OvmfPkg/8254TimerDxe/8254Timer.inf
+INF OvmfPkg/8259InterruptControllerDxe/8259.inf
+!endif
 INF MdeModulePkg/Universal/Metronome/Metronome.inf
 INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
 INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
--
2.32.0.402.g57bb445576-goog

[PATCH v1 3/6] UefiPayloadPkg: Update maximum logic processor to 256

[Cheng-Chieh Huang]

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
 UefiPayloadPkg/UefiPayloadPkg.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index e56e6f4a5379..8aa5f18cd35c 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -40,7 +40,7 @@ [Defines]
   #
   # CPU options
   #
-  DEFINE MAX_LOGICAL_PROCESSORS       = 64
+  DEFINE MAX_LOGICAL_PROCESSORS       = 256
 
   #
   # PCI options
-- 
2.32.0.402.g57bb445576-goog

Re: [edk2-devel] [PATCH v1 3/6] UefiPayloadPkg: Update maximum logic processor to 256

[Guo Dong]

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

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.groups.io
Cc: Cheng-Chieh Huang <chengchieh@google.com>
Subject: [edk2-devel] [PATCH v1 3/6] UefiPayloadPkg: Update maximum logic processor to 256

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
 UefiPayloadPkg/UefiPayloadPkg.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index e56e6f4a5379..8aa5f18cd35c 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -40,7 +40,7 @@ [Defines]
   #
   # CPU options
   #
-  DEFINE MAX_LOGICAL_PROCESSORS       = 64
+  DEFINE MAX_LOGICAL_PROCESSORS       = 256
 
   #
   # PCI options
-- 
2.32.0.402.g57bb445576-goog

Re: [edk2-devel] [PATCH v1 3/6] UefiPayloadPkg: Update maximum logic processor to 256

[Guo Dong]

A typo copy, should be:
Reviewed-by: Guo Dong <guo.dong@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Guo Dong
Sent: Tuesday, August 3, 2021 7:22 PM
To: devel@edk2.groups.io; chengchieh@google.com
Subject: Re: [edk2-devel] [PATCH v1 3/6] UefiPayloadPkg: Update maximum logic processor to 256


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

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.groups.io
Cc: Cheng-Chieh Huang <chengchieh@google.com>
Subject: [edk2-devel] [PATCH v1 3/6] UefiPayloadPkg: Update maximum logic processor to 256

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
 UefiPayloadPkg/UefiPayloadPkg.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index e56e6f4a5379..8aa5f18cd35c 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -40,7 +40,7 @@ [Defines]
   #
   # CPU options
   #
-  DEFINE MAX_LOGICAL_PROCESSORS       = 64
+  DEFINE MAX_LOGICAL_PROCESSORS       = 256
 
   #
   # PCI options
-- 
2.32.0.402.g57bb445576-goog

[PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data

[Cheng-Chieh Huang]

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
 UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index ae16f25c7c0e..70afbf83ed4a 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -517,6 +517,8 @@ BuildGenericHob (
 
   // The UEFI payload FV
   BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase), PcdGet32 (PcdPayloadFdMemSize), EfiBootServicesData);
+  // The UEFI payload config FV
+  BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase) - SIZE_64KB, SIZE_64KB, EfiRuntimeServicesData);
 
   //
   // Build CPU memory space and IO space hob
-- 
2.32.0.402.g57bb445576-goog

Re: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data

[Guo Dong]

why build runtime memory allocation hob here? 
The PayloadEntry should already got the information and build a new HOB list to DXE core, will anyone access these region late?
If yes, maybe you need add LINUXBOOT_PAYLOAD flag for this code, and update commit message on this.

Thanks,
Guo

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.groups.io
Cc: Cheng-Chieh Huang <chengchieh@google.com>
Subject: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
 UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index ae16f25c7c0e..70afbf83ed4a 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -517,6 +517,8 @@ BuildGenericHob (
 
   // The UEFI payload FV
   BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase), PcdGet32 (PcdPayloadFdMemSize), EfiBootServicesData);
+  // The UEFI payload config FV
+  BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase) - SIZE_64KB, SIZE_64KB, EfiRuntimeServicesData);
 
   //
   // Build CPU memory space and IO space hob
-- 
2.32.0.402.g57bb445576-goog

Re: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data

[Cheng-Chieh Huang]

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

Hi Guo,

Thanks for the review. This is a quick fix for today's
PlatformHookLib design. PlatformHookLib (probably via
PlatformBootManagerLib) will be linked by BDS and potentially called again
in Windows bootloader. If we did not reserve this range of memory, system
will crash the system when it was called. This can be reproduced if we boot
Windows 2018. I also did not think reserving this memory til runtime is a
good idea, but I am not sure why we want to link PlatformHookLib to
PlatformBootManagerLib. I tried to remove PlatformHookLib in
PlatformBootManagerLib and it seems to work as well. maybe we should take
this approach. what do you think?

--
Cheng-Chieh

On Wed, Aug 4, 2021 at 10:44 AM Dong, Guo <guo.dong@intel.com> wrote:

>
> why build runtime memory allocation hob here?
> The PayloadEntry should already got the information and build a new HOB
> list to DXE core, will anyone access these region late?
> If yes, maybe you need add LINUXBOOT_PAYLOAD flag for this code, and
> update commit message on this.
>
> Thanks,
> Guo
>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Cheng-Chieh Huang via groups.io
> Sent: Wednesday, July 21, 2021 6:23 AM
> To: devel@edk2.groups.io
> Cc: Cheng-Chieh Huang <chengchieh@google.com>
> Subject: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload
> config in runtime services data
>
> Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
> ---
>  UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
> b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
> index ae16f25c7c0e..70afbf83ed4a 100644
> --- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
> +++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
> @@ -517,6 +517,8 @@ BuildGenericHob (
>
>    // The UEFI payload FV
>    BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase), PcdGet32
> (PcdPayloadFdMemSize), EfiBootServicesData);
> +  // The UEFI payload config FV
> +  BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase) - SIZE_64KB,
> SIZE_64KB, EfiRuntimeServicesData);
>
>    //
>    // Build CPU memory space and IO space hob
> --
> 2.32.0.402.g57bb445576-goog
>
>
>
> 
>
>
>

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

Re: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data

[Guo Dong]

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


Hi Cheng-Chieh,

I see. In order to use same PlatformHookLib to make DebugLib works in both PayloadEntry and DXE phase, the PlatformHookLib gets serial port info from bootloader.
In the DXE phase, UniversalPayloadPlatformHookLib will use HOB instead of getting info from bootloader. In the PayloadEntry module, we should build same serial port HOB so that DXE module could use UniversalPayloadPlatformHookLib Instead of PlatformHookLib.
For this patch, you could add LINUXBOOT_PAYLOAD flag with commit message update, I will revert this patch once above change is completed.

Thanks,
Guo

From: Cheng-Chieh Huang <chengchieh@google.com>
Sent: Tuesday, August 3, 2021 11:24 PM
To: Dong, Guo <guo.dong@intel.com>
Cc: devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data

Hi Guo,

Thanks for the review. This is a quick fix for today's PlatformHookLib design. PlatformHookLib (probably via PlatformBootManagerLib) will be linked by BDS and potentially called again in Windows bootloader. If we did not reserve this range of memory, system will crash the system when it was called. This can be reproduced if we boot Windows 2018. I also did not think reserving this memory til runtime is a good idea, but I am not sure why we want to link PlatformHookLib to PlatformBootManagerLib. I tried to remove PlatformHookLib in PlatformBootManagerLib and it seems to work as well. maybe we should take this approach. what do you think?

--
Cheng-Chieh

On Wed, Aug 4, 2021 at 10:44 AM Dong, Guo <guo.dong@intel.com<mailto:guo.dong@intel.com>> wrote:

why build runtime memory allocation hob here?
The PayloadEntry should already got the information and build a new HOB list to DXE core, will anyone access these region late?
If yes, maybe you need add LINUXBOOT_PAYLOAD flag for this code, and update commit message on this.

Thanks,
Guo

-----Original Message-----
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Cheng-Chieh Huang via groups.io<http://groups.io>
Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Cheng-Chieh Huang <chengchieh@google.com<mailto:chengchieh@google.com>>
Subject: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com<mailto:chengchieh@google.com>>
---
 UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index ae16f25c7c0e..70afbf83ed4a 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -517,6 +517,8 @@ BuildGenericHob (

   // The UEFI payload FV
   BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase), PcdGet32 (PcdPayloadFdMemSize), EfiBootServicesData);
+  // The UEFI payload config FV
+  BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase) - SIZE_64KB, SIZE_64KB, EfiRuntimeServicesData);

   //
   // Build CPU memory space and IO space hob
--
2.32.0.402.g57bb445576-goog






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

[PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

[Cheng-Chieh Huang]

This will allow we compile payload using gcc8

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
 UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 8aa5f18cd35c..fa41c5a24af5 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -30,6 +30,8 @@ [Defines]
   DEFINE PS2_KEYBOARD_ENABLE          = FALSE
   DEFINE UNIVERSAL_PAYLOAD            = FALSE
 
+  DEFINE DISABLE_MMX_SSE              = FALSE
+
   #
   # SBL:      UEFI payload for Slim Bootloader
   # COREBOOT: UEFI payload for coreboot
@@ -96,6 +98,9 @@ [BuildOptions]
   *_*_*_CC_FLAGS                 = -D DISABLE_NEW_DEPRECATED_INTERFACES
 !if $(BOOTLOADER) == "LINUXBOOT"
   *_*_*_CC_FLAGS                 = -D LINUXBOOT_PAYLOAD
+!endif
+!if $(DISABLE_MMX_SSE)
+  *_*_*_CC_FLAGS                 = -mno-mmx -mno-sse
 !endif
   GCC:*_UNIXGCC_*_CC_FLAGS       = -DMDEPKG_NDEBUG
   GCC:RELEASE_*_*_CC_FLAGS       = -DMDEPKG_NDEBUG
-- 
2.32.0.402.g57bb445576-goog

Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

[Michael D Kinney]

Are those flags needed for all packages that build with GCC?

Should this be moved into tools_def.txt?

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
> Sent: Wednesday, July 21, 2021 6:23 AM
> To: devel@edk2.groups.io
> Cc: Cheng-Chieh Huang <chengchieh@google.com>
> Subject: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation
> 
> This will allow we compile payload using gcc8
> 
> Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
> ---
>  UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
> index 8aa5f18cd35c..fa41c5a24af5 100644
> --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> @@ -30,6 +30,8 @@ [Defines]
>    DEFINE PS2_KEYBOARD_ENABLE          = FALSE
>    DEFINE UNIVERSAL_PAYLOAD            = FALSE
> 
> +  DEFINE DISABLE_MMX_SSE              = FALSE
> +
>    #
>    # SBL:      UEFI payload for Slim Bootloader
>    # COREBOOT: UEFI payload for coreboot
> @@ -96,6 +98,9 @@ [BuildOptions]
>    *_*_*_CC_FLAGS                 = -D DISABLE_NEW_DEPRECATED_INTERFACES
>  !if $(BOOTLOADER) == "LINUXBOOT"
>    *_*_*_CC_FLAGS                 = -D LINUXBOOT_PAYLOAD
> +!endif
> +!if $(DISABLE_MMX_SSE)
> +  *_*_*_CC_FLAGS                 = -mno-mmx -mno-sse
>  !endif
>    GCC:*_UNIXGCC_*_CC_FLAGS       = -DMDEPKG_NDEBUG
>    GCC:RELEASE_*_*_CC_FLAGS       = -DMDEPKG_NDEBUG
> --
> 2.32.0.402.g57bb445576-goog
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

[Cheng-Chieh Huang]

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

Yes, we can. I will drop this patch for this  uefipayload batch and send
another one for support DISABLE_GCC_MMX_SSE in tools_de.txt.

--
Cheng-Chieh

On Thu, Jul 22, 2021, 12:35 AM Kinney, Michael D <michael.d.kinney@intel.com>
wrote:

> Are those flags needed for all packages that build with GCC?
>
> Should this be moved into tools_def.txt?
>
> Mike
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Cheng-Chieh Huang via groups.io
> > Sent: Wednesday, July 21, 2021 6:23 AM
> > To: devel@edk2.groups.io
> > Cc: Cheng-Chieh Huang <chengchieh@google.com>
> > Subject: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE
> to avoid generating floating points operation
> >
> > This will allow we compile payload using gcc8
> >
> > Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
> > ---
> >  UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc
> b/UefiPayloadPkg/UefiPayloadPkg.dsc
> > index 8aa5f18cd35c..fa41c5a24af5 100644
> > --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> > +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> > @@ -30,6 +30,8 @@ [Defines]
> >    DEFINE PS2_KEYBOARD_ENABLE          = FALSE
> >    DEFINE UNIVERSAL_PAYLOAD            = FALSE
> >
> > +  DEFINE DISABLE_MMX_SSE              = FALSE
> > +
> >    #
> >    # SBL:      UEFI payload for Slim Bootloader
> >    # COREBOOT: UEFI payload for coreboot
> > @@ -96,6 +98,9 @@ [BuildOptions]
> >    *_*_*_CC_FLAGS                 = -D DISABLE_NEW_DEPRECATED_INTERFACES
> >  !if $(BOOTLOADER) == "LINUXBOOT"
> >    *_*_*_CC_FLAGS                 = -D LINUXBOOT_PAYLOAD
> > +!endif
> > +!if $(DISABLE_MMX_SSE)
> > +  *_*_*_CC_FLAGS                 = -mno-mmx -mno-sse
> >  !endif
> >    GCC:*_UNIXGCC_*_CC_FLAGS       = -DMDEPKG_NDEBUG
> >    GCC:RELEASE_*_*_CC_FLAGS       = -DMDEPKG_NDEBUG
> > --
> > 2.32.0.402.g57bb445576-goog
> >
> >
> >
> > 
> >
>
>

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

回复: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

[gaoliming]

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

Tools_def.txt doesn’t support build flag DISABLE_GCC_MMX_SSE. If this flag is moved into BaseTools\Conf\tools_def.template, -mno-mmx -mno-sse option will be the default GCC options. That means all platforms will apply them. 

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Cheng-Chieh Huang via groups.io
发送时间: 2021年7月22日 1:43
收件人: Kinney, Michael D <michael.d.kinney@intel.com>
抄送: devel@edk2.groups.io
主题: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

 

Yes, we can. I will drop this patch for this  uefipayload batch and send another one for support DISABLE_GCC_MMX_SSE in tools_de.txt.

 

--

Cheng-Chieh 

On Thu, Jul 22, 2021, 12:35 AM Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com> > wrote:

Are those flags needed for all packages that build with GCC?

Should this be moved into tools_def.txt?

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <mailto:devel@edk2.groups.io>  <devel@edk2.groups.io <mailto:devel@edk2.groups.io> > On Behalf Of Cheng-Chieh Huang via groups.io <http://groups.io> 
> Sent: Wednesday, July 21, 2021 6:23 AM
> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io> 
> Cc: Cheng-Chieh Huang <chengchieh@google.com <mailto:chengchieh@google.com> >
> Subject: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation
> 
> This will allow we compile payload using gcc8
> 
> Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com <mailto:chengchieh@google.com> >
> ---
>  UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
> index 8aa5f18cd35c..fa41c5a24af5 100644
> --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> @@ -30,6 +30,8 @@ [Defines]
>    DEFINE PS2_KEYBOARD_ENABLE          = FALSE
>    DEFINE UNIVERSAL_PAYLOAD            = FALSE
> 
> +  DEFINE DISABLE_MMX_SSE              = FALSE
> +
>    #
>    # SBL:      UEFI payload for Slim Bootloader
>    # COREBOOT: UEFI payload for coreboot
> @@ -96,6 +98,9 @@ [BuildOptions]
>    *_*_*_CC_FLAGS                 = -D DISABLE_NEW_DEPRECATED_INTERFACES
>  !if $(BOOTLOADER) == "LINUXBOOT"
>    *_*_*_CC_FLAGS                 = -D LINUXBOOT_PAYLOAD
> +!endif
> +!if $(DISABLE_MMX_SSE)
> +  *_*_*_CC_FLAGS                 = -mno-mmx -mno-sse
>  !endif
>    GCC:*_UNIXGCC_*_CC_FLAGS       = -DMDEPKG_NDEBUG
>    GCC:RELEASE_*_*_CC_FLAGS       = -DMDEPKG_NDEBUG
> --
> 2.32.0.402.g57bb445576-goog
> 
> 
> 
> 
> 




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

Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

[Cheng-Chieh Huang]

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

I mean, I will submit a patch to support DISABLE_GCC_MMX_SSE in tools_def.
What do you think?

--
Cheng-Chieh

On Thu, Jul 22, 2021 at 9:28 AM gaoliming <gaoliming@byosoft.com.cn> wrote:

> Tools_def.txt doesn’t support build flag DISABLE_GCC_MMX_SSE. If this
> flag is moved into BaseTools\Conf\tools_def.template, -mno-mmx -mno-sse
> option will be the default GCC options. That means all platforms will apply
> them.
>
>
>
> Thanks
>
> Liming
>
> *发件人:* devel@edk2.groups.io <devel@edk2.groups.io> *代表 *Cheng-Chieh Huang
> via groups.io
> *发送时间:* 2021年7月22日 1:43
> *收件人:* Kinney, Michael D <michael.d.kinney@intel.com>
> *抄送:* devel@edk2.groups.io
> *主题:* Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE
> to avoid generating floating points operation
>
>
>
> Yes, we can. I will drop this patch for this  uefipayload batch and send
> another one for support DISABLE_GCC_MMX_SSE in tools_de.txt.
>
>
>
> --
>
> Cheng-Chieh
>
> On Thu, Jul 22, 2021, 12:35 AM Kinney, Michael D <
> michael.d.kinney@intel.com> wrote:
>
> Are those flags needed for all packages that build with GCC?
>
> Should this be moved into tools_def.txt?
>
> Mike
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Cheng-Chieh Huang via groups.io
> > Sent: Wednesday, July 21, 2021 6:23 AM
> > To: devel@edk2.groups.io
> > Cc: Cheng-Chieh Huang <chengchieh@google.com>
> > Subject: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE
> to avoid generating floating points operation
> >
> > This will allow we compile payload using gcc8
> >
> > Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
> > ---
> >  UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc
> b/UefiPayloadPkg/UefiPayloadPkg.dsc
> > index 8aa5f18cd35c..fa41c5a24af5 100644
> > --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> > +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> > @@ -30,6 +30,8 @@ [Defines]
> >    DEFINE PS2_KEYBOARD_ENABLE          = FALSE
> >    DEFINE UNIVERSAL_PAYLOAD            = FALSE
> >
> > +  DEFINE DISABLE_MMX_SSE              = FALSE
> > +
> >    #
> >    # SBL:      UEFI payload for Slim Bootloader
> >    # COREBOOT: UEFI payload for coreboot
> > @@ -96,6 +98,9 @@ [BuildOptions]
> >    *_*_*_CC_FLAGS                 = -D DISABLE_NEW_DEPRECATED_INTERFACES
> >  !if $(BOOTLOADER) == "LINUXBOOT"
> >    *_*_*_CC_FLAGS                 = -D LINUXBOOT_PAYLOAD
> > +!endif
> > +!if $(DISABLE_MMX_SSE)
> > +  *_*_*_CC_FLAGS                 = -mno-mmx -mno-sse
> >  !endif
> >    GCC:*_UNIXGCC_*_CC_FLAGS       = -DMDEPKG_NDEBUG
> >    GCC:RELEASE_*_*_CC_FLAGS       = -DMDEPKG_NDEBUG
> > --
> > 2.32.0.402.g57bb445576-goog
> >
> >
> >
> >
> >
>
> 
>

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

回复: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

[gaoliming]

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

How do you add this support in tools_def? Can you give the proposal for it?

 

Thanks

Liming

发件人: Cheng-Chieh Huang <chengchieh@google.com> 
发送时间: 2021年7月22日 10:35
收件人: gaoliming <gaoliming@byosoft.com.cn>
抄送: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
主题: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

 

I mean, I will submit a patch to support DISABLE_GCC_MMX_SSE in tools_def. What do you think?

 

--

Cheng-Chieh

 

On Thu, Jul 22, 2021 at 9:28 AM gaoliming <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn> > wrote:

Tools_def.txt doesn’t support build flag DISABLE_GCC_MMX_SSE. If this flag is moved into BaseTools\Conf\tools_def.template, -mno-mmx -mno-sse option will be the default GCC options. That means all platforms will apply them. 

 

Thanks

Liming

发件人: devel@edk2.groups.io <mailto:devel@edk2.groups.io>  <devel@edk2.groups.io <mailto:devel@edk2.groups.io> > 代表 Cheng-Chieh Huang via groups.io <http://groups.io> 
发送时间: 2021年7月22日 1:43
收件人: Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com> >
抄送: devel@edk2.groups.io <mailto:devel@edk2.groups.io> 
主题: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

 

Yes, we can. I will drop this patch for this  uefipayload batch and send another one for support DISABLE_GCC_MMX_SSE in tools_de.txt.

 

--

Cheng-Chieh 

On Thu, Jul 22, 2021, 12:35 AM Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com> > wrote:

Are those flags needed for all packages that build with GCC?

Should this be moved into tools_def.txt?

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <mailto:devel@edk2.groups.io>  <devel@edk2.groups.io <mailto:devel@edk2.groups.io> > On Behalf Of Cheng-Chieh Huang via groups.io <http://groups.io> 
> Sent: Wednesday, July 21, 2021 6:23 AM
> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io> 
> Cc: Cheng-Chieh Huang <chengchieh@google.com <mailto:chengchieh@google.com> >
> Subject: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation
> 
> This will allow we compile payload using gcc8
> 
> Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com <mailto:chengchieh@google.com> >
> ---
>  UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
> index 8aa5f18cd35c..fa41c5a24af5 100644
> --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> @@ -30,6 +30,8 @@ [Defines]
>    DEFINE PS2_KEYBOARD_ENABLE          = FALSE
>    DEFINE UNIVERSAL_PAYLOAD            = FALSE
> 
> +  DEFINE DISABLE_MMX_SSE              = FALSE
> +
>    #
>    # SBL:      UEFI payload for Slim Bootloader
>    # COREBOOT: UEFI payload for coreboot
> @@ -96,6 +98,9 @@ [BuildOptions]
>    *_*_*_CC_FLAGS                 = -D DISABLE_NEW_DEPRECATED_INTERFACES
>  !if $(BOOTLOADER) == "LINUXBOOT"
>    *_*_*_CC_FLAGS                 = -D LINUXBOOT_PAYLOAD
> +!endif
> +!if $(DISABLE_MMX_SSE)
> +  *_*_*_CC_FLAGS                 = -mno-mmx -mno-sse
>  !endif
>    GCC:*_UNIXGCC_*_CC_FLAGS       = -DMDEPKG_NDEBUG
>    GCC:RELEASE_*_*_CC_FLAGS       = -DMDEPKG_NDEBUG
> --
> 2.32.0.402.g57bb445576-goog
> 
> 
> 
> 
> 




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

Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

[Cheng-Chieh Huang]

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

I got what you mean by "Tools_def.txt doesn’t support build flag" now. I
thought the parser does support some sort of if/else condition, but it
seems to be not the case. Alternatively, we can do this during the copy
(from template to tools_def.txt), but everytime we need to recreate
tools_def.txt which is not ideal).

In that case, I think we should just roll back to only doing this in
UefiPayloadPkg.dsc. If other packs want to add these flags, they will need
to do it on their own.

On Fri, Jul 23, 2021 at 6:29 PM gaoliming <gaoliming@byosoft.com.cn> wrote:

> How do you add this support in tools_def? Can you give the proposal for it?
>
>
>
> Thanks
>
> Liming
>
> *发件人:* Cheng-Chieh Huang <chengchieh@google.com>
> *发送时间:* 2021年7月22日 10:35
> *收件人:* gaoliming <gaoliming@byosoft.com.cn>
> *抄送:* devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> *主题:* Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE
> to avoid generating floating points operation
>
>
>
> I mean, I will submit a patch to support DISABLE_GCC_MMX_SSE in tools_def.
> What do you think?
>
>
>
> --
>
> Cheng-Chieh
>
>
>
> On Thu, Jul 22, 2021 at 9:28 AM gaoliming <gaoliming@byosoft.com.cn>
> wrote:
>
> Tools_def.txt doesn’t support build flag DISABLE_GCC_MMX_SSE. If this
> flag is moved into BaseTools\Conf\tools_def.template, -mno-mmx -mno-sse
> option will be the default GCC options. That means all platforms will apply
> them.
>
>
>
> Thanks
>
> Liming
>
> *发件人:* devel@edk2.groups.io <devel@edk2.groups.io> *代表 *Cheng-Chieh Huang
> via groups.io
> *发送时间:* 2021年7月22日 1:43
> *收件人:* Kinney, Michael D <michael.d.kinney@intel.com>
> *抄送:* devel@edk2.groups.io
> *主题:* Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE
> to avoid generating floating points operation
>
>
>
> Yes, we can. I will drop this patch for this  uefipayload batch and send
> another one for support DISABLE_GCC_MMX_SSE in tools_de.txt.
>
>
>
> --
>
> Cheng-Chieh
>
> On Thu, Jul 22, 2021, 12:35 AM Kinney, Michael D <
> michael.d.kinney@intel.com> wrote:
>
> Are those flags needed for all packages that build with GCC?
>
> Should this be moved into tools_def.txt?
>
> Mike
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Cheng-Chieh Huang via groups.io
> > Sent: Wednesday, July 21, 2021 6:23 AM
> > To: devel@edk2.groups.io
> > Cc: Cheng-Chieh Huang <chengchieh@google.com>
> > Subject: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE
> to avoid generating floating points operation
> >
> > This will allow we compile payload using gcc8
> >
> > Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
> > ---
> >  UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc
> b/UefiPayloadPkg/UefiPayloadPkg.dsc
> > index 8aa5f18cd35c..fa41c5a24af5 100644
> > --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> > +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> > @@ -30,6 +30,8 @@ [Defines]
> >    DEFINE PS2_KEYBOARD_ENABLE          = FALSE
> >    DEFINE UNIVERSAL_PAYLOAD            = FALSE
> >
> > +  DEFINE DISABLE_MMX_SSE              = FALSE
> > +
> >    #
> >    # SBL:      UEFI payload for Slim Bootloader
> >    # COREBOOT: UEFI payload for coreboot
> > @@ -96,6 +98,9 @@ [BuildOptions]
> >    *_*_*_CC_FLAGS                 = -D DISABLE_NEW_DEPRECATED_INTERFACES
> >  !if $(BOOTLOADER) == "LINUXBOOT"
> >    *_*_*_CC_FLAGS                 = -D LINUXBOOT_PAYLOAD
> > +!endif
> > +!if $(DISABLE_MMX_SSE)
> > +  *_*_*_CC_FLAGS                 = -mno-mmx -mno-sse
> >  !endif
> >    GCC:*_UNIXGCC_*_CC_FLAGS       = -DMDEPKG_NDEBUG
> >    GCC:RELEASE_*_*_CC_FLAGS       = -DMDEPKG_NDEBUG
> > --
> > 2.32.0.402.g57bb445576-goog
> >
> >
> >
> >
> >
>
> 
>
>

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

Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

[Michael D Kinney]

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

Can you provide details on why this flag was needed in the UefiPayloadPkg use case?

If the newer versions of the GCC compilers are injecting use of MMX/SSE instructions from optimization of C code, then we need to unconditionally disable that usage in tools_def.txt.  The only place MMX/SSE instructions can be used is in NASM files with proper save/restore of the MMX/SSE state.

Here is an example with use of xmm0 that is saved/restored on the stack.

https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseMemoryLibSse2/X64/CopyMem.nasm

Mike

From: Cheng-Chieh Huang <chengchieh@google.com>
Sent: Friday, July 23, 2021 4:17 AM
To: gaoliming <gaoliming@byosoft.com.cn>
Cc: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

I got what you mean by "Tools_def.txt doesn’t support build flag" now. I thought the parser does support some sort of if/else condition, but it seems to be not the case. Alternatively, we can do this during the copy (from template to tools_def.txt), but everytime we need to recreate tools_def.txt which is not ideal).

In that case, I think we should just roll back to only doing this in UefiPayloadPkg.dsc. If other packs want to add these flags, they will need to do it on their own.

On Fri, Jul 23, 2021 at 6:29 PM gaoliming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>> wrote:
How do you add this support in tools_def? Can you give the proposal for it?

Thanks
Liming
发件人: Cheng-Chieh Huang <chengchieh@google.com<mailto:chengchieh@google.com>>
发送时间: 2021年7月22日 10:35
收件人: gaoliming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
抄送: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
主题: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

I mean, I will submit a patch to support DISABLE_GCC_MMX_SSE in tools_def. What do you think?

--
Cheng-Chieh

On Thu, Jul 22, 2021 at 9:28 AM gaoliming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>> wrote:
Tools_def.txt doesn’t support build flag DISABLE_GCC_MMX_SSE. If this flag is moved into BaseTools\Conf\tools_def.template, -mno-mmx -mno-sse option will be the default GCC options. That means all platforms will apply them.

Thanks
Liming
发件人: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> 代表 Cheng-Chieh Huang via groups.io<http://groups.io>
发送时间: 2021年7月22日 1:43
收件人: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
抄送: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
主题: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

Yes, we can. I will drop this patch for this  uefipayload batch and send another one for support DISABLE_GCC_MMX_SSE in tools_de.txt.

--
Cheng-Chieh
On Thu, Jul 22, 2021, 12:35 AM Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:
Are those flags needed for all packages that build with GCC?

Should this be moved into tools_def.txt?

Mike

> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Cheng-Chieh Huang via groups.io<http://groups.io>
> Sent: Wednesday, July 21, 2021 6:23 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Cheng-Chieh Huang <chengchieh@google.com<mailto:chengchieh@google.com>>
> Subject: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation
>
> This will allow we compile payload using gcc8
>
> Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com<mailto:chengchieh@google.com>>
> ---
>  UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
> index 8aa5f18cd35c..fa41c5a24af5 100644
> --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> @@ -30,6 +30,8 @@ [Defines]
>    DEFINE PS2_KEYBOARD_ENABLE          = FALSE
>    DEFINE UNIVERSAL_PAYLOAD            = FALSE
>
> +  DEFINE DISABLE_MMX_SSE              = FALSE
> +
>    #
>    # SBL:      UEFI payload for Slim Bootloader
>    # COREBOOT: UEFI payload for coreboot
> @@ -96,6 +98,9 @@ [BuildOptions]
>    *_*_*_CC_FLAGS                 = -D DISABLE_NEW_DEPRECATED_INTERFACES
>  !if $(BOOTLOADER) == "LINUXBOOT"
>    *_*_*_CC_FLAGS                 = -D LINUXBOOT_PAYLOAD
> +!endif
> +!if $(DISABLE_MMX_SSE)
> +  *_*_*_CC_FLAGS                 = -mno-mmx -mno-sse
>  !endif
>    GCC:*_UNIXGCC_*_CC_FLAGS       = -DMDEPKG_NDEBUG
>    GCC:RELEASE_*_*_CC_FLAGS       = -DMDEPKG_NDEBUG
> --
> 2.32.0.402.g57bb445576-goog
>
>
>
>
>


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

Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

[Cheng-Chieh Huang]

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

Hi,

This is not about UefiPayload. I did not have GCC5 in my environment, so I
use a newer GCC to compile. However, in newer gcc (let's say GCC8), it
generates floating point instructions in each function and system will
crash as you mentioned that we did not handle it in UEFI.

I uploaded this patch because many people from our group use newer GCC and
it will be more convenient if we can disable it with a flag (otherwise, we
need to maintain a local patch). I guess the real fix (bigger work) is for
EDK2 to support the newer GCC version in its toolchain (create something
like *_GCC8_*).

Back to the question, I think it should be no harm to add these flags to
tool_defs.txt if EDK2 did not expect the compiler to generate MMX/SSE
instructions in the first place.

--
Cheng-Chieh

On Sat, Jul 24, 2021 at 12:46 AM Kinney, Michael D <
michael.d.kinney@intel.com> wrote:

> Can you provide details on why this flag was needed in the UefiPayloadPkg
> use case?
>
>
>
> If the newer versions of the GCC compilers are injecting use of MMX/SSE
> instructions from optimization of C code, then we need to unconditionally
> disable that usage in tools_def.txt.  The only place MMX/SSE instructions
> can be used is in NASM files with proper save/restore of the MMX/SSE state.
>
>
>
> Here is an example with use of xmm0 that is saved/restored on the stack.
>
>
>
>
> https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseMemoryLibSse2/X64/CopyMem.nasm
>
>
>
> Mike
>
>
>
> *From:* Cheng-Chieh Huang <chengchieh@google.com>
> *Sent:* Friday, July 23, 2021 4:17 AM
> *To:* gaoliming <gaoliming@byosoft.com.cn>
> *Cc:* devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> *Subject:* Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add
> DISABLE_MMX_SSE to avoid generating floating points operation
>
>
>
> I got what you mean by "Tools_def.txt doesn’t support build flag" now. I
> thought the parser does support some sort of if/else condition, but it
> seems to be not the case. Alternatively, we can do this during the copy
> (from template to tools_def.txt), but everytime we need to recreate
> tools_def.txt which is not ideal).
>
>
>
> In that case, I think we should just roll back to only doing this in
> UefiPayloadPkg.dsc. If other packs want to add these flags, they will need
> to do it on their own.
>
>
>
> On Fri, Jul 23, 2021 at 6:29 PM gaoliming <gaoliming@byosoft.com.cn>
> wrote:
>
> How do you add this support in tools_def? Can you give the proposal for it?
>
>
>
> Thanks
>
> Liming
>
> *发件人**:* Cheng-Chieh Huang <chengchieh@google.com>
> *发送时间:* 2021年7月22日 10:35
> *收件人:* gaoliming <gaoliming@byosoft.com.cn>
> *抄送:* devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> *主题:* Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE
> to avoid generating floating points operation
>
>
>
> I mean, I will submit a patch to support DISABLE_GCC_MMX_SSE in tools_def.
> What do you think?
>
>
>
> --
>
> Cheng-Chieh
>
>
>
> On Thu, Jul 22, 2021 at 9:28 AM gaoliming <gaoliming@byosoft.com.cn>
> wrote:
>
> Tools_def.txt doesn’t support build flag DISABLE_GCC_MMX_SSE. If this
> flag is moved into BaseTools\Conf\tools_def.template, -mno-mmx -mno-sse
> option will be the default GCC options. That means all platforms will apply
> them.
>
>
>
> Thanks
>
> Liming
>
> *发件人**:* devel@edk2.groups.io <devel@edk2.groups.io> *代表 *Cheng-Chieh
> Huang via groups.io
> *发送时间:* 2021年7月22日 1:43
> *收件人:* Kinney, Michael D <michael.d.kinney@intel.com>
> *抄送:* devel@edk2.groups.io
> *主题:* Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE
> to avoid generating floating points operation
>
>
>
> Yes, we can. I will drop this patch for this  uefipayload batch and send
> another one for support DISABLE_GCC_MMX_SSE in tools_de.txt.
>
>
>
> --
>
> Cheng-Chieh
>
> On Thu, Jul 22, 2021, 12:35 AM Kinney, Michael D <
> michael.d.kinney@intel.com> wrote:
>
> Are those flags needed for all packages that build with GCC?
>
> Should this be moved into tools_def.txt?
>
> Mike
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Cheng-Chieh Huang via groups.io
> > Sent: Wednesday, July 21, 2021 6:23 AM
> > To: devel@edk2.groups.io
> > Cc: Cheng-Chieh Huang <chengchieh@google.com>
> > Subject: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE
> to avoid generating floating points operation
> >
> > This will allow we compile payload using gcc8
> >
> > Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
> > ---
> >  UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc
> b/UefiPayloadPkg/UefiPayloadPkg.dsc
> > index 8aa5f18cd35c..fa41c5a24af5 100644
> > --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> > +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> > @@ -30,6 +30,8 @@ [Defines]
> >    DEFINE PS2_KEYBOARD_ENABLE          = FALSE
> >    DEFINE UNIVERSAL_PAYLOAD            = FALSE
> >
> > +  DEFINE DISABLE_MMX_SSE              = FALSE
> > +
> >    #
> >    # SBL:      UEFI payload for Slim Bootloader
> >    # COREBOOT: UEFI payload for coreboot
> > @@ -96,6 +98,9 @@ [BuildOptions]
> >    *_*_*_CC_FLAGS                 = -D DISABLE_NEW_DEPRECATED_INTERFACES
> >  !if $(BOOTLOADER) == "LINUXBOOT"
> >    *_*_*_CC_FLAGS                 = -D LINUXBOOT_PAYLOAD
> > +!endif
> > +!if $(DISABLE_MMX_SSE)
> > +  *_*_*_CC_FLAGS                 = -mno-mmx -mno-sse
> >  !endif
> >    GCC:*_UNIXGCC_*_CC_FLAGS       = -DMDEPKG_NDEBUG
> >    GCC:RELEASE_*_*_CC_FLAGS       = -DMDEPKG_NDEBUG
> > --
> > 2.32.0.402.g57bb445576-goog
> >
> >
> >
> >
> >
>
> 
>
>

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

Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

[Michael D Kinney]

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

As long as those flags are supported or ignored by the GCC5 compilers, then there should be not harm in adding them to the GCC5 profile.

Mike

From: Cheng-Chieh Huang <chengchieh@google.com>
Sent: Friday, July 23, 2021 10:17 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>
Cc: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

Hi,

This is not about UefiPayload. I did not have GCC5 in my environment, so I use a newer GCC to compile. However, in newer gcc (let's say GCC8), it generates floating point instructions in each function and system will crash as you mentioned that we did not handle it in UEFI.

I uploaded this patch because many people from our group use newer GCC and it will be more convenient if we can disable it with a flag (otherwise, we need to maintain a local patch). I guess the real fix (bigger work) is for EDK2 to support the newer GCC version in its toolchain (create something like *_GCC8_*).

Back to the question, I think it should be no harm to add these flags to tool_defs.txt if EDK2 did not expect the compiler to generate MMX/SSE instructions in the first place.

--
Cheng-Chieh

On Sat, Jul 24, 2021 at 12:46 AM Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:
Can you provide details on why this flag was needed in the UefiPayloadPkg use case?

If the newer versions of the GCC compilers are injecting use of MMX/SSE instructions from optimization of C code, then we need to unconditionally disable that usage in tools_def.txt.  The only place MMX/SSE instructions can be used is in NASM files with proper save/restore of the MMX/SSE state.

Here is an example with use of xmm0 that is saved/restored on the stack.

https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseMemoryLibSse2/X64/CopyMem.nasm

Mike

From: Cheng-Chieh Huang <chengchieh@google.com<mailto:chengchieh@google.com>>
Sent: Friday, July 23, 2021 4:17 AM
To: gaoliming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

I got what you mean by "Tools_def.txt doesn’t support build flag" now. I thought the parser does support some sort of if/else condition, but it seems to be not the case. Alternatively, we can do this during the copy (from template to tools_def.txt), but everytime we need to recreate tools_def.txt which is not ideal).

In that case, I think we should just roll back to only doing this in UefiPayloadPkg.dsc. If other packs want to add these flags, they will need to do it on their own.

On Fri, Jul 23, 2021 at 6:29 PM gaoliming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>> wrote:
How do you add this support in tools_def? Can you give the proposal for it?

Thanks
Liming
发件人: Cheng-Chieh Huang <chengchieh@google.com<mailto:chengchieh@google.com>>
发送时间: 2021年7月22日 10:35
收件人: gaoliming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
抄送: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
主题: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

I mean, I will submit a patch to support DISABLE_GCC_MMX_SSE in tools_def. What do you think?

--
Cheng-Chieh

On Thu, Jul 22, 2021 at 9:28 AM gaoliming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>> wrote:
Tools_def.txt doesn’t support build flag DISABLE_GCC_MMX_SSE. If this flag is moved into BaseTools\Conf\tools_def.template, -mno-mmx -mno-sse option will be the default GCC options. That means all platforms will apply them.

Thanks
Liming
发件人: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> 代表 Cheng-Chieh Huang via groups.io<http://groups.io>
发送时间: 2021年7月22日 1:43
收件人: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
抄送: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
主题: Re: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation

Yes, we can. I will drop this patch for this  uefipayload batch and send another one for support DISABLE_GCC_MMX_SSE in tools_de.txt.

--
Cheng-Chieh
On Thu, Jul 22, 2021, 12:35 AM Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:
Are those flags needed for all packages that build with GCC?

Should this be moved into tools_def.txt?

Mike

> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Cheng-Chieh Huang via groups.io<http://groups.io>
> Sent: Wednesday, July 21, 2021 6:23 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Cheng-Chieh Huang <chengchieh@google.com<mailto:chengchieh@google.com>>
> Subject: [edk2-devel] [PATCH v1 5/6] UefiPayloadPkg: Add DISABLE_MMX_SSE to avoid generating floating points operation
>
> This will allow we compile payload using gcc8
>
> Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com<mailto:chengchieh@google.com>>
> ---
>  UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
> index 8aa5f18cd35c..fa41c5a24af5 100644
> --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> @@ -30,6 +30,8 @@ [Defines]
>    DEFINE PS2_KEYBOARD_ENABLE          = FALSE
>    DEFINE UNIVERSAL_PAYLOAD            = FALSE
>
> +  DEFINE DISABLE_MMX_SSE              = FALSE
> +
>    #
>    # SBL:      UEFI payload for Slim Bootloader
>    # COREBOOT: UEFI payload for coreboot
> @@ -96,6 +98,9 @@ [BuildOptions]
>    *_*_*_CC_FLAGS                 = -D DISABLE_NEW_DEPRECATED_INTERFACES
>  !if $(BOOTLOADER) == "LINUXBOOT"
>    *_*_*_CC_FLAGS                 = -D LINUXBOOT_PAYLOAD
> +!endif
> +!if $(DISABLE_MMX_SSE)
> +  *_*_*_CC_FLAGS                 = -mno-mmx -mno-sse
>  !endif
>    GCC:*_UNIXGCC_*_CC_FLAGS       = -DMDEPKG_NDEBUG
>    GCC:RELEASE_*_*_CC_FLAGS       = -DMDEPKG_NDEBUG
> --
> 2.32.0.402.g57bb445576-goog
>
>
>
>
>


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

[PATCH v1 6/6] UefiPayloadPkg: LinuxBoot: use a text format for the configuration block.

[Cheng-Chieh Huang]

From: Trammell Hudson <hudson@trmm.net>

This adds a text command line for the UefiPayloadPkg that uses
a textual magic number 'LnxBoot1' and a series of white-space
separated key=value[,value...] pairs for the parameters.

The v1 binary configuration structure is used if it is present instead
for backwards compatability.

Currently supported text command line arguments are are:

* `serial=baud,base,width,type,hz,pci`
(only `baud` needs to be specified, the rest have reasonable
defaults)

* `mem=start,end,type`
Types are based on `/sys/firmware/memmaps/*/types`:
    1 == "System RAM"
    3 == "ACPI Tables"
    4 == "ACPI Non-volatile Storage"
    5 == "Reserved"

* `ACPI20=base` pointer to RDSP (from `/sys/firmware/efi/systab`)

* `SMBIOS=base` pointer to the SMBIOS table (also from the EFI table)

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
 UefiPayloadPkg/Include/Linuxboot.h             |  17 +-
 UefiPayloadPkg/Library/LbParseLib/LbParseLib.c | 252 +++++++++++++++++---
 2 files changed, 230 insertions(+), 39 deletions(-)

diff --git a/UefiPayloadPkg/Include/Linuxboot.h b/UefiPayloadPkg/Include/Linuxboot.h
index 34ca18069983..56b39b5a09ff 100644
--- a/UefiPayloadPkg/Include/Linuxboot.h
+++ b/UefiPayloadPkg/Include/Linuxboot.h
@@ -24,8 +24,7 @@ typedef struct MemoryMapEntryStruct {
   UINT32 Type;
 } MemoryMapEntry;
 
-typedef struct UefiPayloadConfigStruct {
-  UINT64 Version;
+typedef struct {
   UINT64 AcpiBase;
   UINT64 AcpiSize;
   UINT64 SmbiosBase;
@@ -33,10 +32,22 @@ typedef struct UefiPayloadConfigStruct {
   SerialPortConfig SerialConfig;
   UINT32 NumMemoryMapEntries;
   MemoryMapEntry MemoryMapEntries[0];
+} UefiPayloadConfigV1;
+
+typedef struct UefiPayloadConfigStruct {
+  UINT64 Version;
+  union {
+    UefiPayloadConfigV1 v1;
+    struct {
+      char cmdline[0]; // up to 64 KB
+    } v2;
+  } config;
 } UefiPayloadConfig;
 #pragma pack()
 
-#define UEFI_PAYLOAD_CONFIG_VERSION 1
+// magic version config is "LnxBoot1"
+#define UEFI_PAYLOAD_CONFIG_VERSION1 1
+#define UEFI_PAYLOAD_CONFIG_VERSION2 0x31746f6f42786e4cULL
 
 #define LINUXBOOT_MEM_RAM 1
 #define LINUXBOOT_MEM_DEFAULT 2
diff --git a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
index 34bfb6a1073f..5e68091cac91 100644
--- a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
+++ b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
@@ -1,13 +1,12 @@
 /** @file
-  This library will parse the linuxboot table in memory and extract those required
-  information.
+  This library will parse the linuxboot table in memory and extract those
+required information.
 
   Copyright (c) 2021, the u-root Authors. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
-
 #include <IndustryStandard/Acpi.h>
 #include <IndustryStandard/SmBios.h>
 #include <Library/BaseLib.h>
@@ -18,17 +17,42 @@
 #include <Library/PcdLib.h>
 #include <Linuxboot.h>
 #include <Uefi/UefiBaseType.h>
+#include <stdint.h>
+#include <stdlib.h>
+//#include <string.h>
+//#include <ctype.h>
+
+#define strncmp(a, b, n) AsciiStrnCmp((a), (b), (n))
+
+static uint64_t parse_int(const char* s, char** end) {
+  UINT64 x;
+
+  if (s[0] == '0' && s[1] == 'x')
+    AsciiStrHexToUint64S(s, end, &x);
+  else
+    AsciiStrDecimalToUint64S(s, end, &x);
+
+  return x;
+}
+
+static int isspace(const char c) { return c == ' ' || c == '\t' || c == '\n'; }
 
 // Retrieve UefiPayloadConfig from Linuxboot's uefiboot
-UefiPayloadConfig* GetUefiPayLoadConfig() {
-  UefiPayloadConfig* config =
+const UefiPayloadConfig* GetUefiPayLoadConfig() {
+  const UefiPayloadConfig* config =
       (UefiPayloadConfig*)(UINTN)(PcdGet32(PcdPayloadFdMemBase) - SIZE_64KB);
-  if (config->Version != UEFI_PAYLOAD_CONFIG_VERSION) {
-    DEBUG((DEBUG_ERROR, "Expect payload config version: %d, but get %d\n",
-           UEFI_PAYLOAD_CONFIG_VERSION, config->Version));
-    CpuDeadLoop ();
-  }
-  return config;
+
+  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1 ||
+      config->Version == UEFI_PAYLOAD_CONFIG_VERSION2)
+    return config;
+
+  DEBUG((DEBUG_ERROR,
+         "Expect payload config version %016lx or %016lx, but get %016lx\n",
+         UEFI_PAYLOAD_CONFIG_VERSION1, UEFI_PAYLOAD_CONFIG_VERSION2,
+         config->Version));
+  CpuDeadLoop();
+  while (1)
+    ;
 }
 
 // Align the address and add memory rang to MemInfoCallback
@@ -54,6 +78,57 @@ void AddMemoryRange(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN UINTN start,
   MemInfoCallback(&MemoryMap, NULL);
 }
 
+const char* cmdline_next(const char* cmdline, const char** option) {
+  // at the end of the string, we're done
+  if (!cmdline || *cmdline == '\0') return NULL;
+
+  // skip any leading whitespace
+  while (isspace(*cmdline)) cmdline++;
+
+  // if we've hit the end of the string, we're done
+  if (*cmdline == '\0') return NULL;
+
+  *option = cmdline;
+
+  // find the end of this option or the string
+  while (!isspace(*cmdline) && *cmdline != '\0') cmdline++;
+
+  // cmdline points to the whitespace or end of string
+  return cmdline;
+}
+
+int cmdline_ints(const char* option, uint64_t args[], int max) {
+  // skip any leading text up to an '='
+  const char* s = option;
+  while (1) {
+    const char c = *s++;
+    if (c == '=') break;
+
+    if (c == '\0' || isspace(c)) {
+      s = option;
+      break;
+    }
+  }
+
+  for (int i = 0; i < max; i++) {
+    char* end;
+    args[i] = parse_int(s, &end);
+
+    // end of string or end of the option?
+    if (*end == '\0' || isspace(*end)) return i + 1;
+
+    // not separator? signal an error if we have consumed any ints,
+    // otherwise return 0 saying that none were found
+    if (*end != ',') return i == 0 ? 0 : -1;
+
+    // skip the , and get the next value
+    s = end + 1;
+  }
+
+  // too many values!
+  return -1;
+}
+
 /**
   Acquire the memory information from the linuxboot table in memory.
 
@@ -67,20 +142,50 @@ void AddMemoryRange(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN UINTN start,
 RETURN_STATUS
 EFIAPI
 ParseMemoryInfo(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN VOID* Params) {
-  UefiPayloadConfig* config;
-  int i;
+  const UefiPayloadConfig* config = GetUefiPayLoadConfig();
+  if (!config) {
+    DEBUG(
+        (DEBUG_ERROR, "ParseMemoryInfo: Could not find UEFI Payload config\n"));
+    return RETURN_SUCCESS;
+  }
+
+  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
+    const UefiPayloadConfigV1* config1 = &config->config.v1;
+    DEBUG(
+        (DEBUG_INFO, "MemoryMap #entries: %d\n", config1->NumMemoryMapEntries));
+
+    for (int i = 0; i < config1->NumMemoryMapEntries; i++) {
+      const MemoryMapEntry* entry = &config1->MemoryMapEntries[i];
+      DEBUG((DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n", entry->Start,
+             entry->End, entry->Type));
+      AddMemoryRange(MemInfoCallback, entry->Start, entry->End, entry->Type);
+    }
+  } else
 
-  config = GetUefiPayLoadConfig();
+      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
+    const char* cmdline = config->config.v2.cmdline;
+    const char* option;
+    uint64_t args[3];
 
-  DEBUG((DEBUG_INFO, "MemoryMap #entries: %d\n", config->NumMemoryMapEntries));
+    // look for the mem=start,end,type
+    while ((cmdline = cmdline_next(cmdline, &option))) {
+      if (strncmp(option, "mem=", 4) != 0) continue;
 
-  MemoryMapEntry* entry = &config->MemoryMapEntries[0];
-  for (i = 0; i < config->NumMemoryMapEntries; i++) {
-    DEBUG((DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n", entry->Start,
-           entry->End, entry->Type));
-    AddMemoryRange(MemInfoCallback, entry->Start, entry->End, entry->Type);
-    entry++;
+      if (cmdline_ints(option, args, 3) != 3) {
+        DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
+        continue;
+      }
+
+      const uint64_t start = args[0];
+      const uint64_t end = args[1];
+      const uint64_t type = args[2];
+
+      DEBUG(
+          (DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n", start, end, type));
+      AddMemoryRange(MemInfoCallback, start, end, type);
+    }
   }
+
   return RETURN_SUCCESS;
 }
 
@@ -96,14 +201,52 @@ ParseMemoryInfo(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN VOID* Params) {
 RETURN_STATUS
 EFIAPI
 ParseSystemTable(OUT SYSTEM_TABLE_INFO* SystemTableInfo) {
-  UefiPayloadConfig* config;
+  const UefiPayloadConfig* config = GetUefiPayLoadConfig();
+  if (!config) {
+    DEBUG((DEBUG_ERROR,
+           "ParseSystemTable: Could not find UEFI Payload config\n"));
+    return RETURN_SUCCESS;
+  }
 
-  config = GetUefiPayLoadConfig();
-  SystemTableInfo->AcpiTableBase = config->AcpiBase;
-  SystemTableInfo->AcpiTableSize = config->AcpiSize;
+  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
+    const UefiPayloadConfigV1* config1 = &config->config.v1;
+    SystemTableInfo->AcpiTableBase = config1->AcpiBase;
+    SystemTableInfo->AcpiTableSize = config1->AcpiSize;
 
-  SystemTableInfo->SmbiosTableBase = config->SmbiosBase;
-  SystemTableInfo->SmbiosTableSize = config->SmbiosSize;
+    SystemTableInfo->SmbiosTableBase = config1->SmbiosBase;
+    SystemTableInfo->SmbiosTableSize = config1->SmbiosSize;
+  } else
+
+      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
+    const char* cmdline = config->config.v2.cmdline;
+    const char* option;
+    uint64_t args[2];
+
+    // look for the acpi config
+    while ((cmdline = cmdline_next(cmdline, &option))) {
+      if (strncmp(option, "ACPI20=", 7) == 0) {
+        const int count = cmdline_ints(option, args, 2);
+        if (count < 0) {
+          DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
+          continue;
+        }
+
+        if (count > 0) SystemTableInfo->AcpiTableBase = args[0];
+        if (count > 1) SystemTableInfo->AcpiTableSize = args[1];
+      }
+
+      if (strncmp(option, "SMBIOS=", 7) == 0) {
+        const int count = cmdline_ints(option, args, 2);
+        if (count < 0) {
+          DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
+          continue;
+        }
+
+        if (count > 0) SystemTableInfo->SmbiosTableBase = args[0];
+        if (count > 1) SystemTableInfo->SmbiosTableSize = args[1];
+      }
+    }
+  }
 
   return RETURN_SUCCESS;
 }
@@ -120,15 +263,52 @@ ParseSystemTable(OUT SYSTEM_TABLE_INFO* SystemTableInfo) {
 RETURN_STATUS
 EFIAPI
 ParseSerialInfo(OUT SERIAL_PORT_INFO* SerialPortInfo) {
-  UefiPayloadConfig* config;
-  config = GetUefiPayLoadConfig();
-
-  SerialPortInfo->BaseAddr = config->SerialConfig.BaseAddr;
-  SerialPortInfo->RegWidth = config->SerialConfig.RegWidth;
-  SerialPortInfo->Type = config->SerialConfig.Type;
-  SerialPortInfo->Baud = config->SerialConfig.Baud;
-  SerialPortInfo->InputHertz = config->SerialConfig.InputHertz;
-  SerialPortInfo->UartPciAddr = config->SerialConfig.UartPciAddr;
+  // fill in some reasonable defaults
+  SerialPortInfo->BaseAddr = 0x3f8;
+  SerialPortInfo->RegWidth = 1;
+  SerialPortInfo->Type = 1;  // uefi.SerialPortTypeIO
+  SerialPortInfo->Baud = 115200;
+  SerialPortInfo->InputHertz = 1843200;
+  SerialPortInfo->UartPciAddr = 0;
+
+  const UefiPayloadConfig* config = GetUefiPayLoadConfig();
+  if (!config) {
+    DEBUG((DEBUG_ERROR, "ParseSerialInfo: using default config\n"));
+    return RETURN_SUCCESS;
+  }
+
+  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
+    const UefiPayloadConfigV1* config1 = &config->config.v1;
+    SerialPortInfo->BaseAddr = config1->SerialConfig.BaseAddr;
+    SerialPortInfo->RegWidth = config1->SerialConfig.RegWidth;
+    SerialPortInfo->Type = config1->SerialConfig.Type;
+    SerialPortInfo->Baud = config1->SerialConfig.Baud;
+    SerialPortInfo->InputHertz = config1->SerialConfig.InputHertz;
+    SerialPortInfo->UartPciAddr = config1->SerialConfig.UartPciAddr;
+  } else
+
+      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
+    const char* cmdline = config->config.v2.cmdline;
+    const char* option;
+    uint64_t args[6] = {};
+
+    while ((cmdline = cmdline_next(cmdline, &option))) {
+      if (strncmp(option, "serial=", 7) != 0) continue;
+
+      const int count = cmdline_ints(option, args, 6);
+      if (count < 0) {
+        DEBUG((DEBUG_ERROR, "Parse error: %a\n", option));
+        continue;
+      }
+
+      if (count > 0) SerialPortInfo->Baud = args[0];
+      if (count > 1) SerialPortInfo->BaseAddr = args[1];
+      if (count > 2) SerialPortInfo->RegWidth = args[2];
+      if (count > 3) SerialPortInfo->Type = args[3];
+      if (count > 4) SerialPortInfo->InputHertz = args[4];
+      if (count > 5) SerialPortInfo->UartPciAddr = args[5];
+    }
+  }
 
   return RETURN_SUCCESS;
 }
-- 
2.32.0.402.g57bb445576-goog

Re: [edk2-devel] [PATCH v1 6/6] UefiPayloadPkg: LinuxBoot: use a text format for the configuration block.

[Marvin Häuser]

On 21.07.21 15:23, Cheng-Chieh Huang via groups.io wrote:
> From: Trammell Hudson <hudson@trmm.net>
>
> This adds a text command line for the UefiPayloadPkg that uses
> a textual magic number 'LnxBoot1' and a series of white-space
> separated key=value[,value...] pairs for the parameters.
>
> The v1 binary configuration structure is used if it is present instead
> for backwards compatability.
>
> Currently supported text command line arguments are are:
>
> * `serial=baud,base,width,type,hz,pci`
> (only `baud` needs to be specified, the rest have reasonable
> defaults)
>
> * `mem=start,end,type`
> Types are based on `/sys/firmware/memmaps/*/types`:
>      1 == "System RAM"
>      3 == "ACPI Tables"
>      4 == "ACPI Non-volatile Storage"
>      5 == "Reserved"
>
> * `ACPI20=base` pointer to RDSP (from `/sys/firmware/efi/systab`)
>
> * `SMBIOS=base` pointer to the SMBIOS table (also from the EFI table)
>
> Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
> ---
>   UefiPayloadPkg/Include/Linuxboot.h             |  17 +-
>   UefiPayloadPkg/Library/LbParseLib/LbParseLib.c | 252 +++++++++++++++++---
>   2 files changed, 230 insertions(+), 39 deletions(-)
>
> diff --git a/UefiPayloadPkg/Include/Linuxboot.h b/UefiPayloadPkg/Include/Linuxboot.h
> index 34ca18069983..56b39b5a09ff 100644
> --- a/UefiPayloadPkg/Include/Linuxboot.h
> +++ b/UefiPayloadPkg/Include/Linuxboot.h
> @@ -24,8 +24,7 @@ typedef struct MemoryMapEntryStruct {
>     UINT32 Type;
>   } MemoryMapEntry;
>   
> -typedef struct UefiPayloadConfigStruct {
> -  UINT64 Version;
> +typedef struct {
>     UINT64 AcpiBase;
>     UINT64 AcpiSize;
>     UINT64 SmbiosBase;
> @@ -33,10 +32,22 @@ typedef struct UefiPayloadConfigStruct {
>     SerialPortConfig SerialConfig;
>     UINT32 NumMemoryMapEntries;
>     MemoryMapEntry MemoryMapEntries[0];
> +} UefiPayloadConfigV1;
> +
> +typedef struct UefiPayloadConfigStruct {
> +  UINT64 Version;
> +  union {
> +    UefiPayloadConfigV1 v1;
> +    struct {
> +      char cmdline[0]; // up to 64 KB
> +    } v2;
> +  } config;
>   } UefiPayloadConfig;
>   #pragma pack()
>   
> -#define UEFI_PAYLOAD_CONFIG_VERSION 1
> +// magic version config is "LnxBoot1"
> +#define UEFI_PAYLOAD_CONFIG_VERSION1 1
> +#define UEFI_PAYLOAD_CONFIG_VERSION2 0x31746f6f42786e4cULL
>   
>   #define LINUXBOOT_MEM_RAM 1
>   #define LINUXBOOT_MEM_DEFAULT 2
> diff --git a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> index 34bfb6a1073f..5e68091cac91 100644
> --- a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> +++ b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> @@ -1,13 +1,12 @@
>   /** @file
> -  This library will parse the linuxboot table in memory and extract those required
> -  information.
> +  This library will parse the linuxboot table in memory and extract those
> +required information.
>   
>     Copyright (c) 2021, the u-root Authors. All rights reserved.<BR>
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>   
>   **/
>   
> -
>   #include <IndustryStandard/Acpi.h>
>   #include <IndustryStandard/SmBios.h>
>   #include <Library/BaseLib.h>
> @@ -18,17 +17,42 @@
>   #include <Library/PcdLib.h>
>   #include <Linuxboot.h>
>   #include <Uefi/UefiBaseType.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +//#include <string.h>
> +//#include <ctype.h>
> +
> +#define strncmp(a, b, n) AsciiStrnCmp((a), (b), (n))
> +
> +static uint64_t parse_int(const char* s, char** end) {
> +  UINT64 x;
> +
> +  if (s[0] == '0' && s[1] == 'x')
> +    AsciiStrHexToUint64S(s, end, &x);
> +  else
> +    AsciiStrDecimalToUint64S(s, end, &x);
> +
> +  return x;
> +}
> +
> +static int isspace(const char c) { return c == ' ' || c == '\t' || c == '\n'; }
>   
>   // Retrieve UefiPayloadConfig from Linuxboot's uefiboot
> -UefiPayloadConfig* GetUefiPayLoadConfig() {
> -  UefiPayloadConfig* config =
> +const UefiPayloadConfig* GetUefiPayLoadConfig() {
> +  const UefiPayloadConfig* config =
>         (UefiPayloadConfig*)(UINTN)(PcdGet32(PcdPayloadFdMemBase) - SIZE_64KB);
> -  if (config->Version != UEFI_PAYLOAD_CONFIG_VERSION) {
> -    DEBUG((DEBUG_ERROR, "Expect payload config version: %d, but get %d\n",
> -           UEFI_PAYLOAD_CONFIG_VERSION, config->Version));
> -    CpuDeadLoop ();
> -  }
> -  return config;
> +
> +  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1 ||
> +      config->Version == UEFI_PAYLOAD_CONFIG_VERSION2)
> +    return config;
> +
> +  DEBUG((DEBUG_ERROR,
> +         "Expect payload config version %016lx or %016lx, but get %016lx\n",
> +         UEFI_PAYLOAD_CONFIG_VERSION1, UEFI_PAYLOAD_CONFIG_VERSION2,
> +         config->Version));
> +  CpuDeadLoop();
> +  while (1)
> +    ;
>   }
>   
>   // Align the address and add memory rang to MemInfoCallback
> @@ -54,6 +78,57 @@ void AddMemoryRange(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN UINTN start,
>     MemInfoCallback(&MemoryMap, NULL);
>   }
>   
> +const char* cmdline_next(const char* cmdline, const char** option) {
> +  // at the end of the string, we're done
> +  if (!cmdline || *cmdline == '\0') return NULL;
> +
> +  // skip any leading whitespace
> +  while (isspace(*cmdline)) cmdline++;
> +
> +  // if we've hit the end of the string, we're done
> +  if (*cmdline == '\0') return NULL;
> +
> +  *option = cmdline;
> +
> +  // find the end of this option or the string
> +  while (!isspace(*cmdline) && *cmdline != '\0') cmdline++;
> +
> +  // cmdline points to the whitespace or end of string
> +  return cmdline;
> +}
> +
> +int cmdline_ints(const char* option, uint64_t args[], int max) {
> +  // skip any leading text up to an '='
> +  const char* s = option;
> +  while (1) {
> +    const char c = *s++;
> +    if (c == '=') break;
> +
> +    if (c == '\0' || isspace(c)) {
> +      s = option;
> +      break;
> +    }
> +  }
> +
> +  for (int i = 0; i < max; i++) {
> +    char* end;
> +    args[i] = parse_int(s, &end);
> +
> +    // end of string or end of the option?
> +    if (*end == '\0' || isspace(*end)) return i + 1;
> +
> +    // not separator? signal an error if we have consumed any ints,
> +    // otherwise return 0 saying that none were found
> +    if (*end != ',') return i == 0 ? 0 : -1;
> +
> +    // skip the , and get the next value
> +    s = end + 1;
> +  }
> +
> +  // too many values!
> +  return -1;
> +}
> +
>   /**
>     Acquire the memory information from the linuxboot table in memory.
>   
> @@ -67,20 +142,50 @@ void AddMemoryRange(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN UINTN start,
>   RETURN_STATUS
>   EFIAPI
>   ParseMemoryInfo(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN VOID* Params) {
> -  UefiPayloadConfig* config;
> -  int i;
> +  const UefiPayloadConfig* config = GetUefiPayLoadConfig();
> +  if (!config) {
> +    DEBUG(
> +        (DEBUG_ERROR, "ParseMemoryInfo: Could not find UEFI Payload config\n"));
> +    return RETURN_SUCCESS;
> +  }
> +
> +  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
> +    const UefiPayloadConfigV1* config1 = &config->config.v1;
> +    DEBUG(
> +        (DEBUG_INFO, "MemoryMap #entries: %d\n", config1->NumMemoryMapEntries));
> +
> +    for (int i = 0; i < config1->NumMemoryMapEntries; i++) {
> +      const MemoryMapEntry* entry = &config1->MemoryMapEntries[i];
> +      DEBUG((DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n", entry->Start,
> +             entry->End, entry->Type));
> +      AddMemoryRange(MemInfoCallback, entry->Start, entry->End, entry->Type);
> +    }
> +  } else
>   
> -  config = GetUefiPayLoadConfig();
> +      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
> +    const char* cmdline = config->config.v2.cmdline;
> +    const char* option;
> +    uint64_t args[3];
>   
> -  DEBUG((DEBUG_INFO, "MemoryMap #entries: %d\n", config->NumMemoryMapEntries));
> +    // look for the mem=start,end,type
> +    while ((cmdline = cmdline_next(cmdline, &option))) {
> +      if (strncmp(option, "mem=", 4) != 0) continue;
>   
> -  MemoryMapEntry* entry = &config->MemoryMapEntries[0];
> -  for (i = 0; i < config->NumMemoryMapEntries; i++) {
> -    DEBUG((DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n", entry->Start,
> -           entry->End, entry->Type));
> -    AddMemoryRange(MemInfoCallback, entry->Start, entry->End, entry->Type);
> -    entry++;
> +      if (cmdline_ints(option, args, 3) != 3) {
> +        DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
> +        continue;
> +      }
> +
> +      const uint64_t start = args[0];
> +      const uint64_t end = args[1];
> +      const uint64_t type = args[2];
> +
> +      DEBUG(
> +          (DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n", start, end, type));
> +      AddMemoryRange(MemInfoCallback, start, end, type);
> +    }
>     }
> +
>     return RETURN_SUCCESS;
>   }
>   
> @@ -96,14 +201,52 @@ ParseMemoryInfo(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN VOID* Params) {
>   RETURN_STATUS
>   EFIAPI
>   ParseSystemTable(OUT SYSTEM_TABLE_INFO* SystemTableInfo) {
> -  UefiPayloadConfig* config;
> +  const UefiPayloadConfig* config = GetUefiPayLoadConfig();
> +  if (!config) {
> +    DEBUG((DEBUG_ERROR,
> +           "ParseSystemTable: Could not find UEFI Payload config\n"));
> +    return RETURN_SUCCESS;
> +  }
>   
> -  config = GetUefiPayLoadConfig();
> -  SystemTableInfo->AcpiTableBase = config->AcpiBase;
> -  SystemTableInfo->AcpiTableSize = config->AcpiSize;
> +  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
> +    const UefiPayloadConfigV1* config1 = &config->config.v1;
> +    SystemTableInfo->AcpiTableBase = config1->AcpiBase;
> +    SystemTableInfo->AcpiTableSize = config1->AcpiSize;
>   
> -  SystemTableInfo->SmbiosTableBase = config->SmbiosBase;
> -  SystemTableInfo->SmbiosTableSize = config->SmbiosSize;
> +    SystemTableInfo->SmbiosTableBase = config1->SmbiosBase;
> +    SystemTableInfo->SmbiosTableSize = config1->SmbiosSize;
> +  } else
> +
> +      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
> +    const char* cmdline = config->config.v2.cmdline;
> +    const char* option;
> +    uint64_t args[2];
> +
> +    // look for the acpi config
> +    while ((cmdline = cmdline_next(cmdline, &option))) {
> +      if (strncmp(option, "ACPI20=", 7) == 0) {
> +        const int count = cmdline_ints(option, args, 2);
> +        if (count < 0) {
> +          DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
> +          continue;
> +        }
> +
> +        if (count > 0) SystemTableInfo->AcpiTableBase = args[0];
> +        if (count > 1) SystemTableInfo->AcpiTableSize = args[1];
> +      }
> +
> +      if (strncmp(option, "SMBIOS=", 7) == 0) {
> +        const int count = cmdline_ints(option, args, 2);
> +        if (count < 0) {
> +          DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
> +          continue;
> +        }
> +
> +        if (count > 0) SystemTableInfo->SmbiosTableBase = args[0];
> +        if (count > 1) SystemTableInfo->SmbiosTableSize = args[1];
> +      }

Sorry, from only a quick peak, can this not leave {Acpi,Smbios}TableBase 
and {Acpi,Smbios}TableSize undefined entirely, while returning 
RETURN_SUCCESS?
Even if not, it looks kind of odd a command-line argument can change the 
base address without changing the size, is there a documentation of this 
behaviour?
What is the structure supposed to carry if no such arguments are given 
at all?

Thanks for your time!

Best regards,
Marvin

> +    }
> +  }
>   
>     return RETURN_SUCCESS;
>   }
> @@ -120,15 +263,52 @@ ParseSystemTable(OUT SYSTEM_TABLE_INFO* SystemTableInfo) {
>   RETURN_STATUS
>   EFIAPI
>   ParseSerialInfo(OUT SERIAL_PORT_INFO* SerialPortInfo) {
> -  UefiPayloadConfig* config;
> -  config = GetUefiPayLoadConfig();
> -
> -  SerialPortInfo->BaseAddr = config->SerialConfig.BaseAddr;
> -  SerialPortInfo->RegWidth = config->SerialConfig.RegWidth;
> -  SerialPortInfo->Type = config->SerialConfig.Type;
> -  SerialPortInfo->Baud = config->SerialConfig.Baud;
> -  SerialPortInfo->InputHertz = config->SerialConfig.InputHertz;
> -  SerialPortInfo->UartPciAddr = config->SerialConfig.UartPciAddr;
> +  // fill in some reasonable defaults
> +  SerialPortInfo->BaseAddr = 0x3f8;
> +  SerialPortInfo->RegWidth = 1;
> +  SerialPortInfo->Type = 1;  // uefi.SerialPortTypeIO
> +  SerialPortInfo->Baud = 115200;
> +  SerialPortInfo->InputHertz = 1843200;
> +  SerialPortInfo->UartPciAddr = 0;
> +
> +  const UefiPayloadConfig* config = GetUefiPayLoadConfig();
> +  if (!config) {
> +    DEBUG((DEBUG_ERROR, "ParseSerialInfo: using default config\n"));
> +    return RETURN_SUCCESS;
> +  }
> +
> +  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
> +    const UefiPayloadConfigV1* config1 = &config->config.v1;
> +    SerialPortInfo->BaseAddr = config1->SerialConfig.BaseAddr;
> +    SerialPortInfo->RegWidth = config1->SerialConfig.RegWidth;
> +    SerialPortInfo->Type = config1->SerialConfig.Type;
> +    SerialPortInfo->Baud = config1->SerialConfig.Baud;
> +    SerialPortInfo->InputHertz = config1->SerialConfig.InputHertz;
> +    SerialPortInfo->UartPciAddr = config1->SerialConfig.UartPciAddr;
> +  } else
> +
> +      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
> +    const char* cmdline = config->config.v2.cmdline;
> +    const char* option;
> +    uint64_t args[6] = {};
> +
> +    while ((cmdline = cmdline_next(cmdline, &option))) {
> +      if (strncmp(option, "serial=", 7) != 0) continue;
> +
> +      const int count = cmdline_ints(option, args, 6);
> +      if (count < 0) {
> +        DEBUG((DEBUG_ERROR, "Parse error: %a\n", option));
> +        continue;
> +      }
> +
> +      if (count > 0) SerialPortInfo->Baud = args[0];
> +      if (count > 1) SerialPortInfo->BaseAddr = args[1];
> +      if (count > 2) SerialPortInfo->RegWidth = args[2];
> +      if (count > 3) SerialPortInfo->Type = args[3];
> +      if (count > 4) SerialPortInfo->InputHertz = args[4];
> +      if (count > 5) SerialPortInfo->UartPciAddr = args[5];
> +    }
> +  }
>   
>     return RETURN_SUCCESS;
>   }

Re: [edk2-devel] [PATCH v1 6/6] UefiPayloadPkg: LinuxBoot: use a text format for the configuration block.

[Cheng-Chieh Huang]

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

Here is the documentation:
https://github.com/osresearch/kexec-load/blob/main/README.md

On Thu, Jul 22, 2021 at 1:09 AM Marvin Häuser <mhaeuser@posteo.de> wrote:

>
>
> On 21.07.21 15:23, Cheng-Chieh Huang via groups.io wrote:
> > From: Trammell Hudson <hudson@trmm.net>
> >
> > This adds a text command line for the UefiPayloadPkg that uses
> > a textual magic number 'LnxBoot1' and a series of white-space
> > separated key=value[,value...] pairs for the parameters.
> >
> > The v1 binary configuration structure is used if it is present instead
> > for backwards compatability.
> >
> > Currently supported text command line arguments are are:
> >
> > * `serial=baud,base,width,type,hz,pci`
> > (only `baud` needs to be specified, the rest have reasonable
> > defaults)
> >
> > * `mem=start,end,type`
> > Types are based on `/sys/firmware/memmaps/*/types`:
> >      1 == "System RAM"
> >      3 == "ACPI Tables"
> >      4 == "ACPI Non-volatile Storage"
> >      5 == "Reserved"
> >
> > * `ACPI20=base` pointer to RDSP (from `/sys/firmware/efi/systab`)
> >
> > * `SMBIOS=base` pointer to the SMBIOS table (also from the EFI table)
> >
> > Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
> > ---
> >   UefiPayloadPkg/Include/Linuxboot.h             |  17 +-
> >   UefiPayloadPkg/Library/LbParseLib/LbParseLib.c | 252
> +++++++++++++++++---
> >   2 files changed, 230 insertions(+), 39 deletions(-)
> >
> > diff --git a/UefiPayloadPkg/Include/Linuxboot.h
> b/UefiPayloadPkg/Include/Linuxboot.h
> > index 34ca18069983..56b39b5a09ff 100644
> > --- a/UefiPayloadPkg/Include/Linuxboot.h
> > +++ b/UefiPayloadPkg/Include/Linuxboot.h
> > @@ -24,8 +24,7 @@ typedef struct MemoryMapEntryStruct {
> >     UINT32 Type;
> >   } MemoryMapEntry;
> >
> > -typedef struct UefiPayloadConfigStruct {
> > -  UINT64 Version;
> > +typedef struct {
> >     UINT64 AcpiBase;
> >     UINT64 AcpiSize;
> >     UINT64 SmbiosBase;
> > @@ -33,10 +32,22 @@ typedef struct UefiPayloadConfigStruct {
> >     SerialPortConfig SerialConfig;
> >     UINT32 NumMemoryMapEntries;
> >     MemoryMapEntry MemoryMapEntries[0];
> > +} UefiPayloadConfigV1;
> > +
> > +typedef struct UefiPayloadConfigStruct {
> > +  UINT64 Version;
> > +  union {
> > +    UefiPayloadConfigV1 v1;
> > +    struct {
> > +      char cmdline[0]; // up to 64 KB
> > +    } v2;
> > +  } config;
> >   } UefiPayloadConfig;
> >   #pragma pack()
> >
> > -#define UEFI_PAYLOAD_CONFIG_VERSION 1
> > +// magic version config is "LnxBoot1"
> > +#define UEFI_PAYLOAD_CONFIG_VERSION1 1
> > +#define UEFI_PAYLOAD_CONFIG_VERSION2 0x31746f6f42786e4cULL
> >
> >   #define LINUXBOOT_MEM_RAM 1
> >   #define LINUXBOOT_MEM_DEFAULT 2
> > diff --git a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> > index 34bfb6a1073f..5e68091cac91 100644
> > --- a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> > +++ b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> > @@ -1,13 +1,12 @@
> >   /** @file
> > -  This library will parse the linuxboot table in memory and extract
> those required
> > -  information.
> > +  This library will parse the linuxboot table in memory and extract
> those
> > +required information.
> >
> >     Copyright (c) 2021, the u-root Authors. All rights reserved.<BR>
> >     SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >   **/
> >
> > -
> >   #include <IndustryStandard/Acpi.h>
> >   #include <IndustryStandard/SmBios.h>
> >   #include <Library/BaseLib.h>
> > @@ -18,17 +17,42 @@
> >   #include <Library/PcdLib.h>
> >   #include <Linuxboot.h>
> >   #include <Uefi/UefiBaseType.h>
> > +#include <stdint.h>
> > +#include <stdlib.h>
> > +//#include <string.h>
> > +//#include <ctype.h>
> > +
> > +#define strncmp(a, b, n) AsciiStrnCmp((a), (b), (n))
> > +
> > +static uint64_t parse_int(const char* s, char** end) {
> > +  UINT64 x;
> > +
> > +  if (s[0] == '0' && s[1] == 'x')
> > +    AsciiStrHexToUint64S(s, end, &x);
> > +  else
> > +    AsciiStrDecimalToUint64S(s, end, &x);
> > +
> > +  return x;
> > +}
> > +
> > +static int isspace(const char c) { return c == ' ' || c == '\t' || c ==
> '\n'; }
> >
> >   // Retrieve UefiPayloadConfig from Linuxboot's uefiboot
> > -UefiPayloadConfig* GetUefiPayLoadConfig() {
> > -  UefiPayloadConfig* config =
> > +const UefiPayloadConfig* GetUefiPayLoadConfig() {
> > +  const UefiPayloadConfig* config =
> >         (UefiPayloadConfig*)(UINTN)(PcdGet32(PcdPayloadFdMemBase) -
> SIZE_64KB);
> > -  if (config->Version != UEFI_PAYLOAD_CONFIG_VERSION) {
> > -    DEBUG((DEBUG_ERROR, "Expect payload config version: %d, but get
> %d\n",
> > -           UEFI_PAYLOAD_CONFIG_VERSION, config->Version));
> > -    CpuDeadLoop ();
> > -  }
> > -  return config;
> > +
> > +  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1 ||
> > +      config->Version == UEFI_PAYLOAD_CONFIG_VERSION2)
> > +    return config;
> > +
> > +  DEBUG((DEBUG_ERROR,
> > +         "Expect payload config version %016lx or %016lx, but get
> %016lx\n",
> > +         UEFI_PAYLOAD_CONFIG_VERSION1, UEFI_PAYLOAD_CONFIG_VERSION2,
> > +         config->Version));
> > +  CpuDeadLoop();
> > +  while (1)
> > +    ;
> >   }
> >
> >   // Align the address and add memory rang to MemInfoCallback
> > @@ -54,6 +78,57 @@ void AddMemoryRange(IN BL_MEM_INFO_CALLBACK
> MemInfoCallback, IN UINTN start,
> >     MemInfoCallback(&MemoryMap, NULL);
> >   }
> >
> > +const char* cmdline_next(const char* cmdline, const char** option) {
> > +  // at the end of the string, we're done
> > +  if (!cmdline || *cmdline == '\0') return NULL;
> > +
> > +  // skip any leading whitespace
> > +  while (isspace(*cmdline)) cmdline++;
> > +
> > +  // if we've hit the end of the string, we're done
> > +  if (*cmdline == '\0') return NULL;
> > +
> > +  *option = cmdline;
> > +
> > +  // find the end of this option or the string
> > +  while (!isspace(*cmdline) && *cmdline != '\0') cmdline++;
> > +
> > +  // cmdline points to the whitespace or end of string
> > +  return cmdline;
> > +}
> > +
> > +int cmdline_ints(const char* option, uint64_t args[], int max) {
> > +  // skip any leading text up to an '='
> > +  const char* s = option;
> > +  while (1) {
> > +    const char c = *s++;
> > +    if (c == '=') break;
> > +
> > +    if (c == '\0' || isspace(c)) {
> > +      s = option;
> > +      break;
> > +    }
> > +  }
> > +
> > +  for (int i = 0; i < max; i++) {
> > +    char* end;
> > +    args[i] = parse_int(s, &end);
> > +
> > +    // end of string or end of the option?
> > +    if (*end == '\0' || isspace(*end)) return i + 1;
> > +
> > +    // not separator? signal an error if we have consumed any ints,
> > +    // otherwise return 0 saying that none were found
> > +    if (*end != ',') return i == 0 ? 0 : -1;
> > +
> > +    // skip the , and get the next value
> > +    s = end + 1;
> > +  }
> > +
> > +  // too many values!
> > +  return -1;
> > +}
> > +
> >   /**
> >     Acquire the memory information from the linuxboot table in memory.
> >
> > @@ -67,20 +142,50 @@ void AddMemoryRange(IN BL_MEM_INFO_CALLBACK
> MemInfoCallback, IN UINTN start,
> >   RETURN_STATUS
> >   EFIAPI
> >   ParseMemoryInfo(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN VOID*
> Params) {
> > -  UefiPayloadConfig* config;
> > -  int i;
> > +  const UefiPayloadConfig* config = GetUefiPayLoadConfig();
> > +  if (!config) {
> > +    DEBUG(
> > +        (DEBUG_ERROR, "ParseMemoryInfo: Could not find UEFI Payload
> config\n"));
> > +    return RETURN_SUCCESS;
> > +  }
> > +
> > +  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
> > +    const UefiPayloadConfigV1* config1 = &config->config.v1;
> > +    DEBUG(
> > +        (DEBUG_INFO, "MemoryMap #entries: %d\n",
> config1->NumMemoryMapEntries));
> > +
> > +    for (int i = 0; i < config1->NumMemoryMapEntries; i++) {
> > +      const MemoryMapEntry* entry = &config1->MemoryMapEntries[i];
> > +      DEBUG((DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n",
> entry->Start,
> > +             entry->End, entry->Type));
> > +      AddMemoryRange(MemInfoCallback, entry->Start, entry->End,
> entry->Type);
> > +    }
> > +  } else
> >
> > -  config = GetUefiPayLoadConfig();
> > +      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
> > +    const char* cmdline = config->config.v2.cmdline;
> > +    const char* option;
> > +    uint64_t args[3];
> >
> > -  DEBUG((DEBUG_INFO, "MemoryMap #entries: %d\n",
> config->NumMemoryMapEntries));
> > +    // look for the mem=start,end,type
> > +    while ((cmdline = cmdline_next(cmdline, &option))) {
> > +      if (strncmp(option, "mem=", 4) != 0) continue;
> >
> > -  MemoryMapEntry* entry = &config->MemoryMapEntries[0];
> > -  for (i = 0; i < config->NumMemoryMapEntries; i++) {
> > -    DEBUG((DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n",
> entry->Start,
> > -           entry->End, entry->Type));
> > -    AddMemoryRange(MemInfoCallback, entry->Start, entry->End,
> entry->Type);
> > -    entry++;
> > +      if (cmdline_ints(option, args, 3) != 3) {
> > +        DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
> > +        continue;
> > +      }
> > +
> > +      const uint64_t start = args[0];
> > +      const uint64_t end = args[1];
> > +      const uint64_t type = args[2];
> > +
> > +      DEBUG(
> > +          (DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n", start, end,
> type));
> > +      AddMemoryRange(MemInfoCallback, start, end, type);
> > +    }
> >     }
> > +
> >     return RETURN_SUCCESS;
> >   }
> >
> > @@ -96,14 +201,52 @@ ParseMemoryInfo(IN BL_MEM_INFO_CALLBACK
> MemInfoCallback, IN VOID* Params) {
> >   RETURN_STATUS
> >   EFIAPI
> >   ParseSystemTable(OUT SYSTEM_TABLE_INFO* SystemTableInfo) {
> > -  UefiPayloadConfig* config;
> > +  const UefiPayloadConfig* config = GetUefiPayLoadConfig();
> > +  if (!config) {
> > +    DEBUG((DEBUG_ERROR,
> > +           "ParseSystemTable: Could not find UEFI Payload config\n"));
> > +    return RETURN_SUCCESS;
> > +  }
> >
> > -  config = GetUefiPayLoadConfig();
> > -  SystemTableInfo->AcpiTableBase = config->AcpiBase;
> > -  SystemTableInfo->AcpiTableSize = config->AcpiSize;
> > +  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
> > +    const UefiPayloadConfigV1* config1 = &config->config.v1;
> > +    SystemTableInfo->AcpiTableBase = config1->AcpiBase;
> > +    SystemTableInfo->AcpiTableSize = config1->AcpiSize;
> >
> > -  SystemTableInfo->SmbiosTableBase = config->SmbiosBase;
> > -  SystemTableInfo->SmbiosTableSize = config->SmbiosSize;
> > +    SystemTableInfo->SmbiosTableBase = config1->SmbiosBase;
> > +    SystemTableInfo->SmbiosTableSize = config1->SmbiosSize;
> > +  } else
> > +
> > +      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
> > +    const char* cmdline = config->config.v2.cmdline;
> > +    const char* option;
> > +    uint64_t args[2];
> > +
> > +    // look for the acpi config
> > +    while ((cmdline = cmdline_next(cmdline, &option))) {
> > +      if (strncmp(option, "ACPI20=", 7) == 0) {
> > +        const int count = cmdline_ints(option, args, 2);
> > +        if (count < 0) {
> > +          DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
> > +          continue;
> > +        }
> > +
> > +        if (count > 0) SystemTableInfo->AcpiTableBase = args[0];
> > +        if (count > 1) SystemTableInfo->AcpiTableSize = args[1];
> > +      }
> > +
> > +      if (strncmp(option, "SMBIOS=", 7) == 0) {
> > +        const int count = cmdline_ints(option, args, 2);
> > +        if (count < 0) {
> > +          DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
> > +          continue;
> > +        }
> > +
> > +        if (count > 0) SystemTableInfo->SmbiosTableBase = args[0];
> > +        if (count > 1) SystemTableInfo->SmbiosTableSize = args[1];
> > +      }
>
> Sorry, from only a quick peak, can this not leave {Acpi,Smbios}TableBase
> and {Acpi,Smbios}TableSize undefined entirely, while returning
> RETURN_SUCCESS?
> Even if not, it looks kind of odd a command-line argument can change the
> base address without changing the size, is there a documentation of this
> behaviour?
> What is the structure supposed to carry if no such arguments are given
> at all?
>
> Thanks for your time!
>
> Best regards,
> Marvin
>
> > +    }
> > +  }
> >
> >     return RETURN_SUCCESS;
> >   }
> > @@ -120,15 +263,52 @@ ParseSystemTable(OUT SYSTEM_TABLE_INFO*
> SystemTableInfo) {
> >   RETURN_STATUS
> >   EFIAPI
> >   ParseSerialInfo(OUT SERIAL_PORT_INFO* SerialPortInfo) {
> > -  UefiPayloadConfig* config;
> > -  config = GetUefiPayLoadConfig();
> > -
> > -  SerialPortInfo->BaseAddr = config->SerialConfig.BaseAddr;
> > -  SerialPortInfo->RegWidth = config->SerialConfig.RegWidth;
> > -  SerialPortInfo->Type = config->SerialConfig.Type;
> > -  SerialPortInfo->Baud = config->SerialConfig.Baud;
> > -  SerialPortInfo->InputHertz = config->SerialConfig.InputHertz;
> > -  SerialPortInfo->UartPciAddr = config->SerialConfig.UartPciAddr;
> > +  // fill in some reasonable defaults
> > +  SerialPortInfo->BaseAddr = 0x3f8;
> > +  SerialPortInfo->RegWidth = 1;
> > +  SerialPortInfo->Type = 1;  // uefi.SerialPortTypeIO
> > +  SerialPortInfo->Baud = 115200;
> > +  SerialPortInfo->InputHertz = 1843200;
> > +  SerialPortInfo->UartPciAddr = 0;
> > +
> > +  const UefiPayloadConfig* config = GetUefiPayLoadConfig();
> > +  if (!config) {
> > +    DEBUG((DEBUG_ERROR, "ParseSerialInfo: using default config\n"));
> > +    return RETURN_SUCCESS;
> > +  }
> > +
> > +  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
> > +    const UefiPayloadConfigV1* config1 = &config->config.v1;
> > +    SerialPortInfo->BaseAddr = config1->SerialConfig.BaseAddr;
> > +    SerialPortInfo->RegWidth = config1->SerialConfig.RegWidth;
> > +    SerialPortInfo->Type = config1->SerialConfig.Type;
> > +    SerialPortInfo->Baud = config1->SerialConfig.Baud;
> > +    SerialPortInfo->InputHertz = config1->SerialConfig.InputHertz;
> > +    SerialPortInfo->UartPciAddr = config1->SerialConfig.UartPciAddr;
> > +  } else
> > +
> > +      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
> > +    const char* cmdline = config->config.v2.cmdline;
> > +    const char* option;
> > +    uint64_t args[6] = {};
> > +
> > +    while ((cmdline = cmdline_next(cmdline, &option))) {
> > +      if (strncmp(option, "serial=", 7) != 0) continue;
> > +
> > +      const int count = cmdline_ints(option, args, 6);
> > +      if (count < 0) {
> > +        DEBUG((DEBUG_ERROR, "Parse error: %a\n", option));
> > +        continue;
> > +      }
> > +
> > +      if (count > 0) SerialPortInfo->Baud = args[0];
> > +      if (count > 1) SerialPortInfo->BaseAddr = args[1];
> > +      if (count > 2) SerialPortInfo->RegWidth = args[2];
> > +      if (count > 3) SerialPortInfo->Type = args[3];
> > +      if (count > 4) SerialPortInfo->InputHertz = args[4];
> > +      if (count > 5) SerialPortInfo->UartPciAddr = args[5];
> > +    }
> > +  }
> >
> >     return RETURN_SUCCESS;
> >   }
>
>

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

Re: [edk2-devel] [PATCH v1 6/6] UefiPayloadPkg: LinuxBoot: use a text format for the configuration block.

[Guo Dong]

Please run BaseTools\Scripts\PatchCheck.py to make sure it passed the check.

Update LbParseLib.c following https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/.
current code is more like Linux style. E.g.
+static uint64_t parse_int(const char* s, char** end) {
+  UINT64 x;

Removed unused comments
+//#include <string.h>
+//#include <ctype.h>


Thanks,
Guo
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.groups.io
Cc: Trammell Hudson <hudson@trmm.net>; Cheng-Chieh Huang <chengchieh@google.com>
Subject: [edk2-devel] [PATCH v1 6/6] UefiPayloadPkg: LinuxBoot: use a text format for the configuration block.

From: Trammell Hudson <hudson@trmm.net>

This adds a text command line for the UefiPayloadPkg that uses a textual magic number 'LnxBoot1' and a series of white-space separated key=value[,value...] pairs for the parameters.

The v1 binary configuration structure is used if it is present instead for backwards compatability.

Currently supported text command line arguments are are:

* `serial=baud,base,width,type,hz,pci`
(only `baud` needs to be specified, the rest have reasonable
defaults)

* `mem=start,end,type`
Types are based on `/sys/firmware/memmaps/*/types`:
    1 == "System RAM"
    3 == "ACPI Tables"
    4 == "ACPI Non-volatile Storage"
    5 == "Reserved"

* `ACPI20=base` pointer to RDSP (from `/sys/firmware/efi/systab`)

* `SMBIOS=base` pointer to the SMBIOS table (also from the EFI table)

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
 UefiPayloadPkg/Include/Linuxboot.h             |  17 +-
 UefiPayloadPkg/Library/LbParseLib/LbParseLib.c | 252 +++++++++++++++++---
 2 files changed, 230 insertions(+), 39 deletions(-)

diff --git a/UefiPayloadPkg/Include/Linuxboot.h b/UefiPayloadPkg/Include/Linuxboot.h
index 34ca18069983..56b39b5a09ff 100644
--- a/UefiPayloadPkg/Include/Linuxboot.h
+++ b/UefiPayloadPkg/Include/Linuxboot.h
@@ -24,8 +24,7 @@ typedef struct MemoryMapEntryStruct {
   UINT32 Type;
 } MemoryMapEntry;
 
-typedef struct UefiPayloadConfigStruct {
-  UINT64 Version;
+typedef struct {
   UINT64 AcpiBase;
   UINT64 AcpiSize;
   UINT64 SmbiosBase;
@@ -33,10 +32,22 @@ typedef struct UefiPayloadConfigStruct {
   SerialPortConfig SerialConfig;
   UINT32 NumMemoryMapEntries;
   MemoryMapEntry MemoryMapEntries[0];
+} UefiPayloadConfigV1;
+
+typedef struct UefiPayloadConfigStruct {
+  UINT64 Version;
+  union {
+    UefiPayloadConfigV1 v1;
+    struct {
+      char cmdline[0]; // up to 64 KB
+    } v2;
+  } config;
 } UefiPayloadConfig;
 #pragma pack()
 
-#define UEFI_PAYLOAD_CONFIG_VERSION 1
+// magic version config is "LnxBoot1"
+#define UEFI_PAYLOAD_CONFIG_VERSION1 1
+#define UEFI_PAYLOAD_CONFIG_VERSION2 0x31746f6f42786e4cULL
 
 #define LINUXBOOT_MEM_RAM 1
 #define LINUXBOOT_MEM_DEFAULT 2
diff --git a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
index 34bfb6a1073f..5e68091cac91 100644
--- a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
+++ b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
@@ -1,13 +1,12 @@
 /** @file
-  This library will parse the linuxboot table in memory and extract those required
-  information.
+  This library will parse the linuxboot table in memory and extract 
+those required information.
 
   Copyright (c) 2021, the u-root Authors. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
-
 #include <IndustryStandard/Acpi.h>
 #include <IndustryStandard/SmBios.h>
 #include <Library/BaseLib.h>
@@ -18,17 +17,42 @@
 #include <Library/PcdLib.h>
 #include <Linuxboot.h>
 #include <Uefi/UefiBaseType.h>
+#include <stdint.h>
+#include <stdlib.h>
+//#include <string.h>
+//#include <ctype.h>
+
+#define strncmp(a, b, n) AsciiStrnCmp((a), (b), (n))
+
+static uint64_t parse_int(const char* s, char** end) {
+  UINT64 x;
+
+  if (s[0] == '0' && s[1] == 'x')
+    AsciiStrHexToUint64S(s, end, &x);
+  else
+    AsciiStrDecimalToUint64S(s, end, &x);
+
+  return x;
+}
+
+static int isspace(const char c) { return c == ' ' || c == '\t' || c == 
+'\n'; }
 
 // Retrieve UefiPayloadConfig from Linuxboot's uefiboot
-UefiPayloadConfig* GetUefiPayLoadConfig() {
-  UefiPayloadConfig* config =
+const UefiPayloadConfig* GetUefiPayLoadConfig() {
+  const UefiPayloadConfig* config =
       (UefiPayloadConfig*)(UINTN)(PcdGet32(PcdPayloadFdMemBase) - SIZE_64KB);
-  if (config->Version != UEFI_PAYLOAD_CONFIG_VERSION) {
-    DEBUG((DEBUG_ERROR, "Expect payload config version: %d, but get %d\n",
-           UEFI_PAYLOAD_CONFIG_VERSION, config->Version));
-    CpuDeadLoop ();
-  }
-  return config;
+
+  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1 ||
+      config->Version == UEFI_PAYLOAD_CONFIG_VERSION2)
+    return config;
+
+  DEBUG((DEBUG_ERROR,
+         "Expect payload config version %016lx or %016lx, but get %016lx\n",
+         UEFI_PAYLOAD_CONFIG_VERSION1, UEFI_PAYLOAD_CONFIG_VERSION2,
+         config->Version));
+  CpuDeadLoop();
+  while (1)
+    ;
 }
 
 // Align the address and add memory rang to MemInfoCallback @@ -54,6 +78,57 @@ void AddMemoryRange(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN UINTN start,
   MemInfoCallback(&MemoryMap, NULL);
 }
 
+const char* cmdline_next(const char* cmdline, const char** option) {
+  // at the end of the string, we're done
+  if (!cmdline || *cmdline == '\0') return NULL;
+
+  // skip any leading whitespace
+  while (isspace(*cmdline)) cmdline++;
+
+  // if we've hit the end of the string, we're done  if (*cmdline == 
+ '\0') return NULL;
+
+  *option = cmdline;
+
+  // find the end of this option or the string  while 
+ (!isspace(*cmdline) && *cmdline != '\0') cmdline++;
+
+  // cmdline points to the whitespace or end of string
+  return cmdline;
+}
+
+int cmdline_ints(const char* option, uint64_t args[], int max) {
+  // skip any leading text up to an '='
+  const char* s = option;
+  while (1) {
+    const char c = *s++;
+    if (c == '=') break;
+
+    if (c == '\0' || isspace(c)) {
+      s = option;
+      break;
+    }
+  }
+
+  for (int i = 0; i < max; i++) {
+    char* end;
+    args[i] = parse_int(s, &end);
+
+    // end of string or end of the option?
+    if (*end == '\0' || isspace(*end)) return i + 1;
+
+    // not separator? signal an error if we have consumed any ints,
+    // otherwise return 0 saying that none were found
+    if (*end != ',') return i == 0 ? 0 : -1;
+
+    // skip the , and get the next value
+    s = end + 1;
+  }
+
+  // too many values!
+  return -1;
+}
+
 /**
   Acquire the memory information from the linuxboot table in memory.
 
@@ -67,20 +142,50 @@ void AddMemoryRange(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN UINTN start,  RETURN_STATUS  EFIAPI  ParseMemoryInfo(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN VOID* Params) {
-  UefiPayloadConfig* config;
-  int i;
+  const UefiPayloadConfig* config = GetUefiPayLoadConfig();  if 
+ (!config) {
+    DEBUG(
+        (DEBUG_ERROR, "ParseMemoryInfo: Could not find UEFI Payload config\n"));
+    return RETURN_SUCCESS;
+  }
+
+  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
+    const UefiPayloadConfigV1* config1 = &config->config.v1;
+    DEBUG(
+        (DEBUG_INFO, "MemoryMap #entries: %d\n", 
+ config1->NumMemoryMapEntries));
+
+    for (int i = 0; i < config1->NumMemoryMapEntries; i++) {
+      const MemoryMapEntry* entry = &config1->MemoryMapEntries[i];
+      DEBUG((DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n", entry->Start,
+             entry->End, entry->Type));
+      AddMemoryRange(MemInfoCallback, entry->Start, entry->End, entry->Type);
+    }
+  } else
 
-  config = GetUefiPayLoadConfig();
+      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
+    const char* cmdline = config->config.v2.cmdline;
+    const char* option;
+    uint64_t args[3];
 
-  DEBUG((DEBUG_INFO, "MemoryMap #entries: %d\n", config->NumMemoryMapEntries));
+    // look for the mem=start,end,type
+    while ((cmdline = cmdline_next(cmdline, &option))) {
+      if (strncmp(option, "mem=", 4) != 0) continue;
 
-  MemoryMapEntry* entry = &config->MemoryMapEntries[0];
-  for (i = 0; i < config->NumMemoryMapEntries; i++) {
-    DEBUG((DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n", entry->Start,
-           entry->End, entry->Type));
-    AddMemoryRange(MemInfoCallback, entry->Start, entry->End, entry->Type);
-    entry++;
+      if (cmdline_ints(option, args, 3) != 3) {
+        DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
+        continue;
+      }
+
+      const uint64_t start = args[0];
+      const uint64_t end = args[1];
+      const uint64_t type = args[2];
+
+      DEBUG(
+          (DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n", start, end, type));
+      AddMemoryRange(MemInfoCallback, start, end, type);
+    }
   }
+
   return RETURN_SUCCESS;
 }
 
@@ -96,14 +201,52 @@ ParseMemoryInfo(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN VOID* Params) {  RETURN_STATUS  EFIAPI  ParseSystemTable(OUT SYSTEM_TABLE_INFO* SystemTableInfo) {
-  UefiPayloadConfig* config;
+  const UefiPayloadConfig* config = GetUefiPayLoadConfig();  if 
+ (!config) {
+    DEBUG((DEBUG_ERROR,
+           "ParseSystemTable: Could not find UEFI Payload config\n"));
+    return RETURN_SUCCESS;
+  }
 
-  config = GetUefiPayLoadConfig();
-  SystemTableInfo->AcpiTableBase = config->AcpiBase;
-  SystemTableInfo->AcpiTableSize = config->AcpiSize;
+  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
+    const UefiPayloadConfigV1* config1 = &config->config.v1;
+    SystemTableInfo->AcpiTableBase = config1->AcpiBase;
+    SystemTableInfo->AcpiTableSize = config1->AcpiSize;
 
-  SystemTableInfo->SmbiosTableBase = config->SmbiosBase;
-  SystemTableInfo->SmbiosTableSize = config->SmbiosSize;
+    SystemTableInfo->SmbiosTableBase = config1->SmbiosBase;
+    SystemTableInfo->SmbiosTableSize = config1->SmbiosSize;  } else
+
+      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
+    const char* cmdline = config->config.v2.cmdline;
+    const char* option;
+    uint64_t args[2];
+
+    // look for the acpi config
+    while ((cmdline = cmdline_next(cmdline, &option))) {
+      if (strncmp(option, "ACPI20=", 7) == 0) {
+        const int count = cmdline_ints(option, args, 2);
+        if (count < 0) {
+          DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
+          continue;
+        }
+
+        if (count > 0) SystemTableInfo->AcpiTableBase = args[0];
+        if (count > 1) SystemTableInfo->AcpiTableSize = args[1];
+      }
+
+      if (strncmp(option, "SMBIOS=", 7) == 0) {
+        const int count = cmdline_ints(option, args, 2);
+        if (count < 0) {
+          DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
+          continue;
+        }
+
+        if (count > 0) SystemTableInfo->SmbiosTableBase = args[0];
+        if (count > 1) SystemTableInfo->SmbiosTableSize = args[1];
+      }
+    }
+  }
 
   return RETURN_SUCCESS;
 }
@@ -120,15 +263,52 @@ ParseSystemTable(OUT SYSTEM_TABLE_INFO* SystemTableInfo) {  RETURN_STATUS  EFIAPI  ParseSerialInfo(OUT SERIAL_PORT_INFO* SerialPortInfo) {
-  UefiPayloadConfig* config;
-  config = GetUefiPayLoadConfig();
-
-  SerialPortInfo->BaseAddr = config->SerialConfig.BaseAddr;
-  SerialPortInfo->RegWidth = config->SerialConfig.RegWidth;
-  SerialPortInfo->Type = config->SerialConfig.Type;
-  SerialPortInfo->Baud = config->SerialConfig.Baud;
-  SerialPortInfo->InputHertz = config->SerialConfig.InputHertz;
-  SerialPortInfo->UartPciAddr = config->SerialConfig.UartPciAddr;
+  // fill in some reasonable defaults
+  SerialPortInfo->BaseAddr = 0x3f8;
+  SerialPortInfo->RegWidth = 1;
+  SerialPortInfo->Type = 1;  // uefi.SerialPortTypeIO  
+ SerialPortInfo->Baud = 115200;  SerialPortInfo->InputHertz = 1843200;  
+ SerialPortInfo->UartPciAddr = 0;
+
+  const UefiPayloadConfig* config = GetUefiPayLoadConfig();  if 
+ (!config) {
+    DEBUG((DEBUG_ERROR, "ParseSerialInfo: using default config\n"));
+    return RETURN_SUCCESS;
+  }
+
+  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
+    const UefiPayloadConfigV1* config1 = &config->config.v1;
+    SerialPortInfo->BaseAddr = config1->SerialConfig.BaseAddr;
+    SerialPortInfo->RegWidth = config1->SerialConfig.RegWidth;
+    SerialPortInfo->Type = config1->SerialConfig.Type;
+    SerialPortInfo->Baud = config1->SerialConfig.Baud;
+    SerialPortInfo->InputHertz = config1->SerialConfig.InputHertz;
+    SerialPortInfo->UartPciAddr = config1->SerialConfig.UartPciAddr;
+  } else
+
+      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
+    const char* cmdline = config->config.v2.cmdline;
+    const char* option;
+    uint64_t args[6] = {};
+
+    while ((cmdline = cmdline_next(cmdline, &option))) {
+      if (strncmp(option, "serial=", 7) != 0) continue;
+
+      const int count = cmdline_ints(option, args, 6);
+      if (count < 0) {
+        DEBUG((DEBUG_ERROR, "Parse error: %a\n", option));
+        continue;
+      }
+
+      if (count > 0) SerialPortInfo->Baud = args[0];
+      if (count > 1) SerialPortInfo->BaseAddr = args[1];
+      if (count > 2) SerialPortInfo->RegWidth = args[2];
+      if (count > 3) SerialPortInfo->Type = args[3];
+      if (count > 4) SerialPortInfo->InputHertz = args[4];
+      if (count > 5) SerialPortInfo->UartPciAddr = args[5];
+    }
+  }
 
   return RETURN_SUCCESS;
 }
--
2.32.0.402.g57bb445576-goog

Re: [edk2-devel] [PATCH v1 6/6] UefiPayloadPkg: LinuxBoot: use a text format for the configuration block.

[Cheng-Chieh Huang]

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

Thanks for the information. I will drop this patch in Patch set v2 for now
and work with Trammell to revise this patch to fit EDK2 style.

--
Cheng-Chieh

On Wed, Aug 4, 2021 at 11:00 AM Dong, Guo <guo.dong@intel.com> wrote:

>
> Please run BaseTools\Scripts\PatchCheck.py to make sure it passed the
> check.
>
> Update LbParseLib.c following
> https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/.
> current code is more like Linux style. E.g.
> +static uint64_t parse_int(const char* s, char** end) {
> +  UINT64 x;
>
> Removed unused comments
> +//#include <string.h>
> +//#include <ctype.h>
>
>
> Thanks,
> Guo
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Cheng-Chieh Huang via groups.io
> Sent: Wednesday, July 21, 2021 6:23 AM
> To: devel@edk2.groups.io
> Cc: Trammell Hudson <hudson@trmm.net>; Cheng-Chieh Huang <
> chengchieh@google.com>
> Subject: [edk2-devel] [PATCH v1 6/6] UefiPayloadPkg: LinuxBoot: use a text
> format for the configuration block.
>
> From: Trammell Hudson <hudson@trmm.net>
>
> This adds a text command line for the UefiPayloadPkg that uses a textual
> magic number 'LnxBoot1' and a series of white-space separated
> key=value[,value...] pairs for the parameters.
>
> The v1 binary configuration structure is used if it is present instead for
> backwards compatability.
>
> Currently supported text command line arguments are are:
>
> * `serial=baud,base,width,type,hz,pci`
> (only `baud` needs to be specified, the rest have reasonable
> defaults)
>
> * `mem=start,end,type`
> Types are based on `/sys/firmware/memmaps/*/types`:
>     1 == "System RAM"
>     3 == "ACPI Tables"
>     4 == "ACPI Non-volatile Storage"
>     5 == "Reserved"
>
> * `ACPI20=base` pointer to RDSP (from `/sys/firmware/efi/systab`)
>
> * `SMBIOS=base` pointer to the SMBIOS table (also from the EFI table)
>
> Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
> ---
>  UefiPayloadPkg/Include/Linuxboot.h             |  17 +-
>  UefiPayloadPkg/Library/LbParseLib/LbParseLib.c | 252 +++++++++++++++++---
>  2 files changed, 230 insertions(+), 39 deletions(-)
>
> diff --git a/UefiPayloadPkg/Include/Linuxboot.h
> b/UefiPayloadPkg/Include/Linuxboot.h
> index 34ca18069983..56b39b5a09ff 100644
> --- a/UefiPayloadPkg/Include/Linuxboot.h
> +++ b/UefiPayloadPkg/Include/Linuxboot.h
> @@ -24,8 +24,7 @@ typedef struct MemoryMapEntryStruct {
>    UINT32 Type;
>  } MemoryMapEntry;
>
> -typedef struct UefiPayloadConfigStruct {
> -  UINT64 Version;
> +typedef struct {
>    UINT64 AcpiBase;
>    UINT64 AcpiSize;
>    UINT64 SmbiosBase;
> @@ -33,10 +32,22 @@ typedef struct UefiPayloadConfigStruct {
>    SerialPortConfig SerialConfig;
>    UINT32 NumMemoryMapEntries;
>    MemoryMapEntry MemoryMapEntries[0];
> +} UefiPayloadConfigV1;
> +
> +typedef struct UefiPayloadConfigStruct {
> +  UINT64 Version;
> +  union {
> +    UefiPayloadConfigV1 v1;
> +    struct {
> +      char cmdline[0]; // up to 64 KB
> +    } v2;
> +  } config;
>  } UefiPayloadConfig;
>  #pragma pack()
>
> -#define UEFI_PAYLOAD_CONFIG_VERSION 1
> +// magic version config is "LnxBoot1"
> +#define UEFI_PAYLOAD_CONFIG_VERSION1 1
> +#define UEFI_PAYLOAD_CONFIG_VERSION2 0x31746f6f42786e4cULL
>
>  #define LINUXBOOT_MEM_RAM 1
>  #define LINUXBOOT_MEM_DEFAULT 2
> diff --git a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> index 34bfb6a1073f..5e68091cac91 100644
> --- a/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> +++ b/UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> @@ -1,13 +1,12 @@
>  /** @file
> -  This library will parse the linuxboot table in memory and extract those
> required
> -  information.
> +  This library will parse the linuxboot table in memory and extract
> +those required information.
>
>    Copyright (c) 2021, the u-root Authors. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  **/
>
> -
>  #include <IndustryStandard/Acpi.h>
>  #include <IndustryStandard/SmBios.h>
>  #include <Library/BaseLib.h>
> @@ -18,17 +17,42 @@
>  #include <Library/PcdLib.h>
>  #include <Linuxboot.h>
>  #include <Uefi/UefiBaseType.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +//#include <string.h>
> +//#include <ctype.h>
> +
> +#define strncmp(a, b, n) AsciiStrnCmp((a), (b), (n))
> +
> +static uint64_t parse_int(const char* s, char** end) {
> +  UINT64 x;
> +
> +  if (s[0] == '0' && s[1] == 'x')
> +    AsciiStrHexToUint64S(s, end, &x);
> +  else
> +    AsciiStrDecimalToUint64S(s, end, &x);
> +
> +  return x;
> +}
> +
> +static int isspace(const char c) { return c == ' ' || c == '\t' || c ==
> +'\n'; }
>
>  // Retrieve UefiPayloadConfig from Linuxboot's uefiboot
> -UefiPayloadConfig* GetUefiPayLoadConfig() {
> -  UefiPayloadConfig* config =
> +const UefiPayloadConfig* GetUefiPayLoadConfig() {
> +  const UefiPayloadConfig* config =
>        (UefiPayloadConfig*)(UINTN)(PcdGet32(PcdPayloadFdMemBase) -
> SIZE_64KB);
> -  if (config->Version != UEFI_PAYLOAD_CONFIG_VERSION) {
> -    DEBUG((DEBUG_ERROR, "Expect payload config version: %d, but get %d\n",
> -           UEFI_PAYLOAD_CONFIG_VERSION, config->Version));
> -    CpuDeadLoop ();
> -  }
> -  return config;
> +
> +  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1 ||
> +      config->Version == UEFI_PAYLOAD_CONFIG_VERSION2)
> +    return config;
> +
> +  DEBUG((DEBUG_ERROR,
> +         "Expect payload config version %016lx or %016lx, but get
> %016lx\n",
> +         UEFI_PAYLOAD_CONFIG_VERSION1, UEFI_PAYLOAD_CONFIG_VERSION2,
> +         config->Version));
> +  CpuDeadLoop();
> +  while (1)
> +    ;
>  }
>
>  // Align the address and add memory rang to MemInfoCallback @@ -54,6
> +78,57 @@ void AddMemoryRange(IN BL_MEM_INFO_CALLBACK MemInfoCallback, IN
> UINTN start,
>    MemInfoCallback(&MemoryMap, NULL);
>  }
>
> +const char* cmdline_next(const char* cmdline, const char** option) {
> +  // at the end of the string, we're done
> +  if (!cmdline || *cmdline == '\0') return NULL;
> +
> +  // skip any leading whitespace
> +  while (isspace(*cmdline)) cmdline++;
> +
> +  // if we've hit the end of the string, we're done  if (*cmdline ==
> + '\0') return NULL;
> +
> +  *option = cmdline;
> +
> +  // find the end of this option or the string  while
> + (!isspace(*cmdline) && *cmdline != '\0') cmdline++;
> +
> +  // cmdline points to the whitespace or end of string
> +  return cmdline;
> +}
> +
> +int cmdline_ints(const char* option, uint64_t args[], int max) {
> +  // skip any leading text up to an '='
> +  const char* s = option;
> +  while (1) {
> +    const char c = *s++;
> +    if (c == '=') break;
> +
> +    if (c == '\0' || isspace(c)) {
> +      s = option;
> +      break;
> +    }
> +  }
> +
> +  for (int i = 0; i < max; i++) {
> +    char* end;
> +    args[i] = parse_int(s, &end);
> +
> +    // end of string or end of the option?
> +    if (*end == '\0' || isspace(*end)) return i + 1;
> +
> +    // not separator? signal an error if we have consumed any ints,
> +    // otherwise return 0 saying that none were found
> +    if (*end != ',') return i == 0 ? 0 : -1;
> +
> +    // skip the , and get the next value
> +    s = end + 1;
> +  }
> +
> +  // too many values!
> +  return -1;
> +}
> +
>  /**
>    Acquire the memory information from the linuxboot table in memory.
>
> @@ -67,20 +142,50 @@ void AddMemoryRange(IN BL_MEM_INFO_CALLBACK
> MemInfoCallback, IN UINTN start,  RETURN_STATUS  EFIAPI  ParseMemoryInfo(IN
> BL_MEM_INFO_CALLBACK MemInfoCallback, IN VOID* Params) {
> -  UefiPayloadConfig* config;
> -  int i;
> +  const UefiPayloadConfig* config = GetUefiPayLoadConfig();  if
> + (!config) {
> +    DEBUG(
> +        (DEBUG_ERROR, "ParseMemoryInfo: Could not find UEFI Payload
> config\n"));
> +    return RETURN_SUCCESS;
> +  }
> +
> +  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
> +    const UefiPayloadConfigV1* config1 = &config->config.v1;
> +    DEBUG(
> +        (DEBUG_INFO, "MemoryMap #entries: %d\n",
> + config1->NumMemoryMapEntries));
> +
> +    for (int i = 0; i < config1->NumMemoryMapEntries; i++) {
> +      const MemoryMapEntry* entry = &config1->MemoryMapEntries[i];
> +      DEBUG((DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n",
> entry->Start,
> +             entry->End, entry->Type));
> +      AddMemoryRange(MemInfoCallback, entry->Start, entry->End,
> entry->Type);
> +    }
> +  } else
>
> -  config = GetUefiPayLoadConfig();
> +      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
> +    const char* cmdline = config->config.v2.cmdline;
> +    const char* option;
> +    uint64_t args[3];
>
> -  DEBUG((DEBUG_INFO, "MemoryMap #entries: %d\n",
> config->NumMemoryMapEntries));
> +    // look for the mem=start,end,type
> +    while ((cmdline = cmdline_next(cmdline, &option))) {
> +      if (strncmp(option, "mem=", 4) != 0) continue;
>
> -  MemoryMapEntry* entry = &config->MemoryMapEntries[0];
> -  for (i = 0; i < config->NumMemoryMapEntries; i++) {
> -    DEBUG((DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n", entry->Start,
> -           entry->End, entry->Type));
> -    AddMemoryRange(MemInfoCallback, entry->Start, entry->End,
> entry->Type);
> -    entry++;
> +      if (cmdline_ints(option, args, 3) != 3) {
> +        DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
> +        continue;
> +      }
> +
> +      const uint64_t start = args[0];
> +      const uint64_t end = args[1];
> +      const uint64_t type = args[2];
> +
> +      DEBUG(
> +          (DEBUG_INFO, "Start: 0x%lx End: 0x%lx Type:%d\n", start, end,
> type));
> +      AddMemoryRange(MemInfoCallback, start, end, type);
> +    }
>    }
> +
>    return RETURN_SUCCESS;
>  }
>
> @@ -96,14 +201,52 @@ ParseMemoryInfo(IN BL_MEM_INFO_CALLBACK
> MemInfoCallback, IN VOID* Params) {  RETURN_STATUS  EFIAPI
> ParseSystemTable(OUT SYSTEM_TABLE_INFO* SystemTableInfo) {
> -  UefiPayloadConfig* config;
> +  const UefiPayloadConfig* config = GetUefiPayLoadConfig();  if
> + (!config) {
> +    DEBUG((DEBUG_ERROR,
> +           "ParseSystemTable: Could not find UEFI Payload config\n"));
> +    return RETURN_SUCCESS;
> +  }
>
> -  config = GetUefiPayLoadConfig();
> -  SystemTableInfo->AcpiTableBase = config->AcpiBase;
> -  SystemTableInfo->AcpiTableSize = config->AcpiSize;
> +  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
> +    const UefiPayloadConfigV1* config1 = &config->config.v1;
> +    SystemTableInfo->AcpiTableBase = config1->AcpiBase;
> +    SystemTableInfo->AcpiTableSize = config1->AcpiSize;
>
> -  SystemTableInfo->SmbiosTableBase = config->SmbiosBase;
> -  SystemTableInfo->SmbiosTableSize = config->SmbiosSize;
> +    SystemTableInfo->SmbiosTableBase = config1->SmbiosBase;
> +    SystemTableInfo->SmbiosTableSize = config1->SmbiosSize;  } else
> +
> +      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
> +    const char* cmdline = config->config.v2.cmdline;
> +    const char* option;
> +    uint64_t args[2];
> +
> +    // look for the acpi config
> +    while ((cmdline = cmdline_next(cmdline, &option))) {
> +      if (strncmp(option, "ACPI20=", 7) == 0) {
> +        const int count = cmdline_ints(option, args, 2);
> +        if (count < 0) {
> +          DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
> +          continue;
> +        }
> +
> +        if (count > 0) SystemTableInfo->AcpiTableBase = args[0];
> +        if (count > 1) SystemTableInfo->AcpiTableSize = args[1];
> +      }
> +
> +      if (strncmp(option, "SMBIOS=", 7) == 0) {
> +        const int count = cmdline_ints(option, args, 2);
> +        if (count < 0) {
> +          DEBUG((DEBUG_ERROR, "Parse error: '%a'\n", option));
> +          continue;
> +        }
> +
> +        if (count > 0) SystemTableInfo->SmbiosTableBase = args[0];
> +        if (count > 1) SystemTableInfo->SmbiosTableSize = args[1];
> +      }
> +    }
> +  }
>
>    return RETURN_SUCCESS;
>  }
> @@ -120,15 +263,52 @@ ParseSystemTable(OUT SYSTEM_TABLE_INFO*
> SystemTableInfo) {  RETURN_STATUS  EFIAPI  ParseSerialInfo(OUT
> SERIAL_PORT_INFO* SerialPortInfo) {
> -  UefiPayloadConfig* config;
> -  config = GetUefiPayLoadConfig();
> -
> -  SerialPortInfo->BaseAddr = config->SerialConfig.BaseAddr;
> -  SerialPortInfo->RegWidth = config->SerialConfig.RegWidth;
> -  SerialPortInfo->Type = config->SerialConfig.Type;
> -  SerialPortInfo->Baud = config->SerialConfig.Baud;
> -  SerialPortInfo->InputHertz = config->SerialConfig.InputHertz;
> -  SerialPortInfo->UartPciAddr = config->SerialConfig.UartPciAddr;
> +  // fill in some reasonable defaults
> +  SerialPortInfo->BaseAddr = 0x3f8;
> +  SerialPortInfo->RegWidth = 1;
> +  SerialPortInfo->Type = 1;  // uefi.SerialPortTypeIO
> + SerialPortInfo->Baud = 115200;  SerialPortInfo->InputHertz = 1843200;
> + SerialPortInfo->UartPciAddr = 0;
> +
> +  const UefiPayloadConfig* config = GetUefiPayLoadConfig();  if
> + (!config) {
> +    DEBUG((DEBUG_ERROR, "ParseSerialInfo: using default config\n"));
> +    return RETURN_SUCCESS;
> +  }
> +
> +  if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION1) {
> +    const UefiPayloadConfigV1* config1 = &config->config.v1;
> +    SerialPortInfo->BaseAddr = config1->SerialConfig.BaseAddr;
> +    SerialPortInfo->RegWidth = config1->SerialConfig.RegWidth;
> +    SerialPortInfo->Type = config1->SerialConfig.Type;
> +    SerialPortInfo->Baud = config1->SerialConfig.Baud;
> +    SerialPortInfo->InputHertz = config1->SerialConfig.InputHertz;
> +    SerialPortInfo->UartPciAddr = config1->SerialConfig.UartPciAddr;
> +  } else
> +
> +      if (config->Version == UEFI_PAYLOAD_CONFIG_VERSION2) {
> +    const char* cmdline = config->config.v2.cmdline;
> +    const char* option;
> +    uint64_t args[6] = {};
> +
> +    while ((cmdline = cmdline_next(cmdline, &option))) {
> +      if (strncmp(option, "serial=", 7) != 0) continue;
> +
> +      const int count = cmdline_ints(option, args, 6);
> +      if (count < 0) {
> +        DEBUG((DEBUG_ERROR, "Parse error: %a\n", option));
> +        continue;
> +      }
> +
> +      if (count > 0) SerialPortInfo->Baud = args[0];
> +      if (count > 1) SerialPortInfo->BaseAddr = args[1];
> +      if (count > 2) SerialPortInfo->RegWidth = args[2];
> +      if (count > 3) SerialPortInfo->Type = args[3];
> +      if (count > 4) SerialPortInfo->InputHertz = args[4];
> +      if (count > 5) SerialPortInfo->UartPciAddr = args[5];
> +    }
> +  }
>
>    return RETURN_SUCCESS;
>  }
> --
> 2.32.0.402.g57bb445576-goog
>
>
>
> 
>
>
>

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

回复: [edk2-devel] [PATCH 0/6] UefiPayloadPkg: LinuxBoot Support in UefiPayload

[gaoliming]

This is new feature. Can you submit one BZ (https://bugzilla.tianocore.org/ ) for it? 

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Cheng-Chieh
> Huang via groups.io
> 发送时间: 2021年7月21日 21:23
> 收件人: devel@edk2.groups.io
> 抄送: Cheng-Chieh Huang <chengchieh@google.com>; Daniel Schaefer
> <daniel.schaefer@hpe.com>; Trammell Hudson <hudson@trmm.net>;
> Maurice Ma <maurice.ma@intel.com>; Guo Dong <guo.dong@intel.com>;
> Benjamin You <benjamin.you@intel.com>
> 主题: [edk2-devel] [PATCH 0/6] UefiPayloadPkg: LinuxBoot Support in
> UefiPayload
> 
> These are necessary patches to Support LinuxBoot in UefiPayload.
> With these paches, we can boot to ESXi and Windows from a linux in QEMU.
> 
> LinuxBoot README:
> https://github.com/linuxboot/edk2/blob/uefipayload/UefiPayloadPkg/READM
> E.md
> 
> PR to tianocore:
> https://github.com/tianocore/edk2/pull/1820
> 
> Cheng-Chieh Huang (5):
>   Add LINUXBOOT payload target
>   Use legacy timer in Linuxboot payload
>   Update maximum logic processor to 256
>   Reserve Payload config in runtime services data
>   Add DISABLE_MMX_SSE to avoid generating floating points operation
> 
> Trammell Hudson (1):
>   LinuxBoot: use a text format for the configuration block.
> 
>  UefiPayloadPkg/UefiPayloadPkg.dsc             |  29 +-
>  UefiPayloadPkg/UefiPayloadPkg.fdf             |   5 +
>  .../Library/LbParseLib/LbParseLib.inf         |  39 ++
>  UefiPayloadPkg/Include/Linuxboot.h            |  58 +++
>  .../Library/LbParseLib/LbParseLib.c           | 348
> ++++++++++++++++++
>  .../PciHostBridgeLib/PciHostBridgeSupport.c   |   6 +-
>  .../UefiPayloadEntry/UefiPayloadEntry.c       |   2 +
>  CryptoPkg/Library/OpensslLib/openssl          |   2 +-
>  8 files changed, 480 insertions(+), 9 deletions(-)
>  create mode 100644 UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf
>  create mode 100644 UefiPayloadPkg/Include/Linuxboot.h
>  create mode 100644 UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> 
> Cc: Cheng-Chieh Huang <chengchieh@google.com>
> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> Cc: Trammell Hudson <hudson@trmm.net>
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Guo Dong <guo.dong@intel.com>
> Cc: Benjamin You <benjamin.you@intel.com>
> --
> 2.32.0.402.g57bb445576-goog
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 0/6] UefiPayloadPkg: LinuxBoot Support in UefiPayload

[Cheng-Chieh Huang]

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

Bug created: https://bugzilla.tianocore.org/show_bug.cgi?id=3505

On Thu, Jul 22, 2021 at 9:29 AM gaoliming <gaoliming@byosoft.com.cn> wrote:

> This is new feature. Can you submit one BZ (
> https://bugzilla.tianocore.org/ ) for it?
>
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Cheng-Chieh
> > Huang via groups.io
> > 发送时间: 2021年7月21日 21:23
> > 收件人: devel@edk2.groups.io
> > 抄送: Cheng-Chieh Huang <chengchieh@google.com>; Daniel Schaefer
> > <daniel.schaefer@hpe.com>; Trammell Hudson <hudson@trmm.net>;
> > Maurice Ma <maurice.ma@intel.com>; Guo Dong <guo.dong@intel.com>;
> > Benjamin You <benjamin.you@intel.com>
> > 主题: [edk2-devel] [PATCH 0/6] UefiPayloadPkg: LinuxBoot Support in
> > UefiPayload
> >
> > These are necessary patches to Support LinuxBoot in UefiPayload.
> > With these paches, we can boot to ESXi and Windows from a linux in QEMU.
> >
> > LinuxBoot README:
> > https://github.com/linuxboot/edk2/blob/uefipayload/UefiPayloadPkg/READM
> > E.md
> >
> > PR to tianocore:
> > https://github.com/tianocore/edk2/pull/1820
> >
> > Cheng-Chieh Huang (5):
> >   Add LINUXBOOT payload target
> >   Use legacy timer in Linuxboot payload
> >   Update maximum logic processor to 256
> >   Reserve Payload config in runtime services data
> >   Add DISABLE_MMX_SSE to avoid generating floating points operation
> >
> > Trammell Hudson (1):
> >   LinuxBoot: use a text format for the configuration block.
> >
> >  UefiPayloadPkg/UefiPayloadPkg.dsc             |  29 +-
> >  UefiPayloadPkg/UefiPayloadPkg.fdf             |   5 +
> >  .../Library/LbParseLib/LbParseLib.inf         |  39 ++
> >  UefiPayloadPkg/Include/Linuxboot.h            |  58 +++
> >  .../Library/LbParseLib/LbParseLib.c           | 348
> > ++++++++++++++++++
> >  .../PciHostBridgeLib/PciHostBridgeSupport.c   |   6 +-
> >  .../UefiPayloadEntry/UefiPayloadEntry.c       |   2 +
> >  CryptoPkg/Library/OpensslLib/openssl          |   2 +-
> >  8 files changed, 480 insertions(+), 9 deletions(-)
> >  create mode 100644 UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf
> >  create mode 100644 UefiPayloadPkg/Include/Linuxboot.h
> >  create mode 100644 UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> >
> > Cc: Cheng-Chieh Huang <chengchieh@google.com>
> > Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> > Cc: Trammell Hudson <hudson@trmm.net>
> > Cc: Maurice Ma <maurice.ma@intel.com>
> > Cc: Guo Dong <guo.dong@intel.com>
> > Cc: Benjamin You <benjamin.you@intel.com>
> > --
> > 2.32.0.402.g57bb445576-goog
> >
> >
> >
> > 
> >
>
>
>
>

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

Re: [edk2-devel] [PATCH 0/6] UefiPayloadPkg: LinuxBoot Support in UefiPayload

[Ni, Ray]

Cheng-Chieh,
Thanks for the detailed explanation in doc https://docs.google.com/document/d/1mU6ICHTh0ot8U45uuRENKOGI8cVzizdyWHGYHpEguVg/edit#heading=h.xzptrog8pyxf.

My original thought was LinuxBoot is a payload that aims to boot OS.
But the idea of chaining UefiPayload producing UEFI services is very brilliant.

Have you considered to produce the universal payload interfaces (https://universalpayload.github.io/documentation/) from LinuxBoot so no LbParseLib is required?

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
> Sent: Wednesday, July 21, 2021 9:23 PM
> To: devel@edk2.groups.io
> Cc: Cheng-Chieh Huang <chengchieh@google.com>; Schaefer, Daniel <daniel.schaefer@hpe.com>; Trammell Hudson
> <hudson@trmm.net>; Ma, Maurice <maurice.ma@intel.com>; Dong, Guo <guo.dong@intel.com>; You, Benjamin
> <benjamin.you@intel.com>
> Subject: [edk2-devel] [PATCH 0/6] UefiPayloadPkg: LinuxBoot Support in UefiPayload
> 
> These are necessary patches to Support LinuxBoot in UefiPayload.
> With these paches, we can boot to ESXi and Windows from a linux in QEMU.
> 
> LinuxBoot README:
> https://github.com/linuxboot/edk2/blob/uefipayload/UefiPayloadPkg/README.md
> 
> PR to tianocore:
> https://github.com/tianocore/edk2/pull/1820
> 
> Cheng-Chieh Huang (5):
>   Add LINUXBOOT payload target
>   Use legacy timer in Linuxboot payload
>   Update maximum logic processor to 256
>   Reserve Payload config in runtime services data
>   Add DISABLE_MMX_SSE to avoid generating floating points operation
> 
> Trammell Hudson (1):
>   LinuxBoot: use a text format for the configuration block.
> 
>  UefiPayloadPkg/UefiPayloadPkg.dsc             |  29 +-
>  UefiPayloadPkg/UefiPayloadPkg.fdf             |   5 +
>  .../Library/LbParseLib/LbParseLib.inf         |  39 ++
>  UefiPayloadPkg/Include/Linuxboot.h            |  58 +++
>  .../Library/LbParseLib/LbParseLib.c           | 348 ++++++++++++++++++
>  .../PciHostBridgeLib/PciHostBridgeSupport.c   |   6 +-
>  .../UefiPayloadEntry/UefiPayloadEntry.c       |   2 +
>  CryptoPkg/Library/OpensslLib/openssl          |   2 +-
>  8 files changed, 480 insertions(+), 9 deletions(-)
>  create mode 100644 UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf
>  create mode 100644 UefiPayloadPkg/Include/Linuxboot.h
>  create mode 100644 UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> 
> Cc: Cheng-Chieh Huang <chengchieh@google.com>
> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> Cc: Trammell Hudson <hudson@trmm.net>
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Guo Dong <guo.dong@intel.com>
> Cc: Benjamin You <benjamin.you@intel.com>
> --
> 2.32.0.402.g57bb445576-goog
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 0/6] UefiPayloadPkg: LinuxBoot Support in UefiPayload

[Daniel Schaefer]

On 7/22/21 9:44 AM, Ni, Ray wrote:
> Cheng-Chieh,
> Thanks for the detailed explanation in doc https://docs.google.com/document/d/1mU6ICHTh0ot8U45uuRENKOGI8cVzizdyWHGYHpEguVg/edit#heading=h.xzptrog8pyxf .
> 
> My original thought was LinuxBoot is a payload that aims to boot OS.
> But the idea of chaining UefiPayload producing UEFI services is very brilliant.

It can be and it is. The main usage of LinuxBoot is to load/boot another 
Linux using the kexec mechanism. But in order to be able to boot 
Windows, ESXI, ... a UEFI interface is required.
There have been a few proposals and POCs (like implementing UEFI 
services in Linux) but UefiPayload is the most practical and easy way to 
do it, for now.

> Have you considered to produce the universal payload interfaces (https://universalpayload.github.io/documentation/ ) from LinuxBoot so no LbParseLib is required?

I don't think we've looked at it. But we liked it to be a string because 
it allows easy forward compatibility and not having to recreate the 
structs in higher-level languages like Go.

> Thanks,
> Ray
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
>> Sent: Wednesday, July 21, 2021 9:23 PM
>> To: devel@edk2.groups.io
>> Cc: Cheng-Chieh Huang <chengchieh@google.com>; Schaefer, Daniel <daniel.schaefer@hpe.com>; Trammell Hudson
>> <hudson@trmm.net>; Ma, Maurice <maurice.ma@intel.com>; Dong, Guo <guo.dong@intel.com>; You, Benjamin
>> <benjamin.you@intel.com>
>> Subject: [edk2-devel] [PATCH 0/6] UefiPayloadPkg: LinuxBoot Support in UefiPayload
>>
>> These are necessary patches to Support LinuxBoot in UefiPayload.
>> With these paches, we can boot to ESXi and Windows from a linux in QEMU.
>>
>> LinuxBoot README:
>> https://github.com/linuxboot/edk2/blob/uefipayload/UefiPayloadPkg/README.md
>>
>> PR to tianocore:
>> https://github.com/tianocore/edk2/pull/1820
>>
>> Cheng-Chieh Huang (5):
>>    Add LINUXBOOT payload target
>>    Use legacy timer in Linuxboot payload
>>    Update maximum logic processor to 256
>>    Reserve Payload config in runtime services data
>>    Add DISABLE_MMX_SSE to avoid generating floating points operation
>>
>> Trammell Hudson (1):
>>    LinuxBoot: use a text format for the configuration block.
>>
>>   UefiPayloadPkg/UefiPayloadPkg.dsc             |  29 +-
>>   UefiPayloadPkg/UefiPayloadPkg.fdf             |   5 +
>>   .../Library/LbParseLib/LbParseLib.inf         |  39 ++
>>   UefiPayloadPkg/Include/Linuxboot.h            |  58 +++
>>   .../Library/LbParseLib/LbParseLib.c           | 348 ++++++++++++++++++
>>   .../PciHostBridgeLib/PciHostBridgeSupport.c   |   6 +-
>>   .../UefiPayloadEntry/UefiPayloadEntry.c       |   2 +
>>   CryptoPkg/Library/OpensslLib/openssl          |   2 +-
>>   8 files changed, 480 insertions(+), 9 deletions(-)
>>   create mode 100644 UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf
>>   create mode 100644 UefiPayloadPkg/Include/Linuxboot.h
>>   create mode 100644 UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
>>
>> Cc: Cheng-Chieh Huang <chengchieh@google.com>
>> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
>> Cc: Trammell Hudson <hudson@trmm.net>
>> Cc: Maurice Ma <maurice.ma@intel.com>
>> Cc: Guo Dong <guo.dong@intel.com>
>> Cc: Benjamin You <benjamin.you@intel.com>
>> --
>> 2.32.0.402.g57bb445576-goog
>>
>>
>>
>> 
>>
> 

Re: [edk2-devel] [PATCH 0/6] UefiPayloadPkg: LinuxBoot Support in UefiPayload

[Cheng-Chieh Huang]

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

On Thu, Jul 22, 2021 at 9:59 AM Daniel Schaefer <daniel.schaefer@hpe.com>
wrote:

> On 7/22/21 9:44 AM, Ni, Ray wrote:
> > Cheng-Chieh,
> > Thanks for the detailed explanation in doc
> https://docs.google.com/document/d/1mU6ICHTh0ot8U45uuRENKOGI8cVzizdyWHGYHpEguVg/edit#heading=h.xzptrog8pyxf
> .
> >
> > My original thought was LinuxBoot is a payload that aims to boot OS.
> > But the idea of chaining UefiPayload producing UEFI services is very
> brilliant.
>
> It can be and it is. The main usage of LinuxBoot is to load/boot another
> Linux using the kexec mechanism. But in order to be able to boot
> Windows, ESXI, ... a UEFI interface is required.
> There have been a few proposals and POCs (like implementing UEFI
> services in Linux) but UefiPayload is the most practical and easy way to
> do it, for now.
>
> > Have you considered to produce the universal payload interfaces (
> https://universalpayload.github.io/documentation/ ) from LinuxBoot so no
> LbParseLib is required?
>
> I don't think we've looked at it. But we liked it to be a string because
> it allows easy forward compatibility and not having to recreate the
> structs in higher-level languages like Go.
>
> Agree to Daniel. In the future, we can look for supporting HOB structure
in u-root, but for now, we prefer to stick with the current approach.

--
Cheng-Chieh


> > Thanks,
> > Ray
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Cheng-Chieh Huang via groups.io
> >> Sent: Wednesday, July 21, 2021 9:23 PM
> >> To: devel@edk2.groups.io
> >> Cc: Cheng-Chieh Huang <chengchieh@google.com>; Schaefer, Daniel <
> daniel.schaefer@hpe.com>; Trammell Hudson
> >> <hudson@trmm.net>; Ma, Maurice <maurice.ma@intel.com>; Dong, Guo <
> guo.dong@intel.com>; You, Benjamin
> >> <benjamin.you@intel.com>
> >> Subject: [edk2-devel] [PATCH 0/6] UefiPayloadPkg: LinuxBoot Support in
> UefiPayload
> >>
> >> These are necessary patches to Support LinuxBoot in UefiPayload.
> >> With these paches, we can boot to ESXi and Windows from a linux in QEMU.
> >>
> >> LinuxBoot README:
> >>
> https://github.com/linuxboot/edk2/blob/uefipayload/UefiPayloadPkg/README.md
> >>
> >> PR to tianocore:
> >> https://github.com/tianocore/edk2/pull/1820
> >>
> >> Cheng-Chieh Huang (5):
> >>    Add LINUXBOOT payload target
> >>    Use legacy timer in Linuxboot payload
> >>    Update maximum logic processor to 256
> >>    Reserve Payload config in runtime services data
> >>    Add DISABLE_MMX_SSE to avoid generating floating points operation
> >>
> >> Trammell Hudson (1):
> >>    LinuxBoot: use a text format for the configuration block.
> >>
> >>   UefiPayloadPkg/UefiPayloadPkg.dsc             |  29 +-
> >>   UefiPayloadPkg/UefiPayloadPkg.fdf             |   5 +
> >>   .../Library/LbParseLib/LbParseLib.inf         |  39 ++
> >>   UefiPayloadPkg/Include/Linuxboot.h            |  58 +++
> >>   .../Library/LbParseLib/LbParseLib.c           | 348 ++++++++++++++++++
> >>   .../PciHostBridgeLib/PciHostBridgeSupport.c   |   6 +-
> >>   .../UefiPayloadEntry/UefiPayloadEntry.c       |   2 +
> >>   CryptoPkg/Library/OpensslLib/openssl          |   2 +-
> >>   8 files changed, 480 insertions(+), 9 deletions(-)
> >>   create mode 100644 UefiPayloadPkg/Library/LbParseLib/LbParseLib.inf
> >>   create mode 100644 UefiPayloadPkg/Include/Linuxboot.h
> >>   create mode 100644 UefiPayloadPkg/Library/LbParseLib/LbParseLib.c
> >>
> >> Cc: Cheng-Chieh Huang <chengchieh@google.com>
> >> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> >> Cc: Trammell Hudson <hudson@trmm.net>
> >> Cc: Maurice Ma <maurice.ma@intel.com>
> >> Cc: Guo Dong <guo.dong@intel.com>
> >> Cc: Benjamin You <benjamin.you@intel.com>
> >> --
> >> 2.32.0.402.g57bb445576-goog
> >>
> >>
> >>
> >> 
> >>
> >
>

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