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

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

Hi Gerd,

The change impact the old way of handling SMM init is only: refine the SmmInitHandler() to handle the code for the different execution branches (actually, it's also related to the patch purpose as I explained why removing the mIsBsp).  It's fine to do this separating work. I will do that in the next version patch.

But for below items, they all belong to the new smbase relocation flow:

>* Assume the biggest possibility of tile size is 8k.

--- for title size, I evaluated this according your comments before, I updated/moved it into the if condition check to avoid the old smm init impact. Because the old smm init should have no such assumption. So, it has to combine with new way handling.    

  GuidHob = GetFirstGuidHob (&gSmmBaseHobGuid);
  if (GuidHob != NULL) {
    //
    // Check whether the Required TileSize is enough.
    //
    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;
    }
	....
  }

>* Combine 2 SMIs (gcSmmInitTemplate & gcSmiHandlerTemplate) into one
>(gcSmiHandlerTemplate), the new SMI handler needs to run to 2 paths:
>one to SmmCpuFeaturesInitializeProcessor(), the other to SMM Core
>Entry Point.

This is about the SMI handling logic with new way. Once SMI happen, there is only one copy of code in smbase+8000 to handling both smm relocation & normal smi. Instead of original 2 copies of code in smbase+8000. We don't change original smi template code. 


>* Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) for first
>SMI init before normal SMI sources happen.
>* Call SmmCpuFeaturesInitializeProcessor() in parallel.

Above 2 are the same for the new way to issue the smi init process within if condition happen. All Excluding Self SMM IPI will achieve the parallel execution (SmmCpuFeaturesInitializeProcessor).


  //
  // For relocated SMBASE, some MSRs & CSRs are still required to be configured in SMM Mode for SMM Initialization.
  // Those MSRs & CSRs must be configured before normal SMI sources happen.
  // So, here is to issue SMI IPI (All Excluding  Self SMM IPI + BSP SMM IPI) to execute first SMI init.
  //
  if (mSmmRelocated) {
    ExecuteFirstSmiInit ();

    //
    // Call hook for BSP to perform extra actions in normal mode after all
    // SMM base addresses have been relocated on all CPUs
    //
    SmmCpuFeaturesSmmRelocationComplete ();
  }

Thanks,
Jiaxin 



> -----Original Message-----
> From: kraxel@redhat.com <kraxel@redhat.com>
> Sent: Friday, February 10, 2023 9:02 PM
> To: Ni, Ray <ray.ni@intel.com>
> Cc: devel@edk2.groups.io; Wu, Jiaxin <jiaxin.wu@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>
> Subject: Re: [edk2-devel] [PATCH v4 3/5] UefiCpuPkg/PiSmmCpuDxeSmm:
> Consume SMM Base Hob for SmBase info
> 
> On Fri, Feb 10, 2023 at 12:37:23PM +0000, Ni, Ray wrote:
> > Gerd,
> > All the 4 are needed to make sure SMM still works.
> 
> The new way (HOB present) of handling SMM init will surely require the
> whole patch series being applied.
> 
> For the old way (HOB not present) of handling SMM init should be
> possible to split up into smaller pieces, which each step being
> individually testable.  Specifically the SMI handler reorganization
> should be possible to split out, and I think the tiling changes too.
> 
> These are preparing changes reorganizing the code, once they are in
> place the final patch adding the new code paths to handle the new SMM
> init should be relatively small.
> 
> take care,
>   Gerd