UefiPayloadEntry/AcpiTable.c open-codes two walks of the XSDT/RSDT from the bootloader-supplied RSDP to find the FADT and MCFG for the ACPI board info HOB, and validates almost nothing on the way: not the RSDP signature, not the SDT signature, not whether the SDT length covers its own header before subtracting it, not whether an entry pointer is NULL. A bootloader that hands over a malformed RSDP or an SDT whose reported length is shorter than its own header sends the loop on a wild walk.
Introduce AcpiTableWalkLib, a BASE library depending only on BaseLib and DebugLib so it links from the SEC-phase payload entry, a DXE_DRIVER NULL library and an outer-firmware UEFI application alike. Its AcpiFindTableFromRsdp() validates everything first, prefers the XSDT and only walks the RSDT when the RSDP is Revision 0 or has no XsdtAddress, and reads XSDT entry pointers with ReadUnaligned64() because the 36-byte common header leaves the 64-bit entry array 4-byte aligned. Convert AcpiTable.c to it; later patches in this series that need an ACPI table walk use the helper from birth instead of growing their own copies. For AcpiTable.c the shared walk is stricter than what it replaces: a wrong RSDP signature, or an RSDT/XSDT shorter than its own header, now yields RETURN_NOT_FOUND. It also prefers XSDT where the open-coded loop tried RSDT first; on any platform with both they must reference the same tables, so the preference changes nothing. Cc: Benjamin Doron <[email protected]> Cc: Gua Guo <[email protected]> Cc: Guo Dong <[email protected]> Cc: James Lu <[email protected]> Cc: Sean Rhodes <[email protected]> Cc: Shuo Liu <[email protected]> Assisted-by: claude-opus-5 Signed-off-by: Alexander Graf <[email protected]> --- .../Include/Library/AcpiTableWalkLib.h | 43 +++++ .../AcpiTableWalkLib/AcpiTableWalkLib.c | 169 ++++++++++++++++++ .../AcpiTableWalkLib/AcpiTableWalkLib.inf | 29 +++ UefiPayloadPkg/UefiPayloadEntry/AcpiTable.c | 74 +------- .../FitUniversalPayloadEntry.inf | 1 + .../UefiPayloadEntry/UefiPayloadEntry.h | 1 + .../UefiPayloadEntry/UefiPayloadEntry.inf | 1 + .../UniversalPayloadEntry.inf | 1 + UefiPayloadPkg/UefiPayloadPkg.dec | 2 + UefiPayloadPkg/UefiPayloadPkg.dsc | 1 + 10 files changed, 256 insertions(+), 66 deletions(-) create mode 100644 UefiPayloadPkg/Include/Library/AcpiTableWalkLib.h create mode 100644 UefiPayloadPkg/Library/AcpiTableWalkLib/AcpiTableWalkLib.c create mode 100644 UefiPayloadPkg/Library/AcpiTableWalkLib/AcpiTableWalkLib.inf diff --git a/UefiPayloadPkg/Include/Library/AcpiTableWalkLib.h b/UefiPayloadPkg/Include/Library/AcpiTableWalkLib.h new file mode 100644 index 0000000000..0b5debc121 --- /dev/null +++ b/UefiPayloadPkg/Include/Library/AcpiTableWalkLib.h @@ -0,0 +1,43 @@ +/** @file + Locate ACPI tables by signature via a bootloader-supplied RSDP. + + Copyright (c) 2026, Amazon.com, Inc. or its affiliates.<BR> + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ + +#ifndef ACPI_TABLE_WALK_LIB_H_ +#define ACPI_TABLE_WALK_LIB_H_ + +#include <Uefi/UefiBaseType.h> +#include <IndustryStandard/Acpi.h> + +/** + Locate an ACPI table by signature via the RSDP. + + Walks the XSDT (or the RSDT if the RSDP is Revision 0 or has no + XSDT address) referenced by @a Rsdp and returns the first table + whose header signature matches @a Signature. + + The RSDP signature, the XSDT/RSDT signature and length, and each + entry pointer are all validated before use. A Length shorter than + the common ACPI description header would underflow the entry-count + subtraction and walk off the end of the table; that case is + rejected. XSDT entry pointers are read with ReadUnaligned64() + because the 36-byte header leaves the 64-bit entry array 4-byte + aligned. + + @param[in] Rsdp Physical address of the ACPI RSDP, or 0. + @param[in] Signature 4-byte ACPI table signature to find. + + @return Pointer to the first matching table's description header, + or NULL if @a Rsdp is 0, the RSDP or SDT header is invalid, + or no table with @a Signature is present. +**/ +EFI_ACPI_DESCRIPTION_HEADER * +EFIAPI +AcpiFindTableFromRsdp ( + IN EFI_PHYSICAL_ADDRESS Rsdp, + IN UINT32 Signature + ); + +#endif // ACPI_TABLE_WALK_LIB_H_ diff --git a/UefiPayloadPkg/Library/AcpiTableWalkLib/AcpiTableWalkLib.c b/UefiPayloadPkg/Library/AcpiTableWalkLib/AcpiTableWalkLib.c new file mode 100644 index 0000000000..057463bcc4 --- /dev/null +++ b/UefiPayloadPkg/Library/AcpiTableWalkLib/AcpiTableWalkLib.c @@ -0,0 +1,169 @@ +/** @file + Locate ACPI tables by signature via a bootloader-supplied RSDP. + + UefiPayloadPkg has three separate places that walk the RSDP's + XSDT/RSDT to find a table by signature: ChainloadApp (before it + builds the payload's HOB list, and again after ExitBootServices()), + AcpiGicPcdLib (from the RSDP handed over in the ACPI HOB), and + UefiPayloadEntry/AcpiTable.c (deriving the ACPI board info HOB). + Each carried its own bounds checking, and each was subtly stricter + or laxer than the others. This library is the single validated + walk they now share. + + The library is BASE and depends only on BaseLib and DebugLib, so it + links into a SEC-phase payload entry, a DXE_DRIVER NULL library and + a UEFI_APPLICATION running under an outer firmware alike. + + Copyright (c) 2026, Amazon.com, Inc. or its affiliates.<BR> + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ + +#include <Base.h> +#include <IndustryStandard/Acpi.h> + +#include <Library/AcpiTableWalkLib.h> +#include <Library/BaseLib.h> +#include <Library/DebugLib.h> + +/** + Return the first table with @a Signature in a validated system + description table. + + @param[in] Sdt The XSDT or RSDT header. + @param[in] SdtSignature The signature @a Sdt must carry. + @param[in] EntrySize sizeof (UINT64) for XSDT, sizeof (UINT32) for RSDT. + @param[in] Signature The 4-byte table signature to find. + + @return Pointer to the matching table header, or NULL. +**/ +STATIC +EFI_ACPI_DESCRIPTION_HEADER * +FindInSdt ( + IN EFI_ACPI_DESCRIPTION_HEADER *Sdt, + IN UINT32 SdtSignature, + IN UINTN EntrySize, + IN UINT32 Signature + ) +{ + EFI_ACPI_DESCRIPTION_HEADER *Tbl; + UINT8 *Entry; + UINTN Count; + UINTN Idx; + UINTN Addr; + + // + // Validate the SDT before deriving an entry count from its length. + // A Length below the header size would wrap the unsigned subtraction + // and walk the loop off the end of the table. + // + if ((Sdt->Signature != SdtSignature) || + (Sdt->Length < sizeof (EFI_ACPI_DESCRIPTION_HEADER))) + { + DEBUG (( + DEBUG_WARN, + "%a: bad SDT at 0x%p: signature 0x%x, length %u\n", + __func__, + Sdt, + Sdt->Signature, + Sdt->Length + )); + return NULL; + } + + Entry = (UINT8 *)(Sdt + 1); + Count = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / EntrySize; + + for (Idx = 0; Idx < Count; Idx++) { + if (EntrySize == sizeof (UINT64)) { + // + // The 36-byte common header leaves the 64-bit entry array + // 4-byte aligned, so an aligned load may fault on a strict + // architecture. + // + Addr = (UINTN)ReadUnaligned64 ((UINT64 *)Entry); + } else { + Addr = (UINTN)ReadUnaligned32 ((UINT32 *)Entry); + } + + Entry += EntrySize; + + Tbl = (EFI_ACPI_DESCRIPTION_HEADER *)Addr; + if ((Tbl != NULL) && (Tbl->Signature == Signature)) { + return Tbl; + } + } + + return NULL; +} + +/** + Locate an ACPI table by signature via the RSDP. + + Walks the XSDT (or the RSDT if the RSDP is Revision 0 or has no + XSDT address) referenced by @a Rsdp and returns the first table + whose header signature matches @a Signature. + + The RSDP signature, the XSDT/RSDT signature and length, and each + entry pointer are all validated before use. A Length shorter than + the common ACPI description header would underflow the entry-count + subtraction and walk off the end of the table; that case is + rejected. XSDT entry pointers are read with ReadUnaligned64() + because the 36-byte header leaves the 64-bit entry array 4-byte + aligned. + + @param[in] Rsdp Physical address of the ACPI RSDP, or 0. + @param[in] Signature 4-byte ACPI table signature to find. + + @return Pointer to the first matching table's description header, + or NULL if @a Rsdp is 0, the RSDP or SDT header is invalid, + or no table with @a Signature is present. +**/ +EFI_ACPI_DESCRIPTION_HEADER * +EFIAPI +AcpiFindTableFromRsdp ( + IN EFI_PHYSICAL_ADDRESS Rsdp, + IN UINT32 Signature + ) +{ + EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_POINTER *Rp; + + if (Rsdp == 0) { + return NULL; + } + + Rp = (EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_POINTER *)(UINTN)Rsdp; + if (Rp->Signature != EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_POINTER_SIGNATURE) { + DEBUG (( + DEBUG_WARN, + "%a: RSDP at 0x%Lx has bad signature 0x%Lx\n", + __func__, + (UINT64)Rsdp, + Rp->Signature + )); + return NULL; + } + + // + // ACPI 6.5 5.2.5.3: XsdtAddress is present only for Revision >= 2. + // Prefer the XSDT when present; fall back to the RSDT otherwise. + // + if ((Rp->Revision >= 2) && (Rp->XsdtAddress != 0)) { + return FindInSdt ( + (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rp->XsdtAddress, + EFI_ACPI_6_5_EXTENDED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE, + sizeof (UINT64), + Signature + ); + } + + if (Rp->RsdtAddress != 0) { + return FindInSdt ( + (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rp->RsdtAddress, + EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_TABLE_SIGNATURE, + sizeof (UINT32), + Signature + ); + } + + return NULL; +} diff --git a/UefiPayloadPkg/Library/AcpiTableWalkLib/AcpiTableWalkLib.inf b/UefiPayloadPkg/Library/AcpiTableWalkLib/AcpiTableWalkLib.inf new file mode 100644 index 0000000000..e1a0e47b36 --- /dev/null +++ b/UefiPayloadPkg/Library/AcpiTableWalkLib/AcpiTableWalkLib.inf @@ -0,0 +1,29 @@ +## @file +# Locate ACPI tables by signature via a bootloader-supplied RSDP. +# +# Copyright (c) 2026, Amazon.com, Inc. or its affiliates.<BR> +# SPDX-License-Identifier: BSD-2-Clause-Patent +## + +[Defines] + INF_VERSION = 0x00010005 + BASE_NAME = AcpiTableWalkLib + FILE_GUID = 25DAFD0A-4FB0-497F-A069-2F7E0DE6819E + MODULE_TYPE = BASE + VERSION_STRING = 1.0 + LIBRARY_CLASS = AcpiTableWalkLib + +# +# VALID_ARCHITECTURES = IA32 X64 AARCH64 RISCV64 +# + +[Sources] + AcpiTableWalkLib.c + +[Packages] + MdePkg/MdePkg.dec + UefiPayloadPkg/UefiPayloadPkg.dec + +[LibraryClasses] + BaseLib + DebugLib diff --git a/UefiPayloadPkg/UefiPayloadEntry/AcpiTable.c b/UefiPayloadPkg/UefiPayloadEntry/AcpiTable.c index 503257efe8..47ec8c773b 100644 --- a/UefiPayloadPkg/UefiPayloadEntry/AcpiTable.c +++ b/UefiPayloadPkg/UefiPayloadEntry/AcpiTable.c @@ -25,81 +25,23 @@ ParseAcpiInfo ( OUT ACPI_BOARD_INFO *AcpiBoardInfo ) { - EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp; - EFI_ACPI_DESCRIPTION_HEADER *Rsdt; - UINT32 *Entry32; - UINTN Entry32Num; EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt; - EFI_ACPI_DESCRIPTION_HEADER *Xsdt; - UINT64 *Entry64; - UINTN Entry64Num; - UINTN Idx; - UINT32 *Signature; EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER *MmCfgHdr; EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE *MmCfgBase; - Rsdp = (EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER *)(UINTN)AcpiTableBase; - DEBUG ((DEBUG_INFO, "Rsdp at 0x%p\n", Rsdp)); - DEBUG ((DEBUG_INFO, "Rsdt at 0x%x, Xsdt at 0x%lx\n", Rsdp->RsdtAddress, Rsdp->XsdtAddress)); - - // - // Search Rsdt First - // - Fadt = NULL; - MmCfgHdr = NULL; - Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)(Rsdp->RsdtAddress); - if (Rsdt != NULL) { - Entry32 = (UINT32 *)(Rsdt + 1); - Entry32Num = (Rsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) >> 2; - for (Idx = 0; Idx < Entry32Num; Idx++) { - Signature = (UINT32 *)(UINTN)Entry32[Idx]; - if (*Signature == EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) { - Fadt = (EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)Signature; - DEBUG ((DEBUG_INFO, "Found Fadt in Rsdt\n")); - } - - if (*Signature == EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE) { - MmCfgHdr = (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER *)Signature; - DEBUG ((DEBUG_INFO, "Found MM config address in Rsdt\n")); - } - - if ((Fadt != NULL) && (MmCfgHdr != NULL)) { - goto Done; - } - } - } - - // - // Search Xsdt Second - // - Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)(Rsdp->XsdtAddress); - if (Xsdt != NULL) { - Entry64 = (UINT64 *)(Xsdt + 1); - Entry64Num = (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) >> 3; - for (Idx = 0; Idx < Entry64Num; Idx++) { - Signature = (UINT32 *)(UINTN)ReadUnaligned64 (&Entry64[Idx]); - if (*Signature == EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) { - Fadt = (EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)Signature; - DEBUG ((DEBUG_INFO, "Found Fadt in Xsdt\n")); - } - - if (*Signature == EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE) { - MmCfgHdr = (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER *)Signature; - DEBUG ((DEBUG_INFO, "Found MM config address in Xsdt\n")); - } - - if ((Fadt != NULL) && (MmCfgHdr != NULL)) { - goto Done; - } - } - } + Fadt = (EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)AcpiFindTableFromRsdp ( + AcpiTableBase, + EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE + ); + MmCfgHdr = (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER *)AcpiFindTableFromRsdp ( + AcpiTableBase, + EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE + ); if (Fadt == NULL) { return RETURN_NOT_FOUND; } -Done: - AcpiBoardInfo->PmCtrlRegBase = Fadt->Pm1aCntBlk; AcpiBoardInfo->PmTimerRegBase = Fadt->PmTmrBlk; AcpiBoardInfo->ResetRegAddress = Fadt->ResetReg.Address; diff --git a/UefiPayloadPkg/UefiPayloadEntry/FitUniversalPayloadEntry.inf b/UefiPayloadPkg/UefiPayloadEntry/FitUniversalPayloadEntry.inf index e93116fb1e..2206641c31 100644 --- a/UefiPayloadPkg/UefiPayloadEntry/FitUniversalPayloadEntry.inf +++ b/UefiPayloadPkg/UefiPayloadEntry/FitUniversalPayloadEntry.inf @@ -55,6 +55,7 @@ UefiPayloadPkg/UefiPayloadPkg.dec [LibraryClasses] + AcpiTableWalkLib BaseMemoryLib DebugLib BaseLib diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h index f314ffb490..4d08702680 100644 --- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h +++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h @@ -10,6 +10,7 @@ #include <PiPei.h> +#include <Library/AcpiTableWalkLib.h> #include <Library/BaseLib.h> #include <Library/BaseMemoryLib.h> #include <Library/MemoryAllocationLib.h> diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf index 56489cba88..5bc085e942 100644 --- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf +++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf @@ -47,6 +47,7 @@ UefiPayloadPkg/UefiPayloadPkg.dec [LibraryClasses] + AcpiTableWalkLib BaseMemoryLib DebugLib BaseLib diff --git a/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf b/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf index a834daa0f3..7270fdfa0e 100644 --- a/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf +++ b/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf @@ -39,6 +39,7 @@ UefiCpuPkg/UefiCpuPkg.dec UefiPayloadPkg/UefiPayloadPkg.dec [LibraryClasses] + AcpiTableWalkLib BaseMemoryLib DebugLib BaseLib diff --git a/UefiPayloadPkg/UefiPayloadPkg.dec b/UefiPayloadPkg/UefiPayloadPkg.dec index 2e13500729..63ccdb06ea 100644 --- a/UefiPayloadPkg/UefiPayloadPkg.dec +++ b/UefiPayloadPkg/UefiPayloadPkg.dec @@ -18,6 +18,8 @@ Include [LibraryClasses] + ## @libraryclass Locate ACPI tables by signature via a bootloader-supplied RSDP. + AcpiTableWalkLib|Include/Library/AcpiTableWalkLib.h PayloadEntryHelperLib|Include/Library/PayloadEntryHelperLib.h [Guids] diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc index 34fb690d17..07fbc3fbf9 100644 --- a/UefiPayloadPkg/UefiPayloadPkg.dsc +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc @@ -407,6 +407,7 @@ BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf !endif UefiCpuBaseArchSupportLib|UefiCpuPkg/Library/BaseArchSupportLib/BaseArchSupportLib.inf + AcpiTableWalkLib|UefiPayloadPkg/Library/AcpiTableWalkLib/AcpiTableWalkLib.inf [LibraryClasses.X64] # -- 2.47.3 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#122089): https://edk2.groups.io/g/devel/message/122089 Mute This Topic: https://groups.io/mt/120797204/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
