Date   

Event: TianoCore Community Meeting - EMEA / NAMO - 06/10/2021 #cal-reminder

devel@edk2.groups.io Calendar <noreply@...>
 

Reminder: TianoCore Community Meeting - EMEA / NAMO

When:
06/10/2021
9:00am to 10:00am
(UTC-07:00) America/Los Angeles

Where:
https://teams.microsoft.com/l/meetup-join/19%3ameeting_N2UyMTVhZjUtOTk3Ni00MmI0LTg0NmItNzIwYTkyMGJhYzNh%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%22b286b53a-1218-4db3-bfc9-3d4c5aa7669e%22%7d

Organizer: Soumya Guptha

View Event

Description:

Microsoft Teams meeting

Join on your computer or mobile app

Click here to join the meeting

Join with a video conferencing device

teams@...

Video Conference ID: 111 422 379 4

Alternate VTC dialing instructions

Or call in (audio only)

+1 916-245-6934,,482062805#   United States, Sacramento

Phone Conference ID: 482 062 805#

Find a local number | Reset PIN

Learn More | Meeting options


Re: [PATCH 1/1] ArmPkg: Move cache defs used in Universal/Smbios into ArmLib.h

Ard Biesheuvel
 

On Tue, 8 Jun 2021 at 15:54, Rebecca Cran <rebecca@nuviainc.com> wrote:

Many of the cache definitions in ArmLibPrivate.h are being used outside
of ArmLib, in Universal/Smbios. Move them into ArmLib.h to make them
public, and remove the include of ArmLibPrivate.h from files in
Universal/Smbios.

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
Hi Rebecca,

If these definitions describe anything more than the software
interface exposed by the library, they really belong under
IndustryStandard/ not Library.


---
ArmPkg/Include/Library/ArmLib.h | 128 +++++++++++++++++++-
ArmPkg/Library/ArmLib/ArmLibPrivate.h | 123 -------------------
ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c | 1 -
ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c | 1 -
ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c | 1 -
ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c | 1 -
6 files changed, 127 insertions(+), 128 deletions(-)

diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index 5c232d779c83..0052adaa70aa 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -2,7 +2,7 @@

Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
Copyright (c) 2011 - 2016, ARM Ltd. All rights reserved.<BR>
- Copyright (c) 2020, NUVIA Inc. All rights reserved.<BR>
+ Copyright (c) 2020 - 2021, NUVIA Inc. All rights reserved.<BR>

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

@@ -113,6 +113,132 @@ typedef enum {
// to 7 levels of cache, L1 through L7.
#define MAX_ARM_CACHE_LEVEL 7

+/// Defines the structure of the CSSELR (Cache Size Selection) register
+typedef union {
+ struct {
+ UINT32 InD :1; ///< Instruction not Data bit
+ UINT32 Level :3; ///< Cache level (zero based)
+ UINT32 TnD :1; ///< Allocation not Data bit
+ UINT32 Reserved :27; ///< Reserved, RES0
+ } Bits; ///< Bitfield definition of the register
+ UINT32 Data; ///< The entire 32-bit value
+} CSSELR_DATA;
+
+/// The cache type values for the InD field of the CSSELR register
+typedef enum
+{
+ /// Select the data or unified cache
+ CsselrCacheTypeDataOrUnified = 0,
+ /// Select the instruction cache
+ CsselrCacheTypeInstruction,
+ CsselrCacheTypeMax
+} CSSELR_CACHE_TYPE;
+
+/// Defines the structure of the CCSIDR (Current Cache Size ID) register
+typedef union {
+ struct {
+ UINT64 LineSize :3; ///< Line size (Log2(Num bytes in cache) - 4)
+ UINT64 Associativity :10; ///< Associativity - 1
+ UINT64 NumSets :15; ///< Number of sets in the cache -1
+ UINT64 Unknown :4; ///< Reserved, UNKNOWN
+ UINT64 Reserved :32; ///< Reserved, RES0
+ } BitsNonCcidx; ///< Bitfield definition of the register when FEAT_CCIDX is not supported.
+ struct {
+ UINT64 LineSize :3; ///< Line size (Log2(Num bytes in cache) - 4)
+ UINT64 Associativity :21; ///< Associativity - 1
+ UINT64 Reserved1 :8; ///< Reserved, RES0
+ UINT64 NumSets :24; ///< Number of sets in the cache -1
+ UINT64 Reserved2 :8; ///< Reserved, RES0
+ } BitsCcidxAA64; ///< Bitfield definition of the register when FEAT_IDX is supported.
+ struct {
+ UINT64 LineSize : 3;
+ UINT64 Associativity : 21;
+ UINT64 Reserved : 8;
+ UINT64 Unallocated : 32;
+ } BitsCcidxAA32;
+ UINT64 Data; ///< The entire 64-bit value
+} CCSIDR_DATA;
+
+/// Defines the structure of the AARCH32 CCSIDR2 register.
+typedef union {
+ struct {
+ UINT32 NumSets :24; ///< Number of sets in the cache - 1
+ UINT32 Reserved :8; ///< Reserved, RES0
+ } Bits; ///< Bitfield definition of the register
+ UINT32 Data; ///< The entire 32-bit value
+} CCSIDR2_DATA;
+
+/** Defines the structure of the CLIDR (Cache Level ID) register.
+ *
+ * The lower 32 bits are the same for both AARCH32 and AARCH64
+ * so we can use the same structure for both.
+**/
+typedef union {
+ struct {
+ UINT32 Ctype1 : 3; ///< Level 1 cache type
+ UINT32 Ctype2 : 3; ///< Level 2 cache type
+ UINT32 Ctype3 : 3; ///< Level 3 cache type
+ UINT32 Ctype4 : 3; ///< Level 4 cache type
+ UINT32 Ctype5 : 3; ///< Level 5 cache type
+ UINT32 Ctype6 : 3; ///< Level 6 cache type
+ UINT32 Ctype7 : 3; ///< Level 7 cache type
+ UINT32 LoUIS : 3; ///< Level of Unification Inner Shareable
+ UINT32 LoC : 3; ///< Level of Coherency
+ UINT32 LoUU : 3; ///< Level of Unification Uniprocessor
+ UINT32 Icb : 3; ///< Inner Cache Boundary
+ } Bits; ///< Bitfield definition of the register
+ UINT32 Data; ///< The entire 32-bit value
+} CLIDR_DATA;
+
+/// The cache types reported in the CLIDR register.
+typedef enum {
+ /// No cache is present
+ ClidrCacheTypeNone = 0,
+ /// There is only an instruction cache
+ ClidrCacheTypeInstructionOnly,
+ /// There is only a data cache
+ ClidrCacheTypeDataOnly,
+ /// There are separate data and instruction caches
+ ClidrCacheTypeSeparate,
+ /// There is a unified cache
+ ClidrCacheTypeUnified,
+ ClidrCacheTypeMax
+} CLIDR_CACHE_TYPE;
+
+#define CLIDR_GET_CACHE_TYPE(x, level) ((x >> (3 * (level))) & 0b111)
+
+/** Reads the CCSIDR register for the specified cache.
+
+ @param CSSELR The CSSELR cache selection register value.
+
+ @return The contents of the CCSIDR_EL1 register for the specified cache, when in AARCH64 mode.
+ Returns the contents of the CCSIDR register in AARCH32 mode.
+**/
+UINTN
+ReadCCSIDR (
+ IN UINT32 CSSELR
+ );
+
+/** Reads the CCSIDR2 for the specified cache.
+
+ @param CSSELR The CSSELR cache selection register value
+
+ @return The contents of the CCSIDR2 register for the specified cache.
+**/
+UINT32
+ReadCCSIDR2 (
+ IN UINT32 CSSELR
+ );
+
+/** Reads the Cache Level ID (CLIDR) register.
+
+ @return The contents of the CLIDR_EL1 register.
+**/
+UINT32
+ReadCLIDR (
+ VOID
+ );
+
UINTN
EFIAPI
ArmDataCacheLineLength (
diff --git a/ArmPkg/Library/ArmLib/ArmLibPrivate.h b/ArmPkg/Library/ArmLib/ArmLibPrivate.h
index 5db83d620bfc..668aefd6a088 100644
--- a/ArmPkg/Library/ArmLib/ArmLibPrivate.h
+++ b/ArmPkg/Library/ArmLib/ArmLibPrivate.h
@@ -52,101 +52,6 @@
#define CACHE_ARCHITECTURE_UNIFIED (0UL)
#define CACHE_ARCHITECTURE_SEPARATE (1UL)

-
-/// Defines the structure of the CSSELR (Cache Size Selection) register
-typedef union {
- struct {
- UINT32 InD :1; ///< Instruction not Data bit
- UINT32 Level :3; ///< Cache level (zero based)
- UINT32 TnD :1; ///< Allocation not Data bit
- UINT32 Reserved :27; ///< Reserved, RES0
- } Bits; ///< Bitfield definition of the register
- UINT32 Data; ///< The entire 32-bit value
-} CSSELR_DATA;
-
-/// The cache type values for the InD field of the CSSELR register
-typedef enum
-{
- /// Select the data or unified cache
- CsselrCacheTypeDataOrUnified = 0,
- /// Select the instruction cache
- CsselrCacheTypeInstruction,
- CsselrCacheTypeMax
-} CSSELR_CACHE_TYPE;
-
-/// Defines the structure of the CCSIDR (Current Cache Size ID) register
-typedef union {
- struct {
- UINT64 LineSize :3; ///< Line size (Log2(Num bytes in cache) - 4)
- UINT64 Associativity :10; ///< Associativity - 1
- UINT64 NumSets :15; ///< Number of sets in the cache -1
- UINT64 Unknown :4; ///< Reserved, UNKNOWN
- UINT64 Reserved :32; ///< Reserved, RES0
- } BitsNonCcidx; ///< Bitfield definition of the register when FEAT_CCIDX is not supported.
- struct {
- UINT64 LineSize :3; ///< Line size (Log2(Num bytes in cache) - 4)
- UINT64 Associativity :21; ///< Associativity - 1
- UINT64 Reserved1 :8; ///< Reserved, RES0
- UINT64 NumSets :24; ///< Number of sets in the cache -1
- UINT64 Reserved2 :8; ///< Reserved, RES0
- } BitsCcidxAA64; ///< Bitfield definition of the register when FEAT_IDX is supported.
- struct {
- UINT64 LineSize : 3;
- UINT64 Associativity : 21;
- UINT64 Reserved : 8;
- UINT64 Unallocated : 32;
- } BitsCcidxAA32;
- UINT64 Data; ///< The entire 64-bit value
-} CCSIDR_DATA;
-
-/// Defines the structure of the AARCH32 CCSIDR2 register.
-typedef union {
- struct {
- UINT32 NumSets :24; ///< Number of sets in the cache - 1
- UINT32 Reserved :8; ///< Reserved, RES0
- } Bits; ///< Bitfield definition of the register
- UINT32 Data; ///< The entire 32-bit value
-} CCSIDR2_DATA;
-
-/** Defines the structure of the CLIDR (Cache Level ID) register.
- *
- * The lower 32 bits are the same for both AARCH32 and AARCH64
- * so we can use the same structure for both.
-**/
-typedef union {
- struct {
- UINT32 Ctype1 : 3; ///< Level 1 cache type
- UINT32 Ctype2 : 3; ///< Level 2 cache type
- UINT32 Ctype3 : 3; ///< Level 3 cache type
- UINT32 Ctype4 : 3; ///< Level 4 cache type
- UINT32 Ctype5 : 3; ///< Level 5 cache type
- UINT32 Ctype6 : 3; ///< Level 6 cache type
- UINT32 Ctype7 : 3; ///< Level 7 cache type
- UINT32 LoUIS : 3; ///< Level of Unification Inner Shareable
- UINT32 LoC : 3; ///< Level of Coherency
- UINT32 LoUU : 3; ///< Level of Unification Uniprocessor
- UINT32 Icb : 3; ///< Inner Cache Boundary
- } Bits; ///< Bitfield definition of the register
- UINT32 Data; ///< The entire 32-bit value
-} CLIDR_DATA;
-
-/// The cache types reported in the CLIDR register.
-typedef enum {
- /// No cache is present
- ClidrCacheTypeNone = 0,
- /// There is only an instruction cache
- ClidrCacheTypeInstructionOnly,
- /// There is only a data cache
- ClidrCacheTypeDataOnly,
- /// There are separate data and instruction caches
- ClidrCacheTypeSeparate,
- /// There is a unified cache
- ClidrCacheTypeUnified,
- ClidrCacheTypeMax
-} CLIDR_CACHE_TYPE;
-
-#define CLIDR_GET_CACHE_TYPE(x, level) ((x >> (3 * (level))) & 0b111)
-
VOID
CPSRMaskInsert (
IN UINT32 Mask,
@@ -158,32 +63,4 @@ CPSRRead (
VOID
);

-/** Reads the CCSIDR register for the specified cache.
-
- @param CSSELR The CSSELR cache selection register value.
-
- @return The contents of the CCSIDR_EL1 register for the specified cache, when in AARCH64 mode.
- Returns the contents of the CCSIDR register in AARCH32 mode.
-**/
-UINTN
-ReadCCSIDR (
- IN UINT32 CSSELR
- );
-
-/** Reads the CCSIDR2 for the specified cache.
-
- @param CSSELR The CSSELR cache selection register value
-
- @return The contents of the CCSIDR2 register for the specified cache.
-**/
-UINT32
-ReadCCSIDR2 (
- IN UINT32 CSSELR
- );
-
-UINT32
-ReadCLIDR (
- VOID
- );
-
#endif // ARM_LIB_PRIVATE_H_
diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
index 0cb56c53975e..02297871c92c 100644
--- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
+++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
@@ -14,7 +14,6 @@
#include <IndustryStandard/SmBios.h>
#include <Library/ArmLib.h>
#include <Library/ArmSmcLib.h>
-#include <Library/ArmLib/ArmLibPrivate.h>
#include <Library/BaseLib.h>
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c
index ddd774b16f83..e5309ff598f9 100644
--- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c
+++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c
@@ -9,7 +9,6 @@

#include <Uefi.h>
#include <Library/ArmLib.h>
-#include <Library/ArmLib/ArmLibPrivate.h>

#include "SmbiosProcessor.h"

diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c
index c78bd41a7e06..8176409b24e6 100644
--- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c
+++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c
@@ -9,7 +9,6 @@

#include <Uefi.h>
#include <Library/ArmLib.h>
-#include <Library/ArmLib/ArmLibPrivate.h>

#include "SmbiosProcessor.h"

diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c
index bccb21cfbb41..07e18b3c04ea 100644
--- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c
+++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c
@@ -11,7 +11,6 @@
#include <IndustryStandard/ArmStdSmc.h>
#include <IndustryStandard/SmBios.h>
#include <Library/ArmLib.h>
-#include <Library/ArmLib/ArmLibPrivate.h>
#include <Library/ArmSmcLib.h>
#include <Library/BaseMemoryLib.h>

--
2.26.2


Re: [PATCH v1 3/3] OvmfPkg/X86QemuLoadImageLib: State fw_cfg dependency in file header

Ard Biesheuvel
 

On Wed, 9 Jun 2021 at 16:39, Laszlo Ersek <lersek@redhat.com> wrote:

On 06/09/21 14:18, Dov Murik wrote:
Make it clear that X86QemuLoadImageLib relies on fw_cfg; prepare the
ground to add a warning about the incompatibility with boot verification
process.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
index 1177582ab051..dc9018f4333b 100644
--- a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
+++ b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
@@ -2,6 +2,9 @@
X86 specific implementation of QemuLoadImageLib library class interface
with support for loading mixed mode images and non-EFI stub images

+ Note that this implementation reads the cmdline (and possibly kernel, setup
+ data, and initrd in the legacy boot mode) from fw_cfg directly.
+
Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>

(drive-by comment, no capacity for more atm):

please update the INF file's comment at the top similarly
+1


Re: [PATCH v1 2/3] OvmfPkg/GenericQemuLoadImageLib: Read cmdline from QemuKernelLoaderFs

Ard Biesheuvel
 

Hello Dov,

On Wed, 9 Jun 2021 at 14:18, Dov Murik <dovmurik@linux.ibm.com> wrote:

Remove the QemuFwCfgLib interface used to read the QEMU cmdline
(-append argument) and the initrd size. Instead, use the synthetic
filesystem QemuKernelLoaderFs which has three files: "kernel", "initrd",
and "cmdline".

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf | 2 +-
OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c | 132 ++++++++++++++++++--
2 files changed, 126 insertions(+), 8 deletions(-)

diff --git a/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
index b262cb926a4d..f462fd6922cf 100644
--- a/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
+++ b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
@@ -27,12 +27,12 @@ [LibraryClasses]
DebugLib
MemoryAllocationLib
PrintLib
- QemuFwCfgLib
UefiBootServicesTableLib

[Protocols]
gEfiDevicePathProtocolGuid
gEfiLoadedImageProtocolGuid
+ gEfiSimpleFileSystemProtocolGuid

[Guids]
gQemuKernelLoaderFsMediaGuid
diff --git a/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c
index 114db7e8441f..7d26c912e14f 100644
--- a/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c
+++ b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c
@@ -11,9 +11,9 @@
#include <Base.h>
#include <Guid/QemuKernelLoaderFsMedia.h>
#include <Library/DebugLib.h>
+#include <Library/FileHandleLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/PrintLib.h>
-#include <Library/QemuFwCfgLib.h>
#include <Library/QemuLoadImageLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Protocol/DevicePath.h>
@@ -30,6 +30,11 @@ typedef struct {
KERNEL_FILE_DEVPATH FileNode;
EFI_DEVICE_PATH_PROTOCOL EndNode;
} KERNEL_VENMEDIA_FILE_DEVPATH;
+
+typedef struct {
+ VENDOR_DEVICE_PATH VenMediaNode;
+ EFI_DEVICE_PATH_PROTOCOL EndNode;
+} SINGLE_VENMEDIA_NODE_DEVPATH;
#pragma pack ()

STATIC CONST KERNEL_VENMEDIA_FILE_DEVPATH mKernelDevicePath = {
@@ -51,6 +56,78 @@ STATIC CONST KERNEL_VENMEDIA_FILE_DEVPATH mKernelDevicePath = {
}
};

+STATIC CONST SINGLE_VENMEDIA_NODE_DEVPATH mQemuKernelLoaderFileSystemDevicePath = {
+ {
+ {
+ MEDIA_DEVICE_PATH, MEDIA_VENDOR_DP,
+ { sizeof (VENDOR_DEVICE_PATH) }
+ },
+ QEMU_KERNEL_LOADER_FS_MEDIA_GUID
+ }, {
+ END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE,
+ { sizeof (EFI_DEVICE_PATH_PROTOCOL) }
+ }
+};
+
+STATIC
+EFI_STATUS
+GetQemuKernelBlobSize (
Could we please use a better name here? 'QemuKernel' is confusing as
it is used for initrd and cmdline only, in this case, right?

'QemuKernelLoader' is better in this regard, or other suggestions
welcome as well.


+ IN EFI_FILE_HANDLE Root,
+ IN CHAR16 *FileName,
+ OUT UINTN *Size
+ )
+{
+ EFI_STATUS Status;
+ EFI_FILE_HANDLE FileHandle;
+ UINT64 FileSize;
+
+ Status = Root->Open (Root, &FileHandle, FileName, EFI_FILE_MODE_READ, 0);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ Status = FileHandleGetSize (FileHandle, &FileSize);
+ if (EFI_ERROR (Status)) {
+ goto CloseFile;
+ }
+ *Size = FileSize;
+ Status = EFI_SUCCESS;
+CloseFile:
+ FileHandle->Close (FileHandle);
+ return Status;
+}
+
+STATIC
+EFI_STATUS
+ReadWholeQemuKernelBlob (
+ IN EFI_FILE_HANDLE Root,
+ IN CHAR16 *FileName,
+ IN UINTN Size,
+ OUT VOID *Buffer
+ )
+{
+ EFI_STATUS Status;
+ EFI_FILE_HANDLE FileHandle;
+ UINTN ReadSize;
+
+ Status = Root->Open (Root, &FileHandle, FileName, EFI_FILE_MODE_READ, 0);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ ReadSize = Size;
+ Status = FileHandle->Read (FileHandle, &ReadSize, Buffer);
+ if (EFI_ERROR (Status)) {
+ goto CloseFile;
+ }
+ if (ReadSize != Size) {
+ Status = EFI_PROTOCOL_ERROR;
+ goto CloseFile;
+ }
+ Status = EFI_SUCCESS;
+CloseFile:
+ FileHandle->Close (FileHandle);
+ return Status;
+}
+
/**
Download the kernel, the initial ramdisk, and the kernel command line from
QEMU's fw_cfg. The kernel will be instructed via its command line to load
@@ -79,6 +156,10 @@ QemuLoadKernelImage (
EFI_STATUS Status;
EFI_HANDLE KernelImageHandle;
EFI_LOADED_IMAGE_PROTOCOL *KernelLoadedImage;
+ EFI_DEVICE_PATH_PROTOCOL *DevicePathNode;
+ EFI_HANDLE FsVolumeHandle;
+ EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *FsProtocol;
Slightly tedious, but better to reindent the whole block.

+ EFI_FILE_HANDLE Root;
UINTN CommandLineSize;
CHAR8 *CommandLine;
UINTN InitrdSize;
@@ -124,8 +205,38 @@ QemuLoadKernelImage (
);
ASSERT_EFI_ERROR (Status);

- QemuFwCfgSelectItem (QemuFwCfgItemCommandLineSize);
- CommandLineSize = (UINTN)QemuFwCfgRead32 ();
+ //
+ // Open the Qemu Kernel Loader abstract filesystem (volume) which will be
+ // used to read the "initrd" and "cmdline" synthetic files.
+ //
+ DevicePathNode = (EFI_DEVICE_PATH_PROTOCOL *) &mQemuKernelLoaderFileSystemDevicePath;
No space between cast type and castee please.

+ Status = gBS->LocateDevicePath (
+ &gEfiSimpleFileSystemProtocolGuid,
+ &DevicePathNode,
+ &FsVolumeHandle
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status = gBS->HandleProtocol (
+ FsVolumeHandle,
+ &gEfiSimpleFileSystemProtocolGuid,
+ (VOID **)&FsProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status = FsProtocol->OpenVolume (FsVolumeHandle, &Root);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status = GetQemuKernelBlobSize (Root, L"cmdline", &CommandLineSize);
+ if (EFI_ERROR (Status)) {
+ goto CloseRoot;
+ }

if (CommandLineSize == 0) {
KernelLoadedImage->LoadOptionsSize = 0;
@@ -136,8 +247,10 @@ QemuLoadKernelImage (
goto UnloadImage;
}

- QemuFwCfgSelectItem (QemuFwCfgItemCommandLineData);
- QemuFwCfgReadBytes (CommandLineSize, CommandLine);
+ Status = ReadWholeQemuKernelBlob (Root, L"cmdline", CommandLineSize, CommandLine);
+ if (EFI_ERROR (Status)) {
+ goto FreeCommandLine;
+ }

//
// Verify NUL-termination of the command line.
@@ -155,8 +268,10 @@ QemuLoadKernelImage (
KernelLoadedImage->LoadOptionsSize = (UINT32)((CommandLineSize - 1) * 2);
}

- QemuFwCfgSelectItem (QemuFwCfgItemInitrdSize);
- InitrdSize = (UINTN)QemuFwCfgRead32 ();
+ Status = GetQemuKernelBlobSize (Root, L"initrd", &InitrdSize);
+ if (EFI_ERROR (Status)) {
+ goto FreeCommandLine;
+ }

if (InitrdSize > 0) {
//
@@ -193,6 +308,7 @@ QemuLoadKernelImage (
}

*ImageHandle = KernelImageHandle;
+ Root->Close (Root);
return EFI_SUCCESS;

FreeCommandLine:
@@ -201,6 +317,8 @@ FreeCommandLine:
}
UnloadImage:
gBS->UnloadImage (KernelImageHandle);
+CloseRoot:
+ Root->Close (Root);

return Status;
}
--
2.25.1



------------
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76267): https://edk2.groups.io/g/devel/message/76267
Mute This Topic: https://groups.io/mt/83418869/5717338
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [ardb+tianocore@kernel.org]
------------


Re: [PATCH v2 2/3] UefiPayloadPkg: Add PayloadLoaderPeim which can load ELF payload

Ni, Ray
 

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Brown
Sent: Thursday, June 10, 2021 6:43 PM
To: devel@edk2.groups.io; mhaeuser@posteo.de; Ni, Ray <ray.ni@intel.com>
Cc: Ma, Maurice <maurice.ma@intel.com>; Dong, Guo <guo.dong@intel.com>; You, Benjamin <benjamin.you@intel.com>
Subject: Re: [edk2-devel] [PATCH v2 2/3] UefiPayloadPkg: Add PayloadLoaderPeim which can load ELF payload

On 10/06/2021 11:13, Marvin Häuser wrote:
On 10.06.21 11:39, Ni, Ray wrote:
Maybe for some context, my main issue at first was that the checks are
all proper runtime checks with no ASSERTs at all, so I got confused how
this situation could happen in a realistic scenario. I needed to trace
the ParseStatus data flow to understand the idea is basically the same
as in the PE library. Code in a way is self-documenting, and this
personally gave me a hard time understanding why it is written this way.
But thanks for clarifying your intention! :)
I assume you are ok with the ParseStatus.
I will send new version based on mail discussion. Thanks!
I don't need to be okay with anything, I'm not a maintainer nor an
authority. But I gave my opinion, which is that it is dead code that
makes the design/flow harder to understand for a third party, at no
obvious benefit.
FWIW, I strongly agree with Marvin on this: having ParseStatus in its
current form is a bad idea since it adds no value but does incur a cost.
OK. I can remove that😊


Re: [PATCH v2 2/3] UefiPayloadPkg: Add PayloadLoaderPeim which can load ELF payload

Michael Brown
 

On 10/06/2021 11:13, Marvin Häuser wrote:
On 10.06.21 11:39, Ni, Ray wrote:
Maybe for some context, my main issue at first was that the checks are
all proper runtime checks with no ASSERTs at all, so I got confused how
this situation could happen in a realistic scenario. I needed to trace
the ParseStatus data flow to understand the idea is basically the same
as in the PE library. Code in a way is self-documenting, and this
personally gave me a hard time understanding why it is written this way.
But thanks for clarifying your intention! :)
I assume you are ok with the ParseStatus.
I will send new version based on mail discussion. Thanks!
I don't need to be okay with anything, I'm not a maintainer nor an authority. But I gave my opinion, which is that it is dead code that makes the design/flow harder to understand for a third party, at no obvious benefit.
FWIW, I strongly agree with Marvin on this: having ParseStatus in its current form is a bad idea since it adds no value but does incur a cost.

Thanks,

Michael


Re: [PATCH v2 2/3] UefiPayloadPkg: Add PayloadLoaderPeim which can load ELF payload

Marvin Häuser
 

On 10.06.21 11:39, Ni, Ray wrote:
Maybe for some context, my main issue at first was that the checks are
all proper runtime checks with no ASSERTs at all, so I got confused how
this situation could happen in a realistic scenario. I needed to trace
the ParseStatus data flow to understand the idea is basically the same
as in the PE library. Code in a way is self-documenting, and this
personally gave me a hard time understanding why it is written this way.
But thanks for clarifying your intention! :)
I assume you are ok with the ParseStatus.
I will send new version based on mail discussion. Thanks!
I don't need to be okay with anything, I'm not a maintainer nor an authority. But I gave my opinion, which is that it is dead code that makes the design/flow harder to understand for a third party, at no obvious benefit.

Thank you for preparing fixes.

Best regards,
Marvin


Re: [Patch V4 0/9] Create multiple Hobs for Universal Payload

Zhiguang Liu
 

Liming,

Bugzilla is created at https://bugzilla.tianocore.org/show_bug.cgi?id=3447

Thanks
Zhiguang

-----Original Message-----
From: gaoliming <gaoliming@byosoft.com.cn>
Sent: Thursday, June 10, 2021 5:14 PM
To: devel@edk2.groups.io; Liu, Zhiguang <zhiguang.liu@intel.com>
Subject: 回复: [edk2-devel] [Patch V4 0/9] Create multiple Hobs for Universal
Payload

Zhiguang:
Can you submit one BZ for this new feature? I will add it into edk2 202108
stable tag planning.

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Zhiguang Liu
发送时间: 2021年6月10日 9:33
收件人: devel@edk2.groups.io
主题: [edk2-devel] [Patch V4 0/9] Create multiple Hobs for Universal
Payload

V1:
This patch set is based on Universal Payload on
https://universalpayload.github.io/documentation/payload-
interfaces/index.
html
This patch set introduce one general header, three different hob types
and how Universal Payload consume these hobs.

V2:
Move all the header files and Guid define to MdeModulePkg Fix code bug
when parsing SmbiosDxe.
Enhance error handling in AcpiTableProtocol.c.
Add AcpiTableDxe.inf in UefiPayload.fdf

V3:
Avoid duplicated code in SmBiosDxe.c

V4:
Add link to spec in header files' file comments Avoid using PLD,
because it may be confusing

All changes can be seen at
https://github.com/LiuZhiguang001/edk2/tree/UniversalPayloadHeaders_v4

Zhiguang Liu (9):
MdeModulePkg: Add Universal Payload general definition header file
MdeModulePkg: Add new structure for the PCI Root Bridge Info Hob
UefiPayloadPkg: UefiPayload retrieve PCI root bridge from Guid Hob
MdeModulePkg: Add new structure for the Universal Payload SMBios
Table
Info Hob
MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables
UefiPayloadPkg: Creat gPldSmbiosTableGuid Hob
MdeModulePkg: Add new structure for the Universal Payload ACPI Table
Info Hob
MdeModulePkg/ACPI: Install ACPI table from HOB.
UefiPayloadPkg: Creat gPldAcpiTableGuid Hob

MdeModulePkg/Include/UniversalPayload/AcpiTable.h |
30 ++++++++++++++++++++++++++++++
MdeModulePkg/Include/UniversalPayload/PciRootBridges.h |
91
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++
+++++++++++++++++++++++++++++
MdeModulePkg/Include/UniversalPayload/SmbiosTable.h |
30 ++++++++++++++++++++++++++++++
MdeModulePkg/Include/UniversalPayload/UniversalPayload.h |
35 +++++++++++++++++++++++++++++++++++
MdeModulePkg/MdeModulePkg.dec
| 15 +++++++++++++++
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.c |
92
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++
+++---------------------------
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTable.h |
38 +++++++++++++++++++++++++++++++++++++-
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf | 8
+++++---
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 171
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++
++++++++++++++++++++++++++++++++++++++++-------
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c |
293
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++
++++++++++++++++++++++++++++++++++++++++++++-
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.h |
65
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++
++-
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf |
5 ++++-
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c |
28 +---------------------------
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h |
5 +----
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf |
4 +---
UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridge.h | 40
++++++++++++++++++++++++++++++++++++++--
UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c | 47
++++++++++++++++++++++++++++++++++++++++++++---
UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf | 8
+++++++-
UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c | 73
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++
++++++++++-
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 23
++++++++++++++++++++++-
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h | 5
+++--
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf | 4
+++-
UefiPayloadPkg/UefiPayloadPkg.dsc |
2 +-
UefiPayloadPkg/UefiPayloadPkg.fdf |
4 ++++
24 files changed, 1029 insertions(+), 87 deletions(-) create mode
100644 MdeModulePkg/Include/UniversalPayload/AcpiTable.h
create mode 100644
MdeModulePkg/Include/UniversalPayload/PciRootBridges.h
create mode 100644
MdeModulePkg/Include/UniversalPayload/SmbiosTable.h
create mode 100644
MdeModulePkg/Include/UniversalPayload/UniversalPayload.h

--
2.30.0.windows.2





Re: [PATCH v2 2/3] UefiPayloadPkg: Add PayloadLoaderPeim which can load ELF payload

Ni, Ray
 

Maybe for some context, my main issue at first was that the checks are
all proper runtime checks with no ASSERTs at all, so I got confused how
this situation could happen in a realistic scenario. I needed to trace
the ParseStatus data flow to understand the idea is basically the same
as in the PE library. Code in a way is self-documenting, and this
personally gave me a hard time understanding why it is written this way.
But thanks for clarifying your intention! :)
I assume you are ok with the ParseStatus.
I will send new version based on mail discussion. Thanks!


回复: [edk2-devel] [PATCH v1 0/8] Measured SEV boot with kernel/initrd/cmdline

gaoliming
 

Dov:
Can you submit one BZ for this new feature? I will add it into edk2 202108 stable tag planning.

Thanks
Liming

-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Laszlo Ersek
发送时间: 2021年6月9日 21:54
收件人: Dov Murik <dovmurik@linux.ibm.com>; devel@edk2.groups.io; Ard
Biesheuvel <ardb+tianocore@kernel.org>
抄送: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>; Tobin
Feldman-Fitzthum <tobin@ibm.com>; Jim Cadden <jcadden@ibm.com>;
James Bottomley <jejb@linux.ibm.com>; Hubertus Franke
<frankeh@us.ibm.com>; Jordan Justen <jordan.l.justen@intel.com>; Ashish
Kalra <ashish.kalra@amd.com>; Brijesh Singh <brijesh.singh@amd.com>;
Erdem Aktas <erdemaktas@google.com>; Jiewen Yao
<jiewen.yao@intel.com>; Min Xu <min.m.xu@intel.com>; Tom Lendacky
<thomas.lendacky@amd.com>
主题: Re: [edk2-devel] [PATCH v1 0/8] Measured SEV boot with
kernel/initrd/cmdline

On 06/09/21 14:25, Dov Murik wrote:


On 08/06/2021 18:59, Laszlo Ersek wrote:
On 06/08/21 14:09, Dov Murik wrote:
On 08/06/2021 13:59, Laszlo Ersek wrote:
On 06/08/21 11:57, Dov Murik wrote:

But if we go with (1) -- do you (and Ard) prefer:

(a) leave X86QemuLoadImageLib as it is in master;

-or-

(b) modify X86QemuLoadImageLib the "main" path to use the
QemuKernelLoaderFs (what I started doing) and leave the "legacy" path
with QemuFwCfg

?
I prefer option (a), with the extension that we need to update the
following file-top comment in the files under
"OvmfPkg/Library/X86QemuLoadImageLib":

X86 specific implementation of QemuLoadImageLib library class
interface
with support for loading mixed mode images and non-EFI stub images
First attempt at this is submitted to the mailing list:
https://edk2.groups.io/g/devel/message/76265


We should add a warning there that this library instance (a) depends on
fw_cfg directly, and (b) is therefore unsuitable for blob verification
purposes.
I'll add the warning (b) when I add the blob verification feature.
That makes sense to me, thanks.
Laszlo





回复: [edk2-devel] [Patch V4 0/9] Create multiple Hobs for Universal Payload

gaoliming
 

Zhiguang:
Can you submit one BZ for this new feature? I will add it into edk2 202108 stable tag planning.

Thanks
Liming

-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Zhiguang Liu
发送时间: 2021年6月10日 9:33
收件人: devel@edk2.groups.io
主题: [edk2-devel] [Patch V4 0/9] Create multiple Hobs for Universal Payload

V1:
This patch set is based on Universal Payload on
https://universalpayload.github.io/documentation/payload-interfaces/index.
html
This patch set introduce one general header, three different hob types and
how Universal Payload consume these hobs.

V2:
Move all the header files and Guid define to MdeModulePkg
Fix code bug when parsing SmbiosDxe.
Enhance error handling in AcpiTableProtocol.c.
Add AcpiTableDxe.inf in UefiPayload.fdf

V3:
Avoid duplicated code in SmBiosDxe.c

V4:
Add link to spec in header files' file comments
Avoid using PLD, because it may be confusing

All changes can be seen at
https://github.com/LiuZhiguang001/edk2/tree/UniversalPayloadHeaders_v4

Zhiguang Liu (9):
MdeModulePkg: Add Universal Payload general definition header file
MdeModulePkg: Add new structure for the PCI Root Bridge Info Hob
UefiPayloadPkg: UefiPayload retrieve PCI root bridge from Guid Hob
MdeModulePkg: Add new structure for the Universal Payload SMBios Table
Info Hob
MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables
UefiPayloadPkg: Creat gPldSmbiosTableGuid Hob
MdeModulePkg: Add new structure for the Universal Payload ACPI Table
Info Hob
MdeModulePkg/ACPI: Install ACPI table from HOB.
UefiPayloadPkg: Creat gPldAcpiTableGuid Hob

MdeModulePkg/Include/UniversalPayload/AcpiTable.h |
30 ++++++++++++++++++++++++++++++
MdeModulePkg/Include/UniversalPayload/PciRootBridges.h |
91
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+++++++++++++++++++++++++++++
MdeModulePkg/Include/UniversalPayload/SmbiosTable.h |
30 ++++++++++++++++++++++++++++++
MdeModulePkg/Include/UniversalPayload/UniversalPayload.h |
35 +++++++++++++++++++++++++++++++++++
MdeModulePkg/MdeModulePkg.dec
| 15 +++++++++++++++
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.c |
92
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+++---------------------------
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTable.h |
38 +++++++++++++++++++++++++++++++++++++-
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf | 8
+++++---
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 171
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++-------
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c |
293
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++-
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.h |
65
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++-
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf |
5 ++++-
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c |
28 +---------------------------
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h |
5 +----
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf |
4 +---
UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridge.h | 40
++++++++++++++++++++++++++++++++++++++--
UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c | 47
++++++++++++++++++++++++++++++++++++++++++++---
UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf | 8
+++++++-
UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c | 73
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++-
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 23
++++++++++++++++++++++-
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h | 5
+++--
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf | 4
+++-
UefiPayloadPkg/UefiPayloadPkg.dsc |
2 +-
UefiPayloadPkg/UefiPayloadPkg.fdf |
4 ++++
24 files changed, 1029 insertions(+), 87 deletions(-)
create mode 100644 MdeModulePkg/Include/UniversalPayload/AcpiTable.h
create mode 100644
MdeModulePkg/Include/UniversalPayload/PciRootBridges.h
create mode 100644
MdeModulePkg/Include/UniversalPayload/SmbiosTable.h
create mode 100644
MdeModulePkg/Include/UniversalPayload/UniversalPayload.h

--
2.30.0.windows.2





[edk2-test][PATCH 1/1] SctPkg: Consume MdeLibs.dsc.inc for RegisterFilterLib

Gao Jie
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3246

MdeLibs.dsc.inc was added for some basic/default library
instances provided by MdePkg, RegisterFilterLibNull which will be
consumed by IoLib and BaseLib was added in MdeLibs.dsc.inc.

To build UefiSct with edk2-stable202105 and later, this file
must be included in dsc file.

Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Cc: Eric Jin <eric.jin@intel.com>
Cc: Arvin Chen <arvinx.chen@intel.com>

Signed-off-by: Barton Gao <gaojie@byosoft.com.cn>
---
uefi-sct/SctPkg/UEFI/IHV_SCT.dsc | 2 ++
uefi-sct/SctPkg/UEFI/UEFI_SCT.dsc | 2 ++
2 files changed, 4 insertions(+)

diff --git a/uefi-sct/SctPkg/UEFI/IHV_SCT.dsc b/uefi-sct/SctPkg/UEFI/IHV_SCT.dsc
index 3371141f..7a4393e0 100644
--- a/uefi-sct/SctPkg/UEFI/IHV_SCT.dsc
+++ b/uefi-sct/SctPkg/UEFI/IHV_SCT.dsc
@@ -136,6 +136,8 @@

[Libraries.IA32,Libraries.X64]

+!include MdePkg/MdeLibs.dsc.inc
+
[LibraryClasses.common]
UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
diff --git a/uefi-sct/SctPkg/UEFI/UEFI_SCT.dsc b/uefi-sct/SctPkg/UEFI/UEFI_SCT.dsc
index 2b4f415f..5b3e5307 100644
--- a/uefi-sct/SctPkg/UEFI/UEFI_SCT.dsc
+++ b/uefi-sct/SctPkg/UEFI/UEFI_SCT.dsc
@@ -138,6 +138,8 @@
[Libraries.RISCV64]
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf

+!include MdePkg/MdeLibs.dsc.inc
+
[LibraryClasses.common]
UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
--
2.31.0.windows.1


Re: [PATCH v2 2/3] UefiPayloadPkg: Add PayloadLoaderPeim which can load ELF payload

Marvin Häuser
 

On 10.06.21 05:40, Ni, Ray wrote:
Without the ParseStatus field, callee cannot know whether ParseElfImage()
is called.

It can by function contracts, the caller guarantees it. I.e. with the PE
library I linked, no other function must be called before the init function.
Your "ParseElfImage" function is very similar. The context is
initialized by it, i.e. it is trash if it is not called, i.e. it must be
called before other functions.
If it is called, which we know, the caller has the return status. For
PE, it means the caller must not proceed with any further PE processing
and abort immediately.
Is there any scenario where this does not work for ELF? Sorry if I
missed something.
Caller might call LoadElfImage() without firstly calling ParseElfImage() by mistake.
ParseStatus is added to catch such mistake.
If ParseElfImage() is not called, nothing will initialize ParseStatus and the load function will read random data. If AllocateZeroPool was used for the context, a common pattern throughout the codebase to harden against memory initialization bugs, it would even report success at all times anyway. Sorry, but I think this is dead code.

Maybe for some context, my main issue at first was that the checks are all proper runtime checks with no ASSERTs at all, so I got confused how this situation could happen in a realistic scenario. I needed to trace the ParseStatus data flow to understand the idea is basically the same as in the PE library. Code in a way is self-documenting, and this personally gave me a hard time understanding why it is written this way. But thanks for clarifying your intention! :)

Best regards,
Marvin


I don't trust the caller would follow the contracts properly😊.


[PATCH] MdeModulePkg/RegularExpressionDxe: Fix memory assert in FreePool()

Nickle Wang
 

Memory buffer that is allocated by malloc() and realloc() will be
shifted by 8 bytes because Oniguruma keeps its memory signature. This 8
bytes shift is not handled while calling free() to release memory. Add
free() function to check Oniguruma signature before release memory
because memory buffer is not touched when using calloc().

Signed-off-by: Nickle Wang <nickle.wang@hpe.com>
---
.../RegularExpressionDxe/OnigurumaUefiPort.c | 19 ++++++++++++++++++-
.../RegularExpressionDxe/OnigurumaUefiPort.h | 14 ++------------
2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.=
c b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c
index 9aa7b0a68e..5c34324db8 100644
--- a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c
+++ b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c
@@ -2,7 +2,7 @@
=0D
Module to rewrite stdlib references within Oniguruma=0D
=0D
- (C) Copyright 2014-2015 Hewlett Packard Enterprise Development LP<BR>=0D
+ (C) Copyright 2014-2021 Hewlett Packard Enterprise Development LP<BR>=0D
Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>=0D
=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
@@ -96,3 +96,20 @@ void* memset (void *dest, char ch, unsigned int count)
return SetMem (dest, count, ch);=0D
}=0D
=0D
+void free(void *ptr)=0D
+{=0D
+ VOID *EvalOnce;=0D
+ ONIGMEM_HEAD *PoolHdr;=0D
+=0D
+ EvalOnce =3D ptr;=0D
+ if (EvalOnce =3D=3D NULL) {=0D
+ return;=0D
+ }=0D
+=0D
+ PoolHdr =3D (ONIGMEM_HEAD *)EvalOnce - 1;=0D
+ if (PoolHdr->Signature =3D=3D ONIGMEM_HEAD_SIGNATURE) {=0D
+ FreePool (PoolHdr);=0D
+ } else {=0D
+ FreePool (EvalOnce);=0D
+ }=0D
+}=0D
diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.=
h b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.h
index 20b75c3361..0bdb7be529 100644
--- a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.h
+++ b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.h
@@ -2,7 +2,7 @@
=0D
Module to rewrite stdlib references within Oniguruma=0D
=0D
- (C) Copyright 2014-2015 Hewlett Packard Enterprise Development LP<BR>=0D
+ (C) Copyright 2014-2021 Hewlett Packard Enterprise Development LP<BR>=0D
Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>=0D
=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
@@ -46,17 +46,6 @@ typedef INTN intptr_t;
#endif=0D
=0D
#define calloc(n,s) AllocateZeroPool((n)*(s))=0D
-=0D
-#define free(p) \=0D
- do { \=0D
- VOID *EvalOnce; \=0D
- \=0D
- EvalOnce =3D (p); \=0D
- if (EvalOnce !=3D NULL) { \=0D
- FreePool (EvalOnce); \=0D
- } \=0D
- } while (FALSE)=0D
-=0D
#define xmemmove(Dest,Src,Length) CopyMem(Dest,Src,Length)=0D
#define xmemcpy(Dest,Src,Length) CopyMem(Dest,Src,Length)=0D
#define xmemset(Buffer,Value,Length) SetMem(Buffer,Length,Value)=0D
@@ -98,6 +87,7 @@ void* malloc(size_t size);
void* realloc(void *ptr, size_t size);=0D
void* memcpy (void *dest, const void *src, unsigned int count);=0D
void* memset (void *dest, char ch, unsigned int count);=0D
+void free(void *ptr);=0D
=0D
#define exit(n) ASSERT(FALSE);=0D
=0D
--=20
2.31.1.windows.1


Re: [Patch V3] UefiPayloadPkg: Use DynamicEx instead of Dynamic to pass PCD across binary

Zhiguang Liu
 

Hi Guo,

Thanks for the comments.
I described the reason in the V2 commint message:
Explicitly define some PCDs as DynamicEx, or their default type will be Dynamic

Thanks
Zhiguang

-----Original Message-----
From: Dong, Guo <guo.dong@intel.com>
Sent: Thursday, June 10, 2021 11:04 AM
To: Liu, Zhiguang <zhiguang.liu@intel.com>; devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@intel.com>; You, Benjamin
<benjamin.you@intel.com>
Subject: RE: [Patch V3] UefiPayloadPkg: Use DynamicEx instead of Dynamic
to pass PCD across binary


This patch 1) changed PCD type from Dynamic to DynamicEX 2) added 3 PCDs.
It would be great if you could describe why 3 PCDs are added in the commit
message.

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

Thanks,
Guo

-----Original Message-----
From: Liu, Zhiguang <zhiguang.liu@intel.com>
Sent: Wednesday, June 9, 2021 6:38 PM
To: devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@intel.com>; Dong, Guo
<guo.dong@intel.com>; You, Benjamin <benjamin.you@intel.com>
Subject: [Patch V3] UefiPayloadPkg: Use DynamicEx instead of Dynamic
to pass PCD across binary

V1:
When passing PCD database from Edk2 boot loader to Universal Payload,
the local token number in boot loader PCD database can be different
with that in Payload PCD database.
Dynamic PCD directly use local token number, while DynamicEx will
search token number by Guid and ExTokenNumber, which are unique pair
and can make sure finding the correct token number in boot loader's
PCD database
V2:
Remove PCD PcdFlashNvStorageFtwWorkingBase and
PcdFlashNvStorageFtwSpareBase, because they are not consumed by any
modules.
Explicitly define some PCDs as DynamicEx, or their default type will
be Dynamic

V3:
Not remove some PCDs for they will be consumed soon

Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>

Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
UefiPayloadPkg/UefiPayloadPkg.dsc | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc
b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 37ad5a0ae7..4b0ec3a059 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -336,11 +336,11 @@



##########################################################
######################

#

-# Pcd Dynamic Section - list of all EDK II PCD Entries defined by
this Platform

+# Pcd DynamicEx Section - list of all EDK II PCD Entries defined by
+this
Platform

#


##########################################################
######################



-[PcdsDynamicDefault]

+[PcdsDynamicExDefault]

gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0

gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0


gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0

@@ -363,6 +363,9 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutColumn|100

gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0

gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize|0

+ gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase|0

+ gEfiMdeModulePkgTokenSpaceGuid.PcdTestKeyUsed|FALSE

+ gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled|0




##########################################################
######################

#

--
2.30.0.windows.2


Re: [PATCH v2 2/3] UefiPayloadPkg: Add PayloadLoaderPeim which can load ELF payload

Ni, Ray
 

Without the ParseStatus field, callee cannot know whether ParseElfImage()
is called.

It can by function contracts, the caller guarantees it. I.e. with the PE
library I linked, no other function must be called before the init function.
Your "ParseElfImage" function is very similar. The context is
initialized by it, i.e. it is trash if it is not called, i.e. it must be
called before other functions.
If it is called, which we know, the caller has the return status. For
PE, it means the caller must not proceed with any further PE processing
and abort immediately.
Is there any scenario where this does not work for ELF? Sorry if I
missed something.
Caller might call LoadElfImage() without firstly calling ParseElfImage() by mistake.
ParseStatus is added to catch such mistake.

I don't trust the caller would follow the contracts properly😊.


Re: [Patch V3] UefiPayloadPkg: Use DynamicEx instead of Dynamic to pass PCD across binary

Guo Dong
 

This patch 1) changed PCD type from Dynamic to DynamicEX 2) added 3 PCDs.
It would be great if you could describe why 3 PCDs are added in the commit message.

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

Thanks,
Guo

-----Original Message-----
From: Liu, Zhiguang <zhiguang.liu@intel.com>
Sent: Wednesday, June 9, 2021 6:38 PM
To: devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@intel.com>; Dong, Guo
<guo.dong@intel.com>; You, Benjamin <benjamin.you@intel.com>
Subject: [Patch V3] UefiPayloadPkg: Use DynamicEx instead of Dynamic to
pass PCD across binary

V1:
When passing PCD database from Edk2 boot loader to Universal Payload, the
local
token number in boot loader PCD database can be different with that in
Payload
PCD database.
Dynamic PCD directly use local token number, while DynamicEx will search
token number
by Guid and ExTokenNumber, which are unique pair and can make sure
finding the correct
token number in boot loader's PCD database
V2:
Remove PCD PcdFlashNvStorageFtwWorkingBase and
PcdFlashNvStorageFtwSpareBase, because they
are not consumed by any modules.
Explicitly define some PCDs as DynamicEx, or their default type will be
Dynamic

V3:
Not remove some PCDs for they will be consumed soon

Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>

Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
UefiPayloadPkg/UefiPayloadPkg.dsc | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc
b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 37ad5a0ae7..4b0ec3a059 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -336,11 +336,11 @@



##########################################################
######################

#

-# Pcd Dynamic Section - list of all EDK II PCD Entries defined by this Platform

+# Pcd DynamicEx Section - list of all EDK II PCD Entries defined by this
Platform

#


##########################################################
######################



-[PcdsDynamicDefault]

+[PcdsDynamicExDefault]

gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0

gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0


gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0

@@ -363,6 +363,9 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutColumn|100

gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0

gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize|0

+ gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase|0

+ gEfiMdeModulePkgTokenSpaceGuid.PcdTestKeyUsed|FALSE

+ gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled|0




##########################################################
######################

#

--
2.30.0.windows.2


Re: [Patch V4 9/9] UefiPayloadPkg: Creat gPldAcpiTableGuid Hob

Guo Dong
 

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

-----Original Message-----
From: Liu, Zhiguang <zhiguang.liu@intel.com>
Sent: Wednesday, June 9, 2021 6:33 PM
To: devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@intel.com>; Dong, Guo
<guo.dong@intel.com>; You, Benjamin <benjamin.you@intel.com>; Ni, Ray
<ray.ni@intel.com>
Subject: [Patch V4 9/9] UefiPayloadPkg: Creat gPldAcpiTableGuid Hob

From SysTableInfo Hob, get ACPI table address, and creat gPldAcpiTableGuid
Hob
to store it. Remove diretly adding ACPI table to ConfigurationTable.
Dxe ACPI driver will parse it and install ACPI table from Guid Hob.

Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c | 17 -----------------
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h | 5 +----
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf | 1 -
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 11 +++++++++++
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h | 2 +-
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf | 1 +
UefiPayloadPkg/UefiPayloadPkg.fdf | 4 ++++
7 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
index 56b85b8e6d..ffd3427fb3 100644
--- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
+++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
@@ -99,7 +99,6 @@ BlDxeEntryPoint (
{

EFI_STATUS Status;

EFI_HOB_GUID_TYPE *GuidHob;

- SYSTEM_TABLE_INFO *SystemTableInfo;

EFI_PEI_GRAPHICS_INFO_HOB *GfxInfo;

ACPI_BOARD_INFO *AcpiBoardInfo;



@@ -113,22 +112,6 @@ BlDxeEntryPoint (
Status = ReserveResourceInGcd (TRUE,
EfiGcdMemoryTypeMemoryMappedIo, 0xFED00000, SIZE_1KB, 0,
ImageHandle); // HPET

ASSERT_EFI_ERROR (Status);



- //

- // Find the system table information guid hob

- //

- GuidHob = GetFirstGuidHob (&gUefiSystemTableInfoGuid);

- ASSERT (GuidHob != NULL);

- SystemTableInfo = (SYSTEM_TABLE_INFO *)GET_GUID_HOB_DATA
(GuidHob);

-

- //

- // Install Acpi Table

- //

- if (SystemTableInfo->AcpiTableBase != 0 && SystemTableInfo-
AcpiTableSize != 0) {
- DEBUG ((DEBUG_ERROR, "Install Acpi Table at 0x%lx, length 0x%x\n",
SystemTableInfo->AcpiTableBase, SystemTableInfo->AcpiTableSize));

- Status = gBS->InstallConfigurationTable (&gEfiAcpiTableGuid, (VOID
*)(UINTN)SystemTableInfo->AcpiTableBase);

- ASSERT_EFI_ERROR (Status);

- }

-

//

// Find the frame buffer information and update PCDs

//

diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h
b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h
index 512105fafd..3332a30eae 100644
--- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h
+++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h
@@ -1,7 +1,7 @@
/** @file

The header file of bootloader support DXE.



-Copyright (c) 2014, Intel Corporation. All rights reserved.<BR>

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

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



**/

@@ -19,12 +19,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/IoLib.h>

#include <Library/HobLib.h>



-#include <Guid/Acpi.h>

#include <Guid/SmBios.h>

#include <Guid/SystemTableInfoGuid.h>

#include <Guid/AcpiBoardInfoGuid.h>

#include <Guid/GraphicsInfoHob.h>



-#include <IndustryStandard/Acpi.h>

-

#endif

diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
index 30f41f8c39..1ccb250991 100644
--- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
+++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
@@ -42,7 +42,6 @@
HobLib



[Guids]

- gEfiAcpiTableGuid

gUefiSystemTableInfoGuid

gUefiAcpiBoardInfoGuid

gEfiGraphicsInfoHobGuid

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index 80f66a3fd5..f44e0ea7f0 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -235,6 +235,7 @@ BuildHobFromBl (
EFI_PEI_GRAPHICS_DEVICE_INFO_HOB GfxDeviceInfo;

EFI_PEI_GRAPHICS_DEVICE_INFO_HOB *NewGfxDeviceInfo;

UNIVERSAL_PAYLOAD_SMBIOS_TABLE *SmBiosTableHob;

+ UNIVERSAL_PAYLOAD_ACPI_TABLE *AcpiTableHob;



//

// Parse memory info and build memory HOBs

@@ -287,6 +288,16 @@ BuildHobFromBl (
SmBiosTableHob->SmBiosEntryPoint = SysTableInfo.SmbiosTableBase;

DEBUG ((DEBUG_INFO, "Create smbios table
gUniversalPayloadSmbiosTableGuid guid hob\n"));



+ //

+ // Creat ACPI table Hob

+ //

+ AcpiTableHob = BuildGuidHob (&gUniversalPayloadAcpiTableGuid, sizeof
(UNIVERSAL_PAYLOAD_ACPI_TABLE));

+ ASSERT (AcpiTableHob != NULL);

+ AcpiTableHob->Header.Revision =
UNIVERSAL_PAYLOAD_ACPI_TABLE_REVISION;

+ AcpiTableHob->Header.Length = sizeof
(UNIVERSAL_PAYLOAD_ACPI_TABLE);

+ AcpiTableHob->Rsdp = SysTableInfo.AcpiTableBase;

+ DEBUG ((DEBUG_INFO, "Create smbios table
gUniversalPayloadAcpiTableGuid guid hob\n"));

+

//

// Create guid hob for acpi board information

//

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
index e7d0d15118..a4c9da128e 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
@@ -32,7 +32,7 @@
#include <Guid/AcpiBoardInfoGuid.h>

#include <Guid/GraphicsInfoHob.h>

#include <UniversalPayload/SmbiosTable.h>

-

+#include <UniversalPayload/AcpiTable.h>



#define LEGACY_8259_MASK_REGISTER_MASTER 0x21

#define LEGACY_8259_MASK_REGISTER_SLAVE 0xA1

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
index fc5b5ce9d4..8d42925fcd 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
@@ -65,6 +65,7 @@
gEfiGraphicsDeviceInfoHobGuid

gUefiAcpiBoardInfoGuid

gUniversalPayloadSmbiosTableGuid

+ gUniversalPayloadAcpiTableGuid



[FeaturePcd.IA32]

gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode ##
CONSUMES

diff --git a/UefiPayloadPkg/UefiPayloadPkg.fdf
b/UefiPayloadPkg/UefiPayloadPkg.fdf
index 8fc509024b..ed7fbcaddb 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.fdf
+++ b/UefiPayloadPkg/UefiPayloadPkg.fdf
@@ -175,6 +175,10 @@ INF
MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
INF MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf

INF MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf



+#

+# ACPI Support

+#

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



#

# Shell

--
2.30.0.windows.2


Re: [Patch V4 6/9] UefiPayloadPkg: Creat gPldSmbiosTableGuid Hob

Guo Dong
 

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

-----Original Message-----
From: Liu, Zhiguang <zhiguang.liu@intel.com>
Sent: Wednesday, June 9, 2021 6:33 PM
To: devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@intel.com>; Dong, Guo
<guo.dong@intel.com>; You, Benjamin <benjamin.you@intel.com>
Subject: [Patch V4 6/9] UefiPayloadPkg: Creat gPldSmbiosTableGuid Hob

From SysTableInfo Hob, get Smbios table address, and creat
gPldSmbiosTableGuid Hob
to store it. Remove diretly adding smbios table to ConfigurationTable.
Dxe module SmbiosDxe will parse it and install smbios table from it.

Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Reviewed-by: Guo Dong <guo.dong@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c | 11 +----------
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf | 3 +--
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 12 +++++++++++-
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h | 3 ++-
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf | 3 ++-
5 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
index a746d0581e..56b85b8e6d 100644
--- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
+++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
@@ -2,7 +2,7 @@
This driver will report some MMIO/IO resources to dxe core, extract smbios
and acpi

tables from bootloader.



- Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>

+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>

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



**/

@@ -129,15 +129,6 @@ BlDxeEntryPoint (
ASSERT_EFI_ERROR (Status);

}



- //

- // Install Smbios Table

- //

- if (SystemTableInfo->SmbiosTableBase != 0 && SystemTableInfo-
SmbiosTableSize != 0) {
- DEBUG ((DEBUG_ERROR, "Install Smbios Table at 0x%lx, length 0x%x\n",
SystemTableInfo->SmbiosTableBase, SystemTableInfo->SmbiosTableSize));

- Status = gBS->InstallConfigurationTable (&gEfiSmbiosTableGuid, (VOID
*)(UINTN)SystemTableInfo->SmbiosTableBase);

- ASSERT_EFI_ERROR (Status);

- }

-

//

// Find the frame buffer information and update PCDs

//

diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
index cebc811355..30f41f8c39 100644
--- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
+++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
@@ -3,7 +3,7 @@
#

# Report some MMIO/IO resources to dxe core, extract smbios and acpi
tables

#

-# Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>

+# Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>

#

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

#

@@ -43,7 +43,6 @@


[Guids]

gEfiAcpiTableGuid

- gEfiSmbiosTableGuid

gUefiSystemTableInfoGuid

gUefiAcpiBoardInfoGuid

gEfiGraphicsInfoHobGuid

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index 805f5448d9..80f66a3fd5 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -1,6 +1,6 @@
/** @file



- Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>

+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>

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



**/

@@ -234,6 +234,7 @@ BuildHobFromBl (
EFI_PEI_GRAPHICS_INFO_HOB *NewGfxInfo;

EFI_PEI_GRAPHICS_DEVICE_INFO_HOB GfxDeviceInfo;

EFI_PEI_GRAPHICS_DEVICE_INFO_HOB *NewGfxDeviceInfo;

+ UNIVERSAL_PAYLOAD_SMBIOS_TABLE *SmBiosTableHob;



//

// Parse memory info and build memory HOBs

@@ -276,6 +277,15 @@ BuildHobFromBl (
DEBUG ((DEBUG_INFO, "Detected Acpi Table at 0x%lx, length 0x%x\n",
SysTableInfo.AcpiTableBase, SysTableInfo.AcpiTableSize));

DEBUG ((DEBUG_INFO, "Detected Smbios Table at 0x%lx, length 0x%x\n",
SysTableInfo.SmbiosTableBase, SysTableInfo.SmbiosTableSize));

}

+ //

+ // Creat SmBios table Hob

+ //

+ SmBiosTableHob = BuildGuidHob (&gUniversalPayloadSmbiosTableGuid,
sizeof (UNIVERSAL_PAYLOAD_SMBIOS_TABLE));

+ ASSERT (SmBiosTableHob != NULL);

+ SmBiosTableHob->Header.Revision =
UNIVERSAL_PAYLOAD_SMBIOS_TABLE_REVISION;

+ SmBiosTableHob->Header.Length = sizeof
(UNIVERSAL_PAYLOAD_SMBIOS_TABLE);

+ SmBiosTableHob->SmBiosEntryPoint = SysTableInfo.SmbiosTableBase;

+ DEBUG ((DEBUG_INFO, "Create smbios table
gUniversalPayloadSmbiosTableGuid guid hob\n"));



//

// Create guid hob for acpi board information

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
index 2c84d6ed53..e7d0d15118 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
@@ -1,6 +1,6 @@
/** @file

*

-* Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>

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

*

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

*

@@ -31,6 +31,7 @@
#include <Guid/MemoryMapInfoGuid.h>

#include <Guid/AcpiBoardInfoGuid.h>

#include <Guid/GraphicsInfoHob.h>

+#include <UniversalPayload/SmbiosTable.h>





#define LEGACY_8259_MASK_REGISTER_MASTER 0x21

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
index cc59f1903b..fc5b5ce9d4 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
@@ -1,7 +1,7 @@
## @file

# This is the first module for UEFI payload.

#

-# Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR>

+# Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>

# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>

#

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

@@ -64,6 +64,7 @@
gEfiGraphicsInfoHobGuid

gEfiGraphicsDeviceInfoHobGuid

gUefiAcpiBoardInfoGuid

+ gUniversalPayloadSmbiosTableGuid



[FeaturePcd.IA32]

gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode ##
CONSUMES

--
2.30.0.windows.2


Re: [Patch V3 5/9] MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables

Zhiguang Liu
 

Hi Patrick

Thanks for catching this issue.
I updated the code, and you can find the code below
https://github.com/LiuZhiguang001/edk2/tree/UniversalPayloadHeaders_v4
Please help confirm.

Thanks
Zhiguang

-----Original Message-----
From: Patrick Rudolph <patrick.rudolph@9elements.com>
Sent: Tuesday, June 8, 2021 5:21 PM
To: Liu, Zhiguang <zhiguang.liu@intel.com>
Cc: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
<hao.a.wu@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Zeng, Star
<star.zeng@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: Re: [Patch V3 5/9] MdeModulePkg/Universal/SmbiosDxe: Scan for
existing tables

On Fri, Jun 4, 2021 at 11:42 AM Zhiguang Liu <zhiguang.liu@intel.com> wrote:

V1:
The default EfiSmbiosProtocol operates on an empty SMBIOS table.
The SMBIOS tables are provided by the bootloader on UefiPayloadPkg.
Scan for existing tables in SmbiosDxe and load them if they seem valid.

This fixes the settings menu not showing any hardware information,
instead only "0 MB RAM" was displayed.

Tests showed that the OS can still see the SMBIOS tables.

V2:
SmbiosDxe will get the SMBIOS from a guid Hob.
Aslo will keep the SmbiosHandle if it is available.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c | 320
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++--
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.h | 4 +++-
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf | 5 ++++-
3 files changed, 325 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
index 3cdb0b1ed7..3579c4d890 100644
--- a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
+++ b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
@@ -2,7 +2,7 @@
This code produces the Smbios protocol. It also responsible for
constructing
SMBIOS table into system table.

-Copyright (c) 2009 - 2018, Intel Corporation. All rights
reserved.<BR>
+Copyright (c) 2009 - 2021, Intel Corporation. All rights
+reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -148,6 +148,31 @@ SMBIOS_TABLE_3_0_ENTRY_POINT
Smbios30EntryPointStructureData = {
//
0
};
+
+/**
+ Validates a SMBIOS table entry point.
+
+ @param TableEntry The SmBios table entry to validate.
+ @param TableAddress On exit, point to the smbios table addres.
+ @param TableMaximumSize On exit, point to the maximum size of the
table.
+
+ @retval TRUE SMBIOS table entry point is valid.
+ @retval FALSE SMBIOS table entry point is malformed.
+
+**/
+typedef
+BOOLEAN
+(* IS_SMBIOS_TABLE_VALID) (
+ IN VOID *TableEntry,
+ OUT VOID **TableAddress,
+ OUT UINTN *TableMaximumSize
+ );
+typedef struct {
+ EFI_GUID *Guid;
+ IS_SMBIOS_TABLE_VALID IsValid;
+} IS_SMBIOS_TABLE_VALID_ENTRY;
+
+
/**

Get the full size of SMBIOS structure including optional strings that follow
the formatted structure.
@@ -1408,6 +1433,296 @@ SmbiosTableConstruction (
}
}

+/**
+ Validates a SMBIOS 2.0 table entry point.
+
+ @param TableEntry The SmBios table entry to validate.
+ @param TableAddress On exit, point to the smbios table addres.
+ @param TableMaximumSize On exit, point to the maximum size of the
table.
+
+ @retval TRUE SMBIOS table entry point is valid.
+ @retval FALSE SMBIOS table entry point is malformed.
+
+**/
+STATIC
+BOOLEAN
+IsValidSmbios20Table (
+ IN VOID *TableEntry,
+ OUT VOID **TableAddress,
+ OUT UINTN *TableMaximumSize
+ )
+{
+ UINT8 Checksum;
+ SMBIOS_TABLE_ENTRY_POINT *SmbiosTable;
+ SmbiosTable = (SMBIOS_TABLE_ENTRY_POINT *) TableEntry;
+
+ if (CompareMem (SmbiosTable->AnchorString, "_SM_", 4) != 0) {
+ return FALSE;
+ }
+
+ if (CompareMem (SmbiosTable->IntermediateAnchorString, "_DMI_",
5) != 0) {
+ return FALSE;
+ }
+
+ //
+ // The actual value of the EntryPointLength should be 1Fh.
+ // However, it was incorrectly stated in version 2.1 of smbios
specification.
+ // Therefore, 0x1F and 0x1E are both accepted.
+ //
+ if (SmbiosTable->EntryPointLength != 0x1E && SmbiosTable-
EntryPointLength != sizeof (SMBIOS_TABLE_ENTRY_POINT)) {
+ return FALSE;
+ }
+
+ //
+ // MajorVersion should not be less than 2.
+ //
+ if (SmbiosTable->MajorVersion < 2) {
+ return FALSE;
+ }
+
+ //
+ // The whole struct check sum should be zero // Checksum =
+ CalculateSum8 (
+ (UINT8 *) SmbiosTable,
+ SmbiosTable->EntryPointLength
+ );
+ if (Checksum != 0) {
+ return FALSE;
+ }
+
+ //
+ // The Intermediate Entry Point Structure check sum should be zero.
+ //
+ Checksum = CalculateSum8 (
+ (UINT8 *) SmbiosTable + OFFSET_OF
(SMBIOS_TABLE_ENTRY_POINT, IntermediateAnchorString),
+ SmbiosTable->EntryPointLength - OFFSET_OF
(SMBIOS_TABLE_ENTRY_POINT, IntermediateAnchorString)
+ );
+ if (Checksum != 0) {
+ return FALSE;
+ }
+
+ *TableAddress = (VOID *) (UINTN) SmbiosTable->TableAddress;
+ *TableMaximumSize = SmbiosTable->TableLength;
+ return TRUE;
+}
+
+/**
+ Validates a SMBIOS 3.0 table entry point.
+
+ @param TableEntry The SmBios table entry to validate.
+ @param TableAddress On exit, point to the smbios table addres.
+ @param TableMaximumSize On exit, point to the maximum size of the
table.
+
+ @retval TRUE SMBIOS table entry point is valid.
+ @retval FALSE SMBIOS table entry point is malformed.
+
+**/
+STATIC
+BOOLEAN
+IsValidSmbios30Table (
+ IN VOID *TableEntry,
+ OUT VOID **TableAddress,
+ OUT UINTN *TableMaximumSize
+ )
+{
+ UINT8 Checksum;
+ SMBIOS_TABLE_3_0_ENTRY_POINT *SmbiosTable;
+ SmbiosTable = (SMBIOS_TABLE_3_0_ENTRY_POINT *) TableEntry;
+
+ if (CompareMem (SmbiosTable->AnchorString, "_SM3_", 5) != 0) {
+ return FALSE;
+ }
+ if (SmbiosTable->EntryPointLength < sizeof
(SMBIOS_TABLE_3_0_ENTRY_POINT)) {
+ return FALSE;
+ }
+ if (SmbiosTable->MajorVersion < 3) {
+ return FALSE;
+ }
+
+ //
+ // The whole struct check sum should be zero // Checksum =
+ CalculateSum8 (
+ (UINT8 *) SmbiosTable,
+ SmbiosTable->EntryPointLength
+ );
+ if (Checksum != 0) {
+ return FALSE;
+ }
+
+ *TableAddress = (VOID *) (UINTN) SmbiosTable->TableAddress;
+ *TableMaximumSize = SmbiosTable->TableMaximumSize;
+ return TRUE;
+}
+
+/**
+ Parse an existing SMBIOS table and insert it using SmbiosAdd.
+
+ @param ImageHandle The EFI_HANDLE to this driver.
+ @param Smbios The SMBIOS table to parse.
+ @param Length The length of the SMBIOS table.
+
+ @retval EFI_SUCCESS SMBIOS table was parsed and installed.
+ @retval EFI_OUT_OF_RESOURCES Record was not added due to lack of
system resources.
+ @retval EFI_INVALID_PARAMETER Smbios is not a correct smbios table
+
+**/
+STATIC
+EFI_STATUS
+ParseAndAddExistingSmbiosTable (
+ IN EFI_HANDLE ImageHandle,
+ IN SMBIOS_STRUCTURE_POINTER Smbios,
+ IN UINTN Length
+ )
+{
+ EFI_STATUS Status;
+ CHAR8 *String;
+ EFI_SMBIOS_HANDLE SmbiosHandle;
+ SMBIOS_STRUCTURE_POINTER SmbiosEnd;
+
+ SmbiosEnd.Raw = Smbios.Raw + Length;
+
+ if (Smbios.Raw >= SmbiosEnd.Raw || Smbios.Raw == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ do {
+ //
+ // Make sure not to access memory beyond SmbiosEnd
+ //
+ if (Smbios.Raw + sizeof (SMBIOS_STRUCTURE) > SmbiosEnd.Raw ||
+ Smbios.Raw + sizeof (SMBIOS_STRUCTURE) < Smbios.Raw) {
+ return EFI_INVALID_PARAMETER;
+ }
+ //
+ // Check for end marker
+ //
+ if (Smbios.Hdr->Type == SMBIOS_TYPE_END_OF_TABLE) {
+ break;
+ }
+ //
+ // Make sure not to access memory beyond SmbiosEnd
+ // Each structure shall be terminated by a double-null (0000h).
+ //
+ if (Smbios.Raw + Smbios.Hdr->Length + 2 * sizeof (UINT8) >
SmbiosEnd.Raw ||
+ Smbios.Raw + Smbios.Hdr->Length + 2 * sizeof (UINT8) < Smbios.Raw)
{
+ return EFI_INVALID_PARAMETER;
+ }
+ //
+ // Install the table
+ //
+ SmbiosHandle = Smbios.Hdr->Handle;
+ Status = SmbiosAdd (
+ &mPrivateData.Smbios,
+ ImageHandle,
+ &SmbiosHandle,
+ Smbios.Hdr
+ );
+
+ ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ //
+ // Go to the next SMBIOS structure. Each SMBIOS structure may include
2 parts:
+ // 1. Formatted section; 2. Unformatted string section. So, 2 steps are
needed
+ // to skip one SMBIOS structure.
+ //
+
+ //
+ // Step 1: Skip over formatted section.
+ //
+ String = (CHAR8 *) (Smbios.Raw + Smbios.Hdr->Length);
+
+ //
+ // Step 2: Skip over unformatted string section.
+ //
+ do {
+ //
+ // Each string is terminated with a NULL(00h) BYTE and the sets of
strings
+ // is terminated with an additional NULL(00h) BYTE.
+ //
+ for ( ; *String != 0; String++) {
+ if ((UINTN) String >= (UINTN) SmbiosEnd.Raw - sizeof (UINT8)) {
+ return EFI_INVALID_PARAMETER;
+ }
+ }
+
+ if (*(UINT8 *) ++String == 0) {
+ //
+ // Pointer to the next SMBIOS structure.
+ //
+ Smbios.Raw = (UINT8 *) ++String;
+ break;
+ }
+ } while (TRUE);
+ } while (Smbios.Raw < SmbiosEnd.Raw);
+
+ return EFI_SUCCESS;
+}
+
+
+IS_SMBIOS_TABLE_VALID_ENTRY mIsSmbiosTableValid[] = {
+ {&gPldSmbios3TableGuid, IsValidSmbios30Table },
+ {&gPldSmbiosTableGuid, IsValidSmbios20Table } };
+
+/**
+ Retrieve SMBIOS from Hob.
+ @param ImageHandle Module's image handle
+
+ @retval EFI_SUCCESS Smbios from Hob is installed.
+ @return EFI_NOT_FOUND Not found Smbios from Hob.
+ @retval Other No Smbios from Hob is installed.
+
+**/
+EFI_STATUS
+RetrieveSmbiosFromHob (
+ IN EFI_HANDLE ImageHandle
+ )
+{
+ EFI_STATUS Status;
+ UINTN Index;
+ SMBIOS_STRUCTURE_POINTER Smbios;
+ EFI_HOB_GUID_TYPE *GuidHob;
+ PLD_SMBIOS_TABLE *SmBiosTableAdress;
+ PLD_GENERIC_HEADER *GenericHeader;
+ VOID *TableAddress;
+ UINTN TableMaximumSize;
+
+ Status = EFI_NOT_FOUND;
+
+ for (Index = 0; Index < ARRAY_SIZE (mIsSmbiosTableValid); Index++) {
+ GuidHob = GetFirstGuidHob (mIsSmbiosTableValid[Index].Guid);
+ if (GuidHob == NULL) {
+ continue;
+ }
+ GenericHeader = (PLD_GENERIC_HEADER *) GET_GUID_HOB_DATA
(GuidHob);
+ if ((sizeof (PLD_GENERIC_HEADER) <= GET_GUID_HOB_DATA_SIZE
(GuidHob)) && (GenericHeader->Length <= GET_GUID_HOB_DATA_SIZE
(GuidHob))) {
+ if (GenericHeader->Revision == PLD_SMBIOS_TABLE_REVISION) {
+ //
+ // PLD_SMBIOS_TABLE structure is used when Revision equals to
PLD_SMBIOS_TABLE_REVISION
+ //
+ SmBiosTableAdress = (PLD_SMBIOS_TABLE *) GET_GUID_HOB_DATA
(GuidHob);
+ if (GenericHeader->Length >= PLD_SIZEOF_THROUGH_FIELD
(PLD_SMBIOS_TABLE, SmBiosEntryPoint)) {
+ if (mIsSmbiosTableValid[Index].IsValid ((VOID *)
(UINTN )SmBiosTableAdress->SmBiosEntryPoint, &TableAddress,
&TableMaximumSize)) {
+ Smbios.Raw = TableAddress;
+ Status = ParseAndAddExistingSmbiosTable (ImageHandle, Smbios,
TableMaximumSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "RetrieveSmbiosFromHob: Failed to
parse preinstalled tables from gPldSmbios3TableGuid Guid Hob\n"));
+ Status = EFI_UNSUPPORTED;
+ } else {
+ return EFI_SUCCESS;
+ }
+ }
+ }
+ }
+ }
+ }
+ return Status;
+}
+
/**

Driver to produce Smbios protocol and pre-allocate 1 page for the final
SMBIOS table.
@@ -1451,5 +1766,6 @@ SmbiosDriverEntryPoint (
&mPrivateData.Smbios
);

- return Status;
That doesn't compile as Status is written, but never read.

+ RetrieveSmbiosFromHob (ImageHandle); return EFI_SUCCESS;
}
diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.h
b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.h
index f97c85ae40..a260cf695e 100644
--- a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.h
+++ b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.h
@@ -1,7 +1,7 @@
/** @file
This code supports the implementation of the Smbios protocol

-Copyright (c) 2009 - 2018, Intel Corporation. All rights
reserved.<BR>
+Copyright (c) 2009 - 2021, Intel Corporation. All rights
+reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -24,6 +24,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/MemoryAllocationLib.h> #include
<Library/UefiBootServicesTableLib.h>
#include <Library/PcdLib.h>
+#include <Library/HobLib.h>
+#include <UniversalPayload/SmbiosTable.h>

#define SMBIOS_INSTANCE_SIGNATURE SIGNATURE_32 ('S', 'B', 'i', 's')
typedef struct { diff --git
a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
index f6c036e1dc..63f468936d 100644
--- a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
+++ b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
@@ -1,7 +1,7 @@
## @file
# This driver initializes and installs the SMBIOS protocol, constructs SMBIOS
table into system configuration table.
#
-# Copyright (c) 2009 - 2018, Intel Corporation. All rights
reserved.<BR>
+# Copyright (c) 2009 - 2021, Intel Corporation. All rights
+reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -41,6 +41,7 @@
UefiDriverEntryPoint
DebugLib
PcdLib
+ HobLib

[Protocols]
gEfiSmbiosProtocolGuid ## PRODUCES
@@ -48,6 +49,8 @@
[Guids]
gEfiSmbiosTableGuid ## SOMETIMES_PRODUCES ##
SystemTable
gEfiSmbios3TableGuid ## SOMETIMES_PRODUCES ##
SystemTable
+ gPldSmbios3TableGuid ## CONSUMES ## HOB
+ gPldSmbiosTableGuid ## SOMETIMES_CONSUMES ##
HOB

[Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion ## CONSUMES
--
2.30.0.windows.2

6061 - 6080 of 82317