Skip to content

Conversation

@mattkur
Copy link
Contributor

@mattkur mattkur commented Dec 5, 2025

Cherry-pick of #2512 (not clean, there was one merge conflict in underhill_core. I have verified all dependent changes are already in this branch...)

This PR supports upcoming changes to have sidecar start CPUs without outstanding IO, but have the kernel start the CPUs with outstanding IO. A 96-vp VM will possibly have 96 CPUs with interrupts, but only have IO outstanding to a few of those at a time. This change allows the restore path to discern.

TEST:
Validated that this does not cause regression in local CI.

Copilot AI review requested due to automatic review settings December 5, 2025 02:26
@mattkur mattkur requested review from a team as code owners December 5, 2025 02:26
@github-actions github-actions bot added release_1.7.2511 Targets the release/1.7.2511 branch. unsafe Related to unsafe code labels Dec 5, 2025
@github-actions
Copy link

github-actions bot commented Dec 5, 2025

⚠️ Unsafe Code Detected

This PR modifies files containing unsafe Rust code. Extra scrutiny is required during review.

For more on why we check whole files, instead of just diffs, check out the Rustonomicon

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refines the save/restore mechanism to distinguish between CPUs with mapped device interrupts and CPUs with outstanding I/O operations. Previously, the system only tracked which CPUs had interrupts mapped, treating all such CPUs uniformly. Now it separates them into two categories: those with interrupts but no pending I/O, and those with both interrupts and outstanding I/O. This enables more granular restore decisions, such as allowing sidecar to start CPUs without outstanding I/O while having the kernel handle CPUs with active I/O operations.

Key changes:

  • Split the cpus_with_mapped_interrupts field into cpus_with_mapped_interrupts_no_io and cpus_with_outstanding_io in the save/restore state
  • Refactored nvme_driver exports to use module-level export pattern
  • Added comprehensive helper module with interrupt state analysis logic and test coverage
  • Updated all integration points to use the new two-field approach

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
vm/loader/loader_defs/src/shim.rs Renamed and split the CPU interrupt tracking field in SavedState to separate CPUs with outstanding IO from those with only mapped interrupts
vm/devices/storage/disk_nvme/nvme_driver/src/lib.rs Changed from exporting individual save_restore items to exporting the entire module
vm/devices/storage/disk_nvme/nvme_driver/src/driver.rs Added documentation to save_restore module and its data structures
openhcl/underhill_core/src/nvme_manager/save_restore_helpers.rs New module with VPInterruptState struct and nvme_interrupt_state function, plus comprehensive tests
openhcl/underhill_core/src/nvme_manager/save_restore.rs Updated import path and removed old cpus_with_interrupts helper function
openhcl/underhill_core/src/nvme_manager/mod.rs Updated type references to use nvme_driver::save_restore module path
openhcl/underhill_core/src/nvme_manager/manager.rs Updated import and type paths to use nvme_driver::save_restore
openhcl/underhill_core/src/nvme_manager/device.rs Updated import to use nvme_driver::save_restore module path
openhcl/underhill_core/src/loader/vtl2_config/mod.rs Updated to accept VPInterruptState and destructure both CPU lists when writing persisted info
openhcl/underhill_core/src/dispatch/mod.rs Changed to call nvme_interrupt_state helper and pass VPInterruptState to write_persisted_info
openhcl/underhill_core/Cargo.toml Added nvme_spec as dev-dependency for tests
openhcl/openhcl_boot/src/host_params/dt/mod.rs Updated to handle both cpus_with_mapped_interrupts_no_io and cpus_with_outstanding_io fields
Cargo.lock Reflects the nvme_spec dev-dependency addition

@github-actions
Copy link

github-actions bot commented Dec 5, 2025

@github-actions
Copy link

github-actions bot commented Dec 5, 2025

smalis-msft
smalis-msft previously approved these changes Dec 12, 2025
@github-actions
Copy link

…hey have interrupts (microsoft#2512)

This PR supports upcoming changes to to have sidecar start CPUs without
outstanding IO, but have the kernel start the CPUs with outstanding IO.
A 96-vp VM will possibly have 96 CPUs with interrupts, but only have IO
outstanding to a few of those at a time. This change allows the restore
path to discern.

TEST:
Validated that this does not cause regression in local CI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_1.7.2511 Targets the release/1.7.2511 branch. unsafe Related to unsafe code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants