From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (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 405F82095B9C9 for ; Thu, 17 Aug 2017 01:49:52 -0700 (PDT) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Aug 2017 01:52:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,387,1498546800"; d="scan'208";a="141362774" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga006.fm.intel.com with ESMTP; 17 Aug 2017 01:52:18 -0700 Received: from fmsmsx125.amr.corp.intel.com (10.18.125.40) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 17 Aug 2017 01:52:18 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by FMSMSX125.amr.corp.intel.com (10.18.125.40) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 17 Aug 2017 01:52:18 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.183]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.114]) with mapi id 14.03.0319.002; Thu, 17 Aug 2017 16:52:16 +0800 From: "Dong, Eric" To: "Kinney, Michael D" , "edk2-devel@lists.01.org" CC: "Ni, Ruiyu" , "Gao, Liming" Thread-Topic: [edk2] [Patch 1/2] UefiCpuPkg: Add comments for PCDs definition. Thread-Index: AQHTFM47sJKwIRcNVkmNNRmPVoJfT6KGoWMAgAF7PvD//4/2gIAAj3sQ Date: Thu, 17 Aug 2017 08:52:15 +0000 Message-ID: References: <1502695374-11368-1-git-send-email-eric.dong@intel.com> <1502695374-11368-2-git-send-email-eric.dong@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [Patch 1/2] UefiCpuPkg: Add comments for PCDs definition. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Aug 2017 08:49:52 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Mike, My reason to defined disable options for memory size and output scheme is I= can use them as my default policy(all in disable status). If not define di= sable options, I don't know which default value are better than others. Als= o disable options also defined clearly invalid tag and valuable to add user= friendly code check like below: if ((ProcTraceData->ProcTraceMemSize >=3D EnumProcTraceMemDisable) || (ProcTraceData->ProcTraceOutputScheme >=3D OutputSchemeInvalid)) { return FALSE; } If you think these definitions are redundant, what's the default value do y= ou think is suitable for memory size and output scheme. Thanks, Eric -----Original Message----- From: Kinney, Michael D=20 Sent: Thursday, August 17, 2017 3:55 PM To: Dong, Eric ; edk2-devel@lists.01.org; Kinney, Mich= ael D Cc: Ni, Ruiyu ; Gao, Liming Subject: RE: [edk2] [Patch 1/2] UefiCpuPkg: Add comments for PCDs definitio= n. Eric, For a setup page, the scheme can be greyed out or hidden if the feature is = disabled. If the feature is disabled, it does not make sense to allow the = user to set the memory size or the scheme. Mike > -----Original Message----- > From: Dong, Eric > Sent: Wednesday, August 16, 2017 11:48 PM > To: Kinney, Michael D ; edk2-=20 > devel@lists.01.org > Cc: Ni, Ruiyu ; Gao, Liming=20 > ; Dong, Eric > Subject: RE: [edk2] [Patch 1/2] UefiCpuPkg: Add comments for PCDs=20 > definition. >=20 > Hi Mike, >=20 > Processor trace feature need to get three inputs from end user. 1.=20 > Enabe/disable, 2. Memory size, 3, Output scheme. > It will have three menus in UI to let end user to select. I want the=20 > default value for all three menus are disabled, so I add invalid or=20 > disable for both PCDs. Maybe I can enhance OutputSchemeInvalid to=20 > OutputSchemeDisable for consistent with EnumProcTraceMemDisable. >=20 > For the BITxx issue, I have submit a patch for it, please help to=20 > review it. >=20 > Thanks, > Eric > -----Original Message----- > From: Kinney, Michael D > Sent: Wednesday, August 16, 2017 11:59 PM > To: Dong, Eric ; edk2-devel@lists.01.org; Kinney,=20 > Michael D > Cc: Ni, Ruiyu ; Gao, Liming > Subject: RE: [edk2] [Patch 1/2] UefiCpuPkg: Add comments for PCDs=20 > definition. >=20 > Eric, >=20 > For the PCD PcdCpuProcTraceOutputScheme, the description of "Invalid=20 > scheme" > does not seem like a good description for disable. >=20 > Do we really need the value 0x02 for > PcdCpuProcTraceOutputScheme at all? > Can we use PcdCpuFeaturesSetting set to 0x10 ProcTraceMemDisable to=20 > know that the feature is disabled, and only look at=20 > PcdCpuProcTraceOutputScheme if PcdCpuFeaturesSetting is set to a value=20 > where the memory size is > 0? >=20 > Also, when looking at where these PCD values are used in=20 > UefiCpuPkg\Library\CpuCommonFeaturesLib\ProcTrace.c, I see use of=20 > BITxx to modify the MSR_IA32_RTIT_CTL values. Please use the bit=20 > fields in the MSR_IA32_RTIT_CTL_REGISTER structure instead. You can=20 > work on this change as a separate patch. >=20 > Thanks, >=20 > Mike >=20 >=20 > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On > Behalf Of > > Eric Dong > > Sent: Monday, August 14, 2017 12:23 AM > > To: edk2-devel@lists.01.org > > Cc: Ni, Ruiyu ; Kinney, Michael D=20 > > ; Gao, Liming > > > Subject: [edk2] [Patch 1/2] UefiCpuPkg: Add comments for > PCDs > > definition. > > > > Add valid/default values for PCD PcdCpuProcTraceMemSize and=20 > > PcdCpuProcTraceOutputScheme in the comment part. > > > > Cc: Liming Gao > > Cc: Ruiyu Ni > > Cc: Michael D Kinney > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Eric Dong > > --- > > UefiCpuPkg/UefiCpuPkg.dec | 28 ++++++++++++++++++++++++++-- > > 1 file changed, 26 insertions(+), 2 deletions(-) > > > > diff --git a/UefiCpuPkg/UefiCpuPkg.dec > b/UefiCpuPkg/UefiCpuPkg.dec > > index 2ddeab4..b4e099d 100644 > > --- a/UefiCpuPkg/UefiCpuPkg.dec > > +++ b/UefiCpuPkg/UefiCpuPkg.dec > > @@ -285,12 +285,36 @@ > > # @ValidList 0x80000001 | 0 > > gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesSetting|{0x00, > 0x00, 0x00, > > 0x00, 0x00, 0x00, 0x00, 0x00}|VOID*|0x00000019 > > > > - ## Contains the size of memory required when CPU > processor trace is > > enabled. > > + ## Contains the size of memory required when CPU > processor > > trace is enabled.

> > + # Default value is 0x10 which disables the processor > > trace.
> > + # 0x0 - 4K.
> > + # 0x1 - 8K.
> > + # 0x2 - 16K.
> > + # 0x3 - 32K.
> > + # 0x4 - 64K.
> > + # 0x5 - 128K.
> > + # 0x6 - 256K.
> > + # 0x7 - 512K.
> > + # 0x8 - 1M.
> > + # 0x9 - 2M.
> > + # 0xA - 4M.
> > + # 0xB - 8M.
> > + # 0xC - 16M.
> > + # 0xD - 32M.
> > + # 0xE - 64M.
> > + # 0xF - 128M.
> > + # 0x10 - ProcTraceMemDisable.
> > # @Prompt The memory size used for processor trace. > > + # @ValidRange 0x80000001 | 0 - 0x10 > > > > > gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceMemSize|0x10|UINT32|0 > x6 > > 0000012 > > > > - ## Contains the processor trace output scheme when CPU > processor > > trace is enabled. > > + ## Contains the processor trace output scheme when CPU > > processor trace is enabled.

> > + # Default value is 2 which disables the processor > trace.
# 0 > > + - Single Range output scheme.
# 1 - ToPA(Table of > physical > > + address) scheme.
# 2 - Invalid scheme.
> > # @Prompt The processor trace output scheme. > > + # @ValidRange 0x80000001 | 0 - 2 > > > > > gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceOutputScheme|0x2|UINT > 8| > > 0x60000015 > > > > [UserExtensions.TianoCore."ExtraFiles"] > > -- > > 2.7.0.windows.1 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel