refactor(vmm): 2-step CpuConfiguration build for x86_64

Build a custom `CpuConfiguration` with 2 steps:
1. Construct a base `CpuConfiguration`.
2. Apply CPU template to the default `CpuConfiguration`.

Previously, all these processes were done in `CpuConfiguration::new()`,
but this approach is a bit complex in the test perspective. In addition,
we will be able to unify `CpuConfiguration` interface between x86_64 and
aarch64 and simplify `configure_system_for_boot()` function in
successive commits.

Signed-off-by: Takahiro Itazuri <[email protected]>

Author: zulinx86

src/vmm/src/builder.rs

@@ -42,7 +42,7 @@ use crate::construct_kvm_mpidrs;
 use crate::device_manager::legacy::PortIODeviceManager;
 use crate::device_manager::mmio::MMIODeviceManager;
 use crate::device_manager::persist::MMIODevManagerConstructorArgs;
-use crate::guest_config::templates::GuestConfigError;
+use crate::guest_config::templates::{GetCpuTemplate, GetCpuTemplateError, GuestConfigError};
 use crate::persist::{MicrovmState, MicrovmStateError};
 use crate::resources::VmResources;
 use crate::vmm_config::boot_source::BootConfig;
@@ -63,8 +63,8 @@ pub enum StartMicrovmError {
     #[error("System configuration error: {0:?}")]
     ConfigureSystem(crate::arch::Error),
     /// Error using CPU template to configure vCPUs
-    #[error("Unable to successfully create cpu configuration usable for guest vCPUs: {0:?}")]
-    CreateCpuConfig(GuestConfigError),
+    #[error("Failed to create guest config: {0:?}")]
+    CreateGuestConfig(#[from] GuestConfigError),
     /// Internal errors are due to resource exhaustion.
     #[error("Cannot create network device. {}", format!("{:?}", .0).replace('\"', ""))]
     CreateNetDevice(devices::virtio::net::Error),
@@ -83,6 +83,9 @@ pub enum StartMicrovmError {
     /// Internal error encountered while starting a microVM.
     #[error("Internal error while starting microVM: {0}")]
     Internal(Error),
+    /// Failed to get CPU template.
+    #[error("Failed to get CPU template: {0}")]
+    GetCpuTemplate(#[from] GetCpuTemplateError),
     /// The kernel command line is invalid.
     #[error("Invalid kernel command line: {0}")]
     KernelCmdline(String),
@@ -780,11 +783,26 @@ pub fn configure_system_for_boot(
 ) -> std::result::Result<(), StartMicrovmError> {
     use self::StartMicrovmError::*;
 
+    let cpu_template = vm_config.cpu_template.get_cpu_template()?;
+
     #[cfg(target_arch = "x86_64")]
     {
-        let cpu_config =
-            crate::guest_config::x86_64::CpuConfiguration::new(&vmm.vm, &vm_config.cpu_template)
-                .map_err(CreateCpuConfig)?;
+        use crate::guest_config::cpuid;
+        // Construct the base CpuConfiguration to apply CPU template onto.
+        let cpuid = cpuid::Cpuid::try_from(cpuid::RawCpuid::from(vmm.vm.supported_cpuid().clone()))
+            .map_err(GuestConfigError::CpuidFromRaw)?;
+        let msr_index_list = cpu_template.get_msr_index_list();
+        let msrs = vcpus[0]
+            .kvm_vcpu
+            .get_msrs(msr_index_list)
+            .map_err(GuestConfigError::VcpuIoctl)?;
+        let cpu_config = crate::guest_config::x86_64::CpuConfiguration { cpuid, msrs };
+
+        // Apply CPU template to the base CpuConfiguration.
+        let cpu_config = crate::guest_config::x86_64::CpuConfiguration::apply_template(
+            cpu_config,
+            &cpu_template,
+        )?;
 
         let vcpu_config = VcpuConfig {
             vcpu_count: vm_config.vcpu_count,
@@ -825,8 +843,7 @@ pub fn configure_system_for_boot(
         let cpu_config = crate::guest_config::aarch64::CpuConfiguration::new(
             &vcpus[0].kvm_vcpu.fd,
             &vm_config.cpu_template,
-        )
-        .map_err(CreateCpuConfig)?;
+        )?;
 
         let vcpu_config = VcpuConfig {
             vcpu_count: vm_config.vcpu_count,

src/vmm/src/guest_config/templates/x86_64.rs

@@ -327,6 +327,16 @@ where
     serializer.serialize_str(bitmap_str.as_str())
 }
 
+impl CustomCpuTemplate {
+    /// Get a list of MSR indices that are modified by the CPU template.
+    pub fn get_msr_index_list(&self) -> Vec<u32> {
+        self.msr_modifiers
+            .iter()
+            .map(|modifier| modifier.addr)
+            .collect()
+    }
+}
+
 // TODO mark with #[cfg(test)] when we combine all crates into
 // one firecracker crate
 impl CustomCpuTemplate {

src/vmm/src/guest_config/x86_64/mod.rs

@@ -9,13 +9,10 @@ pub mod static_cpu_templates_new;
 
 use std::collections::HashMap;
 
-use static_cpu_templates::*;
-
-use super::cpuid::{CpuidKey, RawCpuid};
+use super::cpuid::CpuidKey;
 use super::templates::x86_64::CpuidRegister;
-use super::templates::{CpuTemplateType, CustomCpuTemplate};
+use super::templates::CustomCpuTemplate;
 use crate::guest_config::cpuid::Cpuid;
-use crate::vstate::vm::Vm;
 
 /// Errors thrown while configuring templates.
 #[derive(Debug, PartialEq, Eq, thiserror::Error)]
@@ -32,6 +29,9 @@ pub enum Error {
     /// Can create cpuid from raw.
     #[error("Can create cpuid from raw: {0}")]
     CpuidFromRaw(super::cpuid::CpuidTryFromRawCpuid),
+    /// KVM vcpu ioctls failed.
+    #[error("KVM vcpu ioctl failed: {0}")]
+    VcpuIoctl(crate::vstate::vcpu::VcpuError),
 }
 
 /// CPU configuration for x86_64 CPUs
@@ -46,62 +46,6 @@ pub struct CpuConfiguration {
 }
 
 impl CpuConfiguration {
-    /// Creates new CpuConfig with cpu template changes applied
-    pub fn new(vm: &Vm, template: &Option<CpuTemplateType>) -> Result<Self, Error> {
-        match template {
-            Some(ref cpu_template) => Self::with_template(vm, cpu_template),
-            None => Self::host(vm),
-        }
-    }
-
-    /// Creates new CpuConfig with default values
-    pub fn host(vm: &Vm) -> Result<Self, Error> {
-        let supported_cpuid = vm.supported_cpuid().clone();
-        let supported_cpuid =
-            Cpuid::try_from(RawCpuid::from(supported_cpuid)).map_err(Error::CpuidFromRaw)?;
-
-        Ok(Self {
-            cpuid: supported_cpuid,
-            msrs: Default::default(),
-        })
-    }
-
-    /// Creates new CpuConfig with cpu template changes applied
-    pub fn with_template(vm: &Vm, template: &CpuTemplateType) -> Result<Self, Error> {
-        match template {
-            CpuTemplateType::Custom(template) => Self::host(vm)?.apply_template(template),
-            CpuTemplateType::Static(template) => {
-                let mut config = Self::host(vm)?;
-                // If a template is specified, get the CPUID template, else use `cpuid`.
-                let template_cpuid = match template {
-                    StaticCpuTemplate::C3 => static_cpu_templates::c3::c3(),
-                    StaticCpuTemplate::T2 => static_cpu_templates::t2::t2(),
-                    StaticCpuTemplate::T2S => static_cpu_templates::t2s::t2s(),
-                    StaticCpuTemplate::T2CL => static_cpu_templates::t2cl::t2cl(),
-                    StaticCpuTemplate::T2A => static_cpu_templates::t2a::t2a(),
-                    StaticCpuTemplate::None => unreachable!("None state is invalid"),
-                };
-
-                // Include leaves from host that are not present in CPUID template.
-                config.cpuid = template_cpuid
-                    .include_leaves_from(config.cpuid)
-                    .map_err(Error::CpuidJoin)?;
-
-                match template {
-                    StaticCpuTemplate::T2S => {
-                        static_cpu_templates::t2s::update_t2s_msr_entries(&mut config.msrs);
-                    }
-                    StaticCpuTemplate::T2CL => {
-                        static_cpu_templates::t2cl::update_t2cl_msr_entries(&mut config.msrs);
-                    }
-                    _ => (),
-                }
-
-                Ok(config)
-            }
-        }
-    }
-
     /// Modifies provided config with changes from template
     pub fn apply_template(self, template: &CustomCpuTemplate) -> Result<Self, Error> {
         let Self {

src/vmm/src/vstate/vcpu/mod.rs

@@ -928,30 +928,24 @@ pub mod tests {
         let entry_addr = load_good_kernel(&vm_mem);
 
         #[cfg(target_arch = "x86_64")]
-        vcpu.kvm_vcpu
-            .configure(
-                &vm_mem,
-                entry_addr,
-                &VcpuConfig {
-                    vcpu_count: 1,
-                    smt: false,
-                    cpu_config: crate::guest_config::x86_64::CpuConfiguration::host(&_vm).unwrap(),
-                },
-            )
-            .expect("failed to configure vcpu");
-
-        #[cfg(target_arch = "x86_64")]
-        vcpu.kvm_vcpu
-            .configure(
-                &vm_mem,
-                entry_addr,
-                &VcpuConfig {
-                    vcpu_count: 1,
-                    smt: false,
-                    cpu_config: crate::guest_config::x86_64::CpuConfiguration::host(&_vm).unwrap(),
-                },
-            )
-            .expect("failed to configure vcpu");
+        {
+            use crate::guest_config::cpuid::{Cpuid, RawCpuid};
+            vcpu.kvm_vcpu
+                .configure(
+                    &vm_mem,
+                    entry_addr,
+                    &VcpuConfig {
+                        vcpu_count: 1,
+                        smt: false,
+                        cpu_config: CpuConfiguration {
+                            cpuid: Cpuid::try_from(RawCpuid::from(_vm.supported_cpuid().clone()))
+                                .unwrap(),
+                            msrs: std::collections::HashMap::new(),
+                        },
+                    },
+                )
+                .expect("failed to configure vcpu");
+        }
 
         #[cfg(target_arch = "aarch64")]
         vcpu.kvm_vcpu

src/vmm/src/vstate/vcpu/x86_64.rs

@@ -30,7 +30,7 @@ use crate::vstate::vm::Vm;
 const TSC_KHZ_TOL: f64 = 250.0 / 1_000_000.0;
 
 /// Errors associated with the wrappers over KVM ioctls.
-#[derive(Debug, thiserror::Error)]
+#[derive(Debug, PartialEq, Eq, thiserror::Error)]
 pub enum Error {
     /// A FamStructWrapper operation has failed.
     #[error("Failed FamStructWrapper operation: {0:?}")]
@@ -618,7 +618,11 @@ mod tests {
 
     use super::*;
     use crate::arch::x86_64::cpu_model::CpuModel;
-    use crate::guest_config::templates::{CpuConfiguration, CpuTemplateType, StaticCpuTemplate};
+    use crate::guest_config::cpuid::{Cpuid, RawCpuid};
+    use crate::guest_config::templates::{
+        CpuConfiguration, CpuTemplateType, CustomCpuTemplate, GetCpuTemplate, GuestConfigError,
+        StaticCpuTemplate,
+    };
     use crate::vstate::vm::tests::setup_vm;
     use crate::vstate::vm::Vm;
 
@@ -661,36 +665,48 @@ mod tests {
 
     fn create_vcpu_config(
         vm: &Vm,
-        template: Option<CpuTemplateType>,
-    ) -> std::result::Result<VcpuConfig, crate::guest_config::x86_64::Error> {
-        let custom_cpu_config = CpuConfiguration::new(vm, &template)?;
+        vcpu: &KvmVcpu,
+        template: &CustomCpuTemplate,
+    ) -> std::result::Result<VcpuConfig, GuestConfigError> {
+        let cpuid = Cpuid::try_from(RawCpuid::from(vm.supported_cpuid().clone()))
+            .map_err(GuestConfigError::CpuidFromRaw)?;
+        let msrs = vcpu
+            .get_msrs(template.get_msr_index_list())
+            .map_err(GuestConfigError::VcpuIoctl)?;
+        let base_cpu_config = CpuConfiguration { cpuid, msrs };
+        let cpu_config = CpuConfiguration::apply_template(base_cpu_config, template)?;
         Ok(VcpuConfig {
             vcpu_count: 1,
             smt: false,
-            cpu_config: custom_cpu_config,
+            cpu_config,
         })
     }
 
     #[test]
     fn test_configure_vcpu() {
         let (vm, mut vcpu, vm_mem) = setup_vcpu(0x10000);
 
-        let vcpu_config = create_vcpu_config(&vm, None).unwrap();
+        let vcpu_config = create_vcpu_config(&vm, &vcpu, &CustomCpuTemplate::default()).unwrap();
         assert_eq!(
             vcpu.configure(&vm_mem, GuestAddress(0), &vcpu_config,),
             Ok(())
         );
 
         let try_configure = |vm: &Vm, vcpu: &mut KvmVcpu, template| -> bool {
-            if let Ok(config) = create_vcpu_config(vm, Some(CpuTemplateType::Static(template))) {
-                vcpu.configure(
-                    &vm_mem,
-                    GuestAddress(crate::arch::get_kernel_start()),
-                    &config,
-                )
-                .is_ok()
-            } else {
-                false
+            let cpu_template = Some(CpuTemplateType::Static(template));
+            let template = cpu_template.get_cpu_template();
+            match template {
+                Ok(template) => match create_vcpu_config(vm, vcpu, &template) {
+                    Ok(config) => vcpu
+                        .configure(
+                            &vm_mem,
+                            GuestAddress(crate::arch::get_kernel_start()),
+                            &config,
+                        )
+                        .is_ok(),
+                    Err(_) => false,
+                },
+                Err(_) => false,
             }
         };