public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Michael Kinney <michael.d.kinney@intel.com>, edk2-devel@ml01.01.org
Cc: Liming Gao <liming.gao@intel.com>, Andrew Fish <afish@apple.com>,
	Jeff Fan <jeff.fan@intel.com>
Subject: Re: [Patch 2/2] MdePkg/BaseSynchronizationLib: Add volatile Interlocked*() APIs
Date: Thu, 17 Nov 2016 10:21:24 +0100	[thread overview]
Message-ID: <b7440392-6051-4edb-3f00-cb76ef096af7@redhat.com> (raw)
In-Reply-To: <1479358424-17096-3-git-send-email-michael.d.kinney@intel.com>

On 11/17/16 05:53, Michael Kinney wrote:
> The SpinLock functions in the SynchronicationLib use volatile
> parameters to keep compiler from optimizing these functions
> too much.  The volatile keyword is missing from the Interlocked*()
> functions in this same library instance.  Update the library instance
> to consistently use volatile on all functions in the
> SynchronizationLib class.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Andrew Fish <afish@apple.com>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Michael Kinney <michael.d.kinney@intel.com>
> ---
>  .../Ia32/InterlockedCompareExchange16.asm              |  4 ++--
>  .../Ia32/InterlockedCompareExchange16.c                |  4 ++--
>  .../Ia32/InterlockedCompareExchange16.nasm             |  4 ++--
>  .../Ia32/InterlockedCompareExchange32.asm              |  4 ++--
>  .../Ia32/InterlockedCompareExchange32.c                |  4 ++--
>  .../Ia32/InterlockedCompareExchange32.nasm             |  4 ++--
>  .../Ia32/InterlockedCompareExchange64.asm              |  4 ++--
>  .../Ia32/InterlockedCompareExchange64.c                |  4 ++--
>  .../Ia32/InterlockedCompareExchange64.nasm             |  4 ++--
>  .../Ia32/InterlockedDecrement.asm                      |  2 +-
>  .../BaseSynchronizationLib/Ia32/InterlockedDecrement.c |  4 ++--
>  .../Ia32/InterlockedDecrement.nasm                     |  4 ++--
>  .../Ia32/InterlockedIncrement.asm                      |  4 ++--
>  .../BaseSynchronizationLib/Ia32/InterlockedIncrement.c |  4 ++--
>  .../Ia32/InterlockedIncrement.nasm                     |  4 ++--
>  .../Library/BaseSynchronizationLib/Synchronization.c   | 18 +++++++++---------
>  .../BaseSynchronizationLib/SynchronizationGcc.c        | 18 +++++++++---------
>  .../BaseSynchronizationLib/SynchronizationMsc.c        | 18 +++++++++---------
>  .../X64/InterlockedCompareExchange16.asm               |  6 +++---
>  .../X64/InterlockedCompareExchange16.c                 |  4 ++--
>  .../X64/InterlockedCompareExchange16.nasm              |  6 +++---
>  .../X64/InterlockedCompareExchange32.asm               |  6 +++---
>  .../X64/InterlockedCompareExchange32.c                 |  4 ++--
>  .../X64/InterlockedCompareExchange32.nasm              |  6 +++---
>  .../X64/InterlockedCompareExchange64.asm               |  6 +++---
>  .../X64/InterlockedCompareExchange64.c                 |  4 ++--
>  .../X64/InterlockedCompareExchange64.nasm              |  4 ++--
>  .../X64/InterlockedDecrement.asm                       |  6 +++---
>  .../BaseSynchronizationLib/X64/InterlockedDecrement.c  |  6 +++---
>  .../X64/InterlockedDecrement.nasm                      |  6 +++---
>  .../X64/InterlockedIncrement.asm                       |  6 +++---
>  .../BaseSynchronizationLib/X64/InterlockedIncrement.c  |  6 +++---
>  .../X64/InterlockedIncrement.nasm                      |  6 +++---
>  33 files changed, 97 insertions(+), 97 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange16.asm b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange16.asm
> index 92a0f49..7362fe9 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange16.asm
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange16.asm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>  ; Copyright (c) 2015, Linaro Ltd. All rights reserved.<BR>
>  ; This program and the accompanying materials
>  ; are licensed and made available under the terms and conditions of the BSD License
> @@ -30,7 +30,7 @@
>  ; UINT16
>  ; EFIAPI
>  ; InternalSyncCompareExchange16 (
> -;   IN      UINT16                    *Value,
> +;   IN      volatile UINT16           *Value,
>  ;   IN      UINT16                    CompareValue,
>  ;   IN      UINT16                    ExchangeValue
>  ;   );
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange16.c b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange16.c
> index 3d52137..0c3a4f6 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange16.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange16.c
> @@ -1,7 +1,7 @@
>  /** @file
>    InterlockedCompareExchange16 function
>  
> -  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>    Copyright (c) 2015, Linaro Ltd. All rights reserved.<BR>
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD License
> @@ -36,7 +36,7 @@
>  UINT16
>  EFIAPI
>  InternalSyncCompareExchange16 (
> -  IN      UINT16                    *Value,
> +  IN      volatile UINT16           *Value,
>    IN      UINT16                    CompareValue,
>    IN      UINT16                    ExchangeValue
>    )
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange16.nasm b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange16.nasm
> index 8cabe66..7fbfd75 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange16.nasm
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange16.nasm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>  ; Copyright (c) 2015, Linaro Ltd. All rights reserved.<BR>
>  ; This program and the accompanying materials
>  ; are licensed and made available under the terms and conditions of the BSD License
> @@ -28,7 +28,7 @@
>  ; UINT16
>  ; EFIAPI
>  ; InternalSyncCompareExchange16 (
> -;   IN      UINT16                    *Value,
> +;   IN      volatile UINT16           *Value,
>  ;   IN      UINT16                    CompareValue,
>  ;   IN      UINT16                    ExchangeValue
>  ;   );
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange32.asm b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange32.asm
> index 78ea72c..1a4a611 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange32.asm
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange32.asm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; 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
> @@ -29,7 +29,7 @@
>  ; UINT32
>  ; EFIAPI
>  ; InternalSyncCompareExchange32 (
> -;   IN      UINT32                    *Value,
> +;   IN      volatile UINT32           *Value,
>  ;   IN      UINT32                    CompareValue,
>  ;   IN      UINT32                    ExchangeValue
>  ;   );
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange32.c b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange32.c
> index a2c6838..cf80dde 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange32.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange32.c
> @@ -1,7 +1,7 @@
>  /** @file
>    InterlockedCompareExchange32 function
>  
> -  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
> +  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
> @@ -35,7 +35,7 @@
>  UINT32
>  EFIAPI
>  InternalSyncCompareExchange32 (
> -  IN      UINT32                    *Value,
> +  IN      volatile UINT32           *Value,
>    IN      UINT32                    CompareValue,
>    IN      UINT32                    ExchangeValue
>    )
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange32.nasm b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange32.nasm
> index 5393273..9064c65 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange32.nasm
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange32.nasm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; 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
> @@ -27,7 +27,7 @@
>  ; UINT32
>  ; EFIAPI
>  ; InternalSyncCompareExchange32 (
> -;   IN      UINT32                    *Value,
> +;   IN      volatile UINT32           *Value,
>  ;   IN      UINT32                    CompareValue,
>  ;   IN      UINT32                    ExchangeValue
>  ;   );
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange64.asm b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange64.asm
> index 0fcbd23..e7b02db 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange64.asm
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange64.asm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; 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
> @@ -29,7 +29,7 @@
>  ; UINT64
>  ; EFIAPI
>  ; InternalSyncCompareExchange64 (
> -;   IN      UINT64                    *Value,
> +;   IN      volatile UINT64           *Value,
>  ;   IN      UINT64                    CompareValue,
>  ;   IN      UINT64                    ExchangeValue
>  ;   );
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange64.c b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange64.c
> index 73af6ef..0bdbd79 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange64.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange64.c
> @@ -1,7 +1,7 @@
>  /** @file
>    InterlockedCompareExchange64 function
>  
> -  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
> +  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
> @@ -34,7 +34,7 @@
>  UINT64
>  EFIAPI
>  InternalSyncCompareExchange64 (
> -  IN      UINT64                    *Value,
> +  IN      volatile UINT64           *Value,
>    IN      UINT64                    CompareValue,
>    IN      UINT64                    ExchangeValue
>    )
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange64.nasm b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange64.nasm
> index 206de40..c5c3e12 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange64.nasm
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchange64.nasm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; 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
> @@ -27,7 +27,7 @@
>  ; UINT64
>  ; EFIAPI
>  ; InternalSyncCompareExchange64 (
> -;   IN      UINT64                    *Value,
> +;   IN      volatile UINT64           *Value,
>  ;   IN      UINT64                    CompareValue,
>  ;   IN      UINT64                    ExchangeValue
>  ;   );
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.asm b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.asm
> index 22cb0b2..8dbf1a9 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.asm
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.asm
> @@ -29,7 +29,7 @@
>  ; UINT32
>  ; EFIAPI
>  ; InternalSyncDecrement (
> -;   IN      UINT32                    *Value
> +;   IN      volatile UINT32                    *Value
>  ;   );
>  ;------------------------------------------------------------------------------
>  InternalSyncDecrement   PROC
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
> index 7f18e0b..b6647c7 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.c
> @@ -1,7 +1,7 @@
>  /** @file
>    InterlockedDecrement function
>  
> -  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
> +  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
> @@ -31,7 +31,7 @@
>  UINT32
>  EFIAPI
>  InternalSyncDecrement (
> -  IN      UINT32                    *Value
> +  IN      volatile UINT32                    *Value
>    )
>  {
>    _asm {
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.nasm b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.nasm
> index a7414be..1f32866 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.nasm
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDecrement.nasm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; 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
> @@ -27,7 +27,7 @@
>  ; UINT32
>  ; EFIAPI
>  ; InternalSyncDecrement (
> -;   IN      UINT32                    *Value
> +;   IN      volatile UINT32                    *Value
>  ;   );
>  ;------------------------------------------------------------------------------
>  global ASM_PFX(InternalSyncDecrement)
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.asm b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.asm
> index 51675f6..661ffd2 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.asm
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.asm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; 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
> @@ -29,7 +29,7 @@
>  ; UINT32
>  ; EFIAPI
>  ; InternalSyncIncrement (
> -;   IN      UINT32                    *Value
> +;   IN      volatile UINT32                    *Value
>  ;   );
>  ;------------------------------------------------------------------------------
>  InternalSyncIncrement   PROC
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c
> index cf9f92a..d6200ee 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c
> @@ -1,7 +1,7 @@
>  /** @file
>    InterLockedIncrement function
>  
> -  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
> +  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
> @@ -31,7 +31,7 @@
>  UINT32
>  EFIAPI
>  InternalSyncIncrement (
> -  IN      UINT32                    *Value
> +  IN      volatile UINT32                    *Value
>    )
>  {
>    _asm {
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.nasm b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.nasm
> index 24f24f5..d903dda 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.nasm
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.nasm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; 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
> @@ -27,7 +27,7 @@
>  ; UINT32
>  ; EFIAPI
>  ; InternalSyncIncrement (
> -;   IN      UINT32                    *Value
> +;   IN      volatile UINT32                    *Value
>  ;   );
>  ;------------------------------------------------------------------------------
>  global ASM_PFX(InternalSyncIncrement)
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Synchronization.c b/MdePkg/Library/BaseSynchronizationLib/Synchronization.c
> index bb0c8e9..862a70b 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Synchronization.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/Synchronization.c
> @@ -188,7 +188,7 @@ AcquireSpinLockOrFail (
>  
>    return (BOOLEAN)(
>             InterlockedCompareExchangePointer (
> -             (VOID**)SpinLock,
> +             (volatile VOID**)SpinLock,
>               (VOID*)SPIN_LOCK_RELEASED,
>               (VOID*)SPIN_LOCK_ACQUIRED
>               ) == (VOID*)SPIN_LOCK_RELEASED

(1) With the change that I've proposed for patch #1, this hunk should
become unnecessary. A (TYPE *) argument can be transparently taken by a
(TYPE volatile *) parameter (for which an alternative mode of writing is
(volatile TYPE *)).

For TYPE===(VOID *), the above translates to: a (VOID **) arg can be
taken transparently by a (VOID * volatile *) param.

> @@ -244,7 +244,7 @@ ReleaseSpinLock (
>  UINT32
>  EFIAPI
>  InterlockedIncrement (
> -  IN      UINT32                    *Value
> +  IN      volatile UINT32                    *Value
>    )
>  {
>    ASSERT (Value != NULL);
> @@ -269,7 +269,7 @@ InterlockedIncrement (
>  UINT32
>  EFIAPI
>  InterlockedDecrement (
> -  IN      UINT32                    *Value
> +  IN      volatile UINT32                    *Value
>    )
>  {
>    ASSERT (Value != NULL);
> @@ -298,7 +298,7 @@ InterlockedDecrement (
>  UINT16
>  EFIAPI
>  InterlockedCompareExchange16 (
> -  IN OUT  UINT16                    *Value,
> +  IN OUT  volatile UINT16           *Value,
>    IN      UINT16                    CompareValue,
>    IN      UINT16                    ExchangeValue
>    )
> @@ -329,7 +329,7 @@ InterlockedCompareExchange16 (
>  UINT32
>  EFIAPI
>  InterlockedCompareExchange32 (
> -  IN OUT  UINT32                    *Value,
> +  IN OUT  volatile UINT32           *Value,
>    IN      UINT32                    CompareValue,
>    IN      UINT32                    ExchangeValue
>    )
> @@ -359,7 +359,7 @@ InterlockedCompareExchange32 (
>  UINT64
>  EFIAPI
>  InterlockedCompareExchange64 (
> -  IN OUT  UINT64                    *Value,
> +  IN OUT  volatile UINT64           *Value,
>    IN      UINT64                    CompareValue,
>    IN      UINT64                    ExchangeValue
>    )
> @@ -389,7 +389,7 @@ InterlockedCompareExchange64 (
>  VOID *
>  EFIAPI
>  InterlockedCompareExchangePointer (
> -  IN OUT  VOID                      **Value,
> +  IN OUT  volatile VOID             **Value,
>    IN      VOID                      *CompareValue,
>    IN      VOID                      *ExchangeValue
>    )

(2) Same comment as for patch #1.

> @@ -401,13 +401,13 @@ InterlockedCompareExchangePointer (
>    switch (SizeOfValue) {
>      case sizeof (UINT32):
>        return (VOID*)(UINTN)InterlockedCompareExchange32 (
> -                             (UINT32*)Value,
> +                             (volatile UINT32 *)Value,
>                               (UINT32)(UINTN)CompareValue,
>                               (UINT32)(UINTN)ExchangeValue
>                               );

Yes, this looks good for preserving volatility; we're casting the type
from (pointer-to-volatile-pointer) to (pointer-to-volatile-UINT32).

>      case sizeof (UINT64):
>        return (VOID*)(UINTN)InterlockedCompareExchange64 (
> -                             (UINT64*)Value,
> +                             (volatile UINT64 *)Value,
>                               (UINT64)(UINTN)CompareValue,
>                               (UINT64)(UINTN)ExchangeValue
>                               );
> diff --git a/MdePkg/Library/BaseSynchronizationLib/SynchronizationGcc.c b/MdePkg/Library/BaseSynchronizationLib/SynchronizationGcc.c
> index 4b8c8e5..b7538cc 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/SynchronizationGcc.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/SynchronizationGcc.c
> @@ -199,7 +199,7 @@ AcquireSpinLockOrFail (
>  
>    _ReadWriteBarrier ();
>    Result = InterlockedCompareExchangePointer (
> -             (VOID**)SpinLock,
> +             (volatile VOID**)SpinLock,
>               (VOID*)SPIN_LOCK_RELEASED,
>               (VOID*)SPIN_LOCK_ACQUIRED
>             );

(3) This hunk should be unnecessary.

> @@ -260,7 +260,7 @@ ReleaseSpinLock (
>  UINT32
>  EFIAPI
>  InterlockedIncrement (
> -  IN      UINT32                    *Value
> +  IN      volatile UINT32                    *Value
>    )
>  {
>    ASSERT (Value != NULL);
> @@ -285,7 +285,7 @@ InterlockedIncrement (
>  UINT32
>  EFIAPI
>  InterlockedDecrement (
> -  IN      UINT32                    *Value
> +  IN      volatile UINT32                    *Value
>    )
>  {
>    ASSERT (Value != NULL);
> @@ -314,7 +314,7 @@ InterlockedDecrement (
>  UINT16
>  EFIAPI
>  InterlockedCompareExchange16 (
> -  IN OUT  UINT16                    *Value,
> +  IN OUT  volatile UINT16           *Value,
>    IN      UINT16                    CompareValue,
>    IN      UINT16                    ExchangeValue
>    )
> @@ -345,7 +345,7 @@ InterlockedCompareExchange16 (
>  UINT32
>  EFIAPI
>  InterlockedCompareExchange32 (
> -  IN OUT  UINT32                    *Value,
> +  IN OUT  volatile UINT32           *Value,
>    IN      UINT32                    CompareValue,
>    IN      UINT32                    ExchangeValue
>    )
> @@ -375,7 +375,7 @@ InterlockedCompareExchange32 (
>  UINT64
>  EFIAPI
>  InterlockedCompareExchange64 (
> -  IN OUT  UINT64                    *Value,
> +  IN OUT  volatile UINT64           *Value,
>    IN      UINT64                    CompareValue,
>    IN      UINT64                    ExchangeValue
>    )
> @@ -405,7 +405,7 @@ InterlockedCompareExchange64 (
>  VOID *
>  EFIAPI
>  InterlockedCompareExchangePointer (
> -  IN OUT  VOID                      **Value,
> +  IN OUT  volatile VOID             **Value,
>    IN      VOID                      *CompareValue,
>    IN      VOID                      *ExchangeValue
>    )

(4) The usual remark.

> @@ -417,13 +417,13 @@ InterlockedCompareExchangePointer (
>    switch (SizeOfValue) {
>      case sizeof (UINT32):
>        return (VOID*)(UINTN)InterlockedCompareExchange32 (
> -                             (UINT32*)Value,
> +                             (volatile UINT32 *)Value,
>                               (UINT32)(UINTN)CompareValue,
>                               (UINT32)(UINTN)ExchangeValue
>                               );
>      case sizeof (UINT64):
>        return (VOID*)(UINTN)InterlockedCompareExchange64 (
> -                             (UINT64*)Value,
> +                             (volatile UINT64 *)Value,
>                               (UINT64)(UINTN)CompareValue,
>                               (UINT64)(UINTN)ExchangeValue
>                               );
> diff --git a/MdePkg/Library/BaseSynchronizationLib/SynchronizationMsc.c b/MdePkg/Library/BaseSynchronizationLib/SynchronizationMsc.c
> index db344b7..2107935 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/SynchronizationMsc.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/SynchronizationMsc.c
> @@ -201,7 +201,7 @@ AcquireSpinLockOrFail (
>  
>    _ReadWriteBarrier ();
>    Result = InterlockedCompareExchangePointer (
> -             (VOID**)SpinLock,
> +             (volatile VOID**)SpinLock,
>               (VOID*)SPIN_LOCK_RELEASED,
>               (VOID*)SPIN_LOCK_ACQUIRED
>             );

(5) This hunk shouldn't be necessary.

> @@ -262,7 +262,7 @@ ReleaseSpinLock (
>  UINT32
>  EFIAPI
>  InterlockedIncrement (
> -  IN      UINT32                    *Value
> +  IN      volatile UINT32                    *Value
>    )
>  {
>    ASSERT (Value != NULL);
> @@ -287,7 +287,7 @@ InterlockedIncrement (
>  UINT32
>  EFIAPI
>  InterlockedDecrement (
> -  IN      UINT32                    *Value
> +  IN      volatile UINT32                    *Value
>    )
>  {
>    ASSERT (Value != NULL);
> @@ -316,7 +316,7 @@ InterlockedDecrement (
>  UINT16
>  EFIAPI
>  InterlockedCompareExchange16 (
> -  IN OUT  UINT16                    *Value,
> +  IN OUT  volatile UINT16           *Value,
>    IN      UINT16                    CompareValue,
>    IN      UINT16                    ExchangeValue
>    )
> @@ -347,7 +347,7 @@ InterlockedCompareExchange16 (
>  UINT32
>  EFIAPI
>  InterlockedCompareExchange32 (
> -  IN OUT  UINT32                    *Value,
> +  IN OUT  volatile UINT32           *Value,
>    IN      UINT32                    CompareValue,
>    IN      UINT32                    ExchangeValue
>    )
> @@ -377,7 +377,7 @@ InterlockedCompareExchange32 (
>  UINT64
>  EFIAPI
>  InterlockedCompareExchange64 (
> -  IN OUT  UINT64                    *Value,
> +  IN OUT  volatile UINT64           *Value,
>    IN      UINT64                    CompareValue,
>    IN      UINT64                    ExchangeValue
>    )
> @@ -407,7 +407,7 @@ InterlockedCompareExchange64 (
>  VOID *
>  EFIAPI
>  InterlockedCompareExchangePointer (
> -  IN OUT  VOID                      **Value,
> +  IN OUT  volatile VOID             **Value,
>    IN      VOID                      *CompareValue,
>    IN      VOID                      *ExchangeValue
>    )

(6) The usual comment; volatile should come between **.

> @@ -419,13 +419,13 @@ InterlockedCompareExchangePointer (
>    switch (SizeOfValue) {
>      case sizeof (UINT32):
>        return (VOID*)(UINTN)InterlockedCompareExchange32 (
> -                             (UINT32*)Value,
> +                             (volatile UINT32*)Value,
>                               (UINT32)(UINTN)CompareValue,
>                               (UINT32)(UINTN)ExchangeValue
>                               );
>      case sizeof (UINT64):
>        return (VOID*)(UINTN)InterlockedCompareExchange64 (
> -                             (UINT64*)Value,
> +                             (volatile UINT64*)Value,
>                               (UINT64)(UINTN)CompareValue,
>                               (UINT64)(UINTN)ExchangeValue
>                               );
> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange16.asm b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange16.asm
> index 7926662..262adeb 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange16.asm
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange16.asm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>  ; Copyright (c) 2015, Linaro Ltd. All rights reserved.<BR>
>  ; This program and the accompanying materials
>  ; are licensed and made available under the terms and conditions of the BSD License
> @@ -27,8 +27,8 @@
>  ;------------------------------------------------------------------------------
>  ; UINT16
>  ; EFIAPI
> -; InterlockedCompareExchange16 (
> -;   IN      UINT16                    *Value,
> +; InternalSyncCompareExchange16 (
> +;   IN      volatile UINT16           *Value,
>  ;   IN      UINT16                    CompareValue,
>  ;   IN      UINT16                    ExchangeValue
>  ;   );

(7) This is just a comment block, but the function name should not be
changed, I believe.

> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange16.c b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange16.c
> index 76aa6fb..8b51ecb 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange16.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange16.c
> @@ -1,7 +1,7 @@
>  /** @file
>    InterlockedCompareExchange16 function
>  
> -  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>    Copyright (c) 2015, Linaro Ltd. All rights reserved.<BR>
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD License
> @@ -44,7 +44,7 @@ __int16 _InterlockedCompareExchange16(
>  UINT16
>  EFIAPI
>  InternalSyncCompareExchange16 (
> -  IN      UINT16                    *Value,
> +  IN      volatile UINT16           *Value,
>    IN      UINT16                    CompareValue,
>    IN      UINT16                    ExchangeValue
>    )
> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange16.nasm b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange16.nasm
> index 1d72d07..eebed5a 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange16.nasm
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange16.nasm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>  ; Copyright (c) 2015, Linaro Ltd. All rights reserved.<BR>
>  ; This program and the accompanying materials
>  ; are licensed and made available under the terms and conditions of the BSD License
> @@ -28,8 +28,8 @@
>  ;------------------------------------------------------------------------------
>  ; UINT16
>  ; EFIAPI
> -; InterlockedCompareExchange16 (
> -;   IN      UINT16                    *Value,
> +; InternalSyncCompareExchange16 (
> +;   IN      volatile UINT16           *Value,
>  ;   IN      UINT16                    CompareValue,
>  ;   IN      UINT16                    ExchangeValue
>  ;   );

(8) Unwarranted function rename.

> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange32.asm b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange32.asm
> index ee94ff7..711b399 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange32.asm
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange32.asm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; 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
> @@ -26,8 +26,8 @@
>  ;------------------------------------------------------------------------------
>  ; UINT32
>  ; EFIAPI
> -; InterlockedCompareExchange32 (
> -;   IN      UINT32                    *Value,
> +; InternalSyncCompareExchange32 (
> +;   IN      volatile UINT32           *Value,
>  ;   IN      UINT32                    CompareValue,
>  ;   IN      UINT32                    ExchangeValue
>  ;   );

(9) Ditto.

> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange32.c b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange32.c
> index 9f8f3d3..ef407af 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange32.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange32.c
> @@ -1,7 +1,7 @@
>  /** @file
>    InterlockedCompareExchange32 function
>  
> -  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
> +  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
> @@ -44,7 +44,7 @@ long _InterlockedCompareExchange(
>  UINT32
>  EFIAPI
>  InternalSyncCompareExchange32 (
> -  IN      UINT32                    *Value,
> +  IN      volatile UINT32           *Value,
>    IN      UINT32                    CompareValue,
>    IN      UINT32                    ExchangeValue
>    )
> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange32.nasm b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange32.nasm
> index 4c8c810..5ed4ba5 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange32.nasm
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange32.nasm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; 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
> @@ -27,8 +27,8 @@
>  ;------------------------------------------------------------------------------
>  ; UINT32
>  ; EFIAPI
> -; InterlockedCompareExchange32 (
> -;   IN      UINT32                    *Value,
> +; InternalSyncCompareExchange32 (
> +;   IN      volatile UINT32           *Value,
>  ;   IN      UINT32                    CompareValue,
>  ;   IN      UINT32                    ExchangeValue
>  ;   );

(10) Ditto.

> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange64.asm b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange64.asm
> index b5bcd25..be429d8 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange64.asm
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange64.asm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; 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
> @@ -26,8 +26,8 @@
>  ;------------------------------------------------------------------------------
>  ; UINT64
>  ; EFIAPI
> -; InterlockedCompareExchange64 (
> -;   IN      UINT64                    *Value,
> +; InternalSyncCompareExchange64 (
> +;   IN      volatile UINT64           *Value,
>  ;   IN      UINT64                    CompareValue,
>  ;   IN      UINT64                    ExchangeValue
>  ;   );

(11) Ditto.

> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange64.c b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange64.c
> index 56805b3..a8f9fbb 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange64.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange64.c
> @@ -1,7 +1,7 @@
>  /** @file
>    InterlockedCompareExchange64 function
>  
> -  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
> +  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
> @@ -43,7 +43,7 @@ __int64 _InterlockedCompareExchange64(
>  UINT64
>  EFIAPI
>  InternalSyncCompareExchange64 (
> -  IN      UINT64                    *Value,
> +  IN      volatile UINT64           *Value,
>    IN      UINT64                    CompareValue,
>    IN      UINT64                    ExchangeValue
>    )
> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange64.nasm b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange64.nasm
> index 72c350a..bc97f31 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange64.nasm
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedCompareExchange64.nasm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; 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
> @@ -27,7 +27,7 @@
>  ;------------------------------------------------------------------------------
>  ; UINT64
>  ; EFIAPI
> -; InterlockedCompareExchange64 (
> +; InternalSyncCompareExchange64 (
>  ;   IN      UINT64                    *Value,
>  ;   IN      UINT64                    CompareValue,
>  ;   IN      UINT64                    ExchangeValue

(12) This renames the function, but doesn't change the definition of the
Value parameter :)

> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.asm b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.asm
> index fdea0d4..2bff4f8 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.asm
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.asm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; 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
> @@ -26,8 +26,8 @@
>  ;------------------------------------------------------------------------------
>  ; UINT32
>  ; EFIAPI
> -; InterlockedDecrement (
> -;   IN      UINT32                    *Value
> +; InternalSyncDecrement (
> +;   IN      volatile UINT32                    *Value
>  ;   );

(13) Function rename shouldn't be necessary.

>  ;------------------------------------------------------------------------------
>  InternalSyncDecrement   PROC
> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c
> index 65faf01..c2299af 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c
> @@ -1,7 +1,7 @@
>  /** @file
>    InterlockedDecrement function
>  
> -  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
> +  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
> @@ -38,9 +38,9 @@ long _InterlockedDecrement(
>  UINT32
>  EFIAPI
>  InternalSyncDecrement (
> -  IN      UINT32                    *Value
> +  IN      volatile UINT32                    *Value
>    )
>  {
> -  return _InterlockedDecrement (Value);
> +  return _InterlockedDecrement ((long *)(UINTN)(Value));
>  }
>  

This is very confusing, but ultimately it appears correct. Looking at
this file, it seems that _InterlockedDecrement is a Visual Studio
intrinsic. That also explains why UINT32 is compatible with "long", even
on X64.

> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.nasm b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.nasm
> index e140e44..91c0f8e 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.nasm
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.nasm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; 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
> @@ -27,8 +27,8 @@
>  ;------------------------------------------------------------------------------
>  ; UINT32
>  ; EFIAPI
> -; InterlockedDecrement (
> -;   IN      UINT32                    *Value
> +; InternalSyncDecrement (
> +;   IN      volatile UINT32                    *Value
>  ;   );

(14) Function rename unjustified I think.

>  ;------------------------------------------------------------------------------
>  global ASM_PFX(InternalSyncDecrement)
> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.asm b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.asm
> index 65abcf7..7788557 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.asm
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.asm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; 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
> @@ -26,8 +26,8 @@
>  ;------------------------------------------------------------------------------
>  ; UINT32
>  ; EFIAPI
> -; InterlockedIncrement (
> -;   IN      UINT32                    *Value
> +; InternalSyncIncrement (
> +;   IN      volatile UINT32                    *Value
>  ;   );

(15) Ditto.

>  ;------------------------------------------------------------------------------
>  InternalSyncIncrement   PROC
> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c
> index c0deb60..2b60c2d 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c
> @@ -1,7 +1,7 @@
>  /** @file
>    InterLockedIncrement function
>  
> -  Copyright (c) 2006 - 2008, Intel Corporation. All rights reserved.<BR>
> +  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
> @@ -38,9 +38,9 @@ long _InterlockedIncrement(
>  UINT32
>  EFIAPI
>  InternalSyncIncrement (
> -  IN      UINT32                    *Value
> +  IN      volatile UINT32                    *Value
>    )
>  {
> -  return _InterlockedIncrement (Value);
> +  return _InterlockedIncrement ((long *)(UINTN)(Value));
>  }
>  

Looks okay, based on InternalSyncDecrement() above.

> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.nasm b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.nasm
> index 59d90be..53f2fb5 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.nasm
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.nasm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2006, Intel Corporation. All rights reserved.<BR>
> +; 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
> @@ -27,8 +27,8 @@
>  ;------------------------------------------------------------------------------
>  ; UINT32
>  ; EFIAPI
> -; InterlockedIncrement (
> -;   IN      UINT32                    *Value
> +; InternalSyncIncrement (
> +;   IN      volatile UINT32                    *Value
>  ;   );
>  ;------------------------------------------------------------------------------

(16) Function renaming shouldn't be needed.

>  global ASM_PFX(InternalSyncIncrement)
> 

Thanks
Laszlo


  reply	other threads:[~2016-11-17  9:21 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-17  4:53 [Patch 0/2] MdePkg/BaseSynchronizationLib: Add volatile Michael Kinney
2016-11-17  4:53 ` [Patch 1/2] MdePkg/Include: Add volatile to SynchronizationLib parameters Michael Kinney
2016-11-17  8:51   ` Laszlo Ersek
2016-11-17  4:53 ` [Patch 2/2] MdePkg/BaseSynchronizationLib: Add volatile Interlocked*() APIs Michael Kinney
2016-11-17  9:21   ` Laszlo Ersek [this message]
2016-11-17 15:55     ` Kinney, Michael D
2016-11-17 16:02       ` Laszlo Ersek
2016-11-17 16:08         ` 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=b7440392-6051-4edb-3f00-cb76ef096af7@redhat.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