[Patch 0/4] Enhance the implementation for Proc Trace feature.

public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed

* [Patch 0/4] Enhance the implementation for Proc Trace feature.
@ 2017-08-24  3:03 Eric Dong
  2017-08-24  3:03 ` [Patch 1/4] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition Eric Dong
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Eric Dong @ 2017-08-24  3:03 UTC (permalink / raw)
  To: edk2-devel

V2:
1. Move RTIT_TOPA_MEMORY_SIZE definition to architecturalMsr.h file.
2. Use strcture menbers to do value assignment.
3. Enhance the comments for PCD PcdCpuProcTraceMemSize and PcdCpuProcTraceOutputScheme.

Eric Dong (4):
  UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition.
  UefiCpuPkg/CpuCommonFeaturesLib: Use MSR data structure when change
    MSR value.
  UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition.
  UefiCpuPkg: Update default for
    PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme.

 UefiCpuPkg/Include/Register/ArchitecturalMsr.h     |  79 ++++++++++++-
 .../Library/CpuCommonFeaturesLib/ProcTrace.c       | 131 +++++++++++----------
 UefiCpuPkg/UefiCpuPkg.dec                          |  22 ++--
 UefiCpuPkg/UefiCpuPkg.uni                          |  16 +--
 5 files changed, 200 insertions(+), 91 deletions(-)

-- 
2.7.0.windows.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Patch 1/4] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition.
  2017-08-24  3:03 [Patch 0/4] Enhance the implementation for Proc Trace feature Eric Dong
@ 2017-08-24  3:03 ` Eric Dong
  2017-08-24  3:03 ` [Patch 2/4] UefiCpuPkg/CpuCommonFeaturesLib: Use MSR data structure when change MSR value Eric Dong
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Eric Dong @ 2017-08-24  3:03 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael Kinney, Ruiyu Ni

Add RTIT TOPA table entry definition to architecturalMsr.h file.

V2: Add RTIT_TOPA_MEMORY_SIZE 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 | 79 +++++++++++++++++++++++++-
 1 file changed, 78 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Include/Register/ArchitecturalMsr.h b/UefiCpuPkg/Include/Register/ArchitecturalMsr.h
index 4f9c103..34fdf5b 100644
--- a/UefiCpuPkg/Include/Register/ArchitecturalMsr.h
+++ b/UefiCpuPkg/Include/Register/ArchitecturalMsr.h
@@ -6,7 +6,7 @@
   returned is a single 32-bit or 64-bit value, then a data structure is not
   provided for that MSR.
 
-  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
   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 may be found at
@@ -4534,6 +4534,83 @@ 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;
+
+///
+/// The size of the associated output region usd by Topa.
+///
+typedef enum {
+  RtitTopaMemorySize4K = 0,
+  RtitTopaMemorySize8K,
+  RtitTopaMemorySize16K,
+  RtitTopaMemorySize32K,
+  RtitTopaMemorySize64K,
+  RtitTopaMemorySize128K,
+  RtitTopaMemorySize256K,
+  RtitTopaMemorySize512K,
+  RtitTopaMemorySize1M,
+  RtitTopaMemorySize2M,
+  RtitTopaMemorySize4M,
+  RtitTopaMemorySize8M,
+  RtitTopaMemorySize16M,
+  RtitTopaMemorySize32M,
+  RtitTopaMemorySize64M,
+  RtitTopaMemorySize128M
+} RTIT_TOPA_MEMORY_SIZE;
 
 /**
   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

* [Patch 2/4] UefiCpuPkg/CpuCommonFeaturesLib: Use MSR data structure when change MSR value.
  2017-08-24  3:03 [Patch 0/4] Enhance the implementation for Proc Trace feature Eric Dong
  2017-08-24  3:03 ` [Patch 1/4] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition Eric Dong
@ 2017-08-24  3:03 ` Eric Dong
  2017-08-24  3:03 ` [Patch 3/4] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition Eric Dong
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Eric Dong @ 2017-08-24  3:03 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.

V2: Move architecturalMsr.h file. definition to architecturalMsr.h file.
    Use structure members to do value assignment.

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       | 116 +++++++++++----------
 1 file changed, 61 insertions(+), 55 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
index 40e6321..e4636b2 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
@@ -14,30 +14,17 @@
 
 #include "CpuCommonFeatures.h"
 
-#define MAX_TOPA_ENTRY_COUNT 2
-
 ///
-/// Processor trace buffer size selection.
+/// This macro define the max entries in the Topa table.
+/// 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. 
+/// At least 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.
 ///
-typedef enum {
-  Enum4K    = 0,
-  Enum8K,
-  Enum16K,
-  Enum32K,
-  Enum64K,
-  Enum128K,
-  Enum256K,
-  Enum512K,
-  Enum1M,
-  Enum2M,
-  Enum4M,
-  Enum8M,
-  Enum16M,
-  Enum32M,
-  Enum64M,
-  Enum128M,
-  EnumProcTraceMemDisable
-} PROC_TRACE_MEM_SIZE;
+#define MAX_TOPA_ENTRY_COUNT         2
+
 
 ///
 /// Processor trace output scheme selection.
@@ -70,7 +57,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;
 
 /**
@@ -134,8 +121,8 @@ ProcTraceSupport (
   // Check if ProcTraceMemorySize option is enabled (0xFF means disable by user)
   //
   ProcTraceData = (PROC_TRACE_DATA *) ConfigData;
-  if ((ProcTraceData->ProcTraceMemSize >= EnumProcTraceMemDisable) ||
-      (ProcTraceData->ProcTraceOutputScheme >= OutputSchemeInvalid)) {
+  if ((ProcTraceData->ProcTraceMemSize > RtitTopaMemorySize128M) ||
+      (ProcTraceData->ProcTraceOutputScheme > ProcTraceOutputSchemeToPA)) {
     return FALSE;
   }
 
@@ -186,7 +173,6 @@ ProcTraceInitialize (
   IN BOOLEAN                           State
   )
 {
-  UINT64                               MsrValue;
   UINT32                               MemRegionSize;
   UINTN                                Pages;
   UINTN                                Alignment;
@@ -199,6 +185,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 +212,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,37 +299,38 @@ 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
+    // Program MSR IA32_RTIT_OUTPUT_BASE (0x560) bits[63:7] with the allocated Memory Region
     //
-    MsrValue = (UINT64) MemRegionBaseAddr;
+    OutputBaseReg.Bits.Base = (MemRegionBaseAddr >> 7) & 0x01FFFFFF;
+    OutputBaseReg.Bits.BaseHi = RShiftU64 ((UINT64) MemRegionBaseAddr, 32) & 0xFFFFFFFF;
     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.Bits.MaskOrTableOffset = ((MemRegionSize - 1) >> 7) & 0x01FFFFFF;
+    OutputMaskPtrsReg.Bits.OutputOffset = RShiftU64 ((UINT64) (MemRegionSize - 1), 32) & 0xFFFFFFFF;
     CPU_REGISTER_TABLE_WRITE64 (
       ProcessorNumber,
       Msr,
       MSR_IA32_RTIT_OUTPUT_MASK_PTRS,
-      MsrValue
+      OutputMaskPtrsReg.Uint64
       );
-
   }
 
   //
@@ -408,55 +399,70 @@ 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->Bits.Base = (MemRegionBaseAddr >> 12) & 0x000FFFFF;
+    TopaEntryPtr->Bits.BaseHi = RShiftU64 ((UINT64) MemRegionBaseAddr, 32) & 0xFFFFFFFF;
+    TopaEntryPtr->Bits.Size = ProcTraceData->ProcTraceMemSize;
+    TopaEntryPtr->Bits.END = 0;
+
+    TopaEntryPtr = &TopaTable->TopaEntry[1];
+    TopaEntryPtr->Bits.Base = (TopaTableBaseAddr >> 12) & 0x000FFFFF;
+    TopaEntryPtr->Bits.BaseHi = RShiftU64 ((UINT64) TopaTableBaseAddr, 32) & 0xFFFFFFFF;
+    TopaEntryPtr->Bits.END = 1;
 
     //
-    // Program the MSR IA32_RTIT_OUTPUT_BASE (0x560) bits[47:12] with ToPA base
+    // Program the MSR IA32_RTIT_OUTPUT_BASE (0x560) bits[63:7] with ToPA base
     //
-    MsrValue = (UINT64) TopaTableBaseAddr;
+    OutputBaseReg.Bits.Base = (TopaTableBaseAddr >> 7) & 0x01FFFFFF;
+    OutputBaseReg.Bits.BaseHi = RShiftU64 ((UINT64) TopaTableBaseAddr, 32) & 0xFFFFFFFF;
     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.Bits.MaskOrTableOffset = 0;
+    OutputMaskPtrsReg.Bits.OutputOffset = 0;
     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

* [Patch 3/4] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition.
  2017-08-24  3:03 [Patch 0/4] Enhance the implementation for Proc Trace feature Eric Dong
  2017-08-24  3:03 ` [Patch 1/4] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition Eric Dong
  2017-08-24  3:03 ` [Patch 2/4] UefiCpuPkg/CpuCommonFeaturesLib: Use MSR data structure when change MSR value Eric Dong
@ 2017-08-24  3:03 ` Eric Dong
  2017-08-24  3:03 ` [Patch 4/4] UefiCpuPkg: Update default for PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme Eric Dong
  2017-08-25  0:31 ` [Patch 0/4] Enhance the implementation for Proc Trace feature Kinney, Michael D
  4 siblings, 0 replies; 6+ messages in thread
From: Eric Dong @ 2017-08-24  3:03 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael Kinney, Ruiyu Ni

The EnumProcTraceMemDisable/OutputSchemeInvalid are redundant
definitions. These definitions can be handled by other code,
so remove them.

V2: Change enum members name.

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/Library/CpuCommonFeaturesLib/ProcTrace.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
index e4636b2..167c1be 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
@@ -30,10 +30,9 @@
 /// Processor trace output scheme selection.
 ///
 typedef enum {
-  OutputSchemeSingleRange = 0,
-  OutputSchemeToPA,
-  OutputSchemeInvalid
-} PROC_TRACE_OUTPUT_SCHEME;
+  RtitOutputSchemeSingleRange = 0,
+  RtitOutputSchemeToPA
+} RTIT_OUTPUT_SCHEME;
 
 typedef struct  {
   BOOLEAN  ProcTraceSupported;
@@ -122,7 +121,7 @@ ProcTraceSupport (
   //
   ProcTraceData = (PROC_TRACE_DATA *) ConfigData;
   if ((ProcTraceData->ProcTraceMemSize > RtitTopaMemorySize128M) ||
-      (ProcTraceData->ProcTraceOutputScheme > ProcTraceOutputSchemeToPA)) {
+      (ProcTraceData->ProcTraceOutputScheme > RtitOutputSchemeToPA)) {
     return FALSE;
   }
 
@@ -138,8 +137,8 @@ ProcTraceSupport (
   AsmCpuidEx (CPUID_INTEL_PROCESSOR_TRACE, CPUID_INTEL_PROCESSOR_TRACE_MAIN_LEAF, NULL, NULL, &Ecx.Uint32, NULL);
   ProcTraceData->ProcessorData[ProcessorNumber].TopaSupported = (BOOLEAN) (Ecx.Bits.RTIT == 1);
   ProcTraceData->ProcessorData[ProcessorNumber].SingleRangeSupported = (BOOLEAN) (Ecx.Bits.SingleRangeOutput == 1);
-  if (ProcTraceData->ProcessorData[ProcessorNumber].TopaSupported || 
-      ProcTraceData->ProcessorData[ProcessorNumber].SingleRangeSupported) {
+  if ((ProcTraceData->ProcessorData[ProcessorNumber].TopaSupported && (ProcTraceData->ProcTraceOutputScheme == RtitOutputSchemeToPA)) ||
+      (ProcTraceData->ProcessorData[ProcessorNumber].SingleRangeSupported && (ProcTraceData->ProcTraceOutputScheme == RtitOutputSchemeSingleRange))) {
     return TRUE;
   }
 
@@ -291,7 +290,7 @@ ProcTraceInitialize (
   //  Single Range output scheme
   //
   if (ProcTraceData->ProcessorData[ProcessorNumber].SingleRangeSupported && 
-      (ProcTraceData->ProcTraceOutputScheme == OutputSchemeSingleRange)) {
+      (ProcTraceData->ProcTraceOutputScheme == RtitOutputSchemeSingleRange)) {
     if (FirstIn) {
       DEBUG ((DEBUG_INFO, "ProcTrace: Enabling Single Range Output scheme \n"));
     }
@@ -337,7 +336,7 @@ ProcTraceInitialize (
   //  ToPA(Table of physical address) scheme
   //
   if (ProcTraceData->ProcessorData[ProcessorNumber].TopaSupported && 
-      (ProcTraceData->ProcTraceOutputScheme == OutputSchemeToPA)) {
+      (ProcTraceData->ProcTraceOutputScheme == RtitOutputSchemeToPA)) {
     //
     //  Create ToPA structure aligned at 4KB for each logical thread
     //  with at least 2 entries by 8 bytes size each. The first entry
-- 
2.7.0.windows.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Patch 4/4] UefiCpuPkg: Update default for PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme.
  2017-08-24  3:03 [Patch 0/4] Enhance the implementation for Proc Trace feature Eric Dong
                   ` (2 preceding siblings ...)
  2017-08-24  3:03 ` [Patch 3/4] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition Eric Dong
@ 2017-08-24  3:03 ` Eric Dong
  2017-08-25  0:31 ` [Patch 0/4] Enhance the implementation for Proc Trace feature Kinney, Michael D
  4 siblings, 0 replies; 6+ messages in thread
From: Eric Dong @ 2017-08-24  3:03 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael Kinney, Ruiyu Ni

These two definitions have redundant definition which can be handle by code.
This patch update them to follow new code definitions.

V2: Add more comments for the PCDs and keep consistent in .dec and .uni files.

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/UefiCpuPkg.dec | 22 ++++++++++++----------
 UefiCpuPkg/UefiCpuPkg.uni | 16 +++++++++-------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index b4e099d..3bd8740 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -286,7 +286,9 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesSetting|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}|VOID*|0x00000019
 
   ## Contains the size of memory required when CPU processor trace is enabled.<BR><BR>
-  #  Default value is 0x10 which disables the processor trace.<BR>
+  #  Processor trace is enabled through set BIT44(CPU_FEATURE_PROC_TRACE) in PcdCpuFeaturesSetting.<BR><BR>
+  #  This PCD is ignored if CPU processor trace is disabled.<BR><BR>
+  #  Default value is 0x00 which means 4KB of memory is allocated if CPU processor trace is enabled.<BR>
   #  0x0  -  4K.<BR>
   #  0x1  -  8K.<BR>
   #  0x2  -  16K.<BR>
@@ -303,19 +305,19 @@
   #  0xD  -  32M.<BR>
   #  0xE  -  64M.<BR>
   #  0xF  -  128M.<BR>
-  #  0x10 -  ProcTraceMemDisable.<BR>
-  # @Prompt The memory size used for processor trace.
-  # @ValidRange  0x80000001 | 0 - 0x10
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceMemSize|0x10|UINT32|0x60000012
+  # @Prompt The memory size used for processor trace if processor trace is enabled.
+  # @ValidRange  0x80000001 | 0 - 0xF
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceMemSize|0x0|UINT32|0x60000012
 
   ## Contains the processor trace output scheme when CPU processor trace is enabled.<BR><BR>
-  #  Default value is 2 which disables the processor trace.<BR>
+  #  Processor trace is enabled through set BIT44(CPU_FEATURE_PROC_TRACE) in PcdCpuFeaturesSetting.<BR><BR>
+  #  This PCD is ignored if CPU processor trace is disabled.<BR><BR>
+  #  Default value is 0 which means single range output scheme will be used if CPU processor trace is enabled.<BR>
   #  0 - Single Range output scheme.<BR>
   #  1 - ToPA(Table of physical address) scheme.<BR>
-  #  2 - Invalid scheme.<BR>
-  # @Prompt The processor trace output scheme.
-  # @ValidRange  0x80000001 | 0 - 2
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceOutputScheme|0x2|UINT8|0x60000015
+  # @Prompt The processor trace output scheme used when processor trace is enabled.
+  # @ValidRange  0x80000001 | 0 - 1
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceOutputScheme|0x0|UINT8|0x60000015
 
 [UserExtensions.TianoCore."ExtraFiles"]
   UefiCpuPkgExtra.uni
diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
index 858e4a7..9472b18 100644
--- a/UefiCpuPkg/UefiCpuPkg.uni
+++ b/UefiCpuPkg/UefiCpuPkg.uni
@@ -197,8 +197,10 @@
 
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuProcTraceMemSize_PROMPT  #language en-US "Memory size used by Processor Trace feature."
 
-#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuProcTraceMemSize_HELP  #language en-US "User input the memory size can be used by processor trace feature.<BR><BR>\n"
-                                                                                   "Default value is 0x10 which disables the processor memory trace.<BR>\n"
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuProcTraceMemSize_HELP  #language en-US "User input the size of memory required when CPU processor trace is enabled.<BR><BR>\n"
+                                                                                   "Processor trace is enabled through set BIT44(CPU_FEATURE_PROC_TRACE) in PcdCpuFeaturesSetting.<BR><BR>\n"
+                                                                                   "This PCD is ignored if CPU processor trace is disabled.<BR><BR>\n"
+                                                                                   "Default value is 0x00 which means 4KB of memory is allocated if CPU processor trace is enabled.<BR>\n"
                                                                                    "0x0  -  4K.<BR>\n"
                                                                                    "0x1  -  8K.<BR>\n"
                                                                                    "0x2  -  16K.<BR>\n"
@@ -215,12 +217,12 @@
                                                                                    "0xD  -  32M.<BR>\n"
                                                                                    "0xE  -  64M.<BR>\n"
                                                                                    "0xF  -  128M.<BR>\n"
-                                                                                   "0x10 -  ProcTraceMemDisable.<BR>\n"
 
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuProcTraceOutputScheme_PROMPT  #language en-US "Processor Trace output scheme type."
 
-#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuProcTraceOutputScheme_HELP  #language en-US "User input the processor trace output scheme type.<BR><BR>\n"
-                                                                                        "Default value is 2 which disables the processor memory trace.<BR>\n"

+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuProcTraceOutputScheme_HELP  #language en-US "User input the processor trace output scheme when CPU processor trace is enabled.<BR><BR>\n"
+                                                                                        "Processor trace is enabled through set BIT44(CPU_FEATURE_PROC_TRACE) in PcdCpuFeaturesSetting.<BR><BR>\n"
+                                                                                        "This PCD is ignored if CPU processor trace is disabled.<BR><BR>\n"
+                                                                                        "Default value is 0 which means single range output scheme will be used if CPU processor trace is enabled.<BR>\n"
                                                                                         "0 - Single Range output scheme.<BR>\n"

-                                                                                        "1 - ToPA(Table of physical address) scheme.<BR>\n"

-                                                                                        "2 - Invalid scheme.<BR>\n"
\ No newline at end of file
+                                                                                        "1 - ToPA(Table of physical address) scheme.<BR>\n"
\ No newline at end of file
-- 
2.7.0.windows.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Patch 0/4] Enhance the implementation for Proc Trace feature.
  2017-08-24  3:03 [Patch 0/4] Enhance the implementation for Proc Trace feature Eric Dong
                   ` (3 preceding siblings ...)
  2017-08-24  3:03 ` [Patch 4/4] UefiCpuPkg: Update default for PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme Eric Dong
@ 2017-08-25  0:31 ` Kinney, Michael D
  4 siblings, 0 replies; 6+ messages in thread
From: Kinney, Michael D @ 2017-08-25  0:31 UTC (permalink / raw)
  To: Dong, Eric, edk2-devel@lists.01.org, Kinney, Michael D

Series

Reviewed-by: Michael Kinney <michael.d.kinney@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> Behalf Of Eric Dong
> Sent: Wednesday, August 23, 2017 8:03 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch 0/4] Enhance the implementation for
> Proc Trace feature.
> 
> V2:
> 1. Move RTIT_TOPA_MEMORY_SIZE definition to architecturalMsr.h
> file.
> 2. Use strcture menbers to do value assignment.
> 3. Enhance the comments for PCD PcdCpuProcTraceMemSize and
> PcdCpuProcTraceOutputScheme.
> 
> Eric Dong (4):
>   UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry
> definition.
>   UefiCpuPkg/CpuCommonFeaturesLib: Use MSR data structure when
> change
>     MSR value.
>   UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant
> definition.
>   UefiCpuPkg: Update default for
>     PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme.
> 
>  UefiCpuPkg/Include/Register/ArchitecturalMsr.h     |  79
> ++++++++++++-
>  .../Library/CpuCommonFeaturesLib/ProcTrace.c       | 131
> +++++++++++----------
>  UefiCpuPkg/UefiCpuPkg.dec                          |  22 ++--
>  UefiCpuPkg/UefiCpuPkg.uni                          |  16 +--
>  5 files changed, 200 insertions(+), 91 deletions(-)
> 
> --
> 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

end of thread, other threads:[~2017-08-25  0:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-24  3:03 [Patch 0/4] Enhance the implementation for Proc Trace feature Eric Dong
2017-08-24  3:03 ` [Patch 1/4] UefiCpuPkg/ArchitecturalMsr.h: Add RTIT TOPA table entry definition Eric Dong
2017-08-24  3:03 ` [Patch 2/4] UefiCpuPkg/CpuCommonFeaturesLib: Use MSR data structure when change MSR value Eric Dong
2017-08-24  3:03 ` [Patch 3/4] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition Eric Dong
2017-08-24  3:03 ` [Patch 4/4] UefiCpuPkg: Update default for PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme Eric Dong
2017-08-25  0:31 ` [Patch 0/4] Enhance the implementation for Proc Trace feature Kinney, Michael D

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox