coreboot-gerrit April 2022

coreboot-gerrit@coreboot.org

1 participants
5879 discussions

Change in coreboot[master]: soc/amd/*/Makefile.inc: Do some cosmetics
by build bot (Jenkins) (Code Review) 01 Apr '22

01 Apr '22

Attention is currently required from: Arthur Heymans, Jason Glenesk, Martin Roth, 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 10: 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: 10 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Comment-Date: Fri, 01 Apr 2022 20:22:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Makefile.inc: Add the x86 bootblock as a regular cbfs file
by build bot (Jenkins) (Code Review) 01 Apr '22

01 Apr '22

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 the x86 bootblock as a regular cbfs file ...................................................................... Patch Set 8: 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: 8 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: Fri, 01 Apr 2022 20:22:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cbfs_image.c: Drop compatibility for old images
by Arthur Heymans (Code Review) 01 Apr '22

01 Apr '22

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/63306 ) Change subject: cbfs_image.c: Drop compatibility for old images ...................................................................... cbfs_image.c: Drop compatibility for old images This has long been fixed. Old images should use an old cbfstool. 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/1 diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index 5f30877..a258a81 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -69,40 +69,6 @@ align_up(strlen(name) + 1, CBFS_ATTRIBUTE_ALIGN)); } -/* Only call on legacy CBFSes possessing a master header. */ -static int cbfs_fix_legacy_size(struct cbfs_image *image, char *hdr_loc) -{ - assert(image); - assert(cbfs_is_legacy_cbfs(image)); - // A bug in old cbfstool may produce extra few bytes (by alignment) and - // cause cbfstool to overwrite things after free space -- which is - // usually CBFS header on x86. We need to workaround that. - // Except when we run across a file that contains the actual header, - // in which case this image is a safe, new-style - // `cbfstool add-master-header` based image. - - struct cbfs_file *entry, *first = NULL, *last = NULL; - for (first = entry = cbfs_find_first_entry(image); - entry && cbfs_is_valid_entry(image, entry); - entry = cbfs_find_next_entry(image, entry)) { - /* Is the header guarded by a CBFS file entry? Then exit */ - if (((char *)entry) + be32toh(entry->offset) == hdr_loc) - return 0; - last = entry; - } - if ((char *)first < (char *)hdr_loc && - (char *)entry > (char *)hdr_loc) { - WARN("CBFS image was created with old cbfstool with size bug. " - "Fixing size in last entry...\n"); - last->len = htobe32(be32toh(last->len) - image->header.align); - DEBUG("Last entry has been changed from 0x%x to 0x%x.\n", - cbfs_get_entry_addr(image, entry), - cbfs_get_entry_addr(image, - cbfs_find_next_entry(image, last))); - } - return 0; -} - void cbfs_put_header(void *dest, const struct cbfs_header *header) { struct buffer outheader; @@ -344,7 +310,6 @@ if (header_loc) { cbfs_get_header(&out->header, header_loc); out->has_header = true; - cbfs_fix_legacy_size(out, header_loc); return 0; } else if (offset != ~0u) { ERROR("The -H switch is only valid on legacy images having CBFS master headers.\n"); diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index cebfef6..95e0bbf 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -642,127 +642,6 @@ return 1; } -static void fill_header_offset(void *location, uint32_t offset) -{ - // TODO: When we have a BE target, we'll need to store this as BE - write_le32(location, offset); -} - -static int update_master_header_loc_topswap(struct cbfs_image *image, - void *h_loc, uint32_t header_offset) -{ - struct cbfs_file *entry; - void *ts_h_loc = h_loc; - - entry = cbfs_get_entry(image, "bootblock"); - if (entry == NULL) { - ERROR("Bootblock not in ROM image?!?\n"); - return 1; - } - - /* - * Check if the existing topswap boundary matches with - * the one provided. - */ - if (param.topswap_size != be32toh(entry->len)/2) { - ERROR("Top swap boundary does not match\n"); - return 1; - } - - ts_h_loc -= param.topswap_size; - fill_header_offset(ts_h_loc, header_offset); - - return 0; -} - -static int cbfs_add_master_header(void) -{ - const char * const name = "cbfs master header"; - struct cbfs_image image; - struct cbfs_file *header = NULL; - struct buffer buffer; - int ret = 1; - size_t offset; - size_t size; - void *h_loc; - - if (cbfs_image_from_buffer(&image, param.image_region, - param.headeroffset)) { - ERROR("Selected image region is not a CBFS.\n"); - return 1; - } - - if (cbfs_get_entry(&image, name)) { - ERROR("'%s' already in ROM image.\n", name); - return 1; - } - - if (buffer_create(&buffer, sizeof(struct cbfs_header), name) != 0) - return 1; - - struct cbfs_header *h = (struct cbfs_header *)buffer.data; - h->magic = htobe32(CBFS_HEADER_MAGIC); - h->version = htobe32(CBFS_HEADER_VERSION); - /* The 4 bytes are left out for two reasons: - * 1. the cbfs master header pointer resides there - * 2. some cbfs implementations assume that an image that resides - * below 4GB has a bootblock and get confused when the end of the - * image is at 4GB == 0. - */ - h->bootblocksize = htobe32(4); - h->align = htobe32(CBFS_ALIGNMENT); - /* The offset and romsize fields within the master header are absolute - * values within the boot media. As such, romsize needs to relfect - * the end 'offset' for a CBFS. To achieve that the current buffer - * representing the CBFS region's size is added to the offset of - * the region within a larger image. - */ - offset = buffer_get(param.image_region) - - buffer_get_original_backing(param.image_region); - size = buffer_size(param.image_region); - h->romsize = htobe32(size + offset); - h->offset = htobe32(offset); - h->architecture = htobe32(CBFS_ARCHITECTURE_UNKNOWN); - - /* Never add a hash attribute to the master header. */ - header = cbfs_create_file_header(CBFS_TYPE_CBFSHEADER, - buffer_size(&buffer), name); - if (cbfs_add_entry(&image, &buffer, 0, header, 0) != 0) { - ERROR("Failed to add cbfs master header into ROM image.\n"); - goto done; - } - - struct cbfs_file *entry; - if ((entry = cbfs_get_entry(&image, name)) == NULL) { - ERROR("'%s' not in ROM image?!?\n", name); - goto done; - } - - uint32_t header_offset = CBFS_SUBHEADER(entry) - - buffer_get(&image.buffer); - header_offset = -(buffer_size(&image.buffer) - header_offset); - - h_loc = (void *)(buffer_get(&image.buffer) + - buffer_size(&image.buffer) - 4); - fill_header_offset(h_loc, header_offset); - /* - * If top swap present, update the header - * location in secondary bootblock - */ - if (param.topswap_size) { - if (update_master_header_loc_topswap(&image, h_loc, - header_offset)) - return 1; - } - - ret = maybe_update_metadata_hash(&image); - -done: - free(header); - buffer_delete(&buffer); - return ret; -} - static int add_topswap_bootblock(struct buffer *buffer, uint32_t *offset) { size_t bb_buf_size = buffer_size(buffer); @@ -1718,7 +1597,6 @@ {"add-stage", "a:H:r:f:n:t:c:b:P:QS:p:yvA:gh?", cbfs_add_stage, true, true}, {"add-int", "H:r:i:n:b:vgh?", cbfs_add_integer, true, true}, - {"add-master-header", "H:r:vh?j:", cbfs_add_master_header, true, true}, {"compact", "r:h?", cbfs_compact, true, true}, {"copy", "r:R:h?", cbfs_copy, true, true}, {"create", "M:r:s:B:b:H:o:m:vh?", cbfs_create, true, true}, @@ -1912,9 +1790,6 @@ "Add a 32bit flat mode binary\n" " add-int [-r image,regions] -i INTEGER -n NAME [-b base] " "Add a raw 64-bit integer value\n" - " add-master-header [-r image,regions] \\ \n" - " [-j topswap-size] (Intel CPUs only) " - "Add a legacy CBFS master header\n" " remove [-r image,regions] -n NAME " "Remove a component\n" " compact -r image,regions " -- 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: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: Makefile.inc: Move adding bootblock non-x86 targets
by Arthur Heymans (Code Review) 01 Apr '22

01 Apr '22

Attention is currently required from: Martin Roth. Hello build bot (Jenkins), Sean Rhodes, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63217 to look at the new patch set (#4). Change subject: Makefile.inc: Move adding bootblock non-x86 targets ...................................................................... Makefile.inc: Move adding bootblock non-x86 targets This can be done in a separate Makefile target. Change-Id: I50eae4f00d171d26a221ca969086f4f294fa524b Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M Makefile.inc 1 file changed, 6 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/63217/4 -- 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: 4 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-Attention: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: Makefile.inc: Generate master header and pointer as C structs
by Arthur Heymans (Code Review) 01 Apr '22

01 Apr '22

Attention is currently required from: Arthur Heymans, Martin Roth, Julius Werner, Yu-Ping Wu. Arthur Heymans has uploaded a new patch set (#5) 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. Change-Id: I3ba01be7da1f09a8cac287751497c18cda97d293 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M Makefile.inc M src/arch/x86/Makefile.inc M src/arch/x86/bootblock.ld A src/arch/x86/header_pointer.c 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 8 files changed, 51 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/59132/5 -- 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: 5 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

1 0

Change in coreboot[master]: [UNTESTED] soc/amd/common/block/spi/fch_spi_ctrl: Fix restricted comm...
by Fred Reitberger (Code Review) 01 Apr '22

01 Apr '22

Fred Reitberger has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/63305 ) Change subject: [UNTESTED] soc/amd/common/block/spi/fch_spi_ctrl: Fix restricted command write ...................................................................... [UNTESTED] soc/amd/common/block/spi/fch_spi_ctrl: Fix restricted command write The SPI_RESTRICTED_CMD register is not a PCI configuration register. It is memory mapped from the SPI bar. Verified against PPR 55570 rev 3.16, PPR 56569 rev 3.03, and PPR 57243 rev 1.50 Change-Id: I7c88aaea9ddac200644bb368be3bd4e9be47fd7b Signed-off-by: Fred Reitberger <reitbergerfred(a)gmail.com> --- M src/soc/amd/common/block/spi/fch_spi_ctrl.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/63305/1 diff --git a/src/soc/amd/common/block/spi/fch_spi_ctrl.c b/src/soc/amd/common/block/spi/fch_spi_ctrl.c index b2fe13d..f6cad55 100644 --- a/src/soc/amd/common/block/spi/fch_spi_ctrl.c +++ b/src/soc/amd/common/block/spi/fch_spi_ctrl.c @@ -270,7 +270,7 @@ } /* Final steps to protect region */ - pci_write_config32(SOC_LPC_DEV, SPI_RESTRICTED_CMD1, reg32); + spi_write32(SPI_RESTRICTED_CMD1, reg32); reg32 = spi_read32(SPI_CNTRL0); reg32 &= ~SPI_ACCESS_MAC_ROM_EN; spi_write32(SPI_CNTRL0, reg32); -- To view, visit https://review.coreboot.org/c/coreboot/+/63305 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7c88aaea9ddac200644bb368be3bd4e9be47fd7b Gerrit-Change-Number: 63305 Gerrit-PatchSet: 1 Gerrit-Owner: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/qualcomm/common: Add strict flag to clock_configure()
by Shelley Chen (Code Review) 01 Apr '22

01 Apr '22

Attention is currently required from: Julius Werner. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63289 ) Change subject: soc/qualcomm/common: Add strict flag to clock_configure() ...................................................................... Patch Set 4: (1 comment) File src/soc/qualcomm/sc7280/clock.c: https://review.coreboot.org/c/coreboot/+/63289/comment/f31d2f4c_8db6560d PS4, Line 311: false > Why can't these be checked? I think that I definitely wanted to check for the SPI frequency but didn't want to screw up the other checks in case they were expecting it to round up to the higher frequency if there wasn't an exact match. I actually wasn't sure what the other clocks were but was mainly focusing on the qspi clock frequency, which is why i defaulted everything else to false. Anyway, I can try enabling and making sure everything boots still at least. -- To view, visit https://review.coreboot.org/c/coreboot/+/63289 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9cfad7236241f4d03ff1a56683654649658b68fc Gerrit-Change-Number: 63289 Gerrit-PatchSet: 4 Gerrit-Owner: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Fri, 01 Apr 2022 20:07:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Makefile.inc: Update the master header pointer in a separate target
by Arthur Heymans (Code Review) 01 Apr '22

