coreboot-gerrit July 2021

coreboot-gerrit@coreboot.org

1 participants
5824 discussions

Change in coreboot[master]: WIP: soc/amd/common/block/lpc: Don't disable the HOG bit
by Raul Rangel (Code Review) 03 Jul '21

03 Jul '21

Attention is currently required from: Jason Glenesk, Marshall Dawson, Felix Held. Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56052 ) Change subject: WIP: soc/amd/common/block/lpc: Don't disable the HOG bit ...................................................................... WIP: soc/amd/common/block/lpc: Don't disable the HOG bit This bit breaks SPI DMA. BUG=b:179699789 TEST=Boot guybrush and see SPI DMA working Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: If015869657f36d3533f4ab9ebd1f54b0d4eb283b --- M src/soc/amd/common/block/lpc/lpc.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/56052/1 diff --git a/src/soc/amd/common/block/lpc/lpc.c b/src/soc/amd/common/block/lpc/lpc.c index 2586ba9..6eb5157 100644 --- a/src/soc/amd/common/block/lpc/lpc.c +++ b/src/soc/amd/common/block/lpc/lpc.c @@ -66,7 +66,7 @@ * on on LPC, it holds PCI grant, so no LPC slave cycle can * interrupt and visit LPC. */ - byte &= ~LPC_NOHOG; + // byte &= ~LPC_NOHOG; pci_write_config8(dev, LPC_MISC_CONTROL_BITS, byte); /* -- To view, visit https://review.coreboot.org/c/coreboot/+/56052 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If015869657f36d3533f4ab9ebd1f54b0d4eb283b Gerrit-Change-Number: 56052 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: lib/prog_loaders: Add payload_preload
by Raul Rangel (Code Review) 03 Jul '21

03 Jul '21

Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56051 ) Change subject: lib/prog_loaders: Add payload_preload ...................................................................... lib/prog_loaders: Add payload_preload This method will allow the SoC code to start loading the payload before it is required. BUG=b:177909625 TEST=Boot guybrush and see read/decompress drop by 23 ms. Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: Ifa8f30a0f4f931ece803c2e8e022e4d33d3fe581 --- M src/include/program_loading.h M src/include/symbols.h M src/lib/Kconfig M src/lib/prog_loaders.c 4 files changed, 62 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/56051/1 diff --git a/src/include/program_loading.h b/src/include/program_loading.h index 65a36b9..dbb0ffd 100644 --- a/src/include/program_loading.h +++ b/src/include/program_loading.h @@ -154,6 +154,8 @@ int payload_arch_usable_ram_quirk(uint64_t start, uint64_t size); +/* Tries to asynchronously load the payload before it is required */ +void payload_preload(void); /* Load payload into memory in preparation to run. */ void payload_load(void); diff --git a/src/include/symbols.h b/src/include/symbols.h index 3e4694b..52624d4 100644 --- a/src/include/symbols.h +++ b/src/include/symbols.h @@ -52,6 +52,7 @@ /* Regions for execution units. */ +DECLARE_REGION(payload_preload_cache) DECLARE_REGION(payload) /* "program" always refers to the current execution unit. */ DECLARE_REGION(program) diff --git a/src/lib/Kconfig b/src/lib/Kconfig index b556f7d..3dfadaa 100644 --- a/src/lib/Kconfig +++ b/src/lib/Kconfig @@ -106,3 +106,19 @@ futures and poll them until completion. If this is disabled and futures are in use, they must be polled manually. +config HAVE_PAYLOAD_PRELOAD_CACHE + bool + select FUTURES_EXECUTOR + help + Selected by the SoC to indicate it provides the payload_preload_cache + region. + +config PAYLOAD_PRELOAD_CACHE_SIZE + hex + default 0x30000 + depends on HAVE_PAYLOAD_PRELOAD_CACHE + help + On some systems with SPI DMA controllers, it is possible to preload + the payload while FSP-S is executing. This config sets the size of the + space in RAM allocated for the raw payload. This space is only + populated during non-S3, so it doesn't need to be reserved. diff --git a/src/lib/prog_loaders.c b/src/lib/prog_loaders.c index 40f51eb..04faf8b 100644 --- a/src/lib/prog_loaders.c +++ b/src/lib/prog_loaders.c @@ -126,17 +126,56 @@ static struct prog global_payload = PROG_INIT(PROG_PAYLOAD, CONFIG_CBFS_PREFIX "/payload"); +static struct cbfs_load_async_context payload_preload_context; + +void payload_preload(void) +{ + cb_err_t err; + struct prog *payload = &global_payload; + struct cbfs_load_async_context *context = &payload_preload_context; + + if (!CONFIG(HAVE_PAYLOAD_PRELOAD_CACHE)) + return; + + if (prog_locate_hook(payload)) + return; + + printk(BIOS_INFO, "Preloading payload\n"); + + err = cbfs_load_async(prog_name(payload), context, _payload_preload_cache, + REGION_SIZE(payload_preload_cache)); + + if (err) + printk(BIOS_ERR, "Preloading payload failed: %d\n", err); + else + register_future(&context->future); +} + void payload_load(void) { struct prog *payload = &global_payload; + struct cbfs_load_async_context *context = &payload_preload_context; + void *mapping = NULL; timestamp_add_now(TS_LOAD_PAYLOAD); - if (prog_locate_hook(payload)) - goto out; + if (future_is_valid(&context->future)) { + wait_for_future(&context->future); + if (context->future.error == CB_SUCCESS) { + mapping = context->buffer; + payload->cbfs_type = context->type; + } else { + printk(BIOS_ERR, "%s: Failed to preload payload\n", __func__); + /* Fall through and try the synchronous route */ + } + } else { + if (prog_locate_hook(payload)) + goto out; - payload->cbfs_type = CBFS_TYPE_QUERY; - void *mapping = cbfs_type_map(prog_name(payload), NULL, &payload->cbfs_type); + payload->cbfs_type = CBFS_TYPE_QUERY; + mapping = cbfs_type_map(prog_name(payload), NULL, &payload->cbfs_type); + } + if (!mapping) goto out; -- To view, visit https://review.coreboot.org/c/coreboot/+/56051 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifa8f30a0f4f931ece803c2e8e022e4d33d3fe581 Gerrit-Change-Number: 56051 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: lib/hardwaremain: Call poll_futures
by Raul Rangel (Code Review) 03 Jul '21

03 Jul '21

Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56050 ) Change subject: lib/hardwaremain: Call poll_futures ...................................................................... lib/hardwaremain: Call poll_futures This CL makes the ramstage state machine poll the futures after each callback and during stage transitions. This will keep the futures progressing throughout the boot process. This is a no-op for platforms that don't set FUTURES_EXECUTOR. BUG=b:177909625 TEST=Boot guybrush and see payload read/decompress drop by 23 ms. Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: I15e17ecb1094a4bfb7bf5cabe8c847b9e697ebf2 --- M src/lib/hardwaremain.c 1 file changed, 7 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/56050/1 diff --git a/src/lib/hardwaremain.c b/src/lib/hardwaremain.c index 1bda525..b68951b 100644 --- a/src/lib/hardwaremain.c +++ b/src/lib/hardwaremain.c @@ -12,6 +12,7 @@ #include <bootstate.h> #include <console/console.h> #include <console/post_codes.h> +#include <commonlib/bsd/future.h> #include <commonlib/helpers.h> #include <cbmem.h> #include <version.h> @@ -289,6 +290,8 @@ mono_time_diff_microseconds(&mt_start, &mt_stop) / USECS_PER_MSEC); } + + poll_futures(); continue; } @@ -334,6 +337,7 @@ bs_run_timers(0); + poll_futures(); bs_sample_time(state); bs_call_callbacks(state, current_phase.seq); @@ -342,6 +346,7 @@ * block on the correct phase. */ current_phase.seq = BS_ON_EXIT; + poll_futures(); bs_sample_time(state); post_code(state->post_code); @@ -352,6 +357,7 @@ printk(BIOS_DEBUG, "BS: Exiting %s state.\n", state->name); + poll_futures(); bs_sample_time(state); bs_call_callbacks(state, current_phase.seq); @@ -364,6 +370,7 @@ current_phase.state_id = next_id; current_phase.seq = BS_ON_ENTRY; + poll_futures(); bs_sample_time(state); state->complete = 1; -- To view, visit https://review.coreboot.org/c/coreboot/+/56050 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I15e17ecb1094a4bfb7bf5cabe8c847b9e697ebf2 Gerrit-Change-Number: 56050 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: lib/future: Implement Futures API
by Raul Rangel (Code Review) 03 Jul '21

