Attention is currently required from: Arthur Heymans, Felix Held, Lance Zhao, Tim Wawrzynczak.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76000?usp=email )
Change subject: acpi: Swap XSDT and RSDT in acpi_add_table() ......................................................................
Patch Set 5:
(9 comments)
Patchset:
PS5: Quite a few comments, but only minor things.
File src/acpi/acpi.c:
https://review.coreboot.org/c/coreboot/+/76000/comment/fd595bee_24c73331 : PS5, Line 60: RSDT Please mention why, e.g. "The 32-bit RSDT may not exist if tables live above 4G.".
https://review.coreboot.org/c/coreboot/+/76000/comment/53074fbb_8174319a : PS5, Line 62: xsdt = (acpi_xsdt_t *)((uintptr_t)rsdp->xsdt_address); Please align usage of parentheses. I'd prefer the version with less.
https://review.coreboot.org/c/coreboot/+/76000/comment/55ad99b7_2596312a : PS5, Line 81: R another `R`
https://review.coreboot.org/c/coreboot/+/76000/comment/79084f5c_524780d8 : PS5, Line 94: rsdt->entry[i] = (u32)(uintptr_t)table; Only set `if ((uintptr_t)table <= UINT32_MAX)`?
https://review.coreboot.org/c/coreboot/+/76000/comment/35e37f83_9fb40143 : PS5, Line 97: rsdt->header.length = sizeof(acpi_header_t) + : (sizeof(u32) * (i + 1)); : : /* Re-calculate checksum. */ : rsdt->header.checksum = 0; : rsdt->header.checksum = acpi_checksum((u8 *)rsdt, : rsdt->header.length); Please drop early line breaks if we change the lines anyway.
https://review.coreboot.org/c/coreboot/+/76000/comment/02c2d114_c1184a62 : PS5, Line 1962: /* We need at least an RSDP and an RSDT Table */ Comment needs an update, RSDP/RSDT for ACPI 1.0 compat, otherwise XSDT.
https://review.coreboot.org/c/coreboot/+/76000/comment/d748f016_7837e9f6 : PS5, Line 1967: if (current < UINT32_MAX) { While I couldn't find it specified, I'd assume the whole RSDT should fit 32-bit space, i.e. `current + sizeof(acpi_rsdt_t) - 1 <= UINT32_MAX`.
https://review.coreboot.org/c/coreboot/+/76000/comment/449076c2_965d078a : PS5, Line 1971: } `else printk(BIOS_INFO, "Not adding RSDP because tables reside above 4G.");`?