public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v4 0/3] Enable Cloud Hypervisor support in edk2
@ 2021-07-05 10:06 Jianyong Wu
  2021-07-05 10:06 ` [PATCH v4 1/3] Acpi: reimplement PlatformHasAcpi for Cloud Hypervisor Jianyong Wu
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Jianyong Wu @ 2021-07-05 10:06 UTC (permalink / raw)
  To: devel; +Cc: lersek, sami.mujawar, ardb+tianocore, justin.he, jianyong.wu

Cloud Hypervisor is an open source Virtual Machine Monitor (VMM) that
runs on top of KVM. Cloud Hypervisor is implemented in Rust and is based
on the rust-vmm crates. See [1] to find more.

To support UEFI, Cloud Hypervisor is introduced here.
There are 2 parts to be considered to do this enablement, that is:
  1. specific ACPI service implementation compared with qemu, there is no
     device like Fw-cfg, so we have no elegant way to get the RSDP address.
     A specific ACPI implementation is introduced here.

  2. build configuration file for Cloud Hypervisor

Change log:

v3 to v4:
     (1) remove Tpm support in dsc file
     (2) refine Acpi table install code base on Sami's comments in v3

v2 to v3:
     (1) reuse qemu's memory initialization lib as they are in nearly the same
memory laout.
     (2) split Acpi implemetation into PlatformHasAcpi and
InstallAcpiTable.
     (3) remove lots of dependencies from qemu like "*Fwcfg*" lib.
     (4) lots of code cleanup work to let it more approach to edk2 code
style.

[1] https://github.com/cloud-hypervisor/cloud-hypervisor

Jianyong Wu (3):
  Acpi: reimplement PlatformHasAcpi for Cloud Hypervisor
  Acpi: Install Acpi tables for Cloud hypervisor
  ArmVirtCloudHv: support Cloud Hypervisor in edk2

 ArmVirtPkg/ArmVirtPkg.dec                     |   6 +
 ArmVirtPkg/ArmVirtCloudHv.dsc                 | 364 ++++++++++++++++++
 ArmVirtPkg/ArmVirtCloudHv.fdf                 | 258 +++++++++++++
 .../CloudHvAcpiPlatformDxe.inf                |  47 +++
 .../CloudHvHasAcpiDtDxe.inf                   |  43 +++
 .../CloudHvAcpiPlatformDxe/CloudHvAcpi.c      | 155 ++++++++
 .../CloudHvHasAcpiDtDxe.c                     |  69 ++++
 7 files changed, 942 insertions(+)
 create mode 100644 ArmVirtPkg/ArmVirtCloudHv.dsc
 create mode 100644 ArmVirtPkg/ArmVirtCloudHv.fdf
 create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
 create mode 100644 ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf
 create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
 create mode 100644 ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.c

-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 1/3] Acpi: reimplement PlatformHasAcpi for Cloud Hypervisor
  2021-07-05 10:06 [PATCH v4 0/3] Enable Cloud Hypervisor support in edk2 Jianyong Wu
@ 2021-07-05 10:06 ` Jianyong Wu
  2021-07-05 10:06 ` [PATCH v4 2/3] Acpi: Install Acpi tables for Cloud hypervisor Jianyong Wu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Jianyong Wu @ 2021-07-05 10:06 UTC (permalink / raw)
  To: devel; +Cc: lersek, sami.mujawar, ardb+tianocore, justin.he, jianyong.wu

The current implementation of PlatformHasAcpiDt is not a common
library and is on behalf of qemu. So give a specific version for
Cloud Hypervisor here.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---
 .../CloudHvHasAcpiDtDxe.inf                   | 43 ++++++++++++
 .../CloudHvHasAcpiDtDxe.c                     | 69 +++++++++++++++++++
 2 files changed, 112 insertions(+)
 create mode 100644 ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf
 create mode 100644 ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.c

diff --git a/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf b/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf
new file mode 100644
index 000000000000..eb63a4136545
--- /dev/null
+++ b/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf
@@ -0,0 +1,43 @@
+## @file
+# Decide whether the firmware should expose an ACPI- and/or a Device Tree-based
+# hardware description to the operating system.
+#
+# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION                    = 1.25
+  BASE_NAME                      = CloudHvPlatformHasAcpiDtDxe
+  FILE_GUID                      = 71fe72f9-6dc1-199d-5054-13b4200ee88d
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = PlatformHasAcpiDt
+
+[Sources]
+  CloudHvHasAcpiDtDxe.c
+
+[Packages]
+  ArmVirtPkg/ArmVirtPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  PcdLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+
+[Guids]
+  gEdkiiPlatformHasAcpiGuid       ## SOMETIMES_PRODUCES ## PROTOCOL
+  gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
+
+[Pcd]
+  gArmVirtTokenSpaceGuid.PcdForceNoAcpi
+
+[Depex]
+  gEfiVariableArchProtocolGuid
diff --git a/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.c b/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.c
new file mode 100644
index 000000000000..48a446c68a45
--- /dev/null
+++ b/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.c
@@ -0,0 +1,69 @@
+/** @file
+  Decide whether the firmware should expose an ACPI- and/or a Device Tree-based
+  hardware description to the operating system.
+
+  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Guid/PlatformHasAcpi.h>
+#include <Guid/PlatformHasDeviceTree.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+
+EFI_STATUS
+EFIAPI
+PlatformHasAcpiDt (
+  IN EFI_HANDLE       ImageHandle,
+  IN EFI_SYSTEM_TABLE *SystemTable
+  )
+{
+  EFI_STATUS           Status;
+
+  //
+  // If we fail to install any of the necessary protocols below, the OS will be
+  // unbootable anyway (due to lacking hardware description), so tolerate no
+  // errors here.
+  //
+  if (MAX_UINTN == MAX_UINT64 &&
+      !PcdGetBool (PcdForceNoAcpi))
+  {
+    Status = gBS->InstallProtocolInterface (
+                    &ImageHandle,
+                    &gEdkiiPlatformHasAcpiGuid,
+                    EFI_NATIVE_INTERFACE,
+                    NULL
+                    );
+    if (EFI_ERROR (Status)) {
+      goto Failed;
+    }
+
+    return Status;
+  }
+
+  //
+  // Expose the Device Tree otherwise.
+  //
+  Status = gBS->InstallProtocolInterface (
+                  &ImageHandle,
+                  &gEdkiiPlatformHasDeviceTreeGuid,
+                  EFI_NATIVE_INTERFACE,
+                  NULL
+                  );
+  if (EFI_ERROR (Status)) {
+    goto Failed;
+  }
+
+  return Status;
+
+Failed:
+  ASSERT_EFI_ERROR (Status);
+  CpuDeadLoop ();
+  //
+  // Keep compilers happy.
+  //
+  return Status;
+}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 2/3] Acpi: Install Acpi tables for Cloud hypervisor
  2021-07-05 10:06 [PATCH v4 0/3] Enable Cloud Hypervisor support in edk2 Jianyong Wu
  2021-07-05 10:06 ` [PATCH v4 1/3] Acpi: reimplement PlatformHasAcpi for Cloud Hypervisor Jianyong Wu
@ 2021-07-05 10:06 ` Jianyong Wu
  2021-07-06  8:52   ` Sami Mujawar
  2021-07-05 10:06 ` [PATCH v4 3/3] ArmVirtCloudHv: support Cloud Hypervisor in edk2 Jianyong Wu
  2021-07-16 17:25 ` [PATCH v4 0/3] Enable Cloud Hypervisor support " Ard Biesheuvel
  3 siblings, 1 reply; 10+ messages in thread
From: Jianyong Wu @ 2021-07-05 10:06 UTC (permalink / raw)
  To: devel; +Cc: lersek, sami.mujawar, ardb+tianocore, justin.he, jianyong.wu

There is no device like Fw-cfg in Qemu in Cloud Hypervisor, so a specific
Acpi handler is introduced here.

The handler implemented here is in a very simple way:
1. acquire the RSDP from the PCD variable in the top ".dsc";
2. get the XSDT address from RSDP structure;
3. get the ACPI tables following the XSDT structure and install them
one by one;
4. get DSDT address from FADT and install DSDT table.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
---
 ArmVirtPkg/ArmVirtPkg.dec                     |   6 +
 .../CloudHvAcpiPlatformDxe.inf                |  47 ++++++
 .../CloudHvAcpiPlatformDxe/CloudHvAcpi.c      | 155 ++++++++++++++++++
 3 files changed, 208 insertions(+)
 create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
 create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c

diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index bf82f7f1f3f2..4e4d758015bc 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -66,6 +66,12 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   #
   gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0x4D}|VOID*|0x00000007
 
+  ##
+  # This is the physical address of Rsdp which is the core struct of Acpi.
+  # Cloud Hypervisor has no other way to pass Rsdp address to the guest except use a PCD.
+  #
+  gArmVirtTokenSpaceGuid.PcdCloudHvAcpiRsdpBaseAddress|0x0|UINT64|0x00000005
+
 [PcdsDynamic]
   #
   # Whether to force disable ACPI, regardless of the fw_cfg settings
