-
Notifications
You must be signed in to change notification settings - Fork 1.2k
kvm: implement Hyper-V enlightenments correctly #11213
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: 4.20
Are you sure you want to change the base?
Conversation
This implements Hyper-V enlightenments as per the RHEL docs: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/10/html/configuring_and_managing_windows_virtual_machines/optimizing-windows-virtual-machines#enabling-hyper-v-enlightenments This is enabled only when the guest OS is set to Windows PV. Signed-off-by: Rohit Yadav <[email protected]>
Signed-off-by: Rohit Yadav <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #11213 +/- ##
============================================
+ Coverage 16.14% 16.16% +0.01%
- Complexity 13256 13284 +28
============================================
Files 5656 5656
Lines 497893 498438 +545
Branches 60374 60719 +345
============================================
+ Hits 80394 80558 +164
- Misses 408539 408934 +395
+ Partials 8960 8946 -14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
feaBuilder.append("<"); | ||
feaBuilder.append(e.getKey()); | ||
|
||
if(e.getKey().equals("spinlocks")) feaBuilder.append(" state='" + e.getValue() + "' retries='" + getRetries() + "'"); | ||
else feaBuilder.append(" state='" + e.getValue() + "'"); | ||
if (e.getKey().equals("spinlocks")) feaBuilder.append(" state='" + e.getValue() + "' retries='" + getRetries() + "'"); | ||
else if (e.getKey().equals("vendor_id")) feaBuilder.append(" state='" + e.getValue() + "' value='KVM Hv'"); | ||
else if (e.getKey().equals("stimer")) feaBuilder.append(" state='" + e.getValue() + "'><direct state='" + e.getValue() + "'/>"); | ||
else feaBuilder.append(" state='" + e.getValue() + "'"); | ||
|
||
feaBuilder.append("/>\n"); | ||
if (e.getKey().equals("stimer")) feaBuilder.append("</stimer>\n"); | ||
else feaBuilder.append("/>\n"); |
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.
looks like this could be a bit cleaner as a switch statement:
String key = e.getKey();
feaBuilder.append("<");
feaBuilder.append(key);
switch (key) {
case “spinlocks”:
feaBuilder.append(" state='" + e.getValue() + "' retries='" + getRetries() + "'/>\n”);
break;
case “vendor_id”:
feaBuilder.append(" state='" + e.getValue() + "' value='KVM Hv'/>\n");
break;
case “stimer”:
feaBuilder.append(" state='" + e.getValue() + "'><direct state='" + e.getValue() + "'/></stimer>\n");
break;
default:
feaBuilder.append(" state='" + e.getValue() + "'/>\n”);
}
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.
Agree - in fact the whole class could use refactoring. For my purpose, unfortunately these changes made no visible impact on console performance #10650 (comment)
I won't have time to refactor this today or later, but you and others are welcome to collaborate.
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 these have no results we can merge them on recomendation of redhat, but further effort along this line seems mute to me.
@vishesh92 @weizhouapache this doesn't address the console performance like I originally thought, should I keep it open or close this? cc @DaanHoogland |
@blueorangutan package |
hyv.setFeature("reenlightenment", true); | ||
hyv.setFeature("stimer", true); | ||
hyv.setFeature("ipi", true); | ||
hyv.setFeature("evmcs", true); |
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.
As per the docs "This feature is exclusive to Intel processors.". So, this could potentially cause issues with non-intel processors.
This implements Hyper-V enlightenments as per the RHEL docs: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/10/html/configuring_and_managing_windows_virtual_machines/optimizing-windows-virtual-machines#enabling-hyper-v-enlightenments
This is enabled only when the guest OS is set to Windows PV.
This PR is followup of findings and improvements from #10650 (comment)
Tested on: ACS 4.20.1 with Ubuntu 22.04/KVM with Windows 10 ISO, installed Windows 10 VM
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity