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

["Ni, Ray" <ray.ni@intel.com>]

Will review v6 of PiSmmCpu driver change after you two are aligned.

> -----Original Message-----
> From: Wu, Jiaxin <jiaxin.wu@intel.com>
> Sent: Monday, February 13, 2023 12:15 PM
> To: kraxel@redhat.com; Ni, Ray <ray.ni@intel.com>
> Cc: devel@edk2.groups.io; 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
> 
> 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