On Mon, 2014-09-29 at 01:38 +0000, Fan, Jeff wrote: 
> Please set PcdCpuApStackSize default value to 0x8000 in DEC file. :-)
Yes, I had set the PcdCpuApStackSize value in patch:
"[PATCH v4 07/23] UefiCpuPkg/CpuDxe: introduce PCD value
PcdCpuApStackSize".

Thanks,
Chen

> 
> Thanks!
> Jeff
> -----Original Message-----
> From: Chen, Fan [mailto:[email protected]] 
> Sent: Monday, September 29, 2014 9:31 AM
> To: Jordan Justen
> Cc: [email protected]; Fan, Jeff
> Subject: Re: [RFC PATCH v4 06/23] UefiCpuPkg/CpuDxe: gather the APs amount
> 
> On Fri, 2014-09-26 at 11:57 -0700, Jordan Justen wrote: 
> > On 2014-09-26 03:38:53, Chen Fan wrote:
> > > Due to AP cannot invoke any EFI API to allocate stack memory region. 
> > > so BSP needs to know the APs amount and then could allocate stack 
> > > spaces for them.
> > > 
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Chen Fan <[email protected]>
> > > ---
> > >  UefiCpuPkg/CpuDxe/CpuMp.c | 20 ++++++++++++++++++--
> > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c 
> > > index 8cf711a..2d52b22 100644
> > > --- a/UefiCpuPkg/CpuDxe/CpuMp.c
> > > +++ b/UefiCpuPkg/CpuDxe/CpuMp.c
> > > @@ -18,9 +18,10 @@
> > >  VOID *mCommonStack = 0;
> > >  VOID *mTopOfApCommonStack = 0;
> > >  
> > > +volatile UINTN   mNumberOfProcessors;
> > >  
> > >  /**
> > > -  Application Processor C code entry point
> > > +  Application Processor C code entry point.
> > 
> > This should be done in the original patch that adds it. I updated it 
> > on the ap-startup-example branch.
> OK, I will rework based on your branch.
> 
> > 
> > >  **/
> > >  VOID
> > > @@ -29,11 +30,19 @@ ApEntryPointInC (
> > >    VOID
> > >    )
> > >  {
> > > +  //
> > > +  // AP initialization C code
> > > +  //
> > > +  mNumberOfProcessors++;
> > > +
> > > +  AsmApDoneWithCommonStack ();
> > 
> > This is not quite safe, since the AP is actually still using the 
> > stack.
> > 
> > Instead, can you add a new PCD, PcdCpuMaxLogicalProcessorNumber, with 
> > a default of 64?
> > 
> > The BSP should allocate PcdCpuMaxLogicalProcessorNumber * 
> > PcdCpuApStackSize before starting the APs.
> > 
> > In ApEntryPointInC, the AP can take a part of this buffer and use 
> > BaseLib:SwitchStacks to stop using the common stack, and start running 
> > a new function.
> > 
> > This should also remove the need to update the assembly code beyond 
> > what I provided. I really prefer to minimize assembly code, because it 
> > is a frustrating extra maintenence burden.
> 
> Ok, got it.
> 
> I will change my implementation and use PCD pre-allocated APs stack buffer.
> 
> 
> Thanks,
> Chen
> 
> > 
> > -Jordan
> > 
> > > +
> > > +  CpuSleep ();
> > >  }
> > >  
> > >  
> > >  /**
> > > -  Initialize Multi-processor support
> > > +  Initialize Multi-processor support.
> > >  
> > >  **/
> > >  VOID
> > > @@ -41,5 +50,12 @@ InitializeMpSupport (
> > >    VOID
> > >    )
> > >  {
> > > +  mNumberOfProcessors = 1;
> > > +
> > > +  if (mNumberOfProcessors == 1) {
> > > +    return;
> > > +  }
> > > +
> > > +  DEBUG ((DEBUG_INFO, "Detect CPU count: %d\n", mNumberOfProcessors));
> > >  }
> > >  
> > > -- 
> > > 1.9.3
> > > 
> 

------------------------------------------------------------------------------
Slashdot TV.  Videos for Nerds.  Stuff that Matters.
http://pubads.g.doubleclick.net/gampad/clk?id=160591471&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to