Skip to content

Commit

Permalink
UefiCpuPkg CpuCommFeaturesLib: Fix GP fault issue about ProcTrace
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Cc: Eric Dong <[email protected]>
Cc: Ruiyu Ni <[email protected]>
Cc: Chandana Kumar <[email protected]>
Cc: Kevin Li <[email protected]>
Signed-off-by: Star Zeng <[email protected]>
Reviewed-by: Eric Dong <[email protected]>
  • Loading branch information
@lzeng14
lzeng14 committed Jun 6, 2019
1 parent 484dc05 commit 49fb605
Showing 1 changed file with 30 additions and 32 deletions.
62 changes: 30 additions & 32 deletions UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -44,7 +46,6 @@ typedef struct {
UINTN AllocatedThreads;

UINTN *TopaMemArray;
UINTN TopaMemArrayCount;

PROC_TRACE_PROCESSOR_DATA *ProcessorData;
} PROC_TRACE_DATA;
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}

///
Expand All @@ -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,
Expand All @@ -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 (
Expand All @@ -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 (
Expand Down Expand Up @@ -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;
Expand All @@ -393,36 +396,32 @@ 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;

//
// 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 (
Expand All @@ -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 (
Expand All @@ -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,
Expand All @@ -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;
Expand Down

0 comments on commit 49fb605

Please sign in to comment.