Skip to content

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

kvm: implement Hyper-V enlightenments correctly #11213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5511f4e
kvm: implement Hyper-V enlightnements correctly
rohityadavcloud Jul 16, 2025
f1510c6
fix space after `fi`
rohityadavcloud Jul 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion ...rvisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2547,11 +2547,23 @@ protected void setQuotaAndPeriod(VirtualMachineTO vmTO, CpuTuneDef ctd) {
protected void enlightenWindowsVm(VirtualMachineTO vmTO, FeaturesDef features) {
if (vmTO.getOs().contains("Windows PV")) {
// If OS is Windows PV, then enable the features. Features supported on Windows 2008 and later
// 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
LibvirtVMDef.HyperVEnlightenmentFeatureDef hyv = new LibvirtVMDef.HyperVEnlightenmentFeatureDef();
hyv.setFeature("relaxed", true);
hyv.setFeature("vapic", true);
hyv.setFeature("spinlocks", true);
hyv.setRetries(8096);
hyv.setRetries(8191);
hyv.setFeature("vendor_id", true);
hyv.setFeature("vpindex", true);
hyv.setFeature("runtime", true);
hyv.setFeature("synic", true);
hyv.setFeature("frequencies", true);
hyv.setFeature("reset", true);
hyv.setFeature("tlbflush", true);
hyv.setFeature("reenlightenment", true);
hyv.setFeature("stimer", true);
hyv.setFeature("ipi", true);
hyv.setFeature("evmcs", true);
Copy link
Member

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hyperv on non-intel-like processors is maybe not so big an issue. We can

  1. surround with extra code,
  2. add docs
  3. never mind that possibility
  4. drop this PR
    I am leaning to the higher numbers, here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not about hyper-v, this is about hyper-v emulation on KVM itself. This PR isn't necessary given Vishesh's comments.

features.addHyperVFeature(hyv);
LOGGER.info("Enabling KVM Enlightment Features to VM domain " + vmTO.getUuid());
}
Expand Down
22 changes: 18 additions & 4 deletions plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,18 @@ public static class HyperVEnlightenmentFeatureDef {
enum Enlight {
RELAX("relaxed"),
VAPIC("vapic"),
SPIN("spinlocks");
SPIN("spinlocks"),
VENDOR_ID("vendor_id"),
VPINDEX("vpindex"),
RUNTIME("runtime"),
SYNIC("synic"),
FREQ("frequencies"),
RESET("reset"),
TLBFLUSH("tlbflush"),
REENLIGHTEN("reenlightenment"),
STIMER("stimer"),
IPI("ipi"),
EVMCS("evmcs");

private final String featureName;
Enlight(String featureName) { this.featureName = featureName; }
Expand Down Expand Up @@ -379,10 +390,13 @@ public String toString() {
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");
Comment on lines 390 to +399
Copy link
Contributor

@DaanHoogland DaanHoogland Jul 16, 2025 •
edited
Loading

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”);
               }

Copy link
Member Author

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.

Copy link
Contributor

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.

}
feaBuilder.append("</hyperv>\n");
return feaBuilder.toString();
Expand Down
Loading