On Mon, 2014-11-03 at 08:36 +0000, Fan, Jeff wrote: > Chen, > > 1. Because you used the FreePages () to free the un-used memory space > allocated for stack per the actual processors number, please add ASSERT to > make sure PcdCpuApStackSize is multiple of 4K Bytes and add the comment in > DEC for this PCD. > ASSERT (gApStackSize & (SIZE_4KB - 1) == 0); > > 2. Because you could get actual processors number, could you allocate > internal CPU data buffer by actual CPUs instead of by maximum CPUs? > mMpSystemData.CpuDatas = AllocateZeroPool (sizeof (CPU_DATA_BLOCK) * > gMaxLogicalProcessorNumber); > > 3. Add ## for the following PCDs in INF file. > + gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## CONSUMES > + gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize > ## CONSUMES > > 4. The following 2 lines cannot pass VS2008, please add type case (UINT8 *) > for mApStackStart. > mTopOfApCommonStack = mApStackStart + gApStackSize; > TopOfApStack = mApStackStart + gApStackSize;
Thanks for your review. I will fix them soon. Thanks, Chen > > Thanks! > Jeff > > -----Original Message----- > From: Chen Fan [mailto:chen.fan.f...@cn.fujitsu.com] > Sent: Monday, October 27, 2014 5:30 PM > To: edk2-devel@lists.sourceforge.net > Cc: Fan, Jeff; Jordan Justen > Subject: [RFC PATCH v6 06/27] UefiCpuPkg/CpuDxe: introduce two PCD value > > introduce PCD value: PcdCpuMaxLogicalProcessorNumber and PcdCpuApStackSize, > used for initialize APs stacks. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Chen Fan <chen.fan.f...@cn.fujitsu.com> > --- > UefiCpuPkg/CpuDxe/CpuDxe.inf | 4 ++++ > UefiCpuPkg/CpuDxe/CpuMp.c | 41 ++++++++++++++++++++++++++++++++++++++++- > UefiCpuPkg/UefiCpuPkg.dec | 6 ++++++ > 3 files changed, 50 insertions(+), 1 deletion(-) > > diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.inf b/UefiCpuPkg/CpuDxe/CpuDxe.inf > index c2f12b7..1837560 100644 > --- a/UefiCpuPkg/CpuDxe/CpuDxe.inf > +++ b/UefiCpuPkg/CpuDxe/CpuDxe.inf > @@ -75,6 +75,10 @@ > gIdleLoopEventGuid ## CONSUMES ## > Event > gEfiVectorHandoffTableGuid ## SOMETIMES_CONSUMES ## > SystemTable > > +[Pcd] > + gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber > + gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize > + > [Depex] > TRUE > > diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c index > ea403e8..611e3d5 100644 > --- a/UefiCpuPkg/CpuDxe/CpuMp.c > +++ b/UefiCpuPkg/CpuDxe/CpuMp.c > @@ -15,9 +15,14 @@ > #include "CpuDxe.h" > #include "CpuMp.h" > > +UINTN gMaxLogicalProcessorNumber; > +UINTN gApStackSize; > + > VOID *mCommonStack = 0; > VOID *mTopOfApCommonStack = 0; > +VOID *mApStackStart = 0; > > +volatile UINTN mNumberOfProcessors; > > /** > Application Processor C code entry point. > @@ -29,6 +34,7 @@ ApEntryPointInC ( > VOID > ) > { > + mNumberOfProcessors++; > } > > > @@ -41,5 +47,38 @@ InitializeMpSupport ( > VOID > ) > { > -} > + gMaxLogicalProcessorNumber = (UINTN) PcdGet32 > + (PcdCpuMaxLogicalProcessorNumber); > + if (gMaxLogicalProcessorNumber < 1) { > + DEBUG ((DEBUG_ERROR, "Setting PcdCpuMaxLogicalProcessorNumber should be > more than zero.\n")); > + return; > + } > + > + if (gMaxLogicalProcessorNumber == 1) { > + return; > + } > + > + gApStackSize = (UINTN) PcdGet32 (PcdCpuApStackSize); > + > + mApStackStart = AllocatePages (EFI_SIZE_TO_PAGES > + (gMaxLogicalProcessorNumber * gApStackSize)); ASSERT (mApStackStart > + != NULL); > > + // > + // the first buffer of stack size used for common stack, when the > + amount of AP // more than 1, we should never free the common stack which > maybe used for AP reset. > + // > + mCommonStack = mApStackStart; > + mTopOfApCommonStack = mApStackStart + gApStackSize; mApStackStart = > + mTopOfApCommonStack; > + > + mNumberOfProcessors = 1; > + > + if (mNumberOfProcessors == 1) { > + FreePages (mCommonStack, EFI_SIZE_TO_PAGES (gMaxLogicalProcessorNumber * > gApStackSize)); > + return; > + } > + > + if (mNumberOfProcessors < gMaxLogicalProcessorNumber) { > + FreePages (mApStackStart, EFI_SIZE_TO_PAGES ((gMaxLogicalProcessorNumber > - mNumberOfProcessors) * > + gApStackSize)); > + } > +} > diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec index > c6e73a9..67d7196 100644 > --- a/UefiCpuPkg/UefiCpuPkg.dec > +++ b/UefiCpuPkg/UefiCpuPkg.dec > @@ -54,6 +54,12 @@ > ## Specifies delay value in microseconds after sending out an INIT IPI. > # @Prompt Configure delay value after send an INIT IPI > > gUefiCpuPkgTokenSpaceGuid.PcdCpuInitIpiDelayInMicroSeconds|10000|UINT32|0x30000002 > + ## Specifies max supported number of Logical Processors. > + # @Prompt Configure max supported number of Logical Processorss > + > + gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64|UINT32|0x > + 00000002 ## This value specifies the Application Processor (AP) stack > + size, which used for Mp Service. > + # @Prompt Configure stack size for Application Processor (AP) > + gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize|0x8000|UINT32|0x00000003 > > [UserExtensions.TianoCore."ExtraFiles"] > UefiCpuPkgExtra.uni > -- > 1.9.3 > ------------------------------------------------------------------------------ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
