Attention is currently required from: Jason Glenesk, Martin L Roth, Subrata Banik, Eric Lai, Andrey Petrov, Fred Reitberger, Felix Held.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69412 )
Change subject: drivers/fsp2: Don't die if the FSP signature doesn't match ......................................................................
Patch Set 3:
(2 comments)
Patchset:
PS3:
coreboot should not (and honestly cannot) be responsible for issues inside the FSP. […]
Whether or not the FSP actually does the check is kind of beside the point to me. If it's the completely wrong FSP, presumably it's going to come across an error somewhere and die on it's own, or return to coreboot with an error. OR, if somehow it makes it through with no errors, but didn't configure things properly, coreboot will likely die somewhere else..
So either it's compatible enough to run, or something else will cause the boot to die without us needing to die here.
File src/drivers/intel/fsp2_0/util.c:
https://review.coreboot.org/c/coreboot/+/69412/comment/6c7a3958_05495aed PS3, Line 177: If it doesn't match, the : FSP binaries in CBFS are for a different platform than the platform code trying to use it
Sorry, I think I'm missing your point here. How is this coreboot's responsibility? […]
No, AMD isn't going to use a generic UPD ID. We have an issue where we're changing the UPD id right now which is what prompted me to look at this, but honestly I just don't think it's a good use of die().
I think die() should be used when the platform can't continue. Not when it thinks "there might be an error if I continue running, so let's halt now." The exception is if there's a security risk if we continue, and even then, i think there should be a way to continue booting so that firmware can be updated for example.
Trying to run from memory and there's no memory? Yep, perfect place to die. The system literally cannot continue running.
But this die call is just not needed. Sure, we can print a warning, but coreboot should try to continue booting.