coreboot-gerrit April 2021

coreboot-gerrit@coreboot.org

1 participants
6269 discussions

Change in coreboot[master]: mem_pool: Track the last two allocations (not just one)
by Patrick Georgi (Code Review) 06 Apr '21

06 Apr '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52087 ) Change subject: mem_pool: Track the last two allocations (not just one) ...................................................................... mem_pool: Track the last two allocations (not just one) This patch changes the mem_pool implementation to track the last two allocations (instead of just the last) and allow them both to be freed if the mem_pool_free() calls come in in reverse order. This is intended as a specific optimization for the CBFS cache case when a compressed file is mapped on a platform that doesn't natively support memory-mapping flash. In this case, cbfs_map() (chaining through to _cbfs_alloc() with allocator == NULL) will call mem_pool_alloc(&cbfs_cache) to allocate space for the uncompressed file data. It will then call cbfs_load_and_decompress() to fill that allocation, which will notice the compression and in turn call rdev_mmap_full() to map the compressed data (which on platforms without memory-mapped flash usually results in a second call to mem_pool_alloc(&cbfs_cache)). It then runs the decompression algorithm and calls rdev_munmap() on the compressed data buffer (the latter one in the allocation sequence), leading to a mem_pool_free(). The remaining buffer with the uncompressed data is returned out of cbfs_map() to the caller, which should eventually call cbfs_unmap() to mem_pool_free() that as well. This patch allows this simple case to succeed without leaking any permanent allocations on the cache. (More complicated cases where the caller maps other files before cbfs_unmap()ing the first one may still lead to leaks, but those are very rare in practice.) Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: Ic5c4c56a8482752ed65e10cf35565f9b2d3e4b17 Reviewed-on: https://review.coreboot.org/c/coreboot/+/52087 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Aaron Durbin <adurbin(a)chromium.org> --- M src/commonlib/include/commonlib/mem_pool.h M src/commonlib/mem_pool.c 2 files changed, 20 insertions(+), 12 deletions(-) Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved diff --git a/src/commonlib/include/commonlib/mem_pool.h b/src/commonlib/include/commonlib/mem_pool.h index bde9e41..6c85397 100644 --- a/src/commonlib/include/commonlib/mem_pool.h +++ b/src/commonlib/include/commonlib/mem_pool.h @@ -7,11 +7,14 @@ #include <stdint.h> /* - * The memory pool allows one to allocate memory from a fixed size buffer - * that also allows freeing semantics for reuse. However, the current - * limitation is that the most recent allocation is the only one that - * can be freed. If one tries to free any allocation that isn't the - * most recently allocated it will result in a leak within the memory pool. + * The memory pool allows one to allocate memory from a fixed size buffer that + * also allows freeing semantics for reuse. However, the current limitation is + * that only the two most recent allocations can be freed (in exact reverse + * order). If one tries to free any allocation that isn't at the top of the + * allocation stack, or one allocates more than two buffers in a row without + * freeing, it will result in a leak within the memory pool. (Two allocations + * were chosen to optimize for the CBFS cache case which may need two buffers + * to map a single compressed file, and will free them in reverse order.) * * The memory returned by allocations are at least 8 byte aligned. Note * that this requires the backing buffer to start on at least an 8 byte @@ -22,20 +25,23 @@ uint8_t *buf; size_t size; uint8_t *last_alloc; + uint8_t *second_to_last_alloc; size_t free_offset; }; -#define MEM_POOL_INIT(buf_, size_) \ - { \ - .buf = (buf_), \ - .size = (size_), \ - .last_alloc = NULL, \ - .free_offset = 0, \ +#define MEM_POOL_INIT(buf_, size_) \ + { \ + .buf = (buf_), \ + .size = (size_), \ + .last_alloc = NULL, \ + .second_to_last_alloc = NULL, \ + .free_offset = 0, \ } static inline void mem_pool_reset(struct mem_pool *mp) { mp->last_alloc = NULL; + mp->second_to_last_alloc = NULL; mp->free_offset = 0; } diff --git a/src/commonlib/mem_pool.c b/src/commonlib/mem_pool.c index 2ad1099..c300c65 100644 --- a/src/commonlib/mem_pool.c +++ b/src/commonlib/mem_pool.c @@ -17,6 +17,7 @@ p = &mp->buf[mp->free_offset]; mp->free_offset += sz; + mp->second_to_last_alloc = mp->last_alloc; mp->last_alloc = p; return p; @@ -29,6 +30,7 @@ return; mp->free_offset = mp->last_alloc - mp->buf; + mp->last_alloc = mp->second_to_last_alloc; /* No way to track allocation before this one. */ - mp->last_alloc = NULL; + mp->second_to_last_alloc = NULL; } -- To view, visit https://review.coreboot.org/c/coreboot/+/52087 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic5c4c56a8482752ed65e10cf35565f9b2d3e4b17 Gerrit-Change-Number: 52087 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: verstage: Add debug print when returning from verstage
by Patrick Georgi (Code Review) 06 Apr '21

