From 834b3ecd7cbefbad8f09a9bda4f10cd7842cdbcd Mon Sep 17 00:00:00 2001 From: Julius Werner Date: Wed, 4 Mar 2020 16:52:08 -0800 Subject: cbfs: Simplify load/map API names, remove type arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Change-Id: Ib24325400815a9c3d25f66c61829a24a239bb88e Reviewed-on: https://review.coreboot.org/c/coreboot/+/39304 Reviewed-by: Hung-Te Lin Reviewed-by: Wim Vervoorn Reviewed-by: Mariusz Szafrański Reviewed-by: Patrick Georgi Tested-by: build bot (Jenkins) --- src/lib/bootsplash.c | 4 ++-- src/lib/cbfs.c | 24 +++++++++++++++--------- src/lib/coreboot_table.c | 3 +-- src/lib/fw_config.c | 5 ++--- src/lib/spd_bin.c | 6 +++--- 5 files changed, 23 insertions(+), 19 deletions(-) (limited to 'src/lib') 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); -- cgit v1.2.3