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@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;