-
Notifications
You must be signed in to change notification settings - Fork 18
Enable all host supported processor features for guests #300
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: main
Are you sure you want to change the base?
Conversation
Move make_default_partition_create_arg into struct Mshv in preparation for the next patch that utilizes the hypercall IOCTL in constructing the create partition argument. Keep the global function as-is but mark it deprecated. Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
Do a self-query of HvPartitionPropertyProcessorFeatures to determine which features are supported on the host and present the same feature set to guests by default. As before, VMM can override these features if needed. This approach is more robust than the previous approach where we were handpicking features to enable (on arm64) or blindly enabling all features (in x86). Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
Fix the following warnings:
warning: unused import: `ioctl_with_ref`
--> mshv-ioctls/src/ioctls/vcpu.rs:15:47
|
15 | use vmm_sys_util::ioctl::{ioctl_with_mut_ref, ioctl_with_ref};
| ^^^^^^^^^^^^^^
|
warning: unused import: `std::mem`
--> mshv-ioctls/src/ioctls/vm.rs:868:9
|
868 | use std::mem;
|
Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
2dfa4f6 to
e030f8c
Compare
| ) | ||
| .unwrap(); | ||
|
|
||
| create_args.pt_cpu_fbanks[0] = !host_proc_features0; |
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.
For better understanding - why are these are inverted? Perhaps would be worth commenting. Thanks
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.
Oh right, I'll add a comment. It is inverted because in create_args we need to pass the disabled processor features i.e. if bit is 1 feature is disabled and 0 is enabled.
| #[cfg(target_arch = "x86_64")] | ||
| // SAFETY: access union fields | ||
| unsafe { | ||
| let mut disabled_xsave_features = hv_partition_processor_xsave_features::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.
Shouldn't this query the xsave features similarly as it's done below for states? Thanks
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 didn't do it because I don't know what the implications would be. And below it looks like we're enabling a very selected set of features. @russell-islam what do you think?
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 am not sure about the mapping between processor features and xsave.
Summary of the PR
Query host partition processor features (using a self-query) and provide the same feature set to guests during creation. As before, the VMM is still able to override any of these flags.
This approach is more robust than the previous approach where we were handpicking features to enable (on arm64) or blindly enabling all features (in x86).
Additionally, there is also a commit in this PR to fix some build warnings on arm64.
Requirements
Before submitting your PR, please make sure you addressed the following
requirements:
git commit -s), and the commit message has max 60 characters for thesummary and max 75 characters for each description line.
test.
Release" section of CHANGELOG.md (if no such section exists, please create one).
unsafecode is properly documented.