From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (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 801A02095B9C9 for ; Wed, 23 Aug 2017 16:42:14 -0700 (PDT) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga105.fm.intel.com with ESMTP; 23 Aug 2017 16:44:48 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,419,1498546800"; d="scan'208";a="1007038497" Received: from orsmsx108.amr.corp.intel.com ([10.22.240.6]) by orsmga003.jf.intel.com with ESMTP; 23 Aug 2017 16:44:48 -0700 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.211]) by ORSMSX108.amr.corp.intel.com ([169.254.2.204]) with mapi id 14.03.0319.002; Wed, 23 Aug 2017 16:44:45 -0700 From: "Kinney, Michael D" To: "Dong, Eric" , "edk2-devel@lists.01.org" , "Kinney, Michael D" CC: "Ni, Ruiyu" Thread-Topic: [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition. Thread-Index: AQHTF9IW565hwVWDAUe0Ri1kK8ylhqKSmr3Q Date: Wed, 23 Aug 2017 23:44:45 +0000 Message-ID: References: <1503026917-13296-1-git-send-email-eric.dong@intel.com> <1503026917-13296-2-git-send-email-eric.dong@intel.com> In-Reply-To: <1503026917-13296-2-git-send-email-eric.dong@intel.com> 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.139] MIME-Version: 1.0 Subject: Re: [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry 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: Wed, 23 Aug 2017 23:42:14 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Eric, I see that the legal values for the Size field of=20 RTIT_TOPA_TABLE_ENTRY are documented in the SDM. I think we should add the enum for PROC_TRACE_MEM_SIZE into ArchitecturalMsr.h next to RTIT_TOPA_TABLE_ENTRY. Maybe rename it to RTIT_TOPA_MEMORY_SIZE with enum names that start with "RtitTopaMemorySize". The Copyright date in ArchitecturalMsr.h also needs to=20 be updated. The #define for MAX_TOPA_ENTRY_COUNT need a comment block that describes what it is and why it has a value of 2. I see 2 entries are needed because the list of entries must be terminated by an entry with the END bit set to 1, so 2 entries are required to use a single valid entry. ProcTrace.c: Please update programming of MSR_IA32_RTIT_OUTPUT_BASE to use the named bit fields from the structure. ProcTrace.c: Please update programming of MSR_IA32_RTIT_OUTPUT_MASK_PTRS to use the named bit fields from the structure. ProcTrace.c: I am confused by the programming MSR_IA32_RTIT_OUTPUT_MASK_PTRS to 0x7f. That sets a Reserved field to all 1s. ProcTrace.c: I see a mix of setting TopaEntryPtr using both bit fields and the Uint64 field. Can we change to use only the bit fields. Do we really need to set the address field=20 in entry #1? Isn't setting END to 1 enough? Thanks, Mike > -----Original Message----- > From: Dong, Eric > Sent: Thursday, August 17, 2017 8:29 PM > To: edk2-devel@lists.01.org > Cc: Kinney, Michael D ; Ni, Ruiyu > > Subject: [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT > TOPA table entry definition. >=20 > Add RTIT TOPA table entry definition to architecturalMsr.h file. >=20 > Cc: Michael Kinney > Cc: Ruiyu Ni > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong > --- > UefiCpuPkg/Include/Register/ArchitecturalMsr.h | 55 > ++++++++++++++++++++++++++ > 1 file changed, 55 insertions(+) >=20 > diff --git a/UefiCpuPkg/Include/Register/ArchitecturalMsr.h > b/UefiCpuPkg/Include/Register/ArchitecturalMsr.h > index 4f9c103..40c4383 100644 > --- a/UefiCpuPkg/Include/Register/ArchitecturalMsr.h > +++ b/UefiCpuPkg/Include/Register/ArchitecturalMsr.h > @@ -4534,6 +4534,61 @@ typedef union { > UINT64 Uint64; > } MSR_IA32_RTIT_OUTPUT_MASK_PTRS_REGISTER; >=20 > +/** > + Format of ToPA table entries. > +**/ > +typedef union { > + /// > + /// Individual bit fields > + /// > + struct { > + /// > + /// [Bit 0] END. See Section 35.2.6.2, "Table of Physical > Addresses (ToPA)". > + /// > + UINT32 END:1; > + UINT32 Reserved1:1; > + /// > + /// [Bit 2] INT. See Section 35.2.6.2, "Table of Physical > Addresses (ToPA)". > + /// > + UINT32 INT:1; > + UINT32 Reserved2:1; > + /// > + /// [Bit 4] STOP. See Section 35.2.6.2, "Table of Physical > Addresses (ToPA)". > + /// > + UINT32 STOP:1; > + UINT32 Reserved3:1; > + /// > + /// [Bit 6:9] Indicates the size of the associated output > region. See Section > + /// 35.2.6.2, "Table of Physical Addresses (ToPA)". > + /// > + UINT32 Size:4; > + UINT32 Reserved4:2; > + /// > + /// [Bit 12:31] Output Region Base Physical Address low part. > + /// [Bit 12:31] Output Region Base Physical Address [12:63] > value to match. > + /// ATTENTION: The size of the address field is determined by > the processor's > + /// physical-address width (MAXPHYADDR) in bits, as reported > in > + /// CPUID.80000008H:EAX[7:0]. the above part of address > reserved. > + /// True address field is [12:MAXPHYADDR-1], [MAXPHYADDR:63] > is reserved part. > + /// Detail see Section 35.2.6.2, "Table of Physical Addresses > (ToPA)". > + /// > + UINT32 Base:20; > + /// > + /// [Bit 32:63] Output Region Base Physical Address high > part. > + /// [Bit 32:63] Output Region Base Physical Address [12:63] > value to match. > + /// ATTENTION: The size of the address field is determined by > the processor's > + /// physical-address width (MAXPHYADDR) in bits, as reported > in > + /// CPUID.80000008H:EAX[7:0]. the above part of address > reserved. > + /// True address field is [12:MAXPHYADDR-1], [MAXPHYADDR:63] > is reserved part. > + /// Detail see Section 35.2.6.2, "Table of Physical Addresses > (ToPA)". > + /// > + UINT32 BaseHi:32; > + } Bits; > + /// > + /// All bit fields as a 64-bit value > + /// > + UINT64 Uint64; > +} RTIT_TOPA_TABLE_ENTRY; >=20 > /** > Trace Control Register (R/W). If (CPUID.(EAX=3D07H, > ECX=3D0):EBX[25] =3D 1). > -- > 2.7.0.windows.1