On 13 June 2016 at 16:26, Ard Biesheuvel <[email protected]> wrote: > On some platforms, performing cache maintenance on regions that are backed > by NOR flash result in SErrors. Since cache maintenance is unnecessary in > that case, create a PEIM specific version that only performs said cache > maintenance in its constructor if the module is shadowed in RAM. To avoid > performing the cache maintenance if the MMU code is not used to begin with, > check that explicitly in the constructor. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <[email protected]>
Reviewed-by: Leif Lindholm <[email protected]> > --- > > As discussed in the thread dedicated to this subject, the preferred way of > addressing this to split off the MMU manipulation code from ArmLib into a > separate ArmMmuLib, but this affects other packages and platforms. So in the > mean time, let's merge this patch so that D02 can use the upstream ArmLib > unmodified. > > > ArmPkg/Library/ArmLib/AArch64/{AArch64LibConstructor.c => > AArch64BaseLibConstructor.c} | 0 > ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf > | 2 +- > ArmPkg/Library/ArmLib/AArch64/AArch64LibPei.inf > | 43 +++++++++++ > ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c > | 2 + > ArmPkg/Library/ArmLib/AArch64/AArch64PeiLibConstructor.c > | 75 ++++++++++++++++++++ > 5 files changed, 121 insertions(+), 1 deletion(-) > > diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c > b/ArmPkg/Library/ArmLib/AArch64/AArch64BaseLibConstructor.c > similarity index 100% > rename from ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c > rename to ArmPkg/Library/ArmLib/AArch64/AArch64BaseLibConstructor.c > diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf > b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf > index 58684e8492f2..ef9d261b910d 100644 > --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf > +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf > @@ -32,7 +32,7 @@ [Sources.AARCH64] > > ../Common/AArch64/ArmLibSupport.S > ../Common/ArmLib.c > - AArch64LibConstructor.c > + AArch64BaseLibConstructor.c > > [Packages] > ArmPkg/ArmPkg.dec > diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64LibPei.inf > b/ArmPkg/Library/ArmLib/AArch64/AArch64LibPei.inf > new file mode 100644 > index 000000000000..c8f0b97750d4 > --- /dev/null > +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64LibPei.inf > @@ -0,0 +1,43 @@ > +#/** @file > +# > +# Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR> > +# Portions 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 > +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > IMPLIED. > +# > +# > +#**/ > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = AArch64Lib > + FILE_GUID = ef20ddf5-b334-47b3-94cf-52ff44c29138 > + MODULE_TYPE = PEIM > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = ArmLib|PEIM PEI_CORE > + CONSTRUCTOR = AArch64LibConstructor > + > +[Sources.AARCH64] > + AArch64Lib.c > + AArch64Mmu.c > + AArch64ArchTimer.c > + ArmLibSupportV8.S > + AArch64Support.S > + AArch64ArchTimerSupport.S > + > + ../Common/AArch64/ArmLibSupport.S > + ../Common/ArmLib.c > + AArch64PeiLibConstructor.c > + > +[Packages] > + ArmPkg/ArmPkg.dec > + MdePkg/MdePkg.dec > + > +[LibraryClasses] > + MemoryAllocationLib > + CacheMaintenanceLib > diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c > b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c > index cf9b7222b47b..07864bac28e6 100644 > --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c > +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c > @@ -26,6 +26,8 @@ > // We use this index definition to define an invalid block entry > #define TT_ATTR_INDX_INVALID ((UINT32)~0) > > +INT32 HaveMmuRoutines = 1; > + > STATIC > UINT64 > ArmMemoryAttributeToPageAttribute ( > diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64PeiLibConstructor.c > b/ArmPkg/Library/ArmLib/AArch64/AArch64PeiLibConstructor.c > new file mode 100644 > index 000000000000..2de9c7c54ed9 > --- /dev/null > +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64PeiLibConstructor.c > @@ -0,0 +1,75 @@ > +#/* @file > +# > +# Copyright (c) 2016, Linaro 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 > +# > +# 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 <Base.h> > + > +#include <Library/ArmLib.h> > +#include <Library/CacheMaintenanceLib.h> > +#include <Library/DebugLib.h> > + > +// > +// This is a hack. We define a weak symbol with external linkage, which may > or > +// may not be overridden by a non-weak alternative that is defined with a non > +// zero value in the object that contains the MMU routines. Since static > +// libraries are pulled in on a per-object basis, and since the MMU object > will > +// only be pulled in if any of its other symbols are referenced by the client > +// module, we can use the value below to figure out whether the MMU routines > are > +// in use by this module, and decide whether cache maintenance of the > function > +// ArmReplaceLiveTranslationEntry () is required. > +// > +INT32 __attribute__((weak)) HaveMmuRoutines; > + > +EFI_STATUS > +EFIAPI > +AArch64LibConstructor ( > + IN EFI_PEI_FILE_HANDLE FileHandle, > + IN CONST EFI_PEI_SERVICES **PeiServices > + ) > +{ > + extern UINT32 ArmReplaceLiveTranslationEntrySize; > + > + EFI_FV_FILE_INFO FileInfo; > + EFI_STATUS Status; > + > + if (HaveMmuRoutines == 0) { > + return RETURN_SUCCESS; > + } > + > + ASSERT (FileHandle != NULL); > + > + Status = (*PeiServices)->FfsGetFileInfo (FileHandle, &FileInfo); > + ASSERT_EFI_ERROR (Status); > + > + // > + // Some platforms do not cope very well with cache maintenance being > + // performed on regions backed by NOR flash. Since the cache maintenance > + // is unnecessary to begin with in that case, perform it only when not > + // executing in place. > + // > + if ((UINTN)FileInfo.Buffer <= (UINTN)ArmReplaceLiveTranslationEntry && > + ((UINTN)FileInfo.Buffer + FileInfo.BufferSize >= > + (UINTN)ArmReplaceLiveTranslationEntry + > ArmReplaceLiveTranslationEntrySize)) { > + DEBUG ((EFI_D_INFO, "ArmLib: skipping cache maintence on XIP PEIM\n")); > + } else { > + DEBUG ((EFI_D_INFO, "ArmLib: performing cache maintence on shadowed > PEIM\n")); > + // > + // The ArmReplaceLiveTranslationEntry () helper function may be invoked > + // with the MMU off so we have to ensure that it gets cleaned to the PoC > + // > + WriteBackDataCacheRange (ArmReplaceLiveTranslationEntry, > + ArmReplaceLiveTranslationEntrySize); > + } > + > + return RETURN_SUCCESS; > +} > -- > 1.9.1 > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
