Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37847 )
Change subject: Documentation/soc/amd: Add PSP integration information ......................................................................
Documentation/soc/amd: Add PSP integration information
Change-Id: I05187365158eb5c055be0d4a32f41324d2653f71 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M Documentation/soc/amd/family15h.md M Documentation/soc/amd/family17h.md M Documentation/soc/amd/index.md A Documentation/soc/amd/psp_integration.md 4 files changed, 373 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/37847/1
diff --git a/Documentation/soc/amd/family15h.md b/Documentation/soc/amd/family15h.md index fc41e91..5a8f95d 100644 --- a/Documentation/soc/amd/family15h.md +++ b/Documentation/soc/amd/family15h.md @@ -47,3 +47,4 @@ 3. [Models 30h-3Fh BKDG](https://www.amd.com/system/files/TechDocs/49125_15h_Models_30h-3Fh_BKDG.pdf) 4. [Models 60h-6Fh BKDG](https://www.amd.com/system/files/TechDocs/50742_15h_Models_60h-6Fh_BKDG.pdf) 5. [Models 70h-7Fh BKDG](https://www.amd.com/system/files/TechDocs/55072_AMD_Family_15h_Models_70h-7F...) +6. [PSP Integration](psp_integration.md) diff --git a/Documentation/soc/amd/family17h.md b/Documentation/soc/amd/family17h.md index dc3de13..9608b57 100755 --- a/Documentation/soc/amd/family17h.md +++ b/Documentation/soc/amd/family17h.md @@ -18,8 +18,8 @@ (a.k.a. PSP) in system initialization is addressed here. AMD has historically required an NDA for access to the PSP specification<sup>1</sup>. coreboot relies on util/amdfwtool to build -the structures and add various other firmware to the final image. The -Family 17h PSP design guide adds a new BIOS Directory Table, similar to +the structures and add various other firmware to the final image<sup>2</sup>. +The Family 17h PSP design guide adds a new BIOS Directory Table, similar to the PSP Directory Table.
Support in coreboot for modern AMD products is based on AMD’s @@ -29,12 +29,12 @@ tables, and other features.
AGESA for products earlier than Family 17h is known as v5 or -Arch2008<sup>2</sup>. Also note that coreboot currently contains both +Arch2008<sup>3</sup>. Also note that coreboot currently contains both open source AGESA and closed source implementations (binaryPI) compiled from AGESA.
The first AMD Family 17h device ported to coreboot is codenamed -“Picasso”<sup>3</sup>, and will be added to soc/amd/picasso. +“Picasso”<sup>4</sup>, and will be added to soc/amd/picasso.
## Additional Definitions
@@ -207,7 +207,7 @@
Given the UEFI nature of modern AGESA, and the existing open source work from Intel, Picasso shall support AGESA via an FSP-like prebuilt -image. The Intel Firmware Support Package<sup>4</sup> combines +image. The Intel Firmware Support Package<sup>5</sup> combines reference code with EDK II source to create a modular image with discoverable entry points. coreboot source already contains knowledge of FSP, how to parse it, integrate it, and how to communicate with it. @@ -218,7 +218,7 @@ for AMD Family 17h Processors” (PID #55758) and “AMD Platform Security Processor BIOS Architecture Design Guide” (PID #54267) for earlier products -2. [https://www.amd.com/system/files/TechDocs/44065_Arch2008.pdf%5D(https://www....) -3. [https://en.wikichip.org/wiki/amd/cores/picasso%5D(https://en.wikichip.org/wi...) -4. [https://www.intel.com/content/www/us/en/intelligent-systems/intel-firmware-s...) - +2. [PSP Integration](psp_integration.md) +3. [https://www.amd.com/system/files/TechDocs/44065_Arch2008.pdf%5D(https://www....) +4. [https://en.wikichip.org/wiki/amd/cores/picasso%5D(https://en.wikichip.org/wi...) +5. [https://www.intel.com/content/www/us/en/intelligent-systems/intel-firmware-s...) diff --git a/Documentation/soc/amd/index.md b/Documentation/soc/amd/index.md index 80413b0..e4fa6c9 100644 --- a/Documentation/soc/amd/index.md +++ b/Documentation/soc/amd/index.md @@ -6,6 +6,7 @@
- [Family 15h](family15h.md) - [Family 17h](family17h.md) +- [Platform Security Processor Integration](psp_integration.md)
## amd_blobs Repository License
diff --git a/Documentation/soc/amd/psp_integration.md b/Documentation/soc/amd/psp_integration.md new file mode 100755 index 0000000..ebfb074 --- /dev/null +++ b/Documentation/soc/amd/psp_integration.md @@ -0,0 +1,362 @@ +# AMD Platform Security Processor (PSP) Firmware Integration Guide + +This document describes the integration of modules into PSP tables. Further +details of each Platform Security Processor (PSP) firmware or PSP functionality +are beyond the scope, and may be found in AMD NDA publications. + +## Platform Security Processor (PSP) Overview + +The Platform Security Processor (PSP) is an isolated security processor that +runs independently from the main cores of the platform. Security-sensitive +components may run without being affected by the commodity or untrusted +software running as the main system workload. PSP executes its own firmware +and shares the SPI flash storage that is used by BIOS. + +## Embedded Firmware Structure + +PSP identifies its important tables by first locating the Embedded Firmware +Structure. The following physical addresses are read to find the structure: +* 0xfffa0000 +* 0xfff20000 +* 0xffe20000 +* 0xffc20000 +* 0xff820000 +* 0xff020000 + +Most coreboot implementations provide flexibility to position the structure in +any of the eligible locations. Below are typical definitions within the +structure (for all families combined). Individual features supported vary by +family and model. + + +--------------+---------------+------------------+----------------------------+ + | Field Name | Offset (Hex) | Size (In Bytes) | Description/Purpose | + +--------------+---------------+------------------+----------------------------+ + | Signature | 0x00 | 4 | 0x55aa55aa | + |--------------|---------------|------------------|----------------------------| + | IMC FW | 0x04 | 4 | Integrated Micro | + | | | | Controller: unsupported | + | | | | but functional in some | + | | | | systems | + |--------------|---------------|------------------|----------------------------| + | GbE FW | 0x08 | 4 | Gigabit Ethernet | + |--------------|---------------|------------------|----------------------------| + | xHCI FW | 0x0c | 4 | xHCI firmware | + |--------------|---------------|------------------|----------------------------| + | PSP Dir Tbl | 0x10 | 4 | Pointer to PSP Directory | + | | | | Table (early devices) | + |--------------|---------------|------------------|----------------------------| + | PSP Dir Tbl | 0x14 | 4 | Pointer to PSP Directory | + | | | | Table (later devices and | + | | | | is combo capable) | + |--------------|---------------|------------------|----------------------------| + | BIOS Dir Tbl | 0x18 | 4 | Pointer to BIOS Directory | + | | | | Table for models n | + |--------------|---------------|------------------|----------------------------| + | BIOS Dir Tbl | 0x1c | 4 | Pointer to BIOS Directory | + | | | | Table for models nn | + |--------------|---------------|------------------|----------------------------| + | BIOS Dir Tbl | 0x1c | 4 | Pointer to BIOS Directory | + | | | | Table for models nnn | + |--------------|---------------|------------------|----------------------------| + | … | | | ... | + +--------------+---------------+------------------+----------------------------+ + +## PSP Directory Table + +The PSP Directory Table allows PSP to find and load various images. A second +level table may be generated to allow for updates without the risk of +corrupting the primary table. Certain models support a combo type table, +allowing secondary tables to be referenced by device ID. No coreboot +implementations currently use combo tables. + +### PSP Directory Table Header + + +--------------+---------------+------------------+----------------------------+ + | Field Name | Offset (Hex) | Size (In Bytes) | Description/Purpose | + +--------------+---------------+------------------+----------------------------+ + | PSP Cookie | 0x00 | 4 | PSP cookie "$PSP" to | + | | | | recognize the header. | + | | | | Cookie “$PL2” for level 2 | + |--------------|---------------|------------------|----------------------------| + | Checksum | 0x04 | 4 | 32-bit CRC value of header | + | | | | below this field and | + | | | | including all entries | + |--------------|---------------|------------------|----------------------------| + | Total Entries| 0x08 | 4 | Number of PSP Directory | + | | | | entries in the table | + |--------------|---------------|------------------|----------------------------| + | Reserved | 0x0C | 4 | Reserved - Set to zero | + +--------------+---------------+------------------+----------------------------+ + +### PSP Directory Table Entries + + +--------------+---------------+------------------+----------------------------+ + | Field Name | Offset (Hex) | Size (In Bits) | Description/Purpose | + +--------------+---------------+------------------+----------------------------+ + | Type | 0x00 | 8 | Entry type (see below) | + |--------------|---------------|------------------|----------------------------| + | Sub Program | 0x01 | 8 | Specifies sub program | + |--------------|---------------|------------------|----------------------------| + | Reserved | 0x02 | 16 | Reserved - set to 0 | + |--------------|---------------|------------------|----------------------------| + | size | 0x02 | 32 | Size of PSP entry in bytes | + |--------------|---------------|------------------|----------------------------| + | Location / | 0x08 | 64 | Location: Physical Address | + | Value | | | of SPIROM location where | + | | | | corresponding PSP entry | + | | | | located. | + | | | | | + | | | | Value: 64-bit value for the| + | | | | PSP Entry | + +--------------+---------------+------------------+----------------------------+ + +### PSP Directory Table Types + +**0x00**: AMD public key +* Public key used by on-chip bootcode to verify the signature of PSP boot + loader firmware, read from SPIROM. + +**0x01**: PSP boot loader firmware +* Second stage boot loader firmware to be loaded by on-chip bootcode. + +**0x02**: PSP SecureOS firmware +* Off-chip PSP boot loader will be overwritten in SRAM by the Secure/Trusted + OS during initial boot up. +* PSP SecureOS performs: + * Initialization of OS internal structures and instantiates the fTPM as a + trusted application + * Sets up CPU/BIOS-PSP interface registers + * Enters steady state idling and waiting for commands + * In steady state, on notification, prepares for S3 state + * Verify and loading GFX Firmware + +**0x03**: PSP recovery boot loader firmware +* Recovery PSP boot loader image, loaded by on-chip bootcode in case of + failure in loading PSP boot loader. + +**0x08**: SMU off-chip firmware + +**0x12**: SMU off-chip firmware section 2 +* Power Management firmware, responsible for system power/clock management. + +**0x09**: Secure Debug unlock public key +* Public key token used during Secure Debug unlock process to verify message + payload from AMD server. + +**0x0b**: Soft fuse chain +* Refer to documentation for definitions. + +**0x0c**: PSP trustlet binaries +* Optional file to enable fTPM. + +**0x13**: PSP Secure Debug unlock debug image +* Secure Debug unlock firmware image, used to unlock the device. + +**0x21**: Wrapped iKEK +* Intermediate Key Encryption Key, used to decrypt encrypted firmware images. + This is mandatory in order to support encrypted firmware. + +**0x24**: Security policy binary +* A security policy is applied to restrict the untrusted access to security + sensitive regions. + +**0x25**: MP2 firmware +* The MP2 of the SMU, also known as the Sensor Fusion Integration is used to + aggregate the data from various sensors such as accelerometer, gyrometer, + ambient light sensor, orientation sensor, etc. This is off-chip firmware + for Sensor Fusion Processor (SFP) subsystem of the SMU. + +**0x28**: System driver +* Driver executing on top of SecureOS. + +**0x30 - 0x37**: PSP AGESA binaries +* AGESA Boot Loaders (ABLs) are a set of binary images executed by the PSP. + They are responsible for initializing APU silicon components (including but + not limited to APU memory interface) on S5, S4 and S3, prior to the release + of the main cores. + +**0x3a**: Whitelist +* Optional image containing a signed whitelist of serial number(s). + +**0x40**: Pointer to secondary table +* Pointer to PSP Directory Table level 2. + +**0x52**: PSP boot loader usermode OEM application +* Supported only in certain SKUs. + +**0x22**: PSP Token Unlock data +* Used to support time-bound Secure Debug unlock during boot. This entry may + be omitted if the Token Unlock debug feature is not required. + +### Firmware Version of Binaries + +Every firmware binary contains 256 Bytes of PSP Header, which includes firmware +version. The version is located at offset 0x60 from the start of binary. + +For example, in the PSP BootLoader: + + 0000000: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + 0000010: 2450 5331 c0e1 0000 0100 0000 0000 0000 $PS1............ + 0000020: 5c0a ddb8 b279 4846 e154 aa4c ed7d 414d ....yHF.T.L.}AM + 0000030: 0100 0000 0000 0000 60bb a67e 1a43 4c6b ........`..~.CLk + 0000040: 9807 bc8d fdb4 1f40 0000 0000 0000 0000 .......@........ + 0000050: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + 0000060: 7401 0800 ffff ffff 0001 0000 c0e3 0000 t............... + 0000070: 0000 0000 0000 0000 0000 0000 0100 0000 ................ + 0000080: 4766 9186 9d5f e909 492d 491d d9ee 8e6c Gf..._..I-I....l + 0000090: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + 00000a0: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + 00000b0: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + 00000c0: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + 00000d0: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + 00000e0: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + 00000f0: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + +The PSP BootLoader version is 00.08.01.74. + +Note that only Firmware binary images have versions. Key tokens are not +versioned, as there will not be multiple keys. Keys are unique to processor +family. + +### BIOS Directory Table Entry Types + +All x86 accessible components (both executable and data blobs) are found via +the BIOS Directory Table. A second level table may be generated to allow for +updates without the risk of corrupting the primary table. + +The BIOS Directory table structure is slightly different from PSP Directory: +* Multiple instances of firmware components are allowed for one specific type +* The type field is further structured to reflect attributes of BIOS + components such as "Region Type", "Reset Image", "Copy Image", "Read Only", + allowing design flexibility +* The "Destination Address" field is added for specific entries that are + expected to be copied from boot media to specific memory location + +### BIOS Directory Table Header + + +--------------+---------------+------------------+----------------------------+ + | Field Name | Offset (Hex) | Size (In Bytes) | Description/Purpose | + +--------------+---------------+------------------+----------------------------+ + | BIOS Cookie | 0x00 | 4 | BIOS cookie "$BHD" to | + | | | | recognize the header. | + | | | | Cookie “$BL2” for level 2 | + |--------------|---------------|------------------|----------------------------| + | Checksum | 0x04 | 4 | 32 bit CRC value of header | + | | | | below this field and | + | | | | including all entries | + |--------------|---------------|------------------|----------------------------| + | Total Entries| 0x08 | 4 | Number of BIOS Directory | + | | | | entries in the table | + |--------------|---------------|------------------|----------------------------| + | Reserved | 0x0C | 4 | Reserved - Set to zero | + +--------------+---------------+------------------+----------------------------+ + +### BIOS Directory Table Entries + + +--------------+---------------+------------------+----------------------------+ + |Field name Offset (Hex) Size (In Bits) Description/Purpose | + +--------------+---------------+------------------+----------------------------+ + | Type | 0x00 | 8 | Entry type (see below) | + |--------------|---------------|------------------|----------------------------| + | Region Type | 0x01 | 8 | Setup the memory region's | + | | | | security attribute for the | + | | | | BIOS entry | + |--------------|---------------|------------------|----------------------------| + | Reset Image | 0x02 | 1 | Boolean value to define the| + | | | | BIOS entry is a reset | + | | | | binary image | + |--------------|---------------|------------------|----------------------------| + | Copy Image | 0x02 | 1 | Define the binary image of | + | | | | the BIOS entry is for | + | | | | copying over to the memory | + | | | | region | + |--------------|---------------|------------------|----------------------------| + | Read Only | 0x02 | 1 | Setup the memory region for| + | | | | the BIOS entry to read only| + |--------------|---------------|------------------|----------------------------| + | Compressed | 0x02 | 1 | Compressed using zlib | + | | | | | + |--------------|---------------|------------------|----------------------------| + | Instance | 0x02 | 4 | Specify the Instance of an | + | | | | entry | + |--------------|---------------|------------------|----------------------------| + | Sub Program | 0x03 | 3 | Specify the SubProgram, | + | | | | reference Section | + |--------------|---------------|------------------|----------------------------| + | Reserved | 0x03 | 5 | Reserved - Set to zero | + |--------------|---------------|------------------|----------------------------| + | Size | 0x04 | 32 | Memory Region Size | + |--------------|---------------|------------------|----------------------------| + | Source | 0x08 | 64 | Physical Address of SPIROM | + | Address | | | location where the data for| + | | | | the corresponding entry is | + | | | | located | + |--------------|---------------|------------------|----------------------------| + | Destination | 0x10 | 64 | Destination Address of | + | Address | | | memory location where the | + | | | | data for the corresponding | + | | | | BIOS Entry is copied | + +--------------+---------------+------------------+----------------------------+ + +### BIOS Directory Table Entry Types + +**0x60**: APCB data +* Source field points to the AGESA PSP Customization Block (APCB) data. + +**0x68**: Backup copy of APCB data +* Source field points to the backup copy of AGESA PSP Customization Block + (APCB) data. + +**0x61**: APOB data +* Location field points to the AGESA PSP Output Block (APOB) data. + +**0x62**: BIOS reset image +* Source field points to BIOS binary image in flash. Destination points to + DRAM. + +**0x63**: APOB data NV +* Source field points to AGESA PSP Output Block (APOB) data NV copy. This + data is written by coreboot and consumed by PSP ABLs during S3 resume. + +**0x64**: PMU firmware (instruction) +* Source field points to the instruction portion of Phy Microcontroller Unit + firmware. + +**0x65**: PMU firmware (data) +* Source field points to the data portion of Phy Microcontroller Unit + firmware. + +**0x66**: x86 microcode patch +* Source field points to the microcode patch. + +**0x6a**: MP2 FW config file +* Source field points to the MP2 FW configuration file. + +**0x70**: Pointer to secondary table +* Pointer to BIOS Directory Table level 2. + +## Tools + +### amdcompress + +cbfstool/amdcompress is a helper for creating the BIOS Reset Image (BIOS +Directory Table type 0x62). This is the code PSP uncompresses into DRAM at the +location where it will release the x86 for execution. Typical usage is for +amdcompress to convert an elf file’s program section into a zlib compressed +image. + +### amdfwtool + +All images requiring PSP functionality rely on the amdfwtool utility. +amdfwtool takes image names as command-line arguments, as well as the size of +the flash device, and intended location of the Embedded Firmware Structure. +Its output is a monolithic image with correctly positioned headers, pointers, +structures, and the firmware images passed in. The file, typically named +amdfw.rom, may then added directly into the coreboot image. + +## External Reference + +* NDA document #55758: "AMD Platform Security Processor BIOS Architecture + Design Guide for AMD Family 17h Processors" +* NDA document #54267 “AMD Platform Security Processor BIOS Architecture + Design Guide: For all devices earlier than Family 17h \ No newline at end of file
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37847 )
Change subject: Documentation/soc/amd: Add PSP integration information ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37847/1/Documentation/soc/amd/psp_i... File Documentation/soc/amd/psp_integration.md:
https://review.coreboot.org/c/coreboot/+/37847/1/Documentation/soc/amd/psp_i... PS1, Line 257: |Field name Offset (Hex) Size (In Bits) Description/Purpose | This seems off
Hello Eric Peers, Kyösti Mälkki, Patrick Rudolph, build bot (Jenkins), Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37847
to look at the new patch set (#2).
Change subject: Documentation/soc/amd: Add PSP integration information ......................................................................
Documentation/soc/amd: Add PSP integration information
Change-Id: I05187365158eb5c055be0d4a32f41324d2653f71 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M Documentation/soc/amd/family15h.md M Documentation/soc/amd/family17h.md M Documentation/soc/amd/index.md A Documentation/soc/amd/psp_integration.md 4 files changed, 373 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/37847/2
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 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37847/1/Documentation/soc/amd/psp_i... File Documentation/soc/amd/psp_integration.md:
https://review.coreboot.org/c/coreboot/+/37847/1/Documentation/soc/amd/psp_i... PS1, Line 257: |Field name Offset (Hex) Size (In Bits) Description/Purpose |
This seems off
Ew
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
Hello Eric Peers, Kyösti Mälkki, Patrick Rudolph, build bot (Jenkins), Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37847
to look at the new patch set (#3).
Change subject: Documentation/soc/amd: Add PSP integration information ......................................................................
Documentation/soc/amd: Add PSP integration information
Change-Id: I05187365158eb5c055be0d4a32f41324d2653f71 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M Documentation/soc/amd/family15h.md M Documentation/soc/amd/family17h.md M Documentation/soc/amd/index.md A Documentation/soc/amd/psp_integration.md 4 files changed, 376 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/37847/3
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
Felix Held 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:
(18 comments)
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... File Documentation/soc/amd/psp_integration.md:
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 3: integration of modules into PSP tables "structure of the PSP tables pointing to the integrated modules" maybe?
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 12: add "on the PSP"?
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 13: . "on the x86 cores." maybe?
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 19: , in order "in the following order" at the end of the sentence?
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 21: * 0xfffa0000 : * 0xfff20000 : * 0xffe20000 : * 0xffc20000 : * 0xff820000 : * 0xff020000 those are locations when the spi flash is mapped in the x86's memory space and not the locations in the flash, right? would be useful to add that information
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 28: Most coreboot implementations isn't that common for coreboot and not only "most", since cbfstool can put image parts at defined locations?
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 55: n what does the n/nn/nnn mean here?
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 68: add a "the"?
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 69: for drop "for"?
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 119: , drop ","
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 178: . "from reset." maybe?
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 181: * Optional image containing a signed whitelist of serial number(s). serial numbers?
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 268: add a ", bit 0" here to make things clearer? same for the entries below
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 286: SubProgram one word or space in the middle?
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 322: ABLs ABL?
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 342: i wonder how amdcompress, cbfstool and amdfwtool work together; would be good if you also describe/document that; could also be done in a follow-up patch
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 347: it will release the x86 for execution "the x86 starts execution when released from reset" maybe?
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 357: passed in "added" maybe? "passed in" sounds a bit weird to me; i'm not a native english speaker though
Felix Held 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:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... File Documentation/soc/amd/psp_integration.md:
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 322: ABLs
ABL?
ah, it's defined above
Hello Eric Peers, Kyösti Mälkki, Patrick Rudolph, build bot (Jenkins), Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37847
to look at the new patch set (#4).
Change subject: Documentation/soc/amd: Add PSP integration information ......................................................................
Documentation/soc/amd: Add PSP integration information
Change-Id: I05187365158eb5c055be0d4a32f41324d2653f71 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M Documentation/soc/amd/family15h.md M Documentation/soc/amd/family17h.md M Documentation/soc/amd/index.md A Documentation/soc/amd/psp_integration.md 4 files changed, 382 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/37847/4
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:
(18 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 265: | Reset Image | 0x02 | 1 | Boolean value to define the|
Well the quick answer is it's byte 2, bit 0, 1, 2, etc. I could go either way on clarifying it vs. […]
Ack
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... File Documentation/soc/amd/psp_integration.md:
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 3: integration of modules into PSP tables
"structure of the PSP tables pointing to the integrated modules" maybe?
Done
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 12:
add "on the PSP"?
Done
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 13: .
"on the x86 cores. […]
Done
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 19: , in order
"in the following order" at the end of the sentence?
Done
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 21: * 0xfffa0000 : * 0xfff20000 : * 0xffe20000 : * 0xffc20000 : * 0xff820000 : * 0xff020000
those are locations when the spi flash is mapped in the x86's memory space and not the locations in […]
Done
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 28: Most coreboot implementations
isn't that common for coreboot and not only "most", since cbfstool can put image parts at defined lo […]
It's more about how the amdfw.rom artifact is constructed and not about positioning with cbfs. It's built specifically for sitting at one of the 6 locations. With stoneyridge and picasso, we have an index chooser in Kconfig. Looks like that's not in southbridge/amd.
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 55: n
what does the n/nn/nnn mean here?
See if the change works any better for you.
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 68:
add a "the"?
Lots of people seem to use "PSP" almost like a proper name. I agree with you, treating like a noun feels more natural.
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 69: for
drop "for"?
Done
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 119: ,
drop ","
Done
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 178: .
"from reset. […]
Done
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 181: * Optional image containing a signed whitelist of serial number(s).
serial numbers?
Done
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 268:
add a ", bit 0" here to make things clearer? same for the entries below
Done
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 286: SubProgram
one word or space in the middle?
Done
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 342:
i wonder how amdcompress, cbfstool and amdfwtool work together; would be good if you also describe/d […]
Ack. Kyosti would like for it to change, so I'm not sure how much effort to put into their descriptions now.
amdcompress began as a brand new standalone utility that would simply compress a binary and tack on the header that the PSP expects. Then I added the ability to take an elf file as input and compress only the program portion. So it's only a compressed piece of code with a header on the front. Along the way, there was a suggestion to add the all that to cbfstool vs. a standalone program; very likely because it was borrowing a lot of the elf definitions from cbfsutil.
amdfwutil is the main utility that takes 100% of the PSP blobs (plus the code mentioned above) and creates one single image. It pregenerates all the tables with the correct pointers, the firmwares for the pointers to point to, etc. That artifact is typically called amdfw.rom, then it's simply added into the coreboot image. Once it's added, all the pointers within the tables become correct. (Oh well I was a bit more clear below).
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 347: it will release the x86 for execution
"the x86 starts execution when released from reset" maybe?
Done
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 357: passed in
"added" maybe? "passed in" sounds a bit weird to me; i'm not a native english speaker though
Done. Well that's fine, of course.
You wouldn't use a phrase like "passed on the command line" or "passing arguments"?
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37847 )
Change subject: Documentation/soc/amd: Add PSP integration information ......................................................................
Patch Set 4: Code-Review+1
(9 comments)
Some paragraphs could be reflowed.
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... File Documentation/soc/amd/psp_integration.md:
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 15: system BIOS. Reflow on line above?
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 201: Bytes bytes
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 201: Every firmware binary contains 256 Bytes of a PSP Header, which includes : firmware version *the* firmware version?
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 239: "Region Type" Mark it up with ** so no quotes are needed?
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 338: **0x66**: x86 microcode patch Use microcode update patch?
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 351: cbfstool/amdcompress Mark up with ``?
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 354: elf ELF
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 364: amdfw.rom Mark up with ``?
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 368: "AMD Platform Security Processor BIOS Architecture : Design Guide for AMD Family 17h Processors" Mark up with ** instead of quotes.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37847 )
Change subject: Documentation/soc/amd: Add PSP integration information ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... File Documentation/soc/amd/psp_integration.md:
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 342:
Ack. […]
CB:37490 My suggestion was to move amdcompress outside util/cbfstool and into util/amdfwtool.
AFAICS the only thing cbfstool and amdcompress share is need of ELF headers, which itself is an external header.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37847 )
Change subject: Documentation/soc/amd: Add PSP integration information ......................................................................
Patch Set 4:
(6 comments)
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... File Documentation/soc/amd/psp_integration.md:
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 55: n
See if the change works any better for you.
yes, that makes things clearer
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 181: * Optional image containing a signed whitelist of serial number(s).
Done
my question was more from what components the serial numbers are. are these cpu/chipset serial numbers that are fused into the chips?
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 268:
Done
this makes it much clearer to me
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 342:
Ack. […]
so cbfstool is used twice? first for the x86 coreboot part that gets compressed and a header added by cbfstool/amdcompress; amdfwtool takes the resulting image and the psp blobs and makes an image from that that gets fed back into cbfstool? or does amdfwtool create the image that gets written to the flash? Or does cbfstool/amdcompress compress the stages and then cbfstool adds the output of amdfwtool and hopes that there won't be mismatched offsets/pointers?
haven't looked at the source, but i'd expect this to work like this: the coreboot stages that run on the x86 get built, the stages and the amd x86 fsp blobs get added to a cbfs, the whole cbfs gets compressed and a header added. the resulting image gets consumed by amdfwtool that adds the psp blobs, creates the tables described above and generates a firmware image that gets written to the flash. from what you wrote it works somehow differently, but i'm not completely sure how and why.
if this will likely change in the future, i'm ok with adding that information later.
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 357: passed in
Done. Well that's fine, of course. […]
i'd use those phrases for the arguments/files passed to the tool; here the context is the resulting binary and not the arguments to the tool
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... File Documentation/soc/amd/psp_integration.md:
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 292: please also add the bits for this and the next line
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37847 )
Change subject: Documentation/soc/amd: Add PSP integration information ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... File Documentation/soc/amd/psp_integration.md:
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 342:
so cbfstool is used twice? first for the x86 coreboot part that gets compressed and a header added b […]
or does only the boot block get processed by amdcompress into a BIOS reset image and the other coreboot stages still reside in the flash like on other platforms? i think that is the main question i still have on the boot process on the 17h platforms with coreboot
Hello Kyösti Mälkki, Eric Peers, Patrick Rudolph, Paul Menzel, build bot (Jenkins), Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37847
to look at the new patch set (#5).
Change subject: Documentation/soc/amd: Add PSP integration information ......................................................................
Documentation/soc/amd: Add PSP integration information
Change-Id: I05187365158eb5c055be0d4a32f41324d2653f71 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M Documentation/soc/amd/family15h.md M Documentation/soc/amd/family17h.md M Documentation/soc/amd/index.md A Documentation/soc/amd/psp_integration.md 4 files changed, 387 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/37847/5
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 5:
(11 comments)
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... File Documentation/soc/amd/psp_integration.md:
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 342:
My suggestion was to move amdcompress outside util/cbfstool and into util/amdfwtool.
If I skewed your suggestion, that wasn't my intention. I was only trying to indicate that I didn't want to document the current state of the tools any more meticulously, since they're likely to change.
Felix, it looks like I misstated cbfstool's usage above but I think the text below is still accurate. amdcompress is a standalone utility whose source is in the cbfstool directory and shares some files. With that context, cbfstool is used mostly where it always has been.
or does only the boot block get processed by amdcompress into a BIOS reset image and the other coreboot stages still reside in the flash like on other platforms?
It sounds like you've got it. Here's the newly updated bootflow: * Forget any previous conversation or PSs you may have seen about a hybrid romstage, etc. and assume the "normal" bootblock, romstage, ramstage flow. No postcar, and any x86-based verstage implementation is TBD. * The PSP uncompresses the bootblock into memory for us, modifies the CS shadow register to place the reset vector in RAM, then releases reset. Because the PSP does the initial work, and the reset vector isn't 0xfffffff0, an executable bootblock is not required at the top of flash. * bootblock finds/loads/runs romstage (as always), except it's no longer XIP. romstage is loaded to and run from DRAM. Same with FSP. Same with ramstage of course.
All stages and FSP after bootblock are loaded by the x86 -- there has been discussion of trying to combine them into one large compressed image for the PSP, but that's not the case currently (and nobody is working on it). bootblock is currently the only image the PSP deals with. So: * bootblock.elf is fed to amdcompress and the output is amd_biospsp.img. This is the compressed program portion of bootblock.elf. bootblock is linked to match the runtime addressing, and this keys off of CONFIG_X86_RESET_VECTOR. * amd_biospsp.img is then fed, along with all the various PSP blobs to amdfwtool and the output is amdfw.rom. * cbfstool assembles files and stages like normal. ** amdfw.rom is typically added as a cbfs file, however this is not a requirement. We can also shrink cbfs smaller than the flash device and then add amdfw.rom to coreboot.rom manually.
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... File Documentation/soc/amd/psp_integration.md:
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 15: system BIOS.
Reflow on line above?
The rule described here (https://opusdesign.us/wordcount/typographic-widows-orphans/) is how I learned to write, so I don't want to have a single word on the final line. Arguably the entire paragraph could be massaged to look better, but since md rendering doesn't care about line breaks, I'm not sure it's worthwhile.
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 201: Bytes
bytes
Done
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 201: Every firmware binary contains 256 Bytes of a PSP Header, which includes : firmware version
*the* firmware version?
Done
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 239: "Region Type"
Mark it up with ** so no quotes are needed?
These currently match the AMD documentation.
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 292:
please also add the bits for this and the next line
Thanks, missed those.
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 338: **0x66**: x86 microcode patch
Use microcode update patch?
AMD seems to only call it "microcode patch".
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 351: cbfstool/amdcompress
Mark up with ``?
Done
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 354: elf
ELF
Done
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 364: amdfw.rom
Mark up with ``?
Done
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 368: "AMD Platform Security Processor BIOS Architecture : Design Guide for AMD Family 17h Processors"
Mark up with ** instead of quotes.
Done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37847 )
Change subject: Documentation/soc/amd: Add PSP integration information ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... File Documentation/soc/amd/psp_integration.md:
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 15: system BIOS.
The rule described here (https://opusdesign. […]
Ack
https://review.coreboot.org/c/coreboot/+/37847/4/Documentation/soc/amd/psp_i... PS4, Line 338: **0x66**: x86 microcode patch
AMD seems to only call it "microcode patch".
The TLA *MCU* is used quite a lot, but ok.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37847 )
Change subject: Documentation/soc/amd: Add PSP integration information ......................................................................
Patch Set 5: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... File Documentation/soc/amd/psp_integration.md:
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 342:
My suggestion was to move amdcompress outside util/cbfstool and into util/amdfwtool. […]
it would be good to add this information in a follow-up patch to the documentation, since this was unclear to me before your comment on how things work here
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37847 )
Change subject: Documentation/soc/amd: Add PSP integration information ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... File Documentation/soc/amd/psp_integration.md:
https://review.coreboot.org/c/coreboot/+/37847/3/Documentation/soc/amd/psp_i... PS3, Line 342:
it would be good to add this information in a follow-up patch to the documentation, since this was u […]
ah, it is already sort-of documented in the follow-up patch. I'm ok with improving the documentation after the changes to the details have landed, so that the documentation doesn't have to be reworked too often
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37847 )
Change subject: Documentation/soc/amd: Add PSP integration information ......................................................................
Documentation/soc/amd: Add PSP integration information
Change-Id: I05187365158eb5c055be0d4a32f41324d2653f71 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37847 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Felix Held felix-coreboot@felixheld.de --- M Documentation/soc/amd/family15h.md M Documentation/soc/amd/family17h.md M Documentation/soc/amd/index.md A Documentation/soc/amd/psp_integration.md 4 files changed, 387 insertions(+), 9 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved
diff --git a/Documentation/soc/amd/family15h.md b/Documentation/soc/amd/family15h.md index fc41e91..5a8f95d 100644 --- a/Documentation/soc/amd/family15h.md +++ b/Documentation/soc/amd/family15h.md @@ -47,3 +47,4 @@ 3. [Models 30h-3Fh BKDG](https://www.amd.com/system/files/TechDocs/49125_15h_Models_30h-3Fh_BKDG.pdf) 4. [Models 60h-6Fh BKDG](https://www.amd.com/system/files/TechDocs/50742_15h_Models_60h-6Fh_BKDG.pdf) 5. [Models 70h-7Fh BKDG](https://www.amd.com/system/files/TechDocs/55072_AMD_Family_15h_Models_70h-7F...) +6. [PSP Integration](psp_integration.md) diff --git a/Documentation/soc/amd/family17h.md b/Documentation/soc/amd/family17h.md index dc3de13..9608b57 100755 --- a/Documentation/soc/amd/family17h.md +++ b/Documentation/soc/amd/family17h.md @@ -18,8 +18,8 @@ (a.k.a. PSP) in system initialization is addressed here. AMD has historically required an NDA for access to the PSP specification<sup>1</sup>. coreboot relies on util/amdfwtool to build -the structures and add various other firmware to the final image. The -Family 17h PSP design guide adds a new BIOS Directory Table, similar to +the structures and add various other firmware to the final image<sup>2</sup>. +The Family 17h PSP design guide adds a new BIOS Directory Table, similar to the PSP Directory Table.
Support in coreboot for modern AMD products is based on AMD’s @@ -29,12 +29,12 @@ tables, and other features.
AGESA for products earlier than Family 17h is known as v5 or -Arch2008<sup>2</sup>. Also note that coreboot currently contains both +Arch2008<sup>3</sup>. Also note that coreboot currently contains both open source AGESA and closed source implementations (binaryPI) compiled from AGESA.
The first AMD Family 17h device ported to coreboot is codenamed -“Picasso”<sup>3</sup>, and will be added to soc/amd/picasso. +“Picasso”<sup>4</sup>, and will be added to soc/amd/picasso.
## Additional Definitions
@@ -207,7 +207,7 @@
Given the UEFI nature of modern AGESA, and the existing open source work from Intel, Picasso shall support AGESA via an FSP-like prebuilt -image. The Intel Firmware Support Package<sup>4</sup> combines +image. The Intel Firmware Support Package<sup>5</sup> combines reference code with EDK II source to create a modular image with discoverable entry points. coreboot source already contains knowledge of FSP, how to parse it, integrate it, and how to communicate with it. @@ -218,7 +218,7 @@ for AMD Family 17h Processors” (PID #55758) and “AMD Platform Security Processor BIOS Architecture Design Guide” (PID #54267) for earlier products -2. [https://www.amd.com/system/files/TechDocs/44065_Arch2008.pdf%5D(https://www....) -3. [https://en.wikichip.org/wiki/amd/cores/picasso%5D(https://en.wikichip.org/wi...) -4. [https://www.intel.com/content/www/us/en/intelligent-systems/intel-firmware-s...) - +2. [PSP Integration](psp_integration.md) +3. [https://www.amd.com/system/files/TechDocs/44065_Arch2008.pdf%5D(https://www....) +4. [https://en.wikichip.org/wiki/amd/cores/picasso%5D(https://en.wikichip.org/wi...) +5. [https://www.intel.com/content/www/us/en/intelligent-systems/intel-firmware-s...) diff --git a/Documentation/soc/amd/index.md b/Documentation/soc/amd/index.md index 80413b0..e4fa6c9 100644 --- a/Documentation/soc/amd/index.md +++ b/Documentation/soc/amd/index.md @@ -6,6 +6,7 @@
- [Family 15h](family15h.md) - [Family 17h](family17h.md) +- [Platform Security Processor Integration](psp_integration.md)
## amd_blobs Repository License
diff --git a/Documentation/soc/amd/psp_integration.md b/Documentation/soc/amd/psp_integration.md new file mode 100755 index 0000000..5f53a39 --- /dev/null +++ b/Documentation/soc/amd/psp_integration.md @@ -0,0 +1,376 @@ +# AMD Platform Security Processor (PSP) Firmware Integration Guide + +The following content defines the structures of PSP tables and describes the +firmware images integrated into a functioning system. Further details of +each Platform Security Processor (PSP) firmware blob or PSP feature are +beyond the scope of this document, and may be found in AMD NDA publications. + +The current name for the security technology is "AMD Secure Processor". +To be consistent with the latest documentation, and because of familiarity +with the older name, this document continues with "Platform Security Processor" +and "PSP". + +## Platform Security Processor (PSP) Overview + +The Platform Security Processor (PSP) is an on-die, isolated security processor +that runs independently from the main x86 cores of the platform. +Security-sensitive components run on the PSP without being affected by the +commodity or untrusted software running on the x86 cores. The PSP executes +its own firmware and shares the SPI flash storage that is used by the +system BIOS. + +## Embedded Firmware Structure + +The PSP identifies its important tables by first locating the Embedded Firmware +Structure. It reads specific addresses in the SPI flash, from top to bottom, +attempting to identify the signature. The locations (for clarity, the x86 +physical addresses) checked are: +* 0xfffa0000 +* 0xfff20000 +* 0xffe20000 +* 0xffc20000 +* 0xff820000 +* 0xff020000 + +Most coreboot implementations provide flexibility to position the structure in +any of the eligible locations. Below are typical definitions within the +structure (for all families combined). Individual features supported vary by +family and model. + + +--------------+---------------+------------------+----------------------------+ + | Field Name | Offset (Hex) | Size (In Bytes) | Description/Purpose | + +--------------+---------------+------------------+----------------------------+ + | Signature | 0x00 | 4 | 0x55aa55aa | + |--------------|---------------|------------------|----------------------------| + | IMC FW | 0x04 | 4 | Integrated Micro | + | | | | Controller: unsupported | + | | | | but functional in some | + | | | | systems | + |--------------|---------------|------------------|----------------------------| + | GbE FW | 0x08 | 4 | Gigabit Ethernet | + |--------------|---------------|------------------|----------------------------| + | xHCI FW | 0x0c | 4 | xHCI firmware | + |--------------|---------------|------------------|----------------------------| + | PSP Dir Tbl | 0x10 | 4 | Pointer to PSP Directory | + | | | | Table (early devices) | + |--------------|---------------|------------------|----------------------------| + | PSP Dir Tbl | 0x14 | 4 | Pointer to PSP Directory | + | | | | Table (later devices and | + | | | | is combo capable) | + |--------------|---------------|------------------|----------------------------| + | BIOS Dir Tbl | 0x18 | 4 | Pointer to BIOS Directory | + | | | | Table for models n* | + |--------------|---------------|------------------|----------------------------| + | BIOS Dir Tbl | 0x1c | 4 | Pointer to BIOS Directory | + | | | | Table for models nn | + |--------------|---------------|------------------|----------------------------| + | BIOS Dir Tbl | 0x20 | 4 | Pointer to BIOS Directory | + | | | | Table for models nnn | + |--------------|---------------|------------------|----------------------------| + | … | | | ... | + +--------------+---------------+------------------+----------------------------+ + +* The Embedded Firmware Structure may support pointers to multiple generations + of devices, e.g. Family 17h Models 00h-0Fh, Family 17h Models 10h-1Fh, etc. + Details are specific to the implementation. + +## PSP Directory Table + +The PSP Directory Table allows the PSP to find and load various images. A +second level table may be generated to allow updates without the risk of +corrupting the primary table. Certain models support a combo type table, +allowing secondary tables to be referenced by device ID. No coreboot +implementations currently use combo tables. + +### PSP Directory Table Header + + +--------------+---------------+------------------+----------------------------+ + | Field Name | Offset (Hex) | Size (In Bytes) | Description/Purpose | + +--------------+---------------+------------------+----------------------------+ + | PSP Cookie | 0x00 | 4 | PSP cookie "$PSP" to | + | | | | recognize the header. | + | | | | Cookie “$PL2” for level 2 | + |--------------|---------------|------------------|----------------------------| + | Checksum | 0x04 | 4 | 32-bit CRC value of header | + | | | | below this field and | + | | | | including all entries | + |--------------|---------------|------------------|----------------------------| + | Total Entries| 0x08 | 4 | Number of PSP Directory | + | | | | entries in the table | + |--------------|---------------|------------------|----------------------------| + | Reserved | 0x0C | 4 | Reserved - Set to zero | + +--------------+---------------+------------------+----------------------------+ + +### PSP Directory Table Entries + + +--------------+---------------+------------------+----------------------------+ + | Field Name | Offset (Hex) | Size (In Bits) | Description/Purpose | + +--------------+---------------+------------------+----------------------------+ + | Type | 0x00 | 8 | Entry type (see below) | + |--------------|---------------|------------------|----------------------------| + | Sub Program | 0x01 | 8 | Specifies sub program | + |--------------|---------------|------------------|----------------------------| + | Reserved | 0x02 | 16 | Reserved - set to 0 | + |--------------|---------------|------------------|----------------------------| + | Size | 0x04 | 32 | Size of PSP entry in bytes | + |--------------|---------------|------------------|----------------------------| + | Location / | 0x08 | 64 | Location: Physical Address | + | Value | | | of SPIROM location where | + | | | | corresponding PSP entry | + | | | | located. | + | | | | | + | | | | Value: 64-bit value for the| + | | | | PSP Entry | + +--------------+---------------+------------------+----------------------------+ + +### PSP Directory Table Types + +**0x00**: AMD public key +* Public key used by on-chip bootcode to verify the signature of PSP boot + loader firmware. + +**0x01**: PSP boot loader firmware +* Second stage boot loader firmware to be loaded by on-chip bootcode. + +**0x02**: PSP SecureOS firmware +* Off-chip PSP boot loader will be overwritten in SRAM by the Secure/Trusted + OS during initial boot up. +* PSP SecureOS performs: + * Initialization of OS internal structures and instantiates the fTPM as a + trusted application + * Sets up CPU/BIOS-PSP interface registers + * Enters steady state idling and waiting for commands + * In steady state, on notification, prepares for S3 state + * Verify and loading GFX Firmware + +**0x03**: PSP recovery boot loader firmware +* Recovery PSP boot loader image, loaded by on-chip bootcode in case of + failure in loading PSP boot loader. + +**0x08**: SMU off-chip firmware + +**0x12**: SMU off-chip firmware section 2 +* Power Management firmware, responsible for system power/clock management. + +**0x09**: Secure Debug unlock public key +* Public key token used during Secure Debug unlock process to verify message + payload from AMD server. + +**0x0b**: Soft fuse chain +* Refer to documentation for definitions. (See External References below.) + +**0x0c**: PSP trustlet binaries +* Optional file to enable fTPM. + +**0x13**: PSP Secure Debug unlock debug image +* Secure Debug unlock firmware image, used to unlock the device. + +**0x21**: Wrapped iKEK +* Intermediate Key Encryption Key, used to decrypt encrypted firmware images. + This is mandatory in order to support encrypted firmware. + +**0x24**: Security policy binary +* A security policy is applied to restrict the untrusted access to security + sensitive regions. + +**0x25**: MP2 firmware +* The MP2 of the SMU, also known as the Sensor Fusion Integration is used to + aggregate the data from various sensors such as accelerometer, gyrometer, + ambient light sensor, orientation sensor, etc. This is off-chip firmware + for Sensor Fusion Processor (SFP) subsystem of the SMU. + +**0x28**: System driver +* Driver executing on top of SecureOS. + +**0x30 - 0x37**: PSP AGESA binaries +* AGESA Boot Loaders (ABLs) are a set of binary images executed by the PSP. + They are responsible for initializing APU silicon components (including but + not limited to APU memory interface) on S5, S4 and S3, prior to releasing + the main cores from reset. + +**0x3a**: Whitelist +* Optional image containing a signed whitelist of one or more serial numbers. + +**0x40**: Pointer to secondary table +* Pointer to PSP Directory Table level 2. + +**0x52**: PSP boot loader usermode OEM application +* Supported only in certain SKUs. + +**0x22**: PSP Token Unlock data +* Used to support time-bound Secure Debug unlock during boot. This entry may + be omitted if the Token Unlock debug feature is not required. + +### Firmware Version of Binaries + +Every firmware binary contains 256 bytes of a PSP Header, which includes +the firmware version. The version is made up of the four bytes located at +offset 0x60 in the binary image. + +For example, in the PSP BootLoader: + + 0000000: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + 0000010: 2450 5331 c0e1 0000 0100 0000 0000 0000 $PS1............ + 0000020: 5c0a ddb8 b279 4846 e154 aa4c ed7d 414d ....yHF.T.L.}AM + 0000030: 0100 0000 0000 0000 60bb a67e 1a43 4c6b ........`..~.CLk + 0000040: 9807 bc8d fdb4 1f40 0000 0000 0000 0000 .......@........ + 0000050: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + 0000060: 7401 0800 ffff ffff 0001 0000 c0e3 0000 t............... + 0000070: 0000 0000 0000 0000 0000 0000 0100 0000 ................ + 0000080: 4766 9186 9d5f e909 492d 491d d9ee 8e6c Gf..._..I-I....l + 0000090: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + 00000a0: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + 00000b0: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + 00000c0: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + 00000d0: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + 00000e0: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + 00000f0: 0000 0000 0000 0000 0000 0000 0000 0000 ................ + +The PSP BootLoader version is 00.08.01.74. + +Note that only Firmware binary images have versions. Key tokens are not +versioned, as there will not be multiple keys. Keys are unique to processor +family. + +### BIOS Directory Table Entry Types + +All x86 accessible components (both executable and data blobs) are found via +the BIOS Directory Table. A second level table may be generated to allow for +updates without the risk of corrupting the primary table. + +The BIOS Directory table structure is slightly different from the PSP Directory: +* Multiple instances of firmware components are allowed for one specific type +* The type field is further structured to reflect attributes of BIOS + components such as "Region Type", "Reset Image", "Copy Image", "Read Only", + allowing design flexibility +* The "Destination Address" field is added for specific entries that are + expected to be copied from boot media to specific memory location + +### BIOS Directory Table Header + + +--------------+---------------+------------------+----------------------------+ + | Field Name | Offset (Hex) | Size (In Bytes) | Description/Purpose | + +--------------+---------------+------------------+----------------------------+ + | BIOS Cookie | 0x00 | 4 | BIOS cookie "$BHD" to | + | | | | recognize the header. | + | | | | Cookie “$BL2” for level 2 | + |--------------|---------------|------------------|----------------------------| + | Checksum | 0x04 | 4 | 32 bit CRC value of header | + | | | | below this field and | + | | | | including all entries | + |--------------|---------------|------------------|----------------------------| + | Total Entries| 0x08 | 4 | Number of BIOS Directory | + | | | | entries in the table | + |--------------|---------------|------------------|----------------------------| + | Reserved | 0x0C | 4 | Reserved - Set to zero | + +--------------+---------------+------------------+----------------------------+ + +### BIOS Directory Table Entries + + +--------------+---------------+------------------+----------------------------+ + | Field Name | Offset (Hex) | Size (In Bits) | Description/Purpose | + +--------------+---------------+------------------+----------------------------+ + | Type | 0x00 | 8 | Entry type (see below) | + |--------------|---------------|------------------|----------------------------| + | Region Type | 0x01 | 8 | Setup the memory region's | + | | | | security attribute for the | + | | | | BIOS entry | + |--------------|---------------|------------------|----------------------------| + | Reset Image | 0x02[0] | 1 | Boolean value to define the| + | | | | BIOS entry is a reset | + | | | | binary image | + |--------------|---------------|------------------|----------------------------| + | Copy Image | 0x02[1] | 1 | Define the binary image of | + | | | | the BIOS entry is for | + | | | | copying over to the memory | + | | | | region | + |--------------|---------------|------------------|----------------------------| + | Read Only | 0x02[2] | 1 | Setup the memory region for| + | | | | the BIOS entry to read only| + |--------------|---------------|------------------|----------------------------| + | Compressed | 0x02[3] | 1 | Compressed using zlib | + | | | | | + |--------------|---------------|------------------|----------------------------| + | Instance | 0x02[7:4] | 4 | Specify the Instance of an | + | | | | entry | + |--------------|---------------|------------------|----------------------------| + | SubProgram | 0x03[2:0] | 3 | Specify the SubProgram | + |--------------|---------------|------------------|----------------------------| + | Reserved | 0x03[7:3] | 5 | Reserved - Set to zero | + |--------------|---------------|------------------|----------------------------| + | Size | 0x04 | 32 | Memory Region Size | + |--------------|---------------|------------------|----------------------------| + | Source | 0x08 | 64 | Physical Address of SPIROM | + | Address | | | location where the data for| + | | | | the corresponding entry is | + | | | | located | + |--------------|---------------|------------------|----------------------------| + | Destination | 0x10 | 64 | Destination Address of | + | Address | | | memory location where the | + | | | | data for the corresponding | + | | | | BIOS Entry is copied | + +--------------+---------------+------------------+----------------------------+ + +### BIOS Directory Table Entry Types + +**0x60**: APCB data +* Source field points to the AGESA PSP Customization Block (APCB) data. + +**0x68**: Backup copy of APCB data +* Source field points to the backup copy of the AGESA PSP Customization Block + (APCB) data. + +**0x61**: APOB data +* Location field points to the AGESA PSP Output Block (APOB) data. + +**0x62**: BIOS reset image +* Source field points to BIOS binary image in flash. Destination points to + DRAM. + +**0x63**: APOB data NV +* Source field points to the AGESA PSP Output Block (APOB) data NV copy. + This data is written by coreboot and replayed by PSP ABLs during S3 resume + and in certain S5 boots. + +**0x64**: PMU firmware (instruction) +* Source field points to the instruction portion of Phy Microcontroller Unit + firmware. + +**0x65**: PMU firmware (data) +* Source field points to the data portion of Phy Microcontroller Unit + firmware. + +**0x66**: x86 microcode patch +* Source field points to the microcode patch. + +**0x6a**: MP2 FW config file +* Source field points to the MP2 FW configuration file. + +**0x70**: Pointer to secondary table +* Pointer to BIOS Directory Table level 2. + +## Tools + +### amdcompress + +`cbfstool/amdcompress` is a helper for creating the BIOS Reset Image (BIOS +Directory Table type 0x62). This is the code the PSP uncompresses into DRAM +at the location where the x86 begins execution when released from reset. +Typical usage is for amdcompress to convert an ELF file’s program section +into a zlib compressed image. + +### amdfwtool + +All images requiring PSP functionality rely on the amdfwtool utility. +amdfwtool takes image names as command-line arguments, as well as the size of +the flash device, and intended location of the Embedded Firmware Structure. +Its output is a monolithic image with correctly positioned headers, pointers, +structures, and the firmware images added. The file, typically named +`amdfw.rom`, may then be added directly into the coreboot image. + +## External Reference + +* NDA document #55758: *AMD Platform Security Processor BIOS Architecture + Design Guide for AMD Family 17h Processors* +* NDA document #54267 *AMD Platform Security Processor BIOS Architecture + Design Guide*: For all devices earlier than Family 17h
-
Angel Pons (Code Review) -
Eric Peers (Code Review)
-
Felix Held (Code Review)
-
Kyösti Mälkki (Code Review)
-
Marshall Dawson (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)