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>
diff --git a/src/lib/bootsplash.c b/src/lib/bootsplash.c
index 050d713..0eb94dc 100644
--- a/src/lib/bootsplash.c
+++ b/src/lib/bootsplash.c
@@ -16,8 +16,7 @@
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 @@
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 94dae62..493093e 100644
--- a/src/lib/cbfs.c
+++ b/src/lib/cbfs.c
@@ -70,22 +70,29 @@
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 @@
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 @@
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 4cbf3c7..996e76e 100644
--- a/src/lib/coreboot_table.c
+++ b/src/lib/coreboot_table.c
@@ -455,8 +455,7 @@
#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 e17d40e..2f7186c 100644
--- a/src/lib/fw_config.c
+++ b/src/lib/fw_config.c
@@ -24,9 +24,8 @@
/* 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 38edc5f..863218c 100644
--- a/src/lib/spd_bin.c
+++ b/src/lib/spd_bin.c
@@ -227,12 +227,11 @@
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 @@
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);