On Thu, Jun 16, 2016 at 12:29:30PM +0200, Ard Biesheuvel wrote:
> This introduces a special version of ArmMmuLib for PEIMs that takes care
> only to perform cache maintenance on the live entry replacement routine
> if the module is not executing in place. Not only is such cache maintenance
> unnecessary in that case, it may be actively harmful on some systems that
> fail to tolerate cache maintenance operations on NOR flash regions.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuPeiLibConstructor.c | 60
> ++++++++++++++++++++
> ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf | 36 ++++++++++++
> 2 files changed, 96 insertions(+)
>
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuPeiLibConstructor.c
> b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuPeiLibConstructor.c
> new file mode 100644
> index 000000000000..91dc1157e79a
> --- /dev/null
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuPeiLibConstructor.c
> @@ -0,0 +1,60 @@
> +#/* @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/ArmMmuLib.h>
> +#include <Library/CacheMaintenanceLib.h>
> +#include <Library/DebugLib.h>
> +
> +EFI_STATUS
> +EFIAPI
> +ArmMmuPeiLibConstructor (
> + IN EFI_PEI_FILE_HANDLE FileHandle,
> + IN CONST EFI_PEI_SERVICES **PeiServices
> + )
> +{
> + extern UINT32 ArmReplaceLiveTranslationEntrySize;
> +
> + EFI_FV_FILE_INFO FileInfo;
> + EFI_STATUS Status;
> +
> + 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.
> + //
I think perhaps the wording of the above is confusing me a little bit;
the thing that makes it not matter is that it is executing in place,
not that it is backed by NOR, right? Could the statement be flipped?:
"Perform it only when not executing in place, since otherwise the
cache maintenance is unnecessary."
Someone who is less fond of multiple negations in a singe sentence
than me may prefer to reword it further...
/
Leif
> + if ((UINTN)FileInfo.Buffer <= (UINTN)ArmReplaceLiveTranslationEntry &&
> + ((UINTN)FileInfo.Buffer + FileInfo.BufferSize >=
> + (UINTN)ArmReplaceLiveTranslationEntry +
> ArmReplaceLiveTranslationEntrySize)) {
> + DEBUG ((EFI_D_INFO, "ArmMmuLib: skipping cache maintenance on XIP
> PEIM\n"));
> + } else {
> + DEBUG ((EFI_D_INFO, "ArmMmuLib: performing cache maintenance 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;
> +}
> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> new file mode 100644
> index 000000000000..14ebf8de673d
> --- /dev/null
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> @@ -0,0 +1,36 @@
> +#/** @file
> +#
> +# Copyright (c) 2016 Linaro Ltd. 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 = ArmMmuPeiLib
> + FILE_GUID = b50d8d53-1ad1-44ea-9e69-8c89d4a6d08b
> + MODULE_TYPE = PEIM
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = ArmMmuLib|PEIM
> + CONSTRUCTOR = ArmMmuPeiLibConstructor
> +
> +[Sources.AARCH64]
> + AArch64/ArmMmuLibCore.c
> + AArch64/ArmMmuPeiLibConstructor.c
> + AArch64/ArmMmuLibReplaceEntry.S
> +
> +[Packages]
> + ArmPkg/ArmPkg.dec
> + MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> + ArmLib
> + CacheMaintenanceLib
> + MemoryAllocationLib
> --
> 1.9.1
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel