Skip to content

virt: move SynicPorts to virt crate and decouple from CpuIo#3217

Open
jstarks wants to merge 5 commits intomicrosoft:mainfrom
jstarks:synic_refac
Open

virt: move SynicPorts to virt crate and decouple from CpuIo#3217
jstarks wants to merge 5 commits intomicrosoft:mainfrom
jstarks:synic_refac

Conversation

@jstarks
Copy link
Copy Markdown
Member

@jstarks jstarks commented Apr 7, 2026

Allow backends to provide alternate SynicPortAccess implementations (e.g., for backends where the hypervisor handles synic in-kernel) by extracting SynicPortMap into a standalone type on the partition inner struct and adding Partition::synic_port_access() to the trait.

SynicPortMap holds the connection-ID-keyed registry of message and event ports and exposes handle_post_message/handle_signal_event for direct use by hypercall handlers. SynicPorts wraps a SynicPortMap together with a Synic impl to satisfy the SynicPortAccess trait for VMBus and other consumers.

This removes the synic methods (signal_synic_event, post_synic_message) from the CpuIo trait and the synic field from ChipsetPlusSynic, since hypercall handlers now dispatch directly through their partition's SynicPortMap. The CpuIo type parameter is also removed from all hypercall handler structs, as they no longer need it for synic dispatch.

Allow backends to provide alternate SynicPortAccess implementations
(e.g., for backends where the hypervisor handles synic in-kernel)
by extracting SynicPortMap into a standalone type on the partition
inner struct and adding Partition::synic_port_access() to the trait.

SynicPortMap holds the connection-ID-keyed registry of message and
event ports and exposes handle_post_message/handle_signal_event for
direct use by hypercall handlers. SynicPorts wraps a SynicPortMap
together with a Synic impl to satisfy the SynicPortAccess trait for
VMBus and other consumers.

This removes the synic methods (signal_synic_event, post_synic_message)
from the CpuIo trait and the synic field from ChipsetPlusSynic, since
hypercall handlers now dispatch directly through their partition's
SynicPortMap. The CpuIo type parameter is also removed from all
hypercall handler structs, as they no longer need it for synic dispatch.
@jstarks jstarks requested a review from a team as a code owner April 7, 2026 18:42
Copilot AI review requested due to automatic review settings April 7, 2026 18:42
Copy link
Copy Markdown
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

Note

Copilot was unable to run its full agentic suite in this review.

This PR moves SynIC port routing into the virt crate and decouples SynIC dispatch from CpuIo, enabling backends to supply alternate SynicPortAccess implementations (including in-kernel SynIC).

Changes:

  • Introduces virt::synic::{SynicPortMap, SynicPorts, Synic} and stores SynicPortMap on partition inner structs for shared registration/dispatch.
  • Adds Hv1::synic(self: Arc<Self>) -> Arc<dyn SynicPortAccess> and updates hypercall handlers to call SynicPortMap::handle_post_message/handle_signal_event directly.
  • Removes SynIC methods from CpuIo and updates adapters/callers (including renaming ChipsetPlusSynic to AdaptedChipset).