06 Apr '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52086 ) Change subject: verstage: Add debug print when returning from verstage ...................................................................... verstage: Add debug print when returning from verstage RETURN_FROM_VERSTAGE is a somewhat tricky construct that we don't normally do otherwise in coreboot. While it works remarkably well in general, new development can lead to unintentional interactions with confusing results. This patch adds a debug print to the verstage right before returning to the bootblock so that it's obvious this happens, because otherwise in some cases the last printout in the verstage is about some TPM commands which can be misleading when execution hangs after that point. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I9ca68a32d7a50c95d9a6948d35816fee583611bc Reviewed-on: https://review.coreboot.org/c/coreboot/+/52086 Reviewed-by: Furquan Shaikh <furquan(a)google.com> Reviewed-by: Aaron Durbin <adurbin(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/security/vboot/verstage.c 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Furquan Shaikh: Looks good to me, approved diff --git a/src/security/vboot/verstage.c b/src/security/vboot/verstage.c index d2a9705..d785e0e 100644 --- a/src/security/vboot/verstage.c +++ b/src/security/vboot/verstage.c @@ -19,6 +19,7 @@ if (CONFIG(VBOOT_RETURN_FROM_VERSTAGE)) { verstage_main(); + printk(BIOS_DEBUG, "VBOOT: Returning from verstage.\n"); } else { run_romstage(); hlt(); -- To view, visit https://review.coreboot.org/c/coreboot/+/52086 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9ca68a32d7a50c95d9a6948d35816fee583611bc Gerrit-Change-Number: 52086 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: decompressor: Add CBFS_VERIFICATION support
by Patrick Georgi (Code Review) 06 Apr '21

06 Apr '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52085 ) Change subject: decompressor: Add CBFS_VERIFICATION support ...................................................................... decompressor: Add CBFS_VERIFICATION support CBFS_VERIFICATION requires the CBFS metadata hash anchor to be linked into an uncompressed stage, but for platforms using COMPRESS_BOOTBLOCK, this is only the decompressor stage. The first CBFS accesses are made in the bootblock stage after decompression, so if we want to make CBFS_VERIFICATION work on those platforms, we have to pass the metadata hash anchor from the decompressor into the bootblock. This patch does just that. (Note that this relies on the decompressor data remaining valid in memory for as long as the metadata hash anchor is needed. This is always true even for OVERLAP_DECOMPRESSOR_ROMSTAGE() situations because the FMAP and CBFS metadata necessarily need to have finished verification before a new stage could be loaded.) Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I2e6d7384cfb8339a24369eb6c01fc12f911c974e Reviewed-on: https://review.coreboot.org/c/coreboot/+/52085 Reviewed-by: Aaron Durbin <adurbin(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/include/bootblock_common.h M src/include/metadata_hash.h M src/lib/Kconfig.cbfs_verification M src/lib/Makefile.inc M src/lib/bootblock.c M src/lib/decompressor.c M src/lib/metadata_hash.c 7 files changed, 42 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved diff --git a/src/include/bootblock_common.h b/src/include/bootblock_common.h index da627d2..fccd235 100644 --- a/src/include/bootblock_common.h +++ b/src/include/bootblock_common.h @@ -35,6 +35,7 @@ /* This is the argument structure passed from decompressor to bootblock. */ struct bootblock_arg { uint64_t base_timestamp; + void *metadata_hash_anchor; uint32_t num_timestamps; struct timestamp_entry timestamps[]; }; diff --git a/src/include/metadata_hash.h b/src/include/metadata_hash.h index 2d3b8a8..bfa7ef1 100644 --- a/src/include/metadata_hash.h +++ b/src/include/metadata_hash.h @@ -6,6 +6,11 @@ #include <commonlib/bsd/metadata_hash.h> +/* Return a pointer to the whole anchor. Only used for decompressor builds. */ +void *metadata_hash_export_anchor(void); +/* Import a pointer that points to the anchor. Only used for decompressor builds. */ +void metadata_hash_import_anchor(void *ptr); + /* Verify the an FMAP data structure with the FMAP hash that is stored together with the CBFS metadata hash in the bootblock's metadata hash anchor (when CBFS verification is enabled). */ vb2_error_t metadata_hash_verify_fmap(const void *fmap_base, size_t fmap_size); diff --git a/src/lib/Kconfig.cbfs_verification b/src/lib/Kconfig.cbfs_verification index a28df1f..fa90d9d 100644 --- a/src/lib/Kconfig.cbfs_verification +++ b/src/lib/Kconfig.cbfs_verification @@ -6,7 +6,6 @@ config CBFS_VERIFICATION bool # TODO: make user selectable once it works - depends on !COMPRESS_BOOTBLOCK # TODO: figure out decompressor anchor depends on !VBOOT_STARTS_BEFORE_BOOTBLOCK # this is gonna get tricky... select VBOOT_LIB help diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc index 074cb2a..358035d 100644 --- a/src/lib/Makefile.inc +++ b/src/lib/Makefile.inc @@ -38,6 +38,7 @@ decompressor-$(CONFIG_GENERIC_GPIO_LIB) += gpio.c decompressor-y += memchr.c decompressor-y += memcmp.c +decompressor-$(CONFIG_CBFS_VERIFICATION) += metadata_hash.c decompressor-y += prog_ops.c decompressor-$(CONFIG_COLLECT_TIMESTAMPS) += timestamp.c diff --git a/src/lib/bootblock.c b/src/lib/bootblock.c index 1509c8c..23fb392 100644 --- a/src/lib/bootblock.c +++ b/src/lib/bootblock.c @@ -4,6 +4,7 @@ #include <bootblock_common.h> #include <console/console.h> #include <delay.h> +#include <metadata_hash.h> #include <option.h> #include <post.h> #include <program_loading.h> @@ -88,6 +89,8 @@ void _start(struct bootblock_arg *arg); void _start(struct bootblock_arg *arg) { + if (CONFIG(CBFS_VERIFICATION)) + metadata_hash_import_anchor(arg->metadata_hash_anchor); bootblock_main_with_timestamp(arg->base_timestamp, arg->timestamps, arg->num_timestamps); } diff --git a/src/lib/decompressor.c b/src/lib/decompressor.c index 8ae9358..1d160e0 100644 --- a/src/lib/decompressor.c +++ b/src/lib/decompressor.c @@ -3,6 +3,7 @@ #include <bootblock_common.h> #include <commonlib/bsd/compression.h> #include <delay.h> +#include <metadata_hash.h> #include <program_loading.h> #include <symbols.h> #include <timestamp.h> @@ -42,6 +43,9 @@ if (CONFIG(COLLECT_TIMESTAMPS)) arg.base_timestamp = timestamp_get(); + if (CONFIG(CBFS_VERIFICATION)) + arg.metadata_hash_anchor = metadata_hash_export_anchor(); + decompressor_soc_init(); if (CONFIG(COLLECT_TIMESTAMPS)) diff --git a/src/lib/metadata_hash.c b/src/lib/metadata_hash.c index a823c5f..5619efe 100644 --- a/src/lib/metadata_hash.c +++ b/src/lib/metadata_hash.c @@ -5,6 +5,7 @@ #include <metadata_hash.h> #include <symbols.h> +#if !CONFIG(COMPRESS_BOOTBLOCK) || ENV_DECOMPRESSOR __attribute__((used, section(".metadata_hash_anchor"))) static struct metadata_hash_anchor metadata_hash_anchor = { /* This is the only place in all of coreboot where we actually need to use this. */ @@ -12,15 +13,39 @@ .cbfs_hash = { .algo = CONFIG_CBFS_HASH_ALGO } }; +static struct metadata_hash_anchor *get_anchor(void) +{ + return &metadata_hash_anchor; +} + +void *metadata_hash_export_anchor(void) +{ + return get_anchor(); +} +#else +static struct metadata_hash_anchor *anchor_ptr = NULL; + +static struct metadata_hash_anchor *get_anchor(void) +{ + assert(anchor_ptr != NULL); + return anchor_ptr; +} + +void metadata_hash_import_anchor(void *ptr) +{ + anchor_ptr = ptr; +} +#endif + struct vb2_hash *metadata_hash_get(void) { - return &metadata_hash_anchor.cbfs_hash; + return &get_anchor()->cbfs_hash; } vb2_error_t metadata_hash_verify_fmap(const void *fmap_buffer, size_t fmap_size) { - struct vb2_hash hash = { .algo = metadata_hash_anchor.cbfs_hash.algo }; - memcpy(hash.raw, metadata_hash_anchor_fmap_hash(&metadata_hash_anchor), + struct vb2_hash hash = { .algo = get_anchor()->cbfs_hash.algo }; + memcpy(hash.raw, metadata_hash_anchor_fmap_hash(get_anchor()), vb2_digest_size(hash.algo)); return vb2_hash_verify(fmap_buffer, fmap_size, &hash); } -- To view, visit https://review.coreboot.org/c/coreboot/+/52085 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2e6d7384cfb8339a24369eb6c01fc12f911c974e Gerrit-Change-Number: 52085 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: cbfs: Add file data hashing for CONFIG_CBFS_VERIFICATION
by Patrick Georgi (Code Review) 06 Apr '21

