* [PATCH v2] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value
@ 2018-09-13 8:29 Ruiyu Ni
2018-09-13 9:50 ` Ni, Ruiyu
0 siblings, 1 reply; 2+ messages in thread
From: Ruiyu Ni @ 2018-09-13 8:29 UTC (permalink / raw)
To: edk2-devel; +Cc: Jiewen Yao, Liming Gao, Michael D Kinney
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.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.nasm | 8 ++--
.../X64/InterlockedIncrement.nasm | 5 ++-
14 files changed, 52 insertions(+), 148 deletions(-)
delete mode 100644 MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
delete mode 100644 MdePkg/Library/BaseSynchronizationLib/Ia32/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..427e959f2e 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
+ InterlockedIncrementMsc.c | MSFT
+ InterlockedDecrementMsc.c | 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
+ InterlockedIncrementMsc.c | MSFT
+ InterlockedDecrementMsc.c | 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.nasm b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.nasm
index 4c46041186..dd5a8de3ed 100644
--- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.nasm
+++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/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
@@ -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.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.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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value
2018-09-13 8:29 [PATCH v2] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value Ruiyu Ni
@ 2018-09-13 9:50 ` Ni, Ruiyu
0 siblings, 0 replies; 2+ messages in thread
From: Ni, Ruiyu @ 2018-09-13 9:50 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Yao, Jiewen, Gao, Liming
Please ignore this one. It's an incomplete patch. Please check V3.
Thanks/Ray
> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Ruiyu Ni
> Sent: Thursday, September 13, 2018 4:30 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [edk2] [PATCH v2] 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.
>
> 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.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.nasm | 8 ++--
> .../X64/InterlockedIncrement.nasm | 5 ++-
> 14 files changed, 52 insertions(+), 148 deletions(-) delete mode 100644
> MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
> delete mode 100644
> MdePkg/Library/BaseSynchronizationLib/Ia32/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..427e959f2e 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
> + InterlockedIncrementMsc.c | MSFT
> + InterlockedDecrementMsc.c | 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
> + InterlockedIncrementMsc.c | MSFT
> + InterlockedDecrementMsc.c | 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/BaseSynchronizationLibIntern
> +++ als.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.na
> +++ sm
> @@ -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.na
> +++ sm
> @@ -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.nasm
> b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.nas
> m
> index 60f43111fe..dabdca945e 100644
> ---
> a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.nasm
> +++
> b/MdePkg/Library/BaseSynchronizationLib/X64/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
> @@ -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.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.nas
> +++ m
> @@ -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
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-09-13 9:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-13 8:29 [PATCH v2] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value Ruiyu Ni
2018-09-13 9:50 ` Ni, Ruiyu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox