refactor(vmm): move MSR modification for linux boot

Move MSR modification to boot linux from `CpuConfiguration::host()` to
`KvmVcpu::configure()`.

As CPUID modification is applied in `KvmVcpu::configure()` already,
putting MSR modification in the same place is sensible. In addition,
with this change, there is no need to contain `msrs_to_save` and
`msr_boot_entries` in `CpuConfiguration`.

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

Author: zulinx86

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

@@ -4,17 +4,14 @@
 /// Module with CPU templates for x86_64
 pub mod static_cpu_templates;
 
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 
-use kvm_bindings::kvm_msr_entry;
 use static_cpu_templates::*;
 
 use super::cpuid::{CpuidKey, RawCpuid};
 use super::templates::x86_64::CpuidRegister;
 use super::templates::{CpuTemplateType, CustomCpuTemplate};
-use crate::arch::x86_64::msr::create_boot_msr_entries;
 use crate::guest_config::cpuid::Cpuid;
-use crate::vstate::vcpu::msr_entries_to_save;
 use crate::vstate::vm::Vm;
 
 /// Errors thrown while configuring templates.
@@ -43,12 +40,6 @@ pub struct CpuConfiguration {
     /// Key: MSR address
     /// Value: MSR value
     pub msrs: HashMap<u32, u64>,
-
-    /// Set of supported MSRs
-    pub msrs_to_save: HashSet<u32>,
-
-    /// Architectural MSPs required for boot
-    pub msr_boot_entries: Vec<kvm_msr_entry>,
 }
 
 impl CpuConfiguration {
@@ -69,8 +60,6 @@ impl CpuConfiguration {
         Ok(Self {
             cpuid: supported_cpuid,
             msrs: Default::default(),
-            msrs_to_save: Default::default(),
-            msr_boot_entries: create_boot_msr_entries(),
         })
     }
 
@@ -95,27 +84,12 @@ impl CpuConfiguration {
                     .include_leaves_from(config.cpuid)
                     .map_err(Error::CpuidJoin)?;
 
-                // TODO: Some MSRs depend on values of other MSRs. This dependency will need to
-                // be implemented. For now we define known dependencies statically in the CPU
-                // templates.
-
-                // Depending on which CPU template the user selected, we may need to initialize
-                // additional MSRs for boot to correctly enable some CPU features. As stated in
-                // the previous comment, we get from the template a static list of MSRs we need
-                // to save at snapshot as well.
-                // C3, T2 and T2A currently don't have extra MSRs to save/set.
                 match template {
                     StaticCpuTemplate::T2S => {
-                        config.msrs_to_save.extend(msr_entries_to_save());
-                        static_cpu_templates::t2s::update_t2s_msr_entries(
-                            &mut config.msr_boot_entries,
-                        );
+                        static_cpu_templates::t2s::update_t2s_msr_entries(&mut config.msrs);
                     }
                     StaticCpuTemplate::T2CL => {
-                        config.msrs_to_save.extend(msr_entries_to_save());
-                        static_cpu_templates::t2cl::update_t2cl_msr_entries(
-                            &mut config.msr_boot_entries,
-                        );
+                        static_cpu_templates::t2cl::update_t2cl_msr_entries(&mut config.msrs);
                     }
                     _ => (),
                 }
@@ -130,8 +104,6 @@ impl CpuConfiguration {
         let Self {
             mut cpuid,
             mut msrs,
-            msrs_to_save,
-            msr_boot_entries,
         } = self;
 
         let guest_cpuid = match &mut cpuid {
@@ -185,12 +157,7 @@ impl CpuConfiguration {
             }
         }
 
-        Ok(Self {
-            cpuid,
-            msrs,
-            msrs_to_save,
-            msr_boot_entries,
-        })
+        Ok(Self { cpuid, msrs })
     }
 }
 
@@ -266,26 +233,20 @@ mod tests {
         CpuConfiguration {
             cpuid: Cpuid::Intel(IntelCpuid(BTreeMap::new())),
             msrs: Default::default(),
-            msrs_to_save: Default::default(),
-            msr_boot_entries: Default::default(),
         }
     }
 
     fn supported_cpu_config() -> CpuConfiguration {
         CpuConfiguration {
             cpuid: static_cpu_templates::t2::t2(),
             msrs: HashMap::from([(0x8000, 0b1000), (0x9999, 0b1010)]),
-            msrs_to_save: Default::default(),
-            msr_boot_entries: Default::default(),
         }
     }
 
     fn unsupported_cpu_config() -> CpuConfiguration {
         CpuConfiguration {
             cpuid: static_cpu_templates::t2::t2(),
             msrs: HashMap::from([(0x8000, 0b1000), (0x8001, 0b1010)]),
-            msrs_to_save: Default::default(),
-            msr_boot_entries: Default::default(),
         }
     }
 

src/vmm/src/guest_config/x86_64/static_cpu_templates/t2cl.rs

@@ -1,7 +1,7 @@
 // Copyright 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-use kvm_bindings::kvm_msr_entry;
+use std::collections::HashMap;
 
 use crate::arch::x86_64::msr::ArchCapaMSRFlags;
 use crate::arch_gen::x86::msr_index::MSR_IA32_ARCH_CAPABILITIES;
@@ -10,18 +10,14 @@ use crate::guest_config::cpuid::{Cpuid, CpuidEntry, CpuidKey, CpuidRegisters, In
 
 /// Add the MSR entries specific to this T2S template.
 #[inline]
-pub fn update_t2cl_msr_entries(msr_entries: &mut Vec<kvm_msr_entry>) {
+pub fn update_t2cl_msr_entries(msr_entries: &mut HashMap<u32, u64>) {
     let capabilities = ArchCapaMSRFlags::RDCL_NO
         | ArchCapaMSRFlags::IBRS_ALL
         | ArchCapaMSRFlags::SKIP_L1DFL_VMENTRY
         | ArchCapaMSRFlags::MDS_NO
         | ArchCapaMSRFlags::IF_PSCHANGE_MC_NO
         | ArchCapaMSRFlags::TSX_CTRL;
-    msr_entries.push(kvm_msr_entry {
-        index: MSR_IA32_ARCH_CAPABILITIES,
-        data: capabilities.bits(),
-        ..kvm_msr_entry::default()
-    });
+    msr_entries.insert(MSR_IA32_ARCH_CAPABILITIES, capabilities.bits());
 }
 
 /// This is translated from `cpuid -r` within a T2CL guest microVM on an ec2 m5.metal instance:

src/vmm/src/guest_config/x86_64/static_cpu_templates/t2s.rs

@@ -1,7 +1,7 @@
 // Copyright 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-use kvm_bindings::kvm_msr_entry;
+use std::collections::HashMap;
 
 use crate::arch::x86_64::msr::ArchCapaMSRFlags;
 use crate::arch_gen::x86::msr_index::MSR_IA32_ARCH_CAPABILITIES;
@@ -10,17 +10,13 @@ use crate::guest_config::cpuid::{Cpuid, CpuidEntry, CpuidKey, CpuidRegisters, In
 
 /// Add the MSR entries specific to this T2S template.
 #[inline]
-pub fn update_t2s_msr_entries(msr_entries: &mut Vec<kvm_msr_entry>) {
+pub fn update_t2s_msr_entries(msr_entries: &mut HashMap<u32, u64>) {
     let capabilities = ArchCapaMSRFlags::RSBA
         | ArchCapaMSRFlags::SKIP_L1DFL_VMENTRY
         | ArchCapaMSRFlags::IF_PSCHANGE_MC_NO
         | ArchCapaMSRFlags::MISC_PACKAGE_CTRLS
         | ArchCapaMSRFlags::ENERGY_FILTERING_CTL;
-    msr_entries.push(kvm_msr_entry {
-        index: MSR_IA32_ARCH_CAPABILITIES,
-        data: capabilities.bits(),
-        ..kvm_msr_entry::default()
-    });
+    msr_entries.insert(MSR_IA32_ARCH_CAPABILITIES, capabilities.bits());
 }
 
 /// This is translated from `cpuid -r` within a T2S guest microVM on an ec2 m5.metal instance:

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

@@ -18,7 +18,7 @@ use versionize::{VersionMap, Versionize, VersionizeError, VersionizeResult};
 use versionize_derive::Versionize;
 
 use crate::arch::x86_64::interrupts;
-use crate::arch::x86_64::msr::{SetMsrsError, MSR_IA32_ARCH_CAPABILITIES};
+use crate::arch::x86_64::msr::{create_boot_msr_entries, SetMsrsError};
 use crate::arch::x86_64::regs::{SetupFpuError, SetupRegistersError, SetupSpecialRegistersError};
 use crate::vstate::vcpu::{VcpuConfig, VcpuEmulation};
 use crate::vstate::vm::Vm;
@@ -29,16 +29,6 @@ use crate::vstate::vm::Vm;
 // https://bugzilla.redhat.com/show_bug.cgi?id=1839095
 const TSC_KHZ_TOL: f64 = 250.0 / 1_000_000.0;
 
-#[allow(clippy::missing_docs_in_private_items)]
-static EXTRA_MSR_ENTRIES: &[u32] = &[MSR_IA32_ARCH_CAPABILITIES];
-
-/// Return a list of MSRs specific to this T2S template.
-#[inline]
-#[must_use]
-pub fn msr_entries_to_save() -> &'static [u32] {
-    EXTRA_MSR_ENTRIES
-}
-
 /// Errors associated with the wrappers over KVM ioctls.
 #[derive(Debug, thiserror::Error)]
 pub enum Error {
@@ -228,12 +218,8 @@ impl KvmVcpu {
         vcpu_config: &VcpuConfig,
     ) -> std::result::Result<(), KvmVcpuConfigureError> {
         let mut cpuid = vcpu_config.cpu_config.cpuid.clone();
-        self.msrs_to_save
-            .extend(vcpu_config.cpu_config.msrs_to_save.iter());
-        let msr_boot_entries = vcpu_config.cpu_config.msr_boot_entries.clone();
 
         // Apply machine specific changes to CPUID.
-        // config_cpuid
         cpuid
             .normalize(
                 // The index of the current logical CPU in the range [0..cpu_count].
@@ -246,14 +232,22 @@ impl KvmVcpu {
             .map_err(KvmVcpuConfigureError::NormalizeCpuidError)?;
 
         // Set CPUID.
-        // let kvm_cpuid = kvm_bindings::CpuId::from(joined_cpuid);
         let kvm_cpuid = kvm_bindings::CpuId::from(cpuid);
 
         // Set CPUID in the KVM
         self.fd
             .set_cpuid2(&kvm_cpuid)
             .map_err(KvmVcpuConfigureError::SetCpuid)?;
 
+        // Clone MSR entries that are modified by CPU template from `VcpuConfig`.
+        let mut msrs = vcpu_config.cpu_config.msrs.clone();
+        self.msrs_to_save.extend(msrs.keys());
+
+        // Apply MSR modification to comply the linux boot protocol.
+        create_boot_msr_entries().into_iter().for_each(|entry| {
+            msrs.insert(entry.index, entry.data);
+        });
+
         // TODO - Add/amend MSRs for vCPUs based on cpu_config
         // By this point the Guest CPUID is established. Some CPU features require MSRs
         // to configure and interact with those features. If a MSR is writable from
@@ -266,11 +260,23 @@ impl KvmVcpu {
             .map_err(KvmVcpuConfigureError::MsrsToSaveByCpuid)?;
         self.msrs_to_save.extend(extra_msrs);
 
+        // TODO: Some MSRs depend on values of other MSRs. This dependency will need to
+        // be implemented.
+
         // By this point we know that at snapshot, the list of MSRs we need to
         // save is `architectural MSRs` + `MSRs inferred through CPUID` + `other
         // MSRs defined by the template`
 
-        crate::arch::x86_64::msr::set_msrs(&self.fd, &msr_boot_entries)?;
+        let kvm_msrs = msrs
+            .into_iter()
+            .map(|entry| kvm_bindings::kvm_msr_entry {
+                index: entry.0,
+                data: entry.1,
+                ..Default::default()
+            })
+            .collect::<Vec<_>>();
+
+        crate::arch::x86_64::msr::set_msrs(&self.fd, &kvm_msrs)?;
         crate::arch::x86_64::regs::setup_regs(&self.fd, kernel_start_addr.raw_value())?;
         crate::arch::x86_64::regs::setup_fpu(&self.fd)?;
         crate::arch::x86_64::regs::setup_sregs(guest_mem, &self.fd)?;