06 Apr '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52084 ) Change subject: cbfs: Add file data hashing for CONFIG_CBFS_VERIFICATION ...................................................................... cbfs: Add file data hashing for CONFIG_CBFS_VERIFICATION This patch adds file data hashing for CONFIG_CBFS_VERIFICATION. With this, all CBFS accesses using the new CBFS APIs (cbfs_load/_map/_alloc and variants) will be fully verified when verification is enabled. (Note that some use of legacy APIs remains and thus the CBFS_VERIFICATION feature is not fully finished.) Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: Ic9fff279f69cf3b7c38a0dc2ff3c970eaa756aa8 Reviewed-on: https://review.coreboot.org/c/coreboot/+/52084 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Aaron Durbin <adurbin(a)chromium.org> --- M src/commonlib/bsd/cbfs_private.c M src/commonlib/bsd/include/commonlib/bsd/cbfs_mdata.h M src/lib/cbfs.c 3 files changed, 74 insertions(+), 13 deletions(-) Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved diff --git a/src/commonlib/bsd/cbfs_private.c b/src/commonlib/bsd/cbfs_private.c index 91f81e0..16bab7c 100644 --- a/src/commonlib/bsd/cbfs_private.c +++ b/src/commonlib/bsd/cbfs_private.c @@ -190,3 +190,26 @@ return NULL; } + +const struct vb2_hash *cbfs_file_hash(const union cbfs_mdata *mdata) +{ + /* Hashes are variable-length attributes, so need to manually check the length. */ + const struct cbfs_file_attr_hash *attr = + cbfs_find_attr(mdata, CBFS_FILE_ATTR_TAG_HASH, 0); + if (!attr) + return NULL; /* no hash */ + const size_t asize = be32toh(attr->len); + + const struct vb2_hash *hash = &attr->hash; + const size_t hsize = vb2_digest_size(hash->algo); + if (!hsize) { + ERROR("Hash algo %u for '%s' unsupported.\n", hash->algo, mdata->h.filename); + return NULL; + } + if (hsize != asize - offsetof(struct cbfs_file_attr_hash, hash.raw)) { + ERROR("Hash attribute size for '%s' (%zu) incorrect for algo %u.\n", + mdata->h.filename, asize, hash->algo); + return NULL; + } + return hash; +} diff --git a/src/commonlib/bsd/include/commonlib/bsd/cbfs_mdata.h b/src/commonlib/bsd/include/commonlib/bsd/cbfs_mdata.h index df13427..ed5c378 100644 --- a/src/commonlib/bsd/include/commonlib/bsd/cbfs_mdata.h +++ b/src/commonlib/bsd/include/commonlib/bsd/cbfs_mdata.h @@ -24,4 +24,7 @@ else caller is responsible for checking the |len| field to avoid reading out-of-bounds. */ const void *cbfs_find_attr(const union cbfs_mdata *mdata, uint32_t attr_tag, size_t size_check); +/* Returns pointer to CBFS file hash structure in metadata attributes, or NULL if invalid. */ +const struct vb2_hash *cbfs_file_hash(const union cbfs_mdata *mdata); + #endif /* _COMMONLIB_BSD_CBFS_MDATA_H_ */ diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index 9135b2f..23e8f41 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -186,11 +186,27 @@ return true; } -static size_t cbfs_load_and_decompress(const struct region_device *rdev, - void *buffer, size_t buffer_size, uint32_t compression) +static inline bool cbfs_file_hash_mismatch(const void *buffer, size_t size, + const struct vb2_hash *file_hash) +{ + /* Avoid linking hash functions when verification is disabled. */ + if (!CONFIG(CBFS_VERIFICATION)) + return false; + + /* If there is no file hash, always count that as a mismatch. */ + if (file_hash && vb2_hash_verify(buffer, size, file_hash) == VB2_SUCCESS) + return false; + + printk(BIOS_CRIT, "CBFS file hash mismatch!\n"); + return true; +} + +static size_t cbfs_load_and_decompress(const struct region_device *rdev, void *buffer, + size_t buffer_size, uint32_t compression, + const struct vb2_hash *file_hash) { size_t in_size = region_device_sz(rdev); - size_t out_size; + size_t out_size = 0; void *map; DEBUG("Decompressing %zu bytes to %p with algo %d\n", in_size, buffer, compression); @@ -201,6 +217,8 @@ return 0; if (rdev_readat(rdev, buffer, 0, in_size) != in_size) return 0; + if (cbfs_file_hash_mismatch(buffer, in_size, file_hash)) + return 0; return in_size; case CBFS_COMPRESS_LZ4: @@ -213,9 +231,11 @@ if (map == NULL) return 0; - timestamp_add_now(TS_START_ULZ4F); - out_size = ulz4fn(map, in_size, buffer, buffer_size); - timestamp_add_now(TS_END_ULZ4F); + if (!cbfs_file_hash_mismatch(map, in_size, file_hash)) { + timestamp_add_now(TS_START_ULZ4F); + out_size = ulz4fn(map, in_size, buffer, buffer_size); + timestamp_add_now(TS_END_ULZ4F); + } rdev_munmap(rdev, map); @@ -228,10 +248,12 @@ if (map == NULL) return 0; - /* Note: timestamp not useful for memory-mapped media (x86) */ - timestamp_add_now(TS_START_ULZMA); - out_size = ulzman(map, in_size, buffer, buffer_size); - timestamp_add_now(TS_END_ULZMA); + if (!cbfs_file_hash_mismatch(map, in_size, file_hash)) { + /* Note: timestamp not useful for memory-mapped media (x86) */ + timestamp_add_now(TS_START_ULZMA); + out_size = ulzman(map, in_size, buffer, buffer_size); + timestamp_add_now(TS_END_ULZMA); + } rdev_munmap(rdev, map); @@ -318,11 +340,18 @@ if (size_out) *size_out = size; + const struct vb2_hash *file_hash = NULL; + if (CONFIG(CBFS_VERIFICATION)) + file_hash = cbfs_file_hash(&mdata); + /* allocator == NULL means do a cbfs_map() */ if (allocator) { loc = allocator(arg, size, &mdata); } else if (compression == CBFS_COMPRESS_NONE) { - return rdev_mmap_full(&rdev); + void *mapping = rdev_mmap_full(&rdev); + if (!mapping || cbfs_file_hash_mismatch(mapping, size, file_hash)) + return NULL; + return mapping; } else if (!CBFS_CACHE_AVAILABLE) { ERROR("Cannot map compressed file %s on x86\n", mdata.h.filename); return NULL; @@ -335,7 +364,7 @@ return NULL; } - size = cbfs_load_and_decompress(&rdev, loc, size, compression); + size = cbfs_load_and_decompress(&rdev, loc, size, compression, file_hash); if (!size) return NULL; @@ -384,12 +413,18 @@ prog_set_entry(pstage, prog_start(pstage) + be32toh(sattr->entry_offset), NULL); + const struct vb2_hash *file_hash = NULL; + if (CONFIG(CBFS_VERIFICATION)) + file_hash = cbfs_file_hash(&mdata); + /* Hacky way to not load programs over read only media. The stages * that would hit this path initialize themselves. */ if ((ENV_BOOTBLOCK || ENV_SEPARATE_VERSTAGE) && !CONFIG(NO_XIP_EARLY_STAGES) && CONFIG(BOOT_DEVICE_MEMORY_MAPPED)) { void *mapping = rdev_mmap_full(&rdev); rdev_munmap(&rdev, mapping); + if (cbfs_file_hash_mismatch(mapping, region_device_sz(&rdev), file_hash)) + return CB_CBFS_HASH_MISMATCH; if (mapping == prog_start(pstage)) return CB_SUCCESS; } @@ -405,7 +440,7 @@ } size_t fsize = cbfs_load_and_decompress(&rdev, prog_start(pstage), prog_size(pstage), - compression); + compression, file_hash); if (!fsize) return CB_ERR; -- To view, visit https://review.coreboot.org/c/coreboot/+/52084 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic9fff279f69cf3b7c38a0dc2ff3c970eaa756aa8 Gerrit-Change-Number: 52084 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: cbfs: Simplify cbfs_load_and_decompress() and stop exporting it
by Patrick Georgi (Code Review) 06 Apr '21

