Reviewed-by: Ray Ni Thanks, Ray ________________________________ From: Tan, Dun Sent: Friday, May 10, 2024 18:08 To: devel@edk2.groups.io Cc: Ni, Ray ; Laszlo Ersek ; Kumar, Rahul R ; Gerd Hoffmann ; Wu, Jiaxin Subject: [PATCH 17/18] UefiCpuPkg: Remove GetAcpiCpuData() in CpuS3.c Remove GetAcpiCpuData() in CpuS3.c. The mAcpiCpuData is not needed in S3 boot anymore. Signed-off-by: Dun Tan Cc: Ray Ni Cc: Laszlo Ersek Cc: Rahul Kumar Cc: Gerd Hoffmann Cc: Jiaxin Wu --- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 243 +-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 6 ++---- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 24 ------------------------ 3 files changed, 3 insertions(+), 270 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c index e84bc14de0..78ecf4efc6 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c @@ -9,22 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include "PiSmmCpuDxeSmm.h" #include -// -// Flags used when program the register. -// -typedef struct { - volatile UINTN MemoryMappedLock; // Spinlock used to program mmio - volatile UINT32 *CoreSemaphoreCount; // Semaphore container used to program - // core level semaphore. - volatile UINT32 *PackageSemaphoreCount; // Semaphore container used to program - // package level semaphore. -} PROGRAM_CPU_REGISTER_FLAGS; - -#define LEGACY_REGION_SIZE (2 * 0x1000) -#define LEGACY_REGION_BASE (0xA0000 - LEGACY_REGION_SIZE) - -ACPI_CPU_DATA mAcpiCpuData; -BOOLEAN mRestoreSmmConfigurationInS3 = FALSE; +BOOLEAN mRestoreSmmConfigurationInS3 = FALSE; // // S3 boot flag @@ -266,232 +251,6 @@ InitSmmS3ResumeState ( } } -/** - Copy register table from non-SMRAM into SMRAM. - - @param[in] DestinationRegisterTableList Points to destination register table. - @param[in] SourceRegisterTableList Points to source register table. - @param[in] NumberOfCpus Number of CPUs. - -**/ -VOID -CopyRegisterTable ( - IN CPU_REGISTER_TABLE *DestinationRegisterTableList, - IN CPU_REGISTER_TABLE *SourceRegisterTableList, - IN UINT32 NumberOfCpus - ) -{ - UINTN Index; - CPU_REGISTER_TABLE_ENTRY *RegisterTableEntry; - - CopyMem (DestinationRegisterTableList, SourceRegisterTableList, NumberOfCpus * sizeof (CPU_REGISTER_TABLE)); - for (Index = 0; Index < NumberOfCpus; Index++) { - if (DestinationRegisterTableList[Index].TableLength != 0) { - DestinationRegisterTableList[Index].AllocatedSize = DestinationRegisterTableList[Index].TableLength * sizeof (CPU_REGISTER_TABLE_ENTRY); - RegisterTableEntry = AllocateCopyPool ( - DestinationRegisterTableList[Index].AllocatedSize, - (VOID *)(UINTN)SourceRegisterTableList[Index].RegisterTableEntry - ); - ASSERT (RegisterTableEntry != NULL); - DestinationRegisterTableList[Index].RegisterTableEntry = (EFI_PHYSICAL_ADDRESS)(UINTN)RegisterTableEntry; - } - } -} - -/** - Check whether the register table is empty or not. - - @param[in] RegisterTable Point to the register table. - @param[in] NumberOfCpus Number of CPUs. - - @retval TRUE The register table is empty. - @retval FALSE The register table is not empty. -**/ -BOOLEAN -IsRegisterTableEmpty ( - IN CPU_REGISTER_TABLE *RegisterTable, - IN UINT32 NumberOfCpus - ) -{ - UINTN Index; - - if (RegisterTable != NULL) { - for (Index = 0; Index < NumberOfCpus; Index++) { - if (RegisterTable[Index].TableLength != 0) { - return FALSE; - } - } - } - - return TRUE; -} - -/** - Copy the data used to initialize processor register into SMRAM. - - @param[in,out] CpuFeatureInitDataDst Pointer to the destination CPU_FEATURE_INIT_DATA structure. - @param[in] CpuFeatureInitDataSrc Pointer to the source CPU_FEATURE_INIT_DATA structure. - -**/ -VOID -CopyCpuFeatureInitDatatoSmram ( - IN OUT CPU_FEATURE_INIT_DATA *CpuFeatureInitDataDst, - IN CPU_FEATURE_INIT_DATA *CpuFeatureInitDataSrc - ) -{ - CPU_STATUS_INFORMATION *CpuStatus; - - if (!IsRegisterTableEmpty ((CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataSrc->PreSmmInitRegisterTable, mAcpiCpuData.NumberOfCpus)) { - CpuFeatureInitDataDst->PreSmmInitRegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (mAcpiCpuData.NumberOfCpus * sizeof (CPU_REGISTER_TABLE)); - ASSERT (CpuFeatureInitDataDst->PreSmmInitRegisterTable != 0); - - CopyRegisterTable ( - (CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataDst->PreSmmInitRegisterTable, - (CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataSrc->PreSmmInitRegisterTable, - mAcpiCpuData.NumberOfCpus - ); - } - - if (!IsRegisterTableEmpty ((CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataSrc->RegisterTable, mAcpiCpuData.NumberOfCpus)) { - CpuFeatureInitDataDst->RegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (mAcpiCpuData.NumberOfCpus * sizeof (CPU_REGISTER_TABLE)); - ASSERT (CpuFeatureInitDataDst->RegisterTable != 0); - - CopyRegisterTable ( - (CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataDst->RegisterTable, - (CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataSrc->RegisterTable, - mAcpiCpuData.NumberOfCpus - ); - } - - CpuStatus = &CpuFeatureInitDataDst->CpuStatus; - CopyMem (CpuStatus, &CpuFeatureInitDataSrc->CpuStatus, sizeof (CPU_STATUS_INFORMATION)); - - if (CpuFeatureInitDataSrc->CpuStatus.ThreadCountPerPackage != 0) { - CpuStatus->ThreadCountPerPackage = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateCopyPool ( - sizeof (UINT32) * CpuStatus->PackageCount, - (UINT32 *)(UINTN)CpuFeatureInitDataSrc->CpuStatus.ThreadCountPerPackage - ); - ASSERT (CpuStatus->ThreadCountPerPackage != 0); - } - - if (CpuFeatureInitDataSrc->CpuStatus.ThreadCountPerCore != 0) { - CpuStatus->ThreadCountPerCore = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateCopyPool ( - sizeof (UINT8) * (CpuStatus->PackageCount * CpuStatus->MaxCoreCount), - (UINT32 *)(UINTN)CpuFeatureInitDataSrc->CpuStatus.ThreadCountPerCore - ); - ASSERT (CpuStatus->ThreadCountPerCore != 0); - } - - if (CpuFeatureInitDataSrc->ApLocation != 0) { - CpuFeatureInitDataDst->ApLocation = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateCopyPool ( - mAcpiCpuData.NumberOfCpus * sizeof (EFI_CPU_PHYSICAL_LOCATION), - (EFI_CPU_PHYSICAL_LOCATION *)(UINTN)CpuFeatureInitDataSrc->ApLocation - ); - ASSERT (CpuFeatureInitDataDst->ApLocation != 0); - } -} - -/** - Get ACPI CPU data. - -**/ -VOID -GetAcpiCpuData ( - VOID - ) -{ - ACPI_CPU_DATA *AcpiCpuData; - IA32_DESCRIPTOR *Gdtr; - IA32_DESCRIPTOR *Idtr; - VOID *GdtForAp; - VOID *IdtForAp; - VOID *MachineCheckHandlerForAp; - CPU_STATUS_INFORMATION *CpuStatus; - - if (!mAcpiS3Enable) { - return; - } - - // - // Prevent use of mAcpiCpuData by initialize NumberOfCpus to 0 - // - mAcpiCpuData.NumberOfCpus = 0; - - // - // If PcdCpuS3DataAddress was never set, then do not copy CPU S3 Data into SMRAM - // - AcpiCpuData = (ACPI_CPU_DATA *)(UINTN)PcdGet64 (PcdCpuS3DataAddress); - if (AcpiCpuData == 0) { - return; - } - - // - // For a native platform, copy the CPU S3 data into SMRAM for use on CPU S3 Resume. - // - CopyMem (&mAcpiCpuData, AcpiCpuData, sizeof (mAcpiCpuData)); - - mAcpiCpuData.MtrrTable = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (sizeof (MTRR_SETTINGS)); - ASSERT (mAcpiCpuData.MtrrTable != 0); - - CopyMem ((VOID *)(UINTN)mAcpiCpuData.MtrrTable, (VOID *)(UINTN)AcpiCpuData->MtrrTable, sizeof (MTRR_SETTINGS)); - - mAcpiCpuData.GdtrProfile = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (sizeof (IA32_DESCRIPTOR)); - ASSERT (mAcpiCpuData.GdtrProfile != 0); - - CopyMem ((VOID *)(UINTN)mAcpiCpuData.GdtrProfile, (VOID *)(UINTN)AcpiCpuData->GdtrProfile, sizeof (IA32_DESCRIPTOR)); - - mAcpiCpuData.IdtrProfile = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (sizeof (IA32_DESCRIPTOR)); - ASSERT (mAcpiCpuData.IdtrProfile != 0); - - CopyMem ((VOID *)(UINTN)mAcpiCpuData.IdtrProfile, (VOID *)(UINTN)AcpiCpuData->IdtrProfile, sizeof (IA32_DESCRIPTOR)); - - // - // Copy AP's GDT, IDT and Machine Check handler into SMRAM. - // - Gdtr = (IA32_DESCRIPTOR *)(UINTN)mAcpiCpuData.GdtrProfile; - Idtr = (IA32_DESCRIPTOR *)(UINTN)mAcpiCpuData.IdtrProfile; - - GdtForAp = AllocatePool ((Gdtr->Limit + 1) + (Idtr->Limit + 1) + mAcpiCpuData.ApMachineCheckHandlerSize); - ASSERT (GdtForAp != NULL); - IdtForAp = (VOID *)((UINTN)GdtForAp + (Gdtr->Limit + 1)); - MachineCheckHandlerForAp = (VOID *)((UINTN)IdtForAp + (Idtr->Limit + 1)); - - CopyMem (GdtForAp, (VOID *)Gdtr->Base, Gdtr->Limit + 1); - CopyMem (IdtForAp, (VOID *)Idtr->Base, Idtr->Limit + 1); - CopyMem (MachineCheckHandlerForAp, (VOID *)(UINTN)mAcpiCpuData.ApMachineCheckHandlerBase, mAcpiCpuData.ApMachineCheckHandlerSize); - - Gdtr->Base = (UINTN)GdtForAp; - Idtr->Base = (UINTN)IdtForAp; - mAcpiCpuData.ApMachineCheckHandlerBase = (EFI_PHYSICAL_ADDRESS)(UINTN)MachineCheckHandlerForAp; - - ZeroMem (&mAcpiCpuData.CpuFeatureInitData, sizeof (CPU_FEATURE_INIT_DATA)); - - if (!PcdGetBool (PcdCpuFeaturesInitOnS3Resume)) { - // - // If the CPU features will not be initialized by CpuFeaturesPei module during - // next ACPI S3 resume, copy the CPU features initialization data into SMRAM, - // which will be consumed in SmmRestoreCpu during next S3 resume. - // - CopyCpuFeatureInitDatatoSmram (&mAcpiCpuData.CpuFeatureInitData, &AcpiCpuData->CpuFeatureInitData); - - CpuStatus = &mAcpiCpuData.CpuFeatureInitData.CpuStatus; - - mCpuFlags.CoreSemaphoreCount = AllocateZeroPool ( - sizeof (UINT32) * CpuStatus->PackageCount * - CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount - ); - ASSERT (mCpuFlags.CoreSemaphoreCount != NULL); - - mCpuFlags.PackageSemaphoreCount = AllocateZeroPool ( - sizeof (UINT32) * CpuStatus->PackageCount * - CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount - ); - ASSERT (mCpuFlags.PackageSemaphoreCount != NULL); - - InitializeSpinLock ((SPIN_LOCK *)&mCpuFlags.MemoryMappedLock); - } -} - /** Get ACPI S3 enable flag. diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c index e400bee8d5..f5e113d99f 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c @@ -435,8 +435,8 @@ ExecuteFirstSmiInit ( /** SMM Ready To Lock event notification handler. - The CPU S3 data is copied to SMRAM for security and mSmmReadyToLock is set to - perform additional lock actions that must be performed from SMM on the next SMI. + mSmmReadyToLock is set to perform additional lock actions that must be + performed from SMM on the next SMI. @param[in] Protocol Points to the protocol's unique identifier. @param[in] Interface Points to the interface instance. @@ -452,8 +452,6 @@ SmmReadyToLockEventNotify ( IN EFI_HANDLE Handle ) { - GetAcpiCpuData (); - // // Cache a copy of UEFI memory map before we start profiling feature. // diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h index f42910ddf1..af0fda4da1 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h @@ -1048,15 +1048,6 @@ InitSmmS3ResumeState ( IN UINT32 Cr3 ); -/** - Get ACPI CPU data. - -**/ -VOID -GetAcpiCpuData ( - VOID - ); - /** Restore SMM Configuration in S3 boot path. @@ -1075,21 +1066,6 @@ GetAcpiS3EnableFlag ( VOID ); -/** - Transfer AP to safe hlt-loop after it finished restore CPU features on S3 patch. - - @param[in] ApHltLoopCode The address of the safe hlt-loop function. - @param[in] TopOfStack A pointer to the new stack to use for the ApHltLoopCode. - @param[in] NumberToFinishAddress Address of Semaphore of APs finish count. - -**/ -VOID -TransferApToSafeState ( - IN UINTN ApHltLoopCode, - IN UINTN TopOfStack, - IN UINTN NumberToFinishAddress - ); - /** Set ShadowStack memory. -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118848): https://edk2.groups.io/g/devel/message/118848 Mute This Topic: https://groups.io/mt/106018144/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-