[edk2] [PATCH v2 01/12] Revert "ArmVirtPkg/FdtClientDxe: make DT table installation !ACPI dependent"

Laszlo Ersek posted 12 patches 7 years, 7 months ago
There is a newer version of this series
[edk2] [PATCH v2 01/12] Revert "ArmVirtPkg/FdtClientDxe: make DT table installation !ACPI dependent"
Posted by Laszlo Ersek 7 years, 7 months ago
This reverts commit 78c41ff519b187d8979cda7074f007a6323f9acd.

We realized that DXE drivers that are independent of AcpiPlatformDxe (that
is, independent of QEMU's ACPI generation), such as RamDiskDxe and
BootGraphicsResourceTableDxe, may produce and/or manipulate ACPI tables,
at driver dispatch or even at Ready To Boot.

This makes it unsafe for us to check for ACPI presence in the UEFI system
config table in a Ready To Boot callback, in order to decide about
exposing the DT.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 ArmVirtPkg/ArmVirtPkg.dec                | 10 ++++++++++
 ArmVirtPkg/ArmVirtQemu.dsc               |  5 +++++
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  5 +++--
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 16 +++++-----------
 4 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index efe83a383d55..a5ec42166445 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -58,3 +58,13 @@ [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 0bbbe4a7aa4a..d6b3c0db5530 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -34,6 +34,7 @@ [Defines]
   # -D FLAG=VALUE
   #
   DEFINE SECURE_BOOT_ENABLE      = FALSE
+  DEFINE PURE_ACPI_BOOT_ENABLE   = FALSE
 
 !include ArmVirtPkg/ArmVirt.dsc.inc
 
@@ -94,6 +95,10 @@ [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.inf b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
index 9861f41e968b..00017727c32c 100644
--- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
+++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
@@ -37,16 +37,17 @@ [LibraryClasses]
   HobLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
-  UefiLib
 
 [Protocols]
   gFdtClientProtocolGuid                  ## PRODUCES
 
 [Guids]
-  gEfiAcpi20TableGuid
   gEfiEventReadyToBootGuid
   gFdtHobGuid
   gFdtTableGuid
 
+[FeaturePcd]
+  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
+
 [Depex]
   TRUE
diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
index 21c1074e331c..4cf79f70cb2a 100644
--- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
+++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
@@ -17,11 +17,9 @@
 #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>
@@ -318,16 +316,12 @@ OnReadyToBoot (
   )
 {
   EFI_STATUS      Status;
-  VOID            *Table;
 
-  //
-  // 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) {
+  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);
   }
-- 
2.9.3


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 01/12] Revert "ArmVirtPkg/FdtClientDxe: make DT table installation !ACPI dependent"
Posted by Ard Biesheuvel 7 years, 7 months ago
On 17 March 2017 at 20:47, Laszlo Ersek <lersek@redhat.com> wrote:
> This reverts commit 78c41ff519b187d8979cda7074f007a6323f9acd.
>
> We realized that DXE drivers that are independent of AcpiPlatformDxe (that
> is, independent of QEMU's ACPI generation), such as RamDiskDxe and
> BootGraphicsResourceTableDxe, may produce and/or manipulate ACPI tables,
> at driver dispatch or even at Ready To Boot.
>
> This makes it unsafe for us to check for ACPI presence in the UEFI system
> config table in a Ready To Boot callback, in order to decide about
> exposing the DT.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  ArmVirtPkg/ArmVirtPkg.dec                | 10 ++++++++++
>  ArmVirtPkg/ArmVirtQemu.dsc               |  5 +++++
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  5 +++--
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 16 +++++-----------
>  4 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
> index efe83a383d55..a5ec42166445 100644
> --- a/ArmVirtPkg/ArmVirtPkg.dec
> +++ b/ArmVirtPkg/ArmVirtPkg.dec
> @@ -58,3 +58,13 @@ [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 0bbbe4a7aa4a..d6b3c0db5530 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -34,6 +34,7 @@ [Defines]
>    # -D FLAG=VALUE
>    #
>    DEFINE SECURE_BOOT_ENABLE      = FALSE
> +  DEFINE PURE_ACPI_BOOT_ENABLE   = FALSE
>
>  !include ArmVirtPkg/ArmVirt.dsc.inc
>
> @@ -94,6 +95,10 @@ [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.inf b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> index 9861f41e968b..00017727c32c 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> @@ -37,16 +37,17 @@ [LibraryClasses]
>    HobLib
>    UefiBootServicesTableLib
>    UefiDriverEntryPoint
> -  UefiLib
>
>  [Protocols]
>    gFdtClientProtocolGuid                  ## PRODUCES
>
>  [Guids]
> -  gEfiAcpi20TableGuid
>    gEfiEventReadyToBootGuid
>    gFdtHobGuid
>    gFdtTableGuid
>
> +[FeaturePcd]
> +  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
> +
>  [Depex]
>    TRUE
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> index 21c1074e331c..4cf79f70cb2a 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> @@ -17,11 +17,9 @@
>  #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>
> @@ -318,16 +316,12 @@ OnReadyToBoot (
>    )
>  {
>    EFI_STATUS      Status;
> -  VOID            *Table;
>
> -  //
> -  // 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) {
> +  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);
>    }
> --
> 2.9.3
>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel