From mboxrd@z Thu Jan 1 00:00:00 1970 Subject: Re: [edk2-devel] [PATCH v1 5/5] UefiCpuPkg/AmdSmmCpuFeaturesLib: Handles S3 save state To: Abdul Lateef Attar ,devel@edk2.groups.io From: "Chang, Abner" X-Originating-Location: TW (185.221.70.44) X-Originating-Platform: Windows Chrome 107 User-Agent: GROUPS.IO Web Poster MIME-Version: 1.0 Date: Wed, 07 Dec 2022 21:46:47 -0800 References: In-Reply-To: Message-ID: <24898.1670478407424476568@groups.io> Content-Type: multipart/alternative; boundary="zHKTwmUM7eacyAUYDD2E" --zHKTwmUM7eacyAUYDD2E Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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: >=20 > --- > .../AmdSmmCpuFeaturesLib.inf | 1 + > .../SmmCpuFeaturesLib/Amd/SmramSaveState.h | 19 +++++++++++ > .../SmmCpuFeaturesLib/Amd/SmmCpuFeaturesLib.c | 32 +++++++++++++++++++ > 3 files changed, 52 insertions(+) >=20 > diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.in= f > 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 >=20 > [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 > #include > #include > +#include >=20 > // EFER register LMA bit > #define LMA BIT10 > @@ -106,4 +107,22 @@ InternalSmmCpuFeaturesWriteSaveStateRegister ( > IN CONST VOID *Buffer > ); >=20 > +/** > + 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; >=20 > +// 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 !=3D NULL ) { > + mSmmS3ResumeState->SmmS3ResumeEntryPoint =3D > (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 =3D %x\n", > mSmmS3ResumeState->ReturnCs)); > + DEBUG ((DEBUG_INFO, "SMM S3 Return Entry Point =3D %x\n", > mSmmS3ResumeState->ReturnEntryPoint)); > + DEBUG ((DEBUG_INFO, "SMM S3 Return Context1 =3D %x\n", > mSmmS3ResumeState->ReturnContext1)); > + DEBUG ((DEBUG_INFO, "SMM S3 Return Context2 =3D %x\n", > mSmmS3ResumeState->ReturnContext2)); > + DEBUG ((DEBUG_INFO, "SMM S3 Return Stack Pointer =3D %x\n", > mSmmS3ResumeState->ReturnStackPointer)); > + > + AsmDisablePaging64 ( > + mSmmS3ResumeState->ReturnCs, > + (UINT32)mSmmS3ResumeState->ReturnEntryPoint, > + (UINT32)mSmmS3ResumeState->ReturnContext1, > + (UINT32)mSmmS3ResumeState->ReturnContext2, > + (UINT32)mSmmS3ResumeState->ReturnStackPointer > + ); > } > -- --zHKTwmUM7eacyAUYDD2E Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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, Abdu= l Lateef Attar wrote:
---
.../AmdSmmCpuFeaturesLib.inf | 1 +
.../SmmCpuFeat= uresLib/Amd/SmramSaveState.h | 19 +++++++++++
.../SmmCpuFeaturesLib/Am= d/SmmCpuFeaturesLib.c | 32 +++++++++++++++++++
3 files changed, 52 ins= ertions(+)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd= SmmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeatu= resLib.inf
index 95eb31d16ead..7fd559e91ad8 100644
--- a/UefiCpuP= kg/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/SmramSav= eState.h b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmramSaveState.h
i= ndex 290ebdbc9227..474a5dbd9765 100644
--- a/UefiCpuPkg/Library/SmmCpu= FeaturesLib/Amd/SmramSaveState.h
+++ b/UefiCpuPkg/Library/SmmCpuFeatur= esLib/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/SmramS= aveStateMap.h>
+#include <Guid/AcpiS3Context.h>

//= EFER register LMA bit
#define LMA BIT10
@@ -106,4 +107,22 @@ Int= ernalSmmCpuFeaturesWriteSaveStateRegister (
IN CONST VOID *Buffer
);

+/**
+ Initialize MP synchronization data.
+**/+VOID
+EFIAPI
+InitializeMpSyncData (
+ VOID
+ );+
+/**
+ Perform SMM MP sync Semaphores re-initialization in t= he S3 boot path.
+**/
+VOID
+EFIAPI
+SmmS3MpSemaphoreIn= it (
+ VOID
+ );
+
#endif
diff --git a/UefiCpuPkg/= Library/SmmCpuFeaturesLib/Amd/SmmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmC= puFeaturesLib/Amd/SmmCpuFeaturesLib.c
index 10bed4116397..b855573d9401= 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Amd/SmmCpuFeaturesL= ib.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 mSmmSave= StateRegisterLma;

+// SMM S3 resume state Ptr
+extern SMM_S= 3_RESUME_STATE *mSmmS3ResumeState;
+
/**
Read an SMM Save St= ate register on the target processor. If this function
returns EFI_UNS= UPPORTED, then the caller is responsible for reading the
@@ -441,4 +44= 4,33 @@ SmmCpuFeaturesCompleteSmmReadyToLock (
VOID
)
{
+ if (mSmmS3ResumeState !=3D NULL ) {
+ mSmmS3ResumeState->SmmS3Re= sumeEntryPoint =3D (EFI_PHYSICAL_ADDRESS)(UINTN)SmmS3MpSemaphoreInit;
= + }
+}
+
+/**
+ Perform SMM MP sync Semaphores re-initi= alization in the S3 boot path.
+**/
+VOID
+EFIAPI
+SmmS= 3MpSemaphoreInit (
+ VOID
+ )
+{
+ InitializeMpSyncData= ();
+
+ DEBUG ((DEBUG_INFO, "SMM S3 Return CS =3D %x\n", mSmmS3R= esumeState->ReturnCs));
+ DEBUG ((DEBUG_INFO, "SMM S3 Return Entry = Point =3D %x\n", mSmmS3ResumeState->ReturnEntryPoint));
+ DEBUG ((D= EBUG_INFO, "SMM S3 Return Context1 =3D %x\n", mSmmS3ResumeState->ReturnC= ontext1));
+ DEBUG ((DEBUG_INFO, "SMM S3 Return Context2 =3D %x\n", mS= mmS3ResumeState->ReturnContext2));
+ DEBUG ((DEBUG_INFO, "SMM S3 Re= turn Stack Pointer =3D %x\n", mSmmS3ResumeState->ReturnStackPointer));+
+ AsmDisablePaging64 (
+ mSmmS3ResumeState->ReturnCs,+ (UINT32)mSmmS3ResumeState->ReturnEntryPoint,
+ (UINT32)mSmmS3R= esumeState->ReturnContext1,
+ (UINT32)mSmmS3ResumeState->ReturnC= ontext2,
+ (UINT32)mSmmS3ResumeState->ReturnStackPointer
+ );<= br />}
--
--zHKTwmUM7eacyAUYDD2E--