diff --git a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
new file mode 100644
index 000000000000..01de76486686
--- /dev/null
+++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
@@ -0,0 +1,47 @@
+## @file
+#  ACPI Platform Driver for Cloud Hypervisor
+#
+#  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = CloudHvgAcpiPlatform
+  FILE_GUID                      = 6c76e407-73f2-dc1c-938f-5d6c4691ea93
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = CloudHvAcpiPlatformEntryPoint
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+  VALID_ARCHITECTURES           = AARCH64
+#
+
+[Sources]
+  CloudHvAcpi.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  OvmfPkg/OvmfPkg.dec
+  ArmVirtPkg/ArmVirtPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  MemoryAllocationLib
+  OrderedCollectionLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+
+[Protocols]
+  gEfiAcpiTableProtocolGuid                     # PROTOCOL ALWAYS_CONSUMED
+
+[Pcd]
+  gArmVirtTokenSpaceGuid.PcdCloudHvAcpiRsdpBaseAddress
+
+[Depex]
+  gEfiAcpiTableProtocolGuid
diff --git a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
new file mode 100644
index 000000000000..f5a47aa7f3cd
--- /dev/null
+++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
@@ -0,0 +1,155 @@
+/** @file
+  Install Acpi tables for Cloud Hypervisor
+
+  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <IndustryStandard/Acpi63.h>
+#include <Protocol/AcpiTable.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiDriverEntryPoint.h>
+#include <Library/DebugLib.h>
+
+/**
+   Find Acpi table Protocol and return it
+
+   @return AcpiTable  Protocol, which is used to handle Acpi Table, on SUCCESS or NULL on FAILURE.
+
+**/
+STATIC
+EFI_ACPI_TABLE_PROTOCOL *
+FindAcpiTableProtocol (
+  VOID
+  )
+{
+  EFI_STATUS              Status;
+  EFI_ACPI_TABLE_PROTOCOL *AcpiTable;
+
+  Status = gBS->LocateProtocol (
+                  &gEfiAcpiTableProtocolGuid,
+                  NULL,
+                  (VOID**)&AcpiTable
+                  );
+  ASSERT_EFI_ERROR (Status);
+  return AcpiTable;
+}
+
+/** Install Acpi tables for Cloud Hypervisor
+
+  @param [in]  AcpiProtocol  Acpi Protocol which is used to install Acpi talbles
+
+  @return EFI_SUCCESS            The table was successfully inserted.
+  @return EFI_INVALID_PARAMETER  Either AcpiProtocol, AcpiTablePtr or DsdtPtr is NULL
+                                 and the size field embedded in the ACPI table pointed
+                                 by AcpiTablePtr or DsdtPtr are not in sync.
+  @return EFI_OUT_OF_RESOURCES   Insufficient resources exist to complete the request.
+  @return EFI_NOT_FOUND          DSDT table not found.
+**/
+EFI_STATUS
+EFIAPI
+InstallCloudHvAcpiTables (
+ IN     EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
+ )
+{
+  UINTN          InstalledKey;
+  UINTN          TableSize;
+  UINTN          AcpiTableLength;
+  UINT64         RsdpPtr;
+  UINT64         XsdtPtr;
+  UINT64         TableOffset;
+  UINT64         AcpiTablePtr;
+  UINT64         *DsdtPtr = NULL;
+  EFI_STATUS     Status;
+
+  if (AcpiProtocol == NULL) {
+      return EFI_INVALID_PARAMETER;
+  }
+
+  RsdpPtr = PcdGet64 (PcdCloudHvAcpiRsdpBaseAddress);
+  XsdtPtr = ((EFI_ACPI_6_3_ROOT_SYSTEM_DESCRIPTION_POINTER *)RsdpPtr)->XsdtAddress;
+  AcpiTableLength = ((EFI_ACPI_COMMON_HEADER *)XsdtPtr)->Length;
+  TableOffset = sizeof (EFI_ACPI_DESCRIPTION_HEADER);
+
+  while (TableOffset < AcpiTableLength) {
+    AcpiTablePtr = *(UINT64 *)(XsdtPtr + TableOffset);
+    TableSize = ((EFI_ACPI_COMMON_HEADER *)AcpiTablePtr)->Length;
+
+    //
+    // Install ACPI tables from XSDT
+    //
+    Status = AcpiProtocol->InstallAcpiTable (
+                             AcpiProtocol,
+                             (VOID *)AcpiTablePtr,
+                             TableSize,
+                             &InstalledKey
+                             );
+
+    if (EFI_ERROR(Status)) {
+        return Status;
+    }
+
+    TableOffset += sizeof (UINT64);
+
+    //
+    // Get DSDT from FADT
+    //
+    if (DsdtPtr == NULL
+      && !AsciiStrnCmp ((CHAR8 *)&((EFI_ACPI_COMMON_HEADER *)AcpiTablePtr)->Signature, "FACP", 4)) {
+      DsdtPtr = (UINT64 *)(((EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE *)AcpiTablePtr)->XDsdt);
+    }
+  }
+
+  if (DsdtPtr == NULL) {
+    DEBUG ((DEBUG_ERROR, "%a: no DSDT found\n", __FUNCTION__));
+    return EFI_NOT_FOUND;
+  }
+
+  //
+  // Install DSDT table
+  //
+  TableSize = ((EFI_ACPI_COMMON_HEADER *)DsdtPtr)->Length;
+  Status = AcpiProtocol->InstallAcpiTable (
+             AcpiProtocol,
+             DsdtPtr,
+             TableSize,
+             &InstalledKey
+             );
+
+  return Status;
+}
+
+/** Entry point for Cloud Hypervisor Platform Dxe
+
+  @param [in]  ImageHandle  Handle for this image.
+  @param [in]  SystemTable  Pointer to the EFI system table.
+
+  @return EFI_SUCCESS            The table was successfully inserted.
+  @return EFI_INVALID_PARAMETER  Either AcpiProtocol, AcpiTablePtr or DsdtPtr is NULL
+                                 and the size field embedded in the ACPI table pointed to
+                                 by AcpiTablePtr or DsdtPtr are not in sync.
+  @return EFI_OUT_OF_RESOURCES   Insufficient resources exist to complete the request.
+  @return EFI_NOT_FOUND          DSDT table not found
+**/
+EFI_STATUS
+EFIAPI
+CloudHvAcpiPlatformEntryPoint (
+  IN EFI_HANDLE         ImageHandle,
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_STATUS                         Status;
+
+  Status = InstallCloudHvAcpiTables (FindAcpiTableProtocol ());
+
+  if (EFI_ERROR(Status)) {
+     DEBUG ((DEBUG_ERROR, "%a: Fail to install Acpi table: %r\n", __FUNCTION__,
+       Status));
+     CpuDeadLoop ();
+  }
+
+  return EFI_SUCCESS;
+}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 3/3] ArmVirtCloudHv: support Cloud Hypervisor in edk2
  2021-07-05 10:06 [PATCH v4 0/3] Enable Cloud Hypervisor support in edk2 Jianyong Wu
  2021-07-05 10:06 ` [PATCH v4 1/3] Acpi: reimplement PlatformHasAcpi for Cloud Hypervisor Jianyong Wu
  2021-07-05 10:06 ` [PATCH v4 2/3] Acpi: Install Acpi tables for Cloud hypervisor Jianyong Wu
@ 2021-07-05 10:06 ` Jianyong Wu
  2021-07-16 17:25 ` [PATCH v4 0/3] Enable Cloud Hypervisor support " Ard Biesheuvel
  3 siblings, 0 replies; 10+ messages in thread
From: Jianyong Wu @ 2021-07-05 10:06 UTC (permalink / raw)
  To: devel; +Cc: lersek, sami.mujawar, ardb+tianocore, justin.he, jianyong.wu

Cloud Hypervisor is KVM based VMM and is implemented in rust. Just like
other VMMs it need UEFI support to let ACPI work. That's why
Cloud Hypervisor is introduced here.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
---
 ArmVirtPkg/ArmVirtCloudHv.dsc | 364 ++++++++++++++++++++++++++++++++++
 ArmVirtPkg/ArmVirtCloudHv.fdf | 258 ++++++++++++++++++++++++
 2 files changed, 622 insertions(+)
 create mode 100644 ArmVirtPkg/ArmVirtCloudHv.dsc
 create mode 100644 ArmVirtPkg/ArmVirtCloudHv.fdf

diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc
new file mode 100644
index 000000000000..f292ba6079b2
--- /dev/null
+++ b/ArmVirtPkg/ArmVirtCloudHv.dsc
@@ -0,0 +1,364 @@
+#
+#  Copyright (c) 2021, ARM Limited. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+
+################################################################################
+#
+# Defines Section - statements that will be processed to create a Makefile.
+#
+################################################################################
+[Defines]
+  PLATFORM_NAME                  = ArmVirtCloudHv
+  PLATFORM_GUID                  = DFFED32B-DFFE-D32B-DFFE-D32BDFFED32B
+  PLATFORM_VERSION               = 0.1
+  DSC_SPECIFICATION              = 0x00010005
+  OUTPUT_DIRECTORY               = Build/ArmVirtCloudHv-$(ARCH)
+  SUPPORTED_ARCHITECTURES        = AARCH64|ARM
+  BUILD_TARGETS                  = DEBUG|RELEASE|NOOPT
+  SKUID_IDENTIFIER               = DEFAULT
+  FLASH_DEFINITION               = ArmVirtPkg/ArmVirtCloudHv.fdf
+
+  #
+  # Defines for default states.  These can be changed on the command line.
+  # -D FLAG=VALUE
+  #
+  DEFINE TTY_TERMINAL            = FALSE
+  DEFINE SECURE_BOOT_ENABLE      = FALSE
+
+!include ArmVirtPkg/ArmVirt.dsc.inc
+
+[LibraryClasses.common]
+  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
+  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+
+  # Virtio Support
+  VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
+  VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
+
+  ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
+
+  TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
+  CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
+  BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
+  PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+  PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
+  CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
+  FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
+  QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
+  FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
+  PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+  PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
+  PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
+  PciHostBridgeUtilityLib|ArmVirtPkg/Library/ArmVirtPciHostBridgeUtilityLib/ArmVirtPciHostBridgeUtilityLib.inf
+
+  TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
+
+!include MdePkg/MdeLibs.dsc.inc
+
+[LibraryClasses.common.PEIM]
+  ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
+
+[LibraryClasses.common.DXE_DRIVER]
+  ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
+
+[LibraryClasses.common.UEFI_DRIVER]
+  UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
+
+[BuildOptions]
+!include NetworkPkg/NetworkBuildOptions.dsc.inc
+
+################################################################################
+#
+# Pcd Section - list of all EDK II PCD Entries defined by this Platform
+#
+################################################################################
+
+[PcdsFeatureFlag.common]
+  ## If TRUE, Graphics Output Protocol will be installed on virtual handle created by ConsplitterDxe.
+  #  It could be set FALSE to save size.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
+
+  gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE
+
+[PcdsFixedAtBuild.common]
+!if $(ARCH) == AARCH64
+  gArmTokenSpaceGuid.PcdVFPEnabled|1
+!endif
+
+  gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x4007c000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
+  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
+
+  # Rsdp base address in Cloud Hypervisor
+  gArmVirtTokenSpaceGuid.PcdCloudHvAcpiRsdpBaseAddress|0x40200000
+
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x4000000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x40000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable|TRUE
+!if $(NETWORK_TLS_ENABLE) == TRUE
+  #
+  # The cumulative and individual VOLATILE variable size limits should be set
+  # high enough for accommodating several and/or large CA certificates.
+  #
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x80000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize|0x40000
+!endif
+
+  # Size of the region used by UEFI in permanent memory (Reserved 64MB)
+  gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x04000000
+
+  #
+  # ARM PrimeCell
+  #
+
+  ## PL011 - Serial Terminal
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|38400
+
+  ## Default Terminal Type
+  ## 0-PCANSI, 1-VT100, 2-VT00+, 3-UTF8, 4-TTYTERM
+  gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4
+
+  # System Memory Base -- fixed at 0x4000_0000
+  gArmTokenSpaceGuid.PcdSystemMemoryBase|0x40000000
+
+  # initial location of the device tree blob passed by Cloud Hypervisor -- base of DRAM
+  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress|0x40000000
+
+  gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
+
+  #
+  # The maximum physical I/O addressability of the processor, set with
+  # BuildCpuHob().
+  #
+  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
+
+  #
+  # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
+  #
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
+
+!if $(SECURE_BOOT_ENABLE) == TRUE
+  # override the default values from SecurityPkg to ensure images from all sources are verified in secure boot
+  gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04
+  gEfiSecurityPkgTokenSpaceGuid.PcdFixedMediaImageVerificationPolicy|0x04
+  gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
+!endif
+
+  gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|3
+  gEfiShellPkgTokenSpaceGuid.PcdShellFileOperationSize|0x20000
+
+[PcdsFixedAtBuild.AARCH64]
+  # Clearing BIT0 in this PCD prevents installing a 32-bit SMBIOS entry point,
+  # if the entry point version is >= 3.0. AARCH64 OSes cannot assume the
+  # presence of the 32-bit entry point anyway (because many AARCH64 systems
+  # don't have 32-bit addressable physical RAM), and the additional allocations
+  # below 4 GB needlessly fragment the memory map. So expose the 64-bit entry
+  # point only, for entry point versions >= 3.0.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosEntryPointProvideMethod|0x2
+
+[PcdsDynamicDefault.common]
+  gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
+
+  ## If TRUE, OvmfPkg/AcpiPlatformDxe will not wait for PCI
+  #  enumeration to complete before installing ACPI tables.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|TRUE
+
+  # System Memory Size -- 1 MB initially, actual size will be fetched from DT
+  gArmTokenSpaceGuid.PcdSystemMemorySize|0x00100000
+
+  gArmTokenSpaceGuid.PcdArmArchTimerSecIntrNum|0x0
+  gArmTokenSpaceGuid.PcdArmArchTimerIntrNum|0x0
+  gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum|0x0
+  gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum|0x0
+
+  #
+  # ARM General Interrupt Controller
+  #
+  gArmTokenSpaceGuid.PcdGicDistributorBase|0x0
+  gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x0
+  gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0x0
+
+  ## PL031 RealTimeClock
+  gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x0
+
+  # set PcdPciExpressBaseAddress to MAX_UINT64, which signifies that this
+  # PCD and PcdPciDisableBusEnumeration above have not been assigned yet
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xFFFFFFFFFFFFFFFF
+
+  gArmTokenSpaceGuid.PcdPciIoTranslation|0
+
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress|0x0
+
+[PcdsDynamicHii]
+  gArmVirtTokenSpaceGuid.PcdForceNoAcpi|L"ForceNoAcpi"|gArmVirtVariableGuid|0x0|FALSE|NV,BS
+
+################################################################################
+#
+# Components Section - list of all EDK II Modules needed by this Platform
+#
+################################################################################
+[Components.common]
+  #
+  # PEI Phase modules
+  #
+  ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
+  MdeModulePkg/Core/Pei/PeiMain.inf
+  MdeModulePkg/Universal/PCD/Pei/Pcd.inf {
+    <LibraryClasses>
+      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  }
+  ArmPlatformPkg/PlatformPei/PlatformPeim.inf
+  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
+  ArmPkg/Drivers/CpuPei/CpuPei.inf
+
+  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+
+  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
+    <LibraryClasses>
+      NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
+  }
+
+  #
+  # DXE
+  #
+  MdeModulePkg/Core/Dxe/DxeMain.inf {
+    <LibraryClasses>
+      NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
+      DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
+  }
+  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
+    <LibraryClasses>
+      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  }
+
+  #
+  # Architectural Protocols
+  #
+  ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+  MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
+  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
+    <LibraryClasses>
+      NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
+      # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
+      BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
+  }
+!if $(SECURE_BOOT_ENABLE) == TRUE
+  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
+    <LibraryClasses>
+      NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
+  }
+  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf
+!else
+  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
+!endif
+  MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
+  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
+  MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
+  MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
+  EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf {
+    <LibraryClasses>
+      NULL|ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
+  }
+  EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
+
+  MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
+  MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
+  MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
+  MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
+  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
+
+  MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
+
+  ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
+  ArmPkg/Drivers/TimerDxe/TimerDxe.inf {
+    <LibraryClasses>
+      NULL|ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.inf
+  }
+  MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
+
+  #
+  # Status Code Routing
+  #
+  MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportStatusCodeRouterRuntimeDxe.inf
+
+  #
+  # Platform Driver
+  #
+  ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf
+  ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
+  ArmVirtPkg/HighMemDxe/HighMemDxe.inf
+  OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
+  OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
+  OvmfPkg/VirtioNetDxe/VirtioNet.inf
+  OvmfPkg/VirtioRngDxe/VirtioRng.inf
+
+  #
+  # FAT filesystem + GPT/MBR partitioning + UDF filesystem + virtio-fs
+  #
+  MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
+  MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
+  MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
+  FatPkg/EnhancedFatDxe/Fat.inf
+  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
+  OvmfPkg/VirtioFsDxe/VirtioFsDxe.inf
+
+  #
+  # Bds
+  #
+  MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf {
+    <LibraryClasses>
+      DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
+      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  }
+  MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
+  MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
+  MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
+  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+  MdeModulePkg/Logo/LogoDxe.inf
+  MdeModulePkg/Application/UiApp/UiApp.inf {
+    <LibraryClasses>
+      NULL|MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf
+      NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf
+      NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
+  }
+
+  #
+  # SCSI Bus and Disk Driver
+  #
+  MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
+  MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
+
+  #
+  # PCI support
+  #
+  ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf {
+    <LibraryClasses>
+      NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+  }
+  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
+  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
+    <LibraryClasses>
+      NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+  }
+  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
+  OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
+  OvmfPkg/Virtio10Dxe/Virtio10.inf
+
+  #
+  # ACPI Support
+  #
+  ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf
+[Components.AARCH64]
+  MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
+  ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf {
+    <LibraryClasses>
+      NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+  }
diff --git a/ArmVirtPkg/ArmVirtCloudHv.fdf b/ArmVirtPkg/ArmVirtCloudHv.fdf
new file mode 100644
index 000000000000..13fe8061c3dd
--- /dev/null
+++ b/ArmVirtPkg/ArmVirtCloudHv.fdf
@@ -0,0 +1,258 @@
+#
+#  Copyright (c) 2021, ARM Limited. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+################################################################################
+#
+# FD Section
+# The [FD] Section is made up of the definition statements and a
+# description of what goes into  the Flash Device Image.  Each FD section
+# defines one flash "device" image.  A flash device image may be one of
+# the following: Removable media bootable image (like a boot floppy
+# image,) an Option ROM image (that would be "flashed" into an add-in
+# card,) a System "Flash"  image (that would be burned into a system's
+# flash) or an Update ("Capsule") image that will be used to update and
+# existing system flash.
+#
+################################################################################
+
+[Defines]
+!if $(FD_SIZE_IN_MB) == 2
+  DEFINE FVMAIN_COMPACT_SIZE  = 0x1ff000
+!endif
+!if $(FD_SIZE_IN_MB) == 3
+  DEFINE FVMAIN_COMPACT_SIZE  = 0x2ff000
+!endif
+
+[FD.CLOUDHV_EFI]
+BaseAddress   = 0x00000000|gArmTokenSpaceGuid.PcdFdBaseAddress  # cloud-hypervisor assigns 0 - 0x8000000 for a BootROM
+Size          = $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize         # The size in bytes of the FLASH Device
+ErasePolarity = 1
+
+# This one is tricky, it must be: BlockSize * NumBlocks = Size
+BlockSize     = 0x00001000
+NumBlocks     = $(FD_NUM_BLOCKS)
+
+################################################################################
+#
+# Following are lists of FD Region layout which correspond to the locations of different
+# images within the flash device.
+#
+# Regions must be defined in ascending order and may not overlap.
+#
+# A Layout Region start with a eight digit hex offset (leading "0x" required) followed by
+# the pipe "|" character, followed by the size of the region, also in hex with the leading
+# "0x" characters. Like:
+# Offset|Size
+# PcdOffsetCName|PcdSizeCName
+# RegionType <FV, DATA, or FILE>
+#
+################################################################################
+
+#
+# UEFI has trouble dealing with FVs that reside at physical address 0x0.
+# So instead, put a hardcoded 'jump to 0x1000' at offset 0x0, and put the
+# real FV at offset 0x1000
+#
+0x00000000|0x00001000
+DATA = {
+!if $(ARCH) == AARCH64
+  0x00, 0x04, 0x00, 0x14   # 'b 0x1000' in AArch64 ASM
+!else
+  0xfe, 0x03, 0x00, 0xea   # 'b 0x1000' in AArch32 ASM
+!endif
+}
+
+0x00001000|$(FVMAIN_COMPACT_SIZE)
+gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
+FV = FVMAIN_COMPACT
+
+!include VarStore.fdf.inc
+
+################################################################################
+#
+# FV Section
+#
+# [FV] section is used to define what components or modules are placed within a flash
+# device file.  This section also defines order the components and modules are positioned
+# within the image.  The [FV] section consists of define statements, set statements and
+# module statements.
+#
+################################################################################
+
+[FV.FvMain]
+FvNameGuid         = 2A88A00E-E267-C8BF-0E80-AE1BD504ED90
+BlockSize          = 0x40
+NumBlocks          = 0         # This FV gets compressed so make it just big enough
+FvAlignment        = 16        # FV alignment and FV attributes setting.
+ERASE_POLARITY     = 1
+MEMORY_MAPPED      = TRUE
+STICKY_WRITE       = TRUE
+LOCK_CAP           = TRUE
+LOCK_STATUS        = TRUE
+WRITE_DISABLED_CAP = TRUE
+WRITE_ENABLED_CAP  = TRUE
+WRITE_STATUS       = TRUE
+WRITE_LOCK_CAP     = TRUE
+WRITE_LOCK_STATUS  = TRUE
+READ_DISABLED_CAP  = TRUE
+READ_ENABLED_CAP   = TRUE
+READ_STATUS        = TRUE
+READ_LOCK_CAP      = TRUE
+READ_LOCK_STATUS   = TRUE
+
+  INF MdeModulePkg/Core/Dxe/DxeMain.inf
+  INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
+  INF ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf
+  INF ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
+  INF ArmVirtPkg/HighMemDxe/HighMemDxe.inf
+
+  #
+  # PI DXE Drivers producing Architectural Protocols (EFI Services)
+  #
+  INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+  INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
+  INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
+  INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
+  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
+  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+!if $(SECURE_BOOT_ENABLE) == TRUE
+  INF SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+!endif
+  INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
+  INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
+  INF EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
+  INF EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
+  INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
+
+  #
+  # Multiple Console IO support
+  #
+  INF MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
+  INF MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
+  INF MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
+  INF MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
+  INF MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
+
+  INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
+  INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf
+  INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
+
+  #
+  # FAT filesystem + GPT/MBR partitioning + UDF filesystem + virtio-fs
+  #
+  INF MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
+  INF MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
+  INF FatPkg/EnhancedFatDxe/Fat.inf
+  INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
+  INF MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
+  INF OvmfPkg/VirtioFsDxe/VirtioFsDxe.inf
+
+  #
+  # Status Code Routing
+  #
+  INF MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportStatusCodeRouterRuntimeDxe.inf
+
+  #
+  # Platform Driver
+  #
+  INF OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
+  INF OvmfPkg/VirtioNetDxe/VirtioNet.inf
+  INF OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
+  INF OvmfPkg/VirtioRngDxe/VirtioRng.inf
+
+  #
+  # UEFI application (Shell Embedded Boot Loader)
+  #
+  INF ShellPkg/Application/Shell/Shell.inf
+  INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
+  INF ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
+  INF OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
+
+  #
+  # Bds
+  #
+  INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
+  INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
+  INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
+  INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
+  INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+  INF MdeModulePkg/Application/UiApp/UiApp.inf
+
+  #
+  # SCSI Bus and Disk Driver
+  #
+  INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
+  INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
+
+  #
+  # ACPI Support
+  #
+  INF ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf
+!if $(ARCH) == AARCH64
+  INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+  INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
+  INF ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
+
+  #
+  # EBC support
+  #
+  INF MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
+!endif
+
+  #
+  # PCI support
+  #
+  INF ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
+  INF MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
+  INF MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+  INF OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
+  INF OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
+  INF OvmfPkg/Virtio10Dxe/Virtio10.inf
+
+  #
+  # TianoCore logo (splash screen)
+  #
+  INF MdeModulePkg/Logo/LogoDxe.inf
+
+  #
+  # Ramdisk support
+  #
+  INF MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
+
+[FV.FVMAIN_COMPACT]
+FvAlignment        = 16
+ERASE_POLARITY     = 1
+MEMORY_MAPPED      = TRUE
+STICKY_WRITE       = TRUE
+LOCK_CAP           = TRUE
+LOCK_STATUS        = TRUE
+WRITE_DISABLED_CAP = TRUE
+WRITE_ENABLED_CAP  = TRUE
+WRITE_STATUS       = TRUE
+WRITE_LOCK_CAP     = TRUE
+WRITE_LOCK_STATUS  = TRUE
+READ_DISABLED_CAP  = TRUE
+READ_ENABLED_CAP   = TRUE
+READ_STATUS        = TRUE
+READ_LOCK_CAP      = TRUE
+READ_LOCK_STATUS   = TRUE
+
+  INF ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
+  INF MdeModulePkg/Core/Pei/PeiMain.inf
+  INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf
+  INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
+  INF ArmPkg/Drivers/CpuPei/CpuPei.inf
+  INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
+  INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+  INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+
+  FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
+    SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
+      SECTION FV_IMAGE = FVMAIN
+    }
+  }
+
+!include ArmVirtRules.fdf.inc
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/3] Acpi: Install Acpi tables for Cloud hypervisor
  2021-07-05 10:06 ` [PATCH v4 2/3] Acpi: Install Acpi tables for Cloud hypervisor Jianyong Wu
@ 2021-07-06  8:52   ` Sami Mujawar
  2021-07-07  1:42     ` Jianyong Wu
  0 siblings, 1 reply; 10+ messages in thread
From: Sami Mujawar @ 2021-07-06  8:52 UTC (permalink / raw)
  To: Jianyong Wu, devel@edk2.groups.io
  Cc: lersek@redhat.com, ardb+tianocore@kernel.org, Justin He, nd

Hi Jianyong,

I should have caught this earlier in my review.  However, if you agree, I will do the following changes before pushing the patch.

1.	The subject line of the commit message does not confirm to the edk2 coding standard. It should have ‘ArmVirtPkg: <subject line for the patch>’
2.	The ACPI table signature can be simplified further. Can you try the following and let me know if it works, please?

diff --git a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
index f5a47aa7f3cd..51b012676e7d 100644
--- a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
+++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
@@ -52,8 +52,8 @@ FindAcpiTableProtocol (
 EFI_STATUS
 EFIAPI
 InstallCloudHvAcpiTables (
- IN     EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
- )
+  IN     EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
+  )
 {
   UINTN          InstalledKey;
   UINTN          TableSize;
@@ -97,11 +97,12 @@ InstallCloudHvAcpiTables (
     //
     // Get DSDT from FADT
     //
-    if (DsdtPtr == NULL
-      && !AsciiStrnCmp ((CHAR8 *)&((EFI_ACPI_COMMON_HEADER *)AcpiTablePtr)->Signature, "FACP", 4)) {
+    if ((DsdtPtr == NULL)
+      && (EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE ==
+          ((EFI_ACPI_COMMON_HEADER *)AcpiTablePtr)->Signature)) {
       DsdtPtr = (UINT64 *)(((EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE *)AcpiTablePtr)->XDsdt);
     }
-  }
+  } // while

   if (DsdtPtr == NULL) {
     DEBUG ((DEBUG_ERROR, "%a: no DSDT found\n", __FUNCTION__));

Regards,

Sami Mujawar

On 05/07/2021, 11:07, "Jianyong Wu" <jianyong.wu@arm.com> wrote:

    There is no device like Fw-cfg in Qemu in Cloud Hypervisor, so a specific
    Acpi handler is introduced here.

    The handler implemented here is in a very simple way:
    1. acquire the RSDP from the PCD variable in the top ".dsc";
    2. get the XSDT address from RSDP structure;
    3. get the ACPI tables following the XSDT structure and install them
    one by one;
    4. get DSDT address from FADT and install DSDT table.

    Cc: Laszlo Ersek <lersek@redhat.com>
    Cc: Sami Mujawar <sami.mujawar@arm.com>

    Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
    ---
     ArmVirtPkg/ArmVirtPkg.dec                     |   6 +
     .../CloudHvAcpiPlatformDxe.inf                |  47 ++++++
     .../CloudHvAcpiPlatformDxe/CloudHvAcpi.c      | 155 ++++++++++++++++++
     3 files changed, 208 insertions(+)
     create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
     create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c

    diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
    index bf82f7f1f3f2..4e4d758015bc 100644
    --- a/ArmVirtPkg/ArmVirtPkg.dec
    +++ b/ArmVirtPkg/ArmVirtPkg.dec
    @@ -66,6 +66,12 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
       #
       gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0x4D}|VOID*|0x00000007

    +  ##
    +  # This is the physical address of Rsdp which is the core struct of Acpi.
    +  # Cloud Hypervisor has no other way to pass Rsdp address to the guest except use a PCD.
    +  #
    +  gArmVirtTokenSpaceGuid.PcdCloudHvAcpiRsdpBaseAddress|0x0|UINT64|0x00000005
    +
     [PcdsDynamic]
       #
       # Whether to force disable ACPI, regardless of the fw_cfg settings
    diff --git a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
    new file mode 100644
    index 000000000000..01de76486686
    --- /dev/null
    +++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
    @@ -0,0 +1,47 @@
    +## @file
    +#  ACPI Platform Driver for Cloud Hypervisor
    +#
    +#  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
    +#  SPDX-License-Identifier: BSD-2-Clause-Patent
    +#
    +##
    +
    +[Defines]
    +  INF_VERSION                    = 0x00010005
    +  BASE_NAME                      = CloudHvgAcpiPlatform
    +  FILE_GUID                      = 6c76e407-73f2-dc1c-938f-5d6c4691ea93
    +  MODULE_TYPE                    = DXE_DRIVER
    +  VERSION_STRING                 = 1.0
    +  ENTRY_POINT                    = CloudHvAcpiPlatformEntryPoint
    +
    +#
    +# The following information is for reference only and not required by the build tools.
    +#
    +  VALID_ARCHITECTURES           = AARCH64
    +#
    +
    +[Sources]
    +  CloudHvAcpi.c
    +
    +[Packages]
    +  MdePkg/MdePkg.dec
    +  MdeModulePkg/MdeModulePkg.dec
    +  OvmfPkg/OvmfPkg.dec
    +  ArmVirtPkg/ArmVirtPkg.dec
    +
    +[LibraryClasses]
    +  BaseLib
    +  DebugLib
    +  MemoryAllocationLib
    +  OrderedCollectionLib
    +  UefiBootServicesTableLib
    +  UefiDriverEntryPoint
    +
    +[Protocols]
    +  gEfiAcpiTableProtocolGuid                     # PROTOCOL ALWAYS_CONSUMED
    +
    +[Pcd]
    +  gArmVirtTokenSpaceGuid.PcdCloudHvAcpiRsdpBaseAddress
    +
    +[Depex]
    +  gEfiAcpiTableProtocolGuid
    diff --git a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
    new file mode 100644
    index 000000000000..f5a47aa7f3cd
    --- /dev/null
    +++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
    @@ -0,0 +1,155 @@
    +/** @file
    +  Install Acpi tables for Cloud Hypervisor
    +
    +  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
    +
    +  SPDX-License-Identifier: BSD-2-Clause-Patent
    +**/
    +
    +#include <Library/BaseLib.h>
    +#include <Library/MemoryAllocationLib.h>
    +#include <IndustryStandard/Acpi63.h>
    +#include <Protocol/AcpiTable.h>
    +#include <Library/UefiBootServicesTableLib.h>
    +#include <Library/UefiDriverEntryPoint.h>
    +#include <Library/DebugLib.h>
    +
    +/**
    +   Find Acpi table Protocol and return it
    +
    +   @return AcpiTable  Protocol, which is used to handle Acpi Table, on SUCCESS or NULL on FAILURE.
    +
    +**/
    +STATIC
    +EFI_ACPI_TABLE_PROTOCOL *
    +FindAcpiTableProtocol (
    +  VOID
    +  )
    +{
    +  EFI_STATUS              Status;
    +  EFI_ACPI_TABLE_PROTOCOL *AcpiTable;
    +
    +  Status = gBS->LocateProtocol (
    +                  &gEfiAcpiTableProtocolGuid,
    +                  NULL,
    +                  (VOID**)&AcpiTable
    +                  );
    +  ASSERT_EFI_ERROR (Status);
    +  return AcpiTable;
    +}
    +
    +/** Install Acpi tables for Cloud Hypervisor
    +
    +  @param [in]  AcpiProtocol  Acpi Protocol which is used to install Acpi talbles
    +
    +  @return EFI_SUCCESS            The table was successfully inserted.
    +  @return EFI_INVALID_PARAMETER  Either AcpiProtocol, AcpiTablePtr or DsdtPtr is NULL
    +                                 and the size field embedded in the ACPI table pointed
    +                                 by AcpiTablePtr or DsdtPtr are not in sync.
    +  @return EFI_OUT_OF_RESOURCES   Insufficient resources exist to complete the request.
    +  @return EFI_NOT_FOUND          DSDT table not found.
    +**/
    +EFI_STATUS
    +EFIAPI
    +InstallCloudHvAcpiTables (
    + IN     EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
    + )
    +{
    +  UINTN          InstalledKey;
    +  UINTN          TableSize;
    +  UINTN          AcpiTableLength;
    +  UINT64         RsdpPtr;
    +  UINT64         XsdtPtr;
    +  UINT64         TableOffset;
    +  UINT64         AcpiTablePtr;
    +  UINT64         *DsdtPtr = NULL;
    +  EFI_STATUS     Status;
    +
    +  if (AcpiProtocol == NULL) {
    +      return EFI_INVALID_PARAMETER;
    +  }
    +
    +  RsdpPtr = PcdGet64 (PcdCloudHvAcpiRsdpBaseAddress);
    +  XsdtPtr = ((EFI_ACPI_6_3_ROOT_SYSTEM_DESCRIPTION_POINTER *)RsdpPtr)->XsdtAddress;
    +  AcpiTableLength = ((EFI_ACPI_COMMON_HEADER *)XsdtPtr)->Length;
    +  TableOffset = sizeof (EFI_ACPI_DESCRIPTION_HEADER);
    +
    +  while (TableOffset < AcpiTableLength) {
    +    AcpiTablePtr = *(UINT64 *)(XsdtPtr + TableOffset);
    +    TableSize = ((EFI_ACPI_COMMON_HEADER *)AcpiTablePtr)->Length;
    +
    +    //
    +    // Install ACPI tables from XSDT
    +    //
    +    Status = AcpiProtocol->InstallAcpiTable (
    +                             AcpiProtocol,
    +                             (VOID *)AcpiTablePtr,
    +                             TableSize,
    +                             &InstalledKey
    +                             );
    +
    +    if (EFI_ERROR(Status)) {
    +        return Status;
    +    }
    +
    +    TableOffset += sizeof (UINT64);
    +
    +    //
    +    // Get DSDT from FADT
    +    //
    +    if (DsdtPtr == NULL
    +      && !AsciiStrnCmp ((CHAR8 *)&((EFI_ACPI_COMMON_HEADER *)AcpiTablePtr)->Signature, "FACP", 4)) {
    +      DsdtPtr = (UINT64 *)(((EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE *)AcpiTablePtr)->XDsdt);
    +    }
    +  }
    +
    +  if (DsdtPtr == NULL) {
    +    DEBUG ((DEBUG_ERROR, "%a: no DSDT found\n", __FUNCTION__));
    +    return EFI_NOT_FOUND;
    +  }
    +
    +  //
    +  // Install DSDT table
    +  //
    +  TableSize = ((EFI_ACPI_COMMON_HEADER *)DsdtPtr)->Length;
    +  Status = AcpiProtocol->InstallAcpiTable (
    +             AcpiProtocol,
    +             DsdtPtr,
    +             TableSize,
    +             &InstalledKey
    +             );
    +
    +  return Status;
    +}
    +
    +/** Entry point for Cloud Hypervisor Platform Dxe
    +
    +  @param [in]  ImageHandle  Handle for this image.
    +  @param [in]  SystemTable  Pointer to the EFI system table.
    +
    +  @return EFI_SUCCESS            The table was successfully inserted.
    +  @return EFI_INVALID_PARAMETER  Either AcpiProtocol, AcpiTablePtr or DsdtPtr is NULL
    +                                 and the size field embedded in the ACPI table pointed to
    +                                 by AcpiTablePtr or DsdtPtr are not in sync.
    +  @return EFI_OUT_OF_RESOURCES   Insufficient resources exist to complete the request.
    +  @return EFI_NOT_FOUND          DSDT table not found
    +**/
    +EFI_STATUS
    +EFIAPI
    +CloudHvAcpiPlatformEntryPoint (
    +  IN EFI_HANDLE         ImageHandle,
    +  IN EFI_SYSTEM_TABLE   *SystemTable
    +  )
    +{
    +  EFI_STATUS                         Status;
    +
    +  Status = InstallCloudHvAcpiTables (FindAcpiTableProtocol ());
    +
    +  if (EFI_ERROR(Status)) {
    +     DEBUG ((DEBUG_ERROR, "%a: Fail to install Acpi table: %r\n", __FUNCTION__,
    +       Status));
    +     CpuDeadLoop ();
    +  }
    +
    +  return EFI_SUCCESS;
    +}
    -- 
    2.17.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/3] Acpi: Install Acpi tables for Cloud hypervisor
  2021-07-06  8:52   ` Sami Mujawar
