From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (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 9CA4921E2DA62 for ; Thu, 17 Aug 2017 00:52:48 -0700 (PDT) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga105.jf.intel.com with ESMTP; 17 Aug 2017 00:55:15 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,386,1498546800"; d="scan'208";a="1163534685" Received: from orsmsx105.amr.corp.intel.com ([10.22.225.132]) by orsmga001.jf.intel.com with ESMTP; 17 Aug 2017 00:55:15 -0700 Received: from orsmsx156.amr.corp.intel.com (10.22.240.22) by ORSMSX105.amr.corp.intel.com (10.22.225.132) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 17 Aug 2017 00:55:15 -0700 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.211]) by ORSMSX156.amr.corp.intel.com ([169.254.8.249]) with mapi id 14.03.0319.002; Thu, 17 Aug 2017 00:55:14 -0700 From: "Kinney, Michael D" To: "Dong, Eric" , "edk2-devel@lists.01.org" , "Kinney, Michael D" CC: "Ni, Ruiyu" , "Gao, Liming" Thread-Topic: [edk2] [Patch 1/2] UefiCpuPkg: Add comments for PCDs definition. Thread-Index: AQHTFM46dlWLoHrjk06PgorEwxtss6KHJYaAgAFvqoD//50p8A== Date: Thu, 17 Aug 2017 07:55:13 +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: dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [10.22.254.140] 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 07:52:48 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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=20 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- > devel@lists.01.org > Cc: Ni, Ruiyu ; Gao, Liming > ; Dong, Eric > Subject: RE: [edk2] [Patch 1/2] UefiCpuPkg: Add comments for > PCDs definition. >=20 > Hi Mike, >=20 > Processor trace feature need to get three inputs from end > user. 1. Enabe/disable, 2. Memory size, 3, Output scheme. > It will have three menus in UI to let end user to select. I > want the default value for all three menus are disabled, so I > add invalid or disable for both PCDs. Maybe I can enhance > OutputSchemeInvalid to OutputSchemeDisable for consistent with > EnumProcTraceMemDisable. >=20 > For the BITxx issue, I have submit a patch for it, please help > to 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, Michael D > Cc: Ni, Ruiyu ; Gao, Liming > > Subject: RE: [edk2] [Patch 1/2] UefiCpuPkg: Add comments for > PCDs definition. >=20 > Eric, >=20 > For the PCD PcdCpuProcTraceOutputScheme, the description of > "Invalid 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 know that the feature is disabled, and > only look at PcdCpuProcTraceOutputScheme if > PcdCpuFeaturesSetting is set to a value where the memory size > is > 0? >=20 > Also, when looking at where these PCD values are used in > UefiCpuPkg\Library\CpuCommonFeaturesLib\ProcTrace.c, I see use > of BITxx to modify the MSR_IA32_RTIT_CTL values. Please use > the bit fields in the MSR_IA32_RTIT_CTL_REGISTER structure > instead. You can 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 > > ; Gao, Liming > > > Subject: [edk2] [Patch 1/2] UefiCpuPkg: Add comments for > PCDs > > definition. > > > > Add valid/default values for PCD PcdCpuProcTraceMemSize and > > 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