From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (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 7097E21ECCB23 for ; Wed, 20 Sep 2017 10:28:36 -0700 (PDT) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga105.fm.intel.com with ESMTP; 20 Sep 2017 10:31:41 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,422,1500966000"; d="scan'208";a="1016673157" Received: from orsmsx105.amr.corp.intel.com ([10.22.225.132]) by orsmga003.jf.intel.com with ESMTP; 20 Sep 2017 10:31:41 -0700 Received: from orsmsx114.amr.corp.intel.com (10.22.240.10) by ORSMSX105.amr.corp.intel.com (10.22.225.132) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 20 Sep 2017 10:31:41 -0700 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.135]) by ORSMSX114.amr.corp.intel.com ([169.254.8.209]) with mapi id 14.03.0319.002; Wed, 20 Sep 2017 10:31:40 -0700 From: "Kinney, Michael D" To: Leif Lindholm , "Gao, Liming" , "Kinney, Michael D" CC: "edk2-devel@lists.01.org" , Ard Biesheuvel Thread-Topic: [edk2] [PATCH] MdePkg: add ARM/AARCH64 support to BaseCacheMaintenanceLib Thread-Index: AQHTMhOYeG4UPCHu5kGUIv+SxCAox6K+T2cAgAAFuYD//7GH4A== Date: Wed, 20 Sep 2017 17:31:39 +0000 Message-ID: References: <20170920132252.11761-1-leif.lindholm@linaro.org> <4A89E2EF3DFEDB4C8BFDE51014F606A14E15B35B@SHSMSX152.ccr.corp.intel.com> <20170920150559.4z35v6uv72rpoyic@bivouac.eciton.net> In-Reply-To: <20170920150559.4z35v6uv72rpoyic@bivouac.eciton.net> 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.22.254.140] 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: Wed, 20 Sep 2017 17:28:36 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Leif, Adding the ARM specific functions to the BaseLib to provide access=20 to ARM specific CPU registers/instructions makes sense and is=20 what has already been done for IA32/X64/IPF/EBC. Put the arch specific content in BaseLib.h inside #if clauses. #if defined (MDE_CPU_ARM) #if defined (MDE_CPU_AARCH64) Or for content shared by ARM and AARCH64 put in: #if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64) I see a few ARM and AARCH64 elements are already in BaseLib.h. Best regards, Mike > -----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 >=20 > Hi Liming, >=20 > 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. >=20 > 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. >=20 > 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? >=20 > Regards, >=20 > Leif >=20 > 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