Attention is currently required from: Martin Roth, Michał Żygowski, Marshall Dawson, Subrata Banik, Nikolai Vyssotski, Andrey Petrov, Patrick Rudolph, Nathaniel L Desimone. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56190 )
Change subject: src/drivers/intel/fsp2_0: allow larger FSP 2.0 header ......................................................................
Patch Set 3:
(1 comment)
File src/drivers/intel/fsp2_0/include/fsp/info_header.h:
https://review.coreboot.org/c/coreboot/+/56190/comment/f51ab449_293bf5df PS2, Line 13: however, some FSP 2.0 variants have it as well.
The FSP header has a length field. If the length is actually specified as a fixed length by the header version, this field is redundant.
That is how the spec is defined. The header length is fixed for a particular version. It is a good sanity check to ensure that the header length is what's expected as per the advertised version.
looks like we are always overriding this from FSP package since we introduced FSP2.2 spec.
Subrata, why is that? Is this a miss in the EDK2 CL or is that intentional? I think this needs to be fixed in EDK2, but please correct me if there are reasons why that is not true.
I think the real solution is a fix from intel, either to EDK2 or the FSP spec.
I think this needs to go in EDK2. Nate/Subrata can correct if this expectation is not right.
Until that happens though, I think we need a solution for the current situation.
Pcds can be overriden in FSP package. Cezanne FSP package is already doing that in AmdCezannePkg.dsc for `gIntelFsp2PkgTokenSpaceGuid.PcdTemporaryRamBase`, `gIntelFsp2PkgTokenSpaceGuid.PcdTemporaryRamSize`, etc. Similarly, it can provide `gIntelFsp2PkgTokenSpaceGuid.PcdFspHeaderSpecVersion|0x22`. I agree that this should be fixed in EDK2, but until that is done, this workaround in FSP package should work.
-
Furquan Shaikh (Code Review)