Skip to content

Commit 4eeeaa2

Browse files
committed
vmm: More specific CPU model check for CPU templates
Do CPU model check for static CPU templates in accordance with the doc change in the prev commit. Signed-off-by: Takahiro Itazuri <[email protected]>
1 parent 317c214 commit 4eeeaa2

File tree

5 files changed

+150
-98
lines changed

5 files changed

+150
-98
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ and this project adheres to
2626
WAITPKG CPUID bit in CPUID normalization. The feature enables a guest to put a
2727
physical processor into an idle state, which is undesirable in a FaaS
2828
environment since that is what the host wants to decide.
29+
- [#5142](https://github.com/firecracker-microvm/firecracker/pull/5142):
30+
Clarified what CPU models are supported by each existing CPU template.
31+
Firecracker exits with an error if a CPU template is used on an unsupported
32+
CPU model.
2933

3034
### Deprecated
3135

src/vmm/src/arch/x86_64/cpu_model.rs

Lines changed: 39 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
use std::arch::x86_64::__cpuid as host_cpuid;
5-
use std::cmp::{Eq, Ordering, PartialEq, PartialOrd};
5+
use std::cmp::{Eq, PartialEq};
66

77
/// Structure representing x86_64 CPU model.
88
#[derive(Debug, Eq, PartialEq)]
@@ -19,6 +19,42 @@ pub struct CpuModel {
1919
pub stepping: u8,
2020
}
2121

22+
/// Family / Model / Stepping for Intel Skylake
23+
pub const SKYLAKE_FMS: CpuModel = CpuModel {
24+
extended_family: 0x0,
25+
extended_model: 0x5,
26+
family: 0x6,
27+
model: 0x5,
28+
stepping: 0x4,
29+
};
30+
31+
/// Family / Model / Stepping for Intel Cascade Lake
32+
pub const CASCADE_LAKE_FMS: CpuModel = CpuModel {
33+
extended_family: 0x0,
34+
extended_model: 0x5,
35+
family: 0x6,
36+
model: 0x5,
37+
stepping: 0x7,
38+
};
39+
40+
/// Family / Model / Stepping for Intel Ice Lake
41+
pub const ICE_LAKE_FMS: CpuModel = CpuModel {
42+
extended_family: 0x0,
43+
extended_model: 0x6,
44+
family: 0x6,
45+
model: 0xa,
46+
stepping: 0x6,
47+
};
48+
49+
/// Family / Model / Stepping for AMD Milan
50+
pub const MILAN_FMS: CpuModel = CpuModel {
51+
extended_family: 0xa,
52+
extended_model: 0x0,
53+
family: 0xf,
54+
model: 0x1,
55+
stepping: 0x1,
56+
};
57+
2258
impl CpuModel {
2359
/// Get CPU model from current machine.
2460
pub fn get_cpu_model() -> Self {
@@ -27,19 +63,6 @@ impl CpuModel {
2763
let eax = unsafe { host_cpuid(0x1) }.eax;
2864
CpuModel::from(&eax)
2965
}
30-
31-
/// Check if the current CPU model is Intel Cascade Lake or later.
32-
pub fn is_at_least_cascade_lake(&self) -> bool {
33-
let cascade_lake = CpuModel {
34-
extended_family: 0,
35-
extended_model: 5,
36-
family: 6,
37-
model: 5,
38-
stepping: 7,
39-
};
40-
41-
self >= &cascade_lake
42-
}
4366
}
4467

4568
impl From<&u32> for CpuModel {
@@ -64,49 +87,14 @@ impl From<&CpuModel> for u32 {
6487
}
6588
}
6689

67-
impl PartialOrd for CpuModel {
68-
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
69-
Some(u32::from(self).cmp(&u32::from(other)))
70-
}
71-
}
72-
73-
impl Ord for CpuModel {
74-
fn cmp(&self, other: &Self) -> Ordering {
75-
u32::from(self).cmp(&u32::from(other))
76-
}
77-
}
78-
7990
#[cfg(test)]
8091
mod tests {
8192
use super::*;
8293

83-
const SKYLAKE: CpuModel = CpuModel {
84-
extended_family: 0,
85-
extended_model: 5,
86-
family: 6,
87-
model: 5,
88-
stepping: 4,
89-
};
90-
91-
const CASCADE_LAKE: CpuModel = CpuModel {
92-
extended_family: 0,
93-
extended_model: 5,
94-
family: 6,
95-
model: 5,
96-
stepping: 7,
97-
};
98-
9994
#[test]
10095
fn cpu_model_from() {
10196
let skylake_eax = 0x00050654;
102-
assert_eq!(u32::from(&SKYLAKE), skylake_eax);
103-
assert_eq!(CpuModel::from(&skylake_eax), SKYLAKE);
104-
}
105-
106-
#[test]
107-
fn cpu_model_ord() {
108-
assert_eq!(SKYLAKE, SKYLAKE);
109-
assert!(SKYLAKE < CASCADE_LAKE);
110-
assert!(CASCADE_LAKE > SKYLAKE);
97+
assert_eq!(u32::from(&SKYLAKE_FMS), skylake_eax);
98+
assert_eq!(CpuModel::from(&skylake_eax), SKYLAKE_FMS);
11199
}
112100
}

src/vmm/src/arch/x86_64/vcpu.rs

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -797,7 +797,9 @@ mod tests {
797797

798798
use super::*;
799799
use crate::arch::BootProtocol;
800-
use crate::arch::x86_64::cpu_model::CpuModel;
800+
use crate::arch::x86_64::cpu_model::{
801+
CASCADE_LAKE_FMS, CpuModel, ICE_LAKE_FMS, MILAN_FMS, SKYLAKE_FMS,
802+
};
801803
use crate::cpu_config::templates::{
802804
CpuConfiguration, CpuTemplateType, CustomCpuTemplate, GetCpuTemplate, GuestConfigError,
803805
StaticCpuTemplate,
@@ -832,17 +834,6 @@ mod tests {
832834
(kvm, vm, vcpu)
833835
}
834836

835-
fn is_at_least_cascade_lake() -> bool {
836-
CpuModel::get_cpu_model()
837-
>= (CpuModel {
838-
extended_family: 0,
839-
extended_model: 5,
840-
family: 6,
841-
model: 5,
842-
stepping: 7,
843-
})
844-
}
845-
846837
fn create_vcpu_config(
847838
kvm: &Kvm,
848839
vcpu: &KvmVcpu,
@@ -915,24 +906,37 @@ mod tests {
915906
// Test configure while using the T2S template.
916907
let t2a_res = try_configure(&kvm, &mut vcpu, StaticCpuTemplate::T2A);
917908

909+
let cpu_model = CpuModel::get_cpu_model();
918910
match &cpuid::common::get_vendor_id_from_host().unwrap() {
919911
cpuid::VENDOR_ID_INTEL => {
920-
assert!(t2_res);
921-
assert!(c3_res);
922-
assert!(t2s_res);
923-
if is_at_least_cascade_lake() {
924-
assert!(t2cl_res);
925-
} else {
926-
assert!(!t2cl_res);
927-
}
912+
assert_eq!(
913+
t2_res,
914+
cpu_model == SKYLAKE_FMS
915+
|| cpu_model == CASCADE_LAKE_FMS
916+
|| cpu_model == ICE_LAKE_FMS
917+
);
918+
assert_eq!(
919+
c3_res,
920+
cpu_model == SKYLAKE_FMS
921+
|| cpu_model == CASCADE_LAKE_FMS
922+
|| cpu_model == ICE_LAKE_FMS
923+
);
924+
assert_eq!(
925+
t2s_res,
926+
cpu_model == SKYLAKE_FMS || cpu_model == CASCADE_LAKE_FMS
927+
);
928+
assert_eq!(
929+
t2cl_res,
930+
cpu_model == CASCADE_LAKE_FMS || cpu_model == ICE_LAKE_FMS
931+
);
928932
assert!(!t2a_res);
929933
}
930934
cpuid::VENDOR_ID_AMD => {
931935
assert!(!t2_res);
932936
assert!(!c3_res);
933937
assert!(!t2s_res);
934938
assert!(!t2cl_res);
935-
assert!(t2a_res);
939+
assert_eq!(t2a_res, cpu_model == MILAN_FMS);
936940
}
937941
_ => {
938942
assert!(!t2_res);

src/vmm/src/cpu_config/x86_64/custom_cpu_template.rs

Lines changed: 72 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ use std::borrow::Cow;
88
use serde::de::Error as SerdeError;
99
use serde::{Deserialize, Deserializer, Serialize, Serializer};
1010

11-
use crate::arch::x86_64::cpu_model::CpuModel;
11+
use crate::arch::x86_64::cpu_model::{
12+
CASCADE_LAKE_FMS, CpuModel, ICE_LAKE_FMS, MILAN_FMS, SKYLAKE_FMS,
13+
};
1214
use crate::cpu_config::templates::{
1315
CpuTemplateType, GetCpuTemplate, GetCpuTemplateError, KvmCapability, RegisterValueFilter,
1416
};
@@ -27,12 +29,18 @@ impl GetCpuTemplate for Option<CpuTemplateType> {
2729
CpuTemplateType::Custom(template) => Ok(Cow::Borrowed(template)),
2830
CpuTemplateType::Static(template) => {
2931
let vendor_id = get_vendor_id_from_host().map_err(GetCpuVendor)?;
32+
let cpu_model = CpuModel::get_cpu_model();
3033
match template {
3134
StaticCpuTemplate::C3 => {
3235
if &vendor_id != VENDOR_ID_INTEL {
3336
return Err(CpuVendorMismatched);
37+
} else if cpu_model != SKYLAKE_FMS
38+
&& cpu_model != CASCADE_LAKE_FMS
39+
&& cpu_model != ICE_LAKE_FMS
40+
{
41+
return Err(InvalidCpuModel);
3442
}
35-
if !CpuModel::get_cpu_model().is_at_least_cascade_lake() {
43+
if cpu_model == SKYLAKE_FMS {
3644
warn!(
3745
"On processors that do not enumerate FBSDP_NO, PSDP_NO and \
3846
SBDR_SSDP_NO on IA32_ARCH_CAPABILITIES MSR, the guest kernel \
@@ -45,26 +53,35 @@ impl GetCpuTemplate for Option<CpuTemplateType> {
4553
StaticCpuTemplate::T2 => {
4654
if &vendor_id != VENDOR_ID_INTEL {
4755
return Err(CpuVendorMismatched);
56+
} else if cpu_model != SKYLAKE_FMS
57+
&& cpu_model != CASCADE_LAKE_FMS
58+
&& cpu_model != ICE_LAKE_FMS
59+
{
60+
return Err(InvalidCpuModel);
4861
}
4962
Ok(Cow::Owned(t2::t2()))
5063
}
5164
StaticCpuTemplate::T2S => {
5265
if &vendor_id != VENDOR_ID_INTEL {
5366
return Err(CpuVendorMismatched);
67+
} else if cpu_model != SKYLAKE_FMS && cpu_model != CASCADE_LAKE_FMS {
68+
return Err(InvalidCpuModel);
5469
}
5570
Ok(Cow::Owned(t2s::t2s()))
5671
}
5772
StaticCpuTemplate::T2CL => {
5873
if &vendor_id != VENDOR_ID_INTEL {
5974
return Err(CpuVendorMismatched);
60-
} else if !CpuModel::get_cpu_model().is_at_least_cascade_lake() {
75+
} else if cpu_model != CASCADE_LAKE_FMS && cpu_model != ICE_LAKE_FMS {
6176
return Err(InvalidCpuModel);
6277
}
6378
Ok(Cow::Owned(t2cl::t2cl()))
6479
}
6580
StaticCpuTemplate::T2A => {
6681
if &vendor_id != VENDOR_ID_AMD {
6782
return Err(CpuVendorMismatched);
83+
} else if cpu_model != MILAN_FMS {
84+
return Err(InvalidCpuModel);
6885
}
6986
Ok(Cow::Owned(t2a::t2a()))
7087
}
@@ -230,13 +247,25 @@ mod tests {
230247
#[test]
231248
fn test_get_cpu_template_with_c3_static_template() {
232249
// Test `get_cpu_template()` when C3 static CPU template is specified. The owned
233-
// `CustomCpuTemplate` should be returned if CPU vendor is Intel. Otherwise, it should fail.
250+
// `CustomCpuTemplate` should be returned if CPU vendor is Intel and the CPU model is
251+
// supported. Otherwise, it should fail.
234252
let cpu_template = Some(CpuTemplateType::Static(StaticCpuTemplate::C3));
235253
if &get_vendor_id_from_host().unwrap() == VENDOR_ID_INTEL {
236-
assert_eq!(
237-
cpu_template.get_cpu_template().unwrap(),
238-
Cow::Owned(c3::c3())
239-
);
254+
let cpu_model = CpuModel::get_cpu_model();
255+
if cpu_model != SKYLAKE_FMS
256+
&& cpu_model != CASCADE_LAKE_FMS
257+
&& cpu_model != ICE_LAKE_FMS
258+
{
259+
assert_eq!(
260+
cpu_template.get_cpu_template().unwrap_err(),
261+
GetCpuTemplateError::InvalidCpuModel,
262+
);
263+
} else {
264+
assert_eq!(
265+
cpu_template.get_cpu_template().unwrap(),
266+
Cow::Owned(c3::c3())
267+
);
268+
}
240269
} else {
241270
assert_eq!(
242271
cpu_template.get_cpu_template().unwrap_err(),
@@ -248,13 +277,25 @@ mod tests {
248277
#[test]
249278
fn test_get_cpu_template_with_t2_static_template() {
250279
// Test `get_cpu_template()` when T2 static CPU template is specified. The owned
251-
// `CustomCpuTemplate` should be returned if CPU vendor is Intel. Otherwise, it should fail.
280+
// `CustomCpuTemplate` should be returned if CPU vendor is Intel and the CPU model is
281+
// supported. Otherwise, it should fail.
252282
let cpu_template = Some(CpuTemplateType::Static(StaticCpuTemplate::T2));
253283
if &get_vendor_id_from_host().unwrap() == VENDOR_ID_INTEL {
254-
assert_eq!(
255-
cpu_template.get_cpu_template().unwrap(),
256-
Cow::Owned(t2::t2())
257-
);
284+
let cpu_model = CpuModel::get_cpu_model();
285+
if cpu_model != SKYLAKE_FMS
286+
&& cpu_model != CASCADE_LAKE_FMS
287+
&& cpu_model != ICE_LAKE_FMS
288+
{
289+
assert_eq!(
290+
cpu_template.get_cpu_template().unwrap_err(),
291+
GetCpuTemplateError::InvalidCpuModel,
292+
);
293+
} else {
294+
assert_eq!(
295+
cpu_template.get_cpu_template().unwrap(),
296+
Cow::Owned(t2::t2())
297+
);
298+
}
258299
} else {
259300
assert_eq!(
260301
cpu_template.get_cpu_template().unwrap_err(),
@@ -266,13 +307,22 @@ mod tests {
266307
#[test]
267308
fn test_get_cpu_template_with_t2s_static_template() {
268309
// Test `get_cpu_template()` when T2S static CPU template is specified. The owned
269-
// `CustomCpuTemplate` should be returned if CPU vendor is Intel. Otherwise, it should fail.
310+
// `CustomCpuTemplate` should be returned if CPU vendor is Intel and the CPU model is
311+
// supported. Otherwise, it should fail.
270312
let cpu_template = Some(CpuTemplateType::Static(StaticCpuTemplate::T2S));
271313
if &get_vendor_id_from_host().unwrap() == VENDOR_ID_INTEL {
272-
assert_eq!(
273-
cpu_template.get_cpu_template().unwrap(),
274-
Cow::Owned(t2s::t2s())
275-
);
314+
let cpu_model = CpuModel::get_cpu_model();
315+
if cpu_model != SKYLAKE_FMS && cpu_model != CASCADE_LAKE_FMS {
316+
assert_eq!(
317+
cpu_template.get_cpu_template().unwrap_err(),
318+
GetCpuTemplateError::InvalidCpuModel,
319+
);
320+
} else {
321+
assert_eq!(
322+
cpu_template.get_cpu_template().unwrap(),
323+
Cow::Owned(t2s::t2s())
324+
);
325+
}
276326
} else {
277327
assert_eq!(
278328
cpu_template.get_cpu_template().unwrap_err(),
@@ -299,10 +349,12 @@ mod tests {
299349
#[test]
300350
fn test_get_cpu_template_with_t2cl_static_template() {
301351
// Test `get_cpu_template()` when T2CL static CPU template is specified. The owned
302-
// `CustomCpuTemplate` should be returned if CPU vendor is Intel. Otherwise, it should fail.
352+
// `CustomCpuTemplate` should be returned if CPU vendor is Intel and the CPU model is
353+
// supported. Otherwise, it should fail.
303354
let cpu_template = Some(CpuTemplateType::Static(StaticCpuTemplate::T2CL));
304355
if &get_vendor_id_from_host().unwrap() == VENDOR_ID_INTEL {
305-
if CpuModel::get_cpu_model().is_at_least_cascade_lake() {
356+
let cpu_model = CpuModel::get_cpu_model();
357+
if cpu_model != CASCADE_LAKE_FMS && cpu_model != ICE_LAKE_FMS {
306358
assert_eq!(
307359
cpu_template.get_cpu_template().unwrap(),
308360
Cow::Owned(t2cl::t2cl())

0 commit comments

Comments
 (0)