public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Leif Lindholm <leif.lindholm@linaro.org>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: edk2-devel@lists.01.org, lersek@redhat.com, ryan.harkin@linaro.org
Subject: Re: [PATCH 1/2] ArmPkg: remove DebugUncachedMemoryAllocationLib
Date: Thu, 23 Feb 2017 17:49:02 +0000	[thread overview]
Message-ID: <20170223174902.GX16034@bivouac.eciton.net> (raw)
In-Reply-To: <1487864885-13485-1-git-send-email-ard.biesheuvel@linaro.org>

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
> 


  parent reply	other threads:[~2017-02-23 17:49 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2017-02-23 17:58   ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170223174902.GX16034@bivouac.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox