Attention is currently required from: Subrata Banik.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77176?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: {driver, soc/intel/cmn/cse}: Refactor ISH FW Version implementation ......................................................................
Patch Set 1:
(1 comment)
File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/77176/comment/6ce6357e_5bef6531 : PS1, Line 1303: &version->cur_cse_fw_version, sizeof(struct fw_version))) { Oh, so you actually do need this to persist across multiple boots? Then maybe you do need an EARLY hook. But in that case you need to make sure the space is added at the same point with the same size regardless of boot mode (e.g. also in recovery mode), or it could screw up other EARLY hook spaces.
I would also suggest maybe adding a checksum (e.g. `CRC(..., crc32_byte)`, which it looks like we already link into ramstage for some ACPI stuff anyway) for this so you can be somewhat confident you're actually looking at valid data when you just read a random leftover region in memory. (Also, has anyone ever looked at the security implications of this? What are the potential implications if ISH version is misreported?)