Skip to content

smmu: implement save/restore for SMMUv3 device#3581

Open
jstarks wants to merge 2 commits into
microsoft:mainfrom
jstarks:smmu_sr
Open

smmu: implement save/restore for SMMUv3 device#3581
jstarks wants to merge 2 commits into
microsoft:mainfrom
jstarks:smmu_sr

Conversation

@jstarks
Copy link
Copy Markdown
Member

@jstarks jstarks commented May 28, 2026

The SMMU was already registered as a chipset device and participated in the state unit framework, but its SaveRestore implementation returned SaveError::NotSupported.

This replaces the stub with a real implementation. Both save() and restore() exhaustively destructure their respective structs so that adding a new field without updating serialization is a compile error, following the same pattern already used by reset().

On restore, all derived state in SmmuSharedState is re-synced from the restored register values. This ensures that per-device SmmuTranslatingMemory wrappers, which hold an Arc to the same shared state, resume translating correctly without being reconstructed.

The SMMU was already registered as a chipset device and participated in
the state unit framework, but its SaveRestore implementation returned
SaveError::NotSupported.

This replaces the stub with a real implementation. Both save() and
restore() exhaustively destructure their respective structs so that adding
a new field without updating serialization is a compile error, following
the same pattern already used by reset().

On restore, all derived state in SmmuSharedState is re-synced from the
restored register values. This ensures that per-device
SmmuTranslatingMemory wrappers, which hold an Arc to the same shared
state, resume translating correctly without being reconstructed.
Copilot AI review requested due to automatic review settings May 28, 2026 05:57
@github-actions github-actions Bot added the unsafe Related to unsafe code label May 28, 2026
@github-actions
Copy link
Copy Markdown

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

Implements real save/restore support for the SMMUv3 chipset device so it can participate in VM state persistence (hibernation/migration) instead of failing with SaveError::NotSupported.

Changes:

  • Add SMMU saved-state protobuf schema and implement SaveRestore::save() / restore() for SmmuDevice.
  • Add shared-state helpers to save/restore the persistable subset of EVTQ/GERROR state.
  • Add unit tests validating translation correctness and CMDQ error persistence across save/restore.

Reviewed changes

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

File Description
vm/devices/iommu/smmu/src/shared.rs Adds SavedQueueState plus save/restore helpers for queue + global error state.
vm/devices/iommu/smmu/src/emulator.rs Replaces stub SaveRestore with protobuf-backed saved state and adds save/restore tests.
vm/devices/iommu/smmu/Cargo.toml Adds mesh dependency for protobuf saved state and pal_async for async tests.
Cargo.lock Updates lockfile for the new crate dependencies.

Comment thread vm/devices/iommu/smmu/src/shared.rs
Comment thread vm/devices/iommu/smmu/src/shared.rs
Comment thread vm/devices/iommu/smmu/src/emulator.rs
@jstarks jstarks marked this pull request as ready for review May 28, 2026 07:07
@jstarks jstarks requested a review from a team as a code owner May 28, 2026 07:07
Copilot AI review requested due to automatic review settings May 28, 2026 07:07
Copy link
Copy Markdown
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

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

Comment on lines +732 to +777
let &mut SmmuDevice {
// Static configuration — not saved.
mmio_region: _,
mmio_base: _,
guest_memory: _,
ref shared_state,

// Identification registers — read-only, not saved.
idr0: _,
idr1: _,
idr2: _,
idr3: _,
idr4: _,
idr5: _,
iidr: _,
aidr: _,

// Control registers.
cr0,
cr0ack: _, // mirror of cr0 (immediate ack)
cr1,
cr2,
gbpa,

// Interrupt control.
irq_ctrl,
irq_ctrlack: _, // mirror of irq_ctrl (immediate ack)

// Stream table base.
strtab_base,
strtab_base_cfg,

// Command queue.
cmdq_base,
cmdq_prod,
cmdq_cons,

// Event queue base register.
evtq_base,

// MSI configuration.
ref gerror_msi,
ref evtq_msi,
ref cmdq_msi,
} = self;


impl SavedMsiConfig {
pub(super) fn save(msi: &super::MsiConfig) -> Self {
let super::MsiConfig { addr, data, attr } = *msi;
Comment on lines +353 to +367
pub(crate) fn save_queue_state(&self) -> SavedQueueState {
let qs = self.queue_state.lock();
// Exhaustively destructure to get a compile error if a field is added.
let QueueErrorState {
evtq_base_addr: _,
evtq_log2size: _,
evtq_enabled: _,
evtq_irqen: _,
evtq_prod,
evtq_cons,
gerror,
gerrorn,
gerror_irqen: _,
} = *qs;
SavedQueueState {
Comment on lines +375 to +398
/// Restores the queue and error state from a saved snapshot.
///
/// The caller must re-sync derived fields (`set_evtq_config`,
/// `set_evtq_enabled`, `set_irq_ctrl`) separately after this call.
pub(crate) fn restore_queue_state(&self, state: SavedQueueState) {
let SavedQueueState {
evtq_prod,
evtq_cons,
gerror,
gerrorn,
} = state;
let mut qs = self.queue_state.lock();
qs.evtq_prod = evtq_prod;
qs.evtq_cons = evtq_cons;
qs.gerror = registers::Gerror::from(gerror);
qs.gerrorn = registers::Gerror::from(gerrorn);
self.update_gerror_irq(&qs);
// Sync EVTQ wired interrupt line to match restored queue state.
if qs.evtq_irqen {
if let Some(irq) = &self.evtq_irq {
irq.set_level(qs.evtq_prod != qs.evtq_cons);
}
}
}
Comment on lines +2550 to +2551
// Save and restore.
let saved = dev.save().expect("save");
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