Reviewed changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
vmm_core/virt_whp/src/vp.rs Updates hypercall/MNF paths to dispatch via partition synic_ports instead of CpuIo.
vmm_core/virt_whp/src/synic.rs Switches WHP partition to implement new virt::synic::Synic and expose port_map().
vmm_core/virt_whp/src/lib.rs Adds synic_ports to partition inner and implements Hv1::synic().
vmm_core/virt_whp/src/hypercalls.rs Removes CpuIo generic/bus dependency and dispatches SynIC hypercalls via SynicPortMap.
vmm_core/virt_whp/src/emu.rs Routes MNF signaling through VP/partition synic_ports.
vmm_core/virt_support_x86emu/tests/tests/common.rs Removes now-defunct CpuIo SynIC methods from mock.
vmm_core/virt_mshv/src/lib.rs Adds synic_ports, provides Hv1::synic(), and updates hypercall handler dispatch.
vmm_core/virt_kvm/src/lib.rs Adds synic_ports to KVM partition inner state.
vmm_core/virt_kvm/src/arch/x86_64/mod.rs Adds synic_ports init, implements Hv1::synic(), and dispatches hypercalls via port map.
vmm_core/virt_kvm/src/arch/aarch64/mod.rs Adds synic_ports init and stubs new SynIC APIs with unimplemented!().
vmm_core/virt_hvf/src/lib.rs Adds synic_ports, implements Hv1::synic(), and updates hypercall wiring.
vmm_core/virt_hvf/src/hypercall.rs Removes CpuIo SynIC usage and dispatches via partition synic_ports.
vmm_core/virt/src/synic.rs New shared SynIC port map + adapter (SynicPorts) implementation in virt.
vmm_core/virt/src/lib.rs Exposes the new virt::synic module.
vmm_core/virt/src/io.rs Removes SynIC-related methods from CpuIo.
vmm_core/virt/src/generic.rs Adds Hv1::synic() and removes old Synic/SynicMonitor traits from generic module.
vmm_core/src/vmotherboard_adapter.rs Renames adapter to AdaptedChipset and removes embedded SynIC forwarding from CpuIo.
vmm_core/src/synic.rs Deletes old SynicPorts implementation (moved to virt).
vmm_core/src/lib.rs Removes synic module export.
tmk/tmk_vmm/src/run.rs Removes CpuIo SynIC stubs from test/run IO handler.
openvmm/openvmm_core/src/worker/dispatch.rs Switches SynIC acquisition to partition-provided API and uses AdaptedChipset.
openvmm/openvmm_core/src/partition.rs Imports new virt::synic::Synic and exposes into_synic() via HvlitePartition.
openhcl/virt_mshv_vtl/src/processor/tdx/mod.rs Drops CpuIo SynIC dependencies from hypercall handler plumbing.
openhcl/virt_mshv_vtl/src/processor/snp/mod.rs Drops CpuIo SynIC dependencies from hypercall handler plumbing.
openhcl/virt_mshv_vtl/src/processor/mshv/x64.rs Updates hypercall exit dispatch and MNF signaling to use partition synic_ports.
openhcl/virt_mshv_vtl/src/processor/mshv/arm64.rs Updates hypercall exit dispatch and MNF signaling to use partition synic_ports.
openhcl/virt_mshv_vtl/src/processor/mod.rs Reworks MNF helper + hypercall handler dispatch to use SynicPortMap.
openhcl/virt_mshv_vtl/src/processor/hardware_cvm/mod.rs Adjusts hypercall handler generics after removing CpuIo SynIC coupling.
openhcl/virt_mshv_vtl/src/lib.rs Adds synic_ports, implements new virt::synic::Synic + Hv1::synic().
openhcl/underhill_core/src/worker.rs Switches SynIC acquisition to Hv1::synic() and uses AdaptedChipset.
openhcl/underhill_core/src/vp.rs Updates VP runner interface to accept AdaptedChipset.

// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

//! [`SynicPortAccess`] implementatsion for backends that intercept
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

Typo in module docs: change implementatsion to implementation.

Suggested change
//! [`SynicPortAccess`] implementatsion for backends that intercept
//! [`SynicPortAccess`] implementation for backends that intercept

Copilot uses AI. Check for mistakes.
@github-actions github-actions bot added the unsafe Related to unsafe code label Apr 7, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

⚠️ 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

Copilot AI review requested due to automatic review settings April 7, 2026 18:58
Copy link
Copy Markdown
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

Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.

}

fn synic(self: Arc<Self>) -> Arc<dyn vmcore::synic::SynicPortAccess> {
Arc::new(virt::synic::SynicPorts::new(self))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So this is an Arc, to a trait object, that contains just an Arc...

}

impl Debug for SynicPortMap {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I yearn for #[debug(skip)]

@smalis-msft
Copy link
Copy Markdown
Contributor

Like I said on the call this looks like 99% goodness, there's just the unfortunate fn synic impls.

Copilot AI review requested due to automatic review settings April 7, 2026 20:15
Copy link
Copy Markdown
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

Copilot reviewed 30 out of 30 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

vmm_core/virt_kvm/src/arch/aarch64/mod.rs:804

  • The virt::synic::Synic implementation for KvmPartitionInner on aarch64 is left as unimplemented!() for port_map, post_message, new_guest_event_port, and prefer_os_events. This will panic at runtime as soon as anything calls KvmPartition::synic() (which is now always available via virt::Hv1::synic) and attempts to register ports / deliver synic events/messages. Implement these methods (at minimum port_map should return &self.synic_ports) or gate synic support behind the same feature/capability checks used for aarch64 KVM HV1 support.
impl virt::synic::Synic for KvmPartitionInner {
    fn port_map(&self) -> &virt::synic::SynicPortMap {
        unimplemented!()
    }

    fn post_message(&self, _vtl: Vtl, _vp: VpIndex, _sint: u8, _typ: u32, _payload: &[u8]) {
        unimplemented!()
    }

    fn new_guest_event_port(
        self: Arc<Self>,
        _vtl: Vtl,
        _vp: u32,
        _sint: u8,
        _flag: u16,
    ) -> Box<dyn vmcore::synic::GuestEventPort> {
        unimplemented!()
    }

    fn prefer_os_events(&self) -> bool {
        unimplemented!()
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

unsafe Related to unsafe code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants