[PATCH v2 0/4] ArmVirtPkg: make DT vs ACPI support mutually exclusive

[PATCH v2 0/4] ArmVirtPkg: make DT vs ACPI support mutually exclusive

[Ard Biesheuvel]

Instead of supplying both ACPI and DT hw descriptions, and allow the latter
to be inihibited by setting a compile time define, make DT table installation
dependent on the absence of a ACPI 2.0 table when the ReadyToBoot even fires.

Changes since v1:
- add missing includes
- cosmetic coding style fixes
- reorder event registration with protocol installation (#2)
- add Laszlo's patch to add missing EFIAPI specifiers

As Laszlo has pointed out, this affects the Xen port as well as the QEMU/KVM
one, which I consider to be an advantage. And of course, I am happy to keep
both halves if it turns out I ended up breaking it :-)

Ard Biesheuvel (3):
  ArmVirtPkg/ArmVirtPL031FdtClientLib: unconditionally disable DT node
  ArmVirtPkg/FdtClientDxe: install DT configuration table at ReadyToBoot
  ArmVirtPkg/FdtClientDxe: make DT table installation !ACPI dependent

Laszlo Ersek (1):
  ArmVirtPkg/FdtClientDxe: supplement missing EFIAPI calling conv
    specifiers

 ArmVirtPkg/ArmVirtPkg.dec                                                | 10 ----
 ArmVirtPkg/ArmVirtQemu.dsc                                               |  5 --
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.c                                   | 58 +++++++++++++++++---
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf                                 |  6 +-
 ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c   | 22 ++++----
 ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf |  3 -
 6 files changed, 62 insertions(+), 42 deletions(-)

-- 
2.7.4

[PATCH v2 1/4] ArmVirtPkg/FdtClientDxe: supplement missing EFIAPI calling conv specifiers

[Ard Biesheuvel]

From: Laszlo Ersek <lersek@redhat.com>

Add missing EFIAPI calling convention specifiers to STATIC function
that are exposed via the FdtClientProtocol interface.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
index 7cc0c44ca12a..547a29fce62c 100644
--- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
+++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
@@ -29,6 +29,7 @@ STATIC VOID  *mDeviceTreeBase;
 
 STATIC
 EFI_STATUS
+EFIAPI
 GetNodeProperty (
   IN  FDT_CLIENT_PROTOCOL     *This,
   IN  INT32                   Node,
@@ -55,6 +56,7 @@ GetNodeProperty (
 
 STATIC
 EFI_STATUS
+EFIAPI
 SetNodeProperty (
   IN  FDT_CLIENT_PROTOCOL     *This,
   IN  INT32                   Node,
@@ -267,6 +269,7 @@ FindMemoryNodeReg (
 
 STATIC
 EFI_STATUS
+EFIAPI
 GetOrInsertChosenNode (
   IN  FDT_CLIENT_PROTOCOL     *This,
   OUT INT32                   *Node
-- 
2.7.4

Re: [PATCH v2 1/4] ArmVirtPkg/FdtClientDxe: supplement missing EFIAPI calling conv specifiers

[Laszlo Ersek]

On 03/09/17 18:21, Ard Biesheuvel wrote:
> From: Laszlo Ersek <lersek@redhat.com>
> 
> Add missing EFIAPI calling convention specifiers to STATIC function
> that are exposed via the FdtClientProtocol interface.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> index 7cc0c44ca12a..547a29fce62c 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> @@ -29,6 +29,7 @@ STATIC VOID  *mDeviceTreeBase;
>  
>  STATIC
>  EFI_STATUS
> +EFIAPI
>  GetNodeProperty (
>    IN  FDT_CLIENT_PROTOCOL     *This,
>    IN  INT32                   Node,
> @@ -55,6 +56,7 @@ GetNodeProperty (
>  
>  STATIC
>  EFI_STATUS
> +EFIAPI
>  SetNodeProperty (
>    IN  FDT_CLIENT_PROTOCOL     *This,
>    IN  INT32                   Node,
> @@ -267,6 +269,7 @@ FindMemoryNodeReg (
>  
>  STATIC
>  EFI_STATUS
> +EFIAPI
>  GetOrInsertChosenNode (
>    IN  FDT_CLIENT_PROTOCOL     *This,
>    OUT INT32                   *Node
> 

Well, I can't exactly "review" or "ACK" my own patch (or patches mostly
derived from my patches). Leif, can you pls R-b this?

Thanks
Laszlo

Re: [PATCH v2 1/4] ArmVirtPkg/FdtClientDxe: supplement missing EFIAPI calling conv specifiers

[Leif Lindholm]

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

On 9 Mar 2017 6:29 pm, "Laszlo Ersek" <lersek@redhat.com> wrote:

> On 03/09/17 18:21, Ard Biesheuvel wrote:
> > From: Laszlo Ersek <lersek@redhat.com>
> >
> > Add missing EFIAPI calling convention specifiers to STATIC function
> > that are exposed via the FdtClientProtocol interface.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> > index 7cc0c44ca12a..547a29fce62c 100644
> > --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> > +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> > @@ -29,6 +29,7 @@ STATIC VOID  *mDeviceTreeBase;
> >
> >  STATIC
> >  EFI_STATUS
> > +EFIAPI
> >  GetNodeProperty (
> >    IN  FDT_CLIENT_PROTOCOL     *This,
> >    IN  INT32                   Node,
> > @@ -55,6 +56,7 @@ GetNodeProperty (
> >
> >  STATIC
> >  EFI_STATUS
> > +EFIAPI
> >  SetNodeProperty (
> >    IN  FDT_CLIENT_PROTOCOL     *This,
> >    IN  INT32                   Node,
> > @@ -267,6 +269,7 @@ FindMemoryNodeReg (
> >
> >  STATIC
> >  EFI_STATUS
> > +EFIAPI
> >  GetOrInsertChosenNode (
> >    IN  FDT_CLIENT_PROTOCOL     *This,
> >    OUT INT32                   *Node
> >
>
> Well, I can't exactly "review" or "ACK" my own patch (or patches mostly
> derived from my patches). Leif, can you pls R-b this?
>
> Thanks
> Laszlo
>

[PATCH v2 2/4] ArmVirtPkg/ArmVirtPL031FdtClientLib: unconditionally disable DT node

[Ard Biesheuvel]

Disable the PL031 RTC DT node unconditionally rather than only when
the DT will be exposed to the OS. This allows us to defer the decision
whether to expose it to the OS to a later time without creating an
additional dependency on the FDT client code by the RTC driver.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c   | 22 +++++++++-----------
 ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf |  3 ---
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
index 82de7a51b32e..d168424a52f5 100644
--- a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
+++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
@@ -66,18 +66,16 @@ ArmVirtPL031FdtClientLibConstructor (
 
   DEBUG ((EFI_D_INFO, "Found PL031 RTC @ 0x%Lx\n", RegBase));
 
-  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
-    //
-    // UEFI takes ownership of the RTC hardware, and exposes its functionality
-    // through the UEFI Runtime Services GetTime, SetTime, etc. This means we
-    // need to disable it in the device tree to prevent the OS from attaching
-    // its device driver as well.
-    //
-    Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
-                          "disabled", sizeof ("disabled"));
-    if (EFI_ERROR (Status)) {
-        DEBUG ((EFI_D_WARN, "Failed to set PL031 status to 'disabled'\n"));
-    }
+  //
+  // UEFI takes ownership of the RTC hardware, and exposes its functionality
+  // through the UEFI Runtime Services GetTime, SetTime, etc. This means we
+  // need to disable it in the device tree to prevent the OS from attaching
+  // its device driver as well.
+  //
+  Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
+                        "disabled", sizeof ("disabled"));
+  if (EFI_ERROR (Status)) {
+      DEBUG ((EFI_D_WARN, "Failed to set PL031 status to 'disabled'\n"));
   }
 
   return EFI_SUCCESS;
diff --git a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
index 32dbff6f0852..342193651a86 100644
--- a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
+++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
@@ -42,8 +42,5 @@ [Protocols]
 [Pcd]
   gArmPlatformTokenSpaceGuid.PcdPL031RtcBase
 
-[FeaturePcd]
-  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
-
 [Depex]
   gFdtClientProtocolGuid
-- 
2.7.4

[PATCH v2 3/4] ArmVirtPkg/FdtClientDxe: install DT configuration table at ReadyToBoot

[Ard Biesheuvel]

Defer FDT configuration table installation until ReadyToBoot is signaled.
This allows any driver to make modifications in the mean time, and will
also allow us to defer the decision of whether to install it in the first
place to later on in the boot.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 49 ++++++++++++++++----
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  1 +
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
index 547a29fce62c..4cf79f70cb2a 100644
--- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
+++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
@@ -20,6 +20,7 @@
 #include <Library/HobLib.h>
 #include <libfdt.h>
 
+#include <Guid/EventGroup.h>
 #include <Guid/Fdt.h>
 #include <Guid/FdtHob.h>
 
@@ -306,6 +307,30 @@ STATIC FDT_CLIENT_PROTOCOL mFdtClientProtocol = {
   GetOrInsertChosenNode,
 };
 
+STATIC
+VOID
+EFIAPI
+OnReadyToBoot (
+  EFI_EVENT       Event,
+  VOID            *Context
+  )
+{
+  EFI_STATUS      Status;
+
+  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.
+    //
+    Status = gBS->InstallConfigurationTable (&gFdtTableGuid, mDeviceTreeBase);
+    ASSERT_EFI_ERROR (Status);
+  }
+
+  gBS->CloseEvent (Event);
+}
+
+STATIC EFI_EVENT         mReadyToBootEvent;
+
 EFI_STATUS
 EFIAPI
 InitializeFdtClientDxe (
@@ -333,15 +358,21 @@ InitializeFdtClientDxe (
 
   DEBUG ((EFI_D_INFO, "%a: DTB @ 0x%p\n", __FUNCTION__, mDeviceTreeBase));
 
-  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.
-    //
-    Status = gBS->InstallConfigurationTable (&gFdtTableGuid, DeviceTreeBase);
-    ASSERT_EFI_ERROR (Status);
+  Status = gBS->InstallProtocolInterface (&ImageHandle, &gFdtClientProtocolGuid,
+                  EFI_NATIVE_INTERFACE, &mFdtClientProtocol);
+  if (EFI_ERROR (Status)) {
+    return Status;
   }
 
-  return gBS->InstallProtocolInterface (&ImageHandle, &gFdtClientProtocolGuid,
-                EFI_NATIVE_INTERFACE, &mFdtClientProtocol);
+  Status = gBS->CreateEventEx (
+                  EVT_NOTIFY_SIGNAL,
+                  TPL_CALLBACK,
+                  OnReadyToBoot,
+                  NULL,
+                  &gEfiEventReadyToBootGuid,
+                  &mReadyToBootEvent
+                  );
+  ASSERT_EFI_ERROR (Status);
+
+  return EFI_SUCCESS;
 }
diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
index 3a0cd37040eb..00017727c32c 100644
--- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
+++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
@@ -42,6 +42,7 @@ [Protocols]
   gFdtClientProtocolGuid                  ## PRODUCES
 
 [Guids]
+  gEfiEventReadyToBootGuid
   gFdtHobGuid
   gFdtTableGuid
 
-- 
2.7.4

Re: [PATCH v2 3/4] ArmVirtPkg/FdtClientDxe: install DT configuration table at ReadyToBoot

[Laszlo Ersek]

On 03/09/17 18:21, Ard Biesheuvel wrote:
> Defer FDT configuration table installation until ReadyToBoot is signaled.
> This allows any driver to make modifications in the mean time, and will
> also allow us to defer the decision of whether to install it in the first
> place to later on in the boot.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 49 ++++++++++++++++----
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  1 +
>  2 files changed, 41 insertions(+), 9 deletions(-)
> 
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> index 547a29fce62c..4cf79f70cb2a 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> @@ -20,6 +20,7 @@
>  #include <Library/HobLib.h>
>  #include <libfdt.h>
>  
> +#include <Guid/EventGroup.h>
>  #include <Guid/Fdt.h>
>  #include <Guid/FdtHob.h>
>  
> @@ -306,6 +307,30 @@ STATIC FDT_CLIENT_PROTOCOL mFdtClientProtocol = {
>    GetOrInsertChosenNode,
>  };
>  
> +STATIC
> +VOID
> +EFIAPI
> +OnReadyToBoot (
> +  EFI_EVENT       Event,
> +  VOID            *Context
> +  )
> +{
> +  EFI_STATUS      Status;
> +
> +  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.
> +    //
> +    Status = gBS->InstallConfigurationTable (&gFdtTableGuid, mDeviceTreeBase);
> +    ASSERT_EFI_ERROR (Status);
> +  }
> +
> +  gBS->CloseEvent (Event);
> +}
> +
> +STATIC EFI_EVENT         mReadyToBootEvent;
> +
>  EFI_STATUS
>  EFIAPI
>  InitializeFdtClientDxe (
> @@ -333,15 +358,21 @@ InitializeFdtClientDxe (
>  
>    DEBUG ((EFI_D_INFO, "%a: DTB @ 0x%p\n", __FUNCTION__, mDeviceTreeBase));
>  
> -  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.
> -    //
> -    Status = gBS->InstallConfigurationTable (&gFdtTableGuid, DeviceTreeBase);
> -    ASSERT_EFI_ERROR (Status);
> +  Status = gBS->InstallProtocolInterface (&ImageHandle, &gFdtClientProtocolGuid,
> +                  EFI_NATIVE_INTERFACE, &mFdtClientProtocol);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
>    }
>  
> -  return gBS->InstallProtocolInterface (&ImageHandle, &gFdtClientProtocolGuid,
> -                EFI_NATIVE_INTERFACE, &mFdtClientProtocol);
> +  Status = gBS->CreateEventEx (
> +                  EVT_NOTIFY_SIGNAL,
> +                  TPL_CALLBACK,
> +                  OnReadyToBoot,
> +                  NULL,
> +                  &gEfiEventReadyToBootGuid,
> +                  &mReadyToBootEvent
> +                  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return EFI_SUCCESS;
>  }
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> index 3a0cd37040eb..00017727c32c 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> @@ -42,6 +42,7 @@ [Protocols]
>    gFdtClientProtocolGuid                  ## PRODUCES
>  
>  [Guids]
> +  gEfiEventReadyToBootGuid
>    gFdtHobGuid
>    gFdtTableGuid
>  
> 

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

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

[Ard Biesheuvel]

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.

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
-- 
2.7.4

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

[Laszlo Ersek]

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
> 

Re: [PATCH v2 0/4] ArmVirtPkg: make DT vs ACPI support mutually exclusive

[Ard Biesheuvel]

On 9 March 2017 at 18:21, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Instead of supplying both ACPI and DT hw descriptions, and allow the latter
> to be inihibited by setting a compile time define, make DT table installation
> dependent on the absence of a ACPI 2.0 table when the ReadyToBoot even fires.
>
> Changes since v1:
> - add missing includes
> - cosmetic coding style fixes
> - reorder event registration with protocol installation (#2)
> - add Laszlo's patch to add missing EFIAPI specifiers
>
> As Laszlo has pointed out, this affects the Xen port as well as the QEMU/KVM
> one, which I consider to be an advantage. And of course, I am happy to keep
> both halves if it turns out I ended up breaking it :-)
>

Thanks guys, all pushed now