Skip to content

Commit a2ffc5b

Browse files
committed
refactor: introduce struct VmCommon
This struct will hold all the fields that are common between the x86_64 and aarch64 definition of `struct ArchVm`. For now, this is only the `VmFd`, but this will grow. Signed-off-by: Patrick Roy <[email protected]>
1 parent b020bd1 commit a2ffc5b

File tree

3 files changed

+42
-32
lines changed

3 files changed

+42
-32
lines changed

src/vmm/src/vstate/vm/aarch64.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,16 @@
11
// Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
use kvm_ioctls::VmFd;
54
use serde::{Deserialize, Serialize};
65

7-
use super::VmError;
6+
use super::{VmCommon, VmError};
87
use crate::Kvm;
98
use crate::arch::aarch64::gic::GicState;
109

1110
/// Structure representing the current architecture's understand of what a "virtual machine" is.
1211
#[derive(Debug)]
1312
pub struct ArchVm {
14-
pub(super) fd: VmFd,
13+
pub(super) common: VmCommon,
1514
// On aarch64 we need to keep around the fd obtained by creating the VGIC device.
1615
irqchip_handle: Option<crate::arch::aarch64::gic::GICDevice>,
1716
}
@@ -30,9 +29,9 @@ pub enum ArchVmError {
3029
impl ArchVm {
3130
/// Create a new `Vm` struct.
3231
pub fn new(kvm: &Kvm) -> Result<ArchVm, VmError> {
33-
let fd = Self::create_vm(kvm)?;
32+
let common = Self::create_common(kvm)?;
3433
Ok(ArchVm {
35-
fd,
34+
common,
3635
irqchip_handle: None,
3736
})
3837
}
@@ -52,7 +51,7 @@ impl ArchVm {
5251
/// Creates the GIC (Global Interrupt Controller).
5352
pub fn setup_irqchip(&mut self, vcpu_count: u8) -> Result<(), ArchVmError> {
5453
self.irqchip_handle = Some(
55-
crate::arch::aarch64::gic::create_gic(&self.fd, vcpu_count.into(), None)
54+
crate::arch::aarch64::gic::create_gic(&self.fd(), vcpu_count.into(), None)
5655
.map_err(ArchVmError::VmCreateGIC)?,
5756
);
5857
Ok(())

src/vmm/src/vstate/vm/mod.rs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ pub use arch::{ArchVm as Vm, ArchVmError, VmState};
2424
use crate::Vcpu;
2525
use crate::vstate::vcpu::VcpuError;
2626

27+
#[derive(Debug)]
28+
struct VmCommon {
29+
fd: VmFd,
30+
}
31+
2732
/// Errors associated with the wrappers over KVM ioctls.
2833
/// Needs `rustfmt::skip` to make multiline comments work
2934
#[rustfmt::skip]
@@ -43,7 +48,7 @@ pub enum VmError {
4348

4449
/// Contains Vm functions that are usable across CPU architectures
4550
impl Vm {
46-
fn create_vm(kvm: &crate::vstate::kvm::Kvm) -> Result<VmFd, VmError> {
51+
fn create_common(kvm: &crate::vstate::kvm::Kvm) -> Result<VmCommon, VmError> {
4752
// It is known that KVM_CREATE_VM occasionally fails with EINTR on heavily loaded machines
4853
// with many VMs.
4954
//
@@ -65,18 +70,22 @@ impl Vm {
6570
// retry, they have to start Firecracker from scratch. Doing retries in Firecracker makes
6671
// recovery faster and improves reliability.
6772
const MAX_ATTEMPTS: u32 = 5;
68-
for attempt in 1..=MAX_ATTEMPTS {
73+
let mut attempt = 1;
74+
let fd = loop {
6975
match kvm.fd.create_vm() {
70-
Ok(fd) => return Ok(fd),
76+
Ok(fd) => break fd,
7177
Err(e) if e.errno() == libc::EINTR && attempt < MAX_ATTEMPTS => {
7278
info!("Attempt #{attempt} of KVM_CREATE_VM returned EINTR");
7379
// Exponential backoff (1us, 2us, 4us, and 8us => 15us in total)
7480
std::thread::sleep(std::time::Duration::from_micros(2u64.pow(attempt - 1)));
7581
}
7682
Err(e) => return Err(VmError::CreateVm(e)),
7783
}
78-
}
79-
unreachable!();
84+
85+
attempt += 1;
86+
};
87+
88+
Ok(VmCommon { fd })
8089
}
8190

8291
/// Creates the specified number of [`Vcpu`]s.
@@ -127,15 +136,15 @@ impl Vm {
127136
};
128137

129138
// SAFETY: Safe because the fd is a valid KVM file descriptor.
130-
unsafe { self.fd.set_user_memory_region(memory_region) }
139+
unsafe { self.fd().set_user_memory_region(memory_region) }
131140
})
132141
.map_err(VmError::SetUserMemoryRegion)?;
133142
Ok(())
134143
}
135144

136145
/// Gets a reference to the kvm file descriptor owned by this VM.
137146
pub fn fd(&self) -> &VmFd {
138-
&self.fd
147+
&self.common.fd
139148
}
140149
}
141150

src/vmm/src/vstate/vm/x86_64.rs

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ use kvm_bindings::{
77
KVM_CLOCK_TSC_STABLE, KVM_IRQCHIP_IOAPIC, KVM_IRQCHIP_PIC_MASTER, KVM_IRQCHIP_PIC_SLAVE,
88
KVM_PIT_SPEAKER_DUMMY, MsrList, kvm_clock_data, kvm_irqchip, kvm_pit_config, kvm_pit_state2,
99
};
10-
use kvm_ioctls::{Cap, VmFd};
10+
use kvm_ioctls::Cap;
1111
use serde::{Deserialize, Serialize};
1212

1313
use crate::arch::x86_64::msr::MsrError;
1414
use crate::utils::u64_to_usize;
15-
use crate::vstate::vm::VmError;
15+
use crate::vstate::vm::{VmCommon, VmError};
1616

1717
/// Error type for [`Vm::restore_state`]
1818
#[allow(missing_docs)]
@@ -48,7 +48,7 @@ pub enum ArchVmError {
4848
/// Structure representing the current architecture's understand of what a "virtual machine" is.
4949
#[derive(Debug)]
5050
pub struct ArchVm {
51-
pub(super) fd: VmFd,
51+
pub(super) common: VmCommon,
5252
msrs_to_save: MsrList,
5353
/// Size in bytes requiring to hold the dynamically-sized `kvm_xsave` struct.
5454
///
@@ -59,7 +59,7 @@ pub struct ArchVm {
5959
impl ArchVm {
6060
/// Create a new `Vm` struct.
6161
pub fn new(kvm: &crate::vstate::kvm::Kvm) -> Result<ArchVm, VmError> {
62-
let fd = Self::create_vm(kvm)?;
62+
let common = Self::create_common(kvm)?;
6363

6464
let msrs_to_save = kvm.msrs_to_save().map_err(ArchVmError::GetMsrsToSave)?;
6565

@@ -69,7 +69,7 @@ impl ArchVm {
6969
// https://github.com/torvalds/linux/commit/be50b2065dfa3d88428fdfdc340d154d96bf6848
7070
//
7171
// Cache the value in order not to call it at each vCPU creation.
72-
let xsave2_size = match fd.check_extension_int(Cap::Xsave2) {
72+
let xsave2_size = match common.fd.check_extension_int(Cap::Xsave2) {
7373
// Catch all negative values just in case although the possible negative return value
7474
// of ioctl() is only -1.
7575
..=-1 => {
@@ -83,11 +83,13 @@ impl ArchVm {
8383
ret => Some(usize::try_from(ret).unwrap()),
8484
};
8585

86-
fd.set_tss_address(u64_to_usize(crate::arch::x86_64::layout::KVM_TSS_ADDRESS))
86+
common
87+
.fd
88+
.set_tss_address(u64_to_usize(crate::arch::x86_64::layout::KVM_TSS_ADDRESS))
8789
.map_err(ArchVmError::SetTssAddress)?;
8890

8991
Ok(ArchVm {
90-
fd,
92+
common,
9193
msrs_to_save,
9294
xsave2_size,
9395
})
@@ -113,27 +115,27 @@ impl ArchVm {
113115
/// - [`kvm_ioctls::VmFd::set_irqchip`] errors.
114116
/// - [`kvm_ioctls::VmFd::set_irqchip`] errors.
115117
pub fn restore_state(&mut self, state: &VmState) -> Result<(), ArchVmError> {
116-
self.fd
118+
self.fd()
117119
.set_pit2(&state.pitstate)
118120
.map_err(ArchVmError::SetPit2)?;
119-
self.fd
121+
self.fd()
120122
.set_clock(&state.clock)
121123
.map_err(ArchVmError::SetClock)?;
122-
self.fd
124+
self.fd()
123125
.set_irqchip(&state.pic_master)
124126
.map_err(ArchVmError::SetIrqChipPicMaster)?;
125-
self.fd
127+
self.fd()
126128
.set_irqchip(&state.pic_slave)
127129
.map_err(ArchVmError::SetIrqChipPicSlave)?;
128-
self.fd
130+
self.fd()
129131
.set_irqchip(&state.ioapic)
130132
.map_err(ArchVmError::SetIrqChipIoAPIC)?;
131133
Ok(())
132134
}
133135

134136
/// Creates the irq chip and an in-kernel device model for the PIT.
135137
pub fn setup_irqchip(&self) -> Result<(), ArchVmError> {
136-
self.fd
138+
self.fd()
137139
.create_irq_chip()
138140
.map_err(ArchVmError::VmSetIrqChip)?;
139141
// We need to enable the emulation of a dummy speaker port stub so that writing to port 0x61
@@ -142,40 +144,40 @@ impl ArchVm {
142144
flags: KVM_PIT_SPEAKER_DUMMY,
143145
..Default::default()
144146
};
145-
self.fd
147+
self.fd()
146148
.create_pit2(pit_config)
147149
.map_err(ArchVmError::VmSetIrqChip)
148150
}
149151

150152
/// Saves and returns the Kvm Vm state.
151153
pub fn save_state(&self) -> Result<VmState, ArchVmError> {
152-
let pitstate = self.fd.get_pit2().map_err(ArchVmError::VmGetPit2)?;
154+
let pitstate = self.fd().get_pit2().map_err(ArchVmError::VmGetPit2)?;
153155

154-
let mut clock = self.fd.get_clock().map_err(ArchVmError::VmGetClock)?;
156+
let mut clock = self.fd().get_clock().map_err(ArchVmError::VmGetClock)?;
155157
// This bit is not accepted in SET_CLOCK, clear it.
156158
clock.flags &= !KVM_CLOCK_TSC_STABLE;
157159

158160
let mut pic_master = kvm_irqchip {
159161
chip_id: KVM_IRQCHIP_PIC_MASTER,
160162
..Default::default()
161163
};
162-
self.fd
164+
self.fd()
163165
.get_irqchip(&mut pic_master)
164166
.map_err(ArchVmError::VmGetIrqChip)?;
165167

166168
let mut pic_slave = kvm_irqchip {
167169
chip_id: KVM_IRQCHIP_PIC_SLAVE,
168170
..Default::default()
169171
};
170-
self.fd
172+
self.fd()
171173
.get_irqchip(&mut pic_slave)
172174
.map_err(ArchVmError::VmGetIrqChip)?;
173175

174176
let mut ioapic = kvm_irqchip {
175177
chip_id: KVM_IRQCHIP_IOAPIC,
176178
..Default::default()
177179
};
178-
self.fd
180+
self.fd()
179181
.get_irqchip(&mut ioapic)
180182
.map_err(ArchVmError::VmGetIrqChip)?;
181183

0 commit comments

Comments
 (0)