If multiple QEMU_LOADER_ADD_POINTER linker/loader commands point to the same offset in the same fw_cfg blob, *and* the data at that offset looks like an ACPI table header, OVMF will pass the table to EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() multiple times (once for each QEMU_LOADER_ADD_POINTER command), in the second phase of processing.

This is undesirable for two reasons:

- For some table types, only one installed instance is permitted by the ACPI spec, therefore MdeModulePkg/Universal/Acpi/AcpiTableDxe justifiedly rejects the request with EFI_ACCESS_DENIED. Which in turn aborts and rolls back the processing of the linker/loader script.

- Other types are allowed to have several instances installed (SSDT is one example), therefore MdeModulePkg/Universal/Acpi/AcpiTableDxe will not reject the multi-install. But that's even worse, because we would end up with multiple instances of the exact same table, just because several QEMU_LOADER_ADD_POINTER commands reference it.

The improvement is to short-circuit Process2ndPassCmdAddPointer() [OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c] with memoization. After computing "PointerValue", if it has ever been seen / handled by Process2ndPassCmdAddPointer(), exit immediately with EFI_SUCCESS. This is justified because all actions after that point would be:

- either idempotent:
  - recognize RSDT / XSDT and return with EFI_SUCCESS without doing anything,
  - recognize a non-SDT-header pattern under "PointerValue", and mark the
    containing blob (again) as "hosts some other things than ACPI tables",
    and return with EFI_SUCCESS
- or incorrect:
  - see the duplicate EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() call
    described above.

Calling OrderedCollectionInsert() is entirely right for this, because
- it simultaneously inserts the new item, and finds any existing, colliding
  item;
- if OrderedCollectionInsert() fails due to out of memory, that's good enough
  reason to abort the processing;
- if OrderedCollectionInsert() succeeds, but
  EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() fails (for some other reason), we
  don't even need to roll back the insertion, since the failure of the latter
  is fatal anyway... but, for general cleanliness, we should still invoke
  OrderedCollectionDelete(). (The Node parameter is readily available from
  the successful OrderedCollectionInsert() call.)

QEMU doesn't yet produce a linker/loader script that leads to duplicate table installation, but there's an upcoming use case:

msgid <CAAibmn1eEFMdn_-hcaFudOO8D7crkfYYw+k88=CGJfJ0fmU6JA@mail.gmail.com>
https://www.mail-archive.com/qemu-devel@nongnu.org/msg427875.html

On 02/06/17 17:30, Phil Dennis-Jordan wrote:
>  - OS X/macOS guests currently can't reboot with upstream Qemu, as it
> expects the reset register to be advertised by the FADT, but x86 Qemu
> currently only publishes a rev1 FADT, which lacks said register spec.
> (OVMF still requires a large array of patches to boot OS X/macOS,
> which is a separate issue.)
>  - I therefore would like to update Qemu such that it publishes a rev3
> (ACPI 2.0) or newer FADT, including the reset register.
>  - Windows 10 appears to require both DSDT AND X_DSDT to be filled for
> rev3-rev4 FADTs, else it won't boot at all. (Not sure about 5+)
>  - Guest OS backwards compatibility without a flag is desirable, and
> ACPI 1.0 era guest OSes will not try to find the DSDT via X_DSDT, so
> we need to fill both.
>  - This should work with both SeaBIOS and OVMF, so Qemu needs to set
> up linker commands for both DSDT and X_DSDT, and OVMF itself needs to
> produce a FADT with both fields filled.