From: "Patrick Rudolph" <patrick.rudolph@9elements.com>
To: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: devel@edk2.groups.io, Jian J Wang <jian.j.wang@intel.com>,
Hao A Wu <hao.a.wu@intel.com>, Dandan Bi <dandan.bi@intel.com>,
Star Zeng <star.zeng@intel.com>,
Zhichao Gao <zhichao.gao@intel.com>
Subject: Re: [Patch V3 5/9] MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables
Date: Tue, 8 Jun 2021 11:20:59 +0200 [thread overview]
Message-ID: <CALNFmy3daAg5HHAfLkTdjg7K4CT-ji3_XkjqJp1Xvd2wmbBrNQ@mail.gmail.com> (raw)
In-Reply-To: <20210604094227.1890-6-zhiguang.liu@intel.com>
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
>
next prev parent reply other threads:[~2021-06-08 9:21 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-04 9:42 [Patch V3 0/9] Create multiple Hobs for Universal Payload Zhiguang Liu
2021-06-04 9:42 ` [Patch V3 1/9] MdeModulePkg: Add Universal Payload general defination header file Zhiguang Liu
2021-06-08 5:20 ` Wu, Hao A
2021-06-08 16:25 ` Michael D Kinney
2021-06-09 2:58 ` [edk2-devel] " Ni, Ray
2021-06-09 3:08 ` Ma, Maurice
2021-06-09 5:45 ` Zhiguang Liu
2021-06-04 9:42 ` [Patch V3 2/9] MdeModulePkg: Add new structure for the PCI Root Bridge Info Hob Zhiguang Liu
2021-06-08 5:20 ` Wu, Hao A
2021-06-04 9:42 ` [Patch V3 3/9] UefiPayloadPkg: UefiPayload retrieve PCI root bridge from Guid Hob Zhiguang Liu
2021-06-04 9:42 ` [Patch V3 4/9] MdeModulePkg: Add new structure for the Universal Payload SMBios Table Info Hob Zhiguang Liu
2021-06-08 5:20 ` Wu, Hao A
2021-06-04 9:42 ` [Patch V3 5/9] MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables Zhiguang Liu
2021-06-08 5:21 ` Wu, Hao A
2021-06-08 9:20 ` Patrick Rudolph [this message]
2021-06-10 1:56 ` Zhiguang Liu
2021-06-04 9:42 ` [Patch V3 6/9] UefiPayloadPkg: Creat gPldSmbiosTableGuid Hob Zhiguang Liu
2021-06-04 9:42 ` [Patch V3 7/9] MdeModulePkg: Add new structure for the Universal Payload ACPI Table Info Hob Zhiguang Liu
2021-06-08 5:21 ` Wu, Hao A
2021-06-04 9:42 ` [Patch V3 8/9] MdeModulePkg/ACPI: Install ACPI table from HOB Zhiguang Liu
2021-06-08 5:21 ` Wu, Hao A
2021-06-04 9:42 ` [Patch V3 9/9] UefiPayloadPkg: Creat gPldAcpiTableGuid Hob Zhiguang Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CALNFmy3daAg5HHAfLkTdjg7K4CT-ji3_XkjqJp1Xvd2wmbBrNQ@mail.gmail.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox