From 750b72a0619b011cbb2f1134883b5e3a7d1aeb28 Mon Sep 17 00:00:00 2001 From: Egor Lazarchuk Date: Fri, 24 Apr 2026 12:32:39 +0100 Subject: [PATCH 1/2] refactor: x64: use kvm_bindings::CpuId in Cupid type Previously we were using Btree as an underlying data type for the `Cpuid` type. This required us to convert from `CpuId` type which is what KVM provides for transformations and then convert back into `CpuId` when we need to set it back to KVM. These conversions are unnecessary since the `CpuId` type can be used for the same purpose without any need for intermediate transition to Btree. This commit does this swap to using `CpuId` directly. In addition to removing a need for transition code, this change reduces the number of dynamic allocations/deallocations used in the Btree implementation. The only edge case of updating entries is handled by new `cpuid_insert` function which ensures we preserve the Btree de-duplicating quality. Since the number of entries in `CpuId` is quite low, doing linear searches will not cause any noticable performance change. Signed-off-by: Egor Lazarchuk --- Cargo.lock | 1 + src/cpu-template-helper/Cargo.toml | 3 + .../src/template/dump/x86_64.rs | 82 ++++---- src/vmm/src/arch/x86_64/vcpu.rs | 8 +- .../src/cpu_config/x86_64/cpuid/amd/mod.rs | 74 ++----- .../cpu_config/x86_64/cpuid/amd/normalize.rs | 36 ++-- .../src/cpu_config/x86_64/cpuid/intel/mod.rs | 72 ++----- .../x86_64/cpuid/intel/normalize.rs | 149 +++++-------- src/vmm/src/cpu_config/x86_64/cpuid/mod.rs | 195 ++++++++---------- .../src/cpu_config/x86_64/cpuid/normalize.rs | 134 +++++------- src/vmm/src/cpu_config/x86_64/mod.rs | 38 ++-- 11 files changed, 304 insertions(+), 488 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cb1194958c4..3ed1e9ac3ea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -404,6 +404,7 @@ version = "1.17.0-dev" dependencies = [ "clap", "displaydoc", + "kvm-bindings", "libc", "log-instrument", "serde", diff --git a/src/cpu-template-helper/Cargo.toml b/src/cpu-template-helper/Cargo.toml index 87366de11d6..ad4536990ce 100644 --- a/src/cpu-template-helper/Cargo.toml +++ b/src/cpu-template-helper/Cargo.toml @@ -26,3 +26,6 @@ vmm-sys-util = "0.15.0" [lints] workspace = true + +[dev-dependencies] +kvm-bindings = "0.14.0" diff --git a/src/cpu-template-helper/src/template/dump/x86_64.rs b/src/cpu-template-helper/src/template/dump/x86_64.rs index eaa2553a658..fcdb37350ae 100644 --- a/src/cpu-template-helper/src/template/dump/x86_64.rs +++ b/src/cpu-template-helper/src/template/dump/x86_64.rs @@ -8,7 +8,7 @@ use vmm::arch::x86_64::generated::msr_index::*; use vmm::arch::x86_64::msr::MsrRange; use vmm::cpu_config::templates::{CpuConfiguration, CustomCpuTemplate, RegisterValueFilter}; use vmm::cpu_config::x86_64::cpuid::common::get_vendor_id_from_host; -use vmm::cpu_config::x86_64::cpuid::{Cpuid, VENDOR_ID_AMD}; +use vmm::cpu_config::x86_64::cpuid::{Cpuid, KvmCpuidFlags, VENDOR_ID_AMD}; use vmm::cpu_config::x86_64::custom_cpu_template::{ CpuidLeafModifier, CpuidRegister, CpuidRegisterModifier, RegisterModifier, }; @@ -25,23 +25,26 @@ pub fn config_to_template(cpu_config: &CpuConfiguration) -> CustomCpuTemplate { } fn cpuid_to_modifiers(cpuid: &Cpuid) -> Vec { - cpuid + let mut result: Vec<_> = cpuid .inner() + .as_slice() .iter() - .map(|(key, entry)| { + .map(|entry| { cpuid_leaf_modifier!( - key.leaf, - key.subleaf, - entry.flags, + entry.function, + entry.index, + KvmCpuidFlags(entry.flags), vec![ - cpuid_reg_modifier!(CpuidRegister::Eax, entry.result.eax), - cpuid_reg_modifier!(CpuidRegister::Ebx, entry.result.ebx), - cpuid_reg_modifier!(CpuidRegister::Ecx, entry.result.ecx), - cpuid_reg_modifier!(CpuidRegister::Edx, entry.result.edx), + cpuid_reg_modifier!(CpuidRegister::Eax, entry.eax), + cpuid_reg_modifier!(CpuidRegister::Ebx, entry.ebx), + cpuid_reg_modifier!(CpuidRegister::Ecx, entry.ecx), + cpuid_reg_modifier!(CpuidRegister::Edx, entry.edx), ] ) }) - .collect() + .collect(); + result.sort_by_key(|m| (m.leaf, m.subleaf)); + result } fn msrs_to_modifier(msrs: &BTreeMap) -> Vec { @@ -121,45 +124,36 @@ fn should_exclude_msr_amd(index: u32) -> bool { mod tests { use std::collections::BTreeMap; - use vmm::cpu_config::x86_64::cpuid::{ - CpuidEntry, CpuidKey, CpuidRegisters, IntelCpuid, KvmCpuidFlags, - }; + use vmm::cpu_config::x86_64::cpuid::IntelCpuid; use super::*; fn build_sample_cpuid() -> Cpuid { - Cpuid::Intel(IntelCpuid(BTreeMap::from([ - ( - CpuidKey { - leaf: 0x0, - subleaf: 0x0, - }, - CpuidEntry { - flags: KvmCpuidFlags::EMPTY, - result: CpuidRegisters { - eax: 0xffff_ffff, - ebx: 0x0000_ffff, - ecx: 0xffff_0000, - edx: 0x0000_0000, - }, + Cpuid::Intel(IntelCpuid( + kvm_bindings::CpuId::from_entries(&[ + kvm_bindings::kvm_cpuid_entry2 { + function: 0x0, + index: 0x0, + flags: KvmCpuidFlags::EMPTY.0, + eax: 0xffff_ffff, + ebx: 0x0000_ffff, + ecx: 0xffff_0000, + edx: 0x0000_0000, + ..Default::default() }, - ), - ( - CpuidKey { - leaf: 0x1, - subleaf: 0x1, + kvm_bindings::kvm_cpuid_entry2 { + function: 0x1, + index: 0x1, + flags: KvmCpuidFlags::SIGNIFICANT_INDEX.0, + eax: 0xaaaa_aaaa, + ebx: 0xaaaa_5555, + ecx: 0x5555_aaaa, + edx: 0x5555_5555, + ..Default::default() }, - CpuidEntry { - flags: KvmCpuidFlags::SIGNIFICANT_INDEX, - result: CpuidRegisters { - eax: 0xaaaa_aaaa, - ebx: 0xaaaa_5555, - ecx: 0x5555_aaaa, - edx: 0x5555_5555, - }, - }, - ), - ]))) + ]) + .unwrap(), + )) } fn build_expected_cpuid_modifiers() -> Vec { diff --git a/src/vmm/src/arch/x86_64/vcpu.rs b/src/vmm/src/arch/x86_64/vcpu.rs index 98563aab2e3..d63b63b98fa 100644 --- a/src/vmm/src/arch/x86_64/vcpu.rs +++ b/src/vmm/src/arch/x86_64/vcpu.rs @@ -214,10 +214,8 @@ impl KvmVcpu { u8::from(vcpu_config.vcpu_count > 1 && vcpu_config.smt), )?; - // Set CPUID. - let kvm_cpuid = kvm_bindings::CpuId::try_from(cpuid)?; - // Set CPUID in the KVM + let kvm_cpuid = kvm_bindings::CpuId::from(cpuid); self.fd .set_cpuid2(&kvm_cpuid) .map_err(KvmVcpuConfigureError::SetCpuid)?; @@ -818,7 +816,7 @@ mod tests { CpuConfiguration, CpuTemplateType, CustomCpuTemplate, GetCpuTemplate, GuestConfigError, StaticCpuTemplate, }; - use crate::cpu_config::x86_64::cpuid::{Cpuid, CpuidEntry, CpuidKey}; + use crate::cpu_config::x86_64::cpuid::{Cpuid, CpuidEntry, CpuidKey, CpuidTrait}; use crate::vstate::vm::tests::{setup_vm, setup_vm_with_memory}; impl Default for VcpuState { @@ -997,7 +995,6 @@ mod tests { let state = vcpu.save_state().unwrap(); let cpuid = Cpuid::try_from(state.cpuid).unwrap(); let leaf3 = cpuid - .inner() .get(&CpuidKey { leaf: 0x3, subleaf: 0x0, @@ -1046,7 +1043,6 @@ mod tests { let cpuid = Cpuid::try_from(cpuid).unwrap(); assert_ne!( cpuid - .inner() .get(&CpuidKey { leaf: 0, subleaf: 0, diff --git a/src/vmm/src/cpu_config/x86_64/cpuid/amd/mod.rs b/src/vmm/src/cpu_config/x86_64/cpuid/amd/mod.rs index f6bba245ee3..f523a1341bc 100644 --- a/src/vmm/src/cpu_config/x86_64/cpuid/amd/mod.rs +++ b/src/vmm/src/cpu_config/x86_64/cpuid/amd/mod.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 #![allow(clippy::similar_names, clippy::unreadable_literal)] -use super::{CpuidEntry, CpuidKey, CpuidRegisters, CpuidTrait, KvmCpuidFlags}; +use super::{CpuidEntry, CpuidKey, CpuidTrait, cpuid_insert}; /// CPUID normalize implementation. mod normalize; @@ -12,11 +12,20 @@ pub use normalize::{ }; /// A structure matching the AMD CPUID specification as described in -/// [AMD64 Architecture Programmer’s Manual Volume 3: General-Purpose and System Instructions](https://www.amd.com/system/files/TechDocs/24594.pdf) +/// [AMD64 Architecture Programmer's Manual Volume 3: General-Purpose and System Instructions](https://www.amd.com/system/files/TechDocs/24594.pdf) /// . #[allow(clippy::module_name_repetitions)] -#[derive(Debug, Clone, Eq, PartialEq)] -pub struct AmdCpuid(pub std::collections::BTreeMap); +#[derive(Debug, Clone, PartialEq)] +pub struct AmdCpuid(pub kvm_bindings::CpuId); + +impl Eq for AmdCpuid {} + +impl AmdCpuid { + /// Insert or update a CPUID entry. + pub fn insert(&mut self, key: CpuidKey, entry: CpuidEntry) { + cpuid_insert(&mut self.0, key, entry); + } +} impl CpuidTrait for AmdCpuid { /// Gets a given sub-leaf. @@ -31,60 +40,3 @@ impl CpuidTrait for AmdCpuid { self.0.get_mut(key) } } - -impl From for AmdCpuid { - #[inline] - fn from(kvm_cpuid: kvm_bindings::CpuId) -> Self { - let map = kvm_cpuid - .as_slice() - .iter() - .map(|entry| { - ( - CpuidKey { - leaf: entry.function, - subleaf: entry.index, - }, - CpuidEntry { - flags: KvmCpuidFlags(entry.flags), - result: CpuidRegisters { - eax: entry.eax, - ebx: entry.ebx, - ecx: entry.ecx, - edx: entry.edx, - }, - }, - ) - }) - .collect(); - Self(map) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn get() { - let cpuid = AmdCpuid(std::collections::BTreeMap::new()); - assert_eq!( - cpuid.get(&CpuidKey { - leaf: 0, - subleaf: 0 - }), - None - ); - } - - #[test] - fn get_mut() { - let mut cpuid = AmdCpuid(std::collections::BTreeMap::new()); - assert_eq!( - cpuid.get_mut(&CpuidKey { - leaf: 0, - subleaf: 0 - }), - None - ); - } -} diff --git a/src/vmm/src/cpu_config/x86_64/cpuid/amd/normalize.rs b/src/vmm/src/cpu_config/x86_64/cpuid/amd/normalize.rs index f3a938e9a62..a9de1787079 100644 --- a/src/vmm/src/cpu_config/x86_64/cpuid/amd/normalize.rs +++ b/src/vmm/src/cpu_config/x86_64/cpuid/amd/normalize.rs @@ -132,7 +132,7 @@ impl super::AmdCpuid { // Pass-through host CPUID for leaves 0x8000001e and 0x8000001d. { // 0x8000001e - Processor Topology Information - self.0.insert( + self.insert( CpuidKey::leaf(0x8000001e), CpuidEntry { flags: KvmCpuidFlags::EMPTY, @@ -169,7 +169,7 @@ impl super::AmdCpuid { if cache_type == 0 { break; } - self.0.insert( + self.insert( CpuidKey::subleaf(0x8000001d, subleaf), CpuidEntry { flags: KvmCpuidFlags::SIGNIFICANT_INDEX, @@ -387,8 +387,6 @@ impl super::AmdCpuid { #[cfg(test)] mod tests { - use std::collections::BTreeMap; - use super::*; use crate::cpu_config::x86_64::cpuid::AmdCpuid; @@ -396,7 +394,7 @@ mod tests { fn test_update_structured_extended_entry_invalid() { // `update_structured_extended_entry()` should exit with MissingLeaf0x7Subleaf0 error for // CPUID lacking leaf 0x7 / subleaf 0. - let mut cpuid = AmdCpuid(BTreeMap::new()); + let mut cpuid = AmdCpuid(kvm_bindings::CpuId::from_entries(&[]).unwrap()); assert_eq!( cpuid.update_structured_extended_entry().unwrap_err(), NormalizeCpuidError::MissingLeaf0x7Subleaf0 @@ -407,21 +405,19 @@ mod tests { fn test_update_structured_extended_entry_valid() { // `update_structured_extended_entry()` should succeed for CPUID having leaf 0x7 / subleaf // 0, and bit 29 of EDX (IA32_ARCH_CAPABILITIES MSR enumeration) should be disabled. - let mut cpuid = AmdCpuid(BTreeMap::from([( - CpuidKey { - leaf: 0x7, - subleaf: 0x0, - }, - CpuidEntry { - flags: KvmCpuidFlags::SIGNIFICANT_INDEX, - result: CpuidRegisters { - eax: 0, - ebx: 0, - ecx: 0, - edx: u32::MAX, - }, - }, - )])); + let mut cpuid = AmdCpuid( + kvm_bindings::CpuId::from_entries(&[kvm_bindings::kvm_cpuid_entry2 { + function: 0x7, + index: 0x0, + flags: KvmCpuidFlags::SIGNIFICANT_INDEX.0, + eax: 0, + ebx: 0, + ecx: 0, + edx: u32::MAX, + ..Default::default() + }]) + .unwrap(), + ); cpuid.update_structured_extended_entry().unwrap(); assert_eq!( cpuid diff --git a/src/vmm/src/cpu_config/x86_64/cpuid/intel/mod.rs b/src/vmm/src/cpu_config/x86_64/cpuid/intel/mod.rs index 48e393d5cee..ab42a1376eb 100644 --- a/src/vmm/src/cpu_config/x86_64/cpuid/intel/mod.rs +++ b/src/vmm/src/cpu_config/x86_64/cpuid/intel/mod.rs @@ -12,13 +12,22 @@ mod normalize; pub use normalize::{DeterministicCacheError, NormalizeCpuidError}; -use super::{CpuidEntry, CpuidKey, CpuidRegisters, CpuidTrait, KvmCpuidFlags}; +use super::{CpuidEntry, CpuidKey, CpuidTrait, cpuid_insert}; /// A structure matching the Intel CPUID specification as described in /// [Intel® 64 and IA-32 Architectures Software Developer's Manual Combined Volumes 2A, 2B, 2C, and 2D: Instruction Set Reference, A-Z](https://cdrdv2.intel.com/v1/dl/getContent/671110) /// . -#[derive(Debug, Clone, Eq, PartialEq)] -pub struct IntelCpuid(pub std::collections::BTreeMap); +#[derive(Debug, Clone, PartialEq)] +pub struct IntelCpuid(pub kvm_bindings::CpuId); + +impl Eq for IntelCpuid {} + +impl IntelCpuid { + /// Insert or update a CPUID entry. + pub fn insert(&mut self, key: CpuidKey, entry: CpuidEntry) { + cpuid_insert(&mut self.0, key, entry); + } +} impl CpuidTrait for IntelCpuid { /// Gets a given sub-leaf. @@ -33,60 +42,3 @@ impl CpuidTrait for IntelCpuid { self.0.get_mut(key) } } - -impl From for IntelCpuid { - #[inline] - fn from(kvm_cpuid: kvm_bindings::CpuId) -> Self { - let map = kvm_cpuid - .as_slice() - .iter() - .map(|entry| { - ( - CpuidKey { - leaf: entry.function, - subleaf: entry.index, - }, - CpuidEntry { - flags: KvmCpuidFlags(entry.flags), - result: CpuidRegisters { - eax: entry.eax, - ebx: entry.ebx, - ecx: entry.ecx, - edx: entry.edx, - }, - }, - ) - }) - .collect(); - Self(map) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn get() { - let cpuid = IntelCpuid(std::collections::BTreeMap::new()); - assert_eq!( - cpuid.get(&CpuidKey { - leaf: 0, - subleaf: 0 - }), - None - ); - } - - #[test] - fn get_mut() { - let mut cpuid = IntelCpuid(std::collections::BTreeMap::new()); - assert_eq!( - cpuid.get_mut(&CpuidKey { - leaf: 0, - subleaf: 0 - }), - None - ); - } -} diff --git a/src/vmm/src/cpu_config/x86_64/cpuid/intel/normalize.rs b/src/vmm/src/cpu_config/x86_64/cpuid/intel/normalize.rs index f6ac6872011..2cabae40cba 100644 --- a/src/vmm/src/cpu_config/x86_64/cpuid/intel/normalize.rs +++ b/src/vmm/src/cpu_config/x86_64/cpuid/intel/normalize.rs @@ -268,8 +268,8 @@ impl super::IntelCpuid { for index in 0.. { if let Some(subleaf) = self.get(&CpuidKey::subleaf(0xB, index)) { - self.0 - .insert(CpuidKey::subleaf(0x1F, index), subleaf.clone()); + let cloned = subleaf.clone(); + self.insert(CpuidKey::subleaf(0x1F, index), cloned); } else { break; } @@ -394,11 +394,10 @@ mod tests { clippy::as_conversions )] - use std::collections::BTreeMap; use std::ffi::CStr; use super::*; - use crate::cpu_config::x86_64::cpuid::{CpuidEntry, IntelCpuid, KvmCpuidFlags}; + use crate::cpu_config::x86_64::cpuid::{CpuidKey, IntelCpuid, KvmCpuidFlags}; #[test] fn default_brand_string_test() { @@ -437,25 +436,19 @@ mod tests { #[test] fn test_update_extended_feature_flags_entry() { - let mut cpuid = IntelCpuid(BTreeMap::from([( - CpuidKey { - leaf: 0x7, - subleaf: 0, - }, - CpuidEntry { - flags: KvmCpuidFlags::SIGNIFICANT_INDEX, + let mut cpuid = IntelCpuid( + kvm_bindings::CpuId::from_entries(&[kvm_bindings::kvm_cpuid_entry2 { + function: 0x7, + index: 0, + flags: KvmCpuidFlags::SIGNIFICANT_INDEX.0, ..Default::default() - }, - )])); + }]) + .unwrap(), + ); cpuid.update_extended_feature_flags_entry().unwrap(); - let leaf_7_0 = cpuid - .get(&CpuidKey { - leaf: 0x7, - subleaf: 0, - }) - .unwrap(); + let leaf_7_0 = cpuid.get(&CpuidKey::subleaf(0x7, 0)).unwrap(); assert!((leaf_7_0.result.ebx & (1 << 6)) > 0); assert!((leaf_7_0.result.ebx & (1 << 13)) > 0); assert_eq!((leaf_7_0.result.ecx & (1 << 5)), 0); @@ -463,100 +456,70 @@ mod tests { #[test] fn test_update_extended_topology_v2_entry_no_leaf_0x1f() { - let mut cpuid = IntelCpuid(BTreeMap::from([( - CpuidKey { - leaf: 0xB, - subleaf: 0, - }, - CpuidEntry { - flags: KvmCpuidFlags::SIGNIFICANT_INDEX, + let mut cpuid = IntelCpuid( + kvm_bindings::CpuId::from_entries(&[kvm_bindings::kvm_cpuid_entry2 { + function: 0xB, + index: 0, + flags: KvmCpuidFlags::SIGNIFICANT_INDEX.0, ..Default::default() - }, - )])); + }]) + .unwrap(), + ); cpuid.update_extended_topology_v2_entry(); - assert!( - cpuid - .get(&CpuidKey { - leaf: 0x1F, - subleaf: 0, - }) - .is_none() - ); + assert!(cpuid.get(&CpuidKey::subleaf(0x1F, 0)).is_none()); } #[test] fn test_update_extended_topology_v2_entry() { - let mut cpuid = IntelCpuid(BTreeMap::from([ - ( - CpuidKey { - leaf: 0xB, - subleaf: 0, - }, - CpuidEntry { - flags: KvmCpuidFlags::SIGNIFICANT_INDEX, - result: CpuidRegisters { - eax: 0x1, - ebx: 0x2, - ecx: 0x3, - edx: 0x4, - }, + let mut cpuid = IntelCpuid( + kvm_bindings::CpuId::from_entries(&[ + kvm_bindings::kvm_cpuid_entry2 { + function: 0xB, + index: 0, + flags: KvmCpuidFlags::SIGNIFICANT_INDEX.0, + eax: 0x1, + ebx: 0x2, + ecx: 0x3, + edx: 0x4, + ..Default::default() }, - ), - ( - CpuidKey { - leaf: 0xB, - subleaf: 1, + kvm_bindings::kvm_cpuid_entry2 { + function: 0xB, + index: 1, + flags: KvmCpuidFlags::SIGNIFICANT_INDEX.0, + eax: 0xa, + ebx: 0xb, + ecx: 0xc, + edx: 0xd, + ..Default::default() }, - CpuidEntry { - flags: KvmCpuidFlags::SIGNIFICANT_INDEX, - result: CpuidRegisters { - eax: 0xa, - ebx: 0xb, - ecx: 0xc, - edx: 0xd, - }, + kvm_bindings::kvm_cpuid_entry2 { + function: 0x1F, + index: 0, + flags: KvmCpuidFlags::SIGNIFICANT_INDEX.0, + eax: 0xFFFFFFFF, + ebx: 0xFFFFFFFF, + ecx: 0xFFFFFFFF, + edx: 0xFFFFFFFF, + ..Default::default() }, - ), - ( - CpuidKey { - leaf: 0x1F, - subleaf: 0, - }, - CpuidEntry { - flags: KvmCpuidFlags::SIGNIFICANT_INDEX, - result: CpuidRegisters { - eax: 0xFFFFFFFF, - ebx: 0xFFFFFFFF, - ecx: 0xFFFFFFFF, - edx: 0xFFFFFFFF, - }, - }, - ), - ])); + ]) + .unwrap(), + ); cpuid.update_extended_topology_v2_entry(); // Check leaf 0x1F, subleaf 0 is updated. - let leaf_1f_0 = cpuid - .get(&CpuidKey { - leaf: 0x1F, - subleaf: 0, - }) - .unwrap(); + let leaf_1f_0 = cpuid.get(&CpuidKey::subleaf(0x1F, 0)).unwrap(); assert_eq!(leaf_1f_0.result.eax, 0x1); assert_eq!(leaf_1f_0.result.ebx, 0x2); assert_eq!(leaf_1f_0.result.ecx, 0x3); assert_eq!(leaf_1f_0.result.edx, 0x4); - // Check lefa 0x1F, subleaf 1 is inserted. - let leaf_1f_1 = cpuid - .get(&CpuidKey { - leaf: 0x1F, - subleaf: 1, - }) - .unwrap(); + // Check leaf 0x1F, subleaf 1 is inserted. + let leaf_1f_1 = cpuid.get(&CpuidKey::subleaf(0x1F, 1)).unwrap(); assert_eq!(leaf_1f_1.result.eax, 0xa); assert_eq!(leaf_1f_1.result.ebx, 0xb); assert_eq!(leaf_1f_1.result.ecx, 0xc); diff --git a/src/vmm/src/cpu_config/x86_64/cpuid/mod.rs b/src/vmm/src/cpu_config/x86_64/cpuid/mod.rs index ce249cf1a76..15a5f70eb48 100644 --- a/src/vmm/src/cpu_config/x86_64/cpuid/mod.rs +++ b/src/vmm/src/cpu_config/x86_64/cpuid/mod.rs @@ -355,25 +355,30 @@ impl Cpuid { } } - /// Returns imumutable reference to inner BTreeMap. + /// Returns immutable reference to inner `kvm_bindings::CpuId`. #[inline] #[must_use] - pub fn inner(&self) -> &std::collections::BTreeMap { + pub fn inner(&self) -> &kvm_bindings::CpuId { match self { Self::Intel(intel_cpuid) => &intel_cpuid.0, Self::Amd(amd_cpuid) => &amd_cpuid.0, } } - /// Returns mutable reference to inner BTreeMap. + /// Returns mutable reference to inner `kvm_bindings::CpuId`. #[inline] #[must_use] - pub fn inner_mut(&mut self) -> &mut std::collections::BTreeMap { + pub fn inner_mut(&mut self) -> &mut kvm_bindings::CpuId { match self { Self::Intel(intel_cpuid) => &mut intel_cpuid.0, Self::Amd(amd_cpuid) => &mut amd_cpuid.0, } } + + /// Insert or update a CPUID entry. + pub fn insert(&mut self, key: CpuidKey, entry: CpuidEntry) { + cpuid_insert(self.inner_mut(), key, entry); + } } impl CpuidTrait for Cpuid { @@ -406,21 +411,39 @@ impl TryFrom for Cpuid { .ok_or(CpuidTryFromKvmCpuid::MissingLeaf0)?; match std::str::from_utf8(&vendor_id) { - Ok(VENDOR_ID_INTEL_STR) => Ok(Cpuid::Intel(IntelCpuid::from(kvm_cpuid))), - Ok(VENDOR_ID_AMD_STR) => Ok(Cpuid::Amd(AmdCpuid::from(kvm_cpuid))), + Ok(VENDOR_ID_INTEL_STR) => Ok(Cpuid::Intel(IntelCpuid(kvm_cpuid))), + Ok(VENDOR_ID_AMD_STR) => Ok(Cpuid::Amd(AmdCpuid(kvm_cpuid))), _ => Err(CpuidTryFromKvmCpuid::UnsupportedVendor(vendor_id)), } } } -impl TryFrom for kvm_bindings::CpuId { - type Error = vmm_sys_util::fam::Error; +impl From for kvm_bindings::CpuId { + #[inline] + fn from(cpuid: Cpuid) -> Self { + match cpuid { + Cpuid::Intel(intel_cpuid) => intel_cpuid.0, + Cpuid::Amd(amd_cpuid) => amd_cpuid.0, + } + } +} - fn try_from(cpuid: Cpuid) -> Result { - let entries = cpuid - .inner() - .iter() - .map(|(key, entry)| kvm_bindings::kvm_cpuid_entry2 { +/// Helper to insert or update a CPUID entry in a `kvm_bindings::CpuId`. +#[allow(clippy::needless_pass_by_value)] +fn cpuid_insert(kvm_cpuid: &mut kvm_bindings::CpuId, key: CpuidKey, entry: CpuidEntry) { + if let Some(existing) = kvm_cpuid + .as_mut_slice() + .iter_mut() + .find(|e| e.function == key.leaf && e.index == key.subleaf) + { + existing.flags = entry.flags.0; + existing.eax = entry.result.eax; + existing.ebx = entry.result.ebx; + existing.ecx = entry.result.ecx; + existing.edx = entry.result.edx; + } else { + kvm_cpuid + .push(kvm_bindings::kvm_cpuid_entry2 { function: key.leaf, index: key.subleaf, flags: entry.flags.0, @@ -430,9 +453,7 @@ impl TryFrom for kvm_bindings::CpuId { edx: entry.result.edx, ..Default::default() }) - .collect::>(); - - kvm_bindings::CpuId::from_entries(&entries) + .expect("CPUID entry count exceeded KVM_MAX_CPUID_ENTRIES"); } } @@ -583,29 +604,8 @@ impl From for CpuidRegisters { #[cfg(test)] mod tests { - use std::collections::BTreeMap; - use super::*; - fn build_intel_leaf0_for_cpuid() -> (CpuidKey, CpuidEntry) { - ( - CpuidKey { - leaf: 0x0, - subleaf: 0x0, - }, - CpuidEntry { - flags: KvmCpuidFlags::EMPTY, - result: CpuidRegisters { - eax: 0x1, - // GenuineIntel - ebx: 0x756E6547, - ecx: 0x6C65746E, - edx: 0x49656E69, - }, - }, - ) - } - fn build_intel_leaf0_for_kvmcpuid() -> kvm_bindings::kvm_cpuid_entry2 { kvm_bindings::kvm_cpuid_entry2 { function: 0x0, @@ -620,25 +620,6 @@ mod tests { } } - fn build_amd_leaf0_for_cpuid() -> (CpuidKey, CpuidEntry) { - ( - CpuidKey { - leaf: 0x0, - subleaf: 0x0, - }, - CpuidEntry { - flags: KvmCpuidFlags::EMPTY, - result: CpuidRegisters { - eax: 0x1, - // AuthenticAMD - ebx: 0x68747541, - ecx: 0x444D4163, - edx: 0x69746E65, - }, - }, - ) - } - fn build_amd_leaf0_for_kvmcpuid() -> kvm_bindings::kvm_cpuid_entry2 { kvm_bindings::kvm_cpuid_entry2 { function: 0x0, @@ -653,24 +634,6 @@ mod tests { } } - fn build_sample_leaf_for_cpuid() -> (CpuidKey, CpuidEntry) { - ( - CpuidKey { - leaf: 0x1, - subleaf: 0x2, - }, - CpuidEntry { - flags: KvmCpuidFlags::SIGNIFICANT_INDEX, - result: CpuidRegisters { - eax: 0x3, - ebx: 0x4, - ecx: 0x5, - edx: 0x6, - }, - }, - ) - } - fn build_sample_leaf_for_kvmcpuid() -> kvm_bindings::kvm_cpuid_entry2 { kvm_bindings::kvm_cpuid_entry2 { function: 0x1, @@ -684,13 +647,6 @@ mod tests { } } - fn build_sample_intel_cpuid() -> Cpuid { - Cpuid::Intel(IntelCpuid(BTreeMap::from([ - build_intel_leaf0_for_cpuid(), - build_sample_leaf_for_cpuid(), - ]))) - } - fn build_sample_intel_kvmcpuid() -> kvm_bindings::CpuId { kvm_bindings::CpuId::from_entries(&[ build_intel_leaf0_for_kvmcpuid(), @@ -699,13 +655,6 @@ mod tests { .unwrap() } - fn build_sample_amd_cpuid() -> Cpuid { - Cpuid::Amd(AmdCpuid(BTreeMap::from([ - build_amd_leaf0_for_cpuid(), - build_sample_leaf_for_cpuid(), - ]))) - } - fn build_sample_amd_kvmcpuid() -> kvm_bindings::CpuId { kvm_bindings::CpuId::from_entries(&[ build_amd_leaf0_for_kvmcpuid(), @@ -716,7 +665,7 @@ mod tests { #[test] fn get() { - let cpuid = build_sample_intel_cpuid(); + let cpuid = Cpuid::try_from(build_sample_intel_kvmcpuid()).unwrap(); assert_eq!( cpuid.get(&CpuidKey { leaf: 0x8888, @@ -736,7 +685,7 @@ mod tests { #[test] fn get_mut() { - let mut cpuid = build_sample_intel_cpuid(); + let mut cpuid = Cpuid::try_from(build_sample_intel_kvmcpuid()).unwrap(); assert_eq!( cpuid.get_mut(&CpuidKey { leaf: 0x888, @@ -757,23 +706,12 @@ mod tests { #[test] fn test_kvmcpuid_to_cpuid() { let kvm_cpuid = build_sample_intel_kvmcpuid(); - let cpuid = Cpuid::try_from(kvm_cpuid).unwrap(); - assert_eq!(cpuid, build_sample_intel_cpuid()); + let cpuid = Cpuid::try_from(kvm_cpuid.clone()).unwrap(); + assert_eq!(cpuid, Cpuid::Intel(IntelCpuid(kvm_cpuid))); let kvm_cpuid = build_sample_amd_kvmcpuid(); - let cpuid = Cpuid::try_from(kvm_cpuid).unwrap(); - assert_eq!(cpuid, build_sample_amd_cpuid()); - } - - #[test] - fn test_cpuid_to_kvmcpuid() { - let cpuid = build_sample_intel_cpuid(); - let kvm_cpuid = kvm_bindings::CpuId::try_from(cpuid).unwrap(); - assert_eq!(kvm_cpuid, build_sample_intel_kvmcpuid()); - - let cpuid = build_sample_amd_cpuid(); - let kvm_cpuid = kvm_bindings::CpuId::try_from(cpuid).unwrap(); - assert_eq!(kvm_cpuid, build_sample_amd_kvmcpuid()); + let cpuid = Cpuid::try_from(kvm_cpuid.clone()).unwrap(); + assert_eq!(cpuid, Cpuid::Amd(AmdCpuid(kvm_cpuid))); } #[test] @@ -785,4 +723,51 @@ mod tests { let cpuid = Cpuid::try_from(kvm_cpuid); assert_eq!(cpuid, Err(CpuidTryFromKvmCpuid::UnsupportedVendor([0; 12]))); } + + #[test] + fn test_insert_new_and_update() { + let mut cpuid = Cpuid::try_from(build_sample_intel_kvmcpuid()).unwrap(); + let orig_len = cpuid.inner().as_slice().len(); + + // Update existing + cpuid.insert( + CpuidKey { + leaf: 0x1, + subleaf: 0x2, + }, + CpuidEntry { + flags: KvmCpuidFlags::EMPTY, + result: CpuidRegisters { + eax: 69, + ebx: 0, + ecx: 0, + edx: 0, + }, + }, + ); + assert_eq!(cpuid.inner().as_slice().len(), orig_len); + assert_eq!( + cpuid.get(&CpuidKey::subleaf(0x1, 0x2)).unwrap().result.eax, + 69 + ); + + // Insert new + cpuid.insert( + CpuidKey { + leaf: 0x69, + subleaf: 0x0, + }, + CpuidEntry { + flags: KvmCpuidFlags::EMPTY, + result: CpuidRegisters { + eax: 69, + ebx: 0, + ecx: 0, + edx: 0, + }, + }, + ); + assert_eq!(cpuid.inner().as_slice().len(), orig_len + 1); + assert_eq!(cpuid.get(&CpuidKey::leaf(0x69)).unwrap().result.eax, 69); + } } diff --git a/src/vmm/src/cpu_config/x86_64/cpuid/normalize.rs b/src/vmm/src/cpu_config/x86_64/cpuid/normalize.rs index 7accc3ecd71..4002749ad42 100644 --- a/src/vmm/src/cpu_config/x86_64/cpuid/normalize.rs +++ b/src/vmm/src/cpu_config/x86_64/cpuid/normalize.rs @@ -273,17 +273,20 @@ impl super::Cpuid { // The following commit changed the behavior of KVM_GET_SUPPORTED_CPUID to no longer // include CPUID.(EAX=0BH,ECX=1). // https://lore.kernel.org/all/20221027092036.2698180-1-pbonzini@redhat.com/ - self.inner_mut() - .entry(CpuidKey::subleaf(0xB, 0x1)) - .or_insert(CpuidEntry { - flags: KvmCpuidFlags::SIGNIFICANT_INDEX, - result: CpuidRegisters { - eax: 0x0, - ebx: 0x0, - ecx: 0x0, - edx: 0x0, + if self.get(&CpuidKey::subleaf(0xB, 0x1)).is_none() { + self.insert( + CpuidKey::subleaf(0xB, 0x1), + CpuidEntry { + flags: KvmCpuidFlags::SIGNIFICANT_INDEX, + result: CpuidRegisters { + eax: 0x0, + ebx: 0x0, + ecx: 0x0, + edx: 0x0, + }, }, - }); + ); + } for index in 0.. { if let Some(subleaf) = self.get_mut(&CpuidKey::subleaf(0xB, index)) { @@ -425,10 +428,24 @@ const fn get_max_cpus_per_package(cpu_count: u8) -> Result Cpuid { + // Prepend an Intel leaf 0 so Cpuid::try_from succeeds + let mut all = vec![kvm_bindings::kvm_cpuid_entry2 { + function: 0x0, + index: 0x0, + flags: 0x0, + eax: 0x16, + ebx: 0x756E6547, // Genu + ecx: 0x6C65746E, // ntel + edx: 0x49656E69, // ineI + ..Default::default() + }]; + all.extend_from_slice(entries); + Cpuid::try_from(kvm_bindings::CpuId::from_entries(&all).unwrap()).unwrap() + } #[test] fn get_max_cpus_per_package_test() { @@ -463,34 +480,19 @@ mod tests { #[test] fn test_update_vendor_id() { // Check `update_vendor_id()` passes through the vendor ID from the host correctly. + let mut guest_cpuid = make_cpuid_with_entries(&[]); - // Pseudo CPUID with invalid vendor ID. - let mut guest_cpuid = Cpuid::Intel(IntelCpuid(BTreeMap::from([( - CpuidKey { - leaf: 0x0, - subleaf: 0x0, - }, - CpuidEntry { - flags: KvmCpuidFlags::EMPTY, - result: CpuidRegisters { - eax: 0, - ebx: 0x0123_4567, - ecx: 0x89ab_cdef, - edx: 0x55aa_55aa, - }, - }, - )]))); + // Overwrite vendor ID with garbage (pseudo CPUID with invalid vendor ID) + let leaf0 = guest_cpuid.get_mut(&CpuidKey::leaf(0x0)).unwrap(); + leaf0.result.ebx = 0x0123_4567; + leaf0.result.ecx = 0x89ab_cdef; + leaf0.result.edx = 0x55aa_55aa; // Pass through vendor ID from host. guest_cpuid.update_vendor_id().unwrap(); // Check if the guest vendor ID matches the host one. - let guest_leaf_0 = guest_cpuid - .get(&CpuidKey { - leaf: 0x0, - subleaf: 0x0, - }) - .unwrap(); + let guest_leaf_0 = guest_cpuid.get(&CpuidKey::leaf(0x0)).unwrap(); let host_leaf_0 = cpuid(0x0); assert_eq!(guest_leaf_0.result.ebx, host_leaf_0.ebx); assert_eq!(guest_leaf_0.result.ecx, host_leaf_0.ecx); @@ -512,56 +514,16 @@ mod tests { .ok_or(NormalizeCpuidError::CpuBits(cpu_bits)) .unwrap(); - // Case 1: Intel CPUID - let mut intel_cpuid = Cpuid::Intel(IntelCpuid(BTreeMap::from([( - CpuidKey { - leaf: 0xb, - subleaf: 0, - }, - CpuidEntry { - flags: KvmCpuidFlags::SIGNIFICANT_INDEX, - result: CpuidRegisters { - eax: 0, - ebx: 0, - ecx: 0, - edx: 0, - }, - }, - )]))); - let result = intel_cpuid.update_extended_topology_entry( - cpu_index, - cpu_count, - cpu_bits, - cpus_per_core, - ); - result.unwrap(); - assert!(intel_cpuid.inner().contains_key(&CpuidKey { - leaf: 0xb, - subleaf: 0x1 - })); - - // Case 2: AMD CPUID - let mut amd_cpuid = Cpuid::Amd(AmdCpuid(BTreeMap::from([( - CpuidKey { - leaf: 0xb, - subleaf: 0, - }, - CpuidEntry { - flags: KvmCpuidFlags::SIGNIFICANT_INDEX, - result: CpuidRegisters { - eax: 0, - ebx: 0, - ecx: 0, - edx: 0, - }, - }, - )]))); - let result = - amd_cpuid.update_extended_topology_entry(cpu_index, cpu_count, cpu_bits, cpus_per_core); - result.unwrap(); - assert!(amd_cpuid.inner().contains_key(&CpuidKey { - leaf: 0xb, - subleaf: 0x1 - })); + let mut cpuid = make_cpuid_with_entries(&[kvm_bindings::kvm_cpuid_entry2 { + function: 0xb, + index: 0, + flags: KvmCpuidFlags::SIGNIFICANT_INDEX.0, + ..Default::default() + }]); + + cpuid + .update_extended_topology_entry(cpu_index, cpu_count, cpu_bits, cpus_per_core) + .unwrap(); + assert!(cpuid.get(&CpuidKey::subleaf(0xb, 0x1)).is_some()); } } diff --git a/src/vmm/src/cpu_config/x86_64/mod.rs b/src/vmm/src/cpu_config/x86_64/mod.rs index ddf0b64dea0..4aa12592f1e 100644 --- a/src/vmm/src/cpu_config/x86_64/mod.rs +++ b/src/vmm/src/cpu_config/x86_64/mod.rs @@ -17,7 +17,7 @@ use kvm_bindings::CpuId; use self::custom_cpu_template::CpuidRegister; use super::templates::CustomCpuTemplate; use crate::Vcpu; -use crate::cpu_config::x86_64::cpuid::{Cpuid, CpuidKey}; +use crate::cpu_config::x86_64::cpuid::{Cpuid, CpuidKey, CpuidTrait}; /// Errors thrown while configuring templates. #[derive(Debug, PartialEq, Eq, thiserror::Error, displaydoc::Display)] @@ -67,15 +67,13 @@ impl CpuConfiguration { mut msrs, } = self; - let guest_cpuid = cpuid.inner_mut(); - // Apply CPUID modifiers for mod_leaf in template.cpuid_modifiers.iter() { let cpuid_key = CpuidKey { leaf: mod_leaf.leaf, subleaf: mod_leaf.subleaf, }; - if let Some(entry) = guest_cpuid.get_mut(&cpuid_key) { + if let Some(entry) = cpuid.get_mut(&cpuid_key) { entry.flags = mod_leaf.flags; // Can we modify one reg multiple times???? @@ -124,7 +122,7 @@ mod tests { use super::custom_cpu_template::{CpuidLeafModifier, CpuidRegisterModifier, RegisterModifier}; use super::*; use crate::cpu_config::templates::RegisterValueFilter; - use crate::cpu_config::x86_64::cpuid::{CpuidEntry, IntelCpuid, KvmCpuidFlags}; + use crate::cpu_config::x86_64::cpuid::KvmCpuidFlags; fn build_test_template() -> CustomCpuTemplate { CustomCpuTemplate { @@ -183,19 +181,33 @@ mod tests { } } + /// Helper: build a Cpuid with an Intel leaf 0 + given extra entries. + fn make_intel_cpuid(extra: &[kvm_bindings::kvm_cpuid_entry2]) -> Cpuid { + let mut entries = vec![kvm_bindings::kvm_cpuid_entry2 { + function: 0x0, + index: 0x0, + flags: 0x0, + eax: 0x16, + ebx: 0x756E6547, + ecx: 0x6C65746E, + edx: 0x49656E69, + ..Default::default() + }]; + entries.extend_from_slice(extra); + Cpuid::try_from(kvm_bindings::CpuId::from_entries(&entries).unwrap()).unwrap() + } + fn build_supported_cpuid() -> Cpuid { - Cpuid::Intel(IntelCpuid(BTreeMap::from([( - CpuidKey { - leaf: 0x3, - subleaf: 0x0, - }, - CpuidEntry::default(), - )]))) + make_intel_cpuid(&[kvm_bindings::kvm_cpuid_entry2 { + function: 0x3, + index: 0x0, + ..Default::default() + }]) } fn empty_cpu_config() -> CpuConfiguration { CpuConfiguration { - cpuid: Cpuid::Intel(IntelCpuid(BTreeMap::new())), + cpuid: make_intel_cpuid(&[]), msrs: Default::default(), } } From 5c87dfdc20ed7ecd51b767a4a7b5b0536942dd4c Mon Sep 17 00:00:00 2001 From: Egor Lazarchuk Date: Thu, 14 May 2026 13:59:51 +0100 Subject: [PATCH 2/2] refactor: x64: use kvm_bindings::Msrs type directly in CpuConfiguration Similarly to the previous commit, replace Btree type for Msrs handling with kvm_bindings::Msrs to remove the unnecessary state transitions and memory allocations. Uniqueness of entries is preserved with `msrs_insert` function. Together with previous commit this reduces the binary sizy by ~30KiB and reduces the maximum RSS during `test_all_vcpus_online` test by 50-80 KiB Signed-off-by: Egor Lazarchuk --- src/cpu-template-helper/Cargo.toml | 4 +- .../src/template/dump/x86_64.rs | 34 +++++----- src/vmm/src/arch/x86_64/msr.rs | 65 +++++++++++++++---- src/vmm/src/arch/x86_64/vcpu.rs | 46 +++++-------- src/vmm/src/cpu_config/x86_64/mod.rs | 52 ++++++++++----- src/vmm/src/vstate/vcpu.rs | 4 +- 6 files changed, 127 insertions(+), 78 deletions(-) diff --git a/src/cpu-template-helper/Cargo.toml b/src/cpu-template-helper/Cargo.toml index ad4536990ce..40b0850ff8e 100644 --- a/src/cpu-template-helper/Cargo.toml +++ b/src/cpu-template-helper/Cargo.toml @@ -15,6 +15,7 @@ tracing = ["log-instrument", "vmm/tracing"] [dependencies] clap = { version = "4.6.1", features = ["derive", "string"] } displaydoc = "0.2.6" +kvm-bindings = "0.14.0" libc = "0.2.186" log-instrument = { path = "../log-instrument", optional = true } serde = { version = "1.0.228", features = ["derive"] } @@ -26,6 +27,3 @@ vmm-sys-util = "0.15.0" [lints] workspace = true - -[dev-dependencies] -kvm-bindings = "0.14.0" diff --git a/src/cpu-template-helper/src/template/dump/x86_64.rs b/src/cpu-template-helper/src/template/dump/x86_64.rs index fcdb37350ae..57647eb3935 100644 --- a/src/cpu-template-helper/src/template/dump/x86_64.rs +++ b/src/cpu-template-helper/src/template/dump/x86_64.rs @@ -1,8 +1,7 @@ // Copyright 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -use std::collections::BTreeMap; - +use kvm_bindings::Msrs; use vmm::MSR_RANGE; use vmm::arch::x86_64::generated::msr_index::*; use vmm::arch::x86_64::msr::MsrRange; @@ -47,10 +46,11 @@ fn cpuid_to_modifiers(cpuid: &Cpuid) -> Vec { result } -fn msrs_to_modifier(msrs: &BTreeMap) -> Vec { +fn msrs_to_modifier(msrs: &Msrs) -> Vec { let mut msrs: Vec = msrs + .as_slice() .iter() - .map(|(index, value)| msr_modifier!(*index, *value)) + .map(|entry| msr_modifier!(entry.index, entry.data)) .collect(); msrs.retain(|modifier| !should_exclude_msr(modifier.addr)); @@ -122,8 +122,7 @@ fn should_exclude_msr_amd(index: u32) -> bool { #[cfg(test)] mod tests { - use std::collections::BTreeMap; - + use kvm_bindings::kvm_msr_entry; use vmm::cpu_config::x86_64::cpuid::IntelCpuid; use super::*; @@ -183,24 +182,29 @@ mod tests { ] } - fn build_sample_msrs() -> BTreeMap { - let mut map = BTreeMap::from([ + fn build_sample_msrs() -> Msrs { + let entry = |index, data| kvm_msr_entry { + index, + data, + ..Default::default() + }; + let mut entries = vec![ // should be sorted in the result. - (0x1, 0xffff_ffff_ffff_ffff), - (0x5, 0xffff_ffff_0000_0000), - (0x3, 0x0000_0000_ffff_ffff), - (0x2, 0x0000_0000_0000_0000), - ]); + entry(0x1, 0xffff_ffff_ffff_ffff), + entry(0x5, 0xffff_ffff_0000_0000), + entry(0x3, 0x0000_0000_ffff_ffff), + entry(0x2, 0x0000_0000_0000_0000), + ]; // should be excluded from the result. MSR_EXCLUSION_LIST .iter() .chain(MSR_EXCLUSION_LIST_AMD.iter()) .for_each(|range| { (range.base..(range.base + range.nmsrs)).for_each(|id| { - map.insert(id, 0); + entries.push(entry(id, 0)); }) }); - map + Msrs::from_entries(&entries).unwrap() } fn build_expected_msr_modifiers() -> Vec { diff --git a/src/vmm/src/arch/x86_64/msr.rs b/src/vmm/src/arch/x86_64/msr.rs index 9af3a71e028..a0e6169b2af 100644 --- a/src/vmm/src/arch/x86_64/msr.rs +++ b/src/vmm/src/arch/x86_64/msr.rs @@ -426,21 +426,43 @@ pub fn create_boot_msr_entries() -> Vec { ] } +/// Insert or update an MSR entry in `msrs`, mirroring `BTreeMap::insert` semantics: when an entry +/// with the given index already exists its `data` is overwritten, otherwise a new entry is pushed. +/// +/// # Errors +/// +/// Propagates [`vmm_sys_util::fam::Error`] from [`Msrs::push`] when the FAM struct is at capacity. +pub fn msrs_insert(msrs: &mut Msrs, index: u32, data: u64) { + if let Some(entry) = msrs + .as_mut_slice() + .iter_mut() + .find(|entry| entry.index == index) + { + entry.data = data; + } else { + msrs.push(kvm_msr_entry { + index, + data, + ..Default::default() + }) + .expect("MSRS entry count exceeded KVM_MAX_MSR_ENTRIES"); + } +} + /// Configure Model Specific Registers (MSRs) required to boot Linux for a given x86_64 vCPU. /// /// # Arguments /// /// * `vcpu` - Structure for the VCPU that holds the VCPU's fd. +/// * `msrs` - The MSRs to write to KVM. /// /// # Errors /// /// When: -/// - Failed to create [`vmm_sys_util::fam::FamStructWrapper`] for MSRs. /// - [`kvm_ioctls::ioctls::vcpu::VcpuFd::set_msrs`] errors. /// - [`kvm_ioctls::ioctls::vcpu::VcpuFd::set_msrs`] fails to write all given MSRs entries. -pub fn set_msrs(vcpu: &VcpuFd, msr_entries: &[kvm_msr_entry]) -> Result<(), MsrError> { - let msrs = Msrs::from_entries(msr_entries)?; - vcpu.set_msrs(&msrs) +pub fn set_msrs(vcpu: &VcpuFd, msrs: &Msrs) -> Result<(), MsrError> { + vcpu.set_msrs(msrs) .map_err(MsrError::SetMsrs) .and_then(|msrs_written| { if msrs_written == msrs.as_fam_struct_ref().nmsrs as usize { @@ -487,7 +509,8 @@ mod tests { fn test_setup_msrs() { let vcpu = create_vcpu(); let msr_boot_entries = create_boot_msr_entries(); - set_msrs(&vcpu, &msr_boot_entries).unwrap(); + let msrs = Msrs::from_entries(&msr_boot_entries).unwrap(); + set_msrs(&vcpu, &msrs).unwrap(); // This test will check against the last MSR entry configured (the tenth one). // See create_msr_entries() for details. @@ -515,12 +538,31 @@ mod tests { // Test `set_msrs()` with a valid MSR entry. It should succeed, as IA32_TSC MSR is listed // in supported MSRs as of now. let vcpu = create_vcpu(); - let msr_entries = vec![kvm_msr_entry { + let msrs = Msrs::from_entries(&[kvm_msr_entry { index: MSR_IA32_TSC, data: 0, ..Default::default() - }]; - set_msrs(&vcpu, &msr_entries).unwrap(); + }]) + .unwrap(); + set_msrs(&vcpu, &msrs).unwrap(); + } + + #[test] + fn test_msrs_insert_overwrites_and_pushes() { + let mut msrs = Msrs::new(0).unwrap(); + + msrs_insert(&mut msrs, 0x10, 0xaa); + msrs_insert(&mut msrs, 0x20, 0xbb); + assert_eq!(msrs.as_slice().len(), 2); + assert_eq!(msrs.as_slice()[0].data, 0xaa); + assert_eq!(msrs.as_slice()[1].data, 0xbb); + + msrs_insert(&mut msrs, 0x10, 0xcc); + assert_eq!(msrs.as_slice().len(), 2); + assert_eq!(msrs.as_slice()[0].index, 0x10); + assert_eq!(msrs.as_slice()[0].data, 0xcc); + assert_eq!(msrs.as_slice()[1].index, 0x20); + assert_eq!(msrs.as_slice()[1].data, 0xbb); } #[test] @@ -529,12 +571,13 @@ mod tests { // listed in supported MSRs as of now. If hardware vendor adds this MSR index and KVM // supports this MSR, we need to change the index as needed. let vcpu = create_vcpu(); - let msr_entries = vec![kvm_msr_entry { + let msrs = Msrs::from_entries(&[kvm_msr_entry { index: 2, ..Default::default() - }]; + }]) + .unwrap(); assert_eq!( - set_msrs(&vcpu, &msr_entries).unwrap_err(), + set_msrs(&vcpu, &msrs).unwrap_err(), MsrError::SetMsrsIncomplete ); } diff --git a/src/vmm/src/arch/x86_64/vcpu.rs b/src/vmm/src/arch/x86_64/vcpu.rs index d63b63b98fa..ce6657a3e10 100644 --- a/src/vmm/src/arch/x86_64/vcpu.rs +++ b/src/vmm/src/arch/x86_64/vcpu.rs @@ -5,7 +5,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the THIRD-PARTY file. -use std::collections::BTreeMap; use std::fmt::Debug; use std::sync::Arc; @@ -20,7 +19,7 @@ use vmm_sys_util::fam::{self, FamStruct}; use crate::arch::EntryPoint; use crate::arch::x86_64::generated::msr_index::{MSR_IA32_TSC, MSR_IA32_TSC_DEADLINE}; use crate::arch::x86_64::interrupts; -use crate::arch::x86_64::msr::{MsrError, create_boot_msr_entries}; +use crate::arch::x86_64::msr::{MsrError, create_boot_msr_entries, msrs_insert}; use crate::arch::x86_64::regs::{SetupFpuError, SetupRegistersError, SetupSpecialRegistersError}; use crate::cpu_config::x86_64::{CpuConfiguration, cpuid}; use crate::logger::{IncMetric, METRICS, error, warn}; @@ -222,12 +221,13 @@ impl KvmVcpu { // 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()); + self.msrs_to_save + .extend(msrs.as_slice().iter().map(|entry| entry.index)); // Apply MSR modification to comply the linux boot protocol. - create_boot_msr_entries().into_iter().for_each(|entry| { - msrs.insert(entry.index, entry.data); - }); + for entry in create_boot_msr_entries() { + msrs_insert(&mut msrs, 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 @@ -247,16 +247,7 @@ impl KvmVcpu { // save is `architectural MSRs` + `MSRs inferred through CPUID` + `other // MSRs defined by the template` - let kvm_msrs = msrs - .into_iter() - .map(|entry| kvm_bindings::kvm_msr_entry { - index: entry.0, - data: entry.1, - ..Default::default() - }) - .collect::>(); - - crate::arch::x86_64::msr::set_msrs(&self.fd, &kvm_msrs)?; + crate::arch::x86_64::msr::set_msrs(&self.fd, &msrs)?; crate::arch::x86_64::regs::setup_regs(&self.fd, kernel_entry_point)?; crate::arch::x86_64::regs::setup_fpu(&self.fd)?; crate::arch::x86_64::regs::setup_sregs(guest_mem, &self.fd, kernel_entry_point.protocol)?; @@ -534,19 +525,18 @@ impl KvmVcpu { /// # Errors /// /// * When `KvmVcpu::get_msr_chunks()` returns errors. + /// * When [`kvm_bindings::Msrs::new`] returns errors. pub fn get_msrs( &self, msr_index_iter: impl ExactSizeIterator, - ) -> Result, KvmVcpuError> { - let mut msrs = BTreeMap::new(); - self.get_msr_chunks(msr_index_iter)? - .iter() - .for_each(|msr_chunk| { - msr_chunk.as_slice().iter().for_each(|msr| { - msrs.insert(msr.index, msr.data); - }); - }); - Ok(msrs) + ) -> Result { + let mut combined = Msrs::new(0).map_err(KvmVcpuError::Fam)?; + for chunk in self.get_msr_chunks(msr_index_iter)? { + for entry in chunk.as_slice() { + msrs_insert(&mut combined, entry.index, entry.data); + } + } + Ok(combined) } /// Save the KVM internal state. @@ -1012,7 +1002,7 @@ mod tests { smt: false, cpu_config: CpuConfiguration { cpuid: Cpuid::try_from(vm.kvm().supported_cpuid.clone()).unwrap(), - msrs: BTreeMap::new(), + msrs: Msrs::new(0).unwrap(), }, }; vcpu.configure( @@ -1080,7 +1070,7 @@ mod tests { smt: false, cpu_config: CpuConfiguration { cpuid: Cpuid::try_from(vm.kvm().supported_cpuid.clone()).unwrap(), - msrs: BTreeMap::new(), + msrs: Msrs::new(0).unwrap(), }, }; diff --git a/src/vmm/src/cpu_config/x86_64/mod.rs b/src/vmm/src/cpu_config/x86_64/mod.rs index 4aa12592f1e..8ac99652efc 100644 --- a/src/vmm/src/cpu_config/x86_64/mod.rs +++ b/src/vmm/src/cpu_config/x86_64/mod.rs @@ -10,9 +10,7 @@ pub mod static_cpu_templates; /// Module with test utils for custom CPU templates pub mod test_utils; -use std::collections::BTreeMap; - -use kvm_bindings::CpuId; +use kvm_bindings::{CpuId, Msrs}; use self::custom_cpu_template::CpuidRegister; use super::templates::CustomCpuTemplate; @@ -33,14 +31,19 @@ pub enum CpuConfigurationError { } /// CPU configuration for x86_64 CPUs -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct CpuConfiguration { /// CPUID configuration pub cpuid: Cpuid, - /// Register values as a key pair for model specific registers - /// Key: MSR address - /// Value: MSR value - pub msrs: BTreeMap, + /// Model specific registers, stored directly as the KVM FAM-struct wrapper + /// to avoid conversions between this representation and the one expected by KVM. + pub msrs: Msrs, +} + +impl PartialEq for CpuConfiguration { + fn eq(&self, other: &Self) -> bool { + self.cpuid == other.cpuid && self.msrs.as_slice() == other.msrs.as_slice() + } } impl CpuConfiguration { @@ -102,10 +105,13 @@ impl CpuConfiguration { } for modifier in &template.msr_modifiers { - if let Some(reg_value) = msrs.get_mut(&modifier.addr) { - *reg_value = modifier.bitmap.apply(*reg_value); - } else { - return Err(CpuConfigurationError::MsrNotSupported(modifier.addr)); + match msrs + .as_mut_slice() + .iter_mut() + .find(|entry| entry.index == modifier.addr) + { + Some(entry) => entry.data = modifier.bitmap.apply(entry.data), + None => return Err(CpuConfigurationError::MsrNotSupported(modifier.addr)), } } @@ -115,9 +121,7 @@ impl CpuConfiguration { #[cfg(test)] mod tests { - use std::collections::BTreeMap; - - use kvm_bindings::KVM_CPUID_FLAG_STATEFUL_FUNC; + use kvm_bindings::{KVM_CPUID_FLAG_STATEFUL_FUNC, kvm_msr_entry}; use super::custom_cpu_template::{CpuidLeafModifier, CpuidRegisterModifier, RegisterModifier}; use super::*; @@ -205,24 +209,36 @@ mod tests { }]) } + fn make_msrs(entries: &[(u32, u64)]) -> Msrs { + let entries: Vec = entries + .iter() + .map(|&(index, data)| kvm_msr_entry { + index, + data, + ..Default::default() + }) + .collect(); + Msrs::from_entries(&entries).unwrap() + } + fn empty_cpu_config() -> CpuConfiguration { CpuConfiguration { cpuid: make_intel_cpuid(&[]), - msrs: Default::default(), + msrs: Msrs::new(0).unwrap(), } } fn supported_cpu_config() -> CpuConfiguration { CpuConfiguration { cpuid: build_supported_cpuid(), - msrs: BTreeMap::from([(0x8000, 0b1000), (0x9999, 0b1010)]), + msrs: make_msrs(&[(0x8000, 0b1000), (0x9999, 0b1010)]), } } fn unsupported_cpu_config() -> CpuConfiguration { CpuConfiguration { cpuid: build_supported_cpuid(), - msrs: BTreeMap::from([(0x8000, 0b1000), (0x8001, 0b1010)]), + msrs: make_msrs(&[(0x8000, 0b1000), (0x8001, 0b1010)]), } } diff --git a/src/vmm/src/vstate/vcpu.rs b/src/vmm/src/vstate/vcpu.rs index ffc473a70ac..33fc0e949ce 100644 --- a/src/vmm/src/vstate/vcpu.rs +++ b/src/vmm/src/vstate/vcpu.rs @@ -671,8 +671,6 @@ pub enum VcpuEmulation { pub(crate) mod tests { #![allow(clippy::undocumented_unsafe_blocks)] - #[cfg(target_arch = "x86_64")] - use std::collections::BTreeMap; use std::sync::atomic::Ordering; use std::sync::{Arc, Barrier, Mutex}; @@ -907,7 +905,7 @@ pub(crate) mod tests { smt: false, cpu_config: CpuConfiguration { cpuid: Cpuid::try_from(vm.kvm().supported_cpuid.clone()).unwrap(), - msrs: BTreeMap::new(), + msrs: kvm_bindings::Msrs::new(0).unwrap(), }, }, )