coreboot-gerrit April 2025

coreboot-gerrit@coreboot.org

1 participants
3140 discussions

[S] Change in coreboot[main]: cpu/x86: Conditionally invalidate caches based on self-snooping support
by Jérémy Compostella (Code Review) 07 Apr '25

07 Apr '25

Attention is currently required from: Paul Menzel, Shuo Liu, Subrata Banik. Jérémy Compostella has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/87182?usp=email ) Change subject: cpu/x86: Conditionally invalidate caches based on self-snooping support ...................................................................... Patch Set 3: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/87182/comment/2bf586bc_220f88e3?us… : PS3, Line 16: Rationale: : : - I don't think the highlighted text brings anything. https://review.coreboot.org/c/coreboot/+/87182/comment/f7b82557_ac51e72a?us… : PS3, Line 37: Able to reduce : boot time by ~19-25ms. On all three platforms? -- To view, visit https://review.coreboot.org/c/coreboot/+/87182?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: If32439752d0ceaa03b1d81873ea0bc562092e9d5 Gerrit-Change-Number: 87182 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Comment-Date: Mon, 07 Apr 2025 19:08:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: src/arch/x86/c_start: Delete duplicated code masking stack pointer
by Jérémy Compostella (Code Review) 07 Apr '25

07 Apr '25

Attention is currently required from: NyeonWoo Kim, Shuo Liu. Jérémy Compostella has posted comments on this change by NyeonWoo Kim. ( https://review.coreboot.org/c/coreboot/+/87181?usp=email ) Change subject: src/arch/x86/c_start: Delete duplicated code masking stack pointer ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Could you track down to which commit the duplication got introduced and add it in the commit message ? -- To view, visit https://review.coreboot.org/c/coreboot/+/87181?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: I417ce90279fea4b00432e6a209f77a6dd0c0fee6 Gerrit-Change-Number: 87181 Gerrit-PatchSet: 1 Gerrit-Owner: NyeonWoo Kim Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: NyeonWoo Kim Gerrit-Comment-Date: Mon, 07 Apr 2025 18:57:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

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

07 Apr '25

Martin L Roth has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/87201?usp=email ) Change subject: Documentation: Add chip operations ...................................................................... Documentation: Add chip operations Change-Id: I5373eab2de2e255f9e3576794b9ad02d9711a6c2 Signed-off-by: Martin Roth <gaumless(a)gmail.com> --- A Documentation/internals/chip_operations.md M Documentation/internals/index.md 2 files changed, 538 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/87201/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/87201?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: I5373eab2de2e255f9e3576794b9ad02d9711a6c2 Gerrit-Change-Number: 87201 Gerrit-PatchSet: 2 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>

1 0

[S] Change in coreboot[main]: soc/amd/common/cpu/noncar: Compute core info
by Naresh Solanki (Code Review) 07 Apr '25

07 Apr '25

