[PATCH 2/2] UefiCpuPkg CpuCommFeaturesLib: Fix GP fault issue about ProcTrace

["Zeng, Star" <star.zeng@intel.com>]

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1808

In current code, the values of TopaEntryPtr->Uint64 for TopaTable
and the values of OutputBaseReg.Uint64 and OutputMaskPtrsReg.Uint64
to register table write for RTIT_OUTPUT_BASE and RTIT_OUTPUT_MASK_PTRS
are not been initialized in whole. For example, the reserved bits in
OutputBaseReg.Uint64 are random that will cause GP fault like below
when SetProcessorRegister (in CpuFeaturesInitialize.c) sets register
based on register table.

!!!! X64 Exception Type - 0D(#GP - General Protection)
  CPU Apic ID - 00000000 !!!!
ExceptionData - 0000000000000000
RIP  -0000000064D69576, CS  -0000000000000038, RFLAGS -0000000000010246
RAX  -000000006B9F1001, RCX -0000000000000560, RDX -0000000000000000
RBX  -0000000064EECA18, RSP -000000006CB82BA0, RBP -0000000000000008
RSI  -0000000080000000, RDI -0000000000000011
R8   -000000006B9493D0, R9  -0000000000000010, R10 -00000000000000FF
R11  -000000006CB82A50, R12 -0000000064D70F50, R13 -0000000066547050
R14  -0000000064E3E198, R15 -0000000000000000
DS   -0000000000000030, ES  -0000000000000030, FS  -0000000000000030
GS   -0000000000000030, SS  -0000000000000030
CR0  -0000000080010013, CR2 -0000000000000000, CR3 -000000006C601000
CR4  -0000000000000628, CR8 -0000000000000000
DR0  -0000000000000000, DR1 -0000000000000000, DR2 -0000000000000000
DR3  -0000000000000000, DR6 -00000000FFFF0FF0, DR7 -0000000000000400
GDTR -000000006B8CCF18 0000000000000047, LDTR -0000000000000000
IDTR -000000006687E018 0000000000000FFF,   TR -0000000000000000
FXSAVE_STATE -000000006CB82800

And current code gets MSR_IA32_RTIT_CTL, MSR_IA32_RTIT_OUTPUT_BASE and
MSR_IA32_RTIT_OUTPUT_MASK_PTRS in ProcTraceInitialize() and uses their
values for all processors. But ProcTraceInitialize() is only executed
by BSP, that means the values just for BSP. For good practice, the code
should get MSR_IA32_RTIT_CTL, MSR_IA32_RTIT_OUTPUT_BASE and
MSR_IA32_RTIT_OUTPUT_MASK_PTRS in ProcTraceSupport (executed by all
processors), and then use them in ProcTraceInitialize() for all
processors. This can also resolve the issue that the values of
OutputBaseReg.Uint64 and OutputMaskPtrsReg.Uint64 are not been
initialized in whole.

For TopaEntryPtr->Uint64, this patch updates code to initialize it
in whole explicitly by TopaEntryPtr->Uint64 = 0 before updating its
fields.

At the same time, this patch also eliminates the ProcTraceSupported
field in PROC_TRACE_PROCESSOR_DATA and the TopaMemArrayCount field in
PROC_TRACE_DATA.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Cc: Kevin Li <kevin.y.li@intel.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 .../Library/CpuCommonFeaturesLib/ProcTrace.c  | 62 +++++++++----------
 1 file changed, 30 insertions(+), 32 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
index b98eb116b7b6..611459105d38 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
@@ -29,9 +29,11 @@ typedef enum {
 } RTIT_OUTPUT_SCHEME;
 
 typedef struct  {
-  BOOLEAN  ProcTraceSupported;
-  BOOLEAN  TopaSupported;
-  BOOLEAN  SingleRangeSupported;
+  BOOLEAN                                   TopaSupported;
+  BOOLEAN                                   SingleRangeSupported;
+  MSR_IA32_RTIT_CTL_REGISTER                RtitCtrl;
+  MSR_IA32_RTIT_OUTPUT_BASE_REGISTER        RtitOutputBase;
+  MSR_IA32_RTIT_OUTPUT_MASK_PTRS_REGISTER   RtitOutputMaskPtrs;
 } PROC_TRACE_PROCESSOR_DATA;
 
 typedef struct  {
@@ -44,7 +46,6 @@ typedef struct  {
   UINTN                       AllocatedThreads;
 
   UINTN                       *TopaMemArray;
-  UINTN                       TopaMemArrayCount;
 
   PROC_TRACE_PROCESSOR_DATA   *ProcessorData;
 } PROC_TRACE_DATA;
@@ -124,8 +125,7 @@ ProcTraceSupport (
   // Check if Processor Trace is supported
   //
   AsmCpuidEx (CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS, 0, NULL, &Ebx.Uint32, NULL, NULL);
-  ProcTraceData->ProcessorData[ProcessorNumber].ProcTraceSupported = (BOOLEAN) (Ebx.Bits.IntelProcessorTrace == 1);
-  if (!ProcTraceData->ProcessorData[ProcessorNumber].ProcTraceSupported) {
+  if (Ebx.Bits.IntelProcessorTrace == 0) {
     return FALSE;
   }
 
@@ -134,6 +134,9 @@ ProcTraceSupport (
   ProcTraceData->ProcessorData[ProcessorNumber].SingleRangeSupported = (BOOLEAN) (Ecx.Bits.SingleRangeOutput == 1);
   if ((ProcTraceData->ProcessorData[ProcessorNumber].TopaSupported && (ProcTraceData->ProcTraceOutputScheme == RtitOutputSchemeToPA)) ||
       (ProcTraceData->ProcessorData[ProcessorNumber].SingleRangeSupported && (ProcTraceData->ProcTraceOutputScheme == RtitOutputSchemeSingleRange))) {
+    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);
     return TRUE;
   }
 
@@ -202,7 +205,7 @@ ProcTraceInitialize (
   //
   // Clear MSR_IA32_RTIT_CTL[0] and IA32_RTIT_STS only if MSR_IA32_RTIT_CTL[0]==1b
   //
-  CtrlReg.Uint64 = AsmReadMsr64 (MSR_IA32_RTIT_CTL);
+  CtrlReg.Uint64 = ProcTraceData->ProcessorData[ProcessorNumber].RtitCtrl.Uint64;
   if (CtrlReg.Bits.TraceEn != 0) {
     ///
     /// Clear bit 0 in MSR IA32_RTIT_CTL (570)
@@ -251,9 +254,9 @@ ProcTraceInitialize (
     //
     //   Let BSP allocate and create the necessary memory region (Aligned to the size of
     //   the memory region from setup option(ProcTraceMemSize) which is an integral multiple of 4kB)
-    //   for the all the enabled threads for storing Processor Trace debug data. Then Configure the trace
+    //   for all the enabled threads to store Processor Trace debug data. Then Configure the trace
     //   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 clear.
+    //   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) {
@@ -284,13 +287,12 @@ ProcTraceInitialize (
     }
 
     DEBUG ((DEBUG_INFO, "ProcTrace: Allocated PT mem for %d thread \n", ProcTraceData->AllocatedThreads));
-    MemRegionBaseAddr = ThreadMemRegionTable[0];
+  }
+
+  if (ProcessorNumber < ProcTraceData->AllocatedThreads) {
+    MemRegionBaseAddr = ProcTraceData->ThreadMemRegionTable[ProcessorNumber];
   } else {
-    if (ProcessorNumber < ProcTraceData->AllocatedThreads) {
-      MemRegionBaseAddr = ProcTraceData->ThreadMemRegionTable[ProcessorNumber];
-    } else {
-      return RETURN_SUCCESS;
-    }
+    return RETURN_SUCCESS;
   }
 
   ///
@@ -309,7 +311,6 @@ ProcTraceInitialize (
     //
     // Clear MSR IA32_RTIT_CTL (0x570) ToPA (Bit 8)
     //
-    CtrlReg.Uint64 = AsmReadMsr64 (MSR_IA32_RTIT_CTL);
     CtrlReg.Bits.ToPA = 0;
     CPU_REGISTER_TABLE_WRITE64 (
       ProcessorNumber,
@@ -321,6 +322,7 @@ ProcTraceInitialize (
     //
     // Program MSR IA32_RTIT_OUTPUT_BASE (0x560) bits[63:7] with the allocated Memory Region
     //
+    OutputBaseReg.Uint64 = ProcTraceData->ProcessorData[ProcessorNumber].RtitOutputBase.Uint64;
     OutputBaseReg.Bits.Base = (MemRegionBaseAddr >> 7) & 0x01FFFFFF;
     OutputBaseReg.Bits.BaseHi = RShiftU64 ((UINT64) MemRegionBaseAddr, 32) & 0xFFFFFFFF;
     CPU_REGISTER_TABLE_WRITE64 (
@@ -333,6 +335,7 @@ ProcTraceInitialize (
     //
     // Program the Mask bits for the Memory Region to MSR IA32_RTIT_OUTPUT_MASK_PTRS (561h)
     //
+    OutputMaskPtrsReg.Uint64 = ProcTraceData->ProcessorData[ProcessorNumber].RtitOutputMaskPtrs.Uint64;
     OutputMaskPtrsReg.Bits.MaskOrTableOffset = ((MemRegionSize - 1) >> 7) & 0x01FFFFFF;
     OutputMaskPtrsReg.Bits.OutputOffset = RShiftU64 (MemRegionSize - 1, 32) & 0xFFFFFFFF;
     CPU_REGISTER_TABLE_WRITE64 (
@@ -376,10 +379,10 @@ ProcTraceInitialize (
           if (Index < ProcTraceData->AllocatedThreads) {
             ProcTraceData->AllocatedThreads = Index;
           }
-          DEBUG ((DEBUG_ERROR, "ProcTrace:  Out of mem, allocating ToPA mem only for %d threads\n", ProcTraceData->AllocatedThreads));
+          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
+            // Could not allocate for BSP even
             //
             FreePool ((VOID *) TopaMemArray);
             TopaMemArray = NULL;
@@ -393,29 +396,24 @@ ProcTraceInitialize (
       }
 
       DEBUG ((DEBUG_INFO, "ProcTrace: Allocated ToPA mem for %d thread \n", ProcTraceData->AllocatedThreads));
-      //
-      // BSP gets the first block
-      //
-      TopaTableBaseAddr = TopaMemArray[0];
+    }
+
+    if (ProcessorNumber < ProcTraceData->AllocatedThreads) {
+      TopaTableBaseAddr = ProcTraceData->TopaMemArray[ProcessorNumber];
     } else {
-      //
-      // Count for currently executing AP.
-      //
-      if (ProcessorNumber < ProcTraceData->AllocatedThreads) {
-        TopaTableBaseAddr = ProcTraceData->TopaMemArray[ProcessorNumber];
-      } else {
-        return RETURN_SUCCESS;
-      }
+      return RETURN_SUCCESS;
     }
 
     TopaTable = (PROC_TRACE_TOPA_TABLE *) TopaTableBaseAddr;
     TopaEntryPtr = &TopaTable->TopaEntry[0];
+    TopaEntryPtr->Uint64 = 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->Uint64 = 0;
     TopaEntryPtr->Bits.Base = (TopaTableBaseAddr >> 12) & 0x000FFFFF;
     TopaEntryPtr->Bits.BaseHi = RShiftU64 ((UINT64) TopaTableBaseAddr, 32) & 0xFFFFFFFF;
     TopaEntryPtr->Bits.END = 1;
@@ -423,6 +421,7 @@ ProcTraceInitialize (
     //
     // Program the MSR IA32_RTIT_OUTPUT_BASE (0x560) bits[63:7] with ToPA base
     //
+    OutputBaseReg.Uint64 = ProcTraceData->ProcessorData[ProcessorNumber].RtitOutputBase.Uint64;
     OutputBaseReg.Bits.Base = (TopaTableBaseAddr >> 7) & 0x01FFFFFF;
     OutputBaseReg.Bits.BaseHi = RShiftU64 ((UINT64) TopaTableBaseAddr, 32) & 0xFFFFFFFF;
     CPU_REGISTER_TABLE_WRITE64 (
@@ -435,6 +434,7 @@ ProcTraceInitialize (
     //
     // Set the MSR IA32_RTIT_OUTPUT_MASK (0x561) bits[63:7] to 0
     //
+    OutputMaskPtrsReg.Uint64 = ProcTraceData->ProcessorData[ProcessorNumber].RtitOutputMaskPtrs.Uint64;
     OutputMaskPtrsReg.Bits.MaskOrTableOffset = 0;
     OutputMaskPtrsReg.Bits.OutputOffset = 0;
     CPU_REGISTER_TABLE_WRITE64 (
@@ -446,7 +446,6 @@ ProcTraceInitialize (
     //
     // Enable ToPA output scheme by enabling MSR IA32_RTIT_CTL (0x570) ToPA (Bit 8)
     //
-    CtrlReg.Uint64 = AsmReadMsr64 (MSR_IA32_RTIT_CTL);
     CtrlReg.Bits.ToPA = 1;
     CPU_REGISTER_TABLE_WRITE64 (
       ProcessorNumber,
@@ -459,7 +458,6 @@ ProcTraceInitialize (
   ///
   /// Enable the Processor Trace feature from MSR IA32_RTIT_CTL (570h)
   ///
-  CtrlReg.Uint64 = AsmReadMsr64 (MSR_IA32_RTIT_CTL);
   CtrlReg.Bits.OS = 1;
   CtrlReg.Bits.User = 1;
   CtrlReg.Bits.BranchEn = 1;
-- 
2.21.0.windows.1