[Qemu-devel] [PATCH v2 0/2] Support CPUID signature for TCG

[Qemu-devel] [PATCH v2 0/2] Support CPUID signature for TCG

[Daniel P. Berrange]

This enables report of a signature in CPUID for the TCG
interpretor.

Changed in v2:

 - Rewrite the way we bounds check / cap the CPUID index
   to use a flat switch, instead of nested ifs (Eduardo)
 - Add a 'tcg-cpuid' property to allow it to be hidden
   (Eduardo)
 - Hide the TCG signature for old machine types
 - Force code to a no-op if tcg_enabled() is false (Eduardo)


NB, I did not introduce a general 'hypervisor-cpuid' property
to obsolete the existing 'kvm=off|on' -cpu property, since it
appears impossible to get the back compat semantics right,
as described in a previous reply.

Daniel P. Berrange (2):
  i386: rewrite way CPUID index is validated
  i386: expose "TCGTCGTCGTCG" in the 0x40000000 CPUID leaf

 include/hw/i386/pc.h |  5 ++++
 target/i386/cpu.c    | 73 +++++++++++++++++++++++++++++++++++++++-------------
 target/i386/cpu.h    |  1 +
 3 files changed, 61 insertions(+), 18 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH v2 1/2] i386: rewrite way CPUID index is validated

[Daniel P. Berrange]

Change the nested if statements into a flat switch, to make
it clearer what validation / capping is being performed on
different CPUID index values.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 target/i386/cpu.c | 43 +++++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 13c0985..3d5903c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2628,26 +2628,33 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
     uint32_t pkg_offset;
 
     /* test if maximum index reached */
-    if (index & 0x80000000) {
+    switch (index & 0xF0000000) {
+    case 0:
+        /* Intel documentation states that invalid EAX input will
+         * return the same information as EAX=cpuid_level
+         * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID)
+         */
+        if (index > env->cpuid_level) {
+            index = env->cpuid_level;
+        }
+        break;
+    case 0x80000000:
         if (index > env->cpuid_xlevel) {
-            if (env->cpuid_xlevel2 > 0) {
-                /* Handle the Centaur's CPUID instruction. */
-                if (index > env->cpuid_xlevel2) {
-                    index = env->cpuid_xlevel2;
-                } else if (index < 0xC0000000) {
-                    index = env->cpuid_xlevel;
-                }
-            } else {
-                /* Intel documentation states that invalid EAX input will
-                 * return the same information as EAX=cpuid_level
-                 * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID)
-                 */
-                index =  env->cpuid_level;
-            }
+            index = env->cpuid_xlevel;
         }
-    } else {
-        if (index > env->cpuid_level)
-            index = env->cpuid_level;
+        break;
+    case 0xC0000000:
+        if (index > env->cpuid_xlevel2) {
+            index = env->cpuid_xlevel2;
+        }
+        break;
+    default:
+        /* Intel documentation states that invalid EAX input will
+         * return the same information as EAX=cpuid_level
+         * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID)
+         */
+        index =  env->cpuid_level;
+        break;
     }
 
     switch(index) {
-- 
2.9.3

[Qemu-devel] [PATCH v2 2/2] i386: expose "TCGTCGTCGTCG" in the 0x40000000 CPUID leaf

[Daniel P. Berrange]

Currently when running KVM, we expose "KVMKVMKVM\0\0\0" in
the 0x40000000 CPUID leaf. Other hypervisors (VMWare,
HyperV, Xen, BHyve) all do the same thing, which leaves
TCG as the odd one out.

The CPUID signature is used by software to detect which
virtual environment they are running in and (potentially)
change behaviour in certain ways. For example, systemd
supports a ConditionVirtualization= setting in unit files.
The virt-what command can also report the virt type it is
running on

Currently both these apps have to resort to custom hacks
like looking for 'fw-cfg' entry in the /proc/device-tree
file to identify TCG.

This change thus proposes a signature "TCGTCGTCGTCG" to be
reported when running under TCG.

To hide this, the -cpu option tcg-cpuid=off can be used.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/hw/i386/pc.h |  5 +++++
 target/i386/cpu.c    | 30 ++++++++++++++++++++++++++++++
 target/i386/cpu.h    |  1 +
 3 files changed, 36 insertions(+)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index f278b3a..3aec60f 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -376,6 +376,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 #define PC_COMPAT_2_8 \
     HW_COMPAT_2_8 \
     {\
+        .driver   = TYPE_X86_CPU,\
+        .property = "tcg-cpuid",\
+        .value    = "off",\
+    },\
+    {\
         .driver   = "kvmclock",\
         .property = "x-mach-use-reliable-get-clock",\
         .value    = "off",\
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3d5903c..aa8e14c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2626,6 +2626,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
     X86CPU *cpu = x86_env_get_cpu(env);
     CPUState *cs = CPU(cpu);
     uint32_t pkg_offset;
+    uint32_t signature[3];
 
     /* test if maximum index reached */
     switch (index & 0xF0000000) {
@@ -2638,6 +2639,16 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             index = env->cpuid_level;
         }
         break;
+    case 0x40000000:
+        /* Not sure what we should do here. Intel and KVM
+         * documentation is not explicit about it, but it
+         * looks like KVM will return the highest _basic_
+         * leaf (env->cpuid_level) on that case.
+         */
+        if (index > 0x40000001) {
+            index = env->cpuid_level;
+        }
+        break;
     case 0x80000000:
         if (index > env->cpuid_xlevel) {
             index = env->cpuid_xlevel;
@@ -2879,6 +2890,24 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         }
         break;
     }
+    case 0x40000000:
+        /*
+         * CPUID code in kvm_arch_init_vcpu() ignores stuff
+         * set here, but we restrict to TCG none the less.
+         */
+        if (tcg_enabled() && cpu->expose_tcg) {
+            memcpy(signature, "TCGTCGTCGTCG", 12);
+            *eax = 0;
+            *ebx = signature[0];
+            *ecx = signature[1];
+            *edx = signature[2];
+        } else {
+            *eax = 0;
+            *ebx = 0;
+            *ecx = 0;
+            *edx = 0;
+        }
+        break;
     case 0x80000000:
         *eax = env->cpuid_xlevel;
         *ebx = env->cpuid_vendor1;
@@ -4020,6 +4049,7 @@ static Property x86_cpu_properties[] = {
     DEFINE_PROP_BOOL("kvm-no-smi-migration", X86CPU, kvm_no_smi_migration,
                      false),
     DEFINE_PROP_BOOL("vmware-cpuid-freq", X86CPU, vmware_cpuid_freq, true),
+    DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true),
     DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index c4602ca..c25f0ce 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1216,6 +1216,7 @@ struct X86CPU {
     bool check_cpuid;
     bool enforce_cpuid;
     bool expose_kvm;
+    bool expose_tcg;
     bool migratable;
     bool max_features; /* Enable all supported features automatically */
     uint32_t apic_id;
-- 
2.9.3

Re: [Qemu-devel] [PATCH v2 1/2] i386: rewrite way CPUID index is validated

[Eduardo Habkost]

On Fri, May 05, 2017 at 03:27:42PM +0100, Daniel P. Berrange wrote:
> Change the nested if statements into a flat switch, to make
> it clearer what validation / capping is being performed on
> different CPUID index values.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  target/i386/cpu.c | 43 +++++++++++++++++++++++++------------------
>  1 file changed, 25 insertions(+), 18 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 13c0985..3d5903c 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -2628,26 +2628,33 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>      uint32_t pkg_offset;
>  
>      /* test if maximum index reached */
> -    if (index & 0x80000000) {
> +    switch (index & 0xF0000000) {
> +    case 0:
> +        /* Intel documentation states that invalid EAX input will
> +         * return the same information as EAX=cpuid_level
> +         * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID)
> +         */
> +        if (index > env->cpuid_level) {
> +            index = env->cpuid_level;
> +        }
> +        break;

It looks like we can just move this code to the default branch,
and remove the "case 0:" branch.

> +    case 0x80000000:
>          if (index > env->cpuid_xlevel) {
> -            if (env->cpuid_xlevel2 > 0) {
> -                /* Handle the Centaur's CPUID instruction. */
> -                if (index > env->cpuid_xlevel2) {
> -                    index = env->cpuid_xlevel2;
                                    ^^^ [1]

> -                } else if (index < 0xC0000000) {
> -                    index = env->cpuid_xlevel;
                                    ^^^ [2]
> -                }
> -            } else {
> -                /* Intel documentation states that invalid EAX input will
> -                 * return the same information as EAX=cpuid_level
> -                 * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID)
> -                 */
> -                index =  env->cpuid_level;
                                 ^^^ [3]

> -            }
> +            index = env->cpuid_xlevel;
                            ^^^ [4]

Actually, CPUs do return the max _basic_ leaf (cpuid_level) even
when input EAX is larger than 0x80000000. (See [3])

...except for the existing code at [2], but:

* TCG doesn't support any of the Centaur features.
* KVM is not affected by [2] because CPUID
  limits are handled inside the KVM kernel code (and KVM returns
  CPUID[env->cpuid_level] on that case, see
  linux/arch/x86/kvm/cpuid.c:check_cpuid_limit())

In other words, [2] was dead code, and we only need to reproduce
behavior implemented by [3].

>          }
> -    } else {
> -        if (index > env->cpuid_level)
> -            index = env->cpuid_level;
> +        break;
> +    case 0xC0000000:
> +        if (index > env->cpuid_xlevel2) {
> +            index = env->cpuid_xlevel2;

This line reimplements [1], but (like [2]) [1] was dead code too.
But I suggest setting index=env->cpuid_level for consistency with
KVM code.

> +        }
> +        break;
> +    default:
> +        /* Intel documentation states that invalid EAX input will
> +         * return the same information as EAX=cpuid_level
> +         * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID)
> +         */
> +        index =  env->cpuid_level;
> +        break;
>      }

What about simplifying this even further:

    if (index > 0xC0000000) {
        limit = env->cpuid_xlevel2;
    } else if (index > 0x80000000) {
        limit = env->cpuid_xlevel;
    } else {
        limit = env->cpuid_level;
    }

    if (index > limit) {
        /* Intel documentation states that invalid EAX input will
         * return the same information as EAX=cpuid_level
         * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID)
         */
        index = env->cpuid_level;
    }

>  
>      switch(index) {
> -- 
> 2.9.3
> 

-- 
Eduardo