From 5a9221b9af27d976707128eee06ad56faeb20ba9 Mon Sep 17 00:00:00 2001 From: Jiri Denemark 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 --- src/cpu/cpu_map.xml | 16 ++ src/cpu/cpu_x86.c | 148 ++++++++++++++++-- .../x86-cpuid-Core-i7-5600U-guest.xml | 11 +- .../x86-cpuid-Core2-E6850-guest.xml | 5 +- .../x86-cpuid-Core2-E6850-host.xml | 5 +- .../cputestdata/x86-cpuid-Xeon-5110-guest.xml | 5 +- .../cputestdata/x86-cpuid-Xeon-5110-host.xml | 5 +- .../x86-host+penryn-force-result.xml | 4 +- 8 files changed, 176 insertions(+), 23 deletions(-) diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index 0918f751e6..bf2dfc6a76 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -717,6 +717,7 @@ + @@ -748,6 +749,7 @@ + @@ -781,6 +783,7 @@ + @@ -816,6 +819,7 @@ + @@ -852,6 +856,7 @@ + @@ -894,6 +899,7 @@ + @@ -942,6 +948,7 @@ + @@ -994,6 +1001,7 @@ + @@ -1048,6 +1056,7 @@ + @@ -1104,6 +1113,7 @@ + @@ -1162,6 +1172,7 @@ + @@ -1292,6 +1303,7 @@ + @@ -1321,6 +1333,7 @@ + @@ -1354,6 +1367,7 @@ + @@ -1392,6 +1406,7 @@ + @@ -1440,6 +1455,7 @@ + diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 1d0e7a7d7c..d9646eb1c3 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -135,6 +135,7 @@ typedef virCPUx86Model *virCPUx86ModelPtr; struct _virCPUx86Model { char *name; virCPUx86VendorPtr vendor; + uint32_t signature; virCPUx86Data data; }; @@ -493,6 +494,81 @@ x86DataToVendor(const 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 leaf1 = { .eax_in = 0x1 }; + virCPUx86CPUID *cpuid; + + if (!(cpuid = x86DataCpuid(data, &leaf1))) + return 0; + + return cpuid->eax & SIGNATURE_MASK; +} + + +static int +x86DataAddSignature(virCPUx86Data *data, + uint32_t signature) +{ + virCPUx86CPUID cpuid = { .eax_in = 0x1, .eax = signature }; + + return virCPUx86DataAddCPUID(data, &cpuid); +} + + static virCPUDefPtr x86DataToCPU(const virCPUx86Data *data, virCPUx86ModelPtr model, @@ -898,6 +974,7 @@ x86ModelCopy(virCPUx86ModelPtr model) } copy->vendor = model->vendor; + copy->signature = model->signature; return copy; } @@ -1093,6 +1170,30 @@ x86ModelParse(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 cleanup; + } + + 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 cleanup; + } + + model->signature = x86MakeSignature(sigFamily, sigModel); + } + if (virXPathBoolean("boolean(./vendor)", ctxt)) { vendor = virXPathString("string(./vendor/@name)", ctxt); if (!vendor) { @@ -1480,6 +1581,9 @@ x86Compute(virCPUDefPtr host, &host_model->vendor->cpuid) < 0) 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); @@ -1549,16 +1653,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(virCPUx86ModelPtr current, + virCPUDefPtr cpuCurrent, + virCPUx86ModelPtr candidate, virCPUDefPtr cpuCandidate, + uint32_t signature, const char *preferred, bool checkPolicy) { @@ -1578,9 +1685,26 @@ x86DecodeUseCandidate(virCPUDefPtr cpuCurrent, 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; } @@ -1597,11 +1721,12 @@ x86Decode(virCPUDefPtr cpu, virCPUx86MapPtr map; virCPUx86ModelPtr candidate; virCPUDefPtr cpuCandidate; + virCPUx86ModelPtr model = NULL; virCPUDefPtr cpuModel = NULL; virCPUx86Data copy = VIR_CPU_X86_DATA_INIT; virCPUx86Data features = VIR_CPU_X86_DATA_INIT; - const virCPUx86Data *cpuData = NULL; virCPUx86VendorPtr vendor; + uint32_t signature; ssize_t i; int rc; @@ -1612,6 +1737,7 @@ x86Decode(virCPUDefPtr cpu, return -1; vendor = x86DataToVendor(data, map); + signature = x86DataToSignature(data); /* Walk through the CPU models in reverse order to check newest * models first. @@ -1650,11 +1776,13 @@ x86Decode(virCPUDefPtr cpu, goto cleanup; cpuCandidate->type = cpu->type; - 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 { @@ -1686,7 +1814,7 @@ x86Decode(virCPUDefPtr cpu, } if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) { - if (x86DataCopy(©, cpuData) < 0 || + if (x86DataCopy(©, &model->data) < 0 || x86DataFromCPUFeatures(&features, cpuModel, map) < 0) goto cleanup; diff --git a/tests/cputestdata/x86-cpuid-Core-i7-5600U-guest.xml b/tests/cputestdata/x86-cpuid-Core-i7-5600U-guest.xml index 78bc0a64a8..cd7b4bb8e2 100644 --- a/tests/cputestdata/x86-cpuid-Core-i7-5600U-guest.xml +++ b/tests/cputestdata/x86-cpuid-Core-i7-5600U-guest.xml @@ -1,7 +1,8 @@ x86_64 - Skylake-Client + Broadwell Intel + @@ -18,10 +19,12 @@ + + + + + - - - diff --git a/tests/cputestdata/x86-cpuid-Core2-E6850-guest.xml b/tests/cputestdata/x86-cpuid-Core2-E6850-guest.xml index d9df03e683..dfcbe24dad 100644 --- a/tests/cputestdata/x86-cpuid-Core2-E6850-guest.xml +++ b/tests/cputestdata/x86-cpuid-Core2-E6850-guest.xml @@ -1,7 +1,8 @@ x86_64 - core2duo + Conroe Intel + @@ -9,6 +10,7 @@ + @@ -17,5 +19,4 @@ - diff --git a/tests/cputestdata/x86-cpuid-Core2-E6850-host.xml b/tests/cputestdata/x86-cpuid-Core2-E6850-host.xml index c5ca1cb16c..e7ddc39ceb 100644 --- a/tests/cputestdata/x86-cpuid-Core2-E6850-host.xml +++ b/tests/cputestdata/x86-cpuid-Core2-E6850-host.xml @@ -1,7 +1,8 @@ x86_64 - core2duo + Conroe Intel + @@ -9,6 +10,7 @@ + @@ -17,5 +19,4 @@ - diff --git a/tests/cputestdata/x86-cpuid-Xeon-5110-guest.xml b/tests/cputestdata/x86-cpuid-Xeon-5110-guest.xml index 0b9e3f6cf2..28d112b341 100644 --- a/tests/cputestdata/x86-cpuid-Xeon-5110-guest.xml +++ b/tests/cputestdata/x86-cpuid-Xeon-5110-guest.xml @@ -1,7 +1,8 @@ x86_64 - core2duo + Conroe Intel + @@ -9,6 +10,7 @@ + @@ -16,5 +18,4 @@ - diff --git a/tests/cputestdata/x86-cpuid-Xeon-5110-host.xml b/tests/cputestdata/x86-cpuid-Xeon-5110-host.xml index 88c61c40ff..ca3a84cc5c 100644 --- a/tests/cputestdata/x86-cpuid-Xeon-5110-host.xml +++ b/tests/cputestdata/x86-cpuid-Xeon-5110-host.xml @@ -1,7 +1,8 @@ x86_64 - core2duo + Conroe Intel + @@ -9,6 +10,7 @@ + @@ -16,5 +18,4 @@ - diff --git a/tests/cputestdata/x86-host+penryn-force-result.xml b/tests/cputestdata/x86-host+penryn-force-result.xml index 000bc0d61b..ef0a2c0504 100644 --- a/tests/cputestdata/x86-host+penryn-force-result.xml +++ b/tests/cputestdata/x86-host+penryn-force-result.xml @@ -1,4 +1,6 @@ x86_64 - Nehalem + Penryn + + -- 2.39.5