@ 2021-07-07  1:42     ` Jianyong Wu
  2021-07-08  9:25       ` Sami Mujawar
  0 siblings, 1 reply; 10+ messages in thread
From: Jianyong Wu @ 2021-07-07  1:42 UTC (permalink / raw)
  To: Sami Mujawar, devel@edk2.groups.io
  Cc: lersek@redhat.com, ardb+tianocore@kernel.org, Justin He, nd

Hi Sami,

Thanks for your rework on my patch. I tried the change and it works well. You can do what you like on the patch set.

Thanks
Jianyong

> -----Original Message-----
> From: Sami Mujawar <Sami.Mujawar@arm.com>
> Sent: Tuesday, July 6, 2021 4:52 PM
> To: Jianyong Wu <Jianyong.Wu@arm.com>; devel@edk2.groups.io
> Cc: lersek@redhat.com; ardb+tianocore@kernel.org; Justin He
> <Justin.He@arm.com>; nd <nd@arm.com>
> Subject: Re: [PATCH v4 2/3] Acpi: Install Acpi tables for Cloud hypervisor
> 
> Hi Jianyong,
> 
> I should have caught this earlier in my review.  However, if you agree, I will
> do the following changes before pushing the patch.
> 
> 1.	The subject line of the commit message does not confirm to the edk2
> coding standard. It should have ‘ArmVirtPkg: <subject line for the patch>’
> 2.	The ACPI table signature can be simplified further. Can you try the
> following and let me know if it works, please?
> 
> diff --git a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
> b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
> index f5a47aa7f3cd..51b012676e7d 100644
> --- a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
> +++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
> @@ -52,8 +52,8 @@ FindAcpiTableProtocol (  EFI_STATUS  EFIAPI
> InstallCloudHvAcpiTables (
> - IN     EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
> - )
> +  IN     EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
> +  )
>  {
>    UINTN          InstalledKey;
>    UINTN          TableSize;
> @@ -97,11 +97,12 @@ InstallCloudHvAcpiTables (
>      //
>      // Get DSDT from FADT
>      //
> -    if (DsdtPtr == NULL
> -      && !AsciiStrnCmp ((CHAR8 *)&((EFI_ACPI_COMMON_HEADER
> *)AcpiTablePtr)->Signature, "FACP", 4)) {
> +    if ((DsdtPtr == NULL)
> +      && (EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE ==
> +          ((EFI_ACPI_COMMON_HEADER *)AcpiTablePtr)->Signature)) {
>        DsdtPtr = (UINT64 *)(((EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE
> *)AcpiTablePtr)->XDsdt);
>      }
> -  }
> +  } // while
> 
>    if (DsdtPtr == NULL) {
>      DEBUG ((DEBUG_ERROR, "%a: no DSDT found\n", __FUNCTION__));
> 
> Regards,
> 
> Sami Mujawar
> 
> On 05/07/2021, 11:07, "Jianyong Wu" <jianyong.wu@arm.com> wrote:
> 
>     There is no device like Fw-cfg in Qemu in Cloud Hypervisor, so a specific
>     Acpi handler is introduced here.
> 
>     The handler implemented here is in a very simple way:
>     1. acquire the RSDP from the PCD variable in the top ".dsc";
>     2. get the XSDT address from RSDP structure;
>     3. get the ACPI tables following the XSDT structure and install them
>     one by one;
>     4. get DSDT address from FADT and install DSDT table.
> 
>     Cc: Laszlo Ersek <lersek@redhat.com>
>     Cc: Sami Mujawar <sami.mujawar@arm.com>
> 
>     Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
>     ---
>      ArmVirtPkg/ArmVirtPkg.dec                     |   6 +
>      .../CloudHvAcpiPlatformDxe.inf                |  47 ++++++
>      .../CloudHvAcpiPlatformDxe/CloudHvAcpi.c      | 155
> ++++++++++++++++++
>      3 files changed, 208 insertions(+)
>      create mode 100644
> ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
>      create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
> 
>     diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
>     index bf82f7f1f3f2..4e4d758015bc 100644
>     --- a/ArmVirtPkg/ArmVirtPkg.dec
>     +++ b/ArmVirtPkg/ArmVirtPkg.dec
>     @@ -66,6 +66,12 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
>        #
>        gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6,
> 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1,
> 0x4D}|VOID*|0x00000007
> 
>     +  ##
>     +  # This is the physical address of Rsdp which is the core struct of Acpi.
>     +  # Cloud Hypervisor has no other way to pass Rsdp address to the guest
> except use a PCD.
>     +  #
>     +
> gArmVirtTokenSpaceGuid.PcdCloudHvAcpiRsdpBaseAddress|0x0|UINT64|0
> x00000005
>     +
>      [PcdsDynamic]
>        #
>        # Whether to force disable ACPI, regardless of the fw_cfg settings
>     diff --git
> a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
> b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
>     new file mode 100644
>     index 000000000000..01de76486686
>     --- /dev/null
>     +++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
>     @@ -0,0 +1,47 @@
>     +## @file
>     +#  ACPI Platform Driver for Cloud Hypervisor
>     +#
>     +#  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
>     +#  SPDX-License-Identifier: BSD-2-Clause-Patent
>     +#
>     +##
>     +
>     +[Defines]
>     +  INF_VERSION                    = 0x00010005
>     +  BASE_NAME                      = CloudHvgAcpiPlatform
>     +  FILE_GUID                      = 6c76e407-73f2-dc1c-938f-5d6c4691ea93
>     +  MODULE_TYPE                    = DXE_DRIVER
>     +  VERSION_STRING                 = 1.0
>     +  ENTRY_POINT                    = CloudHvAcpiPlatformEntryPoint
>     +
>     +#
>     +# The following information is for reference only and not required by the
> build tools.
>     +#
>     +  VALID_ARCHITECTURES           = AARCH64
>     +#
>     +
>     +[Sources]
>     +  CloudHvAcpi.c
>     +
>     +[Packages]
>     +  MdePkg/MdePkg.dec
>     +  MdeModulePkg/MdeModulePkg.dec
>     +  OvmfPkg/OvmfPkg.dec
>     +  ArmVirtPkg/ArmVirtPkg.dec
>     +
>     +[LibraryClasses]
>     +  BaseLib
>     +  DebugLib
>     +  MemoryAllocationLib
>     +  OrderedCollectionLib
>     +  UefiBootServicesTableLib
>     +  UefiDriverEntryPoint
>     +
>     +[Protocols]
>     +  gEfiAcpiTableProtocolGuid                     # PROTOCOL ALWAYS_CONSUMED
>     +
>     +[Pcd]
>     +  gArmVirtTokenSpaceGuid.PcdCloudHvAcpiRsdpBaseAddress
>     +
>     +[Depex]
>     +  gEfiAcpiTableProtocolGuid
>     diff --git a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
> b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
>     new file mode 100644
>     index 000000000000..f5a47aa7f3cd
>     --- /dev/null
>     +++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
>     @@ -0,0 +1,155 @@
>     +/** @file
>     +  Install Acpi tables for Cloud Hypervisor
>     +
>     +  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
>     +
>     +  SPDX-License-Identifier: BSD-2-Clause-Patent
>     +**/
>     +
>     +#include <Library/BaseLib.h>
>     +#include <Library/MemoryAllocationLib.h>
>     +#include <IndustryStandard/Acpi63.h>
>     +#include <Protocol/AcpiTable.h>
>     +#include <Library/UefiBootServicesTableLib.h>
>     +#include <Library/UefiDriverEntryPoint.h>
>     +#include <Library/DebugLib.h>
>     +
>     +/**
>     +   Find Acpi table Protocol and return it
>     +
>     +   @return AcpiTable  Protocol, which is used to handle Acpi Table, on
> SUCCESS or NULL on FAILURE.
>     +
>     +**/
>     +STATIC
>     +EFI_ACPI_TABLE_PROTOCOL *
>     +FindAcpiTableProtocol (
>     +  VOID
>     +  )
>     +{
>     +  EFI_STATUS              Status;
>     +  EFI_ACPI_TABLE_PROTOCOL *AcpiTable;
>     +
>     +  Status = gBS->LocateProtocol (
>     +                  &gEfiAcpiTableProtocolGuid,
>     +                  NULL,
>     +                  (VOID**)&AcpiTable
>     +                  );
>     +  ASSERT_EFI_ERROR (Status);
>     +  return AcpiTable;
>     +}
>     +
>     +/** Install Acpi tables for Cloud Hypervisor
>     +
>     +  @param [in]  AcpiProtocol  Acpi Protocol which is used to install Acpi
> talbles
>     +
>     +  @return EFI_SUCCESS            The table was successfully inserted.
>     +  @return EFI_INVALID_PARAMETER  Either AcpiProtocol, AcpiTablePtr or
> DsdtPtr is NULL
>     +                                 and the size field embedded in the ACPI table pointed
>     +                                 by AcpiTablePtr or DsdtPtr are not in sync.
>     +  @return EFI_OUT_OF_RESOURCES   Insufficient resources exist to
> complete the request.
>     +  @return EFI_NOT_FOUND          DSDT table not found.
>     +**/
>     +EFI_STATUS
>     +EFIAPI
>     +InstallCloudHvAcpiTables (
>     + IN     EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
>     + )
>     +{
>     +  UINTN          InstalledKey;
>     +  UINTN          TableSize;
>     +  UINTN          AcpiTableLength;
>     +  UINT64         RsdpPtr;
>     +  UINT64         XsdtPtr;
>     +  UINT64         TableOffset;
>     +  UINT64         AcpiTablePtr;
>     +  UINT64         *DsdtPtr = NULL;
>     +  EFI_STATUS     Status;
>     +
>     +  if (AcpiProtocol == NULL) {
>     +      return EFI_INVALID_PARAMETER;
>     +  }
>     +
>     +  RsdpPtr = PcdGet64 (PcdCloudHvAcpiRsdpBaseAddress);
>     +  XsdtPtr = ((EFI_ACPI_6_3_ROOT_SYSTEM_DESCRIPTION_POINTER
> *)RsdpPtr)->XsdtAddress;
>     +  AcpiTableLength = ((EFI_ACPI_COMMON_HEADER *)XsdtPtr)->Length;
>     +  TableOffset = sizeof (EFI_ACPI_DESCRIPTION_HEADER);
>     +
>     +  while (TableOffset < AcpiTableLength) {
>     +    AcpiTablePtr = *(UINT64 *)(XsdtPtr + TableOffset);
>     +    TableSize = ((EFI_ACPI_COMMON_HEADER *)AcpiTablePtr)->Length;
>     +
>     +    //
>     +    // Install ACPI tables from XSDT
>     +    //
>     +    Status = AcpiProtocol->InstallAcpiTable (
>     +                             AcpiProtocol,
>     +                             (VOID *)AcpiTablePtr,
>     +                             TableSize,
>     +                             &InstalledKey
>     +                             );
>     +
>     +    if (EFI_ERROR(Status)) {
>     +        return Status;
>     +    }
>     +
>     +    TableOffset += sizeof (UINT64);
>     +
>     +    //
>     +    // Get DSDT from FADT
>     +    //
>     +    if (DsdtPtr == NULL
>     +      && !AsciiStrnCmp ((CHAR8 *)&((EFI_ACPI_COMMON_HEADER
> *)AcpiTablePtr)->Signature, "FACP", 4)) {
>     +      DsdtPtr = (UINT64
> *)(((EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE *)AcpiTablePtr)-
> >XDsdt);
>     +    }
>     +  }
>     +
>     +  if (DsdtPtr == NULL) {
>     +    DEBUG ((DEBUG_ERROR, "%a: no DSDT found\n", __FUNCTION__));
>     +    return EFI_NOT_FOUND;
>     +  }
>     +
>     +  //
>     +  // Install DSDT table
>     +  //
>     +  TableSize = ((EFI_ACPI_COMMON_HEADER *)DsdtPtr)->Length;
>     +  Status = AcpiProtocol->InstallAcpiTable (
>     +             AcpiProtocol,
>     +             DsdtPtr,
>     +             TableSize,
>     +             &InstalledKey
>     +             );
>     +
>     +  return Status;
>     +}
>     +
>     +/** Entry point for Cloud Hypervisor Platform Dxe
>     +
>     +  @param [in]  ImageHandle  Handle for this image.
>     +  @param [in]  SystemTable  Pointer to the EFI system table.
>     +
>     +  @return EFI_SUCCESS            The table was successfully inserted.
>     +  @return EFI_INVALID_PARAMETER  Either AcpiProtocol, AcpiTablePtr or
> DsdtPtr is NULL
>     +                                 and the size field embedded in the ACPI table pointed to
>     +                                 by AcpiTablePtr or DsdtPtr are not in sync.
>     +  @return EFI_OUT_OF_RESOURCES   Insufficient resources exist to
> complete the request.
>     +  @return EFI_NOT_FOUND          DSDT table not found
>     +**/
>     +EFI_STATUS
>     +EFIAPI
>     +CloudHvAcpiPlatformEntryPoint (
>     +  IN EFI_HANDLE         ImageHandle,
>     +  IN EFI_SYSTEM_TABLE   *SystemTable
>     +  )
>     +{
>     +  EFI_STATUS                         Status;
>     +
>     +  Status = InstallCloudHvAcpiTables (FindAcpiTableProtocol ());
>     +
>     +  if (EFI_ERROR(Status)) {
>     +     DEBUG ((DEBUG_ERROR, "%a: Fail to install Acpi table: %r\n",
> __FUNCTION__,
>     +       Status));
>     +     CpuDeadLoop ();
>     +  }
>     +
>     +  return EFI_SUCCESS;
>     +}
>     --
>     2.17.1
> 
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/3] Acpi: Install Acpi tables for Cloud hypervisor
  2021-07-07  1:42     ` Jianyong Wu
@ 2021-07-08  9:25       ` Sami Mujawar
  0 siblings, 0 replies; 10+ messages in thread
From: Sami Mujawar @ 2021-07-08  9:25 UTC (permalink / raw)
  To: Jianyong Wu, devel@edk2.groups.io
  Cc: lersek@redhat.com, ardb+tianocore@kernel.org, Justin He, nd

Merged as 4c051c2c65a8..0e3b6bd0ee75

Thanks.

Regards,

Sami Mujawar

On 07/07/2021, 02:43, "Jianyong Wu" <Jianyong.Wu@arm.com> wrote:

    Hi Sami,

    Thanks for your rework on my patch. I tried the change and it works well. You can do what you like on the patch set.

    Thanks
    Jianyong

    > -----Original Message-----
    > From: Sami Mujawar <Sami.Mujawar@arm.com>
    > Sent: Tuesday, July 6, 2021 4:52 PM
    > To: Jianyong Wu <Jianyong.Wu@arm.com>; devel@edk2.groups.io
    > Cc: lersek@redhat.com; ardb+tianocore@kernel.org; Justin He
    > <Justin.He@arm.com>; nd <nd@arm.com>
    > Subject: Re: [PATCH v4 2/3] Acpi: Install Acpi tables for Cloud hypervisor
    > 
    > Hi Jianyong,
    > 
    > I should have caught this earlier in my review.  However, if you agree, I will
    > do the following changes before pushing the patch.
    > 
    > 1.	The subject line of the commit message does not confirm to the edk2
    > coding standard. It should have ‘ArmVirtPkg: <subject line for the patch>’
    > 2.	The ACPI table signature can be simplified further. Can you try the
    > following and let me know if it works, please?
    > 
    > diff --git a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
    > b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
    > index f5a47aa7f3cd..51b012676e7d 100644
    > --- a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
    > +++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
    > @@ -52,8 +52,8 @@ FindAcpiTableProtocol (  EFI_STATUS  EFIAPI
    > InstallCloudHvAcpiTables (
    > - IN     EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
    > - )
    > +  IN     EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
    > +  )
    >  {
    >    UINTN          InstalledKey;
    >    UINTN          TableSize;
    > @@ -97,11 +97,12 @@ InstallCloudHvAcpiTables (
    >      //
    >      // Get DSDT from FADT
    >      //
    > -    if (DsdtPtr == NULL
    > -      && !AsciiStrnCmp ((CHAR8 *)&((EFI_ACPI_COMMON_HEADER
    > *)AcpiTablePtr)->Signature, "FACP", 4)) {
    > +    if ((DsdtPtr == NULL)
    > +      && (EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE ==
    > +          ((EFI_ACPI_COMMON_HEADER *)AcpiTablePtr)->Signature)) {
    >        DsdtPtr = (UINT64 *)(((EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE
    > *)AcpiTablePtr)->XDsdt);
    >      }
    > -  }
    > +  } // while
    > 
    >    if (DsdtPtr == NULL) {
    >      DEBUG ((DEBUG_ERROR, "%a: no DSDT found\n", __FUNCTION__));
    > 
    > Regards,
    > 
    > Sami Mujawar
    > 
    > On 05/07/2021, 11:07, "Jianyong Wu" <jianyong.wu@arm.com> wrote:
    > 
    >     There is no device like Fw-cfg in Qemu in Cloud Hypervisor, so a specific
    >     Acpi handler is introduced here.
    > 
    >     The handler implemented here is in a very simple way:
    >     1. acquire the RSDP from the PCD variable in the top ".dsc";
    >     2. get the XSDT address from RSDP structure;
    >     3. get the ACPI tables following the XSDT structure and install them
    >     one by one;
    >     4. get DSDT address from FADT and install DSDT table.
    > 
    >     Cc: Laszlo Ersek <lersek@redhat.com>
    >     Cc: Sami Mujawar <sami.mujawar@arm.com>
    > 
    >     Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
    >     ---
    >      ArmVirtPkg/ArmVirtPkg.dec                     |   6 +
    >      .../CloudHvAcpiPlatformDxe.inf                |  47 ++++++
    >      .../CloudHvAcpiPlatformDxe/CloudHvAcpi.c      | 155
    > ++++++++++++++++++
    >      3 files changed, 208 insertions(+)
    >      create mode 100644
    > ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
    >      create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
    > 
    >     diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
    >     index bf82f7f1f3f2..4e4d758015bc 100644
    >     --- a/ArmVirtPkg/ArmVirtPkg.dec
    >     +++ b/ArmVirtPkg/ArmVirtPkg.dec
    >     @@ -66,6 +66,12 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
    >        #
    >        gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6,
    > 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1,
    > 0x4D}|VOID*|0x00000007
    > 
    >     +  ##
    >     +  # This is the physical address of Rsdp which is the core struct of Acpi.
    >     +  # Cloud Hypervisor has no other way to pass Rsdp address to the guest
    > except use a PCD.
    >     +  #
    >     +
    > gArmVirtTokenSpaceGuid.PcdCloudHvAcpiRsdpBaseAddress|0x0|UINT64|0
    > x00000005
    >     +
    >      [PcdsDynamic]
    >        #
    >        # Whether to force disable ACPI, regardless of the fw_cfg settings
    >     diff --git
    > a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
    > b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
    >     new file mode 100644
    >     index 000000000000..01de76486686
    >     --- /dev/null
    >     +++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
    >     @@ -0,0 +1,47 @@
    >     +## @file
    >     +#  ACPI Platform Driver for Cloud Hypervisor
    >     +#
    >     +#  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
    >     +#  SPDX-License-Identifier: BSD-2-Clause-Patent
    >     +#
    >     +##
    >     +
    >     +[Defines]
    >     +  INF_VERSION                    = 0x00010005
    >     +  BASE_NAME                      = CloudHvgAcpiPlatform
    >     +  FILE_GUID                      = 6c76e407-73f2-dc1c-938f-5d6c4691ea93
    >     +  MODULE_TYPE                    = DXE_DRIVER
    >     +  VERSION_STRING                 = 1.0
    >     +  ENTRY_POINT                    = CloudHvAcpiPlatformEntryPoint
    >     +
    >     +#
    >     +# The following information is for reference only and not required by the
    > build tools.
    >     +#
    >     +  VALID_ARCHITECTURES           = AARCH64
    >     +#
    >     +
    >     +[Sources]
    >     +  CloudHvAcpi.c
    >     +
    >     +[Packages]
    >     +  MdePkg/MdePkg.dec
    >     +  MdeModulePkg/MdeModulePkg.dec
    >     +  OvmfPkg/OvmfPkg.dec
    >     +  ArmVirtPkg/ArmVirtPkg.dec
    >     +
    >     +[LibraryClasses]
    >     +  BaseLib
    >     +  DebugLib
    >     +  MemoryAllocationLib
    >     +  OrderedCollectionLib
    >     +  UefiBootServicesTableLib
    >     +  UefiDriverEntryPoint
    >     +
    >     +[Protocols]
    >     +  gEfiAcpiTableProtocolGuid                     # PROTOCOL ALWAYS_CONSUMED
    >     +
    >     +[Pcd]
    >     +  gArmVirtTokenSpaceGuid.PcdCloudHvAcpiRsdpBaseAddress
    >     +
    >     +[Depex]
    >     +  gEfiAcpiTableProtocolGuid
    >     diff --git a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
    > b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
    >     new file mode 100644
    >     index 000000000000..f5a47aa7f3cd
    >     --- /dev/null
    >     +++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
    >     @@ -0,0 +1,155 @@
    >     +/** @file
    >     +  Install Acpi tables for Cloud Hypervisor
    >     +
    >     +  Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
    >     +
    >     +  SPDX-License-Identifier: BSD-2-Clause-Patent
    >     +**/
    >     +
    >     +#include <Library/BaseLib.h>
    >     +#include <Library/MemoryAllocationLib.h>
    >     +#include <IndustryStandard/Acpi63.h>
    >     +#include <Protocol/AcpiTable.h>
    >     +#include <Library/UefiBootServicesTableLib.h>
    >     +#include <Library/UefiDriverEntryPoint.h>
    >     +#include <Library/DebugLib.h>
    >     +
    >     +/**
    >     +   Find Acpi table Protocol and return it
    >     +
    >     +   @return AcpiTable  Protocol, which is used to handle Acpi Table, on
    > SUCCESS or NULL on FAILURE.
    >     +
    >     +**/
    >     +STATIC
    >     +EFI_ACPI_TABLE_PROTOCOL *
    >     +FindAcpiTableProtocol (
    >     +  VOID
    >     +  )
    >     +{
    >     +  EFI_STATUS              Status;
    >     +  EFI_ACPI_TABLE_PROTOCOL *AcpiTable;
    >     +
    >     +  Status = gBS->LocateProtocol (
    >     +                  &gEfiAcpiTableProtocolGuid,
    >     +                  NULL,
    >     +                  (VOID**)&AcpiTable
    >     +                  );
    >     +  ASSERT_EFI_ERROR (Status);
    >     +  return AcpiTable;
    >     +}
    >     +
    >     +/** Install Acpi tables for Cloud Hypervisor
    >     +
    >     +  @param [in]  AcpiProtocol  Acpi Protocol which is used to install Acpi
    > talbles
    >     +
    >     +  @return EFI_SUCCESS            The table was successfully inserted.
    >     +  @return EFI_INVALID_PARAMETER  Either AcpiProtocol, AcpiTablePtr or
    > DsdtPtr is NULL
    >     +                                 and the size field embedded in the ACPI table pointed
    >     +                                 by AcpiTablePtr or DsdtPtr are not in sync.
    >     +  @return EFI_OUT_OF_RESOURCES   Insufficient resources exist to
    > complete the request.
    >     +  @return EFI_NOT_FOUND          DSDT table not found.
    >     +**/
    >     +EFI_STATUS
    >     +EFIAPI
    >     +InstallCloudHvAcpiTables (
    >     + IN     EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
    >     + )
    >     +{
    >     +  UINTN          InstalledKey;
    >     +  UINTN          TableSize;
    >     +  UINTN          AcpiTableLength;
    >     +  UINT64         RsdpPtr;
    >     +  UINT64         XsdtPtr;
    >     +  UINT64         TableOffset;
    >     +  UINT64         AcpiTablePtr;
    >     +  UINT64         *DsdtPtr = NULL;
    >     +  EFI_STATUS     Status;
    >     +
    >     +  if (AcpiProtocol == NULL) {
    >     +      return EFI_INVALID_PARAMETER;
    >     +  }
    >     +
    >     +  RsdpPtr = PcdGet64 (PcdCloudHvAcpiRsdpBaseAddress);
    >     +  XsdtPtr = ((EFI_ACPI_6_3_ROOT_SYSTEM_DESCRIPTION_POINTER
    > *)RsdpPtr)->XsdtAddress;
    >     +  AcpiTableLength = ((EFI_ACPI_COMMON_HEADER *)XsdtPtr)->Length;
    >     +  TableOffset = sizeof (EFI_ACPI_DESCRIPTION_HEADER);
    >     +
    >     +  while (TableOffset < AcpiTableLength) {
    >     +    AcpiTablePtr = *(UINT64 *)(XsdtPtr + TableOffset);
    >     +    TableSize = ((EFI_ACPI_COMMON_HEADER *)AcpiTablePtr)->Length;
    >     +
    >     +    //
    >     +    // Install ACPI tables from XSDT
    >     +    //
    >     +    Status = AcpiProtocol->InstallAcpiTable (
    >     +                             AcpiProtocol,
    >     +                             (VOID *)AcpiTablePtr,
    >     +                             TableSize,
    >     +                             &InstalledKey
    >     +                             );
    >     +
    >     +    if (EFI_ERROR(Status)) {
    >     +        return Status;
    >     +    }
    >     +
    >     +    TableOffset += sizeof (UINT64);
    >     +
    >     +    //
    >     +    // Get DSDT from FADT
    >     +    //
    >     +    if (DsdtPtr == NULL
    >     +      && !AsciiStrnCmp ((CHAR8 *)&((EFI_ACPI_COMMON_HEADER
    > *)AcpiTablePtr)->Signature, "FACP", 4)) {
    >     +      DsdtPtr = (UINT64
    > *)(((EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE *)AcpiTablePtr)-
    > >XDsdt);
    >     +    }
    >     +  }
    >     +
    >     +  if (DsdtPtr == NULL) {
    >     +    DEBUG ((DEBUG_ERROR, "%a: no DSDT found\n", __FUNCTION__));
    >     +    return EFI_NOT_FOUND;
    >     +  }
    >     +
    >     +  //
    >     +  // Install DSDT table
    >     +  //
    >     +  TableSize = ((EFI_ACPI_COMMON_HEADER *)DsdtPtr)->Length;
    >     +  Status = AcpiProtocol->InstallAcpiTable (
    >     +             AcpiProtocol,
    >     +             DsdtPtr,
    >     +             TableSize,
    >     +             &InstalledKey
    >     +             );
    >     +
    >     +  return Status;
    >     +}
    >     +
    >     +/** Entry point for Cloud Hypervisor Platform Dxe
    >     +
    >     +  @param [in]  ImageHandle  Handle for this image.
    >     +  @param [in]  SystemTable  Pointer to the EFI system table.
    >     +
    >     +  @return EFI_SUCCESS            The table was successfully inserted.
    >     +  @return EFI_INVALID_PARAMETER  Either AcpiProtocol, AcpiTablePtr or
    > DsdtPtr is NULL
    >     +                                 and the size field embedded in the ACPI table pointed to
    >     +                                 by AcpiTablePtr or DsdtPtr are not in sync.
    >     +  @return EFI_OUT_OF_RESOURCES   Insufficient resources exist to
    > complete the request.
    >     +  @return EFI_NOT_FOUND          DSDT table not found
    >     +**/
    >     +EFI_STATUS
    >     +EFIAPI
    >     +CloudHvAcpiPlatformEntryPoint (
    >     +  IN EFI_HANDLE         ImageHandle,
    >     +  IN EFI_SYSTEM_TABLE   *SystemTable
    >     +  )
    >     +{
    >     +  EFI_STATUS                         Status;
    >     +
    >     +  Status = InstallCloudHvAcpiTables (FindAcpiTableProtocol ());
    >     +
    >     +  if (EFI_ERROR(Status)) {
    >     +     DEBUG ((DEBUG_ERROR, "%a: Fail to install Acpi table: %r\n",
    > __FUNCTION__,
    >     +       Status));
    >     +     CpuDeadLoop ();
    >     +  }
    >     +
    >     +  return EFI_SUCCESS;
    >     +}
    >     --
    >     2.17.1
    > 
    > 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/3] Enable Cloud Hypervisor support in edk2
  2021-07-05 10:06 [PATCH v4 0/3] Enable Cloud Hypervisor support in edk2 Jianyong Wu
                   ` (2 preceding siblings ...)
  2021-07-05 10:06 ` [PATCH v4 3/3] ArmVirtCloudHv: support Cloud Hypervisor in edk2 Jianyong Wu