06 Apr '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52083 ) Change subject: cbfs: Simplify cbfs_load_and_decompress() and stop exporting it ...................................................................... cbfs: Simplify cbfs_load_and_decompress() and stop exporting it With the last external user to cbfs_load_and_decompress() gone, we can stop exporting this function to the rest of coreboot and make it local to cbfs.c. Also remove a couple of arguments that no longer really make a difference and fold the stage-specific code for in-place LZ4 decompression into cbfs_prog_stage_load(). Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I4b459650a28e020c4342a66090f55264fbd26363 Reviewed-on: https://review.coreboot.org/c/coreboot/+/52083 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Aaron Durbin <adurbin(a)chromium.org> Reviewed-by: Furquan Shaikh <furquan(a)google.com> --- M src/include/cbfs.h M src/lib/cbfs.c 2 files changed, 21 insertions(+), 45 deletions(-) Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Furquan Shaikh: Looks good to me, approved diff --git a/src/include/cbfs.h b/src/include/cbfs.h index 37bac30..9ed5233 100644 --- a/src/include/cbfs.h +++ b/src/include/cbfs.h @@ -165,13 +165,6 @@ /* Return mapping of option ROM with revision number. Returns NULL on error. */ void *cbfs_boot_map_optionrom_revision(uint16_t vendor, uint16_t device, uint8_t rev); -/* Load |in_size| bytes from |rdev| at |offset| to the |buffer_size| bytes large |buffer|, - decompressing it according to |compression| in the process. Returns the decompressed file - size, or 0 on error. LZMA files will be mapped for decompression. LZ4 files will be - decompressed in-place with the buffer size requirements outlined in compression.h. */ -size_t cbfs_load_and_decompress(const struct region_device *rdev, size_t offset, - size_t in_size, void *buffer, size_t buffer_size, uint32_t compression); - /********************************************************************************************** * INTERNAL HELPERS FOR INLINES, DO NOT USE. * diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index 65bb721..9135b2f 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -186,19 +186,20 @@ return true; } -size_t cbfs_load_and_decompress(const struct region_device *rdev, size_t offset, size_t in_size, - void *buffer, size_t buffer_size, uint32_t compression) +static size_t cbfs_load_and_decompress(const struct region_device *rdev, + void *buffer, size_t buffer_size, uint32_t compression) { + size_t in_size = region_device_sz(rdev); size_t out_size; void *map; - DEBUG("Decompressing %zu bytes to %p with algo %d\n", buffer_size, buffer, compression); + DEBUG("Decompressing %zu bytes to %p with algo %d\n", in_size, buffer, compression); switch (compression) { case CBFS_COMPRESS_NONE: if (buffer_size < in_size) return 0; - if (rdev_readat(rdev, buffer, offset, in_size) != in_size) + if (rdev_readat(rdev, buffer, 0, in_size) != in_size) return 0; return in_size; @@ -206,9 +207,9 @@ if (!cbfs_lz4_enabled()) return 0; - /* cbfs_stage_load_and_decompress() takes care of in-place LZ4 decompression by + /* cbfs_prog_stage_load() takes care of in-place LZ4 decompression by setting up the rdev to be in memory. */ - map = rdev_mmap(rdev, offset, in_size); + map = rdev_mmap_full(rdev); if (map == NULL) return 0; @@ -223,7 +224,7 @@ case CBFS_COMPRESS_LZMA: if (!cbfs_lzma_enabled()) return 0; - map = rdev_mmap(rdev, offset, in_size); + map = rdev_mmap_full(rdev); if (map == NULL) return 0; @@ -241,33 +242,6 @@ } } -static size_t cbfs_stage_load_and_decompress(const struct region_device *rdev, size_t offset, - size_t in_size, void *buffer, size_t buffer_size, uint32_t compression) -{ - struct region_device rdev_src; - - if (compression == CBFS_COMPRESS_LZ4) { - if (!cbfs_lz4_enabled()) - return 0; - /* Load the compressed image to the end of the available memory area for - in-place decompression. It is the responsibility of the caller to ensure that - buffer_size is large enough (see compression.h, guaranteed by cbfstool for - stages). */ - void *compr_start = buffer + buffer_size - in_size; - if (rdev_readat(rdev, compr_start, offset, in_size) != in_size) - return 0; - /* Create a region device backed by memory. */ - rdev_chain(&rdev_src, &addrspace_32bit.rdev, (uintptr_t)compr_start, in_size); - - return cbfs_load_and_decompress(&rdev_src, 0, in_size, buffer, buffer_size, - compression); - } - - /* All other algorithms can use the generic implementation. */ - return cbfs_load_and_decompress(rdev, offset, in_size, buffer, buffer_size, - compression); -} - static inline int tohex4(unsigned int c) { return (c <= 9) ? (c + '0') : (c - 10 + 'a'); @@ -361,8 +335,7 @@ return NULL; } - size = cbfs_load_and_decompress(&rdev, 0, region_device_sz(&rdev), - loc, size, compression); + size = cbfs_load_and_decompress(&rdev, loc, size, compression); if (!size) return NULL; @@ -421,8 +394,18 @@ return CB_SUCCESS; } - size_t fsize = cbfs_stage_load_and_decompress(&rdev, 0, region_device_sz(&rdev), - prog_start(pstage), prog_size(pstage), compression); + /* LZ4 stages can be decompressed in-place to save mapping scratch space. Load the + compressed data to the end of the buffer and point &rdev to that memory location. */ + if (cbfs_lz4_enabled() && compression == CBFS_COMPRESS_LZ4) { + size_t in_size = region_device_sz(&rdev); + void *compr_start = prog_start(pstage) + prog_size(pstage) - in_size; + if (rdev_readat(&rdev, compr_start, 0, in_size) != in_size) + return CB_ERR; + rdev_chain(&rdev, &addrspace_32bit.rdev, (uintptr_t)compr_start, in_size); + } + + size_t fsize = cbfs_load_and_decompress(&rdev, prog_start(pstage), prog_size(pstage), + compression); if (!fsize) return CB_ERR; -- To view, visit https://review.coreboot.org/c/coreboot/+/52083 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4b459650a28e020c4342a66090f55264fbd26363 Gerrit-Change-Number: 52083 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: mb/intel/adlrvp: Update iDisp Link UPD settings
by Patrick Georgi (Code Review) 06 Apr '21

