Change in coreboot[master]: cbfs: Remove offset and in_size arguments from cbfs_load_and_decompress

[Julius Werner (Code Review)]

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

[Julius Werner (Code Review)]

Julius Werner has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/48837 )

Change subject: cbfs: Remove offset and in_size arguments from cbfs_load_and_decompress
......................................................................

Abandoned

Superseded by CB:52083