From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mx.groups.io with SMTP id smtpd.web08.3792.1622009754344165618 for ; Tue, 25 May 2021 23:15:55 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=DPOGP6YU; spf=pass (domain: intel.com, ip: 134.134.136.31, mailfrom: hao.a.wu@intel.com) IronPort-SDR: Ysieznz/yqyaJQh+GgtejxOIVEahmC97u/04RxIYD5xZFF6Oxf2J9KpRxdtG1NmZc0otYcpX7f 37OP3TQ69FdQ== X-IronPort-AV: E=McAfee;i="6200,9189,9995"; a="263594481" X-IronPort-AV: E=Sophos;i="5.82,330,1613462400"; d="scan'208";a="263594481" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 May 2021 23:15:43 -0700 IronPort-SDR: bTdTUyjk/XhDtkCMBFjTSxEZXmkVI8A1q3JfsjG2aETOlk+Pnx5++9ANhKxNZBzE+6uW5FjG+u 409MBBklHyUw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.82,330,1613462400"; d="scan'208";a="547072121" Received: from orsmsx602.amr.corp.intel.com ([10.22.229.15]) by fmsmga001.fm.intel.com with ESMTP; 25 May 2021 23:15:43 -0700 Received: from orsmsx612.amr.corp.intel.com (10.22.229.25) by ORSMSX602.amr.corp.intel.com (10.22.229.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.4; Tue, 25 May 2021 23:15:42 -0700 Received: from orsmsx602.amr.corp.intel.com (10.22.229.15) by ORSMSX612.amr.corp.intel.com (10.22.229.25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.4; Tue, 25 May 2021 23:15:42 -0700 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) by orsmsx602.amr.corp.intel.com (10.22.229.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.4 via Frontend Transport; Tue, 25 May 2021 23:15:42 -0700 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (104.47.58.177) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2242.4; Tue, 25 May 2021 23:15:29 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LZUsbZWdkiK1uW1se6GMkKOvZCYJ0ZoQOTIl+5UQ9kaWJ2/B/3A7OQMjrcnD0Uond2mWlh2PnbAWwBr4ospsLMQE4ZiT73royAuy8QAkMpXO6YUZjg5ThdyVCb2iPFu+yEYw3IOLXxx45CZuMKyhIFjQptxqz7e9FGcogIaeFpuNw6FlZOS/WmN4b4g8bvtWigQcushNLcdZ5jj797P+Sm1EbrxIcZ3hOH2cCucPuYXeYGRnD7bd7WJGNKu2HTyfG1BXegqDDdCLOhe2snzMGKu2J6sItzobNal3AaO9DbmVrXi2Nbr/MeeZIjfzJk5j2ZCm28wK1Lho+qW6ev1qwg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=05JbHw534k0p9xm2dHD1A33Z+mjV4wyHo/3rWU4FPbg=; b=lwdAadCkY1z6maJbhcWnysjWr3s73rj6fxQJTbmcvBjf24Y/sD7v+kVBezhtmQI1OpXuLUbB52E7RO+Bf94Aje91LtWS4SdzE/bRHk0IQ+q+k8vAwlhqRosAKZq91ZEXnLuNmgXThzchoyNI4JsMZtkhNm8ZfNeqRya9lss04WVw/BCVbN2f7iJgnl/AxQzQ/tBkCWSOnT6kFqADfOe8kEiN0B3HpEEIw1cBG+Agzwbb01h4WUpfULD5TCXc4ZndSGMCJbNYccFIZdziKvolTB/mmGKmiwogXgV2Px0uuQ+ssMBAOMeQSW/CLib8FZfpPJ5ovayVbjFG/EU6sWYoHA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=05JbHw534k0p9xm2dHD1A33Z+mjV4wyHo/3rWU4FPbg=; b=DPOGP6YUiysCwJx56+FcEqalOeL0qIxfXmNGqfp+vVqIQ1yDYJH0mCtCWu5tqCdoiDR6LjaHdbhWknAddhdqhtMkl84/Oxq7MrrINKfFCypUbqmD7wBHaXtQ2wYoZL29Ai0nVJMXb4trFRaoMi6wy89LHT7pd0fcvDUnavKoXLU= Received: from BN8PR11MB3666.namprd11.prod.outlook.com (2603:10b6:408:8c::19) by BN6PR11MB4036.namprd11.prod.outlook.com (2603:10b6:405:82::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4150.27; Wed, 26 May 2021 06:15:28 +0000 Received: from BN8PR11MB3666.namprd11.prod.outlook.com ([fe80::b9d0:5694:1b70:c031]) by BN8PR11MB3666.namprd11.prod.outlook.com ([fe80::b9d0:5694:1b70:c031%4]) with mapi id 15.20.4150.027; Wed, 26 May 2021 06:15:28 +0000 From: "Wu, Hao A" To: "Liu, Zhiguang" , "Ni, Ray" , "devel@edk2.groups.io" CC: "Wang, Jian J" , "Bi, Dandan" , "Zeng, Star" , "Gao, Zhichao" , Patrick Rudolph Subject: Re: [PATCH 5/9] MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables Thread-Topic: [PATCH 5/9] MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables Thread-Index: AQHXUGxDy3aqNf5/2U+BRmKROR98DKr1PSPQ Date: Wed, 26 May 2021 06:15:28 +0000 Message-ID: References: <20210524071234.1056-1-zhiguang.liu@intel.com> <20210524071234.1056-6-zhiguang.liu@intel.com> In-Reply-To: <20210524071234.1056-6-zhiguang.liu@intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.5.1.3 authentication-results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [192.198.147.208] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 17fb68af-a060-4475-f082-08d9200da882 x-ms-traffictypediagnostic: BN6PR11MB4036: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:6790; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: XJcwwv0r2s8KmWXKXXSS+sN7GkFJKTHfaq+v+FRyt4RhmyFvi87E4+CrAHK2vsjbCDVIFNPWcsbUg2Tjgme63bd6gcCgCLyiECAaBm8FJoVApPrh/p36iPC1r8nEw25pOgEeW+E5SRnhu6CIVkmPAEOVaduUaGaPND0BbQfsN+UpxBk89SFesHL7WrkGKCJkTFQcdpPjT2VV6AKPT7ySnKx2+so6ag2za++Z1zHHYKw+ZJQDhfIQp80igW1H2lK1P8JFVsrlBeLei6qATIjTEZF45FR4TYfO6OMVwTtaGo1m5v6YCDI11PEwfbMtQtkdTMdqGyul/ZPh8wdWSrhDVUDQx3sFQH6+nO1qg2s2VKhAhdP3Gvl8akToJ8+VkNdTNHouwDJBvsMmilfWFq6x1ht/TIEpZ4qGzrKuW/swa1JvgpLle4k5v8LMF1FC2NhUtR+GVxmjw2hqcOz8Z2K+oJktIUQ6K0AE09QEeoJKXkHOCmXvVvv6v4+XReEuUMSyWwRuiYVWRT7YUjfF2o4WZfqRY8vQGPyDLWKOQq6n/zRHEbBfupg/LYKVbqZ8uCawEFZPVDIjFnOkhGw9XtGiulV1ShEF4pC7BbszpsaSxQs= x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BN8PR11MB3666.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(376002)(346002)(396003)(39860400002)(366004)(136003)(83380400001)(7696005)(66446008)(64756008)(53546011)(66556008)(6506007)(110136005)(9686003)(66946007)(52536014)(186003)(55016002)(8676002)(4326008)(19627235002)(66476007)(76116006)(478600001)(2906002)(8936002)(30864003)(26005)(5660300002)(122000001)(38100700002)(71200400001)(33656002)(86362001)(54906003)(316002);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?ATIWX9B+TFtUEHYnfvznWuGcXRJfEcz6paU0JMy1khcp91Hyi3Ye+ifzlPZE?= =?us-ascii?Q?OA8OthTUNXdIoHMlZk9jkiYY3GfOcrgLLag9l62o8pA0BtFsiMsrxyHR/i7j?= =?us-ascii?Q?IRJv/tS6rs/u/c4KNYsYAlFtmqWbQLEHlrpnkayYiVQJvla49ds/ikbsp1U8?= =?us-ascii?Q?W976jNL4yCVUVMQI9EaAyfscwBZympMuKgDMHRPyub+cKhtRVg9VXHnnwLq4?= =?us-ascii?Q?RfAX4yh45MR8IwEadmEz2BKnMKzs+CU47hjU48ONXXgdQfmJfE5W2mxOfdyX?= =?us-ascii?Q?4GOIJO2W0ECOkWkryilaiAgr5V76hiTm/H5iqa+EBXDbkAC4FMuTTDqqWIBn?= =?us-ascii?Q?v94RoPwsMMztPK/mjksT7ynj0hf6sUtTHfTo8mpgJvdq5Bj9wVxEPYASnvcB?= =?us-ascii?Q?lGI11peqlYtTbqGfMlVWMhYI6hB29JAOoQTRAWEg5TZ8ai+0K5ra8EYxhJxj?= =?us-ascii?Q?6Dql9UgGQGYqoJi5TdvlekvaDWw5uD+JHywN6BK9XbbV4dYb1+fZXmVvrORY?= =?us-ascii?Q?hpO7mrGBAPAJ7UB19vP4Y0o5W/eA+t12siTnE9Ih+cjepAM8DLkpAyXRqB5w?= =?us-ascii?Q?rnL3Hd3c5RIjgK8n38sRSfNjXpKlz/uYe/2EYtZBFeC/7Gv70cBWTpSpnOf1?= =?us-ascii?Q?EnOADVjPDJgNthMzSsYtqfE0cIXA+MgnNsBDCtxz4jnyQuSoIX30tEQDpjpC?= =?us-ascii?Q?EBqIAulVsUWlG9dsqbBqmWF6Xis0g+0/0prhQgfzfX6Ozbh2TTvbPJLcn/XN?= =?us-ascii?Q?avALr42iCBli+DnocQ8gzOOT0wqRmbp55hJhkwxQl2gdItq6rj5jUd72141o?= =?us-ascii?Q?NOWvOBJEPLZogzPuf+3/w70y2buQRDHwHd6uE0CJlm7/fuC2I4ul1Dpbx2Gw?= =?us-ascii?Q?NljTewXGvxepOSpSfgitHJLdhtMm9DPpehG9DOiYmYTad9Q3Ssem9327hOK4?= =?us-ascii?Q?KmmolG4uEukMWnI96sCrr+nVLqkoUmwaLG0lYnC/fYYyi2ZDGTVif2oZSmwN?= =?us-ascii?Q?SD7jOSodKkiBHK4p3NYZUkawBCnBbvl9WnKIjAJq942uvSyI5ESmchJ0m8+1?= =?us-ascii?Q?+HMmO/KlE04iJ14oQfzAtooQc3yanqLEC+59gM2WOeVF5E5RWhFkGRZvfv5z?= =?us-ascii?Q?/JalrLKsw313Lhe5bn5ItYJgBMGf1eYH7l3Rb6iItL93xdzABJldM4fXN/W3?= =?us-ascii?Q?DWyfOViRWn46DY7CzBMf3fZVEupoofZvPuXOQBMFdfUXANBwJCA0OMZYDwYK?= =?us-ascii?Q?O4Bjc1Y9EE2R5cmCmdNK4TxsfT4Ds9ecQsp8b0H0iOP1dMECrLSol9nsasjo?= =?us-ascii?Q?jz8eY+2/XUqFtbqq6/xZA/Tt?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: BN8PR11MB3666.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 17fb68af-a060-4475-f082-08d9200da882 X-MS-Exchange-CrossTenant-originalarrivaltime: 26 May 2021 06:15:28.1654 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: czNO6lLoiliuPHqMtav19sFtsTBkG/VJyPFLxKV2m8pRozJ6/X8psuhPcQeyzXX0z/iDLppsbgr9Pynptva2vA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR11MB4036 Return-Path: hao.a.wu@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Adding Ray. Hello Ray, I saw most of your comments in previous patch: "[PATCH - resend] MdeModulePkg/Universal/SmbiosDxe: Scan for existing table= s" have been addressed in this patch series. Could you help to check if there are additional comments for this patch? Th= anks. Some inline comments below: > -----Original Message----- > From: Liu, Zhiguang > Sent: Monday, May 24, 2021 3:13 PM > To: devel@edk2.groups.io > Cc: Wang, Jian J ; Wu, Hao A ; > Bi, Dandan ; Zeng, Star ; Gao, > Zhichao ; Patrick Rudolph > > Subject: [PATCH 5/9] MdeModulePkg/Universal/SmbiosDxe: Scan for > existing tables >=20 > 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. >=20 > This fixes the settings menu not showing any hardware information, instea= d > only "0 MB RAM" was displayed. >=20 > Tests showed that the OS can still see the SMBIOS tables. >=20 > V2: > SmbiosDxe will get the SMBIOS from a guid Hob. > Aslo will keep the SmbiosHandle if it is available. >=20 > 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 | 299 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++-- > MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.h | 4 +++- > MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf | 5 ++++- > 3 files changed, 304 insertions(+), 4 deletions(-) >=20 > 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 constr= ucting >=20 > SMBIOS table into system table. >=20 >=20 >=20 > -Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
>=20 > +Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.
>=20 > SPDX-License-Identifier: BSD-2-Clause-Patent >=20 >=20 >=20 > **/ >=20 > @@ -1408,6 +1408,300 @@ SmbiosTableConstruction ( > } >=20 > } >=20 >=20 >=20 > +/** >=20 > + Validates a SMBIOS 2.0 table entry point. >=20 > + >=20 > + @param SmbiosTable The SMBIOS_TABLE_ENTRY_POINT to validate. >=20 > + >=20 > + @retval TRUE SMBIOS table entry point is valid. >=20 > + @retval FALSE SMBIOS table entry point is malformed. >=20 > + >=20 > +**/ >=20 > +STATIC >=20 > +BOOLEAN >=20 > +IsValidSmbios20Table ( >=20 > + IN SMBIOS_TABLE_ENTRY_POINT *SmbiosTable >=20 > + ) >=20 > +{ >=20 > + UINT8 Checksum; >=20 > + >=20 > + if (CompareMem (SmbiosTable->AnchorString, "_SM_", 4) !=3D 0) { >=20 > + return FALSE; >=20 > + } Should a check for the 'IntermediateAnchorString' be added here as well? >=20 > + >=20 > + // >=20 > + // The actual value of the EntryPointLength should be 1Fh. >=20 > + // However, it was incorrectly stated in version 2.1 of smbios specifi= cation. >=20 > + // Therefore, 0x1F and 0x1E are both accepted. >=20 > + // >=20 > + if (SmbiosTable->EntryPointLength !=3D 0x1E || > + SmbiosTable->EntryPointLength !=3D sizeof (SMBIOS_TABLE_ENTRY_POINT)) > { >=20 > + return FALSE; >=20 > + } >=20 > + >=20 > + // >=20 > + // MajorVersion should be 2. >=20 > + // >=20 > + if (SmbiosTable->MajorVersion !=3D 2) { >=20 > + 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 fie= ld 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. >=20 > + } >=20 > + >=20 > + // >=20 > + // The whole struct check sum should be zero >=20 > + // >=20 > + Checksum =3D CalculateSum8 ( >=20 > + (UINT8 *) SmbiosTable, >=20 > + SmbiosTable->EntryPointLength >=20 > + ); >=20 > + if (Checksum !=3D 0) { >=20 > + return FALSE; >=20 > + } >=20 > + >=20 > + // >=20 > + // The Intermediate Entry Point Structure check sum should be zero. >=20 > + // >=20 > + Checksum =3D CalculateSum8 ( >=20 > + (UINT8 *) SmbiosTable + OFFSET_OF > + (SMBIOS_TABLE_ENTRY_POINT, IntermediateAnchorString), >=20 > + SmbiosTable->EntryPointLength - OFFSET_OF > + (SMBIOS_TABLE_ENTRY_POINT, IntermediateAnchorString) >=20 > + ); >=20 > + return (BOOLEAN) (Checksum =3D=3D 0); >=20 > +} >=20 > + >=20 > +/** >=20 > + Validates a SMBIOS 3.0 table entry point. >=20 > + >=20 > + @param SmbiosTable The SMBIOS_TABLE_3_0_ENTRY_POINT to validate. >=20 > + >=20 > + @retval TRUE SMBIOS table entry point is valid. >=20 > + @retval FALSE SMBIOS table entry point is malformed. >=20 > + >=20 > +**/ >=20 > +STATIC >=20 > +BOOLEAN >=20 > +IsValidSmbios30Table ( >=20 > + IN SMBIOS_TABLE_3_0_ENTRY_POINT *SmbiosTable >=20 > + ) >=20 > +{ >=20 > + UINT8 Checksum; >=20 > + >=20 > + if (CompareMem (SmbiosTable->AnchorString, "_SM3_", 5) !=3D 0) { >=20 > + return FALSE; >=20 > + } >=20 > + if (SmbiosTable->EntryPointLength < sizeof > + (SMBIOS_TABLE_3_0_ENTRY_POINT)) { >=20 > + return FALSE; >=20 > + } >=20 > + if (SmbiosTable->MajorVersion < 3) { >=20 > + return FALSE; >=20 > + } >=20 > + >=20 > + // >=20 > + // The whole struct check sum should be zero >=20 > + // >=20 > + Checksum =3D CalculateSum8 ( >=20 > + (UINT8 *) SmbiosTable, >=20 > + SmbiosTable->EntryPointLength >=20 > + ); >=20 > + if (Checksum !=3D 0) { >=20 > + return FALSE; >=20 > + } >=20 > + return TRUE; >=20 > +} >=20 > + >=20 > +/** >=20 > + Parse an existing SMBIOS table and insert it using SmbiosAdd. >=20 > + >=20 > + @param ImageHandle The EFI_HANDLE to this driver. >=20 > + @param Smbios The SMBIOS table to parse. >=20 > + @param Length The length of the SMBIOS table. >=20 > + >=20 > + @retval EFI_SUCCESS SMBIOS table was parsed and installed. >=20 > + @retval EFI_OUT_OF_RESOURCES Record was not added due to lack of > system resources. >=20 > + @retval EFI_INVALID_PARAMETER Smbios is not a correct smbios table >=20 > + >=20 > +**/ >=20 > +STATIC >=20 > +EFI_STATUS >=20 > +ParseAndAddExistingSmbiosTable ( >=20 > + IN EFI_HANDLE ImageHandle, >=20 > + IN SMBIOS_STRUCTURE_POINTER Smbios, >=20 > + IN UINTN Length >=20 > + ) >=20 > +{ >=20 > + EFI_STATUS Status; >=20 > + CHAR8 *String; >=20 > + EFI_SMBIOS_HANDLE SmbiosHandle; >=20 > + SMBIOS_STRUCTURE_POINTER SmbiosEnd; >=20 > + >=20 > + SmbiosEnd.Raw =3D Smbios.Raw + Length; >=20 > + >=20 > + if (Smbios.Raw >=3D SmbiosEnd.Raw || Smbios.Raw =3D=3D NULL) { >=20 > + return EFI_INVALID_PARAMETER; >=20 > + } >=20 > + >=20 > + do { >=20 > + // >=20 > + // Make sure not to access memory beyond SmbiosEnd >=20 > + // >=20 > + if (Smbios.Raw + sizeof (SMBIOS_STRUCTURE) > SmbiosEnd.Raw || >=20 > + Smbios.Raw + sizeof (SMBIOS_STRUCTURE) < Smbios.Raw) { >=20 > + return EFI_INVALID_PARAMETER; >=20 > + } >=20 > + // >=20 > + // Check for end marker >=20 > + // >=20 > + if (Smbios.Hdr->Type =3D=3D SMBIOS_TYPE_END_OF_TABLE) { >=20 > + break; >=20 > + } >=20 > + // >=20 > + // Make sure not to access memory beyond SmbiosEnd >=20 > + // Each structure shall be terminated by a double-null (0000h). >=20 > + // >=20 > + if (Smbios.Raw + Smbios.Hdr->Length + 2 * sizeof (UINT8) > > + SmbiosEnd.Raw || >=20 > + Smbios.Raw + Smbios.Hdr->Length + 2 * sizeof (UINT8) < > + Smbios.Raw) { >=20 > + return EFI_INVALID_PARAMETER; >=20 > + } >=20 > + // >=20 > + // Install the table >=20 > + // >=20 > + SmbiosHandle =3D Smbios.Hdr->Handle; >=20 > + Status =3D SmbiosAdd ( >=20 > + &mPrivateData.Smbios, >=20 > + ImageHandle, >=20 > + &SmbiosHandle, >=20 > + Smbios.Hdr >=20 > + ); >=20 > + >=20 > + ASSERT_EFI_ERROR (Status); >=20 > + if (EFI_ERROR (Status)) { >=20 > + return Status; >=20 > + } >=20 > + // >=20 > + // Go to the next SMBIOS structure. Each SMBIOS structure may includ= e 2 > parts: >=20 > + // 1. Formatted section; 2. Unformatted string section. So, 2 steps > + are needed >=20 > + // to skip one SMBIOS structure. >=20 > + // >=20 > + >=20 > + // >=20 > + // Step 1: Skip over formatted section. >=20 > + // >=20 > + String =3D (CHAR8 *) (Smbios.Raw + Smbios.Hdr->Length); >=20 > + >=20 > + // >=20 > + // Step 2: Skip over unformatted string section. >=20 > + // >=20 > + do { >=20 > + // >=20 > + // Each string is terminated with a NULL(00h) BYTE and the sets > + of strings >=20 > + // is terminated with an additional NULL(00h) BYTE. >=20 > + // >=20 > + for ( ; *String !=3D 0; String++) { >=20 > + if ((UINTN) String >=3D (UINTN) SmbiosEnd.Raw - sizeof (UINT8)) = { >=20 > + return EFI_INVALID_PARAMETER; >=20 > + } >=20 > + } >=20 > + >=20 > + if (*(UINT8 *) ++String =3D=3D 0) { >=20 > + // >=20 > + // Pointer to the next SMBIOS structure. >=20 > + // >=20 > + Smbios.Raw =3D (UINT8 *) ++String; >=20 > + break; >=20 > + } >=20 > + } while (TRUE); >=20 > + } while (Smbios.Raw < SmbiosEnd.Raw); >=20 > + >=20 > + return EFI_SUCCESS; >=20 > +} >=20 > + >=20 > + >=20 > +/** >=20 > + Retrieve SMBIOS from Hob. >=20 > + @param ImageHandle Module's image handle >=20 > + >=20 > + @retval EFI_SUCCESS Smbios from Hob is installed. >=20 > + @return EFI_NOT_FOUND Not found Smbios from Hob. >=20 > + @retval Other No Smbios from Hob is installed. >=20 > + >=20 > +**/ >=20 > +EFI_STATUS >=20 > +EFIAPI I think RetrieveSmbiosFromHob() is an internal function. Please help to remove the 'EFIAPI' keyword here. >=20 > +RetrieveSmbiosFromHob ( >=20 > + IN EFI_HANDLE ImageHandle >=20 > + ) >=20 > +{ >=20 > + EFI_STATUS Status; >=20 > + SMBIOS_TABLE_ENTRY_POINT *SmbiosTable; >=20 > + SMBIOS_TABLE_3_0_ENTRY_POINT *Smbios30Table; >=20 > + SMBIOS_STRUCTURE_POINTER Smbios; >=20 > + EFI_HOB_GUID_TYPE *GuidHob; >=20 > + PLD_SMBIOS_TABLE *SmBiosTableAdress; >=20 > + PLD_GENERIC_HEADER *GenericHeader; >=20 > + >=20 > + Status =3D EFI_NOT_FOUND; >=20 > + // >=20 > + // Scan for existing SMBIOS tables from gPldSmbios3TableGuid Guid Hob >=20 > + // >=20 > + GuidHob =3D GetFirstGuidHob (&gPldSmbios3TableGuid); >=20 > + if (GuidHob !=3D NULL) { >=20 > + GenericHeader =3D (PLD_GENERIC_HEADER *) GET_GUID_HOB_DATA > (GuidHob); >=20 > + if ((sizeof (PLD_GENERIC_HEADER) <=3D GET_GUID_HOB_DATA_SIZE > + (GuidHob)) && (GenericHeader->Length <=3D GET_GUID_HOB_DATA_SIZE > + (GuidHob))) { >=20 > + if (GenericHeader->Revision =3D=3D PLD_SMBIOS_TABLE_REVISION) { >=20 > + // >=20 > + // PLD_SMBIOS_TABLE structure is used when Revision equals to > + PLD_SMBIOS_TABLE_REVISION >=20 > + // >=20 > + SmBiosTableAdress =3D (PLD_SMBIOS_TABLE *) GET_GUID_HOB_DATA > + (GuidHob); >=20 > + if (GenericHeader->Length >=3D PLD_SIZEOF_THROUGH_FIELD > + (PLD_SMBIOS_TABLE, SmBiosEntryPoint)) { >=20 > + Smbios30Table =3D (SMBIOS_TABLE_3_0_ENTRY_POINT *) (UINTN) > + SmBiosTableAdress->SmBiosEntryPoint; >=20 > + if (IsValidSmbios30Table (Smbios30Table)) { >=20 > + Smbios.Raw =3D (UINT8 *) (UINTN) Smbios30Table->TableAddress= ; >=20 > + Status =3D ParseAndAddExistingSmbiosTable (ImageHandle, > + Smbios, Smbios30Table->TableMaximumSize); >=20 > + if (EFI_ERROR (Status)) { >=20 > + DEBUG ((DEBUG_ERROR, "RetrieveSmbiosFromHob: Failed to > + parse preinstalled tables from gPldSmbios3TableGuid Guid Hob\n")); >=20 > + Status =3D EFI_UNSUPPORTED; >=20 > + } else { >=20 > + return EFI_SUCCESS; >=20 > + } >=20 > + } >=20 > + } >=20 > + } else { >=20 > + Status =3D EFI_UNSUPPORTED; >=20 > + } >=20 > + } >=20 > + } >=20 > + >=20 > + // >=20 > + // Scan for existing SMBIOS tables from gPldSmbiosTableGuid Guid Hob, >=20 > + // if gPldSmbios3TableGuid Hob doesn't exist or parsing > + gPldSmbios3TableGuid failed >=20 > + // >=20 > + GuidHob =3D GetFirstGuidHob (&gPldSmbiosTableGuid); >=20 > + >=20 > + if (GuidHob !=3D NULL) { >=20 > + GenericHeader =3D (PLD_GENERIC_HEADER *) GET_GUID_HOB_DATA > (GuidHob); >=20 > + if ((sizeof (PLD_GENERIC_HEADER) <=3D GET_GUID_HOB_DATA_SIZE > + (GuidHob)) && (GenericHeader->Length <=3D GET_GUID_HOB_DATA_SIZE > + (GuidHob))) { >=20 > + if (GenericHeader->Revision =3D=3D PLD_SMBIOS_TABLE_REVISION) { >=20 > + // >=20 > + // PLD_SMBIOS_TABLE structure is used when Revision equals to > + PLD_SMBIOS_TABLE_REVISION >=20 > + // >=20 > + SmBiosTableAdress =3D (PLD_SMBIOS_TABLE *) GET_GUID_HOB_DATA > + (GuidHob); >=20 > + if (GenericHeader->Length >=3D PLD_SIZEOF_THROUGH_FIELD > + (PLD_SMBIOS_TABLE, SmBiosEntryPoint)) { >=20 > + SmbiosTable =3D (SMBIOS_TABLE_ENTRY_POINT *) (UINTN) > + SmBiosTableAdress->SmBiosEntryPoint; >=20 > + if (IsValidSmbios20Table (SmbiosTable)) { >=20 > + Smbios.Raw =3D (UINT8 *) (UINTN) SmbiosTable->TableAddress; >=20 > + Status =3D ParseAndAddExistingSmbiosTable (ImageHandle, > + Smbios, SmbiosTable->TableLength); >=20 > + if (EFI_ERROR (Status)) { >=20 > + DEBUG ((DEBUG_ERROR, "RetrieveSmbiosFromHob: Failed to > + parse preinstalled tables from gPldSmbiosTableGuid Guid Hob\n")); >=20 > + Status =3D EFI_UNSUPPORTED; >=20 > + } >=20 > + return EFI_SUCCESS; >=20 > + } >=20 > + } >=20 > + } else { >=20 > + Status =3D EFI_UNSUPPORTED; >=20 > + } >=20 > + } >=20 > + } 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 >=20 > + return Status; >=20 > +} >=20 > + >=20 > /** >=20 >=20 >=20 > Driver to produce Smbios protocol and pre-allocate 1 page for the fina= l > SMBIOS table. >=20 > @@ -1451,5 +1745,6 @@ SmbiosDriverEntryPoint ( > &mPrivateData.Smbios >=20 > ); >=20 >=20 >=20 > - return Status; >=20 > + RetrieveSmbiosFromHob (ImageHandle); >=20 > + return EFI_SUCCESS; >=20 > } >=20 > 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 >=20 > This code supports the implementation of the Smbios protocol >=20 >=20 >=20 > -Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
>=20 > +Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.
>=20 > SPDX-License-Identifier: BSD-2-Clause-Patent >=20 >=20 >=20 > **/ >=20 > @@ -24,6 +24,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include > >=20 > #include >=20 > #include >=20 > +#include >=20 > +#include >=20 >=20 >=20 > #define SMBIOS_INSTANCE_SIGNATURE SIGNATURE_32 ('S', 'B', 'i', 's') >=20 > typedef struct { >=20 > 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 >=20 > # This driver initializes and installs the SMBIOS protocol, constructs S= MBIOS > table into system configuration table. >=20 > # >=20 > -# Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
>=20 > +# Copyright (c) 2009 - 2021, Intel Corporation. All rights > +reserved.
>=20 > # >=20 > # SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > # >=20 > @@ -41,6 +41,7 @@ > UefiDriverEntryPoint >=20 > DebugLib >=20 > PcdLib >=20 > + HobLib >=20 >=20 >=20 > [Protocols] >=20 > gEfiSmbiosProtocolGuid ## PRODUCES >=20 > @@ -48,6 +49,8 @@ > [Guids] >=20 > gEfiSmbiosTableGuid ## SOMETIMES_PRODUCE= S ## > SystemTable >=20 > gEfiSmbios3TableGuid ## SOMETIMES_PRODUCE= S ## > SystemTable >=20 > + gPldSmbios3TableGuid >=20 > + gPldSmbiosTableGuid >=20 >=20 >=20 > [Pcd] >=20 > gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion ## CONSUMES >=20 > -- > 2.30.0.windows.2