-
Notifications
You must be signed in to change notification settings - Fork 2
CPU Profiles (Part 1: CPUID) #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
this profile is typically called |
|
|
|
||
| /// Describes how values within a CPUID output on two different hosts must relate to another in order for live-migration to be considered acceptable. | ||
| #[derive(Debug, Copy, Clone, Eq, PartialEq)] | ||
| pub enum MigrationCompatibilityRequirement { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we have migration code in crate arch which feels wrong. it is ok for the PoC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps CpuidCompatibilityRequirement would be a better name?
Note that the arch crate already contains code for checking cpuid compatibility, which is only relevant for live migration.
| /// A description of the value obtainable through CPUID | ||
| pub description: &'static str, | ||
| /// The range of bits in the output register corresponding to this feature or value. | ||
| pub bits_range: (u8, u8), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why isnt this a Rust Range?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question!
I Can't quite remember why I did it this way. I think some of the operations perhaps get a bit more convenient with this representation, but perhaps others don't ...
I will look into it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah now I recall the reason,
TL;DR: We can change it to RangeInclusive<u8>.
I initially thought to do more work at compile time in const blocks where you currently can't use iterators. I would still like to at least consider the possibility of generating more thorough CPUID compatibility checks than what currently exists upstream directly from the CPUID definitions introduced here. In that case being able to work with these definitions in const blocks will certainly be useful.
On the other hand, some of the work done when generating the CPU profiles could be simplified a little if we were to use a RangeInclusive for these and it is totally fine to perform all evaluations at runtime in that case.
Since RangeInclusive has a const constructor and also const methods to get the end points it should be totally fine to replace bits_range with RangeInclusive and we both retain the ability to do more computations in const blocks (if/when we want to) and we can simplify some of the profile generation code a bit 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, well done! I think this got bigger than anticipated by all of us. Without knowing all the details, it looks very sensible! Further, I love how you structured your commits which helps reviewing this a lot!
I'll approve once you addresses a few nits and we had a live migration test run in libvirt-tests.
| /// NOTE: The only way to interact with this value (beyond this crate) is via the const [`Self::as_slice()`](Self::as_slice) method. | ||
| pub struct ValueDefinitions(&'static [ValueDefinition]); | ||
| impl ValueDefinitions { | ||
| /// Constructor permitting less than 32 entries. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why less than 32?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Partially addressed in a later commit that should have been back ported.
There can be at most 32 different values packed into a 32-bit register hence the constraint. Perhaps I should update the documentation for ValuDefinitions to explain this?
| // TODO: Consider introducing a const as_slice method to make it impossible for the parameter -> value definitions | ||
| // constraints being broken. | ||
| pub struct CpuidDefinitions<const NUM_PARAMETERS: usize>( | ||
| pub [(Parameters, ValueDefinitions); NUM_PARAMETERS], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: looks unusual to me but you likely have good reason to do it like this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wrapped the inner array in order to centralize documentation and potential (const) methods. I think we can definitely remove the pub for the inner array however and instead have a method
pub const fn as_slice(&self) -> &[(Parameters, ValueDefinitions); NUM_PARAMETERS]though.
I would also be OK with avoiding the wrapping altogether and just assign [(Parameters, ValueDefinitions) ; NUM_PARAMETERS] directly to each constant (INTEL_CPUID_DEFINITION, KVM_CPUID_DEFINITIONS).
| // declare it here. | ||
| #[cfg(not(target_arch = "x86_64"))] | ||
| #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] | ||
| #[serde(rename_all = "kebab-case")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why kebab-case? it's fine, just asking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I recall correctly that is what QEMU uses for its CPU models. I thought it would be a good idea to stay consistent.
| #[allow(dead_code)] | ||
| pub struct CpuProfileData { | ||
| /// The hypervisor used when generating this CPU profile. | ||
| pub(in crate::x86_64) hypervisor: HypervisorType, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL this syntax
| #[cfg(feature = "tdx")] | ||
| pub tdx: bool, | ||
| pub amx: bool, | ||
| pub profile: CpuProfile, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on the long term: isn't there some duplication, e.g. with amx and profile?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes and no.
AMX is somewhat special as it requires additional configuration via arch_prctl and is something you might not want enabled even with the host profile. The pre-existing CpuFeatures struct in vmm::vm_config is where such opt-in features are currently declared/defined and then the fields reappear here for separation of concern considerations I guess. I think moving CpuFeatures into arch already now and re-importing it in vmm::vm_config the way I did with CpuProfile would be a good idea.
In the longer run we probably want to have opt-in features that always apply for the Host profile, but only when explicitly set for any other cpu profile. Whether such configurations should be part of the existing CpuFeatures or the CpuProfile enum itself should become a struct with such configrations (and possibly removing the existing CpuFeatures) will then be a necessary discussion.
| } | ||
|
|
||
| fn sort_entries(mut cpuid: Vec<CpuIdEntry>) -> Vec<CpuIdEntry> { | ||
| cpuid.sort_by(|entry, other_entry| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could use sort_by_unstable which is faster. I always prefer it as sort_by (the stable sorting) is not needed that often.
| fn sort_entries(mut cpuid: Vec<CpuIdEntry>) -> Vec<CpuIdEntry> { | ||
| cpuid.sort_by(|entry, other_entry| { | ||
| let fn_cmp = entry.function.cmp(&other_entry.function); | ||
| if fn_cmp == core::cmp::Ordering::Equal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use core::cmp;
| cpuid | ||
| } | ||
| /// Returns a `u32` where each bit between `first_bit_pos` and `last_bit_pos` is set (including both ends) and all other bits are 0. | ||
| fn bit_range_mask(first_bit_pos: u8, last_bit_pos: u8) -> u32 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not use a normal Rust range here, either exclusive or inclusive?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can do that 👍 See my comment here.
| const fn new(cpuid_descriptions: &'static [ValueDefinition]) -> Self { | ||
| // Note that this function is only called within this module, at compile time, hence it is fine to have some | ||
| // additional sanity checks such as the following assert. | ||
| assert!(cpuid_descriptions.len() <= 32); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be moved to one of the first commits :)
arch/src/bin/generate-cpu-profile.rs
Outdated
| @@ -0,0 +1,14 @@ | |||
| use anyhow::Context; | |||
| use std::io::BufWriter; | |||
| #[cfg(all( | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd use #![cfg here, i.e., crate level attributes
|
Some thing to discuss: |
I agree that this makes sense for production! We need however to see to what extend this is needed for the PoC. If we come to the conclusion that we even need it for the test environment, this is of course something different. |
Yeah we can probably define some known good profile name at the beginning that is expected to be available. Then we can add the API for profile detection later on. I think this can all be done without being a breaking change later on. |
Yes we want to be as compatible with libvirt as possible. I was actually considering not having named profiles at all and just let users load whatever they generate themselves, but that is not the standard libvirt way, hence I decided against it. I think it would be totally reasonable to have a command listing all available cpu profiles and also a command for explaining the details of a particular profile. The latter could use the (Intel) CPUID definitions introduced here and print the description corresponding to each value on a per CPUID leaf basis. We should create a follow up issue for this/these task(s). |
| ValueDefinitions::new(&[ | ||
| ValueDefinition{ short: "x2apic_id", description: "x2APIC ID of current logical CPU", bits_range: (0, 31), policy: ProfilePolicy::Passthrough, migration_compatibility_req: MigrationCompatibilityRequirement::Ignore }, | ||
| ]) | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does it mean to have these values of the topology information here? Does it lead to them being included in the generated CPU profile?
If yes, why do we want them in there, if they are not relevant for migration checks and even will be set by CHV depending on the configured topology in the end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the Passthrough policy is used for those values which basically means: Don't make any adjustments here (just use whatever the host gives you). So no, the values do not gt included in the generated CPU profile.
| @@ -0,0 +1,1814 @@ | |||
| //! This module contains CPUID definitions for Intel CPUs. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I am missing some more general info how this file should be seen and handled.
Is this a complete list of all available cpuid leaves (and even subleaves) or are there missing pieces? Or is it only what we have decided is relevant for migration?
What would it mean for the profile generation and CHV CPUID handling if something is missing here? Will a VM then see the host value or is the leaf then not offered to the VM at all?
Would you recommend some policy if we have to change a value here later on? Is a change here later on backward compatible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great question! I should have documented this module better 👍
It is almost a complete list of all available CPUID leaves (including sub-leaves) for Intel as of June 2025. There are about a handful of leaves left out which I deem not relevant or out of scope for the PoC. Anything not specified here will be zeroed out for the guest (or completely ommitted, I don't quite recall the exact details) unless it is in the leaf range reserved for hyperviors specifically. Hence if some new CPU shows up and someone is running that as the host any features that we did not get round to think about get zeroed out by default when used with one of our profiles. I think that is a reasonable best effort default, although it could be problematic if the leaf happens to be an inverse bit-set (which I think has happened for a particular sub-leaf before, I have to look that up again).
Changes to this table do (as of now) not affect existing CPU profiles unless they are regenerated. If we decide to generate the CPUID compatibility checks from that table as well, then changes to the table are no longer necessarily directly backward compatible, but that is also the case with the current CPUID compatibility checks.
| ValueDefinition{ short: "monitor", description: "MONITOR/MWAIT support", bits_range: (3, 3), policy: ProfilePolicy::Overwrite(0), migration_compatibility_req: MigrationCompatibilityRequirement::ContainsBits }, | ||
| ValueDefinition{ short: "ds_cpl", description: "CPL Qualified Debug Store", bits_range: (4, 4), policy: ProfilePolicy::Overwrite(0), migration_compatibility_req: MigrationCompatibilityRequirement::ContainsBits}, | ||
| // TODO: Is this a good default? Might be useful for nested virtualization ... | ||
| ValueDefinition{ short: "vmx", description: "Virtual Machine Extensions", bits_range: (5, 5), policy: ProfilePolicy::Overwrite(0), migration_compatibility_req: MigrationCompatibilityRequirement::ContainsBits }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tpressure Can you take a look here.
I think CHV currently has nested virtualization enabled per default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It indeed has
| @@ -0,0 +1 @@ | |||
| {"hypervisor":"Kvm","cpu_vendor":"Intel","adjustments":[[{"leaf":0,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":27,"mask":0}],[{"leaf":0,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":1970169159,"mask":0}],[{"leaf":0,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":1818588270,"mask":0}],[{"leaf":0,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":1231384169,"mask":0}],[{"leaf":1,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":526017,"mask":0}],[{"leaf":1,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":1,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":1996108291,"mask":2147484672}],[{"leaf":1,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":260832251,"mask":0}],[{"leaf":2,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":4294967295}],[{"leaf":2,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":2,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":4294967295}],[{"leaf":2,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":4294967295}],[{"leaf":4,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":4294951935}],[{"leaf":4,"sub_leaf":{"start":1,"end":1},"register":"EAX"},{"replacements":0,"mask":4294951935}],[{"leaf":4,"sub_leaf":{"start":2,"end":2},"register":"EAX"},{"replacements":0,"mask":4294951935}],[{"leaf":4,"sub_leaf":{"start":3,"end":3},"register":"EAX"},{"replacements":0,"mask":4294951935}],[{"leaf":4,"sub_leaf":{"start":4,"end":4},"register":"EAX"},{"replacements":0,"mask":4294951935}],[{"leaf":4,"sub_leaf":{"start":5,"end":4294967295},"register":"EAX"},{"replacements":0,"mask":4294951935}],[{"leaf":4,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":4,"sub_leaf":{"start":1,"end":1},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":4,"sub_leaf":{"start":2,"end":2},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":4,"sub_leaf":{"start":3,"end":3},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":4,"sub_leaf":{"start":4,"end":4},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":4,"sub_leaf":{"start":5,"end":4294967295},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":4,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":2147483647}],[{"leaf":4,"sub_leaf":{"start":1,"end":1},"register":"ECX"},{"replacements":0,"mask":2147483647}],[{"leaf":4,"sub_leaf":{"start":2,"end":2},"register":"ECX"},{"replacements":0,"mask":2147483647}],[{"leaf":4,"sub_leaf":{"start":3,"end":3},"register":"ECX"},{"replacements":0,"mask":2147483647}],[{"leaf":4,"sub_leaf":{"start":4,"end":4},"register":"ECX"},{"replacements":0,"mask":2147483647}],[{"leaf":4,"sub_leaf":{"start":5,"end":4294967295},"register":"ECX"},{"replacements":0,"mask":2147483647}],[{"leaf":4,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":7}],[{"leaf":4,"sub_leaf":{"start":1,"end":1},"register":"EDX"},{"replacements":0,"mask":7}],[{"leaf":4,"sub_leaf":{"start":2,"end":2},"register":"EDX"},{"replacements":0,"mask":7}],[{"leaf":4,"sub_leaf":{"start":3,"end":3},"register":"EDX"},{"replacements":0,"mask":7}],[{"leaf":4,"sub_leaf":{"start":4,"end":4},"register":"EDX"},{"replacements":0,"mask":7}],[{"leaf":4,"sub_leaf":{"start":5,"end":4294967295},"register":"EDX"},{"replacements":0,"mask":7}],[{"leaf":5,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":5,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":5,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":5,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":4294967295}],[{"leaf":6,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":4,"mask":0}],[{"leaf":6,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":6,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":65280}],[{"leaf":6,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":4294905600}],[{"leaf":7,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":2,"mask":0}],[{"leaf":7,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":4055836649,"mask":0}],[{"leaf":7,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":406871886,"mask":4063280}],[{"leaf":7,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":2885682448,"mask":0}],[{"leaf":7,"sub_leaf":{"start":1,"end":1},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":7,"sub_leaf":{"start":1,"end":1},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":7,"sub_leaf":{"start":1,"end":1},"register":"EDX"},{"replacements":0,"mask":8192}],[{"leaf":7,"sub_leaf":{"start":2,"end":2},"register":"EDX"},{"replacements":1,"mask":0}],[{"leaf":9,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":4294967295}],[{"leaf":10,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":10,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":10,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":10,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":0}],[{"leaf":11,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":31}],[{"leaf":11,"sub_leaf":{"start":1,"end":4294967295},"register":"EAX"},{"replacements":0,"mask":31}],[{"leaf":11,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":65535}],[{"leaf":11,"sub_leaf":{"start":1,"end":4294967295},"register":"EBX"},{"replacements":0,"mask":65535}],[{"leaf":11,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":65535}],[{"leaf":11,"sub_leaf":{"start":1,"end":4294967295},"register":"ECX"},{"replacements":0,"mask":65535}],[{"leaf":11,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":4294967295}],[{"leaf":11,"sub_leaf":{"start":1,"end":4294967295},"register":"EDX"},{"replacements":0,"mask":4294967295}],[{"leaf":13,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":743,"mask":130304}],[{"leaf":13,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":13,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":4294967295}],[{"leaf":13,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":0}],[{"leaf":13,"sub_leaf":{"start":1,"end":1},"register":"EAX"},{"replacements":15,"mask":0}],[{"leaf":13,"sub_leaf":{"start":1,"end":1},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":13,"sub_leaf":{"start":1,"end":1},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":13,"sub_leaf":{"start":1,"end":1},"register":"EDX"},{"replacements":0,"mask":0}],[{"leaf":13,"sub_leaf":{"start":2,"end":2},"register":"EAX"},{"replacements":256,"mask":0}],[{"leaf":13,"sub_leaf":{"start":3,"end":4},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":13,"sub_leaf":{"start":5,"end":5},"register":"EAX"},{"replacements":64,"mask":0}],[{"leaf":13,"sub_leaf":{"start":6,"end":6},"register":"EAX"},{"replacements":512,"mask":0}],[{"leaf":13,"sub_leaf":{"start":7,"end":7},"register":"EAX"},{"replacements":1024,"mask":0}],[{"leaf":13,"sub_leaf":{"start":8,"end":8},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":13,"sub_leaf":{"start":9,"end":9},"register":"EAX"},{"replacements":8,"mask":0}],[{"leaf":13,"sub_leaf":{"start":10,"end":63},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":13,"sub_leaf":{"start":2,"end":2},"register":"EBX"},{"replacements":576,"mask":0}],[{"leaf":13,"sub_leaf":{"start":3,"end":4},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":13,"sub_leaf":{"start":5,"end":5},"register":"EBX"},{"replacements":1088,"mask":0}],[{"leaf":13,"sub_leaf":{"start":6,"end":6},"register":"EBX"},{"replacements":1152,"mask":0}],[{"leaf":13,"sub_leaf":{"start":7,"end":7},"register":"EBX"},{"replacements":1664,"mask":0}],[{"leaf":13,"sub_leaf":{"start":8,"end":8},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":13,"sub_leaf":{"start":9,"end":9},"register":"EBX"},{"replacements":2688,"mask":0}],[{"leaf":13,"sub_leaf":{"start":10,"end":63},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":13,"sub_leaf":{"start":2,"end":2},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":13,"sub_leaf":{"start":3,"end":4},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":13,"sub_leaf":{"start":5,"end":5},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":13,"sub_leaf":{"start":6,"end":6},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":13,"sub_leaf":{"start":7,"end":7},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":13,"sub_leaf":{"start":8,"end":8},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":13,"sub_leaf":{"start":9,"end":9},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":13,"sub_leaf":{"start":10,"end":63},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":15,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":15,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":0}],[{"leaf":15,"sub_leaf":{"start":1,"end":1},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":15,"sub_leaf":{"start":1,"end":1},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":15,"sub_leaf":{"start":1,"end":1},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":15,"sub_leaf":{"start":1,"end":1},"register":"EDX"},{"replacements":0,"mask":0}],[{"leaf":16,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":16,"sub_leaf":{"start":1,"end":1},"register":"EAX"},{"replacements":0,"mask":31}],[{"leaf":16,"sub_leaf":{"start":1,"end":1},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":16,"sub_leaf":{"start":1,"end":1},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":16,"sub_leaf":{"start":1,"end":1},"register":"EDX"},{"replacements":0,"mask":0}],[{"leaf":16,"sub_leaf":{"start":2,"end":2},"register":"EAX"},{"replacements":0,"mask":31}],[{"leaf":16,"sub_leaf":{"start":2,"end":2},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":16,"sub_leaf":{"start":2,"end":2},"register":"EDX"},{"replacements":0,"mask":0}],[{"leaf":16,"sub_leaf":{"start":2,"end":2},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":16,"sub_leaf":{"start":3,"end":3},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":16,"sub_leaf":{"start":3,"end":3},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":16,"sub_leaf":{"start":3,"end":3},"register":"EDX"},{"replacements":0,"mask":0}],[{"leaf":16,"sub_leaf":{"start":5,"end":5},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":16,"sub_leaf":{"start":5,"end":5},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":16,"sub_leaf":{"start":5,"end":5},"register":"EDX"},{"replacements":0,"mask":0}],[{"leaf":20,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":20,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":20,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":20,"sub_leaf":{"start":1,"end":1},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":20,"sub_leaf":{"start":1,"end":1},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":21,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":4294967295}],[{"leaf":21,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":21,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":4294967295}],[{"leaf":22,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":65535}],[{"leaf":22,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":65535}],[{"leaf":22,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":65535}],[{"leaf":23,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":24,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":4294967295}],[{"leaf":24,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":4294903567}],[{"leaf":24,"sub_leaf":{"start":1,"end":4294967295},"register":"EBX"},{"replacements":0,"mask":4294903567}],[{"leaf":24,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":4294967295}],[{"leaf":24,"sub_leaf":{"start":1,"end":4294967295},"register":"ECX"},{"replacements":0,"mask":4294967295}],[{"leaf":24,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":67092991}],[{"leaf":24,"sub_leaf":{"start":1,"end":4294967295},"register":"EDX"},{"replacements":0,"mask":67092991}],[{"leaf":28,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":28,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":28,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":29,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":29,"sub_leaf":{"start":1,"end":1},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":29,"sub_leaf":{"start":1,"end":1},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":29,"sub_leaf":{"start":1,"end":1},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":30,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":30,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":30,"sub_leaf":{"start":1,"end":1},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":31,"sub_leaf":{"start":0,"end":4294967295},"register":"EAX"},{"replacements":0,"mask":31}],[{"leaf":31,"sub_leaf":{"start":0,"end":4294967295},"register":"EBX"},{"replacements":0,"mask":65535}],[{"leaf":31,"sub_leaf":{"start":0,"end":4294967295},"register":"ECX"},{"replacements":0,"mask":65535}],[{"leaf":31,"sub_leaf":{"start":0,"end":4294967295},"register":"EDX"},{"replacements":0,"mask":4294967295}],[{"leaf":32,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":32,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":33,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":33,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":33,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":0}],[{"leaf":35,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":35,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":35,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":35,"sub_leaf":{"start":1,"end":1},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":35,"sub_leaf":{"start":1,"end":1},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":35,"sub_leaf":{"start":2,"end":2},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":35,"sub_leaf":{"start":3,"end":3},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":35,"sub_leaf":{"start":4,"end":4},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":35,"sub_leaf":{"start":4,"end":4},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":35,"sub_leaf":{"start":5,"end":5},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":35,"sub_leaf":{"start":5,"end":5},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":35,"sub_leaf":{"start":5,"end":5},"register":"ECX"},{"replacements":0,"mask":0}],[{"leaf":35,"sub_leaf":{"start":5,"end":5},"register":"EDX"},{"replacements":0,"mask":0}],[{"leaf":36,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":0}],[{"leaf":36,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":2147483648,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":2147483656,"mask":0}],[{"leaf":2147483648,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":2147483648,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":4294967295}],[{"leaf":2147483648,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":4294967295}],[{"leaf":2147483649,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":268386303}],[{"leaf":2147483649,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":4026597375}],[{"leaf":2147483649,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":289,"mask":0}],[{"leaf":2147483649,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":739248128,"mask":0}],[{"leaf":2147483650,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":4294967295}],[{"leaf":2147483650,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":2147483650,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":4294967295}],[{"leaf":2147483650,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":4294967295}],[{"leaf":2147483651,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":4294967295}],[{"leaf":2147483651,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":2147483651,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":4294967295}],[{"leaf":2147483651,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":4294967295}],[{"leaf":2147483652,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":4294967295}],[{"leaf":2147483652,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":2147483652,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":4294967295}],[{"leaf":2147483652,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":4294967295}],[{"leaf":2147483654,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":4294967295}],[{"leaf":2147483655,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":0}],[{"leaf":2147483656,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":16777215}],[{"leaf":2147483656,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":0}],[{"leaf":1073741824,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":4294967295}],[{"leaf":1073741824,"sub_leaf":{"start":0,"end":0},"register":"EBX"},{"replacements":0,"mask":4294967295}],[{"leaf":1073741824,"sub_leaf":{"start":0,"end":0},"register":"ECX"},{"replacements":0,"mask":4294967295}],[{"leaf":1073741824,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":4294967295}],[{"leaf":1073741825,"sub_leaf":{"start":0,"end":0},"register":"EAX"},{"replacements":0,"mask":17039103}],[{"leaf":1073741825,"sub_leaf":{"start":0,"end":0},"register":"EDX"},{"replacements":0,"mask":1}]],"compatibility_target":[{"function":0,"index":0,"flags":0,"eax":27,"ebx":1970169159,"ecx":1818588270,"edx":1231384169},{"function":1,"index":0,"flags":0,"eax":526017,"ebx":51382272,"ecx":1996108291,"edx":260832251},{"function":2,"index":0,"flags":0,"eax":16711425,"ebx":240,"ecx":0,"edx":0},{"function":3,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":4,"index":0,"flags":1,"eax":469778721,"ebx":46137407,"ecx":63,"edx":0},{"function":4,"index":1,"flags":1,"eax":469778722,"ebx":29360191,"ecx":63,"edx":0},{"function":4,"index":2,"flags":1,"eax":469778755,"ebx":79691839,"ecx":1023,"edx":0},{"function":4,"index":3,"flags":1,"eax":470008163,"ebx":46137407,"ecx":16383,"edx":4},{"function":4,"index":4,"flags":1,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":5,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":6,"index":0,"flags":0,"eax":4,"ebx":0,"ecx":0,"edx":0},{"function":7,"index":0,"flags":1,"eax":2,"ebx":4055836649,"ecx":406871886,"edx":2885682448},{"function":7,"index":1,"flags":1,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":7,"index":2,"flags":1,"eax":0,"ebx":0,"ecx":0,"edx":1},{"function":8,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":9,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":10,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":11,"index":0,"flags":1,"eax":0,"ebx":0,"ecx":0,"edx":3},{"function":12,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":13,"index":0,"flags":1,"eax":743,"ebx":2696,"ecx":2696,"edx":0},{"function":13,"index":1,"flags":1,"eax":15,"ebx":2440,"ecx":0,"edx":0},{"function":13,"index":2,"flags":1,"eax":256,"ebx":576,"ecx":0,"edx":0},{"function":13,"index":5,"flags":1,"eax":64,"ebx":1088,"ecx":0,"edx":0},{"function":13,"index":6,"flags":1,"eax":512,"ebx":1152,"ecx":0,"edx":0},{"function":13,"index":7,"flags":1,"eax":1024,"ebx":1664,"ecx":0,"edx":0},{"function":13,"index":9,"flags":1,"eax":8,"ebx":2688,"ecx":0,"edx":0},{"function":14,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":15,"index":0,"flags":1,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":16,"index":0,"flags":1,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":17,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":18,"index":0,"flags":1,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":19,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":20,"index":0,"flags":1,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":21,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":22,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":23,"index":0,"flags":1,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":24,"index":0,"flags":1,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":25,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":26,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":27,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":1073741824,"index":0,"flags":0,"eax":1073741825,"ebx":1263359563,"ecx":1447775574,"edx":77},{"function":1073741825,"index":0,"flags":0,"eax":16809723,"ebx":0,"ecx":0,"edx":0},{"function":2147483648,"index":0,"flags":0,"eax":2147483656,"ebx":0,"ecx":0,"edx":0},{"function":2147483649,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":289,"edx":739248128},{"function":2147483650,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":2147483651,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":2147483652,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":2147483653,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":2147483654,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":16805952,"edx":0},{"function":2147483655,"index":0,"flags":0,"eax":0,"ebx":0,"ecx":0,"edx":0},{"function":2147483656,"index":0,"flags":0,"eax":12327,"ebx":0,"ecx":0,"edx":0}]} No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"compatibility_target":[{"function":0,"index":0,"flags":0,"eax":27,"ebx":1970169159,"ecx":1818588270,"edx":1231384169},{"function":1,"index":0,"flags":0,"eax":526017,"ebx":51382272,"ecx":1996108291,"edx":260832251}, ...
What are the compatibility_target values needed for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thing I wonder: Does the profile contain the CPU model name? On our Dell Box there are following outputs in cat /proc/cpuinfo:
model : 85
model name : Intel(R) Xeon(R) Gold 6154 CPU @ 3.00GHz
Do we also set those via the profile?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"compatibility_target":[{"function":0,"index":0,"flags":0,"eax":27,"ebx":1970169159,"ecx":1818588270,"edx":1231384169},{"function":1,"index":0,"flags":0,"eax":526017,"ebx":51382272,"ecx":1996108291,"edx":260832251}, ...What are the compatibility_target values needed for?
They are used to check that a guest with a chosen CPU profile can indeed (theoretically) migrate to the compatibility target (the host that generated the CPU profile). Any additional configuration (such as if amx is to be supported or not) is taken into account when this check is performed at runtime and this has to remain possible for any other optional feature if/when it gets added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thing I wonder: Does the profile contain the CPU model name? On our Dell Box there are following outputs in
cat /proc/cpuinfo:model : 85 model name : Intel(R) Xeon(R) Gold 6154 CPU @ 3.00GHzDo we also set those via the profile?
Currently it does: The leaf 0x1 register EAX contains Type, Family, Model, and Stepping ID extracted from the compatibility target, while 0x80000002 - 0x80000004 will show the processor brand string extracted from the host (not the compatibility target).
I think both of these choices are bad as I fear some software may only check for this/these values through CPUID and have a hardcoded available features table as an optimization. Such software will not reliably work with CPU Profiles and we should make sure these optimization attempts fail early. Hence I would suggest we do one of the following for leaves with CPU model information:
- We zero these out for any CPU Profile (except host).
- We inject something simple like "CHV CPU PROFILE".
- We inject the name of the CPU profile. For example "CHV-INTEL-SKYLAKE-PROFILE".
I think 1 or 2 would be best. I fear with number 3 we might not have enough bytes for some profiles.
Do you know if there exists critical software components that will fail to run if we simply zero out this information?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@olivereanderson I think we should handle this similar to what qemu does (which I also don't know right now, but we can test it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I can gleam here it seems like QEMU does set the same values as the compatibility target for the family, model and stepping id.
|
Can you tell us something about how the migration compatibility is checked currently (meaning prior to this change)? Are those compatibility checks now happening after the values are adjusted both on the sender and receiver side? Are sender and receiver side expected to be the same CHV version, so that it is guaranteed that they have the same data behind some CPU profile. What I am after is: Are all changes to a CPU profile breaking changes? Do we have some workflow to do later changes to a CPU profile? In general: I think the current approach with having the profile policies for all the CPUID leaves, subleaves and registers is very flexible while a human still can find out what adjustments are done at a single place. I still cannot quite wrap my head around if we are now able to make changes to those policies and what it means to change a CPU profile, once it is deployed and used by VMs. I think it would be a good idea to schedule a meeting with relevant people to have some guided review session of this PR. This way, we can quickly discuss and resolve issues and hopefully end the meeting with a list of things to adjust and after that the PR is done. Otherwise, I see the potential for endless discussions here. All in all: Very good job. This looks like a solid approach :) |
The current cpuid checks are listed here: https://github.com/cloud-hypervisor/cloud-hypervisor/blob/main/arch/src/x86_64/mod.rs#L348-L460 The checks are indeed now happening after the values have been adjusted on both the sender and receiver side. With regards to forwards/backwards compatibility I see it as follows: Policies can be changed without problems as long as we don't regenerate the profiles. If we do want to regenerate a profile after such a change then we need to suffix it with "V2" or something like that and keep the old one as well. If we make changes to migration compatibility requirements (and we incorporate checks based on what is listed by us, contrary to how it works today). Then we will need to version those tables and the migration receiver needs to be aware of the CHV version of the sender. Forward/backward compatibility of live migration is somewhat orthogonal to the CPU profiles feature though as checking the sender's CHV version will have to be done anyway if the maintainers ever change the existing cpuid compatibility checks (and want migration to be forward compatible in the sense that if migration from CHV version A to CHV version A works then it must also work from version A to version B whenever B >=A).
I agree 👍 |
a9c1c5c to
709f01d
Compare
|
ea5d359 to
5de3032
Compare
ff19943 to
6b63b22
Compare
These data structures will be used when generating CPU profiles and some are also necessary when applying a pre-generated profile. Signed-Off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de> On-behalf-of: SAP oliver.anderson@sap.com
We want CPU profiles to keep a record of the hypervisor type and cpu vendor that they are intended to work with. This is made more convenient if all of these types implement common traits (used for serialization). Signed-Off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de> On-behalf-of: SAP oliver.anderson@sap.com
We introduce essential data structures together with basic functionality that is necessary to apply a CPU profile to a host. Signed-Off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de> On-behalf-of: SAP oliver.anderson@sap.com
We integrate the CPU profile into the various configs that ultimately get set by the user. This quickly ends up involving multiple files, luckily Rust helps us find which ones via compilation errors. Signed-Off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de> On-behalf-of: SAP oliver.anderson@sap.com
If a CPU profile is configured it should result in guests seeing a restricted subset of CPUID. This is what we finally achieve in this commit. Signed-Off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de> On-behalf-of: SAP oliver.anderson@sap.com
Currently when the user configures AMX the corresponding state components get dynamically enabled directly inside the function body of vmm::cpu::CpuManager::new. With our ongoing work on CPU templates/profiles, there will (likely) be one more binary crate for producing CPU profiles that also needs to do this (without creating a CpuManager) and it may also be the case that we will need to call this function prior to `CpuManager::new` during live migrations. We thus add a method for enabling the AMX tile state components on the hypervisor trait that may be called wherever necessary. We argue that this is beneficial for code clarity independently of the upcoming CPU templates/profiles PR that we are working on. The astute reader will notice that the logic introduced here is not 100% the same as what is done inside the vmm::cpu::Cpumanager::new method. We claim that our approach is more in-line with the official documentation. Signed-off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de> On-behalf-of: SAP oliver.anderson@sap.com
We enable AMX tile state components via the hypervisor (as introduced in the previous commit) instead of doing this inline in the body of `CpuManager::new`. Signed-off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de> On-behalf-of: SAP oliver.anderson@sap.com
Signed-off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de> On-behalf-of: SAP oliver.anderson@sap.com
861354b to
4733b20
Compare
This PR introduces the concept of a CPU profile, a mechanism to opt-in to restricting CPU features a guest may use which then in turn makes live migration between hosts running on different hardware more tractable. In other words we are trying to introduce Cloud Hypervisor's analogue of QEMU's CPU models.
We restrict our scope to enforcing CPUID compliance for now and leave MSR restrictions for later.
I encourage reviewers to read the longer explanation below before starting your review. When you have read the description in its entirety you can start reviewing one commit at a time (which I have tried to make as nice as I can, although there are probably some things that could have been done better with fancy git magic).
The feature in more detail
Why do we even need this?
Recall that software is usually developed to run on a variety of processors with various features. In order for the software to dynamically discover which hardware features may be utilized one typically uses the CPUID instruction to query the CPU for information. In some cases one can also obtain CPU information via so called MSRs (model specific registers), but we will leave those out of this discussion for the time being.
Consider now the case that a guest is running some workload on host A which then gets migrated to host B where host B has a different processor than A. If this is a live migration (i.e. it is performed while the software is running), then the guest's workload can easily run into a time of check to time of use error.
Luckily hypervisors are able to manipulate what CPUID returns to guests when called which we can and will take advantage of in order to make live migration safer. Indeed if there is a subset of CPU features and properties that
are shared by all CPUs in a cluster, then if all hosts restrict themselves to that subset then live migration suddenly becomes a lot less problematic (although still not an entirely solved problem).
CPU profiles at a very high level
Using an existing CPU profile
In this PR we patch cloud hypervisor so that users may specify a CPU profile on the command line. This will in turn restrict what CPUID values the guest obtains and may thus affect functionality and performance, but may still be the best tool in the box when live migration to (subtly) different hardware is desired.
From a user perspective one simply includes the
profile=<desired cpu profile>argument to thecpusparameter e.g.in order to utilize the temporary development laptop profile (which is not intended for production). If the host has hardware considered to be compatible with the chosen profile, then cloud hypervisor should work just like before, with the exception that guest's may see certain other values when inspecting CPUID.
Producing a new CPU profile
New CPU profiles can be generated in a semi automated fashion. All you need to do is to run the newly introduced binary
on the host you are interested in being compatible with (the compatibility target) and then you need to manually edit the
arch::x86_64::cpu_profile::CpuProfileenum by adding another variant and updating thedatamethod to deserialize the pre-generated json.The resulting CPU profile should expose a lot of the same functionality as the host where it was generated, but we apply a few extra restrictions to functionality that is either inherently incompatible with live migration, or should not be used in a cloud setting for other reasons. There is currently no way for users to opt-out of these extra restrictions neither during profile generation, nor later when the profile is loaded on a new host.
Note that we also currently only support Intel CPUs and KVM as the hypervisor, but a lot of the logic is agnostic of these things and it should be relatively easy to lift these limitations (more on that later).
The design in a bit of detail
The implementation is based on the understanding that we do not only need to work with bit sets, but we also need to manipulate some multi-bit values. Indeed, some CPUID output values indicate how many leaves there are (such as for instance leaf 0, sub-leaf 0, EAX), while others may tell you the number of sub-leaves, or the size of a state component for instance. Note that if the CPUID instruction is executed with an invalid leaf, some processors will return the data for the highest basic information leaf. In a live-migration setting this could easily lead to a time of check to time of use error!
Since we cannot simply deal with bit sets (at least to the best of our understanding) we unfortunately end up with a rather more complex solution than what we had initially hoped for.
CPU Profile policies
The idea is that we have a static list describing all known values one can obtain from CPUID (on an Intel CPU) and also such a list of the values defined by KVM (within the leaf range reserved for hypervisors).
Although AMDs CPUID descriptions mostly agree with those from Intel, there are exceptions, such as for instance which XSAVE state components corresponds to avx-512 register state. Hence in order to not complicate matters more we decided to focus on Intel for now. If/when we are to include support for AMD I recommend having a separate list describing AMDs values, even if it ends up having say 80% overlap with Intel.
Now for each of these described CPUID values we decide on a CPU profile policy. These (currently) include:
From these policies we generate and serialize CPU profile data which is then later loaded and utilized to adjust
a guest's CPUID whenever the profile is in use.
Immediate Follow up tasks
MigrationCompatibilityRequirementsin the cpuid definitions introduced here.