Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Zheng Bao, Felix Held.
Hello Zheng Bao,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/56937
to review the following change.
Change subject: soc/amd/cezanne: Add PSP Bootloader for AB recovery in fw.cfg
......................................................................
soc/amd/cezanne: Add PSP Bootloader for AB recovery in fw.cfg
Change-Id: I099b4c98d64dba935bf3ea2b7f191da83b9bd95e
Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com>
---
M src/soc/amd/cezanne/fw.cfg
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/56937/1
diff --git a/src/soc/amd/cezanne/fw.cfg b/src/soc/amd/cezanne/fw.cfg
index 9757d72..82ef95e 100644
--- a/src/soc/amd/cezanne/fw.cfg
+++ b/src/soc/amd/cezanne/fw.cfg
@@ -6,6 +6,7 @@
# PSP
AMD_PUBKEY_FILE TypeId0x00_CezannePublicKey.tkn
PSPBTLDR_FILE TypeId0x01_PspBootLoader_CZN.sbin
+PSPBTLDR_AB_STAGE1_FILE TypeId0x01_PspBootLoader_AB_Stage1_CZN.sbin
PSPBTLDR_WL_FILE TypeId0x01_PspBootLoader_WL_CZN.sbin
PSPSECUREOS_FILE TypeId0x02_PspOS_CZN.sbin
PSPRCVR_FILE TypeId0x03_PspRecoveryBootLoader_CZN.sbin
--
To view, visit https://review.coreboot.org/c/coreboot/+/56937
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I099b4c98d64dba935bf3ea2b7f191da83b9bd95e
Gerrit-Change-Number: 56937
Gerrit-PatchSet: 1
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Zheng Bao
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Zheng Bao
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newchange
Attention is currently required from: Bao Zheng, Jason Glenesk, Raul Rangel, Marshall Dawson, Zheng Bao, Felix Held.
Hello build bot (Jenkins), Jason Glenesk, Raul Rangel, Marshall Dawson, Zheng Bao, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56773
to look at the new patch set (#3).
Change subject: amdfwtool: Add support for A/B recovery
......................................................................
amdfwtool: Add support for A/B recovery
The rom layout for A/B recovery:
EFS -> PSP L1 0x48 -> PSP L2 A -> BIOS L2 A
0x4A -> PSP L2 B -> BIOS L2 B
To enable A/B recovery in PSP layout, add "--recovery-ab" to
amdfwtool.
Change-Id: I27f5d3476f648fcecafb8d258ccb6cfad4f50036
Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com>
---
M util/amdfwtool/amdfwtool.c
M util/amdfwtool/amdfwtool.h
M util/amdfwtool/data_parse.c
3 files changed, 215 insertions(+), 54 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/56773/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/56773
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I27f5d3476f648fcecafb8d258ccb6cfad4f50036
Gerrit-Change-Number: 56773
Gerrit-PatchSet: 3
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Zheng Bao
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Julius Werner, Jan Dabros.
Jakub Czapiga has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56601 )
Change subject: tests: Add lib/cbfs-verification-test test case
......................................................................
Patch Set 8:
(9 comments)
File tests/lib/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/56601/comment/2d209490_fb5424e8
PS7, Line 35: tests-y += cbfs-verification-no-sha512-test
> Should explain the rationale for testing both cases in a comment somewhere.
Done
https://review.coreboot.org/c/coreboot/+/56601/comment/854c1222_e6f0c5a0
PS7, Line 188: cbfs-verification-sources := tests/lib/cbfs-verification-test.c \
> I wonder if it makes sense to define a copy_test function for this sort of stuff, so that you could […]
Please see CB:56932
https://review.coreboot.org/c/coreboot/+/56601/comment/ff862803_e7ac7df1
PS7, Line 198: 3rdparty/vboot/firmware/include
> Shouldn't we just put these into the default include path for tests? They are in the default include […]
Yes, those paths were missing. I added them in CB:56933
https://review.coreboot.org/c/coreboot/+/56601/comment/f3d37ebf_e0b58189
PS7, Line 219: CONFIG_CBFS_VERIFICATION=1
> Wasn't the point that this should be 0 here?
Yes, it should. Copy-paste error...
File tests/lib/cbfs-verification-test.c:
https://review.coreboot.org/c/coreboot/+/56601/comment/b3b598fd_c044254d
PS7, Line 147: {
> Actually, I think it would be better to properly mock this too (comparing buf and size to expected v […]
Done
https://review.coreboot.org/c/coreboot/+/56601/comment/a56b367f_62bf100e
PS7, Line 216: &file_valid_hash.attrs_and_data[HASH_ATTR_SIZE], HASH_ATTR_SIZE);
> Uhhh... wait a second... does this test succeed? Because it shouldn't succeed. […]
That's because of a typo in the test configuration. Fixed :)
https://review.coreboot.org/c/coreboot/+/56601/comment/ac95683c_d960be25
PS7, Line 243: assert_null(mapping);
> And this should return a valid mapping because verification is disabled. […]
Done
https://review.coreboot.org/c/coreboot/+/56601/comment/6124fce2_cfcadbbf
PS7, Line 283: /* No cache, so no validation required */
> (nit: comment still makes no sense here)
True. Removed
https://review.coreboot.org/c/coreboot/+/56601/comment/6843ecd1_072cf469
PS7, Line 288: has_mcache
> Well, you can either define NO_CBFS_MCACHE, or you can test mcache stuff here. Not both.
Ack
--
To view, visit https://review.coreboot.org/c/coreboot/+/56601
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1d8cbb1c2d0a9db3236de065428b70a9c2a66330
Gerrit-Change-Number: 56601
Gerrit-PatchSet: 8
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Comment-Date: Thu, 12 Aug 2021 08:39:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Jakub Czapiga has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56932 )
Change subject: tests/Makefile.inc: Add copy-test macro
......................................................................
tests/Makefile.inc: Add copy-test macro
copy-test macro copies attributes from one test to another. It can be
used to create multiple tests with the same subset of attributes values.
Signed-off-by: Jakub Czapiga <jacz(a)semihalf.com>
Change-Id: I9c9b6b12830c7060ffe5dcf35c9486655cbd08e0
---
M tests/Makefile.inc
1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/56932/1
diff --git a/tests/Makefile.inc b/tests/Makefile.inc
index 85d30a2..f3a708a 100644
--- a/tests/Makefile.inc
+++ b/tests/Makefile.inc
@@ -94,6 +94,12 @@
Check your $(dir $(1)$(2))Makefile.inc))
endef
+# Copy attributes of one test to another.
+# $1 - input test name
+# $2 - output test name
+copy-test = $(foreach attr,$(attributes), \
+ $(eval $(strip $(2))-$(attr) := $($(strip $(1))-$(attr))))
+
$(call add-special-class, tests)
$(call evaluate_subdirs)
--
To view, visit https://review.coreboot.org/c/coreboot/+/56932
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9c9b6b12830c7060ffe5dcf35c9486655cbd08e0
Gerrit-Change-Number: 56932
Gerrit-PatchSet: 1
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-MessageType: newchange
Attention is currently required from: Jan Dabros.
Hello build bot (Jenkins), Julius Werner, Jan Dabros,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56813
to look at the new patch set (#5).
Change subject: tests: Add lib/cbfs-lookup-test test case
......................................................................
tests: Add lib/cbfs-lookup-test test case
Signed-off-by: Jakub Czapiga <jacz(a)semihalf.com>
Change-Id: I2ebebba1468c19661741de8a8456605b1c5f56b6
---
M tests/include/tests/lib/cbfs_util.h
M tests/lib/Makefile.inc
A tests/lib/cbfs-lookup-test.c
3 files changed, 506 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/56813/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/56813
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2ebebba1468c19661741de8a8456605b1c5f56b6
Gerrit-Change-Number: 56813
Gerrit-PatchSet: 5
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Jakub Czapiga, Jan Dabros.
Hello build bot (Jenkins), Paul Fagerburg, Julius Werner, Jan Dabros,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56601
to look at the new patch set (#8).
Change subject: tests: Add lib/cbfs-verification-test test case
......................................................................
tests: Add lib/cbfs-verification-test test case
This commit adds test case for lib/cbfs verification mechanisms.
Signed-off-by: Jakub Czapiga <jacz(a)semihalf.com>
Change-Id: I1d8cbb1c2d0a9db3236de065428b70a9c2a66330
---
A tests/include/tests/lib/cbfs_util.h
M tests/lib/Makefile.inc
A tests/lib/cbfs-verification-test.c
A tests/stubs/die.c
4 files changed, 368 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/56601/8
--
To view, visit https://review.coreboot.org/c/coreboot/+/56601
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1d8cbb1c2d0a9db3236de065428b70a9c2a66330
Gerrit-Change-Number: 56601
Gerrit-PatchSet: 8
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com>
Gerrit-MessageType: newpatchset