coreboot-gerrit April 2021

coreboot-gerrit@coreboot.org

1 participants
6269 discussions

Change in coreboot[master]: soc/intel: Remove SMM HECI disable related options
by Aseda Aboagye (Code Review) 08 Apr '21

08 Apr '21

Attention is currently required from: Martin Roth, Furquan Shaikh, Aaron Durbin, Karthik Ramasubramanian. Aseda Aboagye has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52147 ) Change subject: soc/intel: Remove SMM HECI disable related options ...................................................................... Patch Set 3: (2 comments) File src/soc/intel/cannonlake/smihandler.c: https://review.coreboot.org/c/coreboot/+/52147/comment/0dde57cc_6075abf5 PS2, Line 22: const struct device *dev = pcidev_path_on_root(PCH_DEVFN_CSE); : if (!is_dev_enabled(dev)) > The concerned PCI device (device pci 16.0) is also enabled/disabled in devicetree. […] Ack File src/soc/intel/common/block/cse/Makefile.inc: https://review.coreboot.org/c/coreboot/+/52147/comment/c5f3f098_afd7a55b PS3, Line 5: y > We may have to change it to $(CONFIG_SOC_INTEL_COMMON_BLOCK_CSE) There's still the port ID issue on some of the SoCs. Some of them don't define `PID_CSME0`. I assume the component is there, but it'll take same digging to determine what it ought to be. I think I'm going to restrict the changes to just dedede for now and then file a bug for the further cleanup. -- To view, visit https://review.coreboot.org/c/coreboot/+/52147 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iced84faaab8a0cca8400ccd3ec6e4cd525c30e92 Gerrit-Change-Number: 52147 Gerrit-PatchSet: 3 Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Fri, 09 Apr 2021 00:19:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aseda Aboagye <aaboagye(a)google.com> Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/common: Handle power resume after power failure
by Karthik Ramasubramanian (Code Review) 08 Apr '21

08 Apr '21

