April 2022 - coreboot-gerrit - mail.coreboot.org

Change in coreboot[master]: soc/intel/apl: Write to cbfs regions using intermediate targets
by build bot (Jenkins) (Code Review) April 5, 2022

April 5, 2022

Attention is currently required from: Andrey Petrov. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63224 ) Change subject: soc/intel/apl: Write to cbfs regions using intermediate targets ...................................................................... Patch Set 8: Verified+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/63224 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie812084cc243a18cbc2913804ef2190dd9d6ed9b Gerrit-Change-Number: 63224 Gerrit-PatchSet: 8 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> 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: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Tue, 05 Apr 2022 20:22:20 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Makefile.inc: Add x86 bootblock as a separate target
by Julius Werner (Code Review) April 5, 2022

April 5, 2022

Attention is currently required from: Martin Roth, Marshall Dawson, Arthur Heymans, Kyösti Mälkki, Yu-Ping Wu, Felix Held. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56122 ) Change subject: Makefile.inc: Add x86 bootblock as a separate target ...................................................................... Patch Set 14: (3 comments) File src/arch/x86/Makefile.inc: https://review.coreboot.org/c/coreboot/+/56122/comment/e2bc5e60_e7692a80 PS9, Line 107: bootblock-options := $(TS_OPTIONS) $(TXTIBB) > > Can't we just put the -b here as well (maybe if you use = instead of := )? Or is there no way to m […] Thanks, I think that's cleaner. File src/arch/x86/Makefile.inc: https://review.coreboot.org/c/coreboot/+/56122/comment/397feb9a_03cf900a PS11, Line 103: ifneq ($(CONFIG_UPDATE_IMAGE),y) Shouldn't this also depend on TOP_ALIGNED_BOOTBLOCK? (And in that case maybe go in the arch-independent Makefile?) https://review.coreboot.org/c/coreboot/+/56122/comment/4aebb9b0_7cc5edc9 PS11, Line 106: -$(CBFSTOOL) $< remove -n bootblock 2>/dev/null Why are we removing something first? -- To view, visit https://review.coreboot.org/c/coreboot/+/56122 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4de9d7fedf1ae5a37a3310dd42eb07b44c030930 Gerrit-Change-Number: 56122 Gerrit-PatchSet: 14 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(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 20:22:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Kconfig: Add an option to skip adding a cbfs bootblock on x86
by Julius Werner (Code Review) April 5, 2022

April 5, 2022

Attention is currently required from: Arthur Heymans. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63377 ) Change subject: Kconfig: Add an option to skip adding a cbfs bootblock on x86 ...................................................................... Patch Set 4: (1 comment) File src/Kconfig: https://review.coreboot.org/c/coreboot/+/63377/comment/2767d593_62902e05 PS1, Line 1330: TOP_ALIGNED_CBFS_BOOTBLOCK I think something like BOOTBLOCK_IN_CBFS would be a more straight-forward way to describe this? -- To view, visit https://review.coreboot.org/c/coreboot/+/63377 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia42448f7e9dd0635c72857fbc1fab54508932721 Gerrit-Change-Number: 63377 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Tue, 05 Apr 2022 20:18:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: security/intel/cbnt/Makefile.inc: Improve build flow
by build bot (Jenkins) (Code Review) April 5, 2022

April 5, 2022

Attention is currently required from: Arthur Heymans. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56119 ) Change subject: security/intel/cbnt/Makefile.inc: Improve build flow ...................................................................... Patch Set 15: Verified+1 -- 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: 15 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-Comment-Date: Tue, 05 Apr 2022 20:17:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Makefile.inc: Generate master header and pointer as C structs
by Julius Werner (Code Review) April 5, 2022

April 5, 2022

