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.rudo...@9elements.com>
> Sent: Tuesday, June 8, 2021 5:21 PM
> To: Liu, Zhiguang <zhiguang....@intel.com>
> Cc: devel@edk2.groups.io; Wang, Jian J <jian.j.w...@intel.com>; Wu, Hao A
> <hao.a...@intel.com>; Bi, Dandan <dandan...@intel.com>; Zeng, Star
> <star.z...@intel.com>; Gao, Zhichao <zhichao....@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....@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.w...@intel.com>
> > Cc: Hao A Wu <hao.a...@intel.com>
> > Cc: Dandan Bi <dandan...@intel.com>
> > Cc: Star Zeng <star.z...@intel.com>
> > Cc: Zhichao Gao <zhichao....@intel.com>
> > Signed-off-by: Patrick Rudolph <patrick.rudo...@9elements.com>
> > Signed-off-by: Zhiguang Liu <zhiguang....@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
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76312): https://edk2.groups.io/g/devel/message/76312
Mute This Topic: https://groups.io/mt/83304947/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
