From f725e6f6192213aa6248595551e7b52040876f7e Mon Sep 17 00:00:00 2001
From: Riccardo Mancini <mancio@amazon.com>
Date: Tue, 12 Nov 2024 15:39:50 +0000
Subject: [PATCH 01/23] chore: update PR checklist
Improve the PR checklist to make it more clear and explicitly ask
contributors to run the automated checkstyles.
Signed-off-by: Riccardo Mancini <mancio@amazon.com>
---
.github/pull_request_template.md | 22 +-
src/vmm/src/builder.rs | 633 ++++++++++++++++-----------
src/vmm/src/device_manager/legacy.rs | 2 +-
src/vmm/src/device_manager/mmio.rs | 12 +-
4 files changed, 399 insertions(+), 270 deletions(-)
diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md
index 1d6da74ba72..e05f0c2ae15 100644
--- a/.github/pull_request_template.md
+++ b/.github/pull_request_template.md
@@ -15,16 +15,20 @@ Certificate of Origin and signing off your commits, please check
## PR Checklist
+- [ ] I have read and understand [CONTRIBUTING.md][3].
+- [ ] I have run `tools/devtool checkstyle` to verify that the PR passes the
+ automated style checks.
+- [ ] I have described what is done in these changes, why they are needed, and
+ how they are solving the problem in a clear and encompassing way.
+- [ ] I have updated any relevant documentation (both in code and in the docs)
+ in the PR.
+- [ ] I have mentioned all user-facing changes in `CHANGELOG.md`.
- [ ] If a specific issue led to this PR, this PR closes the issue.
-- [ ] The description of changes is clear and encompassing.
-- [ ] Any required documentation changes (code and docs) are included in this
- PR.
-- [ ] API changes follow the [Runbook for Firecracker API changes][2].
-- [ ] User-facing changes are mentioned in `CHANGELOG.md`.
-- [ ] All added/changed functionality is tested.
-- [ ] New `TODO`s link to an issue.
-- [ ] Commits meet
- [contribution quality standards](https://github.com/firecracker-microvm/firecracker/blob/main/CONTRIBUTING.md#contribution-quality-standards).
+- [ ] When making API changes, I have followed the
+ [Runbook for Firecracker API changes][2].
+- [ ] I have tested all new and changed functionalities in unit tests and/or
+ integration tests.
+- [ ] I have linked an issue to every new `TODO`.
______________________________________________________________________
diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs
index 1f52fdad063..4a3eb48e0fc 100644
--- a/src/vmm/src/builder.rs
+++ b/src/vmm/src/builder.rs
@@ -148,24 +148,340 @@ impl std::convert::From<linux_loader::cmdline::Error> for StartMicrovmError {
}
}
-#[cfg_attr(target_arch = "aarch64", allow(unused))]
-fn create_vmm_and_vcpus(
+#[cfg(target_arch = "aarch64")]
+pub mod aarch64 {
+ use super::*;
+
+ fn attach_legacy_devices_aarch64(
+ event_manager: &mut EventManager,
+ vmm: &mut Vmm,
+ cmdline: &mut LoaderKernelCmdline,
+ ) -> Result<(), VmmError> {
+ // Serial device setup.
+ let cmdline_contains_console = cmdline
+ .as_cstring()
+ .map_err(|_| VmmError::Cmdline)?
+ .into_string()
+ .map_err(|_| VmmError::Cmdline)?
+ .contains("console=");
+
+ if cmdline_contains_console {
+ // Make stdout non-blocking.
+ set_stdout_nonblocking();
+ let serial = setup_serial_device(event_manager, std::io::stdin(), std::io::stdout())?;
+ vmm.mmio_device_manager
+ .register_mmio_serial(vmm.vm.fd(), &mut vmm.resource_allocator, serial, None)
+ .map_err(VmmError::RegisterMMIODevice)?;
+ vmm.mmio_device_manager
+ .add_mmio_serial_to_cmdline(cmdline)
+ .map_err(VmmError::RegisterMMIODevice)?;
+ }
+
+ let rtc = RTCDevice(Rtc::with_events(
+ &crate::devices::legacy::rtc_pl031::METRICS,
+ ));
+ vmm.mmio_device_manager
+ .register_mmio_rtc(&mut vmm.resource_allocator, rtc, None)
+ .map_err(VmmError::RegisterMMIODevice)
+ }
+
+ /// Configures the system for booting Linux.
+ pub fn configure_system_for_boot(
+ vmm: &mut Vmm,
+ vcpus: &mut [Vcpu],
+ vm_config: &VmConfig,
+ cpu_template: &CustomCpuTemplate,
+ entry_addr: GuestAddress,
+ initrd: &Option<InitrdConfig>,
+ boot_cmdline: LoaderKernelCmdline,
+ ) -> Result<(), StartMicrovmError> {
+ use self::StartMicrovmError::*;
+
+ // Construct the base CpuConfiguration to apply CPU template onto.
+ let cpu_config = {
+ use crate::arch::aarch64::regs::Aarch64RegisterVec;
+ use crate::arch::aarch64::vcpu::get_registers;
+
+ for vcpu in vcpus.iter_mut() {
+ vcpu.kvm_vcpu
+ .init(&cpu_template.vcpu_features)
+ .map_err(VmmError::VcpuInit)
+ .map_err(Internal)?;
+ }
+
+ let mut regs = Aarch64RegisterVec::default();
+ get_registers(&vcpus[0].kvm_vcpu.fd, &cpu_template.reg_list(), &mut regs)
+ .map_err(GuestConfigError)?;
+ CpuConfiguration { regs }
+ };
+
+ // Apply CPU template to the base CpuConfiguration.
+ let cpu_config = CpuConfiguration::apply_template(cpu_config, cpu_template)?;
+
+ let vcpu_config = VcpuConfig {
+ vcpu_count: vm_config.vcpu_count,
+ smt: vm_config.smt,
+ cpu_config,
+ };
+
+ // Configure vCPUs with normalizing and setting the generated CPU configuration.
+ for vcpu in vcpus.iter_mut() {
+ vcpu.kvm_vcpu
+ .configure(vmm.guest_memory(), entry_addr, &vcpu_config)
+ .map_err(VmmError::VcpuConfigure)
+ .map_err(Internal)?;
+ }
+
+ let vcpu_mpidr = vcpus
+ .iter_mut()
+ .map(|cpu| cpu.kvm_vcpu.get_mpidr())
+ .collect();
+ let cmdline = boot_cmdline.as_cstring()?;
+ crate::arch::aarch64::configure_system(
+ &vmm.guest_memory,
+ cmdline,
+ vcpu_mpidr,
+ vmm.mmio_device_manager.get_device_info(),
+ vmm.vm.get_irqchip(),
+ &vmm.acpi_device_manager.vmgenid,
+ initrd,
+ )
+ .map_err(ConfigureSystem)?;
+
+ Ok(())
+ }
+
+ /// Sets up the irqchip for a aarch64 microVM.
+ pub fn setup_interrupt_controller(vm: &mut Vm, vcpu_count: u8) -> Result<(), VmmError> {
+ vm.setup_irqchip(vcpu_count).map_err(VmmError::Vm)
+ }
+
+ /// On aarch64, the vCPUs need to be created (i.e call KVM_CREATE_VCPU) before setting up the
+ /// IRQ chip because the `KVM_CREATE_VCPU` ioctl will return error if the IRQCHIP
+ /// was already initialized.
+ /// Search for `kvm_arch_vcpu_create` in arch/arm/kvm/arm.c.
+ pub fn create_vcpus(
+ vm: &mut Vm,
+ vcpu_count: u8,
+ exit_evt: &EventFd,
+ ) -> Result<Vec<Vcpu>, VmmError> {
+ let mut vcpus = Vec::with_capacity(vcpu_count as usize);
+ for cpu_idx in 0..vcpu_count {
+ let exit_evt = exit_evt.try_clone().map_err(VmmError::EventFd)?;
+ let vcpu = Vcpu::new(cpu_idx, vm, exit_evt).map_err(VmmError::VcpuCreate)?;
+ vcpus.push(vcpu);
+ }
+
+ setup_interrupt_controller(vm, vcpu_count)?;
+
+ Ok(vcpus)
+ }
+
+ pub fn load_kernel(
+ boot_config: &BootConfig,
+ guest_memory: &GuestMemoryMmap,
+ ) -> Result<GuestAddress, StartMicrovmError> {
+ let mut kernel_file = boot_config
+ .kernel_file
+ .try_clone()
+ .map_err(|err| StartMicrovmError::Internal(VmmError::KernelFile(err)))?;
+
+ let entry_addr = Loader::load::<std::fs::File, GuestMemoryMmap>(
+ guest_memory,
+ Some(GuestAddress(crate::arch::get_kernel_start())),
+ &mut kernel_file,
+ None,
+ )
+ .map_err(StartMicrovmError::KernelLoader)?;
+
+ Ok(entry_addr.kernel_load)
+ }
+
+ pub fn create_vmm_and_vcpus(
+ instance_info: &InstanceInfo,
+ guest_memory: GuestMemoryMmap,
+ uffd: Option<Uffd>,
+ vm_config: &VmConfig,
+ kvm_capabilities: Vec<KvmCapability>,
+ ) -> Result<(Vmm, Vec<Vcpu>), StartMicrovmError> {
+ let mut vmm = build_vmm(
+ instance_info,
+ guest_memory,
+ uffd,
+ vm_config,
+ kvm_capabilities,
+ )?;
+
+ let vcpus =
+ create_vcpus(&mut vm, vm_config.vcpu_count, &vmm.vcpus_exit_evt).map_err(Internal)?;
+
+ Ok((vmm, vcpus))
+ }
+}
+
+#[cfg(target_arch = "x86_64")]
+pub mod x86_64 {
+ use super::*;
+
+ /// Configures the system for booting Linux.
+ pub fn configure_system_for_boot(
+ vmm: &mut Vmm,
+ vcpus: &mut [Vcpu],
+ vm_config: &VmConfig,
+ cpu_template: &CustomCpuTemplate,
+ entry_addr: GuestAddress,
+ initrd: &Option<InitrdConfig>,
+ boot_cmdline: LoaderKernelCmdline,
+ ) -> Result<(), StartMicrovmError> {
+ use self::StartMicrovmError::*;
+
+ // Construct the base CpuConfiguration to apply CPU template onto.
+ let cpu_config = {
+ use crate::cpu_config::x86_64::cpuid;
+ let cpuid = cpuid::Cpuid::try_from(vmm.vm.supported_cpuid().clone())
+ .map_err(GuestConfigError::CpuidFromKvmCpuid)?;
+ let msrs = vcpus[0]
+ .kvm_vcpu
+ .get_msrs(cpu_template.msr_index_iter())
+ .map_err(GuestConfigError::VcpuIoctl)?;
+ CpuConfiguration { cpuid, msrs }
+ };
+
+ // Apply CPU template to the base CpuConfiguration.
+ let cpu_config = CpuConfiguration::apply_template(cpu_config, cpu_template)?;
+
+ let vcpu_config = VcpuConfig {
+ vcpu_count: vm_config.vcpu_count,
+ smt: vm_config.smt,
+ cpu_config,
+ };
+
+ // Configure vCPUs with normalizing and setting the generated CPU configuration.
+ for vcpu in vcpus.iter_mut() {
+ vcpu.kvm_vcpu
+ .configure(vmm.guest_memory(), entry_addr, &vcpu_config)
+ .map_err(VmmError::VcpuConfigure)
+ .map_err(Internal)?;
+ }
+
+ // Write the kernel command line to guest memory. This is x86_64 specific, since on
+ // aarch64 the command line will be specified through the FDT.
+ let cmdline_size = boot_cmdline
+ .as_cstring()
+ .map(|cmdline_cstring| cmdline_cstring.as_bytes_with_nul().len())?;
+
+ linux_loader::loader::load_cmdline::<crate::vstate::memory::GuestMemoryMmap>(
+ vmm.guest_memory(),
+ GuestAddress(crate::arch::x86_64::layout::CMDLINE_START),
+ &boot_cmdline,
+ )
+ .map_err(LoadCommandline)?;
+ crate::arch::x86_64::configure_system(
+ &vmm.guest_memory,
+ &mut vmm.resource_allocator,
+ crate::vstate::memory::GuestAddress(crate::arch::x86_64::layout::CMDLINE_START),
+ cmdline_size,
+ initrd,
+ vcpu_config.vcpu_count,
+ )
+ .map_err(ConfigureSystem)?;
+
+ // Create ACPI tables and write them in guest memory
+ // For the time being we only support ACPI in x86_64
+ acpi::create_acpi_tables(
+ &vmm.guest_memory,
+ &mut vmm.resource_allocator,
+ &vmm.mmio_device_manager,
+ &vmm.acpi_device_manager,
+ vcpus,
+ )?;
+
+ Ok(())
+ }
+
+ /// Sets up the irqchip for a x86_64 microVM.
+ pub fn setup_interrupt_controller(vm: &mut Vm) -> Result<(), VmmError> {
+ vm.setup_irqchip().map_err(VmmError::Vm)
+ }
+
+ /// For x86_64 we need to create the interrupt controller before calling `KVM_CREATE_VCPUS`
+ /// while on aarch64 we need to do it the other way around.
+ pub fn create_vcpus(
+ vm: &mut Vm,
+ vcpu_count: u8,
+ exit_evt: &EventFd,
+ ) -> Result<Vec<Vcpu>, VmmError> {
+ setup_interrupt_controller(vm)?;
+
+ let mut vcpus = Vec::with_capacity(vcpu_count as usize);
+ for cpu_idx in 0..vcpu_count {
+ let exit_evt = exit_evt.try_clone().map_err(VmmError::EventFd)?;
+ let vcpu = Vcpu::new(cpu_idx, vm, exit_evt).map_err(VmmError::VcpuCreate)?;
+ vcpus.push(vcpu);
+ }
+ // Make stdout non blocking.
+ set_stdout_nonblocking();
+
+ Ok(vcpus)
+ }
+
+ pub fn load_kernel(
+ boot_config: &BootConfig,
+ guest_memory: &GuestMemoryMmap,
+ ) -> Result<GuestAddress, StartMicrovmError> {
+ let mut kernel_file = boot_config
+ .kernel_file
+ .try_clone()
+ .map_err(|err| StartMicrovmError::Internal(VmmError::KernelFile(err)))?;
+
+ let entry_addr = Loader::load::<std::fs::File, GuestMemoryMmap>(
+ guest_memory,
+ None,
+ &mut kernel_file,
+ Some(GuestAddress(crate::arch::get_kernel_start())),
+ )
+ .map_err(StartMicrovmError::KernelLoader)?;
+
+ Ok(entry_addr.kernel_load)
+ }
+
+ pub fn create_vmm_and_vcpus(
+ instance_info: &InstanceInfo,
+ guest_memory: GuestMemoryMmap,
+ uffd: Option<Uffd>,
+ vm_config: &VmConfig,
+ kvm_capabilities: Vec<KvmCapability>,
+ ) -> Result<(Vmm, Vec<Vcpu>), StartMicrovmError> {
+ let mut vmm = build_vmm(
+ instance_info,
+ guest_memory,
+ uffd,
+ vm_config,
+ kvm_capabilities,
+ )?;
+
+ let vcpus = create_vcpus(&mut vmm.vm, vm_config.vcpu_count, &vmm.vcpus_exit_evt)
+ .map_err(StartMicrovmError::Internal)?;
+
+ Ok((vmm, vcpus))
+ }
+}
+
+fn build_vmm(
instance_info: &InstanceInfo,
- event_manager: &mut EventManager,
guest_memory: GuestMemoryMmap,
uffd: Option<Uffd>,
- track_dirty_pages: bool,
- vcpu_count: u8,
+ vm_config: &VmConfig,
kvm_capabilities: Vec<KvmCapability>,
-) -> Result<(Vmm, Vec<Vcpu>), StartMicrovmError> {
+) -> Result<Vmm, StartMicrovmError> {
use self::StartMicrovmError::*;
// Set up Kvm Vm and register memory regions.
// Build custom CPU config if a custom template is provided.
- let mut vm = Vm::new(kvm_capabilities)
+ let vm = Vm::new(kvm_capabilities)
.map_err(VmmError::Vm)
.map_err(StartMicrovmError::Internal)?;
- vm.memory_init(&guest_memory, track_dirty_pages)
+ vm.memory_init(&guest_memory, vm_config.track_dirty_pages)
.map_err(VmmError::Vm)
.map_err(StartMicrovmError::Internal)?;
@@ -184,16 +500,10 @@ fn create_vmm_and_vcpus(
// For x86_64 we need to create the interrupt controller before calling `KVM_CREATE_VCPUS`
// while on aarch64 we need to do it the other way around.
#[cfg(target_arch = "x86_64")]
- let (vcpus, pio_device_manager) = {
- setup_interrupt_controller(&mut vm)?;
- let vcpus = create_vcpus(&vm, vcpu_count, &vcpus_exit_evt).map_err(Internal)?;
-
- // Make stdout non blocking.
- set_stdout_nonblocking();
-
+ let pio_device_manager = {
// Serial device setup.
let serial_device =
- setup_serial_device(event_manager, std::io::stdin(), io::stdout()).map_err(Internal)?;
+ setup_serial_device(std::io::stdin(), io::stdout()).map_err(Internal)?;
// x86_64 uses the i8042 reset event as the Vmm exit event.
let reset_evt = vcpus_exit_evt
@@ -209,21 +519,10 @@ fn create_vmm_and_vcpus(
pio_dev_mgr
};
- (vcpus, pio_device_manager)
+ pio_device_manager
};
- // On aarch64, the vCPUs need to be created (i.e call KVM_CREATE_VCPU) before setting up the
- // IRQ chip because the `KVM_CREATE_VCPU` ioctl will return error if the IRQCHIP
- // was already initialized.
- // Search for `kvm_arch_vcpu_create` in arch/arm/kvm/arm.c.
- #[cfg(target_arch = "aarch64")]
- let vcpus = {
- let vcpus = create_vcpus(&vm, vcpu_count, &vcpus_exit_evt).map_err(Internal)?;
- setup_interrupt_controller(&mut vm, vcpu_count)?;
- vcpus
- };
-
- let vmm = Vmm {
+ Ok(Vmm {
events_observer: Some(std::io::stdin()),
instance_info: instance_info.clone(),
shutdown_exit_code: None,
@@ -237,9 +536,7 @@ fn create_vmm_and_vcpus(
#[cfg(target_arch = "x86_64")]
pio_device_manager,
acpi_device_manager,
- };
-
- Ok((vmm, vcpus))
+ })
}
/// Builds and starts a microVM based on the current Firecracker VmResources configuration.
@@ -266,7 +563,11 @@ pub fn build_microvm_for_boot(
.allocate_guest_memory()
.map_err(StartMicrovmError::GuestMemory)?;
- let entry_addr = load_kernel(boot_config, &guest_memory)?;
+ #[cfg(target_arch = "x86_64")]
+ let entry_addr = x86_64::load_kernel(boot_config, &guest_memory)?;
+ #[cfg(target_arch = "aarch64")]
+ let entry_addr = aarch64::load_kernel(boot_config, &guest_memory)?;
+
let initrd = load_initrd_from_config(boot_config, &guest_memory)?;
// Clone the command-line so that a failed boot doesn't pollute the original.
#[allow(unused_mut)]
@@ -274,13 +575,23 @@ pub fn build_microvm_for_boot(
let cpu_template = vm_resources.vm_config.cpu_template.get_cpu_template()?;
- let (mut vmm, mut vcpus) = create_vmm_and_vcpus(
+ #[cfg(target_arch = "x86_64")]
+ let (mut vmm, mut vcpus) = x86_64::create_vmm_and_vcpus(
instance_info,
- event_manager,
guest_memory,
None,
- vm_resources.vm_config.track_dirty_pages,
- vm_resources.vm_config.vcpu_count,
+ &vm_resources.vm_config,
+ cpu_template.kvm_capabilities.clone(),
+ )?;
+ #[cfg(target_arch = "x86_64")]
+ event_manager.add_subscriber(vmm.pio_device_manager.stdio_serial.clone());
+
+ #[cfg(target_arch = "aarch64")]
+ let (mut vmm, mut vcpus) = aarch64::create_vmm_and_vcpus(
+ instance_info,
+ guest_memory,
+ None,
+ &vm_resources.vm_config,
cpu_template.kvm_capabilities.clone(),
)?;
@@ -329,11 +640,23 @@ pub fn build_microvm_for_boot(
}
#[cfg(target_arch = "aarch64")]
- attach_legacy_devices_aarch64(event_manager, &mut vmm, &mut boot_cmdline).map_err(Internal)?;
+ aarch::attach_legacy_devices_aarch64(event_manager, &mut vmm, &mut boot_cmdline)
+ .map_err(Internal)?;
attach_vmgenid_device(&mut vmm)?;
- configure_system_for_boot(
+ #[cfg(target_arch = "x86_64")]
+ x86_64::configure_system_for_boot(
+ &mut vmm,
+ vcpus.as_mut(),
+ &vm_resources.vm_config,
+ &cpu_template,
+ entry_addr,
+ &initrd,
+ boot_cmdline,
+ )?;
+ #[cfg(target_arch = "aarch64")]
+ aarch64::configure_system_for_boot(
&mut vmm,
vcpus.as_mut(),
&vm_resources.vm_config,
@@ -464,13 +787,22 @@ pub fn build_microvm_from_snapshot(
) -> Result<Arc<Mutex<Vmm>>, BuildMicrovmFromSnapshotError> {
// Build Vmm.
debug!("event_start: build microvm from snapshot");
- let (mut vmm, mut vcpus) = create_vmm_and_vcpus(
+ #[cfg(target_arch = "x86_64")]
+ let (mut vmm, mut vcpus) = x86_64::create_vmm_and_vcpus(
instance_info,
- event_manager,
guest_memory.clone(),
uffd,
- vm_resources.vm_config.track_dirty_pages,
- vm_resources.vm_config.vcpu_count,
+ &vm_resources.vm_config,
+ microvm_state.vm_state.kvm_cap_modifiers.clone(),
+ )?;
+ #[cfg(target_arch = "x86_64")]
+ event_manager.add_subscriber(vmm.pio_device_manager.stdio_serial.clone());
+ #[cfg(target_arch = "aarch64")]
+ let (mut vmm, mut vcpus) = aarch64::create_vmm_and_vcpus(
+ instance_info,
+ guest_memory.clone(),
+ uffd,
+ &vm_resources.vm_config,
microvm_state.vm_state.kvm_cap_modifiers.clone(),
)?;
@@ -567,36 +899,6 @@ pub fn build_microvm_from_snapshot(
Ok(vmm)
}
-fn load_kernel(
- boot_config: &BootConfig,
- guest_memory: &GuestMemoryMmap,
-) -> Result<GuestAddress, StartMicrovmError> {
- let mut kernel_file = boot_config
- .kernel_file
- .try_clone()
- .map_err(|err| StartMicrovmError::Internal(VmmError::KernelFile(err)))?;
-
- #[cfg(target_arch = "x86_64")]
- let entry_addr = Loader::load::<std::fs::File, GuestMemoryMmap>(
- guest_memory,
- None,
- &mut kernel_file,
- Some(GuestAddress(crate::arch::get_kernel_start())),
- )
- .map_err(StartMicrovmError::KernelLoader)?;
-
- #[cfg(target_arch = "aarch64")]
- let entry_addr = Loader::load::<std::fs::File, GuestMemoryMmap>(
- guest_memory,
- Some(GuestAddress(crate::arch::get_kernel_start())),
- &mut kernel_file,
- None,
- )
- .map_err(StartMicrovmError::KernelLoader)?;
-
- Ok(entry_addr.kernel_load)
-}
-
fn load_initrd_from_config(
boot_cfg: &BootConfig,
vm_memory: &GuestMemoryMmap,
@@ -660,25 +962,8 @@ where
})
}
-/// Sets up the irqchip for a x86_64 microVM.
-#[cfg(target_arch = "x86_64")]
-pub fn setup_interrupt_controller(vm: &mut Vm) -> Result<(), StartMicrovmError> {
- vm.setup_irqchip()
- .map_err(VmmError::Vm)
- .map_err(StartMicrovmError::Internal)
-}
-
-/// Sets up the irqchip for a aarch64 microVM.
-#[cfg(target_arch = "aarch64")]
-pub fn setup_interrupt_controller(vm: &mut Vm, vcpu_count: u8) -> Result<(), StartMicrovmError> {
- vm.setup_irqchip(vcpu_count)
- .map_err(VmmError::Vm)
- .map_err(StartMicrovmError::Internal)
-}
-
/// Sets up the serial device.
pub fn setup_serial_device(
- event_manager: &mut EventManager,
input: std::io::Stdin,
out: std::io::Stdout,
) -> Result<Arc<Mutex<BusDevice>>, VmmError> {
@@ -695,168 +980,8 @@ pub fn setup_serial_device(
),
input: Some(input),
})));
- event_manager.add_subscriber(serial.clone());
- Ok(serial)
-}
-
-#[cfg(target_arch = "aarch64")]
-fn attach_legacy_devices_aarch64(
- event_manager: &mut EventManager,
- vmm: &mut Vmm,
- cmdline: &mut LoaderKernelCmdline,
-) -> Result<(), VmmError> {
- // Serial device setup.
- let cmdline_contains_console = cmdline
- .as_cstring()
- .map_err(|_| VmmError::Cmdline)?
- .into_string()
- .map_err(|_| VmmError::Cmdline)?
- .contains("console=");
-
- if cmdline_contains_console {
- // Make stdout non-blocking.
- set_stdout_nonblocking();
- let serial = setup_serial_device(event_manager, std::io::stdin(), std::io::stdout())?;
- vmm.mmio_device_manager
- .register_mmio_serial(vmm.vm.fd(), &mut vmm.resource_allocator, serial, None)
- .map_err(VmmError::RegisterMMIODevice)?;
- vmm.mmio_device_manager
- .add_mmio_serial_to_cmdline(cmdline)
- .map_err(VmmError::RegisterMMIODevice)?;
- }
- let rtc = RTCDevice(Rtc::with_events(
- &crate::devices::legacy::rtc_pl031::METRICS,
- ));
- vmm.mmio_device_manager
- .register_mmio_rtc(&mut vmm.resource_allocator, rtc, None)
- .map_err(VmmError::RegisterMMIODevice)
-}
-
-fn create_vcpus(vm: &Vm, vcpu_count: u8, exit_evt: &EventFd) -> Result<Vec<Vcpu>, VmmError> {
- let mut vcpus = Vec::with_capacity(vcpu_count as usize);
- for cpu_idx in 0..vcpu_count {
- let exit_evt = exit_evt.try_clone().map_err(VmmError::EventFd)?;
- let vcpu = Vcpu::new(cpu_idx, vm, exit_evt).map_err(VmmError::VcpuCreate)?;
- vcpus.push(vcpu);
- }
- Ok(vcpus)
-}
-
-/// Configures the system for booting Linux.
-#[cfg_attr(target_arch = "aarch64", allow(unused))]
-pub fn configure_system_for_boot(
- vmm: &mut Vmm,
- vcpus: &mut [Vcpu],
- vm_config: &VmConfig,
- cpu_template: &CustomCpuTemplate,
- entry_addr: GuestAddress,
- initrd: &Option<InitrdConfig>,
- boot_cmdline: LoaderKernelCmdline,
-) -> Result<(), StartMicrovmError> {
- use self::StartMicrovmError::*;
-
- // Construct the base CpuConfiguration to apply CPU template onto.
- #[cfg(target_arch = "x86_64")]
- let cpu_config = {
- use crate::cpu_config::x86_64::cpuid;
- let cpuid = cpuid::Cpuid::try_from(vmm.vm.supported_cpuid().clone())
- .map_err(GuestConfigError::CpuidFromKvmCpuid)?;
- let msrs = vcpus[0]
- .kvm_vcpu
- .get_msrs(cpu_template.msr_index_iter())
- .map_err(GuestConfigError::VcpuIoctl)?;
- CpuConfiguration { cpuid, msrs }
- };
-
- #[cfg(target_arch = "aarch64")]
- let cpu_config = {
- use crate::arch::aarch64::regs::Aarch64RegisterVec;
- use crate::arch::aarch64::vcpu::get_registers;
-
- for vcpu in vcpus.iter_mut() {
- vcpu.kvm_vcpu
- .init(&cpu_template.vcpu_features)
- .map_err(VmmError::VcpuInit)
- .map_err(Internal)?;
- }
-
- let mut regs = Aarch64RegisterVec::default();
- get_registers(&vcpus[0].kvm_vcpu.fd, &cpu_template.reg_list(), &mut regs)
- .map_err(GuestConfigError)?;
- CpuConfiguration { regs }
- };
-
- // Apply CPU template to the base CpuConfiguration.
- let cpu_config = CpuConfiguration::apply_template(cpu_config, cpu_template)?;
-
- let vcpu_config = VcpuConfig {
- vcpu_count: vm_config.vcpu_count,
- smt: vm_config.smt,
- cpu_config,
- };
-
- // Configure vCPUs with normalizing and setting the generated CPU configuration.
- for vcpu in vcpus.iter_mut() {
- vcpu.kvm_vcpu
- .configure(vmm.guest_memory(), entry_addr, &vcpu_config)
- .map_err(VmmError::VcpuConfigure)
- .map_err(Internal)?;
- }
-
- #[cfg(target_arch = "x86_64")]
- {
- // Write the kernel command line to guest memory. This is x86_64 specific, since on
- // aarch64 the command line will be specified through the FDT.
- let cmdline_size = boot_cmdline
- .as_cstring()
- .map(|cmdline_cstring| cmdline_cstring.as_bytes_with_nul().len())?;
-
- linux_loader::loader::load_cmdline::<crate::vstate::memory::GuestMemoryMmap>(
- vmm.guest_memory(),
- GuestAddress(crate::arch::x86_64::layout::CMDLINE_START),
- &boot_cmdline,
- )
- .map_err(LoadCommandline)?;
- crate::arch::x86_64::configure_system(
- &vmm.guest_memory,
- &mut vmm.resource_allocator,
- crate::vstate::memory::GuestAddress(crate::arch::x86_64::layout::CMDLINE_START),
- cmdline_size,
- initrd,
- vcpu_config.vcpu_count,
- )
- .map_err(ConfigureSystem)?;
-
- // Create ACPI tables and write them in guest memory
- // For the time being we only support ACPI in x86_64
- acpi::create_acpi_tables(
- &vmm.guest_memory,
- &mut vmm.resource_allocator,
- &vmm.mmio_device_manager,
- &vmm.acpi_device_manager,
- vcpus,
- )?;
- }
- #[cfg(target_arch = "aarch64")]
- {
- let vcpu_mpidr = vcpus
- .iter_mut()
- .map(|cpu| cpu.kvm_vcpu.get_mpidr())
- .collect();
- let cmdline = boot_cmdline.as_cstring()?;
- crate::arch::aarch64::configure_system(
- &vmm.guest_memory,
- cmdline,
- vcpu_mpidr,
- vmm.mmio_device_manager.get_device_info(),
- vmm.vm.get_irqchip(),
- &vmm.acpi_device_manager.vmgenid,
- initrd,
- )
- .map_err(ConfigureSystem)?;
- }
- Ok(())
+ Ok(serial)
}
/// Attaches a VirtioDevice device to the device manager and event manager.
@@ -1127,7 +1252,7 @@ pub mod tests {
.unwrap();
#[cfg(target_arch = "x86_64")]
- setup_interrupt_controller(&mut vm).unwrap();
+ x86_64::setup_interrupt_controller(&mut vm).unwrap();
#[cfg(target_arch = "aarch64")]
{
@@ -1363,9 +1488,9 @@ pub mod tests {
let evfd = EventFd::new(libc::EFD_NONBLOCK).unwrap();
#[cfg(target_arch = "x86_64")]
- setup_interrupt_controller(&mut vm).unwrap();
+ x86_64::setup_interrupt_controller(&mut vm).unwrap();
- let vcpu_vec = create_vcpus(&vm, vcpu_count, &evfd).unwrap();
+ let vcpu_vec = x86_64::create_vcpus(&mut vm, vcpu_count, &evfd).unwrap();
assert_eq!(vcpu_vec.len(), vcpu_count as usize);
}
diff --git a/src/vmm/src/device_manager/legacy.rs b/src/vmm/src/device_manager/legacy.rs
index 45842d933b2..664f74bc06f 100644
--- a/src/vmm/src/device_manager/legacy.rs
+++ b/src/vmm/src/device_manager/legacy.rs
@@ -252,7 +252,7 @@ mod tests {
let guest_mem = single_region_mem(0x1000);
let mut vm = Vm::new(vec![]).unwrap();
vm.memory_init(&guest_mem, false).unwrap();
- crate::builder::setup_interrupt_controller(&mut vm).unwrap();
+ crate::builder::x86_64::setup_interrupt_controller(&mut vm).unwrap();
let mut ldm = PortIODeviceManager::new(
Arc::new(Mutex::new(BusDevice::Serial(SerialDevice {
serial: Serial::with_events(
diff --git a/src/vmm/src/device_manager/mmio.rs b/src/vmm/src/device_manager/mmio.rs
index 00c155abcfd..774967e8933 100644
--- a/src/vmm/src/device_manager/mmio.rs
+++ b/src/vmm/src/device_manager/mmio.rs
@@ -669,9 +669,9 @@ mod tests {
let mut cmdline = kernel_cmdline::Cmdline::new(4096).unwrap();
let dummy = Arc::new(Mutex::new(DummyDevice::new()));
#[cfg(target_arch = "x86_64")]
- builder::setup_interrupt_controller(&mut vm).unwrap();
+ builder::x86_64::setup_interrupt_controller(&mut vm).unwrap();
#[cfg(target_arch = "aarch64")]
- builder::setup_interrupt_controller(&mut vm, 1).unwrap();
+ builder::aarch64::setup_interrupt_controller(&mut vm, 1).unwrap();
device_manager
.register_virtio_test_device(
@@ -697,9 +697,9 @@ mod tests {
let mut cmdline = kernel_cmdline::Cmdline::new(4096).unwrap();
#[cfg(target_arch = "x86_64")]
- builder::setup_interrupt_controller(&mut vm).unwrap();
+ builder::x86_64::setup_interrupt_controller(&mut vm).unwrap();
#[cfg(target_arch = "aarch64")]
- builder::setup_interrupt_controller(&mut vm, 1).unwrap();
+ builder::aarch64::setup_interrupt_controller(&mut vm, 1).unwrap();
for _i in crate::arch::IRQ_BASE..=crate::arch::IRQ_MAX {
device_manager
@@ -750,9 +750,9 @@ mod tests {
let mem_clone = guest_mem.clone();
#[cfg(target_arch = "x86_64")]
- builder::setup_interrupt_controller(&mut vm).unwrap();
+ builder::x86_64::setup_interrupt_controller(&mut vm).unwrap();
#[cfg(target_arch = "aarch64")]
- builder::setup_interrupt_controller(&mut vm, 1).unwrap();
+ builder::aarch64::setup_interrupt_controller(&mut vm, 1).unwrap();
let mut device_manager = MMIODeviceManager::new();
let mut resource_allocator = ResourceAllocator::new().unwrap();
From 32c79ed369af9547ffa53b19533b05052cd0052d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= <pablob@amazon.com>
Date: Wed, 13 Nov 2024 10:54:57 +0100
Subject: [PATCH 02/23] test: test ARM CPU templates in Linux host 5.10
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Add the SVE CPU template as a valid template in 5.10 since it works.
Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
---
tests/framework/utils_cpu_templates.py | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/tests/framework/utils_cpu_templates.py b/tests/framework/utils_cpu_templates.py
index 5badd7c640a..96bf197efda 100644
--- a/tests/framework/utils_cpu_templates.py
+++ b/tests/framework/utils_cpu_templates.py
@@ -42,18 +42,12 @@ def get_supported_cpu_templates():
SUPPORTED_CPU_TEMPLATES = get_supported_cpu_templates()
-# Custom CPU templates for Aarch64 for testing
-AARCH64_CUSTOM_CPU_TEMPLATES_G2 = ["v1n1"]
-AARCH64_CUSTOM_CPU_TEMPLATES_G3 = [
- "aarch64_with_sve_and_pac",
- "v1n1",
-]
-
def get_supported_custom_cpu_templates():
"""
Return the list of custom CPU templates supported by the platform.
"""
+ # pylint:disable=too-many-return-statements
host_linux = global_props.host_linux_version_tpl
match get_cpu_vendor(), global_props.cpu_codename:
@@ -65,9 +59,11 @@ def get_supported_custom_cpu_templates():
case CpuVendor.AMD, _:
return AMD_TEMPLATES
case CpuVendor.ARM, CpuModel.ARM_NEOVERSE_N1 if host_linux >= (6, 1):
- return AARCH64_CUSTOM_CPU_TEMPLATES_G2
+ return ["v1n1"]
case CpuVendor.ARM, CpuModel.ARM_NEOVERSE_V1 if host_linux >= (6, 1):
- return AARCH64_CUSTOM_CPU_TEMPLATES_G3
+ return ["v1n1", "aarch64_with_sve_and_pac"]
+ case CpuVendor.ARM, CpuModel.ARM_NEOVERSE_V1:
+ return ["aarch64_with_sve_and_pac"]
case _:
return []
From 93a5dab93daba4ef3378229af6d1e7c7cc570f75 Mon Sep 17 00:00:00 2001
From: Jack Thomson <jackabt@amazon.com>
Date: Wed, 13 Nov 2024 11:53:26 +0000
Subject: [PATCH 03/23] chore: Update to v1.10.1 patch
Update release policy to v1.10.1 patch
Signed-off-by: Jack Thomson <jackabt@amazon.com>
---
docs/RELEASE_POLICY.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs/RELEASE_POLICY.md b/docs/RELEASE_POLICY.md
index fb95c55e402..fe2f881d6b6 100644
--- a/docs/RELEASE_POLICY.md
+++ b/docs/RELEASE_POLICY.md
@@ -90,7 +90,7 @@ v3.1 will be patched since were the last two Firecracker releases and less than
| Release | Release Date | Latest Patch | Min. end of support | Official end of Support |
| ------: | -----------: | -----------: | ------------------: | :------------------------------ |
-| v1.10 | 2024-11-07 | v1.10.0 | 2025-05-07 | Supported |
+| v1.10 | 2024-11-07 | v1.10.1 | 2025-05-07 | Supported |
| v1.9 | 2024-09-02 | v1.9.1 | 2025-03-02 | Supported |
| v1.8 | 2024-07-10 | v1.8.0 | 2025-01-10 | Supported |
| v1.7 | 2024-03-18 | v1.7.0 | 2024-09-18 | 2024-09-18 (end of 6mo support) |
From 0d0d4d505b6f8ee99bd51f28d36d69adc12e93f8 Mon Sep 17 00:00:00 2001
From: Takahiro Itazuri <itazur@amazon.com>
Date: Wed, 13 Nov 2024 11:07:12 +0000
Subject: [PATCH 04/23] snapshot: Remove max_connections and max_pending_resets
fields
`TcpIPv4Handler` for MMDS network stack preallocates several buffers
whose sizes are saved into a snapshot as `max_connections` and
`max_pending_resets` in `MmdsNetworkStackState`. But they are always the
same constant hardcoded values (`DEFAULT_MAX_CONNECTIONS` and
`DEFAULT_MAX_PENDING_RESETS`) as of today, which means there is no need
to save them into a snapshot. Even if we change the hardcoded sizes
across Firecracker versions, that should not be a problem. This is
because the snapshot feature does not support migration of network
connections and those buffers are initialized with empty on snapshot
restoration. When migrating from a Firecracker version with larger
buffers to another version with smaller ones, guest workloads that
worked previously might start to fail due to the less buffer spaces.
However, the issue is not a problem of the snapshot feature and it
should also occur even on a purely booted microVM (not restored from a
snapshot). Thus, it is fine to remove those fields from a snapshot.
Since this is a breaking change of the snapshot format, bumps the major
version.
Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
---
CHANGELOG.md | 4 ++++
src/vmm/src/mmds/ns.rs | 15 +++------------
src/vmm/src/mmds/persist.rs | 15 ---------------
src/vmm/src/persist.rs | 2 +-
4 files changed, 8 insertions(+), 28 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5ca0688ce06..55c46a05276 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,10 @@ and this project adheres to
### Changed
+- [#4913](https://github.com/firecracker-microvm/firecracker/pull/4913): Removed
+ unnecessary fields (`max_connections` and `max_pending_resets`) from the
+ snapshot format, bumping the snapshot version to 5.0.0.
+
### Deprecated
### Removed
diff --git a/src/vmm/src/mmds/ns.rs b/src/vmm/src/mmds/ns.rs
index 09d73b21e99..d431d71cbcb 100644
--- a/src/vmm/src/mmds/ns.rs
+++ b/src/vmm/src/mmds/ns.rs
@@ -81,8 +81,6 @@ impl MmdsNetworkStack {
mac_addr: MacAddr,
ipv4_addr: Ipv4Addr,
tcp_port: u16,
- max_connections: NonZeroUsize,
- max_pending_resets: NonZeroUsize,
mmds: Arc<Mutex<Mmds>>,
) -> Self {
MmdsNetworkStack {
@@ -93,8 +91,8 @@ impl MmdsNetworkStack {
tcp_handler: TcpIPv4Handler::new(
ipv4_addr,
tcp_port,
- max_connections,
- max_pending_resets,
+ NonZeroUsize::new(DEFAULT_MAX_CONNECTIONS).unwrap(),
+ NonZeroUsize::new(DEFAULT_MAX_PENDING_RESETS).unwrap(),
),
mmds,
}
@@ -105,14 +103,7 @@ impl MmdsNetworkStack {
let ipv4_addr = mmds_ipv4_addr.unwrap_or_else(|| Ipv4Addr::from(DEFAULT_IPV4_ADDR));
// The unwrap()s are safe because the given literals are greater than 0.
- Self::new(
- mac_addr,
- ipv4_addr,
- DEFAULT_TCP_PORT,
- NonZeroUsize::new(DEFAULT_MAX_CONNECTIONS).unwrap(),
- NonZeroUsize::new(DEFAULT_MAX_PENDING_RESETS).unwrap(),
- mmds,
- )
+ Self::new(mac_addr, ipv4_addr, DEFAULT_TCP_PORT, mmds)
}
pub fn set_ipv4_addr(&mut self, ipv4_addr: Ipv4Addr) {
diff --git a/src/vmm/src/mmds/persist.rs b/src/vmm/src/mmds/persist.rs
index dc0113f8a5c..82feff79bc8 100644
--- a/src/vmm/src/mmds/persist.rs
+++ b/src/vmm/src/mmds/persist.rs
@@ -4,7 +4,6 @@
//! Defines the structures needed for saving/restoring MmdsNetworkStack.
use std::net::Ipv4Addr;
-use std::num::NonZeroUsize;
use std::sync::{Arc, Mutex};
use serde::{Deserialize, Serialize};
@@ -20,8 +19,6 @@ pub struct MmdsNetworkStackState {
mac_addr: [u8; MAC_ADDR_LEN as usize],
ipv4_addr: u32,
tcp_port: u16,
- max_connections: NonZeroUsize,
- max_pending_resets: NonZeroUsize,
}
impl Persist<'_> for MmdsNetworkStack {
@@ -37,8 +34,6 @@ impl Persist<'_> for MmdsNetworkStack {
mac_addr,
ipv4_addr: self.ipv4_addr.into(),
tcp_port: self.tcp_handler.local_port(),
- max_connections: self.tcp_handler.max_connections(),
- max_pending_resets: self.tcp_handler.max_pending_resets(),
}
}
@@ -50,8 +45,6 @@ impl Persist<'_> for MmdsNetworkStack {
MacAddr::from_bytes_unchecked(&state.mac_addr),
Ipv4Addr::from(state.ipv4_addr),
state.tcp_port,
- state.max_connections,
- state.max_pending_resets,
mmds,
))
}
@@ -83,13 +76,5 @@ mod tests {
restored_ns.tcp_handler.local_port(),
ns.tcp_handler.local_port()
);
- assert_eq!(
- restored_ns.tcp_handler.max_connections(),
- ns.tcp_handler.max_connections()
- );
- assert_eq!(
- restored_ns.tcp_handler.max_pending_resets(),
- ns.tcp_handler.max_pending_resets()
- );
}
}
diff --git a/src/vmm/src/persist.rs b/src/vmm/src/persist.rs
index 16d7ed72537..5b01ed49c75 100644
--- a/src/vmm/src/persist.rs
+++ b/src/vmm/src/persist.rs
@@ -157,7 +157,7 @@ pub enum CreateSnapshotError {
}
/// Snapshot version
-pub const SNAPSHOT_VERSION: Version = Version::new(4, 0, 0);
+pub const SNAPSHOT_VERSION: Version = Version::new(5, 0, 0);
/// Creates a Microvm snapshot.
pub fn create_snapshot(
From 3c23ed4a2cea15ee3321a9f8c605521439f51844 Mon Sep 17 00:00:00 2001
From: Takahiro Itazuri <itazur@amazon.com>
Date: Wed, 13 Nov 2024 10:46:22 +0000
Subject: [PATCH 05/23] test(mmds): Do not use MmdsNetworkStack::new() in tests
There is no need to use MmdsNetworkStack::new() instead of
MmdsNetworkStack::new_with_defaults() in tests that pass the same
default values.
Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
---
src/vmm/src/mmds/ns.rs | 30 ++++++------------------------
1 file changed, 6 insertions(+), 24 deletions(-)
diff --git a/src/vmm/src/mmds/ns.rs b/src/vmm/src/mmds/ns.rs
index d431d71cbcb..8075df8cb91 100644
--- a/src/vmm/src/mmds/ns.rs
+++ b/src/vmm/src/mmds/ns.rs
@@ -553,14 +553,8 @@ mod tests {
let ip = Ipv4Addr::from(DEFAULT_IPV4_ADDR);
let other_ip = Ipv4Addr::new(5, 6, 7, 8);
let mac = MacAddr::from_bytes_unchecked(&[0; 6]);
- let mut ns = MmdsNetworkStack::new(
- mac,
- ip,
- DEFAULT_TCP_PORT,
- NonZeroUsize::new(DEFAULT_MAX_CONNECTIONS).unwrap(),
- NonZeroUsize::new(DEFAULT_MAX_PENDING_RESETS).unwrap(),
- Arc::new(Mutex::new(Mmds::default())),
- );
+ let mut ns =
+ MmdsNetworkStack::new_with_defaults(Some(ip), Arc::new(Mutex::new(Mmds::default())));
let mut eth =
EthernetFrame::write_incomplete(buf.as_mut(), mac, mac, ETHERTYPE_ARP).unwrap();
@@ -580,14 +574,8 @@ mod tests {
let ip = Ipv4Addr::from(DEFAULT_IPV4_ADDR);
let other_ip = Ipv4Addr::new(5, 6, 7, 8);
let mac = MacAddr::from_bytes_unchecked(&[0; 6]);
- let ns = MmdsNetworkStack::new(
- mac,
- ip,
- DEFAULT_TCP_PORT,
- NonZeroUsize::new(DEFAULT_MAX_CONNECTIONS).unwrap(),
- NonZeroUsize::new(DEFAULT_MAX_PENDING_RESETS).unwrap(),
- Arc::new(Mutex::new(Mmds::default())),
- );
+ let ns =
+ MmdsNetworkStack::new_with_defaults(Some(ip), Arc::new(Mutex::new(Mmds::default())));
let mut eth =
EthernetFrame::write_incomplete(buf.as_mut(), mac, mac, ETHERTYPE_IPV4).unwrap();
@@ -606,14 +594,8 @@ mod tests {
let ip = Ipv4Addr::from(DEFAULT_IPV4_ADDR);
let other_ip = Ipv4Addr::new(5, 6, 7, 8);
let mac = MacAddr::from_bytes_unchecked(&[0; 6]);
- let mut ns = MmdsNetworkStack::new(
- mac,
- ip,
- DEFAULT_TCP_PORT,
- NonZeroUsize::new(DEFAULT_MAX_CONNECTIONS).unwrap(),
- NonZeroUsize::new(DEFAULT_MAX_PENDING_RESETS).unwrap(),
- Arc::new(Mutex::new(Mmds::default())),
- );
+ let mut ns =
+ MmdsNetworkStack::new_with_defaults(Some(ip), Arc::new(Mutex::new(Mmds::default())));
// try IPv4 with detour_arp
let mut eth =
From 79cdc987b4f1bc84aab07d0473c244483884ea1c Mon Sep 17 00:00:00 2001
From: Takahiro Itazuri <itazur@amazon.com>
Date: Thu, 14 Nov 2024 10:45:13 +0000
Subject: [PATCH 06/23] chore: Clarify user action
We bumped the snapshot version up twice recently, requiring users to
regenerate their snapshot, but the user action isn't clearly stated.
Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
refactor(builder): Refactor vmm builder code to simplify logic
eliminate the unnecessary usage of the event_manager argument and
fix up aarch64 attach_legacy_devices_aarch64 fn
Signed-off-by: tommady <tommady@users.noreply.github.com>
refactor(builder): address comments
remove the aarch64 suffix from the attach_legacy_devices_aarch64
function and ensure that aarch64 smt is always set to false int the
configure_system_for_boot function
Signed-off-by: tommady <tommady@users.noreply.github.com>
---
CHANGELOG.md | 7 ++++---
src/vmm/src/builder.rs | 12 +++++++-----
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 55c46a05276..7da89123eab 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,7 +14,8 @@ and this project adheres to
- [#4913](https://github.com/firecracker-microvm/firecracker/pull/4913): Removed
unnecessary fields (`max_connections` and `max_pending_resets`) from the
- snapshot format, bumping the snapshot version to 5.0.0.
+ snapshot format, bumping the snapshot version to 5.0.0. Users need to
+ regenerate snapshots.
### Deprecated
@@ -26,8 +27,8 @@ and this project adheres to
### Changed
-- [#4907](https://github.com/firecracker-microvm/firecracker/pull/4907): Bump
- snapshot version to 4.0.0.
+- [#4907](https://github.com/firecracker-microvm/firecracker/pull/4907): Bumped
+ the snapshot version to 4.0.0, so users need to regenerate snapshots.
## \[1.10.0\]
diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs
index 4a3eb48e0fc..51baf54b5b2 100644
--- a/src/vmm/src/builder.rs
+++ b/src/vmm/src/builder.rs
@@ -152,7 +152,7 @@ impl std::convert::From<linux_loader::cmdline::Error> for StartMicrovmError {
pub mod aarch64 {
use super::*;
- fn attach_legacy_devices_aarch64(
+ fn attach_legacy_devices(
event_manager: &mut EventManager,
vmm: &mut Vmm,
cmdline: &mut LoaderKernelCmdline,
@@ -168,7 +168,9 @@ pub mod aarch64 {
if cmdline_contains_console {
// Make stdout non-blocking.
set_stdout_nonblocking();
- let serial = setup_serial_device(event_manager, std::io::stdin(), std::io::stdout())?;
+ let serial = setup_serial_device(std::io::stdin(), std::io::stdout())?;
+ event_manager.add_subscriber(serial.clone());
+
vmm.mmio_device_manager
.register_mmio_serial(vmm.vm.fd(), &mut vmm.resource_allocator, serial, None)
.map_err(VmmError::RegisterMMIODevice)?;
@@ -220,7 +222,8 @@ pub mod aarch64 {
let vcpu_config = VcpuConfig {
vcpu_count: vm_config.vcpu_count,
- smt: vm_config.smt,
+ // smt does not exist on aarch64
+ smt: false,
cpu_config,
};
@@ -640,8 +643,7 @@ pub fn build_microvm_for_boot(
}
#[cfg(target_arch = "aarch64")]
- aarch::attach_legacy_devices_aarch64(event_manager, &mut vmm, &mut boot_cmdline)
- .map_err(Internal)?;
+ aarch::attach_legacy_devices(event_manager, &mut vmm, &mut boot_cmdline).map_err(Internal)?;
attach_vmgenid_device(&mut vmm)?;
From 3928f299f27e04b1d71e3b8e4a68a8acf67c9db4 Mon Sep 17 00:00:00 2001
From: Takahiro Itazuri <itazur@amazon.com>
Date: Wed, 13 Nov 2024 11:07:12 +0000
Subject: [PATCH 07/23] snapshot: Remove max_connections and max_pending_resets
fields
`TcpIPv4Handler` for MMDS network stack preallocates several buffers
whose sizes are saved into a snapshot as `max_connections` and
`max_pending_resets` in `MmdsNetworkStackState`. But they are always the
same constant hardcoded values (`DEFAULT_MAX_CONNECTIONS` and
`DEFAULT_MAX_PENDING_RESETS`) as of today, which means there is no need
to save them into a snapshot. Even if we change the hardcoded sizes
across Firecracker versions, that should not be a problem. This is
because the snapshot feature does not support migration of network
connections and those buffers are initialized with empty on snapshot
restoration. When migrating from a Firecracker version with larger
buffers to another version with smaller ones, guest workloads that
worked previously might start to fail due to the less buffer spaces.
However, the issue is not a problem of the snapshot feature and it
should also occur even on a purely booted microVM (not restored from a
snapshot). Thus, it is fine to remove those fields from a snapshot.
Since this is a breaking change of the snapshot format, bumps the major
version.
Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
---
CHANGELOG.md | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6e7be8f518f..36111becb4e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,8 +14,8 @@ and this project adheres to
- [#4913](https://github.com/firecracker-microvm/firecracker/pull/4913): Removed
unnecessary fields (`max_connections` and `max_pending_resets`) from the
- snapshot format, bumping the snapshot version to 5.0.0. Users need to
- regenerate snapshots.
+
+ snapshot format, bumping the snapshot version to 5.0.0.
### Deprecated
From 0a239cbdf3e66b138d35b14fd9f6fdb27e259320 Mon Sep 17 00:00:00 2001
From: Takahiro Itazuri <itazur@amazon.com>
Date: Thu, 14 Nov 2024 10:45:13 +0000
Subject: [PATCH 08/23] chore: Clarify user action
We bumped the snapshot version up twice recently, requiring users to
regenerate their snapshot, but the user action isn't clearly stated.
Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
refactor(builder): address comments
let the x86_64 and aarch64 architectures code can compile and without
warnning
Signed-off-by: tommady <tommady@users.noreply.github.com>
---
CHANGELOG.md | 4 ++--
src/vmm/src/builder.rs | 25 +++++++++++++------------
src/vmm/src/device_manager/persist.rs | 8 +++-----
3 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 36111becb4e..6e7be8f518f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,8 +14,8 @@ and this project adheres to
- [#4913](https://github.com/firecracker-microvm/firecracker/pull/4913): Removed
unnecessary fields (`max_connections` and `max_pending_resets`) from the
-
- snapshot format, bumping the snapshot version to 5.0.0.
+ snapshot format, bumping the snapshot version to 5.0.0. Users need to
+ regenerate snapshots.
### Deprecated
diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs
index 51baf54b5b2..54541efcb45 100644
--- a/src/vmm/src/builder.rs
+++ b/src/vmm/src/builder.rs
@@ -148,11 +148,12 @@ impl std::convert::From<linux_loader::cmdline::Error> for StartMicrovmError {
}
}
+/// This module is dedicated to code tailored specifically for the aarch64 architecture
#[cfg(target_arch = "aarch64")]
pub mod aarch64 {
use super::*;
- fn attach_legacy_devices(
+ pub(crate) fn attach_legacy_devices(
event_manager: &mut EventManager,
vmm: &mut Vmm,
cmdline: &mut LoaderKernelCmdline,
@@ -166,8 +167,6 @@ pub mod aarch64 {
.contains("console=");
if cmdline_contains_console {
- // Make stdout non-blocking.
- set_stdout_nonblocking();
let serial = setup_serial_device(std::io::stdin(), std::io::stdout())?;
event_manager.add_subscriber(serial.clone());
@@ -280,7 +279,7 @@ pub mod aarch64 {
Ok(vcpus)
}
- pub fn load_kernel(
+ pub(crate) fn load_kernel(
boot_config: &BootConfig,
guest_memory: &GuestMemoryMmap,
) -> Result<GuestAddress, StartMicrovmError> {
@@ -300,7 +299,7 @@ pub mod aarch64 {
Ok(entry_addr.kernel_load)
}
- pub fn create_vmm_and_vcpus(
+ pub(crate) fn create_vmm_and_vcpus(
instance_info: &InstanceInfo,
guest_memory: GuestMemoryMmap,
uffd: Option<Uffd>,
@@ -315,13 +314,14 @@ pub mod aarch64 {
kvm_capabilities,
)?;
- let vcpus =
- create_vcpus(&mut vm, vm_config.vcpu_count, &vmm.vcpus_exit_evt).map_err(Internal)?;
+ let vcpus = create_vcpus(&mut vmm.vm, vm_config.vcpu_count, &vmm.vcpus_exit_evt)
+ .map_err(StartMicrovmError::Internal)?;
Ok((vmm, vcpus))
}
}
+/// This module is dedicated to code tailored specifically for the x86_64 architecture
#[cfg(target_arch = "x86_64")]
pub mod x86_64 {
use super::*;
@@ -422,13 +422,11 @@ pub mod x86_64 {
let vcpu = Vcpu::new(cpu_idx, vm, exit_evt).map_err(VmmError::VcpuCreate)?;
vcpus.push(vcpu);
}
- // Make stdout non blocking.
- set_stdout_nonblocking();
Ok(vcpus)
}
- pub fn load_kernel(
+ pub(crate) fn load_kernel(
boot_config: &BootConfig,
guest_memory: &GuestMemoryMmap,
) -> Result<GuestAddress, StartMicrovmError> {
@@ -448,7 +446,7 @@ pub mod x86_64 {
Ok(entry_addr.kernel_load)
}
- pub fn create_vmm_and_vcpus(
+ pub(crate) fn create_vmm_and_vcpus(
instance_info: &InstanceInfo,
guest_memory: GuestMemoryMmap,
uffd: Option<Uffd>,
@@ -643,7 +641,7 @@ pub fn build_microvm_for_boot(
}
#[cfg(target_arch = "aarch64")]
- aarch::attach_legacy_devices(event_manager, &mut vmm, &mut boot_cmdline).map_err(Internal)?;
+ aarch64::attach_legacy_devices(event_manager, &mut vmm, &mut boot_cmdline).map_err(Internal)?;
attach_vmgenid_device(&mut vmm)?;
@@ -983,6 +981,9 @@ pub fn setup_serial_device(
input: Some(input),
})));
+ // Make stdout non-blocking.
+ set_stdout_nonblocking();
+
Ok(serial)
}
diff --git a/src/vmm/src/device_manager/persist.rs b/src/vmm/src/device_manager/persist.rs
index 5773fa0ba09..9400de57d7e 100644
--- a/src/vmm/src/device_manager/persist.rs
+++ b/src/vmm/src/device_manager/persist.rs
@@ -418,13 +418,11 @@ impl<'a> Persist<'a> for MMIODeviceManager {
#[cfg(target_arch = "aarch64")]
{
+ use crate::builder::setup_serial_device;
+
for state in &state.legacy_devices {
if state.type_ == DeviceType::Serial {
- let serial = crate::builder::setup_serial_device(
- constructor_args.event_manager,
- std::io::stdin(),
- std::io::stdout(),
- )?;
+ let serial = setup_serial_device(std::io::stdin(), std::io::stdout())?;
constructor_args
.resource_allocator
From 860a9cdb9ebca32c45fe3cb1e3270c0cd64429d1 Mon Sep 17 00:00:00 2001
From: tommady <tommady@users.noreply.github.com>
Date: Wed, 27 Nov 2024 04:39:30 +0000
Subject: [PATCH 09/23] refactor(builder): fix unitest
make the test all passed
Signed-off-by: tommady <tommady@users.noreply.github.com>
---
src/vmm/src/builder.rs | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs
index 82c735aee12..a8618b600b2 100644
--- a/src/vmm/src/builder.rs
+++ b/src/vmm/src/builder.rs
@@ -414,8 +414,6 @@ pub mod x86_64 {
vcpu_count: u8,
exit_evt: &EventFd,
) -> Result<Vec<Vcpu>, VmmError> {
- setup_interrupt_controller(vm)?;
-
let mut vcpus = Vec::with_capacity(vcpu_count as usize);
for cpu_idx in 0..vcpu_count {
let exit_evt = exit_evt.try_clone().map_err(VmmError::EventFd)?;
@@ -479,12 +477,12 @@ fn build_vmm(
// Set up Kvm Vm and register memory regions.
// Build custom CPU config if a custom template is provided.
- let vm = Vm::new(kvm_capabilities)
+ let mut vm = Vm::new(kvm_capabilities)
.map_err(VmmError::Vm)
- .map_err(StartMicrovmError::Internal)?;
+ .map_err(Internal)?;
vm.memory_init(&guest_memory, vm_config.track_dirty_pages)
.map_err(VmmError::Vm)
- .map_err(StartMicrovmError::Internal)?;
+ .map_err(Internal)?;
let vcpus_exit_evt = EventFd::new(libc::EFD_NONBLOCK)
.map_err(VmmError::EventFd)
@@ -512,6 +510,8 @@ fn build_vmm(
.map_err(VmmError::EventFd)
.map_err(Internal)?;
+ x86_64::setup_interrupt_controller(&mut vm).map_err(Internal)?;
+
// create pio dev manager with legacy devices
let pio_device_manager = {
// TODO Remove these unwraps.
From 26d25e529c4c3ab59704b86164ebb2048dcb7129 Mon Sep 17 00:00:00 2001
From: tommady <tommady@users.noreply.github.com>
Date: Mon, 2 Dec 2024 04:30:49 +0000
Subject: [PATCH 10/23] refactor(builder): fix aarch64 tests
make the aarch64 test all passed
Signed-off-by: tommady <tommady@users.noreply.github.com>
---
src/vmm/src/builder.rs | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs
index a8618b600b2..ebffd1af2e1 100644
--- a/src/vmm/src/builder.rs
+++ b/src/vmm/src/builder.rs
@@ -477,6 +477,9 @@ fn build_vmm(
// Set up Kvm Vm and register memory regions.
// Build custom CPU config if a custom template is provided.
+ //
+ // allow unused_mut for the aarch64 platform.
+ #[allow(unused_mut)]
let mut vm = Vm::new(kvm_capabilities)
.map_err(VmmError::Vm)
.map_err(Internal)?;
@@ -1263,7 +1266,7 @@ pub mod tests {
{
let exit_evt = EventFd::new(libc::EFD_NONBLOCK).unwrap();
let _vcpu = Vcpu::new(1, &vm, exit_evt).unwrap();
- setup_interrupt_controller(&mut vm, 1).unwrap();
+ aarch64::setup_interrupt_controller(&mut vm, 1).unwrap();
}
Vmm {
@@ -1493,9 +1496,14 @@ pub mod tests {
let evfd = EventFd::new(libc::EFD_NONBLOCK).unwrap();
#[cfg(target_arch = "x86_64")]
- x86_64::setup_interrupt_controller(&mut vm).unwrap();
+ let vcpu_vec = {
+ x86_64::setup_interrupt_controller(&mut vm).unwrap();
+ x86_64::create_vcpus(&mut vm, vcpu_count, &evfd).unwrap()
+ };
+
+ #[cfg(target_arch = "aarch64")]
+ let vcpu_vec = aarch64::create_vcpus(&mut vm, vcpu_count, &evfd).unwrap();
- let vcpu_vec = x86_64::create_vcpus(&mut vm, vcpu_count, &evfd).unwrap();
assert_eq!(vcpu_vec.len(), vcpu_count as usize);
}
From 55b4ac36b51687d8ab3c5ead8ebff72233091684 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= <pablob@amazon.com>
Date: Sun, 20 Oct 2024 13:36:58 +0200
Subject: [PATCH 11/23] tests: save metadata on snapshot
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
There is some information in the Microvm class that we don't save in the
snapshot. Some tests do depend on those, so to make the booted/restored
case homogenous, make room in the snapshot to save metadata that we can
then restore.
Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
---
tests/framework/microvm.py | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tests/framework/microvm.py b/tests/framework/microvm.py
index ed02488bc7b..c1be701665a 100644
--- a/tests/framework/microvm.py
+++ b/tests/framework/microvm.py
@@ -75,6 +75,7 @@ class Snapshot:
disks: dict
ssh_key: Path
snapshot_type: SnapshotType
+ meta: dict
@property
def is_diff(self) -> bool:
@@ -110,6 +111,7 @@ def copy_to_chroot(self, chroot) -> "Snapshot":
disks=self.disks,
ssh_key=self.ssh_key,
snapshot_type=self.snapshot_type,
+ meta=self.meta,
)
@classmethod
@@ -125,6 +127,7 @@ def load_from(cls, src: Path) -> "Snapshot":
disks={dsk: src / p for dsk, p in obj["disks"].items()},
ssh_key=src / obj["ssh_key"],
snapshot_type=SnapshotType(obj["snapshot_type"]),
+ meta=obj["meta"],
)
def save_to(self, dst: Path):
@@ -917,6 +920,9 @@ def make_snapshot(
net_ifaces=[x["iface"] for ifname, x in self.iface.items()],
ssh_key=self.ssh_key,
snapshot_type=snapshot_type,
+ meta={
+ "kernel_file": self.kernel_file,
+ },
)
def snapshot_diff(self, *, mem_path: str = "mem", vmstate_path="vmstate"):
@@ -954,6 +960,9 @@ def restore_from_snapshot(
if uffd_path is not None:
mem_backend = {"backend_type": "Uffd", "backend_path": str(uffd_path)}
+ for key, value in snapshot.meta.items():
+ setattr(self, key, value)
+
self.api.snapshot_load.put(
mem_backend=mem_backend,
snapshot_path=str(jailed_vmstate),
From 7df8cbf6f049912ec00784e0c3de9373dc558c63 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= <pablob@amazon.com>
Date: Sun, 20 Oct 2024 12:55:38 +0200
Subject: [PATCH 12/23] tests: refactor test_vulnerabilities
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Some further simplifications:
- Simplify the "_host" tests as they will provide the same result in
both A/B situations.
- Add a second MicrovmFactory fixture with the "A" firecracker.
- Add a uvm_any fixture that includes all CPU templates and also a
boot/restored dimension.
This decreases the need for separate tests. For example the guest test
test_check_vulnerability_files_ab can now test all variants:
2 (restored/booted) * 3 (kernels) * 9 (cpu templates) = 54 tests
Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
---
tests/README.md | 12 +-
tests/conftest.py | 35 ++
tests/framework/ab_test.py | 64 +--
tests/framework/microvm.py | 18 +-
tests/framework/properties.py | 5 +-
tests/framework/utils_cpu_templates.py | 4 +-
.../integration_tests/functional/test_net.py | 43 +-
.../security/test_vulnerabilities.py | 499 +++++-------------
8 files changed, 213 insertions(+), 467 deletions(-)
diff --git a/tests/README.md b/tests/README.md
index ea46ff56786..bf7aba9a547 100644
--- a/tests/README.md
+++ b/tests/README.md
@@ -306,10 +306,14 @@ that are pre-initialized with specific guest kernels and rootfs:
24.04 squashfs as rootfs,
- `uvm_plain` yields a Firecracker process pre-initialized with a 5.10 kernel
and the same Ubuntu 24.04 squashfs.
-
-Generally, tests should use the former if you are testing some interaction
-between the guest and Firecracker, while the latter should be used if
-Firecracker functionality unrelated to the guest is being tested.
+- `uvm_any` yields started microvms, parametrized by all supported kernels, all
+ CPU templates (static, custom and none), and either booted or restored from a
+ snapshot.
+- `uvm_any_booted` works the same as `uvm_any`, but only for booted VMs.
+
+Generally, tests should use `uvm_plain_any` if you are testing some interaction
+between the guest and Firecracker, and `uvm_plain` should be used if Firecracker
+functionality unrelated to the guest is being tested.
### Markers
diff --git a/tests/conftest.py b/tests/conftest.py
index 8f4c2e51ff5..5ad291d01a4 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -459,3 +459,38 @@ def uvm_with_initrd(
uvm = microvm_factory.build(guest_kernel_linux_5_10)
uvm.initrd_file = fs
yield uvm
+
+
+def uvm_booted(microvm_factory, guest_kernel, rootfs, cpu_template):
+ """Return a booted uvm"""
+ uvm = microvm_factory.build(guest_kernel, rootfs)
+ uvm.spawn()
+ uvm.basic_config(vcpu_count=2, mem_size_mib=256)
+ uvm.set_cpu_template(cpu_template)
+ uvm.add_net_iface()
+ uvm.start()
+ return uvm
+
+
+def uvm_restored(microvm_factory, guest_kernel, rootfs, cpu_template):
+ """Return a restored uvm"""
+ uvm = uvm_booted(microvm_factory, guest_kernel, rootfs, cpu_template)
+ snapshot = uvm.snapshot_full()
+ uvm.kill()
+ uvm2 = microvm_factory.build()
+ uvm2.spawn()
+ uvm2.restore_from_snapshot(snapshot, resume=True)
+ uvm2.cpu_template_name = uvm.cpu_template_name
+ return uvm2
+
+
+@pytest.fixture(params=[uvm_booted, uvm_restored])
+def uvm_ctor(request):
+ """Fixture to return uvms with different constructors"""
+ return request.param
+
+
+@pytest.fixture
+def uvm_any(microvm_factory, uvm_ctor, guest_kernel, rootfs, cpu_template_any):
+ """Return booted and restored uvms"""
+ return uvm_ctor(microvm_factory, guest_kernel, rootfs, cpu_template_any)
diff --git a/tests/framework/ab_test.py b/tests/framework/ab_test.py
index cf909d44fa6..2ef3e2350a7 100644
--- a/tests/framework/ab_test.py
+++ b/tests/framework/ab_test.py
@@ -21,7 +21,6 @@
of both invocations is the same, the test passes (with us being alerted to this situtation via a special pipeline that
does not block PRs). If not, it fails, preventing PRs from introducing new vulnerable dependencies.
"""
-import os
import statistics
from pathlib import Path
from tempfile import TemporaryDirectory
@@ -31,14 +30,14 @@
from framework import utils
from framework.defs import FC_WORKSPACE_DIR
-from framework.microvm import Microvm
+from framework.properties import global_props
from framework.utils import CommandReturn
from framework.with_filelock import with_filelock
-from host_tools.cargo_build import DEFAULT_TARGET_DIR, get_firecracker_binaries
+from host_tools.cargo_build import DEFAULT_TARGET_DIR
# Locally, this will always compare against main, even if we try to merge into, say, a feature branch.
# We might want to do a more sophisticated way to determine a "parent" branch here.
-DEFAULT_A_REVISION = os.environ.get("BUILDKITE_PULL_REQUEST_BASE_BRANCH") or "main"
+DEFAULT_A_REVISION = global_props.buildkite_revision_a or "main"
T = TypeVar("T")
@@ -120,11 +119,6 @@ def binary_ab_test(
return result_a, result_b, comparator(result_a, result_b)
-def is_pr() -> bool:
- """Returns `True` iff we are executing in the context of a build kite run on a pull request"""
- return os.environ.get("BUILDKITE_PULL_REQUEST", "false") != "false"
-
-
def git_ab_test_host_command_if_pr(
command: str,
*,
@@ -134,7 +128,7 @@ def git_ab_test_host_command_if_pr(
"""Runs the given bash command as an A/B-Test if we're in a pull request context (asserting that its stdout and
stderr did not change across the PR). Otherwise runs the command, asserting it returns a zero exit code
"""
- if is_pr():
+ if global_props.buildkite_pr:
git_ab_test_host_command(command, comparator=comparator)
return None
@@ -176,56 +170,6 @@ def set_did_not_grow_comparator(
)
-def precompiled_ab_test_guest_command(
- microvm_factory: Callable[[Path, Path], Microvm],
- command: str,
- *,
- comparator: Callable[[CommandReturn, CommandReturn], bool] = default_comparator,
- a_revision: str = DEFAULT_A_REVISION,
- b_revision: Optional[str] = None,
-):
- """The same as git_ab_test_command, but via SSH. The closure argument should setup a microvm using the passed
- paths to firecracker and jailer binaries."""
- b_directory = (
- DEFAULT_B_DIRECTORY
- if b_revision is None
- else FC_WORKSPACE_DIR / "build" / b_revision
- )
-
- def test_runner(bin_dir, _is_a: bool):
- microvm = microvm_factory(bin_dir / "firecracker", bin_dir / "jailer")
- return microvm.ssh.run(command)
-
- (_, old_out, old_err), (_, new_out, new_err), the_same = binary_ab_test(
- test_runner,
- comparator,
- a_directory=FC_WORKSPACE_DIR / "build" / a_revision,
- b_directory=b_directory,
- )
-
- assert (
- the_same
- ), f"The output of running command `{command}` changed:\nOld:\nstdout:\n{old_out}\nstderr\n{old_err}\n\nNew:\nstdout:\n{new_out}\nstderr:\n{new_err}"
-
-
-def precompiled_ab_test_guest_command_if_pr(
- microvm_factory: Callable[[Path, Path], Microvm],
- command: str,
- *,
- comparator=default_comparator,
- check_in_nonpr=True,
-):
- """The same as git_ab_test_command_if_pr, but via SSH"""
- if is_pr():
- precompiled_ab_test_guest_command(
- microvm_factory, command, comparator=comparator
- )
- return None
-
- microvm = microvm_factory(*get_firecracker_binaries())
- return microvm.ssh.run(command, check=check_in_nonpr)
-
-
def check_regression(
a_samples: List[float], b_samples: List[float], *, n_resamples: int = 9999
):
diff --git a/tests/framework/microvm.py b/tests/framework/microvm.py
index c1be701665a..832f56a877e 100644
--- a/tests/framework/microvm.py
+++ b/tests/framework/microvm.py
@@ -244,6 +244,7 @@ def __init__(
self.disks_vhost_user = {}
self.vcpus_count = None
self.mem_size_bytes = None
+ self.cpu_template_name = None
self._pre_cmd = []
if numa_node:
@@ -735,12 +736,14 @@ def basic_config(
smt=smt,
mem_size_mib=mem_size_mib,
track_dirty_pages=track_dirty_pages,
- cpu_template=cpu_template,
huge_pages=huge_pages,
)
self.vcpus_count = vcpu_count
self.mem_size_bytes = mem_size_mib * 2**20
+ if cpu_template is not None:
+ self.set_cpu_template(cpu_template)
+
if self.memory_monitor:
self.memory_monitor.start()
@@ -773,6 +776,19 @@ def basic_config(
if enable_entropy_device:
self.enable_entropy_device()
+ def set_cpu_template(self, cpu_template):
+ """Set guest CPU template."""
+ if cpu_template is None:
+ return
+ # static CPU template
+ if isinstance(cpu_template, str):
+ self.api.machine_config.patch(cpu_template=cpu_template)
+ self.cpu_template_name = cpu_template.lower()
+ # custom CPU template
+ elif isinstance(cpu_template, dict):
+ self.api.cpu_config.put(**cpu_template["template"])
+ self.cpu_template_name = cpu_template["name"].lower()
+
def add_drive(
self,
drive_id,
diff --git a/tests/framework/properties.py b/tests/framework/properties.py
index b40df56249e..7dd1cb46588 100644
--- a/tests/framework/properties.py
+++ b/tests/framework/properties.py
@@ -72,11 +72,14 @@ def __init__(self):
# major.minor.patch
self.host_linux_patch = get_kernel_version(2)
self.os = get_os_version()
- self.host_os = get_host_os()
+ self.host_os = get_host_os() or "NA"
self.libc_ver = "-".join(platform.libc_ver())
self.rust_version = run_cmd("rustc --version |awk '{print $2}'")
+ # Buildkite/PR information
self.buildkite_pipeline_slug = os.environ.get("BUILDKITE_PIPELINE_SLUG")
self.buildkite_build_number = os.environ.get("BUILDKITE_BUILD_NUMBER")
+ self.buildkite_pr = os.environ.get("BUILDKITE_PULL_REQUEST", "false") != "false"
+ self.buildkite_revision_a = os.environ.get("BUILDKITE_PULL_REQUEST_BASE_BRANCH")
if self._in_git_repo():
self.git_commit_id = run_cmd("git rev-parse HEAD")
diff --git a/tests/framework/utils_cpu_templates.py b/tests/framework/utils_cpu_templates.py
index e57ecfe72c7..89326c1da5a 100644
--- a/tests/framework/utils_cpu_templates.py
+++ b/tests/framework/utils_cpu_templates.py
@@ -3,6 +3,8 @@
"""Utilities for CPU template related functionality."""
+# pylint:disable=too-many-return-statements
+
import json
from pathlib import Path
@@ -23,9 +25,7 @@ def get_supported_cpu_templates():
"""
Return the list of CPU templates supported by the platform.
"""
- # pylint:disable=too-many-return-statements
host_linux = global_props.host_linux_version_tpl
-
match get_cpu_vendor(), global_props.cpu_codename:
# T2CL template is only supported on Cascade Lake and newer CPUs.
case CpuVendor.INTEL, CpuModel.INTEL_SKYLAKE:
diff --git a/tests/integration_tests/functional/test_net.py b/tests/integration_tests/functional/test_net.py
index a804b8f90a8..2072d015ca2 100644
--- a/tests/integration_tests/functional/test_net.py
+++ b/tests/integration_tests/functional/test_net.py
@@ -84,14 +84,23 @@ def test_multi_queue_unsupported(uvm_plain):
)
-def run_udp_offload_test(vm):
+@pytest.fixture
+def uvm_any(microvm_factory, uvm_ctor, guest_kernel, rootfs):
+ """Return booted and restored uvm with no CPU templates"""
+ return uvm_ctor(microvm_factory, guest_kernel, rootfs, None)
+
+
+def test_tap_offload(uvm_any):
"""
+ Verify that tap offload features are configured for a booted/restored VM.
+
- Start a socat UDP server in the guest.
- Try to send a UDP message with UDP offload enabled.
If tap offload features are not configured, an attempt to send a message will fail with EIO "Input/output error".
More info (search for "TUN_F_CSUM is a must"): https://blog.cloudflare.com/fr-fr/virtual-networking-101-understanding-tap/
"""
+ vm = uvm_any
port = "81"
out_filename = "/tmp/out.txt"
message = "x"
@@ -112,35 +121,3 @@ def run_udp_offload_test(vm):
# Check that the server received the message
ret = vm.ssh.run(f"cat {out_filename}")
assert ret.stdout == message, f"{ret.stdout=} {ret.stderr=}"
-
-
-def test_tap_offload_booted(uvm_plain_any):
- """
- Verify that tap offload features are configured for a booted VM.
- """
- vm = uvm_plain_any
- vm.spawn()
- vm.basic_config()
- vm.add_net_iface()
- vm.start()
-
- run_udp_offload_test(vm)
-
-
-def test_tap_offload_restored(microvm_factory, guest_kernel, rootfs):
- """
- Verify that tap offload features are configured for a restored VM.
- """
- src = microvm_factory.build(guest_kernel, rootfs, monitor_memory=False)
- src.spawn()
- src.basic_config()
- src.add_net_iface()
- src.start()
- snapshot = src.snapshot_full()
- src.kill()
-
- dst = microvm_factory.build()
- dst.spawn()
- dst.restore_from_snapshot(snapshot, resume=True)
-
- run_udp_offload_test(dst)
diff --git a/tests/integration_tests/security/test_vulnerabilities.py b/tests/integration_tests/security/test_vulnerabilities.py
index fed9dbc96d7..a4461aded78 100644
--- a/tests/integration_tests/security/test_vulnerabilities.py
+++ b/tests/integration_tests/security/test_vulnerabilities.py
@@ -5,21 +5,17 @@
# script from the third party "Spectre & Meltdown Checker" project. This script is under the
# GPL-3.0-only license.
"""Tests vulnerabilities mitigations."""
+
import json
-import os
+from pathlib import Path
import pytest
import requests
from framework import utils
-from framework.ab_test import (
- is_pr,
- precompiled_ab_test_guest_command,
- precompiled_ab_test_guest_command_if_pr,
- set_did_not_grow_comparator,
-)
+from framework.ab_test import git_clone
+from framework.microvm import MicroVMFactory
from framework.properties import global_props
-from framework.utils import CommandReturn
CHECKER_URL = "https://meltdown.ovh"
CHECKER_FILENAME = "spectre-meltdown-checker.sh"
@@ -29,119 +25,71 @@
VULN_DIR = "/sys/devices/system/cpu/vulnerabilities"
-def configure_microvm(
- factory,
- kernel,
- rootfs,
- *,
- firecracker=None,
- jailer=None,
- cpu_template=None,
- custom_cpu_template=None,
-):
- """Build a microvm for vulnerability tests"""
- assert not (cpu_template and custom_cpu_template)
- # Either both or neither are specified
- assert firecracker and jailer or not firecracker and not jailer
-
- if firecracker:
- microvm = factory.build(
- kernel, rootfs, fc_binary_path=firecracker, jailer_binary_path=jailer
- )
- else:
- microvm = factory.build(kernel, rootfs)
-
- microvm.spawn()
- microvm.basic_config(vcpu_count=2, mem_size_mib=256, cpu_template=cpu_template)
- if custom_cpu_template:
- microvm.api.cpu_config.put(**custom_cpu_template["template"])
- microvm.cpu_template = cpu_template
- if cpu_template is None and custom_cpu_template is not None:
- microvm.cpu_template = custom_cpu_template["name"]
- microvm.add_net_iface()
- microvm.start()
- return microvm
+class SpectreMeltdownChecker:
+ """Helper class to use Spectre & Meltdown Checker"""
+ def __init__(self, path):
+ self.path = path
-@pytest.fixture
-def build_microvm(
- microvm_factory,
- guest_kernel_linux_5_10,
- rootfs,
-):
- """Fixture returning a factory function for a normal microvm"""
- return lambda firecracker=None, jailer=None: configure_microvm(
- microvm_factory,
- guest_kernel_linux_5_10,
- rootfs,
- firecracker=firecracker,
- jailer=jailer,
- )
-
-
-@pytest.fixture
-def build_microvm_with_template(
- microvm_factory, guest_kernel_linux_5_10, rootfs, cpu_template
-):
- """Fixture returning a factory function for microvms with our built-in template"""
- return lambda firecracker=None, jailer=None: configure_microvm(
- microvm_factory,
- guest_kernel_linux_5_10,
- rootfs,
- firecracker=firecracker,
- jailer=jailer,
- cpu_template=cpu_template,
- )
-
-
-@pytest.fixture
-def build_microvm_with_custom_template(
- microvm_factory, guest_kernel_linux_5_10, rootfs, custom_cpu_template
-):
- """Fixture returning a factory function for microvms with custom cpu templates"""
- return lambda firecracker=None, jailer=None: configure_microvm(
- microvm_factory,
- guest_kernel_linux_5_10,
- rootfs,
- firecracker=firecracker,
- jailer=jailer,
- custom_cpu_template=custom_cpu_template,
- )
-
-
-def with_restore(factory, microvm_factory):
- """Turns the given microvm factory into one that makes the microvm go through a snapshot-restore cycle"""
-
- def restore(firecracker=None, jailer=None):
- microvm = factory(firecracker, jailer)
-
- snapshot = microvm.snapshot_full()
-
- if firecracker:
- dst_vm = microvm_factory.build(
- fc_binary_path=firecracker, jailer_binary_path=jailer
- )
- else:
- dst_vm = microvm_factory.build()
- dst_vm.spawn()
- # Restore the destination VM from the snapshot
- dst_vm.restore_from_snapshot(snapshot, resume=True)
- dst_vm.cpu_template = microvm.cpu_template
-
- return dst_vm
-
- return restore
-
-
-def with_checker(factory, spectre_meltdown_checker):
- """Turns the given microvm factory function into one that also contains the spectre-meltdown checker script"""
+ def _parse_output(self, output):
+ return {
+ json.dumps(entry) # dict is unhashable
+ for entry in json.loads(output)
+ if entry["VULNERABLE"]
+ }
- def download_checker(firecracker, jailer):
- microvm = factory(firecracker, jailer)
- microvm.ssh.scp_put(spectre_meltdown_checker, REMOTE_CHECKER_PATH)
- return microvm
+ def get_report_for_guest(self, vm) -> set:
+ """Parses the output of `spectre-meltdown-checker.sh --batch json`
+ and returns the set of issues for which it reported 'Vulnerable'.
- return download_checker
+ Sample stdout:
+ ```
+ [
+ {
+ "NAME": "SPECTRE VARIANT 1",
+ "CVE": "CVE-2017-5753",
+ "VULNERABLE": false,
+ "INFOS": "Mitigation: usercopy/swapgs barriers and __user pointer sanitization"
+ },
+ { ... }
+ ]
+ ```
+ """
+ vm.ssh.scp_put(self.path, REMOTE_CHECKER_PATH)
+ res = vm.ssh.run(REMOTE_CHECKER_COMMAND)
+ return self._parse_output(res.stdout)
+
+ def get_report_for_host(self) -> set:
+ """Runs `spectre-meltdown-checker.sh` in the host and returns the set of
+ issues for which it reported 'Vulnerable'.
+ """
+
+ res = utils.check_output(f"sh {self.path} --batch json")
+ return self._parse_output(res.stdout)
+
+ def expected_vulnerabilities(self, cpu_template_name):
+ """
+ There is a REPTAR exception reported on INTEL_ICELAKE when spectre-meltdown-checker.sh
+ script is run inside the guest from below the tests:
+ test_spectre_meltdown_checker_on_guest and
+ test_spectre_meltdown_checker_on_restored_guest
+ The same script when run on host doesn't report the
+ exception which means the instances are actually not vulnerable to REPTAR.
+ The only reason why the script cannot determine if the guest
+ is vulnerable or not because Firecracker does not expose the microcode
+ version to the guest.
+
+ The check in spectre_meltdown_checker is here:
+ https://github.com/speed47/spectre-meltdown-checker/blob/0f2edb1a71733c1074550166c5e53abcfaa4d6ca/spectre-meltdown-checker.sh#L6635-L6637
+
+ Since we have a test on host and the exception in guest is not valid,
+ we add a check to ignore this exception.
+ """
+ if global_props.cpu_codename == "INTEL_ICELAKE" and cpu_template_name is None:
+ return {
+ '{"NAME": "REPTAR", "CVE": "CVE-2023-23583", "VULNERABLE": true, "INFOS": "Your microcode is too old to mitigate the vulnerability"}'
+ }
+ return set()
@pytest.fixture(scope="session", name="spectre_meltdown_checker")
@@ -149,189 +97,32 @@ def download_spectre_meltdown_checker(tmp_path_factory):
"""Download spectre / meltdown checker script."""
resp = requests.get(CHECKER_URL, timeout=5)
resp.raise_for_status()
-
path = tmp_path_factory.mktemp("tmp", True) / CHECKER_FILENAME
path.write_bytes(resp.content)
-
- return path
-
-
-def spectre_meltdown_reported_vulnerablities(
- spectre_meltdown_checker_output: CommandReturn,
-) -> set:
- """
- Parses the output of `spectre-meltdown-checker.sh --batch json` and returns the set of issues
- for which it reported 'Vulnerable'.
-
- Sample stdout:
- ```
- [
- {
- "NAME": "SPECTRE VARIANT 1",
- "CVE": "CVE-2017-5753",
- "VULNERABLE": false,
- "INFOS": "Mitigation: usercopy/swapgs barriers and __user pointer sanitization"
- },
- {
- ...
- }
- ]
- ```
- """
- return {
- json.dumps(entry) # dict is unhashable
- for entry in json.loads(spectre_meltdown_checker_output.stdout)
- if entry["VULNERABLE"]
- }
-
-
-def check_vulnerabilities_on_guest(status):
- """
- There is a REPTAR exception reported on INTEL_ICELAKE when spectre-meltdown-checker.sh
- script is run inside the guest from below the tests:
- test_spectre_meltdown_checker_on_guest and
- test_spectre_meltdown_checker_on_restored_guest
- The same script when run on host doesn't report the
- exception which means the instances are actually not vulnerable to REPTAR.
- The only reason why the script cannot determine if the guest
- is vulnerable or not because Firecracker does not expose the microcode
- version to the guest.
-
- The check in spectre_meltdown_checker is here:
- https://github.com/speed47/spectre-meltdown-checker/blob/0f2edb1a71733c1074550166c5e53abcfaa4d6ca/spectre-meltdown-checker.sh#L6635-L6637
-
- Since we have a test on host and the exception in guest is not valid,
- we add a check to ignore this exception.
- """
- report_guest_vulnerabilities = spectre_meltdown_reported_vulnerablities(status)
- known_guest_vulnerabilities = set()
- if global_props.cpu_codename == "INTEL_ICELAKE":
- known_guest_vulnerabilities = {
- '{"NAME": "REPTAR", "CVE": "CVE-2023-23583", "VULNERABLE": true, "INFOS": "Your microcode is too old to mitigate the vulnerability"}'
- }
- assert report_guest_vulnerabilities == known_guest_vulnerabilities
+ return SpectreMeltdownChecker(path)
# Nothing can be sensibly tested in a PR context here
@pytest.mark.skipif(
- is_pr(), reason="Test depends solely on factors external to GitHub repository"
+ global_props.buildkite_pr,
+ reason="Test depends solely on factors external to GitHub repository",
)
def test_spectre_meltdown_checker_on_host(spectre_meltdown_checker):
- """
- Test with the spectre / meltdown checker on host.
- """
- rc, output, _ = utils.run_cmd(f"sh {spectre_meltdown_checker} --batch json")
-
- if output and rc != 0:
- report = spectre_meltdown_reported_vulnerablities(output)
- expected = {}
- assert report == expected, f"Unexpected vulnerabilities: {report} vs {expected}"
-
-
-def test_spectre_meltdown_checker_on_guest(spectre_meltdown_checker, build_microvm):
- """
- Test with the spectre / meltdown checker on guest.
- """
-
- status = precompiled_ab_test_guest_command_if_pr(
- with_checker(build_microvm, spectre_meltdown_checker),
- REMOTE_CHECKER_COMMAND,
- comparator=set_did_not_grow_comparator(
- spectre_meltdown_reported_vulnerablities
- ),
- check_in_nonpr=False,
- )
- if status and status.returncode != 0:
- check_vulnerabilities_on_guest(status)
-
-
-def test_spectre_meltdown_checker_on_restored_guest(
- spectre_meltdown_checker, build_microvm, microvm_factory
-):
- """
- Test with the spectre / meltdown checker on a restored guest.
- """
- status = precompiled_ab_test_guest_command_if_pr(
- with_checker(
- with_restore(build_microvm, microvm_factory), spectre_meltdown_checker
- ),
- REMOTE_CHECKER_COMMAND,
- comparator=set_did_not_grow_comparator(
- spectre_meltdown_reported_vulnerablities
- ),
- check_in_nonpr=False,
- )
- if status and status.returncode != 0:
- check_vulnerabilities_on_guest(status)
-
-
-def test_spectre_meltdown_checker_on_guest_with_template(
- spectre_meltdown_checker, build_microvm_with_template
-):
- """
- Test with the spectre / meltdown checker on guest with CPU template.
- """
+ """Test with the spectre / meltdown checker on host."""
+ report = spectre_meltdown_checker.get_report_for_host()
+ assert report == set(), f"Unexpected vulnerabilities: {report}"
- precompiled_ab_test_guest_command_if_pr(
- with_checker(build_microvm_with_template, spectre_meltdown_checker),
- REMOTE_CHECKER_COMMAND,
- comparator=set_did_not_grow_comparator(
- spectre_meltdown_reported_vulnerablities
- ),
- )
-
-def test_spectre_meltdown_checker_on_guest_with_custom_template(
- spectre_meltdown_checker, build_microvm_with_custom_template
-):
- """
- Test with the spectre / meltdown checker on guest with a custom CPU template.
- """
- precompiled_ab_test_guest_command_if_pr(
- with_checker(build_microvm_with_custom_template, spectre_meltdown_checker),
- REMOTE_CHECKER_COMMAND,
- comparator=set_did_not_grow_comparator(
- spectre_meltdown_reported_vulnerablities
- ),
- )
-
-
-def test_spectre_meltdown_checker_on_restored_guest_with_template(
- spectre_meltdown_checker, build_microvm_with_template, microvm_factory
-):
- """
- Test with the spectre / meltdown checker on a restored guest with a CPU template.
- """
- precompiled_ab_test_guest_command_if_pr(
- with_checker(
- with_restore(build_microvm_with_template, microvm_factory),
- spectre_meltdown_checker,
- ),
- REMOTE_CHECKER_COMMAND,
- comparator=set_did_not_grow_comparator(
- spectre_meltdown_reported_vulnerablities
- ),
- )
-
-
-def test_spectre_meltdown_checker_on_restored_guest_with_custom_template(
- spectre_meltdown_checker,
- build_microvm_with_custom_template,
- microvm_factory,
-):
- """
- Test with the spectre / meltdown checker on a restored guest with a custom CPU template.
- """
- precompiled_ab_test_guest_command_if_pr(
- with_checker(
- with_restore(build_microvm_with_custom_template, microvm_factory),
- spectre_meltdown_checker,
- ),
- REMOTE_CHECKER_COMMAND,
- comparator=set_did_not_grow_comparator(
- spectre_meltdown_reported_vulnerablities
- ),
- )
+# Nothing can be sensibly tested here in a PR context
+@pytest.mark.skipif(
+ global_props.buildkite_pr,
+ reason="Test depends solely on factors external to GitHub repository",
+)
+def test_vulnerabilities_on_host():
+ """Test vulnerability files on host."""
+ res = utils.run_cmd(f"grep -r Vulnerable {VULN_DIR}")
+ # if grep finds no matching lines, it exits with status 1
+ assert res.returncode == 1, res.stdout
def get_vuln_files_exception_dict(template):
@@ -372,25 +163,14 @@ def get_vuln_files_exception_dict(template):
# Since those bits are not set on Intel Skylake and C3 template makes guests pretend to be AWS
# C3 instance (quite old processor now) by overwriting CPUID.1H:EAX, it is impossible to avoid
# this "Unknown" state.
- if global_props.cpu_codename == "INTEL_SKYLAKE" and template == "C3":
+ if global_props.cpu_codename == "INTEL_SKYLAKE" and template == "c3":
exception_dict["mmio_stale_data"] = "Unknown: No mitigations"
- elif global_props.cpu_codename == "INTEL_SKYLAKE" or template == "T2S":
+ elif global_props.cpu_codename == "INTEL_SKYLAKE" or template == "t2s":
exception_dict["mmio_stale_data"] = "Clear CPU buffers"
return exception_dict
-# Nothing can be sensibly tested here in a PR context
-@pytest.mark.skipif(
- is_pr(), reason="Test depends solely on factors external to GitHub repository"
-)
-def test_vulnerabilities_on_host():
- """
- Test vulnerabilities files on host.
- """
- utils.check_output(f"! grep -r Vulnerable {VULN_DIR}")
-
-
def check_vulnerabilities_files_on_guest(microvm):
"""
Check that the guest's vulnerabilities files do not contain `Vulnerable`.
@@ -400,88 +180,75 @@ def check_vulnerabilities_files_on_guest(microvm):
# Retrieve a list of vulnerabilities files available inside guests.
vuln_dir = "/sys/devices/system/cpu/vulnerabilities"
_, stdout, _ = microvm.ssh.check_output(f"find -D all {vuln_dir} -type f")
- vuln_files = stdout.split("\n")
+ vuln_files = stdout.splitlines()
# Fixtures in this file (test_vulnerabilities.py) add this special field.
- template = microvm.cpu_template
+ template = microvm.cpu_template_name
# Check that vulnerabilities files in the exception dictionary have the expected values and
# the others do not contain "Vulnerable".
exceptions = get_vuln_files_exception_dict(template)
+ results = []
for vuln_file in vuln_files:
- filename = os.path.basename(vuln_file)
+ filename = Path(vuln_file).name
if filename in exceptions:
- _, stdout, _ = microvm.ssh.run(f"cat {vuln_file}")
+ _, stdout, _ = microvm.ssh.check_output(f"cat {vuln_file}")
assert exceptions[filename] in stdout
else:
cmd = f"grep Vulnerable {vuln_file}"
- ecode, stdout, stderr = microvm.ssh.run(cmd)
- assert ecode == 1, f"{vuln_file}: stdout:\n{stdout}\nstderr:\n{stderr}\n"
-
-
-def check_vulnerabilities_files_ab(builder):
- """Does an A/B test on the contents of the /sys/devices/system/cpu/vulnerabilities files in the guest if
- running in a PR pipeline, and otherwise calls `check_vulnerabilities_files_on_guest`
- """
- if is_pr():
- precompiled_ab_test_guest_command(
- builder,
- f"! grep -r Vulnerable {VULN_DIR}",
- comparator=set_did_not_grow_comparator(
- lambda output: set(output.stdout.splitlines())
- ),
- )
- else:
- check_vulnerabilities_files_on_guest(builder())
+ _ecode, stdout, _stderr = microvm.ssh.run(cmd)
+ results.append({"file": vuln_file, "stdout": stdout})
+ return results
-def test_vulnerabilities_files_on_guest(build_microvm):
- """
- Test vulnerabilities files on guest.
- """
- check_vulnerabilities_files_ab(build_microvm)
-
-
-def test_vulnerabilities_files_on_restored_guest(build_microvm, microvm_factory):
- """
- Test vulnerabilities files on a restored guest.
- """
- check_vulnerabilities_files_ab(with_restore(build_microvm, microvm_factory))
+@pytest.fixture
+def microvm_factory_a(record_property):
+ """MicroVMFactory using revision A binaries"""
+ revision_a = global_props.buildkite_revision_a
+ bin_dir = git_clone(Path("../build") / revision_a, revision_a).resolve()
+ fc_bin = bin_dir / "firecracker"
+ jailer_bin = bin_dir / "jailer"
+ record_property("firecracker_bin", str(fc_bin))
+ uvm_factory = MicroVMFactory(fc_bin, jailer_bin)
+ yield uvm_factory
+ uvm_factory.kill()
-def test_vulnerabilities_files_on_guest_with_template(build_microvm_with_template):
- """
- Test vulnerabilities files on guest with CPU template.
- """
- check_vulnerabilities_files_ab(build_microvm_with_template)
-
+@pytest.fixture
+def uvm_any_a(microvm_factory_a, uvm_ctor, guest_kernel, rootfs, cpu_template_any):
+ """Return uvm with revision A firecracker
-def test_vulnerabilities_files_on_guest_with_custom_template(
- build_microvm_with_custom_template,
-):
+ Since pytest caches fixtures, this guarantees uvm_any_a will match a vm from uvm_any.
+ See https://docs.pytest.org/en/stable/how-to/fixtures.html#fixtures-can-be-requested-more-than-once-per-test-return-values-are-cached
"""
- Test vulnerabilities files on guest with a custom CPU template.
- """
- check_vulnerabilities_files_ab(build_microvm_with_custom_template)
+ return uvm_ctor(microvm_factory_a, guest_kernel, rootfs, cpu_template_any)
-def test_vulnerabilities_files_on_restored_guest_with_template(
- build_microvm_with_template, microvm_factory
-):
- """
- Test vulnerabilities files on a restored guest with a CPU template.
- """
- check_vulnerabilities_files_ab(
- with_restore(build_microvm_with_template, microvm_factory)
- )
+def test_check_vulnerability_files_ab(request, uvm_any):
+ """Test vulnerability files on guests"""
+ res_b = check_vulnerabilities_files_on_guest(uvm_any)
+ if global_props.buildkite_pr:
+ # we only get the uvm_any_a fixtures if we need it
+ uvm_a = request.getfixturevalue("uvm_any_a")
+ res_a = check_vulnerabilities_files_on_guest(uvm_a)
+ assert res_b <= res_a
+ else:
+ assert not [x for x in res_b if "Vulnerable" in x["stdout"]]
-def test_vulnerabilities_files_on_restored_guest_with_custom_template(
- build_microvm_with_custom_template, microvm_factory
+def test_spectre_meltdown_checker_on_guest(
+ request,
+ uvm_any,
+ spectre_meltdown_checker,
):
- """
- Test vulnerabilities files on a restored guest with a custom CPU template.
- """
- check_vulnerabilities_files_ab(
- with_restore(build_microvm_with_custom_template, microvm_factory)
- )
+ """Test with the spectre / meltdown checker on any supported guest."""
+ res_b = spectre_meltdown_checker.get_report_for_guest(uvm_any)
+ if global_props.buildkite_pr:
+ # we only get the uvm_any_a fixtures if we need it
+ uvm_a = request.getfixturevalue("uvm_any_a")
+ res_a = spectre_meltdown_checker.get_report_for_guest(uvm_a)
+ assert res_b <= res_a
+ else:
+ assert res_b == spectre_meltdown_checker.expected_vulnerabilities(
+ uvm_any.cpu_template_name
+ )
From c34384ffb546d145b593f6a25fb0cd4c537b7f23 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= <pablob@amazon.com>
Date: Thu, 31 Oct 2024 22:49:26 +0100
Subject: [PATCH 13/23] tests: refactor test_nv.py to use uvm_any_booted
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Use a new uvm_any_booted fixture to make the test simpler.
Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
---
tests/conftest.py | 6 +++++
tests/integration_tests/security/test_nv.py | 30 +++------------------
2 files changed, 9 insertions(+), 27 deletions(-)
diff --git a/tests/conftest.py b/tests/conftest.py
index 5ad291d01a4..a2990a56c98 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -494,3 +494,9 @@ def uvm_ctor(request):
def uvm_any(microvm_factory, uvm_ctor, guest_kernel, rootfs, cpu_template_any):
"""Return booted and restored uvms"""
return uvm_ctor(microvm_factory, guest_kernel, rootfs, cpu_template_any)
+
+
+@pytest.fixture
+def uvm_any_booted(microvm_factory, guest_kernel, rootfs, cpu_template_any):
+ """Return booted uvms"""
+ return uvm_booted(microvm_factory, guest_kernel, rootfs, cpu_template_any)
diff --git a/tests/integration_tests/security/test_nv.py b/tests/integration_tests/security/test_nv.py
index 5dd5acb2308..73f042d5ae3 100644
--- a/tests/integration_tests/security/test_nv.py
+++ b/tests/integration_tests/security/test_nv.py
@@ -16,31 +16,7 @@
start providing the feature by mistake.
"""
-import pytest
-
-@pytest.fixture
-def uvm_with_cpu_template(microvm_factory, guest_kernel, rootfs, cpu_template_any):
- """A microvm fixture parametrized with all possible templates"""
- vm = microvm_factory.build(guest_kernel, rootfs)
- vm.spawn()
- cpu_template = None
- if isinstance(cpu_template_any, str):
- cpu_template = cpu_template_any
- vm.basic_config(cpu_template=cpu_template)
- if isinstance(cpu_template_any, dict):
- vm.api.cpu_config.put(**cpu_template_any["template"])
- vm.add_net_iface()
- vm.start()
- yield vm
-
-
-def test_no_nv_when_using_cpu_templates(uvm_with_cpu_template):
- """
- Double-check that guests using CPU templates don't have Nested Virtualization
- enabled.
- """
-
- vm = uvm_with_cpu_template
- rc, _, _ = vm.ssh.run("[ ! -e /dev/kvm ]")
- assert rc == 0, "/dev/kvm exists"
+def test_no_nested_virtualization(uvm_any_booted):
+ """Validate that guests don't have Nested Virtualization enabled."""
+ uvm_any_booted.ssh.check_output("[ ! -e /dev/kvm ]")
From 62fe7fd816de0442c9270e49e6dc5d16b1ecc281 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= <pablob@amazon.com>
Date: Tue, 29 Oct 2024 16:06:17 +0100
Subject: [PATCH 14/23] tests: refactor test_cpu_features_aarch64 to use
uvm_any
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Use the new uvm_any fixture to make the tests simpler
Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
---
tests/framework/utils_cpu_templates.py | 10 +-
.../functional/test_cpu_features_aarch64.py | 157 ++++--------------
2 files changed, 34 insertions(+), 133 deletions(-)
diff --git a/tests/framework/utils_cpu_templates.py b/tests/framework/utils_cpu_templates.py
index 89326c1da5a..56021a21aaa 100644
--- a/tests/framework/utils_cpu_templates.py
+++ b/tests/framework/utils_cpu_templates.py
@@ -22,9 +22,7 @@
def get_supported_cpu_templates():
- """
- Return the list of CPU templates supported by the platform.
- """
+ """Return the list of static CPU templates supported by the platform."""
host_linux = global_props.host_linux_version_tpl
match get_cpu_vendor(), global_props.cpu_codename:
# T2CL template is only supported on Cascade Lake and newer CPUs.
@@ -44,12 +42,8 @@ def get_supported_cpu_templates():
def get_supported_custom_cpu_templates():
- """
- Return the list of custom CPU templates supported by the platform.
- """
- # pylint:disable=too-many-return-statements
+ """Return the list of custom CPU templates supported by the platform."""
host_linux = global_props.host_linux_version_tpl
-
match get_cpu_vendor(), global_props.cpu_codename:
# T2CL template is only supported on Cascade Lake and newer CPUs.
case CpuVendor.INTEL, CpuModel.INTEL_SKYLAKE:
diff --git a/tests/integration_tests/functional/test_cpu_features_aarch64.py b/tests/integration_tests/functional/test_cpu_features_aarch64.py
index 8357f54b568..1edcb39f151 100644
--- a/tests/integration_tests/functional/test_cpu_features_aarch64.py
+++ b/tests/integration_tests/functional/test_cpu_features_aarch64.py
@@ -3,83 +3,63 @@
"""Tests for the CPU features for aarch64."""
import os
-import platform
-import re
import pytest
-import framework.utils_cpuid as cpuid_utils
from framework import utils
from framework.properties import global_props
from framework.utils_cpuid import CPU_FEATURES_CMD, CpuModel
-PLATFORM = platform.machine()
+pytestmark = pytest.mark.skipif(
+ global_props.cpu_architecture != "aarch64", reason="Only run in aarch64"
+)
-DEFAULT_G2_FEATURES = set(
+G2_FEATS = set(
(
"fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp "
"asimdhp cpuid asimdrdm lrcpc dcpop asimddp ssbs"
- ).split(" ")
+ ).split()
)
-DEFAULT_G3_FEATURES_5_10 = DEFAULT_G2_FEATURES | set(
- "sha512 asimdfhm dit uscat ilrcpc flagm jscvt fcma sha3 sm3 sm4 rng dcpodp i8mm bf16 dgh".split(
- " "
- )
+G3_FEATS = G2_FEATS | set(
+ "sha512 asimdfhm dit uscat ilrcpc flagm jscvt fcma sha3 sm3 sm4 rng dcpodp i8mm bf16 dgh".split()
)
-DEFAULT_G3_FEATURES_WITH_SVE_AND_PAC_5_10 = DEFAULT_G3_FEATURES_5_10 | set(
- "paca pacg sve svebf16 svei8mm".split(" ")
-)
+G3_SVE_AND_PAC = set("paca pacg sve svebf16 svei8mm".split())
-DEFAULT_G3_FEATURES_V1N1 = DEFAULT_G2_FEATURES
+def test_guest_cpu_features(uvm_any):
+ """Check the CPU features for a microvm with different CPU templates"""
-def _check_cpu_features_arm(test_microvm, guest_kv, template_name=None):
- expected_cpu_features = {"Flags": []}
- match cpuid_utils.get_cpu_model_name(), guest_kv, template_name:
- case CpuModel.ARM_NEOVERSE_N1, _, "v1n1":
- expected_cpu_features = DEFAULT_G2_FEATURES
- case CpuModel.ARM_NEOVERSE_N1, _, None:
- expected_cpu_features = DEFAULT_G2_FEATURES
+ vm = uvm_any
+ expected_cpu_features = set()
+ match global_props.cpu_model, vm.cpu_template_name:
+ case CpuModel.ARM_NEOVERSE_N1, "v1n1":
+ expected_cpu_features = G2_FEATS
+ case CpuModel.ARM_NEOVERSE_N1, None:
+ expected_cpu_features = G2_FEATS
# [cm]7g with guest kernel 5.10 and later
- case CpuModel.ARM_NEOVERSE_V1, _, "v1n1":
- expected_cpu_features = DEFAULT_G3_FEATURES_V1N1
- case CpuModel.ARM_NEOVERSE_V1, _, "aarch64_with_sve_and_pac":
- expected_cpu_features = DEFAULT_G3_FEATURES_WITH_SVE_AND_PAC_5_10
- case CpuModel.ARM_NEOVERSE_V1, _, None:
- expected_cpu_features = DEFAULT_G3_FEATURES_5_10
-
- _, stdout, _ = test_microvm.ssh.check_output(CPU_FEATURES_CMD)
- flags = set(stdout.strip().split(" "))
- assert flags == expected_cpu_features
+ case CpuModel.ARM_NEOVERSE_V1, "v1n1":
+ expected_cpu_features = G2_FEATS
+ case CpuModel.ARM_NEOVERSE_V1, "aarch64_with_sve_and_pac":
+ expected_cpu_features = G3_FEATS | G3_SVE_AND_PAC
+ case CpuModel.ARM_NEOVERSE_V1, None:
+ expected_cpu_features = G3_FEATS
+ guest_feats = set(vm.ssh.check_output(CPU_FEATURES_CMD).stdout.split())
+ assert guest_feats == expected_cpu_features
-def get_cpu_template_dir(cpu_template):
- """
- Utility function to return a valid string which will be used as
- name of the directory where snapshot artifacts are stored during
- snapshot test and loaded from during restore test.
- """
- return cpu_template if cpu_template else "none"
-
-
-@pytest.mark.skipif(
- PLATFORM != "aarch64",
- reason="This is aarch64 specific test.",
-)
-def test_host_vs_guest_cpu_features_aarch64(uvm_nano):
+def test_host_vs_guest_cpu_features(uvm_nano):
"""Check CPU features host vs guest"""
vm = uvm_nano
vm.add_net_iface()
vm.start()
- host_feats = set(utils.check_output(CPU_FEATURES_CMD).stdout.strip().split(" "))
- guest_feats = set(vm.ssh.check_output(CPU_FEATURES_CMD).stdout.strip().split(" "))
-
- cpu_model = cpuid_utils.get_cpu_model_name()
+ host_feats = set(utils.check_output(CPU_FEATURES_CMD).stdout.split())
+ guest_feats = set(vm.ssh.check_output(CPU_FEATURES_CMD).stdout.split())
+ cpu_model = global_props.cpu_model
match cpu_model:
case CpuModel.ARM_NEOVERSE_N1:
expected_guest_minus_host = set()
@@ -141,79 +121,6 @@ def test_host_vs_guest_cpu_features_aarch64(uvm_nano):
assert guest_feats - host_feats == expected_guest_minus_host
case _:
if os.environ.get("BUILDKITE") is not None:
- assert False, f"Cpu model {cpu_model} is not supported"
-
-
-@pytest.mark.skipif(
- PLATFORM != "aarch64",
- reason="This is aarch64 specific test.",
-)
-def test_default_cpu_features(microvm_factory, guest_kernel, rootfs):
- """
- Check the CPU features for a microvm with the specified config.
- """
-
- vm = microvm_factory.build(guest_kernel, rootfs, monitor_memory=False)
- vm.spawn()
- vm.basic_config()
- vm.add_net_iface()
- vm.start()
- guest_kv = re.search(r"vmlinux-(\d+\.\d+)", guest_kernel.name).group(1)
- _check_cpu_features_arm(vm, guest_kv)
-
-
-@pytest.mark.skipif(
- PLATFORM != "aarch64",
- reason="This is aarch64 specific test.",
-)
-def test_cpu_features_with_static_template(
- microvm_factory, guest_kernel, rootfs, cpu_template
-):
- """
- Check the CPU features for a microvm with the specified config.
- """
-
- vm = microvm_factory.build(guest_kernel, rootfs, monitor_memory=False)
- vm.spawn()
- vm.basic_config(cpu_template=cpu_template)
- vm.add_net_iface()
- vm.start()
- guest_kv = re.search(r"vmlinux-(\d+\.\d+)", guest_kernel.name).group(1)
- _check_cpu_features_arm(vm, guest_kv, "v1n1")
-
- # Check that cpu features are still correct
- # after snap/restore cycle.
- snapshot = vm.snapshot_full()
- restored_vm = microvm_factory.build()
- restored_vm.spawn()
- restored_vm.restore_from_snapshot(snapshot, resume=True)
- _check_cpu_features_arm(restored_vm, guest_kv, "v1n1")
-
-
-@pytest.mark.skipif(
- PLATFORM != "aarch64",
- reason="This is aarch64 specific test.",
-)
-def test_cpu_features_with_custom_template(
- microvm_factory, guest_kernel, rootfs, custom_cpu_template
-):
- """
- Check the CPU features for a microvm with the specified config.
- """
-
- vm = microvm_factory.build(guest_kernel, rootfs, monitor_memory=False)
- vm.spawn()
- vm.basic_config()
- vm.api.cpu_config.put(**custom_cpu_template["template"])
- vm.add_net_iface()
- vm.start()
- guest_kv = re.search(r"vmlinux-(\d+\.\d+)", guest_kernel.name).group(1)
- _check_cpu_features_arm(vm, guest_kv, custom_cpu_template["name"])
-
- # Check that cpu features are still correct
- # after snap/restore cycle.
- snapshot = vm.snapshot_full()
- restored_vm = microvm_factory.build()
- restored_vm.spawn()
- restored_vm.restore_from_snapshot(snapshot, resume=True)
- _check_cpu_features_arm(restored_vm, guest_kv, custom_cpu_template["name"])
+ assert (
+ False
+ ), f"Cpu model {cpu_model} is not supported, please onboard it."
From fe3d7553d0061646f28793593ffc946ea512c3a6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= <pablob@amazon.com>
Date: Mon, 18 Nov 2024 22:31:29 +0100
Subject: [PATCH 15/23] tests: skip x86_64 tests at module level
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
All tests in the file are x86_64 specific, so do the skip at the
top-level, once.
Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
---
.../functional/test_cpu_features_x86_64.py | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/tests/integration_tests/functional/test_cpu_features_x86_64.py b/tests/integration_tests/functional/test_cpu_features_x86_64.py
index 84d84bd8659..bb86df2eade 100644
--- a/tests/integration_tests/functional/test_cpu_features_x86_64.py
+++ b/tests/integration_tests/functional/test_cpu_features_x86_64.py
@@ -30,6 +30,10 @@
)
DATA_FILES = Path("./data/msr")
+pytestmark = pytest.mark.skipif(
+ global_props.cpu_architecture != "x86_64", reason="Only run in x86_64"
+)
+
def read_msr_csv(fd):
"""Read a CSV of MSRs"""
@@ -105,7 +109,6 @@ def skip_test_based_on_artifacts(snapshot_artifacts_dir):
pytest.skip(re.sub(" +", " ", reason))
-@pytest.mark.skipif(PLATFORM != "x86_64", reason="CPUID is only supported on x86_64.")
@pytest.mark.parametrize(
"num_vcpus",
[1, 2, 16],
@@ -126,7 +129,6 @@ def test_cpuid(uvm_plain_any, num_vcpus, htt):
_check_cpuid_x86(vm, num_vcpus, "true" if num_vcpus > 1 else "false")
-@pytest.mark.skipif(PLATFORM != "x86_64", reason="CPUID is only supported on x86_64.")
@pytest.mark.skipif(
cpuid_utils.get_cpu_vendor() != cpuid_utils.CpuVendor.AMD,
reason="L3 cache info is only present in 0x80000006 for AMD",
@@ -143,9 +145,6 @@ def test_extended_cache_features(uvm_plain_any):
_check_extended_cache_features(vm)
-@pytest.mark.skipif(
- PLATFORM != "x86_64", reason="The CPU brand string is masked only on x86_64."
-)
def test_brand_string(uvm_plain_any):
"""
Ensure good formatting for the guest brand string.
@@ -204,10 +203,6 @@ def test_brand_string(uvm_plain_any):
assert False
-@pytest.mark.skipif(
- PLATFORM != "x86_64",
- reason="This is x86_64 specific test.",
-)
def test_host_vs_guest_cpu_features_x86_64(uvm_nano):
"""Check CPU features host vs guest"""
@@ -929,9 +924,6 @@ def test_cpu_cpuid_restore(microvm_factory, guest_kernel, msr_cpu_template):
)
-@pytest.mark.skipif(
- PLATFORM != "x86_64", reason="CPU features are masked only on x86_64."
-)
@pytest.mark.parametrize("cpu_template", ["T2", "T2S", "C3"])
def test_cpu_template(uvm_plain_any, cpu_template, microvm_factory):
"""
@@ -1249,7 +1241,6 @@ def check_enabled_features(test_microvm, cpu_template):
)
-@pytest.mark.skipif(PLATFORM != "x86_64", reason="This test is specific to x86_64.")
def test_c3_on_skylake_show_warning(uvm_plain, cpu_template):
"""
This test verifies that the warning message about MMIO stale data mitigation
From bfeb1e46ccce1f580fcda0d397f914eae34da662 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= <pablob@amazon.com>
Date: Mon, 18 Nov 2024 22:32:55 +0100
Subject: [PATCH 16/23] tests: refactor test_cpu_features_x86_64.py
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Make the common feature flags to get some clarity on what are the
differences between the CPUs.
Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
---
.../functional/test_cpu_features_x86_64.py | 203 +++++-------------
1 file changed, 56 insertions(+), 147 deletions(-)
diff --git a/tests/integration_tests/functional/test_cpu_features_x86_64.py b/tests/integration_tests/functional/test_cpu_features_x86_64.py
index bb86df2eade..f8bbe1ef52e 100644
--- a/tests/integration_tests/functional/test_cpu_features_x86_64.py
+++ b/tests/integration_tests/functional/test_cpu_features_x86_64.py
@@ -203,6 +203,57 @@ def test_brand_string(uvm_plain_any):
assert False
+INTEL_HOST_ONLY_FEATS = {
+ "acpi",
+ "aperfmperf",
+ "arch_perfmon",
+ "art",
+ "bts",
+ "cat_l3",
+ "cdp_l3",
+ "cqm",
+ "cqm_llc",
+ "cqm_mbm_local",
+ "cqm_mbm_total",
+ "cqm_occup_llc",
+ "dca",
+ "ds_cpl",
+ "dtes64",
+ "dtherm",
+ "dts",
+ "epb",
+ "ept",
+ "ept_ad",
+ "est",
+ "flexpriority",
+ "flush_l1d",
+ "hwp",
+ "hwp_act_window",
+ "hwp_epp",
+ "hwp_pkg_req",
+ "ida",
+ "intel_ppin",
+ "intel_pt",
+ "mba",
+ "monitor",
+ "pbe",
+ "pdcm",
+ "pebs",
+ "pln",
+ "pts",
+ "rdt_a",
+ "sdbg",
+ "smx",
+ "tm",
+ "tm2",
+ "tpr_shadow",
+ "vmx",
+ "vnmi",
+ "vpid",
+ "xtpr",
+}
+
+
def test_host_vs_guest_cpu_features_x86_64(uvm_nano):
"""Check CPU features host vs guest"""
@@ -283,110 +334,14 @@ def test_host_vs_guest_cpu_features_x86_64(uvm_nano):
"tsc_known_freq",
}
case CpuModel.INTEL_SKYLAKE:
- assert host_feats - guest_feats == {
- "acpi",
- "aperfmperf",
- "arch_perfmon",
- "art",
- "bts",
- "cat_l3",
- "cdp_l3",
- "cqm",
- "cqm_llc",
- "cqm_mbm_local",
- "cqm_mbm_total",
- "cqm_occup_llc",
- "dca",
- "ds_cpl",
- "dtes64",
- "dtherm",
- "dts",
- "epb",
- "ept",
- "ept_ad",
- "est",
- "flexpriority",
- "flush_l1d",
- "hwp",
- "hwp_act_window",
- "hwp_epp",
- "hwp_pkg_req",
- "ida",
- "intel_ppin",
- "intel_pt",
- "mba",
- "monitor",
- "pbe",
- "pdcm",
- "pebs",
- "pln",
- "pts",
- "rdt_a",
- "sdbg",
- "smx",
- "tm",
- "tm2",
- "tpr_shadow",
- "vmx",
- "vnmi",
- "vpid",
- "xtpr",
- }
+ assert host_feats - guest_feats == INTEL_HOST_ONLY_FEATS
assert guest_feats - host_feats == {
"hypervisor",
"tsc_known_freq",
"umip",
}
case CpuModel.INTEL_CASCADELAKE:
- expected_host_minus_guest = {
- "acpi",
- "aperfmperf",
- "arch_perfmon",
- "art",
- "bts",
- "cat_l3",
- "cdp_l3",
- "cqm",
- "cqm_llc",
- "cqm_mbm_local",
- "cqm_mbm_total",
- "cqm_occup_llc",
- "dca",
- "ds_cpl",
- "dtes64",
- "dtherm",
- "dts",
- "epb",
- "ept",
- "ept_ad",
- "est",
- "flexpriority",
- "flush_l1d",
- "hwp",
- "hwp_act_window",
- "hwp_epp",
- "hwp_pkg_req",
- "ida",
- "intel_ppin",
- "intel_pt",
- "mba",
- "monitor",
- "pbe",
- "pdcm",
- "pebs",
- "pln",
- "pts",
- "rdt_a",
- "sdbg",
- "smx",
- "tm",
- "tm2",
- "tpr_shadow",
- "vmx",
- "vnmi",
- "vpid",
- "xtpr",
- }
+ expected_host_minus_guest = INTEL_HOST_ONLY_FEATS
expected_guest_minus_host = {
"hypervisor",
"tsc_known_freq",
@@ -414,56 +369,10 @@ def test_host_vs_guest_cpu_features_x86_64(uvm_nano):
assert host_feats - guest_feats == expected_host_minus_guest
assert guest_feats - host_feats == expected_guest_minus_host
case CpuModel.INTEL_ICELAKE:
- host_guest_diff_5_10 = {
- "dtes64",
- "hwp_act_window",
- "pdcm",
- "acpi",
- "aperfmperf",
- "arch_perfmon",
- "art",
- "bts",
- "cat_l3",
- "cqm",
- "cqm_llc",
- "cqm_mbm_local",
- "cqm_mbm_total",
- "cqm_occup_llc",
- "dca",
- "ds_cpl",
- "dtherm",
- "dts",
- "epb",
- "ept",
- "ept_ad",
- "est",
- "flexpriority",
- "flush_l1d",
- "hwp",
- "hwp_epp",
- "hwp_pkg_req",
- "ida",
- "intel_ppin",
- "intel_pt",
- "mba",
- "monitor",
- "pbe",
+ host_guest_diff_5_10 = INTEL_HOST_ONLY_FEATS - {"cdp_l3"} | {
"pconfig",
- "pebs",
- "pln",
- "pts",
- "rdt_a",
- "sdbg",
- "smx",
- "split_lock_detect",
- "tm",
- "tm2",
"tme",
- "tpr_shadow",
- "vmx",
- "vnmi",
- "vpid",
- "xtpr",
+ "split_lock_detect",
}
host_guest_diff_6_1 = host_guest_diff_5_10 - {
"bts",
@@ -1261,7 +1170,7 @@ def test_c3_on_skylake_show_warning(uvm_plain, cpu_template):
"does not apply the mitigation against MMIO stale data "
"vulnerability."
)
- if cpu_template == "C3" and global_props.cpu_codename == "INTEL_SKYLAKE":
+ if uvm.cpu_template_name == "c3" and global_props.cpu_codename == "INTEL_SKYLAKE":
assert message in uvm.log_data
else:
assert message not in uvm.log_data
From 14948da8efa70ada2f41caf986b7ce78a103e62f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= <pablob@amazon.com>
Date: Mon, 18 Nov 2024 23:10:09 +0100
Subject: [PATCH 17/23] tests: move host vs guest cpu features into a single
test
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This is the same test spread over two files. Putting it together in a
new file so they don't drift over time.
Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
---
.../functional/test_cpu_features_aarch64.py | 78 -----
.../test_cpu_features_host_vs_guest.py | 272 ++++++++++++++++++
.../functional/test_cpu_features_x86_64.py | 198 -------------
3 files changed, 272 insertions(+), 276 deletions(-)
create mode 100644 tests/integration_tests/functional/test_cpu_features_host_vs_guest.py
diff --git a/tests/integration_tests/functional/test_cpu_features_aarch64.py b/tests/integration_tests/functional/test_cpu_features_aarch64.py
index 1edcb39f151..2068194a894 100644
--- a/tests/integration_tests/functional/test_cpu_features_aarch64.py
+++ b/tests/integration_tests/functional/test_cpu_features_aarch64.py
@@ -2,11 +2,8 @@
# SPDX-License-Identifier: Apache-2.0
"""Tests for the CPU features for aarch64."""
-import os
-
import pytest
-from framework import utils
from framework.properties import global_props
from framework.utils_cpuid import CPU_FEATURES_CMD, CpuModel
@@ -49,78 +46,3 @@ def test_guest_cpu_features(uvm_any):
guest_feats = set(vm.ssh.check_output(CPU_FEATURES_CMD).stdout.split())
assert guest_feats == expected_cpu_features
-
-
-def test_host_vs_guest_cpu_features(uvm_nano):
- """Check CPU features host vs guest"""
-
- vm = uvm_nano
- vm.add_net_iface()
- vm.start()
- host_feats = set(utils.check_output(CPU_FEATURES_CMD).stdout.split())
- guest_feats = set(vm.ssh.check_output(CPU_FEATURES_CMD).stdout.split())
- cpu_model = global_props.cpu_model
- match cpu_model:
- case CpuModel.ARM_NEOVERSE_N1:
- expected_guest_minus_host = set()
- expected_host_minus_guest = set()
-
- # Upstream kernel v6.11+ hides "ssbs" from "lscpu" on Neoverse-N1 and Neoverse-V1 since
- # they have an errata whereby an MSR to the SSBS special-purpose register does not
- # affect subsequent speculative instructions, permitting speculative store bypassing for
- # a window of time.
- # https://github.com/torvalds/linux/commit/adeec61a4723fd3e39da68db4cc4d924e6d7f641
- #
- # While Amazon Linux kernels (v5.10 and v6.1) backported the above commit, our test
- # ubuntu kernel (v6.8) and our guest kernels (v5.10 and v6.1) don't pick it.
- host_has_ssbs = global_props.host_os not in {
- "amzn2",
- "amzn2023",
- } and global_props.host_linux_version_tpl < (6, 11)
- guest_has_ssbs = vm.guest_kernel_version < (6, 11)
-
- if host_has_ssbs and not guest_has_ssbs:
- expected_host_minus_guest |= {"ssbs"}
- if not host_has_ssbs and guest_has_ssbs:
- expected_guest_minus_host |= {"ssbs"}
-
- assert host_feats - guest_feats == expected_host_minus_guest
- assert guest_feats - host_feats == expected_guest_minus_host
- case CpuModel.ARM_NEOVERSE_V1:
- expected_guest_minus_host = set()
- # KVM does not enable PAC or SVE features by default
- # and Firecracker does not enable them either.
- expected_host_minus_guest = {
- "paca",
- "pacg",
- "sve",
- "svebf16",
- "svei8mm",
- }
-
- # Upstream kernel v6.11+ hides "ssbs" from "lscpu" on Neoverse-N1 and Neoverse-V1 since
- # they have an errata whereby an MSR to the SSBS special-purpose register does not
- # affect subsequent speculative instructions, permitting speculative store bypassing for
- # a window of time.
- # https://github.com/torvalds/linux/commit/adeec61a4723fd3e39da68db4cc4d924e6d7f641
- #
- # While Amazon Linux kernels (v5.10 and v6.1) backported the above commit, our test
- # ubuntu kernel (v6.8) and our guest kernels (v5.10 and v6.1) don't pick it.
- host_has_ssbs = global_props.host_os not in {
- "amzn2",
- "amzn2023",
- } and global_props.host_linux_version_tpl < (6, 11)
- guest_has_ssbs = vm.guest_kernel_version < (6, 11)
-
- if host_has_ssbs and not guest_has_ssbs:
- expected_host_minus_guest |= {"ssbs"}
- if not host_has_ssbs and guest_has_ssbs:
- expected_guest_minus_host |= {"ssbs"}
-
- assert host_feats - guest_feats == expected_host_minus_guest
- assert guest_feats - host_feats == expected_guest_minus_host
- case _:
- if os.environ.get("BUILDKITE") is not None:
- assert (
- False
- ), f"Cpu model {cpu_model} is not supported, please onboard it."
diff --git a/tests/integration_tests/functional/test_cpu_features_host_vs_guest.py b/tests/integration_tests/functional/test_cpu_features_host_vs_guest.py
new file mode 100644
index 00000000000..bbf4ed57d21
--- /dev/null
+++ b/tests/integration_tests/functional/test_cpu_features_host_vs_guest.py
@@ -0,0 +1,272 @@
+# Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+# SPDX-License-Identifier: Apache-2.0
+
+# pylint: disable=too-many-statements
+
+"""
+Check CPU features in the host vs the guest.
+
+This test can highlight differences between the host and what the guest sees.
+
+No CPU templates as we are interested only on what is passed through to the guest by default.
+For that, check test_feat_parity.py
+"""
+
+import os
+
+from framework import utils
+from framework.properties import global_props
+from framework.utils_cpuid import CPU_FEATURES_CMD, CpuModel
+
+CPU_MODEL = global_props.cpu_codename
+
+INTEL_HOST_ONLY_FEATS = {
+ "acpi",
+ "aperfmperf",
+ "arch_perfmon",
+ "art",
+ "bts",
+ "cat_l3",
+ "cdp_l3",
+ "cqm",
+ "cqm_llc",
+ "cqm_mbm_local",
+ "cqm_mbm_total",
+ "cqm_occup_llc",
+ "dca",
+ "ds_cpl",
+ "dtes64",
+ "dtherm",
+ "dts",
+ "epb",
+ "ept",
+ "ept_ad",
+ "est",
+ "flexpriority",
+ "flush_l1d",
+ "hwp",
+ "hwp_act_window",
+ "hwp_epp",
+ "hwp_pkg_req",
+ "ida",
+ "intel_ppin",
+ "intel_pt",
+ "mba",
+ "monitor",
+ "pbe",
+ "pdcm",
+ "pebs",
+ "pln",
+ "pts",
+ "rdt_a",
+ "sdbg",
+ "smx",
+ "tm",
+ "tm2",
+ "tpr_shadow",
+ "vmx",
+ "vnmi",
+ "vpid",
+ "xtpr",
+}
+
+
+def test_host_vs_guest_cpu_features(uvm_nano):
+ """Check CPU features host vs guest"""
+
+ vm = uvm_nano
+ vm.add_net_iface()
+ vm.start()
+ host_feats = set(utils.check_output(CPU_FEATURES_CMD).stdout.split())
+ guest_feats = set(vm.ssh.check_output(CPU_FEATURES_CMD).stdout.split())
+
+ match CPU_MODEL:
+ case CpuModel.AMD_MILAN:
+ host_guest_diff_5_10 = {
+ "amd_ppin",
+ "aperfmperf",
+ "bpext",
+ "cat_l3",
+ "cdp_l3",
+ "cpb",
+ "cqm",
+ "cqm_llc",
+ "cqm_mbm_local",
+ "cqm_mbm_total",
+ "cqm_occup_llc",
+ "decodeassists",
+ "extapic",
+ "extd_apicid",
+ "flushbyasid",
+ "hw_pstate",
+ "ibs",
+ "irperf",
+ "lbrv",
+ "mba",
+ "monitor",
+ "mwaitx",
+ "overflow_recov",
+ "pausefilter",
+ "perfctr_llc",
+ "perfctr_nb",
+ "pfthreshold",
+ "rdpru",
+ "rdt_a",
+ "sev",
+ "sev_es",
+ "skinit",
+ "smca",
+ "sme",
+ "succor",
+ "svm_lock",
+ "tce",
+ "tsc_scale",
+ "v_vmsave_vmload",
+ "vgif",
+ "vmcb_clean",
+ "wdt",
+ }
+
+ host_guest_diff_6_1 = host_guest_diff_5_10 - {
+ "lbrv",
+ "pausefilter",
+ "pfthreshold",
+ "sme",
+ "tsc_scale",
+ "v_vmsave_vmload",
+ "vgif",
+ "vmcb_clean",
+ } | {"brs", "rapl", "v_spec_ctrl"}
+
+ if global_props.host_linux_version_tpl < (6, 1):
+ assert host_feats - guest_feats == host_guest_diff_5_10
+ else:
+ assert host_feats - guest_feats == host_guest_diff_6_1
+
+ assert guest_feats - host_feats == {
+ "hypervisor",
+ "tsc_adjust",
+ "tsc_deadline_timer",
+ "tsc_known_freq",
+ }
+
+ case CpuModel.INTEL_SKYLAKE:
+ assert host_feats - guest_feats == INTEL_HOST_ONLY_FEATS
+ assert guest_feats - host_feats == {
+ "hypervisor",
+ "tsc_known_freq",
+ "umip",
+ }
+
+ case CpuModel.INTEL_CASCADELAKE:
+ expected_host_minus_guest = INTEL_HOST_ONLY_FEATS
+ expected_guest_minus_host = {
+ "hypervisor",
+ "tsc_known_freq",
+ "umip",
+ }
+
+ # Linux kernel v6.4+ passes through the CPUID bit for "flush_l1d" to guests.
+ # https://github.com/torvalds/linux/commit/45cf86f26148e549c5ba4a8ab32a390e4bde216e
+ #
+ # Our test ubuntu host kernel is v6.8 and has the commit.
+ if global_props.host_linux_version_tpl >= (6, 4):
+ expected_host_minus_guest -= {"flush_l1d"}
+
+ # Linux kernel v6.6+ drops the "invpcid_single" synthetic feature bit.
+ # https://github.com/torvalds/linux/commit/54e3d9434ef61b97fd3263c141b928dc5635e50d
+ #
+ # Our test ubuntu host kernel is v6.8 and has the commit.
+ host_has_invpcid_single = global_props.host_linux_version_tpl < (6, 6)
+ guest_has_invpcid_single = vm.guest_kernel_version < (6, 6)
+ if host_has_invpcid_single and not guest_has_invpcid_single:
+ expected_host_minus_guest |= {"invpcid_single"}
+ if not host_has_invpcid_single and guest_has_invpcid_single:
+ expected_guest_minus_host |= {"invpcid_single"}
+
+ assert host_feats - guest_feats == expected_host_minus_guest
+ assert guest_feats - host_feats == expected_guest_minus_host
+
+ case CpuModel.INTEL_ICELAKE:
+ host_guest_diff_5_10 = INTEL_HOST_ONLY_FEATS - {"cdp_l3"} | {
+ "pconfig",
+ "tme",
+ "split_lock_detect",
+ }
+ host_guest_diff_6_1 = host_guest_diff_5_10 - {
+ "bts",
+ "dtes64",
+ "dts",
+ "pebs",
+ }
+
+ if global_props.host_linux_version_tpl < (6, 1):
+ assert host_feats - guest_feats == host_guest_diff_5_10
+ else:
+ assert host_feats - guest_feats == host_guest_diff_6_1
+
+ assert guest_feats - host_feats == {
+ "hypervisor",
+ "tsc_known_freq",
+ }
+
+ case CpuModel.ARM_NEOVERSE_N1:
+ expected_guest_minus_host = set()
+ expected_host_minus_guest = set()
+
+ # Upstream kernel v6.11+ hides "ssbs" from "lscpu" on Neoverse-N1 and Neoverse-V1 since
+ # they have an errata whereby an MSR to the SSBS special-purpose register does not
+ # affect subsequent speculative instructions, permitting speculative store bypassing for
+ # a window of time.
+ # https://github.com/torvalds/linux/commit/adeec61a4723fd3e39da68db4cc4d924e6d7f641
+ #
+ # While Amazon Linux kernels (v5.10 and v6.1) backported the above commit, our test
+ # ubuntu kernel (v6.8) and our guest kernels (v5.10 and v6.1) don't pick it.
+ host_has_ssbs = global_props.host_os not in {
+ "amzn2",
+ "amzn2023",
+ } and global_props.host_linux_version_tpl < (6, 11)
+ guest_has_ssbs = vm.guest_kernel_version < (6, 11)
+
+ if host_has_ssbs and not guest_has_ssbs:
+ expected_host_minus_guest |= {"ssbs"}
+ if not host_has_ssbs and guest_has_ssbs:
+ expected_guest_minus_host |= {"ssbs"}
+
+ assert host_feats - guest_feats == expected_host_minus_guest
+ assert guest_feats - host_feats == expected_guest_minus_host
+
+ case CpuModel.ARM_NEOVERSE_V1:
+ expected_guest_minus_host = set()
+ # KVM does not enable PAC or SVE features by default
+ # and Firecracker does not enable them either.
+ expected_host_minus_guest = {"paca", "pacg", "sve", "svebf16", "svei8mm"}
+
+ # Upstream kernel v6.11+ hides "ssbs" from "lscpu" on Neoverse-N1 and Neoverse-V1 since
+ # they have an errata whereby an MSR to the SSBS special-purpose register does not
+ # affect subsequent speculative instructions, permitting speculative store bypassing for
+ # a window of time.
+ # https://github.com/torvalds/linux/commit/adeec61a4723fd3e39da68db4cc4d924e6d7f641
+ #
+ # While Amazon Linux kernels (v5.10 and v6.1) backported the above commit, our test
+ # ubuntu kernel (v6.8) and our guest kernels (v5.10 and v6.1) don't pick it.
+ host_has_ssbs = global_props.host_os not in {
+ "amzn2",
+ "amzn2023",
+ } and global_props.host_linux_version_tpl < (6, 11)
+ guest_has_ssbs = vm.guest_kernel_version < (6, 11)
+
+ if host_has_ssbs and not guest_has_ssbs:
+ expected_host_minus_guest |= {"ssbs"}
+ if not host_has_ssbs and guest_has_ssbs:
+ expected_guest_minus_host |= {"ssbs"}
+
+ assert host_feats - guest_feats == expected_host_minus_guest
+ assert guest_feats - host_feats == expected_guest_minus_host
+
+ case _:
+ # only fail if running in CI
+ if os.environ.get("BUILDKITE") is not None:
+ assert (
+ guest_feats == host_feats
+ ), f"Cpu model {CPU_MODEL} is not supported"
diff --git a/tests/integration_tests/functional/test_cpu_features_x86_64.py b/tests/integration_tests/functional/test_cpu_features_x86_64.py
index f8bbe1ef52e..a966e253c46 100644
--- a/tests/integration_tests/functional/test_cpu_features_x86_64.py
+++ b/tests/integration_tests/functional/test_cpu_features_x86_64.py
@@ -22,7 +22,6 @@
from framework.defs import SUPPORTED_HOST_KERNELS
from framework.properties import global_props
from framework.utils_cpu_templates import SUPPORTED_CPU_TEMPLATES
-from framework.utils_cpuid import CPU_FEATURES_CMD, CpuModel
PLATFORM = platform.machine()
UNSUPPORTED_HOST_KERNEL = (
@@ -203,203 +202,6 @@ def test_brand_string(uvm_plain_any):
assert False
-INTEL_HOST_ONLY_FEATS = {
- "acpi",
- "aperfmperf",
- "arch_perfmon",
- "art",
- "bts",
- "cat_l3",
- "cdp_l3",
- "cqm",
- "cqm_llc",
- "cqm_mbm_local",
- "cqm_mbm_total",
- "cqm_occup_llc",
- "dca",
- "ds_cpl",
- "dtes64",
- "dtherm",
- "dts",
- "epb",
- "ept",
- "ept_ad",
- "est",
- "flexpriority",
- "flush_l1d",
- "hwp",
- "hwp_act_window",
- "hwp_epp",
- "hwp_pkg_req",
- "ida",
- "intel_ppin",
- "intel_pt",
- "mba",
- "monitor",
- "pbe",
- "pdcm",
- "pebs",
- "pln",
- "pts",
- "rdt_a",
- "sdbg",
- "smx",
- "tm",
- "tm2",
- "tpr_shadow",
- "vmx",
- "vnmi",
- "vpid",
- "xtpr",
-}
-
-
-def test_host_vs_guest_cpu_features_x86_64(uvm_nano):
- """Check CPU features host vs guest"""
-
- vm = uvm_nano
- vm.add_net_iface()
- vm.start()
- host_feats = set(utils.check_output(CPU_FEATURES_CMD).stdout.strip().split(" "))
- guest_feats = set(vm.ssh.check_output(CPU_FEATURES_CMD).stdout.strip().split(" "))
-
- cpu_model = cpuid_utils.get_cpu_codename()
- match cpu_model:
- case CpuModel.AMD_MILAN:
- host_guest_diff_5_10 = {
- "amd_ppin",
- "aperfmperf",
- "bpext",
- "cat_l3",
- "cdp_l3",
- "cpb",
- "cqm",
- "cqm_llc",
- "cqm_mbm_local",
- "cqm_mbm_total",
- "cqm_occup_llc",
- "decodeassists",
- "extapic",
- "extd_apicid",
- "flushbyasid",
- "hw_pstate",
- "ibs",
- "irperf",
- "lbrv",
- "mba",
- "monitor",
- "mwaitx",
- "overflow_recov",
- "pausefilter",
- "perfctr_llc",
- "perfctr_nb",
- "pfthreshold",
- "rdpru",
- "rdt_a",
- "sev",
- "sev_es",
- "skinit",
- "smca",
- "sme",
- "succor",
- "svm_lock",
- "tce",
- "tsc_scale",
- "v_vmsave_vmload",
- "vgif",
- "vmcb_clean",
- "wdt",
- }
-
- host_guest_diff_6_1 = host_guest_diff_5_10 - {
- "lbrv",
- "pausefilter",
- "pfthreshold",
- "sme",
- "tsc_scale",
- "v_vmsave_vmload",
- "vgif",
- "vmcb_clean",
- } | {"brs", "rapl", "v_spec_ctrl"}
-
- if global_props.host_linux_version_tpl < (6, 1):
- assert host_feats - guest_feats == host_guest_diff_5_10
- else:
- assert host_feats - guest_feats == host_guest_diff_6_1
-
- assert guest_feats - host_feats == {
- "hypervisor",
- "tsc_adjust",
- "tsc_deadline_timer",
- "tsc_known_freq",
- }
- case CpuModel.INTEL_SKYLAKE:
- assert host_feats - guest_feats == INTEL_HOST_ONLY_FEATS
- assert guest_feats - host_feats == {
- "hypervisor",
- "tsc_known_freq",
- "umip",
- }
- case CpuModel.INTEL_CASCADELAKE:
- expected_host_minus_guest = INTEL_HOST_ONLY_FEATS
- expected_guest_minus_host = {
- "hypervisor",
- "tsc_known_freq",
- "umip",
- }
-
- # Linux kernel v6.4+ passes through the CPUID bit for "flush_l1d" to guests.
- # https://github.com/torvalds/linux/commit/45cf86f26148e549c5ba4a8ab32a390e4bde216e
- #
- # Our test ubuntu host kernel is v6.8 and has the commit.
- if global_props.host_linux_version_tpl >= (6, 4):
- expected_host_minus_guest -= {"flush_l1d"}
-
- # Linux kernel v6.6+ drops the "invpcid_single" synthetic feature bit.
- # https://github.com/torvalds/linux/commit/54e3d9434ef61b97fd3263c141b928dc5635e50d
- #
- # Our test ubuntu host kernel is v6.8 and has the commit.
- host_has_invpcid_single = global_props.host_linux_version_tpl < (6, 6)
- guest_has_invpcid_single = vm.guest_kernel_version < (6, 6)
- if host_has_invpcid_single and not guest_has_invpcid_single:
- expected_host_minus_guest |= {"invpcid_single"}
- if not host_has_invpcid_single and guest_has_invpcid_single:
- expected_guest_minus_host |= {"invpcid_single"}
-
- assert host_feats - guest_feats == expected_host_minus_guest
- assert guest_feats - host_feats == expected_guest_minus_host
- case CpuModel.INTEL_ICELAKE:
- host_guest_diff_5_10 = INTEL_HOST_ONLY_FEATS - {"cdp_l3"} | {
- "pconfig",
- "tme",
- "split_lock_detect",
- }
- host_guest_diff_6_1 = host_guest_diff_5_10 - {
- "bts",
- "dtes64",
- "dts",
- "pebs",
- }
-
- if global_props.host_linux_version_tpl < (6, 1):
- assert host_feats - guest_feats == host_guest_diff_5_10
- else:
- assert host_feats - guest_feats == host_guest_diff_6_1
-
- assert guest_feats - host_feats == {
- "hypervisor",
- "tsc_known_freq",
- }
- case CpuModel.AMD_GENOA:
- # Return here to allow the test to pass until CPU features to enable are confirmed
- return
- case _:
- if os.environ.get("BUILDKITE") is not None:
- assert (
- guest_feats == host_feats
- ), f"Cpu model {cpu_model} is not supported"
-
-
# From the `Intel® 64 Architecture x2APIC Specification`
# (https://courses.cs.washington.edu/courses/cse451/24wi/documentation/x2apic.pdf):
# > The X2APIC MSRs cannot to be loaded and stored on VMX transitions. A VMX transition fails
From 741dd699f2ba9313fff166211c4742174b9c1096 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= <pablob@amazon.com>
Date: Sat, 23 Nov 2024 11:59:20 +0100
Subject: [PATCH 18/23] tests: add helper method
MicrovmFactory.build_from_snapshot
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This is a fairly common repeated pattern, so extract it into a method to
make writing tests simpler.
Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
---
tests/conftest.py | 4 +-
tests/framework/microvm.py | 7 +++
.../integration_tests/functional/test_api.py | 5 +-
.../functional/test_balloon.py | 4 +-
.../functional/test_cmd_line_parameters.py | 4 +-
.../functional/test_snapshot_basic.py | 52 ++++++-------------
.../functional/test_snapshot_editor.py | 4 +-
.../test_snapshot_not_losing_dirty_pages.py | 7 +--
.../functional/test_vsock.py | 9 +---
9 files changed, 31 insertions(+), 65 deletions(-)
diff --git a/tests/conftest.py b/tests/conftest.py
index a2990a56c98..7cacedb9918 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -477,9 +477,7 @@ def uvm_restored(microvm_factory, guest_kernel, rootfs, cpu_template):
uvm = uvm_booted(microvm_factory, guest_kernel, rootfs, cpu_template)
snapshot = uvm.snapshot_full()
uvm.kill()
- uvm2 = microvm_factory.build()
- uvm2.spawn()
- uvm2.restore_from_snapshot(snapshot, resume=True)
+ uvm2 = microvm_factory.build_from_snapshot(snapshot)
uvm2.cpu_template_name = uvm.cpu_template_name
return uvm2
diff --git a/tests/framework/microvm.py b/tests/framework/microvm.py
index 832f56a877e..572437c758b 100644
--- a/tests/framework/microvm.py
+++ b/tests/framework/microvm.py
@@ -1084,6 +1084,13 @@ def build(self, kernel=None, rootfs=None, **kwargs):
vm.ssh_key = ssh_key
return vm
+ def build_from_snapshot(self, snapshot: Snapshot):
+ """Build a microvm from a snapshot"""
+ vm = self.build()
+ vm.spawn()
+ vm.restore_from_snapshot(snapshot, resume=True)
+ return vm
+
def kill(self):
"""Clean up all built VMs"""
for vm in self.vms:
diff --git a/tests/integration_tests/functional/test_api.py b/tests/integration_tests/functional/test_api.py
index 27366529c39..1e54c7b4fb1 100644
--- a/tests/integration_tests/functional/test_api.py
+++ b/tests/integration_tests/functional/test_api.py
@@ -1166,10 +1166,7 @@ def test_get_full_config_after_restoring_snapshot(microvm_factory, uvm_nano):
]
snapshot = uvm_nano.snapshot_full()
- uvm2 = microvm_factory.build()
- uvm2.spawn()
- uvm2.restore_from_snapshot(snapshot, resume=True)
-
+ uvm2 = microvm_factory.build_from_snapshot(snapshot)
expected_cfg = setup_cfg.copy()
# We expect boot-source to be set with the following values
diff --git a/tests/integration_tests/functional/test_balloon.py b/tests/integration_tests/functional/test_balloon.py
index ee750dcac7d..2c59fd2e814 100644
--- a/tests/integration_tests/functional/test_balloon.py
+++ b/tests/integration_tests/functional/test_balloon.py
@@ -484,9 +484,7 @@ def test_balloon_snapshot(microvm_factory, guest_kernel, rootfs):
assert first_reading > second_reading
snapshot = vm.snapshot_full()
- microvm = microvm_factory.build()
- microvm.spawn()
- microvm.restore_from_snapshot(snapshot, resume=True)
+ microvm = microvm_factory.build_from_snapshot(snapshot)
# Get the firecracker from snapshot pid, and open an ssh connection.
firecracker_pid = microvm.firecracker_pid
diff --git a/tests/integration_tests/functional/test_cmd_line_parameters.py b/tests/integration_tests/functional/test_cmd_line_parameters.py
index 25e47a50e17..79af938b1f7 100644
--- a/tests/integration_tests/functional/test_cmd_line_parameters.py
+++ b/tests/integration_tests/functional/test_cmd_line_parameters.py
@@ -96,9 +96,7 @@ def test_cli_metrics_if_resume_no_metrics(uvm_plain, microvm_factory):
snapshot = uvm1.snapshot_full()
# When: restoring from the snapshot
- uvm2 = microvm_factory.build()
- uvm2.spawn()
- uvm2.restore_from_snapshot(snapshot)
+ uvm2 = microvm_factory.build_from_snapshot(snapshot)
# Then: the old metrics configuration does not exist
metrics2 = Path(uvm2.jailer.chroot_path()) / metrics_path.name
diff --git a/tests/integration_tests/functional/test_snapshot_basic.py b/tests/integration_tests/functional/test_snapshot_basic.py
index ac596440f67..4030bb4e981 100644
--- a/tests/integration_tests/functional/test_snapshot_basic.py
+++ b/tests/integration_tests/functional/test_snapshot_basic.py
@@ -50,39 +50,25 @@ def _get_guest_drive_size(ssh_connection, guest_dev_name="/dev/vdb"):
return lines[1].strip()
-def test_resume_after_restoration(uvm_nano, microvm_factory):
- """Tests snapshot is resumable after restoration.
+@pytest.mark.parametrize("resume_at_restore", [True, False])
+def test_resume(uvm_nano, microvm_factory, resume_at_restore):
+ """Tests snapshot is resumable at or after restoration.
- Check that a restored microVM is resumable by calling PATCH /vm with Resumed
- after PUT /snapshot/load with `resume_vm=False`.
+ Check that a restored microVM is resumable by either
+ a. PUT /snapshot/load with `resume_vm=False`, then calling PATCH /vm resume=True
+ b. PUT /snapshot/load with `resume_vm=True`
"""
vm = uvm_nano
vm.add_net_iface()
vm.start()
-
- snapshot = vm.snapshot_full()
-
- restored_vm = microvm_factory.build()
- restored_vm.spawn()
- restored_vm.restore_from_snapshot(snapshot)
- restored_vm.resume()
-
-
-def test_resume_at_restoration(uvm_nano, microvm_factory):
- """Tests snapshot is resumable at restoration.
-
- Check that a restored microVM is resumable by calling PUT /snapshot/load
- with `resume_vm=True`.
- """
- vm = uvm_nano
- vm.add_net_iface()
- vm.start()
-
snapshot = vm.snapshot_full()
-
restored_vm = microvm_factory.build()
restored_vm.spawn()
- restored_vm.restore_from_snapshot(snapshot, resume=True)
+ restored_vm.restore_from_snapshot(snapshot, resume=resume_at_restore)
+ if not resume_at_restore:
+ assert restored_vm.state == "Paused"
+ restored_vm.resume()
+ assert restored_vm.state == "Running"
def test_snapshot_current_version(uvm_nano):
@@ -228,9 +214,7 @@ def test_patch_drive_snapshot(uvm_nano, microvm_factory):
# Load snapshot in a new Firecracker microVM.
logger.info("Load snapshot, mem %s", snapshot.mem)
- vm = microvm_factory.build()
- vm.spawn()
- vm.restore_from_snapshot(snapshot, resume=True)
+ vm = microvm_factory.build_from_snapshot(snapshot)
# Attempt to connect to resumed microvm and verify the new microVM has the
# right scratch drive.
@@ -319,9 +303,7 @@ def test_negative_postload_api(uvm_plain, microvm_factory):
basevm.kill()
# Do not resume, just load, so we can still call APIs that work.
- microvm = microvm_factory.build()
- microvm.spawn()
- microvm.restore_from_snapshot(snapshot, resume=True)
+ microvm = microvm_factory.build_from_snapshot(snapshot)
fail_msg = "The requested operation is not supported after starting the microVM"
with pytest.raises(RuntimeError, match=fail_msg):
@@ -486,9 +468,7 @@ def test_diff_snapshot_overlay(guest_kernel, rootfs, microvm_factory):
assert not filecmp.cmp(merged_snapshot.mem, first_snapshot_backup, shallow=False)
- new_vm = microvm_factory.build()
- new_vm.spawn()
- new_vm.restore_from_snapshot(merged_snapshot, resume=True)
+ _ = microvm_factory.build_from_snapshot(merged_snapshot)
# Check that the restored VM works
@@ -510,9 +490,7 @@ def test_snapshot_overwrite_self(guest_kernel, rootfs, microvm_factory):
snapshot = base_vm.snapshot_full()
base_vm.kill()
- vm = microvm_factory.build()
- vm.spawn()
- vm.restore_from_snapshot(snapshot, resume=True)
+ vm = microvm_factory.build_from_snapshot(snapshot)
# When restoring a snapshot, vm.restore_from_snapshot first copies
# the memory file (inside of the jailer) to /mem.src
diff --git a/tests/integration_tests/functional/test_snapshot_editor.py b/tests/integration_tests/functional/test_snapshot_editor.py
index 4d466a441ce..9323695628c 100644
--- a/tests/integration_tests/functional/test_snapshot_editor.py
+++ b/tests/integration_tests/functional/test_snapshot_editor.py
@@ -68,6 +68,4 @@ def test_remove_regs(uvm_nano, microvm_factory):
assert MIDR_EL1 not in stdout
# test that we can restore from a snapshot
- new_vm = microvm_factory.build()
- new_vm.spawn()
- new_vm.restore_from_snapshot(snapshot, resume=True)
+ _ = microvm_factory.build_from_snapshot(snapshot)
diff --git a/tests/integration_tests/functional/test_snapshot_not_losing_dirty_pages.py b/tests/integration_tests/functional/test_snapshot_not_losing_dirty_pages.py
index 5584cfceac8..79366f13f0b 100644
--- a/tests/integration_tests/functional/test_snapshot_not_losing_dirty_pages.py
+++ b/tests/integration_tests/functional/test_snapshot_not_losing_dirty_pages.py
@@ -63,9 +63,6 @@ def test_diff_snapshot_works_after_error(
# Now there is enough space for it to work
snap2 = uvm.snapshot_diff()
-
- vm2 = microvm_factory.build()
- vm2.spawn()
- vm2.restore_from_snapshot(snap2, resume=True)
-
uvm.kill()
+
+ _vm2 = microvm_factory.build_from_snapshot(snap2)
diff --git a/tests/integration_tests/functional/test_vsock.py b/tests/integration_tests/functional/test_vsock.py
index 2d540b8f934..dfa02510b37 100644
--- a/tests/integration_tests/functional/test_vsock.py
+++ b/tests/integration_tests/functional/test_vsock.py
@@ -199,10 +199,7 @@ def test_vsock_transport_reset_h2g(
test_vm.kill()
# Load snapshot.
-
- vm2 = microvm_factory.build()
- vm2.spawn()
- vm2.restore_from_snapshot(snapshot, resume=True)
+ vm2 = microvm_factory.build_from_snapshot(snapshot)
# Check that vsock device still works.
# Test guest-initiated connections.
@@ -231,9 +228,7 @@ def test_vsock_transport_reset_g2h(uvm_nano, microvm_factory):
for _ in range(5):
# Load snapshot.
- new_vm = microvm_factory.build()
- new_vm.spawn()
- new_vm.restore_from_snapshot(snapshot, resume=True)
+ new_vm = microvm_factory.build_from_snapshot(snapshot)
# After snap restore all vsock connections should be
# dropped. This means guest socat should exit same way
From 6da314d3276d88b63d6e5d64e39f931f38f836e1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= <pablob@amazon.com>
Date: Sat, 23 Nov 2024 12:03:31 +0100
Subject: [PATCH 19/23] tests: refactor test_rng with uvm_any
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Rewrite test_rng using uvm_any.
Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
---
.../integration_tests/functional/test_rng.py | 68 ++++++++++---------
1 file changed, 37 insertions(+), 31 deletions(-)
diff --git a/tests/integration_tests/functional/test_rng.py b/tests/integration_tests/functional/test_rng.py
index b40aa66033d..1893230c51a 100644
--- a/tests/integration_tests/functional/test_rng.py
+++ b/tests/integration_tests/functional/test_rng.py
@@ -8,11 +8,9 @@
from host_tools.network import SSHConnection
-@pytest.fixture(params=[None])
-def uvm_with_rng(uvm_plain, request):
- """Fixture of a microvm with virtio-rng configured"""
- rate_limiter = request.param
- uvm = uvm_plain
+def uvm_with_rng_booted(microvm_factory, guest_kernel, rootfs, rate_limiter):
+ """Return a booted microvm with virtio-rng configured"""
+ uvm = microvm_factory.build(guest_kernel, rootfs)
uvm.spawn(log_level="INFO")
uvm.basic_config(vcpu_count=2, mem_size_mib=256)
uvm.add_net_iface()
@@ -23,6 +21,34 @@ def uvm_with_rng(uvm_plain, request):
return uvm
+def uvm_with_rng_restored(microvm_factory, guest_kernel, rootfs, rate_limiter):
+ """Return a restored uvm with virtio-rng configured"""
+ uvm = uvm_with_rng_booted(microvm_factory, guest_kernel, rootfs, rate_limiter)
+ snapshot = uvm.snapshot_full()
+ uvm.kill()
+ uvm2 = microvm_factory.build_from_snapshot(snapshot)
+ uvm2.rng_rate_limiter = uvm.rng_rate_limiter
+ return uvm2
+
+
+@pytest.fixture(params=[uvm_with_rng_booted, uvm_with_rng_restored])
+def uvm_ctor(request):
+ """Fixture to return uvms with different constructors"""
+ return request.param
+
+
+@pytest.fixture(params=[None])
+def rate_limiter(request):
+ """Fixture to return different rate limiters"""
+ return request.param
+
+
+@pytest.fixture
+def uvm_any(microvm_factory, uvm_ctor, guest_kernel, rootfs, rate_limiter):
+ """Return booted and restored uvms"""
+ return uvm_ctor(microvm_factory, guest_kernel, rootfs, rate_limiter)
+
+
def list_rng_available(ssh_connection: SSHConnection) -> list[str]:
"""Returns a list of rng devices available in the VM"""
return (
@@ -62,35 +88,17 @@ def test_rng_not_present(uvm_nano):
), "virtio_rng device should not be available in the uvm"
-def test_rng_present(uvm_with_rng):
+def test_rng_present(uvm_any):
"""
Test a guest microVM with an entropy defined configured and ensure
that we can access `/dev/hwrng`
"""
- vm = uvm_with_rng
+ vm = uvm_any
assert_virtio_rng_is_current_hwrng_device(vm.ssh)
check_entropy(vm.ssh)
-def test_rng_snapshot(uvm_with_rng, microvm_factory):
- """
- Test that a virtio-rng device is functional after resuming from
- a snapshot
- """
-
- vm = uvm_with_rng
- assert_virtio_rng_is_current_hwrng_device(vm.ssh)
- check_entropy(vm.ssh)
- snapshot = vm.snapshot_full()
-
- new_vm = microvm_factory.build()
- new_vm.spawn()
- new_vm.restore_from_snapshot(snapshot, resume=True)
- assert_virtio_rng_is_current_hwrng_device(new_vm.ssh)
- check_entropy(new_vm.ssh)
-
-
def _get_percentage_difference(measured, base):
"""Return the percentage delta between the arguments."""
if measured == base:
@@ -199,7 +207,7 @@ def _rate_limiter_id(rate_limiter):
# parametrize the RNG rate limiter
@pytest.mark.parametrize(
- "uvm_with_rng",
+ "rate_limiter",
[
{"bandwidth": {"size": 1000, "refill_time": 100}},
{"bandwidth": {"size": 10000, "refill_time": 100}},
@@ -208,16 +216,14 @@ def _rate_limiter_id(rate_limiter):
indirect=True,
ids=_rate_limiter_id,
)
-def test_rng_bw_rate_limiter(uvm_with_rng):
+@pytest.mark.parametrize("uvm_ctor", [uvm_with_rng_booted], indirect=True)
+def test_rng_bw_rate_limiter(uvm_any):
"""
Test that rate limiter without initial burst budget works
"""
- vm = uvm_with_rng
- # _start_vm_with_rng(vm, rate_limiter)
-
+ vm = uvm_any
size = vm.rng_rate_limiter["bandwidth"]["size"]
refill_time = vm.rng_rate_limiter["bandwidth"]["refill_time"]
-
expected_kbps = size / refill_time
assert_virtio_rng_is_current_hwrng_device(vm.ssh)
From c237b11f7ac873332e5689cbedb29743ffcf79f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= <pablob@amazon.com>
Date: Sat, 23 Nov 2024 13:55:58 +0100
Subject: [PATCH 20/23] tests: drop unused static cpu_templates fixture
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This fixture is not used anymore, as it is mostly superseded by
cpu_template_any.
Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
---
tests/conftest.py | 7 -------
.../functional/test_cpu_features_x86_64.py | 15 +++++++--------
2 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/tests/conftest.py b/tests/conftest.py
index 7cacedb9918..b599ffc79ab 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -292,13 +292,6 @@ def microvm_factory(request, record_property, results_dir):
uvm_factory.kill()
-@pytest.fixture(params=static_cpu_templates_params())
-def cpu_template(request, record_property):
- """Return all static CPU templates supported by the vendor."""
- record_property("static_cpu_template", request.param)
- return request.param
-
-
@pytest.fixture(params=custom_cpu_templates_params())
def custom_cpu_template(request, record_property):
"""Return all dummy custom CPU templates supported by the vendor."""
diff --git a/tests/integration_tests/functional/test_cpu_features_x86_64.py b/tests/integration_tests/functional/test_cpu_features_x86_64.py
index a966e253c46..1af6a39f83a 100644
--- a/tests/integration_tests/functional/test_cpu_features_x86_64.py
+++ b/tests/integration_tests/functional/test_cpu_features_x86_64.py
@@ -952,18 +952,16 @@ def check_enabled_features(test_microvm, cpu_template):
)
-def test_c3_on_skylake_show_warning(uvm_plain, cpu_template):
+def test_c3_on_skylake_show_warning(uvm_plain, cpu_template_any):
"""
This test verifies that the warning message about MMIO stale data mitigation
- is displayed only on Intel Skylake with C3 template.
+ is displayed only on Intel Skylake with static C3 template.
"""
uvm = uvm_plain
uvm.spawn()
- uvm.basic_config(
- vcpu_count=2,
- mem_size_mib=256,
- cpu_template=cpu_template,
- )
+ uvm.basic_config(vcpu_count=2, mem_size_mib=256)
+ uvm.add_net_iface()
+ uvm.set_cpu_template(cpu_template_any)
uvm.start()
message = (
@@ -972,7 +970,8 @@ def test_c3_on_skylake_show_warning(uvm_plain, cpu_template):
"does not apply the mitigation against MMIO stale data "
"vulnerability."
)
- if uvm.cpu_template_name == "c3" and global_props.cpu_codename == "INTEL_SKYLAKE":
+
+ if cpu_template_any == "C3" and global_props.cpu_codename == "INTEL_SKYLAKE":
assert message in uvm.log_data
else:
assert message not in uvm.log_data
From 0861767aa59dc03a2a15de6ce8f990c893e33d9b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= <pablob@amazon.com>
Date: Thu, 28 Nov 2024 23:25:56 +0100
Subject: [PATCH 21/23] tests: simplify rootfs fixture
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Since we only have one rootfs, simplify the fixture to return a single
value. This will have also not show it as part of the test instance
name.
Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
---
tests/conftest.py | 23 ++++++++++++-----------
tests/framework/artifacts.py | 10 ++--------
2 files changed, 14 insertions(+), 19 deletions(-)
diff --git a/tests/conftest.py b/tests/conftest.py
index b599ffc79ab..5ed61083014 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -35,7 +35,7 @@
import host_tools.cargo_build as build_tools
from framework import defs, utils
-from framework.artifacts import kernel_params, rootfs_params
+from framework.artifacts import disks, kernel_params
from framework.microvm import MicroVMFactory
from framework.properties import global_props
from framework.utils_cpu_templates import (
@@ -354,13 +354,6 @@ def guest_kernel_fxt(request, record_property):
return kernel
-def rootfs_fxt(request, record_property):
- """Return all supported rootfs."""
- fs = request.param
- record_property("rootfs", fs.name)
- return fs
-
-
# Fixtures for all guest kernels, and specific versions
guest_kernel = pytest.fixture(guest_kernel_fxt, params=kernel_params("vmlinux-*"))
guest_kernel_acpi = pytest.fixture(
@@ -380,9 +373,17 @@ def rootfs_fxt(request, record_property):
params=kernel_params("vmlinux-6.1*"),
)
-# Fixtures for all Ubuntu rootfs, and specific versions
-rootfs = pytest.fixture(rootfs_fxt, params=rootfs_params("ubuntu-24*.squashfs"))
-rootfs_rw = pytest.fixture(rootfs_fxt, params=rootfs_params("*.ext4"))
+
+@pytest.fixture
+def rootfs():
+ """Return an Ubuntu 24.04 read-only rootfs"""
+ return disks("ubuntu-24.04.squashfs")[0]
+
+
+@pytest.fixture
+def rootfs_rw():
+ """Return an Ubuntu 24.04 ext4 rootfs"""
+ return disks("ubuntu-24.04.ext4")[0]
@pytest.fixture
diff --git a/tests/framework/artifacts.py b/tests/framework/artifacts.py
index 77584f02129..0ed27c16b61 100644
--- a/tests/framework/artifacts.py
+++ b/tests/framework/artifacts.py
@@ -44,9 +44,9 @@ def kernels(glob, artifact_dir: Path = ARTIFACT_DIR) -> Iterator:
break
-def disks(glob) -> Iterator:
+def disks(glob) -> list:
"""Return supported rootfs"""
- yield from sorted(ARTIFACT_DIR.glob(glob))
+ return sorted(ARTIFACT_DIR.glob(glob))
def kernel_params(
@@ -57,12 +57,6 @@ def kernel_params(
yield pytest.param(kernel, id=kernel.name)
-def rootfs_params(glob="ubuntu-*.squashfs") -> Iterator:
- """Return supported rootfs as pytest parameters"""
- for rootfs in disks(glob=glob):
- yield pytest.param(rootfs, id=rootfs.name)
-
-
@dataclass(frozen=True, repr=True)
class FirecrackerArtifact:
"""Utility class for Firecracker binary artifacts."""
From 940b3606aa0d95f6f4d8262eeab099aa8c46c3d6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= <pablob@amazon.com>
Date: Fri, 29 Nov 2024 09:32:07 +0100
Subject: [PATCH 22/23] tests: parametrize uvm_any fixture family with vcpus
and memory
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Add a parameter so that we can control vcpu number and memory.
Right now it uses a hardcoded value, but it can be overridden per test.
Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
---
tests/conftest.py | 54 ++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 46 insertions(+), 8 deletions(-)
diff --git a/tests/conftest.py b/tests/conftest.py
index 5ed61083014..ac73b4bd8ab 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -455,20 +455,34 @@ def uvm_with_initrd(
yield uvm
-def uvm_booted(microvm_factory, guest_kernel, rootfs, cpu_template):
+@pytest.fixture
+def vcpu_count():
+ """Return default vcpu_count. Use indirect parametrization to override."""
+ return 2
+
+
+@pytest.fixture
+def mem_size_mib():
+ """Return memory size. Use indirect parametrization to override."""
+ return 256
+
+
+def uvm_booted(
+ microvm_factory, guest_kernel, rootfs, cpu_template, vcpu_count=2, mem_size_mib=256
+):
"""Return a booted uvm"""
uvm = microvm_factory.build(guest_kernel, rootfs)
uvm.spawn()
- uvm.basic_config(vcpu_count=2, mem_size_mib=256)
+ uvm.basic_config(vcpu_count=vcpu_count, mem_size_mib=mem_size_mib)
uvm.set_cpu_template(cpu_template)
uvm.add_net_iface()
uvm.start()
return uvm
-def uvm_restored(microvm_factory, guest_kernel, rootfs, cpu_template):
+def uvm_restored(microvm_factory, guest_kernel, rootfs, cpu_template, **kwargs):
"""Return a restored uvm"""
- uvm = uvm_booted(microvm_factory, guest_kernel, rootfs, cpu_template)
+ uvm = uvm_booted(microvm_factory, guest_kernel, rootfs, cpu_template, **kwargs)
snapshot = uvm.snapshot_full()
uvm.kill()
uvm2 = microvm_factory.build_from_snapshot(snapshot)
@@ -483,12 +497,36 @@ def uvm_ctor(request):
@pytest.fixture
-def uvm_any(microvm_factory, uvm_ctor, guest_kernel, rootfs, cpu_template_any):
+def uvm_any(
+ microvm_factory,
+ uvm_ctor,
+ guest_kernel,
+ rootfs,
+ cpu_template_any,
+ vcpu_count,
+ mem_size_mib,
+):
"""Return booted and restored uvms"""
- return uvm_ctor(microvm_factory, guest_kernel, rootfs, cpu_template_any)
+ return uvm_ctor(
+ microvm_factory,
+ guest_kernel,
+ rootfs,
+ cpu_template_any,
+ vcpu_count=vcpu_count,
+ mem_size_mib=mem_size_mib,
+ )
@pytest.fixture
-def uvm_any_booted(microvm_factory, guest_kernel, rootfs, cpu_template_any):
+def uvm_any_booted(
+ microvm_factory, guest_kernel, rootfs, cpu_template_any, vcpu_count, mem_size_mib
+):
"""Return booted uvms"""
- return uvm_booted(microvm_factory, guest_kernel, rootfs, cpu_template_any)
+ return uvm_booted(
+ microvm_factory,
+ guest_kernel,
+ rootfs,
+ cpu_template_any,
+ vcpu_count=vcpu_count,
+ mem_size_mib=mem_size_mib,
+ )
From 02d390585bb2c73d06015845509c3fb6033d40b5 Mon Sep 17 00:00:00 2001
From: tommady <tommady@users.noreply.github.com>
Date: Sat, 7 Dec 2024 04:57:41 +0000
Subject: [PATCH 23/23] refactor(vmm_builder): address comments
move interrupt controller into create_vmm_and_vcpus
and fix aarm64 integration test
Signed-off-by: tommady <tommady@users.noreply.github.com>
---
src/vmm/src/builder.rs | 29 ++++++++++-----------------
src/vmm/src/device_manager/persist.rs | 3 +++
2 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs
index ebffd1af2e1..3b36f4bec0a 100644
--- a/src/vmm/src/builder.rs
+++ b/src/vmm/src/builder.rs
@@ -274,8 +274,6 @@ pub mod aarch64 {
vcpus.push(vcpu);
}
- setup_interrupt_controller(vm, vcpu_count)?;
-
Ok(vcpus)
}
@@ -317,6 +315,9 @@ pub mod aarch64 {
let vcpus = create_vcpus(&mut vmm.vm, vm_config.vcpu_count, &vmm.vcpus_exit_evt)
.map_err(StartMicrovmError::Internal)?;
+ setup_interrupt_controller(&mut vmm.vm, vm_config.vcpu_count)
+ .map_err(StartMicrovmError::Internal)?;
+
Ok((vmm, vcpus))
}
}
@@ -459,6 +460,11 @@ pub mod x86_64 {
kvm_capabilities,
)?;
+ setup_interrupt_controller(&mut vmm.vm).map_err(StartMicrovmError::Internal)?;
+ vmm.pio_device_manager
+ .register_devices(vmm.vm.fd())
+ .unwrap();
+
let vcpus = create_vcpus(&mut vmm.vm, vm_config.vcpu_count, &vmm.vcpus_exit_evt)
.map_err(StartMicrovmError::Internal)?;
@@ -477,10 +483,7 @@ fn build_vmm(
// Set up Kvm Vm and register memory regions.
// Build custom CPU config if a custom template is provided.
- //
- // allow unused_mut for the aarch64 platform.
- #[allow(unused_mut)]
- let mut vm = Vm::new(kvm_capabilities)
+ let vm = Vm::new(kvm_capabilities)
.map_err(VmmError::Vm)
.map_err(Internal)?;
vm.memory_init(&guest_memory, vm_config.track_dirty_pages)
@@ -499,8 +502,6 @@ fn build_vmm(
// Instantiate ACPI device manager.
let acpi_device_manager = ACPIDeviceManager::new();
- // For x86_64 we need to create the interrupt controller before calling `KVM_CREATE_VCPUS`
- // while on aarch64 we need to do it the other way around.
#[cfg(target_arch = "x86_64")]
let pio_device_manager = {
// Serial device setup.
@@ -513,17 +514,9 @@ fn build_vmm(
.map_err(VmmError::EventFd)
.map_err(Internal)?;
- x86_64::setup_interrupt_controller(&mut vm).map_err(Internal)?;
-
// create pio dev manager with legacy devices
- let pio_device_manager = {
- // TODO Remove these unwraps.
- let mut pio_dev_mgr = PortIODeviceManager::new(serial_device, reset_evt).unwrap();
- pio_dev_mgr.register_devices(vm.fd()).unwrap();
- pio_dev_mgr
- };
-
- pio_device_manager
+ // TODO: remove this unwrap
+ PortIODeviceManager::new(serial_device, reset_evt).unwrap()
};
Ok(Vmm {
diff --git a/src/vmm/src/device_manager/persist.rs b/src/vmm/src/device_manager/persist.rs
index 9400de57d7e..c95f1eefe88 100644
--- a/src/vmm/src/device_manager/persist.rs
+++ b/src/vmm/src/device_manager/persist.rs
@@ -423,6 +423,9 @@ impl<'a> Persist<'a> for MMIODeviceManager {
for state in &state.legacy_devices {
if state.type_ == DeviceType::Serial {
let serial = setup_serial_device(std::io::stdin(), std::io::stdout())?;
+ constructor_args
+ .event_manager
+ .add_subscriber(serial.clone());
constructor_args
.resource_allocator