-
Notifications
You must be signed in to change notification settings - Fork 2
Cpu profiles (1/3): CPUID adjustments #50
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
base: gardenlinux
Are you sure you want to change the base?
Cpu profiles (1/3): CPUID adjustments #50
Conversation
e352825 to
323094e
Compare
Since enabling AMX tile state components affect the result returned by `Hypervisor::get_supported_cpuid` we want this enabled prior to checking CPUID compatibility between the source and destination VMs. Although this is not required today, it is necessary in order for the upcoming CPU profiles correctly, and it will also be necessary once the check_cpuid_compatibility checks are extended to take state components into account. Signed-off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de> On-behalf-of: SAP oliver.anderson@sap.com
1e7385c to
85f573c
Compare
Temporary workaround until we switch over to the WIP fix upstream Signed-off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de> On-behalf-of: SAP oliver.anderson@sap.com
a90c74b to
9ea0f49
Compare
| XSAVE_FAM_LENGTH | ||
| .set(fam_length) | ||
| .expect("This should only be set once"); | ||
| let _ = XSAVE_FAM_LENGTH.set(fam_length); |
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.
commit message Temporary workaround until we switch over to the WIP fix upstream - is this intentional?
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.
Once cloud-hypervisor#7534 is merged then we can remove this code.
phip1611
left a comment
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.
Amazing, outstanding work! Great git commit history. I left a few remarks.
|
|
||
| if !amx { | ||
| // In this case we will need to wipe out the AMX tile state components (if they are included in the profile) | ||
| for adj in data.adjustments.iter_mut() { |
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.
would it make sense to have some sort of a generic wipe out bits from CPU id function here?
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 would prefer not to make drastic changes like that at this stage. This is all rather complex and there is no clean way of doing things, at least not without refactoring the world.
One relatively simple alternative would be to have two profiles for every supported CPU (one with and one without AMX). Then this wouldn't be necessary at all, but if more complicated features like that get introduced in the future you end up with a combinatorial explosion of profiles that need to be generated.
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.
at least not without refactoring the world.
👍🏻 alright, i'm fine with it
9ea0f49 to
09b1c0e
Compare
09b1c0e to
ac4d939
Compare
These data structures are required to define CPU profiles. 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
ac4d939 to
ed96e07
Compare
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
ed96e07 to
22a2319
Compare
22a2319 to
ca3273a
Compare
ca3273a to
b1b2db6
Compare
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
We include CPU profiles corresponding to Intel Skylake and Sapphire rapids server that we generated using our WIP CPU profile generation tool. Signed-of-by: Oliver Anderson <oliver.anderson@cyberus-technology.de> On-behalf-of: SAP oliver.anderson@sap.com
b1b2db6 to
1b94318
Compare
phip1611
left a comment
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.
LGTM!
| pub enum CpuProfile { | ||
| #[default] | ||
| Host, | ||
| #[cfg(feature = "kvm")] |
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.
is it the right thing to feature gate them for KVM?
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.
Adjusting CPUID to what is set in the profile is in principle hypervisor agnostic. However, the adjustments error if the hypervisor's call to Hypervisor::get_supported_cpuid doesn't return entries for leaf values the CPU profile expects.
If there is a big drift between which CPU features are supported by KVM and MSHV, then since we are currently only generating CPU profiles with KVM as an input to the generation tool, it would then be unlikely that the CPU profiles work with the MSHV hypervisor and it is perhaps then better not to advertise them to the user in that case.
I see two ways forward:
- Generate CPU profiles with both hypervisors. We can definitely make this invisible to the user API and just select the profile corresponding to the hypervisor that is currently in use at runtime. If we choose this path then I think it is also OK that some profiles may only be available for one hypervisor.
- We hope that the two hypervisors support essentially the same set of CPU features (at least among those that make sense in terms of live migration) and we just keep generating CPU profiles with whatever KVM supports.
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.
Please reach out to Thomas and ask him about this. We do not have to take action in this PR but I think Thomas knows more background here and we can then persist this in some form of ticket
This is the first of three PRs that have been split off from #25
This PR contains all the functionality needed to launch CHV with a CPU profile (currently limited to Intel Skylake and Sapphire rapids processors), minus the fact that MSR restrictions are still missing.
The next two PRs will contain CPUID definitions and the CPU profiles generation tool respectively.
Note: This contains a lot of code, because of the generated JSON files corresponding to the CPU profiles. The JSON files are currently pretty printed in order to make debugging easier at this relatively early stage.
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 Intel Sapphire rapids server CPU profile. 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.
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). Note that this list is not included in this PR, but will be the part of our next PR.
From the aforementioned list of CPUID values we built a tool to generate CPU profiles when executed on the hardware that one wishes to be compatible with. The CPU profile generation tool (which will be revealed in a follow up PR) produces
data (currently JSON files) that we load at runtime and use to adjust the CPUID entries that guests will see when executing the CPUID instruction.
Immediate Follow up tasks
MigrationCompatibilityRequirementsin the cpuid definitions introduced here.