Attention is currently required from: Angel Pons, Fred Reitberger, Jason Glenesk, Marvin Drees, Matt DeVillier, Naresh Solanki. Hello Angel Pons, Fred Reitberger, Jason Glenesk, Marvin Drees, Matt DeVillier, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/85640?usp=email to look at the new patch set (#13). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: soc/amd/common/cpu/noncar: Compute core info ...................................................................... soc/amd/common/cpu/noncar: Compute core info In some SoC like Glinda, have 1. Different CPU cores have slightly different max boost frequency, 2. Multiple L3 cache blocks of different size identified by unique ID. Add helper function, 1. get_max_boost_frequency() to compute max boost frequenncy. 2. ap_stash_core_info() to update core_info struct with max boost frequency & all L3 cache block uniq ID & its size. Glinda SoC has multiple L3 caches, each identified by a unique cache UID. Each core is associated with a specific L3 cache, which can be determined based on the CPU core ID. The default implementation (x86_get_cpu_cache_info) retrieves cache information only for the current core and assumes that the same L3 cache is shared across all cores. To accurately determine the total L3 cache size: 1. Retrieves L3 cache information for each CPU core. 2. Identifies the unique cache ID associated with each core. 3. Aggregates cache sizes for all unique cache IDs to compute the total L3 cache size, ensuring correct summation even when L3 cache blocks have different sizes. TEST=Build for Glinda SoC, with L3 cache = 16MB + 8MB. Ran command 'dmidecode -t 7' & verified L3 cache is 24MB. Change-Id: I46947e8ac62c903036a81642e03201e353c3dac6 Signed-off-by: Naresh Solanki <naresh.solanki(a)9elements.com> --- M src/soc/amd/common/block/cpu/noncar/cpu.c M src/soc/amd/common/block/include/amdblocks/cpu.h 2 files changed, 37 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/85640/13 -- To view, visit https://review.coreboot.org/c/coreboot/+/85640?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: I46947e8ac62c903036a81642e03201e353c3dac6 Gerrit-Change-Number: 85640 Gerrit-PatchSet: 13 Gerrit-Owner: Naresh Solanki <naresh.solanki(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marvin Drees <marvin.drees(a)9elements.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Marvin Drees <marvin.drees(a)9elements.com> Gerrit-Attention: Naresh Solanki <naresh.solanki(a)9elements.com>

1 0

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

07 Apr '25

Martin L Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/87202?usp=email ) Change subject: Documentation: Add Device Operations ...................................................................... Documentation: Add Device Operations Change-Id: I3ed78f8ce50bb3914f55b2cbb7f5eb668706949a Signed-off-by: Martin Roth <gaumless(a)gmail.com> --- A Documentation/internals/device_operations.md M Documentation/internals/index.md 2 files changed, 697 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/87202/1 diff --git a/Documentation/internals/device_operations.md b/Documentation/internals/device_operations.md new file mode 100644 index 0000000..c0c98d0 --- /dev/null +++ b/Documentation/internals/device_operations.md @@ -0,0 +1,696 @@ +# Device Operations in coreboot Firmware + +## Introduction + +The `device_operations` structure is a cornerstone of coreboot's +hardware abstraction layer. It represents a set of function pointers +defining how the firmware interacts with a specific hardware device +during various boot stages. This structure lets the coreboot +architecture establish a consistent interface for device initialization, +configuration, resource allocation, and ACPI table generation, while +allowing device-specific implementations behind this common interface. + +At its core, `device_operations` applies the strategy pattern in +systems programming. It decouples algorithms (device operations) from +the core boot sequence, allowing devices to define their own behavior +while the boot process follows a predictable flow. This pattern enables +coreboot to support a wide range of hardware platforms with minimal +changes to the core boot sequence code. + + +## Structure Definition + +The `device_operations` structure, defined in +`src/include/device/device.h`, consists of several function pointers, +each representing a specific operation performed on a device during +boot: + +```c +struct device_operations { + void (*read_resources)(struct device *dev); + void (*set_resources)(struct device *dev); + void (*enable_resources)(struct device *dev); + void (*init)(struct device *dev); + void (*final)(struct device *dev); + void (*scan_bus)(struct device *bus); + void (*enable)(struct device *dev); + void (*vga_disable)(struct device *dev); + void (*reset_bus)(struct bus *bus); + + int (*get_smbios_data)(struct device *dev, int *handle, + unsigned long *current); + void (*get_smbios_strings)(struct device *dev, struct smbios_type11 *t); + + unsigned long (*write_acpi_tables)(const struct device *dev, + unsigned long start, struct acpi_rsdp *rsdp); + void (*acpi_fill_ssdt)(const struct device *dev); + const char *(*acpi_name)(const struct device *dev); + const char *(*acpi_hid)(const struct device *dev); + + const struct pci_operations *ops_pci; + const struct i2c_bus_operations *ops_i2c_bus; + const struct spi_bus_operations *ops_spi_bus; + const struct smbus_bus_operations *ops_smbus_bus; + const struct pnp_mode_ops *ops_pnp_mode; + const struct gpio_operations *ops_gpio; + const struct mdio_bus_operations *ops_mdio; +}; +``` + + +### Core Resource Management Functions + +* `read_resources`: Discovers and collects resources required by the + device (I/O ports, memory regions, IRQs, etc.). This typically + involves reading configuration registers or static device tree + information. +* `set_resources`: Assigns finalized resources to the device after the + resource allocation phase. This writes the assigned base addresses, + lengths, and other parameters back to the device structure, but not + necessarily to hardware registers yet. +* `enable_resources`: Activates the assigned resources in the device's + hardware registers (e.g., writing to PCI BARs, enabling memory + decoding). + + +### Device Lifecycle Functions + +* `init`: Performs device-specific initialization, often requiring + access to the device's assigned resources. This is called after + resources have been enabled. +* `final`: Executes final setup or cleanup operations before the + payload is loaded. This is useful for tasks that depend on other + devices being initialized. +* `enable`: Enables the device in the hardware, often setting bits in + configuration registers to make the device active. Called after + `enable_resources`. + + +### Bus Management Functions + +* `scan_bus`: Enumerates child devices on a bus device (e.g., scanning + a PCI bus for devices, probing I2C devices). Only applicable to + devices that act as buses. +* `reset_bus`: Resets all devices on a specific bus. +* `vga_disable`: Disables VGA decoding on a PCI device when another VGA + device is active. Used to manage legacy VGA resources. + + +### System Table Generation Functions + +* `get_smbios_data`: Provides SMBIOS data specific to the device for + Type 9 (System Slots) or Type 41 (Onboard Devices Extended + Information). +* `get_smbios_strings`: Supplies string information for SMBIOS tables, + often related to the data provided by `get_smbios_data`. +* `write_acpi_tables`: Generates device-specific ACPI tables (like SSDTs) + or contributes data to system-wide tables. +* `acpi_fill_ssdt`: Adds device-specific objects (scopes, methods, data) + to the Secondary System Description Table (SSDT). +* `acpi_name`: Returns the ACPI name for the device (e.g., + `\_SB.PCI0.GFX0`). This defines the device's path in the ACPI + namespace. +* `acpi_hid`: Returns the ACPI Hardware ID (HID) for the device (e.g., + `PNP0A08`). Used by the OS to match drivers. + + +### Bus-Specific Operation Pointers + +These fields point to bus-specific operation structures when a device +functions as a bus controller (or exposes bus-like functionality). See +the "Bus-Specific Operations" section for details. + +* `ops_pci`: Operations for PCI configuration space access. +* `ops_i2c_bus`: Operations for I2C bus transactions (read, write, + transfer). +* `ops_spi_bus`: Operations for SPI bus transactions. +* `ops_smbus_bus`: Operations for SMBus transactions. +* `ops_pnp_mode`: Operations for Plug-and-Play device configuration. +* `ops_gpio`: Operations for GPIO control (get, set, configure + direction/pulls). +* `ops_mdio`: Operations for MDIO (Management Data Input/Output) bus + access, used for Ethernet PHYs. + + +## Device Lifecycle in coreboot + +The function pointers in `device_operations` are called at specific +stages during the boot process, following a sequence defined in +coreboot's boot state machine (`src/lib/hardwaremain.c`). Understanding +this lifecycle helps developers implement appropriate behavior for each +function pointer. + + +### Boot Sequence and Device Operations + +coreboot's main device initialization sequence involves these boot +states: + +1. **BS_DEV_INIT_CHIPS** (`dev_initialize_chips()`): Initializes chip + drivers (`chip_operations`). +2. **BS_DEV_ENUMERATE** (`dev_enumerate()`): Discovers and enumerates + devices. + * Calls `scan_bus()` for each bus to detect child devices. +3. **BS_DEV_RESOURCES** (`dev_configure()`): Allocates resources across + all enumerated devices. + * Calls `read_resources()` for each device to discover required + resources. + * Calls `set_resources()` for each device to assign allocated + resources back to the `struct device`. +4. **BS_DEV_ENABLE** (`dev_enable()`): Enables devices and their + resources. + * Calls `enable_resources()` for each device to activate assigned + resources in hardware. + * Calls `enable()` for each device to perform general hardware + enablement. +5. **BS_DEV_INIT** (`dev_initialize()`): Initializes devices. + * Calls `init()` for each device to perform device-specific setup. +6. **BS_POST_DEVICE** (`dev_finalize()`): Finalizes devices before + payload loading. + * Calls `final()` for each device for any final cleanup or setup. + +The sequence is primarily driven by the `boot_states` array in +`src/lib/hardwaremain.c`: + +```c +static struct boot_state boot_states[] = { + /* ... other states ... */ + BS_INIT_ENTRY(BS_PRE_DEVICE, bs_pre_device), + BS_INIT_ENTRY(BS_DEV_INIT_CHIPS, bs_dev_init_chips), + BS_INIT_ENTRY(BS_DEV_ENUMERATE, bs_dev_enumerate), + BS_INIT_ENTRY(BS_DEV_RESOURCES, bs_dev_resources), + BS_INIT_ENTRY(BS_DEV_ENABLE, bs_dev_enable), + BS_INIT_ENTRY(BS_DEV_INIT, bs_dev_init), + BS_INIT_ENTRY(BS_POST_DEVICE, bs_post_device), + /* ... other states ... */ +}; +``` + +Later stages include ACPI and SMBIOS table generation, where functions +like `write_acpi_tables()`, `acpi_fill_ssdt()`, `get_smbios_data()`, and +`get_smbios_strings()` are invoked as part of the table construction +process. + + +## Inheritance and Code Reuse Patterns + +The `device_operations` structure enables several patterns for code +reuse: + + +### 1. Default Implementations + +coreboot provides default implementations for common device types (like +root devices, PCI devices, PCI bridges), which can be used directly or +extended. Chip or mainboard code often assigns these defaults if no +specific driver is found. + +```c +/* From src/device/root_device.c */ +struct device_operations default_dev_ops_root = { + .read_resources = noop_read_resources, + .set_resources = noop_set_resources, + .scan_bus = scan_static_bus, + .reset_bus = root_dev_reset, +#if CONFIG(HAVE_ACPI_TABLES) + .acpi_name = root_dev_acpi_name, +#endif +}; +``` + + +### 2. No-op Functions + +Simple shim functions (often static inline) are provided for cases where +a device doesn't need to implement specific operations. Using these avoids +leaving function pointers NULL. + +```c +/* From src/include/device/device.h */ +static inline void noop_read_resources(struct device *dev) {} +static inline void noop_set_resources(struct device *dev) {} +``` + + +### 3. Chain of Responsibility / Delegation + +Some implementations delegate to parent devices or use helper functions +when they can't handle an operation themselves or when common logic can +be shared. For example, ACPI name generation often traverses up the +device tree. + +```c +/* Simplified example logic */ +const char *acpi_device_name(const struct device *dev) +{ + const char *name = NULL; + const struct device *pdev = dev; + + /* Check for device specific handler */ + if (dev->ops && dev->ops->acpi_name) { + name = dev->ops->acpi_name(dev); + if (name) + return name; /* Device handled it */ + } + + /* Walk up the tree to find if any parent can provide a name */ + while (pdev->upstream && pdev->upstream->dev) { + pdev = pdev->upstream->dev; + if (pdev->ops && pdev->ops->acpi_name) { + /* Note: Parent's acpi_name might handle the original child 'dev' */ + name = pdev->ops->acpi_name(dev); + if (name) + return name; /* Parent handled it */ + } + } + + /* Fallback or default logic if needed */ + return NULL; +} +``` +This pattern allows parent devices (like buses) to provide default +behavior or naming schemes if a child device doesn't specify its own. + + +## Implementation Examples + +These examples show typical `device_operations` assignments. Actual +implementations might involve more conditional compilation based on +Kconfig options. + + +### PCI Device Operations (Default) + +```c +/* From src/device/pci_device.c */ +struct device_operations default_pci_ops_dev = { + .read_resources = pci_dev_read_resources, + .set_resources = pci_dev_set_resources, + .enable_resources = pci_dev_enable_resources, +#if CONFIG(HAVE_ACPI_TABLES) + .write_acpi_tables = pci_rom_write_acpi_tables, + .acpi_fill_ssdt = pci_rom_ssdt, +#endif + .init = pci_dev_init, + /* Assigns PCI-specific operations */ + .ops_pci = &pci_dev_ops_pci, +}; +``` + + +### CPU Cluster Operations + +```c +/* From src/soc/intel/alderlake/chip.c (representative example) */ +static struct device_operations cpu_bus_ops = { + .read_resources = noop_read_resources, + .set_resources = noop_set_resources, + .enable_resources = cpu_set_north_irqs, +#if CONFIG(HAVE_ACPI_TABLES) + .acpi_fill_ssdt = cpu_fill_ssdt, +#endif + /* CPU clusters often don't need scan_bus, init, etc. */ +}; +``` + + +### GPIO Controller Operations + +```c +/* From src/soc/intel/common/block/gpio/gpio_dev.c */ +static struct gpio_operations gpio_ops = { + .get = gpio_get, + .set = gpio_set, + /* ... other GPIO functions ... */ +}; + +struct device_operations block_gpio_ops = { + .read_resources = noop_read_resources, + .set_resources = noop_set_resources, + /* Assigns GPIO-specific operations */ + .ops_gpio = &gpio_ops, +}; +``` + + +## Registration and Discovery + +How are `device_operations` structures associated with `struct device` +instances? + + +### 1. Static Assignment (via `chip_operations`) + +For devices known at build time (defined in devicetree.cb), the +`device_operations` structure is often assigned in the SOC's or +mainboard's `chip_operations->enable_dev()` function based on the +device path type or other properties. + +```c +/* Example from src/soc/intel/alderlake/chip.c */ +static void soc_enable(struct device *dev) +{ + /* Assign ops based on the device's role in the tree */ + if (dev->path.type == DEVICE_PATH_DOMAIN) + dev->ops = &pci_domain_ops; /* Handles PCI domain resources */ + else if (dev->path.type == DEVICE_PATH_CPU_CLUSTER) + dev->ops = &cpu_bus_ops; /* Handles CPU cluster setup */ + else if (dev->path.type == DEVICE_PATH_GPIO) + block_gpio_enable(dev); /* Assigns block_gpio_ops */ + /* ... other assignments for specific PCI devices, etc. ... */ +} +``` +The `enable_dev` function is part of `struct chip_operations`, which +handles broader chip-level initialization (see "Relationship with +`chip_operations`" section). + + +### 2. Dynamic Detection (PCI Drivers) + +For PCI devices discovered during bus scanning (`scan_bus`), coreboot +looks through a list of registered PCI drivers (`_pci_drivers` array) +to find one matching the device's vendor and device IDs. + +```c +/* Logic from src/device/pci_device.c::set_pci_ops() */ +static void set_pci_ops(struct device *dev) +{ + struct pci_driver *driver; + + /* Check if ops already assigned (e.g., by chip_ops->enable_dev) */ + if (dev->ops) + return; + + /* Look through registered PCI drivers */ + for (driver = &_pci_drivers[0]; driver != &_epci_drivers[0]; driver++) { + if ((driver->vendor == dev->vendor) && + device_id_match(driver, dev->device)) { + /* Found a matching driver, assign its ops */ + dev->ops = (struct device_operations *)driver->ops; + printk(BIOS_SPEW, "%s: Assigned ops from driver for %04x:%04x\n", + dev_path(dev), driver->vendor, driver->device); + return; /* Stop searching */ + } + } + + /* Fall back to default operations if no specific driver found */ + if (!dev->ops) { + if ((dev->hdr_type & 0x7f) == PCI_HEADER_TYPE_BRIDGE) { + dev->ops = get_pci_bridge_ops(dev); /* Special ops for bridges */ + } else { + dev->ops = &default_pci_ops_dev; /* Default for normal devices */ + } + printk(BIOS_SPEW, "%s: Assigned default PCI ops\n", dev_path(dev)); + } +} +``` + + +## Build Process Integration + +The `device_operations` structures are integrated into the coreboot +build process: + +1. **Static Device Tree**: The mainboard's `devicetree.cb` defines the + initial device hierarchy. The build process converts this into C + code (`static.c`) containing `struct device` instances. +2. **PCI Driver Registration**: PCI drivers (containing their own + `device_operations`) register themselves using the `__pci_driver` + linker set macro. + + ```c + /* Example pattern */ + struct pci_driver example_pci_driver __pci_driver = { + .ops = &example_device_ops, /* Pointer to device_operations */ + .vendor = VENDOR_ID, + .device = DEVICE_ID, /* Or .devices for a list */ + }; + ``` +3. **Linking**: The build system collects all structures placed in the + `__pci_driver` section and creates the `_pci_drivers` array used by + `set_pci_ops()`. It ensures all necessary code (default ops, driver + ops, core device functions) is linked into the final firmware image. + + +## Relationship with `chip_operations` + +It's important to distinguish `device_operations` from +`chip_operations` (defined in `src/include/chip.h`). + +* `chip_operations`: Defines operations related to the overall chipset + or mainboard logic. It includes functions called earlier in the boot + process, like `enable_dev`, `init`, and `final`. + * `chip_operations->enable_dev()` is crucial as it often performs + initial setup for static devices and is the primary place where + `device_operations` pointers are *assigned* for non-PCI devices + based on their path or type. + * `chip_operations->init()` runs during `BS_DEV_INIT_CHIPS`, before + most `device_operations` functions. +* `device_operations`: Defines operations for *individual* devices + within the device tree. These are called *after* the corresponding + `chip_operations` stage and operate on a specific `struct device`. + +Essentially, `chip_operations` sets the stage at the SoC/mainboard level, +including assigning the correct `device_operations` to static devices, +while `device_operations` handles the specific actions for each device +later in the boot process. + + +## Bus-Specific Operations + +The `ops_*` pointers within `struct device_operations` (e.g., `ops_pci`, +`ops_i2c_bus`, `ops_spi_bus`, `ops_gpio`) provide a way for devices that +act as bus controllers or expose bus-like interfaces to offer +standardized access methods. + +* **Purpose:** They abstract the low-level details of interacting with + that specific bus type. For example, a PCI host bridge device will + implement `struct pci_operations` via its `ops_pci` pointer, + allowing other code to perform PCI config reads/writes through it + without knowing the exact hardware mechanism. Similarly, an I2C + controller device implements `struct i2c_bus_operations` via + `ops_i2c_bus` to provide standard `read`, `write`, and `transfer` + functions for that bus segment. +* **Usage:** Code needing to interact with a bus first finds the + controller `struct device` in the tree, then accesses the relevant + bus operations through the appropriate `ops_*` pointer, passing the + target address or parameters. For instance, to talk to an I2C device + at address `0x50` on the bus controlled by `i2c_controller_dev`, one + might call: + `i2c_controller_dev->ops->ops_i2c_bus->transfer(...)`. Helper + functions often wrap this access pattern. +* **Implementation:** The structures like `struct pci_operations`, + `struct i2c_bus_operations`, etc., are defined in corresponding + header files (e.g., `src/include/device/pci_ops.h`, + `src/include/drivers/i2c/i2c_bus.h`). Devices acting as controllers + provide concrete implementations of these functions, tailored to their + hardware. + +This mechanism allows coreboot to manage diverse bus types using a +consistent device model, where the controller device itself exposes the +necessary functions for interacting with devices on its bus. + + +## Best Practices + +When implementing `device_operations`: + +1. **Leverage Defaults/No-ops**: Use default or no-op implementations + whenever possible. Only override functions that require custom + behavior for your specific device. +2. **Error Handling**: Check return values from functions called within + your ops implementations and handle errors gracefully (e.g., log an + error, return an error code if applicable). +3. **Resource Management**: In `read_resources`, accurately declare all + resources (MMIO, I/O ports, IRQs) your device needs, specifying + flags like fixed vs. alignment, or bridge vs. standard device. + Incorrect resource declaration is a common source of issues. +4. **Initialization Order**: Be mindful of dependencies in `init`. If + your device relies on another device being fully initialized, consider + deferring that part of the initialization to the `final` callback, + which runs later. +5. **Minimal Implementation**: Only implement the functions relevant to + your device type. A simple MMIO device might only need + `read_resources`, `set_resources`, `enable_resources`, and perhaps + ACPI functions. A bus device additionally needs `scan_bus`. +6. **Bus Operations**: If implementing a bus controller, correctly + implement the corresponding bus operations structure (e.g., + `struct pci_operations`, `struct i2c_bus_operations`) and assign it + to the appropriate `ops_*` field. +7. **ACPI/SMBIOS**: If the device needs OS visibility via ACPI or + SMBIOS, implement the relevant functions (`acpi_name`, `acpi_hid`, + `acpi_fill_ssdt`, `get_smbios_data`, etc.). Ensure ACPI names and + HIDs are correct according to specifications and platform needs. + + +## Logging and Debugging + +Use coreboot's logging facilities (`printk`) within your `device_operations` +functions to provide visibility during development and debugging. Use +appropriate log levels (e.g., `BIOS_DEBUG`, `BIOS_INFO`, `BIOS_ERR`). + +```c +static void example_device_init(struct device *dev) +{ + printk(BIOS_DEBUG, "%s: Initializing device at %s\n", __func__, + dev_path(dev)); + + /* ... Device initialization code ... */ + if (/* some condition */) { + printk(BIOS_SPEW, "%s: Condition met, applying setting X\n", + dev_path(dev)); + /* ... */ + } + + if (/* error condition */) { + printk(BIOS_ERR, "%s: Failed to initialize feature Y!\n", + dev_path(dev)); + /* Handle error */ + } + + printk(BIOS_DEBUG, "%s: Initialization complete for %s\n", __func__, + dev_path(dev)); +} +``` +Consistent logging helps trace the boot process and pinpoint where issues +occur. + + +## Common Troubleshooting + +* **Missing Resource Declarations**: + * *Problem*: Device fails to function, or conflicts arise because a + required resource (MMIO range, I/O port, IRQ) was not declared + in `read_resources`. The resource allocator is unaware of the + need. + * *Solution*: Verify that `read_resources` correctly calls functions + like `pci_dev_read_resources` or manually adds all necessary + resources using functions like `mmio_resource()`, + `io_resource()`, etc. Check PCI BARs or device datasheets. +* **Initialization Order Issues**: + * *Problem*: `init()` fails because it depends on another device + that hasn't been fully initialized yet (e.g., accessing a shared + resource like SMBus before the SMBus controller is ready). + * *Solution*: Move the dependent initialization code to the `final` + callback if possible. Alternatively, ensure the dependency is met + by careful ordering in the device tree or using boot state + callbacks if necessary for complex scenarios. +* **Resource Conflicts**: + * *Problem*: Boot fails during resource allocation, or devices + misbehave because multiple devices requested the same + non-sharable resource (e.g., conflicting fixed MMIO regions). + * *Solution*: Review resource declarations in `read_resources` across + all relevant devices. Ensure fixed resources don't overlap. Check + if bridge windows are correctly defined and large enough. Use + coreboot's resource reporting logs to identify overlaps. +* **ACPI Table Generation Errors**: + * *Problem*: The operating system fails to recognize the device, + assigns the wrong driver, or the device doesn't function correctly + (e.g., power management issues). + * *Solution*: Double-check the `acpi_name`, `acpi_hid`, `_CRS` + (generated from assigned resources), and `acpi_fill_ssdt` + implementations. Verify names match the ACPI hierarchy and HIDs + match expected driver bindings. Ensure SSDT methods correctly + access hardware. Use OS debugging tools (e.g., `acpidump`, Device + Manager errors) to diagnose. +* **Incorrect `ops` Pointer Assigned**: + * *Problem*: Device behaves incorrectly because the wrong + `device_operations` structure was assigned (e.g., default PCI ops + assigned to a device needing a specific driver's ops). + * *Solution*: Check the logic in `chip_operations->enable_dev` (for + static devices) or the PCI driver registration (`__pci_driver` + macro and `set_pci_ops` fallback logic) to ensure the correct + `ops` structure is being selected and assigned based on device + type, path, or PCI ID. Add debug prints to verify which `ops` + structure is assigned. + + +## Advanced Usage + +### Complex Device Hierarchies + +For devices with non-standard interactions or complex initialization, +custom `device_operations` can be created, often inheriting from defaults +but overriding specific functions. + +```c +static void advanced_device_init(struct device *dev) +{ + /* First, perform standard PCI init */ + pci_dev_init(dev); + + /* Then, add custom initialization steps */ + printk(BIOS_DEBUG, "%s: Performing advanced init\n", dev_path(dev)); + /* ... custom register writes, configuration ... */ +} + +static const char *advanced_device_acpi_name(const struct device *dev) +{ + /* Provide a custom ACPI name based on some property */ + if (/* condition */) + return "ADV0001"; + else + return "ADV0002"; +} + +/* Combine default and custom operations */ +static struct device_operations advanced_device_ops = { + /* Inherit resource handling from default PCI ops */ + .read_resources = pci_dev_read_resources, + .set_resources = pci_dev_set_resources, + .enable_resources = pci_dev_enable_resources, + + /* Override init */ + .init = advanced_device_init, + + /* Override ACPI naming */ + .acpi_name = advanced_device_acpi_name, + /* Other functions might use defaults or no-ops */ +}; +``` + +### Dynamic Configuration based on Probing + +Some `init` or other op implementations might probe the device's +capabilities or read configuration data (e.g., from SPD, VPD, or straps) +and alter their behavior accordingly. + +```c +static void conditional_device_init(struct device *dev) +{ + uint8_t feature_flags; + + /* Read capability register from the device */ + feature_flags = pci_read_config8(dev, EXAMPLE_CAP_REG); + + printk(BIOS_DEBUG, "%s: Feature flags: 0x%02x\n", dev_path(dev), + feature_flags); + + /* Conditional initialization based on detected features */ + if (feature_flags & FEATURE_X_ENABLED) { + printk(BIOS_INFO, "%s: Initializing Feature X\n", dev_path(dev)); + init_feature_x(dev); + } + if (feature_flags & FEATURE_Y_ENABLED) { + printk(BIOS_INFO, "%s: Initializing Feature Y\n", dev_path(dev)); + init_feature_y(dev); + } +} +``` + + +## Conclusion + +The `device_operations` structure is a powerful abstraction mechanism in +coreboot. It enables consistent handling of diverse hardware while +allowing for device-specific behavior. By providing a standard interface +for device operations throughout the boot process, it simplifies the +codebase, enhances maintainability, and provides the extensibility needed +to support new hardware platforms. + +Understanding this structure, its relationship with `chip_operations`, +and its role in the boot process is essential for coreboot developers, +particularly when adding support for new devices or debugging hardware +initialization issues. By following the patterns and best practices +outlined here, developers can create robust and reusable device driver +implementations that integrate smoothly into the coreboot architecture. diff --git a/Documentation/internals/index.md b/Documentation/internals/index.md index 9e55965..56c8221 100644 --- a/Documentation/internals/index.md +++ b/Documentation/internals/index.md @@ -11,4 +11,5 @@ coreboot devicetree <devicetree.md> coreboot devicetree language <devicetree_language.md> Chip Operations <chip_operations.md> +Device Operations <device_operations> ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/87202?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: I3ed78f8ce50bb3914f55b2cbb7f5eb668706949a Gerrit-Change-Number: 87202 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>

