File 5a9221b9-cpu_x86-Use-signature-in-CPU-detection... of Package libvirt.10484

Overview Repositories Revisions Requests Users Attributes Meta

File 5a9221b9-cpu_x86-Use-signature-in-CPU-detection-code.patch of Package libvirt.10484 (Revision 18dfa78b5e94dabb268aa28fb155d3e8)

Currently displaying revision 18dfa78b5e94dabb268aa28fb155d3e8 , Show latest

From 5a9221b9af27d976707128eee06ad56faeb20ba9 Mon Sep 17 00:00:00 2001
From: Jiri Denemark <jdenemar@redhat.com>
Date: Thu, 25 Jun 2015 15:06:19 +0200
Subject: [PATCH] cpu_x86: Use signature in CPU detection code

Our current detection code uses just the number of CPU features which
need to be added/removed from the CPU model to fully describe the CPUID
data. The smallest number wins. But this may sometimes generate wrong
results as one can see from the fixed test cases. This patch modifies
the algorithm to prefer the CPU model with matching signature even if
this model results in a longer list of additional features.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/cpu/cpu_map.xml                                |  16 +++
 src/cpu/cpu_x86.c                                  | 148 +++++++++++++++++++--
 .../cputestdata/x86-cpuid-Core-i7-5600U-guest.xml  |  11 +-
 tests/cputestdata/x86-cpuid-Core2-E6850-guest.xml  |   5 +-
 tests/cputestdata/x86-cpuid-Core2-E6850-host.xml   |   5 +-
 tests/cputestdata/x86-cpuid-Xeon-5110-guest.xml    |   5 +-
 tests/cputestdata/x86-cpuid-Xeon-5110-host.xml     |   5 +-
 tests/cputestdata/x86-host+penryn-force-result.xml |   4 +-
 8 files changed, 176 insertions(+), 23 deletions(-)

Index: libvirt-1.2.18.4/src/cpu/cpu_map.xml
===================================================================
--- libvirt-1.2.18.4.orig/src/cpu/cpu_map.xml
+++ libvirt-1.2.18.4/src/cpu/cpu_map.xml
@@ -693,6 +693,7 @@
 
     
     <model name='Conroe'>
+      <signature family='6' model='15'/>
       <vendor name='Intel'/>
       <feature name='apic'/>
       <feature name='clflush'/>
@@ -724,6 +725,7 @@
     </model>
 
     <model name='Penryn'>
+      <signature family='6' model='23'/>
       <vendor name='Intel'/>
       <feature name='apic'/>
       <feature name='clflush'/>
@@ -757,6 +759,7 @@
     </model>
 
     <model name='Nehalem'>
+      <signature family='6' model='26'/>
       <vendor name='Intel'/>
       <feature name='apic'/>
       <feature name='clflush'/>
@@ -792,6 +795,7 @@
     </model>
 
     <model name='Westmere'>
+      <signature family='6' model='44'/>
       <vendor name='Intel'/>
       <feature name='aes'/>
       <feature name='apic'/>
@@ -828,6 +832,7 @@
     </model>
 
     <model name='SandyBridge'>
+      <signature family='6' model='42'/>
       <vendor name='Intel'/>
       <feature name='aes'/>
       <feature name='apic'/>
@@ -870,6 +875,7 @@
     </model>
 
     <model name='Haswell-noTSX'>
+      <signature family='6' model='60'/>
       <vendor name='Intel'/>
       <feature name='aes'/>
       <feature name='apic'/>
@@ -922,6 +928,7 @@
     </model>
 
     <model name='Haswell'>
+      <signature family='6' model='60'/>
       <vendor name='Intel'/>
       <feature name='aes'/>
       <feature name='apic'/>
@@ -976,6 +983,7 @@
     </model>
 
     <model name='Broadwell-noTSX'>
+      <signature family='6' model='61'/>
       <vendor name='Intel'/>
       <feature name='3dnowprefetch'/>
       <feature name='adx'/>
@@ -1032,6 +1040,7 @@
     </model>
 
     <model name='Broadwell'>
+      <signature family='6' model='61'/>
       <vendor name='Intel'/>
       <feature name='3dnowprefetch'/>
       <feature name='adx'/>
@@ -1153,6 +1162,7 @@
     </model>
 
     <model name='Opteron_G1'>
+      <signature family='15' model='6'/>
       <vendor name='AMD'/>
       <feature name='apic'/>
       <feature name='clflush'/>
@@ -1182,6 +1192,7 @@
     </model>
 
     <model name='Opteron_G2'>
+      <signature family='15' model='6'/>
       <vendor name='AMD'/>
       <feature name='apic'/>
       <feature name='clflush'/>
@@ -1215,6 +1226,7 @@
     </model>
 
     <model name='Opteron_G3'>
