Skip to content

pci_core: simplify MSIs#2700

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

pci_core: simplify MSIs#2700
jstarks wants to merge 5 commits intomicrosoft:mainfrom
jstarks:msi_refac

Conversation

@jstarks
Copy link
Member

@jstarks jstarks commented Jan 27, 2026

In the current PCI model, a device must create and enable an MSI object for each MSI address/data pair it wants to signal. This is inconvenient for some types of devices, especially those that are implemented in a model that allows arbitrary address/data pairs to be signaled at any time.

Change this model so that instead a device can just call a function with an arbitrary address/data pair. Update the implementations to match.

The reason for the current model was to anticipate the need to support associating OS events (NT events or Linux eventfds) with MSIs, in order to efficiently support out-of-proc and in-kernel devices such as vhost-user or vhost-net. This may still be useful to support later, but it can be done via an optional extension, rather than force all devices to use the more complicated model.

@jstarks jstarks requested review from a team as code owners January 27, 2026 17:57
Copilot AI review requested due to automatic review settings January 27, 2026 17:57
@github-actions github-actions bot added the unsafe Related to unsafe code label Jan 27, 2026
@github-actions
Copy link

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

Refactors the PCI MSI signaling model so devices can signal arbitrary MSI address/data pairs directly, removing the need to allocate/enable a per-vector “MSI object” ahead of time.

Changes:

  • Replaces the MsiInterruptTarget/MsiControl/MsiInterruptSet model with SignalMsi plus MsiConnection/MsiTarget.
  • Updates hypervisor backends (WHP/KVM) and software interrupt devices (APIC/GIC) to implement the new signaling interface.
  • Plumbs the new MSI target type through device construction paths and updates unit tests/fuzz harnesses accordingly.

Reviewed changes

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

Show a summary per file
File Description
vmm_core/virt_whp/src/synic.rs Switches WHP MSI delivery to SignalMsi::signal_msi.
vmm_core/virt_whp/src/lib.rs Renames partition MSI accessor to as_signal_msi and updates return type.
vmm_core/virt_whp/src/device.rs Implements SignalMsi for WHP device wrapper to forward MSI signaling.
vmm_core/virt_kvm/src/gsi.rs Renames/annotates GSI route helpers after removing irqfd-based MSI path.
vmm_core/virt_kvm/src/arch/x86_64/mod.rs Implements SignalMsi for KVM by directly issuing KVM MSI ioctls.
vmm_core/virt/src/x86/apic_software_device.rs Reworks APIC software VPCI interrupt indirection to use SignalMsi.
vmm_core/virt/src/generic.rs Updates Partition/Hv1 trait APIs to use SignalMsi.
vmm_core/virt/src/aarch64/gic_software_device.rs Converts GIC software device MSI signaling to SignalMsi.
vmm_core/src/device_builder.rs Plumbs MsiConnection/MsiTarget through PCI device build/resolve flows.
vm/devices/virtio/virtio/src/transport/pci.rs Updates virtio PCI interrupt model to take &MsiTarget.
vm/devices/virtio/virtio/src/tests.rs Updates virtio tests to use MsiConnection and connect via SignalMsi.
vm/devices/virtio/virtio/src/resolver.rs Updates resolver input from register_msi to msi_target.
vm/devices/user_driver_emulated_mock/src/lib.rs Updates test emulation MSI wiring to MsiConnection/SignalMsi.
vm/devices/storage/storage_tests/tests/scsidvd_nvme.rs Updates NVMe storage test MSI wiring to MsiConnection.
vm/devices/storage/nvme_test/src/tests/test_helpers.rs Updates test interrupt controller to implement SignalMsi.
vm/devices/storage/nvme_test/src/tests/controller_tests.rs Updates NVMe fault-controller tests to use MsiConnection.
vm/devices/storage/nvme_test/src/resolver.rs Updates resolver input from register_msi to msi_target.
vm/devices/storage/nvme_test/src/pci.rs Updates MSI-X emulator construction to take &MsiTarget.
vm/devices/storage/nvme/src/tests/test_helpers.rs Updates test interrupt controller to implement SignalMsi.
vm/devices/storage/nvme/src/tests/controller_tests.rs Updates NVMe controller tests to use MsiConnection.
vm/devices/storage/nvme/src/resolver.rs Updates resolver input from register_msi to msi_target.
vm/devices/storage/nvme/src/pci.rs Updates NVMe PCI device to pass &MsiTarget into MSI-X emulator.
vm/devices/storage/disk_nvme/nvme_driver/src/tests.rs Updates NVMe driver tests to use MsiConnection.
vm/devices/storage/disk_nvme/nvme_driver/fuzz/fuzz_nvme_driver.rs Updates fuzz harness to use MsiConnection for MSI wiring.
vm/devices/storage/disk_nvme/nvme_driver/fuzz/fuzz_emulated_device.rs Updates fuzz emulated device wrapper to accept MsiConnection.
vm/devices/pci/vpci/src/test_helpers/mod.rs Updates VPCI test interrupt controller to implement SignalMsi.
vm/devices/pci/vpci/src/device.rs Updates VPCI tests to use MsiConnection for MSI-X wiring.
vm/devices/pci/pcie/src/port.rs Updates PCIe port MSI capability creation to use MsiConnection/MsiTarget.
vm/devices/pci/pci_resources/src/lib.rs Changes resolve params to pass &MsiTarget instead of RegisterMsi.
vm/devices/pci/pci_core/src/test_helpers/mod.rs Updates PCI core test interrupt controller to implement SignalMsi.
vm/devices/pci/pci_core/src/msi.rs Introduces SignalMsi, MsiConnection, and MsiTarget.
vm/devices/pci/pci_core/src/capabilities/msix.rs Reworks MSI-X interrupt backing to use MsiTarget and internal pending state.
vm/devices/pci/pci_core/src/capabilities/msi_cap.rs Reworks MSI capability to use MsiTarget and shared interrupt backing.
vm/devices/net/net_mana/src/test.rs Updates MANA device tests to use MsiConnection.
vm/devices/net/mana_driver/src/tests.rs Updates MANA driver tests to use MsiConnection.
vm/devices/net/gdma/src/resolver.rs Updates resolver input from register_msi to msi_target.
vm/devices/net/gdma/src/lib.rs Updates GDMA PCI device to pass &MsiTarget into MSI-X emulator.
openvmm/openvmm_core/src/worker/dispatch.rs Updates VM initialization to use into_signal_msi and MsiConnection.
openvmm/openvmm_core/src/partition.rs Renames partition MSI accessor to into_signal_msi and updates trait bounds.
openhcl/virt_mshv_vtl/src/lib.rs Updates OpenHCL MSI target implementation to the SignalMsi trait.
Comments suppressed due to low confidence (1)

