cpu: Rework cpuCompare* APIs

Both cpuCompare* APIs are renamed to virCPUCompare*. And they should now
work for any guest CPU definition, i.e., even for host-passthrough
(trivial) and host-model CPUs. The implementation in x86 driver is
enhanced to provide a hint about -noTSX Broadwell and Haswell models
when appropriate.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

src/cpu/cpu.c

@@ -91,8 +91,9 @@ cpuGetSubDriverByName(const char *name)
 
 
 /**
- * cpuCompareXML:
+ * virCPUCompareXML:
  *
+ * @arch: CPU architecture
  * @host: host CPU definition
  * @xml: XML description of either guest or host CPU to be compared with @host
  * @failIncompatible: return an error instead of VIR_CPU_COMPARE_INCOMPATIBLE
@@ -107,25 +108,31 @@ cpuGetSubDriverByName(const char *name)
  * two CPUs are incompatible.
  */
 virCPUCompareResult
-cpuCompareXML(virCPUDefPtr host,
+virCPUCompareXML(virArch arch,
-              const char *xml,
+                 virCPUDefPtr host,
-              bool failIncompatible)
+                 const char *xml,
+                 bool failIncompatible)
 {
     xmlDocPtr doc = NULL;
     xmlXPathContextPtr ctxt = NULL;
     virCPUDefPtr cpu = NULL;
     virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR;
 
-    VIR_DEBUG("host=%p, xml=%s", host, NULLSTR(xml));
+    VIR_DEBUG("arch=%s, host=%p, xml=%s",
+              virArchToString(arch), host, NULLSTR(xml));
+
+    if (!xml) {
+        virReportError(VIR_ERR_INVALID_ARG, "%s", _("missing CPU definition"));
+        goto cleanup;
+    }
 
     if (!(doc = virXMLParseStringCtxt(xml, _("(CPU_definition)"), &ctxt)))
         goto cleanup;
 
-    cpu = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_AUTO);
+    if (!(cpu = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_AUTO)))
-    if (cpu == NULL)
         goto cleanup;
 
-    ret = cpuCompare(host, cpu, failIncompatible);
+    ret = virCPUCompare(arch, host, cpu, failIncompatible);
 
  cleanup:
     virCPUDefFree(cpu);
