Attention is currently required from: Arthur Heymans, Martin Roth, Julius Werner, Yu-Ping Wu.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59132 )
Change subject: Makefile.inc: Generate master header and pointer as C structs
......................................................................
Patch Set 6:
(4 comments)
Patchset:
PS4:
> > but Libpayload needs to support FMAP for that.
>
> It does now, for the new CBFS APIs (e.g. cbfs_map()). We still left the old APIs untouched so that payloads have a grace period to migrate (and I guess we should probably start announcing that on the mailing list), but we did migrate depthcharge, and I doubt any other payload actually cared about reading non-primary CBFSes (which IIRC was the only place where the master header still mattered?). So I think anything using libpayload should be good now. But that still leaves the GRUB and SeaBIOS concerns?
Cool. I did send some patches for SeaBIOS some time ago. I guess I can add a Kconfig variable with a deprecated warning to optionally leave it out.
File src/arch/x86/header_pointer.c:
https://review.coreboot.org/c/coreboot/+/59132/comment/b9f3e245_99c4a8d2
PS6, Line 7: __attribute__((used, __section__(".header_pointer"))) const uint32_t header_pointer =
> Wouldn't it be simpler to just merge this with the src/lib version and wrap the section attribute into an #if CONFIG(ARCH_X86)?
good idea.
File src/lib/cbfs_master_header.c:
https://review.coreboot.org/c/coreboot/+/59132/comment/c36923b8_d2a73a58
PS6, Line 13: FMAP_SECTION_FLASH_START
> Isn't this just 0? Rather than assuming that all FMAPs are named "FLASH", I think it's easier to just leave this out?
No it's often the base if the whole flash was memory mapped. Maybe it's worth dropping that usecase and force it to be 0 accross the tree as memory mapping is handled better now that there is no need for this.
File src/lib/master_header_pointer.c:
https://review.coreboot.org/c/coreboot/+/59132/comment/173a25cc_136ed574
PS6, Line 7: __attribute((used))
> I don't think you need this? We've never needed it for other structs...
>
> (Also, I don't think __attribute(()) compiles without the __ on the other end?)
right.
--
To view, visit https://review.coreboot.org/c/coreboot/+/59132
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3ba01be7da1f09a8cac287751497c18cda97d293
Gerrit-Change-Number: 59132
Gerrit-PatchSet: 6
Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Tue, 05 Apr 2022 18:18:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Martin Roth <martinroth(a)google.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63217 )
Change subject: Makefile.inc: Move adding bootblock non-x86 targets
......................................................................
Patch Set 5:
(1 comment)
File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/63217/comment/c6b17e8a_2c8ed305
PS2, Line 1122: ifeq ($(CONFIG_ARCH_X86),y)
> looks like I messed up ^^
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/63217
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I50eae4f00d171d26a221ca969086f4f294fa524b
Gerrit-Change-Number: 63217
Gerrit-PatchSet: 5
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Tue, 05 Apr 2022 18:12:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans.
Hello build bot (Jenkins), Sean Rhodes, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56119
to look at the new patch set (#13).
Change subject: security/intel/cbnt/Makefile.inc: Improve build flow
......................................................................
security/intel/cbnt/Makefile.inc: Improve build flow
Using 'files_added::' is not needed any longer as all files have
already been added to the build. This has the advantage of showing all
final entries in the FIT table and CBFS during the build process as
adding the bpm to cbfs and fit is moved earlier.
Change-Id: I22aa140202f0665b7095a01cb138af4986aa9ac3
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/security/intel/cbnt/Makefile.inc
1 file changed, 8 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/56119/13
--
To view, visit https://review.coreboot.org/c/coreboot/+/56119
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I22aa140202f0665b7095a01cb138af4986aa9ac3
Gerrit-Change-Number: 56119
Gerrit-PatchSet: 13
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newpatchset
Attention is currently required from: Jes Klinke, Christian Walter, Julius Werner.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63158 )
Change subject: tpm: Accept Google Ti50 TPM DID:VID
......................................................................
Patch Set 12:
(1 comment)
File src/mainboard/google/brya/Kconfig:
https://review.coreboot.org/c/coreboot/+/63158/comment/a760058e_b953c6a4
PS11, Line 32: MAINBOARD_HAS_I2C_TPM_TI50
> That is right, thanks. […]
Brask is also cr50 only
--
To view, visit https://review.coreboot.org/c/coreboot/+/63158
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1e1f8eb9b94fc2d5689656335dc1135b47880986
Gerrit-Change-Number: 63158
Gerrit-PatchSet: 12
Gerrit-Owner: Jes Klinke <jbk(a)chromium.org>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Jett Rink <jettrink(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Jes Klinke <jbk(a)chromium.org>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Tue, 05 Apr 2022 18:08:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jes Klinke <jbk(a)chromium.org>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Hung-Te Lin, Jason Glenesk, Raul Rangel, Marshall Dawson, Christian Walter, Julius Werner, Andrey Petrov, Fred Reitberger, Yu-Ping Wu, Felix Held.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63375 )
Change subject: [RFC] CBMEM: Have INIT_HOOKS in every stage
......................................................................
Patch Set 1:
(2 comments)
File src/include/cbmem.h:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-145408):
https://review.coreboot.org/c/coreboot/+/63375/comment/0e68ab48_00b52262
PS1, Line 109: #define CBMEM_INIT_HOOK(init_fn_) \
macros should not use a trailing semicolon
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-145408):
https://review.coreboot.org/c/coreboot/+/63375/comment/990ba98d_471227d1
PS1, Line 115: #define CBMEM_INIT_HOOK_EARLY(init_fn_) \
macros should not use a trailing semicolon
--
To view, visit https://review.coreboot.org/c/coreboot/+/63375
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie24bf4e818ca69f539196c3a814f3c52d4103d7e
Gerrit-Change-Number: 63375
Gerrit-PatchSet: 1
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
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: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 05 Apr 2022 17:53:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/63319 )
Change subject: amdfwtool: Add a macro to set explicitly second gen for old SOCs
......................................................................
amdfwtool: Add a macro to set explicitly second gen for old SOCs
It is more reasonable than getting the value from memset.
For the reserved bits, keep them as they were for old SOCs.
Change-Id: I65caa11e835d2ff52bec4b8904057bbced434891
Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/63319
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de>
---
M util/amdfwtool/amdfwtool.c
M util/amdfwtool/amdfwtool.h
2 files changed, 6 insertions(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Felix Held: Looks good to me, approved
diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c
index 80595ddb..23273e6 100644
--- a/util/amdfwtool/amdfwtool.c
+++ b/util/amdfwtool/amdfwtool.c
@@ -1383,6 +1383,8 @@
}
switch (soc_id) {
case PLATFORM_STONEYRIDGE:
+ amd_romsig->efs_gen.gen = EFS_BEFORE_SECOND_GEN;
+ amd_romsig->efs_gen.reserved = ~0;
amd_romsig->spi_readmode_f15_mod_60_6f = efs_spi_readmode;
amd_romsig->fast_speed_new_f15_mod_60_6f = efs_spi_speed;
break;
@@ -1390,6 +1392,8 @@
case PLATFORM_PICASSO:
/* amd_romsig->efs_gen introduced after RAVEN/PICASSO.
* Leave as 0xffffffff for first gen */
+ amd_romsig->efs_gen.gen = EFS_BEFORE_SECOND_GEN;
+ amd_romsig->efs_gen.reserved = ~0;
amd_romsig->spi_readmode_f17_mod_00_2f = efs_spi_readmode;
amd_romsig->spi_fastspeed_f17_mod_00_2f = efs_spi_speed;
switch (efs_spi_micron_flag) {
@@ -1410,6 +1414,7 @@
case PLATFORM_MENDOCINO:
case PLATFORM_SABRINA:
amd_romsig->efs_gen.gen = EFS_SECOND_GEN;
+ amd_romsig->efs_gen.reserved = 0;
amd_romsig->spi_readmode_f17_mod_30_3f = efs_spi_readmode;
amd_romsig->spi_fastspeed_f17_mod_30_3f = efs_spi_speed;
switch (efs_spi_micron_flag) {
@@ -1803,7 +1808,6 @@
amd_romsig->imc_entry = 0;
amd_romsig->gec_entry = 0;
amd_romsig->xhci_entry = 0;
- amd_romsig->efs_gen.reserved = 0;
if (soc_id != PLATFORM_UNKNOWN) {
retval = set_efs_table(soc_id, amd_romsig, efs_spi_readmode,
diff --git a/util/amdfwtool/amdfwtool.h b/util/amdfwtool/amdfwtool.h
index a241316..3af4e94 100644
--- a/util/amdfwtool/amdfwtool.h
+++ b/util/amdfwtool/amdfwtool.h
@@ -88,6 +88,7 @@
} __attribute__((packed));
#define EFS_SECOND_GEN 0
+#define EFS_BEFORE_SECOND_GEN 1
typedef struct _embedded_firmware {
uint32_t signature; /* 0x55aa55aa */
--
To view, visit https://review.coreboot.org/c/coreboot/+/63319
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I65caa11e835d2ff52bec4b8904057bbced434891
Gerrit-Change-Number: 63319
Gerrit-PatchSet: 3
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Subrata Banik, Tim Wawrzynczak, Paul Menzel, Kane Chen.
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63169 )
Change subject: soc/intel/common: Update CSE sub partition update
......................................................................
Patch Set 6:
(1 comment)
File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/63169/comment/08a6f192_a38d440e
PS5, Line 955: static const char * const cse_regions[] = {"RO", "RW"};
> Since you are also reusing this ("RO" anyway) on line 793, does it make sense to move this declarati […]
Ack
--
To view, visit https://review.coreboot.org/c/coreboot/+/63169
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie9a83b77ab44ea6ffe5bb20673e109a89a148629
Gerrit-Change-Number: 63169
Gerrit-PatchSet: 6
Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Kane Chen <kane.chen(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kane Chen <kane.chen(a)intel.com>
Gerrit-Comment-Date: Tue, 05 Apr 2022 17:34:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik, Paul Menzel, Sridhar Siricilla, Kane Chen.
Hello build bot (Jenkins), Subrata Banik, Tim Wawrzynczak, Kane Chen,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/63169
to look at the new patch set (#6).
Change subject: soc/intel/common: Update CSE sub partition update
......................................................................
soc/intel/common: Update CSE sub partition update
The patch adds support in the CSE Sub partition update procedure
to use GET_BOOT_PARTITION_INFO HECI command output to create the
region device for CSE RO and CSE RW. The GET_BOOT_PARTITION_INFO
HECI command provides CSE's RO and RW boot partition information.
Existing code relies on FMD file to get the CSE's boot partition's
(CSE RO and CSE RW) start and size details. This change make
independent of FMD file declaration with respect to CSE RO and CSE RW.
TEST=Build and verify the CSE RO and CSE RW region device information
through code instrumentation. Also, did boot test on Kano system.
Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Change-Id: Ie9a83b77ab44ea6ffe5bb20673e109a89a148629
---
M src/soc/intel/common/block/cse/cse_lite.c
1 file changed, 33 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/63169/6
--
To view, visit https://review.coreboot.org/c/coreboot/+/63169
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie9a83b77ab44ea6ffe5bb20673e109a89a148629
Gerrit-Change-Number: 63169
Gerrit-PatchSet: 6
Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Kane Chen <kane.chen(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Kane Chen <kane.chen(a)intel.com>
Gerrit-MessageType: newpatchset