06 Apr '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/51353 ) Change subject: mb/intel/adlrvp: Update iDisp Link UPD settings ...................................................................... mb/intel/adlrvp: Update iDisp Link UPD settings This changes updates the iDisp-Link T-mode to 8T required for ADL-M. The update is made because the HW on ADL now supports 8T mode. BUG=None TEST= build and boot ADL-M RVP and verify HDMI/DP audio playback. Signed-off-by: Francois Toguo <francois.toguo.fotso(a)intel.com> Change-Id: I9d0bf7dc76348f7e184e8496f042badc30bf3211 Reviewed-on: https://review.coreboot.org/c/coreboot/+/51353 Reviewed-by: Furquan Shaikh <furquan(a)google.com> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/mainboard/intel/adlrvp/devicetree_m.cb 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve diff --git a/src/mainboard/intel/adlrvp/devicetree_m.cb b/src/mainboard/intel/adlrvp/devicetree_m.cb index 3094010..7c69163 100644 --- a/src/mainboard/intel/adlrvp/devicetree_m.cb +++ b/src/mainboard/intel/adlrvp/devicetree_m.cb @@ -130,7 +130,7 @@ register "PchHdaAudioLinkSndwEnable[0]" = "1" register "PchHdaAudioLinkSndwEnable[1]" = "1" # iDisp-Link T-Mode 0: 2T, 2: 4T, 3: 8T, 4: 16T - register "PchHdaIDispLinkTmode" = "2" + register "PchHdaIDispLinkTmode" = "3" # iDisp-Link Freq 4: 96MHz, 3: 48MHz. register "PchHdaIDispLinkFrequency" = "4" # Not disconnected/enumerable -- To view, visit https://review.coreboot.org/c/coreboot/+/51353 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9d0bf7dc76348f7e184e8496f042badc30bf3211 Gerrit-Change-Number: 51353 Gerrit-PatchSet: 12 Gerrit-Owner: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jairaj Arava <jairaj.arava(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Sathyanarayana Nujella <sathyanarayana.nujella(a)intel.com> Gerrit-Reviewer: Subrata Banik <subi.banik(a)gmail.com> Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: mb/intel/adlrvp: Enable Camera in ADL-M RVP
by Patrick Georgi (Code Review) 06 Apr '21

06 Apr '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/50265 ) Change subject: mb/intel/adlrvp: Enable Camera in ADL-M RVP ...................................................................... mb/intel/adlrvp: Enable Camera in ADL-M RVP 1. Configure Power Enable, Reset and Clock GPIO for both camera 2. Use same ASL code as ADL-P RVP Configure RST, PWR_EN and IMGCLKOUT signals for WFC and UFC TEST=Build, Boot and Verify streaming in both Camera Signed-off-by: Varshit Pandya <varshit.b.pandya(a)intel.com> Change-Id: I70636eaa8d9bdf23d649e811b3ff4f33b1bc604e Reviewed-on: https://review.coreboot.org/c/coreboot/+/50265 Reviewed-by: Ronak Kanabar <ronak.kanabar(a)intel.com> Reviewed-by: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/mainboard/intel/adlrvp/gpio_m.c 1 file changed, 14 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Rizwan Qureshi: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve Ronak Kanabar: Looks good to me, but someone else must approve diff --git a/src/mainboard/intel/adlrvp/gpio_m.c b/src/mainboard/intel/adlrvp/gpio_m.c index 9eef27f..3790847 100644 --- a/src/mainboard/intel/adlrvp/gpio_m.c +++ b/src/mainboard/intel/adlrvp/gpio_m.c @@ -11,14 +11,14 @@ /* H6 : I2C1 SDA */ PAD_CFG_NF(GPP_H6, NONE, DEEP, NF1), /* B16 : I2C5 SDA */ - PAD_CFG_NF(GPP_B16, NONE, DEEP, NF1), + PAD_CFG_NF(GPP_B16, NONE, DEEP, NF2), /* H5 : I2C0 SCL */ PAD_CFG_NF(GPP_H5, NONE, DEEP, NF1), /* H7 : I2C1 SCL */ PAD_CFG_NF(GPP_H7, NONE, DEEP, NF1), /* B17 : I2C5 SCL */ - PAD_CFG_NF(GPP_B17, NONE, DEEP, NF1), + PAD_CFG_NF(GPP_B17, NONE, DEEP, NF2), /* C5 : WWAN_PERST_N */ PAD_CFG_GPO(GPP_C5, 1, PLTRST), @@ -62,6 +62,18 @@ PAD_CFG_NF(GPP_D8, NONE, DEEP, NF1), PAD_CFG_NF(GPP_H19, NONE, DEEP, NF1), + /* CAM1_RST */ + PAD_CFG_GPO(GPP_R5, 1, PLTRST), + /* CAM2_RST */ + PAD_CFG_GPO(GPP_E15, 1, PLTRST), + /* CAM1_PWR_EN */ + PAD_CFG_GPO(GPP_B23, 1, PLTRST), + /* CAM2_PWR_EN */ + PAD_CFG_GPO(GPP_E16, 1, PLTRST), + /* IMGCLKOUT0 */ + PAD_CFG_NF(GPP_D4, NONE, DEEP, NF1), + /* IMGCLKOUT1 */ + PAD_CFG_NF(GPP_H20, NONE, DEEP, NF1), }; void variant_configure_gpio_pads(void) -- To view, visit https://review.coreboot.org/c/coreboot/+/50265 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I70636eaa8d9bdf23d649e811b3ff4f33b1bc604e Gerrit-Change-Number: 50265 Gerrit-PatchSet: 23 Gerrit-Owner: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-CC: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-CC: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com> Gerrit-CC: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-CC: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-CC: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-CC: Thejaswani Putta <thejaswani.putta(a)intel.com> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: mb/google/zork/vilboz: Update the ACPI name of ALC1015 AMP
by Frank Wu (Code Review) 06 Apr '21

