Re: [PATCH] OvmfPkg/GcdNotifyDxe: Install EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL

[Laszlo Ersek <lersek@redhat.com>]

On 04/12/17 21:00, Leo Duran wrote:
> On entry, GcdNotifyDxe scans the GCD MemmorySpace map and produces this
> protocol to get notifications from GCD on MemorySpace Add/Remove operations.
> 
> This patch illustrates how OvmfPkg could take actions on GCD notifications.
> For example: updating the SEV mask on page-table entries for MMIO ranges.
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Brijesh Singh <brijesh.ksingh@gmail.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Leo Duran <leo.duran@amd.com>
> ---
>  OvmfPkg/GcdNotifyDxe/GcdNotifyDxe.c   | 148 ++++++++++++++++++++++++++++++++++
>  OvmfPkg/GcdNotifyDxe/GcdNotifyDxe.inf |  44 ++++++++++
>  OvmfPkg/OvmfPkgIa32X64.dsc            |   2 +
>  OvmfPkg/OvmfPkgIa32X64.fdf            |   3 +
>  4 files changed, 197 insertions(+)
>  create mode 100644 OvmfPkg/GcdNotifyDxe/GcdNotifyDxe.c
>  create mode 100644 OvmfPkg/GcdNotifyDxe/GcdNotifyDxe.inf