03 Jul '21

Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56048 ) Change subject: lib/future: Implement Futures API ...................................................................... lib/future: Implement Futures API This is implemented in coreboot instead of commonlib because commonlib doesn't have things like printk, die and stopwatch. A later CL will add calls to poll_futures to hardwaremain.c. BUG=b:177909625 TEST=boot guybrush and see payload read/decompress time drop 23ms. Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: I350aebcd07024a00b90495bf937f8bca5193d02f --- M src/lib/Kconfig M src/lib/Makefile.inc A src/lib/future.c 3 files changed, 134 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/56048/1 diff --git a/src/lib/Kconfig b/src/lib/Kconfig index 239f613..b556f7d 100644 --- a/src/lib/Kconfig +++ b/src/lib/Kconfig @@ -98,3 +98,11 @@ the associated CAR/SRAM size. In that case every single CBFS file lookup must re-read the same CBFS directory entries from flash to find the respective file. + +config FUTURES_EXECUTOR + bool + help + Enables the futures executor. The executor provides means to register + futures and poll them until completion. If this is disabled and + futures are in use, they must be polled manually. + diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc index baf9c4d..0ae1304 100644 --- a/src/lib/Makefile.inc +++ b/src/lib/Makefile.inc @@ -271,6 +271,13 @@ ramstage-y += crc_byte.c smm-y += crc_byte.c +bootblock-y += future.c +verstage-y += future.c +romstage-y += future.c +postcar-y += future.c +ramstage-y += future.c +smm-y += future.c + postcar-y += bootmode.c postcar-y += boot_device.c postcar-y += cbfs.c diff --git a/src/lib/future.c b/src/lib/future.c new file mode 100644 index 0000000..902f1e0 --- /dev/null +++ b/src/lib/future.c @@ -0,0 +1,119 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <commonlib/bsd/cb_err.h> +#include <commonlib/bsd/future.h> +#include <console/console.h> +#include <stdbool.h> +#include <timer.h> +#include <types.h> + +/* + * This is a very trivial implementation. If we ever need a very large number of + * concurrent futures we can implement a better data structure. + */ +static struct future *futures[8]; +size_t registered_futures; + +void register_future(struct future *future) +{ + if (!CONFIG(FUTURES_EXECUTOR)) + return; + + for (size_t i = 0; i < ARRAY_SIZE(futures); ++i) { + if (futures[i]) + continue; + futures[i] = future; + registered_futures++; + + /* + * We don't poll the future since new futures should already be + * running. + */ + return; + } + die("%s: Polling list is full\n"); +} + +void poll_futures(void) +{ + if (!CONFIG(FUTURES_EXECUTOR)) + return; + + enum future_state state; + + /* Check for re-entry */ + static bool polling = 0; + + /* + * We shouldn't have future produces polling the global list. This + * can cause a circular loop. We could break the loop by removing the + * current future from the list before calling it, but we should just + * avoid this pattern. + */ + if (polling) + die("%s: A futures produces tried to poll the global queue."); + + /* Optimization so we don't always traverse the list */ + if (!registered_futures) + return; + + for (size_t i = 0; i < ARRAY_SIZE(futures); ++i) { + if (!futures[i]) + continue; + + /* + * We only ever perform a non-blocking poll. We leave the + * blocking poll for the future consumer. + */ + state = futures[i]->poll(futures[i], false); + if (state == FUTURE_DONE) { + futures[i] = NULL; + registered_futures--; + } + } +} + +static void unregister_future(struct future *future) +{ + if (!CONFIG(FUTURES_EXECUTOR)) + return; + + for (size_t i = 0; i < ARRAY_SIZE(futures); ++i) { + if (futures[i] == future) { + futures[i] = NULL; + registered_futures--; + return; + } + } +} + +void wait_for_future(struct future *future) +{ + struct stopwatch sw; + + stopwatch_init(&sw); + + printk(BIOS_DEBUG, "waiting for future %p\n", future); + + /* + * Unregister the current future so we don't poll it twice while + * busy waiting. + */ + unregister_future(future); + + while (future->poll(future, true) != FUTURE_DONE) + poll_futures(); + + printk(BIOS_DEBUG, " took %lu us\n", stopwatch_duration_usecs(&sw)); +} + +static inline enum future_state future_complete(struct future *future, bool busy_loop) +{ + return FUTURE_DONE; +} + +void complete_future(struct future *future, cb_err_t err) +{ + future->error = err; + future->poll = future_complete; +} -- To view, visit https://review.coreboot.org/c/coreboot/+/56048 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I350aebcd07024a00b90495bf937f8bca5193d02f Gerrit-Change-Number: 56048 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: cbfs: Introduce and implement cbfs_load_async
by Raul Rangel (Code Review) 03 Jul '21

03 Jul '21

Attention is currently required from: Julius Werner. Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56049 ) Change subject: cbfs: Introduce and implement cbfs_load_async ...................................................................... cbfs: Introduce and implement cbfs_load_async This CL adds support for the Futures API to cbfs. This allows loading a file from CBFS asynchronously. It currently only supports uncompressed files. More APIs will be added as I start to load stages, oproms, etc asynchronously. BUG=b:179699789 TEST=Boot guybrush and see payload read/decompress drop by 23 ms. Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: Idf4cee8a7094a9470aff997e4b06e823cb4811c0 --- M src/include/cbfs.h M src/lib/cbfs.c 2 files changed, 184 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/56049/1 diff --git a/src/include/cbfs.h b/src/include/cbfs.h index 9ed5233..885e4d3 100644 --- a/src/include/cbfs.h +++ b/src/include/cbfs.h @@ -5,6 +5,7 @@ #include <cbmem.h> #include <commonlib/bsd/cbfs_mdata.h> +#include <commonlib/bsd/future.h> #include <commonlib/cbfs.h> #include <commonlib/mem_pool.h> #include <program_loading.h> @@ -58,6 +59,9 @@ * 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. + * + * There are also async flavors of the functions. These will read from the underlying data store + * asynchronously. */ /* @@ -97,6 +101,16 @@ static inline void *cbfs_ro_type_cbmem_alloc(const char *name, uint32_t cbmem_id, size_t *size_out, enum cbfs_type *type); +struct cbfs_load_async_context { + struct future future; + void *buffer; + size_t size; + enum cbfs_type type; +}; + +cb_err_t cbfs_load_async(const char *name, struct cbfs_load_async_context *context, void *buf, + size_t size); + /* 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); diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index 32ebfaf..d653c6d 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -304,6 +304,176 @@ return cbfs_map(name, NULL); } +struct cbfs_load_async_internal_context { + struct cbfs_load_async_context *public_context; + struct rdev_readat_async_context rdev_context; + uint8_t *buffer; + size_t buffer_size; + enum cbfs_type type; + enum cbfs_compression compression; + size_t compressed_size; + size_t decompressed_size; + const struct vb2_hash *file_hash; +}; + +/* + * We avoid malloc and statically allocate a few contexts. Since the list is currently + * so small we just iterate through the list. If we ever need to dramatically increase + * the number of entries, we should maybe use a better data structure and algorithms. + */ +static struct cbfs_load_async_internal_context async_contexts[4]; + +static struct cbfs_load_async_internal_context *get_free_context(void) +{ + struct cbfs_load_async_internal_context *context; + for (unsigned int i = 0; i < ARRAY_SIZE(async_contexts); ++i) { + context = &async_contexts[i]; + if (!context->public_context) { + memset(context, 0, sizeof(*context)); + return context; + } + } + + die("%s: All async_contexts are in use\n", __func__); +} + +static void free_context(struct cbfs_load_async_internal_context *context) +{ + context->public_context = NULL; +} +static struct cbfs_load_async_internal_context *find_context(struct future *future) +{ + struct cbfs_load_async_internal_context *context; + for (unsigned int i = 0; i < ARRAY_SIZE(async_contexts); ++i) { + context = &async_contexts[i]; + if (!context->public_context) + continue; + + if (&context->public_context->future == future) + return context; + } + + die("%s: Failed to locate context for future: %p\n", future); +} + +static enum future_state cbfs_load_poll_future(struct future *future, bool busy_loop) +{ + enum future_state state; + struct cbfs_load_async_internal_context *context = find_context(future); + struct future *rdev_future = &context->rdev_context.future; + + /* Check the rdev future, if it's not done, then we have nothing to do */ + state = rdev_future->poll(rdev_future, busy_loop); + if (state != FUTURE_DONE) + return state; + + if (rdev_future->error != CB_SUCCESS) { + ERROR("async readat failed\n"); + complete_future(future, rdev_future->error); + free_context(context); + return FUTURE_DONE; + } + + /* + * We only want to perform verification and/or decompression when + * the buffer is required. This way we can attribute the CPU time to + * the correct timestamps. + */ + if (!busy_loop) + return FUTURE_PENDING; + + if (context->file_hash) { + if (cbfs_file_hash_mismatch(context->buffer, context->compressed_size, + context->file_hash)) { + complete_future(future, CB_CBFS_HASH_MISMATCH); + free_context(context); + return FUTURE_DONE; + } + } + + // TODO: Add LZ4 decompression + assert(context->compression == CBFS_COMPRESS_NONE); + + /* Complete the future and populate the output values */ + context->public_context->buffer = context->buffer; + context->public_context->size = context->decompressed_size; + context->public_context->type = context->type; + + complete_future(future, CB_SUCCESS); + free_context(context); + + return FUTURE_DONE; +} + +cb_err_t cbfs_load_async(const char *name, struct cbfs_load_async_context *public_context, + void *buffer, size_t buffer_size) +{ + struct region_device rdev; + union cbfs_mdata mdata; + struct cbfs_load_async_internal_context *context; + uint32_t compression = CBFS_COMPRESS_NONE; + size_t decompressed_size; + const struct cbfs_file_attr_compression *cattr; + cb_err_t err; + + DEBUG("%s(name='%s', buffer=%p, buffer_size=%#zx)\n", __func__, name, buffer, + buffer_size); + + assert(public_context); + + public_context->future.poll = NULL; + + if (cbfs_boot_lookup(name, false, &mdata, &rdev)) { + ERROR("file was not found.\n"); + return CB_ERR_ARG; + } + + if (buffer_size < region_device_sz(&rdev)) { + ERROR("target buffer is not large enough.\n"); + return CB_ERR_ARG; + } + + cattr = cbfs_find_attr(&mdata, CBFS_FILE_ATTR_TAG_COMPRESSION, sizeof(*cattr)); + if (cattr) { + compression = be32toh(cattr->compression); + decompressed_size = be32toh(cattr->decompressed_size); + } else { + decompressed_size = region_device_sz(&rdev); + } + + /* TODO: Add support for LZ4. LZMA is tricky since it can't be decompressed in place */ + if (compression != CBFS_COMPRESS_NONE) { + LOG("compressed payloads are not currently supported.\n"); + return CB_ERR; + } + + context = get_free_context(); + context->public_context = public_context; + context->public_context->future.poll = cbfs_load_poll_future; + context->buffer = buffer; + context->buffer_size = buffer_size; + context->type = be32toh(mdata.h.type); + context->compression = compression; + context->compressed_size = region_device_sz(&rdev); + context->decompressed_size = decompressed_size; + + if (CONFIG(CBFS_VERIFICATION)) + context->file_hash = cbfs_file_hash(&mdata); + + DEBUG("Reading %zu bytes to %p asynchronously\n", context->compressed_size, + context->buffer); + + err = rdev_readat_async(&rdev, &context->rdev_context, buffer, 0, + context->compressed_size); + if (err) { + ERROR("Cannot perform async readat\n"); + public_context->future.poll = NULL; + free_context(context); + } + + return err; +} + void *_cbfs_alloc(const char *name, cbfs_allocator_t allocator, void *arg, size_t *size_out, bool force_ro, enum cbfs_type *type) { -- To view, visit https://review.coreboot.org/c/coreboot/+/56049 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idf4cee8a7094a9470aff997e4b06e823cb4811c0 Gerrit-Change-Number: 56049 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: commonlib/bsd/future: Introduce future API
by Raul Rangel (Code Review) 03 Jul '21