01 Apr '22

1 0

Change in coreboot[master]: Makefile.inc: Update the master header pointer in a separate target
by Arthur Heymans (Code Review) 01 Apr '22

01 Apr '22

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: Update the master header pointer in a separate target ...................................................................... Patch Set 4: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/59132/comment/92eee83c_068d892c PS4, Line 9: The makefiles don't like cbfs file names with spaces > I'm good with how it's done now, but honestly, this seems odd to me. We should be able to escape it and make it work. maybe '\\ ' so that after the 1st evaluation we get '\ ' presented to the shell? Thanks, I'll try that. Patchset: PS4: > It might be good to give some background on the cbfs master header in the commit message just so that people have a better understanding of what's going on without having to go search gerrit. (This is not meant as a criticism.) > > Maybe add some of the comments between you and Julius in CB:56120 > > It probably would have been better in the commit message of that revert, but that's too late now. It's just a little confusing going through commits and seeing the comments: > "# the cbfs master header is a deprecated feature only used on x86" and "If FMAP is used anyway there is no need for a cbfs master header.", but still seeing us using it on all platforms. > > Again, not a big deal, just some documentation of what's going on with the master header might be useful. > > Got it. btw I still plan on removing it in the end, but Libpayload needs to support FMAP for that. File Makefile.inc: https://review.coreboot.org/c/coreboot/+/59132/comment/8f207308_ab80de66 PS4, Line 1150: 32 > Nit: Maybe put some of these numbers into variables? Some comments as to what's going on here might be good too. > > # Add a pointer to the cbfs master header in the last 4 bytes of CBFS Now that I think of it. I can probably just create a c struct and directly add something useful here. That would allow to drop the whole add-master-header functionality in cbfstool. -- 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: 4 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: Fri, 01 Apr 2022 19:14:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Makefile.inc: Add the x86 bootblock as a regular cbfs file
by Arthur Heymans (Code Review) 01 Apr '22

