* [edk2-devel] [Patch V3 0/5] Use MpService2Ppi to wakeup CPU in Smm CpuS3 @ 2023-08-30 7:34 duntan 2023-08-30 7:34 ` [edk2-devel] [Patch V3 4/5] UefiCpuPkg/PiSmmCpuDxe: code refinement for CpuS3.c duntan 2023-08-30 7:34 ` [edk2-devel] [Patch V3 5/5] UefiCpuPkg/PiSmmCpuDxe: use MpService2Ppi to wakeup AP in s3 duntan 0 siblings, 2 replies; 5+ messages in thread From: duntan @ 2023-08-30 7:34 UTC (permalink / raw) To: devel In V3 patch set: 1.Adjusted the order of some code in InitializeCpuBeforeRebase/After to make the code easier to understand 2.Remove the function cast of InitializeCpuProcedure. Only the 'code refinement for CpuS3.c' and 'use MpService2Ppi to wakeup AP in s3' patches are modified in V3 patch set. Dun Tan (5): MdeModulePkg: add MpService2Ppi field in SMM_S3_RESUME_STATE UefiCpuPkg/S3Resume2Pei: prepare MpService2Ppi in S3Resume UefiCpuPkg/S3Resume2Pei: assert for invalid excution mode combo UefiCpuPkg/PiSmmCpuDxe: code refinement for CpuS3.c UefiCpuPkg/PiSmmCpuDxe: use MpService2Ppi to wakeup AP in s3 MdeModulePkg/Include/Guid/AcpiS3Context.h | 3 ++- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 292 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------------------------------------------------------------------------------------------------------- UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 25 ++++++++++++++++++++++++- UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf | 3 ++- 4 files changed, 185 insertions(+), 138 deletions(-) -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#108115): https://edk2.groups.io/g/devel/message/108115 Mute This Topic: https://groups.io/mt/101047980/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 5+ messages in thread
* [edk2-devel] [Patch V3 4/5] UefiCpuPkg/PiSmmCpuDxe: code refinement for CpuS3.c 2023-08-30 7:34 [edk2-devel] [Patch V3 0/5] Use MpService2Ppi to wakeup CPU in Smm CpuS3 duntan @ 2023-08-30 7:34 ` duntan 2023-09-06 5:19 ` Ni, Ray 2023-08-30 7:34 ` [edk2-devel] [Patch V3 5/5] UefiCpuPkg/PiSmmCpuDxe: use MpService2Ppi to wakeup AP in s3 duntan 1 sibling, 1 reply; 5+ messages in thread From: duntan @ 2023-08-30 7:34 UTC (permalink / raw) To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar This commit is code logic refinement for s3 boot flow in CpuS3.c. It doesn't change any code functionality. This commit implementes InitializeAp and InitializeBsp as a single function since they are doing almost the same thing. Then both BSP and AP will execute the same function InitializeCpuProcedure to do CPU initialization. This can make the code logic easier to understand. Signed-off-by: Dun Tan <dun.tan@intel.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> --- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 266 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------------------------------------------------------------------------------------- 1 file changed, 138 insertions(+), 128 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c index 0f7ee0372d..b9dbeaef87 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c @@ -539,44 +539,149 @@ SetRegister ( } /** - AP initialization before then after SMBASE relocation in the S3 boot path. + The function is invoked before SMBASE relocation in S3 path to restores CPU status. + + The function is invoked before SMBASE relocation in S3 path. It does first time microcode load + and restores MTRRs for both BSP and APs. + + @param IsBsp The CPU this function executes on is BSP or not. + **/ VOID -InitializeAp ( - VOID +InitializeCpuBeforeRebase ( + IN BOOLEAN IsBsp ) { - UINTN TopOfStack; - UINT8 Stack[128]; - LoadMtrrData (mAcpiCpuData.MtrrTable); SetRegister (TRUE); + ProgramVirtualWireMode (); + if (!IsBsp) { + DisableLvtInterrupts (); + } + // // Count down the number with lock mechanism. // InterlockedDecrement (&mNumberToFinish); + if (IsBsp) { + // + // Bsp wait here till all AP finish the initialization before rebase + // + while (mNumberToFinish > 0) { + CpuPause (); + } + } +} + +/** + The function is invoked after SMBASE relocation in S3 path to restores CPU status. + + The function is invoked after SMBASE relocation in S3 path. It restores configuration according to + data saved by normal boot path for both BSP and APs. + + @param IsBsp The CPU this function executes on is BSP or not. + +**/ +VOID +InitializeCpuAfterRebase ( + IN BOOLEAN IsBsp + ) +{ + UINTN TopOfStack; + UINT8 Stack[128]; + + SetRegister (FALSE); + if (IsBsp) { + while (mNumberToFinish > 0) { + CpuPause (); + } + } else { + // + // Place AP into the safe code, count down the number with lock mechanism in the safe code. + // + TopOfStack = (UINTN)Stack + sizeof (Stack); + TopOfStack &= ~(UINTN)(CPU_STACK_ALIGNMENT - 1); + CopyMem ((VOID *)(UINTN)mApHltLoopCode, mApHltLoopCodeTemplate, sizeof (mApHltLoopCodeTemplate)); + TransferApToSafeState ((UINTN)mApHltLoopCode, TopOfStack, (UINTN)&mNumberToFinish); + } +} + +/** + Cpu initialization procedure. + + @param[in,out] Buffer The pointer to private data buffer. + +**/ +VOID +EFIAPI +InitializeCpuProcedure ( + IN OUT VOID *Buffer + ) +{ + BOOLEAN IsBsp; + + IsBsp = (BOOLEAN)(mBspApicId == GetApicId ()); + // - // Wait for BSP to signal SMM Base relocation done. + // Skip initialization if mAcpiCpuData is not valid // - while (!mInitApsAfterSmmBaseReloc) { - CpuPause (); + if (mAcpiCpuData.NumberOfCpus > 0) { + // + // First time microcode load and restore MTRRs + // + InitializeCpuBeforeRebase (IsBsp); } - ProgramVirtualWireMode (); - DisableLvtInterrupts (); + if (IsBsp) { + DEBUG ((DEBUG_INFO, "SmmRestoreCpu: mSmmRelocated is %d\n", mSmmRelocated)); - SetRegister (FALSE); + // + // Check whether Smm Relocation is done or not. + // If not, will do the SmmBases Relocation here!!! + // + if (!mSmmRelocated) { + // + // Restore SMBASE for BSP and all APs + // + SmmRelocateBases (); + } else { + // + // Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute first SMI init. + // + ExecuteFirstSmiInit (); + } + } // - // Place AP into the safe code, count down the number with lock mechanism in the safe code. + // Skip initialization if mAcpiCpuData is not valid // - TopOfStack = (UINTN)Stack + sizeof (Stack); - TopOfStack &= ~(UINTN)(CPU_STACK_ALIGNMENT - 1); - CopyMem ((VOID *)(UINTN)mApHltLoopCode, mApHltLoopCodeTemplate, sizeof (mApHltLoopCodeTemplate)); - TransferApToSafeState ((UINTN)mApHltLoopCode, TopOfStack, (UINTN)&mNumberToFinish); + if (mAcpiCpuData.NumberOfCpus > 0) { + if (IsBsp) { + // + // mNumberToFinish should be set before AP executes InitializeCpuAfterRebase() + // + mNumberToFinish = (UINT32)(mNumberOfCpus - 1); + // + // Signal that SMM base relocation is complete and to continue initialization for all APs. + // + mInitApsAfterSmmBaseReloc = TRUE; + } else { + // + // AP Wait for BSP to signal SMM Base relocation done. + // + while (!mInitApsAfterSmmBaseReloc) { + CpuPause (); + } + } + + // + // Restore MSRs for BSP and all APs + // + InitializeCpuAfterRebase (IsBsp); + } } /** @@ -627,91 +732,7 @@ PrepareApStartupVector ( mExchangeInfo->BufferStart = (UINT32)StartupVector; mExchangeInfo->Cr3 = (UINT32)(AsmReadCr3 ()); mExchangeInfo->InitializeFloatingPointUnitsAddress = (UINTN)InitializeFloatingPointUnits; -} - -/** - The function is invoked before SMBASE relocation in S3 path to restores CPU status. - - The function is invoked before SMBASE relocation in S3 path. It does first time microcode load - and restores MTRRs for both BSP and APs. - -**/ -VOID -InitializeCpuBeforeRebase ( - VOID - ) -{ - LoadMtrrData (mAcpiCpuData.MtrrTable); - - SetRegister (TRUE); - - ProgramVirtualWireMode (); - - PrepareApStartupVector (mAcpiCpuData.StartupVector); - - if (FeaturePcdGet (PcdCpuHotPlugSupport)) { - ASSERT (mNumberOfCpus <= mAcpiCpuData.NumberOfCpus); - } else { - ASSERT (mNumberOfCpus == mAcpiCpuData.NumberOfCpus); - } - - mNumberToFinish = (UINT32)(mNumberOfCpus - 1); - mExchangeInfo->ApFunction = (VOID *)(UINTN)InitializeAp; - - // - // Execute code for before SmmBaseReloc. Note: This flag is maintained across S3 boots. - // - mInitApsAfterSmmBaseReloc = FALSE; - - // - // Send INIT IPI - SIPI to all APs - // - SendInitSipiSipiAllExcludingSelf ((UINT32)mAcpiCpuData.StartupVector); - - while (mNumberToFinish > 0) { - CpuPause (); - } -} - -/** - The function is invoked after SMBASE relocation in S3 path to restores CPU status. - - The function is invoked after SMBASE relocation in S3 path. It restores configuration according to - data saved by normal boot path for both BSP and APs. - -**/ -VOID -InitializeCpuAfterRebase ( - VOID - ) -{ - if (FeaturePcdGet (PcdCpuHotPlugSupport)) { - ASSERT (mNumberOfCpus <= mAcpiCpuData.NumberOfCpus); - } else { - ASSERT (mNumberOfCpus == mAcpiCpuData.NumberOfCpus); - } - - mNumberToFinish = (UINT32)(mNumberOfCpus - 1); - - // - // Signal that SMM base relocation is complete and to continue initialization for all APs. - // - mInitApsAfterSmmBaseReloc = TRUE; - - // - // Must begin set register after all APs have continue their initialization. - // This is a requirement to support semaphore mechanism in register table. - // Because if semaphore's dependence type is package type, semaphore will wait - // for all Aps in one package finishing their tasks before set next register - // for all APs. If the Aps not begin its task during BSP doing its task, the - // BSP thread will hang because it is waiting for other Aps in the same - // package finishing their task. - // - SetRegister (FALSE); - - while (mNumberToFinish > 0) { - CpuPause (); - } + mExchangeInfo->ApFunction = (VOID *)(UINTN)InitializeCpuProcedure; } /** @@ -813,44 +834,33 @@ SmmRestoreCpu ( InitializeDebugAgent (DEBUG_AGENT_INIT_THUNK_PEI_IA32TOX64, (VOID *)&Ia32Idtr, NULL); } + mBspApicId = GetApicId (); // - // Skip initialization if mAcpiCpuData is not valid + // Skip AP initialization if mAcpiCpuData is not valid // if (mAcpiCpuData.NumberOfCpus > 0) { - // - // First time microcode load and restore MTRRs - // - InitializeCpuBeforeRebase (); - } + if (FeaturePcdGet (PcdCpuHotPlugSupport)) { + ASSERT (mNumberOfCpus <= mAcpiCpuData.NumberOfCpus); + } else { + ASSERT (mNumberOfCpus == mAcpiCpuData.NumberOfCpus); + } - DEBUG ((DEBUG_INFO, "SmmRestoreCpu: mSmmRelocated is %d\n", mSmmRelocated)); + mNumberToFinish = (UINT32)mNumberOfCpus; - // - // Check whether Smm Relocation is done or not. - // If not, will do the SmmBases Relocation here!!! - // - if (!mSmmRelocated) { // - // Restore SMBASE for BSP and all APs + // Execute code for before SmmBaseReloc. Note: This flag is maintained across S3 boots. // - SmmRelocateBases (); - } else { - // - // Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute first SMI init. - // - ExecuteFirstSmiInit (); - } + mInitApsAfterSmmBaseReloc = FALSE; - // - // Skip initialization if mAcpiCpuData is not valid - // - if (mAcpiCpuData.NumberOfCpus > 0) { + PrepareApStartupVector (mAcpiCpuData.StartupVector); // - // Restore MSRs for BSP and all APs + // Send INIT IPI - SIPI to all APs // - InitializeCpuAfterRebase (); + SendInitSipiSipiAllExcludingSelf ((UINT32)mAcpiCpuData.StartupVector); } + InitializeCpuProcedure (NULL); + // // Set a flag to restore SMM configuration in S3 path. // -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#108116): https://edk2.groups.io/g/devel/message/108116 Mute This Topic: https://groups.io/mt/101047981/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [Patch V3 4/5] UefiCpuPkg/PiSmmCpuDxe: code refinement for CpuS3.c 2023-08-30 7:34 ` [edk2-devel] [Patch V3 4/5] UefiCpuPkg/PiSmmCpuDxe: code refinement for CpuS3.c duntan @ 2023-09-06 5:19 ` Ni, Ray 0 siblings, 0 replies; 5+ messages in thread From: Ni, Ray @ 2023-09-06 5:19 UTC (permalink / raw) To: Tan, Dun, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R [-- Attachment #1: Type: text/plain, Size: 11089 bytes --] Reviewed-by: Ray Ni <ray.ni@intel.com> Thanks, Ray ________________________________ From: Tan, Dun <dun.tan@intel.com> Sent: Wednesday, August 30, 2023 3:34 PM To: devel@edk2.groups.io <devel@edk2.groups.io> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com> Subject: [Patch V3 4/5] UefiCpuPkg/PiSmmCpuDxe: code refinement for CpuS3.c This commit is code logic refinement for s3 boot flow in CpuS3.c. It doesn't change any code functionality. This commit implementes InitializeAp and InitializeBsp as a single function since they are doing almost the same thing. Then both BSP and AP will execute the same function InitializeCpuProcedure to do CPU initialization. This can make the code logic easier to understand. Signed-off-by: Dun Tan <dun.tan@intel.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> --- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 266 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------------------------------------------------------------------------------------- 1 file changed, 138 insertions(+), 128 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c index 0f7ee0372d..b9dbeaef87 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c @@ -539,44 +539,149 @@ SetRegister ( } /** - AP initialization before then after SMBASE relocation in the S3 boot path. + The function is invoked before SMBASE relocation in S3 path to restores CPU status. + + The function is invoked before SMBASE relocation in S3 path. It does first time microcode load + and restores MTRRs for both BSP and APs. + + @param IsBsp The CPU this function executes on is BSP or not. + **/ VOID -InitializeAp ( - VOID +InitializeCpuBeforeRebase ( + IN BOOLEAN IsBsp ) { - UINTN TopOfStack; - UINT8 Stack[128]; - LoadMtrrData (mAcpiCpuData.MtrrTable); SetRegister (TRUE); + ProgramVirtualWireMode (); + if (!IsBsp) { + DisableLvtInterrupts (); + } + // // Count down the number with lock mechanism. // InterlockedDecrement (&mNumberToFinish); + if (IsBsp) { + // + // Bsp wait here till all AP finish the initialization before rebase + // + while (mNumberToFinish > 0) { + CpuPause (); + } + } +} + +/** + The function is invoked after SMBASE relocation in S3 path to restores CPU status. + + The function is invoked after SMBASE relocation in S3 path. It restores configuration according to + data saved by normal boot path for both BSP and APs. + + @param IsBsp The CPU this function executes on is BSP or not. + +**/ +VOID +InitializeCpuAfterRebase ( + IN BOOLEAN IsBsp + ) +{ + UINTN TopOfStack; + UINT8 Stack[128]; + + SetRegister (FALSE); + if (IsBsp) { + while (mNumberToFinish > 0) { + CpuPause (); + } + } else { + // + // Place AP into the safe code, count down the number with lock mechanism in the safe code. + // + TopOfStack = (UINTN)Stack + sizeof (Stack); + TopOfStack &= ~(UINTN)(CPU_STACK_ALIGNMENT - 1); + CopyMem ((VOID *)(UINTN)mApHltLoopCode, mApHltLoopCodeTemplate, sizeof (mApHltLoopCodeTemplate)); + TransferApToSafeState ((UINTN)mApHltLoopCode, TopOfStack, (UINTN)&mNumberToFinish); + } +} + +/** + Cpu initialization procedure. + + @param[in,out] Buffer The pointer to private data buffer. + +**/ +VOID +EFIAPI +InitializeCpuProcedure ( + IN OUT VOID *Buffer + ) +{ + BOOLEAN IsBsp; + + IsBsp = (BOOLEAN)(mBspApicId == GetApicId ()); + // - // Wait for BSP to signal SMM Base relocation done. + // Skip initialization if mAcpiCpuData is not valid // - while (!mInitApsAfterSmmBaseReloc) { - CpuPause (); + if (mAcpiCpuData.NumberOfCpus > 0) { + // + // First time microcode load and restore MTRRs + // + InitializeCpuBeforeRebase (IsBsp); } - ProgramVirtualWireMode (); - DisableLvtInterrupts (); + if (IsBsp) { + DEBUG ((DEBUG_INFO, "SmmRestoreCpu: mSmmRelocated is %d\n", mSmmRelocated)); - SetRegister (FALSE); + // + // Check whether Smm Relocation is done or not. + // If not, will do the SmmBases Relocation here!!! + // + if (!mSmmRelocated) { + // + // Restore SMBASE for BSP and all APs + // + SmmRelocateBases (); + } else { + // + // Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute first SMI init. + // + ExecuteFirstSmiInit (); + } + } // - // Place AP into the safe code, count down the number with lock mechanism in the safe code. + // Skip initialization if mAcpiCpuData is not valid // - TopOfStack = (UINTN)Stack + sizeof (Stack); - TopOfStack &= ~(UINTN)(CPU_STACK_ALIGNMENT - 1); - CopyMem ((VOID *)(UINTN)mApHltLoopCode, mApHltLoopCodeTemplate, sizeof (mApHltLoopCodeTemplate)); - TransferApToSafeState ((UINTN)mApHltLoopCode, TopOfStack, (UINTN)&mNumberToFinish); + if (mAcpiCpuData.NumberOfCpus > 0) { + if (IsBsp) { + // + // mNumberToFinish should be set before AP executes InitializeCpuAfterRebase() + // + mNumberToFinish = (UINT32)(mNumberOfCpus - 1); + // + // Signal that SMM base relocation is complete and to continue initialization for all APs. + // + mInitApsAfterSmmBaseReloc = TRUE; + } else { + // + // AP Wait for BSP to signal SMM Base relocation done. + // + while (!mInitApsAfterSmmBaseReloc) { + CpuPause (); + } + } + + // + // Restore MSRs for BSP and all APs + // + InitializeCpuAfterRebase (IsBsp); + } } /** @@ -627,91 +732,7 @@ PrepareApStartupVector ( mExchangeInfo->BufferStart = (UINT32)StartupVector; mExchangeInfo->Cr3 = (UINT32)(AsmReadCr3 ()); mExchangeInfo->InitializeFloatingPointUnitsAddress = (UINTN)InitializeFloatingPointUnits; -} - -/** - The function is invoked before SMBASE relocation in S3 path to restores CPU status. - - The function is invoked before SMBASE relocation in S3 path. It does first time microcode load - and restores MTRRs for both BSP and APs. - -**/ -VOID -InitializeCpuBeforeRebase ( - VOID - ) -{ - LoadMtrrData (mAcpiCpuData.MtrrTable); - - SetRegister (TRUE); - - ProgramVirtualWireMode (); - - PrepareApStartupVector (mAcpiCpuData.StartupVector); - - if (FeaturePcdGet (PcdCpuHotPlugSupport)) { - ASSERT (mNumberOfCpus <= mAcpiCpuData.NumberOfCpus); - } else { - ASSERT (mNumberOfCpus == mAcpiCpuData.NumberOfCpus); - } - - mNumberToFinish = (UINT32)(mNumberOfCpus - 1); - mExchangeInfo->ApFunction = (VOID *)(UINTN)InitializeAp; - - // - // Execute code for before SmmBaseReloc. Note: This flag is maintained across S3 boots. - // - mInitApsAfterSmmBaseReloc = FALSE; - - // - // Send INIT IPI - SIPI to all APs - // - SendInitSipiSipiAllExcludingSelf ((UINT32)mAcpiCpuData.StartupVector); - - while (mNumberToFinish > 0) { - CpuPause (); - } -} - -/** - The function is invoked after SMBASE relocation in S3 path to restores CPU status. - - The function is invoked after SMBASE relocation in S3 path. It restores configuration according to - data saved by normal boot path for both BSP and APs. - -**/ -VOID -InitializeCpuAfterRebase ( - VOID - ) -{ - if (FeaturePcdGet (PcdCpuHotPlugSupport)) { - ASSERT (mNumberOfCpus <= mAcpiCpuData.NumberOfCpus); - } else { - ASSERT (mNumberOfCpus == mAcpiCpuData.NumberOfCpus); - } - - mNumberToFinish = (UINT32)(mNumberOfCpus - 1); - - // - // Signal that SMM base relocation is complete and to continue initialization for all APs. - // - mInitApsAfterSmmBaseReloc = TRUE; - - // - // Must begin set register after all APs have continue their initialization. - // This is a requirement to support semaphore mechanism in register table. - // Because if semaphore's dependence type is package type, semaphore will wait - // for all Aps in one package finishing their tasks before set next register - // for all APs. If the Aps not begin its task during BSP doing its task, the - // BSP thread will hang because it is waiting for other Aps in the same - // package finishing their task. - // - SetRegister (FALSE); - - while (mNumberToFinish > 0) { - CpuPause (); - } + mExchangeInfo->ApFunction = (VOID *)(UINTN)InitializeCpuProcedure; } /** @@ -813,44 +834,33 @@ SmmRestoreCpu ( InitializeDebugAgent (DEBUG_AGENT_INIT_THUNK_PEI_IA32TOX64, (VOID *)&Ia32Idtr, NULL); } + mBspApicId = GetApicId (); // - // Skip initialization if mAcpiCpuData is not valid + // Skip AP initialization if mAcpiCpuData is not valid // if (mAcpiCpuData.NumberOfCpus > 0) { - // - // First time microcode load and restore MTRRs - // - InitializeCpuBeforeRebase (); - } + if (FeaturePcdGet (PcdCpuHotPlugSupport)) { + ASSERT (mNumberOfCpus <= mAcpiCpuData.NumberOfCpus); + } else { + ASSERT (mNumberOfCpus == mAcpiCpuData.NumberOfCpus); + } - DEBUG ((DEBUG_INFO, "SmmRestoreCpu: mSmmRelocated is %d\n", mSmmRelocated)); + mNumberToFinish = (UINT32)mNumberOfCpus; - // - // Check whether Smm Relocation is done or not. - // If not, will do the SmmBases Relocation here!!! - // - if (!mSmmRelocated) { // - // Restore SMBASE for BSP and all APs + // Execute code for before SmmBaseReloc. Note: This flag is maintained across S3 boots. // - SmmRelocateBases (); - } else { - // - // Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execute first SMI init. - // - ExecuteFirstSmiInit (); - } + mInitApsAfterSmmBaseReloc = FALSE; - // - // Skip initialization if mAcpiCpuData is not valid - // - if (mAcpiCpuData.NumberOfCpus > 0) { + PrepareApStartupVector (mAcpiCpuData.StartupVector); // - // Restore MSRs for BSP and all APs + // Send INIT IPI - SIPI to all APs // - InitializeCpuAfterRebase (); + SendInitSipiSipiAllExcludingSelf ((UINT32)mAcpiCpuData.StartupVector); } + InitializeCpuProcedure (NULL); + // // Set a flag to restore SMM configuration in S3 path. // -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#108303): https://edk2.groups.io/g/devel/message/108303 Mute This Topic: https://groups.io/mt/101047981/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- [-- Attachment #2: Type: text/html, Size: 17726 bytes --] ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [edk2-devel] [Patch V3 5/5] UefiCpuPkg/PiSmmCpuDxe: use MpService2Ppi to wakeup AP in s3 2023-08-30 7:34 [edk2-devel] [Patch V3 0/5] Use MpService2Ppi to wakeup CPU in Smm CpuS3 duntan 2023-08-30 7:34 ` [edk2-devel] [Patch V3 4/5] UefiCpuPkg/PiSmmCpuDxe: code refinement for CpuS3.c duntan @ 2023-08-30 7:34 ` duntan 2023-09-06 5:19 ` Ni, Ray 1 sibling, 1 reply; 5+ messages in thread From: duntan @ 2023-08-30 7:34 UTC (permalink / raw) To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar Use MpService2Ppi to wakeup AP in s3 boot flow during initializing CPU. If mSmmS3ResumeState->MpService2Ppi is not 0, then BSP will use MpService2Ppi->StartupAllCPUs to do CPU initialization for both BSP and AP instead of only sending InitSipiSipi for AP. Signed-off-by: Dun Tan <dun.tan@intel.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> --- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 58 +++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c index b9dbeaef87..0bae0e33f1 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c @@ -7,6 +7,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent **/ #include "PiSmmCpuDxeSmm.h" +#include <PiPei.h> +#include <Ppi/MpServices2.h> #pragma pack(1) typedef struct { @@ -594,18 +596,21 @@ InitializeCpuAfterRebase ( UINT8 Stack[128]; SetRegister (FALSE); - if (IsBsp) { - while (mNumberToFinish > 0) { - CpuPause (); + + if (mSmmS3ResumeState->MpService2Ppi == 0) { + if (IsBsp) { + while (mNumberToFinish > 0) { + CpuPause (); + } + } else { + // + // Place AP into the safe code, count down the number with lock mechanism in the safe code. + // + TopOfStack = (UINTN)Stack + sizeof (Stack); + TopOfStack &= ~(UINTN)(CPU_STACK_ALIGNMENT - 1); + CopyMem ((VOID *)(UINTN)mApHltLoopCode, mApHltLoopCodeTemplate, sizeof (mApHltLoopCodeTemplate)); + TransferApToSafeState ((UINTN)mApHltLoopCode, TopOfStack, (UINTN)&mNumberToFinish); } - } else { - // - // Place AP into the safe code, count down the number with lock mechanism in the safe code. - // - TopOfStack = (UINTN)Stack + sizeof (Stack); - TopOfStack &= ~(UINTN)(CPU_STACK_ALIGNMENT - 1); - CopyMem ((VOID *)(UINTN)mApHltLoopCode, mApHltLoopCodeTemplate, sizeof (mApHltLoopCodeTemplate)); - TransferApToSafeState ((UINTN)mApHltLoopCode, TopOfStack, (UINTN)&mNumberToFinish); } } @@ -783,11 +788,12 @@ SmmRestoreCpu ( VOID ) { - SMM_S3_RESUME_STATE *SmmS3ResumeState; - IA32_DESCRIPTOR Ia32Idtr; - IA32_DESCRIPTOR X64Idtr; - IA32_IDT_GATE_DESCRIPTOR IdtEntryTable[EXCEPTION_VECTOR_NUMBER]; - EFI_STATUS Status; + SMM_S3_RESUME_STATE *SmmS3ResumeState; + IA32_DESCRIPTOR Ia32Idtr; + IA32_DESCRIPTOR X64Idtr; + IA32_IDT_GATE_DESCRIPTOR IdtEntryTable[EXCEPTION_VECTOR_NUMBER]; + EFI_STATUS Status; + EDKII_PEI_MP_SERVICES2_PPI *Mp2ServicePpi; DEBUG ((DEBUG_INFO, "SmmRestoreCpu()\n")); @@ -852,15 +858,21 @@ SmmRestoreCpu ( // mInitApsAfterSmmBaseReloc = FALSE; - PrepareApStartupVector (mAcpiCpuData.StartupVector); - // - // Send INIT IPI - SIPI to all APs - // - SendInitSipiSipiAllExcludingSelf ((UINT32)mAcpiCpuData.StartupVector); + if (mSmmS3ResumeState->MpService2Ppi != 0) { + Mp2ServicePpi = (EDKII_PEI_MP_SERVICES2_PPI *)(UINTN)mSmmS3ResumeState->MpService2Ppi; + Mp2ServicePpi->StartupAllCPUs (Mp2ServicePpi, InitializeCpuProcedure, 0, NULL); + } else { + PrepareApStartupVector (mAcpiCpuData.StartupVector); + // + // Send INIT IPI - SIPI to all APs + // + SendInitSipiSipiAllExcludingSelf ((UINT32)mAcpiCpuData.StartupVector); + InitializeCpuProcedure (NULL); + } + } else { + InitializeCpuProcedure (NULL); } - InitializeCpuProcedure (NULL); - // // Set a flag to restore SMM configuration in S3 path. // -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#108117): https://edk2.groups.io/g/devel/message/108117 Mute This Topic: https://groups.io/mt/101047983/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [Patch V3 5/5] UefiCpuPkg/PiSmmCpuDxe: use MpService2Ppi to wakeup AP in s3 2023-08-30 7:34 ` [edk2-devel] [Patch V3 5/5] UefiCpuPkg/PiSmmCpuDxe: use MpService2Ppi to wakeup AP in s3 duntan @ 2023-09-06 5:19 ` Ni, Ray 0 siblings, 0 replies; 5+ messages in thread From: Ni, Ray @ 2023-09-06 5:19 UTC (permalink / raw) To: Tan, Dun, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R [-- Attachment #1: Type: text/plain, Size: 4615 bytes --] Reviewed-by: Ray Ni <ray.ni@intel.com> Thanks, Ray ________________________________ From: Tan, Dun <dun.tan@intel.com> Sent: Wednesday, August 30, 2023 3:34 PM To: devel@edk2.groups.io <devel@edk2.groups.io> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com> Subject: [Patch V3 5/5] UefiCpuPkg/PiSmmCpuDxe: use MpService2Ppi to wakeup AP in s3 Use MpService2Ppi to wakeup AP in s3 boot flow during initializing CPU. If mSmmS3ResumeState->MpService2Ppi is not 0, then BSP will use MpService2Ppi->StartupAllCPUs to do CPU initialization for both BSP and AP instead of only sending InitSipiSipi for AP. Signed-off-by: Dun Tan <dun.tan@intel.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> --- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 58 +++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c index b9dbeaef87..0bae0e33f1 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c @@ -7,6 +7,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent **/ #include "PiSmmCpuDxeSmm.h" +#include <PiPei.h> +#include <Ppi/MpServices2.h> #pragma pack(1) typedef struct { @@ -594,18 +596,21 @@ InitializeCpuAfterRebase ( UINT8 Stack[128]; SetRegister (FALSE); - if (IsBsp) { - while (mNumberToFinish > 0) { - CpuPause (); + + if (mSmmS3ResumeState->MpService2Ppi == 0) { + if (IsBsp) { + while (mNumberToFinish > 0) { + CpuPause (); + } + } else { + // + // Place AP into the safe code, count down the number with lock mechanism in the safe code. + // + TopOfStack = (UINTN)Stack + sizeof (Stack); + TopOfStack &= ~(UINTN)(CPU_STACK_ALIGNMENT - 1); + CopyMem ((VOID *)(UINTN)mApHltLoopCode, mApHltLoopCodeTemplate, sizeof (mApHltLoopCodeTemplate)); + TransferApToSafeState ((UINTN)mApHltLoopCode, TopOfStack, (UINTN)&mNumberToFinish); } - } else { - // - // Place AP into the safe code, count down the number with lock mechanism in the safe code. - // - TopOfStack = (UINTN)Stack + sizeof (Stack); - TopOfStack &= ~(UINTN)(CPU_STACK_ALIGNMENT - 1); - CopyMem ((VOID *)(UINTN)mApHltLoopCode, mApHltLoopCodeTemplate, sizeof (mApHltLoopCodeTemplate)); - TransferApToSafeState ((UINTN)mApHltLoopCode, TopOfStack, (UINTN)&mNumberToFinish); } } @@ -783,11 +788,12 @@ SmmRestoreCpu ( VOID ) { - SMM_S3_RESUME_STATE *SmmS3ResumeState; - IA32_DESCRIPTOR Ia32Idtr; - IA32_DESCRIPTOR X64Idtr; - IA32_IDT_GATE_DESCRIPTOR IdtEntryTable[EXCEPTION_VECTOR_NUMBER]; - EFI_STATUS Status; + SMM_S3_RESUME_STATE *SmmS3ResumeState; + IA32_DESCRIPTOR Ia32Idtr; + IA32_DESCRIPTOR X64Idtr; + IA32_IDT_GATE_DESCRIPTOR IdtEntryTable[EXCEPTION_VECTOR_NUMBER]; + EFI_STATUS Status; + EDKII_PEI_MP_SERVICES2_PPI *Mp2ServicePpi; DEBUG ((DEBUG_INFO, "SmmRestoreCpu()\n")); @@ -852,15 +858,21 @@ SmmRestoreCpu ( // mInitApsAfterSmmBaseReloc = FALSE; - PrepareApStartupVector (mAcpiCpuData.StartupVector); - // - // Send INIT IPI - SIPI to all APs - // - SendInitSipiSipiAllExcludingSelf ((UINT32)mAcpiCpuData.StartupVector); + if (mSmmS3ResumeState->MpService2Ppi != 0) { + Mp2ServicePpi = (EDKII_PEI_MP_SERVICES2_PPI *)(UINTN)mSmmS3ResumeState->MpService2Ppi; + Mp2ServicePpi->StartupAllCPUs (Mp2ServicePpi, InitializeCpuProcedure, 0, NULL); + } else { + PrepareApStartupVector (mAcpiCpuData.StartupVector); + // + // Send INIT IPI - SIPI to all APs + // + SendInitSipiSipiAllExcludingSelf ((UINT32)mAcpiCpuData.StartupVector); + InitializeCpuProcedure (NULL); + } + } else { + InitializeCpuProcedure (NULL); } - InitializeCpuProcedure (NULL); - // // Set a flag to restore SMM configuration in S3 path. // -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#108304): https://edk2.groups.io/g/devel/message/108304 Mute This Topic: https://groups.io/mt/101047983/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- [-- Attachment #2: Type: text/html, Size: 8680 bytes --] ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-09-06 5:19 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-08-30 7:34 [edk2-devel] [Patch V3 0/5] Use MpService2Ppi to wakeup CPU in Smm CpuS3 duntan 2023-08-30 7:34 ` [edk2-devel] [Patch V3 4/5] UefiCpuPkg/PiSmmCpuDxe: code refinement for CpuS3.c duntan 2023-09-06 5:19 ` Ni, Ray 2023-08-30 7:34 ` [edk2-devel] [Patch V3 5/5] UefiCpuPkg/PiSmmCpuDxe: use MpService2Ppi to wakeup AP in s3 duntan 2023-09-06 5:19 ` Ni, Ray
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox