From: "Kinney, Michael D" <michael.d.kinney@intel.com>
To: Laszlo Ersek <lersek@redhat.com>,
"edk2-devel@ml01.01.org" <edk2-devel@ml01.01.org>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: "Gao, Liming" <liming.gao@intel.com>,
Andrew Fish <afish@apple.com>, "Fan, Jeff" <jeff.fan@intel.com>
Subject: Re: [Patch 2/2] MdePkg/BaseSynchronizationLib: Add volatile Interlocked*() APIs
Date: Thu, 17 Nov 2016 16:08:02 +0000 [thread overview]
Message-ID: <E92EE9817A31E24EB0585FDF735412F56485767D@ORSMSX113.amr.corp.intel.com> (raw)
In-Reply-To: <094f833f-32ef-0eed-9236-45bdb77fde51@redhat.com>
Laszlo,
I agree. I will do a different patch to fix the function names
in comments.
Mike
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, November 17, 2016 8:02 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@ml01.01.org
> Cc: Gao, Liming <liming.gao@intel.com>; Andrew Fish <afish@apple.com>; Fan, Jeff
> <jeff.fan@intel.com>
> Subject: Re: [Patch 2/2] MdePkg/BaseSynchronizationLib: Add volatile Interlocked*()
> APIs
>
> On 11/17/16 16:55, Kinney, Michael D wrote:
> > Laszlo,
> >
> > Thanks for the feedback. I will update and send V2.
> >
> > I did not change the names of any functions. The comment
> > blocks had names that did not match the name of the function,
> > so I was only fixing the comment.
>
> Ah! Thank you. That's a completely valid set of updates then; but I
> think it should be in a different patch.
>
> Cheers
> Laszlo
>
> >> -----Original Message-----
> >> From: Laszlo Ersek [mailto:lersek@redhat.com]
> >> Sent: Thursday, November 17, 2016 1:21 AM
> >> To: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@ml01.01.org
> >> Cc: Gao, Liming <liming.gao@intel.com>; Andrew Fish <afish@apple.com>; Fan, Jeff
> >> <jeff.fan@intel.com>
> >> Subject: Re: [Patch 2/2] MdePkg/BaseSynchronizationLib: Add volatile
> Interlocked*()
> >> APIs
> >>
> >> 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
prev parent reply other threads:[~2016-11-17 16:07 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
2016-11-17 15:55 ` Kinney, Michael D
2016-11-17 16:02 ` Laszlo Ersek
2016-11-17 16:08 ` Kinney, Michael D [this message]
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=E92EE9817A31E24EB0585FDF735412F56485767D@ORSMSX113.amr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox