Re: [PATCH v2 4/4] ArmVirtPkg/FdtClientDxe: make DT table installation !ACPI dependent

[Laszlo Ersek <lersek@redhat.com>]

On 03/09/17 18:21, Ard Biesheuvel wrote:
> Instead of having a build time switch to prevent the FDT configuration
> table from being installed, make this behavior dependent on whether we
> are passing ACPI tables to the OS. This is done by looking for the
> ACPI 2.0 configuration table, and only installing the FDT one if the
> ACPI one cannot be found.

I would have preferred if, rather than updating just the blurb, you had
mentioned Xen by name in this commit message. But, I don't want to
obsess about it.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks,
Laszlo

> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/ArmVirtPkg.dec                | 10 ----------
>  ArmVirtPkg/ArmVirtQemu.dsc               |  5 -----
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 16 +++++++++++-----
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  5 ++---
>  4 files changed, 13 insertions(+), 23 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
> index a5ec42166445..efe83a383d55 100644
> --- a/ArmVirtPkg/ArmVirtPkg.dec
> +++ b/ArmVirtPkg/ArmVirtPkg.dec
> @@ -58,13 +58,3 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
>    # EFI_VT_100_GUID.
>    #
>    gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0x4D}|VOID*|0x00000007
> -
> -[PcdsFeatureFlag]
> -  #
> -  # Pure ACPI boot
> -  #
> -  # Inhibit installation of the FDT as a configuration table if this feature
> -  # PCD is TRUE. Otherwise, the OS is presented with both a DT and an ACPI
> -  # description of the platform, and it is up to the OS to choose.
> -  #
> -  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|FALSE|BOOLEAN|0x0000000a
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 477dfdcfc764..7b266b98b949 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -34,7 +34,6 @@ [Defines]
>    # -D FLAG=VALUE
>    #
>    DEFINE SECURE_BOOT_ENABLE      = FALSE
> -  DEFINE PURE_ACPI_BOOT_ENABLE   = FALSE
>  
>  !include ArmVirtPkg/ArmVirt.dsc.inc
>  
> @@ -94,10 +93,6 @@ [PcdsFeatureFlag.common]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
>    gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
>  
> -!if $(PURE_ACPI_BOOT_ENABLE) == TRUE
> -  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|TRUE
> -!endif
> -
>  [PcdsFixedAtBuild.common]
>    gArmPlatformTokenSpaceGuid.PcdCoreCount|1
>  !if $(ARCH) == AARCH64
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> index 4cf79f70cb2a..21c1074e331c 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> @@ -17,9 +17,11 @@
>  #include <Library/DebugLib.h>
>  #include <Library/UefiDriverEntryPoint.h>
>  #include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
>  #include <Library/HobLib.h>
>  #include <libfdt.h>
>  
> +#include <Guid/Acpi.h>
>  #include <Guid/EventGroup.h>
>  #include <Guid/Fdt.h>
>  #include <Guid/FdtHob.h>
> @@ -316,12 +318,16 @@ OnReadyToBoot (
>    )
>  {
>    EFI_STATUS      Status;
> +  VOID            *Table;
>  
> -  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
> -    //
> -    // Only install the FDT as a configuration table if we want to leave it up
> -    // to the OS to decide whether it prefers ACPI over DT.
> -    //
> +  //
> +  // Only install the FDT as a configuration table if we are not exposing
> +  // ACPI 2.0 (or later) tables. Note that the legacy ACPI table GUID has
> +  // no meaning on ARM since we need at least ACPI 5.0 support, and the
> +  // 64-bit ACPI 2.0 table GUID is mandatory in that case.
> +  //
> +  Status = EfiGetSystemConfigurationTable (&gEfiAcpi20TableGuid, &Table);
> +  if (EFI_ERROR (Status) || Table == NULL) {
>      Status = gBS->InstallConfigurationTable (&gFdtTableGuid, mDeviceTreeBase);
>      ASSERT_EFI_ERROR (Status);
>    }
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> index 00017727c32c..9861f41e968b 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> @@ -37,17 +37,16 @@ [LibraryClasses]
>    HobLib
>    UefiBootServicesTableLib
>    UefiDriverEntryPoint
> +  UefiLib
>  
>  [Protocols]
>    gFdtClientProtocolGuid                  ## PRODUCES
>  
>  [Guids]
> +  gEfiAcpi20TableGuid
>    gEfiEventReadyToBootGuid
>    gFdtHobGuid
>    gFdtTableGuid
>  
> -[FeaturePcd]
> -  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
> -
>  [Depex]
>    TRUE
>