[libvirt] [PATCH v2 0/4] Add setting CPU features (CPUID) with libxenlight driver.
Tests (patches 3 and 4) depends on libxl_domain_config test suite: https://www.redhat.com/archives/libvir-list/2017-February/msg01477.html But first two patches can be applied independently. Marek Marczykowski-Górecki (4): cpu: define sub-leaf 0 for leaf 7 in cpu_map.xml libxl: add support for CPUID features policy tests: switch libxlxml2domconfig test to use testXLInintCaps tests: check CPU features handling in libxl driver src/cpu/cpu_map.xml | 58 ++++++------- src/libxl/libxl_conf.c | 77 ++++++++++++++++- src/libxl/libxl_conf.h | 1 +- tests/libxlxml2domconfigdata/basic-pv.xml | 2 +- tests/libxlxml2domconfigdata/fullvirt-cpuid.json | 64 ++++++++++++++- tests/libxlxml2domconfigdata/fullvirt-cpuid.xml | 37 ++++++++- tests/libxlxml2domconfigtest.c | 3 +- 7 files changed, 207 insertions(+), 35 deletions(-) create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.json create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.xml base-commit: fbcc08b245bb7d5502633d9cb4048281cc9d8532 -- git-series 0.9.1
CPUID leaf 7 is sub-leaf aware. Add missing attribute. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes since v1: - format ecx_in='0x00' --- src/cpu/cpu_map.xml | 58 +++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index 29b5b59..8e7ac49 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -195,93 +195,93 @@ <!-- cpuid function 0x7 ecx 0x0 features --> <feature name='fsgsbase'> - <cpuid eax_in='0x07' ebx='0x00000001'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000001'/> </feature> <feature name='tsc_adjust'> <!-- tsc-adjust --> - <cpuid eax_in='0x07' ebx='0x00000002'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000002'/> </feature> <feature name='bmi1'> - <cpuid eax_in='0x07' ebx='0x00000008'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000008'/> </feature> <feature name='hle'> - <cpuid eax_in='0x07' ebx='0x00000010'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000010'/> </feature> <feature name='avx2'> - <cpuid eax_in='0x07' ebx='0x00000020'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000020'/> </feature> <feature name='smep'> - <cpuid eax_in='0x07' ebx='0x00000080'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000080'/> </feature> <feature name='bmi2'> - <cpuid eax_in='0x07' ebx='0x00000100'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000100'/> </feature> <feature name='erms'> - <cpuid eax_in='0x07' ebx='0x00000200'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000200'/> </feature> <feature name='invpcid'> - <cpuid eax_in='0x07' ebx='0x00000400'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000400'/> </feature> <feature name='rtm'> - <cpuid eax_in='0x07' ebx='0x00000800'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000800'/> </feature> <feature name='cmt'> - <cpuid eax_in='0x07' ebx='0x00001000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00001000'/> </feature> <feature name='mpx'> - <cpuid eax_in='0x07' ebx='0x00004000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00004000'/> </feature> <feature name='avx512f'> - <cpuid eax_in='0x07' ebx='0x00010000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00010000'/> </feature> <feature name='avx512dq'> - <cpuid eax_in='0x07' ebx='0x00020000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00020000'/> </feature> <feature name='rdseed'> - <cpuid eax_in='0x07' ebx='0x00040000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00040000'/> </feature> <feature name='adx'> - <cpuid eax_in='0x07' ebx='0x00080000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00080000'/> </feature> <feature name='smap'> - <cpuid eax_in='0x07' ebx='0x00100000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00100000'/> </feature> <feature name='avx512ifma'> - <cpuid eax_in='0x07' ebx='0x00200000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00200000'/> </feature> <feature name='clflushopt'> - <cpuid eax_in='0x07' ebx='0x00800000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00800000'/> </feature> <feature name='avx512pf'> - <cpuid eax_in='0x07' ebx='0x04000000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x04000000'/> </feature> <feature name='avx512er'> - <cpuid eax_in='0x07' ebx='0x08000000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x08000000'/> </feature> <feature name='avx512cd'> - <cpuid eax_in='0x07' ebx='0x10000000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x10000000'/> </feature> <feature name='avx512bw'> - <cpuid eax_in='0x07' ebx='0x40000000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x40000000'/> </feature> <feature name='avx512vl'> - <cpuid eax_in='0x07' ebx='0x80000000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x80000000'/> </feature> <feature name='avx512vbmi'> - <cpuid eax_in='0x07' ecx='0x00000002'/> + <cpuid eax_in='0x07' ecx_in='0x00' ecx='0x00000002'/> </feature> <feature name='pku'> - <cpuid eax_in='0x07' ecx='0x00000008'/> + <cpuid eax_in='0x07' ecx_in='0x00' ecx='0x00000008'/> </feature> <feature name='ospke'> - <cpuid eax_in='0x07' ecx='0x00000010'/> + <cpuid eax_in='0x07' ecx_in='0x00' ecx='0x00000010'/> </feature> <feature name='avx512-4vnniw'> - <cpuid eax_in='0x07' edx='0x00000004'/> + <cpuid eax_in='0x07' ecx_in='0x00' edx='0x00000004'/> </feature> <feature name='avx512-4fmaps'> - <cpuid eax_in='0x07' edx='0x00000008'/> + <cpuid eax_in='0x07' ecx_in='0x00' edx='0x00000008'/> </feature> <!-- Processor Extended State Enumeration sub leaf 1 --> -- git-series 0.9.1
On 07/03/2017 09:03 PM, Marek Marczykowski-Górecki wrote:
CPUID leaf 7 is sub-leaf aware. Add missing attribute.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes since v1: - format ecx_in='0x00' --- src/cpu/cpu_map.xml | 58 +++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 29 deletions(-)
This change seems fine to me, but I'd prefer an ACK from one of the related maintainers, e.g. jdenemar. Regards, Jim
diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index 29b5b59..8e7ac49 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -195,93 +195,93 @@
<!-- cpuid function 0x7 ecx 0x0 features --> <feature name='fsgsbase'> - <cpuid eax_in='0x07' ebx='0x00000001'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000001'/> </feature> <feature name='tsc_adjust'> <!-- tsc-adjust --> - <cpuid eax_in='0x07' ebx='0x00000002'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000002'/> </feature> <feature name='bmi1'> - <cpuid eax_in='0x07' ebx='0x00000008'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000008'/> </feature> <feature name='hle'> - <cpuid eax_in='0x07' ebx='0x00000010'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000010'/> </feature> <feature name='avx2'> - <cpuid eax_in='0x07' ebx='0x00000020'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000020'/> </feature> <feature name='smep'> - <cpuid eax_in='0x07' ebx='0x00000080'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000080'/> </feature> <feature name='bmi2'> - <cpuid eax_in='0x07' ebx='0x00000100'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000100'/> </feature> <feature name='erms'> - <cpuid eax_in='0x07' ebx='0x00000200'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000200'/> </feature> <feature name='invpcid'> - <cpuid eax_in='0x07' ebx='0x00000400'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000400'/> </feature> <feature name='rtm'> - <cpuid eax_in='0x07' ebx='0x00000800'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00000800'/> </feature> <feature name='cmt'> - <cpuid eax_in='0x07' ebx='0x00001000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00001000'/> </feature> <feature name='mpx'> - <cpuid eax_in='0x07' ebx='0x00004000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00004000'/> </feature> <feature name='avx512f'> - <cpuid eax_in='0x07' ebx='0x00010000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00010000'/> </feature> <feature name='avx512dq'> - <cpuid eax_in='0x07' ebx='0x00020000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00020000'/> </feature> <feature name='rdseed'> - <cpuid eax_in='0x07' ebx='0x00040000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00040000'/> </feature> <feature name='adx'> - <cpuid eax_in='0x07' ebx='0x00080000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00080000'/> </feature> <feature name='smap'> - <cpuid eax_in='0x07' ebx='0x00100000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00100000'/> </feature> <feature name='avx512ifma'> - <cpuid eax_in='0x07' ebx='0x00200000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00200000'/> </feature> <feature name='clflushopt'> - <cpuid eax_in='0x07' ebx='0x00800000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x00800000'/> </feature> <feature name='avx512pf'> - <cpuid eax_in='0x07' ebx='0x04000000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x04000000'/> </feature> <feature name='avx512er'> - <cpuid eax_in='0x07' ebx='0x08000000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x08000000'/> </feature> <feature name='avx512cd'> - <cpuid eax_in='0x07' ebx='0x10000000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x10000000'/> </feature> <feature name='avx512bw'> - <cpuid eax_in='0x07' ebx='0x40000000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x40000000'/> </feature> <feature name='avx512vl'> - <cpuid eax_in='0x07' ebx='0x80000000'/> + <cpuid eax_in='0x07' ecx_in='0x00' ebx='0x80000000'/> </feature>
<feature name='avx512vbmi'> - <cpuid eax_in='0x07' ecx='0x00000002'/> + <cpuid eax_in='0x07' ecx_in='0x00' ecx='0x00000002'/> </feature> <feature name='pku'> - <cpuid eax_in='0x07' ecx='0x00000008'/> + <cpuid eax_in='0x07' ecx_in='0x00' ecx='0x00000008'/> </feature> <feature name='ospke'> - <cpuid eax_in='0x07' ecx='0x00000010'/> + <cpuid eax_in='0x07' ecx_in='0x00' ecx='0x00000010'/> </feature>
<feature name='avx512-4vnniw'> - <cpuid eax_in='0x07' edx='0x00000004'/> + <cpuid eax_in='0x07' ecx_in='0x00' edx='0x00000004'/> </feature> <feature name='avx512-4fmaps'> - <cpuid eax_in='0x07' edx='0x00000008'/> + <cpuid eax_in='0x07' ecx_in='0x00' edx='0x00000008'/> </feature>
<!-- Processor Extended State Enumeration sub leaf 1 -->
On Tue, Jul 04, 2017 at 05:03:44 +0200, Marek Marczykowski-Górecki wrote:
CPUID leaf 7 is sub-leaf aware. Add missing attribute.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes since v1: - format ecx_in='0x00' --- src/cpu/cpu_map.xml | 58 +++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 29 deletions(-)
ACK, I pushed this patch since it's not really related to the rest of this series. Jirka
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, accept only "host-passthrough" mode. 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@invisiblethingslab.com> --- Changes since v1: - use new ("libxl") syntax to set only bits explicitly mentioned in domain XML --- src/libxl/libxl_conf.c | 77 ++++++++++++++++++++++++++++++++++++++++--- src/libxl/libxl_conf.h | 1 +- 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index a0a53c2..0cf51a8 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -291,6 +291,44 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf) return 0; } +static const char *libxlTranslateCPUFeature(const char *virCPUFeatureName) +{ + /* libxl use different names for some CPUID bits */ + if (STREQ(virCPUFeatureName, "cx16")) + return "cmpxchg16"; + if (STREQ(virCPUFeatureName, "cid")) + return "cntxid"; + if (STREQ(virCPUFeatureName, "ds_cpl")) + return "dscpl"; + if (STREQ(virCPUFeatureName, "pclmuldq")) + return "pclmulqdq"; + if (STREQ(virCPUFeatureName, "pni")) + return "sse3"; + if (STREQ(virCPUFeatureName, "ht")) + return "htt"; + if (STREQ(virCPUFeatureName, "pn")) + return "psn"; + if (STREQ(virCPUFeatureName, "clflush")) + return "clfsh"; + if (STREQ(virCPUFeatureName, "sep")) + return "sysenter"; + if (STREQ(virCPUFeatureName, "cx8")) + return "cmpxchg8"; + if (STREQ(virCPUFeatureName, "nodeid_msr")) + return "nodeid"; + if (STREQ(virCPUFeatureName, "cr8legacy")) + return "altmovcr8"; + if (STREQ(virCPUFeatureName, "lahf_lm")) + return "lahfsahf"; + if (STREQ(virCPUFeatureName, "cmp_legacy")) + return "cmplegacy"; + if (STREQ(virCPUFeatureName, "fxsr_opt")) + return "ffxsr"; + if (STREQ(virCPUFeatureName, "pdpe1gb")) + return "page1gb"; + return virCPUFeatureName; +} + static int libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_ctx *ctx, @@ -376,10 +414,18 @@ 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; + char xlCPU[32]; + + 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"); @@ -394,20 +440,43 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, case VIR_CPU_FEATURE_DISABLE: case VIR_CPU_FEATURE_FORBID: + snprintf(xlCPU, + sizeof(xlCPU), + "%s=0", + 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; + } 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"))) hasHwVirt = false; break; case VIR_CPU_FEATURE_FORCE: case VIR_CPU_FEATURE_REQUIRE: + 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: break; } } + libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt); } - libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt); } if (def->nsounds > 0) { diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 264df11..8d89ccd 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -60,6 +60,7 @@ # define LIBXL_DUMP_DIR LIBXL_LIB_DIR "/dump" # define LIBXL_CHANNEL_DIR LIBXL_LIB_DIR "/channel/target" # define LIBXL_BOOTLOADER_PATH "pygrub" +# define LIBXL_DEFAULT_CPUID_REG_CONFIG "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" typedef struct _libxlDriverPrivate libxlDriverPrivate; -- git-series 0.9.1
On 07/03/2017 09:03 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, accept only "hostf-passthrough" mode. 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@invisiblethingslab.com> --- Changes since v1: - use new ("libxl") syntax to set only bits explicitly mentioned in domain XML --- src/libxl/libxl_conf.c | 77 ++++++++++++++++++++++++++++++++++++++++--- src/libxl/libxl_conf.h | 1 +- 2 files changed, 74 insertions(+), 4 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index a0a53c2..0cf51a8 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -291,6 +291,44 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf) return 0; }
+static const char *libxlTranslateCPUFeature(const char *virCPUFeatureName) +{ + /* libxl use different names for some CPUID bits */ + if (STREQ(virCPUFeatureName, "cx16")) + return "cmpxchg16"; + if (STREQ(virCPUFeatureName, "cid")) + return "cntxid"; + if (STREQ(virCPUFeatureName, "ds_cpl")) + return "dscpl"; + if (STREQ(virCPUFeatureName, "pclmuldq")) + return "pclmulqdq"; + if (STREQ(virCPUFeatureName, "pni")) + return "sse3"; + if (STREQ(virCPUFeatureName, "ht")) + return "htt"; + if (STREQ(virCPUFeatureName, "pn")) + return "psn"; + if (STREQ(virCPUFeatureName, "clflush")) + return "clfsh"; + if (STREQ(virCPUFeatureName, "sep")) + return "sysenter"; + if (STREQ(virCPUFeatureName, "cx8")) + return "cmpxchg8"; + if (STREQ(virCPUFeatureName, "nodeid_msr")) + return "nodeid"; + if (STREQ(virCPUFeatureName, "cr8legacy")) + return "altmovcr8"; + if (STREQ(virCPUFeatureName, "lahf_lm")) + return "lahfsahf"; + if (STREQ(virCPUFeatureName, "cmp_legacy")) + return "cmplegacy"; + if (STREQ(virCPUFeatureName, "fxsr_opt")) + return "ffxsr"; + if (STREQ(virCPUFeatureName, "pdpe1gb")) + return "page1gb"; + return virCPUFeatureName; +} +
I don't have an alternative, but I see a mapping table becoming stale as new CPU features are added to libxl.
static int libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_ctx *ctx, @@ -376,10 +414,18 @@ 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; + char xlCPU[32]; + + if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported cpu mode '%s'"), + virCPUModeTypeToString(def->cpu->mode)); + return -1; + } +
Although trivial, IMO this change should be in a separate patch where it will be easy to find.
if (ARCH_IS_X86(def->os.arch)) { vmx = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx"); @@ -394,20 +440,43 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
case VIR_CPU_FEATURE_DISABLE: case VIR_CPU_FEATURE_FORBID: + snprintf(xlCPU, + sizeof(xlCPU), + "%s=0", + 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; + } 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")))
Spurious change.
hasHwVirt = false; break;
case VIR_CPU_FEATURE_FORCE: case VIR_CPU_FEATURE_REQUIRE: + 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: break; } } + libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt); } - libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt);
This change also seems unrelated. Perhaps it can be combined with the change forbidding all but host_passthrough CPU model.
}
if (def->nsounds > 0) { diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 264df11..8d89ccd 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -60,6 +60,7 @@ # define LIBXL_DUMP_DIR LIBXL_LIB_DIR "/dump" # define LIBXL_CHANNEL_DIR LIBXL_LIB_DIR "/channel/target" # define LIBXL_BOOTLOADER_PATH "pygrub" +# define LIBXL_DEFAULT_CPUID_REG_CONFIG "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
This doesn't appear to be used anywhere in the patch. Regards, Jim
On Mon, Jul 17, 2017 at 03:57:17PM -0600, Jim Fehlig wrote:
On 07/03/2017 09:03 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, accept only "hostf-passthrough" mode. 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@invisiblethingslab.com> --- Changes since v1: - use new ("libxl") syntax to set only bits explicitly mentioned in domain XML --- src/libxl/libxl_conf.c | 77 ++++++++++++++++++++++++++++++++++++++++--- src/libxl/libxl_conf.h | 1 +- 2 files changed, 74 insertions(+), 4 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index a0a53c2..0cf51a8 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -291,6 +291,44 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf) return 0; } +static const char *libxlTranslateCPUFeature(const char *virCPUFeatureName) +{ + /* libxl use different names for some CPUID bits */ + if (STREQ(virCPUFeatureName, "cx16")) + return "cmpxchg16"; + if (STREQ(virCPUFeatureName, "cid")) + return "cntxid"; + if (STREQ(virCPUFeatureName, "ds_cpl")) + return "dscpl"; + if (STREQ(virCPUFeatureName, "pclmuldq")) + return "pclmulqdq"; + if (STREQ(virCPUFeatureName, "pni")) + return "sse3"; + if (STREQ(virCPUFeatureName, "ht")) + return "htt"; + if (STREQ(virCPUFeatureName, "pn")) + return "psn"; + if (STREQ(virCPUFeatureName, "clflush")) + return "clfsh"; + if (STREQ(virCPUFeatureName, "sep")) + return "sysenter"; + if (STREQ(virCPUFeatureName, "cx8")) + return "cmpxchg8"; + if (STREQ(virCPUFeatureName, "nodeid_msr")) + return "nodeid"; + if (STREQ(virCPUFeatureName, "cr8legacy")) + return "altmovcr8"; + if (STREQ(virCPUFeatureName, "lahf_lm")) + return "lahfsahf"; + if (STREQ(virCPUFeatureName, "cmp_legacy")) + return "cmplegacy"; + if (STREQ(virCPUFeatureName, "fxsr_opt")) + return "ffxsr"; + if (STREQ(virCPUFeatureName, "pdpe1gb")) + return "page1gb"; + return virCPUFeatureName; +} +
I don't have an alternative, but I see a mapping table becoming stale as new CPU features are added to libxl.
This is one reason why v1 used name->bit mapping of libvirt. But it required "old" (xend) config syntax and had a side effect of setting all bits of specified CPU model (yes, it required CPU model specified in XML).
static int libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_ctx *ctx, @@ -376,10 +414,18 @@ 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; + char xlCPU[32]; + + if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported cpu mode '%s'"), + virCPUModeTypeToString(def->cpu->mode)); + return -1; + } +
Although trivial, IMO this change should be in a separate patch where it will be easy to find.
Ok.
if (ARCH_IS_X86(def->os.arch)) { vmx = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx"); @@ -394,20 +440,43 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, case VIR_CPU_FEATURE_DISABLE: case VIR_CPU_FEATURE_FORBID: + snprintf(xlCPU, + sizeof(xlCPU), + "%s=0", + 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; + } 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")))
Spurious change.
hasHwVirt = false; break; case VIR_CPU_FEATURE_FORCE: case VIR_CPU_FEATURE_REQUIRE: + 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: break; } } + libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt); } - libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt);
This change also seems unrelated. Perhaps it can be combined with the change forbidding all but host_passthrough CPU model.
} if (def->nsounds > 0) { diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 264df11..8d89ccd 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -60,6 +60,7 @@ # define LIBXL_DUMP_DIR LIBXL_LIB_DIR "/dump" # define LIBXL_CHANNEL_DIR LIBXL_LIB_DIR "/channel/target" # define LIBXL_BOOTLOADER_PATH "pygrub" +# define LIBXL_DEFAULT_CPUID_REG_CONFIG "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
This doesn't appear to be used anywhere in the patch.
Ah, indeed, leftover of v1. -- 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?
On 07/17/2017 03:57 PM, Jim Fehlig wrote:
On 07/03/2017 09:03 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, accept only "hostf-passthrough" mode. 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@invisiblethingslab.com> --- Changes since v1: - use new ("libxl") syntax to set only bits explicitly mentioned in domain XML --- src/libxl/libxl_conf.c | 77 ++++++++++++++++++++++++++++++++++++++++--- src/libxl/libxl_conf.h | 1 +- 2 files changed, 74 insertions(+), 4 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index a0a53c2..0cf51a8 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -291,6 +291,44 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf) return 0; } +static const char *libxlTranslateCPUFeature(const char *virCPUFeatureName) +{ + /* libxl use different names for some CPUID bits */ + if (STREQ(virCPUFeatureName, "cx16")) + return "cmpxchg16"; + if (STREQ(virCPUFeatureName, "cid")) + return "cntxid"; + if (STREQ(virCPUFeatureName, "ds_cpl")) + return "dscpl"; + if (STREQ(virCPUFeatureName, "pclmuldq")) + return "pclmulqdq"; + if (STREQ(virCPUFeatureName, "pni")) + return "sse3"; + if (STREQ(virCPUFeatureName, "ht")) + return "htt"; + if (STREQ(virCPUFeatureName, "pn")) + return "psn"; + if (STREQ(virCPUFeatureName, "clflush")) + return "clfsh"; + if (STREQ(virCPUFeatureName, "sep")) + return "sysenter"; + if (STREQ(virCPUFeatureName, "cx8")) + return "cmpxchg8"; + if (STREQ(virCPUFeatureName, "nodeid_msr")) + return "nodeid"; + if (STREQ(virCPUFeatureName, "cr8legacy")) + return "altmovcr8"; + if (STREQ(virCPUFeatureName, "lahf_lm")) + return "lahfsahf"; + if (STREQ(virCPUFeatureName, "cmp_legacy")) + return "cmplegacy"; + if (STREQ(virCPUFeatureName, "fxsr_opt")) + return "ffxsr"; + if (STREQ(virCPUFeatureName, "pdpe1gb")) + return "page1gb"; + return virCPUFeatureName; +} +
I don't have an alternative, but I see a mapping table becoming stale as new CPU features are added to libxl.
static int libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_ctx *ctx, @@ -376,10 +414,18 @@ 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; + char xlCPU[32]; + + if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported cpu mode '%s'"), + virCPUModeTypeToString(def->cpu->mode)); + return -1; + } +
Although trivial, IMO this change should be in a separate patch where it will be easy to find.
if (ARCH_IS_X86(def->os.arch)) { vmx = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx"); @@ -394,20 +440,43 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, case VIR_CPU_FEATURE_DISABLE: case VIR_CPU_FEATURE_FORBID: + snprintf(xlCPU, + sizeof(xlCPU), + "%s=0", + 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; + } 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")))
Spurious change.
hasHwVirt = false; break; case VIR_CPU_FEATURE_FORCE: case VIR_CPU_FEATURE_REQUIRE: + 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: break; } } + libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt); } - libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt);
This change also seems unrelated. Perhaps it can be combined with the change forbidding all but host_passthrough CPU model.
} if (def->nsounds > 0) { diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 264df11..8d89ccd 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -60,6 +60,7 @@ # define LIBXL_DUMP_DIR LIBXL_LIB_DIR "/dump" # define LIBXL_CHANNEL_DIR LIBXL_LIB_DIR "/channel/target" # define LIBXL_BOOTLOADER_PATH "pygrub" +# define LIBXL_DEFAULT_CPUID_REG_CONFIG "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
This doesn't appear to be used anywhere in the patch.
Sorry, I forgot to mention that this series should also include a patch for domXML <-> xl.cfg conversions, plus a xlconfigtest :-). IMO, the libxl CPUID format is sufficient for now. Regards, Jim
As name suggests, it's a better choice for libxl test. Important differences: - advertise x86_64 guests support - initialize host CPU caps Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes since v1: - new patch, applicable over Jim's test suite for libxl_domain_config generator Cc: Jim Fehlig <jfehlig@suse.com> --- tests/libxlxml2domconfigdata/basic-pv.xml | 2 +- tests/libxlxml2domconfigtest.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/libxlxml2domconfigdata/basic-pv.xml b/tests/libxlxml2domconfigdata/basic-pv.xml index b3bc601..eba3cc6 100644 --- a/tests/libxlxml2domconfigdata/basic-pv.xml +++ b/tests/libxlxml2domconfigdata/basic-pv.xml @@ -6,7 +6,7 @@ <vcpu>4</vcpu> <bootloader>pygrub</bootloader> <os> - <type arch='i686' machine='xenpv'>linux</type> + <type arch='x86_64' machine='xenpv'>linux</type> </os> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c index d943cf2..ecdb1fe 100644 --- a/tests/libxlxml2domconfigtest.c +++ b/tests/libxlxml2domconfigtest.c @@ -173,7 +173,7 @@ mymain(void) return EXIT_FAILURE; } - if ((xencaps = testXenCapsInit()) == NULL) + if ((xencaps = testXLInitCaps()) == NULL) return EXIT_FAILURE; # define DO_TEST(name) \ -- git-series 0.9.1
On 07/03/2017 09:03 PM, Marek Marczykowski-Górecki wrote:
As name suggests, it's a better choice for libxl test. Important differences: - advertise x86_64 guests support - initialize host CPU caps
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes since v1: - new patch, applicable over Jim's test suite for libxl_domain_config generator
Cc: Jim Fehlig <jfehlig@suse.com> --- tests/libxlxml2domconfigdata/basic-pv.xml | 2 +- tests/libxlxml2domconfigtest.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/libxlxml2domconfigdata/basic-pv.xml b/tests/libxlxml2domconfigdata/basic-pv.xml index b3bc601..eba3cc6 100644 --- a/tests/libxlxml2domconfigdata/basic-pv.xml +++ b/tests/libxlxml2domconfigdata/basic-pv.xml @@ -6,7 +6,7 @@ <vcpu>4</vcpu> <bootloader>pygrub</bootloader> <os> - <type arch='i686' machine='xenpv'>linux</type> + <type arch='x86_64' machine='xenpv'>linux</type> </os> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c index d943cf2..ecdb1fe 100644 --- a/tests/libxlxml2domconfigtest.c +++ b/tests/libxlxml2domconfigtest.c @@ -173,7 +173,7 @@ mymain(void) return EXIT_FAILURE; }
- if ((xencaps = testXenCapsInit()) == NULL) + if ((xencaps = testXLInitCaps()) == NULL) return EXIT_FAILURE;
This patch looks like it could be squashed into "libxl: Add a test suite for libxl_domain_config generator". Regards, Jim
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@invisiblethingslab.com> --- Changes since v1: - rewritten to Jim's test suite for libxl_domain_config generator Cc: Jim Fehlig <jfehlig@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 ecdb1fe..1627e9e 100644 --- a/tests/libxlxml2domconfigtest.c +++ b/tests/libxlxml2domconfigtest.c @@ -189,6 +189,7 @@ mymain(void) DO_TEST("basic-pv"); DO_TEST("basic-hvm"); DO_TEST("moredevs-hvm"); + DO_TEST("fullvirt-cpuid"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- git-series 0.9.1
On 07/03/2017 09:03 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@invisiblethingslab.com> --- Changes since v1: - rewritten to Jim's test suite for libxl_domain_config generator
This looks fine, but as you note depends on the "libxl: Add a test suite for libxl_domain_config generator" patch. And that patch depends on a satisfactory resolution to mocking the <emulator> checks as mentioned here https://www.redhat.com/archives/libvir-list/2017-July/msg00541.html I've stared at that problem too long and might be overlooking the obvious. Any suggestions appreciated. Regards, Jim
Cc: Jim Fehlig <jfehlig@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 ecdb1fe..1627e9e 100644 --- a/tests/libxlxml2domconfigtest.c +++ b/tests/libxlxml2domconfigtest.c @@ -189,6 +189,7 @@ mymain(void) DO_TEST("basic-pv"); DO_TEST("basic-hvm"); DO_TEST("moredevs-hvm"); + DO_TEST("fullvirt-cpuid");
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; }
On Tue, Jul 04, 2017 at 05:03:43AM +0200, Marek Marczykowski-Górecki wrote:
Tests (patches 3 and 4) depends on libxl_domain_config test suite: https://www.redhat.com/archives/libvir-list/2017-February/msg01477.html
But first two patches can be applied independently.
Anything missing here? If test suite cannot be merged for any reason, could you at least merge the functionality itself?
Marek Marczykowski-Górecki (4): cpu: define sub-leaf 0 for leaf 7 in cpu_map.xml libxl: add support for CPUID features policy tests: switch libxlxml2domconfig test to use testXLInintCaps tests: check CPU features handling in libxl driver
src/cpu/cpu_map.xml | 58 ++++++------- src/libxl/libxl_conf.c | 77 ++++++++++++++++- src/libxl/libxl_conf.h | 1 +- tests/libxlxml2domconfigdata/basic-pv.xml | 2 +- tests/libxlxml2domconfigdata/fullvirt-cpuid.json | 64 ++++++++++++++- tests/libxlxml2domconfigdata/fullvirt-cpuid.xml | 37 ++++++++- tests/libxlxml2domconfigtest.c | 3 +- 7 files changed, 207 insertions(+), 35 deletions(-) create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.json create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.xml
base-commit: fbcc08b245bb7d5502633d9cb4048281cc9d8532
-- 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?
participants (3)
-
Jim Fehlig -
Jiri Denemark -
Marek Marczykowski-Górecki