01 Apr '22

Attention is currently required from: Martin Roth, Marshall Dawson, Julius Werner, Kyösti Mälkki, Yu-Ping Wu, Felix Held. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56122 ) Change subject: Makefile.inc: Add the x86 bootblock as a regular cbfs file ...................................................................... Patch Set 7: (1 comment) File Makefile.inc: https://review.coreboot.org/c/coreboot/+/56122/comment/f9f8ade9_a71c2d5e PS3, Line 840: This top aligns files of type bootblock, assuming that only x86 uses those > You're right, in the current state, the bootblock area is no longer needed on AMD Ryzen systems because we use the compressed version inside the AMD FW area. > > I believe that we could actually still use it if we wanted to just by pointing the "BIOS" area in EFS to our uncompressed bootblock region. > > As an aside, I'd actually really like to get away from the whole amdfwtool being used to create a bin binary. Except for the additional CBFS headers, I don't see any advantage to amdfwtool's method over just adding all of the various binaries separately in CBFS, then telling a new EFS generation tool where they are. I think that would allow for a lot more flexibility. Do you mean that you would prefer it to just execute the bootblock XIP at the end top of 4G? Currently we don't use the fact that the bootblock is in memory all that well but it seems to allow for some neat opportunities, that I'd like to explore like having only one stage. Are we though talking about the same thing? -- 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: 7 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: Fri, 01 Apr 2022 19:09:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2022