* [Patch 0/2] UefiCpuPkg/PiSmmCpuDxeSmm: Enhance S3 code. @ 2017-09-28 9:15 Eric Dong 2017-09-28 9:15 ` [Patch 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: Combine INIT-SIPI-SIPI Eric Dong 2017-09-28 9:15 ` [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to avoid duplicated code Eric Dong 0 siblings, 2 replies; 6+ messages in thread From: Eric Dong @ 2017-09-28 9:15 UTC (permalink / raw) To: edk2-devel Combine INIT-SIPI-SIPI code and remove the duplicate code. Eric Dong (2): UefiCpuPkg/PiSmmCpuDxeSmm: Combine INIT-SIPI-SIPI. UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to avoid duplicated code. UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 123 +++++++++++++------------------------- 1 file changed, 43 insertions(+), 80 deletions(-) -- 2.7.0.windows.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Patch 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: Combine INIT-SIPI-SIPI. 2017-09-28 9:15 [Patch 0/2] UefiCpuPkg/PiSmmCpuDxeSmm: Enhance S3 code Eric Dong @ 2017-09-28 9:15 ` Eric Dong 2017-09-28 9:15 ` [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to avoid duplicated code Eric Dong 1 sibling, 0 replies; 6+ messages in thread From: Eric Dong @ 2017-09-28 9:15 UTC (permalink / raw) To: edk2-devel; +Cc: Jiewen Yao, Ruiyu Ni In S3 resume path, current implementation do 2 separate INIT-SIPI-SIPI, this is not necessary. This change combine these 2 INIT-SIPI-SIPI to 1 and add CpuPause between them. Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong <eric.dong@intel.com> --- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 51 ++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c index 9404501..ae4b516 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c @@ -39,6 +39,11 @@ typedef struct { // SPIN_LOCK *mMemoryMappedLock = NULL; +// +// Signal that SMM BASE relocation is complete. +// +volatile BOOLEAN mInitApsAfterSmmBaseReloc; + /** Get starting address and size of the rendezvous entry for APs. Information for fixing a jump instruction in the code is also returned. @@ -342,17 +347,21 @@ SetProcessorRegister ( } } + + /** - AP initialization before SMBASE relocation in the S3 boot path. + AP initialization before then after SMBASE relocation in the S3 boot path. **/ VOID -EarlyMPRendezvousProcedure ( +MPRendezvousProcedure ( VOID ) { CPU_REGISTER_TABLE *RegisterTableList; UINT32 InitApicId; UINTN Index; + UINTN TopOfStack; + UINT8 Stack[128]; LoadMtrrData (mAcpiCpuData.MtrrTable); @@ -368,25 +377,18 @@ EarlyMPRendezvousProcedure ( } } + // // Count down the number with lock mechanism. // InterlockedDecrement (&mNumberToFinish); -} -/** - AP initialization after SMBASE relocation in the S3 boot path. -**/ -VOID -MPRendezvousProcedure ( - VOID - ) -{ - CPU_REGISTER_TABLE *RegisterTableList; - UINT32 InitApicId; - UINTN Index; - UINTN TopOfStack; - UINT8 Stack[128]; + // + // Wait for BSP to signal SMM Base relocation done. + // + while (!mInitApsAfterSmmBaseReloc) { + CpuPause (); + } ProgramVirtualWireMode (); DisableLvtInterrupts (); @@ -500,7 +502,12 @@ EarlyInitializeCpu ( PrepareApStartupVector (mAcpiCpuData.StartupVector); mNumberToFinish = mAcpiCpuData.NumberOfCpus - 1; - mExchangeInfo->ApFunction = (VOID *) (UINTN) EarlyMPRendezvousProcedure; + mExchangeInfo->ApFunction = (VOID *) (UINTN) MPRendezvousProcedure; + + // + // Execute code for before SmmBaseReloc. Note: This flag is maintained across S3 boots. + // + mInitApsAfterSmmBaseReloc = FALSE; // // Send INIT IPI - SIPI to all APs @@ -538,17 +545,11 @@ InitializeCpu ( } mNumberToFinish = mAcpiCpuData.NumberOfCpus - 1; - // - // StackStart was updated when APs were waken up in EarlyInitializeCpu. - // Re-initialize StackAddress to original beginning address. - // - mExchangeInfo->StackStart = (VOID *) (UINTN) mAcpiCpuData.StackAddress; - mExchangeInfo->ApFunction = (VOID *) (UINTN) MPRendezvousProcedure; // - // Send INIT IPI - SIPI to all APs + // Signal that SMM base relocation is complete and to continue initialization. // - SendInitSipiSipiAllExcludingSelf ((UINT32)mAcpiCpuData.StartupVector); + mInitApsAfterSmmBaseReloc = TRUE; while (mNumberToFinish > 0) { CpuPause (); -- 2.7.0.windows.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to avoid duplicated code. 2017-09-28 9:15 [Patch 0/2] UefiCpuPkg/PiSmmCpuDxeSmm: Enhance S3 code Eric Dong 2017-09-28 9:15 ` [Patch 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: Combine INIT-SIPI-SIPI Eric Dong @ 2017-09-28 9:15 ` Eric Dong 2017-09-28 9:30 ` Zeng, Star 1 sibling, 1 reply; 6+ messages in thread From: Eric Dong @ 2017-09-28 9:15 UTC (permalink / raw) To: edk2-devel; +Cc: Jiewen Yao, Ruiyu Ni Refine code to avoid duplicate code to set processor register. Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong <eric.dong@intel.com> --- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 78 ++++++++++----------------------------- 1 file changed, 20 insertions(+), 58 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c index ae4b516..500a0e2 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c @@ -208,18 +208,28 @@ Returns: This function programs registers for the calling processor. - @param RegisterTable Pointer to register table of the running processor. + @param RegisterTableList Pointer to register table of the running processor. **/ VOID SetProcessorRegister ( - IN CPU_REGISTER_TABLE *RegisterTable + IN CPU_REGISTER_TABLE *RegisterTableList ) { CPU_REGISTER_TABLE_ENTRY *RegisterTableEntry; UINTN Index; UINTN Value; SPIN_LOCK *MsrSpinLock; + UINT32 InitApicId; + CPU_REGISTER_TABLE *RegisterTable; + + InitApicId = GetInitialApicId (); + for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { + if (RegisterTableList[Index].InitialApicId == InitApicId) { + RegisterTable = &RegisterTableList[Index]; + break; + } + } // // Traverse Register Table of this logical processor @@ -347,8 +357,6 @@ SetProcessorRegister ( } } - - /** AP initialization before then after SMBASE relocation in the S3 boot path. **/ @@ -357,26 +365,12 @@ MPRendezvousProcedure ( VOID ) { - CPU_REGISTER_TABLE *RegisterTableList; - UINT32 InitApicId; - UINTN Index; UINTN TopOfStack; UINT8 Stack[128]; LoadMtrrData (mAcpiCpuData.MtrrTable); - // - // Find processor number for this CPU. - // - RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.PreSmmInitRegisterTable; - InitApicId = GetInitialApicId (); - for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { - if (RegisterTableList[Index].InitialApicId == InitApicId) { - SetProcessorRegister (&RegisterTableList[Index]); - break; - } - } - + SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.PreSmmInitRegisterTable); // // Count down the number with lock mechanism. @@ -393,14 +387,7 @@ MPRendezvousProcedure ( ProgramVirtualWireMode (); DisableLvtInterrupts (); - RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.RegisterTable; - InitApicId = GetInitialApicId (); - for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { - if (RegisterTableList[Index].InitialApicId == InitApicId) { - SetProcessorRegister (&RegisterTableList[Index]); - break; - } - } + SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.RegisterTable); // // Place AP into the safe code, count down the number with lock mechanism in the safe code. @@ -475,27 +462,13 @@ PrepareApStartupVector ( **/ VOID -EarlyInitializeCpu ( +InitializeCpuBeforeRebase ( VOID ) { - CPU_REGISTER_TABLE *RegisterTableList; - UINT32 InitApicId; - UINTN Index; - LoadMtrrData (mAcpiCpuData.MtrrTable); - // - // Find processor number for this CPU. - // - RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.PreSmmInitRegisterTable; - InitApicId = GetInitialApicId (); - for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { - if (RegisterTableList[Index].InitialApicId == InitApicId) { - SetProcessorRegister (&RegisterTableList[Index]); - break; - } - } + SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.PreSmmInitRegisterTable); ProgramVirtualWireMode (); @@ -527,22 +500,11 @@ EarlyInitializeCpu ( **/ VOID -InitializeCpu ( +InitializeCpuAfterRebase ( VOID ) { - CPU_REGISTER_TABLE *RegisterTableList; - UINT32 InitApicId; - UINTN Index; - - RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.RegisterTable; - InitApicId = GetInitialApicId (); - for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { - if (RegisterTableList[Index].InitialApicId == InitApicId) { - SetProcessorRegister (&RegisterTableList[Index]); - break; - } - } + SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.RegisterTable); mNumberToFinish = mAcpiCpuData.NumberOfCpus - 1; @@ -660,7 +622,7 @@ SmmRestoreCpu ( // // First time microcode load and restore MTRRs // - EarlyInitializeCpu (); + InitializeCpuBeforeRebase (); } // @@ -675,7 +637,7 @@ SmmRestoreCpu ( // // Restore MSRs for BSP and all APs // - InitializeCpu (); + InitializeCpuAfterRebase (); } // -- 2.7.0.windows.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to avoid duplicated code. 2017-09-28 9:15 ` [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to avoid duplicated code Eric Dong @ 2017-09-28 9:30 ` Zeng, Star 2017-09-28 10:04 ` Ni, Ruiyu 0 siblings, 1 reply; 6+ messages in thread From: Zeng, Star @ 2017-09-28 9:30 UTC (permalink / raw) To: Dong, Eric, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu, Yao, Jiewen, Zeng, Star Just FYI, another idea is to declare SetProcessorRegister() like below, then the caller of SetProcessorRegister() has no need to touch mAcpiCpuData. VOID SetProcessorRegister ( IN BOOLEAN PreSmmFlag ) { CPU_REGISTER_TABLE *RegisterTableList; ... if (PreSmmFlag) { RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.PreSmmInitRegisterTable; } else { RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.RegisterTable; } ... Thanks, Star -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Eric Dong Sent: Thursday, September 28, 2017 5:15 PM To: edk2-devel@lists.01.org Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com> Subject: [edk2] [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to avoid duplicated code. Refine code to avoid duplicate code to set processor register. Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong <eric.dong@intel.com> --- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 78 ++++++++++----------------------------- 1 file changed, 20 insertions(+), 58 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c index ae4b516..500a0e2 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c @@ -208,18 +208,28 @@ Returns: This function programs registers for the calling processor. - @param RegisterTable Pointer to register table of the running processor. + @param RegisterTableList Pointer to register table of the running processor. **/ VOID SetProcessorRegister ( - IN CPU_REGISTER_TABLE *RegisterTable + IN CPU_REGISTER_TABLE *RegisterTableList ) { CPU_REGISTER_TABLE_ENTRY *RegisterTableEntry; UINTN Index; UINTN Value; SPIN_LOCK *MsrSpinLock; + UINT32 InitApicId; + CPU_REGISTER_TABLE *RegisterTable; + + InitApicId = GetInitialApicId (); + for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { + if (RegisterTableList[Index].InitialApicId == InitApicId) { + RegisterTable = &RegisterTableList[Index]; + break; + } + } // // Traverse Register Table of this logical processor @@ -347,8 +357,6 @@ SetProcessorRegister ( } } - - /** AP initialization before then after SMBASE relocation in the S3 boot path. **/ @@ -357,26 +365,12 @@ MPRendezvousProcedure ( VOID ) { - CPU_REGISTER_TABLE *RegisterTableList; - UINT32 InitApicId; - UINTN Index; UINTN TopOfStack; UINT8 Stack[128]; LoadMtrrData (mAcpiCpuData.MtrrTable); - // - // Find processor number for this CPU. - // - RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.PreSmmInitRegisterTable; - InitApicId = GetInitialApicId (); - for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { - if (RegisterTableList[Index].InitialApicId == InitApicId) { - SetProcessorRegister (&RegisterTableList[Index]); - break; - } - } - + SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN) + mAcpiCpuData.PreSmmInitRegisterTable); // // Count down the number with lock mechanism. @@ -393,14 +387,7 @@ MPRendezvousProcedure ( ProgramVirtualWireMode (); DisableLvtInterrupts (); - RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.RegisterTable; - InitApicId = GetInitialApicId (); - for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { - if (RegisterTableList[Index].InitialApicId == InitApicId) { - SetProcessorRegister (&RegisterTableList[Index]); - break; - } - } + SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN) + mAcpiCpuData.RegisterTable); // // Place AP into the safe code, count down the number with lock mechanism in the safe code. @@ -475,27 +462,13 @@ PrepareApStartupVector ( **/ VOID -EarlyInitializeCpu ( +InitializeCpuBeforeRebase ( VOID ) { - CPU_REGISTER_TABLE *RegisterTableList; - UINT32 InitApicId; - UINTN Index; - LoadMtrrData (mAcpiCpuData.MtrrTable); - // - // Find processor number for this CPU. - // - RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.PreSmmInitRegisterTable; - InitApicId = GetInitialApicId (); - for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { - if (RegisterTableList[Index].InitialApicId == InitApicId) { - SetProcessorRegister (&RegisterTableList[Index]); - break; - } - } + SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN) + mAcpiCpuData.PreSmmInitRegisterTable); ProgramVirtualWireMode (); @@ -527,22 +500,11 @@ EarlyInitializeCpu ( **/ VOID -InitializeCpu ( +InitializeCpuAfterRebase ( VOID ) { - CPU_REGISTER_TABLE *RegisterTableList; - UINT32 InitApicId; - UINTN Index; - - RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.RegisterTable; - InitApicId = GetInitialApicId (); - for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { - if (RegisterTableList[Index].InitialApicId == InitApicId) { - SetProcessorRegister (&RegisterTableList[Index]); - break; - } - } + SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN) + mAcpiCpuData.RegisterTable); mNumberToFinish = mAcpiCpuData.NumberOfCpus - 1; @@ -660,7 +622,7 @@ SmmRestoreCpu ( // // First time microcode load and restore MTRRs // - EarlyInitializeCpu (); + InitializeCpuBeforeRebase (); } // @@ -675,7 +637,7 @@ SmmRestoreCpu ( // // Restore MSRs for BSP and all APs // - InitializeCpu (); + InitializeCpuAfterRebase (); } // -- 2.7.0.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to avoid duplicated code. 2017-09-28 9:30 ` Zeng, Star @ 2017-09-28 10:04 ` Ni, Ruiyu 2017-09-28 11:08 ` Yao, Jiewen 0 siblings, 1 reply; 6+ messages in thread From: Ni, Ruiyu @ 2017-09-28 10:04 UTC (permalink / raw) To: Zeng, Star, Dong, Eric, edk2-devel@lists.01.org; +Cc: Yao, Jiewen 1. We could either use BOOLEAN flag to tell SetProcessorRegister() which register table to use. Or, pass both RegisterTableList and CpuNum into SetProcessorRegister(). I prefer the latter one. VOID SetProcessorRegister ( IN CPU_REGISTER_TABLE *RegisterTables， IN UINTN RegisterTableCount ); 2. How about change MPRendezvousProcedure to InitializeAp? Thanks/Ray > -----Original Message----- > From: Zeng, Star > Sent: Thursday, September 28, 2017 5:31 PM > To: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Zeng, > Star <star.zeng@intel.com> > Subject: RE: [edk2] [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to > avoid duplicated code. > > Just FYI, another idea is to declare SetProcessorRegister() like below, then the > caller of SetProcessorRegister() has no need to touch mAcpiCpuData. > > VOID > SetProcessorRegister ( > IN BOOLEAN PreSmmFlag > ) > { > CPU_REGISTER_TABLE *RegisterTableList; > ... > if (PreSmmFlag) { > RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) > mAcpiCpuData.PreSmmInitRegisterTable; > } else { > RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) > mAcpiCpuData.RegisterTable; > } > ... > > Thanks, > Star > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Eric > Dong > Sent: Thursday, September 28, 2017 5:15 PM > To: edk2-devel@lists.01.org > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com> > Subject: [edk2] [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to > avoid duplicated code. > > Refine code to avoid duplicate code to set processor register. > > Cc: Jiewen Yao <jiewen.yao@intel.com> > Cc: Ruiyu Ni <ruiyu.ni@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong <eric.dong@intel.com> > --- > UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 78 ++++++++++-------------------------- > --- > 1 file changed, 20 insertions(+), 58 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > index ae4b516..500a0e2 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > @@ -208,18 +208,28 @@ Returns: > > This function programs registers for the calling processor. > > - @param RegisterTable Pointer to register table of the running processor. > + @param RegisterTableList Pointer to register table of the running > processor. > > **/ > VOID > SetProcessorRegister ( > - IN CPU_REGISTER_TABLE *RegisterTable > + IN CPU_REGISTER_TABLE *RegisterTableList > ) > { > CPU_REGISTER_TABLE_ENTRY *RegisterTableEntry; > UINTN Index; > UINTN Value; > SPIN_LOCK *MsrSpinLock; > + UINT32 InitApicId; > + CPU_REGISTER_TABLE *RegisterTable; > + > + InitApicId = GetInitialApicId (); > + for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { > + if (RegisterTableList[Index].InitialApicId == InitApicId) { > + RegisterTable = &RegisterTableList[Index]; > + break; > + } > + } > > // > // Traverse Register Table of this logical processor @@ -347,8 +357,6 @@ > SetProcessorRegister ( > } > } > > - > - > /** > AP initialization before then after SMBASE relocation in the S3 boot path. > **/ > @@ -357,26 +365,12 @@ MPRendezvousProcedure ( > VOID > ) > { > - CPU_REGISTER_TABLE *RegisterTableList; > - UINT32 InitApicId; > - UINTN Index; > UINTN TopOfStack; > UINT8 Stack[128]; > > LoadMtrrData (mAcpiCpuData.MtrrTable); > > - // > - // Find processor number for this CPU. > - // > - RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) > mAcpiCpuData.PreSmmInitRegisterTable; > - InitApicId = GetInitialApicId (); > - for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { > - if (RegisterTableList[Index].InitialApicId == InitApicId) { > - SetProcessorRegister (&RegisterTableList[Index]); > - break; > - } > - } > - > + SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN) > + mAcpiCpuData.PreSmmInitRegisterTable); > > // > // Count down the number with lock mechanism. > @@ -393,14 +387,7 @@ MPRendezvousProcedure ( > ProgramVirtualWireMode (); > DisableLvtInterrupts (); > > - RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) > mAcpiCpuData.RegisterTable; > - InitApicId = GetInitialApicId (); > - for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { > - if (RegisterTableList[Index].InitialApicId == InitApicId) { > - SetProcessorRegister (&RegisterTableList[Index]); > - break; > - } > - } > + SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN) > + mAcpiCpuData.RegisterTable); > > // > // Place AP into the safe code, count down the number with lock mechanism in > the safe code. > @@ -475,27 +462,13 @@ PrepareApStartupVector ( > > **/ > VOID > -EarlyInitializeCpu ( > +InitializeCpuBeforeRebase ( > VOID > ) > { > - CPU_REGISTER_TABLE *RegisterTableList; > - UINT32 InitApicId; > - UINTN Index; > - > LoadMtrrData (mAcpiCpuData.MtrrTable); > > - // > - // Find processor number for this CPU. > - // > - RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) > mAcpiCpuData.PreSmmInitRegisterTable; > - InitApicId = GetInitialApicId (); > - for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { > - if (RegisterTableList[Index].InitialApicId == InitApicId) { > - SetProcessorRegister (&RegisterTableList[Index]); > - break; > - } > - } > + SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN) > + mAcpiCpuData.PreSmmInitRegisterTable); > > ProgramVirtualWireMode (); > > @@ -527,22 +500,11 @@ EarlyInitializeCpu ( > > **/ > VOID > -InitializeCpu ( > +InitializeCpuAfterRebase ( > VOID > ) > { > - CPU_REGISTER_TABLE *RegisterTableList; > - UINT32 InitApicId; > - UINTN Index; > - > - RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) > mAcpiCpuData.RegisterTable; > - InitApicId = GetInitialApicId (); > - for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { > - if (RegisterTableList[Index].InitialApicId == InitApicId) { > - SetProcessorRegister (&RegisterTableList[Index]); > - break; > - } > - } > + SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN) > + mAcpiCpuData.RegisterTable); > > mNumberToFinish = mAcpiCpuData.NumberOfCpus - 1; > > @@ -660,7 +622,7 @@ SmmRestoreCpu ( > // > // First time microcode load and restore MTRRs > // > - EarlyInitializeCpu (); > + InitializeCpuBeforeRebase (); > } > > // > @@ -675,7 +637,7 @@ SmmRestoreCpu ( > // > // Restore MSRs for BSP and all APs > // > - InitializeCpu (); > + InitializeCpuAfterRebase (); > } > > // > -- > 2.7.0.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to avoid duplicated code. 2017-09-28 10:04 ` Ni, Ruiyu @ 2017-09-28 11:08 ` Yao, Jiewen 0 siblings, 0 replies; 6+ messages in thread From: Yao, Jiewen @ 2017-09-28 11:08 UTC (permalink / raw) To: Ni, Ruiyu; +Cc: Zeng, Star, Dong, Eric, edk2-devel@lists.01.org yes, i like the idea to use table as parameter. thank you! Yao, Jiewen > 在 2017年9月28日，下午6:04，Ni, Ruiyu <ruiyu.ni@intel.com> 写道： > > 1. We could either use BOOLEAN flag to tell SetProcessorRegister() which register table to use. > Or, pass both RegisterTableList and CpuNum into SetProcessorRegister(). > I prefer the latter one. > VOID > SetProcessorRegister ( > IN CPU_REGISTER_TABLE *RegisterTables， > IN UINTN RegisterTableCount > ); > > > 2. How about change MPRendezvousProcedure to InitializeAp? > > Thanks/Ray > >> -----Original Message----- >> From: Zeng, Star >> Sent: Thursday, September 28, 2017 5:31 PM >> To: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org >> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Zeng, >> Star <star.zeng@intel.com> >> Subject: RE: [edk2] [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to >> avoid duplicated code. >> >> Just FYI, another idea is to declare SetProcessorRegister() like below, then the >> caller of SetProcessorRegister() has no need to touch mAcpiCpuData. >> >> VOID >> SetProcessorRegister ( >> IN BOOLEAN PreSmmFlag >> ) >> { >> CPU_REGISTER_TABLE *RegisterTableList; >> ... >> if (PreSmmFlag) { >> RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) >> mAcpiCpuData.PreSmmInitRegisterTable; >> } else { >> RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) >> mAcpiCpuData.RegisterTable; >> } >> ... >> >> Thanks, >> Star >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Eric >> Dong >> Sent: Thursday, September 28, 2017 5:15 PM >> To: edk2-devel@lists.01.org >> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com> >> Subject: [edk2] [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to >> avoid duplicated code. >> >> Refine code to avoid duplicate code to set processor register. >> >> Cc: Jiewen Yao <jiewen.yao@intel.com> >> Cc: Ruiyu Ni <ruiyu.ni@intel.com> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Eric Dong <eric.dong@intel.com> >> --- >> UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 78 ++++++++++-------------------------- >> --- >> 1 file changed, 20 insertions(+), 58 deletions(-) >> >> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c >> b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c >> index ae4b516..500a0e2 100644 >> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c >> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c >> @@ -208,18 +208,28 @@ Returns: >> >> This function programs registers for the calling processor. >> >> - @param RegisterTable Pointer to register table of the running processor. >> + @param RegisterTableList Pointer to register table of the running >> processor. >> >> **/ >> VOID >> SetProcessorRegister ( >> - IN CPU_REGISTER_TABLE *RegisterTable >> + IN CPU_REGISTER_TABLE *RegisterTableList >> ) >> { >> CPU_REGISTER_TABLE_ENTRY *RegisterTableEntry; >> UINTN Index; >> UINTN Value; >> SPIN_LOCK *MsrSpinLock; >> + UINT32 InitApicId; >> + CPU_REGISTER_TABLE *RegisterTable; >> + >> + InitApicId = GetInitialApicId (); >> + for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { >> + if (RegisterTableList[Index].InitialApicId == InitApicId) { >> + RegisterTable = &RegisterTableList[Index]; >> + break; >> + } >> + } >> >> // >> // Traverse Register Table of this logical processor @@ -347,8 +357,6 @@ >> SetProcessorRegister ( >> } >> } >> >> - >> - >> /** >> AP initialization before then after SMBASE relocation in the S3 boot path. >> **/ >> @@ -357,26 +365,12 @@ MPRendezvousProcedure ( >> VOID >> ) >> { >> - CPU_REGISTER_TABLE *RegisterTableList; >> - UINT32 InitApicId; >> - UINTN Index; >> UINTN TopOfStack; >> UINT8 Stack[128]; >> >> LoadMtrrData (mAcpiCpuData.MtrrTable); >> >> - // >> - // Find processor number for this CPU. >> - // >> - RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) >> mAcpiCpuData.PreSmmInitRegisterTable; >> - InitApicId = GetInitialApicId (); >> - for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { >> - if (RegisterTableList[Index].InitialApicId == InitApicId) { >> - SetProcessorRegister (&RegisterTableList[Index]); >> - break; >> - } >> - } >> - >> + SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN) >> + mAcpiCpuData.PreSmmInitRegisterTable); >> >> // >> // Count down the number with lock mechanism. >> @@ -393,14 +387,7 @@ MPRendezvousProcedure ( >> ProgramVirtualWireMode (); >> DisableLvtInterrupts (); >> >> - RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) >> mAcpiCpuData.RegisterTable; >> - InitApicId = GetInitialApicId (); >> - for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { >> - if (RegisterTableList[Index].InitialApicId == InitApicId) { >> - SetProcessorRegister (&RegisterTableList[Index]); >> - break; >> - } >> - } >> + SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN) >> + mAcpiCpuData.RegisterTable); >> >> // >> // Place AP into the safe code, count down the number with lock mechanism in >> the safe code. >> @@ -475,27 +462,13 @@ PrepareApStartupVector ( >> >> **/ >> VOID >> -EarlyInitializeCpu ( >> +InitializeCpuBeforeRebase ( >> VOID >> ) >> { >> - CPU_REGISTER_TABLE *RegisterTableList; >> - UINT32 InitApicId; >> - UINTN Index; >> - >> LoadMtrrData (mAcpiCpuData.MtrrTable); >> >> - // >> - // Find processor number for this CPU. >> - // >> - RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) >> mAcpiCpuData.PreSmmInitRegisterTable; >> - InitApicId = GetInitialApicId (); >> - for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { >> - if (RegisterTableList[Index].InitialApicId == InitApicId) { >> - SetProcessorRegister (&RegisterTableList[Index]); >> - break; >> - } >> - } >> + SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN) >> + mAcpiCpuData.PreSmmInitRegisterTable); >> >> ProgramVirtualWireMode (); >> >> @@ -527,22 +500,11 @@ EarlyInitializeCpu ( >> >> **/ >> VOID >> -InitializeCpu ( >> +InitializeCpuAfterRebase ( >> VOID >> ) >> { >> - CPU_REGISTER_TABLE *RegisterTableList; >> - UINT32 InitApicId; >> - UINTN Index; >> - >> - RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) >> mAcpiCpuData.RegisterTable; >> - InitApicId = GetInitialApicId (); >> - for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) { >> - if (RegisterTableList[Index].InitialApicId == InitApicId) { >> - SetProcessorRegister (&RegisterTableList[Index]); >> - break; >> - } >> - } >> + SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN) >> + mAcpiCpuData.RegisterTable); >> >> mNumberToFinish = mAcpiCpuData.NumberOfCpus - 1; >> >> @@ -660,7 +622,7 @@ SmmRestoreCpu ( >> // >> // First time microcode load and restore MTRRs >> // >> - EarlyInitializeCpu (); >> + InitializeCpuBeforeRebase (); >> } >> >> // >> @@ -675,7 +637,7 @@ SmmRestoreCpu ( >> // >> // Restore MSRs for BSP and all APs >> // >> - InitializeCpu (); >> + InitializeCpuAfterRebase (); >> } >> >> // >> -- >> 2.7.0.windows.1 >> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-09-28 11:05 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-09-28 9:15 [Patch 0/2] UefiCpuPkg/PiSmmCpuDxeSmm: Enhance S3 code Eric Dong 2017-09-28 9:15 ` [Patch 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: Combine INIT-SIPI-SIPI Eric Dong 2017-09-28 9:15 ` [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to avoid duplicated code Eric Dong 2017-09-28 9:30 ` Zeng, Star 2017-09-28 10:04 ` Ni, Ruiyu 2017-09-28 11:08 ` Yao, Jiewen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox