Attention is currently required from: Furquan Shaikh, Martin Roth, Michał Żygowski, Marshall Dawson, Subrata Banik, Andrey Petrov, Patrick Rudolph, Nathaniel L Desimone.
Nikolai Vyssotski 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/8e953b59_ff1ed33e
PS2, Line 13: however, some FSP 2.0 variants have it as well.
I don't think we should have the header length hardcoded, and should instead rely on that length f […]
I agree with Martin. This is not a matter of simply reporting a different FSP version via PcdFspHeaderSpecVersion (please note that it goes to 'SpecVersion' header field - naming is somewhat misleading here). If we change it to 2.2 we are implying that our FSP is 2.2 compliant which is not the case. We are FSP 2.0 compliant right now. If we were to report a different header version it would be via 'HeaderRevision' field (FSP_HEADER_REVISION_3 macro currently) but as I mentioned before we have no authority over issuing the FSP header versions so we may have a collision if we pick an arbitrary number to indicate different FSP 2.0 header length. I am ok with the solution you proposed above. It is better to check for a minimum length than not checking it at all.
--
To view, visit
https://review.coreboot.org/c/coreboot/+/56190
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie8422447b2cff0a6c536e13014905ffa15c70586
Gerrit-Change-Number: 56190
Gerrit-PatchSet: 3
Gerrit-Owner: Nikolai Vyssotski
nikolai.vyssotski@amd.corp-partner.google.com
Gerrit-Reviewer: Andrey Petrov
andrey.petrov@gmail.com
Gerrit-Reviewer: Furquan Shaikh
furquan@google.com
Gerrit-Reviewer: Marshall Dawson
marshalldawson3rd@gmail.com
Gerrit-Reviewer: Martin Roth
martinroth@google.com
Gerrit-Reviewer: Nathaniel L Desimone
nathaniel.l.desimone@intel.com
Gerrit-Reviewer: Patrick Rudolph
siro@das-labor.org
Gerrit-Reviewer: Subrata Banik
subrata.banik@intel.com
Gerrit-Reviewer: build bot (Jenkins)
no-reply@coreboot.org
Gerrit-CC: Michał Żygowski
michal.zygowski@3mdeb.com
Gerrit-CC: Raul Rangel
rrangel@chromium.org
Gerrit-Attention: Furquan Shaikh
furquan@google.com
Gerrit-Attention: Martin Roth
martinroth@google.com
Gerrit-Attention: Michał Żygowski
michal.zygowski@3mdeb.com
Gerrit-Attention: Marshall Dawson
marshalldawson3rd@gmail.com
Gerrit-Attention: Subrata Banik
subrata.banik@intel.com
Gerrit-Attention: Andrey Petrov
andrey.petrov@gmail.com
Gerrit-Attention: Patrick Rudolph
siro@das-labor.org
Gerrit-Attention: Nathaniel L Desimone
nathaniel.l.desimone@intel.com
Gerrit-Comment-Date: Wed, 14 Jul 2021 00:09:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh
furquan@google.com
Comment-In-Reply-To: Martin Roth
martinroth@google.com
Comment-In-Reply-To: Michał Żygowski
michal.zygowski@3mdeb.com
Comment-In-Reply-To: Nikolai Vyssotski
nikolai.vyssotski@amd.corp-partner.google.com
Gerrit-MessageType: comment