From bb767506b265b1cdbf0dbec58c9a3f4c6be6ab2b Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 24 Nov 2016 20:49:43 +0100 Subject: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: handle dynamic PcdCpuMaxLogicalProcessorNumber "UefiCpuPkg/UefiCpuPkg.dec" already allows platforms to make PcdCpuMaxLogicalProcessorNumber dynamic, however PiSmmCpuDxeSmm does not take this into account everywhere. As soon as a platform turns the PCD into a dynamic one, at least S3 fails. When the PCD is dynamic, all PcdGet() calls translate into PCD DXE protocol calls, which are only permitted at boot time, not at runtime or during S3 resume. We already have a variable called mMaxNumberOfCpus; it is initialized in the entry point function like this: > // > // If support CPU hot plug, we need to allocate resources for possibly > // hot-added processors > // > if (FeaturePcdGet (PcdCpuHotPlugSupport)) { > mMaxNumberOfCpus = PcdGet32 (PcdCpuMaxLogicalProcessorNumber); > } else { > mMaxNumberOfCpus = mNumberOfCpus; > } There's another use of the PCD a bit higher up, also in the entry point function: > // > // Use MP Services Protocol to retrieve the number of processors and > // number of enabled processors > // > Status = MpServices->GetNumberOfProcessors (MpServices, &mNumberOfCpus, > &NumberOfEnabledProcessors); > ASSERT_EFI_ERROR (Status); > ASSERT (mNumberOfCpus <= PcdGet32 (PcdCpuMaxLogicalProcessorNumber)); Preserve these calls in the entry point function, and replace all other uses of PcdCpuMaxLogicalProcessorNumber -- there are only reads -- with mMaxNumberOfCpus. For PcdCpuHotPlugSupport==TRUE, this is an unobservable change. For PcdCpuHotPlugSupport==FALSE, we even save SMRAM, because we no longer allocate resources needlessly for CPUs that can never appear in the system. PcdCpuMaxLogicalProcessorNumber is also retrieved in "UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c", but only in the library instance constructor, which runs even before the entry point function is called. Cc: Igor Mammedov Cc: Jeff Fan Cc: Jordan Justen Cc: Michael Kinney Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=116 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek Reviewed-by: Jeff Fan --- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 2 +- UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c index 4baba1e9f8..f957de1f47 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c @@ -344,7 +344,7 @@ SmmInitHandler ( AsmWriteIdtr (&gcSmiIdtr); ApicId = GetApicId (); - ASSERT (mNumberOfCpus <= PcdGet32 (PcdCpuMaxLogicalProcessorNumber)); + ASSERT (mNumberOfCpus <= mMaxNumberOfCpus); for (Index = 0; Index < mNumberOfCpus; Index++) { if (ApicId == (UINT32)gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId) { diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c index c1a48d100e..f53819ee24 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c @@ -139,7 +139,7 @@ GetCpuIndex ( ApicId = GetApicId (); - for (Index = 0; Index < PcdGet32 (PcdCpuMaxLogicalProcessorNumber); Index++) { + for (Index = 0; Index < mMaxNumberOfCpus; Index++) { if (gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId == ApicId) { return Index; } @@ -825,13 +825,13 @@ InitSmmProfileInternal ( UINTN MsrDsAreaSizePerCpu; UINTN TotalSize; - mPFEntryCount = (UINTN *)AllocateZeroPool (sizeof (UINTN) * PcdGet32 (PcdCpuMaxLogicalProcessorNumber)); + mPFEntryCount = (UINTN *)AllocateZeroPool (sizeof (UINTN) * mMaxNumberOfCpus); ASSERT (mPFEntryCount != NULL); mLastPFEntryValue = (UINT64 (*)[MAX_PF_ENTRY_COUNT])AllocateZeroPool ( - sizeof (mLastPFEntryValue[0]) * PcdGet32 (PcdCpuMaxLogicalProcessorNumber)); + sizeof (mLastPFEntryValue[0]) * mMaxNumberOfCpus); ASSERT (mLastPFEntryValue != NULL); mLastPFEntryPointer = (UINT64 *(*)[MAX_PF_ENTRY_COUNT])AllocateZeroPool ( - sizeof (mLastPFEntryPointer[0]) * PcdGet32 (PcdCpuMaxLogicalProcessorNumber)); + sizeof (mLastPFEntryPointer[0]) * mMaxNumberOfCpus); ASSERT (mLastPFEntryPointer != NULL); // @@ -872,17 +872,17 @@ InitSmmProfileInternal ( mSmmProfileBase->NumCpus = gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus; if (mBtsSupported) { - mMsrDsArea = (MSR_DS_AREA_STRUCT **)AllocateZeroPool (sizeof (MSR_DS_AREA_STRUCT *) * PcdGet32 (PcdCpuMaxLogicalProcessorNumber)); + mMsrDsArea = (MSR_DS_AREA_STRUCT **)AllocateZeroPool (sizeof (MSR_DS_AREA_STRUCT *) * mMaxNumberOfCpus); ASSERT (mMsrDsArea != NULL); - mMsrBTSRecord = (BRANCH_TRACE_RECORD **)AllocateZeroPool (sizeof (BRANCH_TRACE_RECORD *) * PcdGet32 (PcdCpuMaxLogicalProcessorNumber)); + mMsrBTSRecord = (BRANCH_TRACE_RECORD **)AllocateZeroPool (sizeof (BRANCH_TRACE_RECORD *) * mMaxNumberOfCpus); ASSERT (mMsrBTSRecord != NULL); - mMsrPEBSRecord = (PEBS_RECORD **)AllocateZeroPool (sizeof (PEBS_RECORD *) * PcdGet32 (PcdCpuMaxLogicalProcessorNumber)); + mMsrPEBSRecord = (PEBS_RECORD **)AllocateZeroPool (sizeof (PEBS_RECORD *) * mMaxNumberOfCpus); ASSERT (mMsrPEBSRecord != NULL); mMsrDsAreaBase = (MSR_DS_AREA_STRUCT *)((UINTN)Base + mSmmProfileSize); - MsrDsAreaSizePerCpu = mMsrDsAreaSize / PcdGet32 (PcdCpuMaxLogicalProcessorNumber); + MsrDsAreaSizePerCpu = mMsrDsAreaSize / mMaxNumberOfCpus; mBTSRecordNumber = (MsrDsAreaSizePerCpu - sizeof(PEBS_RECORD) * PEBS_RECORD_NUMBER - sizeof(MSR_DS_AREA_STRUCT)) / sizeof(BRANCH_TRACE_RECORD); - for (Index = 0; Index < PcdGet32 (PcdCpuMaxLogicalProcessorNumber); Index++) { + for (Index = 0; Index < mMaxNumberOfCpus; Index++) { mMsrDsArea[Index] = (MSR_DS_AREA_STRUCT *)((UINTN)mMsrDsAreaBase + MsrDsAreaSizePerCpu * Index); mMsrBTSRecord[Index] = (BRANCH_TRACE_RECORD *)((UINTN)mMsrDsArea[Index] + sizeof(MSR_DS_AREA_STRUCT)); mMsrPEBSRecord[Index] = (PEBS_RECORD *)((UINTN)mMsrDsArea[Index] + MsrDsAreaSizePerCpu - sizeof(PEBS_RECORD) * PEBS_RECORD_NUMBER); -- 2.39.2