cbfs: Simplify load/map API names, remove type arguments

This patch renames cbfs_boot_map_with_leak() and cbfs_boot_load_file()
to cbfs_map() and cbfs_load() respectively. This is supposed to be the
start of a new, better organized CBFS API where the most common
operations have the most simple and straight-forward names. Less
commonly used variants of these operations (e.g. cbfs_ro_load() or
cbfs_region_load()) can be introduced later. It seems unnecessary to
keep carrying around "boot" in the names of most CBFS APIs if the vast
majority of accesses go to the boot CBFS (instead, more unusual
operations should have longer names that describe how they diverge from
the common ones).

cbfs_map() is paired with a new cbfs_unmap() to allow callers to cleanly
reap mappings when desired. A few new cbfs_unmap() calls are added to
generic code where it makes sense, but it seems unnecessary to introduce
this everywhere in platform or architecture specific code where the boot
medium is known to be memory-mapped anyway. In fact, even for
non-memory-mapped platforms, sometimes leaking a mapping to the CBFS
cache is a much cleaner solution than jumping through hoops to provide
some other storage for some long-lived file object, and it shouldn't be
outright forbidden when it makes sense.

Additionally, remove the type arguments from these function signatures.
The goal is to eventually remove type arguments for lookup from the
whole CBFS API. Filenames already uniquely identify CBFS files. The type
field is just informational, and there should be APIs to allow callers
to check it when desired, but it's not clear what we gain from forcing
this as a parameter into every single CBFS access when the vast majority
of the time it provides no additional value and is just clutter.

Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: Ib24325400815a9c3d25f66c61829a24a239bb88e
Reviewed-on: https://review.coreboot.org/c/coreboot/+/39304
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Wim Vervoorn <wvervoorn@eltan.com>
Reviewed-by: Mariusz Szafrański <mariuszx.szafranski@intel.com>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>

5 files changed, 23 insertions, 19 deletions