@ 2021-07-16 17:25 ` Ard Biesheuvel
  2021-07-16 17:27   ` Ard Biesheuvel
  3 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2021-07-16 17:25 UTC (permalink / raw)
  To: Jianyong Wu
  Cc: edk2-devel-groups-io, Laszlo Ersek, Sami Mujawar, Ard Biesheuvel,
	justin.he

On Mon, 5 Jul 2021 at 12:06, Jianyong Wu <jianyong.wu@arm.com> wrote:
>
> Cloud Hypervisor is an open source Virtual Machine Monitor (VMM) that
> runs on top of KVM. Cloud Hypervisor is implemented in Rust and is based
> on the rust-vmm crates. See [1] to find more.
>
> To support UEFI, Cloud Hypervisor is introduced here.
> There are 2 parts to be considered to do this enablement, that is:
>   1. specific ACPI service implementation compared with qemu, there is no
>      device like Fw-cfg, so we have no elegant way to get the RSDP address.
>      A specific ACPI implementation is introduced here.
>
>   2. build configuration file for Cloud Hypervisor
>
> Change log:
>
> v3 to v4:
>      (1) remove Tpm support in dsc file
>      (2) refine Acpi table install code base on Sami's comments in v3
>
> v2 to v3:
>      (1) reuse qemu's memory initialization lib as they are in nearly the same
> memory laout.
>      (2) split Acpi implemetation into PlatformHasAcpi and
> InstallAcpiTable.
>      (3) remove lots of dependencies from qemu like "*Fwcfg*" lib.
>      (4) lots of code cleanup work to let it more approach to edk2 code
> style.
>
> [1] https://github.com/cloud-hypervisor/cloud-hypervisor
>
> Jianyong Wu (3):
>   Acpi: reimplement PlatformHasAcpi for Cloud Hypervisor
>   Acpi: Install Acpi tables for Cloud hypervisor
>   ArmVirtCloudHv: support Cloud Hypervisor in edk2
>

