Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37847 )
Change subject: Documentation/soc/amd: Add PSP integration information ......................................................................
Patch Set 3:
(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 ...
Done. Was trying not to use the phrase "this document" again so soon.
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?
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?
Done
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?
Good catch, thank you. The original content was incorrect.
https://review.coreboot.org/c/coreboot/+/37847/2/Documentation/soc/amd/psp_i... PS2, Line 147: * Refer to documentation for definitions.
which docs?
Done
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?
AFAICS
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?
Well the quick answer is it's byte 2, bit 0, 1, 2, etc. I could go either way on clarifying it vs. not. Since the architecture/endianness of the PSP is never mentioned, and it's an integration guide for an x86 product, I think it's normal to infer the correct order. I'm definitely not a good one to sample though.
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
Done