public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 1/2] ArmPkg: remove DebugUncachedMemoryAllocationLib
@ 2017-02-23 15:48 Ard Biesheuvel
  2017-02-23 15:48 ` [PATCH 2/2] ArmPkg/CpuDxe: remove VirtualUncachedPages protocol and implementation Ard Biesheuvel
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2017-02-23 15:48 UTC (permalink / raw)
  To: edk2-devel, leif.lindholm, lersek; +Cc: ryan.harkin, Ard Biesheuvel

The debug implementation of the UncachedMemoryAllocationLib library
class relies on the creation of an uncached alias of a memory range,
while keeping the original cached mapping, but with read-only attributes
to trap inadvertent write accesses.

This is not a terribly good idea, given that the ARM architecture does
not allow mismatched attributes, and so creating them deliberately is
not something we should encourage by doing it in reference code.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/ArmPkg.dsc                                                                    |   1 -
 ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.c   | 656 --------------------
 ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf |  47 --
 ArmVirtPkg/ArmVirt.dsc.inc                                                           |   2 -
 BeagleBoardPkg/BeagleBoardPkg.dsc                                                    |   1 -
 Omap35xxPkg/Omap35xxPkg.dsc                                                          |   3 +-
 6 files changed, 1 insertion(+), 709 deletions(-)

diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index 0db33eb865b1..1a490d23f7b5 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -112,7 +112,6 @@ [Components.common]
   ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
   ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf
   ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
-  ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
   ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf
   ArmPkg/Library/RvdPeCoffExtraActionLib/RvdPeCoffExtraActionLib.inf
   ArmPkg/Library/SemiHostingDebugLib/SemiHostingDebugLib.inf
diff --git a/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.c b/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.c
deleted file mode 100644
index 00e01a905c85..000000000000
--- a/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.c
+++ /dev/null
@@ -1,656 +0,0 @@
-/** @file
-  Debug version of the UncachedMemoryAllocation lib that uses the VirtualUncachedPages
-  protocol, produced by the DXE CPU driver, to produce debuggable uncached memory buffers.
-
-  The DMA rules for EFI contain the concept of a PCI (DMA master) address for memory and
-  a CPU (C code) address for the memory buffer that don't have to be the same.  There seem to
-  be common errors out there with folks mixing up the two addresses.  This library causes
-  the PCI (DMA master) address to not be mapped into system memory so if the CPU (C code)
-  uses the wrong pointer it will generate a page fault. The CPU (C code) version of the buffer
-  has a virtual address that does not match the physical address. The virtual address has
-  PcdArmUncachedMemoryMask ored into the physical address.
-
-  Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
-
-  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/BaseLib.h>
-#include <Library/BaseMemoryLib.h>
-#include <Library/MemoryAllocationLib.h>
-#include <Library/DebugLib.h>
-#include <Library/UefiBootServicesTableLib.h>
-#include <Library/UncachedMemoryAllocationLib.h>
-#include <Library/PcdLib.h>
-#include <Library/ArmLib.h>
-
-#include <Protocol/Cpu.h>
-#include <Protocol/VirtualUncachedPages.h>
-
-VOID *
-UncachedInternalAllocatePages (
-  IN EFI_MEMORY_TYPE  MemoryType,
-  IN UINTN            Pages
-  );
-
-VOID *
-UncachedInternalAllocateAlignedPages (
-  IN EFI_MEMORY_TYPE  MemoryType,
-  IN UINTN            Pages,
-  IN UINTN            Alignment
-  );
-
-
-
-EFI_CPU_ARCH_PROTOCOL           *gDebugUncachedCpu;
-VIRTUAL_UNCACHED_PAGES_PROTOCOL *gVirtualUncachedPages;
-
-//
-// Assume all of memory has the same cache attributes, unless we do our magic
-//
-UINT64  gAttributes;
-
-typedef struct {
-  VOID        *Buffer;
-  VOID        *Allocation;
-  UINTN       Pages;
-  LIST_ENTRY  Link;
-} FREE_PAGE_NODE;
-
-LIST_ENTRY  mPageList = INITIALIZE_LIST_HEAD_VARIABLE (mPageList);
-
-VOID
-AddPagesToList (
-  IN VOID   *Buffer,
-  IN VOID   *Allocation,
-  UINTN     Pages
-  )
-{
-  FREE_PAGE_NODE  *NewNode;
-
-  NewNode = AllocatePool (sizeof (LIST_ENTRY));
-  if (NewNode == NULL) {
-    ASSERT (FALSE);
-    return;
-  }
-
-  NewNode->Buffer     = Buffer;
-  NewNode->Allocation = Allocation;
-  NewNode->Pages      = Pages;
-
-  InsertTailList (&mPageList, &NewNode->Link);
-}
-
-
-VOID
-RemovePagesFromList (
-  IN VOID   *Buffer,
-  OUT VOID  **Allocation,
-  OUT UINTN *Pages
-  )
-{
-  LIST_ENTRY      *Link;
-  FREE_PAGE_NODE  *OldNode;
-
-  *Allocation = NULL;
-  *Pages = 0;
-
-  for (Link = mPageList.ForwardLink; Link != &mPageList; Link = Link->ForwardLink) {
-    OldNode = BASE_CR (Link, FREE_PAGE_NODE, Link);
-    if (OldNode->Buffer == Buffer) {
-      *Allocation = OldNode->Allocation;
-      *Pages = OldNode->Pages;
-
-      RemoveEntryList (&OldNode->Link);
-      FreePool (OldNode);
-      return;
-    }
-  }
-
-  return;
-}
-
-
-
-EFI_PHYSICAL_ADDRESS
-ConvertToPhysicalAddress (
-  IN VOID *VirtualAddress
-  )
-{
-  UINTN UncachedMemoryMask = (UINTN)PcdGet64 (PcdArmUncachedMemoryMask);
-  UINTN PhysicalAddress;
-
-  PhysicalAddress = (UINTN)VirtualAddress & ~UncachedMemoryMask;
-
-  return (EFI_PHYSICAL_ADDRESS)PhysicalAddress;
-}
-
-
-VOID *
-ConvertToUncachedAddress (
-  IN VOID *Address
-  )
-{
-  UINTN UncachedMemoryMask = (UINTN)PcdGet64 (PcdArmUncachedMemoryMask);
-  UINTN UncachedAddress;
-
-  UncachedAddress = (UINTN)Address | UncachedMemoryMask;
-
-  return (VOID *)UncachedAddress;
-}
-
-
-
-VOID *
-UncachedInternalAllocatePages (
-  IN EFI_MEMORY_TYPE  MemoryType,
-  IN UINTN            Pages
-  )
-{
-  return UncachedInternalAllocateAlignedPages (MemoryType, Pages, EFI_PAGE_SIZE);
-}
-
-
-VOID *
-EFIAPI
-UncachedAllocatePages (
-  IN UINTN  Pages
-  )
-{
-  return UncachedInternalAllocatePages (EfiBootServicesData, Pages);
-}
-
-VOID *
-EFIAPI
-UncachedAllocateRuntimePages (
-  IN UINTN  Pages
-  )
-{
-  return UncachedInternalAllocatePages (EfiRuntimeServicesData, Pages);
-}
-
-VOID *
-EFIAPI
-UncachedAllocateReservedPages (
-  IN UINTN  Pages
-  )
-{
-  return UncachedInternalAllocatePages (EfiReservedMemoryType, Pages);
-}
-
-
-
-VOID
-EFIAPI
-UncachedFreePages (
-  IN VOID   *Buffer,
-  IN UINTN  Pages
-  )
-{
-  UncachedFreeAlignedPages (Buffer, Pages);
-  return;
-}
-
-
-VOID *
-UncachedInternalAllocateAlignedPages (
-  IN EFI_MEMORY_TYPE  MemoryType,
-  IN UINTN            Pages,
-  IN UINTN            Alignment
-  )
-{
-  EFI_STATUS            Status;
-  EFI_PHYSICAL_ADDRESS  Memory;
-  EFI_PHYSICAL_ADDRESS  AlignedMemory;
-  UINTN                 AlignmentMask;
-  UINTN                 UnalignedPages;
-  UINTN                 RealPages;
-
-  //
-  // Alignment must be a power of two or zero.
-  //
-  ASSERT ((Alignment & (Alignment - 1)) == 0);
-
-  if (Pages == 0) {
-    return NULL;
-  }
-  if (Alignment > EFI_PAGE_SIZE) {
-    //
-    // Caculate the total number of pages since alignment is larger than page size.
-    //
-    AlignmentMask  = Alignment - 1;
-    RealPages      = Pages + EFI_SIZE_TO_PAGES (Alignment);
-    //
-    // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not overflow.
-    //
-    ASSERT (RealPages > Pages);
-
-    Status         = gBS->AllocatePages (AllocateAnyPages, MemoryType, RealPages, &Memory);
-    if (EFI_ERROR (Status)) {
-      return NULL;
-    }
-    AlignedMemory  = ((UINTN) Memory + AlignmentMask) & ~AlignmentMask;
-    UnalignedPages = EFI_SIZE_TO_PAGES (AlignedMemory - (UINTN) Memory);
-    if (UnalignedPages > 0) {
-      //
-      // Free first unaligned page(s).
-      //
-      Status = gBS->FreePages (Memory, UnalignedPages);
-      ASSERT_EFI_ERROR (Status);
-    }
-    Memory         = (EFI_PHYSICAL_ADDRESS) (AlignedMemory + EFI_PAGES_TO_SIZE (Pages));
-    UnalignedPages = RealPages - Pages - UnalignedPages;
-    if (UnalignedPages > 0) {
-      //
-      // Free last unaligned page(s).
-      //
-      Status = gBS->FreePages (Memory, UnalignedPages);
-      ASSERT_EFI_ERROR (Status);
-    }
-  } else {
-    //
-    // Do not over-allocate pages in this case.
-    //
-    Status = gBS->AllocatePages (AllocateAnyPages, MemoryType, Pages, &Memory);
-    if (EFI_ERROR (Status)) {
-      return NULL;
-    }
-    AlignedMemory  = (UINTN) Memory;
-  }
-
-  Status = gVirtualUncachedPages->ConvertPages (gVirtualUncachedPages, AlignedMemory, Pages * EFI_PAGE_SIZE, PcdGet64 (PcdArmUncachedMemoryMask), &gAttributes);
-  if (EFI_ERROR (Status)) {
-    return NULL;
-  }
-
-  AlignedMemory = (EFI_PHYSICAL_ADDRESS)(UINTN)ConvertToUncachedAddress ((VOID *)(UINTN)AlignedMemory);
-
-  return (VOID *)(UINTN)AlignedMemory;
-}
-
-
-VOID
-EFIAPI
-UncachedFreeAlignedPages (
-  IN VOID   *Buffer,
-  IN UINTN  Pages
-  )
-{
-  EFI_STATUS            Status;
-  EFI_PHYSICAL_ADDRESS  Memory;
-
-  ASSERT (Pages != 0);
-
-  Memory = ConvertToPhysicalAddress (Buffer);
-
-  Status = gVirtualUncachedPages->RevertPages (gVirtualUncachedPages, Memory, Pages * EFI_PAGE_SIZE, PcdGet64 (PcdArmUncachedMemoryMask), gAttributes);
-
-
-  Status = gBS->FreePages ((EFI_PHYSICAL_ADDRESS) (UINTN) Memory, Pages);
-  ASSERT_EFI_ERROR (Status);
-}
-
-
-
-
-VOID *
-UncachedInternalAllocateAlignedPool (
-  IN EFI_MEMORY_TYPE  PoolType,
-  IN UINTN            AllocationSize,
-  IN UINTN            Alignment
-  )
-{
-  VOID      *AlignedAddress;
-
-  //
-  // Alignment must be a power of two or zero.
-  //
-  ASSERT ((Alignment & (Alignment - 1)) == 0);
-
-  if (Alignment < EFI_PAGE_SIZE) {
-    Alignment = EFI_PAGE_SIZE;
-  }
-
-  AlignedAddress = UncachedInternalAllocateAlignedPages (PoolType, EFI_SIZE_TO_PAGES (AllocationSize), Alignment);
-  if (AlignedAddress == NULL) {
-    return NULL;
-  }
-
-  AddPagesToList ((VOID *)(UINTN)ConvertToPhysicalAddress (AlignedAddress), (VOID *)(UINTN)AlignedAddress, EFI_SIZE_TO_PAGES (AllocationSize));
-
-  return (VOID *) AlignedAddress;
-}
-
-VOID *
-EFIAPI
-UncachedAllocateAlignedPool (
-  IN UINTN  AllocationSize,
-  IN UINTN  Alignment
-  )
-{
-  return UncachedInternalAllocateAlignedPool (EfiBootServicesData, AllocationSize, Alignment);
-}
-
-VOID *
-EFIAPI
-UncachedAllocateAlignedRuntimePool (
-  IN UINTN  AllocationSize,
-  IN UINTN  Alignment
-  )
-{
-  return UncachedInternalAllocateAlignedPool (EfiRuntimeServicesData, AllocationSize, Alignment);
-}
-
-VOID *
-EFIAPI
-UncachedAllocateAlignedReservedPool (
-  IN UINTN  AllocationSize,
-  IN UINTN  Alignment
-  )
-{
-  return UncachedInternalAllocateAlignedPool (EfiReservedMemoryType, AllocationSize, Alignment);
-}
-
-VOID *
-UncachedInternalAllocateAlignedZeroPool (
-  IN EFI_MEMORY_TYPE  PoolType,
-  IN UINTN            AllocationSize,
-  IN UINTN            Alignment
-  )
-{
-  VOID    *Memory;
-  Memory = UncachedInternalAllocateAlignedPool (PoolType, AllocationSize, Alignment);
-  if (Memory != NULL) {
-    Memory = ZeroMem (Memory, AllocationSize);
-  }
-  return Memory;
-}
-
-VOID *
-EFIAPI
-UncachedAllocateAlignedZeroPool (
-  IN UINTN  AllocationSize,
-  IN UINTN  Alignment
-  )
-{
-  return UncachedInternalAllocateAlignedZeroPool (EfiBootServicesData, AllocationSize, Alignment);
-}
-
-VOID *
-EFIAPI
-UncachedAllocateAlignedRuntimeZeroPool (
-  IN UINTN  AllocationSize,
-  IN UINTN  Alignment
-  )
-{
-  return UncachedInternalAllocateAlignedZeroPool (EfiRuntimeServicesData, AllocationSize, Alignment);
-}
-
-VOID *
-EFIAPI
-UncachedAllocateAlignedReservedZeroPool (
-  IN UINTN  AllocationSize,
-  IN UINTN  Alignment
-  )
-{
-  return UncachedInternalAllocateAlignedZeroPool (EfiReservedMemoryType, AllocationSize, Alignment);
-}
-
-VOID *
-UncachedInternalAllocateAlignedCopyPool (
-  IN EFI_MEMORY_TYPE  PoolType,
-  IN UINTN            AllocationSize,
-  IN CONST VOID       *Buffer,
-  IN UINTN            Alignment
-  )
-{
-  VOID  *Memory;
-
-  ASSERT (Buffer != NULL);
-  ASSERT (AllocationSize <= (MAX_ADDRESS - (UINTN) Buffer + 1));
-
-  Memory = UncachedInternalAllocateAlignedPool (PoolType, AllocationSize, Alignment);
-  if (Memory != NULL) {
-    Memory = CopyMem (Memory, Buffer, AllocationSize);
-  }
-  return Memory;
-}
-
-VOID *
-EFIAPI
-UncachedAllocateAlignedCopyPool (
-  IN UINTN       AllocationSize,
-  IN CONST VOID  *Buffer,
-  IN UINTN       Alignment
-  )
-{
-  return UncachedInternalAllocateAlignedCopyPool (EfiBootServicesData, AllocationSize, Buffer, Alignment);
-}
-
-VOID *
-EFIAPI
-UncachedAllocateAlignedRuntimeCopyPool (
-  IN UINTN       AllocationSize,
-  IN CONST VOID  *Buffer,
-  IN UINTN       Alignment
-  )
-{
-  return UncachedInternalAllocateAlignedCopyPool (EfiRuntimeServicesData, AllocationSize, Buffer, Alignment);
-}
-
-VOID *
-EFIAPI
-UncachedAllocateAlignedReservedCopyPool (
-  IN UINTN       AllocationSize,
-  IN CONST VOID  *Buffer,
-  IN UINTN       Alignment
-  )
-{
-  return UncachedInternalAllocateAlignedCopyPool (EfiReservedMemoryType, AllocationSize, Buffer, Alignment);
-}
-
-VOID
-EFIAPI
-UncachedFreeAlignedPool (
-  IN VOID   *Buffer
-  )
-{
-  VOID    *Allocation;
-  UINTN   Pages;
-
-  RemovePagesFromList (Buffer, &Allocation, &Pages);
-
-  UncachedFreePages (Allocation, Pages);
-}
-
-VOID *
-UncachedInternalAllocatePool (
-  IN EFI_MEMORY_TYPE  MemoryType,
-  IN UINTN            AllocationSize
-  )
-{
-  UINTN CacheLineLength = ArmDataCacheLineLength ();
-  return UncachedInternalAllocateAlignedPool (MemoryType, AllocationSize, CacheLineLength);
-}
-
-VOID *
-EFIAPI
-UncachedAllocatePool (
-  IN UINTN  AllocationSize
-  )
-{
-  return UncachedInternalAllocatePool (EfiBootServicesData, AllocationSize);
-}
-
-VOID *
-EFIAPI
-UncachedAllocateRuntimePool (
-  IN UINTN  AllocationSize
-  )
-{
-  return UncachedInternalAllocatePool (EfiRuntimeServicesData, AllocationSize);
-}
-
-VOID *
-EFIAPI
-UncachedAllocateReservedPool (
-  IN UINTN  AllocationSize
-  )
-{
-  return UncachedInternalAllocatePool (EfiReservedMemoryType, AllocationSize);
-}
-
-VOID *
-UncachedInternalAllocateZeroPool (
-  IN EFI_MEMORY_TYPE  PoolType,
-  IN UINTN            AllocationSize
-  )
-{
-  VOID  *Memory;
-
-  Memory = UncachedInternalAllocatePool (PoolType, AllocationSize);
-  if (Memory != NULL) {
-    Memory = ZeroMem (Memory, AllocationSize);
-  }
-  return Memory;
-}
-
-VOID *
-EFIAPI
-UncachedAllocateZeroPool (
-  IN UINTN  AllocationSize
-  )
-{
-  return UncachedInternalAllocateZeroPool (EfiBootServicesData, AllocationSize);
-}
-
-VOID *
-EFIAPI
-UncachedAllocateRuntimeZeroPool (
-  IN UINTN  AllocationSize
-  )
-{
-  return UncachedInternalAllocateZeroPool (EfiRuntimeServicesData, AllocationSize);
-}
-
-VOID *
-EFIAPI
-UncachedAllocateReservedZeroPool (
-  IN UINTN  AllocationSize
-  )
-{
-  return UncachedInternalAllocateZeroPool (EfiReservedMemoryType, AllocationSize);
-}
-
-VOID *
-UncachedInternalAllocateCopyPool (
-  IN EFI_MEMORY_TYPE  PoolType,
-  IN UINTN            AllocationSize,
-  IN CONST VOID       *Buffer
-  )
-{
-  VOID  *Memory;
-
-  ASSERT (Buffer != NULL);
-  ASSERT (AllocationSize <= (MAX_ADDRESS - (UINTN) Buffer + 1));
-
-  Memory = UncachedInternalAllocatePool (PoolType, AllocationSize);
-  if (Memory != NULL) {
-     Memory = CopyMem (Memory, Buffer, AllocationSize);
-  }
-  return Memory;
-}
-
-VOID *
-EFIAPI
-UncachedAllocateCopyPool (
-  IN UINTN       AllocationSize,
-  IN CONST VOID  *Buffer
-  )
-{
-  return UncachedInternalAllocateCopyPool (EfiBootServicesData, AllocationSize, Buffer);
-}
-
-VOID *
-EFIAPI
-UncachedAllocateRuntimeCopyPool (
-  IN UINTN       AllocationSize,
-  IN CONST VOID  *Buffer
-  )
-{
-  return UncachedInternalAllocateCopyPool (EfiRuntimeServicesData, AllocationSize, Buffer);
-}
-
-VOID *
-EFIAPI
-UncachedAllocateReservedCopyPool (
-  IN UINTN       AllocationSize,
-  IN CONST VOID  *Buffer
-  )
-{
-  return UncachedInternalAllocateCopyPool (EfiReservedMemoryType, AllocationSize, Buffer);
-}
-
-VOID
-EFIAPI
-UncachedFreePool (
-  IN VOID   *Buffer
-  )
-{
-  UncachedFreeAlignedPool (Buffer);
-}
-
-VOID
-EFIAPI
-UncachedSafeFreePool (
-  IN VOID   *Buffer
-  )
-{
-  if (Buffer != NULL) {
-    UncachedFreePool (Buffer);
-    Buffer = NULL;
-  }
-}
-
-/**
-  The constructor function caches the pointer of DXE Services Table.
-
-  The constructor function caches the pointer of DXE Services Table.
-  It will ASSERT() if that operation fails.
-  It will ASSERT() if the pointer of DXE Services Table is NULL.
-  It will always return EFI_SUCCESS.
-
-  @param  ImageHandle   The firmware allocated handle for the EFI image.
-  @param  SystemTable   A pointer to the EFI System Table.
-
-  @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
-
-**/
-EFI_STATUS
-EFIAPI
-DebugUncachedMemoryAllocationLibConstructor (
-  IN EFI_HANDLE        ImageHandle,
-  IN EFI_SYSTEM_TABLE  *SystemTable
-  )
-{
-  EFI_STATUS    Status;
-
-  Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&gDebugUncachedCpu);
-  ASSERT_EFI_ERROR(Status);
-
-  Status = gBS->LocateProtocol (&gVirtualUncachedPagesProtocolGuid, NULL, (VOID **)&gVirtualUncachedPages);
-  ASSERT_EFI_ERROR(Status);
-
-  return Status;
-}
-
-
-
diff --git a/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf b/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
deleted file mode 100644
index 213188ac2c39..000000000000
--- a/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
+++ /dev/null
@@ -1,47 +0,0 @@
-#/** @file
-#
-#
-# Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
-#
-#  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                      = UncachedMemoryAllocationLib
-  FILE_GUID                      = 3C1EA826-696A-4E8A-B89D-3C5369B84F2A
-  MODULE_TYPE                    = DXE_DRIVER
-  VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = UncachedMemoryAllocationLib
-  CONSTRUCTOR                    = DebugUncachedMemoryAllocationLibConstructor
-
-[Sources.common]
-  DebugUncachedMemoryAllocationLib.c
-
-[Packages]
-  MdePkg/MdePkg.dec
-  ArmPkg/ArmPkg.dec
-
-
-[LibraryClasses]
-  BaseLib
-  MemoryAllocationLib
-  ArmLib
-
-[Protocols]
-  gEfiCpuArchProtocolGuid
-  gVirtualUncachedPagesProtocolGuid
-
-[FixedPcd]
-  gArmTokenSpaceGuid.PcdArmUncachedMemoryMask
-
-
-[Depex]
-  gEfiCpuArchProtocolGuid AND gVirtualUncachedPagesProtocolGuid
diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index c0d5e7c6aa6d..61d4a6642eb7 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -28,10 +28,8 @@ [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
 [LibraryClasses.common]
 !if $(TARGET) == RELEASE
   DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
-  UncachedMemoryAllocationLib|ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
 !else
   DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
-  UncachedMemoryAllocationLib|ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
 !endif
   DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
 
diff --git a/BeagleBoardPkg/BeagleBoardPkg.dsc b/BeagleBoardPkg/BeagleBoardPkg.dsc
index b24db6cbfb04..a71a01ac7723 100644
--- a/BeagleBoardPkg/BeagleBoardPkg.dsc
+++ b/BeagleBoardPkg/BeagleBoardPkg.dsc
@@ -49,7 +49,6 @@ [LibraryClasses.common]
 !else
   DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
   UncachedMemoryAllocationLib|ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
-#  UncachedMemoryAllocationLib|ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
 !endif
   DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
 
diff --git a/Omap35xxPkg/Omap35xxPkg.dsc b/Omap35xxPkg/Omap35xxPkg.dsc
index 9395570ccaa3..ad7d9898c330 100644
--- a/Omap35xxPkg/Omap35xxPkg.dsc
+++ b/Omap35xxPkg/Omap35xxPkg.dsc
@@ -72,8 +72,7 @@ [LibraryClasses.common]
 
   UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
 
- # UncachedMemoryAllocationLib|ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
-  UncachedMemoryAllocationLib|ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
+  UncachedMemoryAllocationLib|ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
 
   CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
 
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] ArmPkg/CpuDxe: remove VirtualUncachedPages protocol and implementation
  2017-02-23 15:48 [PATCH 1/2] ArmPkg: remove DebugUncachedMemoryAllocationLib Ard Biesheuvel
@ 2017-02-23 15:48 ` Ard Biesheuvel
  2017-02-23 17:50   ` Leif Lindholm
  2017-02-23 16:19 ` [PATCH 1/2] ArmPkg: remove DebugUncachedMemoryAllocationLib Laszlo Ersek
  2017-02-23 17:49 ` Leif Lindholm
  2 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2017-02-23 15:48 UTC (permalink / raw)
  To: edk2-devel, leif.lindholm, lersek; +Cc: ryan.harkin, Ard Biesheuvel

Virtual uncached pages are simply pages that are aliased using mismatched
attributes, which is not allowed by the ARM architecture. So remove the
protocol and its implementation.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/ArmPkg.dec                              |  3 -
 ArmPkg/Drivers/CpuDxe/CpuDxe.c                 |  1 -
 ArmPkg/Drivers/CpuDxe/CpuDxe.h                 |  3 -
 ArmPkg/Drivers/CpuDxe/CpuDxe.inf               |  1 -
 ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c           | 70 --------------------
 ArmPkg/Include/Protocol/VirtualUncachedPages.h | 60 -----------------
 6 files changed, 138 deletions(-)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index 8e9cf199becc..4fd7a5be5158 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -52,9 +52,6 @@ [Ppis]
   ## Include/Ppi/ArmMpCoreInfo.h
   gArmMpCoreInfoPpiGuid = { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d, 0xab, 0x44, 0xe7, 0x54, 0xa8, 0xfc} }
 
-[Protocols.common]
-  gVirtualUncachedPagesProtocolGuid = { 0xAD651C7D, 0x3C22, 0x4DBF, { 0x92, 0xe8, 0x38, 0xa7, 0xcd, 0xae, 0x87, 0xb2 } }
-
 [PcdsFeatureFlag.common]
   gArmTokenSpaceGuid.PcdCpuDxeProduceDebugSupport|FALSE|BOOLEAN|0x00000001
 
diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.c b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
index 7d328d096b1e..5aa5b874144a 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.c
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
@@ -253,7 +253,6 @@ CpuDxeInitialize (
   Status = gBS->InstallMultipleProtocolInterfaces (
                 &mCpuHandle,
                 &gEfiCpuArchProtocolGuid,           &mCpu,
-                &gVirtualUncachedPagesProtocolGuid, &gVirtualUncachedPages,
                 NULL
                 );
 
diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.h b/ArmPkg/Drivers/CpuDxe/CpuDxe.h
index 80c305d53dd1..a00fc3064362 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.h
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.h
@@ -35,7 +35,6 @@
 #include <Protocol/Cpu.h>
 #include <Protocol/DebugSupport.h>
 #include <Protocol/DebugSupportPeriodicCallback.h>
-#include <Protocol/VirtualUncachedPages.h>
 #include <Protocol/LoadedImage.h>
 
 
@@ -169,6 +168,4 @@ SetGcdMemorySpaceAttributes (
   IN UINT64                              Attributes
   );
 
-extern VIRTUAL_UNCACHED_PAGES_PROTOCOL  gVirtualUncachedPages;
-
 #endif // __CPU_DXE_ARM_EXCEPTION_H__
diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
index b31c994f43e2..d068e06803ed 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
@@ -61,7 +61,6 @@ [LibraryClasses]
 [Protocols]
   gEfiCpuArchProtocolGuid
   gEfiDebugSupportPeriodicCallbackProtocolGuid
-  gVirtualUncachedPagesProtocolGuid
 
 [Guids]
   gEfiDebugImageInfoTableGuid
diff --git a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
index 54d9b0163331..ebe593d1c325 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
+++ b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
@@ -211,73 +211,3 @@ CpuSetMemoryAttributes (
     return EFI_SUCCESS;
   }
 }
-
-EFI_STATUS
-EFIAPI
-CpuConvertPagesToUncachedVirtualAddress (
-  IN  VIRTUAL_UNCACHED_PAGES_PROTOCOL  *This,
-  IN  EFI_PHYSICAL_ADDRESS              Address,
-  IN  UINTN                             Length,
-  IN  EFI_PHYSICAL_ADDRESS              VirtualMask,
-  OUT UINT64                           *Attributes     OPTIONAL
-  )
-{
-  EFI_STATUS                      Status;
-  EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor;
-
-  if (Attributes != NULL) {
-    Status = gDS->GetMemorySpaceDescriptor (Address, &GcdDescriptor);
-    if (!EFI_ERROR (Status)) {
-      *Attributes = GcdDescriptor.Attributes;
-    }
-  }
-
-  //
-  // Make this address range page fault if accessed. If it is a DMA buffer than this would
-  // be the PCI address. Code should always use the CPU address, and we will or in VirtualMask
-  // to that address.
-  //
-  Status = SetMemoryAttributes (Address, Length, EFI_MEMORY_RO, 0);
-  if (!EFI_ERROR (Status)) {
-    Status = SetMemoryAttributes (Address | VirtualMask, Length, EFI_MEMORY_UC, VirtualMask);
-  }
-
-  DEBUG ((DEBUG_INFO | DEBUG_LOAD, "CpuConvertPagesToUncachedVirtualAddress()\n    Unmapped 0x%08lx Mapped 0x%08lx 0x%x bytes\n", Address, Address | VirtualMask, Length));
-
-  return Status;
-}
-
-
-EFI_STATUS
-EFIAPI
-CpuReconvertPages (
-  IN  VIRTUAL_UNCACHED_PAGES_PROTOCOL  *This,
-  IN  EFI_PHYSICAL_ADDRESS              Address,
-  IN  UINTN                             Length,
-  IN  EFI_PHYSICAL_ADDRESS              VirtualMask,
-  IN  UINT64                            Attributes
-  )
-{
-  EFI_STATUS      Status;
-
-  DEBUG ((DEBUG_INFO | DEBUG_LOAD, "CpuReconvertPages(%lx, %x, %lx, %lx)\n", Address, Length, VirtualMask, Attributes));
-
-  //
-  // Unmap the aliased Address
-  //
-  Status = SetMemoryAttributes (Address | VirtualMask, Length, EFI_MEMORY_RO, 0);
-  if (!EFI_ERROR (Status)) {
-    //
-    // Restore atttributes
-    //
-    Status = SetMemoryAttributes (Address, Length, Attributes, 0);
-  }
-
-  return Status;
-}
-
-
-VIRTUAL_UNCACHED_PAGES_PROTOCOL  gVirtualUncachedPages = {
-  CpuConvertPagesToUncachedVirtualAddress,
-  CpuReconvertPages
-};
diff --git a/ArmPkg/Include/Protocol/VirtualUncachedPages.h b/ArmPkg/Include/Protocol/VirtualUncachedPages.h
deleted file mode 100644
index 0822184b8931..000000000000
--- a/ArmPkg/Include/Protocol/VirtualUncachedPages.h
+++ /dev/null
@@ -1,60 +0,0 @@
-/** @file
-
-  Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
-
-  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.
-
-**/
-
-#ifndef __VIRTUAL_UNCACHED_PAGES_ROTOCOL_H__
-#define __VIRTUAL_UNCACHED_PAGES_ROTOCOL_H__
-
-//
-// Protocol GUID
-//
-#define VIRTUAL_UNCACHED_PAGES_PROTOCOL_GUID { 0xAD651C7D, 0x3C22, 0x4DBF, { 0x92, 0xe8, 0x38, 0xa7, 0xcd, 0xae, 0x87, 0xb2 } }
-
-
-
-//
-// Protocol interface structure
-//
-typedef struct _VIRTUAL_UNCACHED_PAGES_PROTOCOL  VIRTUAL_UNCACHED_PAGES_PROTOCOL;
-
-
-typedef
-EFI_STATUS
-(EFIAPI *CONVERT_PAGES_TO_UNCACHED_VIRTUAL_ADDRESS) (
-  IN  VIRTUAL_UNCACHED_PAGES_PROTOCOL   *This,
-  IN  EFI_PHYSICAL_ADDRESS              Address,
-  IN  UINTN                             Length,
-  IN  EFI_PHYSICAL_ADDRESS              VirtualMask,
-  OUT UINT64                            *Attributes     OPTIONAL
-  );
-
-typedef
-EFI_STATUS
-(EFIAPI *FREE_CONVERTED_PAGES) (
-  IN  VIRTUAL_UNCACHED_PAGES_PROTOCOL   *This,
-  IN  EFI_PHYSICAL_ADDRESS              Address,
-  IN  UINTN                             Length,
-  IN  EFI_PHYSICAL_ADDRESS              VirtualMask,
-  IN  UINT64                            Attributes
-  );
-
-
-
-struct _VIRTUAL_UNCACHED_PAGES_PROTOCOL {
-  CONVERT_PAGES_TO_UNCACHED_VIRTUAL_ADDRESS  ConvertPages;
-  FREE_CONVERTED_PAGES                       RevertPages;
-};
-
-extern EFI_GUID gVirtualUncachedPagesProtocolGuid;
-
-#endif
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] ArmPkg: remove DebugUncachedMemoryAllocationLib
  2017-02-23 15:48 [PATCH 1/2] ArmPkg: remove DebugUncachedMemoryAllocationLib Ard Biesheuvel
  2017-02-23 15:48 ` [PATCH 2/2] ArmPkg/CpuDxe: remove VirtualUncachedPages protocol and implementation Ard Biesheuvel
@ 2017-02-23 16:19 ` Laszlo Ersek
  2017-02-23 17:49 ` Leif Lindholm
  2 siblings, 0 replies; 6+ messages in thread
From: Laszlo Ersek @ 2017-02-23 16:19 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel, leif.lindholm; +Cc: ryan.harkin

On 02/23/17 16:48, Ard Biesheuvel wrote:
> The debug implementation of the UncachedMemoryAllocationLib library
> class relies on the creation of an uncached alias of a memory range,
> while keeping the original cached mapping, but with read-only attributes
> to trap inadvertent write accesses.
> 
> This is not a terribly good idea, given that the ARM architecture does
> not allow mismatched attributes, and so creating them deliberately is
> not something we should encourage by doing it in reference code.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/ArmPkg.dsc                                                                    |   1 -
>  ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.c   | 656 --------------------
>  ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf |  47 --
>  ArmVirtPkg/ArmVirt.dsc.inc                                                           |   2 -
>  BeagleBoardPkg/BeagleBoardPkg.dsc                                                    |   1 -
>  Omap35xxPkg/Omap35xxPkg.dsc                                                          |   3 +-
>  6 files changed, 1 insertion(+), 709 deletions(-)

Acked-by: Laszlo Ersek <lersek@redhat.com>




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] ArmPkg: remove DebugUncachedMemoryAllocationLib
  2017-02-23 15:48 [PATCH 1/2] ArmPkg: remove DebugUncachedMemoryAllocationLib Ard Biesheuvel
  2017-02-23 15:48 ` [PATCH 2/2] ArmPkg/CpuDxe: remove VirtualUncachedPages protocol and implementation Ard Biesheuvel
  2017-02-23 16:19 ` [PATCH 1/2] ArmPkg: remove DebugUncachedMemoryAllocationLib Laszlo Ersek
@ 2017-02-23 17:49 ` Leif Lindholm
  2017-02-23 17:58   ` Ard Biesheuvel
  2 siblings, 1 reply; 6+ messages in thread
From: Leif Lindholm @ 2017-02-23 17:49 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, lersek, ryan.harkin

On Thu, Feb 23, 2017 at 03:48:04PM +0000, Ard Biesheuvel wrote:
> The debug implementation of the UncachedMemoryAllocationLib library
> class relies on the creation of an uncached alias of a memory range,
> while keeping the original cached mapping, but with read-only attributes
> to trap inadvertent write accesses.
> 
> This is not a terribly good idea, given that the ARM architecture does
> not allow mismatched attributes, and so creating them deliberately is
> not something we should encourage by doing it in reference code.

Agreed.
One comment near the end:

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/ArmPkg.dsc                                                                    |   1 -
>  ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.c   | 656 --------------------
>  ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf |  47 --
>  ArmVirtPkg/ArmVirt.dsc.inc                                                           |   2 -
>  BeagleBoardPkg/BeagleBoardPkg.dsc                                                    |   1 -
>  Omap35xxPkg/Omap35xxPkg.dsc                                                          |   3 +-
>  6 files changed, 1 insertion(+), 709 deletions(-)
> 
> diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
> index 0db33eb865b1..1a490d23f7b5 100644
> --- a/ArmPkg/ArmPkg.dsc
> +++ b/ArmPkg/ArmPkg.dsc
> @@ -112,7 +112,6 @@ [Components.common]
>    ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
>    ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf
>    ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
> -  ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
>    ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf
>    ArmPkg/Library/RvdPeCoffExtraActionLib/RvdPeCoffExtraActionLib.inf
>    ArmPkg/Library/SemiHostingDebugLib/SemiHostingDebugLib.inf
> diff --git a/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.c b/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.c
> deleted file mode 100644
> index 00e01a905c85..000000000000
> --- a/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.c
> +++ /dev/null
> @@ -1,656 +0,0 @@
> -/** @file
> -  Debug version of the UncachedMemoryAllocation lib that uses the VirtualUncachedPages
> -  protocol, produced by the DXE CPU driver, to produce debuggable uncached memory buffers.
> -
> -  The DMA rules for EFI contain the concept of a PCI (DMA master) address for memory and
> -  a CPU (C code) address for the memory buffer that don't have to be the same.  There seem to
> -  be common errors out there with folks mixing up the two addresses.  This library causes
> -  the PCI (DMA master) address to not be mapped into system memory so if the CPU (C code)
> -  uses the wrong pointer it will generate a page fault. The CPU (C code) version of the buffer
> -  has a virtual address that does not match the physical address. The virtual address has
> -  PcdArmUncachedMemoryMask ored into the physical address.
> -
> -  Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
> -
> -  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/BaseLib.h>
> -#include <Library/BaseMemoryLib.h>
> -#include <Library/MemoryAllocationLib.h>
> -#include <Library/DebugLib.h>
> -#include <Library/UefiBootServicesTableLib.h>
> -#include <Library/UncachedMemoryAllocationLib.h>
> -#include <Library/PcdLib.h>
> -#include <Library/ArmLib.h>
> -
> -#include <Protocol/Cpu.h>
> -#include <Protocol/VirtualUncachedPages.h>
> -
> -VOID *
> -UncachedInternalAllocatePages (
> -  IN EFI_MEMORY_TYPE  MemoryType,
> -  IN UINTN            Pages
> -  );
> -
> -VOID *
> -UncachedInternalAllocateAlignedPages (
> -  IN EFI_MEMORY_TYPE  MemoryType,
> -  IN UINTN            Pages,
> -  IN UINTN            Alignment
> -  );
> -
> -
> -
> -EFI_CPU_ARCH_PROTOCOL           *gDebugUncachedCpu;
> -VIRTUAL_UNCACHED_PAGES_PROTOCOL *gVirtualUncachedPages;
> -
> -//
> -// Assume all of memory has the same cache attributes, unless we do our magic
> -//
> -UINT64  gAttributes;
> -
> -typedef struct {
> -  VOID        *Buffer;
> -  VOID        *Allocation;
> -  UINTN       Pages;
> -  LIST_ENTRY  Link;
> -} FREE_PAGE_NODE;
> -
> -LIST_ENTRY  mPageList = INITIALIZE_LIST_HEAD_VARIABLE (mPageList);
> -
> -VOID
> -AddPagesToList (
> -  IN VOID   *Buffer,
> -  IN VOID   *Allocation,
> -  UINTN     Pages
> -  )
> -{
> -  FREE_PAGE_NODE  *NewNode;
> -
> -  NewNode = AllocatePool (sizeof (LIST_ENTRY));
> -  if (NewNode == NULL) {
> -    ASSERT (FALSE);
> -    return;
> -  }
> -
> -  NewNode->Buffer     = Buffer;
> -  NewNode->Allocation = Allocation;
> -  NewNode->Pages      = Pages;
> -
> -  InsertTailList (&mPageList, &NewNode->Link);
> -}
> -
> -
> -VOID
> -RemovePagesFromList (
> -  IN VOID   *Buffer,
> -  OUT VOID  **Allocation,
> -  OUT UINTN *Pages
> -  )
> -{
> -  LIST_ENTRY      *Link;
> -  FREE_PAGE_NODE  *OldNode;
> -
> -  *Allocation = NULL;
> -  *Pages = 0;
> -
> -  for (Link = mPageList.ForwardLink; Link != &mPageList; Link = Link->ForwardLink) {
> -    OldNode = BASE_CR (Link, FREE_PAGE_NODE, Link);
> -    if (OldNode->Buffer == Buffer) {
> -      *Allocation = OldNode->Allocation;
> -      *Pages = OldNode->Pages;
> -
> -      RemoveEntryList (&OldNode->Link);
> -      FreePool (OldNode);
> -      return;
> -    }
> -  }
> -
> -  return;
> -}
> -
> -
> -
> -EFI_PHYSICAL_ADDRESS
> -ConvertToPhysicalAddress (
> -  IN VOID *VirtualAddress
> -  )
> -{
> -  UINTN UncachedMemoryMask = (UINTN)PcdGet64 (PcdArmUncachedMemoryMask);
> -  UINTN PhysicalAddress;
> -
> -  PhysicalAddress = (UINTN)VirtualAddress & ~UncachedMemoryMask;
> -
> -  return (EFI_PHYSICAL_ADDRESS)PhysicalAddress;
> -}
> -
> -
> -VOID *
> -ConvertToUncachedAddress (
> -  IN VOID *Address
> -  )
> -{
> -  UINTN UncachedMemoryMask = (UINTN)PcdGet64 (PcdArmUncachedMemoryMask);
> -  UINTN UncachedAddress;
> -
> -  UncachedAddress = (UINTN)Address | UncachedMemoryMask;
> -
> -  return (VOID *)UncachedAddress;
> -}
> -
> -
> -
> -VOID *
> -UncachedInternalAllocatePages (
> -  IN EFI_MEMORY_TYPE  MemoryType,
> -  IN UINTN            Pages
> -  )
> -{
> -  return UncachedInternalAllocateAlignedPages (MemoryType, Pages, EFI_PAGE_SIZE);
> -}
> -
> -
> -VOID *
> -EFIAPI
> -UncachedAllocatePages (
> -  IN UINTN  Pages
> -  )
> -{
> -  return UncachedInternalAllocatePages (EfiBootServicesData, Pages);
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateRuntimePages (
> -  IN UINTN  Pages
> -  )
> -{
> -  return UncachedInternalAllocatePages (EfiRuntimeServicesData, Pages);
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateReservedPages (
> -  IN UINTN  Pages
> -  )
> -{
> -  return UncachedInternalAllocatePages (EfiReservedMemoryType, Pages);
> -}
> -
> -
> -
> -VOID
> -EFIAPI
> -UncachedFreePages (
> -  IN VOID   *Buffer,
> -  IN UINTN  Pages
> -  )
> -{
> -  UncachedFreeAlignedPages (Buffer, Pages);
> -  return;
> -}
> -
> -
> -VOID *
> -UncachedInternalAllocateAlignedPages (
> -  IN EFI_MEMORY_TYPE  MemoryType,
> -  IN UINTN            Pages,
> -  IN UINTN            Alignment
> -  )
> -{
> -  EFI_STATUS            Status;
> -  EFI_PHYSICAL_ADDRESS  Memory;
> -  EFI_PHYSICAL_ADDRESS  AlignedMemory;
> -  UINTN                 AlignmentMask;
> -  UINTN                 UnalignedPages;
> -  UINTN                 RealPages;
> -
> -  //
> -  // Alignment must be a power of two or zero.
> -  //
> -  ASSERT ((Alignment & (Alignment - 1)) == 0);
> -
> -  if (Pages == 0) {
> -    return NULL;
> -  }
> -  if (Alignment > EFI_PAGE_SIZE) {
> -    //
> -    // Caculate the total number of pages since alignment is larger than page size.
> -    //
> -    AlignmentMask  = Alignment - 1;
> -    RealPages      = Pages + EFI_SIZE_TO_PAGES (Alignment);
> -    //
> -    // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not overflow.
> -    //
> -    ASSERT (RealPages > Pages);
> -
> -    Status         = gBS->AllocatePages (AllocateAnyPages, MemoryType, RealPages, &Memory);
> -    if (EFI_ERROR (Status)) {
> -      return NULL;
> -    }
> -    AlignedMemory  = ((UINTN) Memory + AlignmentMask) & ~AlignmentMask;
> -    UnalignedPages = EFI_SIZE_TO_PAGES (AlignedMemory - (UINTN) Memory);
> -    if (UnalignedPages > 0) {
> -      //
> -      // Free first unaligned page(s).
> -      //
> -      Status = gBS->FreePages (Memory, UnalignedPages);
> -      ASSERT_EFI_ERROR (Status);
> -    }
> -    Memory         = (EFI_PHYSICAL_ADDRESS) (AlignedMemory + EFI_PAGES_TO_SIZE (Pages));
> -    UnalignedPages = RealPages - Pages - UnalignedPages;
> -    if (UnalignedPages > 0) {
> -      //
> -      // Free last unaligned page(s).
> -      //
> -      Status = gBS->FreePages (Memory, UnalignedPages);
> -      ASSERT_EFI_ERROR (Status);
> -    }
> -  } else {
> -    //
> -    // Do not over-allocate pages in this case.
> -    //
> -    Status = gBS->AllocatePages (AllocateAnyPages, MemoryType, Pages, &Memory);
> -    if (EFI_ERROR (Status)) {
> -      return NULL;
> -    }
> -    AlignedMemory  = (UINTN) Memory;
> -  }
> -
> -  Status = gVirtualUncachedPages->ConvertPages (gVirtualUncachedPages, AlignedMemory, Pages * EFI_PAGE_SIZE, PcdGet64 (PcdArmUncachedMemoryMask), &gAttributes);
> -  if (EFI_ERROR (Status)) {
> -    return NULL;
> -  }
> -
> -  AlignedMemory = (EFI_PHYSICAL_ADDRESS)(UINTN)ConvertToUncachedAddress ((VOID *)(UINTN)AlignedMemory);
> -
> -  return (VOID *)(UINTN)AlignedMemory;
> -}
> -
> -
> -VOID
> -EFIAPI
> -UncachedFreeAlignedPages (
> -  IN VOID   *Buffer,
> -  IN UINTN  Pages
> -  )
> -{
> -  EFI_STATUS            Status;
> -  EFI_PHYSICAL_ADDRESS  Memory;
> -
> -  ASSERT (Pages != 0);
> -
> -  Memory = ConvertToPhysicalAddress (Buffer);
> -
> -  Status = gVirtualUncachedPages->RevertPages (gVirtualUncachedPages, Memory, Pages * EFI_PAGE_SIZE, PcdGet64 (PcdArmUncachedMemoryMask), gAttributes);
> -
> -
> -  Status = gBS->FreePages ((EFI_PHYSICAL_ADDRESS) (UINTN) Memory, Pages);
> -  ASSERT_EFI_ERROR (Status);
> -}
> -
> -
> -
> -
> -VOID *
> -UncachedInternalAllocateAlignedPool (
> -  IN EFI_MEMORY_TYPE  PoolType,
> -  IN UINTN            AllocationSize,
> -  IN UINTN            Alignment
> -  )
> -{
> -  VOID      *AlignedAddress;
> -
> -  //
> -  // Alignment must be a power of two or zero.
> -  //
> -  ASSERT ((Alignment & (Alignment - 1)) == 0);
> -
> -  if (Alignment < EFI_PAGE_SIZE) {
> -    Alignment = EFI_PAGE_SIZE;
> -  }
> -
> -  AlignedAddress = UncachedInternalAllocateAlignedPages (PoolType, EFI_SIZE_TO_PAGES (AllocationSize), Alignment);
> -  if (AlignedAddress == NULL) {
> -    return NULL;
> -  }
> -
> -  AddPagesToList ((VOID *)(UINTN)ConvertToPhysicalAddress (AlignedAddress), (VOID *)(UINTN)AlignedAddress, EFI_SIZE_TO_PAGES (AllocationSize));
> -
> -  return (VOID *) AlignedAddress;
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateAlignedPool (
> -  IN UINTN  AllocationSize,
> -  IN UINTN  Alignment
> -  )
> -{
> -  return UncachedInternalAllocateAlignedPool (EfiBootServicesData, AllocationSize, Alignment);
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateAlignedRuntimePool (
> -  IN UINTN  AllocationSize,
> -  IN UINTN  Alignment
> -  )
> -{
> -  return UncachedInternalAllocateAlignedPool (EfiRuntimeServicesData, AllocationSize, Alignment);
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateAlignedReservedPool (
> -  IN UINTN  AllocationSize,
> -  IN UINTN  Alignment
> -  )
> -{
> -  return UncachedInternalAllocateAlignedPool (EfiReservedMemoryType, AllocationSize, Alignment);
> -}
> -
> -VOID *
> -UncachedInternalAllocateAlignedZeroPool (
> -  IN EFI_MEMORY_TYPE  PoolType,
> -  IN UINTN            AllocationSize,
> -  IN UINTN            Alignment
> -  )
> -{
> -  VOID    *Memory;
> -  Memory = UncachedInternalAllocateAlignedPool (PoolType, AllocationSize, Alignment);
> -  if (Memory != NULL) {
> -    Memory = ZeroMem (Memory, AllocationSize);
> -  }
> -  return Memory;
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateAlignedZeroPool (
> -  IN UINTN  AllocationSize,
> -  IN UINTN  Alignment
> -  )
> -{
> -  return UncachedInternalAllocateAlignedZeroPool (EfiBootServicesData, AllocationSize, Alignment);
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateAlignedRuntimeZeroPool (
> -  IN UINTN  AllocationSize,
> -  IN UINTN  Alignment
> -  )
> -{
> -  return UncachedInternalAllocateAlignedZeroPool (EfiRuntimeServicesData, AllocationSize, Alignment);
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateAlignedReservedZeroPool (
> -  IN UINTN  AllocationSize,
> -  IN UINTN  Alignment
> -  )
> -{
> -  return UncachedInternalAllocateAlignedZeroPool (EfiReservedMemoryType, AllocationSize, Alignment);
> -}
> -
> -VOID *
> -UncachedInternalAllocateAlignedCopyPool (
> -  IN EFI_MEMORY_TYPE  PoolType,
> -  IN UINTN            AllocationSize,
> -  IN CONST VOID       *Buffer,
> -  IN UINTN            Alignment
> -  )
> -{
> -  VOID  *Memory;
> -
> -  ASSERT (Buffer != NULL);
> -  ASSERT (AllocationSize <= (MAX_ADDRESS - (UINTN) Buffer + 1));
> -
> -  Memory = UncachedInternalAllocateAlignedPool (PoolType, AllocationSize, Alignment);
> -  if (Memory != NULL) {
> -    Memory = CopyMem (Memory, Buffer, AllocationSize);
> -  }
> -  return Memory;
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateAlignedCopyPool (
> -  IN UINTN       AllocationSize,
> -  IN CONST VOID  *Buffer,
> -  IN UINTN       Alignment
> -  )
> -{
> -  return UncachedInternalAllocateAlignedCopyPool (EfiBootServicesData, AllocationSize, Buffer, Alignment);
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateAlignedRuntimeCopyPool (
> -  IN UINTN       AllocationSize,
> -  IN CONST VOID  *Buffer,
> -  IN UINTN       Alignment
> -  )
> -{
> -  return UncachedInternalAllocateAlignedCopyPool (EfiRuntimeServicesData, AllocationSize, Buffer, Alignment);
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateAlignedReservedCopyPool (
> -  IN UINTN       AllocationSize,
> -  IN CONST VOID  *Buffer,
> -  IN UINTN       Alignment
> -  )
> -{
> -  return UncachedInternalAllocateAlignedCopyPool (EfiReservedMemoryType, AllocationSize, Buffer, Alignment);
> -}
> -
> -VOID
> -EFIAPI
> -UncachedFreeAlignedPool (
> -  IN VOID   *Buffer
> -  )
> -{
> -  VOID    *Allocation;
> -  UINTN   Pages;
> -
> -  RemovePagesFromList (Buffer, &Allocation, &Pages);
> -
> -  UncachedFreePages (Allocation, Pages);
> -}
> -
> -VOID *
> -UncachedInternalAllocatePool (
> -  IN EFI_MEMORY_TYPE  MemoryType,
> -  IN UINTN            AllocationSize
> -  )
> -{
> -  UINTN CacheLineLength = ArmDataCacheLineLength ();
> -  return UncachedInternalAllocateAlignedPool (MemoryType, AllocationSize, CacheLineLength);
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocatePool (
> -  IN UINTN  AllocationSize
> -  )
> -{
> -  return UncachedInternalAllocatePool (EfiBootServicesData, AllocationSize);
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateRuntimePool (
> -  IN UINTN  AllocationSize
> -  )
> -{
> -  return UncachedInternalAllocatePool (EfiRuntimeServicesData, AllocationSize);
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateReservedPool (
> -  IN UINTN  AllocationSize
> -  )
> -{
> -  return UncachedInternalAllocatePool (EfiReservedMemoryType, AllocationSize);
> -}
> -
> -VOID *
> -UncachedInternalAllocateZeroPool (
> -  IN EFI_MEMORY_TYPE  PoolType,
> -  IN UINTN            AllocationSize
> -  )
> -{
> -  VOID  *Memory;
> -
> -  Memory = UncachedInternalAllocatePool (PoolType, AllocationSize);
> -  if (Memory != NULL) {
> -    Memory = ZeroMem (Memory, AllocationSize);
> -  }
> -  return Memory;
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateZeroPool (
> -  IN UINTN  AllocationSize
> -  )
> -{
> -  return UncachedInternalAllocateZeroPool (EfiBootServicesData, AllocationSize);
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateRuntimeZeroPool (
> -  IN UINTN  AllocationSize
> -  )
> -{
> -  return UncachedInternalAllocateZeroPool (EfiRuntimeServicesData, AllocationSize);
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateReservedZeroPool (
> -  IN UINTN  AllocationSize
> -  )
> -{
> -  return UncachedInternalAllocateZeroPool (EfiReservedMemoryType, AllocationSize);
> -}
> -
> -VOID *
> -UncachedInternalAllocateCopyPool (
> -  IN EFI_MEMORY_TYPE  PoolType,
> -  IN UINTN            AllocationSize,
> -  IN CONST VOID       *Buffer
> -  )
> -{
> -  VOID  *Memory;
> -
> -  ASSERT (Buffer != NULL);
> -  ASSERT (AllocationSize <= (MAX_ADDRESS - (UINTN) Buffer + 1));
> -
> -  Memory = UncachedInternalAllocatePool (PoolType, AllocationSize);
> -  if (Memory != NULL) {
> -     Memory = CopyMem (Memory, Buffer, AllocationSize);
> -  }
> -  return Memory;
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateCopyPool (
> -  IN UINTN       AllocationSize,
> -  IN CONST VOID  *Buffer
> -  )
> -{
> -  return UncachedInternalAllocateCopyPool (EfiBootServicesData, AllocationSize, Buffer);
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateRuntimeCopyPool (
> -  IN UINTN       AllocationSize,
> -  IN CONST VOID  *Buffer
> -  )
> -{
> -  return UncachedInternalAllocateCopyPool (EfiRuntimeServicesData, AllocationSize, Buffer);
> -}
> -
> -VOID *
> -EFIAPI
> -UncachedAllocateReservedCopyPool (
> -  IN UINTN       AllocationSize,
> -  IN CONST VOID  *Buffer
> -  )
> -{
> -  return UncachedInternalAllocateCopyPool (EfiReservedMemoryType, AllocationSize, Buffer);
> -}
> -
> -VOID
> -EFIAPI
> -UncachedFreePool (
> -  IN VOID   *Buffer
> -  )
> -{
> -  UncachedFreeAlignedPool (Buffer);
> -}
> -
> -VOID
> -EFIAPI
> -UncachedSafeFreePool (
> -  IN VOID   *Buffer
> -  )
> -{
> -  if (Buffer != NULL) {
> -    UncachedFreePool (Buffer);
> -    Buffer = NULL;
> -  }
> -}
> -
> -/**
> -  The constructor function caches the pointer of DXE Services Table.
> -
> -  The constructor function caches the pointer of DXE Services Table.
> -  It will ASSERT() if that operation fails.
> -  It will ASSERT() if the pointer of DXE Services Table is NULL.
> -  It will always return EFI_SUCCESS.
> -
> -  @param  ImageHandle   The firmware allocated handle for the EFI image.
> -  @param  SystemTable   A pointer to the EFI System Table.
> -
> -  @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -DebugUncachedMemoryAllocationLibConstructor (
> -  IN EFI_HANDLE        ImageHandle,
> -  IN EFI_SYSTEM_TABLE  *SystemTable
> -  )
> -{
> -  EFI_STATUS    Status;
> -
> -  Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&gDebugUncachedCpu);
> -  ASSERT_EFI_ERROR(Status);
> -
> -  Status = gBS->LocateProtocol (&gVirtualUncachedPagesProtocolGuid, NULL, (VOID **)&gVirtualUncachedPages);
> -  ASSERT_EFI_ERROR(Status);
> -
> -  return Status;
> -}
> -
> -
> -
> diff --git a/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf b/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
> deleted file mode 100644
> index 213188ac2c39..000000000000
> --- a/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
> +++ /dev/null
> @@ -1,47 +0,0 @@
> -#/** @file
> -#
> -#
> -# Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
> -#
> -#  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                      = UncachedMemoryAllocationLib
> -  FILE_GUID                      = 3C1EA826-696A-4E8A-B89D-3C5369B84F2A
> -  MODULE_TYPE                    = DXE_DRIVER
> -  VERSION_STRING                 = 1.0
> -  LIBRARY_CLASS                  = UncachedMemoryAllocationLib
> -  CONSTRUCTOR                    = DebugUncachedMemoryAllocationLibConstructor
> -
> -[Sources.common]
> -  DebugUncachedMemoryAllocationLib.c
> -
> -[Packages]
> -  MdePkg/MdePkg.dec
> -  ArmPkg/ArmPkg.dec
> -
> -
> -[LibraryClasses]
> -  BaseLib
> -  MemoryAllocationLib
> -  ArmLib
> -
> -[Protocols]
> -  gEfiCpuArchProtocolGuid
> -  gVirtualUncachedPagesProtocolGuid
> -
> -[FixedPcd]
> -  gArmTokenSpaceGuid.PcdArmUncachedMemoryMask
> -
> -
> -[Depex]
> -  gEfiCpuArchProtocolGuid AND gVirtualUncachedPagesProtocolGuid
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index c0d5e7c6aa6d..61d4a6642eb7 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -28,10 +28,8 @@ [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>  [LibraryClasses.common]
>  !if $(TARGET) == RELEASE
>    DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
> -  UncachedMemoryAllocationLib|ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf

This deletion is of a (not needed) non-Debug version (which is not
described in the commit message). Could you add a note to that effect
in there?

With that:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

>  !else
>    DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> -  UncachedMemoryAllocationLib|ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
>  !endif
>    DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
>  
> diff --git a/BeagleBoardPkg/BeagleBoardPkg.dsc b/BeagleBoardPkg/BeagleBoardPkg.dsc
> index b24db6cbfb04..a71a01ac7723 100644
> --- a/BeagleBoardPkg/BeagleBoardPkg.dsc
> +++ b/BeagleBoardPkg/BeagleBoardPkg.dsc
> @@ -49,7 +49,6 @@ [LibraryClasses.common]
>  !else
>    DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>    UncachedMemoryAllocationLib|ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
> -#  UncachedMemoryAllocationLib|ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
>  !endif
>    DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
>  
> diff --git a/Omap35xxPkg/Omap35xxPkg.dsc b/Omap35xxPkg/Omap35xxPkg.dsc
> index 9395570ccaa3..ad7d9898c330 100644
> --- a/Omap35xxPkg/Omap35xxPkg.dsc
> +++ b/Omap35xxPkg/Omap35xxPkg.dsc
> @@ -72,8 +72,7 @@ [LibraryClasses.common]
>  
>    UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
>  
> - # UncachedMemoryAllocationLib|ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
> -  UncachedMemoryAllocationLib|ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
> +  UncachedMemoryAllocationLib|ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
>  
>    CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
>  
> -- 
> 2.7.4
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] ArmPkg/CpuDxe: remove VirtualUncachedPages protocol and implementation
  2017-02-23 15:48 ` [PATCH 2/2] ArmPkg/CpuDxe: remove VirtualUncachedPages protocol and implementation Ard Biesheuvel
@ 2017-02-23 17:50   ` Leif Lindholm
  0 siblings, 0 replies; 6+ messages in thread
From: Leif Lindholm @ 2017-02-23 17:50 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, lersek, ryan.harkin

On Thu, Feb 23, 2017 at 03:48:05PM +0000, Ard Biesheuvel wrote:
> Virtual uncached pages are simply pages that are aliased using mismatched
> attributes, which is not allowed by the ARM architecture. So remove the
> protocol and its implementation.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Nuke it from orbit:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  ArmPkg/ArmPkg.dec                              |  3 -
>  ArmPkg/Drivers/CpuDxe/CpuDxe.c                 |  1 -
>  ArmPkg/Drivers/CpuDxe/CpuDxe.h                 |  3 -
>  ArmPkg/Drivers/CpuDxe/CpuDxe.inf               |  1 -
>  ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c           | 70 --------------------
>  ArmPkg/Include/Protocol/VirtualUncachedPages.h | 60 -----------------
>  6 files changed, 138 deletions(-)
> 
> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> index 8e9cf199becc..4fd7a5be5158 100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -52,9 +52,6 @@ [Ppis]
>    ## Include/Ppi/ArmMpCoreInfo.h
>    gArmMpCoreInfoPpiGuid = { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d, 0xab, 0x44, 0xe7, 0x54, 0xa8, 0xfc} }
>  
> -[Protocols.common]
> -  gVirtualUncachedPagesProtocolGuid = { 0xAD651C7D, 0x3C22, 0x4DBF, { 0x92, 0xe8, 0x38, 0xa7, 0xcd, 0xae, 0x87, 0xb2 } }
> -
>  [PcdsFeatureFlag.common]
>    gArmTokenSpaceGuid.PcdCpuDxeProduceDebugSupport|FALSE|BOOLEAN|0x00000001
>  
> diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.c b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
> index 7d328d096b1e..5aa5b874144a 100644
> --- a/ArmPkg/Drivers/CpuDxe/CpuDxe.c
> +++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
> @@ -253,7 +253,6 @@ CpuDxeInitialize (
>    Status = gBS->InstallMultipleProtocolInterfaces (
>                  &mCpuHandle,
>                  &gEfiCpuArchProtocolGuid,           &mCpu,
> -                &gVirtualUncachedPagesProtocolGuid, &gVirtualUncachedPages,
>                  NULL
>                  );
>  
> diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.h b/ArmPkg/Drivers/CpuDxe/CpuDxe.h
> index 80c305d53dd1..a00fc3064362 100644
> --- a/ArmPkg/Drivers/CpuDxe/CpuDxe.h
> +++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.h
> @@ -35,7 +35,6 @@
>  #include <Protocol/Cpu.h>
>  #include <Protocol/DebugSupport.h>
>  #include <Protocol/DebugSupportPeriodicCallback.h>
> -#include <Protocol/VirtualUncachedPages.h>
>  #include <Protocol/LoadedImage.h>
>  
>  
> @@ -169,6 +168,4 @@ SetGcdMemorySpaceAttributes (
>    IN UINT64                              Attributes
>    );
>  
> -extern VIRTUAL_UNCACHED_PAGES_PROTOCOL  gVirtualUncachedPages;
> -
>  #endif // __CPU_DXE_ARM_EXCEPTION_H__
> diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
> index b31c994f43e2..d068e06803ed 100644
> --- a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
> +++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
> @@ -61,7 +61,6 @@ [LibraryClasses]
>  [Protocols]
>    gEfiCpuArchProtocolGuid
>    gEfiDebugSupportPeriodicCallbackProtocolGuid
> -  gVirtualUncachedPagesProtocolGuid
>  
>  [Guids]
>    gEfiDebugImageInfoTableGuid
> diff --git a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> index 54d9b0163331..ebe593d1c325 100644
> --- a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> +++ b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> @@ -211,73 +211,3 @@ CpuSetMemoryAttributes (
>      return EFI_SUCCESS;
>    }
>  }
> -
> -EFI_STATUS
> -EFIAPI
> -CpuConvertPagesToUncachedVirtualAddress (
> -  IN  VIRTUAL_UNCACHED_PAGES_PROTOCOL  *This,
> -  IN  EFI_PHYSICAL_ADDRESS              Address,
> -  IN  UINTN                             Length,
> -  IN  EFI_PHYSICAL_ADDRESS              VirtualMask,
> -  OUT UINT64                           *Attributes     OPTIONAL
> -  )
> -{
> -  EFI_STATUS                      Status;
> -  EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor;
> -
> -  if (Attributes != NULL) {
> -    Status = gDS->GetMemorySpaceDescriptor (Address, &GcdDescriptor);
> -    if (!EFI_ERROR (Status)) {
> -      *Attributes = GcdDescriptor.Attributes;
> -    }
> -  }
> -
> -  //
> -  // Make this address range page fault if accessed. If it is a DMA buffer than this would
> -  // be the PCI address. Code should always use the CPU address, and we will or in VirtualMask
> -  // to that address.
> -  //
> -  Status = SetMemoryAttributes (Address, Length, EFI_MEMORY_RO, 0);
> -  if (!EFI_ERROR (Status)) {
> -    Status = SetMemoryAttributes (Address | VirtualMask, Length, EFI_MEMORY_UC, VirtualMask);
> -  }
> -
> -  DEBUG ((DEBUG_INFO | DEBUG_LOAD, "CpuConvertPagesToUncachedVirtualAddress()\n    Unmapped 0x%08lx Mapped 0x%08lx 0x%x bytes\n", Address, Address | VirtualMask, Length));
> -
> -  return Status;
> -}
> -
> -
> -EFI_STATUS
> -EFIAPI
> -CpuReconvertPages (
> -  IN  VIRTUAL_UNCACHED_PAGES_PROTOCOL  *This,
> -  IN  EFI_PHYSICAL_ADDRESS              Address,
> -  IN  UINTN                             Length,
> -  IN  EFI_PHYSICAL_ADDRESS              VirtualMask,
> -  IN  UINT64                            Attributes
> -  )
> -{
> -  EFI_STATUS      Status;
> -
> -  DEBUG ((DEBUG_INFO | DEBUG_LOAD, "CpuReconvertPages(%lx, %x, %lx, %lx)\n", Address, Length, VirtualMask, Attributes));
> -
> -  //
> -  // Unmap the aliased Address
> -  //
> -  Status = SetMemoryAttributes (Address | VirtualMask, Length, EFI_MEMORY_RO, 0);
> -  if (!EFI_ERROR (Status)) {
> -    //
> -    // Restore atttributes
> -    //
> -    Status = SetMemoryAttributes (Address, Length, Attributes, 0);
> -  }
> -
> -  return Status;
> -}
> -
> -
> -VIRTUAL_UNCACHED_PAGES_PROTOCOL  gVirtualUncachedPages = {
> -  CpuConvertPagesToUncachedVirtualAddress,
> -  CpuReconvertPages
> -};
> diff --git a/ArmPkg/Include/Protocol/VirtualUncachedPages.h b/ArmPkg/Include/Protocol/VirtualUncachedPages.h
> deleted file mode 100644
> index 0822184b8931..000000000000
> --- a/ArmPkg/Include/Protocol/VirtualUncachedPages.h
> +++ /dev/null
> @@ -1,60 +0,0 @@
> -/** @file
> -
> -  Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
> -
> -  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.
> -
> -**/
> -
> -#ifndef __VIRTUAL_UNCACHED_PAGES_ROTOCOL_H__
> -#define __VIRTUAL_UNCACHED_PAGES_ROTOCOL_H__
> -
> -//
> -// Protocol GUID
> -//
> -#define VIRTUAL_UNCACHED_PAGES_PROTOCOL_GUID { 0xAD651C7D, 0x3C22, 0x4DBF, { 0x92, 0xe8, 0x38, 0xa7, 0xcd, 0xae, 0x87, 0xb2 } }
> -
> -
> -
> -//
> -// Protocol interface structure
> -//
> -typedef struct _VIRTUAL_UNCACHED_PAGES_PROTOCOL  VIRTUAL_UNCACHED_PAGES_PROTOCOL;
> -
> -
> -typedef
> -EFI_STATUS
> -(EFIAPI *CONVERT_PAGES_TO_UNCACHED_VIRTUAL_ADDRESS) (
> -  IN  VIRTUAL_UNCACHED_PAGES_PROTOCOL   *This,
> -  IN  EFI_PHYSICAL_ADDRESS              Address,
> -  IN  UINTN                             Length,
> -  IN  EFI_PHYSICAL_ADDRESS              VirtualMask,
> -  OUT UINT64                            *Attributes     OPTIONAL
> -  );
> -
> -typedef
> -EFI_STATUS
> -(EFIAPI *FREE_CONVERTED_PAGES) (
> -  IN  VIRTUAL_UNCACHED_PAGES_PROTOCOL   *This,
> -  IN  EFI_PHYSICAL_ADDRESS              Address,
> -  IN  UINTN                             Length,
> -  IN  EFI_PHYSICAL_ADDRESS              VirtualMask,
> -  IN  UINT64                            Attributes
> -  );
> -
> -
> -
> -struct _VIRTUAL_UNCACHED_PAGES_PROTOCOL {
> -  CONVERT_PAGES_TO_UNCACHED_VIRTUAL_ADDRESS  ConvertPages;
> -  FREE_CONVERTED_PAGES                       RevertPages;
> -};
> -
> -extern EFI_GUID gVirtualUncachedPagesProtocolGuid;
> -
> -#endif
> -- 
> 2.7.4
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] ArmPkg: remove DebugUncachedMemoryAllocationLib
  2017-02-23 17:49 ` Leif Lindholm
@ 2017-02-23 17:58   ` Ard Biesheuvel
  0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2017-02-23 17:58 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Laszlo Ersek, Ryan Harkin

