[PATCH v2 0/2] CloudHv/arm: Add specific mem info lib

[PATCH v2 0/2] CloudHv/arm: Add specific mem info lib

[Jianyong Wu]

Currently, the meminfo lib for CloudHv/arm is reusing QEMU's. As the memory
layout for cloud hypervisor is changed, it needs a new meminfo lib.

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

Jianyong Wu (2):
  CloudHv/arm: add PeiMemInfoLib
  CloudHv/arm: switch PeiMemLib to its own

 ArmVirtPkg/ArmVirtCloudHv.dsc                 |   2 +-
 .../CloudHvVirtMemInfoLib.c                   | 230 ++++++++++++++++++
 .../CloudHvVirtMemInfoLib.h                   |  42 ++++
 .../CloudHvVirtMemInfoPeiLib.inf              |  46 ++++
 4 files changed, 319 insertions(+), 1 deletion(-)
 create mode 100644 ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
 create mode 100644 ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.h
 create mode 100644 ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf

-- 
2.17.1

[PATCH v2 1/2] CloudHv/arm: add PeiMemInfoLib

[Jianyong Wu]

Memory layout in CLoud Hypervisor for arm is changed and is different
with Qemu, thus we should build its own PeiMemInfoLib.
The main change in the memory layout is that normal ram may not contiguous
under 4G. The top 64M under 4G is reserved for 32bit device.

What this patch does:
1. get all of the memory node from DT;
2. Add all of the memory nodes to Hob;
3. Init page table for each memory node;

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
---
 .../CloudHvVirtMemInfoLib.c                   | 230 ++++++++++++++++++
 .../CloudHvVirtMemInfoLib.h                   |  42 ++++
 .../CloudHvVirtMemInfoPeiLib.inf              |  46 ++++
 3 files changed, 318 insertions(+)
 create mode 100644 ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
 create mode 100644 ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.h
 create mode 100644 ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf

diff --git a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
new file mode 100644
index 0000000000..d9c434754e
--- /dev/null
+++ b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
@@ -0,0 +1,230 @@
+/** @file
+
+  Copyright (c) 2022, Arm Limited. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <PiPei.h>
+
+#include <Base.h>
+#include <libfdt.h>
+#include <Library/ArmLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
+
+#include <Library/PrePiLib.h>
+
+#include "CloudHvVirtMemInfoLib.h"
+
+CLOUDHV_MEM_NODE_INFO CloudHvMemNode[CLOUDHV_MAX_MEM_NODE_NUM];
+
+RETURN_STATUS
+EFIAPI
+CloudHvVirtMemInfoPeiLibConstructor (
+  VOID
+  )
+{
+  VOID           *DeviceTreeBase;
+  EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
+  INT32          Node, Prev;
+  UINT64         FirMemNodeBase, FirMemNodeSize;
+  UINT64         CurBase, MemBase;
+  UINT64         CurSize;
+  CONST CHAR8    *Type;
+  INT32          Len;
+  CONST UINT64   *RegProp;
+  RETURN_STATUS  PcdStatus;
+  UINT8          Index;
+
+  ZeroMem (CloudHvMemNode, sizeof(CloudHvMemNode));
+
+  FirMemNodeBase = 0;
+  FirMemNodeSize = 0;
+  Index = 0;
+  MemBase = FixedPcdGet64 (PcdSystemMemoryBase);
+  ResourceAttributes = (
+                        EFI_RESOURCE_ATTRIBUTE_PRESENT |
+                        EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
+                        EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
+                        EFI_RESOURCE_ATTRIBUTE_TESTED
+                        );
+  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
+  if (DeviceTreeBase == NULL) {
+    return EFI_NOT_FOUND;
+  }
+
+  //
+  // Make sure we have a valid device tree blob
+  //
+  if (fdt_check_header (DeviceTreeBase) != 0) {
+    return EFI_NOT_FOUND;
+  }
+
+  //
+  // 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
+          ));
+
+        // We should build Hob seperately for the memory node except the first one
+        if (CurBase != MemBase) {
+          BuildResourceDescriptorHob (
+            EFI_RESOURCE_SYSTEM_MEMORY,
+            ResourceAttributes,
+            CurBase,
+            CurSize
+            );
+        } else {
+          FirMemNodeBase = CurBase;
+          FirMemNodeSize = CurSize;
+        }
+
+        CloudHvMemNode[Index].Base = CurBase;
+        CloudHvMemNode[Index].Size = CurSize;
+        Index++;
+
+        if (Index >= CLOUDHV_MAX_MEM_NODE_NUM) {
+          DEBUG ((
+            DEBUG_WARN,
+            "%a: memory node larger than %d will not be included into Memory System\n",
+            __FUNCTION__,
+            CLOUDHV_MAX_MEM_NODE_NUM
+            ));
+          break;
+        }
+      } else {
+        DEBUG ((
+          DEBUG_ERROR,
+          "%a: Failed to parse FDT memory node\n",
+          __FUNCTION__
+          ));
+      }
+    }
+  }
+
+  //
+  // Make sure the start of DRAM matches our expectation
+  //
+  if (FixedPcdGet64 (PcdSystemMemoryBase) != FirMemNodeBase) {
+    return EFI_NOT_FOUND;
+  }
+  PcdStatus = PcdSet64S (PcdSystemMemorySize, FirMemNodeSize);
+  ASSERT_RETURN_ERROR (PcdStatus);
+  ASSERT (
+    (((UINT64)PcdGet64 (PcdFdBaseAddress) +
+      (UINT64)PcdGet32 (PcdFdSize)) <= FirMemNodeBase) ||
+    ((UINT64)PcdGet64 (PcdFdBaseAddress) >= (FirMemNodeBase + FirMemNodeSize))
+    );
+
+  return RETURN_SUCCESS;
+}
+
+/**
+  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;
+  UINT8 Index = 0, MemNodeIndex = 0;
+
+  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
+  while ((MemNodeIndex < CLOUDHV_MAX_MEM_NODE_NUM) && (CloudHvMemNode[MemNodeIndex].Size != 0)) {
+    VirtualMemoryTable[Index].PhysicalBase = CloudHvMemNode[MemNodeIndex].Base;
+    VirtualMemoryTable[Index].VirtualBase  = CloudHvMemNode[MemNodeIndex].Base;
+    VirtualMemoryTable[Index].Length       = CloudHvMemNode[MemNodeIndex].Size;
+    VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
+
+    DEBUG ((
+      DEBUG_INFO,
+      "%a: Dumping System DRAM Memory Node%d Map:\n"
+      "\tPhysicalBase: 0x%lX\n"
+      "\tVirtualBase: 0x%lX\n"
+      "\tLength: 0x%lX\n",
+      __FUNCTION__,
+      MemNodeIndex,
+      VirtualMemoryTable[Index].PhysicalBase,
+      VirtualMemoryTable[Index].VirtualBase,
+      VirtualMemoryTable[Index].Length
+      ));
+    Index++;
+    MemNodeIndex++;
+  }
+  // Memory mapped peripherals (UART, RTC, GIC, virtio-mmio, etc)
+  VirtualMemoryTable[Index].PhysicalBase = MACH_VIRT_PERIPH_BASE;
+  VirtualMemoryTable[Index].VirtualBase  = MACH_VIRT_PERIPH_BASE;
+  VirtualMemoryTable[Index].Length       = MACH_VIRT_PERIPH_SIZE;
+  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+  Index++;
+
+  // Map the FV region as normal executable memory
+  VirtualMemoryTable[Index].PhysicalBase = PcdGet64 (PcdFvBaseAddress);
+  VirtualMemoryTable[Index].VirtualBase  = VirtualMemoryTable[Index].PhysicalBase;
+  VirtualMemoryTable[Index].Length       = FixedPcdGet32 (PcdFvSize);
+  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
+  Index++;
+
+  // Memory mapped for 32bit device (like TPM)
+  VirtualMemoryTable[Index].PhysicalBase = TOP_32BIT_DEVICE_BASE;
+  VirtualMemoryTable[Index].VirtualBase  = TOP_32BIT_DEVICE_BASE;
+  VirtualMemoryTable[Index].Length       = TOP_32BIT_DEVICE_SIZE;
+  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+  Index++;
+
+  // End of Table
+  ZeroMem (&VirtualMemoryTable[Index], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
+
+  *VirtualMemoryMap = VirtualMemoryTable;
+}
diff --git a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.h b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.h
new file mode 100644
index 0000000000..e624373472
--- /dev/null
+++ b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.h
@@ -0,0 +1,42 @@
+/** @file
+
+  Copyright (c) 2022, Arm Limited. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _CLOUDHV_VIRT_MEM_INFO_LIB_H_
+#define _CLOUDHV_VIRT_MEM_INFO_LIB_H_
+
+//
+// Cloud Hypervisor may have more than one memory nodes. Even there is no limit for that,
+// I think 10 is enough in general.
+//
+#define CLOUDHV_MAX_MEM_NODE_NUM 10
+
+// Record memory node info (base address and size)
+typedef struct {
+  UINT64 Base;
+  UINT64 Size;
+} CLOUDHV_MEM_NODE_INFO;
+
+// Number of Virtual Memory Map Descriptors
+#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  (4 + CLOUDHV_MAX_MEM_NODE_NUM)
+
+//
+// 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  0x00400000
+#define MACH_VIRT_PERIPH_SIZE  0x0FC00000
+
+//
+// The top of the 64M memory region under 4GB reserved for device
+//
+#define TOP_32BIT_DEVICE_BASE  0xFC000000
+#define TOP_32BIT_DEVICE_SIZE  0x04000000
+
+#endif // _CLOUDHV_VIRT_MEM_INFO_LIB_H_
diff --git a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf
new file mode 100644
index 0000000000..30626776ae
--- /dev/null
+++ b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf
@@ -0,0 +1,46 @@
+/** @file
+
+  Copyright (c) 2022, Arm Limited. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+[Defines]
+  INF_VERSION                    = 0x0001001B
+  BASE_NAME                      = CloudHvVirtMemInfoPeiLib
+  FILE_GUID                      = c7ada233-d35b-49c3-aa51-e2b5cd80c910
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = ArmVirtMemInfoLib|PEIM
+  CONSTRUCTOR                    = CloudHvVirtMemInfoPeiLibConstructor
+
+[Sources]
+  CloudHvVirtMemInfoLib.c
+  CloudHvVirtMemInfoLib.h
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmVirtPkg/ArmVirtPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  ArmLib
+  BaseMemoryLib
+  DebugLib
+  FdtLib
+  MemoryAllocationLib
+  PcdLib
+
+[Pcd]
+  gArmTokenSpaceGuid.PcdFdBaseAddress
+  gArmTokenSpaceGuid.PcdFvBaseAddress
+  gArmTokenSpaceGuid.PcdSystemMemoryBase
+  gArmTokenSpaceGuid.PcdSystemMemorySize
+
+[FixedPcd]
+  gArmTokenSpaceGuid.PcdFdSize
+  gArmTokenSpaceGuid.PcdFvSize
+  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
-- 
2.17.1

Re: [PATCH v2 1/2] CloudHv/arm: add PeiMemInfoLib

[Sami Mujawar]

Hi Jianyong,

Thank you for the updated patch.

I have one comment marked inline as [SAMI]. Other than that this patch 
looks good to me.

With that fixed.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 19/08/2022 09:09 am, Jianyong Wu wrote:
> Memory layout in CLoud Hypervisor for arm is changed and is different
> with Qemu, thus we should build its own PeiMemInfoLib.
> The main change in the memory layout is that normal ram may not contiguous
> under 4G. The top 64M under 4G is reserved for 32bit device.
>
> What this patch does:
> 1. get all of the memory node from DT;
> 2. Add all of the memory nodes to Hob;
> 3. Init page table for each memory node;
>
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> ---
>   .../CloudHvVirtMemInfoLib.c                   | 230 ++++++++++++++++++
>   .../CloudHvVirtMemInfoLib.h                   |  42 ++++
>   .../CloudHvVirtMemInfoPeiLib.inf              |  46 ++++
>   3 files changed, 318 insertions(+)
>   create mode 100644 ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
>   create mode 100644 ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.h
>   create mode 100644 ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf
>
> diff --git a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
> new file mode 100644
> index 0000000000..d9c434754e
> --- /dev/null
> +++ b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
> @@ -0,0 +1,230 @@
> +/** @file
> +
> +  Copyright (c) 2022, Arm Limited. All rights reserved.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <PiPei.h>
> +
> +#include <Base.h>
> +#include <libfdt.h>
> +#include <Library/ArmLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PcdLib.h>
> +
> +#include <Library/PrePiLib.h>
> +
> +#include "CloudHvVirtMemInfoLib.h"
> +
> +CLOUDHV_MEM_NODE_INFO CloudHvMemNode[CLOUDHV_MAX_MEM_NODE_NUM];
> +
> +RETURN_STATUS
> +EFIAPI
> +CloudHvVirtMemInfoPeiLibConstructor (
> +  VOID
> +  )
> +{
> +  VOID           *DeviceTreeBase;
> +  EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> +  INT32          Node, Prev;
> +  UINT64         FirMemNodeBase, FirMemNodeSize;
> +  UINT64         CurBase, MemBase;
> +  UINT64         CurSize;
> +  CONST CHAR8    *Type;
> +  INT32          Len;
> +  CONST UINT64   *RegProp;
> +  RETURN_STATUS  PcdStatus;
> +  UINT8          Index;
> +
> +  ZeroMem (CloudHvMemNode, sizeof(CloudHvMemNode));
> +
> +  FirMemNodeBase = 0;
> +  FirMemNodeSize = 0;
> +  Index = 0;
> +  MemBase = FixedPcdGet64 (PcdSystemMemoryBase);
> +  ResourceAttributes = (
> +                        EFI_RESOURCE_ATTRIBUTE_PRESENT |
> +                        EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> +                        EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
> +                        EFI_RESOURCE_ATTRIBUTE_TESTED
> +                        );
> +  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
> +  if (DeviceTreeBase == NULL) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  //
> +  // Make sure we have a valid device tree blob
> +  //
> +  if (fdt_check_header (DeviceTreeBase) != 0) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  //
> +  // 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)) {

[SAMI] Apologies I missed this in my earlier review. The above check 
should be

  + if ((Type !=0) && (AsciiStrnCmp (Type, "memory", Len) == 0)) {

as Type is not boolean.

[/SAMI]

> +      //
> +      // 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
> +          ));
> +
> +        // We should build Hob seperately for the memory node except the first one
> +        if (CurBase != MemBase) {
> +          BuildResourceDescriptorHob (
> +            EFI_RESOURCE_SYSTEM_MEMORY,
> +            ResourceAttributes,
> +            CurBase,
> +            CurSize
> +            );
> +        } else {
> +          FirMemNodeBase = CurBase;
> +          FirMemNodeSize = CurSize;
> +        }
> +
> +        CloudHvMemNode[Index].Base = CurBase;
> +        CloudHvMemNode[Index].Size = CurSize;
> +        Index++;
> +
> +        if (Index >= CLOUDHV_MAX_MEM_NODE_NUM) {
> +          DEBUG ((
> +            DEBUG_WARN,
> +            "%a: memory node larger than %d will not be included into Memory System\n",
> +            __FUNCTION__,
> +            CLOUDHV_MAX_MEM_NODE_NUM
> +            ));
> +          break;
> +        }
> +      } else {
> +        DEBUG ((
> +          DEBUG_ERROR,
> +          "%a: Failed to parse FDT memory node\n",
> +          __FUNCTION__
> +          ));
> +      }
> +    }
> +  }
> +
> +  //
> +  // Make sure the start of DRAM matches our expectation
> +  //
> +  if (FixedPcdGet64 (PcdSystemMemoryBase) != FirMemNodeBase) {
> +    return EFI_NOT_FOUND;
> +  }
> +  PcdStatus = PcdSet64S (PcdSystemMemorySize, FirMemNodeSize);
> +  ASSERT_RETURN_ERROR (PcdStatus);
> +  ASSERT (
> +    (((UINT64)PcdGet64 (PcdFdBaseAddress) +
> +      (UINT64)PcdGet32 (PcdFdSize)) <= FirMemNodeBase) ||
> +    ((UINT64)PcdGet64 (PcdFdBaseAddress) >= (FirMemNodeBase + FirMemNodeSize))
> +    );
> +
> +  return RETURN_SUCCESS;
> +}
> +
> +/**
> +  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;
> +  UINT8 Index = 0, MemNodeIndex = 0;
> +
> +  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
> +  while ((MemNodeIndex < CLOUDHV_MAX_MEM_NODE_NUM) && (CloudHvMemNode[MemNodeIndex].Size != 0)) {
> +    VirtualMemoryTable[Index].PhysicalBase = CloudHvMemNode[MemNodeIndex].Base;
> +    VirtualMemoryTable[Index].VirtualBase  = CloudHvMemNode[MemNodeIndex].Base;
> +    VirtualMemoryTable[Index].Length       = CloudHvMemNode[MemNodeIndex].Size;
> +    VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> +
> +    DEBUG ((
> +      DEBUG_INFO,
> +      "%a: Dumping System DRAM Memory Node%d Map:\n"
> +      "\tPhysicalBase: 0x%lX\n"
> +      "\tVirtualBase: 0x%lX\n"
> +      "\tLength: 0x%lX\n",
> +      __FUNCTION__,
> +      MemNodeIndex,
> +      VirtualMemoryTable[Index].PhysicalBase,
> +      VirtualMemoryTable[Index].VirtualBase,
> +      VirtualMemoryTable[Index].Length
> +      ));
> +    Index++;
> +    MemNodeIndex++;
> +  }
> +  // Memory mapped peripherals (UART, RTC, GIC, virtio-mmio, etc)
> +  VirtualMemoryTable[Index].PhysicalBase = MACH_VIRT_PERIPH_BASE;
> +  VirtualMemoryTable[Index].VirtualBase  = MACH_VIRT_PERIPH_BASE;
> +  VirtualMemoryTable[Index].Length       = MACH_VIRT_PERIPH_SIZE;
> +  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +  Index++;
> +
> +  // Map the FV region as normal executable memory
> +  VirtualMemoryTable[Index].PhysicalBase = PcdGet64 (PcdFvBaseAddress);
> +  VirtualMemoryTable[Index].VirtualBase  = VirtualMemoryTable[Index].PhysicalBase;
> +  VirtualMemoryTable[Index].Length       = FixedPcdGet32 (PcdFvSize);
> +  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> +  Index++;
> +
> +  // Memory mapped for 32bit device (like TPM)
> +  VirtualMemoryTable[Index].PhysicalBase = TOP_32BIT_DEVICE_BASE;
> +  VirtualMemoryTable[Index].VirtualBase  = TOP_32BIT_DEVICE_BASE;
> +  VirtualMemoryTable[Index].Length       = TOP_32BIT_DEVICE_SIZE;
> +  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +  Index++;
> +
> +  // End of Table
> +  ZeroMem (&VirtualMemoryTable[Index], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
> +
> +  *VirtualMemoryMap = VirtualMemoryTable;
> +}
> diff --git a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.h b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.h
> new file mode 100644
> index 0000000000..e624373472
> --- /dev/null
> +++ b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.h
> @@ -0,0 +1,42 @@
> +/** @file
> +
> +  Copyright (c) 2022, Arm Limited. All rights reserved.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef _CLOUDHV_VIRT_MEM_INFO_LIB_H_
> +#define _CLOUDHV_VIRT_MEM_INFO_LIB_H_
> +
> +//
> +// Cloud Hypervisor may have more than one memory nodes. Even there is no limit for that,
> +// I think 10 is enough in general.
> +//
> +#define CLOUDHV_MAX_MEM_NODE_NUM 10
> +
> +// Record memory node info (base address and size)
> +typedef struct {
> +  UINT64 Base;
> +  UINT64 Size;
> +} CLOUDHV_MEM_NODE_INFO;
> +
> +// Number of Virtual Memory Map Descriptors
> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  (4 + CLOUDHV_MAX_MEM_NODE_NUM)
> +
> +//
> +// 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  0x00400000
> +#define MACH_VIRT_PERIPH_SIZE  0x0FC00000
> +
> +//
> +// The top of the 64M memory region under 4GB reserved for device
> +//
> +#define TOP_32BIT_DEVICE_BASE  0xFC000000
> +#define TOP_32BIT_DEVICE_SIZE  0x04000000
> +
> +#endif // _CLOUDHV_VIRT_MEM_INFO_LIB_H_
> diff --git a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf
> new file mode 100644
> index 0000000000..30626776ae
> --- /dev/null
> +++ b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf
> @@ -0,0 +1,46 @@
> +/** @file
> +
> +  Copyright (c) 2022, Arm Limited. All rights reserved.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001B
> +  BASE_NAME                      = CloudHvVirtMemInfoPeiLib
> +  FILE_GUID                      = c7ada233-d35b-49c3-aa51-e2b5cd80c910
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = ArmVirtMemInfoLib|PEIM
> +  CONSTRUCTOR                    = CloudHvVirtMemInfoPeiLibConstructor
> +
> +[Sources]
> +  CloudHvVirtMemInfoLib.c
> +  CloudHvVirtMemInfoLib.h
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  ArmVirtPkg/ArmVirtPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  ArmLib
> +  BaseMemoryLib
> +  DebugLib
> +  FdtLib
> +  MemoryAllocationLib
> +  PcdLib
> +
> +[Pcd]
> +  gArmTokenSpaceGuid.PcdFdBaseAddress
> +  gArmTokenSpaceGuid.PcdFvBaseAddress
> +  gArmTokenSpaceGuid.PcdSystemMemoryBase
> +  gArmTokenSpaceGuid.PcdSystemMemorySize
> +
> +[FixedPcd]
> +  gArmTokenSpaceGuid.PcdFdSize
> +  gArmTokenSpaceGuid.PcdFvSize
> +  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress

Re: [PATCH v2 1/2] CloudHv/arm: add PeiMemInfoLib

[Jianyong Wu]

Thanks Sami, I will address all the comments in this patch in next version.

> -----Original Message-----
> From: Sami Mujawar <Sami.Mujawar@arm.com>
> Sent: Thursday, September 1, 2022 11:43 PM
> To: Jianyong Wu <Jianyong.Wu@arm.com>; devel@edk2.groups.io
> Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>; nd
> <nd@arm.com>
> Subject: Re: [PATCH v2 1/2] CloudHv/arm: add PeiMemInfoLib
> 
> Hi Jianyong,
> 
> Thank you for the updated patch.
> 
> I have one comment marked inline as [SAMI]. Other than that this patch
> looks good to me.
> 
> With that fixed.
> 
> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> 
> Regards,
> 
> Sami Mujawar
> 
> On 19/08/2022 09:09 am, Jianyong Wu wrote:
> > Memory layout in CLoud Hypervisor for arm is changed and is different
> > with Qemu, thus we should build its own PeiMemInfoLib.
> > The main change in the memory layout is that normal ram may not
> > contiguous under 4G. The top 64M under 4G is reserved for 32bit device.
> >
> > What this patch does:
> > 1. get all of the memory node from DT; 2. Add all of the memory nodes
> > to Hob; 3. Init page table for each memory node;
> >
> > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> > ---
> >   .../CloudHvVirtMemInfoLib.c                   | 230 ++++++++++++++++++
> >   .../CloudHvVirtMemInfoLib.h                   |  42 ++++
> >   .../CloudHvVirtMemInfoPeiLib.inf              |  46 ++++
> >   3 files changed, 318 insertions(+)
> >   create mode 100644
> ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
> >   create mode 100644
> ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.h
> >   create mode 100644
> >
> ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf
> >
> > diff --git
> > a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
> > b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
> > new file mode 100644
> > index 0000000000..d9c434754e
> > --- /dev/null
> > +++
> b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
> > @@ -0,0 +1,230 @@
> > +/** @file
> > +
> > +  Copyright (c) 2022, Arm Limited. All rights reserved.
> > +
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#include <PiPei.h>
> > +
> > +#include <Base.h>
> > +#include <libfdt.h>
> > +#include <Library/ArmLib.h>
> > +#include <Library/BaseMemoryLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/MemoryAllocationLib.h> #include <Library/PcdLib.h>
> > +
> > +#include <Library/PrePiLib.h>
> > +
> > +#include "CloudHvVirtMemInfoLib.h"
> > +
> > +CLOUDHV_MEM_NODE_INFO
> CloudHvMemNode[CLOUDHV_MAX_MEM_NODE_NUM];
> > +
> > +RETURN_STATUS
> > +EFIAPI
> > +CloudHvVirtMemInfoPeiLibConstructor (
> > +  VOID
> > +  )
> > +{
> > +  VOID           *DeviceTreeBase;
> > +  EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> > +  INT32          Node, Prev;
> > +  UINT64         FirMemNodeBase, FirMemNodeSize;
> > +  UINT64         CurBase, MemBase;
> > +  UINT64         CurSize;
> > +  CONST CHAR8    *Type;
> > +  INT32          Len;
> > +  CONST UINT64   *RegProp;
> > +  RETURN_STATUS  PcdStatus;
> > +  UINT8          Index;
> > +
> > +  ZeroMem (CloudHvMemNode, sizeof(CloudHvMemNode));
> > +
> > +  FirMemNodeBase = 0;
> > +  FirMemNodeSize = 0;
> > +  Index = 0;
> > +  MemBase = FixedPcdGet64 (PcdSystemMemoryBase);
> ResourceAttributes
> > + = (
> > +                        EFI_RESOURCE_ATTRIBUTE_PRESENT |
> > +                        EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> > +                        EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
> > +                        EFI_RESOURCE_ATTRIBUTE_TESTED
> > +                        );
> > +  DeviceTreeBase = (VOID *)(UINTN)PcdGet64
> > + (PcdDeviceTreeInitialBaseAddress);
> > +  if (DeviceTreeBase == NULL) {
> > +    return EFI_NOT_FOUND;
> > +  }
> > +
> > +  //
> > +  // Make sure we have a valid device tree blob  //  if
> > + (fdt_check_header (DeviceTreeBase) != 0) {
> > +    return EFI_NOT_FOUND;
> > +  }
> > +
> > +  //
> > +  // 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)) {
> 
> [SAMI] Apologies I missed this in my earlier review. The above check should
> be
> 
>   + if ((Type !=0) && (AsciiStrnCmp (Type, "memory", Len) == 0)) {
> 
> as Type is not boolean.
> 
> [/SAMI]
> 
> > +      //
> > +      // 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
> > +          ));
> > +
> > +        // We should build Hob seperately for the memory node except the
> first one
> > +        if (CurBase != MemBase) {
> > +          BuildResourceDescriptorHob (
> > +            EFI_RESOURCE_SYSTEM_MEMORY,
> > +            ResourceAttributes,
> > +            CurBase,
> > +            CurSize
> > +            );
> > +        } else {
> > +          FirMemNodeBase = CurBase;
> > +          FirMemNodeSize = CurSize;
> > +        }
> > +
> > +        CloudHvMemNode[Index].Base = CurBase;
> > +        CloudHvMemNode[Index].Size = CurSize;
> > +        Index++;
> > +
> > +        if (Index >= CLOUDHV_MAX_MEM_NODE_NUM) {
> > +          DEBUG ((
> > +            DEBUG_WARN,
> > +            "%a: memory node larger than %d will not be included into Memory
> System\n",
> > +            __FUNCTION__,
> > +            CLOUDHV_MAX_MEM_NODE_NUM
> > +            ));
> > +          break;
> > +        }
> > +      } else {
> > +        DEBUG ((
> > +          DEBUG_ERROR,
> > +          "%a: Failed to parse FDT memory node\n",
> > +          __FUNCTION__
> > +          ));
> > +      }
> > +    }
> > +  }
> > +
> > +  //
> > +  // Make sure the start of DRAM matches our expectation  //  if
> > + (FixedPcdGet64 (PcdSystemMemoryBase) != FirMemNodeBase) {
> > +    return EFI_NOT_FOUND;
> > +  }
> > +  PcdStatus = PcdSet64S (PcdSystemMemorySize, FirMemNodeSize);
> > + ASSERT_RETURN_ERROR (PcdStatus);  ASSERT (
> > +    (((UINT64)PcdGet64 (PcdFdBaseAddress) +
> > +      (UINT64)PcdGet32 (PcdFdSize)) <= FirMemNodeBase) ||
> > +    ((UINT64)PcdGet64 (PcdFdBaseAddress) >= (FirMemNodeBase +
> FirMemNodeSize))
> > +    );
> > +
> > +  return RETURN_SUCCESS;
> > +}
> > +
> > +/**
> > +  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;
> > +  UINT8 Index = 0, MemNodeIndex = 0;
> > +
> > +  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
> > +  while ((MemNodeIndex < CLOUDHV_MAX_MEM_NODE_NUM) &&
> (CloudHvMemNode[MemNodeIndex].Size != 0)) {
> > +    VirtualMemoryTable[Index].PhysicalBase =
> CloudHvMemNode[MemNodeIndex].Base;
> > +    VirtualMemoryTable[Index].VirtualBase  =
> CloudHvMemNode[MemNodeIndex].Base;
> > +    VirtualMemoryTable[Index].Length       =
> CloudHvMemNode[MemNodeIndex].Size;
> > +    VirtualMemoryTable[Index].Attributes   =
> ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> > +
> > +    DEBUG ((
> > +      DEBUG_INFO,
> > +      "%a: Dumping System DRAM Memory Node%d Map:\n"
> > +      "\tPhysicalBase: 0x%lX\n"
> > +      "\tVirtualBase: 0x%lX\n"
> > +      "\tLength: 0x%lX\n",
> > +      __FUNCTION__,
> > +      MemNodeIndex,
> > +      VirtualMemoryTable[Index].PhysicalBase,
> > +      VirtualMemoryTable[Index].VirtualBase,
> > +      VirtualMemoryTable[Index].Length
> > +      ));
> > +    Index++;
> > +    MemNodeIndex++;
> > +  }
> > +  // Memory mapped peripherals (UART, RTC, GIC, virtio-mmio, etc)
> > + VirtualMemoryTable[Index].PhysicalBase = MACH_VIRT_PERIPH_BASE;
> > + VirtualMemoryTable[Index].VirtualBase  = MACH_VIRT_PERIPH_BASE;
> > +  VirtualMemoryTable[Index].Length       = MACH_VIRT_PERIPH_SIZE;
> > +  VirtualMemoryTable[Index].Attributes   =
> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > +  Index++;
> > +
> > +  // Map the FV region as normal executable memory
> > + VirtualMemoryTable[Index].PhysicalBase = PcdGet64
> > + (PcdFvBaseAddress);  VirtualMemoryTable[Index].VirtualBase  =
> VirtualMemoryTable[Index].PhysicalBase;
> > +  VirtualMemoryTable[Index].Length       = FixedPcdGet32 (PcdFvSize);
> > +  VirtualMemoryTable[Index].Attributes   =
> ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> > +  Index++;
> > +
> > +  // Memory mapped for 32bit device (like TPM)
> > + VirtualMemoryTable[Index].PhysicalBase = TOP_32BIT_DEVICE_BASE;
> > + VirtualMemoryTable[Index].VirtualBase  = TOP_32BIT_DEVICE_BASE;
> > +  VirtualMemoryTable[Index].Length       = TOP_32BIT_DEVICE_SIZE;
> > +  VirtualMemoryTable[Index].Attributes   =
> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > +  Index++;
> > +
> > +  // End of Table
> > +  ZeroMem (&VirtualMemoryTable[Index], sizeof
> > + (ARM_MEMORY_REGION_DESCRIPTOR));
> > +
> > +  *VirtualMemoryMap = VirtualMemoryTable; }
> > diff --git
> > a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.h
> > b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.h
> > new file mode 100644
> > index 0000000000..e624373472
> > --- /dev/null
> > +++
> b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.h
> > @@ -0,0 +1,42 @@
> > +/** @file
> > +
> > +  Copyright (c) 2022, Arm Limited. All rights reserved.
> > +
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#ifndef _CLOUDHV_VIRT_MEM_INFO_LIB_H_ #define
> > +_CLOUDHV_VIRT_MEM_INFO_LIB_H_
> > +
> > +//
> > +// Cloud Hypervisor may have more than one memory nodes. Even there
> > +is no limit for that, // I think 10 is enough in general.
> > +//
> > +#define CLOUDHV_MAX_MEM_NODE_NUM 10
> > +
> > +// Record memory node info (base address and size) typedef struct {
> > +  UINT64 Base;
> > +  UINT64 Size;
> > +} CLOUDHV_MEM_NODE_INFO;
> > +
> > +// Number of Virtual Memory Map Descriptors #define
> > +MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  (4 +
> CLOUDHV_MAX_MEM_NODE_NUM)
> > +
> > +//
> > +// 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  0x00400000 #define
> > +MACH_VIRT_PERIPH_SIZE  0x0FC00000
> > +
> > +//
> > +// The top of the 64M memory region under 4GB reserved for device //
> > +#define TOP_32BIT_DEVICE_BASE  0xFC000000 #define
> > +TOP_32BIT_DEVICE_SIZE  0x04000000
> > +
> > +#endif // _CLOUDHV_VIRT_MEM_INFO_LIB_H_
> > diff --git
> >
> a/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.i
> n
> > f
> >
> b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.i
> n
> > f
> > new file mode 100644
> > index 0000000000..30626776ae
> > --- /dev/null
> > +++
> b/ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLi
> > +++ b.inf
> > @@ -0,0 +1,46 @@
> > +/** @file
> > +
> > +  Copyright (c) 2022, Arm Limited. All rights reserved.
> > +
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +[Defines]
> > +  INF_VERSION                    = 0x0001001B
> > +  BASE_NAME                      = CloudHvVirtMemInfoPeiLib
> > +  FILE_GUID                      = c7ada233-d35b-49c3-aa51-e2b5cd80c910
> > +  MODULE_TYPE                    = BASE
> > +  VERSION_STRING                 = 1.0
> > +  LIBRARY_CLASS                  = ArmVirtMemInfoLib|PEIM
> > +  CONSTRUCTOR                    = CloudHvVirtMemInfoPeiLibConstructor
> > +
> > +[Sources]
> > +  CloudHvVirtMemInfoLib.c
> > +  CloudHvVirtMemInfoLib.h
> > +
> > +[Packages]
> > +  ArmPkg/ArmPkg.dec
> > +  ArmVirtPkg/ArmVirtPkg.dec
> > +  EmbeddedPkg/EmbeddedPkg.dec
> > +  MdeModulePkg/MdeModulePkg.dec
> > +  MdePkg/MdePkg.dec
> > +
> > +[LibraryClasses]
> > +  ArmLib
> > +  BaseMemoryLib
> > +  DebugLib
> > +  FdtLib
> > +  MemoryAllocationLib
> > +  PcdLib
> > +
> > +[Pcd]
> > +  gArmTokenSpaceGuid.PcdFdBaseAddress
> > +  gArmTokenSpaceGuid.PcdFvBaseAddress
> > +  gArmTokenSpaceGuid.PcdSystemMemoryBase
> > +  gArmTokenSpaceGuid.PcdSystemMemorySize
> > +
> > +[FixedPcd]
> > +  gArmTokenSpaceGuid.PcdFdSize
> > +  gArmTokenSpaceGuid.PcdFvSize
> > +  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress

[PATCH 2/2] CloudHv/arm: switch PeiMemLib to its own

[Jianyong Wu]

As Cloud Hypervisor has its own PeiMemLib, change it in dsc file
accordingly.

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---
 ArmVirtPkg/ArmVirtCloudHv.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc
index 7559386a1d..7ca7a391d9 100644
--- a/ArmVirtPkg/ArmVirtCloudHv.dsc
+++ b/ArmVirtPkg/ArmVirtCloudHv.dsc
@@ -60,7 +60,7 @@
 !include MdePkg/MdeLibs.dsc.inc
 
 [LibraryClasses.common.PEIM]
-  ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
+  ArmVirtMemInfoLib|ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf
 
 [LibraryClasses.common.DXE_DRIVER]
   ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
-- 
2.17.1

Re: [PATCH 2/2] CloudHv/arm: switch PeiMemLib to its own

[Sami Mujawar]

Hi Jianyong,

Please prefix the patch series version for this patch.

Regards,

Sami Mujawar

On 19/08/2022 09:09 am, Jianyong Wu wrote:
> As Cloud Hypervisor has its own PeiMemLib, change it in dsc file
> accordingly.
>
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>   ArmVirtPkg/ArmVirtCloudHv.dsc | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc
> index 7559386a1d..7ca7a391d9 100644
> --- a/ArmVirtPkg/ArmVirtCloudHv.dsc
> +++ b/ArmVirtPkg/ArmVirtCloudHv.dsc
> @@ -60,7 +60,7 @@
>   !include MdePkg/MdeLibs.dsc.inc
>   
>   [LibraryClasses.common.PEIM]
> -  ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
> +  ArmVirtMemInfoLib|ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf
>   
>   [LibraryClasses.common.DXE_DRIVER]
>     ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf

Re: [PATCH v2 0/2] CloudHv/arm: Add specific mem info lib

[Jianyong Wu]

Ping ...
Any comments?

Thanks
Jianyong
> -----Original Message-----
> From: Jianyong Wu <jianyong.wu@arm.com>
> Sent: Friday, August 19, 2022 4:09 PM
> To: devel@edk2.groups.io; Sami Mujawar <Sami.Mujawar@arm.com>
> Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>; Jianyong
> Wu <Jianyong.Wu@arm.com>
> Subject: [PATCH v2 0/2] CloudHv/arm: Add specific mem info lib
>
> Currently, the meminfo lib for CloudHv/arm is reusing QEMU's. As the
> memory layout for cloud hypervisor is changed, it needs a new meminfo lib.
>
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
>
> Jianyong Wu (2):
>   CloudHv/arm: add PeiMemInfoLib
>   CloudHv/arm: switch PeiMemLib to its own
>
>  ArmVirtPkg/ArmVirtCloudHv.dsc                 |   2 +-
>  .../CloudHvVirtMemInfoLib.c                   | 230 ++++++++++++++++++
>  .../CloudHvVirtMemInfoLib.h                   |  42 ++++
>  .../CloudHvVirtMemInfoPeiLib.inf              |  46 ++++
>  4 files changed, 319 insertions(+), 1 deletion(-)  create mode 100644
> ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
>  create mode 100644
> ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.h
>  create mode 100644
> ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf
>
> --
> 2.17.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[PATCH 0/2] CloudHv/arm: Add specific mem info lib

[Jianyong Wu]

Currently, the meminfo lib for CloudHv/arm is reusing QEMU's. As the memory
layout for cloud hypervisor is changed, it needs a new meminfo lib.

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

Jianyong Wu (2):
  CloudHv/arm: add PeiMemInfoLib
  CloudHv/arm: switch PeiMemLib to its own

 ArmVirtPkg/ArmVirtCloudHv.dsc                 |   2 +-
 .../CloudHvVirtMemInfoLib.c                   | 251 ++++++++++++++++++
 .../CloudHvVirtMemInfoPeiLib.inf              |  46 ++++
 3 files changed, 298 insertions(+), 1 deletion(-)
 create mode 100755 ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoLib.c
 create mode 100755 ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf

-- 
2.17.1

[PATCH 2/2] CloudHv/arm: switch PeiMemLib to its own

[Jianyong Wu]

As Cloud Hypervisor has its own PeiMemLib, change it in dsc file
accordingly.

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
---
 ArmVirtPkg/ArmVirtCloudHv.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc
index 7559386a1d..7ca7a391d9 100644
--- a/ArmVirtPkg/ArmVirtCloudHv.dsc
+++ b/ArmVirtPkg/ArmVirtCloudHv.dsc
@@ -60,7 +60,7 @@
 !include MdePkg/MdeLibs.dsc.inc
 
 [LibraryClasses.common.PEIM]
-  ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
+  ArmVirtMemInfoLib|ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf
 
 [LibraryClasses.common.DXE_DRIVER]
   ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
-- 
2.17.1

Re: [PATCH 2/2] CloudHv/arm: switch PeiMemLib to its own

[Sami Mujawar]

Hi Jianyong,

Thank you for this patch.

This change looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 29/07/2022 08:21 am, Jianyong Wu wrote:
> As Cloud Hypervisor has its own PeiMemLib, change it in dsc file
> accordingly.
>
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> ---
>   ArmVirtPkg/ArmVirtCloudHv.dsc | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc
> index 7559386a1d..7ca7a391d9 100644
> --- a/ArmVirtPkg/ArmVirtCloudHv.dsc
> +++ b/ArmVirtPkg/ArmVirtCloudHv.dsc
> @@ -60,7 +60,7 @@
>   !include MdePkg/MdeLibs.dsc.inc
>   
>   [LibraryClasses.common.PEIM]
> -  ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
> +  ArmVirtMemInfoLib|ArmVirtPkg/Library/CloudHvVirtMemInfoLib/CloudHvVirtMemInfoPeiLib.inf
>   
>   [LibraryClasses.common.DXE_DRIVER]
>     ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf