Eric Peers has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37847 )
Change subject: Documentation/soc/amd: Add PSP integration information ......................................................................
Patch Set 2:
(8 comments)
https://review.coreboot.org/c/coreboot/+/37847/2/Documentation/soc/amd/psp_i... File Documentation/soc/amd/psp_integration.md:
https://review.coreboot.org/c/coreboot/+/37847/2/Documentation/soc/amd/psp_i... PS2, Line 5: are beyond the scope, and may be found in AMD NDA publications. beyond the scope ... of this document ...
https://review.coreboot.org/c/coreboot/+/37847/2/Documentation/soc/amd/psp_i... PS2, Line 13: and shares the SPI flash storage that is used by BIOS. where is the physical core of the PSP located? Presumably in the AP package right?
https://review.coreboot.org/c/coreboot/+/37847/2/Documentation/soc/amd/psp_i... PS2, Line 19: * 0xfffa0000 what do each of these addresses point to?
https://review.coreboot.org/c/coreboot/+/37847/2/Documentation/soc/amd/psp_i... PS2, Line 102: | size | 0x02 | 32 | Size of PSP entry in bytes | is this offset right? shouldn't it be 0x04?
https://review.coreboot.org/c/coreboot/+/37847/2/Documentation/soc/amd/psp_i... PS2, Line 147: * Refer to documentation for definitions. which docs?
https://review.coreboot.org/c/coreboot/+/37847/2/Documentation/soc/amd/psp_i... PS2, Line 194: version. The version is located at offset 0x60 from the start of binary. is it a 4 byte version scheme?
https://review.coreboot.org/c/coreboot/+/37847/2/Documentation/soc/amd/psp_i... PS2, Line 265: | Reset Image | 0x02 | 1 | Boolean value to define the| is this byte 2, bit 0, or bit 7?
https://review.coreboot.org/c/coreboot/+/37847/2/Documentation/soc/amd/psp_i... PS2, Line 355: amdfw.rom, may then added directly into the coreboot image. then be added