Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33234 )
Change subject: security/intel/stm STM support ......................................................................
Patch Set 1:
(6 comments)
Please don't use cammelcase. Most of the code looks very UEFI like.
https://review.coreboot.org/#/c/33234/1/src/security/intel/stm/Kconfig File src/security/intel/stm/Kconfig:
https://review.coreboot.org/#/c/33234/1/src/security/intel/stm/Kconfig@7 PS1, Line 7: depends on (PLATFORM_USES_FSP2_0||PLATFORM_USES_FSP1_1||PLATFORM_USES_FSP1_0) why do you need FSP for that?
https://review.coreboot.org/#/c/33234/1/src/security/intel/stm/Kconfig@12 PS1, Line 12: hex "mseg size" if this is user configureable, what are good values? Add help text
https://review.coreboot.org/#/c/33234/1/src/security/intel/stm/StmPlatformRe... File src/security/intel/stm/StmPlatformResource.c:
https://review.coreboot.org/#/c/33234/1/src/security/intel/stm/StmPlatformRe... PS1, Line 180: // Find max bus number and PCIEX length remove commented code
https://review.coreboot.org/#/c/33234/1/src/security/intel/stm/StmPlatformSm... File src/security/intel/stm/StmPlatformSmm.c:
https://review.coreboot.org/#/c/33234/1/src/security/intel/stm/StmPlatformSm... PS1, Line 27: extern bool StmCheckStmImage(void *StmImage, uint32_t StmImageSize); don't use extern
https://review.coreboot.org/#/c/33234/1/src/security/intel/stm/StmPlatformSm... PS1, Line 38: int LoadStmImage(uint32_t mseg) uintptr_t
https://review.coreboot.org/#/c/33234/1/src/security/intel/stm/StmPlatformSm... PS1, Line 57: StmImageSize = cbfs_boot_load_file("stm.bin", MsegBase, StmBufferSize, CBFS_TYPE_RAW); check for errors