Skip to content

AcpiPlatform: use GUIDed HOBs for VMM-provided ACPI tables#77

Open
jstarks wants to merge 1 commit into
microsoft:mainfrom
jstarks:ppi_blob
Open

AcpiPlatform: use GUIDed HOBs for VMM-provided ACPI tables#77
jstarks wants to merge 1 commit into
microsoft:mainfrom
jstarks:ppi_blob

Conversation

@jstarks
Copy link
Copy Markdown
Member

@jstarks jstarks commented May 15, 2026

Instead of having a dedicated config blob type, PCD pair, and DXE installer function for each ACPI table the VMM can provide (MADT, SRAT, SLIT, PPTT, HMAT, MCFG, SSDT, IORT, ASPT), use a single generic path: PEI validates the embedded ACPI header and emits a GUIDed HOB; DXE iterates the HOB list and installs each table, suppressing the matching FV-resident default.

The MADT installer is kept for the IGVM/TDX path, which needs to post-process the table (MP wakeup mailbox), but is now a no-op when the PCDs are unset. The IGVM SRAT is emitted as a HOB. The NFIT installer is unrelated (synthesized from BIOS device MMIO) and is unchanged. PciHostBridgeLib and PciSegmentInfoLib now read MCFG directly from the HOB list, removing PcdMcfgPtr/PcdMcfgSize.

Instead of having a dedicated config blob type, PCD pair, and DXE
installer function for each ACPI table the VMM can provide (MADT,
SRAT, SLIT, PPTT, HMAT, MCFG, SSDT, IORT, ASPT), use a single
generic path: PEI validates the embedded ACPI header and emits a
GUIDed HOB; DXE iterates the HOB list and installs each table,
suppressing the matching FV-resident default.

The MADT installer is kept for the IGVM/TDX path, which needs to
post-process the table (MP wakeup mailbox), but is now a no-op when
the PCDs are unset. The IGVM SRAT is emitted as a HOB. The NFIT
installer is unrelated (synthesized from BIOS device MMIO) and is
unchanged. PciHostBridgeLib and PciSegmentInfoLib now read MCFG
directly from the HOB list, removing PcdMcfgPtr/PcdMcfgSize.
EFI_ACPI_DESCRIPTION_HEADER *madtHdr = (EFI_ACPI_DESCRIPTION_HEADER*)madtStructure->Madt;

if (madtStructure->Header.Length < (sizeof(UEFI_CONFIG_HEADER) + sizeof(EFI_ACPI_DESCRIPTION_HEADER)) ||
madtHdr->Signature != EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE ||
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

madtHdr->Signature != EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE ||

We lost any per-table validation that used to happen (like here).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I think that's OK, since the validation here is not very interesting and it's really just helping diagnose VMM bugs... and poorly since most of the time you just get a random triple fault or reset if UEFI gives up.

@mebersol
Copy link
Copy Markdown
Collaborator

This change brings support for multi-instance tables provided by the VMM (e.g. SSDT); nice!

Perhaps we need to think about how to make a FV-provided SSDT can not be overridden by a VMM-provided extension. That can probably be deferred to a future PR

#endif
case UefiConfigMcfg:
case UefiConfigSsdt:
case UefiConfigIort:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

want to alphabetize?

In this model, we may not need separate types for all of these; just passing multiple instances of UefiConfigAcpiTable might do...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, we don't need these anymore. I think we should update the VMMs to just use the single generic blob and then we can reclaim all these other enum values.

Copy link
Copy Markdown
Collaborator

@mebersol mebersol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants