openvmm/pcie: add save restore for pcie devices #2639

shenw0000 · 2026-01-14T21:13:04Z

Add save and restore for the PCIe Root Complex and PCIe Switch

Copilot

Pull request overview

This PR adds save and restore functionality for PCIe devices including the PCIe Root Complex and PCIe Switch. Previously, these devices returned SavedStateNotSupported, which prevented VMs with PCIe configurations from being saved and restored. This enhancement enables save/restore for VMs using PCIe emulation.

Changes:

Implemented SaveRestore trait for GenericPcieSwitch and GenericPcieRootComplex
Enhanced PciExpressCapability to save/restore all control and status registers
Updated petri test infrastructure to enable save/restore testing for PCIe configurations

Reviewed changes

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

Show a summary per file

File	Description
vm/devices/pci/pcie/src/switch.rs	Added save/restore implementation for GenericPcieSwitch with upstream and downstream port configuration space state, plus comprehensive tests
vm/devices/pci/pcie/src/root.rs	Added save/restore implementation for GenericPcieRootComplex including bus number validation and root port configuration space state, plus tests
vm/devices/pci/pci_core/src/capabilities/pci_express.rs	Expanded PciExpressCapability saved state to include all control/status registers (device, link, slot, root) and their _2 variants
vm/devices/pci/pcie/Cargo.toml	Added mesh_protobuf dependency required for protobuf serialization
petri/src/vm/openvmm/start.rs	Removed PCIe check from save/restore support detection to enable testing
Cargo.lock	Updated with mesh_protobuf dependency for pcie crate

vm/devices/pci/pcie/src/switch.rs

vm/devices/pci/pcie/src/root.rs

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

mattkur

A few comments. Overall, thank you for adding tests here!

mattkur · 2026-01-15T17:33:06Z

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

            pub device_status: u16,
            #[mesh(3)]
-            pub flr_handler: u16,
+            pub link_control: u16,


Please confirm why it is safe to change this struct at this point. (Asking out of a due diligence)

this was not used before (i.e. save returned error before, so I don't think there will be a migration issue) - and I don't think it is right to save the flr_handler to begin with. flr_handler is a pointer to the handler routine which does not make sense after restore

vm/devices/pci/pcie/src/switch.rs

mattkur · 2026-01-15T17:38:45Z

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

-            Err(SaveError::NotSupported)
+            let state = self.state.lock();
+            Ok(state::SavedState {
+                device_control: state.device_control.into_bits(),


A few comments:

I thought the save operation was to be idempotent, whereas into_bits will take ownership of each of these fields. Paging @smalis-msft to correct me on the idempotency of save.

You can destructure state to make sure that you get a compile error here if you add more state to device_control but forget to adjust the save state.

Why did you choose to use u16 for the save state, rather than the type itself? I had thought mesh supported generic Into/From bytes, but I am almost certainly mistaken. Is that why you're using u16?

into_bits takes ownership but the types are Copy, so it's fine. save does need to be idempotent yes.

And yes please do destructure types in save/restore impls whenever possible.

@mattkur thanks for the comments! for u16 - I did that just because other examples were using that. Is it more recommended to use the actual type?

I'll leave that up to you. It looked a little weird to me to do this shuffle to/from bits explicitly. It also adds work here. But ... I put this squarely in "style" so I'll leave it up to you.

@mattkur I researched around and it seems that this mesh::payload::encoding::ZeroCopyEncoding is what you are looking for? could you please help see if this update makes sense? I did add unit tests and that worked

smalis-msft · 2026-01-15T19:45:30Z

Could we hold off on merging this until we have Gen 1 servicing tests onboarded, to make sure this change doesn't break that flow? Should be pretty soon.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…enw0000/openvmm into add_save_restore_pcie_devices

github-actions · 2026-01-17T00:48:36Z

At least one Petri test failed.

shenw0000 · 2026-01-20T18:30:50Z

Could we hold off on merging this until we have Gen 1 servicing tests onboarded, to make sure this change doesn't break that flow? Should be pretty soon.
@smalis-msft wondering are gen1 servicing tests onboarded yet? thank you!

shenw0000 added 4 commits January 14, 2026 19:27

add save and restore for pcie switch

8095887

save restore root complex

2389cda

remove pcie check in petri

4c20514

remove pcie check in petri

fba91d3

shenw0000 requested a review from a team as a code owner January 14, 2026 21:13

Copilot AI review requested due to automatic review settings January 14, 2026 21:13

shenw0000 requested a review from a team as a code owner January 14, 2026 21:13

Copilot started reviewing on behalf of shenw0000 January 14, 2026 21:13 View session

Copilot AI reviewed Jan 14, 2026

View reviewed changes

vm/devices/pci/pcie/src/switch.rs Show resolved Hide resolved

vm/devices/pci/pcie/src/root.rs Outdated Show resolved Hide resolved

vm/devices/pci/pci_core/src/capabilities/pci_express.rs Show resolved Hide resolved

mattkur reviewed Jan 15, 2026

View reviewed changes

shenw0000 and others added 6 commits January 15, 2026 20:25

CR

0d632b2

Apply suggestion from @Copilot

0771b0c

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

CR: add unit test

b19f4cc

Merge branch 'main' into add_save_restore_pcie_devices

d328c8a

change save restore format

0b679c8

Merge branch 'add_save_restore_pcie_devices' of https://github.com/sh…

5f18cdf

…enw0000/openvmm into add_save_restore_pcie_devices

Merge branch 'main' into add_save_restore_pcie_devices

6fd9c49

Merge branch 'main' into add_save_restore_pcie_devices

b699220

openvmm/pcie: add save restore for pcie devices #2639

Are you sure you want to change the base?

openvmm/pcie: add save restore for pcie devices #2639

Uh oh!

Conversation

shenw0000 commented Jan 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mattkur left a comment

Choose a reason for hiding this comment

Uh oh!

mattkur Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

shenw0000 Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mattkur Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

smalis-msft Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

smalis-msft Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

shenw0000 Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

mattkur Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

shenw0000 Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

smalis-msft commented Jan 15, 2026

Uh oh!

github-actions bot commented Jan 17, 2026

Uh oh!

shenw0000 commented Jan 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shenw0000 Jan 15, 2026 •

edited

Loading