[libvirt] [PATCH] nestedhvm for libxl
This patch adds a nestedhvm directive to the lxm features to allow libxl to create a nested HVM. It also has mask_svm_npt to allow the created HVM to have the svm/npt bits masked so that it cannot in turn run up an nested hvm. Alvin Starr (1): add nested HVM to libxl docs/schemas/domaincommon.rng | 10 ++++++++++ src/conf/domain_conf.c | 6 ++++++ src/conf/domain_conf.h | 2 ++ src/libxl/libxl_conf.c | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+) -- 2.4.3
--- docs/schemas/domaincommon.rng | 10 ++++++++++ src/conf/domain_conf.c | 6 ++++++ src/conf/domain_conf.h | 2 ++ src/libxl/libxl_conf.c | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f196177..402ff16 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4083,6 +4083,16 @@ </element> </optional> <optional> + <element name="nestedhvm"> + <empty/> + </element> + </optional> + <optional> + <element name="mask_svm_npt"> + <empty/> + </element> + </optional> + <optional> <ref name="hyperv"/> </optional> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6df1618..350729e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -140,6 +140,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "apic", "pae", "hap", + "nestedhvm", + "mask_svm_npt", "viridian", "privnet", "hyperv", @@ -15400,6 +15402,8 @@ virDomainDefParseXML(xmlDocPtr xml, case VIR_DOMAIN_FEATURE_ACPI: case VIR_DOMAIN_FEATURE_PAE: case VIR_DOMAIN_FEATURE_HAP: + case VIR_DOMAIN_FEATURE_NESTEDHVM: + case VIR_DOMAIN_FEATURE_MASK_SVM_NPT: case VIR_DOMAIN_FEATURE_VIRIDIAN: case VIR_DOMAIN_FEATURE_PRIVNET: case VIR_DOMAIN_FEATURE_HYPERV: @@ -22054,6 +22058,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, case VIR_DOMAIN_FEATURE_ACPI: case VIR_DOMAIN_FEATURE_PAE: case VIR_DOMAIN_FEATURE_HAP: + case VIR_DOMAIN_FEATURE_NESTEDHVM: + case VIR_DOMAIN_FEATURE_MASK_SVM_NPT: case VIR_DOMAIN_FEATURE_VIRIDIAN: case VIR_DOMAIN_FEATURE_PRIVNET: switch ((virTristateSwitch) def->features[i]) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f043554..a71b081 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1674,6 +1674,8 @@ typedef enum { VIR_DOMAIN_FEATURE_APIC, VIR_DOMAIN_FEATURE_PAE, VIR_DOMAIN_FEATURE_HAP, + VIR_DOMAIN_FEATURE_NESTEDHVM, + VIR_DOMAIN_FEATURE_MASK_SVM_NPT, VIR_DOMAIN_FEATURE_VIRIDIAN, VIR_DOMAIN_FEATURE_PRIVNET, VIR_DOMAIN_FEATURE_HYPERV, diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 35fd495..008f339 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -486,6 +486,21 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) 0, 1) == NULL) return -1; + if (virCapabilitiesAddGuestFeature(guest, + "mask_svm_npt", + 0, + 1) == NULL) + return -1; + if (virCapabilitiesAddGuestFeature(guest, + "nestedhvm", + 0, + 1) == NULL) + return -1; + if (virCapabilitiesAddGuestFeature(guest, + "viridian", + 0, + 1) == NULL) + return -1; } } @@ -660,6 +675,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, b_info->sched_params.weight = 1000; b_info->max_memkb = virDomainDefGetMemoryInitial(def); b_info->target_memkb = def->mem.cur_balloon; + if (hvm) { char bootorder[VIR_DOMAIN_BOOT_LAST + 1]; @@ -672,6 +688,24 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_defbool_set(&b_info->u.hvm.acpi, def->features[VIR_DOMAIN_FEATURE_ACPI] == VIR_TRISTATE_SWITCH_ON); + libxl_defbool_set(&b_info->u.hvm.viridian, + def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] == + VIR_TRISTATE_SWITCH_ON); + libxl_defbool_set(&b_info->u.hvm.nested_hvm, + def->features[VIR_DOMAIN_FEATURE_NESTEDHVM] == + VIR_TRISTATE_SWITCH_ON); + if (def->features[VIR_DOMAIN_FEATURE_MASK_SVM_NPT] == + VIR_TRISTATE_SWITCH_ON) + libxl_cpuid_parse_config(&b_info->cpuid, "svm_npt=0"); + + if ((b_info->max_memkb != b_info->target_memkb) && + (def->features[VIR_DOMAIN_FEATURE_NESTEDHVM] == + VIR_TRISTATE_SWITCH_ON)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("mem and mexmem must be the same if nestedhvm is enabled")); + return -1; + } + for (i = 0; i < def->clock.ntimers; i++) { if (def->clock.timers[i]->name == VIR_DOMAIN_TIMER_NAME_HPET && def->clock.timers[i]->present == 1) { @@ -1854,6 +1888,7 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, libxl_ctx *ctx, libxl_domain_config *d_config) { + libxl_domain_config_init(d_config); if (libxlMakeDomCreateInfo(ctx, def, &d_config->c_info) < 0) -- 2.4.3
On Thu, Oct 01, 2015 at 02:08:58PM -0400, Alvin Starr wrote:
--- docs/schemas/domaincommon.rng | 10 ++++++++++ src/conf/domain_conf.c | 6 ++++++ src/conf/domain_conf.h | 2 ++ src/libxl/libxl_conf.c | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+)
I'm not the one to review the libxl part proper, but by just looking at this, it would be nice if you could split the global part (rng schema and parsing/formatting code) and the libxl-related part into separate patches. Few more hints inline.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f196177..402ff16 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4083,6 +4083,16 @@ </element> </optional> <optional> + <element name="nestedhvm"> + <empty/> + </element> + </optional> + <optional> + <element name="mask_svm_npt"> + <empty/> + </element> + </optional> + <optional> <ref name="hyperv"/> </optional> <optional>
These should be documented in docs/formatdomain.html.in as well.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6df1618..350729e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -140,6 +140,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "apic", "pae", "hap", + "nestedhvm", + "mask_svm_npt", "viridian", "privnet", "hyperv", @@ -15400,6 +15402,8 @@ virDomainDefParseXML(xmlDocPtr xml, case VIR_DOMAIN_FEATURE_ACPI: case VIR_DOMAIN_FEATURE_PAE: case VIR_DOMAIN_FEATURE_HAP: + case VIR_DOMAIN_FEATURE_NESTEDHVM: + case VIR_DOMAIN_FEATURE_MASK_SVM_NPT: case VIR_DOMAIN_FEATURE_VIRIDIAN: case VIR_DOMAIN_FEATURE_PRIVNET: case VIR_DOMAIN_FEATURE_HYPERV: @@ -22054,6 +22058,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, case VIR_DOMAIN_FEATURE_ACPI: case VIR_DOMAIN_FEATURE_PAE: case VIR_DOMAIN_FEATURE_HAP: + case VIR_DOMAIN_FEATURE_NESTEDHVM: + case VIR_DOMAIN_FEATURE_MASK_SVM_NPT: case VIR_DOMAIN_FEATURE_VIRIDIAN: case VIR_DOMAIN_FEATURE_PRIVNET: switch ((virTristateSwitch) def->features[i]) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f043554..a71b081 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1674,6 +1674,8 @@ typedef enum { VIR_DOMAIN_FEATURE_APIC, VIR_DOMAIN_FEATURE_PAE, VIR_DOMAIN_FEATURE_HAP, + VIR_DOMAIN_FEATURE_NESTEDHVM, + VIR_DOMAIN_FEATURE_MASK_SVM_NPT, VIR_DOMAIN_FEATURE_VIRIDIAN, VIR_DOMAIN_FEATURE_PRIVNET, VIR_DOMAIN_FEATURE_HYPERV, diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 35fd495..008f339 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -486,6 +486,21 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) 0, 1) == NULL) return -1; + if (virCapabilitiesAddGuestFeature(guest, + "mask_svm_npt", + 0, + 1) == NULL) + return -1; + if (virCapabilitiesAddGuestFeature(guest, + "nestedhvm", + 0, + 1) == NULL) + return -1; + if (virCapabilitiesAddGuestFeature(guest, + "viridian", + 0, + 1) == NULL) + return -1;
Also this looks like it's not related to what you are doing in this patch, so it could be split outside.
} }
@@ -660,6 +675,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, b_info->sched_params.weight = 1000; b_info->max_memkb = virDomainDefGetMemoryInitial(def); b_info->target_memkb = def->mem.cur_balloon; + if (hvm) { char bootorder[VIR_DOMAIN_BOOT_LAST + 1];
@@ -672,6 +688,24 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_defbool_set(&b_info->u.hvm.acpi, def->features[VIR_DOMAIN_FEATURE_ACPI] == VIR_TRISTATE_SWITCH_ON); + libxl_defbool_set(&b_info->u.hvm.viridian, + def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] == + VIR_TRISTATE_SWITCH_ON); + libxl_defbool_set(&b_info->u.hvm.nested_hvm, + def->features[VIR_DOMAIN_FEATURE_NESTEDHVM] == + VIR_TRISTATE_SWITCH_ON); + if (def->features[VIR_DOMAIN_FEATURE_MASK_SVM_NPT] == + VIR_TRISTATE_SWITCH_ON) + libxl_cpuid_parse_config(&b_info->cpuid, "svm_npt=0"); + + if ((b_info->max_memkb != b_info->target_memkb) && + (def->features[VIR_DOMAIN_FEATURE_NESTEDHVM] == + VIR_TRISTATE_SWITCH_ON)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("mem and mexmem must be the same if nestedhvm is enabled")); + return -1; + } + for (i = 0; i < def->clock.ntimers; i++) { if (def->clock.timers[i]->name == VIR_DOMAIN_TIMER_NAME_HPET && def->clock.timers[i]->present == 1) { @@ -1854,6 +1888,7 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, libxl_ctx *ctx, libxl_domain_config *d_config) { +
Spurious whitespace.
libxl_domain_config_init(d_config);
if (libxlMakeDomCreateInfo(ctx, def, &d_config->c_info) < 0) -- 2.4.3
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Oct 01, 2015 at 02:08:58PM -0400, Alvin Starr wrote:
--- docs/schemas/domaincommon.rng | 10 ++++++++++ src/conf/domain_conf.c | 6 ++++++ src/conf/domain_conf.h | 2 ++ src/libxl/libxl_conf.c | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f196177..402ff16 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4083,6 +4083,16 @@ </element> </optional> <optional> + <element name="nestedhvm"> + <empty/> + </element> + </optional> + <optional> + <element name="mask_svm_npt"> + <empty/> + </element> + </optional> + <optional> <ref name="hyperv"/> </optional> <optional>
We really shouldn't be adding new schema for this. We should just toggle enablement of nested SVM/VMX based on whether the <cpu> model includes the <svm/> or <vmx/> feature flags. This is what we already do on KVM for enabling this feature, and we'd want Xen to work the same way IMHO. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
I wondered about this. I first looked at using the CPU feature bit manipulation but that was, at the time, completely unimplemented. The CPU feature code also looked to be somewhat KVM centric. In the end I went for the simple implementation because the complexity of implementing the feature code would mean that I would need to become a libvirt developer and I mostly just want to be a helpful libvirt user. On 10/02/2015 08:18 AM, Daniel P. Berrange wrote:
On Thu, Oct 01, 2015 at 02:08:58PM -0400, Alvin Starr wrote:
--- docs/schemas/domaincommon.rng | 10 ++++++++++ src/conf/domain_conf.c | 6 ++++++ src/conf/domain_conf.h | 2 ++ src/libxl/libxl_conf.c | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f196177..402ff16 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4083,6 +4083,16 @@ </element> </optional> <optional> + <element name="nestedhvm"> + <empty/> + </element> + </optional> + <optional> + <element name="mask_svm_npt"> + <empty/> + </element> + </optional> + <optional> <ref name="hyperv"/> </optional> <optional> We really shouldn't be adding new schema for this. We should just toggle enablement of nested SVM/VMX based on whether the <cpu> model includes the <svm/> or <vmx/> feature flags. This is what we already do on KVM for enabling this feature, and we'd want Xen to work the same way IMHO.
Regards, Daniel
-- Alvin Starr || voice: (905)513-7688 Netvel Inc. || Cell: (416)806-0133 alvin@netvel.net ||
On Fri, Oct 02, 2015 at 09:50:26AM -0400, Alvin Starr wrote:
I wondered about this.
I first looked at using the CPU feature bit manipulation but that was, at the time, completely unimplemented. The CPU feature code also looked to be somewhat KVM centric.
In the end I went for the simple implementation because the complexity of implementing the feature code would mean that I would need to become a libvirt developer and I mostly just want to be a helpful libvirt user.
Yep, I can certainly understand that POV. Unfortunately I think in this case we really do need to do it the right way. IIUC, Xen does indeed allow the CPUID mask to be configured, so it ought to be possible to map from the libvirt XML to the CPUID mask format. It would certainly be a non-negligble bit of work though. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
I agree that it needs to be done the right way. I am willing to help where I can but my basic knowledge of the code is badly lacking so there is no reasonable way for me to contribute in a significant way. I am ok keeping my own hacks to the code that let me work along on my projects and hope that some day this will get fixed. On 10/02/2015 10:03 AM, Daniel P. Berrange wrote:
I wondered about this.
I first looked at using the CPU feature bit manipulation but that was, at the time, completely unimplemented. The CPU feature code also looked to be somewhat KVM centric.
In the end I went for the simple implementation because the complexity of implementing the feature code would mean that I would need to become a libvirt developer and I mostly just want to be a helpful libvirt user. Yep, I can certainly understand that POV. Unfortunately I think in this case we really do need to do it the right way. IIUC, Xen does indeed allow
On Fri, Oct 02, 2015 at 09:50:26AM -0400, Alvin Starr wrote: the CPUID mask to be configured, so it ought to be possible to map from the libvirt XML to the CPUID mask format. It would certainly be a non-negligble bit of work though.
Regards, Daniel
-- Alvin Starr || voice: (905)513-7688 Netvel Inc. || Cell: (416)806-0133 alvin@netvel.net ||
I have been looking at this once again and thinking about this whole implementation. I can agree with Daniel's assesment of the SVM/NPT bit setting but the nested_hvm feature does not exist at all in KVM at the libvirt level/ I could be wrong but when I last looked nested hvm could only be enabled/disabled in KVM at boot time and not on a per VM basis. What is the accepted policy on features that are only available in a single VM engine? I could remove the SVM/NPT bit twidling and resbumit the patch if it is likely to go somewhere? On 10/02/2015 02:44 PM, Alvin Starr wrote:
I agree that it needs to be done the right way.
I am willing to help where I can but my basic knowledge of the code is badly lacking so there is no reasonable way for me to contribute in a significant way.
I am ok keeping my own hacks to the code that let me work along on my projects and hope that some day this will get fixed.
On 10/02/2015 10:03 AM, Daniel P. Berrange wrote:
I wondered about this.
I first looked at using the CPU feature bit manipulation but that was, at the time, completely unimplemented. The CPU feature code also looked to be somewhat KVM centric.
In the end I went for the simple implementation because the complexity of implementing the feature code would mean that I would need to become a libvirt developer and I mostly just want to be a helpful libvirt user. Yep, I can certainly understand that POV. Unfortunately I think in this case we really do need to do it the right way. IIUC, Xen does indeed allow
On Fri, Oct 02, 2015 at 09:50:26AM -0400, Alvin Starr wrote: the CPUID mask to be configured, so it ought to be possible to map from the libvirt XML to the CPUID mask format. It would certainly be a non-negligble bit of work though.
Regards, Daniel
-- Alvin Starr || voice: (905)513-7688 Netvel Inc. || Cell: (416)806-0133 alvin@netvel.net ||
participants (3)
-
Alvin Starr -
Daniel P. Berrange -
Martin Kletzander