Attention is currently required from: Jason Glenesk, Martin Roth, Marshall Dawson, Karthik Ramasubramanian. Hello build bot (Jenkins), Jason Glenesk, Raul Rangel, Furquan Shaikh, Patrick Georgi, Martin Roth, Marshall Dawson, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/51924 to look at the new patch set (#7). Change subject: soc/amd/common: Handle power resume after power failure ...................................................................... soc/amd/common: Handle power resume after power failure Introduce a power management library to handle the power resume after power failure. Enable HAVE_POWER_STATE_AFTER_FAILURE config when this library is enabled. BUG=b:183739671 TEST=Build Guybrush and Majolica mainboard. Change-Id: Iea4ea57d747425fe6714d40ba6e60f2447febf28 Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> --- A src/soc/amd/common/block/include/amdblocks/pmlib.h A src/soc/amd/common/block/pm/Kconfig A src/soc/amd/common/block/pm/Makefile.inc A src/soc/amd/common/block/pm/pmlib.c 4 files changed, 77 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/51924/7 -- To view, visit https://review.coreboot.org/c/coreboot/+/51924 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iea4ea57d747425fe6714d40ba6e60f2447febf28 Gerrit-Change-Number: 51924 Gerrit-PatchSet: 7 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.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: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> 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: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: cbfs: mcache: Fix size calculation for perfectly full cache
by Julius Werner (Code Review) 08 Apr '21

08 Apr '21

Attention is currently required from: Aaron Durbin. Hello Aaron Durbin, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/52200 to review the following change. Change subject: cbfs: mcache: Fix size calculation for perfectly full cache ...................................................................... cbfs: mcache: Fix size calculation for perfectly full cache cbfs_mcache_real_size() has a subtle flaw: when the cache is perfectly full to the end (so that the termination token sits exactly at the end of the available space), the loop counting the size ends prematurely. This means that when migrating the cache to CBMEM the terminating token is not copied, which isn't actually noticeable unless you're looking for a file that's not in the cache (because it doesn't exist or because not all files fit when building). This patch fixes the problem and slightly changes the error message for when a cache isn't terminated (to make it more clear that this is a different condition from a "normal" cache overflow that can happen when building if there's not enough room to fit all files). Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I8d89e7dadc958f97b173b3a2352f2010c8a3d1d5 --- M src/commonlib/bsd/cbfs_mcache.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/52200/1 diff --git a/src/commonlib/bsd/cbfs_mcache.c b/src/commonlib/bsd/cbfs_mcache.c index 0d5d8e0..965f9fa 100644 --- a/src/commonlib/bsd/cbfs_mcache.c +++ b/src/commonlib/bsd/cbfs_mcache.c @@ -118,7 +118,7 @@ current += ALIGN_UP(data_offset, CBFS_MCACHE_ALIGNMENT); } - ERROR("CBFS mcache overflow!\n"); + ERROR("CBFS mcache is not terminated!\n"); /* should never happen */ return CB_ERR; } @@ -127,7 +127,7 @@ const void *end = mcache + mcache_size; const void *current = mcache; - while (current + sizeof(uint32_t) < end) { + while (current + sizeof(uint32_t) <= end) { const union mcache_entry *entry = current; if (entry->magic == MCACHE_MAGIC_FULL || entry->magic == MCACHE_MAGIC_END) { -- To view, visit https://review.coreboot.org/c/coreboot/+/52200 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8d89e7dadc958f97b173b3a2352f2010c8a3d1d5 Gerrit-Change-Number: 52200 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: prog: Enable CBFS file hashing for programs
by Julius Werner (Code Review) 08 Apr '21

08 Apr '21

Hello Furquan Shaikh, Aaron Durbin, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/48839 to review the following change. Change subject: prog: Enable CBFS file hashing for programs ...................................................................... prog: Enable CBFS file hashing for programs This patch enables CBFS file hashing for all programs (stages, payloads, etc.), except for some special cases with the Intel FSP that will need to be addressed in a later patch. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I3c6e8942e502035d365a193abf2cbe2f312e6561 --- M src/include/program_loading.h M src/lib/cbfs.c M src/lib/fit_payload.c M src/lib/prog_loaders.c M src/lib/rmodule.c M src/lib/selfboot.c 6 files changed, 54 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/48839/1 diff --git a/src/include/program_loading.h b/src/include/program_loading.h index 57efd7b..01b0159 100644 --- a/src/include/program_loading.h +++ b/src/include/program_loading.h @@ -7,6 +7,7 @@ #include <commonlib/bsd/cbfs_serialized.h> #include <commonlib/region.h> #include <types.h> +#include <vb2_sha.h> enum { /* Last segment of program. Can be used to take different actions for @@ -50,6 +51,7 @@ size_t size; /* Program size in memory (including BSS). */ void (*entry)(void *); /* Function pointer to entry point. */ void *arg; /* Optional argument (only valid for some archs). */ + struct vb2_hash hash; /* Content hash for CBFS_VERIFICATION. */ }; #define PROG_INIT(type_, name_) \ @@ -105,6 +107,11 @@ return prog->arg; } +static inline const struct vb2_hash *prog_hash(const struct prog *prog) +{ + return &prog->hash; +} + /* region_device representing the 32-bit flat address space. */ extern const struct mem_region_device addrspace_32bit; @@ -145,6 +152,13 @@ * value prohibits further progress */ int prog_locate_hook(struct prog *prog); +/* rdev_mmap() the program source (with automatic verification). */ +void *prog_mmap(struct prog *prog); +static inline void prog_munmap(struct prog *prog, void *mapping) +{ + rdev_munmap(prog_rdev(prog), mapping); +} + /* Run the program described by prog. */ void prog_run(struct prog *prog); /* Per architecture implementation running a program. */ diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index bfbb7bc..b145177 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -398,13 +398,15 @@ !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), + prog_hash(pstage))) + return -1; if (mapping == prog_start(pstage)) return 0; } - fsize = cbfs_stage_load_and_decompress(rdev, - prog_start(pstage), prog_size(pstage), - prog_compression(pstage), NULL); + fsize = cbfs_stage_load_and_decompress(rdev, prog_start(pstage), + prog_size(pstage), prog_compression(pstage), prog_hash(pstage)); if (!fsize) return -1; diff --git a/src/lib/fit_payload.c b/src/lib/fit_payload.c index b3a03a1..2321826 100644 --- a/src/lib/fit_payload.c +++ b/src/lib/fit_payload.c @@ -174,7 +174,7 @@ struct region kernel = {0}, fdt = {0}, initrd = {0}; void *data; - data = rdev_mmap_full(prog_rdev(payload)); + data = prog_mmap(payload); if (data == NULL) return; @@ -185,14 +185,14 @@ if (!config) { printk(BIOS_ERR, "ERROR: Could not load FIT\n"); - rdev_munmap(prog_rdev(payload), data); + prog_munmap(payload, data); return; } dt = unpack_fdt(config->fdt); if (!dt) { printk(BIOS_ERR, "ERROR: Failed to unflatten the FDT.\n"); - rdev_munmap(prog_rdev(payload), data); + prog_munmap(payload, data); return; } @@ -225,7 +225,7 @@ if (!fit_payload_arch(payload, config, &kernel, &fdt, &initrd)) { printk(BIOS_ERR, "ERROR: Failed to find free memory region\n"); bootmem_dump_ranges(); - rdev_munmap(prog_rdev(payload), data); + prog_munmap(payload, data); return; } @@ -240,7 +240,7 @@ extract(&initrd, config->ramdisk)) { printk(BIOS_ERR, "ERROR: Failed to extract initrd\n"); prog_set_entry(payload, NULL, NULL); - rdev_munmap(prog_rdev(payload), data); + prog_munmap(payload, data); return; } @@ -249,11 +249,11 @@ if (extract(&kernel, config->kernel)) { printk(BIOS_ERR, "ERROR: Failed to extract kernel\n"); prog_set_entry(payload, NULL, NULL); - rdev_munmap(prog_rdev(payload), data); + prog_munmap(payload, data); return; } timestamp_add_now(TS_START_KERNEL); - rdev_munmap(prog_rdev(payload), data); + prog_munmap(payload, data); } diff --git a/src/lib/prog_loaders.c b/src/lib/prog_loaders.c index f75f2ee..e29d335 100644 --- a/src/lib/prog_loaders.c +++ b/src/lib/prog_loaders.c @@ -18,6 +18,7 @@ #include <timestamp.h> #include <fit_payload.h> #include <security/vboot/vboot_common.h> +#include <vb2_sha.h> /* Only can represent up to 1 byte less than size_t. */ const struct mem_region_device addrspace_32bit = @@ -56,9 +57,32 @@ be32toh(stgh->entry_offset), NULL); } + prog->hash.algo = VB2_HASH_INVALID; + if (CONFIG(CBFS_VERIFICATION)) { + const struct vb2_hash *hash = cbfs_file_hash(&mdata); + if (hash) + memcpy(&prog->hash, hash, offsetof(struct vb2_hash, raw) + + vb2_digest_size(hash->algo)); + } + return CB_SUCCESS; } +void *prog_mmap(struct prog *prog) +{ + struct region_device *rdev = prog_rdev(prog); + void *data = rdev_mmap_full(rdev); + + if (data && CONFIG(CBFS_VERIFICATION) && + vb2_hash_verify(data, region_device_sz(rdev), prog_hash(prog))) { + prog_munmap(prog, data); + printk(BIOS_CRIT, "CBFS file hash mismatch!\n"); + return NULL; + } + + return data; +} + void run_romstage(void) { struct prog romstage = diff --git a/src/lib/rmodule.c b/src/lib/rmodule.c index bcd7b18..0e2fcb3 100644 --- a/src/lib/rmodule.c +++ b/src/lib/rmodule.c @@ -263,7 +263,7 @@ prog_name(rsl->prog), rmod_loc, prog_size(prog)); if (!cbfs_load_and_decompress(prog_rdev(prog), rmod_loc, - prog_size(prog), prog_compression(prog), NULL)) + prog_size(prog), prog_compression(prog), prog_hash(prog))) return -1; if (rmodule_parse(rmod_loc, &rmod_stage)) diff --git a/src/lib/selfboot.c b/src/lib/selfboot.c index c5ad525..66782b3 100644 --- a/src/lib/selfboot.c +++ b/src/lib/selfboot.c @@ -225,20 +225,13 @@ return 0; } -static void *selfprepare(struct prog *payload) -{ - void *data; - data = rdev_mmap_full(prog_rdev(payload)); - return data; -} - static bool _selfload(struct prog *payload, checker_t f, void *args) { uintptr_t entry = 0; struct cbfs_payload_segment *cbfssegs; void *data; - data = selfprepare(payload); + data = prog_mmap(payload); if (data == NULL) return false; @@ -252,14 +245,14 @@ printk(BIOS_SPEW, "Loaded segments\n"); - rdev_munmap(prog_rdev(payload), data); + prog_munmap(payload, data); /* Pass cbtables to payload if architecture desires it. */ prog_set_entry(payload, (void *)entry, cbmem_find(CBMEM_ID_CBTABLE)); return true; out: - rdev_munmap(prog_rdev(payload), data); + prog_munmap(payload, data); return false; } -- To view, visit https://review.coreboot.org/c/coreboot/+/48839 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3c6e8942e502035d365a193abf2cbe2f312e6561 Gerrit-Change-Number: 48839 Gerrit-PatchSet: 1 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-MessageType: newchange

1 1

Change in coreboot[master]: cbfs: Add file data hashing to cbfs_load() and cbfs_map()
by Julius Werner (Code Review) 08 Apr '21

08 Apr '21

Hello Furquan Shaikh, Aaron Durbin, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/48838 to review the following change. Change subject: cbfs: Add file data hashing to cbfs_load() and cbfs_map() ...................................................................... cbfs: Add file data hashing to cbfs_load() and cbfs_map() This patch adds the file data hashing part of CONFIG_CBFS_VERIFICATION to the cbfs_load() and cbfs_map() family of functions. To facilitate this, cbfs_load_and_decompress() is expanded to take an optional file_hash attribute that will be verified between loading and decompression. Note that hashing for other file accesses (such as stage loading) are not yet supported and will be added in later patches. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: Icdf6f73489f16f507435857e810495d1034edec9 --- M src/commonlib/bsd/cbfs_private.c M src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h M src/drivers/intel/fsp2_0/util.c M src/include/cbfs.h M src/lib/cbfs.c M src/lib/rmodule.c 6 files changed, 85 insertions(+), 17 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/48838/1 diff --git a/src/commonlib/bsd/cbfs_private.c b/src/commonlib/bsd/cbfs_private.c index 91f81e0..8d11b48 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 for '%s' unsupported. (%u)\n", mdata->h.filename, hash->algo); + 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_private.h b/src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h index 9fe740c9..f8d1a30 100644 --- a/src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h +++ b/src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h @@ -135,4 +135,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_PRIVATE_H_ */ diff --git a/src/drivers/intel/fsp2_0/util.c b/src/drivers/intel/fsp2_0/util.c index e35b378..26de3e7 100644 --- a/src/drivers/intel/fsp2_0/util.c +++ b/src/drivers/intel/fsp2_0/util.c @@ -154,7 +154,7 @@ if (fspm_xip()) return dest; - if (cbfs_load_and_decompress(source_rdev, dest, size, compression_algo) != size) { + if (cbfs_load_and_decompress(source_rdev, dest, size, compression_algo, NULL) != size) { printk(BIOS_ERR, "Failed to load FSP component.\n"); return NULL; } diff --git a/src/include/cbfs.h b/src/include/cbfs.h index 18204a7..dde4876 100644 --- a/src/include/cbfs.h +++ b/src/include/cbfs.h @@ -42,12 +42,14 @@ ("COREBOOT" FMAP region), even when CONFIG(VBOOT) is enabled. */ size_t cbfs_ro_load(const char *name, void *buf, size_t buf_size); /* Load all bytes from |rdev| to the |buffer_size| bytes large |buffer|, - * decompressing it according to |compression| in the process. + * decompressing it according to |compression| in the process. If a |file_hash| + * is passed, use it to verify the data before decompression. * 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, void *buffer, - size_t buffer_size, uint32_t compression); + size_t buffer_size, uint32_t compression, + const struct vb2_hash *file_hash); /* Load stage into memory filling in prog. Return 0 on success. < 0 on error. */ int cbfs_prog_stage_load(struct prog *prog); diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index 3684bde..bfbb7bc 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -95,6 +95,21 @@ return 0; } +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 void *_cbfs_map(const char *name, size_t *size_out, bool force_ro) { struct region_device rdev; @@ -106,7 +121,18 @@ if (size_out != NULL) *size_out = region_device_sz(&rdev); - return rdev_mmap_full(&rdev); + void *mapping = rdev_mmap_full(&rdev); + + if (CONFIG(CBFS_VERIFICATION)) { + const struct vb2_hash *hash = cbfs_file_hash(&mdata); + if (cbfs_file_hash_mismatch(mapping, region_device_sz(&rdev), + hash)) { + rdev_munmap(&rdev, mapping); + return NULL; + } + } + + return mapping; } void *cbfs_map(const char *name, size_t *size_out) @@ -195,7 +221,8 @@ } size_t cbfs_load_and_decompress(const struct region_device *rdev, void *buffer, - size_t buffer_size, uint32_t compression) + 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; @@ -207,6 +234,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: @@ -218,6 +247,8 @@ map = rdev_mmap_full(rdev); if (map == NULL) return 0; + if (cbfs_file_hash_mismatch(map, in_size, file_hash)) + return 0; timestamp_add_now(TS_START_ULZ4F); out_size = ulz4fn(map, in_size, buffer, buffer_size); @@ -233,6 +264,8 @@ map = rdev_mmap_full(rdev); if (map == NULL) return 0; + if (cbfs_file_hash_mismatch(map, in_size, file_hash)) + return 0; /* Note: timestamp not useful for memory-mapped media (x86) */ timestamp_add_now(TS_START_ULZMA); @@ -249,7 +282,8 @@ } static size_t cbfs_stage_load_and_decompress(const struct region_device *rdev, - void *buffer, size_t buffer_size, uint32_t compression) + void *buffer, size_t buffer_size, + uint32_t compression, const struct vb2_hash *file_hash) { size_t in_size = region_device_sz(rdev); struct region_device rdev_src; @@ -268,12 +302,13 @@ rdev_chain(&rdev_src, &addrspace_32bit.rdev, (uintptr_t)compr_start, in_size); - return cbfs_load_and_decompress(&rdev_src, buffer, - buffer_size, compression); + return cbfs_load_and_decompress(&rdev_src, buffer, buffer_size, + compression, file_hash); } /* All other algorithms can use the generic implementation. */ - return cbfs_load_and_decompress(rdev, buffer, buffer_size, compression); + return cbfs_load_and_decompress(rdev, buffer, buffer_size, + compression, file_hash); } static inline int tohex4(unsigned int c) @@ -326,15 +361,20 @@ return 0; uint32_t compression = CBFS_COMPRESS_NONE; - const struct cbfs_file_attr_compression *attr = cbfs_find_attr(&mdata, - CBFS_FILE_ATTR_TAG_COMPRESSION, sizeof(*attr)); - if (attr) { - compression = be32toh(attr->compression); - if (buf_size < be32toh(attr->decompressed_size)) + const struct cbfs_file_attr_compression *cattr = cbfs_find_attr(&mdata, + CBFS_FILE_ATTR_TAG_COMPRESSION, sizeof(*cattr)); + if (cattr) { + compression = be32toh(cattr->compression); + if (buf_size < be32toh(cattr->decompressed_size)) return 0; } - return cbfs_load_and_decompress(&rdev, buf, buf_size, compression); + const struct vb2_hash *file_hash = NULL; + if (CONFIG(CBFS_VERIFICATION)) + file_hash = cbfs_file_hash(&mdata); + + return cbfs_load_and_decompress(&rdev, buf, buf_size, + compression, file_hash); } size_t cbfs_load(const char *name, void *buf, size_t buf_size) @@ -364,7 +404,7 @@ fsize = cbfs_stage_load_and_decompress(rdev, prog_start(pstage), prog_size(pstage), - prog_compression(pstage)); + prog_compression(pstage), NULL); if (!fsize) return -1; diff --git a/src/lib/rmodule.c b/src/lib/rmodule.c index d8cfd36..bcd7b18 100644 --- a/src/lib/rmodule.c +++ b/src/lib/rmodule.c @@ -263,7 +263,7 @@ prog_name(rsl->prog), rmod_loc, prog_size(prog)); if (!cbfs_load_and_decompress(prog_rdev(prog), rmod_loc, - prog_size(prog), prog_compression(prog))) + prog_size(prog), prog_compression(prog), NULL)) return -1; if (rmodule_parse(rmod_loc, &rmod_stage)) -- To view, visit https://review.coreboot.org/c/coreboot/+/48838 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icdf6f73489f16f507435857e810495d1034edec9 Gerrit-Change-Number: 48838 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 1

Change in coreboot[master]: cbfs: Remove offset and in_size arguments from cbfs_load_and_decompress
by Julius Werner (Code Review) 08 Apr '21

08 Apr '21

Hello Furquan Shaikh, Aaron Durbin, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/48837 to review the following change. Change subject: cbfs: Remove offset and in_size arguments from cbfs_load_and_decompress ...................................................................... cbfs: Remove offset and in_size arguments from cbfs_load_and_decompress All current instances of cbfs_load_and_decompress() pass 0 as offset and the region device size as in_size -- we no longer need to be able to decompress smaller sizes of an rdev. To simplify the function, remove those two arguments. If this functionality is ever needed again, it can easily be achieved by rdev_chain()ing a new rdev as well. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I38c3671b3319ab832929a151993bd36e3a9027ab --- M src/drivers/intel/fsp2_0/util.c M src/include/cbfs.h M src/lib/cbfs.c M src/lib/rmodule.c 4 files changed, 20 insertions(+), 23 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/48837/1 diff --git a/src/drivers/intel/fsp2_0/util.c b/src/drivers/intel/fsp2_0/util.c index ae561f6..e35b378 100644 --- a/src/drivers/intel/fsp2_0/util.c +++ b/src/drivers/intel/fsp2_0/util.c @@ -154,8 +154,7 @@ if (fspm_xip()) return dest; - if (cbfs_load_and_decompress(source_rdev, 0, region_device_sz(source_rdev), - dest, size, compression_algo) != size) { + if (cbfs_load_and_decompress(source_rdev, dest, size, compression_algo) != size) { printk(BIOS_ERR, "Failed to load FSP component.\n"); return NULL; } diff --git a/src/include/cbfs.h b/src/include/cbfs.h index cad01c6..18204a7 100644 --- a/src/include/cbfs.h +++ b/src/include/cbfs.h @@ -41,13 +41,13 @@ /* Like cbfs_load(), except that it will always read from the read-only CBFS ("COREBOOT" FMAP region), even when CONFIG(VBOOT) is enabled. */ size_t cbfs_ro_load(const char *name, void *buf, size_t buf_size); -/* Load |in_size| bytes from |rdev| at |offset| to the |buffer_size| bytes - * large |buffer|, decompressing it according to |compression| in the process. +/* Load all bytes from |rdev| 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); +size_t cbfs_load_and_decompress(const struct region_device *rdev, void *buffer, + size_t buffer_size, uint32_t compression); /* Load stage into memory filling in prog. Return 0 on success. < 0 on error. */ int cbfs_prog_stage_load(struct prog *prog); diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index d62fabd..3684bde 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -194,9 +194,10 @@ 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) +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; @@ -204,7 +205,7 @@ 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; @@ -214,7 +215,7 @@ /* cbfs_stage_load_and_decompress() 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; @@ -229,7 +230,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; @@ -248,9 +249,9 @@ } 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) + void *buffer, size_t buffer_size, uint32_t compression) { + size_t in_size = region_device_sz(rdev); struct region_device rdev_src; if (compression == CBFS_COMPRESS_LZ4) { @@ -261,19 +262,18 @@ * 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) + if (rdev_readat(rdev, compr_start, 0, 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); + return cbfs_load_and_decompress(&rdev_src, 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); + return cbfs_load_and_decompress(rdev, buffer, buffer_size, compression); } static inline int tohex4(unsigned int c) @@ -334,8 +334,7 @@ return 0; } - return cbfs_load_and_decompress(&rdev, 0, region_device_sz(&rdev), - buf, buf_size, compression); + return cbfs_load_and_decompress(&rdev, buf, buf_size, compression); } size_t cbfs_load(const char *name, void *buf, size_t buf_size) @@ -363,7 +362,7 @@ return 0; } - fsize = cbfs_stage_load_and_decompress(rdev, 0, region_device_sz(rdev), + fsize = cbfs_stage_load_and_decompress(rdev, prog_start(pstage), prog_size(pstage), prog_compression(pstage)); if (!fsize) diff --git a/src/lib/rmodule.c b/src/lib/rmodule.c index 1252ac1..d8cfd36 100644 --- a/src/lib/rmodule.c +++ b/src/lib/rmodule.c @@ -262,8 +262,7 @@ printk(BIOS_INFO, "Decompressing stage %s @ %p (%zu bytes)\n", prog_name(rsl->prog), rmod_loc, prog_size(prog)); - if (!cbfs_load_and_decompress(prog_rdev(prog), 0, - region_device_sz(prog_rdev(prog)), rmod_loc, + if (!cbfs_load_and_decompress(prog_rdev(prog), rmod_loc, prog_size(prog), prog_compression(prog))) return -1; -- To view, visit https://review.coreboot.org/c/coreboot/+/48837 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I38c3671b3319ab832929a151993bd36e3a9027ab Gerrit-Change-Number: 48837 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 1

Change in coreboot[master]: mb/google/brya: Change GPP_E16 default to high
by Furquan Shaikh (Code Review) 08 Apr '21

08 Apr '21

Attention is currently required from: Alex Levin, Tim Wawrzynczak. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52199 ) Change subject: mb/google/brya: Change GPP_E16 default to high ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/52199 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4f9884d3b2fc8822dda1a6fe743c863aa6c696da Gerrit-Change-Number: 52199 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Levin <levinale(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alex Levin <levinale(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Fri, 09 Apr 2021 00:13:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya: Change GPP_E16 default to high
by Alex Levin (Code Review) 08 Apr '21

08 Apr '21

Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak. Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/52199 to look at the new patch set (#2). Change subject: mb/google/brya: Change GPP_E16 default to high ...................................................................... mb/google/brya: Change GPP_E16 default to high To enable WWAN we want to release it from reset start. BUG=b:180166408 TEST=WWAN enumerates on brya Change-Id: I4f9884d3b2fc8822dda1a6fe743c863aa6c696da Signed-off-by: Alex Levin <levinale(a)google.com> --- M src/mainboard/google/brya/variants/baseboard/gpio.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/52199/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/52199 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4f9884d3b2fc8822dda1a6fe743c863aa6c696da Gerrit-Change-Number: 52199 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Levin <levinale(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/brya: Change GPP_E16 default to high
by Alex Levin (Code Review) 08 Apr '21

08 Apr '21

Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak. Alex Levin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52199 ) Change subject: mb/google/brya: Change GPP_E16 default to high ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/52199/comment/6ee4b674_0a71c424 PS1, Line 15: > No Signed-off-by line in commit message Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/52199 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4f9884d3b2fc8822dda1a6fe743c863aa6c696da Gerrit-Change-Number: 52199 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Levin <levinale(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Fri, 09 Apr 2021 00:09:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel: Remove SMM HECI disable related options
by Karthik Ramasubramanian (Code Review) 08 Apr '21

08 Apr '21

Attention is currently required from: Martin Roth, Furquan Shaikh, Aseda Aboagye, Aaron Durbin. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52147 ) Change subject: soc/intel: Remove SMM HECI disable related options ...................................................................... Patch Set 3: (1 comment) File src/soc/intel/cannonlake/smihandler.c: https://review.coreboot.org/c/coreboot/+/52147/comment/1e354fb5_bd971ee5 PS2, Line 22: const struct device *dev = pcidev_path_on_root(PCH_DEVFN_CSE); : if (!is_dev_enabled(dev)) > CNL doesn't consult the devicetree here. […] The concerned PCI device (device pci 16.0) is also enabled/disabled in devicetree. So I believe the check is sufficient. -- To view, visit https://review.coreboot.org/c/coreboot/+/52147 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iced84faaab8a0cca8400ccd3ec6e4cd525c30e92 Gerrit-Change-Number: 52147 Gerrit-PatchSet: 3 Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Comment-Date: Fri, 09 Apr 2021 00:02:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aseda Aboagye <aaboagye(a)google.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2021