petri/vmm_tests: consolidate pcie topology construction and add more tests#3118
Conversation
There was a problem hiding this comment.
Pull request overview
This PR centralizes PCIe topology construction in the Petri OpenVMM backend and expands PCIe-related integration coverage by adding topology and device enumeration tests.
Changes:
- Introduces
PetriVmConfigOpenVmm::with_pcie_topology(...)plus helpers to attach PCIe NVMe and PCIe (MANA) NIC devices. - Refactors existing PCIe root emulation test to use the new topology helper and adds new PCIe device enumeration assertions.
- Updates the burette network perf test to explicitly construct a PCIe topology and target a specific root port for virtio-net.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| vmm_tests/vmm_tests/tests/tests/multiarch/pcie.rs | Switches tests to with_pcie_topology and adds a Linux PCIe device enumeration test (NVMe + NIC). |
| petri/src/vm/openvmm/modify.rs | Adds PCIe topology/device builder helpers and changes with_virtio_nic to take a port_name. |
| petri/burette/src/tests/network.rs | Updates virtio-net perf path to create a PCIe topology and attach the NIC to s0rc0rp0. |
| let low_mmio_start = self.config.memory.mmio_gaps[0].start(); | ||
| let high_mmio_end = self.config.memory.mmio_gaps[1].end(); | ||
|
|
There was a problem hiding this comment.
with_pcie_topology uses mmio_gaps[1] for high_mmio_end. On configs with VTL2 enabled (3 MMIO gaps), index 1 is not the last gap, so the computed PCI high MMIO range can overlap the VTL2 MMIO gap and make the memory layout invalid. Use mmio_gaps.first()/last() (and handle missing gaps) when deriving these boundaries.
| let low_mmio_start = self.config.memory.mmio_gaps[0].start(); | |
| let high_mmio_end = self.config.memory.mmio_gaps[1].end(); | |
| let mmio_gaps = &self.config.memory.mmio_gaps; | |
| let Some(first_gap) = mmio_gaps.first() else { | |
| // No MMIO gaps configured; cannot derive PCIe MMIO layout. | |
| return self; | |
| }; | |
| let last_gap = mmio_gaps.last().unwrap(); | |
| let low_mmio_start = first_gap.start(); | |
| let high_mmio_end = last_gap.end(); |
jackschefer-msft
changed the title
jackschefer-msft
changed the title
|
This is no longer WIP and is ready for review |
chris-oo
left a comment
chris-oo
left a comment
There was a problem hiding this comment.
nice work, just asking for some comments on the tests so you can tell at a glance what that testcase is testing
| } | ||
|
|
||
| /// Add a PCIe NVMe device to the VM using the NVMe emulator. | ||
| pub fn with_pcie_nvme(mut self, port_name: &str, subsystem_id: Guid) -> Self { |
There was a problem hiding this comment.
this makes some changes i have for pcie + uefi support much easier... neat.
There was a problem hiding this comment.
Yeah, I only added support for a small ramdisk because that seemed easiest for my needs, but should be extensible to actual backing disks
| index: index.try_into().unwrap(), | ||
| name, | ||
| segment: segment.try_into().unwrap(), | ||
| start_bus: start_bus.try_into().unwrap(), | ||
| end_bus: end_bus.try_into().unwrap(), |
There was a problem hiding this comment.
The try_into().unwrap() conversions for index/segment/start_bus/end_bus can panic on out-of-range inputs (e.g., large segment counts or RC counts). Since this is a public helper, prefer validating bounds up front (or returning a Result) and producing a clear error instead of panicking via unwrap().
Uh oh!
There was an error while loading. Please reload this page.