Attention is currently required from: Arthur Heymans, Cliff Huang, David Milosevic, Lance Zhao, Martin L Roth, Maximilian Brune, Patrick Rudolph, Tim Wawrzynczak.
David Milosevic has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78071?usp=email )
Change subject: acpi: Add PPTT support ......................................................................
Patch Set 16:
(2 comments)
File src/acpi/acpi_pptt.c:
https://review.coreboot.org/c/coreboot/+/78071/comment/de5c67cd_3ce62b91 : PS8, Line 13: * Disclaimer: : * : * The generated PPTT table might not be optimized for space, since this : * implementation creates unique resources (caches) for each CPU provided by : * the topology tree. Going by the ACPI 6.4 spec, this is fine. We do this to : * avoid further edge cases and keep the logic as simple as possible. : * : * "though less space efficient, it is also acceptable to declare a node : * for each instance of a resource. In the example above, it would be legal to : * declare an L1 for each processor." : * : * "Compaction of identical resources must be avoided if an implementation requires : * any resource instance to be referenced uniquely. For example, in the above example, : * the L1 resource of each processor must be declared using a dedicated structure to : * permit unique references to it." : * Okay. Newest patchset was successfully tested against the qemu-sbsa board (the mainboard part of the PPTT patch will be upstreamed soon). Converging resource lists works now. So I would consider this
There is a related quirk that is, IMO, much more worth to mention: The current
structures don't allow to converge lists (cf. Fig. 5.15: Cache Type Structure - Type 1 Example; here, L1D and L1I should point to the same L2 object as next level). I'm not sure if that is something for a comment or something to fix.
as done.
Furthermore, I have removed the disclaimer. Would be nice if we could finalize this patch as soon as possible :)
File src/include/acpi/acpi.h:
https://review.coreboot.org/c/coreboot/+/78071/comment/b8808340_7d7a2f70 : PS3, Line 1442: structures
The current structures and code, however, don't express everything ACPI can do
(cf. comment on the "Disclaimer" comment in the source).
The newest patchset is able to converge resource lists. Any further concerns from your side? :)
It's usually hard to get without
multiple, different, real-world examples (and right now we don't even have one under review, AFAICS)
We have implemented PPTT for two mainboards. They will be upstreamed soon.
-
David Milosevic (Code Review)