[edk2-devel] [PATCH v2 2/4] UefiCpuPkg/PiSmmCpuDxeSmm: Consume SMM Base Hob for SmBase info

[Ni, Ray]

>    //
> -  SmmRelocateBases ();
> +  mSmmRelocated = (BOOLEAN)(GetFirstGuidHob (&gSmmBaseHobGuid) != NULL);

Do we support the case when the HOB is produced in normal boot but is not in S3 boot?
To keep code flow simple and not to support unnecessary configuration combinations, I prefer no.
Then, since mSmmRelocated is initialized in normal boot already. How about only adding
check here to make sure the HOB existence status is the same between normal and s3 boot.
This also helps to catch platform bugs that the HOB is only produced in normal boot.

ASSERT (mSmmRelocated == (BOOLEAN)(GetFirstGuidHob (&gSmmBaseHobGuid) != NULL));
If (mSmmRelocated != (BOOLEAN)(GetFirstGuidHob (&gSmmBaseHobGuid) != NULL)) {
  DEBUG ((
    DEBUG_ERROR,
    "SmmBase HOB %a produced in normal boot but %a in S3 boot",
    mSmmRelocated ? "is" : "is not",
    mSmmRelocated ? "is not" : "is"
    ));
    CpuDeadLoop ();
}
> 
>  /**
> -  Initialize PackageBsp Info. Processor specified by mPackageFirstThreadIndex[PackageIndex]
> -  will do the package-scope register programming. Set default CpuIndex to (UINT32)-1, which
> -  means not specified yet.
> +  Initialize mPackageFirstThreadIndex Info. Processor specified by mPackageFirstThreadIndex[PackageIndex]
> +  will do the package-scope register programming. Set default CpuIndex to (UINT32)-1, which means not
> +  specified yet.

The comments update is good. Can you separate in another patch?

> -      if (mIsBsp) {
> +      } else if (IsBsp) {
I agree with this change. But it changes the old logic.
Old logic calls below function even in non-S3 path.
If we want to change the behavior, can we separate in a new patch?

> +VOID
> +EFIAPI
> +ExecuteFirstSmiInit (

Can you remove the EFIAPI?




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98545): https://edk2.groups.io/g/devel/message/98545
Mute This Topic: https://groups.io/mt/96247964/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-