[Patch V2 0/2] Update ProcTrace feature code for new requirements.

[Patch V2 0/2] Update ProcTrace feature code for new requirements.

[duntan]

In V2 patch set:
1.Remove the patch to set MTC to 0.
2.Updated 'Update code to support enable ProcTrace only on BSP' based on Ray's comments.
  PCD name is updated to PcdCpuProcTraceBspOnly and cache the value in ConfigData.
  Use MemRegionBaseAddr and TopaTableBaseAddr instead of the unused local variable to record buffer address.
  
3.Updated 'Update PT code to support enable collect performance' based on Ray's comments.
  PCD name is updated to PcdCpuProcTracePerformanceCollecting and cache the value in ConfigData
  Also, if CYC packet is supported is checked in Support function and recorded in ProcTraceData->ProcessorData


Dun Tan (2):
  UefiCpuPkg: Update code to support enable ProcTrace only on BSP
  UefiCpuPkg: Update PT code to support enable collect performance

 UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf |  12 +++++++-----
 UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c              | 200 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------------------------
 UefiCpuPkg/UefiCpuPkg.dec                                        |  15 +++++++++++++++
 3 files changed, 155 insertions(+), 72 deletions(-)

-- 
2.39.1.windows.1

[Patch V2 1/2] UefiCpuPkg: Update code to support enable ProcTrace only on BSP

[duntan]

Update code to support enable ProcTrace only on BSP. Add a new
dynamic PCD to indicate if enable ProcTrace only on BSP. In
ProcTrace.c code, if this new PCD is true, only allocate buffer
and set CtrlReg.Bits.TraceEn to 1 for BSP.

Bugzila: https://bugzilla.tianocore.org/show_bug.cgi?id=4423
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Xiao X Chen <xiao.x.chen@intel.com>
---
 UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf |   3 ++-
 UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c              | 168 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------
 UefiCpuPkg/UefiCpuPkg.dec                                        |   7 +++++++
 3 files changed, 113 insertions(+), 65 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf
index 7fbcd8da0e..d803012ce2 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf
@@ -4,7 +4,7 @@
 #  This library registers CPU features defined in Intel(R) 64 and IA-32
 #  Architectures Software Developer's Manual.
 #
-# Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -62,3 +62,4 @@
   gUefiCpuPkgTokenSpaceGuid.PcdIsPowerOnReset                ## SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceOutputScheme      ## SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceMemSize           ## SOMETIMES_CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceBspOnly           ## SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
