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

2022-12-13 Thread Attar, AbdulLateef (Abdul Lateef) via groups.io
[AMD Official Use Only - General]

Hi Abner,
I’ll drop this patch from the patch list. We will investigate 
and submit a separate patch for handling the S3 save stage.
Thanks
AbduL

From: abner.chang via groups.io 
Sent: 08 December 2022 11:17
To: Attar, AbdulLateef (Abdul Lateef) ; 
devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v1 5/5] UefiCpuPkg/AmdSmmCpuFeaturesLib: 
Handles S3 save state

Caution: This message originated from an External Source. Use proper caution 
when opening attachments, clicking links, or responding.

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 
#include 
#include 
+#include 

// 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
+ );
}
--


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97328): https://edk2.groups.io/g/devel/message/97328
Mute This Topic: https://groups.io/mt/95492572/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




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

2022-12-07 Thread Chang, Abner via groups.io
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 
> #include 
> #include 
> +#include 
> 
> // 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
> + );
> }
> --


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97121): https://edk2.groups.io/g/devel/message/97121
Mute This Topic: https://groups.io/mt/95492572/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




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

2022-12-06 Thread Abdul Lateef Attar via groups.io
From: Abdul Lateef Attar 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4182

Handles S3 save state restore condition.
Implements SmmCpuFeaturesCompleteSmmReadyToLock() to
sync all processor and update S3 resume entry point.

Cc: Paul Grimes 
Cc: Garrett Kirkendall 
Cc: Abner Chang 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Signed-off-by: Abdul Lateef Attar 
---
 .../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 

 #include 

 #include 

+#include 

 

 // 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

+);

 }

-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97034): https://edk2.groups.io/g/devel/message/97034
Mute This Topic: https://groups.io/mt/95492572/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-