Skip to content

Commit 7cc6e91

Browse files
committed
test(pvtime): Add integration/unit tests and fix x86
- Added integration tests to validate PVTime steal time increases and persists after snapshot - Added unit tests for PVTime internals - Fixed build errors on x86_64 - Updated PVTime::vcpu_count to u64 for clarity and correctness - Bumped snapshot version and updated changelog Signed-off-by: Dakshin Devanand <[email protected]>
1 parent b825c74 commit 7cc6e91

File tree

5 files changed

+179
-21
lines changed

5 files changed

+179
-21
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ and this project adheres to
1010

1111
### Added
1212

13+
- [#5139](https://github.com/firecracker-microvm/firecracker/pull/5139): Added
14+
support for [PVTime](https://docs.kernel.org/virt/kvm/arm/pvtime.html). This is
15+
used to support steal time on ARM machines.
1316
- [#5048](https://github.com/firecracker-microvm/firecracker/pull/5048): Added
1417
support for [PVH boot mode](docs/pvh.md). This is used when an x86 kernel
1518
provides the appropriate ELF Note to indicate that PVH boot mode is supported.

src/vmm/src/arch/aarch64/pvtime.rs

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ pub const STEALTIME_STRUCT_MEM_SIZE: u64 = 64;
1818
#[derive(Debug)]
1919
pub struct PVTime {
2020
/// Number of vCPUs
21-
vcpu_count: u8,
21+
vcpu_count: u64,
2222
/// The base IPA of the shared memory region
2323
base_ipa: GuestAddress,
2424
}
@@ -29,25 +29,25 @@ pub enum PVTimeError {
2929
/// Failed to allocate memory region: {0}
3030
AllocationFailed(vm_allocator::Error),
3131
/// Invalid VCPU ID: {0}
32-
InvalidVcpuIndex(u8),
32+
InvalidVcpuIndex(u64),
3333
/// Error while setting or getting device attributes for vCPU: {0}
3434
DeviceAttribute(kvm_ioctls::Error),
3535
}
3636

3737
impl PVTime {
3838
/// Helper function to get the IPA of the steal_time region for a given vCPU
39-
fn get_steal_time_region_addr(&self, vcpu_index: u8) -> Result<GuestAddress, PVTimeError> {
39+
fn get_steal_time_region_addr(&self, vcpu_index: u64) -> Result<GuestAddress, PVTimeError> {
4040
if vcpu_index >= self.vcpu_count {
4141
return Err(PVTimeError::InvalidVcpuIndex(vcpu_index));
4242
}
4343
Ok(GuestAddress(
44-
self.base_ipa.0 + (vcpu_index as u64 * STEALTIME_STRUCT_MEM_SIZE),
44+
self.base_ipa.0 + (vcpu_index * STEALTIME_STRUCT_MEM_SIZE),
4545
))
4646
}
4747

4848
/// Create a new PVTime device given a base addr
4949
/// - Assumes total shared memory region from base addr is already allocated
50-
fn from_base(base_ipa: GuestAddress, vcpu_count: u8) -> Self {
50+
fn from_base(base_ipa: GuestAddress, vcpu_count: u64) -> Self {
5151
PVTime {
5252
vcpu_count,
5353
base_ipa,
@@ -57,13 +57,13 @@ impl PVTime {
5757
/// Creates a new PVTime device by allocating new system memory for all vCPUs
5858
pub fn new(
5959
resource_allocator: &mut ResourceAllocator,
60-
vcpu_count: u8,
60+
vcpu_count: u64,
6161
) -> Result<Self, PVTimeError> {
6262
// This returns the IPA of the start of our shared memory region for all vCPUs.
6363
let base_ipa: GuestAddress = GuestAddress(
6464
resource_allocator
6565
.allocate_system_memory(
66-
STEALTIME_STRUCT_MEM_SIZE * vcpu_count as u64,
66+
STEALTIME_STRUCT_MEM_SIZE * vcpu_count,
6767
STEALTIME_STRUCT_MEM_SIZE,
6868
vm_allocator::AllocPolicy::LastMatch,
6969
)
@@ -89,7 +89,7 @@ impl PVTime {
8989
/// Register a vCPU with its pre-allocated steal time region
9090
fn register_vcpu(
9191
&self,
92-
vcpu_index: u8,
92+
vcpu_index: u64,
9393
vcpu_fd: &kvm_ioctls::VcpuFd,
9494
) -> Result<(), PVTimeError> {
9595
// Get IPA of the steal_time region for this vCPU
@@ -114,9 +114,7 @@ impl PVTime {
114114
pub fn register_all_vcpus(&self, vcpus: &mut [Vcpu]) -> Result<(), PVTimeError> {
115115
// Register the vcpu with the pvtime device to map its steal time region
116116
for (i, vcpu) in vcpus.iter().enumerate() {
117-
#[allow(clippy::cast_possible_truncation)]
118-
// We know vcpu_count is u8 according to VcpuConfig
119-
self.register_vcpu(i as u8, &vcpu.kvm_vcpu.fd)?;
117+
self.register_vcpu(i as u64, &vcpu.kvm_vcpu.fd)?;
120118
}
121119
Ok(())
122120
}
@@ -135,7 +133,7 @@ pub struct PVTimeConstructorArgs<'a> {
135133
/// For steal_time shared memory region
136134
pub resource_allocator: &'a mut ResourceAllocator,
137135
/// Number of vCPUs (should be consistent with pre-snapshot state)
138-
pub vcpu_count: u8,
136+
pub vcpu_count: u64,
139137
}
140138

141139
impl<'a> Persist<'a> for PVTime {
@@ -158,7 +156,7 @@ impl<'a> Persist<'a> for PVTime {
158156
constructor_args
159157
.resource_allocator
160158
.allocate_system_memory(
161-
STEALTIME_STRUCT_MEM_SIZE * constructor_args.vcpu_count as u64,
159+
STEALTIME_STRUCT_MEM_SIZE * constructor_args.vcpu_count,
162160
STEALTIME_STRUCT_MEM_SIZE,
163161
vm_allocator::AllocPolicy::ExactMatch(state.base_ipa),
164162
)
@@ -169,3 +167,41 @@ impl<'a> Persist<'a> for PVTime {
169167
))
170168
}
171169
}
170+
171+
#[cfg(test)]
172+
mod tests {
173+
use super::*;
174+
use crate::device_manager::resources::ResourceAllocator;
175+
176+
#[test]
177+
fn test_get_steal_time_region_addr() {
178+
let base = GuestAddress(0x1000);
179+
let dev = PVTime::from_base(base, 3);
180+
181+
assert_eq!(
182+
dev.get_steal_time_region_addr(0).unwrap(),
183+
GuestAddress(0x1000)
184+
);
185+
assert_eq!(
186+
dev.get_steal_time_region_addr(1).unwrap(),
187+
GuestAddress(0x1000 + STEALTIME_STRUCT_MEM_SIZE)
188+
);
189+
assert!(matches!(
190+
dev.get_steal_time_region_addr(3),
191+
Err(PVTimeError::InvalidVcpuIndex(3))
192+
));
193+
}
194+
195+
#[test]
196+
fn test_new_pvtime_allocates_correctly() {
197+
let mut allocator = ResourceAllocator::new().unwrap();
198+
let dev = PVTime::new(&mut allocator, 2).unwrap();
199+
200+
assert_eq!(dev.vcpu_count, 2);
201+
assert_eq!(
202+
dev.base_ipa.0 % STEALTIME_STRUCT_MEM_SIZE,
203+
0,
204+
"Base IPA should be aligned"
205+
);
206+
}
207+
}

src/vmm/src/builder.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ use std::sync::{Arc, Mutex};
1111
use event_manager::{MutEventSubscriber, SubscriberOps};
1212
use libc::EFD_NONBLOCK;
1313
use linux_loader::cmdline::Cmdline as LoaderKernelCmdline;
14-
use log::warn;
1514
use userfaultfd::Uffd;
1615
use utils::time::TimestampUs;
1716
#[cfg(target_arch = "aarch64")]
@@ -85,6 +84,7 @@ pub enum StartMicrovmError {
8584
/// Error creating VMGenID device: {0}
8685
CreateVMGenID(VmGenIdError),
8786
/// Error creating PVTime device: {0}
87+
#[cfg(target_arch = "aarch64")]
8888
CreatePVTime(PVTimeError),
8989
/// Invalid Memory Configuration: {0}
9090
GuestMemory(crate::vstate::memory::MemoryError),
@@ -301,7 +301,9 @@ pub fn build_microvm_for_boot(
301301
vmm.pv_time = if PVTime::is_supported(&vcpus[0].kvm_vcpu.fd) {
302302
Some(setup_pv_time(&mut vmm, vcpus.as_mut())?)
303303
} else {
304-
warn!("PVTime is not supported by KVM. Steal time will not be reported to the guest.");
304+
log::warn!(
305+
"PVTime is not supported by KVM. Steal time will not be reported to the guest."
306+
);
305307
None
306308
};
307309
}
@@ -420,6 +422,7 @@ pub enum BuildMicrovmFromSnapshotError {
420422
ACPIDeviManager(#[from] ACPIDeviceManagerRestoreError),
421423
/// VMGenID update failed: {0}
422424
VMGenIDUpdate(std::io::Error),
425+
#[cfg(target_arch = "aarch64")]
423426
/// Failed to restore PVTime device: {0}
424427
RestorePVTime(#[from] PVTimeError),
425428
}
@@ -477,11 +480,9 @@ pub fn build_microvm_from_snapshot(
477480
{
478481
let pvtime_state = microvm_state.pvtime_state;
479482
if let Some(pvtime_state) = pvtime_state {
480-
#[allow(clippy::cast_possible_truncation)]
481-
// We know vcpu_count is u8 according to VcpuConfig
482483
let pvtime_ctor_args = PVTimeConstructorArgs {
483484
resource_allocator: &mut vmm.resource_allocator,
484-
vcpu_count: vcpus.len() as u8,
485+
vcpu_count: vcpus.len() as u64,
485486
};
486487
vmm.pv_time = Some(
487488
PVTime::restore(pvtime_ctor_args, &pvtime_state)
@@ -596,8 +597,7 @@ fn setup_pv_time(vmm: &mut Vmm, vcpus: &mut [Vcpu]) -> Result<PVTime, StartMicro
596597
use crate::arch::aarch64::pvtime::PVTime;
597598

598599
// Create the pvtime device
599-
#[allow(clippy::cast_possible_truncation)] // We know vcpu_count is u8 according to VcpuConfig
600-
let pv_time = PVTime::new(&mut vmm.resource_allocator, vcpus.len() as u8)
600+
let pv_time = PVTime::new(&mut vmm.resource_allocator, vcpus.len() as u64)
601601
.map_err(StartMicrovmError::CreatePVTime)?;
602602

603603
// Register all vcpus with pvtime device

src/vmm/src/persist.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ pub enum CreateSnapshotError {
157157
}
158158

159159
/// Snapshot version
160-
pub const SNAPSHOT_VERSION: Version = Version::new(5, 0, 0);
160+
pub const SNAPSHOT_VERSION: Version = Version::new(6, 1, 0);
161161

162162
/// Creates a Microvm snapshot.
163163
pub fn create_snapshot(
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
# Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
4+
"""Tests for verifying the PVTime device behavior under contention and across snapshots."""
5+
6+
import time
7+
8+
import pytest
9+
10+
from framework.properties import global_props
11+
12+
13+
def get_steal_time_ms(vm):
14+
"""Returns total steal time of vCPUs in VM in milliseconds"""
15+
_, out, _ = vm.ssh.run("grep -w '^cpu' /proc/stat")
16+
steal_time_tck = int(out.strip().split()[8])
17+
clk_tck = int(vm.ssh.run("getconf CLK_TCK").stdout)
18+
return steal_time_tck / clk_tck * 1000
19+
20+
21+
@pytest.mark.skipif(
22+
global_props.cpu_architecture != "aarch64", reason="Only run in aarch64"
23+
)
24+
def test_guest_has_pvtime_enabled(uvm_plain):
25+
"""
26+
Check that the guest kernel has enabled PV steal time.
27+
"""
28+
vm = uvm_plain
29+
vm.spawn()
30+
vm.basic_config()
31+
vm.add_net_iface()
32+
vm.start()
33+
34+
_, stdout, _ = vm.ssh.run("dmesg | grep 'stolen time PV'")
35+
assert (
36+
"stolen time PV" in stdout
37+
), "Guest kernel did not report PV steal time enabled"
38+
39+
40+
def test_pvtime_steal_time_increases(uvm_plain):
41+
"""
42+
Test that PVTime steal time increases when both vCPUs are contended on the same pCPU.
43+
"""
44+
vm = uvm_plain
45+
vm.spawn()
46+
vm.basic_config()
47+
vm.add_net_iface()
48+
vm.start()
49+
50+
# Pin both vCPUs to the same physical CPU to induce contention
51+
vm.pin_vcpu(0, 0)
52+
vm.pin_vcpu(1, 0)
53+
54+
# Start two infinite loops to hog CPU time
55+
hog_cmd = "nohup bash -c 'while true; do :; done' >/dev/null 2>&1 &"
56+
vm.ssh.run(hog_cmd)
57+
vm.ssh.run(hog_cmd)
58+
59+
# Measure before and after steal time
60+
steal_before = get_steal_time_ms(vm)
61+
time.sleep(2)
62+
steal_after = get_steal_time_ms(vm)
63+
64+
# Require increase in steal time
65+
assert (
66+
steal_after > steal_before
67+
), f"Steal time did not increase as expected. Before: {steal_before}, After: {steal_after}"
68+
69+
70+
def test_pvtime_snapshot(uvm_plain, microvm_factory):
71+
"""
72+
Test that PVTime steal time is preserved across snapshot/restore
73+
and continues increasing post-resume.
74+
"""
75+
vm = uvm_plain
76+
vm.spawn()
77+
vm.basic_config()
78+
vm.add_net_iface()
79+
vm.start()
80+
81+
vm.pin_vcpu(0, 0)
82+
vm.pin_vcpu(1, 0)
83+
84+
hog_cmd = "nohup bash -c 'while true; do :; done' >/dev/null 2>&1 &"
85+
vm.ssh.run(hog_cmd)
86+
vm.ssh.run(hog_cmd)
87+
88+
# Snapshot pre-steal time
89+
steal_before = get_steal_time_ms(vm)
90+
91+
snapshot = vm.snapshot_full()
92+
vm.kill()
93+
94+
# Restore microVM from snapshot and resume
95+
restored_vm = microvm_factory.build()
96+
restored_vm.spawn()
97+
restored_vm.restore_from_snapshot(snapshot, resume=False)
98+
snapshot.delete()
99+
100+
restored_vm.pin_vcpu(0, 0)
101+
restored_vm.pin_vcpu(1, 0)
102+
restored_vm.resume()
103+
104+
# Steal time just after restoring
105+
steal_after_snap = get_steal_time_ms(restored_vm)
106+
107+
time.sleep(2)
108+
109+
# Steal time after running resumed VM
110+
steal_after_resume = get_steal_time_ms(restored_vm)
111+
112+
# Ensure steal time persisted and continued increasing
113+
tolerance = 1500 # 1.5 seconds tolerance for persistence check
114+
persisted = steal_after_snap - steal_before < tolerance
115+
increased = steal_after_resume > steal_after_snap
116+
117+
assert (
118+
persisted and increased
119+
), "Steal time did not persist through snapshot or failed to increase after resume"

0 commit comments

Comments
 (0)