diff --git a/src/lib/bootsplash.c b/src/lib/bootsplash.c
index 050d713154..0eb94dc812 100644
--- a/src/lib/bootsplash.c
+++ b/src/lib/bootsplash.c
@@ -16,8 +16,7 @@ void set_bootsplash(unsigned char *framebuffer, unsigned int x_resolution,
 	printk(BIOS_INFO, "Setting up bootsplash in %dx%d@%d\n", x_resolution, y_resolution,
 	       fb_resolution);
 	struct jpeg_decdata *decdata;
-	unsigned char *jpeg =
-		cbfs_boot_map_with_leak("bootsplash.jpg", CBFS_TYPE_BOOTSPLASH, NULL);
+	unsigned char *jpeg = cbfs_map("bootsplash.jpg", NULL);
 	if (!jpeg) {
 		printk(BIOS_ERR, "Could not find bootsplash.jpg\n");
 		return;
@@ -31,6 +30,7 @@ void set_bootsplash(unsigned char *framebuffer, unsigned int x_resolution,
 	decdata = malloc(sizeof(*decdata));
 	int ret = jpeg_decode(jpeg, framebuffer, x_resolution, y_resolution, fb_resolution,
 			      decdata);
+	cbfs_unmap(jpeg);
 	if (ret != 0) {
 		printk(BIOS_ERR, "Bootsplash could not be decoded. jpeg_decode returned %d.\n",
 		       ret);
diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c
index 94dae625b2..493093ebb9 100644
--- a/src/lib/cbfs.c
+++ b/src/lib/cbfs.c
@@ -70,22 +70,29 @@ int cbfs_boot_locate(struct cbfsf *fh, const char *name, uint32_t *type)
 	return 0;
 }
 
-void *cbfs_boot_map_with_leak(const char *name, uint32_t type, size_t *size)
+void *cbfs_map(const char *name, size_t *size_out)
 {
 	struct cbfsf fh;
 	size_t fsize;
 
-	if (cbfs_boot_locate(&fh, name, &type))
+	if (cbfs_boot_locate(&fh, name, NULL))
 		return NULL;
 
 	fsize = region_device_sz(&fh.data);
 
-	if (size != NULL)
-		*size = fsize;
+	if (size_out != NULL)
+		*size_out = fsize;
 
 	return rdev_mmap(&fh.data, 0, fsize);
 }
 
+int cbfs_unmap(void *mapping)
+{
+	/* This works because munmap() only works on the root rdev and never
+	   cares about which chained subregion something was mapped from. */
+	return rdev_munmap(boot_device_ro(), mapping);
+}
+
 int cbfs_locate_file_in_region(struct cbfsf *fh, const char *region_name,
 			       const char *name, uint32_t *type)
 {
@@ -262,7 +269,7 @@ void *cbfs_boot_map_optionrom(uint16_t vendor, uint16_t device)
 	tohex16(vendor, name + 3);
 	tohex16(device, name + 8);
 
-	return cbfs_boot_map_with_leak(name, CBFS_TYPE_OPTIONROM, NULL);
+	return cbfs_map(name, NULL);
 }
 
 void *cbfs_boot_map_optionrom_revision(uint16_t vendor, uint16_t device, uint8_t rev)
@@ -273,17 +280,16 @@ void *cbfs_boot_map_optionrom_revision(uint16_t vendor, uint16_t device, uint8_t
 	tohex16(device, name + 8);
 	tohex8(rev, name + 13);
 
-	return cbfs_boot_map_with_leak(name, CBFS_TYPE_OPTIONROM, NULL);
+	return cbfs_map(name, NULL);
 }
 
-size_t cbfs_boot_load_file(const char *name, void *buf, size_t buf_size,
-			   uint32_t type)
+size_t cbfs_load(const char *name, void *buf, size_t buf_size)
 {
 	struct cbfsf fh;
 	uint32_t compression_algo;
 	size_t decompressed_size;
 
-	if (cbfs_boot_locate(&fh, name, &type) < 0)
+	if (cbfs_boot_locate(&fh, name, NULL) < 0)
 		return 0;
 
 	if (cbfsf_decompression_info(&fh, &compression_algo,
diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c
index 4cbf3c742e..996e76ec11 100644
--- a/src/lib/coreboot_table.c
+++ b/src/lib/coreboot_table.c
@@ -455,8 +455,7 @@ static uintptr_t write_coreboot_table(uintptr_t rom_table_end)
 #if CONFIG(USE_OPTION_TABLE)
 	{
 		struct cmos_option_table *option_table =
-			cbfs_boot_map_with_leak("cmos_layout.bin",
-				CBFS_COMPONENT_CMOS_LAYOUT, NULL);
+			cbfs_map("cmos_layout.bin", NULL);
 		if (option_table) {
 			struct lb_record *rec_dest = lb_new_record(head);
 			/* Copy the option config table, it's already a
diff --git a/src/lib/fw_config.c b/src/lib/fw_config.c
index e17d40e58a..2f7186cc86 100644
--- a/src/lib/fw_config.c
+++ b/src/lib/fw_config.c
@@ -24,9 +24,8 @@ uint64_t fw_config_get(void)
 
 	/* Look in CBFS to allow override of value. */
 	if (CONFIG(FW_CONFIG_SOURCE_CBFS)) {
-		if (cbfs_boot_load_file(CONFIG_CBFS_PREFIX "/fw_config",
-					&fw_config_value, sizeof(fw_config_value),
-					CBFS_TYPE_RAW) != sizeof(fw_config_value)) {
+		if (cbfs_load(CONFIG_CBFS_PREFIX "/fw_config", &fw_config_value,
+			      sizeof(fw_config_value)) != sizeof(fw_config_value)) {
 			printk(BIOS_WARNING, "%s: Could not get fw_config from CBFS\n",
 			       __func__);
 			fw_config_value = 0;
diff --git a/src/lib/spd_bin.c b/src/lib/spd_bin.c
index 38edc5f151..863218c2d3 100644
--- a/src/lib/spd_bin.c
+++ b/src/lib/spd_bin.c
@@ -227,12 +227,11 @@ int read_ddr3_spd_from_cbfs(u8 *buf, int idx)
 	const int SPD_CRC_HI = 127;
 	const int SPD_CRC_LO = 126;
 
-	const char *spd_file;
+	char *spd_file;
 	size_t spd_file_len = 0;
 	size_t min_len = (idx + 1) * CONFIG_DIMM_SPD_SIZE;
 
-	spd_file = cbfs_boot_map_with_leak("spd.bin", CBFS_TYPE_SPD,
-						&spd_file_len);
+	spd_file = cbfs_map("spd.bin", &spd_file_len);
 	if (!spd_file)
 		printk(BIOS_EMERG, "file [spd.bin] not found in CBFS");
 	if (spd_file_len < min_len)
@@ -242,6 +241,7 @@ int read_ddr3_spd_from_cbfs(u8 *buf, int idx)
 
 	memcpy(buf, spd_file + (idx * CONFIG_DIMM_SPD_SIZE),
 		CONFIG_DIMM_SPD_SIZE);
+	cbfs_unmap(spd_file);
 
 	u16 crc = spd_ddr3_calc_crc(buf, CONFIG_DIMM_SPD_SIZE);