* [PATCH 0/2] Fix GP fault issue and enhance code about ProcTrace @ 2019-06-01 8:03 Zeng, Star 2019-06-01 8:03 ` [PATCH 1/2] UefiCpuPkg CpuCommFeaturesLib: Disable TraceEn at the beginning Zeng, Star 2019-06-01 8:03 ` [PATCH 2/2] UefiCpuPkg CpuCommFeaturesLib: Fix GP fault issue about ProcTrace Zeng, Star 0 siblings, 2 replies; 4+ messages in thread From: Zeng, Star @ 2019-06-01 8:03 UTC (permalink / raw) Cc: Star Zeng, Laszlo Ersek, Eric Dong, Ruiyu Ni, Chandana Kumar, Kevin Li This patch series mainly fixes GP fault issue about ProcTrace for https://bugzilla.tianocore.org/show_bug.cgi?id=1808. It aslo enhances code about ProcTrace for https://bugzilla.tianocore.org/show_bug.cgi?id=1809. Please check the individual patches for detail. 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> Star Zeng (2): UefiCpuPkg CpuCommFeaturesLib: Disable TraceEn at the beginning UefiCpuPkg CpuCommFeaturesLib: Fix GP fault issue about ProcTrace .../Library/CpuCommonFeaturesLib/ProcTrace.c | 104 +++++++++--------- 1 file changed, 51 insertions(+), 53 deletions(-) -- 2.21.0.windows.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] UefiCpuPkg CpuCommFeaturesLib: Disable TraceEn at the beginning 2019-06-01 8:03 [PATCH 0/2] Fix GP fault issue and enhance code about ProcTrace Zeng, Star @ 2019-06-01 8:03 ` Zeng, Star 2019-06-01 8:03 ` [PATCH 2/2] UefiCpuPkg CpuCommFeaturesLib: Fix GP fault issue about ProcTrace Zeng, Star 1 sibling, 0 replies; 4+ messages in thread From: Zeng, Star @ 2019-06-01 8:03 UTC (permalink / raw) Cc: Star Zeng, Laszlo Ersek, Eric Dong, Ruiyu Ni, Chandana Kumar, Kevin Li BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1809 Current code disables TraceEn at the end of ProcTraceInitialize(), then there will be much memory allocated even when ProcTrace feature is disabled. This patch updates code to disable TraceEn and return at the beginning of ProcTraceInitialize() when when ProcTrace feature is disabled. 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 | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c index da6f6beb9477..b98eb116b7b6 100644 --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c @@ -199,22 +199,6 @@ ProcTraceInitialize ( ProcTraceData = (PROC_TRACE_DATA *) ConfigData; ASSERT (ProcTraceData != NULL); - MemRegionBaseAddr = 0; - FirstIn = FALSE; - - if (ProcTraceData->ThreadMemRegionTable == NULL) { - FirstIn = TRUE; - DEBUG ((DEBUG_INFO, "Initialize Processor Trace\n")); - } - - /// - /// Refer to PROC_TRACE_MEM_SIZE Table for Size Encoding - /// - MemRegionSize = (UINT32) (1 << (ProcTraceData->ProcTraceMemSize + 12)); - if (FirstIn) { - DEBUG ((DEBUG_INFO, "ProcTrace: MemSize requested: 0x%X \n", MemRegionSize)); - } - // // Clear MSR_IA32_RTIT_CTL[0] and IA32_RTIT_STS only if MSR_IA32_RTIT_CTL[0]==1b // @@ -243,6 +227,26 @@ ProcTraceInitialize ( ); } + if (!State) { + return RETURN_SUCCESS; + } + + MemRegionBaseAddr = 0; + FirstIn = FALSE; + + if (ProcTraceData->ThreadMemRegionTable == NULL) { + FirstIn = TRUE; + DEBUG ((DEBUG_INFO, "Initialize Processor Trace\n")); + } + + /// + /// Refer to PROC_TRACE_MEM_SIZE Table for Size Encoding + /// + MemRegionSize = (UINT32) (1 << (ProcTraceData->ProcTraceMemSize + 12)); + if (FirstIn) { + DEBUG ((DEBUG_INFO, "ProcTrace: MemSize requested: 0x%X \n", MemRegionSize)); + } + if (FirstIn) { // // Let BSP allocate and create the necessary memory region (Aligned to the size of @@ -459,11 +463,7 @@ ProcTraceInitialize ( CtrlReg.Bits.OS = 1; CtrlReg.Bits.User = 1; CtrlReg.Bits.BranchEn = 1; - if (!State) { - CtrlReg.Bits.TraceEn = 0; - } else { - CtrlReg.Bits.TraceEn = 1; - } + CtrlReg.Bits.TraceEn = 1; CPU_REGISTER_TABLE_WRITE64 ( ProcessorNumber, Msr, -- 2.21.0.windows.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] UefiCpuPkg CpuCommFeaturesLib: Fix GP fault issue about ProcTrace 2019-06-01 8:03 [PATCH 0/2] Fix GP fault issue and enhance code about ProcTrace Zeng, Star 2019-06-01 8:03 ` [PATCH 1/2] UefiCpuPkg CpuCommFeaturesLib: Disable TraceEn at the beginning Zeng, Star @ 2019-06-01 8:03 ` Zeng, Star 1 sibling, 0 replies; 4+ messages in thread From: Zeng, Star @ 2019-06-01 8:03 UTC (permalink / raw) Cc: Star Zeng, Laszlo Ersek, Eric Dong, Ruiyu Ni, Chandana Kumar, Kevin Li 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 0/2] Fix GP fault issue and enhance code about ProcTrace @ 2019-06-01 8:08 Zeng, Star 2019-06-01 8:08 ` [PATCH 2/2] UefiCpuPkg CpuCommFeaturesLib: Fix GP fault issue " Zeng, Star 0 siblings, 1 reply; 4+ messages in thread From: Zeng, Star @ 2019-06-01 8:08 UTC (permalink / raw) Cc: Star Zeng, Laszlo Ersek, Eric Dong, Ruiyu Ni, Chandana Kumar, Kevin Li This patch series mainly fixes GP fault issue about ProcTrace for https://bugzilla.tianocore.org/show_bug.cgi?id=1808. It aslo enhances code about ProcTrace for https://bugzilla.tianocore.org/show_bug.cgi?id=1809. Please check the individual patches for detail. 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> Star Zeng (2): UefiCpuPkg CpuCommFeaturesLib: Disable TraceEn at the beginning UefiCpuPkg CpuCommFeaturesLib: Fix GP fault issue about ProcTrace .../Library/CpuCommonFeaturesLib/ProcTrace.c | 104 +++++++++--------- 1 file changed, 51 insertions(+), 53 deletions(-) -- 2.21.0.windows.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/2] UefiCpuPkg CpuCommFeaturesLib: Fix GP fault issue about ProcTrace 2019-06-01 8:08 [PATCH 0/2] Fix GP fault issue and enhance code " Zeng, Star @ 2019-06-01 8:08 ` Zeng, Star 0 siblings, 0 replies; 4+ messages in thread From: Zeng, Star @ 2019-06-01 8:08 UTC (permalink / raw) Cc: Star Zeng, Laszlo Ersek, Eric Dong, Ruiyu Ni, Chandana Kumar, Kevin Li 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-06-01 8:08 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-06-01 8:03 [PATCH 0/2] Fix GP fault issue and enhance code about ProcTrace Zeng, Star 2019-06-01 8:03 ` [PATCH 1/2] UefiCpuPkg CpuCommFeaturesLib: Disable TraceEn at the beginning Zeng, Star 2019-06-01 8:03 ` [PATCH 2/2] UefiCpuPkg CpuCommFeaturesLib: Fix GP fault issue about ProcTrace Zeng, Star -- strict thread matches above, loose matches on Subject: below -- 2019-06-01 8:08 [PATCH 0/2] Fix GP fault issue and enhance code " Zeng, Star 2019-06-01 8:08 ` [PATCH 2/2] UefiCpuPkg CpuCommFeaturesLib: Fix GP fault issue " Zeng, Star
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox