[PATCH 5/9] MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables


Zhiguang Liu
 

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 | 299 +++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++--
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.h | 4 +++-
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf | 5 ++++-
3 files changed, 304 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c b/MdeModulePkg/Un=
iversal/SmbiosDxe/SmbiosDxe.c
index 3cdb0b1ed7..d2aa15d43f 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 construc=
ting=0D
SMBIOS table into system table.=0D
=0D
-Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>=0D
+Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.<BR>=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
=0D
**/=0D
@@ -1408,6 +1408,300 @@ SmbiosTableConstruction (
}=0D
}=0D
=0D
+/**=0D
+ Validates a SMBIOS 2.0 table entry point.=0D
+=0D
+ @param SmbiosTable The SMBIOS_TABLE_ENTRY_POINT to validate.=0D
+=0D
+ @retval TRUE SMBIOS table entry point is valid.=0D
+ @retval FALSE SMBIOS table entry point is malformed.=0D
+=0D
+**/=0D
+STATIC=0D
+BOOLEAN=0D
+IsValidSmbios20Table (=0D
+ IN SMBIOS_TABLE_ENTRY_POINT *SmbiosTable=0D
+ )=0D
+{=0D
+ UINT8 Checksum;=0D
+=0D
+ if (CompareMem (SmbiosTable->AnchorString, "_SM_", 4) !=3D 0) {=0D
+ return FALSE;=0D
+ }=0D
+=0D
+ //=0D
+ // The actual value of the EntryPointLength should be 1Fh.=0D
+ // However, it was incorrectly stated in version 2.1 of smbios specifica=
tion.=0D
+ // Therefore, 0x1F and 0x1E are both accepted.=0D
+ //=0D
+ if (SmbiosTable->EntryPointLength !=3D 0x1E || SmbiosTable->EntryPointLe=
ngth !=3D sizeof (SMBIOS_TABLE_ENTRY_POINT)) {=0D
+ return FALSE;=0D
+ }=0D
+=0D
+ //=0D
+ // MajorVersion should be 2.=0D
+ //=0D
+ if (SmbiosTable->MajorVersion !=3D 2) {=0D
+ return FALSE;=0D
+ }=0D
+=0D
+ //=0D
+ // The whole struct check sum should be zero=0D
+ //=0D
+ Checksum =3D CalculateSum8 (=0D
+ (UINT8 *) SmbiosTable,=0D
+ SmbiosTable->EntryPointLength=0D
+ );=0D
+ if (Checksum !=3D 0) {=0D
+ return FALSE;=0D
+ }=0D
+=0D
+ //=0D
+ // The Intermediate Entry Point Structure check sum should be zero.=0D
+ //=0D
+ Checksum =3D CalculateSum8 (=0D
+ (UINT8 *) SmbiosTable + OFFSET_OF (SMBIOS_TABLE_ENTRY_POINT=
, IntermediateAnchorString),=0D
+ SmbiosTable->EntryPointLength - OFFSET_OF (SMBIOS_TABLE_ENT=
RY_POINT, IntermediateAnchorString)=0D
+ );=0D
+ return (BOOLEAN) (Checksum =3D=3D 0);=0D
+}=0D
+=0D
+/**=0D
+ Validates a SMBIOS 3.0 table entry point.=0D
+=0D
+ @param SmbiosTable The SMBIOS_TABLE_3_0_ENTRY_POINT to validate.=0D
+=0D
+ @retval TRUE SMBIOS table entry point is valid.=0D
+ @retval FALSE SMBIOS table entry point is malformed.=0D
+=0D
+**/=0D
+STATIC=0D
+BOOLEAN=0D
+IsValidSmbios30Table (=0D
+ IN SMBIOS_TABLE_3_0_ENTRY_POINT *SmbiosTable=0D
+ )=0D
+{=0D
+ UINT8 Checksum;=0D
+=0D
+ if (CompareMem (SmbiosTable->AnchorString, "_SM3_", 5) !=3D 0) {=0D
+ return FALSE;=0D
+ }=0D
+ if (SmbiosTable->EntryPointLength < sizeof (SMBIOS_TABLE_3_0_ENTRY_POINT=
)) {=0D
+ return FALSE;=0D
+ }=0D
+ if (SmbiosTable->MajorVersion < 3) {=0D
+ return FALSE;=0D
+ }=0D
+=0D
+ //=0D
+ // The whole struct check sum should be zero=0D
+ //=0D
+ Checksum =3D CalculateSum8 (=0D
+ (UINT8 *) SmbiosTable,=0D
+ SmbiosTable->EntryPointLength=0D
+ );=0D
+ if (Checksum !=3D 0) {=0D
+ return FALSE;=0D
+ }=0D
+ return TRUE;=0D
+}=0D
+=0D
+/**=0D
+ Parse an existing SMBIOS table and insert it using SmbiosAdd.=0D
+=0D
+ @param ImageHandle The EFI_HANDLE to this driver.=0D
+ @param Smbios The SMBIOS table to parse.=0D
+ @param Length The length of the SMBIOS table.=0D
+=0D
+ @retval EFI_SUCCESS SMBIOS table was parsed and installed.=0D
+ @retval EFI_OUT_OF_RESOURCES Record was not added due to lack of system=
resources.=0D
+ @retval EFI_INVALID_PARAMETER Smbios is not a correct smbios table=0D
+=0D
+**/=0D
+STATIC=0D
+EFI_STATUS=0D
+ParseAndAddExistingSmbiosTable (=0D
+ IN EFI_HANDLE ImageHandle,=0D
+ IN SMBIOS_STRUCTURE_POINTER Smbios,=0D
+ IN UINTN Length=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ CHAR8 *String;=0D
+ EFI_SMBIOS_HANDLE SmbiosHandle;=0D
+ SMBIOS_STRUCTURE_POINTER SmbiosEnd;=0D
+=0D
+ SmbiosEnd.Raw =3D Smbios.Raw + Length;=0D
+=0D
+ if (Smbios.Raw >=3D SmbiosEnd.Raw || Smbios.Raw =3D=3D NULL) {=0D
+ return EFI_INVALID_PARAMETER;=0D
+ }=0D
+=0D
+ do {=0D
+ //=0D
+ // Make sure not to access memory beyond SmbiosEnd=0D
+ //=0D
+ if (Smbios.Raw + sizeof (SMBIOS_STRUCTURE) > SmbiosEnd.Raw ||=0D
+ Smbios.Raw + sizeof (SMBIOS_STRUCTURE) < Smbios.Raw) {=0D
+ return EFI_INVALID_PARAMETER;=0D
+ }=0D
+ //=0D
+ // Check for end marker=0D
+ //=0D
+ if (Smbios.Hdr->Type =3D=3D SMBIOS_TYPE_END_OF_TABLE) {=0D
+ break;=0D
+ }=0D
+ //=0D
+ // Make sure not to access memory beyond SmbiosEnd=0D
+ // Each structure shall be terminated by a double-null (0000h).=0D
+ //=0D
+ if (Smbios.Raw + Smbios.Hdr->Length + 2 * sizeof (UINT8) > SmbiosEnd.R=
aw ||=0D
+ Smbios.Raw + Smbios.Hdr->Length + 2 * sizeof (UINT8) < Smbios.Raw) {=
=0D
+ return EFI_INVALID_PARAMETER;=0D
+ }=0D
+ //=0D
+ // Install the table=0D
+ //=0D
+ SmbiosHandle =3D Smbios.Hdr->Handle;=0D
+ Status =3D SmbiosAdd (=0D
+ &mPrivateData.Smbios,=0D
+ ImageHandle,=0D
+ &SmbiosHandle,=0D
+ Smbios.Hdr=0D
+ );=0D
+=0D
+ ASSERT_EFI_ERROR (Status);=0D
+ if (EFI_ERROR (Status)) {=0D
+ return Status;=0D
+ }=0D
+ //=0D
+ // Go to the next SMBIOS structure. Each SMBIOS structure may include =
2 parts:=0D
+ // 1. Formatted section; 2. Unformatted string section. So, 2 steps ar=
e needed=0D
+ // to skip one SMBIOS structure.=0D
+ //=0D
+=0D
+ //=0D
+ // Step 1: Skip over formatted section.=0D
+ //=0D
+ String =3D (CHAR8 *) (Smbios.Raw + Smbios.Hdr->Length);=0D
+=0D
+ //=0D
+ // Step 2: Skip over unformatted string section.=0D
+ //=0D
+ do {=0D
+ //=0D
+ // Each string is terminated with a NULL(00h) BYTE and the sets of s=
trings=0D
+ // is terminated with an additional NULL(00h) BYTE.=0D
+ //=0D
+ for ( ; *String !=3D 0; String++) {=0D
+ if ((UINTN) String >=3D (UINTN) SmbiosEnd.Raw - sizeof (UINT8)) {=
=0D
+ return EFI_INVALID_PARAMETER;=0D
+ }=0D
+ }=0D
+=0D
+ if (*(UINT8 *) ++String =3D=3D 0) {=0D
+ //=0D
+ // Pointer to the next SMBIOS structure.=0D
+ //=0D
+ Smbios.Raw =3D (UINT8 *) ++String;=0D
+ break;=0D
+ }=0D
+ } while (TRUE);=0D
+ } while (Smbios.Raw < SmbiosEnd.Raw);=0D
+=0D
+ return EFI_SUCCESS;=0D
+}=0D
+=0D
+=0D
+/**=0D
+ Retrieve SMBIOS from Hob.=0D
+ @param ImageHandle Module's image handle=0D
+=0D
+ @retval EFI_SUCCESS Smbios from Hob is installed.=0D
+ @return EFI_NOT_FOUND Not found Smbios from Hob.=0D
+ @retval Other No Smbios from Hob is installed.=0D
+=0D
+**/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+RetrieveSmbiosFromHob (=0D
+ IN EFI_HANDLE ImageHandle=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ SMBIOS_TABLE_ENTRY_POINT *SmbiosTable;=0D
+ SMBIOS_TABLE_3_0_ENTRY_POINT *Smbios30Table;=0D
+ SMBIOS_STRUCTURE_POINTER Smbios;=0D
+ EFI_HOB_GUID_TYPE *GuidHob;=0D
+ PLD_SMBIOS_TABLE *SmBiosTableAdress;=0D
+ PLD_GENERIC_HEADER *GenericHeader;=0D
+=0D
+ Status =3D EFI_NOT_FOUND;=0D
+ //=0D
+ // Scan for existing SMBIOS tables from gPldSmbios3TableGuid Guid Hob=0D
+ //=0D
+ GuidHob =3D GetFirstGuidHob (&gPldSmbios3TableGuid);=0D
+ if (GuidHob !=3D NULL) {=0D
+ GenericHeader =3D (PLD_GENERIC_HEADER *) GET_GUID_HOB_DATA (GuidHob);=
=0D
+ if ((sizeof (PLD_GENERIC_HEADER) <=3D GET_GUID_HOB_DATA_SIZE (GuidHob)=
) && (GenericHeader->Length <=3D GET_GUID_HOB_DATA_SIZE (GuidHob))) {=0D
+ if (GenericHeader->Revision =3D=3D PLD_SMBIOS_TABLE_REVISION) {=0D
+ //=0D
+ // PLD_SMBIOS_TABLE structure is used when Revision equals to PLD_=
SMBIOS_TABLE_REVISION=0D
+ //=0D
+ SmBiosTableAdress =3D (PLD_SMBIOS_TABLE *) GET_GUID_HOB_DATA (Guid=
Hob);=0D
+ if (GenericHeader->Length >=3D PLD_SIZEOF_THROUGH_FIELD (PLD_SMBIO=
S_TABLE, SmBiosEntryPoint)) {=0D
+ Smbios30Table =3D (SMBIOS_TABLE_3_0_ENTRY_POINT *) (UINTN) SmBio=
sTableAdress->SmBiosEntryPoint;=0D
+ if (IsValidSmbios30Table (Smbios30Table)) {=0D
+ Smbios.Raw =3D (UINT8 *) (UINTN) Smbios30Table->TableAddress;=
=0D
+ Status =3D ParseAndAddExistingSmbiosTable (ImageHandle, Smbios=
, Smbios30Table->TableMaximumSize);=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_ERROR, "RetrieveSmbiosFromHob: Failed to parse=
preinstalled tables from gPldSmbios3TableGuid Guid Hob\n"));=0D
+ Status =3D EFI_UNSUPPORTED;=0D
+ } else {=0D
+ return EFI_SUCCESS;=0D
+ }=0D
+ }=0D
+ }=0D
+ } else {=0D
+ Status =3D EFI_UNSUPPORTED;=0D
+ }=0D
+ }=0D
+ }=0D
+=0D
+ //=0D
+ // Scan for existing SMBIOS tables from gPldSmbiosTableGuid Guid Hob,=0D
+ // if gPldSmbios3TableGuid Hob doesn't exist or parsing gPldSmbios3Table=
Guid failed=0D
+ //=0D
+ GuidHob =3D GetFirstGuidHob (&gPldSmbiosTableGuid);=0D
+ =0D
+ if (GuidHob !=3D NULL) {=0D
+ GenericHeader =3D (PLD_GENERIC_HEADER *) GET_GUID_HOB_DATA (GuidHob);=
=0D
+ if ((sizeof (PLD_GENERIC_HEADER) <=3D GET_GUID_HOB_DATA_SIZE (GuidHob)=
) && (GenericHeader->Length <=3D GET_GUID_HOB_DATA_SIZE (GuidHob))) {=0D
+ if (GenericHeader->Revision =3D=3D PLD_SMBIOS_TABLE_REVISION) {=0D
+ //=0D
+ // PLD_SMBIOS_TABLE structure is used when Revision equals to PLD_=
SMBIOS_TABLE_REVISION=0D
+ //=0D
+ SmBiosTableAdress =3D (PLD_SMBIOS_TABLE *) GET_GUID_HOB_DATA (Guid=
Hob);=0D
+ if (GenericHeader->Length >=3D PLD_SIZEOF_THROUGH_FIELD (PLD_SMBIO=
S_TABLE, SmBiosEntryPoint)) {=0D
+ SmbiosTable =3D (SMBIOS_TABLE_ENTRY_POINT *) (UINTN) SmBiosTable=
Adress->SmBiosEntryPoint;=0D
+ if (IsValidSmbios20Table (SmbiosTable)) {=0D
+ Smbios.Raw =3D (UINT8 *) (UINTN) SmbiosTable->TableAddress;=0D
+ Status =3D ParseAndAddExistingSmbiosTable (ImageHandle, Smbios=
, SmbiosTable->TableLength);=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_ERROR, "RetrieveSmbiosFromHob: Failed to parse=
preinstalled tables from gPldSmbiosTableGuid Guid Hob\n"));=0D
+ Status =3D EFI_UNSUPPORTED;=0D
+ }=0D
+ return EFI_SUCCESS;=0D
+ }=0D
+ }=0D
+ } else {=0D
+ Status =3D EFI_UNSUPPORTED;=0D
+ }=0D
+ }=0D
+ }=0D
+ return Status;=0D
+}=0D
+=0D
/**=0D
=0D
Driver to produce Smbios protocol and pre-allocate 1 page for the final =
SMBIOS table.=0D
@@ -1451,5 +1745,6 @@ SmbiosDriverEntryPoint (
&mPrivateData.Smbios=0D
);=0D
=0D
- return Status;=0D
+ RetrieveSmbiosFromHob (ImageHandle);=0D
+ return EFI_SUCCESS;=0D
}=0D
diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.h b/MdeModulePkg/Un=
iversal/SmbiosDxe/SmbiosDxe.h
index f97c85ae40..a260cf695e 100644
--- a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.h
+++ b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.h
@@ -1,7 +1,7 @@
/** @file=0D
This code supports the implementation of the Smbios protocol=0D
=0D
-Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>=0D
+Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.<BR>=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
=0D
**/=0D
@@ -24,6 +24,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/MemoryAllocationLib.h>=0D
#include <Library/UefiBootServicesTableLib.h>=0D
#include <Library/PcdLib.h>=0D
+#include <Library/HobLib.h>=0D
+#include <UniversalPayload/SmbiosTable.h>=0D
=0D
#define SMBIOS_INSTANCE_SIGNATURE SIGNATURE_32 ('S', 'B', 'i', 's')=0D
typedef struct {=0D
diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf b/MdeModulePkg/=
Universal/SmbiosDxe/SmbiosDxe.inf
index f6c036e1dc..3286575098 100644
--- a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
+++ b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
@@ -1,7 +1,7 @@
## @file=0D
# This driver initializes and installs the SMBIOS protocol, constructs SMB=
IOS table into system configuration table.=0D
#=0D
-# Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>=0D
+# Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.<BR>=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -41,6 +41,7 @@
UefiDriverEntryPoint=0D
DebugLib=0D
PcdLib=0D
+ HobLib=0D
=0D
[Protocols]=0D
gEfiSmbiosProtocolGuid ## PRODUCES=0D
@@ -48,6 +49,8 @@
[Guids]=0D
gEfiSmbiosTableGuid ## SOMETIMES_PRODUCES =
## SystemTable=0D
gEfiSmbios3TableGuid ## SOMETIMES_PRODUCES =
## SystemTable=0D
+ gPldSmbios3TableGuid=0D
+ gPldSmbiosTableGuid=0D
=0D
[Pcd]=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion ## CONSUMES=0D
--=20
2.30.0.windows.2


Wu, Hao A
 

Adding Ray.

Hello Ray, I saw most of your comments in previous patch:
"[PATCH - resend] MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables"
have been addressed in this patch series.
Could you help to check if there are additional comments for this patch? Thanks.

Some inline comments below:

-----Original Message-----
From: Liu, Zhiguang <zhiguang.liu@intel.com>
Sent: Monday, May 24, 2021 3:13 PM
To: devel@edk2.groups.io
Cc: 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>; Patrick Rudolph
<patrick.rudolph@9elements.com>
Subject: [PATCH 5/9] MdeModulePkg/Universal/SmbiosDxe: Scan for
existing tables

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 | 299
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+++++++--
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.h | 4 +++-
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf | 5 ++++-
3 files changed, 304 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
index 3cdb0b1ed7..d2aa15d43f 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



**/

@@ -1408,6 +1408,300 @@ SmbiosTableConstruction (
}

}



+/**

+ Validates a SMBIOS 2.0 table entry point.

+

+ @param SmbiosTable The SMBIOS_TABLE_ENTRY_POINT to validate.

+

+ @retval TRUE SMBIOS table entry point is valid.

+ @retval FALSE SMBIOS table entry point is malformed.

+

+**/

+STATIC

+BOOLEAN

+IsValidSmbios20Table (

+ IN SMBIOS_TABLE_ENTRY_POINT *SmbiosTable

+ )

+{

+ UINT8 Checksum;

+

+ if (CompareMem (SmbiosTable->AnchorString, "_SM_", 4) != 0) {

+ return FALSE;

+ }

Should a check for the 'IntermediateAnchorString' be added here as well?



+

+ //

+ // 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 be 2.

+ //

+ if (SmbiosTable->MajorVersion != 2) {

+ return FALSE;

I might be wrong on this.
My take with the SMBIOS spec is that a 2.0 table is allowed to has this field greater than 2.
As long as a specific version of the SMBIOS spec still support this format (which is still true for version 3.0).

So I think I check like:
if (EntryPointStructure->MajorVersion < 2) {...}
should be used here.



+ }

+

+ //

+ // 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)

+ );

+ return (BOOLEAN) (Checksum == 0);

+}

+

+/**

+ Validates a SMBIOS 3.0 table entry point.

+

+ @param SmbiosTable The SMBIOS_TABLE_3_0_ENTRY_POINT to validate.

+

+ @retval TRUE SMBIOS table entry point is valid.

+ @retval FALSE SMBIOS table entry point is malformed.

+

+**/

