* [Patch v2 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: TransferApToSafeState() use UINTN params
2016-11-17 21:19 [Patch v2 0/2] Fix ACPI S3 resume failure with GCC 5.4 Michael Kinney
@ 2016-11-17 21:19 ` Michael Kinney
2016-11-17 21:19 ` [Patch v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Add volatile to mNumberToFinish Michael Kinney
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Michael Kinney @ 2016-11-17 21:19 UTC (permalink / raw)
To: edk2-devel; +Cc: Liming Gao, Laszlo Ersek, Andrew Fish, Jeff Fan
Update TransferApToSafeState() use UINTN params to reduce the
number of type casts required in these calls. Also change
the NumberToFinish parameter from UINT32* to UINTN
NumberToFinishAddress to resolve issues with conversion from
a volatile pointer to a non-volatile pointer. The assembly
code that receives the NumberToFinishAddress value must treat
that memory location as a volatile to track the number of APs.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney <michael.d.kinney@intel.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 8 ++++----
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c | 18 +++++++++---------
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 12 ++++++------
UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 18 +++++++++---------
4 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index 3fb6864..4af86b7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -385,7 +385,7 @@ MPRendezvousProcedure (
CPU_REGISTER_TABLE *RegisterTableList;
UINT32 InitApicId;
UINTN Index;
- UINT32 TopOfStack;
+ UINTN TopOfStack;
UINT8 Stack[128];
ProgramVirtualWireMode ();
@@ -403,10 +403,10 @@ MPRendezvousProcedure (
//
// Place AP into the safe code, count down the number with lock mechanism in the safe code.
//
- TopOfStack = (UINT32) (UINTN) Stack + sizeof (Stack);
- TopOfStack &= ~(UINT32) (CPU_STACK_ALIGNMENT - 1);
+ TopOfStack = (UINTN) Stack + sizeof (Stack);
+ TopOfStack &= ~(UINTN) (CPU_STACK_ALIGNMENT - 1);
CopyMem ((VOID *) (UINTN) mApHltLoopCode, mApHltLoopCodeTemplate, sizeof (mApHltLoopCodeTemplate));
- TransferApToSafeState ((UINT32) (UINTN) mApHltLoopCode, TopOfStack, &mNumberToFinish);
+ TransferApToSafeState ((UINTN)mApHltLoopCode, TopOfStack, (UINTN)&mNumberToFinish);
}
/**
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
index 4281698..f4db6c8 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
@@ -129,23 +129,23 @@ InitGdt (
/**
Transfer AP to safe hlt-loop after it finished restore CPU features on S3 patch.
- @param[in] ApHltLoopCode The 32-bit address of the safe hlt-loop function.
- @param[in] TopOfStack A pointer to the new stack to use for the ApHltLoopCode.
- @param[in] NumberToFinish Semaphore of APs finish count.
+ @param[in] ApHltLoopCode The address of the safe hlt-loop function.
+ @param[in] TopOfStack A pointer to the new stack to use for the ApHltLoopCode.
+ @param[in] NumberToFinishAddress Address of Semaphore of APs finish count.
**/
VOID
TransferApToSafeState (
- IN UINT32 ApHltLoopCode,
- IN UINT32 TopOfStack,
- IN UINT32 *NumberToFinish
+ IN UINTN ApHltLoopCode,
+ IN UINTN TopOfStack,
+ IN UINTN NumberToFinishAddress
)
{
SwitchStack (
- (SWITCH_STACK_ENTRY_POINT) (UINTN) ApHltLoopCode,
- NumberToFinish,
+ (SWITCH_STACK_ENTRY_POINT)ApHltLoopCode,
+ (VOID *)NumberToFinishAddress,
NULL,
- (VOID *) (UINTN) TopOfStack
+ (VOID *)TopOfStack
);
//
// It should never reach here
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 907526e..abe5cc6 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -974,16 +974,16 @@ GetAcpiS3EnableFlag (
/**
Transfer AP to safe hlt-loop after it finished restore CPU features on S3 patch.
- @param[in] ApHltLoopCode The 32-bit address of the safe hlt-loop function.
- @param[in] TopOfStack A pointer to the new stack to use for the ApHltLoopCode.
- @param[in] NumberToFinish Semaphore of APs finish count.
+ @param[in] ApHltLoopCode The address of the safe hlt-loop function.
+ @param[in] TopOfStack A pointer to the new stack to use for the ApHltLoopCode.
+ @param[in] NumberToFinishAddress Address of Semaphore of APs finish count.
**/
VOID
TransferApToSafeState (
- IN UINT32 ApHltLoopCode,
- IN UINT32 TopOfStack,
- IN UINT32 *NumberToFinish
+ IN UINTN ApHltLoopCode,
+ IN UINTN TopOfStack,
+ IN UINTN NumberToFinishAddress
);
#endif
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
index d6d6a1a..9fc00c1 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
@@ -129,24 +129,24 @@ GetProtectedModeCS (
/**
Transfer AP to safe hlt-loop after it finished restore CPU features on S3 patch.
- @param[in] ApHltLoopCode The 32-bit address of the safe hlt-loop function.
- @param[in] TopOfStack A pointer to the new stack to use for the ApHltLoopCode.
- @param[in] NumberToFinish Semaphore of APs finish count.
+ @param[in] ApHltLoopCode The address of the safe hlt-loop function.
+ @param[in] TopOfStack A pointer to the new stack to use for the ApHltLoopCode.
+ @param[in] NumberToFinishAddress Address of Semaphore of APs finish count.
**/
VOID
TransferApToSafeState (
- IN UINT32 ApHltLoopCode,
- IN UINT32 TopOfStack,
- IN UINT32 *NumberToFinish
+ IN UINTN ApHltLoopCode,
+ IN UINTN TopOfStack,
+ IN UINTN NumberToFinishAddress
)
{
AsmDisablePaging64 (
GetProtectedModeCS (),
- (UINT32) (UINTN) ApHltLoopCode,
- (UINT32) (UINTN) NumberToFinish,
+ (UINT32)ApHltLoopCode,
+ (UINT32)NumberToFinishAddress,
0,
- TopOfStack
+ (UINT32)TopOfStack
);
//
// It should never reach here
--
2.6.3.windows.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Patch v2 0/2] Fix ACPI S3 resume failure with GCC 5.4
2016-11-17 21:19 [Patch v2 0/2] Fix ACPI S3 resume failure with GCC 5.4 Michael Kinney
` (2 preceding siblings ...)
2016-11-17 23:46 ` [Patch v2 0/2] Fix ACPI S3 resume failure with GCC 5.4 Laszlo Ersek
@ 2016-11-18 0:48 ` Fan, Jeff
3 siblings, 0 replies; 5+ messages in thread
From: Fan, Jeff @ 2016-11-18 0:48 UTC (permalink / raw)
To: Kinney, Michael D, edk2-devel@lists.01.org
Cc: Gao, Liming, Laszlo Ersek, Andrew Fish
Reviewed-by: Jeff Fan <jeff.fan@intel.com>
Thanks!
-----Original Message-----
From: Kinney, Michael D
Sent: Friday, November 18, 2016 5:19 AM
To: edk2-devel@lists.01.org
Cc: Gao, Liming; Laszlo Ersek; Andrew Fish; Fan, Jeff
Subject: [Patch v2 0/2] Fix ACPI S3 resume failure with GCC 5.4
Add volatile qualifier to mNumberToFinish to prevent GCC 5.4 compiler from optimizing away required logic in ACPI S3 resume.
Update TransferApToSafeState() use UINTN params to reduce the number of type casts required in these calls and change the NumberToFinish parameter from UINT32* to UINTN NumberToFinishAddress to resolve compiler warnings from a volatile pointer to a non-volatile pointer conversion.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney <michael.d.kinney@intel.com>
Michael Kinney (2):
UefiCpuPkg/PiSmmCpuDxeSmm: TransferApToSafeState() use UINTN params
UefiCpuPkg/PiSmmCpuDxeSmm: Add volatile to mNumberToFinish
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 10 +++++-----
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c | 18 +++++++++---------
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 12 ++++++------
UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 18 +++++++++---------
4 files changed, 29 insertions(+), 29 deletions(-)
--
2.6.3.windows.1
^ permalink raw reply [flat|nested] 5+ messages in thread