-----Original Message-----

> From: Sami Mujawar <Sami.Mujawar@...>
> Sent: Tuesday, July 6, 2021 4:52 PM
> To: Jianyong Wu <Jianyong.Wu@...>; devel@edk2.groups.io
> Cc: lersek@...; ardb+tianocore@...; Justin He
> <Justin.He@...>; nd <nd@...>
> 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@...> 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@...>
> Cc: Sami Mujawar <sami.mujawar@...>
>
> Signed-off-by: Jianyong Wu <jianyong.wu@...>
> ---
> 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
>
>