coreboot-gerrit April 2025

coreboot-gerrit@coreboot.org

1 participants
3140 discussions

[L] Change in coreboot[main]: Documentation: Add Timestamp documentation
by Martin L Roth (Code Review) 06 Apr '25

06 Apr '25

Attention is currently required from: Nicholas Chin. Martin L Roth has posted comments on this change by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/87186?usp=email ) Change subject: Documentation: Add Timestamp documentation ...................................................................... Patch Set 3: (2 comments) File Documentation/internals/timestamps.md: https://review.coreboot.org/c/coreboot/+/87186/comment/fe7ea68e_da98886e?us… : PS2, Line 1: # coreboot Timestamp Implementation : : : : ## Introduction > Remove extra blank lines here […] Done https://review.coreboot.org/c/coreboot/+/87186/comment/72786acf_51705068?us… : PS2, Line 166: ld > Seems like linker scripts aren't one of the 597 languages pygments supports for syntax highlighting, […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/87186?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6e4cc244edf6cc860cc66165173f134a30a81589 Gerrit-Change-Number: 87186 Gerrit-PatchSet: 3 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Sun, 06 Apr 2025 02:12:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>

1 0

[L] Change in coreboot[main]: Documentation: Add Timestamp documentation
by Martin L Roth (Code Review) 06 Apr '25

06 Apr '25

Attention is currently required from: Martin L Roth. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/87186?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: Documentation: Add Timestamp documentation ...................................................................... Documentation: Add Timestamp documentation Change-Id: I6e4cc244edf6cc860cc66165173f134a30a81589 Signed-off-by: Martin Roth <gaumless(a)gmail.com> --- M Documentation/internals/index.md A Documentation/internals/timestamps.md 2 files changed, 337 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/87186/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/87186?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6e4cc244edf6cc860cc66165173f134a30a81589 Gerrit-Change-Number: 87186 Gerrit-PatchSet: 3 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>

1 0

[XL] Change in coreboot[main]: Documentation/internals: Add devicetree language documentation
by Nicholas Chin (Code Review) 06 Apr '25

06 Apr '25

Attention is currently required from: Martin L Roth. Nicholas Chin has posted comments on this change by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/84710?usp=email ) Change subject: Documentation/internals: Add devicetree language documentation ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84710?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If868ad9a87cb2903bf144996fe3b87d29d4fc755 Gerrit-Change-Number: 84710 Gerrit-PatchSet: 4 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Comment-Date: Sat, 05 Apr 2025 22:44:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[L] Change in coreboot[main]: Documentation/internals: Add devicetree documentation
by Nicholas Chin (Code Review) 06 Apr '25

06 Apr '25

Attention is currently required from: Martin L Roth. Nicholas Chin has posted comments on this change by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/84709?usp=email ) Change subject: Documentation/internals: Add devicetree documentation ...................................................................... Patch Set 4: Code-Review+2 (1 comment) Patchset: PS1: > Needs a toctree entry in some index.md for it to show up. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/84709?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2a43a96911844bd2b682004d5423126ad00a4bf3 Gerrit-Change-Number: 84709 Gerrit-PatchSet: 4 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Comment-Date: Sat, 05 Apr 2025 22:43:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>

1 0

[L] Change in coreboot[main]: Documentation: Add Timestamp documentation
by Nicholas Chin (Code Review) 06 Apr '25

06 Apr '25

Attention is currently required from: Martin L Roth. Nicholas Chin has posted comments on this change by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/87186?usp=email ) Change subject: Documentation: Add Timestamp documentation ...................................................................... Patch Set 2: (2 comments) File Documentation/internals/timestamps.md: https://review.coreboot.org/c/coreboot/+/87186/comment/666d1f3e_9dd5d9ae?us… : PS2, Line 1: # coreboot Timestamp Implementation : : : : ## Introduction Remove extra blank lines here ```suggestion # coreboot Timestamp Implementation ## Introduction ``` https://review.coreboot.org/c/coreboot/+/87186/comment/2fd89bab_741d2ec4?us… : PS2, Line 166: ld Seems like linker scripts aren't one of the 597 languages pygments supports for syntax highlighting, which seems to be what Jenkins is complaining about. -- To view, visit https://review.coreboot.org/c/coreboot/+/87186?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6e4cc244edf6cc860cc66165173f134a30a81589 Gerrit-Change-Number: 87186 Gerrit-PatchSet: 2 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Comment-Date: Sat, 05 Apr 2025 22:36:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in coreboot[main]: Documentation/mainboard/lenovo: Add ThinkCentre M700/M900 Tiny
by Nicholas Chin (Code Review) 06 Apr '25

06 Apr '25

Attention is currently required from: Krystian Hebel, Maciej Pijanowski, Michał Kopeć, Michał Żygowski. Nicholas Chin has posted comments on this change by Michał Kopeć. ( https://review.coreboot.org/c/coreboot/+/87044?usp=email ) Change subject: Documentation/mainboard/lenovo: Add ThinkCentre M700/M900 Tiny ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/87044?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8c3860ba6254919769082c9ed749f8bb287a5f5e Gerrit-Change-Number: 87044 Gerrit-PatchSet: 4 Gerrit-Owner: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Comment-Date: Sat, 05 Apr 2025 22:22:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[L] Change in coreboot[main]: Documentation: Add Ramstage Bootstates
by Martin L Roth (Code Review) 06 Apr '25

06 Apr '25

Martin L Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/87189?usp=email ) Change subject: Documentation: Add Ramstage Bootstates ...................................................................... Documentation: Add Ramstage Bootstates Change-Id: I18801967be50e2f318b4404d08c171ffa7e92bbc Signed-off-by: Martin Roth <gaumless(a)gmail.com> --- M Documentation/internals/index.md A Documentation/internals/ramstage_bootstates.md 2 files changed, 515 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/87189/1 diff --git a/Documentation/internals/index.md b/Documentation/internals/index.md index 4b4d011..a4792bb 100644 --- a/Documentation/internals/index.md +++ b/Documentation/internals/index.md @@ -18,6 +18,7 @@ ```{toctree} :maxdepth: 1 +Ramstage Bootstates & Bootstate Callbacks <ramstage_bootstates.md> Timestamps <timestamps.md> Timers, Stopwatch, and delays <stopwatch.md> Threads <threads.md> diff --git a/Documentation/internals/ramstage_bootstates.md b/Documentation/internals/ramstage_bootstates.md new file mode 100644 index 0000000..e77ecf2 --- /dev/null +++ b/Documentation/internals/ramstage_bootstates.md @@ -0,0 +1,514 @@ +# coreboot Ramstage Bootstates & Bootstate Callbacks + +## Introduction + +The coreboot boot process is divided into several discrete phases, one +of which is **ramstage**. Ramstage is the phase where the main hardware +initialization and device setup occurs after memory initialization. +Within ramstage, a state machine called the **bootstate machine** +manages the sequence of operations needed to initialize the system, +configure devices, and prepare to load and execute the payload (such as +a bootloader, operating system, or firmware utility). + +The bootstate machine provides a structured and extensible way to +organize code execution during the boot process. It allows for clear +separation of concerns between different initialization phases and +provides hooks for component-specific code to run at well-defined +points. + +**Important Note:** The exact execution order of multiple callbacks +registered for the same state and sequence (entry/exit) is not +guaranteed. This means that you cannot depend on one call for the +state/sequence in any other calls to the same state/sequence. If this +ordering is required, join the calls to the two functions into a single +function which specifies the order and create a callback to call the +top-level function instead of the two individual callbacks. + + +## Bootstate Machine Architecture + +The bootstate machine's public API is defined in +`src/include/bootstate.h`, and its core implementation resides in +`src/lib/hardwaremain.c`. At its core, it consists of: + +1. A series of sequential states that represent phases of the boot process +2. A mechanism for callback registration to execute code during state transitions +3. A framework for blocking and unblocking state transitions +4. Timing and debugging facilities to measure and report performance during boot + + +### Key Data Structures + +The primary public data structure for interacting with the bootstate +machine is `struct boot_state_callback`. The internal implementation +also uses `struct boot_state` and `struct boot_phase`. + + +#### Boot State Callback (Public API) + +Callbacks that run during state transitions are defined by this +structure in `src/include/bootstate.h`: + +```c +struct boot_state_callback { + void *arg; // Argument to pass to the callback + void (*callback)(void *arg); // Function pointer to the callback + struct boot_state_callback *next; // Next callback in linked list (internal use) +#if CONFIG(DEBUG_BOOT_STATE) + const char *location; // Source location for debugging +#endif +}; +``` + +#### Boot State Sequence (Public API) + +The boot state sequence type, defined in `src/include/bootstate.h`, +specifies when a callback should run relative to the state's main +action: + +```c +typedef enum { + BS_ON_ENTRY, // Execute before state function + BS_ON_EXIT // Execute after state function +} boot_state_sequence_t; +``` + + +#### Boot State (Internal Implementation) + +The main internal data structure in `src/lib/hardwaremain.c` is +`struct boot_state`, which defines a single state in the bootstate +machine: + +```c +struct boot_state { + const char *name; // Human-readable name of the state + boot_state_t id; // Enumerated identifier for the state + u8 post_code; // POST code to output during state execution + struct boot_phase phases[2]; // Entry and exit phases (internal use) + boot_state_t (*run_state)(void *arg); // Function to execute during the state + void *arg; // Argument to pass to the run_state function + int num_samples; // Counter for timing samples (internal use) + bool complete; // Flag indicating if state has completed (internal use) +}; +``` + + +#### Boot Phase (Internal Implementation) + +Each boot state has two internal phases ("entry" and "exit") represented +by `struct boot_phase` in `src/lib/hardwaremain.c`: + +```c +struct boot_phase { + struct boot_state_callback *callbacks; // Linked list of callbacks + int blockers; // Counter for blocking state transition +}; +``` + +## Bootstate Sequence + +The bootstate machine defines the following sequence of states, executed +in order by the `bs_walk_state_machine` function in +`src/lib/hardwaremain.c`. The sequence is defined by the `boot_state_t` +enum in `src/include/bootstate.h`: + +1. **BS_PRE_DEVICE**: Initial state before any device operations begin +2. **BS_DEV_INIT_CHIPS**: Early chip initialization for critical components +3. **BS_DEV_ENUMERATE**: Device enumeration (discovering devices on buses) +4. **BS_DEV_RESOURCES**: Resource allocation for devices +5. **BS_DEV_ENABLE**: Enabling devices that were discovered +6. **BS_DEV_INIT**: Device initialization +7. **BS_POST_DEVICE**: All device operations have been completed +8. **BS_OS_RESUME_CHECK**: Check if we're resuming from a sleep state +9. **BS_OS_RESUME**: Handle OS resume process (if needed) +10. **BS_WRITE_TABLES**: Write system tables (e.g., ACPI, SMBIOS) +11. **BS_PAYLOAD_LOAD**: Load the payload into memory +12. **BS_PAYLOAD_BOOT**: Boot the payload + +This sequence forms the backbone of the ramstage execution flow. Each +state performs a specific task, runs associated callbacks, and +transitions to the next state upon completion, unless blocked. + + +## Bootstate Details + +### BS_PRE_DEVICE + +**Purpose**: Serves as the initial state before any device tree +operations begin. + +**Key Functions**: +- `bs_pre_device()`: Sets up initial environment and transitions to next + state. + +**Usage**: This state is used for initializing core components that need +to be set up before any device operations. Examples include: +- Setting up global NVRAM variables +- Initializing debugging facilities +- Preparing ACPI tables or other critical system structures + + +### BS_DEV_INIT_CHIPS + +**Purpose**: Initializes critical chips early in the boot process. + +**Key Functions**: +- `bs_dev_init_chips()`: Calls `dev_initialize_chips()` to initialize + all chips in the device tree. + +**Notes**: Chip initialization can disable unused devices, which is why +it happens before device enumeration. + + +### BS_DEV_ENUMERATE + +**Purpose**: Discovers devices in the system. + +**Key Functions**: +- `bs_dev_enumerate()`: Calls `dev_enumerate()` to probe and identify + devices. + +**Notes**: During this phase, the system scans buses and detects +connected devices. + + +### BS_DEV_RESOURCES + +**Purpose**: Allocates and assigns resources (I/O, memory, IRQs) to +devices. + +**Key Functions**: +- `bs_dev_resources()`: Calls `dev_configure()` to compute and assign + bus resources. + +**Notes**: Resource allocation resolves conflicts and ensures each +device has the resources it needs. + + +### BS_DEV_ENABLE + +**Purpose**: Enables devices in the system. + +**Key Functions**: +- `bs_dev_enable()`: Calls `dev_enable()` to enable devices on the bus. + +**Notes**: Some devices may be selectively disabled based on hardware +configuration or policy. + + +### BS_DEV_INIT + +**Purpose**: Initializes enabled devices. + +**Key Functions**: +- `bs_dev_init()`: Calls `dev_initialize()` to initialize devices on the + bus. + +**Notes**: This state performs device-specific initialization routines +for all enabled devices. + + +### BS_POST_DEVICE + +**Purpose**: Final state after all device operations have completed. + +**Key Functions**: +- `bs_post_device()`: Calls `dev_finalize()` to complete any final + device operations. + +**Notes**: This state serves as a checkpoint that all device +initialization is complete. + + +### BS_OS_RESUME_CHECK + +**Purpose**: Checks if the system should resume from a sleep state. + +**Key Functions**: +- `bs_os_resume_check()`: Looks for a wake vector to determine if resume + is needed. + +**Notes**: This state branches the boot flow based on whether the system +is resuming from a sleep state. + +### BS_OS_RESUME + +**Purpose**: Handles the OS resume process. + +**Key Functions**: +- `bs_os_resume()`: Calls `acpi_resume()` with the wake vector to resume + the OS. + +**Notes**: After successful resume, control is transferred to the OS and +does not return to coreboot. + + +### BS_WRITE_TABLES + +**Purpose**: Writes configuration tables for the payload or OS. + +**Key Functions**: +- `bs_write_tables()`: Calls `write_tables()` to generate system tables. + +**Notes**: Tables include ACPI, SMBIOS, and other system configuration +data. + + +### BS_PAYLOAD_LOAD + +**Purpose**: Loads the payload into memory. + +**Key Functions**: +- `bs_payload_load()`: Calls `payload_load()` to load the payload. + +**Notes**: The payload could be a bootloader, an operating system kernel, +or a firmware utility. + + +### BS_PAYLOAD_BOOT + +**Purpose**: Final state that boots the loaded payload. + +**Key Functions**: +- `bs_payload_boot()`: Calls `payload_run()` to execute the payload. + +**Notes**: After successful execution, control is transferred to the +payload and does not return to coreboot. If execution returns (which +indicates an error), a boot failure message is printed. + + +## Driving the State Machine + +The state machine is driven by the `main()` function in +`src/lib/hardwaremain.c`. After initial setup (like initializing the +console and CBMEM), it calls `bs_walk_state_machine()`. + +`bs_walk_state_machine()` loops through the defined boot states: +1. It identifies the current state. +2. Runs all `BS_ON_ENTRY` callbacks for that state. +3. Executes the state's specific function (e.g., `bs_dev_enumerate()`). +4. Runs all `BS_ON_EXIT` callbacks for that state. +5. Transitions to the next state returned by the state function. + +This loop continues until the final state (`BS_PAYLOAD_BOOT` or +`BS_OS_RESUME`) transfers control away from coreboot. + + +## External Functions (Public API) + +The bootstate machine provides several functions in +`src/include/bootstate.h` for interacting with states: + + +### Callback Registration + +```c +int boot_state_sched_on_entry(struct boot_state_callback *bscb, boot_state_t state_id); +``` +Schedules a callback to run when entering a state (`BS_ON_ENTRY`). + +```c +int boot_state_sched_on_exit(struct boot_state_callback *bscb, boot_state_t state_id); +``` +Schedules a callback to run when exiting a state (`BS_ON_EXIT`). + + +### State Transition Control + +```c +int boot_state_block(boot_state_t state, boot_state_sequence_t seq); +``` +Blocks a state transition from occurring after the specified sequence +(entry or exit callbacks). The transition will pause until the block is +removed. + +```c +int boot_state_unblock(boot_state_t state, boot_state_sequence_t seq); +``` +Removes a previously set block on a state transition. + + +### Static Callback Registration + +For registering callbacks at compile time, use the `BOOT_STATE_INIT_ENTRY` +macro defined in `src/include/bootstate.h`: + +```c +BOOT_STATE_INIT_ENTRY(state, when, func, arg) +``` + +This macro creates a static entry in a special section (`.bs_init`) of +the binary. These entries are processed early in `main()` by +`boot_state_schedule_static_entries()` to register the callbacks before +the state machine starts running. + + +## Configuration Options + +The bootstate machine behavior can be modified through Kconfig options: + + +### DEBUG_BOOT_STATE + +``` +config DEBUG_BOOT_STATE + bool "Debug boot state machine" + default n + help + Control debugging of the boot state machine. When selected displays + the state boundaries in ramstage. +``` + +When enabled, this option causes the bootstate machine to output +debugging information via `printk`, including: +- State transition notifications (`Entering/Exiting <state> state.`) +- Callback execution details (address, source location, execution time) +- Timing information for state execution phases (entry, run, exit) + + +## Examples + +### Adding a New Bootstate Callback + +To register a function to be called when entering a specific state using +the static registration method: + +```c +// Function to be called +static void my_init_function(void *arg) +{ + // Initialization code + printk(BIOS_DEBUG, "My initialization running...\n"); +} + +// Register the callback at compile time +BOOT_STATE_INIT_ENTRY(BS_DEV_INIT, BS_ON_ENTRY, my_init_function, NULL); +``` + + +### Runtime Callback Registration + +For dynamic callback registration during runtime (e.g., within another +callback or state function): + +```c +static void runtime_init(void *arg) +{ + // Do something +} + +void register_my_callbacks(void) +{ + // Allocate or define a static callback structure + static struct boot_state_callback bscb = { + .callback = runtime_init, + .arg = NULL, + // .location is automatically handled if DEBUG_BOOT_STATE=y + }; + + // Schedule it + boot_state_sched_on_entry(&bscb, BS_DEV_ENABLE); +} +``` + + +### Blocking State Transition + +To temporarily block a state from progressing until a condition is met, +often used with timers: + +```c +#include <timer.h> // Required for timer functions + +static void wait_for_device(void *arg) +{ + if (!device_is_ready()) { + // Block the transition *after* BS_DEV_INIT exits + boot_state_block(BS_DEV_INIT, BS_ON_EXIT); + + // Schedule a function to check again later (e.g., after 100us) + // Assume schedule_timer exists and works appropriately + schedule_timer(check_device_ready, NULL, 100); + } +} + +static void check_device_ready(void *arg) +{ + if (device_is_ready()) { + // Device is ready, unblock the transition + boot_state_unblock(BS_DEV_INIT, BS_ON_EXIT); + } else { + // Still not ready, check again later + schedule_timer(check_device_ready, NULL, 100); + } +} + +// Register the initial check to run when entering BS_DEV_INIT +BOOT_STATE_INIT_ENTRY(BS_DEV_INIT, BS_ON_ENTRY, wait_for_device, NULL); +``` + + +## Best Practices + +### When Working with Bootstates + +1. **Choose the appropriate state**: Register callbacks at the earliest + state where all dependencies are guaranteed to be initialized, but no + earlier. Check the state descriptions and the functions called by + each state function (`bs_*`) in `hardwaremain.c`. + +2. **Keep callbacks focused**: Each callback should perform a specific, + related task and avoid complex operations that might significantly + delay the boot process. + +3. **Consider dependencies carefully**: Ensure any hardware, data + structures, or other resources your callback needs are available and + initialized at the chosen state and sequence (`BS_ON_ENTRY` vs. + `BS_ON_EXIT`). + +4. **Do not rely on callback order**: Remember that the execution order + of callbacks within the same state and sequence is not guaranteed. + Callbacks should be self-contained and not depend on side effects from + other callbacks that might run before or after them in the same phase. + +5. **Use blocking sparingly**: The blocking mechanism is powerful for + synchronization but can complicate the boot flow and make debugging + harder if overused. Always ensure a corresponding `boot_state_unblock` + call will eventually run. + +6. **Leverage compile-time registration**: Prefer using + `BOOT_STATE_INIT_ENTRY` for callbacks whenever possible. It makes the + registration explicit and easier to find. Runtime registration is + necessary only when the need for the callback is determined dynamically. + +7. **Debug with timestamps and `DEBUG_BOOT_STATE`**: Use the timestamp API + (`timestamp_add_now()`) and enable `DEBUG_BOOT_STATE` to measure + callback execution time, identify bottlenecks, and understand the + flow during development. + +8. **Document state-specific behavior**: When adding callbacks, add + comments explaining why they are placed in a particular state and + sequence. + +9. **Be careful with late states**: Avoid registering non-essential + callbacks in `BS_PAYLOAD_BOOT` or `BS_OS_RESUME`. Callbacks on + `BS_ON_EXIT` for these states are disallowed by compile-time asserts, + as coreboot is about to transfer control. + + +## Related Documentation + +- [Boot Stages in coreboot](https://doc.coreboot.org/getting_started/architecture.html): + Overview of all coreboot boot stages. + + +## References + +- `src/include/bootstate.h`: Public API definitions (callbacks, enums, + scheduling/blocking functions, static registration macro). +- `src/lib/hardwaremain.c`: Internal implementation (state machine driver, + state definitions, state functions). +- `src/ec/google/wilco/chip.c`: Example of bootstate callback usage. +- `src/mainboard/prodrive/hermes/mainboard.c`: Examples of mainboard-specific + bootstate callbacks. -- To view, visit https://review.coreboot.org/c/coreboot/+/87189?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I18801967be50e2f318b4404d08c171ffa7e92bbc Gerrit-Change-Number: 87189 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>

1 0

[L] Change in coreboot[main]: Documentation/internals: Add devicetree documentation
by Martin L Roth (Code Review) 06 Apr '25

06 Apr '25

Attention is currently required from: Martin L Roth, Nicholas Chin. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84709?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: Documentation/internals: Add devicetree documentation ...................................................................... Documentation/internals: Add devicetree documentation Signed-off-by: Martin Roth <gaumless(a)gmail.com> Change-Id: I2a43a96911844bd2b682004d5423126ad00a4bf3 --- M Documentation/index.md A Documentation/internals/devicetree.md A Documentation/internals/index.md 3 files changed, 698 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/84709/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/84709?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2a43a96911844bd2b682004d5423126ad00a4bf3 Gerrit-Change-Number: 84709 Gerrit-PatchSet: 4 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>

1 0

[L] Change in coreboot[main]: Documentation: Add Timers, Stopwatch, and Delays
by Martin L Roth (Code Review) 06 Apr '25

06 Apr '25

Attention is currently required from: Martin L Roth. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/87187?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: Documentation: Add Timers, Stopwatch, and Delays ...................................................................... Documentation: Add Timers, Stopwatch, and Delays Change-Id: I3b58817c1ed06e6d7d5d5668b0e38ec8cfedf122 Signed-off-by: Martin Roth <gaumless(a)gmail.com> --- M Documentation/internals/index.md A Documentation/internals/stopwatch.md 2 files changed, 879 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/87187/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/87187?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3b58817c1ed06e6d7d5d5668b0e38ec8cfedf122 Gerrit-Change-Number: 87187 Gerrit-PatchSet: 2 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>

1 0

[L] Change in coreboot[main]: Documentation: Add Threads
by Martin L Roth (Code Review) 05 Apr '25

05 Apr '25

Martin L Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/87188?usp=email ) Change subject: Documentation: Add Threads ...................................................................... Documentation: Add Threads Change-Id: I49a273118f19e4b332ac7f51b1f34247acf66b85 Signed-off-by: Martin Roth <gaumless(a)gmail.com> --- M Documentation/internals/index.md A Documentation/internals/threads.md 2 files changed, 311 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/87188/1 diff --git a/Documentation/internals/index.md b/Documentation/internals/index.md index 098011c..4b4d011 100644 --- a/Documentation/internals/index.md +++ b/Documentation/internals/index.md @@ -20,5 +20,6 @@ Timestamps <timestamps.md> Timers, Stopwatch, and delays <stopwatch.md> +Threads <threads.md> ``` diff --git a/Documentation/internals/threads.md b/Documentation/internals/threads.md new file mode 100644 index 0000000..edbcb1d --- /dev/null +++ b/Documentation/internals/threads.md @@ -0,0 +1,310 @@ +# coreboot Threads + +## Thread Management + +coreboot provides a cooperative threading system that allows for +concurrent execution of tasks during the boot process. The thread API is +particularly useful for implementing asynchronous operations and +managing hardware initialization sequences. + + +### Thread Creation and Management + +#### thread_run + +```c +int thread_run(struct thread_handle *handle, enum cb_err (*func)(void *), void *arg) +``` +Creates and starts a new thread to execute the specified function. + +**Parameters:** +- `handle`: Pointer to a thread handle structure to track thread state + (Note: `struct thread_handle` is an opaque structure used by the API + to manage the thread's state.) +- `func`: Function to execute in the new thread +- `arg`: Argument to pass to the function + +**Returns:** +- 0 on success +- < 0 on failure + +**Example:** +```c +struct thread_handle th; +enum cb_err thread_func(void *arg) { + // Thread work here + return CB_SUCCESS; +} + +if (thread_run(&th, thread_func, NULL) < 0) { + printk(BIOS_ERR, "Failed to create thread\n"); +} else { + // Wait for the thread to complete and check its status + enum cb_err err = thread_join(&th); + if (err != CB_SUCCESS) { + printk(BIOS_ERR, "Thread failed with error %d\n", err); + } +} +``` + + +#### thread_run_until + +```c +int thread_run_until(struct thread_handle *handle, enum cb_err (*func)(void *), void *arg, + boot_state_t state, boot_state_sequence_t seq) +``` +Creates a thread that blocks boot state transitions until completion. + +**Parameters:** +- `handle`: Pointer to a thread handle structure +- `func`: Function to execute +- `arg`: Argument to pass to the function +- `state`: Boot state to block +- `seq`: Boot state sequence to block + +**Returns:** +- 0 on success +- < 0 on failure + +**Example:** +```c +struct thread_handle th; +enum cb_err init_func(void *arg) { + // Hardware initialization + return CB_SUCCESS; +} + +// Block BS_DEV_ENABLE until initialization completes +thread_run_until(&th, init_func, NULL, BS_DEV_ENABLE, 0); +``` + + +### Thread Synchronization + +#### thread_join + +```c +enum cb_err thread_join(struct thread_handle *handle) +``` +Waits for a thread to complete and returns its error code. + +**Parameters:** +- `handle`: Thread handle to wait for + +**Returns:** +- Thread's error code (e.g., `CB_SUCCESS`, `CB_ERR`). See + `src/include/cb_err.h` for details. + +**Example:** +```c +struct thread_handle th; +// ... create thread ... + +enum cb_err err = thread_join(&th); +if (err != CB_SUCCESS) { + printk(BIOS_ERR, "Thread failed with error %d\n", err); +} +``` + + +### Thread Yielding + +Yielding is crucial in a cooperative multitasking system like +coreboot's. Threads must explicitly yield control using `thread_yield` +or `thread_yield_microseconds` to allow other threads to run. Failure to +yield can lead to a single thread monopolizing the CPU, preventing other +tasks from executing. + +#### thread_yield + +```c +int thread_yield(void) +``` +Yields the current thread's execution to allow other threads to run. + +**Returns:** +- 0 on success +- < 0 if thread cannot yield + +**Example:** +```c +while (!condition) { + if (thread_yield() < 0) { + printk(BIOS_ERR, "Failed to yield thread\n"); + break; + } +} +``` + + +#### thread_yield_microseconds + +```c +int thread_yield_microseconds(unsigned int microsecs) +``` +Yields the current thread for a specified number of microseconds. + +**Parameters:** +- `microsecs`: Number of microseconds to yield + +**Returns:** +- 0 on success +- < 0 if thread cannot yield + +**Example:** +```c +// Wait for 100 microseconds +if (thread_yield_microseconds(100) < 0) { + printk(BIOS_ERR, "Failed to yield thread\n"); +} +``` + + +### Thread Cooperation Control + +#### thread_coop_enable + +```c +void thread_coop_enable(void) +``` +Enables cooperative behavior for the current thread. + +**Example:** +```c +thread_coop_enable(); // Allow thread to yield +``` + +#### thread_coop_disable + +```c +void thread_coop_disable(void) +``` +Disables cooperative behavior for the current thread. + +**Example:** +```c +thread_coop_disable(); // Prevent thread from yielding +``` + + +### Thread Mutexes + +#### thread_mutex_lock + +```c +void thread_mutex_lock(struct thread_mutex *mutex) +``` +Acquires a mutex lock, waiting if necessary. + +**Parameters:** +- `mutex`: Mutex to lock + +**Example:** +```c +struct thread_mutex mtx = THREAD_MUTEX_INITIALIZER; // Or = { .locked = false }; +thread_mutex_lock(&mtx); +// Critical section +thread_mutex_unlock(&mtx); +``` + + +#### thread_mutex_unlock + +```c +void thread_mutex_unlock(struct thread_mutex *mutex) +``` +Releases a mutex lock. + +**Parameters:** +- `mutex`: Mutex to unlock + +## Best Practices + +1. **Thread Safety**: + - Use mutexes to protect shared resources + - Be careful with global variables in threaded code + - Consider thread cooperation when implementing critical sections + +2. **Resource Management**: + - Always join threads that you create using `thread_run` to check + their completion status and clean up resources. Threads started + with `thread_run_until` are implicitly managed by the boot state + machine and typically do not require explicit joining. + - Consistently check return values from thread creation and operation + functions (like `thread_run`, `thread_yield`, `thread_join`) to + detect errors early. + - Clean up resources allocated or used within thread functions before + they exit. + +3. **Performance Considerations**: + - Use thread_yield_microseconds for precise timing + - Minimize time spent in critical sections + - Consider using thread_run_until for hardware initialization + +4. **Error Handling**: + - Check thread creation and operation return values (as noted in + Resource Management). + - Implement proper error handling within thread functions, returning + appropriate `cb_err` values. + - Use `thread_join` (for `thread_run` threads) to check the final + completion status. + + +## Common Patterns + +### Hardware Initialization + +```c +struct thread_handle init_th; +enum cb_err init_hardware(void *arg) { + // Initialize hardware + if (hardware_init() != 0) + return CB_ERR; + return CB_SUCCESS; +} + +// Run initialization in a thread +thread_run_until(&init_th, init_hardware, NULL, BS_DEV_ENABLE, 0); +``` + +### Asynchronous Operation + +```c +struct thread_handle async_th; +enum cb_err async_operation(void *arg) { + // Perform async operation + while (!operation_complete()) { + if (thread_yield() < 0) + return CB_ERR; + } + return CB_SUCCESS; +} + +// Start async operation +thread_run(&async_th, async_operation, NULL); +``` + + +### Critical Section Protection + +```c +struct thread_mutex resource_mtx = { .locked = false }; + +void access_shared_resource(void) { + thread_mutex_lock(&resource_mtx); + // Access shared resource + thread_mutex_unlock(&resource_mtx); +} +``` + +## Limitations + +1. The thread system is cooperative, not preemptive. +2. Threads must explicitly yield to allow other threads to run. +3. Thread operations are typically only available after RAM + initialization (in RAM stage and later). Check specific environment + constraints if unsure. +4. Thread count is limited by the `CONFIG_NUM_THREADS` Kconfig option. +5. Thread stack size is fixed by the `CONFIG_STACK_SIZE` Kconfig option. + -- To view, visit https://review.coreboot.org/c/coreboot/+/87188?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I49a273118f19e4b332ac7f51b1f34247acf66b85 Gerrit-Change-Number: 87188 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2025