On 23 February 2017 at 17:49, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Feb 23, 2017 at 03:48:04PM +0000, Ard Biesheuvel wrote:
>> The debug implementation of the UncachedMemoryAllocationLib library
>> class relies on the creation of an uncached alias of a memory range,
>> while keeping the original cached mapping, but with read-only attributes
>> to trap inadvertent write accesses.
>>
>> This is not a terribly good idea, given that the ARM architecture does
>> not allow mismatched attributes, and so creating them deliberately is
>> not something we should encourage by doing it in reference code.
>
> Agreed.
> One comment near the end:
>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  ArmPkg/ArmPkg.dsc                                                                    |   1 -
>>  ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.c   | 656 --------------------
>>  ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf |  47 --
>>  ArmVirtPkg/ArmVirt.dsc.inc                                                           |   2 -
>>  BeagleBoardPkg/BeagleBoardPkg.dsc                                                    |   1 -
>>  Omap35xxPkg/Omap35xxPkg.dsc                                                          |   3 +-
>>  6 files changed, 1 insertion(+), 709 deletions(-)
>>
>> diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
>> index 0db33eb865b1..1a490d23f7b5 100644
>> --- a/ArmPkg/ArmPkg.dsc
>> +++ b/ArmPkg/ArmPkg.dsc
>> @@ -112,7 +112,6 @@ [Components.common]
>>    ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
>>    ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf
>>    ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
>> -  ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
>>    ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf
>>    ArmPkg/Library/RvdPeCoffExtraActionLib/RvdPeCoffExtraActionLib.inf
>>    ArmPkg/Library/SemiHostingDebugLib/SemiHostingDebugLib.inf
>> diff --git a/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.c b/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.c
>> deleted file mode 100644
>> index 00e01a905c85..000000000000
>> --- a/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.c
>> +++ /dev/null
>> @@ -1,656 +0,0 @@
>> -/** @file
>> -  Debug version of the UncachedMemoryAllocation lib that uses the VirtualUncachedPages
>> -  protocol, produced by the DXE CPU driver, to produce debuggable uncached memory buffers.
>> -
>> -  The DMA rules for EFI contain the concept of a PCI (DMA master) address for memory and
>> -  a CPU (C code) address for the memory buffer that don't have to be the same.  There seem to
>> -  be common errors out there with folks mixing up the two addresses.  This library causes
>> -  the PCI (DMA master) address to not be mapped into system memory so if the CPU (C code)
>> -  uses the wrong pointer it will generate a page fault. The CPU (C code) version of the buffer
>> -  has a virtual address that does not match the physical address. The virtual address has
>> -  PcdArmUncachedMemoryMask ored into the physical address.
>> -
>> -  Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
>> -
>> -  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/BaseLib.h>
>> -#include <Library/BaseMemoryLib.h>
>> -#include <Library/MemoryAllocationLib.h>
>> -#include <Library/DebugLib.h>
>> -#include <Library/UefiBootServicesTableLib.h>
>> -#include <Library/UncachedMemoryAllocationLib.h>
>> -#include <Library/PcdLib.h>
>> -#include <Library/ArmLib.h>
>> -
>> -#include <Protocol/Cpu.h>
>> -#include <Protocol/VirtualUncachedPages.h>
>> -
>> -VOID *
>> -UncachedInternalAllocatePages (
>> -  IN EFI_MEMORY_TYPE  MemoryType,
>> -  IN UINTN            Pages
>> -  );
>> -
>> -VOID *
>> -UncachedInternalAllocateAlignedPages (
>> -  IN EFI_MEMORY_TYPE  MemoryType,
>> -  IN UINTN            Pages,
>> -  IN UINTN            Alignment
>> -  );
>> -
>> -
>> -
>> -EFI_CPU_ARCH_PROTOCOL           *gDebugUncachedCpu;
>> -VIRTUAL_UNCACHED_PAGES_PROTOCOL *gVirtualUncachedPages;
>> -
>> -//
>> -// Assume all of memory has the same cache attributes, unless we do our magic
>> -//
>> -UINT64  gAttributes;
>> -
>> -typedef struct {
>> -  VOID        *Buffer;
>> -  VOID        *Allocation;
>> -  UINTN       Pages;
>> -  LIST_ENTRY  Link;
>> -} FREE_PAGE_NODE;
>> -
>> -LIST_ENTRY  mPageList = INITIALIZE_LIST_HEAD_VARIABLE (mPageList);
>> -
>> -VOID
>> -AddPagesToList (
>> -  IN VOID   *Buffer,
>> -  IN VOID   *Allocation,
>> -  UINTN     Pages
>> -  )
>> -{
>> -  FREE_PAGE_NODE  *NewNode;
>> -
>> -  NewNode = AllocatePool (sizeof (LIST_ENTRY));
>> -  if (NewNode == NULL) {
>> -    ASSERT (FALSE);
>> -    return;
>> -  }
>> -
>> -  NewNode->Buffer     = Buffer;
>> -  NewNode->Allocation = Allocation;
>> -  NewNode->Pages      = Pages;
>> -
>> -  InsertTailList (&mPageList, &NewNode->Link);
>> -}
>> -
>> -
>> -VOID
>> -RemovePagesFromList (
>> -  IN VOID   *Buffer,
>> -  OUT VOID  **Allocation,
>> -  OUT UINTN *Pages
>> -  )
>> -{
>> -  LIST_ENTRY      *Link;
>> -  FREE_PAGE_NODE  *OldNode;
>> -
>> -  *Allocation = NULL;
>> -  *Pages = 0;
>> -
>> -  for (Link = mPageList.ForwardLink; Link != &mPageList; Link = Link->ForwardLink) {
>> -    OldNode = BASE_CR (Link, FREE_PAGE_NODE, Link);
>> -    if (OldNode->Buffer == Buffer) {
>> -      *Allocation = OldNode->Allocation;
>> -      *Pages = OldNode->Pages;
>> -
>> -      RemoveEntryList (&OldNode->Link);
>> -      FreePool (OldNode);
>> -      return;
>> -    }
>> -  }
>> -
>> -  return;
>> -}
>> -
>> -
>> -
>> -EFI_PHYSICAL_ADDRESS
>> -ConvertToPhysicalAddress (
>> -  IN VOID *VirtualAddress
>> -  )
>> -{
>> -  UINTN UncachedMemoryMask = (UINTN)PcdGet64 (PcdArmUncachedMemoryMask);
>> -  UINTN PhysicalAddress;
>> -
>> -  PhysicalAddress = (UINTN)VirtualAddress & ~UncachedMemoryMask;
>> -
>> -  return (EFI_PHYSICAL_ADDRESS)PhysicalAddress;
>> -}
>> -
>> -
>> -VOID *
>> -ConvertToUncachedAddress (
>> -  IN VOID *Address
>> -  )
>> -{
>> -  UINTN UncachedMemoryMask = (UINTN)PcdGet64 (PcdArmUncachedMemoryMask);
>> -  UINTN UncachedAddress;
>> -
>> -  UncachedAddress = (UINTN)Address | UncachedMemoryMask;
>> -
>> -  return (VOID *)UncachedAddress;
>> -}
>> -
>> -
>> -
>> -VOID *
>> -UncachedInternalAllocatePages (
>> -  IN EFI_MEMORY_TYPE  MemoryType,
>> -  IN UINTN            Pages
>> -  )
>> -{
>> -  return UncachedInternalAllocateAlignedPages (MemoryType, Pages, EFI_PAGE_SIZE);
>> -}
>> -
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocatePages (
>> -  IN UINTN  Pages
>> -  )
>> -{
>> -  return UncachedInternalAllocatePages (EfiBootServicesData, Pages);
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateRuntimePages (
>> -  IN UINTN  Pages
>> -  )
>> -{
>> -  return UncachedInternalAllocatePages (EfiRuntimeServicesData, Pages);
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateReservedPages (
>> -  IN UINTN  Pages
>> -  )
>> -{
>> -  return UncachedInternalAllocatePages (EfiReservedMemoryType, Pages);
>> -}
>> -
>> -
>> -
>> -VOID
>> -EFIAPI
>> -UncachedFreePages (
>> -  IN VOID   *Buffer,
>> -  IN UINTN  Pages
>> -  )
>> -{
>> -  UncachedFreeAlignedPages (Buffer, Pages);
>> -  return;
>> -}
>> -
>> -
>> -VOID *
>> -UncachedInternalAllocateAlignedPages (
>> -  IN EFI_MEMORY_TYPE  MemoryType,
>> -  IN UINTN            Pages,
>> -  IN UINTN            Alignment
>> -  )
>> -{
>> -  EFI_STATUS            Status;
>> -  EFI_PHYSICAL_ADDRESS  Memory;
>> -  EFI_PHYSICAL_ADDRESS  AlignedMemory;
>> -  UINTN                 AlignmentMask;
>> -  UINTN                 UnalignedPages;
>> -  UINTN                 RealPages;
>> -
>> -  //
>> -  // Alignment must be a power of two or zero.
>> -  //
>> -  ASSERT ((Alignment & (Alignment - 1)) == 0);
>> -
>> -  if (Pages == 0) {
>> -    return NULL;
>> -  }
>> -  if (Alignment > EFI_PAGE_SIZE) {
>> -    //
>> -    // Caculate the total number of pages since alignment is larger than page size.
>> -    //
>> -    AlignmentMask  = Alignment - 1;
>> -    RealPages      = Pages + EFI_SIZE_TO_PAGES (Alignment);
>> -    //
>> -    // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not overflow.
>> -    //
>> -    ASSERT (RealPages > Pages);
>> -
>> -    Status         = gBS->AllocatePages (AllocateAnyPages, MemoryType, RealPages, &Memory);
>> -    if (EFI_ERROR (Status)) {
>> -      return NULL;
>> -    }
>> -    AlignedMemory  = ((UINTN) Memory + AlignmentMask) & ~AlignmentMask;
>> -    UnalignedPages = EFI_SIZE_TO_PAGES (AlignedMemory - (UINTN) Memory);
>> -    if (UnalignedPages > 0) {
>> -      //
>> -      // Free first unaligned page(s).
>> -      //
>> -      Status = gBS->FreePages (Memory, UnalignedPages);
>> -      ASSERT_EFI_ERROR (Status);
>> -    }
>> -    Memory         = (EFI_PHYSICAL_ADDRESS) (AlignedMemory + EFI_PAGES_TO_SIZE (Pages));
>> -    UnalignedPages = RealPages - Pages - UnalignedPages;
>> -    if (UnalignedPages > 0) {
>> -      //
>> -      // Free last unaligned page(s).
>> -      //
>> -      Status = gBS->FreePages (Memory, UnalignedPages);
>> -      ASSERT_EFI_ERROR (Status);
>> -    }
>> -  } else {
>> -    //
>> -    // Do not over-allocate pages in this case.
>> -    //
>> -    Status = gBS->AllocatePages (AllocateAnyPages, MemoryType, Pages, &Memory);
>> -    if (EFI_ERROR (Status)) {
>> -      return NULL;
>> -    }
>> -    AlignedMemory  = (UINTN) Memory;
>> -  }
>> -
>> -  Status = gVirtualUncachedPages->ConvertPages (gVirtualUncachedPages, AlignedMemory, Pages * EFI_PAGE_SIZE, PcdGet64 (PcdArmUncachedMemoryMask), &gAttributes);
>> -  if (EFI_ERROR (Status)) {
>> -    return NULL;
>> -  }
>> -
>> -  AlignedMemory = (EFI_PHYSICAL_ADDRESS)(UINTN)ConvertToUncachedAddress ((VOID *)(UINTN)AlignedMemory);
>> -
>> -  return (VOID *)(UINTN)AlignedMemory;
>> -}
>> -
>> -
>> -VOID
>> -EFIAPI
>> -UncachedFreeAlignedPages (
>> -  IN VOID   *Buffer,
>> -  IN UINTN  Pages
>> -  )
>> -{
>> -  EFI_STATUS            Status;
>> -  EFI_PHYSICAL_ADDRESS  Memory;
>> -
>> -  ASSERT (Pages != 0);
>> -
>> -  Memory = ConvertToPhysicalAddress (Buffer);
>> -
>> -  Status = gVirtualUncachedPages->RevertPages (gVirtualUncachedPages, Memory, Pages * EFI_PAGE_SIZE, PcdGet64 (PcdArmUncachedMemoryMask), gAttributes);
>> -
>> -
>> -  Status = gBS->FreePages ((EFI_PHYSICAL_ADDRESS) (UINTN) Memory, Pages);
>> -  ASSERT_EFI_ERROR (Status);
>> -}
>> -
>> -
>> -
>> -
>> -VOID *
>> -UncachedInternalAllocateAlignedPool (
>> -  IN EFI_MEMORY_TYPE  PoolType,
>> -  IN UINTN            AllocationSize,
>> -  IN UINTN            Alignment
>> -  )
>> -{
>> -  VOID      *AlignedAddress;
>> -
>> -  //
>> -  // Alignment must be a power of two or zero.
>> -  //
>> -  ASSERT ((Alignment & (Alignment - 1)) == 0);
>> -
>> -  if (Alignment < EFI_PAGE_SIZE) {
>> -    Alignment = EFI_PAGE_SIZE;
>> -  }
>> -
>> -  AlignedAddress = UncachedInternalAllocateAlignedPages (PoolType, EFI_SIZE_TO_PAGES (AllocationSize), Alignment);
>> -  if (AlignedAddress == NULL) {
>> -    return NULL;
>> -  }
>> -
>> -  AddPagesToList ((VOID *)(UINTN)ConvertToPhysicalAddress (AlignedAddress), (VOID *)(UINTN)AlignedAddress, EFI_SIZE_TO_PAGES (AllocationSize));
>> -
>> -  return (VOID *) AlignedAddress;
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateAlignedPool (
>> -  IN UINTN  AllocationSize,
>> -  IN UINTN  Alignment
>> -  )
>> -{
>> -  return UncachedInternalAllocateAlignedPool (EfiBootServicesData, AllocationSize, Alignment);
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateAlignedRuntimePool (
>> -  IN UINTN  AllocationSize,
>> -  IN UINTN  Alignment
>> -  )
>> -{
>> -  return UncachedInternalAllocateAlignedPool (EfiRuntimeServicesData, AllocationSize, Alignment);
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateAlignedReservedPool (
>> -  IN UINTN  AllocationSize,
>> -  IN UINTN  Alignment
>> -  )
>> -{
>> -  return UncachedInternalAllocateAlignedPool (EfiReservedMemoryType, AllocationSize, Alignment);
>> -}
>> -
>> -VOID *
>> -UncachedInternalAllocateAlignedZeroPool (
>> -  IN EFI_MEMORY_TYPE  PoolType,
>> -  IN UINTN            AllocationSize,
>> -  IN UINTN            Alignment
>> -  )
>> -{
>> -  VOID    *Memory;
>> -  Memory = UncachedInternalAllocateAlignedPool (PoolType, AllocationSize, Alignment);
>> -  if (Memory != NULL) {
>> -    Memory = ZeroMem (Memory, AllocationSize);
>> -  }
>> -  return Memory;
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateAlignedZeroPool (
>> -  IN UINTN  AllocationSize,
>> -  IN UINTN  Alignment
>> -  )
>> -{
>> -  return UncachedInternalAllocateAlignedZeroPool (EfiBootServicesData, AllocationSize, Alignment);
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateAlignedRuntimeZeroPool (
>> -  IN UINTN  AllocationSize,
>> -  IN UINTN  Alignment
>> -  )
>> -{
>> -  return UncachedInternalAllocateAlignedZeroPool (EfiRuntimeServicesData, AllocationSize, Alignment);
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateAlignedReservedZeroPool (
>> -  IN UINTN  AllocationSize,
>> -  IN UINTN  Alignment
>> -  )
>> -{
>> -  return UncachedInternalAllocateAlignedZeroPool (EfiReservedMemoryType, AllocationSize, Alignment);
>> -}
>> -
>> -VOID *
>> -UncachedInternalAllocateAlignedCopyPool (
>> -  IN EFI_MEMORY_TYPE  PoolType,
>> -  IN UINTN            AllocationSize,
>> -  IN CONST VOID       *Buffer,
>> -  IN UINTN            Alignment
>> -  )
>> -{
>> -  VOID  *Memory;
>> -
>> -  ASSERT (Buffer != NULL);
>> -  ASSERT (AllocationSize <= (MAX_ADDRESS - (UINTN) Buffer + 1));
>> -
>> -  Memory = UncachedInternalAllocateAlignedPool (PoolType, AllocationSize, Alignment);
>> -  if (Memory != NULL) {
>> -    Memory = CopyMem (Memory, Buffer, AllocationSize);
>> -  }
>> -  return Memory;
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateAlignedCopyPool (
>> -  IN UINTN       AllocationSize,
>> -  IN CONST VOID  *Buffer,
>> -  IN UINTN       Alignment
>> -  )
>> -{
>> -  return UncachedInternalAllocateAlignedCopyPool (EfiBootServicesData, AllocationSize, Buffer, Alignment);
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateAlignedRuntimeCopyPool (
>> -  IN UINTN       AllocationSize,
>> -  IN CONST VOID  *Buffer,
>> -  IN UINTN       Alignment
>> -  )
>> -{
>> -  return UncachedInternalAllocateAlignedCopyPool (EfiRuntimeServicesData, AllocationSize, Buffer, Alignment);
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateAlignedReservedCopyPool (
>> -  IN UINTN       AllocationSize,
>> -  IN CONST VOID  *Buffer,
>> -  IN UINTN       Alignment
>> -  )
>> -{
>> -  return UncachedInternalAllocateAlignedCopyPool (EfiReservedMemoryType, AllocationSize, Buffer, Alignment);
>> -}
>> -
>> -VOID
>> -EFIAPI
>> -UncachedFreeAlignedPool (
>> -  IN VOID   *Buffer
>> -  )
>> -{
>> -  VOID    *Allocation;
>> -  UINTN   Pages;
>> -
>> -  RemovePagesFromList (Buffer, &Allocation, &Pages);
>> -
>> -  UncachedFreePages (Allocation, Pages);
>> -}
>> -
>> -VOID *
>> -UncachedInternalAllocatePool (
>> -  IN EFI_MEMORY_TYPE  MemoryType,
>> -  IN UINTN            AllocationSize
>> -  )
>> -{
>> -  UINTN CacheLineLength = ArmDataCacheLineLength ();
>> -  return UncachedInternalAllocateAlignedPool (MemoryType, AllocationSize, CacheLineLength);
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocatePool (
>> -  IN UINTN  AllocationSize
>> -  )
>> -{
>> -  return UncachedInternalAllocatePool (EfiBootServicesData, AllocationSize);
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateRuntimePool (
>> -  IN UINTN  AllocationSize
>> -  )
>> -{
>> -  return UncachedInternalAllocatePool (EfiRuntimeServicesData, AllocationSize);
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateReservedPool (
>> -  IN UINTN  AllocationSize
>> -  )
>> -{
>> -  return UncachedInternalAllocatePool (EfiReservedMemoryType, AllocationSize);
>> -}
>> -
>> -VOID *
>> -UncachedInternalAllocateZeroPool (
>> -  IN EFI_MEMORY_TYPE  PoolType,
>> -  IN UINTN            AllocationSize
>> -  )
>> -{
>> -  VOID  *Memory;
>> -
>> -  Memory = UncachedInternalAllocatePool (PoolType, AllocationSize);
>> -  if (Memory != NULL) {
>> -    Memory = ZeroMem (Memory, AllocationSize);
>> -  }
>> -  return Memory;
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateZeroPool (
>> -  IN UINTN  AllocationSize
>> -  )
>> -{
>> -  return UncachedInternalAllocateZeroPool (EfiBootServicesData, AllocationSize);
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateRuntimeZeroPool (
>> -  IN UINTN  AllocationSize
>> -  )
>> -{
>> -  return UncachedInternalAllocateZeroPool (EfiRuntimeServicesData, AllocationSize);
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateReservedZeroPool (
>> -  IN UINTN  AllocationSize
>> -  )
>> -{
>> -  return UncachedInternalAllocateZeroPool (EfiReservedMemoryType, AllocationSize);
>> -}
>> -
>> -VOID *
>> -UncachedInternalAllocateCopyPool (
>> -  IN EFI_MEMORY_TYPE  PoolType,
>> -  IN UINTN            AllocationSize,
>> -  IN CONST VOID       *Buffer
>> -  )
>> -{
>> -  VOID  *Memory;
>> -
>> -  ASSERT (Buffer != NULL);
>> -  ASSERT (AllocationSize <= (MAX_ADDRESS - (UINTN) Buffer + 1));
>> -
>> -  Memory = UncachedInternalAllocatePool (PoolType, AllocationSize);
>> -  if (Memory != NULL) {
>> -     Memory = CopyMem (Memory, Buffer, AllocationSize);
>> -  }
>> -  return Memory;
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateCopyPool (
>> -  IN UINTN       AllocationSize,
>> -  IN CONST VOID  *Buffer
>> -  )
>> -{
>> -  return UncachedInternalAllocateCopyPool (EfiBootServicesData, AllocationSize, Buffer);
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateRuntimeCopyPool (
>> -  IN UINTN       AllocationSize,
>> -  IN CONST VOID  *Buffer
>> -  )
>> -{
>> -  return UncachedInternalAllocateCopyPool (EfiRuntimeServicesData, AllocationSize, Buffer);
>> -}
>> -
>> -VOID *
>> -EFIAPI
>> -UncachedAllocateReservedCopyPool (
>> -  IN UINTN       AllocationSize,
>> -  IN CONST VOID  *Buffer
>> -  )
>> -{
>> -  return UncachedInternalAllocateCopyPool (EfiReservedMemoryType, AllocationSize, Buffer);
>> -}
>> -
>> -VOID
>> -EFIAPI
>> -UncachedFreePool (
>> -  IN VOID   *Buffer
>> -  )
>> -{
>> -  UncachedFreeAlignedPool (Buffer);
>> -}
>> -
>> -VOID
>> -EFIAPI
>> -UncachedSafeFreePool (
>> -  IN VOID   *Buffer
>> -  )
>> -{
>> -  if (Buffer != NULL) {
>> -    UncachedFreePool (Buffer);
>> -    Buffer = NULL;
>> -  }
>> -}
>> -
>> -/**
>> -  The constructor function caches the pointer of DXE Services Table.
>> -
>> -  The constructor function caches the pointer of DXE Services Table.
>> -  It will ASSERT() if that operation fails.
>> -  It will ASSERT() if the pointer of DXE Services Table is NULL.
>> -  It will always return EFI_SUCCESS.
>> -
>> -  @param  ImageHandle   The firmware allocated handle for the EFI image.
>> -  @param  SystemTable   A pointer to the EFI System Table.
>> -
>> -  @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
>> -
>> -**/
>> -EFI_STATUS
>> -EFIAPI
>> -DebugUncachedMemoryAllocationLibConstructor (
>> -  IN EFI_HANDLE        ImageHandle,
>> -  IN EFI_SYSTEM_TABLE  *SystemTable
>> -  )
>> -{
>> -  EFI_STATUS    Status;
>> -
>> -  Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&gDebugUncachedCpu);
>> -  ASSERT_EFI_ERROR(Status);
>> -
>> -  Status = gBS->LocateProtocol (&gVirtualUncachedPagesProtocolGuid, NULL, (VOID **)&gVirtualUncachedPages);
>> -  ASSERT_EFI_ERROR(Status);
>> -
>> -  return Status;
>> -}
>> -
>> -
>> -
>> diff --git a/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf b/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
>> deleted file mode 100644
>> index 213188ac2c39..000000000000
>> --- a/ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
>> +++ /dev/null
>> @@ -1,47 +0,0 @@
>> -#/** @file
>> -#
>> -#
>> -# Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
>> -#
>> -#  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                      = UncachedMemoryAllocationLib
>> -  FILE_GUID                      = 3C1EA826-696A-4E8A-B89D-3C5369B84F2A
>> -  MODULE_TYPE                    = DXE_DRIVER
>> -  VERSION_STRING                 = 1.0
>> -  LIBRARY_CLASS                  = UncachedMemoryAllocationLib
>> -  CONSTRUCTOR                    = DebugUncachedMemoryAllocationLibConstructor
>> -
>> -[Sources.common]
>> -  DebugUncachedMemoryAllocationLib.c
>> -
>> -[Packages]
>> -  MdePkg/MdePkg.dec
>> -  ArmPkg/ArmPkg.dec
>> -
>> -
>> -[LibraryClasses]
>> -  BaseLib
>> -  MemoryAllocationLib
>> -  ArmLib
>> -
>> -[Protocols]
>> -  gEfiCpuArchProtocolGuid
>> -  gVirtualUncachedPagesProtocolGuid
>> -
>> -[FixedPcd]
>> -  gArmTokenSpaceGuid.PcdArmUncachedMemoryMask
>> -
>> -
>> -[Depex]
>> -  gEfiCpuArchProtocolGuid AND gVirtualUncachedPagesProtocolGuid
>> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
>> index c0d5e7c6aa6d..61d4a6642eb7 100644
>> --- a/ArmVirtPkg/ArmVirt.dsc.inc
>> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
>> @@ -28,10 +28,8 @@ [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>>  [LibraryClasses.common]
>>  !if $(TARGET) == RELEASE
>>    DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
>> -  UncachedMemoryAllocationLib|ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
>
> This deletion is of a (not needed) non-Debug version (which is not
> described in the commit message). Could you add a note to that effect
> in there?
>
> With that:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

Both pushed, thanks,


>>  !else
>>    DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>> -  UncachedMemoryAllocationLib|ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
>>  !endif
>>    DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
>>
>> diff --git a/BeagleBoardPkg/BeagleBoardPkg.dsc b/BeagleBoardPkg/BeagleBoardPkg.dsc
>> index b24db6cbfb04..a71a01ac7723 100644
>> --- a/BeagleBoardPkg/BeagleBoardPkg.dsc
>> +++ b/BeagleBoardPkg/BeagleBoardPkg.dsc
>> @@ -49,7 +49,6 @@ [LibraryClasses.common]
>>  !else
>>    DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>>    UncachedMemoryAllocationLib|ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
>> -#  UncachedMemoryAllocationLib|ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
>>  !endif
>>    DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
>>
>> diff --git a/Omap35xxPkg/Omap35xxPkg.dsc b/Omap35xxPkg/Omap35xxPkg.dsc
>> index 9395570ccaa3..ad7d9898c330 100644
>> --- a/Omap35xxPkg/Omap35xxPkg.dsc
>> +++ b/Omap35xxPkg/Omap35xxPkg.dsc
>> @@ -72,8 +72,7 @@ [LibraryClasses.common]
>>
>>    UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
>>
>> - # UncachedMemoryAllocationLib|ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
>> -  UncachedMemoryAllocationLib|ArmPkg/Library/DebugUncachedMemoryAllocationLib/DebugUncachedMemoryAllocationLib.inf
>> +  UncachedMemoryAllocationLib|ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
>>
>>    CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
>>
>> --
>> 2.7.4
>>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-02-23 17:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-23 15:48 [PATCH 1/2] ArmPkg: remove DebugUncachedMemoryAllocationLib Ard Biesheuvel
2017-02-23 15:48 ` [PATCH 2/2] ArmPkg/CpuDxe: remove VirtualUncachedPages protocol and implementation Ard Biesheuvel
2017-02-23 17:50   ` Leif Lindholm
2017-02-23 16:19 ` [PATCH 1/2] ArmPkg: remove DebugUncachedMemoryAllocationLib Laszlo Ersek
2017-02-23 17:49 ` Leif Lindholm
2017-02-23 17:58   ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox