Re: [edk2-devel] [PATCH v1 5/5] UefiCpuPkg/AmdSmmCpuFeaturesLib: Handles S3 save state

["Chang, Abner" <abner.chang@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 3884 bytes --]

Apart from the comment given to 4/5, I am afraid those extern variables in C file is not obey the section 5.4.2.1 extern in CCS.

Regards,
Abner

Abner
On Tue, Dec 6, 2022 at 09:23 PM, Abdul Lateef Attar wrote:

> 
> ---
> .../AmdSmmCpuFeaturesLib.inf | 1 +
> .../SmmCpuFeaturesLib/Amd/SmramSaveState.h | 19 +++++++++++
> .../SmmCpuFeaturesLib/Amd/SmmCpuFeaturesLib.c | 32 +++++++++++++++++++
> 3 files changed, 52 insertions(+)
> 
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> index 95eb31d16ead..7fd559e91ad8 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
> @@ -27,6 +27,7 @@ [Sources]
> [Packages]
> MdePkg/MdePkg.dec
> UefiCpuPkg/UefiCpuPkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> 
> [LibraryClasses]
> BaseLib
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmramSaveState.h
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmramSaveState.h
> index 290ebdbc9227..474a5dbd9765 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmramSaveState.h
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmramSaveState.h
> @@ -17,6 +17,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Library/DebugLib.h>
> #include <Library/SmmServicesTableLib.h>
> #include <Register/Amd/SmramSaveStateMap.h>
> +#include <Guid/AcpiS3Context.h>
> 
> // EFER register LMA bit
> #define LMA BIT10
> @@ -106,4 +107,22 @@ InternalSmmCpuFeaturesWriteSaveStateRegister (
> IN CONST VOID *Buffer
> );
> 
> +/**
> + Initialize MP synchronization data.
> +**/
> +VOID
> +EFIAPI
> +InitializeMpSyncData (
> + VOID
> + );
> +
> +/**
> + Perform SMM MP sync Semaphores re-initialization in the S3 boot path.
> +**/
> +VOID
> +EFIAPI
> +SmmS3MpSemaphoreInit (
> + VOID
> + );
> +
> #endif
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmmCpuFeaturesLib.c
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmmCpuFeaturesLib.c
> index 10bed4116397..b855573d9401 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmmCpuFeaturesLib.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmmCpuFeaturesLib.c
> @@ -14,6 +14,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> // The mode of the CPU at the time an SMI occurs
> extern UINT8 mSmmSaveStateRegisterLma;
> 
> +// SMM S3 resume state Ptr
> +extern SMM_S3_RESUME_STATE *mSmmS3ResumeState;
> +
> /**
> Read an SMM Save State register on the target processor. If this function
> returns EFI_UNSUPPORTED, then the caller is responsible for reading the
> @@ -441,4 +444,33 @@ SmmCpuFeaturesCompleteSmmReadyToLock (
> VOID
> )
> {
> + if (mSmmS3ResumeState != NULL ) {
> + mSmmS3ResumeState->SmmS3ResumeEntryPoint =
> (EFI_PHYSICAL_ADDRESS)(UINTN)SmmS3MpSemaphoreInit;
> + }
> +}
> +
> +/**
> + Perform SMM MP sync Semaphores re-initialization in the S3 boot path.
> +**/
> +VOID
> +EFIAPI
> +SmmS3MpSemaphoreInit (
> + VOID
> + )
> +{
> + InitializeMpSyncData ();
> +
> + DEBUG ((DEBUG_INFO, "SMM S3 Return CS = %x\n",
> mSmmS3ResumeState->ReturnCs));
> + DEBUG ((DEBUG_INFO, "SMM S3 Return Entry Point = %x\n",
> mSmmS3ResumeState->ReturnEntryPoint));
> + DEBUG ((DEBUG_INFO, "SMM S3 Return Context1 = %x\n",
> mSmmS3ResumeState->ReturnContext1));
> + DEBUG ((DEBUG_INFO, "SMM S3 Return Context2 = %x\n",
> mSmmS3ResumeState->ReturnContext2));
> + DEBUG ((DEBUG_INFO, "SMM S3 Return Stack Pointer = %x\n",
> mSmmS3ResumeState->ReturnStackPointer));
> +
> + AsmDisablePaging64 (
> + mSmmS3ResumeState->ReturnCs,
> + (UINT32)mSmmS3ResumeState->ReturnEntryPoint,
> + (UINT32)mSmmS3ResumeState->ReturnContext1,
> + (UINT32)mSmmS3ResumeState->ReturnContext2,
> + (UINT32)mSmmS3ResumeState->ReturnStackPointer
> + );
> }
> --

[-- Attachment #2: Type: text/html, Size: 4169 bytes --]