[S] Change in coreboot[master]: drivers/fsp2: Don't die if the FSP signature doesn't match

[Subrata Banik (Code Review)]

Attention is currently required from: Jason Glenesk, Martin L Roth, Eric Lai, Andrey Petrov, Fred Reitberger, Felix Held.

Subrata Banik 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.
>
> If the FSP cannot run, it should be responsible for doing the check instead of pushing the responsibility to coreboot.  We don't do a check for the payload "Oh, this payload is running on a platform it wasn't compiled for" and coreboot shouldn't be responsible for the FSP either.

I agree that coreboot shouldn’t bother to own responsibility to check those and it's the FSP's responsibility to make those additional check but unfortunately Intel FSP doesn’t perform those additional checks. I can ask them by filling a bug to incorporate this for future SoC.

But comparing the payload against SoC ref code may not be applied as payload should be generic in nature and such header signature check doesn't seems required for payload.

File src/drivers/intel/fsp2_0/util.c:

https://review.coreboot.org/c/coreboot/+/69412/comment/e9adb582_078eb387 
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?

I was trying to stitch ADL FSP with MTL coreboot with this CL and I'm able to call into the ADL FSP but later it failed due to PCH ID mismatch inside FSP. IMO, the filtering inside coreboot is much better (as FSP is not doing such a check) as coreboot is stitching those binaries and calling into those, hence, better we just avoid calling into anything blindly and it results in a failure.

Additionally, Intel FSP specification mentions that each platform has a unique signature hence, IMO, it's better to just make sure we are calling into the appropriate ones.

0x00 – 0x07 UPD Region Signature. The signature will be
“XXXXXX_T” for FSP-T
“XXXXXX_M” for FSP-M
“XXXXXX_S” for FSP-S
“XXXXXX_I” for FSP-I
Where XXXXXX is a unique signature

wondering if you/AMD FSP planning to use a generic UPD ID across different SOC platform hence, you would like to skip the `die` or in general you think that `die` is bad idea on production AP image ?