+STATIC

+BOOLEAN

+IsValidSmbios30Table (

+ IN SMBIOS_TABLE_3_0_ENTRY_POINT *SmbiosTable

+ )

+{

+ UINT8 Checksum;

+

+ 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;

+ }

+ 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;

+}

+

+

+/**

+ 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

+EFIAPI

I think RetrieveSmbiosFromHob() is an internal function.
Please help to remove the 'EFIAPI' keyword here.



+RetrieveSmbiosFromHob (

+ IN EFI_HANDLE ImageHandle

+ )

+{

+ EFI_STATUS Status;

+ SMBIOS_TABLE_ENTRY_POINT *SmbiosTable;

+ SMBIOS_TABLE_3_0_ENTRY_POINT *Smbios30Table;

+ SMBIOS_STRUCTURE_POINTER Smbios;

+ EFI_HOB_GUID_TYPE *GuidHob;

+ PLD_SMBIOS_TABLE *SmBiosTableAdress;

+ PLD_GENERIC_HEADER *GenericHeader;

+

+ Status = EFI_NOT_FOUND;

+ //

+ // Scan for existing SMBIOS tables from gPldSmbios3TableGuid Guid Hob

+ //

+ GuidHob = GetFirstGuidHob (&gPldSmbios3TableGuid);

+ if (GuidHob != NULL) {

+ 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)) {

+ Smbios30Table = (SMBIOS_TABLE_3_0_ENTRY_POINT *) (UINTN)
+ SmBiosTableAdress->SmBiosEntryPoint;

+ if (IsValidSmbios30Table (Smbios30Table)) {

+ Smbios.Raw = (UINT8 *) (UINTN) Smbios30Table->TableAddress;

+ Status = ParseAndAddExistingSmbiosTable (ImageHandle,
+ Smbios, Smbios30Table->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;

+ }

+ }

+ }

+ } else {

+ Status = EFI_UNSUPPORTED;

+ }

+ }

+ }

+

+ //

+ // Scan for existing SMBIOS tables from gPldSmbiosTableGuid Guid Hob,

+ // if gPldSmbios3TableGuid Hob doesn't exist or parsing
+ gPldSmbios3TableGuid failed

+ //

+ GuidHob = GetFirstGuidHob (&gPldSmbiosTableGuid);

+

+ if (GuidHob != NULL) {

+ 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)) {

+ SmbiosTable = (SMBIOS_TABLE_ENTRY_POINT *) (UINTN)
+ SmBiosTableAdress->SmBiosEntryPoint;

+ if (IsValidSmbios20Table (SmbiosTable)) {

+ Smbios.Raw = (UINT8 *) (UINTN) SmbiosTable->TableAddress;

+ Status = ParseAndAddExistingSmbiosTable (ImageHandle,
+ Smbios, SmbiosTable->TableLength);

+ if (EFI_ERROR (Status)) {

+ DEBUG ((DEBUG_ERROR, "RetrieveSmbiosFromHob: Failed to
+ parse preinstalled tables from gPldSmbiosTableGuid Guid Hob\n"));

+ Status = EFI_UNSUPPORTED;

+ }

+ return EFI_SUCCESS;

+ }

+ }

+ } else {

+ Status = EFI_UNSUPPORTED;

+ }

+ }

+ }

Is it possible to abstract the codes that:
a) Search & parse of gPldSmbios3TableGuid
b) Search & parse of gPldSmbiosTableGuid
into a function?

I found that most of the logic is identical.

Best Regards,
Hao Wu



+ return Status;

+}

+

/**



Driver to produce Smbios protocol and pre-allocate 1 page for the final
SMBIOS table.

@@ -1451,5 +1745,6 @@ SmbiosDriverEntryPoint (
&mPrivateData.Smbios

);



- return Status;

+ 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..3286575098 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

+ gPldSmbiosTableGuid



[Pcd]

gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion ## CONSUMES

--
2.30.0.windows.2


Wu, Hao A
 

Sorry for missing one comment.
I think we can add below information for GUID information in SmbiosDxe.inf:

gPldSmbios3TableGuid ## CONSUMES ## HOB
gPldSmbiosTableGuid ## SOMETIMES_CONSUMES ## HOB

Best Regards,
Hao Wu

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao
A
Sent: Wednesday, May 26, 2021 2:15 PM
To: Liu, Zhiguang <zhiguang.liu@intel.com>; Ni, Ray <ray.ni@intel.com>;
devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; Bi, Dandan
<dandan.bi@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Zhichao
<zhichao.gao@intel.com>; Patrick Rudolph
<patrick.rudolph@9elements.com>
Subject: Re: [edk2-devel] [PATCH 5/9]
MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables

Adding Ray.

Hello Ray, I saw most of your comments in previous patch:
"[PATCH - resend] MdeModulePkg/Universal/SmbiosDxe: Scan for existing
tables"
have been addressed in this patch series.
Could you help to check if there are additional comments for this patch?
Thanks.

Some inline comments below:


-----Original Message-----
From: Liu, Zhiguang <zhiguang.liu@intel.com>
Sent: Monday, May 24, 2021 3:13 PM
To: devel@edk2.groups.io
Cc: 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>; Patrick
Rudolph <patrick.rudolph@9elements.com>
Subject: [PATCH 5/9] MdeModulePkg/Universal/SmbiosDxe: Scan for
existing tables

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 | 299
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+++++++--
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.h | 4 +++-
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf | 5 ++++-
3 files changed, 304 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
index 3cdb0b1ed7..d2aa15d43f 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



**/

@@ -1408,6 +1408,300 @@ SmbiosTableConstruction (
}

}



+/**

+ Validates a SMBIOS 2.0 table entry point.

+

+ @param SmbiosTable The SMBIOS_TABLE_ENTRY_POINT to validate.

+

+ @retval TRUE SMBIOS table entry point is valid.

+ @retval FALSE SMBIOS table entry point is malformed.

+

+**/

+STATIC

+BOOLEAN

+IsValidSmbios20Table (

+ IN SMBIOS_TABLE_ENTRY_POINT *SmbiosTable

+ )

+{

+ UINT8 Checksum;

+

+ if (CompareMem (SmbiosTable->AnchorString, "_SM_", 4) != 0) {

+ return FALSE;

+ }

Should a check for the 'IntermediateAnchorString' be added here as well?



+

+ //

+ // 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 be 2.

+ //

+ if (SmbiosTable->MajorVersion != 2) {

+ return FALSE;

I might be wrong on this.
My take with the SMBIOS spec is that a 2.0 table is allowed to has this field
greater than 2.
As long as a specific version of the SMBIOS spec still support this format
(which is still true for version 3.0).

So I think I check like:
if (EntryPointStructure->MajorVersion < 2) {...} should be used here.



+ }

+

+ //

+ // 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)

+ );

+ return (BOOLEAN) (Checksum == 0);

+}

+

+/**

+ Validates a SMBIOS 3.0 table entry point.

+

+ @param SmbiosTable The SMBIOS_TABLE_3_0_ENTRY_POINT to
validate.

+

+ @retval TRUE SMBIOS table entry point is valid.

+ @retval FALSE SMBIOS table entry point is malformed.

+

+**/

+STATIC

+BOOLEAN

+IsValidSmbios30Table (

+ IN SMBIOS_TABLE_3_0_ENTRY_POINT *SmbiosTable

+ )

+{

+ UINT8 Checksum;

+

+ 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;

+ }

+ 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;

+}

+

+

+/**

+ 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

+EFIAPI

I think RetrieveSmbiosFromHob() is an internal function.
Please help to remove the 'EFIAPI' keyword here.



+RetrieveSmbiosFromHob (

+ IN EFI_HANDLE ImageHandle

+ )

+{

+ EFI_STATUS Status;

+ SMBIOS_TABLE_ENTRY_POINT *SmbiosTable;

+ SMBIOS_TABLE_3_0_ENTRY_POINT *Smbios30Table;

+ SMBIOS_STRUCTURE_POINTER Smbios;

+ EFI_HOB_GUID_TYPE *GuidHob;

+ PLD_SMBIOS_TABLE *SmBiosTableAdress;

+ PLD_GENERIC_HEADER *GenericHeader;

+

+ Status = EFI_NOT_FOUND;

+ //

+ // Scan for existing SMBIOS tables from gPldSmbios3TableGuid Guid
+ Hob

+ //

+ GuidHob = GetFirstGuidHob (&gPldSmbios3TableGuid);

+ if (GuidHob != NULL) {

+ 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)) {

+ Smbios30Table = (SMBIOS_TABLE_3_0_ENTRY_POINT *) (UINTN)
+ SmBiosTableAdress->SmBiosEntryPoint;

+ if (IsValidSmbios30Table (Smbios30Table)) {

+ Smbios.Raw = (UINT8 *) (UINTN)
+ Smbios30Table->TableAddress;

+ Status = ParseAndAddExistingSmbiosTable (ImageHandle,
+ Smbios, Smbios30Table->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;

+ }

+ }

+ }

+ } else {

+ Status = EFI_UNSUPPORTED;

+ }

+ }

+ }

+

+ //

+ // Scan for existing SMBIOS tables from gPldSmbiosTableGuid Guid
+ Hob,

+ // if gPldSmbios3TableGuid Hob doesn't exist or parsing
+ gPldSmbios3TableGuid failed

+ //

+ GuidHob = GetFirstGuidHob (&gPldSmbiosTableGuid);

+

+ if (GuidHob != NULL) {

+ 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)) {

+ SmbiosTable = (SMBIOS_TABLE_ENTRY_POINT *) (UINTN)
+ SmBiosTableAdress->SmBiosEntryPoint;

+ if (IsValidSmbios20Table (SmbiosTable)) {

+ Smbios.Raw = (UINT8 *) (UINTN) SmbiosTable->TableAddress;

+ Status = ParseAndAddExistingSmbiosTable (ImageHandle,
+ Smbios, SmbiosTable->TableLength);

+ if (EFI_ERROR (Status)) {

+ DEBUG ((DEBUG_ERROR, "RetrieveSmbiosFromHob: Failed to
+ parse preinstalled tables from gPldSmbiosTableGuid Guid Hob\n"));

+ Status = EFI_UNSUPPORTED;

+ }

+ return EFI_SUCCESS;

+ }

+ }

+ } else {

+ Status = EFI_UNSUPPORTED;

+ }

+ }

+ }

Is it possible to abstract the codes that:
a) Search & parse of gPldSmbios3TableGuid
b) Search & parse of gPldSmbiosTableGuid into a function?

I found that most of the logic is identical.

Best Regards,
Hao Wu



+ return Status;

+}

+

/**



Driver to produce Smbios protocol and pre-allocate 1 page for the
final SMBIOS table.

@@ -1451,5 +1745,6 @@ SmbiosDriverEntryPoint (
&mPrivateData.Smbios

);



- return Status;

+ 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..3286575098 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

+ gPldSmbiosTableGuid



[Pcd]

gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion ## CONSUMES

--
2.30.0.windows.2




Zhiguang Liu
 

Hi Hao,

Thanks for the comments. I will send another version soon.

Thanks
Zhiguang

-----Original Message-----
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Wednesday, May 26, 2021 2:33 PM
To: devel@edk2.groups.io; Wu, Hao A <hao.a.wu@intel.com>; Liu, Zhiguang
<zhiguang.liu@intel.com>; Ni, Ray <ray.ni@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Bi, Dandan
<dandan.bi@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Zhichao
<zhichao.gao@intel.com>; Patrick Rudolph
<patrick.rudolph@9elements.com>
Subject: RE: [edk2-devel] [PATCH 5/9] MdeModulePkg/Universal/SmbiosDxe:
Scan for existing tables

Sorry for missing one comment.
I think we can add below information for GUID information in SmbiosDxe.inf:

gPldSmbios3TableGuid ## CONSUMES ## HOB
gPldSmbiosTableGuid ## SOMETIMES_CONSUMES ## HOB

Best Regards,
Hao Wu


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu,
Hao
A
Sent: Wednesday, May 26, 2021 2:15 PM
To: Liu, Zhiguang <zhiguang.liu@intel.com>; Ni, Ray
<ray.ni@intel.com>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; Bi, Dandan
<dandan.bi@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Zhichao
<zhichao.gao@intel.com>; Patrick Rudolph
<patrick.rudolph@9elements.com>
Subject: Re: [edk2-devel] [PATCH 5/9]
MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables

Adding Ray.

Hello Ray, I saw most of your comments in previous patch:
"[PATCH - resend] MdeModulePkg/Universal/SmbiosDxe: Scan for existing
tables"
have been addressed in this patch series.
Could you help to check if there are additional comments for this patch?
Thanks.

Some inline comments below:


-----Original Message-----
From: Liu, Zhiguang <zhiguang.liu@intel.com>
Sent: Monday, May 24, 2021 3:13 PM
To: devel@edk2.groups.io
Cc: 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>; Patrick
Rudolph <patrick.rudolph@9elements.com>
Subject: [PATCH 5/9] MdeModulePkg/Universal/SmbiosDxe: Scan for
existing tables

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 | 299
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+++++++--
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.h | 4 +++-
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf | 5 ++++-
3 files changed, 304 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
index 3cdb0b1ed7..d2aa15d43f 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



**/

@@ -1408,6 +1408,300 @@ SmbiosTableConstruction (
}

}



+/**

+ Validates a SMBIOS 2.0 table entry point.

+

+ @param SmbiosTable The SMBIOS_TABLE_ENTRY_POINT to validate.

+

+ @retval TRUE SMBIOS table entry point is valid.

+ @retval FALSE SMBIOS table entry point is malformed.

+

+**/

+STATIC

+BOOLEAN

+IsValidSmbios20Table (

+ IN SMBIOS_TABLE_ENTRY_POINT *SmbiosTable

+ )

+{

+ UINT8 Checksum;

+

+ if (CompareMem (SmbiosTable->AnchorString, "_SM_", 4) != 0) {

+ return FALSE;

+ }

Should a check for the 'IntermediateAnchorString' be added here as well?



+

+ //

+ // 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 be 2.

+ //

+ if (SmbiosTable->MajorVersion != 2) {

+ return FALSE;

I might be wrong on this.
My take with the SMBIOS spec is that a 2.0 table is allowed to has
this field greater than 2.
As long as a specific version of the SMBIOS spec still support this
format (which is still true for version 3.0).

So I think I check like:
if (EntryPointStructure->MajorVersion < 2) {...} should be used here.



+ }

+

+ //

+ // 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)

+ );

+ return (BOOLEAN) (Checksum == 0);

+}

+

+/**

+ Validates a SMBIOS 3.0 table entry point.

+

+ @param SmbiosTable The SMBIOS_TABLE_3_0_ENTRY_POINT to
validate.

+

+ @retval TRUE SMBIOS table entry point is valid.

+ @retval FALSE SMBIOS table entry point is malformed.

+

+**/

+STATIC

+BOOLEAN

+IsValidSmbios30Table (

+ IN SMBIOS_TABLE_3_0_ENTRY_POINT *SmbiosTable

+ )

+{

+ UINT8 Checksum;

+

+ 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;

+ }

+ 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;

+}

+

+

+/**

+ 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

+EFIAPI

I think RetrieveSmbiosFromHob() is an internal function.
Please help to remove the 'EFIAPI' keyword here.



+RetrieveSmbiosFromHob (

+ IN EFI_HANDLE ImageHandle

+ )

+{

+ EFI_STATUS Status;

+ SMBIOS_TABLE_ENTRY_POINT *SmbiosTable;

+ SMBIOS_TABLE_3_0_ENTRY_POINT *Smbios30Table;

+ SMBIOS_STRUCTURE_POINTER Smbios;

+ EFI_HOB_GUID_TYPE *GuidHob;

+ PLD_SMBIOS_TABLE *SmBiosTableAdress;

+ PLD_GENERIC_HEADER *GenericHeader;

+

+ Status = EFI_NOT_FOUND;

+ //

+ // Scan for existing SMBIOS tables from gPldSmbios3TableGuid Guid
+ Hob

+ //

+ GuidHob = GetFirstGuidHob (&gPldSmbios3TableGuid);

+ if (GuidHob != NULL) {

+ 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)) {

+ Smbios30Table = (SMBIOS_TABLE_3_0_ENTRY_POINT *) (UINTN)
+ SmBiosTableAdress->SmBiosEntryPoint;

+ if (IsValidSmbios30Table (Smbios30Table)) {

+ Smbios.Raw = (UINT8 *) (UINTN)
+ Smbios30Table->TableAddress;

+ Status = ParseAndAddExistingSmbiosTable (ImageHandle,
+ Smbios, Smbios30Table->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;

+ }

+ }

+ }

+ } else {

+ Status = EFI_UNSUPPORTED;

+ }

+ }

+ }

+

+ //

+ // Scan for existing SMBIOS tables from gPldSmbiosTableGuid Guid
+ Hob,

+ // if gPldSmbios3TableGuid Hob doesn't exist or parsing
+ gPldSmbios3TableGuid failed

+ //

+ GuidHob = GetFirstGuidHob (&gPldSmbiosTableGuid);

+

+ if (GuidHob != NULL) {

+ 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)) {

+ SmbiosTable = (SMBIOS_TABLE_ENTRY_POINT *) (UINTN)
+ SmBiosTableAdress->SmBiosEntryPoint;

+ if (IsValidSmbios20Table (SmbiosTable)) {

+ Smbios.Raw = (UINT8 *) (UINTN)
+ SmbiosTable->TableAddress;

+ Status = ParseAndAddExistingSmbiosTable (ImageHandle,
+ Smbios, SmbiosTable->TableLength);

+ if (EFI_ERROR (Status)) {

+ DEBUG ((DEBUG_ERROR, "RetrieveSmbiosFromHob: Failed
+ to parse preinstalled tables from gPldSmbiosTableGuid Guid
+ Hob\n"));

+ Status = EFI_UNSUPPORTED;

+ }

+ return EFI_SUCCESS;

+ }

+ }

+ } else {

+ Status = EFI_UNSUPPORTED;

+ }

+ }

+ }

Is it possible to abstract the codes that:
a) Search & parse of gPldSmbios3TableGuid
b) Search & parse of gPldSmbiosTableGuid into a function?

I found that most of the logic is identical.

Best Regards,
Hao Wu



+ return Status;

+}

+

/**



Driver to produce Smbios protocol and pre-allocate 1 page for the
final SMBIOS table.

@@ -1451,5 +1745,6 @@ SmbiosDriverEntryPoint (
&mPrivateData.Smbios

);



- return Status;

+ 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..3286575098 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

+ gPldSmbiosTableGuid



[Pcd]

gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion ##
CONSUMES

--
2.30.0.windows.2




Patrick Rudolph
 


On Mon, May 24, 2021 at 9:13 AM Zhiguang Liu <zhiguang.liu@...> 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@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Dandan Bi <dandan.bi@...>
Cc: Star Zeng <star.zeng@...>
Cc: Zhichao Gao <zhichao.gao@...>
Signed-off-by: Patrick Rudolph <patrick.rudolph@...>
Signed-off-by: Zhiguang Liu <zhiguang.liu@...>
---
 MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c   | 299 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.h   |   4 +++-
 MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf |   5 ++++-
 3 files changed, 304 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
index 3cdb0b1ed7..d2aa15d43f 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

 **/
