* [Patch v2 0/2] Add new definition for TOPA table entry
@ 2017-08-18 3:28 Eric Dong
2017-08-18 3:28 ` [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition Eric Dong
2017-08-18 3:28 ` [Patch v2 2/2] UefiCpuPkg/CpuCommonFeaturesLib: Use MSR data structure when change MSR value Eric Dong
0 siblings, 2 replies; 6+ messages in thread
From: Eric Dong @ 2017-08-18 3:28 UTC (permalink / raw)
To: edk2-devel
This patch series add new definition for RTIT Topa table entry and
update code to use MSR structure instead of BITxx style to make code
more user friendly.
Eric Dong (2):
UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition.
UefiCpuPkg/CpuCommonFeaturesLib: Use MSR data structure when change
MSR value.
UefiCpuPkg/Include/Register/ArchitecturalMsr.h | 55 +++++++++++++++++
.../Library/CpuCommonFeaturesLib/ProcTrace.c | 70 +++++++++++++---------
2 files changed, 97 insertions(+), 28 deletions(-)
--
2.7.0.windows.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition. 2017-08-18 3:28 [Patch v2 0/2] Add new definition for TOPA table entry Eric Dong @ 2017-08-18 3:28 ` Eric Dong 2017-08-23 23:44 ` Kinney, Michael D 2017-08-18 3:28 ` [Patch v2 2/2] UefiCpuPkg/CpuCommonFeaturesLib: Use MSR data structure when change MSR value Eric Dong 1 sibling, 1 reply; 6+ messages in thread From: Eric Dong @ 2017-08-18 3:28 UTC (permalink / raw) To: edk2-devel; +Cc: Michael Kinney, Ruiyu Ni Add RTIT TOPA table entry definition to architecturalMsr.h file. Cc: Michael Kinney <michael.d.kinney@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong <eric.dong@intel.com> --- UefiCpuPkg/Include/Register/ArchitecturalMsr.h | 55 ++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) 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; +/** + 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; /** Trace Control Register (R/W). If (CPUID.(EAX=07H, ECX=0):EBX[25] = 1). -- 2.7.0.windows.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition. 2017-08-18 3:28 ` [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition Eric Dong @ 2017-08-23 23:44 ` Kinney, Michael D 2017-08-24 3:21 ` Dong, Eric 0 siblings, 1 reply; 6+ messages in thread From: Kinney, Michael D @ 2017-08-23 23:44 UTC (permalink / raw) To: Dong, Eric, edk2-devel@lists.01.org, Kinney, Michael D; +Cc: Ni, Ruiyu Hi Eric, I see that the legal values for the Size field of 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 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 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 <michael.d.kinney@intel.com>; Ni, Ruiyu > <ruiyu.ni@intel.com> > Subject: [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT > TOPA table entry definition. > > Add RTIT TOPA table entry definition to architecturalMsr.h file. > > Cc: Michael Kinney <michael.d.kinney@intel.com> > Cc: Ruiyu Ni <ruiyu.ni@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong <eric.dong@intel.com> > --- > UefiCpuPkg/Include/Register/ArchitecturalMsr.h | 55 > ++++++++++++++++++++++++++ > 1 file changed, 55 insertions(+) > > 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; > > +/** > + 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; > > /** > Trace Control Register (R/W). If (CPUID.(EAX=07H, > ECX=0):EBX[25] = 1). > -- > 2.7.0.windows.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition. 2017-08-23 23:44 ` Kinney, Michael D @ 2017-08-24 3:21 ` Dong, Eric 2017-08-24 3:32 ` Dong, Eric 0 siblings, 1 reply; 6+ messages in thread From: Dong, Eric @ 2017-08-24 3:21 UTC (permalink / raw) To: Kinney, Michael D, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu Mike, Add my comments below. -----Original Message----- From: Kinney, Michael D Sent: Thursday, August 24, 2017 7:45 AM To: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com> Cc: Ni, Ruiyu <ruiyu.ni@intel.com> Subject: RE: [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition. Hi Eric, I see that the legal values for the Size field of 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". [[Eric]] agree, update in the new patch. The Copyright date in ArchitecturalMsr.h also needs to be updated. [[Eric]] agree, update in the new patch. 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. [[Eric]] agree, update in the new patch. 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. [[Eric]] I think current code is easy to write but it may touch the reserve bits. Agree to change to use the bit fields. Update in the new patches. ProcTrace.c: I am confused by the programming MSR_IA32_RTIT_OUTPUT_MASK_PTRS to 0x7f. That sets a Reserved field to all 1s. [[Eric]] Copy from the sdm, default value is all 1. Write to it is ignored. So I think the original code is same as set the others bits all 0. I have update code to directly set other bits to 0. Please check the new patch. [cid:image001.jpg@01D31CCB.2A4EEA30] 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. [[Eric]] same reason as before, use Uint64 is easy to write but may touch the reserved bits. Update code to use bit fields. Do we really need to set the address field in entry #1? Isn't setting END to 1 enough? [[Eric]] Copy below from sdm. This code point to the current table for a circular case. [cid:image002.jpg@01D31CCB.2A4EEA30] Thanks, Mike > -----Original Message----- > From: Dong, Eric > Sent: Thursday, August 17, 2017 8:29 PM > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > Cc: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Ni, Ruiyu > <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>> > Subject: [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA > table entry definition. > > Add RTIT TOPA table entry definition to architecturalMsr.h file. > > Cc: Michael Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> > Cc: Ruiyu Ni <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong <eric.dong@intel.com<mailto:eric.dong@intel.com>> > --- > UefiCpuPkg/Include/Register/ArchitecturalMsr.h | 55 > ++++++++++++++++++++++++++ > 1 file changed, 55 insertions(+) > > 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; > > +/** > + 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; > > /** > Trace Control Register (R/W). If (CPUID.(EAX=07H, ECX=0):EBX[25] = > 1). > -- > 2.7.0.windows.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition. 2017-08-24 3:21 ` Dong, Eric @ 2017-08-24 3:32 ` Dong, Eric 0 siblings, 0 replies; 6+ messages in thread From: Dong, Eric @ 2017-08-24 3:32 UTC (permalink / raw) To: Dong, Eric, Kinney, Michael D, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu Mike, I found the attach pic can't show up. Append the content directly from sdm. -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Dong, Eric Sent: Thursday, August 24, 2017 11:22 AM To: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org Cc: Ni, Ruiyu <ruiyu.ni@intel.com> Subject: Re: [edk2] [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition. Mike, Add my comments below. -----Original Message----- From: Kinney, Michael D Sent: Thursday, August 24, 2017 7:45 AM To: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com> Cc: Ni, Ruiyu <ruiyu.ni@intel.com> Subject: RE: [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition. Hi Eric, I see that the legal values for the Size field of 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". [[Eric]] agree, update in the new patch. The Copyright date in ArchitecturalMsr.h also needs to be updated. [[Eric]] agree, update in the new patch. 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. [[Eric]] agree, update in the new patch. 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. [[Eric]] I think current code is easy to write but it may touch the reserve bits. Agree to change to use the bit fields. Update in the new patches. ProcTrace.c: I am confused by the programming MSR_IA32_RTIT_OUTPUT_MASK_PTRS to 0x7f. That sets a Reserved field to all 1s. [[Eric]] Copy from the sdm, default value is all 1. Write to it is ignored. So I think the original code is same as set the others bits all 0. I have update code to directly set other bits to 0. Please check the new patch. [cid:image001.jpg@01D31CCB.2A4EEA30] [[Eric]] Found the attached pic can't show, copy the context like below: Table 35-9. IA32_RTIT_OUTPUT_MASK_PTRS MSR Position Bit Name At Reset Bit Description 6:0 LowerMask 7FH Forced to 1, writes are ignored 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. [[Eric]] same reason as before, use Uint64 is easy to write but may touch the reserved bits. Update code to use bit fields. Do we really need to set the address field in entry #1? Isn't setting END to 1 enough? [[Eric]] Copy below from sdm. This code point to the current table for a circular case. [cid:image002.jpg@01D31CCB.2A4EEA30] [[Eric]] Found the attached pic can't show, copy the context like below: 35.2.6.2 Table of Physical Addresses (ToPA) When IA32_RTIT_CTL.ToPA is set and IA32_RTIT_CTL.FabricEn is clear, the ToPA output mechanism is utilized. The ToPA mechanism uses a linked list of tables; see Figure 35-1 for an illustrative example. Each entry in the table contains some attribute bits, a pointer to an output region, and the size of the region. The last entry in the table may hold a pointer to the next table. This pointer can either point to the top of the current table (for circular array) or to the base of another table. The table size is not fixed, since the link to the next table can exist at any entry. Thanks, Mike > -----Original Message----- > From: Dong, Eric > Sent: Thursday, August 17, 2017 8:29 PM > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > Cc: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Ni, Ruiyu > <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>> > Subject: [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA > table entry definition. > > Add RTIT TOPA table entry definition to architecturalMsr.h file. > > Cc: Michael Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> > Cc: Ruiyu Ni <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong <eric.dong@intel.com<mailto:eric.dong@intel.com>> > --- > UefiCpuPkg/Include/Register/ArchitecturalMsr.h | 55 > ++++++++++++++++++++++++++ > 1 file changed, 55 insertions(+) > > 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; > > +/** > + 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; > > /** > Trace Control Register (R/W). If (CPUID.(EAX=07H, ECX=0):EBX[25] = > 1). > -- > 2.7.0.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Patch v2 2/2] UefiCpuPkg/CpuCommonFeaturesLib: Use MSR data structure when change MSR value. 2017-08-18 3:28 [Patch v2 0/2] Add new definition for TOPA table entry Eric Dong 2017-08-18 3:28 ` [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition Eric Dong @ 2017-08-18 3:28 ` Eric Dong 1 sibling, 0 replies; 6+ messages in thread From: Eric Dong @ 2017-08-18 3:28 UTC (permalink / raw) To: edk2-devel; +Cc: Michael Kinney, Ruiyu Ni When update MSR values, current code use BITxx to modify it. Enhance the code to use corresponding MSR's data structures to make it more user friendly. Cc: Michael Kinney <michael.d.kinney@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong <eric.dong@intel.com> --- .../Library/CpuCommonFeaturesLib/ProcTrace.c | 70 +++++++++++++--------- 1 file changed, 42 insertions(+), 28 deletions(-) diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c index 40e6321..a90dd4e 100644 --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c @@ -70,7 +70,7 @@ typedef struct { } PROC_TRACE_DATA; typedef struct { - UINT64 TopaEntry[MAX_TOPA_ENTRY_COUNT]; + RTIT_TOPA_TABLE_ENTRY TopaEntry[MAX_TOPA_ENTRY_COUNT]; } PROC_TRACE_TOPA_TABLE; /** @@ -186,7 +186,6 @@ ProcTraceInitialize ( IN BOOLEAN State ) { - UINT64 MsrValue; UINT32 MemRegionSize; UINTN Pages; UINTN Alignment; @@ -199,6 +198,11 @@ ProcTraceInitialize ( PROC_TRACE_TOPA_TABLE *TopaTable; PROC_TRACE_DATA *ProcTraceData; BOOLEAN FirstIn; + MSR_IA32_RTIT_CTL_REGISTER CtrlReg; + MSR_IA32_RTIT_STATUS_REGISTER StatusReg; + MSR_IA32_RTIT_OUTPUT_BASE_REGISTER OutputBaseReg; + MSR_IA32_RTIT_OUTPUT_MASK_PTRS_REGISTER OutputMaskPtrsReg; + RTIT_TOPA_TABLE_ENTRY *TopaEntryPtr; ProcTraceData = (PROC_TRACE_DATA *) ConfigData; @@ -221,29 +225,28 @@ ProcTraceInitialize ( // // Clear MSR_IA32_RTIT_CTL[0] and IA32_RTIT_STS only if MSR_IA32_RTIT_CTL[0]==1b // - MsrValue = AsmReadMsr64 (MSR_IA32_RTIT_CTL); - if ((MsrValue & BIT0) != 0) { + CtrlReg.Uint64 = AsmReadMsr64 (MSR_IA32_RTIT_CTL); + if (CtrlReg.Bits.TraceEn != 0) { /// /// Clear bit 0 in MSR IA32_RTIT_CTL (570) /// - MsrValue &= (UINT64) ~BIT0; + CtrlReg.Bits.TraceEn = 0; CPU_REGISTER_TABLE_WRITE64 ( ProcessorNumber, Msr, MSR_IA32_RTIT_CTL, - MsrValue + CtrlReg.Uint64 ); /// /// Clear MSR IA32_RTIT_STS (571h) to all zeros /// - MsrValue = AsmReadMsr64 (MSR_IA32_RTIT_STATUS); - MsrValue &= 0x0; + StatusReg.Uint64 = 0x0; CPU_REGISTER_TABLE_WRITE64 ( ProcessorNumber, Msr, MSR_IA32_RTIT_STATUS, - MsrValue + StatusReg.Uint64 ); } @@ -309,35 +312,35 @@ ProcTraceInitialize ( // // Clear MSR IA32_RTIT_CTL (0x570) ToPA (Bit 8) // - MsrValue = AsmReadMsr64 (MSR_IA32_RTIT_CTL); - MsrValue &= (UINT64) ~BIT8; + CtrlReg.Uint64 = AsmReadMsr64 (MSR_IA32_RTIT_CTL); + CtrlReg.Bits.ToPA = 0; CPU_REGISTER_TABLE_WRITE64 ( ProcessorNumber, Msr, MSR_IA32_RTIT_CTL, - MsrValue + CtrlReg.Uint64 ); // // Program MSR IA32_RTIT_OUTPUT_BASE (0x560) bits[47:12] with the allocated Memory Region // - MsrValue = (UINT64) MemRegionBaseAddr; + OutputBaseReg.Uint64 = (UINT64) MemRegionBaseAddr; CPU_REGISTER_TABLE_WRITE64 ( ProcessorNumber, Msr, MSR_IA32_RTIT_OUTPUT_BASE, - MsrValue + OutputBaseReg.Uint64 ); // // Program the Mask bits for the Memory Region to MSR IA32_RTIT_OUTPUT_MASK_PTRS (561h) // - MsrValue = (UINT64) MemRegionSize - 1; + OutputMaskPtrsReg.Uint64 = (UINT64) MemRegionSize - 1; CPU_REGISTER_TABLE_WRITE64 ( ProcessorNumber, Msr, MSR_IA32_RTIT_OUTPUT_MASK_PTRS, - MsrValue + OutputMaskPtrsReg.Uint64 ); } @@ -408,55 +411,66 @@ ProcTraceInitialize ( } TopaTable = (PROC_TRACE_TOPA_TABLE *) TopaTableBaseAddr; - TopaTable->TopaEntry[0] = (UINT64) (MemRegionBaseAddr | ((ProcTraceData->ProcTraceMemSize) << 6)) & ~BIT0; - TopaTable->TopaEntry[1] = (UINT64) TopaTableBaseAddr | BIT0; + TopaEntryPtr = &TopaTable->TopaEntry[0]; + TopaEntryPtr->Uint64 = (UINT64) MemRegionBaseAddr; + TopaEntryPtr->Bits.Size = ProcTraceData->ProcTraceMemSize; + TopaEntryPtr->Bits.END = 0; + + TopaEntryPtr = &TopaTable->TopaEntry[1]; + TopaEntryPtr->Uint64 = (UINT64) TopaTableBaseAddr; + TopaEntryPtr->Bits.END = 1; // // Program the MSR IA32_RTIT_OUTPUT_BASE (0x560) bits[47:12] with ToPA base // - MsrValue = (UINT64) TopaTableBaseAddr; + OutputBaseReg.Uint64 = (UINT64) TopaTableBaseAddr; CPU_REGISTER_TABLE_WRITE64 ( ProcessorNumber, Msr, MSR_IA32_RTIT_OUTPUT_BASE, - MsrValue + OutputBaseReg.Uint64 ); // // Set the MSR IA32_RTIT_OUTPUT_MASK (0x561) bits[63:7] to 0 // + OutputMaskPtrsReg.Uint64 = (UINT64) 0x7F; CPU_REGISTER_TABLE_WRITE64 ( ProcessorNumber, Msr, MSR_IA32_RTIT_OUTPUT_MASK_PTRS, - 0x7F + OutputMaskPtrsReg.Uint64 ); // // Enable ToPA output scheme by enabling MSR IA32_RTIT_CTL (0x570) ToPA (Bit 8) // - MsrValue = AsmReadMsr64 (MSR_IA32_RTIT_CTL); - MsrValue |= BIT8; + CtrlReg.Uint64 = AsmReadMsr64 (MSR_IA32_RTIT_CTL); + CtrlReg.Bits.ToPA = 1; CPU_REGISTER_TABLE_WRITE64 ( ProcessorNumber, Msr, MSR_IA32_RTIT_CTL, - MsrValue + CtrlReg.Uint64 ); } /// /// Enable the Processor Trace feature from MSR IA32_RTIT_CTL (570h) /// - MsrValue = AsmReadMsr64 (MSR_IA32_RTIT_CTL); - MsrValue |= (UINT64) BIT0 + BIT2 + BIT3 + BIT13; + CtrlReg.Uint64 = AsmReadMsr64 (MSR_IA32_RTIT_CTL); + CtrlReg.Bits.OS = 1; + CtrlReg.Bits.User = 1; + CtrlReg.Bits.BranchEn = 1; if (!State) { - MsrValue &= (UINT64) ~BIT0; + CtrlReg.Bits.TraceEn = 0; + } else { + CtrlReg.Bits.TraceEn = 1; } CPU_REGISTER_TABLE_WRITE64 ( ProcessorNumber, Msr, MSR_IA32_RTIT_CTL, - MsrValue + CtrlReg.Uint64 ); return RETURN_SUCCESS; -- 2.7.0.windows.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-24 3:30 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-08-18 3:28 [Patch v2 0/2] Add new definition for TOPA table entry Eric Dong 2017-08-18 3:28 ` [Patch v2 1/2] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition Eric Dong 2017-08-23 23:44 ` Kinney, Michael D 2017-08-24 3:21 ` Dong, Eric 2017-08-24 3:32 ` Dong, Eric 2017-08-18 3:28 ` [Patch v2 2/2] UefiCpuPkg/CpuCommonFeaturesLib: Use MSR data structure when change MSR value Eric Dong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox