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

Reply via email to