06 Apr '21

Frank Wu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/52127 ) Change subject: mb/google/zork/vilboz: Update the ACPI name of ALC1015 AMP ...................................................................... mb/google/zork/vilboz: Update the ACPI name of ALC1015 AMP Update the ACPI name from AMDP1015 to 1002105 based on b/177971830#180. BUG=b:177971830 BRANCH=firmware-zork-13434.B TEST=emerge-zork coreboot chromeos-bootimage, then verify with ALC1015 AMP Signed-off-by: Frank Wu <frank_wu(a)compal.corp-partner.google.com> Change-Id: Id8f378ad6f3328d7db949ecdb609a2f16acd3884 --- M src/mainboard/google/zork/variants/vilboz/overridetree.cb 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/52127/1 diff --git a/src/mainboard/google/zork/variants/vilboz/overridetree.cb b/src/mainboard/google/zork/variants/vilboz/overridetree.cb index 3f5bfc3..93fe607 100644 --- a/src/mainboard/google/zork/variants/vilboz/overridetree.cb +++ b/src/mainboard/google/zork/variants/vilboz/overridetree.cb @@ -172,7 +172,7 @@ end end chip drivers/amd/i2s_machine_dev - register "hid" = ""AMDP1015"" + register "hid" = ""10021015"" # DMIC select GPIO for ACP machine device # This GPIO is used to select DMIC0 or DMIC1 by the # kernel driver. It does not really have a polarity -- To view, visit https://review.coreboot.org/c/coreboot/+/52127 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id8f378ad6f3328d7db949ecdb609a2f16acd3884 Gerrit-Change-Number: 52127 Gerrit-PatchSet: 1 Gerrit-Owner: Frank Wu <frank_wu(a)compal.corp-partner.google.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/intel/cannonlake: Drop unused `SataDevSlpRstConfig`
by Angel Pons (Code Review) 06 Apr '21

06 Apr '21

Attention is currently required from: Patrick Rudolph. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52103 ) Change subject: soc/intel/cannonlake: Drop unused `SataDevSlpRstConfig` ...................................................................... Patch Set 2: Code-Review-1 (1 comment) File src/soc/intel/cannonlake/chip.h: https://review.coreboot.org/c/coreboot/+/52103/comment/0ef42007_f56af429 PS2, Line 121: } SataDevSlpRstConfig; > grep pls Oh wait, the issue is that the enum fields are actually used for `SataPortsDevSlpResetConfig` -- To view, visit https://review.coreboot.org/c/coreboot/+/52103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iff3e896463575295c489032e04955bb5a7ba6355 Gerrit-Change-Number: 52103 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Tue, 06 Apr 2021 07:23:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/cannonlake: Drop unused `SataDevSlpRstConfig`
by Angel Pons (Code Review) 06 Apr '21

06 Apr '21

Attention is currently required from: Patrick Rudolph. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52103 ) Change subject: soc/intel/cannonlake: Drop unused `SataDevSlpRstConfig` ...................................................................... Patch Set 2: (1 comment) File src/soc/intel/cannonlake/chip.h: https://review.coreboot.org/c/coreboot/+/52103/comment/17d2f966_a8ea109c PS2, Line 121: } SataDevSlpRstConfig; grep pls -- To view, visit https://review.coreboot.org/c/coreboot/+/52103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iff3e896463575295c489032e04955bb5a7ba6355 Gerrit-Change-Number: 52103 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Tue, 06 Apr 2021 07:22:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2021