Attention is currently required from: Arthur Heymans, Raul Rangel, Nico Huber, Angel Pons, Karthik Ramasubramanian, Felix Held.
Robert Zieba has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67931 )
Change subject: cpu/x86/smm: Add PCI BAR store functionality ......................................................................
Patch Set 5:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/67931/comment/0b6d0cad_0cd00cb8 PS2, Line 11: This commit adds support to the existing SMM to allow storing : PCI BARs in SMRAM and then later retrieved.
Done […]
Done in the sense that it seems like the original question has been answered between this comment https://review.coreboot.org/c/coreboot/+/67931/comments/c2ba1641_2770009d and the discussion here.
File src/cpu/x86/Kconfig:
https://review.coreboot.org/c/coreboot/+/67931/comment/f77e66ac_bc8b57d5 PS5, Line 195: : config SMM_PCI_BAR_STORE : bool : depends on SMM_CHIPSET_STORE : help : This option enables support for storing PCI BARs in SMRAM so they : can't be tampered with. : : config SMM_PCI_BAR_STORE_NUM_SLOTS : int : default 8 : help : Number of slots available to store PCI BARs in SMRAM
I would remove this, since it's not really generic
I think the BAR store part of this is generic enough to leave in. The Intel XHCI event logging code is susceptible to the same problem and could use it as well. Though maybe I should get rid of the chipset store aspect? Because if the BAR store part of this is usable for AMD and Intel then maybe that should go directly into the smm runtime struct and not be contained in a chipset store.