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

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

Sorry, just noticed:

On 05/19/21 08:07, Laszlo Ersek wrote:
> 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

(4) s/Clh/CloudHv/

Thanks
Laszlo

>> +  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;
>> +}
>>
>