From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 38E4020945B96 for ; Wed, 20 Sep 2017 19:20:49 -0700 (PDT) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Sep 2017 19:23:55 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,423,1500966000"; d="scan'208";a="1197411028" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga001.fm.intel.com with ESMTP; 20 Sep 2017 19:23:55 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 20 Sep 2017 19:23:54 -0700 Received: from shsmsx152.ccr.corp.intel.com ([169.254.6.93]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.213]) with mapi id 14.03.0319.002; Thu, 21 Sep 2017 10:23:53 +0800 From: "Gao, Liming" To: "Kinney, Michael D" , Leif Lindholm CC: "edk2-devel@lists.01.org" , Ard Biesheuvel Thread-Topic: [edk2] [PATCH] MdePkg: add ARM/AARCH64 support to BaseCacheMaintenanceLib Thread-Index: AQHTMhOWDeEaJ69PmUS/qOJo0S4xYqK92U4w//+AXYCAACizgIABGbpQ Date: Thu, 21 Sep 2017 02:23:52 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E15B78B@SHSMSX152.ccr.corp.intel.com> References: <20170920132252.11761-1-leif.lindholm@linaro.org> <4A89E2EF3DFEDB4C8BFDE51014F606A14E15B35B@SHSMSX152.ccr.corp.intel.com> <20170920150559.4z35v6uv72rpoyic@bivouac.eciton.net> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] MdePkg: add ARM/AARCH64 support to BaseCacheMaintenanceLib X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Sep 2017 02:20:49 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Leif: I see current ArmLib in ArmPkg. It is not clean enough. ArmLib.h still in= cludes Chipset/ArmV7 header. Its function has no function header. When you = work the patch to move them to BaseLib, please make sure they align to Base= Lib style. Thanks Liming > -----Original Message----- > From: Kinney, Michael D > Sent: Thursday, September 21, 2017 1:32 AM > To: Leif Lindholm ; Gao, Liming ; Kinney, Michael D > Cc: edk2-devel@lists.01.org; Ard Biesheuvel > Subject: RE: [edk2] [PATCH] MdePkg: add ARM/AARCH64 support to BaseCacheM= aintenanceLib >=20 > Leif, >=20 > Adding the ARM specific functions to the BaseLib to provide access > to ARM specific CPU registers/instructions makes sense and is > what has already been done for IA32/X64/IPF/EBC. >=20 > Put the arch specific content in BaseLib.h inside #if clauses. >=20 > #if defined (MDE_CPU_ARM) >=20 > #if defined (MDE_CPU_AARCH64) >=20 > Or for content shared by ARM and AARCH64 put in: >=20 > #if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64) >=20 > I see a few ARM and AARCH64 elements are already in BaseLib.h. >=20 > Best regards, >=20 > Mike >=20 > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On > > Behalf Of Leif Lindholm > > Sent: Wednesday, September 20, 2017 8:06 AM > > To: Gao, Liming > > Cc: Kinney, Michael D ; edk2- > > devel@lists.01.org; Ard Biesheuvel > > Subject: Re: [edk2] [PATCH] MdePkg: add ARM/AARCH64 support to > > BaseCacheMaintenanceLib > > > > Hi Liming, > > > > I understand the purity argument, but the situation (without > > this > > patch) is that: > > 1) There is a non-functional BaseCacheMaintenanceLib > > ARM/AARCH64 > > implementation with misleading information in MdePkg. > > 2) ARM/AARCH64-based needs to include a different cache > > maintenance > > library than all other architectures. And they all need to > > include > > the same one. > > > > 2 is an issue for the logical conclusion of the RFC series I am > > about > > to post for creating common include files for "boilerplate" > > bits of > > .dsf/.fdf files. > > > > An alternative option would be to move ArmLib into MdePkg? > > A casual glance suggests to me that the corresponding X86 > > features > > (like AsmWbinvd) are exposed via BaseLib. Would you see any > > issues > > with merging the ArmLib functionality into BaseLib? > > > > Regards, > > > > Leif > > > > On Wed, Sep 20, 2017 at 02:45:30PM +0000, Gao, Liming wrote: > > > Leif: > > > This change lets MdePkg BaseCacheMaintenanceLib depend on > > ArmPkg > > > ArmLib. But, MdePkg is the basic package. It should not > > depend on > > > other package. I suggest to add this ARM specific > > > BaseCacheMaintenanceLib library instance into ArmPkg. > > > > > > Thanks > > > Liming > > > > -----Original Message----- > > > > From: Leif Lindholm [mailto:leif.lindholm@linaro.org] > > > > Sent: Wednesday, September 20, 2017 9:23 PM > > > > To: edk2-devel@lists.01.org > > > > Cc: Kinney, Michael D ; Gao, > > Liming ; Ard Biesheuvel > > > > > > > > Subject: [PATCH] MdePkg: add ARM/AARCH64 support to > > BaseCacheMaintenanceLib > > > > > > > > ARM platforms have been using a separately located library > > in ArmPkg for > > > > high-level cache maintenance calls. Resolve this anomaly by > > overwriting > > > > ArmCache.c with the contents of > > > > > > ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c, > > and add > > > > the ArmLib dependency for the affected architectures. > > > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > Signed-off-by: Leif Lindholm > > > > --- > > > > > > > > The intent is to delete the ArmPkg version once no upstream > > platforms > > > > are using it. > > > > > > > > MdePkg/Library/BaseCacheMaintenanceLib/ArmCache.c | 222 > > +++++---------------- > > > > .../BaseCacheMaintenanceLib.inf | 2 + > > > > 2 files changed, 55 insertions(+), 169 deletions(-) > > > > > > > > diff --git > > a/MdePkg/Library/BaseCacheMaintenanceLib/ArmCache.c > > b/MdePkg/Library/BaseCacheMaintenanceLib/ArmCache.c > > > > index 79c84a0982..0759e38cd4 100644 > > > > --- a/MdePkg/Library/BaseCacheMaintenanceLib/ArmCache.c > > > > +++ b/MdePkg/Library/BaseCacheMaintenanceLib/ArmCache.c > > > > @@ -1,67 +1,63 @@ > > > > /** @file > > > > - Cache Maintenance Functions. These functions vary by ARM > > architecture so the MdePkg > > > > - versions are null functions used to make sure things > > will compile. > > > > > > > > - Copyright (c) 2006 - 2009, Intel Corporation. All rights > > reserved.
> > > > - Portions copyright (c) 2008 - 2009, Apple Inc. All > > rights reserved.
> > > > + Copyright (c) 2008 - 2009, Apple Inc. All rights > > reserved.
> > > > + Copyright (c) 2011 - 2014, ARM Limited. All rights > > reserved. > > > > + > > > > 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. > > > > + 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 common header file for this module. > > > > -// > > > > #include > > > > +#include > > > > #include > > > > +#include > > > > > > > > -/** > > > > - Invalidates the entire instruction cache in cache > > coherency domain of the > > > > - calling CPU. > > > > - > > > > - Invalidates the entire instruction cache in cache > > coherency domain of the > > > > - calling CPU. > > > > +STATIC > > > > +VOID > > > > +CacheRangeOperation ( > > > > + IN VOID *Start, > > > > + IN UINTN Length, > > > > + IN LINE_OPERATION LineOperation, > > > > + IN UINTN LineLength > > > > + ) > > > > +{ > > > > + UINTN ArmCacheLineAlignmentMask =3D LineLength - 1; > > > > + > > > > + // Align address (rounding down) > > > > + UINTN AlignedAddress =3D (UINTN)Start - ((UINTN)Start & > > ArmCacheLineAlignmentMask); > > > > + UINTN EndAddress =3D (UINTN)Start + Length; > > > > + > > > > + // Perform the line operation on an address in each > > cache line > > > > + while (AlignedAddress < EndAddress) { > > > > + LineOperation(AlignedAddress); > > > > + AlignedAddress +=3D LineLength; > > > > + } > > > > + ArmDataSynchronizationBarrier (); > > > > +} > > > > > > > > -**/ > > > > VOID > > > > EFIAPI > > > > InvalidateInstructionCache ( > > > > VOID > > > > ) > > > > { > > > > - ASSERT(FALSE); > > > > + ASSERT (FALSE); > > > > } > > > > > > > > -/** > > > > - Invalidates a range of instruction cache lines in the > > cache coherency domain > > > > - of the calling CPU. > > > > - > > > > - Invalidates the instruction cache lines specified by > > Address and Length. If > > > > - Address is not aligned on a cache line boundary, then > > entire instruction > > > > - cache line containing Address is invalidated. If Address > > + Length is not > > > > - aligned on a cache line boundary, then the entire > > instruction cache line > > > > - containing Address + Length -1 is invalidated. This > > function may choose to > > > > - invalidate the entire instruction cache if that is more > > efficient than > > > > - invalidating the specified range. If Length is 0, then > > no instruction cache > > > > - lines are invalidated. Address is returned. > > > > - > > > > - If Length is greater than (MAX_ADDRESS - Address + 1), > > then ASSERT(). > > > > - > > > > - @param Address The base address of the instruction > > cache lines to > > > > - invalidate. If the CPU is in a physical > > addressing mode, then > > > > - Address is a physical address. If the > > CPU is in a virtual > > > > - addressing mode, then Address is a > > virtual address. > > > > - > > > > - @param Length The number of bytes to invalidate from > > the instruction cache. > > > > - > > > > - @return Address > > > > +VOID > > > > +EFIAPI > > > > +InvalidateDataCache ( > > > > + VOID > > > > + ) > > > > +{ > > > > + ASSERT (FALSE); > > > > +} > > > > > > > > -**/ > > > > VOID * > > > > EFIAPI > > > > InvalidateInstructionCacheRange ( > > > > @@ -69,56 +65,26 @@ InvalidateInstructionCacheRange ( > > > > IN UINTN Length > > > > ) > > > > { > > > > - ASSERT (Length <=3D MAX_ADDRESS - (UINTN)Address + 1); > > > > - ASSERT(FALSE); > > > > - return Address; > > > > -} > > > > + CacheRangeOperation (Address, Length, > > ArmCleanDataCacheEntryToPoUByMVA, > > > > + ArmDataCacheLineLength ()); > > > > + CacheRangeOperation (Address, Length, > > > > + ArmInvalidateInstructionCacheEntryToPoUByMVA, > > > > + ArmInstructionCacheLineLength ()); > > > > > > > > -/** > > > > - Writes back and invalidates the entire data cache in > > cache coherency domain > > > > - of the calling CPU. > > > > + ArmInstructionSynchronizationBarrier (); > > > > > > > > - Writes Back and Invalidates the entire data cache in > > cache coherency domain > > > > - of the calling CPU. This function guarantees that all > > dirty cache lines are > > > > - written back to system memory, and also invalidates all > > the data cache lines > > > > - in the cache coherency domain of the calling CPU. > > > > + return Address; > > > > +} > > > > > > > > -**/ > > > > VOID > > > > EFIAPI > > > > WriteBackInvalidateDataCache ( > > > > VOID > > > > ) > > > > { > > > > - ASSERT(FALSE); > > > > + ASSERT (FALSE); > > > > } > > > > > > > > -/** > > > > - Writes back and invalidates a range of data cache lines > > in the cache > > > > - coherency domain of the calling CPU. > > > > - > > > > - Writes back and invalidates the data cache lines > > specified by Address and > > > > - Length. If Address is not aligned on a cache line > > boundary, then entire data > > > > - cache line containing Address is written back and > > invalidated. If Address + > > > > - Length is not aligned on a cache line boundary, then the > > entire data cache > > > > - line containing Address + Length -1 is written back and > > invalidated. This > > > > - function may choose to write back and invalidate the > > entire data cache if > > > > - that is more efficient than writing back and > > invalidating the specified > > > > - range. If Length is 0, then no data cache lines are > > written back and > > > > - invalidated. Address is returned. > > > > - > > > > - If Length is greater than (MAX_ADDRESS - Address + 1), > > then ASSERT(). > > > > - > > > > - @param Address The base address of the data cache lines > > to write back and > > > > - invalidate. If the CPU is in a physical > > addressing mode, then > > > > - Address is a physical address. If the > > CPU is in a virtual > > > > - addressing mode, then Address is a > > virtual address. > > > > - @param Length The number of bytes to write back and > > invalidate from the > > > > - data cache. > > > > - > > > > - @return Address > > > > - > > > > -**/ > > > > VOID * > > > > EFIAPI > > > > WriteBackInvalidateDataCacheRange ( > > > > @@ -126,55 +92,20 @@ WriteBackInvalidateDataCacheRange ( > > > > IN UINTN Length > > > > ) > > > > { > > > > - ASSERT (Length <=3D MAX_ADDRESS - (UINTN)Address + 1); > > > > - ASSERT(FALSE); > > > > + CacheRangeOperation(Address, Length, > > ArmCleanInvalidateDataCacheEntryByMVA, > > > > + ArmDataCacheLineLength ()); > > > > return Address; > > > > } > > > > > > > > -/** > > > > - Writes back the entire data cache in cache coherency > > domain of the calling > > > > - CPU. > > > > - > > > > - Writes back the entire data cache in cache coherency > > domain of the calling > > > > - CPU. This function guarantees that all dirty cache lines > > are written back to > > > > - system memory. This function may also invalidate all the > > data cache lines in > > > > - the cache coherency domain of the calling CPU. > > > > - > > > > -**/ > > > > VOID > > > > EFIAPI > > > > WriteBackDataCache ( > > > > VOID > > > > ) > > > > { > > > > - ASSERT(FALSE); > > > > + ASSERT (FALSE); > > > > } > > > > > > > > -/** > > > > - Writes back a range of data cache lines in the cache > > coherency domain of the > > > > - calling CPU. > > > > - > > > > - Writes back the data cache lines specified by Address > > and Length. If Address > > > > - is not aligned on a cache line boundary, then entire > > data cache line > > > > - containing Address is written back. If Address + Length > > is not aligned on a > > > > - cache line boundary, then the entire data cache line > > containing Address + > > > > - Length -1 is written back. This function may choose to > > write back the entire > > > > - data cache if that is more efficient than writing back > > the specified range. > > > > - If Length is 0, then no data cache lines are written > > back. This function may > > > > - also invalidate all the data cache lines in the > > specified range of the cache > > > > - coherency domain of the calling CPU. Address is > > returned. > > > > - > > > > - If Length is greater than (MAX_ADDRESS - Address + 1), > > then ASSERT(). > > > > - > > > > - @param Address The base address of the data cache lines > > to write back. If > > > > - the CPU is in a physical addressing > > mode, then Address is a > > > > - physical address. If the CPU is in a > > virtual addressing > > > > - mode, then Address is a virtual address. > > > > - @param Length The number of bytes to write back from > > the data cache. > > > > - > > > > - @return Address > > > > - > > > > -**/ > > > > VOID * > > > > EFIAPI > > > > WriteBackDataCacheRange ( > > > > @@ -182,58 +113,11 @@ WriteBackDataCacheRange ( > > > > IN UINTN Length > > > > ) > > > > { > > > > - ASSERT (Length <=3D MAX_ADDRESS - (UINTN)Address + 1); > > > > - ASSERT(FALSE); > > > > + CacheRangeOperation(Address, Length, > > ArmCleanDataCacheEntryByMVA, > > > > + ArmDataCacheLineLength ()); > > > > return Address; > > > > } > > > > > > > > -/** > > > > - Invalidates the entire data cache in cache coherency > > domain of the calling > > > > - CPU. > > > > - > > > > - Invalidates the entire data cache in cache coherency > > domain of the calling > > > > - CPU. This function must be used with care because dirty > > cache lines are not > > > > - written back to system memory. It is typically used for > > cache diagnostics. If > > > > - the CPU does not support invalidation of the entire data > > cache, then a write > > > > - back and invalidate operation should be performed on the > > entire data cache. > > > > - > > > > -**/ > > > > -VOID > > > > -EFIAPI > > > > -InvalidateDataCache ( > > > > - VOID > > > > - ) > > > > -{ > > > > - ASSERT(FALSE); > > > > -} > > > > - > > > > -/** > > > > - Invalidates a range of data cache lines in the cache > > coherency domain of the > > > > - calling CPU. > > > > - > > > > - Invalidates the data cache lines specified by Address > > and Length. If Address > > > > - is not aligned on a cache line boundary, then entire > > data cache line > > > > - containing Address is invalidated. If Address + Length > > is not aligned on a > > > > - cache line boundary, then the entire data cache line > > containing Address + > > > > - Length -1 is invalidated. This function must never > > invalidate any cache lines > > > > - outside the specified range. If Length is 0, then no > > data cache lines are > > > > - invalidated. Address is returned. This function must be > > used with care > > > > - because dirty cache lines are not written back to system > > memory. It is > > > > - typically used for cache diagnostics. If the CPU does > > not support > > > > - invalidation of a data cache range, then a write back > > and invalidate > > > > - operation should be performed on the data cache range. > > > > - > > > > - If Length is greater than (MAX_ADDRESS - Address + 1), > > then ASSERT(). > > > > - > > > > - @param Address The base address of the data cache lines > > to invalidate. If > > > > - the CPU is in a physical addressing > > mode, then Address is a > > > > - physical address. If the CPU is in a > > virtual addressing mode, > > > > - then Address is a virtual address. > > > > - @param Length The number of bytes to invalidate from > > the data cache. > > > > - > > > > - @return Address > > > > - > > > > -**/ > > > > VOID * > > > > EFIAPI > > > > InvalidateDataCacheRange ( > > > > @@ -241,7 +125,7 @@ InvalidateDataCacheRange ( > > > > IN UINTN Length > > > > ) > > > > { > > > > - ASSERT (Length <=3D MAX_ADDRESS - (UINTN)Address + 1); > > > > - ASSERT(FALSE); > > > > + CacheRangeOperation(Address, Length, > > ArmInvalidateDataCacheEntryByMVA, > > > > + ArmDataCacheLineLength ()); > > > > return Address; > > > > } > > > > diff --git > > a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLi > > b.inf > > > > > > b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLi > > b.inf > > > > index d659161f33..7440a0062b 100644 > > > > --- > > a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLi > > b.inf > > > > +++ > > b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLi > > b.inf > > > > @@ -59,3 +59,5 @@ > > > > [LibraryClasses.Ipf] > > > > PalLib > > > > > > > > +[LibraryClasses.ARM,LibraryClasses.AARCH64] > > > > + ArmLib > > > > -- > > > > 2.11.0 > > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel