Star,

I agree with it. I will send the v3 for review. Thanks!

Jeff

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zeng, 
Star
Sent: Tuesday, March 29, 2016 4:59 PM
To: Fan, Jeff; edk2-devel@lists.01.org
Cc: Kinney, Michael D; Gao, Liming
Subject: Re: [edk2] [Patch v2] MdePkg/BaseSynchronizationLib: Add spin lock 
alignment for IA32/x64

Do we need add tool chain family | MSFT and | GCC to new added 
InternalGetSpinLockProperties.c?

Thanks,
Star

On 2016/3/22 15:40, Jeff Fan wrote:
>  From Intel(R) 64 and IA-32 Architectures Software Developer's Manual, 
> one lock or semaphore is suggested to be present within a cache line. 
> If the processors are based on Intel NetBurst microarchitecture, two cache 
> lines are suggested.
> This could minimize the bus traffic required to service locks.
>
> Cc: Michael Kinney <michael.d.kin...@intel.com>
> Cc: Liming Gao <liming....@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan <jeff....@intel.com>
> ---
>   .../BaseSynchronizationLib.inf                     |  8 ++-
>   .../BaseSynchronizationLibInternals.h              | 14 ++++-
>   .../Ia32/InternalGetSpinLockProperties.c           | 60 
> ++++++++++++++++++++++
>   .../Ipf/InternalGetSpinLockProperties.c            | 29 +++++++++++
>   .../BaseSynchronizationLib/SynchronizationGcc.c    |  2 +-
>   .../BaseSynchronizationLib/SynchronizationMsc.c    |  2 +-
>   6 files changed, 111 insertions(+), 4 deletions(-)
>   create mode 100644 
> MdePkg/Library/BaseSynchronizationLib/Ia32/InternalGetSpinLockProperties.c
>   create mode 100644 
> MdePkg/Library/BaseSynchronizationLib/Ipf/InternalGetSpinLockPropertie
> s.c
>
> diff --git 
> a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf 
> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> index bd1bec3..6930d21 100755
> --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> +++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> @@ -1,7 +1,7 @@
>   ## @file
>   #  Base Synchronization Library implementation.
>   #
> -#  Copyright (c) 2007 - 2014, Intel Corporation. All rights 
> reserved.<BR>
> +#  Copyright (c) 2007 - 2016, Intel Corporation. All rights 
> +reserved.<BR>
>   #  Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
>   #
>   #  This program and the accompanying materials @@ -30,6 +30,8 @@
>     BaseSynchronizationLibInternals.h
>
>   [Sources.IA32]
> +  Ia32/InternalGetSpinLockProperties.c
> +
>     Ia32/InterlockedCompareExchange64.c | MSFT
>     Ia32/InterlockedCompareExchange32.c | MSFT
>     Ia32/InterlockedCompareExchange16.c | MSFT @@ -48,6 +50,8 @@
>     SynchronizationGcc.c  | GCC
>
>   [Sources.X64]
> +  Ia32/InternalGetSpinLockProperties.c
> +
>     X64/InterlockedCompareExchange64.c | MSFT
>     X64/InterlockedCompareExchange32.c | MSFT
>     X64/InterlockedCompareExchange16.c | MSFT @@ -68,6 +72,8 @@
>     SynchronizationGcc.c  | GCC
>
>   [Sources.IPF]
> +  Ipf/InternalGetSpinLockProperties.c
> +
>     Ipf/Synchronization.c
>     Ipf/InterlockedCompareExchange64.s
>     Ipf/InterlockedCompareExchange32.s
> diff --git 
> a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternal
> s.h 
> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternal
> s.h
> index 76f7023..9716b92 100644
> --- 
> a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternal
> s.h
> +++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInte
> +++ rnals.h
> @@ -1,7 +1,7 @@
>   /** @file
>     Declaration of internal functions in BaseSynchronizationLib.
>
> -  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 @@ -138,4 +138,16 @@ InternalSyncCompareExchange64 (
>     IN      UINT64                    ExchangeValue
>     );
>
> +/**
> +  Internal function to retrieve the architecture specific spin lock 
> +alignment
> +  requirements for optimal spin lock performance.
> +
> +  @return The architecture specific spin lock alignment.
> +
> +**/
> +UINTN
> +InternalGetSpinLockProperties (
> +  VOID
> +  );
> +
>   #endif
> diff --git 
> a/MdePkg/Library/BaseSynchronizationLib/Ia32/InternalGetSpinLockProper
> ties.c 
> b/MdePkg/Library/BaseSynchronizationLib/Ia32/InternalGetSpinLockProper
> ties.c
> new file mode 100644
> index 0000000..49f05fb
> --- /dev/null
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/InternalGetSpinLockPr
> +++ operties.c
> @@ -0,0 +1,60 @@
> +/** @file
> +  Internal function to get spin lock alignment.
> +
> +  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>  
> + This program and the accompanying materials  are licensed and made 
> + available under the terms and conditions of the BSD License  which 
> + accompanies this distribution.  The full text of the license may be 
> + found at  http://opensource.org/licenses/bsd-license.php.
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" 
> + BASIS,  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS 
> OR IMPLIED.
> +
> +**/
> +
> +#include "BaseSynchronizationLibInternals.h"
> +
> +/**
> +  Internal function to retrieve the architecture specific spin lock 
> +alignment
> +  requirements for optimal spin lock performance.
> +
> +  @return The architecture specific spin lock alignment.
> +
> +**/
> +UINTN
> +InternalGetSpinLockProperties (
> +  VOID
> +  )
> +{
> +  UINT32  RegEax;
> +  UINT32  RegEbx;
> +  UINTN   FamilyId;
> +  UINTN   ModelId;
> +  UINTN   CacheLineSize;
> +
> +  //
> +  // Retrieve CPUID Version Information  //  AsmCpuid (0x01, &RegEax, 
> + &RegEbx, NULL, NULL);  //  // EBX: Bits 15 - 08: CLFLUSH line size 
> + (Value * 8 = cache line size)  //  CacheLineSize = ((RegEbx >> 8) & 
> + 0xff) * 8;  //  // Retrieve CPU Family and Model  //  FamilyId = 
> + (RegEax >> 8) & 0xf;  ModelId  = (RegEax >> 4) & 0xf;  if (FamilyId 
> + == 0x0f) {
> +    //
> +    // In processors based on Intel NetBurst microarchitecture, use two 
> cache lines
> +    //
> +    ModelId = ModelId | ((RegEax >> 12) & 0xf0);
> +    if (ModelId <= 0x04 || ModelId == 0x06) {
> +      CacheLineSize *= 2;
> +    }
> +  }
> +
> +  return CacheLineSize;
> +}
> +
> diff --git 
> a/MdePkg/Library/BaseSynchronizationLib/Ipf/InternalGetSpinLockPropert
> ies.c 
> b/MdePkg/Library/BaseSynchronizationLib/Ipf/InternalGetSpinLockPropert
> ies.c
> new file mode 100644
> index 0000000..f6464c2
> --- /dev/null
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ipf/InternalGetSpinLockPro
> +++ perties.c
> @@ -0,0 +1,29 @@
> +/** @file
> +  Internal function to get spin lock alignment.
> +
> +  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>  
> + This program and the accompanying materials  are licensed and made 
> + available under the terms and conditions of the BSD License  which 
> + accompanies this distribution.  The full text of the license may be 
> + found at  http://opensource.org/licenses/bsd-license.php.
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" 
> + BASIS,  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS 
> OR IMPLIED.
> +
> +**/
> +
> +/**
> +  Internal function to retrieve the architecture specific spin lock 
> +alignment
> +  requirements for optimal spin lock performance.
> +
> +  @return The architecture specific spin lock alignment.
> +
> +**/
> +UINTN
> +InternalGetSpinLockProperties (
> +  VOID
> +  )
> +{
> +  return 32;
> +}
> +
> diff --git 
> a/MdePkg/Library/BaseSynchronizationLib/SynchronizationGcc.c 
> b/MdePkg/Library/BaseSynchronizationLib/SynchronizationGcc.c
> index 31f28ec..4b8c8e5 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/SynchronizationGcc.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/SynchronizationGcc.c
> @@ -45,7 +45,7 @@ GetSpinLockProperties (
>     VOID
>     )
>   {
> -  return 32;
> +  return InternalGetSpinLockProperties ();
>   }
>
>   /**
> diff --git 
> a/MdePkg/Library/BaseSynchronizationLib/SynchronizationMsc.c 
> b/MdePkg/Library/BaseSynchronizationLib/SynchronizationMsc.c
> index 821c798..db344b7 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/SynchronizationMsc.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/SynchronizationMsc.c
> @@ -47,7 +47,7 @@ GetSpinLockProperties (
>     VOID
>     )
>   {
> -  return 32;
> +  return InternalGetSpinLockProperties ();
>   }
>
>   /**
>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to