Change in coreboot[master]: cbfs: Add cbfs_alloc() primitive and combine cbfs_load() and cbfs_map()
Attention is currently required from: Furquan Shaikh, Aaron Durbin. Hello Furquan Shaikh, Aaron Durbin,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/49334
to review the following change.
Change subject: cbfs: Add cbfs_alloc() primitive and combine cbfs_load() and cbfs_map() ......................................................................
cbfs: Add cbfs_alloc() primitive and combine cbfs_load() and cbfs_map()
This patchs adds a new CBFS primitive that allows callers to pass in an allocator function that will be called once the size of the file to load is known, to decide on its final location. This can be useful for loading a CBFS file straight into CBMEM, for example. The new primitive is combined with cbfs_map() and cbfs_load() into a single underlying function that can handle all operations, to reduce the amount of code that needs to be duplicated (especially later when file verification is added). Also add a new variation that allows restraining or querying the CBFS type of a file as it is being loaded, and reorganize the documentation/definition of all these accessors and variations in the header file a little.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I5fe0645387c0e9053ad5c15744437940fc904392 --- M src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h M src/include/cbfs.h M src/lib/cbfs.c 3 files changed, 239 insertions(+), 75 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/49334/1
diff --git a/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h b/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h index dd50469..dc14cd5 100644 --- a/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h +++ b/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h @@ -13,6 +13,9 @@ };
enum cbfs_type { + /* QUERY is an alias for DELETED that can be passed to CBFS APIs to + inquire about the type of a file, rather than constrain it. */ + CBFS_TYPE_QUERY = 0, CBFS_TYPE_DELETED = 0x00000000, CBFS_TYPE_NULL = 0xffffffff, CBFS_TYPE_BOOTBLOCK = 0x01, diff --git a/src/include/cbfs.h b/src/include/cbfs.h index 8aaff39..a2195c6 100644 --- a/src/include/cbfs.h +++ b/src/include/cbfs.h @@ -15,27 +15,88 @@ * CBFS FILE ACCESS APIs * **********************************************************************************************/
-/* Map file into memory, returning a pointer to the mapping or NULL on error. If |size_out| is - not NULL, it will pass out the size of the mapped file. - NOTE: Since this may return a direct pointer to memory-mapped hardware, compressed files are - NOT transparently decompressed (unlike cbfs_load()). */ -static inline void *cbfs_map(const char *name, size_t *size_out); +/* + * These are the APIs used to access files in CBFS. In order to keep the calls simple and free + * of clutter in the common cases, but still offer all advanced functionality when needed, there + * are many different variations that are implemented by wrapping the same underlying API with + * static inlines. All accessors have in common that they look up files by name, and will + * transparently decompress files that are compressed. + * + * There are three main flavors of CBFS accessors: + * + * size_t cbfs_load(char *name, void *buf, size_t size): Loads the contents of a CBFS file into + * a buffer provided by the caller (by providing pointer and size to it). Will return the + * amount of bytes loaded on success, or 0 on error. + * + * void *cbfs_map(char *name, size_t *size_out): Maps a file into the address space. If the file + * is not compressed and the platform supports direct memory-mapping for the boot medium, + * a pointer to the platform mapping is returned directly. In all other cases, memory will + * be allocated from the cbfs_cache and file data will be loaded into there. Returns a + * pointer to the mapping on success, or NULL on error. If an optional size_out parameter + * is passed in, it will be filled out with the size of the mapped data. Caller should call + * cbfs_unmap() after it is done using the mapping to free up the cbfs_cache if possible. + * + * void *cbfs_alloc(char *name, cbfs_allocator_t allocator, void *arg, size_t *size_out): Loads + * file data into memory provided by a custom allocator function that the caller passes in. + * The caller may pass an argument that is passed through verbatim to the allocator. + * Returns the pointer returned by the allocator (where the file data was loaded to) on + * success, or NULL on error. If an optional size_out parameter is passed in, it will be + * filled out with the size of the loaded data. + * + * void *cbfs_cbmem_alloc(char *name, uint32_t cbmem_id, size_t *size_out): Wrapper around + * cbfs_alloc() that will provide an allocator function for allocating space for the file + * data in CBMEM, with the provided CBMEM ID. + * + * All of these flavors have variations with any of the following optional parameters added: + * + * ..._ro_...: Will force looking up the CBFS file in the read-only CBFS (the "COREBOOT" FMAP + * section), even when running in an RW stage from one of the RW CBFSs. Only relevant if + * CONFIG_VBOOT=y. + * + * ..._type_...: May pass in an extra enum cbfs_type *type parameter. If the value it points to + * is CBFS_TYPE_QUERY, it will be replaced with the actual CBFS type of the found file. If + * it is anything else, the type will be compared with the actually found type, and the + * operation will fail if they don't match. + */
-/* Like cbfs_map(), except that it will always read from the read-only CBFS (the "COREBOOT" FMAP - region), even when CONFIG(VBOOT) is enabled. */ +/* An allocator function for passing to cbfs_alloc(). Takes the argument that was originally + passed to cbfs_alloc(), and the size of the file to be loaded. Must return a pointer to space + of the requested size where the file data should be loaded, or NULL to make the operation + fail. */ +typedef void *(*cbfs_allocator_t)(void *arg, size_t size); + +static inline size_t cbfs_load(const char *name, void *buf, size_t size); +static inline size_t cbfs_ro_load(const char *name, void *buf, size_t size); +static inline size_t cbfs_type_load(const char *name, void *buf, size_t size, + enum cbfs_type *type); +static inline size_t cbfs_ro_type_load(const char *name, void *buf, size_t size, + enum cbfs_type *type); + +static inline void *cbfs_map(const char *name, size_t *size_out); static inline void *cbfs_ro_map(const char *name, size_t *size_out); +static inline void *cbfs_type_map(const char *name, size_t *size_out, enum cbfs_type *type); +static inline void *cbfs_ro_type_map(const char *name, size_t *size_out, enum cbfs_type *type); + +static inline void *cbfs_alloc(const char *name, cbfs_allocator_t allocator, void *arg, + size_t *size_out); +static inline void *cbfs_ro_alloc(const char *name, cbfs_allocator_t allocator, void *arg, + size_t *size_out); +static inline void *cbfs_type_alloc(const char *name, cbfs_allocator_t allocator, void *arg, + size_t *size_out, enum cbfs_type *type); +static inline void *cbfs_ro_type_alloc(const char *name, cbfs_allocator_t allocator, void *arg, + size_t *size_out, enum cbfs_type *type); + +static inline void *cbfs_cbmem_alloc(const char *name, uint32_t cbmem_id, size_t *size_out); +static inline void *cbfs_ro_cbmem_alloc(const char *name, uint32_t cbmem_id, size_t *size_out); +static inline void *cbfs_type_cbmem_alloc(const char *name, uint32_t cbmem_id, size_t *size_out, + enum cbfs_type *type); +static inline void *cbfs_ro_type_cbmem_alloc(const char *name, uint32_t cbmem_id, + size_t *size_out, enum cbfs_type *type);
/* Removes a previously allocated CBFS mapping. Should try to unmap mappings in strict LIFO order where possible, since mapping backends often don't support more complicated cases. */ void cbfs_unmap(void *mapping);
-/* Load a file from CBFS into a buffer. Returns amount of loaded bytes on success or 0 on error. - File will get decompressed as necessary. */ -static inline size_t cbfs_load(const char *name, void *buf, size_t buf_size); -/* Like cbfs_load(), except that it will always read from the read-only CBFS (the "COREBOOT" - FMAP region), even when CONFIG(VBOOT) is enabled. */ -static inline size_t cbfs_ro_load(const char *name, void *buf, size_t buf_size); - /* Load stage into memory filling in prog. Return 0 on success. < 0 on error. */ int cbfs_prog_stage_load(struct prog *prog);
@@ -105,31 +166,118 @@ /********************************************************************************************** * INTERNAL HELPERS FOR INLINES, DO NOT USE. * **********************************************************************************************/ -size_t _cbfs_load(const char *name, void *buf, size_t buf_size, bool force_ro); -void *_cbfs_map(const char *name, size_t *size_out, bool force_ro); +void *_cbfs_alloc(const char *name, cbfs_allocator_t allocator, void *arg, + size_t *size_out, bool force_ro, enum cbfs_type *type);
+struct _cbfs_default_allocator_arg { + void *buf; + size_t buf_size; +}; +void *_cbfs_default_allocator(void *arg, size_t size); + +void *_cbfs_cbmem_allocator(void *arg, size_t size);
/********************************************************************************************** * INLINE IMPLEMENTATIONS * **********************************************************************************************/ +static inline void *cbfs_alloc(const char *name, cbfs_allocator_t allocator, void *arg, + size_t *size_out) +{ + return cbfs_type_alloc(name, allocator, arg, size_out, NULL); +} + +static inline void *cbfs_ro_alloc(const char *name, cbfs_allocator_t allocator, void *arg, + size_t *size_out) +{ + return cbfs_ro_type_alloc(name, allocator, arg, size_out, NULL); +} + +static inline void *cbfs_type_alloc(const char *name, cbfs_allocator_t allocator, void *arg, + size_t *size_out, enum cbfs_type *type) +{ + return _cbfs_alloc(name, allocator, arg, size_out, false, type); +} + +static inline void *cbfs_ro_type_alloc(const char *name, cbfs_allocator_t allocator, void *arg, + size_t *size_out, enum cbfs_type *type) +{ + return _cbfs_alloc(name, allocator, arg, size_out, true, type); +} + static inline void *cbfs_map(const char *name, size_t *size_out) { - return _cbfs_map(name, size_out, false); + return cbfs_type_map(name, size_out, NULL); }
static inline void *cbfs_ro_map(const char *name, size_t *size_out) { - return _cbfs_map(name, size_out, true); + return cbfs_ro_type_map(name, size_out, NULL); }
-static inline size_t cbfs_load(const char *name, void *buf, size_t buf_size) +static inline void *cbfs_type_map(const char *name, size_t *size_out, enum cbfs_type *type) { - return _cbfs_load(name, buf, buf_size, false); + return cbfs_type_alloc(name, NULL, NULL, size_out, type); }
-static inline size_t cbfs_ro_load(const char *name, void *buf, size_t buf_size) +static inline void *cbfs_ro_type_map(const char *name, size_t *size_out, enum cbfs_type *type) { - return _cbfs_load(name, buf, buf_size, true); + return cbfs_ro_type_alloc(name, NULL, NULL, size_out, type); +} + +static inline size_t _cbfs_load(const char *name, void *buf, size_t size, bool force_ro, + enum cbfs_type *type) +{ + struct _cbfs_default_allocator_arg arg = { .buf = buf, .buf_size = size }; + if (_cbfs_alloc(name, _cbfs_default_allocator, &arg, &size, force_ro, type)) + return size; + else + return 0; +} + +static inline size_t cbfs_load(const char *name, void *buf, size_t size) +{ + return cbfs_type_load(name, buf, size, NULL); +} + +static inline size_t cbfs_type_load(const char *name, void *buf, size_t size, + enum cbfs_type *type) +{ + return cbfs_ro_type_load(name, buf, size, type); +} + +static inline size_t cbfs_ro_load(const char *name, void *buf, size_t size) +{ + return _cbfs_load(name, buf, size, true, NULL); +} + +static inline size_t cbfs_ro_type_load(const char *name, void *buf, size_t size, + enum cbfs_type *type) +{ + return _cbfs_load(name, buf, size, true, type); +} + +static inline void *cbfs_cbmem_alloc(const char *name, uint32_t cbmem_id, size_t *size_out) +{ + return cbfs_type_cbmem_alloc(name, cbmem_id, size_out, NULL); +} + +static inline void *cbfs_ro_cbmem_alloc(const char *name, uint32_t cbmem_id, size_t *size_out) +{ + return cbfs_ro_type_cbmem_alloc(name, cbmem_id, size_out, NULL); +} + +static inline void *cbfs_type_cbmem_alloc(const char *name, uint32_t cbmem_id, size_t *size_out, + enum cbfs_type *type) +{ + return cbfs_type_alloc(name, _cbfs_cbmem_allocator, (void *)(uintptr_t)cbmem_id, + size_out, type); +} + +static inline void *cbfs_ro_type_cbmem_alloc(const char *name, uint32_t cbmem_id, + size_t *size_out, enum cbfs_type *type) +{ + return cbfs_ro_type_alloc(name, _cbfs_cbmem_allocator, (void *)(uintptr_t)cbmem_id, + size_out, type); }
#endif diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index be1f9fa..54896d1 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -103,48 +103,6 @@ return 0; }
-void *_cbfs_map(const char *name, size_t *size_out, bool force_ro) -{ - struct region_device rdev; - union cbfs_mdata mdata; - - if (cbfs_boot_lookup(name, force_ro, &mdata, &rdev)) - return NULL; - - if (size_out != NULL) - *size_out = region_device_sz(&rdev); - - uint32_t compression = CBFS_COMPRESS_NONE; - const struct cbfs_file_attr_compression *attr = cbfs_find_attr(&mdata, - CBFS_FILE_ATTR_TAG_COMPRESSION, sizeof(*attr)); - if (attr) - compression = be32toh(attr->compression); - - if (compression == CBFS_COMPRESS_NONE) - return rdev_mmap_full(&rdev); - - if (!CBFS_CACHE_AVAILABLE) { - ERROR("Cannot map compressed file %s in x86 pre-RAM stage\n", mdata.h.filename); - return NULL; - } - - size_t size = be32toh(attr->decompressed_size); - void *buf = mem_pool_alloc(&cbfs_cache, size); - if (!buf) { - ERROR("CBFS cache out of memory when mapping %s\n", mdata.h.filename); - return NULL; - } - - size = cbfs_load_and_decompress(&rdev, 0, region_device_sz(&rdev), buf, size, - compression); - if (!size) - return NULL; - - if (size_out != NULL) - *size_out = size; - return buf; -} - void cbfs_unmap(void *mapping) { /* @@ -229,6 +187,8 @@ size_t out_size; void *map;
+ DEBUG("Decompressing %zu bytes to %p with algo %d\n", buffer_size, buffer, compression); + switch (compression) { case CBFS_COMPRESS_NONE: if (buffer_size < in_size) @@ -343,25 +303,78 @@ return cbfs_map(name, NULL); }
-size_t _cbfs_load(const char *name, void *buf, size_t buf_size, bool force_ro) +void *_cbfs_alloc(const char *name, cbfs_allocator_t allocator, void *arg, + size_t *size_out, bool force_ro, enum cbfs_type *type) { struct region_device rdev; union cbfs_mdata mdata; + void *loc; + + DEBUG("%s(name='%s', alloc=%p(%p), force_ro=%s, type=%d)\n", __func__, name, allocator, + arg, force_ro ? "true" : "false", type ? *type : -1);
if (cbfs_boot_lookup(name, force_ro, &mdata, &rdev)) - return 0; + return NULL;
- uint32_t compression = CBFS_COMPRESS_NONE; - const struct cbfs_file_attr_compression *attr = cbfs_find_attr(&mdata, - CBFS_FILE_ATTR_TAG_COMPRESSION, sizeof(*attr)); - if (attr) { - compression = be32toh(attr->compression); - if (buf_size < be32toh(attr->decompressed_size)) - return 0; + if (type) { + const enum cbfs_type real_type = be32toh(mdata.h.type); + if (*type == CBFS_TYPE_QUERY) + *type = real_type; + else if (*type != real_type) { + ERROR("'%s' type mismatch (is %u, expected %u)\n", + mdata.h.filename, real_type, *type); + return NULL; + } }
- return cbfs_load_and_decompress(&rdev, 0, region_device_sz(&rdev), - buf, buf_size, compression); + size_t size = region_device_sz(&rdev); + uint32_t compression = CBFS_COMPRESS_NONE; + const struct cbfs_file_attr_compression *cattr = cbfs_find_attr(&mdata, + CBFS_FILE_ATTR_TAG_COMPRESSION, sizeof(*cattr)); + if (cattr) { + compression = be32toh(cattr->compression); + size = be32toh(cattr->decompressed_size); + } + + if (size_out) + *size_out = size; + + /* allocator == NULL means do a cbfs_map() */ + if (allocator) { + loc = allocator(arg, size); + } else if (compression == CBFS_COMPRESS_NONE) { + return rdev_mmap_full(&rdev); + } else if (!CBFS_CACHE_AVAILABLE) { + ERROR("Cannot map compressed file %s in x86 pre-RAM stage\n", mdata.h.filename); + return NULL; + } else { + loc = mem_pool_alloc(&cbfs_cache, size); + } + + if (!loc) { + ERROR("'%s' allocation failure\n", mdata.h.filename); + return NULL; + } + + size = cbfs_load_and_decompress(&rdev, 0, region_device_sz(&rdev), + loc, size, compression); + if (!size) + return NULL; + + return loc; +} + +void *_cbfs_default_allocator(void *arg, size_t size) +{ + struct _cbfs_default_allocator_arg *darg = arg; + if (size > darg->buf_size) + return NULL; + return darg->buf; +} + +void *_cbfs_cbmem_allocator(void *arg, size_t size) +{ + return cbmem_add((uintptr_t)arg, size); }
int cbfs_prog_stage_load(struct prog *pstage)
-
Julius Werner (Code Review)