diff mbox

OvmfPkg: Add ACPI support for Virt Xen ARM

Message ID 1464670786-10424-1-git-send-email-zhaoshenglong@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shannon Zhao May 31, 2016, 4:59 a.m. UTC
From: Shannon Zhao <shannon.zhao@linaro.org>

Add ACPI support for Virt Xen ARM and it gets the ACPI tables through
Xen ARM multiboot protocol.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
The corresponding Xen patches can be fetched from:
http://git.linaro.org/people/shannon.zhao/xen.git/shortlog/refs/heads/domu_acpi
---
 ArmVirtPkg/ArmVirtXen.dsc                         |   6 +
 ArmVirtPkg/ArmVirtXen.fdf                         |   6 +
 OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h            |   6 +
 OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c              | 207 ++++++++++++++++++++++
 OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatform.c      |  38 ++++
 OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf |  59 ++++++
 6 files changed, 322 insertions(+)
 create mode 100644 OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c
 create mode 100644 OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatform.c
 create mode 100644 OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf

Comments

Laszlo Ersek May 31, 2016, 10:35 a.m. UTC | #1
On 05/31/16 06:59, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add ACPI support for Virt Xen ARM and it gets the ACPI tables through
> Xen ARM multiboot protocol.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> The corresponding Xen patches can be fetched from:
> http://git.linaro.org/people/shannon.zhao/xen.git/shortlog/refs/heads/domu_acpi
> ---
>  ArmVirtPkg/ArmVirtXen.dsc                         |   6 +
>  ArmVirtPkg/ArmVirtXen.fdf                         |   6 +
>  OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h            |   6 +
>  OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c              | 207 ++++++++++++++++++++++
>  OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatform.c      |  38 ++++
>  OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf |  59 ++++++
>  6 files changed, 322 insertions(+)
>  create mode 100644 OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c
>  create mode 100644 OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatform.c
>  create mode 100644 OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf

Jordan and I might disagree about this patch, but here's my comments
about it:

I don't think this patch belongs under OvmfPkg. Namely, the only file
that XenArmAcpiPlatformDxe reuses from the existing
OvmfPkg/AcpiPlatformDxe/ directory is "EntryPoint.c", which is (a)
trivial, (b) its conditions and branches should be possible to evaluate
statically for aarch64 Xen. (PcdPciDisableBusEnumeration should be set
to TRUE in ArmVirtXen.dsc.)

Second, the new code uses the FDT library directly (from EmbeddedPkg),
and accesses QEMU's DTB directly (with the fdt_*() APIs). However, in
ArmVirtPkg we have moved away from this, and now we have a custom
(edk2-only, non-standard) FdtClient protocol for accessing the FDT.
Please see:
- ArmVirtPkg/Include/Protocol/FdtClient.h
- ArmVirtPkg/FdtClientDxe/

In order to rebase this patch to the FDT Client Protocol, while keeping
it under OvmfPkg, the "XenArmAcpiPlatformDxe.inf" file would have to
list "ArmVirtPkg/ArmVirtPkg.dec" in the [Packages] section. I think that
would be very ugly. Thus far we have managed to avoid mutual references
between OvmfPkg and ArmVirtPkg: OvmfPkg doesn't consume anything from
ArmVirtPkg, and that's how things should stay in my opinion.

Which is why I suggest to implement this functionality entirely under
ArmVirtPkg, and using the FDT Client Protocol at that.

If a non-trivial chunk of functionality can be identified between
OvmfPkg and ArmVirtPkg in this regard (that currently exists under
OvmfPkg/AcpiPlatformDxe/), then it should be extracted into a library
(under OvmfPkg/Include/Library and OvmfPkg/Library), and the ArmVirtPkg
code should become a client of that library. (You can find many similar
OvmfPkg/Library/ resolutions in the ArmVirtPkg/ DSC files.)

NB: Ard is going to be on vacation for a while, and I think we should
definitely await his feedback on this. First, my participation in
ArmVirtXen matters is quite minimal. Second, Ard designed the FDT Client
Protocol; in case you need extensions to the current APIs for
implementing the feature, then Ard is the one to review them.

Thanks,
Laszlo

> diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc
> index 594ca64..a0d197f 100644
> --- a/ArmVirtPkg/ArmVirtXen.dsc
> +++ b/ArmVirtPkg/ArmVirtXen.dsc
> @@ -216,3 +216,9 @@
>  
>    OvmfPkg/XenBusDxe/XenBusDxe.inf
>    OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> +
> +  #
> +  # ACPI Support
> +  #
> +  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
> +  OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf
> diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf
> index 13412f9..da30e87 100644
> --- a/ArmVirtPkg/ArmVirtXen.fdf
> +++ b/ArmVirtPkg/ArmVirtXen.fdf
> @@ -179,6 +179,12 @@ READ_LOCK_STATUS   = TRUE
>    INF OvmfPkg/XenBusDxe/XenBusDxe.inf
>    INF OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
>  
> +  #
> +  # ACPI Support
> +  #
> +  INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
> +  INF OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf
> +
>  [FV.FVMAIN_COMPACT]
>  FvAlignment        = 16
>  ERASE_POLARITY     = 1
> diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
> index 08dd7f8..325d7e6 100644
> --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
> +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
> @@ -69,6 +69,12 @@ InstallXenTables (
>  
>  EFI_STATUS
>  EFIAPI
> +InstallXenArmTables (
> +  IN   EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
> +  );
> +
> +EFI_STATUS
> +EFIAPI
>  InstallQemuFwCfgTables (
>    IN   EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
>    );
> diff --git a/OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c b/OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c
> new file mode 100644
> index 0000000..c3a351c
> --- /dev/null
> +++ b/OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c
> @@ -0,0 +1,207 @@
> +/** @file
> +  OVMF ACPI Xen ARM support
> +
> +  Copyright (C) 2016, Linaro Ltd. All rights reserved.
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD License
> +  which accompanies this distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/ 
> +
> +#include "AcpiPlatform.h"
> +#include <Library/HobLib.h>
> +#include <Library/PcdLib.h>
> +#include <Guid/XenInfo.h>
> +#include <Library/BaseLib.h>
> +#include <libfdt.h>
> +#include <Library/DxeServicesTableLib.h>
> +
> +EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *XenAcpiRsdpStructurePtr = NULL;
> +
> +/**
> +  Get the address of Xen ACPI Root System Description Pointer (RSDP)
> +  structure.
> +
> +  @param  RsdpStructurePtr   Return pointer to RSDP structure
> +
> +  @return EFI_SUCCESS        Find Xen RSDP structure successfully.
> +  @return EFI_NOT_FOUND      Don't find Xen RSDP structure.
> +  @return EFI_ABORTED        Find Xen RSDP structure, but it's not integrated.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetXenArmAcpiRsdp (
> +  OUT   EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER   **RsdpPtr
> +  )
> +{
> +  VOID                                           *Hob;
> +  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER   *RsdpStructurePtr;
> +  VOID                                           *DeviceTreeBase;
> +  INT32                                          Node, Depth, Len;
> +  CONST CHAR8                                    *Type;
> +  CONST VOID                                     *RegProp;
> +
> +  RsdpStructurePtr = NULL;
> +  //
> +  // Get the RSDP structure address from DeviceTree
> +  //
> +  Hob = GetFirstGuidHob(&gFdtHobGuid);
> +  if (Hob == NULL || GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64)) {
> +    DEBUG ((EFI_D_ERROR, "%a: Failed to get Fdt Hob\n", __FUNCTION__));
> +    return EFI_NOT_FOUND;
> +  }
> +  DeviceTreeBase = (VOID *)(UINTN)*(UINT64 *)GET_GUID_HOB_DATA (Hob);
> +
> +  if (fdt_check_header (DeviceTreeBase) != 0) {
> +    DEBUG ((EFI_D_ERROR, "%a: No DTB found @ 0x%p\n", __FUNCTION__, DeviceTreeBase));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  Node = fdt_path_offset(DeviceTreeBase, "/chosen/modules");
> +  if ( Node < 0 ) {
> +    DEBUG ((EFI_D_ERROR, "%a: NO /chosen/modules found\n", __FUNCTION__));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +
> +  for (Depth = 0;
> +       (Node >= 0) && (Depth >= 0);
> +       Node = fdt_next_node (DeviceTreeBase, Node, &Depth)) {
> +    if (Depth == 1) {
> +        Type = fdt_getprop (DeviceTreeBase, Node, "compatible", &Len);
> +        if (fdt_stringlist_contains (Type, Len, "xen,linux-acpi")) {
> +          RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
> +          ASSERT (Len == 2 * sizeof (UINT64));
> +          RsdpStructurePtr = (EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *)
> +                               (UINTN) fdt64_to_cpu (((UINT64 *)RegProp)[0]);
> +        }
> +    }
> +  }
> +
> +  if (RsdpStructurePtr && RsdpStructurePtr->Revision >= 2) {
> +    *RsdpPtr = RsdpStructurePtr;
> +    return EFI_SUCCESS;
> +  }
> +
> +  return EFI_NOT_FOUND;
> +}
> +
> +/**
> +  Get Xen Acpi tables from the RSDP structure. And installs Xen ACPI tables
> +  into the RSDT/XSDT using InstallAcpiTable. Some signature of the installed
> +  ACPI tables are: FACP, APIC, GTDT, DSDT.
> +
> +  @param  AcpiProtocol           Protocol instance pointer.
> +
> +  @return EFI_SUCCESS            The table was successfully inserted.
> +  @return EFI_INVALID_PARAMETER  Either AcpiTableBuffer is NULL, TableHandle is
> +                                 NULL, or AcpiTableBufferSize and the size
> +                                 field embedded in the ACPI table pointed to
> +                                 by AcpiTableBuffer are not in sync.
> +  @return EFI_OUT_OF_RESOURCES   Insufficient resources exist to complete the request.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +InstallXenArmTables (
> +  IN   EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
> +  )
> +{
> +  EFI_STATUS                                       Status;
> +  UINTN                                            TableHandle;
> +
> +  EFI_ACPI_DESCRIPTION_HEADER                      *Xsdt;
> +  VOID                                             *CurrentTableEntry;
> +  UINTN                                            CurrentTablePointer;
> +  EFI_ACPI_DESCRIPTION_HEADER                      *CurrentTable;
> +  UINTN                                            Index;
> +  UINTN                                            NumberOfTableEntries;
> +  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE        *FadtTable;
> +  EFI_ACPI_DESCRIPTION_HEADER                      *DsdtTable;
> +
> +  FadtTable   = NULL;
> +  DsdtTable   = NULL;
> +  TableHandle = 0;
> +  NumberOfTableEntries = 0;
> +
> +  //
> +  // Try to find Xen ARM ACPI tables
> +  //
> +  Status = GetXenArmAcpiRsdp (&XenAcpiRsdpStructurePtr);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_INFO, "%a: No RSDP table found\n", __FUNCTION__));
> +    return Status;
> +  }
> +
> +  //
> +  // If XSDT table is find, just install its tables. 
> +  //
> +  if (XenAcpiRsdpStructurePtr->XsdtAddress) {
> +    //
> +    // Retrieve the addresses of XSDT and 
> +    // calculate the number of its table entries.
> +    //
> +    Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN)
> +             XenAcpiRsdpStructurePtr->XsdtAddress;
> +    NumberOfTableEntries = (Xsdt->Length -
> +                             sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
> +                             sizeof (UINT64);
> +    //
> +    // Install ACPI tables found in XSDT.
> +    //
> +    for (Index = 0; Index < NumberOfTableEntries; Index++) {
> +      //
> +      // Get the table entry from XSDT
> +      //
> +      CurrentTableEntry = (VOID *) ((UINT8 *) Xsdt +
> +                            sizeof (EFI_ACPI_DESCRIPTION_HEADER) +
> +                            Index * sizeof (UINT64));
> +      CurrentTablePointer = (UINTN) *(UINT64 *)CurrentTableEntry;
> +      CurrentTable = (EFI_ACPI_DESCRIPTION_HEADER *) CurrentTablePointer;
> +
> +      //
> +      // Install the XSDT tables
> +      //
> +      Status = AcpiProtocol->InstallAcpiTable (
> +                 AcpiProtocol,
> +                 CurrentTable,
> +                 CurrentTable->Length,
> +                 &TableHandle
> +                 );
> +
> +      if (EFI_ERROR (Status)) {
> +        return Status;
> +      }
> +
> +      //
> +      // Get the FACS and DSDT table address from the table FADT
> +      //
> +      if (!AsciiStrnCmp ((CHAR8 *) &CurrentTable->Signature, "FACP", 4)) {
> +        FadtTable = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *)
> +                      (UINTN) CurrentTablePointer;
> +        DsdtTable  = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) FadtTable->Dsdt;
> +      }
> +    }
> +  }
> +
> +  //
> +  // Install DSDT table.
> +  //
> +  Status = AcpiProtocol->InstallAcpiTable (
> +             AcpiProtocol,
> +             DsdtTable,
> +             DsdtTable->Length,
> +             &TableHandle
> +             );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatform.c b/OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatform.c
> new file mode 100644
> index 0000000..7f141a9
> --- /dev/null
> +++ b/OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatform.c
> @@ -0,0 +1,38 @@
> +/** @file
> +  OVMF ACPI Platform Driver using Xen ARM multiboot protocol
> +
> +  Copyright (C) 2016, Linaro Ltd. All rights reserved.
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +**/
> +
> +#include "AcpiPlatform.h"
> +
> +/**
> +  Effective entrypoint of Xen ARM Acpi Platform driver.
> +
> +  @param  ImageHandle
> +  @param  SystemTable
> +
> +  @return EFI_SUCCESS
> +  @return EFI_LOAD_ERROR
> +  @return EFI_OUT_OF_RESOURCES
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +InstallAcpiTables (
> +  IN   EFI_ACPI_TABLE_PROTOCOL       *AcpiTable
> +  )
> +{
> +  EFI_STATUS                         Status;
> +
> +  Status = InstallXenArmTables (AcpiTable);
> +  return Status;
> +}
> diff --git a/OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf
> new file mode 100644
> index 0000000..d96e9a1
> --- /dev/null
> +++ b/OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf
> @@ -0,0 +1,59 @@
> +## @file
> +#  OVMF ACPI Platform Driver using Xen ARM multiboot protocol
> +#
> +#  Copyright (C) 2016, Linaro Ltd. All rights reserved.
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD License
> +#  which accompanies this distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = XenArmAcpiPlatform
> +  FILE_GUID                      = 0efc6282-f1e5-469a-8a70-194a8761f9aa
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = AcpiPlatformEntryPoint
> +
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +#  VALID_ARCHITECTURES           = AARCH64
> +#
> +
> +[Sources]
> +  XenArmAcpiPlatform.c
> +  XenArmAcpi.c
> +  EntryPoint.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  HobLib
> +  FdtLib
> +  UefiDriverEntryPoint
> +
> +[Protocols]
> +  gEfiAcpiTableProtocolGuid                     # PROTOCOL ALWAYS_CONSUMED
> +  gEfiPciEnumerationCompleteProtocolGuid        # PROTOCOL SOMETIMES_CONSUMED
> +
> +[Pcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
> +
> +[Depex]
> +  gEfiAcpiTableProtocolGuid
> +
> +[Guids]
> +  gFdtHobGuid
>
Julien Grall June 7, 2016, 1:50 p.m. UTC | #2
Hello Shannon,

On 31/05/16 05:59, Shannon Zhao wrote:
> +EFI_STATUS
> +EFIAPI
> +GetXenArmAcpiRsdp (
> +  OUT   EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER   **RsdpPtr
> +  )
> +{
> +  VOID                                           *Hob;
> +  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER   *RsdpStructurePtr;
> +  VOID                                           *DeviceTreeBase;
> +  INT32                                          Node, Depth, Len;
> +  CONST CHAR8                                    *Type;
> +  CONST VOID                                     *RegProp;
> +
> +  RsdpStructurePtr = NULL;
> +  //
> +  // Get the RSDP structure address from DeviceTree
> +  //
> +  Hob = GetFirstGuidHob(&gFdtHobGuid);
> +  if (Hob == NULL || GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64)) {
> +    DEBUG ((EFI_D_ERROR, "%a: Failed to get Fdt Hob\n", __FUNCTION__));
> +    return EFI_NOT_FOUND;
> +  }
> +  DeviceTreeBase = (VOID *)(UINTN)*(UINT64 *)GET_GUID_HOB_DATA (Hob);
> +
> +  if (fdt_check_header (DeviceTreeBase) != 0) {
> +    DEBUG ((EFI_D_ERROR, "%a: No DTB found @ 0x%p\n", __FUNCTION__, DeviceTreeBase));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  Node = fdt_path_offset(DeviceTreeBase, "/chosen/modules");

I am not sure if we want to mandate the modules to live in "/chosen". 
Would it be possible to look by compatible instead?

Regards,
Shannon Zhao June 23, 2016, 9:24 a.m. UTC | #3
On 2016/5/31 18:35, Laszlo Ersek wrote:
> On 05/31/16 06:59, Shannon Zhao wrote:
>> > From: Shannon Zhao <shannon.zhao@linaro.org>
>> > 
>> > Add ACPI support for Virt Xen ARM and it gets the ACPI tables through
>> > Xen ARM multiboot protocol.
>> > 
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> > ---
>> > The corresponding Xen patches can be fetched from:
>> > http://git.linaro.org/people/shannon.zhao/xen.git/shortlog/refs/heads/domu_acpi
>> > ---
>> >  ArmVirtPkg/ArmVirtXen.dsc                         |   6 +
>> >  ArmVirtPkg/ArmVirtXen.fdf                         |   6 +
>> >  OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h            |   6 +
>> >  OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c              | 207 ++++++++++++++++++++++
>> >  OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatform.c      |  38 ++++
>> >  OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf |  59 ++++++
>> >  6 files changed, 322 insertions(+)
>> >  create mode 100644 OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c
>> >  create mode 100644 OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatform.c
>> >  create mode 100644 OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf
> Jordan and I might disagree about this patch, but here's my comments
> about it:
> 
> I don't think this patch belongs under OvmfPkg. Namely, the only file
> that XenArmAcpiPlatformDxe reuses from the existing
> OvmfPkg/AcpiPlatformDxe/ directory is "EntryPoint.c", which is (a)
> trivial, (b) its conditions and branches should be possible to evaluate
> statically for aarch64 Xen. (PcdPciDisableBusEnumeration should be set
> to TRUE in ArmVirtXen.dsc.)
> 
> Second, the new code uses the FDT library directly (from EmbeddedPkg),
> and accesses QEMU's DTB directly (with the fdt_*() APIs). However, in
> ArmVirtPkg we have moved away from this, and now we have a custom
> (edk2-only, non-standard) FdtClient protocol for accessing the FDT.
> Please see:
> - ArmVirtPkg/Include/Protocol/FdtClient.h
> - ArmVirtPkg/FdtClientDxe/
> 
> In order to rebase this patch to the FDT Client Protocol, while keeping
> it under OvmfPkg, the "XenArmAcpiPlatformDxe.inf" file would have to
> list "ArmVirtPkg/ArmVirtPkg.dec" in the [Packages] section. I think that
> would be very ugly. Thus far we have managed to avoid mutual references
> between OvmfPkg and ArmVirtPkg: OvmfPkg doesn't consume anything from
> ArmVirtPkg, and that's how things should stay in my opinion.
> 
> Which is why I suggest to implement this functionality entirely under
> ArmVirtPkg, and using the FDT Client Protocol at that.
> 
Thanks for your explanation. I will move this into ArmVirtPkg.

> If a non-trivial chunk of functionality can be identified between
> OvmfPkg and ArmVirtPkg in this regard (that currently exists under
> OvmfPkg/AcpiPlatformDxe/), then it should be extracted into a library
> (under OvmfPkg/Include/Library and OvmfPkg/Library), and the ArmVirtPkg
> code should become a client of that library. (You can find many similar
> OvmfPkg/Library/ resolutions in the ArmVirtPkg/ DSC files.)
> 
> NB: Ard is going to be on vacation for a while, and I think we should
> definitely await his feedback on this. First, my participation in
> ArmVirtXen matters is quite minimal. Second, Ard designed the FDT Client
> Protocol; in case you need extensions to the current APIs for
> implementing the feature, then Ard is the one to review them.
I think it will only use the existing FindCompatibleNodeReg() function
of FDT Client. So I'll move on next version.

Thanks,
Shannon Zhao June 23, 2016, 9:25 a.m. UTC | #4
On 2016/6/7 21:50, Julien Grall wrote:
> 
> On 31/05/16 05:59, Shannon Zhao wrote:
>> +EFI_STATUS
>> +EFIAPI
>> +GetXenArmAcpiRsdp (
>> +  OUT   EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER   **RsdpPtr
>> +  )
>> +{
>> +  VOID                                           *Hob;
>> +  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER   *RsdpStructurePtr;
>> +  VOID                                           *DeviceTreeBase;
>> +  INT32                                          Node, Depth, Len;
>> +  CONST CHAR8                                    *Type;
>> +  CONST VOID                                     *RegProp;
>> +
>> +  RsdpStructurePtr = NULL;
>> +  //
>> +  // Get the RSDP structure address from DeviceTree
>> +  //
>> +  Hob = GetFirstGuidHob(&gFdtHobGuid);
>> +  if (Hob == NULL || GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64)) {
>> +    DEBUG ((EFI_D_ERROR, "%a: Failed to get Fdt Hob\n", __FUNCTION__));
>> +    return EFI_NOT_FOUND;
>> +  }
>> +  DeviceTreeBase = (VOID *)(UINTN)*(UINT64 *)GET_GUID_HOB_DATA (Hob);
>> +
>> +  if (fdt_check_header (DeviceTreeBase) != 0) {
>> +    DEBUG ((EFI_D_ERROR, "%a: No DTB found @ 0x%p\n", __FUNCTION__,
>> DeviceTreeBase));
>> +    return EFI_NOT_FOUND;
>> +  }
>> +
>> +  Node = fdt_path_offset(DeviceTreeBase, "/chosen/modules");
> 
> I am not sure if we want to mandate the modules to live in "/chosen".
> Would it be possible to look by compatible instead?
Sure, I will use the compatible string to find the DT node at next version.

Thanks,
diff mbox

Patch

diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc
index 594ca64..a0d197f 100644
--- a/ArmVirtPkg/ArmVirtXen.dsc
+++ b/ArmVirtPkg/ArmVirtXen.dsc
@@ -216,3 +216,9 @@ 
 
   OvmfPkg/XenBusDxe/XenBusDxe.inf
   OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
+
+  #
+  # ACPI Support
+  #
+  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+  OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf
diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf
index 13412f9..da30e87 100644
--- a/ArmVirtPkg/ArmVirtXen.fdf
+++ b/ArmVirtPkg/ArmVirtXen.fdf
@@ -179,6 +179,12 @@  READ_LOCK_STATUS   = TRUE
   INF OvmfPkg/XenBusDxe/XenBusDxe.inf
   INF OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
 
+  #
+  # ACPI Support
+  #
+  INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+  INF OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf
+
 [FV.FVMAIN_COMPACT]
 FvAlignment        = 16
 ERASE_POLARITY     = 1
diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
index 08dd7f8..325d7e6 100644
--- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
@@ -69,6 +69,12 @@  InstallXenTables (
 
 EFI_STATUS
 EFIAPI
+InstallXenArmTables (
+  IN   EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
+  );
+
+EFI_STATUS
+EFIAPI
 InstallQemuFwCfgTables (
   IN   EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
   );
diff --git a/OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c b/OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c
new file mode 100644
index 0000000..c3a351c
--- /dev/null
+++ b/OvmfPkg/AcpiPlatformDxe/XenArmAcpi.c
@@ -0,0 +1,207 @@ 
+/** @file
+  OVMF ACPI Xen ARM support
+
+  Copyright (C) 2016, Linaro Ltd. All rights reserved.
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/ 
+
+#include "AcpiPlatform.h"
+#include <Library/HobLib.h>
+#include <Library/PcdLib.h>
+#include <Guid/XenInfo.h>
+#include <Library/BaseLib.h>
+#include <libfdt.h>
+#include <Library/DxeServicesTableLib.h>
+
+EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *XenAcpiRsdpStructurePtr = NULL;
+
+/**
+  Get the address of Xen ACPI Root System Description Pointer (RSDP)
+  structure.
+
+  @param  RsdpStructurePtr   Return pointer to RSDP structure
+
+  @return EFI_SUCCESS        Find Xen RSDP structure successfully.
+  @return EFI_NOT_FOUND      Don't find Xen RSDP structure.
+  @return EFI_ABORTED        Find Xen RSDP structure, but it's not integrated.
+
+**/
+EFI_STATUS
+EFIAPI
+GetXenArmAcpiRsdp (
+  OUT   EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER   **RsdpPtr
+  )
+{
+  VOID                                           *Hob;
+  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER   *RsdpStructurePtr;
+  VOID                                           *DeviceTreeBase;
+  INT32                                          Node, Depth, Len;
+  CONST CHAR8                                    *Type;
+  CONST VOID                                     *RegProp;
+
+  RsdpStructurePtr = NULL;
+  //
+  // Get the RSDP structure address from DeviceTree
+  //
+  Hob = GetFirstGuidHob(&gFdtHobGuid);
+  if (Hob == NULL || GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64)) {
+    DEBUG ((EFI_D_ERROR, "%a: Failed to get Fdt Hob\n", __FUNCTION__));
+    return EFI_NOT_FOUND;
+  }
+  DeviceTreeBase = (VOID *)(UINTN)*(UINT64 *)GET_GUID_HOB_DATA (Hob);
+
+  if (fdt_check_header (DeviceTreeBase) != 0) {
+    DEBUG ((EFI_D_ERROR, "%a: No DTB found @ 0x%p\n", __FUNCTION__, DeviceTreeBase));
+    return EFI_NOT_FOUND;
+  }
+
+  Node = fdt_path_offset(DeviceTreeBase, "/chosen/modules");
+  if ( Node < 0 ) {
+    DEBUG ((EFI_D_ERROR, "%a: NO /chosen/modules found\n", __FUNCTION__));
+    return EFI_NOT_FOUND;
+  }
+
+
+  for (Depth = 0;
+       (Node >= 0) && (Depth >= 0);
+       Node = fdt_next_node (DeviceTreeBase, Node, &Depth)) {
+    if (Depth == 1) {
+        Type = fdt_getprop (DeviceTreeBase, Node, "compatible", &Len);
+        if (fdt_stringlist_contains (Type, Len, "xen,linux-acpi")) {
+          RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
+          ASSERT (Len == 2 * sizeof (UINT64));
+          RsdpStructurePtr = (EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *)
+                               (UINTN) fdt64_to_cpu (((UINT64 *)RegProp)[0]);
+        }
+    }
+  }
+
+  if (RsdpStructurePtr && RsdpStructurePtr->Revision >= 2) {
+    *RsdpPtr = RsdpStructurePtr;
+    return EFI_SUCCESS;
+  }
+
+  return EFI_NOT_FOUND;
+}
+
+/**
+  Get Xen Acpi tables from the RSDP structure. And installs Xen ACPI tables
+  into the RSDT/XSDT using InstallAcpiTable. Some signature of the installed
+  ACPI tables are: FACP, APIC, GTDT, DSDT.
+
+  @param  AcpiProtocol           Protocol instance pointer.
+
+  @return EFI_SUCCESS            The table was successfully inserted.
+  @return EFI_INVALID_PARAMETER  Either AcpiTableBuffer is NULL, TableHandle is
+                                 NULL, or AcpiTableBufferSize and the size
+                                 field embedded in the ACPI table pointed to
+                                 by AcpiTableBuffer are not in sync.
+  @return EFI_OUT_OF_RESOURCES   Insufficient resources exist to complete the request.
+
+**/
+EFI_STATUS
+EFIAPI
+InstallXenArmTables (
+  IN   EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
+  )
+{
+  EFI_STATUS                                       Status;
+  UINTN                                            TableHandle;
+
+  EFI_ACPI_DESCRIPTION_HEADER                      *Xsdt;
+  VOID                                             *CurrentTableEntry;
+  UINTN                                            CurrentTablePointer;
+  EFI_ACPI_DESCRIPTION_HEADER                      *CurrentTable;
+  UINTN                                            Index;
+  UINTN                                            NumberOfTableEntries;
+  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE        *FadtTable;
+  EFI_ACPI_DESCRIPTION_HEADER                      *DsdtTable;
+
+  FadtTable   = NULL;
+  DsdtTable   = NULL;
+  TableHandle = 0;
+  NumberOfTableEntries = 0;
+
+  //
+  // Try to find Xen ARM ACPI tables
+  //
+  Status = GetXenArmAcpiRsdp (&XenAcpiRsdpStructurePtr);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_INFO, "%a: No RSDP table found\n", __FUNCTION__));
+    return Status;
+  }
+
+  //
+  // If XSDT table is find, just install its tables. 
+  //
+  if (XenAcpiRsdpStructurePtr->XsdtAddress) {
+    //
+    // Retrieve the addresses of XSDT and 
+    // calculate the number of its table entries.
+    //
+    Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN)
+             XenAcpiRsdpStructurePtr->XsdtAddress;
+    NumberOfTableEntries = (Xsdt->Length -
+                             sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
+                             sizeof (UINT64);
+    //
+    // Install ACPI tables found in XSDT.
+    //
+    for (Index = 0; Index < NumberOfTableEntries; Index++) {
+      //
+      // Get the table entry from XSDT
+      //
+      CurrentTableEntry = (VOID *) ((UINT8 *) Xsdt +
+                            sizeof (EFI_ACPI_DESCRIPTION_HEADER) +
+                            Index * sizeof (UINT64));
+      CurrentTablePointer = (UINTN) *(UINT64 *)CurrentTableEntry;
+      CurrentTable = (EFI_ACPI_DESCRIPTION_HEADER *) CurrentTablePointer;
+
+      //
+      // Install the XSDT tables
+      //
+      Status = AcpiProtocol->InstallAcpiTable (
+                 AcpiProtocol,
+                 CurrentTable,
+                 CurrentTable->Length,
+                 &TableHandle
+                 );
+
+      if (EFI_ERROR (Status)) {
+        return Status;
+      }
+
+      //
+      // Get the FACS and DSDT table address from the table FADT
+      //
+      if (!AsciiStrnCmp ((CHAR8 *) &CurrentTable->Signature, "FACP", 4)) {
+        FadtTable = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *)
+                      (UINTN) CurrentTablePointer;
+        DsdtTable  = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) FadtTable->Dsdt;
+      }
+    }
+  }
+
+  //
+  // Install DSDT table.
+  //
+  Status = AcpiProtocol->InstallAcpiTable (
+             AcpiProtocol,
+             DsdtTable,
+             DsdtTable->Length,
+             &TableHandle
+             );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  return EFI_SUCCESS;
+}
diff --git a/OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatform.c b/OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatform.c
new file mode 100644
index 0000000..7f141a9
--- /dev/null
+++ b/OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatform.c
@@ -0,0 +1,38 @@ 
+/** @file
+  OVMF ACPI Platform Driver using Xen ARM multiboot protocol
+
+  Copyright (C) 2016, Linaro Ltd. All rights reserved.
+
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License which accompanies this
+  distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+**/
+
+#include "AcpiPlatform.h"
+
+/**
+  Effective entrypoint of Xen ARM Acpi Platform driver.
+
+  @param  ImageHandle
+  @param  SystemTable
+
+  @return EFI_SUCCESS
+  @return EFI_LOAD_ERROR
+  @return EFI_OUT_OF_RESOURCES
+
+**/
+EFI_STATUS
+EFIAPI
+InstallAcpiTables (
+  IN   EFI_ACPI_TABLE_PROTOCOL       *AcpiTable
+  )
+{
+  EFI_STATUS                         Status;
+
+  Status = InstallXenArmTables (AcpiTable);
+  return Status;
+}
diff --git a/OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf
new file mode 100644
index 0000000..d96e9a1
--- /dev/null
+++ b/OvmfPkg/AcpiPlatformDxe/XenArmAcpiPlatformDxe.inf
@@ -0,0 +1,59 @@ 
+## @file
+#  OVMF ACPI Platform Driver using Xen ARM multiboot protocol
+#
+#  Copyright (C) 2016, Linaro Ltd. All rights reserved.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which accompanies this distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = XenArmAcpiPlatform
+  FILE_GUID                      = 0efc6282-f1e5-469a-8a70-194a8761f9aa
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = AcpiPlatformEntryPoint
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = AARCH64
+#
+
+[Sources]
+  XenArmAcpiPlatform.c
+  XenArmAcpi.c
+  EntryPoint.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  OvmfPkg/OvmfPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  HobLib
+  FdtLib
+  UefiDriverEntryPoint
+
+[Protocols]
+  gEfiAcpiTableProtocolGuid                     # PROTOCOL ALWAYS_CONSUMED
+  gEfiPciEnumerationCompleteProtocolGuid        # PROTOCOL SOMETIMES_CONSUMED
+
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
+
+[Depex]
+  gEfiAcpiTableProtocolGuid
+
+[Guids]
+  gFdtHobGuid