From 072060a6f81b3e43473b9e5dcba7049ad9de4b18 Mon Sep 17 00:00:00 2001 From: Phil Dennis-Jordan Date: Thu, 30 Mar 2017 23:40:57 +1300 Subject: [PATCH] OvmfPkg: Allow multiple add-pointer linker commands to same ACPI table ACPI tables may contain multiple fields which point 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. Previously, if Qemu created QEMU_LOADER_ADD_POINTER linker commands for such instances, the linking process would attempt to install the same pointed-to table repeatedly. For tables of which there must only be one instance, the call to AcpiProtocol->InstallAcpiTable() would fail during the second linker command pointing to the same table, thus entirely aborting the ACPI table linking process. In the case of tables of which there may be multiple instances, the table would end up duplicated. This change adds a memoisation data structure which tracks the table pointers that have already been processed; even if the same pointer is encountered multiple times, it is only processed once. Cc: Jordan Justen Cc: Laszlo Ersek Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Phil Dennis-Jordan Reviewed-by: Laszlo Ersek [lersek@redhat.com: DSDT<->XSDT typo, debug msg, and coding style fixups] Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=368 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek --- OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 121 +++++++++++++++++++++--- 1 file changed, 108 insertions(+), 13 deletions(-) diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c index 7bb2e3f218..1bc5fe297a 100644 --- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c +++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c @@ -99,6 +99,39 @@ BlobCompare ( } +/** + Comparator function for two opaque pointers, ordering on (unsigned) 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; + } + if ((UINTN)Pointer1 < (UINTN)Pointer2) { + return -1; + } + return 1; +} + + /** Process a QEMU_LOADER_ALLOCATE command. @@ -557,6 +590,16 @@ UndoCmdWritePointer ( command identified an ACPI table that is different from RSDT and XSDT. + @param[in,out] SeenPointers The ORDERED_COLLECTION tracking the absolute + target addresses that have been pointed-to by + QEMU_LOADER_ADD_POINTER commands thus far. If a + target address is encountered for the first + time, and it identifies an ACPI table that is + different from RDST and XSDT, the table is + installed. If a target address is seen for the + second or later times, it is skipped without + taking any action. + @retval EFI_INVALID_PARAMETER NumInstalled was outside the allowed range on input. @@ -564,14 +607,15 @@ UndoCmdWritePointer ( table different from RSDT and XSDT, but there was no more room in InstalledKey. - @retval EFI_SUCCESS AddPointer has been processed. Either an ACPI - table different from RSDT and XSDT has been - installed (reflected by InstalledKey and - NumInstalled), or RSDT or XSDT has been - identified but not installed, or the fw_cfg - blob pointed-into by AddPointer has been - marked as hosting something else than just - direct ACPI table contents. + @retval EFI_SUCCESS AddPointer has been processed. Either its + absolute target address has been encountered + before, or an ACPI table different from RSDT + and XSDT has been installed (reflected by + InstalledKey and NumInstalled), or RSDT or + XSDT has been identified but not installed, or + the fw_cfg blob pointed-into by AddPointer has + been marked as hosting something else than + just direct ACPI table contents. @return Error codes returned by AcpiProtocol->InstallAcpiTable(). @@ -584,11 +628,13 @@ 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 *SeenPointers ) { CONST ORDERED_COLLECTION_ENTRY *TrackerEntry; CONST ORDERED_COLLECTION_ENTRY *TrackerEntry2; + ORDERED_COLLECTION_ENTRY *SeenPointerEntry; CONST BLOB *Blob; BLOB *Blob2; CONST UINT8 *PointerField; @@ -620,6 +666,27 @@ Process2ndPassCmdAddPointer ( Blob2Remaining += Blob2->Size; ASSERT (PointerValue < Blob2Remaining); + Status = OrderedCollectionInsert ( + SeenPointers, + &SeenPointerEntry, // for reverting insertion in error case + (VOID *)(UINTN)PointerValue + ); + if (EFI_ERROR (Status)) { + if (Status == RETURN_ALREADY_STARTED) { + // + // Already seen this pointer, don't try to process it again. + // + DEBUG (( + DEBUG_VERBOSE, + "%a: PointerValue=0x%Lx already processed, skipping.\n", + __FUNCTION__, + PointerValue + )); + Status = EFI_SUCCESS; + } + return Status; + } + Blob2Remaining -= (UINTN) PointerValue; DEBUG ((EFI_D_VERBOSE, "%a: checking for ACPI header in \"%a\" at 0x%Lx " "(remaining: 0x%Lx): ", __FUNCTION__, AddPointer->PointeeFile, @@ -682,7 +749,8 @@ Process2ndPassCmdAddPointer ( if (*NumInstalled == INSTALLED_TABLES_MAX) { DEBUG ((EFI_D_ERROR, "%a: can't install more than %d tables\n", __FUNCTION__, INSTALLED_TABLES_MAX)); - return EFI_OUT_OF_RESOURCES; + Status = EFI_OUT_OF_RESOURCES; + goto RollbackSeenPointer; } Status = AcpiProtocol->InstallAcpiTable (AcpiProtocol, @@ -691,10 +759,14 @@ Process2ndPassCmdAddPointer ( if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, "%a: InstallAcpiTable(): %r\n", __FUNCTION__, Status)); - return Status; + goto RollbackSeenPointer; } ++*NumInstalled; return EFI_SUCCESS; + +RollbackSeenPointer: + OrderedCollectionDelete (SeenPointers, SeenPointerEntry, NULL); + return Status; } @@ -739,6 +811,8 @@ InstallQemuFwCfgTables ( UINTN *InstalledKey; INT32 Installed; ORDERED_COLLECTION_ENTRY *TrackerEntry, *TrackerEntry2; + ORDERED_COLLECTION *SeenPointers; + ORDERED_COLLECTION_ENTRY *SeenPointerEntry, *SeenPointerEntry2; Status = QemuFwCfgFindFile ("etc/table-loader", &FwCfgItem, &FwCfgSize); if (EFI_ERROR (Status)) { @@ -827,14 +901,26 @@ InstallQemuFwCfgTables ( goto RollbackWritePointersAndFreeTracker; } + SeenPointers = OrderedCollectionInit (PointerCompare, PointerCompare); + if (SeenPointers == 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, + SeenPointers + ); if (EFI_ERROR (Status)) { goto UninstallAcpiTables; } @@ -870,6 +956,15 @@ UninstallAcpiTables: DEBUG ((EFI_D_INFO, "%a: installed %d tables\n", __FUNCTION__, Installed)); } + for (SeenPointerEntry = OrderedCollectionMin (SeenPointers); + SeenPointerEntry != NULL; + SeenPointerEntry = SeenPointerEntry2) { + SeenPointerEntry2 = OrderedCollectionNext (SeenPointerEntry); + OrderedCollectionDelete (SeenPointers, SeenPointerEntry, NULL); + } + OrderedCollectionUninit (SeenPointers); + +FreeKeys: FreePool (InstalledKey); RollbackWritePointersAndFreeTracker: -- 2.39.2