[edk2-devel] [PATCH v4 2/3] Acpi: Install Acpi tables for Cloud hypervisor

Sami Mujawar sami.mujawar at arm.com
Tue Jul 6 08:52:18 UTC 2021


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 at 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 at redhat.com>
    Cc: Sami Mujawar <sami.mujawar at arm.com>

    Signed-off-by: Jianyong Wu <jianyong.wu at 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




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77503): https://edk2.groups.io/g/devel/message/77503
Mute This Topic: https://groups.io/mt/83994103/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-






More information about the edk2-devel-archive mailing list