@@ -137,8 +144,9 @@ cpuCompareXML(virCPUDefPtr host,
 
 
 /**
- * cpuCompare:
+ * virCPUCompare:
  *
+ * @arch: CPU architecture
  * @host: host CPU definition
  * @cpu: either guest or host CPU to be compared with @host
  * @failIncompatible: return an error instead of VIR_CPU_COMPARE_INCOMPATIBLE
@@ -153,21 +161,23 @@ cpuCompareXML(virCPUDefPtr host,
  * two CPUs are incompatible.
  */
 virCPUCompareResult
-cpuCompare(virCPUDefPtr host,
+virCPUCompare(virArch arch,
-           virCPUDefPtr cpu,
+              virCPUDefPtr host,
-           bool failIncompatible)
+              virCPUDefPtr cpu,
+              bool failIncompatible)
 {
     struct cpuArchDriver *driver;
 
-    VIR_DEBUG("host=%p, cpu=%p", host, cpu);
+    VIR_DEBUG("arch=%s, host=%p, cpu=%p",
+              virArchToString(arch), host, cpu);
 
-    if ((driver = cpuGetSubDriver(host->arch)) == NULL)
+    if (!(driver = cpuGetSubDriver(arch)))
         return VIR_CPU_COMPARE_ERROR;
 
-    if (driver->compare == NULL) {
+    if (!driver->compare) {
         virReportError(VIR_ERR_NO_SUPPORT,
                        _("cannot compare CPUs of %s architecture"),
-                       virArchToString(host->arch));
+                       virArchToString(arch));
         return VIR_CPU_COMPARE_ERROR;
     }
 

src/cpu/cpu.h

@@ -45,7 +45,7 @@ struct _virCPUData {
 
 
 typedef virCPUCompareResult
-(*cpuArchCompare)   (virCPUDefPtr host,
+(*virCPUArchCompare)(virCPUDefPtr host,
                      virCPUDefPtr cpu,
                      bool failIncompatible);
 
@@ -116,7 +116,7 @@ struct cpuArchDriver {
     const char *name;
     const virArch *arch;
     unsigned int narch;
-    cpuArchCompare      compare;
+    virCPUArchCompare   compare;
     cpuArchDecode       decode;
     cpuArchEncode       encode;
     cpuArchDataFree     free;
@@ -134,16 +134,17 @@ struct cpuArchDriver {
 
 
 virCPUCompareResult
-cpuCompareXML(virCPUDefPtr host,
+virCPUCompareXML(virArch arch,
-              const char *xml,
+                 virCPUDefPtr host,
-              bool failIncompatible)
+                 const char *xml,
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+                 bool failIncompatible);
 
 virCPUCompareResult
-cpuCompare  (virCPUDefPtr host,
+virCPUCompare(virArch arch,
-             virCPUDefPtr cpu,
+              virCPUDefPtr host,
-             bool failIncompatible)
+              virCPUDefPtr cpu,
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+              bool failIncompatible)
+    ATTRIBUTE_NONNULL(3);
 
 int
 cpuDecode   (virCPUDefPtr cpu,

src/cpu/cpu_arm.c

@@ -112,9 +112,9 @@ armBaseline(virCPUDefPtr *cpus,
 }
 
 static virCPUCompareResult
-armCompare(virCPUDefPtr host ATTRIBUTE_UNUSED,
+virCPUarmCompare(virCPUDefPtr host ATTRIBUTE_UNUSED,
-           virCPUDefPtr cpu ATTRIBUTE_UNUSED,
+                 virCPUDefPtr cpu ATTRIBUTE_UNUSED,
-           bool failMessages ATTRIBUTE_UNUSED)
+                 bool failMessages ATTRIBUTE_UNUSED)
 {
     return VIR_CPU_COMPARE_IDENTICAL;
 }
@@ -123,7 +123,7 @@ struct cpuArchDriver cpuDriverArm = {
     .name = "arm",
     .arch = archs,
     .narch = ARRAY_CARDINALITY(archs),
-    .compare = armCompare,
+    .compare = virCPUarmCompare,
     .decode = NULL,
     .encode = NULL,
     .free = armDataFree,

src/cpu/cpu_ppc64.c

@@ -634,13 +634,24 @@ ppc64Compute(virCPUDefPtr host,
 }
 
 static virCPUCompareResult
-ppc64DriverCompare(virCPUDefPtr host,
+virCPUppc64Compare(virCPUDefPtr host,
                    virCPUDefPtr cpu,
                    bool failIncompatible)
 {
     virCPUCompareResult ret;
     char *message = NULL;
 
+    if (!host || !host->model) {
+        if (failIncompatible) {
+            virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s",
+                           _("unknown host CPU"));
+        } else {
+            VIR_WARN("unknown host CPU");
+            ret = VIR_CPU_COMPARE_INCOMPATIBLE;
+        }
+        return -1;
+    }
+
     ret = ppc64Compute(host, cpu, NULL, &message);
 
     if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) {
@@ -902,7 +913,7 @@ struct cpuArchDriver cpuDriverPPC64 = {
     .name       = "ppc64",
     .arch       = archs,
     .narch      = ARRAY_CARDINALITY(archs),
-    .compare    = ppc64DriverCompare,
+    .compare    = virCPUppc64Compare,
     .decode     = ppc64DriverDecode,
     .encode     = NULL,
     .free       = ppc64DriverFree,

src/cpu/cpu_x86.c

@@ -1057,7 +1057,8 @@ x86ModelFromCPU(const virCPUDef *cpu,
         policy != -1)
         return x86ModelNew();
 
-    if (policy == VIR_CPU_FEATURE_REQUIRE || policy == -1) {
+    if (cpu->model &&
+        (policy == VIR_CPU_FEATURE_REQUIRE || policy == -1)) {
         if (!(model = x86ModelFind(map, cpu->model))) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Unknown CPU model %s"), cpu->model);
@@ -1519,12 +1520,6 @@ x86Compute(virCPUDefPtr host,
     virArch arch;
     size_t i;
 
-    if (!cpu->model) {
-        virReportError(VIR_ERR_INVALID_ARG, "%s",
-                       _("no guest CPU model specified"));
-        return VIR_CPU_COMPARE_ERROR;
-    }
-
     if (cpu->arch != VIR_ARCH_NONE) {
         bool found = false;
 
@@ -1565,7 +1560,7 @@ x86Compute(virCPUDefPtr host,
     }
 
     if (!(map = virCPUx86GetMap()) ||
-        !(host_model = x86ModelFromCPU(host, map, VIR_CPU_FEATURE_REQUIRE)) ||
+        !(host_model = x86ModelFromCPU(host, map, -1)) ||
         !(cpu_force = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_FORCE)) ||
         !(cpu_require = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_REQUIRE)) ||
         !(cpu_optional = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_OPTIONAL)) ||
@@ -1664,25 +1659,68 @@ x86Compute(virCPUDefPtr host,
 
 
 static virCPUCompareResult
-x86Compare(virCPUDefPtr host,
+virCPUx86Compare(virCPUDefPtr host,
-           virCPUDefPtr cpu,
+                 virCPUDefPtr cpu,
-           bool failIncompatible)
+                 bool failIncompatible)
 {
-    virCPUCompareResult ret;
+    virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR;
+    virCPUx86MapPtr map;
+    virCPUx86ModelPtr model = NULL;
     char *message = NULL;
 
+    if (!host || !host->model) {
+        if (failIncompatible) {
+            virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s",
+                           _("unknown host CPU"));
+        } else {
+            VIR_WARN("unknown host CPU");
+            ret = VIR_CPU_COMPARE_INCOMPATIBLE;
+        }
+        goto cleanup;
+    }
+
     ret = x86Compute(host, cpu, NULL, &message);
 
-    if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) {
+    if (ret == VIR_CPU_COMPARE_INCOMPATIBLE) {
-        ret = VIR_CPU_COMPARE_ERROR;
+        bool noTSX = false;
-        if (message) {
+
-            virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", message);
+        if (STREQ_NULLABLE(cpu->model, "Haswell") ||
-        } else {
+            STREQ_NULLABLE(cpu->model, "Broadwell")) {
-            virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL);
+            if (!(map = virCPUx86GetMap()))
+                goto cleanup;
+
+            if (!(model = x86ModelFromCPU(cpu, map, -1)))
+                goto cleanup;
+
+            noTSX = !x86FeatureInData("hle", &model->data, map) ||
+                    !x86FeatureInData("rtm", &model->data, map);
+        }
+
+        if (failIncompatible) {
+            ret = VIR_CPU_COMPARE_ERROR;
+            if (message) {
+                if (noTSX) {
+                    virReportError(VIR_ERR_CPU_INCOMPATIBLE,
+                                   _("%s; try using '%s-noTSX' CPU model"),
+                                   message, cpu->model);
+                } else {
+                    virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", message);
+                }
+            } else {
+                if (noTSX) {
+                    virReportError(VIR_ERR_CPU_INCOMPATIBLE,
+                                   _("try using '%s-noTSX' CPU model"),
+                                   cpu->model);
+                } else {
+                    virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL);
+                }
+            }
         }
     }
-    VIR_FREE(message);
 
+ cleanup:
+    VIR_FREE(message);
+    x86ModelFree(model);
     return ret;
 }
 
@@ -1693,6 +1731,12 @@ x86GuestData(virCPUDefPtr host,
              virCPUDataPtr *data,
              char **message)
 {
+    if (!guest->model) {
+        virReportError(VIR_ERR_INVALID_ARG, "%s",
+                       _("no guest CPU model specified"));
+        return VIR_CPU_COMPARE_ERROR;
+    }
+
     return x86Compute(host, guest, data, message);
 }
 
@@ -2719,7 +2763,7 @@ struct cpuArchDriver cpuDriverX86 = {
     .name = "x86",
     .arch = archs,
     .narch = ARRAY_CARDINALITY(archs),
-    .compare    = x86Compare,
+    .compare    = virCPUx86Compare,
     .decode     = x86DecodeCPUData,
     .encode     = x86Encode,
     .free       = x86FreeCPUData,

src/libvirt_private.syms

@@ -970,8 +970,6 @@ virSecretObjSetValueSize;
 # cpu/cpu.h
 cpuBaseline;
 cpuBaselineXML;
-cpuCompare;
-cpuCompareXML;
 cpuDataFormat;
 cpuDataFree;
 cpuDataParse;
@@ -981,6 +979,8 @@ cpuGetModels;
 cpuGuestData;
 cpuNodeData;
 virCPUCheckFeature;
+virCPUCompare;
+virCPUCompareXML;
 virCPUDataCheckFeature;
 virCPUTranslate;
 virCPUUpdate;

src/libxl/libxl_driver.c

@@ -6369,18 +6369,8 @@ libxlConnectCompareCPU(virConnectPtr conn,
 
     cfg = libxlDriverConfigGet(driver);
 
-    if (!cfg->caps->host.cpu ||
+    ret = virCPUCompareXML(cfg->caps->host.arch, cfg->caps->host.cpu,
-        !cfg->caps->host.cpu->model) {
+                           xmlDesc, failIncompatible);
-        if (failIncompatible) {
-            virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s",
-                           _("cannot get host CPU capabilities"));
-        } else {
-            VIR_WARN("cannot get host CPU capabilities");
-            ret = VIR_CPU_COMPARE_INCOMPATIBLE;
-        }
-    } else {
-        ret = cpuCompareXML(cfg->caps->host.cpu, xmlDesc, failIncompatible);
-    }
 
     virObjectUnref(cfg);
     return ret;

src/qemu/qemu_driver.c

@@ -12812,18 +12812,8 @@ qemuConnectCompareCPU(virConnectPtr conn,
     if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
         goto cleanup;
 
-    if (!caps->host.cpu ||
+    ret = virCPUCompareXML(caps->host.arch, caps->host.cpu,
-        !caps->host.cpu->model) {
+                           xmlDesc, failIncompatible);
-        if (failIncompatible) {
-            virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s",
-                           _("cannot get host CPU capabilities"));
-        } else {
-            VIR_WARN("cannot get host CPU capabilities");
-            ret = VIR_CPU_COMPARE_INCOMPATIBLE;
-        }
-    } else {
-        ret = cpuCompareXML(caps->host.cpu, xmlDesc, failIncompatible);
-    }
 
  cleanup:
     virObjectUnref(caps);

tests/cputest.c

@@ -215,7 +215,7 @@ cpuTestCompare(const void *arg)
         !(cpu = cpuTestLoadXML(data->arch, data->name)))
         goto cleanup;
 
-    result = cpuCompare(host, cpu, false);
+    result = virCPUCompare(host->arch, host, cpu, false);
     if (data->result == VIR_CPU_COMPARE_ERROR)
         virResetLastError();
 
@@ -360,7 +360,7 @@ cpuTestBaseline(const void *arg)
     for (i = 0; i < ncpus; i++) {
         virCPUCompareResult cmp;
 
-        cmp = cpuCompare(cpus[i], baseline, false);
+        cmp = virCPUCompare(cpus[i]->arch, cpus[i], baseline, false);
         if (cmp != VIR_CPU_COMPARE_SUPERSET &&
             cmp != VIR_CPU_COMPARE_IDENTICAL) {
             VIR_TEST_VERBOSE("\nbaseline CPU is incompatible with CPU %zu\n",