03 Jul '21

Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56047 ) Change subject: commonlib/bsd/future: Introduce future API ...................................................................... commonlib/bsd/future: Introduce future API This CL introduces a futures API. It allows modeling asynchronous operations. This will make it possible to model DMA transfers, I2C transactions, EC operations, etc. It was loosely modeled off the Rust Futures API: https://rust-lang.github.io/async-book/02_execution/02_future.html Since we don't use interrupts while booting, the only option we have is to poll. For this reason I omitted the `wake` callback since there is nothing to call it. I evaluated the existing threads.h API and it has the following down sides: * Each thread requires its own stack. This means that there is a pretty low limit to the number of concurrent async operations, and a lot of overhead. * There are no thread synchronization primitives (e.g., thread handles, join, etc), so it's difficult to wait for an asynchronous operation to complete. * It currently only works in ramstage. * It requires code to be reentrant since we can now have multiple threads of executing going through the same code. This will require adding support for mutexes to make sure critical sections are protected. Auditing this is complicated anid error prone. * It makes it hard to tell from looking at the code if something is running in a different thread. * Timestamp tracking becomes difficult because we can context switch to a task that hogs the CPU for a while. The futures API has the following advantages: * We can easily scale the number of async operations since each operation only needs to store the context to run its state machine. * It provides a synchronization primitive so it's possible to block until the operation is complete. * It works in all stages. * There is no need for mutexes since there is only 1 stack and thread of execution. * Async operations are explicitly modeled so it's clear if the code is asynchronous. * The busy_loop parameter allows putting off CPU intensive operations until they are required. This will allow for proper timestamp accounting. * Creating futures that depend on other futures is pretty simple. BUG=b:179699789 TEST=Implemented the API for rdev and cbfs, it seems pretty ergonomic Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: I125a946233f122c2848704296efbcb3ca3c96079 --- A src/commonlib/bsd/include/commonlib/bsd/future.h 1 file changed, 86 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/56047/1 diff --git a/src/commonlib/bsd/include/commonlib/bsd/future.h b/src/commonlib/bsd/include/commonlib/bsd/future.h new file mode 100644 index 0000000..1ec30c3 --- /dev/null +++ b/src/commonlib/bsd/include/commonlib/bsd/future.h @@ -0,0 +1,86 @@ +/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0-or-later */ + +#ifndef COMMONLIB_BSD_FUTURE_H +#define COMMONLIB_BSD_FUTURE_H + +#include <commonlib/bsd/cb_err.h> +#include <stdbool.h> + +enum future_state { + FUTURE_PENDING, + FUTURE_IN_PROGRESS, + FUTURE_DONE, +}; + +/** + * A future is a representation of an asynchronous operation. + * + * e.g., Using a DMA controller to move memory, performing an I2C transaction, + * starting an expensive EC operation, etc + * + * An output value is omitted from this structure. If an output value is + * required, it is expected this structure will be embedded inside another + * structure that contains the output. + */ +struct future { + /** + * Polls the state of the future. + * + * If the future has not terminated, it will advance the state of the + * future's state machine as much as possible. + * + * e.g., If a DMA controller has been asked to transfer 100 KiB of data + * but it has a hardware limit of 64 KiB, then the transfer needs to be + * split up into two transactions. We need to periodically poll the + * hardware to check if the transaction is done. Once it has completed + * the second transaction can then be started. We continue to + * periodically poll until the second transaction completes. Once the + * second transaction has completed, `FUTURE_DONE` will be returned. + * + * By only polling periodically, we allow the CPU is to continue + * performing other work. + * + * If this function pointer is NULL, the future is uninitialized. + * + * @param busy_loop if the consumer has reached a point where it can no + * longer proceed without the future completing, then this value + * should be set to true. It is an indication to the producer that it + * can now perform any CPU intensive tasks (e.g., hashing, + * decompression, etc). + */ + enum future_state (*poll)(struct future *future, bool busy_loop); + + /* Status of the future once `poll` has returned `FUTURE_DONE`. */ + enum cb_err error; +}; + +static inline bool future_is_valid(struct future *future) +{ + return !!future->poll; +} + +/** + * Registers the future with the executor so the future's poll method will be + * periodically called. Once the future completes it will automatically be + * unregistered. + * + * You should only register top level futures. If a future has dependent futures + * those dependents should be polled by the containing future and not the + * executor. + */ +void register_future(struct future *future); + +/** + * Polls all the registered futures. + * + * This method should not be called by any future producers. + */ +void poll_futures(void); + +/* Wait until the future completes */ +void wait_for_future(struct future *future); + +/* Marks the future as complete */ +void complete_future(struct future *future, cb_err_t err); + +#endif /* COMMONLIB_BSD_FUTURE_H */ -- To view, visit https://review.coreboot.org/c/coreboot/+/56047 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I125a946233f122c2848704296efbcb3ca3c96079 Gerrit-Change-Number: 56047 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/amd/cezanne: Start loading APOB asynchronously while FSP-M executes
by Raul Rangel (Code Review) 03 Jul '21

