ipmi_kcs: host the device in OpenHCL underhill with SEL forwarding - #3837
Draft
Manish Ranjan Mahanta (marma-dev) wants to merge 4 commits into
Draft
Manish Ranjan Mahanta (marma-dev) wants to merge 4 commits into
Manish Ranjan Mahanta (marma-dev) wants to merge 4 commits into
Conversation
added 2 commits
June 29, 2026 21:47
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.
Copilot started reviewing on behalf of
Manish Ranjan Mahanta (marma-dev)
June 29, 2026 16:43
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Hosts the new ipmi_kcs virtual BMC device inside OpenHCL underhill and forwards guest-written SEL (System Event Log) records out to the host via the existing guest-emulation transport/tracing pipeline, gated by a new DPS boolean.
Changes:
- Added
ipmi_kcs+ipmi_kcs_resourcescrates (device implementation, SEL storage/commands, sink + clock abstractions, and a resource handle withforward_sel). - Plumbed a new DPS field (
enable_ipmi→ipmi_enabled) through GET protocol/client to gate device creation. - Wired underhill to instantiate the device for x86_64 UEFI VMs when DPS enables IPMI, and registered the resolver in
openvmm_resources.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| vm/devices/ipmi_kcs/src/sink.rs | Adds SelSink and BmcClock abstractions plus tracing-based SEL forwarding sink. |
| vm/devices/ipmi_kcs/src/sel.rs | Implements SEL storage, SEL IPMI commands, and forwarding hook on add-entry; includes unit tests. |
| vm/devices/ipmi_kcs/src/resolver.rs | Installs sink/clock deps during resource resolution and disables saved-state. |
| vm/devices/ipmi_kcs/src/protocol.rs | Defines KCS/IPMI enums and helpers for NetFn/LUN/state encoding with tests. |
| vm/devices/ipmi_kcs/src/lib.rs | Implements the KCS port I/O device and integrates SEL handling; includes device-level tests. |
| vm/devices/ipmi_kcs/Cargo.toml | New crate manifest for ipmi_kcs. |
| vm/devices/ipmi_kcs_resources/src/lib.rs | Defines IpmiKcsHandle { forward_sel } as a MeshPayload resource handle. |
| vm/devices/ipmi_kcs_resources/Cargo.toml | New crate manifest for ipmi_kcs_resources. |
| vm/devices/get/guest_emulation_transport/src/client.rs | Maps DPS JSON enable_ipmi into the typed ipmi_enabled setting. |
| vm/devices/get/guest_emulation_transport/src/api.rs | Extends platform_settings::General with ipmi_enabled. |
| vm/devices/get/get_protocol/src/dps_json.rs | Extends DPS v1 JSON schema with enable_ipmi (defaultable). |
| openvmm/openvmm_resources/src/lib.rs | Registers IpmiKcsResolver for cfg(guest_arch = "x86_64"). |
| openvmm/openvmm_resources/Cargo.toml | Adds an ipmi_kcs dependency under an x86_64 target-gated dependency section. |
| openhcl/underhill_core/src/worker.rs | Creates and pushes the ipmi_kcs chipset device handle when UEFI + DPS IPMI enabled. |
| openhcl/underhill_core/Cargo.toml | Adds ipmi_kcs_resources under linux+x86_64 target dependencies. |
| Cargo.toml | Adds workspace dependency entries for ipmi_kcs and ipmi_kcs_resources. |
| Cargo.lock | Adds lock entries for the new crates and wires ipmi_kcs into openvmm_resources. |
Comment on lines
+285
to
+289
| 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
+111
to
+115
| fn bmc_time(&self) -> u32 { | ||
| let now = self.deps.clock.now_unix_secs(); | ||
| let adjusted = now.saturating_add(self.time_offset); | ||
| adjusted.max(0) as u32 | ||
| } |
Comment on lines
+558
to
+568
| #[test] | ||
| fn sel_injected_clock_used_for_time() { | ||
| let deps = SelDeps { | ||
| sink: Arc::new(crate::sink::NullSelSink), | ||
| clock: Arc::new(FixedClock(1_700_000_000)), | ||
| }; | ||
| let mut store = SelStore::with_deps(deps); | ||
| let resp = store.handle_command(IpmiCommand::GET_SEL_TIME, &[]); | ||
| let time = u32::from_le_bytes([resp[1], resp[2], resp[3], resp[4]]); | ||
| assert_eq!(time, 1_700_000_000); | ||
| } |
Comment on lines
108
to
112
| [target.'cfg(target_arch = "x86_64")'.dependencies] | ||
| ipmi_kcs.workspace = true | ||
| serial_16550.workspace = true | ||
| serial_debugcon.workspace = true | ||
|
|
Comment on lines
187
to
190
| [target.'cfg(all(target_os = "linux", target_arch = "x86_64"))'.dependencies] | ||
| firmware_pcat.workspace = true | ||
| ipmi_kcs_resources.workspace = true | ||
|
|
Comment on lines
+265
to
+266
| ipmi_kcs = { path = "vm/devices/ipmi_kcs" } | ||
| ipmi_kcs_resources = { path = "vm/devices/ipmi_kcs_resources" } |
Wire the ipmi_kcs device into the paravisor, gated on a new DPS ipmi flag: - TracingSelSink: forwards SEL records via tracelimit, picked up by the OpenHCL tracing pipeline to the host. - IpmiKcsHandle gains forward_sel; resolver builds device with TracingSelSink when set. - underhill_core instantiates ipmi_kcs for x86_64 UEFI when General.ipmi_enabled; new enable_ipmi DPS field (serde default false, back-compat).
Manish Ranjan Mahanta (marma-dev)
force-pushed
the
user/marma/ipmi-kcs-underhill
branch
from
June 29, 2026 16:56
b89a14c to
57d97c2
Compare
Copilot started reviewing on behalf of
Manish Ranjan Mahanta (marma-dev)
June 30, 2026 04:25
View session
Comment on lines
+111
to
+115
| fn bmc_time(&self) -> u32 { | ||
| let now = self.deps.clock.now_unix_secs(); | ||
| let adjusted = now.saturating_add(self.time_offset); | ||
| adjusted.max(0) as u32 | ||
| } |
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; | ||
|
|
Member
|
Manish Ranjan Mahanta (@marma-dev), active PR #4396 appears to supersede this OpenHCL IPMI KCS branch. Could you close it, or identify any remaining behavior that should 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
Hosts the
ipmi_kcsBMC device inside OpenHCL (underhill) and wires its SEL (System Event Log) egress into the existing guest-emulation transport so SEL records written by the guest are forwarded out to the host. Gated behind a Device Platform Settings (DPS) flag so the device is only instantiated when IPMI is explicitly enabled for the VM.This is PR 2 of 4 in the IPMI KCS stack (stacked on
ipmi_kcs-sel):ipmi_kcsdevice + resource handle.no_stdcore + C-ABI staticlib for cross-language reuse.What's included
Underhill hosting —
openhcl/underhill_core/worker.rs— on x86_64 UEFI VMs with IPMI enabled, build theipmi_kcschipset device and push it into the chipset device list.Cargo.toml— take a dependency onipmi_kcs_resources(x86_64 linux target).SEL forwarding sink —
vm/devices/ipmi_kcs/sink.rs— adds aSelSinkimplementation that forwards completed SEL records to the host over the guest-emulation transport.resolver.rs— honors theforward_selflag from the resource handle and installs the forwarding sink (vs. the default no-op).DPS plumbing —
vm/devices/get/get_protocol/src/dps_json.rs,guest_emulation_transport/src/api.rs,client.rs— add the IPMI-enabled field so the host can signal that the IPMI device should be created for this VM.Resource handle —
vm/devices/ipmi_kcs_resources/lib.rs— surface theforward_seltoggle through the handle.Design notes
forward_sel, defaulting off.SelSinkseam introduced in PR1, so the core device stays host-agnostic.Testing
cargo test -p ipmi_kcs -p ipmi_kcs_resources— passes (device + resource tests, incl. the new sink path).cargo check -p get_protocol -p guest_emulation_transport --target x86_64-unknown-linux-musl— clean.underhill_coreitself builds modulo the unrelatedopenssl-syscross-sysroot dependency (pre-existing environment limitation, not introduced here).Risk
Low — additive and double-gated (DPS flag + arch/firmware check). No behavior change for VMs without IPMI enabled; existing chipset device wiring is untouched when the device isn't created.