From: Wim ten Have <wim.ten.have@oracle.com>
Per xen-xl conversions from and to native under host-passthrough
mode we take care for Xen (nestedhvm = mode) applied and inherited
settings generating or processing correct feature policy:
[On Intel (VT-x) architectures]
<feature policy='disable' name='vmx'/>
or
[On AMD (AMD-V) architectures]
<feature policy='disable' name='svm'/>
It will then generate (or parse) for nestedhvm=1 in/from xl format.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com>
---
cfg.mk | 2 +-
src/xenconfig/xen_xl.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 65 insertions(+), 1 deletion(-)
diff --git a/cfg.mk b/cfg.mk
index 69e3f3a..32c3725 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -777,7 +777,7 @@ sc_prohibit_cross_inclusion:
locking/) safe="($$dir|util|conf|rpc)";; \
cpu/| network/| node_device/| rpc/| security/| storage/) \
safe="($$dir|util|conf|storage)";; \
- xenapi/ | xenconfig/ ) safe="($$dir|util|conf|xen)";; \
+ xenapi/ | xenconfig/ ) safe="($$dir|util|conf|xen|cpu)";; \
*) safe="($$dir|$(mid_dirs)|util)";; \
esac; \
in_vc_files="^src/$$dir" \
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 74f68b3..62af8b8 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -34,6 +34,7 @@
#include "virstoragefile.h"
#include "xen_xl.h"
#include "libxl_capabilities.h"
+#include "cpu/cpu.h"
#define VIR_FROM_THIS VIR_FROM_XENXL
@@ -106,6 +107,7 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
const char *bios;
const char *boot;
+ int val = 0;
if (xenConfigGetString(conf, "bios", &bios, NULL) < 0)
return -1;
@@ -164,6 +166,40 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
}
def->os.nBootDevs++;
}
+
+ if (xenConfigGetBool(conf, "nestedhvm", &val, -1) < 0)
+ return -1;
+
+ if (val != -1) {
+ virCPUDefPtr cpu = NULL;
+
+ if (VIR_ALLOC(cpu) < 0)
+ return -1;
+
+ if (val == 0) {
+ bool isVTx = true;
+
+ if (VIR_ALLOC(cpu->features) < 0) {
+ VIR_FREE(cpu);
+ return -1;
+ }
+
+ if (caps && caps->host.cpu && ARCH_IS_X86(def->os.arch))
+ isVTx = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx");
+
+ if (VIR_STRDUP(cpu->features->name, isVTx ? "vmx" : "svm") < 0) {
+ VIR_FREE(cpu->features);
+ VIR_FREE(cpu);
+ return -1;
+ }
+ cpu->features->policy = VIR_CPU_FEATURE_DISABLE;
+ cpu->nfeatures = cpu->nfeatures_max = 1;
+ }
+ cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
+ cpu->type = VIR_CPU_TYPE_GUEST;
+ def->cpu = cpu;
+ }
+
} else {
if (xenConfigCopyStringOpt(conf, "bootloader", &def->os.bootloader) < 0)
return -1;
@@ -897,6 +933,34 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
if (xenConfigSetString(conf, "boot", boot) < 0)
return -1;
+ if (def->cpu &&
+ def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
+ bool hasHwVirt = true;
+
+ if (def->cpu->nfeatures) {
+ for (i = 0; i < def->cpu->nfeatures; i++) {
+
+ switch (def->cpu->features[i].policy) {
+ case VIR_CPU_FEATURE_DISABLE:
+ case VIR_CPU_FEATURE_FORBID:
+ if (STREQ(def->cpu->features[i].name, "vmx") ||
+ STREQ(def->cpu->features[i].name, "svm"))
+ hasHwVirt = false;
+ break;
+
+ case VIR_CPU_FEATURE_FORCE:
+ case VIR_CPU_FEATURE_REQUIRE:
+ case VIR_CPU_FEATURE_OPTIONAL:
+ case VIR_CPU_FEATURE_LAST:
+ break;
+ }
+ }
+ }
+
+ if (xenConfigSetInt(conf, "nestedhvm", hasHwVirt) < 0)
+ return -1;
+ }
+
/* XXX floppy disks */
} else {
if (def->os.bootloader &&
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Wim Ten Have wrote:
> From: Wim ten Have <wim.ten.have@oracle.com>
>
> Per xen-xl conversions from and to native under host-passthrough
> mode we take care for Xen (nestedhvm = mode) applied and inherited
> settings generating or processing correct feature policy:
>
> [On Intel (VT-x) architectures]
> <feature policy='disable' name='vmx'/>
>
> or
>
> [On AMD (AMD-V) architectures]
> <feature policy='disable' name='svm'/>
>
> It will then generate (or parse) for nestedhvm=1 in/from xl format.
>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: Wim ten Have <wim.ten.have@oracle.com>
> ---
> cfg.mk | 2 +-
> src/xenconfig/xen_xl.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/cfg.mk b/cfg.mk
> index 69e3f3a..32c3725 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -777,7 +777,7 @@ sc_prohibit_cross_inclusion:
> locking/) safe="($$dir|util|conf|rpc)";; \
> cpu/| network/| node_device/| rpc/| security/| storage/) \
> safe="($$dir|util|conf|storage)";; \
> - xenapi/ | xenconfig/ ) safe="($$dir|util|conf|xen)";; \
> + xenapi/ | xenconfig/ ) safe="($$dir|util|conf|xen|cpu)";; \
It would be nice to get another libvirt dev opinion on this change. As I said in
the V2 thread, it seems we could remove xenconfig from this check.
> *) safe="($$dir|$(mid_dirs)|util)";; \
E.g. let it be handled in this case.
> esac; \
> in_vc_files="^src/$$dir" \
> diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> index 74f68b3..62af8b8 100644
> --- a/src/xenconfig/xen_xl.c
> +++ b/src/xenconfig/xen_xl.c
> @@ -34,6 +34,7 @@
> #include "virstoragefile.h"
> #include "xen_xl.h"
> #include "libxl_capabilities.h"
> +#include "cpu/cpu.h"
>
> #define VIR_FROM_THIS VIR_FROM_XENXL
>
> @@ -106,6 +107,7 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
> if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
> const char *bios;
> const char *boot;
> + int val = 0;
>
> if (xenConfigGetString(conf, "bios", &bios, NULL) < 0)
> return -1;
> @@ -164,6 +166,40 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
> }
> def->os.nBootDevs++;
> }
> +
> + if (xenConfigGetBool(conf, "nestedhvm", &val, -1) < 0)
> + return -1;
> +
> + if (val != -1) {
> + virCPUDefPtr cpu = NULL;
> +
> + if (VIR_ALLOC(cpu) < 0)
> + return -1;
> +
> + if (val == 0) {
> + bool isVTx = true;
> +
> + if (VIR_ALLOC(cpu->features) < 0) {
> + VIR_FREE(cpu);
> + return -1;
> + }
> +
> + if (caps && caps->host.cpu && ARCH_IS_X86(def->os.arch))
> + isVTx = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx");
> +
> + if (VIR_STRDUP(cpu->features->name, isVTx ? "vmx" : "svm") < 0) {
> + VIR_FREE(cpu->features);
> + VIR_FREE(cpu);
> + return -1;
So if I understand this correctly, the feature would have the name "vmx" if arch
!= x86. If arch == x86 but feature "vmx" is not found, then the feature name
would be "svm".
IMO, it would be better to ignore <cpu> altogether if we can't find the name of
the virt technology feature to disable. Without a <cpu> def, you'd get the libxl
default, which is nestedhvm=disabled (and also the current behavior of
libvirt+libxl). E.g. what do you think of the below diff to your patch?
Regards,
Jim
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index c536e57a0..4f24d457c 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -170,36 +170,48 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def,
virCapsPtr caps)
if (xenConfigGetBool(conf, "nestedhvm", &val, -1) < 0)
return -1;
- if (val != -1) {
- virCPUDefPtr cpu = NULL;
+ if (val == 1) {
+ virCPUDefPtr cpu;
if (VIR_ALLOC(cpu) < 0)
return -1;
- if (val == 0) {
- bool isVTx = true;
+ cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
+ cpu->type = VIR_CPU_TYPE_GUEST;
+ def->cpu = cpu;
+ } else if (val == 0) {
+ const char *vtfeature = NULL;
+
+ if (caps && caps->host.cpu && ARCH_IS_X86(def->os.arch)) {
+ if (virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx"))
+ vtfeature = "vmx";
+ else if (virCPUCheckFeature(caps->host.arch, caps->host.cpu,
"svm"))
+ vtfeature = "svm";
+ }
+
+ if (vtfeature) {
+ virCPUDefPtr cpu;
+
+ if (VIR_ALLOC(cpu) < 0)
+ return -1;
if (VIR_ALLOC(cpu->features) < 0) {
VIR_FREE(cpu);
return -1;
}
- if (caps && caps->host.cpu && ARCH_IS_X86(def->os.arch))
- isVTx = virCPUCheckFeature(caps->host.arch, caps->host.cpu,
"vmx");
-
- if (VIR_STRDUP(cpu->features->name, isVTx ? "vmx" : "svm") < 0) {
+ if (VIR_STRDUP(cpu->features->name, vtfeature) < 0) {
VIR_FREE(cpu->features);
VIR_FREE(cpu);
return -1;
}
cpu->features->policy = VIR_CPU_FEATURE_DISABLE;
cpu->nfeatures = cpu->nfeatures_max = 1;
+ cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
+ cpu->type = VIR_CPU_TYPE_GUEST;
+ def->cpu = cpu;
}
- cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
- cpu->type = VIR_CPU_TYPE_GUEST;
- def->cpu = cpu;
}
-
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, 20 Apr 2017 15:40:22 -0600
Jim Fehlig <jfehlig@suse.com> wrote:
> Wim Ten Have wrote:
> > From: Wim ten Have <wim.ten.have@oracle.com>
> >
> > Per xen-xl conversions from and to native under host-passthrough
> > mode we take care for Xen (nestedhvm = mode) applied and inherited
> > settings generating or processing correct feature policy:
> >
> > [On Intel (VT-x) architectures]
> > <feature policy='disable' name='vmx'/>
> >
> > or
> >
> > [On AMD (AMD-V) architectures]
> > <feature policy='disable' name='svm'/>
> >
> > It will then generate (or parse) for nestedhvm=1 in/from xl format.
> >
> > Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> > Signed-off-by: Wim ten Have <wim.ten.have@oracle.com>
> > ---
> > cfg.mk | 2 +-
> > src/xenconfig/xen_xl.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 65 insertions(+), 1 deletion(-)
> >
> > diff --git a/cfg.mk b/cfg.mk
> > index 69e3f3a..32c3725 100644
> > --- a/cfg.mk
> > +++ b/cfg.mk
> > @@ -777,7 +777,7 @@ sc_prohibit_cross_inclusion:
> > locking/) safe="($$dir|util|conf|rpc)";; \
> > cpu/| network/| node_device/| rpc/| security/| storage/) \
> > safe="($$dir|util|conf|storage)";; \
> > - xenapi/ | xenconfig/ ) safe="($$dir|util|conf|xen)";; \
> > + xenapi/ | xenconfig/ ) safe="($$dir|util|conf|xen|cpu)";; \
>
> It would be nice to get another libvirt dev opinion on this change. As I said in
> the V2 thread, it seems we could remove xenconfig from this check.
>
> > *) safe="($$dir|$(mid_dirs)|util)";; \
>
> E.g. let it be handled in this case.
In that case we have to add 'xen' to "mid_dirs" above.
--- a/cfg.mk
+++ b/cfg.mk
@@ -768,7 +768,7 @@ sc_prohibit_gettext_markup:
# lower-level code must not include higher-level headers.
cross_dirs=$(patsubst $(srcdir)/src/%.,%,$(wildcard $(srcdir)/src/*/.))
cross_dirs_re=($(subst / ,/|,$(cross_dirs)))
-mid_dirs=access|conf|cpu|locking|logging|network|node_device|rpc|security|storage
+mid_dirs=access|conf|cpu|locking|logging|network|node_device|rpc|security|storage|xen
Otherwise there's various other complains. ... sound like this is a bit
deserted area. My selection to add cpu under xenapi/|xenconfig/) was to
have it at lease minimized to the world of xen arena.
> > esac; \
> > in_vc_files="^src/$$dir" \
> > diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> > index 74f68b3..62af8b8 100644
> > --- a/src/xenconfig/xen_xl.c
> > +++ b/src/xenconfig/xen_xl.c
> > @@ -34,6 +34,7 @@
> > #include "virstoragefile.h"
> > #include "xen_xl.h"
> > #include "libxl_capabilities.h"
> > +#include "cpu/cpu.h"
> >
> > #define VIR_FROM_THIS VIR_FROM_XENXL
> >
> > @@ -106,6 +107,7 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
> > if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
> > const char *bios;
> > const char *boot;
> > + int val = 0;
> >
> > if (xenConfigGetString(conf, "bios", &bios, NULL) < 0)
> > return -1;
> > @@ -164,6 +166,40 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
> > }
> > def->os.nBootDevs++;
> > }
> > +
> > + if (xenConfigGetBool(conf, "nestedhvm", &val, -1) < 0)
> > + return -1;
> > +
> > + if (val != -1) {
> > + virCPUDefPtr cpu = NULL;
> > +
> > + if (VIR_ALLOC(cpu) < 0)
> > + return -1;
> > +
> > + if (val == 0) {
> > + bool isVTx = true;
> > +
> > + if (VIR_ALLOC(cpu->features) < 0) {
> > + VIR_FREE(cpu);
> > + return -1;
> > + }
> > +
> > + if (caps && caps->host.cpu && ARCH_IS_X86(def->os.arch))
> > + isVTx = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx");
> > +
> > + if (VIR_STRDUP(cpu->features->name, isVTx ? "vmx" : "svm") < 0) {
> > + VIR_FREE(cpu->features);
> > + VIR_FREE(cpu);
> > + return -1;
>
> So if I understand this correctly, the feature would have the name "vmx" if arch
> != x86. If arch == x86 but feature "vmx" is not found, then the feature name
> would be "svm".
>
> IMO, it would be better to ignore <cpu> altogether if we can't find the name of
> the virt technology feature to disable. Without a <cpu> def, you'd get the libxl
> default, which is nestedhvm=disabled (and also the current behavior of
> libvirt+libxl). E.g. what do you think of the below diff to your patch?
Appreciate below insight and added change adding fixated cpuDefaultFeatures for
testsutils under xen.
Charm on below is that is saves us from the additional brought allocation under
VIR_STRDUP. Let me bring you PATCH v4 next Monday which also includes the signature
correction as suggested initially.
Regards,
-Wim.
> diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> index c536e57a0..4f24d457c 100644
> --- a/src/xenconfig/xen_xl.c
> +++ b/src/xenconfig/xen_xl.c
> @@ -170,36 +170,48 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def,
> virCapsPtr caps)
> if (xenConfigGetBool(conf, "nestedhvm", &val, -1) < 0)
> return -1;
>
> - if (val != -1) {
> - virCPUDefPtr cpu = NULL;
> + if (val == 1) {
> + virCPUDefPtr cpu;
>
> if (VIR_ALLOC(cpu) < 0)
> return -1;
>
> - if (val == 0) {
> - bool isVTx = true;
> + cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
> + cpu->type = VIR_CPU_TYPE_GUEST;
> + def->cpu = cpu;
> + } else if (val == 0) {
> + const char *vtfeature = NULL;
> +
> + if (caps && caps->host.cpu && ARCH_IS_X86(def->os.arch)) {
> + if (virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx"))
> + vtfeature = "vmx";
> + else if (virCPUCheckFeature(caps->host.arch, caps->host.cpu,
> "svm"))
> + vtfeature = "svm";
> + }
> +
> + if (vtfeature) {
> + virCPUDefPtr cpu;
> +
> + if (VIR_ALLOC(cpu) < 0)
> + return -1;
>
> if (VIR_ALLOC(cpu->features) < 0) {
> VIR_FREE(cpu);
> return -1;
> }
>
> - if (caps && caps->host.cpu && ARCH_IS_X86(def->os.arch))
> - isVTx = virCPUCheckFeature(caps->host.arch, caps->host.cpu,
> "vmx");
> -
> - if (VIR_STRDUP(cpu->features->name, isVTx ? "vmx" : "svm") < 0) {
> + if (VIR_STRDUP(cpu->features->name, vtfeature) < 0) {
> VIR_FREE(cpu->features);
> VIR_FREE(cpu);
> return -1;
> }
> cpu->features->policy = VIR_CPU_FEATURE_DISABLE;
> cpu->nfeatures = cpu->nfeatures_max = 1;
> + cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
> + cpu->type = VIR_CPU_TYPE_GUEST;
> + def->cpu = cpu;
> }
> - cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
> - cpu->type = VIR_CPU_TYPE_GUEST;
> - def->cpu = cpu;
> }
> -
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.