ipmi_kcs: productionize KCS BMC device with SEL egress + clock injection - #3835
Draft
Manish Ranjan Mahanta (marma-dev) wants to merge 2 commits into
Draft
Manish Ranjan Mahanta (marma-dev) wants to merge 2 commits into
Manish Ranjan Mahanta (marma-dev) wants to merge 2 commits into
Conversation
Copilot started reviewing on behalf of
Manish Ranjan Mahanta (marma-dev)
June 29, 2026 16:39
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new ipmi_kcs device crate implementing an emulated IPMI BMC over the KCS PIO interface (0xCA2/0xCA3), including in-memory SEL handling with host egress via an injected sink and timestamps from an injected clock. It wires the device into the OpenVMM resource resolver graph (x86_64) and registers new workspace crates.
Changes:
- Introduces
ipmi_kcsdevice implementation (KCS state machine + basic IPMI framing) with SEL command support and inspect visibility. - Adds dependency-injected SEL egress (
SelSink) and time (BmcClock) viaSelDeps, with defaults for OpenVMM usage. - Adds
ipmi_kcs_resourceshandle + resolver wiring and registers the resolver inopenvmm_resourcesand workspace deps/lockfile.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vm/devices/ipmi_kcs/src/lib.rs | Core KCS PIO device, IPMI dispatch, and save/restore stub + unit tests |
| vm/devices/ipmi_kcs/src/protocol.rs | KCS/IPMI constants and helpers (open_enum!) + unit tests |
| vm/devices/ipmi_kcs/src/sel.rs | In-memory SEL store and IPMI Storage NetFn command handlers + unit tests |
| vm/devices/ipmi_kcs/src/sink.rs | SelSink/BmcClock traits and SelDeps injection bundle |
| vm/devices/ipmi_kcs/src/resolver.rs | Static resource resolver returning an IpmiKcsDevice |
| vm/devices/ipmi_kcs/Cargo.toml | New crate manifest and deps |
| vm/devices/ipmi_kcs_resources/src/lib.rs | New resource handle type for declarative instantiation |
| vm/devices/ipmi_kcs_resources/Cargo.toml | New resources crate manifest |
| openvmm/openvmm_resources/src/lib.rs | Registers IpmiKcsResolver for guest_arch = "x86_64" |
| openvmm/openvmm_resources/Cargo.toml | Adds ipmi_kcs dependency for x86_64 builds |
| Cargo.toml | Adds workspace dependency entries for the new crates |
| Cargo.lock | Adds lock entries for ipmi_kcs and ipmi_kcs_resources |
Comment on lines
+284
to
+290
| let new_time = u32::from_le_bytes([data[0], data[1], data[2], data[3]]); | ||
| let now = std::time::SystemTime::now() | ||
| .duration_since(std::time::UNIX_EPOCH) | ||
| .unwrap_or_default() | ||
| .as_secs() as i64; | ||
| self.time_offset = (new_time as i64) - now; | ||
|
|
Comment on lines
+12
to
+17
| /// Resource handle for the IPMI KCS device. | ||
| /// | ||
| /// No configuration fields — the device starts with an empty SEL | ||
| /// and the guest populates it at runtime. | ||
| #[derive(MeshPayload)] | ||
| pub struct IpmiKcsHandle; |
added 2 commits
June 30, 2026 09:55
Import the IPMI KCS device crate and add production-readiness changes over the PoC: - SelSink trait for forwarding SEL entries to a host (no-op by default); enables OpenHCL to publish guest SEL to host ETW. - BmcClock trait to remove std::time dependency from the SEL store; SystemClock default, injectable for paravisor. - with_deps constructors on IpmiKcsDevice/SelStore; register resolver in openvmm_resources (x86_64). - Tests for sink egress and injected clock.
- protocol.rs: use open_enum's inner #![expect(missing_docs)] idiom instead of an outer #[allow], so it reaches the generated associated constants (protocol module is pub). - sel.rs: SelStore::new() is only used by tests now that the lib paths construct via with_deps; move it into a #[cfg(test)] impl. - Cargo.lock: add ipmi_kcs / ipmi_kcs_resources entries.
Manish Ranjan Mahanta (marma-dev)
force-pushed
the
user/marma/ipmi-kcs-sel
branch
from
June 30, 2026 04:25
8c124c4 to
39439e7
Compare
Member
|
Manish Ranjan Mahanta (@marma-dev), active PR #4396 appears to supersede this IPMI KCS implementation. Could you close it, or identify any distinct pieces that still need to be revived? Thanks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Turns the experimental IPMI KCS proof-of-concept into a production-ready emulated BMC device for OpenVMM. The guest talks to a KCS (Keyboard Controller Style) interface over PIO ports
0xCA2/0xCA3; the device implements enough of the IPMI message protocol to accept SEL (System Event Log) writes and forwards each completed SEL record out of the VMM via an injected sink, with timestamps supplied by an injected clock. This is the foundation for IPMI-based guest diagnosability (SEL → host telemetry).This is PR 1 of 4 in a stack:
ipmi_kcsdevice + resource handle.no_stdcore + C-ABI staticlib so the same emulator can be shared with the C++ Legacy HCL stack.What's included
vm/devices/ipmi_kcs/— new device crate:lib.rs—KcsDevicePIO state machine (io_read/io_write/reset),ChipsetDevice+PortIoInterceptintegration, save/restore support.protocol.rs— KCS status/command register model and IPMI request/response framing.sel.rs— SEL record assembly and the IPMI Storage commands needed to add SEL entries.sink.rs— trait-based egress:SelSink(where completed SEL records go),NullSelSink(default no-op), andBmcClock(timestamp source), bundled viaSelDeps.resolver.rs— resource resolver wiring.vm/devices/ipmi_kcs_resources/—IpmiKcsHandleresource (forward_sel: bool) for declarative device instantiation.openvmm_resources— register the IPMI KCS resolver (x86_64).Cargo.toml/Cargo.lock— workspace member registration.Design notes
SelSink) and time (BmcClock) are injected throughSelDepsrather than hard-wired, so the same device hosts cleanly in OpenVMM, OpenHCL, and (via the later FFI crate) the C++ HCL. The default sink is a no-op, so the device is inert unless a consumer opts in.Testing
cargo test -p ipmi_kcs— 23 tests pass (protocol framing, SEL record assembly, KCS state machine).x86_64-unknown-linux-musl(OpenHCL target) and Windows.missing_docs/dead_codewarnings surfaced by the new public surface.Risk
Low — new, self-contained crates that are not instantiated by any existing configuration. No behavior change to existing devices; egress is opt-in and defaults to no-op.