From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 93D6081C0D for ; Wed, 11 Jan 2017 22:08:49 -0800 (PST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga102.fm.intel.com with ESMTP; 11 Jan 2017 22:08:49 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,348,1477983600"; d="scan'208";a="52313821" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga005.fm.intel.com with ESMTP; 11 Jan 2017 22:08:49 -0800 Received: from FMSMSX109.amr.corp.intel.com (10.18.116.9) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 11 Jan 2017 22:08:49 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx109.amr.corp.intel.com (10.18.116.9) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 11 Jan 2017 22:08:49 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.88]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.132]) with mapi id 14.03.0248.002; Thu, 12 Jan 2017 14:08:46 +0800 From: "Zeng, Star" To: "Augustine, Linson P" , "edk2-devel@lists.01.org" CC: "Carsey, Jaben" , "Ni, Ruiyu" , "Zeng, Star" Thread-Topic: [edk2] [PATCH v2 1/1] ShellPkg/SmbiosView: Add decoding of SMBIOS record type 43 Thread-Index: AQHSa+jOnZVp0WW1k0KxsUv23FkjKaEyg6sAgAFQd4CAAIey0A== Date: Thu, 12 Jan 2017 06:08:45 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B81616F@shsmsx102.ccr.corp.intel.com> References: <20170111085703.8180-1-linson.augustine@hpe.com> <4a32ae19-ab57-9715-df71-831e677a7356@intel.com> In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v2 1/1] ShellPkg/SmbiosView: Add decoding of SMBIOS record type 43 X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Jan 2017 06:08:49 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Linson, " no comments to it" I said meant I have no comments to it, I just put a ma= rk at there as my following comments "How about use "Display TPM Device (Ty= pe 43) Characteristics" to align with the comment above?" will refer it. Sorry for confusing. Please update patch. Thanks, Star -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Augu= stine, Linson P Sent: Thursday, January 12, 2017 1:59 PM To: Zeng, Star ; edk2-devel@lists.01.org Cc: Carsey, Jaben ; Ni, Ruiyu Subject: Re: [edk2] [PATCH v2 1/1] ShellPkg/SmbiosView: Add decoding of SMB= IOS record type 43 Hi Star, This is regarding the following comments. > // > + // TPM Device (Type 43) The comment here is using "TPM Device (Type 43)", no comments to it. :) > + // All the comments in the function are using the heading of corresponding tab= le in SMBIOS spec. And for Type 43 it is " TPM Device (Type 43)". Please let me know if you like to change it. Based on this I will send the updated patch (addressing other comments as w= ell). Regards, Linson. -----Original Message----- From: Zeng, Star [mailto:star.zeng@intel.com] Sent: Wednesday, January 11, 2017 3:25 PM To: Augustine, Linson P ; edk2-devel@lists.01.org Cc: jaben.carsey@intel.com; ruiyu.ni@intel.com; star.zeng@intel.com Subject: Re: [edk2] [PATCH v2 1/1] ShellPkg/SmbiosView: Add decoding of SMB= IOS record type 43 Linson, Add minor comments inline. :) On 2017/1/11 16:57, Linson Augustine wrote: > Added decoding of the new SMBIOS Type 43 record. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Augustine Linson P > --- > ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c = | 72 +++++++++++++++++++- > ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.h = | 13 ++++ > ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c = | 6 +- > =20 > ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrin > gs.uni | 8 ++- > 4 files changed, 96 insertions(+), 3 deletions(-) > > diff --git > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c > index 7e17b69d5a..8d4a613c40 100644 > --- > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo > +++ .c > @@ -3,7 +3,7 @@ > > Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
> (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
> - (C) Copyright 2015 Hewlett Packard Enterprise Development LP
> + (C) Copyright 2015-2017 Hewlett Packard Enterprise Development=20 > + LP
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the = BSD License > which accompanies this distribution. The full text of the license=20 > may be found at @@ -1083,6 +1083,20 @@ SmbiosPrintStructure ( > break; > > // > + // TPM Device (Type 43) The comment here is using "TPM Device (Type 43)", no comments to it. :) > + // > + case 43: > + PRINT_BIT_FIELD (Struct, Type43, VendorID, 4); > + PRINT_STRUCT_VALUE_H (Struct, Type43, MajorSpecVersion); > + PRINT_STRUCT_VALUE_H (Struct, Type43, MinorSpecVersion); > + PRINT_STRUCT_VALUE_H (Struct, Type43, FirmwareVersion1); > + PRINT_STRUCT_VALUE_H (Struct, Type43, FirmwareVersion2); > + PRINT_PENDING_STRING (Struct, Type43, Description); > + DisplayTpmCharacteristics (ReadUnaligned64 ((UINT64 *) (UINTN) &(Str= uct->Type43->Characteristics)), Option); > + PRINT_STRUCT_VALUE_H (Struct, Type43, OemDefined); Thanks for the code update for this. > + break; > + > + // > // Inactive (Type 126) > // > case 126: > @@ -3225,3 +3239,59 @@ DisplaySPSCharacteristics ( > ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN (STR_SMBIOSVIEW_PRINTINFO_PO= WER_SUPPLY_NOT_REPLACE), gShellDebug1HiiHandle); > } > } > + > +/** > + Display TPM (Type 43) Device Characteristics. How about use "Display TPM Device (Type 43) Characteristics" to align with = the comment above? > + > + @param[in] Chara The information bits. > + @param[in] Option The optional information. > +**/ > +VOID > +DisplayTpmCharacteristics ( How about to use function name "DisplayTpmDeviceCharacteristics"? > + IN UINT64 Chara, > + IN UINT8 Option > + ) > +{ > + // > + // Print header > + // > + ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN > +(STR_SMBIOSVIEW_PRINTINFO_TPM_CHAR), gShellDebug1HiiHandle); > + // > + // print option > + // > + PRINT_INFO_OPTION (Chara, Option); > + > + // > + // Check all the bits and print information // This function does=20 > + not use Table because table of bits > + // are designed not to deal with UINT64 > + // > + if (BIT (Chara, 0) !=3D 0) { > + ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN > + (STR_SMBIOSVIEW_PRINTINFO_RESERVED_BIT), gShellDebug1HiiHandle); } > + > + if (BIT (Chara, 1) !=3D 0) { > + ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN > + (STR_SMBIOSVIEW_PRINTINFO_RESERVED_BIT), gShellDebug1HiiHandle); }=20 > + if (BIT (Chara, 2) !=3D 0) { > + ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN > + (STR_SMBIOSVIEW_PRINTINFO_TPM_CHAR_NOT_SUPPORTED), > + gShellDebug1HiiHandle); } > + > + if (BIT (Chara, 3) !=3D 0) { > + ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN > + (STR_SMBIOSVIEW_PRINTINFO_TPM_CONFIG_FWU), gShellDebug1HiiHandle); } > + > + if (BIT (Chara, 4) !=3D 0) { > + ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN > + (STR_SMBIOSVIEW_PRINTINFO_TPM_CONFIG_PLAT_SW), > + gShellDebug1HiiHandle); } > + > + if (BIT (Chara, 5) !=3D 0) { > + ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN > + (STR_SMBIOSVIEW_PRINTINFO_TPM_CONFIG_OEM), gShellDebug1HiiHandle); } > + > + // > + // Just print the Reserved > + // > + ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN > + (STR_SMBIOSVIEW_PRINTINFO_BITS_06_63), gShellDebug1HiiHandle); > + > +} > diff --git > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.h > b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.h > index 50667b684c..e5017990aa 100644 > --- > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.h > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo > +++ .h > @@ -2,6 +2,7 @@ > Module to clarify the element info of the smbios structure. > > Copyright (c) 2005 - 2015, Intel Corporation. All rights=20 > reserved.
> + (C) Copyright 2017 Hewlett Packard Enterprise Development LP
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the = BSD License > which accompanies this distribution. The full text of the license=20 > may be found at @@ -420,4 +421,16 @@ DisplaySPSCharacteristics ( > IN UINT8 Option > ); > > +/** > + Display TPM (Type 43) Device Characteristics. > + > + @param[in] Chara The information bits. > + @param[in] Option The optional information. > +**/ > +VOID > +DisplayTpmCharacteristics ( > + IN UINT64 Chara, > + IN UINT8 Option > + ); > + > #endif > diff --git > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c > index d0106c0b55..e1e79c4410 100644 > --- > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTabl > +++ e.c > @@ -3,7 +3,7 @@ > And give a interface of query a string out of a table. > > Copyright (c) 2005 - 2015, Intel Corporation. All rights=20 > reserved.
> - (C) Copyright 2016 Hewlett Packard Enterprise Development LP
> + (C) Copyright 2016-2017 Hewlett Packard Enterprise Development=20 > + LP
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the = BSD License > which accompanies this distribution. The full text of the license=20 > may be found at @@ -3190,6 +3190,10 @@ TABLE_ITEM StructureTypeInfoTable= [] =3D { > L" Management Controller Host Interface" > }, > { > + 43, > + L" TPM Device" > + }, > + { > 0x7E, > L" Inactive" > }, > diff --git > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStr > ings.uni > b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStr > ings.uni > index 9811542013..6c10db6993 100644 > --- > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStr > ings.uni > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosVie > +++ wStrings.uni > @@ -2,7 +2,7 @@ > // > // Copyright (c) 2005 - 2015, Intel Corporation. All rights=20 > reserved.
// (C) Copyright 2014-2015 Hewlett-Packard Development=20 > Company, L.P.
-// (C) Copyright 2015 Hewlett Packard Enterprise=20 > Development LP
> +// (C) Copyright 2015-2017 Hewlett Packard Enterprise Development=20 > +LP
> // This program and the accompanying materials // are licensed and=20 > made available under the terms and conditions of the BSD License //=20 > which accompanies this distribution. The full text of the license may=20 > be found at @@ -488,4 +488,10 @@ > #string STR_SMBIOSVIEW_SMBIOSVIEW_SMBIOS_TABLE #languag= e en-US "SmbiosView: SMBIOS table damaged\r\n" > #string STR_SMBIOSVIEW_SMBIOSVIEW_OUT_OF_MEM #languag= e en-US "SmbiosView: Out of memory\r\n" > #string STR_SMBIOSVIEW_SMBIOSVIEW_CANNOT_ACCESS_STATS #languag= e en-US "SmbiosView: Cannot access statistics table\r\n" > +#string STR_SMBIOSVIEW_PRINTINFO_TPM_CHAR #languag= e en-US "TPM Characteristics: \r\n" > +#string STR_SMBIOSVIEW_PRINTINFO_TPM_CHAR_NOT_SUPPORTED #languag= e en-US "TPM Characteristics Not Supported\r\n" For the two strings above, how about use "TPM_DEVICE" instead "TPM" in STR_= SMBIOSVIEW_PRINTINFO_XXX and use "TPM Device" instead of "TPM" in string co= ntent? > +#string STR_SMBIOSVIEW_PRINTINFO_TPM_CONFIG_FWU #languag= e en-US "TPM Family configurable via firmware update\r\n" > +#string STR_SMBIOSVIEW_PRINTINFO_TPM_CONFIG_PLAT_SW #languag= e en-US "TPM Family configurable via platform software support\r\n" > +#string STR_SMBIOSVIEW_PRINTINFO_TPM_CONFIG_OEM #languag= e en-US "TPM Family configurable via OEM proprietary mechanism\r\n" For the three strings above, how about use "TPM_DEVICE" instead "TPM" in ST= R_SMBIOSVIEW_PRINTINFO_XXX and *remove* TPM in string content as the functi= on should have printed "TPM Device Characteristics:" before printing the Ch= aracteristics? Thanks, Star > +#string STR_SMBIOSVIEW_PRINTINFO_BITS_06_63 #languag= e en-US "Bits 6:63 are reserved\r\n" > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel