public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [RFC PATCH] MdeModulePkg/AcpiTableDxe: inhibit table publication until we have a FADT
@ 2017-03-18 20:58 Ard Biesheuvel
  2017-03-20  2:15 ` Zeng, Star
  2017-03-20 12:13 ` Leif Lindholm
  0 siblings, 2 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2017-03-18 20:58 UTC (permalink / raw)
  To: edk2-devel, lersek, leif.lindholm, star.zeng; +Cc: feng.tian, Ard Biesheuvel

Since commit 78c41ff519b1 ("ArmVirtPkg/FdtClientDxe: make DT table
installation !ACPI dependent"), we inhibit the installation of the core
ACPI tables when booting in DT mode, to relieve the OS from having to
decide which hardware description to use. However, as it turns out,
in presence of the RamdiskDxe or BGRT drivers, some ACPI tables are
still registered with the protocol, and published under the ACPI entry
point configuration tables, ignoring the fact that there is no way the
OS can boot with only NFIT and BGRT tables present.

So let's only publish the ACPI tables if we can reasonably assume that
the tables that the OS requires have been registered with the protocol,
by checking that we have either FADT1 or FADT3 table (or both) before
installing the configuration tables.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---

This is a counterproposal for Laszlo's series [0], which is technically
sound, but a bit unwieldy. Instead of making ARM the odd one out, and
tweaking universal modules via NULL library class resolution, we can
prevent AcpiTableDxe from publishing the entry points when only NFIT
or BGRT tables are present (which sounds like a strange thing to do
in any case)

I am aware that this still leaves some loose ends when it comes to ordering
and the ReadyToBoot callback, which I agree we should solve regardless, but
I hope we can do so without putting the burden on the ARM platforms to
always carry a set of NULL class libraries to keep sane behavior.

[0] https://lists.01.org/pipermail/edk2-devel/2017-March/008684.html

 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
index 4bb848df5203..4c9921f69a8a 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
@@ -130,6 +130,21 @@ PublishTables (
   UINT64                    Buffer64;
 
   //
+  // Don't publish anything yet if we don't have a FADT table. This table is
+  // mandatory for all ACPI compatible OSes, and installing the ACPI entry
+  // point configuration table without a full set of ACPI tables may confuse
+  // some OSes (Linux/arm64). (This may happen when the BGRT or ramdisk drivers
+  // publish their respective ACPI tables without regard for whether ACPI boot
+  // is currently enabled.)
+  //
+  if (AcpiTableInstance->Fadt1 == NULL && AcpiTableInstance->Fadt3 == NULL) {
+    DEBUG ((DEBUG_INFO,
+      "%a: not publishing ACPI tables [yet], no FADT table installed\n",
+      __FUNCTION__));
+    return EFI_SUCCESS;
+  }
+
+  //
   // Reorder tables as some operating systems don't seem to find the
   // FADT correctly if it is not in the first few entries
   //
-- 
2.9.3



^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-03-20 12:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-18 20:58 [RFC PATCH] MdeModulePkg/AcpiTableDxe: inhibit table publication until we have a FADT Ard Biesheuvel
2017-03-20  2:15 ` Zeng, Star
2017-03-20 12:13 ` Leif Lindholm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox