From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com [209.85.214.174]) by mx.groups.io with SMTP id smtpd.web12.9963.1623144071281589794 for ; Tue, 08 Jun 2021 02:21:11 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@9elements.com header.s=google header.b=TBDrx6c5; spf=pass (domain: 9elements.com, ip: 209.85.214.174, mailfrom: patrick.rudolph@9elements.com) Received: by mail-pl1-f174.google.com with SMTP id u7so10311298plq.4 for ; Tue, 08 Jun 2021 02:21:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=9elements.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=HY+zwl3N6ti4VM6NXTMI5Wr3X7W44oabyVWSSQTcKBA=; b=TBDrx6c5oQRyVS7rmtFWYmnl3mdaJZ5hudolXkOSO/mvAdWhTgKa2qE2C5udWxTy2T uqTOcMYwd7bqbnO0Zg5GOioArADpQvjLxZtOtUW3os5SREj9uOS/jD7o9mRdrB68zyhO iKIf90OsOMvrMDLbb5TIvLtAgGSbNlJxt7Om7q99dJ8Ul7Hk1wH5ReLOJvQV7WXtCEOy 1/dfqRHo+TkW+zdv4r2kIrUGlw5GEBsgPoFblpLVqBg4WIGyBsxuZ6q1T5scI+LVJxM5 5d9qJfjJRiyVkm3LO37Woa+qbnXDL4V6e3o1x5X6AIjnRjq7cgfCkqJi6I6jn+7hsjV6 mOpw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=HY+zwl3N6ti4VM6NXTMI5Wr3X7W44oabyVWSSQTcKBA=; b=nHRhezIDcbzocFDtwqlHlEhsTGOYNoPtMuOtk7baDXUiTDbBJxoypSgw71ceMmhTtn n7L+e2Vw5EgtF6PyKk6ezg9XtwwUTAr8n+YT1qDO6+0UikIgQOMJFH+JydOb7oqdpuNK 7gnIo5B78hyznubJCMXLgRFz+wHC+Oj88VgyQduldfaXDvaNZivW98nKgfb5dT006COp UFSMyeAU2hp9+4DtBUtyMzWaElqqiUdwTY77PrAFx50JoUrm8b7CzOQXuQTWgTLUWVF3 UgNDzno6dZVyK7ZVpBVE3hzlCPj0u8cNJ5+TZD9uLnPKBUh1nHkxi6iCLlTL8I6MYDBl sE/A== X-Gm-Message-State: AOAM5321BLnix/sBk0+JvcQAI7D7vxibT3T63ZgP6J5MELZhKs45Xg7/ RGKc+30yZ1Zhd9/fClCXXvOoKpL8uTM42DFXwS3FiA== X-Google-Smtp-Source: ABdhPJyguBYySKHLagRxzuYWpnc3fl0pfdbXwQ7qULIHVSKvYdxkm6SsNg16rUTuUiCYb7gm2qQhAamXnJbDVdoeBo0= X-Received: by 2002:a17:902:ee94:b029:10e:e2c2:b11d with SMTP id a20-20020a170902ee94b029010ee2c2b11dmr22435098pld.31.1623144070720; Tue, 08 Jun 2021 02:21:10 -0700 (PDT) MIME-Version: 1.0 References: <20210604094227.1890-1-zhiguang.liu@intel.com> <20210604094227.1890-6-zhiguang.liu@intel.com> In-Reply-To: <20210604094227.1890-6-zhiguang.liu@intel.com> From: "Patrick Rudolph" Date: Tue, 8 Jun 2021 11:20:59 +0200 Message-ID: Subject: Re: [Patch V3 5/9] MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables To: Zhiguang Liu Cc: devel@edk2.groups.io, Jian J Wang , Hao A Wu , Dandan Bi , Star Zeng , Zhichao Gao Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, Jun 4, 2021 at 11:42 AM 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, instea= d > 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 > Cc: Hao A Wu > Cc: Dandan Bi > Cc: Star Zeng > Cc: Zhichao Gao > Signed-off-by: Patrick Rudolph > Signed-off-by: Zhiguang Liu > --- > 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 constr= ucting > SMBIOS table into system table. > > -Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -148,6 +148,31 @@ SMBIOS_TABLE_3_0_ENTRY_POINT Smbios30EntryPointStruc= tureData =3D { > // > 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 tab= le. > + > + @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 tab= le. > + > + @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 =3D (SMBIOS_TABLE_ENTRY_POINT *) TableEntry; > + > + if (CompareMem (SmbiosTable->AnchorString, "_SM_", 4) !=3D 0) { > + return FALSE; > + } > + > + if (CompareMem (SmbiosTable->IntermediateAnchorString, "_DMI_", 5) != =3D 0) { > + return FALSE; > + } > + > + // > + // The actual value of the EntryPointLength should be 1Fh. > + // However, it was incorrectly stated in version 2.1 of smbios specifi= cation. > + // Therefore, 0x1F and 0x1E are both accepted. > + // > + if (SmbiosTable->EntryPointLength !=3D 0x1E && SmbiosTable->EntryPoint= Length !=3D 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 =3D CalculateSum8 ( > + (UINT8 *) SmbiosTable, > + SmbiosTable->EntryPointLength > + ); > + if (Checksum !=3D 0) { > + return FALSE; > + } > + > + // > + // The Intermediate Entry Point Structure check sum should be zero. > + // > + Checksum =3D CalculateSum8 ( > + (UINT8 *) SmbiosTable + OFFSET_OF (SMBIOS_TABLE_ENTRY_POI= NT, IntermediateAnchorString), > + SmbiosTable->EntryPointLength - OFFSET_OF (SMBIOS_TABLE_E= NTRY_POINT, IntermediateAnchorString) > + ); > + if (Checksum !=3D 0) { > + return FALSE; > + } > + > + *TableAddress =3D (VOID *) (UINTN) SmbiosTable->TableAddress; > + *TableMaximumSize =3D 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 tab= le. > + > + @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 =3D (SMBIOS_TABLE_3_0_ENTRY_POINT *) TableEntry; > + > + if (CompareMem (SmbiosTable->AnchorString, "_SM3_", 5) !=3D 0) { > + return FALSE; > + } > + if (SmbiosTable->EntryPointLength < sizeof (SMBIOS_TABLE_3_0_ENTRY_POI= NT)) { > + return FALSE; > + } > + if (SmbiosTable->MajorVersion < 3) { > + return FALSE; > + } > + > + // > + // The whole struct check sum should be zero > + // > + Checksum =3D CalculateSum8 ( > + (UINT8 *) SmbiosTable, > + SmbiosTable->EntryPointLength > + ); > + if (Checksum !=3D 0) { > + return FALSE; > + } > + > + *TableAddress =3D (VOID *) (UINTN) SmbiosTable->TableAddress; > + *TableMaximumSize =3D 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 syst= em 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 =3D Smbios.Raw + Length; > + > + if (Smbios.Raw >=3D SmbiosEnd.Raw || Smbios.Raw =3D=3D 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 =3D=3D 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 =3D Smbios.Hdr->Handle; > + Status =3D 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 includ= e 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 =3D (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 !=3D 0; String++) { > + if ((UINTN) String >=3D (UINTN) SmbiosEnd.Raw - sizeof (UINT8)) = { > + return EFI_INVALID_PARAMETER; > + } > + } > + > + if (*(UINT8 *) ++String =3D=3D 0) { > + // > + // Pointer to the next SMBIOS structure. > + // > + Smbios.Raw =3D (UINT8 *) ++String; > + break; > + } > + } while (TRUE); > + } while (Smbios.Raw < SmbiosEnd.Raw); > + > + return EFI_SUCCESS; > +} > + > + > +IS_SMBIOS_TABLE_VALID_ENTRY mIsSmbiosTableValid[] =3D { > + {&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 =3D EFI_NOT_FOUND; > + > + for (Index =3D 0; Index < ARRAY_SIZE (mIsSmbiosTableValid); Index++) { > + GuidHob =3D GetFirstGuidHob (mIsSmbiosTableValid[Index].Guid); > + if (GuidHob =3D=3D NULL) { > + continue; > + } > + GenericHeader =3D (PLD_GENERIC_HEADER *) GET_GUID_HOB_DATA (GuidHob)= ; > + if ((sizeof (PLD_GENERIC_HEADER) <=3D GET_GUID_HOB_DATA_SIZE (GuidHo= b)) && (GenericHeader->Length <=3D GET_GUID_HOB_DATA_SIZE (GuidHob))) { > + if (GenericHeader->Revision =3D=3D PLD_SMBIOS_TABLE_REVISION) { > + // > + // PLD_SMBIOS_TABLE structure is used when Revision equals to PL= D_SMBIOS_TABLE_REVISION > + // > + SmBiosTableAdress =3D (PLD_SMBIOS_TABLE *) GET_GUID_HOB_DATA (Gu= idHob); > + if (GenericHeader->Length >=3D PLD_SIZEOF_THROUGH_FIELD (PLD_SMB= IOS_TABLE, SmBiosEntryPoint)) { > + if (mIsSmbiosTableValid[Index].IsValid ((VOID *) (UINTN )SmBio= sTableAdress->SmBiosEntryPoint, &TableAddress, &TableMaximumSize)) { > + Smbios.Raw =3D TableAddress; > + Status =3D ParseAndAddExistingSmbiosTable (ImageHandle, Smbi= os, TableMaximumSize); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "RetrieveSmbiosFromHob: Failed to par= se preinstalled tables from gPldSmbios3TableGuid Guid Hob\n")); > + Status =3D EFI_UNSUPPORTED; > + } else { > + return EFI_SUCCESS; > + } > + } > + } > + } > + } > + } > + return Status; > +} > + > /** > > Driver to produce Smbios protocol and pre-allocate 1 page for the fina= l 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.
> +Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -24,6 +24,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include > #include > #include > +#include > +#include > > #define SMBIOS_INSTANCE_SIGNATURE SIGNATURE_32 ('S', 'B', 'i', 's') > typedef struct { > diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf b/MdeModulePk= g/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 S= MBIOS table into system configuration table. > # > -# Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
> +# Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.
> # > # 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_PRODUCE= S ## SystemTable > gEfiSmbios3TableGuid ## SOMETIMES_PRODUCE= S ## SystemTable > + gPldSmbios3TableGuid ## CONSUMES = ## HOB > + gPldSmbiosTableGuid ## SOMETIMES_CONSUME= S ## HOB > > [Pcd] > gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion ## CONSUMES > -- > 2.30.0.windows.2 >