+      <signature family='15' model='6'/>
       <vendor name='AMD'/>
       <feature name='abm'/>
       <feature name='apic'/>
@@ -1253,6 +1265,7 @@
     </model>
 
     <model name='Opteron_G4'>
+      <signature family='21' model='1'/>
       <vendor name='AMD'/>
       <feature name='3dnowprefetch'/>
       <feature name='abm'/>
@@ -1301,6 +1314,7 @@
     </model>
 
     <model name='Opteron_G5'>
+      <signature family='21' model='2'/>
       <vendor name='AMD'/>
       <feature name='3dnowprefetch'/>
       <feature name='abm'/>
Index: libvirt-1.2.18.4/src/cpu/cpu_x86.c
===================================================================
--- libvirt-1.2.18.4.orig/src/cpu/cpu_x86.c
+++ libvirt-1.2.18.4/src/cpu/cpu_x86.c
@@ -80,6 +80,7 @@ static const struct x86_kvm_feature x86_
 struct x86_model {
     char *name;
     const struct x86_vendor *vendor;
+    uint32_t signature;
     virCPUx86Data *data;
 
     struct x86_model *next;
@@ -430,6 +431,80 @@ x86DataToVendor(virCPUx86Data *data,
 }
 
 
+static uint32_t
+x86MakeSignature(unsigned int family,
+                 unsigned int model)
+{
+    uint32_t sig = 0;
+
+    /*
+     * CPU signature (eax from 0x1 CPUID leaf):
+     *
+     * |31 .. 28|27 .. 20|19 .. 16|15 .. 14|13 .. 12|11 .. 8|7 .. 4|3 .. 0|
+     * |   R    | extFam | extMod |   R    | PType  |  Fam  | Mod  | Step |
+     *
+     * R        reserved
+     * extFam   extended family (valid only if Fam == 0xf)
+     * extMod   extended model
+     * PType    processor type
+     * Fam      family
+     * Mod      model
+     * Step     stepping
+     *
+     * family = eax[27:20] + eax[11:8]
+     * model = eax[19:16] << 4 + eax[7:4]
+     */
+
+    /* extFam */
+    if (family > 0xf) {
+        sig |= (family - 0xf) << 20;
+        family = 0xf;
+    }
+
+    /* extMod */
+    sig |= (model >> 4) << 16;
+
+    /* PType is always 0 */
+
+    /* Fam */
+    sig |= family << 8;
+
+    /* Mod */
+    sig |= (model & 0xf) << 4;
+
+    /* Step is irrelevant, it is used to distinguish different revisions
+     * of the same CPU model
+     */
+
+    return sig;
+}
+
+
+/* Mask out irrelevant bits (R and Step) from processor signature. */
+#define SIGNATURE_MASK  0x0fff3ff0
+
+static uint32_t
+x86DataToSignature(const virCPUx86Data *data)
+{
+    virCPUx86CPUID *cpuid;
+
+    if (!(cpuid = x86DataCpuid(data, 0x1)))
+        return 0;
+
+    return cpuid->eax & SIGNATURE_MASK;
+}
+
+
+static int
+x86DataAddSignature(virCPUx86Data *data,
+                    uint32_t signature)
+{
+    virCPUx86CPUID cpuid = { .function = 0x1, .eax = signature };
+
+    return virCPUx86DataAddCPUID(data, &cpuid);
+}
+
+
 static virCPUDefPtr
 x86DataToCPU(const virCPUx86Data *data,
              const struct x86_model *model,
@@ -844,6 +919,7 @@ x86ModelCopy(const struct x86_model *mod
     }
 
     copy->vendor = model->vendor;
+    copy->signature = model->signature;
 
     return copy;
 }
@@ -1044,6 +1120,30 @@ x86ModelLoad(xmlXPathContextPtr ctxt,
             goto error;
     }
 
+    if (virXPathBoolean("boolean(./signature)", ctxt)) {
+        unsigned int sigFamily = 0;
+        unsigned int sigModel = 0;
+        int rc;
+
+        rc = virXPathUInt("string(./signature/@family)", ctxt, &sigFamily);
+        if (rc < 0 || sigFamily == 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Invalid CPU signature family in model %s"),
+                           model->name);
+            goto ignore;
+        }
+
+        rc = virXPathUInt("string(./signature/@model)", ctxt, &sigModel);
+        if (rc < 0 || sigModel == 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Invalid CPU signature model in model %s"),
+                           model->name);
+            goto ignore;
+        }
+
+        model->signature = x86MakeSignature(sigFamily, sigModel);
+    }
+
     if (virXPathBoolean("boolean(./vendor)", ctxt)) {
         vendor = virXPathString("string(./vendor/@name)", ctxt);
         if (!vendor) {
@@ -1469,6 +1569,9 @@ x86Compute(virCPUDefPtr host,
         if ((guest_model = x86ModelCopy(host_model)) == NULL)
             goto error;
 
+        if (x86DataAddSignature(guest_model->data, host_model->signature) < 0)
+            goto error;
+
         if (cpu->type == VIR_CPU_TYPE_GUEST
             && cpu->match == VIR_CPU_MATCH_EXACT)
             x86DataSubtract(guest_model->data, diff->data);
@@ -1539,16 +1642,19 @@ x86GuestData(virCPUDefPtr host,
 
 
 /*
- * Checks whether cpuCandidate is a better fit for the CPU data than the
- * currently selected one from cpuCurrent.
+ * Checks whether a candidate model is a better fit for the CPU data than the
+ * current model.
  *
- * Returns 0 if cpuCurrent is better,
- *         1 if cpuCandidate is better,
- *         2 if cpuCandidate is the best one (search should stop now).
+ * Returns 0 if current is better,
+ *         1 if candidate is better,
+ *         2 if candidate is the best one (search should stop now).
  */
 static int
-x86DecodeUseCandidate(virCPUDefPtr cpuCurrent,
+x86DecodeUseCandidate(const struct x86_model *current,
+                      virCPUDefPtr cpuCurrent,
+                      const struct x86_model *candidate,
                       virCPUDefPtr cpuCandidate,
+                      uint32_t signature,
                       const char *preferred,
                       bool checkPolicy)
 {
@@ -1568,9 +1674,26 @@ x86DecodeUseCandidate(virCPUDefPtr cpuCu
     if (!cpuCurrent)
         return 1;
 
+    /* Ideally we want to select a model with family/model equal to
+     * family/model of the real CPU. Once we found such model, we only
+     * consider candidates with matching family/model.
+     */
+    if (signature &&
+        current->signature == signature &&
+        candidate->signature != signature)
+        return 0;
+
     if (cpuCurrent->nfeatures > cpuCandidate->nfeatures)
         return 1;
 
+    /* Prefer a candidate with matching signature even though it would
+     * result in longer list of features.
+     */
+    if (signature &&
+        candidate->signature == signature &&
+        current->signature != signature)
+        return 1;
+
     return 0;
 }
 
@@ -1587,10 +1710,11 @@ x86Decode(virCPUDefPtr cpu,
     const struct x86_map *map;
     const struct x86_model *candidate;
     virCPUDefPtr cpuCandidate;
+    const struct x86_model *model = NULL;
     virCPUDefPtr cpuModel = NULL;
     virCPUx86Data *copy = NULL;
     virCPUx86Data *features = NULL;
-    const virCPUx86Data *cpuData = NULL;
+    uint32_t signature;
     size_t i;
     int rc;
 
@@ -1600,6 +1724,8 @@ x86Decode(virCPUDefPtr cpu,
     if (!data || !(map = virCPUx86GetMap()))
         return -1;
 
+    signature = x86DataToSignature(data);
+
     candidate = map->models;
     while (candidate != NULL) {
         if (!cpuModelIsAllowed(candidate->name, models, nmodels)) {
@@ -1634,11 +1760,13 @@ x86Decode(virCPUDefPtr cpu,
             goto next;
         }
 
-        if ((rc = x86DecodeUseCandidate(cpuModel, cpuCandidate, preferred,
+        if ((rc = x86DecodeUseCandidate(model, cpuModel,
+                                        candidate, cpuCandidate,
+                                        signature, preferred,
                                         cpu->type == VIR_CPU_TYPE_HOST))) {
             virCPUDefFree(cpuModel);
             cpuModel = cpuCandidate;
-            cpuData = candidate->data;
+            model = candidate;
             if (rc == 2)
                 break;
         } else {
@@ -1671,7 +1799,7 @@ x86Decode(virCPUDefPtr cpu,
     }
 
     if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) {
-        if (!(copy = x86DataCopy(cpuData)) ||
+        if (!(copy = x86DataCopy(model->data)) ||
             !(features = x86DataFromCPUFeatures(cpuModel, map)))
             goto out;
 
Index: libvirt-1.2.18.4/tests/cputestdata/x86-host+nehalem-force-result.xml
===================================================================
--- libvirt-1.2.18.4.orig/tests/cputestdata/x86-host+nehalem-force-result.xml
+++ libvirt-1.2.18.4/tests/cputestdata/x86-host+nehalem-force-result.xml
@@ -1,4 +1,6 @@
 <cpu mode='custom' match='exact'>
   <arch>x86_64</arch>
-  <model fallback='allow'>Nehalem</model>
+  <model fallback='allow'>Penryn</model>
+  <feature policy='require' name='popcnt'/>
+  <feature policy='require' name='sse4.2'/>
 </cpu>

Places

File 5a9221b9-cpu_x86-Use-signature-in-CPU-detection-code.patch of Package libvirt.10484 (Revision 18dfa78b5e94dabb268aa28fb155d3e8)

Places