[coreboot-gerrit] [L] Change in coreboot[master]: soc/amd/genoa: Add Kconfig/Makefile to generate PSP image

[Marshall Dawson (Code Review)]

Attention is currently required from: Arthur Heymans, Varshit Pandya.

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76498?usp=email )

Change subject: soc/amd/genoa: Add Kconfig/Makefile to generate PSP image
......................................................................

Patch Set 4:

(13 comments)

File src/soc/amd/genoa/Kconfig:

https://review.coreboot.org/c/coreboot/+/76498/comment/77a020ff_051979ef :
PS4, Line 75: config AMD_FWM_POSITION_INDEX
            : 	default 5
With no prompt, this won't be choosable by the developer via menuconfig.

https://review.coreboot.org/c/coreboot/+/76498/comment/557342e2_1e12b84c :
PS4, Line 93: config PSP_LOAD_MP2_FW
Looks like unsupported in Epyc?  So it could be omitted here.

https://review.coreboot.org/c/coreboot/+/76498/comment/40fb4a01_737ade76 :
PS4, Line 117: "3rdparty/amd_blobs_internal/genoa/PSP/wtl-gno.bin"
Ideally this should be in "site-local/3rdparty/amd_blobs/genoa/PSP/wtl-gno.sbin" instead.

https://review.coreboot.org/c/coreboot/+/76498/comment/0af7b8a0_0bbf9aad :
PS4, Line 125: "3rdparty/amd_blobs_internal/genoa/PSP/PspDirL10_Typex55_BLAntiRB.bin
Maybe 3rdparty/amd_blobs/genoa/PSP/Typex55_0_0_0_BLAntiRB.bin?  Is that the same one?

https://review.coreboot.org/c/coreboot/+/76498/comment/8272db6b_3f6f178a :
PS4, Line 137: Bit 29: Disable MP2 firmware loading (Set by PSP_LOAD_MP2_FW)
and here

https://review.coreboot.org/c/coreboot/+/76498/comment/dd1b79e0_562e2b3f :
PS4, Line 139: 55758 
#57299

https://review.coreboot.org/c/coreboot/+/76498/comment/1ca1be35_5852e9bb :
PS4, Line 156: 
Remove extra blank line

https://review.coreboot.org/c/coreboot/+/76498/comment/c34668b6_8436eb6c :
PS4, Line 157: endif
             : 
nit: for consistency maybe

endif # SOC_AMD_GENOA

File src/soc/amd/genoa/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/76498/comment/7e9550f2_f02a1450 :
PS4, Line 23: PSPDIR ADDR  |  BIOSDIR ADDR
If I'm reading the spec right BIOSDIR_ADDR may be 0x28.  Or maybe the spec is ambiguous.

https://review.coreboot.org/c/coreboot/+/76498/comment/c5ccf0a7_3b28d75b :
PS4, Line 26: GENOA_FWM_POSITION=0x20000
Hmm, we seem to do MDN differently than other SOCs.  Without taking the time to dig, it seems like this ought to be derived from the position index instead of hardcoded.

https://review.coreboot.org/c/coreboot/+/76498/comment/c2c4b7d1_bd12b296 :
PS4, Line 51: ifeq ($(CONFIG_PSP_LOAD_MP2_FW),y)
            : OPT_PSP_LOAD_MP2_FW="--load-mp2-fw"
            : else
            : # Disable MP2 firmware loading
            : PSP_SOFTFUSE_BITS += 29
            : endif
remove, I think

https://review.coreboot.org/c/coreboot/+/76498/comment/28708d61_b9dc1ec4 :
PS4, Line 58: ifeq ($(CONFIG_PSP_S0I3_RESUME_VERSTAGE),y)
            : PSP_SOFTFUSE_BITS += 58
            : endif
remove

https://review.coreboot.org/c/coreboot/+/76498/comment/89a0a7dc_944d0250 :
PS4, Line 130: 	$(OPT_PSP_LOAD_MP2_FW) \
remove