8:10 p.m.
Add support for CPUID setting based on <cpu> element. Since libxl format
support only adjusting specific bits over host CPU, only
mode='host-passthrough' is supported - other values are rejected (including
default 'custom'). This will break some configurations working before (bare
<cpu> element with for example NUMA configuration), but libxl driver never
supported full 'custom' mode - it was silently ignored, which might lead to
some unexpected effects.
Since mode='host-passthrough' is now necessary to specify CPU options, do not
enable nested HVM feature by mere presence of this element, require explicit
enabling "vmx" or "svm" bits. Nested HVM is still in
"preview" state, so better
be explicit here.
v2 of this patch series:
https://www.redhat.com/archives/libvir-list/2017-July/msg00050.html
Marek Marczykowski-Górecki (6):
libxl: error out on not supported CPU mode, instead of silently ignoring
libxl: do not enable nested HVM by mere presence of <cpu> element
libxl: add support for CPUID features policy
tests: check CPU features handling in libxl driver
xenconfig: add CPUID handling to domXML <-> xl.cfg conversion
tests: add test case for CPUID in xenconfig driver
src/libxl/libxl_conf.c | 53 +-
src/xenconfig/xen_xl.c | 260 ++++++++-
src/xenconfig/xen_xl.h | 2 +-
tests/libxlxml2domconfigdata/fullvirt-cpuid.json | 64 ++-
tests/libxlxml2domconfigdata/fullvirt-cpuid.xml | 37 +-
tests/libxlxml2domconfigdata/vnuma-hvm.json | 1 +-
tests/libxlxml2domconfigtest.c | 1 +-
tests/xlconfigdata/test-fullvirt-cpuid.cfg | 25 +-
tests/xlconfigdata/test-fullvirt-cpuid.xml | 36 +-
tests/xlconfigdata/test-fullvirt-nestedhvm.xml | 4 +-
tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml | 2 +-
tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml | 2 +-
tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml | 2 +-
tests/xlconfigdata/test-fullvirt-vnuma.xml | 2 +-
tests/xlconfigtest.c | 1 +-
15 files changed, 453 insertions(+), 39 deletions(-)
create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.json
create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
create mode 100644 tests/xlconfigdata/test-fullvirt-cpuid.cfg
create mode 100644 tests/xlconfigdata/test-fullvirt-cpuid.xml
base-commit: 984c534a3f2219444f4cb4df61d77b8c6e5054d7
--
git-series 0.9.1
8:10 p.m.
New subject: [libvirt] [PATCH v3 1/6] libxl: error out on not supported CPU mode, instead of silently ignoring
This change make libvirt XML with plain <cpu> element invalid for libxl,
which affect not only upcoming CPUID support, but also NUMA. In fact,
default mode 'custom' does not match what the driver actually does, so
it was a bug. Adjust xenconfig driver accordingly.
But nevertheless this commit break some configurations that were working
before.
---
Changes since v2:
- change separated from 'libxl: add support for CPUID features policy'
---
src/libxl/libxl_conf.c | 10 ++++++++--
src/xenconfig/xen_xl.c | 1 +-
tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml | 2 +-
tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml | 2 +-
tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml | 2 +-
tests/xlconfigdata/test-fullvirt-vnuma.xml | 2 +-
6 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 970cff2..f39e783 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -353,11 +353,17 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
def->features[VIR_DOMAIN_FEATURE_ACPI] ==
VIR_TRISTATE_SWITCH_ON);
- if (caps &&
- def->cpu && def->cpu->mode ==
(VIR_CPU_MODE_HOST_PASSTHROUGH)) {
+ if (caps && def->cpu) {
bool hasHwVirt = false;
bool svm = false, vmx = false;
+ if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported cpu mode '%s'"),
+ virCPUModeTypeToString(def->cpu->mode));
+ return -1;
+ }
+
if (ARCH_IS_X86(def->os.arch)) {
vmx = virCPUCheckFeature(caps->host.arch, caps->host.cpu,
"vmx");
svm = virCPUCheckFeature(caps->host.arch, caps->host.cpu,
"svm");
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 532d667..9e239a7 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -492,6 +492,7 @@ xenParseXLVnuma(virConfPtr conf,
goto cleanup;
}
+ cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
cpu->type = VIR_CPU_TYPE_GUEST;
def->cpu = cpu;
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
index e3639eb..3c486ad 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
@@ -14,7 +14,7 @@
<apic/>
<pae/>
</features>
- <cpu>
+ <cpu mode='host-passthrough'>
<numa>
<cell id='0' cpus='0,11' memory='2097152'
unit='KiB'>
<distances>
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
index 9cab3ca..17c9ca5 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
@@ -14,7 +14,7 @@
<apic/>
<pae/>
</features>
- <cpu>
+ <cpu mode='host-passthrough'>
<numa>
<cell id='0' cpus='0-1' memory='2097152'
unit='KiB'/>
<cell id='1' cpus='2-3' memory='2097152'
unit='KiB'/>
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
index 084b889..291fc37 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
@@ -14,7 +14,7 @@
<apic/>
<pae/>
</features>
- <cpu>
+ <cpu mode='host-passthrough'>
<numa>
<cell id='0' cpus='0-1' memory='2097152'
unit='KiB'>
<distances>
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma.xml
b/tests/xlconfigdata/test-fullvirt-vnuma.xml
index 5368b0d..9a9f495 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma.xml
+++ b/tests/xlconfigdata/test-fullvirt-vnuma.xml
@@ -14,7 +14,7 @@
<apic/>
<pae/>
</features>
- <cpu>
+ <cpu mode='host-passthrough'>
<numa>
<cell id='0' cpus='0-1' memory='2097152'
unit='KiB'>
<distances>
--
git-series 0.9.1
6:44 p.m.
New subject: [libvirt] [PATCH v3 1/6] libxl: error out on not supported CPU mode, instead of silently ignoring
On 12/09/2017 07:10 PM, Marek Marczykowski-Górecki wrote:
This change make libvirt XML with plain <cpu> element invalid
for libxl,
which affect not only upcoming CPUID support, but also NUMA. In fact,
default mode 'custom' does not match what the driver actually does, so
it was a bug. Adjust xenconfig driver accordingly.
Should we change the default mode in the post-parse callback if NUMA or CPU
features are defined within <cpu>? That would allow existing configs to continue
working, although I doubt there are many since all of this is new to 3.10.0.
But nevertheless this commit break some configurations that were
working
before.
I guess that is fine if we explicitly require mode='host-passthrough'.
---
Changes since v2:
- change separated from 'libxl: add support for CPUID features policy'
---
src/libxl/libxl_conf.c | 10 ++++++++--
src/xenconfig/xen_xl.c | 1 +-
tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml | 2 +-
tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml | 2 +-
tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml | 2 +-
tests/xlconfigdata/test-fullvirt-vnuma.xml | 2 +-
6 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 970cff2..f39e783 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -353,11 +353,17 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
def->features[VIR_DOMAIN_FEATURE_ACPI] ==
VIR_TRISTATE_SWITCH_ON);
- if (caps &&
- def->cpu && def->cpu->mode ==
(VIR_CPU_MODE_HOST_PASSTHROUGH)) {
+ if (caps && def->cpu) {
bool hasHwVirt = false;
bool svm = false, vmx = false;
+ if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
While changing this the unneeded parens around VIR_CPU_MODE_HOST_PASSTHROUGH can
be dropped.
Regards,
Jim
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported cpu mode '%s'"),
+ virCPUModeTypeToString(def->cpu->mode));
+ return -1;
+ }
+
if (ARCH_IS_X86(def->os.arch)) {
vmx = virCPUCheckFeature(caps->host.arch, caps->host.cpu,
"vmx");
svm = virCPUCheckFeature(caps->host.arch, caps->host.cpu,
"svm");
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 532d667..9e239a7 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -492,6 +492,7 @@ xenParseXLVnuma(virConfPtr conf,
goto cleanup;
}
+ cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
cpu->type = VIR_CPU_TYPE_GUEST;
def->cpu = cpu;
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
index e3639eb..3c486ad 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
@@ -14,7 +14,7 @@
<apic/>
<pae/>
</features>
- <cpu>
+ <cpu mode='host-passthrough'>
<numa>
<cell id='0' cpus='0,11' memory='2097152'
unit='KiB'>
<distances>
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
index 9cab3ca..17c9ca5 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
@@ -14,7 +14,7 @@
<apic/>
<pae/>
</features>
- <cpu>
+ <cpu mode='host-passthrough'>
<numa>
<cell id='0' cpus='0-1' memory='2097152'
unit='KiB'/>
<cell id='1' cpus='2-3' memory='2097152'
unit='KiB'/>
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
index 084b889..291fc37 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
@@ -14,7 +14,7 @@
<apic/>
<pae/>
</features>
- <cpu>
+ <cpu mode='host-passthrough'>
<numa>
<cell id='0' cpus='0-1' memory='2097152'
unit='KiB'>
<distances>
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma.xml
b/tests/xlconfigdata/test-fullvirt-vnuma.xml
index 5368b0d..9a9f495 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma.xml
+++ b/tests/xlconfigdata/test-fullvirt-vnuma.xml
@@ -14,7 +14,7 @@
<apic/>
<pae/>
</features>
- <cpu>
+ <cpu mode='host-passthrough'>
<numa>
<cell id='0' cpus='0-1' memory='2097152'
unit='KiB'>
<distances>
8:10 p.m.
New subject: [libvirt] [PATCH v3 2/6] libxl: do not enable nested HVM by mere presence of <cpu> element
<cpu mode='host-passthrough'> element may be used to configure other
features, like NUMA, or CPUID. Do not enable nested HVM (which is in
"preview" state after all) by mere presence of
<cpu mode='host-passthrough'> element, but require explicit <feature
policy='force' name='vmx'/> (or 'svm').
Also, adjust xenconfig driver to appropriately translate to/from
nestedhvm=1.
While at it, adjust xenconfig driver to not override def->cpu if already
set elsewhere. This will help with adding cpuid support.
---
Changes since v2:
- new patch
---
src/libxl/libxl_conf.c | 10 ++-
src/xenconfig/xen_xl.c | 57 +++++++++----------
tests/libxlxml2domconfigdata/vnuma-hvm.json | 1 +-
tests/xlconfigdata/test-fullvirt-nestedhvm.xml | 4 +-
4 files changed, 40 insertions(+), 32 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index f39e783..1846109 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -355,6 +355,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
if (caps && def->cpu) {
bool hasHwVirt = false;
+ int nested_hvm = -1;
bool svm = false, vmx = false;
if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
@@ -379,18 +380,23 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
case VIR_CPU_FEATURE_FORBID:
if ((vmx && STREQ(def->cpu->features[i].name,
"vmx")) ||
(svm && STREQ(def->cpu->features[i].name,
"svm")))
- hasHwVirt = false;
+ nested_hvm = 0;
break;
case VIR_CPU_FEATURE_FORCE:
case VIR_CPU_FEATURE_REQUIRE:
+ if ((vmx && STREQ(def->cpu->features[i].name,
"vmx")) ||
+ (svm && STREQ(def->cpu->features[i].name,
"svm")))
+ nested_hvm = 1;
+ break;
case VIR_CPU_FEATURE_OPTIONAL:
case VIR_CPU_FEATURE_LAST:
break;
}
}
}
- libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt);
+ if (hasHwVirt && nested_hvm != -1)
+ libxl_defbool_set(&b_info->u.hvm.nested_hvm, nested_hvm);
}
if (def->nsounds > 0) {
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 9e239a7..317c7a0 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -170,16 +170,7 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
if (xenConfigGetBool(conf, "nestedhvm", &val, -1) < 0)
return -1;
- if (val == 1) {
- virCPUDefPtr cpu;
-
- if (VIR_ALLOC(cpu) < 0)
- return -1;
-
- cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
- cpu->type = VIR_CPU_TYPE_GUEST;
- def->cpu = cpu;
- } else if (val == 0) {
+ if (val != -1) {
const char *vtfeature = NULL;
if (caps && caps->host.cpu &&
ARCH_IS_X86(def->os.arch)) {
@@ -190,26 +181,29 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
}
if (vtfeature) {
- virCPUDefPtr cpu;
-
- if (VIR_ALLOC(cpu) < 0)
- return -1;
+ if (!def->cpu) {
+ virCPUDefPtr cpu;
+ if (VIR_ALLOC(cpu) < 0)
+ return -1;
- if (VIR_ALLOC(cpu->features) < 0) {
- VIR_FREE(cpu);
- return -1;
+ cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
+ cpu->type = VIR_CPU_TYPE_GUEST;
+ cpu->nfeatures = 0;
+ cpu->nfeatures_max = 0;
+ def->cpu = cpu;
}
- if (VIR_STRDUP(cpu->features->name, vtfeature) < 0) {
- VIR_FREE(cpu->features);
- VIR_FREE(cpu);
- return -1;
+ if (val == 0) {
+ if (virCPUDefAddFeature(def->cpu,
+ vtfeature,
+ VIR_CPU_FEATURE_DISABLE) < 0)
+ return -1;
+ } else if (val == 1) {
+ if (virCPUDefAddFeature(def->cpu,
+ vtfeature,
+ VIR_CPU_FEATURE_FORCE) < 0)
+ 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 {
@@ -1157,6 +1151,7 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
if (def->cpu &&
def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
bool hasHwVirt = true;
+ int nestedhvm = -1;
if (def->cpu->nfeatures) {
for (i = 0; i < def->cpu->nfeatures; i++) {
@@ -1166,11 +1161,15 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
case VIR_CPU_FEATURE_FORBID:
if (STREQ(def->cpu->features[i].name, "vmx")
||
STREQ(def->cpu->features[i].name,
"svm"))
- hasHwVirt = false;
+ nestedhvm = 0;
break;
case VIR_CPU_FEATURE_FORCE:
case VIR_CPU_FEATURE_REQUIRE:
+ if (STREQ(def->cpu->features[i].name, "vmx")
||
+ STREQ(def->cpu->features[i].name,
"svm"))
+ nestedhvm = 1;
+ break;
case VIR_CPU_FEATURE_OPTIONAL:
case VIR_CPU_FEATURE_LAST:
break;
@@ -1178,7 +1177,9 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
}
}
- if (xenConfigSetInt(conf, "nestedhvm", hasHwVirt) < 0)
+ if (hasHwVirt &&
+ nestedhvm != -1 &&
+ xenConfigSetInt(conf, "nestedhvm", nestedhvm) < 0)
return -1;
}
diff --git a/tests/libxlxml2domconfigdata/vnuma-hvm.json
b/tests/libxlxml2domconfigdata/vnuma-hvm.json
index 3a5071e..2437a84 100644
--- a/tests/libxlxml2domconfigdata/vnuma-hvm.json
+++ b/tests/libxlxml2domconfigdata/vnuma-hvm.json
@@ -113,7 +113,6 @@
"pae": "True",
"apic": "True",
"acpi": "True",
- "nested_hvm": "True",
"vga": {
"kind": "cirrus"
},
diff --git a/tests/xlconfigdata/test-fullvirt-nestedhvm.xml
b/tests/xlconfigdata/test-fullvirt-nestedhvm.xml
index 8c02e7a..8a55bea 100644
--- a/tests/xlconfigdata/test-fullvirt-nestedhvm.xml
+++ b/tests/xlconfigdata/test-fullvirt-nestedhvm.xml
@@ -14,7 +14,9 @@
<apic/>
<pae/>
</features>
- <cpu mode='host-passthrough'/>
+ <cpu mode='host-passthrough'>
+ <feature policy='force' name='vmx'/>
+ </cpu>
<clock offset='variable' adjustment='0' basis='utc'/>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
--
git-series 0.9.1
7:01 a.m.
New subject: [libvirt] [PATCH v3 2/6] libxl: do not enable nested HVM by mere presence of <cpu> element
[Sorry for double posting, but I mistakenly forgot to include libvirt list)
+WimT +Daniel
On 12/10/2017 02:10 AM, Marek Marczykowski-Górecki wrote:
<cpu mode='host-passthrough'> element may be used to
configure other
features, like NUMA, or CPUID. Do not enable nested HVM (which is in
"preview" state after all) by mere presence of
<cpu mode='host-passthrough'> element, but require explicit <feature
policy='force' name='vmx'/> (or 'svm').
Also, adjust xenconfig driver to appropriately translate to/from
nestedhvm=1.
While at it, adjust xenconfig driver to not override def->cpu if already
set elsewhere. This will help with adding cpuid support.
I agree with this and it was what we came up in the first version of nested hvm
support[0]. Although Daniel suggested there to use the same semantics of qemu
driver such that host-passthrough enables nested hvm without the use of:
<feature policy='require' name='vmx'/>
(I think you propose policy='force' here which is probably better suited as
opposed to policy='require')
[0] https://www.redhat.com/archives/libvir-list/2017-March/msg00330.html
Some small comments most of them nitpicks that may fall in individuals style
preference.
---
Changes since v2:
- new patch
---
src/libxl/libxl_conf.c | 10 ++-
src/xenconfig/xen_xl.c | 57 +++++++++----------
tests/libxlxml2domconfigdata/vnuma-hvm.json | 1 +-
tests/xlconfigdata/test-fullvirt-nestedhvm.xml | 4 +-
4 files changed, 40 insertions(+), 32 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index f39e783..1846109 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -355,6 +355,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
if (caps && def->cpu) {
bool hasHwVirt = false;
+ int nested_hvm = -1;
Why not assume nested_hvm set to 0 (replicating libxl behaviour of default
disabled) and then [*]
bool svm = false, vmx = false;
if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
@@ -379,18 +380,23 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
case VIR_CPU_FEATURE_FORBID:
if ((vmx && STREQ(def->cpu->features[i].name,
"vmx")) ||
(svm && STREQ(def->cpu->features[i].name,
"svm")))
- hasHwVirt = false;
You remove this, but it might be needed given that right before this chunk we
set hasHwVirt to true depending on whether host cpu supports it.
+ nested_hvm = 0;
break;
[*] Just removing the nested_hvm being set to 0 here ...
case VIR_CPU_FEATURE_FORCE:
case VIR_CPU_FEATURE_REQUIRE:
+ if ((vmx && STREQ(def->cpu->features[i].name,
"vmx")) ||
+ (svm && STREQ(def->cpu->features[i].name,
"svm")))
+ nested_hvm = 1;> + break;
... and instead simply have this one here.
case VIR_CPU_FEATURE_OPTIONAL:
case VIR_CPU_FEATURE_LAST:
break;
}
}
}
- libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt);
+ if (hasHwVirt && nested_hvm != -1)
+ libxl_defbool_set(&b_info->u.hvm.nested_hvm, nested_hvm);
An alternative would be the as to always set like previously done:
libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt && nestedHvm);
And thus not needing the if that you are preceding here.
}
if (def->nsounds > 0) {
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 9e239a7..317c7a0 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -170,16 +170,7 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
if (xenConfigGetBool(conf, "nestedhvm", &val, -1) < 0)
return -1;
- if (val == 1) {
- virCPUDefPtr cpu;
-
- if (VIR_ALLOC(cpu) < 0)
- return -1;
-
- cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
- cpu->type = VIR_CPU_TYPE_GUEST;
- def->cpu = cpu;
- } else if (val == 0) {
+ if (val != -1) {
const char *vtfeature = NULL;
if (caps && caps->host.cpu &&
ARCH_IS_X86(def->os.arch)) {
@@ -190,26 +181,29 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr
caps)
}
if (vtfeature) {
- virCPUDefPtr cpu;
-
- if (VIR_ALLOC(cpu) < 0)
- return -1;
+ if (!def->cpu) {
+ virCPUDefPtr cpu;
+ if (VIR_ALLOC(cpu) < 0)
+ return -1;
- if (VIR_ALLOC(cpu->features) < 0) {
- VIR_FREE(cpu);
- return -1;
+ cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
+ cpu->type = VIR_CPU_TYPE_GUEST;
+ cpu->nfeatures = 0;
+ cpu->nfeatures_max = 0;
+ def->cpu = cpu;
}
- if (VIR_STRDUP(cpu->features->name, vtfeature) < 0) {
- VIR_FREE(cpu->features);
- VIR_FREE(cpu);
- return -1;
+ if (val == 0) {
+ if (virCPUDefAddFeature(def->cpu,
+ vtfeature,
+ VIR_CPU_FEATURE_DISABLE) < 0)
+ return -1;
+ } else if (val == 1) {
+ if (virCPUDefAddFeature(def->cpu,
+ vtfeature,
+ VIR_CPU_FEATURE_FORCE) < 0)
+ return -1;
}
Aren't you forgetting about VIR_FREE(cpu) if virCPUDefAddFeature fails?
- 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 {
@@ -1157,6 +1151,7 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
if (def->cpu &&
def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
bool hasHwVirt = true;
+ int nestedhvm = -1;
Same comment as in the beginning regarding the initialization of nestedhvm to 0.
For consistency (since this block very similar to the previous one) we may want
to stick to nestedhvm in all similar uses of this variable. IOW using nestedhvm
instead nested_hvm in begining of the patch.
if (def->cpu->nfeatures) {
for (i = 0; i < def->cpu->nfeatures; i++) {
@@ -1166,11 +1161,15 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
case VIR_CPU_FEATURE_FORBID:
if (STREQ(def->cpu->features[i].name, "vmx")
||
STREQ(def->cpu->features[i].name,
"svm"))
- hasHwVirt = false;
+ nestedhvm = 0;
break;
case VIR_CPU_FEATURE_FORCE:
case VIR_CPU_FEATURE_REQUIRE:
+ if (STREQ(def->cpu->features[i].name, "vmx")
||
+ STREQ(def->cpu->features[i].name,
"svm"))
+ nestedhvm = 1;
+ break;
case VIR_CPU_FEATURE_OPTIONAL:
case VIR_CPU_FEATURE_LAST:
break;
@@ -1178,7 +1177,9 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
}
}
- if (xenConfigSetInt(conf, "nestedhvm", hasHwVirt) < 0)
+ if (hasHwVirt &&
+ nestedhvm != -1 &&
+ xenConfigSetInt(conf, "nestedhvm", nestedhvm) < 0)
return -1;
If 0 or 1 is used for nestedhvm values you can probably just (hasHwVirt &&
nestedhvm && ...)
}
diff --git a/tests/libxlxml2domconfigdata/vnuma-hvm.json
b/tests/libxlxml2domconfigdata/vnuma-hvm.json
index 3a5071e..2437a84 100644
--- a/tests/libxlxml2domconfigdata/vnuma-hvm.json
+++ b/tests/libxlxml2domconfigdata/vnuma-hvm.json
@@ -113,7 +113,6 @@
"pae": "True",
"apic": "True",
"acpi": "True",
- "nested_hvm": "True",
"vga": {
"kind": "cirrus"
},
diff --git a/tests/xlconfigdata/test-fullvirt-nestedhvm.xml
b/tests/xlconfigdata/test-fullvirt-nestedhvm.xml
index 8c02e7a..8a55bea 100644
--- a/tests/xlconfigdata/test-fullvirt-nestedhvm.xml
+++ b/tests/xlconfigdata/test-fullvirt-nestedhvm.xml
@@ -14,7 +14,9 @@
<apic/>
<pae/>
</features>
- <cpu mode='host-passthrough'/>
+ <cpu mode='host-passthrough'>
+ <feature policy='force' name='vmx'/>
+ </cpu>
<clock offset='variable' adjustment='0' basis='utc'/>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
7:13 a.m.
New subject: [libvirt] [PATCH v3 2/6] libxl: do not enable nested HVM by mere presence of <cpu> element
On Tue, Dec 19, 2017 at 01:01:36PM +0000, Joao Martins wrote:
[Sorry for double posting, but I mistakenly forgot to include libvirt
list)
+WimT +Daniel
On 12/10/2017 02:10 AM, Marek Marczykowski-Górecki wrote:
> <cpu mode='host-passthrough'> element may be used to configure other
> features, like NUMA, or CPUID. Do not enable nested HVM (which is in
> "preview" state after all) by mere presence of
> <cpu mode='host-passthrough'> element, but require explicit
<feature
> policy='force' name='vmx'/> (or 'svm').
> Also, adjust xenconfig driver to appropriately translate to/from
> nestedhvm=1.
>
> While at it, adjust xenconfig driver to not override def->cpu if already
> set elsewhere. This will help with adding cpuid support.
> I agree with this and it was what we came up in the first version of nested hvm
support[0]. Although Daniel suggested there to use the same semantics of qemu
driver such that host-passthrough enables nested hvm without the use of:
<feature policy='require' name='vmx'/>
Yes, the key point of libvirt is to apply consistent semantics across different
drivers, so we should not diverge betweeen QEMU & Xen in this regard.
'host-passthrough' and 'host-model' are supposed to expose *every* feature
that
the host CPUs support (except for those few which the hypervisor may block due
to ability to virtualize them).
So 'host-passthrough' is correct to automatically expose vmx/svm, without
requiring any extra <feature> element, and I don't think we can accept
this patch.
This has been the case for KVM for ages, even though it has been considered
experimental. The only slight difference is that you can block use of svm/vmx
at the host OS level via a kernel arg to the kvm modules.
If you want to not expose svm/vmx to the guest, despite it being available
in the host, then use feature policy=disble when configuring it.
(I think you propose policy='force' here which is probably
better suited as
opposed to policy='require')
It depends on what semantics the Xen hypervisor provides.
'require' means expose the feature to the guest if it is supported
by the host, but raise an error if the host doesn't support it.
'force' means expose the feature to the guest, even if the host does
not support it at all.
For HVM Xen guests there's no real distinction between these, as you
can't run an HVM Xen guest without having hardware virt in your
host. So for 'vmx' / 'svm' force/require are basically the same
thing. For other CPU feature bits they are definitely different.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
7:43 a.m.
New subject: [libvirt] [PATCH v3 2/6] libxl: do not enable nested HVM by mere presence of <cpu> element
On 12/19/2017 01:13 PM, Daniel P. Berrange wrote:
On Tue, Dec 19, 2017 at 01:01:36PM +0000, Joao Martins wrote:
> [Sorry for double posting, but I mistakenly forgot to include libvirt list)
>
> +WimT +Daniel
>
> On 12/10/2017 02:10 AM, Marek Marczykowski-Górecki wrote:
>> <cpu mode='host-passthrough'> element may be used to configure
other
>> features, like NUMA, or CPUID. Do not enable nested HVM (which is in
>> "preview" state after all) by mere presence of
>> <cpu mode='host-passthrough'> element, but require explicit
<feature
>> policy='force' name='vmx'/> (or 'svm').
>> Also, adjust xenconfig driver to appropriately translate to/from
>> nestedhvm=1.
>>
>> While at it, adjust xenconfig driver to not override def->cpu if already
>> set elsewhere. This will help with adding cpuid support.
>
> I agree with this and it was what we came up in the first version of nested hvm
> support[0]. Although Daniel suggested there to use the same semantics of qemu
> driver such that host-passthrough enables nested hvm without the use of:
>
> <feature policy='require' name='vmx'/>
Yes, the key point of libvirt is to apply consistent semantics across different
drivers, so we should not diverge betweeen QEMU & Xen in this regard.
/nods
'host-passthrough' and 'host-model' are supposed to
expose *every* feature that
the host CPUs support (except for those few which the hypervisor may block due
to ability to virtualize them).
So 'host-passthrough' is correct to automatically expose vmx/svm, without
requiring any extra <feature> element, and I don't think we can accept
this patch.
This has been the case for KVM for ages, even though it has been considered
experimental. The only slight difference is that you can block use of svm/vmx
at the host OS level via a kernel arg to the kvm modules.
Ah that's where Xen falls off a little in which there's only libxl
nested_hvm
field to control it, even though is still marked Experimental. There's no global
parameter to block it.
If you want to not expose svm/vmx to the guest, despite it being
available
in the host, then use feature policy=disble when configuring it.
> (I think you propose policy='force' here which is probably better suited as
> opposed to policy='require')
It depends on what semantics the Xen hypervisor provides.
'require' means expose the feature to the guest if it is supported
by the host, but raise an error if the host doesn't support it.
'force' means expose the feature to the guest, even if the host does
not support it at all.
For HVM Xen guests there's no real distinction between these, as you
can't run an HVM Xen guest without having hardware virt in your
host. So for 'vmx' / 'svm' force/require are basically the same
thing. For other CPU feature bits they are definitely different.
7:45 a.m.
New subject: [libvirt] [PATCH v3 2/6] libxl: do not enable nested HVM by mere presence of <cpu> element
On Tue, Dec 19, 2017 at 01:43:24PM +0000, Joao Martins wrote:
On 12/19/2017 01:13 PM, Daniel P. Berrange wrote:
> On Tue, Dec 19, 2017 at 01:01:36PM +0000, Joao Martins wrote:
>> [Sorry for double posting, but I mistakenly forgot to include libvirt list)
>>
>> +WimT +Daniel
>>
>> On 12/10/2017 02:10 AM, Marek Marczykowski-Górecki wrote:
>>> <cpu mode='host-passthrough'> element may be used to configure
other
>>> features, like NUMA, or CPUID. Do not enable nested HVM (which is in
>>> "preview" state after all) by mere presence of
>>> <cpu mode='host-passthrough'> element, but require explicit
<feature
>>> policy='force' name='vmx'/> (or 'svm').
>>> Also, adjust xenconfig driver to appropriately translate to/from
>>> nestedhvm=1.
>>>
>>> While at it, adjust xenconfig driver to not override def->cpu if already
>>> set elsewhere. This will help with adding cpuid support.
>>
>> I agree with this and it was what we came up in the first version of nested hvm
>> support[0]. Although Daniel suggested there to use the same semantics of qemu
>> driver such that host-passthrough enables nested hvm without the use of:
>>
>> <feature policy='require' name='vmx'/>
>
> Yes, the key point of libvirt is to apply consistent semantics across different
> drivers, so we should not diverge betweeen QEMU & Xen in this regard.
>
/nods
> 'host-passthrough' and 'host-model' are supposed to expose *every*
feature that
> the host CPUs support (except for those few which the hypervisor may block due
> to ability to virtualize them).
>
> So 'host-passthrough' is correct to automatically expose vmx/svm, without
> requiring any extra <feature> element, and I don't think we can accept
> this patch.
>
> This has been the case for KVM for ages, even though it has been considered
> experimental. The only slight difference is that you can block use of svm/vmx
> at the host OS level via a kernel arg to the kvm modules.
>
Ah that's where Xen falls off a little in which there's only libxl nested_hvm
field to control it, even though is still marked Experimental. There's no global
parameter to block it.
You could conceivably replicate the host-level control KVM has by using an
/etc/libvirt/libxl.conf driver level config option to indicate whether
nested-virt is permitted or not.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
1:44 p.m.
New subject: [libvirt] [PATCH v3 2/6] libxl: do not enable nested HVM by mere presence of <cpu> element
On Tue, Dec 19, 2017 at 01:45:57PM +0000, Daniel P. Berrange wrote:
On Tue, Dec 19, 2017 at 01:43:24PM +0000, Joao Martins wrote:
> On 12/19/2017 01:13 PM, Daniel P. Berrange wrote:
> > On Tue, Dec 19, 2017 at 01:01:36PM +0000, Joao Martins wrote:
> >> [Sorry for double posting, but I mistakenly forgot to include libvirt
list)
> >>
> >> +WimT +Daniel
> >>
> >> On 12/10/2017 02:10 AM, Marek Marczykowski-Górecki wrote:
> >>> <cpu mode='host-passthrough'> element may be used to
configure other
> >>> features, like NUMA, or CPUID. Do not enable nested HVM (which is in
> >>> "preview" state after all) by mere presence of
> >>> <cpu mode='host-passthrough'> element, but require
explicit <feature
> >>> policy='force' name='vmx'/> (or 'svm').
> >>> Also, adjust xenconfig driver to appropriately translate to/from
> >>> nestedhvm=1.
> >>>
> >>> While at it, adjust xenconfig driver to not override def->cpu if
already
> >>> set elsewhere. This will help with adding cpuid support.
> >>
> >> I agree with this and it was what we came up in the first version of nested
hvm
> >> support[0]. Although Daniel suggested there to use the same semantics of
qemu
> >> driver such that host-passthrough enables nested hvm without the use of:
> >>
> >> <feature policy='require' name='vmx'/>
> >
> > Yes, the key point of libvirt is to apply consistent semantics across
different
> > drivers, so we should not diverge betweeen QEMU & Xen in this regard.
> >
>
> /nods
>
> > 'host-passthrough' and 'host-model' are supposed to expose
*every* feature that
> > the host CPUs support (except for those few which the hypervisor may block due
> > to ability to virtualize them).
> >
> > So 'host-passthrough' is correct to automatically expose vmx/svm,
without
> > requiring any extra <feature> element, and I don't think we can
accept
> > this patch.
My point is you can use <cpu> element to configure various features,
like mentioned above (NUMA etc). As discussed previously, in libxl
driver only 'host-passthrough' mode makes sense, because that's what
libxl allows (enabled/disable various features, not define the whole
CPU). So, you can use something like:
<cpu mode='host-passthrough'>
<numa>
<cell id='0' cpus='0-3' memory='512000'
unit='KiB'/>
<cell id='1' cpus='4-7' memory='512000'
unit='KiB' memAccess='shared'/>
</numa>
</cpu>
Now, this is _very not obvious_ you've just enabled potentially
dangerous feature. Quoting
https://wiki.xenproject.org/wiki/Nested_Virtualization_in_Xen:
This means an L1 admin can DOS the L0 hypervisor. This is a
potential security issue; for this reason, we do not recommend running
nested virtualization in production yet.
Enabling potentially harmful features without explicit consent is not
something that I'd expect from a project meant to be used in production
environment...
Generally I think this is bad idea that placing just <cpu
mode='host-passthrough'>, without any specific setting, change anything
(compared to no <cpu/> at all). At least in context of libxl driver.
> > This has been the case for KVM for ages, even though it has
been considered
> > experimental. The only slight difference is that you can block use of svm/vmx
> > at the host OS level via a kernel arg to the kvm modules.
> >
> Ah that's where Xen falls off a little in which there's only libxl
nested_hvm
> field to control it, even though is still marked Experimental. There's no
global
> parameter to block it.
You could conceivably replicate the host-level control KVM has by using an
/etc/libvirt/libxl.conf driver level config option to indicate whether
nested-virt is permitted or not.
That could work. Is 'nestedhvm' ok for parameter name (disabled by
default)?
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
3:48 a.m.
New subject: [libvirt] [PATCH v3 2/6] libxl: do not enable nested HVM by mere presence of <cpu> element
On Tue, Dec 19, 2017 at 08:44:48PM +0100, Marek Marczykowski-Górecki wrote:
On Tue, Dec 19, 2017 at 01:45:57PM +0000, Daniel P. Berrange wrote:
> On Tue, Dec 19, 2017 at 01:43:24PM +0000, Joao Martins wrote:
> > On 12/19/2017 01:13 PM, Daniel P. Berrange wrote:
> > > On Tue, Dec 19, 2017 at 01:01:36PM +0000, Joao Martins wrote:
> > >> [Sorry for double posting, but I mistakenly forgot to include libvirt
list)
> > >>
> > >> +WimT +Daniel
> > >>
> > >> On 12/10/2017 02:10 AM, Marek Marczykowski-Górecki wrote:
> > >>> <cpu mode='host-passthrough'> element may be used to
configure other
> > >>> features, like NUMA, or CPUID. Do not enable nested HVM (which is
in
> > >>> "preview" state after all) by mere presence of
> > >>> <cpu mode='host-passthrough'> element, but require
explicit <feature
> > >>> policy='force' name='vmx'/> (or
'svm').
> > >>> Also, adjust xenconfig driver to appropriately translate to/from
> > >>> nestedhvm=1.
> > >>>
> > >>> While at it, adjust xenconfig driver to not override def->cpu
if already
> > >>> set elsewhere. This will help with adding cpuid support.
> > >>
> > >> I agree with this and it was what we came up in the first version of
nested hvm
> > >> support[0]. Although Daniel suggested there to use the same semantics
of qemu
> > >> driver such that host-passthrough enables nested hvm without the use
of:
> > >>
> > >> <feature policy='require' name='vmx'/>
> > >
> > > Yes, the key point of libvirt is to apply consistent semantics across
different
> > > drivers, so we should not diverge betweeen QEMU & Xen in this regard.
> > >
> >
> > /nods
> >
> > > 'host-passthrough' and 'host-model' are supposed to expose
*every* feature that
> > > the host CPUs support (except for those few which the hypervisor may block
due
> > > to ability to virtualize them).
> > >
> > > So 'host-passthrough' is correct to automatically expose vmx/svm,
without
> > > requiring any extra <feature> element, and I don't think we can
accept
> > > this patch.
My point is you can use <cpu> element to configure various features,
like mentioned above (NUMA etc). As discussed previously, in libxl
driver only 'host-passthrough' mode makes sense, because that's what
libxl allows (enabled/disable various features, not define the whole
CPU). So, you can use something like:
<cpu mode='host-passthrough'>
<numa>
<cell id='0' cpus='0-3' memory='512000'
unit='KiB'/>
<cell id='1' cpus='4-7' memory='512000'
unit='KiB' memAccess='shared'/>
</numa>
</cpu>
Now, this is _very not obvious_ you've just enabled potentially
dangerous feature. Quoting
https://wiki.xenproject.org/wiki/Nested_Virtualization_in_Xen:
This means an L1 admin can DOS the L0 hypervisor. This is a
potential security issue; for this reason, we do not recommend running
nested virtualization in production yet.
Enabling potentially harmful features without explicit consent is not
something that I'd expect from a project meant to be used in production
environment...
Whoever wrote that XML *has* given explicit consent, because that is applying
the documented semantics of the 'host-passthrough' CPU mode. This is exactly
the same situation as with the KVM driver.
The mistake here is assuming that mode='host-passthrough' is identical to
not listing CPUJ at all.
If you don't want VMX/SVM added when you define NUMA, then do
<cpu mode='host-passthrough'>
<feature name="vmx" policy="disable"/>
<numa>
<cell id='0' cpus='0-3' memory='512000'
unit='KiB'/>
<cell id='1' cpus='4-7' memory='512000'
unit='KiB' memAccess='shared'/>
</numa>
</cpu>
In retrospect the <numa> info should not have been inside the <cpu>
element, but that's something we unfortunately have to live with now
for back compatibility.
Generally I think this is bad idea that placing just <cpu
mode='host-passthrough'>, without any specific setting, change anything
(compared to no <cpu/> at all). At least in context of libxl driver.
There's nothing specific about libxl there - it would do the same for
KVM too if the host supports svm/vmx.
> You could conceivably replicate the host-level control KVM has
by using an
> /etc/libvirt/libxl.conf driver level config option to indicate whether
> nested-virt is permitted or not.
That could work. Is 'nestedhvm' ok for parameter name (disabled by
default)?
Sure, whatever parameter name you feel is best - there's no rules about
parameters / naming for the driver specific global config files.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
8:10 p.m.
New subject: [libvirt] [PATCH v3 3/6] libxl: add support for CPUID features policy
Convert CPU features policy into libxl cpuid policy settings. Use new
("libxl") syntax, which allow to enable/disable specific bits, using
host CPU as a base. For this reason, only "host-passthrough" mode is
accepted.
Libxl do not have distinction between "force" and "required" policy
(there is only "force") and also between "forbid" and
"disable" (there
is only "disable"). So, merge them appropriately. If anything,
"require"
and "forbid" should be enforced outside of specific driver.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
Changes since v2:
- drop spurious changes
- move libxlTranslateCPUFeature function to xen_xl.c, to be reused by
xenconfig driver
Changes since v1:
- use new ("libxl") syntax to set only bits explicitly mentioned in
domain XML
---
src/libxl/libxl_conf.c | 35 +++++++++++++++++++++++++++++++++--
src/xenconfig/xen_xl.c | 34 ++++++++++++++++++++++++++++++++++
src/xenconfig/xen_xl.h | 2 ++
3 files changed, 69 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 1846109..7760c2b 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -50,6 +50,7 @@
#include "secret_util.h"
#include "cpu/cpu.h"
#include "xen_common.h"
+#include "xen_xl.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -357,6 +358,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
bool hasHwVirt = false;
int nested_hvm = -1;
bool svm = false, vmx = false;
+ char xlCPU[32];
if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -379,15 +381,44 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
case VIR_CPU_FEATURE_DISABLE:
case VIR_CPU_FEATURE_FORBID:
if ((vmx && STREQ(def->cpu->features[i].name,
"vmx")) ||
- (svm && STREQ(def->cpu->features[i].name,
"svm")))
+ (svm && STREQ(def->cpu->features[i].name,
"svm"))) {
nested_hvm = 0;
+ continue;
+ }
+
+ snprintf(xlCPU,
+ sizeof(xlCPU),
+ "%s=0",
+ xenTranslateCPUFeature(
+ def->cpu->features[i].name,
+ false));
+ if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU))
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported cpu feature
'%s'"),
+ def->cpu->features[i].name);
+ return -1;
+ }
break;
case VIR_CPU_FEATURE_FORCE:
case VIR_CPU_FEATURE_REQUIRE:
if ((vmx && STREQ(def->cpu->features[i].name,
"vmx")) ||
- (svm && STREQ(def->cpu->features[i].name,
"svm")))
+ (svm && STREQ(def->cpu->features[i].name,
"svm"))) {
nested_hvm = 1;
+ continue;
+ }
+
+ snprintf(xlCPU,
+ sizeof(xlCPU),
+ "%s=1",
+ libxlTranslateCPUFeature(
+ def->cpu->features[i].name));
+ if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU))
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported cpu feature
'%s'"),
+ def->cpu->features[i].name);
+ return -1;
+ }
break;
case VIR_CPU_FEATURE_OPTIONAL:
case VIR_CPU_FEATURE_LAST:
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 317c7a0..356ca3a 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -225,6 +225,40 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
return 0;
}
+/*
+ * Translate CPU feature name from libvirt to libxl (from_libxl=false) or from
+ * libxl to libvirt (from_libxl=true).
+ */
+const char *
+xenTranslateCPUFeature(const char *feature_name, bool from_libxl)
+{
+ static const char *translation_table[][2] = {
+ /* libvirt name, libxl name */
+ { "cx16", "cmpxchg16" },
+ { "cid", "cntxid" },
+ { "ds_cpl", "dscpl" },
+ { "pclmuldq", "pclmulqdq" },
+ { "pni", "sse3" },
+ { "ht", "htt" },
+ { "pn", "psn" },
+ { "clflush", "clfsh" },
+ { "sep", "sysenter" },
+ { "cx8", "cmpxchg8" },
+ { "nodeid_msr", "nodeid" },
+ { "cr8legacy", "altmovcr8" },
+ { "lahf_lm", "lahfsahf" },
+ { "cmp_legacy", "cmplegacy" },
+ { "fxsr_opt", "ffxsr" },
+ { "pdpe1gb", "page1gb" },
+ };
+ size_t i;
+
+ for (i = 0; i < ARRAY_CARDINALITY(translation_table); i++)
+ if (STREQ(translation_table[i][from_libxl], feature_name))
+ return translation_table[i][!from_libxl];
+ return feature_name;
+}
+
static int
xenParseXLSpice(virConfPtr conf, virDomainDefPtr def)
diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h
index dd96326..68f332a 100644
--- a/src/xenconfig/xen_xl.h
+++ b/src/xenconfig/xen_xl.h
@@ -33,4 +33,6 @@ virDomainDefPtr xenParseXL(virConfPtr conn,
virConfPtr xenFormatXL(virDomainDefPtr def, virConnectPtr);
+const char *xenTranslateCPUFeature(const char *feature_name, bool from_libxl);
+
#endif /* __VIR_XEN_XL_H__ */
--
git-series 0.9.1
1:09 p.m.
New subject: [libvirt] [PATCH v3.1 3/6] libxl: add support for CPUID features policy
Convert CPU features policy into libxl cpuid policy settings. Use new
("libxl") syntax, which allow to enable/disable specific bits, using
host CPU as a base. For this reason, only "host-passthrough" mode is
accepted.
Libxl do not have distinction between "force" and "required" policy
(there is only "force") and also between "forbid" and
"disable" (there
is only "disable"). So, merge them appropriately. If anything,
"require"
and "forbid" should be enforced outside of specific driver.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
Changes since v3:
- compile fix (one more s/libxlTranslateCPUFeature/xenTranslateCPUFeature/)
Changes since v2:
- drop spurious changes
- move libxlTranslateCPUFeature function to xen_xl.c, to be reused by
xenconfig driver
Changes since v1:
- use new ("libxl") syntax to set only bits explicitly mentioned in
domain XML
---
src/libxl/libxl_conf.c | 35 +++++++++++++++++++++++++++++++++--
src/xenconfig/xen_xl.c | 34 ++++++++++++++++++++++++++++++++++
src/xenconfig/xen_xl.h | 2 ++
3 files changed, 69 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 184610966..5eae6d10f 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -50,6 +50,7 @@
#include "secret_util.h"
#include "cpu/cpu.h"
#include "xen_common.h"
+#include "xen_xl.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -357,6 +358,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
bool hasHwVirt = false;
int nested_hvm = -1;
bool svm = false, vmx = false;
+ char xlCPU[32];
if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -379,15 +381,44 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
case VIR_CPU_FEATURE_DISABLE:
case VIR_CPU_FEATURE_FORBID:
if ((vmx && STREQ(def->cpu->features[i].name,
"vmx")) ||
- (svm && STREQ(def->cpu->features[i].name,
"svm")))
+ (svm && STREQ(def->cpu->features[i].name,
"svm"))) {
nested_hvm = 0;
+ continue;
+ }
+
+ snprintf(xlCPU,
+ sizeof(xlCPU),
+ "%s=0",
+ xenTranslateCPUFeature(
+ def->cpu->features[i].name,
+ false));
+ if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU))
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported cpu feature
'%s'"),
+ def->cpu->features[i].name);
+ return -1;
+ }
break;
case VIR_CPU_FEATURE_FORCE:
case VIR_CPU_FEATURE_REQUIRE:
if ((vmx && STREQ(def->cpu->features[i].name,
"vmx")) ||
- (svm && STREQ(def->cpu->features[i].name,
"svm")))
+ (svm && STREQ(def->cpu->features[i].name,
"svm"))) {
nested_hvm = 1;
+ continue;
+ }
+
+ snprintf(xlCPU,
+ sizeof(xlCPU),
+ "%s=1",
+ xenTranslateCPUFeature(
+ def->cpu->features[i].name, false));
+ if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU))
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported cpu feature
'%s'"),
+ def->cpu->features[i].name);
+ return -1;
+ }
break;
case VIR_CPU_FEATURE_OPTIONAL:
case VIR_CPU_FEATURE_LAST:
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 317c7a08d..356ca3a4b 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -225,6 +225,40 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
return 0;
}
+/*
+ * Translate CPU feature name from libvirt to libxl (from_libxl=false) or from
+ * libxl to libvirt (from_libxl=true).
+ */
+const char *
+xenTranslateCPUFeature(const char *feature_name, bool from_libxl)
+{
+ static const char *translation_table[][2] = {
+ /* libvirt name, libxl name */
+ { "cx16", "cmpxchg16" },
+ { "cid", "cntxid" },
+ { "ds_cpl", "dscpl" },
+ { "pclmuldq", "pclmulqdq" },
+ { "pni", "sse3" },
+ { "ht", "htt" },
+ { "pn", "psn" },
+ { "clflush", "clfsh" },
+ { "sep", "sysenter" },
+ { "cx8", "cmpxchg8" },
+ { "nodeid_msr", "nodeid" },
+ { "cr8legacy", "altmovcr8" },
+ { "lahf_lm", "lahfsahf" },
+ { "cmp_legacy", "cmplegacy" },
+ { "fxsr_opt", "ffxsr" },
+ { "pdpe1gb", "page1gb" },
+ };
+ size_t i;
+
+ for (i = 0; i < ARRAY_CARDINALITY(translation_table); i++)
+ if (STREQ(translation_table[i][from_libxl], feature_name))
+ return translation_table[i][!from_libxl];
+ return feature_name;
+}
+
static int
xenParseXLSpice(virConfPtr conf, virDomainDefPtr def)
diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h
index dd963268e..68f332aca 100644
--- a/src/xenconfig/xen_xl.h
+++ b/src/xenconfig/xen_xl.h
@@ -33,4 +33,6 @@ virDomainDefPtr xenParseXL(virConfPtr conn,
virConfPtr xenFormatXL(virDomainDefPtr def, virConnectPtr);
+const char *xenTranslateCPUFeature(const char *feature_name, bool from_libxl);
+
#endif /* __VIR_XEN_XL_H__ */
--
2.13.6
7:19 a.m.
New subject: [libvirt] [PATCH v3.1 3/6] libxl: add support for CPUID features policy
On 12/13/2017 07:09 PM, Marek Marczykowski-Górecki wrote:
Convert CPU features policy into libxl cpuid policy settings. Use
new
("libxl") syntax, which allow to enable/disable specific bits, using
host CPU as a base. For this reason, only "host-passthrough" mode is
accepted.
Libxl do not have distinction between "force" and "required" policy
(there is only "force") and also between "forbid" and
"disable" (there
is only "disable"). So, merge them appropriately. If anything,
"require"
and "forbid" should be enforced outside of specific driver.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
This is quite neat Marek :)
I have one comment towards the end, suggesting an alternative to a static
translation table.
---
Changes since v3:
- compile fix (one more s/libxlTranslateCPUFeature/xenTranslateCPUFeature/)
Changes since v2:
- drop spurious changes
- move libxlTranslateCPUFeature function to xen_xl.c, to be reused by
xenconfig driver
Changes since v1:
- use new ("libxl") syntax to set only bits explicitly mentioned in
domain XML
---
src/libxl/libxl_conf.c | 35 +++++++++++++++++++++++++++++++++--
src/xenconfig/xen_xl.c | 34 ++++++++++++++++++++++++++++++++++
src/xenconfig/xen_xl.h | 2 ++
3 files changed, 69 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 184610966..5eae6d10f 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -50,6 +50,7 @@
#include "secret_util.h"
#include "cpu/cpu.h"
#include "xen_common.h"
+#include "xen_xl.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -357,6 +358,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
bool hasHwVirt = false;
int nested_hvm = -1;
bool svm = false, vmx = false;
+ char xlCPU[32];
if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -379,15 +381,44 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
case VIR_CPU_FEATURE_DISABLE:
case VIR_CPU_FEATURE_FORBID:
if ((vmx && STREQ(def->cpu->features[i].name,
"vmx")) ||
- (svm && STREQ(def->cpu->features[i].name,
"svm")))
+ (svm && STREQ(def->cpu->features[i].name,
"svm"))) {
nested_hvm = 0;
+ continue;
+ }
+
+ snprintf(xlCPU,
+ sizeof(xlCPU),
+ "%s=0",
+ xenTranslateCPUFeature(
+ def->cpu->features[i].name,
+ false));
+ if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU))
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported cpu feature
'%s'"),
+ def->cpu->features[i].name);
+ return -1;
+ }
break;
case VIR_CPU_FEATURE_FORCE:
case VIR_CPU_FEATURE_REQUIRE:
if ((vmx && STREQ(def->cpu->features[i].name,
"vmx")) ||
- (svm && STREQ(def->cpu->features[i].name,
"svm")))
+ (svm && STREQ(def->cpu->features[i].name,
"svm"))) {
nested_hvm = 1;
+ continue;
+ }
+
+ snprintf(xlCPU,
+ sizeof(xlCPU),
+ "%s=1",
+ xenTranslateCPUFeature(
+ def->cpu->features[i].name, false));
+ if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU))
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported cpu feature
'%s'"),
+ def->cpu->features[i].name);
+ return -1;
+ }
break;
case VIR_CPU_FEATURE_OPTIONAL:
case VIR_CPU_FEATURE_LAST:
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 317c7a08d..356ca3a4b 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -225,6 +225,40 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
return 0;
}
+/*
+ * Translate CPU feature name from libvirt to libxl (from_libxl=false) or from
+ * libxl to libvirt (from_libxl=true).
+ */
+const char *
+xenTranslateCPUFeature(const char *feature_name, bool from_libxl)
+{
+ static const char *translation_table[][2] = {
+ /* libvirt name, libxl name */
+ { "cx16", "cmpxchg16" },
+ { "cid", "cntxid" },
+ { "ds_cpl", "dscpl" },
+ { "pclmuldq", "pclmulqdq" },
+ { "pni", "sse3" },
+ { "ht", "htt" },
+ { "pn", "psn" },
+ { "clflush", "clfsh" },
+ { "sep", "sysenter" },
+ { "cx8", "cmpxchg8" },
+ { "nodeid_msr", "nodeid" },
+ { "cr8legacy", "altmovcr8" },
+ { "lahf_lm", "lahfsahf" },
+ { "cmp_legacy", "cmplegacy" },
+ { "fxsr_opt", "ffxsr" },
+ { "pdpe1gb", "page1gb" },
+ };
+ size_t i;
+
+ for (i = 0; i < ARRAY_CARDINALITY(translation_table); i++)
+ if (STREQ(translation_table[i][from_libxl], feature_name))
+ return translation_table[i][!from_libxl];
+ return feature_name;
+}
+
Cc'ing Jim as he may have some thoughts on the direction of this.
What happens if the admin decides to change /usr/share/libvirt/cpu_map.xml? Also
you can also add new leafs/features to cpu_map.xml
Maybe an idea instead is to have a table with all leafs that libxl has hardcoded
(i.e. cpuid_flags array on
http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl_cpuid...).
Then adding a new cpu driver routine to look for cpuid data based on feature
name (and the reverse too). Finally you would populate this translation table at
driver load time, or you could even get away without a translation table (but
would be a little more inefficient?).
static int
xenParseXLSpice(virConfPtr conf, virDomainDefPtr def)
diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h
index dd963268e..68f332aca 100644
--- a/src/xenconfig/xen_xl.h
+++ b/src/xenconfig/xen_xl.h
@@ -33,4 +33,6 @@ virDomainDefPtr xenParseXL(virConfPtr conn,
virConfPtr xenFormatXL(virDomainDefPtr def, virConnectPtr);
+const char *xenTranslateCPUFeature(const char *feature_name, bool from_libxl);
+
#endif /* __VIR_XEN_XL_H__ */
6 p.m.
New subject: [libvirt] [PATCH v3.1 3/6] libxl: add support for CPUID features policy
On 12/19/2017 06:19 AM, Joao Martins wrote:
On 12/13/2017 07:09 PM, Marek Marczykowski-Górecki wrote:
> Convert CPU features policy into libxl cpuid policy settings. Use new
> ("libxl") syntax, which allow to enable/disable specific bits, using
> host CPU as a base. For this reason, only "host-passthrough" mode is
> accepted.
> Libxl do not have distinction between "force" and "required"
policy
> (there is only "force") and also between "forbid" and
"disable" (there
> is only "disable"). So, merge them appropriately. If anything,
"require"
> and "forbid" should be enforced outside of specific driver.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
This is quite neat Marek :)
I have one comment towards the end, suggesting an alternative to a static
translation table.
> ---
> Changes since v3:
> - compile fix (one more s/libxlTranslateCPUFeature/xenTranslateCPUFeature/)
> Changes since v2:
> - drop spurious changes
> - move libxlTranslateCPUFeature function to xen_xl.c, to be reused by
> xenconfig driver
> Changes since v1:
> - use new ("libxl") syntax to set only bits explicitly mentioned in
> domain XML
> ---
> src/libxl/libxl_conf.c | 35 +++++++++++++++++++++++++++++++++--
> src/xenconfig/xen_xl.c | 34 ++++++++++++++++++++++++++++++++++
> src/xenconfig/xen_xl.h | 2 ++
> 3 files changed, 69 insertions(+), 2 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 184610966..5eae6d10f 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -50,6 +50,7 @@
> #include "secret_util.h"
> #include "cpu/cpu.h"
> #include "xen_common.h"
> +#include "xen_xl.h"
>
>
> #define VIR_FROM_THIS VIR_FROM_LIBXL
> @@ -357,6 +358,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> bool hasHwVirt = false;
> int nested_hvm = -1;
> bool svm = false, vmx = false;
> + char xlCPU[32];
>
> if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -379,15 +381,44 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> case VIR_CPU_FEATURE_DISABLE:
> case VIR_CPU_FEATURE_FORBID:
> if ((vmx &&
STREQ(def->cpu->features[i].name, "vmx")) ||
> - (svm &&
STREQ(def->cpu->features[i].name, "svm")))
> + (svm &&
STREQ(def->cpu->features[i].name, "svm"))) {
> nested_hvm = 0;
> + continue;
> + }
> +
> + snprintf(xlCPU,
> + sizeof(xlCPU),
> + "%s=0",
> + xenTranslateCPUFeature(
> + def->cpu->features[i].name,
> + false));
> + if (libxl_cpuid_parse_config(&b_info->cpuid,
xlCPU)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unsupported cpu feature
'%s'"),
> + def->cpu->features[i].name);
> + return -1;
> + }
> break;
>
> case VIR_CPU_FEATURE_FORCE:
> case VIR_CPU_FEATURE_REQUIRE:
> if ((vmx &&
STREQ(def->cpu->features[i].name, "vmx")) ||
> - (svm &&
STREQ(def->cpu->features[i].name, "svm")))
> + (svm &&
STREQ(def->cpu->features[i].name, "svm"))) {
> nested_hvm = 1;
> + continue;
> + }
> +
> + snprintf(xlCPU,
> + sizeof(xlCPU),
> + "%s=1",
> + xenTranslateCPUFeature(
> + def->cpu->features[i].name, false));
> + if (libxl_cpuid_parse_config(&b_info->cpuid,
xlCPU)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unsupported cpu feature
'%s'"),
> + def->cpu->features[i].name);
> + return -1;
> + }
> break;
> case VIR_CPU_FEATURE_OPTIONAL:
> case VIR_CPU_FEATURE_LAST:
> diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> index 317c7a08d..356ca3a4b 100644
> --- a/src/xenconfig/xen_xl.c
> +++ b/src/xenconfig/xen_xl.c
> @@ -225,6 +225,40 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr
caps)
> return 0;
> }
>
> +/*
> + * Translate CPU feature name from libvirt to libxl (from_libxl=false) or from
> + * libxl to libvirt (from_libxl=true).
> + */
> +const char *
> +xenTranslateCPUFeature(const char *feature_name, bool from_libxl)
> +{
> + static const char *translation_table[][2] = {
> + /* libvirt name, libxl name */
> + { "cx16", "cmpxchg16" },
> + { "cid", "cntxid" },
> + { "ds_cpl", "dscpl" },
> + { "pclmuldq", "pclmulqdq" },
> + { "pni", "sse3" },
> + { "ht", "htt" },
> + { "pn", "psn" },
> + { "clflush", "clfsh" },
> + { "sep", "sysenter" },
> + { "cx8", "cmpxchg8" },
> + { "nodeid_msr", "nodeid" },
> + { "cr8legacy", "altmovcr8" },
> + { "lahf_lm", "lahfsahf" },
> + { "cmp_legacy", "cmplegacy" },
> + { "fxsr_opt", "ffxsr" },
> + { "pdpe1gb", "page1gb" },
> + };
> + size_t i;
> +
> + for (i = 0; i < ARRAY_CARDINALITY(translation_table); i++)
> + if (STREQ(translation_table[i][from_libxl], feature_name))
> + return translation_table[i][!from_libxl];
> + return feature_name;
> +}
> +
>
Cc'ing Jim as he may have some thoughts on the direction of this.
What happens if the admin decides to change /usr/share/libvirt/cpu_map.xml?
Is changing existing content likely/practical?
Also you can also add new leafs/features to cpu_map.xml
Right. And I suppose the table in libxl_cpuid.c can grow too. And in cases where
they differ we'd need to update the static table, which we'll probably only
remember to do when receiving bug reports. So I like the idea of making this
more dynamic, but I apparently don't have enough brain power today to understand
your suggestion :-).
Maybe an idea instead is to have a table with all leafs that libxl
has hardcoded
(i.e. cpuid_flags array on
http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl_cpuid...).
Where would this table reside? In src/cpu/?
Then adding a new cpu driver routine to look for cpuid data based on
feature
name (and the reverse too). Finally you would populate this translation table at
driver load time, or you could even get away without a translation table (but
would be a little more inefficient?).
I'm having difficulty understanding how this provides a dynamic solution.
Wouldn't the table you mention above need extended anytime the hardcoded one in
libxl was extended? Which would probably only happen after receiving a bug
report? :-)
Hmm, the more I think about it, the more I convince myself that knowing libvirt
and libxl use different names for a CPU feature is static information. I'm
afraid I don't have any better ideas beyond Marek's neat trick.
Regards,
Jim
> static int
> xenParseXLSpice(virConfPtr conf, virDomainDefPtr def)
> diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h
> index dd963268e..68f332aca 100644
> --- a/src/xenconfig/xen_xl.h
> +++ b/src/xenconfig/xen_xl.h
> @@ -33,4 +33,6 @@ virDomainDefPtr xenParseXL(virConfPtr conn,
>
> virConfPtr xenFormatXL(virDomainDefPtr def, virConnectPtr);
>
> +const char *xenTranslateCPUFeature(const char *feature_name, bool from_libxl);
> +
> #endif /* __VIR_XEN_XL_H__ */
>
6:43 p.m.
New subject: [libvirt] [PATCH v3.1 3/6] libxl: add support for CPUID features policy
On Wed, Jan 03, 2018 at 05:00:10PM -0700, Jim Fehlig wrote:
On 12/19/2017 06:19 AM, Joao Martins wrote:
> On 12/13/2017 07:09 PM, Marek Marczykowski-Górecki wrote:
> > +/*
> > + * Translate CPU feature name from libvirt to libxl (from_libxl=false) or
from
> > + * libxl to libvirt (from_libxl=true).
> > + */
> > +const char *
> > +xenTranslateCPUFeature(const char *feature_name, bool from_libxl)
> > +{
> > + static const char *translation_table[][2] = {
> > + /* libvirt name, libxl name */
> > + { "cx16", "cmpxchg16" },
> > + { "cid", "cntxid" },
> > + { "ds_cpl", "dscpl" },
> > + { "pclmuldq", "pclmulqdq" },
> > + { "pni", "sse3" },
> > + { "ht", "htt" },
> > + { "pn", "psn" },
> > + { "clflush", "clfsh" },
> > + { "sep", "sysenter" },
> > + { "cx8", "cmpxchg8" },
> > + { "nodeid_msr", "nodeid" },
> > + { "cr8legacy", "altmovcr8" },
> > + { "lahf_lm", "lahfsahf" },
> > + { "cmp_legacy", "cmplegacy" },
> > + { "fxsr_opt", "ffxsr" },
> > + { "pdpe1gb", "page1gb" },
> > + };
> > + size_t i;
> > +
> > + for (i = 0; i < ARRAY_CARDINALITY(translation_table); i++)
> > + if (STREQ(translation_table[i][from_libxl], feature_name))
> > + return translation_table[i][!from_libxl];
> > + return feature_name;
> > +}
> > +
>
> Cc'ing Jim as he may have some thoughts on the direction of this.
>
> What happens if the admin decides to change /usr/share/libvirt/cpu_map.xml?
Is changing existing content likely/practical?
> Also you can also add new leafs/features to cpu_map.xml
Right. And I suppose the table in libxl_cpuid.c can grow too. And in cases
where they differ we'd need to update the static table, which we'll probably
only remember to do when receiving bug reports. So I like the idea of making
this more dynamic, but I apparently don't have enough brain power today to
understand your suggestion :-).
> Maybe an idea instead is to have a table with all leafs that libxl has hardcoded
> (i.e. cpuid_flags array on
>
http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl_cpuid...).
Where would this table reside? In src/cpu/?
> Then adding a new cpu driver routine to look for cpuid data based on feature
> name (and the reverse too). Finally you would populate this translation table at
> driver load time, or you could even get away without a translation table (but
> would be a little more inefficient?).
I'm having difficulty understanding how this provides a dynamic solution.
Wouldn't the table you mention above need extended anytime the hardcoded one
in libxl was extended? Which would probably only happen after receiving a
bug report? :-)
Probably... And worse thing about such table is it needs to contain all
entries from said libxl_cpuid.c. My solution require only listing
entries with mismatching names.
Alternative would be to not use "new libxl syntax", but "old xend
syntax" (which is still supported by libxl). The later allow to list
specific bits instead of feature names. That was implemented in v1 of
this patch series[1]... The problem with that approach is currently libvirt
does not expose API for lookup of individual features, but only to
construct full CPU and then enumerate its CPUID. That means it isn't
feasible for the current approach (mode='host-passthrough', then
enable/disable individual features). See discussion on v1.
Adding such API to libvirt cpu driver is beyond my knowledge of libvirt
code and available time :/
Hmm, the more I think about it, the more I convince myself that
knowing
libvirt and libxl use different names for a CPU feature is static
information. I'm afraid I don't have any better ideas beyond Marek's neat
trick.
Regards,
Jim
[1] https://www.redhat.com/archives/libvir-list/2017-June/msg01292.html
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
7:26 p.m.
New subject: [libvirt] [PATCH v3.1 3/6] libxl: add support for CPUID features policy
On 01/04/2018 12:43 AM, Marek Marczykowski-Górecki wrote:
On Wed, Jan 03, 2018 at 05:00:10PM -0700, Jim Fehlig wrote:
> On 12/19/2017 06:19 AM, Joao Martins wrote:
>> On 12/13/2017 07:09 PM, Marek Marczykowski-Górecki wrote:
>>> +/*
>>> + * Translate CPU feature name from libvirt to libxl (from_libxl=false) or
from
>>> + * libxl to libvirt (from_libxl=true).
>>> + */
>>> +const char *
>>> +xenTranslateCPUFeature(const char *feature_name, bool from_libxl)
>>> +{
>>> + static const char *translation_table[][2] = {
>>> + /* libvirt name, libxl name */
>>> + { "cx16", "cmpxchg16" },
>>> + { "cid", "cntxid" },
>>> + { "ds_cpl", "dscpl" },
>>> + { "pclmuldq", "pclmulqdq" },
>>> + { "pni", "sse3" },
>>> + { "ht", "htt" },
>>> + { "pn", "psn" },
>>> + { "clflush", "clfsh" },
>>> + { "sep", "sysenter" },
>>> + { "cx8", "cmpxchg8" },
>>> + { "nodeid_msr", "nodeid" },
>>> + { "cr8legacy", "altmovcr8" },
>>> + { "lahf_lm", "lahfsahf" },
>>> + { "cmp_legacy", "cmplegacy" },
>>> + { "fxsr_opt", "ffxsr" },
>>> + { "pdpe1gb", "page1gb" },
>>> + };
>>> + size_t i;
>>> +
>>> + for (i = 0; i < ARRAY_CARDINALITY(translation_table); i++)
>>> + if (STREQ(translation_table[i][from_libxl], feature_name))
>>> + return translation_table[i][!from_libxl];
>>> + return feature_name;
>>> +}
>>> +
>>
>> Cc'ing Jim as he may have some thoughts on the direction of this.
>>
>> What happens if the admin decides to change /usr/share/libvirt/cpu_map.xml?
>
> Is changing existing content likely/practical?
>
>> Also you can also add new leafs/features to cpu_map.xml
>
> Right. And I suppose the table in libxl_cpuid.c can grow too. And in cases
> where they differ we'd need to update the static table, which we'll probably
> only remember to do when receiving bug reports. So I like the idea of making
> this more dynamic, but I apparently don't have enough brain power today to
> understand your suggestion :-).
>
>> Maybe an idea instead is to have a table with all leafs that libxl has hardcoded
>> (i.e. cpuid_flags array on
>>
http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl_cpuid...).
>
> Where would this table reside? In src/cpu/?
>
>> Then adding a new cpu driver routine to look for cpuid data based on feature
>> name (and the reverse too). Finally you would populate this translation table at
>> driver load time, or you could even get away without a translation table (but
>> would be a little more inefficient?).
>
> I'm having difficulty understanding how this provides a dynamic solution.
> Wouldn't the table you mention above need extended anytime the hardcoded one
> in libxl was extended? Which would probably only happen after receiving a
> bug report? :-)
Probably... And worse thing about such table is it needs to contain all
entries from said libxl_cpuid.c. My solution require only listing
entries with mismatching names.
/nods and it would be a gigantic table most likely.
Alternative would be to not use "new libxl syntax", but
"old xend
syntax" (which is still supported by libxl). The later allow to list
specific bits instead of feature names. That was implemented in v1 of
this patch series[1]... The problem with that approach is currently libvirt
does not expose API for lookup of individual features, but only to
construct full CPU and then enumerate its CPUID.
I kinda liked your xend version, provided we could lookup the feature bits as
you are hinting here.
That means it isn't
feasible for the current approach (mode='host-passthrough', then
enable/disable individual features). See discussion on v1.
Adding such API to libvirt cpu driver is beyond my knowledge of libvirt
code and available time :/
The main reason I suggesting this out was because this patch was hardcoding
libvirt feature names. Maybe it's not an issue :)
Joao
7:13 p.m.
New subject: [libvirt] [PATCH v3.1 3/6] libxl: add support for CPUID features policy
On 01/04/2018 12:00 AM, Jim Fehlig wrote:
On 12/19/2017 06:19 AM, Joao Martins wrote:
> On 12/13/2017 07:09 PM, Marek Marczykowski-Górecki wrote:
>> Convert CPU features policy into libxl cpuid policy settings. Use new
>> ("libxl") syntax, which allow to enable/disable specific bits, using
>> host CPU as a base. For this reason, only "host-passthrough" mode is
>> accepted.
>> Libxl do not have distinction between "force" and "required"
policy
>> (there is only "force") and also between "forbid" and
"disable" (there
>> is only "disable"). So, merge them appropriately. If anything,
"require"
>> and "forbid" should be enforced outside of specific driver.
>>
>> Signed-off-by: Marek Marczykowski-Górecki
<marmarek(a)invisiblethingslab.com>
>
> This is quite neat Marek :)
>
> I have one comment towards the end, suggesting an alternative to a static
> translation table.
>
>> ---
>> Changes since v3:
>> - compile fix (one more s/libxlTranslateCPUFeature/xenTranslateCPUFeature/)
>> Changes since v2:
>> - drop spurious changes
>> - move libxlTranslateCPUFeature function to xen_xl.c, to be reused by
>> xenconfig driver
>> Changes since v1:
>> - use new ("libxl") syntax to set only bits explicitly mentioned in
>> domain XML
>> ---
>> src/libxl/libxl_conf.c | 35 +++++++++++++++++++++++++++++++++--
>> src/xenconfig/xen_xl.c | 34 ++++++++++++++++++++++++++++++++++
>> src/xenconfig/xen_xl.h | 2 ++
>> 3 files changed, 69 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>> index 184610966..5eae6d10f 100644
>> --- a/src/libxl/libxl_conf.c
>> +++ b/src/libxl/libxl_conf.c
>> @@ -50,6 +50,7 @@
>> #include "secret_util.h"
>> #include "cpu/cpu.h"
>> #include "xen_common.h"
>> +#include "xen_xl.h"
>>
>>
>> #define VIR_FROM_THIS VIR_FROM_LIBXL
>> @@ -357,6 +358,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>> bool hasHwVirt = false;
>> int nested_hvm = -1;
>> bool svm = false, vmx = false;
>> + char xlCPU[32];
>>
>> if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
>> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> @@ -379,15 +381,44 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>> case VIR_CPU_FEATURE_DISABLE:
>> case VIR_CPU_FEATURE_FORBID:
>> if ((vmx &&
STREQ(def->cpu->features[i].name, "vmx")) ||
>> - (svm &&
STREQ(def->cpu->features[i].name, "svm")))
>> + (svm &&
STREQ(def->cpu->features[i].name, "svm"))) {
>> nested_hvm = 0;
>> + continue;
>> + }
>> +
>> + snprintf(xlCPU,
>> + sizeof(xlCPU),
>> + "%s=0",
>> + xenTranslateCPUFeature(
>> + def->cpu->features[i].name,
>> + false));
>> + if (libxl_cpuid_parse_config(&b_info->cpuid,
xlCPU)) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("unsupported cpu feature
'%s'"),
>> + def->cpu->features[i].name);
>> + return -1;
>> + }
>> break;
>>
>> case VIR_CPU_FEATURE_FORCE:
>> case VIR_CPU_FEATURE_REQUIRE:
>> if ((vmx &&
STREQ(def->cpu->features[i].name, "vmx")) ||
>> - (svm &&
STREQ(def->cpu->features[i].name, "svm")))
>> + (svm &&
STREQ(def->cpu->features[i].name, "svm"))) {
>> nested_hvm = 1;
>> + continue;
>> + }
>> +
>> + snprintf(xlCPU,
>> + sizeof(xlCPU),
>> + "%s=1",
>> + xenTranslateCPUFeature(
>> + def->cpu->features[i].name,
false));
>> + if (libxl_cpuid_parse_config(&b_info->cpuid,
xlCPU)) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("unsupported cpu feature
'%s'"),
>> + def->cpu->features[i].name);
>> + return -1;
>> + }
>> break;
>> case VIR_CPU_FEATURE_OPTIONAL:
>> case VIR_CPU_FEATURE_LAST:
>> diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
>> index 317c7a08d..356ca3a4b 100644
>> --- a/src/xenconfig/xen_xl.c
>> +++ b/src/xenconfig/xen_xl.c
>> @@ -225,6 +225,40 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def,
virCapsPtr caps)
>> return 0;
>> }
>>
>> +/*
>> + * Translate CPU feature name from libvirt to libxl (from_libxl=false) or from
>> + * libxl to libvirt (from_libxl=true).
>> + */
>> +const char *
>> +xenTranslateCPUFeature(const char *feature_name, bool from_libxl)
>> +{
>> + static const char *translation_table[][2] = {
>> + /* libvirt name, libxl name */
>> + { "cx16", "cmpxchg16" },
>> + { "cid", "cntxid" },
>> + { "ds_cpl", "dscpl" },
>> + { "pclmuldq", "pclmulqdq" },
>> + { "pni", "sse3" },
>> + { "ht", "htt" },
>> + { "pn", "psn" },
>> + { "clflush", "clfsh" },
>> + { "sep", "sysenter" },
>> + { "cx8", "cmpxchg8" },
>> + { "nodeid_msr", "nodeid" },
>> + { "cr8legacy", "altmovcr8" },
>> + { "lahf_lm", "lahfsahf" },
>> + { "cmp_legacy", "cmplegacy" },
>> + { "fxsr_opt", "ffxsr" },
>> + { "pdpe1gb", "page1gb" },
>> + };
>> + size_t i;
>> +
>> + for (i = 0; i < ARRAY_CARDINALITY(translation_table); i++)
>> + if (STREQ(translation_table[i][from_libxl], feature_name))
>> + return translation_table[i][!from_libxl];
>> + return feature_name;
>> +}
>> +
>>
>
> Cc'ing Jim as he may have some thoughts on the direction of this.
>
> What happens if the admin decides to change /usr/share/libvirt/cpu_map.xml?
Is changing existing content likely/practical?
Not sure TBH. But I would imagine for every new feature, or if admin wants to
adjust mnemonics with say Linux. But this is guess work of what the people do
nowadays. But I guess the question is whether these feature names are part of
the API or are expected to change.
> Also you can also add new leafs/features to cpu_map.xml
Right. And I suppose the table in libxl_cpuid.c can grow too. And in cases where
they differ we'd need to update the static table, which we'll probably only
remember to do when receiving bug reports. So I like the idea of making this
more dynamic, but I apparently don't have enough brain power today to understand
your suggestion :-).
> Maybe an idea instead is to have a table with all leafs that libxl has hardcoded
> (i.e. cpuid_flags array on
>
http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl_cpuid...).
Where would this table reside? In src/cpu/?
Not sure, but would be placed preferably in src/xenconfig or src/libxl/.
> Then adding a new cpu driver routine to look for cpuid data based
on feature
> name (and the reverse too). Finally you would populate this translation table at
> driver load time, or you could even get away without a translation table (but
> would be a little more inefficient?).
I'm having difficulty understanding how this provides a dynamic solution.
Wouldn't the table you mention above need extended anytime the hardcoded one in
libxl was extended? Which would probably only happen after receiving a bug
report? :-)
Hmm, the more I think about it, the more I convince myself that knowing libvirt
and libxl use different names for a CPU feature is static information. I'm
afraid I don't have any better ideas beyond Marek's neat trick.
I probably didn't explain myself correctly.
What I mentioned above was to have the feature bits as common denominator as
opposed to feature names (that currently vary between libvirt and libxl). And
the array we import from libxl declares both feature name and bits (or just
resorting to xend syntax). So for each libxl feature bits we declare, we look
for the right libvirt feature name.
Regards,
Jim
>
>> static int
>> xenParseXLSpice(virConfPtr conf, virDomainDefPtr def)
>> diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h
>> index dd963268e..68f332aca 100644
>> --- a/src/xenconfig/xen_xl.h
>> +++ b/src/xenconfig/xen_xl.h
>> @@ -33,4 +33,6 @@ virDomainDefPtr xenParseXL(virConfPtr conn,
>>
>> virConfPtr xenFormatXL(virDomainDefPtr def, virConnectPtr);
>>
>> +const char *xenTranslateCPUFeature(const char *feature_name, bool from_libxl);
>> +
>> #endif /* __VIR_XEN_XL_H__ */
>>
>
8:10 p.m.
New subject: [libvirt] [PATCH v3 4/6] tests: check CPU features handling in libxl driver
Test enabling/disabling individual CPU features and also setting
nested HVM support, which is also controlled by CPU features node.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
Changes since v1:
- rewritten to Jim's test suite for libxl_domain_config generator
Cc: Jim Fehlig <jfehlig(a)suse.com>
---
tests/libxlxml2domconfigdata/fullvirt-cpuid.json | 64 +++++++++++++++++-
tests/libxlxml2domconfigdata/fullvirt-cpuid.xml | 37 ++++++++++-
tests/libxlxml2domconfigtest.c | 1 +-
3 files changed, 102 insertions(+)
create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.json
create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
diff --git a/tests/libxlxml2domconfigdata/fullvirt-cpuid.json
b/tests/libxlxml2domconfigdata/fullvirt-cpuid.json
new file mode 100644
index 0000000..234e592
--- /dev/null
+++ b/tests/libxlxml2domconfigdata/fullvirt-cpuid.json
@@ -0,0 +1,64 @@
+{
+ "c_info": {
+ "type": "hvm",
+ "name": "XenGuest2",
+ "uuid": "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+ },
+ "b_info": {
+ "max_vcpus": 1,
+ "avail_vcpus": [
+ 0
+ ],
+ "max_memkb": 592896,
+ "target_memkb": 403456,
+ "video_memkb": 8192,
+ "shadow_memkb": 5656,
+ "cpuid": [
+ {
+ "leaf": 1,
+ "ecx": "xxxxxxxxxxxxxxxxxxxxxxxxxx0xxxx0",
+ "edx": "xxxxxxxxxxxxxxxxxxxxxxxxxxx1xxxx"
+ }
+ ],
+ "sched_params": {
+ "weight": 1000
+ },
+ "type.hvm": {
+ "pae": "True",
+ "apic": "True",
+ "acpi": "True",
+ "vga": {
+
+ },
+ "nested_hvm": "False",
+ "vnc": {
+ "enable": "False"
+ },
+ "sdl": {
+ "enable": "False"
+ },
+ "spice": {
+
+ },
+ "boot": "c",
+ "rdm": {
+
+ }
+ },
+ "arch_arm": {
+
+ }
+ },
+ "disks": [
+ {
+ "pdev_path": "/dev/HostVG/XenGuest2",
+ "vdev": "hda",
+ "backend": "phy",
+ "format": "raw",
+ "removable": 1,
+ "readwrite": 1
+ }
+ ],
+ "on_reboot": "restart",
+ "on_crash": "restart"
+}
diff --git a/tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
b/tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
new file mode 100644
index 0000000..e9eba2e
--- /dev/null
+++ b/tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
@@ -0,0 +1,37 @@
+<domain type='xen'>
+ <name>XenGuest2</name>
+ <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>592896</memory>
+ <currentMemory unit='KiB'>403456</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='xenfv'>hvm</type>
+ </os>
+ <features>
+ <acpi/>
+ <apic/>
+ <pae/>
+ </features>
+ <cpu mode='host-passthrough'>
+ <feature policy='forbid' name='pni'/>
+ <feature policy='forbid' name='vmx'/>
+ <feature policy='require' name='tsc'/>
+ </cpu>
+ <clock offset='variable' adjustment='0' basis='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <devices>
+ <disk type='block' device='disk'>
+ <driver name='phy' type='raw'/>
+ <source dev='/dev/HostVG/XenGuest2'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <video>
+ <model type='cirrus' vram='8192' heads='1'
primary='yes'/>
+ </video>
+ </devices>
+</domain>
diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c
index bd4c3af..db0af23 100644
--- a/tests/libxlxml2domconfigtest.c
+++ b/tests/libxlxml2domconfigtest.c
@@ -191,6 +191,7 @@ mymain(void)
DO_TEST("moredevs-hvm");
DO_TEST("vnuma-hvm");
DO_TEST("multiple-ip");
+ DO_TEST("fullvirt-cpuid");
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
git-series 0.9.1
6:50 p.m.
New subject: [libvirt] [PATCH v3 4/6] tests: check CPU features handling in libxl driver
On 12/09/2017 07:10 PM, Marek Marczykowski-Górecki wrote:
Test enabling/disabling individual CPU features and also setting
nested HVM support, which is also controlled by CPU features node.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
Changes since v1:
- rewritten to Jim's test suite for libxl_domain_config generator
Cc: Jim Fehlig <jfehlig(a)suse.com>
---
tests/libxlxml2domconfigdata/fullvirt-cpuid.json | 64 +++++++++++++++++-
tests/libxlxml2domconfigdata/fullvirt-cpuid.xml | 37 ++++++++++-
tests/libxlxml2domconfigtest.c | 1 +-
3 files changed, 102 insertions(+)
create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.json
create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
Nice! I don't see anything being discussed in 3/6 affecting this patch.
Reviewed-by: Jim Fehlig <jfehlig(a)suse.com>
Regards,
Jim
diff --git a/tests/libxlxml2domconfigdata/fullvirt-cpuid.json
b/tests/libxlxml2domconfigdata/fullvirt-cpuid.json
new file mode 100644
index 0000000..234e592
--- /dev/null
+++ b/tests/libxlxml2domconfigdata/fullvirt-cpuid.json
@@ -0,0 +1,64 @@
+{
+ "c_info": {
+ "type": "hvm",
+ "name": "XenGuest2",
+ "uuid": "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+ },
+ "b_info": {
+ "max_vcpus": 1,
+ "avail_vcpus": [
+ 0
+ ],
+ "max_memkb": 592896,
+ "target_memkb": 403456,
+ "video_memkb": 8192,
+ "shadow_memkb": 5656,
+ "cpuid": [
+ {
+ "leaf": 1,
+ "ecx": "xxxxxxxxxxxxxxxxxxxxxxxxxx0xxxx0",
+ "edx": "xxxxxxxxxxxxxxxxxxxxxxxxxxx1xxxx"
+ }
+ ],
+ "sched_params": {
+ "weight": 1000
+ },
+ "type.hvm": {
+ "pae": "True",
+ "apic": "True",
+ "acpi": "True",
+ "vga": {
+
+ },
+ "nested_hvm": "False",
+ "vnc": {
+ "enable": "False"
+ },
+ "sdl": {
+ "enable": "False"
+ },
+ "spice": {
+
+ },
+ "boot": "c",
+ "rdm": {
+
+ }
+ },
+ "arch_arm": {
+
+ }
+ },
+ "disks": [
+ {
+ "pdev_path": "/dev/HostVG/XenGuest2",
+ "vdev": "hda",
+ "backend": "phy",
+ "format": "raw",
+ "removable": 1,
+ "readwrite": 1
+ }
+ ],
+ "on_reboot": "restart",
+ "on_crash": "restart"
+}
diff --git a/tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
b/tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
new file mode 100644
index 0000000..e9eba2e
--- /dev/null
+++ b/tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
@@ -0,0 +1,37 @@
+<domain type='xen'>
+ <name>XenGuest2</name>
+ <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>592896</memory>
+ <currentMemory unit='KiB'>403456</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='xenfv'>hvm</type>
+ </os>
+ <features>
+ <acpi/>
+ <apic/>
+ <pae/>
+ </features>
+ <cpu mode='host-passthrough'>
+ <feature policy='forbid' name='pni'/>
+ <feature policy='forbid' name='vmx'/>
+ <feature policy='require' name='tsc'/>
+ </cpu>
+ <clock offset='variable' adjustment='0' basis='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <devices>
+ <disk type='block' device='disk'>
+ <driver name='phy' type='raw'/>
+ <source dev='/dev/HostVG/XenGuest2'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <video>
+ <model type='cirrus' vram='8192' heads='1'
primary='yes'/>
+ </video>
+ </devices>
+</domain>
diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c
index bd4c3af..db0af23 100644
--- a/tests/libxlxml2domconfigtest.c
+++ b/tests/libxlxml2domconfigtest.c
@@ -191,6 +191,7 @@ mymain(void)
DO_TEST("moredevs-hvm");
DO_TEST("vnuma-hvm");
DO_TEST("multiple-ip");
+ DO_TEST("fullvirt-cpuid");
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
8:10 p.m.
New subject: [libvirt] [PATCH v3 5/6] xenconfig: add CPUID handling to domXML <-> xl.cfg conversion
Only "libxl" format supported for now. Special care needed around
vmx/svm, because those two are translated into "nestedhvm" setting.
---
Changes since v2:
- new patch
---
src/xenconfig/xen_xl.c | 168 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 168 insertions(+)
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 356ca3a..dc06d4a 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -259,6 +259,94 @@ xenTranslateCPUFeature(const char *feature_name, bool from_libxl)
return feature_name;
}
+static int
+xenParseXLCPUID(virConfPtr conf, virDomainDefPtr def)
+{
+ const char *cpuid_str = NULL;
+ char **cpuid_pairs = NULL;
+ char **name_and_value = NULL;
+ size_t i;
+ int ret = -1;
+ int policy;
+
+ if (xenConfigGetString(conf, "cpuid", &cpuid_str, NULL) < 0)
+ return -1;
+
+ if (!cpuid_str)
+ return 0;
+
+ if (!def->cpu) {
+ if (VIR_ALLOC(def->cpu) < 0)
+ goto cleanup;
+ def->cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
+ def->cpu->type = VIR_CPU_TYPE_GUEST;
+ def->cpu->nfeatures = 0;
+ def->cpu->nfeatures_max = 0;
+ }
+
+ cpuid_pairs = virStringSplit(cpuid_str, ",", 0);
+ if (!cpuid_pairs)
+ return -1;
+
+ if (!cpuid_pairs[0]) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Empty cpuid string"));
+ goto cleanup;
+ }
+
+ if (STRNEQ(cpuid_pairs[0], "host")) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cpuid starting with %s is not supported, only libxl format
is"),
+ cpuid_pairs[0]);
+ goto cleanup;
+ }
+
+ for (i = 1; cpuid_pairs[i]; i++) {
+ name_and_value = virStringSplit(cpuid_pairs[i], "=", 2);
+ if (!name_and_value)
+ goto cleanup;
+ if (!name_and_value[0] || !name_and_value[1]) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid libxl cpuid key=value element: %s"),
+ cpuid_pairs[i]);
+ goto cleanup;
+ }
+ if (STREQ(name_and_value[1], "1")) {
+ policy = VIR_CPU_FEATURE_FORCE;
+ } else if (STREQ(name_and_value[1], "0")) {
+ policy = VIR_CPU_FEATURE_DISABLE;
+ } else if (STREQ(name_and_value[1], "x")) {
+ policy = VIR_CPU_FEATURE_OPTIONAL;
+ } else if (STREQ(name_and_value[1], "k")) {
+ policy = VIR_CPU_FEATURE_OPTIONAL;
+ } else if (STREQ(name_and_value[1], "s")) {
+ policy = VIR_CPU_FEATURE_OPTIONAL;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid libxl cpuid value: %s"),
+ cpuid_pairs[i]);
+ goto cleanup;
+ }
+
+ if (virCPUDefAddFeature(def->cpu,
+ xenTranslateCPUFeature(name_and_value[0], true),
+ policy) < 0)
+ goto cleanup;
+
+ virStringListFree(name_and_value);
+ name_and_value = NULL;
+ }
+
+ ret = 0;
+
+ cleanup:
+ if (name_and_value)
+ virStringListFree(name_and_value);
+ if (cpuid_pairs)
+ virStringListFree(cpuid_pairs);
+ return ret;
+}
+
static int
xenParseXLSpice(virConfPtr conf, virDomainDefPtr def)
@@ -1094,6 +1182,9 @@ xenParseXL(virConfPtr conf,
goto cleanup;
#endif
+ if (xenParseXLCPUID(conf, def) < 0)
+ goto cleanup;
+
if (xenParseXLDisk(conf, def) < 0)
goto cleanup;
@@ -1243,6 +1334,80 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
return 0;
}
+static int
+xenFormatXLCPUID(virConfPtr conf, virDomainDefPtr def)
+{
+ char **cpuid_pairs = NULL;
+ char *cpuid_string = NULL;
+ size_t i, j;
+ int ret = -1;
+
+ if (!def->cpu)
+ return 0;
+
+ if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("'%s' CPU mode not supported, only host-passthrough
is"),
+ virCPUModeTypeToString(def->cpu->mode));
+ goto cleanup;
+ }
+
+ /* "host" + all features + NULL */
+ if (VIR_ALLOC_N(cpuid_pairs, def->cpu->nfeatures + 2) < 0)
+ goto cleanup;
+
+ if (VIR_STRDUP(cpuid_pairs[0], "host") < 0)
+ goto cleanup;
+
+ j = 1;
+ for (i = 0; i < def->cpu->nfeatures; i++) {
+ const char *feature_name = xenTranslateCPUFeature(
+ def->cpu->features[i].name,
+ false);
+ const char *policy = NULL;
+
+ if (STREQ(feature_name, "vmx") || STREQ(feature_name,
"svm"))
+ /* ignore vmx/svm in cpuid option, translated into nestedhvm
+ * elsewhere */
+ continue;
+
+ switch (def->cpu->features[i].policy) {
+ case VIR_CPU_FEATURE_FORCE:
+ case VIR_CPU_FEATURE_REQUIRE:
+ policy = "1";
+ break;
+ case VIR_CPU_FEATURE_OPTIONAL:
+ policy = "x";
+ break;
+ case VIR_CPU_FEATURE_DISABLE:
+ case VIR_CPU_FEATURE_FORBID:
+ policy = "0";
+ break;
+ }
+ if (virAsprintf(&cpuid_pairs[j++], "%s=%s",
+ feature_name,
+ policy) < 0)
+ goto cleanup;
+ }
+ cpuid_pairs[j] = NULL;
+
+ if (j > 1) {
+ cpuid_string = virStringListJoin((const char **)cpuid_pairs, ",");
+ if (!cpuid_string)
+ goto cleanup;
+
+ if (xenConfigSetString(conf, "cpuid", cpuid_string) < 0)
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ virStringListFree(cpuid_pairs);
+ VIR_FREE(cpuid_string);
+ return ret;
+}
+
#ifdef LIBXL_HAVE_VNUMA
static int
xenFormatXLVnode(virConfValuePtr list,
@@ -2008,6 +2173,9 @@ xenFormatXL(virDomainDefPtr def, virConnectPtr conn)
if (xenFormatXLOS(conf, def) < 0)
goto cleanup;
+ if (xenFormatXLCPUID(conf, def) < 0)
+ goto cleanup;
+
#ifdef LIBXL_HAVE_VNUMA
if (xenFormatXLDomainVnuma(conf, def) < 0)
goto cleanup;
--
git-series 0.9.1
8:17 p.m.
New subject: [libvirt] [PATCH v3 5/6] xenconfig: add CPUID handling to domXML <-> xl.cfg conversion
On 12/09/2017 07:10 PM, Marek Marczykowski-Górecki wrote:
Only "libxl" format supported for now. Special care needed
around
vmx/svm, because those two are translated into "nestedhvm" setting.
---
Changes since v2:
- new patch
---
src/xenconfig/xen_xl.c | 168 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 168 insertions(+)
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 356ca3a..dc06d4a 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -259,6 +259,94 @@ xenTranslateCPUFeature(const char *feature_name, bool from_libxl)
return feature_name;
}
+static int
+xenParseXLCPUID(virConfPtr conf, virDomainDefPtr def)
+{
+ const char *cpuid_str = NULL;
+ char **cpuid_pairs = NULL;
+ char **name_and_value = NULL;
+ size_t i;
+ int ret = -1;
+ int policy;
+
+ if (xenConfigGetString(conf, "cpuid", &cpuid_str, NULL) < 0)
+ return -1;
+
+ if (!cpuid_str)
+ return 0;
+
+ if (!def->cpu) {
+ if (VIR_ALLOC(def->cpu) < 0)
+ goto cleanup;
+ def->cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
+ def->cpu->type = VIR_CPU_TYPE_GUEST;
+ def->cpu->nfeatures = 0;
+ def->cpu->nfeatures_max = 0;
+ }
+
+ cpuid_pairs = virStringSplit(cpuid_str, ",", 0);
+ if (!cpuid_pairs)
+ return -1;
goto cleanup; ?
+
+ if (!cpuid_pairs[0]) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Empty cpuid string"));
+ goto cleanup;
+ }
Does xl ignore an empty cpuid string? If so, we should ignore it too.
+
+ if (STRNEQ(cpuid_pairs[0], "host")) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cpuid starting with %s is not supported, only libxl
format is"),
+ cpuid_pairs[0]);
+ goto cleanup;
+ }
It looks like we use VIR_ERR_INTERNAL_ERROR liberally throughout this file even
when a more specific error could be reported. E.g. in this case VIR_ERR_CONF_SYNTAX.
+
+ for (i = 1; cpuid_pairs[i]; i++) {
+ name_and_value = virStringSplit(cpuid_pairs[i], "=", 2);
+ if (!name_and_value)
+ goto cleanup;
+ if (!name_and_value[0] || !name_and_value[1]) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid libxl cpuid key=value element: %s"),
+ cpuid_pairs[i]);
VIR_ERR_CONF_SYNTAX?
+ goto cleanup;
+ }
+ if (STREQ(name_and_value[1], "1")) {
+ policy = VIR_CPU_FEATURE_FORCE;
+ } else if (STREQ(name_and_value[1], "0")) {
+ policy = VIR_CPU_FEATURE_DISABLE;
+ } else if (STREQ(name_and_value[1], "x")) {
+ policy = VIR_CPU_FEATURE_OPTIONAL;
+ } else if (STREQ(name_and_value[1], "k")) {
+ policy = VIR_CPU_FEATURE_OPTIONAL;
+ } else if (STREQ(name_and_value[1], "s")) {
+ policy = VIR_CPU_FEATURE_OPTIONAL;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid libxl cpuid value: %s"),
+ cpuid_pairs[i]);
Same.
+ goto cleanup;
+ }
+
+ if (virCPUDefAddFeature(def->cpu,
+ xenTranslateCPUFeature(name_and_value[0], true),
+ policy) < 0)
+ goto cleanup;
+
+ virStringListFree(name_and_value);
+ name_and_value = NULL;
+ }
+
+ ret = 0;
+
+ cleanup:
+ if (name_and_value)
+ virStringListFree(name_and_value);
+ if (cpuid_pairs)
+ virStringListFree(cpuid_pairs);
It's safe to call virStringListFree with NULL.
+ return ret;
+}
+
static int
xenParseXLSpice(virConfPtr conf, virDomainDefPtr def)
@@ -1094,6 +1182,9 @@ xenParseXL(virConfPtr conf,
goto cleanup;
#endif
+ if (xenParseXLCPUID(conf, def) < 0)
+ goto cleanup;
+
if (xenParseXLDisk(conf, def) < 0)
goto cleanup;
@@ -1243,6 +1334,80 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
return 0;
}
+static int
+xenFormatXLCPUID(virConfPtr conf, virDomainDefPtr def)
+{
+ char **cpuid_pairs = NULL;
+ char *cpuid_string = NULL;
+ size_t i, j;
+ int ret = -1;
+
+ if (!def->cpu)
+ return 0;
+
+ if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("'%s' CPU mode not supported, only
host-passthrough is"),
+ virCPUModeTypeToString(def->cpu->mode));
+ goto cleanup;
+ }
+
+ /* "host" + all features + NULL */
+ if (VIR_ALLOC_N(cpuid_pairs, def->cpu->nfeatures + 2) < 0)
+ goto cleanup;
Nit: no need to jump to cleanup before this point since there is nothing to cleanup.
Regards,
Jim
+
+ if (VIR_STRDUP(cpuid_pairs[0], "host") < 0)
+ goto cleanup;
+
+ j = 1;
+ for (i = 0; i < def->cpu->nfeatures; i++) {
+ const char *feature_name = xenTranslateCPUFeature(
+ def->cpu->features[i].name,
+ false);
+ const char *policy = NULL;
+
+ if (STREQ(feature_name, "vmx") || STREQ(feature_name,
"svm"))
+ /* ignore vmx/svm in cpuid option, translated into nestedhvm
+ * elsewhere */
+ continue;
+
+ switch (def->cpu->features[i].policy) {
+ case VIR_CPU_FEATURE_FORCE:
+ case VIR_CPU_FEATURE_REQUIRE:
+ policy = "1";
+ break;
+ case VIR_CPU_FEATURE_OPTIONAL:
+ policy = "x";
+ break;
+ case VIR_CPU_FEATURE_DISABLE:
+ case VIR_CPU_FEATURE_FORBID:
+ policy = "0";
+ break;
+ }
+ if (virAsprintf(&cpuid_pairs[j++], "%s=%s",
+ feature_name,
+ policy) < 0)
+ goto cleanup;
+ }
+ cpuid_pairs[j] = NULL;
+
+ if (j > 1) {
+ cpuid_string = virStringListJoin((const char **)cpuid_pairs, ",");
+ if (!cpuid_string)
+ goto cleanup;
+
+ if (xenConfigSetString(conf, "cpuid", cpuid_string) < 0)
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ virStringListFree(cpuid_pairs);
+ VIR_FREE(cpuid_string);
+ return ret;
+}
+
#ifdef LIBXL_HAVE_VNUMA
static int
xenFormatXLVnode(virConfValuePtr list,
@@ -2008,6 +2173,9 @@ xenFormatXL(virDomainDefPtr def, virConnectPtr conn)
if (xenFormatXLOS(conf, def) < 0)
goto cleanup;
+ if (xenFormatXLCPUID(conf, def) < 0)
+ goto cleanup;
+
#ifdef LIBXL_HAVE_VNUMA
if (xenFormatXLDomainVnuma(conf, def) < 0)
goto cleanup;
8:10 p.m.
New subject: [libvirt] [PATCH v3 6/6] tests: add test case for CPUID in xenconfig driver
Check conversion of "cpuid" setting, check all supported policy settings
("1", "0", "x"). Also, check interaction with
"nestedhvm" - should not
be included as "vmx=1" in "cpuid" setting.
---
Changes since v2:
- new patch
---
tests/xlconfigdata/test-fullvirt-cpuid.cfg | 25 ++++++++++++++++-
tests/xlconfigdata/test-fullvirt-cpuid.xml | 36 +++++++++++++++++++++++-
tests/xlconfigtest.c | 1 +-
3 files changed, 62 insertions(+)
create mode 100644 tests/xlconfigdata/test-fullvirt-cpuid.cfg
create mode 100644 tests/xlconfigdata/test-fullvirt-cpuid.xml
diff --git a/tests/xlconfigdata/test-fullvirt-cpuid.cfg
b/tests/xlconfigdata/test-fullvirt-cpuid.cfg
new file mode 100644
index 0000000..bb7b9c7
--- /dev/null
+++ b/tests/xlconfigdata/test-fullvirt-cpuid.cfg
@@ -0,0 +1,25 @@
+name = "XenGuest2"
+uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+maxmem = 579
+memory = 394
+vcpus = 1
+pae = 1
+acpi = 0
+apic = 0
+viridian = 0
+rtc_timeoffset = 0
+localtime = 0
+on_poweroff = "destroy"
+on_reboot = "restart"
+on_crash = "restart"
+device_model = "/usr/lib/xen/bin/qemu-system-i386"
+sdl = 0
+vnc = 1
+vncunused = 1
+vnclisten = "127.0.0.1"
+parallel = "none"
+serial = "none"
+builder = "hvm"
+boot = "d"
+nestedhvm = 1
+cpuid = "host,tm=0,sse3=1,page1gb=x"
diff --git a/tests/xlconfigdata/test-fullvirt-cpuid.xml
b/tests/xlconfigdata/test-fullvirt-cpuid.xml
new file mode 100644
index 0000000..277b419
--- /dev/null
+++ b/tests/xlconfigdata/test-fullvirt-cpuid.xml
@@ -0,0 +1,36 @@
+<domain type='xen'>
+ <name>XenGuest2</name>
+ <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>592896</memory>
+ <currentMemory unit='KiB'>403456</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='xenfv'>hvm</type>
+ <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>
+ <boot dev='cdrom'/>
+ </os>
+ <features>
+ <pae/>
+ </features>
+ <cpu mode='host-passthrough'>
+ <feature policy='force' name='vmx'/>
+ <feature policy='disable' name='tm'/>
+ <feature policy='force' name='pni'/>
+ <feature policy='optional' name='pdpe1gb'/>
+ </cpu>
+ <clock offset='variable' adjustment='0' basis='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <devices>
+ <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <graphics type='vnc' port='-1' autoport='yes'
listen='127.0.0.1'>
+ <listen type='address' address='127.0.0.1'/>
+ </graphics>
+ <video>
+ <model type='cirrus' vram='8192' heads='1'
primary='yes'/>
+ </video>
+ </devices>
+</domain>
diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
index 57a0a67..39f51e2 100644
--- a/tests/xlconfigtest.c
+++ b/tests/xlconfigtest.c
@@ -270,6 +270,7 @@ mymain(void)
DO_TEST("fullvirt-multi-timer");
DO_TEST("fullvirt-nestedhvm");
DO_TEST("fullvirt-nestedhvm-disabled");
+ DO_TEST("fullvirt-cpuid");
#ifdef LIBXL_HAVE_VNUMA
DO_TEST("fullvirt-vnuma");
DO_TEST_PARSE("fullvirt-vnuma-autocomplete", false);
--
git-series 0.9.1
8:20 p.m.
New subject: [libvirt] [PATCH v3 6/6] tests: add test case for CPUID in xenconfig driver
On 12/09/2017 07:10 PM, Marek Marczykowski-Górecki wrote:
Check conversion of "cpuid" setting, check all supported
policy settings
("1", "0", "x"). Also, check interaction with
"nestedhvm" - should not
be included as "vmx=1" in "cpuid" setting.
---
Changes since v2:
- new patch
---
tests/xlconfigdata/test-fullvirt-cpuid.cfg | 25 ++++++++++++++++-
tests/xlconfigdata/test-fullvirt-cpuid.xml | 36 +++++++++++++++++++++++-
tests/xlconfigtest.c | 1 +-
3 files changed, 62 insertions(+)
create mode 100644 tests/xlconfigdata/test-fullvirt-cpuid.cfg
create mode 100644 tests/xlconfigdata/test-fullvirt-cpuid.xml
Reviewed-by: Jim Fehlig <jfehlig(a)suse.com>
Regards,
Jim
diff --git a/tests/xlconfigdata/test-fullvirt-cpuid.cfg
b/tests/xlconfigdata/test-fullvirt-cpuid.cfg
new file mode 100644
index 0000000..bb7b9c7
--- /dev/null
+++ b/tests/xlconfigdata/test-fullvirt-cpuid.cfg
@@ -0,0 +1,25 @@
+name = "XenGuest2"
+uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+maxmem = 579
+memory = 394
+vcpus = 1
+pae = 1
+acpi = 0
+apic = 0
+viridian = 0
+rtc_timeoffset = 0
+localtime = 0
+on_poweroff = "destroy"
+on_reboot = "restart"
+on_crash = "restart"
+device_model = "/usr/lib/xen/bin/qemu-system-i386"
+sdl = 0
+vnc = 1
+vncunused = 1
+vnclisten = "127.0.0.1"
+parallel = "none"
+serial = "none"
+builder = "hvm"
+boot = "d"
+nestedhvm = 1
+cpuid = "host,tm=0,sse3=1,page1gb=x"
diff --git a/tests/xlconfigdata/test-fullvirt-cpuid.xml
b/tests/xlconfigdata/test-fullvirt-cpuid.xml
new file mode 100644
index 0000000..277b419
--- /dev/null
+++ b/tests/xlconfigdata/test-fullvirt-cpuid.xml
@@ -0,0 +1,36 @@
+<domain type='xen'>
+ <name>XenGuest2</name>
+ <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>592896</memory>
+ <currentMemory unit='KiB'>403456</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='xenfv'>hvm</type>
+ <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>
+ <boot dev='cdrom'/>
+ </os>
+ <features>
+ <pae/>
+ </features>
+ <cpu mode='host-passthrough'>
+ <feature policy='force' name='vmx'/>
+ <feature policy='disable' name='tm'/>
+ <feature policy='force' name='pni'/>
+ <feature policy='optional' name='pdpe1gb'/>
+ </cpu>
+ <clock offset='variable' adjustment='0' basis='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <devices>
+ <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <graphics type='vnc' port='-1' autoport='yes'
listen='127.0.0.1'>
+ <listen type='address' address='127.0.0.1'/>
+ </graphics>
+ <video>
+ <model type='cirrus' vram='8192' heads='1'
primary='yes'/>
+ </video>
+ </devices>
+</domain>
diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
index 57a0a67..39f51e2 100644
--- a/tests/xlconfigtest.c
+++ b/tests/xlconfigtest.c
@@ -270,6 +270,7 @@ mymain(void)
DO_TEST("fullvirt-multi-timer");
DO_TEST("fullvirt-nestedhvm");
DO_TEST("fullvirt-nestedhvm-disabled");
+ DO_TEST("fullvirt-cpuid");
#ifdef LIBXL_HAVE_VNUMA
DO_TEST("fullvirt-vnuma");
DO_TEST_PARSE("fullvirt-vnuma-autocomplete", false);
2516
days inactive
2542
days old
22 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Jim Fehlig -
Joao Martins -
Marek Marczykowski-Górecki