* [PATCH v2 1/5] UefiCpuPkg/DxeMpLib: Get safe AP loop handler from global variable
2016-11-25 6:03 [PATCH v2 0/5] UefiCpuPkg/DxeMpLib: Allocate new safe stack < 4GB Jeff Fan
@ 2016-11-25 6:03 ` Jeff Fan
2016-11-25 10:44 ` Laszlo Ersek
2016-11-25 6:03 ` [PATCH v2 2/5] UefiCpuPkg/DxeMpLib: Allocate new safe stack < 4GB Jeff Fan
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Jeff Fan @ 2016-11-25 6:03 UTC (permalink / raw)
To: edk2-devel; +Cc: Laszlo Ersek, Feng Tian, Michael D Kinney
AP loop function is already saved into global variable, needn't to get it from
AP function parameter.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 7f3900b..a0d5eeb 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -244,7 +244,7 @@ RelocateApLoop (
CpuMpData = GetCpuMpData ();
MwaitSupport = IsMwaitSupport ();
- AsmRelocateApLoopFunc = (ASM_RELOCATE_AP_LOOP) (UINTN) Buffer;
+ AsmRelocateApLoopFunc = (ASM_RELOCATE_AP_LOOP) (UINTN) mReservedApLoopFunc;
AsmRelocateApLoopFunc (MwaitSupport, CpuMpData->ApTargetCState, CpuMpData->PmCodeSegment);
//
// It should never reach here
@@ -273,7 +273,7 @@ MpInitChangeApLoopCallback (
CpuMpData->SaveRestoreFlag = TRUE;
CpuMpData->PmCodeSegment = GetProtectedModeCS ();
CpuMpData->ApLoopMode = PcdGet8 (PcdCpuApLoopMode);
- WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, mReservedApLoopFunc);
+ WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, NULL);
DEBUG ((DEBUG_INFO, "%a() done!\n", __FUNCTION__));
}
--
2.9.3.windows.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 1/5] UefiCpuPkg/DxeMpLib: Get safe AP loop handler from global variable
2016-11-25 6:03 ` [PATCH v2 1/5] UefiCpuPkg/DxeMpLib: Get safe AP loop handler from global variable Jeff Fan
@ 2016-11-25 10:44 ` Laszlo Ersek
0 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2016-11-25 10:44 UTC (permalink / raw)
To: Jeff Fan, edk2-devel; +Cc: Feng Tian, Michael D Kinney
On 11/25/16 07:03, Jeff Fan wrote:
> AP loop function is already saved into global variable, needn't to get it from
> AP function parameter.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan <jeff.fan@intel.com>
> ---
> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> index 7f3900b..a0d5eeb 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> @@ -244,7 +244,7 @@ RelocateApLoop (
>
> CpuMpData = GetCpuMpData ();
> MwaitSupport = IsMwaitSupport ();
> - AsmRelocateApLoopFunc = (ASM_RELOCATE_AP_LOOP) (UINTN) Buffer;
> + AsmRelocateApLoopFunc = (ASM_RELOCATE_AP_LOOP) (UINTN) mReservedApLoopFunc;
> AsmRelocateApLoopFunc (MwaitSupport, CpuMpData->ApTargetCState, CpuMpData->PmCodeSegment);
> //
> // It should never reach here
> @@ -273,7 +273,7 @@ MpInitChangeApLoopCallback (
> CpuMpData->SaveRestoreFlag = TRUE;
> CpuMpData->PmCodeSegment = GetProtectedModeCS ();
> CpuMpData->ApLoopMode = PcdGet8 (PcdCpuApLoopMode);
> - WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, mReservedApLoopFunc);
> + WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, NULL);
> DEBUG ((DEBUG_INFO, "%a() done!\n", __FUNCTION__));
> }
>
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/5] UefiCpuPkg/DxeMpLib: Allocate new safe stack < 4GB
2016-11-25 6:03 [PATCH v2 0/5] UefiCpuPkg/DxeMpLib: Allocate new safe stack < 4GB Jeff Fan
2016-11-25 6:03 ` [PATCH v2 1/5] UefiCpuPkg/DxeMpLib: Get safe AP loop handler from global variable Jeff Fan
@ 2016-11-25 6:03 ` Jeff Fan
2016-11-25 10:55 ` Laszlo Ersek
2016-11-25 6:03 ` [PATCH v2 3/5] UefiCpuPkg/DxeMpLib: Make sure APs in safe loop code Jeff Fan
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Jeff Fan @ 2016-11-25 6:03 UTC (permalink / raw)
To: edk2-devel; +Cc: Laszlo Ersek, Feng Tian, Michael D Kinney
For long mode DXE, we will disable paging on AP to protected mode to execute AP
safe loop code in reserved memory range under 4GB. But we forget to allocate
stack for AP under 4GB and AP still are using original AP stack. If original AP
stack is larger than 4GB, it cannot be used after AP is transferred to protected
mode. Besides MwaitSupport == TRUE, AP stack is still required during phase of
disabling paging in long mode DXE.
MMoreover, even though AP stack is always under 4GB (a) in Ia32 DXE and (b) with
this patch, after transferring to protected mode from X64 DXE, AP stack
(in BootServiceData) maybe crashed by OS after Exit Boot Service event.
This fix is to allocate reserved memory range under 4GB together with AP safe
loop code. APs will switch to new stack in safe loop code.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 19 +++++++++++++++++--
UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 13 ++++++++++---
UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 ++-
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 3 ++-
4 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index a0d5eeb..5a3b02c 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -18,6 +18,7 @@
#include <Library/UefiBootServicesTableLib.h>
#define AP_CHECK_INTERVAL (EFI_TIMER_PERIOD_MILLISECONDS (100))
+#define AP_SAFE_STACK_SIZE 128
CPU_MP_DATA *mCpuMpData = NULL;
EFI_EVENT mCheckAllApsEvent = NULL;
@@ -25,6 +26,7 @@ EFI_EVENT mMpInitExitBootServicesEvent = NULL;
EFI_EVENT mLegacyBootEvent = NULL;
volatile BOOLEAN mStopCheckAllApsStatus = TRUE;
VOID *mReservedApLoopFunc = NULL;
+UINTN mReservedTopOfApStack;
/**
Get the pointer to CPU MP Data structure.
@@ -241,11 +243,18 @@ RelocateApLoop (
CPU_MP_DATA *CpuMpData;
BOOLEAN MwaitSupport;
ASM_RELOCATE_AP_LOOP AsmRelocateApLoopFunc;
+ UINTN ProcessorNumber;
+ MpInitLibWhoAmI (&ProcessorNumber);
CpuMpData = GetCpuMpData ();
MwaitSupport = IsMwaitSupport ();
AsmRelocateApLoopFunc = (ASM_RELOCATE_AP_LOOP) (UINTN) mReservedApLoopFunc;
- AsmRelocateApLoopFunc (MwaitSupport, CpuMpData->ApTargetCState, CpuMpData->PmCodeSegment);
+ AsmRelocateApLoopFunc (
+ MwaitSupport,
+ CpuMpData->ApTargetCState,
+ CpuMpData->PmCodeSegment,
+ mReservedTopOfApStack - ProcessorNumber * AP_SAFE_STACK_SIZE
+ );
//
// It should never reach here
//
@@ -289,6 +298,7 @@ InitMpGlobalData (
{
EFI_STATUS Status;
EFI_PHYSICAL_ADDRESS Address;
+ UINTN ApSafeBufferSize;
SaveCpuMpData (CpuMpData);
@@ -307,16 +317,21 @@ InitMpGlobalData (
// Allocating it in advance since memory services are not available in
// Exit Boot Services callback function.
//
+ ApSafeBufferSize = CpuMpData->AddressMap.RelocateApLoopFuncSize;
+ ApSafeBufferSize += CpuMpData->CpuCount * AP_SAFE_STACK_SIZE;
+
Address = BASE_4GB - 1;
Status = gBS->AllocatePages (
AllocateMaxAddress,
EfiReservedMemoryType,
- EFI_SIZE_TO_PAGES (sizeof (CpuMpData->AddressMap.RelocateApLoopFuncSize)),
+ EFI_SIZE_TO_PAGES (ApSafeBufferSize),
&Address
);
ASSERT_EFI_ERROR (Status);
mReservedApLoopFunc = (VOID *) (UINTN) Address;
ASSERT (mReservedApLoopFunc != NULL);
+ mReservedTopOfApStack = (UINTN) Address + EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (ApSafeBufferSize));
+ ASSERT ((mReservedTopOfApStack & (UINTN)(CPU_STACK_ALIGNMENT - 1)) == 0);
CopyMem (
mReservedApLoopFunc,
CpuMpData->AddressMap.RelocateApLoopFuncAddress,
diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index 9067f78..7ab136b 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -215,19 +215,26 @@ CProcedureInvoke:
RendezvousFunnelProcEnd:
;-------------------------------------------------------------------------------------
-; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment);
+; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack);
;-------------------------------------------------------------------------------------
global ASM_PFX(AsmRelocateApLoop)
ASM_PFX(AsmRelocateApLoop):
AsmRelocateApLoopStart:
- cmp byte [esp + 4], 1
+ mov eax, esp
+ mov esp, [eax + 16] ; TopOfApStack
+ push dword [eax] ; push return address for stack trace
+ push ebp
+ mov ebp, esp
+ mov ebx, [eax + 8] ; ApTargetCState
+ mov ecx, [eax + 4] ; MwaitSupport
+ cmp cl, 1 ; Check mwait-monitor support
jnz HltLoop
MwaitLoop:
mov eax, esp
xor ecx, ecx
xor edx, edx
monitor
- mov eax, [esp + 8] ; Mwait Cx, Target C-State per eax[7:4]
+ mov eax, ebx ; Mwait Cx, Target C-State per eax[7:4]
shl eax, 4
mwait
jmp MwaitLoop
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index f73a469..e6dea18 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -250,7 +250,8 @@ VOID
(EFIAPI * ASM_RELOCATE_AP_LOOP) (
IN BOOLEAN MwaitSupport,
IN UINTN ApTargetCState,
- IN UINTN PmCodeSegment
+ IN UINTN PmCodeSegment,
+ IN UINTN TopOfApStack
);
/**
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index e7e7d80..7869970 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -222,11 +222,12 @@ CProcedureInvoke:
RendezvousFunnelProcEnd:
;-------------------------------------------------------------------------------------
-; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment);
+; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack);
;-------------------------------------------------------------------------------------
global ASM_PFX(AsmRelocateApLoop)
ASM_PFX(AsmRelocateApLoop):
AsmRelocateApLoopStart:
+ mov rsp, r9
push rcx
push rdx
--
2.9.3.windows.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 2/5] UefiCpuPkg/DxeMpLib: Allocate new safe stack < 4GB
2016-11-25 6:03 ` [PATCH v2 2/5] UefiCpuPkg/DxeMpLib: Allocate new safe stack < 4GB Jeff Fan
@ 2016-11-25 10:55 ` Laszlo Ersek
0 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2016-11-25 10:55 UTC (permalink / raw)
To: Jeff Fan, edk2-devel; +Cc: Feng Tian, Michael D Kinney
On 11/25/16 07:03, Jeff Fan wrote:
> For long mode DXE, we will disable paging on AP to protected mode to execute AP
> safe loop code in reserved memory range under 4GB. But we forget to allocate
> stack for AP under 4GB and AP still are using original AP stack. If original AP
> stack is larger than 4GB, it cannot be used after AP is transferred to protected
> mode. Besides MwaitSupport == TRUE, AP stack is still required during phase of
> disabling paging in long mode DXE.
>
> MMoreover, even though AP stack is always under 4GB (a) in Ia32 DXE and (b) with
Small typo in "MMoreover", can be fixed up before pushing.
Reviewed-by: Laszlo Ersek <lersek@redht.com>
Thanks
Laszlo
> this patch, after transferring to protected mode from X64 DXE, AP stack
> (in BootServiceData) maybe crashed by OS after Exit Boot Service event.
>
> This fix is to allocate reserved memory range under 4GB together with AP safe
> loop code. APs will switch to new stack in safe loop code.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan <jeff.fan@intel.com>
> ---
> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 19 +++++++++++++++++--
> UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 13 ++++++++++---
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 ++-
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 3 ++-
> 4 files changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> index a0d5eeb..5a3b02c 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> @@ -18,6 +18,7 @@
> #include <Library/UefiBootServicesTableLib.h>
>
> #define AP_CHECK_INTERVAL (EFI_TIMER_PERIOD_MILLISECONDS (100))
> +#define AP_SAFE_STACK_SIZE 128
>
> CPU_MP_DATA *mCpuMpData = NULL;
> EFI_EVENT mCheckAllApsEvent = NULL;
> @@ -25,6 +26,7 @@ EFI_EVENT mMpInitExitBootServicesEvent = NULL;
> EFI_EVENT mLegacyBootEvent = NULL;
> volatile BOOLEAN mStopCheckAllApsStatus = TRUE;
> VOID *mReservedApLoopFunc = NULL;
> +UINTN mReservedTopOfApStack;
>
> /**
> Get the pointer to CPU MP Data structure.
> @@ -241,11 +243,18 @@ RelocateApLoop (
> CPU_MP_DATA *CpuMpData;
> BOOLEAN MwaitSupport;
> ASM_RELOCATE_AP_LOOP AsmRelocateApLoopFunc;
> + UINTN ProcessorNumber;
>
> + MpInitLibWhoAmI (&ProcessorNumber);
> CpuMpData = GetCpuMpData ();
> MwaitSupport = IsMwaitSupport ();
> AsmRelocateApLoopFunc = (ASM_RELOCATE_AP_LOOP) (UINTN) mReservedApLoopFunc;
> - AsmRelocateApLoopFunc (MwaitSupport, CpuMpData->ApTargetCState, CpuMpData->PmCodeSegment);
> + AsmRelocateApLoopFunc (
> + MwaitSupport,
> + CpuMpData->ApTargetCState,
> + CpuMpData->PmCodeSegment,
> + mReservedTopOfApStack - ProcessorNumber * AP_SAFE_STACK_SIZE
> + );
> //
> // It should never reach here
> //
> @@ -289,6 +298,7 @@ InitMpGlobalData (
> {
> EFI_STATUS Status;
> EFI_PHYSICAL_ADDRESS Address;
> + UINTN ApSafeBufferSize;
>
> SaveCpuMpData (CpuMpData);
>
> @@ -307,16 +317,21 @@ InitMpGlobalData (
> // Allocating it in advance since memory services are not available in
> // Exit Boot Services callback function.
> //
> + ApSafeBufferSize = CpuMpData->AddressMap.RelocateApLoopFuncSize;
> + ApSafeBufferSize += CpuMpData->CpuCount * AP_SAFE_STACK_SIZE;
> +
> Address = BASE_4GB - 1;
> Status = gBS->AllocatePages (
> AllocateMaxAddress,
> EfiReservedMemoryType,
> - EFI_SIZE_TO_PAGES (sizeof (CpuMpData->AddressMap.RelocateApLoopFuncSize)),
> + EFI_SIZE_TO_PAGES (ApSafeBufferSize),
> &Address
> );
> ASSERT_EFI_ERROR (Status);
> mReservedApLoopFunc = (VOID *) (UINTN) Address;
> ASSERT (mReservedApLoopFunc != NULL);
> + mReservedTopOfApStack = (UINTN) Address + EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (ApSafeBufferSize));
> + ASSERT ((mReservedTopOfApStack & (UINTN)(CPU_STACK_ALIGNMENT - 1)) == 0);
> CopyMem (
> mReservedApLoopFunc,
> CpuMpData->AddressMap.RelocateApLoopFuncAddress,
> diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> index 9067f78..7ab136b 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> @@ -215,19 +215,26 @@ CProcedureInvoke:
> RendezvousFunnelProcEnd:
>
> ;-------------------------------------------------------------------------------------
> -; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment);
> +; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack);
> ;-------------------------------------------------------------------------------------
> global ASM_PFX(AsmRelocateApLoop)
> ASM_PFX(AsmRelocateApLoop):
> AsmRelocateApLoopStart:
> - cmp byte [esp + 4], 1
> + mov eax, esp
> + mov esp, [eax + 16] ; TopOfApStack
> + push dword [eax] ; push return address for stack trace
> + push ebp
> + mov ebp, esp
> + mov ebx, [eax + 8] ; ApTargetCState
> + mov ecx, [eax + 4] ; MwaitSupport
> + cmp cl, 1 ; Check mwait-monitor support
> jnz HltLoop
> MwaitLoop:
> mov eax, esp
> xor ecx, ecx
> xor edx, edx
> monitor
> - mov eax, [esp + 8] ; Mwait Cx, Target C-State per eax[7:4]
> + mov eax, ebx ; Mwait Cx, Target C-State per eax[7:4]
> shl eax, 4
> mwait
> jmp MwaitLoop
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index f73a469..e6dea18 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -250,7 +250,8 @@ VOID
> (EFIAPI * ASM_RELOCATE_AP_LOOP) (
> IN BOOLEAN MwaitSupport,
> IN UINTN ApTargetCState,
> - IN UINTN PmCodeSegment
> + IN UINTN PmCodeSegment,
> + IN UINTN TopOfApStack
> );
>
> /**
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> index e7e7d80..7869970 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> @@ -222,11 +222,12 @@ CProcedureInvoke:
> RendezvousFunnelProcEnd:
>
> ;-------------------------------------------------------------------------------------
> -; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment);
> +; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack);
> ;-------------------------------------------------------------------------------------
> global ASM_PFX(AsmRelocateApLoop)
> ASM_PFX(AsmRelocateApLoop):
> AsmRelocateApLoopStart:
> + mov rsp, r9
> push rcx
> push rdx
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/5] UefiCpuPkg/DxeMpLib: Make sure APs in safe loop code
2016-11-25 6:03 [PATCH v2 0/5] UefiCpuPkg/DxeMpLib: Allocate new safe stack < 4GB Jeff Fan
2016-11-25 6:03 ` [PATCH v2 1/5] UefiCpuPkg/DxeMpLib: Get safe AP loop handler from global variable Jeff Fan
2016-11-25 6:03 ` [PATCH v2 2/5] UefiCpuPkg/DxeMpLib: Allocate new safe stack < 4GB Jeff Fan
@ 2016-11-25 6:03 ` Jeff Fan
2016-11-25 11:06 ` Laszlo Ersek
2016-11-25 6:03 ` [PATCH v2 4/5] UefiCpuPkg/DxeMpLib: Fix bug when getting target C-State from eax Jeff Fan
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Jeff Fan @ 2016-11-25 6:03 UTC (permalink / raw)
To: edk2-devel; +Cc: Laszlo Ersek, Feng Tian, Michael D Kinney
Add one semaphore to make sure BSP to wait till all APs run in AP safe loop
code.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 8 +++++++-
UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 4 +++-
UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 ++-
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 4 +++-
4 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 5a3b02c..8f5074b 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -27,6 +27,7 @@ EFI_EVENT mLegacyBootEvent = NULL;
volatile BOOLEAN mStopCheckAllApsStatus = TRUE;
VOID *mReservedApLoopFunc = NULL;
UINTN mReservedTopOfApStack;
+volatile UINT32 mNumberToFinish = 0;
/**
Get the pointer to CPU MP Data structure.
@@ -253,7 +254,8 @@ RelocateApLoop (
MwaitSupport,
CpuMpData->ApTargetCState,
CpuMpData->PmCodeSegment,
- mReservedTopOfApStack - ProcessorNumber * AP_SAFE_STACK_SIZE
+ mReservedTopOfApStack - ProcessorNumber * AP_SAFE_STACK_SIZE,
+ (UINTN) &mNumberToFinish
);
//
// It should never reach here
@@ -282,7 +284,11 @@ MpInitChangeApLoopCallback (
CpuMpData->SaveRestoreFlag = TRUE;
CpuMpData->PmCodeSegment = GetProtectedModeCS ();
CpuMpData->ApLoopMode = PcdGet8 (PcdCpuApLoopMode);
+ mNumberToFinish = CpuMpData->CpuCount - 1;
WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, NULL);
+ while (mNumberToFinish > 0) {
+ CpuPause ();
+ }
DEBUG ((DEBUG_INFO, "%a() done!\n", __FUNCTION__));
}
diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index 7ab136b..a48f0bc 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -215,7 +215,7 @@ CProcedureInvoke:
RendezvousFunnelProcEnd:
;-------------------------------------------------------------------------------------
-; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack);
+; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish);
;-------------------------------------------------------------------------------------
global ASM_PFX(AsmRelocateApLoop)
ASM_PFX(AsmRelocateApLoop):
@@ -227,6 +227,8 @@ AsmRelocateApLoopStart:
mov ebp, esp
mov ebx, [eax + 8] ; ApTargetCState
mov ecx, [eax + 4] ; MwaitSupport
+ mov eax, [eax + 20] ; CountTofinish
+ lock dec dword [eax] ; (*CountTofinish)--
cmp cl, 1 ; Check mwait-monitor support
jnz HltLoop
MwaitLoop:
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index e6dea18..49305ad 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -251,7 +251,8 @@ VOID
IN BOOLEAN MwaitSupport,
IN UINTN ApTargetCState,
IN UINTN PmCodeSegment,
- IN UINTN TopOfApStack
+ IN UINTN TopOfApStack,
+ IN UINTN NumberToFinish
);
/**
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 7869970..f8f4712 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -222,11 +222,13 @@ CProcedureInvoke:
RendezvousFunnelProcEnd:
;-------------------------------------------------------------------------------------
-; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack);
+; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish);
;-------------------------------------------------------------------------------------
global ASM_PFX(AsmRelocateApLoop)
ASM_PFX(AsmRelocateApLoop):
AsmRelocateApLoopStart:
+ mov rax, [rsp + 40] ; CountTofinish
+ lock dec dword [rax] ; (*CountTofinish)--
mov rsp, r9
push rcx
push rdx
--
2.9.3.windows.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 3/5] UefiCpuPkg/DxeMpLib: Make sure APs in safe loop code
2016-11-25 6:03 ` [PATCH v2 3/5] UefiCpuPkg/DxeMpLib: Make sure APs in safe loop code Jeff Fan
@ 2016-11-25 11:06 ` Laszlo Ersek
0 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2016-11-25 11:06 UTC (permalink / raw)
To: Jeff Fan, edk2-devel; +Cc: Feng Tian, Michael D Kinney
On 11/25/16 07:03, Jeff Fan wrote:
> Add one semaphore to make sure BSP to wait till all APs run in AP safe loop
> code.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan <jeff.fan@intel.com>
> ---
> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 8 +++++++-
> UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 4 +++-
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 ++-
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 4 +++-
> 4 files changed, 15 insertions(+), 4 deletions(-)
Looks good to me.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> index 5a3b02c..8f5074b 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> @@ -27,6 +27,7 @@ EFI_EVENT mLegacyBootEvent = NULL;
> volatile BOOLEAN mStopCheckAllApsStatus = TRUE;
> VOID *mReservedApLoopFunc = NULL;
> UINTN mReservedTopOfApStack;
> +volatile UINT32 mNumberToFinish = 0;
>
> /**
> Get the pointer to CPU MP Data structure.
> @@ -253,7 +254,8 @@ RelocateApLoop (
> MwaitSupport,
> CpuMpData->ApTargetCState,
> CpuMpData->PmCodeSegment,
> - mReservedTopOfApStack - ProcessorNumber * AP_SAFE_STACK_SIZE
> + mReservedTopOfApStack - ProcessorNumber * AP_SAFE_STACK_SIZE,
> + (UINTN) &mNumberToFinish
> );
> //
> // It should never reach here
> @@ -282,7 +284,11 @@ MpInitChangeApLoopCallback (
> CpuMpData->SaveRestoreFlag = TRUE;
> CpuMpData->PmCodeSegment = GetProtectedModeCS ();
> CpuMpData->ApLoopMode = PcdGet8 (PcdCpuApLoopMode);
> + mNumberToFinish = CpuMpData->CpuCount - 1;
> WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, NULL);
> + while (mNumberToFinish > 0) {
> + CpuPause ();
> + }
> DEBUG ((DEBUG_INFO, "%a() done!\n", __FUNCTION__));
> }
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> index 7ab136b..a48f0bc 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> @@ -215,7 +215,7 @@ CProcedureInvoke:
> RendezvousFunnelProcEnd:
>
> ;-------------------------------------------------------------------------------------
> -; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack);
> +; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish);
> ;-------------------------------------------------------------------------------------
> global ASM_PFX(AsmRelocateApLoop)
> ASM_PFX(AsmRelocateApLoop):
> @@ -227,6 +227,8 @@ AsmRelocateApLoopStart:
> mov ebp, esp
> mov ebx, [eax + 8] ; ApTargetCState
> mov ecx, [eax + 4] ; MwaitSupport
> + mov eax, [eax + 20] ; CountTofinish
> + lock dec dword [eax] ; (*CountTofinish)--
> cmp cl, 1 ; Check mwait-monitor support
> jnz HltLoop
> MwaitLoop:
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index e6dea18..49305ad 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -251,7 +251,8 @@ VOID
> IN BOOLEAN MwaitSupport,
> IN UINTN ApTargetCState,
> IN UINTN PmCodeSegment,
> - IN UINTN TopOfApStack
> + IN UINTN TopOfApStack,
> + IN UINTN NumberToFinish
> );
>
> /**
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> index 7869970..f8f4712 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> @@ -222,11 +222,13 @@ CProcedureInvoke:
> RendezvousFunnelProcEnd:
>
> ;-------------------------------------------------------------------------------------
> -; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack);
> +; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish);
> ;-------------------------------------------------------------------------------------
> global ASM_PFX(AsmRelocateApLoop)
> ASM_PFX(AsmRelocateApLoop):
> AsmRelocateApLoopStart:
> + mov rax, [rsp + 40] ; CountTofinish
> + lock dec dword [rax] ; (*CountTofinish)--
> mov rsp, r9
> push rcx
> push rdx
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 4/5] UefiCpuPkg/DxeMpLib: Fix bug when getting target C-State from eax
2016-11-25 6:03 [PATCH v2 0/5] UefiCpuPkg/DxeMpLib: Allocate new safe stack < 4GB Jeff Fan
` (2 preceding siblings ...)
2016-11-25 6:03 ` [PATCH v2 3/5] UefiCpuPkg/DxeMpLib: Make sure APs in safe loop code Jeff Fan
@ 2016-11-25 6:03 ` Jeff Fan
2016-11-25 11:09 ` Laszlo Ersek
2016-11-25 6:03 ` [PATCH v2 5/5] UefiCpuPkg/DxeMpLib: Remove unnecessary ret instruction Jeff Fan
2016-11-25 11:53 ` [PATCH v2 0/5] UefiCpuPkg/DxeMpLib: Allocate new safe stack < 4GB Laszlo Ersek
5 siblings, 1 reply; 12+ messages in thread
From: Jeff Fan @ 2016-11-25 6:03 UTC (permalink / raw)
To: edk2-devel; +Cc: Laszlo Ersek, Feng Tian, Michael D Kinney
AP will get target C-State from eax[7:4]. We do shift in ebx firstly before set
to eax. It will lead ebx is correct in the next time.
The fix is to set ebx to eax firstly and does shift in eax. Thus, ebx could keep
original value.
Reported-by: Laszlo Ersek <lersek@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index f8f4712..34c07a6 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -266,8 +266,8 @@ MwaitLoop:
xor ecx, ecx ; ecx = 0
xor edx, edx ; edx = 0
monitor
- shl ebx, 4
mov eax, ebx ; Mwait Cx, Target C-State per eax[7:4]
+ shl eax, 4
mwait
jmp MwaitLoop
HltLoop:
--
2.9.3.windows.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 4/5] UefiCpuPkg/DxeMpLib: Fix bug when getting target C-State from eax
2016-11-25 6:03 ` [PATCH v2 4/5] UefiCpuPkg/DxeMpLib: Fix bug when getting target C-State from eax Jeff Fan
@ 2016-11-25 11:09 ` Laszlo Ersek
0 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2016-11-25 11:09 UTC (permalink / raw)
To: Jeff Fan, edk2-devel; +Cc: Feng Tian, Michael D Kinney
On 11/25/16 07:03, Jeff Fan wrote:
> AP will get target C-State from eax[7:4]. We do shift in ebx firstly before set
> to eax. It will lead ebx is correct in the next time.
Please replace the word "correct" with "incorrect" in the commit message
-- without the patch, ebx is wrong at the second and further iterations.
With that word changed:
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!
Laszlo
>
> The fix is to set ebx to eax firstly and does shift in eax. Thus, ebx could keep
> original value.
>
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan <jeff.fan@intel.com>
> ---
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> index f8f4712..34c07a6 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> @@ -266,8 +266,8 @@ MwaitLoop:
> xor ecx, ecx ; ecx = 0
> xor edx, edx ; edx = 0
> monitor
> - shl ebx, 4
> mov eax, ebx ; Mwait Cx, Target C-State per eax[7:4]
> + shl eax, 4
> mwait
> jmp MwaitLoop
> HltLoop:
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 5/5] UefiCpuPkg/DxeMpLib: Remove unnecessary ret instruction
2016-11-25 6:03 [PATCH v2 0/5] UefiCpuPkg/DxeMpLib: Allocate new safe stack < 4GB Jeff Fan
` (3 preceding siblings ...)
2016-11-25 6:03 ` [PATCH v2 4/5] UefiCpuPkg/DxeMpLib: Fix bug when getting target C-State from eax Jeff Fan
@ 2016-11-25 6:03 ` Jeff Fan
2016-11-25 11:11 ` Laszlo Ersek
2016-11-25 11:53 ` [PATCH v2 0/5] UefiCpuPkg/DxeMpLib: Allocate new safe stack < 4GB Laszlo Ersek
5 siblings, 1 reply; 12+ messages in thread
From: Jeff Fan @ 2016-11-25 6:03 UTC (permalink / raw)
To: edk2-devel; +Cc: Laszlo Ersek, Feng Tian, Michael D Kinney
Reported-by: Laszlo Ersek <lersek@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 1 -
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 1 -
2 files changed, 2 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index a48f0bc..52363e6 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -244,7 +244,6 @@ HltLoop:
cli
hlt
jmp HltLoop
- ret
AsmRelocateApLoopEnd:
;-------------------------------------------------------------------------------------
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 34c07a6..fa54d01 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -274,7 +274,6 @@ HltLoop:
cli
hlt
jmp HltLoop
- ret
BITS 64
AsmRelocateApLoopEnd:
--
2.9.3.windows.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 5/5] UefiCpuPkg/DxeMpLib: Remove unnecessary ret instruction
2016-11-25 6:03 ` [PATCH v2 5/5] UefiCpuPkg/DxeMpLib: Remove unnecessary ret instruction Jeff Fan
@ 2016-11-25 11:11 ` Laszlo Ersek
0 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2016-11-25 11:11 UTC (permalink / raw)
To: Jeff Fan, edk2-devel; +Cc: Feng Tian, Michael D Kinney
On 11/25/16 07:03, Jeff Fan wrote:
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan <jeff.fan@intel.com>
> ---
> UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 1 -
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 1 -
> 2 files changed, 2 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> index a48f0bc..52363e6 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> @@ -244,7 +244,6 @@ HltLoop:
> cli
> hlt
> jmp HltLoop
> - ret
> AsmRelocateApLoopEnd:
>
> ;-------------------------------------------------------------------------------------
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> index 34c07a6..fa54d01 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> @@ -274,7 +274,6 @@ HltLoop:
> cli
> hlt
> jmp HltLoop
> - ret
> BITS 64
> AsmRelocateApLoopEnd:
>
>
Right, thank you for removing the superfluous RET from the Ia32 code as
well, my report covered the X64 case only, and I noticed Ia32 only later.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Please hold off committing the series just yet, I'd like to test it as
well. I'll report back with results.
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/5] UefiCpuPkg/DxeMpLib: Allocate new safe stack < 4GB
2016-11-25 6:03 [PATCH v2 0/5] UefiCpuPkg/DxeMpLib: Allocate new safe stack < 4GB Jeff Fan
` (4 preceding siblings ...)
2016-11-25 6:03 ` [PATCH v2 5/5] UefiCpuPkg/DxeMpLib: Remove unnecessary ret instruction Jeff Fan
@ 2016-11-25 11:53 ` Laszlo Ersek
5 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2016-11-25 11:53 UTC (permalink / raw)
To: Jeff Fan, edk2-devel; +Cc: Feng Tian, Michael D Kinney
On 11/25/16 07:03, Jeff Fan wrote:
> Allocate safe AP stack under 4GB and make sure BSP wait till all APs running in
> safe code.
>
> v2:
> 1. Update #1 to address the comments in
> https://lists.01.org/pipermail/edk2-devel/2016-November/005136.html
> 2. Update #2 to address the comments in
> https://lists.01.org/pipermail/edk2-devel/2016-November/005137.html
> 3. Update #3 to address the comments in
> https://lists.01.org/pipermail/edk2-devel/2016-November/005138.html
> 4. Add #4 to fix getting target C-State bug.
> 5. Add #5 to remove ret instruction.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan <jeff.fan@intel.com>
>
> Jeff Fan (5):
> UefiCpuPkg/DxeMpLib: Get safe AP loop handler from global variable
> UefiCpuPkg/DxeMpLib: Allocate new safe stack < 4GB
> UefiCpuPkg/DxeMpLib: Make sure APs in safe loop code
> UefiCpuPkg/DxeMpLib: Fix bug when getting target C-State from eax
> UefiCpuPkg/DxeMpLib: Remove unnecessary ret instruction
>
> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 29 ++++++++++++++++++++++----
> UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 16 ++++++++++----
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 4 +++-
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 8 ++++---
> 4 files changed, 45 insertions(+), 12 deletions(-)
>
Tested-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread