Please set PcdCpuApStackSize default value to 0x8000 in DEC file. :-)

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