Re: [edk2-devel] [PATCH v2 1/5] ArmVirtPkg: Library: Memory initialization for Cloud Hypervisor

["Laszlo Ersek" <lersek@redhat.com>]

On 05/17/21 08:50, Jianyong Wu wrote:
> Cloud Hypervisor is kvm based VMM implemented in rust.
> 
> This library populates the system memory map for the
> Cloud Hypervisor virtual platform.
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> ---
>  .../CloudHvVirtMemInfoPeiLib.inf              |  47 ++++++++
>  .../CloudHvVirtMemInfoLib.c                   |  94 ++++++++++++++++
>  .../CloudHvVirtMemInfoPeiLibConstructor.c     | 100 ++++++++++++++++++
>  3 files changed, 241 insertions(+)
>  create mode 100644 ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf
>  create mode 100644 ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
>  create mode 100644 ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLibConstructor.c
> 
> diff --git a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf
> new file mode 100644
> index 000000000000..71dbf9c06ccc
> --- /dev/null
> +++ b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf
> @@ -0,0 +1,47 @@
> +#/* @file
> +#
> +#  Copyright (c) 2011-2015, ARM Limited. All rights reserved.

(1) New file, please refresh the end year in the (C) notice.

> +#  Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#*/
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001A
> +  BASE_NAME                      = ClhVirtMemInfoPeiLib
> +  FILE_GUID                      = 3E29D940-0591-EE6A-CAD4-223A9CF55E75
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = ArmVirtMemInfoLib|PEIM
> +  CONSTRUCTOR                    = CloudHvVirtMemInfoPeiLibConstructor
> +
> +[Sources]
> +  CloudHvVirtMemInfoLib.c
> +  CloudHvVirtMemInfoPeiLibConstructor.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  ArmVirtPkg/ArmVirtPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  ArmLib
> +  BaseMemoryLib
> +  DebugLib
> +  FdtLib
> +  PcdLib
> +  MemoryAllocationLib
> +
> +[Pcd]
> +  gArmTokenSpaceGuid.PcdFdBaseAddress
> +  gArmTokenSpaceGuid.PcdFvBaseAddress
> +  gArmTokenSpaceGuid.PcdSystemMemoryBase
> +  gArmTokenSpaceGuid.PcdSystemMemorySize
> +
> +[FixedPcd]
> +  gArmTokenSpaceGuid.PcdFdSize
> +  gArmTokenSpaceGuid.PcdFvSize
> +  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
> diff --git a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
> new file mode 100644
> index 000000000000..69f4e6ab6cc4
> --- /dev/null
> +++ b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
> @@ -0,0 +1,94 @@
> +/** @file
> +
> +  Copyright (c) 2014-2017, Linaro Limited. All rights reserved.

(2) New file, ARM (C) missing.

> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Base.h>
> +#include <Library/ArmLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +
> +// Number of Virtual Memory Map Descriptors
> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          5
> +
> +//
> +// mach-virt's core peripherals such as the UART, the GIC and the RTC are
> +// all mapped in the 'miscellaneous device I/O' region, which we just map
> +// in its entirety rather than device by device. Note that it does not
> +// cover any of the NOR flash banks or PCI resource windows.
> +//
> +#define MACH_VIRT_PERIPH_BASE       0x08000000
> +#define MACH_VIRT_PERIPH_SIZE       SIZE_128MB
> +
> +//
> +// in cloud-hypervisor, 0x0 ~ 0x8000000 is reserved as normal memory for UEFI
> +//
> +#define CLOUDHV_UEFI_MEM_BASE       0x0
> +#define CLOUDHV_UEFI_MEM_SIZE       0x08000000
> +
> +/**
> +  Return the Virtual Memory Map of your platform
> +
> +  This Virtual Memory Map is used by MemoryInitPei Module to initialize the MMU
> +  on your platform.
> +
> +  @param[out]   VirtualMemoryMap    Array of ARM_MEMORY_REGION_DESCRIPTOR
> +                                    describing a Physical-to-Virtual Memory
> +                                    mapping. This array must be ended by a
> +                                    zero-filled entry. The allocated memory
> +                                    will not be freed.
> +
> +**/
> +VOID
> +ArmVirtGetMemoryMap (
> +  OUT ARM_MEMORY_REGION_DESCRIPTOR   **VirtualMemoryMap
> +  )
> +{
> +  ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
> +
> +  ASSERT (VirtualMemoryMap != NULL);
> +
> +  VirtualMemoryTable = AllocatePool (sizeof (ARM_MEMORY_REGION_DESCRIPTOR) *
> +                                     MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS);
> +
> +  if (VirtualMemoryTable == NULL) {
> +    DEBUG ((DEBUG_ERROR, "%a: Error: Failed AllocatePool()\n", __FUNCTION__));
> +    return;
> +  }
> +
> +  // System DRAM
> +  VirtualMemoryTable[0].PhysicalBase = PcdGet64 (PcdSystemMemoryBase);
> +  VirtualMemoryTable[0].VirtualBase  = VirtualMemoryTable[0].PhysicalBase;
> +  VirtualMemoryTable[0].Length       = PcdGet64 (PcdSystemMemorySize);
> +  VirtualMemoryTable[0].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> +
> +  DEBUG ((DEBUG_INFO, "%a: Dumping System DRAM Memory Map:\n"
> +      "\tPhysicalBase: 0x%lX\n"
> +      "\tVirtualBase: 0x%lX\n"
> +      "\tLength: 0x%lX\n",
> +      __FUNCTION__,
> +      VirtualMemoryTable[0].PhysicalBase,
> +      VirtualMemoryTable[0].VirtualBase,
> +      VirtualMemoryTable[0].Length));
> +
> +  // Memory mapped peripherals (UART, RTC, GIC, virtio-mmio, etc)
> +  VirtualMemoryTable[1].PhysicalBase = MACH_VIRT_PERIPH_BASE;
> +  VirtualMemoryTable[1].VirtualBase  = MACH_VIRT_PERIPH_BASE;
> +  VirtualMemoryTable[1].Length       = MACH_VIRT_PERIPH_SIZE;
> +  VirtualMemoryTable[1].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // Map the FV region as normal executable memory
> +  VirtualMemoryTable[2].PhysicalBase = PcdGet64 (PcdFvBaseAddress);
> +  VirtualMemoryTable[2].VirtualBase  = VirtualMemoryTable[2].PhysicalBase;
> +  VirtualMemoryTable[2].Length       = FixedPcdGet32 (PcdFvSize);
> +  VirtualMemoryTable[2].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> +
> +  // End of Table
> +  ZeroMem (&VirtualMemoryTable[3], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
> +
> +  *VirtualMemoryMap = VirtualMemoryTable;
> +}
> diff --git a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLibConstructor.c b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLibConstructor.c
> new file mode 100644
> index 000000000000..062dfcee1d66
> --- /dev/null
> +++ b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLibConstructor.c
> @@ -0,0 +1,100 @@
> +/** @file
> +
> +  Copyright (c) 2014-2017, Linaro Limited. All rights reserved.

(3) New file, ARM (C) missing.

With those updates:

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

(BTW I'd like at least one of Ard, Leif, and Sami to give an R-b for
this patch set.)

Thanks,
Laszlo

> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Base.h>
> +#include <Library/DebugLib.h>
> +#include <Library/PcdLib.h>
> +#include <libfdt.h>
> +
> +RETURN_STATUS
> +EFIAPI
> +CloudHvVirtMemInfoPeiLibConstructor (
> +  VOID
> +  )
> +{
> +  VOID          *DeviceTreeBase;
> +  INT32         Node, Prev;
> +  UINT64        NewBase, CurBase;
> +  UINT64        NewSize, CurSize;
> +  CONST CHAR8   *Type;
> +  INT32         Len;
> +  CONST UINT64  *RegProp;
> +  RETURN_STATUS PcdStatus;
> +
> +  NewBase = 0;
> +  NewSize = 0;
> +
> +  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
> +  ASSERT (DeviceTreeBase != NULL);
> +
> +  //
> +  // Make sure we have a valid device tree blob
> +  //
> +  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
> +
> +  //
> +  // Look for the lowest memory node
> +  //
> +  for (Prev = 0;; Prev = Node) {
> +    Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
> +    if (Node < 0) {
> +      break;
> +    }
> +
> +    //
> +    // Check for memory node
> +    //
> +    Type = fdt_getprop (DeviceTreeBase, Node, "device_type", &Len);
> +    if (Type && AsciiStrnCmp (Type, "memory", Len) == 0) {
> +      //
> +      // Get the 'reg' property of this node. For now, we will assume
> +      // two 8 byte quantities for base and size, respectively.
> +      //
> +      RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
> +      if (RegProp != 0 && Len == (2 * sizeof (UINT64))) {
> +
> +        CurBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
> +        CurSize = fdt64_to_cpu (ReadUnaligned64 (RegProp + 1));
> +
> +        DEBUG ((DEBUG_INFO, "%a: System RAM @ 0x%lx - 0x%lx\n",
> +          __FUNCTION__, CurBase, CurBase + CurSize - 1));
> +
> +        if (NewBase > CurBase || NewBase == 0) {
> +          NewBase = CurBase;
> +          NewSize = CurSize;
> +        }
> +      } else {
> +        DEBUG ((DEBUG_ERROR, "%a: Failed to parse FDT memory node\n",
> +          __FUNCTION__));
> +      }
> +    }
> +  }
> +
> +  //
> +  // Make sure the start of DRAM matches our expectation
> +  //
> +  ASSERT (FixedPcdGet64 (PcdSystemMemoryBase) == NewBase);
> +  PcdStatus = PcdSet64S (PcdSystemMemorySize, NewSize);
> +  ASSERT_RETURN_ERROR (PcdStatus);
> +
> +  //
> +  // We need to make sure that the machine we are running on has at least
> +  // 128 MB of memory configured, and is currently executing this binary from
> +  // NOR flash. This prevents a device tree image in DRAM from getting
> +  // clobbered when our caller installs permanent PEI RAM, before we have a
> +  // chance of marking its location as reserved or copy it to a freshly
> +  // allocated block in the permanent PEI RAM in the platform PEIM.
> +  //
> +  ASSERT (NewSize >= SIZE_128MB);
> +  ASSERT (
> +    (((UINT64)PcdGet64 (PcdFdBaseAddress) +
> +      (UINT64)PcdGet32 (PcdFdSize)) <= NewBase) ||
> +    ((UINT64)PcdGet64 (PcdFdBaseAddress) >= (NewBase + NewSize)));
> +
> +  return RETURN_SUCCESS;
> +}
>