03 Jul '21

Attention is currently required from: Jason Glenesk, Marshall Dawson, Felix Held. Hello build bot (Jenkins), Jason Glenesk, Marshall Dawson, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/55859 to look at the new patch set (#3). Change subject: soc/amd/cezanne: Start loading APOB asynchronously while FSP-M executes ...................................................................... soc/amd/cezanne: Start loading APOB asynchronously while FSP-M executes In the S3 resume case we will block in `soc_fill_apob_cache` until the whole buffer has been read since the PSP has not placed the APOB contents into RAM. In the non-S3 case, the PSP has placed the APOB contents into RAM, so `soc_fill_apob_cache` uses that for FSP-M. By reading the SPI contents while FSP-M is executing, the data will be ready by the time `soc_update_apob_cache` executes. We currently get lucky that the APOB is only 64 KiB since the SPI DMA controller can perform this transaction in one shot. In order to support larger payloads, or multiple transactions while FSP-M executes, we will need FSP-M to call `poll_futures` periodically. BUG=b:179699789 TEST=Boot guybrush and verify APOB read timestamp has dropped from 10ms to a few uS. FMAP: area RW_MRC_CACHE found @ 0 (65536 bytes) spi_dma_readat_async: dest: 0x020935c0, offset: 0x0, size: 65536 spi_dma_async_poll: current_transaction: 0x020a3620 start_spi_dma_transaction: dest: 0x020935c0, offset: 0x0, remaining: 65536 <FSP-M executes> wait_for_promise: waiting for promise spi_dma_async_poll: current_transaction: 0x020a3620 continue_spi_dma_transaction: dest: 0x020935c0, offset: 0x0, remaining: 65536 spi_dma_async_poll: transaction complete spi_dma_async_poll: current_transaction: 0x00000000 wait_for_promise: took 12 us APOB valid copy is already in flash Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: I4b5c1ef4cad571d1cbca33b1aff017a3cedc1be8 --- M src/soc/amd/cezanne/fsp_m_params.c 1 file changed, 14 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/55859/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/55859 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4b5c1ef4cad571d1cbca33b1aff017a3cedc1be8 Gerrit-Change-Number: 55859 Gerrit-PatchSet: 3 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/amd/common/block/lpc/spi_dma: Implement readat_async
by Raul Rangel (Code Review) 03 Jul '21

03 Jul '21

1 0

Change in coreboot[master]: commonlib/region: Add support for asynchronous readat
by Raul Rangel (Code Review) 03 Jul '21

03 Jul '21

Attention is currently required from: Furquan Shaikh, Marshall Dawson, Julius Werner, Angel Pons. Hello build bot (Jenkins), Furquan Shaikh, Marshall Dawson, Julius Werner, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/55856 to look at the new patch set (#2). Change subject: commonlib/region: Add support for asynchronous readat ...................................................................... commonlib/region: Add support for asynchronous readat This CL adds support asynchronous readat support for region_devices. It uses the new futures API. Asynchronous operations be be performed in the background leaving the AP free to perform other work. AMD SoCs have a dedicated SPI DMA controller that can be used to perform asynchronous reads. The goal is to load certain stages, payloads, and oproms asynchronously into a cache before the items are actually needed. This will reduce the amount of time we spend waiting for SPI transactions. BUG=b:179699789 TEST=compiles Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: I9702c9e6abe992ea401fa08ba71e5080c3c29d9a --- M src/commonlib/include/commonlib/region.h M src/commonlib/region.c 2 files changed, 50 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/55856/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/55856 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9702c9e6abe992ea401fa08ba71e5080c3c29d9a Gerrit-Change-Number: 55856 Gerrit-PatchSet: 2 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/amd/common/apob: Add support for asynchronously reading APOB
by Raul Rangel (Code Review) 03 Jul '21

03 Jul '21

Attention is currently required from: Jason Glenesk, Marshall Dawson, Felix Held. Hello build bot (Jenkins), Jason Glenesk, Marshall Dawson, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/55858 to look at the new patch set (#3). Change subject: soc/amd/common/apob: Add support for asynchronously reading APOB ...................................................................... soc/amd/common/apob: Add support for asynchronously reading APOB This CL adds a method that can starts the processes of reading the APOB from SPI. It does take more RAM in romstage since we no longer mmap the buffer in the async happy path. This will allow us to reduce our boot time by 10ms. We don't register the future with the executor since it's not currently getting called in romstage, and the transfer completes in a single transaction. BUG=b:179699789 TEST=With this and the FSP patch above I can see a 10 ms reduction in boot time on guybrush. Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: I930d58b76eb4558bc4f48ed928c4d6538fefbde7 --- M src/soc/amd/common/block/apob/apob_cache.c M src/soc/amd/common/block/include/amdblocks/apob_cache.h 2 files changed, 33 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/55858/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/55858 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I930d58b76eb4558bc4f48ed928c4d6538fefbde7 Gerrit-Change-Number: 55858 Gerrit-PatchSet: 3 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2021