Agreed. We've seen issues on real platforms with timed-out spinlocks in
DXE causing calls to GetPerformanceCounter and DebugAssert. (DXE has
the same code, with the same issues.)
Note that it's possible to set PcdSpinLockTimeout=0 to work around the
issue on a particular platform, or in a particular module. But if you
have to do that for every module which uses APs, and hence could contend
for a spinlock, it kind of defeats the point.... We're better off
removing the timeout code.
Thanks,
Brian
On 12/19/18 8:08 PM, Yao, Jiewen wrote:
Yes, I agree, if we don't have any real case.
-----Original Message-----
From: Ni, Ruiyu
Sent: Thursday, December 20, 2018 10:07 AM
To: Dong, Eric <[email protected]>; Yao, Jiewen
<[email protected]>; [email protected]
Cc: Laszlo Ersek <[email protected]>
Subject: RE: [edk2] [Patch 1/3] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid
AP calls PeiService.
Can you just change the AcquireSpinLock() behavior to remove the Timeout
PCD consumption?
I haven't seen a real case that the timed acquisition of spin lock is needed.
Thanks/Ray
-----Original Message-----
From: Dong, Eric <[email protected]>
Sent: Thursday, December 20, 2018 9:23 AM
To: Yao, Jiewen <[email protected]>; [email protected]
Cc: Ni, Ruiyu <[email protected]>; Laszlo Ersek <[email protected]>
Subject: RE: [edk2] [Patch 1/3] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid
AP calls PeiService.
Agreed, Maybe it's time to add a new API like
AcquireSpinLockWithoutTimeOut?
Thanks,
Eric
-----Original Message-----
From: Yao, Jiewen
Sent: Thursday, December 20, 2018 9:19 AM
To: Dong, Eric <[email protected]>; [email protected]
Cc: Ni, Ruiyu <[email protected]>; Laszlo Ersek <[email protected]>
Subject: RE: [edk2] [Patch 1/3] UefiCpuPkg/RegisterCpuFeaturesLib:
Avoid AP calls PeiService.
Hi
If we think below code is generic, can we have an API for that?
+ //
+ // Wait for the AP to release the MSR spin lock.
+ //
+ while (!AcquireSpinLockOrFail (&CpuFlags->ConsoleLogLock)) {
+ CpuPause ();
+ }
-----Original Message-----
From: edk2-devel [mailto:[email protected]] On Behalf
Of Eric Dong
Sent: Thursday, December 20, 2018 9:16 AM
To: [email protected]
Cc: Ni, Ruiyu <[email protected]>; Laszlo Ersek <[email protected]>
Subject: [edk2] [Patch 1/3] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid
AP calls PeiService.
In AcquireSpinLock function, it calls GetPerformanceCounter which
final calls PeiService service. This patch avoid to call
AcquireSpinLock function.
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1411
Cc: Ruiyu Ni <[email protected]>
Cc: Laszlo Ersek <[email protected]>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <[email protected]>
---
UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c |
7
++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git
a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 624ddee055..a64326239f 100644
---
a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.
+++ c
@@ -832,7 +832,12 @@ ProgramProcessorRegister (
RegisterTableEntry = &RegisterTableEntryHead[Index];
DEBUG_CODE_BEGIN ();
- AcquireSpinLock (&CpuFlags->ConsoleLogLock);
+ //
+ // Wait for the AP to release the MSR spin lock.
+ //
+ while (!AcquireSpinLockOrFail (&CpuFlags->ConsoleLogLock)) {
+ CpuPause ();
+ }
ThreadIndex = ApLocation->Package *
CpuStatus->MaxCoreCount *
CpuStatus->MaxThreadCount +
ApLocation->Core * CpuStatus->MaxThreadCount +
ApLocation->Thread;
--
2.15.0.windows.1
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
--
Brian J. Johnson
Enterprise X86 Lab
Hewlett Packard Enterprise
[email protected]
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