1 0

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

07 Apr '25

Martin L Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/87201?usp=email ) Change subject: Documentation: Add chip operations ...................................................................... Documentation: Add chip operations Change-Id: I5373eab2de2e255f9e3576794b9ad02d9711a6c2 Signed-off-by: Martin Roth <gaumless(a)gmail.com> --- A Documentation/internals/chip_operations.md M Documentation/internals/index.md 2 files changed, 538 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/87201/1 diff --git a/Documentation/internals/chip_operations.md b/Documentation/internals/chip_operations.md new file mode 100644 index 0000000..cf93a70 --- /dev/null +++ b/Documentation/internals/chip_operations.md @@ -0,0 +1,537 @@ +# The `chip_operations` Structure in coreboot + +## Introduction + +The `chip_operations` structure is a fundamental component of coreboot's +chipset abstraction layer. It provides a standardized interface for chipset- +specific code to interact with coreboot's device initialization framework. +This structure enables coreboot to support a wide variety of chipsets +while maintaining a consistent initialization flow across different hardware +platforms. + +In coreboot's architecture, a "chip" refers to a collection of hardware +components that form a logical unit, such as a System-on-Chip (SoC), +a CPU, or a distinct southbridge/northbridge. The `chip_operations` +structure provides the hooks necessary for coreboot to discover, configure, +and initialize these components during the boot process. + +The `chip_operations` structure is particularly crucial for the ramstage +portion of coreboot, where it connects the static device tree definitions +with the actual hardware initialization code. It serves as the bridge +between the declarative device descriptions and the imperative code that +brings those devices to life. + + +## Structure Definition + +The `chip_operations` structure is defined in `src/include/device/device.h` +as follows: + +```c +struct chip_operations { + void (*enable_dev)(struct device *dev); + void (*init)(void *chip_info); + void (*final)(void *chip_info); + unsigned int initialized : 1; + unsigned int finalized : 1; + const char *name; +}; +``` + + +### Field Descriptions + +- **enable_dev**: A function pointer that takes a `struct device*` +parameter. This function is called for each device associated with the +chip during the device enumeration phase (specifically, within the +`scan_bus` operations triggered by `dev_enumerate`). Its primary +purpose is to set up device operations (`dev->ops`) based on the +device's role in the system. + +- **init**: A function pointer that takes a `void*` parameter pointing to +the chip's configuration data (typically cast to a chip-specific struct). +This function is called during the chip initialization phase +(`BS_DEV_INIT_CHIPS`), before device enumeration. It usually performs +early hardware setup needed before individual devices can be configured. + +- **final**: A function pointer that takes a `void*` parameter pointing to +the chip's configuration data (typically cast to a chip-specific struct). +This function is called during the final table writing phase of coreboot +initialization (`BS_WRITE_TABLES`), after all devices have been +initialized. It performs any necessary cleanup or late initialization +operations. + +- **initialized**: A bit flag indicating whether the chip's init function +has been called. + +- **finalized**: A bit flag indicating whether the chip's final function +has been called. + +- **name**: A string containing the human-readable name of the chip, used +for debugging and logging purposes. + + +## Initialization Sequence and `chip_operations` + +The `chip_operations` structure integrates with coreboot's boot state +machine, which is defined in `src/lib/hardwaremain.c`. The functions in +this structure are called at specific points during the boot process: + +1. **BS_DEV_INIT_CHIPS** state: The `init` function is called for each +chip in the device tree. This is handled by `dev_initialize_chips()` +which iterates through all devices, identifies unique chip instances, +and invokes their `init` functions. + +2. **BS_DEV_ENUMERATE** state: During the execution of this state, +`dev_enumerate()` is called, which triggers bus scanning +(e.g., `pci_scan_bus`). Within these scan routines, the `enable_dev` +function is called for devices associated with a chip. This commonly +assigns the appropriate `device_operations` structure to each device +based on its type and purpose. + +3. **BS_WRITE_TABLES** state: The `final` function is called for each +chip by `dev_finalize_chips()` after all devices have been initialized +and just before payloads are loaded. + +This sequence ensures that chips can perform necessary setup before their +individual devices are configured, and also perform cleanup or finalization +after all devices have been initialized but before the final tables are +written and the payload is executed. + + +## Relationship Between `chip_operations` and `device_operations` + +It's important to understand the distinction and relationship between +`chip_operations` and `device_operations`: + +- **chip_operations**: Operates at the chipset or SoC level, providing +hooks for chip-wide initialization. It's responsible for the overall +setup of a collection of devices that belong to the same logical chip. + +- **device_operations**: Operates at the individual device level, +providing functions to manage specific devices within a chip. These +operations include resource allocation, device initialization, and device- +specific functionality. + +The key relationship is that `chip_operations.enable_dev` is typically +responsible for assigning the appropriate `device_operations` structure +to each device based on its type and function. This is where the bridge +between the chip-level and device-level abstractions occurs. + +For example, a typical implementation of the `enable_dev` function might +look like this: + +```c +static void soc_enable(struct device *dev) +{ + if (dev->path.type == DEVICE_PATH_DOMAIN) + dev->ops = &pci_domain_ops; + else if (dev->path.type == DEVICE_PATH_CPU_CLUSTER) + dev->ops = &cpu_bus_ops; + else if (dev->path.type == DEVICE_PATH_GPIO) + block_gpio_enable(dev); + else if (dev->path.type == DEVICE_PATH_PCI && + dev->path.pci.devfn == PCH_DEVFN_PMC) + dev->ops = &pmc_ops; +} +``` + +This function examines each device's path type and assigns the appropriate +operations based on the device's role in the system. + + +## Integration with the Devicetree + +The `chip_operations` structure is tightly integrated with coreboot's +devicetree mechanism. The devicetree is a hierarchical description of the +hardware platform, defined in `.cb` files (typically `chipset.cb`, +`devicetree.cb`, and optionally `overridetree.cb`). + +In the devicetree, a `chip` directive starts a collection of devices +associated with a particular chip driver. The path specified with the +`chip` directive corresponds to a directory in the coreboot source tree +that contains the chip driver code, including a `chip.c` file that defines +the `chip_operations` structure for that chip. + +For example, a devicetree might contain: + +``` +chip soc/intel/cannonlake + device domain 0 on + device pci 00.0 on end # Host Bridge + device pci 12.0 on end # Thermal Subsystem + # ... more devices ... + end +end +``` + +This connects the devices under this chip directive with the +`chip_operations` structure defined in +`src/soc/intel/cannonlake/chip.c`: + +```c +struct chip_operations soc_intel_cannonlake_ops = { + .name = "Intel Cannonlake", + .enable_dev = &soc_enable, + .init = &soc_init_pre_device, +}; +``` + +During coreboot's build process, the `sconfig` utility processes the +devicetree files and generates code that links the devices defined in the +devicetree with their corresponding `chip_operations` structures. + + +## Chip Configuration Data + +Each chip typically defines a configuration structure in a `chip.h` file +within its source directory. This structure contains configuration settings +that can be specified in the devicetree using `register` directives. + +For example, a chip might define a configuration structure like: + +```c +/* In src/soc/intel/cannonlake/chip.h */ +struct soc_intel_cannonlake_config { + uint8_t pcie_rp_aspm[CONFIG_MAX_ROOT_PORTS]; + uint8_t usb2_ports[16]; + uint8_t usb3_ports[10]; + /* ... more configuration options ... */ +}; +``` + +In the devicetree, you would configure these options using register +directives: + +``` +chip soc/intel/cannonlake + register "pcie_rp_aspm[0]" = "ASPM_AUTO" + register "usb2_ports[5]" = "USB2_PORT_MID(OC_SKIP)" + # ... more register settings ... + + device domain 0 on + # ... devices ... + end +end +``` + +These configuration values are made available to the chip's `init` and +`final` functions through the `chip_info` parameter, which points to +an instance of the chip's configuration structure (after appropriate +casting from `void *`). + + +## Implementation Examples + +### Minimal Implementation + +Some chips may not need extensive initialization and can provide a +minimal implementation of the `chip_operations` structure: + +```c +struct chip_operations soc_ucb_riscv_ops = { + .name = "UCB RISC-V", +}; +``` + +This implementation only provides a name for debugging purposes but +doesn't define any initialization functions. + + +### Basic Implementation with Initialization + +A more typical implementation includes at least initialization hooks: + +```c +struct chip_operations soc_amd_genoa_poc_ops = { + .name = "AMD Genoa SoC Proof of Concept", + .init = soc_init, + .final = soc_final, +}; +``` + +The `init` function might perform chip-wide initialization: + +```c +static void soc_init(void *chip_info) +{ + default_dev_ops_root.write_acpi_tables = soc_acpi_write_tables; + amd_opensil_silicon_init(); + data_fabric_print_mmio_conf(); + fch_init(chip_info); +} +``` + + +### Complete Implementation + +A complete implementation includes all three function pointers: + +```c +struct chip_operations soc_intel_xeon_sp_cpx_ops = { + .name = "Intel Cooper Lake-SP", + .enable_dev = chip_enable_dev, + .init = chip_init, + .final = chip_final, +}; +``` + +The `enable_dev` function would typically assign device operations +based on device types: + +```c +static void chip_enable_dev(struct device *dev) +{ + /* PCI root complex */ + if (dev->path.type == DEVICE_PATH_DOMAIN) + dev->ops = &pci_domain_ops; + /* CPU cluster */ + else if (dev->path.type == DEVICE_PATH_CPU_CLUSTER) + dev->ops = &cpu_cluster_ops; + /* PCIe root ports */ + else if (dev->path.type == DEVICE_PATH_PCI && + PCI_SLOT(dev->path.pci.devfn) == PCIE_PORT1_SLOT) + dev->ops = &pcie_rp_ops; + /* ... other device types ... */ +} +``` + + +### Mainboard Implementation + +It's also common for the mainboard-specific code (e.g., +`src/mainboard/vendor/board/mainboard.c`) to define its own +`chip_operations`, often named `mainboard_ops`. The `mainboard_ops.init` +can perform early board-level setup, and `mainboard_ops.enable_dev` can +assign operations for devices specific to the mainboard or set default +operations. + +```c +/* Example from src/mainboard/google/zork/mainboard.c */ +struct chip_operations mainboard_ops = { + .enable_dev = mainboard_enable, + .init = mainboard_init, + .final = mainboard_final, +}; +``` + + +## Device Registration and Discovery + +The `chip_operations` structure plays a key role in device registration +and discovery within coreboot. Here's how it fits into this process: + +1. **Static Device Definition**: Devices are statically defined in the +devicetree files (`chipset.cb`, `devicetree.cb`, `overridetree.cb`). + +2. **Code Generation**: The `sconfig` utility processes these files and +generates code in `build/static.c` that creates the device structures +and links them to their corresponding chip configuration data. + +3. **Chip Initialization**: During the `BS_DEV_INIT_CHIPS` boot state, +`dev_initialize_chips()` calls each chip's `init` function to perform +chip-wide setup. + +4. **Device Enumeration and Enabling**: During the `BS_DEV_ENUMERATE` + boot state, `dev_enumerate()` initiates bus scanning. The scan + functions call the associated chip's `enable_dev` function for each + device, which assigns the appropriate device operations (`dev->ops`). + +5. **Device Configuration and Initialization**: Subsequent boot states + (`BS_DEV_RESOURCES`, `BS_DEV_ENABLE`, `BS_DEV_INIT`) configure and + initialize the devices according to their assigned device operations. + +6. **Chip Finalization**: After all devices have been initialized, + `dev_finalize_chips()` calls each chip's `final` function during the + `BS_WRITE_TABLES` boot state. + + +## Build Process Integration + +The `chip_operations` structures are integrated into the coreboot build +process through several mechanisms: + +1. **Devicetree Processing**: The `sconfig` utility processes the +devicetree files and generates code that creates and links the device +structures. + +2. **Static Structure Declaration**: Each chip (and often the mainboard) + defines its `chip_operations` structure in its respective `.c` file. + These structures are collected during the build process. + +3. **External References**: The generated code in `build/static.c` +includes external references to these `chip_operations` structures. + +4. **Linking**: The linker collects all the `chip_operations` structures +and includes them in the final firmware image. + +This process ensures that the appropriate chip operations are available +during the boot process for each chip included in the devicetree. + + +## Best Practices for Implementing `chip_operations` + +When implementing the `chip_operations` structure for a new chip, +follow these best practices: + +1. **Provide a Meaningful Name**: The `name` field should be descriptive +and identify the chip clearly for debugging purposes. + +2. **Implement `enable_dev` Correctly**: The `enable_dev` function should +assign the appropriate device operations based on device types and +functions. It should handle all device types that might be part of the chip. +Consider interactions with the mainboard `enable_dev`. + +3. **Use Configuration Data**: The `init` and `final` functions should +make use of the chip configuration data passed via the `chip_info` +parameter (casting it to the correct type) to configure the chip +according to the settings specified in the devicetree. + +4. **Minimize Dependencies**: The `init` function should minimize +dependencies on other chips being initialized, as the order of chip +initialization is not guaranteed. + +5. **Handle Resources Properly**: If the chip manages resources (memory +regions, I/O ports, etc.), ensure that these are properly allocated and +assigned to devices, usually within the associated `device_operations`. + +6. **Implement Error Handling**: Include appropriate error handling in +the initialization functions to handle hardware initialization failures +gracefully. + +7. **Document Special Requirements**: If the chip has special +requirements or dependencies, document these clearly in comments or +accompanying documentation. + + +## Troubleshooting `chip_operations` Issues + +When implementing or debugging `chip_operations`, you might encounter +certain issues: + +1. **Missing Device Operations**: If devices are not being initialized +properly, check that the `enable_dev` function is correctly +assigning device operations based on device types. Ensure it's being +called during bus scanning. + +2. **Initialization Order Problems**: If a chip's initialization depends +on another chip being initialized first, you might need to adjust the +initialization sequence or add explicit dependencies, possibly using +boot state callbacks if necessary. + +3. **Configuration Data Issues**: If chip configuration settings are not +being applied correctly, check that the configuration structure is +correctly defined in `chip.h`, that the register values in the +devicetree match the expected format, and that the `chip_info` pointer +is cast correctly in the `init`/`final` functions. + +4. **Build Errors**: If you encounter build errors related to +`chip_operations`, check that the structure is correctly defined and +that all required symbols are properly exported and linked. Check for +conflicts if multiple files define the same symbol. + +5. **Runtime Failures**: If the chip initialization fails at runtime, +add debug logging (using `printk`) to the `init`, `enable_dev`, and +`final` functions to identify the specific point of failure. + + +## Advanced `chip_operations` Patterns + +### Hierarchical Chip Initialization + +For complex chips with multiple components, you can implement a +hierarchical initialization pattern within the `init` function: + +```c +static void soc_init(void *chip_info) +{ + /* Initialize common components first */ + common_init(chip_info); + + /* Initialize specific blocks */ + pcie_init(chip_info); + usb_init(chip_info); + sata_init(chip_info); + + /* Final SoC-wide configuration */ + power_management_init(chip_info); +} +``` + + +### Variant Support + +For chips with multiple variants, you can implement variant detection +and specific initialization within the `init` function: + +```c +static void soc_init(void *chip_info) +{ + uint32_t variant = read_chip_variant(); + + /* Common initialization */ + common_init(chip_info); + + /* Variant-specific initialization */ + switch (variant) { + case VARIANT_A: + variant_a_init(chip_info); + break; + case VARIANT_B: + variant_b_init(chip_info); + break; + default: + printk(BIOS_WARNING, "Unknown variant %u\\n", variant); + break; + } +} +``` + + +### Conditional Feature Initialization + +You can conditionally initialize features based on configuration settings +passed via `chip_info`: + +```c +static void soc_init(void *chip_info) +{ + struct soc_config *config = chip_info; + + /* Always initialize core components */ + core_init(); + + /* Conditionally initialize optional features */ + if (config->enable_xhci) + xhci_init(config); + + if (config->enable_sata) + sata_init(config); + + if (config->enable_pcie) + pcie_init(config); +} +``` + + +## Conclusion + +The `chip_operations` structure is a fundamental component of coreboot's +chipset abstraction layer. It provides a standardized interface for chipset- +specific code to interact with coreboot's device initialization framework, +enabling support for a wide variety of chipsets while maintaining a +consistent initialization flow. + +By implementing the `chip_operations` structure for a specific chipset +(and often for the mainboard), developers can integrate their +hardware-specific code with coreboot's device enumeration, configuration, +and initialization process. This structure serves as the bridge between +the declarative device descriptions in the devicetree and the imperative +code that initializes the hardware. + +Understanding the `chip_operations` structure and its role in the +coreboot boot process is essential for anyone working on chipset or +mainboard support in coreboot. By following the best practices and +patterns outlined in this document, developers can create robust and +maintainable hardware support code that integrates seamlessly with the +coreboot firmware ecosystem. diff --git a/Documentation/internals/index.md b/Documentation/internals/index.md index 17dee55..9e55965 100644 --- a/Documentation/internals/index.md +++ b/Documentation/internals/index.md @@ -10,5 +10,5 @@ coreboot devicetree <devicetree.md> coreboot devicetree language <devicetree_language.md> - +Chip Operations <chip_operations.md> ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/87201?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: I5373eab2de2e255f9e3576794b9ad02d9711a6c2 Gerrit-Change-Number: 87201 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>