Sami, any thoughts on this code?


>  ArmVirtPkg/ArmVirtPkg.dec                     |   6 +
>  ArmVirtPkg/ArmVirtCloudHv.dsc                 | 364 ++++++++++++++++++
>  ArmVirtPkg/ArmVirtCloudHv.fdf                 | 258 +++++++++++++
>  .../CloudHvAcpiPlatformDxe.inf                |  47 +++
>  .../CloudHvHasAcpiDtDxe.inf                   |  43 +++
>  .../CloudHvAcpiPlatformDxe/CloudHvAcpi.c      | 155 ++++++++
>  .../CloudHvHasAcpiDtDxe.c                     |  69 ++++
>  7 files changed, 942 insertions(+)
>  create mode 100644 ArmVirtPkg/ArmVirtCloudHv.dsc
>  create mode 100644 ArmVirtPkg/ArmVirtCloudHv.fdf
>  create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
>  create mode 100644 ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf
>  create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
>  create mode 100644 ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.c
>
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/3] Enable Cloud Hypervisor support in edk2
  2021-07-16 17:25 ` [PATCH v4 0/3] Enable Cloud Hypervisor support " Ard Biesheuvel
@ 2021-07-16 17:27   ` Ard Biesheuvel
  2021-07-19  8:37     ` Sami Mujawar
  0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2021-07-16 17:27 UTC (permalink / raw)
  To: Jianyong Wu
  Cc: edk2-devel-groups-io, Laszlo Ersek, Sami Mujawar, Ard Biesheuvel,
	justin.he

On Fri, 16 Jul 2021 at 19:25, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 5 Jul 2021 at 12:06, Jianyong Wu <jianyong.wu@arm.com> wrote:
> >
> > Cloud Hypervisor is an open source Virtual Machine Monitor (VMM) that
> > runs on top of KVM. Cloud Hypervisor is implemented in Rust and is based
> > on the rust-vmm crates. See [1] to find more.
> >
> > To support UEFI, Cloud Hypervisor is introduced here.
> > There are 2 parts to be considered to do this enablement, that is:
> >   1. specific ACPI service implementation compared with qemu, there is no
> >      device like Fw-cfg, so we have no elegant way to get the RSDP address.
> >      A specific ACPI implementation is introduced here.
> >
> >   2. build configuration file for Cloud Hypervisor
> >
> > Change log:
> >
> > v3 to v4:
> >      (1) remove Tpm support in dsc file
> >      (2) refine Acpi table install code base on Sami's comments in v3
> >
> > v2 to v3:
> >      (1) reuse qemu's memory initialization lib as they are in nearly the same
> > memory laout.
> >      (2) split Acpi implemetation into PlatformHasAcpi and
> > InstallAcpiTable.
> >      (3) remove lots of dependencies from qemu like "*Fwcfg*" lib.
> >      (4) lots of code cleanup work to let it more approach to edk2 code
> > style.
> >
> > [1] https://github.com/cloud-hypervisor/cloud-hypervisor
> >
> > Jianyong Wu (3):
> >   Acpi: reimplement PlatformHasAcpi for Cloud Hypervisor
> >   Acpi: Install Acpi tables for Cloud hypervisor
> >   ArmVirtCloudHv: support Cloud Hypervisor in edk2
> >
>
> Sami, any thoughts on this code?
>


... or did you already merge the entire series? (My mailbox is
overflowing a bit after 4 weeks of vacation :-))


>
> >  ArmVirtPkg/ArmVirtPkg.dec                     |   6 +
> >  ArmVirtPkg/ArmVirtCloudHv.dsc                 | 364 ++++++++++++++++++
> >  ArmVirtPkg/ArmVirtCloudHv.fdf                 | 258 +++++++++++++
> >  .../CloudHvAcpiPlatformDxe.inf                |  47 +++
> >  .../CloudHvHasAcpiDtDxe.inf                   |  43 +++
> >  .../CloudHvAcpiPlatformDxe/CloudHvAcpi.c      | 155 ++++++++
> >  .../CloudHvHasAcpiDtDxe.c                     |  69 ++++
> >  7 files changed, 942 insertions(+)
> >  create mode 100644 ArmVirtPkg/ArmVirtCloudHv.dsc
> >  create mode 100644 ArmVirtPkg/ArmVirtCloudHv.fdf
> >  create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
> >  create mode 100644 ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf
> >  create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
> >  create mode 100644 ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.c
> >
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/3] Enable Cloud Hypervisor support in edk2
  2021-07-16 17:27   ` Ard Biesheuvel
