Re: [PATCH 2/2] ArmPkg/CpuDxe: remove VirtualUncachedPages protocol and implementation

[Leif Lindholm <leif.lindholm@linaro.org>]

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
>