@@ -1408,6 +1408,300 @@ SmbiosTableConstruction (
   }
 }

+/**
+  Validates a SMBIOS 2.0 table entry point.
+
+  @param  SmbiosTable   The SMBIOS_TABLE_ENTRY_POINT to validate.
+
+  @retval TRUE           SMBIOS table entry point is valid.
+  @retval FALSE          SMBIOS table entry point is malformed.
+
+**/
+STATIC
+BOOLEAN
+IsValidSmbios20Table (
+  IN SMBIOS_TABLE_ENTRY_POINT      *SmbiosTable
+  )
+{
+  UINT8 Checksum;
+
+  if (CompareMem (SmbiosTable->AnchorString, "_SM_", 4) != 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;
+  }
+

This is not correct, it should be

if (SmbiosTable->EntryPointLength != 0x1E && SmbiosTable->EntryPointLength != sizeof (SMBIOS_TABLE_ENTRY_POINT)) {

otherwise the table wouldn't be recognized as valid.

+  //
+  // MajorVersion should be 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)
+               );
+  return (BOOLEAN) (Checksum == 0);
+}
+
+/**
+  Validates a SMBIOS 3.0 table entry point.
+
+  @param  SmbiosTable   The SMBIOS_TABLE_3_0_ENTRY_POINT to validate.
+
+  @retval TRUE           SMBIOS table entry point is valid.
+  @retval FALSE          SMBIOS table entry point is malformed.
+
+**/
+STATIC
+BOOLEAN
+IsValidSmbios30Table (
+  IN SMBIOS_TABLE_3_0_ENTRY_POINT  *SmbiosTable
+  )
+{
+  UINT8 Checksum;
+
+  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;
+  }
+  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;
+}
+
+
+/**
+  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
+EFIAPI
+RetrieveSmbiosFromHob (
+  IN EFI_HANDLE           ImageHandle
+  )
+{
+  EFI_STATUS                    Status;
+  SMBIOS_TABLE_ENTRY_POINT      *SmbiosTable;
+  SMBIOS_TABLE_3_0_ENTRY_POINT  *Smbios30Table;
+  SMBIOS_STRUCTURE_POINTER      Smbios;
+  EFI_HOB_GUID_TYPE             *GuidHob;
+  PLD_SMBIOS_TABLE              *SmBiosTableAdress;
+  PLD_GENERIC_HEADER            *GenericHeader;
+
+  Status = EFI_NOT_FOUND;
+  //
+  // Scan for existing SMBIOS tables from gPldSmbios3TableGuid Guid Hob
+  //
+  GuidHob = GetFirstGuidHob (&gPldSmbios3TableGuid);
+  if (GuidHob != NULL) {
+    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)) {
+          Smbios30Table = (SMBIOS_TABLE_3_0_ENTRY_POINT *) (UINTN) SmBiosTableAdress->SmBiosEntryPoint;
+          if (IsValidSmbios30Table (Smbios30Table)) {
+            Smbios.Raw = (UINT8 *) (UINTN) Smbios30Table->TableAddress;
+            Status = ParseAndAddExistingSmbiosTable (ImageHandle, Smbios, Smbios30Table->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;
+            }
+          }
+        }
+      } else {
+        Status = EFI_UNSUPPORTED;
+      }
+    }
+  }
+
+  //
+  // Scan for existing SMBIOS tables from gPldSmbiosTableGuid Guid Hob,
+  // if gPldSmbios3TableGuid Hob doesn't exist or parsing gPldSmbios3TableGuid failed
+  //
+  GuidHob = GetFirstGuidHob (&gPldSmbiosTableGuid);

+  if (GuidHob != NULL) {
+    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)) {
+          SmbiosTable = (SMBIOS_TABLE_ENTRY_POINT *) (UINTN) SmBiosTableAdress->SmBiosEntryPoint;
+          if (IsValidSmbios20Table (SmbiosTable)) {
+            Smbios.Raw = (UINT8 *) (UINTN) SmbiosTable->TableAddress;
+            Status = ParseAndAddExistingSmbiosTable (ImageHandle, Smbios, SmbiosTable->TableLength);
+            if (EFI_ERROR (Status)) {
+              DEBUG ((DEBUG_ERROR, "RetrieveSmbiosFromHob: Failed to parse preinstalled tables from gPldSmbiosTableGuid Guid Hob\n"));
+              Status = EFI_UNSUPPORTED;
+            }
+            return EFI_SUCCESS;
+          }
+        }
+      } else {
+        Status = EFI_UNSUPPORTED;
+      }
+    }
+  }
+  return Status;
+}
+
 /**

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

-  return Status;
+  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..3286575098 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
+  gPldSmbiosTableGuid

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



------------
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75489): https://edk2.groups.io/g/devel/message/75489
Mute This Topic: https://groups.io/mt/83045509/2917327
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [patrick.rudolph@...]
------------