Hi,
On 09/13/18 11:50, Ruiyu Ni wrote:
> Today's InterlockedIncrement()/InterlockedDecrement() guarantees to
> perform atomic increment/decrement but doesn't guarantee the return
> value equals to the new value.
>
> The patch fixes the behavior to use "XADD" instruction to guarantee
> the return value equals to the new value.
>
> The patch calls intrinsic functions for MSVC tool chain, calls the
> NASM implementation for INTEL tool chain and calls GCC inline
> assembly implementation (GccInline.c) for GCC tool chain.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
> Cc: Jiewen Yao <jiewen....@intel.com>
> Cc: Liming Gao <liming....@intel.com>
> Cc: Michael D Kinney <michael.d.kin...@intel.com>
> ---
> MdePkg/Include/Library/SynchronizationLib.h | 6 +--
> .../BaseSynchronizationLib.inf | 18 ++++-----
> .../BaseSynchronizationLibInternals.h | 6 +--
> .../BaseSynchronizationLib/Ia32/GccInline.c | 18 ++++-----
> .../Ia32/InterlockedDecrement.c | 42 ---------------------
> .../Ia32/InterlockedDecrement.nasm | 10 ++---
> .../Ia32/InterlockedIncrement.c | 43
> ----------------------
> .../Ia32/InterlockedIncrement.nasm | 7 ++--
> ...lockedDecrement.c => InterlockedDecrementMsc.c} | 4 +-
> ...lockedIncrement.c => InterlockedIncrementMsc.c} | 4 +-
> .../BaseSynchronizationLib/Synchronization.c | 6 +--
> .../BaseSynchronizationLib/SynchronizationGcc.c | 6 +--
> .../BaseSynchronizationLib/SynchronizationMsc.c | 6 +--
> .../Library/BaseSynchronizationLib/X64/GccInline.c | 19 +++++-----
> .../X64/InterlockedDecrement.nasm | 8 ++--
> .../X64/InterlockedIncrement.nasm | 5 ++-
> 16 files changed, 56 insertions(+), 152 deletions(-)
> delete mode 100644
> MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
> delete mode 100644
> MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c
> rename MdePkg/Library/BaseSynchronizationLib/{X64/InterlockedDecrement.c =>
> InterlockedDecrementMsc.c} (87%)
> rename MdePkg/Library/BaseSynchronizationLib/{X64/InterlockedIncrement.c =>
> InterlockedIncrementMsc.c} (87%)
This patch (commit 17634d026f96) has regressed OVMF on KVM. Here's the
bisection log:
> git bisect start
> # bad: [17634d026f968c404b039a8d8431b6389dd396ea] MdePkg/SynchronizationLib:
> fix Interlocked[De|In]crement return value
> git bisect bad 17634d026f968c404b039a8d8431b6389dd396ea
> # good: [997731e796f51df57c113dfd966e818622c3d4aa] UefiCpuPkg: Remove
> redundant library classes, Ppis and GUIDs
> git bisect good 997731e796f51df57c113dfd966e818622c3d4aa
> # good: [b602265d559b2f2ade4d09ba55652c23922c141f] MdeModulePkg
> RegularExpressionDxe: Update Oniguruma to 6.9.0
> git bisect good b602265d559b2f2ade4d09ba55652c23922c141f
> # good: [d5b28edd63f4d0fab087c5d6c9db773e5b5adc7d] MdePkg: Add a inf path in
> MdePkg.dsc
> git bisect good d5b28edd63f4d0fab087c5d6c9db773e5b5adc7d
> # good: [ca3e4f8ab82485edff2cfa7eeb87f71b4be38966] MdePkg
> UefiPciLibPciRootBridgeIo: Remove redundant dependency
> git bisect good ca3e4f8ab82485edff2cfa7eeb87f71b4be38966
> # first bad commit: [17634d026f968c404b039a8d8431b6389dd396ea]
> MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value
The symptom is that the boot gets stuck, with all VCPUs spinning, after
the following log is produced:
> Loading PEIM [CpuMpPei]
> Loading PEIM at 0x000BFEA8000 EntryPoint=0x000BFEADE86 CpuMpPei.efi
> Register PPI Notify: [EfiPeiMemoryDiscoveredPpi]
> Notify: PPI Guid: [EfiPeiMemoryDiscoveredPpi], Peim notify entry point:
> BFEB53B1
> AP Loop Mode is 1
> WakeupBufferStart = 9F000, WakeupBufferSize = 1000
MpInitLib uses SynchronizationLib:
> UefiCpuPkg/Library/MpInitLib/Microcode.c:241:
> AcquireSpinLock(&CpuMpData->MpLock);
> UefiCpuPkg/Library/MpInitLib/Microcode.c:244:
> ReleaseSpinLock(&CpuMpData->MpLock);
> UefiCpuPkg/Library/MpInitLib/MpLib.c:117: AcquireSpinLock (&CpuData->ApLock);
> UefiCpuPkg/Library/MpInitLib/MpLib.c:119: ReleaseSpinLock (&CpuData->ApLock);
> UefiCpuPkg/Library/MpInitLib/MpLib.c:555:
> AcquireSpinLock(&CpuMpData->MpLock);
> UefiCpuPkg/Library/MpInitLib/MpLib.c:557:
> ReleaseSpinLock(&CpuMpData->MpLock);
> UefiCpuPkg/Library/MpInitLib/MpLib.c:560:
> InitializeSpinLock(&CpuMpData->CpuData[ProcessorNumber].ApLock);
> UefiCpuPkg/Library/MpInitLib/MpLib.c:609: InterlockedIncrement ((UINT32
> *) &CpuMpData->CpuCount);
> UefiCpuPkg/Library/MpInitLib/MpLib.c:637: InterlockedCompareExchange32 (
> UefiCpuPkg/Library/MpInitLib/MpLib.c:706: InterlockedIncrement ((UINT32 *)
> &CpuMpData->FinishedCount);
> UefiCpuPkg/Library/MpInitLib/MpLib.c:707: InterlockedDecrement ((UINT32 *)
> &CpuMpData->MpCpuExchangeInfo->NumApsExecuting);
> UefiCpuPkg/Library/MpInitLib/MpLib.c:775: while
> (InterlockedCompareExchange32 (
> UefiCpuPkg/Library/MpInitLib/MpLib.c:1645:
> InitializeSpinLock(&CpuMpData->MpLock);
> UefiCpuPkg/Library/MpInitLib/MpLib.c:1718:
> InitializeSpinLock(&CpuMpData->CpuData[Index].ApLock);
I'll try to collect more information.
Thanks,
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
