On Thu, Jun 09, 2016 at 09:17:30 +0200, Peter Krempa wrote:
On Wed, Jun 08, 2016 at 14:41:38 +0200, Jiri Denemark wrote:
> 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.
...
> +/* Mask out irrelevant bits (R and Step) from processor
signature. */
> +#define SIGNATURE_MASK 0x0fff3ff0
This mask is not removing bits 12 and 13 corresponding to the processor
type, but we don't take them into account in our code. If that's
intentional please note it in the comment
Yes, it's intentional.
> +
> +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;
> +}
> +
> +
> @@ -1093,6 +1164,34 @@ 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) {
> + if (rc == -2 || (rc == 0 && !sigFamily)) {
Second part of the condition is a contradiction. Could you please
clarify your intent here?
Heh, I guess I should be more careful when reusing patches I hacked up a
year ago :-)
See v2.
Jirka