[libvirt] [PATCH 04/14] cpu: Move feature expansion out of cpuBaseline

Jiri Denemark jdenemar at redhat.com
Fri Mar 17 16:36:42 UTC 2017 

cpuBaseline is responsible for computing a baseline CPU while feature
expansion is done by virCPUExpandFeatures. The cpuBaselineXML wrapper
(used by hypervisor drivers to implement virConnectBaselineCPU API)
calls cpuBaseline followed by virCPUExpandFeatures if requested by
VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES flag.

The features in the three changed test files had to be sorted using
"sort -k 3" because virCPUExpandFeatures returns a sorted list of
features.

Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
---
 src/cpu/cpu.c                                    |  8 +++
 src/cpu/cpu_x86.c                                | 33 -----------
 tests/cputest.c                                  |  8 +++
 tests/cputestdata/x86_64-baseline-3-expanded.xml | 48 ++++++++--------
 tests/cputestdata/x86_64-baseline-4-expanded.xml | 68 +++++++++++------------
 tests/cputestdata/x86_64-baseline-5-expanded.xml | 70 ++++++++++++------------
 6 files changed, 109 insertions(+), 126 deletions(-)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 33581e5fe..d53a7ef2c 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -498,6 +498,10 @@ cpuBaselineXML(const char **xmlCPUs,
     size_t i;
 
     VIR_DEBUG("ncpus=%u, nmodels=%u", ncpus, nmodels);
+
+    virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES |
+                  VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL);
+
     if (xmlCPUs) {
         for (i = 0; i < ncpus; i++)
             VIR_DEBUG("xmlCPUs[%zu]=%s", i, NULLSTR(xmlCPUs[i]));
@@ -538,6 +542,10 @@ cpuBaselineXML(const char **xmlCPUs,
     if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels, flags)))
         goto error;
 
+    if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) &&
+        virCPUExpandFeatures(cpus[0]->arch, cpu) < 0)
+        goto error;
+
     cpustr = virCPUDefFormat(cpu, NULL, false);
 
  cleanup:
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 9c480398f..388102f35 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -967,28 +967,6 @@ x86FeaturesLoad(virCPUx86MapPtr map,
     return 0;
 }
 
-static int
-x86DataFromCPUFeatures(virCPUx86Data *data,
-                       virCPUDefPtr cpu,
-                       virCPUx86MapPtr map)
-{
-    size_t i;
-
-    for (i = 0; i < cpu->nfeatures; i++) {
-        virCPUx86FeaturePtr feature;
-        if (!(feature = x86FeatureFind(map, cpu->features[i].name))) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Unknown CPU feature %s"), cpu->features[i].name);
-            return -1;
-        }
-
-        if (x86DataAdd(data, &feature->data) < 0)
-            return -1;
-    }
-
-    return 0;
-}
-
 
 static virCPUx86ModelPtr
 x86ModelNew(void)
@@ -1948,17 +1926,6 @@ x86Decode(virCPUDefPtr cpu,
         }
     }
 
-    if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) {
-        if (x86DataCopy(&copy, &model->data) < 0 ||
-            x86DataFromCPUFeatures(&features, cpuModel, map) < 0)
-            goto cleanup;
-
-        x86DataSubtract(&copy, &features);
-        if (x86DataToCPUFeatures(cpuModel, VIR_CPU_FEATURE_REQUIRE,
-                                 &copy, map) < 0)
-            goto cleanup;
-    }
-
     if (vendor && VIR_STRDUP(cpu->vendor, vendor->name) < 0)
         goto cleanup;
 
diff --git a/tests/cputest.c b/tests/cputest.c
index 5e205c501..6396e8670 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -325,6 +325,14 @@ cpuTestBaseline(const void *arg)
         goto cleanup;
 
     baseline = cpuBaseline(cpus, ncpus, NULL, 0, data->flags);
