Re: [edk2-devel] [PATCH v3 3/5] UefiCpuPkg/PiSmmCpuDxeSmm: Consume SMM Base Hob for SmBase info

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

> 
> > +UINT32   mBspApicId       = 0;
> 
> This should be moved to a separate patch with commit message explaining
> the reasons for the change.  My guess would be this is required to allow
> processors running SmmInitHandler in parallel.
> 

Yes, it's part of work to combine 2 SMIs (gcSmmInitTemplate & gcSmiHandlerTemplate) into one (gcSmiHandlerTemplate). And gcSmiHandlerTemplate will call the same SmmInitHandler for the first smi init. For smm CPU driver, we need keep it in the same patch, separate patch will make another patch not work, because we need replace the mIsBsp to IsBsp: 
IsBsp = (BOOLEAN)(mBspApicId == ApicId);

While the mBspApicId is need added one.


> Why mIsBsp is removed but mRebased is not?

mRebased is critical semaphore for each cpu to finish its 1st SMI. Remove it will make the function not work.


> 
> > -  // Allocate buffer for all of the tiles.
> > +  // Check whether the Required TileSize is enough.
> >    //
> > -  // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
> > -  // Volume 3C, Section 34.11 SMBASE Relocation
> > -  //   For Pentium and Intel486 processors, the SMBASE values must be
> > -  //   aligned on a 32-KByte boundary or the processor will enter shutdown
> > -  //   state during the execution of a RSM instruction.
> > +  if (TileSize > SIZE_8KB) {
> > +    DEBUG ((DEBUG_ERROR, "The Range of Smbase in SMRAM is not
> enough -- Required TileSize = 0x%08x, Actual TileSize = 0x%08x\n", TileSize,
> SIZE_8KB));
> > +    CpuDeadLoop ();
> > +    return RETURN_BUFFER_TOO_SMALL;
> > +  }
> 
> Where does the 8K come from?
> 
> This change is not mentioned in the commit message and most likely
> should be a separate patch.
> 

This is about the tile size assumption: 
One processor SMM Base tile size of buffer required to hold in SMRAM:
1. CPU SMRAM Save State Map starts at SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET(0xfc00),
2. extra CPU specific context start starts at SMBASE + SMM_PSD_OFFSET (PROCESSOR SMM DESCRIPTO, 0xfb00),
3. SMI entry point starts at SMBASE + SMM_HANDLER_OFFSET (0x8000).
This size is rounded up to nearest power of 2.
The pre-assigned smbase in hob is based on the biggest possibility of  tile size. We think it's impossible that tile size bigger than 8k, that's the reason we add the check here. I agree to make this as separate patch.


> take care,
>   Gerd
> 
> 
> 
> 
>