On 13 June 2016 at 16:26, Ard Biesheuvel <ard.biesheu...@linaro.org> 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 <ard.biesheu...@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindh...@linaro.org>
> ---
>
> 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
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