I guess this patch depends on

  [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL

so I think we should first await the outcome of that discussion.

Still, is there any particular reason to include the driver in the
OvmfPkgIa32X64 platform only? (No Ia32, no X64?)

Thanks
Laszlo

> 
> diff --git a/OvmfPkg/GcdNotifyDxe/GcdNotifyDxe.c b/OvmfPkg/GcdNotifyDxe/GcdNotifyDxe.c
> new file mode 100644
> index 0000000..1b54584
> --- /dev/null
> +++ b/OvmfPkg/GcdNotifyDxe/GcdNotifyDxe.c
> @@ -0,0 +1,148 @@
> +/** @file
> +
> +  GCD Memory Space Map notification protocol handler.
> +
> +  Copyright (c) 2017, AMD 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 <PiDxe.h>
> +
> +#include <Library/BaseLib.h>
> +#include <Library/UefiLib.h>
> +#include <Library/UefiDriverEntryPoint.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/DxeServicesTableLib.h>
> +#include <Library/DebugLib.h>
> +
> +#include <Protocol/GcdMemorySpaceNotify.h>
> +
> +//
> +// GCD Memory Space Map notification protocol
> +//
> +STATIC EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL  mGcdMemorySpaceNotifyProtocol;
> +
> +///
> +/// Lookup table used to print GCD Memory Space Map
> +///
> +GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 *mGcdMemoryTypeNames[] = {
> +  "EfiGcdMemoryTypeNonExistent",
> +  "EfiGcdMemoryTypeReserved",
> +  "EfiGcdMemoryTypeSystemMemory",
> +  "EfiGcdMemoryTypeMemoryMappedIo",
> +  "EfiGcdMemoryTypePersistentMemory",
> +  "EfiGcdMemoryTypeMoreReliable",
> +  "EfiGcdMemoryTypeMaximum"
> +};
> +
> +
> +/**
> +Notify on: Add a segment of memory to GCD map.
> +
> +@param  GcdMemoryType          Memory type of the segment.
> +@param  BaseAddress            Base address of the segment.
> +@param  Length                 Length of the segment.
> +@param  Capabilities           Alterable attributes of the segment.
> +
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +GcdMemorySpaceAddNotify (
> +IN EFI_GCD_MEMORY_TYPE   GcdMemoryType,
> +IN EFI_PHYSICAL_ADDRESS  BaseAddress,
> +IN UINT64                Length,
> +IN UINT64                Capabilities
> +)
> +{
> +  DEBUG ((EFI_D_INFO, "%a()\n", __FUNCTION__));
> +
> +  if (GcdMemoryType >= EfiGcdMemoryTypeNonExistent && GcdMemoryType <= EfiGcdMemoryTypeMaximum) {
> +    DEBUG ((EFI_D_INFO, " GcdMemoryType = 0x%X (%a)\n", GcdMemoryType, mGcdMemoryTypeNames[GcdMemoryType]));
> +    DEBUG ((EFI_D_INFO, " BaseAddress   = 0x%p\n", BaseAddress));
> +    DEBUG ((EFI_D_INFO, " Length        = 0x%lX\n", Length));
> +
> +    if (GcdMemoryType == EfiGcdMemoryTypeMemoryMappedIo) {
> +      DEBUG ((EFI_D_INFO, " MMIO: Start = 0x%p, Length = 0x%lX\n", BaseAddress, Length));
> +    }
> +  }
> +  else {
> +    DEBUG ((EFI_D_INFO, " Invalid GcdMemoryType = 0x%X\n", GcdMemoryType));
> +  }
> +}
> +
> +
> +/**
> +Notify on: Remove a segment of memory to GCD map.
> +
> +@param  BaseAddress            Base address of the segment.
> +@param  Length                 Length of the segment.
> +
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +GcdMemorySpaceRemoveNotify (
> +IN EFI_PHYSICAL_ADDRESS  BaseAddress,
> +IN UINT64                Length
> +)
> +{
> +  DEBUG ((EFI_D_INFO, "%a()\n", __FUNCTION__));
> +  DEBUG ((EFI_D_INFO, " BaseAddress = 0x%p\n", BaseAddress));
> +  DEBUG ((EFI_D_INFO, " Length = 0x%lX\n", Length));
> +}
> +
> +
> +EFI_STATUS
> +EFIAPI
> +GcdNotifyDxeEntry (
> +  IN EFI_HANDLE         ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_GCD_MEMORY_SPACE_DESCRIPTOR  *MemorySpaceMap;
> +  UINTN                            NumEntries;
> +  UINTN                            Index;
> +  EFI_STATUS                       Status;
> +  EFI_HANDLE                       Handle;
> +
> +  DEBUG((EFI_D_INFO, "%a - ENTRY\n", __FUNCTION__));
> +
> +  //
> +  // Iterate through the current MemorySpace Map 
> +  //
> +  MemorySpaceMap = NULL;
> +  Status = gDS->GetMemorySpaceMap (&NumEntries, &MemorySpaceMap);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  for (Index = 0; Index < NumEntries; ++Index) {
> +    if (MemorySpaceMap[Index].GcdMemoryType == EfiGcdMemoryTypeMemoryMappedIo) {
> +      DEBUG ((DEBUG_INFO, " MMIO: Start = 0x%p, Length = 0x%lX\n",
> +        MemorySpaceMap[Index].BaseAddress, MemorySpaceMap[Index].Length));
> +    }  
> +  }
> + 
> +  //
> +  // Install GCD Memory Space Map notification protocol
> +  //
> +  mGcdMemorySpaceNotifyProtocol.MemorySpaceAddNotify = GcdMemorySpaceAddNotify;
> +  mGcdMemorySpaceNotifyProtocol.MemorySpaceRemoveNotify = GcdMemorySpaceRemoveNotify;
> +  Handle = NULL;
> +  Status = gBS->InstallProtocolInterface (
> +    &Handle,
> +    &gEfiGcdMemorySpaceNotifyProtocolGuid,
> +    EFI_NATIVE_INTERFACE,
> +    &mGcdMemorySpaceNotifyProtocol);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  DEBUG((EFI_D_INFO, "%a - EXIT (Status = %r)\n", __FUNCTION__, Status));
> +  return Status;
> +}
> diff --git a/OvmfPkg/GcdNotifyDxe/GcdNotifyDxe.inf b/OvmfPkg/GcdNotifyDxe/GcdNotifyDxe.inf
> new file mode 100644
> index 0000000..a4c8445
> --- /dev/null
> +++ b/OvmfPkg/GcdNotifyDxe/GcdNotifyDxe.inf
> @@ -0,0 +1,44 @@
> +#/** @file
> +#
> +#  Component description file for GcdNotifyDxe module
> +#
> +#  Copyright (c) 2017, AMD 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                      = GcdNotifyDxe
> +  FILE_GUID                      = 2ec9da37-ee35-4de9-86c5-6d9a81dc38a7
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = GcdNotifyDxeEntry
> +
> +[Sources]
> +  GcdNotifyDxe.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  UefiLib
> +  UefiDriverEntryPoint
> +  UefiBootServicesTableLib
> +  DxeServicesTableLib
> +  DebugLib
> +
> +[Protocols]
> +  gEfiGcdMemorySpaceNotifyProtocolGuid    ## PRODUCES
> +
> +[Depex]
> +  TRUE
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 71ac62f..441d7cc 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -3,6 +3,7 @@
>  #
>  #  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>  #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> +#  Copyright (c) 2017, AMD 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
> @@ -798,6 +799,7 @@
>  !endif
>  
>    OvmfPkg/PlatformDxe/Platform.inf
> +  OvmfPkg/GcdNotifyDxe/GcdNotifyDxe.inf
>  
>  !if $(SMM_REQUIRE) == TRUE
>    OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
> index 5233314..71b118a 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
> @@ -3,6 +3,7 @@
>  #
>  #  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>  #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> +#  Copyright (c) 2017, AMD 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
> @@ -193,6 +194,7 @@ APRIORI DXE {
>  !if $(SMM_REQUIRE) == FALSE
>    INF  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
>  !endif
> +  INF  OvmfPkg/GcdNotifyDxe/GcdNotifyDxe.inf
>  }
>  
>  #
> @@ -351,6 +353,7 @@ INF  RuleOverride=CSM OvmfPkg/Csm/Csm16/Csm16.inf
>  INF  OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
>  INF  OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>  INF  OvmfPkg/PlatformDxe/Platform.inf
> +INF  OvmfPkg/GcdNotifyDxe/GcdNotifyDxe.inf
>  
>  !if $(SMM_REQUIRE) == TRUE
>  INF  OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
>