From: "Ni, Ruiyu" <ruiyu.ni@intel.com>
To: "Yao, Jiewen" <jiewen.yao@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 14:59:05 +0000 [thread overview]
Message-ID: <DB7758C3-1D53-47A7-8748-5D363B7B4180@intel.com> (raw)
In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503AD4D385@shsmsx102.ccr.corp.intel.com>
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 14:59 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 [this message]
2018-09-10 15:17 ` Yao, Jiewen
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=DB7758C3-1D53-47A7-8748-5D363B7B4180@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