On Thu, Jun 29, 2017 at 03:11:41 +0200, Marek Marczykowski-Górecki wrote: Why? Zero is the default value for ecx_in, I don't see a strong reason for mentioning it explicitly. If we do this, we should set the value to 0x00 though. Jirka

Set CPU features in appropriate libxl structure. Use old "xend" syntax, because it allow control over any bit and libvirt already have API for translating features to appropriate cpuid bits. And also features naming in libxl do not match the one of libvirt in multiple cases. Side effect is that all the bits derived from CPU <model> (which is required anyway) are also set as "required". 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. --- src/libxl/libxl_conf.c | 139 ++++++++++++++++++++++++++++++++++-------- src/libxl/libxl_conf.h | 1 +- 2 files changed, 116 insertions(+), 24 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 938e09d..4c400f4 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -291,6 +291,93 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf) return 0; } +static int libxlMakeCPUID(virArch arch, + const virCPUDef *cpu, + libxl_cpuid_policy_list *cpuid) +{ + virCPUDataPtr cpu_policy[4]; + /* forced, required, disabled, forbidden */ + char libxl_policy_char[4] = { '1', '1', '0', '0' }; + int leaf, subleaf, reg; + size_t i, j; + char leaf_text[48]; + char *reg_text; + int policy_index = 0; + uint32_t reg_value; + int ret = -1; + int cpuid_bit; + /* supported values, based on libxl_cpuid.c */ + int leafs[] = { 1, 6, 7, 0x80000001 }; + int subleafs[] = { -1, -1, 0, -1 }; + + if (cpuEncode(arch, + cpu, + &cpu_policy[0], + &cpu_policy[1], + NULL, + &cpu_policy[2], + &cpu_policy[3], + NULL)) + return -1; + + for (i = 0; i < sizeof(leafs)/sizeof(*leafs); i++) { + leaf = leafs[i]; + subleaf = subleafs[i]; + + for (reg = 0; reg < 4; reg++) { + if (subleaf != -1) + snprintf(leaf_text, sizeof(leaf_text), + "%#x,%d:e%cx=" LIBXL_DEFAULT_CPUID_REG_CONFIG, + leaf, subleaf, 'a'+reg); + else + snprintf(leaf_text, sizeof(leaf_text), + "%#x:e%cx=" LIBXL_DEFAULT_CPUID_REG_CONFIG, + leaf, 'a'+reg); + + reg_text = strstr(leaf_text, "=") + 1; + + for (policy_index = 0; policy_index < 4; policy_index++) { + /* search for this leaf in CPUData structure */ + for (j = 0; j < cpu_policy[policy_index]->data.x86.len; j++) { + virCPUx86CPUID *cpuid_policy = + &cpu_policy[policy_index]->data.x86.data[j]; + + if (cpuid_policy->eax_in == leaf && + (subleaf == -1 || cpuid_policy->ecx_in == subleaf)) { + + switch (reg) { + case 0: reg_value = cpuid_policy->eax; break; + case 1: reg_value = cpuid_policy->ebx; break; + case 2: reg_value = cpuid_policy->ecx; break; + case 3: reg_value = cpuid_policy->edx; break; + } + + for (cpuid_bit = 0; cpuid_bit < 32; cpuid_bit++) { + if (reg_value & (1<<cpuid_bit)) + reg_text[31-cpuid_bit] = libxl_policy_char[policy_index]; + } + } + } + } + + /* set it in domain config only if any bit was modified */ + if (STRNEQ(reg_text, LIBXL_DEFAULT_CPUID_REG_CONFIG)) { + if (libxl_cpuid_parse_config_xend(cpuid, leaf_text)) + goto cleanup; + } + } + } + + ret = 0; + + cleanup: + virCPUDataFree(cpu_policy[0]); + virCPUDataFree(cpu_policy[1]); + virCPUDataFree(cpu_policy[2]); + virCPUDataFree(cpu_policy[3]); + return ret; +} + static int libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_ctx *ctx, @@ -376,38 +463,42 @@ 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)) { - bool hasHwVirt = false; - bool svm = false, vmx = false; + if (caps && def->cpu) { + if (def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) { + bool hasHwVirt = false; + bool svm = false, vmx = false; - 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"); - hasHwVirt = vmx | svm; - } + 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"); + hasHwVirt = vmx | svm; + } - if (def->cpu->nfeatures) { - for (i = 0; i < def->cpu->nfeatures; i++) { + if (def->cpu->nfeatures) { + for (i = 0; i < def->cpu->nfeatures; i++) { - switch (def->cpu->features[i].policy) { + switch (def->cpu->features[i].policy) { - 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"))) - hasHwVirt = false; - break; + 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"))) + hasHwVirt = false; + break; - case VIR_CPU_FEATURE_FORCE: - case VIR_CPU_FEATURE_REQUIRE: - case VIR_CPU_FEATURE_OPTIONAL: - case VIR_CPU_FEATURE_LAST: - break; + case VIR_CPU_FEATURE_FORCE: + case VIR_CPU_FEATURE_REQUIRE: + 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 (libxlMakeCPUID(def->os.arch, def->cpu, &b_info->cpuid)) + return -1; } 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

libxl contains a method to dump libxl_domain_config in json format, which is really convenient for tests. Unfortunately it require libxl_ctx, which can be only created when running on Xen (wither in dom0 or domU). Because of this, skip the test in other cases. For now, have just two tests, including just-introduced CPUID handling, but this commit introduces a framework for further tests of src/libxl/libxl_conf.c file. --- tests/Makefile.am | 8 +- tests/libxlxmltest.c | 232 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 238 insertions(+), 2 deletions(-) create mode 100644 tests/libxlxmltest.c diff --git a/tests/Makefile.am b/tests/Makefile.am index 19986dc..06c5c73 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -274,7 +274,7 @@ test_programs += xml2sexprtest sexpr2xmltest \ endif WITH_XEN if WITH_LIBXL -test_programs += xlconfigtest +test_programs += xlconfigtest libxlxmltest endif WITH_LIBXL if WITH_QEMU @@ -528,9 +528,13 @@ libxl_LDADDS += $(LDADDS) xlconfigtest_SOURCES = \ xlconfigtest.c testutilsxen.c testutilsxen.h \ testutils.c testutils.h +libxlxmltest_SOURCES = \ + libxlxmltest.c testutilsxen.c testutilsxen.h \ + testutils.c testutils.h xlconfigtest_LDADD =$(libxl_LDADDS) +libxlxmltest_LDADD =$(libxl_LDADDS) else ! WITH_LIBXL -EXTRA_DIST += xlconfigtest.c +EXTRA_DIST += xlconfigtest.c libxlxmltest.c endif ! WITH_LIBXL QEMUMONITORTESTUTILS_SOURCES = \ diff --git a/tests/libxlxmltest.c b/tests/libxlxmltest.c new file mode 100644 index 0000000..7e11af4 --- /dev/null +++ b/tests/libxlxmltest.c @@ -0,0 +1,232 @@ +/* + * libxlxmltest.c: Test xl.cfg(5) <-> libxl_domain_config conversion + * + * Copyright (C) 2007, 2010-2011, 2014 Red Hat, Inc. + * Copyright (c) 2015 SUSE LINUX Products GmbH, Nuernberg, Germany. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Daniel P. Berrange <berrange(a)redhat.com&gt; + * Author: Kiarie Kahurani <davidkiarie4(a)gmail.com&gt; + * Author: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com&gt; + * + */ + +#include <config.h> + +#include <stdio.h> +#include <string.h> +#include <unistd.h> +#ifdef WITH_YAJL2 +# define HAVE_YAJL_V2 1 +#endif +#include <libxl.h> +#include <libxl_json.h> + +#include "internal.h" +#include "datatypes.h" +#include "viralloc.h" +#include "virstring.h" +#include "testutils.h" +#include "testutilsxen.h" +#include "libxl/libxl_conf.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +static virCapsPtr caps; +static virDomainXMLOptionPtr xmlopt; +xentoollog_logger_stdiostream *logger; +libxl_ctx *ctx; + + +/* + * This function provides a mechanism to replace variables in test + * data files whose values are discovered at built time. + */ +static char * +testReplaceVarsXML(const char *xml) +{ + char *xmlcfgData; + char *replacedXML; + + if (virTestLoadFile(xml, &xmlcfgData) < 0) + return NULL; + + replacedXML = virStringReplace(xmlcfgData, "/LIBXL_FIRMWARE_DIR", + LIBXL_FIRMWARE_DIR); + + /* libxl driver checks for emulator existence */ + replacedXML = virStringReplace(xmlcfgData, "/usr/lib/xen/bin/qemu-system-i386", + "/bin/true"); + + VIR_FREE(xmlcfgData); + return replacedXML; +} + +/* + * Parses domXML to virDomainDef object, which is then converted to json + * config and compared with expected config. + */ +static int +testCompareParseXML(const char *json, const char *xml, bool replaceVars) +{ + char *gotjsonData = NULL; + virConfPtr conf = NULL; + virConnectPtr conn = NULL; + size_t wrote = 0; + int ret = -1; + virDomainDefPtr def = NULL; + char *replacedXML = NULL; + virPortAllocatorPtr reservedGraphicsPorts = NULL; + libxl_domain_config d_config; + yajl_gen gen = NULL; + + conn = virGetConnect(); + if (!conn) goto fail; + + if (replaceVars) { + if (!(replacedXML = testReplaceVarsXML(xml))) + goto fail; + if (!(def = virDomainDefParseString(replacedXML, caps, xmlopt, + NULL, VIR_DOMAIN_XML_INACTIVE))) + goto fail; + } else { + if (!(def = virDomainDefParseFile(xml, caps, xmlopt, + NULL, VIR_DOMAIN_XML_INACTIVE))) + goto fail; + } + + if (!(reservedGraphicsPorts = virPortAllocatorNew("VNC", + LIBXL_VNC_PORT_MIN, + LIBXL_VNC_PORT_MAX, + 0))) + goto fail; + + if (libxlBuildDomainConfig(reservedGraphicsPorts, + def, + NULL, /* channelDir, unused in current version */ + ctx, + caps, + &d_config)) + goto fail; + + if (!(gen = libxl_yajl_gen_alloc(NULL))) + goto fail; + + if (libxl_domain_config_gen_json(gen, &d_config) != yajl_gen_status_ok) + goto fail; + + if (yajl_gen_get_buf(gen, + (const unsigned char **)&gotjsonData, + &wrote) != yajl_gen_status_ok) + goto fail; + + if (virTestCompareToFile(gotjsonData, json) < 0) + goto fail; + + ret = 0; + + fail: + VIR_FREE(replacedXML); + // yajl_gen_free handle also gotjsonData + if (gen) + yajl_gen_free(gen); + if (conf) + virConfFree(conf); + virObjectUnref(reservedGraphicsPorts); + virDomainDefFree(def); + virObjectUnref(conn); + + return ret; +} + +struct testInfo { + const char *name; + bool replaceVars; +}; + +static int +testCompareHelper(const void *data) +{ + int result = -1; + const struct testInfo *info = data; + char *xml = NULL; + char *json = NULL; + + if (virAsprintf(&xml, "%s/xlconfigdata/test-%s.xml", + abs_srcdir, info->name) < 0 || + virAsprintf(&json, "%s/xlconfigdata/test-%s.json", + abs_srcdir, info->name) < 0) + goto cleanup; + + result = testCompareParseXML(json, xml, info->replaceVars); + + cleanup: + VIR_FREE(xml); + VIR_FREE(json); + + return result; +} + + +static int +mymain(void) +{ + int ret = 0; + + logger = xtl_createlogger_stdiostream(stderr, XTL_PROGRESS, 0); + if (!logger) + return EXIT_FAILURE; + + if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, (xentoollog_logger*)logger)) + return EXIT_AM_SKIP; + + if (!(caps = testXLInitCaps())) + return EXIT_FAILURE; + + if (!(xmlopt = libxlCreateXMLConf())) + return EXIT_FAILURE; + +#define DO_TEST_PARSE(name, replace) \ + do { \ + struct testInfo info0 = { name, replace }; \ + if (virTestRun("Xen XML-2-json Parse " name, \ + testCompareHelper, &info0) < 0) \ + ret = -1; \ + } while (0) + +#define DO_TEST(name) \ + do { \ + DO_TEST_PARSE(name, false); \ + } while (0) + +#define DO_TEST_REPLACE_VARS(name) \ + do { \ + DO_TEST_PARSE(name, true); \ + } while (0) + + DO_TEST_REPLACE_VARS("fullvirt-ovmf"); + + DO_TEST("fullvirt-cpuid"); + + virObjectUnref(caps); + virObjectUnref(xmlopt); + libxl_ctx_free(ctx); + xtl_logger_destroy((xentoollog_logger*)logger); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIR_TEST_MAIN(mymain) -- git-series 0.9.1