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

Sami Mujawar sami.mujawar at arm.com
Thu Jul 8 09:25:15 UTC 2021


Merged as 4c051c2c65a8..0e3b6bd0ee75

Thanks.

Regards,

Sami Mujawar

On 07/07/2021, 02:43, "Jianyong Wu" <Jianyong.Wu at 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 at arm.com>
    > Sent: Tuesday, July 6, 2021 4:52 PM
    > To: Jianyong Wu <Jianyong.Wu at arm.com>; devel at edk2.groups.io
    > Cc: lersek at redhat.com; ardb+tianocore at kernel.org; Justin He
    > <Justin.He at arm.com>; nd <nd at 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 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|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
    > 
    > 




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77591): https://edk2.groups.io/g/devel/message/77591
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