index 04e6a60728..367a9f9cfe 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
@@ -1,7 +1,7 @@
 /** @file
   Intel Processor Trace feature.
 
-  Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -46,6 +46,8 @@ typedef struct  {
 
   UINTN                        *TopaMemArray;
 
+  BOOLEAN                      EnableOnBspOnly;
+
   PROC_TRACE_PROCESSOR_DATA    *ProcessorData;
 } PROC_TRACE_DATA;
 
@@ -77,6 +79,7 @@ ProcTraceGetConfigData (
   ConfigData->NumberOfProcessors    = (UINT32)NumberOfProcessors;
   ConfigData->ProcTraceMemSize      = PcdGet32 (PcdCpuProcTraceMemSize);
   ConfigData->ProcTraceOutputScheme = PcdGet8 (PcdCpuProcTraceOutputScheme);
+  ConfigData->EnableOnBspOnly       = PcdGetBool (PcdCpuProcTraceBspOnly);
 
   return ConfigData;
 }
@@ -188,6 +191,7 @@ ProcTraceInitialize (
   MSR_IA32_RTIT_OUTPUT_BASE_REGISTER       OutputBaseReg;
   MSR_IA32_RTIT_OUTPUT_MASK_PTRS_REGISTER  OutputMaskPtrsReg;
   RTIT_TOPA_TABLE_ENTRY                    *TopaEntryPtr;
+  BOOLEAN                                  IsBsp;
 
   //
   // The scope of the MSR_IA32_RTIT_* is core for below processor type, only program
@@ -236,6 +240,12 @@ ProcTraceInitialize (
     return RETURN_SUCCESS;
   }
 
+  IsBsp = (CpuInfo->ProcessorInfo.StatusFlag & PROCESSOR_AS_BSP_BIT) ? TRUE : FALSE;
+
+  if (ProcTraceData->EnableOnBspOnly && !IsBsp) {
+    return RETURN_SUCCESS;
+  }
+
   MemRegionBaseAddr = 0;
   FirstIn           = FALSE;
 
@@ -260,43 +270,59 @@ ProcTraceInitialize (
     //   address base in MSR, IA32_RTIT_OUTPUT_BASE (560h) bits 47:12. Note that all regions must be
     //   aligned based on their size, not just 4K. Thus a 2M region must have bits 20:12 cleared.
     //
-    ThreadMemRegionTable = (UINTN *)AllocatePool (ProcTraceData->NumberOfProcessors * sizeof (UINTN *));
-    if (ThreadMemRegionTable == NULL) {
-      DEBUG ((DEBUG_ERROR, "Allocate ProcTrace ThreadMemRegionTable Failed\n"));
-      return RETURN_OUT_OF_RESOURCES;
-    }
 
-    ProcTraceData->ThreadMemRegionTable = ThreadMemRegionTable;
-
-    for (Index = 0; Index < ProcTraceData->NumberOfProcessors; Index++, ProcTraceData->AllocatedThreads++) {
-      Pages          = EFI_SIZE_TO_PAGES (MemRegionSize);
-      Alignment      = MemRegionSize;
-      AlignedAddress = (UINTN)AllocateAlignedReservedPages (Pages, Alignment);
-      if (AlignedAddress == 0) {
-        DEBUG ((DEBUG_ERROR, "ProcTrace: Out of mem, allocated only for %d threads\n", ProcTraceData->AllocatedThreads));
-        if (Index == 0) {
-          //
-          // Could not allocate for BSP even
-          //
-          FreePool ((VOID *)ThreadMemRegionTable);
-          ThreadMemRegionTable = NULL;
-          return RETURN_OUT_OF_RESOURCES;
+    Pages     = EFI_SIZE_TO_PAGES (MemRegionSize);
+    Alignment = MemRegionSize;
+    if (ProcTraceData->EnableOnBspOnly) {
+      MemRegionBaseAddr = (UINTN)AllocateAlignedReservedPages (Pages, Alignment);
+      if (MemRegionBaseAddr == 0) {
+        //
+        // Could not allocate for BSP even
+        //
+        DEBUG ((DEBUG_ERROR, "ProcTrace: Out of mem, failed to allocate buffer for BSP\n"));
+        return RETURN_OUT_OF_RESOURCES;
+      }
+
+      DEBUG ((DEBUG_INFO, "ProcTrace: Allocated PT MemRegionBaseAddr(aligned) for BSP only: 0x%llX.\n", (UINT64)MemRegionBaseAddr));
+    } else {
+      ThreadMemRegionTable = (UINTN *)AllocatePool (ProcTraceData->NumberOfProcessors * sizeof (UINTN *));
+      if (ThreadMemRegionTable == NULL) {
+        DEBUG ((DEBUG_ERROR, "Allocate ProcTrace ThreadMemRegionTable Failed\n"));
+        return RETURN_OUT_OF_RESOURCES;
+      }
+
+      ProcTraceData->ThreadMemRegionTable = ThreadMemRegionTable;
+
+      for (Index = 0; Index < ProcTraceData->NumberOfProcessors; Index++, ProcTraceData->AllocatedThreads++) {
+        AlignedAddress = (UINTN)AllocateAlignedReservedPages (Pages, Alignment);
+        if (AlignedAddress == 0) {
+          DEBUG ((DEBUG_ERROR, "ProcTrace: Out of mem, allocated only for %d threads\n", ProcTraceData->AllocatedThreads));
+          if (Index == 0) {
+            //
+            // Could not allocate for BSP even
+            //
+            FreePool ((VOID *)ThreadMemRegionTable);
+            ThreadMemRegionTable = NULL;
+            return RETURN_OUT_OF_RESOURCES;
+          }
+
+          break;
         }
 
-        break;
+        ThreadMemRegionTable[Index] = AlignedAddress;
+        DEBUG ((DEBUG_INFO, "ProcTrace: PT MemRegionBaseAddr(aligned) for thread %d: 0x%llX \n", Index, (UINT64)ThreadMemRegionTable[Index]));
       }
 
-      ThreadMemRegionTable[Index] = AlignedAddress;
-      DEBUG ((DEBUG_INFO, "ProcTrace: PT MemRegionBaseAddr(aligned) for thread %d: 0x%llX \n", Index, (UINT64)ThreadMemRegionTable[Index]));
+      DEBUG ((DEBUG_INFO, "ProcTrace: Allocated PT mem for %d thread \n", ProcTraceData->AllocatedThreads));
     }
-
-    DEBUG ((DEBUG_INFO, "ProcTrace: Allocated PT mem for %d thread \n", ProcTraceData->AllocatedThreads));
   }
 
-  if (ProcessorNumber < ProcTraceData->AllocatedThreads) {
-    MemRegionBaseAddr = ProcTraceData->ThreadMemRegionTable[ProcessorNumber];
-  } else {
-    return RETURN_SUCCESS;
+  if (!ProcTraceData->EnableOnBspOnly) {
+    if (ProcessorNumber < ProcTraceData->AllocatedThreads) {
+      MemRegionBaseAddr = ProcTraceData->ThreadMemRegionTable[ProcessorNumber];
+    } else {
+      return RETURN_SUCCESS;
+    }
   }
 
   ///
@@ -367,50 +393,64 @@ ProcTraceInitialize (
     //
     if (FirstIn) {
       DEBUG ((DEBUG_INFO, "ProcTrace: Enabling ToPA scheme \n"));
-      //
-      // Let BSP allocate ToPA table mem for all threads
-      //
-      TopaMemArray = (UINTN *)AllocatePool (ProcTraceData->AllocatedThreads * sizeof (UINTN *));
-      if (TopaMemArray == NULL) {
-        DEBUG ((DEBUG_ERROR, "ProcTrace: Allocate mem for ToPA Failed\n"));
-        return RETURN_OUT_OF_RESOURCES;
-      }
 
-      ProcTraceData->TopaMemArray = TopaMemArray;
+      Pages     = EFI_SIZE_TO_PAGES (sizeof (PROC_TRACE_TOPA_TABLE));
+      Alignment = 0x1000;
 
-      for (Index = 0; Index < ProcTraceData->AllocatedThreads; Index++) {
-        Pages          = EFI_SIZE_TO_PAGES (sizeof (PROC_TRACE_TOPA_TABLE));
-        Alignment      = 0x1000;
-        AlignedAddress = (UINTN)AllocateAlignedReservedPages (Pages, Alignment);
-        if (AlignedAddress == 0) {
-          if (Index < ProcTraceData->AllocatedThreads) {
-            ProcTraceData->AllocatedThreads = Index;
-          }
+      if (ProcTraceData->EnableOnBspOnly) {
+        TopaTableBaseAddr = (UINTN)AllocateAlignedReservedPages (Pages, Alignment);
+        if (TopaTableBaseAddr == 0) {
+          DEBUG ((DEBUG_ERROR, "ProcTrace: Out of mem, failed to allocate ToPA mem for BSP"));
+          return RETURN_OUT_OF_RESOURCES;
+        }
 
-          DEBUG ((DEBUG_ERROR, "ProcTrace:  Out of mem, allocated ToPA mem only for %d threads\n", ProcTraceData->AllocatedThreads));
-          if (Index == 0) {
-            //
-            // Could not allocate for BSP even
-            //
-            FreePool ((VOID *)TopaMemArray);
-            TopaMemArray = NULL;
-            return RETURN_OUT_OF_RESOURCES;
+        DEBUG ((DEBUG_INFO, "ProcTrace: Topa table address(aligned) for BSP only: 0x%llX \n", (UINT64)TopaTableBaseAddr));
+      } else {
+        //
+        // Let BSP allocate ToPA table mem for all threads
+        //
+        TopaMemArray = (UINTN *)AllocatePool (ProcTraceData->AllocatedThreads * sizeof (UINTN *));
+        if (TopaMemArray == NULL) {
+          DEBUG ((DEBUG_ERROR, "ProcTrace: Allocate mem for ToPA Failed\n"));
+          return RETURN_OUT_OF_RESOURCES;
+        }
+
+        ProcTraceData->TopaMemArray = TopaMemArray;
+
+        for (Index = 0; Index < ProcTraceData->AllocatedThreads; Index++) {
+          AlignedAddress = (UINTN)AllocateAlignedReservedPages (Pages, Alignment);
+          if (AlignedAddress == 0) {
+            if (Index < ProcTraceData->AllocatedThreads) {
+              ProcTraceData->AllocatedThreads = Index;
+            }
+
+            DEBUG ((DEBUG_ERROR, "ProcTrace:  Out of mem, allocated ToPA mem only for %d threads\n", ProcTraceData->AllocatedThreads));
+            if (Index == 0) {
+              //
+              // Could not allocate for BSP even
+              //
+              FreePool ((VOID *)TopaMemArray);
+              TopaMemArray = NULL;
+              return RETURN_OUT_OF_RESOURCES;
+            }
+
+            break;
           }
 
-          break;
+          TopaMemArray[Index] = AlignedAddress;
+          DEBUG ((DEBUG_INFO, "ProcTrace: Topa table address(aligned) for thread %d is 0x%llX \n", Index, (UINT64)TopaMemArray[Index]));
         }
 
-        TopaMemArray[Index] = AlignedAddress;
-        DEBUG ((DEBUG_INFO, "ProcTrace: Topa table address(aligned) for thread %d is 0x%llX \n", Index, (UINT64)TopaMemArray[Index]));
+        DEBUG ((DEBUG_INFO, "ProcTrace: Allocated ToPA mem for %d thread \n", ProcTraceData->AllocatedThreads));
       }
-
-      DEBUG ((DEBUG_INFO, "ProcTrace: Allocated ToPA mem for %d thread \n", ProcTraceData->AllocatedThreads));
     }
 
-    if (ProcessorNumber < ProcTraceData->AllocatedThreads) {
-      TopaTableBaseAddr = ProcTraceData->TopaMemArray[ProcessorNumber];
-    } else {
-      return RETURN_SUCCESS;
+    if (!ProcTraceData->EnableOnBspOnly) {
+      if (ProcessorNumber < ProcTraceData->AllocatedThreads) {
+        TopaTableBaseAddr = ProcTraceData->TopaMemArray[ProcessorNumber];
+      } else {
+        return RETURN_SUCCESS;
+      }
     }
 
     TopaTable                 = (PROC_TRACE_TOPA_TABLE *)TopaTableBaseAddr;
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index a5528277ff..14dadfbf53 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -422,5 +422,12 @@
   # @Prompt GHCB Hypervisor Features
   gUefiCpuPkgTokenSpaceGuid.PcdGhcbHypervisorFeatures|0x0|UINT64|0x60000018
 
+  ## This PCD indicates whether CPU processor trace is enabled on BSP only when CPU processor trace is enabled.<BR><BR>
+  #  This PCD is ignored if CPU processor trace is disabled.<BR><BR>
+  #  TRUE  - CPU processor trace is enabled on BSP only.<BR>
+  #  FASLE - CPU processor trace is enabled on all CPU.<BR>
+  # @Prompt Enable CPU processor trace only on BSP.
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceBspOnly|FALSE|BOOLEAN|0x60000019
+
 [UserExtensions.TianoCore."ExtraFiles"]
   UefiCpuPkgExtra.uni
-- 
2.39.1.windows.1

[Patch V2 2/2] UefiCpuPkg: Update PT code to support enable collect performance

[duntan]

Update ProcTrace feature code to support enable collect performance
data by generating CYC and TSC packets. Add a new dynamic
PCD to indicate if enable performance collecting. In ProcTrace.c
code, if this new PCD is true, after check cpuid, CYC and TSC
packets will be generated by setting the corresponding MSR bits
feilds if supported.

Bugzila: https://bugzilla.tianocore.org/show_bug.cgi?id=4423
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Xiao X Chen <xiao.x.chen@intel.com>
---
 UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf | 11 ++++++-----
 UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c              | 34 ++++++++++++++++++++++++++++++----
 UefiCpuPkg/UefiCpuPkg.dec                                        |  8 ++++++++
 3 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf
index d803012ce2..1b823155b1 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf
@@ -58,8 +58,9 @@
   LocalApicLib
 
 [Pcd]
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuClockModulationDutyCycle   ## SOMETIMES_CONSUMES
-  gUefiCpuPkgTokenSpaceGuid.PcdIsPowerOnReset                ## SOMETIMES_CONSUMES
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceOutputScheme      ## SOMETIMES_CONSUMES
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceMemSize           ## SOMETIMES_CONSUMES
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceBspOnly           ## SOMETIMES_CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuClockModulationDutyCycle       ## SOMETIMES_CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdIsPowerOnReset                    ## SOMETIMES_CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceOutputScheme          ## SOMETIMES_CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceMemSize               ## SOMETIMES_CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceBspOnly               ## SOMETIMES_CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTracePerformanceCollecting ## SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
index 367a9f9cfe..92926486df 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
@@ -33,6 +33,7 @@ typedef struct  {
   MSR_IA32_RTIT_CTL_REGISTER                 RtitCtrl;
   MSR_IA32_RTIT_OUTPUT_BASE_REGISTER         RtitOutputBase;
   MSR_IA32_RTIT_OUTPUT_MASK_PTRS_REGISTER    RtitOutputMaskPtrs;
+  BOOLEAN                                    CycPacketSupported;
 } PROC_TRACE_PROCESSOR_DATA;
 
 typedef struct  {
@@ -47,6 +48,7 @@ typedef struct  {
   UINTN                        *TopaMemArray;
 
   BOOLEAN                      EnableOnBspOnly;
+  BOOLEAN                      EnablePerformanceCollecting;
 
   PROC_TRACE_PROCESSOR_DATA    *ProcessorData;
 } PROC_TRACE_DATA;
@@ -76,10 +78,11 @@ ProcTraceGetConfigData (
   ASSERT (ConfigData != NULL);
   ConfigData->ProcessorData = (PROC_TRACE_PROCESSOR_DATA *)((UINT8 *)ConfigData + sizeof (PROC_TRACE_DATA));
 
-  ConfigData->NumberOfProcessors    = (UINT32)NumberOfProcessors;
-  ConfigData->ProcTraceMemSize      = PcdGet32 (PcdCpuProcTraceMemSize);
-  ConfigData->ProcTraceOutputScheme = PcdGet8 (PcdCpuProcTraceOutputScheme);
-  ConfigData->EnableOnBspOnly       = PcdGetBool (PcdCpuProcTraceBspOnly);
+  ConfigData->NumberOfProcessors          = (UINT32)NumberOfProcessors;
+  ConfigData->ProcTraceMemSize            = PcdGet32 (PcdCpuProcTraceMemSize);
+  ConfigData->ProcTraceOutputScheme       = PcdGet8 (PcdCpuProcTraceOutputScheme);
+  ConfigData->EnableOnBspOnly             = PcdGetBool (PcdCpuProcTraceBspOnly);
+  ConfigData->EnablePerformanceCollecting = PcdGetBool (PcdCpuProcTracePerformanceCollecting);
 
   return ConfigData;
 }
@@ -112,6 +115,7 @@ ProcTraceSupport (
   PROC_TRACE_DATA                              *ProcTraceData;
   CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_EBX  Ebx;
   CPUID_INTEL_PROCESSOR_TRACE_MAIN_LEAF_ECX    Ecx;
+  CPUID_INTEL_PROCESSOR_TRACE_MAIN_LEAF_EBX    MainLeafEbx;
 
   //
   // Check if ProcTraceMemorySize option is enabled (0xFF means disable by user)
@@ -141,6 +145,12 @@ ProcTraceSupport (
     ProcTraceData->ProcessorData[ProcessorNumber].RtitCtrl.Uint64           = AsmReadMsr64 (MSR_IA32_RTIT_CTL);
     ProcTraceData->ProcessorData[ProcessorNumber].RtitOutputBase.Uint64     = AsmReadMsr64 (MSR_IA32_RTIT_OUTPUT_BASE);
     ProcTraceData->ProcessorData[ProcessorNumber].RtitOutputMaskPtrs.Uint64 = AsmReadMsr64 (MSR_IA32_RTIT_OUTPUT_MASK_PTRS);
+
+    if (ProcTraceData->EnablePerformanceCollecting) {
+      AsmCpuidEx (CPUID_INTEL_PROCESSOR_TRACE, CPUID_INTEL_PROCESSOR_TRACE_MAIN_LEAF, NULL, &MainLeafEbx.Uint32, NULL, NULL);
+      ProcTraceData->ProcessorData[ProcessorNumber].CycPacketSupported = (BOOLEAN)(MainLeafEbx.Bits.ConfigurablePsb == 1);
+    }
+
     return TRUE;
   }
 
@@ -511,6 +521,22 @@ ProcTraceInitialize (
   CtrlReg.Bits.User     = 1;
   CtrlReg.Bits.BranchEn = 1;
   CtrlReg.Bits.TraceEn  = 1;
+
+  //
+  // Generate CYC/TSC timing packets to to collect performance data.
+  //
+  if (ProcTraceData->EnablePerformanceCollecting) {
+    if (ProcTraceData->ProcessorData[ProcessorNumber].CycPacketSupported) {
+      CtrlReg.Bits.CYCEn     = 1;
+      CtrlReg.Bits.CYCThresh = 5;
+    }
+
+    //
+    // Write to TSCEn is always supported
+    //
+    CtrlReg.Bits.TSCEn = 1;
+  }
+
   CPU_REGISTER_TABLE_WRITE64 (
     ProcessorNumber,
     Msr,
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 14dadfbf53..b7a46e3235 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -429,5 +429,13 @@
   # @Prompt Enable CPU processor trace only on BSP.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceBspOnly|FALSE|BOOLEAN|0x60000019
 
+  ## This PCD indicates if enable performance collecting when CPU processor trace is enabled.<BR><BR>
+  #  CYC/TSC timing packets will be generated to collect performance data if this PCD is TRUE.
+  #  This PCD is ignored if CPU processor trace is disabled.<BR><BR>
+  #  TRUE  - Performance collecting will be enabled in processor trace.<BR>
+  #  FASLE - Performance collecting will be disabled in processor trace.<BR>
+  # @Prompt Enable performance collecting when processor trace is enabled.
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTracePerformanceCollecting|FALSE|BOOLEAN|0x60000020
+
 [UserExtensions.TianoCore."ExtraFiles"]
   UefiCpuPkgExtra.uni
-- 
2.39.1.windows.1

Re: [edk2-devel] [Patch V2 1/2] UefiCpuPkg: Update code to support enable ProcTrace only on BSP

[Ni, Ray]

> +    if (ProcTraceData->EnableOnBspOnly) {

1. can you please add comments here to remind reader that
    this is also the first and only time ProcTraceInitialize() runs?
    Similar comments in the next chunk code.

> +      MemRegionBaseAddr = (UINTN)AllocateAlignedReservedPages (Pages,
> Alignment);
> +      if (MemRegionBaseAddr == 0) {
> +        //
> +        // Could not allocate for BSP even
> +        //
> +        DEBUG ((DEBUG_ERROR, "ProcTrace: Out of mem, failed to allocate
> buffer for BSP\n"));
> +        return RETURN_OUT_OF_RESOURCES;
> +      } 

Re: [Patch V2 2/2] UefiCpuPkg: Update PT code to support enable collect performance

[Ni, Ray]

> @@ -112,6 +115,7 @@ ProcTraceSupport (
>    PROC_TRACE_DATA                              *ProcTraceData;
>    CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_EBX  Ebx;
>    CPUID_INTEL_PROCESSOR_TRACE_MAIN_LEAF_ECX    Ecx;
> +  CPUID_INTEL_PROCESSOR_TRACE_MAIN_LEAF_EBX    MainLeafEbx;

1. can you update the "Ecx" to "ProcTraceEcx", and "MainLeafEbx" to "ProcTraceEbx"?

> 
>    //
>    // Check if ProcTraceMemorySize option is enabled (0xFF means disable by
> user)
> @@ -141,6 +145,12 @@ ProcTraceSupport (
>      ProcTraceData->ProcessorData[ProcessorNumber].RtitCtrl.Uint64           =
> AsmReadMsr64 (MSR_IA32_RTIT_CTL);
>      ProcTraceData->ProcessorData[ProcessorNumber].RtitOutputBase.Uint64
> = AsmReadMsr64 (MSR_IA32_RTIT_OUTPUT_BASE);
>      ProcTraceData-
> >ProcessorData[ProcessorNumber].RtitOutputMaskPtrs.Uint64 =
> AsmReadMsr64 (MSR_IA32_RTIT_OUTPUT_MASK_PTRS);
> +
> +    if (ProcTraceData->EnablePerformanceCollecting) {
> +      AsmCpuidEx (CPUID_INTEL_PROCESSOR_TRACE,
> CPUID_INTEL_PROCESSOR_TRACE_MAIN_LEAF, NULL, &MainLeafEbx.Uint32,
> NULL, NULL);

2. There is an existing Cpuid call earlier. Can you get the "EBX" value in the existing
     Cpuid call? And you don't even need to check "EnablePerformanceCollecting" here
     for the capability detection.

Re: [Patch V2 2/2] UefiCpuPkg: Update PT code to support enable collect performance

[duntan]

Thanks for the comments. I'll update the code in next version patch.

Thanks,
Dun

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Tuesday, April 25, 2023 10:14 PM
To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Chen, Xiao X <xiao.x.chen@intel.com>
Subject: RE: [Patch V2 2/2] UefiCpuPkg: Update PT code to support enable collect performance

> @@ -112,6 +115,7 @@ ProcTraceSupport (
>    PROC_TRACE_DATA                              *ProcTraceData;
>    CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_EBX  Ebx;
>    CPUID_INTEL_PROCESSOR_TRACE_MAIN_LEAF_ECX    Ecx;
> +  CPUID_INTEL_PROCESSOR_TRACE_MAIN_LEAF_EBX    MainLeafEbx;

1. can you update the "Ecx" to "ProcTraceEcx", and "MainLeafEbx" to "ProcTraceEbx"?

> 
>    //
>    // Check if ProcTraceMemorySize option is enabled (0xFF means 
> disable by
> user)
> @@ -141,6 +145,12 @@ ProcTraceSupport (
>      ProcTraceData->ProcessorData[ProcessorNumber].RtitCtrl.Uint64           =
> AsmReadMsr64 (MSR_IA32_RTIT_CTL);
>      
> ProcTraceData->ProcessorData[ProcessorNumber].RtitOutputBase.Uint64
> = AsmReadMsr64 (MSR_IA32_RTIT_OUTPUT_BASE);
>      ProcTraceData-
> >ProcessorData[ProcessorNumber].RtitOutputMaskPtrs.Uint64 =
> AsmReadMsr64 (MSR_IA32_RTIT_OUTPUT_MASK_PTRS);
> +
> +    if (ProcTraceData->EnablePerformanceCollecting) {
> +      AsmCpuidEx (CPUID_INTEL_PROCESSOR_TRACE,
> CPUID_INTEL_PROCESSOR_TRACE_MAIN_LEAF, NULL, &MainLeafEbx.Uint32, 
> NULL, NULL);

2. There is an existing Cpuid call earlier. Can you get the "EBX" value in the existing
     Cpuid call? And you don't even need to check "EnablePerformanceCollecting" here
     for the capability detection.

Re: [edk2-devel] [Patch V2 1/2] UefiCpuPkg: Update code to support enable ProcTrace only on BSP

[duntan]

Thanks for the comments. I'll add comments to explain it.

Thanks,
Dun

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Tuesday, April 25, 2023 10:10 PM
To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Chen, Xiao X <xiao.x.chen@intel.com>
Subject: RE: [edk2-devel] [Patch V2 1/2] UefiCpuPkg: Update code to support enable ProcTrace only on BSP

> +    if (ProcTraceData->EnableOnBspOnly) {

1. can you please add comments here to remind reader that
    this is also the first and only time ProcTraceInitialize() runs?
    Similar comments in the next chunk code.

> +      MemRegionBaseAddr = (UINTN)AllocateAlignedReservedPages (Pages,
> Alignment);
> +      if (MemRegionBaseAddr == 0) {
> +        //
> +        // Could not allocate for BSP even
> +        //
> +        DEBUG ((DEBUG_ERROR, "ProcTrace: Out of mem, failed to allocate
> buffer for BSP\n"));
> +        return RETURN_OUT_OF_RESOURCES;
> +      }