Attention is currently required from: Martin Roth.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63350 )
Change subject: Makefile.inc: Add fmap_config.h as a dependency to cbfs-struct generation
......................................................................
Patch Set 2: Verified+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/63350
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7067ff144d38c1ff058825819419b2a2e7801e17
Gerrit-Change-Number: 63350
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(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: Martin Roth <martinroth(a)google.com>
Gerrit-Comment-Date: Tue, 05 Apr 2022 18:51:01 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63349 )
Change subject: mb/ti/beaglebone/board.fmd: USe 'FLASH' for device name
......................................................................
Patch Set 2: Verified+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/63349
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibba938c72d42ac74dcea8e8e6478ddae510d8c03
Gerrit-Change-Number: 63349
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
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 18:50:42 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Martin Roth, Julius Werner, Arthur Heymans, 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:
(6 comments)
File src/arch/x86/header_pointer.c:
https://review.coreboot.org/c/coreboot/+/59132/comment/72f2fab5_905315c2
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 […]
Done
File src/lib/cbfs_master_header.c:
https://review.coreboot.org/c/coreboot/+/59132/comment/52463f9e_46f4309c
PS6, Line 10: FMAP_SECTION_FLASH_SIZE
> I believe this needs to be […]
Done
https://review.coreboot.org/c/coreboot/+/59132/comment/61b0beab_9294b54c
PS6, Line 11: .bootblocksize = cpu_to_be32(4),
> This should probably have a comment about why this is 4... […]
Done
https://review.coreboot.org/c/coreboot/+/59132/comment/0d128e69_09ddfbbc
PS6, Line 12: .align = cpu_to_be32(64),
> You should be able to use the CBFS_ALIGNMENT constant here.
Done
https://review.coreboot.org/c/coreboot/+/59132/comment/55c16aac_b7c5caec
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 j […]
Done
File src/lib/master_header_pointer.c:
https://review.coreboot.org/c/coreboot/+/59132/comment/86b9c4be_234eeeb9
PS6, Line 7: __attribute((used))
> > I don't think you need this? We've never needed it for other structs... […]
Done
--
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: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Tue, 05 Apr 2022 18:50:27 +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
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/63378 )
Change subject: Kconfig: Add an option to ommit a cbfs master header
......................................................................
Kconfig: Add an option to ommit a cbfs master header
The CBFS master header is legacy feature that has no internal use in
coreboot. Some payloads do depend on it however.
Change-Id: Ib3d5b32412db40df48c0ed4dd6216bdd9cb955e2
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/Kconfig
M src/lib/Makefile.inc
2 files changed, 10 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/63378/1
diff --git a/src/Kconfig b/src/Kconfig
index eb1cac5..ae33e60 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -463,6 +463,14 @@
help
If this option is enabled, coreboot will scan only PCI devices
marked as mandatory in devicetree.cb
+
+config CBFS_MASTER_HEADER
+ bool "Include a legacy cbfs master header"
+ default y
+ help
+ Some payloads depend on this legacy interface. Payloads should use the active
+ region exposed in COREBOOT tables.
+
endmenu
menu "Mainboard"
diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc
index 71a9d93..299bc9d 100644
--- a/src/lib/Makefile.inc
+++ b/src/lib/Makefile.inc
@@ -394,13 +394,13 @@
romstage-$(CONFIG_SPD_CACHE_IN_FMAP) += spd_cache.c
-cbfs-files-y += cbfs_master_header
+cbfs-files-$(CONFIG_CBFS_MASTER_HEADER) += cbfs_master_header
cbfs_master_header-file := cbfs_master_header.c:struct
cbfs_master_header-type := "cbfs header"
cbfs_master_header-position := 0
ifneq ($(CONFIG_TOP_ALIGNED_CBFS_BOOTBLOCK),y)
-cbfs-files-y += header_pointer
+cbfs-files-$(CONFIG_CBFS_MASTER_HEADER) += header_pointer
header_pointer-file := master_header_pointer.c:struct
header_pointer-position := -4
header_pointer-type := "cbfs header"
--
To view, visit https://review.coreboot.org/c/coreboot/+/63378
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib3d5b32412db40df48c0ed4dd6216bdd9cb955e2
Gerrit-Change-Number: 63378
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newchange
Attention is currently required from: Martin Roth, Marshall Dawson, Julius Werner, Kyösti Mälkki, Yu-Ping Wu, Felix Held.
Hello build bot (Jenkins), Martin Roth, Marshall Dawson, Julius Werner, Yu-Ping Wu, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56122
to look at the new patch set (#11).
Change subject: Makefile.inc: Add x86 bootblock as a separate target
......................................................................
Makefile.inc: Add x86 bootblock as a separate target
Some platforms don't need a top aligned bootblock in cbfs like Intel
APL or modern AMD platforms as the bootblock is loaded differently.
So they don't need the top aligned cbfs bootblock.
To not clutter the main make file move out adding the bootblock.
Change-Id: I4de9d7fedf1ae5a37a3310dd42eb07b44c030930
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M Makefile.inc
M src/arch/x86/Makefile.inc
2 files changed, 9 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/56122/11
--
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: 11
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-MessageType: newpatchset
Attention is currently required from: Arthur Heymans, Martin Roth, Julius Werner, Yu-Ping Wu.
Arthur Heymans has uploaded a new patch set (#7) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/59132 )
Change subject: Makefile.inc: Generate master header and pointer as C structs
......................................................................
Makefile.inc: Generate master header and pointer as C structs
The makefiles don't like cbfs file names with spaces in them so update
the file name with '_' instead of spaces. To keep the master header at
the top of cbfs, add a placeholder.
This removes the need to handle the cbfs master header in cbfstool.
This functionality will be dropped in a later CL.
On x86 reserve some space in the linker script to add the pointer.
On non-x86 generate a pointer inside a C struct file.
As a bonus this would actually fix the master header pointer mechanism
on Intel/APL as only the bootblock inside IFWI gets memory mapped.
Change-Id: I3ba01be7da1f09a8cac287751497c18cda97d293
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M Makefile.inc
M src/arch/x86/bootblock.ld
M src/lib/Makefile.inc
A src/lib/cbfs_master_header.c
A src/lib/master_header_pointer.c
M src/security/vboot/Makefile.inc
6 files changed, 62 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/59132/7
--
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: 7
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-MessageType: newpatchset
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