From: "Yao, Jiewen" <jiewen.yao@intel.com>
To: "Ni, Ruiyu" <ruiyu.ni@intel.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
"Gao, Liming" <liming.gao@intel.com>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Subject: Re: [PATCH] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value
Date: Mon, 10 Sep 2018 15:17:23 +0000 [thread overview]
Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503AD4DAB0@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <DB7758C3-1D53-47A7-8748-5D363B7B4180@intel.com>
You are right. My mistake.
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Monday, September 10, 2018 10:59 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Subject: Re: [PATCH] MdePkg/SynchronizationLib: fix
> Interlocked[De|In]crement return value
>
> I didn’t find such instruction in SDM.
>
> 发自我的 iPhone
>
> > 在 2018年9月10日，下午7:37，Yao, Jiewen <jiewen.yao@intel.com>
> 写道：
> >
> > Hi
> > Can we use XSUB for decrement?
> >
> > Thank you
> > Yao Jiewen
> >
> >> -----Original Message-----
> >> From: Ni, Ruiyu
> >> Sent: Monday, September 10, 2018 6:06 PM
> >> To: edk2-devel@lists.01.org
> >> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming
> >> <liming.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> >> Subject: [PATCH] MdePkg/SynchronizationLib: fix
> Interlocked[De|In]crement
> >> return value
> >>
> >> 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.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> >> Cc: Jiewen Yao <jiewen.yao@intel.com>
> >> Cc: Liming Gao <liming.gao@intel.com>
> >> Cc: Michael D Kinney <michael.d.kinney@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 ++--
> >> .../BaseSynchronizationLib/Synchronization.c | 6 +--
> >> .../BaseSynchronizationLib/SynchronizationGcc.c | 6 +--
> >> .../BaseSynchronizationLib/SynchronizationMsc.c | 6 +--
> >> .../Library/BaseSynchronizationLib/X64/GccInline.c | 19 +++++----
> >> .../X64/InterlockedDecrement.c | 46
> >> ----------------------
> >> .../X64/InterlockedDecrement.nasm | 8 ++--
> >> .../X64/InterlockedIncrement.c | 46
> >> ----------------------
> >> .../X64/InterlockedIncrement.nasm | 5 ++-
> >> 16 files changed, 52 insertions(+), 240 deletions(-)
> >> delete mode 100644
> >> MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
> >> delete mode 100644
> >> MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c
> >> delete mode 100644
> >> MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c
> >> delete mode 100644
> >> MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c
> >>
> >> diff --git a/MdePkg/Include/Library/SynchronizationLib.h
> >> b/MdePkg/Include/Library/SynchronizationLib.h
> >> index da69f6ff5e..ce3bce04f5 100644
> >> --- a/MdePkg/Include/Library/SynchronizationLib.h
> >> +++ b/MdePkg/Include/Library/SynchronizationLib.h
> >> @@ -144,8 +144,7 @@ ReleaseSpinLock (
> >>
> >> Performs an atomic increment of the 32-bit unsigned integer specified
> by
> >> Value and returns the incremented value. The increment operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> + performed using MP safe mechanisms.
> >>
> >> If Value is NULL, then ASSERT().
> >>
> >> @@ -166,8 +165,7 @@ InterlockedIncrement (
> >>
> >> Performs an atomic decrement of the 32-bit unsigned integer specified
> >> by
> >> Value and returns the decremented value. The decrement operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> + performed using MP safe mechanisms.
> >>
> >> If Value is NULL, then ASSERT().
> >>
> >> diff --git
> >> a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> >> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> >> index 0be1d4331f..b971cd138d 100755
> >> --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> >> +++
> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> >> @@ -34,9 +34,9 @@ [Sources.IA32]
> >> Ia32/InterlockedCompareExchange64.c | MSFT
> >> Ia32/InterlockedCompareExchange32.c | MSFT
> >> Ia32/InterlockedCompareExchange16.c | MSFT
> >> - Ia32/InterlockedDecrement.c | MSFT
> >> - Ia32/InterlockedIncrement.c | MSFT
> >> - SynchronizationMsc.c | MSFT
> >> + Ia32/InterlockedDecrement.nasm | MSFT
> >> + Ia32/InterlockedIncrement.nasm | MSFT
> >> + SynchronizationMsc.c | MSFT
> >>
> >> Ia32/InterlockedCompareExchange64.nasm| INTEL
> >> Ia32/InterlockedCompareExchange32.nasm| INTEL
> >> @@ -54,17 +54,15 @@ [Sources.X64]
> >> X64/InterlockedCompareExchange64.c | MSFT
> >> X64/InterlockedCompareExchange32.c | MSFT
> >> X64/InterlockedCompareExchange16.c | MSFT
> >> + X64/InterlockedDecrement.nasm | MSFT
> >> + X64/InterlockedIncrement.nasm | MSFT
> >> + SynchronizationMsc.c | MSFT
> >>
> >> X64/InterlockedCompareExchange64.nasm| INTEL
> >> X64/InterlockedCompareExchange32.nasm| INTEL
> >> X64/InterlockedCompareExchange16.nasm| INTEL
> >> -
> >> - X64/InterlockedDecrement.c | MSFT
> >> - X64/InterlockedIncrement.c | MSFT
> >> - SynchronizationMsc.c | MSFT
> >> -
> >> - X64/InterlockedDecrement.nasm| INTEL
> >> - X64/InterlockedIncrement.nasm| INTEL
> >> + X64/InterlockedDecrement.nasm | INTEL
> >> + X64/InterlockedIncrement.nasm | INTEL
> >> Synchronization.c | INTEL
> >>
> >> Ia32/InternalGetSpinLockProperties.c | GCC
> >> diff --git
> >>
> a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternals.
> >> h
> >>
> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternals.
> >> h
> >> index 37edd7c188..8c363a8585 100644
> >> ---
> >>
> a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternals.
> >> h
> >> +++
> >>
> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternals.
> >> h
> >> @@ -27,8 +27,7 @@
> >>
> >> Performs an atomic increment of the 32-bit unsigned integer specified
> by
> >> Value and returns the incremented value. The increment operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> + performed using MP safe mechanisms.
> >>
> >> @param Value A pointer to the 32-bit value to increment.
> >>
> >> @@ -47,8 +46,7 @@ InternalSyncIncrement (
> >>
> >> Performs an atomic decrement of the 32-bit unsigned integer specified
> >> by
> >> Value and returns the decrement value. The decrement operation must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> + performed using MP safe mechanisms.
> >>
> >> @param Value A pointer to the 32-bit value to decrement.
> >>
> >> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> >> b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> >> index 4ab293f243..d82e0205f5 100644
> >> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> >> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> >> @@ -20,8 +20,7 @@
> >>
> >> Performs an atomic increment of the 32-bit unsigned integer specified
> by
> >> Value and returns the incremented value. The increment operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> + performed using MP safe mechanisms.
> >>
> >> @param Value A pointer to the 32-bit value to increment.
> >>
> >> @@ -37,9 +36,10 @@ InternalSyncIncrement (
> >> UINT32 Result;
> >>
> >> __asm__ __volatile__ (
> >> + "movl $1, %%eax \n\t"
> >> "lock \n\t"
> >> - "incl %2 \n\t"
> >> - "movl %2, %%eax "
> >> + "xadd %%eax, %2 \n\t"
> >> + "inc %%eax "
> >> : "=a" (Result), // %0
> >> "=m" (*Value) // %1
> >> : "m" (*Value) // %2
> >> @@ -57,8 +57,7 @@ InternalSyncIncrement (
> >>
> >> Performs an atomic decrement of the 32-bit unsigned integer specified
> >> by
> >> Value and returns the decremented value. The decrement operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> + performed using MP safe mechanisms.
> >>
> >> @param Value A pointer to the 32-bit value to decrement.
> >>
> >> @@ -74,9 +73,10 @@ InternalSyncDecrement (
> >> UINT32 Result;
> >>
> >> __asm__ __volatile__ (
> >> - "lock \n\t"
> >> - "decl %2 \n\t"
> >> - "movl %2, %%eax "
> >> + "movl $-1, %%eax \n\t"
> >> + "lock \n\t"
> >> + "xadd %%eax, %2 \n\t"
> >> + "dec %%eax "
> >> : "=a" (Result), // %0
> >> "=m" (*Value) // %1
> >> : "m" (*Value) // %2
> >> diff --git
> >> a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
> >> b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
> >> deleted file mode 100644
> >> index 354a0e7ab1..0000000000
> >> ---
> a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
> >> +++ /dev/null
> >> @@ -1,42 +0,0 @@
> >> -/** @file
> >> - InterlockedDecrement function
> >> -
> >> - Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> >> - This program and the accompanying materials
> >> - are licensed and made available under the terms and conditions of the
> >> BSD License
> >> - which accompanies this distribution. The full text of the license may
> be
> >> found at
> >> - http://opensource.org/licenses/bsd-license.php.
> >> -
> >> - THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS
> IS"
> >> BASIS,
> >> - WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> >> EXPRESS OR IMPLIED.
> >> -
> >> -**/
> >> -
> >> -
> >> -
> >> -
> >> -/**
> >> - Performs an atomic decrement of an 32-bit unsigned integer.
> >> -
> >> - Performs an atomic decrement of the 32-bit unsigned integer
> specified by
> >> - Value and returns the decrement value. The decrement operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> -
> >> - @param Value A pointer to the 32-bit value to decrement.
> >> -
> >> - @return The decrement value.
> >> -
> >> -**/
> >> -UINT32
> >> -EFIAPI
> >> -InternalSyncDecrement (
> >> - IN volatile UINT32 *Value
> >> - )
> >> -{
> >> - _asm {
> >> - mov eax, Value
> >> - lock dec dword ptr [eax]
> >> - mov eax, [eax]
> >> - }
> >> -}
> >> diff --git
> >>
> a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.nas
> >> m
> >>
> b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.nas
> >> m
> >> index 4c46041186..dd5a8de3ed 100644
> >> ---
> >>
> a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.nas
> >> m
> >> +++
> >>
> b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.nas
> >> m
> >> @@ -1,6 +1,6 @@
> >> ;------------------------------------------------------------------------------
> >> ;
> >> -; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> >> +; Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> >> ; This program and the accompanying materials
> >> ; are licensed and made available under the terms and conditions of the
> >> BSD License
> >> ; which accompanies this distribution. The full text of the license may be
> >> found at
> >> @@ -32,8 +32,8 @@
> >> ;------------------------------------------------------------------------------
> >> global ASM_PFX(InternalSyncDecrement)
> >> ASM_PFX(InternalSyncDecrement):
> >> - mov eax, [esp + 4]
> >> - lock dec dword [eax]
> >> - mov eax, [eax]
> >> + mov ecx, [esp + 4]
> >> + mov eax, 0FFFFFFFFh
> >> + lock xadd dword [ecx], eax
> >> + dec eax
> >> ret
> >> -
> >> diff --git
> >> a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c
> >> b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c
> >> deleted file mode 100644
> >> index c61a550119..0000000000
> >> ---
> a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c
> >> +++ /dev/null
> >> @@ -1,43 +0,0 @@
> >> -/** @file
> >> - InterLockedIncrement function
> >> -
> >> - Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> >> - This program and the accompanying materials
> >> - are licensed and made available under the terms and conditions of the
> >> BSD License
> >> - which accompanies this distribution. The full text of the license may
> be
> >> found at
> >> - http://opensource.org/licenses/bsd-license.php.
> >> -
> >> - THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS
> IS"
> >> BASIS,
> >> - WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> >> EXPRESS OR IMPLIED.
> >> -
> >> -**/
> >> -
> >> -
> >> -
> >> -
> >> -/**
> >> - Performs an atomic increment of an 32-bit unsigned integer.
> >> -
> >> - Performs an atomic increment of the 32-bit unsigned integer specified
> by
> >> - Value and returns the incremented value. The increment operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> -
> >> - @param Value A pointer to the 32-bit value to increment.
> >> -
> >> - @return The incremented value.
> >> -
> >> -**/
> >> -UINT32
> >> -EFIAPI
> >> -InternalSyncIncrement (
> >> - IN volatile UINT32 *Value
> >> - )
> >> -{
> >> - _asm {
> >> - mov eax, Value
> >> - lock inc dword ptr [eax]
> >> - mov eax, [eax]
> >> - }
> >> -}
> >> -
> >> diff --git
> >>
> a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.nasm
> >>
> b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.nasm
> >> index 3902c73275..0677e4bf78 100644
> >> ---
> >>
> a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.nasm
> >> +++
> >>
> b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.nasm
> >> @@ -32,8 +32,9 @@
> >> ;------------------------------------------------------------------------------
> >> global ASM_PFX(InternalSyncIncrement)
> >> ASM_PFX(InternalSyncIncrement):
> >> - mov eax, [esp + 4]
> >> - lock inc dword [eax]
> >> - mov eax, [eax]
> >> + mov ecx, [esp + 4]
> >> + mov eax, 1
> >> + lock xadd dword [ecx], eax
> >> + inc eax
> >> ret
> >>
> >> diff --git a/MdePkg/Library/BaseSynchronizationLib/Synchronization.c
> >> b/MdePkg/Library/BaseSynchronizationLib/Synchronization.c
> >> index 76c5a1275c..0ab59ca702 100644
> >> --- a/MdePkg/Library/BaseSynchronizationLib/Synchronization.c
> >> +++ b/MdePkg/Library/BaseSynchronizationLib/Synchronization.c
> >> @@ -231,8 +231,7 @@ ReleaseSpinLock (
> >>
> >> Performs an atomic increment of the 32-bit unsigned integer specified
> by
> >> Value and returns the incremented value. The increment operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> + performed using MP safe mechanisms.
> >>
> >> If Value is NULL, then ASSERT().
> >>
> >> @@ -256,8 +255,7 @@ InterlockedIncrement (
> >>
> >> Performs an atomic decrement of the 32-bit unsigned integer specified
> >> by
> >> Value and returns the decremented value. The decrement operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> + performed using MP safe mechanisms.
> >>
> >> If Value is NULL, then ASSERT().
> >>
> >> diff --git a/MdePkg/Library/BaseSynchronizationLib/SynchronizationGcc.c
> >> b/MdePkg/Library/BaseSynchronizationLib/SynchronizationGcc.c
> >> index 5ac548b19f..177739d3da 100644
> >> --- a/MdePkg/Library/BaseSynchronizationLib/SynchronizationGcc.c
> >> +++ b/MdePkg/Library/BaseSynchronizationLib/SynchronizationGcc.c
> >> @@ -247,8 +247,7 @@ ReleaseSpinLock (
> >>
> >> Performs an atomic increment of the 32-bit unsigned integer specified
> by
> >> Value and returns the incremented value. The increment operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> + performed using MP safe mechanisms.
> >>
> >> If Value is NULL, then ASSERT().
> >>
> >> @@ -272,8 +271,7 @@ InterlockedIncrement (
> >>
> >> Performs an atomic decrement of the 32-bit unsigned integer specified
> >> by
> >> Value and returns the decremented value. The decrement operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> + performed using MP safe mechanisms.
> >>
> >> If Value is NULL, then ASSERT().
> >>
> >> diff --git
> a/MdePkg/Library/BaseSynchronizationLib/SynchronizationMsc.c
> >> b/MdePkg/Library/BaseSynchronizationLib/SynchronizationMsc.c
> >> index e3298c8ab4..6ab2d71870 100644
> >> --- a/MdePkg/Library/BaseSynchronizationLib/SynchronizationMsc.c
> >> +++ b/MdePkg/Library/BaseSynchronizationLib/SynchronizationMsc.c
> >> @@ -249,8 +249,7 @@ ReleaseSpinLock (
> >>
> >> Performs an atomic increment of the 32-bit unsigned integer specified
> by
> >> Value and returns the incremented value. The increment operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> + performed using MP safe mechanisms.
> >>
> >> If Value is NULL, then ASSERT().
> >>
> >> @@ -274,8 +273,7 @@ InterlockedIncrement (
> >>
> >> Performs an atomic decrement of the 32-bit unsigned integer specified
> >> by
> >> Value and returns the decremented value. The decrement operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> + performed using MP safe mechanisms.
> >>
> >> If Value is NULL, then ASSERT().
> >>
> >> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> >> b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> >> index 5224dd063f..4c4d6e3fc7 100644
> >> --- a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> >> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> >> @@ -15,14 +15,12 @@
> >>
> >>
> >>
> >> -
> >> /**
> >> Performs an atomic increment of an 32-bit unsigned integer.
> >>
> >> Performs an atomic increment of the 32-bit unsigned integer specified
> by
> >> Value and returns the incremented value. The increment operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> + performed using MP safe mechanisms.
> >>
> >> @param Value A pointer to the 32-bit value to increment.
> >>
> >> @@ -38,9 +36,10 @@ InternalSyncIncrement (
> >> UINT32 Result;
> >>
> >> __asm__ __volatile__ (
> >> + "movl $1, %%eax \n\t"
> >> "lock \n\t"
> >> - "incl %2 \n\t"
> >> - "mov %2, %%eax "
> >> + "xadd %%eax, %2 \n\t"
> >> + "inc %%eax "
> >> : "=a" (Result), // %0
> >> "=m" (*Value) // %1
> >> : "m" (*Value) // %2
> >> @@ -57,8 +56,7 @@ InternalSyncIncrement (
> >>
> >> Performs an atomic decrement of the 32-bit unsigned integer specified
> >> by
> >> Value and returns the decremented value. The decrement operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> + performed using MP safe mechanisms.
> >>
> >> @param Value A pointer to the 32-bit value to decrement.
> >>
> >> @@ -74,9 +72,10 @@ InternalSyncDecrement (
> >> UINT32 Result;
> >>
> >> __asm__ __volatile__ (
> >> - "lock \n\t"
> >> - "decl %2 \n\t"
> >> - "mov %2, %%eax "
> >> + "movl $-1, %%eax \n\t"
> >> + "lock \n\t"
> >> + "xadd %%eax, %2 \n\t"
> >> + "dec %%eax "
> >> : "=a" (Result), // %0
> >> "=m" (*Value) // %1
> >> : "m" (*Value) // %2
> >> diff --git
> >> a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c
> >> b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c
> >> deleted file mode 100644
> >> index da402cd4c6..0000000000
> >> ---
> a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c
> >> +++ /dev/null
> >> @@ -1,46 +0,0 @@
> >> -/** @file
> >> - InterlockedDecrement function
> >> -
> >> - Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> >> - This program and the accompanying materials
> >> - are licensed and made available under the terms and conditions of the
> >> BSD License
> >> - which accompanies this distribution. The full text of the license may
> be
> >> found at
> >> - http://opensource.org/licenses/bsd-license.php.
> >> -
> >> - THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS
> IS"
> >> BASIS,
> >> - WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> >> EXPRESS OR IMPLIED.
> >> -
> >> -**/
> >> -
> >> -/**
> >> - Microsoft Visual Studio 7.1 Function Prototypes for I/O Intrinsics.
> >> -**/
> >> -
> >> -long _InterlockedDecrement(
> >> - long * lpAddend
> >> -);
> >> -
> >> -#pragma intrinsic(_InterlockedDecrement)
> >> -
> >> -/**
> >> - Performs an atomic decrement of an 32-bit unsigned integer.
> >> -
> >> - Performs an atomic decrement of the 32-bit unsigned integer
> specified by
> >> - Value and returns the decrement value. The decrement operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> -
> >> - @param Value A pointer to the 32-bit value to decrement.
> >> -
> >> - @return The decrement value.
> >> -
> >> -**/
> >> -UINT32
> >> -EFIAPI
> >> -InternalSyncDecrement (
> >> - IN volatile UINT32 *Value
> >> - )
> >> -{
> >> - return _InterlockedDecrement ((long *)(UINTN)(Value));
> >> -}
> >> -
> >> diff --git
> >>
> a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.nasm
> >>
> b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.nasm
> >> index 60f43111fe..dabdca945e 100644
> >> ---
> >>
> a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.nasm
> >> +++
> >>
> b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.nasm
> >> @@ -1,6 +1,6 @@
> >> ;------------------------------------------------------------------------------
> >> ;
> >> -; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> >> +; Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> >> ; This program and the accompanying materials
> >> ; are licensed and made available under the terms and conditions of the
> >> BSD License
> >> ; which accompanies this distribution. The full text of the license may be
> >> found at
> >> @@ -33,7 +33,7 @@
> >> ;------------------------------------------------------------------------------
> >> global ASM_PFX(InternalSyncDecrement)
> >> ASM_PFX(InternalSyncDecrement):
> >> - lock dec dword [rcx]
> >> - mov eax, [rcx]
> >> + mov eax, 0FFFFFFFFh
> >> + lock xadd dword [rcx], eax
> >> + dec eax
> >> ret
> >> -
> >> diff --git
> >> a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c
> >> b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c
> >> deleted file mode 100644
> >> index bbd1384602..0000000000
> >> ---
> a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c
> >> +++ /dev/null
> >> @@ -1,46 +0,0 @@
> >> -/** @file
> >> - InterLockedIncrement function
> >> -
> >> - Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> >> - This program and the accompanying materials
> >> - are licensed and made available under the terms and conditions of the
> >> BSD License
> >> - which accompanies this distribution. The full text of the license may
> be
> >> found at
> >> - http://opensource.org/licenses/bsd-license.php.
> >> -
> >> - THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS
> IS"
> >> BASIS,
> >> - WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> >> EXPRESS OR IMPLIED.
> >> -
> >> -**/
> >> -
> >> -/**
> >> - Microsoft Visual Studio 7.1 Function Prototypes for I/O Intrinsics.
> >> -**/
> >> -
> >> -long _InterlockedIncrement(
> >> - long * lpAddend
> >> -);
> >> -
> >> -#pragma intrinsic(_InterlockedIncrement)
> >> -
> >> -/**
> >> - Performs an atomic increment of an 32-bit unsigned integer.
> >> -
> >> - Performs an atomic increment of the 32-bit unsigned integer specified
> by
> >> - Value and returns the incremented value. The increment operation
> must
> >> be
> >> - performed using MP safe mechanisms. The state of the return value is
> >> not
> >> - guaranteed to be MP safe.
> >> -
> >> - @param Value A pointer to the 32-bit value to increment.
> >> -
> >> - @return The incremented value.
> >> -
> >> -**/
> >> -UINT32
> >> -EFIAPI
> >> -InternalSyncIncrement (
> >> - IN volatile UINT32 *Value
> >> - )
> >> -{
> >> - return _InterlockedIncrement ((long *)(UINTN)(Value));
> >> -}
> >> -
> >> diff --git
> >>
> a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.nasm
> >>
> b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.nasm
> >> index 7f877b5774..77379d998e 100644
> >> ---
> >>
> a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.nasm
> >> +++
> >>
> b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.nasm
> >> @@ -33,7 +33,8 @@
> >> ;------------------------------------------------------------------------------
> >> global ASM_PFX(InternalSyncIncrement)
> >> ASM_PFX(InternalSyncIncrement):
> >> - lock inc dword [rcx]
> >> - mov eax, [rcx]
> >> + mov eax, 1
> >> + lock xadd dword [rcx], eax
> >> + inc eax
> >> ret
> >>
> >> --
> >> 2.16.1.windows.1
> >
next prev parent reply other threads:[~2018-09-10 15:17 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-10 10:06 [PATCH] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value Ruiyu Ni
2018-09-10 11:37 ` Yao, Jiewen
2018-09-10 14:59 ` Ni, Ruiyu
2018-09-10 15:17 ` Yao, Jiewen [this message]
2018-09-10 16:38 ` Kinney, Michael D
2018-09-11 2:25 ` Ni, Ruiyu
2018-09-11 2:26 ` Ni, Ruiyu
2018-09-12 2:16 ` Ni, Ruiyu
2018-09-12 14:53 ` Kinney, Michael D
2018-09-12 15:08 ` Gao, Liming
2018-09-12 15:12 ` Kinney, Michael D
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=74D8A39837DF1E4DA445A8C0B3885C503AD4DAB0@shsmsx102.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox