[PATCH v1 1/1] OvmfPkg: Allow multiple add-pointer linker commands to same ACPI table

[Phil Dennis-Jordan <lists@philjordan.eu>]

From: Phil Dennis-Jordan <phil@philjordan.eu>

ACPI tables may contain multiple pointer fields to the same destination
table. For example, in some revisions, the FADT contains both DSDT and
X_DSDT fields, and they may both point to the DSDT. Indeed, some operating
systems demand this to be the case.

Previously, if Qemu created "add pointer" linker commands for both fields,
the linking process would fail, as AcpiProtocol->InstallAcpiTable() may
only be called once for each destination table and otherwise returns an
error.

This change adds a memoisation data structure which tracks the table
pointers that have already been installed; even if the same pointer is
encountered multiple times, it is only installed once.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
---
 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 109 ++++++++++++++++----
 1 file changed, 89 insertions(+), 20 deletions(-)

diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
index 7bb2e3f21821..cffa838623cc 100644
--- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
+++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
@@ -100,6 +100,39 @@ BlobCompare (
 
 
 /**
+  Comparator function for two opaque pointers, ordering on pointer value
+  itself.
+  Can be used as both Key and UserStruct comparator.
+
+  @param[in] Pointer1  First pointer.
+
+  @param[in] Pointer2  Second pointer.
+
+  @retval <0  If Pointer1 compares less than Pointer2.
+
+  @retval  0  If Pointer1 compares equal to Pointer2.
+
+  @retval >0  If Pointer1 compares greater than Pointer2.
+**/
+STATIC
+INTN
+EFIAPI
+PointerCompare (
+  IN CONST VOID *Pointer1,
+  IN CONST VOID *Pointer2
+  )
+{
+  if (Pointer1 == Pointer2) {
+    return 0;
+  } else if ((INTN)Pointer1 < (INTN)Pointer2) {
+    return -1;
+  } else {
+    return 1;
+  }
+}
+
+
+/**
   Process a QEMU_LOADER_ALLOCATE command.
 
   @param[in] Allocate     The QEMU_LOADER_ALLOCATE command to process.
@@ -535,27 +568,32 @@ UndoCmdWritePointer (
   This function assumes that the entire QEMU linker/loader command file has
   been processed successfully in a prior first pass.
 
-  @param[in] AddPointer        The QEMU_LOADER_ADD_POINTER command to process.
+  @param[in] AddPointer          The QEMU_LOADER_ADD_POINTER command to process.
 
-  @param[in] Tracker           The ORDERED_COLLECTION tracking the BLOB user
-                               structures.
+  @param[in] Tracker             The ORDERED_COLLECTION tracking the BLOB user
+                                 structures.
 
-  @param[in] AcpiProtocol      The ACPI table protocol used to install tables.
+  @param[in] AcpiProtocol        The ACPI table protocol used to install tables.
 
-  @param[in,out] InstalledKey  On input, an array of INSTALLED_TABLES_MAX UINTN
-                               elements, allocated by the caller. On output,
-                               the function will have stored (appended) the
-                               AcpiProtocol-internal key of the ACPI table that
-                               the function has installed, if the AddPointer
-                               command identified an ACPI table that is
-                               different from RSDT and XSDT.
+  @param[in,out] InstalledKey    On input, an array of INSTALLED_TABLES_MAX UINTN
+                                 elements, allocated by the caller. On output,
+                                 the function will have stored (appended) the
+                                 AcpiProtocol-internal key of the ACPI table that
+                                 the function has installed, if the AddPointer
+                                 command identified an ACPI table that is
+                                 different from RSDT and XSDT.
 
-  @param[in,out] NumInstalled  On input, the number of entries already used in
-                               InstalledKey; it must be in [0,
-                               INSTALLED_TABLES_MAX] inclusive. On output, the
-                               parameter is incremented if the AddPointer
-                               command identified an ACPI table that is
-                               different from RSDT and XSDT.
+  @param[in,out] NumInstalled    On input, the number of entries already used in
+                                 InstalledKey; it must be in [0,
+                                 INSTALLED_TABLES_MAX] inclusive. On output, the
+                                 parameter is incremented if the AddPointer
+                                 command identified an ACPI table that is
+                                 different from RSDT and XSDT.
+
+  @param[in,out] InstalledTables The ORDERED_COLLECTION tracking the ACPI tables
+                                 which have already been installed. If a new
+                                 table is encountered by the function, it is
+                                 added; existing ones will not be installed again.
 
   @retval EFI_INVALID_PARAMETER  NumInstalled was outside the allowed range on
                                  input.
@@ -584,7 +622,8 @@ Process2ndPassCmdAddPointer (
   IN     CONST ORDERED_COLLECTION      *Tracker,
   IN     EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol,
   IN OUT UINTN                         InstalledKey[INSTALLED_TABLES_MAX],
-  IN OUT INT32                         *NumInstalled
+  IN OUT INT32                         *NumInstalled,
+  IN OUT ORDERED_COLLECTION            *InstalledTables
   )
 {
   CONST ORDERED_COLLECTION_ENTRY                     *TrackerEntry;
@@ -679,6 +718,21 @@ Process2ndPassCmdAddPointer (
     return EFI_SUCCESS;
   }
 
+  Status = OrderedCollectionInsert (
+             InstalledTables, NULL, (VOID *)(UINTN)PointerValue);
+  if (EFI_ERROR (Status)) {
+    if (Status == RETURN_ALREADY_STARTED) {
+      //
+      // Already installed this table, don't try to do so again.
+      //
+      DEBUG ((DEBUG_VERBOSE, "%a: AcpiProtocol->InstallAcpiTable reports table "
+        "already installed, skipping. PointerValue=0x%Lx\n",
+        __FUNCTION__, PointerValue));
+      Status = EFI_SUCCESS;
+    }
+    return Status;
+  }
+
   if (*NumInstalled == INSTALLED_TABLES_MAX) {
     DEBUG ((EFI_D_ERROR, "%a: can't install more than %d tables\n",
       __FUNCTION__, INSTALLED_TABLES_MAX));
@@ -739,6 +793,8 @@ InstallQemuFwCfgTables (
   UINTN                    *InstalledKey;
   INT32                    Installed;
   ORDERED_COLLECTION_ENTRY *TrackerEntry, *TrackerEntry2;
+  ORDERED_COLLECTION       *InstalledTables;
+  ORDERED_COLLECTION_ENTRY *InstalledTableEntry;
 
   Status = QemuFwCfgFindFile ("etc/table-loader", &FwCfgItem, &FwCfgSize);
   if (EFI_ERROR (Status)) {
@@ -827,14 +883,21 @@ InstallQemuFwCfgTables (
     goto RollbackWritePointersAndFreeTracker;
   }
 
+  InstalledTables = OrderedCollectionInit (PointerCompare, PointerCompare);
+  if (InstalledTables == NULL) {
+    Status = EFI_OUT_OF_RESOURCES;
+    goto FreeKeys;
+  }
+
   //
   // second pass: identify and install ACPI tables
   //
   Installed = 0;
   for (LoaderEntry = LoaderStart; LoaderEntry < LoaderEnd; ++LoaderEntry) {
     if (LoaderEntry->Type == QemuLoaderCmdAddPointer) {
-      Status = Process2ndPassCmdAddPointer (&LoaderEntry->Command.AddPointer,
-                 Tracker, AcpiProtocol, InstalledKey, &Installed);
+      Status = Process2ndPassCmdAddPointer (
+                 &LoaderEntry->Command.AddPointer, Tracker, AcpiProtocol,
+                 InstalledKey, &Installed, InstalledTables);
       if (EFI_ERROR (Status)) {
         goto UninstallAcpiTables;
       }
@@ -870,6 +933,12 @@ UninstallAcpiTables:
     DEBUG ((EFI_D_INFO, "%a: installed %d tables\n", __FUNCTION__, Installed));
   }
 
+  while (((InstalledTableEntry = OrderedCollectionMax(InstalledTables))) != NULL) {
+    OrderedCollectionDelete (InstalledTables, InstalledTableEntry, NULL);
+  }
+  OrderedCollectionUninit (InstalledTables);
+
+FreeKeys:
   FreePool (InstalledKey);
 
 RollbackWritePointersAndFreeTracker:
-- 
2.3.2 (Apple Git-55)