Attention is currently required from: Arthur Heymans, Martin Roth, Yu-Ping Wu. Julius Werner 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 9: (6 comments) File src/arch/x86/bootblock.ld: https://review.coreboot.org/c/coreboot/+/59132/comment/eea6e6e2_8fc93454 PS7, Line 77: . = 0xfffffffc; : .header_pointer . : { : KEEP(*(.header_pointer)); : } > This needs to be smarter. It looks like the reset vector is wrong when header_pointer is missing. The file is probably 4 bytes too short in that case? I think maybe this could do what you want: . = 0xfffffffc; .header_pointer . : { KEEP(*(.header_pointer)); FILL(0xffffffff); . = 4; } Then again, why would header_pointer be missing? Don't we always have it? Then this shouldn't be a problem. edit: Okay, I guess that works too. File src/lib/cbfs_master_header.c: https://review.coreboot.org/c/coreboot/+/59132/comment/42d2b82a_992623f2 PS9, Line 7: __attribute((used)) Same problem here as the other one. https://review.coreboot.org/c/coreboot/+/59132/comment/927f5f73_c9c1bedf PS9, Line 10: /* The offset and romsize fields within the master header are absolute nit: https://doc.coreboot.org/contributing/coding_style.html#commenting https://review.coreboot.org/c/coreboot/+/59132/comment/6775785f_1eeeb8af PS9, Line 11: relfect typo https://review.coreboot.org/c/coreboot/+/59132/comment/3154cfba_cd6cae21 PS9, Line 16: FMAP_SECTION_COREBOOT_START + FMAP_SECTION_COREBOOT_SIZE Okay, I figured out how that memory-mapped offset thing in these headers works now (and it's honestly pretty horrible, we should try getting rid of that. It seems completely arbitrary which .fmd files define a FLASH@xxx and which don't. Most of our APIs that take a flash offset (e.g. boot_device_ro()) would not know how to deal with a memory-mapped offset like that. I think we only get away with that because we barely use these header constants anymore anyway). But then this line is a problem, because if it's using the memory-mapped offsets this would be a huge number. So you also need to subtract FMAP_SECTION_FLASH_START here again. File src/lib/master_header_pointer.c: https://review.coreboot.org/c/coreboot/+/59132/comment/07d89e25_5f819cfd PS7, Line 13: ATTRIBUTES uint32_t header_pointer = I think an easier way to write this is #if ENV_X86 __attribute__((__section__(".header_pointer"))) #endif uint32_t header_pointer = ...; -- 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: 9 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: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Tue, 05 Apr 2022 20:15:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/block/fast_spi/Makefile.inc: Improve cosmetics
by build bot (Jenkins) (Code Review) April 5, 2022

April 5, 2022

Attention is currently required from: Arthur Heymans, Patrick Rudolph. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56118 ) Change subject: soc/intel/common/block/fast_spi/Makefile.inc: Improve cosmetics ...................................................................... Patch Set 14: Verified+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/56118 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I41bbdabf7b846386651e64f4afb5b7b9fb38e1cb Gerrit-Change-Number: 56118 Gerrit-PatchSet: 14 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> 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-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Tue, 05 Apr 2022 20:14:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/*/Makefile.inc: Do some cosmetics
by build bot (Jenkins) (Code Review) April 5, 2022

April 5, 2022

Attention is currently required from: Jason Glenesk, Marshall Dawson, Fred Reitberger. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56117 ) Change subject: soc/amd/*/Makefile.inc: Do some cosmetics ...................................................................... Patch Set 14: Verified+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/56117 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iea2322ca1abd43900f3631b7965f07fed4235ca0 Gerrit-Change-Number: 56117 Gerrit-PatchSet: 14 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Comment-Date: Tue, 05 Apr 2022 20:08:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Kconfig: Add an option to skip adding a cbfs bootblock on x86
by build bot (Jenkins) (Code Review) April 5, 2022

April 5, 2022

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63377 ) Change subject: Kconfig: Add an option to skip adding a cbfs bootblock on x86 ...................................................................... Patch Set 2: Verified+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/63377 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia42448f7e9dd0635c72857fbc1fab54508932721 Gerrit-Change-Number: 63377 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 05 Apr 2022 20:05:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Makefile.inc: Add x86 bootblock as a separate target
by build bot (Jenkins) (Code Review) April 5, 2022

April 5, 2022

Attention is currently required from: Martin Roth, Marshall Dawson, Julius Werner, Kyösti Mälkki, Yu-Ping Wu, Felix Held. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56122 ) Change subject: Makefile.inc: Add x86 bootblock as a separate target ...................................................................... Patch Set 12: Verified+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/56122 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4de9d7fedf1ae5a37a3310dd42eb07b44c030930 Gerrit-Change-Number: 56122 Gerrit-PatchSet: 12 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(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 20:05:39 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cbfstool/cbfs_image.c: Drop compatibility for old images
by Arthur Heymans (Code Review) April 5, 2022

April 5, 2022

Attention is currently required from: Martin Roth, Patrick Georgi, Arthur Heymans. Hello build bot (Jenkins), Martin Roth, Patrick Georgi, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63306 to look at the new patch set (#6). Change subject: cbfstool/cbfs_image.c: Drop compatibility for old images ...................................................................... cbfstool/cbfs_image.c: Drop compatibility for old images Old images should use an old cbfstool. It's too much maintenance to keep cbfstool compatible with older images so drop code that dealt with images having a legacy cbfs. This also drops the possibility to add a cbfs master header as this is now handled by generating those files directly. Change-Id: I3c612c5d50948502609e5031b660f7519eabff0a Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M util/cbfstool/cbfs_image.c M util/cbfstool/cbfstool.c 2 files changed, 0 insertions(+), 160 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/63306/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/63306 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3c612c5d50948502609e5031b660f7519eabff0a Gerrit-Change-Number: 63306 Gerrit-PatchSet: 6 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0