pci_core: preserve PCIe topology across reset by jstarks · Pull Request #4010 · microsoft/openvmm

jstarks · 2026-07-23T12:39:16Z

PCIe physical presence belongs to the configured device topology rather than resettable or migratable capability-register state. Keeping it in saved register state caused a reboot to discard a dynamically attached endpoint even though the port still owned the device.

Separate external presence from guest-programmable registers and derive live link and slot status from the current topology. Save only state that the emulator actually produces, sanitize control state according to advertised capabilities, and preserve reserved PCIe encodings without panicking. Extend the hotplug regression to verify that an attached endpoint remains enumerated after a guest reboot.

Thanks to @bitranox for identifying the reset/topology issue and suggesting the original fix in #3915.

Copilot

Pull request overview

This PR fixes PCIe hotplug/reset semantics by separating physical presence/topology (externally managed, should survive reset/migration decisions) from guest-programmable PCIe capability registers (resettable/saved as appropriate). It updates the PCIe capability model to avoid panicking on reserved encodings and extends the PCIe hotplug regression to ensure an attached endpoint remains enumerated across a guest reboot.

Changes:

Refactors PCIe capability state to keep “presence detect” out of saved/reset register state and derives live Link/Slot Status from current topology.
Switches several PCIe spec “enum” encodings to open_enum! so reserved values round-trip without unreachable!() panics.
Extends the PCIe hotplug VMM test to validate hot-added devices persist across a guest reboot before removal.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
vmm_tests/vmm_tests/tests/tests/multiarch/pcie.rs	Extends hotplug test to assert the endpoint remains present after a guest reboot.
vm/devices/pci/pcie/src/port.rs	Makes ACS filtering tolerant of unknown/extended `DevicePortType` values.
vm/devices/pci/pci_core/src/spec.rs	Converts PCIe link/port encodings to `open_enum!` and updates bitfield types accordingly.
vm/devices/pci/pci_core/src/capabilities/pci_express.rs	Splits PCIe presence from registers, sanitizes writable control fields, and narrows saved/restored state to emulator-modeled registers/events.

PCIe physical presence belongs to the configured device topology rather than resettable or migratable capability-register state. Keeping it in saved register state caused a reboot to discard a dynamically attached endpoint even though the port still owned the device. Separate external presence from guest-programmable registers and derive live link and slot status from the current topology. Save only state that the emulator actually produces, sanitize control state according to advertised capabilities, and preserve reserved PCIe encodings without panicking. Extend the hotplug regression to verify that an attached endpoint remains enumerated after a guest reboot.

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

vm/devices/pci/pci_core/src/capabilities/pci_express.rs:86

link_status() reports a fixed negotiated speed/width even when data_link_layer_link_active is false. That produces an internally inconsistent Link Status (link down but trained at Gen5 x16), and can confuse guest drivers that interpret non-zero speed/width as “link up”. Consider zeroing the negotiated fields when the link is inactive.

        pci_express::LinkStatus::new()
            .with_current_link_speed(LinkSpeed::Speed32_0GtS)
            .with_negotiated_link_width(LinkWidth::X16)
            .with_data_link_layer_link_active(self.presence_detect_state)
    }

github-actions · 2026-07-23T14:30:15Z

2 Petri tests failed (1 unstable)

Copilot

Pull request overview

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

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

jackschefer-msft · 2026-07-23T16:48:26Z

+            #[derive(Inspect)]
+            #[inspect(debug)]
+            pub enum LinkSpeed: u32 {
+                #![allow(non_upper_case_globals)]


sorry if this is a dumb question, but why is this needed (here and throughout)

open_enum generates associated constants, which Rust expects to LOOK_LIKE_THIS. This allows us to use EnumStyleNames instead.

I didn't want to rename this stuff so I put the allow in. Could have gone the other way, and maybe we will in the future.

github-actions · 2026-07-23T18:54:33Z

2 Petri tests failed (1 unstable)

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

vm/devices/pci/pci_core/src/capabilities/pci_express.rs:802

The #[mesh(n)] field numbers in this saved-state struct were renumbered after removing fields. Even if some fields are no longer saved, reusing their numbers can break snapshot/save-restore compatibility once any state exists in the wild. Other saved-state structs in this repo intentionally leave gaps to preserve compatibility (e.g. MSI-X SavedState uses #[mesh(2)]/#[mesh(3)] in vm/devices/pci/pci_core/src/capabilities/msix.rs:539-544). Suggest keeping the original numbering and leaving gaps for removed fields.

            #[mesh(1)]
            pub device_control: u16,
            #[mesh(2)]
            pub link_control: u16,
            #[mesh(3)]

+    agent.reboot().await?;
+    let agent = vm.wait_for_reset().await?;
+
+    let devices = parse_guest_pci_devices(os_flavor, &agent).await?;
+    let endpoints = devices.iter().filter(|d| d.class_code != 0x060400).count();
+    tracing::info!(?devices, "PCI devices after reboot");
+    assert_eq!(endpoints, 1, "expected hot-added endpoint after reboot");
+


github-actions · 2026-07-24T09:15:20Z

2 Petri tests failed (0 unstable)

jstarks requested a review from a team as a code owner July 23, 2026 12:39

Copilot AI review requested due to automatic review settings July 23, 2026 12:39

Copilot started reviewing on behalf of jstarks July 23, 2026 12:40 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Comment thread vm/devices/pci/pci_core/src/capabilities/pci_express.rs

jstarks added 2 commits July 23, 2026 05:46

tweak

7afa538

jstarks force-pushed the fix-pcie-external-state branch from b4d4595 to 7afa538 Compare July 23, 2026 13:13

Copilot AI review requested due to automatic review settings July 23, 2026 13:13

Copilot started reviewing on behalf of jstarks July 23, 2026 13:14 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Comment thread vm/devices/pci/pci_core/src/capabilities/pci_express.rs Outdated

Comment thread vm/devices/pci/pci_core/src/capabilities/pci_express.rs

cleanup

2c7d4bb

Copilot AI review requested due to automatic review settings July 23, 2026 15:06

Copilot started reviewing on behalf of jstarks July 23, 2026 15:07 View session

margical cleanup

70ef7ab

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 23, 2026 15:14

Copilot started reviewing on behalf of jstarks July 23, 2026 15:15 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Comment thread vm/devices/pci/pci_core/src/capabilities/pci_express.rs

jackschefer-msft reviewed Jul 23, 2026

View reviewed changes

jackschefer-msft approved these changes Jul 23, 2026

View reviewed changes

jstarks enabled auto-merge (squash) July 23, 2026 21:11

disable test on mshv

af5e2fe

Copilot AI review requested due to automatic review settings July 24, 2026 08:09

Copilot started reviewing on behalf of jstarks July 24, 2026 08:10 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

jackschefer-msft approved these changes Jul 24, 2026

View reviewed changes

jstarks merged commit 657d368 into microsoft:main Jul 24, 2026
68 of 69 checks passed

jstarks deleted the fix-pcie-external-state branch July 24, 2026 14:03

Uh oh!

Conversation

jstarks commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

jackschefer-msft Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

jstarks Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

github-actions Bot commented Jul 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jstarks commented Jul 23, 2026 •

edited

Loading