1 0

[XS] Change in coreboot[main]: {commonlib, lib}: Add timestamp for early chip initialization
by Karthik Ramasubramanian (Code Review) 07 Apr '25

07 Apr '25

Attention is currently required from: Julius Werner, Kapil Porwal, Subrata Banik. Karthik Ramasubramanian has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/87152?usp=email ) Change subject: {commonlib, lib}: Add timestamp for early chip initialization ...................................................................... Patch Set 5: Code-Review+2 (1 comment) File src/commonlib/include/commonlib/timestamp_serialized.h: https://review.coreboot.org/c/coreboot/+/87152/comment/b2b3e03f_8bf06ac1?us… : PS5, Line 222: TS_DEVICE_ENUMERATE > > This end_id does not exist in the old coreboot. What is the consequence of that? […] Acknowledged -- To view, visit https://review.coreboot.org/c/coreboot/+/87152?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: Ib6860901c6b1528ec5098fc93240c6e65777642b Gerrit-Change-Number: 87152 Gerrit-PatchSet: 5 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Mon, 07 Apr 2025 17:40:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>

1 0

[XS] Change in coreboot[main]: soc/intel/cmn/cpu: Refactor USE_INTEL_FSP_MP_INIT enablement logic
by Jérémy Compostella (Code Review) 07 Apr '25

07 Apr '25

Attention is currently required from: Intel coreboot Reviewers, Kapil Porwal, Subrata Banik. Jérémy Compostella has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/87161?usp=email ) Change subject: soc/intel/cmn/cpu: Refactor USE_INTEL_FSP_MP_INIT enablement logic ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/87161?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: I9ecc7b50ed6a6b13c4ccde0a49f50a40b606a848 Gerrit-Change-Number: 87161 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Mon, 07 Apr 2025 17:36:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: Documentation/getting_started/cbmem.md: Add cbmem documentation
by 9elements QA (Code Review) 07 Apr '25

07 Apr '25

9elements QA has posted comments on this change by Nick Kochlowski. ( https://review.coreboot.org/c/coreboot/+/84761?usp=email ) Change subject: Documentation/getting_started/cbmem.md: Add cbmem documentation ...................................................................... Patch Set 3: Automatic boot test returned (PASS/FAIL/TOTAL): 11 / 1 / 12 PASS: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES_ and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/238257 PASS: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/238255 PASS: x86_32 "ThinkPad T500" , build config LENOVO_T500 and payload SeaBIOS : https://lava.9esec.io/r/238254 PASS: x86_32 "HP Z220 SFF Workstation" , build config HP_Z220_SFF_WORKSTATION and payload LinuxBoot_BB_kexec : https://lava.9esec.io/r/238253 PASS: x86_64 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC.X86_64 and payload TianoCore : https://lava.9esec.io/r/238252 PASS: x86_64 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC.X86_64 and payload SeaBIOS : https://lava.9esec.io/r/238251 PASS: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload TianoCore : https://lava.9esec.io/r/238250 PASS: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload SeaBIOS : https://lava.9esec.io/r/238249 PASS: x86_64 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_X86_64 and payload SeaBIOS : https://lava.9esec.io/r/238248 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ASAN and payload SeaBIOS : https://lava.9esec.io/r/238247 FAIL: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ and payload SeaBIOS : https://lava.9esec.io/r/238246 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX and payload SeaBIOS : https://lava.9esec.io/r/238245 Please note: This test is under development and might not be accurate at all! -- To view, visit https://review.coreboot.org/c/coreboot/+/84761?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: I514cfbd6df85f332419444c35d066a518d5044f3 Gerrit-Change-Number: 84761 Gerrit-PatchSet: 3 Gerrit-Owner: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Reviewer: Ana Carolina Cabral <ana.cpmelo95(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Mon, 07 Apr 2025 17:07:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No

1 0

[XS] Change in coreboot[main]: soc/intel/cmn/cpu: Refactor USE_INTEL_FSP_MP_INIT enablement logic
by Subrata Banik (Code Review) 07 Apr '25

07 Apr '25

Attention is currently required from: Intel coreboot Reviewers, Kapil Porwal. Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/87161?usp=email ) Change subject: soc/intel/cmn/cpu: Refactor USE_INTEL_FSP_MP_INIT enablement logic ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/87161/comment/b2a93512_a5427c8d?us… : PS3, Line 21: change in the default value, but it is kept for clarity and to explicitly > > `Possible unwrapped commit description (prefer a maximum 72 chars per line)` […] Acknowledged -- To view, visit https://review.coreboot.org/c/coreboot/+/87161?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: I9ecc7b50ed6a6b13c4ccde0a49f50a40b606a848 Gerrit-Change-Number: 87161 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Mon, 07 Apr 2025 17:04:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2025