Conversation
|
This PR modifies files containing For more on why we check whole files, instead of just diffs, check out the Rustonomicon |
There was a problem hiding this comment.
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/MsiInterruptSetmodel withSignalMsiplusMsiConnection/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_sizeis detected from config space, but MSI-X table programming still loops over a fixed0..64. For devices configured with fewer vectors (e.g., NVMe tests usingmsix_count = 2), this will write past the end of the MSI-X table BAR region and may cause unexpected behavior. Consider iterating up tomsix_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();
chris-oo
left a comment
There was a problem hiding this comment.
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> { |
There was a problem hiding this comment.
is it worth keeping this code around if we're not even using it? or you think we'll use it again soon?
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.