@ 2021-07-19  8:37     ` Sami Mujawar
  0 siblings, 0 replies; 10+ messages in thread
From: Sami Mujawar @ 2021-07-19  8:37 UTC (permalink / raw)
  To: Ard Biesheuvel, Jianyong Wu
  Cc: edk2-devel-groups-io, Laszlo Ersek, Ard Biesheuvel, Justin He

Hi Ard,

On 16/07/2021, 18:28, "Ard Biesheuvel" <ardb@kernel.org> wrote:

    On Fri, 16 Jul 2021 at 19:25, Ard Biesheuvel <ardb@kernel.org> wrote:
    >
    > On Mon, 5 Jul 2021 at 12:06, Jianyong Wu <jianyong.wu@arm.com> wrote:
    > >
    > > Cloud Hypervisor is an open source Virtual Machine Monitor (VMM) that
    > > runs on top of KVM. Cloud Hypervisor is implemented in Rust and is based
    > > on the rust-vmm crates. See [1] to find more.
    > >
    > > To support UEFI, Cloud Hypervisor is introduced here.
    > > There are 2 parts to be considered to do this enablement, that is:
    > >   1. specific ACPI service implementation compared with qemu, there is no
    > >      device like Fw-cfg, so we have no elegant way to get the RSDP address.
    > >      A specific ACPI implementation is introduced here.
    > >
    > >   2. build configuration file for Cloud Hypervisor
    > >
    > > Change log:
    > >
    > > v3 to v4:
    > >      (1) remove Tpm support in dsc file
    > >      (2) refine Acpi table install code base on Sami's comments in v3
    > >
    > > v2 to v3:
    > >      (1) reuse qemu's memory initialization lib as they are in nearly the same
    > > memory laout.
    > >      (2) split Acpi implemetation into PlatformHasAcpi and
    > > InstallAcpiTable.
    > >      (3) remove lots of dependencies from qemu like "*Fwcfg*" lib.
    > >      (4) lots of code cleanup work to let it more approach to edk2 code
    > > style.
    > >
    > > [1] https://github.com/cloud-hypervisor/cloud-hypervisor
    > >
    > > Jianyong Wu (3):
    > >   Acpi: reimplement PlatformHasAcpi for Cloud Hypervisor
    > >   Acpi: Install Acpi tables for Cloud hypervisor
    > >   ArmVirtCloudHv: support Cloud Hypervisor in edk2
    > >
    >
    > Sami, any thoughts on this code?
    >


    ... or did you already merge the entire series? (My mailbox is
    overflowing a bit after 4 weeks of vacation :-))
[SAMI] I have merged this series.

Regards,

Sami Mujawar


    >
    > >  ArmVirtPkg/ArmVirtPkg.dec                     |   6 +
    > >  ArmVirtPkg/ArmVirtCloudHv.dsc                 | 364 ++++++++++++++++++
    > >  ArmVirtPkg/ArmVirtCloudHv.fdf                 | 258 +++++++++++++
    > >  .../CloudHvAcpiPlatformDxe.inf                |  47 +++
    > >  .../CloudHvHasAcpiDtDxe.inf                   |  43 +++
    > >  .../CloudHvAcpiPlatformDxe/CloudHvAcpi.c      | 155 ++++++++
    > >  .../CloudHvHasAcpiDtDxe.c                     |  69 ++++
    > >  7 files changed, 942 insertions(+)
    > >  create mode 100644 ArmVirtPkg/ArmVirtCloudHv.dsc
    > >  create mode 100644 ArmVirtPkg/ArmVirtCloudHv.fdf
    > >  create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
    > >  create mode 100644 ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf
    > >  create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
    > >  create mode 100644 ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.c
    > >
    > > --
    > > 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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-07-19  8:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-05 10:06 [PATCH v4 0/3] Enable Cloud Hypervisor support in edk2 Jianyong Wu
2021-07-05 10:06 ` [PATCH v4 1/3] Acpi: reimplement PlatformHasAcpi for Cloud Hypervisor Jianyong Wu
2021-07-05 10:06 ` [PATCH v4 2/3] Acpi: Install Acpi tables for Cloud hypervisor Jianyong Wu
2021-07-06  8:52   ` Sami Mujawar
2021-07-07  1:42     ` Jianyong Wu
2021-07-08  9:25       ` Sami Mujawar
2021-07-05 10:06 ` [PATCH v4 3/3] ArmVirtCloudHv: support Cloud Hypervisor in edk2 Jianyong Wu
2021-07-16 17:25 ` [PATCH v4 0/3] Enable Cloud Hypervisor support " Ard Biesheuvel
2021-07-16 17:27   ` Ard Biesheuvel
2021-07-19  8:37     ` Sami Mujawar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox