Chen,
It seems that you fixed ASSERT issue by getting rid of AcquireSpinLock ().
Could you use the following code to take a try?
while (!AcquireSpinLockOrFail (&CpuData->CpuDataLock)) {
CpuPause();
}
I suspect the ASSERT is caused by TimerLib services consumed by
AcquireSpinLock() implementation.
Thanks!
Jeff
-----Original Message-----
From: Chen Fan [mailto:[email protected]]
Sent: Thursday, September 18, 2014 11:12 AM
To: [email protected]
Cc: Jordan Justen; Fan, Jeff; Andrew Fish
Subject: [RFC v3 16/19] UefiCpuPkg/CpuDxe: introduce CPU bus lock for sync data
Because of AcquireSpinLock()/ReleaseSpinLock() was not MP safe, We should
introduce CPU bus lock to replace them.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chen Fan <[email protected]>
---
UefiCpuPkg/CpuDxe/CpuDxe.inf | 1 -
UefiCpuPkg/CpuDxe/CpuMp.c | 33 ++++++++++++++++-----------------
UefiCpuPkg/CpuDxe/CpuMp.h | 24 ++++++++++++++++++++++--
UefiCpuPkg/CpuDxe/Ia32/MpAsm.nasm | 31 +++++++++++++++++++++++++++++++
UefiCpuPkg/CpuDxe/X64/MpAsm.nasm | 31 +++++++++++++++++++++++++++++++
UefiCpuPkg/UefiCpuPkg.dsc | 1 -
6 files changed, 100 insertions(+), 21 deletions(-)
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.inf b/UefiCpuPkg/CpuDxe/CpuDxe.inf index
ed90ff2..f73b845 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.inf
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.inf
@@ -41,7 +41,6 @@
UefiCpuLib
UefiLib
CpuExceptionHandlerLib
- SynchronizationLib
[Sources]
ApStartup.c
diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c index
2ba1e28..bcf5106 100644
--- a/UefiCpuPkg/CpuDxe/CpuMp.c
+++ b/UefiCpuPkg/CpuDxe/CpuMp.c
@@ -78,9 +78,9 @@ GetApState (
{
CPU_STATE State;
- AcquireSpinLock (&CpuData->CpuDataLock);
+ AsmApAcquireSpinLock ();
State = CpuData->State;
- ReleaseSpinLock (&CpuData->CpuDataLock);
+ AsmApReleaseSpinLock ();
return State;
}
@@ -98,9 +98,9 @@ SetApState (
IN CPU_STATE State
)
{
- AcquireSpinLock (&CpuData->CpuDataLock);
+ AsmApAcquireSpinLock ();
CpuData->State = State;
- ReleaseSpinLock (&CpuData->CpuDataLock);
+ AsmApReleaseSpinLock ();
}
/**
@@ -119,10 +119,10 @@ SetApProcedure (
IN VOID *ProcedureArgument
)
{
- AcquireSpinLock (&CpuData->CpuDataLock);
+ AsmApAcquireSpinLock ();
CpuData->Parameter = ProcedureArgument;
CpuData->Procedure = Procedure;
- ReleaseSpinLock (&CpuData->CpuDataLock);
+ AsmApReleaseSpinLock ();
}
/**
@@ -143,9 +143,9 @@ TestCpuStatusFlag (
{
UINT32 Ret;
- AcquireSpinLock (&CpuData->CpuDataLock);
+ AsmApAcquireSpinLock ();
Ret = CpuData->Info.StatusFlag & Flags;
- ReleaseSpinLock (&CpuData->CpuDataLock);
+ AsmApReleaseSpinLock ();
return !!(Ret);
}
@@ -163,9 +163,9 @@ CpuStatusFlagOr (
IN UINT32 Flags
)
{
- AcquireSpinLock (&CpuData->CpuDataLock);
+ AsmApAcquireSpinLock ();
CpuData->Info.StatusFlag |= Flags;
- ReleaseSpinLock (&CpuData->CpuDataLock);
+ AsmApReleaseSpinLock ();
}
/**
@@ -181,9 +181,9 @@ CpuStatusFlagAndNot (
IN UINT32 Flags
)
{
- AcquireSpinLock (&CpuData->CpuDataLock);
+ AsmApAcquireSpinLock ();
CpuData->Info.StatusFlag &= ~Flags;
- ReleaseSpinLock (&CpuData->CpuDataLock);
+ AsmApReleaseSpinLock ();
}
/**
@@ -671,19 +671,19 @@ ApDoLoop (
CpuData = (CPU_DATA_BLOCK *)Context;
while (TRUE) {
- AcquireSpinLock (&CpuData->CpuDataLock);
+ AsmApAcquireSpinLock ();
ProcedureArgument = CpuData->Parameter;
Procedure = CpuData->Procedure;
- ReleaseSpinLock (&CpuData->CpuDataLock);
+ AsmApReleaseSpinLock ();
if (Procedure != NULL) {
SetApState (CpuData, CPU_STATE_BUSY);
Procedure (ProcedureArgument);
- AcquireSpinLock (&CpuData->CpuDataLock);
+ AsmApAcquireSpinLock ();
CpuData->Procedure = NULL;
- ReleaseSpinLock (&CpuData->CpuDataLock);
+ AsmApReleaseSpinLock ();
SetApState (CpuData, CPU_STATE_FINISHED);
}
@@ -766,7 +766,6 @@ InitMpSystemData (
CpuData->Info.StatusFlag |= PROCESSOR_AS_BSP_BIT;
CpuData->Info.ProcessorId = GetApicId ();
}
- InitializeSpinLock (&CpuData->CpuDataLock);
Status = gBS->CreateEvent (
EVT_TIMER | EVT_NOTIFY_SIGNAL, diff --git
a/UefiCpuPkg/CpuDxe/CpuMp.h b/UefiCpuPkg/CpuDxe/CpuMp.h index 411fe81..236fd22
100644
--- a/UefiCpuPkg/CpuDxe/CpuMp.h
+++ b/UefiCpuPkg/CpuDxe/CpuMp.h
@@ -16,7 +16,6 @@
#define _CPU_MP_H_
#include <Protocol/MpService.h>
-#include <Library/SynchronizationLib.h>
#define AP_STACK_SIZE SIZE_64KB
@@ -110,6 +109,28 @@ AsmApReleaseLock (
VOID
);
+/**
+ the function added a CPU bus lock to protect the same data location
+ in multi processors environment.
+
+**/
+VOID
+EFIAPI
+AsmApAcquireSpinLock (
+ VOID
+ );
+
+/**
+ release the CPU bus lock.
+
+**/
+VOID
+EFIAPI
+AsmApReleaseSpinLock (
+ VOID
+ );
+
+
typedef enum {
CPU_STATE_IDLE,
CPU_STATE_BLOCKED,
@@ -124,7 +145,6 @@ typedef enum {
**/
typedef struct {
EFI_PROCESSOR_INFORMATION Info;
- SPIN_LOCK CpuDataLock;
CPU_STATE volatile State;
EFI_AP_PROCEDURE Procedure;
diff --git a/UefiCpuPkg/CpuDxe/Ia32/MpAsm.nasm
b/UefiCpuPkg/CpuDxe/Ia32/MpAsm.nasm
index 154b3a5..de7d3a4 100644
--- a/UefiCpuPkg/CpuDxe/Ia32/MpAsm.nasm
+++ b/UefiCpuPkg/CpuDxe/Ia32/MpAsm.nasm
@@ -24,6 +24,9 @@ extern ASM_PFX(mCpuExchangeData)
ApStackLock:
dd 0
+ApSpinLock:
+ dd 0
+
;------------------------------------------------------------------------------
; VOID
; EFIAPI
@@ -111,3 +114,31 @@ ASM_PFX(AsmApReleaseLock):
lock btc dword[ApStackLock], 0
ret
+;----------------------------------------------------------------------
+--------
+; VOID
+; EFIAPI
+; AsmApAcquireSpinLock (
+; VOID
+; );
+;----------------------------------------------------------------------
+--------
+global ASM_PFX(AsmApAcquireSpinLock)
+ASM_PFX(AsmApAcquireSpinLock):
+AsmApSpinLock:
+lock bts dword[ApSpinLock], 0
+ pause
+ jc AsmApSpinLock
+
+ ret
+
+;----------------------------------------------------------------------
+--------
+; VOID
+; EFIAPI
+; AsmApReleaseSpinLock (
+; VOID
+; );
+;----------------------------------------------------------------------
+--------
+global ASM_PFX(AsmApReleaseSpinLock)
+ASM_PFX(AsmApReleaseSpinLock):
+lock btc dword[ApSpinLock], 0
+ ret
+
diff --git a/UefiCpuPkg/CpuDxe/X64/MpAsm.nasm b/UefiCpuPkg/CpuDxe/X64/MpAsm.nasm
index 46ed107..dec9ee7 100644
--- a/UefiCpuPkg/CpuDxe/X64/MpAsm.nasm
+++ b/UefiCpuPkg/CpuDxe/X64/MpAsm.nasm
@@ -24,6 +24,9 @@ extern ASM_PFX(mCpuExchangeData)
ApStackLock:
dd 0
+ApSpinLock:
+ dd 0
+
;------------------------------------------------------------------------------
; VOID
; EFIAPI
@@ -109,3 +112,31 @@ ASM_PFX(AsmApReleaseLock):
lock btc dword[ApStackLock], 0
ret
+;----------------------------------------------------------------------
+--------
+; VOID
+; EFIAPI
+; AsmApAcquireSpinLock (
+; VOID
+; );
+;----------------------------------------------------------------------
+--------
+global ASM_PFX(AsmApAcquireSpinLock)
+ASM_PFX(AsmApAcquireSpinLock):
+AsmApSpinLock:
+lock bts dword[ApSpinLock], 0
+ pause
+ jc AsmApSpinLock
+
+ ret
+
+;----------------------------------------------------------------------
+--------
+; VOID
+; EFIAPI
+; AsmApReleaseSpinLock (
+; VOID
+; );
+;----------------------------------------------------------------------
+--------
+global ASM_PFX(AsmApReleaseSpinLock)
+ASM_PFX(AsmApReleaseSpinLock):
+lock btc dword[ApSpinLock], 0
+ ret
+
diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc index
9fa9270..70d5bb0 100644
--- a/UefiCpuPkg/UefiCpuPkg.dsc
+++ b/UefiCpuPkg/UefiCpuPkg.dsc
@@ -52,7 +52,6 @@
LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
CpuExceptionHandlerLib|MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.inf
-
SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
[LibraryClasses.common.PEIM]
MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
--
1.9.3
------------------------------------------------------------------------------
Want excitement?
Manually upgrade your production database.
When you want reliability, choose Perforce
Perforce version control. Predictably reliable.
http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