+
+    if (baseline &&
+        (data->flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) &&
+        virCPUExpandFeatures(data->arch, baseline) < 0) {
+        virCPUDefFree(baseline);
+        baseline = NULL;
+    }
+
     if (data->result < 0) {
         virResetLastError();
         if (!baseline) {
diff --git a/tests/cputestdata/x86_64-baseline-3-expanded.xml b/tests/cputestdata/x86_64-baseline-3-expanded.xml
index f0c2273d8..82857e3d4 100644
--- a/tests/cputestdata/x86_64-baseline-3-expanded.xml
+++ b/tests/cputestdata/x86_64-baseline-3-expanded.xml
@@ -1,35 +1,35 @@
 <cpu mode='custom' match='exact'>
   <model fallback='forbid'>Westmere</model>
-  <feature policy='require' name='fpu'/>
-  <feature policy='require' name='de'/>
-  <feature policy='require' name='pse'/>
-  <feature policy='require' name='tsc'/>
-  <feature policy='require' name='msr'/>
-  <feature policy='require' name='pae'/>
-  <feature policy='require' name='mce'/>
-  <feature policy='require' name='cx8'/>
+  <feature policy='require' name='aes'/>
   <feature policy='require' name='apic'/>
-  <feature policy='require' name='sep'/>
-  <feature policy='require' name='mtrr'/>
-  <feature policy='require' name='pge'/>
-  <feature policy='require' name='mca'/>
-  <feature policy='require' name='cmov'/>
-  <feature policy='require' name='pat'/>
-  <feature policy='require' name='pse36'/>
   <feature policy='require' name='clflush'/>
-  <feature policy='require' name='mmx'/>
+  <feature policy='require' name='cmov'/>
+  <feature policy='require' name='cx16'/>
+  <feature policy='require' name='cx8'/>
+  <feature policy='require' name='de'/>
+  <feature policy='require' name='fpu'/>
   <feature policy='require' name='fxsr'/>
+  <feature policy='require' name='lahf_lm'/>
+  <feature policy='require' name='lm'/>
+  <feature policy='require' name='mca'/>
+  <feature policy='require' name='mce'/>
+  <feature policy='require' name='mmx'/>
+  <feature policy='require' name='msr'/>
+  <feature policy='require' name='mtrr'/>
+  <feature policy='require' name='nx'/>
+  <feature policy='require' name='pae'/>
+  <feature policy='require' name='pat'/>
+  <feature policy='require' name='pge'/>
+  <feature policy='require' name='pni'/>
+  <feature policy='require' name='popcnt'/>
+  <feature policy='require' name='pse'/>
+  <feature policy='require' name='pse36'/>
+  <feature policy='require' name='sep'/>
   <feature policy='require' name='sse'/>
   <feature policy='require' name='sse2'/>
-  <feature policy='require' name='pni'/>
-  <feature policy='require' name='ssse3'/>
-  <feature policy='require' name='cx16'/>
   <feature policy='require' name='sse4.1'/>
   <feature policy='require' name='sse4.2'/>
-  <feature policy='require' name='popcnt'/>
-  <feature policy='require' name='aes'/>
+  <feature policy='require' name='ssse3'/>
   <feature policy='require' name='syscall'/>
-  <feature policy='require' name='nx'/>
-  <feature policy='require' name='lm'/>
-  <feature policy='require' name='lahf_lm'/>
+  <feature policy='require' name='tsc'/>
 </cpu>
diff --git a/tests/cputestdata/x86_64-baseline-4-expanded.xml b/tests/cputestdata/x86_64-baseline-4-expanded.xml
index 7e4578e1a..e54eca026 100644
--- a/tests/cputestdata/x86_64-baseline-4-expanded.xml
+++ b/tests/cputestdata/x86_64-baseline-4-expanded.xml
@@ -1,46 +1,46 @@
 <cpu mode='custom' match='exact'>
   <model fallback='forbid'>Westmere</model>
   <vendor>Intel</vendor>
-  <feature policy='require' name='vme'/>
-  <feature policy='require' name='ss'/>
-  <feature policy='require' name='pclmuldq'/>
-  <feature policy='require' name='pcid'/>
-  <feature policy='require' name='x2apic'/>
-  <feature policy='require' name='tsc-deadline'/>
-  <feature policy='require' name='xsave'/>
-  <feature policy='require' name='osxsave'/>
-  <feature policy='require' name='avx'/>
-  <feature policy='require' name='hypervisor'/>
-  <feature policy='require' name='fpu'/>
-  <feature policy='require' name='de'/>
-  <feature policy='require' name='pse'/>
-  <feature policy='require' name='tsc'/>
-  <feature policy='require' name='msr'/>
-  <feature policy='require' name='pae'/>
-  <feature policy='require' name='mce'/>
-  <feature policy='require' name='cx8'/>
+  <feature policy='require' name='aes'/>
   <feature policy='require' name='apic'/>
-  <feature policy='require' name='sep'/>
-  <feature policy='require' name='mtrr'/>
-  <feature policy='require' name='pge'/>
-  <feature policy='require' name='mca'/>
-  <feature policy='require' name='cmov'/>
-  <feature policy='require' name='pat'/>
-  <feature policy='require' name='pse36'/>
+  <feature policy='require' name='avx'/>
   <feature policy='require' name='clflush'/>
-  <feature policy='require' name='mmx'/>
+  <feature policy='require' name='cmov'/>
+  <feature policy='require' name='cx16'/>
+  <feature policy='require' name='cx8'/>
+  <feature policy='require' name='de'/>
+  <feature policy='require' name='fpu'/>
   <feature policy='require' name='fxsr'/>
+  <feature policy='require' name='hypervisor'/>
+  <feature policy='require' name='lahf_lm'/>
+  <feature policy='require' name='lm'/>
+  <feature policy='require' name='mca'/>
+  <feature policy='require' name='mce'/>
+  <feature policy='require' name='mmx'/>
+  <feature policy='require' name='msr'/>
+  <feature policy='require' name='mtrr'/>
+  <feature policy='require' name='nx'/>
+  <feature policy='require' name='osxsave'/>
+  <feature policy='require' name='pae'/>
+  <feature policy='require' name='pat'/>
+  <feature policy='require' name='pcid'/>
+  <feature policy='require' name='pclmuldq'/>
+  <feature policy='require' name='pge'/>
+  <feature policy='require' name='pni'/>
+  <feature policy='require' name='popcnt'/>
+  <feature policy='require' name='pse'/>
+  <feature policy='require' name='pse36'/>
+  <feature policy='require' name='sep'/>
+  <feature policy='require' name='ss'/>
   <feature policy='require' name='sse'/>
   <feature policy='require' name='sse2'/>
-  <feature policy='require' name='pni'/>
-  <feature policy='require' name='ssse3'/>
-  <feature policy='require' name='cx16'/>
   <feature policy='require' name='sse4.1'/>
   <feature policy='require' name='sse4.2'/>
-  <feature policy='require' name='popcnt'/>
-  <feature policy='require' name='aes'/>
+  <feature policy='require' name='ssse3'/>
   <feature policy='require' name='syscall'/>
-  <feature policy='require' name='nx'/>
-  <feature policy='require' name='lm'/>
-  <feature policy='require' name='lahf_lm'/>
+  <feature policy='require' name='tsc'/>
+  <feature policy='require' name='tsc-deadline'/>
+  <feature policy='require' name='vme'/>
+  <feature policy='require' name='x2apic'/>
+  <feature policy='require' name='xsave'/>
 </cpu>
diff --git a/tests/cputestdata/x86_64-baseline-5-expanded.xml b/tests/cputestdata/x86_64-baseline-5-expanded.xml
index daef2a78f..2c1b40015 100644
--- a/tests/cputestdata/x86_64-baseline-5-expanded.xml
+++ b/tests/cputestdata/x86_64-baseline-5-expanded.xml
@@ -1,47 +1,47 @@
 <cpu mode='custom' match='exact'>
   <model fallback='allow'>SandyBridge</model>
   <vendor>Intel</vendor>
-  <feature policy='require' name='vme'/>
-  <feature policy='require' name='ss'/>
-  <feature policy='require' name='pcid'/>
-  <feature policy='require' name='osxsave'/>
-  <feature policy='require' name='hypervisor'/>
-  <feature policy='disable' name='rdtscp'/>
-  <feature policy='require' name='fpu'/>
-  <feature policy='require' name='de'/>
-  <feature policy='require' name='pse'/>
-  <feature policy='require' name='tsc'/>
-  <feature policy='require' name='msr'/>
-  <feature policy='require' name='pae'/>
-  <feature policy='require' name='mce'/>
-  <feature policy='require' name='cx8'/>
+  <feature policy='require' name='aes'/>
   <feature policy='require' name='apic'/>
-  <feature policy='require' name='sep'/>
-  <feature policy='require' name='mtrr'/>
-  <feature policy='require' name='pge'/>
-  <feature policy='require' name='mca'/>
-  <feature policy='require' name='cmov'/>
-  <feature policy='require' name='pat'/>
-  <feature policy='require' name='pse36'/>
+  <feature policy='require' name='avx'/>
   <feature policy='require' name='clflush'/>
-  <feature policy='require' name='mmx'/>
+  <feature policy='require' name='cmov'/>
+  <feature policy='require' name='cx16'/>
+  <feature policy='require' name='cx8'/>
+  <feature policy='require' name='de'/>
+  <feature policy='require' name='fpu'/>
   <feature policy='require' name='fxsr'/>
+  <feature policy='require' name='hypervisor'/>
+  <feature policy='require' name='lahf_lm'/>
+  <feature policy='require' name='lm'/>
+  <feature policy='require' name='mca'/>
+  <feature policy='require' name='mce'/>
+  <feature policy='require' name='mmx'/>
+  <feature policy='require' name='msr'/>
+  <feature policy='require' name='mtrr'/>
+  <feature policy='require' name='nx'/>
+  <feature policy='require' name='osxsave'/>
+  <feature policy='require' name='pae'/>
+  <feature policy='require' name='pat'/>
+  <feature policy='require' name='pcid'/>
+  <feature policy='require' name='pclmuldq'/>
+  <feature policy='require' name='pge'/>
+  <feature policy='require' name='pni'/>
+  <feature policy='require' name='popcnt'/>
+  <feature policy='require' name='pse'/>
+  <feature policy='require' name='pse36'/>
+  <feature policy='disable' name='rdtscp'/>
+  <feature policy='require' name='sep'/>
+  <feature policy='require' name='ss'/>
   <feature policy='require' name='sse'/>
   <feature policy='require' name='sse2'/>
-  <feature policy='require' name='pni'/>
-  <feature policy='require' name='pclmuldq'/>
-  <feature policy='require' name='ssse3'/>
-  <feature policy='require' name='cx16'/>
   <feature policy='require' name='sse4.1'/>
   <feature policy='require' name='sse4.2'/>
-  <feature policy='require' name='x2apic'/>
-  <feature policy='require' name='popcnt'/>
-  <feature policy='require' name='tsc-deadline'/>
-  <feature policy='require' name='aes'/>
-  <feature policy='require' name='xsave'/>
-  <feature policy='require' name='avx'/>
+  <feature policy='require' name='ssse3'/>
   <feature policy='require' name='syscall'/>
-  <feature policy='require' name='nx'/>
-  <feature policy='require' name='lm'/>
-  <feature policy='require' name='lahf_lm'/>
+  <feature policy='require' name='tsc'/>
+  <feature policy='require' name='tsc-deadline'/>
+  <feature policy='require' name='vme'/>
+  <feature policy='require' name='x2apic'/>
+  <feature policy='require' name='xsave'/>
 </cpu>
-- 
2.12.0

More information about the libvir-list mailing list