vm/devices/user_driver_emulated_mock/src/lib.rs:126

  • msix_table_size is detected from config space, but MSI-X table programming still loops over a fixed 0..64. For devices configured with fewer vectors (e.g., NVMe tests using msix_count = 2), this will write past the end of the MSI-X table BAR region and may cause unexpected behavior. Consider iterating up to msix_table_size (and/or validating it against the BAR length) when initializing the MSI-X table entries.
        // Determine the number of MSI-X vectors.
        let msix_table_size = {
            let mut n = 0;
            device.pci_cfg_read(0x40, &mut n).unwrap();
            ((n >> 16) & 0x7ff) + 1
        } as usize;

        // Connect an interrupt controller.
        let controller = Arc::new(MsiController::new(msix_table_size));
        msi_conn.connect(controller.clone());

        // Enable MSIX.
        for i in 0u64..64 {
            device
                .mmio_write((0x1 << 32) + i * 16, &i.to_ne_bytes())
                .unwrap();
            device
                .mmio_write((0x1 << 32) + i * 16 + 12, &0u32.to_ne_bytes())
                .unwrap();
        }
        device.pci_cfg_write(0x40, 0x80000000).unwrap();

@github-actions
Copy link

Copy link
Member

@chris-oo chris-oo left a comment

Choose a reason for hiding this comment

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

LGTM other than the comment about kvm. I'll leave it to you if you want to fix + push or just leave dead code around.

impl KvmPartitionInner {
/// Reserves a new route, optionally with an associated irqfd event.
#[expect(dead_code)]
pub(crate) fn new_route(self: &Arc<Self>, irqfd_event: Option<Event>) -> Option<GsiRoute> {
Copy link
Member

Choose a reason for hiding this comment

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

is it worth keeping this code around if we're not even using it? or you think we'll use it again soon?

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.

2 participants