This reverts commit 91f5da4776.
Commit breaks booting on MDN (and likely others). Boot hangs on:
[NOTE ] MRC: no data in 'RW_MRC_CACHE'
Change-Id: Id8042690af2764d6e46fe01287be598091b1a239
Signed-off-by: Matt DeVillier <matt.devillier@amd.corp-partner.google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/76718
Reviewed-by: Martin Roth <martin.roth@amd.corp-partner.google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Varshit Pandya <pandyavarshit@gmail.com>
Reviewed-by: Eric Lai <eric_lai@quanta.corp-partner.google.com>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Hash table containing hashes of all signed PSP binaries is compiled at
build time and installed into the concerned CBFS. During boot, PSP
verstage reads the hash table binary and passes it to PSP bootloader.
PSP bootloader in turn uses the hash table to verify the signed PSP
binaries. Currently the hashes for all the signed PSP binaries are
compiled into one hash table. On upcoming platforms with more number of
signed PSP binaries, PSP bootloader does not have resources to handle
one monolithic hash table. Instead PSP bootloader recommends splitting
them into smaller hash tables (currently limited to 3 hash tables).
Update amdfwtool tool to support splitting hash tables. This is done by
adding an optional hash table id to the entries in the amdfw.cfg file.
By default, one hash table binary is always compiled and it's name is of
the format ${signed_rom}.hash. If an entry has a hash table id defined,
then this utility will compile a separate hash table binary whose name
is of the format ${signed_rom}.${N}.hash where N is the hash table id.
BUG=b:277292697
TEST=Build Skyrim BIOS image and boot to OS. Ensure that the hash table
is identical with and without this change. Perform suspend/resume
cycles, warm/cold reset cycles for 50 iterations each.
TEST=Artificially inject hash table id against some entries in
amdfw.cfg and ensure that the concerned hash table binaries are getting
compiled.
Change-Id: I7ef338d67695a34c33b5c166924832939f381191
Signed-off-by: Karthikeyan Ramasubramanian <kramasub@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/75188
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Passing this option tells amdfwtool to create a text file, containing
the versions of the blobs below:
- PSP bootloader (type 0x01),
- SMU firmware (type 0x08),
- AGESA bootloader 0 (type 0x30),
- PSP bootloader AB (type 0x73).
Created file can be embedded into CBFS which allows to read the version
of blobs at runtime. This way version of blobs used to build the
coreboot image can be verified at runtime and also from the binary file.
Format of manifest file is following:
$ cat build/amdfw_manifest
type: 0x01 ver:00.35.00.13
type: 0x08 ver:00.5a.23.a6
type: 0x30 ver:2a.14.b0.10
type: 0x73 ver:00.35.00.13
BUG=b:224780134
TEST=Tested on Skyrim device
Change-Id: Idaa3a02ace524f44cfa656e34308bd896016dff6
Signed-off-by: Grzegorz Bernacki <bernacki@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/74266
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martin.roth@amd.corp-partner.google.com>
This is a PSP FW requirement.
This is only for recovery A/B without ISH header. That means only
Cezanne.
Change-Id: I62616d5a866f66fc71e6c0b31a23c62dc11cf3c6
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/75161
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin L Roth <gaumless@gmail.com>
There are 2 regex patterns defined to process the lines from *fw.cfg:
1) for lines with mandatory entries
2) for lines with mandatory + optional entries
Consolidate the regex pattern. Add enums for matching regex caller
groups so that the human readable group IDs can be used instead of magic
numbers.
BUG=None
TEST=Build Skyrim BIOS which only have mandatory entries. Build Guybrush
BIOS image which have both mandatory and optional entries. Confirm that
the amdfw.rom built before and after this change have matching SHA in
both Skyrim and Guybrush images. This ensures that the optional level
entries in Guybrush are handled as expected. Boot to OS in Skyrim.
Change-Id: I7289ddbbec4d5daefe64f59b687ba3a4af46d052
Signed-off-by: Karthikeyan Ramasubramanian <kramasub@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/74950
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
The MAX_PSP_ENTRIES constant reserves space for the psp directory table
entries. This table is aligned to 4K and the next binary is also aligned
to 4K. The number of psp directory entries on Birman exceeds the
previous limit, so increase it to the maximum that will fit in a 4K
block.
TEST=timeless builds for Birman unchanged
Signed-off-by: Fred Reitberger <reitbergerfred@gmail.com>
Change-Id: I297edc9cccffde0ad1ce7461b375542f9f2f7c23
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73653
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Bao Zheng <fishbaozi@gmail.com>
Reviewed-by: Nikolai Vyssotski <nikolai.vyssotski@amd.corp-partner.google.com>
Reviewed-by: Martin Roth <martin.roth@amd.corp-partner.google.com>
Keep clean copies of PSP and BIOS table. Refresh the working tables
before they are filled with file names and other information at each
iteration.
Change-Id: Ie8339a4d66c38e02180cbf99e13914bfff66dc0f
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73628
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
To reduce the size of amdfwtool.c which is already too big.
Change-Id: Ib80eeb42f59a3dda04402b2feaadc1d178ed989e
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73910
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
Since commit 2f6b7d557d ("amdfwtool: Move the filling of table headers
into functions"), the combo_psp_directory union element in the
embedded_firmware is unused and the new_psp_directory element is used in
all places, so replace the union of new_psp_directory and
combo_psp_directory with just the new_psp_directory struct element.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I35d339b3084ec8f93210095c233f5e68296d0013
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73834
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
Change-Id: I9e83a3ac56d5c42d8d6839cc4d961adf0b656fb5
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73725
Reviewed-by: Eric Lai <eric_lai@quanta.corp-partner.google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin L Roth <gaumless@gmail.com>
Change-Id: Ia98fdbee4c4005562662313ebe2478d0aeb879bc
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73724
Reviewed-by: Eric Lai <eric_lai@quanta.corp-partner.google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin L Roth <gaumless@gmail.com>
This allows individual components to be placed in a location other than
what is specified by the FIRMWARE_LOCATION line.
Signed-off-by: Martin Roth <gaumless@gmail.com>
Change-Id: I3a83e52d081a5909d54eacc575dd2b40b09e4038
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73656
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com>
If Recovery/Backup APCB is not passed, then AMD_BIOS_APCB_BK entry is
not populated. But PSP expects that bios directory entry to be
populated. Also on mainboards where both APCB and recovery APCB are same
(eg. Skyrim), 2 copies of the same APCB are added to amdfw*.rom. Update
amdfwtool to support not passing recovery/backup APCB. If the recovery
APCB is not passed, then populate AMD_BIOS_APCB_BK entry and make it
point to the same offset as AMD_BIOS_APCB entry.
BUG=b:240696002
TEST=Build and boot to OS in Skyrim. Ensure that the device can enter
recovery mode. Perform multiple suspend/resume cycles.
Change-Id: I031ba817573cd35160f5e219b1b373ddce69aa6b
Signed-off-by: Karthikeyan Ramasubramanian <kramasub@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73661
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
On newer SoCs the initial alignment is not required. So skip initial
alignment. This saves 64 KiB flash space on each firmware slots. This
also saves ~5 ms while loading amdfw.rom
BUG=b:240696002
TEST=Build and boot to OS in Skyrim.
Change-Id: I27cbfde2d7d58b62a4c0039c60babc3fb3bd95fa
Signed-off-by: Karthikeyan Ramasubramanian <kramasub@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73654
Reviewed-by: Martin Roth <martin.roth@amd.corp-partner.google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
It was used for printing the dependencies which is now taken by macro
DEP_FILES in soc/amd/common/Makefile.inc.
TEST=binary identical test on google/guybrush amd/chausie
Change-Id: I1b86df2cb2ed178cf0a263c50ccb3e2254a3852b
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73627
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Move main body of PSP padding into a loop which can add a new combo
entry. In the loop, get the FW files from each fw.cfg, create new pack
of PSP, and fill the combo header. Currently Feature COMBO is still
not fully functional. But the non-combo case will not be affected for
sure.
The real changes are
1. Add a do-while loop.
2. Remove a "TODO" comment.
All other changes are re-indenting and re-filling.
Change-Id: I351192a4bc5ed9ec0bfa3f2073c9633b8b44246d
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58554
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
For now, combo index is 0, and only the first entry in config table is
used. The index will grow when there are more combo entries.
Add a command parameter to give fw.cfg for combo index 1. Process the
combo config in the future loop.
Change-Id: I00609d91defc08e17f937ac8339575f84b1bd37c
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73496
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
And move the additional processing to this new function.
Change-Id: Id101d63e4d30a6e57ac1aa79665a4ba22b2956f1
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73509
Reviewed-by: Martin Roth <martin.roth@amd.corp-partner.google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
amdfwtool always assumes that the PSP BIOS binary (type 0x62 BIOS
directory entry) is always compressed. On boards using vboot, sometimes
PSP BIOS binary is uncompressed - specifically when CBFS verification is
enabled and verified boot starts in bootblock. Add an option to indicate
PSP BIOS binary is uncompressed.
BUG=b:261792282
TEST=Build Skyrim BIOS with x86 verstage and CBFS Verification enabled.
Boot to OS.
Change-Id: I4d56c0ba451b194043ebb5cdb0f2b27482beef1f
Signed-off-by: Karthikeyan Ramasubramanian <kramasub@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/71210
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin L Roth <gaumless@gmail.com>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Type 0x5d (MPIO Firmware) was mistakenly placed to PSP Level 1 directory.
It should be in Level 2 PSP directory instead.
Change-Id: Ic5ea00859f1055e0c91600c5f941c5d3acca36e2
Signed-off-by: Nikolai Vyssotski <nikolai.vyssotski@amd.corp-partner.google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73556
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Bao Zheng <fishbaozi@gmail.com>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
Use the inst field when adding entries to the psp tables. Otherwise,
entries that differ by the inst field will appear as duplicates with an
inst of 0.
Signed-off-by: Fred Reitberger <reitbergerfred@gmail.com>
Change-Id: I4a84a0730976f4c65902b5c24ed13e21e95b03bb
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73522
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martin.roth@amd.corp-partner.google.com>
It is similar to PSP combo.
Change-Id: If0523a4a0e1f31969e4bbaa6062dcc0f2d6da420
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/66856
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
If combo is used, fill the EFS header with address of COMBO header.
If not, fill with address of PSP header.
The old code fills with PSP headers all the time.
Change-Id: I0057165aea553d9dc8e4e719e2804557229a0002
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/66855
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
There will be a loop to set up the combo layout. The combo header only
needs to be created once. This change is actually to move the creation
of combo header outside of the loop.
Change-Id: If6ba3d10dfc598133b9adbbb2b6658f356455608
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/66854
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
It is easier to understand what these statements are about.
Change-Id: Ib02c68c9f2ea84020b12682c41fb1a6f8f93d725
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/66852
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
To support 32M flash, the non-vboot also need to split amdfw. Just as
the deleted comment says, we need this feature now.
This is one of series of patches to support 32/64M flash.
BUG=b:255374782
Change-Id: Ic058cfaeebd1a947227cfa9be2db4eb22702aa28
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/69857
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
To support 32M flash, the non-vboot also need to split amdfw.
The amdfw.rom is the default filename added to CBFS.
Keep the default filename and then we don't have to change all the
CBFS definition.
This is one of series of patches to support 32/64M flash.
BUG=b:255374782
Change-Id: Id77b11422d4549cf57a1cd8980c7a9cf3597d1bc
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/72702
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
This filling does not need to be done separately.
Change-Id: I53051349923dce40f4fc3f747ab41a93a3798823
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/66853
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
A few references to "--combo-capable" were left after commit 4bfb36ed68
Change-Id: I6f425db2a8b86d7ad928baee6bc7b07e5190ba37
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73281
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
When the EFS data is being packed, the pointer should be at EFS
header.
After that, it should be at body location.
TEST=binary identical test on amd/birman amd/chausie amd/majolica
amd/gardenia pcengines/apu2 amd/mandolin
Change-Id: Ia81e2bdf9feb02971723f39e7f223b5055807cd8
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73180
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
We need to considering the case the EFS header is given as a relative
address and the other, body location, is given as an absolute one. So
we convert both of them to relative and check the validation.
For relative address case, the location should be between
0 and data size.
Change-Id: I7898bfbca02f5eb1c0fb7c456dc1935bddf685b1
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73024
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
If ctx.address_mode is "physical", it will keep as "physical".
If ctx.address_mode is "relative to table", it will be changed as
"relative to BIOS".
Because the "current table" is the whole flash, the code worked well.
TEST=Binary identical test on amd/birman amd/chausie amd/majolica
amd/gardenia pcengines/apu2 amd/mandolin
Change-Id: I9acb54cc5de149d8a705bb05bf351c44b7d3ced1
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73120
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
Changing the pointer outside the function is not allowed.
Check if it overflows everytime it changes.
TEST=Binary identical on amd/birman amd/chausie amd/majolica
amd/gardenia pcengines/apu2 amd/mandolin
Change-Id: I2c295b489d833201f1ba86a7759ea7dc0e1e672f
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73075
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
Now we use ctx.rom. Remove the wrong statement releasing null
pointer.
Change-Id: I134335ed741dc067e232621106f2057e50ba6a1a
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73118
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
Since it actually depends on the SoC type whether the old PSP
directory table pointer or the new comboable PSP directory table
pointer is used in EFS, get this information from the SoC ID instead
of passing the comboable flag for the SoCs that need to use the new
comboable PSP directory table pointer.
TEST=Binary identical on amd/majolica, pcengines/apu2, amd/gardenia
Change-Id: I0c3f21065939d1b13c2607aba16cbef74dd8d389
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73020
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
The fw.cfg should combine the SOC name.
This is for future combo feature. Each entry in combo has its own
fw.cfg.
The soc_id in struct cb_config can only be available after the fw.cfg
is processed.
Some functions which take soc_id as a parameter can be simplified.
3/5 (and the key one with same change ID)
of split changes of https://review.coreboot.org/c/coreboot/+/58552/28
Change-Id: Ib0eead1f2156542ea03d58145f5ad67683bf9b52
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58552
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
Save the soc_id into a global struct.
Change-Id: I2a0f04a09635086e3076a97b535df8a19d0693ce
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/72450
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
We need to put soc name to fw.cfg for future combo feature.
We skip for now when SOC_NAME is found.
1/5
of split changes https://review.coreboot.org/c/coreboot/+/58552/28
Change-Id: I2b8d7154d22db13675ff57b6abe61c747604c524
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/72456
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
Matt DeVillier